From a2f0fb9e325a5505aeac7e4caf84af692b8be05a Mon Sep 17 00:00:00 2001 From: "paseo-bot[bot]" <266920839+paseo-bot[bot]@users.noreply.github.com> Date: Tue, 3 Nov 2026 07:47:09 +0000 Subject: [PATCH] fix(codex): distinguish task acceptance from hook housekeeping Stopped submit hooks reject input too. Pre-prompt compaction and hook context are housekeeping, so they must not turn a fully rejected task into a successful run. --- .../providers/codex-app-server-agent.test.ts | 262 +++++++++++------- .../agent/providers/codex-app-server-agent.ts | 8 +- .../codex/test-utils/fake-app-server.ts | 10 + 3 files changed, 174 insertions(+), 106 deletions(-) diff --git a/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts b/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts index cc7a7afc4b..fa402885e3 100644 --- a/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts +++ b/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts @@ -33,54 +33,60 @@ import { } from "./codex-app-server-agent.js"; describe("Codex prompt hook rejection", () => { - test("shows a rejected queued follow-up without failing accepted work in the same turn", async () => { - const appServer = createFakeCodexAppServer({ - "turn/steer": () => ({ turn: { id: "native-A" } }), - }); - const { session, paseoTurnId } = await startPublicSteeringSession(appServer); - const events: AgentStreamEvent[] = []; - session.subscribe((event) => events.push(event)); - try { - await session.steerActiveTurn!("Allowed follow-up", { expectedTurnId: paseoTurnId }); - await session.steerActiveTurn!("Rejected follow-up", { expectedTurnId: paseoTurnId }); - appServer.completesHook({ - threadId: "thread-1", - turnId: "native-A", - eventName: "userPromptSubmit", - status: "completed", - entries: [], + test.each([ + { status: "blocked", entryKind: "feedback" }, + { status: "stopped", entryKind: "stop" }, + ])( + "shows a $status queued follow-up without failing accepted work in the same turn", + async ({ status, entryKind }) => { + const appServer = createFakeCodexAppServer({ + "turn/steer": () => ({ turn: { id: "native-A" } }), }); - appServer.completesHook({ - threadId: "thread-1", - turnId: "native-A", - eventName: "userPromptSubmit", - status: "blocked", - entries: [{ kind: "feedback", text: "Follow-up rejected" }], - }); - appServer.says({ threadId: "thread-1", text: "Accepted work completed" }); - const completion = waitForNextTurnCompletion(session); - appServer.completeTurn(); - await expect(completion).resolves.toMatchObject({ type: "turn_completed" }); - expect(events).toContainEqual( - expect.objectContaining({ - type: "timeline", - item: expect.objectContaining({ - type: "assistant_message", - text: "Accepted work completed", + const { session, paseoTurnId } = await startPublicSteeringSession(appServer); + const events: AgentStreamEvent[] = []; + session.subscribe((event) => events.push(event)); + try { + await session.steerActiveTurn!("Allowed follow-up", { expectedTurnId: paseoTurnId }); + await session.steerActiveTurn!("Rejected follow-up", { expectedTurnId: paseoTurnId }); + appServer.completesHook({ + threadId: "thread-1", + turnId: "native-A", + eventName: "userPromptSubmit", + status: "completed", + entries: [], + }); + appServer.completesHook({ + threadId: "thread-1", + turnId: "native-A", + eventName: "userPromptSubmit", + status, + entries: [{ kind: entryKind, text: "Follow-up rejected" }], + }); + appServer.says({ threadId: "thread-1", text: "Accepted work completed" }); + const completion = waitForNextTurnCompletion(session); + appServer.completeTurn(); + await expect(completion).resolves.toMatchObject({ type: "turn_completed" }); + expect(events).toContainEqual( + expect.objectContaining({ + type: "timeline", + item: expect.objectContaining({ + type: "assistant_message", + text: "Accepted work completed", + }), }), - }), - ); - expect(events).toContainEqual({ - type: "timeline", - provider: "codex", - item: { type: "notification", level: "error", message: "Follow-up rejected" }, - turnId: paseoTurnId, - }); - appServer.assertNoErrors(); - } finally { - await session.close(); - } - }); + ); + expect(events).toContainEqual({ + type: "timeline", + provider: "codex", + item: { type: "notification", level: "error", message: "Follow-up rejected" }, + turnId: paseoTurnId, + }); + appServer.assertNoErrors(); + } finally { + await session.close(); + } + }, + ); test.each(["before", "after"])( "keeps an accepted input successful when rejected input arrives %s it, even without a reply", @@ -168,66 +174,116 @@ describe("Codex prompt hook rejection", () => { } }); - test("fails a fully rejected manager run and permits a corrected submission", async () => { - const appServer = createFakeCodexAppServer(); - const provider = createProviderWithFakeAppServer(appServer); - const client: AgentClient = { - provider: "codex", - capabilities: provider.capabilities, - isAvailable: async () => true, - fetchCatalog: async () => ({ models: [], modes: [] }), - createSession: (...args) => provider.createSession(...args), - resumeSession: (...args) => provider.resumeSession(...args), - }; - const manager = new AgentManager({ clients: { codex: client }, logger: createTestLogger() }); - const agent = await manager.resumeAgentFromPersistence( - { provider: "codex", sessionId: "thread-1" }, - createConfig({ cwd: process.cwd() }), - ); - try { - const run = manager.runAgent(agent.id, "Disallowed prompt", { - clientMessageId: "blocked-prompt", - }); - const failure = expect(run).rejects.toThrow("Remove the disallowed text and retry."); - await appServer.waitForTurnStart(); - appServer.startsTurn({ threadId: "thread-1", turnId: "native-A" }); - appServer.completesHook({ - threadId: "thread-1", - turnId: "native-A", - eventName: "userPromptSubmit", - status: "blocked", - entries: [{ kind: "feedback", text: "Remove the disallowed text and retry." }], - }); - appServer.completeTurn(); - await failure; + test.each(["compaction", "hook context"])( + "does not count pre-prompt %s as accepted task work", + async (housekeeping) => { + const appServer = createFakeCodexAppServer(); + const { session } = await startPublicSteeringSession(appServer); + try { + if (housekeeping === "compaction") { + appServer.startsCompaction({ threadId: "thread-1", itemId: "pre-prompt-compaction" }); + appServer.completesCompaction({ threadId: "thread-1", itemId: "pre-prompt-compaction" }); + } else { + appServer.addsHookContext({ + threadId: "thread-1", + turnId: "native-A", + itemId: "hook-context", + text: "Housekeeping context", + }); + } + appServer.completesHook({ + threadId: "thread-1", + turnId: "native-A", + eventName: "userPromptSubmit", + status: "blocked", + entries: [{ kind: "feedback", text: "Rejected after compaction" }], + }); + const completion = waitForNextTurnCompletion(session); + appServer.completeTurn(); + await expect(completion).resolves.toMatchObject({ + type: "turn_failed", + error: "Rejected after compaction", + }); + appServer.assertNoErrors(); + } finally { + await session.close(); + } + }, + ); - expect(manager.getTimeline(agent.id)).toEqual([ - { - type: "user_message", - text: "Disallowed prompt", - messageId: "blocked-prompt", + test.each([ + { status: "blocked", entryKind: "feedback" }, + { status: "stopped", entryKind: "stop" }, + ])( + "fails a fully $status manager run and permits a corrected submission", + async ({ status, entryKind }) => { + const appServer = createFakeCodexAppServer(); + const provider = createProviderWithFakeAppServer(appServer); + const client: AgentClient = { + provider: "codex", + capabilities: provider.capabilities, + isAvailable: async () => true, + fetchCatalog: async () => ({ models: [], modes: [] }), + createSession: (...args) => provider.createSession(...args), + resumeSession: (...args) => provider.resumeSession(...args), + }; + const manager = new AgentManager({ clients: { codex: client }, logger: createTestLogger() }); + const agent = await manager.resumeAgentFromPersistence( + { provider: "codex", sessionId: "thread-1" }, + createConfig({ cwd: process.cwd() }), + ); + try { + const run = manager.runAgent(agent.id, "Disallowed prompt", { clientMessageId: "blocked-prompt", - }, - { type: "notification", level: "error", message: "Remove the disallowed text and retry." }, - { type: "assistant_message", text: "[System Error] Remove the disallowed text and retry." }, - ]); - expect(manager.hasInFlightRun(agent.id)).toBe(false); - expect(manager.getAgent(agent.id)?.lastError).toBe("Remove the disallowed text and retry."); + }); + const failure = expect(run).rejects.toThrow("Remove the disallowed text and retry."); + await appServer.waitForTurnStart(); + appServer.startsTurn({ threadId: "thread-1", turnId: "native-A" }); + appServer.completesHook({ + threadId: "thread-1", + turnId: "native-A", + eventName: "userPromptSubmit", + status, + entries: [{ kind: entryKind, text: "Remove the disallowed text and retry." }], + }); + appServer.completeTurn(); + await failure; - const retry = manager.runAgent(agent.id, "Corrected prompt", { - clientMessageId: "corrected-prompt", - }); - await appServer.waitForTurnStart(2); - appServer.startsTurn({ threadId: "thread-1", turnId: "native-B" }); - appServer.says({ threadId: "thread-1", text: "Accepted" }); - appServer.completeTurn(); - await expect(retry).resolves.toMatchObject({ finalText: "Accepted", canceled: false }); - expect(manager.hasInFlightRun(agent.id)).toBe(false); - appServer.assertNoErrors(); - } finally { - await manager.closeAgent(agent.id); - } - }); + expect(manager.getTimeline(agent.id)).toEqual([ + { + type: "user_message", + text: "Disallowed prompt", + messageId: "blocked-prompt", + clientMessageId: "blocked-prompt", + }, + { + type: "notification", + level: "error", + message: "Remove the disallowed text and retry.", + }, + { + type: "assistant_message", + text: "[System Error] Remove the disallowed text and retry.", + }, + ]); + expect(manager.hasInFlightRun(agent.id)).toBe(false); + expect(manager.getAgent(agent.id)?.lastError).toBe("Remove the disallowed text and retry."); + + const retry = manager.runAgent(agent.id, "Corrected prompt", { + clientMessageId: "corrected-prompt", + }); + await appServer.waitForTurnStart(2); + appServer.startsTurn({ threadId: "thread-1", turnId: "native-B" }); + appServer.says({ threadId: "thread-1", text: "Accepted" }); + appServer.completeTurn(); + await expect(retry).resolves.toMatchObject({ finalText: "Accepted", canceled: false }); + expect(manager.hasInFlightRun(agent.id)).toBe(false); + appServer.assertNoErrors(); + } finally { + await manager.closeAgent(agent.id); + } + }, + ); test("shows blocking feedback and accepts the next turn", async () => { const appServer = createFakeCodexAppServer(); diff --git a/packages/server/src/server/agent/providers/codex-app-server-agent.ts b/packages/server/src/server/agent/providers/codex-app-server-agent.ts index 5933f82580..1fdc979696 100644 --- a/packages/server/src/server/agent/providers/codex-app-server-agent.ts +++ b/packages/server/src/server/agent/providers/codex-app-server-agent.ts @@ -6442,7 +6442,7 @@ export class CodexAppServerAgentSession implements AgentSession { ): void { const rejectsCurrentPrompt = parsed.eventName === "userPromptSubmit" && - parsed.status === "blocked" && + (parsed.status === "blocked" || parsed.status === "stopped") && this.currentTurnId !== null && parsed.turnId === this.currentTurnId; if (!rejectsCurrentPrompt) return; @@ -6523,8 +6523,10 @@ export class CodexAppServerAgentSession implements AgentSession { private recordAcceptedTurnWork( parsed: Extract, ): void { - // Rejected prompts produce no user-message or work items. Any accepted input or - // autonomous work in this native turn keeps a rejected follow-up from failing it. + // Pre-prompt housekeeping can precede a rejected input. Only admitted input or + // task work keeps a rejected follow-up from failing the native turn. + const itemType = normalizeCodexThreadItemType(parsed.item.type); + if (itemType === CODEX_CONTEXT_COMPACTION_TYPE || itemType === "hookPrompt") return; if ( this.currentTurnId !== null && parsed.threadId === this.currentThreadId && diff --git a/packages/server/src/server/agent/providers/codex/test-utils/fake-app-server.ts b/packages/server/src/server/agent/providers/codex/test-utils/fake-app-server.ts index 0fe5d2b9bb..48c1e7c594 100644 --- a/packages/server/src/server/agent/providers/codex/test-utils/fake-app-server.ts +++ b/packages/server/src/server/agent/providers/codex/test-utils/fake-app-server.ts @@ -87,6 +87,7 @@ export interface FakeCodexAppServer { itemId: string; text: string; }): void; + addsHookContext(params: { threadId: string; turnId: string; itemId: string; text: string }): void; completesHook(params: FakeHookCompletion): void; completeTurn(params?: { threadId?: string; @@ -443,6 +444,15 @@ export function createFakeCodexAppServer( writeNotification("item/started", params); writeNotification("item/completed", params); }, + addsHookContext({ threadId, turnId, itemId, text }) { + const params = { + threadId, + turnId, + item: { type: "hookPrompt", id: itemId, fragments: [{ text, hookRunId: "hook-run" }] }, + }; + writeNotification("item/started", params); + writeNotification("item/completed", params); + }, completesHook({ threadId, turnId, ...run }) { writeNotification("hook/completed", { threadId, turnId, run }); },