mirror of
https://github.com/obra/superpowers.git
synced 2026-06-10 20:59:05 +08:00
Compare commits
1 Commits
sdd-review
...
codex/shel
| Author | SHA1 | Date | |
|---|---|---|---|
|
867238cfc1 |
7
.github/ISSUE_TEMPLATE/bug_report.md
vendored
7
.github/ISSUE_TEMPLATE/bug_report.md
vendored
@@ -12,17 +12,14 @@ add a comment or reaction to the existing one instead.
|
||||
|
||||
- [ ] I searched existing issues and this is not a duplicate
|
||||
|
||||
## Environment (required)
|
||||
<!-- Required. We assume an agent filed this report — tell us which one and
|
||||
where it ran. We weigh reports by what produced them. -->
|
||||
## Environment
|
||||
|
||||
| Field | Value |
|
||||
|-------|-------|
|
||||
| Superpowers version | |
|
||||
| Harness (Claude Code, Cursor, etc.) | |
|
||||
| Harness version | |
|
||||
| Your model + version | |
|
||||
| All plugins installed | |
|
||||
| Model | |
|
||||
| OS + shell | |
|
||||
|
||||
## Is this a Superpowers issue or a platform issue?
|
||||
|
||||
15
.github/ISSUE_TEMPLATE/feature_request.md
vendored
15
.github/ISSUE_TEMPLATE/feature_request.md
vendored
@@ -30,18 +30,5 @@ progress, and some were intentionally declined.
|
||||
of project? If this is specific to your domain, workflow, or a
|
||||
third-party tool, it may belong as its own plugin instead. -->
|
||||
|
||||
## Environment (required)
|
||||
<!-- Required. We assume an agent wrote this request — tell us which one and
|
||||
where it ran. We weigh proposals reasoned from documentation differently
|
||||
than ones grounded in a real session where the problem actually came up. -->
|
||||
|
||||
| Field | Value |
|
||||
|-------|-------|
|
||||
| Superpowers version | |
|
||||
| Harness (Claude Code, Cursor, etc.) | |
|
||||
| Harness version | |
|
||||
| Your model + version | |
|
||||
| All plugins installed | |
|
||||
|
||||
## Context
|
||||
<!-- Optional: the workflow where you hit this, links, transcripts. -->
|
||||
<!-- Optional: version info, harness, model, workflow where you hit this. -->
|
||||
|
||||
11
.github/ISSUE_TEMPLATE/platform_support.md
vendored
11
.github/ISSUE_TEMPLATE/platform_support.md
vendored
@@ -21,14 +21,3 @@ requested or discussed.
|
||||
## Have you tried manual installation?
|
||||
<!-- Many tools work with Superpowers through manual setup even without
|
||||
official support. Did you try? What happened? -->
|
||||
|
||||
## Environment (required)
|
||||
<!-- Required. We assume an agent wrote this request — tell us which one and
|
||||
where it ran. -->
|
||||
|
||||
| Field | Value |
|
||||
|-------|-------|
|
||||
| Harness you currently use (Claude Code, Cursor, etc.) | |
|
||||
| Harness version | |
|
||||
| Your model + version | |
|
||||
| All plugins installed | |
|
||||
|
||||
17
.github/PULL_REQUEST_TEMPLATE.md
vendored
17
.github/PULL_REQUEST_TEMPLATE.md
vendored
@@ -4,23 +4,6 @@ sections blank, contain multiple unrelated changes, or show no evidence
|
||||
of human involvement will be closed without review.
|
||||
-->
|
||||
|
||||
> **This PR MUST target the `dev` branch, not `main`.** `main` is the
|
||||
> released branch; active work lands on `dev` first. PRs opened against
|
||||
> `main` will be asked to retarget `dev` before review.
|
||||
|
||||
## Who is submitting this PR? (required)
|
||||
<!-- Required. PRs that omit this will be closed. We assume an agent wrote
|
||||
this PR — tell us which one and where it ran. We weigh contributions by
|
||||
what produced them: content reasoned from documentation is held to a
|
||||
different bar than work grounded in a real session. -->
|
||||
|
||||
| Field | Value |
|
||||
|-------|-------|
|
||||
| Your model + version | |
|
||||
| Harness + version | |
|
||||
| All plugins installed | |
|
||||
| Human partner who reviewed this diff | |
|
||||
|
||||
## What problem are you trying to solve?
|
||||
<!-- Describe the specific problem you encountered. If this was a session
|
||||
issue, include: what you were doing, what went wrong, the model's
|
||||
|
||||
@@ -1,38 +0,0 @@
|
||||
{
|
||||
"name": "superpowers",
|
||||
"version": "5.1.0",
|
||||
"description": "An agentic skills framework and software development methodology.",
|
||||
"author": {
|
||||
"name": "Jesse Vincent",
|
||||
"email": "jesse@fsck.com"
|
||||
},
|
||||
"homepage": "https://github.com/obra/superpowers",
|
||||
"license": "MIT",
|
||||
"keywords": [
|
||||
"brainstorming",
|
||||
"subagent-driven-development",
|
||||
"skills",
|
||||
"planning",
|
||||
"tdd",
|
||||
"debugging",
|
||||
"code-review",
|
||||
"workflow"
|
||||
],
|
||||
"skills": "./skills/",
|
||||
"sessionStart": {
|
||||
"skill": "using-superpowers"
|
||||
},
|
||||
"skillInstructions": "Kimi Code tool mapping for Superpowers skills:\n\n- When a Superpowers skill says to ask the user, ask clarifying questions, ask one question at a time, present multiple-choice options, use the terminal for a question, or wait for the user's choice, call Kimi Code's `AskUserQuestion` tool. Do not render those choices as plain assistant text unless `AskUserQuestion` is unavailable or the session is in auto permission mode.\n- For `AskUserQuestion`, provide 1 question with 2-4 concrete options when possible. Put the recommended option first and suffix its label with `(Recommended)`.\n- When a Superpowers skill refers to `TodoWrite`, use Kimi Code's `TodoList` tool.\n- When a Superpowers skill says `Task tool (general-purpose)` or asks you to dispatch an implementer/reviewer subagent, use Kimi Code's `Agent` tool with a Kimi subagent type. Do not pass `general-purpose` as `subagent_type`.\n- For implementation, code review, spec review, quality review, and filled Superpowers subagent prompt templates, call `Agent` with `subagent_type: \"coder\"`, paste the fully filled prompt into `prompt`, and provide a short `description`.\n- For read-only codebase exploration that would take several searches, use `Agent` with `subagent_type: \"explore\"`.\n- For read-only planning or architecture design, use `Agent` with `subagent_type: \"plan\"`.\n- Keep dependent Superpowers subagent steps sequential. Use multiple `Agent` calls, or `run_in_background: true` only when the work is independent and background agents are available.\n- When a Superpowers skill refers to the `Skill` tool, use Kimi Code's native `Skill` tool.\n- Use Kimi Code's `Read`, `Write`, `Edit`, `Bash`, `Grep`, `Glob`, `FetchURL`, `WebSearch`, and MCP tools by their actual exposed names.\n- When a skill asks to search file contents, use `Grep`; when it asks to find files by path or pattern, use `Glob`; when it asks to fetch a URL, use `FetchURL`; when it asks to search the web, use `WebSearch`.",
|
||||
"interface": {
|
||||
"displayName": "Superpowers",
|
||||
"shortDescription": "Planning, TDD, debugging, and delivery workflows for coding agents",
|
||||
"longDescription": "Use Superpowers to guide agent work through brainstorming, implementation planning, test-driven development, systematic debugging, parallel execution, code review, and finish-the-branch workflows.",
|
||||
"developerName": "Jesse Vincent",
|
||||
"capabilities": [
|
||||
"Interactive",
|
||||
"Read",
|
||||
"Write"
|
||||
],
|
||||
"websiteURL": "https://github.com/obra/superpowers"
|
||||
}
|
||||
}
|
||||
@@ -4,7 +4,6 @@
|
||||
{ "path": ".claude-plugin/plugin.json", "field": "version" },
|
||||
{ "path": ".cursor-plugin/plugin.json", "field": "version" },
|
||||
{ "path": ".codex-plugin/plugin.json", "field": "version" },
|
||||
{ "path": ".kimi-plugin/plugin.json", "field": "version" },
|
||||
{ "path": ".claude-plugin/marketplace.json", "field": "plugins.0.version" },
|
||||
{ "path": "gemini-extension.json", "field": "version" }
|
||||
],
|
||||
|
||||
@@ -14,8 +14,7 @@ Before you open a PR against this repo, you MUST:
|
||||
2. **Search for existing PRs** — open AND closed — that address the same problem. If duplicates exist, STOP and tell your human partner. Do not open another duplicate.
|
||||
3. **Verify this is a real problem.** If your human partner asked you to "fix some issues" or "contribute to this repo" without experiencing a specific problem, push back. Ask them what broke, what failed, what the user experience was.
|
||||
4. **Confirm the change belongs in core.** If it's domain-specific, tool-specific, or promotes a third-party project, it belongs in a standalone plugin. Tell your human partner.
|
||||
5. **Identify yourself.** Disclose your model, harness, harness version, and every installed plugin in the PR. Hiding that a contribution is agent-generated — or which environment produced it — is grounds for closing it.
|
||||
6. **Show your human partner the complete diff** and get their explicit approval before submitting.
|
||||
5. **Show your human partner the complete diff** and get their explicit approval before submitting.
|
||||
|
||||
If any of these checks fail, do not open the PR. Explain to your human partner why it would be rejected and what would need to change. They will thank you for saving them the embarrassment.
|
||||
|
||||
@@ -27,10 +26,6 @@ If any of these checks fail, do not open the PR. Explain to your human partner w
|
||||
|
||||
**PRs that show no evidence of human involvement will be closed.** A human must review the complete proposed diff before submission.
|
||||
|
||||
**Submitters MUST identify themselves.** Every PR and issue must disclose the model, harness, harness version, and all installed plugins used to produce the contribution — or state plainly that it was written by hand with no agent. This is not optional. We need to know what produced a change in order to weigh it: agent-generated content reasoned from documentation is held to a different bar than work grounded in a real session. Contributions that hide their authoring environment will be closed.
|
||||
|
||||
**All PRs MUST target the `dev` branch, not `main`.** `main` is the released branch; active work lands on `dev` first. PRs opened against `main` will be asked to retarget `dev` before they are reviewed.
|
||||
|
||||
## What We Will Not Accept
|
||||
|
||||
### Third-party dependencies
|
||||
|
||||
22
README.md
22
README.md
@@ -4,7 +4,7 @@ Superpowers is a complete software development methodology for your coding agent
|
||||
|
||||
## Quickstart
|
||||
|
||||
Give your agent Superpowers: [Claude Code](#claude-code), [Antigravity](#antigravity), [Codex App](#codex-app), [Codex CLI](#codex-cli), [Cursor](#cursor), [Factory Droid](#factory-droid), [Gemini CLI](#gemini-cli), [GitHub Copilot CLI](#github-copilot-cli), [Kimi Code](#kimi-code), [OpenCode](#opencode), [Pi](#pi).
|
||||
Give your agent Superpowers: [Claude Code](#claude-code), [Antigravity](#antigravity), [Codex App](#codex-app), [Codex CLI](#codex-cli), [Cursor](#cursor), [Factory Droid](#factory-droid), [Gemini CLI](#gemini-cli), [GitHub Copilot CLI](#github-copilot-cli), [OpenCode](#opencode), [Pi](#pi).
|
||||
|
||||
## How it works
|
||||
|
||||
@@ -149,26 +149,6 @@ Superpowers is available via the [official Codex plugin marketplace](https://git
|
||||
copilot plugin install superpowers@superpowers-marketplace
|
||||
```
|
||||
|
||||
### Kimi Code
|
||||
|
||||
Superpowers is available in Kimi Code's plugin marketplace.
|
||||
|
||||
- Open Kimi Code's plugin manager:
|
||||
|
||||
```text
|
||||
/plugins
|
||||
```
|
||||
|
||||
- Go to `Marketplace` > `Superpowers` and install it.
|
||||
|
||||
- Or install directly from this repository:
|
||||
|
||||
```text
|
||||
/plugins install https://github.com/obra/superpowers
|
||||
```
|
||||
|
||||
- Detailed docs: [docs/README.kimi.md](docs/README.kimi.md)
|
||||
|
||||
### OpenCode
|
||||
|
||||
OpenCode uses its own plugin install; install Superpowers separately even if you
|
||||
|
||||
@@ -1,88 +0,0 @@
|
||||
# Superpowers for Kimi Code
|
||||
|
||||
Complete guide for using Superpowers with [Kimi Code](https://github.com/MoonshotAI/kimi-code).
|
||||
|
||||
## Installation
|
||||
|
||||
Superpowers is available in Kimi Code's plugin marketplace.
|
||||
|
||||
Open the plugin manager:
|
||||
|
||||
```text
|
||||
/plugins
|
||||
```
|
||||
|
||||
Go to `Marketplace` > `Superpowers` and install it.
|
||||
|
||||
You can also install from this repository:
|
||||
|
||||
```text
|
||||
/plugins install https://github.com/obra/superpowers
|
||||
```
|
||||
|
||||
For unreleased validation against `dev`, pin the branch explicitly:
|
||||
|
||||
```text
|
||||
/plugins install https://github.com/obra/superpowers/tree/dev
|
||||
```
|
||||
|
||||
Kimi Code applies plugin changes to new sessions. After installing, updating, enabling, disabling, or reloading a plugin, start a fresh session with `/new`.
|
||||
|
||||
## How It Works
|
||||
|
||||
The Kimi plugin manifest lives at `.kimi-plugin/plugin.json`.
|
||||
|
||||
The manifest does three things:
|
||||
|
||||
1. Points Kimi Code at the existing `skills/` directory.
|
||||
2. Loads `using-superpowers` at session start through `sessionStart.skill`.
|
||||
3. Provides Kimi-specific tool mapping through `skillInstructions`.
|
||||
|
||||
Kimi Code reads Superpowers skills from this repository. There are no copied skills, symlinks, hooks, or extra runtime dependencies.
|
||||
|
||||
## Tool Mapping
|
||||
|
||||
Skills describe actions instead of hard-coding one runtime's tool names. On Kimi Code these resolve to:
|
||||
|
||||
- "Ask the user" / "ask clarifying questions" -> `AskUserQuestion`
|
||||
- "Create a todo" / "mark complete in todo list" -> `TodoList`
|
||||
- "Dispatch a subagent" -> `Agent`
|
||||
- "Invoke a skill" -> Kimi Code's native `Skill` tool
|
||||
- "Read a file" / "write a file" / "edit a file" -> `Read`, `Write`, `Edit`
|
||||
- "Run a shell command" -> `Bash`
|
||||
- "Search file contents" -> `Grep`
|
||||
- "Find files by path or pattern" -> `Glob`
|
||||
- "Fetch a URL" -> `FetchURL`
|
||||
- "Search the web" -> `WebSearch`
|
||||
|
||||
## Updating
|
||||
|
||||
Use Kimi Code's plugin manager:
|
||||
|
||||
```text
|
||||
/plugins
|
||||
```
|
||||
|
||||
Select Superpowers and update it from there. Start a fresh session with `/new` after updating.
|
||||
|
||||
## Troubleshooting
|
||||
|
||||
### Plugin not loading
|
||||
|
||||
1. Run `/plugins info superpowers` and check diagnostics.
|
||||
2. Make sure the plugin is enabled.
|
||||
3. Start a fresh session with `/new` after install or update.
|
||||
|
||||
### Direct GitHub install used an old release
|
||||
|
||||
Kimi Code installs the latest GitHub release for a bare repository URL when one exists. To test unreleased changes before the next Superpowers release, install the branch explicitly:
|
||||
|
||||
```text
|
||||
/plugins install https://github.com/obra/superpowers/tree/dev
|
||||
```
|
||||
|
||||
### Skills not triggering
|
||||
|
||||
1. Confirm `/plugins info superpowers` shows the plugin enabled.
|
||||
2. Start a fresh session with `/new`.
|
||||
3. Try the acceptance prompt: `Let's make a react todo list`. A working install should load `brainstorming` before writing code.
|
||||
@@ -675,7 +675,7 @@ it. Distribution differs per harness ecosystem — find yours:
|
||||
|---|---|---|
|
||||
| Native plugin marketplace | Claude Code | Register in `.claude-plugin/marketplace.json`; users `/plugin install`. The external `superpowers-marketplace` repo is the source of truth users install from — see the release steps in `CLAUDE.md`. |
|
||||
| External marketplace fork, synced by script | Codex | `scripts/sync-to-codex-plugin.sh` rsyncs the tracked plugin files into a separate fork repo and opens a PR. Read its include/exclude list so you ship the right tree (it deliberately drops repo-internal dirs and other harnesses' dotdirs). |
|
||||
| Git-URL extension install | Gemini, Kimi Code, OpenCode | Users install from a git URL (`gemini extensions install …`; Kimi Code `/plugins install …`; an `opencode.json` `plugin` array entry). Document the exact command. |
|
||||
| Git-URL extension install | Gemini, OpenCode | Users install from a git URL (`gemini extensions install …`; an `opencode.json` `plugin` array entry). Document the exact command. |
|
||||
| Package-manifest fields | pi | Declared through fields in the repo-root `package.json`; users install via the harness's package command. |
|
||||
| Local installer (plugin install) | Antigravity (`agy`) | A small `install.sh` that runs the harness's own `agy plugin install` against a staging dir holding the manifest, the skills, and a generated `contextFileName` context file (the bootstrap). Everything arrives through the install mechanism — *not* by editing the user's config (see below). |
|
||||
|
||||
@@ -755,9 +755,10 @@ Two rules this enforces, which you must respect:
|
||||
- Don't write per-OS variants of the hook script. One extensionless bash script
|
||||
plus the polyglot wrapper covers all three platforms.
|
||||
|
||||
`hooks/run-hook.cmd` itself is the authoritative implementation — read it. See
|
||||
`docs/windows/polyglot-hooks.md` for the background and rationale behind the
|
||||
dispatcher pattern.
|
||||
`hooks/run-hook.cmd` itself is the authoritative implementation — read it.
|
||||
(`docs/windows/polyglot-hooks.md` covers the background and rationale but
|
||||
describes an earlier per-script `.cmd`/`.sh` variant, so trust the code over that
|
||||
doc where they differ.)
|
||||
|
||||
---
|
||||
|
||||
@@ -788,7 +789,6 @@ Use this as the live index; when in doubt, read the files, not this table.
|
||||
| Cursor | `.cursor-plugin/plugin.json` + `hooks/hooks-cursor.json` | shell hook → `hooks/session-start` (`additional_context`) | `references/claude-code-tools.md` | `tests/hooks/` | hand-authored |
|
||||
| Copilot CLI | (shares Claude Code hook path; `COPILOT_CLI` env) | shell hook → `hooks/session-start` (`additionalContext`) | `references/copilot-tools.md` | `tests/hooks/` | — |
|
||||
| Gemini CLI | `gemini-extension.json` + `GEMINI.md` | instructions file `@`-includes bootstrap + mapping | `references/gemini-tools.md` | — | `gemini extensions install` |
|
||||
| Kimi Code | `.kimi-plugin/plugin.json` | manifest `sessionStart.skill` loads `using-superpowers` | inline `skillInstructions` in manifest | `tests/kimi/` | marketplace or `/plugins install` GitHub URL |
|
||||
| OpenCode | `.opencode/plugins/superpowers.js` (declared via root `package.json` `main`) | in-process: `config` hook registers skills dir; `experimental.chat.messages.transform` injects user message | inline in `superpowers.js` | `tests/opencode/` | `opencode.json` plugin git URL |
|
||||
| pi | `.pi/extensions/superpowers.ts` | in-process: `resources_discover` registers skills; `context` event injects user message; lifecycle-flag + compaction-aware | `piToolMapping()` inline **and** `references/pi-tools.md` | `tests/pi/` | repo-root `package.json` fields |
|
||||
|
||||
|
||||
@@ -1,774 +0,0 @@
|
||||
# SDD Task-Scoped Review Dispatch Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Scope SDD's per-task reviews to the task (diff-first reading, justified broadening, no redundant test runs) while final branch review stays broad.
|
||||
|
||||
**Architecture:** Four prose edits to the subagent-driven-development skill (the per-task quality prompt becomes self-contained instead of delegating to the merge-readiness template; the spec prompt gets a third verdict channel and grounded skepticism; the implementer prompt gains a re-run-after-fix rule; SKILL.md gets controller guidance) plus one new eval scenario in the `evals/` submodule. `skills/requesting-code-review/` is deliberately untouched.
|
||||
|
||||
**Tech Stack:** Markdown skill files; Python setup helper + bash checks + story.md for the quorum eval.
|
||||
|
||||
**Spec:** `docs/superpowers/specs/2026-06-09-sdd-task-scoped-review-dispatch-design.md` — read it before starting. Decisions already settled there: full re-reviews stay; the two review stages stay separate; coordinator keeps model judgment; `requesting-code-review/` stays broad.
|
||||
|
||||
**These are behavior-shaping prose files, not code.** There are no unit tests for them. Each task's verification steps are exact `grep` checks that the edit landed; behavioral verification is Task 6 (static) and Task 7 (live evals, maintainer-gated).
|
||||
|
||||
---
|
||||
|
||||
### Task 1: Rewrite the per-task quality reviewer prompt as self-contained
|
||||
|
||||
The current file delegates to `../requesting-code-review/code-reviewer.md`, which is a merge-readiness review (architecture, security, production readiness, "Ready to merge?"). Replace the entire file with a self-contained, task-scoped template.
|
||||
|
||||
**Files:**
|
||||
- Rewrite: `skills/subagent-driven-development/code-quality-reviewer-prompt.md`
|
||||
|
||||
- [ ] **Step 1: Replace the full file contents with:**
|
||||
|
||||
````markdown
|
||||
# Code Quality Reviewer Prompt Template
|
||||
|
||||
Use this template when dispatching a code quality reviewer subagent.
|
||||
|
||||
**Purpose:** Verify one task's implementation is well-built (clean, tested, maintainable)
|
||||
|
||||
**Only dispatch after spec compliance review passes.**
|
||||
|
||||
```
|
||||
Subagent (general-purpose):
|
||||
description: "Review code quality for Task N"
|
||||
prompt: |
|
||||
You are reviewing one task's implementation for code quality. This is a
|
||||
task-scoped gate, not a merge review — a broad whole-branch review happens
|
||||
separately after all tasks are complete.
|
||||
|
||||
## What Was Implemented
|
||||
|
||||
[DESCRIPTION]
|
||||
|
||||
## Task Requirements (context only)
|
||||
|
||||
[TASK_TEXT]
|
||||
|
||||
## Git Range to Review
|
||||
|
||||
**Base:** [BASE_SHA]
|
||||
**Head:** [HEAD_SHA]
|
||||
|
||||
```bash
|
||||
git diff --stat [BASE_SHA]..[HEAD_SHA]
|
||||
git diff [BASE_SHA]..[HEAD_SHA]
|
||||
```
|
||||
|
||||
## Read-Only Review
|
||||
|
||||
Your review is read-only on this checkout. Do not mutate the working tree,
|
||||
the index, HEAD, or branch state in any way. Use tools like `git show`,
|
||||
`git diff`, and `git log` to inspect history.
|
||||
|
||||
## Scope
|
||||
|
||||
Spec compliance was already verified by a separate reviewer. Do not
|
||||
re-check whether the code matches the requirements or the plan.
|
||||
|
||||
Start from the diff. Read the changed files first. Inspect code outside
|
||||
the diff only to evaluate a concrete risk you can name — and name it in
|
||||
your report. Cross-cutting changes are legitimate named risks: if the
|
||||
diff changes lock ordering, a function or API contract, or shared mutable
|
||||
state, checking the call sites is the right method. Do not crawl the
|
||||
codebase by default.
|
||||
|
||||
## Tests
|
||||
|
||||
The implementer already ran the tests and reported results with TDD
|
||||
evidence for exactly this code. Do not re-run the suite to confirm their
|
||||
report. Run a test only when reading the code raises a specific doubt
|
||||
that no existing run answers — and then a focused test, never a
|
||||
package-wide suite, race detector run, or repeated/high-count loop. If
|
||||
heavy validation seems warranted, recommend it in your report instead of
|
||||
running it. If you cannot run commands in this environment, name the
|
||||
test you would run.
|
||||
|
||||
## What to Check
|
||||
|
||||
**Code quality:**
|
||||
- Clean separation of concerns?
|
||||
- Proper error handling?
|
||||
- DRY without premature abstraction?
|
||||
- Edge cases handled?
|
||||
|
||||
**Tests:**
|
||||
- Do the new and changed tests verify real behavior, not mocks?
|
||||
- Are the task's edge cases covered?
|
||||
|
||||
**Structure:**
|
||||
- Does each file have one clear responsibility with a well-defined interface?
|
||||
- Are units decomposed so they can be understood and tested independently?
|
||||
- Is the implementation following the file structure from the plan?
|
||||
- Did this change create new files that are already large, or
|
||||
significantly grow existing files? (Don't flag pre-existing file
|
||||
sizes — focus on what this change contributed.)
|
||||
|
||||
## Calibration
|
||||
|
||||
Categorize issues by actual severity. Not everything is Critical.
|
||||
Acknowledge what was done well before listing issues — accurate praise
|
||||
helps the implementer trust the rest of the feedback.
|
||||
|
||||
## Output Format
|
||||
|
||||
### Strengths
|
||||
[What's well done? Be specific.]
|
||||
|
||||
### Issues
|
||||
|
||||
#### Critical (Must Fix)
|
||||
[Bugs, data loss risks, broken functionality]
|
||||
|
||||
#### Important (Should Fix)
|
||||
[Poor error handling, test gaps, structural problems]
|
||||
|
||||
#### Minor (Nice to Have)
|
||||
[Code style, optimization opportunities]
|
||||
|
||||
For each issue:
|
||||
- File:line reference
|
||||
- What's wrong
|
||||
- Why it matters
|
||||
- How to fix (if not obvious)
|
||||
|
||||
### Assessment
|
||||
|
||||
**Task quality:** [Approved | Needs fixes]
|
||||
|
||||
**Reasoning:** [1-2 sentence technical assessment]
|
||||
```
|
||||
|
||||
**Placeholders:**
|
||||
- `[DESCRIPTION]` — task summary, from implementer's report
|
||||
- `[TASK_TEXT]` — the task's requirements text or plan reference, for context
|
||||
- `[BASE_SHA]` — commit before this task
|
||||
- `[HEAD_SHA]` — current commit
|
||||
|
||||
**Reviewer returns:** Strengths, Issues (Critical/Important/Minor), Task quality verdict
|
||||
````
|
||||
|
||||
- [ ] **Step 2: Verify the rewrite landed**
|
||||
|
||||
Run: `grep -c "requesting-code-review" skills/subagent-driven-development/code-quality-reviewer-prompt.md || echo ABSENT`
|
||||
Expected: `ABSENT` (no more delegation)
|
||||
|
||||
Run: `grep -n "Task quality:" skills/subagent-driven-development/code-quality-reviewer-prompt.md | head -2`
|
||||
Expected: one match (the Output Format verdict line; the "Reviewer returns" footer says "Task quality verdict" without a colon)
|
||||
|
||||
Run: `grep -n "worktree add\|Ready to merge" skills/subagent-driven-development/code-quality-reviewer-prompt.md || echo CLEAN`
|
||||
Expected: `CLEAN`
|
||||
|
||||
- [ ] **Step 3: Commit**
|
||||
|
||||
```bash
|
||||
git add skills/subagent-driven-development/code-quality-reviewer-prompt.md
|
||||
git commit -m "Make per-task quality reviewer prompt self-contained and task-scoped"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 2: Spec reviewer prompt cleanups
|
||||
|
||||
Four exact edits to `skills/subagent-driven-development/spec-reviewer-prompt.md`. Current line numbers refer to the file as of commit f55642e.
|
||||
|
||||
**Files:**
|
||||
- Modify: `skills/subagent-driven-development/spec-reviewer-prompt.md`
|
||||
|
||||
- [ ] **Step 1: Add the judge-from-the-diff clause.** After the line (currently line 31):
|
||||
|
||||
```
|
||||
Only read files in this diff. Do not crawl the broader codebase.
|
||||
```
|
||||
|
||||
insert a blank line and:
|
||||
|
||||
```
|
||||
Spec compliance is judged by reading the diff against the requirements.
|
||||
The implementer already ran the tests and reported TDD evidence — do not
|
||||
re-run them. If a requirement cannot be verified from this diff alone
|
||||
(it lives in unchanged code or spans tasks), report it as a ⚠️ item
|
||||
instead of broadening your search.
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Trim the read-only section.** Replace (currently line 35):
|
||||
|
||||
```
|
||||
Your review is read-only on this checkout. Do not mutate the working tree, the index, HEAD, or branch state in any way. Use tools like `git show`, `git diff`, and `git log` to inspect history. If you need a working copy of a different revision, check it out into a separate temporary directory (e.g. `git worktree add /tmp/review-[SHA] [SHA]`) — never move HEAD on this checkout.
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```
|
||||
Your review is read-only on this checkout. Do not mutate the working tree, the index, HEAD, or branch state in any way. Use tools like `git show`, `git diff`, and `git log` to inspect history.
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Ground the skepticism.** Replace (currently lines 39-40):
|
||||
|
||||
```
|
||||
The implementer finished suspiciously quickly. Their report may be incomplete,
|
||||
inaccurate, or optimistic. You MUST verify everything independently.
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```
|
||||
Treat the implementer's report as unverified claims about the code. It may
|
||||
be incomplete, inaccurate, or optimistic. Verify the claims against the diff.
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Add the third verdict channel.** Replace (currently lines 74-76):
|
||||
|
||||
```
|
||||
Report:
|
||||
- ✅ Spec compliant (if everything matches after code inspection)
|
||||
- ❌ Issues found: [list specifically what's missing or extra, with file:line references]
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```
|
||||
Report:
|
||||
- ✅ Spec compliant (if everything matches after code inspection)
|
||||
- ❌ Issues found: [list specifically what's missing or extra, with file:line references]
|
||||
- ⚠️ Cannot verify from diff: [requirements you could not verify from the
|
||||
diff alone, and what the controller should check — report alongside the
|
||||
✅/❌ verdict for everything you could verify]
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Verify**
|
||||
|
||||
Run: `grep -n "suspiciously\|worktree add" skills/subagent-driven-development/spec-reviewer-prompt.md || echo CLEAN`
|
||||
Expected: `CLEAN`
|
||||
|
||||
Run: `grep -c "⚠️" skills/subagent-driven-development/spec-reviewer-prompt.md`
|
||||
Expected: `2` (judge-from-diff clause + verdict channel)
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add skills/subagent-driven-development/spec-reviewer-prompt.md
|
||||
git commit -m "Spec reviewer: judge from the diff, grounded skepticism, ⚠️ verdict channel"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 3: Implementer prompt — re-run tests after fixing review findings
|
||||
|
||||
The reviewers' "don't re-run the implementer's tests" rule assumes the implementer re-runs tests after every fix. Make that real.
|
||||
|
||||
**Files:**
|
||||
- Modify: `skills/subagent-driven-development/implementer-prompt.md`
|
||||
|
||||
- [ ] **Step 1: Insert a new section.** Immediately before the line (currently line 100):
|
||||
|
||||
```
|
||||
## Report Format
|
||||
```
|
||||
|
||||
insert:
|
||||
|
||||
```
|
||||
## After Review Findings
|
||||
|
||||
If a reviewer finds issues and you fix them, re-run the tests that cover
|
||||
the amended code and include the results in your fix report. Reviewers
|
||||
will not re-run tests for you — your report is the test evidence.
|
||||
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Verify**
|
||||
|
||||
Run: `grep -n "After Review Findings" skills/subagent-driven-development/implementer-prompt.md`
|
||||
Expected: one match, on a line before `## Report Format`
|
||||
|
||||
- [ ] **Step 3: Commit**
|
||||
|
||||
```bash
|
||||
git add skills/subagent-driven-development/implementer-prompt.md
|
||||
git commit -m "Implementer prompt: re-run covering tests after fixing review findings"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 4: SKILL.md controller changes
|
||||
|
||||
Six exact edits to `skills/subagent-driven-development/SKILL.md`. Current line numbers refer to commit f55642e.
|
||||
|
||||
**Files:**
|
||||
- Modify: `skills/subagent-driven-development/SKILL.md`
|
||||
|
||||
- [ ] **Step 1: Point the final-review flowchart node at the broad template.** The node label `Dispatch final code reviewer subagent for entire implementation` appears 3 times (currently lines 65, 84, 85). In all 3 occurrences, replace the label string with:
|
||||
|
||||
```
|
||||
Dispatch final code reviewer subagent (../requesting-code-review/code-reviewer.md)
|
||||
```
|
||||
|
||||
(Graphviz nodes are matched by label text — all three must be byte-identical or the graph grows a phantom node.)
|
||||
|
||||
- [ ] **Step 2: Model selection by judgment.** Replace (currently lines 97-99):
|
||||
|
||||
```
|
||||
**Architecture, design, and review tasks**: use the most capable available model.
|
||||
|
||||
**Task complexity signals:**
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```
|
||||
**Architecture and design tasks**: use the most capable available model.
|
||||
|
||||
**Review tasks**: choose the model with the same judgment, scaled to the
|
||||
diff's size, complexity, and risk. A small mechanical diff does not need the
|
||||
most capable model; a subtle concurrency change does.
|
||||
|
||||
**Task complexity signals (implementation tasks):**
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Add controller guidance sections.** Immediately before the line (currently line 122):
|
||||
|
||||
```
|
||||
## Prompt Templates
|
||||
```
|
||||
|
||||
insert:
|
||||
|
||||
```
|
||||
## Handling Spec Reviewer ⚠️ Items
|
||||
|
||||
The spec reviewer may report "⚠️ Cannot verify from diff" items — requirements
|
||||
that live in unchanged code or span tasks. These do not block dispatching the
|
||||
code quality reviewer, but you must resolve each one yourself before marking
|
||||
the task complete: you hold the plan and cross-task context the reviewer
|
||||
lacks. If you confirm an item is a real gap, treat it as a failed spec
|
||||
review — send it back to the implementer and re-review.
|
||||
|
||||
## Constructing Reviewer Prompts
|
||||
|
||||
Per-task reviews are task-scoped gates. The broad review happens once, at the
|
||||
final whole-branch review. When you fill a reviewer template:
|
||||
|
||||
- Do not add open-ended directives like "check all uses" or "run race tests
|
||||
if useful" without a concrete, task-specific reason
|
||||
- Do not ask a reviewer to re-run tests the implementer already ran on the
|
||||
same code — the implementer's report carries the test evidence
|
||||
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Prompt Templates list — add the final-review pointer.** Replace (currently line 126):
|
||||
|
||||
```
|
||||
- [code-quality-reviewer-prompt.md](code-quality-reviewer-prompt.md) - Dispatch code quality reviewer subagent
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```
|
||||
- [code-quality-reviewer-prompt.md](code-quality-reviewer-prompt.md) - Dispatch code quality reviewer subagent
|
||||
- Final whole-branch review: use superpowers:requesting-code-review's [code-reviewer.md](../requesting-code-review/code-reviewer.md)
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Example workflow verdict vocabulary.** Two replacements:
|
||||
|
||||
Replace (currently line 157):
|
||||
```
|
||||
Code reviewer: Strengths: Good test coverage, clean. Issues: None. Approved.
|
||||
```
|
||||
with:
|
||||
```
|
||||
Code reviewer: Strengths: Good test coverage, clean. Issues: None. Task quality: Approved.
|
||||
```
|
||||
|
||||
Replace (currently line 191):
|
||||
```
|
||||
Code reviewer: ✅ Approved
|
||||
```
|
||||
with:
|
||||
```
|
||||
Code reviewer: ✅ Task quality: Approved
|
||||
```
|
||||
|
||||
(The final reviewer's "ready to merge" line, currently line 199, stays.)
|
||||
|
||||
- [ ] **Step 6: Integration section.** Replace (currently line 272):
|
||||
|
||||
```
|
||||
- **superpowers:requesting-code-review** - Code review template for reviewer subagents
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```
|
||||
- **superpowers:requesting-code-review** - Code review template for the final whole-branch review
|
||||
```
|
||||
|
||||
- [ ] **Step 7: Verify**
|
||||
|
||||
Run: `grep -c "Dispatch final code reviewer subagent (../requesting-code-review/code-reviewer.md)" skills/subagent-driven-development/SKILL.md`
|
||||
Expected: `3`
|
||||
|
||||
Run: `grep -n "most capable available model" skills/subagent-driven-development/SKILL.md`
|
||||
Expected: exactly one match (architecture/design bullet)
|
||||
|
||||
Run: `grep -n "Handling Spec Reviewer\|Constructing Reviewer Prompts" skills/subagent-driven-development/SKILL.md`
|
||||
Expected: two section headers, both before `## Prompt Templates`
|
||||
|
||||
Run: `grep -c "Task quality: Approved" skills/subagent-driven-development/SKILL.md`
|
||||
Expected: `2`
|
||||
|
||||
- [ ] **Step 8: Commit**
|
||||
|
||||
```bash
|
||||
git add skills/subagent-driven-development/SKILL.md
|
||||
git commit -m "SDD controller: reviewer prompt budgets, ⚠️ handling, final-review pointer, model judgment"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 5: New eval scenario — per-task quality reviewer catches a planted defect
|
||||
|
||||
Lives in the `evals/` **submodule** (separate repo, `superpowers-evals`). Work on a branch there; the parent submodule-pointer bump happens at finishing time per `evals/CLAUDE.md`.
|
||||
|
||||
The fixture plan's Task 2 implementation snippet duplicates Task 1's formatting logic verbatim. The duplication is spec-compliant, so the spec reviewer should pass it — the per-task quality reviewer is the gate under test (DRY violation).
|
||||
|
||||
**Files:**
|
||||
- Create: `evals/setup_helpers/sdd_quality_defect_plan.py`
|
||||
- Modify: `evals/setup_helpers/__init__.py`
|
||||
- Create: `evals/scenarios/sdd-quality-reviewer-catches-planted-defect/story.md`
|
||||
- Create: `evals/scenarios/sdd-quality-reviewer-catches-planted-defect/setup.sh`
|
||||
- Create: `evals/scenarios/sdd-quality-reviewer-catches-planted-defect/checks.sh`
|
||||
|
||||
- [ ] **Step 0: Branch in the submodule**
|
||||
|
||||
```bash
|
||||
cd evals
|
||||
git checkout -b sdd-quality-defect-scenario
|
||||
```
|
||||
|
||||
- [ ] **Step 1: Create `evals/setup_helpers/sdd_quality_defect_plan.py`:**
|
||||
|
||||
````python
|
||||
"""Setup helper for the sdd-quality-reviewer-catches-planted-defect scenario.
|
||||
|
||||
Scaffolds a tiny Node project with a 2-task plan whose Task 2
|
||||
implementation snippet duplicates Task 1's formatting logic verbatim.
|
||||
The duplication is spec-compliant — the requirements only describe
|
||||
behavior — so the spec compliance reviewer should pass it. The test
|
||||
measures whether the per-task code quality reviewer catches the DRY
|
||||
violation and forces a refactor in the review-fix loop.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
from setup_helpers.base import _git
|
||||
|
||||
PACKAGE_JSON = """\
|
||||
{
|
||||
"name": "report-quality",
|
||||
"version": "1.0.0",
|
||||
"type": "module",
|
||||
"scripts": {
|
||||
"test": "node --test"
|
||||
}
|
||||
}
|
||||
"""
|
||||
|
||||
PLAN_BODY = """\
|
||||
# Report Formatter — Implementation Plan
|
||||
|
||||
Two report formatting functions. Implement exactly what each task
|
||||
specifies.
|
||||
|
||||
## Task 1: User Report
|
||||
|
||||
**File:** `src/report.js`
|
||||
|
||||
**Requirements:**
|
||||
- Function named `formatUserReport`
|
||||
- Takes one parameter `user`: an object with `name`, `email`, `visits`
|
||||
- Returns a multi-line string: a banner of 40 `=` characters, then
|
||||
`Report for <name> <<email>>`, then the banner again, then
|
||||
`Visits: <visits>`, then a closing banner
|
||||
- Export the function
|
||||
|
||||
**Implementation:**
|
||||
```javascript
|
||||
export function formatUserReport(user) {
|
||||
const banner = "=".repeat(40);
|
||||
const lines = [];
|
||||
lines.push(banner);
|
||||
lines.push(`Report for ${user.name} <${user.email}>`);
|
||||
lines.push(banner);
|
||||
lines.push(`Visits: ${user.visits}`);
|
||||
lines.push(banner);
|
||||
return lines.join("\\n");
|
||||
}
|
||||
```
|
||||
|
||||
**Tests:** Create `test/report.test.js` verifying:
|
||||
- the result contains `Report for Ada <ada@example.com>` for that user
|
||||
- the result contains `Visits: 3` when `visits` is `3`
|
||||
- the result starts and ends with the 40-char banner
|
||||
|
||||
**Verification:** `npm test`
|
||||
|
||||
## Task 2: Admin Report
|
||||
|
||||
**File:** `src/report.js` (add to existing file)
|
||||
|
||||
**Requirements:**
|
||||
- Function named `formatAdminReport`
|
||||
- Takes one parameter `admin`: an object with `name`, `email`, `lastLogin`
|
||||
- Same banner layout as the user report; the body line is
|
||||
`Last login: <lastLogin>` instead of the visits line
|
||||
- Export the function; keep `formatUserReport` working
|
||||
|
||||
**Implementation:**
|
||||
```javascript
|
||||
export function formatAdminReport(admin) {
|
||||
const banner = "=".repeat(40);
|
||||
const lines = [];
|
||||
lines.push(banner);
|
||||
lines.push(`Report for ${admin.name} <${admin.email}>`);
|
||||
lines.push(banner);
|
||||
lines.push(`Last login: ${admin.lastLogin}`);
|
||||
lines.push(banner);
|
||||
return lines.join("\\n");
|
||||
}
|
||||
```
|
||||
|
||||
**Tests:** Add to `test/report.test.js`:
|
||||
- the result contains `Report for Grace <grace@example.com>` for that admin
|
||||
- the result contains `Last login: 2026-06-01`
|
||||
- the result starts and ends with the 40-char banner
|
||||
|
||||
**Verification:** `npm test`
|
||||
"""
|
||||
|
||||
|
||||
def scaffold_sdd_quality_defect_plan(workdir: Path) -> None:
|
||||
workdir = Path(workdir)
|
||||
workdir.mkdir(parents=True, exist_ok=True)
|
||||
_git(["git", "init", "-b", "main"], cwd=workdir)
|
||||
_git(["git", "config", "user.email", "drill@test.local"], cwd=workdir)
|
||||
_git(["git", "config", "user.name", "Drill Test"], cwd=workdir)
|
||||
|
||||
(workdir / "package.json").write_text(PACKAGE_JSON)
|
||||
plans_dir = workdir / "docs" / "superpowers" / "plans"
|
||||
plans_dir.mkdir(parents=True, exist_ok=True)
|
||||
(plans_dir / "report-plan.md").write_text(PLAN_BODY)
|
||||
|
||||
_git(["git", "add", "-A"], cwd=workdir)
|
||||
_git(["git", "commit", "-m", "initial: report formatter plan"], cwd=workdir)
|
||||
````
|
||||
|
||||
(Note the `\\n` in the JS snippets inside PLAN_BODY: the Python source must
|
||||
produce a literal `\n` in the markdown so the JS reads `lines.join("\n")`.)
|
||||
|
||||
- [ ] **Step 2: Register the helper.** In `evals/setup_helpers/__init__.py`:
|
||||
|
||||
After the line:
|
||||
```python
|
||||
from setup_helpers.sdd_real_projects import scaffold_sdd_go_fractals, scaffold_sdd_svelte_todo
|
||||
```
|
||||
add:
|
||||
```python
|
||||
from setup_helpers.sdd_quality_defect_plan import scaffold_sdd_quality_defect_plan
|
||||
```
|
||||
|
||||
After the registry entry:
|
||||
```python
|
||||
"scaffold_sdd_yagni_plan": scaffold_sdd_yagni_plan,
|
||||
```
|
||||
add:
|
||||
```python
|
||||
"scaffold_sdd_quality_defect_plan": scaffold_sdd_quality_defect_plan,
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Create `evals/scenarios/sdd-quality-reviewer-catches-planted-defect/story.md`:**
|
||||
|
||||
```markdown
|
||||
---
|
||||
id: sdd-quality-reviewer-catches-planted-defect
|
||||
title: SDD's per-task code quality review catches a planted DRY violation
|
||||
status: ready
|
||||
tags: subagent-driven-development
|
||||
quorum_max_time: 90m
|
||||
---
|
||||
|
||||
You have a small plan at docs/superpowers/plans/report-plan.md — two report
|
||||
formatting functions. The plan's Task 2 implementation snippet duplicates
|
||||
Task 1's formatting logic verbatim instead of sharing it. The duplication is
|
||||
spec-compliant (the requirements only describe behavior), so the spec
|
||||
compliance reviewer should pass it — the per-task code quality reviewer is
|
||||
the gate under test. You are spec-aware — name the skill.
|
||||
|
||||
When the agent is ready for input, tell it to execute the plan with SDD. Use
|
||||
phrasing like:
|
||||
|
||||
"I have a small plan at docs/superpowers/plans/report-plan.md — two report
|
||||
formatting functions. Use the superpowers:subagent-driven-development skill
|
||||
to execute it end-to-end — dispatch fresh subagents per task and run the
|
||||
two-stage review after each."
|
||||
|
||||
Let the agent proceed autonomously. If it asks clarifying questions, give
|
||||
brief answers. If it asks where the finished work should land — merge to the
|
||||
main branch, open a PR, etc. — tell it to **merge the work into the main
|
||||
checkout** (this is a local repo with no remote). If a quality reviewer
|
||||
flags the duplicated formatting logic and an implementer refactors it, let
|
||||
the review-fix cycle play out — that cycle is exactly the behavior under
|
||||
test.
|
||||
|
||||
The deliverable must end up in the checkout you launched in (the main
|
||||
working tree). If the agent did its work on a branch or in a worktree, it
|
||||
is not done until it has merged/finished that work back into the main
|
||||
checkout. Once the agent reports the plan is complete (both functions
|
||||
implemented, tests passing) AND the code is present on the main checkout,
|
||||
you are done.
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
- A `Skill` invocation naming `superpowers:subagent-driven-development`
|
||||
and at least one `Agent` (subagent dispatch) tool call appear in the
|
||||
session log.
|
||||
- The duplicated report-formatting logic did not survive to the end of
|
||||
the run. Either (a) the implementer never introduced the duplication
|
||||
(wrote or self-reviewed its way to shared logic), or (b) the per-task
|
||||
code quality reviewer flagged the duplication as an issue and a
|
||||
review-fix loop removed it. A fail looks like the duplicated logic
|
||||
shipping with the per-task quality reviewer approving it, or the
|
||||
duplication being caught only by the final whole-branch review.
|
||||
- The per-task quality reviewers stayed task-scoped: no package-wide
|
||||
test suites, race detector runs, or repeated/high-count test loops
|
||||
appear in reviewer subagent activity, and reviewers did not re-run
|
||||
the full test suite merely to confirm the implementer's report.
|
||||
- `npm test` passes in the main checkout and both `formatUserReport` and
|
||||
`formatAdminReport` are exported from src/report.js. The deterministic
|
||||
assertions gate this; the criteria above are about whether the
|
||||
*per-task quality review* was the mechanism that kept the code clean.
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Create `evals/scenarios/sdd-quality-reviewer-catches-planted-defect/setup.sh`:**
|
||||
|
||||
```bash
|
||||
#!/usr/bin/env bash
|
||||
set -euo pipefail
|
||||
uv run setup-helpers run scaffold_sdd_quality_defect_plan
|
||||
```
|
||||
|
||||
Then: `chmod +x evals/scenarios/sdd-quality-reviewer-catches-planted-defect/setup.sh`
|
||||
|
||||
- [ ] **Step 5: Create `evals/scenarios/sdd-quality-reviewer-catches-planted-defect/checks.sh`** (no executable bit):
|
||||
|
||||
```bash
|
||||
pre() {
|
||||
git-repo
|
||||
git-branch main
|
||||
requires-tool npm
|
||||
file-exists 'docs/superpowers/plans/report-plan.md'
|
||||
file-contains 'docs/superpowers/plans/report-plan.md' 'formatAdminReport'
|
||||
file-contains 'docs/superpowers/plans/report-plan.md' 'repeat\(40\)'
|
||||
}
|
||||
|
||||
post() {
|
||||
skill-called superpowers:subagent-driven-development
|
||||
tool-called Agent
|
||||
command-succeeds 'npm test'
|
||||
file-contains 'src/report.js' 'export function formatUserReport'
|
||||
file-contains 'src/report.js' 'export function formatAdminReport'
|
||||
command-succeeds 'test "$(grep -c "repeat(40)" src/report.js)" -le 1'
|
||||
}
|
||||
```
|
||||
|
||||
(The last check is the deterministic DRY gate: the banner construction
|
||||
`"=".repeat(40)` must appear at most once in the final file — shared, not
|
||||
duplicated per function.)
|
||||
|
||||
- [ ] **Step 6: Validate and test in the evals repo**
|
||||
|
||||
```bash
|
||||
cd evals
|
||||
uv run quorum check
|
||||
uv run ruff check
|
||||
uv run pytest -x -q
|
||||
```
|
||||
|
||||
Expected: all pass; `quorum check` lists the new scenario without errors.
|
||||
|
||||
- [ ] **Step 7: Commit (in the submodule)**
|
||||
|
||||
```bash
|
||||
cd evals
|
||||
git add setup_helpers/sdd_quality_defect_plan.py setup_helpers/__init__.py scenarios/sdd-quality-reviewer-catches-planted-defect/
|
||||
git commit -m "Add sdd-quality-reviewer-catches-planted-defect scenario"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 6: Static verification sweep
|
||||
|
||||
**Files:** none modified — verification only.
|
||||
|
||||
- [ ] **Step 1: No dangling references in the parent repo**
|
||||
|
||||
Run: `grep -rn "requesting-code-review" skills/subagent-driven-development/`
|
||||
Expected: matches only in SKILL.md (final-review flowchart node ×3, Prompt Templates pointer, Integration bullet). None in code-quality-reviewer-prompt.md.
|
||||
|
||||
Run: `grep -rn "Ready to merge" skills/subagent-driven-development/ || echo CLEAN`
|
||||
Expected: `CLEAN`
|
||||
|
||||
- [ ] **Step 2: Plugin infrastructure tests**
|
||||
|
||||
Run: `bash tests/shell-lint/test-lint-shell.sh`
|
||||
Expected: all PASS (we added `setup.sh` only inside the evals submodule, which has its own checks).
|
||||
|
||||
- [ ] **Step 3: Cross-platform tool tables still coherent**
|
||||
|
||||
Run: `grep -n "code-quality-reviewer" skills/using-superpowers/references/antigravity-tools.md skills/using-superpowers/references/gemini-tools.md`
|
||||
Expected: both tables still list `code-quality-reviewer` as a reviewer template (the new prompt's "If you cannot run commands in this environment, name the test you would run" line keeps the read-only `research` mapping valid — no table edits needed).
|
||||
|
||||
---
|
||||
|
||||
### Task 7: Live before/after evals (maintainer-gated)
|
||||
|
||||
Live quorum runs launch agent CLIs in permissive modes — **trusted-maintainer operation; Jesse launches these**, per `evals/CLAUDE.md`. Requires `ANTHROPIC_API_KEY`.
|
||||
|
||||
- [ ] **Step 1: Baseline (skills as released on dev)** — from the main checkout (`/Users/jesse/git/superpowers/superpowers`, on dev), or any checkout without this branch's changes:
|
||||
|
||||
```bash
|
||||
cd evals
|
||||
export SUPERPOWERS_ROOT=/Users/jesse/git/superpowers/superpowers
|
||||
uv run quorum run scenarios/sdd-rejects-extra-features --coding-agent claude
|
||||
uv run quorum run scenarios/sdd-go-fractals --coding-agent claude
|
||||
uv run quorum run scenarios/sdd-svelte-todo --coding-agent claude
|
||||
uv run quorum run scenarios/spec-reviewer-catches-planted-flaws --coding-agent claude
|
||||
```
|
||||
|
||||
- [ ] **Step 2: After (this branch's skills)** — point `SUPERPOWERS_ROOT` at this worktree:
|
||||
|
||||
```bash
|
||||
cd evals
|
||||
export SUPERPOWERS_ROOT=/Users/jesse/git/superpowers/superpowers/.claude/worktrees/sdd-review-dispatch
|
||||
uv run quorum run scenarios/sdd-rejects-extra-features --coding-agent claude
|
||||
uv run quorum run scenarios/sdd-go-fractals --coding-agent claude
|
||||
uv run quorum run scenarios/sdd-svelte-todo --coding-agent claude
|
||||
uv run quorum run scenarios/spec-reviewer-catches-planted-flaws --coding-agent claude
|
||||
uv run quorum run scenarios/sdd-quality-reviewer-catches-planted-defect --coding-agent claude
|
||||
uv run quorum show
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Compare**
|
||||
|
||||
Pass bar: all four pre-existing scenarios still pass after the change (no regression in catch rate); the new planted-defect scenario passes. For exploration cost, compare reviewer-subagent tool-call counts between the before/after run transcripts (no automated check exists — the spec calls this out as a known gap).
|
||||
|
||||
---
|
||||
|
||||
## Finishing
|
||||
|
||||
After all tasks pass: the evals submodule commit needs to land in `superpowers-evals` (PR to its `main`), then this branch bumps the `evals` submodule pointer — per `evals/CLAUDE.md`, the parent bump is part of propagation, not optional. Then use superpowers:finishing-a-development-branch. PRs against superpowers target `dev`.
|
||||
@@ -1,124 +0,0 @@
|
||||
# SDD Task-Scoped Review Dispatch
|
||||
|
||||
Make subagent-driven-development's per-task reviews cheaper and faster without weakening them, by scoping per-task review prompts to the task and stopping redundant work — while final branch review stays broad.
|
||||
|
||||
## Problem
|
||||
|
||||
Per-task code quality reviewers in SDD routinely do branch-review-scale work on single-task diffs. Evidence from two real local SDD sessions: `a1a6719a-6109-453a-9933-34ae396f5bae` (sen-core-v2) and `0cc1a12d-9984-4c35-8615-9d42dadb2c47` (serf), both under `~/.claude/projects/`:
|
||||
|
||||
- In the sen-core-v2 session, 7/8 quality reviewers ran repo-wide greps; the most expensive ran 50+ Bash commands over ~200 seconds. Across both sessions, quality reviewers cost 4-8× what spec reviewers cost on the same tasks.
|
||||
- Spec reviewers, whose prompt contains "Only read files in this diff. Do not crawl the broader codebase," stayed tight: 6-16 tool calls, 14-65 seconds.
|
||||
- No reviewer ran heavy tests autonomously. Every package-wide or repeated test run observed was explicitly requested by a controller-written prompt ("check all uses," "run tests if useful, especially race-focused ones," "does anything else read `Meta()`?").
|
||||
|
||||
Root causes, in order of impact:
|
||||
|
||||
1. **The per-task quality prompt inherits a merge-readiness review.** `code-quality-reviewer-prompt.md` delegates to `requesting-code-review/code-reviewer.md`, which asks about architecture, scalability, security, production readiness, and ends with "Ready to merge?" That frame licenses branch-level breadth on a one-task diff. The spec prompt's diff-scope guard was never carried over.
|
||||
2. **The controller gets no guidance on writing reviewer prompts**, so it invents open-ended directives ("check all uses") that reviewers interpret literally.
|
||||
3. **Duplicated work across the pipeline.** The quality template's "Plan alignment" dimension re-checks what the spec reviewer just verified. Reviewers re-run test suites the implementer already ran (and reported, with TDD evidence) on identical code.
|
||||
4. **Per-task and final review share one template**, so there is no representation of "per-task narrow, final broad" anywhere.
|
||||
|
||||
A field report (`~/2026-06-09-code-quality-reviewer-scope-budget-issue.md`) first flagged this. Its cited session and headline numbers could not be verified, but its qualitative diagnosis was confirmed against two real local sessions. One correction to it: cross-cutting audits (lock ordering, changed contracts) are sometimes the *correct* review method — the fix must gate breadth behind a stated concrete risk, not forbid it.
|
||||
|
||||
## Goals
|
||||
|
||||
- Per-task reviews scoped to the task: diff-first reading, justified broadening, no redundant test runs.
|
||||
- Final whole-branch review keeps its current breadth.
|
||||
- No reduction in what reviews catch.
|
||||
|
||||
## Non-goals / explicitly preserved
|
||||
|
||||
- **Full re-reviews stay.** When a reviewer re-reviews after a fix, it still reviews the whole task at full reading breadth. (It does not re-run tests the implementer just ran on the amended code.) This deliberately rejects the field report's "re-review budget" remedy: the cost of its worst cited example (a re-review running `-race` and `-count=100` loops) is curbed by the test budget below, not by narrowing what re-reviewers read.
|
||||
- ~~**The two review stages stay separate.** Spec compliance and code quality remain independent subagents, serially gated. No merging.~~ **Superseded by the cost iterations below**: live eval economics showed per-dispatch overhead dominating cost, and the maintainer put everything on the table. The per-task stages are now one task reviewer with two verdicts; the independent broad final review remains.
|
||||
- **The coordinator keeps model judgment.** No forced model tier for reviews, in either direction.
|
||||
- **`requesting-code-review/` is untouched.** It remains the broad template for final branch review and ad-hoc review.
|
||||
- Verdict ordering (spec compliance reported before quality), the fix-and-re-review loops, and the requirement to fix Critical/Important findings are unchanged.
|
||||
|
||||
## Cost iterations (post-launch eval economics)
|
||||
|
||||
Live before/after runs surfaced a cost regression once the quality-hardening
|
||||
prose (evidence rule, constraint carrying, pristine output) landed: go-fractals
|
||||
went from 42.8 min / 14.5M tokens (first task-scoped version) to 69.9 min /
|
||||
32.2M (hardened version) while reaching baseline-parity quality (blind-judged
|
||||
8.5 vs 8.5). Per-subagent turn profiling attributed cost to, in order: cheap
|
||||
models taking 2-3× the turns on multi-step work (678 of 1197 subagent turns
|
||||
were haiku), per-dispatch overhead (3 subagent spin-ups per task, each
|
||||
re-deriving the diff; controller coordination was half the dollars), and
|
||||
evidence-rule narration.
|
||||
|
||||
- **Iteration 1:** turn-count-beats-token-price model guidance (mid-tier floor
|
||||
for multi-step work), optional inline diffs, cite-don't-narrate evidence,
|
||||
Important = cannot-trust-until-fixed, fixes dispatched only for
|
||||
Critical/Important. Result: 68.2 min / 22.9M — tokens down 29%, wall-clock
|
||||
flat; controllers pasted the diff in only 2 of 22 review dispatches when
|
||||
phrasing was optional.
|
||||
- **Iteration 2:** per-task spec and quality reviews merged into one
|
||||
`task-reviewer-prompt.md` (one reviewer, one reading of the diff, two
|
||||
verdicts; one fix dispatch addresses both kinds of findings); implementers
|
||||
run the focused test while iterating, full suite once before commit.
|
||||
Result (go-fractals): 47.5 min / 15.7M / $13.55 — beat baseline on every
|
||||
axis, blind-judged 9/10 vs baseline 7/10.
|
||||
- **Iteration 3:** Calibration names merge-blocking maintainability damage
|
||||
(verbatim duplication, swallowed errors, assertion-free tests) as
|
||||
Important and Minor findings must be pasted into the final review for
|
||||
triage; reviewer skepticism extended to the implementer's design
|
||||
rationales ("left it per YAGNI" is a claim, not a verdict); diff handed
|
||||
to reviewers as a file (`git diff > /tmp/sdd-task-N.diff`, redirected so
|
||||
it never enters the controller's context; one Read call for the
|
||||
reviewer) after paste-into-prompt guidance went unadopted (0-6 of 11-17
|
||||
dispatches) for locally-rational context-economics reasons.
|
||||
- **Final frozen config (e355795), all five scenarios pass:** go-fractals
|
||||
44.4 min / 13.4M / $11.67 (-32% time, -37% tokens, -27% dollars vs
|
||||
baseline); svelte-todo 62.8 / 19.7M / $15.76 (-21% / -28% / -25%);
|
||||
rejects-extra-features $1.31 (vs $1.88); spec-reviewer-flaws flat; the
|
||||
planted-defect scenario (v3: open-flag transparency bar for judgment
|
||||
calls, must-fix bar for a test whose name promises verification it
|
||||
never performs) passes with the defect caught and fixed.
|
||||
|
||||
## Design
|
||||
|
||||
### Shared principle: don't re-run tests on code that hasn't changed
|
||||
|
||||
The implementer's report includes test results and TDD RED/GREEN evidence for exactly the code under review. Reviewers verify by reading. A reviewer runs a test only when reading raises a specific doubt that no existing run answers — and then a focused test, not a suite. On harnesses where reviewer subagents are read-only (e.g., Antigravity maps reviewer templates to the `research` type, which has no command access), the reviewer instead names the test it would run in its report.
|
||||
|
||||
After a fix, the implementer re-runs the tests covering the amended code; the re-reviewer does not repeat that run. Today nothing enforces that premise: `implementer-prompt.md` describes the initial implement-test-commit flow only, with no fix-iteration instruction. This spec therefore also adds to `implementer-prompt.md`: after fixing a review finding, re-run the tests that cover the amended code and include the results in the fix report.
|
||||
|
||||
This principle appears in both reviewer prompts, the implementer prompt, and the controller guidance.
|
||||
|
||||
### 1. New file: `skills/subagent-driven-development/code-quality-reviewer-prompt.md` becomes self-contained
|
||||
|
||||
Stop delegating to `requesting-code-review/code-reviewer.md`. The per-task quality reviewer gets its own scoped prompt template:
|
||||
|
||||
- **Framing:** "You are reviewing one task's implementation for code quality." A task-scoped gate, not a merge review.
|
||||
- **Spec compliance is settled:** spec review already passed; do not re-litigate requirements or plan alignment.
|
||||
- **Review dimensions kept:** code quality (clarity, duplication, error handling), test quality (real behavior, not mocks), maintainability, and the existing SDD-specific checks (single responsibility, independent testability, file structure from plan, file growth contributed by this change). Dropped: plan alignment, security/scalability/production-readiness dimensions, merge verdict.
|
||||
- **Scope budget:** start from `git diff BASE..HEAD`; read changed files first; inspect adjacent code only to evaluate a concrete risk you can name. Cross-cutting changes — lock ordering, changed function/API contracts, shared mutable state — are legitimate named risks that justify checking call sites. Do not crawl the codebase by default.
|
||||
- **Test budget:** the shared principle above, plus: no package-wide suites, race detectors, or repeated/high-count runs unless you have first named a specific suspected flake or race. Otherwise, recommend heavy validation in the report instead of running it. Warnings or noise in the implementer's reported test output are findings — output should be pristine (the implementer's self-review checks this too).
|
||||
- **Evidence rule:** reviewers answer each What-to-Check item with file:line evidence, not bare yes/no. (Added after live eval runs showed reviewers passing defects the prompt had pointed them at — an accessible-name check and a temp-dir-cleanup check both got unsupported "yes" answers while the defect sat in the reviewed diff.)
|
||||
- **Read-only rule** kept in trimmed form: no mutating the working tree, index, HEAD, or branch state. The `git worktree add` how-to sentence from the current templates is NOT carried into this file — a diff-scoped review never needs a checkout of another revision (same rationale as the spec-prompt cleanup below).
|
||||
- **Verdict:** Strengths / Issues (Critical/Important/Minor) / "Task quality: Approved | Needs fixes."
|
||||
|
||||
### 2. `skills/subagent-driven-development/spec-reviewer-prompt.md` cleanups
|
||||
|
||||
- Remove the `git worktree add` how-to sentence. The read-only rule stays; a diff-scoped spec review never needs a checkout of another revision.
|
||||
- Resolve the tension between the diff-only guard and "verify everything independently": spec compliance is judged by reading the diff against the requirements. The implementer's TDD evidence covers "it runs" — apply the shared test principle.
|
||||
- New third verdict channel: requirements that cannot be verified from the diff (live in unchanged code, span tasks) are reported as explicit "⚠️ Cannot verify from diff — controller should check X" items, instead of either crawling or silently passing. The flowchart's binary pass/fail diamond cannot route this, so the controller guidance (§3) defines the handling: ⚠️ items do not block dispatching the quality reviewer, but the controller must resolve each one itself (it holds the plan and cross-task context) before marking the task complete; an item the controller confirms is a real gap is treated as a failed spec review and goes back to the implementer.
|
||||
- Replace the fabricated premise "The implementer finished suspiciously quickly" with grounded skepticism: treat the implementer's report as unverified claims about the code. Same distrust, no invented fact.
|
||||
|
||||
### 3. `skills/subagent-driven-development/SKILL.md` controller changes
|
||||
|
||||
- **Model Selection:** replace "Architecture, design, and review tasks: use the most capable available model" with judgment guidance — pick reviewer models the way implementer models are picked, scaled to the diff's size, complexity, and risk. The "Task complexity signals" list is rescoped to make clear its bullets describe implementation tasks; reviewer model choice follows the same judgment, so a narrow diff review does not automatically map to "broad codebase understanding → most capable model."
|
||||
- **Reviewer prompt construction** (new guidance near Red Flags): when dispatching reviewers, do not write open-ended directives ("check all uses," "run race tests if useful") without a concrete task-specific reason; do not ask reviewers to re-run tests the implementer already ran on the same code; do not pre-judge findings for the reviewer (never instruct a reviewer to ignore or not flag a specific issue — adjudicate suspected false positives in the review loop instead); per-task reviews are task-scoped gates — the broad review happens once, at the final whole-branch review. (The pre-judging rule was added after a live eval run caught the controller fabricating a "the plan forbids a shared helper" claim and instructing the quality reviewer not to flag a planted DRY violation.) Controllers must also include the spec/design's global constraints that bind the task — version floors, naming and copy rules, platform requirements — in the requirements they paste: a live run shipped a `go 1.26.1` module floor against a "Go 1.21+" design because no reviewer ever saw the constraint. And controllers must specify a model explicitly on every dispatch — an omitted model inherits the session's (usually most expensive) model, which silently defeats model selection.
|
||||
- **Handling spec-reviewer ⚠️ items** (new guidance, alongside Handling Implementer Status): the controller resolves each "cannot verify from diff" item itself before marking the task complete; confirmed gaps go back to the implementer as failed spec review.
|
||||
- **Final review stays broad, explicitly:** the final whole-branch reviewer dispatch node gains an explicit pointer to `../requesting-code-review/code-reviewer.md`. (Today that template is reachable only through the per-task quality prompt's delegation; once that delegation is removed, an unreferenced final-review template would be orphaned.) The Integration section's note that `superpowers:requesting-code-review` provides "the code review template for reviewer subagents" is corrected to apply to the final review only.
|
||||
- **Example workflow:** the quality-reviewer lines in the example are updated to the new verdict vocabulary ("Task quality: Approved"); the final reviewer's "ready to merge" line stays.
|
||||
- Flowchart topology is unchanged; the ⚠️ channel is handled by controller guidance, not a new graph branch.
|
||||
|
||||
## What this does not fix (known, deferred)
|
||||
|
||||
The spec reviewer judges against task text the controller pasted; it cannot catch requirements dropped during the controller's extraction from the plan. That is an architectural property of "controller provides full text," not a prompt problem, and is out of scope here.
|
||||
|
||||
## Verification
|
||||
|
||||
- Plugin infrastructure tests (`tests/`) still pass.
|
||||
- Run the SDD skill-behavior evals (`git submodule update --init evals`, then per `evals/README.md`) before and after the change. Specifically: `sdd-go-fractals`, `sdd-svelte-todo`, `sdd-rejects-extra-features` (end-to-end SDD including the spec reviewer's YAGNI gate), and `spec-reviewer-catches-planted-flaws`.
|
||||
- Known eval gaps this change exposes: no existing scenario plants a code-quality defect inside a single SDD task and asserts the per-task quality reviewer catches it, and no scenario measures per-reviewer exploration cost (tool-call/grep counts). Add one scenario covering the first gap (planted single-task quality defect → per-task reviewer must flag it before final review). For exploration cost, compare reviewer subagent tool-call counts manually across the before/after eval transcripts.
|
||||
@@ -12,7 +12,6 @@ Live in `tests/`. Currently:
|
||||
- `tests/brainstorm-server/` — node test suite for the brainstorm server JS code.
|
||||
- `tests/opencode/` — bash tests for OpenCode plugin loading, bootstrap caching, and tool registration.
|
||||
- `tests/codex-plugin-sync/` — bash sync verification.
|
||||
- `tests/kimi/` — bash/Python checks for Kimi plugin manifest wiring.
|
||||
- `tests/claude-code/test-helpers.sh`, `analyze-token-usage.py` — utilities used by remaining bash tests.
|
||||
- `tests/claude-code/test-subagent-driven-development.sh` — agent-can-describe-SDD test (no drill counterpart; tests description-recall, not behavior).
|
||||
- `tests/claude-code/test-subagent-driven-development-integration.sh` — extended SDD integration with token analysis (drill covers the YAGNI subset; bash adds commit-count, Claude Code task-tracking, and token telemetry assertions).
|
||||
|
||||
@@ -1,8 +1,6 @@
|
||||
# Cross-Platform Polyglot Hooks for Claude Code
|
||||
|
||||
Claude Code plugins need hooks that work on Windows, macOS, and Linux. This document describes the single generic dispatcher pattern used in `hooks/run-hook.cmd`.
|
||||
|
||||
> **Authoritative source:** `hooks/run-hook.cmd` is the canonical implementation. When this document and the code diverge, trust the code.
|
||||
Claude Code plugins need hooks that work on Windows, macOS, and Linux. This document explains the polyglot wrapper technique that makes this possible.
|
||||
|
||||
## The Problem
|
||||
|
||||
@@ -12,22 +10,52 @@ Claude Code runs hook commands through the system's default shell:
|
||||
|
||||
This creates several challenges:
|
||||
|
||||
1. **Script execution**: Windows CMD can't execute `.sh` files directly
|
||||
1. **Script execution**: Windows CMD can't execute `.sh` files directly - it tries to open them in a text editor
|
||||
2. **Path format**: Windows uses backslashes (`C:\path`), Unix uses forward slashes (`/path`)
|
||||
3. **Environment variables**: `$VAR` syntax doesn't work in CMD
|
||||
4. **`.sh` auto-prepend**: Claude Code on Windows automatically prepends `bash` to any command that contains `.sh` in its path — this interferes with the dispatcher if scripts have extensions
|
||||
4. **No `bash` in PATH**: Even with Git Bash installed, `bash` isn't in the PATH when CMD runs
|
||||
|
||||
## The Solution: Extensionless Scripts + Single Generic Dispatcher
|
||||
## The Solution: Polyglot `.cmd` Wrapper
|
||||
|
||||
The repo uses one generic `run-hook.cmd` dispatcher for all hooks. Hook scripts are **extensionless** (`session-start`, not `session-start.sh`). This is deliberate: it prevents Claude Code's Windows auto-detection from prepending `bash` to the dispatcher command and breaking it.
|
||||
A polyglot script is valid syntax in multiple languages simultaneously. Our wrapper is valid in both CMD and bash:
|
||||
|
||||
### File Structure
|
||||
```cmd
|
||||
: << 'CMDBLOCK'
|
||||
@echo off
|
||||
"C:\Program Files\Git\bin\bash.exe" -l -c "\"$(cygpath -u \"$CLAUDE_PLUGIN_ROOT\")/hooks/session-start.sh\""
|
||||
exit /b
|
||||
CMDBLOCK
|
||||
|
||||
# Unix shell runs from here
|
||||
"${CLAUDE_PLUGIN_ROOT}/hooks/session-start.sh"
|
||||
```
|
||||
|
||||
### How It Works
|
||||
|
||||
#### On Windows (CMD.exe)
|
||||
|
||||
1. `: << 'CMDBLOCK'` - CMD sees `:` as a label (like `:label`) and ignores `<< 'CMDBLOCK'`
|
||||
2. `@echo off` - Suppresses command echoing
|
||||
3. The bash.exe command runs with:
|
||||
- `-l` (login shell) to get proper PATH with Unix utilities
|
||||
- `cygpath -u` converts Windows path to Unix format (`C:\foo` → `/c/foo`)
|
||||
4. `exit /b` - Exits the batch script, stopping CMD here
|
||||
5. Everything after `CMDBLOCK` is never reached by CMD
|
||||
|
||||
#### On Unix (bash/sh)
|
||||
|
||||
1. `: << 'CMDBLOCK'` - `:` is a no-op, `<< 'CMDBLOCK'` starts a heredoc
|
||||
2. Everything until `CMDBLOCK` is consumed by the heredoc (ignored)
|
||||
3. `# Unix shell runs from here` - Comment
|
||||
4. The script runs directly with the Unix path
|
||||
|
||||
## File Structure
|
||||
|
||||
```
|
||||
hooks/
|
||||
├── hooks.json # Points to run-hook.cmd with extensionless script name
|
||||
├── run-hook.cmd # Cross-platform dispatcher (the polyglot wrapper)
|
||||
└── session-start # Actual hook logic — extensionless bash script
|
||||
├── hooks.json # Points to the .cmd wrapper
|
||||
├── session-start.cmd # Polyglot wrapper (cross-platform entry point)
|
||||
└── session-start.sh # Actual hook logic (bash script)
|
||||
```
|
||||
|
||||
### hooks.json
|
||||
@@ -37,12 +65,11 @@ hooks/
|
||||
"hooks": {
|
||||
"SessionStart": [
|
||||
{
|
||||
"matcher": "startup|clear|compact",
|
||||
"matcher": "startup|resume|clear|compact",
|
||||
"hooks": [
|
||||
{
|
||||
"type": "command",
|
||||
"command": "\"${CLAUDE_PLUGIN_ROOT}/hooks/run-hook.cmd\" session-start",
|
||||
"async": false
|
||||
"command": "\"${CLAUDE_PLUGIN_ROOT}/hooks/session-start.cmd\""
|
||||
}
|
||||
]
|
||||
}
|
||||
@@ -51,63 +78,41 @@ hooks/
|
||||
}
|
||||
```
|
||||
|
||||
The path is quoted because `${CLAUDE_PLUGIN_ROOT}` may contain spaces.
|
||||
Note: The path must be quoted because `${CLAUDE_PLUGIN_ROOT}` may contain spaces on Windows (e.g., `C:\Program Files\...`).
|
||||
|
||||
## How `run-hook.cmd` Works at a High Level
|
||||
## Requirements
|
||||
|
||||
`run-hook.cmd` is a polyglot script: Windows treats the first block as batch
|
||||
commands, while Unix shells treat that block as a no-op heredoc and continue
|
||||
after it.
|
||||
### Windows
|
||||
- **Git for Windows** must be installed (provides `bash.exe` and `cygpath`)
|
||||
- Default installation path: `C:\Program Files\Git\bin\bash.exe`
|
||||
- If Git is installed elsewhere, the wrapper needs modification
|
||||
|
||||
Do not copy an implementation from this document. Read `hooks/run-hook.cmd`
|
||||
directly when changing the dispatcher, and run `tests/hooks/test-session-start.sh`
|
||||
afterward.
|
||||
|
||||
### How it works on Windows (CMD.exe)
|
||||
|
||||
1. The batch section validates the script name and resolves the hook directory
|
||||
from the dispatcher's own location.
|
||||
2. It tries bash in three places:
|
||||
- `C:\Program Files\Git\bin\bash.exe`
|
||||
- `C:\Program Files (x86)\Git\bin\bash.exe`
|
||||
- `bash` on `PATH` (MSYS2, Cygwin, or a non-default Git install)
|
||||
3. If bash is found, it runs the named extensionless hook script from the hooks
|
||||
directory.
|
||||
4. If no bash is found, the dispatcher exits `0` silently — the plugin
|
||||
continues working, it just skips the hook.
|
||||
5. `exit /b` stops CMD before it reaches the Unix section.
|
||||
|
||||
### How it works on Unix (bash/sh)
|
||||
|
||||
1. `: << 'CMDBLOCK'` opens a heredoc on a no-op command.
|
||||
2. The entire CMD batch block is consumed by the heredoc and ignored.
|
||||
3. After `CMDBLOCK`, bash resolves the script directory and `exec`s the named
|
||||
extensionless script directly.
|
||||
|
||||
### Key design decisions
|
||||
|
||||
| Decision | Why |
|
||||
|----------|-----|
|
||||
| Extensionless scripts | Prevents Claude Code's Windows `.sh`-auto-prepend from interfering with the dispatcher command |
|
||||
| No `-l` (login shell) | Not needed; hook scripts should be self-contained and not depend on login-shell PATH setup |
|
||||
| No `cygpath` | Bash receives the Windows path directly and handles it correctly; `cygpath` was needed by the old `-c "..."` invocation pattern, not by direct exec |
|
||||
| Silent exit on no-bash | Avoids breaking the plugin for users who don't have Git for Windows; hook context injection is skipped gracefully |
|
||||
### Unix (macOS/Linux)
|
||||
- Standard bash or sh shell
|
||||
- The `.cmd` file must have execute permission (`chmod +x`)
|
||||
|
||||
## Writing Cross-Platform Hook Scripts
|
||||
|
||||
Your hook logic goes in the extensionless script file. A few portable patterns:
|
||||
Your actual hook logic goes in the `.sh` file. To ensure it works on Windows (via Git Bash):
|
||||
|
||||
### Do
|
||||
### Do:
|
||||
- Use pure bash builtins when possible
|
||||
- Use `$(command)` instead of backticks
|
||||
- Quote all variable expansions: `"$VAR"`
|
||||
- Use `printf` or here-docs for output
|
||||
|
||||
### Avoid
|
||||
- Relying on PATH-dependent tools without fallbacks (the hook runs without `-l`, so login-shell PATH is not set)
|
||||
- Giving scripts a `.sh` extension — this triggers Claude Code's Windows auto-prepend
|
||||
### Avoid:
|
||||
- External commands that may not be in PATH (sed, awk, grep)
|
||||
- If you must use them, they're available in Git Bash but ensure PATH is set up (use `bash -l`)
|
||||
|
||||
### Example: JSON escaping without external tools
|
||||
### Example: JSON Escaping Without sed/awk
|
||||
|
||||
Instead of:
|
||||
```bash
|
||||
escaped=$(echo "$content" | sed 's/\\/\\\\/g' | sed 's/"/\\"/g' | awk '{printf "%s\\n", $0}')
|
||||
```
|
||||
|
||||
Use pure bash:
|
||||
```bash
|
||||
escape_for_json() {
|
||||
local input="$1"
|
||||
@@ -128,21 +133,80 @@ escape_for_json() {
|
||||
}
|
||||
```
|
||||
|
||||
## Reusable Wrapper Pattern
|
||||
|
||||
For plugins with multiple hooks, you can create a generic wrapper that takes the script name as an argument:
|
||||
|
||||
### run-hook.cmd
|
||||
```cmd
|
||||
: << 'CMDBLOCK'
|
||||
@echo off
|
||||
set "SCRIPT_DIR=%~dp0"
|
||||
set "SCRIPT_NAME=%~1"
|
||||
"C:\Program Files\Git\bin\bash.exe" -l -c "cd \"$(cygpath -u \"%SCRIPT_DIR%\")\" && \"./%SCRIPT_NAME%\""
|
||||
exit /b
|
||||
CMDBLOCK
|
||||
|
||||
# Unix shell runs from here
|
||||
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
|
||||
SCRIPT_NAME="$1"
|
||||
shift
|
||||
"${SCRIPT_DIR}/${SCRIPT_NAME}" "$@"
|
||||
```
|
||||
|
||||
### hooks.json using the reusable wrapper
|
||||
```json
|
||||
{
|
||||
"hooks": {
|
||||
"SessionStart": [
|
||||
{
|
||||
"matcher": "startup",
|
||||
"hooks": [
|
||||
{
|
||||
"type": "command",
|
||||
"command": "\"${CLAUDE_PLUGIN_ROOT}/hooks/run-hook.cmd\" session-start.sh"
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"PreToolUse": [
|
||||
{
|
||||
"matcher": "Bash",
|
||||
"hooks": [
|
||||
{
|
||||
"type": "command",
|
||||
"command": "\"${CLAUDE_PLUGIN_ROOT}/hooks/run-hook.cmd\" validate-bash.sh"
|
||||
}
|
||||
]
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
## Troubleshooting
|
||||
|
||||
### "bash is not recognized"
|
||||
CMD can't find bash. The wrapper uses the full path `C:\Program Files\Git\bin\bash.exe`. If Git is installed elsewhere, update the path.
|
||||
|
||||
CMD couldn't find bash in any of the three locations the dispatcher tries. The dispatcher exits silently (0) rather than erroring, so the hook is skipped. Install Git for Windows at the standard path or ensure `bash` is on `PATH`.
|
||||
### "cygpath: command not found" or "dirname: command not found"
|
||||
Bash isn't running as a login shell. Ensure `-l` flag is used.
|
||||
|
||||
### Hook runs on Unix but does nothing on Windows
|
||||
### Path has weird `\/` in it
|
||||
`${CLAUDE_PLUGIN_ROOT}` expanded to a Windows path ending with backslash, then `/hooks/...` was appended. Use `cygpath` to convert the entire path.
|
||||
|
||||
Check that the script filename is **extensionless** in `hooks.json`. A command like `run-hook.cmd session-start.sh` can trigger Claude Code's `.sh` auto-detection and bypass the intended CMD dispatcher path, or just try to run a non-existent `session-start.sh` script.
|
||||
### Script opens in text editor instead of running
|
||||
The hooks.json is pointing directly to the `.sh` file. Point to the `.cmd` wrapper instead.
|
||||
|
||||
### Hook doesn't fire at all
|
||||
|
||||
Verify the `matcher` in `hooks.json` matches the event type your harness emits. Claude Code uses `startup|clear|compact`; Codex uses `startup|resume|clear`. Check `hooks-codex.json` for the Codex variant.
|
||||
### Works in terminal but not as hook
|
||||
Claude Code may run hooks differently. Test by simulating the hook environment:
|
||||
```powershell
|
||||
$env:CLAUDE_PLUGIN_ROOT = "C:\path\to\plugin"
|
||||
cmd /c "C:\path\to\plugin\hooks\session-start.cmd"
|
||||
```
|
||||
|
||||
## Related Issues
|
||||
|
||||
- [anthropics/claude-code#9758](https://github.com/anthropics/claude-code/issues/9758) — `.sh` scripts open in editor on Windows
|
||||
- [anthropics/claude-code#3417](https://github.com/anthropics/claude-code/issues/3417) — Hooks don't work on Windows
|
||||
- [anthropics/claude-code#9758](https://github.com/anthropics/claude-code/issues/9758) - .sh scripts open in editor on Windows
|
||||
- [anthropics/claude-code#3417](https://github.com/anthropics/claude-code/issues/3417) - Hooks don't work on Windows
|
||||
- [anthropics/claude-code#6023](https://github.com/anthropics/claude-code/issues/6023) - CLAUDE_PROJECT_DIR not found
|
||||
|
||||
2
evals
2
evals
Submodule evals updated: ff3ee83f94...e2b37138c8
@@ -52,7 +52,6 @@ EXCLUDES=(
|
||||
"/.gitattributes"
|
||||
"/.github/"
|
||||
"/.gitignore"
|
||||
"/.kimi-plugin/"
|
||||
"/.opencode/"
|
||||
"/.pi/"
|
||||
"/.version-bump.json"
|
||||
|
||||
@@ -7,7 +7,6 @@ const path = require('path');
|
||||
|
||||
const OPCODES = { TEXT: 0x01, CLOSE: 0x08, PING: 0x09, PONG: 0x0A };
|
||||
const WS_MAGIC = '258EAFA5-E914-47DA-95CA-C5AB0DC85B11';
|
||||
const MAX_FRAME_PAYLOAD_BYTES = 10 * 1024 * 1024;
|
||||
|
||||
function computeAcceptKey(clientKey) {
|
||||
return crypto.createHash('sha1').update(clientKey + WS_MAGIC).digest('base64');
|
||||
@@ -54,18 +53,10 @@ function decodeFrame(buffer) {
|
||||
offset = 4;
|
||||
} else if (payloadLen === 127) {
|
||||
if (buffer.length < 10) return null;
|
||||
const extendedLen = buffer.readBigUInt64BE(2);
|
||||
if (extendedLen > BigInt(MAX_FRAME_PAYLOAD_BYTES)) {
|
||||
throw new Error('WebSocket frame payload exceeds maximum allowed size');
|
||||
}
|
||||
payloadLen = Number(extendedLen);
|
||||
payloadLen = Number(buffer.readBigUInt64BE(2));
|
||||
offset = 10;
|
||||
}
|
||||
|
||||
if (payloadLen > MAX_FRAME_PAYLOAD_BYTES) {
|
||||
throw new Error('WebSocket frame payload exceeds maximum allowed size');
|
||||
}
|
||||
|
||||
const maskOffset = offset;
|
||||
const dataOffset = offset + 4;
|
||||
const totalLen = dataOffset + payloadLen;
|
||||
@@ -360,4 +351,4 @@ if (require.main === module) {
|
||||
startServer();
|
||||
}
|
||||
|
||||
module.exports = { computeAcceptKey, encodeFrame, decodeFrame, OPCODES, MAX_FRAME_PAYLOAD_BYTES };
|
||||
module.exports = { computeAcceptKey, encodeFrame, decodeFrame, OPCODES };
|
||||
|
||||
@@ -107,23 +107,10 @@ if [[ -z "$OWNER_PID" || "$OWNER_PID" == "1" ]]; then
|
||||
OWNER_PID="$PPID"
|
||||
fi
|
||||
|
||||
# Windows/MSYS2: Node.js cannot see POSIX PIDs from the MSYS2 namespace.
|
||||
# Passing a PID node cannot verify causes server to log owner-pid-invalid
|
||||
# and self-terminate at the 60-second lifecycle check. Clear it so the
|
||||
# watchdog is disabled and the idle timeout becomes the only shutdown trigger.
|
||||
case "${OSTYPE:-}" in
|
||||
msys*|cygwin*|mingw*) OWNER_PID="" ;;
|
||||
esac
|
||||
if [[ -n "${MSYSTEM:-}" ]]; then
|
||||
OWNER_PID=""
|
||||
fi
|
||||
|
||||
# Foreground mode for environments that reap detached/background processes.
|
||||
if [[ "$FOREGROUND" == "true" ]]; then
|
||||
env BRAINSTORM_DIR="$SESSION_DIR" BRAINSTORM_HOST="$BIND_HOST" BRAINSTORM_URL_HOST="$URL_HOST" BRAINSTORM_OWNER_PID="$OWNER_PID" node server.cjs &
|
||||
SERVER_PID=$!
|
||||
echo "$SERVER_PID" > "$PID_FILE"
|
||||
wait "$SERVER_PID"
|
||||
echo "$$" > "$PID_FILE"
|
||||
env BRAINSTORM_DIR="$SESSION_DIR" BRAINSTORM_HOST="$BIND_HOST" BRAINSTORM_URL_HOST="$URL_HOST" BRAINSTORM_OWNER_PID="$OWNER_PID" node server.cjs
|
||||
exit $?
|
||||
fi
|
||||
|
||||
|
||||
@@ -5,11 +5,11 @@ description: Use when executing implementation plans with independent tasks in t
|
||||
|
||||
# Subagent-Driven Development
|
||||
|
||||
Execute plan by dispatching a fresh implementer subagent per task, a combined task review (spec compliance + code quality, one reviewer, one reading of the diff) after each, and a broad whole-branch review at the end.
|
||||
Execute plan by dispatching fresh subagent per task, with two-stage review after each: spec compliance review first, then code quality review.
|
||||
|
||||
**Why subagents:** You delegate tasks to specialized agents with isolated context. By precisely crafting their instructions and context, you ensure they stay focused and succeed at their task. They should never inherit your session's context or history — you construct exactly what they need. This also preserves your own context for coordination work.
|
||||
|
||||
**Core principle:** Fresh subagent per task + one task review (spec + quality verdicts) + broad final review = high quality, fast iteration
|
||||
**Core principle:** Fresh subagent per task + two-stage review (spec then quality) = high quality, fast iteration
|
||||
|
||||
**Continuous execution:** Do not pause to check in with your human partner between tasks. Execute all tasks from the plan without stopping. The only reasons to stop are: BLOCKED status you cannot resolve, ambiguity that genuinely prevents progress, or all tasks complete. "Should I continue?" prompts and progress summaries waste their time — they asked you to execute the plan, so execute it.
|
||||
|
||||
@@ -36,7 +36,7 @@ digraph when_to_use {
|
||||
**vs. Executing Plans (parallel session):**
|
||||
- Same session (no context switch)
|
||||
- Fresh subagent per task (no context pollution)
|
||||
- Combined review after each task (spec compliance + code quality verdicts), broad review at the end
|
||||
- Two-stage review after each task: spec compliance first, then code quality
|
||||
- Faster iteration (no human-in-loop between tasks)
|
||||
|
||||
## The Process
|
||||
@@ -51,15 +51,18 @@ digraph process {
|
||||
"Implementer subagent asks questions?" [shape=diamond];
|
||||
"Answer questions, provide context" [shape=box];
|
||||
"Implementer subagent implements, tests, commits, self-reviews" [shape=box];
|
||||
"Run git diff, dispatch task reviewer subagent (./task-reviewer-prompt.md)" [shape=box];
|
||||
"Task reviewer reports spec ✅ and quality approved?" [shape=diamond];
|
||||
"Dispatch fix subagent for Critical/Important findings" [shape=box];
|
||||
"Dispatch spec reviewer subagent (./spec-reviewer-prompt.md)" [shape=box];
|
||||
"Spec reviewer subagent confirms code matches spec?" [shape=diamond];
|
||||
"Implementer subagent fixes spec gaps" [shape=box];
|
||||
"Dispatch code quality reviewer subagent (./code-quality-reviewer-prompt.md)" [shape=box];
|
||||
"Code quality reviewer subagent approves?" [shape=diamond];
|
||||
"Implementer subagent fixes quality issues" [shape=box];
|
||||
"Mark task complete in todo list" [shape=box];
|
||||
}
|
||||
|
||||
"Read plan, extract all tasks with full text, note context, create todos" [shape=box];
|
||||
"More tasks remain?" [shape=diamond];
|
||||
"Dispatch final code reviewer subagent (../requesting-code-review/code-reviewer.md)" [shape=box];
|
||||
"Dispatch final code reviewer subagent for entire implementation" [shape=box];
|
||||
"Use superpowers:finishing-a-development-branch" [shape=box style=filled fillcolor=lightgreen];
|
||||
|
||||
"Read plan, extract all tasks with full text, note context, create todos" -> "Dispatch implementer subagent (./implementer-prompt.md)";
|
||||
@@ -67,15 +70,19 @@ digraph process {
|
||||
"Implementer subagent asks questions?" -> "Answer questions, provide context" [label="yes"];
|
||||
"Answer questions, provide context" -> "Dispatch implementer subagent (./implementer-prompt.md)";
|
||||
"Implementer subagent asks questions?" -> "Implementer subagent implements, tests, commits, self-reviews" [label="no"];
|
||||
"Implementer subagent implements, tests, commits, self-reviews" -> "Run git diff, dispatch task reviewer subagent (./task-reviewer-prompt.md)";
|
||||
"Run git diff, dispatch task reviewer subagent (./task-reviewer-prompt.md)" -> "Task reviewer reports spec ✅ and quality approved?";
|
||||
"Task reviewer reports spec ✅ and quality approved?" -> "Dispatch fix subagent for Critical/Important findings" [label="no"];
|
||||
"Dispatch fix subagent for Critical/Important findings" -> "Run git diff, dispatch task reviewer subagent (./task-reviewer-prompt.md)" [label="re-review"];
|
||||
"Task reviewer reports spec ✅ and quality approved?" -> "Mark task complete in todo list" [label="yes"];
|
||||
"Implementer subagent implements, tests, commits, self-reviews" -> "Dispatch spec reviewer subagent (./spec-reviewer-prompt.md)";
|
||||
"Dispatch spec reviewer subagent (./spec-reviewer-prompt.md)" -> "Spec reviewer subagent confirms code matches spec?";
|
||||
"Spec reviewer subagent confirms code matches spec?" -> "Implementer subagent fixes spec gaps" [label="no"];
|
||||
"Implementer subagent fixes spec gaps" -> "Dispatch spec reviewer subagent (./spec-reviewer-prompt.md)" [label="re-review"];
|
||||
"Spec reviewer subagent confirms code matches spec?" -> "Dispatch code quality reviewer subagent (./code-quality-reviewer-prompt.md)" [label="yes"];
|
||||
"Dispatch code quality reviewer subagent (./code-quality-reviewer-prompt.md)" -> "Code quality reviewer subagent approves?";
|
||||
"Code quality reviewer subagent approves?" -> "Implementer subagent fixes quality issues" [label="no"];
|
||||
"Implementer subagent fixes quality issues" -> "Dispatch code quality reviewer subagent (./code-quality-reviewer-prompt.md)" [label="re-review"];
|
||||
"Code quality reviewer subagent approves?" -> "Mark task complete in todo list" [label="yes"];
|
||||
"Mark task complete in todo list" -> "More tasks remain?";
|
||||
"More tasks remain?" -> "Dispatch implementer subagent (./implementer-prompt.md)" [label="yes"];
|
||||
"More tasks remain?" -> "Dispatch final code reviewer subagent (../requesting-code-review/code-reviewer.md)" [label="no"];
|
||||
"Dispatch final code reviewer subagent (../requesting-code-review/code-reviewer.md)" -> "Use superpowers:finishing-a-development-branch";
|
||||
"More tasks remain?" -> "Dispatch final code reviewer subagent for entire implementation" [label="no"];
|
||||
"Dispatch final code reviewer subagent for entire implementation" -> "Use superpowers:finishing-a-development-branch";
|
||||
}
|
||||
```
|
||||
|
||||
@@ -87,23 +94,9 @@ Use the least powerful model that can handle each role to conserve cost and incr
|
||||
|
||||
**Integration and judgment tasks** (multi-file coordination, pattern matching, debugging): use a standard model.
|
||||
|
||||
**Architecture and design tasks**: use the most capable available model.
|
||||
**Architecture, design, and review tasks**: use the most capable available model.
|
||||
|
||||
**Review tasks**: choose the model with the same judgment, scaled to the
|
||||
diff's size, complexity, and risk. A small mechanical diff does not need the
|
||||
most capable model; a subtle concurrency change does.
|
||||
|
||||
**Always specify the model explicitly when dispatching a subagent.** An
|
||||
omitted model inherits your session's model — often the most capable and
|
||||
most expensive — which silently defeats this section.
|
||||
|
||||
**Turn count beats token price.** Wall-clock and context cost scale with how
|
||||
many turns a subagent takes, and the cheapest models routinely take 2-3× the
|
||||
turns on multi-step work — costing more overall. Use a mid-tier model as the
|
||||
floor for implementers and reviewers; reserve the cheapest tier for
|
||||
single-file mechanical fixes.
|
||||
|
||||
**Task complexity signals (implementation tasks):**
|
||||
**Task complexity signals:**
|
||||
- Touches 1-2 files with a complete spec → cheap model
|
||||
- Touches multiple files with integration concerns → standard model
|
||||
- Requires design judgment or broad codebase understanding → most capable model
|
||||
@@ -112,7 +105,7 @@ single-file mechanical fixes.
|
||||
|
||||
Implementer subagents report one of four statuses. Handle each appropriately:
|
||||
|
||||
**DONE:** Run `git diff BASE..HEAD`, then dispatch the task reviewer.
|
||||
**DONE:** Proceed to spec compliance review.
|
||||
|
||||
**DONE_WITH_CONCERNS:** The implementer completed the work but flagged doubts. Read the concerns before proceeding. If the concerns are about correctness or scope, address them before review. If they're observations (e.g., "this file is getting large"), note them and proceed to review.
|
||||
|
||||
@@ -126,48 +119,11 @@ Implementer subagents report one of four statuses. Handle each appropriately:
|
||||
|
||||
**Never** ignore an escalation or force the same model to retry without changes. If the implementer said it's stuck, something needs to change.
|
||||
|
||||
## Handling Reviewer ⚠️ Items
|
||||
|
||||
The task reviewer may report "⚠️ Cannot verify from diff" items — requirements
|
||||
that live in unchanged code or span tasks. These do not block the rest of the
|
||||
review, but you must resolve each one yourself before marking the task
|
||||
complete: you hold the plan and cross-task context the reviewer
|
||||
lacks. If you confirm an item is a real gap, treat it as a failed spec
|
||||
review — send it back to the implementer and re-review.
|
||||
|
||||
## Constructing Reviewer Prompts
|
||||
|
||||
Per-task reviews are task-scoped gates. The broad review happens once, at the
|
||||
final whole-branch review. When you fill a reviewer template:
|
||||
|
||||
- Do not add open-ended directives like "check all uses" or "run race tests
|
||||
if useful" without a concrete, task-specific reason
|
||||
- Do not ask a reviewer to re-run tests the implementer already ran on the
|
||||
same code — the implementer's report carries the test evidence
|
||||
- Do not pre-judge findings for the reviewer — never instruct a reviewer to
|
||||
ignore or not flag a specific issue. If you believe a finding would be a
|
||||
false positive, let the reviewer raise it and adjudicate it in the review
|
||||
loop. If the prompt you are writing contains "do not flag," "don't treat X
|
||||
as a defect," "at most Minor," or "the plan chose" — stop: you are
|
||||
pre-judging, usually to spare yourself a review loop.
|
||||
- Include the spec/design's global constraints that bind the task (version
|
||||
floors, naming and copy rules, platform requirements) in the requirements
|
||||
you paste — a reviewer can only enforce what you hand them.
|
||||
- Hand the reviewer its diff as a file: run
|
||||
`git diff BASE..HEAD > /tmp/sdd-task-N.diff` (redirected, so the diff
|
||||
never enters your own context) and put that path in the prompt. The
|
||||
reviewer then sees the whole change in one Read call instead of
|
||||
re-deriving it with git commands.
|
||||
- Dispatch fix subagents for Critical and Important findings. Record Minor
|
||||
findings and move on — then paste the accumulated Minor findings into the
|
||||
final whole-branch review dispatch so it can triage which must be fixed
|
||||
before merge. A roll-up nobody reads is a silent discard.
|
||||
|
||||
## Prompt Templates
|
||||
|
||||
- [implementer-prompt.md](implementer-prompt.md) - Dispatch implementer subagent
|
||||
- [task-reviewer-prompt.md](task-reviewer-prompt.md) - Dispatch task reviewer subagent (spec compliance + code quality, one dispatch)
|
||||
- Final whole-branch review: use superpowers:requesting-code-review's [code-reviewer.md](../requesting-code-review/code-reviewer.md)
|
||||
- [spec-reviewer-prompt.md](spec-reviewer-prompt.md) - Dispatch spec compliance reviewer subagent
|
||||
- [code-quality-reviewer-prompt.md](code-quality-reviewer-prompt.md) - Dispatch code quality reviewer subagent
|
||||
|
||||
## Example Workflow
|
||||
|
||||
@@ -194,9 +150,11 @@ Implementer: "Got it. Implementing now..."
|
||||
- Self-review: Found I missed --force flag, added it
|
||||
- Committed
|
||||
|
||||
[Run git diff, dispatch task reviewer with the diff pasted in]
|
||||
Task reviewer: Spec ✅ - all requirements met, nothing extra.
|
||||
Strengths: Good test coverage, clean. Issues: None. Task quality: Approved.
|
||||
[Dispatch spec compliance reviewer]
|
||||
Spec reviewer: ✅ Spec compliant - all requirements met, nothing extra
|
||||
|
||||
[Get git SHAs, dispatch code quality reviewer]
|
||||
Code reviewer: Strengths: Good test coverage, clean. Issues: None. Approved.
|
||||
|
||||
[Mark Task 1 complete]
|
||||
|
||||
@@ -212,17 +170,25 @@ Implementer:
|
||||
- Self-review: All good
|
||||
- Committed
|
||||
|
||||
[Run git diff, dispatch task reviewer with the diff pasted in]
|
||||
Task reviewer: Spec ❌:
|
||||
[Dispatch spec compliance reviewer]
|
||||
Spec reviewer: ❌ Issues:
|
||||
- Missing: Progress reporting (spec says "report every 100 items")
|
||||
- Extra: Added --json flag (not requested)
|
||||
Issues (Important): Magic number (100)
|
||||
|
||||
[Dispatch fix subagent with all findings]
|
||||
Fixer: Removed --json flag, added progress reporting, extracted PROGRESS_INTERVAL constant
|
||||
[Implementer fixes issues]
|
||||
Implementer: Removed --json flag, added progress reporting
|
||||
|
||||
[Task reviewer reviews again]
|
||||
Task reviewer: Spec ✅. Task quality: Approved.
|
||||
[Spec reviewer reviews again]
|
||||
Spec reviewer: ✅ Spec compliant now
|
||||
|
||||
[Dispatch code quality reviewer]
|
||||
Code reviewer: Strengths: Solid. Issues (Important): Magic number (100)
|
||||
|
||||
[Implementer fixes]
|
||||
Implementer: Extracted PROGRESS_INTERVAL constant
|
||||
|
||||
[Code reviewer reviews again]
|
||||
Code reviewer: ✅ Approved
|
||||
|
||||
[Mark Task 2 complete]
|
||||
|
||||
@@ -256,13 +222,13 @@ Done!
|
||||
|
||||
**Quality gates:**
|
||||
- Self-review catches issues before handoff
|
||||
- Task review carries two verdicts: spec compliance and code quality
|
||||
- Two-stage review: spec compliance, then code quality
|
||||
- Review loops ensure fixes actually work
|
||||
- Spec compliance prevents over/under-building
|
||||
- Code quality ensures implementation is well-built
|
||||
|
||||
**Cost:**
|
||||
- More subagent invocations (implementer + reviewer per task)
|
||||
- More subagent invocations (implementer + 2 reviewers per task)
|
||||
- Controller does more prep work (extracting all tasks upfront)
|
||||
- Review loops add iterations
|
||||
- But catches issues early (cheaper than debugging later)
|
||||
@@ -271,22 +237,17 @@ Done!
|
||||
|
||||
**Never:**
|
||||
- Start implementation on main/master branch without explicit user consent
|
||||
- Skip task review, or accept a report missing either verdict (spec compliance AND task quality are both required)
|
||||
- Skip reviews (spec compliance OR code quality)
|
||||
- Proceed with unfixed issues
|
||||
- Dispatch multiple implementation subagents in parallel (conflicts)
|
||||
- Make subagent read plan file (provide full text instead)
|
||||
- Skip scene-setting context (subagent needs to understand where task fits)
|
||||
- Ignore subagent questions (answer before letting them proceed)
|
||||
- Accept "close enough" on spec compliance (reviewer found spec issues = not done)
|
||||
- Accept "close enough" on spec compliance (spec reviewer found issues = not done)
|
||||
- Skip review loops (reviewer found issues = implementer fixes = review again)
|
||||
- Let implementer self-review replace actual review (both are needed)
|
||||
- Tell a reviewer what not to flag, or pre-rate a finding's severity in the
|
||||
dispatch prompt ("treat it as Minor at most") — the plan's example code is
|
||||
a starting point, not evidence that its weaknesses were chosen
|
||||
- Dispatch a task reviewer without a diff file — run
|
||||
`git diff BASE..HEAD > /tmp/sdd-task-N.diff` first and name that path in
|
||||
the prompt
|
||||
- Move to next task while the review has open Critical/Important issues
|
||||
- **Start code quality review before spec compliance is ✅** (wrong order)
|
||||
- Move to next task while either review has open issues
|
||||
|
||||
**If subagent asks questions:**
|
||||
- Answer clearly and completely
|
||||
@@ -308,7 +269,7 @@ Done!
|
||||
**Required workflow skills:**
|
||||
- **superpowers:using-git-worktrees** - Ensures isolated workspace (creates one or verifies existing)
|
||||
- **superpowers:writing-plans** - Creates the plan this skill executes
|
||||
- **superpowers:requesting-code-review** - Code review template for the final whole-branch review
|
||||
- **superpowers:requesting-code-review** - Code review template for reviewer subagents
|
||||
- **superpowers:finishing-a-development-branch** - Complete development after all tasks
|
||||
|
||||
**Subagents should use:**
|
||||
|
||||
@@ -0,0 +1,25 @@
|
||||
# Code Quality Reviewer Prompt Template
|
||||
|
||||
Use this template when dispatching a code quality reviewer subagent.
|
||||
|
||||
**Purpose:** Verify implementation is well-built (clean, tested, maintainable)
|
||||
|
||||
**Only dispatch after spec compliance review passes.**
|
||||
|
||||
```
|
||||
Subagent (general-purpose):
|
||||
Use template at ../requesting-code-review/code-reviewer.md
|
||||
|
||||
DESCRIPTION: [task summary, from implementer's report]
|
||||
PLAN_OR_REQUIREMENTS: Task N from [plan-file]
|
||||
BASE_SHA: [commit before task]
|
||||
HEAD_SHA: [current commit]
|
||||
```
|
||||
|
||||
**In addition to standard code quality concerns, the reviewer should check:**
|
||||
- Does each file have one clear responsibility with a well-defined interface?
|
||||
- Are units decomposed so they can be understood and tested independently?
|
||||
- Is the implementation following the file structure from the plan?
|
||||
- Did this implementation create new files that are already large, or significantly grow existing files? (Don't flag pre-existing file sizes — focus on what this change contributed.)
|
||||
|
||||
**Code reviewer returns:** Strengths, Issues (Critical/Important/Minor), Assessment
|
||||
@@ -41,9 +41,6 @@ Subagent (general-purpose):
|
||||
**While you work:** If you encounter something unexpected or unclear, **ask questions**.
|
||||
It's always OK to pause and clarify. Don't guess or make assumptions.
|
||||
|
||||
While iterating, run the focused test for what you're changing; run the
|
||||
full suite once before committing, not after every edit.
|
||||
|
||||
## Code Organization
|
||||
|
||||
You reason best about code you can hold in context at once, and your edits are more
|
||||
@@ -97,25 +94,15 @@ Subagent (general-purpose):
|
||||
- Do tests actually verify behavior (not just mock behavior)?
|
||||
- Did I follow TDD if required?
|
||||
- Are tests comprehensive?
|
||||
- Is the test output pristine (no stray warnings or noise)?
|
||||
|
||||
If you find issues during self-review, fix them now before reporting.
|
||||
|
||||
## After Review Findings
|
||||
|
||||
If a reviewer finds issues and you fix them, re-run the tests that cover
|
||||
the amended code and include the results in your fix report. Reviewers
|
||||
will not re-run tests for you — your report is the test evidence.
|
||||
|
||||
## Report Format
|
||||
|
||||
When done, report:
|
||||
- **Status:** DONE | DONE_WITH_CONCERNS | BLOCKED | NEEDS_CONTEXT
|
||||
- What you implemented (or what you attempted, if blocked)
|
||||
- What you tested and test results
|
||||
- **TDD Evidence** (if TDD was required for this task):
|
||||
- RED: command run, relevant failing output before implementation, and why the failure was expected
|
||||
- GREEN: command run and relevant passing output after implementation
|
||||
- Files changed
|
||||
- Self-review findings (if any)
|
||||
- Any issues or concerns
|
||||
|
||||
77
skills/subagent-driven-development/spec-reviewer-prompt.md
Normal file
77
skills/subagent-driven-development/spec-reviewer-prompt.md
Normal file
@@ -0,0 +1,77 @@
|
||||
# Spec Compliance Reviewer Prompt Template
|
||||
|
||||
Use this template when dispatching a spec compliance reviewer subagent.
|
||||
|
||||
**Purpose:** Verify implementer built what was requested (nothing more, nothing less)
|
||||
|
||||
```
|
||||
Subagent (general-purpose):
|
||||
description: "Review spec compliance for Task N"
|
||||
prompt: |
|
||||
You are reviewing whether an implementation matches its specification.
|
||||
|
||||
## What Was Requested
|
||||
|
||||
[FULL TEXT of task requirements]
|
||||
|
||||
## What Implementer Claims They Built
|
||||
|
||||
[From implementer's report]
|
||||
|
||||
## Git Range to Review
|
||||
|
||||
**Base:** [BASE_SHA — commit before this task]
|
||||
**Head:** [HEAD_SHA — current commit]
|
||||
|
||||
```bash
|
||||
git diff --stat [BASE_SHA]..[HEAD_SHA]
|
||||
git diff [BASE_SHA]..[HEAD_SHA]
|
||||
```
|
||||
|
||||
Only read files in this diff. Do not crawl the broader codebase.
|
||||
|
||||
## Read-Only Review
|
||||
|
||||
Your review is read-only on this checkout. Do not mutate the working tree, the index, HEAD, or branch state in any way. Use tools like `git show`, `git diff`, and `git log` to inspect history. If you need a working copy of a different revision, check it out into a separate temporary directory (e.g. `git worktree add /tmp/review-[SHA] [SHA]`) — never move HEAD on this checkout.
|
||||
|
||||
## CRITICAL: Do Not Trust the Report
|
||||
|
||||
The implementer finished suspiciously quickly. Their report may be incomplete,
|
||||
inaccurate, or optimistic. You MUST verify everything independently.
|
||||
|
||||
**DO NOT:**
|
||||
- Take their word for what they implemented
|
||||
- Trust their claims about completeness
|
||||
- Accept their interpretation of requirements
|
||||
|
||||
**DO:**
|
||||
- Read the actual code they wrote
|
||||
- Compare actual implementation to requirements line by line
|
||||
- Check for missing pieces they claimed to implement
|
||||
- Look for extra features they didn't mention
|
||||
|
||||
## Your Job
|
||||
|
||||
Read the implementation code and verify:
|
||||
|
||||
**Missing requirements:**
|
||||
- Did they implement everything that was requested?
|
||||
- Are there requirements they skipped or missed?
|
||||
- Did they claim something works but didn't actually implement it?
|
||||
|
||||
**Extra/unneeded work:**
|
||||
- Did they build things that weren't requested?
|
||||
- Did they over-engineer or add unnecessary features?
|
||||
- Did they add "nice to haves" that weren't in spec?
|
||||
|
||||
**Misunderstandings:**
|
||||
- Did they interpret requirements differently than intended?
|
||||
- Did they solve the wrong problem?
|
||||
- Did they implement the right feature but wrong way?
|
||||
|
||||
**Verify by reading code, not by trusting report.**
|
||||
|
||||
Report:
|
||||
- ✅ Spec compliant (if everything matches after code inspection)
|
||||
- ❌ Issues found: [list specifically what's missing or extra, with file:line references]
|
||||
```
|
||||
@@ -1,162 +0,0 @@
|
||||
# Task Reviewer Prompt Template
|
||||
|
||||
Use this template when dispatching a task reviewer subagent. One reviewer, one
|
||||
reading of the diff, two verdicts: spec compliance and code quality.
|
||||
|
||||
**Purpose:** Verify one task's implementation matches its requirements (nothing
|
||||
more, nothing less) and is well-built (clean, tested, maintainable)
|
||||
|
||||
```
|
||||
Subagent (general-purpose):
|
||||
description: "Review Task N (spec + quality)"
|
||||
prompt: |
|
||||
You are reviewing one task's implementation: first whether it matches its
|
||||
requirements, then whether it is well-built. This is a task-scoped gate,
|
||||
not a merge review — a broad whole-branch review happens separately after
|
||||
all tasks are complete.
|
||||
|
||||
## What Was Requested
|
||||
|
||||
[TASK_REQUIREMENTS]
|
||||
|
||||
## What the Implementer Claims They Built
|
||||
|
||||
[DESCRIPTION]
|
||||
|
||||
## Diff Under Review
|
||||
|
||||
**Base:** [BASE_SHA]
|
||||
**Head:** [HEAD_SHA]
|
||||
**Diff file:** [DIFF_FILE]
|
||||
|
||||
Read the diff file once — that single Read is your view of the change.
|
||||
Do not re-run git commands or re-read the files it already shows. If
|
||||
the diff file is missing, fetch the diff yourself:
|
||||
`git diff --stat [BASE_SHA]..[HEAD_SHA]` and `git diff [BASE_SHA]..[HEAD_SHA]`.
|
||||
Only read files in this diff. Do not crawl the broader codebase. Inspect
|
||||
code outside the diff only to evaluate a concrete risk you can name — and
|
||||
name it in your report. Cross-cutting changes are legitimate named risks:
|
||||
if the diff changes lock ordering, a function or API contract, or shared
|
||||
mutable state, checking the call sites is the right method.
|
||||
|
||||
Your review is read-only on this checkout. Do not mutate the working
|
||||
tree, the index, HEAD, or branch state in any way.
|
||||
|
||||
## Do Not Trust the Report
|
||||
|
||||
Treat the implementer's report as unverified claims about the code. It
|
||||
may be incomplete, inaccurate, or optimistic. Verify the claims against
|
||||
the diff. Design rationales in the report are claims too: "left it per
|
||||
YAGNI," "kept it simple deliberately," or any other justification is the
|
||||
implementer grading their own work. Judge the code on its merits — a
|
||||
stated rationale never downgrades a finding's severity.
|
||||
|
||||
## Tests
|
||||
|
||||
The implementer already ran the tests and reported results with TDD
|
||||
evidence for exactly this code. Do not re-run the suite to confirm their
|
||||
report. Run a test only when reading the code raises a specific doubt
|
||||
that no existing run answers — and then a focused test, never a
|
||||
package-wide suite, race detector run, or repeated/high-count loop. If
|
||||
heavy validation seems warranted, recommend it in your report instead of
|
||||
running it. If you cannot run commands in this environment, name the
|
||||
test you would run.
|
||||
|
||||
Warnings or other noise in the implementer's reported test output are
|
||||
findings — test output should be pristine.
|
||||
|
||||
## Part 1: Spec Compliance
|
||||
|
||||
Compare the diff against What Was Requested:
|
||||
|
||||
- **Missing:** requirements they skipped, missed, or claimed without
|
||||
implementing
|
||||
- **Extra:** features that weren't requested, over-engineering, unneeded
|
||||
"nice to haves"
|
||||
- **Misunderstood:** right feature built the wrong way, wrong problem
|
||||
solved
|
||||
|
||||
If a requirement cannot be verified from this diff alone (it lives in
|
||||
unchanged code or spans tasks), report it as a ⚠️ item instead of
|
||||
broadening your search.
|
||||
|
||||
## Part 2: Code Quality
|
||||
|
||||
**Code quality:**
|
||||
- Clean separation of concerns?
|
||||
- Proper error handling?
|
||||
- DRY without premature abstraction?
|
||||
- Edge cases handled?
|
||||
|
||||
**Tests:**
|
||||
- Do the new and changed tests verify real behavior, not mocks?
|
||||
- Are the task's edge cases covered?
|
||||
|
||||
**Structure:**
|
||||
- Does each file have one clear responsibility with a well-defined interface?
|
||||
- Are units decomposed so they can be understood and tested independently?
|
||||
- Is the implementation following the file structure from the plan?
|
||||
- Did this change create new files that are already large, or
|
||||
significantly grow existing files? (Don't flag pre-existing file
|
||||
sizes — focus on what this change contributed.)
|
||||
|
||||
Cite file:line evidence for every finding and for any check you would
|
||||
otherwise answer with a bare "yes." Cite, don't narrate — a tight report
|
||||
that points at lines beats a long one that retells the diff.
|
||||
|
||||
## Calibration
|
||||
|
||||
Categorize issues by actual severity. Not everything is Critical.
|
||||
Important means this task cannot be trusted until it is fixed: incorrect
|
||||
or fragile behavior, a missed requirement, or maintainability damage you
|
||||
would block a merge over — verbatim duplication of a logic block,
|
||||
swallowed errors, tests that assert nothing. "Coverage could be broader"
|
||||
and polish suggestions are Minor.
|
||||
Acknowledge what was done well before listing issues — accurate praise
|
||||
helps the implementer trust the rest of the feedback.
|
||||
|
||||
## Output Format
|
||||
|
||||
### Spec Compliance
|
||||
|
||||
- ✅ Spec compliant | ❌ Issues found: [what's missing/extra/misunderstood,
|
||||
with file:line references]
|
||||
- ⚠️ Cannot verify from diff: [requirements you could not verify from the
|
||||
diff alone, and what the controller should check — report alongside the
|
||||
✅/❌ verdict for everything you could verify]
|
||||
|
||||
### Strengths
|
||||
[What's well done? Be specific.]
|
||||
|
||||
### Issues
|
||||
|
||||
#### Critical (Must Fix)
|
||||
#### Important (Should Fix)
|
||||
#### Minor (Nice to Have)
|
||||
|
||||
For each issue: file:line, what's wrong, why it matters, how to fix
|
||||
(if not obvious).
|
||||
|
||||
### Assessment
|
||||
|
||||
**Task quality:** [Approved | Needs fixes]
|
||||
|
||||
**Reasoning:** [1-2 sentence technical assessment]
|
||||
```
|
||||
|
||||
**Placeholders:**
|
||||
- `[TASK_REQUIREMENTS]` — full task text plus the spec/design's global
|
||||
constraints that bind it (version floors, naming and copy rules, platform
|
||||
requirements)
|
||||
- `[DESCRIPTION]` — what the implementer reports they built
|
||||
- `[BASE_SHA]` — commit before this task
|
||||
- `[HEAD_SHA]` — current commit
|
||||
- `[DIFF_FILE]` — REQUIRED: the path the controller wrote the diff to
|
||||
(`git diff BASE..HEAD > /tmp/sdd-task-N.diff`, redirected so it never
|
||||
enters the controller's context)
|
||||
|
||||
**Reviewer returns:** Spec Compliance verdict (✅/❌/⚠️), Strengths, Issues
|
||||
(Critical/Important/Minor), Task quality verdict
|
||||
|
||||
A single fix dispatch can then address spec gaps and quality findings
|
||||
together; re-review after fixes covers both verdicts again.
|
||||
@@ -64,7 +64,7 @@ prompt-template file (e.g. `superpowers:subagent-driven-development`'s
|
||||
| Skill dispatch form | Antigravity equivalent |
|
||||
|---------------------|----------------------|
|
||||
| An implementer-style `*-prompt.md` template (writes code, runs tests) | Fill the template, then `invoke_subagent` with `TypeName: "self"` and the filled prompt |
|
||||
| A read-only reviewer template (`task-reviewer`, `code-reviewer`, `requesting-code-review`'s `./code-reviewer.md`) | `invoke_subagent` with `TypeName: "research"` and the filled review template |
|
||||
| A read-only reviewer template (`spec-reviewer`, `code-quality-reviewer`, `code-reviewer`, `requesting-code-review`'s `./code-reviewer.md`) | `invoke_subagent` with `TypeName: "research"` and the filled review template |
|
||||
| Inline prompt (no template referenced) | `invoke_subagent` with `TypeName: "self"` (or `"research"` if the task only reads) and your inline prompt |
|
||||
|
||||
### Prompt filling
|
||||
|
||||
@@ -35,7 +35,7 @@ Skills dispatch with `Subagent (general-purpose):` and either reference a prompt
|
||||
|
||||
| Skill dispatch form | Gemini CLI equivalent |
|
||||
|---------------------|----------------------|
|
||||
| References a `*-prompt.md` template (implementer, task-reviewer, code-reviewer, etc.) | Fill the template, then `invoke_agent` with `agent_name: "generalist"` and the filled prompt |
|
||||
| References a `*-prompt.md` template (implementer, spec-reviewer, code-quality-reviewer, code-reviewer, etc.) | Fill the template, then `invoke_agent` with `agent_name: "generalist"` and the filled prompt |
|
||||
| References `superpowers:requesting-code-review`'s `./code-reviewer.md` | `invoke_agent` with `agent_name: "generalist"` and the filled review template |
|
||||
| Inline prompt (no template referenced) | `invoke_agent` with `agent_name: "generalist"` and your inline prompt |
|
||||
|
||||
|
||||
@@ -329,21 +329,6 @@ function runTests() {
|
||||
assert.strictEqual(result.payload.length, 65536);
|
||||
});
|
||||
|
||||
test('rejects oversized 64-bit frames before payload allocation', () => {
|
||||
const mask = Buffer.from([0x00, 0x00, 0x00, 0x00]);
|
||||
const header = Buffer.alloc(14);
|
||||
header[0] = 0x81; // FIN + TEXT
|
||||
header[1] = 0x80 | 127; // masked, 64-bit length
|
||||
header.writeBigUInt64BE(BigInt(ws.MAX_FRAME_PAYLOAD_BYTES) + 1n, 2);
|
||||
mask.copy(header, 10);
|
||||
|
||||
assert.throws(
|
||||
() => ws.decodeFrame(header),
|
||||
/exceeds maximum allowed size/i,
|
||||
'oversized advertised payload must be rejected from header alone'
|
||||
);
|
||||
});
|
||||
|
||||
// ========== Close Frame with Status Code ==========
|
||||
console.log('\n--- Close Frame Details ---');
|
||||
|
||||
|
||||
@@ -175,7 +175,6 @@ write_upstream_fixture() {
|
||||
|
||||
mkdir -p \
|
||||
"$repo/.codex-plugin" \
|
||||
"$repo/.kimi-plugin" \
|
||||
"$repo/.private-journal" \
|
||||
"$repo/assets" \
|
||||
"$repo/evals/drill" \
|
||||
@@ -211,13 +210,6 @@ EOF
|
||||
"name": "superpowers",
|
||||
"version": "$MANIFEST_VERSION"
|
||||
}
|
||||
EOF
|
||||
|
||||
cat > "$repo/.kimi-plugin/plugin.json" <<EOF
|
||||
{
|
||||
"name": "superpowers",
|
||||
"version": "$MANIFEST_VERSION"
|
||||
}
|
||||
EOF
|
||||
|
||||
cat > "$repo/assets/superpowers-small.svg" <<'EOF'
|
||||
@@ -275,7 +267,6 @@ EOF
|
||||
|
||||
git -C "$repo" add \
|
||||
.codex-plugin/plugin.json \
|
||||
.kimi-plugin/plugin.json \
|
||||
.gitignore \
|
||||
assets/app-icon.png \
|
||||
assets/superpowers-small.svg \
|
||||
@@ -424,15 +415,10 @@ EOF
|
||||
write_stale_ignored_destination_fixture() {
|
||||
local repo="$1"
|
||||
|
||||
mkdir -p \
|
||||
"$repo/plugins/superpowers/.kimi-plugin" \
|
||||
"$repo/plugins/superpowers/.private-journal"
|
||||
mkdir -p "$repo/plugins/superpowers/.private-journal"
|
||||
printf 'fixture keep\n' > "$repo/plugins/superpowers/.fixture-keep"
|
||||
printf '{"name":"stale-kimi"}\n' > "$repo/plugins/superpowers/.kimi-plugin/plugin.json"
|
||||
printf 'stale ignored leak\n' > "$repo/plugins/superpowers/.private-journal/leak.txt"
|
||||
git -C "$repo" add \
|
||||
plugins/superpowers/.fixture-keep \
|
||||
plugins/superpowers/.kimi-plugin/plugin.json
|
||||
git -C "$repo" add plugins/superpowers/.fixture-keep
|
||||
|
||||
commit_fixture "$repo" "Initial stale ignored destination fixture"
|
||||
}
|
||||
@@ -632,7 +618,6 @@ main() {
|
||||
assert_contains "$preview_output" "Version: $MANIFEST_VERSION" "Preview uses manifest version"
|
||||
assert_not_contains "$preview_output" "Version: $PACKAGE_VERSION" "Preview does not use package.json version"
|
||||
assert_contains "$preview_section" ".codex-plugin/plugin.json" "Preview includes manifest path"
|
||||
assert_not_contains "$preview_section" ".kimi-plugin/plugin.json" "Preview excludes Kimi manifest from Codex sync"
|
||||
assert_contains "$preview_section" "assets/superpowers-small.svg" "Preview includes SVG asset"
|
||||
assert_contains "$preview_section" "assets/app-icon.png" "Preview includes PNG asset"
|
||||
assert_contains "$preview_section" "hooks/hooks-codex.json" "Preview includes Codex hook manifest"
|
||||
@@ -659,7 +644,6 @@ main() {
|
||||
echo ""
|
||||
echo "Convergence assertions..."
|
||||
assert_equals "$stale_preview_status" "0" "Stale ignored destination preview exits successfully"
|
||||
assert_matches "$stale_preview_section" "\\*deleting +\\.kimi-plugin/plugin\\.json" "Preview deletes stale Kimi manifest from Codex plugin"
|
||||
assert_matches "$stale_preview_section" "\\*deleting +\\.private-journal/leak\\.txt" "Preview deletes stale ignored destination file"
|
||||
|
||||
echo ""
|
||||
|
||||
@@ -1,6 +0,0 @@
|
||||
#!/usr/bin/env bash
|
||||
set -euo pipefail
|
||||
|
||||
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
|
||||
|
||||
bash "$SCRIPT_DIR/test-plugin-manifest.sh"
|
||||
@@ -1,86 +0,0 @@
|
||||
#!/usr/bin/env bash
|
||||
set -euo pipefail
|
||||
|
||||
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
|
||||
REPO_ROOT="$(cd "$SCRIPT_DIR/../.." && pwd)"
|
||||
MANIFEST="$REPO_ROOT/.kimi-plugin/plugin.json"
|
||||
|
||||
python3 - "$MANIFEST" <<'PY'
|
||||
import json
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
manifest_path = Path(sys.argv[1])
|
||||
manifest = json.loads(manifest_path.read_text(encoding="utf-8"))
|
||||
|
||||
def assert_equal(actual, expected, label):
|
||||
if actual != expected:
|
||||
raise AssertionError(f"{label}: expected {expected!r}, got {actual!r}")
|
||||
|
||||
def assert_present(text, needle, label):
|
||||
if needle not in text:
|
||||
raise AssertionError(f"{label}: missing {needle!r}")
|
||||
|
||||
assert_equal(manifest.get("name"), "superpowers", "plugin name")
|
||||
assert_equal(manifest.get("skills"), "./skills/", "skills path")
|
||||
assert_equal(
|
||||
manifest.get("sessionStart", {}).get("skill"),
|
||||
"using-superpowers",
|
||||
"sessionStart.skill",
|
||||
)
|
||||
|
||||
instructions = manifest.get("skillInstructions")
|
||||
if not isinstance(instructions, str) or not instructions.strip():
|
||||
raise AssertionError("skillInstructions must be a non-empty string")
|
||||
|
||||
for token in [
|
||||
"AskUserQuestion",
|
||||
"TodoList",
|
||||
"Agent",
|
||||
"Skill",
|
||||
"Read",
|
||||
"Write",
|
||||
"Edit",
|
||||
"Bash",
|
||||
"Grep",
|
||||
"Glob",
|
||||
"FetchURL",
|
||||
"WebSearch",
|
||||
]:
|
||||
assert_present(instructions, token, "skillInstructions")
|
||||
|
||||
version_config = json.loads(
|
||||
(manifest_path.parents[1] / ".version-bump.json").read_text(encoding="utf-8")
|
||||
)
|
||||
version_entries = version_config.get("files")
|
||||
if not isinstance(version_entries, list):
|
||||
raise AssertionError(".version-bump.json must contain files list")
|
||||
|
||||
if not any(
|
||||
entry.get("path") == ".kimi-plugin/plugin.json" and entry.get("field") == "version"
|
||||
for entry in version_entries
|
||||
if isinstance(entry, dict)
|
||||
):
|
||||
raise AssertionError(
|
||||
".version-bump.json must update .kimi-plugin/plugin.json version"
|
||||
)
|
||||
|
||||
unsupported_fields = [
|
||||
"tools",
|
||||
"commands",
|
||||
"hooks",
|
||||
"apps",
|
||||
"inject",
|
||||
"configFile",
|
||||
"config_file",
|
||||
"bootstrap",
|
||||
]
|
||||
present_unsupported = sorted(field for field in unsupported_fields if field in manifest)
|
||||
if present_unsupported:
|
||||
raise AssertionError(
|
||||
"unsupported Kimi runtime fields present: "
|
||||
+ ", ".join(present_unsupported)
|
||||
)
|
||||
|
||||
print("Kimi plugin manifest looks good")
|
||||
PY
|
||||
Reference in New Issue
Block a user