From 52da777d96c4e8b3e8c6577ecfdb32e5ae60a36d Mon Sep 17 00:00:00 2001 From: Ingwannu Date: Mon, 17 Aug 2026 00:20:07 +0000 Subject: [PATCH 1/3] fix(windows): keep catalog discovery off request event loop --- .../content/docs/guides/sub-agent-surface.md | 6 + src/codex/app-server-processes.ts | 283 +++++++++++++++--- src/server/responses/collaboration.ts | 7 +- structure/03_catalog-and-subagents.md | 10 + tests/codex-app-server-processes.test.ts | 87 ++++++ 5 files changed, 354 insertions(+), 39 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 6d09880fc6..8fb0e4d49e 100644 --- a/docs-site/src/content/docs/guides/sub-agent-surface.md +++ b/docs-site/src/content/docs/guides/sub-agent-surface.md @@ -259,6 +259,12 @@ timestamp, an unreadable process start time, or a failed process enumeration — 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`. +On Windows, this advisory check uses asynchronous PowerShell/CIM discovery on the v2 request path. +Concurrent cold checks share one in-flight discovery and successful results are cached briefly. A +slow or failing CIM query can delay or suppress only OpenCodex-authored model guidance; it does not +block the Bun event loop, `/healthz`, or unrelated proxy traffic. Explicit CLI/service lifecycle +operations retain the synchronous, fail-closed process collector because they may signal processes. + Only a real change counts. A sync whose result is byte-identical to the catalog already on disk leaves the file untouched, so restarting the proxy or re-syncing an unchanged model set does not make a running Codex look stale. diff --git a/src/codex/app-server-processes.ts b/src/codex/app-server-processes.ts index c70c3af5c5..8d64b829d2 100644 --- a/src/codex/app-server-processes.ts +++ b/src/codex/app-server-processes.ts @@ -7,7 +7,7 @@ * Never match broad `*codex*` patterns that hit unrelated tools such as * `hermes-codex-bridge-mcp`. */ -import { execFileSync } from "node:child_process"; +import { execFile, execFileSync, type ExecFileException } from "node:child_process"; import { existsSync, readdirSync, readFileSync, statSync } from "node:fs"; import { isProcessAlive, waitForExit } from "../lib/process-control"; import { @@ -98,9 +98,30 @@ export interface CodexAppServerProcessIo { waitExit?: (pid: number, timeoutMs: number) => boolean; now?: () => number; readStartMs?: (pid: number) => number | null; + /** Async process-list seam used by the request-path Windows collector. */ + listSnapshotsAsync?: () => Promise; + /** Async batch start-time seam used by the request-path Windows collector. */ + readStartMsBatchAsync?: (pids: readonly number[]) => Promise>; catalogMtimeMs?: () => number | null; } +function execFileTextAsync( + file: string, + args: readonly string[], + timeoutMs: number, +): Promise { + return new Promise((resolve, reject) => { + execFile(file, [...args], { + encoding: "utf-8", + timeout: timeoutMs, + windowsHide: true, + }, (error: ExecFileException | null, stdout: string) => { + if (error) reject(error); + else resolve(stdout); + }); + }); +} + /** Split a process command line into argv-like tokens (handles simple quotes). */ export function tokenizeCommandLine(commandLine: string): string[] { const tokens: string[] = []; @@ -375,7 +396,7 @@ export function parseWindowsSnapshotOutput(output: string): ProcessSnapshot[] { return out; } -export function listWindowsSnapshots(runPowerShell?: (psCommand: string) => string): ProcessSnapshot[] { +function windowsSnapshotPowerShellCommand(): string { // Newlines keep -Command as a real script (space-joined statements need ';'). // Double-quoted format string so `t expands to a real tab. // Codex candidates only: basename token codex / codex.exe / codex.cmd / @@ -384,7 +405,7 @@ export function listWindowsSnapshots(runPowerShell?: (psCommand: string) => stri // path with "opencodex". const basenameMatch = powerShellSingleQuotedIgnoreCaseMatch(WINDOWS_CODEX_BASENAME_CANDIDATE_RE.source); const codeModeMatch = powerShellSingleQuotedIgnoreCaseMatch(WINDOWS_CODEX_CODE_MODE_HOST_CANDIDATE_RE.source); - const psCommand = [ + return [ "$ErrorActionPreference='SilentlyContinue'", "$me=[System.Security.Principal.WindowsIdentity]::GetCurrent().Name", // -ErrorAction Stop plus the outer try is what makes a TOP-LEVEL query failure @@ -412,9 +433,13 @@ export function listWindowsSnapshots(runPowerShell?: (psCommand: string) => stri "}", "} catch { \"__OCX_ENUM_INCOMPLETE__\" }", ].join("\n"); +} + +export function listWindowsSnapshots(runPowerShell?: (psCommand: string) => string): ProcessSnapshot[] { // Top-level exec failure propagates (see listDarwinSnapshots note). The // executable resolves from the trusted System32 directory (never PATH), and // windowsHide keeps the enumeration console-less on desktop sessions (#1278). + const psCommand = windowsSnapshotPowerShellCommand(); const output = runPowerShell ? runPowerShell(psCommand) : execFileSync(resolveTrustedWindowsPowerShellExe(), [ @@ -425,6 +450,15 @@ export function listWindowsSnapshots(runPowerShell?: (psCommand: string) => stri return parseWindowsSnapshotOutput(output); } +async function listWindowsSnapshotsAsync(): Promise { + const output = await execFileTextAsync(resolveTrustedWindowsPowerShellExe(), [ + "-NoProfile", "-NoLogo", "-NonInteractive", + "-Command", + windowsSnapshotPowerShellCommand(), + ], 8_000); + return parseWindowsSnapshotOutput(output); +} + function defaultListSnapshots(platform: NodeJS.Platform, getuid: () => number | undefined): ProcessSnapshot[] { if (platform === "win32") return listWindowsSnapshots(); if (platform === "darwin") return listDarwinSnapshots(getuid()); @@ -533,6 +567,41 @@ export function readProcessStartMs(pid: number, platform: NodeJS.Platform = proc return readLinuxProcStartMs(pid); } +function windowsProcessStartPowerShellCommand(pids: readonly number[]): string { + const filter = pids.map(pid => `ProcessId=${pid}`).join(" OR "); + return `Get-CimInstance Win32_Process -Filter "${filter}" | ForEach-Object { "$($_.ProcessId)\t$($_.CreationDate.ToUniversalTime().ToString("o"))" }`; +} + +function parseWindowsProcessStartTimes( + stdout: string, + pids: readonly number[], +): Map { + const byPid = new Map(); + for (const line of stdout.split(/\r?\n/)) { + const tab = line.indexOf("\t"); + if (tab <= 0) continue; + const pid = Number(line.slice(0, tab)); + const parsed = Date.parse(line.slice(tab + 1).trim()); + if (Number.isSafeInteger(pid) && Number.isFinite(parsed)) byPid.set(pid, parsed); + } + return new Map(pids.map(pid => [pid, byPid.get(pid) ?? null])); +} + +async function readWindowsProcessStartMsBatchAsync( + pids: readonly number[], +): Promise> { + try { + const stdout = await execFileTextAsync(resolveTrustedWindowsPowerShellExe(), [ + "-NoProfile", "-NoLogo", "-NonInteractive", + "-Command", + windowsProcessStartPowerShellCommand(pids), + ], 5_000); + return parseWindowsProcessStartTimes(stdout, pids); + } catch { + return new Map(pids.map(pid => [pid, null])); + } +} + /** * Start times for many pids in ONE platform call where possible, so the * staleness check does not serialize per-process ps/PowerShell invocations @@ -568,22 +637,12 @@ export function readProcessStartMsBatch( } if (platform === "win32") { try { - const filter = pids.map(pid => `ProcessId=${pid}`).join(" OR "); const stdout = execFileSync(resolveTrustedWindowsPowerShellExe(), [ "-NoProfile", "-NoLogo", "-NonInteractive", "-Command", - `Get-CimInstance Win32_Process -Filter "${filter}" | ForEach-Object { "$($_.ProcessId)\t$($_.CreationDate.ToUniversalTime().ToString("o"))" }`, + windowsProcessStartPowerShellCommand(pids), ], { encoding: "utf-8", stdio: ["ignore", "pipe", "ignore"], timeout: 5_000, windowsHide: true }); - const byPid = new Map(); - for (const line of stdout.split(/\r?\n/)) { - const tab = line.indexOf("\t"); - if (tab <= 0) continue; - const pid = Number(line.slice(0, tab)); - const parsed = Date.parse(line.slice(tab + 1).trim()); - if (Number.isSafeInteger(pid) && Number.isFinite(parsed)) byPid.set(pid, parsed); - } - for (const pid of pids) out.set(pid, byPid.get(pid) ?? null); - return out; + return parseWindowsProcessStartTimes(stdout, pids); } catch { for (const pid of pids) out.set(pid, null); return out; @@ -610,9 +669,65 @@ function defaultCatalogMtimeMs(): number | null { } } +function codexAppServerProcessesFromSnapshots( + snapshots: readonly ProcessSnapshot[], +): CodexAppServerProcess[] { + const processes: CodexAppServerProcess[] = []; + const seen = new Set(); + for (const snapshot of snapshots) { + if (seen.has(snapshot.pid)) continue; + if (!isCodexAppServerCommandLine(snapshot.commandLine, snapshot.executable)) continue; + seen.add(snapshot.pid); + processes.push({ pid: snapshot.pid, commandLine: snapshot.commandLine }); + } + return processes; +} + +function catalogStatusFromProcesses( + processes: readonly CodexAppServerProcess[], + catalogMtimeMs: number | null, + starts: ReadonlyMap, +): CodexAppServerCatalogStatus { + const withStarts = processes.map(proc => ({ + pid: proc.pid, + startedAtMs: starts.get(proc.pid) ?? null, + })); + if (catalogMtimeMs === null || withStarts.some(proc => proc.startedAtMs === null)) { + return { state: "unknown", processes: withStarts, catalogMtimeMs }; + } + // `<=` is deliberate: coarse clocks (ps lstart is second-granularity) can + // report equal values when the catalog actually changed after startup. + const stale = withStarts.some(proc => proc.startedAtMs! <= catalogMtimeMs); + return { state: stale ? "stale" : "fresh", processes: withStarts, catalogMtimeMs }; +} + // Short TTL: process listing + stat run once per window even under per-turn // guidance calls (#857). let catalogStateCache: { atMs: number; status: CodexAppServerCatalogStatus } | null = null; +interface RequestCatalogStateIdentity { + platform: NodeJS.Platform; + listSnapshots?: CodexAppServerProcessIo["listSnapshots"]; + listSnapshotsAsync?: CodexAppServerProcessIo["listSnapshotsAsync"]; + readStartMs?: CodexAppServerProcessIo["readStartMs"]; + readStartMsBatchAsync?: CodexAppServerProcessIo["readStartMsBatchAsync"]; + catalogMtimeMs?: CodexAppServerProcessIo["catalogMtimeMs"]; + now?: CodexAppServerProcessIo["now"]; +} + +interface RequestCatalogStateFlight { + generation: number; + identity: RequestCatalogStateIdentity; + promise: Promise; +} + +let requestCatalogStateGeneration = 0; +let requestCatalogStateCache: { + generation: number; + identity: RequestCatalogStateIdentity; + atMs: number; + status: CodexAppServerCatalogStatus; +} | null = null; +let requestCatalogStateFlight: RequestCatalogStateFlight | null = null; const CATALOG_STATE_TTL_MS = 5_000; /** * `unknown` is a failure to observe, not an observation, so it gets a much shorter @@ -627,6 +742,19 @@ export function catalogStateTtlMs(state: CodexAppServerCatalogState): number { return state === "unknown" ? CATALOG_STATE_UNKNOWN_TTL_MS : CATALOG_STATE_TTL_MS; } +function sameRequestCatalogStateIdentity( + left: RequestCatalogStateIdentity, + right: RequestCatalogStateIdentity, +): boolean { + return left.platform === right.platform + && left.listSnapshots === right.listSnapshots + && left.listSnapshotsAsync === right.listSnapshotsAsync + && left.readStartMs === right.readStartMs + && left.readStartMsBatchAsync === right.readStartMsBatchAsync + && left.catalogMtimeMs === right.catalogMtimeMs + && left.now === right.now; +} + /** * Compare the on-disk catalog mtime against the start time of running Codex * app-servers (#857): a server that started before the catalog changed keeps @@ -676,33 +804,17 @@ export function collectCodexAppServerCatalogState( snapshots = []; enumerationFailed = true; } - const processes: CodexAppServerProcess[] = []; - const seen = new Set(); - for (const snapshot of snapshots) { - if (seen.has(snapshot.pid)) continue; - if (!isCodexAppServerCommandLine(snapshot.commandLine, snapshot.executable)) continue; - seen.add(snapshot.pid); - processes.push({ pid: snapshot.pid, commandLine: snapshot.commandLine }); - } + const processes = codexAppServerProcessesFromSnapshots(snapshots); if (processes.length === 0) { return enumerationFailed ? { state: "unknown", processes: [], catalogMtimeMs: null } : { state: "not_running", processes: [], catalogMtimeMs: null }; } const catalogMtimeMs = (io.catalogMtimeMs ?? defaultCatalogMtimeMs)(); - const withStarts = io.readStartMs - ? processes.map(proc => ({ pid: proc.pid, startedAtMs: io.readStartMs!(proc.pid) })) - : (() => { - const batch = readProcessStartMsBatch(processes.map(proc => proc.pid), platform); - return processes.map(proc => ({ pid: proc.pid, startedAtMs: batch.get(proc.pid) ?? null })); - })(); - if (catalogMtimeMs === null || withStarts.some(proc => proc.startedAtMs === null)) { - return { state: "unknown", processes: withStarts, catalogMtimeMs }; - } - // `<=` is deliberate: coarse clocks (ps lstart is second-granularity) can - // report equal values when the catalog actually changed after startup. - const stale = withStarts.some(proc => proc.startedAtMs! <= catalogMtimeMs); - return { state: stale ? "stale" : "fresh", processes: withStarts, catalogMtimeMs }; + const starts = io.readStartMs + ? new Map(processes.map(proc => [proc.pid, io.readStartMs!(proc.pid)] as const)) + : readProcessStartMsBatch(processes.map(proc => proc.pid), platform); + return catalogStatusFromProcesses(processes, catalogMtimeMs, starts); }; const status = compute(); if (fullyDefault) { @@ -711,9 +823,108 @@ export function collectCodexAppServerCatalogState( return status; } +/** + * Request-path catalog state collector. + * + * [Decision Log] + * - 목적과 의도: keep Windows CIM discovery from blocking Bun's event loop while v2 guidance is built. + * - 기존 구현 및 제약 조건: CLI/service operations still need the synchronous, fail-closed collector; the request path needs only advisory state. + * - 검토한 주요 대안: remove stale-catalog guidance, move all process work to workers, or add a Windows-only async boundary. + * - 선택한 방식: retain the synchronous API and use async PowerShell plus an identity-scoped in-flight refresh, short cache, and invalidation generation only for Windows requests. + * - 다른 대안 대신 이 방식을 선택한 이유: it fixes unrelated `/healthz` starvation without widening the process-matching or restart contract. + * - 장점, 단점 및 영향: concurrent turns share one CIM walk, invalidated pre-write results cannot repopulate the cache, and the event loop stays responsive; a cold v2 turn can still await the bounded advisory probe. + */ +export async function collectCodexAppServerCatalogStateForRequest( + io: CodexAppServerProcessIo = {}, +): Promise { + const platform = io.platform ?? process.platform; + if (platform !== "win32") return collectCodexAppServerCatalogState(io); + + const now = (io.now ?? Date.now)(); + const generation = requestCatalogStateGeneration; + const identity: RequestCatalogStateIdentity = { + platform, + listSnapshots: io.listSnapshots, + listSnapshotsAsync: io.listSnapshotsAsync, + readStartMs: io.readStartMs, + readStartMsBatchAsync: io.readStartMsBatchAsync, + catalogMtimeMs: io.catalogMtimeMs, + now: io.now, + }; + if (requestCatalogStateCache + && requestCatalogStateCache.generation === generation + && sameRequestCatalogStateIdentity(requestCatalogStateCache.identity, identity) + && now - requestCatalogStateCache.atMs < CATALOG_STATE_TTL_MS) { + return requestCatalogStateCache.status; + } + if (requestCatalogStateFlight + && requestCatalogStateFlight.generation === generation + && sameRequestCatalogStateIdentity(requestCatalogStateFlight.identity, identity)) { + return requestCatalogStateFlight.promise; + } + + const refresh = async (): Promise => { + let snapshots: ProcessSnapshot[]; + try { + snapshots = io.listSnapshotsAsync + ? await io.listSnapshotsAsync() + : io.listSnapshots + ? io.listSnapshots() + : await listWindowsSnapshotsAsync(); + } catch { + return { state: "unknown", processes: [], catalogMtimeMs: null }; + } + const processes = codexAppServerProcessesFromSnapshots(snapshots); + if (processes.length === 0) { + return { state: "not_running", processes: [], catalogMtimeMs: null }; + } + let catalogMtimeMs: number | null; + try { + catalogMtimeMs = (io.catalogMtimeMs ?? defaultCatalogMtimeMs)(); + } catch { + catalogMtimeMs = null; + } + const pids = processes.map(proc => proc.pid); + const starts = io.readStartMsBatchAsync + ? await io.readStartMsBatchAsync(pids) + : io.readStartMs + ? new Map(pids.map(pid => [pid, io.readStartMs!(pid)] as const)) + : await readWindowsProcessStartMsBatchAsync(pids); + return catalogStatusFromProcesses(processes, catalogMtimeMs, starts); + }; + + const pending = refresh().catch(() => ({ + state: "unknown" as const, + processes: [], + catalogMtimeMs: null, + })); + let flight: RequestCatalogStateFlight; + const promise = pending.then(status => { + // A catalog write can invalidate while slow CIM is still running. Never + // let that pre-write result repopulate the post-write cache. + if (requestCatalogStateGeneration === generation && requestCatalogStateFlight === flight) { + requestCatalogStateCache = { + generation, + identity, + atMs: (io.now ?? Date.now)(), + status, + }; + } + return status; + }).finally(() => { + if (requestCatalogStateFlight === flight) requestCatalogStateFlight = null; + }); + flight = { generation, identity, promise }; + requestCatalogStateFlight = flight; + return flight.promise; +} + /** Test hook: drop the memoized catalog state. */ export function resetCodexAppServerCatalogStateCache(): void { catalogStateCache = null; + requestCatalogStateGeneration += 1; + requestCatalogStateCache = null; + requestCatalogStateFlight = null; } export interface RestartCodexAppServersResult { diff --git a/src/server/responses/collaboration.ts b/src/server/responses/collaboration.ts index 37fca4777f..7ab1037d13 100644 --- a/src/server/responses/collaboration.ts +++ b/src/server/responses/collaboration.ts @@ -197,7 +197,8 @@ export interface MultiAgentGuidanceDeps { configuredModels: readonly string[], surface: SpawnAgentSurface, ) => EffectiveSubagentRoster | Promise; - collectCatalogState?: () => { state: "fresh" | "stale" | "not_running" | "unknown" }; + collectCatalogState?: () => { state: "fresh" | "stale" | "not_running" | "unknown" } + | Promise<{ state: "fresh" | "stale" | "not_running" | "unknown" }>; } async function defaultCollectCatalogState(): Promise<{ state: "fresh" | "stale" | "not_running" | "unknown" }> { @@ -207,8 +208,8 @@ async function defaultCollectCatalogState(): Promise<{ state: "fresh" | "stale" if (override === "fresh" || override === "stale" || override === "not_running" || override === "unknown") { return { state: override }; } - const { collectCodexAppServerCatalogState } = await import("../../codex/app-server-processes"); - return collectCodexAppServerCatalogState(); + const { collectCodexAppServerCatalogStateForRequest } = await import("../../codex/app-server-processes"); + return collectCodexAppServerCatalogStateForRequest(); } diff --git a/structure/03_catalog-and-subagents.md b/structure/03_catalog-and-subagents.md index fe347602d4..5109ba2c86 100644 --- a/structure/03_catalog-and-subagents.md +++ b/structure/03_catalog-and-subagents.md @@ -59,6 +59,16 @@ deleting, or editing a provider's shape clears that per-provider cache; a disabl deliberately does not, because a disabled provider is already excluded from the catalog gather instead. Codex's own `models_cache.json` is a different cache, invalidated by catalog refresh. +### Windows request-path catalog-state discovery + +[Decision Log] +- 목적과 의도: Prevent Windows PowerShell/CIM process discovery from blocking Bun's event loop while v2 sub-agent guidance is assembled. +- 기존 구현 및 제약 조건: The stale-catalog check is advisory on the request path, but CLI/service lifecycle operations use the same process evidence before warning or terminating narrowly matched app-servers. +- 검토한 주요 대안: Remove stale-catalog guidance, move every platform collector into workers, or isolate only the Windows request path behind asynchronous child processes. +- 선택한 방식: Keep the synchronous fail-closed collector for explicit lifecycle operations; v2 requests use asynchronous trusted-System32 PowerShell, one identity-scoped in-flight refresh, and the existing short cache. Cache invalidation advances a generation so a pre-write CIM result cannot repopulate post-write state. +- 다른 대안 대신 이 방식을 선택한 이유: This preserves process ownership and matching invariants while preventing a slow CIM query from starving `/healthz` and unrelated proxy traffic. +- 장점, 단점 및 영향: Concurrent v2 turns do not multiply CIM walks and the event loop remains responsive. A cold request can still await the bounded advisory check, and collection failure suppresses OpenCodex-authored model guidance as `unknown`. + ## Startup readiness Each `startServer` invocation owns a private, one-shot readiness gate created before the listener diff --git a/tests/codex-app-server-processes.test.ts b/tests/codex-app-server-processes.test.ts index 4a63be2f2e..0a363c2523 100644 --- a/tests/codex-app-server-processes.test.ts +++ b/tests/codex-app-server-processes.test.ts @@ -8,6 +8,7 @@ import { attachStaleAppServerHint, catalogStateTtlMs, collectCodexAppServerCatalogState, + collectCodexAppServerCatalogStateForRequest, formatStaleCodexAppServerWarning, isCodexAppServerCommandLine, isWindowsCodexCandidateCommandLine, @@ -74,6 +75,92 @@ describe("collectCodexAppServerCatalogState (#857)", () => { expect(noCatalog.state).toBe("unknown"); }); + test("Windows request collection yields to the event loop while CIM enumeration is slow (#1852)", async () => { + let releaseSnapshots: ((snapshots: Array<{ pid: number; commandLine: string }>) => void) | undefined; + const snapshots = new Promise>(resolve => { + releaseSnapshots = resolve; + }); + const collection = collectCodexAppServerCatalogStateForRequest({ + platform: "win32", + listSnapshotsAsync: () => snapshots, + readStartMsBatchAsync: async pids => new Map(pids.map(pid => [pid, 2_000])), + catalogMtimeMs: () => 1_000, + }); + + const first = await Promise.race([ + collection.then(() => "collection"), + new Promise<"timer">(resolve => setTimeout(() => resolve("timer"), 10)), + ]); + expect(first).toBe("timer"); + + releaseSnapshots?.([{ pid: 42, commandLine: APP_SERVER_CMD }]); + await expect(collection).resolves.toMatchObject({ state: "fresh" }); + }); + + test("Windows request collection shares one in-flight refresh and its short cache (#1852)", async () => { + resetCodexAppServerCatalogStateCache(); + let calls = 0; + let now = 1_000; + const io = { + platform: "win32" as const, + now: () => now, + listSnapshotsAsync: async () => { + calls += 1; + await Bun.sleep(10); + return [{ pid: 42, commandLine: APP_SERVER_CMD }]; + }, + readStartMsBatchAsync: async (pids: readonly number[]) => new Map(pids.map(pid => [pid, 2_000])), + catalogMtimeMs: () => 1_000, + }; + + const [first, joined] = await Promise.all([ + collectCodexAppServerCatalogStateForRequest(io), + collectCodexAppServerCatalogStateForRequest(io), + ]); + expect(first.state).toBe("fresh"); + expect(joined).toBe(first); + expect(calls).toBe(1); + + now += 4_999; + expect((await collectCodexAppServerCatalogStateForRequest(io)).state).toBe("fresh"); + expect(calls).toBe(1); + + now += 2; + expect((await collectCodexAppServerCatalogStateForRequest(io)).state).toBe("fresh"); + expect(calls).toBe(2); + resetCodexAppServerCatalogStateCache(); + }); + + test("cache invalidation cannot be undone by an older in-flight Windows refresh (#1852)", async () => { + resetCodexAppServerCatalogStateCache(); + let calls = 0; + let releaseFirst: ((snapshots: Array<{ pid: number; commandLine: string }>) => void) | undefined; + const firstSnapshots = new Promise>(resolve => { + releaseFirst = resolve; + }); + const io = { + platform: "win32" as const, + listSnapshotsAsync: async () => { + calls += 1; + if (calls === 1) return firstSnapshots; + return []; + }, + readStartMsBatchAsync: async (pids: readonly number[]) => new Map(pids.map(pid => [pid, 2_000])), + catalogMtimeMs: () => 1_000, + }; + + const staleFlight = collectCodexAppServerCatalogStateForRequest(io); + resetCodexAppServerCatalogStateCache(); + releaseFirst?.([{ pid: 42, commandLine: APP_SERVER_CMD }]); + await expect(staleFlight).resolves.toMatchObject({ state: "fresh" }); + + await expect(collectCodexAppServerCatalogStateForRequest(io)).resolves.toMatchObject({ + state: "not_running", + }); + expect(calls).toBe(2); + resetCodexAppServerCatalogStateCache(); + }); + test("unrelated processes never enter the comparison", () => { const status = collectCodexAppServerCatalogState({ listSnapshots: () => [ From b2563de1526c42f2dc1c76e51801f1c66778fff4 Mon Sep 17 00:00:00 2001 From: Ingwannu Date: Mon, 17 Aug 2026 00:38:50 +0000 Subject: [PATCH 2/3] test(windows): pin request catalog cache boundaries --- .../content/docs/guides/sub-agent-surface.md | 2 +- src/codex/app-server-processes.ts | 7 ++++- tests/codex-app-server-processes.test.ts | 27 +++++++++++++++++++ tests/multi-agent-compat.test.ts | 2 +- 4 files changed, 35 insertions(+), 3 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 8fb0e4d49e..e5773a4f4d 100644 --- a/docs-site/src/content/docs/guides/sub-agent-surface.md +++ b/docs-site/src/content/docs/guides/sub-agent-surface.md @@ -260,7 +260,7 @@ separately by `ocx doctor`. `stale` clears only after every detected Codex app-s the final catalog write; it does not necessarily clear `unknown`. On Windows, this advisory check uses asynchronous PowerShell/CIM discovery on the v2 request path. -Concurrent cold checks share one in-flight discovery and successful results are cached briefly. A +Concurrent cold checks share one in-flight discovery and results are cached briefly. A slow or failing CIM query can delay or suppress only OpenCodex-authored model guidance; it does not block the Bun event loop, `/healthz`, or unrelated proxy traffic. Explicit CLI/service lifecycle operations retain the synchronous, fail-closed process collector because they may signal processes. diff --git a/src/codex/app-server-processes.ts b/src/codex/app-server-processes.ts index 8d64b829d2..086c1d95ff 100644 --- a/src/codex/app-server-processes.ts +++ b/src/codex/app-server-processes.ts @@ -919,7 +919,12 @@ export async function collectCodexAppServerCatalogStateForRequest( return flight.promise; } -/** Test hook: drop the memoized catalog state. */ +/** + * Drop memoized catalog state after a relevant catalog/cache write and before + * the post-write state read. Advancing the generation prevents an older + * in-flight Windows CIM refresh from publishing its pre-write result after the + * write has completed. + */ export function resetCodexAppServerCatalogStateCache(): void { catalogStateCache = null; requestCatalogStateGeneration += 1; diff --git a/tests/codex-app-server-processes.test.ts b/tests/codex-app-server-processes.test.ts index 0a363c2523..487c3aa3e9 100644 --- a/tests/codex-app-server-processes.test.ts +++ b/tests/codex-app-server-processes.test.ts @@ -161,6 +161,33 @@ describe("collectCodexAppServerCatalogState (#857)", () => { resetCodexAppServerCatalogStateCache(); }); + test("Windows request collection briefly caches failed CIM enumeration (#1852)", async () => { + resetCodexAppServerCatalogStateCache(); + let calls = 0; + let now = 1_000; + const io = { + platform: "win32" as const, + now: () => now, + listSnapshotsAsync: async () => { + calls += 1; + throw new Error("windows_enum_incomplete"); + }, + catalogMtimeMs: () => 1_000, + }; + + await expect(collectCodexAppServerCatalogStateForRequest(io)).resolves.toMatchObject({ + state: "unknown", + }); + now += 10; + await expect(collectCodexAppServerCatalogStateForRequest(io)).resolves.toMatchObject({ + state: "unknown", + }); + // Failure is advisory and fail-closed, but caching it briefly prevents a + // broken CIM provider from spawning one PowerShell process per request. + expect(calls).toBe(1); + resetCodexAppServerCatalogStateCache(); + }); + test("unrelated processes never enter the comparison", () => { const status = collectCodexAppServerCatalogState({ listSnapshots: () => [ diff --git a/tests/multi-agent-compat.test.ts b/tests/multi-agent-compat.test.ts index 360727a00a..afbddab3d9 100644 --- a/tests/multi-agent-compat.test.ts +++ b/tests/multi-agent-compat.test.ts @@ -129,7 +129,7 @@ describe("multiAgentGuidanceText", () => { for (const state of ["stale", "unknown"] as const) { const text = await multiAgentGuidanceText(parsed, options, { - collectCatalogState: () => ({ state }), + collectCatalogState: async () => ({ state }), }); // #1395: withhold OpenCodex's disk-derived claims, but do not prohibit // options the active spawn_agent tool advertises — the global catalog From 125156c3e1d0ed3c339978b6e84aacd21296582b Mon Sep 17 00:00:00 2001 From: Ingwannu Date: Tue, 18 Aug 2026 12:35:38 +0000 Subject: [PATCH 3/3] fix(windows): preserve short unknown catalog retries --- docs-site/src/content/docs/guides/sub-agent-surface.md | 3 ++- src/codex/app-server-processes.ts | 2 +- tests/codex-app-server-processes.test.ts | 6 ++++++ 3 files changed, 9 insertions(+), 2 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 e5773a4f4d..4a56c810bd 100644 --- a/docs-site/src/content/docs/guides/sub-agent-surface.md +++ b/docs-site/src/content/docs/guides/sub-agent-surface.md @@ -260,7 +260,8 @@ separately by `ocx doctor`. `stale` clears only after every detected Codex app-s the final catalog write; it does not necessarily clear `unknown`. On Windows, this advisory check uses asynchronous PowerShell/CIM discovery on the v2 request path. -Concurrent cold checks share one in-flight discovery and results are cached briefly. A +Concurrent cold checks share one in-flight discovery. Observed states are cached for five seconds; +an `unknown` failure is cached for only 250 milliseconds so a transient CIM error retries quickly. A slow or failing CIM query can delay or suppress only OpenCodex-authored model guidance; it does not block the Bun event loop, `/healthz`, or unrelated proxy traffic. Explicit CLI/service lifecycle operations retain the synchronous, fail-closed process collector because they may signal processes. diff --git a/src/codex/app-server-processes.ts b/src/codex/app-server-processes.ts index 086c1d95ff..d10f26a2ae 100644 --- a/src/codex/app-server-processes.ts +++ b/src/codex/app-server-processes.ts @@ -854,7 +854,7 @@ export async function collectCodexAppServerCatalogStateForRequest( if (requestCatalogStateCache && requestCatalogStateCache.generation === generation && sameRequestCatalogStateIdentity(requestCatalogStateCache.identity, identity) - && now - requestCatalogStateCache.atMs < CATALOG_STATE_TTL_MS) { + && now - requestCatalogStateCache.atMs < catalogStateTtlMs(requestCatalogStateCache.status.state)) { return requestCatalogStateCache.status; } if (requestCatalogStateFlight diff --git a/tests/codex-app-server-processes.test.ts b/tests/codex-app-server-processes.test.ts index 487c3aa3e9..e6707787f0 100644 --- a/tests/codex-app-server-processes.test.ts +++ b/tests/codex-app-server-processes.test.ts @@ -185,6 +185,12 @@ describe("collectCodexAppServerCatalogState (#857)", () => { // Failure is advisory and fail-closed, but caching it briefly prevents a // broken CIM provider from spawning one PowerShell process per request. expect(calls).toBe(1); + + now += 241; + await expect(collectCodexAppServerCatalogStateForRequest(io)).resolves.toMatchObject({ + state: "unknown", + }); + expect(calls).toBe(2); resetCodexAppServerCatalogStateCache(); });