mirror of
https://github.com/getpaseo/paseo.git
synced 2026-11-04 23:44:41 +00:00
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.
This commit is contained in:
paseo-bot[bot]
@@ -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();
|
||||
|
||||
@@ -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<ParsedCodexNotification, { kind: "item_started" | "item_completed" }>,
|
||||
): 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 &&
|
||||
|
||||
@@ -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 });
|
||||
},
|
||||
|
||||
Reference in New Issue
Block a user