Skip to content

Commit 3753764

Browse files
hotlongclaude
andauthored
fix(pm): name the ratchet invocation CI runs in dispatch-gates' test-file entry (#8611)
* fix(pm): name the ratchet invocation CI runs in the dispatch-gates test-file entry The convention entry for a change that adds a test file emitted check:type-check-coverage with a rationale that describes the TEST_DEBT ratchet — a half only check:type-check-debt (--re-measure) runs. A dev seat that ran the named command in good faith reported green while the ratchet had never run locally; CI then found four new type errors. Both invocations are now emitted, each stating the question it answers, and the ratchet line carries its built-closure prerequisite inline with the exact build command lint.yml runs before that step — an unexplained throw on a cold worktree reads as "not applicable to me". Fixes #8545 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018WuTtyckQa1VcXwgd52JpN * docs(pm): correct check-dispatch-gates' header on comment masking, and fix the sweep count The header justified its unquoted repo paths with "watch-hint extraction ... does not skip comments". extractWatchHints has opened with maskComments since #8478, and that function's docblock names this file as the specimen it retires — so the file carrying the workaround was still explaining it as live behaviour. Measured with extractWatchHints, every repo path in the header rewritten into backticks: 1 hint today (the module-body constant, the same hint the file ships with) against 10 under the pre-masking extractor. Comment masking alone accounts for it. The incident is kept as history, the section now says the convention is no longer required, and the rule that survives is stated: a module-body literal is what masking cannot reach. The separate-gate-file argument gets today's numbers too — 4 hints, not 49, and three of them are directory bases wide enough to keep the decision standing on narrower grounds. Fixes #8604 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018WuTtyckQa1VcXwgd52JpN --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent ffb090e commit 3753764

2 files changed

Lines changed: 87 additions & 13 deletions

File tree

scripts/pm/check-dispatch-gates.mjs

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@
66
*
77
* node scripts/pm/check-dispatch-gates.mjs # runs the tool's --self-test
88
*
9-
* ⚠️ This header names repo paths UNQUOTED on purpose — see the last section.
10-
*
119
* ## Why the gate exists
1210
*
1311
* scripts/pm/dispatch-gates.mjs derives the "local gates for this card" line of
@@ -53,21 +51,48 @@
5351
* none"), so shipping it as the price of gating the tool would have taken more
5452
* from every dispatch prompt than the gate gives back.
5553
*
54+
* Those numbers are PRE-MASKING, and the decision they justify survives on
55+
* narrower grounds than they describe. maskSelfTests now blanks the fixture
56+
* half outright: measured on this tree, the tool's own source yields 4 hints,
57+
* not 49 — .github/workflows, which it really reads, and packages/plugins,
58+
* packages/drivers, packages/services, the bases its package resolver probes.
59+
* Those three are real reads and still cover three of the largest directories
60+
* in the tree, so a directly-wired gate would print MATCHED for every card
61+
* under them — a smaller fabrication than the fixture one, of the same kind.
62+
* The spec filter path from the incident above no longer matches at all.
63+
*
5664
* A separate gate file is also what the other two pm gates look like
5765
* (check-skill-line-ratchet.mjs, check-skill-id-lint.mjs). Its watch hints are
5866
* the one constant below: this gate is matched for a card that edits the tool,
5967
* and for nothing else — which is the blind spot #8162 is about.
6068
*
61-
* ## Why the paths above are unquoted
69+
* ## Why the paths above are unquoted, and why that is no longer required
6270
*
63-
* Watch-hint extraction reads any quoted-looking span, backticks included, and
64-
* does not skip comments. Written the ordinary way, with each path in backticks,
65-
* this header alone yielded ten hints — packages/spec/src, packages/objectql,
71+
* The incident is real and worth keeping. Watch-hint extraction reads any
72+
* quoted-looking span, backticks included, and it USED TO read comments as
73+
* well. Written the ordinary way, with each path in backticks, this header
74+
* alone yielded ten hints — packages/spec/src, packages/objectql,
6675
* packages/plugins, packages/drivers, .claude/agents, .changeset among them —
6776
* and reproduced, from the file explaining the pollution, the exact false
6877
* MATCHED leads it exists to avoid (measured, not predicted: the first draft of
69-
* this file did it). So paths are named unquoted here, and the only quoted path
70-
* in this file is the one input this gate genuinely has.
78+
* this file did it). Hence the convention.
79+
*
80+
* The extractor no longer works that way: extractWatchHints opens with
81+
* maskComments, whose own docblock names this file as the specimen it retires.
82+
* Measured on this tree, with every repo path this header names rewritten into
83+
* backticks: 1 hint under today's extractor — scripts/pm/dispatch-gates.mjs,
84+
* the same single hint the file ships with — against 10 under the pre-masking
85+
* one, reaching spec, objectql, plugins, drivers, .claude/agents and .changeset
86+
* exactly as the incident describes. Comment masking alone accounts for the
87+
* difference: masking self-test bodies instead changes nothing here, because
88+
* this file has none.
89+
*
90+
* So the unquoting is no longer load-bearing, and this section is history
91+
* rather than an instruction: quoting a path in a comment here is now free, and
92+
* the paths stay unquoted because rewriting them buys nothing. What is NOT free
93+
* is a path literal in a module body — masking cannot reach one — so the one
94+
* quoted path below is still the one input this gate genuinely has, and that is
95+
* the rule to carry into a new gate's header rather than the unquoting.
7196
*
7297
* Nothing else belongs in this file. Assertions go in the tool's own self-test,
7398
* beside the code they judge; this is the CI invocation and its reason.

scripts/pm/dispatch-gates.mjs

Lines changed: 54 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -719,6 +719,31 @@ export function i18nBundlePackageDirs() {
719719
* CI does not silently stop being suggested — the run prints it as STALE and
720720
* says to fix this table. A hand-written list that reports its own rot is a
721721
* different object from one that quietly ages.
722+
* - Every `name` here is an INVOCATION, not a script. One check script can be
723+
* wired into CI under two package scripts that answer different questions, and
724+
* a rationale that names the script instead of the invocation sends a seat to
725+
* a command which cannot reproduce the failure it describes.
726+
* `check:type-check-coverage` and `check:type-check-debt` are one file
727+
* (`scripts/check-type-check-coverage.mjs`); only the second passes
728+
* `--re-measure`, which is the half a new test file's type errors move. This
729+
* entry named the first while explaining the second, so a dev seat ran it in
730+
* good faith, reported the union green, and CI found four new type errors.
731+
* Swept over this tree when that was fixed: the workflows discover 96
732+
* families resolving to 73 distinct script files, and 8 of those files are
733+
* reached by more than one family — 7 of the 8 in the other shape, a `check:`
734+
* script beside a direct `node scripts/check-x.mjs` step in a second
735+
* workflow, which `derive` discovers as its own family and prints with its
736+
* own runnable invocation. The pair below is the only one where two ROOT
737+
* SCRIPTS differ by a flag, so this is a one-off today and what generalises
738+
* is the rule, not the fix.
739+
* - Prose in a `why` is a MODULE-BODY string, so it is scanned for watch hints
740+
* like any other literal — comment masking cannot reach it. The ratchet
741+
* entry's remedy command therefore spells its `--filter` values unquoted (and
742+
* says to quote them for the shell): measured, the shell-quoted spelling adds
743+
* both of its glob filter values to THIS file's own hint set as hints, inert
744+
* only because `hintCovers` rejects one that collapses to a bare top-level
745+
* directory. A gate list that fabricates hints out of its own explanations is
746+
* the failure this whole script is written against.
722747
* - Each entry is deletable, with a stated criterion:
723748
* - test-file entry: when a gate on it grows a discoverable path literal,
724749
* the ordinary derivation names it and its line becomes redundant.
@@ -741,7 +766,11 @@ export const CHANGE_KIND_GATES = [
741766
},
742767
{
743768
name: 'check:type-check-coverage',
744-
why: "TEST_DEBT ratchets a package's test-layer type errors, so a new test file that does not typecheck cleanly moves it",
769+
why: "the STRUCTURAL half: a package whose test files sit outside every tsc program accounting for it must carry a TEST_DEBT entry, so a new test file no tsconfig reaches moves this one. It re-measures no count — the ratchet is the invocation below",
770+
},
771+
{
772+
name: 'check:type-check-debt',
773+
why: "the RATCHET half, and the invocation CI runs for it: `--re-measure` re-runs tsc per ledger entry and fails when a count drifts up, so a new test file that does not typecheck cleanly moves it. Needs the workspace closure BUILT — on an unbuilt worktree it refuses outright, and that throw means NOT MEASURED, never `not applicable to me`. Build first, exactly as lint.yml does: pnpm exec turbo run build --filter=./packages/* --filter=./packages/*/* (quote the filter values for your shell)",
745774
},
746775
],
747776
},
@@ -1077,8 +1106,8 @@ function selfTest() {
10771106

10781107
const resolved = (name) => `pnpm ${name}`;
10791108
const kindHit = changeKindLines(['packages/objectql/src/engine.test.ts'], resolved);
1080-
t('a test path emits the convention section', kindHit.length === 3 && kindHit[0].includes('adds or edits a test file'));
1081-
// Both halves anchor on the rendered DELIMITERS (`- pnpm x —`), for the
1109+
t('a test path emits the convention section', kindHit.length === 4 && kindHit[0].includes('adds or edits a test file'));
1110+
// All three halves anchor on the rendered DELIMITERS (`- pnpm x —`), for the
10821111
// reason the i18n entry's pins below state at length: a bare `includes` is
10831112
// satisfied by every name that merely STARTS WITH the expected one, so a
10841113
// prefix-preserving rename is invisible to it — the single rot class the STALE
@@ -1087,7 +1116,20 @@ function selfTest() {
10871116
// `check:type-check-coverage-v2` in CHANGE_KIND_GATES left the substring form
10881117
// green at 61/61 while the live run printed both as STALE; anchored, the same
10891118
// rename fails this case. The two conventions in this file now agree.
1090-
t('the section names both convention gates, runnably', kindHit.some((l) => l.includes('- pnpm check:query-options-erasure —')) && kindHit.some((l) => l.includes('- pnpm check:type-check-coverage —')));
1119+
//
1120+
// The coverage/debt PAIR is pinned as a pair on purpose (#8545): they are two
1121+
// invocations of one script, and the anchored form is what tells them apart —
1122+
// `includes('pnpm check:type-check-coverage')` is satisfied by the debt line's
1123+
// absence AND by a `-v2` rename, which is how a rationale describing the
1124+
// ratchet went on naming the invocation that never runs it.
1125+
t('the section names all three convention gates, runnably', kindHit.some((l) => l.includes('- pnpm check:query-options-erasure —')) && kindHit.some((l) => l.includes('- pnpm check:type-check-coverage —')) && kindHit.some((l) => l.includes('- pnpm check:type-check-debt —')));
1126+
// The ratchet line's prerequisite is part of the product, not decoration: a
1127+
// seat that runs `--re-measure` on an unbuilt worktree gets a throw, and an
1128+
// unexplained throw reads as "not applicable to me" — which is a green report
1129+
// over a gate that never ran. So the printed line must carry both the
1130+
// condition and a command that satisfies it.
1131+
const debtLine = kindHit.find((l) => l.includes('- pnpm check:type-check-debt —')) ?? '';
1132+
t('the ratchet line states its built-closure prerequisite', /closure BUILT|BUILT closure/.test(debtLine) && debtLine.includes('turbo run build'));
10911133
t('a non-test path emits nothing', changeKindLines(['scripts/pm/dispatch-gates.mjs'], resolved).length === 0);
10921134

10931135
// i18n change-kind derivation — the pure judgments first, each mirroring one
@@ -1287,7 +1329,14 @@ function selfTest() {
12871329
// The table's own rot detector: a name no live run discovers must say so,
12881330
// never disappear quietly.
12891331
const stale = changeKindLines(['a.test.ts'], () => null);
1290-
t('an undiscoverable gate renders as STALE', stale.filter((l) => l.includes('STALE')).length === 2);
1332+
t('an undiscoverable gate renders as STALE', stale.filter((l) => l.includes('STALE')).length === 3);
1333+
// Per NAME, anchored on both sides of the rendered name (`⚠ x: STALE`), so the
1334+
// pair that shares one script is reported apart: a count alone stays green if
1335+
// one of the two is dropped from the table and something else is added, and a
1336+
// leading substring stays green through a `-v2` rename — the two ways this
1337+
// table has actually rotted.
1338+
t('the coverage half renders STALE under its own name', stale.some((l) => l.includes('⚠ check:type-check-coverage: STALE')));
1339+
t('the ratchet half renders STALE under its own name', stale.some((l) => l.includes('⚠ check:type-check-debt: STALE')));
12911340
const i18nStale = changeKindLines(['packages/services/service-messaging/scripts/i18n-extract.config.ts'], () => null);
12921341
t('an undiscoverable check:i18n renders as STALE', i18nStale.filter((l) => l.includes('⚠ check:i18n: STALE')).length === 1);
12931342
t('every declared convention gate carries a reason', CHANGE_KIND_GATES.every((k) => k.gates.every((g) => g.name && g.why)));

0 commit comments

Comments
 (0)