Skip to content

fix(apm-review-panel): restore in-context persona model (per agentskills.io)#908

Merged
danielmeppiel merged 1 commit intomainfrom
fix/panel-review-restore-in-context-model
Apr 24, 2026
Merged

fix(apm-review-panel): restore in-context persona model (per agentskills.io)#908
danielmeppiel merged 1 commit intomainfrom
fix/panel-review-restore-in-context-model

Conversation

@danielmeppiel
Copy link
Copy Markdown
Collaborator

Problem

PR #907 added a 'do not delegate' clause for emission, but the panel still fails. Run 24899623301: 2 personas (Growth Hacker, Supply Chain) posted their findings as separate comments via safeoutputs-add_comment, no synthesized CEO verdict. Earlier run 24897368527: zero comments.

Root cause

The user noted: the panel was working way better before #882. Confirmed via git history.

PR #882 changed the skill from a clean progressive-disclosure model into a sub-agent dispatch model:

  • 'Do not open linked persona files in the orchestrator thread.'
  • 'Dispatch one sub-agent per persona via the task tool.'

This breaks two ways:

  1. Sub-agents launched via the task tool inherit the parent's MCP gateway -- including safeoutputs. They will happily call safeoutputs-add_comment themselves. The 'instruct each sub-agent: do not post' clause from chore(panel-review): tighten one-comment contract; safety cap 1 -> 7 #905 was lossy guidance, not enforcement. Result: per-persona comment spam.

  2. The orchestrator hangs waiting on background sub-agents. It frequently times out before all 5 return, and the session ends without ever reaching CEO arbitration or single-comment emission.

Per agentskills.io

Agents load skills through progressive disclosure, in three stages:

  1. Discovery: load only the name and description
  2. Activation: read the full SKILL.md instructions into context
  3. Execution: follow the instructions, optionally executing bundled code or loading referenced files as needed.

A skill is instructions + resources loaded into ONE agent loop. There is no spawning of sub-agents. The pre-#882 SKILL.md got this right.

Fix

Restore the in-context persona model:

  • Notes: the panel is a sequence of reasoning passes inside one agent loop; persona files are loaded on demand when you take that lens.
  • Execution checklist step 3: replaced 'Dispatch contract' with 'Persona pass procedure' -- read persona file, take its lens, write findings to working notes, drop the lens.
  • Execution checklist step 5: CEO arbitration is also an in-context lens (load apm-ceo.agent.md), not a separate sub-agent.
  • Execution checklist step 7: explicit single-emission rule.
  • Auth Expert routing wording: 'dispatch' -> 'activate the lens'.

What's preserved from #882

  • Hybrid E auth routing (fast-path + fallback self-check)
  • Pre-arbitration completeness gate (now checks working-notes sections instead of sub-agent returns)
  • assets/verdict-template.md as the verdict shape
  • Cap-7 fail-soft ceiling in workflow frontmatter

Workflow .md is unchanged

It already just says 'use the apm-review-panel skill on this PR' -- exactly what the user asked for. All review behavior lives in the skill.

Verification

Will be verified live by re-applying the panel-review label on PR #889 after merge.

Closes #906.

Copilot AI review requested due to automatic review settings April 24, 2026 16:37
…lls.io)

PR #882 introduced a sub-agent dispatch model: 'dispatch one sub-agent
per persona via the task tool; do not open persona files in the
orchestrator thread.' This broke the panel in two ways:

1. Sub-agents launched via the task tool inherit the parent's MCP
   gateway -- including safeoutputs. They will happily call
   safeoutputs-add_comment themselves, producing per-persona comment
   spam instead of one synthesized verdict (run 24899623301: 2
   personas posted, no synthesized comment).

2. The orchestrator hangs waiting on background sub-agents, often
   times out before all 5 return, and the session ends without ever
   reaching CEO arbitration or single-comment emission (run
   24897368527: zero comments).

Per agentskills.io, the skill model is progressive disclosure inside
ONE agent loop: the agent loads the SKILL.md, then loads referenced
files (persona .agent.md) on demand, then produces output. There is
no spawning of sub-agents -- a skill is instructions+resources, not
multi-agent fan-out.

Fix: restore the pre-#882 in-context persona model:

- 'Notes' section: rewritten to say the panel is a sequence of
  reasoning passes inside one agent loop; persona files are loaded
  on demand when you take that lens.
- 'Execution checklist' step 3: replaced 'Dispatch contract' with
  'Persona pass procedure' -- read persona file, take its lens,
  write findings to working notes, drop the lens.
- 'Execution checklist' step 5: CEO arbitration is also an
  in-context lens (load apm-ceo.agent.md), not a separate sub-agent.
- 'Execution checklist' step 7: explicit single-emission rule.
- Auth Expert routing: 'dispatch' -> 'activate the lens' wording.

Keeps everything valuable from #882: Hybrid E auth routing, the
completeness gate, the verdict-template asset, the cap-7 fail-soft
ceiling.

Workflow .md is unchanged -- it already just delegates to the skill,
which is exactly what the user asked for.

Closes #906

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@danielmeppiel danielmeppiel force-pushed the fix/panel-review-restore-in-context-model branch from 1eab7c2 to 81b80fc Compare April 24, 2026 16:40
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Restores the apm-review-panel skill to an in-context, single-agent progressive-disclosure model (no sub-agent dispatch) to prevent per-persona emissions and ensure a single synthesized verdict.

Changes:

  • Rewrites the panel contract to run persona “lenses” sequentially in one agent loop, with a completeness gate and CEO arbitration in-context.
  • Adds an explicit conditional activation rule for the Auth Expert (fast-path file list + fallback self-check).
  • Updates the roster/routing/quality gates/output contract to reflect the lens-based approach and single-emission rule.
Show a summary per file
File Description
.github/skills/apm-review-panel/SKILL.md Updates the bundled skill used by the workflow to enforce in-context persona passes, completeness gating, and single-comment emission.
.apm/skills/apm-review-panel/SKILL.md Mirrors the same skill changes in the source-of-truth .apm/ location.

Copilot's findings

Comments suppressed due to low confidence (2)

.github/skills/apm-review-panel/SKILL.md:225

  • safe-outputs.add-comment.max is documented here as a fail-soft ceiling "currently 7", but the workflow frontmatter in .github/workflows/pr-review-panel.md sets safe-outputs.add-comment.max: 1. This mismatch can cause the panel to follow an outdated contract; please either update the workflow cap to 7 (if intended) or adjust this text to match the workflow (or avoid hard-coding the number).
- Produce **exactly one** comment per panel run. The
  `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.

.apm/skills/apm-review-panel/SKILL.md:225

  • safe-outputs.add-comment.max is documented here as a fail-soft ceiling "currently 7", but the workflow frontmatter in .github/workflows/pr-review-panel.md sets safe-outputs.add-comment.max: 1. This mismatch can cause the panel to follow an outdated contract; please either update the workflow cap to 7 (if intended) or adjust this text to match the workflow (or avoid hard-coding the number).
- Produce **exactly one** comment per panel run. The
  `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.
  • Files reviewed: 2/2 changed files
  • Comments generated: 2

Comment on lines 194 to +199
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. You (the orchestrator) write
the comment; never delegate emission to a sub-agent and never call
the GitHub API directly.

### Dispatch contract

For each persona being dispatched, run this exact procedure:

1. Dispatch one sub-agent for that persona only -- never chain
multiple personas inside a single sub-agent invocation.
2. Pass only:
- the PR title and body summary,
- the relevant diff context for that persona's scope,
- why this persona is in scope (or, for Auth Expert, the rule
that activated it),
- the required return shape (findings only; never the final
comment text and never top-level verdict headings).
3. Capture the raw return in working notes under
`<persona-name>: <findings>` or, for an inactive Auth Expert,
`Auth Expert inactive reason: <one sentence>`.
4. Do not summarise unopened persona files yourself; do not paste
persona file contents into the orchestrator context.
`safe-outputs.add-comment` channel. Never call the GitHub API
directly. This is the ONLY output emission for the entire panel
run -- no per-persona comments, no progress comments.
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This skill requires assets/verdict-template.md (referenced in the routing matrix and steps 6/Output contract), but there is no assets/ directory under .github/skills/apm-review-panel/ (and none under .apm/skills/apm-review-panel/ either). As written, step 6 will fail when the agent tries to load the missing template; please add the template file to the skill bundle (and ensure it is mirrored into both .apm/ and .github/) or remove the dependency and keep the verdict structure entirely in SKILL.md.

This issue also appears on line 222 of the same file.

Suggested change
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. You (the orchestrator) write
the comment; never delegate emission to a sub-agent and never call
the GitHub API directly.
### Dispatch contract
For each persona being dispatched, run this exact procedure:
1. Dispatch one sub-agent for that persona only -- never chain
multiple personas inside a single sub-agent invocation.
2. Pass only:
- the PR title and body summary,
- the relevant diff context for that persona's scope,
- why this persona is in scope (or, for Auth Expert, the rule
that activated it),
- the required return shape (findings only; never the final
comment text and never top-level verdict headings).
3. Capture the raw return in working notes under
`<persona-name>: <findings>` or, for an inactive Auth Expert,
`Auth Expert inactive reason: <one sentence>`.
4. Do not summarise unopened persona files yourself; do not paste
persona file contents into the orchestrator context.
`safe-outputs.add-comment` channel. Never call the GitHub API
directly. This is the ONLY output emission for the entire panel
run -- no per-persona comments, no progress comments.
6. Now (and only now) assemble the final verdict using the
`## Output contract` section in this file. Do not load any
external template file.
7. Emit the completed verdict as exactly ONE comment via the
workflow's `safe-outputs.add-comment` channel. Never call the
GitHub API directly. This is the ONLY output emission for the
entire panel run -- no per-persona comments, no progress
comments.

Copilot uses AI. Check for mistakes.
Comment on lines 194 to 196
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
Copy link

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This skill requires assets/verdict-template.md (referenced in the routing matrix and steps 6/Output contract), but there is no assets/ directory under .apm/skills/apm-review-panel/ (and none under .github/skills/apm-review-panel/ either). As written, step 6 will fail when the agent tries to load the missing template; please add the template file to the skill bundle (and ensure it is mirrored into both .apm/ and .github/) or remove the dependency and keep the verdict structure entirely in SKILL.md.

This issue also appears on line 222 of the same file.

Copilot uses AI. Check for mistakes.
@danielmeppiel danielmeppiel merged commit ea6558e into main Apr 24, 2026
7 checks passed
@danielmeppiel danielmeppiel deleted the fix/panel-review-restore-in-context-model branch April 24, 2026 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[aw] PR Review Panel failed

2 participants