Skip to content

Commit a84f61a

Browse files
os-justinclaude
andauthored
fix(pm): a review of record on the card thread counts at the queue, and one constant holds both tools to it (#18738)
Fixes #18701 Clause-②: no ## The defect `check-clause2-carriers.mjs --template` prints, and has always printed, that a review of record is `ONE comment on the PR or its card`; C6/C7 judge it on both threads through `locateReviewOfRecord`; the governed text says the same in four places. The Governed Surface Queue Guard's REFERENCES tier read **the pull request's thread alone**. The two tools already shared every recogniser — the heading marker, the head-sha span test, the newest-of resolution, the `Reviewed-by:` key line, the `Served-tier:` reader — and the one thing they did not share was the **set of threads** to run them over, which is the only fact a spelled-out copy of the filter could not copy. Measured cost, on one pull request in one afternoon: the skills seat posted its `## Contract review` on carrier card #18426 (comment `5716694216`, 15:10:43Z), `--pair 18689` read it as C6-RECORD on the `card thread`, the `merge_group` leg (run `35238999808`) refused with `0 comment(s) read on the PR thread`, PR #18689 was dequeued `CI_FAILURE` at 15:36:18Z, and only a **second copy** of the same comment on the PR thread (`5717108355`) cured it. ## The canonical sentence — the governed text is not silent Read on `origin/main` (`080688b561`, the branch base). Quoted verbatim and untranslated, because rewriting a quoted ruling rewrites the ruling: - `.claude/skills/pm-dispatch/references/contract-review.md` **:27** — 「复核记录 = 一条评论落 PR 或卡,达档与默认档同形;散文、dev 自评、`os-dev-report` 恒不算。」 - same file **:40** — 「落地前检三条:① 席内条款②复核 PASS 在案,即 PR 或卡上同形的复核记录(档位按实测)。」 - same file **:59** — 「PASS、FAIL 与作废都落 PR 或卡;……」 - `.claude/skills/pm-dispatch/SKILL.md` 〈入队与落地〉 **:640** — 「交付后复核归派发席:达档席内审,未达档循保险丝起子代理;记录 = 同形评论落 PR 或卡。」 (The claim comment's aside that `references/contract-review.md` carries no such sentence is the one premise this card's dispatch got wrong; :27 is that sentence, and it is on `main`. Nothing else in the premise moved — the guard, the template, the docblocks and the #18689 evidence all read as the card describes them.) **Which tool disagrees with it: `scripts/pm/check-governed-queue-guard.mjs`.** So the GUARD moves, all of it. No governed file is touched by this PR. ## Why the guard and not the template — the four axes - **实际业务需求.** The record on the card is not a hypothesis: it is what a seat did, on 2026-09-17, following this repo's own `--template`. Both carriers are live on every pair — the clause-② declaration is hung on both, `--pair` judges both, the carrier label is stripped from both — and `check-clause2-carriers` has read both threads since #17302. The population that would be served by moving the template instead is empty: nobody is asking for the card to stop counting. - **项目长远合理性.** Contract-first. The governed text is the contract and the two tools are its consumers; a consumer reading a narrower set than the contract declares is the defect, and the repair belongs at the consumer, not at the contract. 基本裁决原则 puts 声明 above 实现 above 文档面 — moving the template would be narrowing the declaration to match an implementation gap, which is the one direction that rule forbids. - **防 AI 写代码犯错.** This is a declared-but-not-honoured capability in its purest form: an instruction tells an author where to put an artifact, and a gate silently does not look there. The structural repair is not "say the same thing in more places" — it is ONE exported constant that the instruction and the reader both derive from, plus a pin that reds if either is edited alone. Moving the template would leave three more hand-written location sentences (the guard's option-3 text and two docblocks) free to drift the same way one layer along. - **创业阶段不扩散需求.** Moving the guard is the SMALLER change and adds no capability: the helper already exists, the card number is read off the pull object the head read already fetches (zero extra requests for it), and at most one extra thread read is bought — only for a references-tier pull request with no authorized approval, a population refused outright today, so nothing that passes now pays anything. Moving the template would be a RETIREMENT of a carrier the protocol uses everywhere else, across governed files, which is a maintainer's ruling and not a dev PR. No axis conflicts with another. A fifth, decisive fact: the template side cannot move from here at all — its sentences live in `.claude/skills/pm-dispatch/**`, off this card's file surface and governed. ## The shared constant, and one derivation of "the record on this head" `check-clause2-carriers.mjs` — the file that owns the reader — now declares the thread set once: ```js export const REVIEW_OF_RECORD_THREADS = Object.freeze([ Object.freeze({ where: 'PR', rows: 'prComments', number: 'pr', words: 'the PR' }), Object.freeze({ where: 'card', rows: 'cardComments', number: 'card', words: 'its card' }), ]); export const REVIEW_OF_RECORD_LOCATION = REVIEW_OF_RECORD_THREADS.map((thread) => thread.words).join(' or '); ``` Three consumers, all derived from it and none spelling a set of its own: 1. `locateReviewOfRecord` builds the rows it searches, its gap sentences and its per-thread read counts from the list. `--template`'s and `--pair`'s output is **byte-identical** to `main`'s. 2. `contractReviewTemplateLines` builds its printed sentence from `REVIEW_OF_RECORD_LOCATION`, so an instruction can never offer a thread no reader searches. 3. `check-governed-queue-guard.mjs` fetches **one thread per entry in the loaded list** — and its whole record reading is now `locateReviewOfRecord`, imported through the existing lazy loader rather than re-spelled. The guard's own two-line filter is gone; what is left there is the tier gate on top of the located record (`Served-tier:` stands, and the #18060 model-identifier refusal), which is that leg's own fact. The card the record may live on is **derived, never configured**: `deliveredCardNumber(pull)` enumerates the numbers the body or branch could be naming and asks `deliveryEvidence` — the one relation `derivePairs`, H8 and H31 already pair a PR with its card by — so it can never accept a card that relation rejects. A pull request naming no card, or two cards at equal strength, searches NO card thread and the refusal says so: the refusing direction, the only one a governed reading may be wrong in. The card's number and the head sha come from ONE `GET /repos/{owner}/{repo}/pulls/{number}` — the request the head read already made — so the card thread costs no extra call for the number, and the guard keeps its zero-write, event-payload-only posture with no checkout. `REVIEW_OF_RECORD_LOCATION` is mirrored at the guard's module scope for the renderer only (`renderGuardVerdict` is pure and synchronous, and a module-scope import of the sibling is the measured exit-13 deadlock its own docblock records). The SET the leg acts on is never a mirror — it is the loaded list — and the mirror's words are pinned to the constant in the guard's battery. ## The pin `check-clause2-carriers.mjs` owns the reader, so the pin lives in its self-test — and it is a **measurement**, not two lists written side by side: - **STATED** — the locations read out of the text `contractReviewTemplateLines()` actually prints. - **READ** — obtained by **driving `check-governed-queue-guard.mjs`'s `runGuard`** once per entry in `REVIEW_OF_RECORD_THREADS`, with a record built from this file's own `contractReviewRecordLines()` sitting on that thread **alone**, and collecting the threads whose run answers `exitCode === EXIT_CLEAR` with `record.state === 'stands'`. The case asserts `stated === read === the constant's own set`, plus its own controls: the set has two locations (not vacuous); each run located the record on the thread it was posted to; the guard fetched exactly one thread per entry, no more and no fewer; and a run with the record on NEITHER thread is refused, so the pin can fail. `selfTest()` became `async` to take that lazy import — the dispatch uses `.then(...)`, never a top-level `await`. ## Counterfactual — the #18689 dequeue, offline One fixture, driven through `runGuard` with no network and no git: a references-tier pair whose record sits only on the card thread. The same script runs against either generation (it passes both the old `fetchPullHead` and the new `fetchPull`). | | exitCode | conclusion | `record.state` | `record.where` | |---|---|---|---|---| | **before** (`080688b561`) | `3` REFUSED | `refused` | `absent` | — | | **after** | `0` | `cleared` | `stands` | `card` | Before, rendered: `⛔ NO review of record on this head either (1 comment(s) read on the PR thread)` — the #18689 sentence, reproduced. After, rendered: `✅ review of record on this head: comment 5716694216 (2026-09-16T15:10:43Z) on the card thread is a ## Contract review comment naming ...` ## Ablation Two legs, each: on-disk proof of the mutation (`grep -c` of the removed text and of the injected text, plus the `git hash-object` blob before and after), then the self-tests, then restore, then a byte-identity check against the HEAD blob. Both scripts carry `trap restore EXIT INT TERM` with an absolute path. No build or `dist/` is involved — these two files are run directly by node and by `package.json`, so the on-disk proof is the whole preflight. **Leg A — the loop that unifies the thread set** (`for (const thread of recognisers.threads)` narrowed to the first entry). Blob `9a111f6c` → `b57227d0`; removed-text count 1 → 0, injected-text count 0 → 1. Cross-tool pin: **4 of 802 failed**. Guard battery: **25 of 253 failed** — the loop is load-bearing for the whole references leg, not only for the card. **Leg B — the line that supplies the card's number** (`card: card.card` → `card: null`), which reproduces the pre-#18701 thread set exactly. Blob `9a111f6c` → `9d935d92`; counts 1 → 0 and 0 → 1. This is the clean reading the card asks for: - **new pin RED**: `check-clause2-carriers` **3 of 802 failed**, all three the new battery — `stated=[PR,card] read=[PR]`, the card run locating nothing, and the fetch list `[[4101],[4101]]` instead of `[[4101,4102],[4101,4102]]`. - **new guard cases RED**: `check-governed-queue-guard` **5 of 253 failed**, all five in the `#18701` battery (record on the card alone, both threads fetched, the rendering naming the thread, newest-governs across threads, the branch-name fallback). - **old cases GREEN**: 799 of 802 and 248 of 253 — every pre-existing battery in both files, `#18020` included, passes under the ablation. Restore, both legs: worktree blob `9a111f6c…` equals the HEAD blob, `git diff HEAD` empty, `git status --porcelain` empty, and both self-tests back to exit 0. ## Self-tests | family | before | after | |---|---|---| | `node scripts/pm/check-governed-queue-guard.mjs --self-test` | 239 cases, 21 batteries declared | **253 cases**, 22 batteries — new: `⭐ #18701: the record lives on the PR or its card, and BOTH are read` (14, floor 14); `SELF_TEST_BATTERY_FLOOR` 20 → 21 | | `pnpm check:pm-clause2-carriers` | 786 cases, 28 batteries declared | **802 cases**, 29 batteries — new: `#18701: ONE thread set -- what the template STATES is what the queue guard READS` (16, floor 16); `SELF_TEST_BATTERY_FLOOR` 27 → 28 | Both exit 0. `--template`, `--help` and the no-event refusal path (exit 1, its own message) all still answer. ## Gates `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived from the worktree with no hand-fed path list (change set: the two scripts, 2 committed / 0 working tree / 0 untracked, three-dot against merge base `080688b56`). **36 derived, 36 run, every one exit 0**, reconciled with `--ran` after a fresh `git fetch origin main` — `✓ dispatch-gates --ran: 36 derived famil(ies) accounted for — 36 run, 0 NOT-MEASURED (a DERIVED zero — all 36 recorded an exit code and none of them is 3)`. ```text node scripts/check-adr-0087-registration.mjs --base origin/main :: exit 0 node scripts/check-adr-0087-registration.mjs --self-test :: exit 0 node scripts/check-changeset-no-major.mjs --base origin/main :: exit 0 node scripts/check-changeset-no-major.mjs --self-test :: exit 0 node scripts/check-ci-filter-parity.mjs :: exit 0 node scripts/check-closing-keyword-parity.mjs :: exit 0 node scripts/check-closing-keyword-parity.mjs --self-test :: exit 0 node scripts/check-comment-mask-corpus.mjs :: exit 0 node scripts/check-declaration-mirrors.mjs :: exit 0 node scripts/check-declaration-mirrors.mjs --self-test :: exit 0 node scripts/check-scripts-symbol-anchors.mjs :: exit 0 node scripts/check-scripts-symbol-anchors.mjs --self-test :: exit 0 node scripts/check-self-test-wired.mjs :: exit 0 node scripts/check-self-test-wired.mjs --self-test :: exit 0 node scripts/check-self-test-workflow-commands.mjs :: exit 0 node scripts/check-self-test-workflow-commands.mjs --self-test :: exit 0 node scripts/check-whole-set-label-write.mjs :: exit 0 node scripts/check-whole-set-label-write.mjs --self-test :: exit 0 node scripts/pm/bare-root-worklist.mjs --self-test :: exit 0 node scripts/pm/check-governed-queue-guard.mjs --self-test :: exit 0 pnpm check:agent-test-spelling :: exit 0 pnpm check:bash32-floor :: exit 0 pnpm check:changeset-gate-self-tests :: exit 0 pnpm check:cli-command-ids :: exit 0 pnpm check:closing-target-claim :: exit 0 pnpm check:cross-package-test-inputs :: exit 0 pnpm check:driver-memory-census :: exit 0 pnpm check:entry-guard :: exit 0 pnpm check:nul-bytes :: exit 0 pnpm check:parse-guard :: exit 0 pnpm check:pm-clause2-carriers :: exit 0 pnpm check:pm-dispatch-gates :: exit 0 pnpm check:pnpm-filter-targets :: exit 0 pnpm check:ratchet-remedy-authority :: exit 0 pnpm check:refd-timer-probe :: exit 0 pnpm check:watch-hint-literal :: exit 0 ``` Repo-wide `pnpm lint` (`eslint . --no-inline-config`, the whole tree, as PR #18654 did) :: **exit 0**, no output. Every exit code above was captured with redirect-then-`$?`, never across a pipe. `origin/main` moved to `62bce5c297` while this ran; its three commits touch `packages/**` and `.changeset/**` only — nothing under `scripts/**` and nothing this diff reads — so no merge was taken and the derivation above is unaffected. ## Scope Two files, `scripts/pm/check-governed-queue-guard.mjs` and `scripts/pm/check-clause2-carriers.mjs`. No governed file. `skip-changeset`: nothing published moves — `scripts/pm/**` is not in any package's `files[]`. ⚠️ **The card read this PR adds is built on `readCardComments`'s paged sibling, not on the un-paged path #18683 names.** The guard's own `makeCommentReader` is paginated (100 per page, 10 pages, and it THROWS rather than truncating), and it is what serves both threads here — `GET /repos/{owner}/{repo}/issues/{number}/comments` is the same route for a pull request and for a card. #18683 remains exactly as it was, on `check-clause2-carriers`' own reader, untouched by this PR. Not in scope, as dispatched: `check-clause2-carriers`' un-paged `readCardComments` (#18683 — read, not fixed); #18536's machine-side lane/tier re-keying; the REFERENCES-tier rule itself (which paths qualify), untouched. ## Acceptance notes - `deliveryEvidence`'s ranking had no exported ordering, so `DELIVERY_EVIDENCE_PRECEDENCE` is declared in `check-clause2-carriers.mjs` as a MIRROR and measured against that function pair by pair in the self-test rather than trusted. If `check-half-states.mjs` ever wants to own it, that is a one-line move; noted, not filed — no PR or person reaches for a ranking that today has exactly one caller. - The guard no longer loads `check-half-states.mjs` directly: every recogniser it reads now arrives through the file that composes them, and a rename upstream stops THAT module loading, which the leg already turns into a fail-closed refusal with the loader's own reason. `RECOGNISER_SOURCES` is down to one entry and the self-test pins that. Keeping the old direct load would have been a check on something this file no longer reads. --- _Generated by [Claude Code](https://claude.ai/code/session_01Gqi43smmqjJ5sUrhfoPeKu)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 7636cd9 commit a84f61a

2 files changed

Lines changed: 610 additions & 143 deletions

File tree

0 commit comments

Comments
 (0)