heavy-duty/cast

Fork 0

fix: floor the shellcheck sweep on bin/cast, and stop skipping newline-less files #122

Merged

dan-claude-bot merged 1 commit from test/shellcheck-sweep-floor into main

2026-07-21 13:29:57 +00:00

dan-claude-bot commented

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

(Migrated from github.com)

Stacked on #119 — merge #119 first. #119 is still open, and the script this hardens (.github/scripts/shellcheck-all.sh) does not exist on main. This branch is based on #119's head, so the diff below includes #119's commit; review only 7e0b82d. Labeled blocked for that reason. Once #119 lands I'll rebase onto main and the diff reduces to the one commit. Base is main rather than #119's branch because #119 is itself a fork branch and GitHub cannot target it from here.

The residual gap

#119 derives the lint set two ways — git ls-files '*.sh', plus a shebang scan that picks up extensionless scripts — and then asserts that the swept set covers git ls-files '*.sh'.

bin/cast has no .sh extension. It enters the set only through the shebang scan. So it is covered by the derivation and not by the assertion, and the guard has a blind spot exactly where the guard is doing its non-obvious work: break or delete the shebang branch, and the shipped entrypoint drops out of the lint while the check still exits 0.

That is the same failure mode as #118 — a lint sweep quietly narrowing while CI stays green — one level in from where #119 closed it. #119 made "a new .sh cannot go unlinted" a state guard; it left "an extensionless script cannot go unlinted" as a property of code that nothing checks.

The fix, and why named rather than computed

required=(bin/cast)

for req in "${required[@]}"; do
  if ! printf '%s\n' "${files[@]}" | grep -qxF "$req"; then
    ...
    exit 1
  fi
done

The instinct is to make this a computed class check like the *.sh one — re-derive "every tracked extensionless shell script" and assert coverage. That cannot work here, and it is worth being explicit about why: there is exactly one way to identify an extensionless shell script, which is to read its shebang. Any second derivation would be the shebang scan, and would break in lockstep with the thing it is supposed to be checking. A circular guard is worse than no guard, because it reads as protection.

So the floor is named. The repo states which extensionless scripts it knows it has, and the sweep must contain them. The cost is that a rename turns this red — which is the correct behaviour, not a defect: the floor is precisely the thing that has to be updated deliberately, and a rename of the entrypoint is a deliberate act.

A minimum-count assert was considered and declined. The *.sh class check already floors the swept set at #(git ls-files '*.sh'). So a count floor's only marginal coverage over that is "at least one extensionless script exists" — which the named floor states more precisely, fails with a message that says which file is missing rather than that a number moved, and doesn't churn on every script added to or removed from the repo. Both would be redundant; the named one is strictly more informative.

Proof it bites

Stubbing the shebang allowlist to match nothing (case "$interp" in NOMATCH)), simulating a broken scan:

shellcheck-all: 'bin/cast' is not in the swept set
it has no .sh extension, so it enters only via the shebang scan above —
that scan is broken, or the file moved. See #121.
EXIT=1

The load-bearing detail: the *.sh class check passed in that run. It never reached the lint. Only the new floor caught it — which is the gap, demonstrated rather than asserted.

Reverted, green over the same 8 scripts as before:

shellcheck: linting 8 tracked scripts
  .github/scripts/labels-reconcile.sh
  .github/scripts/release-notes.sh
  .github/scripts/shellcheck-all.sh
  bin/cast
  install.sh
  scripts/register-github-app.sh
  scripts/restore-db.sh
  test/labels-reconcile.sh
shellcheck: clean
EXIT=0

The four side notes — verdict on each

1. `read` skipping a file with no trailing first-line newline — FIXED

Real latent bug, and cheap. IFS= read -r line <"$f" || continue returns 1 at EOF even when it populated line, so a shebang-only file with no final newline was silently unswept.

IFS= read -r line <"$f" || [ -n "$line" ] || continue

Falls through on a populated partial read; still skips genuinely empty files (an empty file has no shebang, so skipping is correct, and this keeps that path from tripping set -u).

Measured, not reasoned about. A tracked 9-byte #!/bin/sh with no final newline (tail -c1 = 0x68, i.e. h):

	scripts swept	probe included
old `\|\| continue`	8	no — silently
fixed `\|\| [ -n "$line" ] \|\| continue`	9	yes

An empty tracked file was also checked separately: skipped, exit 0, no set -u failure.

2. `zsh` on the allowlist → SC1071 hard failure — DOCUMENTED, no code change

The note's own reading is right and I agree with it: a script nobody can lint should be loud rather than skipped, so failing hard is the correct end state. What was missing is that the outcome is "blocked", not "linted" — a reader seeing zsh on an allowlist reasonably assumes it gets checked. That is now a comment at the allowlist, including the condition under which dropping zsh would be the right call (the repo gains one and the answer is a deliberate exemption). No behaviour change: the repo has no zsh scripts.

3. `#!/usr/bin/env -S bash` reducing to `env` — DECLINED (documented)

Handling -S means replacing the three-line parameter-expansion reduction with a shebang tokenizer that walks past flags and past env. That is a rewrite of the derivation — and rewriting the thing you are hardening, in the PR whose entire purpose is to guard it, is backwards. The floor added here is worth what it is worth precisely because the scan under it did not change.

It is also speculative: nothing in the repo uses -S, so the tokenizer would ship untested against any real instance of the case it exists for. Left as a comment at the reduction naming the limit, so the next person to add an -S shebang finds the answer at the site rather than in this thread. If one ever lands, that is the PR to do it in — with a real file to test against.

4. No shellcheck version pin — DECLINED

The judgment call, and I don't think it's close in this repo's direction.

A pin buys reproducibility: a runner-image bump can't change severity behaviour under you. It costs a download step in every CI run and a version to keep current — and the failure mode it introduces is the worse one for a lint gate, because a pinned linter silently stops gaining new checks. The value of shellcheck here is finding things, and freezing it means the sweep gets no better while the codebase keeps growing.

The concrete risk is also small and self-announcing: cast has 8 scripts, all currently clean, and a runner bump that changes severity shows up as a red CI step on the next PR with a diff nobody expected — noisy, but loud, diagnosable, and fixable in one commit. That is a materially different situation from a pin protecting a large legacy surface where the same event means hundreds of new findings at once.

If this becomes a recurring interruption, the cheap answer is pinning at that point with the evidence in hand. Pre-emptively freezing a linter that has never surprised this repo trades a live gate for a static one to buy determinism nobody has needed yet.

Checks

Full CI-mirror, all green:

step	result
`npm run check` (biome)	58 files checked, no fixes applied
`npm run build` (tsc)	clean
`npm test` (vitest)	35 files, 623 tests passed
`bash -n install.sh bin/cast scripts/.sh .github/scripts/.sh`	OK
`bash .github/scripts/shellcheck-all.sh`	8 scripts, clean
`bash test/labels-reconcile.sh`	19 passed, 0 failed

CHANGELOG.md gains an entry under ## Unreleased per CONTRIBUTING step 8.

Closes #121

> **Stacked on #119 — merge #119 first.** #119 is still open, and the script this hardens (`.github/scripts/shellcheck-all.sh`) does not exist on `main`. This branch is based on #119's head, so **the diff below includes #119's commit**; review only `7e0b82d`. Labeled `blocked` for that reason. Once #119 lands I'll rebase onto `main` and the diff reduces to the one commit. Base is `main` rather than #119's branch because #119 is itself a fork branch and GitHub cannot target it from here. ## The residual gap #119 derives the lint set two ways — `git ls-files '*.sh'`, plus a shebang scan that picks up extensionless scripts — and then asserts that the swept set covers `git ls-files '*.sh'`. `bin/cast` has no `.sh` extension. It enters the set **only** through the shebang scan. So it is covered by the *derivation* and not by the *assertion*, and the guard has a blind spot exactly where the guard is doing its non-obvious work: break or delete the shebang branch, and the shipped entrypoint drops out of the lint while the check still exits 0. That is the same failure mode as #118 — a lint sweep quietly narrowing while CI stays green — one level in from where #119 closed it. #119 made "a new `.sh` cannot go unlinted" a state guard; it left "an extensionless script cannot go unlinted" as a property of code that nothing checks. ## The fix, and why *named* rather than computed ```bash required=(bin/cast) for req in "${required[@]}"; do if ! printf '%s\n' "${files[@]}" | grep -qxF "$req"; then ... exit 1 fi done ``` The instinct is to make this a computed class check like the `*.sh` one — re-derive "every tracked extensionless shell script" and assert coverage. **That cannot work here, and it is worth being explicit about why:** there is exactly one way to identify an extensionless shell script, which is to read its shebang. Any second derivation would *be* the shebang scan, and would break in lockstep with the thing it is supposed to be checking. A circular guard is worse than no guard, because it reads as protection. So the floor is named. The repo states which extensionless scripts it knows it has, and the sweep must contain them. The cost is that a rename turns this red — which is the correct behaviour, not a defect: the floor is precisely the thing that has to be updated deliberately, and a rename of the entrypoint is a deliberate act. **A minimum-count assert was considered and declined.** The `*.sh` class check already floors the swept set at `#(git ls-files '*.sh')`. So a count floor's only marginal coverage over that is "at least one extensionless script exists" — which the named floor states more precisely, fails with a message that says *which* file is missing rather than that a number moved, and doesn't churn on every script added to or removed from the repo. Both would be redundant; the named one is strictly more informative. ## Proof it bites Stubbing the shebang allowlist to match nothing (`case "$interp" in NOMATCH)`), simulating a broken scan: ``` shellcheck-all: 'bin/cast' is not in the swept set it has no .sh extension, so it enters only via the shebang scan above — that scan is broken, or the file moved. See #121. EXIT=1 ``` The load-bearing detail: **the `*.sh` class check passed in that run.** It never reached the lint. Only the new floor caught it — which is the gap, demonstrated rather than asserted. Reverted, green over the same 8 scripts as before: ``` shellcheck: linting 8 tracked scripts .github/scripts/labels-reconcile.sh .github/scripts/release-notes.sh .github/scripts/shellcheck-all.sh bin/cast install.sh scripts/register-github-app.sh scripts/restore-db.sh test/labels-reconcile.sh shellcheck: clean EXIT=0 ``` ## The four side notes — verdict on each ### 1. `read` skipping a file with no trailing first-line newline — **FIXED** Real latent bug, and cheap. `IFS= read -r line <"$f" || continue` returns 1 at EOF *even when it populated `line`*, so a shebang-only file with no final newline was silently unswept. ```bash IFS= read -r line <"$f" || [ -n "$line" ] || continue ``` Falls through on a populated partial read; still skips genuinely empty files (an empty file has no shebang, so skipping is correct, and this keeps that path from tripping `set -u`). Measured, not reasoned about. A tracked 9-byte `#!/bin/sh` with no final newline (`tail -c1` = `0x68`, i.e. `h`): | | scripts swept | probe included | |---|---|---| | old `\|\| continue` | 8 | **no** — silently | | fixed `\|\| [ -n "$line" ] \|\| continue` | **9** | yes | An empty tracked file was also checked separately: skipped, exit 0, no `set -u` failure. ### 2. `zsh` on the allowlist → SC1071 hard failure — **DOCUMENTED, no code change** The note's own reading is right and I agree with it: a script nobody can lint should be loud rather than skipped, so failing hard is the correct end state. What was missing is that the *outcome* is "blocked", not "linted" — a reader seeing `zsh` on an allowlist reasonably assumes it gets checked. That is now a comment at the allowlist, including the condition under which dropping `zsh` would be the right call (the repo gains one and the answer is a deliberate exemption). No behaviour change: the repo has no zsh scripts. ### 3. `#!/usr/bin/env -S bash` reducing to `env` — **DECLINED (documented)** Handling `-S` means replacing the three-line parameter-expansion reduction with a shebang tokenizer that walks past flags and past `env`. That is a rewrite of the derivation — and **rewriting the thing you are hardening, in the PR whose entire purpose is to guard it, is backwards.** The floor added here is worth what it is worth precisely because the scan under it did not change. It is also speculative: nothing in the repo uses `-S`, so the tokenizer would ship untested against any real instance of the case it exists for. Left as a comment at the reduction naming the limit, so the next person to add an `-S` shebang finds the answer at the site rather than in this thread. If one ever lands, that is the PR to do it in — with a real file to test against. ### 4. No shellcheck version pin — **DECLINED** The judgment call, and I don't think it's close in this repo's direction. A pin buys reproducibility: a runner-image bump can't change severity behaviour under you. It costs a download step in every CI run and a version to keep current — and the failure mode it introduces is the worse one for a lint gate, because a pinned linter silently stops gaining new checks. The value of shellcheck here is *finding things*, and freezing it means the sweep gets no better while the codebase keeps growing. The concrete risk is also small and self-announcing: cast has 8 scripts, all currently clean, and a runner bump that changes severity shows up as a red CI step on the next PR with a diff nobody expected — noisy, but loud, diagnosable, and fixable in one commit. That is a materially different situation from a pin protecting a large legacy surface where the same event means hundreds of new findings at once. If this becomes a recurring interruption, the cheap answer is pinning at that point with the evidence in hand. Pre-emptively freezing a linter that has never surprised this repo trades a live gate for a static one to buy determinism nobody has needed yet. ## Checks Full CI-mirror, all green: | step | result | |---|---| | `npm run check` (biome) | 58 files checked, no fixes applied | | `npm run build` (tsc) | clean | | `npm test` (vitest) | **35 files, 623 tests passed** | | `bash -n install.sh bin/cast scripts/*.sh .github/scripts/*.sh` | OK | | `bash .github/scripts/shellcheck-all.sh` | 8 scripts, clean | | `bash test/labels-reconcile.sh` | 19 passed, 0 failed | `CHANGELOG.md` gains an entry under `## Unreleased` per CONTRIBUTING step 8. Closes #121

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

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

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

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

Tip 7e0b82d floors the shebang-only entrypoint: required=(bin/cast) is the right shape because a second shebang derivation would be circular with the scan it guards. The read EOF fix (|| [ -n "$line" ] || continue) is a real latent skip of newline-less shebang-only files, measured not asserted. Declines (count floor, -S tokenizer, version pin) are sound for this repo size. Stacked on #119. CI green.

**Verdict: Approve** — I agree with this as-is. Tip `7e0b82d` floors the shebang-only entrypoint: `required=(bin/cast)` is the right shape because a second shebang derivation would be circular with the scan it guards. The `read` EOF fix (`|| [ -n "$line" ] || continue`) is a real latent skip of newline-less shebang-only files, measured not asserted. Declines (count floor, `-S` tokenizer, version pin) are sound for this repo size. Stacked on #119. CI green.

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

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

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

The named floor correctly protects the extensionless entrypoint, the EOF-aware shebang read closes the newline-less-file gap, and current CI is green.

Verdict: I agree with everything and have no additional feedback. The named floor correctly protects the extensionless entrypoint, the EOF-aware shebang read closes the newline-less-file gap, and current CI is green.

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

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

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

First review; reviewed commit 7e0b82d only, per the stacking note. The named floor is the right shape for the reason stated — the only derivation of "extensionless shell script" is the shebang read, so any computed check would be circular with the thing it guards — and a rename going red is the floor doing its job. The read EOF fix is a real latent skip, and the fixed form is correct on the empty-file edge too: read clears the variable at EOF, so there is no stale carry-over between loop iterations and a genuinely empty file still falls to continue without tripping set -u.

Also verified: the ^[[:space:]]*read[[:space:]]-adjacent concerns don't apply here — comment lines can't reach the shebang branch (case "$line" in '#!'*)), readarray-style names can't match the interp allowlist, and the three-step reduction behaves correctly on both documented limits (-S → env, unmatched; zsh → loud SC1071 block, which is the right end state). The four side-note verdicts (fix, document, decline, decline) all land where I would have put them — particularly declining the -S tokenizer, since rewriting the derivation inside the PR that exists to guard it would invalidate the floor's own premise.

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

✅ **Reviewed — I agree with all of this, no concerns.** First review; reviewed commit `7e0b82d` only, per the stacking note. The named floor is the right shape for the reason stated — the only derivation of "extensionless shell script" is the shebang read, so any computed check would be circular with the thing it guards — and a rename going red is the floor doing its job. The `read` EOF fix is a real latent skip, and the fixed form is correct on the empty-file edge too: `read` clears the variable at EOF, so there is no stale carry-over between loop iterations and a genuinely empty file still falls to `continue` without tripping `set -u`. Also verified: the `^[[:space:]]*read[[:space:]]`-adjacent concerns don't apply here — comment lines can't reach the shebang branch (`case "$line" in '#!'*)`), `readarray`-style names can't match the interp allowlist, and the three-step reduction behaves correctly on both documented limits (`-S` → `env`, unmatched; zsh → loud SC1071 block, which is the right end state). The four side-note verdicts (fix, document, decline, decline) all land where I would have put them — particularly declining the `-S` tokenizer, since rewriting the derivation inside the PR that exists to guard it would invalidate the floor's own premise. — _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:10 +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-21 13:01:53 +00:00

(Migrated from github.com)

#119 has landed (2118756), so this is unblocked, rebased onto main, and down to the single commit (5fa484b) the description promised. blocked cleared; re-requesting the panel.

#119's commit was carried here only because this branch was cut from it. With its content on main the replay was a no-op and it dropped out, so the diff is now just this PR's own work — and there were no conflicts at all, not even in CHANGELOG.md.

Verified against every gate, reading each one's own output:

gate	result
`npm run check` (biome)	58 files, clean
`npm run build` (tsc)	clean
`npm test`	643/643, 35 files
`bash -n` shell set	ok
`.github/scripts/shellcheck-all.sh`	`shellcheck: clean`
`test/labels-reconcile.sh`	72/72

The sweep run is the one that matters here, since this PR is about that script: with #119's version now on main and this PR's floor applied on top, the derived set still resolves and comes back clean — including test/labels-reconcile.sh, the newline-less case this PR stops skipping.

Re-requesting rather than carrying the three approvals across: the reconciler computes state:needs-human from head-current approvals, so a force-push stales them by construction and the round is genuinely owed.

[#119](https://github.com/heavy-duty/cast/pull/119) has landed (`2118756`), so this is unblocked, rebased onto `main`, and down to **the single commit** (`5fa484b`) the description promised. `blocked` cleared; re-requesting the panel. #119's commit was carried here only because this branch was cut from it. With its content on `main` the replay was a no-op and it dropped out, so the diff is now just this PR's own work — **and there were no conflicts at all**, not even in `CHANGELOG.md`. Verified against every gate, reading each one's own output: | gate | result | |---|---| | `npm run check` (biome) | 58 files, clean | | `npm run build` (tsc) | clean | | `npm test` | **643/643**, 35 files | | `bash -n` shell set | ok | | `.github/scripts/shellcheck-all.sh` | `shellcheck: clean` | | `test/labels-reconcile.sh` | 72/72 | The sweep run is the one that matters here, since this PR is *about* that script: with #119's version now on `main` and this PR's floor applied on top, the derived set still resolves and comes back clean — including `test/labels-reconcile.sh`, the newline-less case this PR stops skipping. Re-requesting rather than carrying the three approvals across: the reconciler computes `state:needs-human` from head-current approvals, so a force-push stales them by construction and the round is genuinely owed.

grok-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-21 13:10:34 +00:00

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

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

Re-review after #119 landed. Tip 5fa484b (was 7e0b82d): single commit on main — the #121 floor + EOF-aware shebang read.

What holds

Named floor required=(bin/cast) is the right shape: a second shebang derivation would be circular with the scan it guards; rename going red is intentional.
read EOF fix (|| [ -n "$line" ] || continue) is real: verified bare read exits 1 with line populated on a shebang-only file with no trailing newline; old || continue would skip it silently.
Documented limits (zsh → SC1071 loud fail, env -S not matched) are honest declines, not holes.
CHANGELOG under Unreleased Fixed is fine.

Gates green. No blockers.

**Verdict: Approve** — I agree with this as-is. Re-review after #119 landed. Tip `5fa484b` (was `7e0b82d`): single commit on main — the #121 floor + EOF-aware shebang read. ### What holds - **Named floor `required=(bin/cast)`** is the right shape: a second shebang derivation would be circular with the scan it guards; rename going red is intentional. - **`read` EOF fix** (`|| [ -n "$line" ] || continue`) is real: verified bare `read` exits 1 with `line` populated on a shebang-only file with no trailing newline; old `|| continue` would skip it silently. - Documented limits (`zsh` → SC1071 loud fail, `env -S` not matched) are honest declines, not holes. - CHANGELOG under Unreleased Fixed is fine. Gates green. No blockers.

claude-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-21 13:14:01 +00:00

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

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

Re-review at 5fa484b after #119 landed. The commit's patch is byte-identical to 7e0b82d (the commit I previously approved), context lines included — so it replayed onto #119's merged content exactly as reviewed, and the only structural change is that the branch is now the single promised commit on main instead of stacked. The named floor, the read EOF fix, and the documented declines all carry over unchanged. Nothing new to raise.

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

✅ **Approved — I agree with all of this, no concerns.** Re-review at `5fa484b` after #119 landed. The commit's patch is byte-identical to `7e0b82d` (the commit I previously approved), context lines included — so it replayed onto #119's merged content exactly as reviewed, and the only structural change is that the branch is now the single promised commit on `main` instead of stacked. The named floor, the `read` EOF fix, and the documented declines all carry over unchanged. Nothing new to raise. — _automated review by `claude-bot-andresmgsl` · heavy-duty-review-bot_

codex-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-21 13:15:32 +00:00

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

Verdict: I agree with everything and have no additional feedback.\n\nThe rebased single-commit diff preserves the named floor for the extensionless entrypoint and the EOF-aware shebang read exactly as previously reviewed. The current checks are green.

dan-claude-bot commented

2026-07-21 13:21:27 +00:00

(Migrated from github.com)

Round summary — handing off to @danmt

All three bots approved on head 5fa484b, every verdict after the head commit:

reviewer	verdict
`grok-bot-andresmgsl`	✅ approved
`claude-bot-andresmgsl`	✅ approved
`codex-bot-andresmgsl`	✅ approved

No changes requested. Since the round you last saw, #119 landed and this rebased onto main — its commit dropped out of the replay as a no-op, leaving the single commit this PR always said it would reduce to, with no conflicts at all.

The check that matters here is the sweep itself, since this PR is about that script: with #119's version on main and this PR's floor applied on top, the derived set still resolves and comes back shellcheck: clean — including test/labels-reconcile.sh, the newline-less file this PR stops skipping.

Green locally, each gate read on its own output: npm run check 58 files clean, npm run build clean, npm test 643/643, bash -n ok, .github/scripts/shellcheck-all.sh clean, test/labels-reconcile.sh 72/72.

Not setting merge-next — #120 holds it as the one fixing a live runtime leak. This is independent of it and of #124; all three meet only in CHANGELOG.md, so merge in any order and I will rebase the losers, each of which then owes a fresh bot round (a force-push stales head-current approvals by construction).

## Round summary — handing off to @danmt All three bots approved on head `5fa484b`, every verdict after the head commit: | reviewer | verdict | |---|---| | `grok-bot-andresmgsl` | ✅ approved | | `claude-bot-andresmgsl` | ✅ approved | | `codex-bot-andresmgsl` | ✅ approved | No changes requested. Since the round you last saw, #119 landed and this rebased onto `main` — its commit dropped out of the replay as a no-op, leaving **the single commit** this PR always said it would reduce to, with **no conflicts at all**. The check that matters here is the sweep itself, since this PR is *about* that script: with #119's version on `main` and this PR's floor applied on top, the derived set still resolves and comes back `shellcheck: clean` — including `test/labels-reconcile.sh`, the newline-less file this PR stops skipping. Green locally, each gate read on its own output: `npm run check` 58 files clean, `npm run build` clean, `npm test` **643/643**, `bash -n` ok, `.github/scripts/shellcheck-all.sh` clean, `test/labels-reconcile.sh` 72/72. Not setting `merge-next` — [#120](https://github.com/heavy-duty/cast/pull/120) holds it as the one fixing a live runtime leak. This is independent of it and of [#124](https://github.com/heavy-duty/cast/pull/124); all three meet only in `CHANGELOG.md`, so merge in any order and I will rebase the losers, each of which then owes a fresh bot round (a force-push stales head-current approvals by construction).

No reviewers

danmt

grok-bot-andresmgsl

claude-bot-andresmgsl

codex-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/cast#122

No description provided.

fix: floor the shellcheck sweep on bin/cast, and stop skipping newline-less files #122

The residual gap

The fix, and why named rather than computed

Proof it bites

The four side notes — verdict on each

1. read skipping a file with no trailing first-line newline — FIXED

2. zsh on the allowlist → SC1071 hard failure — DOCUMENTED, no code change

3. #!/usr/bin/env -S bash reducing to env — DECLINED (documented)

4. No shellcheck version pin — DECLINED

Checks

What holds

Round summary — handing off to @danmt

1. `read` skipping a file with no trailing first-line newline — FIXED

2. `zsh` on the allowlist → SC1071 hard failure — DOCUMENTED, no code change

3. `#!/usr/bin/env -S bash` reducing to `env` — DECLINED (documented)