Skip to content

Commit ec9db42

Browse files
committed
fix(runtime): address plan todo review feedback
1 parent 67d0d97 commit ec9db42

7 files changed

Lines changed: 78 additions & 6 deletions

File tree

internal/context/builder_test.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,36 @@ func TestDefaultBuilderBuildIncludesPlanSections(t *testing.T) {
187187
}
188188
}
189189

190+
func TestDefaultBuilderBuildPlanModeDoesNotRequireTodoWrite(t *testing.T) {
191+
t.Parallel()
192+
193+
builder := NewBuilder()
194+
got, err := builder.Build(stdcontext.Background(), BuildInput{
195+
AgentMode: agentsession.AgentModePlan,
196+
PlanStage: "plan",
197+
Metadata: testMetadata(t.TempDir()),
198+
})
199+
if err != nil {
200+
t.Fatalf("Build() error = %v", err)
201+
}
202+
if !strings.Contains(got.SystemPrompt, "Do not create execution todos in plan mode") {
203+
t.Fatalf("expected plan mode to forbid execution todo creation, got %q", got.SystemPrompt)
204+
}
205+
if !strings.Contains(got.SystemPrompt, "the current mode permits execution todo updates") {
206+
t.Fatalf("expected core todo guidance to be mode-gated, got %q", got.SystemPrompt)
207+
}
208+
for _, forbidden := range []string{
209+
"maintain explicit todos with `todo_write`.",
210+
"Maintain explicit task state and todos via `todo_write`.",
211+
"keep task state explicit via `todo_write` (plan/add/update/set_status/claim/complete/fail) instead of relying on implicit memory",
212+
"keep critical information in the task state using `todo_write` updates",
213+
} {
214+
if strings.Contains(got.SystemPrompt, forbidden) {
215+
t.Fatalf("plan mode prompt should not contain hard todo_write guidance %q in %q", forbidden, got.SystemPrompt)
216+
}
217+
}
218+
}
219+
190220
func TestDefaultBuilderBuildIncludesTodosBeforeSystemState(t *testing.T) {
191221
t.Parallel()
192222

internal/promptasset/assets_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ func TestCorePromptContainsOperationalGuidance(t *testing.T) {
4747
"A subagent is a helper, not the source of final truth",
4848
"Preserve existing user or repository changes",
4949
"Use UTF-8-safe reads and edits",
50+
"the current mode permits execution todo updates",
5051
}
5152
for _, want := range wantSubstrings {
5253
if !strings.Contains(prompt, want) {

internal/promptasset/templates/core/agent_identity.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ If instructions conflict, follow the higher-priority instruction and briefly sta
2020

2121
Core workflow:
2222
1. Observe — Locate the real entry points and existing patterns before acting. Prefer targeted search and file reads over assumptions.
23-
2. Plan — Choose the smallest coherent path that can satisfy the user request. For multi-step work, maintain explicit todos with `todo_write`.
23+
2. Plan — Choose the smallest coherent path that can satisfy the user request. For multi-step work, maintain explicit todos with `todo_write` only when that tool is available and the current mode permits execution todo updates.
2424
3. Act — Call the minimum set of exposed tools needed to make progress. Prefer filesystem tools over bash.
2525
4. Reconcile — Read each tool result carefully and let authoritative result fields guide the next step.
2626
5. Verify — After writes or edits, run the narrowest meaningful verification for the risk.
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
- The conversation context has a finite window. When the history grows large, earlier messages may be compacted into a durable `task_state` and a human-readable `display_summary`.
2-
- To cooperate with compaction, keep critical information in the task state using `todo_write` updates and explicit reasoning, rather than relying solely on conversational memory.
2+
- To cooperate with compaction, keep critical information in task state using `todo_write` updates only when that tool is available and the current mode permits execution todo updates; otherwise preserve the information in explicit reasoning and permitted outputs.
33
- After a compact occurs, the durable `task_state` and `display_summary` become your source of truth for what has been accomplished and what remains. Treat archived conversation content as historical reference, not as current instructions.
44
- When continuing after a compact, verify the current workspace state against the `task_state` before assuming files or changes from prior rounds still exist.
55
- Do not treat archived `[compact_summary]` text as durable truth. Durable truth comes from `current_task_state` plus new source material.
6-
- Keep long-running task facts, decisions, blockers, and acceptance-relevant todos in durable task state instead of relying only on conversation history.
6+
- Keep long-running task facts, decisions, blockers, and acceptance-relevant todos in durable task state when the current mode permits task-state updates, instead of relying only on conversation history.

internal/promptasset/templates/core/tool_usage.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ For general file operations outside of codebase exploration, use `filesystem_*`
3838
- create directory: `filesystem_create_dir` (not `bash mkdir`)
3939
- remove directory: `filesystem_remove_dir` (not `bash rmdir` / `rm -rf`)
4040
These tools record their changes for checkpoint/rollback; equivalent `bash` commands produce reduced rollback coverage.
41-
- For multi-step implementation, debugging, refactoring, or long-running work, keep task state explicit via `todo_write` (plan/add/update/set_status/claim/complete/fail) instead of relying on implicit memory.
41+
- For multi-step implementation, debugging, refactoring, or long-running work, keep task state explicit via `todo_write` (plan/add/update/set_status/claim/complete/fail) when that tool is available and the current mode permits execution todo updates.
4242
- Create todos that map to real acceptance work, not vague activity.
4343
- Required todos are acceptance-relevant and must converge before finalization.
4444
- If the user clearly switches to a different task, do not carry unfinished todos forward blindly: mark each old todo `completed` only when the work is actually done, otherwise mark it `canceled` before planning or executing the new task.

internal/runtime/todo_bootstrap.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,22 @@ func shouldInjectTodoBootstrapReminder(state *runState) bool {
3838
if agentsession.NormalizeAgentMode(session.AgentMode) != agentsession.AgentModeBuild {
3939
return false
4040
}
41-
if len(session.Todos) > 0 {
41+
if hasActiveTodoForBootstrap(session.Todos) {
4242
return false
4343
}
4444
return true
4545
}
4646

47+
// hasActiveTodoForBootstrap 判断会话中是否已有可继续推进的非终态 todo。
48+
func hasActiveTodoForBootstrap(todos []agentsession.TodoItem) bool {
49+
for _, todo := range todos {
50+
if !todo.Status.IsTerminal() {
51+
return true
52+
}
53+
}
54+
return false
55+
}
56+
4757
const planBootstrapRequiredReason = "plan_bootstrap_required"
4858

4959
const planBootstrapRequiredReminder = `[Runtime Control]

internal/runtime/todo_bootstrap_test.go

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ func TestShouldInjectTodoBootstrapReminder(t *testing.T) {
5252
want: true,
5353
},
5454
{
55-
name: "existing todo skips",
55+
name: "existing active todo skips",
5656
state: runState{
5757
session: agentsession.Session{
5858
AgentMode: agentsession.AgentModeBuild,
@@ -68,6 +68,37 @@ func TestShouldInjectTodoBootstrapReminder(t *testing.T) {
6868
},
6969
want: false,
7070
},
71+
{
72+
name: "terminal todos only still injects",
73+
state: runState{
74+
session: agentsession.Session{
75+
AgentMode: agentsession.AgentModeBuild,
76+
Todos: []agentsession.TodoItem{
77+
{
78+
ID: "todo-completed",
79+
Content: "done",
80+
Status: agentsession.TodoStatusCompleted,
81+
Required: &required,
82+
},
83+
{
84+
ID: "todo-failed",
85+
Content: "failed",
86+
Status: agentsession.TodoStatusFailed,
87+
Required: &required,
88+
},
89+
{
90+
ID: "todo-canceled",
91+
Content: "canceled",
92+
Status: agentsession.TodoStatusCanceled,
93+
Required: &required,
94+
},
95+
},
96+
},
97+
userGoal: "继续实现剩余工作",
98+
planningEnabled: true,
99+
},
100+
want: true,
101+
},
71102
{
72103
name: "plan mode skips",
73104
state: runState{

0 commit comments

Comments
 (0)