From 9071d38b3a8614064507e94b0490acebd1e0af1f Mon Sep 17 00:00:00 2001 From: Claudius Date: Sat, 16 May 2026 00:07:51 +0000 Subject: [PATCH] feat(codex): factory cleanup hook + expanded conformance + parity log line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three small parity improvements building on the Codex backend foundation (#174, #175, #176): 1. **Cleanup hook in codex factory.** Kilo/OpenCode return a `cleanup: stopServer` callback from their factory `init` so the dispatcher can clean up on hot-reload or shutdown. Codex now returns a `cleanup` that calls `resetCodexState()` — clearing the cached Codex instance + frontend label + gateway port. Each `runStreamed` spawns a fresh subprocess so there's no long-lived server to stop, but resetting the module-scoped state matters for test isolation and future hot-reload paths. 2. **Backend conformance expanded to cover Codex.** Two of the four conformance tests (`routeDelivery` route parity + synthetic-error label prefix) now drive all three remote-route-using backends (Kilo / OpenCode / Codex) through the same shared `routeDelivery` helper and assert identical decisions + identical `⚠️ : ` formatting. The MCP-rotation + SSE event tests stay Kilo/OpenCode-only since Codex's MCP wiring is constructor-time TOML, not HTTP-API registration. 3. **Codex model-resolution log line.** Kilo emits `[chatId] Kilo model resolved: provider=X model=Y`; OpenCode emits the same. Codex now emits `[chatId] Codex model resolved: ` on every turn so the agent log has a uniform per-turn fingerprint across all four backends. Tests: 2305 passing, 12 skipped (live-tier), 0 failing. --- src/__tests__/backend-conformance.test.ts | 50 +++++++++++++++++++---- src/backend/codex/factory.ts | 14 ++++++- src/backend/codex/handler.ts | 1 + 3 files changed, 55 insertions(+), 10 deletions(-) diff --git a/src/__tests__/backend-conformance.test.ts b/src/__tests__/backend-conformance.test.ts index d7824191e..c460aaf52 100644 --- a/src/__tests__/backend-conformance.test.ts +++ b/src/__tests__/backend-conformance.test.ts @@ -345,10 +345,11 @@ describe("backend conformance — finalizePartsIntoState", () => { // ── Conformance: delivery routing ────────────────────────────────────────── describe("backend conformance — routeDelivery", () => { - it("returns the same route for both backends given equivalent state", async () => { - // Text-part route + it("returns the same route for all three remote-server backends given equivalent state", async () => { + // Text-part route — Kilo / OpenCode / Codex all use shared routeDelivery const stateA = createStreamState(); const stateB = createStreamState(); + const stateC = createStreamState(); const kiloDecision = await routeDelivery({ backendLabel: "Kilo", chatId: "c", @@ -361,38 +362,58 @@ describe("backend conformance — routeDelivery", () => { state: stateB, responseText: "Hello", }); + const codexDecision = await routeDelivery({ + backendLabel: "Codex", + chatId: "c", + state: stateC, + responseText: "Hello", + }); expect(kiloDecision.route).toBe(opencodeDecision.route); + expect(kiloDecision.route).toBe(codexDecision.route); expect(kiloDecision.chars).toBe(opencodeDecision.chars); + expect(kiloDecision.chars).toBe(codexDecision.chars); // Tool route - const stateC = createStreamState(); - Object.assign(stateC, { deliveredTextNorms: ["delivered"] }); - const stateD = createStreamState(); - Object.assign(stateD, { deliveredTextNorms: ["delivered"] }); + const toolStateA = createStreamState(); + Object.assign(toolStateA, { deliveredTextNorms: ["delivered"] }); + const toolStateB = createStreamState(); + Object.assign(toolStateB, { deliveredTextNorms: ["delivered"] }); + const toolStateC = createStreamState(); + Object.assign(toolStateC, { deliveredTextNorms: ["delivered"] }); const kT = await routeDelivery({ backendLabel: "Kilo", chatId: "c", - state: stateC, + state: toolStateA, responseText: "ignored", }); const oT = await routeDelivery({ backendLabel: "OpenCode", chatId: "c", - state: stateD, + state: toolStateB, + responseText: "ignored", + }); + const cT = await routeDelivery({ + backendLabel: "Codex", + chatId: "c", + state: toolStateC, responseText: "ignored", }); expect(kT.route).toBe(oT.route); + expect(kT.route).toBe(cT.route); expect(kT.route).toBe("tool"); }); - it("uses backend label in synthetic-error message prefix", async () => { + it("uses backend label in synthetic-error message prefix (all three remote-server backends)", async () => { const kiloMessages: string[] = []; const opencodeMessages: string[] = []; + const codexMessages: string[] = []; const stateA = createStreamState(); Object.assign(stateA, { syntheticError: "boom" }); const stateB = createStreamState(); Object.assign(stateB, { syntheticError: "boom" }); + const stateC = createStreamState(); + Object.assign(stateC, { syntheticError: "boom" }); await routeDelivery({ backendLabel: "Kilo", @@ -414,8 +435,19 @@ describe("backend conformance — routeDelivery", () => { }, }); + await routeDelivery({ + backendLabel: "Codex", + chatId: "c", + state: stateC, + responseText: "", + onTextBlock: async (text) => { + codexMessages.push(text); + }, + }); + expect(kiloMessages[0]).toBe("⚠️ Kilo: boom"); expect(opencodeMessages[0]).toBe("⚠️ OpenCode: boom"); + expect(codexMessages[0]).toBe("⚠️ Codex: boom"); }); }); diff --git a/src/backend/codex/factory.ts b/src/backend/codex/factory.ts index d75b4edb8..ec16ea7d9 100644 --- a/src/backend/codex/factory.ts +++ b/src/backend/codex/factory.ts @@ -15,6 +15,7 @@ import { log } from "../../util/log.js"; import { initCodexAgent } from "./init.js"; import { handleMessage as codexHandleMessage } from "./handler.js"; import { runOneShotAgent as codexRunOneShotAgent } from "./one-shot.js"; +import { resetState as resetCodexState } from "./state.js"; import { resolveModel, getModelInfo, @@ -48,7 +49,18 @@ const codexFactory: BackendFactory = { backendLabel: "Codex", }; - return { backend }; + return { + backend, + // Cleanup: drop the cached Codex instance + state. The Codex + // SDK's instances are stateless from our perspective (each + // `runStreamed` spawns a fresh subprocess), so there's no + // long-running server to stop — clearing module-scoped state + // is enough for hot-reload + test isolation. + cleanup: () => { + resetCodexState(); + log("bot", "Codex backend cleaned up"); + }, + }; }, }; diff --git a/src/backend/codex/handler.ts b/src/backend/codex/handler.ts index 2e9329827..5a4ee3121 100644 --- a/src/backend/codex/handler.ts +++ b/src/backend/codex/handler.ts @@ -122,6 +122,7 @@ export async function handleMessage( // pass through whatever the chat settings hold (default `gpt-5-codex`). const chatSettings = getChatSettings(chatId); const activeModel = chatSettings.model ?? config.model ?? CODEX_DEFAULT_MODEL; + log("agent", `[${chatId}] Codex model resolved: ${activeModel}`); // First-turn system-prompt rebuild + Codex-specific delivery suffix. const systemPrompt = prepareSystemPrompt({