diff --git a/electron/providers/codex-app-server-runtime.ts b/electron/providers/codex-app-server-runtime.ts index 47241340..47ca8377 100644 --- a/electron/providers/codex-app-server-runtime.ts +++ b/electron/providers/codex-app-server-runtime.ts @@ -803,6 +803,7 @@ function createCodexConnectedToolStatusEntry(args: { class CodexAppServerClient { private process: ChildProcessWithoutNullStreams | null = null; + private processStartedAt: number | null = null; private startupPromise: Promise | null = null; private nextRequestId = 1; private pendingResponses = new Map< @@ -885,6 +886,10 @@ class CodexAppServerClient { return this.lastErrorMessage; } + getProcessStartedAt() { + return this.processStartedAt; + } + dispose(message = "Codex App Server closed.") { if (!this.process) { this.lastErrorMessage = message; @@ -898,6 +903,7 @@ class CodexAppServerClient { this.teardownProcess("Restarting Codex App Server."); } + const processStartedAt = Date.now(); const child = spawn( this.executablePath, ["app-server", "--listen", "stdio://"], @@ -911,6 +917,7 @@ class CodexAppServerClient { }, ); this.process = child; + this.processStartedAt = processStartedAt; this.initialized = false; const stdoutLineBuffer = new Utf8LineBuffer({ label: "codex-app-server stdout", @@ -1101,6 +1108,7 @@ class CodexAppServerClient { private teardownProcess(message: string) { const current = this.process; this.process = null; + this.processStartedAt = null; this.initialized = false; this.lastErrorMessage = message; if (current && !current.killed) { @@ -2375,10 +2383,12 @@ export async function streamCodexWithAppServer( codexGlobalMcpConfigRefreshTracker.check({ scopeKey: codexMcpScope, paths: codexMcpConfigPaths.globalPaths, + processStartedAt: client.getProcessStartedAt() ?? undefined, }), codexProjectMcpConfigRefreshTracker.check({ scopeKey: `${codexMcpScope}:${runtimeCwd}`, paths: codexMcpConfigPaths.projectPaths, + processStartedAt: client.getProcessStartedAt() ?? undefined, }), ]); if (globalMcpRefresh.changed || projectMcpRefresh.changed) { diff --git a/electron/providers/mcp-config-refresh.ts b/electron/providers/mcp-config-refresh.ts index 0aeeba8a..df51f2f2 100644 --- a/electron/providers/mcp-config-refresh.ts +++ b/electron/providers/mcp-config-refresh.ts @@ -147,22 +147,28 @@ export function getCodexMcpConfigPaths(args: CodexMcpConfigPathOptions) { } async function getMcpConfigFingerprint(paths: readonly string[]) { - const entries = await Promise.all( + const metadata = await Promise.all( paths.map(async (filePath) => { try { - const metadata = await stat(filePath); - return `${filePath}:${metadata.mtimeMs}:${metadata.size}`; + const fileMetadata = await stat(filePath); + return { + fingerprint: `${filePath}:${fileMetadata.mtimeMs}:${fileMetadata.size}`, + modifiedAt: fileMetadata.mtimeMs, + }; } catch (error) { if ((error as NodeJS.ErrnoException).code === "ENOENT") { - return `${filePath}:missing`; + return { fingerprint: `${filePath}:missing`, modifiedAt: 0 }; } // The provider will surface a native config error when it loads an // unreadable file. Do not reset a healthy session for a stat failure. - return `${filePath}:unavailable`; + return { fingerprint: `${filePath}:unavailable`, modifiedAt: 0 }; } }), ); - return entries.join("|"); + return { + fingerprint: metadata.map((entry) => entry.fingerprint).join("|"), + latestModifiedAt: Math.max(0, ...metadata.map((entry) => entry.modifiedAt)), + }; } /** @@ -193,6 +199,11 @@ export class McpConfigRefreshTracker { check(args: { scopeKey: string; paths: readonly string[]; + /** + * When the provider process predates this tracker's first check, detect a + * config edit that happened after the process took its native snapshot. + */ + processStartedAt?: number; force?: boolean; minIntervalMs?: number; }): Promise { @@ -221,9 +232,12 @@ export class McpConfigRefreshTracker { } scope.pendingCheck = getMcpConfigFingerprint(args.paths).then( - (fingerprint) => { + ({ fingerprint, latestModifiedAt }) => { const changed = - scope.fingerprint !== null && scope.fingerprint !== fingerprint; + (scope.fingerprint !== null && scope.fingerprint !== fingerprint) || + (scope.fingerprint === null && + typeof args.processStartedAt === "number" && + latestModifiedAt > args.processStartedAt); scope.fingerprint = fingerprint; if (changed) { scope.generation += 1; diff --git a/tests/codex-app-server-mcp-lifecycle.test.ts b/tests/codex-app-server-mcp-lifecycle.test.ts index 600bb373..845ad4c8 100644 --- a/tests/codex-app-server-mcp-lifecycle.test.ts +++ b/tests/codex-app-server-mcp-lifecycle.test.ts @@ -612,4 +612,44 @@ describe("Codex App Server MCP lifecycle mapping", () => { ), ).toBe(false); }); + + test("restarts a prestarted App Server when MCP config changes before the first turn", async () => { + const cwd = await mkdtemp( + path.join(tmpdir(), "stave-codex-prestarted-mcp-refresh-"), + ); + tempDirectories.push(cwd); + const binaryPath = "/tmp/fake-codex-prestarted-mcp-refresh"; + const runtime = await import( + `../electron/providers/codex-app-server-runtime?prestarted-mcp-refresh-test=${Date.now()}-${Math.random()}` + ); + + await runtime.getCodexMcpRuntimeStatus({ + runtimeOptions: { codexBinaryPath: binaryPath }, + }); + expect(fakeChildren).toHaveLength(1); + + await new Promise((resolve) => setTimeout(resolve, 5)); + const dotCodexFolder = path.join(cwd, ".codex"); + await mkdir(dotCodexFolder, { recursive: true }); + await writeFile( + path.join(dotCodexFolder, "config.toml"), + "[mcp_servers.crane]\nurl = 'http://one'\n", + ); + + await runtime.streamCodexWithAppServer({ + providerId: "codex", + taskId: "task-prestarted-mcp-refresh", + prompt: "Inspect the runtime", + cwd, + runtimeOptions: { codexBinaryPath: binaryPath }, + }); + + expect(fakeChildren).toHaveLength(2); + expect(fakeChildren[0]?.killed).toBe(true); + expect( + fakeChildren[1]?.receivedMessages.some( + (message) => message.method === "thread/start", + ), + ).toBe(true); + }); }); diff --git a/tests/mcp-config-refresh.test.ts b/tests/mcp-config-refresh.test.ts index 276adc7b..f5907275 100644 --- a/tests/mcp-config-refresh.test.ts +++ b/tests/mcp-config-refresh.test.ts @@ -166,6 +166,34 @@ describe("MCP config refresh tracking", () => { expect((await tracker.check(args)).changed).toBe(true); }); + test("detects a config newer than a provider process on the first check", async () => { + const directory = await makeTempDirectory(); + const configPath = path.join(directory, "config.toml"); + const processStartedAt = Date.now(); + await new Promise((resolve) => setTimeout(resolve, 5)); + await writeFile(configPath, "[mcp_servers.crane]\nurl = 'http://one'\n"); + + const tracker = new McpConfigRefreshTracker(); + expect( + ( + await tracker.check({ + scopeKey: "codex:prestarted", + paths: [configPath], + processStartedAt, + }) + ).changed, + ).toBe(true); + expect( + ( + await tracker.check({ + scopeKey: "codex:prestarted", + paths: [configPath], + processStartedAt, + }) + ).changed, + ).toBe(false); + }); + test("keeps workspace scopes independent and deduplicates concurrent checks", async () => { const directory = await makeTempDirectory(); const first = path.join(directory, "first.json");