From adc1573b096206d18f58899a1a849451e97d4938 Mon Sep 17 00:00:00 2001 From: Will Li Date: Wed, 9 Jul 2025 08:12:45 -0700 Subject: [PATCH] refactor and fixed copy --- packages/types/src/global-settings.ts | 2 +- src/core/timeout/TimeoutManager.ts | 26 +- src/core/timeout/ToolExecutionWrapper.ts | 133 +---- .../__tests__/ai-fallback-real.spec.ts | 234 -------- .../timeout/__tests__/e2e-ai-test.spec.ts | 179 ------ .../__tests__/timeout-fallback.spec.ts | 554 ++++++++++++++++++ .../__tests__/timeout-integration.spec.ts | 375 +++++++----- .../timeout/__tests__/timeout-manager.spec.ts | 178 ++++++ .../__tests__/tool-injection-test.spec.ts | 98 ---- .../timeout/__tests__/ui-integration.spec.ts | 70 --- src/core/tools/executeCommandTool.ts | 6 +- src/core/webview/ClineProvider.ts | 4 +- .../settings/AutoApproveSettings.tsx | 2 +- .../src/context/ExtensionStateContext.tsx | 6 +- webview-ui/src/i18n/locales/en/chat.json | 2 +- webview-ui/src/i18n/locales/en/settings.json | 2 +- 16 files changed, 997 insertions(+), 874 deletions(-) delete mode 100644 src/core/timeout/__tests__/ai-fallback-real.spec.ts delete mode 100644 src/core/timeout/__tests__/e2e-ai-test.spec.ts create mode 100644 src/core/timeout/__tests__/timeout-fallback.spec.ts create mode 100644 src/core/timeout/__tests__/timeout-manager.spec.ts delete mode 100644 src/core/timeout/__tests__/tool-injection-test.spec.ts delete mode 100644 src/core/timeout/__tests__/ui-integration.spec.ts diff --git a/packages/types/src/global-settings.ts b/packages/types/src/global-settings.ts index 130ba09b23..618a6c9cd8 100644 --- a/packages/types/src/global-settings.ts +++ b/packages/types/src/global-settings.ts @@ -231,7 +231,7 @@ export const EVALS_SETTINGS: RooCodeSettings = { enableCheckpoints: false, // Timeout settings - toolExecutionTimeoutMs: 300000, // 5 minutes default + toolExecutionTimeoutMs: 60000, // 1 minute default timeoutFallbackEnabled: false, rateLimitSeconds: 0, diff --git a/src/core/timeout/TimeoutManager.ts b/src/core/timeout/TimeoutManager.ts index 28d361ef07..72af1beb49 100644 --- a/src/core/timeout/TimeoutManager.ts +++ b/src/core/timeout/TimeoutManager.ts @@ -32,10 +32,10 @@ export class TimeoutManager extends EventEmitter { private static instance: TimeoutManager | undefined private activeOperations = new Map() /** - * Multiple timeout events can be run at once - * eg. running a command while reading a file + * Assumes there is only on active tool-- does not timeout edge cases + * like "Proceed While Running" */ - private timeoutEvents: TimeoutEvent[] = [] + private lastTimeoutEvent: TimeoutEvent | null = null private constructor() { super() @@ -93,7 +93,7 @@ export class TimeoutManager extends EventEmitter { const timedOut = controller.signal.aborted if (timedOut) { - // Log timeout event + // Store the last timeout event const timeoutEvent: TimeoutEvent = { toolName: config.toolName, timeoutMs: config.timeoutMs, @@ -102,7 +102,7 @@ export class TimeoutManager extends EventEmitter { timestamp: Date.now(), } - this.timeoutEvents.push(timeoutEvent) + this.lastTimeoutEvent = timeoutEvent this.emit("timeout", timeoutEvent) return { @@ -154,17 +154,17 @@ export class TimeoutManager extends EventEmitter { } /** - * Get timeout events for debugging/monitoring + * Get the last timeout event */ - public getTimeoutEvents(limit = 100): TimeoutEvent[] { - return this.timeoutEvents.slice(-limit) + public getLastTimeoutEvent(): TimeoutEvent | null { + return this.lastTimeoutEvent } /** - * Clear timeout event history + * Clear the last timeout event */ - public clearTimeoutEvents(): void { - this.timeoutEvents = [] + public clearLastTimeoutEvent(): void { + this.lastTimeoutEvent = null } /** @@ -183,7 +183,7 @@ export class TimeoutManager extends EventEmitter { } private generateOperationId(toolName: ToolName, taskId?: string): string { - return `${toolName}:${taskId || "default"}:${Date.now()}` + return `${toolName}:${taskId || "default"}` } /** @@ -192,7 +192,7 @@ export class TimeoutManager extends EventEmitter { public dispose(): void { this.cancelAllOperations() this.removeAllListeners() - this.timeoutEvents = [] + this.lastTimeoutEvent = null } } diff --git a/src/core/timeout/ToolExecutionWrapper.ts b/src/core/timeout/ToolExecutionWrapper.ts index a67d10f2a5..50ac9c765f 100644 --- a/src/core/timeout/ToolExecutionWrapper.ts +++ b/src/core/timeout/ToolExecutionWrapper.ts @@ -18,7 +18,7 @@ export class ToolExecutionWrapper { public static async execute( operation: (signal: AbortSignal) => Promise, options: ToolExecutionOptions, - defaultTimeoutMs = 300000, // 5 minutes default + defaultTimeoutMs = 60000, // 1 minute default ): Promise> { const config: TimeoutConfig = { toolName: options.toolName, @@ -29,135 +29,4 @@ export class ToolExecutionWrapper { return timeoutManager.executeWithTimeout(operation, config) } - - /** - * Wrap a promise-based operation to support AbortSignal - */ - public static wrapPromise(promiseFactory: () => Promise, signal: AbortSignal): Promise { - return new Promise((resolve, reject) => { - // Check if already aborted - if (signal.aborted) { - reject(new Error("Operation was aborted before starting")) - return - } - - // Set up abort listener - const abortListener = () => { - reject(new Error("Operation was aborted")) - } - - signal.addEventListener("abort", abortListener) - - // Execute the operation - promiseFactory() - .then((result) => { - signal.removeEventListener("abort", abortListener) - resolve(result) - }) - .catch((error) => { - signal.removeEventListener("abort", abortListener) - reject(error) - }) - }) - } - - /** - * Wrap a callback-based operation to support AbortSignal - */ - public static wrapCallback( - operation: (callback: (error: Error | null, result?: T) => void, signal: AbortSignal) => void, - signal: AbortSignal, - ): Promise { - return new Promise((resolve, reject) => { - // Check if already aborted - if (signal.aborted) { - reject(new Error("Operation was aborted before starting")) - return - } - - // Set up abort listener - const abortListener = () => { - reject(new Error("Operation was aborted")) - } - - signal.addEventListener("abort", abortListener) - - // Execute the operation - operation((error, result) => { - signal.removeEventListener("abort", abortListener) - - if (error) { - reject(error) - } else { - resolve(result!) - } - }, signal) - }) - } - - /** - * Create an abortable delay - */ - public static delay(ms: number, signal: AbortSignal): Promise { - return new Promise((resolve, reject) => { - if (signal.aborted) { - reject(new Error("Delay was aborted before starting")) - return - } - - const timeoutId = setTimeout(() => { - signal.removeEventListener("abort", abortListener) - resolve() - }, ms) - - const abortListener = () => { - clearTimeout(timeoutId) - reject(new Error("Delay was aborted")) - } - - signal.addEventListener("abort", abortListener) - }) - } - - /** - * Execute multiple operations in parallel with timeout protection - */ - public static async executeParallel( - operations: Array<{ - operation: (signal: AbortSignal) => Promise - options: ToolExecutionOptions - }>, - defaultTimeoutMs = 300000, - ): Promise[]> { - const promises = operations.map(({ operation, options }) => - ToolExecutionWrapper.execute(operation, options, defaultTimeoutMs), - ) - - return Promise.all(promises) - } - - /** - * Execute operations in sequence with timeout protection - */ - public static async executeSequential( - operations: Array<{ - operation: (signal: AbortSignal) => Promise - options: ToolExecutionOptions - }>, - defaultTimeoutMs = 300000, - ): Promise[]> { - const results: TimeoutResult[] = [] - - for (const { operation, options } of operations) { - const result = await ToolExecutionWrapper.execute(operation, options, defaultTimeoutMs) - results.push(result) - - // Stop execution if any operation fails or times out - if (!result.success) { - break - } - } - - return results - } } diff --git a/src/core/timeout/__tests__/ai-fallback-real.spec.ts b/src/core/timeout/__tests__/ai-fallback-real.spec.ts deleted file mode 100644 index 7b378a2b01..0000000000 --- a/src/core/timeout/__tests__/ai-fallback-real.spec.ts +++ /dev/null @@ -1,234 +0,0 @@ -// npx vitest run src/core/timeout/__tests__/ai-fallback-real.spec.ts - -import { describe, test, expect, beforeEach, vitest } from "vitest" -import { TimeoutFallbackHandler } from "../TimeoutFallbackHandler" -import type { ApiHandler, SingleCompletionHandler } from "../../../api" -import type { Task } from "../../task/Task" - -// Create a mock API handler that extends ApiHandler and includes completePrompt -interface MockApiHandler extends ApiHandler, SingleCompletionHandler {} - -describe("TimeoutFallbackHandler - Real AI Implementation", () => { - let mockApiHandler: MockApiHandler - let mockTask: Partial - - beforeEach(() => { - vitest.clearAllMocks() - - // Mock API handler that simulates real AI responses - mockApiHandler = { - createMessage: vitest.fn(), - getModel: vitest.fn().mockReturnValue({ id: "test-model", info: { maxTokens: 4096 } }), - countTokens: vitest.fn().mockResolvedValue(100), - completePrompt: vitest.fn(), - } - - // Mock task with API handler - mockTask = { - api: mockApiHandler, - } - }) - - test("should use AI to generate contextual suggestions when available", async () => { - // Mock AI response with numbered suggestions - const mockAiResponse = `Here are some suggestions for the timeout: - -1. Break the npm install command into smaller package installations -2. Clear npm cache and try again with npm cache clean --force -3. Use npm install --no-optional to skip optional dependencies -4. Check network connectivity and try with different registry` - - ;(mockApiHandler.completePrompt as any).mockResolvedValueOnce(mockAiResponse) - - const context = { - toolName: "execute_command" as const, - timeoutMs: 30000, - executionTimeMs: 35000, - toolParams: { command: "npm install" }, - } - - const result = await TimeoutFallbackHandler.generateAiFallback(context, mockTask as Task) - - expect(result.success).toBe(true) - expect(result.toolCall).toBeDefined() - expect(result.toolCall?.name).toBe("ask_followup_question") - expect(result.toolCall?.params.question).toContain("execute_command") - expect(result.toolCall?.params.question).toContain("30 seconds") - - // Check that AI-generated suggestions are included - const followUp = result.toolCall?.params.follow_up || "" - expect(followUp).toContain("Break the npm install command into smaller package installations") - expect(followUp).toContain("Clear npm cache and try again") - expect(followUp).toContain("Use npm install --no-optional") - expect(followUp).toContain("Check network connectivity") - - // Verify AI was called with proper prompt - expect(mockApiHandler.completePrompt).toHaveBeenCalledWith( - expect.stringContaining("execute_command operation has timed out"), - ) - expect(mockApiHandler.completePrompt).toHaveBeenCalledWith(expect.stringContaining("npm install")) - }) - - test("should fallback to static suggestions when AI fails", async () => { - // Mock AI failure - ;(mockApiHandler.completePrompt as any).mockRejectedValueOnce(new Error("API Error")) - - const context = { - toolName: "execute_command" as const, - timeoutMs: 30000, - executionTimeMs: 35000, - toolParams: { command: "npm test" }, - } - - const result = await TimeoutFallbackHandler.generateAiFallback(context, mockTask as Task) - - expect(result.success).toBe(true) - expect(result.toolCall).toBeDefined() - expect(result.toolCall?.name).toBe("ask_followup_question") - - // Should contain static fallback suggestions - const followUp = result.toolCall?.params.follow_up || "" - expect(followUp).toContain('Break "npm test" into smaller') - expect(followUp).toContain("background using") - expect(followUp).toContain("alternative approach") - expect(followUp).toContain("Increase the timeout") - }) - - test("should fallback to static suggestions when API handler is unavailable", async () => { - // Task without API handler - const taskWithoutApi = {} - - const context = { - toolName: "read_file" as const, - timeoutMs: 5000, - executionTimeMs: 6000, - toolParams: { path: "/large/file.txt" }, - } - - const result = await TimeoutFallbackHandler.generateAiFallback(context, taskWithoutApi as Task) - - expect(result.success).toBe(true) - expect(result.toolCall).toBeDefined() - - // Should contain static fallback suggestions for read_file - const followUp = result.toolCall?.params.follow_up || "" - expect(followUp).toContain('Read "/large/file.txt" in smaller chunks') - expect(followUp).toContain("accessible and not locked") - }) - - test("should parse AI response with different numbering formats", async () => { - // Test different numbering formats - const mockAiResponse = `Here are the suggestions: - -1) Try breaking the command into parts -2. Use a different approach -3) Check system resources -4. Increase timeout duration` - - ;(mockApiHandler.completePrompt as any).mockResolvedValueOnce(mockAiResponse) - - const context = { - toolName: "execute_command" as const, - timeoutMs: 10000, - executionTimeMs: 12000, - toolParams: { command: "build script" }, - } - - const result = await TimeoutFallbackHandler.generateAiFallback(context, mockTask as Task) - - expect(result.success).toBe(true) - const followUp = result.toolCall?.params.follow_up || "" - expect(followUp).toContain("Try breaking the command into parts") - expect(followUp).toContain("Use a different approach") - expect(followUp).toContain("Check system resources") - expect(followUp).toContain("Increase timeout duration") - }) - - test("should handle AI response without numbered list", async () => { - // AI response without clear numbering - const mockAiResponse = `You could try splitting the operation. Another option is to check the network. Maybe increase the timeout. Consider using a different tool.` - - ;(mockApiHandler.completePrompt as any).mockResolvedValueOnce(mockAiResponse) - - const context = { - toolName: "browser_action" as const, - timeoutMs: 15000, - executionTimeMs: 16000, - toolParams: { action: "click" }, - } - - const result = await TimeoutFallbackHandler.generateAiFallback(context, mockTask as Task) - - expect(result.success).toBe(true) - const followUp = result.toolCall?.params.follow_up || "" - - // Should extract sentences as suggestions - expect(followUp).toContain("You could try splitting the operation") - expect(followUp).toContain("Another option is to check the network") - }) - - test("should include task context in AI prompt when available", async () => { - const mockAiResponse = `1. Try a different approach\n2. Check the working directory\n3. Break into steps` - ;(mockApiHandler.completePrompt as any).mockResolvedValueOnce(mockAiResponse) - - const context = { - toolName: "search_files" as const, - timeoutMs: 20000, - executionTimeMs: 22000, - toolParams: { path: "/project", regex: ".*\\.ts$" }, - taskContext: { - currentStep: "Finding TypeScript files", - workingDirectory: "/project/src", - previousActions: ["read package.json", "list files"], - }, - } - - const result = await TimeoutFallbackHandler.generateAiFallback(context, mockTask as Task) - - expect(result.success).toBe(true) - - // Verify the prompt included task context - const calledPrompt = (mockApiHandler.completePrompt as any).mock.calls[0][0] - expect(calledPrompt).toContain("Current step: Finding TypeScript files") - expect(calledPrompt).toContain("Working directory: /project/src") - expect(calledPrompt).toContain("search_files") - expect(calledPrompt).toContain("ts$") // Just check for the pattern ending - }) - - test("should limit suggestions to maximum of 4", async () => { - // AI response with many suggestions - const mockAiResponse = `Here are many suggestions: - -1. First suggestion -2. Second suggestion -3. Third suggestion -4. Fourth suggestion -5. Fifth suggestion -6. Sixth suggestion -7. Seventh suggestion` - - ;(mockApiHandler.completePrompt as any).mockResolvedValueOnce(mockAiResponse) - - const context = { - toolName: "write_to_file" as const, - timeoutMs: 8000, - executionTimeMs: 9000, - toolParams: { path: "/output.txt" }, - } - - const result = await TimeoutFallbackHandler.generateAiFallback(context, mockTask as Task) - - expect(result.success).toBe(true) - const followUp = result.toolCall?.params.follow_up || "" - - // Count the number of tags - const suggestCount = (followUp.match(//g) || []).length - expect(suggestCount).toBeLessThanOrEqual(4) - - // Should include first 4 suggestions - expect(followUp).toContain("First suggestion") - expect(followUp).toContain("Fourth suggestion") - // Should not include 5th and beyond - expect(followUp).not.toContain("Fifth suggestion") - }) -}) diff --git a/src/core/timeout/__tests__/e2e-ai-test.spec.ts b/src/core/timeout/__tests__/e2e-ai-test.spec.ts deleted file mode 100644 index 40bd9e5406..0000000000 --- a/src/core/timeout/__tests__/e2e-ai-test.spec.ts +++ /dev/null @@ -1,179 +0,0 @@ -// End-to-end test to verify AI fallback generation works -// npx vitest run src/core/timeout/__tests__/e2e-ai-test.spec.ts - -import { describe, test, expect, beforeEach, vitest } from "vitest" -import { TimeoutFallbackHandler } from "../TimeoutFallbackHandler" -import type { ApiHandler, SingleCompletionHandler } from "../../../api" -import type { Task } from "../../task/Task" - -// Mock API handler that simulates a real AI provider -interface TestApiHandler extends ApiHandler, SingleCompletionHandler {} - -describe("TimeoutFallbackHandler - End-to-End AI Test", () => { - test("should generate realistic AI suggestions for execute_command timeout", async () => { - // Create a realistic mock API handler - const mockApiHandler: TestApiHandler = { - createMessage: vitest.fn(), - getModel: vitest.fn().mockReturnValue({ id: "claude-3-sonnet", info: { maxTokens: 4096 } }), - countTokens: vitest.fn().mockResolvedValue(150), - completePrompt: vitest.fn().mockResolvedValue( - ` -Here are some suggestions for the npm install timeout: - -1. Clear npm cache with "npm cache clean --force" and retry -2. Break installation into smaller chunks by installing packages individually -3. Use "npm install --no-optional" to skip optional dependencies -4. Check network connectivity and try with a different registry - `.trim(), - ), - } - - const mockTask: Partial = { - api: mockApiHandler, - } - - const context = { - toolName: "execute_command" as const, - timeoutMs: 60000, - executionTimeMs: 65000, - toolParams: { - command: "npm install", - cwd: "/project", - }, - } - - const result = await TimeoutFallbackHandler.generateAiFallback(context, mockTask as Task) - - // Verify the result structure - expect(result.success).toBe(true) - expect(result.toolCall).toBeDefined() - expect(result.toolCall?.name).toBe("ask_followup_question") - expect(result.toolCall?.params.question).toContain("execute_command") - expect(result.toolCall?.params.question).toContain("60 seconds") - - // Verify AI-generated suggestions are included - const followUp = result.toolCall?.params.follow_up || "" - expect(followUp).toContain("Clear npm cache") - expect(followUp).toContain("Break installation into smaller chunks") - expect(followUp).toContain("no-optional") - expect(followUp).toContain("network connectivity") - - // Verify the AI was called with a proper prompt - expect(mockApiHandler.completePrompt).toHaveBeenCalledWith( - expect.stringContaining("execute_command operation has timed out"), - ) - expect(mockApiHandler.completePrompt).toHaveBeenCalledWith(expect.stringContaining("npm install")) - expect(mockApiHandler.completePrompt).toHaveBeenCalledWith(expect.stringContaining("60 seconds")) - }) - - test("should handle AI response with different formatting", async () => { - // Mock AI response with different numbering style - const mockApiHandler: TestApiHandler = { - createMessage: vitest.fn(), - getModel: vitest.fn().mockReturnValue({ id: "gpt-4", info: { maxTokens: 8192 } }), - countTokens: vitest.fn().mockResolvedValue(200), - completePrompt: vitest.fn().mockResolvedValue( - ` -Based on the search_files timeout, here are my recommendations: - -• Limit search to specific subdirectories instead of entire project -• Use more specific regex patterns to reduce matches -• Try list_files first to understand directory structure -• Consider breaking search into multiple smaller operations - `.trim(), - ), - } - - const mockTask: Partial = { - api: mockApiHandler, - } - - const context = { - toolName: "search_files" as const, - timeoutMs: 30000, - executionTimeMs: 32000, - toolParams: { - path: "/large-project", - regex: ".*", - file_pattern: "*.ts", - }, - } - - const result = await TimeoutFallbackHandler.generateAiFallback(context, mockTask as Task) - - expect(result.success).toBe(true) - expect(result.toolCall).toBeDefined() - - // Should extract suggestions even with bullet points - const followUp = result.toolCall?.params.follow_up || "" - expect(followUp).toContain("Narrow the search scope") - expect(followUp).toContain("simpler search patterns") - expect(followUp).toContain("file type filters") - expect(followUp).toContain("incrementally in smaller batches") - }) - - test("should gracefully handle AI failure and use static fallback", async () => { - // Mock API handler that fails - const mockApiHandler: TestApiHandler = { - createMessage: vitest.fn(), - getModel: vitest.fn().mockReturnValue({ id: "test-model", info: { maxTokens: 4096 } }), - countTokens: vitest.fn().mockResolvedValue(100), - completePrompt: vitest.fn().mockRejectedValue(new Error("API rate limit exceeded")), - } - - const mockTask: Partial = { - api: mockApiHandler, - } - - const context = { - toolName: "read_file" as const, - timeoutMs: 10000, - executionTimeMs: 12000, - toolParams: { - path: "/very/large/file.log", - }, - } - - const result = await TimeoutFallbackHandler.generateAiFallback(context, mockTask as Task) - - expect(result.success).toBe(true) - expect(result.toolCall).toBeDefined() - - // Should contain static fallback suggestions for read_file - const followUp = result.toolCall?.params.follow_up || "" - expect(followUp).toContain('Read "/very/large/file.log" in smaller chunks') - expect(followUp).toContain("accessible and not locked") - expect(followUp).toContain("different approach") - expect(followUp).toContain("Increase the timeout") - - // Verify AI was attempted but failed gracefully - expect(mockApiHandler.completePrompt).toHaveBeenCalled() - }) - - test("should work without task API handler", async () => { - // Task without API handler - const mockTask: Partial = {} - - const context = { - toolName: "browser_action" as const, - timeoutMs: 15000, - executionTimeMs: 16500, - toolParams: { - action: "click", - coordinate: "450,300", - }, - } - - const result = await TimeoutFallbackHandler.generateAiFallback(context, mockTask as Task) - - expect(result.success).toBe(true) - expect(result.toolCall).toBeDefined() - - // Should contain static fallback suggestions for browser_action - const followUp = result.toolCall?.params.follow_up || "" - expect(followUp).toContain('Simplify the "click"') - expect(followUp).toContain("Wait for specific elements") - expect(followUp).toContain("direct API calls") - expect(followUp).toContain("Reset the browser session") - }) -}) diff --git a/src/core/timeout/__tests__/timeout-fallback.spec.ts b/src/core/timeout/__tests__/timeout-fallback.spec.ts new file mode 100644 index 0000000000..d835bbf6ac --- /dev/null +++ b/src/core/timeout/__tests__/timeout-fallback.spec.ts @@ -0,0 +1,554 @@ +// npx vitest run src/core/timeout/__tests__/timeout-fallback.spec.ts + +import { describe, test, expect, beforeEach, vitest } from "vitest" +import { TimeoutFallbackHandler } from "../TimeoutFallbackHandler" +import type { TimeoutFallbackResult } from "../TimeoutFallbackHandler" +import type { ApiHandler, SingleCompletionHandler } from "../../../api" +import type { Task } from "../../task/Task" + +// Create a mock API handler that extends ApiHandler and includes completePrompt +interface MockApiHandler extends ApiHandler, SingleCompletionHandler {} + +describe("TimeoutFallbackHandler", () => { + let mockApiHandler: MockApiHandler + let mockTask: Partial + + beforeEach(() => { + vitest.clearAllMocks() + + // Mock API handler that simulates real AI responses + mockApiHandler = { + createMessage: vitest.fn(), + getModel: vitest.fn().mockReturnValue({ id: "test-model", info: { maxTokens: 4096 } }), + countTokens: vitest.fn().mockResolvedValue(100), + completePrompt: vitest.fn(), + } + + // Mock task with API handler + mockTask = { + api: mockApiHandler, + assistantMessageContent: [], + cwd: "/test/dir", + say: vitest.fn(), + } + }) + + describe("AI Fallback Generation", () => { + test("should use AI to generate contextual suggestions when available", async () => { + // Mock AI response with numbered suggestions + const mockAiResponse = `Here are some suggestions for the timeout: + +1. Break the npm install command into smaller package installations +2. Clear npm cache and try again with npm cache clean --force +3. Use npm install --no-optional to skip optional dependencies +4. Check network connectivity and try with different registry` + + ;(mockApiHandler.completePrompt as any).mockResolvedValueOnce(mockAiResponse) + + const context = { + toolName: "execute_command" as const, + timeoutMs: 30000, + executionTimeMs: 35000, + toolParams: { command: "npm install" }, + } + + const result = await TimeoutFallbackHandler.generateAiFallback(context, mockTask as Task) + + expect(result.success).toBe(true) + expect(result.toolCall).toBeDefined() + expect(result.toolCall?.name).toBe("ask_followup_question") + expect(result.toolCall?.params.question).toContain("execute_command") + expect(result.toolCall?.params.question).toContain("30 seconds") + + // Check that AI-generated suggestions are included + const followUp = result.toolCall?.params.follow_up || "" + expect(followUp).toContain("Break the npm install command into smaller package installations") + expect(followUp).toContain("Clear npm cache and try again") + expect(followUp).toContain("Use npm install --no-optional") + expect(followUp).toContain("Check network connectivity") + + // Verify AI was called with proper prompt + expect(mockApiHandler.completePrompt).toHaveBeenCalledWith( + expect.stringContaining("execute_command operation has timed out"), + ) + expect(mockApiHandler.completePrompt).toHaveBeenCalledWith(expect.stringContaining("npm install")) + }) + + test("should fallback to static suggestions when AI fails", async () => { + // Mock AI failure + ;(mockApiHandler.completePrompt as any).mockRejectedValueOnce(new Error("API Error")) + + const context = { + toolName: "execute_command" as const, + timeoutMs: 30000, + executionTimeMs: 35000, + toolParams: { command: "npm test" }, + } + + const result = await TimeoutFallbackHandler.generateAiFallback(context, mockTask as Task) + + expect(result.success).toBe(true) + expect(result.toolCall).toBeDefined() + expect(result.toolCall?.name).toBe("ask_followup_question") + + // Should contain static fallback suggestions + const followUp = result.toolCall?.params.follow_up || "" + expect(followUp).toContain('Break "npm test" into smaller') + expect(followUp).toContain("background using") + expect(followUp).toContain("alternative approach") + expect(followUp).toContain("Increase the timeout") + }) + + test("should fallback to static suggestions when API handler is unavailable", async () => { + // Task without API handler + const taskWithoutApi = {} + + const context = { + toolName: "read_file" as const, + timeoutMs: 5000, + executionTimeMs: 6000, + toolParams: { path: "/large/file.txt" }, + } + + const result = await TimeoutFallbackHandler.generateAiFallback(context, taskWithoutApi as Task) + + expect(result.success).toBe(true) + expect(result.toolCall).toBeDefined() + + // Should contain static fallback suggestions for read_file + const followUp = result.toolCall?.params.follow_up || "" + expect(followUp).toContain('Read "/large/file.txt" in smaller chunks') + expect(followUp).toContain("accessible and not locked") + }) + + test("should parse AI response with different numbering formats", async () => { + // Test different numbering formats + const mockAiResponse = `Here are the suggestions: + +1) Try breaking the command into parts +2. Use a different approach +3) Check system resources +4. Increase timeout duration` + + ;(mockApiHandler.completePrompt as any).mockResolvedValueOnce(mockAiResponse) + + const context = { + toolName: "execute_command" as const, + timeoutMs: 10000, + executionTimeMs: 12000, + toolParams: { command: "build script" }, + } + + const result = await TimeoutFallbackHandler.generateAiFallback(context, mockTask as Task) + + expect(result.success).toBe(true) + const followUp = result.toolCall?.params.follow_up || "" + expect(followUp).toContain("Try breaking the command into parts") + expect(followUp).toContain("Use a different approach") + expect(followUp).toContain("Check system resources") + expect(followUp).toContain("Increase timeout duration") + }) + + test("should handle AI response without numbered list", async () => { + // AI response without clear numbering + const mockAiResponse = `You could try splitting the operation. Another option is to check the network. Maybe increase the timeout. Consider using a different tool.` + + ;(mockApiHandler.completePrompt as any).mockResolvedValueOnce(mockAiResponse) + + const context = { + toolName: "browser_action" as const, + timeoutMs: 15000, + executionTimeMs: 16000, + toolParams: { action: "click" }, + } + + const result = await TimeoutFallbackHandler.generateAiFallback(context, mockTask as Task) + + expect(result.success).toBe(true) + const followUp = result.toolCall?.params.follow_up || "" + + // Should extract sentences as suggestions + expect(followUp).toContain("You could try splitting the operation") + expect(followUp).toContain("Another option is to check the network") + }) + + test("should include task context in AI prompt when available", async () => { + const mockAiResponse = `1. Try a different approach\n2. Check the working directory\n3. Break into steps` + ;(mockApiHandler.completePrompt as any).mockResolvedValueOnce(mockAiResponse) + + const context = { + toolName: "search_files" as const, + timeoutMs: 20000, + executionTimeMs: 22000, + toolParams: { path: "/project", regex: ".*\\.ts$" }, + taskContext: { + currentStep: "Finding TypeScript files", + workingDirectory: "/project/src", + previousActions: ["read package.json", "list files"], + }, + } + + const result = await TimeoutFallbackHandler.generateAiFallback(context, mockTask as Task) + + expect(result.success).toBe(true) + + // Verify the prompt included task context + const calledPrompt = (mockApiHandler.completePrompt as any).mock.calls[0][0] + expect(calledPrompt).toContain("Current step: Finding TypeScript files") + expect(calledPrompt).toContain("Working directory: /project/src") + expect(calledPrompt).toContain("search_files") + expect(calledPrompt).toContain("ts$") // Just check for the pattern ending + }) + + test("should limit suggestions to maximum of 4", async () => { + // AI response with many suggestions + const mockAiResponse = `Here are many suggestions: + +1. First suggestion +2. Second suggestion +3. Third suggestion +4. Fourth suggestion +5. Fifth suggestion +6. Sixth suggestion +7. Seventh suggestion` + + ;(mockApiHandler.completePrompt as any).mockResolvedValueOnce(mockAiResponse) + + const context = { + toolName: "write_to_file" as const, + timeoutMs: 8000, + executionTimeMs: 9000, + toolParams: { path: "/output.txt" }, + } + + const result = await TimeoutFallbackHandler.generateAiFallback(context, mockTask as Task) + + expect(result.success).toBe(true) + const followUp = result.toolCall?.params.follow_up || "" + + // Count the number of tags + const suggestCount = (followUp.match(//g) || []).length + expect(suggestCount).toBeLessThanOrEqual(4) + + // Should include first 4 suggestions + expect(followUp).toContain("First suggestion") + expect(followUp).toContain("Fourth suggestion") + // Should not include 5th and beyond + expect(followUp).not.toContain("Fifth suggestion") + }) + }) + + describe("End-to-End AI Tests", () => { + test("should generate realistic AI suggestions for execute_command timeout", async () => { + // Create a realistic mock API handler + const testApiHandler: MockApiHandler = { + createMessage: vitest.fn(), + getModel: vitest.fn().mockReturnValue({ id: "claude-3-sonnet", info: { maxTokens: 4096 } }), + countTokens: vitest.fn().mockResolvedValue(150), + completePrompt: vitest.fn().mockResolvedValue( + ` +Here are some suggestions for the npm install timeout: + +1. Clear npm cache with "npm cache clean --force" and retry +2. Break installation into smaller chunks by installing packages individually +3. Use "npm install --no-optional" to skip optional dependencies +4. Check network connectivity and try with a different registry + `.trim(), + ), + } + + const testTask: Partial = { + api: testApiHandler, + } + + const context = { + toolName: "execute_command" as const, + timeoutMs: 60000, + executionTimeMs: 65000, + toolParams: { + command: "npm install", + cwd: "/project", + }, + } + + const result = await TimeoutFallbackHandler.generateAiFallback(context, testTask as Task) + + // Verify the result structure + expect(result.success).toBe(true) + expect(result.toolCall).toBeDefined() + expect(result.toolCall?.name).toBe("ask_followup_question") + expect(result.toolCall?.params.question).toContain("execute_command") + expect(result.toolCall?.params.question).toContain("60 seconds") + + // Verify AI-generated suggestions are included + const followUp = result.toolCall?.params.follow_up || "" + expect(followUp).toContain("Clear npm cache") + expect(followUp).toContain("Break installation into smaller chunks") + expect(followUp).toContain("no-optional") + expect(followUp).toContain("network connectivity") + + // Verify the AI was called with a proper prompt + expect(testApiHandler.completePrompt).toHaveBeenCalledWith( + expect.stringContaining("execute_command operation has timed out"), + ) + expect(testApiHandler.completePrompt).toHaveBeenCalledWith(expect.stringContaining("npm install")) + expect(testApiHandler.completePrompt).toHaveBeenCalledWith(expect.stringContaining("60 seconds")) + }) + + test("should handle AI response with different formatting", async () => { + // Mock AI response with different numbering style + const testApiHandler: MockApiHandler = { + createMessage: vitest.fn(), + getModel: vitest.fn().mockReturnValue({ id: "gpt-4", info: { maxTokens: 8192 } }), + countTokens: vitest.fn().mockResolvedValue(200), + completePrompt: vitest.fn().mockResolvedValue( + ` +Based on the search_files timeout, here are my recommendations: + +• Limit search to specific subdirectories instead of entire project +• Use more specific regex patterns to reduce matches +• Try list_files first to understand directory structure +• Consider breaking search into multiple smaller operations + `.trim(), + ), + } + + const testTask: Partial = { + api: testApiHandler, + } + + const context = { + toolName: "search_files" as const, + timeoutMs: 30000, + executionTimeMs: 32000, + toolParams: { + path: "/large-project", + regex: ".*", + file_pattern: "*.ts", + }, + } + + const result = await TimeoutFallbackHandler.generateAiFallback(context, testTask as Task) + + expect(result.success).toBe(true) + expect(result.toolCall).toBeDefined() + + // Should extract suggestions even with bullet points + const followUp = result.toolCall?.params.follow_up || "" + expect(followUp).toContain("Narrow the search scope") + expect(followUp).toContain("simpler search patterns") + expect(followUp).toContain("file type filters") + expect(followUp).toContain("incrementally in smaller batches") + }) + + test("should gracefully handle AI failure and use static fallback", async () => { + // Mock API handler that fails + const testApiHandler: MockApiHandler = { + createMessage: vitest.fn(), + getModel: vitest.fn().mockReturnValue({ id: "test-model", info: { maxTokens: 4096 } }), + countTokens: vitest.fn().mockResolvedValue(100), + completePrompt: vitest.fn().mockRejectedValue(new Error("API rate limit exceeded")), + } + + const testTask: Partial = { + api: testApiHandler, + } + + const context = { + toolName: "read_file" as const, + timeoutMs: 10000, + executionTimeMs: 12000, + toolParams: { + path: "/very/large/file.log", + }, + } + + const result = await TimeoutFallbackHandler.generateAiFallback(context, testTask as Task) + + expect(result.success).toBe(true) + expect(result.toolCall).toBeDefined() + + // Should contain static fallback suggestions for read_file + const followUp = result.toolCall?.params.follow_up || "" + expect(followUp).toContain('Read "/very/large/file.log" in smaller chunks') + expect(followUp).toContain("accessible and not locked") + expect(followUp).toContain("different approach") + expect(followUp).toContain("Increase the timeout") + + // Verify AI was attempted but failed gracefully + expect(testApiHandler.completePrompt).toHaveBeenCalled() + }) + + test("should work without task API handler", async () => { + // Task without API handler + const testTask: Partial = {} + + const context = { + toolName: "browser_action" as const, + timeoutMs: 15000, + executionTimeMs: 16500, + toolParams: { + action: "click", + coordinate: "450,300", + }, + } + + const result = await TimeoutFallbackHandler.generateAiFallback(context, testTask as Task) + + expect(result.success).toBe(true) + expect(result.toolCall).toBeDefined() + + // Should contain static fallback suggestions for browser_action + const followUp = result.toolCall?.params.follow_up || "" + expect(followUp).toContain('Simplify the "click"') + expect(followUp).toContain("Wait for specific elements") + expect(followUp).toContain("direct API calls") + expect(followUp).toContain("Reset the browser session") + }) + }) + + describe("Tool Call Response Generation", () => { + test("should return response with ask_followup_question tool instructions", async () => { + // Mock the TimeoutFallbackHandler to return a successful AI result + const mockAiResult: TimeoutFallbackResult = { + success: true, + toolCall: { + name: "ask_followup_question", + params: { + question: + "The execute_command operation timed out after 5 seconds. How would you like to proceed?", + follow_up: + "Try a different approach\nBreak into smaller steps", + }, + }, + } + + // Spy on the generateAiFallback method + const generateSpy = vitest.spyOn(TimeoutFallbackHandler, "generateAiFallback") + generateSpy.mockResolvedValue(mockAiResult) + + // Call createTimeoutResponse + const response = await TimeoutFallbackHandler.createTimeoutResponse( + "execute_command", + 5000, + 6000, + { command: "npm install" }, + mockTask as Task, + ) + + // Check that the response contains the base timeout message + expect(response).toContain("timed out after 5 seconds") + expect(response).toContain("Execution Time: 6s") + + // Check that the response includes instructions to use ask_followup_question + expect(response).toContain("You MUST now use the ask_followup_question tool") + expect(response).toContain("") + expect(response).toContain(`${mockAiResult.toolCall?.params.question}`) + expect(response).toContain("") + expect(response).toContain(mockAiResult.toolCall?.params.follow_up) + expect(response).toContain("") + expect(response).toContain("") + expect(response).toContain("This is required to help the user decide how to proceed after the timeout.") + + // Verify that assistantMessageContent was NOT modified + expect(mockTask.assistantMessageContent).toHaveLength(0) + + generateSpy.mockRestore() + }) + + test("should return fallback message when AI generation fails", async () => { + // Spy on the generateAiFallback method to return a failure + const generateSpy = vitest.spyOn(TimeoutFallbackHandler, "generateAiFallback") + generateSpy.mockResolvedValue({ + success: false, + error: "AI generation failed", + }) + + // Call createTimeoutResponse + const response = await TimeoutFallbackHandler.createTimeoutResponse( + "execute_command", + 5000, + 6000, + { command: "npm install" }, + mockTask as Task, + ) + + // Check that the response contains the base timeout message + expect(response).toContain("timed out after 5 seconds") + expect(response).toContain("Execution Time: 6s") + + // Check that the response contains the fallback message + expect(response).toContain( + "The operation timed out. Please consider breaking this into smaller steps or trying a different approach.", + ) + + // Should not contain ask_followup_question instructions + expect(response).not.toContain("ask_followup_question") + + generateSpy.mockRestore() + }) + }) + + describe("UI Integration", () => { + test("should generate AI fallbacks using static method", async () => { + const context = { + toolName: "execute_command" as const, + timeoutMs: 30000, + executionTimeMs: 25000, + toolParams: { command: "npm install" }, + } + + const result = await TimeoutFallbackHandler.generateAiFallback(context) + + expect(result.success).toBe(true) + expect(result.toolCall).toBeDefined() + expect(result.toolCall?.name).toBe("ask_followup_question") + expect(result.toolCall?.params.question).toContain("execute_command") + }) + + test("should create timeout response with AI fallbacks", async () => { + const response = await TimeoutFallbackHandler.createTimeoutResponse("execute_command", 30000, 25000, { + command: "npm install", + }) + + expect(response).toContain("execute_command") + expect(response).toContain("timed out") + expect(response.length).toBeGreaterThan(100) // Should contain substantial content + }) + + test("should handle different tool types with AI fallbacks", async () => { + const commandResponse = await TimeoutFallbackHandler.createTimeoutResponse( + "execute_command", + 30000, + 25000, + { + command: "npm test", + }, + ) + + const browserResponse = await TimeoutFallbackHandler.createTimeoutResponse("browser_action", 30000, 25000, { + action: "click", + }) + + expect(commandResponse).toContain("execute_command") + expect(browserResponse).toContain("browser_action") + expect(commandResponse).not.toEqual(browserResponse) + }) + + test("should validate UI setting flow", () => { + // This test validates that timeout settings can be toggled + const settings = { + timeoutFallbackEnabled: true, + toolExecutionTimeoutMs: 30000, + } + + // Simulate UI toggle + settings.timeoutFallbackEnabled = false + expect(settings.timeoutFallbackEnabled).toBe(false) + + // Simulate timeout duration change + settings.toolExecutionTimeoutMs = 60000 + expect(settings.toolExecutionTimeoutMs).toBe(60000) + }) + }) +}) diff --git a/src/core/timeout/__tests__/timeout-integration.spec.ts b/src/core/timeout/__tests__/timeout-integration.spec.ts index 5bd741e51c..acc9161486 100644 --- a/src/core/timeout/__tests__/timeout-integration.spec.ts +++ b/src/core/timeout/__tests__/timeout-integration.spec.ts @@ -11,163 +11,264 @@ describe("Timeout Integration Tests", () => { vitest.clearAllMocks() }) - test("TimeoutManager should handle basic timeout operations", async () => { - const manager = TimeoutManager.getInstance() + describe("TimeoutManager Integration", () => { + test("should handle basic timeout operations", async () => { + const manager = TimeoutManager.getInstance() - // Test successful operation within timeout - const result = await manager.executeWithTimeout( - async () => { + // Test successful operation within timeout + const result = await manager.executeWithTimeout( + async () => { + await new Promise((resolve) => setTimeout(resolve, 10)) + return "success" + }, + { + toolName: "execute_command", + timeoutMs: 100, + enableFallback: true, + }, + ) + + expect(result.success).toBe(true) + expect(result.result).toBe("success") + expect(result.timedOut).toBe(false) + }) + + test("should handle timeout scenarios", async () => { + const manager = TimeoutManager.getInstance() + + // Test operation that times out + const result = await manager.executeWithTimeout( + async () => { + await new Promise((resolve) => setTimeout(resolve, 200)) + return "should not reach here" + }, + { + toolName: "execute_command", + timeoutMs: 50, + enableFallback: true, + }, + ) + + expect(result.success).toBe(false) + expect(result.timedOut).toBe(true) + expect(result.error?.message).toContain("Operation timed out") + }) + }) + + describe("ToolExecutionWrapper Integration", () => { + test("should wrap operations correctly", async () => { + const mockOperation = vitest.fn().mockImplementation(async (signal: AbortSignal) => { + // Simulate checking abort signal + if (signal.aborted) { + throw new Error("Operation was aborted") + } await new Promise((resolve) => setTimeout(resolve, 10)) - return "success" - }, - { - toolName: "execute_command", - timeoutMs: 100, - enableFallback: true, - }, - ) + return [false, "test result"] + }) - expect(result.success).toBe(true) - expect(result.result).toBe("success") - expect(result.timedOut).toBe(false) - }) + const result = await ToolExecutionWrapper.execute( + mockOperation, + { + toolName: "execute_command", + taskId: "test-task", + timeoutMs: 100, + enableFallback: true, + }, + 100, + ) - test("TimeoutManager should handle timeout scenarios", async () => { - const manager = TimeoutManager.getInstance() + expect(result.success).toBe(true) + expect(result.result).toEqual([false, "test result"]) + expect(mockOperation).toHaveBeenCalledWith(expect.any(AbortSignal)) + }) - // Test operation that times out - const result = await manager.executeWithTimeout( - async () => { + test("should handle timeout with fallback", async () => { + const mockOperation = vitest.fn().mockImplementation(async () => { await new Promise((resolve) => setTimeout(resolve, 200)) - return "should not reach here" - }, - { - toolName: "execute_command", - timeoutMs: 50, - enableFallback: true, - }, - ) + return [false, "should not reach here"] + }) - expect(result.success).toBe(false) - expect(result.timedOut).toBe(true) - expect(result.error?.message).toContain("Operation timed out") - }) + const result = await ToolExecutionWrapper.execute( + mockOperation, + { + toolName: "execute_command", + taskId: "test-task", + timeoutMs: 50, + enableFallback: true, + }, + 50, + ) - test("ToolExecutionWrapper should wrap operations correctly", async () => { - const mockOperation = vitest.fn().mockImplementation(async (signal: AbortSignal) => { - // Simulate checking abort signal - if (signal.aborted) { - throw new Error("Operation was aborted") - } - await new Promise((resolve) => setTimeout(resolve, 10)) - return [false, "test result"] + expect(result.success).toBe(false) + expect(result.timedOut).toBe(true) + expect(result.fallbackTriggered).toBe(true) }) - - const result = await ToolExecutionWrapper.execute( - mockOperation, - { - toolName: "execute_command", - taskId: "test-task", - timeoutMs: 100, - enableFallback: true, - }, - 100, - ) - - expect(result.success).toBe(true) - expect(result.result).toEqual([false, "test result"]) - expect(mockOperation).toHaveBeenCalledWith(expect.any(AbortSignal)) }) - test("ToolExecutionWrapper should handle timeout with fallback", async () => { - const mockOperation = vitest.fn().mockImplementation(async () => { - await new Promise((resolve) => setTimeout(resolve, 200)) - return [false, "should not reach here"] + describe("TimeoutFallbackHandler Integration", () => { + test("should create AI-powered responses", async () => { + // Create a mock task to test tool injection + const mockTask = { + assistantMessageContent: [], + cwd: "/test/dir", + say: vitest.fn().mockResolvedValue(undefined), + } as unknown as Task + + const response = await TimeoutFallbackHandler.createTimeoutResponse( + "execute_command", + 5000, + 6000, + { command: "npm install" }, + mockTask, + ) + + // The response should contain the basic timeout information + expect(response).toContain("execute_command") + expect(response).toContain("5 seconds") + expect(response).toContain("6s") + expect(response.length).toBeGreaterThan(50) + + // The response should now contain instructions to use ask_followup_question + expect(response).toContain("You MUST now use the ask_followup_question tool") + expect(response).toContain("") + expect(response).toContain("") + expect(response).toContain("timed out") + expect(response).toContain("") + expect(response).toContain("") + expect(response).toContain("") + expect(response).toContain("") + + // Verify that assistantMessageContent was NOT modified + expect(mockTask.assistantMessageContent).toHaveLength(0) }) - - const result = await ToolExecutionWrapper.execute( - mockOperation, - { - toolName: "execute_command", - taskId: "test-task", - timeoutMs: 50, - enableFallback: true, - }, - 50, - ) - - expect(result.success).toBe(false) - expect(result.timedOut).toBe(true) - expect(result.fallbackTriggered).toBe(true) }) - test("TimeoutFallbackHandler should create AI-powered responses", async () => { - // Create a mock task to test tool injection - const mockTask = { - assistantMessageContent: [], - cwd: "/test/dir", - say: vitest.fn().mockResolvedValue(undefined), - } as unknown as Task + describe("AbortSignal Integration", () => { + test("should be properly handled", async () => { + const mockOperation = vitest.fn().mockImplementation(async (signal: AbortSignal) => { + // Simulate a long-running operation that checks abort signal + return new Promise((resolve, reject) => { + const timeout = setTimeout(() => { + if (signal.aborted) { + reject(new Error("Operation was aborted")) + } else { + resolve("success") + } + }, 100) - const response = await TimeoutFallbackHandler.createTimeoutResponse( - "execute_command", - 5000, - 6000, - { command: "npm install" }, - mockTask, - ) - - // The response should contain the basic timeout information - expect(response).toContain("execute_command") - expect(response).toContain("5 seconds") - expect(response).toContain("6s") - expect(response.length).toBeGreaterThan(50) - - // The response should now contain instructions to use ask_followup_question - expect(response).toContain("You MUST now use the ask_followup_question tool") - expect(response).toContain("") - expect(response).toContain("") - expect(response).toContain("timed out") - expect(response).toContain("") - expect(response).toContain("") - expect(response).toContain("") - expect(response).toContain("") - - // Verify that assistantMessageContent was NOT modified - expect(mockTask.assistantMessageContent).toHaveLength(0) - }) - - test("AbortSignal should be properly handled", async () => { - const mockOperation = vitest.fn().mockImplementation(async (signal: AbortSignal) => { - // Simulate a long-running operation that checks abort signal - return new Promise((resolve, reject) => { - const timeout = setTimeout(() => { - if (signal.aborted) { + signal.addEventListener("abort", () => { + clearTimeout(timeout) reject(new Error("Operation was aborted")) - } else { - resolve("success") - } - }, 100) - - signal.addEventListener("abort", () => { - clearTimeout(timeout) - reject(new Error("Operation was aborted")) + }) }) }) + + const result = await ToolExecutionWrapper.execute( + mockOperation, + { + toolName: "execute_command", + taskId: "test-task", + timeoutMs: 50, // Shorter timeout to trigger abort + enableFallback: false, + }, + 50, + ) + + expect(result.success).toBe(false) + expect(result.timedOut).toBe(true) + }) + }) + + describe("Cross-Component Integration", () => { + test("should coordinate between TimeoutManager and ToolExecutionWrapper", async () => { + const manager = TimeoutManager.getInstance() + + const mockOperation = vitest.fn().mockImplementation(async (signal: AbortSignal) => { + // Simulate operation that respects abort signal + await new Promise((resolve, reject) => { + const timeout = setTimeout(resolve, 150) + signal.addEventListener("abort", () => { + clearTimeout(timeout) + reject(new Error("Operation was aborted")) + }) + }) + return [false, "completed"] + }) + + // Use ToolExecutionWrapper directly + const result = await ToolExecutionWrapper.execute( + mockOperation, + { + toolName: "read_file", + taskId: "integration-test", + timeoutMs: 100, + enableFallback: true, + }, + 100, + ) + + expect(result.success).toBe(false) + expect(result.timedOut).toBe(true) }) - const result = await ToolExecutionWrapper.execute( - mockOperation, - { - toolName: "execute_command", - taskId: "test-task", - timeoutMs: 50, // Shorter timeout to trigger abort - enableFallback: false, - }, - 50, - ) + test("should handle nested timeout scenarios", async () => { + const manager = TimeoutManager.getInstance() - expect(result.success).toBe(false) - expect(result.timedOut).toBe(true) + // Test nested timeout operations + const outerResult = await manager.executeWithTimeout( + async () => { + // Inner timeout operation + return await manager.executeWithTimeout( + async () => { + await new Promise((resolve) => setTimeout(resolve, 150)) + return "inner success" + }, + { + toolName: "search_files", + timeoutMs: 100, + enableFallback: true, + }, + ) + }, + { + toolName: "execute_command", + timeoutMs: 200, + enableFallback: true, + }, + ) + + // The inner operation should timeout, but the outer should succeed with the timeout result + expect(outerResult.success).toBe(true) + if (outerResult.result) { + expect(outerResult.result.success).toBe(false) + expect(outerResult.result.timedOut).toBe(true) + } + }) + + test("should maintain timeout event tracking across components", async () => { + const manager = TimeoutManager.getInstance() + manager.clearLastTimeoutEvent() + + // Execute operation that will timeout + await manager.executeWithTimeout( + async () => { + await new Promise((resolve) => setTimeout(resolve, 200)) + return "should timeout" + }, + { + toolName: "browser_action", + timeoutMs: 50, + enableFallback: true, + taskId: "tracking-test", + }, + ) + + // Verify timeout event was tracked + const timeoutEvent = manager.getLastTimeoutEvent() + expect(timeoutEvent).toBeTruthy() + expect(timeoutEvent?.toolName).toBe("browser_action") + expect(timeoutEvent?.taskId).toBe("tracking-test") + expect(timeoutEvent?.timeoutMs).toBe(50) + }) }) }) diff --git a/src/core/timeout/__tests__/timeout-manager.spec.ts b/src/core/timeout/__tests__/timeout-manager.spec.ts new file mode 100644 index 0000000000..5b218d19f5 --- /dev/null +++ b/src/core/timeout/__tests__/timeout-manager.spec.ts @@ -0,0 +1,178 @@ +// npx vitest run src/core/timeout/__tests__/timeout-manager.spec.ts + +import { describe, test, expect, beforeEach, vitest } from "vitest" +import { TimeoutManager } from "../TimeoutManager" + +describe("TimeoutManager", () => { + let manager: TimeoutManager + + beforeEach(() => { + manager = TimeoutManager.getInstance() + manager.clearLastTimeoutEvent() + vitest.clearAllMocks() + }) + + describe("Basic Operations", () => { + test("should handle successful operation within timeout", async () => { + const result = await manager.executeWithTimeout( + async () => { + await new Promise((resolve) => setTimeout(resolve, 10)) + return "success" + }, + { + toolName: "execute_command", + timeoutMs: 100, + enableFallback: true, + }, + ) + + expect(result.success).toBe(true) + expect(result.result).toBe("success") + expect(result.timedOut).toBe(false) + }) + + test("should handle timeout scenarios", async () => { + const result = await manager.executeWithTimeout( + async () => { + await new Promise((resolve) => setTimeout(resolve, 200)) + return "should not reach here" + }, + { + toolName: "execute_command", + timeoutMs: 50, + enableFallback: true, + }, + ) + + expect(result.success).toBe(false) + expect(result.timedOut).toBe(true) + expect(result.error?.message).toContain("Operation timed out") + }) + }) + + describe("Timeout Event Tracking", () => { + test("should track only the last timeout event", async () => { + // First timeout + await manager.executeWithTimeout( + async () => { + await new Promise((resolve) => setTimeout(resolve, 200)) + return "should timeout" + }, + { + toolName: "execute_command", + timeoutMs: 50, + enableFallback: true, + taskId: "task-1", + }, + ) + + const firstTimeout = manager.getLastTimeoutEvent() + expect(firstTimeout).toBeTruthy() + expect(firstTimeout?.toolName).toBe("execute_command") + expect(firstTimeout?.taskId).toBe("task-1") + + // Second timeout should replace the first + await manager.executeWithTimeout( + async () => { + await new Promise((resolve) => setTimeout(resolve, 200)) + return "should timeout" + }, + { + toolName: "read_file", + timeoutMs: 50, + enableFallback: true, + taskId: "task-2", + }, + ) + + const secondTimeout = manager.getLastTimeoutEvent() + expect(secondTimeout).toBeTruthy() + expect(secondTimeout?.toolName).toBe("read_file") + expect(secondTimeout?.taskId).toBe("task-2") + expect(secondTimeout?.timestamp).toBeGreaterThan(firstTimeout!.timestamp) + }) + + test("should clear last timeout event", async () => { + // Create a timeout + await manager.executeWithTimeout( + async () => { + await new Promise((resolve) => setTimeout(resolve, 200)) + return "should timeout" + }, + { + toolName: "execute_command", + timeoutMs: 50, + enableFallback: true, + }, + ) + + expect(manager.getLastTimeoutEvent()).toBeTruthy() + + // Clear it + manager.clearLastTimeoutEvent() + expect(manager.getLastTimeoutEvent()).toBeNull() + }) + + test("should return null when no timeout has occurred", () => { + expect(manager.getLastTimeoutEvent()).toBeNull() + }) + }) + + describe("Operation Management", () => { + test("cancelOperation should work with simplified operation IDs", async () => { + // Start a long-running operation + const operationPromise = manager.executeWithTimeout( + async (signal) => { + await new Promise((resolve, reject) => { + const timeout = setTimeout(resolve, 1000) + signal.addEventListener("abort", () => { + clearTimeout(timeout) + reject(new Error("Operation was aborted")) + }) + }) + return "should be cancelled" + }, + { + toolName: "execute_command", + timeoutMs: 2000, + enableFallback: true, + taskId: "cancel-test", + }, + ) + + // Cancel it immediately + const cancelled = manager.cancelOperation("execute_command", "cancel-test") + expect(cancelled).toBe(true) + + // Verify it was cancelled + const result = await operationPromise + expect(result.success).toBe(false) + expect(result.error?.message).toContain("Operation was aborted") + }) + + test("isOperationActive should work with simplified operation IDs", async () => { + // Start an operation + const operationPromise = manager.executeWithTimeout( + async () => { + await new Promise((resolve) => setTimeout(resolve, 100)) + return "success" + }, + { + toolName: "read_file", + timeoutMs: 200, + enableFallback: true, + taskId: "active-test", + }, + ) + + // Check if it's active + expect(manager.isOperationActive("read_file", "active-test")).toBe(true) + + // Wait for completion + await operationPromise + + // Should no longer be active + expect(manager.isOperationActive("read_file", "active-test")).toBe(false) + }) + }) +}) diff --git a/src/core/timeout/__tests__/tool-injection-test.spec.ts b/src/core/timeout/__tests__/tool-injection-test.spec.ts deleted file mode 100644 index 49c9d0d225..0000000000 --- a/src/core/timeout/__tests__/tool-injection-test.spec.ts +++ /dev/null @@ -1,98 +0,0 @@ -import { describe, test, expect, vi, beforeEach } from "vitest" -import { TimeoutFallbackHandler } from "../TimeoutFallbackHandler" -import { type TimeoutFallbackResult } from "../TimeoutFallbackHandler" -import { Task } from "../../task/Task" - -// Import the real module first -import { TimeoutFallbackHandler as RealTimeoutFallbackHandler } from "../TimeoutFallbackHandler" - -// Mock only the generateAiFallback method -vi.spyOn(RealTimeoutFallbackHandler, "generateAiFallback") - -describe("Tool Call Response Test", () => { - let mockTask: Task - - beforeEach(() => { - // Create a minimal mock task with assistantMessageContent array - mockTask = { - assistantMessageContent: [], - cwd: "/test/dir", - say: vi.fn(), - } as unknown as Task - - vi.clearAllMocks() - }) - - test("should return response with ask_followup_question tool instructions", async () => { - // Mock the TimeoutFallbackHandler to return a successful AI result - const mockAiResult: TimeoutFallbackResult = { - success: true, - toolCall: { - name: "ask_followup_question", - params: { - question: "The execute_command operation timed out after 5 seconds. How would you like to proceed?", - follow_up: - "Try a different approach\nBreak into smaller steps", - }, - }, - } - - // Mock the generateAiFallback method - vi.mocked(RealTimeoutFallbackHandler.generateAiFallback).mockResolvedValue(mockAiResult) - - // Call createTimeoutResponse - const response = await TimeoutFallbackHandler.createTimeoutResponse( - "execute_command", - 5000, - 6000, - { command: "npm install" }, - mockTask, - ) - - // Check that the response contains the base timeout message - expect(response).toContain("timed out after 5 seconds") - expect(response).toContain("Execution Time: 6s") - - // Check that the response includes instructions to use ask_followup_question - expect(response).toContain("You MUST now use the ask_followup_question tool") - expect(response).toContain("") - expect(response).toContain(`${mockAiResult.toolCall?.params.question}`) - expect(response).toContain("") - expect(response).toContain(mockAiResult.toolCall?.params.follow_up) - expect(response).toContain("") - expect(response).toContain("") - expect(response).toContain("This is required to help the user decide how to proceed after the timeout.") - - // Verify that assistantMessageContent was NOT modified - expect(mockTask.assistantMessageContent).toHaveLength(0) - }) - - test("should return fallback message when AI generation fails", async () => { - // Mock the generateAiFallback to return a failure - vi.mocked(RealTimeoutFallbackHandler.generateAiFallback).mockResolvedValue({ - success: false, - error: "AI generation failed", - }) - - // Call createTimeoutResponse - const response = await TimeoutFallbackHandler.createTimeoutResponse( - "execute_command", - 5000, - 6000, - { command: "npm install" }, - mockTask, - ) - - // Check that the response contains the base timeout message - expect(response).toContain("timed out after 5 seconds") - expect(response).toContain("Execution Time: 6s") - - // Check that the response contains the fallback message - expect(response).toContain( - "The operation timed out. Please consider breaking this into smaller steps or trying a different approach.", - ) - - // Should not contain ask_followup_question instructions - expect(response).not.toContain("ask_followup_question") - }) -}) diff --git a/src/core/timeout/__tests__/ui-integration.spec.ts b/src/core/timeout/__tests__/ui-integration.spec.ts deleted file mode 100644 index 8065aa12ed..0000000000 --- a/src/core/timeout/__tests__/ui-integration.spec.ts +++ /dev/null @@ -1,70 +0,0 @@ -import { describe, it, expect, vi } from "vitest" -import { TimeoutFallbackHandler } from "../TimeoutFallbackHandler" - -describe("UI Integration - AI Timeout Fallbacks", () => { - it("should generate AI fallbacks using static method", async () => { - const context = { - toolName: "execute_command" as const, - timeoutMs: 30000, - executionTimeMs: 25000, - toolParams: { command: "npm install" }, - } - - const result = await TimeoutFallbackHandler.generateAiFallback(context) - - expect(result.success).toBe(true) - expect(result.toolCall).toBeDefined() - expect(result.toolCall?.name).toBe("ask_followup_question") - expect(result.toolCall?.params.question).toContain("execute_command") - }) - - it("should create timeout response with AI fallbacks", async () => { - const response = await TimeoutFallbackHandler.createTimeoutResponse("execute_command", 30000, 25000, { - command: "npm install", - }) - - expect(response).toContain("execute_command") - expect(response).toContain("timed out") - expect(response.length).toBeGreaterThan(100) // Should contain substantial content - }) - - it("should create timeout response when AI fallbacks fail", async () => { - const response = await TimeoutFallbackHandler.createTimeoutResponse("execute_command", 30000, 25000, { - command: "npm install", - }) - - expect(response).toContain("execute_command") - expect(response).toContain("timed out") - expect(response.length).toBeGreaterThan(50) // Should contain basic timeout message - }) - - it("should validate UI setting flow", () => { - // This test validates that timeout settings can be toggled - const settings = { - timeoutFallbackEnabled: true, - toolExecutionTimeoutMs: 30000, - } - - // Simulate UI toggle - settings.timeoutFallbackEnabled = false - expect(settings.timeoutFallbackEnabled).toBe(false) - - // Simulate timeout duration change - settings.toolExecutionTimeoutMs = 60000 - expect(settings.toolExecutionTimeoutMs).toBe(60000) - }) - - it("should handle different tool types with AI fallbacks", async () => { - const commandResponse = await TimeoutFallbackHandler.createTimeoutResponse("execute_command", 30000, 25000, { - command: "npm test", - }) - - const browserResponse = await TimeoutFallbackHandler.createTimeoutResponse("browser_action", 30000, 25000, { - action: "click", - }) - - expect(commandResponse).toContain("execute_command") - expect(browserResponse).toContain("browser_action") - expect(commandResponse).not.toEqual(browserResponse) - }) -}) diff --git a/src/core/tools/executeCommandTool.ts b/src/core/tools/executeCommandTool.ts index f1ca671143..fcbf74e7b2 100644 --- a/src/core/tools/executeCommandTool.ts +++ b/src/core/tools/executeCommandTool.ts @@ -16,6 +16,8 @@ import { TerminalRegistry } from "../../integrations/terminal/TerminalRegistry" import { Terminal } from "../../integrations/terminal/Terminal" import { ToolExecutionWrapper, TimeoutFallbackHandler } from "../timeout" +const DEFAULT_TOOL_EXECUTION_TIMEOUT_MS = 60000 // 1 minute default + class ShellIntegrationError extends Error {} export async function executeCommandTool( @@ -64,7 +66,7 @@ export async function executeCommandTool( const { terminalOutputLineLimit = 500, terminalShellIntegrationDisabled = false, - toolExecutionTimeoutMs = 300000, // 5 minutes default + toolExecutionTimeoutMs = 60000, // 1 minute default } = clineProviderState ?? {} const options: ExecuteCommandOptions = { @@ -136,7 +138,7 @@ export async function executeCommand( // Get timeout from settings if not provided const clineProvider = await cline.providerRef.deref() const clineProviderState = await clineProvider?.getState() - const defaultTimeoutMs = clineProviderState?.toolExecutionTimeoutMs ?? 300000 // 5 minutes default + const defaultTimeoutMs = clineProviderState?.toolExecutionTimeoutMs ?? DEFAULT_TOOL_EXECUTION_TIMEOUT_MS const actualTimeoutMs = timeoutMs ?? defaultTimeoutMs const timeoutFallbackEnabled = clineProviderState?.timeoutFallbackEnabled ?? true diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 3530d98c5d..08682cc223 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -1524,7 +1524,7 @@ export class ClineProvider hasOpenedModeSelector: this.getGlobalState("hasOpenedModeSelector") ?? false, alwaysAllowFollowupQuestions: alwaysAllowFollowupQuestions ?? false, followupAutoApproveTimeoutMs: followupAutoApproveTimeoutMs ?? 60000, - toolExecutionTimeoutMs: toolExecutionTimeoutMs ?? 300000, + toolExecutionTimeoutMs: toolExecutionTimeoutMs ?? 60000, timeoutFallbackEnabled: timeoutFallbackEnabled ?? false, } } @@ -1638,7 +1638,7 @@ export class ClineProvider terminalZshP10k: stateValues.terminalZshP10k ?? false, terminalZdotdir: stateValues.terminalZdotdir ?? false, terminalCompressProgressBar: stateValues.terminalCompressProgressBar ?? true, - toolExecutionTimeoutMs: stateValues.toolExecutionTimeoutMs ?? 300000, // 5 minutes default + toolExecutionTimeoutMs: stateValues.toolExecutionTimeoutMs ?? 60000, timeoutFallbackEnabled: stateValues.timeoutFallbackEnabled ?? false, mode: stateValues.mode ?? defaultModeSlug, language: stateValues.language ?? formatLanguage(vscode.env.language), diff --git a/webview-ui/src/components/settings/AutoApproveSettings.tsx b/webview-ui/src/components/settings/AutoApproveSettings.tsx index f5685bacba..75d38f5222 100644 --- a/webview-ui/src/components/settings/AutoApproveSettings.tsx +++ b/webview-ui/src/components/settings/AutoApproveSettings.tsx @@ -71,7 +71,7 @@ export const AutoApproveSettings = ({ followupAutoApproveTimeoutMs = 60000, allowedCommands, timeoutFallbackEnabled, - toolExecutionTimeoutMs, + toolExecutionTimeoutMs = 60000, setCachedStateField, ...props }: AutoApproveSettingsProps) => { diff --git a/webview-ui/src/context/ExtensionStateContext.tsx b/webview-ui/src/context/ExtensionStateContext.tsx index 1bc78e3634..137e23cbb1 100644 --- a/webview-ui/src/context/ExtensionStateContext.tsx +++ b/webview-ui/src/context/ExtensionStateContext.tsx @@ -46,7 +46,7 @@ export interface ExtensionStateContextType extends ExtensionState { setFollowupAutoApproveTimeoutMs: (value: number) => void // Setter for the timeout timeoutFallbackEnabled?: boolean // New property for timeout fallback enabled setTimeoutFallbackEnabled: (value: boolean) => void // Setter for timeout fallback enabled - toolExecutionTimeoutMs?: number // New property for tool execution timeout + toolExecutionTimeoutMs: number | undefined // New property for tool execution timeout setToolExecutionTimeoutMs: (value: number) => void // Setter for tool execution timeout condensingApiConfigId?: string setCondensingApiConfigId: (value: string) => void @@ -230,7 +230,7 @@ export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode alwaysAllowUpdateTodoList: true, // Timeout settings timeoutFallbackEnabled: false, // Default to disabled - toolExecutionTimeoutMs: 300000, // 5 minutes default + toolExecutionTimeoutMs: undefined, // Will be set from global settings }) const [didHydrateState, setDidHydrateState] = useState(false) @@ -245,7 +245,7 @@ export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode const [alwaysAllowFollowupQuestions, setAlwaysAllowFollowupQuestions] = useState(false) // Add state for follow-up questions auto-approve const [followupAutoApproveTimeoutMs, setFollowupAutoApproveTimeoutMs] = useState(undefined) // Will be set from global settings const [timeoutFallbackEnabled, setTimeoutFallbackEnabledState] = useState(false) // Add state for timeout fallback enabled - const [toolExecutionTimeoutMs, setToolExecutionTimeoutMsState] = useState(300000) // Add state for tool execution timeout (5 minutes default) + const [toolExecutionTimeoutMs, setToolExecutionTimeoutMsState] = useState(undefined) // Will be set from global settings const [marketplaceInstalledMetadata, setMarketplaceInstalledMetadata] = useState({ project: {}, global: {}, diff --git a/webview-ui/src/i18n/locales/en/chat.json b/webview-ui/src/i18n/locales/en/chat.json index 15f3039d1e..99b267bc6c 100644 --- a/webview-ui/src/i18n/locales/en/chat.json +++ b/webview-ui/src/i18n/locales/en/chat.json @@ -230,7 +230,7 @@ "hasQuestion": "Roo has a question:" }, "taskCompleted": "Task Completed", - "toolTimeout": "Tool Timeout", + "toolTimeout": "Automatic Tool Timeout: Suggesting alternatives...", "error": "Error", "diffError": { "title": "Edit Unsuccessful" diff --git a/webview-ui/src/i18n/locales/en/settings.json b/webview-ui/src/i18n/locales/en/settings.json index 5b200778bb..f9436c8594 100644 --- a/webview-ui/src/i18n/locales/en/settings.json +++ b/webview-ui/src/i18n/locales/en/settings.json @@ -176,7 +176,7 @@ "description": "Configure how Roo handles tool operations that exceed their timeout limits", "timeoutFallbackEnabled": { "label": "Enable timeout handling", - "description": "Enable automatic timeout detection and fallback suggestions for long-running operations" + "description": "Automatically timeout long-running operations and suggest fallback options." }, "toolExecutionTimeoutMs": { "label": "Tool execution timeout (ms)",