perf(pr-management-code-review): select, scan and dispose as code, one file per outcome

.apache-magpie-overrides/tools/vetted-ops/config.toml

@@ -67,6 +67,19 @@ close_reasons = ["completed", "not planned"]
     "repo-file",
     "label-list",
     "gql-pr-review-threads",
+    "team-members",
+    "gql-pr-liveness",
+    "gql-cr-open",
+    "gql-cr-pr",
+    "gql-cr-pr-stack",
+    "cr-codeowners-github",
+    "cr-codeowners-root",
+    "cr-codeowners-docs",
+    "cr-pr-template",
+    "cr-viewer-commits",
+    "cr-commit-files",
+    "cr-path-commits",
+    "cr-commit-pulls",
 ]
 
 # pr-management-triage reads a whole sweep through `vetted-op-read --save`,

docs/pr-management/README.md

@@ -139,7 +139,7 @@ says which file is missing.
 | [`mentoring-config.md`](../../plugins/magpie-setup/templates/mentoring-config.md) | Tone knobs and hand-off protocol for the thread-level mentoring skill. | `mentor` |
 | [`pr-management-triage-ci-check-map.md`](../../plugins/magpie-setup/templates/pr-management-triage-ci-check-map.md) | CI-check name pattern → category name + doc-URL mapping for the violations comment. | `pr-triage` |
 | [`privacy-llm.md`](../../plugins/magpie-setup/templates/privacy-llm.md) | Which model tier may see which class of content, for projects routing foundation-private information away from third-party models. | `pr-triage`, `reviewer-routing` |
-| [`release-trains.md`](../../plugins/magpie-setup/templates/release-trains.md) | Active release branches, release-manager attribution per cut, rotation rosters, security-team roster. | `code-review`, `reviewer-routing` |
+| [`release-trains.md`](../../plugins/magpie-setup/templates/release-trains.md) | Active release branches, release-manager attribution per cut, rotation rosters, security-team roster. | `reviewer-routing` |
 | [`stale-sweep-config.md`](../../plugins/magpie-setup/templates/stale-sweep-config.md) | Grace windows and exemption labels for stale sweeps. Absent, the framework defaults apply. | `pr-stale-sweep` |
 
 <!-- END generated: skill-config -->

plugins/magpie-pr-management/skills/code-review/background-subagents.md

@@ -0,0 +1,173 @@
+<!-- SPDX-License-Identifier: Apache-2.0
+     https://www.apache.org/licenses/LICENSE-2.0 -->
+
+# Background analysis subagents
+
+## Why
+
+Step 2 (`gh pr view` + `gh pr diff` + per-tree `AGENTS.md`
+discovery) and Step 4 (line-by-line classification against the
+criteria source files) together dominate the per-PR
+wall-clock cost. While the maintainer is reading the current
+PR's draft, those steps can run for the *next* PRs in
+parallel — when the maintainer reaches them, the package is
+already drafted and only Step 5 (when a tool-path adversarial
+reviewer is configured), Step 6 (disposition pick) and Step 7
+(confirmation) are left to run.
+
+The maintainer never sees the subagents directly. They run
+silently in the background; their output is what powers the
+"instant headline" experience.
+
+## When to fire
+
+After the working list is resolved (Step 1 of `SKILL.md`), and
+again after each PR is posted or skipped (Step 9 above), the
+parent skill keeps a **lookahead window** of size `K` filled
+with in-flight subagents.
+
+```text
+queue: [N0_current, N1, N2, N3, N4, N5, ...]
+        ^foreground   ^^^^^^^^^^^   ^prefetched (K=3)
+                       lookahead
+```
+
+Default `K` is **3**. Tune via `lookahead:<N>` at session start
+or disable entirely with `no-prefetch`.
+
+Subagents are launched with `run_in_background=true` so the
+parent does not block; the parent picks up their results when
+the maintainer reaches the corresponding PR (or earlier, if
+several finish before the maintainer is done with the current
+one — that's fine, the results just sit in memory).
+
+## Subagent contract
+
+Each subagent is a `general-purpose` Agent invocation. The
+prompt is **self-contained** (no shared conversation context)
+and includes:
+
+- **Inputs** — the PR number, the target repo, the maintainer's
+  GitHub login (for the self-review guard), the working
+  directory path so the subagent can read the criteria source
+  files locally, AND the **pre-fetched PR payload inline in
+  the prompt**: the JSON metadata blob, the unified diff, and
+  the unresolved-review-threads JSON. The parent runs `gh pr
+  view`, `gh pr diff`, and the review-threads GraphQL query
+  itself before invoking the subagent and embeds the raw
+  output in the prompt.
+
+  Why pre-fetch in the parent: in many harness configurations
+  subagents do **not** inherit the parent session's Bash
+  permission grants for `gh` — they start a fresh permission
+  context and are denied. Embedding the data inline lets the
+  subagent run with Read-only tool access (the criteria files
+  are already on disk) and removes the permission failure
+  mode entirely. The parent's `gh` calls are cheap (3 per PR)
+  and run in parallel with the maintainer's confirmation of
+  the *previous* PR — no wall-clock cost.
+
+- **Task** — walk every category of findings against
+  [`criteria.md`](criteria.md), produce the structured
+  package below. The subagent does NOT need to (and should
+  not try to) hit GitHub itself.
+- **Output schema** (exact, parseable by the parent):
+
+  ```text
+  HEAD_SHA: <head_oid>
+
+  ## Headline
+  PR #<N> — <title>
+    Author: <login> (<assoc>)
+    Base: <base> • Head: <head_short>
+    CI: <state> • Threads: <unresolved> unresolved • Reviews: <summary>
+    Files: <N> changed +<add> -<del>
+    Labels: <comma-list>
+
+  ## What it does
+  <one-paragraph plain-English summary>
+
+  ## Findings
+  <YAML list per the schema in review-flow.md Step 4, or "none">
+
+  ## Suggested disposition
+  APPROVE | REQUEST_CHANGES | COMMENT — <one-line reason>
+
+  ## Summary line
+  <the one-sentence summary for render --summary>
+  ```
+
+- **Forbidden** — the subagent may NOT:
+  - call `gh pr review`, `gh pr merge`, `gh pr edit`,
+    `gh pr comment`, `gh issue comment`, or any GitHub
+    write-mutation command;
+  - call any `mcp__github__*` `create_*` / `update_*` /
+    `add_*` / `merge_*` mutation;
+  - modify any file in the working tree (no `Write`,
+    `Edit`, no `git` write commands);
+  - invoke other Agents (no nested subagents);
+  - **write a review body.** The parent composes the body with
+    `code-review render` from the subagent's findings and summary
+    line, so the footer and the handle escaping are never left to
+    a subagent's paraphrase.
+
+  Posting is reserved to the parent skill, gated by maintainer
+  confirmation. Subagents are pure read-and-think workers.
+
+- **Skip-reason short-circuits** — if the subagent's first
+  fetch shows the PR is closed, merged, drafted, or authored
+  by `<viewer>`, it returns `SKIP: <reason>` instead of a
+  package. The parent skill skips the PR with that reason
+  attached to the session summary.
+
+## Folding subagent output into Step 1
+
+When the maintainer reaches a prefetched PR, the parent skill:
+
+1. **Compares head SHA** between the subagent's snapshot and
+   the live PR. If different, the analysis is stale — by
+   default re-fire a fresh subagent before showing anything.
+   The maintainer can opt to see the stale draft anyway via
+   `[P]ost-anyway` after the parent surfaces the SHA delta.
+2. **Renders the package**: runs `context` (cheap — the reads
+   are saved), `disposition` and `render` over the subagent's
+   findings, and shows the Step 1-through-7 block without
+   re-reading the diff.
+3. **Holds at Step 7's confirmation gate** identically to the
+   no-prefetch path. The only thing different is the source
+   of the draft; the maintainer's interaction is unchanged.
+
+## Wasted prefetch — accept it
+
+If the maintainer `[S]kip`s a prefetched PR, the subagent run
+was wasted. That's acceptable — the cost of an unused
+subagent is small compared to the wall-clock savings on the
+cases where the maintainer engages. Don't try to be clever
+about "will the maintainer skip this one?" — just keep the
+window full.
+
+## Concurrency cap
+
+Don't run more than `K` subagents at once (default `K=3`).
+Each subagent issues 2–3 `gh` calls and reads ~5 source
+files; `K=3` keeps GitHub-API and file-IO load well under
+the maintainer's hourly quota while giving instant headlines
+on the next 3 PRs.
+
+If the maintainer's queue is very small (`max:1`–`max:2`),
+the wall-clock benefit is nil and the cost of lookahead is
+pure waste — pass `no-prefetch` to disable. The skill auto-
+disables prefetch when only one PR remains.
+
+## Disagreeing with the subagent
+
+The subagent's draft is **advisory**. The parent skill — and
+the maintainer — are free to reject or rewrite it before
+posting. If the parent agent reading the package thinks the
+subagent missed something material, raise it explicitly to
+the maintainer at Step 6 ("subagent suggested APPROVE; I'd
+downgrade to COMMENT because…") rather than silently
+overriding. Disagreements between the two layers are signal
+the maintainer should see, not noise to be smoothed over.
+
+---

plugins/magpie-pr-management/skills/code-review/budget-discipline.md

@@ -3,22 +3,10 @@
 
 ## Budget discipline
 
-This skill's practical GraphQL / `gh` budget per PR is:
-
-- 1 query for the working PR list (one-shot, at session start)
-- 1 `gh pr view --json body,reviewRequests,reviews,statusCheckRollup,commits,labels,...` per PR
-- 1 `gh pr diff` per PR
-- 0–1 calls into the adversarial reviewer (out-of-band, not
-  GitHub API)
-- 0–1 `CODEOWNERS` read + 0–3 `commits?path=` reads for the
-  Step 4.5 reviewer suggestions (the PR-review-history source is
-  reached only when the cheap sources fall short)
-- 1 `gh pr review` mutation per posted review
-
-That's ~3 GitHub calls per PR plus one optional plugin call.
-A normal review pass (5–10 PRs) stays well under 100 GitHub-API
-points — a tiny fraction of the maintainer's 5000/h budget. If a
-session starts approaching the limit, the skill is
-mis-batching (most likely: re-fetching the diff after every
-finding instead of caching it locally) — stop and fix the call
-pattern, do not work around it with rate-limit sleeps.
+Per session: one paginated sweep of open PRs (`gql-cr-open`) and the
+once-per-session reads `queue` asks for (the ownership file, team rosters,
+the viewer's recent commits). Per reviewed PR: the full read (`gql-cr-pr`),
+the diff, 0–3 path-history reads for reviewer suggestions, the liveness read
+before posting, and one post — `session record --calls` tallies them and
+flags a session past ~100 calls. That is mis-batching (most likely a
+re-fetch per finding): stop and fix the call pattern; never sleep and retry.

plugins/magpie-pr-management/skills/code-review/classifications/backport.md

@@ -0,0 +1,31 @@
+<!-- SPDX-License-Identifier: Apache-2.0
+     https://www.apache.org/licenses/LICENSE-2.0 -->
+
+# Backport base branch
+
+[`code-review`](../../../../../tools/pr-management/README.md#code-review--pr-management-code-review) `context` matched the PR's base against the backport pattern in `<project-config>/pr-management-code-review-criteria.md`. Note the base in the headline.
+
+If the adopter's
+`<project-config>/pr-management-code-review-criteria.md` declares
+a backport branch pattern (see its `Backports / version-specific
+PRs` table), PRs whose base branch matches the pattern are
+treated as backports of already-merged `main` work and get a
+lighter-touch calibration:
+
+- **Diff parity**: does this match what was merged on `main`?
+- **Cherry-pick conflicts**: did the resolution introduce new
+  changes that need scrutiny?
+- **API/migration version markers**: backports should not
+  introduce new version bumps in any
+  versioning-sensitive subsystem the adopter calls out; if they
+  do, cite the relevant `API correctness` anchor from the
+  adopter's `Section anchors` table.
+
+For these PRs, prefer `COMMENT` over `REQUEST_CHANGES` unless
+the cherry-pick has clearly drifted from the `main` change.
+
+If the adopter config has no backport pattern declared, this
+section is a no-op and every PR is reviewed under the same
+calibration.
+
+---

plugins/magpie-pr-management/skills/code-review/classifications/disposition-approve.md

@@ -0,0 +1,8 @@
+<!-- SPDX-License-Identifier: Apache-2.0
+     https://www.apache.org/licenses/LICENSE-2.0 -->
+
+# Disposition `APPROVE`
+
+[`code-review`](../../../../../tools/pr-management/README.md#code-review--pr-management-code-review) `disposition` picks it only when every gate holds: rollup `SUCCESS` with real CI (Golden rule 8), no unresolved review thread, no other maintainer's standing `CHANGES_REQUESTED`, no unanswered maintainer question (Golden rule 7), and no finding above `nit`.
+Summary line: specific, never boilerplate — *"LGTM — clean N+1 fix with regression test, CI green."*
+The maintainer may still downgrade it; an upgrade to `APPROVE` past a gate the tool named needs the maintainer to say why.

plugins/magpie-pr-management/skills/code-review/classifications/disposition-comment.md

@@ -0,0 +1,7 @@
+<!-- SPDX-License-Identifier: Apache-2.0
+     https://www.apache.org/licenses/LICENSE-2.0 -->
+
+# Disposition `COMMENT`
+
+Everything that is neither: findings above `nit` without a gating count, CI pending or failing for reasons outside the diff, threads open, another maintainer's concern standing, or observations the maintainer wants to leave without gating the merge.
+`approval_gates` in the result names exactly what kept `APPROVE` off the table — cite it in the summary: *"Approach looks reasonable; a few observations inline that I'd like resolved before merging — none blocking."*

plugins/magpie-pr-management/skills/code-review/classifications/disposition-request-changes.md

@@ -0,0 +1,8 @@
+<!-- SPDX-License-Identifier: Apache-2.0
+     https://www.apache.org/licenses/LICENSE-2.0 -->
+
+# Disposition `REQUEST_CHANGES`
+
+[`code-review`](../../../../../tools/pr-management/README.md#code-review--pr-management-code-review) `disposition` picks it for at least one `blocking` finding, two or more `major`, one `major` with an unanswered author question (`--unanswered-question`), or a CI failure you judged diff-caused (`--ci-diff-caused`).
+Summary line names the reason: *"Found 1 blocking issue (potential SQL injection in `where` clause) that needs to land before this can merge."*
+On a backport base, prefer `COMMENT` unless the cherry-pick has clearly drifted ([`backport.md`](backport.md)).

plugins/magpie-pr-management/skills/code-review/classifications/footer-approve.md

@@ -0,0 +1,7 @@
+<!-- SPDX-License-Identifier: Apache-2.0
+     https://www.apache.org/licenses/LICENSE-2.0 -->
+
+# Footer — approve
+
+Used for `APPROVE`. `render` appends it verbatim from the tool's templates, with only `<PROJECT>` and the contributing-docs URL substituted (no URL configured: the last two lines are dropped rather than linking to a guess), and `verify_footer` confirms it before the post command is offered.
+Never paraphrase or shorten it; per-PR edits must not drop it (Golden rule 5). `APPROVE` and `REQUEST_CHANGES` always carry the maintainer-confirmed wording because GitHub refuses them without write access; `COMMENT` has no such gate, so its wording follows the permission read.

plugins/magpie-pr-management/skills/code-review/classifications/footer-comment-maintainer.md

@@ -0,0 +1,7 @@
+<!-- SPDX-License-Identifier: Apache-2.0
+     https://www.apache.org/licenses/LICENSE-2.0 -->
+
+# Footer — comment maintainer
+
+Used for `COMMENT` from an account with confirmed write access (`admin` / `maintain` / `write`). `render` appends it verbatim from the tool's templates, with only `<PROJECT>` and the contributing-docs URL substituted (no URL configured: the last two lines are dropped rather than linking to a guess), and `verify_footer` confirms it before the post command is offered.
+Never paraphrase or shorten it; per-PR edits must not drop it (Golden rule 5). `APPROVE` and `REQUEST_CHANGES` always carry the maintainer-confirmed wording because GitHub refuses them without write access; `COMMENT` has no such gate, so its wording follows the permission read.

plugins/magpie-pr-management/skills/code-review/classifications/footer-comment-role-neutral.md

@@ -0,0 +1,7 @@
+<!-- SPDX-License-Identifier: Apache-2.0
+     https://www.apache.org/licenses/LICENSE-2.0 -->
+
+# Footer — comment role neutral
+
+Used for `COMMENT` from an account without confirmed write access (`triage`, `read`, none). `render` appends it verbatim from the tool's templates, with only `<PROJECT>` and the contributing-docs URL substituted (no URL configured: the last two lines are dropped rather than linking to a guess), and `verify_footer` confirms it before the post command is offered.
+Never paraphrase or shorten it; per-PR edits must not drop it (Golden rule 5). `APPROVE` and `REQUEST_CHANGES` always carry the maintainer-confirmed wording because GitHub refuses them without write access; `COMMENT` has no such gate, so its wording follows the permission read.

plugins/magpie-pr-management/skills/code-review/classifications/footer-request-changes.md

@@ -0,0 +1,7 @@
+<!-- SPDX-License-Identifier: Apache-2.0
+     https://www.apache.org/licenses/LICENSE-2.0 -->
+
+# Footer — request changes
+
+Used for `REQUEST_CHANGES`. `render` appends it verbatim from the tool's templates, with only `<PROJECT>` and the contributing-docs URL substituted (no URL configured: the last two lines are dropped rather than linking to a guess), and `verify_footer` confirms it before the post command is offered.
+Never paraphrase or shorten it; per-PR edits must not drop it (Golden rule 5). `APPROVE` and `REQUEST_CHANGES` always carry the maintainer-confirmed wording because GitHub refuses them without write access; `COMMENT` has no such gate, so its wording follows the permission read.

plugins/magpie-pr-management/skills/code-review/classifications/security-disclosure.md

@@ -0,0 +1,25 @@
+<!-- SPDX-License-Identifier: Apache-2.0
+     https://www.apache.org/licenses/LICENSE-2.0 -->
+
+# Possible undisclosed security fix
+
+[`code-review`](../../../../../tools/pr-management/README.md#code-review--pr-management-code-review) `context` matched security-nature language (`security_disclosure.matches`: location, matched text, context) in the title, body or commit messages.
+ASF policy (`https://www.apache.org/security/committers.html`) requires that no reference to the security nature of a commit appear in public-facing content until the vulnerability is formally announced:
+
+> *"Messages associated with any commits should not make any reference to the security nature of the commit."*
+
+Surface the pre-review warning before Step 4 and wait for an explicit acknowledgment:
+
+> ⚠ **Possible undisclosed security fix detected**
+>
+> The PR title / body / commit messages contain language that may indicate a security vulnerability fix:
+>
+> - [each match: its location and the quoted context]
+>
+> ASF policy requires that no reference to the security nature of a commit appear in public-facing content until the CVE is formally announced. See `https://www.apache.org/security/committers.html`.
+>
+> **Before merging:** verify that the CVE disclosure process for this fix is complete (CVE status `READY`, public announcement sent to the standard destinations). If disclosure is not yet complete, this PR should be closed and the fix applied through the private security channel instead.
+>
+> *Acknowledge and continue review? `[Y]es` / `[Q]uit`.*
+
+Include the warning as the leading note of the review body whatever the disposition (`render --security-note <file>`): the contributor needs to see it even on `APPROVE`.

plugins/magpie-pr-management/skills/code-review/classifications/selector-area.md

@@ -0,0 +1,8 @@
+<!-- SPDX-License-Identifier: Apache-2.0
+     https://www.apache.org/licenses/LICENSE-2.0 -->
+
+# Selector `area:<LBL>` — filter by area label
+
+[`code-review`](../../../../../tools/pr-management/README.md#code-review--pr-management-code-review) keeps PRs carrying the label. Literal labels (`area:scheduler`) and prefix wildcards (`area:provider*`, `provider:amazon` — some projects use `provider:` instead of `area:`) both work; the wildcard is matched after the fetch, since GitHub search does not expand label wildcards.
+
+It filters the union, not each signal: `area:scheduler` drops any PR — review-requested or touching-mine — without that label. To keep only review-requested PRs in the area, add `requested-only`. Combine with `ready` for the area filter without the my-reviews signals.

plugins/magpie-pr-management/skills/code-review/classifications/selector-ready.md

@@ -0,0 +1,7 @@
+<!-- SPDX-License-Identifier: Apache-2.0
+     https://www.apache.org/licenses/LICENSE-2.0 -->
+
+# Selector `ready` — the curated triage queue
+
+[`code-review`](../../../../../tools/pr-management/README.md#code-review--pr-management-code-review) sources PRs carrying the configured `ready for maintainer review` label, whoever is on the request list — the broader pool triage already deemed ready.
+Usually combined with `area:<LBL>`; without it the pool is often too broad for one sitting.