diff --git a/internal/agent/premature_end_turn.go b/internal/agent/premature_end_turn.go index 023e5918..543984d9 100644 --- a/internal/agent/premature_end_turn.go +++ b/internal/agent/premature_end_turn.go @@ -28,19 +28,33 @@ var prematureActionPrefixes = []string{ "then ", "finally ", "continue ", + "continuing ", "start ", + "starting ", "retry ", + "retrying ", "rerun ", + "rerunning ", "re-run ", + "re-running ", "check ", + "checking ", "inspect ", + "inspecting ", "verify ", + "verifying ", "run ", + "running ", "execute ", + "executing ", "update ", + "updating ", "edit ", + "editing ", "write ", + "writing ", "fix ", + "fixing ", "我来", "我先", "我现在", @@ -74,11 +88,14 @@ var prematureActionPrefixes = []string{ "跑", } -// shouldRecoverPrematureEndTurn recognizes the narrow failure shape observed -// in DeepSeek sessions: an Agent-mode reply stops at an action lead-in ending -// in a colon, yet contains no structured tool call. Requiring both the dangling -// colon and an immediate-action prefix avoids retrying ordinary final answers, -// questions, headings, Plan replies, and tool-suppressed internal requests. +// shouldRecoverPrematureEndTurn catches an Agent-mode reply that stops at an +// action lead-in without issuing a tool call. A trailing colon alone triggers +// recovery: a colon-terminated final answer invites continuation, and the +// nudge's escape hatch plus the 2-nudge cap bound a false positive to one extra +// model call. Non-colon text falls back to the action-prefix scan, which also +// matches inflected -ing forms (fixing, running, updating), so the known +// "."-terminated instance and gerund lead-ins are still caught. Outer gates +// stay: Agent mode, tools available, SuppressTools off, end_turn, no tool calls. func shouldRecoverPrematureEndTurn(msg core.Message, mode session.Mode, opts RunOptions, toolsAvailable bool) bool { if mode != session.ModeAgent || opts.SuppressTools || !toolsAvailable { return false @@ -87,11 +104,11 @@ func shouldRecoverPrematureEndTurn(msg core.Message, mode session.Mode, opts Run return false } text := strings.TrimSpace(msg.Text) - if !strings.HasSuffix(text, ":") && !strings.HasSuffix(text, "：") { - return false + if strings.HasSuffix(text, ":") || strings.HasSuffix(text, "：") { + return true } - clause := trailingActionClause(strings.TrimSpace(strings.TrimSuffix(strings.TrimSuffix(text, ":"), "："))) + clause := trailingActionClause(text) clause = strings.ToLower(strings.TrimSpace(clause)) for _, prefix := range prematureActionPrefixes { if strings.HasPrefix(clause, prefix) { diff --git a/internal/agent/premature_end_turn_test.go b/internal/agent/premature_end_turn_test.go index bfb1ab48..4172d0ea 100644 --- a/internal/agent/premature_end_turn_test.go +++ b/internal/agent/premature_end_turn_test.go @@ -122,11 +122,19 @@ func TestShouldRecoverPrematureEndTurn(t *testing.T) { tests := []struct { name string text string + finishReason core.FinishReason + toolCalls []core.ToolCall mode session.Mode opts RunOptions toolsAvailable bool want bool }{ + // Colon-terminated lead-ins: the colon is the primary trigger since the + // guard was widened. 3 of 4 known announce-then-stop instances end in + // ":", including the gerund forms the old verb-prefix scan missed + // ("Fixing:", "Running tests:"). + {name: "gerund lead-in fixing colon", text: "a deliberate asymmetry the comment now contradicts. Fixing:", mode: session.ModeAgent, toolsAvailable: true, want: true}, + {name: "running tests lead-in", text: "Running tests:", mode: session.ModeAgent, toolsAvailable: true, want: true}, {name: "observed chinese verify lead-in", text: "现在验证 workflow 语法：", mode: session.ModeAgent, toolsAvailable: true, want: true}, {name: "observed chinese retry lead-in", text: "API 没改到权限。换端点重试：", mode: session.ModeAgent, toolsAvailable: true, want: true}, {name: "observed chinese patch lead-in", text: "第一个 edit 没改到 description，我写重复了，补上：", mode: session.ModeAgent, toolsAvailable: true, want: true}, @@ -138,22 +146,85 @@ func TestShouldRecoverPrematureEndTurn(t *testing.T) { {name: "chinese next step lead-in", text: "下一步检查 CI：", mode: session.ModeAgent, toolsAvailable: true, want: true}, {name: "english action lead-in", text: "The config is present. Now verify the workflow:", mode: session.ModeAgent, toolsAvailable: true, want: true}, {name: "english final action lead-in", text: "The checks passed. Finally merge the pull request:", mode: session.ModeAgent, toolsAvailable: true, want: true}, + {name: "then commit colon", text: "The diff is staged, then commit:", mode: session.ModeAgent, toolsAvailable: true, want: true}, + {name: "inspect plus resolve colon", text: "Inspect + resolve keeping both:", mode: session.ModeAgent, toolsAvailable: true, want: true}, + // Non-colon fallback: the one known "."-terminated announce-then-stop + // instance is caught by the trailing action-clause prefix scan. + {name: "dot-terminated action clause fallback", text: "Amend tests into commit, then append maintainer reply draft.", mode: session.ModeAgent, toolsAvailable: true, want: true}, + // Non-colon inflected lead-ins: the prefix scan also matches -ing + // gerund forms, so "fixing"/"running"/"updating"/"writing" without a + // trailing colon are recovered instead of slipping through. + {name: "non-colon gerund fixing", text: "The typo is clear. Fixing the permission check", mode: session.ModeAgent, toolsAvailable: true, want: true}, + {name: "non-colon gerund running", text: "The suite is ready. Running the tests", mode: session.ModeAgent, toolsAvailable: true, want: true}, + {name: "non-colon gerund updating", text: "Rebuild needed. Updating the dependency", mode: session.ModeAgent, toolsAvailable: true, want: true}, + {name: "non-colon gerund writing", text: "Understood. Writing the patch", mode: session.ModeAgent, toolsAvailable: true, want: true}, + {name: "non-colon gerund retrying", text: "Rate limited. Retrying the request", mode: session.ModeAgent, toolsAvailable: true, want: true}, + // Accepted colon false positives: bounded by the 2-nudge cap, harmless + // per the nudge's "If no action remains, provide the complete final + // answer" escape hatch. + {name: "fixture analysis colon accepted", text: "The fixture analysis:", mode: session.ModeAgent, toolsAvailable: true, want: true}, + {name: "ordinary heading accepted", text: "Details:", mode: session.ModeAgent, toolsAvailable: true, want: true}, + {name: "ordinary final heading accepted", text: "最后：", mode: session.ModeAgent, toolsAvailable: true, want: true}, + {name: "user choice prompt accepted", text: "请选择：", mode: session.ModeAgent, toolsAvailable: true, want: true}, + // Negatives: no colon and no action-prefix clause stay final answers. {name: "complete answer", text: "The workflow is valid.", mode: session.ModeAgent, toolsAvailable: true}, - {name: "ordinary heading", text: "Details:", mode: session.ModeAgent, toolsAvailable: true}, - {name: "ordinary final heading", text: "最后：", mode: session.ModeAgent, toolsAvailable: true}, - {name: "user choice prompt", text: "请选择：", mode: session.ModeAgent, toolsAvailable: true}, + {name: "complete answer the fix is simple", text: "The fix is simple.", mode: session.ModeAgent, toolsAvailable: true}, + {name: "final answer with future-tense clause", text: "The user will decide the direction.", mode: session.ModeAgent, toolsAvailable: true}, + {name: "colon heading with content", text: "Details: the workflow is valid.", mode: session.ModeAgent, toolsAvailable: true}, {name: "plan reply", text: "接下来执行：", mode: session.ModePlan, toolsAvailable: true}, {name: "ask reply", text: "现在检查：", mode: session.ModeAsk, toolsAvailable: true}, {name: "tools suppressed", text: "Now verify:", mode: session.ModeAgent, opts: RunOptions{SuppressTools: true}, toolsAvailable: true}, {name: "no tools available", text: "Now verify:", mode: session.ModeAgent, toolsAvailable: false}, + // Remaining outer gates: non-end-turn finishes and turns that already + // carried a structured tool call are never recovered. + {name: "tool use finish reason", text: "Now verify:", finishReason: core.FinishReasonToolUse, mode: session.ModeAgent, toolsAvailable: true}, + {name: "cancelled finish reason", text: "Now verify:", finishReason: core.FinishReasonCanceled, mode: session.ModeAgent, toolsAvailable: true}, + {name: "has tool calls", text: "Now verify:", toolCalls: []core.ToolCall{{ID: "tc-1", Name: "echo", Input: "{}"}}, mode: session.ModeAgent, toolsAvailable: true}, + // Text-shape edges around the colon check. + {name: "trailing whitespace after colon", text: "Fixing: \n", mode: session.ModeAgent, toolsAvailable: true, want: true}, + {name: "bare colon", text: ":", mode: session.ModeAgent, toolsAvailable: true, want: true}, + {name: "bare fullwidth colon", text: "：", mode: session.ModeAgent, toolsAvailable: true, want: true}, + {name: "empty text", text: " ", mode: session.ModeAgent, toolsAvailable: true}, + {name: "no colon no prefix bullet", text: "The output is ready.", mode: session.ModeAgent, toolsAvailable: true}, + {name: "non-action gerund not matched", text: "The response was helpful. Singing the final note is next.", mode: session.ModeAgent, toolsAvailable: true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { msg := base msg.Text = tt.text + if tt.finishReason != "" { + msg.FinishReason = tt.finishReason + } + msg.ToolCalls = tt.toolCalls if got := shouldRecoverPrematureEndTurn(msg, tt.mode, tt.opts, tt.toolsAvailable); got != tt.want { t.Fatalf("shouldRecoverPrematureEndTurn(%q) = %v, want %v", tt.text, got, tt.want) } }) } } + +func TestTrailingActionClause(t *testing.T) { + tests := []struct { + name string + text string + want string + }{ + {name: "sentence boundary", text: "The checks passed. Then merge it", want: "Then merge it"}, + {name: "newline boundary", text: "Step one done\nNext verify", want: "Next verify"}, + {name: "comma clause", text: "commit, then append reply", want: "then append reply"}, + {name: "chinese sentence boundary", text: "第一步完成。然后验证", want: "然后验证"}, + {name: "chinese comma boundary", text: "检查完毕，补上说明", want: "补上说明"}, + {name: "no boundary keeps whole", text: "fix the permission check", want: "fix the permission check"}, + {name: "strips list markers", text: "Amend tests:\n- then run them", want: "then run them"}, + {name: "strips numbered marker", text: "Steps:\n2) then verify", want: "then verify"}, + {name: "strips heading marker", text: "Steps:\n## then verify", want: "then verify"}, + {name: "empty input", text: "", want: ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := trailingActionClause(tt.text); got != tt.want { + t.Fatalf("trailingActionClause(%q) = %q, want %q", tt.text, got, tt.want) + } + }) + } +} diff --git a/internal/agent/system_prompt.go b/internal/agent/system_prompt.go index 582029d8..ec2354ed 100644 --- a/internal/agent/system_prompt.go +++ b/internal/agent/system_prompt.go @@ -33,6 +33,7 @@ func (a *Agent) buildImmutableSystemBlocksWithTools(_ *core.ToolRegistry, opts . systemBlocks = append(systemBlocks, "For questions about the current date or time, use an available read-only shell/time command to verify the answer instead of guessing from model memory.") systemBlocks = append(systemBlocks, renderToolPolicyBlock()) systemBlocks = append(systemBlocks, "For branch decisions or key assumptions requiring user choice, call request_user_input instead of presenting long A/B/C prose menus.") + systemBlocks = append(systemBlocks, "Execute, don't narrate: never end a turn by announcing further work — do it now, in this turn, with tool calls when the work requires an available tool, otherwise finish directly. A turn ends only when the user explicitly asked for it (e.g. \"stop\", \"summarize\", \"hand off\"), the task is genuinely complete, or you need user input.") return systemBlocks } diff --git a/internal/agent/system_prompt_test.go b/internal/agent/system_prompt_test.go index 71ac03f7..4898a290 100644 --- a/internal/agent/system_prompt_test.go +++ b/internal/agent/system_prompt_test.go @@ -513,3 +513,45 @@ func TestImmutableSystemPromptToolPolicyDoesNotDependOnToolRegistry(t *testing.T t.Fatalf("immutable system prompt changed with tool registry\nwithout tools:\n%s\n\nwith tools:\n%s", a, b) } } + +// TestImmutableSystemBlocksIncludeAnnounceThenStop verifies the announce- +// then-stop instruction is a static immutable block, so it reaches every +// entrypoint (ACP, terminal CLI, subagents) that shares the turn loop. +func TestImmutableSystemBlocksIncludeAnnounceThenStop(t *testing.T) { + a := NewAgentWithRegistry(nil, nil, core.NewToolRegistry(nil), WithProjectMemory(false, 0, nil, "/repo")) + joined := strings.Join(a.buildImmutableSystemBlocks(), "\n\n") + + for _, want := range []string{ + "Execute, don't narrate", + "never end a turn by announcing further work", + "do it now, in this turn, with tool calls", + `"stop"`, + } { + if !strings.Contains(joined, want) { + t.Fatalf("announce-then-stop block missing %q:\n%s", want, joined) + } + } + if got := strings.Count(joined, "Execute, don't narrate"); got != 1 { + t.Fatalf("announce-then-stop block appears %d times, want exactly once:\n%s", got, joined) + } + // The instruction is a contract, not runtime context: it must never depend + // on the tool registry or session mode, and it must not be duplicated into + // the runtime blocks (which are re-rendered per turn). + runtime := strings.Join(a.buildRuntimeSystemBlocks(), "\n\n") + if strings.Contains(runtime, "Execute, don't narrate") { + t.Fatalf("announce-then-stop block leaked into runtime blocks:\n%s", runtime) + } + modes := []session.Mode{session.ModeAgent, session.ModeAsk, session.ModePlan} + for _, m := range modes { + withMode := NewAgentWithRegistry(nil, nil, core.NewToolRegistry(nil), WithSessionMode(m)) + if got := strings.Join(withMode.buildImmutableSystemBlocks(), "\n\n"); !strings.Contains(got, "Execute, don't narrate") { + t.Fatalf("mode %v missing announce-then-stop block", m) + } + } + // Immutable blocks are shared by every entrypoint via the turn loop; the + // tool registry must not change the block (it is appended unconditionally). + withTools := NewAgentWithRegistry(nil, nil, core.NewToolRegistry([]core.Tool{&recordingWorkflowTool{}})) + if got := strings.Join(withTools.buildImmutableSystemBlocks(), "\n\n"); !strings.Contains(got, "Execute, don't narrate") { + t.Fatalf("with-tools agent missing announce-then-stop block") + } +}