Review Retrospective — PR #750
PR: feat: auto-fix feedback loop — tests fail → /fix → retest
Merged: 2026-04-23 by PureWeen
Review runs: GitHub API not accessible (gh not authenticated; no GitHub MCP tool access). Analysis via static code inspection of merged content.
Context
PR #750 was a large initial commit (448 files) introducing the review infrastructure itself — expert-reviewer.agent.md, review-shared.md, auto-fix-on-failure.yml, and all lock files. The expert review ran on the very files that define how it works.
Skill Coverage Analysis
| Skill |
Relevant? |
Referenced? |
Gap? |
| gh-aw-guide |
✅ Yes (workflow files introduced) |
Unable to verify |
⚠️ See findings |
| processing-state-safety |
❌ No |
— |
— |
| multi-agent-orchestration |
❌ No |
— |
— |
| performance-optimization |
❌ No |
— |
— |
| copilot-sdk-reference |
❌ No |
— |
— |
Review Accuracy
| Metric |
Count |
| True Positives |
Unknown (GitHub API inaccessible) |
| True Positives (Fixed) |
Unknown |
| False Positives |
Unknown |
| False Negatives (code analysis) |
2 |
| Unresolved |
2 |
Missed Findings (False Negatives)
1. 🟡 MODERATE — expert-reviewer.agent.md has dispatch instructions duplicating review-shared.md
File: .github/agents/expert-reviewer.agent.md, line 21
Section 2 ("Multi-Model Review") instructs: Dispatch **3 parallel sub-agents** via the task tool.
review-shared.md Step 2 (line 56) also instructs the orchestrator: Launch **exactly 3 sub-agents in parallel** using the task tool.
Risk: The orchestrator reads expert-reviewer.agent.md as the "agent to use" and receives two conflicting sets of dispatch instructions. This creates potential 3→9 nested fan-out:
- Orchestrator follows
expert-reviewer.agent.md → dispatches 3 sub-agents
- Each sub-agent reads
expert-reviewer.agent.md → dispatches 3 more = 9 total
The sub-agent prompts in review-shared.md include "do NOT dispatch sub-agents or use the task tool" as a guardrail, but the conflict between files is unreliable across model versions and degrades gracefully only when the inline instructions win.
Fix: Remove Section 2 entirely from expert-reviewer.agent.md. The file should be a single-reviewer spec only (context gathering + findings format). All orchestration (dispatch, consensus, posting) belongs exclusively in review-shared.md.
Which skill catches it: gh-aw-guide anti-patterns note — "Sub-agent prompts that say 'Read and follow expert-reviewer.agent.md' risk recursive sub-agent spawning. Add 'Do NOT dispatch sub-agents' guard or inline review dimensions instead of referencing the agent file." The fix is to make the agent file itself safe to read (no dispatch instructions), not just guard sub-agents against it.
2. 🟢 MINOR — auto-fix-on-failure.yml fix-count uses fragile free-text matching
File: .github/workflows/auto-fix-on-failure.yml, line 65
The attempt counter uses:
[.comments[].body | select(contains("Auto-fix triggered"))] | length
```
**Failure scenarios:**
- A human posts a comment containing "Auto-fix triggered" → counter inflates → loop stops before 3 real attempts
- Comment template changes in a future PR → counter drops to 0 → loop runs past max
**Fix:** Embed a hidden HTML marker in the auto-fix comment body:
```
Match via contains(""). HTML comments are invisible to users and immune to false matches in discussion text.
Improvement Suggestions
-
expert-reviewer.agent.md → single-reviewer spec. Remove Section 2 ("Multi-Model Review"). Orchestration belongs only in review-shared.md. This makes the agent file safe to read without triggering recursive fan-out.
-
auto-fix-on-failure.yml → HTML marker for attempt counting. Replace free-text "Auto-fix triggered" match with a hidden `` marker.
-
Retro data gap. This retrospective cannot access GitHub API review history — gh CLI requires GH_TOKEN (not set), and GitHub MCP tools are unavailable. Add a read-only token via a steps: pre-step in the retro workflow so the agent can retrieve actual review comments for TP/FP/FN classification.
Generated by Review Retrospective for issue #750 · ◷
Review Retrospective — PR #750
PR: feat: auto-fix feedback loop — tests fail → /fix → retest
Merged: 2026-04-23 by PureWeen
Review runs: GitHub API not accessible (gh not authenticated; no GitHub MCP tool access). Analysis via static code inspection of merged content.
Context
PR #750 was a large initial commit (448 files) introducing the review infrastructure itself —
expert-reviewer.agent.md,review-shared.md,auto-fix-on-failure.yml, and all lock files. The expert review ran on the very files that define how it works.Skill Coverage Analysis
Review Accuracy
Missed Findings (False Negatives)
1. 🟡 MODERATE —
expert-reviewer.agent.mdhas dispatch instructions duplicatingreview-shared.mdFile:
.github/agents/expert-reviewer.agent.md, line 21Section 2 ("Multi-Model Review") instructs:
Dispatch **3 parallel sub-agents** via the task tool.review-shared.mdStep 2 (line 56) also instructs the orchestrator:Launch **exactly 3 sub-agents in parallel** using the task tool.Risk: The orchestrator reads
expert-reviewer.agent.mdas the "agent to use" and receives two conflicting sets of dispatch instructions. This creates potential 3→9 nested fan-out:expert-reviewer.agent.md→ dispatches 3 sub-agentsexpert-reviewer.agent.md→ dispatches 3 more = 9 totalThe sub-agent prompts in
review-shared.mdinclude"do NOT dispatch sub-agents or use the task tool"as a guardrail, but the conflict between files is unreliable across model versions and degrades gracefully only when the inline instructions win.Fix: Remove Section 2 entirely from
expert-reviewer.agent.md. The file should be a single-reviewer spec only (context gathering + findings format). All orchestration (dispatch, consensus, posting) belongs exclusively inreview-shared.md.Which skill catches it:
gh-aw-guideanti-patterns note — "Sub-agent prompts that say 'Read and follow expert-reviewer.agent.md' risk recursive sub-agent spawning. Add 'Do NOT dispatch sub-agents' guard or inline review dimensions instead of referencing the agent file." The fix is to make the agent file itself safe to read (no dispatch instructions), not just guard sub-agents against it.2. 🟢 MINOR —
auto-fix-on-failure.ymlfix-count uses fragile free-text matchingFile:
.github/workflows/auto-fix-on-failure.yml, line 65The attempt counter uses:
Match via
contains(""). HTML comments are invisible to users and immune to false matches in discussion text.Improvement Suggestions
expert-reviewer.agent.md→ single-reviewer spec. Remove Section 2 ("Multi-Model Review"). Orchestration belongs only inreview-shared.md. This makes the agent file safe to read without triggering recursive fan-out.auto-fix-on-failure.yml→ HTML marker for attempt counting. Replace free-text"Auto-fix triggered"match with a hidden `` marker.Retro data gap. This retrospective cannot access GitHub API review history —
ghCLI requiresGH_TOKEN(not set), and GitHub MCP tools are unavailable. Add a read-only token via asteps:pre-step in the retro workflow so the agent can retrieve actual review comments for TP/FP/FN classification.