Skip to content

Commit 30288da

Browse files
os-teslaclaude
andauthored
fix(pm): the merge-queue guard refuses an oversized PR and the post-merge sweep lists one — the SIZE limb reaches the queue (#19175)
Fixes #19036 Clause-②: no ## What this PR does The maintainer's 2026-09-18 ruling — 「修改代码量超过某个行数(比如5000)就应该人工审核」 — landed seat-side in PR #19033 (`HUMAN_MERGE_LINE_THRESHOLD = 5000`, `testVerdict(paths, { size })`, `sizeVerdict`, `landsByHumanMerge` in `scripts/pm/check-governed-merges.mjs`). The merge-queue guard's `merge_group` leg kept handing the predicate no size, so a PR over 5,000 changed lines that a seat enqueued still merged — exactly what PR #18971 did (+238,310 / −119, nothing governed). This PR carries the limb to the two places the card names. **Queue guard (`scripts/pm/check-governed-queue-guard.mjs`)** — a fourth leg, `runSizeGuard` / `sizeGuardVerdict` / `renderSizeVerdict`: - reads every queued pull request's `additions` / `deletions` off the same pull object the head read uses — `makePullReader` grew a `size` field (through the sibling's own `pullSizeFrom`), no second endpoint, no new workflow scope (`pull-requests: read` already covers the pull object; the workflow file is untouched and the self-test pins that no `issues:` scope appeared); - judges it through the sibling's IMPORTED predicate: `testVerdict([], { size })` is the size limb alone (the path limb is the governed leg's question, already answered on the lifted rows), and `landsByHumanMerge` reads it — this file declares no threshold and spells no comparison, pinned against its own source; - REFUSES on a new exit code `EXIT_REFUSED_OVERSIZED = 8`, printing the two numbers, their sum, the threshold, the limb (SIZE), the source (`GET /repos/{o}/{r}/pulls/{n}`) and the one remedy — a human merge (the ruling quoted untranslated; it never advises making the diff smaller); - an unreadable size (the pull read throws, or the object carries no pair) is `EXIT_REFUSED_SIZE_UNREADABLE = 9`, fail-CLOSED — this read DECIDES, unlike the PR head, which has decided nothing since 2026-09-04; - the `pull_request` leg is silent and read-free (renders `''`), so that leg's output stays byte-identical; - the three-leg exit precedence is a pure function, `groupExitCode`: governed, then size, then carrier — every block is always printed, one code exits. **Post-merge sweep (`check-governed-merges.mjs` default mode)**: - `classifyCommit` is now `landsByHumanMerge(testVerdict(paths, { size }))`, so a landing is an entry when EITHER limb fired — a governed merge as before, and an oversized landing with no governed path at all; - the size is read LOCALLY off the landed diff: `commitChanges` runs one `git diff-tree --numstat --no-renames -m --first-parent` per mainline commit (replacing `commitPaths`' `--name-only`; the self-test pins the path list byte-identical against `--name-only` on a real fixture, a merge commit read against its first parent, a binary row at zero); - `renderReport` counts such rows apart (`N governed merge(s) and M oversized landing(s) with no governed path`), prints a `⛔ SIZE:` row with the numbers and the threshold, keeps the same attribution column, and prints GitHub's own pair beside the landed number only when the two differ (it rides the attribution GET the row already pays for; it never decides the listing); - `--json` entries carry `size` and `humanMerge`. ## The PM's mechanism assumptions, measured - 「the sweep already reads each merged PR (it prints `merged_by`)」 — **falsified.** The attribution loop reads `GET /pulls/{n}` only for rows `classifyCommit` already produced (governed merges); an ungoverned landing is never read at all, so no API read could have made it an entry. Route taken instead: the local `--numstat` reading above, the same source `--branch` already uses in this file, zero API, and it also lets the sweep classify before any attribution is spent. - 「the size reading cannot ride the existing head read for an oversized PR with no governed path; one `GET /pulls/N` per PR in the group」 — **held.** The size leg reads every queued PR through the same reader; the call count is pinned (`apiCalls === carrierPullsInGroup(rows).length`, deduplicated, group order). - 「fail-closed on an unreadable size」 — **taken**, on a code of its own (9) rather than the governed leg's 4, for the same reason 6/7 are split from 3/4: the legs' refusals must stay separable in a log. - 「pick the exit code already highest in the table」 — **taken**: a governed-unsatisfied AND oversized group prints both limbs and exits 3. Consequence worth stating: a merge group naming no pull request now exits on the size leg's 9 rather than the carrier's 7 (both blocks still print their own refusal); pinned. - **Boundary not decided here:** no authorized APPROVED review and no review of record lifts the size limb — the landed predicate says a human MERGE, and nothing has ruled the number the way 2026-08-27 ruled paths. The rendering says so; widening it is a one-line maintainer decision in the sibling. Listed under open questions in the report, not implemented. ## Acceptance (the seat's checklist) - oversized PR, NO governed path, in a merge group → refused, size limb named: the #18971 replay pin (`⭐ #18971-replay-an-OVERSIZED-PR-with-NO-governed-path-is-REFUSED-at-the-queue-on-the-size-code`) — governed leg clear at zero reads, size leg exit 8, `groupExitCode` 8; the rendered text names `#18971`, `238429 changed line(s) (+238310 / -119)`, `EXCEEDS the human-merge line 5000`, `HUMAN MERGE`; - governed path AND oversized → both limbs rendered, one exit (3), pinned; - exactly at the threshold → clear (guard: `⭐ exactly-the-threshold-is-WITHIN-the-comparison-is-strictly-greater`; sweep: `⭐ exactly-the-threshold-is-NOT-listed-the-comparison-is-strictly-greater`, and a real fixture commit of exactly 5,000 lines is not listed); - `pull_request` leg byte-identical: `renderSizeVerdict` returns `''` there and a throwing spy proves zero reads; - unreadable size → exit 9 (throw, missing pair, no recorded reading, group naming no PR), never a pass; - sweep lists an oversized merged PR, not an at-threshold one: unit pins plus a REAL CLI sweep over a fixture repo (`PR #5001` listed with `⛔ SIZE`, `PR #5000` not), on stdout and in `--json`; - self-tests green with counts up: guard 261 → 296, governed-merges 410 → 435; battery rosters +1 each, floors 21 → 22 and 29 → 30; - workflow file untouched. ## Verification Self-tests (worktree at `5c7caff` — the branch merged with `origin/main` `c229223`, which touched neither file; after `pnpm install`): - `node scripts/pm/check-governed-queue-guard.mjs --self-test` → exit 0, `296 cases pass` (was `261 cases pass` at `e8667ee`). - `node scripts/pm/check-governed-merges.mjs --self-test` → exit 0, `435 assertions` (was `410 assertions`). Ablation — the comparison inverted in the SIBLING, watched from both files (proves the guard imports the predicate rather than restating it), through `scripts/ablation-replace.mjs` in WRAP mode against the committed head `5c7caff` (the final of three runs; the first two are recorded below because each taught something): - mutation: anchor `exceeds: changedLines > HUMAN_MERGE_LINE_THRESHOLD,` × 1 → the same line with the greater-than sign replaced by a less-than sign, × 1; blob `6e1112bcd55e` → `c7a42160aabd`; on-disk counts read back inside the mutated window: replacement 1, original 0. - guard self-test under mutation: exit 1, `14 of 296 case(s) failed`, all in the #19036 battery (`threshold-plus-one-changed-line-is-OVERSIZED`, `one-under-is-WITHIN`, `an-OVERSIZED-queued-PR-REFUSES-with-code-8`, `a-within-sibling-does-NOT-carry-an-oversized-PR-through-the-group`, `#18971-replay-an-OVERSIZED-PR-with-NO-governed-path-is-REFUSED-…`, `governed-AND-oversized-prints-BOTH-limbs-…`, `an-authorized-APPROVAL-…-lifts-NOTHING-from-the-size-…`, `a-certified-pure-regeneration-lifts-…-NOTHING-from-the-size-at-the-queue-either`, …). - governed-merges self-test under mutation: exit 1, `30 failure(s)`, 0 TypeErrors — the pre-existing SIZE battery (`5001-changed-lines-is-OVER-it`, `the-PR-that-prompted-the-ruling-reads-238429-…`, `--test-with-5001-changed-lines-…-exits-3`, …), the new #19036 battery (`an-oversized-landing-with-NO-governed-path-is-a-sweep-ENTRY-…`, `the-head-counts-the-oversized-landing-APART-…`, `a-REAL-sweep-LISTS-the-over-by-one-landing-…`, …) and two #13307 live-mirror sweep pins that now see a phantom oversized row (every small landing reads over an inverted line — the expected direction). - restore: blob after restore `6e1112bcd55e…` == `HEAD` blob, `git diff HEAD` empty, `git status --porcelain` empty, anchor count back to 1. - Run 1 (on `3c7f5ec`): guard 14/296 red as above; the merges leg exited 1 by a **TypeError** in my new battery (a pin dereferenced the fixture that classifies to null under the mutation), so its failures were not named — fixed in `28faa51` (null-guarded pins; a red case must be a named one). Run 2: run against that fix while it was still UNCOMMITTED; the tool restores to `HEAD` by design, so the fix was discarded by the restore — caught by the pre/post blob compare (`6e1112…` before, `859965…` after), re-applied, committed, and run 3 is the record above. The "commit the fix first" rule, measured on the fix to the pins. Gates (derived in the worktree with `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands` — 33 commands, identical to the dispatch's list; reconciled with `--ran`; exit codes captured after redirection, never through a pipe; run at `5c7caff`, the final head, after the ablation's restore was proven): | # | command | exit | verdict line (from the gate's own output) | wall | |---|---|---|---|---| | 1 | `node scripts/check-ci-filter-parity.mjs` | 0 | OK: all 184 declared cross-package glob(s) (131 unique) are covered by `core` or `crosspkg`, every `crosspkg` entry still covers one, and the `test` job's `if:` | 0s | | 2 | `node scripts/check-closing-keyword-parity.mjs` | 0 | check-closing-keyword-parity: OK (3 parsers agree on all 9 keywords and both measured separators; sweep found 5 file(s) carrying the grammar across 8993 tracked | 1s | | 3 | `node scripts/check-closing-keyword-parity.mjs --self-test` | 0 | ✓ check-closing-keyword-parity --self-test: 40 assertions, 5 mutations of the shipped parsers each driven to red. | 3s | | 4 | `node scripts/check-comment-mask-corpus.mjs` | 0 | ✓ comment-mask corpus sweep [scripts/js-comment-mask.mjs]: 6896 files, 0 disagree, 0 unparseable, 81.9s (comparator self-test: 26 cases pass). | 83s | | 5 | `node scripts/check-declaration-mirrors.mjs` | 0 | OK: 10 hand-written declaration(s) agree with their modules on name, kind and required arity. | 0s | | 6 | `node scripts/check-declaration-mirrors.mjs --self-test` | 0 | All 29 self-test cases passed. | 0s | | 7 | `node scripts/check-scripts-symbol-anchors.mjs` | 0 | ✅ check-scripts-symbol-anchors: 3524 anchors across 265 scripts resolve — 52 symbol (52 declaration, 0 literal), 3472 file-level, 0 cross-repo, 1 exempt, 2 cont | 4s | | 8 | `node scripts/check-scripts-symbol-anchors.mjs --self-test` | 0 | ✅ check-scripts-symbol-anchors --self-test: every finding class provoked, comment-prose projection wired, declined shapes counted not missed, allowance rows exa | 4s | | 9 | `node scripts/check-self-test-wired.mjs` | 0 | ✓ check-self-test-wired: every one of the 214 script(s) CI runs that ship a `--self-test` has that self-test run by CI. | 2s | | 10 | `node scripts/check-self-test-wired.mjs --self-test` | 0 | check-self-test-wired --self-test: 3 live ledger row(s) verified, plus the comment mask, the right boundary, alias resolution and both audit directions — 10 dec | 2s | | 11 | `node scripts/check-self-test-workflow-commands.mjs` | 0 | ✓ check-self-test-workflow-commands: no self-test CI runs prints a line the Actions runner would parse as a workflow command. | 51s | | 12 | `node scripts/check-self-test-workflow-commands.mjs --self-test` | 0 | check-self-test-workflow-commands --self-test: both measured parse rules pinned (legacy form anywhere in a line, current form only at line start), the innocent- | 3s | | 13 | `node scripts/check-skills-token-ratchet.mjs` | 0 | ✓ check-skills-token-ratchet: 34 authored bundle file(s) within their ceilings; 10 generator-owned file(s) measured, not ratcheted. | 0s | | 14 | `node scripts/check-skills-token-ratchet.mjs --self-test` | 0 | ✓ check-skills-token-ratchet self-test: 65 cases pass. | 1s | | 15 | `node scripts/check-whole-set-label-write.mjs` | 0 | ✓ check-whole-set-label-write: 0 violations — 328 file(s) over 3 root(s) · 12 raw mention(s) · 12 in comments/prose (cleared) · 0 in EXECUTABLE content (judged) · 191 `uses:` pin(s) over 18 di | 2s | | 16 | `node scripts/check-whole-set-label-write.mjs --self-test` | 0 | ✓ check-whole-set-label-write --self-test: all cases pass (24 fixture trees + 5 refusals + 1 allowlist hatch) | 0s | | 17 | `node scripts/pm/bare-root-worklist.mjs --self-test` | 0 | OK self-test: 81 live row(s), 59 unreachable as spelled, 46 recorded verdict(s) — none stale, none missing, none contradicted (12 row(s) whose gate carries the | 29s | | 18 | `node scripts/pm/check-governed-queue-guard.mjs --self-test` | 0 | ✓ check-governed-queue-guard self-test: 296 cases pass (register-driven verdicts, the queue/PR event split, latest-decisive approval reduction, the 2026-09-04 a | 0s | | 19 | `pnpm check:agent-test-spelling` | 0 | ✓ check-agent-test-spelling: 0 violations — 518 file(s) · 9204 bare `--` token(s) · 1749 launcher-rooted run(s) · 13 separator(s) JUDGED · 6 vitest-backed scrip | 4s | | 20 | `pnpm check:bash32-floor` | 0 | ✓ check-bash32-floor: 31 tracked shell file(s) under scripts/**, .claude/hooks/**, .githooks/** name no bash 4+ construct outside a comment, a guarded ${VAR:-} | 2s | | 21 | `pnpm check:cli-command-ids` | 0 | ✓ check-cli-command-ids: 63 module(s) under packages/cli/src/commands examined, all of them default-export a class whose inheritance chain reaches oclif's `Comm | 11s | | 22 | `pnpm check:closing-target-claim` | 0 | ✓ check-closing-target-claim self-test: 105 cases pass. | 1s | | 23 | `pnpm check:cross-package-test-inputs` | 0 | OK: 29 package(s) read outside themselves, all declared, and turbo.json hashes every declared glob (6 of them on a split "test:repo" task); 13 walked root(s) ju | 20s | | 24 | `pnpm check:driver-memory-census` | 0 | check-driver-memory-census: OK — every declaration is ledgered, every ledger entry is live, and every ruled file states "#6664 census: 2 ruled consumers". This | 4s | | 25 | `pnpm check:entry-guard` | 0 | ✓ check:entry-guard: 265 scripts/ file(s) — every entry guard goes through invoked-as.mjs; 206 export bindings, 206 of them inert on import (0 known-unsafe, ⛔ S | 47s | | 26 | `pnpm check:nul-bytes` | 0 | check-nul-bytes: OK (scanned 8986 text file(s) -- 8986 tracked, 0 untracked-not-ignored; skipped 7 binary; no raw ASCII control bytes). | 3s | | 27 | `pnpm check:parse-guard` | 0 | ✓ check:parse-guard: 264 scripts/ file(s) — every TypeScript parse goes through ts-parse.mjs. | 2s | | 28 | `pnpm check:pm-governed-merges` | 0 | ✓ check-governed-merges --self-test: 435 assertions (the unified governed predicate + near misses, subject→PR spellings, window parsing, the #12633 landing wind | 6s | | 29 | `pnpm check:pnpm-filter-targets` | 0 | ✓ check:pnpm-filter-targets: 151/199 `--filter` occurrence(s) across 40 file(s) resolve against 81 workspace package(s); 48 not judged (2 foreign, 26 interpolat | 3s | | 30 | `pnpm check:ratchet-remedy-authority` | 0 | OK check-ratchet-remedy-authority: 259 scripts swept (scripts/*.{mjs,mts} + scripts/pm/*.{mjs,mts}); 15 mark the expanding remedy ⛔ MAINTAINER-ONLY, 5 turn it | 4s | | 31 | `pnpm check:refd-timer-probe` | 0 | OK check-refd-timer-probe: 6891 source file(s) swept; the process-global timer probe is read in packages/qa/refd-timer-testkit/src/index.ts and nowhere else. | 12s | | 32 | `pnpm check:watch-hint-literal` | 0 | ✓ check-watch-hint-literal: 71 declaration(s) across 4 rostered name(s) -- ROOT_DIR_WATCH_HINTS 47, ROOT_FILE_WATCH_HINTS 13, ROOT_WATCH_HINTS 3, DECLARED_WATCH | 5s | 32 command(s); 0 non-zero exit(s). Reconciliation (`node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --ran` over a record whose every line carries `:: exit N`): `33 derived, 32 run, 0 NOT-MEASURED, 1 UNRUN` — the one unrun family is `pnpm check:pm-dispatch-gates`, which is **NOT MEASURED at PR-open time**: it runs only under the shared verify lock (≈1,000 s under contention); a first run started on `3c7f5ec` overlapped the ablation window (`dispatch-gates.mjs` imports `sizeVerdict` from the sibling, so a child it spawned in that window could have read the inverted comparison), and a second run on `5c7caff` is queued behind it. Its verdict lands in the `os-dev-report` comment on #19036, not here — this body is written once. Lint, narrowed and measured: `npx eslint --no-inline-config --format json` on the two changed files → exit 0, 2 files, 0 errors, 0 warnings. Narrowing evidence: ① eslint's configured population (`eslint.config.mjs`, the `**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}` block) includes `scripts/pm/*.mjs`, and `--print-config` resolves a config for the file (parser `typescript-eslint/parser`); ② the JSON output counts 2 files; ③ type-aware linting is not enabled (`parserOptions.project` and `projectService` both absent in the resolved config), so this diff cannot move any untouched file's verdict. The repo-level `pnpm lint` sweep is CI's run. ## Acceptance notes (noted, not filed) - The `pull_request` event payload carries `additions` / `deletions`, so a size early warning on the PR leg would cost zero reads. Not taken: the ruling is about the landing, the seat-side pre-check already refuses before arming, and the PR leg's byte-identity is a standing constraint. 承接者: the skills seat, if the maintainer wants the forecast. - The carrier leg and the size leg each read `GET /pulls/{n}` once per queued PR — the same endpoint twice. A shared per-run pull read would halve it; kept separate so each leg's refusal and count stay separable in a log. Groups are small. 承接者: none. - No governed reference text enumerates the guard's exit table, so codes 8/9 leave no Tier S doc stale (`grep` over `.claude/skills/pm-dispatch/references/**` and `SKILL.md` for `exit 6`/`exit 7`/`EXIT_REFUSED_CARRIER`: 0 hits). `skip-changeset`: `scripts/pm/**` publishes nothing from any released package (no `files[]` of any package ships it). --- _Generated by [Claude Code](https://claude.ai/code/session_01W5y9kRg1YtYaMQYExVLRc2)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 83649b8 commit 30288da

2 files changed

Lines changed: 926 additions & 50 deletions

File tree

0 commit comments

Comments
 (0)