diff --git a/src/main/agent-hooks/agent-session-pane-bindings.test.ts b/src/main/agent-hooks/agent-session-pane-bindings.test.ts new file mode 100644 index 000000000000..fa70d8923250 --- /dev/null +++ b/src/main/agent-hooks/agent-session-pane-bindings.test.ts @@ -0,0 +1,66 @@ +import { describe, expect, it } from 'vitest' + +import { AgentSessionPaneBindings } from './agent-session-pane-bindings' +import { makePaneKey } from '../../shared/stable-pane-id' + +const PANE_A = makePaneKey('tab-1', 'aaaaaaaa-1111-4111-8111-111111111111') +const PANE_B = makePaneKey('tab-1', 'bbbbbbbb-2222-4222-8222-222222222222') + +describe('AgentSessionPaneBindings', () => { + it('resolves only the source it was bound under', () => { + const bindings = new AgentSessionPaneBindings() + bindings.bind('claude', 'sess-1', { paneKey: PANE_A, ptyId: 'pty-1' }) + + expect(bindings.resolve('claude', 'sess-1')?.paneKey).toBe(PANE_A) + expect(bindings.resolve('codex', 'sess-1')).toBeNull() + expect(bindings.resolve('claude', 'sess-2')).toBeNull() + expect(bindings.resolve('claude', null)).toBeNull() + }) + + it('follows a session that re-binds to another pane', () => { + const bindings = new AgentSessionPaneBindings() + bindings.bind('claude', 'sess-1', { paneKey: PANE_A, ptyId: 'pty-1' }) + bindings.bind('claude', 'sess-1', { paneKey: PANE_B, ptyId: 'pty-2' }) + + expect(bindings.resolve('claude', 'sess-1')?.paneKey).toBe(PANE_B) + expect(bindings.size()).toBe(1) + }) + + it('drops every binding a dying PTY established, and only those', () => { + const bindings = new AgentSessionPaneBindings() + bindings.bind('claude', 'sess-1', { paneKey: PANE_A, ptyId: 'pty-1' }) + bindings.bind('claude', 'sess-2', { paneKey: PANE_A, ptyId: 'pty-1' }) + bindings.bind('claude', 'sess-3', { paneKey: PANE_B, ptyId: 'pty-2' }) + + bindings.clearForPty('pty-1') + + expect(bindings.resolve('claude', 'sess-1')).toBeNull() + expect(bindings.resolve('claude', 'sess-2')).toBeNull() + expect(bindings.resolve('claude', 'sess-3')?.paneKey).toBe(PANE_B) + }) + + it('evicts least-recently-bound sessions past the cap, keeping a re-bound one alive', () => { + const bindings = new AgentSessionPaneBindings() + bindings.bind('claude', 'sess-0', { paneKey: PANE_A, ptyId: 'pty-0' }) + for (let i = 1; i < 512; i += 1) { + bindings.bind('claude', `sess-${i}`, { paneKey: PANE_B, ptyId: `pty-${i}` }) + } + // Touch the oldest so it is no longer the eviction candidate. + bindings.bind('claude', 'sess-0', { paneKey: PANE_A, ptyId: 'pty-0' }) + bindings.bind('claude', 'sess-512', { paneKey: PANE_B, ptyId: 'pty-512' }) + + expect(bindings.size()).toBe(512) + expect(bindings.resolve('claude', 'sess-0')?.paneKey).toBe(PANE_A) + expect(bindings.resolve('claude', 'sess-1')).toBeNull() + expect(bindings.resolve('claude', 'sess-512')?.paneKey).toBe(PANE_B) + }) + + it('ignores incomplete bindings rather than storing an unroutable row', () => { + const bindings = new AgentSessionPaneBindings() + bindings.bind('claude', ' ', { paneKey: PANE_A, ptyId: 'pty-1' }) + bindings.bind('claude', 'sess-1', { paneKey: '', ptyId: 'pty-1' }) + bindings.bind('claude', 'sess-2', { paneKey: PANE_A, ptyId: '' }) + + expect(bindings.size()).toBe(0) + }) +}) diff --git a/src/main/agent-hooks/agent-session-pane-bindings.ts b/src/main/agent-hooks/agent-session-pane-bindings.ts new file mode 100644 index 000000000000..df26b0a1a11b --- /dev/null +++ b/src/main/agent-hooks/agent-session-pane-bindings.ts @@ -0,0 +1,84 @@ +import type { AgentHookSource } from '../../shared/agent-hook-relay' + +export type AgentSessionPaneBinding = { + paneKey: string + /** The PTY whose spawn established this binding; the binding dies with it. */ + ptyId: string + /** Why carried: the inherited env names the DAEMON's workspace too, so a + * corrected event would otherwise keep filing itself under the wrong one. */ + worktreeId?: string +} + +// Why: one entry per pinned session. Generous headroom over any realistic live +// count while still bounding a caller that leaks registrations. +const MAX_BINDINGS = 512 + +/** + * Maps a provider session id to the pane Orca spawned it into. + * + * Why this layer exists: a pane's identity reaches an agent's hooks only + * through `ORCA_PANE_KEY` in the process environment, and a CLI that hosts + * sessions inside a shared prewarmed daemon runs its hooks in a worker that + * inherited the DAEMON's env — whichever pane happened to start the daemon, + * possibly days ago in another workspace. The posted pane key is then simply + * wrong, and the hook payload carries no other pane coordinate, so no + * listener-side heuristic can recover the right one. The binding is therefore + * established where Orca still knows both halves — at spawn, where it mints the + * session id and owns the pane — and consulted at ingest. + * + * Session-scoped rather than process-scoped on purpose: the daemon's worker is + * not the pane's PTY and can outlive it, so process ancestry proves nothing. + * Transport-agnostic for the same reason — a remote host's daemon inherits a + * stale key exactly like a local one, and both ingest seams resolve here. + */ +export class AgentSessionPaneBindings { + private bindings = new Map() + + private static key(source: AgentHookSource, sessionId: string): string { + // Why: source-scoped so two CLIs cannot collide on a shared id namespace. + return `${source}\u0000${sessionId}` + } + + bind(source: AgentHookSource, sessionId: string, binding: AgentSessionPaneBinding): void { + const sessionKey = sessionId.trim() + if (!sessionKey || !binding.paneKey || !binding.ptyId) { + return + } + const key = AgentSessionPaneBindings.key(source, sessionKey) + // Why: delete first so re-binding an existing session moves it to the end + // of the insertion order — eviction stays least-recently-bound. + this.bindings.delete(key) + this.bindings.set(key, binding) + while (this.bindings.size > MAX_BINDINGS) { + const oldest = this.bindings.keys().next().value + if (oldest === undefined) { + break + } + this.bindings.delete(oldest) + } + } + + resolve( + source: AgentHookSource, + sessionId: string | null | undefined + ): AgentSessionPaneBinding | null { + if (!sessionId) { + return null + } + return this.bindings.get(AgentSessionPaneBindings.key(source, sessionId.trim())) ?? null + } + + /** Drops every binding a dying PTY established, so a reused id space cannot + * re-route a later session onto a pane that no longer runs it. */ + clearForPty(ptyId: string): void { + for (const [key, binding] of this.bindings) { + if (binding.ptyId === ptyId) { + this.bindings.delete(key) + } + } + } + + size(): number { + return this.bindings.size + } +} diff --git a/src/main/agent-hooks/server-agent-session-pane-attribution.test.ts b/src/main/agent-hooks/server-agent-session-pane-attribution.test.ts new file mode 100644 index 000000000000..b567ad22e596 --- /dev/null +++ b/src/main/agent-hooks/server-agent-session-pane-attribution.test.ts @@ -0,0 +1,247 @@ +import { describe, expect, it, vi } from 'vitest' + +import { AgentHookServer, isValidPaneKey, _internals } from './server' +import { + createHookListenerState, + normalizeHookPayload, + readHookBodyProviderSessionId +} from '../../shared/agent-hook-listener' +import { makePaneKey } from '../../shared/stable-pane-id' + +vi.mock('../telemetry/client', () => ({ track: vi.fn() })) +vi.mock('../telemetry/cohort-classifier', () => ({ getCohortAtEmit: () => ({}) })) + +// Why: split panes live in ONE tab — same tabId, different leaf. The daemon's +// inherited key names a pane in a different workspace entirely. +const SPLIT_LEAF_A = 'aaaaaaaa-1111-4111-8111-111111111111' +const SPLIT_LEAF_B = 'bbbbbbbb-2222-4222-8222-222222222222' +const DAEMON_LEAF = 'cccccccc-3333-4333-8333-333333333333' + +const SPLIT_PANE_A = makePaneKey('tab-split', SPLIT_LEAF_A) +const SPLIT_PANE_B = makePaneKey('tab-split', SPLIT_LEAF_B) +/** The pane that first spawned the shared daemon; every later worker inherits its env. */ +const DAEMON_PANE = makePaneKey('tab-daemon', DAEMON_LEAF) + +const SESSION_A = '0192a4b1-1111-4111-8111-aaaaaaaaaaaa' +const SESSION_B = '0192a4b1-2222-4222-8222-bbbbbbbbbbbb' +const UNPINNED_SESSION = '0192a4b1-9999-4999-8999-999999999999' + +function buildDaemonHostedBody(sessionId: string, prompt: string): Record { + return { + // The daemon worker's env, NOT the pane the user is typing in. + paneKey: DAEMON_PANE, + tabId: 'tab-daemon', + worktreeId: 'wt-daemon', + env: 'production', + payload: { + hook_event_name: 'UserPromptSubmit', + prompt, + session_id: sessionId + } + } +} + +async function withServer( + run: (server: AgentHookServer, post: (body: unknown) => Promise) => Promise +): Promise { + _internals.resetCachesForTests() + const server = new AgentHookServer() + await server.start({ env: 'production' }) + try { + const env = server.buildPtyEnv() + const post = async (body: unknown): Promise => { + const response = await fetch(`http://127.0.0.1:${env.ORCA_AGENT_HOOK_PORT}/hook/claude`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'X-Orca-Agent-Hook-Token': env.ORCA_AGENT_HOOK_TOKEN + }, + body: JSON.stringify(body) + }) + return response.status + } + await run(server, post) + } finally { + server.stop() + } +} + +function panesInSnapshot(server: AgentHookServer): { paneKey: string; prompt?: string }[] { + return server + .getStatusSnapshot() + .map((entry) => ({ paneKey: entry.paneKey, prompt: entry.prompt })) +} + +describe('shared-daemon pane attribution (local hook path)', () => { + it('routes a daemon-hosted turn to the pane the session was spawned into, not the inherited pane key', async () => { + await withServer(async (server, post) => { + server.bindAgentSessionPane('claude', SESSION_A, { paneKey: SPLIT_PANE_A, ptyId: 'pty-a' }) + + expect(await post(buildDaemonHostedBody(SESSION_A, 'ship the fix'))).toBe(204) + + expect(panesInSnapshot(server)).toEqual([{ paneKey: SPLIT_PANE_A, prompt: 'ship the fix' }]) + }) + }) + + it('re-files a corrected event under the workspace it was spawned in, not the daemon’s', async () => { + await withServer(async (server, post) => { + server.bindAgentSessionPane('claude', SESSION_A, { + paneKey: SPLIT_PANE_A, + ptyId: 'pty-a', + worktreeId: 'wt-real' + }) + + expect(await post(buildDaemonHostedBody(SESSION_A, 'cross-workspace turn'))).toBe(204) + + // 'wt-daemon' is the daemon's inherited workspace and must not survive. + expect(server.getStatusSnapshot()).toEqual([ + expect.objectContaining({ paneKey: SPLIT_PANE_A, worktreeId: 'wt-real' }) + ]) + }) + }) + + it('keeps two split panes in one tab on their own rows when both post the same inherited key', async () => { + await withServer(async (server, post) => { + server.bindAgentSessionPane('claude', SESSION_A, { paneKey: SPLIT_PANE_A, ptyId: 'pty-a' }) + server.bindAgentSessionPane('claude', SESSION_B, { paneKey: SPLIT_PANE_B, ptyId: 'pty-b' }) + + expect(await post(buildDaemonHostedBody(SESSION_A, 'left pane work'))).toBe(204) + expect(await post(buildDaemonHostedBody(SESSION_B, 'right pane work'))).toBe(204) + + // Without per-session binding both turns collapse onto DAEMON_PANE and the + // second prompt clobbers the first. + expect(panesInSnapshot(server).sort((a, b) => a.paneKey.localeCompare(b.paneKey))).toEqual([ + { paneKey: SPLIT_PANE_A, prompt: 'left pane work' }, + { paneKey: SPLIT_PANE_B, prompt: 'right pane work' } + ]) + }) + }) + + it('leaves an unpinned session on the pane it posted', async () => { + await withServer(async (server, post) => { + server.bindAgentSessionPane('claude', SESSION_A, { paneKey: SPLIT_PANE_A, ptyId: 'pty-a' }) + + expect(await post(buildDaemonHostedBody(UNPINNED_SESSION, 'typed claude'))).toBe(204) + + // A hand-typed `claude` in a plain shell has no pin; today's behavior stands. + expect(panesInSnapshot(server)).toEqual([{ paneKey: DAEMON_PANE, prompt: 'typed claude' }]) + }) + }) + + it('stops re-routing once the pinning PTY exits', async () => { + await withServer(async (server, post) => { + server.bindAgentSessionPane('claude', SESSION_A, { paneKey: SPLIT_PANE_A, ptyId: 'pty-a' }) + server.clearAgentSessionPaneBindingsForPty('pty-a') + + expect(await post(buildDaemonHostedBody(SESSION_A, 'after pane closed'))).toBe(204) + + expect(panesInSnapshot(server)).toEqual([ + { paneKey: DAEMON_PANE, prompt: 'after pane closed' } + ]) + }) + }) + + it('does not let one agent binding capture another agent that reuses the session id', async () => { + await withServer(async (server, post) => { + // Same id, different hook source: bindings are source-scoped. + server.bindAgentSessionPane('codex', SESSION_A, { paneKey: SPLIT_PANE_A, ptyId: 'pty-a' }) + + expect(await post(buildDaemonHostedBody(SESSION_A, 'claude turn'))).toBe(204) + + expect(panesInSnapshot(server)).toEqual([{ paneKey: DAEMON_PANE, prompt: 'claude turn' }]) + }) + }) + + it('refuses to bind a pane key the status pipeline cannot route', async () => { + // Why #14018 reported an unroutable "$$:L$$" key: any key that is not + // `:` is rejected at the hook boundary, so a binding must + // never be able to introduce one. + const remintedShape = '$$q7v2m9c4:L$$' + expect(isValidPaneKey(remintedShape)).toBe(false) + + await withServer(async (server, post) => { + server.bindAgentSessionPane('claude', SESSION_A, { paneKey: remintedShape, ptyId: 'pty-a' }) + + expect(await post(buildDaemonHostedBody(SESSION_A, 'unroutable target'))).toBe(204) + + expect(panesInSnapshot(server)).toEqual([ + { paneKey: DAEMON_PANE, prompt: 'unroutable target' } + ]) + }) + }) +}) + +describe('shared-daemon pane attribution (relay path)', () => { + it('routes a remote daemon-hosted turn to the spawn-pinned pane', () => { + _internals.resetCachesForTests() + const server = new AgentHookServer() + server.bindAgentSessionPane('claude', SESSION_A, { + paneKey: SPLIT_PANE_A, + ptyId: 'pty-remote', + worktreeId: 'wt-real' + }) + + const event = normalizeHookPayload( + createHookListenerState(), + 'claude', + buildDaemonHostedBody(SESSION_A, 'remote turn'), + 'production' + ) + if (!event) { + throw new Error('normalizeHookPayload rejected a known-good relay fixture') + } + expect(event.providerSession?.id).toBe(SESSION_A) + server.ingestRemote({ ...event, source: 'claude' }, 'conn-1') + + expect(panesInSnapshot(server)).toEqual([{ paneKey: SPLIT_PANE_A, prompt: 'remote turn' }]) + // Why asserted here too: the relay seam has its own worktree override, and + // 'wt-daemon' is the workspace the stale env named. + expect(server.getStatusSnapshot()[0]?.worktreeId).toBe('wt-real') + }) +}) + +describe('an already-correct pane key', () => { + it('is left alone rather than overwritten from the binding', async () => { + // Why this matters beyond an early-out: the binding also carries a + // worktreeId. A session that posts its own (correct) pane must not have its + // workspace restamped from a binding recorded at spawn time. + await withServer(async (server, post) => { + server.bindAgentSessionPane('claude', SESSION_A, { + paneKey: DAEMON_PANE, + ptyId: 'pty-a', + worktreeId: 'wt-at-spawn' + }) + + expect(await post(buildDaemonHostedBody(SESSION_A, 'posted its own pane'))).toBe(204) + + expect(server.getStatusSnapshot()).toEqual([ + expect.objectContaining({ paneKey: DAEMON_PANE, worktreeId: 'wt-daemon' }) + ]) + }) + }) +}) + +describe('readHookBodyProviderSessionId', () => { + it('reads the id from both the object and JSON-string payload forms', () => { + const body = buildDaemonHostedBody(SESSION_A, 'x') + expect(readHookBodyProviderSessionId('claude', body)).toBe(SESSION_A) + expect( + readHookBodyProviderSessionId('claude', { ...body, payload: JSON.stringify(body.payload) }) + ).toBe(SESSION_A) + }) + + it('ignores a Codex child hook, whose session id belongs to the child not the pane', () => { + expect( + readHookBodyProviderSessionId('codex', { + paneKey: DAEMON_PANE, + payload: { session_id: SESSION_A, agent_id: 'child-1' } + }) + ).toBeNull() + }) + + it('returns null for malformed payloads instead of throwing', () => { + expect(readHookBodyProviderSessionId('claude', null)).toBeNull() + expect(readHookBodyProviderSessionId('claude', { payload: '{not json' })).toBeNull() + expect(readHookBodyProviderSessionId('claude', { payload: 7 })).toBeNull() + }) +}) diff --git a/src/main/agent-hooks/server.ts b/src/main/agent-hooks/server.ts index 2f450d6a6b98..f32a497ba69b 100644 --- a/src/main/agent-hooks/server.ts +++ b/src/main/agent-hooks/server.ts @@ -32,6 +32,7 @@ import { resolveCachedClaudeCompactOwnership, resolveHookSource, preparePendingGrokResultDiscovery, + readHookBodyProviderSessionId, seedClaudeLeadTurnFromPersistedStatus, seedClaudeSubagentRosterFromSnapshots, seedCodexStateFromSnapshot, @@ -85,6 +86,10 @@ import { type AgentProviderSessionMetadata } from '../../shared/agent-session-resume' import { isCommandCodeNewTurnWhileWorking } from '../../shared/command-code-turn-boundary' +import { + AgentSessionPaneBindings, + type AgentSessionPaneBinding +} from './agent-session-pane-bindings' export type { AgentHookSource } @@ -685,6 +690,9 @@ export class AgentHookServer { private state: HookListenerState = createHookListenerState() // Why: hydrated rows give UI continuity but aren't evidence of live agent work in this runtime. private runtimeObservedStatusPaneKeys = new Set() + // Why: a daemon-hosted session's hooks post the daemon's inherited pane key. + // Spawn-time session pins are the only evidence of the true pane. + private agentSessionPaneBindings = new AgentSessionPaneBindings() private hydratedAuthorityCommitments: readonly AgentHookAuthorityEvidence[] = Object.freeze([]) private hydratedLaunchTokenHashByPaneKey = new Map() private persistedAuthorityCommitmentsByPaneKey = new Map() @@ -1838,6 +1846,66 @@ export class AgentHookServer { return changed } + /** Records the pane a freshly spawned agent session belongs to. Called from + * the spawn path, which is the last place both halves are known: the session + * id Orca minted into the launch command, and the pane it launched into. */ + bindAgentSessionPane( + source: AgentHookSource, + sessionId: string, + binding: AgentSessionPaneBinding + ): void { + if (!isValidPaneKey(binding.paneKey)) { + return + } + this.agentSessionPaneBindings.bind(source, sessionId, binding) + } + + clearAgentSessionPaneBindingsForPty(ptyId: string): void { + this.agentSessionPaneBindings.clearForPty(ptyId) + } + + /** The pane a session was spawned into, when it differs from the key the + * event posted. Null means "no correction" — an unpinned session (a + * hand-typed `claude`, an agent Orca never launched) keeps today's behavior, + * so this can only ever move status ONTO a pane Orca itself started. */ + private resolveBoundPaneOverride( + source: AgentHookSource, + sessionId: string | null, + postedPaneKey: unknown + ): AgentSessionPaneBinding | null { + const bound = this.agentSessionPaneBindings.resolve(source, sessionId) + if (!bound || bound.paneKey === postedPaneKey) { + return null + } + return bound + } + + private normalizeHookBodyAgentSessionPane(source: AgentHookSource, body: unknown): unknown { + if (typeof body !== 'object' || body === null) { + return body + } + const record = body as Record + const bound = this.resolveBoundPaneOverride( + source, + readHookBodyProviderSessionId(source, body), + typeof record.paneKey === 'string' ? record.paneKey.trim() : '' + ) + if (!bound) { + return body + } + // Why: the posted key came from the worker's inherited env; the spawn-time + // binding names the pane that actually owns this session. Rewrite tabId + // with it too, or normalizeHookPayload's tabId/paneKey agreement check + // rejects the event outright — and the worktree, since the same inherited + // env named the daemon's workspace. + return { + ...record, + paneKey: bound.paneKey, + tabId: parsePaneKey(bound.paneKey)?.tabId, + ...(bound.worktreeId ? { worktreeId: bound.worktreeId } : {}) + } + } + private normalizeHookBodyPaneKeyAlias(body: unknown): unknown { if (typeof body !== 'object' || body === null) { return body @@ -2026,7 +2094,18 @@ export class AgentHookServer { } // Why: trim paneKey to match the HTTP path, else remote-vs-local events for one pane diverge. const physicalPaneKey = envelope.paneKey.trim() - const paneKey = this.resolvePaneKeyAlias(physicalPaneKey) + const source = isAgentHookSource(envelope.source) ? envelope.source : undefined + // Why: a remote host's shared agent daemon inherits a stale ORCA_PANE_KEY + // exactly like a local one, so SSH/WSL panes need the same spawn-time + // session binding, resolved before pane-migration aliasing applies on top. + const boundPane = source + ? this.resolveBoundPaneOverride( + source, + normalizeAgentProviderSession(envelope.providerSession)?.id ?? null, + physicalPaneKey + ) + : null + const paneKey = this.resolvePaneKeyAlias(boundPane?.paneKey ?? physicalPaneKey) const parsedPaneKey = parsePaneKey(paneKey) if (paneKey.length === 0) { track('agent_hook_unattributed', { reason: 'empty_pane_key' }) @@ -2061,7 +2140,6 @@ export class AgentHookServer { typeof envelope.hookEventName === 'string' && envelope.hookEventName.trim().length > 0 ? envelope.hookEventName.trim() : undefined - const source = isAgentHookSource(envelope.source) ? envelope.source : undefined const providerPromptId = source === 'claude' ? normalizeClaudePromptId(envelope.providerPromptId) : undefined const compactTrigger = @@ -2076,10 +2154,13 @@ export class AgentHookServer { if (statusDisposition === 'suppress') { return } + // Why: a corrected pane brings its own workspace — the envelope's came from + // the same inherited env that named the wrong pane. const worktreeId = - envelope.worktreeId !== undefined && envelope.worktreeId.trim().length > 0 + boundPane?.worktreeId ?? + (envelope.worktreeId !== undefined && envelope.worktreeId.trim().length > 0 ? envelope.worktreeId.trim() - : undefined + : undefined) const promptInteractionKey = typeof envelope.promptInteractionKey === 'string' && envelope.promptInteractionKey.trim().length > 0 @@ -2275,7 +2356,10 @@ export class AgentHookServer { } trackEmptyPaneKeyHook(body) - const aliasedBody = this.normalizeHookBodyPaneKeyAlias(body) + // Why: session attribution runs first — it corrects an inherited key to + // the real pane, and pane-migration aliasing then applies on top of it. + const attributedBody = this.normalizeHookBodyAgentSessionPane(source, body) + const aliasedBody = this.normalizeHookBodyPaneKeyAlias(attributedBody) const normalized = this.normalizeLocalHookPayload(source, aliasedBody) const statusDisposition = normalized.event ? this.getAgentStatusDisposition(normalized.event.paneKey, { diff --git a/src/main/ipc/pty-ipc-mock-registry.ts b/src/main/ipc/pty-ipc-mock-registry.ts index 4a1b48972be0..c99c0e6f6a6a 100644 --- a/src/main/ipc/pty-ipc-mock-registry.ts +++ b/src/main/ipc/pty-ipc-mock-registry.ts @@ -37,6 +37,8 @@ export const unregisterPtyMock: Mock = vi.fn() export const setMigrationUnsupportedPtyMock: Mock = vi.fn() export const clearMigrationUnsupportedPtyMock: Mock = vi.fn() export const clearMigrationUnsupportedPtysForPaneKeyMock: Mock = vi.fn() +export const bindAgentSessionPaneMock: Mock = vi.fn() +export const clearAgentSessionPaneBindingsForPtyMock: Mock = vi.fn() export const clearPaneKeyAliasesForPtyMock: Mock = vi.fn() export const recordCodexPaneAccountMock: Mock = vi.fn() export const forgetCodexPaneAccountMock: Mock = vi.fn() @@ -117,7 +119,9 @@ export const agentHookServerModuleMock = () => ({ buildPtyEnv: buildAgentHookEnvMock, clearPaneState: clearAgentHookPaneStateMock, registerPaneKeyAlias: registerPaneKeyAliasMock, - clearPaneKeyAliasesForPty: clearPaneKeyAliasesForPtyMock + clearPaneKeyAliasesForPty: clearPaneKeyAliasesForPtyMock, + bindAgentSessionPane: bindAgentSessionPaneMock, + clearAgentSessionPaneBindingsForPty: clearAgentSessionPaneBindingsForPtyMock } }) diff --git a/src/main/ipc/pty-ipc-suite-environment.ts b/src/main/ipc/pty-ipc-suite-environment.ts index d1be80005519..ddf9e1a4787f 100644 --- a/src/main/ipc/pty-ipc-suite-environment.ts +++ b/src/main/ipc/pty-ipc-suite-environment.ts @@ -32,6 +32,8 @@ import { setMigrationUnsupportedPtyMock, clearMigrationUnsupportedPtyMock, clearMigrationUnsupportedPtysForPaneKeyMock, + bindAgentSessionPaneMock, + clearAgentSessionPaneBindingsForPtyMock, clearPaneKeyAliasesForPtyMock, recordCodexPaneAccountMock, forgetCodexPaneAccountMock, @@ -123,6 +125,8 @@ export function createPtyIpcSuiteEnvironment(): PtyIpcSuiteEnvironment { setMigrationUnsupportedPtyMock.mockReset() clearMigrationUnsupportedPtyMock.mockReset() clearMigrationUnsupportedPtysForPaneKeyMock.mockReset() + bindAgentSessionPaneMock.mockReset() + clearAgentSessionPaneBindingsForPtyMock.mockReset() clearPaneKeyAliasesForPtyMock.mockReset() recordCodexPaneAccountMock.mockReset() forgetCodexPaneAccountMock.mockReset() diff --git a/src/main/ipc/pty-login-shell-startup-commands.test.ts b/src/main/ipc/pty-login-shell-startup-commands.test.ts index bd74321fe891..0c548093788e 100644 --- a/src/main/ipc/pty-login-shell-startup-commands.test.ts +++ b/src/main/ipc/pty-login-shell-startup-commands.test.ts @@ -2,7 +2,8 @@ import { describe, expect, it, vi } from 'vitest' import { loginPreflightExecFileMock, spawnMock, - openCodeBuildPtyEnvMock + openCodeBuildPtyEnvMock, + bindAgentSessionPaneMock } from './pty-ipc-mock-registry' import { posixOnlyIt } from './pty-ipc-test-constants' import { setupPtyIpcSuite } from './pty-ipc-test-harness' @@ -229,7 +230,60 @@ describe('registerPtyHandlers', () => { mockProc.emitData('\x1b]133;A\x07% ') await Promise.resolve() vi.runAllTimers() - expect(mockProc.proc.write).toHaveBeenCalledWith('claude\n') + // Why the pin: a Claude launch carries a minted --session-id so a + // daemon-hosted session's hooks can still be traced to this pane (#9236). + expect(mockProc.proc.write).toHaveBeenCalledTimes(1) + expect(mockProc.proc.write.mock.calls[0][0]).toMatch( + /^claude --session-id [0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\n$/ + ) + } finally { + vi.useRealTimers() + } + } + ) + posixOnlyIt( + 'binds the minted session id to the spawning pane, so hooks can be traced back to it', + async () => { + // Why this asserts the SPAWN side: the hook server's own tests bind by + // hand, so without this the wiring that actually records the pane — + // the only thing that makes the correction fire in production — is + // deletable with every suite still green. + vi.useFakeTimers() + const mockProc = createMockProc() + spawnMock.mockReturnValue(mockProc.proc) + const tabId = 'tab-1' + const leafId = 'aaaaaaaa-1111-4111-8111-111111111111' + const paneKey = `${tabId}:${leafId}` + + try { + registerPtyHandlers(mainWindow as never) + await handlers.get('pty:spawn')!(null, { + cols: 80, + rows: 24, + cwd: '/tmp', + command: 'claude', + tabId, + leafId, + worktreeId: 'wt-real', + env: { ORCA_PANE_KEY: paneKey, ORCA_TAB_ID: tabId, ORCA_WORKTREE_ID: 'wt-real' } + }) + + mockProc.emitData('last login: today\r\n') + vi.runOnlyPendingTimers() + mockProc.emitData('\x1b]133;A\x07% ') + await Promise.resolve() + vi.runAllTimers() + + const written = String(mockProc.proc.write.mock.calls[0][0]) + const mintedId = /--session-id ([0-9a-f-]{36})/.exec(written)?.[1] + expect(mintedId).toBeDefined() + // The id on the command line and the id recorded against the pane must + // be the same one, or the correction can never resolve. + expect(bindAgentSessionPaneMock).toHaveBeenCalledWith( + 'claude', + mintedId, + expect.objectContaining({ paneKey, worktreeId: 'wt-real' }) + ) } finally { vi.useRealTimers() } diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index 3c3e148ffb2a..0414cd514fac 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -118,6 +118,9 @@ import { markClaudePtyExited, markClaudePtySpawned } from '../claude-accounts/live-pty-gate' +import { pinClaudeLaunchSessionId } from '../../shared/claude-session-pin-launch-command' +import type { AgentStartupShell } from '../../shared/tui-agent-startup-shell' +import { resolveSpawnStartupShell } from '../pty/spawn-startup-shell' import { ensureLinuxTerminalOrcaCliShimDir } from '../cli/linux-terminal-orca-cli-shim' import { isLegacyTerminalShimPathEntry, @@ -1996,6 +1999,31 @@ function isClaudeLaunchCommand(command: string | undefined): boolean { ) } +type ClaudeSessionPinPlan = { command: string | undefined; sessionId: string | null } + +/** Mints a session id into a Claude launch so hook events can be traced back to + * the pane Orca spawned them into. + * + * Why: Claude Code >= 2.1.206 hosts TUI sessions as workers under a shared + * daemon, and the daemon forwards only its own allowlisted env — so a worker's + * `ORCA_PANE_KEY` names whichever pane first started the daemon, not the pane + * the user is looking at (#9236). The session id is the only pane-independent + * identity a hook payload carries, so binding it here is what lets the hook + * server route status to the true pane. Pinning is best-effort by design: a + * command it cannot splice safely is left untouched and keeps today's behavior. + */ +function planClaudeSessionPin( + command: string | undefined, + shell: AgentStartupShell +): ClaudeSessionPinPlan { + if (command === undefined || !isClaudeLaunchCommand(command)) { + return { command, sessionId: null } + } + const sessionId = randomUUID() + const pinned = pinClaudeLaunchSessionId(command, sessionId, shell) + return pinned ? { command: pinned, sessionId } : { command, sessionId: null } +} + function routesFreshSpawnsToLocalProvider(provider: IPtyProvider): boolean { return provider.routesFreshSpawnsToLocalProvider === true } @@ -2123,6 +2151,9 @@ export function clearProviderPtyState( piTitlebarExtensionService.clearPty(id) // Why: SSH exit/teardown paths bypass pty.ts's local onExit but still must release Claude account-switch guards. markClaudePtyExited(id) + // Why: a dead PTY's session pin must not re-route a later session onto a pane + // it no longer runs. + agentHookServer.clearAgentSessionPaneBindingsForPty(id) ptySizes.delete(id) ptyIncarnationById.delete(id) lastInputAtByPty.delete(id) @@ -4653,7 +4684,18 @@ export function registerPtyHandlers( const codexResumeHome = codexResumeLaunch.codexResumeHome // Why: the drop still applies here, but this controller's result has no field for // notifyResumeUnavailable — runtime/relay panes start fresh without the notice. - const launchCommand = codexResumeLaunch.command + // Why not gated on isClaudeLaunch: that flag is local-only (it guards + // account switching); a remote host's daemon inherits a stale pane key too. + const claudeSessionPin = planClaudeSessionPin( + codexResumeLaunch.command, + resolveSpawnStartupShell({ + connectionId: args.connectionId, + windowsWslDistro: terminalRuntimeOptions.terminalWindowsWslDistro, + shellOverride: daemonShellOverride, + platform: process.platform + }) + ) + const launchCommand = claudeSessionPin.command const claudeAuth = isClaudeLaunch && prepareClaudeAuth ? await prepareClaudeAuth(codexSelectionTarget) : null if (isClaudeLaunch && isClaudeAuthSwitchInProgress()) { @@ -5423,6 +5465,13 @@ export function registerPtyHandlers( } // Why: runtime-owned CLI PTYs bypass the renderer pty:spawn handler; record paneKey here too since hook titles and cache cleanup need this reverse lookup. const paneKey = rememberPaneKeyForPty(result.id, env?.ORCA_PANE_KEY) + if (claudeSessionPin.sessionId && paneKey) { + agentHookServer.bindAgentSessionPane('claude', claudeSessionPin.sessionId, { + paneKey, + ptyId: result.id, + worktreeId: args.worktreeId + }) + } const pendingSerializer = paneKey ? pendingByPaneKey.get(paneKey) : undefined const inheritRendererReadiness = result.isReattach === true && @@ -6438,7 +6487,18 @@ export function registerPtyHandlers( ? await resolveCodexResumeLaunch(args.command, codexResumePreparation) : noCodexResumeLaunch(preAdoptedStablePane ? undefined : args.command) const codexResumeHome = codexResumeLaunch.codexResumeHome - const launchCommand = codexResumeLaunch.command + // Why not gated on isClaudeLaunch: that flag is local-only (it guards + // account switching); a remote host's daemon inherits a stale pane key too. + const claudeSessionPin = planClaudeSessionPin( + codexResumeLaunch.command, + resolveSpawnStartupShell({ + connectionId: args.connectionId, + windowsWslDistro: terminalRuntimeOptions.terminalWindowsWslDistro, + shellOverride: initialShellOverride, + platform: process.platform + }) + ) + const launchCommand = claudeSessionPin.command baseEnv = stripSequencedStartupResumeArgv(baseEnv, codexResumeLaunch) // Why: declared after the strip so a local-provider spawn cannot capture the // pre-strip env — only the daemon branch below re-derives this from baseEnv. @@ -7090,6 +7150,13 @@ export function registerPtyHandlers( const rememberedPaneKey = validatedPaneKey ? rememberPaneKeyForPty(result.id, validatedPaneKey) : null + if (claudeSessionPin.sessionId && rememberedPaneKey) { + agentHookServer.bindAgentSessionPane('claude', claudeSessionPin.sessionId, { + paneKey: rememberedPaneKey, + ptyId: result.id, + worktreeId: args.worktreeId + }) + } if (legacySpawnPaneKey && migrationUnsupportedPaneKey) { agentHookServer.registerPaneKeyAlias( legacySpawnPaneKey.paneKey, diff --git a/src/main/pty/spawn-startup-shell.test.ts b/src/main/pty/spawn-startup-shell.test.ts new file mode 100644 index 000000000000..a819dceab17c --- /dev/null +++ b/src/main/pty/spawn-startup-shell.test.ts @@ -0,0 +1,57 @@ +import { describe, expect, it } from 'vitest' + +import { resolveSpawnStartupShell } from './spawn-startup-shell' + +const base = { + connectionId: null, + windowsWslDistro: null, + shellOverride: undefined, + platform: 'darwin' as NodeJS.Platform +} + +describe('resolveSpawnStartupShell', () => { + it('uses posix on macOS and Linux', () => { + expect(resolveSpawnStartupShell(base)).toBe('posix') + expect(resolveSpawnStartupShell({ ...base, platform: 'linux' })).toBe('posix') + }) + + it('uses posix for an SSH pane even when the client is on Windows', () => { + expect(resolveSpawnStartupShell({ ...base, platform: 'win32', connectionId: 'conn-1' })).toBe( + 'posix' + ) + }) + + it('uses posix for a WSL pane on a Windows host', () => { + expect( + resolveSpawnStartupShell({ ...base, platform: 'win32', windowsWslDistro: 'Ubuntu' }) + ).toBe('posix') + }) + + it('defaults a native Windows pane to powershell', () => { + expect(resolveSpawnStartupShell({ ...base, platform: 'win32' })).toBe('powershell') + expect( + resolveSpawnStartupShell({ + ...base, + platform: 'win32', + shellOverride: 'C:\\Program Files\\PowerShell\\7\\pwsh.exe' + }) + ).toBe('powershell') + }) + + it('recognizes cmd and Git Bash overrides on Windows', () => { + expect( + resolveSpawnStartupShell({ + ...base, + platform: 'win32', + shellOverride: 'C:\\Windows\\System32\\cmd.exe' + }) + ).toBe('cmd') + expect( + resolveSpawnStartupShell({ + ...base, + platform: 'win32', + shellOverride: 'C:\\Program Files\\Git\\bin\\bash.exe' + }) + ).toBe('posix') + }) +}) diff --git a/src/main/pty/spawn-startup-shell.ts b/src/main/pty/spawn-startup-shell.ts new file mode 100644 index 000000000000..9c9d49a5dc98 --- /dev/null +++ b/src/main/pty/spawn-startup-shell.ts @@ -0,0 +1,25 @@ +import type { AgentStartupShell } from '../../shared/tui-agent-startup-shell' + +/** The dialect that parses a spawn's launch command. + * + * Why not just `process.platform`: a WSL pane and an SSH pane both run a POSIX + * shell while Orca itself is on Windows, and a Windows pane can be pointed at + * cmd or Git Bash instead of PowerShell. Callers that splice into the command + * text must model the shell that will actually parse it. + */ +export function resolveSpawnStartupShell(opts: { + connectionId: string | null | undefined + windowsWslDistro: string | null | undefined + shellOverride: string | undefined + platform: NodeJS.Platform +}): AgentStartupShell { + if (opts.connectionId || opts.windowsWslDistro || opts.platform !== 'win32') { + return 'posix' + } + const base = + opts.shellOverride?.trim().replaceAll('\\', '/').split('/').pop()?.toLowerCase() ?? '' + if (/^(?:ba|z|k|da|)sh(?:\.exe)?$/.test(base)) { + return 'posix' + } + return base.startsWith('cmd') ? 'cmd' : 'powershell' +} diff --git a/src/shared/agent-hook-listener.ts b/src/shared/agent-hook-listener.ts index dc349fcdaedb..4ec68dd9ecf3 100644 --- a/src/shared/agent-hook-listener.ts +++ b/src/shared/agent-hook-listener.ts @@ -4297,6 +4297,44 @@ function readStringField(record: Record, key: string): string | return trimmed.length > 0 ? trimmed : undefined } +/** + * The provider session id a raw hook body reports, read exactly as + * `normalizeHookPayload` reads it (same JSON guard, same per-source extractor, + * same Codex child carve-out). + * + * Why it is exposed: pane attribution has to run BEFORE normalization. + * `normalizeHookPayload` keys its prompt/compaction state machine on the posted + * pane key, so correcting the key afterwards would leave that state on the + * wrong pane. Callers that re-derive the session id themselves drift from this + * parser the moment a source changes shape. + */ +export function readHookBodyProviderSessionId( + source: AgentHookSource, + body: unknown +): string | null { + if (typeof body !== 'object' || body === null) { + return null + } + const rawPayload = (body as Record).payload + let hookPayload: unknown = rawPayload + if (typeof rawPayload === 'string') { + try { + hookPayload = parseAgentHookJson(rawPayload) + } catch { + return null + } + } + if (typeof hookPayload !== 'object' || hookPayload === null) { + return null + } + const record = hookPayload as Record + // Why: Codex child hooks expose the child's session_id on the parent's pane. + if (source === 'codex' && readString(record, 'agent_id')) { + return null + } + return extractAgentProviderSession(source, record)?.id ?? null +} + export function normalizeHookPayload( state: HookListenerState, source: AgentHookSource, diff --git a/src/shared/agent-resume-launch-command.ts b/src/shared/agent-resume-launch-command.ts index cda40878c284..a9a7ac5d5b04 100644 --- a/src/shared/agent-resume-launch-command.ts +++ b/src/shared/agent-resume-launch-command.ts @@ -1,6 +1,7 @@ import type { ResumableTuiAgent } from './agent-session-resume' +import { findClaudeExecutableIndex } from './claude-launch-executable-token' import { - isPosixStartupShell, + isFullyModelableStartupCommand, quoteStartupArg, tokenizeStartupCommand, type AgentStartupShell @@ -20,40 +21,6 @@ function isClaudeResumeSelector(token: string): boolean { return token === '-r' || token.startsWith('-r=') || token === '-c' || token.startsWith('-c=') } -function isClaudeExecutableToken(token: string): boolean { - const base = token.split(/[\\/]/).pop() ?? '' - return /^claude(\.(exe|cmd|bat|ps1))?$/i.test(base) -} - -/** Accepts a claude token only in command position — index 0, right after a - * wrapper's `--`, behind PowerShell's `&` call operator, or preceded solely by - * NAME=value assignments — so an argument that merely ends in /claude (an ssh - * key, a project dir) can never be mistaken for the executable. */ -function findClaudeExecutableIndex(tokens: readonly string[], shell: AgentStartupShell): number { - let commandPosition = true - for (let i = 0; i < tokens.length; i += 1) { - const token = tokens[i] - if (commandPosition) { - if (isClaudeExecutableToken(token)) { - return i - } - if ( - // Why: `NAME=value cmd` is sh-family syntax (fish included, 3.1+); on - // cmd/PowerShell such a token is a bogus executable name, not a prefix. - (isPosixStartupShell(shell) && /^[A-Za-z_][A-Za-z0-9_]*=/.test(token)) || - (shell === 'powershell' && token === '&' && i === 0) - ) { - continue - } - commandPosition = false - } - if (token === '--') { - commandPosition = true - } - } - return -1 -} - /** Joins the resolved base command with the agent's resume argv. Claude goes * through the selector guard below; other agents keep plain appending. */ export function buildAgentResumeLaunchCommand( @@ -100,33 +67,8 @@ export function buildClaudeResumeLaunchCommand( if (claudeIndex === -1) { return appended } - // Why: any token the tokenizer cannot model for this shell — an operator, - // comment, expansion, or cmd single-quoted region — means the splice could - // cut live syntax or misread a literal as a selector. The whole base must - // be modelable, including the executable itself; only PowerShell's leading - // call operator is a known-safe divergent token. - for (let i = 0; i <= tokens.length; i += 1) { - const gapStart = i === 0 ? 0 : spans[i - 1].end - const gapEnd = i === tokens.length ? baseCommand.length : spans[i].start - if (!/^[ \t]*$/.test(baseCommand.slice(gapStart, gapEnd))) { - return appended - } - if (i === tokens.length) { - break - } - // Why: a bare `--%` makes PowerShell pass the rest of the line to the - // child literally, so appended quoting would arrive as literal bytes. A - // quoted `--%` can also stop parsing, but only before a parameter token, - // where the base is already mangled with or without the guard. - if (shell === 'powershell' && baseCommand.slice(spans[i].start, spans[i].end) === '--%') { - return appended - } - if (spans[i].divergesFromShell) { - const isCallOperator = shell === 'powershell' && i === 0 && tokens[i] === '&' - if (!isCallOperator) { - return appended - } - } + if (!isFullyModelableStartupCommand(baseCommand, tokens, spans, shell)) { + return appended } const cuts: { start: number; end: number }[] = [] let terminatorStart: number | null = null diff --git a/src/shared/claude-launch-executable-token.ts b/src/shared/claude-launch-executable-token.ts new file mode 100644 index 000000000000..664efd0d53fa --- /dev/null +++ b/src/shared/claude-launch-executable-token.ts @@ -0,0 +1,38 @@ +import { isPosixStartupShell, type AgentStartupShell } from './tui-agent-startup-shell' + +export function isClaudeExecutableToken(token: string): boolean { + const base = token.split(/[\\/]/).pop() ?? '' + return /^claude(\.(exe|cmd|bat|ps1))?$/i.test(base) +} + +/** Accepts a claude token only in command position — index 0, right after a + * wrapper's `--`, behind PowerShell's `&` call operator, or preceded solely by + * NAME=value assignments — so an argument that merely ends in /claude (an ssh + * key, a project dir) can never be mistaken for the executable. */ +export function findClaudeExecutableIndex( + tokens: readonly string[], + shell: AgentStartupShell +): number { + let commandPosition = true + for (let i = 0; i < tokens.length; i += 1) { + const token = tokens[i] + if (commandPosition) { + if (isClaudeExecutableToken(token)) { + return i + } + if ( + // Why: `NAME=value cmd` is sh-family syntax (fish included, 3.1+); on + // cmd/PowerShell such a token is a bogus executable name, not a prefix. + (isPosixStartupShell(shell) && /^[A-Za-z_][A-Za-z0-9_]*=/.test(token)) || + (shell === 'powershell' && token === '&' && i === 0) + ) { + continue + } + commandPosition = false + } + if (token === '--') { + commandPosition = true + } + } + return -1 +} diff --git a/src/shared/claude-session-pin-launch-command.test.ts b/src/shared/claude-session-pin-launch-command.test.ts new file mode 100644 index 000000000000..16e310c608d7 --- /dev/null +++ b/src/shared/claude-session-pin-launch-command.test.ts @@ -0,0 +1,104 @@ +import { describe, expect, it } from 'vitest' + +import { pinClaudeLaunchSessionId } from './claude-session-pin-launch-command' + +const SESSION = '0192a4b1-1111-4111-8111-aaaaaaaaaaaa' + +describe('pinClaudeLaunchSessionId', () => { + it('pins a bare launch', () => { + expect(pinClaudeLaunchSessionId('claude', SESSION, 'posix')).toBe( + `claude --session-id ${SESSION}` + ) + }) + + it('pins path-qualified and Windows executables', () => { + expect(pinClaudeLaunchSessionId('/usr/local/bin/claude --model opus', SESSION, 'posix')).toBe( + `/usr/local/bin/claude --session-id ${SESSION} --model opus` + ) + expect(pinClaudeLaunchSessionId('claude.cmd', SESSION, 'cmd')).toBe( + `claude.cmd --session-id ${SESSION}` + ) + expect(pinClaudeLaunchSessionId('& claude', SESSION, 'powershell')).toBe( + `& claude --session-id ${SESSION}` + ) + }) + + it('pins behind posix env assignments, which are still command position', () => { + expect(pinClaudeLaunchSessionId('FOO=1 claude', SESSION, 'posix')).toBe( + `FOO=1 claude --session-id ${SESSION}` + ) + }) + + it('keeps the pin in option position, before claude’s own -- terminator', () => { + expect(pinClaudeLaunchSessionId('claude -- --weird', SESSION, 'posix')).toBe( + `claude --session-id ${SESSION} -- --weird` + ) + }) + + // Why: `--session-id` is a ROOT option. `claude mcp list --session-id ` + // exits with "error: unknown option '--session-id'", so a trailing pin would + // break a launch that works today — the pin must precede the subcommand. + it.each([ + ['mcp list', 'claude mcp list'], + ['doctor', 'claude doctor'], + ['update', 'claude update'], + ['setup-token', 'claude setup-token'], + ['install stable', 'claude install stable'] + ])('pins ahead of the %s subcommand, never after it', (_label, command) => { + const rest = command.slice('claude'.length) + expect(pinClaudeLaunchSessionId(command, SESSION, 'posix')).toBe( + `claude --session-id ${SESSION}${rest}` + ) + }) + + it('preserves the base bytes verbatim, including quoting', () => { + expect(pinClaudeLaunchSessionId('claude "fix the bug"', SESSION, 'posix')).toBe( + `claude --session-id ${SESSION} "fix the bug"` + ) + }) + + it('leaves a selector that follows claude’s own -- terminator alone', () => { + // Why: past `--` the tokens are the child's argv, not claude flags, so they + // are not a competing selector and must not suppress the pin. + expect(pinClaudeLaunchSessionId('claude -- --resume abc', SESSION, 'posix')).toBe( + `claude --session-id ${SESSION} -- --resume abc` + ) + }) + + it.each([ + ['--session-id', `claude --session-id ${SESSION}`], + ['--session-id=', `claude --session-id=${SESSION}`], + ['--resume', 'claude --resume abc'], + ['--resume=', 'claude --resume=abc'], + ['--continue', 'claude --continue'], + ['--continue=', 'claude --continue=abc'], + ['--fork-session', 'claude --fork-session'], + ['-r', 'claude -r abc'], + ['-r=', 'claude -r=abc'], + ['-c', 'claude -c'], + ['-c=', 'claude -c=abc'] + ])('refuses to compete with an existing %s selector', (_label, command) => { + expect(pinClaudeLaunchSessionId(command, SESSION, 'posix')).toBeNull() + }) + + it.each([ + ['compound', 'claude && echo done'], + ['sequenced', 'claude; echo done'], + ['piped', 'claude | tee log'], + ['redirected', 'claude > out.txt'], + ['wrapper-launched', 'nvm exec claude'], + ['not claude at all', 'codex'], + ['a path that merely ends in claude', 'ssh -i ~/.ssh/claude host'] + ])('refuses to pin a %s command', (_label, command) => { + expect(pinClaudeLaunchSessionId(command, SESSION, 'posix')).toBeNull() + }) + + it('refuses a PowerShell --% stop-parsing command', () => { + expect(pinClaudeLaunchSessionId('claude --% --raw', SESSION, 'powershell')).toBeNull() + }) + + it('refuses a session id that is not a UUID', () => { + expect(pinClaudeLaunchSessionId('claude', 'not-a-uuid', 'posix')).toBeNull() + expect(pinClaudeLaunchSessionId('claude', '', 'posix')).toBeNull() + }) +}) diff --git a/src/shared/claude-session-pin-launch-command.ts b/src/shared/claude-session-pin-launch-command.ts new file mode 100644 index 000000000000..1d53d979d637 --- /dev/null +++ b/src/shared/claude-session-pin-launch-command.ts @@ -0,0 +1,86 @@ +import { findClaudeExecutableIndex } from './claude-launch-executable-token' +import { + isFullyModelableStartupCommand, + tokenizeStartupCommand, + type AgentStartupShell +} from './tui-agent-startup-shell' + +/** Every flag that makes the CLI choose its own session identity. If the user's + * command already carries one, Orca must not add a competing `--session-id`. */ +function isClaudeSessionSelector(token: string): boolean { + return ( + token === '--session-id' || + token.startsWith('--session-id=') || + token === '--resume' || + token.startsWith('--resume=') || + token === '--continue' || + token.startsWith('--continue=') || + token === '--fork-session' || + token === '-r' || + token.startsWith('-r=') || + token === '-c' || + token.startsWith('-c=') + ) +} + +const SESSION_ID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/ + +/** + * Splices `--session-id ` into a Claude launch so the host keeps a + * process-independent handle on the session it is about to start. + * + * Why this exists: Claude Code >= 2.1.206 hosts TUI sessions as workers under a + * shared daemon, and the daemon forwards only its own allowlisted env — so a + * pane's `ORCA_PANE_KEY` never reaches the worker that runs the hooks. The + * session id is the one identity that does survive into the hook payload, so + * pinning it at spawn is what lets the host recover the true pane later. + * + * Fails CLOSED — returns null rather than a best-effort command. Pinning is an + * attribution improvement, never a launch requirement, so any doubt about the + * command's shape (untokenizable, wrapper-launched, compound, redirected, + * already session-selecting) leaves the user's command byte-for-byte alone. + * Bytes outside the insertion point are always preserved verbatim; the command + * is spliced by source span and never re-quoted. + */ +export function pinClaudeLaunchSessionId( + baseCommand: string, + sessionId: string, + shell: AgentStartupShell +): string | null { + if (!SESSION_ID_RE.test(sessionId)) { + return null + } + const tokenized = tokenizeStartupCommand(baseCommand, shell) + if (!tokenized.ok) { + return null + } + const { tokens, spans } = tokenized + const claudeIndex = findClaudeExecutableIndex(tokens, shell) + if (claudeIndex === -1) { + return null + } + // Why: modelability is what rules out compounds, pipelines and redirects — + // their operators tokenize as shell-divergent, so `claude && deploy` can + // never take a pin that would land on the wrong word. + if (!isFullyModelableStartupCommand(baseCommand, tokens, spans, shell)) { + return null + } + for (let i = claudeIndex + 1; i < tokens.length; i += 1) { + const token = tokens[i] + // Why: `--` is claude's own terminator, so anything after it is the child's + // argv, not a claude flag — a selector there is not ours to compete with. + if (token === '--') { + break + } + if (isClaudeSessionSelector(token)) { + return null + } + } + // Why immediately after the executable rather than appended: `--session-id` is + // a ROOT option, and `claude ` (`mcp list`, `doctor`, `update`) + // rejects unknown options, so a trailing pin makes those launches fail + // outright. Root position is also before claude's own `--` terminator, which + // a trailing pin would fall past. + const insertAt = spans[claudeIndex].end + return `${baseCommand.slice(0, insertAt)} --session-id ${sessionId}${baseCommand.slice(insertAt)}` +} diff --git a/src/shared/tui-agent-startup-shell.ts b/src/shared/tui-agent-startup-shell.ts index 94df0f80ac54..d6dcd32ff33e 100644 --- a/src/shared/tui-agent-startup-shell.ts +++ b/src/shared/tui-agent-startup-shell.ts @@ -162,6 +162,45 @@ export function tokenizeStartupCommand( : tokenizeCustomCommandTemplate(value) } +/** True when every byte of `value` is accounted for by `tokens`/`spans` under + * `shell` — the precondition for splicing an argument in by source span. + * + * Why: any token the tokenizer cannot model for this shell — an operator, + * comment, expansion, or cmd single-quoted region — means a splice could cut + * live syntax or misread a literal as a flag. The whole command must be + * modelable, including the executable itself; only PowerShell's leading call + * operator is a known-safe divergent token. A bare `--%` makes PowerShell pass + * the rest of the line to the child literally, so spliced quoting would arrive + * as literal bytes. + */ +export function isFullyModelableStartupCommand( + value: string, + tokens: readonly string[], + spans: readonly CommandTokenSpan[], + shell: AgentStartupShell +): boolean { + for (let i = 0; i <= tokens.length; i += 1) { + const gapStart = i === 0 ? 0 : spans[i - 1].end + const gapEnd = i === tokens.length ? value.length : spans[i].start + if (!/^[ \t]*$/.test(value.slice(gapStart, gapEnd))) { + return false + } + if (i === tokens.length) { + break + } + if (shell === 'powershell' && value.slice(spans[i].start, spans[i].end) === '--%') { + return false + } + if (spans[i].divergesFromShell) { + const isCallOperator = shell === 'powershell' && i === 0 && tokens[i] === '&' + if (!isCallOperator) { + return false + } + } + } + return true +} + export function resolveStartupShell( platform: NodeJS.Platform, shell?: AgentStartupShell