From 5b5d37f211fca3030687bcea403d14f8fd97e425 Mon Sep 17 00:00:00 2001 From: itisbryan <55928614+itisbryan@users.noreply.github.com> Date: Thu, 16 Jul 2026 11:54:15 +0700 Subject: [PATCH] fix(pi): suppress subagent agent_end so nested sessions don't fire false notifications (#8545) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 * fix(pi): gate nested status hooks at registration --------- Co-authored-by: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Co-authored-by: Orca --- .../pi/agent-status-extension-source.test.ts | 75 +++++++++++++++++-- src/main/pi/agent-status-handler-source.ts | 6 ++ 2 files changed, 76 insertions(+), 5 deletions(-) diff --git a/src/main/pi/agent-status-extension-source.test.ts b/src/main/pi/agent-status-extension-source.test.ts index 47ac259f1..1dff2eccc 100644 --- a/src/main/pi/agent-status-extension-source.test.ts +++ b/src/main/pi/agent-status-extension-source.test.ts @@ -24,7 +24,11 @@ type Harness = { readFileSync: ReturnType } handlers: Record + processEnv: Record callHook: (name: string, event?: unknown) => Promise + // 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 +// 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 + 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 = {} - register({ - on(name: string, handler: HookHandler) { - handlers[name] = handler - } - }) + const registerInto = (target: Record): 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', diff --git a/src/main/pi/agent-status-handler-source.ts b/src/main/pi/agent-status-handler-source.ts index 625e89df7..bc002cf57 100644 --- a/src/main/pi/agent-status-handler-source.ts +++ b/src/main/pi/agent-status-handler-source.ts @@ -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 ?? '' })", ' })',