diff --git a/src/main/agent-hooks/agent-session-pane-bindings.test.ts b/src/main/agent-hooks/agent-session-pane-bindings.test.ts deleted file mode 100644 index fa70d8923250..000000000000 --- a/src/main/agent-hooks/agent-session-pane-bindings.test.ts +++ /dev/null @@ -1,66 +0,0 @@ -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 deleted file mode 100644 index df26b0a1a11b..000000000000 --- a/src/main/agent-hooks/agent-session-pane-bindings.ts +++ /dev/null @@ -1,84 +0,0 @@ -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 deleted file mode 100644 index b567ad22e596..000000000000 --- a/src/main/agent-hooks/server-agent-session-pane-attribution.test.ts +++ /dev/null @@ -1,247 +0,0 @@ -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 c3515f442827..58ad669784be 100644 --- a/src/main/agent-hooks/server.ts +++ b/src/main/agent-hooks/server.ts @@ -33,7 +33,6 @@ import { resolveCachedClaudeCompactOwnership, resolveHookSource, preparePendingGrokResultDiscovery, - readHookBodyProviderSessionId, seedClaudeLeadTurnFromPersistedStatus, seedClaudeSubagentRosterFromSnapshots, seedCodexStateFromSnapshot, @@ -99,10 +98,6 @@ 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 } @@ -724,9 +719,6 @@ export class AgentHookServer { }) // 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() @@ -2021,66 +2013,6 @@ 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 @@ -2273,18 +2205,7 @@ 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 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 paneKey = this.resolvePaneKeyAlias(physicalPaneKey) const parsedPaneKey = parsePaneKey(paneKey) if (paneKey.length === 0) { track('agent_hook_unattributed', { reason: 'empty_pane_key' }) @@ -2319,6 +2240,7 @@ 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 = @@ -2335,17 +2257,14 @@ export class AgentHookServer { } if (statusDisposition === 'restart') { // Why: same rebind as the HTTP path — a retired pane taking a new turn is a new session. - // Why paneKey, not envelope.paneKey: it is already corrected to the session's - // true pane, so the rebind cannot land on the daemon's inherited key. + // Why paneKey, not envelope.paneKey: alias resolution already mapped it to the + // stable pane, so the rebind cannot land on a legacy key. this.observations.rebind(paneKey) } - // Why: a corrected pane brings its own workspace — the envelope's came from - // the same inherited env that named the wrong pane. const worktreeId = - boundPane?.worktreeId ?? - (envelope.worktreeId !== undefined && envelope.worktreeId.trim().length > 0 + envelope.worktreeId !== undefined && envelope.worktreeId.trim().length > 0 ? envelope.worktreeId.trim() - : undefined) + : undefined const promptInteractionKey = typeof envelope.promptInteractionKey === 'string' && envelope.promptInteractionKey.trim().length > 0 @@ -2544,10 +2463,7 @@ export class AgentHookServer { } trackEmptyPaneKeyHook(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 aliasedBody = this.normalizeHookBodyPaneKeyAlias(body) 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 48ac401b29f1..24dc333e6d12 100644 --- a/src/main/ipc/pty-ipc-mock-registry.ts +++ b/src/main/ipc/pty-ipc-mock-registry.ts @@ -39,8 +39,6 @@ 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() @@ -123,9 +121,7 @@ export const agentHookServerModuleMock = () => ({ buildPtyEnv: buildAgentHookEnvMock, clearPaneState: clearAgentHookPaneStateMock, registerPaneKeyAlias: registerPaneKeyAliasMock, - clearPaneKeyAliasesForPty: clearPaneKeyAliasesForPtyMock, - bindAgentSessionPane: bindAgentSessionPaneMock, - clearAgentSessionPaneBindingsForPty: clearAgentSessionPaneBindingsForPtyMock + clearPaneKeyAliasesForPty: clearPaneKeyAliasesForPtyMock } }) diff --git a/src/main/ipc/pty-ipc-suite-environment.ts b/src/main/ipc/pty-ipc-suite-environment.ts index 45d23c3d9402..f20e0c3885ff 100644 --- a/src/main/ipc/pty-ipc-suite-environment.ts +++ b/src/main/ipc/pty-ipc-suite-environment.ts @@ -32,8 +32,6 @@ import { setMigrationUnsupportedPtyMock, clearMigrationUnsupportedPtyMock, clearMigrationUnsupportedPtysForPaneKeyMock, - bindAgentSessionPaneMock, - clearAgentSessionPaneBindingsForPtyMock, clearPaneKeyAliasesForPtyMock, recordCodexPaneAccountMock, forgetCodexPaneAccountMock, @@ -125,8 +123,6 @@ 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 5310346501b7..16d8ff4e0931 100644 --- a/src/main/ipc/pty-login-shell-startup-commands.test.ts +++ b/src/main/ipc/pty-login-shell-startup-commands.test.ts @@ -2,8 +2,7 @@ import { describe, expect, it, vi } from 'vitest' import { loginPreflightExecFileMock, spawnMock, - openCodeBuildPtyEnvMock, - bindAgentSessionPaneMock + openCodeBuildPtyEnvMock } from './pty-ipc-mock-registry' import { posixOnlyIt } from './pty-ipc-test-constants' import { setupPtyIpcSuite } from './pty-ipc-test-harness' @@ -230,60 +229,7 @@ describe('registerPtyHandlers', () => { mockProc.emitData('\x1b]133;A\x07% ') await Promise.resolve() vi.runAllTimers() - // 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' }) - ) + expect(mockProc.proc.write).toHaveBeenCalledWith('claude\n') } finally { vi.useRealTimers() } diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index ad1a4fb544cc..3a8586e66aff 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -118,9 +118,6 @@ 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, @@ -2007,31 +2004,6 @@ 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 } @@ -2159,9 +2131,6 @@ 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) @@ -4692,18 +4661,7 @@ 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. - // 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 launchCommand = codexResumeLaunch.command const claudeAuth = isClaudeLaunch && prepareClaudeAuth ? await prepareClaudeAuth(codexSelectionTarget) : null if (isClaudeLaunch && isClaudeAuthSwitchInProgress()) { @@ -5473,13 +5431,6 @@ 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 && @@ -6496,18 +6447,7 @@ export function registerPtyHandlers( ? await resolveCodexResumeLaunch(args.command, codexResumePreparation) : noCodexResumeLaunch(preAdoptedStablePane ? undefined : args.command) const codexResumeHome = codexResumeLaunch.codexResumeHome - // 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 + const launchCommand = codexResumeLaunch.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. @@ -7159,13 +7099,6 @@ 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 deleted file mode 100644 index a819dceab17c..000000000000 --- a/src/main/pty/spawn-startup-shell.test.ts +++ /dev/null @@ -1,57 +0,0 @@ -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 deleted file mode 100644 index 9c9d49a5dc98..000000000000 --- a/src/main/pty/spawn-startup-shell.ts +++ /dev/null @@ -1,25 +0,0 @@ -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 d5a52b5a6177..da80e1718410 100644 --- a/src/shared/agent-hook-listener.ts +++ b/src/shared/agent-hook-listener.ts @@ -4351,44 +4351,6 @@ 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 a9a7ac5d5b04..cda40878c284 100644 --- a/src/shared/agent-resume-launch-command.ts +++ b/src/shared/agent-resume-launch-command.ts @@ -1,7 +1,6 @@ import type { ResumableTuiAgent } from './agent-session-resume' -import { findClaudeExecutableIndex } from './claude-launch-executable-token' import { - isFullyModelableStartupCommand, + isPosixStartupShell, quoteStartupArg, tokenizeStartupCommand, type AgentStartupShell @@ -21,6 +20,40 @@ 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( @@ -67,8 +100,33 @@ export function buildClaudeResumeLaunchCommand( if (claudeIndex === -1) { return appended } - if (!isFullyModelableStartupCommand(baseCommand, tokens, spans, shell)) { - 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 + } + } } 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 deleted file mode 100644 index 664efd0d53fa..000000000000 --- a/src/shared/claude-launch-executable-token.ts +++ /dev/null @@ -1,38 +0,0 @@ -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 deleted file mode 100644 index 16e310c608d7..000000000000 --- a/src/shared/claude-session-pin-launch-command.test.ts +++ /dev/null @@ -1,104 +0,0 @@ -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 deleted file mode 100644 index 1d53d979d637..000000000000 --- a/src/shared/claude-session-pin-launch-command.ts +++ /dev/null @@ -1,86 +0,0 @@ -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 d6dcd32ff33e..94df0f80ac54 100644 --- a/src/shared/tui-agent-startup-shell.ts +++ b/src/shared/tui-agent-startup-shell.ts @@ -162,45 +162,6 @@ 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