From b0ccca555685b1534a0028cb7bfdcad8fe2e477a Mon Sep 17 00:00:00 2001 From: Brayden Moon Date: Thu, 17 Apr 2025 15:20:19 +1000 Subject: [PATCH] fix: allow continuing after interrupting assistant (#178) ## Description This PR fixes the issue where the CLI can't continue after interrupting the assistant with ESC ESC (Fixes #114). The problem was caused by duplicate code in the `cancel()` method and improper state reset after cancellation. ## Changes - Fixed duplicate code in the `cancel()` method of the `AgentLoop` class - Added proper reset of the `currentStream` property in the `cancel()` method - Created a new `AbortController` after aborting the current one to ensure future tool calls work - Added a system message to indicate the interruption to the user - Added a comprehensive test to verify the fix ## Benefits - Users can now continue using the CLI after interrupting the assistant - Improved user experience by providing feedback when interruption occurs - Better state management in the agent loop ## Testing - Added a dedicated test that verifies the agent can process new input after cancellation - Manually tested the fix by interrupting the assistant and confirming that new input is processed correctly --------- Signed-off-by: crazywolf132 --- .../src/components/chat/terminal-chat.tsx | 16 ++ codex-cli/src/utils/agent/agent-loop.ts | 28 ++-- .../tests/agent-interrupt-continue.test.ts | 146 ++++++++++++++++++ 3 files changed, 173 insertions(+), 17 deletions(-) create mode 100644 codex-cli/tests/agent-interrupt-continue.test.ts diff --git a/codex-cli/src/components/chat/terminal-chat.tsx b/codex-cli/src/components/chat/terminal-chat.tsx index 35fecec5..90e398c7 100644 --- a/codex-cli/src/components/chat/terminal-chat.tsx +++ b/codex-cli/src/components/chat/terminal-chat.tsx @@ -308,6 +308,22 @@ export default function TerminalChat({ } agent.cancel(); setLoading(false); + + // Add a system message to indicate the interruption + setItems((prev) => [ + ...prev, + { + id: `interrupt-${Date.now()}`, + type: "message", + role: "system", + content: [ + { + type: "input_text", + text: "⏹️ Execution interrupted by user. You can continue typing.", + }, + ], + }, + ]); }} submitInput={(inputs) => { agent.run(inputs, lastResponseId || ""); diff --git a/codex-cli/src/utils/agent/agent-loop.ts b/codex-cli/src/utils/agent/agent-loop.ts index ddf067c8..51129766 100644 --- a/codex-cli/src/utils/agent/agent-loop.ts +++ b/codex-cli/src/utils/agent/agent-loop.ts @@ -108,6 +108,9 @@ export class AgentLoop { if (this.terminated) { return; } + + // Reset the current stream to allow new requests + this.currentStream = null; if (isLoggingEnabled()) { log( `AgentLoop.cancel() invoked – currentStream=${Boolean( @@ -122,22 +125,16 @@ export class AgentLoop { )?.controller?.abort?.(); this.canceled = true; + + // Abort any in-progress tool calls this.execAbortController?.abort(); + + // Create a new abort controller for future tool calls + this.execAbortController = new AbortController(); if (isLoggingEnabled()) { log("AgentLoop.cancel(): execAbortController.abort() called"); } - // If we have *not* seen any function_call IDs yet there is nothing that - // needs to be satisfied in a follow‑up request. In that case we clear - // the stored lastResponseId so a subsequent run starts a clean turn. - if (this.pendingAborts.size === 0) { - try { - this.onLastResponseId(""); - } catch { - /* ignore */ - } - } - // NOTE: We intentionally do *not* clear `lastResponseId` here. If the // stream produced a `function_call` before the user cancelled, OpenAI now // expects a corresponding `function_call_output` that must reference that @@ -155,11 +152,6 @@ export class AgentLoop { } } - // NOTE: We intentionally do *not* clear `lastResponseId` here. If the - // stream produced a `function_call` before the user cancelled, OpenAI now - // expects a corresponding `function_call_output` that must reference that - // very same response ID. We therefore keep the ID around so the - // follow‑up request can still satisfy the contract. this.onLoading(false); /* Inform the UI that the run was aborted by the user. */ @@ -400,8 +392,10 @@ export class AgentLoop { // identified and dropped. const thisGeneration = ++this.generation; - // Reset cancellation flag for a fresh run. + // Reset cancellation flag and stream for a fresh run. this.canceled = false; + this.currentStream = null; + // Create a fresh AbortController for this run so that tool calls from a // previous run do not accidentally get signalled. this.execAbortController = new AbortController(); diff --git a/codex-cli/tests/agent-interrupt-continue.test.ts b/codex-cli/tests/agent-interrupt-continue.test.ts new file mode 100644 index 00000000..db20bc9c --- /dev/null +++ b/codex-cli/tests/agent-interrupt-continue.test.ts @@ -0,0 +1,146 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { AgentLoop } from "../src/utils/agent/agent-loop.js"; + +// Create a state holder for our mocks +const openAiState = { + createSpy: vi.fn(), +}; + +// Mock the OpenAI client +vi.mock("openai", () => { + return { + default: class MockOpenAI { + responses = { + create: openAiState.createSpy, + }; + }, + }; +}); + +describe("Agent interrupt and continue", () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + vi.resetAllMocks(); + }); + + it("allows continuing after interruption", async () => { + // Track received items + const received: Array = []; + let loadingState = false; + + // Create the agent + const agent = new AgentLoop({ + model: "test-model", + instructions: "", + approvalPolicy: { mode: "auto" } as any, + config: { + model: "test-model", + instructions: "", + }, + onItem: (item) => received.push(item), + onLoading: (loading) => { + loadingState = loading; + }, + getCommandConfirmation: async () => ({ review: "yes" } as any), + onLastResponseId: () => {}, + }); + + // First user message + const firstMessage = [ + { + type: "message", + role: "user", + content: [{ type: "input_text", text: "first message" }], + }, + ]; + + // Setup the first mock response + openAiState.createSpy.mockImplementation(() => { + // Return a mock stream object + return { + controller: { + abort: vi.fn(), + }, + on: (event: string, callback: (...args: Array) => void) => { + if (event === "message") { + // Schedule a message to be delivered + setTimeout(() => { + callback({ + type: "message", + role: "assistant", + content: [{ type: "input_text", text: "First response" }], + }); + }, 10); + } + return { controller: { abort: vi.fn() } }; + }, + }; + }); + + // Start the first run + const firstRunPromise = agent.run(firstMessage as any); + + // Advance timers to allow the stream to start + await vi.advanceTimersByTimeAsync(5); + + // Interrupt the agent + agent.cancel(); + + // Verify loading state is reset + expect(loadingState).toBe(false); + + // Second user message + const secondMessage = [ + { + type: "message", + role: "user", + content: [{ type: "input_text", text: "second message" }], + }, + ]; + + // Reset the mock to track the second call + openAiState.createSpy.mockClear(); + + // Setup the second mock response + openAiState.createSpy.mockImplementation(() => { + // Return a mock stream object + return { + controller: { + abort: vi.fn(), + }, + on: (event: string, callback: (...args: Array) => void) => { + if (event === "message") { + // Schedule a message to be delivered + setTimeout(() => { + callback({ + type: "message", + role: "assistant", + content: [{ type: "input_text", text: "Second response" }], + }); + }, 10); + } + return { controller: { abort: vi.fn() } }; + }, + }; + }); + + // Start the second run + const secondRunPromise = agent.run(secondMessage as any); + + // Advance timers to allow the second stream to complete + await vi.advanceTimersByTimeAsync(20); + + // Ensure both promises resolve + await Promise.all([firstRunPromise, secondRunPromise]); + + // Verify the second API call was made + expect(openAiState.createSpy).toHaveBeenCalled(); + + // Verify that the agent can process new input after cancellation + expect(loadingState).toBe(false); + }); +});