feat(plan): discontinue plan run; a plan is delivered in your session - #95
Conversation
📝 WalkthroughWalkthroughThe autonomous ChangesPlan delivery transition
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant csdd_plan
participant plan_dev
participant csdd_spec
User->>csdd_plan: approve plan
csdd_plan-->>User: approved plan
User->>plan_dev: deliver feat
plan_dev->>csdd_spec: validate phase
csdd_spec-->>User: present phase
User->>csdd_spec: approve phase
plan_dev-->>User: report feat delivery and checks
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
internal/templater/templates/plan/plan.md.tmpl (2)
8-17: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winFrontmatter comment still describes "autonomous work" between approve and PR.
"Two human gates bracket the whole flow... and everything between is contract-checked machine work" describes the discontinued autonomous loop. Since delivery is now human-driven in-session via
plan-dev, this should describe that model instead of implying unattended machine execution happens between approval and PR.🤖 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/templater/templates/plan/plan.md.tmpl` around lines 8 - 17, Update the introductory frontmatter comment in the plan template to describe delivery as human-driven in-session work via plan-dev after approval, rather than unattended autonomous or machine-executed work between approval and the pull request. Preserve the existing explanation of the two human gates and the machine-parsed sections.
51-61: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winQuality Gates comment still describes the discontinued runner's
done-verdict verification."as part of the self-check behind its
doneverdict" and "The runner itself executes no gates; it verifies the artifacts the session leaves behind... and refuses adonethat they contradict" describe exactly the autonomous verdict protocol this PR retires (plan-dev's skill now explicitly states there is no verdict schema). This is the template scaffolded into every new plan viacsdd plan init, so it ships stale guidance to every future plan author.🤖 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/templater/templates/plan/plan.md.tmpl` around lines 51 - 61, The Quality Gates comment in the plan template still documents the retired autonomous done-verdict and runner verification protocol. Update the comment under the “Quality Gates” section to remove references to the runner, done verdict, self-check, artifact verification, and refusal behavior, while retaining only current guidance about quality-gate commands and their execution timing.internal/templater/templates/root/knowledge-section.md.tmpl (1)
75-85: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winStale runner "done"/"continue" verdict protocol left in a shipped template.
This bullet, immediately following the rewritten 69-74 block, still describes the discontinued autonomous loop: a
done/continueverdict, a session declaringdoneon its own checks, and an "Audit the run afterwards indocs/plans/<slug>/log.md" step. This directly contradicts the new human-gated, in-session model described just above it (and inplan-dev's SKILL.md.tmpl, which now says there is no verdict schema at all). This file is shipped into every workspace viacsdd init, so it actively misleads users about how delivery works now.🤖 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/templater/templates/root/knowledge-section.md.tmpl` around lines 75 - 85, Remove the stale autonomous-loop guidance from the bullet following the rewritten block in the knowledge-section template, including the done/continue verdict protocol, session self-declaration, and post-run audit instructions. Replace it with wording consistent with the human-gated in-session workflow and the no-verdict-schema model established by the surrounding template guidance.
🧹 Nitpick comments (1)
internal/templater/templates/skills/prd/SKILL.md.tmpl (1)
132-136: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win"start the runner" leftover contradicts the "there is no runner" framing.
README.md now states plainly "There is no runner," and
plan-dev's SKILL.md.tmpl was swept clean of loop/runner language, but this line still tells the PRD skill not to "start the runner." Worth aligning wording (e.g., "or start delivering it yourself") so this skill's Gotchas don't reference a concept the rest of the PR retires.🤖 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/templater/templates/skills/prd/SKILL.md.tmpl` around lines 132 - 136, The PRD skill’s final instruction still references the retired “runner” concept. Update the guidance near the final csdd plan validation and approval steps to replace “start the runner” with wording consistent with the no-runner model, while preserving the instruction not to approve, generate specs, or begin delivery on the user’s behalf.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@internal/templater/templater_test.go`:
- Around line 580-627: Extend TestNoShippedArtifactMentionsPlanRun to inspect
templates/root/knowledge-section.md.tmpl, since csdd init renders it into the
workspace CLAUDE.md. Load the template through the existing Static helper or the
established init/render path and assert its contents do not contain “plan run,”
while preserving the current root and artifact checks.
---
Outside diff comments:
In `@internal/templater/templates/plan/plan.md.tmpl`:
- Around line 8-17: Update the introductory frontmatter comment in the plan
template to describe delivery as human-driven in-session work via plan-dev after
approval, rather than unattended autonomous or machine-executed work between
approval and the pull request. Preserve the existing explanation of the two
human gates and the machine-parsed sections.
- Around line 51-61: The Quality Gates comment in the plan template still
documents the retired autonomous done-verdict and runner verification protocol.
Update the comment under the “Quality Gates” section to remove references to the
runner, done verdict, self-check, artifact verification, and refusal behavior,
while retaining only current guidance about quality-gate commands and their
execution timing.
In `@internal/templater/templates/root/knowledge-section.md.tmpl`:
- Around line 75-85: Remove the stale autonomous-loop guidance from the bullet
following the rewritten block in the knowledge-section template, including the
done/continue verdict protocol, session self-declaration, and post-run audit
instructions. Replace it with wording consistent with the human-gated in-session
workflow and the no-verdict-schema model established by the surrounding template
guidance.
---
Nitpick comments:
In `@internal/templater/templates/skills/prd/SKILL.md.tmpl`:
- Around line 132-136: The PRD skill’s final instruction still references the
retired “runner” concept. Update the guidance near the final csdd plan
validation and approval steps to replace “start the runner” with wording
consistent with the no-runner model, while preserving the instruction not to
approve, generate specs, or begin delivery on the user’s behalf.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 779f05f4-064d-400f-8ce5-e08834ce8ea8
📒 Files selected for processing (24)
README.mdinternal/cli/cli.gointernal/cli/plan.gointernal/cli/plan_bridge_cli_test.gointernal/cli/plan_cli_test.gointernal/cli/plan_skill_test.gointernal/cli/plan_telegram_test.gointernal/cli/sandbox.gointernal/cli/spec.gointernal/cli/telegram.gointernal/plan/brief.gointernal/plan/brief_test.gointernal/templater/templater_test.gointernal/templater/templates/agents/quality-gate.md.tmplinternal/templater/templates/commands/csdd-plan-run.md.tmplinternal/templater/templates/commands/prd.md.tmplinternal/templater/templates/plan/plan.md.tmplinternal/templater/templates/root/CLAUDE.md.tmplinternal/templater/templates/root/knowledge-section.md.tmplinternal/templater/templates/sandbox/Dockerfile.tmplinternal/templater/templates/sandbox/devcontainer.json.tmplinternal/templater/templates/sandbox/init-firewall.sh.tmplinternal/templater/templates/skills/plan-dev/SKILL.md.tmplinternal/templater/templates/skills/prd/SKILL.md.tmpl
💤 Files with no reviewable changes (3)
- internal/templater/templates/commands/csdd-plan-run.md.tmpl
- internal/cli/plan_telegram_test.go
- internal/cli/plan_skill_test.go
|
|
||
| // TestNoShippedArtifactMentionsPlanRun sweeps everything `csdd init` writes into a | ||
| // workspace for the retired command. | ||
| // | ||
| // `csdd plan run` drove an autonomous loop and is discontinued; the CLI answers it | ||
| // with a refusal. What makes that stick is not the refusal but this sweep: the | ||
| // command was named across a skill, three sandbox files, two PRD surfaces, an agent | ||
| // and the root CLAUDE.md, and an instruction that survives in a shipped artifact is | ||
| // an instruction a model will follow — it will read "run the loop" long after the | ||
| // loop stopped existing. A hit here means a reference crept back in an edit. | ||
| // | ||
| // The one deliberate exception is templates/plan/CLAUDE.md.tmpl, which is the | ||
| // runner's own entry document. The runner is on standby, not deleted, and that file | ||
| // is never installed into a workspace — nothing reads it unless the loop comes back. | ||
| func TestNoShippedArtifactMentionsPlanRun(t *testing.T) { | ||
| sets := map[string]func(fs.FS) (map[string]string, error){ | ||
| "agents": AgentFiles, | ||
| "skills": SkillFiles, | ||
| "rules": RuleFiles, | ||
| "commands": CommandFiles, | ||
| "workflow": WorkflowTemplateFiles, | ||
| } | ||
| for label, load := range sets { | ||
| files, err := load(FS) | ||
| if err != nil { | ||
| t.Fatalf("%s: %v", label, err) | ||
| } | ||
| for name, body := range files { | ||
| if strings.Contains(body, "plan run") { | ||
| t.Errorf("%s/%s still instructs `plan run`; the loop is discontinued", label, name) | ||
| } | ||
| } | ||
| } | ||
| root, err := Static(FS, "templates/root/CLAUDE.md.tmpl") | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| if strings.Contains(root, "plan run") { | ||
| t.Error("the root CLAUDE.md still mentions `plan run`") | ||
| } | ||
| // The human gate is unconditional again: the exception that let a plan session | ||
| // approve its own spec phases existed only for the loop. | ||
| for _, gone := range []string{"sanctioned exception", "self-approves"} { | ||
| if strings.Contains(root, gone) { | ||
| t.Errorf("the root CLAUDE.md still carries the autonomous-loop carve-out (%q)", gone) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== files ==\n'
git ls-files | rg '^templates/root/|^internal/templater/|^cmd/|^internal/.*templater.*|^README.md$'
printf '\n== search knowledge-section / CLAUDE references ==\n'
rg -n 'knowledge-section|CLAUDE.md.tmpl|plan run|shipped artifact|root artifact|csdd init' templates internal cmd README.md
printf '\n== inspect templater test file around the cited lines ==\n'
sed -n '540,660p' internal/templater/templater_test.go
printf '\n== inspect root template files ==\n'
for f in templates/root/CLAUDE.md.tmpl templates/root/knowledge-section.md.tmpl; do
if [ -f "$f" ]; then
echo "--- $f"
sed -n '1,220p' "$f"
fi
done
printf '\n== inspect templater loader helpers ==\n'
ast-grep outline internal/templater -g '*.go' --view expanded || trueRepository: protonspy/csdd
Length of output: 18081
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== internal/cli/init.go scaffold list ==\n'
sed -n '340,430p' internal/cli/init.go
printf '\n== internal/cli/claudemd.go ==\n'
sed -n '1,220p' internal/cli/claudemd.go
printf '\n== templater.go root-related helpers ==\n'
sed -n '120,260p' internal/templater/templater.go
printf '\n== search for knowledge-section template generation/tests ==\n'
rg -n 'knowledge-section\.md\.tmpl|knowledge-section|claudemd|Entry\(root\)|templates/root/CLAUDE\.md\.tmpl' internal/cli internal/templaterRepository: protonspy/csdd
Length of output: 13685
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== templates/root/CLAUDE.md.tmpl (relevant sections) ==\n'
nl -ba internal/templater/templates/root/CLAUDE.md.tmpl | sed -n '1,260p'
printf '\n== templates/root/knowledge-section.md.tmpl ==\n'
nl -ba internal/templater/templates/root/knowledge-section.md.tmpl | sed -n '1,220p'
printf '\n== init calls to ensureKnowledgeSection ==\n'
rg -n 'ensureKnowledgeSection|ensureSteeringImports|initWorkspace\\(' internal/cli
printf '\n== test coverage around init output / CLAUDE.md contents ==\n'
sed -n '1,120p' internal/templater/templater_test.go
sed -n '120,220p' internal/templater/templater_test.go
sed -n '220,340p' internal/templater/templater_test.goRepository: protonspy/csdd
Length of output: 248
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== templates/root/CLAUDE.md.tmpl (first 260 lines) ==\n'
sed -n '1,260p' internal/templater/templates/root/CLAUDE.md.tmpl | cat -n
printf '\n== templates/root/knowledge-section.md.tmpl ==\n'
sed -n '1,220p' internal/templater/templates/root/knowledge-section.md.tmpl | cat -n
printf '\n== init calls to ensureKnowledgeSection / ensureSteeringImports ==\n'
rg -n 'ensureKnowledgeSection|ensureSteeringImports|initWorkspace\\(' internal/cli
printf '\n== templater tests around shipped artifacts and init output ==\n'
sed -n '1,220p' internal/templater/templater_test.go | cat -nRepository: protonspy/csdd
Length of output: 21615
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== ensureKnowledgeSection call sites ==\n'
rg -n -F 'ensureKnowledgeSection' internal/cli internal/templater
printf '\n== initWorkspace tail / post-init wiring ==\n'
sed -n '430,560p' internal/cli/init.go
printf '\n== claudemd usage from CLI commands ==\n'
rg -n -F 'claudemd.go' internal/cli internalRepository: protonspy/csdd
Length of output: 4178
Add templates/root/knowledge-section.md.tmpl to this sweep. csdd init injects that template into CLAUDE.md, so a stale plan run reference there can still ship to initialized workspaces even though this test never reads it. Either include it here or exercise the same init/render path.
🤖 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/templater/templater_test.go` around lines 580 - 627, Extend
TestNoShippedArtifactMentionsPlanRun to inspect
templates/root/knowledge-section.md.tmpl, since csdd init renders it into the
workspace CLAUDE.md. Load the template through the existing Static helper or the
established init/render path and assert its contents do not contain “plan run,”
while preserving the current root and artifact checks.
The autonomous loop cost an order of magnitude more than the same work done interactively and frequently delivered nothing. That is its design, not a defect in it: a session was rebuilt from zero per feat AND per attempt, so every discovery was re-paid at orchestrator prices with a cold cache, and a `done` the verdict gate refused threw away a session already paid for in full. So the command answers with a refusal that names where the work moved — `plan next` picks the feat, `plan brief` is its context, the `plan-dev` skill is the method. Everything else about plans is unchanged: init, list, validate, approve, status, next, brief, generate. `internal/plan` — runner, worktrees, ledger, watchdog, verdict gate — is untouched and still tested. The idea is on standby, and reviving it should be a revert rather than a rewrite. Three consequences follow, and they are the actual change: The human gate is unconditional again. The "one sanctioned exception" in the root CLAUDE.md let a plan session approve its own validated spec phases; it existed only because the loop had no human in it. Every phase is approved by a human now, and `plan-dev` presents artifacts instead of approving them. The brief is context and nothing else. The role preamble and the verdict protocol addressed a headless session whose entire prompt was that text. A session reading a brief now has its own CLAUDE.md, its skills and a human in the room, so the brief holds what only the plan knows — the feat, its governors, where it lives, what is already there — and stops. Every shipped artifact stops naming the command. It was in a skill, three sandbox files, two PRD surfaces, an agent and the root CLAUDE.md, and an instruction that survives in a shipped artifact is one a model will follow. TestNoShippedArtifactMentionsPlanRun sweeps them so it cannot creep back. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARBSaXgJ7ssW1hvpy6r64y
4555e14 to
4e27dd8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@README.md`:
- Line 296: Update the fenced ASCII diagram in the README to declare the text
language on its opening fence, using a text fence while preserving the diagram
content and rendering.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 28a65cbe-353b-4eed-9187-4fbcf06bc130
📒 Files selected for processing (24)
README.mdinternal/cli/cli.gointernal/cli/plan.gointernal/cli/plan_bridge_cli_test.gointernal/cli/plan_cli_test.gointernal/cli/plan_skill_test.gointernal/cli/plan_telegram_test.gointernal/cli/sandbox.gointernal/cli/spec.gointernal/cli/telegram.gointernal/plan/brief.gointernal/plan/brief_test.gointernal/templater/templater_test.gointernal/templater/templates/agents/quality-gate.md.tmplinternal/templater/templates/commands/csdd-plan-run.md.tmplinternal/templater/templates/commands/prd.md.tmplinternal/templater/templates/plan/plan.md.tmplinternal/templater/templates/root/CLAUDE.md.tmplinternal/templater/templates/root/knowledge-section.md.tmplinternal/templater/templates/sandbox/Dockerfile.tmplinternal/templater/templates/sandbox/devcontainer.json.tmplinternal/templater/templates/sandbox/init-firewall.sh.tmplinternal/templater/templates/skills/plan-dev/SKILL.md.tmplinternal/templater/templates/skills/prd/SKILL.md.tmpl
💤 Files with no reviewable changes (3)
- internal/cli/plan_skill_test.go
- internal/templater/templates/commands/csdd-plan-run.md.tmpl
- internal/cli/plan_telegram_test.go
🚧 Files skipped from review as they are similar to previous changes (18)
- internal/templater/templates/plan/plan.md.tmpl
- internal/templater/templates/agents/quality-gate.md.tmpl
- internal/templater/templates/sandbox/Dockerfile.tmpl
- internal/templater/templates/sandbox/devcontainer.json.tmpl
- internal/cli/telegram.go
- internal/templater/templates/commands/prd.md.tmpl
- internal/templater/templates/skills/prd/SKILL.md.tmpl
- internal/templater/templates/root/knowledge-section.md.tmpl
- internal/templater/templater_test.go
- internal/cli/plan_bridge_cli_test.go
- internal/cli/spec.go
- internal/plan/brief.go
- internal/cli/sandbox.go
- internal/templater/templates/skills/plan-dev/SKILL.md.tmpl
- internal/cli/plan_cli_test.go
- internal/plan/brief_test.go
- internal/templater/templates/root/CLAUDE.md.tmpl
- internal/cli/plan.go
| ### Two human gates, machine work between | ||
| ### Delivering a plan in your session | ||
|
|
||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Specify a language for the fenced diagram.
markdownlint reports MD040 here. Use ```text for this ASCII diagram to preserve rendering and satisfy the lint rule.
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 296-296: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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` at line 296, Update the fenced ASCII diagram in the README to
declare the text language on its opening fence, using a text fence while
preserving the diagram content and rendering.
Source: Linters/SAST tools
Why
The autonomous loop cost an order of magnitude more than the same work done interactively and frequently delivered nothing. That is its design, not a defect in it:
donethe verdict gate refused threw away a session already paid for in full;startedrows that never settled.What changes
csdd plan runanswers with a refusal naming where the work moved (exit 1):Everything else about plans is unchanged:
init,list,validate,approve,status,next,brief,generate.internal/plan— runner, worktrees, ledger, watchdog, verdict gate — is untouched and still tested: the idea is on standby, and reviving it should be a revert rather than a rewrite.Three consequences follow, and they are the real change:
1 · The human gate is unconditional again
The "One sanctioned exception — the autonomous
csdd plan runloop" paragraph in the rootCLAUDE.mdlet a plan session approve its own validated spec phases. It existed only because the loop had no human in it. It is gone: STOP rules 2, 4 and 5 now govern without exception, andplan-devpresents each artifact forcsdd spec approveinstead of calling it.2 · The brief is context and nothing else
The role preamble (
You are a senior software engineer…) and the verdict protocol ({"status":"done"}, "do not stop for a human at a phase gate") addressed a headless session whose entire prompt was that text. A session reading a brief now already has itsCLAUDE.md, its skills and a human in the room. So the brief holds what only the plan knows — the feat, its governors, where it lives, what is already there, what is missing, the pitfalls, the gates — and stops. AI enrichment stays exactly as it is; only the methodology around it is gone.3 · No shipped artifact names the command
It was in the
plan-devskill, three sandbox files, both PRD surfaces, thequality-gateagent,knowledge-section,plan.md's own header comment and the rootCLAUDE.md. An instruction that survives in a shipped artifact is one a model will follow — it would read "run the loop" long after the loop stopped existing./csdd-plan-runis deleted; the rest is reworded to the in-session flow.plan-devkeeps its name, its Tier-1/2/3 discipline, its delegation tospec-author/implementer/quality-gate/code-reviewer, and its feat-exit checks. It loses the authority carve-out, the verdict schema, the attempt budget and the worktree framing, and gains how a feat is picked (plan next→plan brief). One feat per invocation — no walking to the next.Verification
make checkgreen (gofmt + vet +go test -race, every package)../csdd plan run photos→ the refusal above, exit 1../csdd initinto a fresh workspace → noplan run, no "sanctioned exception", nocsdd-plan-run.md../csdd skill validate plan-devpasses on the rewritten skill.TestNoShippedArtifactMentionsPlanRunsweeps every shipped set (agents, skills, rules, commands, workflow templates, root CLAUDE.md) for the string and fails on a hit — the one deliberate exception,templates/plan/CLAUDE.md.tmpl, is the standby runner's own entry doc and is never installed into a workspace.TestPlanRunIsDiscontinuedrefuses on an approved plan (the case that used to run), asserting exit 1 and the path forward.TestBriefCarriesNoProcessnow asserts the brief does not carryplan-dev,CLAUDE.mdor the verdict shape — the inverse of what it asserted before.TestPlanRunSquadLimitBounds,plan_telegram_test.go(the run-scoped notifier auto-start;csdd telegram runis unaffected).TestMisspelledFlagNamesItsNeighbourmoved toplan brief— it guards the CLI's typo suggestion, not any one command.🤖 Generated with Claude Code
https://claude.ai/code/session_01ARBSaXgJ7ssW1hvpy6r64y
Summary by CodeRabbit
csdd plan run; it now refuses and directs you to the replacementplan next/plan briefworkflow.