From a5deb6e92794b2df3f60911b7a831b1bb7bcbdfd Mon Sep 17 00:00:00 2001 From: shoryabansalgithub Date: Tue, 18 Aug 2026 22:07:24 +0530 Subject: [PATCH 01/17] feat: configurable agent-loop retry limits with pause-and-ask on tool loops Adds a nanocoder.retries section to agents.config.json exposing the previously hardcoded conversation-loop caps: maxRepeatedToolCalls (default 3), maxEmptyTurns (default 2), maxMalformedRetries (default 2). Defaults preserve existing behaviour exactly. When the repeated-tool-call limit is hit in an interactive session the loop now pauses and asks the user whether to continue (granting another window of attempts) or stop, instead of always hard-stopping. Headless and non-interactive runs keep the hard stop since there is nobody to ask. Named 'retries' rather than the issue's proposed 'maxRetries' because maxRetries is already a public per-provider setting that caps network request retries. Closes #897 --- .changeset/configurable-retry-limits.md | 5 + docs/configuration/index.md | 24 ++ source/config/index.spec.ts | 133 ++++++++ source/config/index.ts | 62 ++++ .../conversation/conversation-loop.spec.ts | 300 ++++++++++++++++++ .../conversation/conversation-loop.tsx | 92 ++++-- source/types/config.ts | 18 ++ 7 files changed, 611 insertions(+), 23 deletions(-) create mode 100644 .changeset/configurable-retry-limits.md diff --git a/.changeset/configurable-retry-limits.md b/.changeset/configurable-retry-limits.md new file mode 100644 index 000000000..b1d3cd3ee --- /dev/null +++ b/.changeset/configurable-retry-limits.md @@ -0,0 +1,5 @@ +--- +"@nanocollective/nanocoder": minor +--- + +Add configurable agent-loop retry limits to prevent token drain (#897). A new `nanocoder.retries` section in `agents.config.json` exposes the previously hardcoded caps: `maxRepeatedToolCalls` (default 3), `maxEmptyTurns` (default 2), and `maxMalformedRetries` (default 2). When the repeated-tool-call limit is hit in an interactive session, Nanocoder now pauses and asks whether to continue (granting another window of attempts) or stop, instead of always hard-stopping; non-interactive runs keep the hard stop. diff --git a/docs/configuration/index.md b/docs/configuration/index.md index 5dcecebfa..bce6acdd1 100644 --- a/docs/configuration/index.md +++ b/docs/configuration/index.md @@ -173,6 +173,30 @@ When the cap is reached, the loop does **not** error out and discard work. On th One turn is a single LLM response plus its batch of tool executions. The default of 200 is high enough for long iterative jobs to finish while still bounding cost and wall-clock time for an unattended run that gets stuck. +### Retry Limits + +Caps on how many times the interactive conversation loop auto-retries a failing pattern without user intervention, so a stuck model cannot silently drain tokens. These are agent-loop limits — the per-provider `maxRetries` setting is unrelated and governs network request retries (see [Providers](providers/index.md)). + +```json +{ + "nanocoder": { + "retries": { + "maxRepeatedToolCalls": 3, + "maxEmptyTurns": 2, + "maxMalformedRetries": 2 + } + } +} +``` + +| Option | Type | Default | Description | +|--------|------|---------|-------------| +| `maxRepeatedToolCalls` | number | `3` | Consecutive identical tool calls allowed before the loop pauses (minimum 2). In an interactive session you are asked whether to continue — useful when the repetition is legitimate, such as polling a long-running job — or stop. Non-interactive runs stop with an error. | +| `maxEmptyTurns` | number | `2` | Consecutive empty assistant turns that are auto-nudged before the loop compacts the context, retries once, and gives up (minimum 0). | +| `maxMalformedRetries` | number | `2` | Malformed tool-call self-correction retries allowed on the XML fallback path before the loop gives up (minimum 0). | + +Choosing "Continue" at the repeated-tool-call prompt grants another window of the same size, so a genuinely stuck model is re-checked rather than left looping. + ### Paste Handling Configure how pasted text is handled in the input. By default, single-line pastes of 800 characters or fewer are inserted directly, while longer or multi-line pastes are collapsed into a `[Paste #N: X chars]` placeholder. diff --git a/source/config/index.spec.ts b/source/config/index.spec.ts index eece07722..0d095203f 100644 --- a/source/config/index.spec.ts +++ b/source/config/index.spec.ts @@ -578,6 +578,139 @@ test.serial('headless maxTurns ignores invalid env var', async t => { ); }); +// Tests for agent-loop retry limits (nanocoder.retries) +async function withRetriesConfig( + subdir: string, + configBody: unknown, + assertion: (retries: { + maxRepeatedToolCalls: number; + maxEmptyTurns: number; + maxMalformedRetries: number; + }) => void, +): Promise { + const originalCwd = process.cwd(); + const originalConfigDir = process.env.NANOCODER_CONFIG_DIR; + const testSubdir = join(headlessTestDir, subdir); + mkdirSync(testSubdir, {recursive: true}); + + try { + writeFileSync( + join(testSubdir, 'agents.config.json'), + JSON.stringify(configBody), + 'utf-8', + ); + process.chdir(testSubdir); + process.env.NANOCODER_CONFIG_DIR = join(testSubdir, 'nonexistent-global'); + + const {reloadAppConfig: reload, getAppConfig} = await import('./index.js'); + reload(); + const retries = getAppConfig().retries; + if (!retries) { + throw new Error('Resolved config should always carry retry limits'); + } + assertion(retries); + } finally { + process.chdir(originalCwd); + if (originalConfigDir !== undefined) { + process.env.NANOCODER_CONFIG_DIR = originalConfigDir; + } else { + delete process.env.NANOCODER_CONFIG_DIR; + } + } +} + +test.serial('retry limits default to the historical caps when not configured', async t => { + await withRetriesConfig('retries-default', {nanocoder: {}}, retries => { + t.is(retries.maxRepeatedToolCalls, 3); + t.is(retries.maxEmptyTurns, 2); + t.is(retries.maxMalformedRetries, 2); + }); +}); + +test.serial('retry limits load custom values from config', async t => { + await withRetriesConfig( + 'retries-config', + { + nanocoder: { + retries: { + maxRepeatedToolCalls: 10, + maxEmptyTurns: 5, + maxMalformedRetries: 4, + }, + }, + }, + retries => { + t.is(retries.maxRepeatedToolCalls, 10); + t.is(retries.maxEmptyTurns, 5); + t.is(retries.maxMalformedRetries, 4); + }, + ); +}); + +test.serial('retry limits apply defaults for fields not configured', async t => { + await withRetriesConfig( + 'retries-partial', + {nanocoder: {retries: {maxRepeatedToolCalls: 7}}}, + retries => { + t.is(retries.maxRepeatedToolCalls, 7); + t.is(retries.maxEmptyTurns, 2); + t.is(retries.maxMalformedRetries, 2); + }, + ); +}); + +test.serial('retry limits clamp maxRepeatedToolCalls to at least 2', async t => { + await withRetriesConfig( + 'retries-clamp-repeated', + {nanocoder: {retries: {maxRepeatedToolCalls: 1}}}, + retries => { + // A fresh tool call already counts as 1 repeat, so anything below 2 + // would pause on every single tool call. + t.is(retries.maxRepeatedToolCalls, 2); + }, + ); +}); + +test.serial('retry limits clamp negative values to their minimums', async t => { + await withRetriesConfig( + 'retries-clamp-negative', + { + nanocoder: { + retries: { + maxRepeatedToolCalls: -5, + maxEmptyTurns: -1, + maxMalformedRetries: -1, + }, + }, + }, + retries => { + t.is(retries.maxRepeatedToolCalls, 2); + t.is(retries.maxEmptyTurns, 0); + t.is(retries.maxMalformedRetries, 0); + }, + ); +}); + +test.serial('retry limits ignore non-numeric values', async t => { + await withRetriesConfig( + 'retries-invalid-types', + { + nanocoder: { + retries: { + maxRepeatedToolCalls: 'lots', + maxEmptyTurns: null, + maxMalformedRetries: {nope: true}, + }, + }, + }, + retries => { + t.is(retries.maxRepeatedToolCalls, 3); + t.is(retries.maxEmptyTurns, 2); + t.is(retries.maxMalformedRetries, 2); + }, + ); +}); + // Tests for modeProviders async function withModeProvidersConfig( testName: string, diff --git a/source/config/index.ts b/source/config/index.ts index f0f16bc79..03687fef9 100644 --- a/source/config/index.ts +++ b/source/config/index.ts @@ -13,6 +13,11 @@ import { loadPreferences, } from '@/config/preferences'; import {defaultTheme, getThemeColors} from '@/config/themes'; +import { + MAX_EMPTY_TURNS, + MAX_MALFORMED_RETRIES, + MAX_REPEATED_TOOL_CALLS, +} from '@/constants'; import type { AppConfig, AutoCompactConfig, @@ -24,6 +29,7 @@ import type { NotificationsConfig, PasteConfig, ProviderConfig, + RetryLimitsConfig, SystemPromptConfig, TuneConfig, } from '@/types/index'; @@ -323,6 +329,58 @@ function loadHeadlessConfig(): AppConfig['headless'] { ); } +// Load agent-loop retry limits from `nanocoder.retries` in agents.config.json. +// Defaults mirror the historical hardcoded caps in constants.ts, so behaviour +// is unchanged unless the user opts in. Distinct from the per-provider +// `maxRetries` setting, which caps network request retries. +function loadRetryLimitsConfig(): RetryLimitsConfig { + const defaults: RetryLimitsConfig = { + maxRepeatedToolCalls: MAX_REPEATED_TOOL_CALLS, + maxEmptyTurns: MAX_EMPTY_TURNS, + maxMalformedRetries: MAX_MALFORMED_RETRIES, + }; + + // A fresh tool-call signature already counts as 1 repeat, so a cap below 2 + // would pause on every single tool call. The nudge/self-correction caps may + // go to 0 (= give up on the first failing turn). + const normalizeLimit = ( + value: unknown, + min: number, + fallback: number, + ): number => { + if (typeof value === 'number' && Number.isFinite(value)) { + return Math.max(min, Math.round(value)); + } + return fallback; + }; + + return ( + loadHierarchicalConfig('agents.config.json', 'retries', config => { + const retries = config.nanocoder?.retries; + if (retries && typeof retries === 'object') { + return { + maxRepeatedToolCalls: normalizeLimit( + retries.maxRepeatedToolCalls, + 2, + defaults.maxRepeatedToolCalls, + ), + maxEmptyTurns: normalizeLimit( + retries.maxEmptyTurns, + 0, + defaults.maxEmptyTurns, + ), + maxMalformedRetries: normalizeLimit( + retries.maxMalformedRetries, + 0, + defaults.maxMalformedRetries, + ), + }; + } + return null; + }) ?? defaults + ); +} + // Load paste configuration and Returns default config if not specified function loadPasteConfig(): PasteConfig { const defaults: PasteConfig = { @@ -524,6 +582,9 @@ function loadAppConfig(): AppConfig { // Load headless conversation limits const headless = loadHeadlessConfig(); + // Load agent-loop retry limits + const retries = loadRetryLimitsConfig(); + // Load paste configuration const paste = loadPasteConfig(); @@ -553,6 +614,7 @@ function loadAppConfig(): AppConfig { autoCompact, sessions, headless, + retries, paste, nanocoderTools, alwaysAllow, diff --git a/source/hooks/chat-handler/conversation/conversation-loop.spec.ts b/source/hooks/chat-handler/conversation/conversation-loop.spec.ts index ab7f75c9c..9c0aa957e 100644 --- a/source/hooks/chat-handler/conversation/conversation-loop.spec.ts +++ b/source/hooks/chat-handler/conversation/conversation-loop.spec.ts @@ -19,6 +19,7 @@ import { resetSessionContextLimit, setSessionContextLimit, } from '@/models/models-dev-client.js'; +import {setGlobalQuestionHandler} from '@/utils/question-queue.js'; import {setGlobalToolConfirmHandler} from '@/utils/tool-confirm-queue.js'; // The ShutdownManager singleton is created as a side effect of transitive @@ -1991,4 +1992,303 @@ test.serial('processAssistantResponse - does not start request on orphaned tool if (getAppConfig().sessions && originalMaxMessages !== undefined) { getAppConfig().sessions!.maxMessages = originalMaxMessages; } +}); + +// ============================================================================ +// Configurable Retry Limits (issue #897) +// ============================================================================ + +// Client that returns the identical tool call on every turn — the signature +// the repeated-call loop detector keys on. +const createRepeatingToolClient = (onChat?: () => void) => ({ + chat: async (): Promise => { + onChat?.(); + return { + choices: [ + { + message: { + role: 'assistant', + content: '', + tool_calls: [ + { + id: 'call_loop', + function: {name: 'read_file', arguments: '{"path": "/tmp/x"}'}, + }, + ], + }, + }, + ], + toolsDisabled: false, + }; + }, +}); + +const repeatingToolManager = () => + createMockToolManager({tools: ['read_file'], needsApproval: false}); + +test.serial('repeated-tool-call limit pauses and stops when the user declines', async t => { + let chatCallCount = 0; + let questionCount = 0; + const queuedComponents: any[] = []; + + setGlobalQuestionHandler(async question => { + questionCount += 1; + t.regex(question.question, /repeated the same tool call/); + // First option is the safe default: stop. + return question.options[0]; + }); + + const params = createDefaultParams({ + client: createRepeatingToolClient(() => chatCallCount++), + toolManager: repeatingToolManager(), + addToChatQueue: (component: any) => queuedComponents.push(component), + }); + + await processAssistantResponse(params); + + // Default maxRepeatedToolCalls = 3: turns 1 and 2 execute, turn 3 trips + // the limit and prompts. Declining stops the loop. + t.is(chatCallCount, 3, 'Loop should stop at the default limit of 3'); + t.is(questionCount, 1, 'Should pause and ask exactly once'); + const stopMessage = queuedComponents.find( + (c: any) => + typeof c.props?.message === 'string' && + c.props.message.includes('repeated the same tool call'), + ); + t.truthy(stopMessage, 'Should queue the loop-detected ErrorMessage'); +}); + +test.serial('repeated-tool-call limit grants another window when the user continues', async t => { + let chatCallCount = 0; + let questionCount = 0; + const queuedComponents: any[] = []; + + setGlobalQuestionHandler(async question => { + questionCount += 1; + // Continue on the first prompt, stop on the second. + return questionCount === 1 ? question.options[1] : question.options[0]; + }); + + const params = createDefaultParams({ + client: createRepeatingToolClient(() => chatCallCount++), + toolManager: repeatingToolManager(), + addToChatQueue: (component: any) => queuedComponents.push(component), + }); + + await processAssistantResponse(params); + + // Continuing resets the streak: 3 turns to the first prompt, then 3 more + // identical turns to the second prompt, where the user stops. + t.is(chatCallCount, 6, 'Continue should grant a full new window of turns'); + t.is(questionCount, 2, 'Should re-prompt after the granted window is spent'); + const continueNotice = queuedComponents.find( + (c: any) => + typeof c.props?.message === 'string' && + c.props.message.includes('Continuing'), + ); + t.truthy(continueNotice, 'Should queue an InfoMessage when continuing'); +}); + +test.serial('repeated-tool-call limit does not fire one call under the limit', async t => { + let chatCallCount = 0; + let questionCount = 0; + + setGlobalQuestionHandler(async question => { + questionCount += 1; + return question.options[0]; + }); + + // Two identical tool turns (one under the default limit of 3), then a + // terminal text-only response. + const client = { + chat: async (): Promise => { + chatCallCount += 1; + if (chatCallCount <= 2) { + return { + choices: [ + { + message: { + role: 'assistant', + content: '', + tool_calls: [ + { + id: 'call_loop', + function: { + name: 'read_file', + arguments: '{"path": "/tmp/x"}', + }, + }, + ], + }, + }, + ], + toolsDisabled: false, + }; + } + return { + choices: [{message: {role: 'assistant', content: 'Done.'}}], + toolsDisabled: false, + }; + }, + }; + + const params = createDefaultParams({ + client, + toolManager: repeatingToolManager(), + }); + + await processAssistantResponse(params); + + t.is(chatCallCount, 3, 'All three turns should run to natural completion'); + t.is(questionCount, 0, 'Must not prompt below the limit'); +}); + +test.serial('repeated-tool-call limit honors a custom configured value', async t => { + const retries = getAppConfig().retries; + t.truthy(retries, 'Resolved config should always carry retry limits'); + const originalLimit = retries!.maxRepeatedToolCalls; + retries!.maxRepeatedToolCalls = 2; + + let chatCallCount = 0; + let questionCount = 0; + + setGlobalQuestionHandler(async question => { + questionCount += 1; + return question.options[0]; + }); + + try { + const params = createDefaultParams({ + client: createRepeatingToolClient(() => chatCallCount++), + toolManager: repeatingToolManager(), + }); + + await processAssistantResponse(params); + + t.is(chatCallCount, 2, 'Loop should stop at the configured limit of 2'); + t.is(questionCount, 1, 'Should pause and ask at the configured limit'); + } finally { + retries!.maxRepeatedToolCalls = originalLimit; + } +}); + +test.serial('repeated-tool-call limit hard-stops without prompting in non-interactive mode', async t => { + let chatCallCount = 0; + let questionCount = 0; + const queuedComponents: any[] = []; + + setGlobalQuestionHandler(async question => { + questionCount += 1; + return question.options[1]; + }); + + const params = createDefaultParams({ + client: createRepeatingToolClient(() => chatCallCount++), + toolManager: repeatingToolManager(), + nonInteractiveMode: true, + addToChatQueue: (component: any) => queuedComponents.push(component), + }); + + await processAssistantResponse(params); + + t.is(chatCallCount, 3, 'Loop should hard-stop at the limit'); + t.is(questionCount, 0, 'Must not prompt when there is nobody to ask'); + const stopMessage = queuedComponents.find( + (c: any) => + typeof c.props?.message === 'string' && + c.props.message.includes('repeated the same tool call'), + ); + t.truthy(stopMessage, 'Should still queue the loop-detected ErrorMessage'); +}); + +test.serial('malformed-retry limit honors a custom configured value', async t => { + const retries = getAppConfig().retries; + const originalLimit = retries!.maxMalformedRetries; + retries!.maxMalformedRetries = 0; + + let chatCallCount = 0; + const queuedComponents: any[] = []; + + const alwaysMalformedClient = { + chat: async (): Promise => { + chatCallCount += 1; + return { + choices: [ + { + message: { + role: 'assistant', + content: '[tool_use: read_file]', + tool_calls: undefined, + }, + }, + ], + toolsDisabled: true, + }; + }, + }; + + try { + const params = createDefaultParams({ + client: alwaysMalformedClient, + addToChatQueue: (component: any) => queuedComponents.push(component), + }); + + await processAssistantResponse(params); + + t.is(chatCallCount, 1, 'Limit 0 should give up after the first bad turn'); + const giveUpMessage = queuedComponents.find( + (c: any) => + typeof c.props?.message === 'string' && + c.props.message.includes('malformed tool calls 1 times'), + ); + t.truthy(giveUpMessage, 'Give-up message should reflect the custom limit'); + } finally { + retries!.maxMalformedRetries = originalLimit; + } +}); + +test.serial('empty-turn limit honors a custom configured value', async t => { + const retries = getAppConfig().retries; + const originalLimit = retries!.maxEmptyTurns; + retries!.maxEmptyTurns = 0; + + let chatCallCount = 0; + const queuedComponents: any[] = []; + + const alwaysEmptyClient = { + chat: async (): Promise => { + chatCallCount += 1; + return { + choices: [ + {message: {role: 'assistant', content: '', tool_calls: undefined}}, + ], + toolsDisabled: false, + }; + }, + }; + + try { + const params = createDefaultParams({ + client: alwaysEmptyClient, + addToChatQueue: (component: any) => queuedComponents.push(component), + }); + + await processAssistantResponse(params); + + // Limit 0 skips all nudges: at most the initial turn plus one + // compact-and-retry cycle (MAX_COMPACT_RETRIES = 1). The default limit + // would make at least 3 calls before giving up. + t.true( + chatCallCount <= 2, + `Limit 0 should give up within 2 calls; got ${chatCallCount}`, + ); + const giveUpMessage = queuedComponents.find( + (c: any) => + typeof c.props?.message === 'string' && + c.props.message.includes('produced no output'), + ); + t.truthy(giveUpMessage, 'Should queue the give-up ErrorMessage'); + } finally { + retries!.maxEmptyTurns = originalLimit; + } }); \ No newline at end of file diff --git a/source/hooks/chat-handler/conversation/conversation-loop.tsx b/source/hooks/chat-handler/conversation/conversation-loop.tsx index 3d477b4e8..944973453 100644 --- a/source/hooks/chat-handler/conversation/conversation-loop.tsx +++ b/source/hooks/chat-handler/conversation/conversation-loop.tsx @@ -39,6 +39,7 @@ import {capMessagesForModel} from '@/utils/message-capping'; import {compressMessages} from '@/utils/message-compression'; import {infoMsg} from '@/utils/message-factory'; import {getLastBuiltPrompt} from '@/utils/prompt-builder'; +import {signalQuestion} from '@/utils/question-queue'; import {calculateTokens} from '@/utils/token-calculator'; import {createCancellationResults} from '@/utils/tool-cancellation'; import {signalToolConfirm} from '@/utils/tool-confirm-queue'; @@ -114,7 +115,8 @@ interface ProcessAssistantResponseParams { // threads it forward; every other recursion site resets it to undefined. lastToolSignature?: string; // How many consecutive turns have emitted the same tool-call signature. - // Reaching MAX_REPEATED_TOOL_CALLS stops the loop with an actionable error. + // Reaching the configured repeated-call limit pauses to ask the user + // (interactive) or stops with an actionable error (non-interactive). repeatedToolCallCount?: number; } @@ -191,6 +193,15 @@ export const processAssistantResponse = async ( const startTime = conversationStartTime ?? Date.now(); + // Agent-loop retry limits: configurable via `nanocoder.retries` in + // agents.config.json, falling back to the historical hardcoded caps. + const retryLimits = getAppConfig().retries; + const maxRepeatedToolCalls = + retryLimits?.maxRepeatedToolCalls ?? MAX_REPEATED_TOOL_CALLS; + const maxEmptyTurns = retryLimits?.maxEmptyTurns ?? MAX_EMPTY_TURNS; + const maxMalformedRetries = + retryLimits?.maxMalformedRetries ?? MAX_MALFORMED_RETRIES; + // Helper to flush live task list to the static chat queue const flushLiveTaskList = async () => { if (!onSetLiveTaskList) return; @@ -377,12 +388,12 @@ export const processAssistantResponse = async ( // Cap malformed-retry recursion. Without this, a model stuck producing // bad XML loops forever, appending two messages per iteration, until // Node's heap exhausts. - if (malformedRetryCount >= MAX_MALFORMED_RETRIES) { + if (malformedRetryCount >= maxMalformedRetries) { await flushAll(); addToChatQueue( , ); @@ -691,31 +702,66 @@ export const processAssistantResponse = async ( // Count consecutive identical signatures and stop once the cap is hit so // we surface an actionable error instead of looping until abort. const currentToolSignature = computeToolCallSignature(validToolCalls); - const currentRepeatedCount = + let currentRepeatedCount = currentToolSignature && currentToolSignature === lastToolSignature ? repeatedToolCallCount + 1 : 1; - if (currentRepeatedCount >= MAX_REPEATED_TOOL_CALLS) { - await flushAll(); - // Keep the AI SDK's 1:1 tool-call/result mapping intact: the assistant - // message with these tool_calls is already in history, so pair each - // with a cancellation result before stopping. - const loopBuilder = new MessageBuilder(updatedMessages); - loopBuilder.addToolResults(createCancellationResults(validToolCalls)); - setMessages(loopBuilder.build()); + if (currentRepeatedCount >= maxRepeatedToolCalls) { + // Interactive sessions pause and ask instead of hard-stopping: the + // repetition may be legitimate (e.g. polling a long-running job), and + // the user is the only one who can tell. Headless / non-interactive + // runs have nobody to ask, so they keep the hard stop. + const liveMode = developmentModeRef?.current ?? developmentMode; + let continueAnyway = false; + if (!nonInteractiveMode && liveMode !== 'headless') { + await flushAll(); + setIsGenerating(false); + const stopOption = 'Stop and return to prompt'; + const continueOption = `Continue (allow ${maxRepeatedToolCalls} more)`; + const answer = await signalQuestion({ + question: `The model has repeated the same tool call ${currentRepeatedCount} times in a row without making progress. It may be stuck in a loop that drains tokens. Continue anyway?`, + options: [stopOption, continueOption], + allowFreeform: false, + questionType: 'confirmation', + }); + continueAnyway = answer === continueOption; + } + + if (!continueAnyway) { + await flushAll(); + // Keep the AI SDK's 1:1 tool-call/result mapping intact: the assistant + // message with these tool_calls is already in history, so pair each + // with a cancellation result before stopping. + const loopBuilder = new MessageBuilder(updatedMessages); + loopBuilder.addToolResults(createCancellationResults(validToolCalls)); + setMessages(loopBuilder.build()); + addToChatQueue( + , + ); + setIsGenerating(false); + if (onConversationComplete) { + onConversationComplete(); + } + return; + } + + // User granted another window: reset the streak so the next + // maxRepeatedToolCalls identical calls prompt again instead of + // stopping, then resume execution of this turn's tools. + currentRepeatedCount = 0; + setIsGenerating(true); addToChatQueue( - , ); - setIsGenerating(false); - if (onConversationComplete) { - onConversationComplete(); - } - return; } // The SDK never auto-executes tools (execute is stripped). We evaluate @@ -916,7 +962,7 @@ export const processAssistantResponse = async ( // Cap consecutive empty turns. Without this, a model that keeps // returning nothing (common with GPT-5 reasoning that exhausts the // token budget on thinking) would loop forever. - if (emptyTurnCount >= MAX_EMPTY_TURNS) { + if (emptyTurnCount >= maxEmptyTurns) { setLiveComponent?.(null); // If we still have compact-and-retry budget, mechanically compress // the context and nudge the model to continue instead of giving up @@ -978,7 +1024,7 @@ export const processAssistantResponse = async ( addToChatQueue( , ); @@ -1020,7 +1066,7 @@ export const processAssistantResponse = async ( // gets cleared at the top of processAssistantResponse so the // streaming UI for the retry is unobstructed. const attempt = emptyTurnCount + 1; - const total = MAX_EMPTY_TURNS + 1; + const total = maxEmptyTurns + 1; setLiveComponent?.( Date: Tue, 18 Aug 2026 22:11:07 +0530 Subject: [PATCH 02/17] refactor: thread repeated-call streak to recursion via explicit variable Review follow-up: keep currentRepeatedCount const (it names the detected streak used in user-facing messages) and carry the post-prompt streak in repeatedCountForNextTurn so the recursion site shows where a reset can come from. --- .../chat-handler/conversation/conversation-loop.tsx | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/source/hooks/chat-handler/conversation/conversation-loop.tsx b/source/hooks/chat-handler/conversation/conversation-loop.tsx index 944973453..fc2bfd76d 100644 --- a/source/hooks/chat-handler/conversation/conversation-loop.tsx +++ b/source/hooks/chat-handler/conversation/conversation-loop.tsx @@ -702,10 +702,13 @@ export const processAssistantResponse = async ( // Count consecutive identical signatures and stop once the cap is hit so // we surface an actionable error instead of looping until abort. const currentToolSignature = computeToolCallSignature(validToolCalls); - let currentRepeatedCount = + const currentRepeatedCount = currentToolSignature && currentToolSignature === lastToolSignature ? repeatedToolCallCount + 1 : 1; + // Streak carried into the recursive continuation below. Reset to 0 when + // the user grants another window at the limit prompt. + let repeatedCountForNextTurn = currentRepeatedCount; if (currentRepeatedCount >= maxRepeatedToolCalls) { // Interactive sessions pause and ask instead of hard-stopping: the @@ -753,7 +756,7 @@ export const processAssistantResponse = async ( // User granted another window: reset the streak so the next // maxRepeatedToolCalls identical calls prompt again instead of // stopping, then resume execution of this turn's tools. - currentRepeatedCount = 0; + repeatedCountForNextTurn = 0; setIsGenerating(true); addToChatQueue( Date: Tue, 18 Aug 2026 23:47:20 +0530 Subject: [PATCH 03/17] no-mistakes(review): fix retry-limit doc scope, test config isolation, duplicate flush --- docs/configuration/index.md | 2 +- .../conversation/conversation-loop.spec.ts | 162 ++++++++++-------- .../conversation/conversation-loop.tsx | 8 +- source/types/config.ts | 5 +- 4 files changed, 99 insertions(+), 78 deletions(-) diff --git a/docs/configuration/index.md b/docs/configuration/index.md index bce6acdd1..e7ea80813 100644 --- a/docs/configuration/index.md +++ b/docs/configuration/index.md @@ -193,7 +193,7 @@ Caps on how many times the interactive conversation loop auto-retries a failing |--------|------|---------|-------------| | `maxRepeatedToolCalls` | number | `3` | Consecutive identical tool calls allowed before the loop pauses (minimum 2). In an interactive session you are asked whether to continue — useful when the repetition is legitimate, such as polling a long-running job — or stop. Non-interactive runs stop with an error. | | `maxEmptyTurns` | number | `2` | Consecutive empty assistant turns that are auto-nudged before the loop compacts the context, retries once, and gives up (minimum 0). | -| `maxMalformedRetries` | number | `2` | Malformed tool-call self-correction retries allowed on the XML fallback path before the loop gives up (minimum 0). | +| `maxMalformedRetries` | number | `2` | Malformed self-correction retries allowed for text-parsed tool calls before the loop gives up (minimum 0). Applies to the XML fallback path and to native-tool models that emit tool-call text instead of native tool calls. | Choosing "Continue" at the repeated-tool-call prompt grants another window of the same size, so a genuinely stuck model is re-checked rather than left looping. diff --git a/source/hooks/chat-handler/conversation/conversation-loop.spec.ts b/source/hooks/chat-handler/conversation/conversation-loop.spec.ts index 9c0aa957e..e9f85e369 100644 --- a/source/hooks/chat-handler/conversation/conversation-loop.spec.ts +++ b/source/hooks/chat-handler/conversation/conversation-loop.spec.ts @@ -9,6 +9,7 @@ import type { ToolCall, ToolResult, } from '@/types/core'; +import type {RetryLimitsConfig} from '@/types/index'; import { resetAutoCompactSession, setAutoCompactEnabled, @@ -1431,6 +1432,15 @@ function setupAutoCompactTestEnv() { setAutoCompactThreshold(50); resetSessionContextLimit(); clearAppConfig(); + // Pin the agent-loop retry limits to their shipped defaults. Many tests here + // count exact chat calls, and a `nanocoder.retries` block in the developer's + // project or personal config would otherwise change those counts. + const retries = getAppConfig().retries; + if (retries) { + retries.maxRepeatedToolCalls = 3; + retries.maxEmptyTurns = 2; + retries.maxMalformedRetries = 2; + } resetFallbackNotice(); resetLastTurnHadReasoning(); } @@ -2026,6 +2036,27 @@ const createRepeatingToolClient = (onChat?: () => void) => ({ const repeatingToolManager = () => createMockToolManager({tools: ['read_file'], needsApproval: false}); +// Pin the resolved retry limits for the duration of a test and restore them +// afterwards. Without this the assertions read whatever `nanocoder.retries` the +// developer happens to have in their project or personal config, since the +// per-test clearAppConfig() re-resolves from disk. +const withRetryLimits = async ( + overrides: Partial, + run: () => Promise, +) => { + const retries = getAppConfig().retries; + if (!retries) { + throw new Error('Resolved config should always carry retry limits'); + } + const originals = {...retries}; + Object.assign(retries, overrides); + try { + await run(); + } finally { + Object.assign(retries, originals); + } +}; + test.serial('repeated-tool-call limit pauses and stops when the user declines', async t => { let chatCallCount = 0; let questionCount = 0; @@ -2038,13 +2069,15 @@ test.serial('repeated-tool-call limit pauses and stops when the user declines', return question.options[0]; }); - const params = createDefaultParams({ - client: createRepeatingToolClient(() => chatCallCount++), - toolManager: repeatingToolManager(), - addToChatQueue: (component: any) => queuedComponents.push(component), - }); + await withRetryLimits({maxRepeatedToolCalls: 3}, async () => { + const params = createDefaultParams({ + client: createRepeatingToolClient(() => chatCallCount++), + toolManager: repeatingToolManager(), + addToChatQueue: (component: any) => queuedComponents.push(component), + }); - await processAssistantResponse(params); + await processAssistantResponse(params); + }); // Default maxRepeatedToolCalls = 3: turns 1 and 2 execute, turn 3 trips // the limit and prompts. Declining stops the loop. @@ -2069,13 +2102,15 @@ test.serial('repeated-tool-call limit grants another window when the user contin return questionCount === 1 ? question.options[1] : question.options[0]; }); - const params = createDefaultParams({ - client: createRepeatingToolClient(() => chatCallCount++), - toolManager: repeatingToolManager(), - addToChatQueue: (component: any) => queuedComponents.push(component), - }); + await withRetryLimits({maxRepeatedToolCalls: 3}, async () => { + const params = createDefaultParams({ + client: createRepeatingToolClient(() => chatCallCount++), + toolManager: repeatingToolManager(), + addToChatQueue: (component: any) => queuedComponents.push(component), + }); - await processAssistantResponse(params); + await processAssistantResponse(params); + }); // Continuing resets the streak: 3 turns to the first prompt, then 3 more // identical turns to the second prompt, where the user stops. @@ -2132,23 +2167,20 @@ test.serial('repeated-tool-call limit does not fire one call under the limit', a }, }; - const params = createDefaultParams({ - client, - toolManager: repeatingToolManager(), - }); + await withRetryLimits({maxRepeatedToolCalls: 3}, async () => { + const params = createDefaultParams({ + client, + toolManager: repeatingToolManager(), + }); - await processAssistantResponse(params); + await processAssistantResponse(params); + }); t.is(chatCallCount, 3, 'All three turns should run to natural completion'); t.is(questionCount, 0, 'Must not prompt below the limit'); }); test.serial('repeated-tool-call limit honors a custom configured value', async t => { - const retries = getAppConfig().retries; - t.truthy(retries, 'Resolved config should always carry retry limits'); - const originalLimit = retries!.maxRepeatedToolCalls; - retries!.maxRepeatedToolCalls = 2; - let chatCallCount = 0; let questionCount = 0; @@ -2157,19 +2189,17 @@ test.serial('repeated-tool-call limit honors a custom configured value', async t return question.options[0]; }); - try { + await withRetryLimits({maxRepeatedToolCalls: 2}, async () => { const params = createDefaultParams({ client: createRepeatingToolClient(() => chatCallCount++), toolManager: repeatingToolManager(), }); await processAssistantResponse(params); + }); - t.is(chatCallCount, 2, 'Loop should stop at the configured limit of 2'); - t.is(questionCount, 1, 'Should pause and ask at the configured limit'); - } finally { - retries!.maxRepeatedToolCalls = originalLimit; - } + t.is(chatCallCount, 2, 'Loop should stop at the configured limit of 2'); + t.is(questionCount, 1, 'Should pause and ask at the configured limit'); }); test.serial('repeated-tool-call limit hard-stops without prompting in non-interactive mode', async t => { @@ -2182,14 +2212,16 @@ test.serial('repeated-tool-call limit hard-stops without prompting in non-intera return question.options[1]; }); - const params = createDefaultParams({ - client: createRepeatingToolClient(() => chatCallCount++), - toolManager: repeatingToolManager(), - nonInteractiveMode: true, - addToChatQueue: (component: any) => queuedComponents.push(component), - }); + await withRetryLimits({maxRepeatedToolCalls: 3}, async () => { + const params = createDefaultParams({ + client: createRepeatingToolClient(() => chatCallCount++), + toolManager: repeatingToolManager(), + nonInteractiveMode: true, + addToChatQueue: (component: any) => queuedComponents.push(component), + }); - await processAssistantResponse(params); + await processAssistantResponse(params); + }); t.is(chatCallCount, 3, 'Loop should hard-stop at the limit'); t.is(questionCount, 0, 'Must not prompt when there is nobody to ask'); @@ -2202,10 +2234,6 @@ test.serial('repeated-tool-call limit hard-stops without prompting in non-intera }); test.serial('malformed-retry limit honors a custom configured value', async t => { - const retries = getAppConfig().retries; - const originalLimit = retries!.maxMalformedRetries; - retries!.maxMalformedRetries = 0; - let chatCallCount = 0; const queuedComponents: any[] = []; @@ -2227,31 +2255,25 @@ test.serial('malformed-retry limit honors a custom configured value', async t => }, }; - try { + await withRetryLimits({maxMalformedRetries: 0}, async () => { const params = createDefaultParams({ client: alwaysMalformedClient, addToChatQueue: (component: any) => queuedComponents.push(component), }); await processAssistantResponse(params); + }); - t.is(chatCallCount, 1, 'Limit 0 should give up after the first bad turn'); - const giveUpMessage = queuedComponents.find( - (c: any) => - typeof c.props?.message === 'string' && - c.props.message.includes('malformed tool calls 1 times'), - ); - t.truthy(giveUpMessage, 'Give-up message should reflect the custom limit'); - } finally { - retries!.maxMalformedRetries = originalLimit; - } + t.is(chatCallCount, 1, 'Limit 0 should give up after the first bad turn'); + const giveUpMessage = queuedComponents.find( + (c: any) => + typeof c.props?.message === 'string' && + c.props.message.includes('malformed tool calls 1 times'), + ); + t.truthy(giveUpMessage, 'Give-up message should reflect the custom limit'); }); test.serial('empty-turn limit honors a custom configured value', async t => { - const retries = getAppConfig().retries; - const originalLimit = retries!.maxEmptyTurns; - retries!.maxEmptyTurns = 0; - let chatCallCount = 0; const queuedComponents: any[] = []; @@ -2267,28 +2289,26 @@ test.serial('empty-turn limit honors a custom configured value', async t => { }, }; - try { + await withRetryLimits({maxEmptyTurns: 0}, async () => { const params = createDefaultParams({ client: alwaysEmptyClient, addToChatQueue: (component: any) => queuedComponents.push(component), }); await processAssistantResponse(params); + }); - // Limit 0 skips all nudges: at most the initial turn plus one - // compact-and-retry cycle (MAX_COMPACT_RETRIES = 1). The default limit - // would make at least 3 calls before giving up. - t.true( - chatCallCount <= 2, - `Limit 0 should give up within 2 calls; got ${chatCallCount}`, - ); - const giveUpMessage = queuedComponents.find( - (c: any) => - typeof c.props?.message === 'string' && - c.props.message.includes('produced no output'), - ); - t.truthy(giveUpMessage, 'Should queue the give-up ErrorMessage'); - } finally { - retries!.maxEmptyTurns = originalLimit; - } + // Limit 0 skips all nudges: at most the initial turn plus one + // compact-and-retry cycle (MAX_COMPACT_RETRIES = 1). The default limit + // would make at least 3 calls before giving up. + t.true( + chatCallCount <= 2, + `Limit 0 should give up within 2 calls; got ${chatCallCount}`, + ); + const giveUpMessage = queuedComponents.find( + (c: any) => + typeof c.props?.message === 'string' && + c.props.message.includes('produced no output'), + ); + t.truthy(giveUpMessage, 'Should queue the give-up ErrorMessage'); }); \ No newline at end of file diff --git a/source/hooks/chat-handler/conversation/conversation-loop.tsx b/source/hooks/chat-handler/conversation/conversation-loop.tsx index fc2bfd76d..01c7965e1 100644 --- a/source/hooks/chat-handler/conversation/conversation-loop.tsx +++ b/source/hooks/chat-handler/conversation/conversation-loop.tsx @@ -382,8 +382,9 @@ export const processAssistantResponse = async ( ); } - // Check for malformed tool calls and send error back to model for self-correction - // (only happens on the XML fallback path) + // Check for malformed tool calls and send error back to model for + // self-correction. Reachable from any text-parsed path: the XML fallback, + // and native responses that emit tool-call text instead of native calls. if (!parseResult.success) { // Cap malformed-retry recursion. Without this, a model stuck producing // bad XML loops forever, appending two messages per iteration, until @@ -717,8 +718,8 @@ export const processAssistantResponse = async ( // runs have nobody to ask, so they keep the hard stop. const liveMode = developmentModeRef?.current ?? developmentMode; let continueAnyway = false; + await flushAll(); if (!nonInteractiveMode && liveMode !== 'headless') { - await flushAll(); setIsGenerating(false); const stopOption = 'Stop and return to prompt'; const continueOption = `Continue (allow ${maxRepeatedToolCalls} more)`; @@ -732,7 +733,6 @@ export const processAssistantResponse = async ( } if (!continueAnyway) { - await flushAll(); // Keep the AI SDK's 1:1 tool-call/result mapping intact: the assistant // message with these tool_calls is already in history, so pair each // with a cancellation result before stopping. diff --git a/source/types/config.ts b/source/types/config.ts index aaaebdc92..599c71f99 100644 --- a/source/types/config.ts +++ b/source/types/config.ts @@ -108,8 +108,9 @@ export interface RetryLimitsConfig { // Consecutive empty assistant turns auto-nudged before compact-and-retry // kicks in and the loop gives up. maxEmptyTurns: number; - // Malformed tool-call self-correction retries allowed on the XML fallback - // path before the loop gives up. + // Malformed self-correction retries allowed for text-parsed tool calls + // before the loop gives up. Covers the XML fallback path and native + // responses that emit tool-call text instead of native tool calls. maxMalformedRetries: number; } From f95c2d262d61dba64d5e4030a1a79ddce8492908 Mon Sep 17 00:00:00 2001 From: shoryabansalgithub Date: Tue, 18 Aug 2026 23:58:33 +0530 Subject: [PATCH 04/17] no-mistakes(review): report cumulative repeat streak, refresh retry constant docs --- source/constants.ts | 29 ++++++++++-------- .../conversation/conversation-loop.spec.ts | 20 +++++++++++-- .../conversation/conversation-loop.tsx | 30 +++++++++++++++---- 3 files changed, 59 insertions(+), 20 deletions(-) diff --git a/source/constants.ts b/source/constants.ts index aebbec35b..eb1a17631 100644 --- a/source/constants.ts +++ b/source/constants.ts @@ -124,23 +124,28 @@ export const MAX_URL_CONTENT_BYTES = 100_000; // ~100 KB // === AI SDK === export const MAX_TOOL_STEPS = 10; -// Cap how many consecutive empty assistant turns we'll auto-nudge through -// before surfacing an error. Some models (notably GPT-5 reasoning models) -// can produce reasoning-only turns; one or two retries usually clears it, -// but unbounded recursion would loop forever. +// Default for `nanocoder.retries.maxEmptyTurns` (see source/config/index.ts): +// how many consecutive empty assistant turns we'll auto-nudge through before +// surfacing an error. Some models (notably GPT-5 reasoning models) can produce +// reasoning-only turns; one or two retries usually clears it, but unbounded +// recursion would loop forever. export const MAX_EMPTY_TURNS = 2; // After hitting the empty-turn cap, mechanically compact the context and // retry. This many compact-and-retry cycles are allowed before giving up. export const MAX_COMPACT_RETRIES = 1; -// Cap how many consecutive malformed-XML self-correction recursions we'll -// attempt before surfacing an error. Without this, a model stuck producing -// bad XML loops async and appends two messages per iteration until Node's -// heap exhausts (~1.4GB). +// Default for `nanocoder.retries.maxMalformedRetries` (see +// source/config/index.ts): how many consecutive malformed-tool-call +// self-correction recursions we'll attempt before surfacing an error. Without +// this, a model stuck producing bad XML loops async and appends two messages +// per iteration until Node's heap exhausts (~1.4GB). export const MAX_MALFORMED_RETRIES = 2; -// Cap how many times the model may emit the exact same tool call(s) on -// consecutive turns. Small models can get stuck re-issuing an identical failing -// call forever; once the same signature repeats this many times in a row we -// stop and surface an actionable error instead of looping. +// Default for `nanocoder.retries.maxRepeatedToolCalls` (see +// source/config/index.ts): how many times the model may emit the exact same +// tool call(s) on consecutive turns. Small models can get stuck re-issuing an +// identical failing call forever. Once the same signature repeats this many +// times in a row, interactive sessions pause and ask the user whether to stop +// or allow another window; non-interactive and headless runs, which have nobody +// to ask, stop with an actionable error. export const MAX_REPEATED_TOOL_CALLS = 3; // === MCP === diff --git a/source/hooks/chat-handler/conversation/conversation-loop.spec.ts b/source/hooks/chat-handler/conversation/conversation-loop.spec.ts index e9f85e369..dd6f83be8 100644 --- a/source/hooks/chat-handler/conversation/conversation-loop.spec.ts +++ b/source/hooks/chat-handler/conversation/conversation-loop.spec.ts @@ -2064,7 +2064,7 @@ test.serial('repeated-tool-call limit pauses and stops when the user declines', setGlobalQuestionHandler(async question => { questionCount += 1; - t.regex(question.question, /repeated the same tool call/); + t.regex(question.question, /repeated the same tool call 3 times in a row/); // First option is the safe default: stop. return question.options[0]; }); @@ -2086,7 +2086,7 @@ test.serial('repeated-tool-call limit pauses and stops when the user declines', const stopMessage = queuedComponents.find( (c: any) => typeof c.props?.message === 'string' && - c.props.message.includes('repeated the same tool call'), + c.props.message.includes('repeated the same tool call 3 times in a row'), ); t.truthy(stopMessage, 'Should queue the loop-detected ErrorMessage'); }); @@ -2094,10 +2094,12 @@ test.serial('repeated-tool-call limit pauses and stops when the user declines', test.serial('repeated-tool-call limit grants another window when the user continues', async t => { let chatCallCount = 0; let questionCount = 0; + const questionTexts: string[] = []; const queuedComponents: any[] = []; setGlobalQuestionHandler(async question => { questionCount += 1; + questionTexts.push(question.question); // Continue on the first prompt, stop on the second. return questionCount === 1 ? question.options[1] : question.options[0]; }); @@ -2116,12 +2118,26 @@ test.serial('repeated-tool-call limit grants another window when the user contin // identical turns to the second prompt, where the user stops. t.is(chatCallCount, 6, 'Continue should grant a full new window of turns'); t.is(questionCount, 2, 'Should re-prompt after the granted window is spent'); + // The per-window streak resets on continue, but the reported count is the + // true cumulative streak, so the second prompt says 6 rather than 3 again. + t.regex(questionTexts[0], /repeated the same tool call 3 times in a row/); + t.regex(questionTexts[1], /repeated the same tool call 6 times in a row/); const continueNotice = queuedComponents.find( (c: any) => typeof c.props?.message === 'string' && c.props.message.includes('Continuing'), ); t.truthy(continueNotice, 'Should queue an InfoMessage when continuing'); + const stopMessage = queuedComponents.find( + (c: any) => + typeof c.props?.message === 'string' && + c.props.message.includes('repeated the same tool call'), + ); + t.regex( + stopMessage?.props?.message ?? '', + /repeated the same tool call 6 times in a row/, + 'Stop message should report the cumulative streak, not the window count', + ); }); test.serial('repeated-tool-call limit does not fire one call under the limit', async t => { diff --git a/source/hooks/chat-handler/conversation/conversation-loop.tsx b/source/hooks/chat-handler/conversation/conversation-loop.tsx index 01c7965e1..d5520e6fd 100644 --- a/source/hooks/chat-handler/conversation/conversation-loop.tsx +++ b/source/hooks/chat-handler/conversation/conversation-loop.tsx @@ -114,10 +114,15 @@ interface ProcessAssistantResponseParams { // Used to detect an identical-call loop. The tool-execution continuation // threads it forward; every other recursion site resets it to undefined. lastToolSignature?: string; - // How many consecutive turns have emitted the same tool-call signature. - // Reaching the configured repeated-call limit pauses to ask the user - // (interactive) or stops with an actionable error (non-interactive). + // How many consecutive turns have emitted the same tool-call signature + // within the current window. Reaching the configured repeated-call limit + // pauses to ask the user (interactive) or stops with an actionable error + // (non-interactive). Resets to 0 when the user grants another window. repeatedToolCallCount?: number; + // How many consecutive turns have emitted the same tool-call signature in + // total, across every window the user granted. Never reset by a granted + // continuation, so user-facing counts report the true repetition streak. + repeatedToolCallTotal?: number; } // Module-level flag: show XML fallback notice only once per process lifetime. @@ -186,6 +191,7 @@ export const processAssistantResponse = async ( compactRetryCount = 0, lastToolSignature, repeatedToolCallCount = 0, + repeatedToolCallTotal = 0, privacySessionMapRef, privacyEnabled = false, onPrivacyEvent, @@ -446,6 +452,7 @@ export const processAssistantResponse = async ( malformedRetryCount: malformedRetryCount + 1, lastToolSignature: undefined, repeatedToolCallCount: 0, + repeatedToolCallTotal: 0, }); return; } @@ -691,6 +698,7 @@ export const processAssistantResponse = async ( malformedRetryCount: 0, lastToolSignature: undefined, repeatedToolCallCount: 0, + repeatedToolCallTotal: 0, }); return; } @@ -708,8 +716,15 @@ export const processAssistantResponse = async ( ? repeatedToolCallCount + 1 : 1; // Streak carried into the recursive continuation below. Reset to 0 when - // the user grants another window at the limit prompt. + // the user grants another window at the limit prompt, so the re-prompt + // cadence stays one full window rather than firing every turn. let repeatedCountForNextTurn = currentRepeatedCount; + // True consecutive-repeat streak, never reset by a granted window, so + // user-facing counts don't restart at the limit after each continue. + const currentRepeatedTotal = + currentToolSignature && currentToolSignature === lastToolSignature + ? repeatedToolCallTotal + 1 + : 1; if (currentRepeatedCount >= maxRepeatedToolCalls) { // Interactive sessions pause and ask instead of hard-stopping: the @@ -724,7 +739,7 @@ export const processAssistantResponse = async ( const stopOption = 'Stop and return to prompt'; const continueOption = `Continue (allow ${maxRepeatedToolCalls} more)`; const answer = await signalQuestion({ - question: `The model has repeated the same tool call ${currentRepeatedCount} times in a row without making progress. It may be stuck in a loop that drains tokens. Continue anyway?`, + question: `The model has repeated the same tool call ${currentRepeatedTotal} times in a row without making progress. It may be stuck in a loop that drains tokens. Continue anyway?`, options: [stopOption, continueOption], allowFreeform: false, questionType: 'confirmation', @@ -742,7 +757,7 @@ export const processAssistantResponse = async ( addToChatQueue( , ); @@ -953,6 +968,7 @@ export const processAssistantResponse = async ( malformedRetryCount: 0, lastToolSignature: currentToolSignature, repeatedToolCallCount: repeatedCountForNextTurn, + repeatedToolCallTotal: currentRepeatedTotal, }); return; } @@ -1015,6 +1031,7 @@ export const processAssistantResponse = async ( compactRetryCount: compactRetryCount + 1, lastToolSignature: undefined, repeatedToolCallCount: 0, + repeatedToolCallTotal: 0, }); return; } catch (_err) { @@ -1100,6 +1117,7 @@ export const processAssistantResponse = async ( malformedRetryCount: 0, lastToolSignature: undefined, repeatedToolCallCount: 0, + repeatedToolCallTotal: 0, }); return; } From 921a882ae45ea482759c871168fb798f94476902 Mon Sep 17 00:00:00 2001 From: shoryabansalgithub Date: Wed, 19 Aug 2026 00:45:11 +0530 Subject: [PATCH 05/17] no-mistakes(document): document provider maxRetries and yolo repeated-call pause --- docs/configuration/providers/index.md | 1 + docs/features/development-modes.md | 5 +++-- .../hooks/chat-handler/conversation/conversation-loop.tsx | 7 ++++--- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/docs/configuration/providers/index.md b/docs/configuration/providers/index.md index 6bdaca75c..ffcf44b45 100644 --- a/docs/configuration/providers/index.md +++ b/docs/configuration/providers/index.md @@ -82,6 +82,7 @@ Use dedicated AI SDK packages for native API support, enabled via the `sdkProvid | `disableToolModels` | List of model names to disable tool calling for (optional) | | `requestTimeout` | Overall request timeout in milliseconds (default: 120,000). Set to `-1` to disable (optional) | | `socketTimeout` | Socket-level timeout in milliseconds, uses `requestTimeout` if not set. Set to `-1` to disable (optional) | +| `maxRetries` | How many times a failed network request is retried (default: 2). Unrelated to the agent-loop [Retry Limits](../index.md#retry-limits), which cap how often the model may repeat itself (optional) | | `connectionPool` | Connection pool settings (optional, see [Timeouts & Connection Pooling](#timeouts--connection-pooling)) | ### Context Window Overrides diff --git a/docs/features/development-modes.md b/docs/features/development-modes.md index 2d3bad0c7..35287522f 100644 --- a/docs/features/development-modes.md +++ b/docs/features/development-modes.md @@ -45,11 +45,12 @@ Automatically accepts and executes most tool calls without confirmation. Some hi Automatically accepts and executes **every** tool call without exception — including bash commands and destructive git operations. -- No confirmation prompts at all — everything runs immediately +- No tool confirmation prompts at all — everything runs immediately - Bash commands, hard resets, force deletes, stash drops — all auto-accepted - The status bar turns red to make it clear you're in yolo mode +- One safeguard remains: if the model repeats the identical tool call too many times in a row, Nanocoder pauses and asks whether to continue, so a stuck loop cannot drain tokens unattended. See [Retry Limits](../configuration/index.md#retry-limits) -**When to use:** When you fully trust the AI and want zero interruptions. Use with caution — there are no safety nets other than basic tool validators. +**When to use:** When you fully trust the AI and want zero interruptions. Use with caution — there are no safety nets other than basic tool validators and the repeated-call pause above. ## Plan Mode diff --git a/source/hooks/chat-handler/conversation/conversation-loop.tsx b/source/hooks/chat-handler/conversation/conversation-loop.tsx index d5520e6fd..ced99eb2f 100644 --- a/source/hooks/chat-handler/conversation/conversation-loop.tsx +++ b/source/hooks/chat-handler/conversation/conversation-loop.tsx @@ -103,9 +103,10 @@ interface ProcessAssistantResponseParams { // nudged in this loop. The empty-response branch increments and // recurses; every other recursion site resets to 0. emptyTurnCount?: number; - // Number of consecutive malformed-XML self-correction recursions that - // have already happened. The malformed branch increments and recurses; - // every other recursion site resets to 0. + // Number of consecutive malformed tool-call self-correction recursions + // that have already happened, on any text-parsed path (XML fallback or a + // native response that emitted tool-call text). The malformed branch + // increments and recurses; every other recursion site resets to 0. malformedRetryCount?: number; // Number of compact-and-retry cycles attempted after exhausting empty-turn // nudges. Once MAX_COMPACT_RETRIES is reached we surface the error. From 9de2c2e22f3c984d97769e69db0fefecd31466c2 Mon Sep 17 00:00:00 2001 From: shoryabansalgithub Date: Wed, 19 Aug 2026 00:57:24 +0530 Subject: [PATCH 06/17] feat(plain): apply nanocoder.retries caps in the --plain runtime The plain runtime (auto-selected for 'nanocoder run' in CI and non-TTY environments) previously had no repeated-tool-call, empty-turn, or malformed-retry protection at all - it was bounded only by headless.maxTurns (default 200), which is exactly the unattended token-drain scenario issue #897 targets. The loop now honors the same nanocoder.retries limits as the interactive runtime: consecutive identical tool calls, consecutive empty turns (nudged up to the cap), and malformed tool-call self-correction retries each hard-stop with a clear error naming the setting once their cap is hit - there is no user to ask in a plain run. Empty assistant messages are no longer appended to history (providers reject them), matching the interactive loop. Docs and the changeset now describe both runtimes. --- .changeset/configurable-retry-limits.md | 2 +- docs/configuration/index.md | 6 +- source/plain/conversation.spec.ts | 352 +++++++++++++++++++++++- source/plain/conversation.ts | 167 +++++++++-- 4 files changed, 491 insertions(+), 36 deletions(-) diff --git a/.changeset/configurable-retry-limits.md b/.changeset/configurable-retry-limits.md index b1d3cd3ee..1c0bcd93c 100644 --- a/.changeset/configurable-retry-limits.md +++ b/.changeset/configurable-retry-limits.md @@ -2,4 +2,4 @@ "@nanocollective/nanocoder": minor --- -Add configurable agent-loop retry limits to prevent token drain (#897). A new `nanocoder.retries` section in `agents.config.json` exposes the previously hardcoded caps: `maxRepeatedToolCalls` (default 3), `maxEmptyTurns` (default 2), and `maxMalformedRetries` (default 2). When the repeated-tool-call limit is hit in an interactive session, Nanocoder now pauses and asks whether to continue (granting another window of attempts) or stop, instead of always hard-stopping; non-interactive runs keep the hard stop. +Add configurable agent-loop retry limits to prevent token drain (#897). A new `nanocoder.retries` section in `agents.config.json` exposes the previously hardcoded caps: `maxRepeatedToolCalls` (default 3), `maxEmptyTurns` (default 2), and `maxMalformedRetries` (default 2). When the repeated-tool-call limit is hit in an interactive session, Nanocoder now pauses and asks whether to continue (granting another window of attempts) or stop, instead of always hard-stopping; non-interactive runs keep the hard stop. The same limits now also protect the `--plain` runtime used by `nanocoder run` in CI and non-TTY environments, which previously had no repeated-call, empty-turn, or malformed-retry caps at all: each cap hard-stops with a clear error there. diff --git a/docs/configuration/index.md b/docs/configuration/index.md index e7ea80813..fc53b8ce6 100644 --- a/docs/configuration/index.md +++ b/docs/configuration/index.md @@ -175,7 +175,7 @@ One turn is a single LLM response plus its batch of tool executions. The default ### Retry Limits -Caps on how many times the interactive conversation loop auto-retries a failing pattern without user intervention, so a stuck model cannot silently drain tokens. These are agent-loop limits — the per-provider `maxRetries` setting is unrelated and governs network request retries (see [Providers](providers/index.md)). +Caps on how many times the conversation loop auto-retries a failing pattern without user intervention, so a stuck model cannot silently drain tokens. They apply in both runtimes: the interactive TUI loop and the `--plain` runtime used by `nanocoder run "..."` in CI and non-TTY environments (where they act within the [Headless](#headless) `maxTurns` ceiling). These are agent-loop limits — the per-provider `maxRetries` setting is unrelated and governs network request retries (see [Providers](providers/index.md)). ```json { @@ -191,8 +191,8 @@ Caps on how many times the interactive conversation loop auto-retries a failing | Option | Type | Default | Description | |--------|------|---------|-------------| -| `maxRepeatedToolCalls` | number | `3` | Consecutive identical tool calls allowed before the loop pauses (minimum 2). In an interactive session you are asked whether to continue — useful when the repetition is legitimate, such as polling a long-running job — or stop. Non-interactive runs stop with an error. | -| `maxEmptyTurns` | number | `2` | Consecutive empty assistant turns that are auto-nudged before the loop compacts the context, retries once, and gives up (minimum 0). | +| `maxRepeatedToolCalls` | number | `3` | Consecutive identical tool calls allowed before the loop pauses (minimum 2). In an interactive session you are asked whether to continue — useful when the repetition is legitimate, such as polling a long-running job — or stop. `--plain` and other non-interactive runs stop with a clear error. | +| `maxEmptyTurns` | number | `2` | Consecutive empty assistant turns that are auto-nudged before giving up (minimum 0). The interactive loop additionally compacts the context and retries once before stopping; the `--plain` runtime stops directly after the nudges. | | `maxMalformedRetries` | number | `2` | Malformed self-correction retries allowed for text-parsed tool calls before the loop gives up (minimum 0). Applies to the XML fallback path and to native-tool models that emit tool-call text instead of native tool calls. | Choosing "Continue" at the repeated-tool-call prompt grants another window of the same size, so a genuinely stuck model is re-checked rather than left looping. diff --git a/source/plain/conversation.spec.ts b/source/plain/conversation.spec.ts index 5ded2a4ea..c97d379ac 100644 --- a/source/plain/conversation.spec.ts +++ b/source/plain/conversation.spec.ts @@ -1,5 +1,5 @@ import test from "ava"; -import { reloadAppConfig } from "@/config/index"; +import { getAppConfig, reloadAppConfig } from "@/config/index"; import { setToolManagerGetter, setToolRegistryGetter } from "@/message-handler"; import type { ToolManager } from "@/tools/tool-manager"; import type { @@ -108,12 +108,13 @@ test("returns success when model emits content and no tool calls", async (t) => t.is(outcome.kind, "success"); }); -test("returns error when model emits empty response with no tool calls", async (t) => { +test("nudges through empty responses up to the cap, then returns error", async (t) => { + // Default maxEmptyTurns = 2: initial empty + 2 nudged retries = 3 calls. const client = makeFakeClient({ responses: [ - { - choices: [{ message: { role: "assistant", content: "" } }], - }, + { choices: [{ message: { role: "assistant", content: "" } }] }, + { choices: [{ message: { role: "assistant", content: "" } }] }, + { choices: [{ message: { role: "assistant", content: "" } }] }, ], }); const toolManager = makeFakeToolManager(); @@ -130,10 +131,34 @@ test("returns error when model emits empty response with no tool calls", async ( t.is(outcome.kind, "error"); if (outcome.kind === "error") { - t.regex(outcome.message, /empty response/i); + t.regex(outcome.message, /produced no output after 3 attempts/i); } }); +test("recovers when a nudge after an empty response gets the model talking", async (t) => { + const client = makeFakeClient({ + responses: [ + { choices: [{ message: { role: "assistant", content: "" } }] }, + { choices: [{ message: { role: "assistant", content: "recovered" } }] }, + ], + }); + const toolManager = makeFakeToolManager(); + + const outcome = await runPlainConversation({ + client, + toolManager, + systemMessage: SYSTEM, + initialMessages: [USER], + developmentMode: "auto-accept", + nonInteractiveAlwaysAllow: [], + abortSignal: new AbortController().signal, + }); + + // finalText stays empty here because the fake client never streams tokens; + // the success outcome (driven by the message content) is the behavior under test. + t.is(outcome.kind, "success"); +}); + test("executes a tool call that does not need approval and recurses to success", async (t) => { const toolCall: ToolCall = { id: "call-1", @@ -990,3 +1015,318 @@ test.serial( }); }, ); + +// --- Agent-loop retry limits (nanocoder.retries) — issue #897 --- + +function repeatingToolResponse(): Partial { + return { + choices: [ + { + message: { + role: "assistant", + content: "", + tool_calls: [ + { + id: "call-loop", + function: { name: "safe_tool", arguments: { path: "/tmp/x" } }, + }, + ], + }, + }, + ], + }; +} + +// Scope a retry-limit override to one test; afterEach.always reloads config, +// but restore explicitly so a mid-test failure cannot leak into another test. +async function withRetryLimit( + key: K, + value: number, + body: () => Promise, +): Promise { + const retries = getAppConfig().retries; + if (!retries) throw new Error("resolved config must carry retry limits"); + const original = retries[key]; + retries[key] = value; + try { + await body(); + } finally { + retries[key] = original; + } +} + +test.serial( + "repeated identical tool calls hard-stop at the default limit", + async (t) => { + // Default maxRepeatedToolCalls = 3: the tool executes on turns 1 and 2; + // turn 3's identical call trips the cap before executing. + const calls: RecordedCall[] = []; + const client = makeRecordingClient( + [repeatingToolResponse(), repeatingToolResponse(), repeatingToolResponse()], + calls, + ); + const toolManager = makeFakeToolManager({ + knownTools: new Set(["safe_tool"]), + needsApprovalByName: { safe_tool: false }, + }); + let handlerCalls = 0; + setToolRegistryGetter(() => ({ + safe_tool: (async () => { + handlerCalls++; + return "tool-output"; + }) as ToolHandler, + })); + + const outcome = await runPlainConversation({ + client, + toolManager, + systemMessage: SYSTEM, + initialMessages: [USER], + developmentMode: "auto-accept", + nonInteractiveAlwaysAllow: [], + abortSignal: new AbortController().signal, + }); + + t.is(outcome.kind, "error"); + if (outcome.kind === "error") { + t.regex(outcome.message, /repeated the same tool call 3 times/i); + t.regex(outcome.message, /maxRepeatedToolCalls/); + } + t.is(calls.length, 3, "third identical turn trips the cap"); + t.is(handlerCalls, 2, "the capped turn must not execute the tool again"); + }, +); + +test.serial( + "identical tool calls one under the limit do not trip the cap", + async (t) => { + const calls: RecordedCall[] = []; + const client = makeRecordingClient( + [ + repeatingToolResponse(), + repeatingToolResponse(), + { choices: [{ message: { role: "assistant", content: "all done" } }] }, + ], + calls, + ); + const toolManager = makeFakeToolManager({ + knownTools: new Set(["safe_tool"]), + needsApprovalByName: { safe_tool: false }, + }); + setToolRegistryGetter(() => ({ + safe_tool: (async () => "tool-output") as ToolHandler, + })); + + const outcome = await runPlainConversation({ + client, + toolManager, + systemMessage: SYSTEM, + initialMessages: [USER], + developmentMode: "auto-accept", + nonInteractiveAlwaysAllow: [], + abortSignal: new AbortController().signal, + }); + + t.is(outcome.kind, "success"); + t.is(calls.length, 3); + }, +); + +test.serial( + "different tool calls between repeats reset the streak", + async (t) => { + const otherToolResponse: Partial = { + choices: [ + { + message: { + role: "assistant", + content: "", + tool_calls: [ + { + id: "call-other", + function: { name: "other_tool", arguments: {} }, + }, + ], + }, + }, + ], + }; + const calls: RecordedCall[] = []; + const client = makeRecordingClient( + [ + repeatingToolResponse(), + repeatingToolResponse(), + otherToolResponse, + repeatingToolResponse(), + { choices: [{ message: { role: "assistant", content: "all done" } }] }, + ], + calls, + ); + const toolManager = makeFakeToolManager({ + knownTools: new Set(["safe_tool", "other_tool"]), + needsApprovalByName: { safe_tool: false, other_tool: false }, + }); + setToolRegistryGetter(() => ({ + safe_tool: (async () => "tool-output") as ToolHandler, + other_tool: (async () => "other-output") as ToolHandler, + })); + + const outcome = await runPlainConversation({ + client, + toolManager, + systemMessage: SYSTEM, + initialMessages: [USER], + developmentMode: "auto-accept", + nonInteractiveAlwaysAllow: [], + abortSignal: new AbortController().signal, + }); + + t.is(outcome.kind, "success"); + t.is(calls.length, 5, "streak resets on a different signature"); + }, +); + +test.serial( + "repeated-tool-call cap honors a custom configured limit", + async (t) => { + await withRetryLimit("maxRepeatedToolCalls", 2, async () => { + const calls: RecordedCall[] = []; + const client = makeRecordingClient( + [repeatingToolResponse(), repeatingToolResponse()], + calls, + ); + const toolManager = makeFakeToolManager({ + knownTools: new Set(["safe_tool"]), + needsApprovalByName: { safe_tool: false }, + }); + setToolRegistryGetter(() => ({ + safe_tool: (async () => "tool-output") as ToolHandler, + })); + + const outcome = await runPlainConversation({ + client, + toolManager, + systemMessage: SYSTEM, + initialMessages: [USER], + developmentMode: "auto-accept", + nonInteractiveAlwaysAllow: [], + abortSignal: new AbortController().signal, + }); + + t.is(outcome.kind, "error"); + if (outcome.kind === "error") { + t.regex(outcome.message, /repeated the same tool call 2 times/i); + } + t.is(calls.length, 2, "configured limit of 2 trips on the second turn"); + }); + }, +); + +test.serial( + "empty-turn cap honors a custom configured limit of zero", + async (t) => { + await withRetryLimit("maxEmptyTurns", 0, async () => { + const client = makeFakeClient({ + responses: [ + { choices: [{ message: { role: "assistant", content: "" } }] }, + ], + }); + const toolManager = makeFakeToolManager(); + + const outcome = await runPlainConversation({ + client, + toolManager, + systemMessage: SYSTEM, + initialMessages: [USER], + developmentMode: "auto-accept", + nonInteractiveAlwaysAllow: [], + abortSignal: new AbortController().signal, + }); + + t.is(outcome.kind, "error"); + if (outcome.kind === "error") { + t.regex(outcome.message, /produced no output after 1 attempts/i); + } + }); + }, +); + +test.serial( + "malformed tool calls self-correct up to the cap, then hard-stop", + async (t) => { + // Default maxMalformedRetries = 2: initial + 2 retries = 3 calls. + // '[tool_use: name]' is a shape the XML parser rejects as malformed. + const malformed: Partial = { + choices: [ + { message: { role: "assistant", content: "[tool_use: safe_tool]" } }, + ], + toolsDisabled: true, + }; + const calls: RecordedCall[] = []; + const client = makeRecordingClient([malformed, malformed, malformed], calls); + const toolManager = makeFakeToolManager({ + knownTools: new Set(["safe_tool"]), + }); + + const outcome = await runPlainConversation({ + client, + toolManager, + systemMessage: SYSTEM, + initialMessages: [USER], + developmentMode: "auto-accept", + nonInteractiveAlwaysAllow: [], + abortSignal: new AbortController().signal, + }); + + t.is(outcome.kind, "error"); + if (outcome.kind === "error") { + t.regex(outcome.message, /malformed tool calls 3 times/i); + t.regex(outcome.message, /maxMalformedRetries/); + } + t.is(calls.length, 3); + + // The self-correction feedback must reach the model on retry turns. + const retryTurnMessages = calls[1].messages; + const feedback = retryTurnMessages.find( + (m) => + m.role === "user" && + /contained a malformed tool call/i.test(String(m.content)), + ); + t.truthy(feedback, "retry turn must carry the parse-error feedback"); + }, +); + +test.serial( + "malformed tool call recovers when the model self-corrects", + async (t) => { + const client = makeFakeClient({ + responses: [ + { + choices: [ + { message: { role: "assistant", content: "[tool_use: safe_tool]" } }, + ], + toolsDisabled: true, + }, + { + choices: [{ message: { role: "assistant", content: "recovered" } }], + toolsDisabled: true, + }, + ], + }); + const toolManager = makeFakeToolManager({ + knownTools: new Set(["safe_tool"]), + }); + + const outcome = await runPlainConversation({ + client, + toolManager, + systemMessage: SYSTEM, + initialMessages: [USER], + developmentMode: "auto-accept", + nonInteractiveAlwaysAllow: [], + abortSignal: new AbortController().signal, + }); + + t.is(outcome.kind, "success"); + }, +); diff --git a/source/plain/conversation.ts b/source/plain/conversation.ts index e12dfae89..c0d75f4b6 100644 --- a/source/plain/conversation.ts +++ b/source/plain/conversation.ts @@ -1,4 +1,10 @@ import {DEFAULT_HEADLESS_MAX_TURNS, getAppConfig} from '@/config/index'; +import { + MAX_EMPTY_TURNS, + MAX_MALFORMED_RETRIES, + MAX_REPEATED_TOOL_CALLS, +} from '@/constants'; +import {computeToolCallSignature} from '@/hooks/chat-handler/utils/tool-signature'; import {processToolUse} from '@/message-handler'; import {color, write, writeError, writeLine, writeStatus} from '@/plain/writer'; import {parseToolCalls} from '@/tool-calling/index'; @@ -83,6 +89,11 @@ const FINAL_TURN_INSTRUCTION = * The turn ceiling guards against a wedged model looping unbounded in an * unattended run. It defaults to DEFAULT_HEADLESS_MAX_TURNS and is overridable * via the NANOCODER_MAX_TURNS env var or `nanocoder.headless.maxTurns` config. + * + * Within that ceiling the `nanocoder.retries` limits also apply, mirroring the + * interactive loop: consecutive identical tool calls, consecutive empty turns, + * and malformed tool-call retries each hard-stop with a clear error once their + * cap is hit (there is no user to ask in a plain run). */ export async function runPlainConversation( options: RunPlainConversationOptions, @@ -124,6 +135,23 @@ export async function runPlainConversation( const maxTurns = getAppConfig().headless?.maxTurns ?? DEFAULT_HEADLESS_MAX_TURNS; + // Agent-loop retry limits (`nanocoder.retries`): the same caps the + // interactive loop applies. There is nobody to ask in a plain run, so + // hitting any of them hard-stops with a clear error instead of pausing. + const retryLimits = getAppConfig().retries; + const maxRepeatedToolCalls = + retryLimits?.maxRepeatedToolCalls ?? MAX_REPEATED_TOOL_CALLS; + const maxEmptyTurns = retryLimits?.maxEmptyTurns ?? MAX_EMPTY_TURNS; + const maxMalformedRetries = + retryLimits?.maxMalformedRetries ?? MAX_MALFORMED_RETRIES; + + // Consecutive-failure streaks. Each kind of failing turn increments its own + // counter and resets the others; any healthy turn resets all of them. + let emptyTurnCount = 0; + let malformedRetryCount = 0; + let lastToolSignature = ''; + let repeatedToolCallCount = 0; + for (let turn = 0; turn < maxTurns; turn++) { if (abortSignal.aborted) { return { @@ -251,17 +279,41 @@ export async function runPlainConversation( }; if (!xmlParse.success) { + // Same self-correction loop the interactive runtime runs: feed the + // parse error back to the model, capped so a model stuck producing + // bad tool calls cannot drain tokens unbounded. + if (malformedRetryCount >= maxMalformedRetries) { + const message = `Model produced malformed tool calls ${maxMalformedRetries + 1} times in a row and cannot self-correct — stopping (nanocoder.retries.maxMalformedRetries = ${maxMalformedRetries}).`; + if (!isJson) { + writeError(message); + } + return { + kind: 'error', + message, + finalText: accumulatedFinalText, + reasoning: accumulatedReasoning || null, + toolCalls: toolCallsLog, + usage: getUsage(), + }; + } + malformedRetryCount += 1; + emptyTurnCount = 0; + lastToolSignature = ''; + repeatedToolCallCount = 0; if (!isJson) { - writeError(`Malformed tool call: ${xmlParse.error}`); + writeError( + `Malformed tool call: ${xmlParse.error} — asking the model to retry (${malformedRetryCount}/${maxMalformedRetries}).`, + ); } - return { - kind: 'error', - message: xmlParse.error, - finalText: accumulatedFinalText, - reasoning: accumulatedReasoning || null, - toolCalls: toolCallsLog, - usage: getUsage(), - }; + messages = [ + ...messages, + {role: 'assistant', content: fullContent}, + { + role: 'user', + content: `Your previous response contained a malformed tool call. ${xmlParse.error}\n\n${xmlParse.examples}\n\nPlease try again using the correct format.`, + }, + ]; + continue; } const allToolCalls: ToolCall[] = [ @@ -296,31 +348,66 @@ export async function runPlainConversation( validToolCalls.push(toolCall); } - messages = [ - ...messages, - { - role: 'assistant', - content: cleanedContent, - tool_calls: validToolCalls.length > 0 ? validToolCalls : undefined, - reasoning: streamedReasoning || undefined, - }, - ]; + // Skip appending a fully-empty assistant message (no content, no tool + // calls): providers reject them, and the empty-turn nudge below re-asks + // without one — same rule the interactive loop applies. + const hasAssistantPayload = + cleanedContent.trim() || + validToolCalls.length > 0 || + errorResults.length > 0; + if (hasAssistantPayload) { + messages = [ + ...messages, + { + role: 'assistant', + content: cleanedContent, + tool_calls: validToolCalls.length > 0 ? validToolCalls : undefined, + reasoning: streamedReasoning || undefined, + }, + ]; + } if (errorResults.length > 0) { + emptyTurnCount = 0; + malformedRetryCount = 0; + lastToolSignature = ''; + repeatedToolCallCount = 0; messages = [...messages, ...errorResults]; continue; } if (validToolCalls.length === 0) { if (!cleanedContent.trim()) { - return { - kind: 'error', - message: 'Model returned an empty response with no tool calls', - finalText: accumulatedFinalText, - reasoning: accumulatedReasoning || null, - toolCalls: toolCallsLog, - usage: getUsage(), - }; + // Nudge through consecutive empty turns up to the cap, mirroring + // the interactive loop, then stop so a silent model cannot spin. + if (emptyTurnCount >= maxEmptyTurns) { + const message = `Model produced no output after ${maxEmptyTurns + 1} attempts — stopping (nanocoder.retries.maxEmptyTurns = ${maxEmptyTurns}).`; + if (!isJson) { + writeError(message); + } + return { + kind: 'error', + message, + finalText: accumulatedFinalText, + reasoning: accumulatedReasoning || null, + toolCalls: toolCallsLog, + usage: getUsage(), + }; + } + emptyTurnCount += 1; + malformedRetryCount = 0; + lastToolSignature = ''; + repeatedToolCallCount = 0; + if (!isJson) { + writeStatus( + `empty response — retry ${emptyTurnCount}/${maxEmptyTurns}`, + ); + } + messages = [ + ...messages, + {role: 'user', content: 'Please continue with the task.'}, + ]; + continue; } return { kind: 'success', @@ -331,6 +418,34 @@ export async function runPlainConversation( }; } + // Loop detection: a model re-issuing the identical tool call(s) on + // consecutive turns is almost certainly stuck. In the interactive + // runtime this pauses and asks; here it hard-stops before executing + // the repeat that hits the cap. + const currentToolSignature = computeToolCallSignature(validToolCalls); + const currentRepeatedCount = + currentToolSignature && currentToolSignature === lastToolSignature + ? repeatedToolCallCount + 1 + : 1; + if (currentRepeatedCount >= maxRepeatedToolCalls) { + const message = `Model repeated the same tool call ${currentRepeatedCount} times in a row without making progress — stopping to avoid a loop (nanocoder.retries.maxRepeatedToolCalls = ${maxRepeatedToolCalls}).`; + if (!isJson) { + writeError(message); + } + return { + kind: 'error', + message, + finalText: accumulatedFinalText, + reasoning: accumulatedReasoning || null, + toolCalls: toolCallsLog, + usage: getUsage(), + }; + } + lastToolSignature = currentToolSignature; + repeatedToolCallCount = currentRepeatedCount; + emptyTurnCount = 0; + malformedRetryCount = 0; + const toolsNeedingApproval: string[] = []; const toolsToExecute: ToolCall[] = []; for (const toolCall of validToolCalls) { From b3eb7ca2761326c90a22467cade741d606cb248e Mon Sep 17 00:00:00 2001 From: shoryabansalgithub Date: Wed, 19 Aug 2026 01:06:58 +0530 Subject: [PATCH 07/17] no-mistakes(review): extract shared getRetryLimits helper for both runtimes --- source/config/index.ts | 17 +++++++++++++++++ .../conversation/conversation-loop.tsx | 17 ++++------------- source/plain/conversation.ts | 17 ++++++----------- 3 files changed, 27 insertions(+), 24 deletions(-) diff --git a/source/config/index.ts b/source/config/index.ts index 03687fef9..25925b27f 100644 --- a/source/config/index.ts +++ b/source/config/index.ts @@ -639,6 +639,23 @@ export function getAppConfig(): AppConfig { return _appConfig; } +/** + * Agent-loop retry limits, read live from the current app config so runtime + * edits (and tests that mutate `getAppConfig().retries`) are picked up. The + * hardcoded caps are the single fallback for configs loaded before `retries` + * existed. + * @public + */ +export function getRetryLimits(): RetryLimitsConfig { + return ( + getAppConfig().retries ?? { + maxRepeatedToolCalls: MAX_REPEATED_TOOL_CALLS, + maxEmptyTurns: MAX_EMPTY_TURNS, + maxMalformedRetries: MAX_MALFORMED_RETRIES, + } + ); +} + // Function to reload the app configuration (useful after config file changes) export function reloadAppConfig(): void { _appConfig = loadAppConfig(); diff --git a/source/hooks/chat-handler/conversation/conversation-loop.tsx b/source/hooks/chat-handler/conversation/conversation-loop.tsx index ced99eb2f..b9036cce8 100644 --- a/source/hooks/chat-handler/conversation/conversation-loop.tsx +++ b/source/hooks/chat-handler/conversation/conversation-loop.tsx @@ -3,13 +3,8 @@ import type {ConversationStateManager} from '@/app/utils/conversation-state'; import AssistantMessage from '@/components/assistant-message'; import AssistantReasoning from '@/components/assistant-reasoning'; import {ErrorMessage, InfoMessage} from '@/components/message-box'; -import {getAppConfig} from '@/config/index'; -import { - MAX_COMPACT_RETRIES, - MAX_EMPTY_TURNS, - MAX_MALFORMED_RETRIES, - MAX_REPEATED_TOOL_CALLS, -} from '@/constants'; +import {getAppConfig, getRetryLimits} from '@/config/index'; +import {MAX_COMPACT_RETRIES} from '@/constants'; import {generateKey} from '@/session/key-generator'; import { parseToolCalls, @@ -202,12 +197,8 @@ export const processAssistantResponse = async ( // Agent-loop retry limits: configurable via `nanocoder.retries` in // agents.config.json, falling back to the historical hardcoded caps. - const retryLimits = getAppConfig().retries; - const maxRepeatedToolCalls = - retryLimits?.maxRepeatedToolCalls ?? MAX_REPEATED_TOOL_CALLS; - const maxEmptyTurns = retryLimits?.maxEmptyTurns ?? MAX_EMPTY_TURNS; - const maxMalformedRetries = - retryLimits?.maxMalformedRetries ?? MAX_MALFORMED_RETRIES; + const {maxRepeatedToolCalls, maxEmptyTurns, maxMalformedRetries} = + getRetryLimits(); // Helper to flush live task list to the static chat queue const flushLiveTaskList = async () => { diff --git a/source/plain/conversation.ts b/source/plain/conversation.ts index c0d75f4b6..cf8dc702f 100644 --- a/source/plain/conversation.ts +++ b/source/plain/conversation.ts @@ -1,9 +1,8 @@ -import {DEFAULT_HEADLESS_MAX_TURNS, getAppConfig} from '@/config/index'; import { - MAX_EMPTY_TURNS, - MAX_MALFORMED_RETRIES, - MAX_REPEATED_TOOL_CALLS, -} from '@/constants'; + DEFAULT_HEADLESS_MAX_TURNS, + getAppConfig, + getRetryLimits, +} from '@/config/index'; import {computeToolCallSignature} from '@/hooks/chat-handler/utils/tool-signature'; import {processToolUse} from '@/message-handler'; import {color, write, writeError, writeLine, writeStatus} from '@/plain/writer'; @@ -138,12 +137,8 @@ export async function runPlainConversation( // Agent-loop retry limits (`nanocoder.retries`): the same caps the // interactive loop applies. There is nobody to ask in a plain run, so // hitting any of them hard-stops with a clear error instead of pausing. - const retryLimits = getAppConfig().retries; - const maxRepeatedToolCalls = - retryLimits?.maxRepeatedToolCalls ?? MAX_REPEATED_TOOL_CALLS; - const maxEmptyTurns = retryLimits?.maxEmptyTurns ?? MAX_EMPTY_TURNS; - const maxMalformedRetries = - retryLimits?.maxMalformedRetries ?? MAX_MALFORMED_RETRIES; + const {maxRepeatedToolCalls, maxEmptyTurns, maxMalformedRetries} = + getRetryLimits(); // Consecutive-failure streaks. Each kind of failing turn increments its own // counter and resets the others; any healthy turn resets all of them. From ed1768c9893c40831a404b57ae9270ba1f88d149 Mon Sep 17 00:00:00 2001 From: shoryabansalgithub Date: Wed, 19 Aug 2026 01:59:17 +0530 Subject: [PATCH 08/17] no-mistakes(document): sync run-mode, ACP scope, and CLAUDE.md docs for retry limits --- CLAUDE.md | 2 +- docs/configuration/index.md | 2 ++ docs/features/commands.md | 5 ++++- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index e2973b267..c5e50e010 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -84,7 +84,7 @@ File editing uses a content-based approach: - `string_replace`: Primary edit tool — replaces exact content - `write_file`: Whole file overwrites -Two execution paths exist: native tool calling (preferred, via AI SDK) and an XML fallback for models that don't support tools. `LLMChatResponse.toolsDisabled` signals which path produced the response; the conversation loop only runs `parseToolCalls()` (in `source/tool-calling/`) when `toolsDisabled` is true. +Two execution paths exist: native tool calling (preferred, via AI SDK) and an XML fallback for models that don't support tools. `LLMChatResponse.toolsDisabled` signals which path produced the response. The conversation loop runs `parseToolCalls()` (in `source/tool-calling/`) whenever the response has no native tool calls — always on the fallback path, and on the native path too, since models marketed as native-tool-capable sometimes regress to emitting tool-call text. Malformed text there feeds the self-correction retry loop capped by `nanocoder.retries.maxMalformedRetries`. ### Command System diff --git a/docs/configuration/index.md b/docs/configuration/index.md index fc53b8ce6..336473206 100644 --- a/docs/configuration/index.md +++ b/docs/configuration/index.md @@ -197,6 +197,8 @@ Caps on how many times the conversation loop auto-retries a failing pattern with Choosing "Continue" at the repeated-tool-call prompt grants another window of the same size, so a genuinely stuck model is re-checked rather than left looping. +Unlike [Headless](#headless), these limits do not cover the ACP loop (`--acp`, used by editor clients), which is bounded by `maxTurns` alone, nor delegated [subagent](../features/subagents.md) runs, which run their own loop. + ### Paste Handling Configure how pasted text is handled in the input. By default, single-line pastes of 800 characters or fewer are inserted directly, while longer or multi-line pastes are collapsed into a `[Paste #N: X chars]` placeholder. diff --git a/docs/features/commands.md b/docs/features/commands.md index 46a47174a..546055c5c 100644 --- a/docs/features/commands.md +++ b/docs/features/commands.md @@ -99,6 +99,9 @@ nanocoder --mode yolo run "update README and push" ``` If a tool requires approval that the active mode won't grant, nanocoder prints `Tool approval required for: ...` and exits with status code `1`. + +Because there is nobody to answer a prompt in a `run`, the agent-loop [retry limits](../configuration/index.md#retry-limits) hard-stop instead of pausing: a model that repeats the same tool call, returns empty responses, or keeps emitting malformed tool calls past its configured cap ends the run with an error naming the setting. Under the `--plain` runtime (used automatically in CI and non-TTY environments) that exits with status code `1`. + ### JSON Output For CI pipelines, scripting, and tool chaining, pass `--json` (alias `--output-format json`) alongside `run` to get a single structured JSON object on `stdout` instead of streamed markdown: @@ -152,4 +155,4 @@ The emitted object looks like: Two more fields appear conditionally: `message` (the error text, when `kind` is `"error"`) and `toolNames` (the tools awaiting approval, when `kind` is `"tool-approval-required"`). -On error (e.g. an untrusted workspace directory, or the turn limit being hit without a final answer), `kind` is `"error"` and the object still includes whatever `toolCalls` were captured before the failure, so partial progress isn't silently dropped. +On error (e.g. an untrusted workspace directory, the turn limit being hit without a final answer, or an agent-loop [retry limit](../configuration/index.md#retry-limits) being hit), `kind` is `"error"` and the object still includes whatever `toolCalls` were captured before the failure, so partial progress isn't silently dropped. From 863717a88d9ee3f4ad14a24f490adf5df282fbb3 Mon Sep 17 00:00:00 2001 From: shoryabansalgithub Date: Wed, 19 Aug 2026 02:30:24 +0530 Subject: [PATCH 09/17] feat(retries): count unknown-tool turns in the repeated-call streak and cap subagent loops Apply the four captain-decided fixes from review round 2 of #897: - Unknown-tool turns now count toward the maxRepeatedToolCalls streak in both runtimes, so a model stuck calling a nonexistent tool trips the cap (interactive: pause-and-ask; plain: hard stop) instead of looping until the turn ceiling. The streak/prompt logic is shared between the valid and unknown-tool branches. - The subagent executor loop, previously uncapped, now applies maxRepeatedToolCalls and stops with an error naming the setting. - Docs: loud CI warning that legitimate polling patterns in --plain runs hard-stop at the cap and need maxRepeatedToolCalls raised; fixed the off-by-one in the retry-limits table (a limit of 3 executes the repeated call twice and pauses on the third emission) and reworded the continue option to 'check again after N more' to match the implementation. - Tests for the unknown-tool path in both loops and for all subagent cap paths (trip, one-under, unknown-tool, custom limit). Also fixes a misplaced biome-ignore in chat-panel-harness.ts that left two standing lint warnings. --- .changeset/configurable-retry-limits.md | 2 +- docs/configuration/index.md | 8 +- docs/features/commands.md | 2 + .../conversation/conversation-loop.spec.ts | 117 +++++++++++++ .../conversation/conversation-loop.tsx | 158 +++++++++++------- source/plain/conversation.spec.ts | 84 ++++++++++ source/plain/conversation.ts | 70 +++++--- source/subagents/subagent-executor.spec.ts | 106 ++++++++++++ source/subagents/subagent-executor.ts | 27 ++- source/vscode/chat-panel-harness.ts | 6 +- 10 files changed, 481 insertions(+), 99 deletions(-) diff --git a/.changeset/configurable-retry-limits.md b/.changeset/configurable-retry-limits.md index 1c0bcd93c..8bddb584c 100644 --- a/.changeset/configurable-retry-limits.md +++ b/.changeset/configurable-retry-limits.md @@ -2,4 +2,4 @@ "@nanocollective/nanocoder": minor --- -Add configurable agent-loop retry limits to prevent token drain (#897). A new `nanocoder.retries` section in `agents.config.json` exposes the previously hardcoded caps: `maxRepeatedToolCalls` (default 3), `maxEmptyTurns` (default 2), and `maxMalformedRetries` (default 2). When the repeated-tool-call limit is hit in an interactive session, Nanocoder now pauses and asks whether to continue (granting another window of attempts) or stop, instead of always hard-stopping; non-interactive runs keep the hard stop. The same limits now also protect the `--plain` runtime used by `nanocoder run` in CI and non-TTY environments, which previously had no repeated-call, empty-turn, or malformed-retry caps at all: each cap hard-stops with a clear error there. +Add configurable agent-loop retry limits to prevent token drain (#897). A new `nanocoder.retries` section in `agents.config.json` exposes the previously hardcoded caps: `maxRepeatedToolCalls` (default 3), `maxEmptyTurns` (default 2), and `maxMalformedRetries` (default 2). When the repeated-tool-call limit is hit in an interactive session, Nanocoder now pauses and asks whether to continue (granting another window of attempts) or stop, instead of always hard-stopping; non-interactive runs keep the hard stop. The same limits now also protect the `--plain` runtime used by `nanocoder run` in CI and non-TTY environments, which previously had no repeated-call, empty-turn, or malformed-retry caps at all: each cap hard-stops with a clear error there. Calls to unknown tools count toward the repeated-call streak in both runtimes, so a model stuck on a nonexistent tool trips the same cap instead of looping until the turn ceiling. Delegated subagent runs, whose loop previously had no cap at all, now apply `maxRepeatedToolCalls` too and stop with an error naming the setting. diff --git a/docs/configuration/index.md b/docs/configuration/index.md index 336473206..42cf536f0 100644 --- a/docs/configuration/index.md +++ b/docs/configuration/index.md @@ -191,13 +191,15 @@ Caps on how many times the conversation loop auto-retries a failing pattern with | Option | Type | Default | Description | |--------|------|---------|-------------| -| `maxRepeatedToolCalls` | number | `3` | Consecutive identical tool calls allowed before the loop pauses (minimum 2). In an interactive session you are asked whether to continue — useful when the repetition is legitimate, such as polling a long-running job — or stop. `--plain` and other non-interactive runs stop with a clear error. | +| `maxRepeatedToolCalls` | number | `3` | Pause threshold for consecutive identical tool calls (minimum 2). The check fires when the same call (or set of calls) is emitted for the Nth consecutive turn, before that call runs - so the default of 3 executes the repeated call twice and pauses on the third emission. In an interactive session you are asked whether to continue - useful when the repetition is legitimate, such as polling a long-running job - or stop. `--plain` and other non-interactive runs stop with a clear error. Calls to unknown tools count toward the streak too, so a model stuck on a nonexistent tool hits the same cap. | | `maxEmptyTurns` | number | `2` | Consecutive empty assistant turns that are auto-nudged before giving up (minimum 0). The interactive loop additionally compacts the context and retries once before stopping; the `--plain` runtime stops directly after the nudges. | | `maxMalformedRetries` | number | `2` | Malformed self-correction retries allowed for text-parsed tool calls before the loop gives up (minimum 0). Applies to the XML fallback path and to native-tool models that emit tool-call text instead of native tool calls. | -Choosing "Continue" at the repeated-tool-call prompt grants another window of the same size, so a genuinely stuck model is re-checked rather than left looping. +Choosing "Continue" at the repeated-tool-call prompt runs the paused call and re-checks after `maxRepeatedToolCalls` further identical calls, so a genuinely stuck model is re-prompted rather than left looping. -Unlike [Headless](#headless), these limits do not cover the ACP loop (`--acp`, used by editor clients), which is bounded by `maxTurns` alone, nor delegated [subagent](../features/subagents.md) runs, which run their own loop. +> **Warning - CI polling patterns:** in `--plain` runs (`nanocoder run "..."` in CI and non-TTY environments) there is no prompt to answer, so `maxRepeatedToolCalls` is a hard stop. A workflow whose model legitimately repeats the identical command - polling a deploy, re-running the same check while waiting on an external state change - aborts with exit code `1` once the cap is hit, by default on the third consecutive identical call. Raise `nanocoder.retries.maxRepeatedToolCalls` in that project's `agents.config.json` before relying on such a polling pattern. + +Unlike [Headless](#headless), these limits do not cover the ACP loop (`--acp`, used by editor clients), which is bounded by `maxTurns` alone. Delegated [subagent](../features/subagents.md) runs apply `maxRepeatedToolCalls` - a stuck subagent stops with an error naming the setting, since there is nobody to ask inside a delegated run - but not the other two limits: a subagent's loop ends on its own after an empty turn, and it does not use text-parsed tool calls. ### Paste Handling diff --git a/docs/features/commands.md b/docs/features/commands.md index 546055c5c..ef77e2fcf 100644 --- a/docs/features/commands.md +++ b/docs/features/commands.md @@ -102,6 +102,8 @@ If a tool requires approval that the active mode won't grant, nanocoder prints ` Because there is nobody to answer a prompt in a `run`, the agent-loop [retry limits](../configuration/index.md#retry-limits) hard-stop instead of pausing: a model that repeats the same tool call, returns empty responses, or keeps emitting malformed tool calls past its configured cap ends the run with an error naming the setting. Under the `--plain` runtime (used automatically in CI and non-TTY environments) that exits with status code `1`. +> **Warning - CI polling patterns:** the repeated-call hard stop triggers on *legitimate* repetition too. If your workflow's model is expected to run the identical command repeatedly - polling a deploy, waiting on a slow job by re-running the same check - the run aborts once `maxRepeatedToolCalls` consecutive identical calls are emitted (default 3). Raise `nanocoder.retries.maxRepeatedToolCalls` in that project's `agents.config.json` before relying on such a pattern in CI. + ### JSON Output For CI pipelines, scripting, and tool chaining, pass `--json` (alias `--output-format json`) alongside `run` to get a single structured JSON object on `stdout` instead of streamed markdown: diff --git a/source/hooks/chat-handler/conversation/conversation-loop.spec.ts b/source/hooks/chat-handler/conversation/conversation-loop.spec.ts index dd6f83be8..945945422 100644 --- a/source/hooks/chat-handler/conversation/conversation-loop.spec.ts +++ b/source/hooks/chat-handler/conversation/conversation-loop.spec.ts @@ -2249,6 +2249,123 @@ test.serial('repeated-tool-call limit hard-stops without prompting in non-intera t.truthy(stopMessage, 'Should still queue the loop-detected ErrorMessage'); }); +// Client that always calls a tool the tool manager does not know — this goes +// through the unknown-tool error branch rather than tool execution. +const createUnknownToolClient = (onChat?: () => void) => ({ + chat: async (): Promise => { + onChat?.(); + return { + choices: [ + { + message: { + role: 'assistant', + content: '', + tool_calls: [ + { + id: 'call_ghost', + function: {name: 'ghost_tool', arguments: '{"x": 1}'}, + }, + ], + }, + }, + ], + toolsDisabled: false, + }; + }, +}); + +test.serial('repeated unknown-tool calls count toward the repeated-call limit', async t => { + let chatCallCount = 0; + let questionCount = 0; + const queuedComponents: any[] = []; + + setGlobalQuestionHandler(async question => { + questionCount += 1; + t.regex(question.question, /repeated the same tool call 3 times in a row/); + // Stop: the safe default. + return question.options[0]; + }); + + await withRetryLimits({maxRepeatedToolCalls: 3}, async () => { + const params = createDefaultParams({ + client: createUnknownToolClient(() => chatCallCount++), + toolManager: repeatingToolManager(), + addToChatQueue: (component: any) => queuedComponents.push(component), + }); + + await processAssistantResponse(params); + }); + + // The unknown-tool self-correction branch must not recurse unbounded: the + // third identical unknown-tool turn trips the same cap as a real call. + t.is(chatCallCount, 3, 'Third identical unknown-tool turn trips the limit'); + t.is(questionCount, 1, 'Should pause and ask instead of recursing unbounded'); + const stopMessage = queuedComponents.find( + (c: any) => + typeof c.props?.message === 'string' && + c.props.message.includes('repeated the same tool call 3 times in a row'), + ); + t.truthy(stopMessage, 'Should queue the loop-detected ErrorMessage'); +}); + +test.serial('repeated unknown-tool calls grant another window when the user continues', async t => { + let chatCallCount = 0; + let questionCount = 0; + const questionTexts: string[] = []; + + setGlobalQuestionHandler(async question => { + questionCount += 1; + questionTexts.push(question.question); + // Continue on the first prompt, stop on the second. + return questionCount === 1 ? question.options[1] : question.options[0]; + }); + + await withRetryLimits({maxRepeatedToolCalls: 3}, async () => { + const params = createDefaultParams({ + client: createUnknownToolClient(() => chatCallCount++), + toolManager: repeatingToolManager(), + }); + + await processAssistantResponse(params); + }); + + t.is(chatCallCount, 6, 'Continue should grant a full new window of turns'); + t.is(questionCount, 2, 'Should re-prompt after the granted window is spent'); + t.regex(questionTexts[0], /repeated the same tool call 3 times in a row/); + t.regex(questionTexts[1], /repeated the same tool call 6 times in a row/); +}); + +test.serial('repeated unknown-tool calls hard-stop without prompting in non-interactive mode', async t => { + let chatCallCount = 0; + let questionCount = 0; + const queuedComponents: any[] = []; + + setGlobalQuestionHandler(async question => { + questionCount += 1; + return question.options[1]; + }); + + await withRetryLimits({maxRepeatedToolCalls: 3}, async () => { + const params = createDefaultParams({ + client: createUnknownToolClient(() => chatCallCount++), + toolManager: repeatingToolManager(), + nonInteractiveMode: true, + addToChatQueue: (component: any) => queuedComponents.push(component), + }); + + await processAssistantResponse(params); + }); + + t.is(chatCallCount, 3, 'Loop should hard-stop at the limit'); + t.is(questionCount, 0, 'Must not prompt when there is nobody to ask'); + const stopMessage = queuedComponents.find( + (c: any) => + typeof c.props?.message === 'string' && + c.props.message.includes('repeated the same tool call'), + ); + t.truthy(stopMessage, 'Should still queue the loop-detected ErrorMessage'); +}); + test.serial('malformed-retry limit honors a custom configured value', async t => { let chatCallCount = 0; const queuedComponents: any[] = []; diff --git a/source/hooks/chat-handler/conversation/conversation-loop.tsx b/source/hooks/chat-handler/conversation/conversation-loop.tsx index b9036cce8..12f895636 100644 --- a/source/hooks/chat-handler/conversation/conversation-loop.tsx +++ b/source/hooks/chat-handler/conversation/conversation-loop.tsx @@ -645,6 +645,79 @@ export const processAssistantResponse = async ( setStreamingContent(''); setStreamingReasoning(''); + // This turn's repeated-call streak, computed over every call the model + // emitted — unknown tools included, so a model stuck calling a nonexistent + // tool trips the same cap as one re-running a real call. + const currentToolSignature = computeToolCallSignature(allToolCalls); + const currentRepeatedCount = + currentToolSignature && currentToolSignature === lastToolSignature + ? repeatedToolCallCount + 1 + : 1; + // Streak carried into the recursive continuation below. Reset to 0 when + // the user grants another window at the limit prompt, so the re-prompt + // cadence stays one full window rather than firing every turn. + let repeatedCountForNextTurn = currentRepeatedCount; + // True consecutive-repeat streak, never reset by a granted window, so + // user-facing counts don't restart at the limit after each continue. + const currentRepeatedTotal = + currentToolSignature && currentToolSignature === lastToolSignature + ? repeatedToolCallTotal + 1 + : 1; + + // When the repeated-call streak hits the cap: pause and ask in an + // interactive session (the repetition may be legitimate, e.g. polling a + // long-running job, and the user is the only one who can tell). Headless + // and non-interactive runs have nobody to ask, so they never continue. + // Returns true when the user granted another window. + const promptContinueRepeatedCalls = async (): Promise => { + const liveMode = developmentModeRef?.current ?? developmentMode; + await flushAll(); + if (nonInteractiveMode || liveMode === 'headless') { + return false; + } + setIsGenerating(false); + const stopOption = 'Stop and return to prompt'; + const continueOption = `Continue (check again after ${maxRepeatedToolCalls} more)`; + const answer = await signalQuestion({ + question: `The model has repeated the same tool call ${currentRepeatedTotal} times in a row without making progress. It may be stuck in a loop that drains tokens. Continue anyway?`, + options: [stopOption, continueOption], + allowFreeform: false, + questionType: 'confirmation', + }); + if (answer !== continueOption) { + return false; + } + // User granted another window: reset the streak so the next + // maxRepeatedToolCalls identical calls prompt again instead of + // stopping, then let the caller resume this turn. + repeatedCountForNextTurn = 0; + setIsGenerating(true); + addToChatQueue( + , + ); + return true; + }; + + // Surface the loop-detected stop. Callers must have paired this turn's + // tool calls with results in history before stopping. + const stopForRepeatedCalls = () => { + addToChatQueue( + , + ); + setIsGenerating(false); + if (onConversationComplete) { + onConversationComplete(); + } + }; + // Handle error results for non-existent tools if (errorResults.length > 0) { // Show the user a concise notice. The full recovery hint (including the @@ -680,6 +753,18 @@ export const processAssistantResponse = async ( const updatedMessagesWithError = errorBuilder.build(); setMessages(updatedMessagesWithError); + // Unknown-tool turns count toward the repeated-call streak, so a model + // stuck calling a nonexistent tool cannot recurse unbounded. The error + // and aborted results above are already paired in history, so a stop + // here keeps the 1:1 tool-call/result mapping intact. + if (currentRepeatedCount >= maxRepeatedToolCalls) { + const continueAnyway = await promptContinueRepeatedCalls(); + if (!continueAnyway) { + stopForRepeatedCalls(); + return; + } + } + // Continue the main conversation loop with error messages as context await processAssistantResponse({ ...params, @@ -688,9 +773,9 @@ export const processAssistantResponse = async ( conversationStartTime: startTime, emptyTurnCount: 0, malformedRetryCount: 0, - lastToolSignature: undefined, - repeatedToolCallCount: 0, - repeatedToolCallTotal: 0, + lastToolSignature: currentToolSignature, + repeatedToolCallCount: repeatedCountForNextTurn, + repeatedToolCallTotal: currentRepeatedTotal, }); return; } @@ -700,45 +785,11 @@ export const processAssistantResponse = async ( // Loop detection: if the model re-issues the exact same tool call(s) it // made last turn, it is almost certainly stuck (a small model re-running // an identical failing command, or repeatedly reading the same file). - // Count consecutive identical signatures and stop once the cap is hit so - // we surface an actionable error instead of looping until abort. - const currentToolSignature = computeToolCallSignature(validToolCalls); - const currentRepeatedCount = - currentToolSignature && currentToolSignature === lastToolSignature - ? repeatedToolCallCount + 1 - : 1; - // Streak carried into the recursive continuation below. Reset to 0 when - // the user grants another window at the limit prompt, so the re-prompt - // cadence stays one full window rather than firing every turn. - let repeatedCountForNextTurn = currentRepeatedCount; - // True consecutive-repeat streak, never reset by a granted window, so - // user-facing counts don't restart at the limit after each continue. - const currentRepeatedTotal = - currentToolSignature && currentToolSignature === lastToolSignature - ? repeatedToolCallTotal + 1 - : 1; - + // The streak is computed above (shared with the unknown-tool branch); + // once the cap is hit, pause and ask — or stop with an actionable error + // when there is nobody to ask. if (currentRepeatedCount >= maxRepeatedToolCalls) { - // Interactive sessions pause and ask instead of hard-stopping: the - // repetition may be legitimate (e.g. polling a long-running job), and - // the user is the only one who can tell. Headless / non-interactive - // runs have nobody to ask, so they keep the hard stop. - const liveMode = developmentModeRef?.current ?? developmentMode; - let continueAnyway = false; - await flushAll(); - if (!nonInteractiveMode && liveMode !== 'headless') { - setIsGenerating(false); - const stopOption = 'Stop and return to prompt'; - const continueOption = `Continue (allow ${maxRepeatedToolCalls} more)`; - const answer = await signalQuestion({ - question: `The model has repeated the same tool call ${currentRepeatedTotal} times in a row without making progress. It may be stuck in a loop that drains tokens. Continue anyway?`, - options: [stopOption, continueOption], - allowFreeform: false, - questionType: 'confirmation', - }); - continueAnyway = answer === continueOption; - } - + const continueAnyway = await promptContinueRepeatedCalls(); if (!continueAnyway) { // Keep the AI SDK's 1:1 tool-call/result mapping intact: the assistant // message with these tool_calls is already in history, so pair each @@ -746,32 +797,9 @@ export const processAssistantResponse = async ( const loopBuilder = new MessageBuilder(updatedMessages); loopBuilder.addToolResults(createCancellationResults(validToolCalls)); setMessages(loopBuilder.build()); - addToChatQueue( - , - ); - setIsGenerating(false); - if (onConversationComplete) { - onConversationComplete(); - } + stopForRepeatedCalls(); return; } - - // User granted another window: reset the streak so the next - // maxRepeatedToolCalls identical calls prompt again instead of - // stopping, then resume execution of this turn's tools. - repeatedCountForNextTurn = 0; - setIsGenerating(true); - addToChatQueue( - , - ); } // The SDK never auto-executes tools (execute is stripped). We evaluate diff --git a/source/plain/conversation.spec.ts b/source/plain/conversation.spec.ts index c97d379ac..c6095746f 100644 --- a/source/plain/conversation.spec.ts +++ b/source/plain/conversation.spec.ts @@ -1186,6 +1186,90 @@ test.serial( }, ); +function unknownToolResponse(): Partial { + return { + choices: [ + { + message: { + role: "assistant", + content: "", + tool_calls: [ + { + id: "call-ghost", + function: { name: "ghost_tool", arguments: { x: 1 } }, + }, + ], + }, + }, + ], + }; +} + +test.serial( + "repeated unknown-tool calls count toward the repeated-call cap", + async (t) => { + // A model stuck calling a nonexistent tool must trip + // maxRepeatedToolCalls rather than draining tokens until maxTurns. + const calls: RecordedCall[] = []; + const client = makeRecordingClient( + [unknownToolResponse(), unknownToolResponse(), unknownToolResponse()], + calls, + ); + const toolManager = makeFakeToolManager(); + + const outcome = await runPlainConversation({ + client, + toolManager, + systemMessage: SYSTEM, + initialMessages: [USER], + developmentMode: "auto-accept", + nonInteractiveAlwaysAllow: [], + abortSignal: new AbortController().signal, + }); + + t.is(outcome.kind, "error"); + if (outcome.kind === "error") { + t.regex(outcome.message, /repeated the same tool call 3 times/i); + t.regex(outcome.message, /maxRepeatedToolCalls/); + } + t.is(calls.length, 3, "third identical unknown-tool turn trips the cap"); + t.is( + outcome.toolCalls.filter((c) => c.error?.includes("Unknown tool")).length, + 3, + "every unknown-tool turn is logged as an error", + ); + }, +); + +test.serial( + "unknown-tool calls one under the cap still let the model recover", + async (t) => { + const calls: RecordedCall[] = []; + const client = makeRecordingClient( + [ + unknownToolResponse(), + unknownToolResponse(), + { choices: [{ message: { role: "assistant", content: "all done" } }] }, + ], + calls, + ); + const toolManager = makeFakeToolManager(); + + const outcome = await runPlainConversation({ + client, + toolManager, + systemMessage: SYSTEM, + initialMessages: [USER], + developmentMode: "auto-accept", + nonInteractiveAlwaysAllow: [], + abortSignal: new AbortController().signal, + }); + + t.is(outcome.kind, "success"); + t.is(calls.length, 3, "the recovery turn runs to natural completion"); + }, +); + test.serial( "repeated-tool-call cap honors a custom configured limit", async (t) => { diff --git a/source/plain/conversation.ts b/source/plain/conversation.ts index cf8dc702f..6a7f7d59b 100644 --- a/source/plain/conversation.ts +++ b/source/plain/conversation.ts @@ -147,6 +147,39 @@ export async function runPlainConversation( let lastToolSignature = ''; let repeatedToolCallCount = 0; + // Count this turn's tool-call signature against the repeated-call streak. + // Returns the hard-stop outcome when the cap is hit, null otherwise. Called + // for unknown-tool turns too: a model stuck re-emitting the same + // nonexistent tool is looping just as surely as one re-running a real call. + const trackRepeatedToolCalls = ( + turnToolCalls: ToolCall[], + ): PlainConversationOutcome | null => { + const currentToolSignature = computeToolCallSignature(turnToolCalls); + const currentRepeatedCount = + currentToolSignature && currentToolSignature === lastToolSignature + ? repeatedToolCallCount + 1 + : 1; + if (currentRepeatedCount >= maxRepeatedToolCalls) { + const message = `Model repeated the same tool call ${currentRepeatedCount} times in a row without making progress — stopping to avoid a loop (nanocoder.retries.maxRepeatedToolCalls = ${maxRepeatedToolCalls}).`; + if (!isJson) { + writeError(message); + } + return { + kind: 'error', + message, + finalText: accumulatedFinalText, + reasoning: accumulatedReasoning || null, + toolCalls: toolCallsLog, + usage: getUsage(), + }; + } + lastToolSignature = currentToolSignature; + repeatedToolCallCount = currentRepeatedCount; + emptyTurnCount = 0; + malformedRetryCount = 0; + return null; + }; + for (let turn = 0; turn < maxTurns; turn++) { if (abortSignal.aborted) { return { @@ -363,10 +396,14 @@ export async function runPlainConversation( } if (errorResults.length > 0) { - emptyTurnCount = 0; - malformedRetryCount = 0; - lastToolSignature = ''; - repeatedToolCallCount = 0; + // Unknown-tool turns count toward the repeated-call streak, so a model + // stuck calling a nonexistent tool trips the same cap instead of + // draining tokens until maxTurns. The signature covers every call the + // model emitted this turn, valid and unknown alike. + const stopped = trackRepeatedToolCalls(allToolCalls); + if (stopped) { + return stopped; + } messages = [...messages, ...errorResults]; continue; } @@ -417,29 +454,10 @@ export async function runPlainConversation( // consecutive turns is almost certainly stuck. In the interactive // runtime this pauses and asks; here it hard-stops before executing // the repeat that hits the cap. - const currentToolSignature = computeToolCallSignature(validToolCalls); - const currentRepeatedCount = - currentToolSignature && currentToolSignature === lastToolSignature - ? repeatedToolCallCount + 1 - : 1; - if (currentRepeatedCount >= maxRepeatedToolCalls) { - const message = `Model repeated the same tool call ${currentRepeatedCount} times in a row without making progress — stopping to avoid a loop (nanocoder.retries.maxRepeatedToolCalls = ${maxRepeatedToolCalls}).`; - if (!isJson) { - writeError(message); - } - return { - kind: 'error', - message, - finalText: accumulatedFinalText, - reasoning: accumulatedReasoning || null, - toolCalls: toolCallsLog, - usage: getUsage(), - }; + const stopped = trackRepeatedToolCalls(validToolCalls); + if (stopped) { + return stopped; } - lastToolSignature = currentToolSignature; - repeatedToolCallCount = currentRepeatedCount; - emptyTurnCount = 0; - malformedRetryCount = 0; const toolsNeedingApproval: string[] = []; const toolsToExecute: ToolCall[] = []; diff --git a/source/subagents/subagent-executor.spec.ts b/source/subagents/subagent-executor.spec.ts index cfb6cc6f9..ddebdd798 100644 --- a/source/subagents/subagent-executor.spec.ts +++ b/source/subagents/subagent-executor.spec.ts @@ -1,5 +1,6 @@ import test from 'ava'; import {SubagentExecutor} from './subagent-executor.js'; +import {getAppConfig} from '@/config/index'; import {getModelContextLimit} from '@/models'; import {SubagentLoader, getSubagentLoader} from './subagent-loader.js'; import type {ToolManager} from '@/tools/tool-manager'; @@ -312,6 +313,111 @@ test.serial('handles unknown tool calls', async t => { t.is(result.output, 'Handled missing tool'); }); +// --- Agent-loop retry limits (nanocoder.retries) — issue #897 --- + +const repeatedCallResponse = (name = 'read_file') => ({ + content: '', + tool_calls: [ + {id: 'tc-loop', function: {name, arguments: '{"path": "x"}'}}, + ], +}); + +test.serial('repeated identical tool calls trip the retry cap', async t => { + const toolManager = createMockToolManager({ + read_file: {handler: async () => 'same output', readOnly: true}, + }); + // Default maxRepeatedToolCalls = 3: the identical call executes on turns 1 + // and 2; the third consecutive emission stops the run before executing. + const client = createMockClient([ + repeatedCallResponse(), + repeatedCallResponse(), + repeatedCallResponse(), + {content: 'never reached'}, + ]); + const executor = new SubagentExecutor(toolManager, client); + + const result = await executor.execute({ + subagent_type: 'explore', + description: 'Loop forever', + }); + + t.false(result.success); + t.regex(result.error || '', /repeated the same tool call 3 times/i); + t.regex(result.error || '', /maxRepeatedToolCalls/); +}); + +test.serial('identical tool calls one under the retry cap complete normally', async t => { + const toolManager = createMockToolManager({ + read_file: {handler: async () => 'same output', readOnly: true}, + }); + const client = createMockClient([ + repeatedCallResponse(), + repeatedCallResponse(), + {content: 'done after two repeats'}, + ]); + const executor = new SubagentExecutor(toolManager, client); + + const result = await executor.execute({ + subagent_type: 'explore', + description: 'Repeat twice then finish', + }); + + t.true(result.success); + t.is(result.output, 'done after two repeats'); +}); + +test.serial('repeated unknown-tool calls trip the retry cap', async t => { + // A subagent stuck calling a nonexistent tool must trip the cap too — the + // signature covers every emitted call, not just executable ones. + const toolManager = createMockToolManager(); + const client = createMockClient([ + repeatedCallResponse('ghost_tool'), + repeatedCallResponse('ghost_tool'), + repeatedCallResponse('ghost_tool'), + {content: 'never reached'}, + ]); + const executor = new SubagentExecutor(toolManager, client); + + const result = await executor.execute({ + subagent_type: 'explore', + description: 'Loop on a missing tool', + }); + + t.false(result.success); + t.regex(result.error || '', /repeated the same tool call 3 times/i); +}); + +test.serial('retry cap honors a custom configured maxRepeatedToolCalls', async t => { + const retries = getAppConfig().retries; + if (!retries) { + t.fail('resolved config must carry retry limits'); + return; + } + const original = retries.maxRepeatedToolCalls; + retries.maxRepeatedToolCalls = 2; + try { + const toolManager = createMockToolManager({ + read_file: {handler: async () => 'same output', readOnly: true}, + }); + const client = createMockClient([ + repeatedCallResponse(), + repeatedCallResponse(), + {content: 'never reached'}, + ]); + const executor = new SubagentExecutor(toolManager, client); + + const result = await executor.execute({ + subagent_type: 'explore', + description: 'Loop with a tight cap', + }); + + t.false(result.success); + t.regex(result.error || '', /repeated the same tool call 2 times/i); + } finally { + retries.maxRepeatedToolCalls = original; + } +}); + test.serial('respects abort signal', async t => { const toolManager = createMockToolManager({ read_file: {handler: async () => 'content', readOnly: true}, diff --git a/source/subagents/subagent-executor.ts b/source/subagents/subagent-executor.ts index c3e254af0..643224b6a 100644 --- a/source/subagents/subagent-executor.ts +++ b/source/subagents/subagent-executor.ts @@ -6,7 +6,8 @@ */ import {createLLMClient} from '@/client-factory'; -import {getAppConfig} from '@/config/index'; +import {getAppConfig, getRetryLimits} from '@/config/index'; +import {computeToolCallSignature} from '@/hooks/chat-handler/utils/tool-signature'; import { appendSubagentTool, getSubagentProgress, @@ -387,6 +388,16 @@ export class SubagentExecutor { let streamingText = ''; let streamingReasoning = ''; + // Repeated-call cap (`nanocoder.retries.maxRepeatedToolCalls`): the same + // agent-loop guard the main runtimes apply. A subagent re-issuing the + // identical tool call(s) on consecutive turns is stuck; there is no user + // to ask inside a delegated run, so hitting the cap stops with an error. + // The signature covers every emitted call, so a subagent stuck on an + // unknown tool trips the cap too. + const {maxRepeatedToolCalls} = getRetryLimits(); + let lastToolSignature = ''; + let repeatedToolCallCount = 0; + if (agentId) { initSubagentSession(agentId, config.name, messages); } @@ -490,6 +501,20 @@ export class SubagentExecutor { return responseContent; } + const currentToolSignature = computeToolCallSignature(toolCalls); + const currentRepeatedCount = + currentToolSignature && currentToolSignature === lastToolSignature + ? repeatedToolCallCount + 1 + : 1; + if (currentRepeatedCount >= maxRepeatedToolCalls) { + emitProgress('error'); + throw new Error( + `Subagent repeated the same tool call ${currentRepeatedCount} times in a row without making progress — stopping to avoid a loop (nanocoder.retries.maxRepeatedToolCalls = ${maxRepeatedToolCalls}).`, + ); + } + lastToolSignature = currentToolSignature; + repeatedToolCallCount = currentRepeatedCount; + // Count tokens from tool call arguments for (const tc of toolCalls) { const argStr = diff --git a/source/vscode/chat-panel-harness.ts b/source/vscode/chat-panel-harness.ts index b4a43989b..a26917bba 100644 --- a/source/vscode/chat-panel-harness.ts +++ b/source/vscode/chat-panel-harness.ts @@ -43,9 +43,9 @@ const SHELL_IDS = [ 'send-stop-btn', ]; -// biome-ignore lint/suspicious/noExplicitAny: the panel assigns arbitrary -// properties (onclick, oninput, ...) to the nodes it builds, so the stub has to -// stay open-ended. +// The panel assigns arbitrary properties (onclick, oninput, ...) to the nodes +// it builds, so the stub has to stay open-ended. +// biome-ignore lint/suspicious/noExplicitAny: see above export type StubElement = any; /** From 08708a18888b3ef519e201671f5bfe3c2a1fe5d1 Mon Sep 17 00:00:00 2001 From: shoryabansalgithub Date: Wed, 19 Aug 2026 03:42:33 +0530 Subject: [PATCH 10/17] no-mistakes(review): deliver unknown-tool feedback and harden retry limits --- docs/configuration/index.md | 2 +- source/acp/acp-conversation.ts | 22 ++- source/config/index.spec.ts | 23 +++ source/config/index.ts | 23 +-- .../conversation/conversation-loop.spec.ts | 33 ++++ .../conversation/conversation-loop.tsx | 13 +- .../hooks/chat-handler/utils/tool-filters.ts | 16 +- source/plain/conversation.spec.ts | 165 ++++++++++++++++++ source/plain/conversation.ts | 38 +++- source/subagents/subagent-executor.spec.ts | 21 +++ source/subagents/subagent-executor.ts | 36 +++- source/tools/agent-tool.tsx | 11 +- 12 files changed, 377 insertions(+), 26 deletions(-) diff --git a/docs/configuration/index.md b/docs/configuration/index.md index 42cf536f0..ee2cadae2 100644 --- a/docs/configuration/index.md +++ b/docs/configuration/index.md @@ -193,7 +193,7 @@ Caps on how many times the conversation loop auto-retries a failing pattern with |--------|------|---------|-------------| | `maxRepeatedToolCalls` | number | `3` | Pause threshold for consecutive identical tool calls (minimum 2). The check fires when the same call (or set of calls) is emitted for the Nth consecutive turn, before that call runs - so the default of 3 executes the repeated call twice and pauses on the third emission. In an interactive session you are asked whether to continue - useful when the repetition is legitimate, such as polling a long-running job - or stop. `--plain` and other non-interactive runs stop with a clear error. Calls to unknown tools count toward the streak too, so a model stuck on a nonexistent tool hits the same cap. | | `maxEmptyTurns` | number | `2` | Consecutive empty assistant turns that are auto-nudged before giving up (minimum 0). The interactive loop additionally compacts the context and retries once before stopping; the `--plain` runtime stops directly after the nudges. | -| `maxMalformedRetries` | number | `2` | Malformed self-correction retries allowed for text-parsed tool calls before the loop gives up (minimum 0). Applies to the XML fallback path and to native-tool models that emit tool-call text instead of native tool calls. | +| `maxMalformedRetries` | number | `2` | Malformed self-correction retries allowed for text-parsed tool calls before the loop gives up (minimum 0). Applies to the XML fallback path in both runtimes. The interactive loop also parses tool-call text from native-tool models that emit it instead of native calls, so the cap covers that case there; the `--plain` runtime only parses text on the XML fallback path. | Choosing "Continue" at the repeated-tool-call prompt runs the paused call and re-checks after `maxRepeatedToolCalls` further identical calls, so a genuinely stuck model is re-prompted rather than left looping. diff --git a/source/acp/acp-conversation.ts b/source/acp/acp-conversation.ts index edc7ff3f2..3ba591c44 100644 --- a/source/acp/acp-conversation.ts +++ b/source/acp/acp-conversation.ts @@ -214,12 +214,14 @@ export async function runAcpConversation( const cleanedContent = xmlParse.cleanedContent; const validToolCalls: ToolCall[] = []; + const unknownToolCalls: ToolCall[] = []; const errorResults: ToolResult[] = []; for (const toolCall of allToolCalls) { if ( toolCall.function.name === '__xml_validation_error__' || !toolManager.hasTool(toolCall.function.name) ) { + unknownToolCalls.push(toolCall); errorResults.push({ tool_call_id: toolCall.id, role: 'tool', @@ -231,18 +233,34 @@ export async function runAcpConversation( validToolCalls.push(toolCall); } + // Unknown calls belong in the assistant message too: a tool result whose + // call is missing from history is orphaned and dropped before the next + // request, so the model would never see why its call failed and would + // keep re-emitting it. + const emittedToolCalls = [...validToolCalls, ...unknownToolCalls]; + messages = [ ...messages, { role: 'assistant', content: cleanedContent, - tool_calls: validToolCalls.length > 0 ? validToolCalls : undefined, + tool_calls: emittedToolCalls.length > 0 ? emittedToolCalls : undefined, reasoning: streamedReasoning || undefined, }, ]; if (errorResults.length > 0) { - messages = [...messages, ...errorResults]; + // The turn is abandoned for self-correction, so the valid calls never + // run, so pair each with a cancellation result to keep every tool_call + // above matched. + const abortedResults: ToolResult[] = validToolCalls.map(toolCall => ({ + tool_call_id: toolCall.id, + role: 'tool', + name: toolCall.function.name, + content: + 'Execution aborted because another tool call in this request was invalid. Please fix the invalid tool call and try again.', + })); + messages = [...messages, ...errorResults, ...abortedResults]; continue; } diff --git a/source/config/index.spec.ts b/source/config/index.spec.ts index 0d095203f..4a77ca1f9 100644 --- a/source/config/index.spec.ts +++ b/source/config/index.spec.ts @@ -711,6 +711,29 @@ test.serial('retry limits ignore non-numeric values', async t => { ); }); +test.serial('getRetryLimits falls back per field, not per object', async t => { + // A retries object missing a key would otherwise hand callers `undefined`, + // and every `count >= limit` guard reading it evaluates false, silently + // disabling the cap. + const { + getAppConfig, + getRetryLimits, + reloadAppConfig: reload, + } = await import('./index.js'); + const config = getAppConfig(); + const original = config.retries; + config.retries = {maxEmptyTurns: 5} as unknown as typeof original; + try { + const limits = getRetryLimits(); + t.is(limits.maxEmptyTurns, 5); + t.is(limits.maxRepeatedToolCalls, 3); + t.is(limits.maxMalformedRetries, 2); + } finally { + config.retries = original; + reload(); + } +}); + // Tests for modeProviders async function withModeProvidersConfig( testName: string, diff --git a/source/config/index.ts b/source/config/index.ts index 25925b27f..6034f4544 100644 --- a/source/config/index.ts +++ b/source/config/index.ts @@ -641,19 +641,22 @@ export function getAppConfig(): AppConfig { /** * Agent-loop retry limits, read live from the current app config so runtime - * edits (and tests that mutate `getAppConfig().retries`) are picked up. The - * hardcoded caps are the single fallback for configs loaded before `retries` - * existed. + * edits (and tests that mutate `getAppConfig().retries`) are picked up. + * + * The fallback is applied per field, not to the object as a whole: a `retries` + * object missing one key would otherwise hand callers `undefined`, and every + * `count >= limit` guard reading it evaluates false, silently disabling the + * very cap this feature exists to enforce. * @public */ export function getRetryLimits(): RetryLimitsConfig { - return ( - getAppConfig().retries ?? { - maxRepeatedToolCalls: MAX_REPEATED_TOOL_CALLS, - maxEmptyTurns: MAX_EMPTY_TURNS, - maxMalformedRetries: MAX_MALFORMED_RETRIES, - } - ); + const retries = getAppConfig().retries; + return { + maxRepeatedToolCalls: + retries?.maxRepeatedToolCalls ?? MAX_REPEATED_TOOL_CALLS, + maxEmptyTurns: retries?.maxEmptyTurns ?? MAX_EMPTY_TURNS, + maxMalformedRetries: retries?.maxMalformedRetries ?? MAX_MALFORMED_RETRIES, + }; } // Function to reload the app configuration (useful after config file changes) diff --git a/source/hooks/chat-handler/conversation/conversation-loop.spec.ts b/source/hooks/chat-handler/conversation/conversation-loop.spec.ts index 945945422..75478d6f6 100644 --- a/source/hooks/chat-handler/conversation/conversation-loop.spec.ts +++ b/source/hooks/chat-handler/conversation/conversation-loop.spec.ts @@ -1,4 +1,5 @@ import test from 'ava'; +import {dropOrphanedToolResults} from '@/ai-sdk-client/converters/message-converter.js'; import {clearAppConfig, getAppConfig} from '@/config/index.js'; import {resetShutdownManager} from '@/utils/shutdown/shutdown-manager.js'; import {processAssistantResponse, resetFallbackNotice, resetLastTurnHadReasoning} from './conversation-loop.js'; @@ -2335,6 +2336,38 @@ test.serial('repeated unknown-tool calls grant another window when the user cont t.regex(questionTexts[1], /repeated the same tool call 6 times in a row/); }); +test.serial('unknown-tool feedback is paired with its call so it reaches the model', async t => { + // The error result only reaches the model when its tool_call is in the + // assistant message: dropOrphanedToolResults strips results whose call is + // missing, leaving the next turn's context unchanged and the model repeating + // the same ghost call until the cap trips. + const messageSnapshots: Message[][] = []; + + setGlobalQuestionHandler(async question => question.options[0]); + + await withRetryLimits({maxRepeatedToolCalls: 3}, async () => { + const params = createDefaultParams({ + client: createUnknownToolClient(), + toolManager: repeatingToolManager(), + setMessages: (msgs: Message[]) => messageSnapshots.push(msgs), + }); + + await processAssistantResponse(params); + }); + + const latest = messageSnapshots[messageSnapshots.length - 1]; + const assistant = latest.find(m => m.role === 'assistant'); + t.true( + (assistant?.tool_calls ?? []).some(tc => tc.id === 'call_ghost'), + 'the ghost call must be in the assistant message', + ); + const delivered = dropOrphanedToolResults(latest); + t.true( + delivered.some(m => m.role === 'tool' && m.tool_call_id === 'call_ghost'), + 'the unknown-tool error must survive orphan pruning', + ); +}); + test.serial('repeated unknown-tool calls hard-stop without prompting in non-interactive mode', async t => { let chatCallCount = 0; let questionCount = 0; diff --git a/source/hooks/chat-handler/conversation/conversation-loop.tsx b/source/hooks/chat-handler/conversation/conversation-loop.tsx index 12f895636..4a1dc6f2f 100644 --- a/source/hooks/chat-handler/conversation/conversation-loop.tsx +++ b/source/hooks/chat-handler/conversation/conversation-loop.tsx @@ -520,22 +520,29 @@ export const processAssistantResponse = async ( ); } - const {validToolCalls, errorResults} = filterValidToolCalls( + const {validToolCalls, unknownToolCalls, errorResults} = filterValidToolCalls( allToolCalls, toolManager, ); + // The assistant message carries every call the model emitted, unknown tools + // included. Their error results below only reach the model when a matching + // tool_call is in history (dropOrphanedToolResults strips results with no + // preceding call), and without that feedback the model re-emits the same + // ghost call until the repeated-call cap stops the turn. + const emittedToolCalls = [...validToolCalls, ...unknownToolCalls]; + // Add assistant message to conversation history only if it has content or tool_calls // Empty assistant messages cause API errors: "Assistant message must have either content or tool_calls" const assistantMsg: Message = { role: 'assistant', content: cleanedContent, - tool_calls: validToolCalls.length > 0 ? validToolCalls : undefined, + tool_calls: emittedToolCalls.length > 0 ? emittedToolCalls : undefined, reasoning: fullReasoning, }; const hasValidAssistantMessage = - cleanedContent.trim() || validToolCalls.length > 0; + cleanedContent.trim() || emittedToolCalls.length > 0; // Build updated messages array using MessageBuilder const builder = new MessageBuilder(messages); diff --git a/source/hooks/chat-handler/utils/tool-filters.ts b/source/hooks/chat-handler/utils/tool-filters.ts index 4e5248c85..8ee8c9113 100644 --- a/source/hooks/chat-handler/utils/tool-filters.ts +++ b/source/hooks/chat-handler/utils/tool-filters.ts @@ -8,12 +8,23 @@ import type {ToolCall, ToolResult} from '@/types/core'; * Handles: * - Empty tool calls (missing id or function name) * - Tools that don't exist in the tool manager + * + * `unknownToolCalls` holds the calls that produced `errorResults`, paired 1:1 + * and in the same order. Callers must put them in the assistant message's + * tool_calls: a tool result whose call is absent from history is orphaned and + * dropped before it reaches the model, so the self-correction hint would never + * arrive and the model would just repeat the nonexistent call. */ export const filterValidToolCalls = ( toolCalls: ToolCall[], toolManager: ToolManager | null, -): {validToolCalls: ToolCall[]; errorResults: ToolResult[]} => { +): { + validToolCalls: ToolCall[]; + unknownToolCalls: ToolCall[]; + errorResults: ToolResult[]; +} => { const validToolCalls: ToolCall[] = []; + const unknownToolCalls: ToolCall[] = []; const errorResults: ToolResult[] = []; for (const toolCall of toolCalls) { @@ -36,6 +47,7 @@ export const filterValidToolCalls = ( available.length > 0 ? ` Available tools are: ${available.join(', ')}.` : ''; + unknownToolCalls.push(toolCall); errorResults.push({ tool_call_id: toolCall.id, role: 'tool' as const, @@ -48,5 +60,5 @@ export const filterValidToolCalls = ( validToolCalls.push(toolCall); } - return {validToolCalls, errorResults}; + return {validToolCalls, unknownToolCalls, errorResults}; }; diff --git a/source/plain/conversation.spec.ts b/source/plain/conversation.spec.ts index c6095746f..4db185fdd 100644 --- a/source/plain/conversation.spec.ts +++ b/source/plain/conversation.spec.ts @@ -1,4 +1,5 @@ import test from "ava"; +import { dropOrphanedToolResults } from "@/ai-sdk-client/converters/message-converter"; import { getAppConfig, reloadAppConfig } from "@/config/index"; import { setToolManagerGetter, setToolRegistryGetter } from "@/message-handler"; import type { ToolManager } from "@/tools/tool-manager"; @@ -1414,3 +1415,167 @@ test.serial( t.is(outcome.kind, "success"); }, ); + +test.serial( + "unknown-tool feedback reaches the model instead of being pruned as orphaned", + async (t) => { + // The error result is only delivered if its tool_call is in history: + // dropOrphanedToolResults strips any result whose call is missing, which + // would leave the next turn's context identical and the model repeating + // the same ghost call until the repeated-call cap trips. + const calls: RecordedCall[] = []; + const client = makeRecordingClient( + [ + unknownToolResponse(), + { choices: [{ message: { role: "assistant", content: "recovered" } }] }, + ], + calls, + ); + const toolManager = makeFakeToolManager(); + + const outcome = await runPlainConversation({ + client, + toolManager, + systemMessage: SYSTEM, + initialMessages: [USER], + developmentMode: "auto-accept", + nonInteractiveAlwaysAllow: [], + abortSignal: new AbortController().signal, + }); + + t.is(outcome.kind, "success"); + t.is(calls.length, 2); + + const retryMessages = calls[1].messages; + const assistant = retryMessages.find((m) => m.role === "assistant"); + t.true( + (assistant?.tool_calls ?? []).some((tc) => tc.id === "call-ghost"), + "the ghost call must be in the assistant message", + ); + const delivered = dropOrphanedToolResults(retryMessages); + t.true( + delivered.some( + (m) => m.role === "tool" && m.tool_call_id === "call-ghost", + ), + "the unknown-tool error must survive orphan pruning", + ); + }, +); + +test.serial( + "a turn mixing a valid and an unknown call keeps every tool_call paired", + async (t) => { + const calls: RecordedCall[] = []; + const client = makeRecordingClient( + [ + { + choices: [ + { + message: { + role: "assistant", + content: "", + tool_calls: [ + { + id: "call-good", + function: { name: "safe_tool", arguments: {} }, + }, + { + id: "call-ghost", + function: { name: "ghost_tool", arguments: {} }, + }, + ], + }, + }, + ], + }, + { choices: [{ message: { role: "assistant", content: "recovered" } }] }, + ], + calls, + ); + const toolManager = makeFakeToolManager({ + knownTools: new Set(["safe_tool"]), + needsApprovalByName: { safe_tool: false }, + }); + setToolRegistryGetter(() => ({ + safe_tool: (async () => "tool-output") as ToolHandler, + })); + + await runPlainConversation({ + client, + toolManager, + systemMessage: SYSTEM, + initialMessages: [USER], + developmentMode: "auto-accept", + nonInteractiveAlwaysAllow: [], + abortSignal: new AbortController().signal, + }); + + const retryMessages = calls[1].messages; + const emitted = retryMessages.flatMap((m) => m.tool_calls ?? []); + t.is(emitted.length, 2); + const resultIds = new Set( + retryMessages + .filter((m) => m.role === "tool") + .map((m) => m.tool_call_id), + ); + for (const toolCall of emitted) { + t.true( + resultIds.has(toolCall.id), + `tool call ${toolCall.id} must have a paired result`, + ); + } + }, +); + +test.serial( + "a malformed turn's streamed text is rolled back out of finalText", + async (t) => { + // The rejected turn's tokens land in the accumulator before the parse + // error is known; a later successful run must not ship them. + const responses = [ + { content: "[tool_use: safe_tool]", toolsDisabled: true }, + { content: "the real answer", toolsDisabled: true }, + ]; + let callIndex = 0; + const client = { + getCurrentModel: () => "fake-model", + setModel: () => undefined, + getContextSize: () => 100_000, + getAvailableModels: async () => ["fake-model"], + getProviderConfig: () => ({}) as never, + clearContext: async () => undefined, + getTimeout: () => undefined, + chat: async ( + _messages: Message[], + _tools: Record, + callbacks: { onToken?: (token: string) => void }, + ) => { + const response = responses[callIndex++]; + callbacks.onToken?.(response.content); + return { + choices: [ + { message: { role: "assistant", content: response.content } }, + ], + toolsDisabled: response.toolsDisabled, + } as LLMChatResponse; + }, + } as unknown as LLMClient; + const toolManager = makeFakeToolManager({ + knownTools: new Set(["safe_tool"]), + }); + + const outcome = await runPlainConversation({ + client, + toolManager, + systemMessage: SYSTEM, + initialMessages: [USER], + developmentMode: "auto-accept", + nonInteractiveAlwaysAllow: [], + abortSignal: new AbortController().signal, + outputFormat: "json", + }); + + t.is(outcome.kind, "success"); + t.is(outcome.finalText, "the real answer"); + }, +); diff --git a/source/plain/conversation.ts b/source/plain/conversation.ts index 6a7f7d59b..e762ec532 100644 --- a/source/plain/conversation.ts +++ b/source/plain/conversation.ts @@ -213,6 +213,14 @@ export async function runPlainConversation( let reasoningPrinted = false; let contentStarted = false; + // Streamed text lands in the accumulators as it arrives, before we know + // whether the turn is usable. A turn rejected as malformed is discarded + // and retried, so keep a pre-turn snapshot to roll back to. Otherwise + // the rejected tool-call blob stays glued to the front of the finalText + // a later successful turn returns (visible to --json consumers). + const finalTextBeforeTurn = accumulatedFinalText; + const reasoningBeforeTurn = accumulatedReasoning; + const sessionConfig = getAppConfig().sessions; const maxMessages = sessionConfig?.maxMessages ?? 1000; const cappedMessages = capMessagesForModel(messages, maxMessages); @@ -328,6 +336,8 @@ export async function runPlainConversation( emptyTurnCount = 0; lastToolSignature = ''; repeatedToolCallCount = 0; + accumulatedFinalText = finalTextBeforeTurn; + accumulatedReasoning = reasoningBeforeTurn; if (!isJson) { writeError( `Malformed tool call: ${xmlParse.error} — asking the model to retry (${malformedRetryCount}/${maxMalformedRetries}).`, @@ -351,6 +361,7 @@ export async function runPlainConversation( const cleanedContent = xmlParse.cleanedContent; const validToolCalls: ToolCall[] = []; + const unknownToolCalls: ToolCall[] = []; const errorResults: ToolResult[] = []; for (const toolCall of allToolCalls) { if ( @@ -358,6 +369,7 @@ export async function runPlainConversation( !toolManager.hasTool(toolCall.function.name) ) { const errorMsg = `Unknown tool: ${toolCall.function.name}`; + unknownToolCalls.push(toolCall); errorResults.push({ tool_call_id: toolCall.id, role: 'tool', @@ -376,20 +388,26 @@ export async function runPlainConversation( validToolCalls.push(toolCall); } + // The assistant message carries every call the model emitted, unknown + // tools included. Their error results only reach the model if a matching + // tool_call sits in history (`dropOrphanedToolResults` strips any result + // with no preceding call), and without that feedback the model re-emits + // the same ghost call until the repeated-call cap stops the run. + const emittedToolCalls = [...validToolCalls, ...unknownToolCalls]; + // Skip appending a fully-empty assistant message (no content, no tool // calls): providers reject them, and the empty-turn nudge below re-asks // without one — same rule the interactive loop applies. const hasAssistantPayload = - cleanedContent.trim() || - validToolCalls.length > 0 || - errorResults.length > 0; + cleanedContent.trim() || emittedToolCalls.length > 0; if (hasAssistantPayload) { messages = [ ...messages, { role: 'assistant', content: cleanedContent, - tool_calls: validToolCalls.length > 0 ? validToolCalls : undefined, + tool_calls: + emittedToolCalls.length > 0 ? emittedToolCalls : undefined, reasoning: streamedReasoning || undefined, }, ]; @@ -404,7 +422,17 @@ export async function runPlainConversation( if (stopped) { return stopped; } - messages = [...messages, ...errorResults]; + // The turn is abandoned for self-correction, so the valid calls never + // execute. Pair each with a cancellation result so every tool_call in + // the assistant message above still has a matching result. + const abortedResults: ToolResult[] = validToolCalls.map(toolCall => ({ + tool_call_id: toolCall.id, + role: 'tool', + name: toolCall.function.name, + content: + 'Execution aborted because another tool call in this request was invalid. Please fix the invalid tool call and try again.', + })); + messages = [...messages, ...errorResults, ...abortedResults]; continue; } diff --git a/source/subagents/subagent-executor.spec.ts b/source/subagents/subagent-executor.spec.ts index ddebdd798..f8dd7ea82 100644 --- a/source/subagents/subagent-executor.spec.ts +++ b/source/subagents/subagent-executor.spec.ts @@ -346,6 +346,27 @@ test.serial('repeated identical tool calls trip the retry cap', async t => { t.regex(result.error || '', /maxRepeatedToolCalls/); }); +test.serial('a tripped retry cap still returns the work done before the stop', async t => { + const toolManager = createMockToolManager({ + read_file: {handler: async () => 'same output', readOnly: true}, + }); + const client = createMockClient([ + {...repeatedCallResponse(), content: 'found the config file'}, + {...repeatedCallResponse(), content: 'it sets the timeout to 30s'}, + repeatedCallResponse(), + ]); + const executor = new SubagentExecutor(toolManager, client); + + const result = await executor.execute({ + subagent_type: 'explore', + description: 'Loop after doing useful work', + }); + + t.false(result.success); + t.regex(result.output, /found the config file/); + t.regex(result.output, /it sets the timeout to 30s/); +}); + test.serial('identical tool calls one under the retry cap complete normally', async t => { const toolManager = createMockToolManager({ read_file: {handler: async () => 'same output', readOnly: true}, diff --git a/source/subagents/subagent-executor.ts b/source/subagents/subagent-executor.ts index 643224b6a..d8d463750 100644 --- a/source/subagents/subagent-executor.ts +++ b/source/subagents/subagent-executor.ts @@ -49,6 +49,21 @@ const MAX_SUBAGENT_DEPTH = 2; /** Maximum number of concurrent subagents */ export const MAX_CONCURRENT_AGENTS = 5; +/** + * Thrown when the conversation loop stops itself (repeated-call cap). Carries + * the assistant text produced before the stop so the parent still receives the + * work the subagent did complete instead of an empty result. + */ +class SubagentLoopStopError extends Error { + readonly partialOutput: string; + + constructor(message: string, partialOutput: string) { + super(message); + this.name = 'SubagentLoopStopError'; + this.partialOutput = partialOutput; + } +} + /** * SubagentExecutor manages the execution of delegated tasks to subagents. * Each subagent runs in an isolated context with filtered tools. @@ -191,7 +206,11 @@ export class SubagentExecutor { } catch (error) { return { subagentName: task.subagent_type, - output: '', + // A loop stop still returns whatever the subagent produced before + // it got stuck, so the parent can use the partial work instead of + // being handed an empty result. + output: + error instanceof SubagentLoopStopError ? error.partialOutput : '', success: false, error: formatError(error), executionTimeMs: Date.now() - startTime, @@ -394,10 +413,19 @@ export class SubagentExecutor { // to ask inside a delegated run, so hitting the cap stops with an error. // The signature covers every emitted call, so a subagent stuck on an // unknown tool trips the cap too. + // + // Deliberately out of scope for #897: a hard turn ceiling (the plain and + // ACP `maxTurns` equivalent) and alternating-pattern detection, so a + // subagent cycling A, B, A, B... is still only bounded by the parent's + // abort signal. Revisit as its own change. const {maxRepeatedToolCalls} = getRetryLimits(); let lastToolSignature = ''; let repeatedToolCallCount = 0; + // Assistant text from every turn so far. The repeated-call stop hands + // this to the parent instead of discarding the work already done. + const assistantTranscript: string[] = []; + if (agentId) { initSubagentSession(agentId, config.name, messages); } @@ -494,6 +522,9 @@ export class SubagentExecutor { ); const responseContent = response.choices[0]?.message.content || ''; + if (responseContent.trim()) { + assistantTranscript.push(responseContent); + } const toolCalls = response.choices[0]?.message.tool_calls; if (!toolCalls || toolCalls.length === 0) { @@ -508,8 +539,9 @@ export class SubagentExecutor { : 1; if (currentRepeatedCount >= maxRepeatedToolCalls) { emitProgress('error'); - throw new Error( + throw new SubagentLoopStopError( `Subagent repeated the same tool call ${currentRepeatedCount} times in a row without making progress — stopping to avoid a loop (nanocoder.retries.maxRepeatedToolCalls = ${maxRepeatedToolCalls}).`, + assistantTranscript.join('\n\n'), ); } lastToolSignature = currentToolSignature; diff --git a/source/tools/agent-tool.tsx b/source/tools/agent-tool.tsx index 7395706c9..3a21bb597 100644 --- a/source/tools/agent-tool.tsx +++ b/source/tools/agent-tool.tsx @@ -142,7 +142,16 @@ async function executeAgent( ); if (!result.success) { - throw new Error(result.error || 'Subagent execution failed'); + const reason = result.error || 'Subagent execution failed'; + // A failed run can still carry work (e.g. a subagent stopped by the + // repeated-call cap after several useful turns). Hand it to the caller + // alongside the reason rather than throwing it away. + if (result.output.trim()) { + throw new Error( + `${reason}\n\nPartial output produced before stopping:\n${result.output}`, + ); + } + throw new Error(reason); } return result.output; From 2742ee32e66115031cd1ef93fdd14c3485693c2a Mon Sep 17 00:00:00 2001 From: shoryabansalgithub Date: Wed, 19 Aug 2026 04:06:43 +0530 Subject: [PATCH 11/17] no-mistakes(review): preserve subagent partial output and dedupe tool-call pairing --- .changeset/configurable-retry-limits.md | 2 +- source/acp/acp-conversation.ts | 45 ++-------- .../conversation/conversation-loop.tsx | 36 ++------ .../conversation/tool-executor.spec.ts | 89 ++++++++++++++++++ .../conversation/tool-executor.tsx | 20 ++++- .../chat-handler/utils/tool-filters.spec.ts | 81 ++++++++++++++++- .../hooks/chat-handler/utils/tool-filters.ts | 90 +++++++++++++++++-- source/plain/conversation.spec.ts | 2 +- source/plain/conversation.ts | 67 +++++--------- 9 files changed, 315 insertions(+), 117 deletions(-) diff --git a/.changeset/configurable-retry-limits.md b/.changeset/configurable-retry-limits.md index 8bddb584c..dfb7e2f4d 100644 --- a/.changeset/configurable-retry-limits.md +++ b/.changeset/configurable-retry-limits.md @@ -2,4 +2,4 @@ "@nanocollective/nanocoder": minor --- -Add configurable agent-loop retry limits to prevent token drain (#897). A new `nanocoder.retries` section in `agents.config.json` exposes the previously hardcoded caps: `maxRepeatedToolCalls` (default 3), `maxEmptyTurns` (default 2), and `maxMalformedRetries` (default 2). When the repeated-tool-call limit is hit in an interactive session, Nanocoder now pauses and asks whether to continue (granting another window of attempts) or stop, instead of always hard-stopping; non-interactive runs keep the hard stop. The same limits now also protect the `--plain` runtime used by `nanocoder run` in CI and non-TTY environments, which previously had no repeated-call, empty-turn, or malformed-retry caps at all: each cap hard-stops with a clear error there. Calls to unknown tools count toward the repeated-call streak in both runtimes, so a model stuck on a nonexistent tool trips the same cap instead of looping until the turn ceiling. Delegated subagent runs, whose loop previously had no cap at all, now apply `maxRepeatedToolCalls` too and stop with an error naming the setting. +Add configurable agent-loop retry limits to prevent token drain (#897). A new `nanocoder.retries` section in `agents.config.json` exposes the previously hardcoded caps: `maxRepeatedToolCalls` (default 3), `maxEmptyTurns` (default 2), and `maxMalformedRetries` (default 2). When the repeated-tool-call limit is hit in an interactive session, Nanocoder now pauses and asks whether to continue (granting another window of attempts) or stop, instead of always hard-stopping; non-interactive runs keep the hard stop. The same limits now also protect the `--plain` runtime used by `nanocoder run` in CI and non-TTY environments, which previously had no repeated-call cap at all: each cap hard-stops with a clear error there. Note this also loosens `--plain` in two places: it used to return an error on the *first* empty response and on the *first* malformed tool call, and it now nudges or asks the model to self-correct up to `maxEmptyTurns` / `maxMalformedRetries` before stopping, so a silent or malformed-output model costs up to 3 model calls instead of 1. Set either limit to `0` to restore the old fail-fast behaviour. Calls to unknown tools count toward the repeated-call streak in both runtimes, so a model stuck on a nonexistent tool trips the same cap instead of looping until the turn ceiling. Delegated subagent runs, whose loop previously had no cap at all, now apply `maxRepeatedToolCalls` too and stop with an error naming the setting. diff --git a/source/acp/acp-conversation.ts b/source/acp/acp-conversation.ts index 3ba591c44..d973902be 100644 --- a/source/acp/acp-conversation.ts +++ b/source/acp/acp-conversation.ts @@ -8,6 +8,10 @@ import {requestUserChoice} from '@/acp/acp-question'; import type {AcpSession} from '@/acp/acp-session'; import {type AcpToolCallMeta, buildToolCallMeta} from '@/acp/acp-tool-call'; import {DEFAULT_HEADLESS_MAX_TURNS, getAppConfig} from '@/config/index'; +import { + buildAbandonedTurnMessages, + partitionUnknownToolCalls, +} from '@/hooks/chat-handler/utils/tool-filters'; import {processToolUse} from '@/message-handler'; import { getAllSubagentProgress, @@ -213,31 +217,10 @@ export async function runAcpConversation( ]; const cleanedContent = xmlParse.cleanedContent; - const validToolCalls: ToolCall[] = []; - const unknownToolCalls: ToolCall[] = []; - const errorResults: ToolResult[] = []; - for (const toolCall of allToolCalls) { - if ( - toolCall.function.name === '__xml_validation_error__' || - !toolManager.hasTool(toolCall.function.name) - ) { - unknownToolCalls.push(toolCall); - errorResults.push({ - tool_call_id: toolCall.id, - role: 'tool', - name: toolCall.function.name, - content: `Unknown tool: ${toolCall.function.name}`, - }); - continue; - } - validToolCalls.push(toolCall); - } - - // Unknown calls belong in the assistant message too: a tool result whose - // call is missing from history is orphaned and dropped before the next - // request, so the model would never see why its call failed and would - // keep re-emitting it. - const emittedToolCalls = [...validToolCalls, ...unknownToolCalls]; + const partition = partitionUnknownToolCalls(allToolCalls, toolManager); + const {validToolCalls, errorResults} = partition; + const {emittedToolCalls, resultsForAbandonedTurn} = + buildAbandonedTurnMessages(partition); messages = [ ...messages, @@ -250,17 +233,7 @@ export async function runAcpConversation( ]; if (errorResults.length > 0) { - // The turn is abandoned for self-correction, so the valid calls never - // run, so pair each with a cancellation result to keep every tool_call - // above matched. - const abortedResults: ToolResult[] = validToolCalls.map(toolCall => ({ - tool_call_id: toolCall.id, - role: 'tool', - name: toolCall.function.name, - content: - 'Execution aborted because another tool call in this request was invalid. Please fix the invalid tool call and try again.', - })); - messages = [...messages, ...errorResults, ...abortedResults]; + messages = [...messages, ...resultsForAbandonedTurn]; continue; } diff --git a/source/hooks/chat-handler/conversation/conversation-loop.tsx b/source/hooks/chat-handler/conversation/conversation-loop.tsx index 4a1dc6f2f..6d24cd420 100644 --- a/source/hooks/chat-handler/conversation/conversation-loop.tsx +++ b/source/hooks/chat-handler/conversation/conversation-loop.tsx @@ -40,7 +40,10 @@ import {createCancellationResults} from '@/utils/tool-cancellation'; import {signalToolConfirm} from '@/utils/tool-confirm-queue'; import {displayCompactCountsSummary} from '@/utils/tool-result-display'; import {closeAllDiffsInVSCode} from '@/vscode/index'; -import {filterValidToolCalls} from '../utils/tool-filters'; +import { + buildAbandonedTurnMessages, + filterValidToolCalls, +} from '../utils/tool-filters'; import {computeToolCallSignature} from '../utils/tool-signature'; import {buildAutoDiagnosticsMessage} from './auto-diagnostics'; import { @@ -520,17 +523,10 @@ export const processAssistantResponse = async ( ); } - const {validToolCalls, unknownToolCalls, errorResults} = filterValidToolCalls( - allToolCalls, - toolManager, - ); - - // The assistant message carries every call the model emitted, unknown tools - // included. Their error results below only reach the model when a matching - // tool_call is in history (dropOrphanedToolResults strips results with no - // preceding call), and without that feedback the model re-emits the same - // ghost call until the repeated-call cap stops the turn. - const emittedToolCalls = [...validToolCalls, ...unknownToolCalls]; + const partition = filterValidToolCalls(allToolCalls, toolManager); + const {validToolCalls, errorResults} = partition; + const {emittedToolCalls, resultsForAbandonedTurn} = + buildAbandonedTurnMessages(partition); // Add assistant message to conversation history only if it has content or tool_calls // Empty assistant messages cause API errors: "Assistant message must have either content or tool_calls" @@ -740,23 +736,9 @@ export const processAssistantResponse = async ( ); } - // FIX: Satisfy the AI SDK's strict 1:1 Tool Call/Result mapping. - // If we are aborting this turn to self-correct the bad tools, - // we MUST provide a cancellation result for the valid tools we are skipping. - const abortedResults: ToolResult[] = validToolCalls.map(tc => ({ - tool_call_id: tc.id, - role: 'tool', - name: tc.function.name, - content: - 'Execution aborted because another tool call in this request was invalid. Please fix the invalid tool call and try again.', - })); - - // Combine the actual errors with the aborted placeholders - const allResultsForThisTurn = [...errorResults, ...abortedResults]; - // Send error results back to model for self-correction const errorBuilder = new MessageBuilder(updatedMessages); - errorBuilder.addToolResults(allResultsForThisTurn); + errorBuilder.addToolResults(resultsForAbandonedTurn); const updatedMessagesWithError = errorBuilder.build(); setMessages(updatedMessagesWithError); diff --git a/source/hooks/chat-handler/conversation/tool-executor.spec.ts b/source/hooks/chat-handler/conversation/tool-executor.spec.ts index b4fa636d4..bd01072a1 100644 --- a/source/hooks/chat-handler/conversation/tool-executor.spec.ts +++ b/source/hooks/chat-handler/conversation/tool-executor.spec.ts @@ -818,6 +818,95 @@ test.serial( }, ); +test.serial( + 'executeToolsDirectly - a failed subagent still hands its partial output to the parent', + async t => { + // A subagent stopped by the repeated-call cap returns the work it did + // before getting stuck. Collapsing that to "Error: ..." would leave the + // parent model with nothing to build on. + const {setAgentToolExecutor} = await import('@/tools/agent-tool'); + + setAgentToolExecutor({ + execute: async () => ({ + subagentName: 'fake', + output: 'found the config in src/app.ts', + success: false, + error: 'Subagent repeated the same tool call 3 times in a row', + executionTimeMs: 1, + }), + } as never); + + const toolCalls: ToolCall[] = [ + { + id: 'call_agent_3', + function: { + name: 'agent', + arguments: JSON.stringify({ + subagent_type: 'fake', + description: 'test', + }), + }, + }, + ]; + + const results = await executeToolsDirectly( + toolCalls, + createMockToolManager() as any, + createMockConversationStateManager() as any, + () => {}, + {compactDisplay: true}, + ); + + t.is(results.length, 1); + t.true( + results[0].content.startsWith('Error: '), + 'callers detect a failed agent result by the Error: prefix', + ); + t.true( + results[0].content.includes('found the config in src/app.ts'), + `expected the partial output to survive, got: ${results[0].content}`, + ); + }, +); + +test.serial( + 'executeToolsDirectly - a failed subagent with no output reports only the reason', + async t => { + const {setAgentToolExecutor} = await import('@/tools/agent-tool'); + + setAgentToolExecutor({ + execute: async () => ({ + subagentName: 'fake', + output: ' ', + success: false, + error: 'boom', + executionTimeMs: 1, + }), + } as never); + + const results = await executeToolsDirectly( + [ + { + id: 'call_agent_4', + function: { + name: 'agent', + arguments: JSON.stringify({ + subagent_type: 'fake', + description: 'test', + }), + }, + }, + ], + createMockToolManager() as any, + createMockConversationStateManager() as any, + () => {}, + {compactDisplay: true}, + ); + + t.is(results[0].content, 'Error: boom'); + }, +); + test('executeToolsDirectly - compact mode renders errors instead of counting them', async t => { const toolCalls: ToolCall[] = [ {id: 'call_1', function: {name: 'failing_tool', arguments: '{}'}}, diff --git a/source/hooks/chat-handler/conversation/tool-executor.tsx b/source/hooks/chat-handler/conversation/tool-executor.tsx index 84ea777df..e5c93c259 100644 --- a/source/hooks/chat-handler/conversation/tool-executor.tsx +++ b/source/hooks/chat-handler/conversation/tool-executor.tsx @@ -246,6 +246,24 @@ const groupForParallelExecution = ( return groups; }; +/** + * Renders a failed subagent run for the parent model. + * + * A failed run can still carry work — a subagent stopped by the repeated-call + * cap returns everything it produced before it got stuck. Hand that to the + * parent alongside the reason rather than throwing it away. The `Error: ` + * prefix is load-bearing: callers detect a failed agent result by it. + */ +const buildFailedAgentContent = (agentResult: { + content: string; + error?: string; +}): string => { + const reason = `Error: ${agentResult.error || 'Subagent execution failed'}`; + return agentResult.content.trim() + ? `${reason}\n\nPartial output produced before stopping:\n${agentResult.content}` + : reason; +}; + /** * Execute a batch of agent tool calls in parallel. * Returns tool results for all agents. @@ -370,7 +388,7 @@ const executeAgentBatch = async ( name: e.toolCall.function.name, content: agentResult.success ? agentResult.content - : `Error: ${agentResult.error || 'Subagent execution failed'}`, + : buildFailedAgentContent(agentResult), }; results.push({toolCall: e.toolCall, result}); diff --git a/source/hooks/chat-handler/utils/tool-filters.spec.ts b/source/hooks/chat-handler/utils/tool-filters.spec.ts index ff25e678e..5175a28a2 100644 --- a/source/hooks/chat-handler/utils/tool-filters.spec.ts +++ b/source/hooks/chat-handler/utils/tool-filters.spec.ts @@ -1,5 +1,9 @@ import test from 'ava'; -import {filterValidToolCalls} from './tool-filters.js'; +import { + buildAbandonedTurnMessages, + filterValidToolCalls, + partitionUnknownToolCalls, +} from './tool-filters.js'; import type {ToolCall} from '@/types/core'; import type {ToolManager} from '@/tools/tool-manager'; @@ -135,3 +139,78 @@ test('filterValidToolCalls - allows different tool calls', t => { t.is(validToolCalls.length, 3); }); + +// ============================================================================ +// partitionUnknownToolCalls / buildAbandonedTurnMessages +// ============================================================================ + +const managerWith = (knownTools: string[]) => + ({ + hasTool: (name: string) => knownTools.includes(name), + }) as unknown as ToolManager; + +test('partitionUnknownToolCalls - treats the XML validation marker as unknown', t => { + const {validToolCalls, unknownToolCalls, errorResults} = + partitionUnknownToolCalls( + [ + {id: 'call_1', function: {name: 'known_tool', arguments: {}}}, + {id: 'call_2', function: {name: '__xml_validation_error__', arguments: {}}}, + {id: 'call_3', function: {name: 'ghost_tool', arguments: {}}}, + ], + managerWith(['known_tool', '__xml_validation_error__']), + ); + + t.deepEqual( + validToolCalls.map(c => c.id), + ['call_1'], + ); + t.deepEqual( + unknownToolCalls.map(c => c.id), + ['call_2', 'call_3'], + ); + t.deepEqual( + errorResults.map(r => r.content), + ['Unknown tool: __xml_validation_error__', 'Unknown tool: ghost_tool'], + ); +}); + +test('buildAbandonedTurnMessages - every emitted call has a matching result', t => { + // The invariant: a result whose tool_call is missing from the assistant + // message is orphaned and pruned before the request goes out. + const partition = partitionUnknownToolCalls( + [ + {id: 'call_good', function: {name: 'known_tool', arguments: {}}}, + {id: 'call_ghost', function: {name: 'ghost_tool', arguments: {}}}, + ], + managerWith(['known_tool']), + ); + + const {emittedToolCalls, resultsForAbandonedTurn} = + buildAbandonedTurnMessages(partition); + + t.deepEqual( + emittedToolCalls.map(c => c.id), + ['call_good', 'call_ghost'], + ); + const resultIds = new Set(resultsForAbandonedTurn.map(r => r.tool_call_id)); + for (const toolCall of emittedToolCalls) { + t.true(resultIds.has(toolCall.id), `${toolCall.id} must be paired`); + } + const aborted = resultsForAbandonedTurn.find( + r => r.tool_call_id === 'call_good', + ); + t.regex(String(aborted?.content), /Execution aborted/); +}); + +test('buildAbandonedTurnMessages - a clean turn produces no abandoned results', t => { + const partition = partitionUnknownToolCalls( + [{id: 'call_good', function: {name: 'known_tool', arguments: {}}}], + managerWith(['known_tool']), + ); + + const {emittedToolCalls, resultsForAbandonedTurn} = + buildAbandonedTurnMessages(partition); + + t.is(emittedToolCalls.length, 1); + t.is(resultsForAbandonedTurn.length, 0); +}); diff --git a/source/hooks/chat-handler/utils/tool-filters.ts b/source/hooks/chat-handler/utils/tool-filters.ts index 8ee8c9113..00a491568 100644 --- a/source/hooks/chat-handler/utils/tool-filters.ts +++ b/source/hooks/chat-handler/utils/tool-filters.ts @@ -15,14 +15,16 @@ import type {ToolCall, ToolResult} from '@/types/core'; * dropped before it reaches the model, so the self-correction hint would never * arrive and the model would just repeat the nonexistent call. */ -export const filterValidToolCalls = ( - toolCalls: ToolCall[], - toolManager: ToolManager | null, -): { +export type PartitionedToolCalls = { validToolCalls: ToolCall[]; unknownToolCalls: ToolCall[]; errorResults: ToolResult[]; -} => { +}; + +export const filterValidToolCalls = ( + toolCalls: ToolCall[], + toolManager: ToolManager | null, +): PartitionedToolCalls => { const validToolCalls: ToolCall[] = []; const unknownToolCalls: ToolCall[] = []; const errorResults: ToolResult[] = []; @@ -62,3 +64,81 @@ export const filterValidToolCalls = ( return {validToolCalls, unknownToolCalls, errorResults}; }; + +/** Marker name the XML tool-call parser emits for a call it could not validate. */ +const XML_VALIDATION_ERROR_TOOL = '__xml_validation_error__'; + +/** + * Same partition as `filterValidToolCalls`, with the compact + * `Unknown tool: ` feedback the non-interactive loops send instead of + * the interactive loop's longer recovery hint. + */ +export const partitionUnknownToolCalls = ( + toolCalls: ToolCall[], + toolManager: ToolManager, +): PartitionedToolCalls => { + const validToolCalls: ToolCall[] = []; + const unknownToolCalls: ToolCall[] = []; + const errorResults: ToolResult[] = []; + + for (const toolCall of toolCalls) { + if ( + toolCall.function.name === XML_VALIDATION_ERROR_TOOL || + !toolManager.hasTool(toolCall.function.name) + ) { + unknownToolCalls.push(toolCall); + errorResults.push({ + tool_call_id: toolCall.id, + role: 'tool' as const, + name: toolCall.function.name, + content: `Unknown tool: ${toolCall.function.name}`, + isError: true, + }); + continue; + } + validToolCalls.push(toolCall); + } + + return {validToolCalls, unknownToolCalls, errorResults}; +}; + +/** + * Builds the message payload for a turn that carried an unknown tool call. + * + * Two pairing rules keep the provider's 1:1 tool-call/result mapping intact, + * and both are easy to break independently: + * + * - Unknown calls stay in the assistant message. A tool result whose call is + * missing from history is orphaned and dropped by `dropOrphanedToolResults` + * before the request goes out, so the self-correction hint would never reach + * the model and it would keep re-emitting the same ghost call. + * - The turn is abandoned for self-correction, so its valid calls never run. + * Each gets a cancellation result so no emitted call is left unmatched. + * + * `resultsForAbandonedTurn` is empty when nothing was unknown — that turn + * executes normally and produces its own results. + */ +export const buildAbandonedTurnMessages = ( + partition: PartitionedToolCalls, +): {emittedToolCalls: ToolCall[]; resultsForAbandonedTurn: ToolResult[]} => { + const {validToolCalls, unknownToolCalls, errorResults} = partition; + const emittedToolCalls = [...validToolCalls, ...unknownToolCalls]; + + if (errorResults.length === 0) { + return {emittedToolCalls, resultsForAbandonedTurn: []}; + } + + return { + emittedToolCalls, + resultsForAbandonedTurn: [ + ...errorResults, + ...validToolCalls.map(toolCall => ({ + tool_call_id: toolCall.id, + role: 'tool' as const, + name: toolCall.function.name, + content: + 'Execution aborted because another tool call in this request was invalid. Please fix the invalid tool call and try again.', + })), + ], + }; +}; diff --git a/source/plain/conversation.spec.ts b/source/plain/conversation.spec.ts index 4db185fdd..969318410 100644 --- a/source/plain/conversation.spec.ts +++ b/source/plain/conversation.spec.ts @@ -1330,7 +1330,7 @@ test.serial( t.is(outcome.kind, "error"); if (outcome.kind === "error") { - t.regex(outcome.message, /produced no output after 1 attempts/i); + t.regex(outcome.message, /produced no output after 1 attempt\b/i); } }); }, diff --git a/source/plain/conversation.ts b/source/plain/conversation.ts index e762ec532..794a99986 100644 --- a/source/plain/conversation.ts +++ b/source/plain/conversation.ts @@ -3,6 +3,10 @@ import { getAppConfig, getRetryLimits, } from '@/config/index'; +import { + buildAbandonedTurnMessages, + partitionUnknownToolCalls, +} from '@/hooks/chat-handler/utils/tool-filters'; import {computeToolCallSignature} from '@/hooks/chat-handler/utils/tool-signature'; import {processToolUse} from '@/message-handler'; import {color, write, writeError, writeLine, writeStatus} from '@/plain/writer'; @@ -360,40 +364,20 @@ export async function runPlainConversation( ]; const cleanedContent = xmlParse.cleanedContent; - const validToolCalls: ToolCall[] = []; - const unknownToolCalls: ToolCall[] = []; - const errorResults: ToolResult[] = []; - for (const toolCall of allToolCalls) { - if ( - toolCall.function.name === '__xml_validation_error__' || - !toolManager.hasTool(toolCall.function.name) - ) { - const errorMsg = `Unknown tool: ${toolCall.function.name}`; - unknownToolCalls.push(toolCall); - errorResults.push({ - tool_call_id: toolCall.id, - role: 'tool', - name: toolCall.function.name, - content: errorMsg, - isError: true, - }); - toolCallsLog.push({ - name: toolCall.function.name, - arguments: toolCall.function.arguments || {}, - result: null, - error: errorMsg, - }); - continue; - } - validToolCalls.push(toolCall); + const partition = partitionUnknownToolCalls(allToolCalls, toolManager); + const {validToolCalls, unknownToolCalls, errorResults} = partition; + // errorResults is paired 1:1 with unknownToolCalls, in the same order. + for (const [index, toolCall] of unknownToolCalls.entries()) { + toolCallsLog.push({ + name: toolCall.function.name, + arguments: toolCall.function.arguments || {}, + result: null, + error: errorResults[index].content, + }); } - // The assistant message carries every call the model emitted, unknown - // tools included. Their error results only reach the model if a matching - // tool_call sits in history (`dropOrphanedToolResults` strips any result - // with no preceding call), and without that feedback the model re-emits - // the same ghost call until the repeated-call cap stops the run. - const emittedToolCalls = [...validToolCalls, ...unknownToolCalls]; + const {emittedToolCalls, resultsForAbandonedTurn} = + buildAbandonedTurnMessages(partition); // Skip appending a fully-empty assistant message (no content, no tool // calls): providers reject them, and the empty-turn nudge below re-asks @@ -422,17 +406,7 @@ export async function runPlainConversation( if (stopped) { return stopped; } - // The turn is abandoned for self-correction, so the valid calls never - // execute. Pair each with a cancellation result so every tool_call in - // the assistant message above still has a matching result. - const abortedResults: ToolResult[] = validToolCalls.map(toolCall => ({ - tool_call_id: toolCall.id, - role: 'tool', - name: toolCall.function.name, - content: - 'Execution aborted because another tool call in this request was invalid. Please fix the invalid tool call and try again.', - })); - messages = [...messages, ...errorResults, ...abortedResults]; + messages = [...messages, ...resultsForAbandonedTurn]; continue; } @@ -441,7 +415,8 @@ export async function runPlainConversation( // Nudge through consecutive empty turns up to the cap, mirroring // the interactive loop, then stop so a silent model cannot spin. if (emptyTurnCount >= maxEmptyTurns) { - const message = `Model produced no output after ${maxEmptyTurns + 1} attempts — stopping (nanocoder.retries.maxEmptyTurns = ${maxEmptyTurns}).`; + const attempts = maxEmptyTurns + 1; + const message = `Model produced no output after ${attempts} attempt${attempts === 1 ? '' : 's'} — stopping (nanocoder.retries.maxEmptyTurns = ${maxEmptyTurns}).`; if (!isJson) { writeError(message); } @@ -459,8 +434,10 @@ export async function runPlainConversation( lastToolSignature = ''; repeatedToolCallCount = 0; if (!isJson) { + // Count attempts, not nudges, so the denominator matches the + // "no output after N attempts" stop message below. writeStatus( - `empty response — retry ${emptyTurnCount}/${maxEmptyTurns}`, + `empty response — retry ${emptyTurnCount}/${maxEmptyTurns + 1}`, ); } messages = [ From edea0ee8a84fae499151662196ea91ff35f498a0 Mon Sep 17 00:00:00 2001 From: shoryabansalgithub Date: Wed, 19 Aug 2026 04:45:32 +0530 Subject: [PATCH 12/17] no-mistakes(test): stop double-printing plain retry-limit messages --- source/plain/conversation.spec.ts | 51 +++++++++++++++++++++++++++++++ source/plain/conversation.ts | 14 +++------ 2 files changed, 56 insertions(+), 9 deletions(-) diff --git a/source/plain/conversation.spec.ts b/source/plain/conversation.spec.ts index 969318410..a3223ab95 100644 --- a/source/plain/conversation.spec.ts +++ b/source/plain/conversation.spec.ts @@ -1098,6 +1098,57 @@ test.serial( }, ); +test.serial( + "a retry-limit stop does not print its message itself", + async (t) => { + // The message travels back on the `error` outcome and the caller + // (runPlainShell) prints it once. Printing it here too double-printed it. + const client = makeRecordingClient( + [ + repeatingToolResponse(), + repeatingToolResponse(), + repeatingToolResponse(), + ], + [], + ); + const toolManager = makeFakeToolManager({ + knownTools: new Set(["safe_tool"]), + needsApprovalByName: { safe_tool: false }, + }); + setToolRegistryGetter(() => ({ + safe_tool: (async () => "tool-output") as ToolHandler, + })); + + const stderrChunks: string[] = []; + const originalWrite = process.stderr.write.bind(process.stderr); + process.stderr.write = ((chunk: string | Uint8Array) => { + stderrChunks.push(chunk.toString()); + return true; + }) as typeof process.stderr.write; + + let outcome: Awaited>; + try { + outcome = await runPlainConversation({ + client, + toolManager, + systemMessage: SYSTEM, + initialMessages: [USER], + developmentMode: "auto-accept", + nonInteractiveAlwaysAllow: [], + abortSignal: new AbortController().signal, + }); + } finally { + process.stderr.write = originalWrite; + } + + t.is(outcome.kind, "error"); + t.false( + stderrChunks.join("").includes("maxRepeatedToolCalls"), + "the stop message must be printed by the caller only", + ); + }, +); + test.serial( "identical tool calls one under the limit do not trip the cap", async (t) => { diff --git a/source/plain/conversation.ts b/source/plain/conversation.ts index 794a99986..ec7a734da 100644 --- a/source/plain/conversation.ts +++ b/source/plain/conversation.ts @@ -164,10 +164,10 @@ export async function runPlainConversation( ? repeatedToolCallCount + 1 : 1; if (currentRepeatedCount >= maxRepeatedToolCalls) { + // No writeError here: the caller prints every `error` outcome's + // message once (source/plain/shell.ts), and --json reports it in the + // report instead. const message = `Model repeated the same tool call ${currentRepeatedCount} times in a row without making progress — stopping to avoid a loop (nanocoder.retries.maxRepeatedToolCalls = ${maxRepeatedToolCalls}).`; - if (!isJson) { - writeError(message); - } return { kind: 'error', message, @@ -323,10 +323,8 @@ export async function runPlainConversation( // parse error back to the model, capped so a model stuck producing // bad tool calls cannot drain tokens unbounded. if (malformedRetryCount >= maxMalformedRetries) { + // The caller prints the `error` outcome message; see above. const message = `Model produced malformed tool calls ${maxMalformedRetries + 1} times in a row and cannot self-correct — stopping (nanocoder.retries.maxMalformedRetries = ${maxMalformedRetries}).`; - if (!isJson) { - writeError(message); - } return { kind: 'error', message, @@ -416,10 +414,8 @@ export async function runPlainConversation( // the interactive loop, then stop so a silent model cannot spin. if (emptyTurnCount >= maxEmptyTurns) { const attempts = maxEmptyTurns + 1; + // The caller prints the `error` outcome message; see above. const message = `Model produced no output after ${attempts} attempt${attempts === 1 ? '' : 's'} — stopping (nanocoder.retries.maxEmptyTurns = ${maxEmptyTurns}).`; - if (!isJson) { - writeError(message); - } return { kind: 'error', message, From cc6f2f82d342f47eed3801965e7939b5122fa198 Mon Sep 17 00:00:00 2001 From: shoryabansalgithub Date: Wed, 19 Aug 2026 04:53:54 +0530 Subject: [PATCH 13/17] no-mistakes(document): sync docs for subagent and plain-run retry limits --- docs/configuration/index.md | 2 ++ docs/features/commands.md | 2 +- docs/features/skills.md | 6 ++++++ docs/features/subagents.md | 8 ++++++++ source/types/config.ts | 18 ++++++++++++------ 5 files changed, 29 insertions(+), 7 deletions(-) diff --git a/docs/configuration/index.md b/docs/configuration/index.md index ee2cadae2..af4ddb5d8 100644 --- a/docs/configuration/index.md +++ b/docs/configuration/index.md @@ -197,6 +197,8 @@ Caps on how many times the conversation loop auto-retries a failing pattern with Choosing "Continue" at the repeated-tool-call prompt runs the paused call and re-checks after `maxRepeatedToolCalls` further identical calls, so a genuinely stuck model is re-prompted rather than left looping. +Setting `maxEmptyTurns` or `maxMalformedRetries` to `0` disables the nudge entirely, so the first empty or malformed turn ends the run - the fail-fast behaviour `--plain` had before these limits existed, worth setting when a silent or malformed-output model should cost one model call rather than three. The interactive loop still runs its single compact-and-retry after an empty turn even at `0`. + > **Warning - CI polling patterns:** in `--plain` runs (`nanocoder run "..."` in CI and non-TTY environments) there is no prompt to answer, so `maxRepeatedToolCalls` is a hard stop. A workflow whose model legitimately repeats the identical command - polling a deploy, re-running the same check while waiting on an external state change - aborts with exit code `1` once the cap is hit, by default on the third consecutive identical call. Raise `nanocoder.retries.maxRepeatedToolCalls` in that project's `agents.config.json` before relying on such a polling pattern. Unlike [Headless](#headless), these limits do not cover the ACP loop (`--acp`, used by editor clients), which is bounded by `maxTurns` alone. Delegated [subagent](../features/subagents.md) runs apply `maxRepeatedToolCalls` - a stuck subagent stops with an error naming the setting, since there is nobody to ask inside a delegated run - but not the other two limits: a subagent's loop ends on its own after an empty turn, and it does not use text-parsed tool calls. diff --git a/docs/features/commands.md b/docs/features/commands.md index ef77e2fcf..36f840cb3 100644 --- a/docs/features/commands.md +++ b/docs/features/commands.md @@ -100,7 +100,7 @@ nanocoder --mode yolo run "update README and push" If a tool requires approval that the active mode won't grant, nanocoder prints `Tool approval required for: ...` and exits with status code `1`. -Because there is nobody to answer a prompt in a `run`, the agent-loop [retry limits](../configuration/index.md#retry-limits) hard-stop instead of pausing: a model that repeats the same tool call, returns empty responses, or keeps emitting malformed tool calls past its configured cap ends the run with an error naming the setting. Under the `--plain` runtime (used automatically in CI and non-TTY environments) that exits with status code `1`. +Because there is nobody to answer a prompt in a `run`, the agent-loop [retry limits](../configuration/index.md#retry-limits) hard-stop instead of pausing: a model that repeats the same tool call, returns empty responses, or keeps emitting malformed tool calls past its configured cap ends the run with an error. Under the `--plain` runtime (used automatically in CI and non-TTY environments) the error names the limit that fired and the run exits with status code `1`. > **Warning - CI polling patterns:** the repeated-call hard stop triggers on *legitimate* repetition too. If your workflow's model is expected to run the identical command repeatedly - polling a deploy, waiting on a slow job by re-running the same check - the run aborts once `maxRepeatedToolCalls` consecutive identical calls are emitted (default 3). Raise `nanocoder.retries.maxRepeatedToolCalls` in that project's `agents.config.json` before relying on such a pattern in CI. diff --git a/docs/features/skills.md b/docs/features/skills.md index ab3386315..5fe52a2bd 100644 --- a/docs/features/skills.md +++ b/docs/features/skills.md @@ -198,6 +198,12 @@ mode (no foreground prompts, no `ask_user`, no `agent`). The `confirm: true` opt-in below switches a specific subscription to plan mode instead. +Triggered runs are subagent runs, so the +[`maxRepeatedToolCalls`](../configuration/index.md#retry-limits) cap +applies: a triggered skill whose model gets stuck repeating the same +tool call stops with an error instead of burning tokens unattended (see +[Loop Protection](./subagents.md#loop-protection)). + ## Inspecting and creating skills ``` diff --git a/docs/features/subagents.md b/docs/features/subagents.md index 61503a754..1b79573bd 100644 --- a/docs/features/subagents.md +++ b/docs/features/subagents.md @@ -143,6 +143,14 @@ A project-level agent with the same `name` as a built-in or user-level agent ove - The `tools` key in the agent definition controls which tools the subagent can access. Use this to restrict subagents to only the tools they need. - The `alwaysAllow` setting in `agents.config.json` applies to tools within subagents, so you can configure which tools run without prompts. +## Loop Protection + +A subagent that re-issues the identical tool call(s) on consecutive turns is stopped by the same [`maxRepeatedToolCalls`](../configuration/index.md#retry-limits) cap the main agent uses (default 3). There is nobody to ask inside a delegated run, so it never pauses: the run fails with an error naming the setting. Calls to nonexistent tools count toward the streak too, so a subagent stuck on a tool it doesn't have hits the same cap. + +Whatever the subagent produced before it got stuck is still handed to the main agent under a `Partial output produced before stopping:` heading, so useful work isn't discarded along with the failure. + +The other two retry limits don't apply to subagents: their loop ends on its own when a turn comes back with no tool calls (so `maxEmptyTurns` is moot), and they don't use text-parsed tool calls (so `maxMalformedRetries` is too). There is also no turn ceiling: apart from the repeated-call cap, a subagent runs until it stops calling tools or the main agent's run is cancelled. + ## Development Modes and Tune Profiles ### Plan Mode diff --git a/source/types/config.ts b/source/types/config.ts index 599c71f99..4652cc211 100644 --- a/source/types/config.ts +++ b/source/types/config.ts @@ -102,15 +102,21 @@ export interface PasteConfig { // auto-retries a failing pattern without user intervention. Distinct from the // per-provider `maxRetries` setting, which governs network request retries. export interface RetryLimitsConfig { - // Consecutive identical tool calls allowed before the loop pauses and asks - // the user whether to continue (interactive) or stops (non-interactive). + // Consecutive turns emitting the identical tool call(s) before the loop + // pauses and asks the user whether to continue (interactive) or stops + // (--plain, headless, subagent runs). The check fires before the Nth + // repeat runs, so the default of 3 executes it twice. Unknown-tool calls + // count toward the streak too. maxRepeatedToolCalls: number; - // Consecutive empty assistant turns auto-nudged before compact-and-retry - // kicks in and the loop gives up. + // Consecutive empty assistant turns auto-nudged before the loop gives up. + // The interactive loop additionally compacts the context and retries once; + // --plain stops straight after the nudges. Not used by subagent runs, + // whose loop ends on its own when a turn has no tool calls. maxEmptyTurns: number; // Malformed self-correction retries allowed for text-parsed tool calls - // before the loop gives up. Covers the XML fallback path and native - // responses that emit tool-call text instead of native tool calls. + // before the loop gives up. Covers the XML fallback path in both runtimes, + // plus (interactive only) native responses that emit tool-call text + // instead of native tool calls. Not used by subagent runs. maxMalformedRetries: number; } From 66aca0bfca0a8b4373c6ed52bfe22fe0ad51b41c Mon Sep 17 00:00:00 2001 From: shoryabansalgithub Date: Wed, 19 Aug 2026 18:57:37 +0530 Subject: [PATCH 14/17] fix(vscode): load mention-utils.js in the chat-panel test harness chat-panel.js reads globalThis.NanocoderMentionUtils at boot since the @-mention autocomplete landed, and the real webview loads mention-utils.js first (see chat-panel.html). The VM harness only evaluated chat-panel.js, so every chat-panel spec failed with an undefined-global TypeError. --- source/vscode/chat-panel-harness.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/source/vscode/chat-panel-harness.ts b/source/vscode/chat-panel-harness.ts index a26917bba..9d7f79a58 100644 --- a/source/vscode/chat-panel-harness.ts +++ b/source/vscode/chat-panel-harness.ts @@ -14,6 +14,15 @@ const PANEL_SOURCE = readFileSync( 'utf8', ); +// The real webview loads mention-utils.js before chat-panel.js (see +// chat-panel.html); the panel reads globalThis.NanocoderMentionUtils at boot. +const MENTION_UTILS_SOURCE = readFileSync( + fileURLToPath( + new URL('../../plugins/vscode/media/mention-utils.js', import.meta.url), + ), + 'utf8', +); + const SHELL_IDS = [ 'add-image-btn', 'attach-btn', @@ -237,6 +246,7 @@ export function createPanel(options: {marked?: boolean} = {}) { } createContext(sandbox); + runInContext(MENTION_UTILS_SOURCE, sandbox); runInContext(PANEL_SOURCE, sandbox); const container = findById(root, 'messages-container') as StubElement; From 9ad805e6d9c6c462e855bc23157c7f0cfa3fbd91 Mon Sep 17 00:00:00 2001 From: shoryabansalgithub Date: Wed, 19 Aug 2026 19:05:47 +0530 Subject: [PATCH 15/17] fix(tool-calls): pair unconfirmed tools with cancellation results on the non-interactive approval exit The non-interactive approval exit saved history where the assistant message announced confirmTools' tool_calls but none of them received a tool result. A later session resume replays that history, and strict OpenAI-compatible providers reject announced tool_calls without results. Pair each unconfirmed tool with a cancellation result, the same pattern the interactive decline path already uses. --- .../conversation/conversation-loop.spec.ts | 49 +++++++++++++++++++ .../conversation/conversation-loop.tsx | 8 ++- 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/source/hooks/chat-handler/conversation/conversation-loop.spec.ts b/source/hooks/chat-handler/conversation/conversation-loop.spec.ts index 75478d6f6..baceb92aa 100644 --- a/source/hooks/chat-handler/conversation/conversation-loop.spec.ts +++ b/source/hooks/chat-handler/conversation/conversation-loop.spec.ts @@ -2368,6 +2368,55 @@ test.serial('unknown-tool feedback is paired with its call so it reaches the mod ); }); +test.serial('non-interactive approval exit pairs unconfirmed tools with cancellation results', async t => { + const messageSnapshots: Message[][] = []; + + const params = createDefaultParams({ + client: { + chat: async (): Promise => ({ + choices: [ + { + message: { + role: 'assistant', + content: '', + tool_calls: [ + { + id: 'call_guarded', + function: {name: 'guarded_tool', arguments: '{}'}, + }, + ], + }, + }, + ], + toolsDisabled: false, + }), + }, + toolManager: createMockToolManager({ + tools: ['guarded_tool'], + needsApproval: true, + }), + nonInteractiveMode: true, + setMessages: (msgs: Message[]) => messageSnapshots.push(msgs), + }); + + await processAssistantResponse(params); + + // The exit is terminal, so the saved history must pair the announced call + // with a result - a later resume would otherwise replay an assistant + // tool_call that never received one, which strict providers reject. + const latest = messageSnapshots[messageSnapshots.length - 1]; + const assistant = latest.find(m => m.role === 'assistant' && m.tool_calls); + t.true( + (assistant?.tool_calls ?? []).some(tc => tc.id === 'call_guarded'), + 'the guarded call must be announced in the assistant message', + ); + const result = latest.find( + m => m.role === 'tool' && m.tool_call_id === 'call_guarded', + ); + t.truthy(result, 'the unconfirmed tool must get a cancellation result'); + t.regex(String(result?.content), /cancelled/i); +}); + test.serial('repeated unknown-tool calls hard-stop without prompting in non-interactive mode', async t => { let chatCallCount = 0; let questionCount = 0; diff --git a/source/hooks/chat-handler/conversation/conversation-loop.tsx b/source/hooks/chat-handler/conversation/conversation-loop.tsx index 6d24cd420..316065cd9 100644 --- a/source/hooks/chat-handler/conversation/conversation-loop.tsx +++ b/source/hooks/chat-handler/conversation/conversation-loop.tsx @@ -876,7 +876,13 @@ export const processAssistantResponse = async ( />, ); const builder = new MessageBuilder(updatedMessages); - builder.addToolResults(turnResults); + // The assistant message already announces confirmTools' tool_calls; + // pair each with a cancellation result so the saved history keeps the + // provider-required 1:1 call/result mapping. + builder.addToolResults([ + ...turnResults, + ...createCancellationResults(confirmTools), + ]); builder.addMessage({role: 'assistant', content: errorMsg}); setMessages(builder.build()); setIsGenerating(false); From 91486d0bf1d6a5c7e43a59abf5a0413b47a467c5 Mon Sep 17 00:00:00 2001 From: shoryabansalgithub Date: Wed, 19 Aug 2026 19:42:56 +0530 Subject: [PATCH 16/17] no-mistakes(review): distinguish approval-unavailable results, share subagent failure text --- .../conversation/conversation-loop.spec.ts | 7 ++-- .../conversation/conversation-loop.tsx | 9 ++++-- .../conversation/tool-executor.tsx | 15 +++------ source/subagents/failure-message.spec.ts | 19 +++++++++++ source/subagents/failure-message.ts | 22 +++++++++++++ source/tools/agent-tool.tsx | 12 ++----- source/utils/tool-cancellation.spec.ts | 32 ++++++++++++++++++- source/utils/tool-cancellation.ts | 32 ++++++++++++++++++- 8 files changed, 121 insertions(+), 27 deletions(-) create mode 100644 source/subagents/failure-message.spec.ts create mode 100644 source/subagents/failure-message.ts diff --git a/source/hooks/chat-handler/conversation/conversation-loop.spec.ts b/source/hooks/chat-handler/conversation/conversation-loop.spec.ts index baceb92aa..5bb4fbb43 100644 --- a/source/hooks/chat-handler/conversation/conversation-loop.spec.ts +++ b/source/hooks/chat-handler/conversation/conversation-loop.spec.ts @@ -2413,8 +2413,11 @@ test.serial('non-interactive approval exit pairs unconfirmed tools with cancella const result = latest.find( m => m.role === 'tool' && m.tool_call_id === 'call_guarded', ); - t.truthy(result, 'the unconfirmed tool must get a cancellation result'); - t.regex(String(result?.content), /cancelled/i); + t.truthy(result, 'the unconfirmed tool must get a paired result'); + // Not the user-cancellation wording: nobody declined this tool, and a + // resumed interactive session must not read the history as a refusal. + t.regex(String(result?.content), /approval unavailable in non-interactive/i); + t.notRegex(String(result?.content), /cancelled by the user/i); }); test.serial('repeated unknown-tool calls hard-stop without prompting in non-interactive mode', async t => { diff --git a/source/hooks/chat-handler/conversation/conversation-loop.tsx b/source/hooks/chat-handler/conversation/conversation-loop.tsx index 316065cd9..b7e0e63d5 100644 --- a/source/hooks/chat-handler/conversation/conversation-loop.tsx +++ b/source/hooks/chat-handler/conversation/conversation-loop.tsx @@ -36,7 +36,10 @@ import {infoMsg} from '@/utils/message-factory'; import {getLastBuiltPrompt} from '@/utils/prompt-builder'; import {signalQuestion} from '@/utils/question-queue'; import {calculateTokens} from '@/utils/token-calculator'; -import {createCancellationResults} from '@/utils/tool-cancellation'; +import { + createApprovalUnavailableResults, + createCancellationResults, +} from '@/utils/tool-cancellation'; import {signalToolConfirm} from '@/utils/tool-confirm-queue'; import {displayCompactCountsSummary} from '@/utils/tool-result-display'; import {closeAllDiffsInVSCode} from '@/vscode/index'; @@ -877,11 +880,11 @@ export const processAssistantResponse = async ( ); const builder = new MessageBuilder(updatedMessages); // The assistant message already announces confirmTools' tool_calls; - // pair each with a cancellation result so the saved history keeps the + // pair each with a result so the saved history keeps the // provider-required 1:1 call/result mapping. builder.addToolResults([ ...turnResults, - ...createCancellationResults(confirmTools), + ...createApprovalUnavailableResults(confirmTools), ]); builder.addMessage({role: 'assistant', content: errorMsg}); setMessages(builder.build()); diff --git a/source/hooks/chat-handler/conversation/tool-executor.tsx b/source/hooks/chat-handler/conversation/tool-executor.tsx index e5c93c259..6558ff88d 100644 --- a/source/hooks/chat-handler/conversation/tool-executor.tsx +++ b/source/hooks/chat-handler/conversation/tool-executor.tsx @@ -10,6 +10,7 @@ import { resetSubagentProgressById, } from '@/services/subagent-events'; import {generateKey} from '@/session/key-generator'; +import {buildSubagentFailureMessage} from '@/subagents/failure-message'; import {MAX_CONCURRENT_AGENTS} from '@/subagents/subagent-executor'; import type {AgentToolArgs} from '@/tools/agent-tool'; import {startAgentExecution} from '@/tools/agent-tool'; @@ -249,20 +250,14 @@ const groupForParallelExecution = ( /** * Renders a failed subagent run for the parent model. * - * A failed run can still carry work — a subagent stopped by the repeated-call - * cap returns everything it produced before it got stuck. Hand that to the - * parent alongside the reason rather than throwing it away. The `Error: ` - * prefix is load-bearing: callers detect a failed agent result by it. + * The `Error: ` prefix is load-bearing: callers detect a failed agent result + * by it. The rest is shared with the native agent tool's failure path. */ const buildFailedAgentContent = (agentResult: { content: string; error?: string; -}): string => { - const reason = `Error: ${agentResult.error || 'Subagent execution failed'}`; - return agentResult.content.trim() - ? `${reason}\n\nPartial output produced before stopping:\n${agentResult.content}` - : reason; -}; +}): string => + `Error: ${buildSubagentFailureMessage(agentResult.error, agentResult.content)}`; /** * Execute a batch of agent tool calls in parallel. diff --git a/source/subagents/failure-message.spec.ts b/source/subagents/failure-message.spec.ts new file mode 100644 index 000000000..968860032 --- /dev/null +++ b/source/subagents/failure-message.spec.ts @@ -0,0 +1,19 @@ +import test from 'ava'; +import {buildSubagentFailureMessage} from './failure-message'; + +console.log(`\nfailure-message.spec.ts`); + +test('buildSubagentFailureMessage - appends partial output when the run produced some', t => { + t.is( + buildSubagentFailureMessage('Repeated tool call limit reached', 'half done'), + 'Repeated tool call limit reached\n\nPartial output produced before stopping:\nhalf done', + ); +}); + +test('buildSubagentFailureMessage - returns only the reason when output is blank', t => { + t.is(buildSubagentFailureMessage('boom', ' \n'), 'boom'); +}); + +test('buildSubagentFailureMessage - falls back to a generic reason', t => { + t.is(buildSubagentFailureMessage(undefined, ''), 'Subagent execution failed'); +}); diff --git a/source/subagents/failure-message.ts b/source/subagents/failure-message.ts new file mode 100644 index 000000000..36780f47e --- /dev/null +++ b/source/subagents/failure-message.ts @@ -0,0 +1,22 @@ +/** + * Shared rendering for a failed subagent run. + * + * A failed run can still carry work — a subagent stopped by the repeated-call + * cap returns everything it produced before it got stuck. Hand that to the + * caller alongside the reason rather than throwing it away. + * + * Both execution paths (the native agent tool and the conversation loop's agent + * batch) render this text, so it lives here to keep them from drifting. + * + * @param error - Reason the run failed, if the executor reported one + * @param output - Whatever the subagent produced before stopping + */ +export function buildSubagentFailureMessage( + error: string | undefined, + output: string, +): string { + const reason = error || 'Subagent execution failed'; + return output.trim() + ? `${reason}\n\nPartial output produced before stopping:\n${output}` + : reason; +} diff --git a/source/tools/agent-tool.tsx b/source/tools/agent-tool.tsx index 3a21bb597..c0830200e 100644 --- a/source/tools/agent-tool.tsx +++ b/source/tools/agent-tool.tsx @@ -7,6 +7,7 @@ */ import {randomUUID} from 'node:crypto'; +import {buildSubagentFailureMessage} from '@/subagents/failure-message.js'; import type {SubagentExecutor} from '@/subagents/subagent-executor.js'; import {getSubagentLoader} from '@/subagents/subagent-loader.js'; import {jsonSchema, tool} from '@/types/core'; @@ -142,16 +143,7 @@ async function executeAgent( ); if (!result.success) { - const reason = result.error || 'Subagent execution failed'; - // A failed run can still carry work (e.g. a subagent stopped by the - // repeated-call cap after several useful turns). Hand it to the caller - // alongside the reason rather than throwing it away. - if (result.output.trim()) { - throw new Error( - `${reason}\n\nPartial output produced before stopping:\n${result.output}`, - ); - } - throw new Error(reason); + throw new Error(buildSubagentFailureMessage(result.error, result.output)); } return result.output; diff --git a/source/utils/tool-cancellation.spec.ts b/source/utils/tool-cancellation.spec.ts index 46fd50c34..9ab94bb1a 100644 --- a/source/utils/tool-cancellation.spec.ts +++ b/source/utils/tool-cancellation.spec.ts @@ -1,6 +1,9 @@ import type {ToolCall} from '@/types/index'; import test from 'ava'; -import {createCancellationResults} from './tool-cancellation'; +import { + createApprovalUnavailableResults, + createCancellationResults, +} from './tool-cancellation'; console.log(`\ntool-cancellation.spec.ts`); @@ -228,3 +231,30 @@ test('createCancellationResults - realistic multi-tool cancellation', t => { t.is(results[2].name, 'ExecuteBash'); t.is(results[3].name, 'WriteFile'); }); + +test('createApprovalUnavailableResults - pairs each call with a non-cancellation reason', t => { + const toolCalls = [ + createMockToolCall('call-1', 'ExecuteBash', {command: 'ls'}), + createMockToolCall('call-2', 'WriteFile', {path: '/tmp/a'}), + ]; + const results = createApprovalUnavailableResults(toolCalls); + + t.is(results.length, 2); + results.forEach((result, index) => { + t.is(result.tool_call_id, toolCalls[index].id); + t.is(result.role, 'tool'); + t.is(result.name, toolCalls[index].function.name); + t.is( + result.content, + 'Tool was not executed: approval unavailable in non-interactive mode.', + ); + }); +}); + +test('createApprovalUnavailableResults - never claims the user cancelled', t => { + const results = createApprovalUnavailableResults([ + createMockToolCall('call-1', 'ExecuteBash'), + ]); + + t.false(results[0].content.includes('cancelled by the user')); +}); diff --git a/source/utils/tool-cancellation.ts b/source/utils/tool-cancellation.ts index e692c7dea..0a98be5a2 100644 --- a/source/utils/tool-cancellation.ts +++ b/source/utils/tool-cancellation.ts @@ -19,10 +19,40 @@ import type {ToolCall, ToolResult} from '@/types/index'; * })); */ export function createCancellationResults(toolCalls: ToolCall[]): ToolResult[] { + return createUnexecutedResults( + toolCalls, + 'Tool execution was cancelled by the user.', + ); +} + +/** + * Create results for tool calls that could not be approved because the run has + * no one to ask (non-interactive mode). + * + * Deliberately distinct from the cancellation wording: nobody declined these + * tools, so a resumed interactive session must not read the saved history as a + * user refusal and stop retrying them. + * + * @param toolCalls - Array of tool calls left unapproved + * @returns Array of tool results explaining why they did not run + */ +export function createApprovalUnavailableResults( + toolCalls: ToolCall[], +): ToolResult[] { + return createUnexecutedResults( + toolCalls, + 'Tool was not executed: approval unavailable in non-interactive mode.', + ); +} + +function createUnexecutedResults( + toolCalls: ToolCall[], + content: string, +): ToolResult[] { return toolCalls.map(toolCall => ({ tool_call_id: toolCall.id, role: 'tool' as const, name: toolCall.function.name, - content: 'Tool execution was cancelled by the user.', + content, })); } From ae3c90cf7f296a4c89d2ebedaa0bbcdcd0640575 Mon Sep 17 00:00:00 2001 From: shoryabansalgithub Date: Wed, 19 Aug 2026 19:51:27 +0530 Subject: [PATCH 17/17] no-mistakes(review): pair skipped tools on confirm-loop abort break --- .../conversation/conversation-loop.spec.ts | 78 +++++++++++++++++++ .../conversation/conversation-loop.tsx | 11 ++- 2 files changed, 87 insertions(+), 2 deletions(-) diff --git a/source/hooks/chat-handler/conversation/conversation-loop.spec.ts b/source/hooks/chat-handler/conversation/conversation-loop.spec.ts index 5bb4fbb43..1a810de7e 100644 --- a/source/hooks/chat-handler/conversation/conversation-loop.spec.ts +++ b/source/hooks/chat-handler/conversation/conversation-loop.spec.ts @@ -2420,6 +2420,84 @@ test.serial('non-interactive approval exit pairs unconfirmed tools with cancella t.notRegex(String(result?.content), /cancelled by the user/i); }); +test.serial('escape mid-execution pairs the tools the confirm loop never reached', async t => { + const messageSnapshots: Message[][] = []; + const controller = new AbortController(); + + // Approve the first tool, then abort before it finishes so the loop breaks + // with the second tool still pending. + setGlobalToolConfirmHandler(async () => { + controller.abort(); + return true; + }); + + let chatCallCount = 0; + const params = createDefaultParams({ + client: { + chat: async (): Promise => { + chatCallCount += 1; + if (chatCallCount > 1) { + return { + choices: [{message: {role: 'assistant', content: 'Done.'}}], + toolsDisabled: false, + }; + } + return { + choices: [ + { + message: { + role: 'assistant', + content: '', + tool_calls: [ + { + id: 'call_first', + function: {name: 'guarded_tool', arguments: '{}'}, + }, + { + id: 'call_second', + function: {name: 'guarded_tool', arguments: '{}'}, + }, + ], + }, + }, + ], + toolsDisabled: false, + }; + }, + }, + toolManager: createMockToolManager({ + tools: ['guarded_tool'], + needsApproval: true, + }), + abortController: controller, + setMessages: (msgs: Message[]) => messageSnapshots.push(msgs), + }); + + await processAssistantResponse(params); + + setGlobalToolConfirmHandler(async () => false); + + // The assistant message announces both calls, so the saved history must + // carry a result for the tool the abort skipped too. + const latest = messageSnapshots[messageSnapshots.length - 1]; + const assistant = latest.find(m => m.role === 'assistant' && m.tool_calls); + t.deepEqual( + (assistant?.tool_calls ?? []).map(tc => tc.id), + ['call_first', 'call_second'], + 'both calls must be announced in the assistant message', + ); + for (const id of ['call_first', 'call_second']) { + t.truthy( + latest.find(m => m.role === 'tool' && m.tool_call_id === id), + `${id} must receive a paired tool result`, + ); + } + const skipped = latest.find( + m => m.role === 'tool' && m.tool_call_id === 'call_second', + ); + t.regex(String(skipped?.content), /cancelled by the user/i); +}); + test.serial('repeated unknown-tool calls hard-stop without prompting in non-interactive mode', async t => { let chatCallCount = 0; let questionCount = 0; diff --git a/source/hooks/chat-handler/conversation/conversation-loop.tsx b/source/hooks/chat-handler/conversation/conversation-loop.tsx index b7e0e63d5..33771b081 100644 --- a/source/hooks/chat-handler/conversation/conversation-loop.tsx +++ b/source/hooks/chat-handler/conversation/conversation-loop.tsx @@ -949,8 +949,15 @@ export const processAssistantResponse = async ( // Escape during execution: stop prompting further tools; the abort // unwinds on the continuation's next LLM call (same as the auto - // path), surfacing as "Interrupted by user.". - if (controller.signal.aborted) break; + // path), surfacing as "Interrupted by user.". Cancel the tools we + // never reached so the saved history keeps the provider-required + // 1:1 tool_call/result pairing. + if (controller.signal.aborted) { + turnResults.push( + ...createCancellationResults(confirmTools.slice(i + 1)), + ); + break; + } } }