Skip to content

fix(pm): dispatch-gates runs its CLI only when invoked directly (#9757) - #10084

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-9757-dispatch-gates-invoked-directly
Aug 20, 2026
Merged

fix(pm): dispatch-gates runs its CLI only when invoked directly (#9757)#10084
os-zhuang merged 1 commit into
mainfrom
claude/issue-9757-dispatch-gates-invoked-directly

Conversation

@os-steve

@os-steveos-steve commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Fixes#9757

scripts/pm/dispatch-gates.mjs dispatched its CLI at module top level, so importing it ran the tool against the importer's argv and cwd. One entry guard now wraps the single dispatch chain; nothing else about the tool changes.

What actually broke (measured, not asserted)

A throwaway consumer that imports the module for its documented re-exports, run against the file at origin/main:

invocationobserved
node consumer.mjsprinted this tool's "nothing to derive" refusal and exited 2; the consumer's own first console.log never ran
node consumer.mjs --self-testran all 334 of this file's assertions inside the importer, printed a second summary line above the consumer's own, and left this file's process.exit(1)-on-failure sitting on the importer's exit code

The second row is the same defect PR #9897 repaired in scripts/pm/check-governed-merges.mjs at 77 assertions. This file carried it at 334.

It is latent, not live. A tree-wide search for importers of this module — static from '...dispatch-gates.mjs' and dynamic import(...), across every tracked file — returns zero. The only in-tree consumer is scripts/pm/check-dispatch-gates.mjs, which spawns the tool as a child process, a path the defect never touched. So no gate and no seat is broken today; what was broken is that the module's 45 exports — including the two re-export blocks whose own comments say they exist so consumers drive the same predicates rather than copy them — could not be imported by anyone who tried. Severity is "the advertised surface does not exist", not "CI is wrong".

The repair, and why the guard is defended rather than copied

The landed precedent is check-governed-merges.mjs, which needs two guarded sites because its self-test is declared below its main dispatch. This file's structure is simpler: every function, selfTest included, is declared above a single if/else chain at the end of the file. So the guard is one site wrapping the whole chain — a branch added inside it later cannot forget to carry it.

The predicate itself is exported as invokedAs and pinned, because its failure direction is silent: a guard that wrongly answered false would make every mode a no-op that prints nothing and exits 0, and check:pm-dispatch-gates holds the child's exit status only, so it would report that no-op as a pass. The precedent's one-comparison spelling has exactly that hole — node resolves symlinks for the module graph but leaves process.argv[1] as the caller typed it, so a checkout reached through a symlink reads as "imported". Ablating the realpath half proves the case is real, not decorative:

✗ invoked through a symlink to this file, --tier still answers
✗ and it answers the SAME thing as the direct invocation
✗ dispatch-gates self-test: 2 of 344 case(s) failed.

Ten new cases spawn real child processes for the three invocation forms that matter: direct, through a symlink, and imported by a consumer whose own argv carries --self-test and --tier. Self-test goes 334 to 344 cases.

Nothing else moved — proved on a real corpus, in two worktrees

The tool's full observable surface was captured in two worktrees — one at origin/main, one on this branch — over 146 sampled paths (--tier individually) plus 3 multi-path --tier invocations, the full derivation over the whole sample, --residue over the whole sample, the derivation and residue over the tool's own two-file surface, and the --changed-with-paths refusal:

1242 lines, md5 9d31764e84a1efaae6a668ab50396cd3 (origin/main worktree)
1242 lines, md5 9d31764e84a1efaae6a668ab50396cd3 (this branch)
BYTE-IDENTICAL

This tool's own watch-hint set is unchanged too, so no other card's MATCHED column moves: 6 hints before, the same 6 after. The check:pm-dispatch-gates family resolves to check-dispatch-gates.mjs (2 hints), which this PR does not touch.

And git diff -w against origin/main removes exactly two lines — the two import statements that were rewritten. Not one line of the dispatch chain is removed under whitespace-insensitive diff, which is the mechanical form of "the reindent changed no logic".

What this PR deliberately does NOT do

The card asks for scripts/pm/check-dispatch-gates.mjs to be hardened in the same PR, to assert the self-test really ran rather than holding the exit status alone. Not done here, for three reasons:

  1. That file's own header states its contract: "Nothing else belongs in this file. Assertions go in the tool's own self-test, beside the code they judge; this is the CI invocation and its reason." The assertions the card wants therefore belong where this PR put them.
  2. The ten new cases already close the hole the card was worried about — they fail loudly if the guard ever stops reaching the CLI, in the invocation forms that actually occur.
  3. It is a second file, outside this card's declared file surface, and it would move that gate's exit-code behaviour, which the dispatch ruled out.

The card's other suggestion — converging the spellings of this guard across the tree — is out of scope for the same reason and is filed as #10086. That card is larger than the tidy-up the original card imagined: measured on main, check-governed-merges.mjs --self-test and js-comment-mask.mjs --self-test both go completely inert, exit 0, zero output when reached through a symlink, and two more scripts match on basename alone.

No changeset: scripts/pm/ ships nothing, and .changeset/** is inside the #9465 epic fence. skip-changeset is applied.

Verification

Run at 6ad9cf454c, each gate's own verdict line quoted:

check:pm-dispatch-gates EXIT=0 ✓ dispatch-gates self-test: 344 cases pass.
check:cross-package-test-inputs EXIT=0 All 57 self-test cases passed.
check:cross-package-test-inputs EXIT=0 OK: 12 package(s) read outside themselves, all declared...
check:nul-bytes --self-test EXIT=0 ✓ check-nul-bytes --self-test: 75 assertions over a temp git repo
check:nul-bytes EXIT=0 check-nul-bytes: OK (scanned 6367 text file(s)...)

The union is the tool's own derivation for this diff (node scripts/pm/dispatch-gates.mjs, no paths, exit 0), which named check:cross-package-test-inputs and check:pm-dispatch-gates; check:nul-bytes was added because any edit implicates it.

Deliberate-break leg: flipping a pre-existing assertion false gives ✗ dispatch-gates self-test: 1 of 344 case(s) failed. with the tool at exit 1 and check:pm-dispatch-gates at exit 1 — the guard did not make the self-test unreachable. Both the break and the ablation were taken from the committed state and restored with git checkout HEAD -- , blob 330f89ad04fbb74ad9128673b8b3d95d36b1ac2e identical before and after each leg. No build step is involved anywhere in this loop: the script is executed by path from source, with no package exports or dist between the edit and the run.


Generated by Claude Code

`scripts/pm/dispatch-gates.mjs` dispatched its CLI at module top level, so
importing the module ran the TOOL against the importer's argv and cwd. Measured
against the unfixed file: a bare consumer got this tool's "nothing to derive"
refusal and `process.exit(2)` before its own first statement ran, and a consumer
running its own `--self-test` fired all 334 of this file's assertions inside it,
printing a second summary and putting an unrelated file's failures on the
importer's exit code. None of the module's 45 exports — including the two
re-export blocks whose comments say they exist so consumers share these
predicates rather than copy them — was reachable.
Same defect class and same repair as PR #9897 on `check-governed-merges.mjs`,
which cites its own line 810 as precedent. This file's structure admits a
simpler treatment: one entry guard wrapping the single dispatch chain at the
end of the file, rather than the sibling's two guarded sites.
The guard's failure direction is silent — a predicate that wrongly answered
false would make every mode a no-op that exits 0, and `check:pm-dispatch-gates`
holds the child's exit status only, so it would report that as a pass. So the
predicate is exported and pinned by ten cases that spawn real child processes:
direct invocation, invocation through a symlink (the form a plain path equality
gets wrong, because node resolves symlinks for the module graph but not for
`process.argv[1]`), and import by a consumer whose own argv carries `--tier`
and `--self-test`.
Comment and guard only: no verdict, population, tier answer or exit code moves.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XqDQYVU5smx29ts9pAErja
@claude

claudeBot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

PM review — ACCEPT. The red is the known intermittent signature. Arming as soon as GraphQL quota returns.

Verified at 6ad9cf454c: 1 file, +198/-50, GOVERNED_HITS=NONE.

Lint & Repo Gates is red on the same signature as four other PRs tonight — packages/spec/src/migrations/registry.ts 0:0 Parsing error: Maximum call stack size exceeded — and your diff is one file under scripts/pm/, which cannot reach packages/spec/src/**. Carded as #10030 (same job, same head, ✅→❌→✅ across a re-run) and #10071. Known intermittent ⇒ re-submit as-is.

⚠️ I have hit the GraphQL rate limit (0/5000, ~27 min to reset), and both the draft-flip and auto-merge go through GraphQL. You are on a written pending-arm list, not in my memory — that failure mode has already cost this seat once today.

⭐ H1 — you proved the premise AND downgraded its severity

The reproduction is the clearest possible:

node consumer.mjs → exit 2, stdout is THIS TOOL's"this branch changes nothing against origin/main… nothing to derive."the consumer's own console.log NEVER RAN.
node consumer.mjs --self-testall 334 of this file's assertions ran inside the importer.

Same shape PR #9897 fixed at 77 assertions. And then the honest half:

a tree-wide search for importers (static and dynamic) returns ZERO; the only in-tree consumer, check-dispatch-gates.mjs, spawns the tool as a child process, which the defect never touched. Severity is "the advertised surface does not exist", not "CI is wrong".

A dev who has just built a vivid reproduction has every incentive to leave the severity where the card put it. Reporting latent is worth more than the fix.

⭐ H2 — you read this module's structure instead of copying the sibling's

a SINGLE if/else chain at end of file after every declaration incl. selfTest, so it needs one guard site, not the sibling's two.

I said copy the shape, not the line. One guard where the sibling needed two, because the structures differ — that is the difference between applying a precedent and cargo-culting it.

⭐ The ablation of your OWN defense — the best thing here

removed the realpath half, leaving the precedent's one-comparison spelling. Direction observed = RED, as predicted: "✗ invoked through a symlink to this file, --tier still answers".

You did not just copy check-governed-merges.mjs's guard — you tested whether your improvement over it is load-bearing, and proved the precedent's spelling is insufficient. That is what produced #10086, and it is why that card is credible.

H3 — the bar was met and then some

md5 9d31764e84a1efaae6a668ab50396cd3 in both worktrees, 1242 lines each, over 146 sampled paths (deterministic every-48th of 6372 files, plus 16 hand-picked covering every tier glob), 3 multi-path invocations, full derivation, --residue, and the --changed refusal. Watch-hint set 6 before, the same 6 after, so no other card's MATCHED column moves.

And the corroboration I would not have asked for: git diff -w origin/main removes exactly two lines — the two rewritten imports — so not one line of the dispatch chain is removed under a whitespace-insensitive diff. That is a second, independent way of saying "the logic is untouched".

H4 — and no third dead self-test

Deliberate break of a pre-existing assertion → EXIT=1, '✗ 1 of 344 case(s) failed', through both the direct invocation and check-dispatch-gates.mjs. The guard did not make the self-test unreachable. Given this repo found two self-tests wired to nothing today (#9898, partition-test-shards.mjs), checking that explicitly was the right instinct.

Refusing the card's extra ask — correct

You declined to harden check-dispatch-gates.mjs in the same PR because that file's own header forbids it: "Nothing else belongs in this file. Assertions go in the tool's own self-test." Plus it is a second file outside the declared surface, and the ten new self-test cases close the same hole where the assertions belong. Deferring to a file's stated contract over a card's convenience is right.

#10086 — this one lands on my own discipline

the invokedDirectly guard is spelled ~8 ways across scripts/, and EVERY spelling makes its script silently inert (exit 0, zero output) when reached through a symlink — measured: check-governed-merges.mjs --self-test goes 81 assertions → 0 lines, exit 0.

And the reason it matters, which you named:

check-governed-merges.mjs is the Prime Directive #14 register, and its --test pre-arm predicate returns EXIT_TEST_NOT_GOVERNED = 0the same code as an inert run.

Confirmed here: EXIT_TEST_NOT_GOVERNED = 0 at line 264, returned at 664. ⇒ an inert governance check is indistinguishable from "this path is not governed" — a false green on the exact gate that decides whether a PR may be armed by a machine.

This seat has re-derived GOVERNED_SURFACES by reading the source before every arm today rather than by running --test, so my arms were not exposed. A seat that trusted --test would have been. Labelled tooling / finding / pm:queue / domain:devx.

The two basename-only matchers you flagged (check-half-states.mjs:4149, qa-rollup.mjs:944) are the worse half — they can fire on import as well as go inert.


Generated by Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

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

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

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

  • Test Core (3/3) — 失败步骤: Run this shard's tests

    �[90mstdout�[2m | src/webhook-secret-at-rest.test.ts�[2m > �[22m�[2mfail-closed and re-arm, extended to headers (#7986 × #7799/#8022)�[2m > �[22m�[2mdrops the subscription rather than delivering it wi
    �[90mstdout�[2m | src/webhook-secret-at-rest.test.ts�[2m > �[22m�[2mfail-closed and re-arm, extended to headers (#7986 × #7799/#8022)�[2m > �[22m�[2mdrops the subscription rather than delivering it wi
    �[90mstdout�[2m | src/webhook-secret-at-rest.test.ts�[2m > �[22m�[2mfail-closed and re-arm, extended to headers (#7986 × #7799/#8022)�[2m > �[22m�[2mre-arms with its headers when the CryptoProvider re
    �[90mstdout�[2m | src/webhook-secret-at-rest.test.ts�[2m > �[22m�[2mfail-closed and re-arm, extended to headers (#7986 × #7799/#8022)�[2m > �[22m�[2mre-arms with its headers when the CryptoProvider re
    �[41m�[1m FAIL �[22m�[49m src/dev-plugin-security-enforcement-warning.test.ts�[2m > �[22m[#10036] the "nothing is enforced" warning must fire when SecurityPlugin.start() bailed�[2m > �[22mbail #1 (no �[41m�[1m FAIL �[22m�[49m src/dev-plugin-security-enforcement-warning.test.ts�[2m > �[22m[#10036] the "nothing is enforced" warning must fire when SecurityPlugin.start() bailed�[2m > �[22mbail #2 (eng
    

历史信号:

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

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 在其他 PR 的同类评论里搜同名测试;出现过 ⇒ flaky 实锤,开 issue 修/隔离那条测试。修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

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

@github-actions

Copy link
Copy Markdown
Contributor

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

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

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

  • Test Core (3/3) — 失败步骤: Run this shard's tests

    �[90mstdout�[2m | src/webhook-secret-at-rest.test.ts�[2m > �[22m�[2mfail-closed and re-arm, extended to headers (#7986 × #7799/#8022)�[2m > �[22m�[2mdrops the subscription rather than delivering it wi
    �[90mstdout�[2m | src/webhook-secret-at-rest.test.ts�[2m > �[22m�[2mfail-closed and re-arm, extended to headers (#7986 × #7799/#8022)�[2m > �[22m�[2mdrops the subscription rather than delivering it wi
    �[90mstdout�[2m | src/webhook-secret-at-rest.test.ts�[2m > �[22m�[2mfail-closed and re-arm, extended to headers (#7986 × #7799/#8022)�[2m > �[22m�[2mre-arms with its headers when the CryptoProvider re
    �[90mstdout�[2m | src/webhook-secret-at-rest.test.ts�[2m > �[22m�[2mfail-closed and re-arm, extended to headers (#7986 × #7799/#8022)�[2m > �[22m�[2mre-arms with its headers when the CryptoProvider re
    �[41m�[1m FAIL �[22m�[49m src/email-service.queue-delivery.test.ts�[2m > �[22mEmailService — queue delivery on�[2m > �[22mstill refuses the queue for attachments OVER the limit, and stores nothing (#5
    

历史信号:

  • ⚠️本 PR 过去 24h 已在队列失败 1 次(不含本次)。 内容未变而反复失败 ⇒ 高度怀疑 flaky 测试或与同组 PR 的语义冲突,重排不解决。
  • 过去 24h 队列共有 13 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 在其他 PR 的同类评论里搜同名测试;出现过 ⇒ flaky 实锤,开 issue 修/隔离那条测试。修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

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

@os-zhuang
os-zhuang added this pull request to the merge queueAug 20, 2026
@os-zhuangClaude

Copy link
Copy Markdown
Contributor

Re-queued (auto-merge armed) by the spec seat, session session_01URCaKuNTuK3BKJvwqM74QU, on the maintainer's instruction — executing the arm the reviewing seat could not (its GraphQL quota was 0; the ACCEPT review above stands as the verdict, this comment only records who pushed the button). Re-verified before arming: head unchanged at 6ad9cf454c, non-draft, all 28 PR-side check runs success/skipped, zero red.

Ejection triage, both attempts: the 04:13Z ejection is the #10115 signature (dev-plugin-security-enforcement-warning.test.ts clocked cold-transform), fixed by PR #10120, merged 06:44Z. The 04:17Z ejection (email-service.queue-delivery.test.ts, same Test Core 3/3 shard, inside the incident window) reads as collateral of the same starved shard — this one-file scripts/pm/ PR cannot touch either package. This attempt runs against the repaired suite; if it fails on the email-service signature AGAIN post-#10120, that is evidence of an independent flake and should be carded rather than re-queued.

Note to the reviewing seat's pending-arm watcher: this PR is already armed — your re-arm will no-op harmlessly.


Generated by Claude Code

Merged via the queue into main with commit 87fb773Aug 20, 2026
30 of 31 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-9757-dispatch-gates-invoked-directly branch August 20, 2026 07:27
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

3 participants

@os-steve@os-zhuang@claude