mirror of
https://github.com/davidkaya/aryx.git
synced 2026-08-08 20:58:45 +02:00
fix: scope sidecar exit handling to the active process
Wait for an intentional sidecar dispose to exit before refreshing capabilities, and ignore exit events from stale sidecar processes. This prevents a refresh from killing the old process and then having that old exit reject the new capabilities request with an unexpected sidecar exit error. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -1,4 +1,5 @@
|
|||||||
import { app } from 'electron';
|
import { app } from 'electron';
|
||||||
|
import { once } from 'node:events';
|
||||||
import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process';
|
import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process';
|
||||||
|
|
||||||
import type {
|
import type {
|
||||||
@@ -12,7 +13,10 @@ import type {
|
|||||||
} from '@shared/contracts/sidecar';
|
} from '@shared/contracts/sidecar';
|
||||||
import type { ChatMessageRecord } from '@shared/domain/session';
|
import type { ChatMessageRecord } from '@shared/domain/session';
|
||||||
import { createSidecarEnvironment } from '@main/sidecar/sidecarEnvironment';
|
import { createSidecarEnvironment } from '@main/sidecar/sidecarEnvironment';
|
||||||
import { shouldRestartSidecarOnCapabilityRefresh } from '@main/sidecar/sidecarRefresh';
|
import {
|
||||||
|
shouldHandleSidecarExit,
|
||||||
|
shouldRestartSidecarOnCapabilityRefresh,
|
||||||
|
} from '@main/sidecar/sidecarRefresh';
|
||||||
import {
|
import {
|
||||||
markRunTurnPendingErrored,
|
markRunTurnPendingErrored,
|
||||||
shouldHandleRunTurnEvent,
|
shouldHandleRunTurnEvent,
|
||||||
@@ -72,12 +76,18 @@ export class SidecarClient {
|
|||||||
}
|
}
|
||||||
|
|
||||||
async dispose(): Promise<void> {
|
async dispose(): Promise<void> {
|
||||||
if (!this.process) {
|
const sidecar = this.process;
|
||||||
|
if (!sidecar) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
this.process.kill();
|
if (sidecar.exitCode !== null || sidecar.signalCode !== null) {
|
||||||
this.process = undefined;
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
const exitPromise = once(sidecar, 'exit');
|
||||||
|
sidecar.kill();
|
||||||
|
await exitPromise;
|
||||||
}
|
}
|
||||||
|
|
||||||
hasActiveRunTurn(): boolean {
|
hasActiveRunTurn(): boolean {
|
||||||
@@ -101,25 +111,30 @@ export class SidecarClient {
|
|||||||
resourcesPath: process.resourcesPath,
|
resourcesPath: process.resourcesPath,
|
||||||
platform: process.platform,
|
platform: process.platform,
|
||||||
});
|
});
|
||||||
this.process = spawn(sidecar.command, sidecar.args, {
|
const childProcess = spawn(sidecar.command, sidecar.args, {
|
||||||
cwd: sidecar.cwd,
|
cwd: sidecar.cwd,
|
||||||
env: createSidecarEnvironment(process.env),
|
env: createSidecarEnvironment(process.env),
|
||||||
stdio: 'pipe',
|
stdio: 'pipe',
|
||||||
windowsHide: true,
|
windowsHide: true,
|
||||||
});
|
});
|
||||||
|
this.process = childProcess;
|
||||||
|
|
||||||
this.process.stdout.setEncoding('utf8');
|
childProcess.stdout.setEncoding('utf8');
|
||||||
this.process.stdout.on('data', (chunk: string) => {
|
childProcess.stdout.on('data', (chunk: string) => {
|
||||||
this.stdoutBuffer += chunk;
|
this.stdoutBuffer += chunk;
|
||||||
this.flushStdoutBuffer();
|
this.flushStdoutBuffer();
|
||||||
});
|
});
|
||||||
|
|
||||||
this.process.stderr.setEncoding('utf8');
|
childProcess.stderr.setEncoding('utf8');
|
||||||
this.process.stderr.on('data', (chunk: string) => {
|
childProcess.stderr.on('data', (chunk: string) => {
|
||||||
console.error('[kopaya sidecar]', chunk.trim());
|
console.error('[kopaya sidecar]', chunk.trim());
|
||||||
});
|
});
|
||||||
|
|
||||||
this.process.on('exit', (code) => {
|
childProcess.on('exit', (code) => {
|
||||||
|
if (!shouldHandleSidecarExit(this.process?.pid, childProcess.pid)) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
const error = new Error(`The .NET sidecar exited unexpectedly with code ${code ?? 'unknown'}.`);
|
const error = new Error(`The .NET sidecar exited unexpectedly with code ${code ?? 'unknown'}.`);
|
||||||
for (const pending of this.pending.values()) {
|
for (const pending of this.pending.values()) {
|
||||||
pending.reject(error);
|
pending.reject(error);
|
||||||
@@ -129,7 +144,7 @@ export class SidecarClient {
|
|||||||
this.stdoutBuffer = '';
|
this.stdoutBuffer = '';
|
||||||
});
|
});
|
||||||
|
|
||||||
return this.process;
|
return childProcess;
|
||||||
}
|
}
|
||||||
|
|
||||||
private async dispatch<TResult>(
|
private async dispatch<TResult>(
|
||||||
|
|||||||
@@ -1,3 +1,10 @@
|
|||||||
export function shouldRestartSidecarOnCapabilityRefresh(hasActiveRunTurn: boolean): boolean {
|
export function shouldRestartSidecarOnCapabilityRefresh(hasActiveRunTurn: boolean): boolean {
|
||||||
return !hasActiveRunTurn;
|
return !hasActiveRunTurn;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export function shouldHandleSidecarExit(
|
||||||
|
activeProcessId: number | undefined,
|
||||||
|
exitingProcessId: number | undefined,
|
||||||
|
): boolean {
|
||||||
|
return activeProcessId !== undefined && exitingProcessId !== undefined && activeProcessId === exitingProcessId;
|
||||||
|
}
|
||||||
|
|||||||
@@ -1,6 +1,9 @@
|
|||||||
import { describe, expect, test } from 'bun:test';
|
import { describe, expect, test } from 'bun:test';
|
||||||
|
|
||||||
import { shouldRestartSidecarOnCapabilityRefresh } from '@main/sidecar/sidecarRefresh';
|
import {
|
||||||
|
shouldHandleSidecarExit,
|
||||||
|
shouldRestartSidecarOnCapabilityRefresh,
|
||||||
|
} from '@main/sidecar/sidecarRefresh';
|
||||||
|
|
||||||
describe('shouldRestartSidecarOnCapabilityRefresh', () => {
|
describe('shouldRestartSidecarOnCapabilityRefresh', () => {
|
||||||
test('restarts the sidecar when no run-turn is active', () => {
|
test('restarts the sidecar when no run-turn is active', () => {
|
||||||
@@ -11,3 +14,18 @@ describe('shouldRestartSidecarOnCapabilityRefresh', () => {
|
|||||||
expect(shouldRestartSidecarOnCapabilityRefresh(true)).toBe(false);
|
expect(shouldRestartSidecarOnCapabilityRefresh(true)).toBe(false);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('shouldHandleSidecarExit', () => {
|
||||||
|
test('handles exit events from the currently active sidecar process', () => {
|
||||||
|
expect(shouldHandleSidecarExit(1234, 1234)).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('ignores exit events from a stale sidecar process', () => {
|
||||||
|
expect(shouldHandleSidecarExit(1234, 5678)).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('ignores exit events when either process id is missing', () => {
|
||||||
|
expect(shouldHandleSidecarExit(undefined, 5678)).toBe(false);
|
||||||
|
expect(shouldHandleSidecarExit(1234, undefined)).toBe(false);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user