Skip to content

fix(devx): read a quoted blob as an argument, not as a program being run (#10889) - #10904

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-10889-invokes-script-quote-aware
Aug 22, 2026
Merged

fix(devx): read a quoted blob as an argument, not as a program being run (#10889)#10904
os-zhuang merged 1 commit into
mainfrom
claude/issue-10889-invokes-script-quote-aware

Conversation

@claude

@claudeclaudeBot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Fixes#10889

invokesScript() gained adjacency for #6589, but the tokenizer under it was
command.split(/\s+/) followed by token.replace(/^['"]|['"]$/g, ''). Shell does not
word-split inside quotes, so a quoted string handed to echo was torn into pieces and the
script name plus its flag inside that string read exactly like a program being run with an
argument.

Measured against the real exported invokesScript on origin/main (145ba75), before any
edit:

ok | got=true want=true | genuine invocation
MISS | got=true want=false | ECHO quoting it (THE FINDING)
ok | got=false want=false | ::error:: fused (card case 3)
MISS | got=true want=false | ::error:: WITH A SPACE
MISS | got=true want=false | echo "run node ... to reproduce"
ok | got=true want=true | QUOTED ARG on real invocation
ok | got=false want=false | #6589 bare --verify substring
ok | got=false want=false | #6589 git show-ref --verify

The card's warning about the third row holds exactly: echo "::error::check-shard-attestation.mjs --verify exited 1"
is green only because ::error:: fuses onto the basename token. Write the same annotation
with a space and it counted.
Both spellings are now pinned, because a fix that only handles
the obvious one leaves the fragile one behind.

Direction is why this was worth fixing: invokesScript feeds presence questions -- does an
attesting job end with the --emit pair, does an aggregate gate run --verify -- and a presence
assertion fails toward a silent green. The symptom is the guard reporting that a leg attests
when it does not.

The packaging decision: shared, and the sibling now imports it

The two files already had exactly one import edge, and it points
check-required-contexts -> check-shard-attestation
(that file has imported
shellCommands from this one since #10884). That settles the question mechanically rather than
by taste: a helper kept in the importer can never be reached from the importee without a
cycle, so "adopt the sibling's helper" means moving it down to the lexer whose output it
consumes. commandWords and invokes now live beside shellCommands -- commands out of a
run: block, then words out of a command, one layer above the other in one home -- and
check-required-contexts.mjs imports them over the edge that already existed.

Rejected alternatives, with reasons:

  • Re-derive locally. That is the hand-mirror fix(scripts): guard check-cross-package-test-inputs dispatch, and stop mirroring globToRegExp #10628 had to undo. Its mirror was justified
    at the time by a real obstacle -- the source module ran its gate at load time, so importing it
    executed a second gate. That obstacle does not exist here: check-shard-attestation.mjs has
    been behind isEntrypoint() all along, which is why the sibling already imports from it.
  • A new scripts/shell-words.mjs. A third home for a two-layer lexer whose lower layer is
    already exported from, and imported out of, this file. Keeping the layers together would mean
    moving shellCommands too -- a larger blast radius for no gain.

One correction to the card, since it moves the decision. The card cites #10628 as a caution
that "extract a shared module is not automatically the right answer". Read directly, #10628
deleted a hand-mirrored helper and replaced it with a shared import, after measuring the
two copies textually and behaviourally identical. It is evidence against mirroring, and it
argues for sharing wherever the import is safe -- which here it demonstrably is.

The asymmetry, preserved

The program word must be unquoted; arguments may be quoted freely. Refusing a quoted
--verify would narrow a presence assertion into a false red, which is the failure mode that
gets a pin loosened rather than fixed. Pinned in both directions.

#6589 is not regressed

Every #6589 pin is untouched and green: the script in one command with the flag in another,
git rev-parse --verify, git show-ref --verify on a continuation line, the leading- and
trailing-comment cases, and the && boundary. The bare-substring case that motivated adjacency
stays false, and the two quoted-# / quoted-|| cases stay true.

The live verdict did NOT move

The card says this is latent, not currently mis-firing, and that is what the tree shows. The
real pass is byte-identical before and after:

before: ✓ check-shard-attestation: 2 aggregate gate(s) count 3 declared leg(s) across 3 attesting job(s).
after: ✓ check-shard-attestation: 2 aggregate gate(s) count 3 declared leg(s) across 3 attesting job(s).

Self-test, 92 -> 103 assertions (elisions marked; the rest is the tool's own line, verbatim):

before: ✓ check-shard-attestation --self-test: 92 assertions (dominance experiment + [...] + the #6589 classifier pins).
after: ✓ check-shard-attestation --self-test: 103 assertions (dominance experiment + [...] + the #6589 classifier pins + the #10889 quoting pins).

Ablations -- predictions written down BEFORE each run

Both self-tests collect failures rather than aborting, so each run lists the full failing set
and a prediction can name all of it. Every mutation was proven on disk by grep counts of the
injected and the removed text -- never by an editing tool's exit code. (A first attempt with
perl -0pi was a silent no-op: exit 0, counts unchanged. It was caught by the count and
redone.) Every restore leg was proven byte-identical with git diff --quiet (exit 0).

A -- drop the unquoted-PROGRAM condition only. Predicted, non-obviously, that the two halves
are independently load-bearing and that this is the narrower half: once a quoted region is one
word, the echo blobs no longer resemble the basename at all, so only a quoted word that is
exactly the program spelling needs this condition. Predicted exactly one failure in
check-shard-attestation and green in check-required-contexts.

Observed: exactly that.

check-shard-attestation --self-test -- 1 failure(s)
- #10889: a QUOTED program word is a string being passed to something else -- here, to `echo`
check-required-contexts --self-test: 150 assertions [green]

B -- revert commandWords to the pre-#10889 tokenizer (the defect itself). Predicted eight
named failures in check-shard-attestation and, critically, that check-required-contexts would
also redden
-- the proof that the helper is genuinely shared and not a leftover local copy.
Predicted not failing: the fused ::error:: pin (green even under the defect -- the accident
the card warned about) and both asymmetry pins (the old tokenizer stripped quotes off arguments,
so a quoted argument still read as present).

Observed: 8 failures, exactly the eight named, with exactly those exclusions; and the sibling
reddened with exactly the two predicted pins:

check-shard-attestation --self-test -- 8 failure(s)
- #10889: an `echo` QUOTING the invocation is prose -- the quoted blob is ONE argument, not a program being run
- #10889: ...and single quotes word-split no more than double quotes do
- #10889: prose WRAPPED AROUND the invocation is still prose
- #10889: ...and the same annotation written with a SPACE, which is the spelling that used to count
- #10889: the same defect on the `--emit` presence question, which decides whether a shard ATTESTS
- #10889: a QUOTED program word is a string being passed to something else -- here, to `echo`
- commandWords: a quoted region is ONE word and is marked as quoted; unquoted words split on whitespace
- invokes: the same reading generalised over any program -- this is the seam check-required-contexts imports (#10877/#10889)
check-required-contexts --self-test -- 2 failure(s)
- recognizer: a step that only ECHOES --verify-required-set does not INVOKE it -- the presence half must not go green on a patrol that reads nothing (#10877)
- wiring: an `echo` QUOTING the invocation is prose -- the quoted blob is one argument, not a program being run (#10877)

C -- delete the shared import. Predicted loud, never silent. Observed
ReferenceError: invokes is not defined at check-required-contexts.mjs:763, exit 1.

Gates -- union re-run against final HEAD b9dec72

Gate set derived from the real changeset by node scripts/pm/dispatch-gates.mjs with no
hand-made path list
(it read the merge base itself: 2 paths vs merge-base 145ba75, three-dot
semantics). All nine matched families green, each quoted from the gate's own verdict line:

gateverdict line
pnpm check:shard-attestation103 assertions (... + the #10889 quoting pins). / 2 aggregate gate(s) count 3 declared leg(s) across 3 attesting job(s).
pnpm check:required-contexts150 assertions (...) / 6 required context name(s) pinned across 2 workflow(s); 5 instruction surface(s) scanned against 2 retired name(s) (#9491).
pnpm check:entry-guardcheck:entry-guard: 130 scripts/ file(s) -- every entry guard goes through invoked-as.mjs; 88 export bindings, 78 of them inert on import
pnpm check:parse-guardcheck:parse-guard: 129 scripts/ file(s) -- every TypeScript parse goes through ts-parse.mjs.
pnpm check:cross-package-test-inputsAll 104 self-test cases passed. / OK: 13 package(s) read outside themselves, all declared...
node scripts/check-ci-filter-parity.mjsOK: all 82 declared cross-package glob(s) (71 unique) are covered...
node scripts/check-cross-package-test-inputs.mjsOK: 13 package(s) read outside themselves, all declared...
node scripts/check-required-contexts.mjs6 required context name(s) pinned across 2 workflow(s)...
node scripts/check-shard-attestation.mjs2 aggregate gate(s) count 3 declared leg(s) across 3 attesting job(s).

Plus pnpm check:nul-bytes -- check-nul-bytes: OK (scanned 6300 text file(s) ... no raw ASCII control bytes). -- and a manual control-byte scan of both edited files (no matches).
eslint --no-inline-config on both changed files: exit 0, no findings.

Every exit code was captured before any pipe (cmd > log 2>&1; ec=$?), never through tail.
The #10853 filter trap was exercised as a negative control -- pnpm --filter @objectstack/definitely-not-a-package test printed No projects matched the filters and
exited 0 -- and it could not have greened anything here: the verification path above uses no
--filter at all.

No changeset

scripts/**-only, publishes nothing -- a pure bug fix (AGENTS.md:943). skip-changeset applied
additively via POST .../labels, then read back.

Generated by Claude Code


Generated by Claude Code

…run (#10889)
`invokesScript()` in check-shard-attestation added adjacency for #6589 but
tokenized with `command.split(/\s+/)` plus a per-token quote strip, so a quoted
string handed to `echo` was torn into words and the script name plus its flag
INSIDE that string read as a genuine invocation.
The reading is now the quote-aware one #10884 built for the sibling gate, moved
to the lexer whose output it consumes rather than mirrored: `commandWords` and
`invokes` live beside `shellCommands` in check-shard-attestation, and
check-required-contexts imports them over the one import edge that already
existed between the two files.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt
@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 21, 2026
@os-zhuang
os-zhuang marked this pull request as ready for review August 22, 2026 03:54
@os-zhuang
os-zhuang added this pull request to the merge queueAug 22, 2026
Merged via the queue into main with commit bd003d8Aug 22, 2026
33 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-10889-invokes-script-quote-aware branch August 22, 2026 04:09
@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 32550290075 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Console Pin Gate — 失败步骤: Build the Console SPA at the pinned objectui SHA

    ✗ Build failed in 6.05s
    

↳ 失败原因 是判读的关键:超时Test timed out in … / Hook timed out in …)多半是负载/时序,不是本 PR 的回归;
断言AssertionError: …)才指向真实的行为改变。两者的 FAIL 行长得一模一样,只有这一行能区分。

跨 PR 相同签名(24h,按失败测试文件聚合):

  • ⚠️本次没有可用的聚合签名(日志里没有能解析出测试文件名的 FAIL 行)—— 这不是「没有同签名的其他 PR」,是这一轮没测到。跨 PR 聚合本次不可用,请手工比对其他 PR 的同类评论。
  • ⚠️ 24h 评论账本没读完(超过 5 页仍未读到窗口尽头),所以上面的「不同 PR 数」是下界,不是全量。

历史信号:

  • 本 PR 过去 24h 无队列失败记录(首次)。
  • 过去 24h 队列共有 72 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

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

Labels

size/mskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[finding] check-shard-attestation's invokesScript() reads an echo QUOTING the invocation as an invocation — adjacency without quote-awareness

2 participants

@os-zhuang@claude