From ffa879ba2c79a45e758dc7fa2a8c9523631dac38 Mon Sep 17 00:00:00 2001 From: Colin Mason Date: Mon, 13 Jul 2026 05:28:43 -0400 Subject: [PATCH] keep process cwd while other cells are live --- .../coding-agent/src/eval/js/worker-core.ts | 27 ++++- .../test/eval/worker-core.test.ts | 102 ++++++++++++++++++ 2 files changed, 125 insertions(+), 4 deletions(-) diff --git a/packages/coding-agent/src/eval/js/worker-core.ts b/packages/coding-agent/src/eval/js/worker-core.ts index 69222f885..86d2d9772 100644 --- a/packages/coding-agent/src/eval/js/worker-core.ts +++ b/packages/coding-agent/src/eval/js/worker-core.ts @@ -223,8 +223,8 @@ export class WorkerCore { } } - #ensureRuntime(snapshot: SessionSnapshot): JsRuntime { - this.#syncProcessCwd(snapshot.cwd); + #ensureRuntime(snapshot: SessionSnapshot, currentRunId?: string): JsRuntime { + this.#syncProcessCwd(snapshot.cwd, currentRunId); if (this.#runtime) { this.#runtime.setCwd(snapshot.cwd); return this.#runtime; @@ -237,8 +237,27 @@ export class WorkerCore { return this.#runtime; } - #syncProcessCwd(cwd: string): void { + #syncProcessCwd(cwd: string, currentRunId?: string): void { if (this.#options.mode !== "isolated" || !this.#options.chdir) return; + try { + if (process.cwd() === cwd) return; + } catch { + // The current cwd was deleted; the chdir below is the recovery. + } + // Process cwd is realm-wide state. Moving it while another cell is mid-run + // would silently redirect that cell's `process.cwd()`, relative fs access, + // and child spawns, so keep it in place; this run still resolves against + // its own virtual cwd, and the next cell to start alone lands the move. + for (const runId of this.#runs.keys()) { + if (runId === currentRunId) continue; + this.#transport.send({ + type: "log", + level: "warn", + msg: "JS eval subprocess kept its process cwd: other cells are mid-run", + meta: { cwd }, + }); + return; + } try { this.#options.chdir(cwd); } catch (error) { @@ -263,7 +282,7 @@ export class WorkerCore { }; let result: RunResult; try { - const runtime = this.#ensureRuntime(snapshot); + const runtime = this.#ensureRuntime(snapshot, runId); runtime.setCwd(snapshot.cwd); const value = await runtime.run(code, filename, hooks, { runId, cwd: snapshot.cwd }); runtime.displayValue(value, hooks); diff --git a/packages/coding-agent/test/eval/worker-core.test.ts b/packages/coding-agent/test/eval/worker-core.test.ts index 546d0b690..85ed6fc8b 100644 --- a/packages/coding-agent/test/eval/worker-core.test.ts +++ b/packages/coding-agent/test/eval/worker-core.test.ts @@ -362,6 +362,108 @@ describe("WorkerCore", () => { } }); + it("keeps the process cwd while another cell is mid-run", async () => { + const dirA = await fs.mkdtemp(path.join(os.tmpdir(), "omp-cwd-a-")); + const dirB = await fs.mkdtemp(path.join(os.tmpdir(), "omp-cwd-b-")); + const chdirs: string[] = []; + const hostListeners = new Set<(message: WorkerOutbound) => void>(); + const workerListeners = new Set<(message: WorkerInbound) => void>(); + const transport: Transport = { + send: message => { + queueMicrotask(() => { + for (const listener of hostListeners) listener(message); + }); + }, + onMessage: handler => { + workerListeners.add(handler); + return () => workerListeners.delete(handler); + }, + close: () => {}, + }; + new WorkerCore(transport, { mode: "isolated", chdir: cwd => chdirs.push(cwd) }); + const harness: WorkerHarness = { + send(message) { + queueMicrotask(() => { + for (const listener of workerListeners) listener(message); + }); + }, + onMessage(handler) { + hostListeners.add(handler); + return () => hostListeners.delete(handler); + }, + }; + + const gate = Promise.withResolvers(); + const entered = Promise.withResolvers(); + (globalThis as { __omp_worker_cwd_gate?: { entered(): void; wait: Promise } }).__omp_worker_cwd_gate = { + entered: () => entered.resolve(), + wait: gate.promise, + }; + try { + await initializeWorker(harness, { cwd: dirA, sessionId: "cwd-race", localRoots: {} }); + expect(chdirs).toEqual([dirA]); + + const holdResult = waitForMessage( + harness, + message => message.type === "result" && message.runId === "cwd-hold", + ); + harness.send({ + type: "run", + runId: "cwd-hold", + code: "globalThis.__omp_worker_cwd_gate.entered(); await globalThis.__omp_worker_cwd_gate.wait;", + filename: "[cwd-race-hold].js", + snapshot: { cwd: dirA, sessionId: "cwd-race", localRoots: {} }, + }); + await entered.promise; + + // A second cell with a different cwd while the first is suspended must + // not move the realm-wide process cwd out from under the live cell. + const skipLog = waitForMessage( + harness, + message => message.type === "log" && message.msg.includes("kept its process cwd"), + ); + const overlapResult = waitForMessage( + harness, + message => message.type === "result" && message.runId === "cwd-overlap", + ); + harness.send({ + type: "run", + runId: "cwd-overlap", + code: "1 + 1;", + filename: "[cwd-race-overlap].js", + snapshot: { cwd: dirB, sessionId: "cwd-race", localRoots: {} }, + }); + expect(await overlapResult).toMatchObject({ type: "result", runId: "cwd-overlap", ok: true }); + expect(chdirs).not.toContain(dirB); + await skipLog; + + gate.resolve(); + expect(await holdResult).toMatchObject({ type: "result", runId: "cwd-hold", ok: true }); + + // With the realm quiet again, the next cell lands the deferred move. + const soloResult = waitForMessage( + harness, + message => message.type === "result" && message.runId === "cwd-solo", + ); + harness.send({ + type: "run", + runId: "cwd-solo", + code: "2 + 2;", + filename: "[cwd-race-solo].js", + snapshot: { cwd: dirB, sessionId: "cwd-race", localRoots: {} }, + }); + expect(await soloResult).toMatchObject({ type: "result", runId: "cwd-solo", ok: true }); + expect(chdirs.at(-1)).toBe(dirB); + } finally { + gate.resolve(); + delete (globalThis as { __omp_worker_cwd_gate?: { entered(): void; wait: Promise } }) + .__omp_worker_cwd_gate; + harness.send({ type: "close" }); + await fs.rm(dirA, { recursive: true, force: true }); + await fs.rm(dirB, { recursive: true, force: true }); + } + }); + it("survives concurrent same-realm setCwd in a child process with postmortem loaded", async () => { // Process-level oracle: the production crash was postmortem killing the process // after an unhandled rejection from concurrent inline setCwd. This must stay green