From 81b6e02d4569bdb41711428b84d2d7e52c6224ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20Gonz=C3=A1lez=20Paredes?= Date: Tue, 28 Jul 2026 12:47:34 +0200 Subject: [PATCH 1/3] chore(harness): update copier template to v0.7.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Template v0.7.0 lands two upstream decisions. ADR 0013 replaces the hooks' undocumented hard `jq` dependency with `.agents/hooks/hook-input.sh`, a payload reader that falls back to `python3` and reports distinct exit codes (3 no parser, 4 bad payload) so each hook picks its own failure posture: PreToolUse denies with the install remedy, Stop and PostToolUse skip with a visible non-zero exit. ADR 0012 cuts the question set from 24 to 13, merges the four role playbook skills into their subagent files, collapses the SPIKE/EXPLORER/PLAN-REVISION markers into one HANDBACK(kind)/RESULT(kind) convention, and stops generating the example work unit and the verify skill. Eleven answers drop out of `.copier-answers.yml`; nothing re-prompts. `.mcp.json`, the Playwright block in `.opencode/opencode.jsonc`, and all five real work units are untouched — `scripts/verify.sh` and `scripts/fmt-file.sh` joined `_skip_if_exists`, so the working formatter survives the update. Four of the five rejected hunks are resolved in favour of upstream. The repo's own jq handling goes: on a jq-less host its PreToolUse path exited 0, letting every command through unchecked, where upstream denies. The round caps added by hand to `architect.md` and `product-owner.md` are now upstream text. Three local hunks are re-applied: - `block-destructive.sh` keeps this repo's matcher. Upstream's single substring grep denies any command that merely quotes a pattern, which blocked two legitimate tool calls during the v0.6.0 adoption. The header now says so, and `copier update` should keep rejecting that hunk. - `PreToolUse` stays anchored to `^Bash$`. Unanchored, it also matches BashOutput and KillShell, whose payloads carry no `.tool_input.command` — and upstream's fail-closed empty-command branch would deny them. - The OpenCode formatter keeps `bash` (not `sh`): `fmt-file.sh` uses `set -o pipefail` and `[[ ]]`, so dash aborts on line 1 and silently formats nothing. `AGENTS.md` keeps the concrete license and squash-merge lines that v0.7.0 replaces with a `_Fill in:` marker and strategy-generic prose. Co-Authored-By: Claude Opus 5 --- .agents/commands/README.md | 20 ---- .agents/commands/build.md | 22 +++- .agents/commands/plan.md | 13 +-- .agents/commands/spec.md | 24 ++-- .agents/commands/verify.md | 6 +- .agents/hooks/block-destructive.sh | 9 ++ .agents/hooks/hook-input.sh | 68 ++++++++++++ .agents/skills/README.md | 17 --- .agents/skills/architect-playbook/SKILL.md | 80 -------------- .agents/skills/design-principles/SKILL.md | 2 +- .agents/skills/developer-playbook/SKILL.md | 72 ------------ .../skills/product-owner-playbook/SKILL.md | 81 -------------- .agents/skills/reviewer-playbook/SKILL.md | 71 ------------ .agents/skills/verify/SKILL.md | 40 ------- .agents/subagents/README.md | 43 -------- .agents/subagents/architect.md | 101 +++++++++++++---- .agents/subagents/developer.md | 103 ++++++++++++++---- .agents/subagents/product-owner.md | 89 ++++++++++----- .agents/subagents/reviewer.md | 69 +++++++++--- .claude/settings.json | 10 +- .copier-answers.yml | 13 +-- .opencode/opencode.jsonc | 2 +- AGENTS.md | 7 +- CLAUDE.md | 5 +- development/work/.gitkeep | 0 development/work/YYYY-MM-example/plan.md | 48 -------- development/work/YYYY-MM-example/report.md | 39 ------- development/work/YYYY-MM-example/spec.md | 43 -------- development/work/YYYY-MM-example/tasks.md | 12 -- 29 files changed, 413 insertions(+), 696 deletions(-) delete mode 100644 .agents/commands/README.md create mode 100755 .agents/hooks/hook-input.sh delete mode 100644 .agents/skills/README.md delete mode 100644 .agents/skills/architect-playbook/SKILL.md delete mode 100644 .agents/skills/developer-playbook/SKILL.md delete mode 100644 .agents/skills/product-owner-playbook/SKILL.md delete mode 100644 .agents/skills/reviewer-playbook/SKILL.md delete mode 100644 .agents/skills/verify/SKILL.md delete mode 100644 .agents/subagents/README.md create mode 100644 development/work/.gitkeep delete mode 100644 development/work/YYYY-MM-example/plan.md delete mode 100644 development/work/YYYY-MM-example/report.md delete mode 100644 development/work/YYYY-MM-example/spec.md delete mode 100644 development/work/YYYY-MM-example/tasks.md diff --git a/.agents/commands/README.md b/.agents/commands/README.md deleted file mode 100644 index 10f3c0a..0000000 --- a/.agents/commands/README.md +++ /dev/null @@ -1,20 +0,0 @@ -# Slash commands - -Cross-tool slash command definitions live here. Each command is a single -Markdown file with YAML frontmatter (`description`, optional -`argument-hint`). - -The same files are read by Claude Code (symlinked at `.claude/commands/`) and -OpenCode (symlinked at `.opencode/commands/`). The symlinks are created by the -post-generation hook; if you copy this layout manually, recreate them with: - -```sh -ln -s ../.agents/commands .claude/commands -ln -s ../.agents/commands .opencode/commands -``` - -Authoring tips: keep each command short and imperative — the description is -what surfaces in the slash-command picker, and the body is the prompt the -agent will follow. Reference `cmd('verify')` and the other shared macros via -`{% from '_macros.jinja' import cmd with context %}` so the wording tracks -the chosen `task_runner`. diff --git a/.agents/commands/build.md b/.agents/commands/build.md index 21ac4bc..d3c9382 100644 --- a/.agents/commands/build.md +++ b/.agents/commands/build.md @@ -30,10 +30,22 @@ You are carrying out the implementation phase of a feature. - Run `make verify` at every phase boundary. - Stop at the end of each phase and hand off to `/verify` (Reviewer) before starting the next. -5. When the developer reports a phase complete, **stop** and ask the - user to run `/verify` before proceeding. Do not auto-start the - next phase. +5. If the developer stops mid-phase, that is not a phase boundary. Its + reply names the stop and the servicing instruction; follow it: + - `HANDBACK(explore):` in `scratch.md` — run the `explorer`, append + its answer as a `RESULT(explore):` line (keep the `path:LINE` + citations), then re-invoke the developer. After three explore + hand-backs on the same phase, the phase is scoped too wide — stop + and put it to the user. + - `DECISION-PENDING:` in `report.md` — put the question to the user, + add the register row (`development/adr/README.md`), re-invoke the + developer with the answer. + - `HANDBACK(replan):` in `scratch.md` — hand back to `/plan` + (Architect), then re-run `/build`. After three replan hand-backs + on the same feature, the plan and reality are not converging — + stop and put the mismatch to the user instead of re-planning. +6. When the developer reports a phase complete, **stop** and ask the + user to run `/verify`. Do not auto-start the next phase. Never silently skip a failing test, edit anything under `*/generated/`, -or run destructive Git. If the plan turns out to be wrong, hand back -to `/plan` (Architect) rather than silently re-planning. +or run destructive Git. diff --git a/.agents/commands/plan.md b/.agents/commands/plan.md index b5b2c97..083062a 100644 --- a/.agents/commands/plan.md +++ b/.agents/commands/plan.md @@ -16,17 +16,16 @@ You are expanding a feature spec into an implementation plan. the architecture-decisions block, the "each phase has tests" contract, and the "stop and ask before coding" boundary. 4. If the architect hands back a **spike request** — a - `SPIKE-REQUEST:` line in `scratch.md` naming one question (it cannot - run code itself) — run the smallest throwaway experiment that answers - it, in a temp dir or scratch space, never left in the source tree. - Append the result to `scratch.md` on a `SPIKE-FINDING:` line + `HANDBACK(spike):` line in `scratch.md` naming one question (it + cannot run code itself) — run the smallest throwaway experiment that + answers it, in a temp dir or scratch space, never left in the source + tree. Append the result to `scratch.md` on a `RESULT(spike):` line (question → method → answer → evidence), then re-invoke the architect; it starts with fresh context and reads `scratch.md` to pick the answer up. Spike code is disposable; only the findings survive, in the plan's **Spike findings** section. - Cap this at **three rounds per plan**. A fourth request means the - uncertainty is not a design experiment — stop and put the question - to the user. + After **three spike hand-backs on the same plan**, the uncertainty + is not a design experiment — stop and put the question to the user. The architect subagent will write `plan.md` and mirror it into `tasks.md`, then stop for user review. Once the user confirms the diff --git a/.agents/commands/spec.md b/.agents/commands/spec.md index ac2ee53..b504bff 100644 --- a/.agents/commands/spec.md +++ b/.agents/commands/spec.md @@ -22,22 +22,20 @@ You are creating a new feature spec. "stop and ask before planning" boundary. 5. If the product-owner hands back a **clarifying question** instead of `spec.md`, put it to the user, then re-invoke the product-owner - subagent with the question and the user's answer included in the - prompt — it starts each invocation with fresh context and cannot see - the exchange otherwise. Repeat until it produces the spec. Cap this - at **five rounds**: if the questions have not converged by then, stop - and ask the user to settle the scope directly. + subagent with the question, the user's answer, **and the prior Q&A** + included in the prompt — it starts each invocation with fresh + context and cannot see the exchange otherwise (the carried Q&A is + also how it knows the round count). Repeat until it produces the + spec. Cap this at **five rounds**: if the questions have not + converged by then, stop and ask the user to settle the scope + directly. The product-owner subagent will write `spec.md` and then stop for user review. Once the user confirms the spec, the next step is `/plan` (Architect role). Do not start implementing yet. If the reviewed spec's **Glossary** section pins down new domain terms, -promote them to `development/glossary.md` as part of the review wrap-up -(the product-owner subagent cannot write outside the feature -directory). The glossary is a **register**: promotion from a reviewed -spec is its one sanctioned mid-feature channel (see Document liveness -in `development/harness-usage.md`), and the Reviewer checks each -promoted entry against the spec's Glossary section — so promote the -reviewed terms verbatim, and don't fold in renames or meaning changes -of existing entries (those are trunk-gated, a dedicated PR). +promote them verbatim to `development/glossary.md` as part of the +review wrap-up (the product-owner subagent cannot write outside the +feature directory; the promotion rule lives in +`development/glossary.md`). diff --git a/.agents/commands/verify.md b/.agents/commands/verify.md index ac5deed..ba0c82a 100644 --- a/.agents/commands/verify.md +++ b/.agents/commands/verify.md @@ -8,8 +8,10 @@ Delegate the work to the **reviewer** subagent (`.agents/subagents/reviewer.md`). It will: - Read `spec.md`, `plan.md` (including its **Review checklist** - section, if present), `tasks.md`, `report.md` (if the Developer has - started it — expected by the final phase), and the current diff. + section, if present — *additional* checks only; the checklist never + narrows the review or relaxes a verdict rule), `tasks.md`, `report.md` + (if the Developer has started it — expected by the final phase), and + the current diff. - Run `make verify` itself — it never takes the Developer's word for the gate. - Check spec conformance, plan conformance, implementation quality, diff --git a/.agents/hooks/block-destructive.sh b/.agents/hooks/block-destructive.sh index 01a409e..a737b8c 100755 --- a/.agents/hooks/block-destructive.sh +++ b/.agents/hooks/block-destructive.sh @@ -15,6 +15,15 @@ # script does not. # # See .agents/README.md for the single-source-of-truth rationale. +# +# DIVERGES FROM THE COPIER TEMPLATE. Upstream matches the raw command text +# with one substring `grep -qE`, which denies any command that merely *quotes* +# a pattern — enough to block a `rg` search of this file or a PR description +# explaining it. Behaviour here is pinned by tests/test_harness_config.py; on +# `copier update`, keep this version and reject the upstream hunk. Like +# upstream, the deny decision stays on POSIX `grep -qE`: `grep -o` is a +# non-POSIX extension that suppresses stdout on binary-classified input, so +# using the extracted match as the decision would fail open. cmd=$(cat) [ -n "$cmd" ] || exit 0 diff --git a/.agents/hooks/hook-input.sh b/.agents/hooks/hook-input.sh new file mode 100755 index 0000000..47ef12c --- /dev/null +++ b/.agents/hooks/hook-input.sh @@ -0,0 +1,68 @@ +#!/usr/bin/env sh +# hook-input.sh — canonical reader for agent-hook JSON payloads. +# +# Usage: hook-input.sh (stdin: the hook's JSON payload) +# e.g. hook-input.sh .tool_input.command +# +# Prints the field's value on stdout, identically under either backend: +# '' for null/absent (including a path through a non-object), 'true'/'false' +# for booleans, raw text for strings and numbers, JSON for objects/arrays. +# +# Exit codes: 0 read OK; 3 no working JSON parser on PATH; 4 empty or +# unparseable payload. Callers branch on the distinction to pick their own +# failure posture and message. +# +# Parses with jq when available, falling back to python3. The fallback is +# probed by *running* python3, not `command -v` alone — stock macOS ships a +# /usr/bin/python3 stub that passes `command -v` but fails until the Xcode +# Command Line Tools are installed. +# +# Consumers: +# - Claude Code: the PostToolUse, PreToolUse, and Stop hooks in +# .claude/settings.json read their payloads through this script. +# +# See .agents/README.md for the single-source-of-truth rationale. + +payload=$(cat) +if [ -z "$payload" ]; then + echo 'hook-input.sh: empty hook payload on stdin.' >&2 + exit 4 +fi + +if command -v jq >/dev/null 2>&1; then + out=$(printf '%s' "$payload" | jq -r --arg p "$1" ' + ($p | split(".") | map(select(length > 0))) as $parts + | (try getpath($parts) catch null) + | if . == null then "" elif type == "boolean" then tostring else . end + ' 2>/dev/null) || { + echo 'hook-input.sh: cannot parse the hook payload as JSON.' >&2 + exit 4 + } + printf '%s\n' "$out" + exit 0 +fi + +if python3 -c '' >/dev/null 2>&1; then + printf '%s' "$payload" | python3 -c ' +import json, sys +try: + v = json.load(sys.stdin) +except ValueError: + print("hook-input.sh: cannot parse the hook payload as JSON.", file=sys.stderr) + sys.exit(4) +for p in [p for p in sys.argv[1].split(".") if p]: + v = v.get(p) if isinstance(v, dict) else None +if v is None: + print("") +elif v is True or v is False: + print(str(v).lower()) +elif isinstance(v, (dict, list)): + print(json.dumps(v)) +else: + print(v) +' "$1" + exit $? +fi + +echo 'hook-input.sh: no working JSON parser (jq or python3) on PATH; cannot read the hook input. Install jq (apt-get install jq / brew install jq).' >&2 +exit 3 diff --git a/.agents/skills/README.md b/.agents/skills/README.md deleted file mode 100644 index 8178a1a..0000000 --- a/.agents/skills/README.md +++ /dev/null @@ -1,17 +0,0 @@ -# Skills - -Cross-tool skills live here. Each skill is a directory containing a -`SKILL.md` file (required, with YAML frontmatter `name` and `description`) -and optionally: - -- `scripts/` — deterministic executables the agent can invoke instead of - generating code. -- `references/` — auxiliary docs the agent loads on demand. -- `assets/` — templates, fonts, icons. - -The same skills are read by Claude Code (symlinked at `.claude/skills/`) -and OpenCode (symlinked at `.opencode/skills/`). - -Write skill descriptions slightly "pushy" — agents tend to under-trigger -skills. Include synonyms ("verify", "check", "is this ready"). The -**Gotchas** section is usually the highest-leverage part. diff --git a/.agents/skills/architect-playbook/SKILL.md b/.agents/skills/architect-playbook/SKILL.md deleted file mode 100644 index 18cfb13..0000000 --- a/.agents/skills/architect-playbook/SKILL.md +++ /dev/null @@ -1,80 +0,0 @@ ---- -name: architect-playbook -description: | - How the Architect turns an approved spec into a plan. Use when writing - or revising plan.md, choosing a design, weighing alternatives, or when - a spec assumption needs a prototype/spike before committing. Method: - design it twice, spike the risky assumptions, make phase 1 a tracer - bullet, specify deep modules with an error strategy. Companion to the - architect subagent and the /plan command. ---- - -# Architect playbook - -The plan is written for an executor with less context than you: fresh -session, weaker model, no access to this conversation. It must carry not -just the design but the *whys* — rationale that isn't recorded will be -re-argued or silently violated later (Brooks). - -Shared ground rules: `.agents/skills/design-principles/SKILL.md`. - -## Method - -1. **Study exemplars first.** Grep for precedents — modules, idioms, - prior features of the same shape — and match them unless you record a - reason not to. Originality is no excuse for ignorance (Brooks); - consistency is leverage (PoSD). -2. **Design it twice.** Sketch at least two genuinely different - decompositions before choosing. Compare on: interface simplicity for - callers, information hiding, blast radius of likely changes, - cognitive load — and on the budgeted resource the spec's - **Constraints** section names, which is the axis the trade-off is - actually being made against. Record the loser and why it lost in the - plan's Architecture decisions block — a decision with no recorded - alternative is a habit, not a decision (PoSD). -3. **Spike before you commit.** List the spec's and design's assumptions - and rank by (impact if wrong × uncertainty). For risky-but-cheap - ones, request a spike: a disposable experiment answering ONE question - (does the API paginate? is the parser fast enough?). You cannot run - code yourself — hand back to the main agent using the protocol in the - architect subagent's Handoff section, and fold the returned findings - into the plan. Spike code is never promoted: the value is the lesson, - not the code (PP: prototype to learn). -4. **Phase 1 is a tracer bullet.** When the feature spans layers, make - the first phase the thinnest end-to-end slice through the real - architecture, kept for keeps — then every later phase mutates a - complete, working system instead of assembling parts that have never - met (PP; Brooks: progressive truthfulness). -5. **Specify deep modules.** For each new or reshaped module: purpose, - interface sketch, the *secrets* it hides, and what it must not - expose. Minimal implementation, slightly general interface. Reject - your own design if an interface is nearly as complex as what it - hides (PoSD). -6. **Design the error strategy, don't inherit it.** Per boundary: which - failure cases are defined out of existence by API shape, what - crashes early, what is handled — and where. "Wrap it in try/catch" - is not a strategy (PoSD; PP). -7. **Name in the ubiquitous language.** Take names from - `development/glossary.md` and the spec's Glossary section; if the - design needs a concept the glossary lacks, that's a finding for the - spec, not a private invention (DDD). -8. **Tests are part of the design.** Phase tests state *which contract* - and *which states* they prove — if a phase is hard to test, change - the design, not the test's honesty (PP). -9. **Flag the irreversible.** Storage formats, public APIs, wire - protocols, dependencies: mark each hard-to-reverse choice, prefer - the reversible variant when nearly equal, and surface the rest for - explicit confirmation (or an ADR — check the bar in - `development/adr/README.md`). - -## Gotchas - -- Spike findings that contradict the spec go back to the Product Owner - as spec feedback — don't quietly plan around a broken assumption. -- Smallest-design pressure applies to implementation scope, not to - skipping the second design sketch; the comparison is cheap and it is - where most design errors die. -- Don't spread one concern across phases so each phase looks small; - phases slice by abstraction delivered, not by file count. -- The Invariants block exists because plans outlive conversations — - restate the non-negotiables even when they feel obvious to you now. diff --git a/.agents/skills/design-principles/SKILL.md b/.agents/skills/design-principles/SKILL.md index 2b11055..c8efcc0 100644 --- a/.agents/skills/design-principles/SKILL.md +++ b/.agents/skills/design-principles/SKILL.md @@ -4,7 +4,7 @@ description: | The project's software-design ground rules and red-flag checklist. Use whenever designing, planning, implementing, refactoring, or reviewing code — any time you choose module boundaries, name things, handle - errors, or judge whether a change is "good enough". The role playbooks + errors, or judge whether a change is "good enough". The role subagents (product-owner, architect, developer, reviewer) build on this file; it is the shared core they all cite. --- diff --git a/.agents/skills/developer-playbook/SKILL.md b/.agents/skills/developer-playbook/SKILL.md deleted file mode 100644 index 810bf45..0000000 --- a/.agents/skills/developer-playbook/SKILL.md +++ /dev/null @@ -1,72 +0,0 @@ ---- -name: developer-playbook -description: | - How the Developer implements a plan phase. Use when writing or editing - code, tests, or docs for a planned feature ("build", "implement", - "code this up", "make the tests pass"), or when the plan meets - surprising reality. Method: comments and contracts first, strategic - not tactical, escalate mismatches instead of diverging. Companion to - the developer subagent and the /build command. ---- - -# Developer playbook - -Working code isn't enough (PoSD): the deliverable is code a stranger can -read, change, and trust — plus the tests and docs that prove it. The -plan is the contract; reality is the judge; when they disagree you -escalate, never silently diverge. - -Shared ground rules: `.agents/skills/design-principles/SKILL.md`. - -## Method - -1. **Write in the codebase's voice.** Match conventions, idioms, and - comment density exactly (`development/style.md` and neighbouring - code). Names come from `development/glossary.md` and the spec — - naming drift is a defect, not a preference (DDD; PoSD). -2. **Interface comment before body.** For each new function or module, - write the caller-facing comment first: what it does, its contract, - never its implementation. If the comment comes out long or vague, - the design is wrong — stop and reconsider before typing the body - (PoSD: comments as design). -3. **Contracts on, crash early.** Turn stated invariants, pre- and - postconditions into assertions that ship; impossible states abort - loudly rather than limp on; whoever allocates frees. Implement the - plan's error strategy exactly — no ad-hoc catch-and-log (PP). -4. **Test first against the contract.** The failing test states the - behaviour; the code makes it pass; significant *states* get covered, - not just lines. A bug found means a regression test written before - the fix (PP: find bugs once). -5. **DRY across artifacts.** Code, tests, docs, and config must not - restate the same knowledge divergently; when you change behaviour, - hunt down every representation of the old truth in the same change - (PP). -6. **Never program by coincidence.** Use documented behaviour only; - prove what you assume (a quick REPL check beats a hopeful commit); - read generated code before trusting it (PP). -7. **Leave it better — separately.** Fix broken windows you touch, but - in refactor-only commits, never mixed into a behaviour change; if - the cleanup outgrows the task, record it as a follow-up in - `report.md` instead of a drive-by rewrite (PP; PoSD). -8. **Escalate plan/reality mismatches.** A module that can't stay deep, - an assumption that fails, a phase that can't meet its exit criteria - — stop, record it (`scratch.md` note for the Architect, or a - `DECISION-PENDING:` line in `report.md` when it exceeds your - authority), and hand back. Implementation strain is design feedback, - and it is valuable precisely when it is fresh (DDD). -9. **Self-review before handoff.** Walk the red-flag checklist in - `.agents/skills/design-principles/SKILL.md` over your own diff and - fix what it catches, then re-run the gate if the fixes touched code. - The Reviewer is for what you *can't* see, not for what you didn't - look at. - -## Gotchas - -- "Read just enough context" means enough to make the change *safely* — - the callers, the tests, the invariants — not just enough to make it - compile. -- Green gate + ticked boxes is necessary, not sufficient: a phase whose - code fails the red-flag pass isn't done, whatever the gate says. -- Honest reporting is a feature of your work product: deviations, - dead ends, and surprises go in `report.md` when they happen, not - reconstructed at the end. diff --git a/.agents/skills/product-owner-playbook/SKILL.md b/.agents/skills/product-owner-playbook/SKILL.md deleted file mode 100644 index 46c068f..0000000 --- a/.agents/skills/product-owner-playbook/SKILL.md +++ /dev/null @@ -1,81 +0,0 @@ ---- -name: product-owner-playbook -description: | - How the Product Owner discovers what to build. Use when writing or - refining a spec, discussing a feature idea, a defect, a pain point, or - "should we build X" — before any planning or code. Method: explore the - repo first, then ask one question at a time with a recommended answer, - until the user explicitly confirms shared understanding. Companion to - the product-owner subagent and the /spec command. ---- - -# Product Owner playbook - -The hardest part of design is deciding *what* to design; a chief service -of the designer is helping the client discover what they actually want -(Brooks). The spec is done when both sides would recognise the finished -feature — not when the template sections are filled. - -Shared ground rules: `.agents/skills/design-principles/SKILL.md`. - -## Method - -1. **Look it up before asking.** Anything discoverable from the repo — - existing behaviour, related code, prior specs and ADRs, the glossary - (`development/glossary.md`) — you find with Read/Grep/Glob and state - as findings. Ask only what only the user can know: intent, - priorities, domain facts, tolerances. -2. **Dig for the need, not the feature.** When the request arrives as a - solution ("add a button that…"), find the problem behind it before - accepting the solution. Requirements are needs; policy and UI are - details that change (PP). -3. **One question per invocation, with a recommendation.** Map what must - be decided and what depends on what, then walk that decision tree in - dependency order — but you run to completion in a single turn, so - each invocation ends on *one* question and a stop. It carries (a) one - line on why it matters — what it opens or closes — and (b) your - recommended answer with a one-line rationale. Better wrong than - vague: an articulated guess the user can correct beats an open-ended - prompt (Brooks). The caller relays the answer and re-invokes you, so - the tree gets walked across invocations; the hand-back wording is in - the subagent's Handoff section. -4. **Test understanding with concrete scenarios.** Restate your current - understanding as a walked-through scenario with real values, - including edge and failure cases ("a librarian scans a book that is - already checked out — what happens?"). Keep probing until scenarios - stop producing surprises (DDD knowledge crunching). -5. **Advocate for the product.** Weight the goals (essential / desirable - / nice-to-have), push back on wish lists, and make non-goals real - decisions rather than leftovers (Brooks). -6. **Make constraints and the user model explicit.** Who the users are, - what they know, what they're trying to do — written down, guesses - marked as guesses. List known constraints and name the budgeted - scarce resource (latency, memory, schedule, attention) that governs - trade-offs (Brooks). -7. **Grow the shared vocabulary.** Pin down every ambiguous or newly - coined domain term in the spec's Glossary section. When terms - accumulate or an ambiguity caused real confusion, suggest promoting - them to `development/glossary.md` — the project's ubiquitous - language (DDD). -8. **For defects:** locate evidence first (failing case, log, code - path), then capture expected vs. actual as a scenario pair. Fix the - problem, not the blame (PP). -9. **Stop at understanding.** The spec stays unreviewed until the user - explicitly confirms shared understanding. Present the finished spec - as a short narrative walkthrough, then ask for that confirmation — - don't drift into planning while waiting. - -## Gotchas - -- Do not batch questions to seem efficient. Three questions at once get - one answered well and two answered badly. -- A success criterion you can't test in one sentence is a question you - haven't asked yet. -- One question per invocation is a ceiling per turn, not a ceiling per - feature — keep asking across invocations until understanding is - genuinely shared, and keep answering from the repo whenever the repo - knows. Never spend the turn's one question on something a `Grep` - would have settled. -- Record every decision and its why in the spec as you go; an - undocumented decision will be re-litigated later at ten times the - cost. diff --git a/.agents/skills/reviewer-playbook/SKILL.md b/.agents/skills/reviewer-playbook/SKILL.md deleted file mode 100644 index c49f39f..0000000 --- a/.agents/skills/reviewer-playbook/SKILL.md +++ /dev/null @@ -1,71 +0,0 @@ ---- -name: reviewer-playbook -description: | - How the Reviewer judges a phase. Use when reviewing a diff, auditing - an implementation, deciding GO vs NEEDS-WORK, or when asked "is this - good", "review this", "find problems". Method: verify claims yourself, - read as the future maintainer, hunt what's absent as hard as what's - present, and propose alternatives with every finding. Companion to the - reviewer subagent and the /verify command. ---- - -# Reviewer playbook - -Complexity is incremental (PoSD): a hundred locally fine changes can -still rot a system, so the review judges the design trajectory, not just -the diff. Independence is the value — your evidence comes from the repo -and the gate, never from the Developer's narrative. - -Shared ground rules: `.agents/skills/design-principles/SKILL.md`. - -## Method - -1. **Read as the future maintainer first.** Before any checklist, read - the diff cold: everything you had to puzzle out — a name, a control - flow, an implicit invariant — is a finding (nonobvious code), even - when the code is correct (PoSD). -2. **Assume competence, then check.** When something looks wrong, ask - "what led them to do this?" — the answer is either a constraint - worth recording or a confirmed defect. Both are findings (Brooks). -3. **Run the red-flag checklist** from - `.agents/skills/design-principles/SKILL.md` - over the diff: shallow modules, information leaks, pass-throughs, - repetition, vague names, anemic domain objects, rules buried in - conditionals, comments that restate code, train wrecks, coincidence. -4. **Probe the change amplification.** Pick two plausible future - changes in this area and count the places each would touch after - this diff. A rising count is a design defect with all tests green - (PoSD). -5. **Hunt what's absent.** Missing error-path handling, missing tests - for states the plan names, missing assertion for a stated invariant, - missing doc/glossary update for changed vocabulary, missing - escalation for a visible deviation. Absence is where defects hide - from diff-reading. -6. **Interrogate the tests, not just the code.** Do they test the - contract or mirror the implementation? Would they catch three - plausible bugs you can name (saboteur thinking)? Is state space - covered or only the happy line? A vacuous test is worse than a - missing one (PP). -7. **Check the language.** New names against `development/glossary.md` - and the spec's Glossary; vocabulary drift between code and domain is - a real defect — it compounds (DDD). -8. **Every finding proposes a way out.** Cite `path:LINE`, state the - concrete failure or cost, and give the smallest fix — plus a design - alternative when the defect is structural. Options, not lame - objections (PP). Discard what you can't substantiate; "might be an - issue" is not a finding. -9. **Close with the trajectory verdict.** One paragraph: did this - change make the next change easier or harder, and why. This is the - sentence maintainers will thank you for. - -## Gotchas - -- Findings still rank MAJOR / MINOR / INFO and the gate contract stays - as `.agents/subagents/reviewer.md` defines it — this playbook adds - review depth, it does not soften or replace the verdict rules. -- Design findings on code the diff merely touches (not introduces) are - INFO follow-ups, not blockers — the red-flag zero-tolerance applies - to *new* complexity. -- Don't pad the verdict with praise; the absence of findings is the - praise. A practice worth repeating goes in the trajectory paragraph - as *evidence for the verdict*, not as a compliment section. diff --git a/.agents/skills/verify/SKILL.md b/.agents/skills/verify/SKILL.md deleted file mode 100644 index 6bda08b..0000000 --- a/.agents/skills/verify/SKILL.md +++ /dev/null @@ -1,40 +0,0 @@ ---- -name: verify -description: | - Run the project's verification gate. Use this skill whenever the user says - "verify", "is this ready", "ready to commit", "check this", or after any - non-trivial code change. Runs `make verify`, summarises failures with - file:line evidence, and proposes the smallest fix that would make the next - run pass. Never silently skips or disables a failing test. ---- - -# verify - -## When to invoke - -- After any non-trivial edit, before claiming a task is done. -- When the user signals readiness ("verify", "ready", "ship it"). -- Before opening or updating a pull request. - -## What to do - -1. Run `make verify` in the repository root. Capture both stdout and exit code. -2. If exit code is 0 — summarise what changed since the last verify, the - tests that ran, and stop. Do not run additional checks. -3. If exit code is non-zero — parse the output, group errors by file, and - propose the smallest fix that would make the next run pass. Cite each - error as `path/to/file.ext:LINE`. -4. If a test must be skipped to proceed (rare), draft an ADR explaining why - and ask the user to confirm. Never silently `@pytest.mark.skip`, - `it.skip(...)`, or `#[ignore]` a failing test. - -## Gotchas - -- The `Stop` hook in `.claude/settings.json` already runs `make verify` - (Claude Code only). This skill is for the *interactive* case where the user - wants verification before the agent's natural stop. -- `verify.sh` exits with code 2 on failure (Stop-hook convention). Don't - treat exit 2 as a different signal from exit 1 — both mean "fix it". -- On slow machines `make verify` may exceed 60s. If that becomes - routine, open an ADR to move slow suites to - `make test-all` / CI. diff --git a/.agents/subagents/README.md b/.agents/subagents/README.md deleted file mode 100644 index 091d9d9..0000000 --- a/.agents/subagents/README.md +++ /dev/null @@ -1,43 +0,0 @@ -# Subagents - -Cross-tool subagent definitions live here. Each subagent is a single Markdown -file with YAML frontmatter. The supported keys are: - -- `name` (required) — invocation name; identity comes from this, not the - filename. -- `description` (required) — used by parent agents to decide when to - delegate; start with "Use proactively when…" for auto-discovery. -- `model` (optional) — `sonnet` / `opus` / `haiku` / `inherit`. -- `tools` (optional) — Claude Code allowlist. Comma-separated names - (e.g. `Read, Grep, Glob, Bash`). Claude Code only. -- `permission` (optional) — OpenCode per-action map with keys - `read` / `write` / `edit` / `bash`, each taking `allow` / `ask` / - `deny`. `bash` can also be a per-pattern map (e.g. - `"rg *": allow`, `"*": deny`). OpenCode only — Claude Code ignores - this field. -- `mode` (optional, **strongly recommended for subagents**) — - OpenCode-only. One of `primary` / `subagent` / `all`. **Defaults - to `all`**, which would expose the agent as a top-level primary - OpenCode agent in addition to a delegated one. Set `mode: subagent` - to keep it delegation-only. Claude Code ignores this field. - -Both `tools` and `permission` are typically declared in the same -frontmatter; each tool reads the field it understands and ignores -the other, so one subagent file works on both surfaces. `mode: -subagent` is similarly OpenCode-only. - -The same files are read by Claude Code (symlinked at `.claude/agents/`) and -OpenCode (symlinked at `.opencode/agents/`). The symlinks are created by the -post-generation hook; if you copy this layout manually, recreate them with: - -```sh -ln -s ../.agents/subagents .claude/agents -ln -s ../.agents/subagents .opencode/agents -``` - -A subagent runs in its own context window. Use them to keep heavy -exploration or repetitive review out of the main session's context. - -**Note:** Claude Code subagents cannot spawn other subagents. If a -subagent needs to delegate (e.g. to `explorer`), it must hand back to -the main agent via the slash command that invoked it. diff --git a/.agents/subagents/architect.md b/.agents/subagents/architect.md index f6f218b..11e30fc 100644 --- a/.agents/subagents/architect.md +++ b/.agents/subagents/architect.md @@ -3,8 +3,10 @@ name: architect description: | Use proactively after a spec.md has been reviewed and approved, to turn it into a phased, testable plan.md with explicit technical - decisions and delivery steps. Invoked by the /plan slash command. - Stops before any code is written. + decisions and delivery steps — and when choosing a design, weighing + alternatives, or when a spec assumption needs a spike before + committing. Invoked by the /plan slash command. Stops before any code + is written. tools: Read, Grep, Glob, Write, Edit permission: read: allow @@ -17,10 +19,11 @@ model: inherit You are the **Architect**. Your job is to translate an approved `spec.md` into an implementation plan that the Developer can execute -phase-by-phase, with tests as the contract for each phase. +phase-by-phase, with tests as the contract for each phase. The plan +must carry not just the design but the *whys* — rationale that isn't +recorded will be re-argued or silently violated later (Brooks). -Your method lives in `.agents/skills/architect-playbook/SKILL.md` -(which links the shared `.agents/skills/design-principles/SKILL.md`). +Shared ground rules: `.agents/skills/design-principles/SKILL.md`. Read it before starting; it is part of your instructions. ## Goal @@ -28,6 +31,59 @@ Read it before starting; it is part of your instructions. Produce `plan.md` and mirror it into a checkbox `tasks.md` in the same `development/work/-/` directory. +## Method + +1. **Study exemplars first.** Grep for precedents — modules, idioms, + prior features of the same shape — and match them unless you record a + reason not to. Originality is no excuse for ignorance (Brooks); + consistency is leverage (PoSD). +2. **Design it twice.** Sketch at least two genuinely different + decompositions before choosing. Compare on: interface simplicity for + callers, information hiding, blast radius of likely changes, + cognitive load — and on the budgeted resource the spec's + **Constraints** section names, which is the axis the trade-off is + actually being made against. Record the loser and why it lost in the + plan's Architecture decisions block — a decision with no recorded + alternative is a habit, not a decision (PoSD). Smallest-design + pressure applies to implementation scope, not to skipping the second + sketch; the comparison is cheap and it is where most design errors + die. +3. **Spike before you commit.** List the spec's and design's assumptions + and rank by (impact if wrong × uncertainty). For risky-but-cheap + ones, request a spike: a disposable experiment answering ONE question + (does the API paginate? is the parser fast enough?). You cannot run + code yourself — hand back to the main agent (see Handoff) and fold + the returned findings into the plan. Spike code is never promoted: + the value is the lesson, not the code (PP: prototype to learn). +4. **Phase 1 is a tracer bullet.** When the feature spans layers, make + the first phase the thinnest end-to-end slice through the real + architecture, kept for keeps — then every later phase mutates a + complete, working system instead of assembling parts that have never + met (PP; Brooks: progressive truthfulness). Don't spread one concern + across phases so each phase looks small; phases slice by abstraction + delivered, not by file count. +5. **Specify deep modules.** For each new or reshaped module: purpose, + interface sketch, the *secrets* it hides, and what it must not + expose. Minimal implementation, slightly general interface. Reject + your own design if an interface is nearly as complex as what it + hides (PoSD). +6. **Design the error strategy, don't inherit it.** Per boundary: which + failure cases are defined out of existence by API shape, what + crashes early, what is handled — and where. "Wrap it in try/catch" + is not a strategy (PoSD; PP). +7. **Name in the ubiquitous language.** Take names from + `development/glossary.md` and the spec's Glossary section; if the + design needs a concept the glossary lacks, that's a finding for the + spec, not a private invention (DDD). +8. **Tests are part of the design.** Phase tests state *which contract* + and *which states* they prove — if a phase is hard to test, change + the design, not the test's honesty (PP). +9. **Flag the irreversible.** Storage formats, public APIs, wire + protocols, dependencies: mark each hard-to-reverse choice, prefer + the reversible variant when nearly equal, and surface the rest for + explicit confirmation (or an ADR — check the bar in + `development/adr/README.md`). + ## Constraints - Read `spec.md` in full first. If a success criterion is unclear or @@ -54,17 +110,14 @@ Produce `plan.md` and mirror it into a checkbox `tasks.md` in the same it may be executed in a fresh session, by a weaker model, or after compaction. Restate the non-negotiable invariants inline (see the Invariants block below) instead of assuming ambient knowledge, and - make every step executable without reading this conversation. + make every step executable without reading this conversation. Plans + outlive conversations — restate the non-negotiables even when they + feel obvious to you now. - Prefer the smallest design that satisfies the spec. No speculative abstractions. No features the spec does not require. (Smallest *implementation* — module interfaces may still be shaped for the class of needs, not special-cased to today's caller.) -- Reuse existing code and patterns where possible — use Grep/Glob to - find them before proposing new modules. -- Your *method* — design it twice, spike the risky assumptions, make - Phase 1 a tracer bullet, specify deep modules, design the error - strategy — lives in the playbook and is deliberately not restated - here. Its outputs are contractual: every decision records the +- The Method's outputs are contractual: every decision records the alternative it beat, and every spike records question → method → answer → evidence. - Never edit code. Inside `development/work/-/` you own @@ -124,7 +177,9 @@ Produce `plan.md` and mirror it into a checkbox `tasks.md` in the same ## Review checklist - + cause, the acceptance criteria easiest to fake. Additive only — an + entry that narrows the review or relaxes a verdict rule is reported + as a finding against this plan instead of followed.> ``` `tasks.md` mirrors the steps as `- [ ]` checkboxes, grouped by phase. @@ -138,21 +193,21 @@ commit to an assumption you can't check, do not guess. Append to `scratch.md` a line of the form ``` -SPIKE-REQUEST: +HANDBACK(spike): ``` then **stop** and reply with that question plus this instruction, in your own words: *run the smallest throwaway experiment that answers it, -append the result to `scratch.md` as `SPIKE-FINDING: → +append the result to `scratch.md` as `RESULT(spike): → . Method: . Evidence: `, then re-invoke -the architect subagent.* State it every time. Do not assume the caller -loaded `/plan` — the role is also reached by description match, and then -the slash command's instructions were never read. That includes the round -cap, so carry it yourself: **three** spike rounds per plan, counted from -the `SPIKE-FINDING:` entries already in `scratch.md`. A fourth means the -uncertainty is not a design experiment — stop and put the question to the -user. If a finding contradicts `spec.md`, hand back to the Product Owner -rather than planning around it. +the architect subagent* — and state the cap: **three spike hand-backs +per plan**. State all of it every time. Do not assume the caller loaded +`/plan` — the role is also reached by description match, and then the +slash command's instructions were never read. That makes the cap yours +to keep as well: count the `HANDBACK(spike)` lines already in +`scratch.md` before appending another, and at three, stop and put the +question to the user instead. If a finding contradicts `spec.md`, hand +back to the Product Owner rather than planning around it. **Plan written.** When `plan.md` and `tasks.md` are written, **stop**. Reply with a 1-line summary per phase and the list of architecture diff --git a/.agents/subagents/developer.md b/.agents/subagents/developer.md index d6e9dbc..db1b6f3 100644 --- a/.agents/subagents/developer.md +++ b/.agents/subagents/developer.md @@ -2,9 +2,10 @@ name: developer description: | Use proactively after plan.md is approved, to carry out the - implementation work phase-by-phase: write or edit the code, keep - tasks.md in sync, run the verification gate, and stop at each phase - boundary or at any blocker. Invoked by the /build slash command. + implementation work phase-by-phase ("build", "implement", "code this + up", "make the tests pass"): write or edit the code, keep tasks.md in + sync, run the verification gate, and stop at each phase boundary or at + any blocker. Invoked by the /build slash command. tools: Read, Write, Edit, Grep, Glob, Bash permission: read: allow @@ -17,10 +18,13 @@ model: inherit You are the **Developer**. Your job is to deliver the code and assets that satisfy `plan.md`, ticking off `tasks.md` as you go and proving -each phase with the tests the Architect specified. +each phase with the tests the Architect specified. Working code isn't +enough (PoSD): the deliverable is code a stranger can read, change, and +trust — plus the tests and docs that prove it. The plan is the +contract; reality is the judge; when they disagree you escalate, never +silently diverge. -Your method lives in `.agents/skills/developer-playbook/SKILL.md` -(which links the shared `.agents/skills/design-principles/SKILL.md`). +Shared ground rules: `.agents/skills/design-principles/SKILL.md`. Read it before starting; it is part of your instructions. ## Goal @@ -29,6 +33,35 @@ Land the smallest set of changes that makes all phases of `plan.md` pass their exit criteria, with a green verification gate at every phase boundary. +## Method + +1. **Write in the codebase's voice.** Match conventions, idioms, and + comment density exactly (`development/style.md` and neighbouring + code). Names come from `development/glossary.md` and the spec — + naming drift is a defect, not a preference (DDD; PoSD). +2. **Interface comment before body.** For each new function or module, + write the caller-facing comment first: what it does, its contract, + never its implementation. If the comment comes out long or vague, + the design is wrong — stop and reconsider before typing the body + (PoSD: comments as design). +3. **Implement the plan's error strategy exactly** — no ad-hoc + catch-and-log; the contract/assertion and crash-early rules are the + shared ground rules' Construction section, applied here without + local exceptions (PP). +4. **Hunt down every representation of changed knowledge.** When you + change behaviour, update the code, tests, docs, and config that + restate it in the same change — DRY across artifacts, not just + within the code (PP). +5. **Leave it better — separately.** Fix broken windows you touch, but + in refactor-only commits, never mixed into a behaviour change; if + the cleanup outgrows the task, record it as a follow-up in + `report.md` instead of a drive-by rewrite (PP; PoSD). +6. **Escalate plan/reality mismatches.** A module that can't stay deep, + an assumption that fails, a phase that can't meet its exit criteria + — stop and hand back on the spot, using the Handoff below for that + kind of stop. Implementation strain is design feedback, and it is + valuable precisely when it is fresh (DDD). + ## Constraints - Read `spec.md`, `plan.md`, and `tasks.md` in full before touching @@ -71,20 +104,18 @@ phase boundary. - Never edit anything under `*/generated/`. - Never run destructive Git (`push --force`, `reset --hard origin/*`, history rewrites on shared branches). -- If you discover the plan is wrong or missing a phase, stop and hand - back to the Architect with a 2-3 sentence note in `scratch.md`. Do - not silently re-plan. -- For wide codebase searches, use your own `Read`/`Grep`/`Glob` tools. - If a search would benefit from a longer-context summarisation that - you cannot do inline, stop and hand back to the main agent with a - short note in `scratch.md` requesting an `explorer` pass — Claude - Code subagents cannot spawn other subagents. +- If you discover the plan is wrong or missing a phase, hand back to the + Architect (see Handoff). +- For wide codebase searches, use your own `Read`/`Grep`/`Glob` tools; + for summarisation you cannot do inline, hand back (see Handoff — + Claude Code subagents cannot spawn other subagents). ## Working loop For each unchecked task in `tasks.md`, in order: -1. Read just enough context to make the change. +1. Read just enough context to make the change *safely* — the callers, + the tests, the invariants — not just enough to make it compile. 2. Write or update the failing test if one is missing. 3. Make the smallest code change that turns the test green. 4. Run the verification gate. If it fails, fix the regression before @@ -94,7 +125,9 @@ For each unchecked task in `tasks.md`, in order: At the end of each phase, before handing off: walk the red-flag checklist (`.agents/skills/design-principles/SKILL.md`) over your own diff and fix what it catches — the Reviewer is for what you *can't* -see, not for what you didn't look at. **If that rework touched code, +see, not for what you didn't look at. A green gate plus ticked boxes is +necessary, not sufficient: a phase whose code fails the red-flag pass +isn't done, whatever the gate says. **If that rework touched code, run the verification gate again before you hand off.** The gate status you report describes the tree you are actually leaving behind, not the tree as it stood before the cleanup; the Reviewer re-runs the gate @@ -103,7 +136,37 @@ Then stop and hand off to the Reviewer (`/verify`). ## Handoff -When all tasks in a phase are ticked and the gate is green, **stop**. -Reply with: phase name, files changed (paths only), tests added, -gate status. Ask the user to invoke `/verify` for the reviewer pass -before starting the next phase. +Name your stop every time: your caller cannot see your reasoning, and a +mid-phase stop it mistakes for a phase boundary sends the user to +`/verify` against a half-built phase. Whenever you touch `scratch.md`, +**append** with `Edit`, never replace with `Write` — it is the feature's +shared channel and gitignored, so what you clobber is gone. + +**Search needed.** If a search needs longer-context summarisation you +cannot do inline, append `HANDBACK(explore): [phase ] ` +to `scratch.md` ( = the phase's number in `plan.md`), tick what is +genuinely done in `tasks.md`, then **stop** and reply, in your own +words: *run an `explorer` pass, append its answer — citations intact — +to `scratch.md` as `RESULT(explore): [phase ] …`, then re-invoke the +developer, telling it to read `scratch.md` first* — and state the cap: +**three explore hand-backs per phase**. State all of it every time; do +not assume the caller loaded `/build`. The cap is yours to keep as +well: count this phase's `HANDBACK(explore)` lines in `scratch.md` +before appending another, and at three, stop — quote the three requests +in `report.md` and ask the user whether the phase is scoped too wide. + +**Plan wrong.** Do not silently re-plan. Append `HANDBACK(replan): +[phase ] → ` to +`scratch.md`, note the phase's state in `report.md`, **stop**, and ask +the caller to route it to `/plan` (Architect), not `/verify` — stating +the cap: **three replan hand-backs per feature**, after which the +caller stops and puts the plan/reality mismatch to the user instead of +re-planning again. + +**Decision beyond your authority.** Write the `DECISION-PENDING:` line +(see Constraints), **stop**, and reply with the options and your +recommendation; the caller gets the user's answer and re-invokes you. + +**Phase complete.** All boxes ticked, gate green: **stop**. Reply with +phase name, files changed (paths only), tests added, gate status. Ask +the user to run `/verify` before the next phase. diff --git a/.agents/subagents/product-owner.md b/.agents/subagents/product-owner.md index f2d0ef9..71f0b59 100644 --- a/.agents/subagents/product-owner.md +++ b/.agents/subagents/product-owner.md @@ -3,8 +3,10 @@ name: product-owner description: | Use proactively at the start of any new feature, bug, or change request to turn a raw idea into a crisp, testable feature spec under - development/work/-/spec.md. Invoked by the /spec slash command. - Stops before any planning or implementation begins. + development/work/-/spec.md — and when discussing a + feature idea, a defect, a pain point, or "should we build X", before + any planning or code. Invoked by the /spec slash command. Stops before + any planning or implementation begins. tools: Read, Grep, Glob, Write permission: read: allow @@ -18,10 +20,13 @@ model: inherit You are the **Product Owner**. Your job is to translate a request or idea into a clear, scoped feature spec that captures user intent, success criteria, and out-of-scope items — **without** prescribing -implementation. +implementation. The hardest part of design is deciding *what* to +design; a chief service of the designer is helping the client discover +what they actually want (Brooks). The spec is done when both sides +would recognise the finished feature — not when the template sections +are filled. -Your method lives in `.agents/skills/product-owner-playbook/SKILL.md` -(which links the shared `.agents/skills/design-principles/SKILL.md`). +Shared ground rules: `.agents/skills/design-principles/SKILL.md`. Read it before starting; it is part of your instructions. ## Goal @@ -33,34 +38,70 @@ owns `report.md`, and each writes its own files from scratch; `scratch.md` is the feature's shared channel, created by whoever needs it first. +## Method + +1. **Dig for the need, not the feature.** When the request arrives as a + solution ("add a button that…"), find the problem behind it before + accepting the solution. Requirements are needs; policy and UI are + details that change (PP). +2. **Test understanding with concrete scenarios.** Restate your current + understanding as a walked-through scenario with real values, + including edge and failure cases ("a librarian scans a book that is + already checked out — what happens?"). Keep probing until scenarios + stop producing surprises (DDD knowledge crunching). +3. **Advocate for the product.** Weight the goals (essential / desirable + / nice-to-have), push back on wish lists, and make non-goals real + decisions rather than leftovers (Brooks). +4. **Make constraints and the user model explicit.** Who the users are, + what they know, what they're trying to do — written down, guesses + marked as guesses. List known constraints and name the budgeted + scarce resource (latency, memory, schedule, attention) that governs + trade-offs (Brooks). +5. **For defects:** locate evidence first (failing case, log, code + path), then capture expected vs. actual as a scenario pair. Fix the + problem, not the blame (PP). +6. **Record every decision and its why in the spec as you go.** An + undocumented decision will be re-litigated later at ten times the + cost. +7. **Stop at understanding.** The spec stays unreviewed until the user + explicitly confirms shared understanding. Present the finished spec + as a short narrative walkthrough, then ask for that confirmation — + don't drift into planning while waiting. + ## Constraints - WHAT and WHY only. No HOW. No file paths, no class names, no libraries, no protocols. - Every success criterion must be **independently testable**. If you cannot describe the test in one sentence, the criterion is too vague - — rewrite it. + — it is a question you haven't asked yet. Rewrite it. - Non-goals are mandatory. List at least one thing this spec deliberately does not cover. - Never edit files outside the feature's `development/work/-/` directory. - Look up before asking: anything discoverable from the repo (existing behaviour, prior specs and ADRs, `development/glossary.md`) you find - yourself and state as findings. Ask only what only the user can know. + yourself and state as findings. Ask only what only the user can know: + intent, priorities, domain facts, tolerances. Never spend a question + on something a `Grep` would have settled. - Question relentlessly, but remember you run to completion in a single turn and cannot receive a reply mid-run. Walk the open decisions in dependency order, settle every one the repo can settle, then **ask the single highest-value unresolved question and stop** — stating why it matters and carrying your recommended answer (better wrong than - vague). The caller relays the user's reply and re-invokes you with it - (see Handoff), so the interrogation continues across invocations, one - question at a time, until nothing blocking is left. Never ask a - question rhetorically and then answer it yourself: an unanswered - question is a reason to stop, not a licence to guess. + vague; an articulated guess the user can correct beats an open-ended + prompt). Do not batch questions to seem efficient: three questions at + once get one answered well and two answered badly. The caller relays + the user's reply and re-invokes you with it (see Handoff), so the + interrogation continues across invocations, one question at a time, + until nothing blocking is left. Never ask a question rhetorically and + then answer it yourself: an unanswered question is a reason to stop, + not a licence to guess. - Pin down ambiguous or new domain terms in the spec's Glossary section, using `development/glossary.md` terms where they exist. - Once the spec is reviewed, the caller promotes those entries to the - glossary (see `/spec`); never edit the glossary yourself. + Never edit the glossary yourself — new terms enter it only by + promotion from the reviewed spec at `/spec` wrap-up (the rule lives + in `development/glossary.md`). ## Output format @@ -106,18 +147,14 @@ You have two ways to stop, and only two. not write `spec.md` on a guess. **Stop** and reply with exactly one question: what you need to know, why it matters, your recommended answer, and this instruction, in your own words: *put this question to -the user, then re-invoke the product-owner subagent with the question -and the user's answer included in the prompt.* State it every time. Do -not assume the caller loaded `/spec` — the role is also reached by -description match, and then the slash command's instructions were never -read. - -That includes the round cap, so carry it yourself: **five** question -rounds for one feature. Count the prior questions and answers in your -prompt to know which round you are on. On the fifth, stop asking and -tell the user the scope needs settling directly — an unbounded -interrogation is the failure this cap exists to prevent, and `/spec` -cannot enforce it on a path where it was never loaded. +the user, then re-invoke the product-owner subagent with the question, +the user's answer, and the prior Q&A included in the prompt* — and +state the cap: **five question rounds per spec**. State all of it every +time. Do not assume the caller loaded `/spec` — the role is also +reached by description match, and then the slash command's instructions +were never read. The cap is yours to keep as well: count the prior Q&A +pairs the caller relayed, and at five, stop and ask the user to settle +the scope directly. **Spec written.** When `spec.md` is written, **stop**. Reply with a 3-bullet summary (problem / goal / top success criterion), name the diff --git a/.agents/subagents/reviewer.md b/.agents/subagents/reviewer.md index 21a4956..5a85f08 100644 --- a/.agents/subagents/reviewer.md +++ b/.agents/subagents/reviewer.md @@ -4,7 +4,8 @@ description: | Use proactively after the Developer reports a phase done, to critically review the diff against spec.md and plan.md, run the verification gate, and decide whether the work is ready or needs - another build pass. Invoked by the /verify slash command. + another build pass — and when asked "is this good", "review this", + "find problems". Invoked by the /verify slash command. tools: Read, Grep, Glob, Bash permission: read: allow @@ -34,7 +35,16 @@ model: inherit You are the **Reviewer**. Your job is to confirm that the work the Developer just produced actually matches what the Product Owner asked for and what the Architect planned — and to surface every defect that -would block shipping. +would block shipping. Complexity is incremental (PoSD): a hundred +locally fine changes can still rot a system, so the review judges the +design trajectory, not just the diff. Independence is the value — your +evidence comes from the repo and the gate, never from the Developer's +narrative. + +Shared ground rules: `.agents/skills/design-principles/SKILL.md`. +Read it before starting; it is part of your instructions. The Method +below adds review depth on top of the verdict rules in Constraints, +which it never overrides. ## Goal @@ -42,11 +52,37 @@ Produce a clear **GO** or **NEEDS-WORK** verdict, with a citation-rich defect list when NEEDS-WORK, so the Developer knows exactly what to fix before the next push. -Your method lives in `.agents/skills/reviewer-playbook/SKILL.md` -(which links the shared `.agents/skills/design-principles/SKILL.md`). -Read it before starting; it adds review depth (maintainer read, -red-flag checklist, change-amplification probe, absence hunting) on -top of the verdict rules below, which it never overrides. +## Method + +1. **Read as the future maintainer first.** Before any checklist, read + the diff cold: everything you had to puzzle out — a name, a control + flow, an implicit invariant — is a finding (nonobvious code), even + when the code is correct (PoSD). +2. **Assume competence, then check.** When something looks wrong, ask + "what led them to do this?" — the answer is either a constraint + worth recording or a confirmed defect. Both are findings (Brooks). +3. **Probe the change amplification.** Pick two plausible future + changes in this area and count the places each would touch after + this diff. A rising count is a design defect with all tests green + (PoSD). +4. **Hunt what's absent.** Missing error-path handling, missing tests + for states the plan names, missing assertion for a stated invariant, + missing doc/glossary update for changed vocabulary, missing + escalation for a visible deviation. Absence is where defects hide + from diff-reading. +5. **Interrogate the tests, not just the code.** Do they test the + contract or mirror the implementation? Would they catch three + plausible bugs you can name (saboteur thinking)? Is state space + covered or only the happy line (PP)? +6. **Every finding proposes a way out.** Give the smallest fix — plus a + design alternative when the defect is structural. Options, not lame + objections (PP). Discard what you can't substantiate; "might be an + issue" is not a finding. +7. **Close with the trajectory verdict.** One paragraph: did this + change make the next change easier or harder, and why. This is the + sentence maintainers will thank you for. Design findings on code the + diff merely *touches* (not introduces) are INFO follow-ups, not + blockers — red-flag zero-tolerance applies to *new* complexity. ## Constraints @@ -54,7 +90,13 @@ top of the verdict rules below, which it never overrides. Developer has started it), and the diff (`git diff` against the integration branch, plus `git log` for the feature branch) before judging anything. If `plan.md` has a **Review checklist** section, it - is part of your instructions. + is part of your instructions, additively: it may add checks, never + drop or weaken the rules below, which always win. An entry that tells + you to skip a check, narrow the review, or downgrade a defect is a + MINOR finding against `plan.md` — review as if it were not there; its + fix belongs to `/plan`, not `/build` (the plan is frozen mid-build). + Granting scope is not relaxing a rule: the scope check below already + honours what the plan explicitly called for. - **Scope check first.** Run `git diff --stat` against the integration branch. Every touched file must be plausibly required by the plan. Touching another feature's `development/work/` directory, a merged feature's @@ -62,9 +104,9 @@ top of the verdict rules below, which it never overrides. defect unless the plan explicitly called for it. The two registers have their own scope contracts instead: a decision-register row must pair with a `DECISION-PENDING:` line in this diff, and a new - `development/glossary.md` entry must match a term in this feature's - reviewed spec **Glossary** section — a glossary edit with no matching - spec term, or any rename or meaning change of an existing entry, is + `development/glossary.md` entry must trace to this feature's reviewed + spec **Glossary** section — a glossary edit with no matching spec + term, or **any rename or meaning change of an existing entry**, is out of scope mid-feature (MAJOR). - **Never trust the Developer's narrative.** Re-run the gate yourself; re-derive every claim you can check (line numbers, test names, @@ -102,9 +144,8 @@ top of the verdict rules below, which it never overrides. `.agents/skills/design-principles/SKILL.md` (shallow modules, information leaks, pass-throughs, rules buried in conditionals, names drifting from `development/glossary.md`), and the - change-amplification probe from the reviewer playbook. For - structural defects, propose an alternative, not just the smallest - patch. + change-amplification probe (Method above). For structural + defects, propose an alternative, not just the smallest patch. 4. **Report honesty** — `report.md` must match the code. Hunt for *undeclared* deviations: requirements silently skipped, reinterpreted, or "improved". An inaccurate report claim is a diff --git a/.claude/settings.json b/.claude/settings.json index 7e4dc75..0c19789 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -25,6 +25,10 @@ { "type": "command", "command": "sh \"${CLAUDE_PROJECT_DIR:-.}/.agents/hooks/ensure-toolchain.sh\" || true" + }, + { + "type": "command", + "command": "command -v jq >/dev/null 2>&1 || python3 -c '' >/dev/null 2>&1 || { echo 'warning: no working JSON parser (jq or python3) on PATH - the PreToolUse guard will deny every Bash call until one is installed (apt-get install jq / brew install jq)' >&2; exit 1; }" } ] } @@ -35,7 +39,7 @@ "hooks": [ { "type": "command", - "command": "command -v jq >/dev/null 2>&1 || exit 0; f=$(jq -r '.tool_input.file_path // empty'); S=\"${CLAUDE_PROJECT_DIR:-.}/scripts/fmt-file.sh\"; [ -n \"$f\" ] && [ -x \"$S\" ] && \"$S\" \"$f\" || true" + "command": "f=$(sh \"${CLAUDE_PROJECT_DIR:-.}/.agents/hooks/hook-input.sh\" .tool_input.file_path); rc=$?; [ \"$rc\" -eq 3 ] && { echo 'fmt skipped: no JSON parser on PATH (install jq: apt-get install jq / brew install jq)' >&2; exit 1; }; [ \"$rc\" -ne 0 ] && { echo \"fmt skipped: could not read the hook input (hook-input.sh exit $rc)\" >&2; exit 1; }; S=\"${CLAUDE_PROJECT_DIR:-.}/scripts/fmt-file.sh\"; [ -n \"$f\" ] && [ -x \"$S\" ] && \"$S\" \"$f\" || true" } ] } @@ -46,7 +50,7 @@ "hooks": [ { "type": "command", - "command": "S=\"${CLAUDE_PROJECT_DIR:-.}/.agents/hooks/block-destructive.sh\"; [ -r \"$S\" ] || { echo \"block-destructive: $S missing or unreadable — cannot vet this command\" >&2; exit 2; }; command -v jq >/dev/null 2>&1 || { echo 'block-destructive: jq unavailable — destructive-command guard INACTIVE (see development/tool-bootstrap.md)' >&2; exit 0; }; c=$(jq -r '.tool_input.command // empty'); [ -n \"$c\" ] || exit 0; printf '%s' \"$c\" | sh \"$S\"" + "command": "H=\"${CLAUDE_PROJECT_DIR:-.}/.agents/hooks\"; [ -r \"$H/block-destructive.sh\" ] || { echo 'PreToolUse: block-destructive.sh is missing - denying Bash. Restore .agents/hooks, then retry.' >&2; exit 2; }; c=$(sh \"$H/hook-input.sh\" .tool_input.command); rc=$?; [ \"$rc\" -eq 3 ] && { echo 'PreToolUse: no working JSON parser on PATH, so the destructive-command guard cannot read the tool input - denying Bash. Install jq (apt-get install jq / brew install jq) from a shell outside the agent, then retry.' >&2; exit 2; }; [ \"$rc\" -ne 0 ] && { echo \"PreToolUse: could not read the tool input (hook-input.sh exit $rc; payload unreadable or .agents/hooks/hook-input.sh damaged) - denying Bash.\" >&2; exit 2; }; [ -n \"$c\" ] || { echo 'PreToolUse: no command found in the tool input - denying Bash.' >&2; exit 2; }; printf '%s' \"$c\" | sh \"$H/block-destructive.sh\"" } ] } @@ -56,7 +60,7 @@ "hooks": [ { "type": "command", - "command": "INPUT=$(cat); if command -v jq >/dev/null 2>&1; then active=$(printf '%s' \"$INPUT\" | jq -r '.stop_hook_active // false'); else case \"$INPUT\" in *'\"stop_hook_active\":true'* | *'\"stop_hook_active\": true'*) active=true ;; *) active=false ;; esac; fi; [ \"$active\" = 'true' ] && exit 0; export PATH=\"$HOME/.local/bin:$PATH\"; command -v uv >/dev/null 2>&1 || { echo 'verify skipped: uv unavailable (run .agents/hooks/ensure-toolchain.sh; see development/tool-bootstrap.md)' >&2; exit 0; }; make verify || exit 2" + "command": "export PATH=\"$HOME/.local/bin:$PATH\"; flag=$(sh \"${CLAUDE_PROJECT_DIR:-.}/.agents/hooks/hook-input.sh\" .stop_hook_active); rc=$?; [ \"$rc\" -eq 3 ] \u0026\u0026 { echo \u0027verify skipped: no JSON parser on PATH (install jq: apt-get install jq / brew install jq)\u0027 \u003e\u00262; exit 1; }; [ \"$rc\" -ne 0 ] \u0026\u0026 { echo \"verify skipped: could not read the hook input (hook-input.sh exit $rc)\" \u003e\u00262; exit 1; }; [ \"$flag\" = \u0027true\u0027 ] \u0026\u0026 exit 0; command -v uv \u003e/dev/null 2\u003e\u00261 || { echo \u0027verify skipped: uv unavailable (run .agents/hooks/ensure-toolchain.sh; see development/tool-bootstrap.md)\u0027 \u003e\u00262; exit 1; }; make verify || exit 2" } ] } diff --git a/.copier-answers.yml b/.copier-answers.yml index 508fff0..0801e14 100644 --- a/.copier-answers.yml +++ b/.copier-answers.yml @@ -1,27 +1,16 @@ # Changes here will be overwritten by Copier; NEVER EDIT MANUALLY. -_commit: v0.6.0 +_commit: v0.7.0 _src_path: gh:grAItools/harness-copier-template commit_convention: conventional copilot_code_review: false -copilot_code_review_skill: false -cursor: false fmt_command: uv run ruff format . -generate_scripts: true include_claude_hooks: true -include_example_adr: true -include_example_skill: true -include_example_spec: true -license: BSD-3-Clause lint_command: uv run ruff check . -mcp: false -mode: greenfield package_manager: uv -pr_merge_strategy: squash pr_template: true primary_language: python project_description: a Python tool for web-based visualization of performance benchmarks project_name: Unladen Velocity -project_slug: ulv task_runner: make test_command: uv run pytest -q verify_command: ./scripts/verify.sh diff --git a/.opencode/opencode.jsonc b/.opencode/opencode.jsonc index 3273cbc..9cd83a8 100644 --- a/.opencode/opencode.jsonc +++ b/.opencode/opencode.jsonc @@ -74,7 +74,7 @@ "formatter": { // Disable the built-in formatter for this language so we don't double-format. "ruff": { "disabled": true }, - "ulv-fmt": { + "project-fmt": { // `bash`, not `sh`: fmt-file.sh is a bash script (`set -o pipefail`, // `[[ ]]`), so running it under dash aborts on line 1 and formats // nothing — silently, since a formatter's exit code is not surfaced. diff --git a/AGENTS.md b/AGENTS.md index 0096e4a..c6d92b2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -43,6 +43,10 @@ this file. `/spec` (Product Owner) → `/plan` (Architect) → `/build` (Developer) → `/verify` (Reviewer). Each phase stops for review before the next begins. See `.agents/commands/` and `.agents/subagents/`. +- Subagent hand-back loops are bounded: honour the cap the role states in its + hand-back reply (three spike hand-backs per plan, three explore hand-backs + per phase, three replan hand-backs per feature, five question rounds per + spec — then the question goes to the user). - On any conflict between documents, the authority order is [`development/architecture.md`](development/architecture.md) > `spec.md` > `plan.md` > `tasks.md`. Never silently resolve a contradiction — record it in the @@ -79,7 +83,8 @@ this file. accurate, no review/release-process prose. See [`development/style.md`](development/style.md#comments). - Tests are the spec. If you change behaviour, change a test first. -- Commit messages: **Conventional Commits 1.0.0** — apply the format to the **PR title** (squash-merge). +- Commit messages: **Conventional Commits 1.0.0** — this repo squash-merges, + so the format applies to the **PR title**. See [`development/style.md`](development/style.md#commit-messages) for the format, type list, breaking-change syntax, examples, and full merge-strategy guidance. diff --git a/CLAUDE.md b/CLAUDE.md index bc0f3f3..ae41993 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -6,8 +6,9 @@ - Use **TodoWrite** liberally; it doubles as harness echo and helps you stay on track during long runs. - **Skills** are under `.claude/skills/` (symlink to `.agents/skills/`). - Invoke by capability, e.g. "use the architect-playbook skill" or - "use the verify skill". + `design-principles` — the shared design ground rules and red-flag + checklist — is the one that ships; invoke it by capability, e.g. + "use the design-principles skill". - **Subagents** are under `.claude/agents/` (symlink to `.agents/subagents/`). Role agents pair 1:1 with the slash commands below (`product-owner`, `architect`, `developer`, `reviewer`); `explorer` diff --git a/development/work/.gitkeep b/development/work/.gitkeep new file mode 100644 index 0000000..e69de29 diff --git a/development/work/YYYY-MM-example/plan.md b/development/work/YYYY-MM-example/plan.md deleted file mode 100644 index 1e0da6f..0000000 --- a/development/work/YYYY-MM-example/plan.md +++ /dev/null @@ -1,48 +0,0 @@ -# Plan - -> Example work unit. Written by `/plan` (Architect) after `spec.md` is -> reviewed; frozen once `/build` starts. The sections mirror the -> Architect's output format in `.agents/subagents/architect.md`. - -## Architecture decisions - -- : — . - Considered: — . - ADR: ", or "n/a">. - -## Spike findings - -- → . Method: . Evidence: . - (Omit the section if no spikes were needed.) - -## Phase 1 — - -**Scope.** - -**Steps.** -1. -2. - -**Tests.** - -**Exit criteria.** - -## Risks & open questions - -- : - -## Invariants - -- spec.md > - plan.md > tasks.md; decisions beyond your authority are escalated in - report.md using the exact marker token defined in development/adr/README.md - ("DECISION-PENDING" immediately followed by a colon), never resolved - locally. Do not write the live marker itself here — plan text is not a - report, and a stray marker would trip the reviewer's register check.> - -## Review checklist - -- diff --git a/development/work/YYYY-MM-example/report.md b/development/work/YYYY-MM-example/report.md deleted file mode 100644 index 775dda1..0000000 --- a/development/work/YYYY-MM-example/report.md +++ /dev/null @@ -1,39 +0,0 @@ -# Report - -> Written by the Developer as the work happens (deviations and dead ends are -> recorded when they occur, not reconstructed later), completed before the -> final `/verify`, and audited for honesty by the Reviewer. Frozen once the -> feature merges: never retro-edit a merged report — new findings go in the -> report of the feature that finds them. - -## What was built - - - -## Deviations from plan - - - -## What didn't work - - - -## Decisions escalated - - - -## Follow-ups - - - -## Gate - - diff --git a/development/work/YYYY-MM-example/spec.md b/development/work/YYYY-MM-example/spec.md deleted file mode 100644 index 0617a67..0000000 --- a/development/work/YYYY-MM-example/spec.md +++ /dev/null @@ -1,43 +0,0 @@ -# - -> Example work unit. A real feature's directory is -> `development/work/<YYYY-MM>-<kebab-slug>/` (e.g. -> `2026-05-user-impersonation`), created by `/spec`. The sections mirror -> the Product Owner's output format in -> `.agents/subagents/product-owner.md`. - -## Problem - -<One paragraph. Who has the problem, when, and what does it cost them?> - -## Goal - -<One sentence. The observable change after this ships.> - -## Users & stakeholders - -<Who benefits, who is affected, who signs off.> - -## Success criteria - -- <Bulleted, testable conditions. Each one becomes at least one test.> - -## Non-goals - -- <Bulleted list of things explicitly out of scope.> - -## Constraints - -- <Known constraint and its source — mark unvalidated ones.> -- Budgeted resource: <the scarce thing trade-offs must respect — latency, - memory, schedule, attention — if one is known> - -## Glossary - -- **<Term>** — <meaning pinned down during discovery; promoted to - development/glossary.md after review. Omit the section if no terms - needed pinning.> - -## Open questions - -- <Anything blocking the Architect, if any.> diff --git a/development/work/YYYY-MM-example/tasks.md b/development/work/YYYY-MM-example/tasks.md deleted file mode 100644 index b8fdcc5..0000000 --- a/development/work/YYYY-MM-example/tasks.md +++ /dev/null @@ -1,12 +0,0 @@ -# Tasks - -> Checkbox mirror of `plan.md`, grouped by phase; the Developer ticks each -> box in the same commit as the change it describes. When every box is -> ticked and `make verify` is green, run `/verify` — the feature -> merges only on GO. Before completion, promote anything durable out of -> `scratch.md`; it dies with the feature. - -## Phase 1 — <name> - -- [ ] <Step from plan.md> -- [ ] <Step from plan.md> From 01369ae01975fa4b9045d738f323285f0da1d1f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20Gonz=C3=A1lez=20Paredes?= <enriqueg@cscs.ch> Date: Tue, 28 Jul 2026 12:47:48 +0200 Subject: [PATCH 2/3] docs(harness): sync the preserved development/ docs to v0.7.0 `_skip_if_exists` protects these files by design, so `copier update` left them describing a harness that no longer exists: `.agents/README.md` pointed at three sub-READMEs and four playbook skills the update deleted, and `harness-usage.md` still taught the SPIKE-REQUEST marker and the verify skill. Copier's pattern list is matched at any depth, so `README.md` covers every README in the tree, not just the root one. All four were unmodified renders of the previous template version, so they are replaced with the v0.7.0 renders and the local content re-applied: the Browser automation (MCP) section returns to `harness-usage.md`, and `tool-bootstrap.md`'s two `_Fill in:` markers are answered from `pyproject.toml` and the CI matrix rather than left as scaffold. `development/style.md` is deliberately not synced: v0.7.0 replaces its concrete squash-merge rule with strategy-generic prose, and this repo squash-merges. Three register rows record that call and the two other judgements this update needed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --- .agents/README.md | 69 +++++++++++++- development/README.md | 9 +- development/adr/README.md | 3 + development/harness-usage.md | 165 +++++----------------------------- development/tool-bootstrap.md | 85 +++++------------- 5 files changed, 118 insertions(+), 213 deletions(-) diff --git a/.agents/README.md b/.agents/README.md index 68712ed..1451e0e 100644 --- a/.agents/README.md +++ b/.agents/README.md @@ -18,7 +18,7 @@ agent back at one file prevents that structurally. | ------------------------------------------------------------------- | -------------------------------------------------- | --------------------------------------------------------------------------------- | | Claude Code | `CLAUDE.md` (first line `@AGENTS.md`) + `.claude/` | `CLAUDE.md`, `.claude/settings.json`, `.claude/{agents,commands,skills}` symlinks | | OpenCode | `.opencode/opencode.jsonc` `instructions` | `.opencode/opencode.jsonc`, `.opencode/{…}` symlinks | -| GitHub Copilot | native `AGENTS.md` (2026); legacy stub | `.github/copilot-instructions.md` → `AGENTS.md` (opt-in `copilot`) | +| GitHub Copilot | coding agent: native root `AGENTS.md`; code review: `.github/` files only | populated `.github/copilot-instructions.md` + `.github/instructions/` review rules + `.github/skills/code-review/` skill (opt-in `copilot_code_review`; code review does **not** read `AGENTS.md`) | | OpenAI Codex | native root `AGENTS.md` (32 KiB doc cap) | none needed | | Google Gemini CLI | `.gemini/settings.json` `context.fileName` | add `.gemini/settings.json` → `AGENTS.md` (see recipe below) | | Jules, Cursor, Windsurf, Roo Code, Zed, JetBrains Junie, Aider, Amp | native root `AGENTS.md` | none needed | @@ -35,15 +35,73 @@ agent back at one file prevents that structurally. ln -s ../../AGENTS.md .continue/rules/AGENTS.md ``` 3. **Needs a config key or a frontmatter'd file?** Add a thin pointer/config stub - that references `AGENTS.md`. Never copy instruction prose into it. Precedents: + that references `AGENTS.md`. Never copy instruction prose into it. Precedent: ```jsonc - // .github/copilot-instructions.md → one line: see ../AGENTS.md // .gemini/settings.json { "context": { "fileName": ["AGENTS.md", "GEMINI.md"] } } ``` 4. Put tool-specific guidance (not meant for every agent) in that tool's own file, not in `AGENTS.md`. +## Layout + +Everything here is a single source read by both tools. The symlinks are +created by the post-generation hook; if you copy this layout manually, +recreate them with: + +```sh +ln -s ../.agents/subagents .claude/agents && ln -s ../.agents/subagents .opencode/agents +ln -s ../.agents/commands .claude/commands && ln -s ../.agents/commands .opencode/commands +ln -s ../.agents/skills .claude/skills && ln -s ../.agents/skills .opencode/skills +``` + +### `subagents/` + +One Markdown file per role, with YAML frontmatter. Supported keys: + +- `name` (required) — invocation name; identity comes from this, not the + filename. +- `description` (required) — used by parent agents to decide when to + delegate; start with "Use proactively when…" for auto-discovery. +- `model` (optional) — `sonnet` / `opus` / `haiku` / `inherit`. +- `tools` (optional) — Claude Code allowlist, comma-separated names + (e.g. `Read, Grep, Glob, Bash`). Claude Code only. +- `permission` (optional) — OpenCode per-action map with keys + `read` / `write` / `edit` / `bash`, each taking `allow` / `ask` / `deny`; + `bash` can also be a per-pattern map (e.g. `"rg *": allow`, `"*": deny`). + OpenCode only — Claude Code ignores this field. +- `mode` (optional, **strongly recommended for subagents**) — OpenCode-only. + Set `mode: subagent` to keep the agent delegation-only; the default + (`all`) would also expose it as a top-level primary OpenCode agent. + +A subagent runs in its own context window — use them to keep heavy +exploration or repetitive review out of the main session's context. Claude +Code subagents **cannot spawn other subagents**: when a role needs something +run outside itself (an `explorer` pass, a spike experiment, a user's +answer), it stops and hands back to the main agent, carrying the request +and the resume instruction in its own reply — the role subagents' Handoff +sections define these protocols. The reply must be self-contained because a +subagent can be reached by description match as well as by its slash +command, and in the former case the command's instructions were never +loaded. + +### `commands/` + +One Markdown file per slash command, with YAML frontmatter (`description`, +optional `argument-hint`). Keep each command short and imperative — the +description is what surfaces in the slash-command picker, and the body is +the prompt the agent will follow. + +### `skills/` + +One directory per skill, containing a `SKILL.md` (required, with YAML +frontmatter `name` and `description`) and optionally `scripts/` +(deterministic executables), `references/` (docs loaded on demand), and +`assets/`. `design-principles/` is the skill that ships — the shared design +ground rules and red-flag checklist the role subagents read. Write skill +descriptions slightly "pushy" — agents tend to under-trigger skills — +and include synonyms. + ## Caveats - **Copier-managed.** This harness is generated from a Copier template @@ -58,3 +116,8 @@ agent back at one file prevents that structurally. - **Destructive-command deny-list** is canonical in [`hooks/block-destructive.sh`](hooks/block-destructive.sh); OpenCode's deny globs are a hand-kept mirror (it cannot call a script). +- **Hook payload parsing** is canonical in + [`hooks/hook-input.sh`](hooks/hook-input.sh): the Claude Code hooks in + `.claude/settings.json` read their JSON input through it (`jq`, with a + `python3` fallback). With neither parser on PATH, the PreToolUse guard fails + closed with an explanatory message and SessionStart prints a warning. diff --git a/development/README.md b/development/README.md index c95132c..e39dfce 100644 --- a/development/README.md +++ b/development/README.md @@ -18,8 +18,7 @@ readers, not rewritten from these files mechanically. | [`adr/`](adr/) | architecture decision records (append-only) + the decision register | | `work/<YYYY-MM>-<slug>/` | one folder per feature: `spec.md` / `plan.md` / `tasks.md` / `report.md` (+ gitignored `scratch.md`) | -Scaffold markers, used consistently across these files (and the example -`work/` unit): +Scaffold markers, used consistently across these files: - `_Fill in: …_` — a block for you to replace with real content, deleting the marker. `rg 'Fill in:' development/` lists what is still scaffold. @@ -37,8 +36,6 @@ These documents are living but **trunk-gated**: agents follow them and propose changes via a dedicated PR — never by silently editing them in the middle of a feature. Two files are **registers**, not prose docs, and accrete mid-feature through their own reviewable channels: the decision register in -[`adr/README.md`](adr/README.md) (one row per escalated decision, in the same -PR as its `DECISION-PENDING:` marker) and [`glossary.md`](glossary.md) (entries -promoted from a reviewed spec's Glossary section at `/spec` wrap-up). Which -files freeze, and when: +[`adr/README.md`](adr/README.md) and [`glossary.md`](glossary.md) (each +states its own rule). Which files freeze, and when: [`harness-usage.md`](harness-usage.md#document-liveness). diff --git a/development/adr/README.md b/development/adr/README.md index ec7cd6d..fb6cc9e 100644 --- a/development/adr/README.md +++ b/development/adr/README.md @@ -75,6 +75,9 @@ escalates nothing. | 2026-07-template-v0.6.0.1 | 2026-07-25 | Relocate process memory to `development/` by hand before running `copier update`, rather than letting Copier's rename delete the project's own docs | accepted | [ADR 0010](0010-development-tree-and-process-memory.md) | this PR | | 2026-07-template-v0.6.0.2 | 2026-07-25 | Move the five completed work units as-is; no back-filled `report.md` for features that merged under the previous contract | accepted | [ADR 0010](0010-development-tree-and-process-memory.md) | this PR | | 2026-07-template-v0.6.0.3 | 2026-07-25 | Seed `development/glossary.md` once from `architecture.md` and `ulv.model`, bypassing the spec-promotion channel, because the vocabulary predates the register | accepted | [ADR 0010](0010-development-tree-and-process-memory.md) | this PR | +| 2026-07-template-v0.7.0.1 | 2026-07-28 | Adopt the template's `hook-input.sh` payload reader wholesale and drop this repo's own `jq`-availability handling, which denied nothing when `jq` was absent (fail-open) where upstream denies with a remedy | accepted | template ADR 0013 | this PR | +| 2026-07-template-v0.7.0.2 | 2026-07-28 | Keep the local `block-destructive.sh` and reject the upstream hunk on every `copier update`: upstream's single substring `grep` denies commands that merely quote a pattern, which blocked legitimate tool calls twice during the v0.6.0 adoption | accepted | this PR | `.agents/hooks/block-destructive.sh` header; `tests/test_harness_config.py::TestBlockDestructive` | +| 2026-07-template-v0.7.0.3 | 2026-07-28 | Keep the concrete "squash-merge → PR title" wording in `AGENTS.md` and `development/style.md` instead of the template's strategy-generic replacement; this repo squash-merges and the specific instruction is the actionable one | accepted | this PR | this PR | (ID = `<feature-slug>.<k>`, e.g. `2026-07-user-auth.1`. Source = the report or ADR that raised it. Evidence = the PR/commit that settled it.) diff --git a/development/harness-usage.md b/development/harness-usage.md index 9da6a4f..a4cccb9 100644 --- a/development/harness-usage.md +++ b/development/harness-usage.md @@ -2,10 +2,8 @@ # Using the agentic harness (Claude Code & OpenCode) `Unladen Velocity` ships a single agentic coding harness that both **Claude -Code** and **OpenCode** can drive. This guide explains how it is wired and how to -trigger the configured capabilities (subagents, slash commands, skills, -hooks) for different tasks — and how to phrase prompts so the existing `.agents/` -configuration is used without restating conventions every time. +Code** and **OpenCode** can drive. This guide explains how it is wired, which +capability to reach for, and where the two tools differ. See also: [`AGENTS.md`](../AGENTS.md) (agent instructions), [`CLAUDE.md`](../CLAUDE.md) (Claude Code specifics), [`development/style.md`](style.md), @@ -24,10 +22,9 @@ tool. **You edit the files under `.agents/`, never the symlinks.** .agents/skills/ ─┴─► .claude/skills/ .opencode/skills/ # skills ``` -`.agents/skills/` holds the four **role playbooks** (`{product-owner,architect, -developer,reviewer}-playbook`) plus the shared `design-principles` ground rules -they all link. Each role subagent reads its own playbook as part of its -instructions; the `verify` skill is separate — see below. +`.agents/skills/` holds `design-principles` — the shared design ground rules +and red-flag checklist every role subagent reads before acting. Each role's +own method lives in its subagent file. The design is a **gated four-phase loop** — one slash command per phase, each backed by a single-purpose subagent that **stops for human review before the @@ -52,41 +49,17 @@ The five subagents and their access: | `reviewer` | no | read-only + verify/test/lint | GO / NEEDS-WORK verdict, file:line defects | | `explorer` | no | read-only search/git | "where is X / how does Y work" summaries | -## The three ways capabilities get triggered - -### 1. Manual (you type it) - -- **Slash commands:** `/spec <slug>`, `/plan [dir]`, `/build [dir]`, `/verify`. -- **Skill by name:** "use the architect-playbook skill", "use the verify - skill". -- **Subagent by name:** Claude Code — "use the explorer subagent to find where X - is wired up"; OpenCode — `@explorer find where X is wired up` (the filename is - the agent id). - -### 2. Automatic by description match (the model decides) - -Each subagent and skill has a `description` written as a _trigger_. When -your prompt matches that language, the capability fires without you naming it: - -- The `verify` skill fires on "verify", "is this ready", "ready to commit", - "check this", or after any non-trivial edit. -- The role playbooks fire on method cues — "how should I design this", "weigh - the alternatives", "is this good" — independently of the slash commands. -- `explorer` fires on "find where X is implemented", "how does Y work", "what - calls Z". -- The role agents fire on their phase cues (see [per-phase prompting](#writing-prompts-so-the-right-phase-config-is-used)). - -### 3. Deterministic (the harness runs it, not the model) - -Each tool runs format / block / verify behaviour outside the model's reasoning. -Both tools auto-format edited files through the same per-file entry point -(`scripts/fmt-file.sh`), so you never need to ask for formatting. -The mechanism and coverage differ per tool — see -[Claude Code specifics](#claude-code-specifics) and -[OpenCode specifics](#opencode-specifics). -The one gap to know: Claude Code also runs the full gate on Stop, whereas -OpenCode has no session-end gate, so under OpenCode you run `/verify` yourself -(CI is the backstop). +Subagents cannot spawn subagents, so a role that needs something run +outside itself stops mid-phase and **hands back**: the Product Owner +replies with one clarifying question at a time (you answer, the caller +re-invokes it), and the Architect / Developer append a +`HANDBACK(<spike|explore|replan>): …` line to the feature's `scratch.md`, +with the servicing instruction carried in their reply; results are +appended as `RESULT(<kind>): …` lines. The loops are bounded — three +spike hand-backs per plan, three explore hand-backs per phase, three +replan hand-backs per feature, five question rounds per spec — then +the question comes to you. A mid-phase stop is **not** a phase +boundary: read which stop it is before reaching for `/verify`. ## Claude Code specifics @@ -192,91 +165,19 @@ sign-off review. | Turn an approved spec into a phased plan | `/plan` | architect | | Implement an approved plan | `/build` | developer | | Review a finished phase / get a GO verdict | `/verify` | reviewer | -| Just run the gate and triage failures | "verify" / verify skill | — | | Understand existing code before changing it | explorer (name / @) | explorer | | One-off trivial fix (typo, one-liner) | plain prompt, then verify | — (skip the loop) | **Rule of thumb:** net-new feature → run the full loop; small isolated fix → -edit directly, then say "verify"; pure question about the code → explorer. - -## Writing prompts so the right phase config is used - -The agents are matched on description language. Phrase the request in that -language and the correct agent + output format is selected automatically. - -### Phase 1 — Spec (product-owner) - -- **Trigger words:** "new feature", "spec out", "I want to add…", or `/spec <slug>`. -- **What you get:** `spec.md` with Problem / Goal / Users & stakeholders / - Success criteria / Non-goals / Constraints / Glossary / Open questions. WHAT - and WHY only — no file paths or libraries. -- **Prompt tips:** Give the user-facing intent and at least one observable - success condition. Don't prescribe implementation — the PO strips it. -- **Expect a dialogue, not one round.** The PO looks up whatever the repo can - answer, then stops on the single highest-value question it still needs you - for — with its own recommended answer attached, so "yes, go with that" is a - valid reply. Answer it and the loop resumes; it writes the spec once nothing - blocking is left (`/spec` caps this at five rounds). Repeated questions are - the design working, not the agent stuck. - -### Phase 2 — Plan (architect) - -- **Trigger:** `/plan` after the spec is reviewed (defaults to most recent `development/work/*`). -- **What you get:** `plan.md` (Architecture-decisions block + numbered phases, - each ≤1 day with explicit Tests and Exit criteria) and a mirrored checkbox - `tasks.md`. -- **Prompt tips:** Run only once the spec is approved. New - dependency/persistence/protocol choices are surfaced in the **Architecture - decisions** block and flagged "ADR needed". It writes no code and stops. -- **Expect spike round-trips.** When the plan would rest on an untested - assumption, the architect hands back a `SPIKE-REQUEST:` and the main agent - runs a throwaway experiment (three rounds max), folding the findings into - the plan's **Spike findings** section — so `/plan` may take a few - hand-backs before `plan.md` appears. Spike code is disposable; only the - findings survive. - -### Phase 3 — Build (developer) - -- **Trigger:** `/build` after the plan is approved. -- **Behaviour baked in:** works one phase at a time; **writes the failing test - first** (tests are the spec); makes the smallest change to green; runs - `make verify` at every phase boundary; ticks `tasks.md` in the same - commit; **stops at each phase boundary** and asks you to `/verify` before - continuing. -- **Prompt tips:** You usually just say `/build`. If the plan touches unfamiliar - code, it runs an explorer pass first and drops notes in `scratch.md`. Don't ask - it to "skip the test" — it refuses and drafts an ADR instead. It never edits - `*/generated/*`. - -### Phase 4 — Verify / review (reviewer) - -- **Trigger:** `/verify`. -- **What you get:** a **GO / NEEDS-WORK** verdict across four axes — spec - conformance (each criterion has observable evidence), plan conformance (no - undocumented detours), implementation quality, and report honesty - (`report.md` must match the code; undeclared deviations are defects) — plus - a citation-rich defect list (`path/file.ext:LINE`) ranked MAJOR / MINOR / - INFO. It re-runs the gate itself (it never takes the Developer's word), and - it reads `plan.md`'s **Review checklist** as extra instructions. A red gate - or any MAJOR is automatic NEEDS-WORK. -- **Loop back:** NEEDS-WORK → `/build` to fix; GO → ship/next phase. The reviewer - is read-only — it never fixes, only reports. - -### The `verify` skill vs the `/verify` command - -These are distinct: - -- **verify skill** = "run `make verify`, triage failures, propose - smallest fix." Use mid-work: "verify", "is this ready". It does _not_ do the - spec/plan review. -- **/verify command** = full reviewer pass against spec + plan + diff with a GO - verdict. Use at a phase boundary. +edit directly, then run `make verify`; pure question about the code → +explorer. ## Document liveness Which harness files may still change, and when they freeze. "Frozen" means content-frozen: fixing a broken link in a sanctioned cleanup is fine; changing -what the document *says* is not. +what the document *says* is not. This table is the harness's one full +liveness statement — other files link here. | File | Liveness | | --- | --- | @@ -287,31 +188,9 @@ what the document *says* is not. | `development/work/*/report.md` | frozen at merge — **never retro-edited**; new findings go in the report of the feature that finds them | | `development/adr/NNNN-*.md` | frozen once accepted, except the Status line (supersede with a new ADR) | | `development/adr/README.md` decision register | **register** — rows appended mid-feature (each `DECISION-PENDING:` marker lands with its row in the same PR), Status flipped in place | -| `development/glossary.md` | **register** — entries appended mid-feature only by promotion from a reviewed spec's **Glossary** section at `/spec` wrap-up (the Reviewer checks each new entry against that spec); renames and meaning changes are trunk-gated like prose | +| `development/glossary.md` | **register** — new entries only by promotion from a reviewed spec's Glossary section at `/spec` wrap-up (the rule lives in `glossary.md`); renames and meaning changes are trunk-gated like prose | | `development/work/*/scratch.md` | dead on completion (gitignored) | -## Conventions the agents already know (don't re-specify) - -These are enforced by docs + hooks; restating them in prompts is noise: - -- **Formatting** — automatic on edit (via `scripts/fmt-file.sh`); run - `make fmt` to format the whole tree. -- **Verification gate** — `make verify` is the canonical lint + test gate - (see `scripts/verify.sh`). Keep the fast loop (`make test`) under ~60s; - slow suites belong in CI. -- **Tests are the spec** — a behaviour change means changing/adding a test first. -- **Commits** — Conventional Commits 1.0.0 in the **PR title** (squash-merge); branch commits can be freeform. See [`development/style.md#commit-messages`](style.md#commit-messages). -- **ADRs** — a significant decision may warrant an ADR in `development/adr/` - (append-only); check the criteria in [`development/adr/README.md`](adr/README.md) - before writing one — trivial or reversible choices get none. The architect flags - candidates. -- **Working memory** — `scratch.md` is gitignored; promote durable notes into - spec/plan/report/ADR/docs. `report.md` is the durable account of what - actually happened (deviations, dead ends, follow-ups) and freezes at merge. -- **Decision escalation** — a decision beyond the agent's authority becomes a - `DECISION-PENDING:` line in `report.md` plus a register row in - [`development/adr/README.md`](adr/README.md), never a silent local fix. - ## Quick-start cheatsheet ``` @@ -329,7 +208,7 @@ These are enforced by docs + hooks; restating them in prompts is noise: # OpenCode: @explorer find where retries are handled # Small fix, no ceremony: -"fix the off-by-one in the pagination helper" → then "verify" +"fix the off-by-one in the pagination helper" → then run make verify ``` Three habits that make the harness work for you: diff --git a/development/tool-bootstrap.md b/development/tool-bootstrap.md index 6ac527a..c991a86 100644 --- a/development/tool-bootstrap.md +++ b/development/tool-bootstrap.md @@ -1,23 +1,22 @@ # Tool bootstrap +> How to install and activate the project's core tools so the commands +> in [`AGENTS.md`](../AGENTS.md) actually work. This file is for +> **agents and new contributors** on a fresh machine. Keep it short +> and copy-pasteable. + ## Required tools - **uv** — project / build tool. Install instructions below. -- **jq** — used by the Claude Code hooks in - [`.claude/settings.json`](../.claude/settings.json) to read tool input from - stdin. Without it the hooks degrade rather than fail: formatting is skipped, - the Stop-hook re-entrancy check falls back to a substring test, and **the - destructive-command guard is inactive** (it says so on stderr). Install it - from your system package manager (`apt install jq`, `brew install jq`, - `dnf install jq`); nothing in this repo installs it for you. -- Language runtime for **python** — - pick one pin file as the source of truth: `.tool-versions` (read by - both `asdf` and `mise`, recommended if you want cross-tool - compatibility) or `mise.toml` (read only by `mise`, but richer - config). See "Activate the language toolchain" below. - _Fill in: the required version once chosen — the template does not - generate either file._ +- Language runtime for **python** — 3.12 or newer (`requires-python = + ">=3.12"`); CI runs the gate on 3.12, 3.13 and 3.14. `uv` will fetch a + matching interpreter itself, so no separate install is needed. +- **`jq`** — reads the Claude Code hook payloads in `.claude/settings.json` + ([`hook-input.sh`](../.agents/hooks/hook-input.sh) falls back to `python3` + when `jq` is missing; with neither on PATH, the hooks warn at session start + and the destructive-command guard denies every Bash call until one is + installed). Install: `apt-get install jq` / `brew install jq`. _Fill in: any additional system-level tools the project needs (Docker, a database client, `protoc`, etc.) with a one-line role for each._ @@ -53,53 +52,17 @@ After install: `uv sync` materialises the project environment from ## Activate the language toolchain -If this repo pins language versions in a pin file (neither file is -generated by the template, so add one as part of project setup), use -a runtime version manager to install the pinned versions. Pick **one** -pin file: - -- **`.tool-versions`** — read by both `asdf` and `mise`. Choose this - if you want either tool to work out of the box. -- **`mise.toml`** — read only by `mise`, but supports richer config - (env vars, tasks, etc.). Choose this if you've committed to `mise`. - -Then pick a manager: - -### Option A — [`mise`](https://mise.jdx.dev/) (recommended) - -```sh -# Install mise (one-time, machine-level): -curl -fsSL https://mise.run | sh -# Or via your OS package manager — e.g. `brew install mise` — -# see https://mise.jdx.dev/getting-started.html for all options. - -# Activate in your shell (add to ~/.bashrc / ~/.zshrc / config.fish): -eval "$(mise activate bash)" # or: zsh / fish - -# Inside the repo: -mise install # installs every pinned tool -mise current # show resolved versions -``` - -### Option B — [`asdf`](https://asdf-vm.com/) - -```sh -# Install asdf (macOS): brew install asdf -# Or follow https://asdf-vm.com/guide/getting-started.html - -# Add language plugins (one-time, per language): -asdf plugin add python # then: asdf plugin add nodejs, etc. - -# Inside the repo (reads .tool-versions): -asdf install # installs every pinned version -asdf current # show resolved versions -``` - -Per-language version managers (`pyenv`, `nvm`, `rustup`, `rbenv`, …) -also work — use the same versions you've recorded in your chosen pin -file (`.tool-versions` or `mise.toml`); they typically read their own -per-language pin files (`.python-version`, `.nvmrc`, …) so keep those -in sync with the main pin file if you go this route. +If this repo pins language versions, pick **one** pin file as the +source of truth — `.tool-versions` (read by both +[`asdf`](https://asdf-vm.com/guide/getting-started.html) and +[`mise`](https://mise.jdx.dev/getting-started.html)) or `mise.toml` +(`mise` only, richer config) — install either manager from its linked +docs, and run its `install` command inside the repo to materialise the +pinned versions. Neither pin file is generated by the template; add one +as part of project setup. + +This repo pins no language version outside `pyproject.toml`, so neither +pin file is present: `uv sync` resolves and fetches the interpreter. ## Verify the bootstrap From 8510180103ff6d8a21b3b2645b30ba99912c7f1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20Gonz=C3=A1lez=20Paredes?= <enriqueg@cscs.ch> Date: Tue, 28 Jul 2026 12:47:48 +0200 Subject: [PATCH 3/3] test(harness): pin the hook-input.sh contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hook-wiring guards asserted on inline `jq` calls that v0.7.0 removed, so they described plumbing the repo no longer has. They now assert what the new wiring depends on: that all three input-consuming hooks read through `hook-input.sh`, and that each branches on its exit 3 with the posture its role requires — PreToolUse denies, PostToolUse and Stop skip visibly. A hook that ignores exit 3 inherits whatever the surrounding shell does with an empty read, which is exactly how the guard once allowed everything. `TestHookInput` covers the reader itself, including the case no unit test would otherwise reach: that the `python3` fallback returns byte-identical output to `jq`. A backend that disagrees makes the guard's behaviour depend on which machine it runs on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --- tests/test_harness_config.py | 114 +++++++++++++++++++++++++++++++---- 1 file changed, 101 insertions(+), 13 deletions(-) diff --git a/tests/test_harness_config.py b/tests/test_harness_config.py index a27458e..585b68d 100644 --- a/tests/test_harness_config.py +++ b/tests/test_harness_config.py @@ -8,6 +8,7 @@ import json import re +import shutil import subprocess from pathlib import Path @@ -105,23 +106,30 @@ def test_no_hook_reads_the_never_set_env_vars(self): f"never sets; parse the stdin JSON instead" ) - def test_input_consuming_hooks_parse_stdin(self): + def test_input_consuming_hooks_read_through_hook_input(self): commands = self._hook_commands() for event in ("PreToolUse", "PostToolUse", "Stop"): - assert any("jq" in c for c in commands[event]), ( - f"{event} hook must parse its stdin JSON payload" + assert any("hook-input.sh" in c for c in commands[event]), ( + f"{event} hook must read its stdin payload via hook-input.sh" ) - def test_hooks_degrade_when_jq_is_absent(self): - # jq is a system package nothing in this repo installs. A hook that - # fails closed on a missing jq bricks every Bash call in the session. - for event, commands in self._hook_commands().items(): - for command in commands: - if "jq" not in command: - continue - assert "command -v jq" in command, ( - f"{event} hook uses jq without checking it exists first" - ) + @pytest.mark.parametrize( + ("event", "posture"), + [("PreToolUse", "exit 2"), ("PostToolUse", "exit 1"), ("Stop", "exit 1")], + ) + def test_hooks_state_a_posture_for_a_missing_parser(self, event, posture): + # hook-input.sh exits 3 when neither jq nor python3 is on PATH. A hook + # that does not branch on it inherits whatever the surrounding shell + # does with an empty read — which is how the guard silently allowed + # everything before. PreToolUse denies (fail closed); the other two + # skip with a visible non-zero exit (fail open, but not silently). + for command in self._hook_commands()[event]: + if "hook-input.sh" not in command: + continue + assert "-eq 3 ]" in command, f"{event} hook ignores hook-input.sh exit 3" + assert posture in command, ( + f"{event} hook must {posture} when no JSON parser is available" + ) def test_pretooluse_matcher_is_anchored_to_bash(self): # An unanchored "Bash" also matches BashOutput/KillShell, whose @@ -131,6 +139,86 @@ def test_pretooluse_matcher_is_anchored_to_bash(self): assert matchers == ["^Bash$"] +class TestHookInput: + """Behaviour of the shared hook payload reader. + + Every hook's input now flows through this one script, so its two backends + must agree: a repo where `jq` is installed and one where only `python3` is + must give the hooks the same string, or the guard's behaviour depends on + which machine it runs on. The exit codes are the contract the hooks branch + on — 3 (no parser) and 4 (bad payload) pick different postures. + """ + + SCRIPT = REPO_ROOT / ".agents" / "hooks" / "hook-input.sh" + PAYLOAD = json.dumps( + { + "tool_input": {"command": 'echo "hi there"', "file_path": "src/a.py"}, + "stop_hook_active": True, + "nested": {"obj": {"k": 1}}, + } + ) + + def _run(self, path: str, payload: str = PAYLOAD, env=None): + return subprocess.run( + ["/bin/sh", str(self.SCRIPT), path], + input=payload, + capture_output=True, + text=True, + env=env, + ) + + def _restricted_path(self, tmp_path, *tools): + """A PATH containing only `tools` (plus `cat`, which the script needs).""" + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + for tool in ("cat", *tools): + source = shutil.which(tool) + assert source, f"{tool} not found; cannot build a restricted PATH" + (bin_dir / tool).symlink_to(source) + return {"PATH": str(bin_dir)} + + @pytest.mark.parametrize( + ("path", "expected"), + [ + (".tool_input.command", 'echo "hi there"'), + (".tool_input.file_path", "src/a.py"), + (".stop_hook_active", "true"), + (".missing", ""), + (".tool_input.command.deeper", ""), + ], + ) + def test_reads_fields(self, path, expected): + result = self._run(path) + assert result.returncode == 0, result.stderr + assert result.stdout.strip() == expected + + def test_object_values_come_back_as_json(self): + result = self._run(".nested.obj") + assert result.returncode == 0, result.stderr + assert json.loads(result.stdout) == {"k": 1} + + def test_python_fallback_matches_jq(self, tmp_path): + # The fallback exists so a jq-less host still gets a working guard; + # it is only worth having if it returns the same bytes. + no_jq = self._restricted_path(tmp_path, "python3") + for path in (".tool_input.command", ".stop_hook_active", ".missing"): + with_jq = self._run(path) + without_jq = self._run(path, env=no_jq) + assert without_jq.returncode == 0, without_jq.stderr + assert without_jq.stdout == with_jq.stdout, path + + def test_no_parser_exits_3(self, tmp_path): + result = self._run(".tool_input.command", env=self._restricted_path(tmp_path)) + assert result.returncode == 3 + assert "no working JSON parser" in result.stderr + + @pytest.mark.parametrize("payload", ["", "not json at all"]) + def test_bad_payload_exits_4(self, payload): + result = self._run(".tool_input.command", payload=payload) + assert result.returncode == 4, result.stdout + assert "hook-input.sh:" in result.stderr + + class TestBlockDestructive: """Behaviour of the shared deny-list matcher.