diff --git a/cli/src/agent/runners/runAgentSession.test.ts b/cli/src/agent/runners/runAgentSession.test.ts index fe188657..7872f535 100644 --- a/cli/src/agent/runners/runAgentSession.test.ts +++ b/cli/src/agent/runners/runAgentSession.test.ts @@ -17,7 +17,9 @@ const harness = vi.hoisted(() => ({ vi.mock('@/agent/sessionFactory', () => ({ bootstrapSession: vi.fn(async () => ({ + // bootstrapSession returns ApiSessionClient as `session` (not a wrapper). session: { + sessionId: 'hub-session-acp-1', updateAgentState: vi.fn(), onUserMessage: vi.fn((handler) => { harness.userMessageHandler = handler @@ -167,7 +169,9 @@ describe('runAgentSession', () => { '--url', 'http://127.0.0.1:1234', '--tools', - 'change_title,display_image,display_video,display_media,list_peers,ping_peer,inspect_peer,skill_lookup' + 'change_title,display_image,display_video,display_media,list_peers,ping_peer,inspect_peer,skill_lookup', + '--hapi-session-id', + 'hub-session-acp-1', ]) expect(harness.newSessionOptions).toMatchObject({ cwd: '/tmp/project', diff --git a/cli/src/agent/runners/runAgentSession.ts b/cli/src/agent/runners/runAgentSession.ts index d937ee54..cc7bc0a2 100644 --- a/cli/src/agent/runners/runAgentSession.ts +++ b/cli/src/agent/runners/runAgentSession.ts @@ -75,12 +75,15 @@ export async function runAgentSession(opts: { flavor: opts.agentType } }); + // Stamp hub row id for orphan MCP reap (#2003). const bridgeCommand = getHappyCliCommand([ 'mcp', '--url', happyServer.url, '--tools', - happyServer.toolNames.join(',') + happyServer.toolNames.join(','), + '--hapi-session-id', + session.sessionId, ]); const mcpServers = [ { diff --git a/cli/src/codex/happyMcpStdioBridge.ts b/cli/src/codex/happyMcpStdioBridge.ts index be0d03d0..e1bcd979 100644 --- a/cli/src/codex/happyMcpStdioBridge.ts +++ b/cli/src/codex/happyMcpStdioBridge.ts @@ -36,6 +36,11 @@ function parseArgs(argv: string[]): { url: string | null; toolNames: Set } else if (a === '--tools' && i + 1 < argv.length) { toolNames = new Set(argv[i + 1].split(',').map((name) => name.trim()).filter(Boolean)); i++; + } else if (a === '--hapi-session-id' && i + 1 < argv.length) { + // Orphan-reap stamp only (#2003); consume so the value is not left as a stray argv token. + i++; + } else if (a.startsWith('--hapi-session-id=')) { + // `--hapi-session-id=` form — nothing further to consume. } } return { url, toolNames }; diff --git a/cli/src/codex/utils/buildHapiMcpBridge.test.ts b/cli/src/codex/utils/buildHapiMcpBridge.test.ts index 8af0d577..efba2583 100644 --- a/cli/src/codex/utils/buildHapiMcpBridge.test.ts +++ b/cli/src/codex/utils/buildHapiMcpBridge.test.ts @@ -71,7 +71,9 @@ describe('buildHapiMcpBridge skill lookup config', () => { '--url', 'http://127.0.0.1:43006/', '--tools', - 'change_title,display_image,display_video,display_media,list_peers,ping_peer,inspect_peer,skill_lookup' + 'change_title,display_image,display_video,display_media,list_peers,ping_peer,inspect_peer,skill_lookup', + '--hapi-session-id', + 'hub-session-1', ]) expect(bridge.mcpServers.hapi.tools).toEqual({ change_title: { approval_mode: 'approve' }, @@ -86,7 +88,15 @@ describe('buildHapiMcpBridge skill lookup config', () => { it('does not expose skill_lookup for native-skill bridge callers', async () => { const bridge = await buildHapiMcpBridge(createClient()) - expect(harness.cliArgs.at(-1)).toBe('change_title,display_image,display_video,display_media,list_peers,ping_peer,inspect_peer') + expect(harness.cliArgs).toEqual([ + 'mcp', + '--url', + 'http://127.0.0.1:43006/', + '--tools', + 'change_title,display_image,display_video,display_media,list_peers,ping_peer,inspect_peer', + '--hapi-session-id', + 'hub-session-1', + ]) expect(bridge.mcpServers.hapi.tools).toEqual({ change_title: { approval_mode: 'approve' }, display_image: { approval_mode: 'prompt' }, diff --git a/cli/src/codex/utils/buildHapiMcpBridge.ts b/cli/src/codex/utils/buildHapiMcpBridge.ts index 0d6ae485..8091db55 100644 --- a/cli/src/codex/utils/buildHapiMcpBridge.ts +++ b/cli/src/codex/utils/buildHapiMcpBridge.ts @@ -84,12 +84,16 @@ export async function buildHapiMcpBridge( enableChangeTitle: options.enableChangeTitle, skillLookup: options.skillLookup }); + // Stamp the hub row id so orphanReap can find MCP helpers after the agent + // root dies without a full tree kill (#2003 / #1911 follow-up). const bridgeCommand = getHappyCliCommand([ 'mcp', '--url', happyServer.url, '--tools', - happyServer.toolNames.join(',') + happyServer.toolNames.join(','), + '--hapi-session-id', + client.sessionId, ]); const tools: Record = { display_image: { diff --git a/cli/src/runner/orphanReap.test.ts b/cli/src/runner/orphanReap.test.ts index 89d52091..11d6a6b3 100644 --- a/cli/src/runner/orphanReap.test.ts +++ b/cli/src/runner/orphanReap.test.ts @@ -3,6 +3,7 @@ import { dirname, join } from 'node:path' import { fileURLToPath } from 'node:url' import { describe, expect, it } from 'vitest' import { + commandMatchesHapiMcpBridgeForSession, commandMatchesRunnerSpawnedSession, findRunnerSpawnedOrphanTargets, findStopSessionOrphanTargets, @@ -45,6 +46,34 @@ describe('orphanReap argv matching', () => { expect(commandMatchesRunnerSpawnedSession(cmd, sessionId)).toBe(false) }) + it('matches stamped HAPI MCP bridge helpers without --started-by (#2003)', () => { + const cmd = `hapi-hapi-soup mcp --url http://127.0.0.1:45619/ --tools display_image --hapi-session-id ${sessionId}` + expect(commandMatchesHapiMcpBridgeForSession(cmd, sessionId)).toBe(true) + expect(commandMatchesRunnerSpawnedSession(cmd, sessionId)).toBe(false) + }) + + it('rejects MCP helpers for a different session id', () => { + const cmd = `hapi mcp --url http://127.0.0.1:1/ --hapi-session-id other-sess` + expect(commandMatchesHapiMcpBridgeForSession(cmd, sessionId)).toBe(false) + }) + + it('rejects unstamped legacy MCP helpers (no session flag)', () => { + const cmd = 'hapi mcp --url http://127.0.0.1:1/ --tools display_image' + expect(commandMatchesHapiMcpBridgeForSession(cmd, sessionId)).toBe(false) + }) + + it('selectOrphanPidsForSession includes stamped MCP bridges alongside agent roots', () => { + const pids = selectOrphanPidsForSession( + [ + { pid: 10, cmd: `bun src/index.ts cursor --started-by runner --existing-session-id ${sessionId}`, name: 'bun' }, + { pid: 11, cmd: `hapi mcp --url http://127.0.0.1:9/ --hapi-session-id ${sessionId}`, name: 'hapi' }, + { pid: 12, cmd: `hapi mcp --url http://127.0.0.1:9/ --hapi-session-id other`, name: 'hapi' }, + ], + sessionId + ) + expect(pids.sort((a, b) => a - b)).toEqual([10, 11]) + }) + it('treats empty Windows process-list stdout as scan_failed (not no-orphans)', async () => { // listWindowsProcessesWithCommandLine throws on empty stdout; catch → // scan_failed so archive does not fail-open as already_gone (#1911). diff --git a/cli/src/runner/orphanReap.ts b/cli/src/runner/orphanReap.ts index 831f9f2f..d4d5d690 100644 --- a/cli/src/runner/orphanReap.ts +++ b/cli/src/runner/orphanReap.ts @@ -7,6 +7,9 @@ * the hub archives by HAPI session id — matching `--started-by runner` plus * an explicit `--existing-session-id` / `--hapi-session-id` flag. * + * Also matches HAPI MCP stdio bridge helpers (`mcp --url … --hapi-session-id`) + * which the agent spawns without `--started-by runner` (#2003). + * * Windows: do not use ps-list. Its fastlist vendor binary is often missing from * single-exe bundles and never returns CommandLine — argv matching needs CIM. */ @@ -45,6 +48,16 @@ export function isHapiDriverCliCommand(cmd: string, name = ''): boolean { ) } +function commandHasHapiSessionIdFlag(cmd: string, sessionId: string): boolean { + if (!sessionId) return false + const escaped = sessionId.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + // Require an explicit HAPI id flag. Do not match bare `--resume ` + // — that is the agent session, not the HAPI row id. + return new RegExp( + `(?:^|\\s)(?:--existing-session-id|--hapi-session-id)(?:\\s+|=)${escaped}(?:\\s|$)` + ).test(cmd) +} + /** * Token-aware match: session id must appear as its own argv token (or after * `--existing-session-id=` / `--hapi-session-id=`), and the process must claim @@ -53,13 +66,21 @@ export function isHapiDriverCliCommand(cmd: string, name = ''): boolean { export function commandMatchesRunnerSpawnedSession(cmd: string, sessionId: string): boolean { if (!sessionId || !cmd.includes('--started-by')) return false if (!/(?:^|\s)--started-by(?:\s+|=)runner(?:\s|$)/.test(cmd)) return false + return commandHasHapiSessionIdFlag(cmd, sessionId) +} - const escaped = sessionId.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') - // Require an explicit HAPI id flag. Do not match bare `--resume ` - // — that is the agent session, not the HAPI row id. - return new RegExp( - `(?:^|\\s)(?:--existing-session-id|--hapi-session-id)(?:\\s+|=)${escaped}(?:\\s|$)` - ).test(cmd) +/** + * HAPI MCP stdio bridge helpers (`hapi … mcp --url …`) are spawned by the agent, + * not the runner — they never carry `--started-by runner`. After #1911 roots are + * reaped, these helpers often survive (OOM of the root only, KillMode=process + * races) and keep holding RAM. Match them by stamped `--hapi-session-id` (#2003). + */ +export function commandMatchesHapiMcpBridgeForSession(cmd: string, sessionId: string): boolean { + if (!sessionId) return false + // Subcommand token `mcp` near a `--url` flag (bridge signature). + if (!/(?:^|\s)mcp(?:\s|$)/.test(cmd)) return false + if (!/(?:^|\s)--url(?:\s+|=)/.test(cmd)) return false + return commandHasHapiSessionIdFlag(cmd, sessionId) } export function selectOrphanTargetsForSession( @@ -77,7 +98,12 @@ export function selectOrphanTargetsForSession( const cmd = proc.cmd || '' const name = proc.name || '' if (!isHapiDriverCliCommand(cmd, name)) continue - if (!commandMatchesRunnerSpawnedSession(cmd, sessionId)) continue + if ( + !commandMatchesRunnerSpawnedSession(cmd, sessionId) + && !commandMatchesHapiMcpBridgeForSession(cmd, sessionId) + ) { + continue + } targets.push({ pid, startMarker: proc.startMarker === undefined ? null : proc.startMarker,