From ca75643dbce5b7ece34633075cbdc9261e06ddca Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Tue, 27 Oct 2026 03:23:35 +0800 Subject: [PATCH] fix(setup): preserve registration choices and own prewarm --- docs/client-configuration.md | 6 +- docs/installation.md | 3 +- src/application/ClientRegistration.ts | 5 +- src/application/ClientRegistrationStatus.ts | 3 +- src/application/Setup.recovery.test.ts | 45 ++++ src/application/Setup.ts | 10 + src/application/SetupHost.ts | 1 + .../SetupRegistrationPrewarm.test.ts | 133 ++++++++++- src/application/SetupRegistrationPrewarm.ts | 212 +++++++++++------- .../filesystem/clientLifecycle.test.ts | 4 +- .../clientRegistrationRefresh.test.ts | 36 +++ .../clientRegistrationStatus.test.ts | 6 +- 12 files changed, 373 insertions(+), 91 deletions(-) create mode 100644 tests/boundary/filesystem/clientRegistrationRefresh.test.ts diff --git a/docs/client-configuration.md b/docs/client-configuration.md index d50b5404fc..f4922eba38 100644 --- a/docs/client-configuration.md +++ b/docs/client-configuration.md @@ -659,8 +659,10 @@ become active again. Malformed applicable files block setup before changes. Grok Build loads `[mcp_servers.rea]` from `$GROK_HOME/config.toml`, or from `~/.grok/config.toml` when `GROK_HOME` is unset or empty. Setup edits that server table, `[mcp_servers.rea.env]`, and a root `disabled_mcp_servers` entry that -names `rea`. It sets `startup_timeout_sec = 30` and leaves every other name -in that list. The shared skill installed under `~/.agents/skills` is already +names `rea`. It defaults `startup_timeout_sec` to 30 seconds, preserves an +existing positive timeout, and leaves every other name in that list. +[Grok supports longer startup timeouts](https://docs.x.ai/build/features/mcp-servers) +for cold package-runner launches. The shared skill installed under `~/.agents/skills` is already on Grok Build's skill path. Grok Bot (`grok_bot`) is detected from `~/.grokbot`, or from `SAND_DATA_ROOT` diff --git a/docs/installation.md b/docs/installation.md index ad0655c2de..6c5cca18bf 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -204,7 +204,8 @@ identity remains available at `identity.catalog`. Setup pins package-runner MCP registrations to the exact installed REA version, installs the matching skill and on-demand references in the same plan, and adds -`startup_timeout_sec = 30` for Codex and Grok Build. Codex also receives a +`startup_timeout_sec = 30` for Codex and Grok Build when no positive timeout +is already configured. Codex also receives a `tool_timeout_sec` derived from known provider startup plus operation headroom. ## Limit REA to one project or agent profile diff --git a/src/application/ClientRegistration.ts b/src/application/ClientRegistration.ts index 9ae1b0b12a..02d78f6f1f 100644 --- a/src/application/ClientRegistration.ts +++ b/src/application/ClientRegistration.ts @@ -246,9 +246,8 @@ export const refreshClientRegistration = ( registration.enable = false; if (client.name === "codex" || client.name === "grok_build") registration.startup_timeout_sec = - (client.name === "codex" - ? positiveTimeout(existing.startup_timeout_sec) - : undefined) ?? MCP_STARTUP_POLICY.codexStartupTimeoutSeconds; + positiveTimeout(existing.startup_timeout_sec) ?? + MCP_STARTUP_POLICY.codexStartupTimeoutSeconds; if (client.name === "codex") registration.tool_timeout_sec = positiveTimeout(existing.tool_timeout_sec) ?? diff --git a/src/application/ClientRegistrationStatus.ts b/src/application/ClientRegistrationStatus.ts index d233a0f7d7..9bed5d4e3f 100644 --- a/src/application/ClientRegistrationStatus.ts +++ b/src/application/ClientRegistrationStatus.ts @@ -441,8 +441,7 @@ const registrationAligned = ({ if (identity === "foreign") return "stale"; if ( client.name === "grok_build" && - registration.startup_timeout_sec !== - MCP_STARTUP_POLICY.codexStartupTimeoutSeconds + registration.startup_timeout_sec === undefined ) return "stale"; if ( diff --git a/src/application/Setup.recovery.test.ts b/src/application/Setup.recovery.test.ts index 2891bb7663..b621f11356 100644 --- a/src/application/Setup.recovery.test.ts +++ b/src/application/Setup.recovery.test.ts @@ -774,3 +774,48 @@ it.each(["return", "throw"] as const)( expect(failure.hasCleanupOwner).toBe(false); }, ); + +it("stops setup after interrupted prewarm and retains cleanup through host close retry", async () => { + const host = new FakeSetupHost(); + host.hopper = "/Applications/Hopper"; + host.clients = [{ name: "codex", configPath: "/fixture/codex.json" }]; + let doctorCalls = 0; + const doctor = host.doctor.bind(host); + host.doctor = async (...args) => { + doctorCalls++; + return doctor(...args); + }; + let callsAtInterrupt = -1; + let released = false; + const cleanup = new SetupCleanupOwner([ + { + close: async () => { + if (!released) throw new Error("controlled runner cleanup refusal"); + }, + }, + ]); + const selected = Object.assign(host, { + prewarmRegistrationRunner: async () => { + callsAtInterrupt = doctorCalls; + return { + status: "failed" as const, + reason: "interrupted prewarm", + interruptedSignal: "SIGINT", + }; + }, + close: () => cleanup.close(), + }); + const result = await runSetup( + { ...options(true), clientIds: ["codex"], installSkill: false }, + selected, + ); + expect(callsAtInterrupt).toBeGreaterThanOrEqual(0); + expect(doctorCalls).toBe(callsAtInterrupt); + expect(result.status).toBe("needs_human"); + expect(result.appliedActions).toContain("configured_codex"); + expect(result.prewarm).toMatchObject({ interruptedSignal: "SIGINT" }); + expect(result.cleanup?.reason).toContain("controlled runner cleanup refusal"); + released = true; + await cleanup.close(); + expect(result.cleanup).toBeUndefined(); +}); diff --git a/src/application/Setup.ts b/src/application/Setup.ts index c1d1aa8afe..8f173c32fd 100644 --- a/src/application/Setup.ts +++ b/src/application/Setup.ts @@ -401,6 +401,16 @@ const runSetupWorkflow = async ( detail: "Preparing the pinned package entry", }); prewarm = await host.prewarmRegistrationRunner(); + if (prewarm.status === "failed" && prewarm.interruptedSignal !== undefined) + return { + status: "cancelled", + plannedActions, + appliedActions, + clients, + clientStates, + doctor: summarizeDoctor(discovery.initialDoctor), + prewarm, + }; if (prewarm.status === "failed") emitProgress(options, { actionId: "prewarm_registration_runner", diff --git a/src/application/SetupHost.ts b/src/application/SetupHost.ts index 53da0672c7..f76396e916 100644 --- a/src/application/SetupHost.ts +++ b/src/application/SetupHost.ts @@ -196,6 +196,7 @@ export const systemSetupHost = ( materializeRegistrationRunnerCache( registration.registrationCommand, environment, + resources, ); const cleanup = new SetupCleanupOwner([ macCleanup, diff --git a/src/application/SetupRegistrationPrewarm.test.ts b/src/application/SetupRegistrationPrewarm.test.ts index 0362089381..af189b5249 100644 --- a/src/application/SetupRegistrationPrewarm.test.ts +++ b/src/application/SetupRegistrationPrewarm.test.ts @@ -1,4 +1,7 @@ -import { describe, expect, it } from "vitest"; +import { describe, expect, it, onTestFinished } from "vitest"; +import { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; +import { spawnOwnedProviderProcess } from "../process/ProviderProcess.js"; +import { cleanupOwnedProcessGroup } from "../process/ProcessOwnership.js"; import { PRODUCT_IDENTITY } from "../identity.js"; import { @@ -9,6 +12,12 @@ import { type RegistrationRunnerLauncher, } from "./SetupRegistrationPrewarm.js"; +const scope = () => { + const resources = new ArtifactResourceScope(); + onTestFinished(() => resources.close()); + return resources; +}; + const specifier = PRODUCT_IDENTITY.registrationPackageSpecifier; const launch = ( @@ -70,6 +79,7 @@ describe("materializeRegistrationRunnerCache", () => { const result = await materializeRegistrationRunnerCache( ["/opt/rea/rea", "mcp"], {}, + scope(), launcher, ); expect(result).toEqual({ status: "skipped", reason: "direct_launcher" }); @@ -92,6 +102,7 @@ describe("materializeRegistrationRunnerCache", () => { const result = await materializeRegistrationRunnerCache( ["npx", "-y", specifier, "mcp"], { REA_PREWARM_PROBE: "1" }, + scope(), launcher, ); expect(result).toEqual({ status: "warmed" }); @@ -102,6 +113,7 @@ describe("materializeRegistrationRunnerCache", () => { const result = await materializeRegistrationRunnerCache( ["npx", "-y", specifier, "mcp"], {}, + scope(), async () => launch({ exitCode: 1, stderr: "spawn EPERM\n" }), ); expect(result.status).toBe("failed"); @@ -116,6 +128,7 @@ describe("materializeRegistrationRunnerCache", () => { const result = await materializeRegistrationRunnerCache( ["npx", "-y", specifier, "mcp"], {}, + scope(), async () => launch({ exitCode: null, timedOut: true }), ); expect(result.status).toBe("failed"); @@ -127,6 +140,7 @@ describe("materializeRegistrationRunnerCache", () => { const result = await materializeRegistrationRunnerCache( ["npx", "-y", specifier, "mcp"], {}, + scope(), async () => launch({ exitCode: null, spawnError: "spawn npx ENOENT" }), ); expect(result.status).toBe("failed"); @@ -138,6 +152,7 @@ describe("materializeRegistrationRunnerCache", () => { const result = await materializeRegistrationRunnerCache( ["npx", "-y", specifier, "mcp"], {}, + scope(), async () => launch({ exitCode: null, interruptedSignal: "SIGINT" }), ); expect(result.status).toBe("failed"); @@ -170,6 +185,7 @@ describe("launchRegistrationRunnerCommand", () => { cwd: process.cwd(), timeoutMs: 5000, platform: process.platform, + resources: scope(), }, ); expect(result.exitCode).toBe(1); @@ -180,6 +196,7 @@ describe("launchRegistrationRunnerCommand", () => { const prewarm = await materializeRegistrationRunnerCache( ["npx", "-y", specifier, "mcp"], {}, + scope(), async () => result, ); expect(prewarm.status).toBe("failed"); @@ -198,6 +215,7 @@ describe("launchRegistrationRunnerCommand", () => { cwd: process.cwd(), timeoutMs: 30_000, platform: process.platform, + resources: scope(), }, ); expect(result.exitCode).toBe(0); @@ -215,9 +233,122 @@ describe("launchRegistrationRunnerCommand", () => { cwd: process.cwd(), timeoutMs: 300, platform: process.platform, + resources: scope(), }, ); expect(result.timedOut).toBe(true); expect(Date.now() - startedAt).toBeLessThan(10_000); }); }); + +it.runIf(process.platform !== "win32")( + "settles the prewarm deadline after a detached descendant inherits the exited runner's pipes", + async () => { + const result = await launchRegistrationRunnerCommand( + process.execPath, + [ + "-e", + ` + const { spawn } = require('node:child_process'); + const child = spawn(process.execPath, ['-e', 'setTimeout(() => process.stdout.write("natural-descendant-exit"), 1500)'], { + detached: true, stdio: ['ignore', 'inherit', 'inherit'] + }); + process.stdout.write('descendant:' + child.pid + '\\n'); + child.unref(); + `, + ], + { + env: process.env, + cwd: process.cwd(), + timeoutMs: 100, + platform: process.platform, + resources: scope(), + }, + ); + expect(result.timedOut).toBe(true); + expect(result.stdout).toContain("descendant:"); + expect(result.cleanupFailure).toBeUndefined(); + expect(result.stdout).not.toContain("natural-descendant-exit"); + }, +); + +it.each([false, true])( + "retains the exact runner cleanup for scope close and retry (interrupt=%s)", + async (interrupt) => { + const resources = new ArtifactResourceScope(); + let allowCleanup = false; + let attempts = 0; + onTestFinished(async () => { + allowCleanup = true; + await resources.close(); + }); + const result = await launchRegistrationRunnerCommand( + process.execPath, + ["--version"], + { + env: process.env, + cwd: process.cwd(), + timeoutMs: 5000, + platform: process.platform, + resources, + launcher: async (options) => { + const launched = await spawnOwnedProviderProcess(options); + return { + ...launched, + cleanup: async () => { + attempts++; + if (interrupt && attempts === 1) process.emit("SIGINT"); + return allowCleanup + ? cleanupOwnedProcessGroup(launched.ownership) + : { + cleaned: false as const, + reason: "controlled runner cleanup refusal", + }; + }, + }; + }, + }, + ); + expect(result.exitCode).toBe(0); + expect(result.cleanupFailure).toContain( + "controlled runner cleanup refusal", + ); + expect(result.interruptedSignal).toBe(interrupt ? "SIGINT" : undefined); + expect(resources.hasPending).toBe(true); + const prewarm = await materializeRegistrationRunnerCache( + ["npx", "-y", specifier, "mcp"], + {}, + resources, + async () => result, + ); + expect(prewarm).toMatchObject({ status: "failed" }); + await expect(resources.close()).rejects.toThrow(); + expect(attempts).toBe(2); + allowCleanup = true; + await resources.close(); + expect(attempts).toBe(3); + expect(resources.hasPending).toBe(false); + }, +); + +it("captures the selected environment at admission and accepts reconfiguration on the next prewarm", async () => { + const env = { ...process.env, REA_PREWARM_SELECTED_VALUE: "first" }; + const resources = scope(); + const invoke = () => + launchRegistrationRunnerCommand( + process.execPath, + ["-e", "process.stdout.write(process.env.REA_PREWARM_SELECTED_VALUE)"], + { + env, + cwd: process.cwd(), + timeoutMs: 5000, + platform: process.platform, + resources, + }, + ); + const first = invoke(); + env.REA_PREWARM_SELECTED_VALUE = "second"; + expect((await first).stdout).toBe("first"); + expect((await invoke()).stdout).toBe("second"); + expect(resources.hasPending).toBe(false); +}); diff --git a/src/application/SetupRegistrationPrewarm.ts b/src/application/SetupRegistrationPrewarm.ts index 7457ff87c3..d132521e2e 100644 --- a/src/application/SetupRegistrationPrewarm.ts +++ b/src/application/SetupRegistrationPrewarm.ts @@ -1,8 +1,20 @@ -import { spawn, type ChildProcess } from "node:child_process"; +import { snapshotEnvironment } from "../process/snapshotEnvironment.js"; +import { randomUUID } from "node:crypto"; +import { + ArtifactResourceScope, + type ArtifactResourceOwner, +} from "../artifacts/ArtifactResourceScope.js"; +import { AnalysisCleanupLease } from "../artifacts/AnalysisCleanupLease.js"; +import { + ProviderProcessSupervisor, + spawnOwnedProviderProcess, +} from "../process/ProviderProcess.js"; +import { cleanupOwnedProcessGroup } from "../process/ProcessOwnership.js"; +import { waitForAbortableDelay } from "../process/ProviderDeadline.js"; +import { ownedCommandResources } from "../process/OwnedCommand.js"; import { tmpdir } from "node:os"; import { BoundedOutputExcerpt } from "../process/BoundedOutputExcerpt.js"; -import { cleanupWindowsProcessTree } from "../process/ProcessOwnership.js"; import { inspectClientRegistrationCommand } from "./ClientRegistrationIdentity.js"; /** @@ -13,7 +25,11 @@ import { inspectClientRegistrationCommand } from "./ClientRegistrationIdentity.j export type SetupRegistrationPrewarm = | { readonly status: "skipped"; readonly reason: "direct_launcher" } | { readonly status: "warmed" } - | { readonly status: "failed"; readonly reason: string }; + | { + readonly status: "failed"; + readonly reason: string; + readonly interruptedSignal?: string; + }; /** One cold package install at setup time; client startup deadlines stay untouched. */ const PREWARM_TIMEOUT_MS = 180_000; @@ -28,6 +44,7 @@ export interface RegistrationRunnerLaunch { readonly stderr: string; readonly timedOut: boolean; readonly spawnError?: string; + readonly cleanupFailure?: string; /** Terminal interrupt that was forwarded to the runner tree, if any. */ readonly interruptedSignal?: string; } @@ -43,6 +60,8 @@ export type RegistrationRunnerLauncher = ( readonly cwd: string; readonly timeoutMs: number; readonly platform: NodeJS.Platform; + readonly resources: ArtifactResourceScope; + readonly launcher?: typeof spawnOwnedProviderProcess; }, ) => Promise; @@ -68,6 +87,7 @@ export const registrationPrewarmCommand = ( export const materializeRegistrationRunnerCache = async ( command: readonly string[], environment: Readonly, + resources: ArtifactResourceScope, launchCommand: RegistrationRunnerLauncher = launchRegistrationRunnerCommand, ): Promise => { const prewarm = registrationPrewarmCommand(command); @@ -78,117 +98,155 @@ export const materializeRegistrationRunnerCache = async ( cwd: tmpdir(), timeoutMs: PREWARM_TIMEOUT_MS, platform: process.platform, + resources, }); return launch.timedOut || launch.spawnError !== undefined || + launch.cleanupFailure !== undefined || launch.interruptedSignal !== undefined || launch.exitCode !== 0 - ? { status: "failed", reason: prewarmFailureReason(launch) } + ? { + status: "failed", + reason: prewarmFailureReason(launch), + ...(launch.interruptedSignal === undefined + ? {} + : { interruptedSignal: launch.interruptedSignal }), + } : { status: "warmed" }; }; -/** - * Stop the whole runner tree: killing only the direct child leaves npm, which - * inherited the output pipes, holding them open past the deadline. - */ -const stopRunnerTree = ( - child: ChildProcess, - platform: NodeJS.Platform, -): Promise => { - const pid = child.pid; - if (pid === undefined) return Promise.resolve(); - if (platform === "win32") - return cleanupWindowsProcessTree(pid).then(() => undefined); - return new Promise((resolve) => { - try { - process.kill(-pid, "SIGKILL"); - } catch { - // The process group may already be gone. - } - resolve(); - }); -}; - +/** Own the launcher and token-marked descendants until cleanup is verified. */ export const launchRegistrationRunnerCommand: RegistrationRunnerLauncher = ( command, arguments_, options, -) => - new Promise((resolve) => { - const child = spawn(command, [...arguments_], { - env: options.env, - cwd: options.cwd, - stdio: ["ignore", "pipe", "pipe"], - // POSIX runs the runner in its own group so one signal stops the tree; - // an interrupt must therefore be forwarded below, because the detached - // group no longer receives the terminal's signal. - ...(options.platform === "win32" ? {} : { detached: true }), - }); +) => { + const environment = snapshotEnvironment(options.env, options.platform); + return options.resources.run(async () => { const stdout = new BoundedOutputExcerpt(PREWARM_CAPTURE_LIMIT); const stderr = new BoundedOutputExcerpt(PREWARM_CAPTURE_LIMIT); - child.stdout?.on("data", (chunk: Buffer) => stdout.append(chunk)); - child.stderr?.on("data", (chunk: Buffer) => stderr.append(chunk)); + const controller = new AbortController(); let timedOut = false; let spawnError: string | undefined; - let interruptedSignal: string | undefined; + let cleanupFailure: string | undefined; + let interruptedSignal: NodeJS.Signals | undefined; + let supervisor: ProviderProcessSupervisor | undefined; + let resource: ArtifactResourceOwner | undefined; const timer = setTimeout(() => { timedOut = true; - void stopRunnerTree(child, options.platform).finally(() => { - child.kill("SIGKILL"); - }); + controller.abort(); }, options.timeoutMs); - const detachSignalForwarding = () => { - for (const [signal, handler] of signalHandlers) - process.removeListener(signal, handler); - }; const signalHandlers = new Map void>( FORWARD_SIGNALS.map((signal) => [ signal, () => { if (interruptedSignal !== undefined) return; interruptedSignal = signal; - clearTimeout(timer); - void stopRunnerTree(child, options.platform).finally(() => { - child.kill("SIGKILL"); - detachSignalForwarding(); - // Restore default termination for the interrupt that arrived. - process.kill(process.pid, signal); - }); + controller.abort(); }, ]), ); for (const [signal, handler] of signalHandlers) process.on(signal, handler); - child.on("error", (error: NodeJS.ErrnoException) => { - spawnError = error.message; - }); - child.on("close", (code, signal) => { - clearTimeout(timer); - detachSignalForwarding(); - resolve({ - exitCode: code ?? signal, - signal: signal ?? null, - stdout: stdout.text, - stderr: stderr.text, - timedOut, - ...(spawnError === undefined ? {} : { spawnError }), - ...(interruptedSignal === undefined ? {} : { interruptedSignal }), + try { + const launched = await (options.launcher ?? spawnOwnedProviderProcess)({ + command, + arguments: arguments_, + cwd: options.cwd, + hostEnvironment: environment, + platform: options.platform, + runId: `rea-registration-prewarm-${randomUUID()}`, + expectedCommand: null, + sweepTokenOwnedProcesses: true, + signal: controller.signal, }); - }); + supervisor = new ProviderProcessSupervisor( + { + ...launched, + ownsProcessLifetime: true, + cleanup: + launched.cleanup ?? + (() => cleanupOwnedProcessGroup(launched.ownership)), + }, + { + captureStdout: false, + maxDiagnosticBytes: 0, + onDiagnostic: (event) => { + if (event.type === "error") spawnError = event.message; + }, + }, + ); + const owner = supervisor; + const lease = new AnalysisCleanupLease(async () => { + const stopped = await owner.stop(); + if (stopped.status === "incomplete" || stopped.status === "not-owned") + throw new Error( + stopped.status === "incomplete" + ? stopped.reason + : "Registration runner ownership was not confirmed.", + ); + }, ownedCommandResources(launched.ownership)); + resource = { kind: "lease", resource: command, lease }; + options.resources.retain(resource); + launched.process.stdout?.on("data", (chunk: Buffer) => + stdout.append(chunk), + ); + launched.process.stderr?.on("data", (chunk: Buffer) => + stderr.append(chunk), + ); + while (!supervisor.observedOutputClose() && !controller.signal.aborted) + await waitForAbortableDelay(25, controller.signal); + } catch (cause: unknown) { + if (!controller.signal.aborted) + spawnError = cause instanceof Error ? cause.message : String(cause); + } finally { + clearTimeout(timer); + try { + if (resource !== undefined) { + const released = await options.resources.release(resource); + if (released.kind === "failed") throw released.cause; + } + } catch (cause: unknown) { + cleanupFailure = cause instanceof Error ? cause.message : String(cause); + } + for (const [signal, handler] of signalHandlers) + process.removeListener(signal, handler); + // Restore default termination only after stopping the owned runner tree. + if (interruptedSignal !== undefined && cleanupFailure === undefined) + process.kill(process.pid, interruptedSignal); + } + const exit = supervisor?.observedExit(); + return { + exitCode: exit?.code ?? exit?.signal ?? null, + signal: exit?.signal ?? null, + stdout: stdout.text, + stderr: stderr.text, + timedOut, + ...(spawnError === undefined ? {} : { spawnError }), + ...(cleanupFailure === undefined ? {} : { cleanupFailure }), + ...(interruptedSignal === undefined ? {} : { interruptedSignal }), + }; }); +}; /** Terminal interrupts that must also stop a detached package-runner tree. */ const FORWARD_SIGNALS: readonly NodeJS.Signals[] = ["SIGINT", "SIGTERM"]; const prewarmFailureReason = (launch: RegistrationRunnerLaunch): string => { const outcome = - launch.spawnError !== undefined - ? "spawn failed" - : launch.interruptedSignal !== undefined - ? `the package runner was stopped by ${launch.interruptedSignal}` - : launch.timedOut - ? `the package runner timed out after the bounded prewarm window` - : `exit ${launch.exitCode === null ? "unknown" : String(launch.exitCode)}`; - const normalized = (launch.spawnError ?? launch.stderr) + launch.cleanupFailure !== undefined + ? "runner cleanup remains incomplete" + : launch.spawnError !== undefined + ? "spawn failed" + : launch.interruptedSignal !== undefined + ? `the package runner was stopped by ${launch.interruptedSignal}` + : launch.timedOut + ? `the package runner timed out after the bounded prewarm window` + : `exit ${launch.exitCode === null ? "unknown" : String(launch.exitCode)}`; + const normalized = ( + launch.cleanupFailure ?? + launch.spawnError ?? + launch.stderr + ) .replace(/\u001b(?:\[[0-9;?]*[ -/]*[@-~]|\([0-9A-Za-z]|[@-Z\\^_])/gu, "") .replace(/[\u0000-\u001f\u007f]+/gu, " ") .replace(/\s+/gu, " ") diff --git a/tests/boundary/filesystem/clientLifecycle.test.ts b/tests/boundary/filesystem/clientLifecycle.test.ts index 2952cee12f..7028284a40 100644 --- a/tests/boundary/filesystem/clientLifecycle.test.ts +++ b/tests/boundary/filesystem/clientLifecycle.test.ts @@ -950,7 +950,7 @@ describe("Grok Build scalar and nested values", () => { await mkdir(dirname(configPath), { recursive: true }); await writeFile( configPath, - 'mcp_servers = { rea.command = "rea", rea.args = ["mcp"], rea.startup_timeout_sec = 10 }\n', + 'mcp_servers = { rea.command = "old-rea", rea.args = ["mcp"], rea.startup_timeout_sec = 10 }\n', ); expect( await configureClientConfiguration( @@ -963,7 +963,7 @@ describe("Grok Build scalar and nested values", () => { expect(configured).not.toMatch(/^\[mcp_servers\.rea\]$/mu); expect(parseToml(configured)).toMatchObject({ mcp_servers: { - rea: { command: "rea", args: ["mcp"], startup_timeout_sec: 30 }, + rea: { command: "rea", args: ["mcp"], startup_timeout_sec: 10 }, }, }); }); diff --git a/tests/boundary/filesystem/clientRegistrationRefresh.test.ts b/tests/boundary/filesystem/clientRegistrationRefresh.test.ts new file mode 100644 index 0000000000..ec1fe47c1e --- /dev/null +++ b/tests/boundary/filesystem/clientRegistrationRefresh.test.ts @@ -0,0 +1,36 @@ +import { readFile, writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { expect, it } from "vitest"; +import { configureClientConfiguration } from "../../../src/application/SetupClientConfiguration.js"; +import { parseClientConfiguration } from "../../../src/application/ClientConfigurationDocument.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; + +it.each([90, 120])( + "preserves caller-selected Grok startup timeout %s when refreshing its launcher", + async (timeout) => { + const directory = await createTestTempDirectory("rea-grok-refresh-"); + const configPath = join(directory, "config.toml"); + const source = `[mcp_servers.rea]\ncommand = "old-rea"\nargs = ["mcp"]\nstartup_timeout_sec = ${timeout}\ncustom_policy = "caller"\n\n[mcp_servers.rea.env]\nPATH = "/caller/bin:/usr/bin"\nCUSTOM = "preserved"\n`; + await writeFile(configPath, source); + const client = { name: "grok_build", configPath, format: "grok" } as const; + expect( + await configureClientConfiguration(client, {}, ["rea", "mcp"]), + ).toMatchObject({ status: "configured" }); + const refreshed = await readFile(configPath, "utf8"); + expect( + parseClientConfiguration(refreshed, "grok").servers.rea, + ).toMatchObject({ + command: "rea", + args: ["mcp"], + startup_timeout_sec: timeout, + custom_policy: "caller", + env: { PATH: "/caller/bin:/usr/bin", CUSTOM: "preserved" }, + }); + expect(await readFile(`${configPath}.rea.backup`, "utf8")).toBe(source); + expect( + await configureClientConfiguration(client, {}, ["rea", "mcp"]), + ).toEqual({ status: "unchanged" }); + expect(await readFile(configPath, "utf8")).toBe(refreshed); + expect(await readFile(`${configPath}.rea.backup`, "utf8")).toBe(source); + }, +); diff --git a/tests/boundary/filesystem/clientRegistrationStatus.test.ts b/tests/boundary/filesystem/clientRegistrationStatus.test.ts index afd66c7921..a46a1d3981 100644 --- a/tests/boundary/filesystem/clientRegistrationStatus.test.ts +++ b/tests/boundary/filesystem/clientRegistrationStatus.test.ts @@ -569,8 +569,8 @@ describe("Node-wrapped registration policy", () => { }); }); - it.each([1, 30])( - "checks Grok Build startup timeout %d independently of launcher", + it.each([1, 30, 90])( + "accepts Grok Build caller-selected startup timeout %d independently of launcher", async (timeout) => { const home = await createTestTempDirectory("rea-grok-registration-"); await mkdir(join(home, ".grok")); @@ -585,7 +585,7 @@ describe("Node-wrapped registration policy", () => { expect(statuses).toEqual([ expect.objectContaining({ client: "grok_build", - state: timeout === 30 ? "aligned" : "stale", + state: "aligned", }), ]); },