Skip to content

fix: prevent tool call mixing and retry storms #974

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Jpkoech30 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: prevent tool call mixing and retry storms #974

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions src/core/assistant-message/presentAssistantMessage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ export async function presentAssistantMessage(cline: Task) {
cline.presentAssistantMessageLocked = true
cline.presentAssistantMessageHasPendingUpdates = false

// Reset batch skip counter at the start of each present cycle.
// This ensures the count is fresh for each batch of tool calls.
;(cline as any).batchSkipCount = 0

Comment on lines +74 to +77

@coderabbitai coderabbitai Bot Jul 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Fix batchSkipCount reset to prevent it from resetting on every block.

Because presentAssistantMessage recursively calls itself to process each subsequent block, resetting batchSkipCount here causes it to be 0 for every single tool evaluation. This breaks the skip counting logic, meaning nextSkipCount will always incorrectly evaluate to 1.

Only reset the counter at the start of the entire batch (i.e., when processing the first block).

🐛 Proposed fix for the skip counter 
 	// Reset batch skip counter at the start of each present cycle.
 	// This ensures the count is fresh for each batch of tool calls.
-	;(cline as any).batchSkipCount = 0
+	if (cline.currentStreamingContentIndex === 0) {
+		;(cline as any).batchSkipCount = 0
+	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Reset batch skip counter at the start of each present cycle.
// This ensures the count is fresh for each batch of tool calls.
;(cline as any).batchSkipCount = 0
// Reset batch skip counter at the start of each present cycle.
// This ensures the count is fresh for each batch of tool calls.
if (cline.currentStreamingContentIndex === 0) {
;(cline as any).batchSkipCount = 0
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/core/assistant-message/presentAssistantMessage.ts` around lines 74 - 77,
Update presentAssistantMessage so batchSkipCount is reset only when processing
the first block of the batch, not on recursive calls for subsequent blocks.
Guard the reset using the existing block or batch position indicator, preserving
accumulated counts so nextSkipCount reflects prior tool evaluations.

if (cline.currentStreamingContentIndex >= cline.assistantMessageContent.length) {
// This may happen if the last content block was completed before
// streaming could finish. If streaming is finished, and we're out of
Expand Down Expand Up @@ -111,8 +115,11 @@ export async function presentAssistantMessage(cline: Task) {
if (cline.didRejectTool) {
// For native protocol, we must send a tool_result for every tool_use to avoid API errors
const toolCallId = mcpBlock.id
const skipCount = (cline as any).batchSkipCount ?? 0
const nextSkipCount = skipCount + 1
;(cline as any).batchSkipCount = nextSkipCount
const errorMessage = !mcpBlock.partial
? `Skipping MCP tool ${mcpBlock.name} due to user rejecting a previous tool.`
? `Skipping MCP tool ${mcpBlock.name} due to user rejecting a previous tool. (Skipped ${nextSkipCount} tool${nextSkipCount > 1 ? "s" : ""} in this batch.)`
: `MCP tool ${mcpBlock.name} was interrupted and not executed due to user rejecting a previous tool.`

if (toolCallId) {
Expand Down Expand Up @@ -391,8 +398,11 @@ export async function presentAssistantMessage(cline: Task) {
if (cline.didRejectTool) {
// Ignore any tool content after user has rejected tool once.
// For native tool calling, we must send a tool_result for every tool_use to avoid API errors
const skipCount = (cline as any).batchSkipCount ?? 0
const nextSkipCount = skipCount + 1
;(cline as any).batchSkipCount = nextSkipCount
const errorMessage = !block.partial
? `Skipping tool ${toolDescription()} due to user rejecting a previous tool.`
? `Skipping tool ${toolDescription()} due to user rejecting a previous tool. (Skipped ${nextSkipCount} tool${nextSkipCount > 1 ? "s" : ""} in this batch.)`
: `Tool ${toolDescription()} was interrupted and not executed due to user rejecting a previous tool.`

cline.pushToolResultToUserContent({
Expand Down
83 changes: 83 additions & 0 deletions src/core/task/Task.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,13 @@ const DEFAULT_USAGE_COLLECTION_TIMEOUT_MS = 5000 // 5 seconds
const FORCED_CONTEXT_REDUCTION_PERCENT = 75 // Keep 75% of context (remove 25%) on context window errors
const MAX_CONTEXT_WINDOW_RETRIES = 3 // Maximum retries for context window errors

/**
* Maximum number of consecutive retries allowed for the same tool within a single turn.
* After this limit, further retries are blocked until the user provides new input.
* This prevents retry storms where the model repeatedly tries the same failing tool.
*/
const MAX_TOOL_RETRY_BUDGET = 3

export interface TaskOptions extends CreateTaskOptions {
provider: ClineProvider
apiConfiguration: ProviderSettings
Expand Down Expand Up @@ -321,6 +328,22 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
consecutiveNoAssistantMessagesCount: number = 0
toolUsage: ToolUsage = {}

/**
* Retry budget: tracks how many times the same tool has been retried
* consecutively within the current turn. Keyed by tool name (e.g., "execute_command").
* After MAX_TOOL_RETRY_BUDGET (3) retries of the same tool in a row,
* further retries are blocked until the user provides new input.
* The counter resets when user feedback arrives (handleWebviewAskResponse with messageResponse)
* or at the start of each new API request.
*/
toolRetryBudget: Map<string, number> = new Map()

/**
* When true, the retry budget for the current tool has been exceeded and
* no further retries of that tool are allowed until user feedback arrives.
*/
toolRetryBudgetExceeded: boolean = false

// Checkpoints
enableCheckpoints: boolean
checkpointTimeout: number
Expand Down Expand Up @@ -1431,6 +1454,15 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
this.askResponseText = text
this.askResponseImages = images

// Reset retry budget when user provides new input (messageResponse).
// This allows the model to retry tools after the user has had a chance to
// provide guidance. The budget is NOT reset on yesButtonClicked (tool approval)
// since that's the model continuing its own turn, not user feedback.
if (askResponse === "messageResponse") {
this.toolRetryBudget.clear()
this.toolRetryBudgetExceeded = false
}

// Create a checkpoint whenever the user sends a message.
// Use allowEmpty=true to ensure a checkpoint is recorded even if there are no file changes.
// Suppress the checkpoint_saved chat row for this particular checkpoint to keep the timeline clean.
Expand Down Expand Up @@ -2716,6 +2748,10 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
this.didToolFailInCurrentTurn = false
this.presentAssistantMessageLocked = false
this.presentAssistantMessageHasPendingUpdates = false
// Reset retry budget for each new API request (new assistant turn).
// This gives the model a fresh budget of 3 retries per tool per turn.
this.toolRetryBudget.clear()
this.toolRetryBudgetExceeded = false
// No legacy text-stream tool parser.
this.streamingToolCallIndices.clear()
// Clear any leftover streaming tool call state from previous interrupted streams
Expand Down Expand Up @@ -4734,4 +4770,51 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
console.error(`[Task] Queue processing error:`, e)
}
}

/**
* Checks if the retry budget for a given tool has been exceeded.
* Each tool gets MAX_TOOL_RETRY_BUDGET (3) consecutive retries per turn.
* After that, further retries are blocked until user feedback arrives.
*
* @param toolName - The name of the tool being retried
* @returns true if the retry budget is still available, false if blocked
*/
public checkToolRetryBudget(toolName: string): boolean {
if (this.toolRetryBudgetExceeded) {
console.warn(
`[Task#${this.taskId}] Tool retry budget globally exceeded. Blocking further retries for "${toolName}".`,
)
return false
}

const currentRetries = this.toolRetryBudget.get(toolName) ?? 0

if (currentRetries >= MAX_TOOL_RETRY_BUDGET) {
this.toolRetryBudgetExceeded = true
console.warn(
`[Task#${this.taskId}] Tool retry budget exceeded for "${toolName}": ` +
`${currentRetries} retries >= ${MAX_TOOL_RETRY_BUDGET} max. ` +
"Blocking further retries until user provides new input.",
)
return false
}

// Increment the retry counter for this tool
this.toolRetryBudget.set(toolName, currentRetries + 1)
return true
}

/**
* Records a tool use as successful, which resets the retry budget for that tool.
* This is called when a tool completes successfully, allowing the model to use
* the tool again (the budget is per-retry, not per-use).
*
* @param toolName - The name of the tool that succeeded
*/
public recordSuccessfulToolUse(toolName: string): void {
// Only reset the retry budget for this specific tool, not all tools.
// This allows the model to switch to a different tool after exhausting
// the budget on one tool.
this.toolRetryBudget.delete(toolName)
}
Comment on lines +4774 to +4819

@coderabbitai coderabbitai Bot Jul 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Search for usages of the retry budget methods across the codebase.

rg -n 'checkToolRetryBudget|recordSuccessfulToolUse'

Repository: Zoo-Code-Org/Zoo-Code

Length of output: 159

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n## Files mentioning retry budget or tool retries\n'
rg -n -i 'retry budget|toolRetryBudget|MAX_TOOL_RETRY_BUDGET|recordSuccessfulToolUse|checkToolRetryBudget|retry.*tool' src || true

printf '\n## Task.ts context around the methods\n'
FILE='src/core/task/Task.ts'
if [ -f "$FILE" ]; then
  line=$(rg -n 'checkToolRetryBudget|recordSuccessfulToolUse' "$FILE" | head -n 1 | cut -d: -f1 || true)
  if [ -n "${line:-}" ]; then
    start=$((line-40)); [ "$start" -lt 1 ] && start=1
    end=$((line+80))
    sed -n "${start},${end}p" "$FILE"
  fi
fi

printf '\n## presentAssistantMessage.ts references\n'
if [ -f 'src/core/task/presentAssistantMessage.ts' ]; then
  rg -n -i 'retry|toolRetryBudget|checkToolRetryBudget|recordSuccessfulToolUse|tool' src/core/task/presentAssistantMessage.ts || true
fi

Repository: Zoo-Code-Org/Zoo-Code

Length of output: 6904

Wire the retry budget into the tool execution path
These helpers are only defined here; nothing in the task flow calls them, so the retry budget never blocks retries. Hook them into the tool retry/success path, or remove the dead code if it is no longer needed.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/core/task/Task.ts` around lines 4774 - 4819, The retry-budget helpers are
unused, so retries are never blocked. Wire checkToolRetryBudget into the tool
retry path before allowing a retry, and invoke recordSuccessfulToolUse after
successful tool completion using the same tool name; otherwise remove both
helpers and their state if the budget is not intended to be enforced.

}
45 changes: 24 additions & 21 deletions src/core/task/validateToolResultIds.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,23 @@ export function validateAndFixToolResultIds(
)
}

// Match tool_results to tool_uses by position and fix incorrect IDs
// Log the mismatched IDs instead of silently falling back to positional matching.
// Positional matching caused cross-wiring of tool results when the ordering of
// tool_results did not match the ordering of tool_use blocks (e.g., parallel tool
// calls where results arrive in a different order than the calls were made).
// The correct behavior is to surface the mismatch so it can be diagnosed and fixed
// at the source, not silently remapped.
console.warn(
"[validateAndFixToolResultIds] Tool result ID mismatch detected — removing positional fallback. " +
`tool_result IDs: [${toolResultIdList.join(", ")}], ` +
`tool_use IDs: [${toolUseIdList.join(", ")}]. ` +
"Mismatched tool_result blocks will be dropped to prevent cross-wiring.",
)

// Filter out tool_results with invalid or duplicate IDs instead of remapping them.
// This is safer than positional fallback: dropping a misattributed result is
// preferable to wiring it to the wrong tool_use, which would cause subtle
// correctness bugs in the LLM's understanding of tool outputs.
const usedToolUseIds = new Set<string>()
const contentArray = userMessage.content as Anthropic.Messages.ContentBlockParam[]

Expand All @@ -175,31 +191,18 @@ export function validateAndFixToolResultIds(
return block
}

// If the ID is already valid and not yet used, keep it
// If the ID is valid and not yet used, keep it
if (validToolUseIds.has(block.tool_use_id) && !usedToolUseIds.has(block.tool_use_id)) {
usedToolUseIds.add(block.tool_use_id)
return block
}

// Find which tool_result index this block is by comparing references.
// This correctly handles duplicate tool_use_ids - we find the actual block's
// position among all tool_results, not the first block with a matching ID.
const toolResultIndex = toolResults.indexOf(block as Anthropic.ToolResultBlockParam)

// Try to match by position - only fix if there's a corresponding tool_use
if (toolResultIndex !== -1 && toolResultIndex < toolUseBlocks.length) {
const correctId = toolUseBlocks[toolResultIndex].id
// Only use this ID if it hasn't been used yet
if (!usedToolUseIds.has(correctId)) {
usedToolUseIds.add(correctId)
return {
...block,
tool_use_id: correctId,
}
}
}

// No corresponding tool_use for this tool_result, or the ID is already used
// Invalid or duplicate tool_result ID — drop it instead of remapping
console.warn(
`[validateAndFixToolResultIds] Dropping tool_result with tool_use_id "${block.tool_use_id}": ` +
`${validToolUseIds.has(block.tool_use_id) ? "duplicate ID" : "ID not found in tool_use blocks"}. ` +
"This prevents cross-wiring of tool results.",
)
return null
})
.filter((block): block is NonNullable<typeof block> => block !== null)
Expand Down
80 changes: 74 additions & 6 deletions src/core/tools/ExecuteCommandTool.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,12 @@ export class ExecuteCommandTool extends BaseTool<"execute_command"> {
pushToolResult(formatResponse.rooIgnoreError(ignoredFileAttemptedToAccess))
return
}

// Clear any prior in-flight ask state before proceeding with execution.
// This prevents a stale ask from a previous command invocation (e.g., a
// shell integration fallback) from racing with the fresh approval prompt
// in the current invocation.
task.supersedePendingAsk()

task.consecutiveMistakeCount = 0

Expand Down Expand Up @@ -150,11 +156,34 @@ export class ExecuteCommandTool extends BaseTool<"execute_command"> {

try {
const [rejected, result] = await executeCommandInTerminal(task, options)
} catch (error: unknown) {
// Invalidate pending ask from first execution to prevent race condition
task.supersedePendingAsk()

if (canRetryShellIntegrationError(error)) {
// Silent retry via execa — shell startup race, command was not submitted.
const status: CommandExecutionStatus = { executionId, status: "fallback" }
provider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) })

const [rejected, result] = await executeCommandInTerminal(task, {
...options,
terminalShellIntegrationDisabled: true,
})

if (rejected) {
task.didRejectTool = true
}

// Fix: Mark the first execution result as consumed so we don't send
// duplicate pushToolResult calls. The shell integration retry below
// will produce the single authoritative result.
// Also await onCompletedPromise to avoid a race where the first
// command's onCompleted fires after we've already started the retry,
// corrupting shared state (completed, persistedResult, etc.).
if (!rejected && !runInBackground) {
await onCompletedPromise
}

pushToolResult(result)
} catch (error: unknown) {
// Invalidate pending ask from first execution to prevent race condition
Comment on lines 157 to 189

@coderabbitai coderabbitai Bot Jul 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Remove the duplicated and malformed try-catch block.

There is a severe syntax error here caused by a botched git merge. Lines 159-188 duplicate the catch block and contain invalid references (such as onCompletedPromise, which belongs inside executeCommandInTerminal, not execute). The invalid syntax } catch () { ... } catch () { completely breaks parser execution and surfaces upstream as static analysis errors.

🐛 Proposed fix for the merge conflict 
 			try {
 				const [rejected, result] = await executeCommandInTerminal(task, options)
-			} catch (error: unknown) {
-				// Invalidate pending ask from first execution to prevent race condition
-				task.supersedePendingAsk()
-
-				if (canRetryShellIntegrationError(error)) {
-					// Silent retry via execa — shell startup race, command was not submitted.
-					const status: CommandExecutionStatus = { executionId, status: "fallback" }
-					provider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) })
-
-					const [rejected, result] = await executeCommandInTerminal(task, {
-						...options,
-						terminalShellIntegrationDisabled: true,
-					})
-
-				if (rejected) {
-					task.didRejectTool = true
-				}
-
-				// Fix: Mark the first execution result as consumed so we don't send
-				// duplicate pushToolResult calls. The shell integration retry below
-				// will produce the single authoritative result.
-				// Also await onCompletedPromise to avoid a race where the first
-				// command's onCompleted fires after we've already started the retry,
-				// corrupting shared state (completed, persistedResult, etc.).
-				if (!rejected && !runInBackground) {
-					await onCompletedPromise
-				}
-
-				pushToolResult(result)
-			} catch (error: unknown) {
+				
+				if (rejected) {
+					task.didRejectTool = true
+				}
+				
+				pushToolResult(result)
+			} catch (error: unknown) {
 				// Invalidate pending ask from first execution to prevent race condition
 				task.supersedePendingAsk()
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
try {
const [rejected, result] = await executeCommandInTerminal(task, options)
} catch (error: unknown) {
// Invalidate pending ask from first execution to prevent race condition
task.supersedePendingAsk()
if (canRetryShellIntegrationError(error)) {
// Silent retry via execa — shell startup race, command was not submitted.
const status: CommandExecutionStatus = { executionId, status: "fallback" }
provider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) })
const [rejected, result] = await executeCommandInTerminal(task, {
...options,
terminalShellIntegrationDisabled: true,
})
if (rejected) {
task.didRejectTool = true
}
// Fix: Mark the first execution result as consumed so we don't send
// duplicate pushToolResult calls. The shell integration retry below
// will produce the single authoritative result.
// Also await onCompletedPromise to avoid a race where the first
// command's onCompleted fires after we've already started the retry,
// corrupting shared state (completed, persistedResult, etc.).
if (!rejected && !runInBackground) {
await onCompletedPromise
}
pushToolResult(result)
} catch (error: unknown) {
// Invalidate pending ask from first execution to prevent race condition
try {
const [rejected, result] = await executeCommandInTerminal(task, options)
if (rejected) {
task.didRejectTool = true
}
pushToolResult(result)
} catch (error: unknown) {
// Invalidate pending ask from first execution to prevent race condition
task.supersedePendingAsk()
🧰 Tools
🪛 Biome (2.5.3)

[error] 188-188: Expected a statement but instead found 'catch (error: unknown)'.

(parse)

🪛 GitHub Actions: Code QA Roo Code / 0_compile.txt

[error] 188-188: ESLint parsing error during linting. 'try' expected.

🪛 GitHub Actions: Code QA Roo Code / compile

[error] 188-188: ESLint failed with parsing error: "'try' expected".

🪛 GitHub Actions: E2E Tests (Mocked) / 0_e2e-mock.txt

[error] 188-188: esbuild: ERROR: Unexpected "catch" (Build failed with 1 error).

🪛 GitHub Actions: E2E Tests (Mocked) / e2e-mock

[error] 188-188: esbuild failed: ERROR: Unexpected "catch" (core/tools/ExecuteCommandTool.ts:188:5).

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/core/tools/ExecuteCommandTool.ts` around lines 157 - 189, Remove the
duplicated malformed catch block within the execute flow in ExecuteCommandTool,
retaining only the valid error-handling path for executeCommandInTerminal.
Delete references that do not belong to this scope, including onCompletedPromise
and the duplicate retry/result logic, and ensure the surrounding try/catch
structure is syntactically valid.

Expand All @@ -165,10 +194,17 @@ export class ExecuteCommandTool extends BaseTool<"execute_command"> {
const status: CommandExecutionStatus = { executionId, status: "fallback" }
provider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) })

const [rejected, result] = await executeCommandInTerminal(task, {
// Fix: When retrying with shell integration fallback, ensure we use a
// fresh terminal rather than the VSCode terminal that may have a stale
// shell integration state. This prevents double-execution where the
// command runs both in the VSCode terminal AND via execa.
const retryOptions = {
...options,
terminalShellIntegrationDisabled: true,
})
forceNewTerminal: true,
}

const [rejected, result] = await executeCommandInTerminal(task, retryOptions)

if (rejected) {
task.didRejectTool = true
Expand All @@ -181,10 +217,16 @@ export class ExecuteCommandTool extends BaseTool<"execute_command"> {

if (error instanceof ShellIntegrationError) {
pushToolResult(
"Command was submitted in the VS Code terminal, but shell integration did not report its output or completion status. Do not run the command again automatically.",
formatResponse.toolError(
"Command was submitted in the terminal, but its output and completion status could not be tracked due to a shell integration error. The command may still be running. Do NOT re-run the command automatically — inspect the terminal manually and report the result.",
),
)
} else {
pushToolResult(`Command failed to execute in terminal due to a shell integration error.`)
pushToolResult(
formatResponse.toolError(
`Command failed to execute in terminal: ${error instanceof Error ? error.message : String(error)}. Check the terminal state before retrying.`,
),
)
}
}
}
Expand All @@ -209,6 +251,8 @@ export type ExecuteCommandOptions = {
terminalShellIntegrationDisabled?: boolean
commandExecutionTimeout?: number
agentTimeout?: number
/** When true, forces creation of a fresh terminal even for retries. */
forceNewTerminal?: boolean
}

export async function executeCommandInTerminal(
Expand Down Expand Up @@ -430,7 +474,10 @@ export async function executeCommandInTerminal(
}
}

const terminal = await TerminalRegistry.getOrCreateTerminal(workingDir, task.taskId, terminalProvider)
// Fix: Use getOrCreateCommandTerminal to ensure each execute_command gets a
// dedicated terminal, preventing output interleaving when multiple commands
// run sequentially on the same working directory.
const terminal = await TerminalRegistry.getOrCreateCommandTerminal(workingDir, task.taskId, terminalProvider)

if (terminal instanceof Terminal) {
terminal.terminal.show(true)
Expand Down Expand Up @@ -475,6 +522,14 @@ export async function executeCommandInTerminal(
racers.push(
new Promise<void>((_, reject) => {
userTimeoutId = setTimeout(() => {
// Fix timeout race: Only fire the timeout if the command hasn't
// already completed. Check `completed` under a microtask to avoid
// a race where onCompleted sets completed=true between the timeout
// firing and this check.
if (completed) {
resolve()
return
}
isUserTimedOut = true
task.terminalProcess?.abort()
reject(new Error(`Command execution timed out after ${commandExecutionTimeout}ms`))
Expand All @@ -483,7 +538,14 @@ export async function executeCommandInTerminal(
)
}

await Promise.race(racers)
// Fix timeout race: After Promise.race resolves (either process completed,
// agent timeout, or user timeout), ensure the other timeout is cleared
// immediately. Without this, a user timeout could fire AFTER the command
// has already completed, incorrectly aborting it.
clearTimeout(agentTimeoutId)
agentTimeoutId = undefined
clearTimeout(userTimeoutId)
userTimeoutId = undefined
} catch (error) {
if (isUserTimedOut) {
const status: CommandExecutionStatus = { executionId, status: "timeout" }
Expand Down Expand Up @@ -525,6 +587,12 @@ export async function executeCommandInTerminal(
await onCompletedPromise
}

// If the command completed during the onCompleted wait (e.g., a very fast
// command that finished before the timeout was set), make sure the timeout
// race fix above didn't leave a dangling timeout that could fire later.
clearTimeout(agentTimeoutId)
clearTimeout(userTimeoutId)

if (message) {
const { text, images } = message
await task.say("user_feedback", text, images)
Expand Down
Loading
Loading