Use this skill to run a multi-persona expert advisory review on a labelled pull request in microsoft/apm. The panel fans out to five mandatory specialists plus a test-coverage specialist (active on every PR that touches src/) plus two conditional specialists (auth, doc-writer), all running in their own agent threads, and a CEO synthesizer. The orchestrator is the sole writer to the PR: ONE recommendation comment, no verdict labels, no merge gating. The panel is advisory -- it surfaces finding...
Scanned 5/27/2026
Install via CLI
Skills Directoryopenskills install microsoft/apm---
name: apm-review-panel
description: >-
Use this skill to run a multi-persona expert advisory review on a labelled
pull request in microsoft/apm. The panel fans out to five mandatory
specialists plus a test-coverage specialist (active on every PR that
touches src/) plus two conditional specialists (auth, doc-writer),
all running in their own agent threads, and a CEO
synthesizer. The orchestrator is the sole writer to the PR: ONE
recommendation comment, no verdict labels, no merge gating. The panel
is advisory -- it surfaces findings, prioritizes follow-ups, and renders
a ship-recommendation that the maintainer and author weigh. Activate
when a non-trivial PR needs a cross-cutting recommendation
(architecture, CLI logging, DevX UX, supply-chain security,
growth/positioning, optionally auth, docs, and test coverage, with CEO
arbitration).
---
# APM Review Panel - 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.
## 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.
## Agent roster
| Agent | Role | Always active? |
|-------|------|----------------|
| [Python Architect](../../agents/python-architect.agent.md) | Architectural Reviewer + supplies mermaid diagrams | Yes |
| [CLI Logging Expert](../../agents/cli-logging-expert.agent.md) | Output UX Reviewer | Yes |
| [DevX UX Expert](../../agents/devx-ux-expert.agent.md) | Package-Manager UX | Yes |
| [Supply Chain Security Expert](../../agents/supply-chain-security-expert.agent.md) | Threat-Model Reviewer | Yes |
| [OSS Growth Hacker](../../agents/oss-growth-hacker.agent.md) | Adoption Strategist | Yes |
| [Auth Expert](../../agents/auth-expert.agent.md) | Auth / Token Reviewer | Conditional (see below) |
| [Doc Writer](../../agents/doc-writer.agent.md) | Documentation Reviewer | Conditional (see below) |
| [Test Coverage Expert](../../agents/test-coverage-expert.agent.md) | Test-Presence Reviewer (paired with DevX UX) | Yes (skipped only on docs-only PRs -- see below) |
| [APM CEO](../../agents/apm-ceo.agent.md) | Strategic Arbiter / Synthesizer | Yes |
## Topology
```
apm-review-panel SKILL (orchestrator thread)
|
FAN-OUT via task tool (panelists in parallel)
|
+-----+-------+-------+-----+-----+------+-----------+----------+
v v v v v v v v v (cond.)
py cli dx-ux sec grw auth doc-writer test-cov
| | | | | | | |
| each returns JSON per panelist-return-schema.json
+-----+-------+-------+-----+-----+------+-----------+----------+
|
v <-- S4 schema-validate
v <-- on malformed: re-spawn that persona
v
task: apm-ceo synthesizer
- aggregates findings across panelists
- resolves dissent
- emits headline + arbitration prose + principle alignment
- emits curated recommended_followups (prioritized)
- emits ship_recommendation (stance + prose)
- returns ceo-return-schema.json
|
v <-- S4 schema-validate
v
orchestrator (sole writer)
| |
v v
add-comment remove-labels
(max:2) [panel-review,
panel-approved,
panel-rejected]
(trigger reset +
legacy verdict sweep)
```
## Conditional panelists
Two personas are conditional (auth, doc-writer). A third
(test-coverage) is mandatory on every PR that touches `src/` and only
skipped on documentation-only PRs -- see its section below for why.
The orchestrator ALWAYS spawns ALL three tasks to keep the schema
return shape uniform; the prompt instructs the subagent to set
`active: false` with an `inactive_reason` if the condition does not
hold.
### 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 YES."
### 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 YES."
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.
### Test Coverage Expert
**Active by default on every PR that touches `src/**/*.py`.** The only
condition that flips this persona to `active: false` is a
documentation-only PR -- the diff contains zero `src/**/*.py` files.
In that case set `inactive_reason: "documentation-only PR -- no
runtime code paths to defend"`.
The activation rule is intentionally narrow: under the advisory regime,
test outcomes are LOAD-BEARING for CEO arbitration (passed / failed /
missing test evidence outranks opinion-only findings -- see
`apm-ceo.agent.md` and `panelist-return-schema.json` evidence block).
A persona whose findings carry that weight cannot be silently skipped
on a heuristic. Better to spawn it on a pure refactor and have it
return a single `nit`-severity "no behavior surface touched -- no
coverage finding" line than to skip it and leave the CEO without
evidence to weigh. (Earlier revisions of this skill paired test-coverage
with auth and doc-writer as conditional for symmetry; that symmetry
broke when test evidence became load-bearing.)
The test-coverage-expert is paired with the devx-ux-expert lens and
defends the user-promise contracts the DevX persona enumerates (CLI
surface, error wording, install idempotency, lockfile determinism, auth
resolution). It MUST verify "no test exists" claims with `view`/`grep`
on the test tree before emitting a finding -- false-positive coverage
findings destroy trust in the field. It does NOT compute coverage
percentages, does NOT flag tests for pure refactors, and does NOT
duplicate python-architect on test-code design.
## Routing matrix (CEO synthesis emphasis only)
These routes describe WHICH specialist's findings the CEO weights more
heavily for a given PR type. They do NOT change which personas run --
every mandatory persona always runs. Routing is a CEO synthesis hint.
- **Architecture-heavy PR** -> CEO weights Python Architect on
abstraction calls; CLI Logging on consistency.
- **CLI UX PR** -> CEO weights DevX UX on command surface; CLI Logging
on output paths; Growth Hacker on first-run conversion.
- **Security PR** -> CEO biases toward Supply Chain Security on default
behavior; DevX UX flags ergonomics regression from any mitigation.
- **Auth PR** (auth-expert active) -> CEO weights Auth Expert on
AuthResolver / token precedence; Supply Chain on token-scoping.
- **Docs / release / comms PR** (doc-writer active) -> CEO weights Doc
Writer on accuracy and voice; Growth Hacker on hook and story angle.
- **Behavior-change PR** (test-coverage active) -> CEO weights Test
Coverage Expert on regression-trap presence; DevX UX on which user
promises the change touches. A blocking-severity coverage finding on
a critical-promise surface (auth, lockfile, install, marketplace,
hooks) is the highest signal in this routing.
- **Full panel** (default) -> CEO synthesizes equally; calls out any
dissent in `dissent_notes`.
## Execution checklist
Work through these steps in order. Do not skip ahead. Do not emit any
output to the PR before step 6.
1. **Read PR context** (the orchestrating workflow already fetched it
via `gh pr view` / `gh pr diff`). Identify changed files for the
conditional panelist routing decisions (auth-expert and doc-writer).
2. **Resolve the conditional panelists** using the rules above. Decide
for EACH conditional persona: spawn active OR spawn with
`active: false` + an `inactive_reason`. Either way, all three
conditional personas ARE spawned -- the schema requires uniform
return shape.
3. **Fan out panelist tasks.** Spawn the following tasks in PARALLEL
via the `task` tool, one task per persona:
- `python-architect` (also asked to supply `extras.diagrams`:
`class_diagram` (mermaid `classDiagram`), `component` (mermaid
`flowchart TD`), and OPTIONAL `sequence` (mermaid
`sequenceDiagram`) blocks per the persona's section 1/2/3 contract)
- `cli-logging-expert`
- `devx-ux-expert`
- `supply-chain-security-expert`
- `oss-growth-hacker`
- `auth-expert` (always - active per step 2)
- `doc-writer` (always - active per step 2)
- `test-coverage-expert` (always - active per step 2)
Each task prompt MUST:
- Reference its persona file by relative path so the subagent loads
its own scope, lens, and anti-patterns.
- Include the PR number, title, body, and diff (passed inline).
- Cite `assets/panelist-return-schema.json` and require the subagent
to emit JSON matching that schema as its FINAL message.
- State the calibrated severity contract: "Use `severity: blocking`
ONLY for correctness regressions, security/auth bypasses, or
architectural faults that compound, with explicit rationale.
Default substantive feedback to `recommended`. Use `nit` for
one-line polish. The panel is advisory; nothing you return blocks
merge -- pick the severity that honestly matches your signal
strength."
- Restate the output contract: NO `gh` write commands, NO posting
comments, NO label changes, NO touching PR state. JSON return only.
4. **S4 schema gate.** When each panelist task returns, parse the JSON
and validate against `assets/panelist-return-schema.json`. On
validation failure:
- Re-spawn that ONE panelist with an explicit error message pointing
at the violated rule.
- Maximum two re-spawn attempts per panelist. If still malformed,
synthesize a placeholder
`{persona: "<slug>", active: true, summary: "Schema failure -- see
extras.", findings: [], extras: {schema_failure: "<reason>"}}`
and surface the failure in the CEO arbitration prompt.
5. **Spawn the CEO synthesizer task.** Pass the full set of validated
panelist JSON returns to a `task` invocation that loads
`../../agents/apm-ceo.agent.md`. The prompt MUST:
- Provide all panelist returns as structured input.
- Ask for: headline, arbitration prose, principle alignment (only
applicable principles), curated recommended_followups (prioritized
by signal, NOT a re-listing of every finding), ship_recommendation
(stance + prose).
- Cite `assets/ceo-return-schema.json` and require JSON return.
- Restate the contract: the panel is advisory. The CEO does NOT pick
a verdict label. The `ship_recommendation.stance` is prose for the
human reviewer, not a gate. NO `gh` write commands.
Validate the CEO return against `assets/ceo-return-schema.json`. On
failure, re-spawn once with the violation cited.
6. **Resolve the notification audience.** The advisory comment must
surface in the inboxes of the people who will act on it. Run:
```
gh pr view <PR_NUMBER> --json author,reviewRequests
```
Build `notify_audience` as the deduplicated list:
- the PR author's `@login` (always included);
- every requested reviewer's `@login` (these are the
CODEOWNERS-resolved reviewers GitHub auto-requested for the
touched paths, plus any explicitly-requested human reviewers);
- every requested team's `@org/team-slug` (CODEOWNERS team
entries).
Filter out:
- bot logins (login ending in `[bot]` or matching
`dependabot|github-actions|copilot-pull-request-reviewer`);
- the orchestrator's own identity (avoid self-ping).
Cap the final list at 6 handles to avoid notification noise (PR
author + up to 5 reviewers/teams). If the cap trims, prefer team
handles over individual logins. Pass the resulting list to the
template renderer as `notify_audience`.
This step replaces the maintainer-notification signal that the
pre-advisory verdict labels carried. It is the only mechanism by
which a fresh panel pass announces itself.
7. **Render the comment.** Load `assets/recommendation-template.md`,
fill the placeholders from the panelist + CEO JSON, and emit it as
exactly ONE comment.
Filling rules:
- The per-persona summary table renders ONLY active panelists, one
row per persona, with finding counts by severity and the persona's
`summary` field.
- The mermaid diagrams come from `python-architect.extras.diagrams`.
If absent, render the placeholder lines from the template (do NOT
invent diagrams).
- The recommended follow-ups list renders the CEO's curated subset,
not every finding. Full per-persona findings collapse at the bottom.
- NEVER render the words "Verdict", "APPROVE", "REJECT", "blocked",
"merge gate", or any equivalent. The panel is advisory.
8. **Sweep labels** via `safe-outputs.remove-labels`. The list MUST be
`[panel-review, panel-approved, panel-rejected]` -- always all three,
regardless of which are currently on the PR. `panel-review` is the
re-run idempotency reset; the other two are LEGACY VERDICT LABELS
from the pre-advisory regime that have no meaning under the advisory
contract and would mislead readers if left on a freshly-reviewed PR.
`safe-outputs.remove-labels` is idempotent on missing labels, so
sweeping all three on every run is safe and self-healing. NO
verdict labels are applied.
## Output contract (non-negotiable)
- Exactly ONE comment per panel run, rendered from
`assets/recommendation-template.md`. The `safe-outputs.add-comment.max:
2` is a fail-soft ceiling; the discipline lives here.
- Exactly ONE `remove-labels` call sweeping
`[panel-review, panel-approved, panel-rejected]`.
- NO `add-labels` call. The advisory regime has no verdict to encode.
- Subagents (panelists + CEO) NEVER write to PR state, NEVER call `gh
pr comment`, NEVER call `gh pr edit --add-label`. They return JSON.
The orchestrator is the sole writer.
- Never invent new top-level template sections or drop existing ones.
## Gotchas
- **Roster invariant.** The frontmatter description, the roster table,
the conditional rules, the recommendation template, and the JSON
schema MUST agree on the persona set. If you change one, change all
in the same edit.
- **Calibrated severity discipline.** The advisory regime relies on
panelists honestly distinguishing `blocking` from `recommended`. If a
panelist marks everything `blocking`, the comment becomes noisy and
the maintainer learns to ignore the field. The panelist prompts state
the contract explicitly; the CEO arbitration prose is the safety
valve when a panelist over-flags.
- **Mermaid diagrams are template-required.** The python-architect
persona is asked to supply `extras.diagrams.class_diagram`,
`extras.diagrams.component`, and the OPTIONAL
`extras.diagrams.sequence`. The template renders nothing when they
are missing -- it does NOT invent diagrams. Real diagrams are
what makes the comment scannable for the human reviewer.
- **Mermaid `classDiagram` `:::cssClass` shorthand gotcha.** GitHub's
mermaid renderer rejects `:::cssClass` appended to relationship
lines (e.g. `A *-- B:::touched`); use standalone
`class Name:::cssClass` declarations instead. Authority:
`python-architect.agent.md:146-154`.
- **Doc-writer detects DRIFT, not just edits.** When the PR changes
user-facing code that SHOULD have updated docs but did not, doc-writer
surfaces that as a finding. The conditional rule above is necessary
but not sufficient -- doc-writer reasons about doc consistency given
the diff, not just whether doc files were touched.
- **False-negative auth gotcha.** Auth regressions can be introduced
from non-auth files that change the inputs to auth -- host
classification, dependency parsing, clone URL construction, HTTP
authorization headers, or call sites that bypass `AuthResolver`. If
a diff changes how a remote host, org, token source, or fallback path
is selected and you are not certain it is auth-neutral, activate
auth-expert as `active: true`.
- **Test-coverage probe is mandatory.** The test-coverage-expert MUST
verify "no test exists for X" via `view`/`grep` on the `tests/` tree
before emitting a finding. A false-positive coverage finding (test
exists but persona claimed it does not) destroys maintainer trust in
the field. The persona scope file enforces this; the orchestrator
passes the diff and trusts the persona to probe.
- **Subagent write enforcement is contract-based, not sandbox-based.**
Tool permissions are workflow-scoped, not subagent-scoped, so every
spawned task technically inherits the same `gh` toolset. The
"subagents must not write" rule is enforced by the prompt contract in
each `.agent.md` plus the `safe-outputs.add-comment.max: 2`
fail-soft. If a subagent ever tries to post a comment, the cap
catches it.
- **No verdict-label reset workflow.** The previous regime had a
companion workflow `pr-panel-label-reset.yml` that stripped verdict
labels on every push. The advisory regime has no verdict labels to
strip; that workflow is removed.
No comments yet. Be the first to comment!
