Skip to content

tooling(pm): the half-state patrol reports a governed PR nobody was asked to review (H43) - #15536

Merged
os-steve merged 3 commits into
mainfrom
claude/issue-14967-governed-review-request-patrol
Sep 4, 2026
Merged

tooling(pm): the half-state patrol reports a governed PR nobody was asked to review (H43)#15536
os-steve merged 3 commits into
mainfrom
claude/issue-14967-governed-review-request-patrol

Conversation

@claude

@claude claude Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Fixes #14967

The row

H43, one report-only row in the half-state patrol. Its predicate: an open PR whose changed-file list hits the governed register (governedPathsIn, imported from the governed-merge audit — the same matcher the queue guard and the pre-arm --test verdict use, never a third one) and whose coverage union of requested_reviewersassignees ∪ accounts that have already SUBMITTED a review does not contain every member of GOVERNED_APPROVERS (imported from the queue guard). Each of the three terms is load-bearing. assignees encodes the documented author-identity fallback exactly as the card's #14498 row describes it: when the PR's author is itself an approver a review request naming that account is impossible — GitHub answers 422 — so the fallback is to assign that account and request the other, and the row counts it as covered only if that assignment was actually taken. Authorship alone is never read as coverage, because "the author would surely look" is the assumption that produced zero. The submitted-review term is required in the opposite direction: GitHub drops a pending request the moment its reviewer submits, so without it the healthiest possible governed PR — one an approver has already reviewed — would fire this row forever.

The row is banded stall: nothing can ship wrong through this gap (the queue guard still refuses an unapproved governed enqueue), but the PR is stopped and no other sweep moves it. It never exits non-zero, writes no label, and requests nobody — the patrol's report-only contract is unchanged.

The register is imported, and loaded lazily

Both constants are imported rather than restated, so this row cannot drift from the register the guard enforces. The load is lazy and caught, which is a portability decision rather than a style one: this file is copied verbatim into sibling repos, and a sibling install carries the sweeper and its invoked-as helper and nothing else — measured while writing this, the objectui copy's scripts/pm/ holds check-half-states.mjs alone. A static import there would not degrade the patrol, it would end it at module load with ERR_MODULE_NOT_FOUND before any predicate runs, taking every other row down to buy this one. That is the same failure the patrol workflow's own adopt list already records against invoked-as.mjs. So an absent register makes this row alone report NOT MEASURED, with the reason and the remedy on the summary line, and the rest of the sweep is untouched.

The four-repo scope in the card is satisfied the way every other row is: the patrol is repo-agnostic and each install sweeps its own board, so objectui, cloud and hotcrm get this row when their copy of the script is refreshed. Those installs additionally need the governed pair beside the sweeper to move from NOT MEASURED to measured; without it they lose this row and nothing else.

Numbering

H43, not H41. The sweep emits H1–H38 (H17 and H22 file no rows), H39/H40 render as reserved sections, and H41/H42 are already in use as self-test fixture codes in the row-registry parser's cases — one of which pins H41 as UNREGISTERED, so banding it turns that case red. The row's own documentation is its code-site banner, following every row since H29; the head docblock's index stops at H28 by convention.

Test matrix

All cases are in the script's own offline --self-test (42 new cases; 2,106 total pass).

Case Input Expected
Both approvers requested governed, requested = [os-zhuang, hotlong] no row
Assignment instead of request governed, assignees = both no row
One approver requested governed, requested = [hotlong] row, naming os-zhuang only
Nobody asked governed, both fields empty row, naming both accounts
Approver-authored, fallback taken author hotlong, assigned hotlong, requested os-zhuang no row
Approver-authored, fallback not taken author hotlong, both fields empty row, naming the 422 and the assign fallback
Non-author approver missing author is not an approver row, and no 422 clause
Approver already reviewed requested = [hotlong], review submitted by os-zhuang no row
Unrelated reviewer review by a non-approver row
Not governed, nobody asked empty governed slice no row
Review leg unread reviewed is null row, declaring the leg missing rather than negative
Draft vs ready same PR both ways byte-identical verdict
Injected approver set a wholly different approver list drives the row; no name is baked in
Gathering policy not governed / already covered / short no probe / no probe / probe
Register the real imported matcher and approver set governed and clear both answered
Register absent ERR_MODULE_NOT_FOUND a named reason, never silence
Forwarding contract the four count keys present in the enumerated list
Band + registry familyBand, source scan stall, and the push is seen

Every one of these was shown failable. Five ablations, each mutated on disk (anchor gone, injected marker present, blob hash moved off the HEAD blob) and each restored to the HEAD blob byte-for-byte with an empty git diff HEAD: dropping the submitted-review leg reddened 2 cases; letting authorship count as coverage reddened 3; removing the band entry reddened 2 including the registry-coverage check; breaking the register import reddened only the positive control, which is also the proof that a sibling install without the pair degrades instead of dying; and dropping one count key reddened the forwarding-contract case. No build step is involved — the script runs from source, so there is no dist leg to rebuild.

Measured on the live board

The card's table is a snapshot the director sweep changed afterwards, so the population was re-derived. Two live sweeps of this repo, second one after the fix below:

  • 9 of the open PRs whose changed-file page was read hit the governed register; 4 fired the row — one with 33 ADR files and nobody asked, one skills/ PR with nobody asked, and two authored by an approver where the assign fallback was not taken, both of which rendered the 422 clause and neither of the other two did.
  • One governed PR requested one approver while the other approver had already submitted a review. It correctly produced no row — the live specimen for why the third union term exists.

The first live sweep found a real defect in this change and it is fixed in the second commit: the summary clause rendered 0 of the open PR(s) … 0 of 0 while filing four rows, because the counters were computed on stats and never copied into counts. That is precisely the failure the enumerated forwarding contract exists to end — a missing key and a genuine zero render identically — so all four keys now ride that list and a case pins them against the contract rather than against a rendered sentence.

REST cost per run

Zero additional changed-file requests. The pages are the shared-file row's, already fetched for a population wider than the governed one, so this row reuses them. The only request it buys is the submitted-review page, and only for a governed PR whose request/assignment union is already short — a PR that is already covered is decided without any probe. Measured on the live board: 30 open PRs, 29 file pages already in hand, 9 governed, 5 review pages, one page each. The cap is 25 per run, oldest-first because the PR nobody has asked about for longest is the one a budget must not hide; a candidate past the cap and a failed probe both leave the row standing on the two cheap fields with a sentence that says so.

Gate verdicts

Derived with dispatch-gates --repo objectstack-ai/objectstack --commands (no hand-made path list; the tool read the change set itself: 1 path). All run at 61e5f34d5, after merging current main so the derivation was not made against a stale tree.

  • 26 of the 27 derived commands exit 0, including pnpm check:pm-half-states (✓ check-half-states self-test: 2106 cases pass.), pnpm check:pm-dispatch-gates, pnpm check:nul-bytes (check-nul-bytes: OK … no raw ASCII control bytes), pnpm check:agent-test-spelling and pnpm check:watch-hint-literal.
  • The 27th, the bare check-partof-closing-keyword.mjs, exits 2 with its own words: NOT WIRED — neither PR_BODY nor PR_NUMBER is set … This is a wiring or usage failure, NOT a verdict. Its gate form, pnpm check:partof-closing-keyword, passes 28 cases, and it was additionally run against this body.
  • pnpm lint — the full repo scan, not a narrowed one — exits 0.
  • Two importers the path derivation does not name were run because this change alters a module they load: pnpm check:pm-clause2-carriers (150 cases) and the CI-failure reporter's self-test, both green.
  • check-governed-merges.mjs --test on the final file list: NOT governed, exit 0. This PR is an ordinary queue landing after review.
  • The script's watch-hint set is byte-identical before and after this change, so the two new module specifiers add no gate-derivation pollution.

Lock verdict for the gate union: os-verify-lock: VERDICT command-exit 0 · held the lock 423s, and for lint VERDICT command-exit 0 · held the lock 69s.

🤖 Generated with Claude Code

Generated by Claude Code


Generated by Claude Code

…sked to review (H43)

A governed-surface PR could sit open for days with no review request or
assignment to either `GOVERNED_APPROVERS` member, and no mechanical reading
covered that state: the queue guard asks about an approval at enqueue time, the
governed-merge audit is post-hoc, and every label-keyed row in this patrol is
blind to pull requests. Seven of ten open governed PRs across the four repos
were missing `os-zhuang` and two had asked nobody, until the maintainer asked
why no review requests were showing up.

H43 is one report-only row over the changed-file pages H36 already fetches, so
it buys zero extra file requests. Coverage is the union of `requested_reviewers`,
`assignees` and accounts that have already SUBMITTED a review — the third leg is
load-bearing because GitHub drops a pending request the moment its review lands,
and the second encodes the documented author-identity fallback (assign the
approver who cannot be requested, request the other). Authorship alone is never
read as coverage: that assumption is what produced zero.

`GOVERNED_SURFACES`/`governedPathsIn` and `GOVERNED_APPROVERS` are imported, not
restated, and loaded LAZILY: this file is copied verbatim into sibling repos that
carry no governed pair, where a static import would end the whole patrol at
module load instead of degrading one row. An absent register reports itself as
NOT MEASURED with the remedy, and every other row is untouched.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019RfFHiRCSs3JXLK4cwcfox
The first live sweep of the new row rendered "0 of the open PR(s) whose
changed-file page was read hit the governed register … 0 of 0" while filing FOUR
H43 findings. The counters were computed on `stats` and never copied into
`counts`, which is exactly the failure `SWEEP_COUNT_KEYS` was enumerated to end:
`counts.x ?? 0` renders a missing key and a genuine zero identically, so the one
mechanism whose job is to prove a pass examined something can go quiet with no
evidence that it did.

`governedRegisterReason` rides the same list although it is a string, not a
counter — without it a NOT MEASURED row would render as `0 of 0`, which is the
same confusion one layer down. A self-test case now pins all four against the
contract rather than against a rendered sentence.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019RfFHiRCSs3JXLK4cwcfox
@claude claude Bot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Sep 4, 2026
@github-actions github-actions Bot added the size/m label Sep 4, 2026
@os-steve
os-steve marked this pull request as ready for review September 4, 2026 20:38
@os-steve
os-steve enabled auto-merge September 4, 2026 20:38
@os-steve
os-steve added this pull request to the merge queue Sep 4, 2026
Merged via the queue into main with commit f7fd5c5 Sep 4, 2026
37 checks passed
@os-steve
os-steve deleted the claude/issue-14967-governed-review-request-patrol branch September 4, 2026 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/m skip-changeset PR has no user-facing published change; bypasses the changeset gate

Projects

None yet

2 participants