diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 3b7a172..235cf4d 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "freethinking", - "version": "0.7.0", + "version": "0.8.0", "description": "Shared engineering skills: standard workflows and prompts for day-to-day development.", "author": { "name": "Developer Handbook contributors" diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 8c408f0..db107be 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -59,21 +59,22 @@ ticket. The [pull request template](pull_request_template.md) is the one place that says how the issue is linked and when to write `Closes #`. A new skill, agent, rule or hook is also added to the `component` dropdown of -the "Report a misbehaving skill or agent", "Propose a new or changed skill, -agent or rule" and "Propose an eval case" forms. +every issue form. ## Issues -New issues start from one of three forms: +New issues start from one of four forms: - **Report a misbehaving skill or agent**, for something the plugin did wrong. - **Propose a new or changed skill, agent or rule**, for something it should do. - **Propose an eval case**, for a behaviour an eval should guard. +- **Propose a spike**, for a question that has to be answered before work can + be designed. Submitting a form labels the issue `triage` and sets its Issue_Type: `Bug`, -`Feature` and `Task` respectively. Blank issues are off. The new-issue page -also links questions to Discussions Q&A, ideas to Discussions Ideas, and +`Feature`, `Task` and `Spike` respectively. Blank issues are off. The new-issue +page also links questions to Discussions Q&A, ideas to Discussions Ideas, and vulnerabilities to the security policy page. At least weekly, a maintainer triages: @@ -94,8 +95,8 @@ really a question is converted to a discussion at triage. ## Labels and milestones -Every issue has one Issue_Type, `Bug`, `Feature` or `Task`, which says what -sort of thing it is. Labels are lowercase, and there are nine: +Every issue has one Issue_Type, `Bug`, `Feature`, `Task` or `Spike`, which +says what sort of thing it is. Labels are lowercase, and there are nine: - `priority: high`, `priority: medium` and `priority: low` say how soon the issue should be done. diff --git a/.github/ISSUE_TEMPLATE/spike.yml b/.github/ISSUE_TEMPLATE/spike.yml new file mode 100644 index 0000000..c3c114a --- /dev/null +++ b/.github/ISSUE_TEMPLATE/spike.yml @@ -0,0 +1,73 @@ +name: Propose a spike +description: Propose a time-boxed investigation that answers a question before a design is committed to. +labels: ["triage"] +type: Spike +body: + - type: markdown + attributes: + value: Before you file, read the [Issues section of the contributing guide](https://github.com/freethinkingit/dev_tools/blob/main/.github/CONTRIBUTING.md#issues). + - type: checkboxes + id: preflight + attributes: + label: Before you file + options: + - label: I searched the existing issues. + required: true + - label: The question cannot be answered by reading the code or the docs. + required: true + - type: dropdown + id: component + attributes: + label: Component + description: The component the question is about. Leave it empty when it spans several. + options: + - start-task (skill) + - implement-spec (skill) + - pr (skill) + - bugfix (skill) + - task-executor (agent) + - wave-reviewer (agent) + - code-changes (rule) + - collaboration (rule) + - comments (rule) + - workflow (rule) + - SessionStart hook + - SubagentStart hook + - Something else + validations: + required: false + - type: textarea + id: question + attributes: + label: Question + description: The question the spike answers. + validations: + required: true + - type: textarea + id: why + attributes: + label: Why it matters + description: What decision waits on the answer, and what it costs to guess. + validations: + required: true + - type: textarea + id: known + attributes: + label: What is known + description: The options already on the table, and what rules each in or out so far. + validations: + required: true + - type: input + id: timebox + attributes: + label: Time box + description: How long before the spike reports back whatever it found. + validations: + required: true + - type: textarea + id: outcome + attributes: + label: Outcome + description: "What the spike produces: a decision recorded on the issue, a proposal issue, a spec, or a decision not to proceed." + validations: + required: true diff --git a/README.md b/README.md index 2738dca..32f94e6 100644 --- a/README.md +++ b/README.md @@ -40,7 +40,7 @@ claude plugin update freethinking@freethinkingit | Skill | Run it with | What it does | | ----- | ----------- | ------------ | | `start-task` | `/freethinking:start-task `, or ask Claude to change something, or to start or spec a task | The front door for any change: reads the code the request touches, then sends one contained change to the `task-executor` agent, a defect to `bugfix` and everything else to a spec, asking you before taking either of the first two. On the spec route, writes `requirements.md`, `design.md` and `tasks.md` in `.claude/specs//` before any code, following the team template, with you approving each document before the next. | -| `implement-spec` | `/freethinking:implement-spec [task] [task\|wave]`, or ask Claude to implement, build or continue a spec | Builds a task from its approved spec, wave by wave from `tasks.md`, each leaf task in its own sub-agent, ticking a task only once its tests pass, running the full suite at the end of every wave, and handing back to you after each task or each wave, whichever you chose. Pauses with a proposal when the design cannot be followed, and hands off to `pr` when the spec is done. | +| `implement-spec` | `/freethinking:implement-spec [task] [task\|wave\|spec]`, or ask Claude to implement, build or continue a spec | Builds a task from its approved spec, wave by wave from `tasks.md`, each leaf task in its own sub-agent, ticking a task only once its tests pass, running the full suite at the end of every wave, and handing back to you after each task, after each wave, or only once every wave is done (with a short report after each wave and a final review of the whole run), whichever you chose. Pauses with a proposal when the design cannot be followed, and hands off to `pr` when the spec is done. | | `bugfix` | `/freethinking:bugfix `, or ask Claude to fix a bug | Reads the ticket, reproduces the bug before touching code, confirms the root cause with you, fixes the cause with a regression test, verifies the fix, then hands off to `pr`. | | `pr` | `/freethinking:pr [ticket] [base-branch]`, or ask Claude to open a PR | Takes the ticket as its first argument and reads it, offers to create the branch and commit if needed, walks the branch's changes with you one at a time with a short set of review prompts, drafts the title and description from your answers, and opens the PR with `gh` once you approve. Needs the `gh` CLI signed in. | @@ -91,7 +91,7 @@ What the prompt carries: | Agent | What it does | What it returns | | ----- | ------------ | --------------- | | `task-executor` | Builds one well-specified change as the smallest change in the project's style, proves it, and deletes its scratch work. Stops and asks instead of guessing when the task is unclear, the design does not fit the code, or the task would need a new dependency, framework or tool. `start-task` launches it for one contained change, and `implement-spec` for every leaf task. | A Summary: `status` (`done` or `stopped`), `files changed`, `checks run` with their results, the ephemeral check it ran, or why none applied, what it is `unsure` of, and, when stopped, the question or evidence under `stopped because`. | -| `wave-reviewer` | Reviews one set of changes against the requirements and design they implement and the files that cite them, gathering the diff itself and changing nothing. `implement-spec` launches it at the end of every wave. | Findings: a `verdict` (`holds` or `findings`), then each problem with its `file`, an exact `quote`, the `problem` and a `severity` (`blocker`, `should-fix` or `nit`), and what it is `unsure` of. | +| `wave-reviewer` | Reviews one set of changes against the requirements and design they implement and the files that cite them, gathering the diff itself and changing nothing. `implement-spec` launches it at the end of every wave, and once more over the whole run when you chose to hand back only at the end. | Findings: a `verdict` (`holds` or `findings`), the `base` commit the diffs were measured against, then each problem with its `file`, an exact `quote`, the `problem` and a `severity` (`blocker`, `should-fix` or `nit`), and what it is `unsure` of. | The executor works in one of two modes, set by the launch prompt alone: diff --git a/agents/wave-reviewer.md b/agents/wave-reviewer.md index 23b2cc9..603203f 100644 --- a/agents/wave-reviewer.md +++ b/agents/wave-reviewer.md @@ -1,6 +1,6 @@ --- name: wave-reviewer -description: Reviews one set of changes against the requirements and design they implement and the files they must agree with, gathering the diff itself and returning Findings, a verdict followed by each problem with an exact quote and a severity. Use it directly on any diff; implement-spec launches it once at the end of every wave. +description: Reviews one set of changes against the requirements and design they implement and the files they must agree with, gathering the diff itself and returning Findings, a verdict followed by each problem with an exact quote and a severity. Use it directly on any diff; implement-spec launches it at the end of every wave, and once more over the whole run when the author chose to hand back only at the end. tools: Read, Glob, Grep, Bash model: opus effort: high @@ -8,13 +8,13 @@ effort: high # Review one set of changes -Review the changes your launch prompt names: the files one wave of a spec changed or created, or any diff an author hands you. Gather the diff yourself, hold each change against the requirement and design excerpts it implements and against the files that cite it, and return the Findings below as your final report, with nothing after it. You are the one reader of these changes, judging each by what it is; there is no second pass with another lens and no refuter after you, so what you do not report, nobody will. +Review the changes your launch prompt names: the files one wave of a spec changed or created, every file a whole run of waves changed, or any diff an author hands you. Gather the diff yourself, hold each change against the requirement and design excerpts it implements and against the files that cite it, and return the Findings below as your final report, with nothing after it. You are the one reader of these changes, judging each by what it is; there is no second pass with another lens and no refuter after you, so what you do not report, nobody will. You read; you change nothing. Do not edit, write, create or delete a file, commit, stage, stash or check out, or run anything else that changes the working tree, and leave no scratch file. `Bash` is for `git diff`, `git status`, reading, and a check that leaves no file behind, not even bytecode or a cache, so that `git status` afterwards shows what it showed before. A validator that only reads qualifies; `python3 -m py_compile` does not, because it writes a `.pyc` under `__pycache__/`, so parse a Python file in memory instead, with `python3 -c 'import ast, sys; ast.parse(open(sys.argv[1]).read())' `. Nothing that writes, and not the test suite, which the launcher runs. ## Procedure -1. **Gather.** Run `git status --short --untracked-files=all` to tell tracked files from new ones, file by file. For each listed tracked file run `git diff HEAD -- `, or `git diff` over the range the launch names in place of a list, and read the full content of each new file, which `git diff` does not show. Then read the requirement and design excerpts the launch quotes and each file it says cites a changed one. If any of these is missing, stop: return `verdict: findings` with a `blocker` for each missing input, naming it, and do not review what you have as if it were complete. Missing means a listed file you cannot find, a `git diff` that fails or that comes back empty for a listed file `git status` does not show as new, or a cited excerpt or citing file you can neither read nor find quoted in the launch; a launch that gives fewer of these, no design excerpt, say, or no citing files, is complete as it is, and you review against what it gives. +1. **Gather.** Run `git status --short --untracked-files=all` to tell tracked files from new ones, file by file. Find the branch's base, so that a commit made between waves does not hide an earlier wave's changes: the default branch is `git rev-parse --abbrev-ref origin/HEAD` with the `origin/` prefix removed, or `main` when that command fails, as it does when `origin/HEAD` is unset, even though it still prints `origin/HEAD`; the base is `git merge-base origin/ HEAD`; when there is no `origin`, it is `git merge-base HEAD` with the local default branch, if that branch exists and is not the current one; when neither exists, or the merge base cannot be found, the base is `HEAD`. For each listed tracked file run `git diff -- `, from the base to the working tree, or `git diff` over the range the launch names in place of a list, and read the full content of each new file, which `git diff` does not show. Then read the requirement and design excerpts the launch quotes and each file it says cites a changed one. If any of these is missing, stop: return `verdict: findings` with a `blocker` for each missing input, naming it, and do not review what you have as if it were complete. Missing means a listed file you cannot find, a `git diff` that fails or that comes back empty against the base for a listed file `git status` does not show as new, or a cited excerpt or citing file you can neither read nor find quoted in the launch; a launch that gives fewer of these, no design excerpt, say, or no citing files, is complete as it is, and you review against what it gives. 2. **Classify.** Decide what each change is: text that instructs (a skill, an agent, a rule, a prompt, a README); code no test here exercises, because the suite cannot run on this machine or because nothing covers the change yet; a file that other files cite. A change can be more than one of these; apply every reading that fits. 3. **Judge.** Hold every change, whatever its class, against the excerpts it implements: does it do what they say, all of it, and nothing they rule out. Then, by class: - Instructing text: dry-run it. Take one concrete case the text is written for, the launch's own situation or one you name, follow the text step by step as its reader would, and report every point where you did not know what to do next, where two readings were open, or where it sent you to something that is not there. @@ -30,6 +30,7 @@ The team rules in your context are the bar: a change that breaks one is a findin ``` verdict: holds | findings +base: , merge base with , HEAD, or the launch's range> findings: - file: quote: @@ -39,4 +40,4 @@ unsure: - ``` -`verdict` is `findings` when the list has an entry and `holds` when it is empty, never one without the other. `file` is the path the quote comes from, a changed file or a citing one; for a missing input it is the path that is missing, and the quote is the launch's words that name it. `quote` is text copied exactly, so the launcher can find it by search; what you cannot quote goes under `unsure`. `severity` is `blocker` when the change does the wrong thing: it contradicts a cited excerpt or a citing file, breaks a team rule, or gives a wrong result on a case the cited text names; `blocker` also when the review could not be done. `should-fix` is for what a reader or caller would stumble on: a two-way reading, a gap, or a wrong result on an edge case the cited text only implies. `nit` is for wording alone. Report what you read; mark anything inferred as inferred, and put what you noticed outside the changes under `unsure`. The team rules are in your context; if they are not, say so under `unsure`. +`verdict` is `findings` when the list has an entry and `holds` when it is empty, never one without the other. `base` is the one line that tells the launcher what the diffs were measured against. `file` is the path the quote comes from, a changed file or a citing one; for a missing input it is the path that is missing, and the quote is the launch's words that name it. `quote` is text copied exactly, so the launcher can find it by search; what you cannot quote goes under `unsure`. `severity` is `blocker` when the change does the wrong thing: it contradicts a cited excerpt or a citing file, breaks a team rule, or gives a wrong result on a case the cited text names; `blocker` also when the review could not be done. `should-fix` is for what a reader or caller would stumble on: a two-way reading, a gap, or a wrong result on an edge case the cited text only implies. `nit` is for wording alone. Report what you read; mark anything inferred as inferred, and put what you noticed outside the changes under `unsure`. The team rules are in your context; if they are not, say so under `unsure`. diff --git a/checks/test_descriptions.py b/checks/test_descriptions.py new file mode 100644 index 0000000..b6e0ab1 --- /dev/null +++ b/checks/test_descriptions.py @@ -0,0 +1,51 @@ +"""Every skill's and agent's frontmatter description is present and at most 1,024 characters. + +Claude Code rejects a skill description over 1,024 characters, and the validator +gives no warning on the way there. Agent descriptions have no documented limit +but load into every session's context, so they are held to the same ceiling. +""" + +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parent.parent +AGENTS_DIR = ROOT / "agents" +SKILLS_DIR = ROOT / "skills" + +DESCRIPTION_LIMIT = 1024 + + +def read_frontmatter(path): + """Return the flat ``key: value`` pairs between the leading ``---`` fences, or None if there is no block.""" + lines = path.read_text(encoding="utf-8").splitlines() + if not lines or lines[0].strip() != "---": + return None + fields = {} + for line in lines[1:]: + if line.strip() == "---": + return fields + key, separator, value = line.partition(":") + if separator: + fields[key.strip()] = value.strip() + return None + + +class DescriptionLengthTest(unittest.TestCase): + def test_descriptions_are_present_and_within_the_limit(self): + paths = sorted(SKILLS_DIR.glob("*/SKILL.md")) + sorted(AGENTS_DIR.glob("*.md")) + self.assertTrue(paths, "no skills/*/SKILL.md or agents/*.md found") + for path in paths: + with self.subTest(file=str(path.relative_to(ROOT))): + fields = read_frontmatter(path) + self.assertIsNotNone(fields, "no frontmatter block between --- fences") + description = fields.get("description", "") + self.assertTrue(description, "description is empty or missing") + self.assertLessEqual( + len(description), + DESCRIPTION_LIMIT, + "description has {} characters; the limit is {}".format(len(description), DESCRIPTION_LIMIT), + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/evals/README.md b/evals/README.md index 2256d6f..6a4e88a 100644 --- a/evals/README.md +++ b/evals/README.md @@ -29,7 +29,7 @@ Every case carries its component's name as a tag, and each target is `claude plugin eval . --tag ` plus the flags its cases need, which live in the Makefile's "Per-target flags" block. Anything extra goes through `EVAL_ARGS`, and the per-invocation cost ceiling through `EVAL_MAX_COST_USD` -(default $2, which the full suite needs raised): +(default $2, which the full suite and `make eval-skill-implement-spec` need raised): ```bash make eval-skill-pr EVAL_ARGS="--runs 1" # one run per case while iterating diff --git a/evals/skills/implement-spec/works-all-waves/README.md b/evals/skills/implement-spec/works-all-waves/README.md new file mode 100644 index 0000000..dfb68ce --- /dev/null +++ b/evals/skills/implement-spec/works-all-waves/README.md @@ -0,0 +1,62 @@ +# works-all-waves + +Tests the implement-spec work loop at spec cadence: work every wave without +waiting, each with its executor, its own test run, its wave review and its +full suite, write a short report after each wave, have the reviewer agent read +the whole run once more in a final review, and hand back once at the end. + +## What the scaffold builds + +The same workspace as works-one-wave: a small Python package on branch +`calc-subtract` with a committed spec under `.claude/specs/calc-subtract/`, +two tasks in two waves (add `subtract`, then test it). The package is +`evals/fixtures/calc/`, built by `evals/fixtures/setup.sh`; the spec is this +case's `files/` folder, a copy of works-one-wave's, copied in before that +script's one commit. Tests run with `python3 -m unittest`. `bin/` holds +wrappers for `git` and `python3`, because the `/usr/bin` shims on macOS need an +xcrun cache the sandbox forbids; the Makefile's implement-spec target puts +`bin` on the PATH and grants Bash, Write and Edit. + +The prompt gives the start answers up front (spec cadence, no optional tests, +already on a branch), since nothing can answer mid-run. + +## Graders + +- `launches-sub-agents`: the Agent tool was called at least three times. A + full run makes five calls (two executors, two wave reviewers, one final + reviewer); three proves the loop went past the first wave. +- `runs-tests-itself`: the orchestrator ran `unittest` through Bash, not just + the sub-agents. +- `adds-subtract`: `calc/ops.py` contains the function task 1 describes. +- `works-wave-1`: `tests/test_ops.py` gained `test_subtract`, so wave 1 ran + without a hand-back after wave 0. works-one-wave grades the opposite. +- `reports-every-wave`: the final report carries the wave review line of + wave 1, so the report covers each wave and not only the first. +- `reviews-the-run`: the final report carries the final review's verdict, so + the reviewer was launched once more over the whole run. +- `reports-wave-0-inline`: reads the whole trace in order and needs the wave 0 + review line to be followed, later in the session, by an executor launch. + That only happens when the wave 0 report was written before wave 1 began, + so it grades the inline report; a run that writes the review lines only in + the final report has no launch after them and fails. One run on 2026-10-14 + did exactly that, with four short progress lines and then the final report. + +## What it cannot check + +The ticks, for the reason works-one-wave gives: inside an eval run, Claude +Code refuses edits under `.claude/`, so each tick is denied and the skill is +expected to say so. At spec cadence that refusal must not end the run early; +if it does, `works-wave-1` and `reviews-the-run` fail, and the transcript +shows the hand-back after wave 0. + +## Running it + +```bash +make eval-skill-implement-spec EVAL_ARGS="--case works-all-waves" +``` + +About $0.90 a run at list price, roughly double works-one-wave; it spawns five +sub-agents, two executors and three reviewers, all on Opus. With this case the +`make eval-skill-implement-spec` target runs past the default $2 +`EVAL_MAX_COST_USD` ceiling at three runs a case, so raise it, for example +`EVAL_MAX_COST_USD=6`, or run one case at a time with `--case`. diff --git a/evals/skills/implement-spec/works-all-waves/case.yaml b/evals/skills/implement-spec/works-all-waves/case.yaml new file mode 100644 index 0000000..782ce95 --- /dev/null +++ b/evals/skills/implement-spec/works-all-waves/case.yaml @@ -0,0 +1,4 @@ +schema_version: "1.1" +name: works-all-waves +context: + scaffold_script: scaffold.sh diff --git a/evals/skills/implement-spec/works-all-waves/files/.claude/specs/calc-subtract/design.md b/evals/skills/implement-spec/works-all-waves/files/.claude/specs/calc-subtract/design.md new file mode 100644 index 0000000..a4627e3 --- /dev/null +++ b/evals/skills/implement-spec/works-all-waves/files/.claude/specs/calc-subtract/design.md @@ -0,0 +1,12 @@ +# Design Document + +## Overview + +One function in `calc/ops.py`, next to `add` and `multiply`, and one test method in `tests/test_ops.py`, in the existing `OpsTest` class. Tests run with `python3 -m unittest`. + +## Components and Interfaces + +| Component | Responsibility | Requirements | +|-----------|----------------|--------------| +| `calc/ops.py` `subtract(a, b)` | returns a minus b | 1.1 | +| `tests/test_ops.py` `test_subtract` | asserts subtract(5, 3) == 2 | 1.2 | diff --git a/evals/skills/implement-spec/works-all-waves/files/.claude/specs/calc-subtract/requirements.md b/evals/skills/implement-spec/works-all-waves/files/.claude/specs/calc-subtract/requirements.md new file mode 100644 index 0000000..a2132da --- /dev/null +++ b/evals/skills/implement-spec/works-all-waves/files/.claude/specs/calc-subtract/requirements.md @@ -0,0 +1,20 @@ +# Requirements Document + +## Introduction + +Add subtraction to the calc package so callers have the fourth basic operation. + +## Glossary + +- **Package**: The `calc` Python package. + +## Requirements + +### Requirement 1: Subtract + +**User Story:** As a caller, I want `subtract(a, b)`, so that I can compute a minus b. + +#### Acceptance Criteria + +1. WHEN a caller calls `subtract(a, b)`, THE Package SHALL return a minus b. +2. THE Package SHALL cover `subtract` with a unit test in the existing test module. diff --git a/evals/skills/implement-spec/works-all-waves/files/.claude/specs/calc-subtract/tasks.md b/evals/skills/implement-spec/works-all-waves/files/.claude/specs/calc-subtract/tasks.md new file mode 100644 index 0000000..3e95760 --- /dev/null +++ b/evals/skills/implement-spec/works-all-waves/files/.claude/specs/calc-subtract/tasks.md @@ -0,0 +1,28 @@ +# Implementation Plan + +## Overview + +Two tasks: the function, then its test. Tests run with `python3 -m unittest`. + +## Tasks + +- [ ] 1. Add `subtract(a, b)` to `calc/ops.py`, returning a minus b, in the style of `add` + - _Requirements: 1.1_ + +- [ ] 2. Add `test_subtract` to `OpsTest` in `tests/test_ops.py`, asserting subtract(5, 3) == 2 + - _Requirements: 1.2_ + +## Notes + +- Task numbers identify tasks; the graph decides the order. + +## Task Dependency Graph + +```json +{ + "waves": [ + { "id": 0, "tasks": ["1"] }, + { "id": 1, "tasks": ["2"] } + ] +} +``` diff --git a/evals/skills/implement-spec/works-all-waves/graders/adds-subtract.md b/evals/skills/implement-spec/works-all-waves/graders/adds-subtract.md new file mode 100644 index 0000000..cb61aba --- /dev/null +++ b/evals/skills/implement-spec/works-all-waves/graders/adds-subtract.md @@ -0,0 +1,5 @@ +--- +type: regex +pattern: 'def subtract\(a, b\)' +target: { source: file, path: calc/ops.py } +--- diff --git a/evals/skills/implement-spec/works-all-waves/graders/launches-sub-agents.md b/evals/skills/implement-spec/works-all-waves/graders/launches-sub-agents.md new file mode 100644 index 0000000..5cd54af --- /dev/null +++ b/evals/skills/implement-spec/works-all-waves/graders/launches-sub-agents.md @@ -0,0 +1,5 @@ +--- +type: tool_used +tool: Agent +min: 3 +--- diff --git a/evals/skills/implement-spec/works-all-waves/graders/reports-every-wave.md b/evals/skills/implement-spec/works-all-waves/graders/reports-every-wave.md new file mode 100644 index 0000000..8ebdde2 --- /dev/null +++ b/evals/skills/implement-spec/works-all-waves/graders/reports-every-wave.md @@ -0,0 +1,4 @@ +--- +type: regex +pattern: 'Wave review of wave 1:' +--- diff --git a/evals/skills/implement-spec/works-all-waves/graders/reports-wave-0-inline.md b/evals/skills/implement-spec/works-all-waves/graders/reports-wave-0-inline.md new file mode 100644 index 0000000..41d06a6 --- /dev/null +++ b/evals/skills/implement-spec/works-all-waves/graders/reports-wave-0-inline.md @@ -0,0 +1,6 @@ +--- +type: regex +pattern: 'Wave review of wave 0:[\s\S]*freethinking:task-executor' +match: contains +target: trace +--- diff --git a/evals/skills/implement-spec/works-all-waves/graders/reviews-the-run.md b/evals/skills/implement-spec/works-all-waves/graders/reviews-the-run.md new file mode 100644 index 0000000..cffaef2 --- /dev/null +++ b/evals/skills/implement-spec/works-all-waves/graders/reviews-the-run.md @@ -0,0 +1,4 @@ +--- +type: regex +pattern: 'Final review of the run:' +--- diff --git a/evals/skills/implement-spec/works-all-waves/graders/runs-tests-itself.md b/evals/skills/implement-spec/works-all-waves/graders/runs-tests-itself.md new file mode 100644 index 0000000..fd83798 --- /dev/null +++ b/evals/skills/implement-spec/works-all-waves/graders/runs-tests-itself.md @@ -0,0 +1,5 @@ +--- +type: tool_used +tool: Bash +input_match: 'unittest' +--- diff --git a/evals/skills/implement-spec/works-all-waves/graders/works-wave-1.md b/evals/skills/implement-spec/works-all-waves/graders/works-wave-1.md new file mode 100644 index 0000000..93d0260 --- /dev/null +++ b/evals/skills/implement-spec/works-all-waves/graders/works-wave-1.md @@ -0,0 +1,5 @@ +--- +type: regex +pattern: 'test_subtract' +target: { source: file, path: tests/test_ops.py } +--- diff --git a/evals/skills/implement-spec/works-all-waves/prompt.md b/evals/skills/implement-spec/works-all-waves/prompt.md new file mode 100644 index 0000000..2d0fdbd --- /dev/null +++ b/evals/skills/implement-spec/works-all-waves/prompt.md @@ -0,0 +1,10 @@ +--- +description: On a scaffolded project with a committed two-wave spec, implement-spec works the whole spec at spec cadence. It works wave 0 and wave 1 without waiting between them, each with its executor agent, its own test run, its wave-reviewer agent and its full suite, writes a report after each wave, has the reviewer agent read the whole run once more in a final review, and hands back once at the end. Needs the Makefile's implement-spec target, which grants Bash, Write and Edit, runs the scaffold, and puts the wrappers in bin on the PATH. +tags: [skill, implement-spec] +max_turns: 70 +allowed_tools: [Read, Glob, Grep, Agent] +expected_outcome: Claude validates the graph, launches the task-executor agent for task 1, runs the tests itself, launches the wave-reviewer agent for wave 0, writes the wave's report and starts wave 1 without waiting, does the same for task 2, then launches the wave-reviewer agent once more over the whole run and hands back with a final report carrying a Wave review line for each wave and a Final review line. Its attempts to tick the tasks are refused, because the sandbox denies edits under .claude/, and it says so rather than working around it. +--- +/freethinking:implement-spec calc-subtract spec + +I will not be able to answer questions during this run. Cadence: spec, run every wave and hand back only at the end. There are no optional tests. The branch is already calc-subtract. This sandbox refuses edits under .claude/, so if ticking a task in tasks.md is refused, say so in the report and carry on to the next wave as if it were ticked. diff --git a/evals/skills/implement-spec/works-all-waves/scaffold.sh b/evals/skills/implement-spec/works-all-waves/scaffold.sh new file mode 100755 index 0000000..bd522f2 --- /dev/null +++ b/evals/skills/implement-spec/works-all-waves/scaffold.sh @@ -0,0 +1,10 @@ +#!/usr/bin/env bash +# Builds the workspace for this case: the calc fixture on branch calc-subtract +# with the two-wave spec from files/.claude/specs/calc-subtract/ committed +# alongside it, and the bin wrappers the Makefile's implement-spec target puts +# on the PATH. The spec is copied in before the setup script runs so that its +# one commit holds it, on main and on the branch. +set -euo pipefail +ROOT=$(git -C "$(dirname "$0")" rev-parse --show-toplevel) +cp -R "$(dirname "$0")/files/." . +bash "$ROOT/evals/fixtures/setup.sh" calc calc-subtract wrappers diff --git a/rules/workflow.md b/rules/workflow.md index d73fae9..ac1f345 100644 --- a/rules/workflow.md +++ b/rules/workflow.md @@ -5,7 +5,7 @@ These rules say which handbook skill to reach for and when. They apply in every The first bullet that matches the request is the route. - **Bug named: reproduce before fixing.** When the author calls the request a bug, or hands over a ticket filed as one, run `/freethinking:bugfix`. Reproduce the bug first, confirm the root cause with the author before changing code, and guard the fix with a regression test. "It works now" is not a fix. -- **Spec exists: follow it.** If `.claude/specs//tasks.md` exists for the work in hand, run `/freethinking:implement-spec`. It works the tasks in wave order, ticks a task's checkbox when its tests pass, and stops for review at the author's cadence, after each task or after each wave. Do not skip ahead or merge tasks. +- **Spec exists: follow it.** If `.claude/specs//tasks.md` exists for the work in hand, run `/freethinking:implement-spec`. It works the tasks in wave order; do not skip ahead or merge tasks. - **Pull request: use the skill.** Open and update PRs with `/freethinking:pr`. It walks the changes with the author and drafts the description to the team template. Do not open a PR with a bare `gh pr create`. - **Small changes stay small.** A typo, a one-line fix or a rename needs no skill. Say that the change is small enough to skip `start-task`, then make it. - **Anything else that changes code: start the task.** Run `/freethinking:start-task`. It reads the code and decides between the executor for one contained change, `bugfix` for a defect, and a spec for the rest, and asks before taking the first two. Do not judge the size yourself. diff --git a/skills/implement-spec/SKILL.md b/skills/implement-spec/SKILL.md index 5a975cb..ba906ce 100644 --- a/skills/implement-spec/SKILL.md +++ b/skills/implement-spec/SKILL.md @@ -1,13 +1,13 @@ --- name: implement-spec -description: Build a task whose spec already exists, wave by wave from its tasks.md, each task built by the task-executor agent and each wave read by the wave-reviewer agent before the wave ends. Use when asked to implement, build, work, continue or resume a spec, a task list, or the tasks in .claude/specs//. Reads the spec, validates its dependency graph, asks the author how often to hand back (after each task or each wave) and whether to write the optional tests, then works the waves in order with one sub-agent per leaf task, ticking each checkbox only when its tests pass or a stated manual check is done, running the full suite at the end of every wave, and handing back for review at the chosen cadence. Never edits the spec beyond checkboxes without the author's approval, and pauses with a proposal when the design cannot be followed. -argument-hint: "[task-name] [task|wave]" +description: Build a task whose spec already exists, wave by wave from its tasks.md, with the task-executor agent building each task and the wave-reviewer agent reading each wave. Use when asked to implement, build, work, continue or resume a spec, a task list, or the tasks in .claude/specs//. Reads the spec and validates its dependency graph, then asks once how often to hand back (after each task, each wave, or only at the end) and how to treat optional tests (ask, always, never). Runs one sub-agent per leaf task, ticks a checkbox only when its tests pass or a stated manual check is done, and runs the full suite at the end of every wave. Never edits the spec beyond checkboxes without the author's approval, and pauses with a proposal when the design cannot be followed. +argument-hint: "[task-name] [task|wave|spec]" allowed-tools: Bash(git status *) Bash(git branch *) Bash(git rev-parse *) Bash(git log *) Bash(git diff *) Bash(ls .claude/specs*) Bash(cat .claude/specs/*) --- # Implement a spec -Build a task from its approved spec: read `.claude/specs//`, check that its task list can be executed, and then work the waves in order, each leaf task built by its own executor and each wave read by the reviewer before the wave ends, ticking each checkbox only when its tests have passed or, for a task with nothing to test, after a stated manual check. Hand back to the author as often as they chose, after every task or after every wave, with a report of what was built and tested. +Build a task from its approved spec: read `.claude/specs//`, check that its task list can be executed, and then work the waves in order, each leaf task built by its own executor and each wave read by the reviewer before the wave ends, ticking each checkbox only when its tests have passed or, for a task with nothing to test, after a stated manual check. Hand back to the author as often as they chose, after every task, after every wave, or only once every wave is done, with a report of what was built and tested. The author is accountable for the code and owns the spec. The spec is the source of truth: `requirements.md` says what, `design.md` says how, and `tasks.md` says in what order. Do not change any of them, beyond ticking checkboxes, without the author's approval, and do not guess your way past a design that cannot be followed; pause and propose the change instead. @@ -18,18 +18,30 @@ Terms used below: - A **leaf task** is a checkbox task in `tasks.md` with no sub-tasks: a top-level task with no sub-tasks is a leaf; a parent with sub-tasks is not. A leaf task marked `*` is an **optional test**. - An **ephemeral check** is a scratch script or call that exercises new code to prove a task works, is never added to the codebase, and is deleted before the task is done. It is how a code task is verified when its tests are a task of their own. - A **wave** is one entry of the Task Dependency Graph at the end of `tasks.md`. Waves run in order. Task numbers only identify tasks; the graph decides the order of work. -- The **cadence** is the author's choice of when a run hands back: after each leaf task, or after each wave. +- The **cadence** is the author's choice of when a run hands back: after each leaf task (task cadence), after each wave (wave cadence), or only at the end of the run (spec cadence). At spec cadence every wave still ends with its review, its full suite and its report, but the report is written into the conversation without waiting for the author, and the next wave starts at once, so the session holds one short report per wave and never the diff; once the last wave is done, the reviewer reads the whole run once more, in a final review, and the run hands back with the final report. Before the end it hands back only for a failure, an unfixed finding or a proposal. +- The **optional-tests choice** is the author's choice of what to do with optional tests: ask about them (at the start of each wave that has one, or at spec cadence once at the end of the run), always do them, or never do them. - A **skipped test** is an optional test the author chose not to do in this run. It stays unticked. - An **executor** is the plugin's `task-executor` agent, launched as `freethinking:task-executor`, one per leaf task. Its own definition holds its procedure, so the launch carries only the task: it gathers the context the task needs, implements exactly what the task describes, proves it, and returns a summary with a `status` of `done` or `stopped`, the files it changed, the checks it ran, its ephemeral check's code, what it is unsure of and, when stopped, why. Tests are tasks of their own in a spec, so an executor writes a test only when its task is a test task. -- The **reviewer** is the plugin's `wave-reviewer` agent, launched as `freethinking:wave-reviewer`, one per wave. It is read-only and changes nothing; it gathers the wave's diff itself and returns Findings: a `verdict` of `holds` or `findings`, then each finding with its file, an exact quote, the problem and a severity of `blocker`, `should-fix` or `nit`, then what it is `unsure` of. Its own definition holds its procedure, so the launch carries only what to review. +- The **reviewer** is the plugin's `wave-reviewer` agent, launched as `freethinking:wave-reviewer`, one per wave, and at spec cadence once more over the whole run. It is read-only and changes nothing; it gathers the wave's diff itself and returns Findings: a `verdict` of `holds` or `findings`, then each finding with its file, an exact quote, the problem and a severity of `blocker`, `should-fix` or `nit`, then what it is `unsure` of. Its own definition holds its procedure, so the launch carries only what to review. - A **sub-agent**, below, means either. +## Working with the author + +- **Stops.** Most stops happen only in some runs; a run with a valid graph in which nothing goes wrong stops only at the first group. + - Every run: the set-up round of step 4, with whichever of its questions apply; the hand-back of step 6 at the chosen cadence. At spec cadence that is the report of step 6 written at the end of each wave, which does not wait for the author, and the final report of step 8, which does. + - Only when the author left something out: the choice among several specs in step 1, when no task name was given. + - Only when something is wrong: the approval of a graph fix in step 2; the question in step 6 of how to proceed after a failing suite, a task that will not pass or an unfixed finding; the proposal in step 6 when the design cannot be followed or a task needs a new dependency. + - Only when the author asks for it: the question in step 3 whether to do the remaining optional tests of an otherwise complete spec; the optional-tests question at the start of each wave that has one, in step 5, when the author chose to be asked, or at spec cadence the one offer of every skipped optional test at the end of the run, in step 8; the confirmation in step 7 before skipping a non-optional task or working out of wave order; the request in step 5 for a task done by hand, when the spec has one. +- **How it asks.** With the AskUserQuestion tool, fixed options plus free text. The set-up round of step 4 is one call holding the cadence (after each task; after each wave; run every wave and hand back only at the end), the optional tests (ask about them, at each wave that has one or at spec cadence once at the end; always do them; never do them) and the branch (create it under the proposed name). The tool also asks the per-wave optional-tests question (do them in this wave, skip them), the end-of-run optional-tests offer at spec cadence (do them now, leave them), the choice of spec in step 1, the confirmations of steps 3 and 7, and the approval of a graph fix or spec edit once the edit is shown in full. The question of how to proceed after a hand-back, and the request for a task done by hand, are prose, since the answer is the author's own words. +- **Hand-backs.** The two tables of step 2; the report of step 6 in its fixed shape, with the failing output verbatim or the proposed edit quoted as it would read when the hand-back is for one; at spec cadence, the report of step 6 for each wave, written without waiting, and the report of step 8, covering the run with a line for the final review. +- **Hand-offs.** `start-task` when there is no spec or no `tasks.md`; `pr` when every non-optional task is ticked, at step 3 or step 8. + ## 1. Find the spec -Take the task name from the arguments: the first word that is not `task` or `wave`. The cadence is a second argument of `task` or `wave`, or an explicit phrase in the author's message such as "one task at a time" or "a wave at a time". The word "task" on its own in a sentence is not a cadence. +Take the task name from the arguments: the first word that is not `task`, `wave` or `spec`. The cadence is a second argument of `task`, `wave` or `spec`, or an explicit phrase in the author's message such as "one task at a time", "a wave at a time" or "run all the waves". The word "task" or "spec" on its own in a sentence is not a cadence. The optional-tests choice is settled here only by an explicit phrase in the author's message, such as "skip the optional tests". - With a task name, the spec is `.claude/specs//`. -- With no task name, run `ls .claude/specs/*/tasks.md` and look at each list for an unticked non-optional leaf task. Exactly one such spec: name it and confirm it with the author. Several: list them and ask which to work. None: say there is nothing to implement, name any spec whose only unticked tasks are optional tests, recommend `/freethinking:start-task` otherwise, and stop. +- With no task name, run `ls .claude/specs/*/tasks.md` and look at each list for an unticked non-optional leaf task. Exactly one such spec: name it and proceed. Several: list them and ask which to work. None: say there is nothing to implement, name any spec whose only unticked tasks are optional tests, recommend `/freethinking:start-task` otherwise, and stop. If the spec has no `tasks.md`, say so, recommend `/freethinking:start-task`, and stop without changing anything. One look at `.claude/specs//tasks.md` settles it; do not search elsewhere for the spec. @@ -62,21 +74,23 @@ State where things stand: the wave found in step 3, every leaf task in it with w Then ask the author one round of questions, batched, and only the ones that apply: -- **Cadence**, if step 1 did not settle it: hand back after each leaf task, or after each wave? -- **Optional tests**, if the wave from step 3 contains any and step 3 did not already settle it: do them in this wave, or skip them? The question is asked again for each later wave that contains one. -- **Branch**, if `git branch --show-current` is the repository's default branch (`git rev-parse --abbrev-ref origin/HEAD` with the `origin/` prefix removed, or `main` if that is unset): propose the task name as the branch name, and once the author confirms create it and switch to it with `git checkout -b `. The workflow rule forbids working on the default branch. +- **Cadence**, if step 1 did not settle it: hand back after each leaf task, after each wave, or run every wave and hand back only at the end? +- **Optional tests**, if the wave from step 3 or a later one contains an unticked optional test and neither step 1 nor step 3 already settled it: ask about them (at the start of each wave that has one, or at spec cadence once at the end of the run), always do them, or never do them? +- **Branch**, if `git branch --show-current` is the repository's default branch (`git rev-parse --abbrev-ref origin/HEAD` with the `origin/` prefix removed, or `main` when that command fails because it is unset): propose the task name as the branch name, and once the author confirms create it and switch to it with `git checkout -b `. The workflow rule forbids working on the default branch. + +Ask the cadence and optional-tests questions in one AskUserQuestion call, with the branch question when it applies. They are asked only here: both answers hold for the whole run, through every resume in step 7. Make no code change until the author has answered. ## 5. Work the wave -Work one wave at a time; waves run in order. Each leaf task in a wave gets its own executor, launched with the Agent tool as `subagent_type: freethinking:task-executor`, which returns at once and runs the sub-agent in the background; once every executor of the wave has returned, the wave gets one reviewer. At wave cadence, launch every task of the wave before waiting on any of them, all in one message, so they run concurrently. At task cadence, launch one and wait for its result before the next, so that the hand-back after each task still makes sense. Each agent runs on the model its own definition pins; verifying, ticking, the full suite and the report are not delegated: they are done in this session, on the session's own model. +Work one wave at a time; waves run in order. Each leaf task in a wave gets its own executor, launched with the Agent tool as `subagent_type: freethinking:task-executor`, which returns at once and runs the sub-agent in the background; once every executor of the wave has returned, the wave gets one reviewer. At wave or spec cadence, launch every task of the wave before waiting on any of them, all in one message, so they run concurrently. At task cadence, launch one and wait for its result before the next, so that the hand-back after each task still makes sense. Each agent runs on the model its own definition pins; verifying, ticking, the full suite and the report are not delegated: they are done in this session, on the session's own model. Beyond ticking checkboxes, do not edit `tasks.md`, `requirements.md` or `design.md` in this step. A change any of them needs goes to step 6 as a proposal. ### Starting a wave -If the wave contains optional tests and the author has not yet said whether to do them in this wave, ask once now. Then check the code for every unticked task in the wave other than a skipped test: look for the behaviour, file or test the task describes, and tell the author which tasks already appear done. A task whose text is an action for the author to take by hand, such as running a session and recording what happened, gets no sub-agent either: ask the author to do it, and tick it when they report the result. It stays outside the wave review, since the reviewer has no change of it to read; if it did change files, the author names them and they join the reviewer's file list. A task that appears done gets no sub-agent. Verify it yourself: the test it names if it is a test task, otherwise an ephemeral check of the behaviour plus the existing tests for the code it touches, or a stated manual check if there is nothing to test. A pass does not tick it at once: it waits for the wave's review and is ticked with the rest, and the files its task text names, or its check touched, join the reviewer's file list. A failure means it was not done, so launch an executor for it after all. The same holds for a task that appears done when a run resumes after a hand-back. +If the wave contains optional tests: when the author chose to be asked, ask once now whether to do them in this wave, except at spec cadence, where they are skipped tests for now and offered once at the end of the run in step 8; when they chose always, they are worked like any other task; when they chose never, they are skipped tests. Then check the code for every unticked task in the wave other than a skipped test: look for the behaviour, file or test the task describes, and tell the author which tasks already appear done. A task whose text is an action for the author to take by hand, such as running a session and recording what happened, gets no sub-agent either: ask the author to do it, and tick it when they report the result. It stays outside the wave review, since the reviewer has no change of it to read; if it did change files, the author names them and they join the reviewer's file list. A task that appears done gets no sub-agent. Verify it yourself: the test it names if it is a test task, otherwise an ephemeral check of the behaviour plus the existing tests for the code it touches, or a stated manual check if there is nothing to test. A pass does not tick it at once: it waits for the wave's review and is ticked with the rest, and the files its task text names, or its check touched, join the reviewer's file list. A failure means it was not done, so launch an executor for it after all. The same holds for a task that appears done when a run resumes after a hand-back. ### Launching a task @@ -92,7 +106,7 @@ Nothing more. The agent's own definition carries its procedure, the shape of its ### When a task comes back -At wave cadence, wait until every executor of the wave has returned before verifying or ticking any task that had one, so that nothing is run while a sibling is still editing and a file two of them changed is judged as a whole. At task cadence handle each task as it returns. For each task: +At wave or spec cadence, wait until every executor of the wave has returned before verifying or ticking any task that had one, so that nothing is run while a sibling is still editing and a file two of them changed is judged as a whole. At task cadence handle each task as it returns. For each task: 1. Read the summary's `status` first. `stopped` is drift: the agent found that the design does not fit the code, that the task would need a new dependency, framework or tool, or that the task is unclear, and it changed nothing more. Do not let it work around that, and do not relaunch or retry the task unprompted. The task stays unticked, and its `files changed` goes under the report's Files changed as what it had already touched. Finish judging the wave's other tasks as below, through the wave review, then go to step 6 once, with its `stopped because` as the evidence, and propose the spec edit. The rest of this list is for a summary with `status: done`. 2. Read the rest of the summary. A summary whose `checks run` lists nothing and whose `ephemeral check` gives no reason why none applied goes back to the executor. @@ -102,7 +116,7 @@ At wave cadence, wait until every executor of the wave has returned before verif Then: -- Tests pass: tick the task's checkbox in `tasks.md`, after the wave review below: at wave cadence every task waits for it, and at task cadence only the wave's last task does, the earlier ones being ticked at their hand-backs. Never tick a task whose tests, or stated manual check, you have not run yourself in this session. When every sub-task of a top-level task is ticked, tick the top-level task too. +- Tests pass: tick the task's checkbox in `tasks.md`, after the wave review below: at wave or spec cadence every task waits for it, and at task cadence only the wave's last task does, the earlier ones being ticked at their hand-backs. Never tick a task whose tests, or stated manual check, you have not run yourself in this session. When every sub-task of a top-level task is ticked, tick the top-level task too. - Tests fail: send the output to the same executor to fix, or to a new one launched as above and given its summary and the failing output, for up to two fix attempts. If they still fail, the task stays unticked; once the wave review below is done, go to step 6 with the output verbatim. Other tasks of the wave already in flight, including their fix attempts within the two allowed, are left to finish and judged as above; no further task of the wave is launched. Skipped tests stay unticked and are listed as skipped in the report. @@ -118,7 +132,7 @@ Launch the reviewer with `subagent_type: freethinking:wave-reviewer`, once, and - the requirement and design excerpts the wave's tasks cite, quoted; - the files that cite the changed ones: search the repository for each changed file's path and name. -Nothing more, and never the diff: the reviewer runs `git diff` and reads the files itself, so the diff never enters this session. Its own definition carries its procedure and the shape of its Findings, and the `SubagentStart` hook puts the handbook rules in its context, so the launch restates none of them. Wait for its Findings. One reviewer per wave: no second reviewer with another lens, no refuter after it, and no second review after the fixes. +Nothing more, and never the diff: the reviewer runs `git diff` and reads the files itself, so the diff never enters this session. Its own definition carries its procedure and the shape of its Findings, and the `SubagentStart` hook puts the handbook rules in its context, so the launch restates none of them. Wait for its Findings. One reviewer per wave: no second reviewer with another lens, no refuter after it, and no second review after the fixes. The one other review is the final review of the whole run at spec cadence, in step 8. Collect every `unsure` item from the wave's summaries, and the reviewer's own, beside its findings. Then: @@ -133,11 +147,11 @@ Tick only after that: every task of the wave whose checks passed, any that appea ### Ending a wave -With the review done and every task in the wave ticked, other than a skipped test or a task the author chose to skip under step 7, run the project's full test suite, the way the project runs it. If it passes, go to step 6 and report, with the suite's result in the report; at task cadence that is the report for the wave's last task. If it fails, go to step 6 with the failing output verbatim and ask the author how to proceed before starting the next wave. Never start a task from a later wave while a task in this wave, other than a skipped test, is unticked. +With the review done and every task in the wave ticked, other than a skipped test or a task the author chose to skip under step 7, run the project's full test suite, the way the project runs it. If it passes, go to step 6 and report, with the suite's result in the report; at task cadence that is the report for the wave's last task. At spec cadence a pass goes to step 6 too, for the wave's report, but does not wait: write the report in full, in the report shape, as a message of its own, then start the next wave at once. A line saying the wave is done is not the report; the report is what the final review reads back. If it fails, go to step 6 with the failing output verbatim and ask the author how to proceed before starting the next wave. Never start a task from a later wave while a task in this wave, other than a skipped test, is unticked. ## 6. Hand back and report -Hand back here at the end of a cadence unit (after a task at task cadence, after a wave at wave cadence), after a failed full suite, after a task whose tests still fail after two fix attempts, after a review finding that is still not fixed after two attempts, or when an executor reports that the design cannot be followed or needs a new dependency. For the final wave, when its full suite passes, step 8 takes this step's place. Write the report, in this shape and order, then wait for the author: +Hand back here at the end of a cadence unit (after a task at task cadence, after a wave at wave or spec cadence), after a failed full suite, after a task whose tests still fail after two fix attempts, after a review finding that is still not fixed after two attempts, or when an executor reports that the design cannot be followed or needs a new dependency. For the final wave, when its full suite passes, step 8 takes this step's place; at spec cadence, write that wave's report first, without waiting, as for any other wave, so that every wave's report is in the conversation for step 8. Write the report, in this shape and order, as the last thing in the message with nothing after it, then wait for the author; at spec cadence, after a wave whose full suite passed, do not wait, and start the next wave at once: ``` ## Done @@ -146,6 +160,7 @@ Hand back here at the end of a cadence unit (after a task at task cadence, after - ``: , per test command and ephemeral check run - Full suite `` at end of wave : , when a wave ended - Wave review of wave : ; carried: , when a wave was reviewed +- Final review of the run: ; carried: , in the final report at spec cadence ## Files changed - ## Skipped / follow-ups @@ -154,6 +169,8 @@ Hand back here at the end of a cadence unit (after a task at task cadence, after - , or the proposal below ``` +At spec cadence each report covers its own wave, and the reports stack up in the conversation, one per wave, for the final review and the final report of step 8 to read back. A hand-back that waits, for a failure, an unfixed finding or a proposal, covers the wave it stops in, with the earlier waves' reports already above it. + Add to the report, depending on why you handed back: - **A failing full suite or a task that will not pass**: the failing output verbatim, then which causes are confirmed and which are open. Ask the author how to proceed. Do not start the next wave. @@ -164,7 +181,7 @@ A problem found outside the current task is noted under follow-ups and left alon ## 7. Continue -When the author says to continue, resume at step 5 with the same cadence and the optional-test choices already made for their waves: the next unticked task in the current wave, or the next wave if this one is done, where the optional-test question is asked afresh if it has one. +When the author says to continue, resume at step 5 with the same cadence and the same optional-tests choice: the next unticked task in the current wave, or the next wave if this one is done, whose optional tests are asked about at its start only when the author chose to be asked and the cadence is not spec. At spec cadence, when the hand-back came from the final review or the end-of-run optional tests of step 8, resume there. When the author approves a spec edit, apply it to the named file exactly as approved, and resume where the run handed back: at the task that raised it, or at step 3 if the edit was a graph fix made before any work. @@ -172,4 +189,19 @@ When the author asks to skip a non-optional task or to work out of wave order, s ## 8. Finish -When every leaf task is ticked, apart from skipped tests and any non-optional task the author chose to skip under step 7, and the final wave's full suite passes, write the report for the final wave with its full suite result in place of step 6, note any skipped tests, and recommend `/freethinking:pr` to open the pull request. Then stop. +When every leaf task is ticked, apart from skipped tests and any non-optional task the author chose to skip under step 7, and the final wave's full suite passes, finish. + +At task or wave cadence, write the report for the final wave with its full suite result in place of step 6, note any skipped tests, and recommend `/freethinking:pr` to open the pull request. Then stop. + +At spec cadence, finish in three parts: + +1. **Optional tests.** When the author chose to be asked and the run skipped optional tests, offer them once, all together, with the AskUserQuestion tool: do them now, or leave them. If the author says do them, work them as one final set as in step 5: an executor each, launched together, their checks run here, their own wave review, their ticks, and the full suite. A failure there goes to step 6 as for any wave. Left, they stay unticked as skipped tests. When the author chose always or never, there is nothing to offer. +2. **Final review.** Launch the reviewer once more, with `subagent_type: freethinking:wave-reviewer`, over the whole run, and a prompt that holds: + - the sentence, verbatim, "These changes come from a spec wave", which tells the reviewer that a code task correctly lacks a test; + - the full list of files the run changed or created, taken from every wave's Files changed; + - the wave reports of the run as written, each wave's Done, Tests, Wave review line and follow-ups; + - the requirement and design excerpts the run's tasks cite, quoted; + - the files that cite the changed ones, found as for a wave review. + + Nothing more, and never the diff, which the reviewer gathers itself. Its job is the whole: consistency across waves, nits and follow-ups carried from wave to wave that have piled up, and anything one wave's review could not see. Handle its Findings and `unsure` items by the rules of "Reviewing the wave" in step 5: a `blocker`, a `should-fix` or an `unsure` item that is a defect is fixed by an executor within two attempts, with the owning task's checks re-run here; a `nit` is fixed or carried; a finding in the spec goes to step 6 as a proposal; and a finding still not fixed after two attempts goes to step 6. If anything was fixed, run the full suite once more; a failure goes to step 6. There is no second final review. +3. **Final report.** Write the report in the shape of step 6, covering the whole run: every task ticked in it, a Full suite line and a Wave review line for each wave, the Final review line, and the files the run changed. Note any skipped tests and recommend `/freethinking:pr` to open the pull request. Then stop. diff --git a/skills/start-task/SKILL.md b/skills/start-task/SKILL.md index 7b96556..c461efe 100644 --- a/skills/start-task/SKILL.md +++ b/skills/start-task/SKILL.md @@ -125,4 +125,4 @@ Then show the author the document and ask with the AskUserQuestion tool, header ## 7. Hand off -When `tasks.md` is approved, tell the author where the three files are and how to proceed: run `/freethinking:implement-spec `, which builds each task in its own sub-agent in wave order, ticks each checkbox when its tests pass, runs the full suite at the end of each wave, and stops for review after each task or each wave, as the author chooses. Point at the first wave. Then stop. +When `tasks.md` is approved, tell the author where the three files are and how to proceed: run `/freethinking:implement-spec `. Point at the first wave. Then stop.