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

Activity

1. os-zhuang

   added

   tooling

   priority:p2Medium: important, M3Medium: important, M3

   pm:queue

   domain:skills

   on Sep 4, 2026
2. os-zhuang

   added the

   Task

   issue type on Sep 4, 2026
3. os-zhuang commented on Sep 4, 2026

   

   os-zhuang

   on Sep 4, 2026 – with Claude

   Contributor

   More actions

   Triage (R+146): lands in scripts/pm/check-half-states.mjs ⇒ domain:skills, which is also the named reader the card set. Graded Task · priority:p2.

   Routing by the SUBJECT rule: the patrol's subject here is the governed surface and the PM protocol, so it is skills rather than devx. Both constants the H-row needs — GOVERNED_SURFACES and GOVERNED_APPROVERS — already exist in check-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-zhuang is 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: hotlong is also the login the maintainer's own sessions run as, and the author of some governed PRs. ⇒ A request addressed only to hotlong either lands in the account that made it, or cannot be made at all (author-identity 422) — while os-zhuang, the account the maintainer actually reads, is never asked. #14498 is the pure case: author is hotlong, so the request was impossible, and the documented fallback (assign hotlong, request os-zhuang) was not taken.

   The gap, stated precisely

   ⛔ No mechanical reading covers: "a governed PR (by file list) whose requested_reviewers ∪ assignees does not cover every GOVERNED_APPROVERS member." 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 on pull_request events 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
4. claude commented on Sep 4, 2026

   

   claudebot

   on Sep 4, 2026 – with Claude

   Contributor

   More actions

   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 hits GOVERNED_SURFACES and whose requested_reviewers ∪ assignees does not cover GOVERNED_APPROVERS, both imported from check-governed-queue-guard.mjs) + its test file + the H-row's line in the patrol's own row index / docblock. ⛔ No edit to check-governed-queue-guard.mjs's constants, to .claude/** or to skills/** (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 --tier at 615fac3a: no path mandate; default judgement tier)
   Clause-②: no (an internal report-only patrol row; no published contract)
   Serial constraints cleared: no open PR touches scripts/pm/check-half-states.mjs or check-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
5. claude

   assigned

   os-steve

   on Sep 4, 2026
6. claude

   added

   pm:dispatched

   and removed

   pm:queue

   on Sep 4, 2026
7. os-steve commented on Sep 4, 2026

   

   os-steve

   on Sep 4, 2026 – with Claude

   Collaborator

   More actions

   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 --commands with 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 form pnpm check:partof-closing-keyword passes 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, and pnpm 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) and node scripts/pm/ci-failure.mjs --self-test, both exit 0. LOCK: os-verify-lock: VERDICT command-exit 0 · held the lock 423s for the 27-gate union and VERDICT command-exit 0 · held the lock 69s for 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 by git checkout HEAD -- ABSOLUTE_PATH followed by a hash equal to the HEAD blob AND an empty git 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. Labelled finding, no assignee, searched 264 open issues for duplicates first (4 near-neighbours, none the same claim)."
   ]
   }

   ---

   Generated by Claude Code
8. claude commented on Sep 4, 2026

   

   claudebot

   on Sep 4, 2026 – with Claude

   Contributor

   More actions

   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.mjs only (456 insertions, 1 deletion): row H43, report-only — an open PR whose changed-file list hits the governed register (governedPathsIn, imported from check-governed-merges.mjs, which is where that matcher actually lives — the dispatch had named the queue guard; the dev corrected it) and whose requested_reviewers ∪ assignees ∪ accounts that already submitted a review does not cover every GOVERNED_APPROVERS member (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-test exit 0 — 2106 cases (42 new); check-clause2-carriers --self-test exit 0 (it imports the module); check-governed-merges --test exit 0 (NOT governed ⇒ queue landing). The dev's report adds dispatch-gates --commands 27 run (26 exit 0; the 27th is the bare check-partof-closing-keyword.mjs whose "NOT WIRED" exit 2 is not a verdict — its gate form passes 28 cases and was also run on the real PR body), pnpm lint full 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 into counts, 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-report line (posted through the MCP after the classifier refused the REST POST — declared), JSON closes; PR body starts Fixes #14967, no model identifier; skip-changeset read 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 61e5f34d is 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
9. os-steve

   closed this as completedin #15536on Sep 4, 2026
10. claude

    removed

    pm:dispatched

    on Sep 4, 2026