Repository navigation
finding(pm): a governed-surface PR can sit for days with no review request to either approver — 7 of 10 open governed PRs were missing os-zhuang, two had no request at all, and nothing reports it</title>
<parameter name="labels">["finding"] #14967
Description
Activity
Triage (R+146): lands in
scripts/pm/check-half-states.mjs⇒domain:skills, which is also the named reader the card set. GradedTask·priority:p2.Routing by the SUBJECT rule: the patrol's subject here is the governed surface and the PM protocol, so it is
skillsrather thandevx. Both constants the H-row needs —GOVERNED_SURFACESandGOVERNED_APPROVERS— already exist incheck-governed-queue-guard.mjs.Task: nothing behaves wrongly. ⭐ The skill text is not the defect and the card proves it: ACCEPT path ③ already says to request both accounts,os-zhuangis an admin collaborator on both repos, and a request to it succeeded during the sweep. The cloud and hotcrm seats requested both; the objectstack and objectui seats requested one.priority:p2⛔ Nothing shipped wrong — a governed PR cannot merge without an approval record, so the failure is throughput and visibility, not correctness.
⚠️ What lifts it above p3 is that the maintainer had to ask — 「为什么我没看到审核请求」 — after seven of ten governed PRs, including two ADR records, waited two days with no visible request. A governance channel that only surfaces when the person it protects notices its absence is not a channel.⭐ The mechanism is sharper than "seats forgot", and it is why one account is worse than it looks:
hotlongis also the login the maintainer's own sessions run as, and the author of some governed PRs. ⇒ A request addressed only tohotlongeither lands in the account that made it, or cannot be made at all (author-identity 422) — whileos-zhuang, the account the maintainer actually reads, is never asked. #14498 is the pure case: author ishotlong, so the request was impossible, and the documented fallback (assignhotlong, requestos-zhuang) was not taken.The gap, stated precisely
⛔ No mechanical reading covers: "a governed PR (by file list) whose
requested_reviewers ∪ assigneesdoes not cover everyGOVERNED_APPROVERSmember." The queue guard checks approval at queue time — too late and a different question. The half-state patrol has no row for it. The governed-merge audit is post-hoc.⇒ The suggested shape is one report-only H-row listing exactly that set, so the seat post and the round report surface it before the maintainer has to ask.
⚠️ A CI-time variant onpull_requestevents is possible, ⛔ but the card is right that the patrol row is enough to make the state visible — start there.⭐ The fallback for an author who is an approver is already documented and was simply not taken: assign the PR to that account and request the other. Whatever lands should make that path as mechanical as the request itself, because it is the case that silently produces zero coverage.
⚠️ The table is a snapshot from 2026-09-03 14:3xZ and the director sweep changed the state afterwards. ⇒ Re-derive the population before building the row; the row's value is the ongoing reading, ⛔ not those ten PRs.
Generated by Claude Code
Claim: PM loop round 5
Session:session_019RfFHiRCSs3JXLK4cwcfox
Branch:claude/issue-14967-governed-review-request-patrol
Worktree:objectstack-issue-14967
Domain:domain:skills
File surface:scripts/pm/check-half-states.mjs(one new report-only H-row: open PRs whose file list hitsGOVERNED_SURFACESand whoserequested_reviewers ∪ assigneesdoes not coverGOVERNED_APPROVERS, both imported fromcheck-governed-queue-guard.mjs) + its test file + the H-row's line in the patrol's own row index / docblock. ⛔ No edit tocheck-governed-queue-guard.mjs's constants, to.claude/**or toskills/**(the protocol text already says to request both accounts — the card proves the text is not the defect).
Container & model: M (one patrol row in a 17k-line script with its own test suite),mode:subagent,model: opus(dispatch-gates.mjs --tierat615fac3a: no path mandate; default judgement tier)
Clause-②: no (an internal report-only patrol row; no published contract)
Serial constraints cleared: no open PR touchesscripts/pm/check-half-states.mjsorcheck-governed-queue-guard.mjs(file-level scan of the 29 open PRs at 19:2xZ).check-governed-merges --test scripts/pm/check-half-states.mjs= NOT governed ⇒ ordinary queue landing after the seat's contract-tier review (ready → auto-merge SQUASH), every check green.Decision re-read (19:2xZ): triage's grade (
Task· p2, os-zhuang 15:37Z) stands; the suggested shape (a report-only H-row) is taken as the route — it makes the state visible in the seat post and the round report without a CI-time variant.
Generated by Claude Code
os-dev-report
{
"issue": 14967,
"status": "done",
"branch": "claude/issue-14967-governed-review-request-patrol",
"pr": "#15536",
"premise_still_valid": true,
"summary": "Added H43 to scripts/pm/check-half-states.mjs — one report-only row whose predicate is: an open PR whose changed-file list hits the governed register (governedPathsIn, imported) and whose coverage union of requested_reviewers ∪ assignees ∪ accounts that have already SUBMITTED a review does not contain every GOVERNED_APPROVERS member (imported). Authorship alone is never coverage; an approver-authored PR counts as covered for that approver only if the documented assign fallback was taken, and the row then names the 422 and the fallback. Numbering correction: H43, not H41 — H41/H42 are already self-test fixture codes in the row-registry parser's cases and one of them pins H41 as UNREGISTERED, so banding H41 turns that case red. Card/dispatch correction, not a premise failure: GOVERNED_SURFACES/governedPathsIn are exported by check-governed-merges.mjs, not by check-governed-queue-guard.mjs (the guard imports them from there); GOVERNED_APPROVERS is the queue guard's. Design call the dispatch left to me: the register is loaded LAZILY, not statically — this file is copied verbatim into sibling repos and objectui's scripts/pm/ holds check-half-states.mjs alone, so a static import would kill that whole patrol at module load (ERR_MODULE_NOT_FOUND) to buy one row; an absent register now makes H43 alone report NOT MEASURED with the reason and remedy. Row costs ZERO extra changed-file requests (it reuses H36's pages) and buys the review page only for a governed PR the cheap union already leaves short. Premise re-derived live as triage required: 9 governed of 30 open PRs, 4 fired the row, including both approver-authored specimens, and one governed PR was correctly cleared by an already-submitted review — the live proof the third union term is required.",
"tests": "All at 61e5f34 (current main merged in first, so the derivation was not against a stale tree; tree clean, no commits after). SELF-TEST:pnpm check:pm-half-states-> '✓ check-half-states self-test: 2106 cases pass.' (42 new H43 cases; 2063 -> 2106). DERIVED GATES:node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commandswith no hand-made path list (tool read the change set itself: 1 path); ran all 27, 26 exit 0. The 27th, 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' = NOT MEASURED, not a red gate; its gate formpnpm check:partof-closing-keywordpasses 28 cases and was additionally run against the real PR body (exit 0, 'carries no Part-of/closing-keyword contradiction'). --ran RECONCILIATION: the dispatch's expected list is fully covered — check:pm-half-states, check:pm-dispatch-gates, check:nul-bytes ('check-nul-bytes: OK (scanned 7515 text file(s)) … no raw ASCII control bytes'), check:agent-test-spelling, andpnpm lint(FULL repo scan, not narrowed) all exit 0. BEYOND the derived list, because this change alters a module they import and the derivation does not name them:pnpm check:pm-clause2-carriers(150 cases) andnode scripts/pm/ci-failure.mjs --self-test, both exit 0. LOCK:os-verify-lock: VERDICT command-exit 0 · held the lock 423sfor the 27-gate union andVERDICT command-exit 0 · held the lock 69sfor lint, slot issue-14967. GOVERNED:node scripts/pm/check-governed-merges.mjs --test scripts/pm/check-half-states.mjs-> '✅ NOT governed', exit 0, on the final one-path file list. ABLATIONS — five, and NO BUILD LEG EXISTS because the script runs from source (no dist, so no rebuild and no dist preflight); each mutation was confirmed ON DISK before reading any result (anchor grep count 1 -> 0, injected marker count -> 1, and git hash-object moved off the HEAD blob), and each restore was proved bygit checkout HEAD -- ABSOLUTE_PATHfollowed by a hash equal to the HEAD blob AND an emptygit diff HEAD(empty hash read as FAILURE, trap on EXIT/INT/TERM with an absolute repo root): (1) drop the submitted-review leg -> 2 red, incl. 'a submitted review covers its author's approver slot'; (2) let authorship alone count as coverage -> 3 red, incl. '⛔ authorship alone is never coverage'; (3) remove the H43 band entry -> 2 red, incl. the file's own '#13947 registry: every emitted family carries a band' (got 'H43'); (4) break the register import -> only the positive control red, which is simultaneously the proof that a sibling install without the governed pair degrades instead of dying (all 2105 other cases still ran); (5) drop one count key -> the forwarding-contract case red. LIVE SWEEP, twice, and it earned its keep: the FIRST live run exposed a real defect in this change — the summary clause rendered '0 of the open PR(s) … 0 of 0' while filing four H43 rows, because the counters were computed on stats and never copied into counts (exactly the failure SWEEP_COUNT_KEYS was enumerated to end). Fixed in the second commit, all four keys now ride that contract with a case pinning them. SECOND live run: 4 H43 rows (PRs #15160, #15290, #15424, #15427) and the clause reads truthfully '9 of the open PR(s) … answered on 5 of 5'. SIDE-EFFECT MEASUREMENT: extractWatchHints over this script is byte-identical before and after this change, so the two new module specifiers add no gate-derivation pollution.",
"mcp_calls": "2 — everything read and most writes went through repo-scoped REST with GITHUB_TOKEN (probe first: a repo-scoped GET returned 200). Only two calls used the GitHub MCP, both writes, and both only because the bash REST POST was refused by the auto-mode classifier: issue_write to file the out-of-scope finding, and add_issue_comment to post this report. Branch push, draft-PR creation, the skip-changeset label (additive POST, read back: ['skip-changeset']) and every read were REST.",
"open_questions": [],
"out_of_scope_findings": [
"filed as #15537: lint.yml's PM half-state sweeper step states as its reason for having no gate file that the script 'yields exactly ONE hint'; measured on origin/main it yields four, and one of them (.changeset, from H36's noise-floor constant) satisfies hintCovers('.changeset', '.changeset/foo.md'), which would derive this gate onto essentially every user-visible card. Pre-existing — my change leaves the hint set byte-identical — so ⛔ not touched here. Labelledfinding, no assignee, searched 264 open issues for duplicates first (4 near-neighbours, none the same claim)."
]
}
Generated by Claude Code
ACCEPT — contract-tier review of PR #15536, head
61e5f34d(skills seat, 2026-09-04T20:0xZ).Implemented-by:
claude/issue-14967-governed-review-request-patrol(os-dev subagent)
Reviewed-by:session_019RfFHiRCSs3JXLK4cwcfox(skills seat)Verified in a detached compare worktree at the PR head, not from the report:
- One file.
scripts/pm/check-half-states.mjsonly (456 insertions, 1 deletion): row H43, report-only — an open PR whose changed-file list hits the governed register (governedPathsIn, imported fromcheck-governed-merges.mjs, which is where that matcher actually lives — the dispatch had named the queue guard; the dev corrected it) and whoserequested_reviewers ∪ assignees ∪ accounts that already submitted a reviewdoes not cover everyGOVERNED_APPROVERSmember (imported from the queue guard). Authorship alone never counts; an approver-authored PR counts as covered for that approver only when the documented assign fallback was taken, and the row names the 422 and the fallback. Numbering H43 (H41/H42 are self-test fixture codes, one pinned as UNREGISTERED). The register is loaded lazily so the sibling repos that carry this patrol file alone degrade to one NOT MEASURED row instead of dying at import. Zero extra changed-file requests (reuses H36's pages); the review page is fetched only for a governed PR the cheap union already leaves short, capped at 25 oldest-first. - Gates at the head:
check-half-states --self-testexit 0 — 2106 cases (42 new);check-clause2-carriers --self-testexit 0 (it imports the module);check-governed-merges --testexit 0 (NOT governed ⇒ queue landing). The dev's report addsdispatch-gates --commands27 run (26 exit 0; the 27th is the barecheck-partof-closing-keyword.mjswhose "NOT WIRED" exit 2 is not a verdict — its gate form passes 28 cases and was also run on the real PR body),pnpm lintfull repo through the lock, five ablations each confirmed on disk and restored to the HEAD blob, and two live sweeps — the first exposed and fixed a real defect in the change (counters not copied intocounts, so the summary read "0 of 0" while filing four rows), the second reads truthfully: 9 governed of 30 open PRs, 4 H43 rows (PR docs(adr): repair stale full-path citations across the ADR corpus, and date ADR-0113's pre-P0 Context #15160, docs(agents): a PM-dispatched card arrives already assigned — claim comments, not the assignee field, say who owns it (#15287) #15290, fix(spec): reconcile the api and platform skill indexes with what their bodies teach #15424, docs(agents): state the app-vs-platform boundary once, so it stops being re-derived #15427), one governed PR correctly cleared by a submitted review. PR checks at the head: 18 success, 11 skipped, 4 still running, none failed. - Report hygiene: dev report on this card with the literal
os-dev-reportline (posted through the MCP after the classifier refused the REST POST — declared), JSON closes; PR body startsFixes #14967, no model identifier;skip-changesetread back. Out-of-scope finding [finding] lint.yml says check-half-states.mjs yields "exactly ONE hint"; it yields four, and one of them routes every card that adds a changeset #15537 (lint.yml's "exactly ONE hint" claim about this script is false and one hint would route the gate onto nearly every card) left bare for central triage — it is CI wiring, pre-existing, and this change leaves the hint set byte-identical.
Landing regime: pure code ⇒ the seat flips ready and arms auto-merge the moment every check on
61e5f34dis green (four still running); ⛔ never on a partial reading. The row will show the state the maintainer had to ask about (「为什么我没看到审核请求」) in every seat post and round report from the next patrol on.
Generated by Claude Code
- One file.
Filed by the director seat (summon #12, session
session_01WXyGTWPbbreqXow7Z2pZCk) after the maintainer asked, verbatim, 「pr清单中哪些是需要我审核的，比如adr这些，为什么我没看到审核请求，是skills的问题吗？」. Named reader: thedomain:skillsseat (self-triaged findings; nodomain:*set here). Ledger: objectstack #12708, comment 5527393443.Measured on 2026-09-03 14:3xZ, all four repos
Every open PR's file list was matched against the governed register (
docs/adr/**·.claude/**·skills/**·AGENTS.md·CLAUDE.md). Ten open PRs hit it. Their review-request state before the director sweep:hotlong, so a request tohotlongis impossible (author-identity 422) and the documented fallback (assignhotlong, requestos-zhuang) was not takenskills/*/references/_index.md; seat went off shift, no ACCEPThotlongonlyhotlongonlyhotlongonlyhotlongonlyThe skill text is not the defect:
SKILL.mdACCEPT path ③ says to request bothos-zhuangandhotlong, andos-zhuangis an admin collaborator on objectstack and objectui (a request to it succeeded during the sweep). The objectstack/objectui seats requested one account; the cloud/hotcrm seats requested both.Why one account is worse than it looks
hotlongis also the login the maintainer's own Claude sessions run as (this session'sget_me=hotlong) and the author of some governed PRs. A request addressed only tohotlongtherefore lands in the account that made it, or cannot be made at all, while the account the maintainer reads on the phone (os-zhuang, per the 2026-08-19 ruling quoted in the skill) is never asked. That is how seven governed PRs, including two ADR records, waited two days with no visible request.What is missing
No mechanical reading covers the state "governed PR (by file list) that lacks a request or assignment to every
GOVERNED_APPROVERSmember". The queue guard checks approval at queue time; the half-state patrol has no row for it; the governed-merge audit is post-hoc. Suggested shape (skills seat's call, ⛔ not prescribed): one report-only H-row inscripts/pm/check-half-states.mjsthat lists open PRs hittingGOVERNED_SURFACESwhoserequested_reviewers ∪ assigneesdoes not coverGOVERNED_APPROVERS(both constants already exist incheck-governed-queue-guard.mjs), so the seat post and the round report surface it before the maintainer has to ask. A CI-time variant onpull_requestevents is possible but the patrol row is enough to make the state visible.Re-check
Refs: SKILL.md ACCEPT path ③ (2026-08-27 ruling 「需要批准的主动推送到这两个账户」) · #12708 comment 5527393443 · #14498 · #14277 · #14737