fix(pi): suppress subagent agent_end so nested sessions don't fire false notifications (#8545)
* fix(pi): suppress subagent agent_end so nested sessions don't fire false notifications Pi/OMP run subagents as nested child processes that inherit the lead pane's env (including ORCA_PANE_KEY), each loading its own copy of the orca-agent-status.ts extension. A child's agent_end was attributed to the lead pane, firing a false "agent finished" notification on every subagent completion. Gate status reporting to the top-level Pi process per pane via an ORCA_PI_STATUS_OWNED env marker: the first process claims the pane and sets the marker; any Pi it spawns inherits it and stays silent. Keyed on process nesting (not hasUI) so a top-level non-interactive `pi -p` run still reports its own completion. * test(pi): assert nested-subagent silence for every guarded event, not just agent_end * fix(pi): key status owner on pid so extension reload doesn't silence the lead The subagent-suppression gate marked the pane owner with a boolean ORCA_PI_STATUS_OWNED='1'. Pi re-invokes an extension's default export on every in-process reload (/reload, live edit, settings reload), so the lead process re-ran the factory, read the '1' it had set on first load, and treated itself as a nested subagent — silencing all its own status (working, tools, done, notifications) for the rest of the session. Record the owning process's pid instead. A same-process reload matches its own pid and keeps reporting; a spawned subagent inherits the lead's pid, sees it differs from its own, and stays silent. Startup was unaffected (bindExtensions runs the factory once), which is why the single-run trace missed it. Add a reload regression test that re-invokes the factory in the same process and asserts the lead still reports; it fails against the boolean marker. Co-authored-by: Orca <help@stably.ai> * fix(pi): gate nested status hooks at registration --------- Co-authored-by: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
parent
f62ad69260
commit
5b5d37f211
|
|
@ -24,7 +24,11 @@ type Harness = {
|
|||
readFileSync: ReturnType<typeof vi.fn>
|
||||
}
|
||||
handlers: Record<string, HookHandler>
|
||||
processEnv: Record<string, string | undefined>
|
||||
callHook: (name: string, event?: unknown) => Promise<void>
|
||||
// Re-invoke the extension factory in the same process (as Pi does on an
|
||||
// in-process extension reload), swapping in the freshly registered handlers.
|
||||
reload: () => void
|
||||
}
|
||||
|
||||
const BASE_ENV = {
|
||||
|
|
@ -38,9 +42,14 @@ const BASE_ENV = {
|
|||
ORCA_AGENT_HOOK_VERSION: '1.2.3'
|
||||
} satisfies Record<string, string>
|
||||
|
||||
// Why: ownership keys on process.pid, so reload and child-process tests need
|
||||
// stable, distinct identities.
|
||||
const SELF_PID = 4242
|
||||
|
||||
function createHarness(args: {
|
||||
kind: 'pi' | 'omp'
|
||||
env?: Record<string, string | undefined>
|
||||
pid?: number
|
||||
title?: string
|
||||
argv?: string[]
|
||||
existsSync?: (path: string) => boolean
|
||||
|
|
@ -95,6 +104,7 @@ function createHarness(args: {
|
|||
...BASE_ENV,
|
||||
...args.env
|
||||
},
|
||||
pid: args.pid ?? SELF_PID,
|
||||
title: args.title ?? 'node',
|
||||
argv: args.argv ?? ['node', '/usr/bin/orca']
|
||||
}
|
||||
|
|
@ -134,11 +144,14 @@ function createHarness(args: {
|
|||
}
|
||||
|
||||
const handlers: Record<string, HookHandler> = {}
|
||||
register({
|
||||
on(name: string, handler: HookHandler) {
|
||||
handlers[name] = handler
|
||||
}
|
||||
})
|
||||
const registerInto = (target: Record<string, HookHandler>): void => {
|
||||
register({
|
||||
on(name: string, handler: HookHandler) {
|
||||
target[name] = handler
|
||||
}
|
||||
})
|
||||
}
|
||||
registerInto(handlers)
|
||||
|
||||
return {
|
||||
fetchMock,
|
||||
|
|
@ -146,8 +159,15 @@ function createHarness(args: {
|
|||
spawnedChildren,
|
||||
fsMock,
|
||||
handlers,
|
||||
processEnv: processMock.env,
|
||||
callHook: async (name, event) => {
|
||||
await handlers[name]?.(event)
|
||||
},
|
||||
reload: () => {
|
||||
for (const key of Object.keys(handlers)) {
|
||||
delete handlers[key]
|
||||
}
|
||||
registerInto(handlers)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -167,6 +187,51 @@ describe('getPiAgentStatusExtensionSource', () => {
|
|||
expect(harness.spawnMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it.each(['pi', 'omp'] as const)(
|
||||
'registers no status handlers for a nested %s subagent process',
|
||||
(kind) => {
|
||||
// Why: inheriting the lead's owner PID must disable the extension as a
|
||||
// whole, so future hook additions cannot reopen the notification leak.
|
||||
const lead = createHarness({ kind, pid: SELF_PID })
|
||||
const child = createHarness({ kind, pid: SELF_PID + 1, env: lead.processEnv })
|
||||
const grandchild = createHarness({ kind, pid: SELF_PID + 2, env: child.processEnv })
|
||||
|
||||
expect(child.handlers).toEqual({})
|
||||
expect(grandchild.handlers).toEqual({})
|
||||
expect(child.processEnv.ORCA_PI_STATUS_OWNED).toBe(String(SELF_PID))
|
||||
expect(grandchild.processEnv.ORCA_PI_STATUS_OWNED).toBe(String(SELF_PID))
|
||||
expect(child.fetchMock).not.toHaveBeenCalled()
|
||||
expect(child.spawnMock).not.toHaveBeenCalled()
|
||||
}
|
||||
)
|
||||
|
||||
it('reports agent_end for a top-level run (including non-interactive) and claims the pane by pid', async () => {
|
||||
// Why: non-interactive top-level runs still own their pane and must report.
|
||||
const harness = createHarness({ kind: 'pi', pid: SELF_PID, argv: ['node', 'pi', '-p'] })
|
||||
|
||||
await harness.callHook('agent_end')
|
||||
|
||||
expect(harness.fetchMock).toHaveBeenCalledTimes(1)
|
||||
const body = JSON.parse(String(harness.fetchMock.mock.calls[0]?.[1]?.body))
|
||||
expect(body.payload).toEqual({ hook_event_name: 'agent_end' })
|
||||
expect(harness.processEnv.ORCA_PI_STATUS_OWNED).toBe(String(SELF_PID))
|
||||
})
|
||||
|
||||
it('keeps reporting after the lead re-runs the extension factory on reload', async () => {
|
||||
// Why: Pi reloads extensions in-process, so the lead must recognize its PID
|
||||
// instead of mistaking its own marker for a nested child.
|
||||
const harness = createHarness({ kind: 'pi', pid: SELF_PID })
|
||||
|
||||
expect(harness.processEnv.ORCA_PI_STATUS_OWNED).toBe(String(SELF_PID))
|
||||
|
||||
harness.reload()
|
||||
await harness.callHook('agent_end')
|
||||
|
||||
expect(harness.fetchMock).toHaveBeenCalledTimes(1)
|
||||
const body = JSON.parse(String(harness.fetchMock.mock.calls[0]?.[1]?.body))
|
||||
expect(body.payload).toEqual({ hook_event_name: 'agent_end' })
|
||||
})
|
||||
|
||||
it('keeps native fetch as the only path even when the runtime looks like WSL', async () => {
|
||||
const harness = createHarness({
|
||||
kind: 'omp',
|
||||
|
|
|
|||
|
|
@ -29,7 +29,13 @@ export function getPiAgentStatusHandlerSourceLines(): string[] {
|
|||
'// etc.), so we forward the raw object verbatim under the same field',
|
||||
'// names Claude uses (tool_name / tool_input) and let the server pick the',
|
||||
'// preview. Keeps tool-name knowledge centralized on the receiver side.',
|
||||
"// Why: child agents inherit the lead's pane env; only its process may",
|
||||
'// register status hooks. PID identity keeps in-process reloads reporting.',
|
||||
'export default function (pi): void {',
|
||||
' const ownerPid = process.env.ORCA_PI_STATUS_OWNED',
|
||||
' const selfPid = String(process.pid)',
|
||||
' if (ownerPid && ownerPid !== selfPid) return',
|
||||
' process.env.ORCA_PI_STATUS_OWNED = selfPid',
|
||||
" pi.on('before_agent_start', (event) => {",
|
||||
" post('before_agent_start', { prompt: event.prompt ?? '' })",
|
||||
' })',
|
||||
|
|
|
|||
Loading…
Reference in New Issue