autopilot-pr-review-worker

# autopilot-pr-review-worker - Fan-Out Advisory Review The panel is FAN-OUT + SYNTHESIZER. Each persona runs in its own agent thread (via the `task` tool) and returns JSON matching `assets/panelist-return-schema.json`. The orchestrator schema-validates each return, hands all returns to the apm-ceo synthesizer (also a task thread, returns JSON matching `assets/ceo-return-schema.json`), then renders ONE recommendation comment from `assets/recommendation-template.md`. This skill is ADVISORY by design. It does not compute a binary verdict, it does not apply verdict labels, and it does not gate merge. The panel surfaces findings; the maintainer and the PR author decide ship. ## Activation card `activation_card: on`. Before any PR read or GitHub write, emit this Enter card with every field filled. Missing field -> stop. ```text skill: autopilot-pr-review-worker skill_path: <resolved directory of this SKILL.md> mode: run subject: microsoft/apm#<pr-number> path: review intent: advise one already-selected PR origin: unattended | actor-session write: on | off debug: off | on repo: microsoft/apm pr: <positive integer> invocation: agentic-workflow | actor-session invocation_mode: session-review | direct-user-review | composed-implementation-review panel-mode: full | lean | delta ``` Rules: - `write` defaults to `on` when the caller omitted it. - `write: off` returns the filled template only. Do not comment, add labels, remove labels, or request reviewers. - `write: on` posts the one advisory comment via `autopilot-comment` (`source_skill: autopilot-pr-review-worker`) and may clear `panel-review`. Never assign. Actor-session may request `@me` as a supplemental reviewer. Do not call `gh pr comment` or `safe-outputs.add-comment` yourself. - `panel-mode` defaults to `lean` when omitted or unknown. Omitted `panel-mode` is not a missing-field stop. Caller (merge-worker, scheduler, or human) SHOULD set it. Unknown fails cheap (`lean`), not heavy (`full`). - `debug` defaults to `off` when omitted or unknown. Omitted `debug` is not a missing-field stop. Pass it through to `autopilot-comment`. `debug: on` prefixes `[i] Skill debug is on.` The filled recommendation template is `public_body` (and `debug_body` when debug is on). - `origin` fail-closed unknown -> `unattended`. - Unattended never assigns and never requests reviewers. - One PR. Do not nest a scheduler path. After the panel, emit this Exit receipt: ```text skill: autopilot-pr-review-worker subject: microsoft/apm#<pr-number> path: review write: on | off posted: yes | no reviewer_requested: yes | no | skipped approved: n/a panel-mode: full | lean | delta personas_spawned: <comma-separated slugs> ``` ## Architecture invariants - **Advisory regime, not gate regime.** There is no `APPROVE` / `REJECT`, no `panel-approved` / `panel-rejected` label, no deterministic verdict computation. The CEO returns a `ship_recommendation.stance` (`ship_now` / `ship_with_followups` / `needs_discussion` / `needs_rework`); this is prose for the human reviewer, never auto-applied as a label or status check. This is the architectural fix for the previous regime's over-strictness: removing the binary gate removes the incentive for panelists to inflate `required[]` defensively. - **Three severity buckets, none of them gate.** Findings carry `severity: blocking | recommended | nit`. `blocking` is the highest signal a panelist can send and renders prominently in the comment; it still does not block merge. `recommended` is the default for substantive feedback. `nit` is one-line polish. The orchestrator never reads severity to gate anything. - **Single-writer interlock.** Only the orchestrator writes to the PR: exactly one `add-comment` and one `remove-labels` call. The `remove-labels` call always sweeps `panel-review` (trigger idempotency) AND defensively removes `panel-approved` / `panel-rejected` if present (legacy verdict labels from the pre-advisory regime; they have no meaning here and would mislead readers if left on a PR after a fresh advisory pass). NO `add-labels` call -- there are no verdict labels to apply. Panelist subagents and the CEO subagent return JSON only and MUST NOT call any `gh` write command, post comments, apply labels, or touch the PR state. - **Single-emission discipline.** Exactly one comment per panel run, rendered from `assets/recommendation-template.md` after all subagents return. - **Non-empty turn exit (the run's hard contract).** gh-aw decides success by inspecting `agent_output` AFTER your turn ends: a turn that ends with zero safe outputs (`agent_output = {"items":[]}`) is detected as a failure, the safe-output detection job is skipped, the `add-comment` job never runs, and the workflow opens a "No Safe Outputs Generated" issue. Therefore your turn MUST end with at least one safe output -- the rendered comment on success (step 7), or an explicit `noop` if the run genuinely cannot produce one. NEVER end the turn empty. - **Synchronous fan-out -- never spawn-and-forget.** Every `task` spawn (each panelist AND the CEO synthesizer) is BLOCKING: spawn it, WAIT for its JSON return, then continue. Use the `task` tool's synchronous mode; do NOT use its background/detached mode -- the variant that returns an `agent_id` immediately and runs the subagent in the background -- for any panelist or the CEO. Their returns are LOAD-BEARING: the comment cannot be rendered without them. Spawning the CEO (or a panelist) detached and then ending the turn while it is still running is the documented cause of the empty-output failure above. - **Invocation contract is explicit.** Resolve ORIGIN and INTENT before any GitHub write. They decide reviewer requests only. They never authorize assignee writes, never remove existing reviewers, and never override CODEOWNERS. - **CODEOWNERS is paramount.** GitHub's current `reviewRequests` and submitted reviews from CODEOWNERS-resolved users/teams are authoritative runtime ownership. Advice may request supplemental expertise. It must not replace, reorder, drop, or contradict those owners. A comment that contradicts CODEOWNERS is a failed emission. The CODEOWNERS last-comment gate decides whether the panel proceeds at all. ## Invocation contract Resolve ORIGIN and INTENT before gathering context. Future automations declare these axes; do not add a new harness by editing a name list. ORIGIN (who is running): - `unattended` -- GitHub Agentic Workflow / gh-aw / GitHub Actions / future scheduled automations with no session actor. Alias: `agentic-workflow`. - `actor-session` -- Direct user harness, Copilot App, Cloud Agent, Remote Agent, or any future session-backed runner. `@me` is that runner's GitHub identity (human or agent). INTENT (what this run is doing): - `review` -- standalone advisory review (this skill). - `implement` -- parent implementation owns assignment. COMPOSED is true when any implementation parent invokes this skill. Do not infer COMPOSED from parent skill names. | ORIGIN | INTENT | COMPOSED | Assignee | Reviewer request | |--------|--------|----------|----------|------------------| | `unattended` | any | any | Never | Never | | `actor-session` | `review` | false | Never | Request authenticated `@me` as a supplemental reviewer only | | `actor-session` | `review` | true | Never (parent owns assignment) | Never | | unknown | any | any | Never | Never | Caller tokens still accepted as `INVOCATION_MODE`: - `agentic-workflow` -> ORIGIN=`unattended` - `session-review` (alias `direct-user-review`) -> ORIGIN=`actor-session`, INTENT=`review` - `composed-implementation-review` -> ORIGIN=`actor-session`, INTENT=`review`, COMPOSED=true Resolution order: 1. Honor caller-supplied ORIGIN, INTENT, COMPOSED, or INVOCATION_MODE. 2. If `GH_AW_*` or `GITHUB_ACTIONS` is set, ORIGIN=`unattended`. 3. Else if `gh api user --jq .login` succeeds, ORIGIN=`actor-session`. 4. Else ORIGIN=`unattended` (fail closed: no ownership writes). Reviewer requests are additive. Never remove, replace, or reorder existing users or teams. Never convert a failed reviewer request into an assignee write. If `@me` is the PR author, GitHub cannot request a self-review. Record `self-review-red-flag` in working notes, leave reviewers unchanged, and continue the advisory. Do not stop. This skill never assigns issues or PRs in any mode. This skill owns the actor-session `@me` reviewer request (`gh pr edit --add-reviewer @me`) and the one recommendation comment. The PR review scheduler never comments, labels, assigns, or requests reviewers. ## Agent roster Load `assets/panel-mode.md` before fan-out. Spawn only personas that are active for this `panel-mode` and surface. Do not spawn `active: false` stubs. | Agent | Role | When spawned | |-------|------|----------------| | [Python Architect](.apm/agents/python-architect.agent.md) | Architectural Reviewer + mermaid in `full` | Core, unless docs/changelog-only. Optional in `lean` on tiny diffs. | | [CLI Logging Expert](.apm/agents/cli-logging-expert.agent.md) | Output UX Reviewer | Surface-gated | | [DevX UX Expert](.apm/agents/devx-ux-expert.agent.md) | Package-Manager UX | Surface-gated | | [Supply Chain Security Expert](.apm/agents/supply-chain-security-expert.agent.md) | Threat-Model Reviewer | Surface-gated | | [OSS Growth Hacker](.apm/agents/oss-growth-hacker.agent.md) | Adoption Strategist | Surface-gated | | [Auth Expert](.apm/agents/auth-expert.agent.md) | Auth / Token Reviewer | Surface-gated | | [Doc Writer](.apm/agents/doc-writer.agent.md) | Documentation Reviewer | Surface-gated | | [Test Coverage Expert](.apm/agents/test-coverage-expert.agent.md) | Test-Presence Reviewer | Core unless docs-only (`src/` untouched) | | [Performance Expert](.apm/agents/performance-expert.agent.md) | Package-Manager Performance Reviewer | Surface-gated | | [APM CEO](.apm/agents/apm-ceo.agent.md) | Strategic Arbiter / Synthesizer | Always | ## Topology ``` autopilot-pr-review-worker SKILL (orchestrator thread) | gather full PR context once -> shared brief packet | FAN-OUT via task (ONLY active personas for this panel-mode) | each returns JSON per panelist-return-schema.json | v <-- S4 schema-validate task: apm-ceo synthesizer (always) | v orchestrator (sole writer): one comment or noop ``` ## panel-mode Caller (merge-worker, scheduler, or human) SHOULD set `panel-mode: full | lean | delta`. Omitted or unknown -> `lean`. | Mode | Roster | Context | Comment | |------|--------|---------|---------| | full | Surface-gated specialists + CEO | Shared brief + owned files | yes | | lean | Highest-signal surface owner, test-coverage if `src/` touched, CEO. Architect optional on tiny diffs. | Shared brief only | yes | | delta | CEO + previously-active personas whose owned files changed since last panel head; test-coverage only if tests or `src/` changed in range | Brief + last comment + diff | yes if head or watermark changed; else noop | Even `full` is surface-gated. Never spawn inactive stubs. Lean cap: at most those three (plus architect when not tiny). Delta: no new persona that was inactive on the prior panel unless a new fast-path file appeared or the orchestrator writes an `adhoc_reason`. ## Shared brief Orchestrator gathers **once**. Children MUST NOT re-fetch the PR conversation or walk the repo "to be sure." Packet target < 8 KB. Shape: `assets/shared-brief.example.json`. Each panelist prompt MUST include: - JSON only. Do not run gh. Do not read files outside this packet unless a path is listed as owned by you. - Finding cap: 3. Nits allowed only in `full`. - Architect mermaid: `full` only. `lean` / `delta` omit diagrams. ## Models When spawning via `task`, pin class so a missing default cannot retry the whole roster: - high-capability: `apm-ceo`, `auth-expert`, `supply-chain-security-expert`, `python-architect` - cheap/fast: `test-coverage-expert`, `doc-writer`, `cli-logging-expert`, `devx-ux-expert`, `oss-growth-hacker`, `performance-expert` If a pinned model is unavailable: fall back **once** per persona to the other class, then stub that persona with `active: false` and `inactive_reason: model unavailable`. Do not relaunch the roster. This is the only allowed `active: false` spawn. ## Conditional panelists (surface-gated) Spawn the persona when the fast-path matches, or when the orchestrator decides the PR still needs that lens after reading the brief (ad-hoc add with a one-line `adhoc_reason`). Fast-path is the default include set, not the ceiling. Do not spawn a stub when the condition misses and you have no reason. Do not add personas "to be sure." ### Auth Expert Activate when the PR changes any of: - `src/apm_cli/core/auth.py` - `src/apm_cli/core/token_manager.py` - `src/apm_cli/core/azure_cli.py` - `src/apm_cli/deps/github_downloader.py` - `src/apm_cli/marketplace/client.py` - `src/apm_cli/utils/github_host.py` - `src/apm_cli/install/validation.py` - `src/apm_cli/install/pipeline.py` - `src/apm_cli/deps/registry_proxy.py` Fallback self-check (when no fast-path file matched): "Does this PR change authentication behavior, token management, credential resolution, host classification used by `AuthResolver`, git or HTTP authorization headers, or remote-host fallback semantics? If unsure, answer NO." ### Doc Writer Activate when the PR changes any of: - `README.md` - `CHANGELOG.md` - `MANIFESTO.md` - `docs/src/content/docs/**` - `.apm/skills/**/*.md` - `.apm/agents/**/*.md` - `.github/skills/**/*.md` - `.github/agents/**/*.md` - `.github/instructions/**/*.md` - `.github/workflows/*.md` (gh-aw natural-language workflows) - `packages/apm-guide/**` Fallback self-check (when no fast-path file matched): "Does this PR change user-facing documentation, agent or skill prose, instruction files, CHANGELOG entries, README claims, or any natural-language artifact a reader will rely on? If unsure, answer NO." When the doc-writer is active and the PR includes documentation changes, the persona reviews them for: (a) consistency with the existing voice and structure, (b) accuracy against the code being changed, (c) completeness for the typical reader (no orphan claims, no missing prerequisites), (d) discoverability (cross-links, sidebar order if Starlight content). When the doc-writer is active because of code changes that SHOULD have updated docs but did not, the persona surfaces that gap as a finding. ### Performance Expert Activate when the PR changes any of: - `src/apm_cli/cache/**` - `src/apm_cli/deps/**` - `src/apm_cli/install/phases/**` - `src/apm_cli/install/pipeline.py` - `src/apm_cli/install/resolve.py` - `src/apm_cli/utils/**` - `src/apm_cli/marketplace/**`