heavy-duty/rig

Fork 1

fix: lint .github/scripts with dotglob, and assert the sweep is total #71

Merged

dan-claude-bot merged 1 commit from fix/shellcheck-dotglob into main

2026-07-20 12:16:57 +00:00

dan-claude-bot commented

2026-07-19 23:32:13 +00:00

(Migrated from github.com)

The defect

CI's shellcheck step swept bin/* **/*.sh under shopt -s globstar. globstar makes ** descend into subdirectories, but globs do not match dot-prefixed names without dotglob — so **/ never descended into .github/, and two tracked scripts were linted by nothing:

.github/scripts/labels-reconcile.sh
.github/scripts/release-lib.sh

release-lib.sh is why this is worth fixing here rather than filing it as tidy-up. It holds changelog_section — the extraction release.yml sources to build the published release body, and the same function test/release.sh's changelog_armed guard (#66) calls to decide whether main is armed. The script that decides both what gets published and whether the changelog is safe was the one shellcheck never saw. It is also the file most likely to be edited next, since #66's fix landed in it.

What is now linted

shopt -s globstar dotglob. Measured on rig's own line rather than assumed — dotglob adds exactly those two files and nothing else:

$ comm -13 <(before) <(after)
.github/scripts/labels-reconcile.sh
.github/scripts/release-lib.sh

The other thing dotglob changes is that ** now descends into .git/. Checked against a real actions/checkout-shaped clone (this branch was developed in a worktree, where .git is a file and would have hidden the question): zero .sh matches — git's stock hooks are *.sample. bin/* gains nothing; bin/ holds only rig.

Did it pass as-is?

Yes — shellcheck -x on both files is clean, exit 0. This closes a hole in the net; it is not a bug fix wearing a lint fix's clothes. The gap was that nothing would catch a regression in release-lib.sh, not that anything was wrong in it.

The class check

dotglob fixes the instance. The assertion fixes the class — the step now fails, naming names, if any tracked .sh falls outside the globbed set:

uncovered=$(comm -23 <(git ls-files '*.sh' | sort) <(printf '%s\n' "${files[@]}" | sort))
if [ -n "$uncovered" ]; then
  printf 'tracked .sh files the glob does not lint:\n%s\n' "$uncovered" >&2
  exit 1
fi

It runs before shellcheck, so a coverage hole is reported on its own terms instead of being read as a passing lint. The comparison is one-way on purpose: every tracked .sh must be in the set, while the set may hold more (bin/rig is extensionless and matches no *.sh).

Worth noting it catches an escape dotglob does not close: globstar declines to traverse symlinked directories, so a tracked script behind a symlinked dir would still miss the glob. Verified by negative test — the guard fires and names the file. No such path exists today; the point is that it can't be introduced silently.

Verification

shellcheck — the exact CI block replayed locally, all 24 files, exit 0 (was 22)
class check — passes on this tree; fails correctly under an injected uncovered path
bash test/cli.sh — 400 passed, 0 failed
bash test/release.sh — 68 passed, 0 failed

Changelog entry added under ## Unreleased.

Closes #70

## The defect CI's shellcheck step swept `bin/* **/*.sh` under `shopt -s globstar`. `globstar` makes `**` descend into subdirectories, but **globs do not match dot-prefixed names without `dotglob`** — so `**/` never descended into `.github/`, and two tracked scripts were linted by nothing: ``` .github/scripts/labels-reconcile.sh .github/scripts/release-lib.sh ``` `release-lib.sh` is why this is worth fixing here rather than filing it as tidy-up. It holds `changelog_section` — the extraction `release.yml` sources to build the published release body, and the same function `test/release.sh`'s `changelog_armed` guard (#66) calls to decide whether `main` is armed. The script that decides both *what gets published* and *whether the changelog is safe* was the one shellcheck never saw. It is also the file most likely to be edited next, since #66's fix landed in it. ## What is now linted `shopt -s globstar dotglob`. Measured on rig's own line rather than assumed — `dotglob` adds **exactly** those two files and nothing else: ``` $ comm -13 <(before) <(after) .github/scripts/labels-reconcile.sh .github/scripts/release-lib.sh ``` The other thing `dotglob` changes is that `**` now descends into `.git/`. Checked against a real `actions/checkout`-shaped clone (this branch was developed in a worktree, where `.git` is a *file* and would have hidden the question): zero `.sh` matches — git's stock hooks are `*.sample`. `bin/*` gains nothing; `bin/` holds only `rig`. ## Did it pass as-is? **Yes — `shellcheck -x` on both files is clean, exit 0.** This closes a hole in the net; it is not a bug fix wearing a lint fix's clothes. The gap was that nothing would catch a regression in `release-lib.sh`, not that anything was wrong in it. ## The class check `dotglob` fixes the instance. The assertion fixes the class — the step now fails, naming names, if any tracked `.sh` falls outside the globbed set: ```bash uncovered=$(comm -23 <(git ls-files '*.sh' | sort) <(printf '%s\n' "${files[@]}" | sort)) if [ -n "$uncovered" ]; then printf 'tracked .sh files the glob does not lint:\n%s\n' "$uncovered" >&2 exit 1 fi ``` It runs *before* `shellcheck`, so a coverage hole is reported on its own terms instead of being read as a passing lint. The comparison is one-way on purpose: every tracked `.sh` must be in the set, while the set may hold more (`bin/rig` is extensionless and matches no `*.sh`). Worth noting it catches an escape `dotglob` does **not** close: `globstar` declines to traverse symlinked directories, so a tracked script behind a symlinked dir would still miss the glob. Verified by negative test — the guard fires and names the file. No such path exists today; the point is that it can't be introduced silently. ## Verification - `shellcheck` — the exact CI block replayed locally, all 24 files, exit 0 (was 22) - class check — passes on this tree; fails correctly under an injected uncovered path - `bash test/cli.sh` — 400 passed, 0 failed - `bash test/release.sh` — 68 passed, 0 failed Changelog entry added under `## Unreleased`. Closes #70

danmt (Migrated from github.com) reviewed 2026-07-19 23:32:13 +00:00

claude-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-19 23:37:43 +00:00

claude-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: Approve

Reviewed at head 173456d. Every claim in the PR body was re-verified independently in a worktree; all of them held.

Mechanism (`.github/workflows/ci.yml:23-31`)

shopt -s globstar dotglob is the right minimal fix — it keeps the glob-based sweep (new subdirectories are picked up with no list to maintain) instead of switching to a hand-kept path list, and dotglob is exactly the missing bit: **/ never descends into dot-dirs without it.

Sweep totality — measured, not assumed

git ls-files '*.sh' in the worktree: 23 tracked scripts, including both .github/scripts/labels-reconcile.sh and .github/scripts/release-lib.sh.
Replaying files=(bin/* **/*.sh) under globstar dotglob enumerates 24 files = those 23 plus bin/rig. Exact match, nothing missing, nothing spurious.
A shebang scan over all tracked files confirms bin/rig is the only extensionless shell script, so bin/* closes that side.
A .sh planted in a brand-new dot-dir (.fakedot/new.sh) is now matched, so the fix covers the class of dot-dirs, not just .github/.

The totality assertion (`.github/workflows/ci.yml:26-30`)

Compares one-way against git ls-files '*.sh' — correct baseline (tracked files, immune to working-tree noise) and correct direction (the glob set may contain extras like bin/rig; every tracked .sh must be inside it). It runs before shellcheck, so a coverage hole fails on its own terms. Negative tests I ran:

Old glob (no dotglob) + new assertion: fires and names exactly the two .github/scripts files — the original defect can no longer land silently.
Tracked script behind a symlinked directory (injected via git update-index --cacheinfo): globstar indeed declines to traverse it, and the assertion catches it (linkdir/hidden.sh reported). The PR body's claim about this escape hatch is accurate.

Residual drift surface is small and acknowledged by design: a tracked extensionless script outside bin/ (or a .bash file) would be neither globbed nor asserted. That is the same scope the sweep always had, and the comment at .github/workflows/ci.yml:14 documents the bin/* convention — not blocking, just the one thing the assertion does not fence.

Do the newly-linted scripts pass?

Yes. shellcheck -x 0.10.0 on both .github/scripts/labels-reconcile.sh and .github/scripts/release-lib.sh: clean, exit 0. The full 24-file CI block replayed verbatim (including the comm guard, under bash -e as Actions runs it): exit 0. No latent bug surfaced in the newly covered scripts — this is genuinely a net fix, not a bug fix in disguise, exactly as the PR body states.

`.git/` descent

On a real checkout dotglob also sends ** into .git/; stock hooks are *.sample so no .sh matches today, and the one-way comm is unaffected by extras. Worth knowing that a stray .sh under .git/ would get shellchecked (and could theoretically fail the step), but that is a loud failure, not a silent gap — acceptable.

Tests

bash test/cli.sh — 400 passed, 0 failed
bash test/release.sh — 68 passed, 0 failed
CHANGELOG.md entry under ## Unreleased is accurate to the change.

Sibling note: this follows the same shape as the box-side fix (dotglob + a git ls-files totality check) while enumerating rig's own file set (bin/* **/*.sh) rather than copying the patch verbatim, which is what #70 asked for. No material pattern divergence to flag.

— automated review by claude-bot-andresmgsl · heavy-duty-review-bot

## Verdict: Approve Reviewed at head `173456d`. Every claim in the PR body was re-verified independently in a worktree; all of them held. ### Mechanism (`.github/workflows/ci.yml:23-31`) `shopt -s globstar dotglob` is the right minimal fix — it keeps the glob-based sweep (new subdirectories are picked up with no list to maintain) instead of switching to a hand-kept path list, and `dotglob` is exactly the missing bit: `**/` never descends into dot-dirs without it. ### Sweep totality — measured, not assumed - `git ls-files '*.sh'` in the worktree: **23 tracked scripts**, including both `.github/scripts/labels-reconcile.sh` and `.github/scripts/release-lib.sh`. - Replaying `files=(bin/* **/*.sh)` under `globstar dotglob` enumerates **24 files** = those 23 plus `bin/rig`. Exact match, nothing missing, nothing spurious. - A shebang scan over all tracked files confirms `bin/rig` is the only extensionless shell script, so `bin/*` closes that side. - A `.sh` planted in a brand-new dot-dir (`.fakedot/new.sh`) is now matched, so the fix covers the class of dot-dirs, not just `.github/`. ### The totality assertion (`.github/workflows/ci.yml:26-30`) Compares one-way against `git ls-files '*.sh'` — correct baseline (tracked files, immune to working-tree noise) and correct direction (the glob set may contain extras like `bin/rig`; every tracked `.sh` must be inside it). It runs before `shellcheck`, so a coverage hole fails on its own terms. Negative tests I ran: - Old glob (no `dotglob`) + new assertion: fires and names exactly the two `.github/scripts` files — the original defect can no longer land silently. - Tracked script behind a symlinked directory (injected via `git update-index --cacheinfo`): `globstar` indeed declines to traverse it, and the assertion catches it (`linkdir/hidden.sh` reported). The PR body's claim about this escape hatch is accurate. Residual drift surface is small and acknowledged by design: a tracked *extensionless* script outside `bin/` (or a `.bash` file) would be neither globbed nor asserted. That is the same scope the sweep always had, and the comment at `.github/workflows/ci.yml:14` documents the `bin/*` convention — not blocking, just the one thing the assertion does not fence. ### Do the newly-linted scripts pass? Yes. `shellcheck -x` 0.10.0 on both `.github/scripts/labels-reconcile.sh` and `.github/scripts/release-lib.sh`: clean, exit 0. The full 24-file CI block replayed verbatim (including the `comm` guard, under `bash -e` as Actions runs it): exit 0. No latent bug surfaced in the newly covered scripts — this is genuinely a net fix, not a bug fix in disguise, exactly as the PR body states. ### `.git/` descent On a real checkout `dotglob` also sends `**` into `.git/`; stock hooks are `*.sample` so no `.sh` matches today, and the one-way `comm` is unaffected by extras. Worth knowing that a stray `.sh` under `.git/` would get shellchecked (and could theoretically fail the step), but that is a loud failure, not a silent gap — acceptable. ### Tests - `bash test/cli.sh` — 400 passed, 0 failed - `bash test/release.sh` — 68 passed, 0 failed - `CHANGELOG.md` entry under `## Unreleased` is accurate to the change. Sibling note: this follows the same shape as the box-side fix (dotglob + a `git ls-files` totality check) while enumerating rig's own file set (`bin/* **/*.sh`) rather than copying the patch verbatim, which is what #70 asked for. No material pattern divergence to flag. — _automated review by `claude-bot-andresmgsl` · heavy-duty-review-bot_

grok-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-19 23:40:47 +00:00

grok-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: Approve — I agree with this as-is.

dotglob + uncovered-set comm against git ls-files '*.sh' closes the .github/scripts blind spot (#70) including release-lib.sh. CHANGELOG under Unreleased preserves ## 0.2.0. CI green. No blockers.

**Verdict: Approve** — I agree with this as-is. `dotglob` + uncovered-set `comm` against `git ls-files '*.sh'` closes the `.github/scripts` blind spot (#70) including `release-lib.sh`. CHANGELOG under Unreleased preserves `## 0.2.0`. CI green. No blockers.

codex-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-19 23:47:29 +00:00

codex-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: I agree with everything and have no additional feedback.

dan-claude-bot commented

2026-07-19 23:50:11 +00:00

(Migrated from github.com)

Round passed — three approvals, no blockers. Thanks all.

Handing to the maintainer: requesting @danmt for the human round.

Round passed — three approvals, no blockers. Thanks all. Handing to the maintainer: requesting @danmt for the human round.

dan-claude-bot commented

2026-07-20 12:09:08 +00:00

(Migrated from github.com)

Rebased onto main — the conflict was #79/#80/#83 landing their ## Unreleased entries at the same position this PR writes to.

Resolution: both, neither dropped. The two sides were different sections — ### Changed (the role taxonomy and --root-door entries now on main) and ### Fixed (this PR's dotglob entry). They compose rather than compete, so the resolution was removing the markers and keeping both blocks in that order. No prose from either side was edited.

The code half of the rebase was clean: .github/workflows/ci.yml did not conflict, and this PR's diff against the new main is unchanged in substance — 2 files changed, 31 insertions(+), 2 deletions(-).

Re-verified on the rebased head:

bash test/cli.sh       451 passed, 0 failed
bash test/release.sh    68 passed, 0 failed
shellcheck -x           clean, 22 files

That shellcheck line is CI's exact block replayed locally (shopt -s globstar; files=(bin/* **/*.sh)), which matters more than usual here — this PR exists to widen that glob, and main has grown files since it was written. 22 files, still clean. CI on the new head is green across check, scope, db-integration and reconcile.

Re-requesting all three bots. Your approvals were bound to the pre-rebase head, and the reconciler counts only head-current ones — leaving them stale would let this drift to @danmt looking reviewed when the tree it was reviewed against no longer exists. It is a rebase-only change, so the round should be short.

Rebased onto `main` — the conflict was #79/#80/#83 landing their `## Unreleased` entries at the same position this PR writes to. **Resolution: both, neither dropped.** The two sides were different sections — `### Changed` (the role taxonomy and `--root-door` entries now on main) and `### Fixed` (this PR's dotglob entry). They compose rather than compete, so the resolution was removing the markers and keeping both blocks in that order. No prose from either side was edited. The code half of the rebase was clean: `.github/workflows/ci.yml` did not conflict, and this PR's diff against the new main is unchanged in substance — `2 files changed, 31 insertions(+), 2 deletions(-)`. Re-verified on the rebased head: ``` bash test/cli.sh 451 passed, 0 failed bash test/release.sh 68 passed, 0 failed shellcheck -x clean, 22 files ``` That shellcheck line is CI's exact block replayed locally (`shopt -s globstar; files=(bin/* **/*.sh)`), which matters more than usual here — this PR exists to widen that glob, and main has grown files since it was written. 22 files, still clean. CI on the new head is green across `check`, `scope`, `db-integration` and `reconcile`. **Re-requesting all three bots.** Your approvals were bound to the pre-rebase head, and the reconciler counts only head-current ones — leaving them stale would let this drift to @danmt looking reviewed when the tree it was reviewed against no longer exists. It is a rebase-only change, so the round should be short.

grok-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-20 12:09:54 +00:00

grok-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: Approve — I agree with this as-is.

Tip 26ff383 is the same fix rebased onto main after #79/#80/#83 landed Unreleased entries. Prior approve was at 173456d.

What holds

shopt -s globstar dotglob is still the right minimal fix so **/ descends into .github/ and catches release-lib.sh / labels-reconcile.sh.
comm -23 against git ls-files '*.sh' still fails the step on under-coverage before shellcheck, so the hole cannot reopen quietly (including symlink escapes globstar will not follow).
Rebase resolution is additive. CHANGELOG keeps the main-side ### Changed taxonomy / --root-door material and this PR's ### Fixed dotglob entry; shipped ## 0.2.0 intact. ci.yml half did not conflict.

No remaining blockers. CI green on the rebased head.

**Verdict: Approve** — I agree with this as-is. Tip `26ff383` is the same fix rebased onto main after #79/#80/#83 landed Unreleased entries. Prior approve was at `173456d`. ### What holds - **`shopt -s globstar dotglob`** is still the right minimal fix so `**/` descends into `.github/` and catches `release-lib.sh` / `labels-reconcile.sh`. - **`comm -23` against `git ls-files '*.sh'`** still fails the step on under-coverage before shellcheck, so the hole cannot reopen quietly (including symlink escapes globstar will not follow). - **Rebase resolution is additive.** CHANGELOG keeps the main-side `### Changed` taxonomy / `--root-door` material and this PR's `### Fixed` dotglob entry; shipped `## 0.2.0` intact. ci.yml half did not conflict. No remaining blockers. CI green on the rebased head.

codex-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-20 12:15:03 +00:00

codex-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: I agree with everything and have no additional feedback.

claude-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-20 12:15:25 +00:00

claude-bot-andresmgsl (Migrated from github.com) left a comment

✅ Approved — I agree with all of this, no concerns.

Re-review of head 26ff383 after the rebase onto main. The code half is byte-for-byte what I approved at 173456d — shopt -s globstar dotglob plus the comm -23 coverage assertion in .github/workflows/ci.yml:23-31. The changelog conflict resolution is correct: the ### Changed entries from #79/#80/#83 and this PR's ### Fixed block are distinct sections that compose; nothing was dropped or reworded. CI is green on the new head across all four jobs. Nothing new to add.

— automated review by claude-bot-andresmgsl · heavy-duty-review-bot

✅ **Approved — I agree with all of this, no concerns.** Re-review of head `26ff383` after the rebase onto main. The code half is byte-for-byte what I approved at `173456d` — `shopt -s globstar dotglob` plus the `comm -23` coverage assertion in `.github/workflows/ci.yml:23-31`. The changelog conflict resolution is correct: the `### Changed` entries from #79/#80/#83 and this PR's `### Fixed` block are distinct sections that compose; nothing was dropped or reworded. CI is green on the new head across all four jobs. Nothing new to add. — _automated review by `claude-bot-andresmgsl` · heavy-duty-review-bot_

No reviewers

danmt

grok-bot-andresmgsl

codex-bot-andresmgsl

claude-bot-andresmgsl

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: heavy-duty/rig#71

No description provided.

fix: lint .github/scripts with dotglob, and assert the sweep is total #71

The defect

What is now linted

Did it pass as-is?

The class check

Verification

Verdict: Approve

Mechanism (.github/workflows/ci.yml:23-31)

Sweep totality — measured, not assumed

The totality assertion (.github/workflows/ci.yml:26-30)

Do the newly-linted scripts pass?

.git/ descent

Tests

What holds

Mechanism (`.github/workflows/ci.yml:23-31`)

The totality assertion (`.github/workflows/ci.yml:26-30`)

`.git/` descent