skill(subagent-driven-development): DRY team conventions (#5)#10
Conversation
…am-conventions.md Extract repeated boilerplate from implementer/spec-reviewer/code-reviewer prompts into a single shared file. Each prompt template now references agents/team-conventions.md instead of inlining TDD discipline, self-review checklist, review protocol, and sanitization rules. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…after Documents the before/after of extracting team conventions from inline prompt boilerplate to agents/team-conventions.md. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR reduces repeated boilerplate across the subagent-driven-development role prompt templates by centralizing shared discipline/review/sanitization rules into a single agents/team-conventions.md and updating the skill and templates to reference it.
Changes:
- Added
agents/team-conventions.mdas the shared source of team-wide conventions. - Replaced large inlined sections in implementer/spec-reviewer/code-reviewer prompt templates with references to the shared conventions file.
- Updated
skills/subagent-driven-development/SKILL.mdand added a before/after fixture documenting the DRY refactor.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| skills/subagent-driven-development/test-fixtures/dry-conventions/before-after.md | New fixture documenting the before/after DRY change. |
| skills/subagent-driven-development/spec-reviewer-prompt.md | Now references shared team conventions instead of inlining reviewer discipline. |
| skills/subagent-driven-development/implementer-prompt.md | Now references shared team conventions instead of inlining TDD/self-review boilerplate. |
| skills/subagent-driven-development/code-quality-reviewer-prompt.md | Updated to reference shared conventions for reviewer protocol/checklists. |
| skills/subagent-driven-development/SKILL.md | Replaced inline placeholders with references to the shared conventions file. |
| agents/team-conventions.md | New shared conventions file for implementer/spec-reviewer/code-reviewer/all-agents rules. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
A: conventions.md code-reviewer — bug-class checklist + verdict vocabulary now reference skills/requesting-code-review/SKILL.md explicitly. B: conventions.md implementer — version-skew audit now references skills/finishing-a-development-branch/SKILL.md Step 1c. C: implementer-prompt.md — same version-skew audit Step 1c reference. D: code-quality-reviewer-prompt.md — split adversarial framing from bug-class checklist + verdict vocabulary; latter now point to skills/requesting-code-review/SKILL.md. E: conventions.md code-reviewer — adversarial framing exception clause preserved: if fewer than 3 issues found, document every bug-class check run rather than manufacturing issues. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
A: implementer-prompt.md — restore explicit "DM spec-reviewer when ready" step so the spec-compliance gate isn't stalled after implementation. B: SKILL.md — fix stale "checklist below" reference in implementer workflow step 4; now points to agents/team-conventions.md. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
A: implementer-prompt.md — DM team-lead with branch/commit + "ready for merge" (not PR link; PR is created by team-lead via finishing-a-branch after all tasks complete, not by the implementer). B: SKILL.md code-reviewer section — split bug-class checklist + verdict vocabulary to their canonical source (skills/requesting-code-review/ SKILL.md); team-conventions only hosts adversarial framing + output format. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Follow team conventions: see `agents/team-conventions.md` (committed in | ||
| this repo) for adversarial framing and per-finding inline output format. | ||
| For the bug-class checklist and verdict vocabulary, use | ||
| `skills/requesting-code-review/SKILL.md`. |
There was a problem hiding this comment.
Inside this dispatch template you now point reviewers at skills/requesting-code-review/SKILL.md for adversarial framing + per-finding inline output, but the referenced base template (skills/requesting-code-review/code-reviewer.md) uses a different (validation-framing, strengths/issues) structure. This creates conflicting instructions and makes it unclear which output format/checklist is authoritative. Consider either (a) updating the referenced base template to match the SKILL’s adversarial/per-finding requirements, or (b) switching this dispatch to reference a template that already implements the SKILL’s format and vocabulary.
…ixture SKILL.md implementer Team Conventions block: replace enumeration that implied "self-review checklist" is inline with explicit "all rules are defined in agents/team-conventions.md, not repeated here"; also sync version-skew audit pointer to finishing-a-development-branch Step 1c. Fixture: add 5 invariants the "after" state must hold, including: - no "checklist below" / "see below" / "inlined here" language - every version-skew audit reference points to Step 1c - every bug-class / verdict reference points to requesting-code-review - adversarial-framing exception clause preserved verbatim Verify command excludes test-fixtures dir to avoid self-matching. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
A: fixture verify command — switch to grep -rEn with ERE; replace \b (non-portable) with ([[:space:][:punct:]]|$) for reliable Self-Review boundary match across grep implementations. B: team-conventions.md sanitization rule — tighten "no version references" to "no specific internal project/company/product-version/incident references"; explicitly allow dependency/runtime/tooling version numbers needed for accurate technical guidance (version-skew audit, pin refs). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Follow team conventions: see `agents/team-conventions.md` (committed in | ||
| this repo) for adversarial framing and per-finding inline output format. | ||
| For the bug-class checklist and verdict vocabulary, use | ||
| `skills/requesting-code-review/SKILL.md`. |
There was a problem hiding this comment.
In this Task tool block, the earlier line Use template at requesting-code-review/code-reviewer.md points to a non-existent path in this repo (the file is skills/requesting-code-review/code-reviewer.md). Please update the reference so dispatchers can find the correct template.
…ef check code-quality-reviewer-prompt.md: fix broken path "requesting-code-review/ code-reviewer.md" → "skills/requesting-code-review/code-reviewer.md" (missing skills/ prefix; file exists at the correct path). Fixture: add 2 new invariant checks: - Dead skill file refs: grep all skills/*.md references, verify each resolves to an existing file (catches missing-prefix bugs like this one). - Bare requesting-code-review refs: grep for refs without skills/ prefix (both checks exclude test-fixtures dir to avoid self-matching). All three invariant checks PASS on current tree. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| - DRY: when re-stating a convention, prefer "per `<skill>`" over | ||
| inlining the rule. | ||
| - DM team-lead when blocked, when CI breaks, when ready for merge. |
There was a problem hiding this comment.
“DM team-lead …” uses a hyphenated role name, but other parts of this skill use “team lead”. If this is intended to be a literal message recipient, consider standardizing the name (or specifying the exact recipient ID) to avoid misdirected DMs.
| - DM team-lead when blocked, when CI breaks, when ready for merge. | |
| - DM team lead when blocked, when CI breaks, when ready for merge. |
… all role prompts Round-7 fix: use hyphenated team-lead consistently with agents/team-conventions.md and implementer-prompt.md usage pattern, avoiding ambiguity with the Agent Teams role identifier. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| As team-lead, your job is now orchestration: | ||
| - Monitor task completions via TaskList |
There was a problem hiding this comment.
Within this document, the team lead role is referred to as both "Team Lead" (in the team graph) and "team-lead" (in workflow text). Consider standardizing on one spelling, and if "team-lead" is intended to be a SendMessage recipient name, explicitly define that mapping (e.g., "DM Lead (team lead)") to avoid ambiguity for subagents.
…der syntax + team-lead graph label
- Switch {N}/{team-name} curly-brace placeholders to <N>/<team-name> angle-bracket
style in implementer-prompt.md and spec-reviewer-prompt.md, consistent with
all other skill templates
- Update DOT graph node label from "Team Lead" to "team-lead" to match the
SendMessage recipient identifier used throughout the workflow text
- Add fixture invariant #6 + verify command for curly-brace placeholder detection
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…line bare refs Round-10 finding: [^/]requesting-code-review/ requires a preceding character, so a bare ref at the start of a line would be missed. Switch to (^|[^/])requesting-code-review/ to cover both cases. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…on — point to team-conventions only Round-11 findings: both implementer-prompt.md and the embedded implementer prompt in SKILL.md enumerate convention details (TDD, regression-invariant, etc.) that are already centralized in agents/team-conventions.md. This re-inlining contradicts the DRY goal of the refactor. Trim to a simple pointer; the single source of truth is the conventions file. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| WHAT_WAS_IMPLEMENTED: [from implementer's report] | ||
| PLAN_OR_REQUIREMENTS: Task N from [plan-file] | ||
| BASE_SHA: [commit before task] | ||
| HEAD_SHA: [current commit] | ||
| DESCRIPTION: [task summary] |
There was a problem hiding this comment.
The referenced skills/requesting-code-review/code-reviewer.md template includes a {PLAN_REFERENCE} placeholder, but this dispatch template doesn’t provide a PLAN_REFERENCE: value. This will leave an unexpanded placeholder (or missing context) in code-reviewer runs; add the missing field or align the field names with the referenced template’s placeholders.
…FERENCE field + grammar
- Add PLAN_REFERENCE field to code-quality-reviewer dispatch template to match
the {PLAN_REFERENCE} placeholder in skills/requesting-code-review/code-reviewer.md
(distinct from PLAN_OR_REQUIREMENTS used in the instructions section)
- Fix grammar in agents/team-conventions.md: "Shared rules every team agent"
→ "Shared rules that every team agent"
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ts/boundary-classes.md The PR codifying P12 (one-sided boundary wiring) itself demonstrated P12 in action: three skills listed the boundary-class pairs inline with inconsistent coverage (producer→consumer/caller→callee only in some; plugin→host and sender→handler in others). Meta-irony: the pattern the PR was adding was not self-applied across the PR's own edits. Fix: create agents/boundary-classes.md as the canonical source of truth (parallel to agents/team-conventions.md from PR #10's DRY refactor). All three skills now carry the full inline list PLUS a reference pointer to the canonical file. Future additions to the boundary-class list only need to be made once. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…across three skills (#11) * docs: add design doc for P11/P12 skill patterns (shortcut-bias + one-sided boundary wiring) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs: add implementation plan for P11/P12 skill patterns Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(requesting-code-review): add P11 + P12 to bug-class checklist * feat(test-driven-development): add P11 proper-solution rule * feat(runtime-launch-validation): add P12 interface boundary change row * feat(test-driven-development): add P12 interface-boundary coverage rule * fix: address code-reviewer findings on P11/P12 additions - F1+F2: Replace mental-question formulation with executable revert-and-restore directive in TDD P11 section; fix incorrect "RED phase" label to "Verify Regression Invariant step, between GREEN and REFACTOR" - F3: Add sender→handler to requesting-code-review P12 boundary pair list to match the four pairs defined in the TDD P12 section - F4: Add interface boundary change to runtime-launch-validation "When this applies" trigger list so implementers reach the new table row Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(pr11-round1): expand interface boundary types to all 4 pairs across runtime-launch-validation and plan docs Copilot round-1 finding: per-change-class row and plan docs listed only producer→consumer/caller→callee; expand to include plugin→host and sender→handler consistently with the trigger list and other skills. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(boundary-classes): extract canonical boundary-class list to agents/boundary-classes.md The PR codifying P12 (one-sided boundary wiring) itself demonstrated P12 in action: three skills listed the boundary-class pairs inline with inconsistent coverage (producer→consumer/caller→callee only in some; plugin→host and sender→handler in others). Meta-irony: the pattern the PR was adding was not self-applied across the PR's own edits. Fix: create agents/boundary-classes.md as the canonical source of truth (parallel to agents/team-conventions.md from PR #10's DRY refactor). All three skills now carry the full inline list PLUS a reference pointer to the canonical file. Future additions to the boundary-class list only need to be made once. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(pr11-round2): align plan+design docs to canonical 4 boundary pairs Copilot round-2 finding: docs/plans still used incomplete or inconsistent boundary-pair lists (missing sender→handler; using client→server instead of canonical pairs). Align all occurrences in design doc and plan doc to producer→consumer, caller→callee, plugin→host, sender→handler — matching agents/boundary-classes.md and the three updated skill files. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(pr11-round3): fix plugin→host example direction, replace hardcoded repo refs with placeholders, update design doc to reflect boundary-classes.md addition Round-3 Copilot findings: - agents/boundary-classes.md: plugin→host example "host invoking a plugin hook" went the wrong direction (host→plugin). Fix: "plugin calling a host API" — both examples now consistently plugin→host. - docs/plans: gh api commands and DM template hardcoded GoCodeAlone/ claude-superpowers, conflicting with the plan's own sanitization rule. Fix: replace with <owner>/<repo> placeholders. - docs/plans/design: Approach section said "No new shared operational pattern docs" but the PR adds agents/boundary-classes.md. Fix: update Approach and Out of Scope to accurately reflect the new file. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(pr11-round4): clarify interface-boundary applicability condition in TDD skill Copilot round-4 finding: opening definition said "when a change touches both sides of such a boundary" — this contradicts the heuristic below, which explicitly asks whether either side would pass even if the other side had NOT been updated (i.e., the pattern triggers even when only one side has been changed so far). Rephrase: "when a change introduces or modifies a contract across such a boundary" — this correctly captures the trigger (contract change) without implying both sides must already be modified. Adds a sentence pointing to the heuristic for detection. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
agents/team-conventions.md## Team Conventions→See agents/team-conventions.mdreferencesskills/subagent-driven-development/test-fixtures/dry-conventions/before-after.mddocuments the before/afterDesign
See:
docs/plans/2026-04-25-superpowers-skills-improvements-design.mdImplementation Plan
See:
docs/plans/2026-04-25-superpowers-skills-improvements.md(Tasks 27–29)Changes
agents/team-conventions.md(new): Implementer, spec-reviewer, code-reviewer, and all-agents convention sectionsskills/subagent-driven-development/implementer-prompt.md: Replaced 87-line template with 20-line reference templateskills/subagent-driven-development/spec-reviewer-prompt.md: Replaced 69-line template with 22-line reference templateskills/subagent-driven-development/code-quality-reviewer-prompt.md: Updated to reference team conventionsskills/subagent-driven-development/SKILL.md: Replaced[Content from *.md]placeholders withagents/team-conventions.mdreferencesskills/subagent-driven-development/test-fixtures/dry-conventions/before-after.md(new): DRY fixture🤖 Generated with Claude Code