fix(team): harden MCP HTTP startup cleanup
This commit is contained in:
parent
2cae0a080a
commit
48826af00b
4 changed files with 121 additions and 22 deletions
|
|
@ -168,37 +168,57 @@ export class AgentTeamsMcpHttpServer {
|
|||
this.handle = null;
|
||||
}
|
||||
};
|
||||
child.once('exit', (code, signal) => {
|
||||
clearIfCurrent();
|
||||
logger.warn(
|
||||
`Agent Teams MCP HTTP server exited${typeof code === 'number' ? ` with code ${code}` : ''}${
|
||||
signal ? ` (${signal})` : ''
|
||||
}`
|
||||
);
|
||||
});
|
||||
child.once('error', (error) => {
|
||||
clearIfCurrent();
|
||||
logger.warn(
|
||||
`Agent Teams MCP HTTP server process error: ${
|
||||
error instanceof Error ? error.message : String(error)
|
||||
}`
|
||||
);
|
||||
});
|
||||
child.stderr?.on('data', (chunk: Buffer) => {
|
||||
const text = chunk.toString('utf8').trim();
|
||||
if (text) {
|
||||
logger.debug(`Agent Teams MCP HTTP stderr: ${text.slice(0, 1000)}`);
|
||||
}
|
||||
});
|
||||
this.child = child;
|
||||
|
||||
let startupSettled = false;
|
||||
const startupFailure = new Promise<never>((_, reject) => {
|
||||
child.once('exit', (code, signal) => {
|
||||
clearIfCurrent();
|
||||
const codeSuffix = typeof code === 'number' ? ` with code ${code}` : '';
|
||||
const signalSuffix = signal ? ` (${signal})` : '';
|
||||
const message = `Agent Teams MCP HTTP server exited before startup completed${codeSuffix}${signalSuffix}`;
|
||||
if (!startupSettled) {
|
||||
reject(new Error(message));
|
||||
}
|
||||
logger.warn(message);
|
||||
});
|
||||
child.once('error', (error) => {
|
||||
clearIfCurrent();
|
||||
const message = `Agent Teams MCP HTTP server process error: ${
|
||||
error instanceof Error ? error.message : String(error)
|
||||
}`;
|
||||
if (!startupSettled) {
|
||||
reject(error instanceof Error ? error : new Error(message));
|
||||
}
|
||||
logger.warn(message);
|
||||
});
|
||||
});
|
||||
|
||||
try {
|
||||
await waitForPort(MCP_HTTP_HOST, port, MCP_HTTP_READY_TIMEOUT_MS);
|
||||
await Promise.race([
|
||||
waitForPort(MCP_HTTP_HOST, port, MCP_HTTP_READY_TIMEOUT_MS),
|
||||
startupFailure,
|
||||
]);
|
||||
if (this.child !== child) {
|
||||
throw new Error('Agent Teams MCP HTTP server exited before startup completed');
|
||||
}
|
||||
} catch (error) {
|
||||
startupSettled = true;
|
||||
if (this.child === child) {
|
||||
this.child = null;
|
||||
this.handle = null;
|
||||
}
|
||||
killProcessTree(child, 'SIGKILL');
|
||||
throw error;
|
||||
}
|
||||
|
||||
this.child = child;
|
||||
startupSettled = true;
|
||||
this.handle = {
|
||||
url: `http://${MCP_HTTP_HOST}:${port}${MCP_HTTP_ENDPOINT}`,
|
||||
port,
|
||||
|
|
|
|||
|
|
@ -96,13 +96,13 @@ export async function cleanupManagedOpenCodeServeProcesses(
|
|||
}
|
||||
|
||||
const details = await readDetails(row.pid);
|
||||
const isManagedByWindowsCommand =
|
||||
platform === 'win32' && isAppManagedWindowsOpenCodeServeCommand(row.command);
|
||||
const isManaged =
|
||||
platform === 'win32'
|
||||
? isAppManagedWindowsOpenCodeServeCommand(row.command) ||
|
||||
Boolean(details && isManagedOpenCodeServeProcessDetails(details))
|
||||
: Boolean(details && isManagedOpenCodeServeProcessDetails(details));
|
||||
isManagedByWindowsCommand || Boolean(details && isManagedOpenCodeServeProcessDetails(details));
|
||||
const hasRequiredDetailsMarkers =
|
||||
requiredDetailsMarkers.length === 0 ||
|
||||
(isManagedByWindowsCommand && details === null) ||
|
||||
Boolean(details && processDetailsIncludeMarkers(details, requiredDetailsMarkers));
|
||||
if (!isManaged || !hasRequiredDetailsMarkers) {
|
||||
result.candidates.push({
|
||||
|
|
|
|||
|
|
@ -145,6 +145,58 @@ describe('AgentTeamsMcpHttpServer', () => {
|
|||
expect(spawnProcess).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('fails startup promptly when the child exits before readiness', async () => {
|
||||
const child = new FakeChildProcess();
|
||||
const server = new AgentTeamsMcpHttpServer({
|
||||
resolveLaunchSpec: async () => ({
|
||||
command: 'node',
|
||||
args: ['mcp-server/dist/index.js'],
|
||||
}),
|
||||
allocatePort: async () => 41003,
|
||||
spawnProcess: vi.fn(() => child as any),
|
||||
waitForPort: vi.fn(() => {
|
||||
child.emit('exit', 1, null);
|
||||
return new Promise<void>(() => {
|
||||
// Keep readiness pending so startup resolves only through the child exit.
|
||||
});
|
||||
}),
|
||||
});
|
||||
|
||||
await expect(server.ensureStarted()).rejects.toThrow(
|
||||
'Agent Teams MCP HTTP server exited before startup completed with code 1'
|
||||
);
|
||||
expect(hoisted.killProcessTreeMock).toHaveBeenCalledWith(child, 'SIGKILL');
|
||||
expect(vi.mocked(console.warn).mock.calls[0]?.join(' ')).toContain(
|
||||
'Agent Teams MCP HTTP server exited before startup completed with code 1'
|
||||
);
|
||||
vi.mocked(console.warn).mockClear();
|
||||
});
|
||||
|
||||
it('does not return a handle if the child exits during readiness polling', async () => {
|
||||
const child = new FakeChildProcess();
|
||||
const server = new AgentTeamsMcpHttpServer({
|
||||
resolveLaunchSpec: async () => ({
|
||||
command: 'node',
|
||||
args: ['mcp-server/dist/index.js'],
|
||||
}),
|
||||
allocatePort: async () => 41004,
|
||||
spawnProcess: vi.fn(() => child as any),
|
||||
waitForPort: vi.fn(async () => {
|
||||
await Promise.resolve();
|
||||
child.emit('exit', 0, null);
|
||||
}),
|
||||
});
|
||||
|
||||
await expect(server.ensureStarted()).rejects.toThrow(
|
||||
'Agent Teams MCP HTTP server exited before startup completed'
|
||||
);
|
||||
expect(hoisted.killProcessTreeMock).toHaveBeenCalledWith(child, 'SIGKILL');
|
||||
expect(vi.mocked(console.warn).mock.calls[0]?.join(' ')).toContain(
|
||||
'Agent Teams MCP HTTP server exited before startup completed with code 0'
|
||||
);
|
||||
vi.mocked(console.warn).mockClear();
|
||||
});
|
||||
|
||||
it('waits for the HTTP health endpoint before marking the server ready', async () => {
|
||||
const child = new FakeChildProcess();
|
||||
const port = await allocateLoopbackPort();
|
||||
|
|
|
|||
|
|
@ -403,6 +403,33 @@ describe('OpenCodeManagedHostProcessCleanup', () => {
|
|||
expect(result.diagnostics).toEqual([]);
|
||||
});
|
||||
|
||||
it('does not require unreadable Windows details for app-managed command fallback cleanup', async () => {
|
||||
const killProcess = vi.fn();
|
||||
|
||||
const result = await cleanupManagedOpenCodeServeProcesses({
|
||||
mode: 'force',
|
||||
platform: 'win32',
|
||||
requiredDetailsMarkers: ['CLAUDE_TEAM_APP_INSTANCE_ID=app-1'],
|
||||
listProcessRows: () =>
|
||||
resolved([
|
||||
{
|
||||
pid: 71629,
|
||||
ppid: 86256,
|
||||
command:
|
||||
'"C:\\Users\\User\\AppData\\Roaming\\claude-agent-teams-ui\\data\\runtimes\\opencode\\versions\\1.14.48\\opencode-windows-x64\\opencode.exe" serve --hostname 127.0.0.1 --port 49914',
|
||||
},
|
||||
]),
|
||||
readProcessDetails: () => resolved(null),
|
||||
disposeServeHost: () => resolved(undefined),
|
||||
isProcessAlive: () => false,
|
||||
killProcess,
|
||||
});
|
||||
|
||||
expect(killProcess).toHaveBeenCalledWith(71629);
|
||||
expect(result.candidates[0]).toMatchObject({ pid: 71629, action: 'killed' });
|
||||
expect(result.diagnostics).toEqual([]);
|
||||
});
|
||||
|
||||
it('keeps app-managed Windows OpenCode serve processes while their parent is still alive', async () => {
|
||||
const killProcess = vi.fn();
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue