diff --git a/packages/code/src/native-process-child.test.ts b/packages/code/src/native-process-child.test.ts index 1d4866ae..8aca3652 100644 --- a/packages/code/src/native-process-child.test.ts +++ b/packages/code/src/native-process-child.test.ts @@ -33,7 +33,7 @@ for (const signal of ['SIGINT', 'SIGHUP', 'SIGTERM'] as const) { // installation finished without requiring platform SRT dependencies. child.once('message', () => child.kill(signal)); child.send({ id: 'startup-probe', type: 'probe' }); - assert.deepEqual(await exited, { code: 1, signal: null }); + assert.deepEqual(await exited, { code: 0, signal: null }); }, ); } diff --git a/packages/code/src/native-process-child.ts b/packages/code/src/native-process-child.ts index b5221731..05ffedfe 100644 --- a/packages/code/src/native-process-child.ts +++ b/packages/code/src/native-process-child.ts @@ -25,7 +25,10 @@ const shutdown = () => { if (shuttingDown) return; shuttingDown = true; active?.controller.abort(); - void (sandbox?.close() ?? Promise.resolve()).finally(() => process.exit(1)); + void (sandbox?.close() ?? Promise.resolve()).then( + () => process.exit(0), + () => process.exit(1), + ); setTimeout(() => process.exit(1), 5000); }; process.on('disconnect', shutdown); diff --git a/packages/code/src/native-process.test.ts b/packages/code/src/native-process.test.ts index 8dcfd4c1..800dcf11 100644 --- a/packages/code/src/native-process.test.ts +++ b/packages/code/src/native-process.test.ts @@ -301,6 +301,76 @@ test('executor close drains an active command before closing IPC', async () => { await assert.rejects(sandbox.execute(request), /unavailable/); }); +test('executor close resolves when the child exits during the close handshake', async () => { + const fake = fixture(); + const sandbox = new NativeProcessWorkspaceCommandSandbox( + { workspaceRoot: '/workspace' }, + fake.fork, + ); + await sandbox.prepare(); + Object.assign(fake.child, { + send(message: Record, callback: (error: null) => void) { + fake.messages.push(message); + callback(null); + queueMicrotask(() => { + Object.assign(fake.child, { connected: false }); + fake.child.emit('exit', 1, null); + fake.child.emit('disconnect'); + }); + return true; + }, + }); + await sandbox.close(); + assert.equal(fake.messages.filter((m) => m.type === 'close').length, 1); + await assert.rejects(sandbox.execute(request), /unavailable/); +}); + +test('executor close still reports a cleanup failure the child replies with', async () => { + const fake = fixture(); + const sandbox = new NativeProcessWorkspaceCommandSandbox( + { workspaceRoot: '/workspace' }, + fake.fork, + ); + await sandbox.prepare(); + Object.assign(fake.child, { + send(message: Record, callback: (error: null) => void) { + fake.messages.push(message); + callback(null); + queueMicrotask(() => + fake.child.emit('message', { + id: message.id, + ok: false, + code: 'COMMAND_UNAVAILABLE', + errorMessage: 'scratch cleanup failed', + mutation: false, + requiresQuarantine: false, + }), + ); + return true; + }, + }); + await assert.rejects(sandbox.close(), /scratch cleanup failed/); + assert.equal(fake.killCalls, 1); + await assert.rejects(sandbox.execute(request), /unavailable/); +}); + +test('executor close skips the handshake once the child is already lost', async () => { + const fake = fixture(); + const sandbox = new NativeProcessWorkspaceCommandSandbox( + { workspaceRoot: '/workspace' }, + fake.fork, + ); + await sandbox.prepare(); + Object.assign(fake.child, { connected: false }); + fake.child.emit('exit', 1, null); + await sandbox.close(); + assert.equal( + fake.messages.some((m) => m.type === 'close'), + false, + ); + await assert.rejects(sandbox.execute(request), /unavailable/); +}); + test('executor startup loss is not reported as an applied mutation', async () => { const fake = fixture(); const sandbox = new NativeProcessWorkspaceCommandSandbox( diff --git a/packages/code/src/native-process.ts b/packages/code/src/native-process.ts index 96d89355..e1a02498 100644 --- a/packages/code/src/native-process.ts +++ b/packages/code/src/native-process.ts @@ -73,6 +73,15 @@ export function nativeExecutorEnvironment( ); } +/** The executor process was lost, refused a send, or stalled past its + * deadline, as opposed to a failure the executor reported explicitly. */ +class NativeExecutorUnavailableError extends WorkspaceToolError { + constructor(mutation: boolean) { + super('Native executor is unavailable', 'COMMAND_UNAVAILABLE', mutation); + this.name = 'NativeExecutorUnavailableError'; + } +} + /** One persistent, process-isolated SRT manager per workspace. No automatic * restart/replay: losing IPC after execution starts is an ambiguous mutation. */ export class NativeProcessWorkspaceCommandSandbox @@ -109,11 +118,7 @@ export class NativeProcessWorkspaceCommandSandbox } private unavailable(mutation: boolean): WorkspaceToolError { - return new WorkspaceToolError( - 'Native executor is unavailable', - 'COMMAND_UNAVAILABLE', - mutation, - ); + return new NativeExecutorUnavailableError(mutation); } private async start(): Promise { @@ -340,12 +345,17 @@ export class NativeProcessWorkspaceCommandSandbox return this.closing; } + /** An executor that exits, disconnects, or stalls while closing is + * terminated in `finally` regardless, and the active command has already + * drained, so only a failure the executor reports explicitly is surfaced. */ private async stop(): Promise { await this.active?.catch(() => undefined); await this.ready?.catch(() => undefined); try { if (this.child?.connected && !this.failed) - await this.rpc('close', {}, 10_000, false); + await this.rpc('close', {}, 10_000, false).catch((error: unknown) => { + if (!(error instanceof NativeExecutorUnavailableError)) throw error; + }); } finally { this.failed = true; this.terminate();