From 550091d01126fe9fa6710c86b676c2d61769c475 Mon Sep 17 00:00:00 2001 From: Abhishek Sharma Date: Sun, 9 Aug 2026 23:33:33 -0700 Subject: [PATCH 1/2] fix(collaboration): scope catalog-state guidance to what we can attribute MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `collectCodexAppServerCatalogState()` folds every app-server owned by the current user into ONE global observation, and an inbound request carries no sender PID or catalog fingerprint. So a stale process A makes the global state `stale` even when the request came from a fresh process B, and `unknown` can be reached by a process-enumeration failure that says nothing about any particular server. v2 guidance nonetheless injected do not set model or reasoning_effort overrides until Codex restarts off that global observation. Two problems follow: * it prohibits options the active `spawn_agent` tool legitimately advertises, for a request that cannot be attributed to the stale process (#1395); and * the same text was emitted for `unknown`, asserting a cause that was never established and a restart that cannot clear a failed measurement (#1354). For `stale` and `unknown`, withhold opencodex's own disk-derived claims — preferred model, roster, fallback, custom guidance — and say nothing about overrides, leaving the active tool schema authoritative. `fresh` and `not_running` are unchanged. The previous regression asserted that both states contain `do not set`, which codified the scoping bug; it is replaced by coverage that a mixed stale/fresh process set resolving globally to `stale` produces no blanket no-override instruction for an unidentified request. Documents the four-state guidance contract in the sub-agent surface guide, including why `unknown` is not a synonym for `stale`. Verified: bun test tests/multi-agent-compat.test.ts -> 43 pass, 0 fail bun test tests/codex-app-server-processes.test.ts -> 30 pass, 1 skip, 0 fail bun x tsc --noEmit -> clean Fixes #1354 Fixes #1395 --- .../content/docs/guides/sub-agent-surface.md | 26 +++++++ src/server/responses/collaboration.ts | 19 ++++- tests/multi-agent-compat.test.ts | 71 ++++++++++++++++++- 3 files changed, 111 insertions(+), 5 deletions(-) diff --git a/docs-site/src/content/docs/guides/sub-agent-surface.md b/docs-site/src/content/docs/guides/sub-agent-surface.md index ab690bf6a..c03b52e62 100644 --- a/docs-site/src/content/docs/guides/sub-agent-surface.md +++ b/docs-site/src/content/docs/guides/sub-agent-surface.md @@ -204,6 +204,32 @@ to v1. A `"v2"`, `null`, or absent surface value is eligible; a real `"v1"` pin No. Start a new Codex session after changing the mode. If a long-running App host still shows stale catalog state, run `ocx sync` and restart that Codex surface. +### What happens when opencodex cannot trust the catalog? + +opencodex compares the on-disk model catalog against the start time of every Codex app-server owned +by the current user, producing one of four states: + +| State | Meaning | v2 guidance | +|---|---|---| +| `fresh` | Every app-server started after the catalog was written | Full guidance: preferred model, roster, fallbacks | +| `not_running` | No app-server detected | Full guidance | +| `stale` | At least one app-server predates the catalog | **No opencodex-authored model guidance** | +| `unknown` | The comparison could not be made | **No opencodex-authored model guidance** | + +For `stale` and `unknown`, opencodex withholds its own disk-derived claims — preferred model, roster, +fallback and custom guidance — because the running Codex may not be able to spawn what the disk +catalog advertises. + +It does **not** instruct the model to stop setting `model` or `reasoning_effort`. That observation is +global across every app-server for the user, while an inbound request carries no sender identity, so +a stale process cannot be attributed to the request in front of us. Prohibiting overrides on that +basis would block options the active `spawn_agent` tool legitimately advertises, for a session that +may well be fresh. The active tool schema stays authoritative. + +`unknown` is not a synonym for `stale`. It means the comparison itself failed — an unreadable catalog +timestamp, an unreadable process start time, or a failed process enumeration — and it is reported +separately by `ocx doctor`. Restarting Codex clears `stale`; it does not necessarily clear `unknown`. + ### Reasoning effort `injectionEffort` affects only delegated-worker guidance and, when explicitly enabled, native Codex diff --git a/src/server/responses/collaboration.ts b/src/server/responses/collaboration.ts index 0b28267ce..17e28b245 100644 --- a/src/server/responses/collaboration.ts +++ b/src/server/responses/collaboration.ts @@ -249,9 +249,24 @@ 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)(); + // #1354 / #1395: `collectCodexAppServerCatalogState()` folds every app-server + // owned by the current user into ONE global observation, and the inbound + // request carries no sender PID or catalog fingerprint. So a stale process A + // makes the global state `stale` even when this request came from a fresh + // process B, and `unknown` can be reached by a process-enumeration failure + // that says nothing about any particular server. + // + // Emitting "do not set model or reasoning_effort overrides" off that global + // observation prohibits options the active `spawn_agent` tool legitimately + // advertises, for a request we cannot attribute to the stale process. The + // safe behaviour is to withhold OpenCodex's own disk-derived claims — + // preferred model, roster, fallback, custom guidance — and stay silent about + // overrides, leaving the active tool schema authoritative. + // + // `fresh` and `not_running` are unchanged: there we can positively describe + // the catalog, so the guidance below still applies. if (catalogState.state === "stale" || catalogState.state === "unknown") { - return "The model catalog changed after Codex started; do not set " - + "model or reasoning_effort overrides until Codex restarts."; + return null; } // 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 diff --git a/tests/multi-agent-compat.test.ts b/tests/multi-agent-compat.test.ts index 8e9740cb1..8fa717dd0 100644 --- a/tests/multi-agent-compat.test.ts +++ b/tests/multi-agent-compat.test.ts @@ -11,6 +11,7 @@ import { injectDeveloperMessage, multiAgentGuidanceText, sanitizeEncryptedConten import { parseRequest } from "../src/responses/parser"; import type { OcxParsedRequest } from "../src/types"; import { CODEX_ACCOUNT_BOUND_CATALOG_KIND, effectiveSubagentRoster } from "../src/codex/catalog"; +import { collectCodexAppServerCatalogState } from "../src/codex/app-server-processes"; import { clearDebugSettings, setDebugSettings } from "../src/lib/debug-settings"; import { getInjectionDebugLogEntries, @@ -130,9 +131,10 @@ describe("multiAgentGuidanceText", () => { const text = await multiAgentGuidanceText(parsed, options, { collectCatalogState: () => ({ state }), }); - expect(text).toContain("do not set"); - expect(text).not.toContain("Preferred sub-agent"); - expect(text).not.toContain("Available models"); + // #1395: withhold OpenCodex's disk-derived claims, but do not prohibit + // options the active spawn_agent tool advertises — the global catalog + // observation cannot be attributed to the request that triggered it. + expect(text).toBeNull(); } for (const state of ["fresh", "not_running"] as const) { @@ -143,6 +145,69 @@ describe("multiAgentGuidanceText", () => { } }); + test("a mixed stale/fresh process set does not produce a blanket no-override instruction (#1395)", async () => { + // The scoping bug this guards: `collectCodexAppServerCatalogState()` folds + // every current-user app-server into ONE global observation. Process 42 + // predates the catalog and process 43 does not, so the global state is + // `stale` — but the inbound request carries no sender PID, so we cannot tell + // whether it came from the stale server or the fresh one. + const appServerCmd = "/usr/local/bin/codex app-server"; + const global = collectCodexAppServerCatalogState({ + listSnapshots: () => [ + { pid: 42, commandLine: appServerCmd }, + { pid: 43, commandLine: appServerCmd }, + ], + readStartMs: pid => (pid === 42 ? 500 : 3_000), + catalogMtimeMs: () => 1_000, + }); + expect(global.state).toBe("stale"); + + 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 text = await multiAgentGuidanceText(parsed, options, { + collectCatalogState: () => ({ state: global.state }), + }); + + // A request we cannot attribute to the stale process must not be told to + // stop setting model or reasoning_effort — that prohibits options the active + // spawn_agent tool legitimately advertises, for a session that may be fresh. + expect(text).toBeNull(); + }); + + test("stale and unknown withhold OpenCodex's own catalog claims (#1354, #1395)", 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" }; + + for (const state of ["stale", "unknown"] as const) { + const text = await multiAgentGuidanceText(parsed, options, { + collectCatalogState: () => ({ state }), + }); + // No preferred model, no roster, no fallback, and no override prohibition. + // The active tool schema stays authoritative. + expect(text).toBeNull(); + } + + // `fresh` and `not_running` are unchanged: there the catalog can be + // positively described, so the designation guidance still applies. + for (const state of ["fresh", "not_running"] as const) { + const text = await multiAgentGuidanceText(parsed, options, { + collectCatalogState: () => ({ state }), + }); + expect(text).toContain("Preferred sub-agent"); + } + }); + test("v2 built-in guidance is schema-agnostic and keeps fork rules", async () => { const dir = codexHomeFixture(V2_ON); catalogFixture(dir, [{ From 04222e364fe1aea53a88c09131f744e1c93b2348 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Tue, 11 Aug 2026 07:10:29 +0200 Subject: [PATCH 2/2] Update sub-agent-surface.md for clarity on states Clarify the distinction between 'stale' and 'unknown' states in the documentation. --- docs-site/src/content/docs/guides/sub-agent-surface.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs-site/src/content/docs/guides/sub-agent-surface.md b/docs-site/src/content/docs/guides/sub-agent-surface.md index c03b52e62..233975fad 100644 --- a/docs-site/src/content/docs/guides/sub-agent-surface.md +++ b/docs-site/src/content/docs/guides/sub-agent-surface.md @@ -228,7 +228,8 @@ may well be fresh. The active tool schema stays authoritative. `unknown` is not a synonym for `stale`. It means the comparison itself failed — an unreadable catalog timestamp, an unreadable process start time, or a failed process enumeration — and it is reported -separately by `ocx doctor`. Restarting Codex clears `stale`; it does not necessarily clear `unknown`. +separately by `ocx doctor`. `stale` clears only after every detected Codex app-server starts after +the final catalog write; it does not necessarily clear `unknown`. ### Reasoning effort