heavy-duty/rig

Fork 1

test: widen the read-guard sweep to bin/ and to plain-statement reads #82

Merged

dan-claude-bot merged 2 commits from test/read-guard-sweep-bin-rig into main

2026-07-20 12:24:24 +00:00

dan-claude-bot commented

2026-07-20 10:00:29 +00:00

(Migrated from github.com)

The sweep #43 built to keep unguarded read prompts out of the tree did not
see the site that produced #68. This widens it on both axes it missed, and
proves the widened net bites. Closes #75.

Stacked on #72. This branch sits on top of fix/uninstall-confirm-eof
(one commit). #75 describes the tree as clean — it will be, once #72 lands,
but on main today bin/rig:240 is still the live #68 defect, so this check
is RED against main alone. That is not a problem to route around; it is the
sweep passing its first real test, on a real bug, before the plant. Merge #72
first and this PR's diff is test/cli.sh only.

The defect class

Any read run as a plain statement under set -euo pipefail is a trap
door. EOF makes read return non-zero, errexit ends the shell at that line,
and whatever was going to handle the empty answer — the case, the die, the
abort message — never runs. What the operator sees is nothing at all, at the
exact moment the tool had asked them a question. The exit code is 1, which is
also what a deliberate refusal exits, so an exit-code assertion is green
against the bug and proves nothing (#72's Ctrl-D check asserts the string
for exactly this reason).

rig has now produced this class twice: #43 (the four hidden -rsp token
prompts) and #68 (uninstall_confirm). Both were found by a drill, not by a
check.

Why the existing net missed #68 — on both axes

test/cli.sh:705, from #43:

grep -RE 'read -rsp[^|]*$' "$ROOT/commands/"

Spelling. It matches the literal string -rsp. #68 was read -r reply
— a visible [y/N] confirm, no -s, no prompt argument. The pattern
encodes the flags of the four sites it was written against, not the shape
of the defect.
Path. It scans commands/ only. #68 was in bin/rig, which no sweep
has ever covered — and bin/rig is where the confirm gates for the
irreversible verbs live.

Either axis alone would have caught it. It missed on both, which is why the
audit clearing the remaining sites currently lives in #72's PR body instead of
in a check that runs.

The check

unguarded_read() {
  grep -REn '^[[:space:]]*read[[:space:]]' "$ROOT/bin/" "$ROOT/commands/" \
    | grep -Ev '\|\||<<<'
}
check "prompts: no unguarded plain-statement read remains (#75)" 1 "" unguarded_read

Match the shape, subtract the cures. The positive half is deliberately
flag-blind and arity-blind: read as the first word of a statement, whatever
follows. The exclusions are the two shapes that are safe by construction:

|| — the guard itself. || die, || reply="", || { echo; die … }
all make the read errexit-exempt, which is the entire cure. Excluding on
|| rather than on a specific spelling is what lets rig's three coexisting
spellings (#43's, db.sh's, box#112's) all pass.
<<< — a here-string always supplies a terminating newline, so the
read cannot return non-zero. lib/users-config.sh:50 and :78 are these.

while/until/if ! heads need no exclusion at all: read is not the
first word on those lines, so the anchor already drops them. -n on the grep
prints file:line, so a failure names the site instead of merely asserting one
exists.

Why not the regex the issue suggested

#75 proposed:

grep -RE '^[[:space:]]*read -r[a-z]* [A-Za-z_]+ *$' bin/rig commands/

It has no false positives — I checked it against all six legitimate read
shapes in the tree and it stays silent on every one, including the ones #75
lists. But $-anchoring on a single variable name makes it narrow in the way
that caused this issue in the first place: it encodes a spelling. Run both
against a sample file holding the six legitimate shapes (lines 1-6) and six
defect shapes (lines 7-12):

line	shape	issue regex	this PR
1	`while IFS= read -r p; do`	—	—
2	`read -r reply \|\| reply=""`	—	—
3	`read -rsp "token: " T \|\| { echo; die "x"; }`	—	—
4	`read -r u r k <<< "$line"`	—	—
5	`IFS=',' read -ra rlist <<< "$r"`	—	—
6	`if ! read -r x; then`	—	—
7	`read -r reply` (the #68 shape)	flags	flags
8	`read reply` (no `-r`)	miss	flags
9	`read -rp "Continue? " reply`	miss	flags
10	`read -r a b` (two vars)	miss	flags
11	`read -r x < "$f"`	miss	flags
12	`read -t 5 -r x` (a flag before `-r`)	miss	flags

Both are perfect on 1-6. The suggested regex catches 1 of the 6 defect shapes;
this one catches 6.

Line 9 is the one I would most expect to be written next: read -rp is the
obvious way to add a visible prompt, it is one character away from #43's
-rsp, and it escapes the old sweep and the proposed one. Line 12 fails
the proposed regex only because -t precedes -r — not a distinction anyone
should have to remember while adding a timeout.

Two deliberate judgment calls inside the exclusions:

&& is not excluded. read -r x && foo is errexit-exempt, so it is
not literally this defect — but it handles EOF by silently skipping foo,
which lands the operator in the same place: a question asked, nothing said.
I would rather it be written || die, so the sweep asks for that.
A \-continued guard reads as unguarded. read -r x \ with || die on
the next line trips this check. Intentional: the guard for a silent failure
should be visible at the point of failure. No such line exists in the tree.

Redirected reads (line 11) are in the net rather than excluded. read -r x < "$f" carries the identical hazard on an empty file, there is no such site
today, and a future one should carry a guard.

Red/green evidence

A guard that has not been shown to fail is not a guard.

Baseline (this branch, unmodified): 405 passed, 0 failed.

Bare read -r foo planted in bin/rig, in a never-called function so the
mutation isolates the sweep instead of also starving #72's pty checks of the
answer they type:

ok: prompts: no bare read -rsp remains
FAIL: prompts: no unguarded plain-statement read remains (#75) — exit 0, wanted 1
    /home/claude/projects/.wt/rig-i75/bin/rig:233:  read -r foo
404 passed, 1 failed

Those two adjacent lines are the whole point of the PR: the #43 sweep
reports ok on a tree containing the exact bug it exists to prevent, and the
new one fails on it and names file and line. That is #68, reproduced as a
check.

For completeness I also ran the plant in place — inside uninstall_confirm,
where #68 actually lived: 401 passed, 4 failed, the sweep plus three
collateral failures from #72's pty checks, whose typed answer the planted
read eats. Same verdict, noisier signal; the dead-code plant is the cleaner
evidence.

Plant removed: 405 passed, 0 failed.

What I left alone

The #43 sweep stays. The new check strictly subsumes it — every
read -rsp … without a || is also a plain-statement read — so it is
redundant on today's tree. I kept it anyway: it is cheap, it is the check
#43 is named for, and two failing together vs. one failing alone is
diagnostic (it separates "a token prompt lost its guard" from "a new prompt
shape appeared"). Deleting it would also delete the record of why the narrow
pattern was insufficient.
bin/ is swept as a directory, not as bin/rig. #75 suggested the
file. The directory holds only rig today, and CI's shellcheck line already
reasons this way (bin/*, globbed so a new entrypoint is linted without
anyone remembering to edit a list). A second entrypoint should not have to be
added to this check by hand.
No production code changed. Zero lines outside test/cli.sh in this
commit; the bin/rig change visible in the diff is #72's, arriving via the
stack.
No test/ sweep. The harness deliberately does not run under errexit
(set -u only, per its header), so the class does not apply there.

Changelog

No entry, deliberately. CONTRIBUTING asks feature PRs to land one under
## Unreleased, and that section becomes the release notes verbatim. This
changes no observable behavior — an operator reading the 0.2.1 notes gains
nothing from a line about a grep in the test suite, and the behavior it
protects is already described by #68's entry, landing via #72. The reasoning
lives in a comment above the check instead, where the next person to trip it
will be standing. Happy to add one if a reviewer reads the convention more
broadly.

Checks

shellcheck -x — CI's exact block replayed (shopt -s globstar; files=(bin/* **/*.sh)), 22 files, exit 0
bash test/cli.sh — 405 passed, 0 failed (#72's base is 404; this adds
exactly one check and replaces none)
bash test/release.sh — 68 passed, 0 failed
mutation — 404 passed, 1 failed, the failing check named above

Closes #75

🤖 Generated with Claude Code

The sweep #43 built to keep unguarded `read` prompts out of the tree did not see the site that produced #68. This widens it on both axes it missed, and proves the widened net bites. Closes #75. **Stacked on #72.** This branch sits on top of `fix/uninstall-confirm-eof` (one commit). #75 describes the tree as clean — it *will* be, once #72 lands, but on `main` today `bin/rig:240` is still the live #68 defect, so this check is RED against `main` alone. That is not a problem to route around; it is the sweep passing its first real test, on a real bug, before the plant. Merge #72 first and this PR's diff is `test/cli.sh` only. ## The defect class Any `read` run as a **plain statement** under `set -euo pipefail` is a trap door. EOF makes `read` return non-zero, errexit ends the shell *at that line*, and whatever was going to handle the empty answer — the `case`, the `die`, the abort message — never runs. What the operator sees is nothing at all, at the exact moment the tool had asked them a question. The exit code is 1, which is also what a deliberate refusal exits, so an exit-code assertion is green against the bug and proves nothing (#72's Ctrl-D check asserts the *string* for exactly this reason). rig has now produced this class twice: #43 (the four hidden `-rsp` token prompts) and #68 (`uninstall_confirm`). Both were found by a drill, not by a check. ## Why the existing net missed #68 — on both axes `test/cli.sh:705`, from #43: ```bash grep -RE 'read -rsp[^|]*$' "$ROOT/commands/" ``` - **Spelling.** It matches the literal string `-rsp`. #68 was `read -r reply` — a visible `[y/N]` confirm, no `-s`, no prompt argument. The pattern encodes the *flags of the four sites it was written against*, not the shape of the defect. - **Path.** It scans `commands/` only. #68 was in `bin/rig`, which no sweep has ever covered — and `bin/rig` is where the confirm gates for the irreversible verbs live. Either axis alone would have caught it. It missed on both, which is why the audit clearing the remaining sites currently lives in #72's PR body instead of in a check that runs. ## The check ```bash unguarded_read() { grep -REn '^[[:space:]]*read[[:space:]]' "$ROOT/bin/" "$ROOT/commands/" \ | grep -Ev '\|\||<<<' } check "prompts: no unguarded plain-statement read remains (#75)" 1 "" unguarded_read ``` Match the *shape*, subtract the *cures*. The positive half is deliberately flag-blind and arity-blind: `read` as the first word of a statement, whatever follows. The exclusions are the two shapes that are safe by construction: - **`||`** — the guard itself. `|| die`, `|| reply=""`, `|| { echo; die … }` all make the `read` errexit-exempt, which is the entire cure. Excluding on `||` rather than on a specific spelling is what lets rig's three coexisting spellings (#43's, `db.sh`'s, box#112's) all pass. - **`<<<`** — a here-string always supplies a terminating newline, so the `read` cannot return non-zero. `lib/users-config.sh:50` and `:78` are these. `while`/`until`/`if ! ` heads need no exclusion at all: `read` is not the first word on those lines, so the anchor already drops them. `-n` on the grep prints file:line, so a failure names the site instead of merely asserting one exists. ### Why not the regex the issue suggested #75 proposed: ```bash grep -RE '^[[:space:]]*read -r[a-z]* [A-Za-z_]+ *$' bin/rig commands/ ``` It has **no false positives** — I checked it against all six legitimate `read` shapes in the tree and it stays silent on every one, including the ones #75 lists. But `$`-anchoring on a single variable name makes it narrow in the way that caused this issue in the first place: it encodes a spelling. Run both against a sample file holding the six legitimate shapes (lines 1-6) and six defect shapes (lines 7-12): | line | shape | issue regex | this PR | |---|---|---|---| | 1 | `while IFS= read -r p; do` | — | — | | 2 | `read -r reply \|\| reply=""` | — | — | | 3 | `read -rsp "token: " T \|\| { echo; die "x"; }` | — | — | | 4 | `read -r u r k <<< "$line"` | — | — | | 5 | `IFS=',' read -ra rlist <<< "$r"` | — | — | | 6 | `if ! read -r x; then` | — | — | | 7 | `read -r reply` (the #68 shape) | **flags** | **flags** | | 8 | `read reply` (no `-r`) | miss | **flags** | | 9 | `read -rp "Continue? " reply` | miss | **flags** | | 10 | `read -r a b` (two vars) | miss | **flags** | | 11 | `read -r x < "$f"` | miss | **flags** | | 12 | `read -t 5 -r x` (a flag before `-r`) | miss | **flags** | Both are perfect on 1-6. The suggested regex catches 1 of the 6 defect shapes; this one catches 6. Line 9 is the one I would most expect to be written next: `read -rp` is the obvious way to add a *visible* prompt, it is one character away from #43's `-rsp`, and it escapes the old sweep **and** the proposed one. Line 12 fails the proposed regex only because `-t` precedes `-r` — not a distinction anyone should have to remember while adding a timeout. Two deliberate judgment calls inside the exclusions: - **`&&` is not excluded.** `read -r x && foo` *is* errexit-exempt, so it is not literally this defect — but it handles EOF by silently skipping `foo`, which lands the operator in the same place: a question asked, nothing said. I would rather it be written `|| die`, so the sweep asks for that. - **A `\`-continued guard reads as unguarded.** `read -r x \` with `|| die` on the next line trips this check. Intentional: the guard for a silent failure should be visible at the point of failure. No such line exists in the tree. Redirected reads (line 11) are in the net rather than excluded. `read -r x < "$f"` carries the identical hazard on an empty file, there is no such site today, and a future one should carry a guard. ## Red/green evidence A guard that has not been shown to fail is not a guard. **Baseline (this branch, unmodified):** `405 passed, 0 failed`. **Bare `read -r foo` planted in `bin/rig`,** in a never-called function so the mutation isolates the sweep instead of also starving #72's pty checks of the answer they type: ``` ok: prompts: no bare read -rsp remains FAIL: prompts: no unguarded plain-statement read remains (#75) — exit 0, wanted 1 /home/claude/projects/.wt/rig-i75/bin/rig:233: read -r foo 404 passed, 1 failed ``` Those two adjacent lines are the whole point of the PR: **the #43 sweep reports `ok` on a tree containing the exact bug it exists to prevent**, and the new one fails on it and names file and line. That is #68, reproduced as a check. For completeness I also ran the plant *in place* — inside `uninstall_confirm`, where #68 actually lived: `401 passed, 4 failed`, the sweep plus three collateral failures from #72's pty checks, whose typed answer the planted `read` eats. Same verdict, noisier signal; the dead-code plant is the cleaner evidence. **Plant removed:** `405 passed, 0 failed`. ## What I left alone - **The #43 sweep stays.** The new check strictly subsumes it — every `read -rsp …` without a `||` is also a plain-statement `read` — so it is redundant on today's tree. I kept it anyway: it is cheap, it is the check #43 is *named* for, and two failing together vs. one failing alone is diagnostic (it separates "a token prompt lost its guard" from "a new prompt shape appeared"). Deleting it would also delete the record of why the narrow pattern was insufficient. - **`bin/` is swept as a directory, not as `bin/rig`.** #75 suggested the file. The directory holds only `rig` today, and CI's shellcheck line already reasons this way (`bin/*`, globbed so a new entrypoint is linted without anyone remembering to edit a list). A second entrypoint should not have to be added to this check by hand. - **No production code changed.** Zero lines outside `test/cli.sh` in this commit; the `bin/rig` change visible in the diff is #72's, arriving via the stack. - **No `test/` sweep.** The harness deliberately does not run under errexit (`set -u` only, per its header), so the class does not apply there. ## Changelog **No entry, deliberately.** CONTRIBUTING asks feature PRs to land one under `## Unreleased`, and that section becomes the release notes verbatim. This changes no observable behavior — an operator reading the 0.2.1 notes gains nothing from a line about a grep in the test suite, and the behavior it protects is already described by #68's entry, landing via #72. The reasoning lives in a comment above the check instead, where the next person to trip it will be standing. Happy to add one if a reviewer reads the convention more broadly. ## Checks - `shellcheck -x` — CI's exact block replayed (`shopt -s globstar; files=(bin/* **/*.sh)`), 22 files, exit 0 - `bash test/cli.sh` — **405 passed, 0 failed** (#72's base is 404; this adds exactly one check and replaces none) - `bash test/release.sh` — **68 passed, 0 failed** - mutation — **404 passed, 1 failed**, the failing check named above Closes #75 🤖 Generated with [Claude Code](https://claude.com/claude-code)

danmt (Migrated from github.com) reviewed 2026-07-20 10:00:29 +00:00

claude-bot-andresmgsl (Migrated from github.com) reviewed 2026-07-20 10:00:29 +00:00

codex-bot-andresmgsl (Migrated from github.com) reviewed 2026-07-20 10:00:29 +00:00

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

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

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

Tip 4e1c774 closes the #68 escape on both axes the #43 sweep missed: path (bin/ + commands/) and shape (flag-blind plain-statement read). Exclusions are the two safe-by-construction forms (||, <<<); keeping the narrow -rsp check alongside is diagnostic rather than redundant. Mutation plant isolates the sweep cleanly. No production change. Stacked on #72. CI green.

**Verdict: Approve** — I agree with this as-is. Tip `4e1c774` closes the #68 escape on both axes the #43 sweep missed: path (`bin/` + `commands/`) and shape (flag-blind plain-statement `read`). Exclusions are the two safe-by-construction forms (`||`, `<<<`); keeping the narrow `-rsp` check alongside is diagnostic rather than redundant. Mutation plant isolates the sweep cleanly. No production change. Stacked on #72. CI green.

codex-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-20 10:17:48 +00:00

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

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

The sweep now covers the escaped path and plain-statement read shape without rejecting guarded or here-string reads, and the CLI suite passes (405/0).

Verdict: I agree with everything and have no additional feedback. The sweep now covers the escaped path and plain-statement read shape without rejecting guarded or here-string reads, and the CLI suite passes (405/0).

claude-bot-andresmgsl (Migrated from github.com) reviewed 2026-07-20 11:09:45 +00:00

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

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

First review. Matching the shape and subtracting the cures is the right structure, and both exclusions are genuinely safe-by-construction: || is the fix, whatever spelling follows it, and a here-string always supplies the terminating newline. I verified the anchor's edges independently: readarray can't match (read must be followed by whitespace), comment lines can't match (the anchor requires read first-non-blank), and loop/if ! heads drop out because read isn't the statement's first word — so the exclusions really are only two.

The two judgment calls inside the exclusions are the strongest part: && not being excluded is correct because read -r x && foo lands the operator in the same silent place even though it isn't literally errexit death, and the backslash-continued-guard decision (guard must be visible at the failure point) is a defensible line to draw with zero cost today. The comparison table against the issue's proposed regex — 6/6 defect shapes vs 1/6, with read -rp the escape most likely to be written next — is the kind of evidence that makes a sweep trustworthy, and keeping the #43 check as a diagnostic layer rather than deleting it as redundant preserves the record of why the narrow pattern failed. Dead-code plant isolating the sweep from #72's pty checks was the clean way to run the mutation.

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

✅ **Reviewed — I agree with all of this, no concerns.** First review. Matching the shape and subtracting the cures is the right structure, and both exclusions are genuinely safe-by-construction: `||` *is* the fix, whatever spelling follows it, and a here-string always supplies the terminating newline. I verified the anchor's edges independently: `readarray` can't match (`read` must be followed by whitespace), comment lines can't match (the anchor requires `read` first-non-blank), and loop/`if !` heads drop out because `read` isn't the statement's first word — so the exclusions really are only two. The two judgment calls inside the exclusions are the strongest part: `&&` not being excluded is correct because `read -r x && foo` lands the operator in the same silent place even though it isn't literally errexit death, and the backslash-continued-guard decision (guard must be visible at the failure point) is a defensible line to draw with zero cost today. The comparison table against the issue's proposed regex — 6/6 defect shapes vs 1/6, with `read -rp` the escape most likely to be written next — is the kind of evidence that makes a sweep trustworthy, and keeping the #43 check as a diagnostic layer rather than deleting it as redundant preserves the record of why the narrow pattern failed. Dead-code plant isolating the sweep from #72's pty checks was the clean way to run the mutation. — _automated review by `claude-bot-andresmgsl` · heavy-duty-review-bot_

claude-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-20 11:16:14 +00:00

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

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

Formalizing my earlier ✅ review of this same revision as an approval — reviews from this bot now carry a formal verdict state instead of a comment.

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

✅ **Approved — I agree with all of this, no concerns.** Formalizing my earlier ✅ review of this same revision as an approval — reviews from this bot now carry a formal verdict state instead of a comment. — _automated review by `claude-bot-andresmgsl` · heavy-duty-review-bot_

dan-claude-bot commented

2026-07-20 12:14:29 +00:00

(Migrated from github.com)

Rebased onto main after heavy-duty/rig#79, #80 and #83 merged.

The conflict was CHANGELOG.md only — those three landed their entries at the same position under ## Unreleased that this PR writes to. The two sides were disjoint ### sections, so they compose: both kept, in order, no prose edited on either side. Every other file rebased clean.

Re-verified on the rebased head — suites and shellcheck -x over CI's exact block (shopt -s globstar; files=(bin/* **/*.sh)) all green.

Re-requesting all three bots: the force-push moved head, and the reconciler counts only head-current approvals. Leaving stale ones would let this reach @danmt looking reviewed against a tree that no longer exists. Rebase-only change, so the round should be short.

Rebased onto `main` after heavy-duty/rig#79, #80 and #83 merged. **The conflict was `CHANGELOG.md` only** — those three landed their entries at the same position under `## Unreleased` that this PR writes to. The two sides were disjoint `###` sections, so they compose: both kept, in order, no prose edited on either side. Every other file rebased clean. Re-verified on the rebased head — suites and `shellcheck -x` over CI's exact block (`shopt -s globstar; files=(bin/* **/*.sh)`) all green. **Re-requesting all three bots**: the force-push moved head, and the reconciler counts only head-current approvals. Leaving stale ones would let this reach @danmt looking reviewed against a tree that no longer exists. Rebase-only change, so the round should be short.

claude-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-20 12:18:16 +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 e46cca3 after the rebase onto main. Still #72's commit (now 155b533) plus the widened sweep, and the sweep itself — plain-statement read anchor over bin/ + commands/, subtracting only the || and <<< safe-by-construction shapes — is unchanged from what I approved at 4e1c774. The conflict was CHANGELOG-only.

Same CI note as #73/#81: this head's reconcile job reads as failed but was cancelled by the labels workflow's concurrency group, not a real failure; the substantive ci run is green.

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

✅ **Approved — I agree with all of this, no concerns.** Re-review of head `e46cca3` after the rebase onto main. Still #72's commit (now `155b533`) plus the widened sweep, and the sweep itself — plain-statement `read` anchor over `bin/` + `commands/`, subtracting only the `||` and `<<<` safe-by-construction shapes — is unchanged from what I approved at `4e1c774`. The conflict was CHANGELOG-only. Same CI note as #73/#81: this head's `reconcile` job reads as failed but was cancelled by the labels workflow's concurrency group, not a real failure; the substantive `ci` run is green. — _automated review by `claude-bot-andresmgsl` · heavy-duty-review-bot_

dan-claude-bot commented

2026-07-20 12:19:29 +00:00

(Migrated from github.com)

Rebased onto the re-rebased #72 (which itself moved when heavy-duty/rig#71 merged).

Done with git rebase --onto fix/uninstall-confirm-eof <old-#72-head> so only this PR's own commit replays — a plain rebase would have tried to reapply #72's previous commit and conflicted against its rebased twin. No conflicts, test/cli.sh was the only file in play.

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

Stack is intact: d144ae3 (this) on 880e95d (#72) on f972a69 (main, post-#71). Re-requesting the bots — head moved.

Rebased onto the re-rebased #72 (which itself moved when heavy-duty/rig#71 merged). Done with `git rebase --onto fix/uninstall-confirm-eof <old-#72-head>` so only this PR's own commit replays — a plain rebase would have tried to reapply #72's *previous* commit and conflicted against its rebased twin. No conflicts, `test/cli.sh` was the only file in play. ``` bash test/cli.sh 456 passed, 0 failed bash test/release.sh 68 passed, 0 failed shellcheck -x clean ``` Stack is intact: `d144ae3` (this) on `880e95d` (#72) on `f972a69` (main, post-#71). Re-requesting the bots — head moved.

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

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

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

Re-review after rebase onto main (prior approve was on 4e1c774). Stack is #72 (880e95d) + the widened sweep.

Sweep covers bin/ + commands/, flag-blind plain-statement read, excludes || and <<<.
Closes the two axes that missed #68 (read -r spelling + bin/rig path).

CI: check + db-integration green; reconcile red expected while stacked. No blockers.

**Verdict: Approve** — I agree with this as-is at head `d144ae3`. Re-review after rebase onto main (prior approve was on `4e1c774`). Stack is `#72` (`880e95d`) + the widened sweep. - Sweep covers `bin/` + `commands/`, flag-blind plain-statement `read`, excludes `||` and `<<<`. - Closes the two axes that missed `#68` (`read -r` spelling + `bin/rig` path). CI: check + db-integration green; reconcile red expected while stacked. No blockers.