diff --git a/internal/templater/templater_test.go b/internal/templater/templater_test.go index fd0a00c..4711409 100644 --- a/internal/templater/templater_test.go +++ b/internal/templater/templater_test.go @@ -301,16 +301,21 @@ func TestSpecAuthorAgentShipped(t *testing.T) { } // Least-privilege tools: it writes spec artifacts, so Edit/Write/Bash are in, // but it must not approve or implement. - // effort is high (not medium): spec authoring is the SDD contract — EARS, - // the design boundary map, traceability — a judgment task, and the repo runs - // every judgment sub-agent at high (code-reviewer, implementer, - // security-reviewer). A shallow first draft pushes the real authoring back - // onto the opus reviewer via fix-lists, defeating the cost split. + // + // effort is medium, and it used to be high. The argument for high was that + // authoring is judgment — EARS, the boundary map, traceability — and a shallow + // draft pushes the real work back onto the opus reviewer as fix-lists, which + // defeats the cost split. What a measured run showed is that the split was + // already lost at the other end: of one feat's $4.69, $3.15 was sonnet, and + // this agent is dispatched three times per feat (requirements, design, tasks) + // against a SCAFFOLDED artifact whose shape the template already fixes. High + // effort was buying deliberation over a form that is largely filled in, three + // times over. If fix-lists grow after this, that is the signal to move it back. for _, want := range []string{ "name: spec-author", "tools: Read, Grep, Glob, Edit, Write, Bash", "model: sonnet", - "effort: high", + "effort: medium", } { if !strings.Contains(body, want) { t.Errorf("spec-author.md frontmatter missing %q", want) @@ -348,6 +353,19 @@ func TestSpecAuthorAgentEncodesDiscipline(t *testing.T) { t.Errorf("spec-author.md must not instruct approving its own phase: %q", bad) } } + // The fix loop is BOUNDED. "Re-validate until it returns 0" is an open loop, + // and the finding it cannot resolve — usually a tasks.md shape that does not + // match the spec's development_flow — is exactly the one it will not resolve on + // the fourth pass either. Each pass re-reads an 800-line spec to retry a fix it + // already tried, which costs more than the orchestrator spends deciding. + if strings.Contains(body, "re-validate until it returns 0") { + t.Error("spec-author.md must not instruct an unbounded validate/fix loop") + } + for _, want := range []string{"at most twice", "two rounds"} { + if !strings.Contains(body, want) { + t.Errorf("spec-author.md should bound the fix loop (%q)", want) + } + } } func TestImplementerAgentEncodesDiscipline(t *testing.T) { agents, err := AgentFiles(FS) @@ -576,3 +594,46 @@ func TestPlanSessionEntryCarriesTheLoopContract(t *testing.T) { } } } + +// TestAgentEffortLadder pins where each sub-agent's reasoning effort sits. +// +// The dial is not per-agent taste. It is dispatch frequency weighed against what +// being shallow costs, and the two pull in opposite directions: +// +// - `implementer` fires ~7 times per feat, but each dispatch is one small, +// already-bounded task with a cycle skill to follow. It stays high because it +// WRITES the code — a shallow implementation is the expensive kind. +// - `spec-author` fires 3 times per feat against a SCAFFOLDED artifact whose +// shape the template already fixes. Medium: deliberation over a form that is +// largely filled in, three times over, was not buying its cost. +// - `code-reviewer` / `security-reviewer` fire once or twice, and they are pure +// judgment. Lowering them saves almost nothing and risks the defect the gate +// exists to catch: one measured feat arrived "implemented" with 691 green +// tests and a drawer that never collapsed at any width — the ARIA and state +// were there, the layout was not. The review is what found it. +// - `quality-gate` runs commands and relays their real output; its own prompt +// says it decides nothing about the code. Low. +// +// Change a value here only with the reasoning that moved it, in this comment. +func TestAgentEffortLadder(t *testing.T) { + agents, err := AgentFiles(FS) + if err != nil { + t.Fatal(err) + } + for name, want := range map[string]string{ + "implementer.md": "effort: high", + "spec-author.md": "effort: medium", + "code-reviewer.md": "effort: high", + "security-reviewer.md": "effort: high", + "quality-gate.md": "effort: low", + } { + body, ok := agents[name] + if !ok { + t.Errorf("AgentFiles is missing %s", name) + continue + } + if !strings.Contains(body, want) { + t.Errorf("%s should carry %q", name, want) + } + } +} diff --git a/internal/templater/templates/agents/quality-gate.md.tmpl b/internal/templater/templates/agents/quality-gate.md.tmpl index 23165fc..2cf18c1 100644 --- a/internal/templater/templates/agents/quality-gate.md.tmpl +++ b/internal/templater/templates/agents/quality-gate.md.tmpl @@ -3,7 +3,7 @@ name: quality-gate description: Runs the feat-exit verification gate — the plan's Quality Gate commands plus the Tier-3 test-report with coverage — and reports pass/fail with the real output. Use at feat exit, once every implementer has finished, dispatched alongside code-reviewer and security-reviewer. Not for per-task verification: that is the implementer's Tier-2 gate. tools: Read, Grep, Glob, Bash model: sonnet -effort: medium +effort: low color: cyan --- diff --git a/internal/templater/templates/agents/spec-author.md.tmpl b/internal/templater/templates/agents/spec-author.md.tmpl index e824e3d..6250fd2 100644 --- a/internal/templater/templates/agents/spec-author.md.tmpl +++ b/internal/templater/templates/agents/spec-author.md.tmpl @@ -3,7 +3,7 @@ name: spec-author description: Authors ONE spec phase artifact (requirements, design, or tasks) for a feat — scaffolds it with `csdd spec generate`, consults the knowledge graph for the governing code/ADRs/stack, writes the body to EARS and traceability, validates, and reports. Sonnet writes; the orchestrator (opus) reviews, validates, and approves. Does not implement, does not approve, does not touch the approved plan. tools: Read, Grep, Glob, Edit, Write, Bash model: sonnet -effort: high +effort: medium skills: - graph color: purple @@ -75,10 +75,14 @@ so reading past what the phase needs costs real time and buys nothing. `_Depends:_` IDs exist; no two `(P)` tasks share a boundary. Shape follows `development_flow` — the scaffolded template already carries the right shape; keep it. Sub-tasks are 1–3 hours of focused work. -4. **Validate — and fix what it finds.** - `csdd spec validate ` returns 2 on findings. Fix every finding in the - artifact body and re-validate until it returns 0. Do not `--force` past a - real finding. +4. **Validate — and fix what it finds, at most twice.** + `csdd spec validate ` returns 2 on findings. Fix them in the artifact + body and re-validate — but give it **two rounds, not an open loop**. A finding + that survives two honest attempts is one you are reading wrong, and each + further round re-reads a spec that is commonly 800+ lines for a fix you have + already tried: it costs more than the orchestrator spends deciding. Report + what is left, quoting the finding verbatim and what you tried. Do not + `--force` past a real finding. 5. **Report.** State the phase, the artifact path, a one-line summary of what it covers, the validate result (0 / findings left), and any unresolved gap or open question for the orchestrator. Your report **is** the handoff the @@ -114,8 +118,9 @@ so reading past what the phase needs costs real time and buys nothing. - [ ] The named phase artifact is scaffolded and authored under `specs//`. - [ ] EARS (requirements) / traceability + boundary map + stack-ADR conformance (design) / annotations + flow-correct shape (tasks) hold. -- [ ] `csdd spec validate ` returns 0 (or the remaining findings are named - and explicitly left for the orchestrator). +- [ ] `csdd spec validate ` returns 0 (or, after two fix rounds, the + remaining findings are quoted verbatim and explicitly left for the + orchestrator, with what you tried). - [ ] No `spec approve` called; `spec.json` frontmatter, the plan, and `.csdd/` untouched. - [ ] Report states: phase, artifact path, summary, validate result, open gaps. \ No newline at end of file