From 9bf2770ccf05db25eb6db549a92df217658f6b86 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Fri, 24 Apr 2026 17:50:03 +0200 Subject: [PATCH] fix(panel-review): orchestrator self-arbitrates and emits in skill contract The previous wording in #905 framed the CEO as a separate emitter and told the orchestrator to instruct each sub-agent 'do not post any comment'. This produced two regressions: 1. Sub-agents launched via the task tool do not have safe-outputs MCP access to begin with -- the no-op instruction added orchestration bloat without protecting anything. 2. The 'CEO synthesizes the single verdict' phrasing made the orchestrator wait for a CEO sub-agent that was never dispatched. Result: agent_output.json={"items":[]} -- zero comments posted. Fix lives in the skill (the authoritative source), not the workflow: - SKILL.md execution checklist step 5: orchestrator runs CEO arbitration as itself, never delegates to a sub-agent. - SKILL.md execution checklist step 7: orchestrator writes the comment to safe-outputs.add-comment, never calls GitHub API. - SKILL.md output contract: cap reference updated to fail-soft ceiling of 7 (matches workflow frontmatter). Workflow .md is now a thin shell: gather PR context, then defer to the skill for routing, dispatch, arbitration, emission. No more duplicated orchestration logic at the workflow boundary. Closes #906 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .apm/skills/apm-review-panel/SKILL.md | 15 ++++++++---- .github/skills/apm-review-panel/SKILL.md | 15 ++++++++---- .github/workflows/pr-review-panel.md | 29 +++--------------------- 3 files changed, 23 insertions(+), 36 deletions(-) diff --git a/.apm/skills/apm-review-panel/SKILL.md b/.apm/skills/apm-review-panel/SKILL.md index 754726b5a..4b693a548 100644 --- a/.apm/skills/apm-review-panel/SKILL.md +++ b/.apm/skills/apm-review-panel/SKILL.md @@ -184,12 +184,16 @@ the final step. - No persona return is missing or empty. If any check fails, re-invoke the missing persona and repeat the gate. Do not proceed to step 5 until the gate passes. -5. Run the CEO arbitration pass over the collected findings. CEO - arbitration may run only after the completeness gate has passed. +5. Run the CEO arbitration pass over the collected findings **as + yourself** (the orchestrator). Do NOT dispatch a separate sub-agent + for arbitration -- you are the arbiter. CEO arbitration may run + only after the completeness gate has passed. 6. Now (and only now) load `assets/verdict-template.md` and fill it in with the collected findings + arbitration. 7. Emit the filled template as exactly ONE comment via the workflow's - `safe-outputs.add-comment` channel. + `safe-outputs.add-comment` channel. You (the orchestrator) write + the comment; never delegate emission to a sub-agent and never call + the GitHub API directly. ### Dispatch contract @@ -217,8 +221,9 @@ panel review that lands as one cohesive verdict and one that fragments into per-persona noise. - Produce **exactly one** comment per panel run. The - `safe-outputs.add-comment.max: 1` cap in the workflow is a backstop; - the discipline lives here. + `safe-outputs.add-comment.max` cap in the workflow is a fail-soft + ceiling (currently 7, one per persona slot); the one-comment + discipline lives here. - Use `assets/verdict-template.md` as the comment body. Keep its section headings exactly as written. Adapt the body of each section to the PR. Do not invent new top-level sections or drop existing diff --git a/.github/skills/apm-review-panel/SKILL.md b/.github/skills/apm-review-panel/SKILL.md index 754726b5a..4b693a548 100644 --- a/.github/skills/apm-review-panel/SKILL.md +++ b/.github/skills/apm-review-panel/SKILL.md @@ -184,12 +184,16 @@ the final step. - No persona return is missing or empty. If any check fails, re-invoke the missing persona and repeat the gate. Do not proceed to step 5 until the gate passes. -5. Run the CEO arbitration pass over the collected findings. CEO - arbitration may run only after the completeness gate has passed. +5. Run the CEO arbitration pass over the collected findings **as + yourself** (the orchestrator). Do NOT dispatch a separate sub-agent + for arbitration -- you are the arbiter. CEO arbitration may run + only after the completeness gate has passed. 6. Now (and only now) load `assets/verdict-template.md` and fill it in with the collected findings + arbitration. 7. Emit the filled template as exactly ONE comment via the workflow's - `safe-outputs.add-comment` channel. + `safe-outputs.add-comment` channel. You (the orchestrator) write + the comment; never delegate emission to a sub-agent and never call + the GitHub API directly. ### Dispatch contract @@ -217,8 +221,9 @@ panel review that lands as one cohesive verdict and one that fragments into per-persona noise. - Produce **exactly one** comment per panel run. The - `safe-outputs.add-comment.max: 1` cap in the workflow is a backstop; - the discipline lives here. + `safe-outputs.add-comment.max` cap in the workflow is a fail-soft + ceiling (currently 7, one per persona slot); the one-comment + discipline lives here. - Use `assets/verdict-template.md` as the comment body. Keep its section headings exactly as written. Adapt the body of each section to the PR. Do not invent new top-level sections or drop existing diff --git a/.github/workflows/pr-review-panel.md b/.github/workflows/pr-review-panel.md index 9f61833bd..c5f41319b 100644 --- a/.github/workflows/pr-review-panel.md +++ b/.github/workflows/pr-review-panel.md @@ -117,29 +117,6 @@ gh pr diff "$PR" Load the **apm-review-panel** skill and follow its execution checklist and output contract exactly. The skill owns reviewer routing, persona dispatch, the Auth Expert conditional rule, the pre-arbitration -completeness gate, template loading, and verdict shape. - -Auth Expert is the only conditional panelist. The skill decides -activation from the already-fetched PR title/body/files/diff using a -fast-path file list plus a fallback self-check. Do not invent a -separate scope-analysis sub-agent for this. If the skill marks Auth -Expert inactive, do not dispatch it; keep the Auth Expert heading in -the final verdict and fill it with `Not activated -- `. - -## Step 3: Output contract - -- You may post **exactly one** comment for this entire panel run, and it - **must** be the final synthesized verdict from the **CEO** (after - arbitration). Sub-agent personas (Python Architect, CLI Logging - Expert, DevX UX Expert, Supply Chain Security Expert, OSS Growth - Hacker, Auth Expert when active) **do not** post comments — they - return their findings to the CEO, who synthesizes the single verdict. - When dispatching each sub-agent, instruct it explicitly: "do not post - any comment; return your findings to the orchestrator." -- Do not call the GitHub API directly. Write the comment via the - provided output channel; a downstream publisher posts it. - -## Step 4: Emit the verdict - -Write the CEO's final verdict comment body to the agent output channel. -The downstream publisher will post it to PR #$PR. +completeness gate, CEO arbitration, template loading, verdict shape, +and the one-comment emission contract -- including writing the final +comment to `safe-outputs.add-comment` rather than the GitHub API.