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 <crazywolf132@gmail.com>
This commit is contained in:
@@ -308,6 +308,22 @@ export default function TerminalChat({
|
|||||||
}
|
}
|
||||||
agent.cancel();
|
agent.cancel();
|
||||||
setLoading(false);
|
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) => {
|
submitInput={(inputs) => {
|
||||||
agent.run(inputs, lastResponseId || "");
|
agent.run(inputs, lastResponseId || "");
|
||||||
|
|||||||
@@ -108,6 +108,9 @@ export class AgentLoop {
|
|||||||
if (this.terminated) {
|
if (this.terminated) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Reset the current stream to allow new requests
|
||||||
|
this.currentStream = null;
|
||||||
if (isLoggingEnabled()) {
|
if (isLoggingEnabled()) {
|
||||||
log(
|
log(
|
||||||
`AgentLoop.cancel() invoked – currentStream=${Boolean(
|
`AgentLoop.cancel() invoked – currentStream=${Boolean(
|
||||||
@@ -122,22 +125,16 @@ export class AgentLoop {
|
|||||||
)?.controller?.abort?.();
|
)?.controller?.abort?.();
|
||||||
|
|
||||||
this.canceled = true;
|
this.canceled = true;
|
||||||
|
|
||||||
|
// Abort any in-progress tool calls
|
||||||
this.execAbortController?.abort();
|
this.execAbortController?.abort();
|
||||||
|
|
||||||
|
// Create a new abort controller for future tool calls
|
||||||
|
this.execAbortController = new AbortController();
|
||||||
if (isLoggingEnabled()) {
|
if (isLoggingEnabled()) {
|
||||||
log("AgentLoop.cancel(): execAbortController.abort() called");
|
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
|
// NOTE: We intentionally do *not* clear `lastResponseId` here. If the
|
||||||
// stream produced a `function_call` before the user cancelled, OpenAI now
|
// stream produced a `function_call` before the user cancelled, OpenAI now
|
||||||
// expects a corresponding `function_call_output` that must reference that
|
// 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);
|
this.onLoading(false);
|
||||||
|
|
||||||
/* Inform the UI that the run was aborted by the user. */
|
/* Inform the UI that the run was aborted by the user. */
|
||||||
@@ -400,8 +392,10 @@ export class AgentLoop {
|
|||||||
// identified and dropped.
|
// identified and dropped.
|
||||||
const thisGeneration = ++this.generation;
|
const thisGeneration = ++this.generation;
|
||||||
|
|
||||||
// Reset cancellation flag for a fresh run.
|
// Reset cancellation flag and stream for a fresh run.
|
||||||
this.canceled = false;
|
this.canceled = false;
|
||||||
|
this.currentStream = null;
|
||||||
|
|
||||||
// Create a fresh AbortController for this run so that tool calls from a
|
// Create a fresh AbortController for this run so that tool calls from a
|
||||||
// previous run do not accidentally get signalled.
|
// previous run do not accidentally get signalled.
|
||||||
this.execAbortController = new AbortController();
|
this.execAbortController = new AbortController();
|
||||||
|
|||||||
146
codex-cli/tests/agent-interrupt-continue.test.ts
Normal file
146
codex-cli/tests/agent-interrupt-continue.test.ts
Normal file
@@ -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<any> = [];
|
||||||
|
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<any>) => 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<any>) => 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);
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user