From 65a809a0da0555aa8eeafbc2ec2559517471be02 Mon Sep 17 00:00:00 2001 From: Chris Ozsvath <142112319+MegaPhoenix92@users.noreply.github.com> Date: Thu, 8 Oct 2026 19:18:07 -0400 Subject: [PATCH] fix(process): avoid cold ownership helper deadline exhaustion (#1139) * fix(process): avoid cold ownership helper deadline exhaustion Fixes #1137. * test(process): fix cold ownership test deadlines and cleanup * fix(process): isolate Swift cache overrides and retain diagnostics * test(electron): assert blank path rejection without trim assumptions --------- Co-authored-by: Chris Co-authored-by: morluto <76467478+morluto@users.noreply.github.com> --- src/javascript/sourceMaps/SourceMapCommand.ts | 21 +++++ src/process/DarwinProcessRunTokenReader.ts | 17 ++-- .../applications/electronBlankPathCli.test.ts | 2 +- tests/acceptance/webSourceLocationCli.test.ts | 13 ++- .../javascript/sourceMapDecoder.test.ts | 28 +++++- .../darwinProcessRunTokenReader.test.ts | 91 ++++++++++++++++++- 6 files changed, 156 insertions(+), 16 deletions(-) diff --git a/src/javascript/sourceMaps/SourceMapCommand.ts b/src/javascript/sourceMaps/SourceMapCommand.ts index eec1f8844..2caebcdb2 100644 --- a/src/javascript/sourceMaps/SourceMapCommand.ts +++ b/src/javascript/sourceMaps/SourceMapCommand.ts @@ -26,6 +26,7 @@ import { type OwnedProviderProcessSpawnOptions, type SpawnedOwnedProviderProcess, } from "../../process/ProviderProcess.js"; +import { prepareProcessOwnershipInspection } from "../../process/ProcessOwnershipObservation.js"; const OPERATION = "trace_web_source_location"; const PROVIDER = "source-map-decoder"; @@ -43,6 +44,8 @@ export interface SourceMapDecoderDependencies { readonly launcher?: SourceMapCodecLauncher; readonly createRuntime?: () => Promise; readonly environment?: Readonly; + /** Prepare host ownership inspection outside the codec execution deadline. */ + readonly prepareOwnership?: (signal?: AbortSignal) => Promise; } /** Run a trusted codec command and release its process/root before returning output. */ @@ -52,6 +55,24 @@ export const runSourceMapCommand = async ( options: ExecutionOptions | undefined, dependencies: SourceMapDecoderDependencies, ): Promise> => { + try { + await (dependencies.prepareOwnership ?? prepareProcessOwnershipInspection)( + options?.signal, + ); + } catch (cause: unknown) { + return err( + options?.signal?.aborted === true + ? new AnalysisCancelledError(OPERATION) + : new ProviderAdapterError(PROVIDER, OPERATION, { + cause, + diagnostics: { + map_path: path, + error_message: message(cause), + phase: "process_ownership_preparation", + }, + }), + ); + } const deadline = new ProviderStartupDeadline( WEB_SOURCE_MAP_LIMITS.decodeTimeoutMs, options?.signal, diff --git a/src/process/DarwinProcessRunTokenReader.ts b/src/process/DarwinProcessRunTokenReader.ts index 176688063..5c176dc07 100644 --- a/src/process/DarwinProcessRunTokenReader.ts +++ b/src/process/DarwinProcessRunTokenReader.ts @@ -149,21 +149,18 @@ export const createDarwinProcessRunTokenReader = ( exitCleanupInstalled = true; if (signal.aborted || closed) throw abortReason(signal); const output = join(operationRoot, "reader"); + const compilerEnvironment = { ...process.env }; + // Use Swift's reusable default cache for REA's internal helper compilation. + delete compilerEnvironment.CLANG_MODULE_CACHE_PATH; try { await execFileOutput( options.xcrun ?? "/usr/bin/xcrun", - [ - "swiftc", - "-module-cache-path", - join(operationRoot, "modules"), - ...sourceFiles, - "-o", - output, - ], + ["swiftc", ...sourceFiles, "-o", output], { timeout: 60_000, maxBuffer: 1024 * 1024, signal, + env: compilerEnvironment, }, ); } catch (cause: unknown) { @@ -173,7 +170,9 @@ export const createDarwinProcessRunTokenReader = ( ? String(cause.code) : "unknown"; throw new DarwinProcessOwnershipInspectionError( - `macOS process ownership inspection requires the Apple Swift compiler via xcrun (compiler result: ${code})`, + code === "ENOENT" + ? `macOS process ownership inspection requires the Apple Swift compiler via xcrun (compiler result: ${code})` + : `macOS process ownership helper compilation failed via xcrun (compiler result: ${code}): ${cause instanceof Error ? cause.message : String(cause)}`, { cause }, ); } diff --git a/tests/acceptance/applications/electronBlankPathCli.test.ts b/tests/acceptance/applications/electronBlankPathCli.test.ts index f060aed34..a03968ca1 100644 --- a/tests/acceptance/applications/electronBlankPathCli.test.ts +++ b/tests/acceptance/applications/electronBlankPathCli.test.ts @@ -24,7 +24,7 @@ cliTest.for(["", " \t "])( details: { operation: "capture_electron_scenario", issues: expect.arrayContaining([ - { path: ["application_root"], reason: "out_of_range", minimum: 1 }, + expect.objectContaining({ path: ["application_root"] }), ]), }, }); diff --git a/tests/acceptance/webSourceLocationCli.test.ts b/tests/acceptance/webSourceLocationCli.test.ts index 2e7b3a2fe..92baa900c 100644 --- a/tests/acceptance/webSourceLocationCli.test.ts +++ b/tests/acceptance/webSourceLocationCli.test.ts @@ -36,8 +36,9 @@ cliTest( environment: { REA_LOG_LEVEL: "silent", REA_BROWSER_EXECUTABLE: "relative-unconfigured-browser", + CLANG_MODULE_CACHE_PATH: map, }, - timeoutMs: 15000, + timeoutMs: 45000, }); expect(response.exitCode).toBe(0); expect(response.json).toMatchObject({ @@ -56,7 +57,11 @@ cliTest( fileURLToPath(new URL("../../scripts/rea.mjs", import.meta.url)), "mcp", ], - env: { ...getDefaultEnvironment(), REA_LOG_LEVEL: "silent" }, + env: { + ...getDefaultEnvironment(), + REA_LOG_LEVEL: "silent", + CLANG_MODULE_CACHE_PATH: map, + }, stderr: "pipe", }); const client = new Client({ @@ -96,7 +101,7 @@ cliTest( "--json", ], environment: { REA_LOG_LEVEL: "silent" }, - timeoutMs: 15000, + timeoutMs: 45000, }); expect(invalid.exitCode).toBe(1); expect(invalid.json).toMatchObject({ @@ -104,4 +109,6 @@ cliTest( details: { issues: [{ path: ["generated_position"] }] }, }); }, + // Allow cold CLI and MCP ownership preparation across the full parity sequence. + 180_000, ); diff --git a/tests/boundary/javascript/sourceMapDecoder.test.ts b/tests/boundary/javascript/sourceMapDecoder.test.ts index 397485479..6b3938b3c 100644 --- a/tests/boundary/javascript/sourceMapDecoder.test.ts +++ b/tests/boundary/javascript/sourceMapDecoder.test.ts @@ -1,7 +1,7 @@ import { fileURLToPath } from "node:url"; import type { SourceMapDecoderDependencies } from "../../../src/javascript/sourceMaps/SourceMapDecoder.js"; import { access } from "node:fs/promises"; -import { expect, it } from "vitest"; +import { expect, it, vi } from "vitest"; import { SourceMapDecoder } from "../../../src/javascript/sourceMaps/SourceMapDecoder.js"; import { PrivateRuntimeRoot } from "../../../src/process/PrivateRuntimeRoot.js"; import { spawnOwnedProviderProcess } from "../../../src/process/ProviderProcess.js"; @@ -41,6 +41,32 @@ const input = { position: webSourceLocationArgs.generated_position, }; +it("starts the codec deadline after cold ownership preparation", async () => { + vi.useFakeTimers(); + try { + let prepared = false; + const responsePromise = realDecoder({ + prepareOwnership: async (signal) => { + await vi.advanceTimersByTimeAsync(20_001); + expect(signal?.aborted).not.toBe(true); + prepared = true; + }, + launcher: (options) => { + expect(prepared).toBe(true); + expect(options.signal?.aborted).not.toBe(true); + return Promise.reject(new Error("observed launch after preparation")); + }, + }).trace(input); + const response = await responsePromise; + if (response.ok) throw new Error("expected launch failure"); + expect(response.error).toMatchObject({ + diagnostics: { error_message: "observed launch after preparation" }, + }); + } finally { + vi.useRealTimers(); + } +}); + it("runs the real pinned codec with an independent heap and removes its private request directory", async () => { let path: string | undefined; const environment = { diff --git a/tests/boundary/process/darwinProcessRunTokenReader.test.ts b/tests/boundary/process/darwinProcessRunTokenReader.test.ts index 8e2977f50..e32dcc813 100644 --- a/tests/boundary/process/darwinProcessRunTokenReader.test.ts +++ b/tests/boundary/process/darwinProcessRunTokenReader.test.ts @@ -2,6 +2,7 @@ import { execFile, spawn, type ChildProcess } from "node:child_process"; import { access, chmod, + mkdir, mkdtemp, readFile, rm, @@ -14,6 +15,7 @@ import { fileURLToPath } from "node:url"; import { promisify } from "node:util"; import { _electron as electron } from "playwright-core"; import { expect, it, vi } from "vitest"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; import { createSystemProcessOwnershipHost, @@ -41,10 +43,10 @@ const state = fs.existsSync(statePath) ? JSON.parse(fs.readFileSync(statePath, "utf8")) : { calls: 0 }; state.calls += 1; -const moduleIndex = process.argv.indexOf("-module-cache-path"); -state.root = path.dirname(process.argv[moduleIndex + 1]); const outputIndex = process.argv.indexOf("-o"); state.output = process.argv[outputIndex + 1]; +state.root = path.dirname(state.output); +state.arguments = process.argv.slice(2); state.releasePath = releasePath; fs.writeFileSync(statePath, JSON.stringify(state)); if (state.calls === 1) { @@ -106,6 +108,10 @@ const waitForCompileState = async (statePath: string) => { root: parsed.root, output: parsed.output, releasePath: parsed.releasePath, + arguments: + "arguments" in parsed && Array.isArray(parsed.arguments) + ? parsed.arguments + : [], }; } catch { await new Promise((resolve) => setTimeout(resolve, 10)); @@ -161,6 +167,87 @@ it("reports an actionable missing Swift compiler without installing it", async ( } }); +it.skipIf(process.platform === "win32")( + "preserves compiler diagnostics when Swift preparation fails", + async () => { + const directory = await createTestTempDirectory( + "rea-process-token-compiler-failure-test-", + ); + const executable = join(directory, "failing-xcrun"); + await writeFile( + executable, + '#!/usr/bin/env node\nprocess.stderr.write("module cache is not writable\\n"); process.exitCode = 1;\n', + ); + await chmod(executable, 0o755); + const reader = createDarwinProcessRunTokenReader({ xcrun: executable }); + try { + await expect(reader.prepare()).rejects.toThrow( + /compilation failed.*module cache is not writable/su, + ); + } finally { + await reader.close(); + } + }, +); + +it + .skipIf(!onDarwin || process.getuid?.() === 0) + .each(["file", "read-only directory"])( + "ignores unusable inherited Swift module cache: %s", + async (kind) => { + const directory = await createTestTempDirectory( + "rea-process-token-inherited-cache-test-", + ); + const cacheParent = join(directory, "cache-parent"); + if (kind === "file") await writeFile(cacheParent, "not a directory"); + else { + await mkdir(cacheParent); + await chmod(cacheParent, 0o500); + } + const cachePath = join(cacheParent, "modules"); + vi.stubEnv("CLANG_MODULE_CACHE_PATH", cachePath); + const reader = createDarwinProcessRunTokenReader(); + try { + const executable = await reader.prepare(); + const { stdout } = await execFileOutput( + executable, + ["--identities", String(process.pid)], + { timeout: 15_000 }, + ); + expect(JSON.parse(stdout)).toMatchObject({ + results: [{ pid: process.pid, state: "readable" }], + }); + expect(process.env.CLANG_MODULE_CACHE_PATH).toBe(cachePath); + } finally { + vi.unstubAllEnvs(); + if (kind === "read-only directory") await chmod(cacheParent, 0o700); + await reader.close(); + } + }, +); + +it.skipIf(process.platform === "win32")( + "uses the compiler-managed module cache instead of rebuilding system modules per REA process", + async () => { + const directory = await createTestTempDirectory( + "rea-process-token-module-cache-test-", + ); + const fakeCompiler = await writeBlockingCompiler(directory); + const reader = createDarwinProcessRunTokenReader({ + xcrun: fakeCompiler.executable, + }); + const preparation = reader.prepare(); + try { + const state = await waitForCompileState(fakeCompiler.statePath); + expect(state.arguments).not.toContain("-module-cache-path"); + await writeFile(fakeCompiler.releasePath, "release"); + await preparation; + } finally { + await Promise.all([reader.close(), Promise.allSettled([preparation])]); + } + }, +); + it.skipIf(process.platform === "win32")( "rethrows compile cancellation, removes its temporary root, and permits retry", async () => {