diff --git a/src/server/responses/collaboration.ts b/src/server/responses/collaboration.ts index 0b28267ce..97620bfcb 100644 --- a/src/server/responses/collaboration.ts +++ b/src/server/responses/collaboration.ts @@ -249,10 +249,22 @@ export async function multiAgentGuidanceText( // Codex cannot actually spawn makes spawn_agent reject the override, so // suppress positive model claims while the state is stale or unknown. const catalogState = await (deps.collectCatalogState ?? defaultCollectCatalogState)(); - if (catalogState.state === "stale" || catalogState.state === "unknown") { + if (catalogState.state === "stale") { return "The model catalog changed after Codex started; do not set " + "model or reasoning_effort overrides until Codex restarts."; } + // #1354: `unknown` is not `stale`. It means the comparison could not be made + // — the app-server start time or the catalog mtime was unreadable (a + // transient code-mode-host process exiting between enumeration and the + // start-time read is one way to land here). Suppressing overrides is still + // correct, but the stale text asserts a cause that was never established and + // prescribes a restart that cannot clear `unknown`. Mirror the honest + // phrasing `ocx doctor` already uses for this state. + if (catalogState.state === "unknown") { + return "Could not verify whether Codex's model catalog is current " + + "(app-server start time or catalog timestamp unreadable); do not set model or " + + "reasoning_effort overrides for this turn."; + } // codex-rs supplies the Proactive text on v2; the proxy only adds model-designation // guidance, and only when there is something concrete to designate: a configured // injectionModel and/or a roster entry that resolves in the injected catalog. diff --git a/tests/multi-agent-compat.test.ts b/tests/multi-agent-compat.test.ts index 8e9740cb1..bfdfd7465 100644 --- a/tests/multi-agent-compat.test.ts +++ b/tests/multi-agent-compat.test.ts @@ -143,6 +143,47 @@ describe("multiAgentGuidanceText", () => { } }); + test("v2 guidance does not assert a catalog change or a restart remedy when the state is unknown (#1354)", async () => { + const dir = codexHomeFixture(V2_ON); + catalogFixture(dir, [{ + slug: "anthropic/claude-sonnet-5", + efforts: ["low", "medium", "high", "xhigh"], + }]); + const parsed = parsedFixture({ reasoning: "medium", tools: [{ name: "spawn_agent" }] }); + const options = { injectionModel: "anthropic/claude-sonnet-5" }; + + const stale = await multiAgentGuidanceText(parsed, options, { + collectCatalogState: () => ({ state: "stale" }), + }); + const unknown = await multiAgentGuidanceText(parsed, options, { + collectCatalogState: () => ({ state: "unknown" }), + }); + + // "stale" is an established fact: the catalog really did change after the + // app-server started, and restarting Codex really does clear it. + expect(stale).toContain("The model catalog changed after Codex started"); + expect(stale).toContain("until Codex restarts"); + + // "unknown" means the comparison could not be made at all — the process + // start time or the catalog mtime was unreadable. Neither the cause nor the + // remedy is established, and a restart cannot clear it, so the guidance must + // not assert either. + expect(unknown).not.toContain("The model catalog changed after Codex started"); + expect(unknown).not.toContain("until Codex restarts"); + expect(unknown).toContain("Could not verify"); + + // The suppression itself is correct in both states and must not regress: + // withhold the override instruction and any positive model claims. + for (const text of [stale, unknown]) { + expect(text).toContain("do not set"); + expect(text).not.toContain("Preferred sub-agent"); + expect(text).not.toContain("Available models"); + } + + // The two states must stay distinguishable to an operator reading the turn. + expect(unknown).not.toBe(stale); + }); + test("v2 built-in guidance is schema-agnostic and keeps fork rules", async () => { const dir = codexHomeFixture(V2_ON); catalogFixture(dir, [{