feat(skills): add shipping-features, capturing-knowledge & learning-from-review - #9874
feat(skills): add shipping-features, capturing-knowledge & learning-from-review#9874pa-lem wants to merge 1 commit into
Conversation
…rom-review Introduce the agent skills suite under .agents/skills/: - shipping-features: classified, resumable conductor that drives a unit of work spec -> plan -> implement -> review -> CI -> PR, delegating each step to existing tools (phases/ references for classification, manifest, pr-split, reliability). - capturing-knowledge: persist non-obvious domain/code facts into the project's knowledge/guidelines/guides docs. - learning-from-review: reconstruct a reviewed PR's proposed -> rejected -> corrected delta, diagnose why an agent would have proposed the rejected version, and hand the durable lesson to capturing-knowledge. learning-from-review is wired as step 2 of shipping-features' phase 6 (the knowledge step). The phase draws an explicit three-way boundary so the tools do not overlap: capturing-knowledge records domain/code facts, learning-from-review distills review lessons, and speckit-opsmill-retrospect improves the process & tooling surface. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
5 issues found across 8 files
Confidence score: 3/5
- In
.agents/skills/shipping-features/SKILL.md, Phase 3 and the classification lane table conflict on whether Medium/Large bugs require a light plan, which can cause agents to follow different workflows for the same bug and produce inconsistent delivery quality — align both sections to one explicit rule before merging. - In
.agents/skills/shipping-features/SKILL.md, Phase 4 requires runningpruning-residues, but no such skill/command exists under.agents/; this creates a dead instruction that can stall execution or lead to skipped cleanup steps — either add the tool or replace/remove that step before merge. - The process docs are drifting:
.agents/skills/shipping-features/docs/overview-and-design.mdlists Phase 6 sub-steps in a different order thanSKILL.md, and.agents/skills/capturing-knowledge/SKILL.mdomits root discovery paths in its primary command examples; this can drive inconsistent agent behavior across docs — sync the step order and update discovery commands to includeknowledge/,guidelines/, andguides/.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".agents/skills/shipping-features/phases/pr-split.md">
<violation number="1" location=".agents/skills/shipping-features/phases/pr-split.md:26">
P3: "Revertable" is non-standard; "revertible" is the expected adjective (able to be reverted). Same pattern as convertible, reversible.</violation>
</file>
<file name=".agents/skills/capturing-knowledge/SKILL.md">
<violation number="1" location=".agents/skills/capturing-knowledge/SKILL.md:68">
P3: The discovery shell commands search under `dev docs` but miss the root-level paths (`knowledge/`, `guidelines/`, `guides/`) listed as valid locations in the same section. The fallback chain ultimately catches these via AGENTS.md or the user prompt, so discovery won't break — but the inconsistency could cause an unnecessary extra turn in projects that use root-level doc directories. Consider widening the search to include `.` or the specific root-level paths alongside `dev docs` so the probe matches all the locations the text describes.</violation>
</file>
<file name=".agents/skills/shipping-features/SKILL.md">
<violation number="1" location=".agents/skills/shipping-features/SKILL.md:95">
P3: The discovery table lists `pruning-residues` as an in-repo tool for residue cleanup, and Phase 4 instructs the agent to run it first. However, no `pruning-residues` skill or command exists anywhere under `.agents/`. The priority chain handles this via fallbacks (`code-simplifier` → manual agent cleanup) and the table advises reconciling against `.agents/`, so it degrades gracefully. Still, the column heading claims this is 'what actually ships here today,' which is inaccurate for this entry. Consider either adding the skill or marking the entry with a note (e.g., 'planned' or 'via fallback only') to avoid confusing agents.</violation>
<violation number="2" location=".agents/skills/shipping-features/SKILL.md:178">
P1: Phase 3 says it's for features M/L only, but the classification lane table says M/L bugs get a light plan. This is a direct contradiction — on a Medium bug, one instruction says skip plan, the other says do a light plan. The `(feature M/L; skipped otherwise)` in the Phase 3 header should be updated to match the lane-table intent (e.g., `(feature M/L; light plan on bug M/L; skipped on chore/S)`), or the bug column should say `skip` if plan really is feature-only.</violation>
</file>
<file name=".agents/skills/shipping-features/docs/overview-and-design.md">
<violation number="1" location=".agents/skills/shipping-features/docs/overview-and-design.md:88">
P3: The pipeline table in overview-and-design.md lists the Phase 6 sub-steps in a different order than SKILL.md Phase 6 defines them. The overview has retrospective before audit-docs, but the SKILL.md runs audit-docs (step 3) before retrospective (step 4). Reorder the overview table to match the authoritative phase definition.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
| - **chore:** short inline brief. **Gate:** scope stated in one paragraph. | ||
| **Checkpoint:** show the artifact + open questions. | ||
|
|
||
| ### Phase 3 — Plan (feature `M`/`L`; skipped otherwise) |
There was a problem hiding this comment.
P1: Phase 3 says it's for features M/L only, but the classification lane table says M/L bugs get a light plan. This is a direct contradiction — on a Medium bug, one instruction says skip plan, the other says do a light plan. The (feature M/L; skipped otherwise) in the Phase 3 header should be updated to match the lane-table intent (e.g., (feature M/L; light plan on bug M/L; skipped on chore/S)), or the bug column should say skip if plan really is feature-only.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .agents/skills/shipping-features/SKILL.md, line 178:
<comment>Phase 3 says it's for features M/L only, but the classification lane table says M/L bugs get a light plan. This is a direct contradiction — on a Medium bug, one instruction says skip plan, the other says do a light plan. The `(feature M/L; skipped otherwise)` in the Phase 3 header should be updated to match the lane-table intent (e.g., `(feature M/L; light plan on bug M/L; skipped on chore/S)`), or the bug column should say `skip` if plan really is feature-only.</comment>
<file context>
@@ -0,0 +1,259 @@
+- **chore:** short inline brief. **Gate:** scope stated in one paragraph.
+**Checkpoint:** show the artifact + open questions.
+
+### Phase 3 — Plan (feature `M`/`L`; skipped otherwise)
+**Diverge** (3 `Plan` framings: minimal / refactor-friendly / test-first) → **synthesize** one
+ordered task list; formalize with `/speckit-plan` + `/speckit-tasks` → `plan.md`, `tasks.md`.
</file context>
| ### Phase 3 — Plan (feature `M`/`L`; skipped otherwise) | |
| ### Phase 3 — Plan (feature `M`/`L`; light on bug `M`/`L`; skipped otherwise) |
| - Independent concerns (unrelated drive-by refactor, or backend + frontend independently reviewable). | ||
| - Different reviewers needed (infra/CI vs. product). | ||
| - Different risk profiles (low-risk config + high-risk feature). | ||
| - Revertable in isolation. |
There was a problem hiding this comment.
P3: "Revertable" is non-standard; "revertible" is the expected adjective (able to be reverted). Same pattern as convertible, reversible.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .agents/skills/shipping-features/phases/pr-split.md, line 26:
<comment>"Revertable" is non-standard; "revertible" is the expected adjective (able to be reverted). Same pattern as convertible, reversible.</comment>
<file context>
@@ -0,0 +1,39 @@
+ - Independent concerns (unrelated drive-by refactor, or backend + frontend independently reviewable).
+ - Different reviewers needed (infra/CI vs. product).
+ - Different risk profiles (low-risk config + high-risk feature).
+ - Revertable in isolation.
+- **Do NOT recommend a split when:**
+ - ❌ Changes are coupled (feature + its own tests + its own docs).
</file context>
| - Guidelines: `dev/guidelines/`, `docs/guidelines/`, `guidelines/` | ||
| - Guides: `dev/guides/`, `docs/guides/`, `guides/` | ||
|
|
||
| ```bash |
There was a problem hiding this comment.
P3: The discovery shell commands search under dev docs but miss the root-level paths (knowledge/, guidelines/, guides/) listed as valid locations in the same section. The fallback chain ultimately catches these via AGENTS.md or the user prompt, so discovery won't break — but the inconsistency could cause an unnecessary extra turn in projects that use root-level doc directories. Consider widening the search to include . or the specific root-level paths alongside dev docs so the probe matches all the locations the text describes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .agents/skills/capturing-knowledge/SKILL.md, line 68:
<comment>The discovery shell commands search under `dev docs` but miss the root-level paths (`knowledge/`, `guidelines/`, `guides/`) listed as valid locations in the same section. The fallback chain ultimately catches these via AGENTS.md or the user prompt, so discovery won't break — but the inconsistency could cause an unnecessary extra turn in projects that use root-level doc directories. Consider widening the search to include `.` or the specific root-level paths alongside `dev docs` so the probe matches all the locations the text describes.</comment>
<file context>
@@ -0,0 +1,177 @@
+ - Guidelines: `dev/guidelines/`, `docs/guidelines/`, `guidelines/`
+ - Guides: `dev/guides/`, `docs/guides/`, `guides/`
+
+ ```bash
+ fd -t d 'knowledge|guidelines|guides' dev docs 2>/dev/null || \
+ find dev docs -type d \( -name knowledge -o -name guidelines -o -name guides \) 2>/dev/null
</file context>
| | Worktree isolation | — | `superpowers:using-git-worktrees` | `isolation: "worktree"` on Agent calls | | ||
| | Parallel execution | — | `superpowers:subagent-driven-development` / `dispatching-parallel-agents` | parallel Agent calls in one message | | ||
| | Code review | `speckit-review-run` (or per-lens `speckit-review-{code,tests,types,errors,comments}`), `speckit-review-simplify`, `speckit-critique-run` | `coderabbit:code-review`, `code-simplifier`, `superpowers:requesting-code-review` | `general-purpose` reviewers + `/security-review` | | ||
| | Residue cleanup (post-implement) | `pruning-residues` | `code-simplifier` | agent prunes leftover debug logs, dead code, orphaned files/imports, and redundant comments; else skip | |
There was a problem hiding this comment.
P3: The discovery table lists pruning-residues as an in-repo tool for residue cleanup, and Phase 4 instructs the agent to run it first. However, no pruning-residues skill or command exists anywhere under .agents/. The priority chain handles this via fallbacks (code-simplifier → manual agent cleanup) and the table advises reconciling against .agents/, so it degrades gracefully. Still, the column heading claims this is 'what actually ships here today,' which is inaccurate for this entry. Consider either adding the skill or marking the entry with a note (e.g., 'planned' or 'via fallback only') to avoid confusing agents.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .agents/skills/shipping-features/SKILL.md, line 95:
<comment>The discovery table lists `pruning-residues` as an in-repo tool for residue cleanup, and Phase 4 instructs the agent to run it first. However, no `pruning-residues` skill or command exists anywhere under `.agents/`. The priority chain handles this via fallbacks (`code-simplifier` → manual agent cleanup) and the table advises reconciling against `.agents/`, so it degrades gracefully. Still, the column heading claims this is 'what actually ships here today,' which is inaccurate for this entry. Consider either adding the skill or marking the entry with a note (e.g., 'planned' or 'via fallback only') to avoid confusing agents.</comment>
<file context>
@@ -0,0 +1,259 @@
+| Worktree isolation | — | `superpowers:using-git-worktrees` | `isolation: "worktree"` on Agent calls |
+| Parallel execution | — | `superpowers:subagent-driven-development` / `dispatching-parallel-agents` | parallel Agent calls in one message |
+| Code review | `speckit-review-run` (or per-lens `speckit-review-{code,tests,types,errors,comments}`), `speckit-review-simplify`, `speckit-critique-run` | `coderabbit:code-review`, `code-simplifier`, `superpowers:requesting-code-review` | `general-purpose` reviewers + `/security-review` |
+| Residue cleanup (post-implement) | `pruning-residues` | `code-simplifier` | agent prunes leftover debug logs, dead code, orphaned files/imports, and redundant comments; else skip |
+| Knowledge capture | `capturing-knowledge` | — | skip |
+| Learn from review | `learning-from-review` | — | skip |
</file context>
| | 3 | Plan | `/speckit-plan`+`/speckit-tasks` | gate + parallel (+ skeptic on risk) | | ||
| | 4 | Implement | `/bug-tdd`+`/bug-fix` / TDD agents · prune residues | gate + adversarial verify | | ||
| | 5 | Review | `speckit-review-run`, `coderabbit`, `/security-review` | gate + parallel + verify | | ||
| | 6 | Knowledge, learning & retrospective | `capturing-knowledge` + `learning-from-review` + `speckit-opsmill-retrospect` + `/audit-docs`→`/add-docs` | conditional | |
There was a problem hiding this comment.
P3: The pipeline table in overview-and-design.md lists the Phase 6 sub-steps in a different order than SKILL.md Phase 6 defines them. The overview has retrospective before audit-docs, but the SKILL.md runs audit-docs (step 3) before retrospective (step 4). Reorder the overview table to match the authoritative phase definition.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .agents/skills/shipping-features/docs/overview-and-design.md, line 88:
<comment>The pipeline table in overview-and-design.md lists the Phase 6 sub-steps in a different order than SKILL.md Phase 6 defines them. The overview has retrospective before audit-docs, but the SKILL.md runs audit-docs (step 3) before retrospective (step 4). Reorder the overview table to match the authoritative phase definition.</comment>
<file context>
@@ -0,0 +1,111 @@
+| 3 | Plan | `/speckit-plan`+`/speckit-tasks` | gate + parallel (+ skeptic on risk) |
+| 4 | Implement | `/bug-tdd`+`/bug-fix` / TDD agents · prune residues | gate + adversarial verify |
+| 5 | Review | `speckit-review-run`, `coderabbit`, `/security-review` | gate + parallel + verify |
+| 6 | Knowledge, learning & retrospective | `capturing-knowledge` + `learning-from-review` + `speckit-opsmill-retrospect` + `/audit-docs`→`/add-docs` | conditional |
+| 7 | CI gate | `/pre-ci` | gate |
+| 8 | Commit & PR | `commit`, `pr`, split assessment | parallel (split) |
</file context>
|
done in #9873 |
Summary
Adds the agent skills suite under
.agents/skills/(none of these exist ondevelopyet):phases/references (classification, manifest, pr-split, reliability) and a design overview.capturing-knowledge. Works from a PR (primary) or the current conversation (fallback), and requires cited evidence (PR#, review comment, before/after hunk) for every lesson.Why
Review feedback is where an agent's mistakes get corrected, but that learning normally evaporates — the next agent repeats the rejected shape.
learning-from-reviewcaptures it as a preventive lesson so the agent gets it right the first time next round.Wiring & overlap
learning-from-reviewis wired as step 2 of shipping-features' phase 6 (the knowledge step). The phase draws an explicit three-way boundary so the tools don't overlap:capturing-knowledgelearning-from-reviewspeckit-opsmill-retrospectScope notes
developare touched.cut-release.mdrefactor and apython_sdksubmodule pointer change that live on the source branch are intentionally excluded.opsmill-skillsplugin repo (deterministic grader RED 0.0 / GREEN 1.0;skillgrade --smokepass@5 = 100%). The grader/eval harness lives there, not in this repo.🤖 Generated with Claude Code
Summary by cubic
Adds three new agent skills:
shipping-features(end‑to‑end, resumable conductor with aship.mdmanifest and reliability gates),capturing-knowledge(persists non‑obvious domain facts with user confirmation), andlearning-from-review(turns PR review deltas into durable, cited lessons routed tocapturing-knowledge). This enables repeatable feature delivery, preserves learnings, and improves review quality.New Features
shipping-features— orchestrates spec → plan → implement → review → CI → PR; classifies work, writes aship.mdmanifest, gates each phase, assesses PR splitting, and watches CI. Phase 6 wires tocapturing-knowledgeandlearning-from-review.capturing-knowledge— finds the project’s knowledge/guidelines/guides docs, dedupes, drafts the smallest edit, and only writes after approval.learning-from-review— links review comments to corrections, filters noise, requires cited evidence (PR, comment, before/after hunk), and delegates persistence tocapturing-knowledge.Migration
superpowers,coderabbit,commit-commands,code-simplifier.shipping-features <ticket>or invokingcapturing-knowledge/learning-from-reviewdirectly.Written for commit 84456f6. Summary will update on new commits.