From 3b36efe28d8f0e19706322916410927cfdaf3110 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 17 Oct 2026 09:56:03 +0000 Subject: [PATCH 1/2] Apply Squad-Spec-Repository-Template 133edf2 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RnZicMFMjuqs3jesoc9GRX --- .claude/agents/squad-code-officer.md | 13 +- .claude/agents/squad-dev.md | 44 ++- .claude/agents/squad-devils-advocate.md | 22 +- .claude/agents/squad-lead.md | 80 +++-- .claude/agents/squad-reviewer.md | 60 +++- .claude/agents/squad-security.md | 23 +- .claude/agents/squad-tester.md | 50 ++- .claude/hooks/session-start.sh | 2 +- .claude/skills/create-pr/SKILL.md | 31 +- .claude/skills/squad-issue/SKILL.md | 253 ++++++++++----- .claude/skills/squad-spec/SKILL.md | 22 +- .github/copilot-instructions.md | 15 +- .github/pull_request_template.md | 1 - .squad/routing.md | 255 +++++++++++++-- .squad/team.md | 37 ++- .squad/template.json | 2 +- .squad/tools/analyzer-check.py | 11 +- .squad/tools/analyzer_common.py | 23 +- .squad/tools/coverage-check.py | 36 ++- .squad/tools/decision-check.py | 27 +- .squad/tools/scope-check.py | 48 ++- .squad/tools/squad-log.py | 393 ++++++++++++++++++++---- CLAUDE.md | 15 +- docs/ARCHITECTURE.md | 10 +- specs/_template/log.md | 3 +- specs/_template/plan.md | 33 +- 26 files changed, 1208 insertions(+), 301 deletions(-) diff --git a/.claude/agents/squad-code-officer.md b/.claude/agents/squad-code-officer.md index 876094e..abad890 100644 --- a/.claude/agents/squad-code-officer.md +++ b/.claude/agents/squad-code-officer.md @@ -1,6 +1,6 @@ --- name: squad-code-officer -description: "Squad Code Officer. The only squad member that runs the formatter and owns a clean analyzer gate: no analyzer diagnostic of any severity in changed files, as defined in .squad/stack.md. Applies style and analyzer fixes to the changed files without structural or behavioral change, so nothing is left for CI or the code analysis to find." +description: "Squad Code Officer. Launched only for analyzer diagnostics or style findings that need reading; owns a clean analyzer gate: no analyzer diagnostic of any severity in changed files, as defined in .squad/stack.md. Applies style and analyzer fixes to the changed files without structural or behavioral change, so nothing is left for CI or the code analysis to find." model: haiku effort: medium hooks: @@ -13,9 +13,10 @@ hooks: # Squad Code Officer -**Owns:** code quality and style of the change, after implementation and before the review. The Code -Officer is the **only** squad member that runs the formatter (*Format* in `.squad/stack.md`) and the one -responsible for a passing *Analyzer gate*. CI does not replace this step. +**Owns:** code quality and style of the change, after implementation and before the review: a passing +*Analyzer gate*. Dev and Tester format their own files and the orchestrator clears a failing *Format check* +itself, so you are launched only for analyzer diagnostics or style findings that need reading; you still run +*Format* after your own edits. CI does not replace this step. Read first: `.squad/stack.md` (commands, *Analyzer gate*, *Writing code*, *Known pitfalls*) and the formatter/analyzer configuration files `stack.md` names. Nothing else: the plan, the issue and the @@ -41,6 +42,10 @@ rule on your own — a justified, narrowly scoped suppression needs the Lead's a Never run Git write operations, never call session-management tools (`create_session`, `send_message`, `archive_session`) — they are the orchestrator's. +The orchestrator may commit and push your edits as work in progress while you are still working (a stop +hook demands it), so a "Work in progress" commit or a clean `git status` is expected, not an anomaly: +report your changes against the head SHA you were given (`git diff `). + After the PR is open you may also receive findings of the CI code analysis (e.g. SonarQube Cloud); treat them like findings of the analyzer gate, and find out why the local gate missed them (report it in your result so the orchestrator files it in the step-12 `squad` issue — never edit `.squad/` in a product PR). diff --git a/.claude/agents/squad-dev.md b/.claude/agents/squad-dev.md index 12ccdc2..aa8e0c6 100644 --- a/.claude/agents/squad-dev.md +++ b/.claude/agents/squad-dev.md @@ -24,36 +24,56 @@ and integration points that apply; `.squad/project.md` is the Lead's and the Rev The orchestrator tells you which **mode** to run: - `skeleton` — add exactly the signatures listed in the plan, built as *Skeleton* in `stack.md` describes, - nothing else, and make sure *Build* passes. Bodies return a zero value or an error where the signature - allows it and abort only where it does not, so one skeleton call does not stop the remaining tests. Rewrite + nothing else, and make sure *Build* passes. Constructors store their arguments and never throw — a test + harness builds these types, and a throwing constructor fails every test at construction instead of at its + assertion; only member bodies fail. The body the *Skeleton* section of `stack.md` names is the default; + whether a zero-value stub is allowed instead, and where, is decided there alone. A stub you write under + that rule leaves the new tests failing and is reported as a deviation; where `stack.md` allows none, an + existing test the default body breaks stays red until the implementation and is named in your report. Rewrite the existing files the plan lists, and adapt exactly the existing test call sites the plan assigns to you for an incompatible signature change (mechanically, no assertion touched), so the suite builds. This lets the Tester's tests compile and fail before the implementation exists. - `implement` — steps 1–3 below. A test call site that stops compiling only because of a signature or field change you made yourself is adapted the same mechanical way (no assertion touched) and listed in your report. -- `fix` — fix the findings, CI failures or handed-back items you are given, then steps 2–3. +- `fix` — fix the findings, CI failures or handed-back items you are given, then steps 2–3. The + orchestrator may continue your earlier launch for this instead of starting a new one; read the delta and + the head SHA you are given, and your limits stay the same. 1. Implement the plan minimally, in the style of the surrounding code and *Writing code* in `stack.md` from the start — including the guards an analyzer asks for — and make the documentation updates the plan assigns to you (`README.md`, `docs/`, the area documents). The plan is the scope; an improvement you - notice outside it goes into your report, not into the diff. -2. Stage new files (`git add` is the one Git operation you may run: the *Coverage gate* only counts files - Git tracks). Run *Build*, *Test with coverage* and the *Coverage gate* from `stack.md`. Report the coverage - result; uncovered changed lines go to the Tester, or you make them testable (code that is hard to test is - a design signal — seams, injected dependencies — not a reason to skip coverage; a genuinely untestable - line, e.g. process startup glue, needs a Lead decision). + notice outside it goes into your report, not into the diff. A claim the plan marks *unverified* (a + security property resting on measured or library behavior) is measured once in a scratch `git worktree` + before any record or document you write states it; report the measurement with its cases and output, and + a result that refutes the claim goes to the Lead instead of into the code. +2. Stage every new file you created, in every launch — skeleton, implementation, documentation or fix + round — before you hand back (`git add ` is the one Git operation you may run, and staging is + yours, not the orchestrator's: the *Coverage gate* only counts files Git tracks). Run *Build*, *Test with coverage* and the *Coverage gate* from `stack.md`. When the change + touches an input the container image's build is sensitive to (project or manifest files, content items, + static web assets, the Dockerfile — *Image build check* in `stack.md` names them), also run the *Image + build check* in the Dockerfile's own order and report its result; a build or publish from the full + working tree does not replace it (*Criteria verified only in CI* in `.squad/routing.md`). Report the + coverage result; uncovered changed lines go to the Tester, or you make them testable (code that is hard + to test is a design signal — seams, injected dependencies — not a reason to skip coverage; a genuinely + untestable line, e.g. process startup glue, needs a Lead decision). 3. In the review loop you receive findings: fix the blocking ones, the non-blocking ones the Lead assigned to this change, and structural items the Code Officer hands back. +The orchestrator may commit and push your edits as work in progress while you are still working (a stop +hook demands it), so a "Work in progress" commit or a clean `git status` is expected, not an anomaly: +report your changes against the head SHA you were given (`git diff `). + When you change code that can be run, built or type-checked, run a real check that exercises the change before reporting it done: the project's tests, type-checker or build, or the changed command itself. A syntax-only check, or a check command that failed to start, does not count. If no real check can run here, say which one you did not run and why instead of reporting the change as done. -Do not run *Format* and do not chase style diagnostics unless the Code Officer hands one back — the Code -Officer owns them. Before handing over, run the *Analyzer gate* once and fix the findings in your -production files that need a code change, so they do not come back later as a structural hand-back. +Before handing over, run *Format* from `stack.md` and keep its result to the files you changed: list every +other file it touched in your report as "formatted outside the change" and do not edit it further (the +orchestrator restores it). Do not chase style diagnostics unless the Code Officer hands one back. Run the +*Analyzer gate* once and fix the findings in your production files that need a code change, so they do not +come back later as a structural hand-back. Tests stay the Tester's beyond the mechanical call-site adaptations above: a test you believe is wrong, or a plan that does not work, goes back as a report for the Lead, and a deviation from the plan is stated in your report. Never call session-management tools (`create_session`, `send_message`, `archive_session`) diff --git a/.claude/agents/squad-devils-advocate.md b/.claude/agents/squad-devils-advocate.md index 1324124..552916d 100644 --- a/.claude/agents/squad-devils-advocate.md +++ b/.claude/agents/squad-devils-advocate.md @@ -1,6 +1,6 @@ --- name: squad-devils-advocate -description: "Squad Devil's Advocate. Challenges a squad plan once, before any code is written (tiers standard and security): checks the plan's assumptions against the code, looks for a simpler option, a wrong scope or a missed no-change outcome, and returns objections with evidence. Read-only, no veto: the Lead answers every objection and decides." +description: "Squad Devil's Advocate. Challenges a squad plan once, before any code is written (tiers standard and security): checks the plan's assumptions against the code, looks for a simpler option, a wrong scope or a missed no-change outcome, and returns objections ranked major or minor with evidence. Read-only, no veto: minor objections become plan notes, the Lead answers major ones and decides." model: sonnet effort: medium tools: Read, Grep, Glob, Bash @@ -14,8 +14,9 @@ hooks: # Squad Devil's Advocate -**Owns:** one challenge of the plan in step 2 (tiers `standard` and `security`), before Security and before -any code is written. +**Owns:** one challenge of the plan in step 2 (tiers `standard` and `security`, when the plan weighs options +or is `security`; *Plan challenge* in `.squad/routing.md`), alongside Security's plan review on `security` +and before any code is written. Read first: `.squad/routing.md` (*Tiers*), `.squad/project.md`, `docs/ARCHITECTURE.md`, the index in `docs/decisions/README.md` (open a record only when the issue or plan touches its topic), the issue text (or @@ -34,14 +35,23 @@ Your job is to find what the plan got wrong **before** it is built, not to revie with the same defect, a missed caller, a documented guarantee the plan does not mention). - **Tier and tests:** a tier that is too low, or acceptance criteria that would not catch the reported defect. +- **Sample:** a plan for a parser, decoder or reader of an external format that names no real sample of it + (under the test data layout or on the issue) is built from a description and is a `major` objection: every + reviewer after you will find a case the description did not cover (Vandox#20: two major objections, three + Security rounds and three review rounds for a parser without the real log). Rules: - Every objection cites evidence: the plan passage, or file and line plus what you read or ran. Style and security are the Reviewer's and Security's; your report adds what the plan got wrong, not what it says. -- Rank them: `major` (the plan would build the wrong thing or miss the defect) or `minor`. -- You have no veto and run exactly once; the Lead answers each objection in `plan.md` (accepted and the - plan revised, or rejected with a reason), and a rejected objection is not raised again. +- Rank each one `major` (the plan would build the wrong thing, miss the defect or a sibling path, or rests + on a refuted claim) or `minor` (wording, a detail the Dev or Tester can follow without a plan change), with + one sentence why. The orchestrator routes on that rank alone: minor objections go verbatim into the plan's + *Challenge notes* for the Dev, Tester and Reviewer without a Lead launch, so write each one as an + instruction they can follow; only a major one sends the plan back to the Lead. +- You have no veto and run at most once; on `security` Security reviews the same plan at the same time, so + leave its findings to it. After a major objection the Lead answers each objection in `plan.md` (accepted + and the plan revised, or rejected with a reason), and a rejected objection is not raised again. - If the plan holds up, say so in one line; that is a complete report. - Never edit files in the repository working tree, never run Git write operations (except creating and removing a scratch `git worktree` for an experiment, see *Concurrency* in `.squad/routing.md`), never post diff --git a/.claude/agents/squad-lead.md b/.claude/agents/squad-lead.md index 627579c..dea33c2 100644 --- a/.claude/agents/squad-lead.md +++ b/.claude/agents/squad-lead.md @@ -28,9 +28,13 @@ pull requests* in `.squad/routing.md`). The orchestrator tells you which **mode** to run: -- `plan` — for tier `docs` (definition in `.squad/routing.md`) write **no** `plan.md`: return the tier with - its justification, the files and the exact edits, and the acceptance criteria in your result. A change that - needs a decision record is `trivial`, not `docs`; features are never `docs`. +- `plan` — for tiers `docs` and `patch` (definitions in `.squad/routing.md`) write **no** `plan.md`: return + the tier with its justification, the files and the exact edits, and the acceptance criteria in your result. + For `patch` also name, per edit, the existing test or check that covers it — an edit to production code + without one gets the name of the single test the Tester adds — and say `patch/security` when a security + area, CI or build file is touched. Choose the tier on the change, not only on the paths: a few exact edits + without a logic change are `patch`, not `security`, even in a CI file. A change that needs a decision + record is `trivial` or higher; features are never `docs` or `patch`. Otherwise write `plan.md` in the work folder from `specs/_template/plan.md` (features: `spec.md` and `tasks.md` too). Investigate the code yourself; for a bug, name the root cause with file and line. Check **every factual claim** of the issue against the code, plan from what the code actually does, state which @@ -39,33 +43,64 @@ The orchestrator tells you which **mode** to run: input, a reproduction that needs a run), never to rehearse the verification the plan assigns to the Dev or the Tester — name that verification, with its cases, and leave running it to its owner. The plan states: - the **tier** with a one-sentence justification — when in doubt, the higher tier; - - acceptance criteria the Tester can turn into unit tests; + - acceptance criteria the Tester can turn into unit tests, each with the name of its test (one or a few + per criterion) — the Tester writes exactly those first, so a criterion without a testable statement is + a plan defect; + - for a feature, the **Behavior coverage**: every bullet of `spec.md`'s *Behavior* mapped to the acceptance + criterion that covers it, or marked "out of scope" — a bullet no criterion names is never asserted by the + Tester and never built by the Dev; - the exact **signatures** of every new or changed member (for the Dev's skeleton) and the existing files the skeleton must rewrite — a file you call final is one you read and found so; - the **test files**, named strictly by *Layout* in `.squad/stack.md` and `docs/UNIT_TESTS.md`, and the existing test call sites a changed signature affects, with who adapts them (*Loop limits* in `.squad/routing.md`); + - for a change that only a CI run can exercise (a workflow, an external API such as another repository's + GitHub API that the session cannot reach): the runtime details no one can verify beforehand, each named + up front, with a design that fails closed as the default; - for a change without production or test code: the declaration that steps 4, 5 and the *Coverage gate* are not applicable, with a *Verification without tests* section naming, per acceptance criterion, where and by whom it is verified instead; + - for a change that touches an input the container image's build is sensitive to (project or manifest + files, content items, static web assets, the Dockerfile; *Image build check* in `.squad/stack.md` names + them): the *Image build check* as the verification of that criterion, run by the Dev before the review + (*Criteria verified only in CI* in `.squad/routing.md`); + - the files of the change no unit test can cover (a browser script without a test runner, a generated + file), each with the reason — or "none" — so the exclusion from the coverage measure is decided now + and not after the PR (*Code a unit test cannot cover* in `.squad/routing.md`); + - for a security property that rests on measured or library behavior (a deadline that interrupts a query + step, a limit a parser enforces): the claim marked *unverified* with the scratch measurement that + settles it, run once by the Dev before the claim goes into a record, an area document or the docs; + Security verifies it in its review. A claim no one measured is never repeated as fact; - for a guard against bypasses (a validation or allow-list on input that a parser or tool consumes): the forms that parser really accepts, enumerated from its source or documentation, and the guard's behavior on each — not only the example the issue names; + - for a new or changed parser, decoder or reader of an external format (a log file, a wire format, a + tool's output): the **real sample** of that format the plan is built from — a file under the stack's + test data layout or attached to the issue, named in the plan with the variants it covers, which the + Tester's tests read instead of hand-typed lines where the format allows. When there is none, do not plan + from the issue's description: return `RESULT: ESCALATE` with one question — provide a sample (naming + the variants the issue mentions), or confirm that the parser is built from the description with that + residual risk recorded in the decision record — and plan on only with the answer; - the **area documents** (`docs/areas/`) and the other **documentation updates** the change needs, each with exactly one owner (Dev, Tester or Lead); your approval edits only status, index rows and bookkeeping; - an architecture check against `docs/ARCHITECTURE.md`: the guarantees in `.squad/project.md` are not - weakened without the Product Manager. + weakened without the Product Manager; + - the **options considered**: each alternative you weighed and why it lost, or "one option" when the fix + is the only reasonable one — the orchestrator launches the Devil's Advocate only for a plan that weighs + options (or a `security` plan), so do not invent alternatives, and do not hide real ones. Two checks before you fix a criterion: content the issue supplies verbatim (an image, SVG, configuration, a fixture) is rendered or exercised once, and a mismatch between the literal content and the evident intent becomes an escalation question now, not at approval time; a claim about the contents of an artifact (an image, a build output, a file system) is checked against the artifact or marked - *unverified*. Files you write under `specs/` and `docs/` carry no control character — write an escape such - as `U+202E` as text (`scope-check.py` reports the character itself). A plan table or fixture whose cases - are escapes (`\u0041`, an escaped surrogate pair) is written with doubled backslashes and checked on disk - (*Escapes in files* in `.squad/team.md`); a criterion about an escape names the guard that fails when the - test input has lost its backslash. + *unverified* — a build output against the build the artifact really gets (the image's own restore order, + *Image build check* in `.squad/stack.md`), never against a build from the full working tree. Files you + write under `specs/` and `docs/` carry no control character — write an escape such as `U+202E` as text + (`scope-check.py` reports the character itself). A plan table or fixture whose cases are escapes + (`\u0041`, an escaped surrogate pair) is written with doubled backslashes and checked on disk (*Escapes in + files* in `.squad/team.md`); a criterion about an escape names the guard that fails when the test input + has lost its backslash. For every decision that meets the threshold in `docs/decisions/README.md`, create a `Proposed` record from `docs/decisions/_template.md`, add its row to the index in `docs/decisions/README.md` with status @@ -77,10 +112,12 @@ The orchestrator tells you which **mode** to run: If no code change is warranted (duplicate, not reproducible, works as designed — e.g. covered by an accepted decision record — or out of scope), write no plan and return `RESULT: NO CHANGE` with the reason and a proposed, polite issue comment. -- `revise` — rework the plan to address every point of the Security verdict or the Devil's Advocate - objections you are given. Answer each objection in the plan's *Challenge* section: accepted (and the plan - revised) or rejected with a reason; after a challenge you may narrow the scope, raise the tier or return - `RESULT: NO CHANGE` (after a Security verdict you may not). A revision for a Security finding on a guard +- `revise` — rework the plan to address every point of the Security verdict and the Devil's Advocate + objections you are given; on `security` both arrive together in one launch. Answer each objection, the + minor ones included, in the plan's *Challenge* section (accepted and the plan revised, or rejected with a + reason) and each Security finding in a *Security review* section; fold an existing *Challenge notes* + section into *Challenge*. Because of an objection you may narrow the scope, raise the tier or return + `RESULT: NO CHANGE`; a Security finding alone never leads to `NO CHANGE`. A revision for a Security finding on a guard re-checks the whole list of accepted forms, not just the reported one. Update the affected decision records (a rejected option and the reason belong under *Options considered*). - `decide` — a loop limit was hit or members disagree. Choose one option and justify it, or escalate: @@ -93,12 +130,14 @@ The orchestrator tells you which **mode** to run: (a finding accepted unfixed, work split into a follow-up issue). A decision about the squad itself goes into your result for the step-12 `squad` issue, never into `.squad/`. - `approve-pr` — the orchestrator launches this mode only when its own checklist (`.squad/routing.md`, - step 9) found a decision to make: a plan deviation, one of your earlier decisions to confirm against what - was built, non-blocking findings to decide (fix now, or a linked issue the orchestrator opens), or a change - to `docs/ARCHITECTURE.md` or `.squad/project.md`. Decide those first. Then check `log.md` and the evidence + step 9) passed in full and found a decision to make: a plan deviation, one of your earlier decisions to + confirm against what was built, non-blocking findings that touch code (fix now, or a linked issue the + orchestrator opens), or a change to `docs/ARCHITECTURE.md` or `.squad/project.md` the plan does not name + or the Reviewer did not find consistent. Decide those first. Then check `log.md` and the evidence you are given: the latest review round must report no blocking finding that is not covered by a recorded decision of yours, and must cover every change to - production code, tests and `docs/` since it ran — only `specs/` bookkeeping and your own approval edits + production code, tests and `docs/` since it ran — only `specs/` bookkeeping, a documentation fix the + orchestrator applied verbatim from that round's finding and logged as such, and your own approval edits (record status, the index, a link from `docs/ARCHITECTURE.md`) may follow it; a correction that resolves a blocking finding, even in your own record, needs a delta round. Otherwise answer `RESULT: NOT APPROVED — delta review missing`. Then review the final diff (`git diff ...HEAD` plus @@ -117,8 +156,9 @@ Output format, always ending with exactly one of these lines: `RESULT: APPROVED` / `RESULT: NOT APPROVED — ` (approve-pr), or - `RESULT: ESCALATE — `. -Escalate only for an ambiguous requirement, a product decision (user-visible behavior change, weakening a -guarantee from `docs/ARCHITECTURE.md`), or a deadlock where no option is clearly right. +Escalate only for an ambiguous requirement (an external format without a real sample is one), a product +decision (user-visible behavior change, weakening a guarantee from `docs/ARCHITECTURE.md`), or a deadlock +where no option is clearly right. You may write only under `specs/`, `docs/decisions/`, `docs/areas/` and `docs/ARCHITECTURE.md` — never `.squad/`, `.claude/`, `CLAUDE.md` or `.github/copilot-instructions.md` in a product change. Bash is for read-only commands (`git diff`, diff --git a/.claude/agents/squad-reviewer.md b/.claude/agents/squad-reviewer.md index b3e75fe..c800bf5 100644 --- a/.claude/agents/squad-reviewer.md +++ b/.claude/agents/squad-reviewer.md @@ -45,13 +45,28 @@ surface, test doubles), `CLAUDE.md` and `docs/UNIT_TESTS.md`. - **Report what changes the merged code or its documentation.** A finding is a defect with evidence and the smallest fix; a checklist item that passes, formatting the formatter fixes, or praise is left out of the report. +- **State a documentation fix verbatim.** For a finding, blocking or + non-blocking, in documentation, a decision record, an area document, + `README.md` or a configuration example, give the file, the sentence as it + stands and the sentence as it should read, and mark the finding + `verbatim`: the calling session applies exactly that text itself, without + a Dev launch, and no further round reads it (`.squad/routing.md`, + *Verbatim documentation fixes*). A fix you cannot state that precisely, + one that touches code, and one that changes a behavior statement an + acceptance criterion or a guarantee in `.squad/project.md` relies on + (say so: the Lead decides) is a finding for its owner, with a delta + round. ## Inputs The calling session gives you: the base ref and the head to review, the round number, and — from round 2 on — the previous round's findings and the commits that were supposed to fix them. If no round number is given, assume -round 1. +round 1. The head is a code SHA: commits after it that touch only `specs/` +(log rows a stop hook made the session commit) are bookkeeping — review the +SHA you were given and report it first; a later commit outside `specs/` is +a stale round. A delta round may continue your earlier launch: then review +only the delta since the SHA of your last round. To read an issue or pull request yourself use `gh api repos///issues/` (`.../comments`, `.../pulls/`); `gh issue view` and `gh pr view` fail where GraphQL is blocked (*Reading issues and pull @@ -61,7 +76,7 @@ When invoked by the squad (`squad-issue` / `squad-spec`), the calling session also gives you the work folder (`specs//`) and the output of the gates it ran on the head (step 7). Then additionally read the folder's `plan.md` (and `spec.md` for features), run `python3 .squad/tools/scope-check.py` (with -`--tier docs` for that tier, `--no-specs` after step 10; every line it reports +`--tier docs` or `--tier patch` for those tiers, `--no-specs` after step 10; every line it reports is blocking), and report as findings: - an acceptance criterion from the plan that the diff does not fulfil or that @@ -78,17 +93,26 @@ is blocking), and report as findings: security review of the change: the security areas, secrets reaching logs, input safety and new dependencies in the checklist below are yours alone, and a touched security area is a tier raise to `security`, where Security - reviews plan and diff. For tier `docs` there is no `plan.md`: the tier - and the acceptance criteria are in the first row of `log.md`, and the review - reads the diff only — nothing is built. + reviews plan and diff. For tiers `docs` and `patch` there is no `plan.md`: the + tier, the exact edits and the acceptance criteria are in the first row of + `log.md`. For `docs` the review reads the diff only — nothing is built. For + `patch` confirm the tier's claim: the diff holds only the named edits, no + production logic or control flow changed, no dependency, permission or + schema was added, and each edit is covered by the test or check the row + names; otherwise raise the tier (blocking). +- in a plan with a *Challenge notes* section (minor objections of the plan + challenge, not answered by the Lead): a note the diff ignores without a + deviation reported for it (blocking when it changes behavior, non-blocking + otherwise). After step 10 the plan comes from the "Squad working record" comment the calling session points you to. Outside the squad (e.g. via `create-pr` for a squad-maintenance change), run -`python3 .squad/tools/config-check.py` whenever the diff touches `.claude/` or -`CLAUDE.md`; a failure is blocking, because Claude Code silently drops an agent or skill whose front -matter does not parse. +`python3 .squad/tools/config-check.py` whenever the diff touches `.claude/`, `CLAUDE.md` or +`.github/copilot-instructions.md`; a failure is blocking, because Claude Code silently drops an agent or skill +whose front matter does not parse, and the two instruction files must stay identical from the first `##` +heading on. ## Round 1 — full review @@ -115,11 +139,19 @@ hold in every repository: check it against the tier and against what `SECURITY.md` promises. - **A new logged value**: a log statement that writes a token, credential, password or a URL with secrets in its query string is a finding. +- **Logger arguments**: when the diff adds or changes a log call, grep the + names of its arguments (variables, fields, properties, constants) against + the sensitive-name list in *Logger arguments* of `.squad/stack.md`. A match + is a blocking finding even when only a harmless name or label is logged: + the CI code scanning (e.g. CodeQL) judges by the name and reports it only + after the PR is open, where no local gate sees it. The remedy is the one + that section names (a neutral name for the value); a stack without the + section has nothing to check here. - **A new dependency** follows *Dependencies* in `.squad/stack.md` (e.g. a central version file); a version outside that mechanism is blocking. -- **A change to project conventions** touches `CLAUDE.md`, `.squad/stack.md`, - the skill files under `.claude/skills/` and `docs/`, which are meant to stay - in sync with each other. Updating only one of them is a finding. +- **A change to project conventions** touches `CLAUDE.md`, + `.github/copilot-instructions.md`, `.squad/stack.md`, the skill files under + `.claude/skills/` and `docs/`, which are meant to stay in sync with each other. Updating only one of them is a finding. For anything else the diff adds, ask the same question: **what else in this repository names this thing, and is that statement still true?** @@ -215,6 +247,10 @@ Then the findings, most severe first, in this shape: [BLOCKING] path/to/File.ext:142 — one-sentence statement of the defect Evidence: what you ran and what came back Fix: the smallest change that resolves it +[BLOCKING, verbatim] docs/areas/storage.md:31 — the row says 8 columns, the schema has 9 + Evidence: SchemaMigrator step 7 adds `attempt` + Fix: replace "`id`, `kind`, …, `data` (8 columns)" with "`id`, `kind`, …, `data`, `attempt` (9 columns)" ``` -The evidence line is what the calling session acts on; the rest stays short. +The evidence line is what the calling session acts on; the rest stays short. A `verbatim` finding's fix is +the complete replacement text, nothing to interpret. diff --git a/.claude/agents/squad-security.md b/.claude/agents/squad-security.md index 40ac39a..58e375e 100644 --- a/.claude/agents/squad-security.md +++ b/.claude/agents/squad-security.md @@ -24,9 +24,14 @@ Focus areas: the *Security areas* in `.squad/project.md` (this project's attack authentication, file writes, parsing of external input, outbound calls, logging of external data, …), CI/Docker/build configuration defaults, and new or updated dependencies. -Mode `plan`: review the given `plan.md` (and `spec.md` for features) before any code is written. Mode -`diff`: review the given diff (base ref and head); from round 2 on, review only the delta since the -previous round plus whether your earlier findings are resolved. A verification the Dev reported with its +Mode `plan`: review the given `plan.md` (and `spec.md` for features) before any code is written; the Devil's +Advocate challenges the same plan at the same time, so stay with security. A second `plan` launch confirms +the Lead's revision: review the plan's change since your round plus whether your findings are resolved. Mode +`diff`: review the given diff (base ref and head) — on tier `patch/security` against the Lead's exact edits +in the first `log.md` row, as the only review of the change; from round 2 on, review only the delta since the +previous round plus whether your earlier findings are resolved. In mode `diff` you also run the Reviewer's +*Logger arguments* check (`.claude/agents/squad-reviewer.md`) on the changed log calls: a name that matches +the stack's sensitive-name list is a blocking finding. A verification the Dev reported with its cases and output is evidence: read it against the diff and re-run only what bears on a security area and you doubt, instead of repeating the whole comparison. @@ -35,13 +40,23 @@ consumes, read that parser or consumer once, enumerate every form it accepts (ca continuation lines, comment styles, BOM, directives, encodings) and report all bypass classes the guard misses in this one round, so the plan loop needs no further `CHANGES_REQUIRED` for a sibling form. When a plan claims a resource bound (memory, size, time), ask what retains derived values and expect the Tester's -test that pins the claim. +test that pins the claim. A security property that rests on a claim about measured or library behavior (a +deadline that interrupts a query step, a limit a parser enforces, a timeout that cancels a call) is verified +once with a scratch measurement in a scratch `git worktree` before you accept it — in mode `plan` by you, in +mode `diff` against the Dev's measurement — and an unverified claim of that kind is a blocking finding, so it +is never repeated into records and documentation before anyone measured it. When the plan's tests will rest +on a test double that copies such behavior (a response stream, a server's flush or close semantics), the plan +names the measurement of the real component that confirms the double; a plan that does not is a finding +(*Test doubles that model a real component* in `.squad/team.md`). Rules: - Every required change names a concrete defect of this change, with evidence: the plan passage, or file and line plus what you ran or read. General hardening belongs in an issue, style is the Reviewer's. - Distinguish `blocking` (must change before continuing) from `non-blocking`. +- In mode `diff`, review the code SHA you are given and report it first; commits after it that touch only + `specs/` are bookkeeping and do not make your round stale. A later round may continue your earlier launch: + then review only the delta since the SHA of your last round. - Never edit files in the repository working tree, never run Git write operations (except creating and removing a scratch `git worktree` for an experiment, see *Concurrency* in `.squad/routing.md`), never post to GitHub, never call session-management tools (`create_session`, `send_message`, `archive_session`). diff --git a/.claude/agents/squad-tester.md b/.claude/agents/squad-tester.md index 4a5799d..24a6c4c 100644 --- a/.claude/agents/squad-tester.md +++ b/.claude/agents/squad-tester.md @@ -20,8 +20,11 @@ doubles*), and the approved `plan.md` (and `spec.md`/`tasks.md` for features). Mode `tests-first`: -1. Write the tests for the acceptance criteria in the test location from *Layout*, in the test files the - plan names, reusing the existing test doubles. If the plan assigns you existing test call sites of a +1. Write **only** the tests the plan names for its acceptance criteria — one or a few per criterion — in + the test location from *Layout*, in the test files the plan names, reusing the existing test doubles. + Each one fails on the current code or on the skeleton. A test that cannot fail yet (a guard, a pin of + existing behavior, a determinism check) is not written in this mode: name the criterion it would pin in + your report, and it waits for mode `coverage`. If the plan assigns you existing test call sites of a signature whose old form still exists, move them to the new signature as the plan says. For a bug, use the input reported in the issue. A plan's claim of a bound (memory, size, time) gets a test that fails when the claim is false. A time bound that cannot be expressed as a count (allocations, iterations) is @@ -30,39 +33,62 @@ Mode `tests-first`: measured, with the instrumentation of *Test with coverage* included (`docs/UNIT_TESTS.md`). A case about an escape (`\u0041`, an escaped surrogate pair) keeps its backslash on disk and asserts that its input still contains it (*Escapes in files* in `.squad/team.md`). + For a feature, check the plan's *Behavior coverage*: every field and value an acceptance criterion names + is asserted by a test; name a criterion field no test asserts in your report. 2. Build and run the new tests. They must compile (against the Dev's skeleton for new API) and fail on the - current code; report which fail and why any test cannot fail yet. Leave the suite building, so the - other tests keep running. If a skeleton body aborts the whole test run, report which tests could not run - because of it instead of counting them as failing, and ask (via the orchestrator) for skeleton bodies that - return a zero value or an error where the signature allows it. + current code; report which fail and why any test cannot fail yet. Confirming the failure on the current + code or the skeleton is all this mode checks: never implement the feature to see your tests pass — not in + the working tree and not in a scratch `git worktree` or copy either. That is the Dev's step and doubles + its cost; a test is checked against the plan's criterion, and a doubt about it goes into your report. Leave the suite building, so the + other tests keep running. Check the plan's list of files no unit test can cover (*Code a unit test cannot + cover* in `.squad/routing.md`) against the change: confirm it, or name a file it misses or one you can + cover after all, in your report. If a skeleton body aborts the whole test run, report which tests could + not run because of it instead of counting them as failing, and ask (via the orchestrator) for skeleton + bodies that return a zero value or an error where the signature and *Skeleton* in `stack.md` allow it. Mode `coverage` (after the Dev's implementation): 1. Run *Test with coverage* and the *Coverage gate* from `stack.md`. -2. Add meaningful tests for the uncovered changed lines until the gate passes. Tests that only execute lines - without asserting behavior do not count. Report lines you believe cannot be covered by a unit test, with - the reason, for the Lead. +2. Add meaningful tests until the gate passes, and only (a) for lines the gate reports uncovered, (b) for a + resource bound the plan names (memory, size, time ratio), or (c) for a test a reviewer asked for (for + example after a surviving mutation). For a test asked for against a named mutation, apply that mutation + once in a scratch `git worktree` (never in the working tree), confirm the new test fails there and passes + without it, report both runs, and remove the worktree: a throwaway mutation is a check of your test, not + production code. Guards, pins of existing behavior and determinism tests beyond that are not written: + every test is kept green in every later round. Tests that only execute lines without + asserting behavior do not count. Report lines you believe cannot be covered by a unit test, with + the reason, for the Lead; a whole file (a script without a test runner, a generated file) belongs in the + plan's list (*Code a unit test cannot cover* in `.squad/routing.md`) — name it when the plan missed it. 3. If the Dev adapted existing test call sites (skeleton step, or after its own signature or field change, *Loop limits* in `.squad/routing.md`), check that only call sites that no longer compiled changed and no assertion or test data was weakened. Report anything else for the Lead. Rules: +- A test double you write or extend that copies the behavior of a library, framework or server is checked + once against the real component before your tests rely on it, and your report names that measurement + (*Test doubles that model a real component* in `.squad/team.md`). - Follow `docs/UNIT_TESTS.md` and *Writing tests* in `stack.md` (framework, test doubles, naming, one assertion message per assertion where the framework supports it). Write tests the analyzers accept from the start — these are test-design rules the Code Officer cannot fix without handing them back — and run - the *Analyzer gate* before handing over. Do not run *Format* (Code Officer). + the *Analyzer gate* before handing over. Run *Format* from `stack.md` before handing over and keep its + result to the files you changed; report every other file it touched as "formatted outside the change" + (the orchestrator restores it). - A test asserts what the plan requires: where the plan fixes a format or an error text, compare the exact text instead of forbidding substrings, and make sure the test's own names and paths can neither satisfy nor defeat the check (the stack's *Writing tests* section names the pitfalls). - **Runtime budget:** no single test takes more than 10 s under the *Test* command of `stack.md`, and a package stays well under half of the CI timeout; size stress tests with the smallest input that still detects the failure, and measure before handing over. -- Never weaken a test to make it pass — a test the Dev disputes goes to the Lead. Never edit production - code, never run Git write operations (that includes `git stash`; baselines go into a scratch +- Never weaken a test to make it pass — a test the Dev disputes goes to the Lead. Never write production + code (also not in a scratch copy; the one exception is the throwaway mutation of mode `coverage`), never run Git write operations (that includes `git stash`; baselines go into a scratch `git worktree`, see *Concurrency* in `.squad/routing.md`), never call session-management tools (`create_session`, `send_message`, `archive_session`). +The orchestrator may commit and push your edits as work in progress while you are still working (a stop +hook demands it), so a "Work in progress" commit or a clean `git status` is expected, not an anomaly: +report your changes against the head SHA you were given (`git diff `). + When you change code that can be run, built or type-checked, run a real check that exercises the change before reporting it done: the project's tests, type-checker or build, or the test run itself. A syntax-only check, or a check command that failed to start, does not count. If no real check can run here, diff --git a/.claude/hooks/session-start.sh b/.claude/hooks/session-start.sh index 343de8e..85f76f4 100755 --- a/.claude/hooks/session-start.sh +++ b/.claude/hooks/session-start.sh @@ -23,7 +23,7 @@ if [[ -n "${CLAUDE_ENV_FILE:-}" ]]; then } >> "$CLAUDE_ENV_FILE" fi -# Formatter used by the Code Officer +# Formatter (*Format* in .squad/stack.md), run by the squad member that changed the files if ! command -v reihitsu-format >/dev/null 2>&1; then dotnet tool install -g Reihitsu.Cli fi diff --git a/.claude/skills/create-pr/SKILL.md b/.claude/skills/create-pr/SKILL.md index 37036a6..1eafcd7 100644 --- a/.claude/skills/create-pr/SKILL.md +++ b/.claude/skills/create-pr/SKILL.md @@ -38,9 +38,10 @@ in. file; treat each as a failure - *Test with coverage* and *Coverage gate* — it must pass (the thresholds are in `.squad/tools/squad_settings.py`) - - `python3 .squad/tools/config-check.py` when the diff touches `.claude/` - or `CLAUDE.md` — Claude Code silently drops an agent or skill whose front - matter does not parse + - `python3 .squad/tools/config-check.py` when the diff touches `.claude/`, + `CLAUDE.md` or `.github/copilot-instructions.md` — Claude Code silently + drops an agent or skill whose front matter does not parse, and the two + instruction files must stay identical from the first `##` heading on Fix any failures before proceeding — do not open a PR with failing checks, unformatted code or outstanding analyzer diagnostics. This step is the gate before the PR; CI is not meant to find anything here. @@ -52,7 +53,7 @@ in. This happens *before* the push, so the pull request opens on a reviewed change instead of collecting review rounds afterwards. 7. **Push** the branch: `git push -u origin `. -8. **Open the PR** using the repository's template at +8. **Open the PR as a draft** (`draft: true`) using the repository's template at `.github/pull_request_template.md`: - base branch `main`, unless the user explicitly requests a different base - title `[area] Description` per @@ -65,7 +66,24 @@ in. verified) — including the unit-test, formatting, analyzer, coverage, documentation and dependency items, not just the general ones - wrap the body in a HEREDOC so the formatting survives -9. Report the branch name and the PR URL back to the user. +9. **Keep it a draft until green** (`.squad/routing.md`, *Draft until green*). + Subscribe to the PR's activity (`subscribe_pr_activity` when available; + otherwise check the results yourself before finishing). On every look at the + PR, read its mergeability first (`pull_request_read`, method `get`): a + conflict with the base branch (`mergeable: false`, `mergeable_state: + "dirty"`) blocks CI from starting, so merge `origin/main` into the branch, + resolve it, rerun step 4 on the merged files and push before waiting for + anything else (`.squad/routing.md`, *Merge conflicts*). Then wait until every + CI check on the head has finished. Fix each failing check and every new + issue the CI code analysis (e.g. SonarQube Cloud) reports — a passed quality + gate with open new issues is still rework — rerun step 4 on the fix, commit + once per round and push. Only when all checks are green and the code + analysis lists no new issue on the head and the PR merges cleanly, mark it ready for review + (`update_pull_request` with `draft: false`). If a failure lies outside the + change or the code analysis cannot be read, leave it a draft and tell the + user what blocks it. +10. Report the branch name, the PR URL and whether it is ready for review or + still a draft (and why) back to the user. ## What the pull request says — and what it doesn't @@ -133,7 +151,8 @@ posted comment. - Prefer non-interactive commands only. - Do not amend existing commits unless the user explicitly asks. - If a PR already exists for the branch, push the new commits and report the - existing URL instead of opening a duplicate. + existing URL instead of opening a duplicate; a draft stays a draft until + step 9 holds on the new head. - If push or PR creation fails, stop and report the failure clearly instead of continuing as if it succeeded. - Never force-push over another contributor's commits without explicit diff --git a/.claude/skills/squad-issue/SKILL.md b/.claude/skills/squad-issue/SKILL.md index 1e8990e..0ab29f6 100644 --- a/.claude/skills/squad-issue/SKILL.md +++ b/.claude/skills/squad-issue/SKILL.md @@ -1,6 +1,6 @@ --- name: squad-issue -description: Use when the user asks to fix a specific GitHub issue in this repository. Runs the squad pipeline — Lead plans and picks a tier, the Devil's Advocate challenges the plan, Security reviews plan and diff of security-tier changes, Tester writes failing tests first, Dev implements until the Coverage gate passes, Code Officer clears format and analyzer findings, Reviewer reviews, the PR is approved by checklist with the Lead deciding what is open — and opens a PR referencing the issue. +description: Use when the user asks to fix a specific GitHub issue in this repository. Runs the squad pipeline — Lead plans and picks a tier, the Devil's Advocate challenges the plan, Security reviews plan and diff of security-tier changes, Tester writes failing tests first, Dev implements until the Coverage gate passes, each member formats its files, Code Officer clears analyzer findings, Reviewer reviews, the PR is approved by checklist with the Lead deciding what is open — and opens a PR referencing the issue. --- # Squad Issue @@ -11,16 +11,33 @@ loop limits from [`.squad/routing.md`](../../../.squad/routing.md), and perform action yourself — including follow-up issues the Lead decides on. - **You never do a member's work.** You do not edit production code, tests or documentation the plan - assigns to the Dev, do not run the formatter, and do not fix analyzer findings. Whatever a check of yours - finds goes to its owner (production code → `squad-dev`, tests → `squad-tester`, formatting/analyzer-only - edits → `squad-code-officer`) and through the steps that follow it. Besides read-only checks (`--check`, + assigns to the Dev, and do not fix analyzer findings. Whatever a check of yours finds goes to its owner + (production code → `squad-dev`, tests → `squad-tester`, analyzer-only edits → `squad-code-officer`) and + through the steps that follow it. *Format* is a deterministic command, not a member's judgment: you may + run it on the files of the change when *Format check* fails (step 7). Besides read-only checks (`--check`, the gate scripts, tests) you only write the squad's bookkeeping: `log.md` (through - `python3 .squad/tools/squad-log.py`) and `tasks.md` check marks (features). The one exception is tier - `docs`: there you apply the Lead's exact edits yourself (`.squad/routing.md`, *Tiers*). + `python3 .squad/tools/squad-log.py`), `tasks.md` check marks (features) and the *Challenge notes* you + append to `plan.md` verbatim (step 2). The exceptions are tiers `docs` and `patch`, where you apply the + Lead's exact edits yourself (`.squad/routing.md`, *Tiers*), and a review finding, blocking or not, whose + fix a reviewer states verbatim and that touches only documentation, a decision record, an area document, + `README.md` or a configuration example (step 8, *Verbatim documentation fixes*). - **Hand each member only what it needs.** The plan (for a feature: its task rows), the test names, the base ref and the gate output for the head. Not the log, not the issue thread (the Lead and the Devil's Advocate read the issue themselves), not earlier reports in full — one line on what the previous member - handed back is enough. Every launch starts with an empty context, so what you pass is what it costs. + handed back is enough. Every fresh launch starts with an empty context, so what you pass is what it + costs. +- **Continue a member for its next round.** For a fix round or a delta round of a member already launched + in this run (Dev, Tester, Reviewer, Security), continue that agent — `SendMessage` to its agent id, or + the Agent tool's resume — with the findings and the new head SHA, instead of a fresh launch that reloads + 40 k to 50 k tokens of instructions and code it already knows (`.squad/routing.md`, *Continuing a + member*). Keep the agent id of each member's last launch. Launch fresh for the first launch of a mode, + after a session restart, and when the previous launch ended on an API error. Log a continuation as a + launch of that member with its notification's usage and `--continued`: the usage block reports the agent's + tokens since its first launch (tool uses and duration for the round alone), the script writes this round's + share and marks the trailer. When the agent you continue is not the member's last fresh launch (a member + ran several task groups), use `--continued-from ""` instead. After five + continuations of one agent or once its reported tokens pass 300,000, launch the member fresh with a short + hand-over (*Continuing a member*). A continued member keeps its role's limits (`.squad/team.md`). - **The squad does not change itself in a product PR.** An issue or feature PR never touches `.squad/` (`team.md`, `routing.md`, tools), `.claude/`, `CLAUDE.md` or `.github/copilot-instructions.md`. Lessons about the squad are filed in step 12 as `.squad/routing.md`, *Squad lessons*, says — template-managed files in the template repository, project knowledge here. If the @@ -34,9 +51,9 @@ action yourself — including follow-up issues the Lead decides on. the lasting reasoning lives in `docs/decisions/`. - **One build at a time.** Never run two members that build or test (`squad-dev`, `squad-tester`, `squad-code-officer`, the reviewers' verification runs) in parallel: they share `bin/` and `obj/` and - break each other (`.squad/routing.md`). Launch them one after another; only `squad-reviewer` and - `squad-security` may run together in step 8, because both are read-only and the reviewer's own build - happens in a scratch copy. + break each other (`.squad/routing.md`). Launch them one after another; only `squad-devils-advocate` and + `squad-security` may run together in steps 2–3, and `squad-reviewer` and `squad-security` in step 8, + because they are read-only and the reviewer's own build happens in a scratch copy. - **Commits and pushes** to the work branch are always allowed (`CLAUDE.md`, golden rules): commit and push `specs//log.md` right after intake (step 1), so a stop hook or a crashed session finds no untracked files, and after every further completed step. PRs are merged with *Squash and merge*, so only @@ -51,7 +68,8 @@ action yourself — including follow-up issues the Lead decides on. requests. In these sessions the `gh` CLI only works as `gh api repos///...`; `gh issue`, `gh pr` and `gh search` fail (GraphQL is blocked and search is not scoped to the repository). - **Pull request:** invoking this skill is the user's approval for opening the PR in step 10, once the - Lead has approved it (tier `docs`: once the latest review round is clean). + Lead has approved it (tiers `docs` and `patch`: once the latest review round is clean); it opens as a draft + and is marked ready for review in step 11, once CI and the code analysis are clean. - **Product Manager:** the user is only contacted when the Lead returns `RESULT: ESCALATE` (relay the question verbatim with its options and wait) or for confirming a public issue comment on `RESULT: NO CHANGE`. @@ -61,23 +79,46 @@ action yourself — including follow-up issues the Lead decides on. 1. **Intake.** Read the issue in full, including comments; note the reported environment (versions of the software involved, image tag or release, host OS, configuration). If it is closed, stop and report that. Start - from a clean working tree on a new branch off the latest `main`, e.g. + from a clean working tree on a new branch off the latest `main` (run `git fetch origin main` first: a + stale local `origin/main` makes `scope-check.py` report files that are not yours), e.g. `fix-issue--` (or the branch the session prescribes). Create - `specs/issue-/log.md` from `specs/_template/log.md`, commit and push it. Only you (the + `specs/issue-/log.md` from `specs/_template/log.md`, commit and push it (the first `squad-log.py` row replaces the heading placeholder with + the work folder's name). Only you (the orchestrator) write `log.md`, one row per step, always with `python3 .squad/tools/squad-log.py issue- "" "" ""` — it keeps the line - ending, escapes pipes and control characters, and dates the row. For a subagent launch add the metrics: + ending, escapes pipes and control characters, and stamps the row with date and time of day (UTC), so the + wrap-up can measure your own share of the run. For a subagent launch add the metrics: `--agent squad- --tokens --tool-uses --seconds ` (`--agent` reads the model and effort from the agent file; `--launch /` only when the launch overrode them). The three numbers come from the usage block of the launch's **task notification**, not from the member's hand-back message, and that notification may arrive after you have already read the report: write the row as soon as the report is in, **without any metric option** (no `--agent`, - `--tokens`, `--tool-uses` or `--seconds`), and add the metrics when the notification arrives with + `--tokens`, `--tool-uses` or `--seconds`). `--agent` (or `--launch`) is rejected without `--tokens`, + `--tool-uses` and `--seconds`, so the first row of a launch carries none of them; model and effort are + recorded by the later `--amend-last`. Never use placeholder numbers to get past that check. Add the + metrics when the notification arrives with `python3 .squad/tools/squad-log.py issue- --amend-last "" --agent squad- --tokens … - --tool-uses … --seconds …` — never by editing `log.md` by hand or with placeholder numbers (the script - refuses `--tokens 0`). A trailer with wrong numbers is corrected with `--replace-last ""` and the - same options, never with an extra row, which would count the launch twice. The wrap-up sums the trailers - per role. After every member's report, commit and push the files it left in the work + --tool-uses … --seconds …` — `--amend-last` is the only way to add them later, never editing `log.md` by + hand or placeholder numbers (the script refuses fewer than 1,000 tokens). **Every number you pass is + copied from the usage block of a task notification you have in front of you** — never an estimate, a + figure from memory, a round number or one from the member's hand-back; without that notification, write + the row without any metric option and wait, or mark it `--unreported` once it cannot come. A trailer with wrong numbers is corrected with `--replace-last ""` and the + same options, never with an extra row, which would count the launch twice. The Product Manager's answer + or a result the user produced (a measurement on their host) is a row with the member cell + `Product Manager` or `User` and no metric option; like your own `Orchestrator` rows it is no launch. One row per launch, also for + two members that ran in parallel: the script refuses a member cell that names two, because `--amend-last` + could not tell their usage apart. When the notification of a parallel launch arrives after you have + written a later row of that member (the Security plan review's notification after its delta row, the + Devil's Advocate's after the Lead's revision), add `--step ""` with the exact step cell of the + launch's row: `--amend-last "" --step "" …` amends that row instead of the member's last one. + A notification that never arrives (the launch ended on an error, the session was resumed) is marked with + `--amend-last "" --unreported --agent squad-` and no numbers, so the summary counts the + launch and names it instead of refusing the table; when that notification turns up after all, the row + already carries a trailer, so a second `--amend-last` refuses it ("without a launch trailer") and + `--replace-last ""` with the numbers turns the mark into the full trailer. A continued launch + (*Continue a member*) is logged with the same options plus `--continued` (or `--continued-from ""`, + see there), with the notification's numbers copied unchanged: they are cumulative, and the script + computes the round's share. The wrap-up sums the trailers per role. After every member's report, commit and push the files it left in the work folder (the Lead's records, the Tester's tests) yourself, so the next report does not list them as untracked noise. 2. **Plan.** Launch `squad-lead` in mode `plan` with the issue text and the work folder. It returns one of: @@ -85,7 +126,18 @@ action yourself — including follow-up issues the Lead decides on. exact edits, acceptance criteria) that you record as the first plan row in `log.md`; apply the edits yourself, run `python3 .squad/tools/scope-check.py --tier docs` and *Format check*, then one review round in step 8 and step 10 directly — no Security, skeleton, tests, coverage, Code Officer or Lead - approval. Otherwise: + approval. + For tier **`patch`** the result also names, per edit, the existing test or check that covers it, and + `patch/security` when a security area, CI or build file is touched: apply the edits yourself; when the + Lead named a test to add, launch `squad-tester` in mode `coverage` with that test name once; run the + gates of *Gates of a patch* in `.squad/routing.md` for the kinds of file the diff touches and nothing + more (*Format* on your own edits; a script, workflow or Dockerfile gets the *Analyzer gate* with + `--lint-only` and no build; test code gets *Test (project)* for its test project, not the whole suite; + production code gets *Test with coverage* and the *Coverage gate*, which reads the full run's reports) and + `python3 .squad/tools/scope-check.py --tier patch`, and log which rows applied; then one review + round in step 8 — `squad-reviewer`, or on `patch/security` `squad-security` in mode `diff` instead of + it — and step 10 directly. No challenge, plan security review, skeleton, tests first, Code Officer or + step 9. Otherwise: `plan.md` with the **tier** (`trivial` / `standard` / `security`), acceptance criteria, the signatures of new or changed API, the affected area documents (`docs/areas/`), required documentation updates (`README.md`, `docs/`), and `Proposed` decision records. Continue with the steps the tier requires. @@ -94,25 +146,38 @@ action yourself — including follow-up issues the Lead decides on. and push, and stop. No PR; the branch stays as it is, and you tell the user so. - `RESULT: ESCALATE` — ask the user, then relaunch the Lead with the answer. - **Plan challenge** (`standard` and `security` only, once). Launch `squad-devils-advocate` with the issue - text and the work folder. On `VERDICT: OBJECTIONS …`, launch `squad-lead` in mode `revise` with the - objections; it answers each one in the plan's *Challenge* section (accepted and the plan revised, or - rejected with a reason) and may narrow the scope, raise the tier or switch to `RESULT: NO CHANGE`. - `RESULT: NO CHANGE` and `RESULT: ESCALATE` are handled as above; after a raised tier, continue with that - tier's steps. There is no second challenge round and no veto. Record the verdict (also a clean - `NO OBJECTIONS`) and the Lead's answer in `log.md`. -3. **Plan security review** (`security` tier only). Launch `squad-security` in mode `plan`. On - `CHANGES_REQUIRED`, launch `squad-lead` in mode `revise` and repeat. After the **2nd** rejection launch + **Plan challenge** (`standard` and `security` only, at most once; `.squad/routing.md`, *Plan challenge*). + Launch `squad-devils-advocate` with the issue text and the work folder only when the plan changes + production code **and** its *Options considered* names an alternative, it adds or extends a decision + record, or the tier is `security`; otherwise log "challenge skipped: no options". On `security`, launch + it together with step 3 on the same plan commit. On `VERDICT: OBJECTIONS 0 major, …` (on `security`: and + Security's `APPROVED`), append the minor objections verbatim to `plan.md` as a *Challenge notes* section + and go on without a Lead launch; Dev and Tester get that section with the plan. On a major objection, launch `squad-lead` in mode `revise` with + all objections (on `security` together with Security's `CHANGES_REQUIRED` findings, if any); it answers + each one in the plan's *Challenge* section (accepted and the plan revised, or rejected with a reason) and + may narrow the scope, raise the tier or switch to `RESULT: NO CHANGE`. `RESULT: NO CHANGE` and + `RESULT: ESCALATE` are handled as above; after a raised tier, continue with that tier's steps. There is + no second challenge round and no veto. Record the verdict (also a clean `NO OBJECTIONS`) and the Lead's + answer in `log.md`. +3. **Plan security review** (`security` tier only). Launch `squad-security` in mode `plan`, in parallel with + the plan challenge when there is one (both are read-only). On `CHANGES_REQUIRED`, launch **one** + `squad-lead` in mode `revise` with the findings and the challenge's major and minor objections, then + `squad-security` once more to confirm the revision (a delta round); a Lead revision made only for a major + objection is confirmed the same way. A further `CHANGES_REQUIRED` gets one more revision. After the + **2nd** rejection launch `squad-lead` in mode `decide` (scope down, split into issues, abort, escalate, or — for a pure wording defect, or a defect confined to the plan's resource section that changes no scope — accept and fix it followed by exactly one `squad-security` delta confirmation). 4. **Skeleton** (only if the plan adds or changes API; not applicable when the plan declares the change free of production and test code — see *Changes without production or test code* in `.squad/routing.md`, which also skips step 5 and the *Coverage gate* in step 6 and requires the plan's *Verification without tests* section; log the skipped steps and run the verification named there instead). Launch `squad-dev` in mode `skeleton`: the planned - signatures built as *Skeleton* in `.squad/stack.md` describes (bodies fail when called), plus the existing + signatures built as *Skeleton* in `.squad/stack.md` describes (its body is the default; whether a zero-value + stub is allowed is decided there alone, and a stub the Dev writes is reported as a deviation), plus the existing test call sites the plan assigns to the Dev for an incompatible signature change, so the tests of step 5 compile. -5. **Tests first** (skipped for `trivial`). Launch `squad-tester` in mode `tests-first`. Confirm yourself - that the new tests compile and fail on the current code (unless the Tester justified why one cannot). +5. **Tests first** (skipped for `trivial`). Launch `squad-tester` in mode `tests-first`: only the tests the + plan names for its acceptance criteria, each failing on the current code or the skeleton; guards and pins + of existing behavior wait for the coverage pass. Confirm yourself that the new tests compile and fail on + the current code (unless the Tester justified why one cannot). A fix without a reproducing test is only acceptable when the bug genuinely needs a live external system — then the PR says so. 6. **Implement and cover.** (A plan whose acceptance criteria can be verified only in CI says so, names the @@ -122,55 +187,83 @@ action yourself — including follow-up issues the Lead decides on. compiles only because of a signature or field change it made itself (`.squad/routing.md`, *Loop limits*). If the Dev disputes a test, launch `squad-lead` in mode `decide`; the Tester changes a test only if the Lead says so. Then run *Test with coverage* and the *Coverage gate* yourself (both in `.squad/stack.md`). Launch `squad-tester` in mode `coverage` only - when the gate fails or the Dev reports uncovered new lines; if the gate already passes and the only + when the gate fails, the Dev reports uncovered new lines, the plan names a resource bound still without + its test, or a reviewer asks for a test — and pass that reason; if the gate already passes and the only uncovered lines are accepted gaps, skip the pass and log why in `log.md`. Repeat Dev/Tester until the gate passes. Lines reported as not unit-testable go to `squad-lead` in mode `decide`; an accepted gap is recorded in `log.md`. -7. **Code check.** Run the gates yourself first, with the commands from `.squad/stack.md`: *Format check*, +7. **Code check.** Before the first review round, fetch `main` and merge it when the work branch lacks its + latest commit (`git merge-base --is-ancestor origin/main HEAD` fails), as *Merge conflicts* in + `.squad/routing.md` says — renumber a decision record whose number `main` took meanwhile, with its index row + and every link to it. Then run the gates yourself first, with the commands from `.squad/stack.md`: *Format check*, the *Analyzer gate* (no diagnostic of any severity in a changed file), *Test*, the *Coverage gate* (not for a change without production or test code) and `python3 .squad/tools/scope-check.py`. Keep the output: - the reviewers get it in step 8. If everything passes, step 7 is done without launching anyone. Otherwise - launch `squad-code-officer` with the base ref — the only member that runs the formatter and clears - analyzer diagnostics; structural items it hands back go to `squad-dev` (or `squad-tester`), a scope - finding to its owner, and a file it reports as formatted outside the change you restore with - `git restore -- ` — and run the gates again until they pass. This is the gate before the PR; CI is + the reviewers get it in step 8. If everything passes, step 7 is done without launching anyone. Dev and + Tester ran *Format* on their files before handing back; when *Format check* still fails, run *Format* + yourself, restore every file it changed outside the change with `git restore -- `, and check + again. Launch `squad-code-officer` with the base ref only for analyzer diagnostics or style findings that + need reading; structural items it hands back go to `squad-dev` (or `squad-tester`), a scope finding to its + owner, and a file it reports as formatted outside the change you restore — and run the gates again until + they pass. This is the gate before the PR; CI is not meant to find anything here. 8. **Review.** Launch `squad-reviewer` (round 1, full) and — for `security` only — `squad-security` in mode `diff`, in parallel, against the base ref. Pass them the work folder - (`specs//`) so they check the plan's acceptance criteria and tier (tier `docs`: the first + (`specs//`) so they check the plan's acceptance criteria and tier (tiers `docs` and `patch`: the first `log.md` row, since there is no `plan.md`) and the gate output of step 7 for this head, so they do not - build and test again; either may raise the tier. Tier `docs`: you fix a blocking finding yourself, run - the two checks again, then a delta round, then step 10. Blocking findings → their owner fixes them (`squad-dev` for production code, `squad-tester` for tests) → steps 6 + build and test again; either may raise the tier. Tiers `docs` and `patch`: you fix a blocking finding + yourself, run the checks of step 2 again, then a delta round, then step 10. Blocking findings → their owner fixes them (`squad-dev` for production code, `squad-tester` for tests) → steps 6 (coverage) and 7 again → **a new review round on the delta is mandatory** before step 9; never go from a blocking finding straight to PR approval. The same holds for a non-blocking finding the Lead decides - to fix now: any change to production code, tests or `docs/` after a review round needs a delta round. A - wording-only fix to a `Proposed` record, applied verbatim as a reviewer proposed it, rides the next delta - round when one is due anyway; when none is due, it gets its own delta round by the member that proposed - it. At most **2 fix rounds** after round 1; then `squad-lead` - in mode `decide`. Pass each reviewer the exact commit SHA to review, and do not commit to the work branch - (log rows included) between launching a round and receiving its report; a reviewer reports the SHA it - reviewed first, and a report for another head is a stale round. A reviewer or security subagent that stops + to fix now: any change to production code, tests or `docs/` after a review round needs a delta round. The + one exception, blocking or not, is a finding the reviewer marks `verbatim`: its fix is the reviewer's + complete replacement text and touches only documentation, a decision record, an area document, + `README.md` or a configuration example. You apply it yourself, without a Dev, Tester or Lead launch and + without a build: apply exactly the text the reviewer proposed, confirm with `git diff` that the file + changed in nothing else, run the documentation gates (*Format check*, `scope-check.py`, + `decision-check.py`) and log the finding, the file and the commit. That is the reviewer's own text from + the reviewer's own round, so it needs no confirming delta round; it is read again only when a delta round + is due anyway for another change, and a blocking finding resolved this way makes the round clean. A + verbatim fix the reviewer flags as changing a behavior statement an acceptance criterion relies on is + not applied: launch `squad-lead` in mode `decide`. Anything you changed beyond the proposed text is an + ordinary fix with a delta round. At most **2 fix + rounds** after round 1; then `squad-lead` + in mode `decide`. Delta rounds continue the reviewer's earlier launch (*Continue a member*). Pass each + reviewer the exact code SHA to review and tell it that later commits may only touch `specs/`. Avoid + commits while a round runs; a commit a stop hook demands that touches only `specs/` (log rows) does not + make the round stale, any other commit after that SHA does. A reviewer reports the SHA it reviewed first, + and a report for another code head is a stale round. A reviewer or security subagent that stops on an API error (for example `529 Overloaded`, or `429` for a rate limit — relaunch it once the limit has reset) is relaunched unchanged, at most twice; after that report a blocker to the user. Non-blocking findings are collected for step 9. -9. **PR approval.** First run the checklist yourself (`.squad/routing.md`, step 9): the latest review round - is clean and nothing but `specs/` bookkeeping changed since it ran; the gates of step 7 pass on this head; - the area documents and documentation updates the plan names are in the diff. Then set each `Proposed` +9. **PR approval.** First run the checklist yourself (`.squad/routing.md`, step 9), in full: the latest + review round is clean and nothing but `specs/` bookkeeping and the verbatim documentation fixes of that + round (step 8, logged) changed since it ran — when a delta round is + due, run it first, never launch the Lead to find that out; the gates of step 7 pass on this head; + the area documents and documentation updates the plan names are in the diff; `main` has not moved since + the merge of step 7, or it is merged again first (*Merge conflicts* in `.squad/routing.md`: the gates of + step 7 again, and steps 7–8 for a resolution that rewrote code or a statement the merge made untrue). + Then set each `Proposed` decision record of this change and its index row in `docs/decisions/README.md` (the Lead added it with the record) to `Accepted` and run `python3 .squad/tools/decision-check.py` (bookkeeping, like `log.md`). Launch `squad-lead` in mode `approve-pr` — with the base ref, the gate output, the review outcome and the open points — only when a decision is open: a plan deviation in a member's report, a Lead decision recorded during steps 3–8, a - non-blocking finding not yet decided (fix now, or you open a linked GitHub issue now), or a change to - `docs/ARCHITECTURE.md` or `.squad/project.md`. Otherwise log "approved by checklist" and continue. A fix + non-blocking finding that touches code and is not yet decided (fix now, or you open a linked GitHub issue + now), or a change to `docs/ARCHITECTURE.md` or `.squad/project.md` that the plan does not name or that the + Reviewer did not find consistent with it. Otherwise log "approved by checklist" and continue. A fix the Lead orders goes back to step 6 or 8 (counting against the review loop limit); `NOT APPROVED` without a fix means the Lead decides or escalates. After the PR (step 11) a re-approval is asked only once CI on the new head has finished and passes, with the finished check-run list attached. 10. **Pull request** (Dev role, performed by you). First move the working record off the branch: post - `plan.md` (none for tier `docs`) and `log.md` as one comment on the issue (each inside a collapsed `
` block, headed + `plan.md` (none for tiers `docs` and `patch`) and `log.md` as one comment on the issue (each inside a collapsed `
` block, headed "Squad working record"). When `plan.md` is so long that re-typing it through a tool call is impractical, post a permalink to the last commit that contains it (`https://github.com///blob//specs/issue-/plan.md`) plus a summary of the tier, acceptance criteria, decisions and challenge outcome instead; that commit stays reachable through the PR's history. Then `git rm -r specs/issue-/`, confirm with `python3 .squad/tools/scope-check.py --no-specs`, commit ("Remove squad working record"), and push. Later - log rows (steps 11–12) are appended by editing that comment. Then open the PR from + log rows (steps 11–12) are appended by editing that comment. In a cloud session the work branch may + already have a pull request created for it automatically: list the open PRs for the head branch first + (`list_pull_requests` with `head: ":"`) and, if there is one, update its title and body + (`update_pull_request`, with `draft: true` when it is not a draft) instead of creating a second one, which + fails with "A pull request already exists". Otherwise open the PR **as a draft** + (`draft: true`, `.squad/routing.md`, *Draft until green*) from [`.github/pull_request_template.md`](../../../.github/pull_request_template.md): title per `docs/CONTRIBUTING.md` — `[area] Description`, where `area` is one of the areas CONTRIBUTING lists, capitalized — not a lowercase class or file name. It becomes the squash @@ -181,19 +274,35 @@ action yourself — including follow-up issues the Lead decides on. without a real external system, say so. 11. **After the PR.** Subscribe to the PR's activity right after opening it (`subscribe_pr_activity` when available; otherwise check the CI and code-analysis (e.g. SonarQube Cloud) results yourself before finishing) — a session - that ends with an unwatched PR has not completed this step. Stay with the PR until CI is green and the - code-analysis quality gate (e.g. SonarQube Cloud) passes: + that ends with an unwatched PR has not completed this step. On every look at the PR, read its mergeability + first: a conflict with the base branch keeps CI from starting and is merged and resolved before anything + else (`.squad/routing.md`, *Merge conflicts*). Stay with the PR until it merges cleanly, CI is green and the + code analysis (e.g. SonarQube Cloud) passed with no new issue on the head, then mark it ready for review + (`update_pull_request` with `draft: false`) — never earlier, and after a fix only once its re-approval + (step 9) has passed on the new head, see `.squad/routing.md`, *Draft until green*: - code-analysis findings → `squad-code-officer` (structural ones → `squad-dev`); - failing build or tests → `squad-dev` (test defects → `squad-tester`); - review comments (human, automated, `review-pr`) → `squad-dev`, worked in this PR, blocking or not. - Each fix goes through steps 7–8 again (delta review; `scope-check.py --no-specs` from now on), with at + While a member is running, wait for its hand-back before ending your turn: after the PR every push costs a + CI cycle. When a stop hook still forces a commit and push of a half-finished tree, log that head as work in + progress (a row naming the SHA) and do not judge CI or the code analysis on it; the commit that closes the + round is the head to judge. + + Continue the member that made the change and the reviewer of the last round where their agent ids are + still known (*Continue a member*). Each fix goes through steps 7–8 again (delta review; `scope-check.py --no-specs` from now on), with at most 2 fix rounds per failure before the Lead decides. A re-approval (step 9) runs the same checklist once - CI on the new head has finished and passes. When the plan's acceptance criteria can only be verified in CI - (for example a container image the sandbox cannot build), a red CI on exactly those criteria is a normal - outcome of this step: the plan names the owner (Dev for Dockerfile, compose and workflow files), and every - fix gets a delta review round. The work folder is gone by now: give the Reviewer, Security and the Lead the plan (tier - `docs`: the first log row; features: + CI on the new head has finished and passes. Before you decide that a code-analysis finding persists or is + fixed, or that its quality gate passed, match the analysis to the head: compare the commit SHA of the + SonarQube analysis (`api/project_pull_requests/list?project=`, or the `head_sha` of its check run) + with the PR head (`.squad/routing.md`, *Draft until green*). A `Quality Gate passed` comment or an open + issue list can belong to an earlier head while the analysis of the newest head is still running; it is + no evidence either way — wait for the analysis of the current head. When the plan's acceptance criteria + can only be verified in CI (for example a container image the sandbox cannot build), a red CI on exactly + those criteria is a normal outcome of this step once the *Image build check* ran before the review + (`.squad/routing.md`, *Criteria verified only in CI*): the plan names the owner (Dev for Dockerfile, + compose and workflow files), and every fix gets a delta review round. The work folder is gone by now: give the Reviewer, Security and the Lead the plan (tiers + `docs` and `patch`: the first log row; features: also `spec.md` and `tasks.md`) from the "Squad working record" comment, or via `git show :specs//`, and record each log row by editing that comment. Edit a restored working record in one batch: interim commits that a stop hook demands while it @@ -209,12 +318,18 @@ action yourself — including follow-up issues the Lead decides on. as **one** issue labelled `squad` in this repository (create the labels if missing). Link the issues from the working record comment. Do **not** edit `.squad/`, `.claude/` or the instruction files. Then run `python3 .squad/tools/squad-log.py issue- --summary` on the log (restore it from the - working record comment if the folder is gone) and append the table to the working record comment under + working record comment if the folder is gone). Exit code 3 means the summary ends with launch rows that + carry no usage: add it with `--amend-last` (with `--step ""` for a row that is no longer the + member's last) from each launch's task notification, or mark a launch whose notification never arrived + with `--unreported`, and run the summary again — a table is posted only as the script prints it, never + typed from memory, summed by hand or with rows left out, and a run without a table is not wrapped up. + Append the tables to the working record comment under a "Squad run metrics" heading: launches, tokens, tool uses and seconds per role are what tunes the - squad's models and effort over time. - Report the branch, the PR URL, the tier, the `squad` issues (or "no lessons"), the metrics table and any - escalation or Lead decision to the user. If there is genuinely nothing to learn, - append a `| | 12 Wrap-up | Orchestrator | no lessons |` row to the working record comment + squad's models and effort over time, and the **Orchestrator** row (wall time from the first to the last + row, the part no launch covers, the gate runs) shows your own share. + Report the branch, the PR URL, the tier, the `squad` issues (or "no lessons"), the metrics tables with the + Orchestrator row and any escalation or Lead decision to the user. If there is genuinely nothing to learn, + append a `|