diff --git a/src/bundled/codex/README.md b/src/bundled/codex/README.md index c4ed9cabd..156591689 100644 --- a/src/bundled/codex/README.md +++ b/src/bundled/codex/README.md @@ -40,7 +40,10 @@ BUZZODZ_PROFILE=codex-agents BUZZ_DEV_VIEWER= bin/just desktop While work is active, new mentions steer it, incorporating the follow-up into the ongoing task. Messages arriving during startup wait for the turn to start and then steer it; idle conversations continue the saved session. If a turn finishes -while a follow-up is being delivered, the follow-up starts another turn. +while a follow-up is being delivered, the follow-up starts another turn. A +follow-up that picks a model or effort other than the running turn's cannot +steer it, so it waits and runs as the next turn, and later follow-ups wait +behind it to keep their order. A successful final send stops the request counting as working at once; Codex still ends its turn with one more model call. Interrupting the turn instead would leave a `` note in the session after every reply. diff --git a/src/bundled/codex/runtime.test.ts b/src/bundled/codex/runtime.test.ts index e77f74f4d..d30e6f19d 100644 --- a/src/bundled/codex/runtime.test.ts +++ b/src/bundled/codex/runtime.test.ts @@ -713,6 +713,88 @@ it("starts a turn on the model its owner picked, at an effort that model support { model: "test-model", effort: "low" }, ]); }); +it("runs a mid-turn pick that differs from the running turn as the next turn instead of steering", async () => { + const f = fixture(); + const settings = (wire: Wire) => { + const { threadId, model, effort } = wire.params as Record; + return { threadId, model, effort }; + }; + await f.runtime.run(f.delivery("first")); + await vi.waitFor(() => expect(f.starts()).toHaveLength(1)); + await f.runtime.run({ + ...f.delivery("deeper"), + model: "deep-model", + effort: "xhigh", + }); + await f.complete(0, "FIRST"); + await vi.waitFor(() => expect(f.starts()).toHaveLength(2)); + // An effort pick alone queues too, on the agent's own model. + await f.runtime.run({ ...f.delivery("harder"), effort: "high" }); + await f.complete(1, "SECOND"); + await vi.waitFor(() => expect(f.starts()).toHaveLength(3)); + expect(f.sent.filter((wire) => wire.method === "turn/steer")).toHaveLength(0); + expect(f.starts().map(settings)).toEqual([ + { threadId: "thread-1", model: "test-model", effort: "low" }, + { threadId: "thread-1", model: "deep-model", effort: "xhigh" }, + { threadId: "thread-1", model: "test-model", effort: "high" }, + ]); + f.complete(2, "THIRD"); + await vi.waitFor(() => expect(f.publish).toHaveBeenCalledTimes(3)); + expect( + f.publish.mock.calls.map(([event]) => (event as EventData).content), + ).toEqual(["FIRST", "SECOND", "THIRD"]); +}); +it("keeps a plain follow-up behind a queued differing pick instead of steering past it", async () => { + const f = fixture(); + await f.runtime.run(f.delivery("first")); + await vi.waitFor(() => expect(f.starts()).toHaveLength(1)); + await f.runtime.run({ ...f.delivery("deeper"), model: "deep-model" }); + await f.runtime.run(f.delivery("plain")); + await f.complete(0, "FIRST"); + await vi.waitFor(() => expect(f.starts()).toHaveLength(2)); + // "plain" is still queued while "deeper" runs, so this cannot steer either. + await f.runtime.run(f.delivery("later")); + await new Promise((resolve) => setTimeout(resolve, 0)); + await f.complete(1, "SECOND"); + await vi.waitFor(() => expect(f.starts()).toHaveLength(3)); + await f.complete(2, "THIRD"); + await vi.waitFor(() => expect(f.starts()).toHaveLength(4)); + expect(f.sent.filter((wire) => wire.method === "turn/steer")).toHaveLength(0); + const input = (index: number) => JSON.stringify(f.starts()[index]?.params); + expect(input(1)).toContain("deeper"); + expect(input(1)).toContain('"model":"deep-model"'); + expect(input(2)).toContain("plain"); + expect(input(2)).toContain('"model":"test-model"'); + expect(input(3)).toContain("later"); + f.complete(3, "FOURTH"); + await vi.waitFor(() => expect(f.publish).toHaveBeenCalledTimes(4)); + expect( + f.publish.mock.calls.map(([event]) => (event as EventData).content), + ).toEqual(["FIRST", "SECOND", "THIRD", "FOURTH"]); +}); +it("steers a mid-turn message whose pick matches the running turn, or has none", async () => { + const f = fixture(); + await f.runtime.run({ + ...f.delivery("first"), + model: "test-model", + effort: "low", + }); + await vi.waitFor(() => expect(f.starts()).toHaveLength(1)); + await f.runtime.run({ + ...f.delivery("same"), + model: "test-model", + effort: "low", + }); + await f.runtime.run(f.delivery("plain")); + await vi.waitFor(() => + expect(f.sent.filter((wire) => wire.method === "turn/steer")).toHaveLength( + 2, + ), + ); + f.complete(); + await vi.waitFor(() => expect(f.publish).toHaveBeenCalledTimes(1)); + expect(f.starts()).toHaveLength(1); +}); it("serializes owner follow-ups and routes tool replies to the latest accepted request", async () => { const f = fixture(); await f.runtime.run(f.delivery("first")); diff --git a/src/bundled/codex/runtime.ts b/src/bundled/codex/runtime.ts index d0cde7f3d..ab94c34fd 100644 --- a/src/bundled/codex/runtime.ts +++ b/src/bundled/codex/runtime.ts @@ -67,6 +67,9 @@ type Active = { steering: Promise; reply: Request; finished: boolean; + /** What the turn runs on, once resolved; a differing pick queues a turn. */ + model?: string; + effort?: string; workspace: string; signal: AbortSignal; version: number; @@ -78,6 +81,8 @@ type Active = { }; type Lane = { tail: Promise; + /** Turns waiting on `tail` to start; steering past them would reorder. */ + queued: number; active?: Active; }; type Entry = { @@ -410,7 +415,7 @@ export class CodexRuntime { const entry = this.entry(request); let lane = entry.lanes.get(key); if (!lane) { - lane = { tail: entry.after }; + lane = { tail: entry.after, queued: 0 }; entry.lanes.set(key, lane); } if (lane.active) { @@ -423,7 +428,16 @@ export class CodexRuntime { request.failActivity("Codex was stopped"); return; } - if (active.finished) { + // Steering cannot carry a model or effort, so a differing pick + // waits for this turn and runs as the next one. Everything after + // it queues too until the lane drains: steering would run later + // messages first. + if ( + active.finished || + lane.queued || + (request.model && request.model !== active.model) || + (request.effort && request.effort !== active.effort) + ) { this.start(request, text, key, lane, entry); return; } @@ -495,9 +509,11 @@ export class CodexRuntime { entry: Entry, ) { const current = lane; + current.queued++; const job = lane.tail .catch(() => undefined) .then(async () => { + current.queued--; if (entry.abort.signal.aborted) { request.failActivity("Codex was stopped"); return; @@ -776,6 +792,8 @@ export class CodexRuntime { [request.effort, settings.effort].find( (level) => level && supported.includes(level), ) ?? selected.defaultReasoningEffort; + active.model = model; + active.effort = effort; const mcp = Object.fromEntries( Object.entries(normalized.config.mcp_servers ?? {}).map( ([name, server]) => [