Uh oh!
There was an error while loading. Please reload this page.
tooling(pm): refuse to render a tail the cut left unresolved - #15126
Merged
Conversation
`DIRECT_CHECK_INVOCATION` ends an argv tail at the first `;|&<>()`, which is
the shell boundary it has always drawn. A value written as a command
substitution is therefore cut INSIDE its own construct:
`--base "$(git merge-base origin/main HEAD)"` leaves the tail `--base "$`,
`argvTokens` swallows the unclosed quote into one token, and the token carries
no `WORKFLOW_VALUE_SOURCE` hit — so it classified as a LITERAL and the key
`scripts/check-x.mjs --base "$` rendered into `--commands` as a runnable
command. A truncated argv that looks runnable is the outcome this file's own
docblock names as the worst of the three.
`unresolvedRemainder` reads the tokeniser's own unterminated branch back off
the finished token and names what it could not resolve. That remainder is a
value the workflow supplies just as surely as `${{ … }}` is, so it rides the
channel the classification already has: any entry in `variables` marks the row
NOT RUNNABLE LOCALLY and keeps the key out of `--commands`. This is the file's
rule applied rather than a new one — a missing lead, never a fabricated one.
Refusing the argv half instead would hand back a BARE path key, and a bare run
is an invocation CI never makes: the fabrication the value-bearing render
retired.
One predicate covers the whole class, because the three spellings are one
state: an unclosed quote (`$(…)`, or a terminator inside a quoted value like
`--gate 'a;b'`), an unclosed `${{ … }}` (an expression holding a terminator),
and a token left ending in a bare `$`.
Measured at this commit: 114 direct invocations across `.github/workflows` and
zero carry any of them — the three paren hits in the workflow text are comment
lines. `--commands` for `ci.yml` and `pr-automation.yml` is byte-identical
before and after. The truncation pin now asserts, over every derived key, that
carrying a truncation and rendering as runnable are mutually exclusive, read
with a quote scanner independent of `argvTokens`; a written fixture carries the
non-vacuous subjects (the `$(…)` shape, the `'a;b'` shape, and the control that
`--gate 'Test Core'` and `--shard ${{ matrix.shard }}` classify exactly as
today), and the live half asserts the same property over the workflows.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019RfFHiRCSs3JXLK4cwcfox
This was referenced Sep 4, 2026
os-steve
marked this pull request as ready for review
September 4, 2026 02:58
os-steve
enabled auto-merge
September 4, 2026 02:58
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Sep 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#15116
scripts/pm/dispatch-gates.mjsonly. One commit, verified at headb52fcc23.The shape, reproduced at the base before any edit
At
f594e70d(the merge base and this branch's base), a fixture workflow written in the self-test's own style:derives, through
extractCheckInvocations:The key is
scripts/check-x.mjs --base "$— cut at the(, unclosed quote swallowed into one token byargvTokens, noWORKFLOW_VALUE_SOURCEhit, so zero variables: a LITERAL, which is what puts a row into--commandsas a command a dev can paste. That is the truncated argv the file's own docblock names as the worst of the three outcomes.The live tree, re-measured at this base
114 direct invocations across
.github/workflows/*.yml. Zero carry an unclosed quote, an unclosed${{ ... }}or a token ending in a bare$; zero direct matches are cut at a paren. The three paren hits in the workflow text are all comment lines:This confirms the seat's reading at
e98bfc87: the defect is latent, and no derived key moves.Outcome taken: (i) classify the unresolved remainder as a VARIABLE
Measured against the file's existing invariants, outcome (ii) — refuse the argv half and keep the bare key — is the one the file already rules out.
renderedArgv's docblock reservesnullfor "an invocation with no argv at all — that one keeps the bare path key, because bare is what CI runs", and the #15083 half of the same docblock says a bare key for a script CI only ever invokes with argv "is not a neutral omission — it is a command this tool made up". Refusing here would re-mint exactly that fabrication for a script whose only invocation carries a value. Outcome (i) leaves the invocation named, with its workflow named, on the NOT RUNNABLE LOCALLY line, and out of--commands: a missing lead, never a fabricated one.On the four axes:
--commandsoutput verbatim, and--ranreconciles against those same strings. A truncated key that renders runnable hands that loop a command that exits non-zero for a reason unrelated to the tree. The producer side is live too:$(git merge-base ...)is the ordinary spelling for the--basegates, andpr-automation.ymlalready pins a base — today through a step output rather than a substitution, which is why the shape is latent rather than active.DIRECT_CHECK_INVOCATION's terminator set. The cut at(stays the shell boundary it has always been; this reads what the cut left. No workaround, and nothing downstream re-derives the answer: the classification travels on the invocation as it already did.argvVariables.lengthalready drivesnotRunnableindiscoverFamilies; the fix adds entries to a list that already exists. Net surface: one module-private function.The fix
unresolvedRemainder(token)replays the tokeniser's own unterminated branch over the finished token and returns what it could not resolve, or null.renderedArgvnames that remainder invariablesalongside theWORKFLOW_VALUE_SOURCEhits.Three spellings reach that state and they are one state, so they get one predicate rather than a list — the contract's "extend one guard to close the whole class":
"$from a$(...)value;'afrom a terminator inside a quoted value like--gate 'a;b'),${{ ... }}(a GitHub expression holding a terminator —${{ inputs.x || 'y' }}is the live-looking spelling),$(the same cut with the value unquoted).The replay is faithful to
argvTokensbecause that branch copies the remainder verbatim (text.slice(i)) and a closing delimiter is always inside the token that used it — so the replay reaches the same verdict the tokeniser did. The unbalanced state is therefore observable AFTER tokenisation (the mechanism question the dispatch asked): it did not have to be detected during it.After the fix, the same fixture:
The control is unchanged: the quoted literal still contributes no variable, the expression still contributes exactly one.
The pin, extended rather than replaced
The truncation pin kept both of its existing clauses (no continuation backslash; no redirection fd surviving as an argument) and gained a third, stated over every derived key as the contrapositive:
That is the form outcome (i) makes true. A flat "every key has balanced quotes" would be unsatisfiable beside a fixture that quotes the
$(...)shape, because under outcome (i) the key keeps the truncation and is made harmless by the classification rather than by the key text.The pin reads the key with a quote scanner independent of
argvTokens— the shell's own outermost-quote rule, so a'inside a"..."is text, not a delimiter. A parity count would not do: zero live keys nest a quote today, and the day one does a parity scan would call a balanced key truncated.The same property is asserted on the live half (
directinvocations read from the workflows). It is vacuous today by the measurement above, and says so in its comment; the non-vacuous subjects are in the fixture.Fixtures
A written fixture — the first in that block that is not quoted from live workflow text, and it says so, because the tree does not have this shape:
node scripts/check-x.mjs --base "$(git merge-base origin/main HEAD)"— the key really isscripts/check-x.mjs --base "$(pinned, so the case cannot go vacuous) and it is NOT runnable, remainder"$named.node scripts/check-y.mjs --gate 'a;b'— a terminator inside a quoted value lands the same way, remainder'anamed.node scripts/check-z.mjs --gate 'Test Core' --shard ${{ matrix.shard }}— tokenises and classifies exactly as today, one variable, the quoted literal contributing none.Unit-level cases were added to the existing
renderedArgvandargvTokenspins on the same call rather than as new cases, including the closed-quote control on the same line.--commandsbefore and after — byte-identicalf594e70d59cf418666b26b5fb80386b8f8ab54ccb52fcc2359cf418666b26b5fb80386b8f8ab54cccmpreports the two byte-identical. No live key changes.Reverse verification (ablation)
From the committed state, the naming of the remainder in
renderedArgvwas replaced with an inert binding. Landing proved on disk before measuring: removed-text hits 1 to 0, injected-marker hits 1, worktree blob1be05934...different from the HEAD blob36500aff.... No build ordist/is involved — this script is run from source bynode.Direction was as predicted: turns red. Restore leg proven the same way — restored blob
36500aff...equal to the HEAD blob,git diff HEADempty (0 bytes), marker hits back to 0, naming back to 1.Gates — derived at head
b52fcc23, each quoting its own verdict lineRe-derived with
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands(no paths). The derivation at this head returns the dispatch list minusnode scripts/pm/bare-root-worklist.mjs --self-test; it was run anyway, per the card's clause, and is green.node scripts/check-ci-filter-parity.mjsOK: all 143 declared cross-package glob(s) (99 unique) are covered by core or crosspkg …node scripts/check-closing-keyword-parity.mjscheck-closing-keyword-parity: OK (3 parsers agree on all 9 keywords …)node scripts/check-closing-keyword-parity.mjs --self-test✓ check-closing-keyword-parity --self-test: 24 assertions, 5 mutations of the shipped parsers each driven to red.node scripts/check-comment-mask-corpus.mjs✓ comment-mask corpus sweep: 5853 files, 0 disagree, 0 unparseable, 40.5snode scripts/check-self-test-wired.mjs✓ check-self-test-wired: every one of the 166 script(s) CI runs that ship a --self-test has that self-test run by CI.node scripts/check-self-test-wired.mjs --self-test4 live ledger row(s) verified … 7 declared batteries, 50 cases registered, every battery at or above its pinned floor.node scripts/check-whole-set-label-write.mjs✓ check-whole-set-label-write: 0 violations — 257 file(s) over 3 root(s) …node scripts/check-whole-set-label-write.mjs --self-test✓ all cases pass (24 fixture trees + 5 refusals + 1 allowlist hatch)node scripts/pm/bare-root-worklist.mjs --self-testOK self-test: 69 live row(s), 61 unreachable as spelled, 61 recorded verdict(s) — none stale, none missing, none contradicted.pnpm check:agent-test-spelling✓ check-agent-test-spelling: 0 violations — 437 file(s) …pnpm check:bash32-floor✓ check-bash32-floor: 26 tracked shell file(s) … name no bash 4+ construct …pnpm check:cli-command-ids✓ check-cli-command-ids: 324 command-id literal(s) across 114 file(s) … all resolvepnpm check:cross-package-test-inputsOK: 26 package(s) read outside themselves, all declared, and turbo.json hashes every declared glob.pnpm check:declared-population-live✓ 198 of 249 famil(ies) declare a path population, and every one of them reaches this tree's 8233 tracked file(s).pnpm check:entry-guard✓ check:entry-guard: 206 scripts/ file(s) — every entry guard goes through invoked-as.mjs …pnpm check:nul-bytes✓ check-nul-bytes --self-test: 75 assertions over a temp git repo (real scan() path)pnpm check:parse-guard✓ check:parse-guard: 205 scripts/ file(s) — every TypeScript parse goes through ts-parse.mjs.pnpm check:pm-dispatch-gates✓ dispatch-gates self-test: 1343 cases pass.pnpm check:pnpm-filter-targets✓ 142/181 --filter occurrence(s) across 33 file(s) resolve against 79 workspace package(s) …pnpm check:refd-timer-probeOK check-refd-timer-probe: 5848 source file(s) swept …pnpm check:watch-hint-literal✓ check-watch-hint-literal: 48 declaration(s) across 4 rostered name(s) …pnpm lint(repo-wide ESLint, through the lock)os-verify-lock: VERDICT command-exit 0 · held the lock 59s · waited 187s— eslint printed no findingsEvery exit code was captured by redirecting first (
cmd > file 2>&1; EXIT=$?), never read across a pipe. The two long runs (--self-test,pnpm lint) went throughscripts/pm/os-verify-lock.sh; their VERDICT lines are the readings quoted, and the lock's own disclaimer applies — those seconds are shared-box seconds.check-test-completenessis a value-bearing family and is NOT MEASURED here, in its own words.The script's own suites, owed separately from the derived families: its
--self-test(1343 cases, green) andscripts/pm/check-dispatch-gates.mjsviapnpm check:pm-dispatch-gates(green).git grep dispatch-gatesover*.test.tsreturns five files; all five mention it in prose comments only and none pins this script.scripts/pm/bare-root-worklist.mjs --self-testis green at this head — no finding to report there, as expected since no live key changes.scripts/pm/bare-root-worklist.mjswas not touched; #15091 remains a separate flight.Nothing is published from any package, so this carries the
skip-changesetlabel rather than a changeset.Generated by Claude Code