Skip to content

Commit f1a1d3b

Browse files
authored
fix(codex): keep goal commands active through model turns (#288)
1 parent 2ef632a commit f1a1d3b

2 files changed

Lines changed: 61 additions & 4 deletions

File tree

src/supervisor/agents/codex/acp.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -658,6 +658,8 @@ export class CodexStructuredSession implements StructuredSessionHandle {
658658
];
659659

660660
if (goalCommand) {
661+
const canStartModelTurn = goalCommand.kind === "set" || goalCommand.kind === "resume";
662+
const expectsModelTurn = canStartModelTurn && config.mode !== "plan";
661663
this.emitRuntimeEvents([
662664
{ type: "turn.started", threadId: this.threadId, turnId },
663665
...userEvents,
@@ -673,6 +675,9 @@ export class CodexStructuredSession implements StructuredSessionHandle {
673675
this.emitUpdate({ status: "idle", attention: "none" });
674676
return;
675677
}
678+
if (expectsModelTurn || (canStartModelTurn && this.activeTurnId)) {
679+
return;
680+
}
676681
this.emitRuntimeEvents([
677682
{ type: "turn.completed", threadId: this.threadId, turnId, state: "completed" },
678683
]);

src/supervisor/agents/codex/codex.test.ts

Lines changed: 56 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -930,7 +930,7 @@ describe("CodexStructuredSession", () => {
930930
expect(requests[2]?.params.serviceTier).toBeNull();
931931
});
932932

933-
it("dispatches /goal <objective> to thread/goal/set without starting a model turn", async () => {
933+
it("keeps /goal <objective> working until the model turn completes", async () => {
934934
const requests: Array<{ method: string; params: Record<string, unknown> }> = [];
935935
const structuredSession = makeStructuredSession(requests);
936936
const runtimeEvents: RuntimeEvent[] = [];
@@ -939,6 +939,20 @@ describe("CodexStructuredSession", () => {
939939
onRuntimeEvent: (event: RuntimeEvent) => runtimeEvents.push(event),
940940
onUpdate: (update: unknown) => updates.push(update),
941941
};
942+
(structuredSession as unknown as Record<string, unknown>)["rpc"] = {
943+
request: async (method: string, params: Record<string, unknown>) => {
944+
requests.push({ method, params });
945+
dispatchNotification(structuredSession, {
946+
jsonrpc: "2.0",
947+
method: "turn/started",
948+
params: {
949+
threadId: "provider-thread",
950+
turn: { id: "goal-turn", threadId: "provider-thread" },
951+
},
952+
});
953+
return {};
954+
},
955+
};
942956

943957
await structuredSession.startTurn("/goal ship unified GUI goal support", { model: "gpt-5.4" });
944958

@@ -953,14 +967,52 @@ describe("CodexStructuredSession", () => {
953967
},
954968
]);
955969
// The goal item itself is produced by the canonical mapper from the
956-
// `thread/goal/updated` notification — startTurn should only emit the
957-
// user-facing turn/user-message envelope around the RPC.
970+
// `thread/goal/updated` notification. `thread/goal/set` starts a real
971+
// model turn, so its native completion notification must settle the turn.
958972
expect(runtimeEvents.map((event) => event.type)).toEqual([
959973
"turn.started",
960974
"item.started",
961975
"item.completed",
962-
"turn.completed",
976+
"turn.started",
963977
]);
978+
expect(updates.at(-1)).toEqual({ status: "working", attention: "working" });
979+
});
980+
981+
it("does not settle /goal <objective> before a delayed model turn starts", async () => {
982+
const requests: Array<{ method: string; params: Record<string, unknown> }> = [];
983+
const structuredSession = makeStructuredSession(requests);
984+
const runtimeEvents: RuntimeEvent[] = [];
985+
const updates: unknown[] = [];
986+
(structuredSession as unknown as Record<string, unknown>)["listener"] = {
987+
onRuntimeEvent: (event: RuntimeEvent) => runtimeEvents.push(event),
988+
onUpdate: (update: unknown) => updates.push(update),
989+
};
990+
991+
await structuredSession.startTurn("/goal continue when idle", { model: "gpt-5.4" });
992+
993+
expect(runtimeEvents).not.toContainEqual(expect.objectContaining({ type: "turn.completed" }));
994+
expect(updates).not.toContainEqual({ status: "idle", attention: "none" });
995+
});
996+
997+
it("settles /goal <objective> when Codex does not start a model turn", async () => {
998+
const requests: Array<{ method: string; params: Record<string, unknown> }> = [];
999+
const structuredSession = makeStructuredSession(requests);
1000+
const runtimeEvents: RuntimeEvent[] = [];
1001+
const updates: unknown[] = [];
1002+
(structuredSession as unknown as Record<string, unknown>)["listener"] = {
1003+
onRuntimeEvent: (event: RuntimeEvent) => runtimeEvents.push(event),
1004+
onUpdate: (update: unknown) => updates.push(update),
1005+
};
1006+
1007+
await structuredSession.startTurn("/goal plan without continuing", {
1008+
model: "gpt-5.4",
1009+
mode: "plan",
1010+
});
1011+
1012+
expect(runtimeEvents.at(-1)).toMatchObject({
1013+
type: "turn.completed",
1014+
state: "completed",
1015+
});
9641016
expect(updates.at(-1)).toEqual({ status: "idle", attention: "none" });
9651017
});
9661018

0 commit comments

Comments
 (0)