Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -302,9 +302,9 @@ 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/<slug>/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.
What a session is handed is **the feat, not the method**. The brief opens by naming the role it is written for — the whole thing is fed to `claude -p` as the prompt, so its first lines are where the session's role is set — then 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/<slug>/briefs/<feat>.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 <slug> --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.
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/<slug>/briefs/<feat>.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 <slug> --feat F` prints the brief and runs the pass itself when the feat has no pack yet, which is how you review (and, by editing the JSON, correct) what a session will be handed. A pack already on disk is reused as-is — even when the feat's row has since changed, which is only reported, since briefing is something you do repeatedly while editing a plan and regenerating costs a model call; `--refresh` is how you replace it. `--enrich-model none` turns the pass off and briefs from the plan alone.
Comment on lines +305 to +307

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Reconcile the ADR-body documentation.

This update says briefs carry governing refs, but README line 292 still says cited ADRs are inlined in full. FeatBrief emits only adr:<slug> references and directs the session to csdd graph explain; update that earlier sentence in this section.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@README.md` around lines 305 - 307, The README’s earlier description of feat
briefs incorrectly says cited ADRs are inlined in full. Update that sentence to
state that briefs carry only adr:<slug> references, with sessions using csdd
graph explain to retrieve the ADR body, while preserving the surrounding
explanation of governing references.


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.

Expand All @@ -328,7 +328,8 @@ csdd plan run <slug> --yes # pre-accept the unverified-sandbox
--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 <slug> --feat F --refresh # re-run the context pass and print what the session gets
csdd plan brief <slug> --feat F # print what the session gets (enriches on first use)
csdd plan brief <slug> --feat F --refresh # discard the stored context pack and run the pass again
```

#### When a run ends without completing
Expand Down
69 changes: 54 additions & 15 deletions internal/cli/plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,7 @@ 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 = ""
}
enrichModel = normalizeEnrichModel(enrichModel)
if autonomous {
render.Warn("--autonomous is deprecated and now a no-op: plan run always runs bypass-mode")
}
Expand Down Expand Up @@ -521,14 +517,26 @@ func planNext(args []string) int {
}
}

// normalizeEnrichModel maps `none` — how a human spells "no context pass" — onto
// the empty model every enrichment caller already reads as disabled. It is shared
// so `plan run` and `plan brief` cannot disagree about what `none` means: for a
// while only the runner honored it, and `plan brief --enrich-model none` spawned a
// session for a model literally named "none".
func normalizeEnrichModel(model string) string {
if strings.EqualFold(strings.TrimSpace(model), "none") {
return ""
}
return model
}

func planBrief(args []string) int {
fs := flag.NewFlagSet("plan brief", flag.ContinueOnError)
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/<slug>/briefs/<feat>.json, correct — what a session will be handed.")
fs.StringVar(&enrichModel, "enrich-model", "sonnet", "Model --refresh runs the context pass on (claude --model).")
fs.BoolVar(&refresh, "refresh", false, "Discard this feat's stored context pack and run the pass again, even when the stored one is still current. Without it the pass runs only when there is nothing usable on disk — a stored pack whose plan row has not changed is reused as-is.")
fs.StringVar(&enrichModel, "enrich-model", "sonnet", "Model the context pass runs on (claude --model). `none` turns the pass off and briefs from the plan alone.")
positionals, err := parseFlags(fs, args)
if err != nil {
return failOnFlagParse(err)
Expand Down Expand Up @@ -561,25 +569,56 @@ 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.
// The discovered half is not an extra. Without it the brief is the plan restated
// — the feat row a human already wrote — and the session rediscovers the tree at
// orchestrator prices, which is the whole cost the pass exists to avoid. So the
// pass runs BY DEFAULT here, exactly as it does before `plan run` dispatches a
// feat. `--enrich-model none` is how a caller that must not spawn a model — CI, a
// scripted diff — opts out.
//
// A pack already on disk is kept, and that is a stronger rule here than in the
// runner: it is reused even when the feat's row has moved on since it was
// written. Regenerating costs a model call, briefing is something a human does
// repeatedly while editing a plan, and re-spending on every one of those edits is
// not a default anybody would choose. A stale pack costs a line on stderr and
// `--refresh` is the way to replace it. (`plan run` keeps invalidating on a
// changed row: what it dispatches a session to build has to match what it briefed
// that session with.)
//
// The pass runs 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.
hook := plan.EnrichHook(normalizeEnrichModel(enrichModel))
if refresh {
if hook == nil {
render.Err("--refresh needs a model: pass --enrich-model (or drop --refresh)")
return 1
}
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
}
pack, err := plan.LoadPack(r, doc.Slug, target.Slug)
if err != nil {
render.Warn(err.Error())
}
switch {
case pack != nil:
if pack.Key != plan.PackKey(doc, target) {
render.Warn("the stored context pack for " + target.Slug + " predates the current plan row; `--refresh` regenerates it")
}
case hook != nil:
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")
}
default:
// Nothing on disk and the pass switched off: say so, rather than printing a
// brief that is silently missing half of itself.
render.Warn("no stored context pack for " + target.Slug + " and the context pass is off (--enrich-model none); briefing from the plan alone")
}
out, err := plan.FeatBrief(r, doc, target)
if err != nil {
Expand Down
5 changes: 3 additions & 2 deletions internal/cli/plan_adr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,9 @@ func TestADRValidateE2E(t *testing.T) {
}

// The brief lists the governing ADR as a short ref and directs fetching the
// body via the graph; it must NOT inline the ADR title or body.
code, out, _ := run(t, "plan", "brief", "photos", "--feat", "upload", "--root", dir)
// body via the graph; it must NOT inline the ADR title or body. The context pass
// is off so the test never spawns a `claude` — it now runs by default.
code, out, _ := run(t, "plan", "brief", "photos", "--feat", "upload", "--root", dir, "--enrich-model", "none")
if code != 0 {
t.Fatalf("plan brief failed: %d", code)
}
Expand Down
71 changes: 70 additions & 1 deletion internal/cli/plan_bridge_cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,9 @@ func TestPlanNextAndBriefCLI(t *testing.T) {
}
// 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)
// `--enrich-model none` because the context pass now runs by DEFAULT: without it
// this test would spawn a real `claude` on any machine that has one installed.
code, out, errOut := run(t, "plan", "brief", "photos", "--root", dir, "--enrich-model", "none")
if code != 0 {
t.Fatalf("plan brief failed (code=%d): %s", code, errOut)
}
Expand Down Expand Up @@ -152,3 +154,70 @@ func TestPlanNextDriftExitCode(t *testing.T) {
t.Errorf("drifted plan next should exit 5, got %d", code)
}
}

// TestPlanBriefNamesAMissingContextPack pins the notice that replaced a silent
// omission.
//
// The discovered half of a brief — what the feat touches, what governs it, what is
// already there — is simply absent when no context pack has been stored, and the
// brief used to render without it and without a word. That is indistinguishable
// from a pass that ran and found nothing, and it is exactly how a reader concludes
// the enrichment is broken. The pass runs by default now, so the only way to reach
// that state is to turn it off — and then it has to say so.
func TestPlanBriefNamesAMissingContextPack(t *testing.T) {
dir := scaffoldApprovedPlan(t)
code, _, errOut := run(t, "plan", "brief", "photos", "--root", dir, "--enrich-model", "none")
if code != 0 {
t.Fatalf("plan brief exit=%d: %s", code, errOut)
}
if !strings.Contains(errOut, "no stored context pack") {
t.Errorf("a brief with no pack and the pass switched off must say so on stderr:\n%s", errOut)
}
}

// TestPlanBriefRefreshRefusesWithNoModel keeps --refresh honest: it forces the pass
// to run, so asking for it with the pass turned off is a contradiction the CLI must
// reject rather than silently print a stale brief for.
func TestPlanBriefRefreshRefusesWithNoModel(t *testing.T) {
dir := scaffoldApprovedPlan(t)
code, _, errOut := run(t, "plan", "brief", "photos", "--root", dir, "--refresh", "--enrich-model", "none")
if code != 1 {
t.Fatalf("--refresh with no model should exit 1, got %d: %s", code, errOut)
}
if !strings.Contains(errOut, "--refresh needs a model") {
t.Errorf("the refusal should name the flag conflict:\n%s", errOut)
}
}

// TestPlanBriefKeepsAStoredPackEvenWhenStale pins the reuse rule that separates
// `plan brief` from the runner: a pack on disk is what the human gets, whether or
// not the feat's row has changed since it was written.
//
// Regenerating is a model call, and briefing is what a human does over and over
// while editing a plan — invalidating on every edit turns reading a brief into a
// recurring charge. So the staleness is REPORTED and the pack is still used;
// `--refresh` is the way to replace it. The pass is switched off here so the test
// spawns nothing: with a pack present it would not run either way, which is the
// property under test.
func TestPlanBriefKeepsAStoredPackEvenWhenStale(t *testing.T) {
dir := scaffoldApprovedPlan(t)
packs := filepath.Join(dir, ".csdd", "plan", "photos", "briefs")
if err := os.MkdirAll(packs, 0o755); err != nil {
t.Fatal(err)
}
// `key` is deliberately not the current plan row's key: this pack is stale.
pack := `{"touches":[{"path":"app/upload.go","why":"the upload handler lives here"}],"key":"stale"}`
if err := os.WriteFile(filepath.Join(packs, "upload.json"), []byte(pack), 0o644); err != nil {
t.Fatal(err)
}
code, out, errOut := run(t, "plan", "brief", "photos", "--root", dir, "--enrich-model", "none")
if code != 0 {
t.Fatalf("plan brief exit=%d: %s", code, errOut)
}
if !strings.Contains(out, "the upload handler lives here") {
t.Errorf("the stored pack must be rendered into the brief:\n%s", out)
}
if !strings.Contains(errOut, "predates the current plan row") {
t.Errorf("a stale pack must be reported, not silently reused:\n%s", errOut)
}
}
17 changes: 14 additions & 3 deletions internal/plan/brief.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ import (
"github.com/protonspy/csdd/internal/paths"
)

// 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
// FeatBrief assembles the mission pack for one whole feat (R7): who is reading it,
// 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
Expand All @@ -40,6 +40,17 @@ 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...) }

// 0. Who is reading this. The brief is fed to `claude -p` on stdin as the entire
// prompt, so its opening lines are the only place the session's role is set —
// without them the first thing the model sees is a feat row, and it has to infer
// from context what it is supposed to be. It stays four lines on purpose: the
// process this engineer follows is NOT here (see the pointer at the end), and a
// preamble that grows is re-read on every turn of every session.
w("You are a senior software engineer. You deliver ONE feat of an approved plan to\n")
w("completion on your own, in an autonomous session with no human at the gate.\n")
w("Everything below is the mission: what this feat is, what governs it, where it\n")
w("lives in this repository, and what verifies it.\n\n")

// 1. The feat itself.
w("# Feat: %s — plan %s\n\n", feat.Slug, doc.Slug)
w("- Objective: %s\n", orDash(feat.Objective))
Expand Down
36 changes: 36 additions & 0 deletions internal/plan/brief_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,3 +185,39 @@ func TestBriefWithoutPackIsPlanOnly(t *testing.T) {
t.Errorf("brief should still carry the feat's objective:\n%s", out)
}
}

// TestBriefOpensByNamingTheRole pins the first four lines.
//
// The brief IS the prompt — it is fed to `claude -p` on stdin with nothing in front
// of it — so whatever it opens with is where the session's role is established. It
// used to open on a feat row, leaving the model to infer what it was supposed to be
// from a table of objectives. The preamble is deliberately short: it names the role
// and the shape of what follows, and stops. Process belongs to the plan-session
// CLAUDE.md and the `plan-dev` skill (see TestBriefCarriesNoProcess), so a preamble
// that starts explaining the cycle is this boundary eroding again.
func TestBriefOpensByNamingTheRole(t *testing.T) {
out := briefFor(t, setupWorkspace(t, "p", briefPlan), "p", "upload")
if !strings.HasPrefix(out, "You are a senior software engineer.") {
t.Errorf("the brief must open by naming the role it is written for:\n%s", firstLines(out, 6))
}
// The role sits BEFORE the feat header — everything after it reads as material
// for that role rather than as a document that happened to arrive.
role, feat := strings.Index(out, "You are a senior software engineer"), strings.Index(out, "# Feat:")
if feat < 0 || role > feat {
t.Errorf("the role must come before the feat header:\n%s", firstLines(out, 8))
}
// Four lines, then the mission. A preamble long enough to need a heading has
// stopped being a preamble.
if head, _, _ := strings.Cut(out, "# Feat:"); strings.Count(head, "\n") > 6 {
t.Errorf("the role preamble should stay short, got:\n%s", head)
Comment on lines +200 to +212

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Pin the complete four-line preamble.

The test fixes only the first sentence and permits extra lines; changes to the remaining role/mission text would still pass. Assert the exact preamble, including its blank separator before # Feat:.

Proposed test tightening
-	if !strings.HasPrefix(out, "You are a senior software engineer.") {
+	const preamble = "You are a senior software engineer. You deliver ONE feat of an approved plan to\n" +
+		"completion on your own, in an autonomous session with no human at the gate.\n" +
+		"Everything below is the mission: what this feat is, what governs it, where it\n" +
+		"lives in this repository, and what verifies it.\n\n"
+	if !strings.HasPrefix(out, preamble) {
 		t.Errorf("the brief must open by naming the role it is written for:\n%s", firstLines(out, 6))
 	}
@@
-	if head, _, _ := strings.Cut(out, "# Feat:"); strings.Count(head, "\n") > 6 {
-		t.Errorf("the role preamble should stay short, got:\n%s", head)
-	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if !strings.HasPrefix(out, "You are a senior software engineer.") {
t.Errorf("the brief must open by naming the role it is written for:\n%s", firstLines(out, 6))
}
// The role sits BEFORE the feat header — everything after it reads as material
// for that role rather than as a document that happened to arrive.
role, feat := strings.Index(out, "You are a senior software engineer"), strings.Index(out, "# Feat:")
if feat < 0 || role > feat {
t.Errorf("the role must come before the feat header:\n%s", firstLines(out, 8))
}
// Four lines, then the mission. A preamble long enough to need a heading has
// stopped being a preamble.
if head, _, _ := strings.Cut(out, "# Feat:"); strings.Count(head, "\n") > 6 {
t.Errorf("the role preamble should stay short, got:\n%s", head)
const preamble = "You are a senior software engineer. You deliver ONE feat of an approved plan to\n" +
"completion on your own, in an autonomous session with no human at the gate.\n" +
"Everything below is the mission: what this feat is, what governs it, where it\n" +
"lives in this repository, and what verifies it.\n\n"
if !strings.HasPrefix(out, preamble) {
t.Errorf("the brief must open by naming the role it is written for:\n%s", firstLines(out, 6))
}
// The role sits BEFORE the feat header — everything after it reads as material
// for that role rather than as a document that happened to arrive.
role, feat := strings.Index(out, "You are a senior software engineer"), strings.Index(out, "# Feat:")
if feat < 0 || role > feat {
t.Errorf("the role must come before the feat header:\n%s", firstLines(out, 8))
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/plan/brief_test.go` around lines 200 - 212, The brief preamble
assertions in the test must validate the complete expected four-line
role/mission text, not just its opening sentence or maximum length. Replace the
permissive checks around strings.HasPrefix and the # Feat: boundary with an
exact comparison that includes the required blank separator immediately before
"# Feat:", while preserving the existing diagnostic output on failure.

}
}

// firstLines returns at most n lines of s, for readable failure output.
func firstLines(s string, n int) string {
lines := strings.Split(s, "\n")
if len(lines) > n {
lines = lines[:n]
}
return strings.Join(lines, "\n")
}
Loading