From 75f161463c4f0b32a2b12446fe2a2064d97b3205 Mon Sep 17 00:00:00 2001 From: syf2211 Date: Thu, 27 Aug 2026 00:09:16 +0000 Subject: [PATCH] fix(agent): keep model focused after failed tool calls Add a system-prompt guardrail and inject a per-round focus hint when built-in or MCP tools fail, covering TUI, headless, and ACP loops. Fixes #18 --- src/acp/agent.ts | 8 ++++++- src/agent/headless-agent.ts | 6 +++++ src/agent/system-prompt.ts | 1 + src/agent/tool-failure-focus.test.ts | 35 ++++++++++++++++++++++++++++ src/agent/tool-failure-focus.ts | 11 +++++++++ src/screens/repl.ts | 14 +++++++++++ 6 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 src/agent/tool-failure-focus.test.ts create mode 100644 src/agent/tool-failure-focus.ts diff --git a/src/acp/agent.ts b/src/acp/agent.ts index ee8358c..bb9d4be 100644 --- a/src/acp/agent.ts +++ b/src/acp/agent.ts @@ -32,6 +32,7 @@ import { initLocalDb } from "../tools/local-db.js"; import { compactMessagesForApi } from "../agent/compaction.js"; import { seedSystemMessages } from "../agent/system-prompt.js"; import { stripStrayTextToolCallArtifacts } from "../agent/text-tool-artifacts.js"; +import { isToolFailure, TOOL_FAILURE_FOCUS_HINT } from "../agent/tool-failure-focus.js"; import { checkPermission, loadPermissions, persistAlwaysAllow, SAFE_TOOLS, type PermDecision, type PermissionsFile, @@ -270,7 +271,7 @@ export class AcpAgent { // one, apply_patch can touch several — that one falls back to text). const oldText = kind === "edit" && name !== "apply_patch" && path && existsSync(path) ? safeRead(path) : null; const result = await executeTools(tc, projectRoot, client); - const failed = /^Error[:\s]/i.test(result); + const failed = isToolFailure(result); let content: ToolCallContent[]; if (kind === "edit" && name !== "apply_patch" && path && !failed) { @@ -363,11 +364,16 @@ export class AcpAgent { } loopRefusals = 0; + let roundHadToolFailure = false; for (const tc of pendingToolCalls) { if (state.cancelled) return { stopReason: "cancelled" as StopReason }; const result = await this.runTool(params.sessionId, state.projectRoot, client, tc, perms, sessionApproved); + if (isToolFailure(result)) roundHadToolFailure = true; state.messages.push({ role: "tool", content: result.slice(0, 20_000), tool_call_id: tc.id }); } + if (roundHadToolFailure) { + state.messages.push({ role: "system", content: TOOL_FAILURE_FOCUS_HINT }); + } continue; } diff --git a/src/agent/headless-agent.ts b/src/agent/headless-agent.ts index f2d013a..00daceb 100644 --- a/src/agent/headless-agent.ts +++ b/src/agent/headless-agent.ts @@ -14,6 +14,7 @@ import { KlaatAIClient, type Message, type ToolCall, type ToolDefinition } from import { executeTools, TOOL_DEFINITIONS } from "../tools/index.js"; import { compactMessagesForApi } from "./compaction.js"; import { costUsd } from "../pricing.js"; +import { isToolFailure, TOOL_FAILURE_FOCUS_HINT } from "./tool-failure-focus.js"; export interface HeadlessResult { finalText: string; @@ -166,12 +167,17 @@ export async function runHeadlessAgent( } loopRefusals = 0; + let roundHadToolFailure = false; for (const tc of pendingToolCalls) { const out = await executeTools(tc, projectRoot, client); res.toolCalls += 1; opts.onProgress?.({ kind: "tool", detail: tc.function.name }); + if (isToolFailure(out)) roundHadToolFailure = true; apiMessages = [...apiMessages, { role: "tool", content: out.slice(0, 20_000), tool_call_id: tc.id }]; } + if (roundHadToolFailure) { + apiMessages = [...apiMessages, { role: "system", content: TOOL_FAILURE_FOCUS_HINT }]; + } continue; } diff --git a/src/agent/system-prompt.ts b/src/agent/system-prompt.ts index 8c72404..86e9e53 100644 --- a/src/agent/system-prompt.ts +++ b/src/agent/system-prompt.ts @@ -61,6 +61,7 @@ You ALWAYS have filesystem and shell access through your tools (read_file, run_c - For a scoped sub-problem that needs many steps, use delegate_task so the main conversation stays small: agent "explore" for read-only search (several in one turn run in parallel), "review" for code review, "build" for scoped implementation. Only the agent's final report enters this conversation. - For long or independent side-work, add background:true to delegate_task — it returns a task id immediately so you keep working; poll with task_status(id), and a note appears when it finishes. Never idle-wait on a background task. - Maintain todo_write for multi-step tasks so the user can see progress; mark items done as you finish them. +- When a tool call fails, stay on the user's request: retry with corrected input, ask for clarification with ask_user, or explain what blocked you. Do NOT pivot to unrelated tools or tasks. # Editing discipline diff --git a/src/agent/tool-failure-focus.test.ts b/src/agent/tool-failure-focus.test.ts new file mode 100644 index 0000000..e38c7b8 --- /dev/null +++ b/src/agent/tool-failure-focus.test.ts @@ -0,0 +1,35 @@ +import { expect, test } from "bun:test"; +import { isToolFailure, TOOL_FAILURE_FOCUS_HINT } from "./tool-failure-focus.js"; + +test("isToolFailure: built-in Error prefix", () => { + expect(isToolFailure("Error: File not found: foo.ts")).toBe(true); +}); + +test("isToolFailure: MCP tool error prefix", () => { + expect(isToolFailure("MCP tool error (browser/navigate): 404 Not Found")).toBe(true); +}); + +test("isToolFailure: success results", () => { + expect(isToolFailure("Wrote 42 bytes to src/foo.ts")).toBe(false); + expect(isToolFailure("[exit 0]\nok")).toBe(false); +}); + +test("isToolFailure: permission and MCP transport errors", () => { + expect(isToolFailure("Error: User denied permission for this tool call.")).toBe(true); + expect(isToolFailure('Error: MCP server "browser" is not connected (status: error)')).toBe(true); + expect(isToolFailure('Error calling MCP tool "browser/navigate": timeout')).toBe(true); +}); + +test("isToolFailure: run_command non-zero exit", () => { + expect(isToolFailure("[exit 1]\ncommand failed")).toBe(true); + expect(isToolFailure("[exit 0]\nok")).toBe(false); +}); + +test("isToolFailure: doom-loop refusal is not a failure", () => { + expect(isToolFailure("Refused: doom-loop detected — change approach.")).toBe(false); +}); + +test("TOOL_FAILURE_FOCUS_HINT mentions staying focused", () => { + expect(TOOL_FAILURE_FOCUS_HINT).toContain("Stay focused"); + expect(TOOL_FAILURE_FOCUS_HINT).toContain("ask_user"); +}); diff --git a/src/agent/tool-failure-focus.ts b/src/agent/tool-failure-focus.ts new file mode 100644 index 0000000..38ef42b --- /dev/null +++ b/src/agent/tool-failure-focus.ts @@ -0,0 +1,11 @@ +/** Detect tool results that represent a failure (built-in or MCP). */ +export function isToolFailure(result: string): boolean { + if (result.startsWith("Refused:")) return false; + if (result.startsWith("Error") || result.startsWith("MCP tool error")) return true; + const exitMatch = result.match(/^\[exit (\d+)\]/); + return exitMatch !== null && exitMatch[1] !== "0"; +} + +/** Injected after a failed tool round so the model stays on the user's request. */ +export const TOOL_FAILURE_FOCUS_HINT = + "Previous tool call(s) failed. Stay focused on the user's original request — retry with corrected input, ask the user with ask_user if unclear, or explain what blocked you. Do NOT pivot to unrelated tools or tasks."; diff --git a/src/screens/repl.ts b/src/screens/repl.ts index 280a1a3..ff56482 100644 --- a/src/screens/repl.ts +++ b/src/screens/repl.ts @@ -78,6 +78,7 @@ import { COMPACTION_PROMPT, extractSummary, MAX_CONSECUTIVE_COMPACT_FAILURES } f import { compactMessagesForApi } from "../agent/compaction.js"; import { stripStrayTextToolCallArtifacts, maskTextToolXmlForDisplay } from "../agent/text-tool-artifacts.js"; import { looksLikeUnfulfilledActionPromise } from "../agent/action-promise.js"; +import { isToolFailure, TOOL_FAILURE_FOCUS_HINT } from "../agent/tool-failure-focus.js"; import { loadMemory, buildDistillationMessages, parseDistillation, flattenTranscriptTail, writeProjectMemory, writeUserMemory, clearMemory, DISTILL_EVERY_USER_TURNS, @@ -4155,6 +4156,7 @@ export async function runREPL( SAFE_TOOLS.has(t.function.name) || (t.function.name === "delegate_task" && getPersona(parseDelegateArgs(t).agent).readonly); const batches: ToolCall[][] = []; + let roundHadToolFailure = false; for (const tc of pendingToolCalls) { const last = batches[batches.length - 1]; if (isBatchable(tc) && last && isBatchable(last[0]!)) { @@ -4207,6 +4209,7 @@ export async function runREPL( for (let bi = 0; bi < batch.length; bi++) { const tc = batch[bi]!; const toolResult = batchResults[bi]!; + if (isToolFailure(toolResult)) roundHadToolFailure = true; const toolLines = toolResult.split("\n").length; const editDiff = toolResult.startsWith("Error") ? undefined : diffForTool(tc); const toolMsg = placeholders[bi]!; @@ -4321,6 +4324,17 @@ export async function runREPL( app.requestRender(); } } + if (roundHadToolFailure) { + currentApiMessages = [ + ...currentApiMessages, + { role: "system", content: TOOL_FAILURE_FOCUS_HINT }, + ]; + messages.push({ + role: "system", + content: "↻ Tool failure — focus reminder injected.", + }); + chatLinesDirty = true; + } if (interrupted) break outerLoop; // 9.5: reclassify the agent phase from this round's tools. phaseTracker.noteTools(pendingToolCalls.map(t => t.function.name));