Skip to content

ci(lint): gate the bash-3.2 floor repo-wide over every shell file the repo ships - #12319

Merged
yinlianghui merged 2 commits into
mainfrom
claude/issue-12221-bash4-construct-gate
Aug 25, 2026
Merged

ci(lint): gate the bash-3.2 floor repo-wide over every shell file the repo ships#12319
yinlianghui merged 2 commits into
mainfrom
claude/issue-12221-bash4-construct-gate

Conversation

@yinlianghui

Copy link
Copy Markdown
Collaborator

Fixes#12221

A repo-wide check: for bash-4-only constructs in the shell this repo ships, with a --self-test that runs the gate against a known-bad fixture tree so it can be shown to fail rather than asserted to work.

/usr/bin/env bash is bash 3.2.57 on macOS (Apple ships no bash 4+, for licensing reasons) and CI runs bash 5, where every construct this gate refuses works perfectly. So the class is invisible to a normal green run in both directions — the defect passes CI and so does its repair — and it surfaces on an operator's Mac at status 127, at the moment they most needed the script to work.

Surface

filechange
scripts/check-bash32-floor.mjsnew gate + --self-test (98 cases)
package.jsonone check:bash32-floor alias
.github/workflows/lint.ymlone step

Nothing else. In particular: the two existing file-scoped scans are not touched.

⛔ This does not supersede the two file-scoped scans

check:objectui-changeset's R7 leg is load-bearing for #12071's property and is paired with R7a/R7a2/R7b, a simulation harnessenable -n mapfile readarray via BASH_ENV, plus unset of the bash-5 variables — that proves the real code path completes without the builtins. The verify lock's own self-test does the same for its acquisition path.

That pairing is not redundant with a static scan, and this PR depends on it: a static scan cannot see a construct assembled at runtime (eval "mapfile -t x < f", bash -c 'mapfile ...'). That hole is stated in the gate's header as a deliberate limit, and it is precisely the half the simulated runs hold. Two instruments, one class, different blind spots. I found nothing redundant to remove.

Question 1 — the construct set, and why each exclusion

15 constructs, all of them. Every candidate on the card cleared the two bars that matter: it is above the 3.2 floor, and it can be anchored precisely enough not to fire on 3.2-legal text.

mapfile/readarray (4.0) · declare -A/local -A/typeset -A/readonly -A (4.0) · declare -n/-g (4.3/4.2) · coproc (4.0) · wait -n (4.3) · shopt -s globstar/lastpipe (4.0/4.2) · exec {fd}> (4.1) · ${x^^}/${x,,} (4.0) · ${x@Q} (4.4) · ${arr[-1]} (4.2) · ;;& (4.0) · [[ -v (4.2) · printf '%(fmt)T' (4.2) · &>> (4.0) · EPOCHSECONDS/EPOCHREALTIME (5.0).

Excluded, with the reason:

  • &> (non-append) — checked, not recalled: this is a near neighbour of &>> and it is not bash 4. It redirects both streams on 3.x, so flagging it would refuse a portable line. &>> is 4.0 and stays in. Pinned green in --self-test.
  • ;& (single-semicolon case fall-through) — genuinely 4.0, and deliberately out: unlike ;;& the two-character literal is not anchorable, occurring in sed programs, URLs, and ordinary cmd ; sequences. A pattern that cannot be anchored buys false positives, and a false positive on this gate is what re-opens the "just add an allowlist" door.
  • bare-word globstar/lastpipe — narrowed rather than dropped: matched only as shopt arguments, the one position where they change shell behaviour.
  • declare -a/-r/-i/-p, ${x:0:1}, ${x//,/ }, ${#arr[@]}, read -n 1, exec 9> — checked and found 3.2-legal; each is pinned green in --self-test so a later widening of a pattern cannot quietly start refusing portable shell.

⚠️Honest limit on the version column. The GNU documentation hosts (gnu.org, tiswww.case.edu, savannah.gnu.org) are egress-blocked from this seat, so the per-row since could not be re-verified against upstream NEWS. The tier-1 six carry the versions this repo already recorded beside its own repairs; the rest carry the reference manual's. This is stated in the table's docblock rather than hidden, and it is not load-bearing: nothing branches on since — the floor is 3.2 and every row is above it, so the verdict is identical whether a construct arrived in 4.0 or 5.0. The column exists so a failure message can tell a reader why their green local run proves nothing.

What is verified, on every run: each pattern matches a real, bash -n-parseable instance of the construct it claims to describe, and does not match the 3.2 replacement beside it. That is the property that actually decides findings, and a scanner's silent death is a pattern matching nothing real.

Question 2 — telling a hunter from a user

Measured first. With the tree's existing rule (full-line comments exempt, everything else a use) a repo-wide scan produces 8 findings, every one a false positive, all in scripts/pm/os-verify-lock.sh — the card's "reddens itself on day one", reproduced. So a comment-only exemption is not enough, and this is why:

linesshapewhy it is not a use
210, 225, 252local s="${EPOCHSECONDS:-}"a guarded read — the :- supplies a value when the name is unbound, so the line behaves identically on 3.2
1134, 1267unset EPOCHSECONDS EPOCHREALTIMEa bare word is not a read at all; on 3.2 this is a no-op
1135enable -n mapfile readarrayremoves the builtin — what a simulated-3.2 harness does, the opposite of a use
1140st_case '…and mapfile disabled' …a token inside a quoted argument invokes nothing

Not an allowlist of filenames — that rots, and it is how this class survived. ⛔ Not a waiver comment either: os-verify-lock.sh is out of surface here, so any rule requiring an edit there is not a rule this PR can adopt. The exemptions are three properties of the shell, each mechanically decidable from the line alone:

  • E1 — full-line comment (^\s*#). Inherited verbatim from objectui-changeset-digest.mjs, whose own comment states the reason: a file refusing a construct has to name it. Full-line only; a trailing comment does not exempt the code beside it.
  • E2 — a variable is only read through a sigil, and a guarded read is the fix.$EPOCHSECONDS / ${EPOCHSECONDS} are unguarded reads → RED. ${EPOCHSECONDS:-} is not a tolerated mention, it is the repair. A bare word is not a read.
  • E3 — a builtin only executes in command position. Start of line, or after a separator, allowing leading reserved words and VAR=value prefixes.

Each is pinned in both directions in --self-test, including the exact line shapes from the table above.

Question 3 — the population, measured

.sh is the wrong population, and the measurement says so on this tree. Membership is a .sh name or a shell shebang whatever the name:

population by .sh: 18
population by shebang-or-.sh: 20
EXTRA (shebang, no .sh): .githooks/pre-commit :: #!/bin/sh
EXTRA (shebang, no .sh): .githooks/pre-push :: #!/bin/sh

Two tracked hooks a *.sh glob runs straight past — and their #!/bin/sh floor is tighter than bash 3.2, not looser (/bin/sh is bash 3.2 in posix mode on macOS, dash on Debian). An extension-only census would have declared this population complete at 18.

Deliberately out:package.json script bodies and heredocs inside .mjs. Both really can carry shell; both are excluded for one reason — the scanner would have to decide which spans of a non-shell file are shell before it could judge a line, and a wrong answer there fabricates a finding out of JavaScript. The population is files whose whole content is shell, which is decidable from the name and the first line and from nothing else.

Discovery reads the git index, so an ignored or generated file is never scanned and a newly tracked script is covered the moment it is staged. An empty population is a refusal, not a quiet pass (#4690).

Question 4 — the census, re-measured on this base

readingcard (at b9438a425d)this branch
tracked *.sh1818
tracked shell by shebang-or-.shnot measured20
carrying a construct (comment-only rule)08, all false positives
carrying a construct (this gate's rule)0

The .sh count is unchanged; the population grew by 2 the moment the census stopped being extension-shaped. That is itself the answer to question 3.

Landing obligations — both made unreachable, no ledger moved

Rather than buying a row in either shrink-only ledger, the population roots are spelled as subtree globs, and the walk roots are derived from them so declaration and scan cannot drift:

exportconstPOPULATION_ROOTS=['scripts/**','.claude/hooks/**','.githooks/**'];exportconstWALK_ROOTS=POPULATION_ROOTS.map((glob)=>glob.replace(/\/\*\*$/,''));

They are source literals on purpose — an assembled `${r}/**` builds no watch hint at all, which is the invisible half of the bare-root species. Pinned in --self-test, including that no walk root appears anywhere in the file as a bare literal.

Both tools run deliberately, verdicts quoted:

$ node scripts/pm/bare-root-worklist.mjs --self-test
OK self-test: 46 live row(s), 39 unreachable as spelled, 39 recorded verdict(s) — none stale,
none missing.
$ pnpm check:pm-dispatch-gates
✓ dispatch-gates self-test: 633 cases pass.

bare-root-worklist names no row for this gate and reports 0 untriaged row(s), so the TRIAGE map needs no verdict. escapableLiteralRows skips any hint carrying /, so no escapable-literal row either. Neither ledger moves.

And the declaration does the job it exists for — the gate is nameable by a dispatch brief:

$ node scripts/pm/dispatch-gates.mjs scripts/build-console.sh .githooks/pre-push
- pnpm check:bash32-floor [lint.yml] matched via scripts/build-console.sh ⇢ gate source
'scripts/**'; .githooks/pre-push ⇢ gate source '.githooks/**'

Ablation — direction predicted in writing, then measured

Predicted before running: planting mapfile -t _abl < /dev/null at command position in a real tracked script turns the production leg RED with exactly one finding naming that file and line as mapfile / readarray — bash 4.0; --self-test stays green, because its verdict does not read tree contents.

Observed, exactly that. The mutation was confirmed on disk before any reading was taken — the edit tool's exit code is not evidence, since perl -i exits 0 on zero matches:

== anchor present before mutation: 1 ==
== injected-marker count on disk: 1 ==
scripts/build-console.sh | 1 +
PRODUCTION_EXIT=1
✗ check-bash32-floor: 1 bash-4+ construct(s) in shell this repo ships.
scripts/build-console.sh:29
mapfile -t _abl < /dev/null
mapfile / readarray — bash 4.0; the floor is 3.2 (macOS ships 3.2.57).
SELFTEST_EXIT=0 ✓ check-bash32-floor self-test: 98 cases pass.
· real tree: 20 shell file(s) — 1 finding(s)

Restore proven byte-identical, with a trap … EXIT INT TERM so a container SIGTERM mid-mutation could not leave the tree mutated:

HEAD blob : 6be4b62b2b221622aa8a8b1a6da2500633aad58a
on-disk blob: 6be4b62b2b221622aa8a8b1a6da2500633aad58a
byte-identical: YES
marker absent after restore: 0 (want 0)
git status --porcelain: []

No build step is involved — the gate is a plain .mjs read directly by node, with no dist/ between the edit and the run — so there is no stale-artifact direction for this ablation to fall into.

Gates

Union derived at the final commit 185e17abc4 with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack. The first derivation printed ⚠️ STALE TREE (3 commits behind, objectui-changeset-digest.mjs and dispatch-gates.mjs both changed across the range — #12298 landing); origin/main was merged in and the union re-derived from 185e17abc4, where the warning is gone. Every exit code captured before any pipe.

All 25 matched families green, plus check:nul-bytes and the two obligation tools:

check:bash32-floor · check:agent-test-spelling · check:cli-command-ids · check:cross-package-test-inputs · check:entry-guard · check:node-version · check:parse-guard · check:pnpm-acquisition · check:pnpm-filter-targets · check:required-contexts · check:shard-attestation · check:type-check-coverage · check:workflow-status-functions · check-aggregator-roster · check-ci-filter-parity · check-cross-package-test-inputs · check-required-contexts · check-self-test-wired · check-self-test-workflow-commands · check-shard-attestation · check-step-collectors · check-whole-set-label-write · docs-audit/check-drift-comment · pm/ci-failure --self-test · check:nul-bytes · check:pm-dispatch-gates · pm/bare-root-worklist --self-test

The self-test-wiring obligation is discharged rather than assumed:

✓ check-self-test-wired: every one of the 138 script(s) CI runs that ship a
`--self-test` has that self-test run by CI.

Repo-wide ESLint run in full, not narrowed:

$ pnpm lint # eslint . --no-inline-config
LINT_EXIT=0 os-verify-lock: VERDICT command-exit 0 · held the lock 73s

One declared narrowing, with its warrant:check:type-check-debt (check-type-check-coverage.mjs --re-measure) was not run. It refuses by design without the built dependency closure on disk — Error: --re-measure cannot run: 56 workspace dependenc(ies) … have no built type entry point, which is the gate correctly refusing to measure a different world (#6376), not a failure. Building that closure is a full-farm turbo build, and this diff contains zero TypeScript files and no tsconfig.json or package typecheck script, so no number in DEBT/TEST_DEBT can move. The structural half ran green: ✓ check:type-check-coverage --self-test — 47 semantic + 65 observation + 29 re-measure + 28 built-closure + 19 auto-lowering case(s) hold and check-type-check-coverage: OK — 65/78 workspace packages type-checked. CI builds the closure before that step and runs it regardless.

Changeset

None — skip-changeset. scripts/** + a workflow step + one root package.jsoncheck:* alias publish nothing.

Out of surface, on purpose


Generated by Claude Code

… repo ships
`/usr/bin/env bash` is bash 3.2.57 on macOS, and CI runs bash 5 — so a
bash-4-only construct in a hand-run script is invisible to a normal green
run in both directions: the defect passes and so does its repair. Two
incidents, four sites, both found by hand and late.
Adds `scripts/check-bash32-floor.mjs`, its `--self-test` (98 cases,
including a known-bad fixture tree the gate is shown to fail on, and a
simulated-3.2 harness proving the flagged constructs really break once
the bash-4 capability is removed), a `check:bash32-floor` alias, and a
`lint.yml` step.
The population is tracked shell under declared subtree globs — a `.sh`
name OR a shell shebang, which is the half a `*.sh` glob misses: it adds
`.githooks/pre-commit` and `.githooks/pre-push`, whose `#!/bin/sh` floor
is tighter than bash 3.2, not looser (18 -> 20 files).
Legitimate mentions are exempted by three properties of the shell, never
by a filename allowlist: full-line comments, guarded `${VAR:-}` reads,
and command position for builtins.
The two existing file-scoped scans are NOT superseded — they pair their
scan with a simulated run that reaches runtime-assembled commands a
static scan cannot see.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UjM2ia8Av1v5NqfqQEQmC6
@yinlianghuiyinlianghui added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 25, 2026 — with Claude
@github-actionsgithub-actionsBot added size/l ci/cd dependencies Pull requests that update a dependency file labels Aug 25, 2026
@yinlianghuiClaude

Copy link
Copy Markdown
CollaboratorAuthor

ACCEPT — PM review of #12221.

I did not read this off your table. Four claims re-measured here, on origin/main as it stands now (0d91eaa6ea), which has moved 4 commits past this branch's base (15249270fb).

1. The census, re-measured independently

tracked *.sh (repo-wide) : 18
population by shebang-or-.sh under the 3 declared roots : 20
EXTRA: .githooks/pre-commit :: #!/bin/sh
EXTRA: .githooks/pre-push :: #!/bin/sh

Same two files, same shebangs, same 18→20. Note what falls out of the two numbers together: all 18 .sh files already live under the three declared roots, so the roots are complete for the extension half as well — the globs are not merely sufficient for the delta you found.

2. The delta is coverage, not a bigger number — ablated in the file the argument is about

Your ablation was in scripts/build-console.sh, a file the old *.sh shape already reached. That leg cannot distinguish "the gate works" from "the gate works on the population we already had". So I ran it against the file the census argument rests on:

== marker on disk: 1 (want 1) ==
ABLATED_EXIT=1
✗ check-bash32-floor: 1 bash-4+ construct(s) in shell this repo ships.
.githooks/pre-push:2
mapfile -t _pmabl < /dev/null
mapfile / readarray — bash 4.0; the floor is 3.2 (macOS ships 3.2.57).
== restored, marker: 0 (want 0); status: [] ==

The +2 is reachable coverage.

3. The stale-base reading, taken on the tree CI will actually build

Your union was derived at 185e17abc4 against a base that is now 4 commits old — the same shape that put a previous card of this lane's red into main. The range matters here specifically: #12306 landed in it, and it rewrote a bash-3.2 comment block inside this gate's own population (scripts/bump-objectui.sh), naming mapfile, readarray, declare -A, ${x^^}, ${x,,}. So I re-ran your gate against origin/main's tree:

PRODUCTION_EXIT=0
✓ check-bash32-floor: 20 tracked shell file(s) under scripts/**, .claude/hooks/**, .githooks/**
name no bash 4+ construct outside a comment, a guarded ${VAR:-} read, or a non-command position.
census: 18 by .sh extension, 2 by shebang alone; 15 constructs checked, floor bash 3.2.
SELFTEST_EXIT=0 ✓ check-bash32-floor self-test: 98 cases pass.

E1 holds against the newest text in the population, not just the text that existed when you measured.

4. Both ledgers — checked at the source, not from the quoted verdict

A tool's own 0 untriaged row(s) is the thing under test when the question is "will this create an obligation nobody can see". Read at origin/main:scripts/pm/dispatch-gates.mjs:2199:

if(hint.includes('/')||plain.startsWith('.'))continue;

All three declared roots carry /; two also start with .. No escapable-literal row exists by construction, not by a verdict that could change. That is the remedy shape this lane wanted — neither ledger is a place a future gate has to remember to visit.

On the one limit you flagged

The egress-blocked since column is accepted as stated. It is inert by the gate's own structure: the floor is 3.2 and every row is above it, so no verdict changes whether a construct arrived in 4.0 or 5.0. Flagging it in the docblock rather than quietly carrying it is the right call. What decides findings — each pattern matching a real bash -n-parseable instance and not matching its 3.2 replacement — is verified on every run, and that is the property that matters.

Your refusal to supersede the two file-scoped scans is also correct and I am not going to re-open it: a static scan cannot see eval "mapfile -t x < f", and the simulated-3.2 harness holds exactly that half.

Landing conditions

  1. Lint & Repo Gates reports completed + success, read by name — not from an aggregate and not from a check_suite.completed notice.
  2. Nothing else. The branch being 4 behind is handled by the merge queue building the merge ref; my §3 reading already covers the one file in the range that touches this gate's population.

Generated by Claude Code

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci/cddependenciesPull requests that update a dependency filesize/lskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A repo-wide check: for bash-4-only constructs in tracked *.sh — measured 4-for-4, but it needs a new gate file plus wiring

2 participants

@yinlianghui@claude