[#16] implement-spec: CX block, spec cadence and final review

.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"

.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 #<number>`.
 
 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.

.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

README.md

@@ -40,7 +40,7 @@ claude plugin update freethinking@freethinkingit
 | Skill | Run it with | What it does |
 | ----- | ----------- | ------------ |
 | `start-task` | `/freethinking:start-task <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/<task>/` 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 <ticket>`, 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:
 

agents/wave-reviewer.md

@@ -1,20 +1,20 @@
 ---
 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
 ---
 
 # 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())' <path>`. 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 -- <path>`, 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/<default> HEAD`; when there is no `origin`, it is `git merge-base <default> 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 <base> -- <path>`, 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: <the commit the diffs were taken from, and how it was found: merge base with origin/<default>, merge base with <default>, HEAD, or the launch's range>
 findings:
 - file: <path>
   quote: <exact text>
@@ -39,4 +40,4 @@ unsure:
 - <anything the reviewer could not settle, or "nothing">
 ```
 
-`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`.

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()

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 <name>` 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