diff --git a/src/main/claude/claude-agent-sdk-process-spawn.test.ts b/src/main/claude/claude-agent-sdk-process-spawn.test.ts index 11e5be6d524..43729e9fb9a 100644 --- a/src/main/claude/claude-agent-sdk-process-spawn.test.ts +++ b/src/main/claude/claude-agent-sdk-process-spawn.test.ts @@ -87,14 +87,27 @@ describe('claude agent SDK process spawn', () => { expect(spawn.pid).toBe(4321) expect(spec.program).toBe(globalThis.process.execPath) expect(spec.args?.[0]).toBe('-e') + expect(spec.args?.slice(2)).toEqual([ + '--', + '/usr/local/bin/claude', + '--output-format', + 'stream-json' + ]) expect(spec.detached).toBe(true) expect(spec.cwd).toBe('/work/repo') const supervisorSpec = JSON.parse( Buffer.from(String(spec.env?.ORCA_PROVIDER_SUPERVISOR_SPEC), 'base64').toString() ) + // A gone Orca closes Claude as its own close does (stdin end and SIGTERM), not with the + // root-only stdin-end drain a managed provider gets by default. + expect(vi.mocked(createProviderSpawnSpec)).toHaveBeenLastCalledWith( + expect.anything(), + expect.anything(), + platform, + { closeRequest: 'stdin-end-and-sigterm' } + ) expect(supervisorSpec).toMatchObject({ - command: '/usr/local/bin/claude', - args: ['--output-format', 'stream-json'], + closeRequest: 'stdin-end-and-sigterm', cwd: '/work/repo', ownerPid: globalThis.process.pid }) diff --git a/src/main/claude/claude-model-catalog-probe.test.ts b/src/main/claude/claude-model-catalog-probe.test.ts index 6e1bd13e1b5..25a9a28653b 100644 --- a/src/main/claude/claude-model-catalog-probe.test.ts +++ b/src/main/claude/claude-model-catalog-probe.test.ts @@ -1,6 +1,8 @@ -import { homedir } from 'node:os' +import { chmodSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { homedir, tmpdir } from 'node:os' import { join } from 'node:path' import { describe, expect, it, vi } from 'vitest' +import { PROVIDER_STDIN_END_GRACE_MS } from '../provider-process/provider-process-supervisor' import { createClaudeModelCatalogProbe } from './claude-model-catalog-probe' import { resolveClaudeStructuredInvocation } from './claude-structured-launch-resolution' import type { discoverModelsLocal } from '../text-generation/commit-message-model-discovery' @@ -124,4 +126,45 @@ describe('claude model catalog probe', () => { }) await expect(probe('/homes/a')).rejects.toThrow(/listed no models/) }) + + it.runIf(process.platform !== 'win32')( + 'lists through a supervised one-shot that answers after its input ends', + async () => { + const folder = mkdtempSync(join(tmpdir(), 'orca-claude-probe-')) + try { + const parentFile = join(folder, 'parent-pid') + const standIn = join(folder, 'claude') + // Answers only after the session stdin-end grace, the way a slow `claude -p` does. + writeFileSync( + standIn, + `#!${process.execPath} +let input = '' +process.stdin.on('data', (chunk) => (input += chunk)) +process.stdin.on('end', () => setTimeout(() => { + require('node:fs').writeFileSync(${JSON.stringify(parentFile)}, String(process.ppid)) + const request = JSON.parse(input.trim().split('\\n').at(-1)) + if (request.request.subtype !== 'list_models') process.exit(2) + console.log(JSON.stringify({ type: 'control_response', response: { subtype: 'success', + request_id: request.request_id, response: { models: [{ value: 'sonnet', displayName: 'Sonnet' }] } } })) +}, ${PROVIDER_STDIN_END_GRACE_MS * 1.5})) +` + ) + chmodSync(standIn, 0o755) + const probe = createClaudeModelCatalogProbe({ + ...probeDeps(), + resolveCommand: () => standIn, + resolveInheritedEnv: async () => ({ PATH: process.env.PATH ?? '', HOME: folder }) + }) + + await expect(probe(join(folder, 'account'))).resolves.toMatchObject({ + origin: 'probe', + models: [{ id: 'sonnet', label: 'Sonnet' }] + }) + // The stand-in's parent is the supervisor, never Orca itself. + expect(Number(readFileSync(parentFile, 'utf8'))).not.toBe(process.pid) + } finally { + rmSync(folder, { recursive: true, force: true }) + } + } + ) }) diff --git a/src/main/claude/claude-stream-json-connection.test.ts b/src/main/claude/claude-stream-json-connection.test.ts index 142c29dc329..de6925aa821 100644 --- a/src/main/claude/claude-stream-json-connection.test.ts +++ b/src/main/claude/claude-stream-json-connection.test.ts @@ -111,15 +111,9 @@ async function open( } function launchedArgv(spec: ProcessSpec | undefined): string[] { - const supervised = spec?.env?.ORCA_PROVIDER_SUPERVISOR_SPEC - if (!supervised) { - return [spec?.program ?? '', ...(spec?.args ?? [])] - } - const launch: unknown = JSON.parse(Buffer.from(supervised, 'base64').toString()) - if (!launch || typeof launch !== 'object' || !('command' in launch) || !('args' in launch)) { - return [] - } - return [String(launch.command), ...(Array.isArray(launch.args) ? launch.args.map(String) : [])] + const argv = [spec?.program ?? '', ...(spec?.args ?? [])] + // A supervised launch carries the provider's argv after the supervisor script's '--'. + return spec?.env?.ORCA_PROVIDER_SUPERVISOR_SPEC ? argv.slice(argv.indexOf('--') + 1) : argv } function childEnv(): Record { @@ -685,7 +679,10 @@ describe('Claude stream-json connection', () => { const reported = await until(() => exit, 'the supervised spawn failure') // The supervisor spawned, so this is its exit; only its stderr can say why. - expect(reported.message).toMatch(/\(code 127\).*ENOENT/s) + // Reads as the direct spawn's own error, never the supervisor's internal report. + expect(reported.message).toBe( + `claude stream-json exited (code 127): spawn ${missingCli} ENOENT` + ) // A first-hand root exit, which releases the lease like a processless start did. expect(connection.exitVerdict.root).toBe('exited') } diff --git a/src/main/claude/claude-stream-json-connection.ts b/src/main/claude/claude-stream-json-connection.ts index c9f5ee2689d..e8ada34f0b9 100644 --- a/src/main/claude/claude-stream-json-connection.ts +++ b/src/main/claude/claude-stream-json-connection.ts @@ -23,6 +23,7 @@ import { createClaudeUserMessageQueue } from './claude-agent-sdk-user-message-queue' import type { ClaudeStructuredSdkOptions } from './claude-structured-launch-resolution' +import { providerStderrForDisplay } from '../provider-process/provider-spawn-failure-report' export { ClaudeControlRequestError } @@ -103,7 +104,7 @@ export type ClaudeStreamJsonConnection = ClaudeControlSurface & { type ExitStatus = { code: number | null; signal: NodeJS.Signals | null } function exitError(stderrTail: string, status: ExitStatus | null, cause?: Error): Error { - const detail = stderrTail.trim() + const detail = providerStderrForDisplay(stderrTail).trim() // The status is the diagnostic a signed-out or refused start leaves behind; // it has to survive every wrapper between here and the user. const how = diff --git a/src/main/codex/codex-app-server-connection.test.ts b/src/main/codex/codex-app-server-connection.test.ts index 3e41a821dc6..71be8472f60 100644 --- a/src/main/codex/codex-app-server-connection.test.ts +++ b/src/main/codex/codex-app-server-connection.test.ts @@ -4,6 +4,7 @@ import { tmpdir } from 'node:os' import { PassThrough } from 'node:stream' import { providerDiagnosticOf } from '../../shared/agent-session-failure' import { afterEach, describe, expect, it, vi } from 'vitest' +import type { ProcessSpec } from '../../shared/child-process/process-spec' import type { spawnProcess } from '../../shared/child-process/run-process' import { isCodexAppServerRequestError, @@ -171,6 +172,31 @@ function responseLine(targetBytes: number, id: number): string { } describe('openCodexAppServerConnection', () => { + it.runIf(process.platform !== 'win32')( + 'has its supervisor close Codex by its stdin end, the way its own close does', + async () => { + const { child, spawnImpl } = stubChild() + const specs: ProcessSpec[] = [] + answerInitialize(child) + + const connection = await openCodexAppServerConnection( + { command: 'codex', args: ['app-server'] }, + {}, + (spec: ProcessSpec) => { + specs.push(spec) + return spawnImpl(spec) + } + ) + + expect( + JSON.parse( + Buffer.from(String(specs[0]?.env?.ORCA_PROVIDER_SUPERVISOR_SPEC), 'base64').toString() + ) + ).toMatchObject({ closeRequest: 'stdin-end' }) + await connection.close() + } + ) + it('advertises the experimental API required for rollout-path resume', async () => { const { child, spawnImpl, written } = stubChild() answerInitialize(child) diff --git a/src/main/codex/codex-app-server-exit-error.test.ts b/src/main/codex/codex-app-server-exit-error.test.ts index 63ec9a63743..9c3159286a5 100644 --- a/src/main/codex/codex-app-server-exit-error.test.ts +++ b/src/main/codex/codex-app-server-exit-error.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from 'vitest' import { providerDiagnosticOf } from '../../shared/agent-session-failure' import { buildCodexAppServerExitError } from './codex-app-server-exit-error' +import { PROVIDER_SPAWN_FAILURE_MARKER } from '../provider-process/provider-spawn-failure-report' describe('buildCodexAppServerExitError', () => { it("keeps the stderr tail apart from Orca's wording, as log text", () => { @@ -17,4 +18,16 @@ describe('buildCodexAppServerExitError', () => { expect(providerDiagnosticOf(error)).toBeUndefined() expect(providerDiagnosticOf(buildCodexAppServerExitError(''))).toBeUndefined() }) + + it('shows a supervisor spawn failure as the spawn error a direct spawn gives', () => { + const report = JSON.stringify({ + thrown: false, + code: 'ENOENT', + message: 'spawn /opt/codex ENOENT' + }) + const error = buildCodexAppServerExitError(`${PROVIDER_SPAWN_FAILURE_MARKER}${report}\n`) + + expect(error.message).toBe('codex app-server connection ended: spawn /opt/codex ENOENT') + expect(providerDiagnosticOf(error)?.text).toBe('spawn /opt/codex ENOENT') + }) }) diff --git a/src/main/codex/codex-app-server-exit-error.ts b/src/main/codex/codex-app-server-exit-error.ts index 8812e6698a0..87eaa6f88d6 100644 --- a/src/main/codex/codex-app-server-exit-error.ts +++ b/src/main/codex/codex-app-server-exit-error.ts @@ -5,11 +5,12 @@ import { providerDiagnostic, withProviderDiagnostic } from '../../shared/agent-session-failure' import { stderrIndicatesMissingAppServer } from './codex-app-server-capability-signal' import { CodexAppServerUnsupportedError } from './codex-app-server-session' +import { providerStderrForDisplay } from '../provider-process/provider-spawn-failure-report' const EXIT_DETAIL_MAX_CHARS = 400 export function buildCodexAppServerExitError(stderrTail: string, cause?: Error): Error { - const tail = stderrTail.trim().slice(0, EXIT_DETAIL_MAX_CHARS) + const tail = providerStderrForDisplay(stderrTail).trim().slice(0, EXIT_DETAIL_MAX_CHARS) // The tail is Codex's own stderr: a log, kept behind Details. const diagnostic = providerDiagnostic(tail, 'log') if (stderrIndicatesMissingAppServer(stderrTail)) { diff --git a/src/main/provider-process/managed-provider-process.ts b/src/main/provider-process/managed-provider-process.ts index 55b8f59e29f..c256aeda270 100644 --- a/src/main/provider-process/managed-provider-process.ts +++ b/src/main/provider-process/managed-provider-process.ts @@ -63,8 +63,12 @@ export function spawnManagedProviderProcess( options: ManagedProviderProcessOptions ): ManagedProviderProcess { const platform = options.platform ?? process.platform - const spec = createProviderSpawnSpec(launch, options.inheritedEnv ?? process.env, platform) - const policy = (options.policy ?? rootOnlyProviderClosePolicy)(spec.supervised) + const closePolicy = options.policy ?? rootOnlyProviderClosePolicy + const spec = createProviderSpawnSpec(launch, options.inheritedEnv ?? process.env, platform, { + // A gone owner gets the close this provider's own close would make under the supervisor. + closeRequest: closePolicy(true).signalSupervisorOnClose ? 'stdin-end-and-sigterm' : 'stdin-end' + }) + const policy = closePolicy(spec.supervised) if (spec.supervised && !(policy.gracefulExitMs >= PROVIDER_SUPERVISOR_MAX_STOP_MS)) { throw new RangeError( `Supervised provider graceful exit must wait at least ${PROVIDER_SUPERVISOR_MAX_STOP_MS} ms; received ${policy.gracefulExitMs} ms` diff --git a/src/main/provider-process/provider-process-supervisor.integration.test.ts b/src/main/provider-process/provider-process-supervisor.integration.test.ts index b3b7f068b7d..2d049dd2d21 100644 --- a/src/main/provider-process/provider-process-supervisor.integration.test.ts +++ b/src/main/provider-process/provider-process-supervisor.integration.test.ts @@ -4,13 +4,13 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, describe, expect, it } from 'vitest' import { - POSIX_PROVIDER_SUPERVISOR_SCRIPT, PROVIDER_SIGTERM_GRACE_MS, PROVIDER_STDIN_END_GRACE_MS, PROVIDER_SUPERVISOR_MAX_STOP_MS, supervisedPosixLaunch, type ProviderSupervisorOptions } from './provider-process-supervisor' +import { supervisedProviderSpawnFailure } from './provider-spawn-failure-report' // The provider leads its own group; its grandchild shares that group and ignores SIGTERM. const PROVIDER = String.raw` @@ -35,12 +35,40 @@ const PROVIDER = String.raw` setInterval(() => {}, 60000) ` +// Dies on SIGTERM at once; its grandchild, like an MCP server or a tool's child, needs 300 ms to clean up. +const CLEANS_UP_AFTER_SIGTERM_PROVIDER = String.raw` + const { spawn } = require('node:child_process') + const grandchild = spawn( + process.execPath, + ['-e', "process.on('SIGTERM', () => setTimeout(() => { require('node:fs').writeFileSync(process.env.ORCA_TEST_CLEANUP_FILE, 'done'); process.exit(0) }, 300)); process.stdout.write('armed'); setInterval(() => {}, 60000)"], + { stdio: ['ignore', 'pipe', 'ignore'] } + ) + grandchild.stdout.once('data', () => + process.stdout.write(JSON.stringify({ provider: process.pid, grandchild: grandchild.pid }) + '\n') + ) + setInterval(() => {}, 60000) +` + // Exits the moment its stdin ends, as Codex does on a normal close. const EXITS_ON_STDIN_END_PROVIDER = String.raw` process.stdin.on('end', () => process.exit(0)).resume() process.stdout.write(JSON.stringify({ provider: process.pid }) + '\n') ` +// Flushes for a moment once its stdin ends, then exits; records a SIGTERM if one arrives first. +const FLUSHES_ON_STDIN_END_PROVIDER = String.raw` + const { writeFileSync } = require('node:fs') + process.on('SIGTERM', () => { + writeFileSync(process.env.ORCA_TEST_PROVIDER_SIGNAL_FILE, 'SIGTERM') + process.exit(143) + }) + process.stdin.on('end', () => setTimeout(() => { + writeFileSync(process.env.ORCA_TEST_PROVIDER_FLUSH_FILE, 'flushed') + process.exit(0) + }, 200)).resume() + process.stdout.write(JSON.stringify({ provider: process.pid }) + '\n') +` + // Ignores stdin end and SIGTERM, recording when SIGTERM arrived, so only SIGKILL ends it. const RECORDS_SIGTERM_PROVIDER = String.raw` process.on('SIGTERM', () => { @@ -50,21 +78,37 @@ const RECORDS_SIGTERM_PROVIDER = String.raw` setInterval(() => {}, 60000) ` +// Answers a one-shot request only after the session stdin-end grace, then exits with its own code. +const ONE_SHOT_PROVIDER = String.raw` + process.on('SIGTERM', () => { + require('node:fs').writeFileSync(process.env.ORCA_TEST_PROVIDER_SIGNAL_FILE, 'SIGTERM') + }) + process.stdin.resume().on('end', () => { + setTimeout(() => { + process.stdout.write('ok') + process.exitCode = 3 + }, Number(process.env.ORCA_TEST_PROVIDER_ANSWER_DELAY_MS)) + }) +` + // Stands in for Orca: launches the supervisor as its own child, then can be killed outright. A // second child holds the supervisor's stdin open, so only the parent-death watch can notice. -// A clean-quit owner has no holder and exits normally on SIGUSR2, the way Orca quits. +// A clean-quit owner has no holder and exits normally on SIGUSR2, the way Orca quits. A one-shot +// owner ends the supervisor's stdin at once, as Orca does after writing a one-shot's request. const OWNER = String.raw` const { spawn } = require('node:child_process') const quitsCleanly = Boolean(process.env.ORCA_TEST_OWNER_QUITS_CLEANLY) + const endsStdin = Boolean(process.env.ORCA_TEST_OWNER_ENDS_STDIN) if (quitsCleanly) process.on('SIGUSR2', () => process.exit(0)) const spec = JSON.parse(Buffer.from(process.env.ORCA_PROVIDER_SUPERVISOR_SPEC, 'base64').toString()) spec.ownerPid = process.pid - const supervisor = spawn(process.execPath, ['-e', process.env.ORCA_TEST_SUPERVISOR_SCRIPT], { + const supervisor = spawn(process.execPath, JSON.parse(process.env.ORCA_TEST_SUPERVISOR_ARGS), { env: { ...process.env, ORCA_PROVIDER_SUPERVISOR_SPEC: Buffer.from(JSON.stringify(spec)).toString('base64') }, stdio: ['pipe', 'pipe', 'ignore'], detached: true }) - const holder = quitsCleanly + if (endsStdin) supervisor.stdin.end() + const holder = quitsCleanly || endsStdin ? null : spawn(process.execPath, ['-e', 'setInterval(() => {}, 60000)'], { stdio: ['ignore', supervisor.stdin, 'ignore'] @@ -86,6 +130,19 @@ const SIGNAL_AFTER_SPAWN_PRELOAD = String.raw` } ` +// Preloaded into the supervisor: its spawn fails the way EMFILE does, with no pid and no pipes. +const SPAWN_WITHOUT_PID_PRELOAD = String.raw` + const childProcess = require('node:child_process') + const { EventEmitter } = require('node:events') + childProcess.spawn = (command) => { + const child = Object.assign(new EventEmitter(), { pid: undefined, stdin: null, stdout: null, stderr: null }) + process.nextTick(() => + child.emit('error', Object.assign(new Error('spawn ' + command + ' EMFILE'), { code: 'EMFILE' })) + ) + return child + } +` + const recordedPids = new Set() const tempDirs: string[] = [] @@ -151,12 +208,13 @@ function launchSupervisor( command: process.execPath, args: ['-e', PROVIDER] }, - nodeArgs: string[] = [] + nodeArgs: string[] = [], + stderr: 'pipe' | 'ignore' = 'ignore' ): { supervisor: ChildProcess; exit: Promise<{ code: number | null; signal: string | null }> } { const launch = supervisedPosixLaunch(provider, { ...process.env, ...env }, options) const supervisor = spawn(launch.command, [...nodeArgs, ...launch.args], { env: launch.env, - stdio: ['pipe', 'pipe', 'ignore'], + stdio: ['pipe', 'pipe', stderr], detached: true }) recordedPids.add(supervisor.pid!) @@ -168,19 +226,23 @@ function launchSupervisor( async function launchUnderOwner( options: ProviderSupervisorOptions, - env: Record = {} + env: Record = {}, + provider: { script: string; pids: readonly string[] } = { + script: PROVIDER, + pids: ['provider', 'grandchild'] + } ): Promise<{ owner: ChildProcess; pids: Record }> { const launch = supervisedPosixLaunch( - { command: process.execPath, args: ['-e', PROVIDER] }, + { command: process.execPath, args: ['-e', provider.script] }, { ...process.env, ...env }, options ) const owner = spawn(process.execPath, ['-e', OWNER], { - env: { ...launch.env, ORCA_TEST_SUPERVISOR_SCRIPT: POSIX_PROVIDER_SUPERVISOR_SCRIPT }, + env: { ...launch.env, ORCA_TEST_SUPERVISOR_ARGS: JSON.stringify(launch.args) }, stdio: ['ignore', 'pipe', 'ignore'] }) recordedPids.add(owner.pid!) - const pids = await readPids(owner, ['supervisor', 'provider', 'grandchild']) + const pids = await readPids(owner, ['supervisor', ...provider.pids]) return { owner, pids } } @@ -213,6 +275,35 @@ describe.runIf(process.platform !== 'win32')('POSIX provider supervisor processe expect(alive(grandchild)).toBe(false) }) + it('kills the rest of a one-shot group at once when the provider exits on a stop', async () => { + const { supervisor, exit } = launchSupervisor({ lifetime: 'one-shot' }) + const { provider, grandchild } = await readPids(supervisor, ['provider', 'grandchild']) + + const signalledAt = Date.now() + supervisor.kill('SIGTERM') + + await expect(exit).resolves.toEqual({ code: null, signal: 'SIGTERM' }) + // The grandchild ignores SIGTERM; waiting out the 3 s grace for it would be the old cost. + expect(Date.now() - signalledAt).toBeLessThan(PROVIDER_SIGTERM_GRACE_MS / 3) + expect(alive(provider)).toBe(false) + expect(alive(grandchild)).toBe(false) + }) + + it('lets a session provider group finish its SIGTERM cleanup after the provider exits', async () => { + const cleanupFile = join(tempDir(), 'grandchild-cleaned-up') + const { supervisor, exit } = launchSupervisor( + {}, + { ORCA_TEST_CLEANUP_FILE: cleanupFile }, + { command: process.execPath, args: ['-e', CLEANS_UP_AFTER_SIGTERM_PROVIDER] } + ) + await readPids(supervisor, ['provider', 'grandchild']) + + supervisor.kill('SIGTERM') + + await expect(exit).resolves.toEqual({ code: null, signal: 'SIGTERM' }) + expect(existsSync(cleanupFile)).toBe(true) + }) + it('escalates a SIGTERM-ignoring provider to SIGKILL after the grace from the spec', async () => { const graceMs = 200 const { supervisor, exit } = launchSupervisor( @@ -352,4 +443,206 @@ describe.runIf(process.platform !== 'win32')('POSIX provider supervisor processe expect(await waitFor(() => !alive(-pids.provider), 3_000)).toBe(true) expect(existsSync(signalFile) && readFileSync(signalFile, 'utf8')).toBe('SIGTERM') }) + + it('stops a session closed with SIGTERM at once when its owner dies', async () => { + const signalFile = join(tempDir(), 'provider-sigterm-at') + const { owner, pids } = await launchUnderOwner( + { sigtermGraceMs: 200 }, + { ORCA_TEST_PROVIDER_SIGNAL_FILE: signalFile }, + { script: RECORDS_SIGTERM_PROVIDER, pids: ['provider'] } + ) + + const killedAt = Date.now() + owner.kill('SIGKILL') + + // Non-empty, not just present: a read between create and write would pass any bound. + const signalledAt = (): number => + existsSync(signalFile) ? Number(readFileSync(signalFile, 'utf8')) : 0 + expect(await waitFor(() => signalledAt() > 0, 3_000)).toBe(true) + // No unwatched stdin-end grace: the owner-death watch's 100 ms poll is the whole delay. + expect(signalledAt() - killedAt).toBeGreaterThanOrEqual(0) + expect(signalledAt() - killedAt).toBeLessThan(PROVIDER_STDIN_END_GRACE_MS / 2) + expect(await waitFor(() => !alive(-pids.provider), 3_000)).toBe(true) + }) + + it('gives a session closed by its stdin end that EOF and grace when its owner dies', async () => { + const dir = tempDir() + const signalFile = join(dir, 'provider-signal') + const flushFile = join(dir, 'provider-flushed') + const { owner, pids } = await launchUnderOwner( + { closeRequest: 'stdin-end' }, + { ORCA_TEST_PROVIDER_SIGNAL_FILE: signalFile, ORCA_TEST_PROVIDER_FLUSH_FILE: flushFile }, + { script: FLUSHES_ON_STDIN_END_PROVIDER, pids: ['provider'] } + ) + + owner.kill('SIGKILL') + + expect(await waitFor(() => !alive(pids.provider), PROVIDER_STDIN_END_GRACE_MS)).toBe(true) + expect(await waitFor(() => !alive(pids.supervisor), 3_000)).toBe(true) + expect(existsSync(flushFile)).toBe(true) + expect(existsSync(signalFile)).toBe(false) + }) + + it('stops a stdin-end session that ignores its EOF once the grace after owner death ends', async () => { + const signalFile = join(tempDir(), 'provider-sigterm-at') + const stdinEndGraceMs = 400 + const { owner, pids } = await launchUnderOwner( + { closeRequest: 'stdin-end', stdinEndGraceMs, sigtermGraceMs: 200 }, + { ORCA_TEST_PROVIDER_SIGNAL_FILE: signalFile }, + { script: RECORDS_SIGTERM_PROVIDER, pids: ['provider'] } + ) + + const killedAt = Date.now() + owner.kill('SIGKILL') + + expect(await waitFor(() => !alive(-pids.provider), 3_000)).toBe(true) + // Timers may fire a tick early against another process's clock. + expect(Number(readFileSync(signalFile, 'utf8')) - killedAt).toBeGreaterThanOrEqual( + stdinEndGraceMs - 20 + ) + expect(await waitFor(() => !alive(pids.supervisor), 3_000)).toBe(true) + }) + + describe('one-shot lifetime', () => { + it('lets a one-shot answer after its stdin ends and relays its exit code', async () => { + const signalFile = join(tempDir(), 'provider-signal') + const { supervisor, exit } = launchSupervisor( + { lifetime: 'one-shot' }, + { + ORCA_TEST_PROVIDER_SIGNAL_FILE: signalFile, + ORCA_TEST_PROVIDER_ANSWER_DELAY_MS: String(PROVIDER_STDIN_END_GRACE_MS * 1.5) + }, + { command: process.execPath, args: ['-e', ONE_SHOT_PROVIDER] } + ) + let stdout = '' + supervisor.stdout!.on('data', (chunk: Buffer) => (stdout += chunk.toString())) + + supervisor.stdin!.end('request') + + await expect(exit).resolves.toEqual({ code: 3, signal: null }) + expect(stdout).toBe('ok') + expect(existsSync(signalFile)).toBe(false) + }) + + it.each([ + ['dies', 'SIGKILL', {}], + ['quits cleanly', 'SIGUSR2', { ORCA_TEST_OWNER_QUITS_CLEANLY: '1' }] + ] as const)( + 'reaps a one-shot that ignores its stdin end when its owner %s', + async (_, signal, env) => { + const { owner, pids } = await launchUnderOwner( + { lifetime: 'one-shot', sigtermGraceMs: 300 }, + { ORCA_TEST_OWNER_ENDS_STDIN: '1', ...env } + ) + // Past the session grace: a stdin end alone must not have stopped it. + await new Promise((resolve) => setTimeout(resolve, PROVIDER_STDIN_END_GRACE_MS + 300)) + expect(alive(-pids.provider)).toBe(true) + + owner.kill(signal) + + expect(await waitFor(() => !alive(-pids.provider), PROVIDER_SUPERVISOR_MAX_STOP_MS)).toBe( + true + ) + expect(await waitFor(() => !alive(pids.supervisor), 3_000)).toBe(true) + expect(alive(pids.grandchild)).toBe(false) + } + ) + + it('escalates a SIGTERM-ignoring one-shot to SIGKILL after the grace from the spec', async () => { + const graceMs = 200 + const { supervisor, exit } = launchSupervisor( + { lifetime: 'one-shot', sigtermGraceMs: graceMs }, + { ORCA_TEST_PROVIDER_IGNORES_SIGTERM: '1' } + ) + const { provider, grandchild } = await readPids(supervisor, ['provider', 'grandchild']) + supervisor.stdin!.end() + + const signalledAt = Date.now() + supervisor.kill('SIGTERM') + + await expect(exit).resolves.toEqual({ code: null, signal: 'SIGTERM' }) + expect(Date.now() - signalledAt).toBeGreaterThanOrEqual(graceMs) + expect(Date.now() - signalledAt).toBeLessThan(PROVIDER_SIGTERM_GRACE_MS) + expect(alive(provider)).toBe(false) + expect(alive(grandchild)).toBe(false) + }) + + it('keeps the user Node options from the supervisor and hands them to the provider', async () => { + const nodeOptions = `--require ${join(tempDir(), 'missing-preload.js')}` + const { supervisor, exit } = launchSupervisor( + { lifetime: 'one-shot' }, + { NODE_OPTIONS: nodeOptions }, + { command: '/bin/sh', args: ['-c', 'printf %s "$NODE_OPTIONS"'] } + ) + let stdout = '' + supervisor.stdout!.on('data', (chunk: Buffer) => (stdout += chunk.toString())) + supervisor.stdin!.end() + + await expect(exit).resolves.toEqual({ code: 0, signal: null }) + expect(stdout).toBe(nodeOptions) + }) + + it('reports a spawn that failed without a pid through the marked line', async () => { + const preload = join(tempDir(), 'spawn-without-pid.js') + writeFileSync(preload, SPAWN_WITHOUT_PID_PRELOAD) + const { supervisor, exit } = launchSupervisor( + { lifetime: 'one-shot' }, + {}, + { command: '/opt/agent', args: [] }, + ['--require', preload], + 'pipe' + ) + let stderr = '' + supervisor.stderr!.on('data', (chunk: Buffer) => (stderr += chunk.toString())) + supervisor.stdin!.end() + + await expect(exit).resolves.toEqual({ code: 127, signal: null }) + expect(supervisedProviderSpawnFailure(127, stderr)).toMatchObject({ + thrown: false, + error: { code: 'EMFILE', message: 'spawn /opt/agent EMFILE' } + }) + }) + + it('hands the provider a near-cap argv prompt intact', async () => { + const prompt = 'x'.repeat(110 * 1024) + const { supervisor, exit } = launchSupervisor( + { lifetime: 'one-shot' }, + {}, + { + command: process.execPath, + args: ['-e', 'process.stdout.write(String(process.argv[1].length))', prompt] + } + ) + let stdout = '' + supervisor.stdout!.on('data', (chunk: Buffer) => (stdout += chunk.toString())) + supervisor.stdin!.end() + + await expect(exit).resolves.toEqual({ code: 0, signal: null }) + expect(stdout).toBe(String(prompt.length)) + }) + + it('relays all of the provider output to a slow owner before exiting', async () => { + const bytes = 4 * 1024 * 1024 + const { supervisor, exit } = launchSupervisor( + { lifetime: 'one-shot' }, + {}, + { + command: process.execPath, + args: ['-e', `process.stdout.write('x'.repeat(${bytes})); process.exitCode = 3`] + } + ) + let received = 0 + supervisor.stdout!.on('data', (chunk: Buffer) => { + received += chunk.byteLength + supervisor.stdout!.pause() + setTimeout(() => supervisor.stdout!.resume(), 5) + }) + const closed = new Promise((resolve) => supervisor.once('close', resolve)) + supervisor.stdin!.end() + + await expect(exit).resolves.toEqual({ code: 3, signal: null }) + await closed + expect(received).toBe(bytes) + }) + }) }) diff --git a/src/main/provider-process/provider-process-supervisor.test.ts b/src/main/provider-process/provider-process-supervisor.test.ts index a35f8102d6b..d83f6aed82e 100644 --- a/src/main/provider-process/provider-process-supervisor.test.ts +++ b/src/main/provider-process/provider-process-supervisor.test.ts @@ -22,16 +22,23 @@ describe('structured provider supervision', () => { const spec = supervisedPosixLaunch(command, childEnv) expect(spec.command).toBe(process.execPath) - expect(spec.args).toEqual(['-e', POSIX_PROVIDER_SUPERVISOR_SCRIPT]) + expect(spec.args).toEqual([ + '-e', + POSIX_PROVIDER_SUPERVISOR_SCRIPT, + '--', + '/opt/codex', + 'app-server', + '--flag' + ]) expect(spec.env.PATH).toBe('/bin') expect( JSON.parse(Buffer.from(spec.env.ORCA_PROVIDER_SUPERVISOR_SPEC!, 'base64').toString()) ).toEqual( expect.objectContaining({ - command: '/opt/codex', - args: ['app-server', '--flag'], cwd: '/work/repo', ownerPid: process.pid, + lifetime: 'session', + closeRequest: 'stdin-end-and-sigterm', stdinEndGraceMs: PROVIDER_STDIN_END_GRACE_MS, sigtermGraceMs: PROVIDER_SIGTERM_GRACE_MS }) @@ -50,6 +57,51 @@ describe('structured provider supervision', () => { expect(POSIX_PROVIDER_SUPERVISOR_SCRIPT).not.toContain('process.ppid === 1') }) + it('carries a one-shot lifetime through the spawn spec', () => { + const spec = createProviderSpawnSpec(launch, { PATH: '/bin' }, 'darwin', { + lifetime: 'one-shot' + }) + + expect(spec).toMatchObject({ program: process.execPath, detached: true, supervised: true }) + expect( + JSON.parse(Buffer.from(spec.env.ORCA_PROVIDER_SUPERVISOR_SPEC!, 'base64').toString()) + ).toMatchObject({ lifetime: 'one-shot' }) + }) + + it('keeps the resolved Node options out of the supervisor env and in the provider spec', () => { + // The launch's override goes through the one env rule, then only the provider gets it. + const spec = createProviderSpawnSpec( + { ...command, env: { NODE_OPTIONS: '--require /missing.js' } }, + { PATH: '/bin', NODE_OPTIONS: '--inherited', NODE_REPL_EXTERNAL_MODULE: '/repl.js' }, + 'linux' + ) + + expect(spec.env).not.toHaveProperty('NODE_OPTIONS') + expect(spec.env).not.toHaveProperty('NODE_REPL_EXTERNAL_MODULE') + expect( + JSON.parse(Buffer.from(spec.env.ORCA_PROVIDER_SUPERVISOR_SPEC!, 'base64').toString()).nodeEnv + ).toEqual({ NODE_OPTIONS: '--require /missing.js', NODE_REPL_EXTERNAL_MODULE: '/repl.js' }) + }) + + it('keeps every argv and env string of a 120 KiB argv prompt under the Linux 128 KiB cap', () => { + const prompt = 'x'.repeat(120 * 1024) + const spec = createProviderSpawnSpec( + { command: '/opt/agent', args: ['--print', prompt] }, + { PATH: '/bin' }, + 'linux', + { lifetime: 'one-shot' } + ) + const strings = [ + ...spec.args, + ...Object.entries(spec.env).map(([key, value]) => `${key}=${value}`) + ] + + expect(spec.args.slice(-2)).toEqual(['--print', prompt]) + for (const value of strings) { + expect(Buffer.byteLength(value)).toBeLessThan(128 * 1024) + } + }) + it('only accepts a resolved env, never a launch whose env it would ignore', () => { // @ts-expect-error env/envToDelete are resolved by createProviderSpawnSpec, not here. const spec = supervisedPosixLaunch(launch, { PATH: '/bin' }) diff --git a/src/main/provider-process/provider-process-supervisor.ts b/src/main/provider-process/provider-process-supervisor.ts index ac47a4a5112..10fc1c5bc3a 100644 --- a/src/main/provider-process/provider-process-supervisor.ts +++ b/src/main/provider-process/provider-process-supervisor.ts @@ -1,4 +1,5 @@ import { resolveProviderChildEnv, type ProviderProcessLaunch } from './provider-process-launch' +import { PROVIDER_SPAWN_FAILURE_MARKER } from './provider-spawn-failure-report' /** Time the provider gets to exit on its own after its stdin ends, before SIGTERM. */ export const PROVIDER_STDIN_END_GRACE_MS = 1_000 @@ -10,6 +11,8 @@ export const PROVIDER_SIGTERM_GRACE_MS = 3_000 * recovery on a process stuck in the kernel, such as one blocked on a hung network drive. */ export const PROVIDER_GROUP_REAP_TIMEOUT_MS = 1_500 +/** Longest a supervisor waits, after its provider exits, to relay output still in the pipes. */ +export const PROVIDER_OUTPUT_DRAIN_TIMEOUT_MS = 1_000 /** * Longest a supervisor can take to stop once asked (stdin end, grace, SIGTERM, grace, SIGKILL, * reap); a SIGKILL sooner can orphan its group. The graces are also the largest a spec may carry. @@ -21,6 +24,8 @@ export const PROVIDER_SUPERVISOR_MAX_STOP_MS = export const POSIX_PROVIDER_SUPERVISOR_SCRIPT = ` const { spawn } = require('node:child_process') const spec = JSON.parse(Buffer.from(process.env.ORCA_PROVIDER_SUPERVISOR_SPEC, 'base64').toString()) +// The provider's own argv, after this script's '--'. +const [providerCommand, ...providerArgs] = process.argv.slice(1) // A detached supervisor is reparented when its owner exits. The new parent may // be PID 1 or a platform subreaper, so any other parent means no live owner. const ownerGone = () => process.ppid !== spec.ownerPid @@ -28,18 +33,31 @@ const ownerGone = () => process.ppid !== spec.ownerPid for (const signal of ['SIGTERM', 'SIGINT', 'SIGHUP']) process.on(signal, () => stopProviderGroup(signal)) // Orca can die before this runs; spawning then would start a provider nothing watches. if (ownerGone()) process.exit(1) -const childEnv = { ...process.env } +const childEnv = { ...process.env, ...spec.nodeEnv } delete childEnv.ORCA_PROVIDER_SUPERVISOR_SPEC delete childEnv.ELECTRON_RUN_AS_NODE -const child = spawn(spec.command, spec.args, { - cwd: spec.cwd, - env: childEnv, - stdio: ['pipe', 'pipe', 'pipe'], - detached: true -}) +// The owner sees only this pid's exit; this marked last stderr line says the provider never started. +const exitWithSpawnFailure = (error, thrown) => { + const report = { thrown, code: (error && error.code) || 'UNKNOWN', message: String(error && error.message) } + try { require('node:fs').writeSync(2, ${JSON.stringify(PROVIDER_SPAWN_FAILURE_MARKER)} + JSON.stringify(report) + '\\n') } catch {} + process.exit(127) +} +let child +try { + child = spawn(providerCommand, providerArgs, { + cwd: spec.cwd, + env: childEnv, + stdio: ['pipe', 'pipe', 'pipe'], + detached: true + }) +} catch (error) { + // Node throws most spawn failures (ENOEXEC, ENOTDIR, ...) rather than emitting them. + exitWithSpawnFailure(error, true) +} let timer let ownerShutdownTimer let settling = false +let providerExited = false const providerGroupExists = () => { if (!child.pid) return false try { @@ -49,10 +67,10 @@ const providerGroupExists = () => { return Boolean(error && error.code !== 'ESRCH') } } -const waitForProviderGroupExit = async (timeoutMs) => { +const waitForProviderGroupExit = async (timeoutMs, untilProviderExits = false) => { const deadline = Date.now() + timeoutMs while (providerGroupExists()) { - if (Date.now() >= deadline) return false + if (Date.now() >= deadline || (untilProviderExits && providerExited)) return false await new Promise((resolve) => setTimeout(resolve, 25)) } return true @@ -78,7 +96,8 @@ const stopProviderGroup = (receivedSignal) => { clearInterval(timer) if (ownerShutdownTimer) clearTimeout(ownerShutdownTimer) try { process.kill(-child.pid, 'SIGTERM') } catch {} - void waitForProviderGroupExit(spec.sigtermGraceMs) + // A one-shot's helpers die with it once it has exited; a session's keep the grace to clean up. + void waitForProviderGroupExit(spec.sigtermGraceMs, spec.lifetime === 'one-shot') .then((exited) => exited || reapOwnedProviderGroup()) .then((reaped) => { if (!reaped) return process.exit(1) @@ -92,14 +111,31 @@ const scheduleOwnerShutdown = () => { ownerShutdownTimer = setTimeout(() => stopProviderGroup(null), spec.stdinEndGraceMs) ownerShutdownTimer.unref() } -process.stdin.once('end', scheduleOwnerShutdown) -process.stdin.once('close', scheduleOwnerShutdown) -process.stdin.pipe(child.stdin) -child.stdout.pipe(process.stdout) -child.stderr.pipe(process.stderr) -// A dead owner's stdout pipe raises EPIPE; unhandled, it would end this pid before the group. -for (const stream of [process.stdin, process.stdout, process.stderr, child.stdin, child.stdout, child.stderr]) { - stream.on('error', () => {}) +// A one-shot's stdin end is the end of its request, not a stop; only its owner's death stops it. +if (spec.lifetime !== 'one-shot') { + process.stdin.once('end', scheduleOwnerShutdown) + process.stdin.once('close', scheduleOwnerShutdown) +} +// A spawn that failed outright (EMFILE, ENFILE) has no pid and no pipes; its 'error' reports it. +if (child.pid) { + process.stdin.pipe(child.stdin) + child.stdout.pipe(process.stdout) + child.stderr.pipe(process.stderr) + // A dead owner's stdout pipe raises EPIPE; unhandled, it would end this pid before the group. + for (const stream of [process.stdin, process.stdout, process.stderr, child.stdin, child.stdout, child.stderr]) { + stream.on('error', () => {}) + } +} +// Exit can land before the provider's last output is relayed; a one-shot's answer is that output. +const drainProviderOutput = () => { + const ended = (stream) => stream.readableEnded || stream.destroyed ? null : new Promise((resolve) => { + stream.once('end', resolve) + stream.once('close', resolve) + }) + const flushed = (stream) => new Promise((resolve) => stream.write('', resolve)) + const drained = Promise.all([child.stdout, child.stderr].map(ended)) + .then(() => Promise.all([process.stdout, process.stderr].map(flushed))) + return Promise.race([drained, new Promise((resolve) => setTimeout(resolve, ${PROVIDER_OUTPUT_DRAIN_TIMEOUT_MS}))]) } const reapProviderExit = async (code, signal) => { if (settling) return @@ -107,30 +143,57 @@ const reapProviderExit = async (code, signal) => { clearInterval(timer) if (ownerShutdownTimer) clearTimeout(ownerShutdownTimer) if (!(await reapOwnedProviderGroup())) return process.exit(1) + await drainProviderOutput() finishWithProviderOutcome(code, signal) } +// An owner that is gone gets the close its owner would have asked for, made here on its behalf. timer = setInterval(() => { - if (ownerGone()) stopProviderGroup(null) + if (!ownerGone()) return + clearInterval(timer) + process.stdin.unpipe(child.stdin) + try { child.stdin.end() } catch {} + // A one-shot's stdin end was its request, so only a stop is left to ask for. + if (spec.lifetime === 'one-shot' || spec.closeRequest !== 'stdin-end') return stopProviderGroup(null) + scheduleOwnerShutdown() }, 100) timer.unref() child.once('error', (error) => { clearInterval(timer) - // The owner sees only this pid's exit; stderr is where a missing provider binary can say so. - process.stderr.write(String(error && error.message) + '\\n', () => process.exit(127)) + exitWithSpawnFailure(error, false) }) child.once('exit', (code, signal) => { + providerExited = true void reapProviderExit(code, signal) }) ` +/** + * `session`: the owner ending stdin is a close, so the provider is stopped after a grace. + * `one-shot`: stdin end only completes the request; the provider runs until it exits or is stopped. + */ +export type ProviderSupervisorLifetime = 'session' | 'one-shot' + +/** + * How a provider's owner closes it: by ending its stdin, after which a session gets its grace (a + * drain), or by ending stdin and sending SIGTERM at once (`signalSupervisorOnClose` in its close + * policy). A gone owner gets the same request. + */ +export type ProviderCloseRequest = 'stdin-end' | 'stdin-end-and-sigterm' + export type ProviderSupervisorOptions = { cwd?: string + lifetime?: ProviderSupervisorLifetime + /** Defaults to the immediate stop; a provider that drains on its stdin end opts into 'stdin-end'. */ + closeRequest?: ProviderCloseRequest /** The process the supervisor serves; it must be the supervisor's parent. */ ownerPid?: number stdinEndGraceMs?: number sigtermGraceMs?: number } +// The user's Node startup options, as the CLI launchers stash them away from Electron's bootstrap. +const PROVIDER_ONLY_NODE_ENV_KEYS = ['NODE_OPTIONS', 'NODE_REPL_EXTERNAL_MODULE'] as const + // A longer grace than the max stop allows would let recovery or close SIGKILL mid-stop. function assertGraceWithin(name: string, graceMs: number, maxMs: number): void { if (!(graceMs >= 0 && graceMs <= maxMs)) { @@ -150,29 +213,43 @@ export function supervisedPosixLaunch( { cwd = launch.cwd ?? process.cwd(), ownerPid = process.pid, + lifetime = 'session', + closeRequest = 'stdin-end-and-sigterm', stdinEndGraceMs = PROVIDER_STDIN_END_GRACE_MS, sigtermGraceMs = PROVIDER_SIGTERM_GRACE_MS }: ProviderSupervisorOptions = {} ): { command: string; args: string[]; env: NodeJS.ProcessEnv } { assertGraceWithin('stdin-end', stdinEndGraceMs, PROVIDER_STDIN_END_GRACE_MS) assertGraceWithin('SIGTERM', sigtermGraceMs, PROVIDER_SIGTERM_GRACE_MS) + // Only small fields ride in the env: Linux caps one env string at 128 KiB, and argv prompts near it. + // Electron's Node bootstrap honours these too; held in the spec, they reach only the provider. + const supervisorEnv = { ...childEnv } + const nodeEnv: Record = {} + for (const key of PROVIDER_ONLY_NODE_ENV_KEYS) { + const value = supervisorEnv[key] + delete supervisorEnv[key] + if (value !== undefined) { + nodeEnv[key] = value + } + } const supervisorSpec = Buffer.from( JSON.stringify({ - command: launch.command, - args: launch.args, cwd, ownerPid, + lifetime, + closeRequest, stdinEndGraceMs, - sigtermGraceMs + sigtermGraceMs, + nodeEnv }) ).toString('base64') return { command: process.execPath, - args: ['-e', POSIX_PROVIDER_SUPERVISOR_SCRIPT], + args: ['-e', POSIX_PROVIDER_SUPERVISOR_SCRIPT, '--', launch.command, ...launch.args], // Electron's executable needs Node mode for the inline supervisor. The // marker is removed above so providers never inherit Electron semantics. env: { - ...childEnv, + ...supervisorEnv, ELECTRON_RUN_AS_NODE: '1', ORCA_PROVIDER_SUPERVISOR_SPEC: supervisorSpec } @@ -182,7 +259,8 @@ export function supervisedPosixLaunch( export function createProviderSpawnSpec( launch: ProviderProcessLaunch, baseEnv: NodeJS.ProcessEnv, - platform: NodeJS.Platform + platform: NodeJS.Platform, + { lifetime, closeRequest }: Pick = {} ): { program: string args: string[] @@ -198,7 +276,8 @@ export function createProviderSpawnSpec( ? null : supervisedPosixLaunch( { command: launch.command, args: launch.args, cwd: launch.cwd }, - childEnv + childEnv, + { lifetime, closeRequest } ) return { program: supervisor?.command ?? launch.command, diff --git a/src/main/provider-process/provider-spawn-failure-report.test.ts b/src/main/provider-process/provider-spawn-failure-report.test.ts new file mode 100644 index 00000000000..2c02a9ba8d3 --- /dev/null +++ b/src/main/provider-process/provider-spawn-failure-report.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, it } from 'vitest' +import { + PROVIDER_SPAWN_FAILURE_MARKER, + providerStderrForDisplay, + supervisedProviderSpawnFailure +} from './provider-spawn-failure-report' + +describe('provider spawn failure report', () => { + const report = (thrown: boolean, code: string, message: string): string => + `${PROVIDER_SPAWN_FAILURE_MARKER}${JSON.stringify({ thrown, code, message })}\n` + + it.each([ + ['an emitted ENOENT', report(false, 'ENOENT', 'spawn /opt/my tools/claude ENOENT'), false], + ['a thrown ENOTDIR', report(true, 'ENOTDIR', 'spawn ENOTDIR'), true], + [ + 'a report after a runtime warning', + `Warning: Ignoring extra certs from \`/missing.pem\`, load failed\n${report(false, 'EACCES', 'spawn /opt/claude EACCES')}`, + false + ] + ])('reads %s from the supervisor last stderr line', (_, stderr, thrown) => { + const failure = supervisedProviderSpawnFailure(127, stderr) + + expect(failure?.thrown).toBe(thrown) + expect(failure?.error.code).toMatch(/^E[A-Z]+$/) + expect(failure?.error.message).toMatch(/^spawn /) + }) + + it.each([ + ['another exit code', 1, report(false, 'ENOENT', 'spawn claude ENOENT')], + ['a provider line after the report', 127, `${report(true, 'ENOEXEC', 'spawn ENOEXEC')}more\n`], + ['an unmarked spawn line', 127, 'spawn claude ENOENT\n'], + ['a malformed report', 127, `${PROVIDER_SPAWN_FAILURE_MARKER}{"thrown":true}\n`] + ])('reads no spawn failure from %s', (_, code, stderr) => { + expect(supervisedProviderSpawnFailure(code, stderr)).toBeNull() + }) + + it('shows a report as the spawn error a direct spawn gives, and other stderr unchanged', () => { + expect( + providerStderrForDisplay( + `Warning: a runtime notice\n${report(false, 'ENOENT', 'spawn /opt/claude ENOENT')}` + ) + ).toBe('Warning: a runtime notice\nspawn /opt/claude ENOENT\n') + expect(providerStderrForDisplay('Error: auth failed\n')).toBe('Error: auth failed\n') + }) +}) diff --git a/src/main/provider-process/provider-spawn-failure-report.ts b/src/main/provider-process/provider-spawn-failure-report.ts new file mode 100644 index 00000000000..d959e806836 --- /dev/null +++ b/src/main/provider-process/provider-spawn-failure-report.ts @@ -0,0 +1,52 @@ +/** Starts the one stderr line a provider supervisor writes when it could not start its provider. */ +export const PROVIDER_SPAWN_FAILURE_MARKER = '[orca-provider-supervisor] spawn failed: ' + +type SpawnFailureReport = { thrown: boolean; code: string; message: string } + +function parseSpawnFailureReport(line: string): SpawnFailureReport | null { + if (!line.startsWith(PROVIDER_SPAWN_FAILURE_MARKER)) { + return null + } + let report: unknown + try { + report = JSON.parse(line.slice(PROVIDER_SPAWN_FAILURE_MARKER.length)) + } catch { + return null + } + if ( + !report || + typeof report !== 'object' || + !('thrown' in report && typeof report.thrown === 'boolean') || + !('code' in report && typeof report.code === 'string') || + !('message' in report && typeof report.message === 'string') + ) { + return null + } + return { thrown: report.thrown, code: report.code, message: report.message } +} + +/** A provider the supervisor could not start: `thrown` when a direct spawn would have thrown. */ +export type SupervisedProviderSpawnFailure = { thrown: boolean; error: NodeJS.ErrnoException } + +/** The failure a supervisor reported on its last stderr line before exiting 127; null otherwise. */ +export function supervisedProviderSpawnFailure( + code: number | null, + stderr: string +): SupervisedProviderSpawnFailure | null { + const report = + code === 127 ? parseSpawnFailureReport(stderr.trimEnd().split('\n').at(-1) ?? '') : null + return report + ? { + thrown: report.thrown, + error: Object.assign(new Error(report.message), { code: report.code }) + } + : null +} + +/** Provider stderr as a direct spawn leaves it: a supervisor's report reads as Node's spawn error. */ +export function providerStderrForDisplay(stderr: string): string { + return stderr + .split('\n') + .map((line) => parseSpawnFailureReport(line)?.message ?? line) + .join('\n') +} diff --git a/src/main/provider-process/supervised-child-process-stop.ts b/src/main/provider-process/supervised-child-process-stop.ts new file mode 100644 index 00000000000..d87c808a1e9 --- /dev/null +++ b/src/main/provider-process/supervised-child-process-stop.ts @@ -0,0 +1,41 @@ +import type { ChildProcessHandle } from '../../shared/child-process/process-spec' +import { + closeProviderProcess, + rootOnlyProviderClosePolicy, + type ProviderProcessCloseResult +} from './provider-process-close' +import type { ProviderCloseRequest } from './provider-process-supervisor' +import { terminateProviderProcessTree } from './provider-process-teardown' + +type SupervisedChild = Pick< + ChildProcessHandle, + 'pid' | 'kill' | 'stdin' | 'exitCode' | 'signalCode' | 'once' +> + +/** + * Closes a supervised child that is not a managed provider process: the shared close, with the + * root-only policy, its tree forced (filed under `site`) only after the supervisor's full stop. + */ +export async function stopSupervisedChildProcess( + child: SupervisedChild, + { + site, + closeRequest = 'stdin-end-and-sigterm' + }: { site: string; closeRequest?: ProviderCloseRequest } +): Promise { + const exited = (): boolean => child.exitCode !== null || child.signalCode !== null + if (exited()) { + return { root: 'exited', tree: null } + } + return closeProviderProcess({ + child, + exitPromise: new Promise((resolve) => child.once('exit', () => resolve())), + rootVerdict: () => (exited() ? 'exited' : 'live'), + supervised: true, + policy: { + ...rootOnlyProviderClosePolicy(true), + signalSupervisorOnClose: closeRequest === 'stdin-end-and-sigterm' + }, + terminateTree: () => terminateProviderProcessTree(child, { site }) + }) +} diff --git a/src/main/text-generation/commit-message-model-discovery.ts b/src/main/text-generation/commit-message-model-discovery.ts index 80dbbb205a3..8691e2c92c0 100644 --- a/src/main/text-generation/commit-message-model-discovery.ts +++ b/src/main/text-generation/commit-message-model-discovery.ts @@ -4,6 +4,7 @@ import type { CommitMessagePlan } from '../../shared/commit-message-plan' import { getAgentModelProbeSpec } from '../../shared/agent-model-probe-spec' import type { TuiAgent } from '../../shared/tui-agent' import { resolveCodexHomeProcessLockKeyForSpawnEnv } from '../codex-cli/codex-home-process-lock' +import { supervisedProviderSpawnFailure } from '../provider-process/provider-spawn-failure-report' import { isSshRequestOutcomeUnverifiable } from '../ssh/ssh-channel-multiplexer' import { WINDOWS_BATCH_UNSAFE_ARGUMENTS_ERROR } from '../win32-utils' import { @@ -57,6 +58,7 @@ export async function discoverModelsLocal(input: { input.options.wslDistro ? 'linux' : process.platform ) + const couldNotStart = `${spec.label} model discovery could not be started. Check the agent CLI configuration and try again.` const startDiscovery = (): LocalProcessExecution => { let markProcessClosed!: () => void const processClosed = new Promise((resolve) => { @@ -82,10 +84,7 @@ export async function discoverModelsLocal(input: { } catch (error) { markProcessClosed() console.error('[commit-message] Failed to spawn model discovery:', error) - resolve({ - success: false, - error: `${spec.label} model discovery could not be started. Check the agent CLI configuration and try again.` - }) + resolve({ success: false, error: couldNotStart }) return } @@ -153,6 +152,19 @@ export async function discoverModelsLocal(input: { } const onClose = (code: number | null): void => { markClosedAfterTermination() + // A supervised spawn failure reads as the same failure a direct spawn reports. + const spawnFailure = outputLimitExceeded + ? null + : supervisedProviderSpawnFailure(code, stderr) + if (spawnFailure?.thrown) { + console.error('[commit-message] Failed to spawn model discovery:', spawnFailure.error) + finish({ success: false, error: couldNotStart }) + return + } + if (spawnFailure) { + onError(spawnFailure.error) + return + } finish( outputLimitExceeded ? { success: false, error: `${spec.label} returned too much model data.` } diff --git a/src/main/text-generation/commit-message-text-generation-cancellation.test.ts b/src/main/text-generation/commit-message-text-generation-cancellation.test.ts index a40912290b7..dc46c7c9e41 100644 --- a/src/main/text-generation/commit-message-text-generation-cancellation.test.ts +++ b/src/main/text-generation/commit-message-text-generation-cancellation.test.ts @@ -1,6 +1,6 @@ import { spawn } from 'node:child_process' import type * as ChildProcess from 'node:child_process' -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { cancelGenerateCommitMessageLocal, cancelGeneratePullRequestFieldsLocal, @@ -34,13 +34,26 @@ const spawnMock = vi.mocked(spawn) const expectChildTerminated = createChildTerminationExpectation(terminateWindowsProcessTreeMock) +// These suites drive fake children down the Windows direct-child path, taskkill included. The POSIX +// supervised stop with the Codex home lock (generation: timeout, cancel, output limit; discovery: +// timeout, output limit) is in source-control-local-process.test.ts. +const hostPlatform = process.platform + +afterEach(() => { + Object.defineProperty(process, 'platform', { configurable: true, value: hostPlatform }) + vi.unstubAllEnvs() +}) + beforeEach(() => { + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + // Windows resolves a bare agent name on PATH; the host's own installs must not answer it. + vi.stubEnv('PATH', '') terminateWindowsProcessTreeMock.mockClear() terminateWindowsProcessTreeMock.mockResolvedValue(undefined) spawnMock.mockClear() }) -describe('generateCommitMessageFromContext', () => { +describe('generateCommitMessageFromContext on the Windows direct-child path', () => { it('fails clearly before spawning when a jcode argv prompt exceeds the Windows command line', async () => { await withPlatform('win32', async () => { const pending = generateCommitMessageFromContext( diff --git a/src/main/text-generation/commit-message-text-generation-local-subprocess.test.ts b/src/main/text-generation/commit-message-text-generation-local-subprocess.test.ts index f73152255da..8dabc0f2bdf 100644 --- a/src/main/text-generation/commit-message-text-generation-local-subprocess.test.ts +++ b/src/main/text-generation/commit-message-text-generation-local-subprocess.test.ts @@ -1,6 +1,6 @@ import { spawn } from 'node:child_process' import type * as ChildProcess from 'node:child_process' -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { generateCommitMessageFromContext } from './commit-message-text-generation' import { createChildTerminationExpectation, @@ -27,13 +27,26 @@ const spawnMock = vi.mocked(spawn) const expectChildTerminated = createChildTerminationExpectation(terminateWindowsProcessTreeMock) +// These suites drive fake children down the Windows direct-child path, taskkill included. The POSIX +// supervised stop with the Codex home lock (generation: timeout, cancel, output limit; discovery: +// timeout, output limit) is in source-control-local-process.test.ts. +const hostPlatform = process.platform + +afterEach(() => { + Object.defineProperty(process, 'platform', { configurable: true, value: hostPlatform }) + vi.unstubAllEnvs() +}) + beforeEach(() => { + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + // Windows resolves a bare agent name on PATH; the host's own installs must not answer it. + vi.stubEnv('PATH', '') terminateWindowsProcessTreeMock.mockClear() terminateWindowsProcessTreeMock.mockResolvedValue(undefined) spawnMock.mockClear() }) -describe('generateCommitMessageFromContext', () => { +describe('generateCommitMessageFromContext on the Windows direct-child path', () => { it('caps local agent output before buffering unbounded data', async () => { const listeners = new Map void>() const child = { diff --git a/src/main/text-generation/commit-message-text-generation-model-discovery.test.ts b/src/main/text-generation/commit-message-text-generation-model-discovery.test.ts index d935dc33c92..d124349e5df 100644 --- a/src/main/text-generation/commit-message-text-generation-model-discovery.test.ts +++ b/src/main/text-generation/commit-message-text-generation-model-discovery.test.ts @@ -1,6 +1,6 @@ import { spawn } from 'node:child_process' import type * as ChildProcess from 'node:child_process' -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { createSshDisposalError, SSH_MUX_REQUEST_TIMEOUT_CODE @@ -9,9 +9,11 @@ import { discoverCommitMessageModelsLocal, discoverCommitMessageModelsRemote } from './commit-message-text-generation' +import { PROVIDER_SPAWN_FAILURE_MARKER } from '../provider-process/provider-spawn-failure-report' import { createChildTerminationExpectation, createMockDiscoveryChild, + spawnedAgentArgv, withPlatform } from './commit-message-text-generation-test-harness' @@ -33,15 +35,32 @@ vi.mock('child_process', async (importOriginal) => { const spawnMock = vi.mocked(spawn) +function spawnError(errno: string): Error { + return Object.assign(new Error(`spawn claude ${errno}`), { code: errno }) +} + const expectChildTerminated = createChildTerminationExpectation(terminateWindowsProcessTreeMock) +// These suites drive fake children down the Windows direct-child path, taskkill included. The POSIX +// supervised stop with the Codex home lock (generation: timeout, cancel, output limit; discovery: +// timeout, output limit) is in source-control-local-process.test.ts. +const hostPlatform = process.platform + +afterEach(() => { + Object.defineProperty(process, 'platform', { configurable: true, value: hostPlatform }) + vi.unstubAllEnvs() +}) + beforeEach(() => { + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + // Windows resolves a bare agent name on PATH; the host's own installs must not answer it. + vi.stubEnv('PATH', '') terminateWindowsProcessTreeMock.mockClear() terminateWindowsProcessTreeMock.mockResolvedValue(undefined) spawnMock.mockClear() }) -describe('discoverCommitMessageModelsLocal', () => { +describe('discoverCommitMessageModelsLocal on the Windows direct-child path', () => { it('returns static catalog models without spawning for static agents', async () => { const result = await discoverCommitMessageModelsLocal('amp', undefined) @@ -54,6 +73,8 @@ describe('discoverCommitMessageModelsLocal', () => { }) it('discovers dynamic models through the agent CLI', async () => { + // The host's own spawn shape: supervised on POSIX, direct on Windows. + Object.defineProperty(process, 'platform', { configurable: true, value: hostPlatform }) const listeners = new Map void>() const child = { pid: 123, @@ -78,14 +99,13 @@ describe('discoverCommitMessageModelsLocal', () => { { id: 'gpt-5.2', label: 'GPT-5.2' } ] }) - expect(spawnMock).toHaveBeenCalledWith( - 'cursor-agent', - ['--list-models'], - expect.objectContaining({ windowsHide: true }) - ) + expect(spawnedAgentArgv(spawnMock.mock.calls[0]!)).toEqual(['cursor-agent', '--list-models']) + expect(spawnMock.mock.calls[0]![2]).toMatchObject({ windowsHide: true }) }) it('writes the Claude list_models request to stdin and parses the control response', async () => { + // The host's own spawn shape: supervised on POSIX, direct on Windows. + Object.defineProperty(process, 'platform', { configurable: true, value: hostPlatform }) const listeners = new Map void>() const child = { pid: 123, @@ -135,11 +155,19 @@ describe('discoverCommitMessageModelsLocal', () => { { id: 'haiku', label: 'Haiku' } ] }) - expect(spawnMock).toHaveBeenCalledWith( + expect(spawnedAgentArgv(spawnMock.mock.calls[0]!)).toEqual([ 'claude', - ['-p', '--input-format', 'stream-json', '--output-format', 'stream-json', '--verbose'], - expect.objectContaining({ windowsHide: true, stdio: ['pipe', 'pipe', 'pipe'] }) - ) + '-p', + '--input-format', + 'stream-json', + '--output-format', + 'stream-json', + '--verbose' + ]) + expect(spawnMock.mock.calls[0]![2]).toMatchObject({ + windowsHide: true, + stdio: ['pipe', 'pipe', 'pipe'] + }) expect(child.stdin.end).toHaveBeenCalledWith(expect.stringContaining('"list_models"')) }) @@ -174,6 +202,8 @@ describe('discoverCommitMessageModelsLocal', () => { }) it('discovers dynamic models through the configured agent command override', async () => { + // The host's own spawn shape: supervised on POSIX, direct on Windows. + Object.defineProperty(process, 'platform', { configurable: true, value: hostPlatform }) const listeners = new Map void>() const child = { pid: 123, @@ -201,11 +231,11 @@ describe('discoverCommitMessageModelsLocal', () => { expect.objectContaining({ windowsHide: true }) ) } else { - expect(spawnMock).toHaveBeenCalledWith( + expect(spawnedAgentArgv(spawnMock.mock.calls[0]!)).toEqual([ 'npx', - ['cursor-agent', '--list-models'], - expect.objectContaining({ windowsHide: true }) - ) + 'cursor-agent', + '--list-models' + ]) } }) @@ -334,6 +364,51 @@ describe('discoverCommitMessageModelsLocal', () => { }) }) + const notFound = 'claude not found on PATH. Install Claude to discover models.' + const failedToStart = + 'Claude model discovery failed to start. Check the agent CLI configuration and try again.' + const couldNotStart = + 'Claude model discovery could not be started. Check the agent CLI configuration and try again.' + it.each([ + ['ENOENT', false, notFound], + ['EACCES', false, failedToStart], + ['ENOTDIR', true, couldNotStart] + ])( + 'reports a supervisor %s spawn failure as a direct spawn does', + async (errno, thrown, error) => { + const child = createMockDiscoveryChild() + spawnMock.mockReturnValue(child as never) + + const pending = discoverCommitMessageModelsLocal('claude', undefined) + child.stderr.emit( + 'data', + Buffer.from( + `Warning: an Electron startup notice\n${PROVIDER_SPAWN_FAILURE_MARKER}${JSON.stringify({ + thrown, + code: errno, + message: `spawn claude ${errno}` + })}\n` + ) + ) + child.emit('close', 127) + + await expect(pending).resolves.toEqual({ success: false, error }) + } + ) + + it.each([ + ['ENOENT', notFound], + ['EACCES', failedToStart] + ])('reports an emitted %s spawn error as before', async (errno, error) => { + const child = createMockDiscoveryChild() + spawnMock.mockReturnValue(child as never) + + const pending = discoverCommitMessageModelsLocal('claude', undefined) + child.emit('error', spawnError(errno)) + + await expect(pending).resolves.toEqual({ success: false, error }) + }) + it('settles and detaches model discovery when timeout kill is ignored', async () => { vi.useFakeTimers() const child = createMockDiscoveryChild() @@ -445,7 +520,7 @@ describe('discoverCommitMessageModelsLocal', () => { }) }) -describe('generateCommitMessageFromContext', () => { +describe('generateCommitMessageFromContext on the Windows direct-child path', () => { it('discovers dynamic models through a remote execution plan', async () => { const execute = vi.fn(async (plan, cwd, timeoutMs) => { expect(plan).toEqual({ diff --git a/src/main/text-generation/commit-message-text-generation-test-harness.ts b/src/main/text-generation/commit-message-text-generation-test-harness.ts index 8ba925ef087..9c98bf34603 100644 --- a/src/main/text-generation/commit-message-text-generation-test-harness.ts +++ b/src/main/text-generation/commit-message-text-generation-test-harness.ts @@ -30,7 +30,8 @@ export function withPlatform(platform: NodeJS.Platform, fn: () => T): T { } // Binds the caller's hoisted tree-kill mock so test bodies keep calling -// expectChildTerminated(child) with no extra argument. +// expectChildTerminated(child) with no extra argument. Asserts the Windows direct-child kill; a +// supervised POSIX child is stopped with SIGTERM instead (source-control-local-process.test.ts). export function createChildTerminationExpectation( terminateWindowsProcessTreeMock: ReturnType ): (child: { pid: number; kill: ReturnType }) => Promise { @@ -47,3 +48,11 @@ export function createChildTerminationExpectation( await vi.waitFor(() => expect(child.kill).toHaveBeenCalledWith('SIGKILL')) } } + +/** The argv a spawn started as the agent: past the supervisor script's '--' when supervised. */ +export function spawnedAgentArgv([file, args]: readonly unknown[]): unknown[] { + const argv: unknown[] = Array.isArray(args) ? args : [] + return file === process.execPath && argv[0] === '-e' + ? argv.slice(argv.indexOf('--') + 1) + : [file, ...argv] +} diff --git a/src/main/text-generation/source-control-agent-launch.test.ts b/src/main/text-generation/source-control-agent-launch.test.ts new file mode 100644 index 00000000000..7c1670361c7 --- /dev/null +++ b/src/main/text-generation/source-control-agent-launch.test.ts @@ -0,0 +1,183 @@ +import { EventEmitter } from 'node:events' +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' +import type * as RunProcess from '../../shared/child-process/run-process' +import type { ProcessSpec } from '../../shared/child-process/process-spec' +import { POSIX_PROVIDER_SUPERVISOR_SCRIPT } from '../provider-process/provider-process-supervisor' +import { generateCommitMessageFromContext } from './commit-message-text-generation' +import { withPlatform } from './commit-message-text-generation-test-harness' +import { spawnSourceControlAgent } from './source-control-agent-launch' + +const { spawnProcessMock } = vi.hoisted(() => ({ spawnProcessMock: vi.fn() })) + +vi.mock('../../shared/child-process/run-process', async (importOriginal) => { + const actual = await importOriginal() + spawnProcessMock.mockImplementation(actual.spawnProcess) + return { ...actual, spawnProcess: spawnProcessMock } +}) + +function fakeChild(): EventEmitter & { stdin: { on: () => void; end: () => void } } { + return Object.assign(new EventEmitter(), { stdin: { on: vi.fn(), end: vi.fn() } }) +} + +function lastSpawnSpec(): ProcessSpec { + return spawnProcessMock.mock.calls.at(-1)![0] +} + +function decodedSupervisorSpec(spec: ProcessSpec): Record { + return JSON.parse(Buffer.from(spec.env!.ORCA_PROVIDER_SUPERVISOR_SPEC!, 'base64').toString()) +} + +const folder = mkdtempSync(join(tmpdir(), 'orca-agent-launch-')) +afterAll(() => rmSync(folder, { recursive: true, force: true })) + +beforeEach(() => { + spawnProcessMock.mockClear() +}) + +describe('spawnSourceControlAgent', () => { + it.each([ + ['in its own cwd', true, '/work/repo'], + ['in the process cwd', false, process.cwd()] + ])('runs a POSIX agent one-shot under the provider supervisor %s', (_, useCwd, cwd) => { + spawnProcessMock.mockReturnValueOnce(fakeChild()) + const child = withPlatform('linux', () => + spawnSourceControlAgent({ + binary: '/opt/agent/claude', + args: ['-p', '--verbose'], + cwd: '/work/repo', + env: { PATH: '/usr/bin', AGENT_TOKEN: 'kept' }, + stdinMode: 'pipe', + useCwdForNative: useCwd + }) + ) + + const spec = lastSpawnSpec() + expect(child.supervised).toBe(true) + expect(spec).toMatchObject({ + program: process.execPath, + args: ['-e', POSIX_PROVIDER_SUPERVISOR_SCRIPT, '--', '/opt/agent/claude', '-p', '--verbose'], + cwd, + detached: true, + stdio: ['pipe', 'pipe', 'pipe'] + }) + expect(spec.env).toMatchObject({ PATH: '/usr/bin', AGENT_TOKEN: 'kept' }) + expect(decodedSupervisorSpec(spec)).toMatchObject({ cwd, lifetime: 'one-shot' }) + }) + + it('spawns a Windows agent directly, without a supervisor', () => { + spawnProcessMock.mockReturnValueOnce(fakeChild()) + const child = withPlatform('win32', () => + spawnSourceControlAgent({ + binary: 'C:/tools/agent.exe', + args: ['--print'], + env: { PATH: 'C:/tools' }, + stdinMode: 'pipe', + useCwdForNative: false + }) + ) + + expect(child.supervised).toBeUndefined() + expect(lastSpawnSpec()).toMatchObject({ program: 'C:/tools/agent.exe', args: ['--print'] }) + expect(lastSpawnSpec().env).not.toHaveProperty('ORCA_PROVIDER_SUPERVISOR_SPEC') + }) +}) + +describe.runIf(process.platform !== 'win32')('supervised agent processes', () => { + it('reports a missing agent binary as not found, as a direct spawn does', async () => { + const missing = join(folder, 'missing-agent') + + await expect( + generateCommitMessageFromContext( + { branch: 'main', stagedSummary: 'M README.md', stagedPatch: '+test' }, + { agentId: 'custom', model: '', customAgentCommand: `"${missing}"` }, + { kind: 'local', cwd: folder } + ) + ).resolves.toEqual({ + success: false, + error: `${missing} not found on PATH. Install ${missing} to use AI commit messages.` + }) + }) + + it('reports an agent that cannot be executed as failing to start, as a direct spawn does', async () => { + const agent = join(folder, 'not-executable-agent') + writeFileSync(agent, '#!/bin/sh\necho never\n', { mode: 0o644 }) + + await expect( + generateCommitMessageFromContext( + { branch: 'main', stagedSummary: 'M README.md', stagedPatch: '+test' }, + { agentId: 'custom', model: '', customAgentCommand: `"${agent}"` }, + { kind: 'local', cwd: folder } + ) + ).resolves.toEqual({ + success: false, + error: `${agent} failed to start. Check the agent command in Settings and try again.` + }) + }) + + it.each([ + // Only macOS throws ENOEXEC; glibc's execvp hands such a file to /bin/sh, direct or supervised. + ...(process.platform === 'darwin' + ? [['an executable that is not a program', 'garbage', (path: string) => path] as const] + : []), + ['a path through a file', 'file', (path: string) => join(path, 'agent')] as const + ])('reports %s as not startable, as a direct spawn does', async (_, name, commandFor) => { + const file = join(folder, name) + writeFileSync(file, Buffer.from([0, 1, 2, 3]), { mode: 0o755 }) + const command = commandFor(file) + + await expect( + generateCommitMessageFromContext( + { branch: 'main', stagedSummary: 'M README.md', stagedPatch: '+test' }, + { agentId: 'custom', model: '', customAgentCommand: `"${command}"` }, + { kind: 'local', cwd: folder } + ) + ).resolves.toEqual({ + success: false, + error: `${command} could not be started. Check the agent command in Settings and try again.` + }) + }) + + // Electron prints startup warnings (a bad NODE_EXTRA_CA_CERTS); Node's debug log stands in here. + it('reads a missing binary as not found past the runtime own stderr output', async () => { + const missing = join(folder, 'missing-behind-warning') + + await expect( + generateCommitMessageFromContext( + { branch: 'main', stagedSummary: 'M README.md', stagedPatch: '+test' }, + { agentId: 'custom', model: '', customAgentCommand: `"${missing}"` }, + { + kind: 'local', + cwd: folder, + env: { ...process.env, NODE_DEBUG: 'child_process' } + } + ) + ).resolves.toEqual({ + success: false, + error: `${missing} not found on PATH. Install ${missing} to use AI commit messages.` + }) + }) + + it('passes the stdin end through so the agent can answer its request', async () => { + const agent = join(folder, 'echo-agent.cjs') + writeFileSync( + agent, + `let prompt = '' +process.stdin.on('data', (chunk) => (prompt += chunk)) +process.stdin.on('end', () => setTimeout(() => console.log(prompt.includes('README') ? 'Update README' : 'no prompt'), 1200)) +` + ) + + await expect( + generateCommitMessageFromContext( + { branch: 'main', stagedSummary: 'M README.md', stagedPatch: '+test' }, + { agentId: 'custom', model: '', customAgentCommand: `"${process.execPath}" "${agent}"` }, + { kind: 'local', cwd: folder } + ) + ).resolves.toMatchObject({ success: true, message: 'Update README' }) + expect(lastSpawnSpec().program).toBe(process.execPath) + expect(decodedSupervisorSpec(lastSpawnSpec())).toMatchObject({ lifetime: 'one-shot' }) + }) +}) diff --git a/src/main/text-generation/source-control-agent-launch.ts b/src/main/text-generation/source-control-agent-launch.ts index 709c0481390..056a3eb4b47 100644 --- a/src/main/text-generation/source-control-agent-launch.ts +++ b/src/main/text-generation/source-control-agent-launch.ts @@ -1,9 +1,11 @@ import { spawnProcess } from '../../shared/child-process/run-process' import { withCliRuntimeOnPath } from '../../shared/node-cli-command-resolution' import { resolveCliCommand } from '../codex-cli/command' +import { createProviderSpawnSpec } from '../provider-process/provider-process-supervisor' import { wslAwareSpawn } from '../git/runner' import { getSpawnArgsForWindows } from '../win32-utils' import type { + SourceControlAgentSpawnInput, SpawnedSourceControlAgentProcess, SpawnSourceControlAgent } from './source-control-text-generation-types' @@ -57,20 +59,55 @@ export const spawnSourceControlAgent: SpawnSourceControlAgent = (input) => { } ) as SpawnedSourceControlAgentProcess } - const resolvedBinary = + const child = process.platform === 'win32' - ? resolveCliCommand(input.binary, { pathEnv: spawnEnv.PATH ?? spawnEnv.Path ?? null }) - : input.binary - const { spawnCmd, spawnArgs } = getSpawnArgsForWindows(resolvedBinary, input.args) - const child = spawnProcess({ - program: spawnCmd, - args: spawnArgs, - env: withCliRuntimeOnPath(resolvedBinary, spawnEnv), - ...(input.useCwdForNative ? { cwd: input.cwd } : {}) - }) + ? spawnWindowsAgent(input, spawnEnv) + : spawnSupervisedAgent(input, spawnEnv) if (input.stdinMode === 'ignore') { child.stdin?.on?.('error', () => {}) child.stdin?.end() } return child } + +function spawnWindowsAgent( + input: SourceControlAgentSpawnInput, + spawnEnv: NodeJS.ProcessEnv +): SpawnedSourceControlAgentProcess { + const resolvedBinary = resolveCliCommand(input.binary, { + pathEnv: spawnEnv.PATH ?? spawnEnv.Path ?? null + }) + const { spawnCmd, spawnArgs } = getSpawnArgsForWindows(resolvedBinary, input.args) + return spawnProcess({ + program: spawnCmd, + args: spawnArgs, + env: withCliRuntimeOnPath(resolvedBinary, spawnEnv), + ...(input.useCwdForNative ? { cwd: input.cwd } : {}) + }) +} + +// Under the provider supervisor, an Orca that quits or dies mid-run still stops the agent's group. +function spawnSupervisedAgent( + input: SourceControlAgentSpawnInput, + spawnEnv: NodeJS.ProcessEnv +): SpawnedSourceControlAgentProcess { + const spec = createProviderSpawnSpec( + { + command: input.binary, + args: input.args, + ...(input.useCwdForNative && input.cwd !== undefined ? { cwd: input.cwd } : {}) + }, + withCliRuntimeOnPath(input.binary, spawnEnv), + process.platform, + { lifetime: 'one-shot' } + ) + const child = spawnProcess({ + program: spec.program, + args: spec.args, + env: spec.env, + cwd: spec.cwd, + detached: spec.detached, + stdio: ['pipe', 'pipe', 'pipe'] + }) + return Object.assign(child, { supervised: spec.supervised }) +} diff --git a/src/main/text-generation/source-control-local-process.test.ts b/src/main/text-generation/source-control-local-process.test.ts new file mode 100644 index 00000000000..508df035f08 --- /dev/null +++ b/src/main/text-generation/source-control-local-process.test.ts @@ -0,0 +1,411 @@ +import { EventEmitter } from 'node:events' +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { rootOnlyProviderClosePolicy } from '../provider-process/provider-process-close' +import { PROVIDER_SUPERVISOR_MAX_STOP_MS } from '../provider-process/provider-process-supervisor' +import { SOURCE_CONTROL_GENERATION_TIMEOUT_MS } from './source-control-generation-limits' +import { discoverModelsLocal } from './commit-message-model-discovery' +import { cancelGenerateCommitMessageLocal } from './commit-message-text-generation' +import { spawnSourceControlAgent } from './source-control-agent-launch' +import { killSourceControlAgentProcess } from './source-control-local-process' +import { generateCommitMessage } from './source-control-text-generation-requests' +import type { SpawnedSourceControlAgentProcess } from './source-control-text-generation-types' + +const { terminateTreeMock } = vi.hoisted(() => ({ + terminateTreeMock: vi.fn(async () => true) +})) + +vi.mock('../provider-process/provider-process-teardown', () => ({ + terminateProviderProcessTree: terminateTreeMock +})) + +type FakeSupervisor = EventEmitter & { + pid: number + exitCode: number | null + signalCode: NodeJS.Signals | null + supervised: true + kill: ReturnType boolean>> +} + +function fakeSupervisor(exitsOn: NodeJS.Signals | null): FakeSupervisor { + const child: FakeSupervisor = Object.assign(new EventEmitter(), { + pid: 4242, + exitCode: null, + signalCode: null, + supervised: true as const, + kill: vi.fn((signal?: NodeJS.Signals) => { + if (signal === exitsOn) { + child.signalCode = signal + child.emit('exit', null, signal) + } + return true + }) + }) + return child +} + +// Behaves as the supervisor does: a SIGTERM ends it, after the time its provider takes to stop. +function fakeSupervisedAgent(stopMs: number): FakeSupervisor & { + stdout: EventEmitter + stderr: EventEmitter + stdin: { on: () => void; end: () => void } +} { + const child = Object.assign(fakeSupervisor(null), { + stdout: new EventEmitter(), + stderr: new EventEmitter(), + stdin: { on: vi.fn(), end: vi.fn() } + }) + child.kill.mockImplementation((signal?: NodeJS.Signals) => { + if (signal === 'SIGTERM') { + setTimeout(() => { + child.signalCode = 'SIGTERM' + child.emit('exit', null, 'SIGTERM') + child.emit('close', null, 'SIGTERM') + }, stopMs) + } + return true + }) + return child +} + +function asSpawned(child: FakeSupervisor): SpawnedSourceControlAgentProcess { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the stop and the generation read only pid, exit state, kill, the stdio streams and the child events, which the fakes implement. + return child as unknown as SpawnedSourceControlAgentProcess +} + +function isAlive(pid: number): boolean { + try { + process.kill(pid, 0) + return true + } catch { + return false + } +} + +afterEach(() => { + vi.useRealTimers() + terminateTreeMock.mockClear() +}) + +describe('killSourceControlAgentProcess for a supervised agent', () => { + it('asks the supervisor to stop and leaves its group to it', async () => { + const child = fakeSupervisor('SIGTERM') + + await killSourceControlAgentProcess(asSpawned(child)) + + expect(child.kill.mock.calls).toEqual([['SIGTERM']]) + expect(terminateTreeMock).not.toHaveBeenCalled() + }) + + it('tears the tree down only once the supervisor has had its full stop time', async () => { + vi.useFakeTimers() + const child = fakeSupervisor(null) + + const stopped = killSourceControlAgentProcess(asSpawned(child)) + await vi.advanceTimersByTimeAsync(PROVIDER_SUPERVISOR_MAX_STOP_MS - 1) + expect(child.kill.mock.calls).toEqual([['SIGTERM']]) + expect(terminateTreeMock).not.toHaveBeenCalled() + + await vi.advanceTimersByTimeAsync(1) + expect(terminateTreeMock).toHaveBeenCalledWith(child, { + site: 'source-control-text-generation' + }) + // The shared close then gives the forced root its own short wait. + await vi.advanceTimersByTimeAsync(rootOnlyProviderClosePolicy(true).forcedExitMs) + await stopped + expect(child.kill).not.toHaveBeenCalledWith('SIGKILL') + }) + + it('signals nothing once the supervisor has exited', async () => { + const child = fakeSupervisor(null) + child.exitCode = 0 + + await killSourceControlAgentProcess(asSpawned(child)) + + expect(child.kill).not.toHaveBeenCalled() + expect(terminateTreeMock).not.toHaveBeenCalled() + }) +}) + +describe('a supervised agent one-shot that runs out of time', () => { + it('times out at once but holds the Codex home until the supervisor has stopped', async () => { + vi.useFakeTimers() + const stopMs = 2_000 + const agents = [fakeSupervisedAgent(stopMs), fakeSupervisedAgent(stopMs)] + const spawnAgent = vi.fn(() => asSpawned(agents[spawnAgent.mock.calls.length - 1]!)) + const request = (cwd: string): ReturnType => + generateCommitMessage({ + context: { branch: 'main', stagedSummary: 'M README.md', stagedPatch: '+test' }, + params: { agentId: 'codex', model: 'gpt-5.5' }, + target: { kind: 'local', cwd, env: { CODEX_HOME: '/codex/timeout-home' } }, + spawnAgent + }) + + const first = request('/repo/first') + await vi.advanceTimersByTimeAsync(SOURCE_CONTROL_GENERATION_TIMEOUT_MS) + await expect(first).resolves.toMatchObject({ + success: false, + error: expect.stringMatching(/timed out/) + }) + expect(agents[0]!.kill.mock.calls).toEqual([['SIGTERM']]) + + const second = request('/repo/second') + await vi.advanceTimersByTimeAsync(stopMs - 1) + expect(spawnAgent).toHaveBeenCalledTimes(1) + await vi.advanceTimersByTimeAsync(1) + expect(spawnAgent).toHaveBeenCalledTimes(2) + + agents[1]!.stdout.emit('data', Buffer.from('Update README\n')) + agents[1]!.emit('close', 0) + await expect(second).resolves.toMatchObject({ success: true, message: 'Update README' }) + expect(agents[0]!.kill).not.toHaveBeenCalledWith('SIGKILL') + expect(terminateTreeMock).not.toHaveBeenCalled() + }) +}) + +describe('a supervised Codex model discovery that runs out of time', () => { + it('times out at once but holds the Codex home until the supervisor has stopped', async () => { + vi.useFakeTimers() + const stopMs = 2_000 + const agents = [fakeSupervisedAgent(stopMs), fakeSupervisedAgent(stopMs)] + const spawnAgent = vi.fn(() => asSpawned(agents[spawnAgent.mock.calls.length - 1]!)) + const discover = (): ReturnType => + discoverModelsLocal({ + agentId: 'codex', + env: { CODEX_HOME: '/codex/discovery-timeout-home' }, + options: {}, + backslash: 'escape', + spawnAgent + }) + + const first = discover() + await vi.advanceTimersByTimeAsync(SOURCE_CONTROL_GENERATION_TIMEOUT_MS) + await expect(first).resolves.toMatchObject({ + success: false, + error: expect.stringMatching(/timed out/) + }) + expect(agents[0]!.kill.mock.calls).toEqual([['SIGTERM']]) + + const second = discover() + await vi.advanceTimersByTimeAsync(stopMs - 1) + expect(spawnAgent).toHaveBeenCalledTimes(1) + await vi.advanceTimersByTimeAsync(1) + expect(spawnAgent).toHaveBeenCalledTimes(2) + + agents[1]!.stdout.emit( + 'data', + Buffer.from(JSON.stringify({ models: [{ slug: 'gpt-5.5', display_name: 'GPT-5.5' }] })) + ) + agents[1]!.emit('close', 0) + await expect(second).resolves.toMatchObject({ success: true }) + expect(agents[0]!.kill).not.toHaveBeenCalledWith('SIGKILL') + expect(terminateTreeMock).not.toHaveBeenCalled() + }) +}) + +describe.runIf(process.platform !== 'win32')( + 'killSourceControlAgentProcess on a real agent', + () => { + it('stops an agent that ignores its stdin end through its supervisor', async () => { + const child = spawnSourceControlAgent({ + binary: process.execPath, + args: ['-e', 'console.log(process.pid); setInterval(() => {}, 60000)'], + env: process.env, + stdinMode: 'ignore', + useCwdForNative: false + }) + const agentPid = await new Promise((resolve) => + child.stdout.once('data', (chunk: Buffer) => resolve(Number(chunk.toString().trim()))) + ) + + try { + await killSourceControlAgentProcess(child) + + expect(child.signalCode ?? child.exitCode).not.toBeNull() + expect(() => process.kill(agentPid, 0)).toThrow(expect.objectContaining({ code: 'ESRCH' })) + expect(terminateTreeMock).not.toHaveBeenCalled() + } finally { + for (const pid of [agentPid, child.pid!]) { + try { + process.kill(pid, 'SIGKILL') + } catch { + // Already gone, as the test expects. + } + } + } + }) + + it('holds the Codex home until a canceled agent has stopped through its supervisor', async () => { + const folder = mkdtempSync(join(tmpdir(), 'orca-supervised-cancel-')) + const pids: number[] = [] + try { + const home = join(folder, 'codex-home') + for (const repo of ['first', 'second']) { + mkdirSync(join(folder, repo)) + } + const pidFile = join(folder, 'first-pid') + const stoppedFile = join(folder, 'first-stopped') + // Ignores its stdin end, then takes a moment to stop on SIGTERM, as a CLI flushing state does. + const slowStop = join(folder, 'slow-stop.cjs') + writeFileSync( + slowStop, + `require('node:fs').writeFileSync(${JSON.stringify(pidFile)}, String(process.pid)) +process.stdin.resume() +process.on('SIGTERM', () => setTimeout(() => { + require('node:fs').writeFileSync(${JSON.stringify(stoppedFile)}, 'stopped') + process.exit(0) +}, 300)) +setInterval(() => {}, 60000) +` + ) + const answers = join(folder, 'answers.cjs') + writeFileSync( + answers, + "process.stdin.resume().on('end', () => console.log('Update README'))\n" + ) + const firstAliveAtSecondSpawn: boolean[] = [] + const spawnAgent = vi.fn((input: Parameters[0]) => { + if (pids.length > 0) { + firstAliveAtSecondSpawn.push(isAlive(pids[0]!)) + } + return spawnSourceControlAgent(input) + }) + const request = (cwd: string, script: string): ReturnType => + generateCommitMessage({ + context: { branch: 'main', stagedSummary: 'M README.md', stagedPatch: '+test' }, + params: { + agentId: 'codex', + model: 'gpt-5.5', + agentCommandOverride: `CODEX_HOME="${home}" "${process.execPath}" "${script}"` + }, + target: { kind: 'local', cwd, env: process.env }, + spawnAgent + }) + + const first = request(join(folder, 'first'), slowStop) + await vi.waitFor(() => expect(existsSync(pidFile)).toBe(true), { timeout: 10_000 }) + pids.push(Number(readFileSync(pidFile, 'utf8'))) + cancelGenerateCommitMessageLocal(join(folder, 'first')) + await expect(first).resolves.toMatchObject({ canceled: true }) + + await expect(request(join(folder, 'second'), answers)).resolves.toMatchObject({ + success: true, + message: 'Update README' + }) + expect(firstAliveAtSecondSpawn).toEqual([false]) + expect(readFileSync(stoppedFile, 'utf8')).toBe('stopped') + expect(terminateTreeMock).not.toHaveBeenCalled() + } finally { + for (const pid of pids) { + if (isAlive(pid)) { + process.kill(pid, 'SIGKILL') + } + } + rmSync(folder, { recursive: true, force: true }) + } + }) + + it('stops an agent through its supervisor once its output passes the limit', async () => { + const folder = mkdtempSync(join(tmpdir(), 'orca-supervised-output-limit-')) + const pids: number[] = [] + try { + const pidFile = join(folder, 'agent-pid') + const agent = join(folder, 'floods.cjs') + // Floods stdout past the limit, then waits on its never-answered request. + writeFileSync( + agent, + `require('node:fs').writeFileSync(${JSON.stringify(pidFile)}, String(process.pid)) +process.stdin.resume() +process.stdout.write('x'.repeat(5 * 1024 * 1024)) +setInterval(() => {}, 60000) +` + ) + + await expect( + generateCommitMessage({ + context: { branch: 'main', stagedSummary: 'M README.md', stagedPatch: '+test' }, + params: { + agentId: 'custom', + model: '', + customAgentCommand: `"${process.execPath}" "${agent}"` + }, + target: { kind: 'local', cwd: folder, env: process.env }, + spawnAgent: spawnSourceControlAgent + }) + ).resolves.toMatchObject({ + success: false, + error: expect.stringMatching(/too much output/) + }) + pids.push(Number(readFileSync(pidFile, 'utf8'))) + expect(isAlive(pids[0]!)).toBe(false) + expect(terminateTreeMock).not.toHaveBeenCalled() + } finally { + for (const pid of pids) { + if (isAlive(pid)) { + process.kill(pid, 'SIGKILL') + } + } + rmSync(folder, { recursive: true, force: true }) + } + }) + + it('stops a Codex discovery through its supervisor once its output passes the limit, holding the home', async () => { + const folder = mkdtempSync(join(tmpdir(), 'orca-supervised-discovery-limit-')) + const pids: number[] = [] + try { + const home = join(folder, 'codex-home') + const pidFile = join(folder, 'flood-pid') + const floods = join(folder, 'floods.cjs') + // Floods stdout past the limit, then takes a moment to stop on SIGTERM. + writeFileSync( + floods, + `require('node:fs').writeFileSync(${JSON.stringify(pidFile)}, String(process.pid)) +process.on('SIGTERM', () => setTimeout(() => process.exit(0), 300)) +process.stdout.write('x'.repeat(5 * 1024 * 1024)) +setInterval(() => {}, 60000) +` + ) + const lists = join(folder, 'lists.cjs') + writeFileSync( + lists, + "console.log(JSON.stringify({ models: [{ slug: 'gpt-5.5', display_name: 'GPT-5.5' }] }))\n" + ) + const floodAliveAtSecondSpawn: boolean[] = [] + const spawnAgent = vi.fn((input: Parameters[0]) => { + if (pids.length > 0) { + floodAliveAtSecondSpawn.push(isAlive(pids[0]!)) + } + return spawnSourceControlAgent(input) + }) + const discover = (script: string): ReturnType => + discoverModelsLocal({ + agentId: 'codex', + env: process.env, + agentCommandOverride: `CODEX_HOME="${home}" "${process.execPath}" "${script}"`, + options: { cwd: folder }, + backslash: 'escape', + spawnAgent + }) + + await expect(discover(floods)).resolves.toEqual({ + success: false, + error: 'Codex returned too much model data.' + }) + pids.push(Number(readFileSync(pidFile, 'utf8'))) + + await expect(discover(lists)).resolves.toMatchObject({ success: true }) + expect(floodAliveAtSecondSpawn).toEqual([false]) + expect(terminateTreeMock).not.toHaveBeenCalled() + } finally { + for (const pid of pids) { + if (isAlive(pid)) { + process.kill(pid, 'SIGKILL') + } + } + rmSync(folder, { recursive: true, force: true }) + } + }) + } +) diff --git a/src/main/text-generation/source-control-local-process.ts b/src/main/text-generation/source-control-local-process.ts index 1a2d6c52b82..664ad4b182a 100644 --- a/src/main/text-generation/source-control-local-process.ts +++ b/src/main/text-generation/source-control-local-process.ts @@ -1,4 +1,6 @@ import type { CommitMessagePlan } from '../../shared/commit-message-plan' +import { supervisedProviderSpawnFailure } from '../provider-process/provider-spawn-failure-report' +import { stopSupervisedChildProcess } from '../provider-process/supervised-child-process-stop' import { UnsafeWindowsBatchArgumentsError } from '../win32-utils' import { terminateWindowsProcessTree } from '../windows-process-tree-kill' import { @@ -22,6 +24,8 @@ import type { TextGenerationOperation } from './source-control-text-generation-types' +const SOURCE_CONTROL_KILL_SITE = 'source-control-text-generation' + export async function killSourceControlAgentProcess( child: SpawnedSourceControlAgentProcess ): Promise { @@ -29,12 +33,16 @@ export async function killSourceControlAgentProcess( if (!pid) { return } + if (child.supervised) { + await stopSupervisedChildProcess(child, { site: SOURCE_CONTROL_KILL_SITE }) + return + } if (process.platform === 'win32') { // taskkill owns the tree, but the own-Chromium gate can refuse the // pid-addressed walk; the handle-addressed root kill below cannot reach the // recycled pid it refused, and callers release the managed-home lock on this // promise, so it must not resolve having killed nothing. - await terminateWindowsProcessTree(pid, { site: 'source-control-text-generation' }) + await terminateWindowsProcessTree(pid, { site: SOURCE_CONTROL_KILL_SITE }) } try { child.kill('SIGKILL') @@ -93,6 +101,7 @@ export function runLocalSourceControlPlan(input: { const processClosed = new Promise((resolve) => { markProcessClosed = resolve }) + const couldNotStart = `${plan.label} could not be started. Check the agent command in Settings and try again.` const result = new Promise((resolve) => { let child: SpawnedSourceControlAgentProcess try { @@ -121,10 +130,7 @@ export function runLocalSourceControlPlan(input: { return } console.error('[commit-message] Failed to spawn local generator:', error) - resolve({ - success: false, - error: `${plan.label} could not be started. Check the agent command in Settings and try again.` - }) + resolve({ success: false, error: couldNotStart }) return } @@ -223,6 +229,17 @@ export function runLocalSourceControlPlan(input: { }) return } + // A supervised spawn failure reads as the same failure a direct spawn reports. + const spawnFailure = supervisedProviderSpawnFailure(code, stderr) + if (spawnFailure?.thrown) { + console.error('[commit-message] Failed to spawn local generator:', spawnFailure.error) + finalize({ success: false, error: couldNotStart }) + return + } + if (spawnFailure) { + onError(spawnFailure.error) + return + } finalize( finalizeFromAgentOutput({ code, diff --git a/src/main/text-generation/source-control-text-generation-types.ts b/src/main/text-generation/source-control-text-generation-types.ts index a7b74c35d3a..20b0e0d3e92 100644 --- a/src/main/text-generation/source-control-text-generation-types.ts +++ b/src/main/text-generation/source-control-text-generation-types.ts @@ -77,12 +77,15 @@ export type LocalProcessExecution = { processClosed: Promise } -export type SpawnedSourceControlAgentProcess = ReturnType +export type SpawnedSourceControlAgentProcess = ReturnType & { + /** True when the child is the POSIX provider supervisor: SIGTERM stops the agent's group, then itself. */ + readonly supervised?: boolean +} export type LocalGenerationTarget = Extract export type RemoteGenerationTarget = Extract -export type SpawnSourceControlAgent = (input: { +export type SourceControlAgentSpawnInput = { binary: string args: string[] cwd?: string @@ -92,4 +95,8 @@ export type SpawnSourceControlAgent = (input: { commandEnv?: Record stdinMode: 'ignore' | 'pipe' useCwdForNative: boolean -}) => SpawnedSourceControlAgentProcess +} + +export type SpawnSourceControlAgent = ( + input: SourceControlAgentSpawnInput +) => SpawnedSourceControlAgentProcess