From 5a1c6f3b9b5d94ee89a7319879331c1f1bcd4d7c Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 12 Aug 2026 20:39:36 -0700 Subject: [PATCH] fix(terminal): fully retire attribution shim --- .../app/h/[hostId]/session/[worktreeId].tsx | 57 +++- .../session/ai-vault-resume-launch.test.ts | 81 ++++- mobile/src/session/ai-vault-resume-launch.ts | 16 +- .../ai-vault-resume-preparation.test.ts | 15 +- ...mobile-terminal-attribution-compat.test.ts | 81 +++++ .../mobile-terminal-attribution-compat.ts | 34 ++ .../src/session/pr-ai-triage-launch.test.ts | 54 ++- mobile/src/session/pr-ai-triage-launch.ts | 27 +- ...se-mobile-diff-review-send-actions.test.ts | 68 ++++ .../use-mobile-diff-review-send-actions.ts | 27 +- mobile/src/transport/rpc-client.ts | 11 +- mobile/src/transport/types.ts | 1 + src/cli/handlers/terminal.test.ts | 64 ++++ src/cli/handlers/terminal.ts | 69 +++- src/cli/index.test.ts | 284 +++++++++++----- src/cli/runtime/client.ts | 1 + src/cli/runtime/transport.test.ts | 42 +++ src/cli/runtime/transport.ts | 1 + src/main/daemon/pty-subprocess.test.ts | 41 ++- src/main/daemon/pty-subprocess.ts | 5 + src/main/index.ts | 4 +- src/main/ipc/pty.test.ts | 43 +++ src/main/ipc/pty.ts | 5 + ...ntime-environment-connectivity-handlers.ts | 4 +- ...runtime-environment-request-connections.ts | 6 +- .../runtime-environment-transport-routing.ts | 12 +- src/main/ipc/runtime-environments.test.ts | 48 ++- src/main/ipc/runtime-environments.ts | 4 +- src/main/persistence.test.ts | 44 +++ src/main/persistence.ts | 28 +- src/main/providers/local-pty-provider.test.ts | 18 +- src/main/providers/local-pty-provider.ts | 3 + src/main/providers/provider-dispatch.test.ts | 2 + src/main/pty/legacy-terminal-shim-dir.test.ts | 295 ++++++++++++---- src/main/pty/legacy-terminal-shim-dir.ts | 315 +++++++++++++++--- src/main/runtime/rpc/core.ts | 1 + .../rpc/dispatcher-computer-errors.test.ts | 22 +- src/main/runtime/rpc/dispatcher.ts | 21 ++ .../rpc/methods/terminal-split-env.test.ts | 37 ++ src/main/runtime/rpc/methods/terminal.ts | 2 + src/main/runtime/rpc/streaming.test.ts | 22 ++ src/preload/api-types.ts | 2 + src/preload/index.ts | 2 + .../runtime-environment-subscriptions.test.ts | 9 +- .../runtime-environment-subscriptions.ts | 2 + src/relay/pty-handler.test.ts | 52 ++- src/relay/pty-handler.ts | 3 + .../terminal-pane/pty-transport.test.ts | 60 +++- .../remote-runtime-pty-transport.test.ts | 193 +++++++++-- .../remote-runtime-pty-transport.ts | 121 +++++-- ...ch-agent-background-session-remote.test.ts | 5 +- .../lib/runtime-agent-background-create.ts | 26 +- .../worktree-activation-created-agent.test.ts | 74 +++- .../worktree-activation-empty-remote.test.ts | 65 +++- .../abortable-runtime-environment-call.ts | 12 +- .../remote-agent-session-launch.test.ts | 136 +++++--- .../runtime/remote-agent-session-launch.ts | 71 ++-- .../runtime/runtime-environment-authority.ts | 46 +++ ...untime-rpc-client-pairing-revision.test.ts | 60 +++- .../src/runtime/runtime-rpc-client.ts | 6 +- .../runtime-rpc-environment-call.test.ts | 28 ++ .../runtime/runtime-rpc-environment-call.ts | 7 +- .../src/runtime/web-runtime-session.test.ts | 215 +++++++++--- .../src/runtime/web-runtime-session.ts | 102 ++++-- .../legacy-terminal-attribution-env.test.ts | 99 ++++++ src/shared/legacy-terminal-attribution-env.ts | 68 ++++ src/shared/protocol-version.ts | 4 + src/shared/remote-runtime-client.test.ts | 29 +- src/shared/remote-runtime-client.ts | 6 +- src/shared/remote-runtime-memory-limits.ts | 1 + ...remote-runtime-shared-control-admission.ts | 11 +- ...emote-runtime-shared-control-connection.ts | 22 +- .../remote-runtime-shared-control-send.ts | 3 +- ...ntime-shared-control-subscription-start.ts | 5 +- ...ntime-shared-control-subscriptions.test.ts | 34 ++ ...te-runtime-shared-control-subscriptions.ts | 2 + .../remote-runtime-shared-control-types.ts | 1 + src/shared/runtime-rpc-envelope.ts | 2 + 78 files changed, 2934 insertions(+), 565 deletions(-) create mode 100644 mobile/src/session/mobile-terminal-attribution-compat.test.ts create mode 100644 mobile/src/session/mobile-terminal-attribution-compat.ts create mode 100644 src/main/runtime/rpc/methods/terminal-split-env.test.ts create mode 100644 src/renderer/src/runtime/runtime-environment-authority.ts create mode 100644 src/renderer/src/runtime/runtime-rpc-environment-call.test.ts create mode 100644 src/shared/legacy-terminal-attribution-env.test.ts create mode 100644 src/shared/legacy-terminal-attribution-env.ts diff --git a/mobile/app/h/[hostId]/session/[worktreeId].tsx b/mobile/app/h/[hostId]/session/[worktreeId].tsx index cf0f1dd7edb..f0aebf02efc 100644 --- a/mobile/app/h/[hostId]/session/[worktreeId].tsx +++ b/mobile/app/h/[hostId]/session/[worktreeId].tsx @@ -127,6 +127,15 @@ import type { TerminalLiveInputSender } from '../../../../src/terminal/terminal- import { isTerminalSendRpcAccepted } from '../../../../src/terminal/terminal-send-rpc-response' import { sendMobileTerminalQueryReply } from '../../../../src/terminal/mobile-terminal-query-reply' import { TERMINAL_QUERY_REPLY_INPUT_RUNTIME_CAPABILITY } from '../../../../../src/shared/protocol-version' +import { + addLegacyTerminalAttributionDisableRequest, + MOBILE_TERMINAL_CREATE_ATTRIBUTION_UPDATE_REQUIRED_MESSAGE, + withLegacyTerminalAttributionDisabledEnv +} from '../../../../../src/shared/legacy-terminal-attribution-env' +import { + assertMobileTerminalAttributionDisableSupported, + MOBILE_TERMINAL_CREATE_RPC_OPTIONS +} from '../../../../src/session/mobile-terminal-attribution-compat' import { useTerminalLiveInputCommit } from '../../../../src/terminal/use-terminal-live-input-commit' import { resolveMobileTerminalInputGate } from '../../../../src/terminal/terminal-input-connection-gate' import { @@ -3650,20 +3659,27 @@ export default function SessionScreen() { .slice(2, 10)}` try { - const response = await client.sendRequest('session.tabs.createTerminal', { - worktree: `id:${worktreeId}`, - afterTabId: activeSessionTabId ?? undefined, - clientMutationId, - ...(options?.startupCommand ? { command: options.startupCommand } : {}), - ...(options?.startupCommandDelivery - ? { startupCommandDelivery: options.startupCommandDelivery } - : {}), - ...(options?.agentPrompt ? { agentPrompt: options.agentPrompt } : {}), - ...(agent ? { agent } : {}), - activate: false, - select: true, - navigation: 'caller' - }) + const authority = await assertMobileTerminalAttributionDisableSupported(client) + const response = await client.sendRequest( + 'session.tabs.createTerminal', + { + worktree: `id:${worktreeId}`, + afterTabId: activeSessionTabId ?? undefined, + clientMutationId, + ...(options?.startupCommand ? { command: options.startupCommand } : {}), + ...(options?.startupCommandDelivery + ? { startupCommandDelivery: options.startupCommandDelivery } + : {}), + env: withLegacyTerminalAttributionDisabledEnv(undefined), + envToDelete: addLegacyTerminalAttributionDisableRequest(undefined), + ...(options?.agentPrompt ? { agentPrompt: options.agentPrompt } : {}), + ...(agent ? { agent } : {}), + activate: false, + select: true, + navigation: 'caller' + }, + { ...MOBILE_TERMINAL_CREATE_RPC_OPTIONS, expectedRuntimeId: authority.runtimeId } + ) if (response.ok) { const result = (response as RpcSuccess).result as TerminalCreateResult const created = result.tab @@ -3763,10 +3779,17 @@ export default function SessionScreen() { showToast(message, 1800) } } - } catch { - const message = options?.errorToast ?? 'Failed to create terminal' + } catch (error) { + const errorMessage = error instanceof Error ? error.message : null + const message = + errorMessage === MOBILE_TERMINAL_CREATE_ATTRIBUTION_UPDATE_REQUIRED_MESSAGE + ? errorMessage + : (options?.errorToast ?? errorMessage ?? 'Failed to create terminal') setCreateError(message) - if (options?.errorToast) { + if ( + options?.errorToast || + errorMessage === MOBILE_TERMINAL_CREATE_ATTRIBUTION_UPDATE_REQUIRED_MESSAGE + ) { triggerError() showToast(message, 1800) } diff --git a/mobile/src/session/ai-vault-resume-launch.test.ts b/mobile/src/session/ai-vault-resume-launch.test.ts index 9efc0e50f1a..3e9af99191c 100644 --- a/mobile/src/session/ai-vault-resume-launch.test.ts +++ b/mobile/src/session/ai-vault-resume-launch.test.ts @@ -226,6 +226,13 @@ describe('resumeAiVaultSessionInTerminal', () => { it('creates a fresh terminal and sends the command with Enter', async () => { const sendRequest = vi .fn() + .mockResolvedValueOnce({ + ok: true, + result: { + runtimeId: 'runtime', + capabilities: ['terminal.attribution-removed.v1'] + } + }) .mockResolvedValueOnce({ ok: true, result: { tab: { type: 'terminal', id: 'tab-1', terminal: 'pty-1', title: 'Terminal' } } @@ -249,13 +256,20 @@ describe('resumeAiVaultSessionInTerminal', () => { navigation: 'caller' }) ).resolves.toMatchObject({ id: 'tab-1', terminal: 'pty-1' }) + expect(sendRequest).toHaveBeenNthCalledWith(1, 'status.get', undefined, { + timeoutMs: 30_000, + budgetSpansConnect: true + }) expect(sendRequest).toHaveBeenNthCalledWith( - 1, + 2, 'session.tabs.createTerminal', { worktree: 'id:worktree-1', - env: { ANTHROPIC_BASE_URL: 'http://localhost:3000' }, - envToDelete: ['CODEX_HOME', 'ORCA_CODEX_HOME'], + env: { + ANTHROPIC_BASE_URL: 'http://localhost:3000', + ORCA_ATTRIBUTION_BYPASS: '1' + }, + envToDelete: ['CODEX_HOME', 'ORCA_CODEX_HOME', 'ORCA_ENABLE_GIT_ATTRIBUTION'], launchConfig: { agentCommand: 'claude', agentArgs: '', @@ -269,10 +283,14 @@ describe('resumeAiVaultSessionInTerminal', () => { }, // Why: a socket drop mid-resume must reject within the request timeout // instead of parking on the reconnect waiter with the spinner pinned. - { timeoutMs: RESUME_RPC_TIMEOUT_MS } + { + timeoutMs: RESUME_RPC_TIMEOUT_MS, + budgetSpansConnect: true, + expectedRuntimeId: 'runtime' + } ) expect(sendRequest).toHaveBeenNthCalledWith( - 2, + 3, 'terminal.send', { terminal: 'pty-1', @@ -284,17 +302,35 @@ describe('resumeAiVaultSessionInTerminal', () => { }) it('throws when terminal creation fails', async () => { - const sendRequest = vi.fn().mockResolvedValueOnce({ - ok: false, - error: { message: 'no terminal' } - }) + const sendRequest = vi + .fn() + .mockResolvedValueOnce({ + ok: true, + result: { + runtimeId: 'runtime', + capabilities: ['terminal.attribution-removed.v1'] + } + }) + .mockResolvedValueOnce({ + ok: false, + error: { message: 'no terminal' } + }) await expect( resumeAiVaultSessionInTerminal({ sendRequest }, 'worktree-1', { command: 'command' }) ).rejects.toThrow('no terminal') }) it('throws when the created terminal response is malformed', async () => { - const sendRequest = vi.fn().mockResolvedValueOnce({ ok: true, result: { tab: { id: 'x' } } }) + const sendRequest = vi + .fn() + .mockResolvedValueOnce({ + ok: true, + result: { + runtimeId: 'runtime', + capabilities: ['terminal.attribution-removed.v1'] + } + }) + .mockResolvedValueOnce({ ok: true, result: { tab: { id: 'x' } } }) await expect( resumeAiVaultSessionInTerminal({ sendRequest }, 'worktree-1', { command: 'command' }) ).rejects.toThrow('Created terminal response was invalid') @@ -303,6 +339,13 @@ describe('resumeAiVaultSessionInTerminal', () => { it('throws when terminal send fails or is locked', async () => { const failedSend = vi .fn() + .mockResolvedValueOnce({ + ok: true, + result: { + runtimeId: 'runtime', + capabilities: ['terminal.attribution-removed.v1'] + } + }) .mockResolvedValueOnce({ ok: true, result: { tab: { type: 'terminal', id: 'tab-1', terminal: 'pty-1' } } @@ -316,6 +359,13 @@ describe('resumeAiVaultSessionInTerminal', () => { const lockedSend = vi .fn() + .mockResolvedValueOnce({ + ok: true, + result: { + runtimeId: 'runtime', + capabilities: ['terminal.attribution-removed.v1'] + } + }) .mockResolvedValueOnce({ ok: true, result: { tab: { type: 'terminal', id: 'tab-1', terminal: 'pty-1' } } @@ -327,6 +377,17 @@ describe('resumeAiVaultSessionInTerminal', () => { }) ).rejects.toThrow('Terminal input is locked') }) + + it('refuses an old host before creating a resume terminal', async () => { + const sendRequest = vi + .fn() + .mockResolvedValueOnce({ ok: true, result: { appVersion: '1.4.89' } }) + + await expect( + resumeAiVaultSessionInTerminal({ sendRequest }, 'worktree-1', { command: 'command' }) + ).rejects.toThrow('Update the host and try again') + expect(sendRequest).toHaveBeenCalledTimes(1) + }) }) describe('createMobileAiVaultResumeMutationRegistry', () => { diff --git a/mobile/src/session/ai-vault-resume-launch.ts b/mobile/src/session/ai-vault-resume-launch.ts index aa8c1bf98a6..6975b6f355d 100644 --- a/mobile/src/session/ai-vault-resume-launch.ts +++ b/mobile/src/session/ai-vault-resume-launch.ts @@ -16,6 +16,10 @@ import { normalizeAiVaultResumeFilePath } from '../../../src/shared/ai-vault-res import type { TuiAgent } from '../../../src/shared/types' import { parseWslUncPath } from '../../../src/shared/wsl-paths' import { resolveWindowsShellStartupFamily } from '../../../src/shared/windows-terminal-shell' +import { + addLegacyTerminalAttributionDisableRequest, + withLegacyTerminalAttributionDisabledEnv +} from '../../../src/shared/legacy-terminal-attribution-env' import type { RpcClient } from '../transport/rpc-client' import { readMobileReviewCreatedTerminal, @@ -23,6 +27,7 @@ import { type MobileReviewTerminalTab } from './mobile-diff-review-rpc' import type { MobileAiVaultResumeTargetStatus } from '../agent-history/agent-history-resume-target' +import { assertMobileTerminalAttributionDisableSupported } from './mobile-terminal-attribution-compat' const NODE_PLATFORMS = new Set([ 'aix', @@ -169,12 +174,13 @@ export async function resumeAiVaultSessionInTerminal( worktreeId: string, launch: MobileAiVaultResumeLaunch & { clientMutationId?: string } ): Promise { + const authority = await assertMobileTerminalAttributionDisableSupported(client) const created = await client.sendRequest( 'session.tabs.createTerminal', { worktree: `id:${worktreeId}`, - ...(launch.env ? { env: launch.env } : {}), - ...(launch.envToDelete ? { envToDelete: launch.envToDelete } : {}), + env: withLegacyTerminalAttributionDisabledEnv(launch.env), + envToDelete: addLegacyTerminalAttributionDisableRequest(launch.envToDelete), ...(launch.launchConfig ? { launchConfig: launch.launchConfig } : {}), ...(launch.launchAgent ? { launchAgent: launch.launchAgent } : {}), ...(launch.clientMutationId ? { clientMutationId: launch.clientMutationId } : {}), @@ -182,7 +188,11 @@ export async function resumeAiVaultSessionInTerminal( select: true, navigation: 'caller' }, - { timeoutMs: RESUME_RPC_TIMEOUT_MS } + { + timeoutMs: RESUME_RPC_TIMEOUT_MS, + budgetSpansConnect: true, + expectedRuntimeId: authority.runtimeId + } ) if (!created.ok) { throw new Error(created.error?.message || 'Failed to create terminal') diff --git a/mobile/src/session/ai-vault-resume-preparation.test.ts b/mobile/src/session/ai-vault-resume-preparation.test.ts index a0eed5515c6..3e64a06a0f8 100644 --- a/mobile/src/session/ai-vault-resume-preparation.test.ts +++ b/mobile/src/session/ai-vault-resume-preparation.test.ts @@ -53,6 +53,14 @@ describe('prepareMobileAiVaultSessionResume', () => { const sendRequest = vi .fn() .mockResolvedValueOnce({ ok: false, error }) + .mockResolvedValueOnce({ + ok: true, + result: { + runtimeId: 'runtime-1', + appVersion: '1.4.90', + capabilities: ['terminal.attribution-removed.v1'] + } + }) .mockResolvedValueOnce({ ok: true, result: { tab: { type: 'terminal', id: 'tab-1', terminal: 'pty-1', title: 'Terminal' } } @@ -64,9 +72,12 @@ describe('prepareMobileAiVaultSessionResume', () => { await resumeAiVaultSessionInTerminal({ sendRequest }, 'worktree-1', launch) expect(prepared).toBe(legacy) - expect(sendRequest.mock.calls[1]?.[1]).not.toHaveProperty('envToDelete') + expect(sendRequest.mock.calls[2]?.[1]).toMatchObject({ + env: { ORCA_ATTRIBUTION_BYPASS: '1' }, + envToDelete: ['ORCA_ENABLE_GIT_ATTRIBUTION'] + }) expect(sendRequest).toHaveBeenNthCalledWith( - 3, + 4, 'terminal.send', { terminal: 'pty-1', diff --git a/mobile/src/session/mobile-terminal-attribution-compat.test.ts b/mobile/src/session/mobile-terminal-attribution-compat.test.ts new file mode 100644 index 00000000000..9f0ebf873bd --- /dev/null +++ b/mobile/src/session/mobile-terminal-attribution-compat.test.ts @@ -0,0 +1,81 @@ +import { describe, expect, it, vi } from 'vitest' +import { MOBILE_TERMINAL_CREATE_ATTRIBUTION_UPDATE_REQUIRED_MESSAGE } from '../../../src/shared/legacy-terminal-attribution-env' +import { assertMobileTerminalAttributionDisableSupported } from './mobile-terminal-attribution-compat' + +function status(result: unknown) { + return { + id: 'status', + ok: true as const, + result: { runtimeId: 'runtime', ...(result as object) }, + _meta: { runtimeId: 'runtime' } + } +} + +describe('mobile terminal attribution compatibility', () => { + it('refuses hosts before terminal creation environment forwarding', async () => { + const sendRequest = vi.fn().mockResolvedValue( + status({ + capabilities: [ + 'runtime.status.compat.v1', + 'runtime.environments.v1', + 'mobile.tasks.v1', + 'workspace-run-context.v1' + ] + }) + ) + + await expect(assertMobileTerminalAttributionDisableSupported({ sendRequest })).rejects.toThrow( + MOBILE_TERMINAL_CREATE_ATTRIBUTION_UPDATE_REQUIRED_MESSAGE + ) + expect(sendRequest).toHaveBeenCalledWith('status.get', undefined, { + timeoutMs: 30_000, + budgetSpansConnect: true + }) + }) + + it('rejects historical hosts without explicit removal capability', async () => { + const sendRequest = vi.fn().mockResolvedValue(status({ appVersion: '1.4.90' })) + + await expect(assertMobileTerminalAttributionDisableSupported({ sendRequest })).rejects.toThrow( + MOBILE_TERMINAL_CREATE_ATTRIBUTION_UPDATE_REQUIRED_MESSAGE + ) + }) + + it('accepts capability-proven current development hosts', async () => { + const sendRequest = vi + .fn() + .mockResolvedValue( + status({ appVersion: '0.0.0-dev', capabilities: ['terminal.attribution-removed.v1'] }) + ) + + await expect(assertMobileTerminalAttributionDisableSupported({ sendRequest })).resolves.toEqual( + { runtimeId: 'runtime' } + ) + }) + + it('fails closed when host status cannot be verified', async () => { + const sendRequest = vi.fn().mockResolvedValue({ + id: 'status', + ok: false, + error: { code: 'offline', message: 'Host is offline' }, + _meta: { runtimeId: 'runtime' } + }) + + await expect(assertMobileTerminalAttributionDisableSupported({ sendRequest })).rejects.toThrow( + 'Host is offline' + ) + }) + + it('fails closed with an actionable error for malformed host status', async () => { + const sendRequest = vi.fn().mockResolvedValue( + status({ + appVersion: 1.49, + capabilities: 'terminal.attribution-removed.v1' + }) + ) + + await expect(assertMobileTerminalAttributionDisableSupported({ sendRequest })).rejects.toThrow( + 'Update the host and try again' + ) + }) +}) diff --git a/mobile/src/session/mobile-terminal-attribution-compat.ts b/mobile/src/session/mobile-terminal-attribution-compat.ts new file mode 100644 index 00000000000..ceb2d3f694e --- /dev/null +++ b/mobile/src/session/mobile-terminal-attribution-compat.ts @@ -0,0 +1,34 @@ +import { + hostSupportsSessionTabTerminalCreateAttributionDisable, + MOBILE_TERMINAL_CREATE_ATTRIBUTION_UPDATE_REQUIRED_MESSAGE +} from '../../../src/shared/legacy-terminal-attribution-env' +import type { RpcClient } from '../transport/rpc-client' + +export const MOBILE_TERMINAL_CREATE_RPC_TIMEOUT_MS = 30_000 +export const MOBILE_TERMINAL_CREATE_RPC_OPTIONS = { + timeoutMs: MOBILE_TERMINAL_CREATE_RPC_TIMEOUT_MS, + budgetSpansConnect: true +} as const + +export type MobileTerminalAttributionAuthority = Readonly<{ runtimeId: string }> + +export async function assertMobileTerminalAttributionDisableSupported( + client: Pick +): Promise { + const response = await client.sendRequest( + 'status.get', + undefined, + MOBILE_TERMINAL_CREATE_RPC_OPTIONS + ) + if (!response.ok) { + throw new Error(response.error?.message || 'Unable to verify the workspace host version.') + } + if (!hostSupportsSessionTabTerminalCreateAttributionDisable(response.result)) { + throw new Error(MOBILE_TERMINAL_CREATE_ATTRIBUTION_UPDATE_REQUIRED_MESSAGE) + } + const runtimeId = Reflect.get(response.result as object, 'runtimeId') + if (typeof runtimeId !== 'string' || !runtimeId) { + throw new Error(MOBILE_TERMINAL_CREATE_ATTRIBUTION_UPDATE_REQUIRED_MESSAGE) + } + return Object.freeze({ runtimeId }) +} diff --git a/mobile/src/session/pr-ai-triage-launch.test.ts b/mobile/src/session/pr-ai-triage-launch.test.ts index c2273c9046e..fe9271f5e4d 100644 --- a/mobile/src/session/pr-ai-triage-launch.test.ts +++ b/mobile/src/session/pr-ai-triage-launch.test.ts @@ -12,6 +12,10 @@ function failure(message: string): RpcResponse { const createdTerminal = success({ tab: { type: 'terminal', id: 't1', terminal: 'term-1' } }) const sendAccepted = success({ send: { accepted: true } }) +const safeHost = success({ + runtimeId: 'r', + capabilities: ['terminal.attribution-removed.v1'] +}) function clientReturning(...responses: RpcResponse[]) { const sendRequest = vi.fn(async () => responses[sendRequest.mock.calls.length - 1]) @@ -20,16 +24,27 @@ function clientReturning(...responses: RpcResponse[]) { describe('createTerminalAndSendPrompt', () => { it('creates a terminal then sends the prompt with enter', async () => { - const client = clientReturning(createdTerminal, sendAccepted) + const client = clientReturning(safeHost, createdTerminal, sendAccepted) await createTerminalAndSendPrompt(client, 'wt-1', 'do the thing') - expect(client.sendRequest).toHaveBeenNthCalledWith(1, 'session.tabs.createTerminal', { - worktree: 'id:wt-1', - activate: false, - select: true, - navigation: 'caller' + expect(client.sendRequest).toHaveBeenNthCalledWith(1, 'status.get', undefined, { + timeoutMs: 30_000, + budgetSpansConnect: true }) - expect(client.sendRequest).toHaveBeenNthCalledWith(2, 'terminal.send', { + expect(client.sendRequest).toHaveBeenNthCalledWith( + 2, + 'session.tabs.createTerminal', + { + worktree: 'id:wt-1', + env: { ORCA_ATTRIBUTION_BYPASS: '1' }, + envToDelete: ['ORCA_ENABLE_GIT_ATTRIBUTION'], + activate: false, + select: true, + navigation: 'caller' + }, + { timeoutMs: 30_000, budgetSpansConnect: true, expectedRuntimeId: 'r' } + ) + expect(client.sendRequest).toHaveBeenNthCalledWith(3, 'terminal.send', { terminal: 'term-1', text: 'do the thing', enter: true @@ -37,28 +52,41 @@ describe('createTerminalAndSendPrompt', () => { }) it('throws and skips terminal.send when createTerminal fails', async () => { - const client = clientReturning(failure('boom')) + const client = clientReturning(safeHost, failure('boom')) await expect(createTerminalAndSendPrompt(client, 'wt-1', 'p')).rejects.toThrow('boom') - expect(client.sendRequest).toHaveBeenCalledTimes(1) + expect(client.sendRequest).toHaveBeenCalledTimes(2) }) it('throws when the created-terminal response is malformed', async () => { - const client = clientReturning(success({ tab: { type: 'terminal' } })) + const client = clientReturning(safeHost, success({ tab: { type: 'terminal' } })) await expect(createTerminalAndSendPrompt(client, 'wt-1', 'p')).rejects.toThrow( 'Created terminal response was invalid' ) - expect(client.sendRequest).toHaveBeenCalledTimes(1) + expect(client.sendRequest).toHaveBeenCalledTimes(2) }) it('throws when terminal.send returns a failure', async () => { - const client = clientReturning(createdTerminal, failure('send failed')) + const client = clientReturning(safeHost, createdTerminal, failure('send failed')) await expect(createTerminalAndSendPrompt(client, 'wt-1', 'p')).rejects.toThrow('send failed') }) it('throws when terminal input is locked', async () => { - const client = clientReturning(createdTerminal, success({ send: { accepted: false } })) + const client = clientReturning( + safeHost, + createdTerminal, + success({ send: { accepted: false } }) + ) await expect(createTerminalAndSendPrompt(client, 'wt-1', 'p')).rejects.toThrow( 'Terminal input is locked' ) }) + + it('refuses an old host before terminal creation', async () => { + const client = clientReturning(success({ appVersion: '1.4.89' })) + + await expect(createTerminalAndSendPrompt(client, 'wt-1', 'p')).rejects.toThrow( + 'Update the host and try again' + ) + expect(client.sendRequest).toHaveBeenCalledTimes(1) + }) }) diff --git a/mobile/src/session/pr-ai-triage-launch.ts b/mobile/src/session/pr-ai-triage-launch.ts index 6d6c0bbc8b2..1f764be1789 100644 --- a/mobile/src/session/pr-ai-triage-launch.ts +++ b/mobile/src/session/pr-ai-triage-launch.ts @@ -3,6 +3,14 @@ import { readMobileReviewCreatedTerminal, readMobileReviewTerminalSendAccepted } from './mobile-diff-review-rpc' +import { + addLegacyTerminalAttributionDisableRequest, + withLegacyTerminalAttributionDisabledEnv +} from '../../../src/shared/legacy-terminal-attribution-env' +import { + assertMobileTerminalAttributionDisableSupported, + MOBILE_TERMINAL_CREATE_RPC_OPTIONS +} from './mobile-terminal-attribution-compat' // Pure launch path for the PR triage actions ("Fix checks with AI" / "Resolve // conflicts with AI"). Reuses the same two RPCs the diff-review send flow uses — @@ -15,12 +23,19 @@ export async function createTerminalAndSendPrompt( worktreeId: string, prompt: string ): Promise { - const created = await client.sendRequest('session.tabs.createTerminal', { - worktree: `id:${worktreeId}`, - activate: false, - select: true, - navigation: 'caller' - }) + const authority = await assertMobileTerminalAttributionDisableSupported(client) + const created = await client.sendRequest( + 'session.tabs.createTerminal', + { + worktree: `id:${worktreeId}`, + env: withLegacyTerminalAttributionDisabledEnv(undefined), + envToDelete: addLegacyTerminalAttributionDisableRequest(undefined), + activate: false, + select: true, + navigation: 'caller' + }, + { ...MOBILE_TERMINAL_CREATE_RPC_OPTIONS, expectedRuntimeId: authority.runtimeId } + ) if (!created.ok) { throw new Error(created.error?.message || 'Failed to create terminal') } diff --git a/mobile/src/session/use-mobile-diff-review-send-actions.test.ts b/mobile/src/session/use-mobile-diff-review-send-actions.test.ts index 1d06190948c..9ae2749412b 100644 --- a/mobile/src/session/use-mobile-diff-review-send-actions.test.ts +++ b/mobile/src/session/use-mobile-diff-review-send-actions.test.ts @@ -163,6 +163,74 @@ describe('useMobileDiffReviewSendActions', () => { expect(setSendSheet).toHaveBeenCalledWith(null) }) + it('creates an unattributed terminal before sending review notes', async () => { + const sendRequest = vi + .fn() + .mockResolvedValueOnce({ + id: 'status', + ok: true, + result: { + runtimeId: 'runtime', + capabilities: ['terminal.attribution-removed.v1'] + }, + _meta: { runtimeId: 'runtime' } + }) + .mockResolvedValueOnce({ + id: 'create', + ok: true, + result: { tab: { type: 'terminal', id: 'tab-1', terminal: 'terminal-1' } }, + _meta: { runtimeId: 'runtime' } + }) + .mockResolvedValueOnce(sendResponse(true)) + await mount({ sendRequest } as unknown as RpcClient) + + await act(async () => { + await actions?.createTerminalAndSend([COMMENT]) + }) + + expect(sendRequest).toHaveBeenNthCalledWith(1, 'status.get', undefined, { + timeoutMs: 30_000, + budgetSpansConnect: true + }) + expect(sendRequest).toHaveBeenNthCalledWith( + 2, + 'session.tabs.createTerminal', + { + worktree: 'id:wt-1', + env: { ORCA_ATTRIBUTION_BYPASS: '1' }, + envToDelete: ['ORCA_ENABLE_GIT_ATTRIBUTION'], + activate: false, + select: true, + navigation: 'caller' + }, + { timeoutMs: 30_000, budgetSpansConnect: true, expectedRuntimeId: 'runtime' } + ) + expect(sendRequest).toHaveBeenNthCalledWith( + 3, + 'terminal.send', + expect.objectContaining({ terminal: 'terminal-1', enter: true }) + ) + }) + + it('refuses to create review terminals on hosts that strip the bypass', async () => { + const sendRequest = vi.fn().mockResolvedValue({ + id: 'status', + ok: true, + result: { appVersion: '1.4.89' }, + _meta: { runtimeId: 'runtime' } + }) + await mount({ sendRequest } as unknown as RpcClient) + + let error: unknown + await act(async () => { + error = await actions?.createTerminalAndSend([COMMENT]).catch((err) => err) + }) + + expect((error as Error).message).toContain('Update the host and try again') + expect(sendRequest).toHaveBeenCalledTimes(1) + expect(sendRequest).not.toHaveBeenCalledWith('terminal.send', expect.anything()) + }) + it('only heals the terminal that was marked', async () => { const sendRequest = vi.fn().mockResolvedValue(sendResponse(true)) await mount({ sendRequest } as unknown as RpcClient) diff --git a/mobile/src/session/use-mobile-diff-review-send-actions.ts b/mobile/src/session/use-mobile-diff-review-send-actions.ts index 0867a3884b3..266caaf6879 100644 --- a/mobile/src/session/use-mobile-diff-review-send-actions.ts +++ b/mobile/src/session/use-mobile-diff-review-send-actions.ts @@ -13,6 +13,14 @@ import { } from './mobile-diff-review-rpc' import { healMobileNativeChatStaleInput } from './mobile-native-chat-stale-input' import type { ReviewScreenState, SendSheetState } from './mobile-diff-review-screen-model' +import { + addLegacyTerminalAttributionDisableRequest, + withLegacyTerminalAttributionDisabledEnv +} from '../../../src/shared/legacy-terminal-attribution-env' +import { + assertMobileTerminalAttributionDisableSupported, + MOBILE_TERMINAL_CREATE_RPC_OPTIONS +} from './mobile-terminal-attribution-compat' type SendActionsInput = { client: RpcClient | null @@ -104,12 +112,19 @@ export function useMobileDiffReviewSendActions(input: SendActionsInput) { if (!client || connState !== 'connected') { throw new Error('Waiting for desktop...') } - const response = await client.sendRequest('session.tabs.createTerminal', { - worktree: `id:${worktreeId}`, - activate: false, - select: true, - navigation: 'caller' - }) + const authority = await assertMobileTerminalAttributionDisableSupported(client) + const response = await client.sendRequest( + 'session.tabs.createTerminal', + { + worktree: `id:${worktreeId}`, + env: withLegacyTerminalAttributionDisabledEnv(undefined), + envToDelete: addLegacyTerminalAttributionDisableRequest(undefined), + activate: false, + select: true, + navigation: 'caller' + }, + { ...MOBILE_TERMINAL_CREATE_RPC_OPTIONS, expectedRuntimeId: authority.runtimeId } + ) if (!response.ok) { throw new Error(response.error?.message || 'Failed to create terminal') } diff --git a/mobile/src/transport/rpc-client.ts b/mobile/src/transport/rpc-client.ts index 9b0afe937f3..f316f8c2b76 100644 --- a/mobile/src/transport/rpc-client.ts +++ b/mobile/src/transport/rpc-client.ts @@ -54,6 +54,7 @@ type ConnectWaiter = { export type SendRequestOptions = { timeoutMs?: number + expectedRuntimeId?: string /** Spend `timeoutMs` across connect-wait AND the request instead of giving each * phase its own. Interactive chat writes need it: they run as sequential loops * under one shared budget, so a per-phase clock lets the composer sit `sending` @@ -1013,7 +1014,15 @@ export function connect( } }) - if (!sendEncrypted({ id, deviceToken, method, params })) { + if ( + !sendEncrypted({ + id, + deviceToken, + method, + params, + ...(options?.expectedRuntimeId ? { expectedRuntimeId: options.expectedRuntimeId } : {}) + }) + ) { pending.delete(id) clearTimeout(timeout) reject(new Error('Connection interrupted')) diff --git a/mobile/src/transport/types.ts b/mobile/src/transport/types.ts index 9d96f0561ad..8fe2bdd78f9 100644 --- a/mobile/src/transport/types.ts +++ b/mobile/src/transport/types.ts @@ -18,6 +18,7 @@ export type RpcRequest = { deviceToken: string method: string params?: unknown + expectedRuntimeId?: string } export type RpcSuccess = { diff --git a/src/cli/handlers/terminal.test.ts b/src/cli/handlers/terminal.test.ts index 7dadf41c2d3..8c5ff460c77 100644 --- a/src/cli/handlers/terminal.test.ts +++ b/src/cli/handlers/terminal.test.ts @@ -61,3 +61,67 @@ describe('terminal close CLI', () => { expect(help).toContain('durable persistence') }) }) + +describe('terminal split CLI', () => { + afterEach(() => { + vi.restoreAllMocks() + }) + + it('sends the old-host attribution bypass through the legacy-compatible env field', async () => { + const call = vi.fn(async (method: string) => + method === 'status.get' + ? { + result: { + runtimeId: 'runtime-1', + capabilities: ['terminal.attribution-removed.v1'] + } + } + : { result: { split: { handle: 'term-2', parentHandle: 'term-1' } } } + ) + vi.spyOn(console, 'log').mockImplementation(() => {}) + + await TERMINAL_HANDLERS['terminal split']({ + flags: new Map([ + ['terminal', 'term-1'], + ['direction', 'horizontal'] + ]), + client: { call } as unknown as RuntimeClient, + cwd: '/tmp/worktree', + json: true + }) + + expect(call).toHaveBeenNthCalledWith(1, 'status.get') + expect(call).toHaveBeenNthCalledWith( + 2, + 'terminal.split', + { + terminal: 'term-1', + direction: 'horizontal', + command: undefined, + env: { ORCA_ATTRIBUTION_BYPASS: '1' }, + envToDelete: ['ORCA_ENABLE_GIT_ATTRIBUTION'] + }, + { expectedRuntimeId: 'runtime-1' } + ) + }) + + it('refuses legacy hosts because renderer-owned splits discard environment fields', async () => { + const call = vi.fn().mockResolvedValue({ + result: { appVersion: '1.4.181', capabilities: ['mobile.tasks.v1'] } + }) + + await expect( + TERMINAL_HANDLERS['terminal split']({ + flags: new Map([['terminal', 'term-1']]), + client: { call } as unknown as RuntimeClient, + cwd: '/tmp/worktree', + json: true + }) + ).rejects.toMatchObject({ + code: 'runtime_update_required', + message: expect.stringContaining('Update the host and try again') + }) + expect(call).toHaveBeenCalledTimes(1) + expect(call).not.toHaveBeenCalledWith('terminal.split', expect.anything()) + }) +}) diff --git a/src/cli/handlers/terminal.ts b/src/cli/handlers/terminal.ts index 551ab8d95c1..677cdc81034 100644 --- a/src/cli/handlers/terminal.ts +++ b/src/cli/handlers/terminal.ts @@ -8,7 +8,8 @@ import type { RuntimeTerminalSend, RuntimeTerminalShow, RuntimeTerminalSplit, - RuntimeTerminalWait + RuntimeTerminalWait, + RuntimeStatus } from '../../shared/runtime-types' import type { CommandHandler } from '../dispatch' import { shouldUseRendererBackedInteractiveTerminal } from '../codex-command-classification' @@ -37,6 +38,14 @@ import { getRequiredWorktreeSelector, getTerminalHandle } from '../selectors' +import { + addLegacyTerminalAttributionDisableRequest, + hostSupportsTerminalCreateAttributionDisable, + hostSupportsTerminalSplitAttributionDisable, + TERMINAL_CREATE_ATTRIBUTION_UPDATE_REQUIRED_MESSAGE, + TERMINAL_SPLIT_ATTRIBUTION_UPDATE_REQUIRED_MESSAGE, + withLegacyTerminalAttributionDisabledEnv +} from '../../shared/legacy-terminal-attribution-env' // Why: terminal wait legitimately needs to outlive the CLI's default RPC // timeout. Even without an explicit server timeout, the client must allow @@ -137,17 +146,30 @@ export const TERMINAL_HANDLERS: Record = { const useRendererBackedInteractiveTerminal = !client.isRemote && shouldUseRendererBackedInteractiveTerminal(command) const focus = flags.get('focus') === true - const result = await client.call<{ terminal: RuntimeTerminalCreate }>('terminal.create', { - worktree: await getBrowserWorktreeSelector(flags, cwd, client), - command, - title: getOptionalStringFlag(flags, 'title'), - // Why: interactive local agent TUIs need the renderer-backed terminal - // path for browser-side features, but CLI creates must stay backgrounded - // unless the caller explicitly asks for focus. - focus, - ...(focus ? { presentation: 'focused' } : {}), - ...(useRendererBackedInteractiveTerminal ? { rendererBacked: true, activate: focus } : {}) - }) + const status = await client.call('status.get') + if (!hostSupportsTerminalCreateAttributionDisable(status.result)) { + throw new RuntimeClientError( + 'runtime_update_required', + TERMINAL_CREATE_ATTRIBUTION_UPDATE_REQUIRED_MESSAGE + ) + } + const result = await client.call<{ terminal: RuntimeTerminalCreate }>( + 'terminal.create', + { + worktree: await getBrowserWorktreeSelector(flags, cwd, client), + command, + env: withLegacyTerminalAttributionDisabledEnv(undefined), + envToDelete: addLegacyTerminalAttributionDisableRequest(undefined), + title: getOptionalStringFlag(flags, 'title'), + // Why: interactive local agent TUIs need the renderer-backed terminal + // path for browser-side features, but CLI creates must stay backgrounded + // unless the caller explicitly asks for focus. + focus, + ...(focus ? { presentation: 'focused' } : {}), + ...(useRendererBackedInteractiveTerminal ? { rendererBacked: true, activate: focus } : {}) + }, + { expectedRuntimeId: status.result.runtimeId } + ) printResult(result, json, formatTerminalCreate) }, // `focus` resolves to this canonical path via CommandSpec.aliases before dispatch. @@ -168,11 +190,24 @@ export const TERMINAL_HANDLERS: Record = { ) { throw new RuntimeClientError('invalid_argument', '--direction must be horizontal or vertical') } - const result = await client.call<{ split: RuntimeTerminalSplit }>('terminal.split', { - terminal: await getTerminalHandle(flags, cwd, client), - direction: directionFlag, - command: getOptionalStringFlag(flags, 'command') - }) + const status = await client.call('status.get') + if (!hostSupportsTerminalSplitAttributionDisable(status.result)) { + throw new RuntimeClientError( + 'runtime_update_required', + TERMINAL_SPLIT_ATTRIBUTION_UPDATE_REQUIRED_MESSAGE + ) + } + const result = await client.call<{ split: RuntimeTerminalSplit }>( + 'terminal.split', + { + terminal: await getTerminalHandle(flags, cwd, client), + direction: directionFlag, + command: getOptionalStringFlag(flags, 'command'), + env: withLegacyTerminalAttributionDisabledEnv(undefined), + envToDelete: addLegacyTerminalAttributionDisableRequest(undefined) + }, + { expectedRuntimeId: status.result.runtimeId } + ) printResult(result, json, formatTerminalSplit) } } diff --git a/src/cli/index.test.ts b/src/cli/index.test.ts index 4867aebc29b..2f86ee9583f 100644 --- a/src/cli/index.test.ts +++ b/src/cli/index.test.ts @@ -3112,6 +3112,10 @@ describe('orca cli worktree awareness', () => { it('passes explicit focus through terminal.create', async () => { queueFixtures( callMock, + okFixture('req_status', { + runtimeId: 'runtime-1', + capabilities: ['terminal.attribution-removed.v1'] + }), okFixture('req_terminal_create', { terminal: { handle: 'term_1', @@ -3136,13 +3140,19 @@ describe('orca cli worktree awareness', () => { '/tmp/repo' ) - expect(callMock).toHaveBeenCalledWith('terminal.create', { - worktree: 'path:/tmp/repo/feature', - command: undefined, - title: 'RUNNER', - focus: true, - presentation: 'focused' - }) + expect(callMock).toHaveBeenCalledWith( + 'terminal.create', + { + worktree: 'path:/tmp/repo/feature', + command: undefined, + env: { ORCA_ATTRIBUTION_BYPASS: '1' }, + envToDelete: ['ORCA_ENABLE_GIT_ATTRIBUTION'], + title: 'RUNNER', + focus: true, + presentation: 'focused' + }, + { expectedRuntimeId: 'runtime-1' } + ) }) it('prints terminal.read fallback screen lines in json mode', async () => { @@ -3184,6 +3194,10 @@ describe('orca cli worktree awareness', () => { it('keeps interactive Codex startup commands backgrounded unless focus is explicit', async () => { queueFixtures( callMock, + okFixture('req_status', { + runtimeId: 'runtime-1', + capabilities: ['terminal.attribution-removed.v1'] + }), okFixture('req_terminal_create', { terminal: { handle: 'term_1', @@ -3209,19 +3223,29 @@ describe('orca cli worktree awareness', () => { '/tmp/repo' ) - expect(callMock).toHaveBeenCalledWith('terminal.create', { - worktree: 'path:/tmp/repo/feature', - command: 'codex', - title: 'Codex', - focus: false, - rendererBacked: true, - activate: false - }) + expect(callMock).toHaveBeenCalledWith( + 'terminal.create', + { + worktree: 'path:/tmp/repo/feature', + command: 'codex', + env: { ORCA_ATTRIBUTION_BYPASS: '1' }, + envToDelete: ['ORCA_ENABLE_GIT_ATTRIBUTION'], + title: 'Codex', + focus: false, + rendererBacked: true, + activate: false + }, + { expectedRuntimeId: 'runtime-1' } + ) }) it('keeps explicit focus semantics when forcing Codex through the renderer path', async () => { queueFixtures( callMock, + okFixture('req_status', { + runtimeId: 'runtime-1', + capabilities: ['terminal.attribution-removed.v1'] + }), okFixture('req_terminal_create', { terminal: { handle: 'term_1', @@ -3248,20 +3272,30 @@ describe('orca cli worktree awareness', () => { '/tmp/repo' ) - expect(callMock).toHaveBeenCalledWith('terminal.create', { - worktree: 'path:/tmp/repo/feature', - command: 'codex', - title: 'Codex', - focus: true, - presentation: 'focused', - rendererBacked: true, - activate: true - }) + expect(callMock).toHaveBeenCalledWith( + 'terminal.create', + { + worktree: 'path:/tmp/repo/feature', + command: 'codex', + env: { ORCA_ATTRIBUTION_BYPASS: '1' }, + envToDelete: ['ORCA_ENABLE_GIT_ATTRIBUTION'], + title: 'Codex', + focus: true, + presentation: 'focused', + rendererBacked: true, + activate: true + }, + { expectedRuntimeId: 'runtime-1' } + ) }) it('does not force the visible terminal path for explicit Codex exec commands', async () => { queueFixtures( callMock, + okFixture('req_status', { + runtimeId: 'runtime-1', + capabilities: ['terminal.attribution-removed.v1'] + }), okFixture('req_terminal_create', { terminal: { handle: 'term_1', @@ -3287,17 +3321,27 @@ describe('orca cli worktree awareness', () => { '/tmp/repo' ) - expect(callMock).toHaveBeenCalledWith('terminal.create', { - worktree: 'path:/tmp/repo/feature', - command: 'codex exec summarize', - title: 'Codex exec', - focus: false - }) + expect(callMock).toHaveBeenCalledWith( + 'terminal.create', + { + worktree: 'path:/tmp/repo/feature', + command: 'codex exec summarize', + env: { ORCA_ATTRIBUTION_BYPASS: '1' }, + envToDelete: ['ORCA_ENABLE_GIT_ATTRIBUTION'], + title: 'Codex exec', + focus: false + }, + { expectedRuntimeId: 'runtime-1' } + ) }) it('does not force the visible terminal path for Codex exec commands after global options', async () => { queueFixtures( callMock, + okFixture('req_status', { + runtimeId: 'runtime-1', + capabilities: ['terminal.attribution-removed.v1'] + }), okFixture('req_terminal_create', { terminal: { handle: 'term_1', @@ -3323,17 +3367,27 @@ describe('orca cli worktree awareness', () => { '/tmp/repo' ) - expect(callMock).toHaveBeenCalledWith('terminal.create', { - worktree: 'path:/tmp/repo/feature', - command: 'codex -m gpt-5 --sandbox workspace-write exec summarize', - title: 'Codex exec', - focus: false - }) + expect(callMock).toHaveBeenCalledWith( + 'terminal.create', + { + worktree: 'path:/tmp/repo/feature', + command: 'codex -m gpt-5 --sandbox workspace-write exec summarize', + env: { ORCA_ATTRIBUTION_BYPASS: '1' }, + envToDelete: ['ORCA_ENABLE_GIT_ATTRIBUTION'], + title: 'Codex exec', + focus: false + }, + { expectedRuntimeId: 'runtime-1' } + ) }) it('does not force the visible terminal path for Codex review commands after long options', async () => { queueFixtures( callMock, + okFixture('req_status', { + runtimeId: 'runtime-1', + capabilities: ['terminal.attribution-removed.v1'] + }), okFixture('req_terminal_create', { terminal: { handle: 'term_1', @@ -3359,17 +3413,27 @@ describe('orca cli worktree awareness', () => { '/tmp/repo' ) - expect(callMock).toHaveBeenCalledWith('terminal.create', { - worktree: 'path:/tmp/repo/feature', - command: 'codex --model=gpt-5 --sandbox=workspace-write review', - title: 'Codex review', - focus: false - }) + expect(callMock).toHaveBeenCalledWith( + 'terminal.create', + { + worktree: 'path:/tmp/repo/feature', + command: 'codex --model=gpt-5 --sandbox=workspace-write review', + env: { ORCA_ATTRIBUTION_BYPASS: '1' }, + envToDelete: ['ORCA_ENABLE_GIT_ATTRIBUTION'], + title: 'Codex review', + focus: false + }, + { expectedRuntimeId: 'runtime-1' } + ) }) it('does not force the visible terminal path for Codex help commands', async () => { queueFixtures( callMock, + okFixture('req_status', { + runtimeId: 'runtime-1', + capabilities: ['terminal.attribution-removed.v1'] + }), okFixture('req_terminal_create', { terminal: { handle: 'term_1', @@ -3395,17 +3459,27 @@ describe('orca cli worktree awareness', () => { '/tmp/repo' ) - expect(callMock).toHaveBeenCalledWith('terminal.create', { - worktree: 'path:/tmp/repo/feature', - command: 'codex --help', - title: 'Codex help', - focus: false - }) + expect(callMock).toHaveBeenCalledWith( + 'terminal.create', + { + worktree: 'path:/tmp/repo/feature', + command: 'codex --help', + env: { ORCA_ATTRIBUTION_BYPASS: '1' }, + envToDelete: ['ORCA_ENABLE_GIT_ATTRIBUTION'], + title: 'Codex help', + focus: false + }, + { expectedRuntimeId: 'runtime-1' } + ) }) it('keeps Codex prompts after global options backgrounded unless focus is explicit', async () => { queueFixtures( callMock, + okFixture('req_status', { + runtimeId: 'runtime-1', + capabilities: ['terminal.attribution-removed.v1'] + }), okFixture('req_terminal_create', { terminal: { handle: 'term_1', @@ -3431,19 +3505,29 @@ describe('orca cli worktree awareness', () => { '/tmp/repo' ) - expect(callMock).toHaveBeenCalledWith('terminal.create', { - worktree: 'path:/tmp/repo/feature', - command: 'codex -m gpt-5 "fix the flaky test"', - title: 'Codex prompt', - focus: false, - rendererBacked: true, - activate: false - }) + expect(callMock).toHaveBeenCalledWith( + 'terminal.create', + { + worktree: 'path:/tmp/repo/feature', + command: 'codex -m gpt-5 "fix the flaky test"', + env: { ORCA_ATTRIBUTION_BYPASS: '1' }, + envToDelete: ['ORCA_ENABLE_GIT_ATTRIBUTION'], + title: 'Codex prompt', + focus: false, + rendererBacked: true, + activate: false + }, + { expectedRuntimeId: 'runtime-1' } + ) }) it('keeps interactive Claude startup commands backgrounded unless focus is explicit', async () => { queueFixtures( callMock, + okFixture('req_status', { + runtimeId: 'runtime-1', + capabilities: ['terminal.attribution-removed.v1'] + }), okFixture('req_terminal_create', { terminal: { handle: 'term_1', @@ -3469,19 +3553,29 @@ describe('orca cli worktree awareness', () => { '/tmp/repo' ) - expect(callMock).toHaveBeenCalledWith('terminal.create', { - worktree: 'path:/tmp/repo/feature', - command: 'claude', - title: 'Claude', - focus: false, - rendererBacked: true, - activate: false - }) + expect(callMock).toHaveBeenCalledWith( + 'terminal.create', + { + worktree: 'path:/tmp/repo/feature', + command: 'claude', + env: { ORCA_ATTRIBUTION_BYPASS: '1' }, + envToDelete: ['ORCA_ENABLE_GIT_ATTRIBUTION'], + title: 'Claude', + focus: false, + rendererBacked: true, + activate: false + }, + { expectedRuntimeId: 'runtime-1' } + ) }) it('keeps Claude print commands on the background terminal path', async () => { queueFixtures( callMock, + okFixture('req_status', { + runtimeId: 'runtime-1', + capabilities: ['terminal.attribution-removed.v1'] + }), okFixture('req_terminal_create', { terminal: { handle: 'term_1', @@ -3507,12 +3601,18 @@ describe('orca cli worktree awareness', () => { '/tmp/repo' ) - expect(callMock).toHaveBeenCalledWith('terminal.create', { - worktree: 'path:/tmp/repo/feature', - command: 'claude -p "summarize"', - title: 'Claude print', - focus: false - }) + expect(callMock).toHaveBeenCalledWith( + 'terminal.create', + { + worktree: 'path:/tmp/repo/feature', + command: 'claude -p "summarize"', + env: { ORCA_ATTRIBUTION_BYPASS: '1' }, + envToDelete: ['ORCA_ENABLE_GIT_ATTRIBUTION'], + title: 'Claude print', + focus: false + }, + { expectedRuntimeId: 'runtime-1' } + ) }) it('uses the resolved enclosing worktree for other worktree consumers', async () => { @@ -3819,6 +3919,10 @@ describe('orca cli worktree awareness', () => { it('sends explicit remote terminal create worktree selectors unchanged', async () => { queueFixtures( callMock, + okFixture('req_status', { + runtimeId: 'runtime-1', + capabilities: ['terminal.attribution-removed.v1'] + }), okFixture('req_terminal_create', { terminal: { handle: 'term_1', @@ -3842,12 +3946,18 @@ describe('orca cli worktree awareness', () => { '/tmp/client/repo/src' ) - expect(callMock).toHaveBeenCalledWith('terminal.create', { - worktree: 'id:repo-1::/srv/orca/feature', - command: undefined, - title: undefined, - focus: false - }) + expect(callMock).toHaveBeenCalledWith( + 'terminal.create', + { + worktree: 'id:repo-1::/srv/orca/feature', + command: undefined, + env: { ORCA_ATTRIBUTION_BYPASS: '1' }, + envToDelete: ['ORCA_ENABLE_GIT_ATTRIBUTION'], + title: undefined, + focus: false + }, + { expectedRuntimeId: 'runtime-1' } + ) }) it('collects and formats memory diagnostics', async () => { @@ -3955,6 +4065,10 @@ describe('orca cli worktree awareness', () => { it('does not force remote Codex terminal creates through a local renderer path', async () => { queueFixtures( callMock, + okFixture('req_status', { + runtimeId: 'runtime-1', + capabilities: ['terminal.attribution-removed.v1'] + }), okFixture('req_terminal_create', { terminal: { handle: 'term_1', @@ -3982,12 +4096,18 @@ describe('orca cli worktree awareness', () => { '/tmp/client/repo/src' ) - expect(callMock).toHaveBeenCalledWith('terminal.create', { - worktree: 'id:repo-1::/srv/orca/feature', - command: 'codex', - title: 'Codex', - focus: false - }) + expect(callMock).toHaveBeenCalledWith( + 'terminal.create', + { + worktree: 'id:repo-1::/srv/orca/feature', + command: 'codex', + env: { ORCA_ATTRIBUTION_BYPASS: '1' }, + envToDelete: ['ORCA_ENABLE_GIT_ATTRIBUTION'], + title: 'Codex', + focus: false + }, + { expectedRuntimeId: 'runtime-1' } + ) }) it('does not resolve implicit remote browser targets from client cwd', async () => { diff --git a/src/cli/runtime/client.ts b/src/cli/runtime/client.ts index 3a8951de554..b6c49ff7c22 100644 --- a/src/cli/runtime/client.ts +++ b/src/cli/runtime/client.ts @@ -87,6 +87,7 @@ export class RuntimeClient { } : {} const envelope = { + expectedRuntimeId: options?.expectedRuntimeId, orchestrationCapability: options?.orchestrationCapability, orchestrationContractVersion: method.startsWith('orchestration.') ? ORCHESTRATION_CONTRACT_VERSION diff --git a/src/cli/runtime/transport.test.ts b/src/cli/runtime/transport.test.ts index 8de35f619e5..7858d6cd131 100644 --- a/src/cli/runtime/transport.test.ts +++ b/src/cli/runtime/transport.test.ts @@ -50,6 +50,48 @@ describe('runtime transport timeout validation', () => { // Why: these tests create Unix domain socket servers in temp directories. // Windows does not support Unix domain sockets in the same way. describe.skipIf(process.platform === 'win32')('runtime transport', () => { + it('forwards the expected runtime fence in local requests', async () => { + const userDataPath = mkdtempSync(join(tmpdir(), 'orca-runtime-transport-')) + const endpoint = join(userDataPath, 'runtime.sock') + let receivedRequest: Record | null = null + const server = createServer((socket) => { + sockets.add(socket) + socket.once('close', () => sockets.delete(socket)) + socket.once('data', (data) => { + receivedRequest = JSON.parse(String(data).trim()) as Record + socket.write( + `${JSON.stringify({ + id: receivedRequest.id, + ok: true, + result: { terminal: { handle: 'terminal-1' } }, + _meta: { runtimeId: 'runtime-1' } + })}\n` + ) + }) + }) + servers.add(server) + await new Promise((resolve) => server.listen(endpoint, resolve)) + + await sendRequest( + { + runtimeId: 'runtime-1', + pid: 123, + transports: [{ kind: 'unix', endpoint }], + authToken: 'token', + startedAt: 1 + }, + 'terminal.create', + { worktree: 'id:worktree-1' }, + 1_000, + { expectedRuntimeId: 'runtime-1' } + ) + + expect(receivedRequest).toMatchObject({ + method: 'terminal.create', + expectedRuntimeId: 'runtime-1' + }) + }) + it('refreshes the per-call timeout when the runtime sends keepalive frames', async () => { const userDataPath = mkdtempSync(join(tmpdir(), 'orca-runtime-transport-')) const endpoint = join(userDataPath, 'runtime.sock') diff --git a/src/cli/runtime/transport.ts b/src/cli/runtime/transport.ts index 091ca9da0e0..998052dc4b6 100644 --- a/src/cli/runtime/transport.ts +++ b/src/cli/runtime/transport.ts @@ -186,6 +186,7 @@ export async function sendRequest( authToken: metadata.authToken, method, params, + expectedRuntimeId: envelope?.expectedRuntimeId, orchestrationCapability: envelope?.orchestrationCapability, orchestrationContractVersion: envelope?.orchestrationContractVersion, orchestrationRequestId: envelope?.orchestrationRequestId, diff --git a/src/main/daemon/pty-subprocess.test.ts b/src/main/daemon/pty-subprocess.test.ts index c7b41d182da..81d77da5720 100644 --- a/src/main/daemon/pty-subprocess.test.ts +++ b/src/main/daemon/pty-subprocess.test.ts @@ -78,6 +78,10 @@ vi.mock('../providers/windows-conpty-process-membership', () => ({ import { createPtySubprocess, checkPtySpawnHealth } from './pty-subprocess' import { PREVIOUS_DAEMON_PROTOCOL_VERSIONS, PROTOCOL_VERSION } from './types' import { TERMINAL_GIT_CREDENTIAL_GUARD_POLICY_ENV } from '../../shared/terminal-git-credential-guard' +import { + LEGACY_TERMINAL_SHIM_ENV_KEYS, + stripLegacyTerminalShimEnv +} from '../pty/legacy-terminal-shim-dir' const ORCA_SHELL_WRAPPER_ENV = [ 'ORCA_OPENCODE_CONFIG_DIR', @@ -1275,6 +1279,35 @@ describe('createPtySubprocess', () => { expect(env.ELECTRON_RUN_AS_NODE).toBeUndefined() }) + it('does not inherit legacy attribution state from a pre-upgrade daemon', () => { + const proc = mockPtyProcess() + spawnMock.mockReturnValue(proc) + const saved = Object.fromEntries( + [...LEGACY_TERMINAL_SHIM_ENV_KEYS, 'PATH'].map((key) => [key, process.env[key]]) + ) + process.env.ORCA_ENABLE_GIT_ATTRIBUTION = '1' + process.env.ORCA_ATTRIBUTION_SHIM_DIR = '/tmp/orca-terminal-attribution/posix' + process.env.PATH = `/tmp/orca-terminal-attribution/posix${delimiter}/usr/bin` + + try { + createPtySubprocess({ sessionId: 'test', cols: 80, rows: 24 }) + } finally { + for (const [key, value] of Object.entries(saved)) { + if (value === undefined) { + delete process.env[key] + } else { + process.env[key] = value + } + } + } + + const env = spawnMock.mock.calls.at(-1)?.[2].env + expect(env.PATH).toBe('/usr/bin') + for (const key of LEGACY_TERMINAL_SHIM_ENV_KEYS) { + expect(env[key]).toBeUndefined() + } + }) + it('does not inherit NODE_ENV from the daemon process env', () => { // Why: a dev-mode Orca forks the daemon with NODE_ENV=development; leaking // Orca's build mode into user shells breaks `next build` and Vitest. @@ -1295,7 +1328,9 @@ describe('createPtySubprocess', () => { const env = spawnMock.mock.calls.at(-1)?.[2].env expect(env.NODE_ENV).toBeUndefined() - expect(env.PATH).toBe(process.env.PATH) + const expectedEnv = { PATH: process.env.PATH ?? '' } + stripLegacyTerminalShimEnv(expectedEnv, process.platform) + expect(env.PATH).toBe(expectedEnv.PATH) }) it('keeps an explicitly requested NODE_ENV for daemon PTY shells', () => { @@ -2207,6 +2242,8 @@ describe('createPtySubprocess', () => { const proc = mockPtyProcess() spawnMock.mockReturnValue(proc) const platform = Object.getOwnPropertyDescriptor(process, 'platform') + const expectedEnv = { PATH: process.env.PATH ?? '' } + stripLegacyTerminalShimEnv(expectedEnv, 'win32') Object.defineProperty(process, 'platform', { value: 'win32' }) try { @@ -2218,7 +2255,7 @@ describe('createPtySubprocess', () => { } const env = spawnMock.mock.calls.at(-1)![2].env - expect(env.PATH).toBe(process.env.PATH) + expect(env.PATH).toBe(expectedEnv.PATH) }) it('preserves a duplicated path block supplied by main', () => { diff --git a/src/main/daemon/pty-subprocess.ts b/src/main/daemon/pty-subprocess.ts index 67c4319cd70..a6eade9f621 100644 --- a/src/main/daemon/pty-subprocess.ts +++ b/src/main/daemon/pty-subprocess.ts @@ -35,6 +35,7 @@ import { isHostCodexHomeForWsl, isWslCodexHomeForHost } from '../pty/codex-home- import { removeInheritedNoColor } from '../pty/terminal-color-env' import { removeAppImageRuntimeEnv } from '../pty/appimage-terminal-env' import { stripInheritedBuildModeEnv } from '../pty/build-mode-env' +import { stripLegacyTerminalShimEnv } from '../pty/legacy-terminal-shim-dir' import { resolvePathEnvKey } from '../pty/windows-environment-path' import { parseWslPath } from '../wsl' import { addWslEnvKeys } from '../wsl-env' @@ -629,6 +630,8 @@ export function createPtySubprocess(opts: PtySubprocessOptions): SubprocessHandl // Why: `supports-hyperlinks` gates OSC 8 on a TERM_PROGRAM allowlist excluding Orca; force it since xterm.js parses OSC 8 for clickable links. FORCE_HYPERLINK: '1' } as Record + // Why: an older client may not ask a newly upgraded daemon to delete inherited shim state. + stripLegacyTerminalShimEnv(env, process.platform) composeGuardedDaemonGitConfigEnv(env, opts.env, opts.launchAgent) deleteRequestedDaemonEnvKeys(env, opts.envToDelete) if (opts.env?.TERM) { @@ -834,6 +837,8 @@ export function createPtySubprocess(opts: PtySubprocessOptions): SubprocessHandl ? requestedEnv[resolvePathEnvKey(requestedEnv, process.platform)] : undefined promoteAgentTeamsShimPath(env, requestedPath) + // Why: raw requested PATH promotion runs after the inherited-env scrub. + stripLegacyTerminalShimEnv(env, process.platform) // Why: asar packaging can strip +x from node-pty's spawn-helper; the daemon is a separate forked process from the main-process fix. ensureNodePtySpawnHelperExecutable() diff --git a/src/main/index.ts b/src/main/index.ts index a2a3998c85c..b4e0e1b5ae4 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -185,7 +185,7 @@ import { logStartupMilestone } from './startup/startup-diagnostics' import { ensureWindowsUserDataAclGrant } from './startup/windows-user-data-acl' -import { removeLegacyTerminalShimDir } from './pty/legacy-terminal-shim-dir' +import { neutralizeLegacyTerminalShimDir } from './pty/legacy-terminal-shim-dir' import { shouldQuitWhenAllWindowsClosed } from './startup/window-all-closed-quit-policy' import { createServeDesktopActivationGate, @@ -2172,7 +2172,7 @@ void app.whenReady().then(async () => { const activeOrcaProfile = ensureActiveOrcaProfile() store = new Store({ dataFile: activeOrcaProfile.dataFile }) // Why: must precede PTY handler registration and run in headless serve too, which returns before openMainWindow. - removeLegacyTerminalShimDir(app.getPath('userData')) + neutralizeLegacyTerminalShimDir(app.getPath('userData')) const windowsShellPathHydration = createWindowsShellPathHydration() configureWindowsHostGitEnvironmentReadiness( process.platform === 'win32' ? windowsShellPathHydration.whenReady : null diff --git a/src/main/ipc/pty.test.ts b/src/main/ipc/pty.test.ts index e89a4c21806..ae9166148af 100644 --- a/src/main/ipc/pty.test.ts +++ b/src/main/ipc/pty.test.ts @@ -248,6 +248,7 @@ import { type PrepareCodexSessionResume } from './pty' import { __resetPersistedWindowsPathCacheForTests } from '../pty/windows-environment-path' +import { LEGACY_TERMINAL_SHIM_REMOTE_ENV_KEYS } from '../pty/legacy-terminal-shim-dir' import { __setWindowsPathRegistryLoaderForTests } from '../pty/windows-path-registry-reader' import { resetMacosLoginShellPreflightForTests } from '../providers/macos-tcc-login-shell' import { @@ -4399,6 +4400,16 @@ describe('registerPtyHandlers', () => { expect(spawnOptions.env.ORCA_AGENT_HOOK_TOKEN).toBe('agent-token') }) + it('asks surviving pre-upgrade daemons to delete legacy attribution env', async () => { + const spawnOptions = await daemonSpawnAndGetOptions({}) + + expect(spawnOptions.envToDelete).toEqual( + expect.arrayContaining([...LEGACY_TERMINAL_SHIM_REMOTE_ENV_KEYS]) + ) + expect(spawnOptions.envToDelete).not.toContain('ORCA_REAL_GIT') + expect(spawnOptions.envToDelete).not.toContain('ORCA_REAL_GH') + }) + it('deletes stale Claude scoped settings env from runtime-created daemon PTYs', async () => { type RuntimeSpawnController = { spawn(args: { @@ -4436,6 +4447,38 @@ describe('registerPtyHandlers', () => { expect(spawnOptions.env.ORCA_AGENT_HOOK_TOKEN).toBe('agent-token') }) + it('asks surviving pre-upgrade daemons to delete legacy attribution env for runtime PTYs', async () => { + type RuntimeSpawnController = { + spawn(args: { + cols: number + rows: number + worktreeId?: string + env?: Record + }): Promise<{ id: string }> + } + const daemonSpawn = setupDaemonAdapter() + const runtime = { + setPtyController: vi.fn(), + registerPty: vi.fn(), + noteTerminalSpawnCommand: vi.fn(), + onPtySpawned: vi.fn(), + onPtyExit: vi.fn(), + onPtyData: vi.fn() + } + handlers.clear() + registerPtyHandlers(mainWindow as never, runtime as never) + const controller = runtime.setPtyController.mock.calls[0]?.[0] as RuntimeSpawnController + + await controller.spawn({ cols: 80, rows: 24, worktreeId: 'wt-runtime', env: {} }) + + const spawnOptions = daemonSpawn.mock.calls.at(-1)?.[0] as DaemonSpawnCall + expect(spawnOptions.envToDelete).toEqual( + expect.arrayContaining([...LEGACY_TERMINAL_SHIM_REMOTE_ENV_KEYS]) + ) + expect(spawnOptions.envToDelete).not.toContain('ORCA_REAL_GIT') + expect(spawnOptions.envToDelete).not.toContain('ORCA_REAL_GH') + }) + it('strips inherited Claude child-session stamps from runtime-created PTYs', async () => { // Why: the runtime controller is the `orca` CLI / automation spawn path and // assembles envToDelete separately from the renderer's pty:spawn handler; diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index ddc829d2d13..67c0cb04edc 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -113,6 +113,7 @@ import { import { ensureLinuxTerminalOrcaCliShimDir } from '../cli/linux-terminal-orca-cli-shim' import { isLegacyTerminalShimPathEntry, + LEGACY_TERMINAL_SHIM_REMOTE_ENV_KEYS, stripLegacyTerminalShimEnv } from '../pty/legacy-terminal-shim-dir' import { registerPty, unregisterPty } from '../memory/pty-registry' @@ -4715,6 +4716,8 @@ export function registerPtyHandlers( spawnOptions.envToDelete = mergePtyEnvDeletions( authEnvToDelete, args.envToDelete ?? [], + // Why: disable old hosts without removing ORCA_REAL_* while their Windows shim remains on PATH. + isDaemonHostSpawn || args.connectionId ? LEGACY_TERMINAL_SHIM_REMOTE_ENV_KEYS : [], isDaemonHostSpawn ? getInheritedAgentHookEnvKeysToDelete(env) : [], // Why: ungated, unlike the agent-hook keys — the local provider and the relay host also spread their own process.env into every spawn. getInheritedClaudeSessionStampEnvKeysToDelete(env) @@ -6354,6 +6357,8 @@ export function registerPtyHandlers( envToDelete, args.envToDelete ?? [], agentTeamsEnvToDelete ?? [], + // Why: disable old hosts without removing ORCA_REAL_* while their Windows shim remains on PATH. + isDaemonHostSpawn || args.connectionId ? LEGACY_TERMINAL_SHIM_REMOTE_ENV_KEYS : [], isDaemonHostSpawn ? getInheritedAgentHookEnvKeysToDelete(spawnEnv) : [], getInheritedClaudeSessionStampEnvKeysToDelete(spawnEnv), skipCodexHomeEnv ? CODEX_HOME_ENV_KEYS : [], diff --git a/src/main/ipc/runtime-environment-connectivity-handlers.ts b/src/main/ipc/runtime-environment-connectivity-handlers.ts index 9d3732f886b..2c4d1462540 100644 --- a/src/main/ipc/runtime-environment-connectivity-handlers.ts +++ b/src/main/ipc/runtime-environment-connectivity-handlers.ts @@ -183,6 +183,7 @@ function registerPassiveCallHandler(getUserDataPath: () => string): void { params?: unknown timeoutMs?: number expectedEnvironmentPairingRevision?: number + expectedRuntimeId?: string } ): Promise> => { const environment = resolveEnvironment(getUserDataPath(), args.selector) @@ -197,7 +198,8 @@ function registerPassiveCallHandler(getUserDataPath: () => string): void { args.method, args.params, args.timeoutMs, - args.expectedEnvironmentPairingRevision + args.expectedEnvironmentPairingRevision, + args.expectedRuntimeId ? { expectedRuntimeId: args.expectedRuntimeId } : undefined ) } catch (error) { const failure = runtimeEnvironmentCallFailure(environment, args.method, error) diff --git a/src/main/ipc/runtime-environment-request-connections.ts b/src/main/ipc/runtime-environment-request-connections.ts index 58fb95dd6e1..c9782d61772 100644 --- a/src/main/ipc/runtime-environment-request-connections.ts +++ b/src/main/ipc/runtime-environment-request-connections.ts @@ -77,13 +77,15 @@ export function subscribeRemoteRuntimeSharedControlRequest( onBinary?: (bytes: Uint8Array) => void onError: (error: { code: string; message: string }) => void onClose?: () => void - } + }, + envelope?: RuntimeOrchestrationEnvelope ): Promise { return getSharedControlConnection(environmentId, pairing).subscribe( method, params, timeoutMs, - callbacks + callbacks, + envelope?.expectedRuntimeId ) } diff --git a/src/main/ipc/runtime-environment-transport-routing.ts b/src/main/ipc/runtime-environment-transport-routing.ts index 884b0b2874a..0792a1ea009 100644 --- a/src/main/ipc/runtime-environment-transport-routing.ts +++ b/src/main/ipc/runtime-environment-transport-routing.ts @@ -193,11 +193,14 @@ export async function subscribeRuntimeEnvironment( | { type: 'close' } ) => void onClose: () => void - } + }, + expectedRuntimeId?: string ): Promise { const environment = resolveEnvironment(userDataPath, selector) const pairing = getPreferredPairingOffer(environment) const effectiveTimeoutMs = timeoutMs ?? DEFAULT_REMOTE_RUNTIME_TIMEOUT_MS + const envelope: RuntimeOrchestrationEnvelope | undefined = + expectedRuntimeId === undefined ? undefined : { expectedRuntimeId } let markedUsed = false const markUsedOnce = (runtimeId: string): void => { if (markedUsed) { @@ -240,7 +243,8 @@ export async function subscribeRuntimeEnvironment( method, params, effectiveTimeoutMs, - callbacksWithMarkUsed + callbacksWithMarkUsed, + envelope ) } return await subscribeRemoteRuntimeRequest( @@ -248,7 +252,9 @@ export async function subscribeRuntimeEnvironment( method, params, effectiveTimeoutMs, - callbacksWithMarkUsed + callbacksWithMarkUsed, + undefined, + envelope ) } catch (error) { if (error instanceof Error) { diff --git a/src/main/ipc/runtime-environments.test.ts b/src/main/ipc/runtime-environments.test.ts index 9ca8da98600..b351b88681a 100644 --- a/src/main/ipc/runtime-environments.test.ts +++ b/src/main/ipc/runtime-environments.test.ts @@ -1094,7 +1094,13 @@ describe('registerRuntimeEnvironmentHandlers', () => { await add(null, { name: 'desk', pairingCode: pairingCode() }) const subscribe = handler< - { selector: string; method: string; params?: unknown; subscriptionId?: string }, + { + selector: string + method: string + params?: unknown + subscriptionId?: string + expectedRuntimeId?: string + }, { subscriptionId: string; requestId: string } >('runtimeEnvironments:subscribe') await subscribe( @@ -1107,7 +1113,12 @@ describe('registerRuntimeEnvironmentHandlers', () => { removeListener: vi.fn() } }, - { selector: 'desk', method: 'browser.screencast', params: { pageId: 'page-1' } } + { + selector: 'desk', + method: 'browser.screencast', + params: { pageId: 'page-1' }, + expectedRuntimeId: 'runtime-1' + } ) await subscribe( { @@ -1127,14 +1138,18 @@ describe('registerRuntimeEnvironmentHandlers', () => { 'browser.screencast', { pageId: 'page-1' }, 15_000, - expect.any(Object) + expect.any(Object), + undefined, + { expectedRuntimeId: 'runtime-1' } ) expect(subscribeRemoteRuntimeRequestMock).toHaveBeenCalledWith( expect.any(Object), 'terminal.multiplex', { client: { id: 'client-1' } }, 15_000, - expect.any(Object) + expect.any(Object), + undefined, + undefined ) expect(subscribeRemoteRuntimeSharedControlRequestMock).not.toHaveBeenCalled() }) @@ -1163,7 +1178,13 @@ describe('registerRuntimeEnvironmentHandlers', () => { await add(null, { name: 'desk', pairingCode: pairingCode() }) const subscribe = handler< - { selector: string; method: string; params?: unknown; subscriptionId?: string }, + { + selector: string + method: string + params?: unknown + subscriptionId?: string + expectedRuntimeId?: string + }, { subscriptionId: string; requestId: string } >('runtimeEnvironments:subscribe') await expect( @@ -1177,7 +1198,11 @@ describe('registerRuntimeEnvironmentHandlers', () => { removeListener: vi.fn() } }, - { selector: 'desk', method: 'session.tabs.subscribeAll' } + { + selector: 'desk', + method: 'session.tabs.subscribeAll', + expectedRuntimeId: 'runtime-1' + } ) ).resolves.toMatchObject({ requestId: 'tabs-shared' }) @@ -1187,7 +1212,8 @@ describe('registerRuntimeEnvironmentHandlers', () => { 'session.tabs.subscribeAll', undefined, 15_000, - expect.any(Object) + expect.any(Object), + { expectedRuntimeId: 'runtime-1' } ) expect(subscribeRemoteRuntimeRequestMock).not.toHaveBeenCalled() }) @@ -1315,7 +1341,9 @@ describe('registerRuntimeEnvironmentHandlers', () => { 'session.tabs.subscribeAll', undefined, 15_000, - expect.any(Object) + expect.any(Object), + undefined, + undefined ) expect(subscribeRemoteRuntimeSharedControlRequestMock).not.toHaveBeenCalled() }) @@ -1659,7 +1687,9 @@ describe('registerRuntimeEnvironmentHandlers', () => { 'terminal.subscribe', { terminal: 't1' }, 25, - expect.any(Object) + expect.any(Object), + undefined, + undefined ) expect(sent).toEqual([ expect.objectContaining({ subscriptionId: result.subscriptionId, type: 'response' }), diff --git a/src/main/ipc/runtime-environments.ts b/src/main/ipc/runtime-environments.ts index e31f8569dec..3ddf4e2aa6c 100644 --- a/src/main/ipc/runtime-environments.ts +++ b/src/main/ipc/runtime-environments.ts @@ -90,6 +90,7 @@ export function registerRuntimeEnvironmentHandlers(store: Store): void { timeoutMs?: number subscriptionId?: string expectedEnvironmentPairingRevision?: number + expectedRuntimeId?: string } ): Promise<{ subscriptionId: string; requestId: string }> => { const subscriptionId = @@ -180,7 +181,8 @@ export function registerRuntimeEnvironmentHandlers(store: Store): void { retained?.removeDestroyedListener() remoteRuntimeSubscriptions.delete(subscriptionId) } - } + }, + args.expectedRuntimeId ) } catch (error) { removeDestroyedListener() diff --git a/src/main/persistence.test.ts b/src/main/persistence.test.ts index 06ee8a9d6a0..b0a1ee2ac92 100644 --- a/src/main/persistence.test.ts +++ b/src/main/persistence.test.ts @@ -5659,6 +5659,50 @@ describe('Store', () => { expect(persisted.settings).not.toHaveProperty('terminalScrollbackBytes') }) + it('retires the persisted GitHub attribution setting without dropping unknown settings', async () => { + const settledStore = await createStore() + settledStore.flush() + const settled = readDataFile() as { settings: Record } + settled.settings.enableGitHubAttribution = true + settled.settings.futureSetting = { enabled: true } + writeDataFile(settled) + + vi.useFakeTimers() + try { + const store = await createStore() + + expect(store.getSettings()).not.toHaveProperty('enableGitHubAttribution') + expect(store.getSettings()).toHaveProperty('futureSetting', { enabled: true }) + vi.advanceTimersByTime(5_000) + await store.waitForPendingWrite() + } finally { + vi.useRealTimers() + } + const persisted = readDataFile() as { settings?: Record } + expect(persisted.settings).not.toHaveProperty('enableGitHubAttribution') + expect(persisted.settings).toHaveProperty('futureSetting', { enabled: true }) + }) + + it('ignores retired GitHub attribution updates and strips stale in-memory values on save', async () => { + const store = await createStore() + const listener = vi.fn() + store.onSettingsChanged(listener) + + const updated = store.updateSettings({ enableGitHubAttribution: true } as never, { + notifyListeners: true + }) + + expect(updated).not.toHaveProperty('enableGitHubAttribution') + expect(listener).not.toHaveBeenCalled() + const settings = store.getSettings() as GlobalSettings & Record + settings.enableGitHubAttribution = false + settings.futureSetting = 'kept' + store.flush() + const persisted = readDataFile() as { settings?: Record } + expect(persisted.settings).not.toHaveProperty('enableGitHubAttribution') + expect(persisted.settings?.futureSetting).toBe('kept') + }) + it('normalizes terminal cursor style before persistence and listener broadcasts', async () => { const store = await createStore() store.updateSettings({ terminalCursorStyle: 'underline' }) diff --git a/src/main/persistence.ts b/src/main/persistence.ts index b62b25f7977..fbb4a2089aa 100644 --- a/src/main/persistence.ts +++ b/src/main/persistence.ts @@ -651,6 +651,11 @@ type LegacyTerminalScrollbackSettings = { terminalScrollbackBytes?: unknown } +type RetiredGlobalSettings = { + terminalScrollbackBytes?: unknown + enableGitHubAttribution?: unknown +} + const LEGACY_TERMINAL_TUI_SCROLL_SENSITIVITY_DEFAULT = 3 function readLegacyTerminalScrollbackSettings(settings: unknown): LegacyTerminalScrollbackSettings { @@ -659,12 +664,16 @@ function readLegacyTerminalScrollbackSettings(settings: unknown): LegacyTerminal : {} } -function stripLegacyTerminalScrollbackBytes( +function stripRetiredGlobalSettings( settings: Partial | undefined ): Partial { - const { terminalScrollbackBytes: _legacyScrollbackBytes, ...rest } = (settings ?? - {}) as Partial & { terminalScrollbackBytes?: unknown } + const { + terminalScrollbackBytes: _legacyScrollbackBytes, + enableGitHubAttribution: _legacyGitHubAttribution, + ...rest + } = (settings ?? {}) as Partial & RetiredGlobalSettings void _legacyScrollbackBytes + void _legacyGitHubAttribution return rest } @@ -3149,6 +3158,13 @@ export class Store { if (migratedTerminalScrollback.needsSave) { this.loadNeedsSave = true } + if ( + parsed.settings && + typeof parsed.settings === 'object' && + Object.hasOwn(parsed.settings, 'enableGitHubAttribution') + ) { + this.loadNeedsSave = true + } const migratedTerminalTuiScrollSensitivity = migrateTerminalTuiScrollSensitivityDefault( parsed.settings ) @@ -3421,7 +3437,7 @@ export class Store { settings: { ...defaults.settings, // Why (#7977): keep persisted experimentalNewWorktreeCardStyle:true — v1.4.130's onboarding auto-wrote it as a plain boolean, so it's indistinguishable from a real opt-in; only the default changed. - ...stripLegacyTerminalScrollbackBytes(parsed.settings), + ...stripRetiredGlobalSettings(parsed.settings), prBotAuthorOverrides: normalizePRBotAuthorOverrides( parsed.settings?.prBotAuthorOverrides ), @@ -4013,7 +4029,7 @@ export class Store { ) })), settings: { - ...this.state.settings, + ...stripRetiredGlobalSettings(this.state.settings), opencodeSessionCookie: encryptToSentinel( PROTECTED_SECRET_SLOT.opencodeSessionCookie, this.state.settings.opencodeSessionCookie @@ -5819,7 +5835,7 @@ export class Store { updates: Partial, options: { notifyListeners?: boolean; originWebContentsId?: number } = {} ): GlobalSettings { - const sanitizedUpdates = stripLegacyTerminalScrollbackBytes(updates) + const sanitizedUpdates = stripRetiredGlobalSettings(updates) if ('opencodeSessionCookie' in updates && !updates.opencodeSessionCookie) { this.protectedSecrets.removeRetainedBlob(PROTECTED_SECRET_SLOT.opencodeSessionCookie) } diff --git a/src/main/providers/local-pty-provider.test.ts b/src/main/providers/local-pty-provider.test.ts index 178b60d6caa..eedcb87abfc 100644 --- a/src/main/providers/local-pty-provider.test.ts +++ b/src/main/providers/local-pty-provider.test.ts @@ -2,6 +2,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { delimiter } from 'node:path' import type * as MacosTccLoginShell from './macos-tcc-login-shell' +import { stripLegacyTerminalShimEnv } from '../pty/legacy-terminal-shim-dir' const { existsSyncMock, @@ -576,7 +577,9 @@ describe('LocalPtyProvider', () => { const spawnCall = spawnMock.mock.calls.at(-1)! expect(spawnCall[2].env.NODE_ENV).toBeUndefined() - expect(spawnCall[2].env.PATH).toBe(process.env.PATH) + const expectedEnv = { PATH: process.env.PATH ?? '' } + stripLegacyTerminalShimEnv(expectedEnv, process.platform) + expect(spawnCall[2].env.PATH).toBe(expectedEnv.PATH) }) it('keeps an explicitly requested NODE_ENV for spawned terminals', async () => { @@ -752,6 +755,19 @@ describe('LocalPtyProvider', () => { expect(spawnCall[2].env.ORCA_STALE_TEST_ENV).toBeUndefined() }) + it('does not re-promote a legacy attribution path for Agent Teams', async () => { + await provider.spawn({ + cols: 80, + rows: 24, + env: { + PATH: '/tmp/orca-terminal-attribution/posix:/usr/bin', + ORCA_AGENT_TEAMS_TEAM_ID: 'team-test' + } + }) + + expect(spawnMock.mock.calls.at(-1)?.[2].env.PATH).toBe('/usr/bin') + }) + it('drops stale inherited Git config indices behind a smaller explicit count', async () => { const keys = [ 'GIT_CONFIG_COUNT', diff --git a/src/main/providers/local-pty-provider.ts b/src/main/providers/local-pty-provider.ts index 3026ded42a3..b4af3c907fb 100644 --- a/src/main/providers/local-pty-provider.ts +++ b/src/main/providers/local-pty-provider.ts @@ -36,6 +36,7 @@ import type { ShellReadySignal } from './local-pty-shell-ready' import { removeInheritedNoColor } from '../pty/terminal-color-env' import { removeAppImageRuntimeEnv } from '../pty/appimage-terminal-env' import { stripInheritedBuildModeEnv } from '../pty/build-mode-env' +import { stripLegacyTerminalShimEnv } from '../pty/legacy-terminal-shim-dir' import { SessionNotFoundError } from '../daemon/daemon-errors' import { resolvePathEnvKey } from '../pty/windows-environment-path' import { isHostCodexHomeForWsl, isWslCodexHomeForHost } from '../pty/codex-home-wsl-env' @@ -829,6 +830,8 @@ export class LocalPtyProvider implements IPtyProvider { finalEnv, requestedEnv ? requestedEnv[resolvePathEnvKey(requestedEnv, process.platform)] : undefined ) + // Why: raw requested PATH promotion runs after the host-env scrub. + stripLegacyTerminalShimEnv(finalEnv, process.platform) // Why: worktree-scoped HISTFILE — without it worktrees share one global history (terminal-history-scope-design §7–§10). const worktreeId = args.worktreeId diff --git a/src/main/providers/provider-dispatch.test.ts b/src/main/providers/provider-dispatch.test.ts index 57d7e3f815c..2ba058ffd9c 100644 --- a/src/main/providers/provider-dispatch.test.ts +++ b/src/main/providers/provider-dispatch.test.ts @@ -62,6 +62,7 @@ import { unregisterSshPtyProvider } from '../ipc/pty' import type { IPtyProvider } from './types' +import { LEGACY_TERMINAL_SHIM_REMOTE_ENV_KEYS } from '../pty/legacy-terminal-shim-dir' describe('PTY provider dispatch', () => { const handlers = new Map unknown>() @@ -147,6 +148,7 @@ describe('PTY provider dispatch', () => { const sshSpawnArgs = vi.mocked(mockSshProvider.spawn).mock.calls.at(-1)![0] expect([...(sshSpawnArgs.envToDelete ?? [])].sort()).toEqual( [ + ...LEGACY_TERMINAL_SHIM_REMOTE_ENV_KEYS, 'CLAUDE_CODE_CHILD_SESSION', 'CLAUDE_CODE_SESSION_ID', 'CLAUDE_CODE_BRIDGE_SESSION_ID' diff --git a/src/main/pty/legacy-terminal-shim-dir.test.ts b/src/main/pty/legacy-terminal-shim-dir.test.ts index f7115f74613..2bf90e9d9fe 100644 --- a/src/main/pty/legacy-terminal-shim-dir.test.ts +++ b/src/main/pty/legacy-terminal-shim-dir.test.ts @@ -1,64 +1,187 @@ -import { describe, expect, it, beforeEach } from 'vitest' -import { chmodSync, existsSync, mkdirSync, mkdtempSync, writeFileSync } from 'node:fs' +import { spawn } from 'node:child_process' +import { + chmodSync, + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + statSync, + writeFileSync +} from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { - __resetLegacyTerminalShimRemovalForTests, - removeLegacyTerminalShimDir, + __resetLegacyTerminalShimNeutralizationForTests, + neutralizeLegacyTerminalShimDir, stripLegacyTerminalShimEnv } from './legacy-terminal-shim-dir' -// Why: the failure case is produced with a read-only parent dir, which Windows ignores and root bypasses. +const itOnPosix = process.platform === 'win32' ? it.skip : it +// Why: the failure case uses directory permissions, which Windows ignores and root bypasses. const itOnPosixNonRoot = process.platform === 'win32' || process.getuid?.() === 0 ? it.skip : it -describe('legacy terminal shim removal', () => { +describe('legacy terminal shim neutralization', () => { beforeEach(() => { - __resetLegacyTerminalShimRemovalForTests() + __resetLegacyTerminalShimNeutralizationForTests() }) - it('deletes the orphaned wrapper directory left by older installs', () => { - const userData = mkdtempSync(join(tmpdir(), 'orca-legacy-shim-')) - const shimDir = join(userData, 'orca-terminal-attribution', 'posix') - mkdirSync(shimDir, { recursive: true }) - writeFileSync(join(shimDir, 'git'), '#!/usr/bin/env bash\n') - - removeLegacyTerminalShimDir(userData) - - expect(existsSync(join(userData, 'orca-terminal-attribution'))).toBe(false) - expect(existsSync(userData)).toBe(true) + afterEach(() => { + vi.useRealTimers() }) - it('does not throw when nothing is left to remove', () => { - const userData = mkdtempSync(join(tmpdir(), 'orca-legacy-shim-')) - expect(() => removeLegacyTerminalShimDir(userData)).not.toThrow() - }) - - itOnPosixNonRoot('stays retryable when the removal fails, and latches once it succeeds', () => { + it('atomically replaces the legacy command paths with executable tombstones', () => { const userData = mkdtempSync(join(tmpdir(), 'orca-legacy-shim-')) const legacyRoot = join(userData, 'orca-terminal-attribution') - mkdirSync(join(legacyRoot, 'posix'), { recursive: true }) - writeFileSync(join(legacyRoot, 'posix', 'git'), '#!/usr/bin/env bash\n') - // Why: a read-only parent makes the unlink fail the way a locked Windows wrapper does. - chmodSync(userData, 0o500) + const posixDir = join(legacyRoot, 'posix') + const win32Dir = join(legacyRoot, 'win32') + mkdirSync(posixDir, { recursive: true }) + mkdirSync(win32Dir, { recursive: true }) + writeFileSync(join(posixDir, 'git'), 'legacy attribution wrapper') + writeFileSync(join(win32Dir, 'gh.cmd'), 'legacy attribution wrapper') + + neutralizeLegacyTerminalShimDir(userData) + + for (const path of [ + join(posixDir, 'git'), + join(posixDir, 'gh'), + join(win32Dir, 'git.cmd'), + join(win32Dir, 'gh.cmd') + ]) { + expect(existsSync(path)).toBe(true) + expect(readFileSync(path, 'utf8')).not.toContain('Co-authored-by: Orca') + if (process.platform !== 'win32') { + expect(statSync(path).mode & 0o111).not.toBe(0) + } + } + expect(readFileSync(join(legacyRoot, 'VERSION'), 'utf8')).toBe('7\n') + }) + + it('rejects stale Windows real-command paths inside the wrapper directory', () => { + const userData = mkdtempSync(join(tmpdir(), 'orca-legacy-shim-')) + const win32Dir = join(userData, 'orca-terminal-attribution', 'win32') + + neutralizeLegacyTerminalShimDir(userData) + + const cmd = readFileSync(join(win32Dir, 'git.cmd'), 'utf8') + expect(cmd).toContain( + 'if defined orca_real for %%G in ("%orca_real%") do if /I "%%~dpG"=="%~dp0" set "orca_real="' + ) + const powershell = readFileSync(join(win32Dir, 'git-wrapper.ps1'), 'utf8') + expect(powershell).toContain('[StringComparison]::OrdinalIgnoreCase') + expect(powershell).toContain('$realCommand = $null') + expect(powershell.indexOf('[StringComparison]::OrdinalIgnoreCase')).toBeLessThan( + powershell.indexOf('Test-Path -LiteralPath $realCommand') + ) + }) + + it('removes every Windows PATH occurrence of both captured wrapper directories', () => { + const userData = mkdtempSync(join(tmpdir(), 'orca-legacy-shim-')) + const win32Dir = join(userData, 'orca-terminal-attribution', 'win32') + + neutralizeLegacyTerminalShimDir(userData) + + const cmd = readFileSync(join(win32Dir, 'git.cmd'), 'utf8') + const cmdCapture = 'set "orca_legacy_wrapper_dir=%ORCA_ATTRIBUTION_SHIM_DIR%"' + expect(cmd.indexOf(cmdCapture)).toBeLessThan(cmd.indexOf('set "ORCA_ATTRIBUTION_SHIM_DIR="')) + expect(cmd).toContain('for %%P in ("%PATH:;=" "%") do call :orca_append_path "%%~P"') + expect(cmd).toContain('if /I "%orca_path_entry_dir%"=="%orca_wrapper_dir%" exit /b') + expect(cmd).toContain( + 'if defined orca_legacy_wrapper_dir for %%G in ("%orca_legacy_wrapper_dir%") do if /I "%orca_path_entry_dir%"=="%%~fG\\" exit /b' + ) + + const powershell = readFileSync(join(win32Dir, 'git-wrapper.ps1'), 'utf8') + expect(powershell.indexOf('$legacyWrapperDir = $env:ORCA_ATTRIBUTION_SHIM_DIR')).toBeLessThan( + powershell.indexOf('Remove-Item "Env:$_"') + ) + expect(powershell).toContain('$wrapperDirs = @($wrapperDir, $legacyWrapperDir)') + expect(powershell).toContain("$env:PATH = (($env:PATH -split ';') | Where-Object {") + expect(powershell).toContain('[StringComparison]::OrdinalIgnoreCase') + }) + + it('does not throw when the legacy directory is absent', () => { + const userData = mkdtempSync(join(tmpdir(), 'orca-legacy-shim-')) + + expect(() => neutralizeLegacyTerminalShimDir(userData)).not.toThrow() + expect(existsSync(join(userData, 'orca-terminal-attribution', 'posix', 'git'))).toBe(true) + }) + + itOnPosixNonRoot('retries a startup failure in-process and latches after success', async () => { + vi.useFakeTimers() + const userData = mkdtempSync(join(tmpdir(), 'orca-legacy-shim-')) + const posixDir = join(userData, 'orca-terminal-attribution', 'posix') + const gitWrapper = join(posixDir, 'git') + mkdirSync(posixDir, { recursive: true }) + writeFileSync(gitWrapper, 'legacy attribution wrapper') + chmodSync(posixDir, 0o500) try { - removeLegacyTerminalShimDir(userData) - expect(existsSync(legacyRoot)).toBe(true) + neutralizeLegacyTerminalShimDir(userData) + expect(readFileSync(gitWrapper, 'utf8')).toBe('legacy attribution wrapper') } finally { - chmodSync(userData, 0o700) + chmodSync(posixDir, 0o700) } - // The failed attempt must not have latched the guard. - removeLegacyTerminalShimDir(userData) - expect(existsSync(legacyRoot)).toBe(false) + try { + await vi.advanceTimersByTimeAsync(1_000) + expect(readFileSync(gitWrapper, 'utf8')).not.toContain('legacy attribution wrapper') - // A success latches it: a directory recreated afterwards is left alone. - mkdirSync(legacyRoot, { recursive: true }) - removeLegacyTerminalShimDir(userData) - expect(existsSync(legacyRoot)).toBe(true) + writeFileSync(gitWrapper, 'recreated after success') + neutralizeLegacyTerminalShimDir(userData) + expect(readFileSync(gitWrapper, 'utf8')).toBe('recreated after success') + } finally { + vi.useRealTimers() + } + }) + + itOnPosix('keeps a real Bash command hash working as an inert pass-through', async () => { + const userData = mkdtempSync(join(tmpdir(), 'orca-legacy-shim-')) + const shimDir = join(userData, 'orca-terminal-attribution', 'posix') + const shimGit = join(shimDir, 'git') + const realBin = join(userData, 'real-bin') + const realGit = join(realBin, 'git') + mkdirSync(shimDir, { recursive: true }) + mkdirSync(realBin, { recursive: true }) + writeFileSync(shimGit, '#!/usr/bin/env bash\nexit 99\n', { mode: 0o755 }) + writeFileSync( + realGit, + "#!/usr/bin/env bash\nprintf 'arg=<%s>\\n' \"$@\"\ncat\nprintf 'fixture stderr\\n' >&2\nexit 23\n", + { mode: 0o755 } + ) + const child = spawn('bash', ['--noprofile', '--norc'], { + env: { + ...process.env, + PATH: `${shimDir}:${realBin}:${process.env.PATH ?? ''}`, + ORCA_ENABLE_GIT_ATTRIBUTION: '1', + ORCA_GIT_COMMIT_TRAILER: 'Co-authored-by: Orca ', + ORCA_ATTRIBUTION_SHIM_DIR: shimDir + }, + stdio: ['pipe', 'pipe', 'pipe'] + }) + let stdout = '' + let stderr = '' + child.stdout.setEncoding('utf8') + child.stderr.setEncoding('utf8') + child.stdout.on('data', (chunk: string) => { + stdout += chunk + }) + child.stderr.on('data', (chunk: string) => { + stderr += chunk + }) + const ready = waitForOutput(child.stdout, '__ORCA_HASH_READY__\n') + const closed = new Promise((resolve) => child.once('close', resolve)) + child.stdin.write(`hash -p ${quoteBash(shimGit)} git\nprintf '__ORCA_HASH_READY__\\n'\n`) + + await ready + neutralizeLegacyTerminalShimDir(userData) + child.stdin.end("printf 'stdin payload\\n' | git commit -m 'subject with spaces'; exit $?\n") + + expect(await closed).toBe(23) + expect(stdout).toContain('arg=\narg=<-m>\narg=\nstdin payload\n') + expect(stdout).not.toContain('Co-authored-by: Orca') + expect(stderr).toBe('fixture stderr\n') }) it('drops inherited shim env and its PATH entry without touching real entries', () => { - // Why: a daemon predating the removal reseeds these from its own process.env. const env: Record = { PATH: `/home/u/.orca/orca-terminal-attribution/posix:/usr/local/bin:/usr/bin`, ORCA_ENABLE_GIT_ATTRIBUTION: '1', @@ -73,20 +196,50 @@ describe('legacy terminal shim removal', () => { stripLegacyTerminalShimEnv(env, 'linux') - expect(env.PATH).toBe('/usr/local/bin:/usr/bin') - expect(env.ORCA_ENABLE_GIT_ATTRIBUTION).toBeUndefined() - expect(env.ORCA_GIT_COMMIT_TRAILER).toBeUndefined() - expect(env.ORCA_GH_PR_FOOTER).toBeUndefined() - expect(env.ORCA_GH_ISSUE_FOOTER).toBeUndefined() - expect(env.ORCA_ATTRIBUTION_SHIM_DIR).toBeUndefined() - expect(env.ORCA_REAL_GIT).toBeUndefined() - expect(env.ORCA_REAL_GH).toBeUndefined() - expect(env.HOME).toBe('/home/u') + expect(env).toEqual({ PATH: '/usr/local/bin:/usr/bin', HOME: '/home/u' }) }) - it('strips the Windows shim entry on the inherited Path spelling', () => { + it('uses the captured POSIX shim directory literally when it contains a colon', () => { + const shimDir = '/tmp/orca:user/orca-terminal-attribution/posix' const env: Record = { - Path: `C:\\Users\\u\\AppData\\Roaming\\Orca\\orca-terminal-attribution\\win32;C:\\Windows\\System32` + PATH: `/usr/local/bin:${shimDir}:/usr/bin`, + ORCA_ATTRIBUTION_SHIM_DIR: shimDir + } + + stripLegacyTerminalShimEnv(env, 'linux') + + expect(env).toEqual({ PATH: '/usr/local/bin:/usr/bin' }) + }) + + it('treats legacy Windows environment keys case-insensitively', () => { + const shimDir = 'C:\\Users\\orca;user\\orca-terminal-attribution\\win32' + const env: Record = { + Path: `${shimDir};C:\\Windows\\System32`, + orca_attribution_shim_dir: shimDir, + Orca_Enable_Git_Attribution: '1', + orca_real_git: 'C:\\Git\\git.exe' + } + + stripLegacyTerminalShimEnv(env, 'win32') + + expect(env).toEqual({ Path: 'C:\\Windows\\System32' }) + }) + + it('strips legacy entries from every Windows PATH spelling', () => { + const env: Record = { + PATH: 'C:\\Orca\\orca-terminal-attribution\\win32', + Path: 'C:\\Orca\\orca-terminal-attribution\\win32;C:\\Windows\\System32' + } + + stripLegacyTerminalShimEnv(env, 'win32') + + expect(env.PATH).toBeUndefined() + expect(env.Path).toBe('C:\\Windows\\System32') + }) + + it('matches a re-cased Windows shim path', () => { + const env: Record = { + Path: 'C:\\Orca\\Orca-Terminal-Attribution\\Win32;C:\\Windows\\System32' } stripLegacyTerminalShimEnv(env, 'win32') @@ -94,25 +247,26 @@ describe('legacy terminal shim removal', () => { expect(env.Path).toBe('C:\\Windows\\System32') }) - it('matches a re-cased shim entry on case-insensitive filesystems', () => { - const env: Record = { - Path: `C:\\Users\\u\\AppData\\Roaming\\Orca\\Orca-Terminal-Attribution\\win32;C:\\Windows\\System32` - } + it('preserves explicit empty PATH values', () => { + const windowsEnv: Record = { PATH: '', Path: 'C:\\Windows' } + const posixEnv: Record = { PATH: '' } - stripLegacyTerminalShimEnv(env, 'win32') + stripLegacyTerminalShimEnv(windowsEnv, 'win32') + stripLegacyTerminalShimEnv(posixEnv, 'linux') - expect(env.Path).toBe('C:\\Windows\\System32') + expect(windowsEnv).toEqual({ PATH: '', Path: 'C:\\Windows' }) + expect(posixEnv).toEqual({ PATH: '' }) }) it('keeps neighbouring directories that merely share the name prefix', () => { const env: Record = { - PATH: '/opt/orca-terminal-attribution:/home/u/orca-terminal-attribution-notes/bin:/usr/bin' + PATH: '/opt/orca-terminal-attribution:/opt/orca-terminal-attribution/custom-tools:/home/u/orca-terminal-attribution-notes/bin:/usr/bin' } stripLegacyTerminalShimEnv(env, 'linux') expect(env.PATH).toBe( - '/opt/orca-terminal-attribution:/home/u/orca-terminal-attribution-notes/bin:/usr/bin' + '/opt/orca-terminal-attribution:/opt/orca-terminal-attribution/custom-tools:/home/u/orca-terminal-attribution-notes/bin:/usr/bin' ) }) @@ -122,3 +276,30 @@ describe('legacy terminal shim removal', () => { expect(env.PATH).toBe('/usr/local/bin:/usr/bin') }) }) + +function quoteBash(value: string): string { + return `'${value.replaceAll("'", `'\\''`)}'` +} + +function waitForOutput(stream: NodeJS.ReadableStream, marker: string): Promise { + return new Promise((resolve, reject) => { + let output = '' + const onData = (chunk: string | Buffer): void => { + output += chunk.toString() + if (output.includes(marker)) { + cleanup() + resolve() + } + } + const onEnd = (): void => { + cleanup() + reject(new Error(`Bash exited before emitting ${marker}`)) + } + const cleanup = (): void => { + stream.off('data', onData) + stream.off('end', onEnd) + } + stream.on('data', onData) + stream.on('end', onEnd) + }) +} diff --git a/src/main/pty/legacy-terminal-shim-dir.ts b/src/main/pty/legacy-terminal-shim-dir.ts index d46eb7c6516..5d89839a0e5 100644 --- a/src/main/pty/legacy-terminal-shim-dir.ts +++ b/src/main/pty/legacy-terminal-shim-dir.ts @@ -1,71 +1,310 @@ -import { rmSync } from 'node:fs' +import { chmodSync, mkdirSync, renameSync, rmSync, writeFileSync } from 'node:fs' import { join } from 'node:path' -import { resolvePathEnvKey } from './windows-environment-path' +import { + LEGACY_TERMINAL_ATTRIBUTION_BYPASS_ENV_KEY, + LEGACY_TERMINAL_ATTRIBUTION_ENABLE_ENV_KEY +} from '../../shared/legacy-terminal-attribution-env' const LEGACY_SHIM_ROOT_DIR = 'orca-terminal-attribution' -const LEGACY_SHIM_ENV_KEYS = [ +const LEGACY_SHIM_VERSION = '7' +const NEUTRALIZATION_RETRY_DELAYS_MS = [1_000, 5_000, 15_000, 30_000] +const SHELL_DOLLAR = '$' +export const LEGACY_TERMINAL_SHIM_ENV_KEYS = [ 'ORCA_ENABLE_GIT_ATTRIBUTION', 'ORCA_GIT_COMMIT_TRAILER', 'ORCA_GH_PR_FOOTER', 'ORCA_GH_ISSUE_FOOTER', 'ORCA_ATTRIBUTION_SHIM_DIR', 'ORCA_REAL_GIT', - 'ORCA_REAL_GH' + 'ORCA_REAL_GH', + LEGACY_TERMINAL_ATTRIBUTION_BYPASS_ENV_KEY +] as const +export const LEGACY_TERMINAL_SHIM_REMOTE_ENV_KEYS = [ + LEGACY_TERMINAL_ATTRIBUTION_ENABLE_ENV_KEY ] as const -let removed = false +const POSIX_PASSTHROUGH_WRAPPER = String.raw`#!/usr/bin/env bash +set -u -/** Why: a daemon that outlives an upgrade keeps seeding these from its own process.env - * (pty-subprocess reads process.env as the authoritative base), and the wrappers they - * point at are gated only on inherited env. Deleting the scripts makes both inert. */ -export function removeLegacyTerminalShimDir(userDataPath: string): void { - if (removed) { +command_name="__ORCA_COMMAND__" +wrapper_dir="$(cd -- "$(dirname -- "${SHELL_DOLLAR}{BASH_SOURCE[0]}")" && pwd)" +legacy_wrapper_dir="${SHELL_DOLLAR}{ORCA_ATTRIBUTION_SHIM_DIR:-}" +cleaned_path="${SHELL_DOLLAR}{PATH:-}" + +remove_path_entry() { + local entry="$1" + [[ -n "$entry" ]] || return + while [[ "$cleaned_path" == "$entry" || "$cleaned_path" == "$entry":* || "$cleaned_path" == *:"$entry" || "$cleaned_path" == *:"$entry":* ]]; do + if [[ "$cleaned_path" == "$entry" ]]; then + cleaned_path="" + elif [[ "$cleaned_path" == "$entry":* ]]; then + cleaned_path="${SHELL_DOLLAR}{cleaned_path#"$entry":}" + elif [[ "$cleaned_path" == *:"$entry" ]]; then + cleaned_path="${SHELL_DOLLAR}{cleaned_path%:"$entry"}" + else + cleaned_path="${SHELL_DOLLAR}{cleaned_path//:"$entry":/:}" + fi + done +} + +remove_path_entry "$legacy_wrapper_dir" +remove_path_entry "$wrapper_dir" +unset ORCA_ENABLE_GIT_ATTRIBUTION ORCA_GIT_COMMIT_TRAILER ORCA_GH_PR_FOOTER +unset ORCA_GH_ISSUE_FOOTER ORCA_ATTRIBUTION_SHIM_DIR ORCA_REAL_GIT ORCA_REAL_GH ORCA_ATTRIBUTION_BYPASS + +real_command="$(PATH="$cleaned_path" type -P "$command_name" || true)" +if [[ -z "$real_command" ]]; then + printf 'Orca compatibility wrapper could not locate %s on PATH.\n' "$command_name" >&2 + exit 127 +fi +PATH="$cleaned_path" exec "$real_command" "$@" +` + +const WIN32_PASSTHROUGH_WRAPPER = String.raw`@echo off +setlocal +set "orca_real=%ORCA_REAL___ORCA_UPPER_COMMAND__%" +set "orca_wrapper_dir=%~dp0" +set "orca_legacy_wrapper_dir=%ORCA_ATTRIBUTION_SHIM_DIR%" +set "orca_clean_path=" +for %%P in ("%PATH:;=" "%") do call :orca_append_path "%%~P" +set "PATH=%orca_clean_path%" +set "ORCA_ENABLE_GIT_ATTRIBUTION=" +set "ORCA_GIT_COMMIT_TRAILER=" +set "ORCA_GH_PR_FOOTER=" +set "ORCA_GH_ISSUE_FOOTER=" +set "ORCA_ATTRIBUTION_SHIM_DIR=" +set "ORCA_ATTRIBUTION_BYPASS=" +set "ORCA_REAL_GIT=" +set "ORCA_REAL_GH=" +if defined orca_real for %%G in ("%orca_real%") do if /I "%%~dpG"=="%~dp0" set "orca_real=" +if defined orca_real if exist "%orca_real%" goto run +for /f "delims=" %%G in ('where.exe __ORCA_COMMAND__.exe 2^>nul') do if not defined orca_real set "orca_real=%%G" +if not defined orca_real ( + echo Orca compatibility wrapper could not locate __ORCA_COMMAND__ on PATH. 1>&2 + exit /b 127 +) +:run +"%orca_real%" %* +exit /b %ERRORLEVEL% + +:orca_append_path +for %%G in ("%~1") do set "orca_path_entry_dir=%%~fG\" +if /I "%orca_path_entry_dir%"=="%orca_wrapper_dir%" exit /b +if defined orca_legacy_wrapper_dir for %%G in ("%orca_legacy_wrapper_dir%") do if /I "%orca_path_entry_dir%"=="%%~fG\" exit /b +if defined orca_clean_path (set "orca_clean_path=%orca_clean_path%;%~1") else set "orca_clean_path=%~1" +exit /b +` + +const POWERSHELL_PASSTHROUGH_WRAPPER = String.raw`$ErrorActionPreference = 'Stop' +$commandName = '__ORCA_COMMAND__' +$realCommand = [Environment]::GetEnvironmentVariable('ORCA_REAL___ORCA_UPPER_COMMAND__') +$wrapperDir = Split-Path -Parent $MyInvocation.MyCommand.Path +$legacyWrapperDir = $env:ORCA_ATTRIBUTION_SHIM_DIR +$wrapperDirs = @($wrapperDir, $legacyWrapperDir) | Where-Object { $_ } | ForEach-Object { $_.TrimEnd('\') } +$env:PATH = (($env:PATH -split ';') | Where-Object { + $pathEntry = $_ + $pathEntry -and -not ($wrapperDirs | Where-Object { + [string]::Equals($_, $pathEntry.TrimEnd('\'), [StringComparison]::OrdinalIgnoreCase) + }) +}) -join ';' +'ORCA_ENABLE_GIT_ATTRIBUTION', 'ORCA_GIT_COMMIT_TRAILER', 'ORCA_GH_PR_FOOTER', 'ORCA_GH_ISSUE_FOOTER', 'ORCA_ATTRIBUTION_SHIM_DIR', 'ORCA_REAL_GIT', 'ORCA_REAL_GH', 'ORCA_ATTRIBUTION_BYPASS' | ForEach-Object { Remove-Item "Env:$_" -ErrorAction SilentlyContinue } +if ($realCommand) { + try { + $capturedDir = Split-Path -Parent ([IO.Path]::GetFullPath($realCommand)) + if ([string]::Equals($capturedDir.TrimEnd('\'), $wrapperDir.TrimEnd('\'), [StringComparison]::OrdinalIgnoreCase)) { + $realCommand = $null + } + } catch { + $realCommand = $null + } +} +if (-not $realCommand -or -not (Test-Path -LiteralPath $realCommand)) { + $resolved = Get-Command "$commandName.exe" -CommandType Application -ErrorAction SilentlyContinue | Select-Object -First 1 + $realCommand = if ($resolved) { $resolved.Source } else { $null } +} +if (-not $realCommand) { + [Console]::Error.WriteLine("Orca compatibility wrapper could not locate $commandName on PATH.") + exit 127 +} +& $realCommand @args +exit $LASTEXITCODE +` + +let neutralized = false +let neutralizationRetryTimer: ReturnType | null = null +let neutralizationRetryAttempt = 0 + +export function neutralizeLegacyTerminalShimDir(userDataPath: string): void { + if (neutralized) { return } try { - // Why: a surviving pre-upgrade pane can hold the wrapper open on Windows; retry like - // the other userData removals rather than forfeiting cleanup for the whole run. - rmSync(join(userDataPath, LEGACY_SHIM_ROOT_DIR), { - recursive: true, - force: true, - maxRetries: 3, - retryDelay: 50 - }) - removed = true + const rootDir = join(userDataPath, LEGACY_SHIM_ROOT_DIR) + writeNeutralWrappers(rootDir) + writeFileAtomically(join(rootDir, 'VERSION'), `${LEGACY_SHIM_VERSION}\n`, 0o644) + neutralized = true + clearNeutralizationRetry() } catch { - // Best-effort: a locked file must not block startup, and the next launch retries. + // Why: Windows can keep a running .cmd open briefly after startup. + scheduleNeutralizationRetry(userDataPath) + } +} + +function scheduleNeutralizationRetry(userDataPath: string): void { + if ( + neutralizationRetryTimer || + neutralizationRetryAttempt >= NEUTRALIZATION_RETRY_DELAYS_MS.length + ) { + return + } + const delayMs = NEUTRALIZATION_RETRY_DELAYS_MS[neutralizationRetryAttempt] + neutralizationRetryAttempt += 1 + neutralizationRetryTimer = setTimeout(() => { + neutralizationRetryTimer = null + neutralizeLegacyTerminalShimDir(userDataPath) + }, delayMs) + neutralizationRetryTimer.unref?.() +} + +function clearNeutralizationRetry(): void { + if (neutralizationRetryTimer) { + clearTimeout(neutralizationRetryTimer) + neutralizationRetryTimer = null + } + neutralizationRetryAttempt = 0 +} + +function writeNeutralWrappers(rootDir: string): void { + const posixDir = join(rootDir, 'posix') + const win32Dir = join(rootDir, 'win32') + mkdirSync(posixDir, { recursive: true }) + mkdirSync(win32Dir, { recursive: true }) + for (const command of ['git', 'gh']) { + const upperCommand = command.toUpperCase() + writeFileAtomically( + join(posixDir, command), + POSIX_PASSTHROUGH_WRAPPER.replaceAll('__ORCA_COMMAND__', command), + 0o755 + ) + writeFileAtomically( + join(win32Dir, `${command}.cmd`), + renderWindowsWrapper(WIN32_PASSTHROUGH_WRAPPER, command, upperCommand), + 0o755 + ) + writeFileAtomically( + join(win32Dir, `${command}-wrapper.ps1`), + renderWindowsWrapper(POWERSHELL_PASSTHROUGH_WRAPPER, command, upperCommand), + 0o755 + ) + } +} + +function renderWindowsWrapper(template: string, command: string, upperCommand: string): string { + return template + .replaceAll('__ORCA_UPPER_COMMAND__', upperCommand) + .replaceAll('__ORCA_COMMAND__', command) +} + +function writeFileAtomically(filePath: string, contents: string, mode: number): void { + const temporaryPath = `${filePath}.orca-neutralizing-${process.pid}` + try { + rmSync(temporaryPath, { force: true, recursive: true }) + writeFileSync(temporaryPath, contents, { encoding: 'utf8', flag: 'wx', mode }) + chmodSync(temporaryPath, mode) + renameSync(temporaryPath, filePath) + } finally { + rmSync(temporaryPath, { force: true, recursive: true }) } } export function isLegacyTerminalShimPathEntry(entry: string): boolean { - return entry.replaceAll('\\', '/').toLowerCase().includes(`/${LEGACY_SHIM_ROOT_DIR}/`) + const normalized = entry.replaceAll('\\', '/').replace(/\/+$/, '').toLowerCase() + return ( + normalized.endsWith(`/${LEGACY_SHIM_ROOT_DIR}/posix`) || + normalized.endsWith(`/${LEGACY_SHIM_ROOT_DIR}/win32`) + ) } export function stripLegacyTerminalShimEnv( env: Record, platform: NodeJS.Platform = process.platform ): void { - for (const key of LEGACY_SHIM_ENV_KEYS) { - delete env[key] + const windows = platform === 'win32' + const legacyKeySet = new Set( + LEGACY_TERMINAL_SHIM_ENV_KEYS.map((key) => (windows ? key.toLowerCase() : key)) + ) + const shimDirKey = 'ORCA_ATTRIBUTION_SHIM_DIR'.toLowerCase() + const explicitShimDirs = Object.entries(env) + .filter(([key]) => + windows ? key.toLowerCase() === shimDirKey : key === 'ORCA_ATTRIBUTION_SHIM_DIR' + ) + .map(([, value]) => value) + .filter(Boolean) + for (const key of Object.keys(env)) { + if (windows ? legacyKeySet.has(key.toLowerCase()) : legacyKeySet.has(key)) { + delete env[key] + } } - const pathKey = resolvePathEnvKey(env, platform) - const current = env[pathKey] - if (!current) { - return + const delimiter = windows ? ';' : ':' + const pathKeys = windows + ? Object.keys(env).filter((key) => key.toLowerCase() === 'path') + : ['PATH'] + for (const pathKey of pathKeys) { + const current = env[pathKey] + if (!current) { + continue + } + const withoutExplicitDirs = explicitShimDirs.reduce( + (pathValue, shimDir) => removeLiteralPathEntry(pathValue, shimDir, delimiter, windows), + current + ) + const cleaned = withoutExplicitDirs + .split(delimiter) + .filter((entry) => entry && !isLegacyTerminalShimPathEntry(entry)) + .join(delimiter) + if (cleaned) { + env[pathKey] = cleaned + } else { + delete env[pathKey] + } } - const delimiter = platform === 'win32' ? ';' : ':' - const cleaned = current - .split(delimiter) - .filter((entry) => entry && !isLegacyTerminalShimPathEntry(entry)) - .join(delimiter) - if (cleaned) { - env[pathKey] = cleaned - } else { - delete env[pathKey] +} + +function removeLiteralPathEntry( + pathValue: string, + entry: string, + delimiter: string, + caseInsensitive: boolean +): string { + let result = pathValue + const needle = caseInsensitive ? entry.toLowerCase() : entry + let searchStart = 0 + for (;;) { + const comparable = caseInsensitive ? result.toLowerCase() : result + const index = comparable.indexOf(needle, searchStart) + if (index === -1) { + return result + } + const end = index + entry.length + const startsAtBoundary = index === 0 || result[index - 1] === delimiter + const endsAtBoundary = end === result.length || result[end] === delimiter + if (!startsAtBoundary || !endsAtBoundary) { + searchStart = index + 1 + continue + } + if (result[end] === delimiter) { + result = result.slice(0, index) + result.slice(end + 1) + } else if (index > 0) { + result = result.slice(0, index - 1) + result.slice(end) + } else { + result = '' + } + searchStart = Math.max(0, index - 1) } } /** Test-only: the module-level once-guard would otherwise leak across cases. */ -export function __resetLegacyTerminalShimRemovalForTests(): void { - removed = false +export function __resetLegacyTerminalShimNeutralizationForTests(): void { + neutralized = false + clearNeutralizationRetry() } diff --git a/src/main/runtime/rpc/core.ts b/src/main/runtime/rpc/core.ts index b415ddd0e71..fa3c812ce82 100644 --- a/src/main/runtime/rpc/core.ts +++ b/src/main/runtime/rpc/core.ts @@ -46,6 +46,7 @@ export type RpcRequest = { authToken: string method: string params?: unknown + expectedRuntimeId?: string orchestrationCapability?: string orchestrationContractVersion?: number orchestrationRequestId?: string diff --git a/src/main/runtime/rpc/dispatcher-computer-errors.test.ts b/src/main/runtime/rpc/dispatcher-computer-errors.test.ts index 862973e3913..10bc94b771b 100644 --- a/src/main/runtime/rpc/dispatcher-computer-errors.test.ts +++ b/src/main/runtime/rpc/dispatcher-computer-errors.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import { z } from 'zod' import { RpcDispatcher } from './dispatcher' import { defineMethod, InvalidArgumentError, type RpcRequest } from './core' @@ -41,6 +41,26 @@ const METHODS = [ ] describe('RpcDispatcher computer-use validation errors', () => { + it('rejects a replacement runtime before invoking a one-shot handler', async () => { + const handler = vi.fn(() => ({ ok: true })) + const dispatcher = new RpcDispatcher({ + runtime: makeRuntime(), + methods: [defineMethod({ name: 'terminal.create', params: z.object({}), handler })] + }) + + const response = await dispatcher.dispatch({ + ...makeRequest('terminal.create', {}), + expectedRuntimeId: 'previous-runtime' + }) + + expect(response).toMatchObject({ + ok: false, + error: { code: 'runtime_replaced' }, + _meta: { runtimeId: 'test-runtime' } + }) + expect(handler).not.toHaveBeenCalled() + }) + it('adds recovery steps to one-shot computer schema failures', async () => { const dispatcher = new RpcDispatcher({ runtime: makeRuntime(), methods: METHODS }) diff --git a/src/main/runtime/rpc/dispatcher.ts b/src/main/runtime/rpc/dispatcher.ts index be11b2b0c64..995d0f00d64 100644 --- a/src/main/runtime/rpc/dispatcher.ts +++ b/src/main/runtime/rpc/dispatcher.ts @@ -43,6 +43,14 @@ export class RpcDispatcher { async dispatch(request: RpcRequest, options?: { signal?: AbortSignal }): Promise { const meta = this.meta() + if (request.expectedRuntimeId && request.expectedRuntimeId !== meta.runtimeId) { + return errorResponse( + request.id, + meta, + 'runtime_replaced', + 'The runtime changed before the request could be applied. Refresh and try again.' + ) + } const method = this.registry.get(request.method) if (!method) { return errorResponse( @@ -139,6 +147,19 @@ export class RpcDispatcher { options?: RpcDispatchStreamingOptions ): Promise { const meta = this.meta() + if (request.expectedRuntimeId && request.expectedRuntimeId !== meta.runtimeId) { + reply( + JSON.stringify( + errorResponse( + request.id, + meta, + 'runtime_replaced', + 'The runtime changed before the request could be applied. Refresh and try again.' + ) + ) + ) + return + } const method = this.registry.get(request.method) if (!method) { reply( diff --git a/src/main/runtime/rpc/methods/terminal-split-env.test.ts b/src/main/runtime/rpc/methods/terminal-split-env.test.ts new file mode 100644 index 00000000000..3bbf1291f00 --- /dev/null +++ b/src/main/runtime/rpc/methods/terminal-split-env.test.ts @@ -0,0 +1,37 @@ +import { describe, expect, it, vi } from 'vitest' +import type { OrcaRuntimeService } from '../../orca-runtime' +import type { RpcRequest } from '../core' +import { RpcDispatcher } from '../dispatcher' +import { TERMINAL_METHODS } from './terminal' + +describe('terminal split environment forwarding', () => { + it('forwards requested environment deletions to the runtime owner', async () => { + const splitTerminal = vi.fn().mockResolvedValue({ handle: 'term-split' }) + const runtime = { + getRuntimeId: () => 'runtime-1', + splitTerminal + } as unknown as OrcaRuntimeService + const dispatcher = new RpcDispatcher({ runtime, methods: TERMINAL_METHODS }) + const request: RpcRequest = { + id: 'split-1', + authToken: 'token', + method: 'terminal.split', + params: { + terminal: 'term-1', + direction: 'horizontal', + env: { ORCA_ATTRIBUTION_BYPASS: '1' }, + envToDelete: ['ORCA_ENABLE_GIT_ATTRIBUTION'] + } + } + + await dispatcher.dispatch(request) + + expect(splitTerminal).toHaveBeenCalledWith('term-1', { + direction: 'horizontal', + command: undefined, + env: { ORCA_ATTRIBUTION_BYPASS: '1' }, + envToDelete: ['ORCA_ENABLE_GIT_ATTRIBUTION'], + telemetrySource: undefined + }) + }) +}) diff --git a/src/main/runtime/rpc/methods/terminal.ts b/src/main/runtime/rpc/methods/terminal.ts index 69f4eb72190..32c31149f3f 100644 --- a/src/main/runtime/rpc/methods/terminal.ts +++ b/src/main/runtime/rpc/methods/terminal.ts @@ -985,6 +985,7 @@ const TerminalSplit = TerminalHandle.extend({ .optional(), command: OptionalString, env: z.record(z.string(), z.string()).optional(), + envToDelete: z.array(z.string().min(1).max(256)).max(32).optional(), telemetrySource: z.enum(TERMINAL_PANE_SPLIT_SOURCES).optional() }) @@ -1472,6 +1473,7 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ direction: params.direction, command: params.command, env: params.env, + envToDelete: params.envToDelete, telemetrySource: params.telemetrySource }) }) diff --git a/src/main/runtime/rpc/streaming.test.ts b/src/main/runtime/rpc/streaming.test.ts index 382353de803..4b4ce8a9f21 100644 --- a/src/main/runtime/rpc/streaming.test.ts +++ b/src/main/runtime/rpc/streaming.test.ts @@ -231,6 +231,28 @@ describe('RpcDispatcher streaming', () => { expect(response.streaming).toBeUndefined() }) + it('rejects a replacement runtime before streaming dispatch invokes a mutation', async () => { + const messages: string[] = [] + const handler = vi.fn(() => ({ terminal: { handle: 'terminal-1' } })) + const dispatcher = new RpcDispatcher({ + runtime: stubRuntime(), + methods: [defineMethod({ name: 'terminal.create', params: z.object({}), handler })] + }) + + await dispatcher.dispatchStreaming( + { ...makeRequest('terminal.create', {}), expectedRuntimeId: 'previous-runtime' }, + (message) => messages.push(message) + ) + + expect(messages).toHaveLength(1) + expect(JSON.parse(messages[0]!)).toMatchObject({ + ok: false, + error: { code: 'runtime_replaced' }, + _meta: { runtimeId: 'test-runtime' } + }) + expect(handler).not.toHaveBeenCalled() + }) + it('returns error for unknown method via dispatchStreaming', async () => { const messages: string[] = [] const dispatcher = new RpcDispatcher({ diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index 3401c8939f6..0f90182d053 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -3413,6 +3413,7 @@ export type PreloadApi = { params?: unknown timeoutMs?: number expectedEnvironmentPairingRevision?: number + expectedRuntimeId?: string }) => Promise> subscribe: ( args: { @@ -3421,6 +3422,7 @@ export type PreloadApi = { params?: unknown timeoutMs?: number expectedEnvironmentPairingRevision?: number + expectedRuntimeId?: string }, callbacks: { onResponse: (response: RuntimeRpcResponse) => void diff --git a/src/preload/index.ts b/src/preload/index.ts index 040d7b63cf3..58aec899f03 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -4428,6 +4428,7 @@ const api = { params?: unknown timeoutMs?: number expectedEnvironmentPairingRevision?: number + expectedRuntimeId?: string }): Promise> => ipcRenderer.invoke('runtimeEnvironments:call', args), subscribe: async ( @@ -4437,6 +4438,7 @@ const api = { params?: unknown timeoutMs?: number expectedEnvironmentPairingRevision?: number + expectedRuntimeId?: string }, callbacks: { onResponse: (response: RuntimeRpcResponse) => void diff --git a/src/preload/runtime-environment-subscriptions.test.ts b/src/preload/runtime-environment-subscriptions.test.ts index 585b0f75fa9..a77614e1196 100644 --- a/src/preload/runtime-environment-subscriptions.test.ts +++ b/src/preload/runtime-environment-subscriptions.test.ts @@ -68,7 +68,12 @@ describe('subscribeRuntimeEnvironmentFromPreload', () => { const cleanupPromise = subscribeRuntimeEnvironmentFromPreload( ipc, - { selector: 'desk', method: 'terminal.subscribe' }, + { + selector: 'desk', + method: 'terminal.subscribe', + expectedEnvironmentPairingRevision: 7, + expectedRuntimeId: 'runtime-1' + }, { onResponse, onBinary }, () => 'sub-1' ) @@ -80,6 +85,8 @@ describe('subscribeRuntimeEnvironmentFromPreload', () => { expect(ipc.invoke).toHaveBeenCalledWith('runtimeEnvironments:subscribe', { selector: 'desk', method: 'terminal.subscribe', + expectedEnvironmentPairingRevision: 7, + expectedRuntimeId: 'runtime-1', subscriptionId: 'sub-1' }) diff --git a/src/preload/runtime-environment-subscriptions.ts b/src/preload/runtime-environment-subscriptions.ts index f9d46b467aa..c804c651f73 100644 --- a/src/preload/runtime-environment-subscriptions.ts +++ b/src/preload/runtime-environment-subscriptions.ts @@ -5,6 +5,8 @@ type RuntimeEnvironmentSubscribeArgs = { method: string params?: unknown timeoutMs?: number + expectedEnvironmentPairingRevision?: number + expectedRuntimeId?: string } type RuntimeEnvironmentSubscriptionCallbacks = { diff --git a/src/relay/pty-handler.test.ts b/src/relay/pty-handler.test.ts index b97c1bd0847..06eaa15d5ff 100644 --- a/src/relay/pty-handler.test.ts +++ b/src/relay/pty-handler.test.ts @@ -11,6 +11,7 @@ import { SETUP_AGENT_SEQUENCE_STARTUP_COMMAND_ENV } from '../shared/setup-agent-sequencing' import { PTY_STARTUP_INGRESS_VERSION } from '../shared/pty-startup-ingress' +import { stripLegacyTerminalShimEnv } from '../main/pty/legacy-terminal-shim-dir' const { mockPtySpawn, mockPtyInstance, mockCreateShellPromptReadinessProbe } = vi.hoisted(() => ({ mockPtySpawn: vi.fn(), @@ -308,7 +309,56 @@ describe('PtyHandler', () => { const spawnOptions = mockPtySpawn.mock.calls[0][2] as { env: Record } expect(spawnOptions.env.NODE_ENV).toBeUndefined() - expect(spawnOptions.env.PATH).toBe(process.env.PATH) + const expectedEnv = { PATH: process.env.PATH ?? '' } + stripLegacyTerminalShimEnv(expectedEnv, process.platform) + expect(spawnOptions.env.PATH).toBe(expectedEnv.PATH) + }) + + it('does not inherit legacy attribution state from the relay process', async () => { + const keys = ['ORCA_ENABLE_GIT_ATTRIBUTION', 'ORCA_ATTRIBUTION_SHIM_DIR', 'PATH'] as const + const saved = Object.fromEntries(keys.map((key) => [key, process.env[key]])) + process.env.ORCA_ENABLE_GIT_ATTRIBUTION = '1' + process.env.ORCA_ATTRIBUTION_SHIM_DIR = '/tmp/orca-terminal-attribution/posix' + process.env.PATH = '/tmp/orca-terminal-attribution/posix:/usr/bin' + + try { + await dispatcher.callRequest('pty.spawn', { cols: 80, rows: 24 }) + const spawnedEnv = mockPtySpawn.mock.calls.at(-1)?.[2] as { + env: Record + } + expect(spawnedEnv.env.PATH).toBe('/usr/bin') + expect(spawnedEnv.env.ORCA_ENABLE_GIT_ATTRIBUTION).toBeUndefined() + expect(spawnedEnv.env.ORCA_ATTRIBUTION_SHIM_DIR).toBeUndefined() + + const state = (await dispatcher.callRequest('pty.serialize', { + ids: ['pty-1'] + })) as string + await handler.dispose({ waitForPhysicalExit: false }) + mockPtySpawn.mockClear() + dispatcher = createMockDispatcher() + handler = new PtyHandler(dispatcher as unknown as RelayDispatcher) + const killSpy = vi.spyOn(process, 'kill').mockImplementation(() => true) + try { + await dispatcher.callRequest('pty.revive', { state }) + } finally { + killSpy.mockRestore() + } + + const revivedEnv = mockPtySpawn.mock.calls.at(-1)?.[2] as { + env: Record + } + expect(revivedEnv.env.PATH).toBe('/usr/bin') + expect(revivedEnv.env.ORCA_ENABLE_GIT_ATTRIBUTION).toBeUndefined() + expect(revivedEnv.env.ORCA_ATTRIBUTION_SHIM_DIR).toBeUndefined() + } finally { + for (const [key, value] of Object.entries(saved)) { + if (value === undefined) { + delete process.env[key] + } else { + process.env[key] = value + } + } + } }) it('keeps a renderer-supplied NODE_ENV for the spawned shell', async () => { diff --git a/src/relay/pty-handler.ts b/src/relay/pty-handler.ts index e7c09df95de..3f8bd593e08 100644 --- a/src/relay/pty-handler.ts +++ b/src/relay/pty-handler.ts @@ -47,6 +47,7 @@ import { isTuiAgent } from '../shared/tui-agent-config' import type { TuiAgent } from '../shared/types' import { forceKillPosixPtyProcessGroups } from '../main/pty/posix-pty-process-groups' import { stripInheritedBuildModeEnv } from '../main/pty/build-mode-env' +import { stripLegacyTerminalShimEnv } from '../main/pty/legacy-terminal-shim-dir' import { PTY_STARTUP_INGRESS_VERSION, PtyStartupIngress, @@ -615,6 +616,8 @@ export class PtyHandler { } } const result = mergeGitConfigEnvProtocol(baseEnv, augmented) as Record + // Why: an older client may not ask a newly upgraded relay to delete inherited shim state. + stripLegacyTerminalShimEnv(result, process.platform) // Why: match local/daemon precedence so defaults/augmenters can't resurrect explicitly-removed values. for (const key of envToDelete) { delete result[key] diff --git a/src/renderer/src/components/terminal-pane/pty-transport.test.ts b/src/renderer/src/components/terminal-pane/pty-transport.test.ts index 1954b857d70..a86b0c6bd1e 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport.test.ts @@ -9,6 +9,7 @@ import { encodeTerminalStreamText } from '../../../../shared/terminal-stream-protocol' import { createTerminalSessionStateSaveFailureMessage } from '../../../../shared/terminal-session-state-save-failure' +import { TERMINAL_ATTRIBUTION_REMOVED_RUNTIME_CAPABILITY } from '../../../../shared/protocol-version' import { TERMINAL_INPUT_CHUNK_MAX_BYTES, TERMINAL_INPUT_MAX_BYTES @@ -2512,7 +2513,10 @@ describe('createRemoteRuntimePtyTransport', () => { result: { runtimeProtocolVersion: 3, minCompatibleRuntimeClientVersion: 2, - capabilities: ['agent-session.host-authority.v1'] + capabilities: [ + 'agent-session.host-authority.v1', + TERMINAL_ATTRIBUTION_REMOVED_RUNTIME_CAPABILITY + ] }, _meta: { runtimeId: 'runtime-remote' } } @@ -2611,12 +2615,14 @@ describe('createRemoteRuntimePtyTransport', () => { worktree: 'id:repo1::/remote/wt', clientMutationId: expect.any(String), command: 'claude', - env: { ORCA_TAB_ID: 'tab-1' }, + env: { ORCA_TAB_ID: 'tab-1', ORCA_ATTRIBUTION_BYPASS: '1' }, + envToDelete: ['ORCA_ENABLE_GIT_ATTRIBUTION'], tabId: 'tab-1', leafId: '11111111-1111-4111-8111-111111111111', focus: false, presentation: 'background' }, + expectedEnvironmentPairingRevision: undefined, timeoutMs: 15_000 }) expect(runtimeSubscribe).toHaveBeenCalledWith( @@ -2668,20 +2674,34 @@ describe('createRemoteRuntimePtyTransport', () => { }) it('reports a host stable-pane adoption as reattach without fresh-spawn ownership', async () => { - runtimeCall.mockResolvedValue({ - id: 'rpc-create', - ok: true, - result: { - terminal: { - handle: 'term-original', - worktreeId: 'repo1::/remote/wt', - title: 'Original', - surface: 'background', - isReattach: true - } - }, - _meta: { runtimeId: 'runtime-remote' } - }) + runtimeCall.mockImplementation(async (args: { method?: string }) => + args.method === 'status.get' + ? { + id: 'rpc-status', + ok: true, + result: { + runtimeId: 'runtime-remote', + runtimeProtocolVersion: 3, + minCompatibleRuntimeClientVersion: 2, + capabilities: [TERMINAL_ATTRIBUTION_REMOVED_RUNTIME_CAPABILITY] + }, + _meta: { runtimeId: 'runtime-remote' } + } + : { + id: 'rpc-create', + ok: true, + result: { + terminal: { + handle: 'term-original', + worktreeId: 'repo1::/remote/wt', + title: 'Original', + surface: 'background', + isReattach: true + } + }, + _meta: { runtimeId: 'runtime-remote' } + } + ) const onPtySpawn = vi.fn() const onReattachDetermined = vi.fn() const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') @@ -2809,6 +2829,7 @@ describe('createRemoteRuntimePtyTransport', () => { }, presentation: 'background' }, + expectedEnvironmentPairingRevision: undefined, timeoutMs: 15_000 }) expect(runtimeCall).not.toHaveBeenCalledWith( @@ -2834,9 +2855,13 @@ describe('createRemoteRuntimePtyTransport', () => { id: 'rpc-status', ok: true, result: { + runtimeId: 'runtime-remote', runtimeProtocolVersion: 3, minCompatibleRuntimeClientVersion: 2, - capabilities: ['agent-session.host-authority.v1'] + capabilities: [ + 'agent-session.host-authority.v1', + TERMINAL_ATTRIBUTION_REMOVED_RUNTIME_CAPABILITY + ] } } ) @@ -2882,6 +2907,7 @@ describe('createRemoteRuntimePtyTransport', () => { }, presentation: 'background' }, + expectedEnvironmentPairingRevision: undefined, timeoutMs: 15_000 }) expect(runtimeCall).not.toHaveBeenCalledWith( diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts index 4087cf59a15..c160188f64d 100644 --- a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts @@ -14,7 +14,12 @@ import { TERMINAL_INPUT_MAX_BYTES } from '../../../../shared/terminal-input' import { CLIPBOARD_TEXT_MEASURE_YIELD_CODE_UNITS } from '../../../../shared/clipboard-text' -import { TERMINAL_CREATE_IDEMPOTENCY_RUNTIME_CAPABILITY } from '../../../../shared/protocol-version' +import { + MIN_COMPATIBLE_RUNTIME_CLIENT_VERSION, + RUNTIME_PROTOCOL_VERSION, + TERMINAL_ATTRIBUTION_REMOVED_RUNTIME_CAPABILITY, + TERMINAL_CREATE_IDEMPOTENCY_RUNTIME_CAPABILITY +} from '../../../../shared/protocol-version' describe('createRemoteRuntimePtyTransport', () => { const runtimeCall = vi.fn() @@ -113,6 +118,19 @@ describe('createRemoteRuntimePtyTransport', () => { } } + function currentRuntimeStatus(capabilities: string[] = []): unknown { + return { + ok: true, + result: { + runtimeId: 'runtime-1', + graphStatus: 'ready', + runtimeProtocolVersion: RUNTIME_PROTOCOL_VERSION, + minCompatibleRuntimeClientVersion: MIN_COMPATIBLE_RUNTIME_CLIENT_VERSION, + capabilities: [TERMINAL_ATTRIBUTION_REMOVED_RUNTIME_CAPABILITY, ...capabilities] + } + } + } + function emitOutput(streamId: number, data: string): void { subscriptionCallbacks?.onBinary?.( encodeTerminalStreamFrame({ @@ -187,6 +205,18 @@ describe('createRemoteRuntimePtyTransport', () => { subscriptionSendBinary.mockReset() refreshSessionTabsSnapshot.mockClear() runtimeCall.mockImplementation(async (request: { method: string; params?: unknown }) => { + if (request.method === 'status.get') { + return { + ok: true, + result: { + runtimeId: 'runtime-1', + graphStatus: 'ready', + runtimeProtocolVersion: RUNTIME_PROTOCOL_VERSION, + minCompatibleRuntimeClientVersion: MIN_COMPATIBLE_RUNTIME_CLIENT_VERSION, + capabilities: [TERMINAL_ATTRIBUTION_REMOVED_RUNTIME_CAPABILITY] + } + } + } if (request.method === 'session.tabs.activate') { const params = request.params as { tabId: string; leafId?: string } const resolvedLeafId = params.leafId ?? 'pane:1' @@ -541,10 +571,7 @@ describe('createRemoteRuntimePtyTransport', () => { let createCalls = 0 runtimeCall.mockImplementation(async (args: { method: string; params?: unknown }) => { if (args.method === 'status.get') { - return { - ok: true, - result: { capabilities: [TERMINAL_CREATE_IDEMPOTENCY_RUNTIME_CAPABILITY] } - } + return currentRuntimeStatus([TERMINAL_CREATE_IDEMPOTENCY_RUNTIME_CAPABILITY]) } if (args.method === 'terminal.create') { createCalls += 1 @@ -592,13 +619,14 @@ describe('createRemoteRuntimePtyTransport', () => { try { const startedAt = Date.now() let createCalls = 0 + let statusCalls = 0 runtimeCall.mockImplementation(async (args: { method: string }) => { if (args.method === 'status.get') { - vi.setSystemTime(startedAt + 59_000) - return { - ok: true, - result: { capabilities: [TERMINAL_CREATE_IDEMPOTENCY_RUNTIME_CAPABILITY] } + statusCalls += 1 + if (statusCalls > 1) { + vi.setSystemTime(startedAt + 59_000) } + return currentRuntimeStatus([TERMINAL_CREATE_IDEMPOTENCY_RUNTIME_CAPABILITY]) } if (args.method === 'terminal.create') { createCalls += 1 @@ -632,7 +660,7 @@ describe('createRemoteRuntimePtyTransport', () => { it('does not retry an unknown create outcome against an older runtime', async () => { runtimeCall.mockImplementation(async (args: { method: string }) => { if (args.method === 'status.get') { - return { ok: true, result: { capabilities: [] } } + return currentRuntimeStatus() } throw Object.assign(new Error('Timed out waiting for the remote Orca runtime.'), { code: 'runtime_timeout' @@ -653,12 +681,16 @@ describe('createRemoteRuntimePtyTransport', () => { }) it('surfaces an authoritative capability-probe failure after an unknown create outcome', async () => { + let statusCalls = 0 runtimeCall.mockImplementation(async (args: { method: string }) => { - if (args.method === 'status.get') { + if (args.method === 'status.get' && ++statusCalls > 1) { throw Object.assign(new Error('Remote runtime pairing credentials expired.'), { code: 'unauthorized' }) } + if (args.method === 'status.get') { + return currentRuntimeStatus([TERMINAL_CREATE_IDEMPOTENCY_RUNTIME_CAPABILITY]) + } throw Object.assign(new Error('Timed out waiting for the remote Orca runtime.'), { code: 'runtime_timeout' }) @@ -694,10 +726,7 @@ describe('createRemoteRuntimePtyTransport', () => { }, args.timeoutMs) }) } - return { - ok: true, - result: { capabilities: [TERMINAL_CREATE_IDEMPOTENCY_RUNTIME_CAPABILITY] } - } + return currentRuntimeStatus([TERMINAL_CREATE_IDEMPOTENCY_RUNTIME_CAPABILITY]) } if (args.method === 'terminal.create' && reachable) { return { ok: true, result: { terminal: { handle: 'terminal-recovered' } } } @@ -763,9 +792,10 @@ describe('createRemoteRuntimePtyTransport', () => { return { ok: true, result: { + runtimeId: 'runtime-1', runtimeProtocolVersion: 3, minCompatibleRuntimeClientVersion: 2, - capabilities: ['agent-session.host-authority.v1'] + capabilities: ['agent-session.host-authority.v1', 'terminal.attribution-removed.v1'] } } } @@ -813,9 +843,65 @@ describe('createRemoteRuntimePtyTransport', () => { .filter((args) => args.method === 'terminal.createAgentSession') expect(allCreates.every((args) => args.params?.clientOperationId === operationId)).toBe(true) expect(runtimeCall.mock.calls.some(([args]) => args.method === 'terminal.create')).toBe(false) - expect(runtimeCall.mock.calls.filter(([args]) => args.method === 'status.get')).toHaveLength( - 1 + expect( + runtimeCall.mock.calls.filter(([args]) => args.method === 'status.get').length + ).toBeGreaterThan(1) + transport.destroy?.() + } finally { + vi.useRealTimers() + } + }) + + it('refuses structured replay after an in-place host downgrade', async () => { + vi.useFakeTimers() + try { + let statusCalls = 0 + const onError = vi.fn() + runtimeCall.mockImplementation(async (args: { method: string }) => { + if (args.method === 'status.get') { + statusCalls += 1 + return { + ok: true, + result: { + runtimeId: 'runtime-1', + runtimeProtocolVersion: 3, + minCompatibleRuntimeClientVersion: 2, + capabilities: + statusCalls === 1 + ? ['agent-session.host-authority.v1', 'terminal.attribution-removed.v1'] + : ['agent-session.host-authority.v1'] + } + } + } + if (args.method === 'terminal.createAgentSession') { + throw Object.assign(new Error('Timed out waiting for the remote Orca runtime.'), { + code: 'runtime_timeout' + }) + } + return { ok: true, result: {} } + }) + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const transport = createRemoteRuntimePtyTransport('env-1', { + worktreeId: 'wt-1', + tabId: 'tab-1', + leafId: 'pane:1', + launchAgent: 'codex' + }) + + const connect = transport.connect({ url: '', callbacks: { onError } }) + await vi.advanceTimersByTimeAsync(250) + await connect + + expect(onError).toHaveBeenCalledWith( + expect.stringContaining('no fallback launch was attempted') ) + expect( + runtimeCall.mock.calls.filter(([args]) => args.method === 'terminal.createAgentSession') + ).toHaveLength(1) + expect(runtimeCall.mock.calls.filter(([args]) => args.method === 'status.get')).toHaveLength( + 2 + ) + expect(runtimeCall.mock.calls.some(([args]) => args.method === 'terminal.create')).toBe(false) transport.destroy?.() } finally { vi.useRealTimers() @@ -3855,6 +3941,9 @@ describe('createRemoteRuntimePtyTransport', () => { it('closes a remote terminal created after the pane was destroyed', async () => { let resolveCreate: (value: unknown) => void = () => {} runtimeCall.mockImplementation((args) => { + if (args.method === 'status.get') { + return Promise.resolve(currentRuntimeStatus()) + } if (args.method === 'terminal.create') { return new Promise((resolve) => { resolveCreate = resolve @@ -3870,6 +3959,11 @@ describe('createRemoteRuntimePtyTransport', () => { }) const connect = transport.connect({ url: '', callbacks: {} }) + await vi.waitFor(() => + expect(runtimeCall).toHaveBeenCalledWith( + expect.objectContaining({ method: 'terminal.create' }) + ) + ) transport.destroy?.() resolveCreate({ ok: true, result: { terminal: { handle: 'terminal-late' } } }) await connect @@ -3878,13 +3972,17 @@ describe('createRemoteRuntimePtyTransport', () => { selector: 'env-1', method: 'terminal.close', params: { terminal: 'terminal-late' }, - timeoutMs: 15_000 + timeoutMs: 15_000, + expectedEnvironmentPairingRevision: undefined }) }) it('cannot let a stale create completion replace a newer attached terminal', async () => { let resolveCreate: (value: unknown) => void = () => {} runtimeCall.mockImplementation((args) => { + if (args.method === 'status.get') { + return Promise.resolve(currentRuntimeStatus()) + } if (args.method === 'terminal.create') { return new Promise((resolve) => { resolveCreate = resolve @@ -3900,6 +3998,11 @@ describe('createRemoteRuntimePtyTransport', () => { }) const connect = transport.connect({ url: '', callbacks: {} }) + await vi.waitFor(() => + expect(runtimeCall).toHaveBeenCalledWith( + expect.objectContaining({ method: 'terminal.create' }) + ) + ) transport.attach({ existingPtyId: 'remote:env-2@@terminal-attached', callbacks: {} }) resolveCreate({ ok: true, result: { terminal: { handle: 'terminal-late' } } }) await connect @@ -3910,7 +4013,8 @@ describe('createRemoteRuntimePtyTransport', () => { selector: 'env-1', method: 'terminal.close', params: { terminal: 'terminal-late' }, - timeoutMs: 15_000 + timeoutMs: 15_000, + expectedEnvironmentPairingRevision: undefined }) transport.destroy?.() }) @@ -3924,7 +4028,7 @@ describe('createRemoteRuntimePtyTransport', () => { result: { runtimeProtocolVersion: 3, minCompatibleRuntimeClientVersion: 2, - capabilities: ['agent-session.host-authority.v1'] + capabilities: ['agent-session.host-authority.v1', 'terminal.attribution-removed.v1'] } }) } @@ -3974,7 +4078,7 @@ describe('createRemoteRuntimePtyTransport', () => { result: { runtimeProtocolVersion: 3, minCompatibleRuntimeClientVersion: 2, - capabilities: ['agent-session.host-authority.v1'] + capabilities: ['agent-session.host-authority.v1', 'terminal.attribution-removed.v1'] } }) } @@ -4089,7 +4193,7 @@ describe('createRemoteRuntimePtyTransport', () => { method: 'terminal.create', params: expect.objectContaining({ command: "codex 'linked issue context'", - envToDelete: ['CODEX_HOME', 'ORCA_CODEX_HOME'], + envToDelete: ['CODEX_HOME', 'ORCA_CODEX_HOME', 'ORCA_ENABLE_GIT_ATTRIBUTION'], startupCommandDelivery: 'shell-ready', terminalColorQueryReplies: { foreground: '#ffffff', background: '#282c34' } }) @@ -4105,7 +4209,11 @@ describe('createRemoteRuntimePtyTransport', () => { result: { runtimeProtocolVersion: 3, minCompatibleRuntimeClientVersion: 2, - capabilities: ['agent-session.host-authority.v1', 'agent-session.omp-resume-path.v1'] + capabilities: [ + 'agent-session.host-authority.v1', + 'agent-session.omp-resume-path.v1', + 'terminal.attribution-removed.v1' + ] } } : { ok: true, result: { terminal: { handle: 'terminal-1' } } } @@ -4169,7 +4277,7 @@ describe('createRemoteRuntimePtyTransport', () => { result: { runtimeProtocolVersion: 3, minCompatibleRuntimeClientVersion: 2, - capabilities: ['agent-session.host-authority.v1'] + capabilities: ['agent-session.host-authority.v1', 'terminal.attribution-removed.v1'] } } : { @@ -4219,9 +4327,10 @@ describe('createRemoteRuntimePtyTransport', () => { ? { ok: true, result: { + runtimeId: 'runtime-1', runtimeProtocolVersion: 3, minCompatibleRuntimeClientVersion: 2, - capabilities: [] + capabilities: [TERMINAL_ATTRIBUTION_REMOVED_RUNTIME_CAPABILITY] } } : { ok: true, result: { terminal: { handle: 'terminal-legacy' } } } @@ -4257,7 +4366,12 @@ describe('createRemoteRuntimePtyTransport', () => { worktree: 'id:wt-1', clientMutationId: expect.any(String), command: "codex '--model' 'gpt-5' 'resume' 'session-1'", - env: { CODEX_PROFILE: 'captured', ORCA_AGENT_LAUNCH_TOKEN: 'fresh-token' }, + env: { + CODEX_PROFILE: 'captured', + ORCA_AGENT_LAUNCH_TOKEN: 'fresh-token', + ORCA_ATTRIBUTION_BYPASS: '1' + }, + envToDelete: ['ORCA_ENABLE_GIT_ATTRIBUTION'], launchConfig: { agentArgs: '--model gpt-5', agentEnv: { CODEX_PROFILE: 'captured' } @@ -4269,7 +4383,9 @@ describe('createRemoteRuntimePtyTransport', () => { focus: false, presentation: 'background' }, - timeoutMs: 15_000 + timeoutMs: 15_000, + expectedEnvironmentPairingRevision: undefined, + expectedRuntimeId: 'runtime-1' }) expect(runtimeCall).not.toHaveBeenCalledWith( expect.objectContaining({ method: 'terminal.createAgentSession' }) @@ -5686,6 +5802,9 @@ describe('createRemoteRuntimePtyTransport', () => { it('returns runtime acceptance for acknowledged terminal input', async () => { runtimeCall.mockImplementation((args) => { + if (args.method === 'status.get') { + return Promise.resolve(currentRuntimeStatus()) + } if (args.method === 'terminal.create') { return Promise.resolve({ ok: true, result: { terminal: { handle: 'terminal-1' } } }) } @@ -5725,6 +5844,9 @@ describe('createRemoteRuntimePtyTransport', () => { vi.useFakeTimers() try { runtimeCall.mockImplementation((args) => { + if (args.method === 'status.get') { + return Promise.resolve(currentRuntimeStatus()) + } if (args.method === 'terminal.create') { return Promise.resolve({ ok: true, result: { terminal: { handle: 'terminal-1' } } }) } @@ -5770,6 +5892,9 @@ describe('createRemoteRuntimePtyTransport', () => { it('returns false when acknowledged terminal input is rejected by the runtime', async () => { runtimeCall.mockImplementation((args) => { + if (args.method === 'status.get') { + return Promise.resolve(currentRuntimeStatus()) + } if (args.method === 'terminal.create') { return Promise.resolve({ ok: true, result: { terminal: { handle: 'terminal-1' } } }) } @@ -5795,6 +5920,9 @@ describe('createRemoteRuntimePtyTransport', () => { it('splits large acknowledged remote input before terminal.send RPCs', async () => { runtimeCall.mockImplementation((args) => { + if (args.method === 'status.get') { + return Promise.resolve(currentRuntimeStatus()) + } if (args.method === 'terminal.create') { return Promise.resolve({ ok: true, result: { terminal: { handle: 'terminal-1' } } }) } @@ -5834,6 +5962,9 @@ describe('createRemoteRuntimePtyTransport', () => { vi.useFakeTimers() try { runtimeCall.mockImplementation((args) => { + if (args.method === 'status.get') { + return Promise.resolve(currentRuntimeStatus()) + } if (args.method === 'terminal.create') { return Promise.resolve({ ok: true, result: { terminal: { handle: 'terminal-1' } } }) } @@ -5883,6 +6014,9 @@ describe('createRemoteRuntimePtyTransport', () => { const firstChunk = 'x'.repeat(TERMINAL_INPUT_CHUNK_MAX_BYTES) const rejectedChunk = `tail${'y'.repeat(TERMINAL_INPUT_CHUNK_MAX_BYTES - 4)}` runtimeCall.mockImplementation((args) => { + if (args.method === 'status.get') { + return Promise.resolve(currentRuntimeStatus()) + } if (args.method === 'terminal.create') { return Promise.resolve({ ok: true, result: { terminal: { handle: 'terminal-1' } } }) } @@ -5921,6 +6055,9 @@ describe('createRemoteRuntimePtyTransport', () => { it('rejects oversized acknowledged remote input before runtime RPCs', async () => { runtimeCall.mockImplementation((args) => { + if (args.method === 'status.get') { + return Promise.resolve(currentRuntimeStatus()) + } if (args.method === 'terminal.create') { return Promise.resolve({ ok: true, result: { terminal: { handle: 'terminal-1' } } }) } diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts index 6eff59d84ce..35189447c57 100644 --- a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts @@ -20,8 +20,11 @@ import type { RuntimeTerminalSend } from '../../../../shared/runtime-types' import { + AGENT_SESSION_HOST_AUTHORITY_RUNTIME_CAPABILITY, AGENT_SESSION_OMP_RESUME_PATH_RUNTIME_CAPABILITY, - TERMINAL_CREATE_IDEMPOTENCY_RUNTIME_CAPABILITY + TERMINAL_ATTRIBUTION_REMOVED_RUNTIME_CAPABILITY, + TERMINAL_CREATE_IDEMPOTENCY_RUNTIME_CAPABILITY, + type RuntimeCapability } from '../../../../shared/protocol-version' import { isTerminalInputTooLargeWithDeferredMeasurement, @@ -34,7 +37,11 @@ import type { PtyTransportRecoveryState } from './pty-transport-types' import { createPtyOutputProcessor } from './pty-transport' -import { RuntimeRpcCallError, unwrapRuntimeRpcResult } from '../../runtime/runtime-rpc-client' +import { + RuntimeRpcCallError, + unwrapRuntimeRpcResult, + type LiveRuntimeEnvironmentAuthority +} from '../../runtime/runtime-rpc-client' import { getRemoteRuntimePtyEnvironmentId, getRemoteRuntimeTerminalHandle, @@ -83,6 +90,10 @@ import { ptyShutdownLifecycleHandlers } from './pty-shutdown-data-suspension' import { getRuntimeEnvironmentRevision } from '@/runtime/runtime-environment-revision' +import { + addLegacyTerminalAttributionDisableRequest, + withLegacyTerminalAttributionDisabledEnv +} from '../../../../shared/legacy-terminal-attribution-env' const REMOTE_TERMINAL_INPUT_FLUSH_MS = 8 const REMOTE_TERMINAL_VIEWPORT_FLUSH_MS = 33 @@ -92,6 +103,8 @@ const HOST_SESSION_ATTACH_TIMEOUT_MS = 15_000 const HOST_SESSION_INVENTORY_MAX_WINDOWS_PER_RECOVERY = 2 const HOST_SESSION_SAME_HANDLE_END_REUSE_LIMIT = 2 const TERMINAL_CREATE_RETRY_DELAYS_MS = [250, 500, 1000, 2000, 4000, 8000, 15_000, 30_000] as const +const AGENT_SESSION_REPLAY_UPDATE_REQUIRED_MESSAGE = + 'Remote agent recovery requires the same updated Orca server that accepted the launch. Update the workspace host and try again; no fallback launch was attempted.' type HostHandleReplacementPolicy = 'reuse' | 'prefer-replacement' | 'require-replacement' @@ -358,6 +371,7 @@ export function createRemoteRuntimePtyTransport( // Why: reconnect retries must replay one host operation instead of creating // another fresh agent when the first response was lost. const agentCreateOperation = createAgentSessionCreateOperation() + let agentSessionCreateAuthority: LiveRuntimeEnvironmentAuthority | null = null const outputProcessor = createPtyOutputProcessor({ onTitleChange, onBell, @@ -852,14 +866,16 @@ export function createRemoteRuntimePtyTransport( environmentId: string, method: string, params?: unknown, - timeoutMs = 15_000 + timeoutMs = 15_000, + expectedRuntimeId?: string ): Promise { const response = await window.api.runtimeEnvironments.call({ selector: environmentId, method, params, timeoutMs, - expectedEnvironmentPairingRevision: runtimeEnvironmentPairingRevision + expectedEnvironmentPairingRevision: runtimeEnvironmentPairingRevision, + ...(expectedRuntimeId ? { expectedRuntimeId } : {}) }) return unwrapRuntimeRpcResult(response as RuntimeRpcResponse) } @@ -908,7 +924,8 @@ export function createRemoteRuntimePtyTransport( reconcileExisting: boolean ) => Promise, environmentId: string, - expectedLifecycleEpoch: number + expectedLifecycleEpoch: number, + beforeReplay?: (timeoutMs: number) => Promise ): Promise { let retryAttempt = 0 // Structured operations already carry their replay proof; ordinary terminal.create @@ -993,6 +1010,9 @@ export function createRemoteRuntimePtyTransport( break } try { + if (reconcileExisting) { + await beforeReplay?.(Math.min(5_000, createRemainingMs ?? 5_000)) + } return await invoke(Math.min(15_000, createRemainingMs ?? 15_000), reconcileExisting) } catch (error) { lastError = error @@ -1975,8 +1995,10 @@ export function createRemoteRuntimePtyTransport( const commandToSend = options.command ?? command const startupCommandDeliveryToSend = options.startupCommandDelivery ?? startupCommandDelivery - const envToSend = options.env ?? env - const envToDeleteToSend = options.envToDelete ?? envToDelete + const envToSend = withLegacyTerminalAttributionDisabledEnv(options.env ?? env) + const envToDeleteToSend = addLegacyTerminalAttributionDisableRequest( + options.envToDelete ?? envToDelete + ) const launchConfigToSend = options.launchConfig ?? launchConfig const resumeProviderSessionToSend = options.resumeProviderSession ?? resumeProviderSession const launchTokenToSend = options.launchToken ?? launchToken @@ -2004,7 +2026,7 @@ export function createRemoteRuntimePtyTransport( presentation: 'background' as const, ...(activate === true ? { activate: true } : {}) } - const legacyCreate = () => + const legacyCreate = ({ authority }: { authority: LiveRuntimeEnvironmentAuthority }) => createWithUnknownOutcomeRecovery( 'terminal', (timeoutMs, reconcileExisting) => @@ -2015,13 +2037,49 @@ export function createRemoteRuntimePtyTransport( ...legacyCreateParams, ...(reconcileExisting ? { reconcileExisting: true } : {}) }, - timeoutMs + timeoutMs, + authority.runtimeId ), createEnvironmentId, connectLifecycleEpoch ) - const hostAuthorityCreate = () => - createWithUnknownOutcomeRecovery( + const requiredAgentSessionCapabilities: readonly RuntimeCapability[] = + resumeProviderSessionToSend && launchAgentToSend === 'omp' + ? [AGENT_SESSION_OMP_RESUME_PATH_RUNTIME_CAPABILITY] + : [] + const revalidateAgentSessionReplay = async ( + timeoutMs: number, + authority: LiveRuntimeEnvironmentAuthority + ): Promise => { + const status = await callRuntimeForEnvironment( + createEnvironmentId, + 'status.get', + undefined, + timeoutMs, + authority.runtimeId + ) + const requiredCapabilities = [ + AGENT_SESSION_HOST_AUTHORITY_RUNTIME_CAPABILITY, + TERMINAL_ATTRIBUTION_REMOVED_RUNTIME_CAPABILITY, + ...requiredAgentSessionCapabilities + ] + const capabilities = Array.isArray(status.capabilities) ? status.capabilities : [] + if ( + status.runtimeId !== authority.runtimeId || + requiredCapabilities.some((capability) => !capabilities.includes(capability)) + ) { + throw new Error(AGENT_SESSION_REPLAY_UPDATE_REQUIRED_MESSAGE) + } + } + const hostAuthorityCreate = (authority: LiveRuntimeEnvironmentAuthority) => { + if ( + agentSessionCreateAuthority && + agentSessionCreateAuthority.runtimeId !== authority.runtimeId + ) { + throw new Error(AGENT_SESSION_REPLAY_UPDATE_REQUIRED_MESSAGE) + } + agentSessionCreateAuthority ??= authority + return createWithUnknownOutcomeRecovery( 'agent-session', (timeoutMs) => resumeProviderSessionToSend @@ -2043,7 +2101,8 @@ export function createRemoteRuntimePtyTransport( placement: { tabId, leafId }, presentation: 'background' }, - timeoutMs + timeoutMs, + authority.runtimeId ) : callRuntimeForEnvironment( createEnvironmentId, @@ -2065,23 +2124,37 @@ export function createRemoteRuntimePtyTransport( }, agentCreateOperation.clientOperationId ), - timeoutMs + timeoutMs, + authority.runtimeId ), createEnvironmentId, - connectLifecycleEpoch + connectLifecycleEpoch, + (timeoutMs) => revalidateAgentSessionReplay(timeoutMs, authority) ) + } const created = launchAgentToSend ? agentSessionRequiresHostAuthorityReplay - ? await hostAuthorityCreate() + ? agentSessionCreateAuthority + ? await hostAuthorityCreate(agentSessionCreateAuthority) + : await runRemoteAgentSessionLaunch({ + environmentId: createEnvironmentId, + expectedEnvironmentPairingRevision: runtimeEnvironmentPairingRevision, + hostAuthority: hostAuthorityCreate, + requiredHostAuthorityCapabilities: requiredAgentSessionCapabilities, + legacy: legacyCreate + }) : await runRemoteAgentSessionLaunch({ environmentId: createEnvironmentId, + expectedEnvironmentPairingRevision: runtimeEnvironmentPairingRevision, hostAuthority: hostAuthorityCreate, - ...(resumeProviderSessionToSend && launchAgentToSend === 'omp' - ? { hostAuthorityCapability: AGENT_SESSION_OMP_RESUME_PATH_RUNTIME_CAPABILITY } - : {}), + requiredHostAuthorityCapabilities: requiredAgentSessionCapabilities, legacy: legacyCreate }) - : await legacyCreate() + : await runRemoteAgentSessionLaunch({ + environmentId: createEnvironmentId, + expectedEnvironmentPairingRevision: runtimeEnvironmentPairingRevision, + legacy: legacyCreate + }) if (!created) { if (!destroyed && lifecycleEpoch === connectLifecycleEpoch) { connecting = false @@ -2157,13 +2230,21 @@ export function createRemoteRuntimePtyTransport( if (!destroyed && lifecycleEpoch === connectLifecycleEpoch) { connecting = false const message = runtimeTerminalErrorMessage(error) + const recoverable = isRecoverableRemoteRuntimeConnectionError( + toRemoteRuntimeClientErrorLike(error) + ) if (isRemoteTerminalGoneMessage(message)) { recovery.cancel() handleRemoteTerminalError(error) } else if ( - isRecoverableRemoteRuntimeConnectionError(toRemoteRuntimeClientErrorLike(error)) + recoverable || + terminalCreateNeedsReconciliation || + agentSessionRequiresHostAuthorityReplay ) { recovery.markDisconnected() + if (!recoverable) { + surfaceErrorMessage(message) + } } else { recovery.cancel() emitRecoveryState() diff --git a/src/renderer/src/lib/launch-agent-background-session-remote.test.ts b/src/renderer/src/lib/launch-agent-background-session-remote.test.ts index 6cf8e9b717d..4066f67522f 100644 --- a/src/renderer/src/lib/launch-agent-background-session-remote.test.ts +++ b/src/renderer/src/lib/launch-agent-background-session-remote.test.ts @@ -378,6 +378,8 @@ describe('launchAgentBackgroundSession remote runtime and SSH startup delivery', ) expect(mockRuntimeEnvironmentCall).toHaveBeenCalledWith({ selector: 'env-1', + expectedEnvironmentPairingRevision: undefined, + expectedRuntimeId: 'remote-runtime', method: 'terminal.createAgentSession', params: expect.objectContaining({ clientOperationId: expect.stringMatching(/^\d{13}-[0-9a-f]{32}$/), @@ -420,7 +422,7 @@ describe('launchAgentBackgroundSession remote runtime and SSH startup delivery', graphStatus: 'ready', runtimeProtocolVersion: 3, minCompatibleRuntimeClientVersion: 2, - capabilities: [] + capabilities: ['terminal.attribution-removed.v1'] } }) } @@ -442,6 +444,7 @@ describe('launchAgentBackgroundSession remote runtime and SSH startup delivery', expect(mockRuntimeEnvironmentTransportCall).toHaveBeenCalledWith( expect.objectContaining({ + expectedRuntimeId: 'old-runtime', method: 'terminal.create', params: expect.objectContaining({ worktree: 'id:wt-1', diff --git a/src/renderer/src/lib/runtime-agent-background-create.ts b/src/renderer/src/lib/runtime-agent-background-create.ts index 70915ae63d4..a6c430f172b 100644 --- a/src/renderer/src/lib/runtime-agent-background-create.ts +++ b/src/renderer/src/lib/runtime-agent-background-create.ts @@ -3,6 +3,11 @@ import type { StartupCommandDelivery } from '../../../shared/codex-startup-deliv import type { SessionOptionValue } from '../../../shared/native-chat-session-options' import type { RuntimeTerminalCreate } from '../../../shared/runtime-types' import type { TuiAgent } from '../../../shared/types' +import { TERMINAL_ATTRIBUTION_REMOVED_RUNTIME_CAPABILITY } from '../../../shared/protocol-version' +import { + addLegacyTerminalAttributionDisableRequest, + withLegacyTerminalAttributionDisabledEnv +} from '../../../shared/legacy-terminal-attribution-env' import { createAgentSessionCreateOperation, toAgentLaunchPreferences, @@ -33,7 +38,8 @@ export async function createRuntimeAgentBackgroundTerminal(args: { const launchPreferences = toAgentLaunchPreferences(args.sessionOptions) return await runRemoteAgentSessionLaunch({ environmentId: args.environmentId, - hostAuthority: () => + requiredHostAuthorityCapabilities: [TERMINAL_ATTRIBUTION_REMOVED_RUNTIME_CAPABILITY], + hostAuthority: (authority) => operation.run((clientOperationId) => callRuntimeRpc<{ terminal: RuntimeTerminalCreate }>( { kind: 'environment', environmentId: args.environmentId }, @@ -52,10 +58,14 @@ export async function createRuntimeAgentBackgroundTerminal(args: { }, clientOperationId ), - { timeoutMs: 15_000 } + { + timeoutMs: 15_000, + expectedEnvironmentPairingRevision: authority.expectedEnvironmentPairingRevision, + expectedRuntimeId: authority.runtimeId + } ) ), - legacy: ({ skipCompatibilityCheck }) => + legacy: ({ skipCompatibilityCheck, authority }) => callRuntimeRpc<{ terminal: RuntimeTerminalCreate }>( { kind: 'environment', environmentId: args.environmentId }, 'terminal.create', @@ -65,7 +75,8 @@ export async function createRuntimeAgentBackgroundTerminal(args: { ...(args.legacy.startupCommandDelivery ? { startupCommandDelivery: args.legacy.startupCommandDelivery } : {}), - env: args.legacy.env, + env: withLegacyTerminalAttributionDisabledEnv(args.legacy.env), + envToDelete: addLegacyTerminalAttributionDisableRequest(undefined), launchConfig: args.legacy.launchConfig, launchToken: args.legacy.launchToken, launchAgent: args.agent, @@ -74,7 +85,12 @@ export async function createRuntimeAgentBackgroundTerminal(args: { leafId: args.leafId, presentation: 'background' }, - { timeoutMs: 15_000, skipCompatibilityCheck } + { + timeoutMs: 15_000, + skipCompatibilityCheck, + expectedEnvironmentPairingRevision: authority.expectedEnvironmentPairingRevision, + expectedRuntimeId: authority.runtimeId + } ) }) } diff --git a/src/renderer/src/lib/worktree-activation-created-agent.test.ts b/src/renderer/src/lib/worktree-activation-created-agent.test.ts index cc2af6e7d7c..e8a1ef3f052 100644 --- a/src/renderer/src/lib/worktree-activation-created-agent.test.ts +++ b/src/renderer/src/lib/worktree-activation-created-agent.test.ts @@ -1,5 +1,6 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import { getDefaultSettings } from '../../../shared/constants' +import { TERMINAL_ATTRIBUTION_REMOVED_RUNTIME_CAPABILITY } from '../../../shared/protocol-version' import { useAppStore } from '@/store' import { activateAndRevealWorktree, @@ -23,6 +24,35 @@ function makeWebRuntimeWorktree() { } } +function makeWakeTerminalRuntimeCall(worktreeId: string) { + return vi.fn(async (args: { method?: string }) => { + if (args.method === 'status.get') { + return { + ok: true, + result: { capabilities: [TERMINAL_ATTRIBUTION_REMOVED_RUNTIME_CAPABILITY] } + } + } + if (args.method === 'session.tabs.list') { + return { + ok: true, + result: { + worktree: worktreeId, + publicationEpoch: 'epoch-1', + snapshotVersion: 1, + activeGroupId: null, + activeTabId: null, + activeTabType: null, + tabs: [] + } + } + } + return { + ok: true, + result: { tab: { id: 'host-tab-1::leaf-1', leafId: 'leaf-1' } } + } + }) +} + /** Activates and asserts a focusable tab appeared with no queued startup, returning its id. */ function activateAndExpectNoRelaunch( worktreeId: string, @@ -536,16 +566,7 @@ describe('activateAndRevealWorktree', () => { it('respawns a host terminal when waking a slept web workspace with dead local PTYs', async () => { const worktree = makeWebRuntimeWorktree() - const callRuntimeEnvironment = vi - .fn() - .mockResolvedValueOnce({ - ok: true, - result: { repoId: worktree.repoId, worktreeId: worktree.id, activated: true } - }) - .mockResolvedValueOnce({ - ok: true, - result: { tabId: 'host-tab-1', terminal: 'term_host' } - }) + const callRuntimeEnvironment = makeWakeTerminalRuntimeCall(worktree.id) ;(globalThis as { __ORCA_WEB_CLIENT__?: boolean }).__ORCA_WEB_CLIENT__ = true vi.stubGlobal('window', { api: { @@ -612,9 +633,19 @@ describe('activateAndRevealWorktree', () => { }) ensureWebRuntimeWorktreeTerminalAfterWake(worktree.id) - await new Promise((resolve) => setTimeout(resolve, 0)) + await vi.waitFor(() => { + expect(callRuntimeEnvironment).toHaveBeenCalledTimes(3) + }) - expect(callRuntimeEnvironment).toHaveBeenCalledWith( + expect(callRuntimeEnvironment).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + selector: 'web-runtime-1', + method: 'status.get' + }) + ) + expect(callRuntimeEnvironment).toHaveBeenNthCalledWith( + 2, expect.objectContaining({ selector: 'web-runtime-1', method: 'session.tabs.createTerminal' @@ -624,10 +655,7 @@ describe('activateAndRevealWorktree', () => { it('respawns wake terminals on the explicit owner runtime when focus changed', async () => { const worktree = makeWorktree() - const callRuntimeEnvironment = vi.fn().mockResolvedValue({ - ok: true, - result: { tabId: 'host-tab-1', terminal: 'term_host' } - }) + const callRuntimeEnvironment = makeWakeTerminalRuntimeCall(worktree.id) ;(globalThis as { __ORCA_WEB_CLIENT__?: boolean }).__ORCA_WEB_CLIENT__ = true vi.stubGlobal('window', { api: { @@ -676,9 +704,19 @@ describe('activateAndRevealWorktree', () => { }) ensureWebRuntimeWorktreeTerminalAfterWake(worktree.id) - await new Promise((resolve) => setTimeout(resolve, 0)) + await vi.waitFor(() => { + expect(callRuntimeEnvironment).toHaveBeenCalledTimes(3) + }) - expect(callRuntimeEnvironment).toHaveBeenCalledWith( + expect(callRuntimeEnvironment).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + selector: 'owner-runtime', + method: 'status.get' + }) + ) + expect(callRuntimeEnvironment).toHaveBeenNthCalledWith( + 2, expect.objectContaining({ selector: 'owner-runtime', method: 'session.tabs.createTerminal' diff --git a/src/renderer/src/lib/worktree-activation-empty-remote.test.ts b/src/renderer/src/lib/worktree-activation-empty-remote.test.ts index 49f38e7aa7c..c4e2edc4907 100644 --- a/src/renderer/src/lib/worktree-activation-empty-remote.test.ts +++ b/src/renderer/src/lib/worktree-activation-empty-remote.test.ts @@ -1,6 +1,7 @@ import path from 'node:path' import { afterEach, describe, expect, it, vi } from 'vitest' import { getDefaultSettings } from '../../../shared/constants' +import { TERMINAL_ATTRIBUTION_REMOVED_RUNTIME_CAPABILITY } from '../../../shared/protocol-version' import type { Worktree } from '../../../shared/types' import { resetWebRuntimeWakeTerminalRespawnForTests } from '@/runtime/web-runtime-wake-terminal-respawn' import { resetWebSessionTabsSnapshotFreshnessForTests } from '@/runtime/web-session-tabs-sync' @@ -48,21 +49,43 @@ function makeWorktree(): Worktree { describe('empty remote worktree activation', () => { it('creates a host terminal when waking an empty remote workspace', async () => { const worktree = makeWorktree() - const callRuntimeEnvironment = vi.fn().mockResolvedValueOnce({ - ok: true, - result: { - tab: { - type: 'terminal', - id: 'host-tab-1::leaf-1', - parentTabId: 'host-tab-1', - leafId: 'leaf-1', - title: 'Terminal 1', - terminal: 'term_host', - status: 'ready', - isActive: true - }, - publicationEpoch: 'epoch-1', - snapshotVersion: 1 + const callRuntimeEnvironment = vi.fn(async (args: { method?: string }) => { + if (args.method === 'status.get') { + return { + ok: true, + result: { capabilities: [TERMINAL_ATTRIBUTION_REMOVED_RUNTIME_CAPABILITY] } + } + } + if (args.method === 'session.tabs.list') { + return { + ok: true, + result: { + worktree: worktree.id, + publicationEpoch: 'epoch-1', + snapshotVersion: 1, + activeGroupId: null, + activeTabId: null, + activeTabType: null, + tabs: [] + } + } + } + return { + ok: true, + result: { + tab: { + type: 'terminal', + id: 'host-tab-1::leaf-1', + parentTabId: 'host-tab-1', + leafId: 'leaf-1', + title: 'Terminal 1', + terminal: 'term_host', + status: 'ready', + isActive: true + }, + publicationEpoch: 'epoch-1', + snapshotVersion: 1 + } } }) ;(globalThis as { __ORCA_WEB_CLIENT__?: boolean }).__ORCA_WEB_CLIENT__ = true @@ -100,10 +123,18 @@ describe('empty remote worktree activation', () => { ensureWebRuntimeWorktreeTerminalAfterWake(worktree.id) await vi.waitFor(() => { - expect(callRuntimeEnvironment).toHaveBeenCalled() + expect(callRuntimeEnvironment).toHaveBeenCalledTimes(3) }) - expect(callRuntimeEnvironment).toHaveBeenCalledWith( + expect(callRuntimeEnvironment).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + selector: 'web-runtime-1', + method: 'status.get' + }) + ) + expect(callRuntimeEnvironment).toHaveBeenNthCalledWith( + 2, expect.objectContaining({ selector: 'web-runtime-1', method: 'session.tabs.createTerminal', diff --git a/src/renderer/src/runtime/abortable-runtime-environment-call.ts b/src/renderer/src/runtime/abortable-runtime-environment-call.ts index 619dfc517ce..357bf1b2d08 100644 --- a/src/renderer/src/runtime/abortable-runtime-environment-call.ts +++ b/src/renderer/src/runtime/abortable-runtime-environment-call.ts @@ -12,7 +12,8 @@ export async function callAbortableRuntimeEnvironment( params: unknown, timeoutMs: number | undefined, signal: AbortSignal, - expectedEnvironmentPairingRevision?: number + expectedEnvironmentPairingRevision?: number, + expectedRuntimeId?: string ): Promise> { if (signal.aborted) { throw createRuntimeRpcAbortError() @@ -47,7 +48,14 @@ export async function callAbortableRuntimeEnvironment( signal.addEventListener('abort', onAbort, { once: true }) void window.api.runtimeEnvironments .subscribe( - { selector: environmentId, method, params, timeoutMs, expectedEnvironmentPairingRevision }, + { + selector: environmentId, + method, + params, + timeoutMs, + expectedEnvironmentPairingRevision, + expectedRuntimeId + }, { onResponse: (response) => finish(() => resolve(response)), onError: (error) => finish(() => reject(new Error(error.message))), diff --git a/src/renderer/src/runtime/remote-agent-session-launch.test.ts b/src/renderer/src/runtime/remote-agent-session-launch.test.ts index 0fb6399d688..aea62019626 100644 --- a/src/renderer/src/runtime/remote-agent-session-launch.test.ts +++ b/src/renderer/src/runtime/remote-agent-session-launch.test.ts @@ -1,8 +1,6 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' -const mocks = vi.hoisted(() => ({ - supportsCapability: vi.fn() -})) +const mocks = vi.hoisted(() => ({ probe: vi.fn() })) vi.mock('./runtime-rpc-client', () => ({ RuntimeRpcCallError: class RuntimeRpcCallError extends Error { @@ -12,75 +10,110 @@ vi.mock('./runtime-rpc-client', () => ({ this.code = response.error.code } }, - runtimeEnvironmentSupportsCapability: mocks.supportsCapability + probeLiveRuntimeEnvironmentCapabilities: mocks.probe })) import { RuntimeRpcCallError } from './runtime-rpc-client' import { runRemoteAgentSessionLaunch } from './remote-agent-session-launch' +const authority = { + runtimeId: 'runtime-1', + expectedEnvironmentPairingRevision: 7, + capabilities: [ + 'terminal.attribution-removed.v1', + 'agent-session.host-authority.v1', + 'agent-session.omp-resume-path.v1' + ] +} as const + describe('remote agent-session launch routing', () => { beforeEach(() => { - mocks.supportsCapability.mockReset() + mocks.probe.mockReset() + mocks.probe.mockResolvedValue({ supported: true, authority }) }) - it('uses host authority only when the host advertises it', async () => { + it('uses one live all-capability probe before host authority', async () => { + const hostAuthority = vi.fn().mockResolvedValue('structured') + const legacy = vi.fn().mockResolvedValue('legacy') + + await expect( + runRemoteAgentSessionLaunch({ + environmentId: 'env-1', + expectedEnvironmentPairingRevision: 7, + hostAuthority, + requiredHostAuthorityCapabilities: ['agent-session.omp-resume-path.v1'], + legacy + }) + ).resolves.toBe('structured') + + expect(mocks.probe).toHaveBeenCalledWith({ + environmentId: 'env-1', + requiredCapabilities: [ + 'terminal.attribution-removed.v1', + 'agent-session.host-authority.v1', + 'agent-session.omp-resume-path.v1' + ], + timeoutMs: 15_000, + expectedEnvironmentPairingRevision: 7 + }) + expect(hostAuthority).toHaveBeenCalledWith(authority) + expect(legacy).not.toHaveBeenCalled() + }) + + it('uses fenced legacy when a structured-only capability is absent', async () => { + mocks.probe.mockResolvedValue({ supported: false, authority }) const hostAuthority = vi.fn().mockResolvedValue('structured') const legacy = vi.fn().mockResolvedValue('legacy') - mocks.supportsCapability.mockResolvedValue(true) await expect( runRemoteAgentSessionLaunch({ environmentId: 'env-1', hostAuthority, - hostAuthorityCapability: 'agent-session.omp-resume-path.v1', + requiredHostAuthorityCapabilities: ['agent-session.omp-resume-path.v1'], legacy }) - ).resolves.toBe('structured') - expect(mocks.supportsCapability).toHaveBeenCalledWith( - 'env-1', - 'agent-session.omp-resume-path.v1' - ) - expect(hostAuthority).toHaveBeenCalledOnce() + ).resolves.toBe('legacy') + + expect(hostAuthority).not.toHaveBeenCalled() + expect(legacy).toHaveBeenCalledWith({ skipCompatibilityCheck: true, authority }) + }) + + it('fails closed without attribution-removal capability evidence', async () => { + mocks.probe.mockResolvedValue({ + supported: false, + authority: { ...authority, capabilities: ['agent-session.host-authority.v1'] } + }) + const legacy = vi.fn().mockResolvedValue('legacy') + + await expect( + runRemoteAgentSessionLaunch({ environmentId: 'env-1', hostAuthority: vi.fn(), legacy }) + ).rejects.toThrow('Update the host and try again') expect(legacy).not.toHaveBeenCalled() }) - it('preserves the exact legacy path when the capability is absent', async () => { - const hostAuthority = vi.fn().mockResolvedValue('structured') + it('fails closed when the live capability probe fails', async () => { + mocks.probe.mockRejectedValue( + Object.assign(new Error('status temporarily unavailable'), { code: 'runtime_timeout' }) + ) const legacy = vi.fn().mockResolvedValue('legacy') - mocks.supportsCapability.mockResolvedValue(false) - await expect( - runRemoteAgentSessionLaunch({ environmentId: 'env-1', hostAuthority, legacy }) - ).resolves.toBe('legacy') - expect(legacy).toHaveBeenCalledOnce() - expect(hostAuthority).not.toHaveBeenCalled() + const result = expect( + runRemoteAgentSessionLaunch({ environmentId: 'env-1', hostAuthority: vi.fn(), legacy }) + ).rejects + await result.toThrow('Update the host and try again') + await result.toMatchObject({ code: 'runtime_timeout' }) + expect(legacy).not.toHaveBeenCalled() }) - it('keeps legacy behavior when a read-only capability probe fails', async () => { - const hostAuthority = vi.fn().mockResolvedValue('structured') - const legacy = vi.fn().mockResolvedValue('legacy') - mocks.supportsCapability.mockRejectedValue(new Error('status temporarily unavailable')) - - await expect( - runRemoteAgentSessionLaunch({ environmentId: 'env-1', hostAuthority, legacy }) - ).resolves.toBe('legacy') - expect(legacy).toHaveBeenCalledOnce() - expect(hostAuthority).not.toHaveBeenCalled() - }) - - it('does not bypass an incompatible runtime protocol', async () => { + it('preserves an incompatible runtime protocol error', async () => { const compatibilityError = Object.assign(new Error('runtime incompatible'), { code: 'runtime_compat_block' }) + mocks.probe.mockRejectedValue(compatibilityError) const legacy = vi.fn().mockResolvedValue('legacy') - mocks.supportsCapability.mockRejectedValue(compatibilityError) await expect( - runRemoteAgentSessionLaunch({ - environmentId: 'env-1', - hostAuthority: vi.fn(), - legacy - }) + runRemoteAgentSessionLaunch({ environmentId: 'env-1', hostAuthority: vi.fn(), legacy }) ).rejects.toBe(compatibilityError) expect(legacy).not.toHaveBeenCalled() }) @@ -88,7 +121,6 @@ describe('remote agent-session launch routing', () => { it('never downgrades after structured dispatch has started', async () => { const structuredError = new Error('structured response was lost') const legacy = vi.fn().mockResolvedValue('legacy') - mocks.supportsCapability.mockResolvedValue(true) await expect( runRemoteAgentSessionLaunch({ @@ -100,14 +132,13 @@ describe('remote agent-session launch routing', () => { expect(legacy).not.toHaveBeenCalled() }) - it('uses legacy only for the host pre-side-effect lower-owner response', async () => { - const legacy = vi.fn().mockResolvedValue('legacy') - mocks.supportsCapability.mockResolvedValue(true) + it('uses fenced legacy for the pre-side-effect lower-owner response', async () => { const legacyRequired = new RuntimeRpcCallError({ id: 'request-1', ok: false, error: { code: 'agent_session_legacy_required', message: 'legacy required' } }) + const legacy = vi.fn().mockResolvedValue('legacy') await expect( runRemoteAgentSessionLaunch({ @@ -116,17 +147,16 @@ describe('remote agent-session launch routing', () => { legacy }) ).resolves.toBe('legacy') - expect(legacy).toHaveBeenCalledOnce() + expect(legacy).toHaveBeenCalledWith({ skipCompatibilityCheck: true, authority }) }) - it('uses legacy when a replaced old host does not recognize the structured method', async () => { - const legacy = vi.fn().mockResolvedValue('legacy') - mocks.supportsCapability.mockResolvedValue(true) + it('does not downgrade when a replacement host rejects the structured method', async () => { const methodNotFound = new RuntimeRpcCallError({ id: 'request-1', ok: false, error: { code: 'method_not_found', message: 'Unknown method' } }) + const legacy = vi.fn().mockResolvedValue('legacy') await expect( runRemoteAgentSessionLaunch({ @@ -134,16 +164,16 @@ describe('remote agent-session launch routing', () => { hostAuthority: vi.fn().mockRejectedValue(methodNotFound), legacy }) - ).resolves.toBe('legacy') - expect(legacy).toHaveBeenCalledOnce() + ).rejects.toBe(methodNotFound) + expect(legacy).not.toHaveBeenCalled() }) - it('uses legacy directly when no structured form exists', async () => { + it('probes and fences legacy when no structured form exists', async () => { const legacy = vi.fn().mockResolvedValue('legacy') await expect(runRemoteAgentSessionLaunch({ environmentId: 'env-1', legacy })).resolves.toBe( 'legacy' ) - expect(mocks.supportsCapability).not.toHaveBeenCalled() + expect(legacy).toHaveBeenCalledWith({ skipCompatibilityCheck: true, authority }) }) }) diff --git a/src/renderer/src/runtime/remote-agent-session-launch.ts b/src/renderer/src/runtime/remote-agent-session-launch.ts index b00c6cbbdab..82c61d89ab1 100644 --- a/src/renderer/src/runtime/remote-agent-session-launch.ts +++ b/src/renderer/src/runtime/remote-agent-session-launch.ts @@ -1,46 +1,61 @@ import { AGENT_SESSION_HOST_AUTHORITY_CAPABILITY } from '../../../shared/agent-session-host-authority' -import type { RuntimeCapability } from '../../../shared/protocol-version' -import { RuntimeRpcCallError, runtimeEnvironmentSupportsCapability } from './runtime-rpc-client' +import { + TERMINAL_ATTRIBUTION_REMOVED_RUNTIME_CAPABILITY, + type RuntimeCapability +} from '../../../shared/protocol-version' +import { TERMINAL_CREATE_ATTRIBUTION_UPDATE_REQUIRED_MESSAGE } from '../../../shared/legacy-terminal-attribution-env' +import { + probeLiveRuntimeEnvironmentCapabilities, + RuntimeRpcCallError, + type LiveRuntimeEnvironmentAuthority +} from './runtime-rpc-client' import { isRuntimeCompatBlockError } from './runtime-protocol-compat' export async function runRemoteAgentSessionLaunch(args: { environmentId: string - hostAuthority?: () => Promise - hostAuthorityCapability?: RuntimeCapability - legacy: (options: { skipCompatibilityCheck: boolean }) => Promise + expectedEnvironmentPairingRevision?: number + hostAuthority?: (authority: LiveRuntimeEnvironmentAuthority) => Promise + requiredHostAuthorityCapabilities?: readonly RuntimeCapability[] + legacy: (options: { + skipCompatibilityCheck: boolean + authority: LiveRuntimeEnvironmentAuthority + }) => Promise }): Promise { - if (!args.hostAuthority) { - return await args.legacy({ skipCompatibilityCheck: false }) - } - let supported: boolean + const requiredCapabilities = [ + TERMINAL_ATTRIBUTION_REMOVED_RUNTIME_CAPABILITY, + ...(args.hostAuthority ? [AGENT_SESSION_HOST_AUTHORITY_CAPABILITY] : []), + ...(args.requiredHostAuthorityCapabilities ?? []) + ] + let probe: Awaited> try { - supported = await runtimeEnvironmentSupportsCapability( - args.environmentId, - args.hostAuthorityCapability ?? AGENT_SESSION_HOST_AUTHORITY_CAPABILITY - ) + probe = await probeLiveRuntimeEnvironmentCapabilities({ + environmentId: args.environmentId, + requiredCapabilities, + timeoutMs: 15_000, + expectedEnvironmentPairingRevision: args.expectedEnvironmentPairingRevision + }) } catch (error) { if (isRuntimeCompatBlockError(error)) { throw error } - // Why: a failed read-only probe has not launched anything, so preserving - // the legacy path cannot duplicate an agent and keeps transient upgrades neutral. - return await args.legacy({ skipCompatibilityCheck: true }) + const code = error && typeof error === 'object' ? Reflect.get(error, 'code') : undefined + // Why: unknown-outcome recovery still needs to classify a transient failed probe. + throw Object.assign( + new Error(TERMINAL_CREATE_ATTRIBUTION_UPDATE_REQUIRED_MESSAGE, { cause: error }), + typeof code === 'string' ? { code } : {} + ) } - // Why: choose before invoking either path; an ambiguous structured outcome - // must never trigger a legacy retry that could spawn a duplicate. - if (!supported) { - return await args.legacy({ skipCompatibilityCheck: true }) + if (!probe.authority.capabilities.includes(TERMINAL_ATTRIBUTION_REMOVED_RUNTIME_CAPABILITY)) { + throw new Error(TERMINAL_CREATE_ATTRIBUTION_UPDATE_REQUIRED_MESSAGE) + } + if (!args.hostAuthority || !probe.supported) { + return await args.legacy({ skipCompatibilityCheck: true, authority: probe.authority }) } try { - return await args.hostAuthority() + return await args.hostAuthority(probe.authority) } catch (error) { - if ( - error instanceof RuntimeRpcCallError && - (error.code === 'agent_session_legacy_required' || error.code === 'method_not_found') - ) { - // Why: both responses prove no structured side effect began: the new host rejected an old - // lower owner before dispatch, or an old host never recognized the method. - return await args.legacy({ skipCompatibilityCheck: true }) + if (error instanceof RuntimeRpcCallError && error.code === 'agent_session_legacy_required') { + return await args.legacy({ skipCompatibilityCheck: true, authority: probe.authority }) } throw error } diff --git a/src/renderer/src/runtime/runtime-environment-authority.ts b/src/renderer/src/runtime/runtime-environment-authority.ts new file mode 100644 index 00000000000..f773941bdf3 --- /dev/null +++ b/src/renderer/src/runtime/runtime-environment-authority.ts @@ -0,0 +1,46 @@ +import type { RuntimeRpcResponse } from '../../../shared/runtime-rpc-envelope' +import type { RuntimeStatus } from '../../../shared/runtime-types' +import type { RuntimeCapability } from '../../../shared/protocol-version' +import { callRuntimeEnvironmentWithRevision } from './runtime-rpc-environment-call' +import { captureRuntimeEnvironmentRequestRevision } from './runtime-environment-revision' +import { assertRuntimeStatusCompatible } from './runtime-protocol-compat' +import { unwrapRuntimeRpcResult } from './runtime-rpc-result' + +export type LiveRuntimeEnvironmentAuthority = Readonly<{ + runtimeId: string + expectedEnvironmentPairingRevision: number | undefined + capabilities: readonly RuntimeCapability[] +}> + +export async function probeLiveRuntimeEnvironmentCapabilities(args: { + environmentId: string + requiredCapabilities: readonly RuntimeCapability[] + timeoutMs?: number + expectedEnvironmentPairingRevision?: number +}): Promise<{ supported: boolean; authority: LiveRuntimeEnvironmentAuthority }> { + const environmentId = args.environmentId.trim() + const expectedEnvironmentPairingRevision = captureRuntimeEnvironmentRequestRevision( + environmentId, + args.expectedEnvironmentPairingRevision + ) + const response = await callRuntimeEnvironmentWithRevision({ + environmentId, + method: 'status.get', + params: undefined, + timeoutMs: args.timeoutMs, + expectedEnvironmentPairingRevision + }) + const status = unwrapRuntimeRpcResult( + response as RuntimeRpcResponse + ) + assertRuntimeStatusCompatible(status) + const capabilities = status.capabilities ?? [] + return { + supported: args.requiredCapabilities.every((capability) => capabilities.includes(capability)), + authority: Object.freeze({ + runtimeId: status.runtimeId, + expectedEnvironmentPairingRevision, + capabilities: Object.freeze([...capabilities]) + }) + } +} diff --git a/src/renderer/src/runtime/runtime-rpc-client-pairing-revision.test.ts b/src/renderer/src/runtime/runtime-rpc-client-pairing-revision.test.ts index b3e4c103f4f..00995d0db6d 100644 --- a/src/renderer/src/runtime/runtime-rpc-client-pairing-revision.test.ts +++ b/src/renderer/src/runtime/runtime-rpc-client-pairing-revision.test.ts @@ -3,7 +3,11 @@ import { MIN_COMPATIBLE_RUNTIME_CLIENT_VERSION, RUNTIME_PROTOCOL_VERSION } from '../../../shared/protocol-version' -import { callRuntimeRpc, clearRuntimeCompatibilityCacheForTests } from './runtime-rpc-client' +import { + callRuntimeRpc, + clearRuntimeCompatibilityCacheForTests, + probeLiveRuntimeEnvironmentCapabilities +} from './runtime-rpc-client' import { replaceRuntimeEnvironmentRevisions } from './runtime-environment-revision' const runtimeEnvironmentCall = vi.fn() @@ -62,3 +66,57 @@ it('captures the pairing revision before awaiting the compatibility probe', asyn expectedEnvironmentPairingRevision: 10 }) }) + +it('live capability probes ignore a warm cache and pin the captured pairing revision', async () => { + replaceRuntimeEnvironmentRevisions([{ id: 'env-live', createdAt: 1, pairingRevision: 20 }]) + runtimeEnvironmentCall + .mockResolvedValueOnce({ + id: 'warm-status', + ok: true, + result: { + runtimeId: 'runtime-a', + graphStatus: 'ready', + runtimeProtocolVersion: RUNTIME_PROTOCOL_VERSION, + minCompatibleRuntimeClientVersion: MIN_COMPATIBLE_RUNTIME_CLIENT_VERSION, + capabilities: ['terminal.attribution-removed.v1'] + } + }) + .mockResolvedValueOnce({ id: 'repo', ok: true, result: { repos: [] } }) + .mockImplementationOnce(async () => { + replaceRuntimeEnvironmentRevisions([{ id: 'env-live', createdAt: 1, pairingRevision: 21 }]) + return { + id: 'live-status', + ok: true, + result: { + runtimeId: 'runtime-b', + graphStatus: 'ready', + runtimeProtocolVersion: RUNTIME_PROTOCOL_VERSION, + minCompatibleRuntimeClientVersion: MIN_COMPATIBLE_RUNTIME_CLIENT_VERSION, + capabilities: [] + } + } + }) + + await callRuntimeRpc({ kind: 'environment', environmentId: 'env-live' }, 'repo.list') + await expect( + probeLiveRuntimeEnvironmentCapabilities({ + environmentId: 'env-live', + requiredCapabilities: ['terminal.attribution-removed.v1'] + }) + ).resolves.toEqual({ + supported: false, + authority: { + runtimeId: 'runtime-b', + expectedEnvironmentPairingRevision: 20, + capabilities: [] + } + }) + + expect(runtimeEnvironmentCall).toHaveBeenLastCalledWith({ + selector: 'env-live', + method: 'status.get', + params: undefined, + timeoutMs: undefined, + expectedEnvironmentPairingRevision: 20 + }) +}) diff --git a/src/renderer/src/runtime/runtime-rpc-client.ts b/src/renderer/src/runtime/runtime-rpc-client.ts index 57ba17b75a0..5f129da43a0 100644 --- a/src/renderer/src/runtime/runtime-rpc-client.ts +++ b/src/renderer/src/runtime/runtime-rpc-client.ts @@ -19,6 +19,8 @@ export { RuntimeRpcCallError, unwrapRuntimeRpcResult } from './runtime-rpc-result' +export { probeLiveRuntimeEnvironmentCapabilities } from './runtime-environment-authority' +export type { LiveRuntimeEnvironmentAuthority } from './runtime-environment-authority' const RUNTIME_COMPATIBILITY_CACHE_MAX = 32 const RECENT_RUNTIME_COMPATIBILITY_FAILURE_TTL_MS = 60_000 @@ -54,6 +56,7 @@ export async function callRuntimeRpc( skipCompatibilityCheck?: boolean signal?: AbortSignal expectedEnvironmentPairingRevision?: number + expectedRuntimeId?: string } = {} ): Promise { const expectedEnvironmentPairingRevision = @@ -88,7 +91,8 @@ export async function callRuntimeRpc( params: nextParams, timeoutMs: options.timeoutMs, signal: options.signal, - expectedEnvironmentPairingRevision + expectedEnvironmentPairingRevision, + expectedRuntimeId: options.expectedRuntimeId }) return unwrapRuntimeRpcResult(response as RuntimeRpcResponse) } diff --git a/src/renderer/src/runtime/runtime-rpc-environment-call.test.ts b/src/renderer/src/runtime/runtime-rpc-environment-call.test.ts new file mode 100644 index 00000000000..3f8b8c53c5c --- /dev/null +++ b/src/renderer/src/runtime/runtime-rpc-environment-call.test.ts @@ -0,0 +1,28 @@ +import { expect, it, vi } from 'vitest' +import { callRuntimeEnvironmentWithRevision } from './runtime-rpc-environment-call' + +it('preserves abort semantics for runtime-fenced requests', async () => { + const controller = new AbortController() + const unsubscribe = vi.fn() + const call = vi.fn() + const subscribe = vi.fn().mockResolvedValue({ unsubscribe, sendBinary: vi.fn() }) + vi.stubGlobal('window', { api: { runtimeEnvironments: { call, subscribe } } }) + + const request = callRuntimeEnvironmentWithRevision({ + environmentId: 'env-1', + method: 'status.get', + params: undefined, + signal: controller.signal, + expectedRuntimeId: 'runtime-1' + }) + await vi.waitFor(() => expect(subscribe).toHaveBeenCalled()) + expect(subscribe).toHaveBeenCalledWith( + expect.objectContaining({ expectedRuntimeId: 'runtime-1' }), + expect.any(Object) + ) + controller.abort() + + await expect(request).rejects.toMatchObject({ name: 'AbortError' }) + await vi.waitFor(() => expect(unsubscribe).toHaveBeenCalledOnce()) + expect(call).not.toHaveBeenCalled() +}) diff --git a/src/renderer/src/runtime/runtime-rpc-environment-call.ts b/src/renderer/src/runtime/runtime-rpc-environment-call.ts index 5acc0fa0a85..dc5f85f8a62 100644 --- a/src/renderer/src/runtime/runtime-rpc-environment-call.ts +++ b/src/renderer/src/runtime/runtime-rpc-environment-call.ts @@ -7,6 +7,7 @@ export async function callRuntimeEnvironmentWithRevision(args: { timeoutMs?: number signal?: AbortSignal expectedEnvironmentPairingRevision?: number + expectedRuntimeId?: string }): Promise { if (args.signal) { return callAbortableRuntimeEnvironment( @@ -15,7 +16,8 @@ export async function callRuntimeEnvironmentWithRevision(args: { args.params, args.timeoutMs, args.signal, - args.expectedEnvironmentPairingRevision + args.expectedEnvironmentPairingRevision, + args.expectedRuntimeId ) } return window.api.runtimeEnvironments.call({ @@ -23,6 +25,7 @@ export async function callRuntimeEnvironmentWithRevision(args: { method: args.method, params: args.params, timeoutMs: args.timeoutMs, - expectedEnvironmentPairingRevision: args.expectedEnvironmentPairingRevision + expectedEnvironmentPairingRevision: args.expectedEnvironmentPairingRevision, + expectedRuntimeId: args.expectedRuntimeId }) } diff --git a/src/renderer/src/runtime/web-runtime-session.test.ts b/src/renderer/src/runtime/web-runtime-session.test.ts index ba309a39f91..3e64191dcab 100644 --- a/src/renderer/src/runtime/web-runtime-session.test.ts +++ b/src/renderer/src/runtime/web-runtime-session.test.ts @@ -56,9 +56,12 @@ const mocks = vi.hoisted(() => ({ deliverLaunchPromptToAgentTab: vi.fn(), seedNativeChatLaunchDraftForAgentTab: vi.fn(), getRuntimeEnvironmentIdForWorktree: vi.fn(), - hasMaterializedWebRuntimeBrowserPage: vi.fn() + hasMaterializedWebRuntimeBrowserPage: vi.fn(), + toastError: vi.fn() })) +vi.mock('sonner', () => ({ toast: { error: mocks.toastError } })) + vi.mock('../store', () => ({ useAppStore: { getState: mocks.getState, @@ -112,6 +115,21 @@ function makeSnapshot(): RuntimeMobileSessionTabsResult { } } +function makeAttributionSafeStatus(capabilities: string[] = ['terminal.attribution-removed.v1']) { + return { + id: 'status', + ok: true, + result: { + runtimeId: 'runtime-1', + graphStatus: 'ready', + runtimeProtocolVersion: 3, + minCompatibleRuntimeClientVersion: 2, + appVersion: '1.4.181', + capabilities + } + } +} + describe('refreshWebRuntimeSessionTabsSnapshot', () => { afterEach(() => { resetWebAgentSessionHandoffsForTests() @@ -1430,6 +1448,7 @@ describe('createWebRuntimeSessionTerminal', () => { }) const runtimeCall = vi .fn() + .mockResolvedValueOnce(makeAttributionSafeStatus()) .mockResolvedValueOnce({ id: 'create', ok: true, @@ -1450,6 +1469,43 @@ describe('createWebRuntimeSessionTerminal', () => { expect(selectedHosts).toEqual([RUNTIME_EXECUTION_HOST_ID]) }) + it('refuses session-tab creation when an older host drops terminal environment fields', async () => { + const runtimeCall = vi.fn().mockResolvedValue({ + id: 'status', + ok: true, + result: { capabilities: ['mobile.tasks.v1'] } + }) + vi.stubGlobal('window', { + api: { runtimeEnvironments: { call: runtimeCall } } + }) + + await expect(createWebRuntimeSessionTerminal({ worktreeId: WORKTREE_ID })).resolves.toEqual({ + status: 'failed', + message: + 'Creating terminals requires a newer workspace host that can verify safe terminal environment forwarding. Update the host and try again.' + }) + + expect(runtimeCall.mock.calls.map(([request]) => request.method)).toEqual(['status.get']) + }) + + it('returns a failed outcome when attribution cleanup exceeds the deletion limit', async () => { + const runtimeCall = vi.fn() + vi.stubGlobal('window', { + api: { runtimeEnvironments: { call: runtimeCall } } + }) + + await expect( + createWebRuntimeSessionTerminal({ + worktreeId: WORKTREE_ID, + envToDelete: Array.from({ length: 32 }, (_, index) => `KEY_${index}`) + }) + ).resolves.toEqual({ + status: 'failed', + message: 'Terminal environment deletion limit leaves no room for attribution cleanup' + }) + expect(runtimeCall).not.toHaveBeenCalled() + }) + it.each([ { sessionKind: 'fresh' as const, activate: true }, { sessionKind: 'fresh' as const, activate: false }, @@ -1469,7 +1525,7 @@ describe('createWebRuntimeSessionTerminal', () => { graphStatus: 'ready', runtimeProtocolVersion: 3, minCompatibleRuntimeClientVersion: 2, - capabilities: ['agent-session.host-authority.v1'] + capabilities: ['agent-session.host-authority.v1', 'terminal.attribution-removed.v1'] } } } @@ -1558,7 +1614,7 @@ describe('createWebRuntimeSessionTerminal', () => { graphStatus: 'ready', runtimeProtocolVersion: 3, minCompatibleRuntimeClientVersion: 2, - capabilities: ['agent-session.host-authority.v1'] + capabilities: ['agent-session.host-authority.v1', 'terminal.attribution-removed.v1'] } }) .mockResolvedValueOnce({ @@ -1622,6 +1678,7 @@ describe('createWebRuntimeSessionTerminal', () => { expect(runtimeCall).toHaveBeenNthCalledWith(2, { selector: ENVIRONMENT_ID, expectedEnvironmentPairingRevision: undefined, + expectedRuntimeId: 'runtime-1', method: 'terminal.createAgentSession', params: { clientOperationId: expect.stringMatching(/^\d{13}-[0-9a-f]{32}$/), @@ -1666,6 +1723,7 @@ describe('createWebRuntimeSessionTerminal', () => { it('keeps exact legacy ordering when structured creation cannot express afterTabId', async () => { const runtimeCall = vi .fn() + .mockResolvedValueOnce(makeAttributionSafeStatus()) .mockResolvedValueOnce({ id: 'legacy-create', ok: true, @@ -1691,8 +1749,10 @@ describe('createWebRuntimeSessionTerminal', () => { }) ).resolves.toEqual({ status: 'created' }) - expect(runtimeCall).toHaveBeenNthCalledWith(1, { + expect(runtimeCall).toHaveBeenNthCalledWith(2, { selector: ENVIRONMENT_ID, + expectedEnvironmentPairingRevision: undefined, + expectedRuntimeId: 'runtime-1', method: 'session.tabs.createTerminal', params: { worktree: `id:${WORKTREE_ID}`, @@ -1700,6 +1760,8 @@ describe('createWebRuntimeSessionTerminal', () => { targetGroupId: 'group-left', command: undefined, cwd: undefined, + env: { ORCA_ATTRIBUTION_BYPASS: '1' }, + envToDelete: ['ORCA_ENABLE_GIT_ATTRIBUTION'], startupCommandDelivery: undefined, agent: 'codex', activate: false, @@ -1709,6 +1771,7 @@ describe('createWebRuntimeSessionTerminal', () => { timeoutMs: 15_000 }) expect(runtimeCall.mock.calls.map(([request]) => request.method)).toEqual([ + 'status.get', 'session.tabs.createTerminal', 'session.tabs.list' ]) @@ -1725,6 +1788,7 @@ describe('createWebRuntimeSessionTerminal', () => { }) const runtimeCall = vi .fn() + .mockResolvedValueOnce(makeAttributionSafeStatus()) .mockResolvedValueOnce({ id: 'create-terminal', ok: true, @@ -1781,7 +1845,7 @@ describe('createWebRuntimeSessionTerminal', () => { graphStatus: 'ready', runtimeProtocolVersion: 3, minCompatibleRuntimeClientVersion: 2, - capabilities: ['agent-session.host-authority.v1'] + capabilities: ['agent-session.host-authority.v1', 'terminal.attribution-removed.v1'] } } } @@ -1842,7 +1906,7 @@ describe('createWebRuntimeSessionTerminal', () => { graphStatus: 'ready', runtimeProtocolVersion: 3, minCompatibleRuntimeClientVersion: 2, - capabilities: ['agent-session.host-authority.v1'] + capabilities: ['agent-session.host-authority.v1', 'terminal.attribution-removed.v1'] } } } @@ -1895,7 +1959,7 @@ describe('createWebRuntimeSessionTerminal', () => { graphStatus: 'ready', runtimeProtocolVersion: 3, minCompatibleRuntimeClientVersion: 2, - capabilities: [] + capabilities: ['terminal.attribution-removed.v1'] } } } @@ -1926,6 +1990,8 @@ describe('createWebRuntimeSessionTerminal', () => { expect(runtimeCall).toHaveBeenNthCalledWith(2, { selector: ENVIRONMENT_ID, + expectedEnvironmentPairingRevision: undefined, + expectedRuntimeId: 'old-runtime', method: 'session.tabs.createTerminal', params: { worktree: `id:${WORKTREE_ID}`, @@ -1933,6 +1999,8 @@ describe('createWebRuntimeSessionTerminal', () => { targetGroupId: 'group-left', command: undefined, cwd: undefined, + env: { ORCA_ATTRIBUTION_BYPASS: '1' }, + envToDelete: ['ORCA_ENABLE_GIT_ATTRIBUTION'], startupCommandDelivery: undefined, launchAgent: 'codex', activate: false, @@ -1959,7 +2027,7 @@ describe('createWebRuntimeSessionTerminal', () => { graphStatus: 'ready', runtimeProtocolVersion: 3, minCompatibleRuntimeClientVersion: 2, - capabilities: [] + capabilities: ['terminal.attribution-removed.v1'] } } } @@ -1994,6 +2062,8 @@ describe('createWebRuntimeSessionTerminal', () => { expect(runtimeCall).toHaveBeenNthCalledWith(2, { selector: ENVIRONMENT_ID, + expectedEnvironmentPairingRevision: undefined, + expectedRuntimeId: 'old-runtime', method: 'session.tabs.createTerminal', params: { worktree: `id:${WORKTREE_ID}`, @@ -2001,7 +2071,8 @@ describe('createWebRuntimeSessionTerminal', () => { targetGroupId: undefined, command: "codex resume 'session-1'", cwd: undefined, - env: { CODEX_PROFILE: 'captured' }, + env: { CODEX_PROFILE: 'captured', ORCA_ATTRIBUTION_BYPASS: '1' }, + envToDelete: ['ORCA_ENABLE_GIT_ATTRIBUTION'], startupCommandDelivery: undefined, launchConfig: { agentCommand: 'codex', @@ -2030,7 +2101,7 @@ describe('createWebRuntimeSessionTerminal', () => { graphStatus: 'ready', runtimeProtocolVersion: 3, minCompatibleRuntimeClientVersion: 2, - capabilities: ['agent-session.host-authority.v1'] + capabilities: ['agent-session.host-authority.v1', 'terminal.attribution-removed.v1'] } } } @@ -2071,9 +2142,14 @@ describe('createWebRuntimeSessionTerminal', () => { expect(methods).toEqual(['status.get', 'session.tabs.createTerminal', 'session.tabs.list']) expect(runtimeCall.mock.calls[1]?.[0]).toMatchObject({ + expectedRuntimeId: 'new-runtime', params: { command: "omp --resume '/custom/omp/project/session.jsonl'", - env: { PI_CODING_AGENT_DIR: '/custom/omp' }, + env: { + PI_CODING_AGENT_DIR: '/custom/omp', + ORCA_ATTRIBUTION_BYPASS: '1' + }, + envToDelete: ['ORCA_ENABLE_GIT_ATTRIBUTION'], launchAgent: 'omp' } }) @@ -2090,7 +2166,7 @@ describe('createWebRuntimeSessionTerminal', () => { graphStatus: 'ready', runtimeProtocolVersion: 3, minCompatibleRuntimeClientVersion: 2, - capabilities: ['agent-session.host-authority.v1'] + capabilities: ['agent-session.host-authority.v1', 'terminal.attribution-removed.v1'] } } } @@ -2152,7 +2228,7 @@ describe('createWebRuntimeSessionTerminal', () => { graphStatus: 'ready', runtimeProtocolVersion: 3, minCompatibleRuntimeClientVersion: 2, - capabilities: ['agent-session.host-authority.v1'] + capabilities: ['agent-session.host-authority.v1', 'terminal.attribution-removed.v1'] } } } @@ -2782,17 +2858,25 @@ describe('splitWebRuntimeTerminal', () => { }) it('passes telemetry source to the host split while allowing the mirrored split event to be suppressed', async () => { - const runtimeCall = vi.fn().mockResolvedValue({ - id: 'split', - ok: true, - result: { - split: { - handle: 'terminal-2', - tabId: 'tab-1', - paneRuntimeId: -1 - } - } - }) + const runtimeCall = vi.fn(async (request: { method: string }) => + request.method === 'status.get' + ? { + id: 'status', + ok: true, + result: { capabilities: ['terminal.attribution-removed.v1'] } + } + : { + id: 'split', + ok: true, + result: { + split: { + handle: 'terminal-2', + tabId: 'tab-1', + paneRuntimeId: -1 + } + } + } + ) vi.stubGlobal('window', { api: { runtimeEnvironments: { @@ -2811,13 +2895,22 @@ describe('splitWebRuntimeTerminal', () => { consumePendingWebRuntimeSplitMirrorTelemetry('remote:web-env-1@@terminal-1', 'horizontal') ).toBe(true) - await vi.waitFor(() => expect(runtimeCall).toHaveBeenCalledTimes(1)) - expect(runtimeCall).toHaveBeenCalledWith({ + await vi.waitFor(() => expect(runtimeCall).toHaveBeenCalledTimes(2)) + expect(runtimeCall).toHaveBeenNthCalledWith(1, { selector: 'web-env-1', + expectedEnvironmentPairingRevision: undefined, + method: 'status.get', + timeoutMs: 15_000 + }) + expect(runtimeCall).toHaveBeenNthCalledWith(2, { + selector: 'web-env-1', + expectedEnvironmentPairingRevision: undefined, method: 'terminal.split', params: { terminal: 'terminal-1', direction: 'horizontal', + env: { ORCA_ATTRIBUTION_BYPASS: '1' }, + envToDelete: ['ORCA_ENABLE_GIT_ATTRIBUTION'], telemetrySource: 'keyboard' }, timeoutMs: 15_000 @@ -2826,11 +2919,19 @@ describe('splitWebRuntimeTerminal', () => { it('does not track rejected host split RPCs', async () => { const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) - const runtimeCall = vi.fn().mockResolvedValue({ - id: 'split', - ok: false, - error: { code: 'terminal_exited', message: 'Terminal exited' } - }) + const runtimeCall = vi.fn(async (request: { method: string }) => + request.method === 'status.get' + ? { + id: 'status', + ok: true, + result: { capabilities: ['terminal.attribution-removed.v1'] } + } + : { + id: 'split', + ok: false, + error: { code: 'terminal_exited', message: 'Terminal exited' } + } + ) vi.stubGlobal('window', { api: { runtimeEnvironments: { @@ -2843,23 +2944,53 @@ describe('splitWebRuntimeTerminal', () => { splitWebRuntimeTerminal('remote:web-env-1@@terminal-1', 'vertical', 'context_menu') ).toBe(true) - await vi.waitFor(() => expect(runtimeCall).toHaveBeenCalledTimes(1)) + await vi.waitFor(() => expect(runtimeCall).toHaveBeenCalledTimes(2)) await vi.waitFor(() => expect(warnSpy).toHaveBeenCalledTimes(1)) + expect(mocks.toastError).toHaveBeenCalledWith('Terminal exited') expect(mocks.trackTerminalPaneSplit).not.toHaveBeenCalled() }) - it('ignores local panes but delegates remote runtime panes from desktop or web clients', async () => { + it('refuses legacy hosts because renderer-owned splits discard environment fields', async () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) const runtimeCall = vi.fn().mockResolvedValue({ - id: 'split', + id: 'status', ok: true, - result: { - split: { - handle: 'terminal-2', - tabId: 'tab-1', - ptyId: 'pty-2' - } - } + result: { appVersion: '1.4.181', capabilities: ['mobile.tasks.v1'] } }) + vi.stubGlobal('window', { + api: { runtimeEnvironments: { call: runtimeCall } } + }) + + expect(splitWebRuntimeTerminal('remote:web-env-1@@terminal-1', 'vertical', 'keyboard')).toBe( + true + ) + + await vi.waitFor(() => expect(warnSpy).toHaveBeenCalledTimes(1)) + expect(runtimeCall).toHaveBeenCalledTimes(1) + expect(runtimeCall).not.toHaveBeenCalledWith( + expect.objectContaining({ method: 'terminal.split' }) + ) + expect(mocks.toastError).toHaveBeenCalledWith( + expect.stringContaining('Update the host and try again') + ) + }) + + it('ignores local panes but delegates remote runtime panes from desktop or web clients', async () => { + const runtimeCall = vi.fn(async (request: { method: string }) => + request.method === 'status.get' + ? { id: 'status', ok: true, result: { capabilities: ['terminal.attribution-removed.v1'] } } + : { + id: 'split', + ok: true, + result: { + split: { + handle: 'terminal-2', + tabId: 'tab-1', + ptyId: 'pty-2' + } + } + } + ) vi.stubGlobal('window', { api: { runtimeEnvironments: { @@ -2874,7 +3005,7 @@ describe('splitWebRuntimeTerminal', () => { true ) - await vi.waitFor(() => expect(runtimeCall).toHaveBeenCalledTimes(1)) + await vi.waitFor(() => expect(runtimeCall).toHaveBeenCalledTimes(2)) }) }) diff --git a/src/renderer/src/runtime/web-runtime-session.ts b/src/renderer/src/runtime/web-runtime-session.ts index 9cb617d5692..089ceafd53c 100644 --- a/src/renderer/src/runtime/web-runtime-session.ts +++ b/src/renderer/src/runtime/web-runtime-session.ts @@ -8,6 +8,7 @@ import type { RuntimeMobileSessionTabMoveResult, RuntimeMobileSessionTabsResult, RuntimeSessionTabCloseReason, + RuntimeStatus, RuntimeTerminalCreate, RuntimeTerminalClose, RuntimeTerminalSplit @@ -18,7 +19,10 @@ import type { SleepingAgentLaunchConfig, AgentProviderSessionMetadata } from '../../../shared/agent-session-resume' -import { AGENT_SESSION_OMP_RESUME_PATH_RUNTIME_CAPABILITY } from '../../../shared/protocol-version' +import { + AGENT_SESSION_OMP_RESUME_PATH_RUNTIME_CAPABILITY, + TERMINAL_ATTRIBUTION_REMOVED_RUNTIME_CAPABILITY +} from '../../../shared/protocol-version' import type { AgentLaunchPreferences, AgentPromptDelivery, @@ -81,6 +85,15 @@ import { throwIfE2eWebRuntimeBrowserCapabilityUnavailable, throwIfE2eWebRuntimeBrowserReconciliationFails } from './web-runtime-browser-creation-e2e-fault' +import { + addLegacyTerminalAttributionDisableRequest, + hostSupportsSessionTabTerminalCreateAttributionDisable, + hostSupportsTerminalSplitAttributionDisable, + SESSION_TAB_TERMINAL_CREATE_ATTRIBUTION_UPDATE_REQUIRED_MESSAGE, + TERMINAL_SPLIT_ATTRIBUTION_UPDATE_REQUIRED_MESSAGE, + withLegacyTerminalAttributionDisabledEnv +} from '../../../shared/legacy-terminal-attribution-env' +import { toast } from 'sonner' export { HOST_TERMINAL_SURFACE_SEPARATOR, @@ -130,6 +143,7 @@ function captureRuntimeEnvironmentCall( method: string params?: unknown timeoutMs?: number + expectedRuntimeId?: string }) => Promise> { return (args) => window.api.runtimeEnvironments.call({ @@ -264,6 +278,14 @@ async function createWebRuntimeSessionTerminalResult( } const intentOwner = captureWebSessionIntentOwner(environmentId) const callEnvironment = captureRuntimeEnvironmentCall(environmentId, intentOwner.pairingRevision) + const assertSessionTabCreateAttributionDisableSupported = async (): Promise => { + const response = await callEnvironment({ method: 'status.get', timeoutMs: 15_000 }) + const status = unwrapRuntimeRpcResult(response as RuntimeRpcResponse) + if (!hostSupportsSessionTabTerminalCreateAttributionDisable(status)) { + throw new Error(SESSION_TAB_TERMINAL_CREATE_ATTRIBUTION_UPDATE_REQUIRED_MESSAGE) + } + return status + } if (args.selectWorktree !== false) { selectWebRuntimeSessionWorktree(args.worktreeId, environmentId) @@ -272,6 +294,8 @@ async function createWebRuntimeSessionTerminalResult( let createdTabId: string | undefined let createdLeafId: string | undefined try { + const env = withLegacyTerminalAttributionDisabledEnv(args.env) + const envToDelete = addLegacyTerminalAttributionDisableRequest(args.envToDelete) const agent = args.launchAgent ?? args.agent const agentArgsOverride = args.agentArgs !== undefined ? args.agentArgs : args.launchConfig?.agentArgs @@ -283,7 +307,7 @@ async function createWebRuntimeSessionTerminalResult( ? undefined : args.agentSessionKind === 'resume' ? args.providerSession - ? async () => + ? async (authority: { runtimeId: string }) => unwrapRuntimeRpcResult( (await callEnvironment({ method: 'terminal.ensureAgentSession', @@ -301,11 +325,12 @@ async function createWebRuntimeSessionTerminalResult( : {}), presentation: 'background' }, - timeoutMs: 15_000 + timeoutMs: 15_000, + expectedRuntimeId: authority.runtimeId })) as RuntimeRpcResponse ) : undefined - : async () => + : async (authority: { runtimeId: string }) => await createAgentSessionCreateOperation().run(async (clientOperationId) => unwrapRuntimeRpcResult( (await callEnvironment({ @@ -328,7 +353,8 @@ async function createWebRuntimeSessionTerminalResult( }, clientOperationId ), - timeoutMs: 15_000 + timeoutMs: 15_000, + expectedRuntimeId: authority.runtimeId })) as RuntimeRpcResponse ) ) @@ -336,11 +362,15 @@ async function createWebRuntimeSessionTerminalResult( terminal: CreatedAgentTerminalIdentity }>({ environmentId, + expectedEnvironmentPairingRevision: intentOwner.pairingRevision, ...(hostAuthority ? { hostAuthority } : {}), - ...(args.agentSessionKind === 'resume' && agent === 'omp' - ? { hostAuthorityCapability: AGENT_SESSION_OMP_RESUME_PATH_RUNTIME_CAPABILITY } - : {}), - legacy: async () => { + requiredHostAuthorityCapabilities: [ + TERMINAL_ATTRIBUTION_REMOVED_RUNTIME_CAPABILITY, + ...(args.agentSessionKind === 'resume' && agent === 'omp' + ? [AGENT_SESSION_OMP_RESUME_PATH_RUNTIME_CAPABILITY] + : []) + ], + legacy: async ({ authority }) => { const response = await callEnvironment({ method: 'session.tabs.createTerminal', params: { @@ -349,8 +379,8 @@ async function createWebRuntimeSessionTerminalResult( targetGroupId: args.targetGroupId, command: args.command, cwd: args.cwd, - ...(args.env ? { env: args.env } : {}), - ...(args.envToDelete ? { envToDelete: args.envToDelete } : {}), + env, + ...(envToDelete ? { envToDelete } : {}), startupCommandDelivery: args.startupCommandDelivery, ...(args.launchConfig ? { launchConfig: args.launchConfig } : {}), ...(args.launchToken ? { launchToken: args.launchToken } : {}), @@ -362,7 +392,8 @@ async function createWebRuntimeSessionTerminalResult( select: args.activate !== false, navigation: 'caller' }, - timeoutMs: 15_000 + timeoutMs: 15_000, + expectedRuntimeId: authority.runtimeId }) const legacyCreated = unwrapRuntimeRpcResult( response as RuntimeRpcResponse @@ -394,6 +425,7 @@ async function createWebRuntimeSessionTerminalResult( }) } } else { + const status = await assertSessionTabCreateAttributionDisableSupported() const response = await callEnvironment({ method: 'session.tabs.createTerminal', params: { @@ -402,8 +434,8 @@ async function createWebRuntimeSessionTerminalResult( targetGroupId: args.targetGroupId, command: args.command, cwd: args.cwd, - ...(args.env ? { env: args.env } : {}), - ...(args.envToDelete ? { envToDelete: args.envToDelete } : {}), + env, + ...(envToDelete ? { envToDelete } : {}), startupCommandDelivery: args.startupCommandDelivery, ...(args.launchConfig ? { launchConfig: args.launchConfig } : {}), ...(args.launchToken ? { launchToken: args.launchToken } : {}), @@ -413,7 +445,8 @@ async function createWebRuntimeSessionTerminalResult( select: args.activate !== false, navigation: 'caller' }, - timeoutMs: 15_000 + timeoutMs: 15_000, + expectedRuntimeId: status.runtimeId }) const created = unwrapRuntimeRpcResult( response as RuntimeRpcResponse @@ -1185,26 +1218,37 @@ export function splitWebRuntimeTerminal( direction, pendingMirrorSuppressionId ) - void window.api.runtimeEnvironments - .call({ - selector: environmentId, - method: 'terminal.split', - params: { - terminal: remote.handle, - direction, - telemetrySource - }, - timeoutMs: 15_000 + const callEnvironment = captureRuntimeEnvironmentCall(environmentId) + void callEnvironment({ + method: 'status.get', + timeoutMs: 15_000 + }) + .then((response) => { + const status = unwrapRuntimeRpcResult(response as RuntimeRpcResponse) + if (!hostSupportsTerminalSplitAttributionDisable(status)) { + throw new Error(TERMINAL_SPLIT_ATTRIBUTION_UPDATE_REQUIRED_MESSAGE) + } + return callEnvironment({ + method: 'terminal.split', + params: { + terminal: remote.handle, + direction, + env: withLegacyTerminalAttributionDisabledEnv(undefined), + envToDelete: addLegacyTerminalAttributionDisableRequest(undefined), + telemetrySource + }, + timeoutMs: 15_000, + expectedRuntimeId: status.runtimeId + }) }) .then((response) => { unwrapRuntimeRpcResult(response as RuntimeRpcResponse<{ split: RuntimeTerminalSplit }>) }) .catch((error) => { releasePendingMirrorSuppression() - console.warn( - '[web-runtime-session] failed to split terminal:', - error instanceof Error ? error.message : String(error) - ) + const message = error instanceof Error ? error.message : String(error) + toast.error(message) + console.warn('[web-runtime-session] failed to split terminal:', message) }) return true } diff --git a/src/shared/legacy-terminal-attribution-env.test.ts b/src/shared/legacy-terminal-attribution-env.test.ts new file mode 100644 index 00000000000..1bb84cc5f96 --- /dev/null +++ b/src/shared/legacy-terminal-attribution-env.test.ts @@ -0,0 +1,99 @@ +import { describe, expect, it } from 'vitest' +import { + addLegacyTerminalAttributionDisableRequest, + hostSupportsSessionTabTerminalCreateAttributionDisable, + hostSupportsTerminalCreateAttributionDisable, + hostSupportsTerminalSplitAttributionDisable, + LEGACY_TERMINAL_ATTRIBUTION_BYPASS_ENV_KEY, + LEGACY_TERMINAL_ATTRIBUTION_ENABLE_ENV_KEY, + withLegacyTerminalAttributionDisabledEnv +} from './legacy-terminal-attribution-env' + +describe('legacy terminal attribution environment', () => { + it('adds the inert old-host gate once', () => { + expect(addLegacyTerminalAttributionDisableRequest(['CODEX_HOME'])).toEqual([ + 'CODEX_HOME', + LEGACY_TERMINAL_ATTRIBUTION_ENABLE_ENV_KEY + ]) + expect( + addLegacyTerminalAttributionDisableRequest([LEGACY_TERMINAL_ATTRIBUTION_ENABLE_ENV_KEY]) + ).toEqual([LEGACY_TERMINAL_ATTRIBUTION_ENABLE_ENV_KEY]) + }) + + it('fails instead of dropping a caller deletion at the wire limit', () => { + const full = Array.from({ length: 32 }, (_, index) => `KEY_${index}`) + expect(() => addLegacyTerminalAttributionDisableRequest(full)).toThrow( + 'Terminal environment deletion limit' + ) + }) + + it('adds the wrapper bypass without mutating caller env', () => { + const env = { CODEX_HOME: '/tmp/codex' } + expect(withLegacyTerminalAttributionDisabledEnv(env)).toEqual({ + CODEX_HOME: '/tmp/codex', + [LEGACY_TERMINAL_ATTRIBUTION_BYPASS_ENV_KEY]: '1' + }) + expect(env).toEqual({ CODEX_HOME: '/tmp/codex' }) + }) + + it('accepts only split hosts that removed attribution for every pane owner', () => { + expect(hostSupportsTerminalSplitAttributionDisable({ appVersion: '1.4.18' })).toBe(false) + expect(hostSupportsTerminalSplitAttributionDisable({ appVersion: '1.4.19' })).toBe(false) + expect(hostSupportsTerminalSplitAttributionDisable({ appVersion: 'v1.4.181' })).toBe(false) + expect(hostSupportsTerminalSplitAttributionDisable({ capabilities: ['mobile.tasks.v1'] })).toBe( + false + ) + expect( + hostSupportsTerminalSplitAttributionDisable({ + capabilities: ['terminal.attribution-removed.v1'] + }) + ).toBe(true) + }) + + it('accepts only capability-proven terminal creation hosts', () => { + expect(hostSupportsSessionTabTerminalCreateAttributionDisable({ appVersion: '1.4.89' })).toBe( + false + ) + expect(hostSupportsSessionTabTerminalCreateAttributionDisable({ appVersion: '1.4.90' })).toBe( + false + ) + expect( + hostSupportsSessionTabTerminalCreateAttributionDisable({ appVersion: 'not-semver' }) + ).toBe(false) + expect( + hostSupportsSessionTabTerminalCreateAttributionDisable({ + capabilities: ['workspace-run-context.v1'] + }) + ).toBe(false) + expect( + hostSupportsSessionTabTerminalCreateAttributionDisable({ + capabilities: ['terminal.quick-commands.v1'] + }) + ).toBe(false) + }) + + it('accepts capability-proven current hosts regardless of development version', () => { + const status = { + appVersion: '0.0.0-dev', + capabilities: ['terminal.attribution-removed.v1'] + } + expect(hostSupportsTerminalSplitAttributionDisable(status)).toBe(true) + expect(hostSupportsTerminalCreateAttributionDisable(status)).toBe(true) + expect(hostSupportsSessionTabTerminalCreateAttributionDisable(status)).toBe(true) + }) + + it('fails closed for malformed host status fields', () => { + expect( + hostSupportsSessionTabTerminalCreateAttributionDisable({ + appVersion: 1.49, + capabilities: 'terminal.attribution-removed.v1' + }) + ).toBe(false) + expect( + hostSupportsTerminalSplitAttributionDisable({ + appVersion: {}, + capabilities: ['terminal.attribution-removed.v1', 1] + }) + ).toBe(false) + }) +}) diff --git a/src/shared/legacy-terminal-attribution-env.ts b/src/shared/legacy-terminal-attribution-env.ts new file mode 100644 index 00000000000..5237280a486 --- /dev/null +++ b/src/shared/legacy-terminal-attribution-env.ts @@ -0,0 +1,68 @@ +import { TERMINAL_ATTRIBUTION_REMOVED_RUNTIME_CAPABILITY } from './protocol-version' + +export const LEGACY_TERMINAL_ATTRIBUTION_ENABLE_ENV_KEY = 'ORCA_ENABLE_GIT_ATTRIBUTION' +export const LEGACY_TERMINAL_ATTRIBUTION_BYPASS_ENV_KEY = 'ORCA_ATTRIBUTION_BYPASS' +export const TERMINAL_SPLIT_ATTRIBUTION_UPDATE_REQUIRED_MESSAGE = + 'Terminal splitting requires an updated workspace host that can verify attribution removal for every pane owner. Update the host and try again.' +export const TERMINAL_CREATE_ATTRIBUTION_UPDATE_REQUIRED_MESSAGE = + 'Creating terminals requires an updated workspace host that can verify attribution removal. Update the host and try again.' +export const MOBILE_TERMINAL_CREATE_ATTRIBUTION_UPDATE_REQUIRED_MESSAGE = + 'Creating terminals from Orca Mobile requires a newer workspace host that can verify safe terminal environment forwarding. Update the host and try again.' +export const SESSION_TAB_TERMINAL_CREATE_ATTRIBUTION_UPDATE_REQUIRED_MESSAGE = + 'Creating terminals requires a newer workspace host that can verify safe terminal environment forwarding. Update the host and try again.' +const MAX_TERMINAL_ENV_DELETION_KEYS = 32 + +type TerminalAttributionHostStatus = { + appVersion?: string + capabilities?: string[] +} + +function readTerminalAttributionHostStatus(value: unknown): TerminalAttributionHostStatus { + if (!value || typeof value !== 'object') { + return {} + } + const appVersion = Reflect.get(value, 'appVersion') + const capabilities = Reflect.get(value, 'capabilities') + return { + ...(typeof appVersion === 'string' ? { appVersion } : {}), + ...(Array.isArray(capabilities) && capabilities.every((entry) => typeof entry === 'string') + ? { capabilities } + : {}) + } +} + +export function hostSupportsTerminalSplitAttributionDisable(value: unknown): boolean { + const status = readTerminalAttributionHostStatus(value) + // Why: only removal-capable hosts cover both runtime- and renderer-owned split targets. + return status.capabilities?.includes(TERMINAL_ATTRIBUTION_REMOVED_RUNTIME_CAPABILITY) === true +} + +export function hostSupportsTerminalCreateAttributionDisable(value: unknown): boolean { + const status = readTerminalAttributionHostStatus(value) + return status.capabilities?.includes(TERMINAL_ATTRIBUTION_REMOVED_RUNTIME_CAPABILITY) === true +} + +export function hostSupportsSessionTabTerminalCreateAttributionDisable(value: unknown): boolean { + const status = readTerminalAttributionHostStatus(value) + return status.capabilities?.includes(TERMINAL_ATTRIBUTION_REMOVED_RUNTIME_CAPABILITY) === true +} + +export function withLegacyTerminalAttributionDisabledEnv( + env: Record | undefined +): Record { + return { ...env, [LEGACY_TERMINAL_ATTRIBUTION_BYPASS_ENV_KEY]: '1' } +} + +export function addLegacyTerminalAttributionDisableRequest( + envToDelete: readonly string[] | undefined +): string[] | undefined { + const deduplicated = [...new Set(envToDelete ?? [])] + const gateIndex = deduplicated.indexOf(LEGACY_TERMINAL_ATTRIBUTION_ENABLE_ENV_KEY) + if (gateIndex !== -1) { + deduplicated.splice(gateIndex, 1) + } + if (deduplicated.length >= MAX_TERMINAL_ENV_DELETION_KEYS) { + throw new Error('Terminal environment deletion limit leaves no room for attribution cleanup') + } + return [...deduplicated, LEGACY_TERMINAL_ATTRIBUTION_ENABLE_ENV_KEY] +} diff --git a/src/shared/protocol-version.ts b/src/shared/protocol-version.ts index eb61be79c97..28a89f9814b 100644 --- a/src/shared/protocol-version.ts +++ b/src/shared/protocol-version.ts @@ -84,6 +84,9 @@ export const AGENT_SESSION_HOST_AUTHORITY_RUNTIME_CAPABILITY = 'agent-session.host-authority.v1' as const export const AGENT_SESSION_OMP_RESUME_PATH_RUNTIME_CAPABILITY = 'agent-session.omp-resume-path.v1' as const +// Why: old host-authority launches cannot accept env deletions and may still inject attribution. +export const TERMINAL_ATTRIBUTION_REMOVED_RUNTIME_CAPABILITY = + 'terminal.attribution-removed.v1' as const // Why: older runtimes strip mutation owner fields, so clients must fence writes before RPC. export const FILE_MUTATION_OWNERSHIP_RUNTIME_CAPABILITY = 'files.mutation-ownership.v1' as const export const FILE_MUTATION_OWNERSHIP_UPDATE_REQUIRED_MESSAGE = @@ -121,6 +124,7 @@ export const RUNTIME_CAPABILITIES = [ REMOTE_SERVER_UPDATE_CAPABILITY, AGENT_SESSION_HOST_AUTHORITY_RUNTIME_CAPABILITY, AGENT_SESSION_OMP_RESUME_PATH_RUNTIME_CAPABILITY, + TERMINAL_ATTRIBUTION_REMOVED_RUNTIME_CAPABILITY, FILE_MUTATION_OWNERSHIP_RUNTIME_CAPABILITY, ACCOUNT_IMPORT_RUNTIME_CAPABILITY, CODEX_RESET_CREDIT_RUNTIME_CAPABILITY diff --git a/src/shared/remote-runtime-client.test.ts b/src/shared/remote-runtime-client.test.ts index 9ead47f4067..4c3bfba011e 100644 --- a/src/shared/remote-runtime-client.test.ts +++ b/src/shared/remote-runtime-client.test.ts @@ -35,6 +35,25 @@ afterEach(async () => { }) describe('subscribeRemoteRuntimeRequest', () => { + it('serializes the expected runtime fence on subscription requests', async () => { + const server = await createSubscriptionServer() + const subscription = await subscribeRemoteRuntimeRequest( + server.pairing, + 'terminal.subscribe', + {}, + 1000, + { onResponse: vi.fn(), onError: vi.fn() }, + undefined, + { expectedRuntimeId: 'runtime-1' } + ) + + await expect(server.nextRequest).resolves.toMatchObject({ + method: 'terminal.subscribe', + expectedRuntimeId: 'runtime-1' + }) + subscription.close() + }) + it('includes WebSocket close details when subscription admission is rejected', async () => { const server = await createClosingServer(1013, 'Maximum connections reached') @@ -335,6 +354,7 @@ async function createSubscriptionServer( pairing: PairingOffer nextBinary: Promise nextAuth: Promise + nextRequest: Promise> }> { const serverKeyPair = generateKeyPair() let resolveBinary: (bytes: Uint8Array) => void = () => {} @@ -345,6 +365,10 @@ async function createSubscriptionServer( const nextAuth = new Promise((resolve) => { resolveAuth = resolve }) + let resolveRequest: (request: Record) => void = () => {} + const nextRequest = new Promise>((resolve) => { + resolveRequest = resolve + }) const wss = new WebSocketServer({ port: 0, autoPong: options.disableAutoPong !== true }) servers.push(wss) @@ -386,7 +410,8 @@ async function createSubscriptionServer( return } - const request = JSON.parse(plaintext) as { id: string } + const request = JSON.parse(plaintext) as { id: string } & Record + resolveRequest(request) sendEncrypted(ws, sharedKey, { id: request.id, ok: true, @@ -419,7 +444,7 @@ async function createSubscriptionServer( if (!pairing) { throw new Error('Failed to create test pairing') } - return { pairing, nextBinary, nextAuth } + return { pairing, nextBinary, nextAuth, nextRequest } } function sendEncrypted(ws: WebSocket, sharedKey: Uint8Array, message: unknown): void { diff --git a/src/shared/remote-runtime-client.ts b/src/shared/remote-runtime-client.ts index c53565dc8a2..3b308724b57 100644 --- a/src/shared/remote-runtime-client.ts +++ b/src/shared/remote-runtime-client.ts @@ -497,14 +497,16 @@ export async function subscribeRemoteRuntimeRequest( params: unknown, timeoutMs: number, callbacks: RemoteRuntimeSubscriptionCallbacks, - livenessOptions?: RemoteRuntimeSocketLivenessOptions + livenessOptions?: RemoteRuntimeSocketLivenessOptions, + envelope?: RuntimeOrchestrationEnvelope ): Promise { const requestId = randomUUID() const serializedRequest = serializeRemoteRuntimeRpcRequest({ requestId, deviceToken: pairing.deviceToken, method, - params + params, + envelope }) const serializedAuth = serializeRemoteRuntimePayload({ type: 'e2ee_auth', diff --git a/src/shared/remote-runtime-memory-limits.ts b/src/shared/remote-runtime-memory-limits.ts index a41409ffdfe..8b5ce04cc56 100644 --- a/src/shared/remote-runtime-memory-limits.ts +++ b/src/shared/remote-runtime-memory-limits.ts @@ -71,6 +71,7 @@ export function serializeRemoteRuntimeRpcRequest(args: { deviceToken: args.deviceToken, method: args.method, params: args.params, + expectedRuntimeId: args.envelope?.expectedRuntimeId, orchestrationCapability: args.envelope?.orchestrationCapability, orchestrationContractVersion: args.envelope?.orchestrationContractVersion, orchestrationRequestId: args.envelope?.orchestrationRequestId, diff --git a/src/shared/remote-runtime-shared-control-admission.ts b/src/shared/remote-runtime-shared-control-admission.ts index e526ba1bb95..f6638ed7f63 100644 --- a/src/shared/remote-runtime-shared-control-admission.ts +++ b/src/shared/remote-runtime-shared-control-admission.ts @@ -12,6 +12,7 @@ export function admitSharedControlSubscription(args: { deviceToken: string method: string params: unknown + expectedRuntimeId?: string }): number { if (args.subscriptions.size >= REMOTE_RUNTIME_MAX_SUBSCRIPTIONS) { throw new RemoteRuntimeClientError( @@ -33,12 +34,18 @@ export function admitSharedControlSubscription(args: { return retainedParamsBytes } -function serializeRequest(args: { deviceToken: string; method: string; params: unknown }): void { +function serializeRequest(args: { + deviceToken: string + method: string + params: unknown + expectedRuntimeId?: string +}): void { serializeRemoteRuntimeRpcRequest({ requestId: '00000000-0000-4000-8000-000000000000', deviceToken: args.deviceToken, method: args.method, - params: args.params + params: args.params, + envelope: { expectedRuntimeId: args.expectedRuntimeId } }) } diff --git a/src/shared/remote-runtime-shared-control-connection.ts b/src/shared/remote-runtime-shared-control-connection.ts index 4fb40525245..93925ccf15e 100644 --- a/src/shared/remote-runtime-shared-control-connection.ts +++ b/src/shared/remote-runtime-shared-control-connection.ts @@ -37,7 +37,7 @@ export class RemoteRuntimeSharedControlConnection { private sharedKey: Uint8Array | null = null private socketCleanup: (() => void) | null = null private readonly reconnect = new SharedControlReconnectScheduler() - private readonly readyStableReset: SharedControlReadyStableResetTimer + private readonly stableReset: SharedControlReadyStableResetTimer private intentionallyClosed = false private lastConnectedAt: number | null = null private lastClose: { code: number; reason: string } | null = null @@ -51,15 +51,13 @@ export class RemoteRuntimeSharedControlConnection { constructor( private readonly pairing: PairingOffer, - private readonly options: { + private readonly opts: { environmentId?: string reconnectStableResetMs?: number liveness?: RemoteRuntimeSocketLivenessOptions } = {} ) { - this.readyStableReset = new SharedControlReadyStableResetTimer( - options.reconnectStableResetMs ?? 30_000 - ) + this.stableReset = new SharedControlReadyStableResetTimer(opts.reconnectStableResetMs ?? 30_000) } request( @@ -85,13 +83,15 @@ export class RemoteRuntimeSharedControlConnection { method: string, params: unknown, timeoutMs: number, - callbacks: SharedControlSubscriptionCallbacks + callbacks: SharedControlSubscriptionCallbacks, + expectedRuntimeId?: string ): Promise { return startSharedControlSubscription({ subscriptions: this.subscriptions, deviceToken: this.pairing.deviceToken, method, params, + expectedRuntimeId, callbacks, ensureReady: () => this.ensureReadyWithTimeout(timeoutMs), sendSubscription: (subscription) => this.sendSubscription(subscription), @@ -182,7 +182,7 @@ export class RemoteRuntimeSharedControlConnection { onError: (error) => this.handleSocketClosed(error, socketGeneration), onTextFrame: (frame) => this.handleTextFrame(frame, socketGeneration), liveness: { - options: this.options.liveness, + options: this.opts.liveness, onDead: (error) => this.handleSocketClosed(error, socketGeneration) } }) @@ -204,7 +204,7 @@ export class RemoteRuntimeSharedControlConnection { frame, state: this.state, sharedKey: this.sharedKey, - environmentId: this.options.environmentId, + environmentId: this.opts.environmentId, deviceToken: this.pairing.deviceToken, pendingRequests: this.pendingRequests, subscriptions: this.subscriptions, @@ -217,7 +217,7 @@ export class RemoteRuntimeSharedControlConnection { sendEncrypted: (payload) => this.sendEncrypted(payload), markReady: () => { this.lastConnectedAt = Date.now() - this.readyStableReset.schedule({ + this.stableReset.schedule({ getState: () => this.state, getSocket: () => this.ws, reset: () => this.reconnect.resetAttempt() @@ -301,7 +301,7 @@ export class RemoteRuntimeSharedControlConnection { private closeSocket(error?: Error, preserveReadyWaitersAndPendingRequests = false): void { closeSharedControlSocket({ - environmentId: this.options.environmentId, + environmentId: this.opts.environmentId, state: this.state, pendingRequests: this.pendingRequests, subscriptions: this.subscriptions, @@ -311,7 +311,7 @@ export class RemoteRuntimeSharedControlConnection { ws: this.ws, error, preserveReadyWaitersAndPendingRequests, - clearReadyStableTimer: () => this.readyStableReset.clear() + clearReadyStableTimer: () => this.stableReset.clear() }) this.ws = this.sharedKey = null this.socketCleanup = null diff --git a/src/shared/remote-runtime-shared-control-send.ts b/src/shared/remote-runtime-shared-control-send.ts index 2a7ff16cfa5..1862751cd6b 100644 --- a/src/shared/remote-runtime-shared-control-send.ts +++ b/src/shared/remote-runtime-shared-control-send.ts @@ -36,7 +36,8 @@ export function sendSharedControlSubscription(args: { id: args.subscription.requestId, deviceToken: args.deviceToken, method: args.subscription.method, - params: args.subscription.params + params: args.subscription.params, + expectedRuntimeId: args.subscription.expectedRuntimeId }) ) { args.subscription.sent = true diff --git a/src/shared/remote-runtime-shared-control-subscription-start.ts b/src/shared/remote-runtime-shared-control-subscription-start.ts index 2b22aaabc5b..1cc4a91a5a8 100644 --- a/src/shared/remote-runtime-shared-control-subscription-start.ts +++ b/src/shared/remote-runtime-shared-control-subscription-start.ts @@ -14,6 +14,7 @@ export async function startSharedControlSubscription(args: { deviceToken: string method: string params: unknown + expectedRuntimeId?: string callbacks: SharedControlSubscriptionCallbacks ensureReady: () => Promise sendSubscription: (subscription: SharedControlLogicalSubscription) => void @@ -23,13 +24,15 @@ export async function startSharedControlSubscription(args: { subscriptions: args.subscriptions, deviceToken: args.deviceToken, method: args.method, - params: args.params + params: args.params, + expectedRuntimeId: args.expectedRuntimeId }) const requestId = randomUUID() const subscription = createSharedControlSubscription({ requestId, method: args.method, params: args.params, + expectedRuntimeId: args.expectedRuntimeId, retainedParamsBytes, callbacks: args.callbacks }) diff --git a/src/shared/remote-runtime-shared-control-subscriptions.test.ts b/src/shared/remote-runtime-shared-control-subscriptions.test.ts index de5d5718b55..b61b2eff34e 100644 --- a/src/shared/remote-runtime-shared-control-subscriptions.test.ts +++ b/src/shared/remote-runtime-shared-control-subscriptions.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it, vi } from 'vitest' import type { RuntimeRpcResponse } from './runtime-rpc-envelope' +import { sendSharedControlSubscription } from './remote-runtime-shared-control-send' import { closeSharedControlLogicalSubscription, createSharedControlSubscription, @@ -33,6 +34,39 @@ function okResponse(subscriptionId: string): RuntimeRpcResponse { } describe('closeSharedControlLogicalSubscription — replay-window leak', () => { + it('preserves the runtime fence when reconnect replay resends a subscription', () => { + const subscriptions = new Map>() + const subscription = createSharedControlSubscription({ + requestId: 'req-fenced', + method: 'runtime.clientEvents.subscribe', + params: null, + expectedRuntimeId: 'runtime-1', + retainedParamsBytes: 0, + callbacks: { onResponse: vi.fn(), onError: vi.fn() } + }) + subscriptions.set(subscription.requestId, subscription) + const payloads: unknown[] = [] + const send = (current: SharedControlLogicalSubscription): void => + sendSharedControlSubscription({ + subscriptions, + subscription: current, + deviceToken: 'device-token', + send: (payload) => { + payloads.push(payload) + return true + } + }) + + send(subscription) + replaySharedControlSubscriptions({ subscriptions, send }) + + expect(payloads).toHaveLength(2) + expect(payloads).toEqual([ + expect.objectContaining({ expectedRuntimeId: 'runtime-1' }), + expect.objectContaining({ expectedRuntimeId: 'runtime-1' }) + ]) + }) + it('sends the unsubscribe when closed after an established subscribe replay completes', () => { const { subscriptions, subscription } = makeSubscriptions() // First establishment: server assigned a concrete subscription id. diff --git a/src/shared/remote-runtime-shared-control-subscriptions.ts b/src/shared/remote-runtime-shared-control-subscriptions.ts index 4a93076d23f..92da86bcd5d 100644 --- a/src/shared/remote-runtime-shared-control-subscriptions.ts +++ b/src/shared/remote-runtime-shared-control-subscriptions.ts @@ -14,6 +14,7 @@ export function createSharedControlSubscription(args: { requestId: string method: string params: unknown + expectedRuntimeId?: string retainedParamsBytes: number callbacks: SharedControlSubscriptionCallbacks }): SharedControlLogicalSubscription { @@ -21,6 +22,7 @@ export function createSharedControlSubscription(args: { requestId: args.requestId, method: args.method, params: args.params, + expectedRuntimeId: args.expectedRuntimeId, retainedParamsBytes: args.retainedParamsBytes, callbacks: args.callbacks, sent: false, diff --git a/src/shared/remote-runtime-shared-control-types.ts b/src/shared/remote-runtime-shared-control-types.ts index c6e55134762..592fdc47168 100644 --- a/src/shared/remote-runtime-shared-control-types.ts +++ b/src/shared/remote-runtime-shared-control-types.ts @@ -33,6 +33,7 @@ export type SharedControlLogicalSubscription = { requestId: string method: string params: unknown + expectedRuntimeId?: string retainedParamsBytes: number callbacks: SharedControlSubscriptionCallbacks sent: boolean diff --git a/src/shared/runtime-rpc-envelope.ts b/src/shared/runtime-rpc-envelope.ts index d407de72cb9..38cbc4659b1 100644 --- a/src/shared/runtime-rpc-envelope.ts +++ b/src/shared/runtime-rpc-envelope.ts @@ -76,6 +76,8 @@ export type RuntimeRpcFailure = { export type RuntimeRpcResponse = RuntimeRpcSuccess | RuntimeRpcFailure export type RuntimeOrchestrationEnvelope = { + /** Reject before dispatch when the request reaches a replacement runtime. */ + expectedRuntimeId?: string orchestrationCapability?: string orchestrationContractVersion?: number orchestrationRequestId?: string