diff --git a/README.md b/README.md index aa9200f..b83433a 100644 --- a/README.md +++ b/README.md @@ -302,6 +302,10 @@ csdd plan approve ──▶ csdd plan run ──▶ Pull Request The session's verdict declares one of three intents: `done` (the whole feat is delivered and the session's own checks — `csdd spec validate`, `csdd graph analyze --strict`, the plan's Quality Gates — pass), `continue` (honest partial work; the summary is the handoff to the next session on the same feat), or `blocked` (the feat turned out to need another feat of this plan that has not landed — it parks without spending an attempt, and the discovered edge is recorded in `.csdd/plan//discovered-deps.json`, never written back into `plan.md`). +What a session is handed is **the feat, not the method**. The brief carries the feat row, its governing refs, the seeds, the Executor Notes and the plan's Quality Gates; the development process — the session's authority to approve its own phases, the cycle, what it delegates, the git it owns and what makes a `done` acceptable — lives in the plan-session `CLAUDE.md` written into each worktree and in the `plan-dev` skill, both of which the session reads every turn anyway. + +Before a feat is dispatched, a **context pass** (`--enrich-model`, default `sonnet`) reads its worktree once and records what the feat touches, which decisions and stack rows govern it, what is already there and what is not, into `.csdd/plan//briefs/.json`. Every path and every `adr:`/`stack:` citation is checked against the workspace before it is stored — an unverifiable claim is dropped, not rendered — and the pack is keyed to the feat row, so a feat's later attempts reuse it instead of rediscovering the same tree at the orchestrator's price. `csdd plan brief --feat F --refresh` re-runs the pass and prints the brief, which is how you review (and, by editing the JSON, correct) what a session will be handed. `--enrich-model none` turns the pass off and briefs from the plan alone. + Feats are handed out in **dependency order**, not table order: the `Depends` column the plan already declares is what schedules the run, so a feat is only dispatched once everything it builds on is delivered. If a feat is blocked, everything downstream of it is reported as unreachable with the root cause named, instead of being dispatched into a tree where its foundation does not exist. `--squad-limit N` (1..6, default 1) runs up to **N sessions at once, each on its own feat and each in its own git worktree**. Filesystem isolation is what makes it safe: every concurrent session gets a worktree under `.csdd/plan//trees/` on branch `csdd//`, so no two agents ever share an index, a build, or a half-written file. Scheduling is the `Depends` graph alone — the `(P)` column is **not** consulted, because it used to mean "consents to share a working tree" and feats no longer share one. @@ -323,6 +327,8 @@ csdd plan run # bypass-mode loop — alerts + asks csdd plan run --yes # pre-accept the unverified-sandbox alert (non-interactive) --session-budget 5 # per-session USD cap · --max-iterations (100) · --stall (10) --squad-limit 3 # run up to 3 sessions at once (1..6, default 1), one git worktree each + --enrich-model none # skip the per-feat context pass (default: sonnet) +csdd plan brief --feat F --refresh # re-run the context pass and print what the session gets ``` #### When a run ends without completing diff --git a/internal/cli/plan.go b/internal/cli/plan.go index 823e496..7eb7649 100644 --- a/internal/cli/plan.go +++ b/internal/cli/plan.go @@ -61,7 +61,7 @@ func runPlan(args []string, templates embed.FS) int { func planRun(args []string, templates embed.FS) int { fs := flag.NewFlagSet("plan run", flag.ContinueOnError) - var root, model, effort string + var root, model, effort, enrichModel string var autonomous, assumeYes, noTelegram bool var sessionBudget float64 var maxIterations, stall, featAttempts, maxRetries, maxRepairs, squadLimit int @@ -72,6 +72,7 @@ func planRun(args []string, templates embed.FS) int { fs.BoolVar(&autonomous, "autonomous", false, "Deprecated no-op: plan run always runs bypass-mode (--dangerously-skip-permissions).") fs.StringVar(&model, "model", "opus", "Model the orchestrating session runs on (claude --model): sonnet|opus|haiku|fable or a full model ID. It reviews and decides the spec; spec authoring is delegated to the `spec-author` sub-agent (sonnet) and task implementation to the `implementer` sub-agent (each on its own, cheaper model). Empty inherits the ambient default.") fs.StringVar(&effort, "effort", "medium", "Reasoning effort the orchestrating session runs at (claude --effort): low|medium|high|xhigh|max. Empty inherits the ambient default.") + fs.StringVar(&enrichModel, "enrich-model", "sonnet", "Model the per-feat context pass runs on (claude --model). Before each feat is dispatched it reads the worktree once and records what the feat touches, what governs it and what is already there, so the orchestrator does not rediscover it every attempt. `none` turns the pass off and briefs from the plan alone.") fs.Float64Var(&sessionBudget, "session-budget", 0, "Per-session cap in USD (claude --max-budget-usd). Default 0 = no cap; the session runs under the Claude account's own limits.") fs.IntVar(&maxIterations, "max-iterations", 30, "Sessions the run may spend; one iteration is one claude session.") fs.IntVar(&stall, "stall", 10, "Stop early after this many consecutive sessions without a step advancing.") @@ -100,6 +101,11 @@ func planRun(args []string, templates embed.FS) int { render.Err(fmt.Sprintf("--squad-limit must be between 1 and %d (got %d)", planSquadLimitMax, squadLimit)) return 1 } + // `none` is how a human spells "no enrichment pass"; the runner reads an empty + // model as disabled. + if strings.EqualFold(strings.TrimSpace(enrichModel), "none") { + enrichModel = "" + } if autonomous { render.Warn("--autonomous is deprecated and now a no-op: plan run always runs bypass-mode") } @@ -136,6 +142,7 @@ func planRun(args []string, templates embed.FS) int { FeatAttempts: featAttempts, SessionIdle: sessionIdle, SquadLimit: squadLimit, + EnrichModel: enrichModel, WorktreeEntry: planEntryDoc(templates), Out: os.Stdout, }) @@ -516,15 +523,18 @@ func planNext(args []string) int { func planBrief(args []string) int { fs := flag.NewFlagSet("plan brief", flag.ContinueOnError) - var root, feat string + var root, feat, enrichModel string + var refresh bool addRoot(fs, &root) fs.StringVar(&feat, "feat", "", "Feat to brief (default: the sequencer's next feat).") + fs.BoolVar(&refresh, "refresh", false, "Re-run the context pass for this feat before printing, replacing its stored pack. This is the same pass `plan run` makes before dispatching a feat, so it is how you review — and, by editing .csdd/plan//briefs/.json, correct — what a session will be handed.") + fs.StringVar(&enrichModel, "enrich-model", "sonnet", "Model --refresh runs the context pass on (claude --model).") positionals, err := parseFlags(fs, args) if err != nil { return failOnFlagParse(err) } if len(positionals) < 1 { - render.Err("usage: " + prog() + " plan brief SLUG [--feat F]") + render.Err("usage: " + prog() + " plan brief SLUG [--feat F] [--refresh]") return 1 } r, doc, code := resolvePlan(root, positionals[0]) @@ -551,6 +561,26 @@ func planBrief(args []string) int { } target = f } + // --refresh runs the pass against the workspace itself rather than a worktree: + // there is no run in flight to have cut one, and the tree a human is looking at + // is the tree they are asking about. Diagnostics go to stderr so the brief on + // stdout stays a clean artifact to pipe or diff. + if refresh { + if err := plan.RemovePack(r, doc.Slug, target.Slug); err != nil { + render.Err(err.Error()) + return 1 + } + hook := plan.EnrichHook(enrichModel) + if hook == nil { + render.Err("--refresh needs a model: pass --enrich-model") + return 1 + } + if plan.EnsurePack(r, doc, target, r, hook, func(format string, a ...any) { + render.Warn(strings.TrimSpace(fmt.Sprintf(format, a...))) + }) == nil { + render.Warn("the context pass produced nothing for " + target.Slug + "; briefing from the plan alone") + } + } out, err := plan.FeatBrief(r, doc, target) if err != nil { render.Err(err.Error()) diff --git a/internal/cli/plan_adr_test.go b/internal/cli/plan_adr_test.go index 931b11f..9dd60ce 100644 --- a/internal/cli/plan_adr_test.go +++ b/internal/cli/plan_adr_test.go @@ -101,9 +101,6 @@ func TestADRValidateE2E(t *testing.T) { if strings.Contains(out, "BODY_TOKEN") { t.Errorf("brief must NOT inline the ADR body (the gate enforces citation now):\n%s", out) } - if !strings.Contains(out, "docs/stack.md Decided row") { - t.Errorf("brief should carry the record-your-decisions mission line") - } // Broken citation: rewrite the ADR to a different slug. mustWrite(t, adrPath("0001-pick-store.md"), "# renamed\n\nbody\n") // still slug pick-store diff --git a/internal/cli/plan_bridge_cli_test.go b/internal/cli/plan_bridge_cli_test.go index 6899245..f87564b 100644 --- a/internal/cli/plan_bridge_cli_test.go +++ b/internal/cli/plan_bridge_cli_test.go @@ -91,13 +91,16 @@ func TestPlanNextAndBriefCLI(t *testing.T) { if !strings.Contains(out, `"feat": "upload"`) { t.Errorf("unexpected next feat: %s", out) } - // brief prints the whole-feat mission pack for that feat. + // brief prints the feat's mission pack: the feat itself, what governs it and the + // plan's gates — the development process lives in the worktree's CLAUDE.md. code, out, errOut := run(t, "plan", "brief", "photos", "--root", dir) if code != 0 { t.Fatalf("plan brief failed (code=%d): %s", code, errOut) } - if !strings.Contains(out, "Mission — photos / upload") || !strings.Contains(out, "Forbidden actions") { - t.Errorf("brief output incomplete: %s", out) + for _, want := range []string{"# Feat: upload — plan photos", "Objective:", "Quality gates for this plan"} { + if !strings.Contains(out, want) { + t.Errorf("brief output is missing %q: %s", want, out) + } } } diff --git a/internal/plan/adr_test.go b/internal/plan/adr_test.go index c62caa7..7bd9844 100644 --- a/internal/plan/adr_test.go +++ b/internal/plan/adr_test.go @@ -249,11 +249,6 @@ func TestBriefListsGoverningADRs(t *testing.T) { if !strings.Contains(out, "adr:ghost-ref") || !strings.Contains(out, "WARNING") { t.Errorf("brief should warn on the unresolved ADR ref:\n%s", out) } - // The mission tells the session to record any technology / hard-to-reverse - // trade-off it makes (stack row + ADR), never adopt one silently. - if !strings.Contains(out, "docs/stack.md Decided row") { - t.Errorf("brief should carry the record-your-decisions mission line:\n%s", out) - } } func joinIssues(issues []validator.Issue) string { diff --git a/internal/plan/brief.go b/internal/plan/brief.go index 4a50914..a91c31d 100644 --- a/internal/plan/brief.go +++ b/internal/plan/brief.go @@ -11,37 +11,28 @@ import ( "github.com/protonspy/csdd/internal/paths" ) -// missionSteps is the whole-feat contract every brief carries: the session owns -// the entire spec lifecycle and the implementation, drives its own git, and returns -// a verdict. The runner runs no gates of its own — it checks only the artifacts a -// `done` verdict implies (R10) — so the -// checks the runner used to enforce become checks the session runs itself before -// declaring the feat done — see the "Before you declare `done`" block the brief -// writes, which splits them into the state checks the session reads itself and -// the command gate it delegates to the `quality-gate` sub-agent. -var missionSteps = []string{ - "Author the spec if it does not exist yet: `csdd spec init [--flow unit|tdd|tdd-e2e]`, then for each phase DELEGATE the authoring to the `spec-author` sub-agent (sonnet): dispatch it with the feat, the phase, and any governing refs (ADRs/stack/wiki); it scaffolds (`csdd spec generate --artifact requirements|design|tasks`), consults the graph, authors the body, and runs `csdd spec validate `. REVIEW its artifact and run `csdd spec validate ` yourself, then `csdd spec approve --phase requirements|design|tasks`. If the review or the validate finds gaps, RE-DISPATCH spec-author with a fix-list rather than hand-editing inline. Authoring runs on the cheap model; the JUDGMENT — review, validate, approve — stays on your (orchestrator) model. CHOOSE THE FLOW DELIBERATELY: `unit` (the default) for surfaces whose behaviors are render/CRUD states, which halves the verification round-trips per behavior; `tdd` REQUIRED for money, auth, tenancy, and anything irreversible. The flow you pick decides the shape of tasks.md, and `csdd spec validate` enforces that shape.", - "Implement the tasks by DELEGATING each one to the `implementer` sub-agent (Agent/Task tool) — one leaf task per sub-agent. You orchestrate and decide; the implementer executes the already-made decision on its own (fast) model under the spec's `development_flow`, so do NOT hand-write task code inline. HAND IT THE TASK, NOT THE SPEC: the dispatch carries the task text with its `_Requirements:_`/`_Boundary:_`/`_Depends:_`, the acceptance criteria those IDs name, and the `### ` section of design.md that owns the boundary — you already read the spec to author it, and making every implementer re-read all of it pays that cost again per sub-agent. Dispatch `(P)` tasks in DIFFERENT `_Boundary:_` groups concurrently (worktree isolation); honor every `_Depends:_` and keep same-boundary tasks sequential. Check each task's box `[x]` in specs//tasks.md as its implementer lands it green. (If the `implementer` sub-agent is not installed in this workspace, fall back to running the cycle skill that matches the flow yourself — `tdd-cycle` under tdd/tdd-e2e, `unit-cycle` under unit — one task at a time.)", - "COMMIT YOUR WORK, and commit it where you already are. You are running inside a git worktree the runner created for this feat, already checked out on this feat's own branch — do NOT create a branch, do NOT switch branches, do NOT push, and do NOT open a PR. Commit via /csdd-commit (the pre-push gate runs there) as you land each task, and make sure NOTHING is left uncommitted before you declare `done`: the runner merges your BRANCH into the run's base, so work you left in the working tree does not exist as far as the plan is concerned. The runner refuses a `done` whose worktree is dirty and hands the feat straight back. One PR covers the whole run and a human opens it after the run ends.", - "Record any technology or hard-to-reverse trade-off the contract does not already cover — a docs/stack.md Decided row, plus a docs/adr record when the why needs more than a line. Prefer the option that deviates least from the decided stack.", -} - -// forbiddenActions is the short list the session must not touch: the approved plan -// is the contract, and .csdd/ is the loop's operational state. -var forbiddenActions = []string{ - "Do NOT edit plan.md or plan.json — the approved plan is the contract. Leave .csdd/ (the loop's operational state, including progress.json) alone.", -} - -// FeatBrief assembles the deterministic mission pack for one whole feat (R7). It -// draws only on explicit content — the feat row, its seeds, the governing ADR/stack +// FeatBrief assembles the mission pack for one whole feat (R7): what this feat is, +// what governs it, what it is likely to touch, and what verifies it. It does NOT +// carry the development process — how a session authors and approves its spec +// phases, what it may delegate, what git it owns and what makes a `done` acceptable +// live in the plan-session CLAUDE.md the runner writes into every worktree and in +// the `plan-dev` skill, both of which the session already reads every turn. Stating +// them a second time here cost more than the duplication: the two copies had begun +// to disagree about the feat-exit checks, and a brief that argues against CLAUDE.md's +// interactive STOP rules is arguing against a document the worktree no longer has. +// +// It draws on explicit content — the feat row, its seeds, the governing ADR/stack // refs (slugs/names/paths only — never ADR bodies, stack row details, or wiki -// descriptions), the Executor Notes, and the feat's own quality gates — so the same -// plan and feat always produce a byte-identical brief (R7.3). Compliance with the -// governors is enforced mechanically — designConformance requires design.md to cite -// each governing ADR, `plan validate` rejects tech outside the Decided stack, and -// the code-reviewer gate rejects it in code — not by inlining their bodies in the -// brief (R7.2). The autonomous run context (handoff, failure trail) is appended -// after this by the runner, so this prefix stays stable across a feat's sessions. +// descriptions), the Executor Notes, the plan's quality gates — plus the feat's +// stored context pack when one exists. Compliance with the governors is enforced +// mechanically (designConformance requires design.md to cite each governing ADR, +// `plan validate` rejects tech outside the Decided stack, the code-reviewer gate +// rejects it in code), not by inlining their bodies (R7.2). +// +// It stays deterministic (R7.3): the pack is read from disk like the seeds are, so +// the same workspace state always renders the same brief. The autonomous run context +// (handoff, failure trail) is appended after this by the runner, so this prefix stays +// stable across a feat's sessions. func FeatBrief(root string, doc *PlanDoc, feat Feat) (string, error) { if _, ok := doc.Feat(feat.Slug); !ok { return "", fmt.Errorf("feat %q is not in plan %q", feat.Slug, doc.Slug) @@ -49,46 +40,16 @@ func FeatBrief(root string, doc *PlanDoc, feat Feat) (string, error) { var b strings.Builder w := func(format string, a ...any) { fmt.Fprintf(&b, format, a...) } - w("# Mission — %s / %s\n\n", doc.Slug, feat.Slug) - w("You are a fresh session in a self-correcting autonomous loop. Your mission is to\n") - w("fully deliver ONE feat of an approved csdd plan: %s. Drive the entire spec\n", feat.Slug) - w("lifecycle and its implementation yourself, then return the verdict schema.\n\n") - - // 1. The mission contract. - w("## Your mission (own the whole flow)\n\n") - for i, s := range missionSteps { - w("%d. %s\n", i+1, s) - } - - // Authority: neutralize the interactive-development STOP rules (CLAUDE.md) that - // otherwise make an autonomous session refuse to self-approve its spec phases and - // stall waiting for a human. The plan-level `plan approve` gate already carried - // the human authorization; inside this loop the session IS the approver. - w("\n**Your authority — inside this loop, YOU are the approver:**\n") - w("- A human already opened the gate for this whole plan by running `csdd plan approve %s`. That IS the human authorization CLAUDE.md's phase-gate and STOP rules require, and it covers every spec you author for this plan.\n", doc.Slug) - w("- So you approve THIS feat's spec phases yourself: after each phase validates, run `csdd spec approve %s --phase requirements|design|tasks`. Do NOT pause for a human, do NOT return `continue` because a phase \"still needs approval\", and do NOT treat approving your own spec as routing around a gate — approving it is the mission.\n", feat.Slug) - w("- CLAUDE.md's \"a human authorizes\" / \"STOP and surface the blocked item\" rules govern INTERACTIVE development. This is the autonomous plan loop: a blocked gate means fix the artifact until it validates, then approve and continue (use `--force` only to clear a stale prior-phase hash you have just re-validated).\n") - w("- Follow the `plan-dev` skill for the exact per-phase workflow and completion criteria when it is installed (`.claude/skills/plan-dev/`); it restates these rules as an executable checklist.\n") - - w("\n**Forbidden actions:**\n") - for _, f := range forbiddenActions { - w("- %s\n", f) - } - w("\n**Verdict protocol:**\n") - w("- `done` — the WHOLE feat is delivered and every self-check below passes. The loop checks this claim against your artifacts before accepting it (see below), so never claim done on hope.\n") - w("- `continue` — honest partial work: you are out of room before the feat is complete. Put the handoff for your successor in `summary` (what is done, what remains, what to try next).\n") - w("\n") - - // 2. Feat context. - w("## Feat: %s\n\n", feat.Slug) + // 1. The feat itself. + w("# Feat: %s — plan %s\n\n", feat.Slug, doc.Slug) w("- Objective: %s\n", orDash(feat.Objective)) w("- Milestone: %s\n", orDash(feat.Milestone)) if len(feat.Depends) > 0 { - w("- Depends on (delivered earlier in the plan): %s\n", strings.Join(feat.Depends, ", ")) + w("- Depends on (delivered earlier in this run): %s\n", strings.Join(feat.Depends, ", ")) } w("\n") - // 3. Governing stack — refs only. The brief used to inline each Decided row + // 2. Governing stack — refs only. The brief used to inline each Decided row // (choice/version/why) so a design could not drift from the stack; with the // bodies gone, compliance is enforced mechanically — `csdd spec validate` + // `plan validate` reject tech outside the Decided table, and the @@ -108,7 +69,7 @@ func FeatBrief(root string, doc *PlanDoc, feat Feat) (string, error) { w("\n") } - // 3b. Governing decisions — refs only. The brief used to inline each cited + // 2b. Governing decisions — refs only. The brief used to inline each cited // ADR's title + body so a design could not silently ignore the decisions it // was bound to; with the bodies gone, `designConformance` requires the // authored design.md to cite each governing adr:, and the session @@ -116,7 +77,7 @@ func FeatBrief(root string, doc *PlanDoc, feat Feat) (string, error) { if len(feat.ADRRefs) > 0 { adrs := ScanADRs(root) w("## Governing decisions (docs/adr — fetch the why; cite each in design)\n\n") - w("The brief no longer inlines ADR bodies. For each governor run `csdd graph explain adr:` for the decision; `csdd spec validate` requires your design.md to cite it.\n\n") + w("The brief does not inline ADR bodies. For each governor run `csdd graph explain adr:` for the decision; `csdd spec validate` requires your design.md to cite it.\n\n") for _, slug := range feat.ADRRefs { if _, res := adrs.Resolve(slug); res != ADRResolved { w("- adr:%s — WARNING: does not resolve to a docs/adr record (validate should have caught this).\n", slug) @@ -127,7 +88,7 @@ func FeatBrief(root string, doc *PlanDoc, feat Feat) (string, error) { w("\n") } - // 4. Wiki refs — path only (the session reads what it needs, when it needs it). + // 3. Wiki refs — path only (the session reads what it needs, when it needs it). if len(feat.WikiRefs) > 0 { w("## Reference pages (read what you need; do not assume the content)\n\n") for _, ref := range feat.WikiRefs { @@ -141,7 +102,7 @@ func FeatBrief(root string, doc *PlanDoc, feat Feat) (string, error) { w("\n") } - // 5. Seeds — pre-authored artifacts to fold into this feat's spec. + // 4. Seeds — pre-authored artifacts to fold into this feat's spec. if seeds := featSeedFiles(root, doc.Slug, feat.Slug); len(seeds) > 0 { w("## Seeds (pre-authored inputs for this feat's spec)\n\n") for _, s := range seeds { @@ -150,57 +111,95 @@ func FeatBrief(root string, doc *PlanDoc, feat Feat) (string, error) { w("\n") } + // 5. The discovered half: where this feat lives in the tree, what constrains it, + // what is already there. Read from disk, so the brief stays deterministic. + pack, err := LoadPack(root, doc.Slug, feat.Slug) + if err != nil { + // A corrupt pack is worth naming, but never worth failing a brief over: the + // rest of it is exact. + w("_(the stored context pack for this feat could not be read: %s)_\n\n", firstLine(err.Error())) + } + renderPack(&b, pack) + // 6. Executor Notes — verbatim. if doc.ExecutorNotes != "" { w("## Executor Notes\n\n%s\n\n", doc.ExecutorNotes) } - // 7. Graph-first consultation. - w("## Consult the knowledge graph BEFORE reading code\n\n") - w("- `csdd graph query ` — find the nodes for a concept.\n") - w("- `csdd graph explain ` — see a node's neighborhood and provenance.\n") - w("- `csdd graph path ` — trace how two artifacts connect.\n") - w("Traverse the graph to locate the relevant code; do not grep the whole tree.\n\n") - - // 8. Self-checks. The loop trusts the session's judgment but verifies its - // artifacts (R10), so the brief states both halves: what only the session can - // check, and what the gate will check regardless of what the verdict claims. - w("## Before you declare `done` — run these checks YOURSELF\n\n") - w("Run each of these ONCE, here at feat exit — not after every task:\n\n") - w("- `csdd spec validate %s` (the spec is structurally sound: EARS, traceability, tasks)\n", feat.Slug) - w("- `csdd graph analyze --strict` (spec↔task↔component traceability holds)\n") - w("- every task box in specs/%s/tasks.md is `[x]`\n\n", feat.Slug) - w("DELEGATE the command gate to the `quality-gate` sub-agent, dispatched together\n") - w("with `code-reviewer` (and `security-reviewer` when auth/secrets/input were\n") - w("touched) once the last implementer has finished. It runs the commands below\n") - w("plus the Tier-3 `csdd spec test-report %s --run --lang ` (coverage on,\n", feat.Slug) - w("no --fast) and returns PASS/FAIL with the real failing output. Running them\n") - w("inline instead lands the whole suite, lint and typecheck output in YOUR\n") - w("context, which you then re-read on every remaining turn. The reviewers are\n") - w("limited by their model and the gate by CPU, so they overlap for free.\n\n") + // 7. The plan's own verification contract. + w("## Quality gates for this plan\n\n") + w("Delegate these to the `quality-gate` sub-agent once, at feat exit:\n\n") for _, g := range planGateCommands(doc) { w("- %s\n", g) } - w("\n## What the loop checks before accepting `done`\n\n") - w("The loop trusts your JUDGMENT — it never reviews your code or re-runs your\n") - w("suites — but it does check your ARTIFACTS. A `done` verdict is accepted only\n") - w("when all of these hold on disk:\n\n") - w("- every task box in specs/%s/tasks.md is `[x]`\n", feat.Slug) - w("- specs/%s/spec.json has all three phases approved and `ready_for_implementation` true\n", feat.Slug) - w("- specs/%s/test-report.json is green with no open attentions\n\n", feat.Slug) - w("If any of them fails, your `done` becomes a `continue` and comes back to the\n") - w("next session with a note naming what was missing. A feat that spends its whole\n") - w("attempt budget is stopped and surfaced for a human.\n\n") - w("An honest `continue` and a refused `done` cost the same one attempt — but the\n") - w("`continue` spends it carrying YOUR handoff, which says what you learned and\n") - w("what to try next. A refused `done` spends it to be told what you could have\n") - w("checked yourself.\n\n") - w("Only then return `{\"status\":\"done\"}`. If you ran out of room first, return\n") - w("`{\"status\":\"continue\"}` with the handoff in `summary`.\n") + + // 8. Where the process lives. One pointer, not a copy: both documents are in the + // session's context already, and the copy is what drifted. + w("\n---\n\n") + w("Your authority, the cycle, delegation, git and the `done` gate are in this\n") + w("worktree's CLAUDE.md and the `plan-dev` skill — follow them, and do not stop\n") + w("for a human at a phase gate. Return `{\"status\":\"done\"}` when the whole feat is\n") + w("delivered, or `{\"status\":\"continue\"}` with the handoff for your successor in\n") + w("`summary`.\n") return b.String(), nil } +// renderPack writes the discovered half of the brief. Nothing here is authored by +// csdd or by the plan: every line came from the enrichment pass and survived +// VerifyPack, so the section exists only when there is verified content to put in it. +func renderPack(b *strings.Builder, p *EnrichPack) { + if p.Empty() { + return + } + w := func(format string, a ...any) { fmt.Fprintf(b, format, a...) } + + if len(p.Touches) > 0 { + w("## Where this feat lives\n\n") + for _, t := range p.Touches { + w("- `%s` — %s\n", t.Path, t.Why) + } + w("\n") + } + if len(p.Governors) > 0 { + w("## Governing constraints\n\n") + for _, g := range p.Governors { + if g.Declared { + w("- %s — %s\n", g.ID, g.Constraint) + continue + } + w("- %s (discovered) — %s\n", g.ID, g.Constraint) + } + w("\nA **(discovered)** governor is not in the plan's Refs column: use it as context,\n") + w("but `spec validate` only requires your design to cite the declared ones.\n\n") + } + if len(p.Exists) > 0 { + w("## Already there (do not redo)\n\n") + for _, s := range p.Exists { + w("- %s\n", s) + } + w("\n") + } + if len(p.Missing) > 0 { + w("## Still missing\n\n") + for _, s := range p.Missing { + w("- %s\n", s) + } + w("\n") + } + if len(p.Traps) > 0 { + w("## Pitfalls found in this repository\n\n") + for _, s := range p.Traps { + w("- %s\n", s) + } + w("\n") + } + if p.Flow.Choice != "" { + w("## Suggested flow: `%s`\n\n%s\n\n", p.Flow.Choice, p.Flow.Why) + w("The flow is yours to decide — this is a reading of the context, not an order.\n\n") + } +} + // planGateCommands renders the plan's Quality Gates as backticked commands, or a // placeholder note when the plan declared none. func planGateCommands(doc *PlanDoc) []string { diff --git a/internal/plan/brief_test.go b/internal/plan/brief_test.go index 7e70747..b97f823 100644 --- a/internal/plan/brief_test.go +++ b/internal/plan/brief_test.go @@ -76,9 +76,9 @@ func TestBriefContentAndDeterminism(t *testing.T) { if strings.Contains(out, "SECRET_BODY_TOKEN") { t.Errorf("brief must NOT inline the wiki page body (token leaked)") } - // Forbidden actions and Executor Notes verbatim. - if !strings.Contains(out, "Do NOT edit plan.md") { - t.Errorf("brief should carry the forbidden-actions contract") + // The plan's own verification contract, and the Executor Notes verbatim. + if !strings.Contains(out, "make check") { + t.Errorf("brief should carry the plan's Quality Gates") } if !strings.Contains(out, "Run gofmt before committing") { t.Errorf("brief should carry the Executor Notes verbatim") @@ -89,60 +89,6 @@ func TestBriefContentAndDeterminism(t *testing.T) { } } -func TestBriefSelfChecks(t *testing.T) { - root := setupWorkspace(t, "p", briefPlan) - out := briefFor(t, root, "p", "upload") - // The self-checks the session runs before declaring done: spec validate, graph - // analyze, and the plan's own Quality Gates. - if !strings.Contains(out, "csdd spec validate upload") { - t.Errorf("brief should tell the session to run spec validate itself") - } - if !strings.Contains(out, "graph analyze --strict") { - t.Errorf("brief should list the graph traceability self-check") - } - if !strings.Contains(out, "make check") { - t.Errorf("brief should inject the plan Quality Gates as self-checks") - } - // The verdict protocol is done|continue only. - if !strings.Contains(out, "`done`") || !strings.Contains(out, "`continue`") { - t.Errorf("brief should teach the done|continue verdict protocol") - } -} - -// TestBriefApprovalAuthority: the brief must grant the autonomous session authority -// to approve its own spec phases, so CLAUDE.md's interactive "a human authorizes" -// STOP rules do not stall the loop at approval. -func TestBriefApprovalAuthority(t *testing.T) { - root := setupWorkspace(t, "p", briefPlan) - out := briefFor(t, root, "p", "upload") - if !strings.Contains(out, "YOU are the approver") { - t.Errorf("brief should assert the session's approval authority: %s", out) - } - if !strings.Contains(out, "csdd plan approve p") { - t.Errorf("brief should point at the plan-level human gate already given") - } - if !strings.Contains(out, "csdd spec approve upload") { - t.Errorf("brief should tell the session to self-approve its spec phases") - } - if !strings.Contains(out, "plan-dev") { - t.Errorf("brief should point the session at the plan-dev skill") - } -} - -// TestBriefDelegatesImplementation: the mission must route task implementation to -// the `implementer` sub-agent (the orchestrator decides, the fast sub-agent -// executes), not have the orchestrating session hand-write task code inline. -func TestBriefDelegatesImplementation(t *testing.T) { - root := setupWorkspace(t, "p", briefPlan) - out := briefFor(t, root, "p", "upload") - if !strings.Contains(out, "implementer") { - t.Errorf("brief should delegate task implementation to the implementer sub-agent:\n%s", out) - } - if !strings.Contains(out, "DELEGATING") && !strings.Contains(out, "delegat") { - t.Errorf("brief should tell the session to delegate implementation, not hand-write it inline") - } -} - func TestBriefUnknownFeat(t *testing.T) { root := setupWorkspace(t, "p", briefPlan) doc, _ := Load(root, "p") @@ -151,68 +97,91 @@ func TestBriefUnknownFeat(t *testing.T) { } } -// TestBriefIsFlowAware guards the brief against re-hardcoding one development flow. -// It used to say the implementer works "test-first" and to name `tdd-cycle` as the -// only fallback, which silently overrode a spec that declared `unit`: the flow field -// existed, the validator now enforces the task shape it implies, and the brief told -// the session to ignore both. -func TestBriefIsFlowAware(t *testing.T) { - root := setupWorkspace(t, "p", briefPlan) - out := briefFor(t, root, "p", "upload") - for _, want := range []string{ - "development_flow", // the brief points at the spec's declared flow - "tdd-cycle", // …and names the cycle skill for each side of it - "unit-cycle", - "--flow", // authoring picks the flow deliberately +// TestBriefCarriesNoProcess is the boundary the brief now holds: it describes the +// FEAT, never the development process. +// +// The process lives in the worktree's CLAUDE.md and the `plan-dev` skill, both of +// which the session reads every turn anyway. Restating it here was not merely +// duplicate — the copies drifted, and the brief's authority block argued against +// STOP rules that the plan-session CLAUDE.md replacing the interactive one no longer +// contains. Anything on this list reappearing means the two are diverging again. +func TestBriefCarriesNoProcess(t *testing.T) { + out := briefFor(t, setupWorkspace(t, "p", briefPlan), "p", "upload") + for _, banned := range []string{ + "YOU are the approver", // authority — CLAUDE.md's job + "csdd spec approve", // the phase-gate workflow + "csdd spec init", // the cycle + "implementer", // delegation + "spec-author", // … + "/csdd-commit", // git + "do NOT create a branch", // … + "Forbidden actions", // the hard rules + "Verdict protocol", // the verdict contract + "An honest `continue`", // the attempt economics + "Before you declare `done`", // the feat-exit checklist } { - if !strings.Contains(out, want) { - t.Errorf("brief should mention %q so the session honours the spec's flow:\n%s", want, out) + if strings.Contains(out, banned) { + t.Errorf("the brief carries process again (%q); that belongs in the plan-session CLAUDE.md:\n%s", banned, out) } } - // The old wording made test-first unconditional. Any reintroduction of a bare - // "test-first" instruction re-breaks the unit flow. - if strings.Contains(out, "test-first") { - t.Errorf("brief hardcodes 'test-first', which overrides a spec declaring development_flow 'unit'") + // What it does carry instead: one pointer at where the process lives, and the + // verdict shape the runner parses. + if !strings.Contains(out, "plan-dev") || !strings.Contains(out, "CLAUDE.md") { + t.Errorf("the brief should point at the process rather than restate it:\n%s", out) + } + if !strings.Contains(out, `{"status":"done"}`) { + t.Errorf("the brief should still name the verdict the runner parses:\n%s", out) } } -// TestBriefDelegatesTheCommandGate guards the split the feat-exit block encodes: -// the session reads state itself, and delegates the command gate. -// -// Running the plan's Quality Gates inline lands the whole suite, lint and -// typecheck output in the orchestrator's context — which is then re-read on every -// remaining turn of the feat. That is 58% of a measured session's wall clock -// spent in API calls, and reading exit codes does not need the orchestrator's -// model. -func TestBriefDelegatesTheCommandGate(t *testing.T) { +// TestBriefRendersTheContextPack: a verified pack is the discovered half of the +// brief — where the feat lives, what constrains it, what is already there. +func TestBriefRendersTheContextPack(t *testing.T) { root := setupWorkspace(t, "p", briefPlan) + writeFile(t, filepath.Join(root, "docs", "wiki", "pages", "storage-design.md"), "---\ntitle: t\n---\n") + if err := SavePack(root, "p", "upload", &EnrichPack{ + Touches: []PackTouch{{Path: "docs/wiki/pages/storage-design.md", Why: "describes the old uploader"}}, + Governors: []PackGovernor{{ID: "stack:go", Constraint: "no new runtime deps", Declared: true}}, + Exists: []string{"the bucket client already exists"}, + Missing: []string{"no checksum on write"}, + Traps: []string{"the fixture server rejects chunked uploads"}, + Flow: PackFlow{Choice: "unit", Why: "render/CRUD only"}, + }); err != nil { + t.Fatal(err) + } out := briefFor(t, root, "p", "upload") for _, want := range []string{ - "quality-gate", // the sub-agent that owns the command gate - "code-reviewer", // dispatched with it, so the two overlap - "--run --lang", // and it records the Tier-3 evidence through csdd - "csdd spec validate", - "graph analyze --strict", + "Where this feat lives", + "describes the old uploader", + "no new runtime deps", + "Already there (do not redo)", + "the bucket client already exists", + "no checksum on write", + "the fixture server rejects chunked uploads", + "Suggested flow: `unit`", + "not an order", // the flow stays the session's decision } { if !strings.Contains(out, want) { - t.Errorf("feat-exit block should mention %q:\n%s", want, out) + t.Errorf("the brief should render the pack's %q:\n%s", want, out) } } - // The evidence artifact is what the verdict gate reads; a delegated gate that - // only reports in prose leaves it stale and the `done` is refused. - if !strings.Contains(out, "test-report") { - t.Error("the delegated gate must still record test-report.json through csdd") + // A pack is still deterministic input: same disk state, same brief (R7.3). + if out2 := briefFor(t, root, "p", "upload"); out != out2 { + t.Errorf("a brief with a pack is not byte-deterministic") } } -// TestBriefDispatchesTheTaskNotTheSpec keeps the implementer dispatch narrow. -// Seven implementers each re-reading an 800-line spec pays the authoring cost -// once per sub-agent, in contexts that re-read it on every one of their turns. -func TestBriefDispatchesTheTaskNotTheSpec(t *testing.T) { +// TestBriefWithoutPackIsPlanOnly: enrichment is an optimization, never a +// dependency. With no pack on disk the brief is the plan's own content and nothing +// else — which is exactly what a run with `--enrich-model none` gets. +func TestBriefWithoutPackIsPlanOnly(t *testing.T) { out := briefFor(t, setupWorkspace(t, "p", briefPlan), "p", "upload") - for _, want := range []string{"_Boundary:", "acceptance criteria", "design.md"} { - if !strings.Contains(out, want) { - t.Errorf("the implementer dispatch should name %q as what to hand over:\n%s", want, out) + for _, absent := range []string{"Where this feat lives", "Already there", "Suggested flow"} { + if strings.Contains(out, absent) { + t.Errorf("brief should have no %q section without a pack:\n%s", absent, out) } } + if !strings.Contains(out, "Ingest and store photos") { + t.Errorf("brief should still carry the feat's objective:\n%s", out) + } } diff --git a/internal/plan/enrich.go b/internal/plan/enrich.go new file mode 100644 index 0000000..2855cf4 --- /dev/null +++ b/internal/plan/enrich.go @@ -0,0 +1,400 @@ +package plan + +import ( + "crypto/sha256" + "encoding/hex" + "encoding/json" + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/protonspy/csdd/internal/workspace" +) + +// The enrichment pass answers the one question the deterministic brief cannot: what +// does THIS feat actually touch in THIS repository. Everything the plan already +// states — the feat row, its declared refs, the gates, the Executor Notes — stays +// derived, because there it is exact and free. The pass covers only what has to be +// discovered by reading the tree, which is otherwise discovered by the session's own +// orchestrator model, once per attempt, at orchestrator prices. +// +// It answers into a SCHEMA rather than prose, and that is the load-bearing decision +// rather than a serialization detail. Asked for "a prompt for this feat", both a +// cheap and a strong model re-emitted the development process — and the stronger one +// re-emitted it worse, because it faithfully reproduced the INTERACTIVE contract +// from the repository's CLAUDE.md: create a branch, get a human to approve each +// phase, open a PR. Every one of those instructions contradicts the plan loop, and +// the "approve with the human" one deadlocks a session that has no human. A schema +// with no free-text field cannot carry any of it — the failure is removed by +// construction rather than forbidden by wording. + +// EnrichPack is the discovered half of a feat's brief. Every field is bounded by +// enrichSchema and every citation is checked against the workspace before the pack +// is stored (see VerifyPack), so a hallucinated path or decision never reaches the +// session as though the runner had vouched for it. +type EnrichPack struct { + Touches []PackTouch `json:"touches"` + Governors []PackGovernor `json:"governors"` + Exists []string `json:"exists"` + Missing []string `json:"missing"` + Flow PackFlow `json:"flow"` + Traps []string `json:"traps"` + // Key fingerprints the inputs the pack was derived from. It is written by the + // runner, never by the model, and is what makes the pack a cache rather than a + // per-attempt cost: a feat's second attempt reuses the first attempt's pack + // unless the plan row that produced it changed. + Key string `json:"key,omitempty"` +} + +// PackTouch is one file or directory the feat changes, and what changes in it. +type PackTouch struct { + Path string `json:"path"` + Why string `json:"why"` +} + +// PackGovernor is one decision or stack row that constrains the feat. Declared +// says whether the plan's own Refs column named it: `spec validate` requires the +// design to cite the DECLARED ones, so a discovered governor is context the session +// may use and must not be blocked on citing. +type PackGovernor struct { + ID string `json:"id"` + Constraint string `json:"constraint"` + Declared bool `json:"declared"` +} + +// PackFlow is the enricher's reading of which development_flow fits the feat. It is +// advice, not an instruction — the flow is the session's decision, and the brief +// renders it as such. +type PackFlow struct { + Choice string `json:"choice"` + Why string `json:"why"` +} + +// Empty reports whether the pack carries nothing worth rendering. +func (p *EnrichPack) Empty() bool { + return p == nil || (len(p.Touches) == 0 && len(p.Governors) == 0 && len(p.Exists) == 0 && + len(p.Missing) == 0 && len(p.Traps) == 0 && strings.TrimSpace(p.Flow.Choice) == "") +} + +// enrichSchemaVersion is bumped whenever enrichSchema or the enricher prompt +// changes in a way that makes a stored pack the wrong shape. It is part of the +// cache key, so a bump invalidates every pack on disk without anyone deleting one. +const enrichSchemaVersion = "1" + +// enrichSchema is the contract the enricher's answer must satisfy. The bounds are +// the pack's size ceiling: at these caps a full pack renders to roughly 3 KB, which +// is what the brief can afford to spend on discovery. There is deliberately no +// free-text field — see the note at the top of this file. +const enrichSchema = `{"type":"object","additionalProperties":false,` + + `"required":["touches","governors","exists","missing","flow","traps"],"properties":{` + + `"touches":{"type":"array","maxItems":8,"items":{"type":"object","additionalProperties":false,` + + `"required":["path","why"],"properties":{"path":{"type":"string","maxLength":120},` + + `"why":{"type":"string","maxLength":200}}}},` + + `"governors":{"type":"array","maxItems":6,"items":{"type":"object","additionalProperties":false,` + + `"required":["id","constraint","declared"],"properties":{"id":{"type":"string","pattern":"^(adr|stack):","maxLength":120},` + + `"constraint":{"type":"string","maxLength":200},"declared":{"type":"boolean"}}}},` + + `"exists":{"type":"array","maxItems":6,"items":{"type":"string","maxLength":200}},` + + `"missing":{"type":"array","maxItems":6,"items":{"type":"string","maxLength":200}},` + + `"flow":{"type":"object","additionalProperties":false,"required":["choice","why"],` + + `"properties":{"choice":{"enum":["unit","tdd","tdd-e2e"]},"why":{"type":"string","maxLength":200}}},` + + `"traps":{"type":"array","maxItems":6,"items":{"type":"string","maxLength":200}}}}` + +// EnrichRequest is what the Enrich hook is handed: a deterministic prompt, the +// schema its answer must satisfy, and the worktree to answer from. The worktree +// matters as much as it does for a session — a feat whose dependencies were merged +// into its tree is describable there and nowhere else. +type EnrichRequest struct { + Feat Feat + Prompt string + Schema string + Dir string +} + +// enrichPrompt builds the enricher's instructions for one feat. It is deterministic +// (same plan + feat -> same prompt) and it hands over the feat row verbatim: the +// enricher is told WHICH feat to describe rather than asked to pick one, because +// choosing the next feat belongs to the sequencer and an enricher invited to have an +// opinion about ordering produces instructions to halt. +func enrichPrompt(doc *PlanDoc, feat Feat) string { + var b strings.Builder + w := func(format string, a ...any) { fmt.Fprintf(&b, format, a...) } + + w("You are the context enricher for `csdd plan run`. Your output is consumed by a\n") + w("machine, not read by a human: fill the schema for ONE feat and nothing else.\n\n") + w("Plan: docs/plans/%s/plan.md\n", doc.Slug) + w("Feat, verbatim from the `## Feats` table:\n\n") + w("- slug: %s\n", feat.Slug) + w("- objective: %s\n", orDash(feat.Objective)) + w("- milestone: %s\n", orDash(feat.Milestone)) + if len(feat.Depends) > 0 { + w("- depends on: %s\n", strings.Join(feat.Depends, ", ")) + } + if len(feat.Refs) > 0 { + w("- declared refs: %s\n", strings.Join(feat.Refs, " ")) + } + w("\nInvestigate the repository before answering — do not answer from memory:\n") + w("- `csdd graph query ` and `csdd graph explain ` to find the nodes\n") + w("- read docs/adr/, docs/stack.md, docs/wiki/pages/ and the code the feat touches\n") + w("- check what the feats delivered before this one already left in the tree\n\n") + w("Rules — breaking any one of them invalidates the whole pack:\n\n") + w("- Do NOT describe process. No workflow, no phase order, no `spec init`, no branch,\n") + w(" commit, PR, `/clear`, human approval, gates or commands to run. Whoever delivers\n") + w(" the feat already has that contract; repeating it here contradicts the runner.\n") + w("- Do not comment on ordering or blockers. The scheduler decides when the feat runs.\n") + w("- `touches[].path` — a file or directory that EXISTS today, relative to the\n") + w(" workspace root. `why` says what the feat changes in it.\n") + w("- `governors[].id` — `adr:` or `stack:` that\n") + w(" really resolves. `constraint` is the restriction in one line, not a summary of\n") + w(" the record. Set `declared: true` only for an id that appears in the declared\n") + w(" refs above; governors you found yourself carry `declared: false`.\n") + w("- `exists` / `missing` — what is already in the tree versus what is not, verified,\n") + w(" so the feat does not redo delivered work.\n") + w("- `flow.choice` — `unit` unless the feat touches money, auth, tenancy or something\n") + w(" irreversible. One line of `why`.\n") + w("- `traps` — concrete pitfalls of this feat in this repository. No generic\n") + w(" engineering advice.\n") + w("- Invent nothing: what you did not confirm by reading does not go in.\n") + return b.String() +} + +// PackKey fingerprints everything the pack is derived from. A pack whose key still +// matches is reused untouched; anything else is re-enriched. The plan row is the +// whole input — the tree the enricher reads is not hashed, deliberately: re-reading +// a worktree that moved is exactly what the next feat's own pass does, and hashing +// the tree would re-enrich every feat after every merge for no gain. +func PackKey(doc *PlanDoc, feat Feat) string { + h := sha256.New() + // hash.Hash never returns an error; the discard is errcheck's price for that. + _, _ = fmt.Fprintf(h, "v%s\n%s\n%s\n%s\n%s\n%s\n%s\n", enrichSchemaVersion, doc.Slug, + feat.Slug, feat.Objective, feat.Milestone, + strings.Join(feat.Depends, ","), strings.Join(feat.Refs, " ")) + return hex.EncodeToString(h.Sum(nil))[:16] +} + +// packPath is where a feat's pack lives: under the runner's own transient state, not +// in docs/. The pack is a derived cache the CLI can rebuild at will, and .csdd/ is +// where derived, regenerable, never-committed state belongs. +func packPath(root, slug, feat string) string { + return filepath.Join(stateDir(root, slug), "briefs", feat+".json") +} + +// LoadPack reads a feat's stored pack. A missing or unreadable pack is not an error +// — the brief renders without it — so callers get (nil, nil) rather than a failure +// they would all have to ignore. +func LoadPack(root, slug, feat string) (*EnrichPack, error) { + data, err := os.ReadFile(packPath(root, slug, feat)) + if err != nil { + return nil, nil + } + var p EnrichPack + if err := json.Unmarshal(data, &p); err != nil { + return nil, fmt.Errorf("the stored context pack for %s is not valid JSON: %w", feat, err) + } + return &p, nil +} + +// SavePack stores a verified pack. +func SavePack(root, slug, feat string, p *EnrichPack) error { + data, err := json.MarshalIndent(p, "", " ") + if err != nil { + return err + } + return workspace.AtomicWrite(packPath(root, slug, feat), append(data, '\n'), 0o644) +} + +// RemovePack forgets a feat's stored pack, so the next EnsurePack re-runs the +// discovery pass. A pack that was never there is not an error — the caller wanted it +// gone, and it is. +func RemovePack(root, slug, feat string) error { + if err := os.Remove(packPath(root, slug, feat)); err != nil && !os.IsNotExist(err) { + return err + } + return nil +} + +// ParsePack reads a pack out of whatever the enricher returned — the bare object, +// the `--output-format json` envelope, or an object embedded in text. +func ParsePack(output []byte) (*EnrichPack, error) { + var p EnrichPack + if json.Unmarshal(output, &p) == nil && !p.Empty() { + return &p, nil + } + var env struct { + Result string `json:"result"` + } + if json.Unmarshal(output, &env) == nil && env.Result != "" { + if json.Unmarshal([]byte(env.Result), &p) == nil && !p.Empty() { + return &p, nil + } + if m := reJSONObject.FindString(env.Result); m != "" { + if json.Unmarshal([]byte(m), &p) == nil && !p.Empty() { + return &p, nil + } + } + } + if m := reJSONObject.FindString(string(output)); m != "" { + if json.Unmarshal([]byte(m), &p) == nil && !p.Empty() { + return &p, nil + } + } + return nil, fmt.Errorf("could not parse a context pack from the enricher output") +} + +// VerifyPack drops every claim the workspace does not corroborate and returns what +// it dropped. This is the half of the design that keeps a generated pack honest: a +// path that does not exist or a decision that does not resolve reads as authoritative +// to the session precisely because the runner put it there, which makes an unchecked +// citation worse than a missing one. Resolution goes through the same helpers the +// validators use — ScanADRs and decidedRows — so the pack cannot disagree with +// `plan validate` about what a governor is. +// +// It also corrects `declared` rather than trusting it: whether the plan named a +// governor is a fact about the feat row, not a judgment call, and getting it wrong +// in either direction misleads about what the design must cite. +func VerifyPack(root string, feat Feat, p *EnrichPack) []string { + if p == nil { + return nil + } + var dropped []string + declared := declaredRefs(feat) + + touches := p.Touches[:0] + for _, t := range p.Touches { + rel := strings.TrimSpace(t.Path) + if rel == "" || !safeRelPath(rel) { + dropped = append(dropped, "touches "+t.Path+" (unsafe path)") + continue + } + if _, err := os.Stat(filepath.Join(root, filepath.FromSlash(rel))); err != nil { + dropped = append(dropped, "touches "+rel+" (does not exist)") + continue + } + t.Path = rel + touches = append(touches, t) + } + p.Touches = touches + + adrs := ScanADRs(root) + rows := decidedRows(root) + govs := p.Governors[:0] + for _, g := range p.Governors { + id := strings.TrimSpace(g.ID) + switch { + case strings.HasPrefix(id, "adr:"): + if _, res := adrs.Resolve(strings.TrimPrefix(id, "adr:")); res != ADRResolved { + dropped = append(dropped, id+" (does not resolve to a docs/adr record)") + continue + } + case strings.HasPrefix(id, "stack:"): + if _, ok := rows[normalizeTechName(strings.TrimPrefix(id, "stack:"))]; !ok { + dropped = append(dropped, id+" (not in the Decided stack table)") + continue + } + default: + dropped = append(dropped, id+" (not an adr: or stack: reference)") + continue + } + g.ID = id + g.Declared = declared[strings.ToLower(id)] + govs = append(govs, g) + } + p.Governors = govs + + if p.Flow.Choice != "" && !validFlow(p.Flow.Choice) { + dropped = append(dropped, "flow "+p.Flow.Choice+" (not a development_flow)") + p.Flow = PackFlow{} + } + return dropped +} + +// safeRelPath reports whether a model-authored path is a workspace-relative one. +// The pack is the one part of the brief a language model wrote, so its paths get the +// same treatment as any other hostile input: an absolute path or one that climbs out +// with `..` is dropped rather than stat-ed and rendered, which would put a file from +// outside the workspace in front of the session as though the runner had vouched for +// it. +func safeRelPath(rel string) bool { + if filepath.IsAbs(rel) || strings.HasPrefix(rel, "/") || strings.HasPrefix(rel, `\`) { + return false + } + if vol := filepath.VolumeName(rel); vol != "" { + return false + } + for _, seg := range strings.Split(filepath.ToSlash(rel), "/") { + if seg == ".." { + return false + } + } + return true +} + +// declaredRefs indexes the feat row's own Refs column, lowercased, so a governor's +// `declared` flag is decided by the plan rather than by the enricher. +func declaredRefs(feat Feat) map[string]bool { + out := map[string]bool{} + for _, slug := range feat.ADRRefs { + out["adr:"+strings.ToLower(strings.TrimSpace(slug))] = true + } + for _, name := range feat.StackRefs { + out["stack:"+strings.ToLower(strings.TrimSpace(name))] = true + } + return out +} + +// validFlow reports whether s is a development_flow the spec layer accepts. +func validFlow(s string) bool { + switch strings.ToLower(strings.TrimSpace(s)) { + case "unit", "tdd", "tdd-e2e": + return true + } + return false +} + +// EnsurePack returns the feat's context pack, enriching only when there is no +// current one. It never fails a caller: enrichment is an optimization over the +// deterministic brief, and a run that dies because a cheap discovery pass errored +// would be a strictly worse trade than a run whose brief carries less context. Every +// failure is reported through logf and swallowed, and the brief renders without it. +func EnsurePack(root string, doc *PlanDoc, feat Feat, dir string, enrich func(EnrichRequest) (string, error), logf func(string, ...any)) *EnrichPack { + if logf == nil { + logf = func(string, ...any) {} + } + key := PackKey(doc, feat) + if p, err := LoadPack(root, doc.Slug, feat.Slug); err == nil && p != nil && p.Key == key { + return p + } + if enrich == nil { + return nil + } + out, err := enrich(EnrichRequest{ + Feat: feat, + Prompt: enrichPrompt(doc, feat), + Schema: enrichSchema, + Dir: dir, + }) + if err != nil { + logf(" · context enrichment for %s failed (%v); briefing without it", feat.Slug, err) + return nil + } + p, err := ParsePack([]byte(out)) + if err != nil { + logf(" · context enrichment for %s returned nothing usable; briefing without it", feat.Slug) + return nil + } + if dropped := VerifyPack(root, feat, p); len(dropped) > 0 { + logf(" · context pack for %s: dropped %d unverifiable claim(s): %s", + feat.Slug, len(dropped), strings.Join(dropped, "; ")) + } + if p.Empty() { + logf(" · nothing in the context pack for %s survived verification; briefing without it", feat.Slug) + return nil + } + p.Key = key + if err := SavePack(root, doc.Slug, feat.Slug, p); err != nil { + // Worth saying out loud: the pack is still used for this session, but the + // next attempt at this feat will pay for it again. + logf(" · could not store the context pack for %s (%v); the next attempt will re-enrich", feat.Slug, err) + } + return p +} diff --git a/internal/plan/enrich_test.go b/internal/plan/enrich_test.go new file mode 100644 index 0000000..396b8cd --- /dev/null +++ b/internal/plan/enrich_test.go @@ -0,0 +1,247 @@ +package plan + +import ( + "encoding/json" + "errors" + "path/filepath" + "strings" + "testing" +) + +// enrichPlan carries one feat whose Refs declare a stack row and a wiki page, so a +// pack's `declared` flag has both a true and a false case to get wrong. +const enrichPlan = `--- +name: p +status: draft +--- +## Feats + +| # | Feat | Objective | Depends | Milestone | (P) | Refs | +|---|------|-----------|---------|-----------|-----|------| +| 1 | upload | Ingest and store photos | — | M1 | | stack:go [[storage-design]] | +` + +// errStub is any error the enricher might return; only its presence matters. +var errStub = errors.New("the enricher went away") + +func enrichFeat(t *testing.T, root string) (*PlanDoc, Feat) { + t.Helper() + doc, err := Load(root, "p") + if err != nil { + t.Fatal(err) + } + f, ok := doc.Feat("upload") + if !ok { + t.Fatal("feat upload not in plan") + } + return doc, f +} + +// TestVerifyPackDropsUnverifiableClaims is the guarantee that makes a generated +// pack safe to put in a brief: a claim the workspace does not corroborate is +// dropped, not rendered. +// +// An unchecked citation is worse than a missing one — the session trusts it +// precisely because the runner handed it over — and a model asked to describe a +// repository will occasionally name a file that sounds like it should exist. +func TestVerifyPackDropsUnverifiableClaims(t *testing.T) { + root := setupWorkspace(t, "p", enrichPlan) + writeADRs(t, root, map[string]string{"0001-pick-store.md": "# Pick the store\n\nbody\n"}) + _, feat := enrichFeat(t, root) + + p := &EnrichPack{ + Touches: []PackTouch{ + {Path: "docs/stack.md", Why: "real"}, + {Path: "docs/nowhere.md", Why: "invented"}, + {Path: "../outside.md", Why: "climbs out"}, + }, + Governors: []PackGovernor{ + {ID: "adr:pick-store", Constraint: "resolves"}, + {ID: "adr:never-written", Constraint: "does not resolve"}, + {ID: "stack:go", Constraint: "decided", Declared: false}, + {ID: "stack:rust", Constraint: "not decided"}, + {ID: "wiki:storage-design", Constraint: "wrong prefix"}, + }, + Flow: PackFlow{Choice: "vibes", Why: "not a flow"}, + } + dropped := VerifyPack(root, feat, p) + + if len(p.Touches) != 1 || p.Touches[0].Path != "docs/stack.md" { + t.Errorf("only the existing path should survive, got %+v", p.Touches) + } + if len(p.Governors) != 2 { + t.Fatalf("only the resolvable governors should survive, got %+v", p.Governors) + } + // `declared` is a fact about the feat row, not the enricher's opinion: the plan + // declares stack:go and nothing else, and VerifyPack corrects both directions. + for _, g := range p.Governors { + want := g.ID == "stack:go" + if g.Declared != want { + t.Errorf("%s: declared=%v, want %v (the feat row decides this)", g.ID, g.Declared, want) + } + } + if p.Flow.Choice != "" { + t.Errorf("an invalid development_flow should be dropped, got %q", p.Flow.Choice) + } + if len(dropped) != 6 { + t.Errorf("every drop should be reported, got %d: %v", len(dropped), dropped) + } +} + +// TestEnsurePackIsCachedPerFeat: discovery is paid once per feat, not once per +// attempt. A feat that comes back for a second session reuses the pack its first +// session was briefed with — the point of storing it at all. +func TestEnsurePackIsCachedPerFeat(t *testing.T) { + root := setupWorkspace(t, "p", enrichPlan) + doc, feat := enrichFeat(t, root) + calls := 0 + enrich := func(EnrichRequest) (string, error) { + calls++ + return `{"touches":[{"path":"docs/stack.md","why":"the contract"}],"governors":[],` + + `"exists":[],"missing":[],"flow":{"choice":"unit","why":"docs"},"traps":[]}`, nil + } + + first := EnsurePack(root, doc, feat, root, enrich, nil) + if first == nil || len(first.Touches) != 1 { + t.Fatalf("first pass should produce a pack, got %+v", first) + } + if EnsurePack(root, doc, feat, root, enrich, nil) == nil || calls != 1 { + t.Errorf("a second dispatch should reuse the stored pack (calls=%d)", calls) + } + // A feat whose row changed is a different feat as far as the pack is concerned. + feat.Objective = "Ingest, store and thumbnail photos" + if EnsurePack(root, doc, feat, root, enrich, nil); calls != 2 { + t.Errorf("an edited feat row should re-enrich (calls=%d)", calls) + } +} + +// TestEnsurePackNeverFailsTheDispatch: enrichment is an optimization over the +// deterministic brief. Every way the pass can go wrong has to end with the feat +// still dispatchable — a run that died because a cheap discovery pass errored would +// be strictly worse than one whose brief carried less context. +func TestEnsurePackNeverFailsTheDispatch(t *testing.T) { + root := setupWorkspace(t, "p", enrichPlan) + doc, feat := enrichFeat(t, root) + + for name, enrich := range map[string]func(EnrichRequest) (string, error){ + "enricher errored": func(EnrichRequest) (string, error) { return "", errStub }, + "answer is not JSON": func(EnrichRequest) (string, error) { return "I could not find the feat.", nil }, + "answer is empty": func(EnrichRequest) (string, error) { return "{}", nil }, + "nothing verifies": func(EnrichRequest) (string, error) { + return `{"touches":[{"path":"docs/ghost.md","why":"x"}],"governors":[],"exists":[],` + + `"missing":[],"flow":{"choice":"","why":""},"traps":[]}`, nil + }, + } { + var logged []string + got := EnsurePack(root, doc, feat, root, enrich, func(f string, a ...any) { + logged = append(logged, f) + }) + if got != nil { + t.Errorf("%s: expected no pack, got %+v", name, got) + } + if len(logged) == 0 { + t.Errorf("%s: a failed pass should say so in the run log", name) + } + if p, _ := LoadPack(root, "p", "upload"); p != nil { + t.Errorf("%s: nothing should be stored from a failed pass", name) + } + } + // And with no hook at all — `--enrich-model none` — the pass is simply skipped. + if EnsurePack(root, doc, feat, root, nil, nil) != nil { + t.Errorf("a nil hook should disable enrichment, not synthesize a pack") + } +} + +// TestEnrichPromptForbidsProcess guards the prompt against the failure the schema +// was chosen to prevent. +// +// Asked in prose for "a prompt to develop this feat", both a cheap and a strong +// model wrote the development process instead of the context — and the strong one +// wrote it wrong, reproducing the INTERACTIVE contract (create a branch, have a +// human approve each phase, open a PR). "Have a human approve" deadlocks a session +// that has no human. The schema removes the slot; the prompt says so too. +func TestEnrichPromptForbidsProcess(t *testing.T) { + root := setupWorkspace(t, "p", enrichPlan) + doc, feat := enrichFeat(t, root) + prompt := enrichPrompt(doc, feat) + + for _, want := range []string{ + "Do NOT describe process", + "human approval", + "Do not comment on ordering", + "Invent nothing", + "upload", // the feat is handed over, never chosen + "Ingest and store photos", // …verbatim from the row + "stack:go [[storage-design]]", // …with its declared refs, which decide `declared` + } { + if !strings.Contains(prompt, want) { + t.Errorf("the enricher prompt should carry %q:\n%s", want, prompt) + } + } + // Deterministic: the same plan and feat always ask the same question. + if enrichPrompt(doc, feat) != prompt { + t.Errorf("the enricher prompt is not deterministic") + } +} + +// TestEnrichSchemaIsBounded: the pack's size ceiling is the schema's, not a request +// for brevity a model may ignore. +func TestEnrichSchemaIsBounded(t *testing.T) { + var schema map[string]any + if err := json.Unmarshal([]byte(enrichSchema), &schema); err != nil { + t.Fatalf("the enrich schema is not valid JSON: %v", err) + } + if schema["additionalProperties"] != false { + t.Errorf("the schema must be closed: a free-text field is where process leaks back in") + } + props, _ := schema["properties"].(map[string]any) + for _, field := range []string{"touches", "governors", "exists", "missing", "flow", "traps"} { + if _, ok := props[field]; !ok { + t.Errorf("the schema is missing the %q field", field) + } + } + for _, field := range []string{"touches", "governors", "exists", "missing", "traps"} { + f, _ := props[field].(map[string]any) + if _, ok := f["maxItems"]; !ok { + t.Errorf("%s has no maxItems: the pack would have no size ceiling", field) + } + } +} + +// TestParsePackReadsEveryEnvelope: the pass may answer with the bare object, the +// `--output-format json` envelope, or an object wrapped in text. +func TestParsePackReadsEveryEnvelope(t *testing.T) { + body := `{"touches":[{"path":"docs/stack.md","why":"x"}],"governors":[],"exists":[],` + + `"missing":[],"flow":{"choice":"unit","why":"y"},"traps":[]}` + envelope, err := json.Marshal(map[string]string{"result": body}) + if err != nil { + t.Fatal(err) + } + for name, in := range map[string]string{ + "bare object": body, + "json envelope": string(envelope), + "wrapped in prose": "Here is the pack:\n" + body + "\nHope that helps.", + } { + p, err := ParsePack([]byte(in)) + if err != nil { + t.Errorf("%s: %v", name, err) + continue + } + if len(p.Touches) != 1 { + t.Errorf("%s: parsed the wrong thing: %+v", name, p) + } + } + if _, err := ParsePack([]byte("no json at all")); err == nil { + t.Errorf("prose with no object should not parse as a pack") + } +} + +// TestPackPathIsTransientState: a pack is derived, regenerable and never committed, +// so it belongs under .csdd/ with the rest of the run's state — not in docs/, which +// is the human's. +func TestPackPathIsTransientState(t *testing.T) { + got := filepath.ToSlash(packPath("/w", "p", "upload")) + if !strings.Contains(got, "/.csdd/") || !strings.HasSuffix(got, "/briefs/upload.json") { + t.Errorf("unexpected pack path: %s", got) + } +} diff --git a/internal/plan/runner.go b/internal/plan/runner.go index 46bcb39..4f6381f 100644 --- a/internal/plan/runner.go +++ b/internal/plan/runner.go @@ -62,6 +62,11 @@ type Hooks struct { // sleeps until the session window reopens; tests inject a stub that records the // duration and returns immediately. Sleep func(d time.Duration) + // Enrich runs the cheap discovery pass that produces a feat's context pack, + // returning the raw JSON its model answered with. Nil disables enrichment: the + // brief then carries only what the plan states, which is the behavior every run + // had before the pass existed. + Enrich func(EnrichRequest) (string, error) } // SessionRequest is everything one dispatch hands the Session hook. It is a struct @@ -125,6 +130,13 @@ type RunOptions struct { // ledger, the journal and the run's bookkeeping are written in exactly the order // they were before — only the sessions themselves overlap. SquadLimit int + // EnrichModel is the claude --model the context-enrichment pass runs on. It is a + // separate knob from Model because it is a separate job: the pass reads the tree + // and fills a bounded schema, which a cheap model does well, and it exists to + // keep that reading OFF the orchestrator's model. Empty disables enrichment + // entirely — the brief then carries only what the plan states. The CLI defaults + // it to sonnet (--enrich-model, `none` to turn it off). + EnrichModel string // WorktreeEntry is the CLAUDE.md written into each feat's worktree, replacing // the repository's own for the life of the run. Empty leaves the repository's // file in place. The CLI fills it from the plan-session template; the runner @@ -452,17 +464,12 @@ func openDispatch(opts RunOptions, doc *PlanDoc, feat Feat, st *runState, iter i logf := runLogf(opts) key := feat.Slug - base, err := FeatBrief(opts.Root, doc, feat) - if err != nil { - st.hist(key).add("brief assembly failed", err.Error()) - journal(opts, feat.Slug, "failed", "brief assembly: "+firstLine(err.Error())) - logf(" ✗ brief error: %v", err) - return nil - } - // The worktree is prepared before the attempt is charged, for the same reason - // the brief is: a feat that could not be given a tree to work in has not been - // tried, and spending one of its bounded attempts on the runner's own failure - // would surface it as unable to converge (R1.2, R10.4). + // The worktree is prepared before the attempt is charged: a feat that could not + // be given a tree to work in has not been tried, and spending one of its bounded + // attempts on the runner's own failure would surface it as unable to converge + // (R1.2, R10.4). It also comes first because both steps below read it — the + // enricher describes the tree the session will actually work in, which for a feat + // with dependencies is only correct after they have been merged into it. dir, err := opts.Hooks.Trees.Ensure(feat.Slug) if err != nil { st.hist(key).add("worktree setup failed", err.Error()) @@ -470,6 +477,19 @@ func openDispatch(opts RunOptions, doc *PlanDoc, feat Feat, st *runState, iter i logf(" ✗ could not prepare an isolated worktree for %s: %v", feat.Slug, err) return nil } + // Discovery, once per feat rather than once per attempt: EnsurePack reuses the + // stored pack whenever the feat row that produced it has not changed. It cannot + // fail the dispatch — enrichment is an optimization over the deterministic brief, + // so every failure inside it is logged and the brief renders without it. + EnsurePack(opts.Root, doc, feat, dir, opts.Hooks.Enrich, logf) + + base, err := FeatBrief(opts.Root, doc, feat) + if err != nil { + st.hist(key).add("brief assembly failed", err.Error()) + journal(opts, feat.Slug, "failed", "brief assembly: "+firstLine(err.Error())) + logf(" ✗ brief error: %v", err) + return nil + } d := &dispatch{feat: feat, iter: iter, attempt: st.nextAttempt(key), brief: base + runContext(st, key), dir: dir} recordSession(opts, d, SessionStarted, "", false) @@ -1137,6 +1157,7 @@ func fillRunDefaults(opts *RunOptions) { tty: tty, now: opts.Hooks.Now, }) + installEnrichHook(&opts.Hooks, opts.EnrichModel) } // syncWriter serializes writes to one destination so concurrent sessions cannot diff --git a/internal/plan/runner_exec.go b/internal/plan/runner_exec.go index d6df5ea..3867c4d 100644 --- a/internal/plan/runner_exec.go +++ b/internal/plan/runner_exec.go @@ -49,6 +49,73 @@ func installRealHooks(h *Hooks, runModel, runEffort string, sess sessionEnv) { } } +// installEnrichHook fills the enrichment seam, unless the run turned it off with an +// empty model. It is separate from installRealHooks because the model it needs is a +// run option rather than a session parameter, and because a test that wants a real +// session but a stubbed enricher (or the reverse) should not have to fight the +// wiring for it. +func installEnrichHook(h *Hooks, enrichModel string) { + if h.Enrich == nil { + h.Enrich = EnrichHook(enrichModel) + } +} + +// EnrichHook builds the enrichment hook for callers outside the runner — `csdd plan +// brief --refresh`, which is how a human regenerates and reviews a feat's pack +// before approving the plan it belongs to. An empty model yields a nil hook, which +// every caller already treats as "enrichment is off". +func EnrichHook(model string) func(EnrichRequest) (string, error) { + if strings.TrimSpace(model) == "" { + return nil + } + return func(req EnrichRequest) (string, error) { + return execClaudeEnrich(req, model) + } +} + +// enrichIdle bounds the discovery pass. It is far tighter than a session's idle +// budget because the pass is bounded work — read the tree, fill a schema — and one +// that has gone quiet for this long is stuck, not thinking. A feat whose enrichment +// times out is briefed without a pack rather than delayed further. +const enrichIdle = 3 * time.Minute + +// enrichTools is the least privilege the pass can do its job with: read the tree, +// search it, and consult the knowledge graph. It authors nothing and runs no gates, +// so it gets no general shell and no write tools — the pass is the one part of the +// loop whose output a language model composes freely, and the narrowest tool scope +// is what keeps that from becoming a way to change the workspace. +const enrichTools = "Read,Grep,Glob,Bash(csdd graph:*),Bash(npx @protonspy/csdd graph:*)" + +// execClaudeEnrich runs the discovery pass for one feat and returns the raw JSON it +// answered with. It runs in the feat's own worktree for the same reason the session +// does: the tree with this feat's dependencies merged in is the only place the feat +// is describable. +// +// Every error here is the caller's to swallow (EnsurePack does), so this reports what +// went wrong and nothing more — an enrichment that fails costs the run a leaner +// brief, never a feat. +func execClaudeEnrich(req EnrichRequest, model string) (string, error) { + args := []string{ + claudeFlags.print, + claudeFlags.outputFormat, "json", + claudeFlags.jsonSchema, req.Schema, + claudeFlags.model, model, + claudeFlags.allowedTools, enrichTools, + } + cmd := exec.Command("claude", args...) + cmd.Dir = req.Dir + // Same reason the session's brief rides on stdin: the prompt is well past what + // Windows allows on a command line, and `claude -p` with no positional reads it + // from there. + cmd.Stdin = strings.NewReader(req.Prompt) + + stdout, stderr, err := supervised{cmd: cmd, idle: enrichIdle}.run() + if err != nil { + return "", fmt.Errorf("%w: %s", err, firstLine(strings.TrimSpace(stderr))) + } + return stdout, nil +} + // sessionEnv carries what the real Session hook needs beyond the feat itself: the // watchdog's idle budget and the seams the live session view reports through. It is // passed rather than read from globals so a test can drive a real session end-to-end. @@ -74,7 +141,7 @@ const verdictSchema = `{"type":"object","required":["status"],` + // claudeFlags are every `claude` flag the runner relies on, pinned in one place so // a version drift is a single, reviewable edit (risk register §8). var claudeFlags = struct { - print, outputFormat, verbose, jsonSchema, maxBudget, model, effort, bypass string + print, outputFormat, verbose, jsonSchema, maxBudget, model, effort, bypass, allowedTools string }{ print: "-p", outputFormat: "--output-format", @@ -84,6 +151,7 @@ var claudeFlags = struct { model: "--model", effort: "--effort", bypass: "--dangerously-skip-permissions", + allowedTools: "--allowedTools", } // sessionArgs builds the `claude` argument vector for one plan session. It is pure diff --git a/internal/templater/templater_test.go b/internal/templater/templater_test.go index ed1c671..fd0a00c 100644 --- a/internal/templater/templater_test.go +++ b/internal/templater/templater_test.go @@ -524,3 +524,55 @@ func TestSpecTemplatesCiteTheirRules(t *testing.T) { } } } + +// TestPlanSessionEntryCarriesTheLoopContract guards the plan-session CLAUDE.md as +// the ONE place the autonomous loop's process is written down. +// +// It used to be written down three times — here, in every feat's brief, and in the +// `plan-dev` skill — and the copies had begun to disagree: the brief named checks +// and sub-agents this file did not. The brief is now feat context only, so anything +// missing here is missing from the session's context entirely. +func TestPlanSessionEntryCarriesTheLoopContract(t *testing.T) { + doc, err := PlanEntry(FS) + if err != nil { + t.Fatal(err) + } + for _, want := range []string{ + // Authority: without it the session stalls at its own phase gates waiting + // for a human that the loop does not have. + "You are the approver", + "csdd plan approve", + "csdd spec approve", + // Delegation: authoring and implementation both run on cheaper models, and + // the orchestrator only reviews and decides. + "spec-author", + "implementer", + "development_flow", + "tdd-cycle", + "unit-cycle", + // The feat-exit gate and the evidence artifact the verdict gate reads. + "quality-gate", + "code-reviewer", + "security-reviewer", + "test-report", + "--run --lang", + "graph analyze --strict", + // Git: the session commits in the worktree it was given and opens nothing. + "/csdd-commit", + "Do not create or switch branches", + // The contract the plan itself is. + "plan.md", + ".csdd/", + // Decisions get recorded where the next feat can find them, never adopted + // silently: the brief used to carry this line. + "docs/stack.md", + "docs/adr", + // The verdict and what the loop checks it against. + "done", + "continue", + } { + if !strings.Contains(doc, want) { + t.Errorf("the plan-session CLAUDE.md should carry %q — the brief no longer does", want) + } + } +} diff --git a/internal/templater/templates/plan/CLAUDE.md.tmpl b/internal/templater/templates/plan/CLAUDE.md.tmpl index a956471..e4618c3 100644 --- a/internal/templater/templates/plan/CLAUDE.md.tmpl +++ b/internal/templater/templates/plan/CLAUDE.md.tmpl @@ -21,6 +21,10 @@ return `continue` because something "needs approval". contract; `.csdd/` is the loop's state. - Do not create or switch branches, push, or open a PR. You are already on this feat's branch. One PR covers the whole run and a human opens it. +- Record every technology choice and hard-to-reverse trade-off the contract does + not already cover: a `docs/stack.md` Decided row, plus a `docs/adr` record when + the why needs more than a line. Prefer the option that deviates least from the + decided stack. ## The cycle @@ -28,11 +32,20 @@ return `continue` because something "needs approval". spec init → (generate → validate → approve) ×3 → implement → commit → verdict ``` -1. **Spec.** `csdd spec init --flow `, then for each phase in order: - `csdd spec generate --artifact requirements|design|tasks`, - `csdd spec validate `, `csdd spec approve --phase `. +1. **Spec.** `csdd spec init --flow `, then for each phase in order, + delegate the authoring to the **`spec-author`** sub-agent: dispatch it with the + feat, the phase, and the governing refs. It scaffolds + (`csdd spec generate --artifact requirements|design|tasks`), consults the + graph, writes the body and runs `csdd spec validate `. You then **review** + it, run `csdd spec validate ` yourself, and + `csdd spec approve --phase `. Gaps go back to `spec-author` as a + fix-list — do not hand-edit the artifact inline. Authoring runs on the cheap + model; the judgment stays on yours. 2. **Implement.** One leaf task at a time, each delegated to the `implementer` - sub-agent. Check its box `[x]` in `tasks.md` as it lands green. + sub-agent, working under the spec's `development_flow`. Check its box `[x]` in + `tasks.md` as it lands green. (If `implementer` is not installed here, run the + cycle skill that matches the flow yourself — `tdd-cycle` under `tdd`/`tdd-e2e`, + `unit-cycle` under `unit` — one task at a time.) 3. **Commit** via `/csdd-commit` as you land each task. Nothing may be left uncommitted: the runner merges your *branch*, so uncommitted work does not exist. 4. **Verdict.** `done` only when the three checks below hold; otherwise `continue` @@ -61,8 +74,12 @@ You are metered. These are the four things that actually move the number: name, and the one `### ` section that owns the boundary. Making every sub-agent re-read the whole spec pays that cost once per sub-agent. - **Delegate the gate.** Run the suite, lint and typecheck through the - `quality-gate` sub-agent, once, at feat exit. Running them inline lands the full - output in your context and you re-read it on every remaining turn. + `quality-gate` sub-agent, once, at feat exit — dispatched together with + `code-reviewer` (and `security-reviewer` when auth, secrets or input were + touched). It also records the Tier-3 evidence with + `csdd spec test-report --run --lang ` (coverage on, no `--fast`), + which is the artifact the verdict gate reads. Running any of it inline lands the + full output in your context and you re-read it on every remaining turn. - **Traverse the graph, don't grep the tree.** `csdd graph query `, `csdd graph explain `, `csdd graph path `. @@ -74,10 +91,11 @@ Dispatch `(P)` tasks in different `_Boundary:_` groups concurrently; honor every Run each of these once, at feat exit — not after every task: - `csdd spec validate ` passes +- `csdd graph analyze --strict` passes (spec ↔ task ↔ component traceability holds) - every task box in `specs//tasks.md` is `[x]` - `specs//test-report.json` is green with no open attentions -The loop checks these three on disk before accepting your verdict. It does not +The loop checks the last three on disk before accepting your verdict. It does not review your code and does not re-run your suite — it reads your artifacts. A `done` that fails any of them comes back as a `continue` and costs a whole extra session.