diff --git a/src/main/agent-hooks/server-agent-presence.test.ts b/src/main/agent-hooks/server-agent-presence.test.ts index 0d159fad3a50..250d188e3d74 100644 --- a/src/main/agent-hooks/server-agent-presence.test.ts +++ b/src/main/agent-hooks/server-agent-presence.test.ts @@ -366,8 +366,10 @@ describe('host-owned hook presence', () => { expect(probe).not.toHaveBeenCalled() expect(visible(server)).toBe(true) await hook(server, 'SessionStart', 'relaunch', undefined, 4002) - await vi.waitFor(() => expect(probe).toHaveBeenCalledOnce()) - await vi.waitFor(() => expect(visible(server)).toBe(false)) + expect(probe).toHaveBeenCalledOnce() + expect( + server.getStatusSnapshot().find((row) => row.paneKey === PANE)?.providerSession?.id + ).toBe('relaunch') await hook(server, 'UserPromptSubmit', 'relaunch', undefined, 4002) expect(state(server)).toBe('working') }) @@ -505,9 +507,11 @@ describe('retained agent exit', () => { finish = resolve }) ) - await hook(server, 'SessionStart', 'replacement', undefined, 4002) + const replacement = hook(server, 'SessionStart', 'replacement', undefined, 4002) + await vi.waitFor(() => expect(probe).toHaveBeenCalledOnce()) const overlapping = server.checkAgentPresenceSince(PANE, attachedAt) finish('exited') + await replacement expect(await overlapping).not.toBe('exited') }) diff --git a/src/main/agent-hooks/server-claude-owed-notification-expiry.test.ts b/src/main/agent-hooks/server-claude-owed-notification-expiry.test.ts index ce86479d2b5f..339d52d0c292 100644 --- a/src/main/agent-hooks/server-claude-owed-notification-expiry.test.ts +++ b/src/main/agent-hooks/server-claude-owed-notification-expiry.test.ts @@ -9,6 +9,10 @@ vi.mock('../telemetry/client', () => ({ track: vi.fn() })) vi.mock('../telemetry/cohort-classifier', () => ({ getCohortAtEmit: vi.fn(() => ({ nth_repo_added: 2 })) })) +// The fixture's owner process is fictional; the pane's owning Claude is still running. +vi.mock('../../shared/agent-process-presence-probe', () => ({ + probeAgentProcessPresence: vi.fn(async () => 'live') +})) beforeEach(() => { _internals.resetCachesForTests() diff --git a/src/main/agent-hooks/server-nested-same-provider-owner.test.ts b/src/main/agent-hooks/server-nested-same-provider-owner.test.ts new file mode 100644 index 000000000000..5c39becf1b52 --- /dev/null +++ b/src/main/agent-hooks/server-nested-same-provider-owner.test.ts @@ -0,0 +1,80 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { AgentHookServer } from './server' +import { buildBody, PANE, postHookEvent } from './server.test-fixtures' +import type { probeAgentProcessPresence } from '../../shared/agent-process-presence-probe' + +vi.mock('../telemetry/client', () => ({ track: vi.fn() })) +vi.mock('../telemetry/cohort-classifier', () => ({ getCohortAtEmit: () => ({}) })) +const probe = vi.hoisted(() => vi.fn(async () => 'live')) +vi.mock('../../shared/agent-process-presence-probe', () => ({ probeAgentProcessPresence: probe })) +const servers: AgentHookServer[] = [] +const parent = { pid: 4001, platform: 'linux', startTime: 'birth-4001' } +const child = { pid: 4002, platform: 'linux', startTime: 'birth-4002' } + +afterEach(() => { + for (const server of servers.splice(0)) { + server.stop() + } + probe.mockReset().mockResolvedValue('live') +}) + +async function claudeHook( + server: AgentHookServer, + process: typeof parent, + payload: Record +): Promise { + await postHookEvent(server, buildBody(payload, { agentProcess: JSON.stringify(process) })) +} + +async function parentWorking(): Promise { + const server = new AgentHookServer() + servers.push(server) + await server.start({ env: 'production' }) + await claudeHook(server, parent, { + hook_event_name: 'UserPromptSubmit', + session_id: 'claude-parent', + prompt: 'Parent task' + }) + return server +} + +describe('a second Claude process in the same pane', () => { + it('does not replace the live owner row when a nested claude -p fails', async () => { + const server = await parentWorking() + const emitted = vi.fn() + server.setListener(emitted) + await claudeHook(server, child, { + hook_event_name: 'UserPromptSubmit', + session_id: 'claude-child', + prompt: 'Child task' + }) + await claudeHook(server, child, { + hook_event_name: 'StopFailure', + session_id: 'claude-child', + error: 'rate_limit' + }) + const row = server.getStatusSnapshot().find((entry) => entry.paneKey === PANE) + expect(row).toMatchObject({ + state: 'working', + prompt: 'Parent task', + providerSession: { id: 'claude-parent' } + }) + expect(emitted.mock.calls.some(([event]) => event.payload.state === 'done')).toBe(false) + }) + + it('lets a new Claude take the pane once the old owner process exited', async () => { + const server = await parentWorking() + probe.mockResolvedValue('exited') + await claudeHook(server, child, { + hook_event_name: 'UserPromptSubmit', + session_id: 'claude-next', + prompt: 'Next task' + }) + const row = server.getStatusSnapshot().find((entry) => entry.paneKey === PANE) + expect(row).toMatchObject({ + state: 'working', + prompt: 'Next task', + providerSession: { id: 'claude-next' } + }) + }) +}) diff --git a/src/main/agent-hooks/server/server-status-update.ts b/src/main/agent-hooks/server/server-status-update.ts index 614846a2bd74..f4c994f81dc4 100644 --- a/src/main/agent-hooks/server/server-status-update.ts +++ b/src/main/agent-hooks/server/server-status-update.ts @@ -120,6 +120,8 @@ export abstract class AgentHookServerStatusUpdate extends AgentHookServerStatusA } : undefined, incoming: terminalOwnedPayload.payload.agentType, + // Why local only: a relayed row carries its execution host's owner verdict, not a sender. + incomingProcess: payload.connectionId ? undefined : incoming.agentPresence?.process, now }) if ( diff --git a/src/shared/agent-hook-listener.ts b/src/shared/agent-hook-listener.ts index a067e24e4b11..45c94a0674d7 100644 --- a/src/shared/agent-hook-listener.ts +++ b/src/shared/agent-hook-listener.ts @@ -1,4 +1,4 @@ -import { readAgentProcessIdentity } from './agent-process-presence' +import { readHookAgentProcess } from './agent-process-presence' import { resolveAgentStatusIdentity } from './agent-status-identity' import { sameAgentStatusTerminalOwner } from './agent-status-terminal-owner' import { normalizeAgentStatusPayload, type AgentMainAgentStatus } from './agent-status-types' @@ -154,6 +154,7 @@ export function normalizeHookPayload( return null } const previousStatus = state.lastStatusByPaneKey.get(paneKey) + const agentProcess = readHookAgentProcess(source, record.agentProcess) const previousReceivedAt = previousStatus && 'receivedAt' in previousStatus && @@ -177,6 +178,7 @@ export function normalizeHookPayload( agentPresence: previousStatus.providerSessionOnly ? undefined : previousStatus.agentPresence }, incoming: source, + incomingProcess: agentProcess, now: Date.now() }).inheritedFromActivePane ) { @@ -222,9 +224,6 @@ export function normalizeHookPayload( } } - // Why: presence needs the agent's own process; without it the hook cannot speak for liveness. - const agentProcess = - source === 'claude' ? readAgentProcessIdentity(record.agentProcess) : undefined const agentPresence = agentProcess ? { agent: source, process: agentProcess, sessionOwner: true as const } : undefined diff --git a/src/shared/agent-hook-owner-presence.ts b/src/shared/agent-hook-owner-presence.ts index 64c7aaebf9b4..a2892a22af40 100644 --- a/src/shared/agent-hook-owner-presence.ts +++ b/src/shared/agent-hook-owner-presence.ts @@ -1,34 +1,76 @@ import { parseHookEnvelope } from './agent-hook-listener/hook-envelope' import type { HookListenerState } from './agent-hook-listener/listener-state' import type { AgentHookSource } from './agent-hook-relay' -import { hasOtherAgentProcessOwner } from './agent-process-presence' +import { + hasOtherAgentProcessOwner, + readHookAgentProcess, + type AgentProcessIdentity, + type AgentProcessPresence +} from './agent-process-presence' import { sameAgentStatusTerminalOwner } from './agent-status-terminal-owner' -/** Confirm an old owner's exit on its execution host before accepting a replacement. */ -export function checkHookOwnerPresence( +function readHookPaneOwner( state: HookListenerState, source: AgentHookSource, body: unknown, - env: string, - check: (paneKey: string) => Promise -): Promise | undefined { + env: string +): + | { + paneKey: string + owner: AgentProcessPresence | undefined + sender: { agent: AgentHookSource; process?: AgentProcessIdentity } + } + | undefined { const envelope = parseHookEnvelope(state, source, body, env) if (!envelope) { - return + return undefined } const previous = state.lastStatusByPaneKey.get(envelope.paneKey) - // A suspended owner may have resumed without emitting another hook. - const presence = previous?.agentPresence if ( - previous && - !previous.providerSessionOnly && - sameAgentStatusTerminalOwner( + !previous || + previous.providerSessionOnly || + !sameAgentStatusTerminalOwner( { connectionId: previous.connectionId ?? null, worktreeId: previous.worktreeId }, { connectionId: null, worktreeId: envelope.worktreeId } - ) && - hasOtherAgentProcessOwner(presence && { ...presence, suspended: undefined }, source) + ) + ) { + return undefined + } + return { + paneKey: envelope.paneKey, + owner: previous.agentPresence, + sender: { agent: source, process: readHookAgentProcess(source, envelope.record.agentProcess) } + } +} + +/** Confirm an old owner's exit on its execution host before accepting a replacement. */ +export function checkHookOwnerPresence( + state: HookListenerState, + source: AgentHookSource, + body: unknown, + env: string, + check: (paneKey: string) => Promise +): Promise | undefined { + const hook = readHookPaneOwner(state, source, body, env) + // A suspended owner may have resumed without emitting another hook. + if ( + hook && + hasOtherAgentProcessOwner(hook.owner && { ...hook.owner, suspended: undefined }, hook.sender) ) { - return check(envelope.paneKey) + return check(hook.paneKey) } return undefined } + +/** Another live process of the owner's provider posted this hook, e.g. a nested `claude -p`. */ +export function isOwnerProviderGuestHook( + state: HookListenerState, + source: AgentHookSource, + body: unknown, + env: string +): boolean { + const hook = readHookPaneOwner(state, source, body, env) + return Boolean( + hook && hook.owner?.agent === source && hasOtherAgentProcessOwner(hook.owner, hook.sender) + ) +} diff --git a/src/shared/agent-process-presence.ts b/src/shared/agent-process-presence.ts index d9e522a17a0f..bf6098d97a61 100644 --- a/src/shared/agent-process-presence.ts +++ b/src/shared/agent-process-presence.ts @@ -50,6 +50,14 @@ export function readAgentProcessIdentity(value: unknown): AgentProcessIdentity | return { pid, platform, startTime } } +// Why: only Claude's hook command captures its agent's process; other hooks cannot speak for liveness. +export function readHookAgentProcess( + source: string, + value: unknown +): AgentProcessIdentity | undefined { + return source === 'claude' ? readAgentProcessIdentity(value) : undefined +} + export function readAgentProcessPresence(value: unknown): AgentProcessPresence | undefined { if ( !value || @@ -77,16 +85,18 @@ export function isSameAgentProcess(a: AgentProcessIdentity, b: AgentProcessIdent return a.pid === b.pid && a.platform === b.platform && a.startTime === b.startTime } +/** A hook without a process identity can only be told apart from the owner by its provider. */ export function hasOtherAgentProcessOwner( presence: AgentProcessPresence | undefined, - incoming: string + incoming: { agent: string; process?: AgentProcessIdentity } ): boolean { return Boolean( presence?.process && presence.sessionOwner && !presence.ended && !presence.suspended && - presence.agent !== incoming && - presence.agent !== 'unknown' + presence.agent !== 'unknown' && + (presence.agent !== incoming.agent || + (incoming.process && !isSameAgentProcess(presence.process, incoming.process))) ) } diff --git a/src/shared/agent-status-identity.ts b/src/shared/agent-status-identity.ts index b9ca92a89592..658b055dd9b4 100644 --- a/src/shared/agent-status-identity.ts +++ b/src/shared/agent-status-identity.ts @@ -4,7 +4,11 @@ import { type AgentStatusState, type AgentType } from './agent-status-types' -import { hasOtherAgentProcessOwner, type AgentProcessPresence } from './agent-process-presence' +import { + hasOtherAgentProcessOwner, + type AgentProcessIdentity, + type AgentProcessPresence +} from './agent-process-presence' import { resolveCanonicalPaneAgentIdentity } from './pane-agent-identity-adapter' import type { TuiAgent } from './tui-agent' @@ -46,6 +50,7 @@ function isActiveExistingIdentity( export function resolveAgentStatusIdentity(args: { existing?: ExistingAgentIdentity incoming?: AgentType + incomingProcess?: AgentProcessIdentity now: number staleAfterMs?: number }): AgentIdentityResolution { @@ -59,12 +64,23 @@ export function resolveAgentStatusIdentity(args: { inheritedFromActivePane: false } } - if (!args.existing || !existingAgentType || existingAgentType === incomingAgentType) { + if (!args.existing || !existingAgentType) { return { agentType: incomingAgentType, inheritedFromActivePane: false } } + const otherProcessOwner = hasOtherAgentProcessOwner(args.existing.agentPresence, { + agent: incomingAgentType, + process: args.incomingProcess + }) + if (existingAgentType === incomingAgentType) { + return { + agentType: incomingAgentType, + // Why: a nested `claude -p` shares the pane key; only the owner's own process speaks for it. + inheritedFromActivePane: otherProcessOwner + } + } const canonical = resolveCanonicalPaneAgentIdentity({ hookAgent: incomingAgentType as TuiAgent, hookIsLive: true, @@ -72,8 +88,7 @@ export function resolveAgentStatusIdentity(args: { }) if ( !args.existing.agentPresence?.suspended && - (hasOtherAgentProcessOwner(args.existing.agentPresence, incomingAgentType) || - isActiveExistingIdentity(args.existing, args.now, staleAfterMs)) + (otherProcessOwner || isActiveExistingIdentity(args.existing, args.now, staleAfterMs)) ) { return { // Why: child agent CLIs inherit ORCA_PANE_KEY from their parent terminal. diff --git a/src/wsl-guest/agent-hook-event-admission.ts b/src/wsl-guest/agent-hook-event-admission.ts index 27f95e8ad583..13042425c375 100644 --- a/src/wsl-guest/agent-hook-event-admission.ts +++ b/src/wsl-guest/agent-hook-event-admission.ts @@ -42,7 +42,10 @@ export function applyRelayHookEvent( previous && !previous.providerSessionOnly && sameAgentStatusTerminalOwner(previous, incoming) && - hasOtherAgentProcessOwner(previous.agentPresence, incoming.payload.agentType ?? 'unknown') + hasOtherAgentProcessOwner(previous.agentPresence, { + agent: incoming.payload.agentType ?? 'unknown', + process: incoming.agentPresence?.process + }) ) { return undefined } diff --git a/src/wsl-guest/agent-hook-provider-normalization.ts b/src/wsl-guest/agent-hook-provider-normalization.ts index dfc913323e2c..e366a2ba214f 100644 --- a/src/wsl-guest/agent-hook-provider-normalization.ts +++ b/src/wsl-guest/agent-hook-provider-normalization.ts @@ -1,4 +1,5 @@ import { hasOtherAgentProcessOwner } from '../shared/agent-process-presence' +import { isOwnerProviderGuestHook } from '../shared/agent-hook-owner-presence' import { sameAgentStatusTerminalOwner } from '../shared/agent-status-terminal-owner' import { normalizeHookPayload } from '../shared/agent-hook-listener' import { @@ -22,6 +23,10 @@ export class RelayProviderHookNormalization { env: string, options: Parameters[4] = {} ): AgentHookEventPayload | null { + // Why: the owner's own provider folds would take the guest's prompt and tools. + if (isOwnerProviderGuestHook(canonical, source, body, env)) { + return null + } let state = this.states.get(source) if (!state) { state = createHookListenerState() @@ -45,7 +50,10 @@ export class RelayProviderHookNormalization { owner && !owner.providerSessionOnly && sameAgentStatusTerminalOwner(owner, event) && - hasOtherAgentProcessOwner(owner.agentPresence, source) + hasOtherAgentProcessOwner(owner.agentPresence, { + agent: source, + process: event.agentPresence?.process + }) ) { return event } diff --git a/src/wsl-guest/relay-nested-same-provider-owner.test.ts b/src/wsl-guest/relay-nested-same-provider-owner.test.ts new file mode 100644 index 000000000000..d06c4b9e32ac --- /dev/null +++ b/src/wsl-guest/relay-nested-same-provider-owner.test.ts @@ -0,0 +1,68 @@ +import { afterEach, expect, it, vi } from 'vitest' +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { RelayAgentHookServer } from './agent-hook-server' +import type { AgentHookRelayEnvelope } from '../shared/agent-hook-relay' +import { makePaneKey } from '../shared/stable-pane-id' +import type { probeAgentProcessPresence } from '../shared/agent-process-presence-probe' + +const probe = vi.hoisted(() => vi.fn(async () => 'live')) +vi.mock('../shared/agent-process-presence-probe', () => ({ probeAgentProcessPresence: probe })) +afterEach(() => probe.mockReset().mockResolvedValue('live')) + +it('keeps a nested claude -p out of the owner row, then admits a Claude after the owner exits', async () => { + const dir = await mkdtemp(join(tmpdir(), 'relay-nested-same-provider-')) + const forward = vi.fn<(event: AgentHookRelayEnvelope) => void>() + const server = new RelayAgentHookServer({ endpointDir: dir, forward }) + const paneKey = makePaneKey('tab-1', '11111111-1111-4111-8111-111111111111') + try { + await server.start() + const { port, token } = server.getCoordinates() + const post = async (pid: number, payload: Record) => { + const response = await fetch(`http://127.0.0.1:${port}/hook/claude`, { + method: 'POST', + headers: { 'Content-Type': 'application/json', 'X-Orca-Agent-Hook-Token': token }, + body: JSON.stringify({ + paneKey, + tabId: 'tab-1', + worktreeId: 'wt-1', + agentProcess: { pid, platform: 'linux', startTime: `birth-${pid}` }, + payload + }) + }) + expect(response.status).toBe(204) + } + await post(4001, { + hook_event_name: 'UserPromptSubmit', + session_id: 'claude-parent', + prompt: 'Parent task' + }) + forward.mockClear() + await post(4002, { + hook_event_name: 'UserPromptSubmit', + session_id: 'claude-child', + prompt: 'Child task' + }) + await post(4002, { hook_event_name: 'StopFailure', session_id: 'claude-child' }) + expect(forward).not.toHaveBeenCalled() + await post(4001, { hook_event_name: 'Stop', session_id: 'claude-parent' }) + expect(forward.mock.lastCall?.[0]).toMatchObject({ + providerSession: { id: 'claude-parent' }, + payload: { state: 'done', prompt: 'Parent task' } + }) + probe.mockResolvedValue('exited') + await post(4003, { + hook_event_name: 'UserPromptSubmit', + session_id: 'claude-next', + prompt: 'Next task' + }) + expect(forward.mock.lastCall?.[0]).toMatchObject({ + providerSession: { id: 'claude-next' }, + payload: { state: 'working', prompt: 'Next task' } + }) + } finally { + server.stop() + await rm(dir, { recursive: true, force: true }) + } +})