Skip to content

Commit 1db5322

Browse files
fix(pm): check-governed-merges refuses a flag its mode does not read, and a bare --pr beside a PM_SWEEP_REPO naming another repository (#21690)
Fixes #21675 Clause-②: no `scripts/pm/check-governed-merges.mjs` now has a closed argument set for each mode. If a run passes a flag its mode does not read, the tool refuses with a usage error (exit 1) and names the flag. It does this before any git read, network read or child process. A bare `--pr N` is refused when `PM_SWEEP_REPO` names a repository other than the one a bare number answers. That refusal prescribes the documented `--pr OWNER/REPO#N` spelling. No new `--repo` meaning was added: `--repo` is refused like any other flag the tool does not read, and the refusal spells out the qualified number. The exit code table is unchanged, because every new refusal uses the existing code 1. `--test`, `--branch`, the sweep and `--self-test` answer exactly as before for every argument list they accepted before. One file changed: `scripts/pm/check-governed-merges.mjs` (+383 / -7). ## The ruling this implements (triage comment on the card, quoted) > **Direction: the card's second option, which adds no new spelling.** > - An unrecognised flag is a usage refusal (exit 1), as `label-write.mjs`, `post-stamped.mjs` and `issue-create.mjs` already refuse. > - A bare `--pr N` is refused when `--repo` or `PM_SWEEP_REPO` names any repository other than objectstack. The refusal prescribes the documented `owner/repo#N` spelling. > - ⛔ No new `--repo` meaning for this tool: one spelling, the documented one. > - **Pins (one self-test row per refusal):** an unknown flag is refused; `--pr N --repo objectstack-ai/objectui` is refused; `PM_SWEEP_REPO=objectstack-ai/objectui --pr N` is refused; `--pr objectstack-ai/objectui#N` answers as today. ## Reading 1: the card's four invocations, before and after Before: base `7d0781482d` (`origin/main` at worktree creation). After: head `6854ff6062`. Each `--pr` row made real read-only `GET`s against the GitHub API. Angle-bracket placeholders in the tool's usage text are written below as `N`, `OWNER/REPO` and `PATH`, because the body sanitizer removes angle-bracket fragments. | # | invocation | before (`7d0781482d`) | after (`6854ff6062`) | |:--|:--|:--|:--| | 1 | `--pr 11590 --repo objectstack-ai/objectui` | exit 0, read `objectstack-ai/objectstack/pulls/11590` (7 files), ✅ NOT governed | exit 1, refused, no read, prescribes `--pr objectstack-ai/objectui#11590` | | 2 | `PM_SWEEP_REPO=objectstack-ai/objectui` + `--pr 11590` | exit 0, same objectstack PR (7 files), ✅ NOT governed | exit 1, refused before the first API read, prescribes both qualified spellings | | 3 | `--pr 11590 --bogus-flag x` | exit 0, same objectstack PR (7 files), ✅ NOT governed | exit 1, refused by name, no read | | 4 | `--pr objectstack-ai/objectui#11590` | exit 0, read `objectstack-ai/objectui/pulls/11590` (18 files), ✅ NOT governed | exit 0, **byte-identical** stdout and stderr to before (`diff` empty); also identical with `PM_SWEEP_REPO=objectstack-ai/objectui` set | Before, rows 1 to 3 (all three printed the same thing): ``` derived from GET /repos/objectstack-ai/objectstack/pulls/11590/files (three-dot by construction): 7 path(s) from 7 changed file(s), over 1 page(s). ✅ NOT governed — ordinary queue landing applies to a PR with exactly this file list. ``` After, row 1: ``` ❌ `--repo` is not a flag --pr reads. An unrecognised flag is REFUSED, never ignored: an ignored flag still lets the run print a verdict, about a question nobody asked, under the same ✅ as a real answer. This tool takes no `--repo`, and a bare `--pr 11590` answers THIS checkout's own repository whatever `--repo` names. Name the repository inside --pr, the one spelling it takes: node scripts/pm/check-governed-merges.mjs --pr objectstack-ai/objectui#11590 (usage block follows: the active mode's line, then every mode's line) ``` After, row 2: ``` ❌ --pr 11590 is a bare number, and a bare number answers THIS checkout's own repository (objectstack-ai/objectstack) — but PM_SWEEP_REPO names objectstack-ai/objectui. This tool takes no repository from the environment, so the run is refused rather than answered about a different PR in a different repository. Name the repository inside --pr, the one spelling it takes: node scripts/pm/check-governed-merges.mjs --pr objectstack-ai/objectui#11590 or, if this checkout's own PR is the one you mean: node scripts/pm/check-governed-merges.mjs --pr #11590 ``` After, row 3: ``` ❌ `--bogus-flag` is not a flag --pr reads. An unrecognised flag is REFUSED, never ignored: an ignored flag still lets the run print a verdict, about a question nobody asked, under the same ✅ as a real answer. --pr reads: node scripts/pm/check-governed-merges.mjs --pr N | --pr OWNER/REPO#N [--root PATH] [--json] ``` Two more invocations show the same problem through a flag that belongs to another mode. Both were measured on the base and are closed by the per-mode sets: - `--pr 11590 --repos objectui`: before, exit 0, read objectstack's PR 11590. After, exit 1, ``--repos` is not a flag --pr reads``. - `--test src/x.ts --since 7d`: before, exit 0 with `0 of 2 path(s)`, because `7d` had joined the path list. After, exit 1, ``--since` is not a flag --test reads``. ## Reading 2: self-test cases and battery floors, before and after | | base `7d0781482d` | head `6854ff6062` | |:--|:--|:--| | `node scripts/pm/check-governed-merges.mjs --self-test` | `✓ … 454 assertions`, exit 0 | `✓ … 476 assertions`, exit 0 | | declared batteries / `SELF_TEST_BATTERY_FLOOR` | 31 / 31 | 32 / 32 | | new battery `⭐ the argv is CLOSED: …` | (none) | 22 cases registered, floor 22 | How the batteries and floors were handled: the rows pin a different predicate from the ones in the existing `#17003` battery. That battery is about how the file list is derived. These rows are about which arguments a run may carry at all. So they get a battery of their own instead of raising another battery's count. As AGENTS.md requires ("pin battery NAMES, never one total"), the roster floor moves from 31 to 32 in the same edit. If the new battery were deleted from the roster, it would fail at the floor instead of disappearing without a trace. The battery floor equals the cases the battery registers (22): 454 + 22 = 476. Each ruled pin has its own row, and the API is a fake on 127.0.0.1 (`GITHUB_API_URL`) that records every request. The child process gets both token variables blanked, `PM_SWEEP_REPO` cleared and `NO_PROXY` set for loopback, so the self-test still makes no external connection: - unknown flag: `⭐ e2e-pr-N-with-a-bogus-flag-exits-1-names-it-prints-no-verdict-and-READS-NOTHING`, plus pure rows for the walker, flags from another mode, `--since=7d` (the refusal prescribes `--since 7d`), and `--pr --bogus` (a flag is never taken as a value) - `--pr N --repo objectstack-ai/objectui`: `⭐ e2e-pr-N---repo-objectui-exits-1-prescribes---pr-objectui#N-and-READS-NOTHING`, plus pure rows. A governed repo id after `--repo` is written out as its slug. `--repo` naming objectstack itself is still refused, so no new meaning exists. - `PM_SWEEP_REPO=objectstack-ai/objectui --pr N`: `⭐ e2e-PM_SWEEP_REPO-objectui-with-a-bare---pr-N-exits-1-prescribes---pr-objectui#N-and-READS-NOTHING`, plus pure rows. If the variable names this checkout's own repository (in any letter case), or is blank or unset, nothing is refused. - `--pr objectstack-ai/objectui#N` answers as today: `⭐ e2e---pr-objectui#N-answers-as-today-NOT-governed-on-exit-0-every-read-from-objectuis-PR-N` (exactly 3 reads, all under `/repos/objectstack-ai/objectui/`), and `and-PM_SWEEP_REPO-beside-the-qualified-spelling-changes-not-one-byte-of-the-answer`. - Control and preservation rows: - `PM_SWEEP_REPO` naming objectstack with a bare number still answers from objectstack. - Every argument list the earlier batteries spawn still passes. This includes `--additions`/`--deletions` beside `--branch`, so that mode's own "two readings" refusal still fires. - Every usage example the header gives (13, read from the header itself) still passes. - Every flag a mode's usage line names is a flag that mode reads. ### Ablations: each refusal's pin turns red, and the file is restored to the HEAD blob Each ablation ran through `node scripts/ablation-replace.mjs --file ABS_PATH --anchor … --replacement … -- node ABS_PATH --self-test`. The fix was committed first, each anchor count went from 1 to 0, and each restore reported `ok restored: blob == HEAD (89af2f9) and git diff HEAD is empty`, which was checked again independently after each leg. | leg | mutation | self-test | rows that turned red | |:--|:--|:--|:--| | A1 unknown flag | `if (unreadRefusal !== null)` changed to `if (false && …)` in `main()` | exit 1, 3 failures | e2e bogus-flag, e2e `--repo` and e2e `--test … --since`: each `status=0`, and the first two read `/repos/objectstack-ai/objectstack/pulls/11590…`, which is the card's measured wrong answer | | A2 `--repo` | `'--repo': 'value'` added to the `--pr` table (`--repo` silently accepted) | exit 1, 3 failures | the pure `--repo` prescription row, the "no new spelling" row, and e2e `--repo` (`status=0`, read objectstack's PR) | | A3 `PM_SWEEP_REPO` | `if (ambiguous !== null)` changed to `if (false && …)` in `runPullMode` | exit 1, 1 failure | e2e `PM_SWEEP_REPO` (`status=0`, read objectstack's PR) | | A4 qualified spelling | `bareTargetRefusal` also refuses a qualified `--pr` (over-refusal) | exit 1, 3 failures | the pure qualified row, e2e "answers as today" (`status=1`, `reads=[]`), and the byte-identical row | ## Gates (run at head `6854ff6062`) `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` (no path argument) derived 32 commands from the change set: 1 path against merge base `7d0781482`, +383 / -7. That list is identical to the one in the dispatch. All 31 commands outside the lock exited 0, and each printed its own pass line. Among them: `pnpm check:pm-governed-merges` (476 assertions), `check-scripts-symbol-anchors` (3760 anchors resolve), `check:entry-guard` (221 export bindings, all inert on import), `check-self-test-wired` and `check-self-test-workflow-commands`, `check-comment-mask-corpus` (8140 files, 0 disagree) and `check:nul-bytes` (OK). `pnpm check:pm-dispatch-gates` ran under the shared verify lock and passed: `VERDICT command-exit 0`, `✓ check:pm-dispatch-gates --self-test: the exit contract holds in all three directions.` and `✓ dispatch-gates self-test: 1976 cases pass.` It held the lock for 1224s (20m24s). The box was shared, and unlocked sibling work ran alongside it. The first attempt was still queued behind another seat's run of the same gate when its budget ran out (exit 99, slot kept). The resumed slot acquired after 476s more. Reconciliation: `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --ran ran.list` (one `COMMAND :: exit CODE` line per command, recorded as each one ran) printed `✓ dispatch-gates --ran: 32 derived famil(ies) accounted for — 32 run, 0 NOT-MEASURED (a DERIVED zero — all 32 recorded an exit code and none of them is 3).` Lint, narrowed and stated as such: `eslint --no-inline-config --format json scripts/pm/check-governed-merges.mjs` checked 1 file and found 0 errors and 0 warnings. The resolved config for the file (`--print-config`) applies 2 rules (`no-restricted-imports`, `comment-swallow/no-code-inside-block-comment`) with `parserOptions` limited to `ecmaVersion`/`sourceType`. Type-aware linting is off (no `project`/`projectService` anywhere in `eslint.config.mjs`), so this diff cannot change the lint result for any file it does not touch. The repo-wide `pnpm lint` is left to CI. ## Acceptance notes - **How "objectstack" in the ruling is read.** The `PM_SWEEP_REPO` comparison is made against the repository a bare number actually answers, which is the slug this checkout's `origin` parses to. That is `objectstack-ai/objectstack` in every run from this repo, so the behaviour is exactly the ruling's. It differs only under `--root` pointing at another checkout, where it also refuses the reverse mismatch. The comparison ignores letter case. A malformed `PM_SWEEP_REPO` value also refuses, and the prescription then uses an `OWNER/REPO` placeholder. - **Why the sets are per mode and not one list for the whole tool.** With a single tool-wide list, `--pr 11590 --repos objectui` would still answer about objectstack's PR, which is the same class of wrong answer. Both cases were measured on the base (Reading 1). `--additions`/`--deletions` stay readable to `--pr`/`--branch` only so that those modes' existing, more specific refusal keeps its own words. - **Where the refusals happen.** The closed-set refusal happens in `main()`, before the proxy re-exec, so a refused run starts no child process. The `PM_SWEEP_REPO` refusal happens in `runPullMode` beside the existing `--pr` argument errors. In a proxied container that means after the one re-exec, but still before any API read. - **What this does not cover.** - `GITHUB_REPOSITORY` is not read: the ruling names two spellings. The board tools' `resolveSweepRepo` falls back to it, but nothing here adopts that. - `--self-test` keeps its own arguments. Its exit 1 means "a finding about this file", and it takes the `--fixture-child` marker. - Positional tokens no mode consumes (`--pr 42 43`), and a repeated single-value flag (`--pr 1 --pr 2` reads the first), are still accepted. The ruling covers flags, and these were read from the code but not measured. Recorded here, not filed. - No changeset: `scripts/pm/**` belongs to no published package (`skip-changeset`). --- _Generated by [Claude Code](https://claude.ai/code/session_01CB6W87z22K2yjUCDyVrJRk)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent ced217c commit 1db5322

1 file changed

Lines changed: 383 additions & 7 deletions

File tree

0 commit comments

Comments
 (0)