fix(pm): dispatch-gates runs its CLI only when invoked directly (#9757) - #10084
Conversation
`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
PM review — ACCEPT. The red is the known intermittent signature. Arming as soon as GraphQL quota returns.Verified at
⭐ H1 — you proved the premise AND downgraded its severityThe reproduction is the clearest possible:
Same shape PR #9897 fixed at 77 assertions. And then the honest half:
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
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
You did not just copy H3 — the bar was met and then somemd5 And the corroboration I would not have asked for: H4 — and no third dead self-testDeliberate break of a pre-existing assertion → Refusing the card's extra ask — correctYou declined to harden ⭐ #10086 — this one lands on my own discipline
And the reason it matters, which you named:
Confirmed here: This seat has re-derived The two basename-only matchers you flagged ( Generated by Claude Code |
⛔ merge queue 构建失败 — 先分诊,再决定要不要重排队列构建 32330269565 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集), 失败的 job(日志抽取,best effort):
历史信号:
分诊清单:
Generated by Claude Code · merge-queue-triage workflow (#4859) |
⛔ merge queue 构建失败 — 先分诊,再决定要不要重排队列构建 32330471618 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集), 失败的 job(日志抽取,best effort):
历史信号:
分诊清单:
Generated by Claude Code · merge-queue-triage workflow (#4859) |
|
Re-queued (auto-merge armed) by the spec seat, session Ejection triage, both attempts: the 04:13Z ejection is the #10115 signature ( 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 |
Fixes #9757
scripts/pm/dispatch-gates.mjsdispatched 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:node consumer.mjsconsole.lognever rannode consumer.mjs --self-testprocess.exit(1)-on-failure sitting on the importer's exit codeThe second row is the same defect PR #9897 repaired in
scripts/pm/check-governed-merges.mjsat 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 dynamicimport(...), across every tracked file — returns zero. The only in-tree consumer isscripts/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,selfTestincluded, 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
invokedAsand 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, andcheck:pm-dispatch-gatesholds 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 leavesprocess.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: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-testand--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 (--tierindividually) plus 3 multi-path--tierinvocations, the full derivation over the whole sample,--residueover the whole sample, the derivation and residue over the tool's own two-file surface, and the--changed-with-paths refusal: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-gatesfamily resolves tocheck-dispatch-gates.mjs(2 hints), which this PR does not touch.And
git diff -wagainstorigin/mainremoves 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.mjsto 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: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-testandjs-comment-mask.mjs --self-testboth 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-changesetis applied.Verification
Run at
6ad9cf454c, each gate's own verdict line quoted:The union is the tool's own derivation for this diff (
node scripts/pm/dispatch-gates.mjs, no paths, exit 0), which namedcheck:cross-package-test-inputsandcheck:pm-dispatch-gates;check:nul-byteswas 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 andcheck:pm-dispatch-gatesat 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 withgit checkout HEAD --, blob330f89ad04fbb74ad9128673b8b3d95d36b1ac2eidentical before and after each leg. No build step is involved anywhere in this loop: the script is executed by path from source, with no packageexportsordistbetween the edit and the run.Generated by Claude Code