Merge pull request #2319 from obra/review-focus

Reviewer judges the spec as a vision document; plans list the five implied cases most likely to bite

README.md

@@ -284,7 +284,7 @@ turn loses the bootstrap — start a fresh session if skills stop triggering.
 
 3. **writing-plans** - Activates with approved design. Breaks work into bite-sized tasks (2-5 minutes each). Every task has exact file paths, complete code, verification steps.
 
-4. **subagent-driven-development** or **executing-plans** - Activates with plan. Dispatches fresh subagent per task with two-stage review (spec compliance, then code quality), or executes in batches with human checkpoints.
+4. **subagent-driven-development** or **executing-plans** - Activates with plan. Either dispatches a fresh subagent per task with a review after each (most thorough), or implements every task inline in the current session with one fresh review of the whole branch at the end (cheapest).
 
 5. **test-driven-development** - Activates during implementation. Enforces RED-GREEN-REFACTOR: write failing test, watch it fail, write minimal code, watch it pass, commit. Deletes code written before tests.
 
@@ -326,7 +326,7 @@ Superpowers is built by [Jesse Vincent](https://blog.fsck.com) and the rest of t
 **Collaboration** 
 - **brainstorming** - Socratic design refinement
 - **writing-plans** - Detailed implementation plans
-- **executing-plans** - Batch execution with checkpoints
+- **executing-plans** - Inline plan execution: one context, one final review
 - **dispatching-parallel-agents** - Concurrent subagent workflows
 - **requesting-code-review** - Pre-review checklist
 - **receiving-code-review** - Responding to feedback

skills/executing-plans/SKILL.md

@@ -1,64 +1,373 @@
 ---
 name: executing-plans
-description: Use when you have a written implementation plan to execute in a separate session with review checkpoints
+description: Use when executing an implementation plan in the current session as the implementer yourself — your human partner chose inline execution, or no subagent tool is available
 ---
 
 # Executing Plans
 
-## Overview
+Execute the plan yourself, task by task, in this session: no implementer
+subagent per task, no reviewer per task. One fresh-context review of the
+whole branch at the end.
 
-Load plan, review critically, execute all tasks, report when complete.
+**Why inline:** Subagent-driven development pays for a fresh implementer
+and a fresh reviewer on every task, each re-reading the codebase from zero.
+Inline execution pays for one context (yours) plus one reviewer at the end.
+What it gives up is a fresh context per task and a second pair of eyes per
+task. This skill keeps what those two things bought, by other means: the
+brief is the spec, the ledger is your memory, TDD is the per-task gate, and
+the final reviewer is the second pair of eyes.
 
-**Announce at start:** "I'm using the executing-plans skill to implement this plan."
+**Core principle:** The plan already did the thinking. Execute it exactly,
+prove each step with a test you watched fail and then pass, and leave a
+record that survives your own forgetting.
 
-**Note:** Tell your human partner that Superpowers works much better with access to subagents (Claude Code, Codex CLI, Codex App, Copilot CLI, and Gemini CLI all qualify; see the per-platform tool refs in `../using-superpowers/references/`). If subagents are available, use superpowers:subagent-driven-development instead of this skill.
+**Narration:** between tool calls, narrate at most one short line — the
+ledger and the tool results carry the record.
+
+**Continuous execution:** Do not pause to check in with your human partner
+between tasks. They chose inline execution to spend less, not to answer
+"should I continue?" after every task. Execute all tasks from the plan
+without stopping.
+
+**Rulings, not stalls.** Conflicts, ambiguities, plan defects — decide them.
+The spec is the binding authority, the plan is its argument, and your
+judgment settles what neither answers. Record every decision in the ledger
+as `Ruling: <what you decided> — <why> — <what it costs if wrong>`, and keep
+going. Deviating from the plan without a ledgered ruling is a decision made
+in secret.
+
+Four things stop you, and only these: an irreversible or destructive
+operation; a security-sensitive action; a side effect outside this worktree
+that norms say you ask about first (a merge, a push to a shared branch, a
+publish); and a plan so broken that every path forward is a guess. For
+those, stop and ask.
+
+## When to Use
+
+- You have a plan from superpowers:writing-plans and your human partner
+  chose inline execution at the handoff.
+- Your harness has no subagent tool (see the per-platform references in
+  `../using-superpowers/references/`). Never fabricate a dispatch; run
+  the plan here.
+- Tasks are mostly independent — the same precondition as
+  superpowers:subagent-driven-development.
+
+A fully specified plan makes inline execution transcription plus testing:
+it runs well on a mid-tier session model, and the one place the most
+capable model earns its cost is the final review, which this skill
+dispatches separately. Tell your human partner so when they choose inline.
+
+Prefer superpowers:subagent-driven-development when your human partner
+wants a review gate on every task, or when the plan is long enough that
+its later tasks would run on a compacted context. Inline execution over a
+long plan still works — the ledger is what makes it recoverable — but the
+last tasks get the least of you.
 
 ## The Process
 
-### Step 1: Load and Review Plan
-1. Ensure an isolated workspace: use superpowers:using-git-worktrees to create one or verify the existing one
-2. Read plan file
-3. Review critically - identify any questions or concerns about the plan
-4. If concerns: Raise them with your human partner before starting
-5. If no concerns: Create todos for the plan items and proceed
+```dot
+digraph process {
+    rankdir=TB;
+
+    subgraph cluster_per_task {
+        label="Per Task";
+        "task-start: brief + BASE; read the brief" [shape=box];
+        "Work the steps in order: TDD, run every verification, read every output" [shape=box];
+        "Step output matches plan's Expected?" [shape=diamond];
+        "Plan wrong? Rule and ledger. Code wrong? systematic-debugging" [shape=box];
+        "Commit as the plan's commit steps say" [shape=box];
+        "Completion contract met?" [shape=diamond];
+        "task-done: run tests, ledger the result; mark todo complete" [shape=box];
+    }
+
+    "Setup: worktree, workspace + ledger, read plan + spec, pre-flight scan" [shape=box];
+    "More tasks remain?" [shape=diamond];
+    "Final whole-branch review (fresh reviewer if you have one)" [shape=box];
+    "Re-grade, then: Critical/Important → ONE fix pass, each fix RED→GREEN + green suite; Minor → ledger" [shape=box];
+    "Final review clean: delete this plan's workspace" [shape=box];
+    "Use superpowers:finishing-a-development-branch" [shape=box style=filled fillcolor=lightgreen];
+
+    "Setup: worktree, workspace + ledger, read plan + spec, pre-flight scan" -> "task-start: brief + BASE; read the brief";
+    "task-start: brief + BASE; read the brief" -> "Work the steps in order: TDD, run every verification, read every output";
+    "Work the steps in order: TDD, run every verification, read every output" -> "Step output matches plan's Expected?";
+    "Step output matches plan's Expected?" -> "Plan wrong? Rule and ledger. Code wrong? systematic-debugging" [label="no"];
+    "Plan wrong? Rule and ledger. Code wrong? systematic-debugging" -> "Work the steps in order: TDD, run every verification, read every output";
+    "Step output matches plan's Expected?" -> "Commit as the plan's commit steps say" [label="yes, last step"];
+    "Commit as the plan's commit steps say" -> "Completion contract met?";
+    "Completion contract met?" -> "Work the steps in order: TDD, run every verification, read every output" [label="no - finish the task"];
+    "Completion contract met?" -> "task-done: run tests, ledger the result; mark todo complete" [label="yes"];
+    "task-done: run tests, ledger the result; mark todo complete" -> "More tasks remain?";
+    "More tasks remain?" -> "task-start: brief + BASE; read the brief" [label="yes"];
+    "More tasks remain?" -> "Final whole-branch review (fresh reviewer if you have one)" [label="no"];
+    "Final whole-branch review (fresh reviewer if you have one)" -> "Re-grade, then: Critical/Important → ONE fix pass, each fix RED→GREEN + green suite; Minor → ledger";
+    "Re-grade, then: Critical/Important → ONE fix pass, each fix RED→GREEN + green suite; Minor → ledger" -> "Final review clean: delete this plan's workspace";
+    "Final review clean: delete this plan's workspace" -> "Use superpowers:finishing-a-development-branch";
+}
+```
+
+## Setup
+
+Ensure the work happens in an isolated workspace: use
+superpowers:using-git-worktrees to create one or verify the existing one.
+Never start implementation on a main/master branch without your human
+partner's explicit consent.
+
+Conversation memory does not survive compaction. An inline executor that
+loses its place re-implements tasks whose commits already exist — the same
+failure as a controller re-dispatching them, paid for in your own context.
+Track progress in a ledger file, not only in todos. Harness todos are a
+live view; the ledger is the record.
+
+The workspace and ledger are shared with superpowers:subagent-driven-development
+— same directory, same format — so a plan can change executors mid-flight
+and the new one resumes from the same ledger.
+
+- Each plan owns a workspace: at skill start, run
+  `../subagent-driven-development/scripts/sdd-workspace PLAN_FILE` — it
+  prints the plan's git-ignored directory
+  (`<repo-root>/.superpowers/sdd/<plan-basename>/`), home to every
+  artifact for THIS plan: ledger, briefs, review packages. Another plan's
+  directory is never yours to read or write.
+- Check for this plan's ledger at `<workspace>/progress.md`. If its first
+  line names your plan file, tasks with a `Task <N>: complete` line are
+  DONE — do not redo them; resume at the first task without one. Their
+  commits exist in git even when your context no longer remembers making
+  them: after compaction, trust the ledger and `git log` over your own
+  recollection. A ledger whose first line names a different plan file is
+  another plan's progress: leave it and start your own, fresh.
+- Create the ledger with its identity as the first line:
+  `# SDD ledger — plan: <plan file path>`.
+- `git clean -fdx` will destroy the workspace (it's git-ignored scratch);
+  if that happens, recover from `git log`.
+
+Read the plan once, note its context and Global Constraints, and create a
+todo per task. If the plan names a Spec, read that too: the spec is the
+authority the plan argues from, and conflicts inside the plan resolve
+against it. A plan with no reachable spec gets a ledger note saying so —
+rulings made without one are provisional.
+
+**REQUIRED SUB-SKILL:** load superpowers:test-driven-development now,
+before Task 1. It governs every step of every task below; a plan whose
+steps already say "write the failing test first" does not exempt you
+from reading it.
+
+Before Task 1, scan the plan for conflicts between tasks. The plan's
+Interfaces blocks tell you where to look: for every task that consumes
+what an earlier task produces, one ledger row — the two tasks, what one
+produces against what the other consumes, and what you found. Tasks that
+share nothing get no row; a plan whose tasks share nothing gets the single
+line `Pre-flight: no shared interfaces`. Rule on each conflict a row
+surfaces with the spec as the binding authority, record the ruling beside
+its row, and start Task 1. Each task's own text is checked when you read
+its brief, not here.
+
+## The Task Loop
+
+Everything you print, and every tool result, stays resident in your
+context for the rest of the session. Redirect long test output to a file
+in the workspace and read its tail; read a brief, not the whole plan.
+
+### 1. Take the task
+
+- Run this skill's `scripts/task-start PLAN_FILE N`. It prints the brief
+  path and BASE (the commit the task's range is cut from) in one call.
+  Read the brief for every task, including ones you remember from setup:
+  what you remember is a summary, the brief has the exact values,
+  signatures, and test cases.
+- Mark the task's todo in_progress.
+
+Every tool call is a turn that re-reads your whole context. Bookkeeping
+rides along with work — a ledger append in the same call as the commit,
+never in a call of its own.
+
+### 2. Work the steps
+
+The plan's steps are already in RED-GREEN order; follow them in that
+order under superpowers:test-driven-development, loaded at setup. A test
+step's code is written first and run first. Watching it fail is a step,
+not a formality — a test that passes before the implementation exists is
+a finding about the test.
+
+Every step that runs a command has an `Expected:` line. Run the command,
+read its output, and compare. Three outcomes:
+
+- **Matches.** Next step.
+- **The code is wrong.** Use superpowers:systematic-debugging. Find the
+  cause; never patch the symptom to make the step's output match.
+- **The plan is wrong** — a step contradicts the spec, an interface from an
+  earlier task doesn't match what this task consumes, a command that
+  cannot work. Rule on the smallest change that satisfies the spec, ledger
+  it as `Task <N>: Ruling: <finding> — <what you decided and why>`, and
+  continue. The ruling is carried, not remembered: later tasks that touch
+  the same interface read it from the ledger.
+
+Commit as the plan's commit steps say. A task that spans several commits
+is fine; BASE is what the review range is cut from, never `HEAD~1`.
+
+### 3. The completion contract
+
+Before a task's ledger line, all of the following are true, with evidence
+in this session — not inferred from the diff looking right:
+
+- Every test the brief names exists and ran in this task, and you read
+  the output.
+- The final test run for the task passed — `task-done` is that run, and
+  it writes the command and result into the ledger line.
+- Every `Expected:` line in the brief was compared against real output.
+- Every deviation from the brief has a `Ruling:` line in the ledger.
+
+**REQUIRED SUB-SKILL:** superpowers:verification-before-completion governs
+the claim. If any item is missing, the task is not complete: finish it.
+
+### 4. Complete the task
+
+Run this skill's `scripts/task-done PLAN_FILE N BASE -- <test command>`
+with the test command the brief names for the whole task. It runs the
+tests, keeps the full output in the workspace, prints the tail, and — only
+if they pass — appends the completion line to the ledger:
+
+`Task <N>: complete (commits <base7>..<head7>, tests: <command> → <result>)`
+
+A failing run records nothing; the task is not complete. When it records,
+mark the todo complete and take the next task.
+
+## Final Review
+
+Run `../subagent-driven-development/scripts/review-package PLAN_FILE MERGE_BASE HEAD`
+(MERGE_BASE = the commit the branch started from, e.g.
+`git merge-base main HEAD`) and review from the file it prints.
+
+**With a subagent tool:** dispatch the reviewer on the most capable
+available model — the whole-branch review is a judgment task — using
+superpowers:requesting-code-review's
+[code-reviewer.md](../requesting-code-review/code-reviewer.md), with the
+package path, the plan and spec paths, the plan's Review Focus section
+verbatim if it has one (the input classes and failure modes the plan's
+tests do not exercise — the reviewer checks each deliberately), and a
+pointer to the ledger's `Ruling:` lines so it can weigh the calls you
+made. Specify the model
+explicitly; an omitted model inherits the session's, which may not be the
+most capable. This is the one fresh context the whole run buys. Do not
+skip it, and do not replace it with your own read of the diff.
+
+**Without a subagent tool:** read code-reviewer.md and perform that review
+yourself against the package, as a separate pass after the last task's
+ledger line. Write `Final review: self-review (no subagent tool)` to the
+ledger, and say so in your final message: a self-review by the author is
+weaker than a fresh reviewer, and your human partner decides whether that
+is enough before merge.
+
+Sort the findings before you act on any of them. The reviewer's severity
+labels are advice; the gate is yours. Its "Declined to judge" list is
+yours too: every line there is a ruling you make and ledger, exactly like
+a plan conflict — `Final: Ruling: <behavior the reviewer set aside> —
+<what a reasonable person using this software gets, and why that stands
+or why it is now a finding> — <cost if wrong>`. Re-grade first, by effect: the
+spec is a vision document, and a finding's grade is what a reasonable
+person using this software gets if it ships, not whether the spec names
+the input that triggers it — a reviewer who set a finding at Minor
+because the spec was silent has graded the spec, not the effect. Then:
+
+- **Critical and Important** enter the fix pass.
+- **Minor** goes to the ledger as `Final: minor (deferred): <one-liner>`
+  and to your final message under "Deferred minors". Minors never enter
+  the fix pass, and never become rulings — a ruling is a decision about a
+  conflict, not a note that you declined a polish suggestion.
+
+Fix the Critical and Important findings yourself — you are the
+implementer here — in ONE pass. Each fix is verified by TDD, not by a
+second reviewer: write the test that reproduces the finding, watch it
+fail, make it pass, then run the whole suite. Record each in the ledger as
+`Final: fixed <finding> — <test name> RED→GREEN, suite <N>/<N>`. A fix
+without a test that failed first is not verified; a suite that is not
+green after the pass means the pass is not over. Do not dispatch a
+re-review: it would re-read a diff whose covering tests already answer
+"addressed" and whose suite run already answers "broke nothing".
+
+A finding you decide not to fix is a ruling — `Final: Ruling: <finding> —
+<why the code stands> — <cost if wrong>` — and reaches your human partner
+in the rulings list. There is no second fix pass.
+
+## Finish
+
+Before you delete anything, collect every ledger line containing
+`Ruling:` into your final message under "Rulings I made", in the order you
+made them, each with what it costs if wrong, and every `minor (deferred)`
+line under "Deferred minors". Both lists are exhaustive. Your final
+message is the only place the decisions you took on your human partner's
+behalf — and the findings you chose not to act on — reach them.
+
+When the final review is clean and its fixes are committed, delete this
+plan's workspace directory — the git history is the record now. Sibling
+directories belong to other plans; leave them alone.
+
+Use superpowers:finishing-a-development-branch.
+
+## Common Rationalizations
+
+| Excuse | Reality |
+|--------|---------|
+| "I remember what Task N says" | You remember a summary. The brief has the exact values. Read it. |
+| "The plan's code is right, skip watching the test fail" | A test you never saw fail proves nothing. It is one step. Run it. |
+| "I'll run the full suite at the end instead of per step" | Per-step runs are how you learn which step broke it. The end-of-task run is the contract, not a substitute. |
+| "The plan is wrong here, I'll just do the right thing" | Do the right thing and ledger the ruling. Unledgered deviation is a decision made in secret. |
+| "I'll write the ledger lines after a few tasks" | Compaction does not wait for a convenient moment. One line per task, in the same message as the commit. |
+| "Let me check in before the next task" | They chose inline to spend less. Progress prompts spend their time instead. Only the four stops stop you. |
+| "I read my own diff carefully; the final reviewer is redundant" | Same author, same blind spots. The reviewer is the only fresh context this run buys. |
+| "Tests should pass, the change was trivial" | "Should" is not evidence. The contract requires the command and its output. |
+| "Subagents are slow and expensive, I'll skip the final review too" | Inline already removed the per-task reviewers. One review of the whole branch is the floor, not the ceiling. |
+| "The reviewer said Minor, so it's Minor" | The label graded the spec's silence. Grade what the person gets. Re-grade, then gate. |
+| "The fix is obvious, no need for a failing test first" | The failing test is the only proof the finding was real and is now gone. Without it you have a diff and a hope. |
+| "I'll fix the minors too while I'm in there" | Every minor you fix is a test, a fix, and a suite run your partner did not ask for. Ledger them; your partner decides. |
+
+## Example Workflow
+
+```
+You: I'm using the executing-plans skill to implement this plan inline.
 
-### Step 2: Execute Tasks
+[Setup: worktree verified]
+[Read plan once: docs/superpowers/plans/feature-plan.md; spec read]
+[Resolve workspace: sdd-workspace docs/superpowers/plans/feature-plan.md — no ledger inside, fresh start]
+[Pre-flight scan: 2 shared-interface rows, 4 self-consistency rows, clean; written to ledger]
+[Create todos for all tasks]
 
-For each task:
-1. Mark as in_progress
-2. Follow each step exactly (plan has bite-sized steps)
-3. Run verifications as specified
-4. Mark as completed
+Task 1: Hook installation script
 
-### Step 3: Complete Development
+[task-start plan 1 → brief read; BASE a1b2c3d]
+[Step 1: write failing test — written]
+[Step 2: run it — FAIL: install_hook not defined. Matches Expected.]
+[Step 3: implement — written]
+[Step 4: run it — PASS 1/1. Matches Expected.]
+[Step 5: commit — d4e5f6a]
+[Contract: tests ran, output read, no deviations]
+[task-done plan 1 a1b2c3d -- npm test -- hooks → ledger: Task 1: complete (commits a1b2c3d..d4e5f6a, tests: npm test -- hooks → 1/1 pass)]
 
-After all tasks complete and verified:
-- Announce: "I'm using the finishing-a-development-branch skill to complete this work."
-- **REQUIRED SUB-SKILL:** Use superpowers:finishing-a-development-branch
-- Follow that skill to verify tests, present options, execute choice
+Task 2: Recovery modes
 
-## When to Stop and Ask for Help
+[task-start plan 2 → brief read; BASE d4e5f6a]
+[Step 2: run failing test — FAIL, but on an import error: Task 1 exported
+ installHook, brief consumes install_hook]
+[Ruling: brief's consumer name is a typo against Task 1's Produces block;
+ use installHook — Ledger: Task 2: Ruling: install_hook → installHook — matches Task 1 Produces — cost if wrong: one rename]
+[Steps 2-5 as planned; commit b7c8d9e]
+[task-done plan 2 d4e5f6a -- npm test -- recovery → ledger: Task 2: complete (commits d4e5f6a..b7c8d9e, tests: npm test -- recovery → 8/8 pass)]
 
-**STOP executing immediately when:**
-- Hit a blocker (missing dependency, test fails, instruction unclear)
-- Plan has critical gaps preventing starting
-- You don't understand an instruction
-- Verification fails repeatedly
+...
 
-**Ask for clarification rather than guessing.**
+[After all tasks: review-package plan MERGE_BASE HEAD; dispatch code-reviewer, most capable model]
+Reviewer: One Important finding — progress reporting interval hardcoded. Two Minor.
+[Re-grade: Important stands; minors → ledger as deferred]
+[Fix pass: test_progress_interval_configurable RED → extract PROGRESS_INTERVAL → GREEN; suite 12/12; commit]
+[Ledger: Final: fixed hardcoded interval — test_progress_interval_configurable RED→GREEN, suite 12/12]
 
-## When to Revisit Earlier Steps
+Rulings I made:
+- Task 2: install_hook → installHook (brief typo; cost if wrong: one rename)
 
-**Return to Review (Step 1) when:**
-- Partner updates the plan based on your feedback
-- Fundamental approach needs rethinking
+Deferred minors:
+- README lacks a usage example
+- recovery.js could split verify/repair into two files
 
-**Don't force through blockers** - stop and ask.
+[Delete this plan's workspace — the record now lives in git]
 
-## Remember
-- Review plan critically first
-- Follow plan steps exactly
-- Don't skip verifications
-- Reference skills when plan says to
-- Stop when blocked, don't guess
-- Never start implementation on main/master branch without explicit user consent
+Using superpowers:finishing-a-development-branch.
+```

skills/executing-plans/scripts/task-done

@@ -0,0 +1,52 @@
+#!/usr/bin/env bash
+# Close one task of an inline plan execution in a single call: run the task's
+# test command, keep its full output in the workspace, print the tail, and —
+# only if the command succeeded — append the completion line to the ledger.
+# A failing command records nothing: the task is not complete.
+#
+# Usage: task-done PLAN_FILE TASK_NUMBER BASE -- TEST_COMMAND [ARGS...]
+#   BASE is the SHA task-start printed; the completion line records BASE..HEAD.
+# Exit: the test command's exit status.
+set -euo pipefail
+
+if [ $# -lt 5 ] || [ "$4" != "--" ]; then
+  echo "usage: task-done PLAN_FILE TASK_NUMBER BASE -- TEST_COMMAND [ARGS...]" >&2
+  exit 2
+fi
+
+plan=$1
+n=$2
+base=$3
+shift 4
+sdd="$(cd "$(dirname "$0")/../../subagent-driven-development/scripts" && pwd)"
+
+git rev-parse --verify --quiet "$base" >/dev/null || { echo "bad BASE: $base" >&2; exit 2; }
+
+dir=$("$sdd/sdd-workspace" "$plan")
+log="$dir/task-${n}-tests.log"
+ledger="$dir/progress.md"
+
+# Render the command the way a person would type it, for the ledger line.
+cmd=""
+for a in "$@"; do
+  case "$a" in
+    *[[:space:]\"\;\|\&]*) cmd="$cmd '$a'" ;;
+    *) cmd="$cmd $a" ;;
+  esac
+done
+cmd=${cmd# }
+
+rc=0
+"$@" > "$log" 2>&1 || rc=$?
+
+tail -n 5 "$log"
+if [ "$rc" -ne 0 ]; then
+  echo "task-done: test command exited $rc; Task $n NOT recorded (full output: $log)" >&2
+  exit "$rc"
+fi
+
+last=$(grep -v '^[[:space:]]*$' "$log" | tail -n 1)
+[ -f "$ledger" ] || printf '# SDD ledger — plan: %s\n' "$plan" > "$ledger"
+line="Task $n: complete (commits $(git rev-parse --short=7 "$base")..$(git rev-parse --short=7 HEAD), tests: $cmd → $last)"
+printf '%s\n' "$line" >> "$ledger"
+echo "ledger: $line"

skills/executing-plans/scripts/task-start

@@ -0,0 +1,28 @@
+#!/usr/bin/env bash
+# Begin one task of an inline plan execution in a single call: extract the
+# task's brief (via subagent-driven-development's task-brief, so both skills
+# share one workspace) and record BASE, the commit the task's review range is
+# cut from. One tool call instead of two, because every call in an inline
+# session is a turn that re-reads the whole context.
+#
+# Usage: task-start PLAN_FILE TASK_NUMBER
+# Prints:
+#   brief: <path to the task's brief file>
+#   base:  <full SHA of HEAD>
+set -euo pipefail
+
+if [ $# -ne 2 ]; then
+  echo "usage: task-start PLAN_FILE TASK_NUMBER" >&2
+  exit 2
+fi
+
+plan=$1
+n=$2
+sdd="$(cd "$(dirname "$0")/../../subagent-driven-development/scripts" && pwd)"
+
+out=$("$sdd/task-brief" "$plan" "$n")
+brief=$(printf '%s\n' "$out" | sed -n 's/^wrote \(.*\): [0-9][0-9]* lines$/\1/p')
+[ -n "$brief" ] || { echo "task-brief did not report a path: $out" >&2; exit 1; }
+
+echo "brief: $brief"
+echo "base: $(git rev-parse HEAD)"

skills/requesting-code-review/code-reviewer.md

@@ -30,6 +30,23 @@ Subagent (general-purpose):
     git diff [BASE_SHA]..[HEAD_SHA]
     ```
 
+    ## The spec is a vision document
+
+    The spec says what the software must do. It does not enumerate every
+    input, environment, or condition the software will meet. For behavior
+    the spec is silent on, judge by what a reasonable person using this
+    software would expect: a reasonable person's expectation is a
+    requirement, and a spec's silence is not permission. Grade such
+    findings by their effect on that person, not by whether the spec
+    mentions the trigger.
+
+    ## Declined to judge
+
+    Before your verdict, list every behavior you considered and set aside
+    as outside the plan or spec, one line each, with the reason. The
+    executor rules on each line; nothing you set aside is dropped
+    silently. An empty list means you set nothing aside.
+
     ## 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.

skills/subagent-driven-development/SKILL.md

@@ -36,25 +36,25 @@ stop and ask.
 digraph when_to_use {
     "Have implementation plan?" [shape=diamond];
     "Tasks mostly independent?" [shape=diamond];
-    "Stay in this session?" [shape=diamond];
+    "Partner chose inline, or no subagent tool?" [shape=diamond];
     "subagent-driven-development" [shape=box];
     "executing-plans" [shape=box];
     "Manual execution or brainstorm first" [shape=box];
 
     "Have implementation plan?" -> "Tasks mostly independent?" [label="yes"];
     "Have implementation plan?" -> "Manual execution or brainstorm first" [label="no"];
-    "Tasks mostly independent?" -> "Stay in this session?" [label="yes"];
+    "Tasks mostly independent?" -> "Partner chose inline, or no subagent tool?" [label="yes"];
     "Tasks mostly independent?" -> "Manual execution or brainstorm first" [label="no - tightly coupled"];
-    "Stay in this session?" -> "subagent-driven-development" [label="yes"];
-    "Stay in this session?" -> "executing-plans" [label="no - parallel session"];
+    "Partner chose inline, or no subagent tool?" -> "executing-plans" [label="yes"];
+    "Partner chose inline, or no subagent tool?" -> "subagent-driven-development" [label="no"];
 }
 ```
 
-**vs. Executing Plans (parallel session):**
-- Same session (no context switch)
-- Fresh subagent per task (no context pollution)
-- Review after each task (spec compliance + code quality), broad review at the end
-- Faster iteration (no human-in-loop between tasks)
+**vs. Executing Plans (inline):**
+- Fresh subagent per task (no context pollution) instead of one context doing every task
+- Review after each task (spec compliance + code quality) instead of only at the end
+- Costs a fresh context per task and per review; inline costs one context plus one final reviewer
+- Both run in this session, share the same plan workspace and ledger, and never pause between tasks
 
 ## The Process
 

skills/writing-plans/SKILL.md

@@ -76,6 +76,18 @@ naming and copy rules, platform requirements — one line each, with exact
 values copied verbatim from the spec. Every task's requirements implicitly
 include this section.]
 
+## Review Focus
+
+[The five input classes or failure modes the spec implies but no task's
+tests exercise that are most likely to bite a person using this software
+— one line each, naming the input or condition and the behavior a
+reasonable person would expect, most likely first. The spec is a vision
+document: it says what the software must do, not everything it will
+meet, and its silence on an input is not permission for that input to
+break the program. Write the list here, once, with the spec in front of
+you. Then, for each line, add the test that pins it to the task that
+owns the code, in that task's own step style.]
+
 ---
 ```
 
@@ -148,6 +160,8 @@ After writing the complete plan, look at the spec with fresh eyes and check the
 
 **3. Type consistency:** Do the types, method signatures, and property names you used in later tasks match what you defined in earlier tasks? A function called `clearLayers()` in Task 3 but `clearFullLayers()` in Task 7 is a bug.
 
+**4. Review Focus:** For each input class or failure mode the spec implies, is there a task whose tests exercise it? The five uncovered ones most likely to bite a person go in the Review Focus section, and each line there gets its test added to the owning task. An empty section means you checked and found none, not that you skipped the check.
+
 If you find issues, fix them inline. No need to re-review — just fix and move on. If you find a spec requirement with no task, add the task.
 
 ## Execution Handoff
@@ -160,22 +174,19 @@ them to review the plan and choose an execution method before implementation.
 
 **When no execution method has already been supplied:**
 
-**"Plan complete and saved to `docs/superpowers/plans/<filename>.md`. Please review the plan. Two execution options:**
-
-**1. Subagent-Driven (recommended)** - I dispatch a fresh subagent per task, review between tasks, fast iteration
+**"Plan complete and saved to `docs/superpowers/plans/<filename>.md`. Please review the plan. Which execution approach would you prefer?**
 
-**2. Inline Execution** - Execute tasks in this session using executing-plans, batch execution with checkpoints
+- **Subagent-driven** - A fresh subagent implements each task and a fresh reviewer checks it before the next one starts, then a whole-branch review at the end. Most thorough; costs a fresh context per task and per review.
+- **Native** - I implement every task myself in this session, the way this harness runs work, then one fresh reviewer on the most capable model checks the whole branch. Cheapest and fastest; no independent review until the end. Runs well with a mid-tier session model, since the plan carries the design.
 
-**Does the plan capture what you want, and which approach should we use?"**
+**For this plan I recommend <one of the two>, because <one sentence from the plan: how much the tasks depend on each other's interfaces, how many there are, what a shipped mistake would cost>. Does the plan capture what you want, and which approach should we use?"**
 
 **When an execution method has already been supplied:**
 
 **"Plan complete and saved to `docs/superpowers/plans/<filename>.md`. Please review the plan. Does it capture what you want?"**
 
-**If Subagent-Driven chosen:**
+**If Subagent-driven chosen:**
 - **REQUIRED SUB-SKILL:** Use superpowers:subagent-driven-development
-- Fresh subagent per task + two-stage review
 
-**If Inline Execution chosen:**
+**If Native chosen:**
 - **REQUIRED SUB-SKILL:** Use superpowers:executing-plans
-- Batch execution with checkpoints for review

tests/claude-code/run-skill-tests.sh

@@ -76,6 +76,7 @@ done
 tests=(
     "test-worktree-path-policy.sh"
     "test-sdd-workspace.sh"
+    "test-executing-plans-scripts.sh"
     "test-subagent-driven-development.sh"
 )
 

tests/claude-code/test-executing-plans-scripts.sh

@@ -0,0 +1,139 @@
+#!/usr/bin/env bash
+# Tests for executing-plans' bookkeeping helpers: scripts/task-start extracts
+# the brief and records BASE in one call; scripts/task-done runs the task's
+# test command, records the result in the ledger, and refuses to record a
+# failing task.
+set -euo pipefail
+
+SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
+REPO_ROOT="$(cd "$SCRIPT_DIR/../.." && pwd)"
+EP_SCRIPTS="$REPO_ROOT/skills/executing-plans/scripts"
+
+FAILURES=0
+TEST_ROOT=""
+
+pass() { echo "  [PASS] $1"; }
+fail() {
+    echo "  [FAIL] $1"
+    FAILURES=$((FAILURES + 1))
+}
+
+cleanup() {
+    if [[ -n "$TEST_ROOT" && -d "$TEST_ROOT" ]]; then
+        rm -rf "$TEST_ROOT"
+    fi
+}
+
+main() {
+    echo "=== Test: executing-plans scripts ==="
+
+    TEST_ROOT="$(mktemp -d)"
+    trap cleanup EXIT
+
+    git init -q -b main "$TEST_ROOT/repo"
+    local repo
+    repo="$(cd "$TEST_ROOT/repo" && git rev-parse --show-toplevel)"
+    local git_id=(-c user.email=t@example.com -c user.name=t -c commit.gpgsign=false)
+
+    cat > "$repo/plan.md" <<'PLAN'
+# Plan
+
+## Task 1: First thing
+
+Do the first thing.
+
+## Task 2: Second thing
+
+Do the second thing.
+PLAN
+    ( cd "$repo" && git add plan.md && git "${git_id[@]}" commit -qm fixture )
+    local base
+    base="$(cd "$repo" && git rev-parse HEAD)"
+
+    # --- task-start: argument validation ---
+    local rc=0
+    (cd "$repo" && "$EP_SCRIPTS/task-start" plan.md >/dev/null 2>&1) || rc=$?
+    if [[ "$rc" -eq 2 ]]; then
+        pass "task-start without a task number errors with exit 2"
+    else
+        fail "task-start without a task number errors with exit 2 (got $rc)"
+    fi
+
+    # --- task-start: brief path + BASE in one call ---
+    local out
+    out="$(cd "$repo" && "$EP_SCRIPTS/task-start" plan.md 1)"
+    if [[ "$out" == *"brief: $repo/.superpowers/sdd/plan/task-1-brief.md"* ]]; then
+        pass "task-start prints the brief path under the plan's workspace"
+    else
+        fail "task-start prints the brief path under the plan's workspace"
+        echo "    got: $out"
+    fi
+    if [[ "$out" == *"base: $base"* ]]; then
+        pass "task-start prints BASE as the current HEAD"
+    else
+        fail "task-start prints BASE as the current HEAD"
+        echo "    got: $out"
+    fi
+    if [[ -s "$repo/.superpowers/sdd/plan/task-1-brief.md" ]]; then
+        pass "task-start writes the brief file"
+    else
+        fail "task-start writes the brief file"
+    fi
+
+    # --- task-done: records a passing task ---
+    ( cd "$repo" && echo x > work.txt && git add work.txt && git "${git_id[@]}" commit -qm "task 1" )
+    local head
+    head="$(cd "$repo" && git rev-parse HEAD)"
+    out="$(cd "$repo" && "$EP_SCRIPTS/task-done" plan.md 1 "$base" -- sh -c 'echo "Ran 3 tests"; echo OK')"
+    rc=$?
+    local ledger="$repo/.superpowers/sdd/plan/progress.md"
+    local expected="Task 1: complete (commits ${base:0:7}..${head:0:7}, tests: sh -c 'echo \"Ran 3 tests\"; echo OK' → OK)"
+    if [[ -f "$ledger" ]] && grep -qF "$expected" "$ledger"; then
+        pass "task-done appends the completion line with commit range and test result"
+    else
+        fail "task-done appends the completion line with commit range and test result"
+        echo "    expected: $expected"
+        echo "    ledger:"; sed 's/^/      /' "$ledger" 2>/dev/null || echo "      (missing)"
+    fi
+    if [[ "$out" == *"OK"* ]]; then
+        pass "task-done prints the tail of the test output"
+    else
+        fail "task-done prints the tail of the test output"
+        echo "    got: $out"
+    fi
+    if [[ -s "$repo/.superpowers/sdd/plan/task-1-tests.log" ]]; then
+        pass "task-done keeps the full test output in the workspace"
+    else
+        fail "task-done keeps the full test output in the workspace"
+    fi
+
+    # --- task-done: refuses to record a failing task ---
+    rc=0
+    out="$(cd "$repo" && "$EP_SCRIPTS/task-done" plan.md 2 "$head" -- sh -c 'echo "FAILED (errors=1)"; exit 1' 2>&1)" || rc=$?
+    if [[ "$rc" -ne 0 ]]; then
+        pass "task-done exits non-zero when the test command fails"
+    else
+        fail "task-done exits non-zero when the test command fails"
+    fi
+    if ! grep -q "Task 2: complete" "$ledger"; then
+        pass "task-done does not record a failing task as complete"
+    else
+        fail "task-done does not record a failing task as complete"
+    fi
+    if [[ "$out" == *"FAILED"* ]]; then
+        pass "task-done shows the failing output"
+    else
+        fail "task-done shows the failing output"
+        echo "    got: $out"
+    fi
+
+    echo
+    if [[ "$FAILURES" -eq 0 ]]; then
+        echo "PASS"
+    else
+        echo "FAIL ($FAILURES)"
+        exit 1
+    fi
+}
+
+main "$@"