feat(skills/fanning-out-with-worktrees): two-stage sub-PR review#12
Merged
Conversation
Replace the single per-sub-PR `review` pass at ripening with SDD's review discipline: a spec-compliance pass (acceptance criteria mapped to code + tests, scope creep flagged) that must come back clean before a code-quality pass runs. Findings route back to the worktree subagent and the same pass re-runs until clean — the orchestrator reviews, the author fixes, never the reverse. Both passes are the local `review` skill, scoped differently, so the plugin keeps one review mechanism. The spec-before-quality order is a gate: quality findings on code that's about to change are noise. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the fanning-out-with-worktrees skill documentation to replace the single sub-PR review step with a gated, two-stage review process (spec-compliance first, then code-quality), including an explicit fix-loop that routes findings back to the worktree subagent.
Changes:
- Replaces the single “Review” step in per-sub-PR ripening with a two-stage review: spec-compliance (gate) → code-quality.
- Adds a fix-loop protocol (author fixes in the worktree; orchestrator re-reviews; rerun the same pass until clean).
- Adds new anti-patterns and red-flag table entries to prevent collapsing stages, reviewing quality before spec, and orchestrator fixing in place.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - **Dispatching without the conventions block.** Contracts make the work compile; conventions make it cohere. Hand a subagent contracts but no `## Conventions` and it invents its own layout and names — the drift surfaces at the integration PR when it's expensive. If the plan has no conventions block, go back to `planning-a-feature` Step 6. | ||
| - **Skipping wave dependencies.** Consumers dispatched before producers' contracts are realized produce code against an imagined shape. Dispatch wave N+1 only after wave N is fully self-merged and contracts locked. | ||
| - **Self-merging without orchestrator review.** The integration PR is where external review lands, but sub-PRs still need a review pass before going into the feature branch. Skipping it dumps issues onto the integration PR reviewer. | ||
| - **Collapsing the two stages into one "looks good" pass.** Spec-compliance and code-quality answer different questions — "does it do the job" vs "is it well-built". Reviewing them together lets a momentum-driven skim pass a PR that quietly misses an acceptance criterion or builds unrequested scope. Run the spec gate to clean first, then quality. |
Comment on lines
+143
to
+144
| | "Tests pass and the diff looks clean — one review pass is enough" | Spec-compliance and quality are different questions. The spec gate runs to clean first, then quality; one blended pass skims past a missed acceptance criterion. | | ||
| | "The spec pass found a gap but I'll quality-review now to save a round-trip" | Quality findings on code about to change are noise. Send the gap back, re-run the spec pass to clean, then quality. | |
sourcehawk
force-pushed
the
feature/two-stage-sub-pr-review
branch
from
0ceb3a0 to
0a6a7fc
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Replaces the single per-sub-PR review at ripening with a two-stage review: a spec-compliance gate that must pass before a code-quality pass runs. The previous single pass let a momentum-driven skim approve a sub-PR that quietly missed an acceptance criterion or built unrequested scope; separating "does it do the job" from "is it well-built" closes that gap before the diff reaches the integration PR.
Changes
fanning-out-with-worktreesStep 5 ripening: spec-compliance pass (acceptance criteria mapped to code + tests, scope creep flagged) gates the code-quality pass.reviewskill, scoped differently — one review mechanism across the plugin.Testing
Built via
superpowers:writing-skillswith isolated baselines. RED baselines did one blended "looks good" pass with no spec-vs-quality separation and no fix-loop structure. With the skill, an isolated orchestrator kept the quality pass closed behind the spec gate, routed findings back to the subagent, re-ran the spec pass, and only surfaced quality findings after the gate cleared.