The shellcheck class check only covers *.sh, so bin/cast could drop out of the sweep silently #121

New issue

Closed

opened 2026-07-19 23:54:14 +00:00 by dan-claude-bot · 0 comments

dan-claude-bot commented

2026-07-19 23:54:14 +00:00

(Migrated from github.com)

Non-blocking observations from claude-bot-andresmgsl's review of #119, filed so they do not rot in a PR thread.

The residual gap

#119 derives the lint set from git ls-files '*.sh' plus a shebang scan that picks up extensionless scripts — which is how bin/cast gets linted without being named.

But the class check asserts only that the swept set covers git ls-files '*.sh'. bin/cast is not a *.sh file, so it is covered by the derivation and not by the assertion. If the shebang-matching branch were ever broken or deleted, bin/cast would drop out of the sweep silently while the check stayed green.

That is the same failure mode #118 was about — a lint sweep quietly narrowing while CI passes — one level in from where #119 closed it.

Suggested fix

A one-line floor assert alongside the existing check:

printf '%s\n' "${files[@]}" | grep -qx 'bin/cast'

or a minimum-count assert. Either turns a silent narrowing into a red step. Cheap, and it closes the guard's own blind spot.

Smaller notes from the same review (all theoretical here)

shellcheck-all.sh:41 — IFS= read -r line <"$f" || continue skips a file whose first line lacks a trailing newline: read returns 1 at EOF even though line is populated. A shebang-only file with no final newline would be silently unswept.
zsh in the interpreter allowlist (:50) — shellcheck emits SC1071 (unsupported shell) for zsh, so adding a zsh script would make the sweep fail hard. Loud failure is arguably right; just worth naming that the outcome is "blocked", not "linted".
#!/usr/bin/env -S bash shebangs reduce to env and would be missed. Not used anywhere in the repo today.
No shellcheck version pin — CI relies on the version preinstalled on ubuntu-latest. Severity behaviour could shift with runner image updates.

Why filed rather than fixed in #119

#119 had three head-current approvals; a new commit would have invalidated them for a guard-against-reintroduction on a sweep that is correct today. The reviewing bot's own framing was "fine as a follow-up or never."

Refs

#119 (where these were raised), #118 (the original sweep gap), heavy-duty/box#112 (the class-check precedent)

Non-blocking observations from `claude-bot-andresmgsl`'s review of #119, filed so they do not rot in a PR thread. ## The residual gap #119 derives the lint set from `git ls-files '*.sh'` **plus** a shebang scan that picks up extensionless scripts — which is how `bin/cast` gets linted without being named. But the class check asserts only that the swept set covers `git ls-files '*.sh'`. `bin/cast` is not a `*.sh` file, so it is covered by the *derivation* and not by the *assertion*. If the shebang-matching branch were ever broken or deleted, `bin/cast` would drop out of the sweep silently while the check stayed green. That is the same failure mode #118 was about — a lint sweep quietly narrowing while CI passes — one level in from where #119 closed it. ## Suggested fix A one-line floor assert alongside the existing check: ```bash printf '%s\n' "${files[@]}" | grep -qx 'bin/cast' ``` or a minimum-count assert. Either turns a silent narrowing into a red step. Cheap, and it closes the guard's own blind spot. ## Smaller notes from the same review (all theoretical here) 1. **`shellcheck-all.sh:41`** — `IFS= read -r line <"$f" || continue` skips a file whose *first* line lacks a trailing newline: `read` returns 1 at EOF even though `line` is populated. A shebang-only file with no final newline would be silently unswept. 2. **`zsh` in the interpreter allowlist (`:50`)** — shellcheck emits SC1071 (unsupported shell) for zsh, so adding a zsh script would make the sweep fail hard. Loud failure is arguably right; just worth naming that the outcome is "blocked", not "linted". 3. **`#!/usr/bin/env -S bash`** shebangs reduce to `env` and would be missed. Not used anywhere in the repo today. 4. **No shellcheck version pin** — CI relies on the version preinstalled on `ubuntu-latest`. Severity behaviour could shift with runner image updates. ## Why filed rather than fixed in #119 #119 had three head-current approvals; a new commit would have invalidated them for a guard-against-reintroduction on a sweep that is correct today. The reviewing bot's own framing was "fine as a follow-up or never." ## Refs #119 (where these were raised), #118 (the original sweep gap), heavy-duty/box#112 (the class-check precedent)

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#121

No description provided.