diff --git a/config/reliability-gates.jsonc b/config/reliability-gates.jsonc index 8a0036e2acc..06a2231f0f6 100644 --- a/config/reliability-gates.jsonc +++ b/config/reliability-gates.jsonc @@ -3474,12 +3474,12 @@ "providers": ["local", "daemon", "ssh", "remote-runtime"], "coveredPlatforms": ["macos"], "coveredProviders": ["local", "ssh"], - "coverageNotes": "A macOS unit harness loads the installed patched node-pty WindowsTerminal prototype without constructing native state, proving pre-ready kill/destroy dispatch immediately. A relay artifact harness reproduces vanilla npm node-pty retaining one deferred kill with zero native close calls, then proves both Windows relay artifacts hash, ship, apply, include the local ConPTY console-list fallback, and idempotently verify the fix. Relay tests simulate win32 shutdown, repeated stale-spawn native failure, one-shot ownership, and retry after native failure without deleting the only handle before close is proven. Relay boot UUIDs scope recycled pty-N counters through initial spawn, durable-lease persistence, reattach, and SSH target-id migration; mismatched generations expire before raw attach and release their exact in-memory retry owner, while pane identity protects legacy leases. Main-process retry owners persist local daemon and remote-runtime teardown intent across renderer and app restarts, coalesce by exact provider identity, use one shared one-shot timer per owner, and fence SSH relay plus remote-runtime generation reuse before close. Local Windows shutdown retains provider records and listeners after native throw unless onExit independently proves death. A disconnected SSH provider rejects shutdown without tombstoning its attachable lease. Paired/mobile headless close refuses to depersist a terminal until the controller accepts durable retry ownership. Project removal keeps local or remote ownership until provider-verified shutdown and successful VM cleanup; runtime repo removal drains pre-spawn trust/team/command preparation plus create-first worktree creation, headless, renderer-backed mobile, renderer split, and selector-less renderer terminal admissions before fresh ownership/liveness snapshots, stop, and metadata mutation. Desktop and duplicate-host repo removal route through the same transaction; legacy worktree creation, renderer PTY spawn, and every local/SSH/folder worktree removal outcome—including Windows stale registration and orphan-compatible fallback—are fenced and verified before destructive mutation or metadata purge. Hidden local/SSH and remote-runtime background-launch cleanup retain exact PTY/handle retry ownership with bounded one-shot scheduling. Crash-durable teardown intent uses an O(1) revisioned append journal with exact SSH set/remove records, compacted to one checkpoint after a successful full-state save, instead of synchronously serializing every retained owner for each mutation. Targeted terminal.stop probes skip unrelated sessions. The same relay contract serves paired web/mobile clients; no live Windows ConPTY or Windows-over-SSH run is recorded yet.", + "coverageNotes": "A macOS unit harness loads the installed patched node-pty WindowsTerminal prototype without constructing native state, proving pre-ready kill/destroy dispatch immediately. A relay artifact harness reproduces vanilla npm node-pty retaining one deferred kill with zero native close calls, then proves both Windows relay artifacts hash, ship, apply, include the local ConPTY console-list fallback, and idempotently verify the fix. Relay tests simulate win32 shutdown, repeated stale-spawn native failure, one-shot ownership, and retry after native failure without deleting the only handle before close is proven. Relay boot UUIDs scope recycled pty-N counters through initial spawn, durable-lease persistence, reattach, and SSH target-id migration; mismatched generations expire before raw attach and release their exact in-memory retry owner, while pane identity protects legacy leases. Main-process retry owners persist local daemon and remote-runtime teardown intent across renderer and app restarts, coalesce by exact provider identity, use one shared one-shot timer per owner, and fence SSH relay plus remote-runtime generation reuse before close. Local and daemon providers retain pane/tab identity and reject stale teardown before native or daemon kill; daemon protocol v22 carries the identity across process boundaries, while legacy adapters fail closed unless main observed the exact pane. Lease-less SSH retry owners survive transport replacement and re-drive the replacement provider. Local Windows shutdown retains provider records and listeners after native throw unless onExit independently proves death. A disconnected SSH provider rejects shutdown without tombstoning its attachable lease. Paired/mobile headless close refuses to depersist a terminal until the controller accepts durable retry ownership. Project removal keeps local or remote ownership until provider-verified shutdown and successful VM cleanup; runtime repo removal drains pre-spawn trust/team/command preparation plus create-first worktree creation, headless, renderer-backed mobile, renderer split, and selector-less renderer terminal admissions before fresh ownership/liveness snapshots, stop, and metadata mutation. Desktop and duplicate-host repo removal route through the same transaction; unresolved terminal creation may continue for an unrelated host while the removed host remains fenced. Legacy worktree creation, renderer PTY spawn, and every local/SSH/folder worktree removal outcome—including Windows stale registration and orphan-compatible fallback—are fenced and verified before destructive mutation or metadata purge. Hidden local/SSH and remote-runtime background-launch cleanup retain exact PTY/handle retry ownership with bounded one-shot scheduling. Crash-durable teardown intent uses an O(1) revisioned append journal with exact SSH set/remove records, compacted to one checkpoint after a successful full-state save, instead of synchronously serializing every retained owner for each mutation. Targeted terminal.stop probes skip unrelated sessions. The same relay contract serves paired web/mobile clients; no live Windows ConPTY or Windows-over-SSH run is recorded yet.", "motivatingLinks": [ "https://github.com/stablyai/orca/issues/8260", "https://github.com/stablyai/orca/pull/8279" ], - "invariant": "A Windows PTY closed before its first output executes exactly one no-argument node-pty shutdown immediately; Orca never deletes ownership without accepted native shutdown or an observed exit, and a later retry can target only the same relay generation and pane while project removal fences both worktree and terminal creation.", + "invariant": "A Windows PTY closed before its first output executes exactly one no-argument node-pty shutdown immediately; Orca never deletes ownership without accepted native shutdown or an observed exit, and a later retry can target only the same provider generation and pane while project removal fences the removed host without blocking unrelated hosts.", "oracle": "Load the installed patched WindowsTerminal prototype in a pre-ready state and assert kill and destroy each close the terminal and native agent once with zero deferred work, while signal arguments still throw. Build both Windows SSH relay artifacts, reproduce vanilla npm node-pty with close=0/native-kill=0/deferred=1, apply the shipped patch, and assert close=1/native-kill=1/deferred=0, AttachConsole failure falls back to the shell pid, and the artifact version derives from all patch bytes. Assert remote npm install executes patch enforcement before its runtime probe and never finalizes a failed patch. In local and relay providers, assert native throws retain exact ownership/listeners and retry succeeds, while a synchronous onExit is accepted as independent death proof; provider absence must retain the SSH lease and reject shutdown. Persist relay generation with every durable lease, expire mismatches before attach while releasing their exact retry owner, and preserve generation through target migration. Reject local daemon and remote-runtime teardown, reload the durable store, wait for the startup provider before replay, replace an in-flight provider, and prove the same caller re-drives the replacement before settling, stronger coalesced shutdown options persist, and capped retries drain exact identities with zero final retry timers; persisted close intent for an already-removed runtime must drain without an unhandled rejection or retry wake loop. A generationless SSH id must not mutate or tombstone a generation-qualified replacement lease. Paired/mobile headless close must preserve persisted UI state when controller ownership is rejected. Create and clear 65 local plus 65 remote-runtime owners, reload immediately at both boundaries, and assert 260 O(1) journal records stay below 100 KiB with a 1 KiB per-record cap; repeat the same byte and record bounds for 50 generation-qualified SSH owners. Synchronous full-store flush count must remain zero, both a partial JSON tail and a complete record missing its newline must preserve all complete records, the next mutation must first rewrite an atomic recovered checkpoint, and the next full-state save must compact the journal to one revision-fenced checkpoint; an older valid sidecar must lose to a newer full-store fallback revision. Replace SSH relay and remote-runtime generations while a retry is retained and prove the new owner is never closed. Start pre-spawn trust/team preparation plus worktree and terminal creation surfaces before whole-project and host-scoped project removal and assert removal performs no ownership/liveness snapshot until they publish; host-scoped liveness must query only its selected provider so an unrelated SSH outage cannot veto removal. Removal-first and failed/concurrent admissions must reject or release without deadlock. Start a real worktree removal, pause its preflight, and assert a late renderer terminal admission is rejected; assert every successful registered, orphaned, leftover, already-missing, and stale-registration removal path verifies shutdown before mutation or metadata purge; assert desktop repo removal, worktree create/remove, and PTY spawn handlers all enter their runtime-owned transactions before mutation or provider work.", "commands": [ "pnpm exec vitest run --config config/vitest.config.ts config/scripts/build-relay-windows-node-pty-patch.test.mjs src/main/ssh/ssh-relay-native-deps-install.test.ts src/relay/node-pty-windows-pre-ready-shutdown.test.ts src/relay/pty-handler.test.ts src/renderer/src/components/terminal-pane/pty-transport.test.ts src/renderer/src/store/slices/repos-remove-project-purge-leak.test.ts src/renderer/src/store/slices/repos.test.ts src/renderer/src/store/slices/repos-runtime-vm-removal.test.ts src/renderer/src/lib/ephemeral-vm-runtime-cleanup.test.ts", @@ -3490,7 +3490,8 @@ "pnpm exec vitest run --config config/vitest.config.ts src/main/ipc/repos-create.test.ts src/main/window/attach-main-window-services.test.ts src/main/ipc/worktrees.test.ts src/main/ipc/worktrees-windows.test.ts src/main/ipc/pty.test.ts src/main/runtime/orca-runtime.test.ts src/renderer/src/lib/launch-agent-background-session.test.ts src/renderer/src/store/slices/repos-host-identity-routing.test.ts", "pnpm exec vitest run --config config/vitest.config.ts src/shared/ssh-pty-id.test.ts src/shared/ssh-types.test.ts src/main/providers/local-pty-provider.test.ts src/main/providers/ssh-pty-provider.test.ts src/main/persistence.test.ts src/main/ssh/ssh-relay-session.test.ts src/main/ssh/ssh-relay-generation-reattach.test.ts src/main/ssh/ssh-target-id-migration.test.ts src/relay/pty-handler.test.ts src/relay/subprocess.test.ts src/renderer/src/components/terminal-pane/pty-transport.test.ts src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.test.ts src/renderer/src/lib/pty-kill-retry-ownership.test.ts src/renderer/src/lib/runtime-terminal-close-retry-ownership.test.ts src/renderer/src/lib/ephemeral-vm-runtime-cleanup.test.ts src/renderer/src/lib/launch-agent-background-session.test.ts src/renderer/src/store/slices/repos.test.ts src/renderer/src/store/slices/repos-host-identity-routing.test.ts src/main/runtime/orca-runtime.test.ts src/main/ipc/pty.test.ts", "pnpm exec vitest run --config config/vitest.config.ts src/main/ipc/repos-create.test.ts src/main/window/attach-main-window-services.test.ts src/main/ipc/worktrees.test.ts src/main/ipc/worktrees-windows.test.ts src/main/ipc/pty.test.ts src/main/runtime/orca-runtime.test.ts src/renderer/src/lib/launch-agent-background-session.test.ts src/renderer/src/lib/runtime-terminal-close-retry-ownership.test.ts src/renderer/src/store/slices/repos-host-identity-routing.test.ts", - "pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/orca-runtime.test.ts src/main/ipc/worktrees.test.ts src/main/ipc/pty.test.ts src/main/ipc/runtime-environments.test.ts src/main/runtime/rpc/terminal-close-generation.test.ts src/main/persistence.test.ts src/main/ssh/ssh-relay-session.test.ts src/main/ipc/ssh.test.ts src/renderer/src/store/slices/worktrees.test.ts" + "pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/orca-runtime.test.ts src/main/ipc/worktrees.test.ts src/main/ipc/pty.test.ts src/main/ipc/runtime-environments.test.ts src/main/runtime/rpc/terminal-close-generation.test.ts src/main/persistence.test.ts src/main/ssh/ssh-relay-session.test.ts src/main/ipc/ssh.test.ts src/renderer/src/store/slices/worktrees.test.ts", + "pnpm exec vitest run --config config/vitest.config.ts src/main/providers/local-pty-provider.test.ts src/main/daemon/daemon-pty-adapter.test.ts src/main/daemon/terminal-host.test.ts src/main/ipc/pty.test.ts src/main/ipc/runtime-terminal-close-retry-owner.test.ts src/main/runtime/orca-runtime.test.ts" ], "testFiles": [ "config/scripts/build-relay-windows-node-pty-patch.test.mjs", @@ -3506,6 +3507,8 @@ "src/shared/ssh-pty-id.test.ts", "src/shared/ssh-types.test.ts", "src/main/providers/local-pty-provider.test.ts", + "src/main/daemon/daemon-pty-adapter.test.ts", + "src/main/daemon/terminal-host.test.ts", "src/main/providers/ssh-pty-provider.test.ts", "src/main/persistence.test.ts", "src/main/ssh/ssh-relay-session.test.ts", @@ -3515,6 +3518,7 @@ "src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.test.ts", "src/renderer/src/lib/pty-kill-retry-ownership.test.ts", "src/main/ipc/pty.test.ts", + "src/main/ipc/runtime-terminal-close-retry-owner.test.ts", "src/main/ipc/repos-create.test.ts", "src/main/ipc/worktrees.test.ts", "src/main/ipc/worktrees-windows.test.ts", @@ -3600,6 +3604,7 @@ "host-scoped project removal drains create-first work and rejects removal-first creation before mutating one duplicate-host row", "duplicate repo and worktree ids isolate admissions and PTY liveness by execution host while whole-project removal still drains every host scope", "host-scoped terminal stop succeeds when an unrelated provider is unavailable without disconnecting that host's cached PTYs", + "workspace resolution on an unrelated host proceeds while the removed host remains admission-fenced", "worktree removal rejects a late renderer terminal admission while destructive preflight is still pending", "already-missing exceptional removal verifies terminal stop before metadata purge", "failed and concurrent admissions release without deadlocking project removal", @@ -3622,7 +3627,20 @@ { "file": "src/main/providers/local-pty-provider.test.ts", "assertions": [ - "a synchronous Windows native kill throw retains PTY ownership and listeners so retry can succeed" + "a synchronous Windows native kill throw retains PTY ownership and listeners so retry can succeed", + "a stale pane identity is rejected before the local PTY can be killed" + ] + }, + { + "file": "src/main/daemon/daemon-pty-adapter.test.ts", + "assertions": [ + "daemon shutdown carries pane identity across RPC and rejects a stale kill" + ] + }, + { + "file": "src/main/daemon/terminal-host.test.ts", + "assertions": [ + "a reused live daemon session rejects attach and kill from a replacement pane identity" ] }, { @@ -3679,7 +3697,14 @@ "generationless identity-mismatch failure cannot tombstone a replacement relay-generation lease", "paired/mobile controller kill fails closed when offline SSH intent cannot be made durable", "host-scoped liveness lists only the selected provider instead of fanning out to unrelated SSH hosts", - "relay generation reuse removes stale retry ownership without touching the new PTY" + "relay generation reuse removes stale retry ownership without touching the new PTY", + "lease-less SSH shutdown survives provider unregister and re-drives the replacement transport" + ] + }, + { + "file": "src/main/ipc/runtime-terminal-close-retry-owner.test.ts", + "assertions": [ + "a late runtime close completion cannot delete a replacement-generation retry owner" ] }, { diff --git a/src/main/daemon/daemon-foreground-confirmation-protocol.test.ts b/src/main/daemon/daemon-foreground-confirmation-protocol.test.ts index 4fbb3047d6a..ce83d70a572 100644 --- a/src/main/daemon/daemon-foreground-confirmation-protocol.test.ts +++ b/src/main/daemon/daemon-foreground-confirmation-protocol.test.ts @@ -3,7 +3,7 @@ import { PREVIOUS_DAEMON_PROTOCOL_VERSIONS, PROTOCOL_VERSION } from './types' describe('foreground-confirmation daemon protocol', () => { it('rejects daemons from before the fresh-confirmation RPC', () => { - expect(PROTOCOL_VERSION).toBeGreaterThan(19) - expect(PREVIOUS_DAEMON_PROTOCOL_VERSIONS).toContain(19) + expect(PROTOCOL_VERSION).toBeGreaterThan(21) + expect(PREVIOUS_DAEMON_PROTOCOL_VERSIONS).toContain(21) }) }) diff --git a/src/main/daemon/daemon-pty-adapter.test.ts b/src/main/daemon/daemon-pty-adapter.test.ts index 06a77789f4b..d7e5560e4e3 100644 --- a/src/main/daemon/daemon-pty-adapter.test.ts +++ b/src/main/daemon/daemon-pty-adapter.test.ts @@ -377,6 +377,30 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => { }) describe('shutdown', () => { + it('carries pane identity through the daemon and rejects a stale kill', async () => { + const { id } = await adapter.spawn({ + cols: 80, + rows: 24, + env: { ORCA_PANE_KEY: 'tab-b:leaf-b', ORCA_TAB_ID: 'tab-b' } + }) + + await expect( + adapter.shutdown(id, { + immediate: true, + expectedPaneKey: 'tab-a:leaf-a', + expectedTabId: 'tab-a' + }) + ).rejects.toThrow('PTY identity mismatch') + expect(lastSubprocess.forceKill).not.toHaveBeenCalled() + expect(await adapter.listProcesses()).toHaveLength(1) + + await adapter.shutdown(id, { + immediate: true, + expectedPaneKey: 'tab-b:leaf-b', + expectedTabId: 'tab-b' + }) + }) + it('kills the session', async () => { const { id } = await adapter.spawn({ cols: 80, rows: 24 }) await adapter.shutdown(id, { immediate: false }) diff --git a/src/main/daemon/daemon-pty-adapter.ts b/src/main/daemon/daemon-pty-adapter.ts index e0d5ec598ca..be38e03ce4b 100644 --- a/src/main/daemon/daemon-pty-adapter.ts +++ b/src/main/daemon/daemon-pty-adapter.ts @@ -24,6 +24,7 @@ import type { PtyBackgroundStreamEvent, PtyProviderBufferSnapshot, PtyProcessInfo, + PtyShutdownOptions, PtySpawnOptions, PtySpawnResult } from '../providers/types' @@ -31,6 +32,11 @@ import { isShellProcess } from '../../shared/agent-detection' import { recognizeAgentProcessFromCommandLine } from '../../shared/agent-process-recognition' import { shouldUseShellReadyStartupDelivery } from '../../shared/codex-startup-delivery' import type { TerminalOscLinkRange } from '../../shared/terminal-osc-link-ranges' +import { + assertPtyPaneIdentity, + getPtyPaneIdentityFromEnv, + type PtyPaneIdentity +} from '../pty/pty-shutdown-identity' type ColdRestorePayload = { scrollback: string @@ -102,6 +108,7 @@ export class DaemonPtyAdapter implements IPtyProvider { private coldRestoreCache = new Map() private sleepRestoreSessionIds = new Set() private activeSessionIds = new Set() + private paneIdentityBySessionId = new Map() private dirtySessionVersions = new Map() // Why: a cold-restored session is a fresh shell whose on-disk checkpoint and // log belong to the pre-crash session. Incremental appends would land on @@ -168,16 +175,35 @@ export class DaemonPtyAdapter implements IPtyProvider { return this.historyManager } + private rememberSessionIdentity(session: SessionInfo): void { + if (session.paneKey || session.tabId) { + this.paneIdentityBySessionId.set(session.sessionId, { + paneKey: session.paneKey ?? null, + tabId: session.tabId ?? null + }) + } + } + async spawn(opts: PtySpawnOptions): Promise { return this.withDaemonRetry(() => this.doSpawn(opts)) } private async doSpawn(opts: PtySpawnOptions): Promise { const sessionId = opts.sessionId ?? mintPtySessionId(opts.worktreeId) + const requestedIdentity = getPtyPaneIdentityFromEnv(opts.env) if (this.killedSessionTombstones.has(sessionId)) { throw new TerminalKilledError(sessionId) } + if (this.protocolVersion < 22 && this.activeSessionIds.has(sessionId)) { + const knownIdentity = this.paneIdentityBySessionId.get(sessionId) + if (knownIdentity) { + assertPtyPaneIdentity(sessionId, knownIdentity, { + expectedPaneKey: requestedIdentity.paneKey ?? undefined, + expectedTabId: requestedIdentity.tabId ?? undefined + }) + } + } if (opts.isNewSession) { await this.replaceUnhealthyMacResolverDaemonBeforeNewPty() @@ -238,6 +264,8 @@ export class DaemonPtyAdapter implements IPtyProvider { command: opts.command, startupCommandDelivery: opts.startupCommandDelivery, launchAgent: opts.launchAgent, + ...(requestedIdentity.paneKey ? { paneKey: requestedIdentity.paneKey } : {}), + ...(requestedIdentity.tabId ? { tabId: requestedIdentity.tabId } : {}), // Why: without this, the daemon always spawns cmd.exe (COMSPEC) or // PowerShell as a fallback — regardless of which shell the renderer // asked for in the "+" menu or persisted as the default. Forwarding @@ -253,6 +281,7 @@ export class DaemonPtyAdapter implements IPtyProvider { let scrollback = restoreInfo ? getRecoveredHistorySeed(restoreInfo) : null let result = await createOrAttach(scrollback) + this.paneIdentityBySessionId.set(sessionId, requestedIdentity) const launchIdentity = (): { launchAgent?: NonNullable } => result.launchAgent ? { launchAgent: result.launchAgent } : {} @@ -485,7 +514,19 @@ export class DaemonPtyAdapter implements IPtyProvider { this.client.notify('setSessionBackground', { sessionId: id, background: safeBackground }) } - async shutdown(id: string, opts: { immediate?: boolean; keepHistory?: boolean }): Promise { + async shutdown(id: string, opts: PtyShutdownOptions): Promise { + if ( + this.protocolVersion < 22 && + (opts.expectedPaneKey !== undefined || opts.expectedTabId !== undefined) + ) { + // Why: preserved pre-v22 daemons ignore kill identity fields. Fail + // closed unless this main process observed the exact attached pane. + assertPtyPaneIdentity( + id, + this.paneIdentityBySessionId.get(id) ?? { paneKey: null, tabId: null }, + opts + ) + } // Why: sleep/exact-stop kills the live PTY before the periodic checkpoint may run. // Force a final snapshot so wake can restore the pane users left. if (opts.keepHistory) { @@ -500,12 +541,18 @@ export class DaemonPtyAdapter implements IPtyProvider { this.sleepRestoreSessionIds.add(id) } } - await this.client.request('kill', { sessionId: id, immediate: opts.immediate ?? false }) + await this.client.request('kill', { + sessionId: id, + immediate: opts.immediate ?? false, + expectedPaneKey: opts.expectedPaneKey, + expectedTabId: opts.expectedTabId + }) this.activeSessionIds.delete(id) this.dirtySessionVersions.delete(id) if (!opts.keepHistory) { this.coldRestoreCache.delete(id) this.sleepRestoreSessionIds.delete(id) + this.paneIdentityBySessionId.delete(id) } // Why: the !keepHistory close path doesn't take a final checkpoint, so a // session stranded in sessionsNeedingFullCheckpoint would never be cleared. @@ -714,6 +761,7 @@ export class DaemonPtyAdapter implements IPtyProvider { const killed: string[] = [] for (const session of result.sessions) { + this.rememberSessionIdentity(session) if (!session.isAlive) { continue } @@ -746,6 +794,9 @@ export class DaemonPtyAdapter implements IPtyProvider { async listProcesses(): Promise { await this.ensureConnected() const result = await this.client.request('listSessions', undefined) + for (const session of result.sessions) { + this.rememberSessionIdentity(session) + } return result.sessions .filter((s) => s.isAlive) .map((s) => ({ @@ -763,6 +814,9 @@ export class DaemonPtyAdapter implements IPtyProvider { async listSessions(): Promise { await this.ensureConnected() const result = await this.client.request('listSessions', undefined) + for (const session of result.sessions) { + this.rememberSessionIdentity(session) + } return result.sessions.filter((s) => s.isAlive) } @@ -1326,6 +1380,7 @@ export class DaemonPtyAdapter implements IPtyProvider { this.backgroundedSessionIds.delete(event.sessionId) if (!this.sleepRestoreSessionIds.has(event.sessionId)) { this.coldRestoreCache.delete(event.sessionId) + this.paneIdentityBySessionId.delete(event.sessionId) } // Why: an exited session can never be checkpointed again, so its pending // full-checkpoint flag is dead state. Without this, a cold-restored diff --git a/src/main/daemon/daemon-pty-provider.ts b/src/main/daemon/daemon-pty-provider.ts index 7c569f755b3..b24cd3a4f94 100644 --- a/src/main/daemon/daemon-pty-provider.ts +++ b/src/main/daemon/daemon-pty-provider.ts @@ -1,5 +1,7 @@ import { DaemonClient } from './client' import type { DaemonEvent, CreateOrAttachResult } from './types' +import type { PtyShutdownOptions } from '../providers/types' +import { getPtyPaneIdentityFromEnv } from '../pty/pty-shutdown-identity' export type DaemonPtyProviderOptions = { socketPath: string @@ -38,6 +40,7 @@ export class DaemonPtyProvider { async spawn(opts: DaemonSpawnOptions): Promise { await this.client.ensureConnected() this.setupEventRouting() + const identity = getPtyPaneIdentityFromEnv(opts.env) const result = await this.client.request('createOrAttach', { sessionId: opts.sessionId, @@ -46,7 +49,9 @@ export class DaemonPtyProvider { cwd: opts.cwd, env: opts.env, envToDelete: opts.envToDelete, - command: opts.command + command: opts.command, + ...(identity.paneKey ? { paneKey: identity.paneKey } : {}), + ...(identity.tabId ? { tabId: identity.tabId } : {}) }) return { @@ -64,8 +69,13 @@ export class DaemonPtyProvider { this.client.notify('resize', { sessionId: id, cols, rows }) } - async shutdown(id: string, opts: { immediate?: boolean; keepHistory?: boolean }): Promise { - await this.client.request('kill', { sessionId: id, immediate: opts.immediate ?? false }) + async shutdown(id: string, opts: PtyShutdownOptions): Promise { + await this.client.request('kill', { + sessionId: id, + immediate: opts.immediate ?? false, + expectedPaneKey: opts.expectedPaneKey, + expectedTabId: opts.expectedTabId + }) } onData(callback: (payload: { id: string; data: string }) => void): () => void { diff --git a/src/main/daemon/daemon-pty-router.ts b/src/main/daemon/daemon-pty-router.ts index 18bf0f926b5..4f94432a65b 100644 --- a/src/main/daemon/daemon-pty-router.ts +++ b/src/main/daemon/daemon-pty-router.ts @@ -4,6 +4,7 @@ import type { PtyBackgroundStreamEvent, PtyProviderBufferSnapshot, PtyProcessInfo, + PtyShutdownOptions, PtySpawnOptions, PtySpawnResult } from '../providers/types' @@ -94,7 +95,7 @@ export class DaemonPtyRouter implements IPtyProvider { this.adapterFor(id).setPtyBackgrounded(id, background) } - async shutdown(id: string, opts: { immediate?: boolean; keepHistory?: boolean }): Promise { + async shutdown(id: string, opts: PtyShutdownOptions): Promise { await this.adapterFor(id).shutdown(id, opts) // Why: sleep passes keepHistory=true and re-spawns against the same // sessionId on wake. If we delete the routing entry here, adapterFor() diff --git a/src/main/daemon/daemon-server.ts b/src/main/daemon/daemon-server.ts index fe87022e5c5..192a753b2ec 100644 --- a/src/main/daemon/daemon-server.ts +++ b/src/main/daemon/daemon-server.ts @@ -345,6 +345,8 @@ export class DaemonServer { // Why: daemon RPC payloads are untrusted JSON. Persist only the // allowlisted enum used for byte routing, never arbitrary identity. ...(isTuiAgent(p.launchAgent) ? { launchAgent: p.launchAgent } : {}), + ...(typeof p.paneKey === 'string' ? { paneKey: p.paneKey } : {}), + ...(typeof p.tabId === 'string' ? { tabId: p.tabId } : {}), shellOverride: p.shellOverride, terminalWindowsWslDistro: p.terminalWindowsWslDistro, terminalWindowsPowerShellImplementation: p.terminalWindowsPowerShellImplementation, @@ -498,7 +500,11 @@ export class DaemonServer { sessionId: request.payload.sessionId, immediate: request.payload.immediate === true }) - this.host.kill(request.payload.sessionId, { immediate: request.payload.immediate }) + this.host.kill(request.payload.sessionId, { + immediate: request.payload.immediate, + expectedPaneKey: request.payload.expectedPaneKey, + expectedTabId: request.payload.expectedTabId + }) return {} case 'signal': diff --git a/src/main/daemon/session.ts b/src/main/daemon/session.ts index b9c82a48174..5954eb1b106 100644 --- a/src/main/daemon/session.ts +++ b/src/main/daemon/session.ts @@ -76,6 +76,8 @@ export type SubprocessHandle = { export type SessionOptions = { sessionId: string + paneKey?: string + tabId?: string cols: number rows: number terminalHandle?: string @@ -101,6 +103,8 @@ type AttachedClient = { export class Session { readonly sessionId: string + readonly paneKey: string | null + readonly tabId: string | null readonly terminalHandle: string | null readonly launchAgent: TuiAgent | null private _state: SessionState = 'running' @@ -128,6 +132,8 @@ export class Session { constructor(opts: SessionOptions) { this.sessionId = opts.sessionId + this.paneKey = opts.paneKey ?? null + this.tabId = opts.tabId ?? null this.terminalHandle = opts.terminalHandle ?? null this.launchAgent = opts.launchAgent ?? null this.subprocess = opts.subprocess diff --git a/src/main/daemon/terminal-host-create-contract.ts b/src/main/daemon/terminal-host-create-contract.ts index a60570ac5fe..3fa99a108a5 100644 --- a/src/main/daemon/terminal-host-create-contract.ts +++ b/src/main/daemon/terminal-host-create-contract.ts @@ -12,6 +12,8 @@ export type CreateOrAttachOptions = { command?: string startupCommandDelivery?: StartupCommandDelivery launchAgent?: TuiAgent + paneKey?: string + tabId?: string /** Explicit shell the renderer asked for, forwarded to the subprocess. */ shellOverride?: string terminalWindowsWslDistro?: string | null diff --git a/src/main/daemon/terminal-host.test.ts b/src/main/daemon/terminal-host.test.ts index 46e495e5f2e..f488ff3877b 100644 --- a/src/main/daemon/terminal-host.test.ts +++ b/src/main/daemon/terminal-host.test.ts @@ -72,6 +72,29 @@ describe('TerminalHost', () => { }) describe('createOrAttach', () => { + it('rejects attaching a replacement pane to a reused live session id', async () => { + await host.createOrAttach({ + sessionId: 'session-1', + paneKey: 'tab-a:leaf-a', + tabId: 'tab-a', + cols: 80, + rows: 24, + streamClient: { onData: vi.fn(), onExit: vi.fn() } + }) + + await expect( + host.createOrAttach({ + sessionId: 'session-1', + paneKey: 'tab-b:leaf-b', + tabId: 'tab-b', + cols: 80, + rows: 24, + streamClient: { onData: vi.fn(), onExit: vi.fn() } + }) + ).rejects.toThrow('PTY identity mismatch') + expect(spawnFn).toHaveBeenCalledOnce() + }) + it('creates a new session when none exists', async () => { const result = await host.createOrAttach({ sessionId: 'session-1', @@ -361,6 +384,27 @@ describe('TerminalHost', () => { }) describe('kill', () => { + it('rejects a stale pane kill before force-killing the current session', async () => { + await host.createOrAttach({ + sessionId: 'session-1', + paneKey: 'tab-b:leaf-b', + tabId: 'tab-b', + cols: 80, + rows: 24, + streamClient: { onData: vi.fn(), onExit: vi.fn() } + }) + + expect(() => + host.kill('session-1', { + immediate: true, + expectedPaneKey: 'tab-a:leaf-a', + expectedTabId: 'tab-a' + }) + ).toThrow('PTY identity mismatch') + expect(lastSubprocess.forceKill).not.toHaveBeenCalled() + expect(host.listSessions()).toHaveLength(1) + }) + it('kills the session and tombstones it', async () => { await host.createOrAttach({ sessionId: 'session-1', diff --git a/src/main/daemon/terminal-host.ts b/src/main/daemon/terminal-host.ts index b15420fed14..157ab6e63b0 100644 --- a/src/main/daemon/terminal-host.ts +++ b/src/main/daemon/terminal-host.ts @@ -7,6 +7,7 @@ import { buildStartupCommandSubmission } from '../../shared/startup-command-subm import type { SessionInfo, TakePendingOutputResult, TerminalSnapshot } from './types' import { SessionNotFoundError } from './types' import type { CreateOrAttachOptions, CreateOrAttachResult } from './terminal-host-create-contract' +import { assertPtyPaneIdentity } from '../pty/pty-shutdown-identity' export type { CreateOrAttachOptions, CreateOrAttachResult } from './terminal-host-create-contract' @@ -67,6 +68,11 @@ export class TerminalHost { // subsequent operation (write/kill/resize) would fail once the subprocess // finally exits. Treat terminating sessions the same as fully-exited ones. if (existing && existing.isAlive && !existing.isTerminating) { + assertPtyPaneIdentity( + opts.sessionId, + { paneKey: existing.paneKey, tabId: existing.tabId }, + { expectedPaneKey: opts.paneKey, expectedTabId: opts.tabId } + ) const snapshot = existing.getSnapshot() existing.detachAllClients() const token = existing.attachClient(opts.streamClient) @@ -117,6 +123,8 @@ export class TerminalHost { const session = new Session({ sessionId: opts.sessionId, + paneKey: opts.paneKey, + tabId: opts.tabId, cols: size.cols, rows: size.rows, terminalHandle: opts.env?.ORCA_TERMINAL_HANDLE, @@ -193,8 +201,12 @@ export class TerminalHost { this.sessions.get(sessionId)?.resumeProducer() } - kill(sessionId: string, opts: { immediate?: boolean } = {}): void { + kill( + sessionId: string, + opts: { immediate?: boolean; expectedPaneKey?: string; expectedTabId?: string } = {} + ): void { const session = this.getAliveSession(sessionId) + assertPtyPaneIdentity(sessionId, { paneKey: session.paneKey, tabId: session.tabId }, opts) this.recordTombstone(sessionId) if (opts.immediate) { session.forceKillAndDisposeSubprocess() @@ -326,6 +338,8 @@ export class TerminalHost { const size = session.getAppliedSize() result.push({ sessionId: session.sessionId, + ...(session.paneKey ? { paneKey: session.paneKey } : {}), + ...(session.tabId ? { tabId: session.tabId } : {}), state: session.state, shellState: session.shellState, isAlive: true, diff --git a/src/main/daemon/types.ts b/src/main/daemon/types.ts index 550b211aa74..7941bf1ba52 100644 --- a/src/main/daemon/types.ts +++ b/src/main/daemon/types.ts @@ -12,13 +12,12 @@ export type { // ─── Protocol Version ──────────────────────────────────────────────── import type { StartupCommandDelivery } from '../../shared/codex-startup-delivery' import type { TuiAgent } from '../../shared/types' -// Why: daemons can survive app updates. Bump for IPC wire-shape changes, or -// when daemon-baked behavior cannot be delivered by on-disk wrapper refresh. -// Why: bump when adding daemon wire behavior so same-version old daemons do -// not silently accept the handshake and then reject new RPCs. -export const PROTOCOL_VERSION = 21 +import type * as PtyIdentity from '../pty/pty-shutdown-identity' +// Why: daemons survive app updates; bump for wire changes so old daemons do +// not accept handshakes for unsupported RPC behavior. +export const PROTOCOL_VERSION = 22 export const PREVIOUS_DAEMON_PROTOCOL_VERSIONS = [ - 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20 + 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21 ] as const // ─── Session State Machine ────────────────────────────────────────── @@ -98,7 +97,7 @@ export type HelloResponse = { export type CreateOrAttachRequest = { id: string type: 'createOrAttach' - payload: { + payload: PtyIdentity.PtyPaneIdentityFields & { sessionId: string cols: number rows: number @@ -190,7 +189,7 @@ export type SetSessionBackgroundRequest = { export type KillRequest = { id: string type: 'kill' - payload: { + payload: PtyIdentity.PtyExpectedPaneIdentityFields & { sessionId: string immediate?: boolean } @@ -380,7 +379,7 @@ export type SystemResolverHealthResult = { health: SystemResolverHealth } -export type SessionInfo = { +export type SessionInfo = PtyIdentity.PtyPaneIdentityFields & { sessionId: string state: SessionState shellState: ShellReadyState diff --git a/src/main/ipc/pty.test.ts b/src/main/ipc/pty.test.ts index ad3b0f6aa1d..95ddf777b62 100644 --- a/src/main/ipc/pty.test.ts +++ b/src/main/ipc/pty.test.ts @@ -202,6 +202,7 @@ import { setLocalPtyProvider, rebindLocalProviderListeners, unregisterSshPtyProvider, + releasePendingSshShutdownsForTarget, getLocalPtyProvider } from './pty' import { @@ -438,6 +439,7 @@ describe('registerPtyHandlers', () => { 'ssh-runtime-env' ]) { unregisterSshPtyProvider(leakedConnectionId) + releasePendingSshShutdownsForTarget(leakedConnectionId) } setLocalPtyProvider(new LocalPtyProvider()) if (savedProcessPlatform) { @@ -3651,6 +3653,53 @@ describe('registerPtyHandlers', () => { vi.useRealTimers() }) + it('re-drives a lease-less SSH kill after provider unregister and replacement', async () => { + vi.useFakeTimers() + const oldShutdown = makeDeferred() + const oldProviderShutdown = vi.fn(() => oldShutdown.promise) + const newProviderShutdown = vi.fn().mockResolvedValue(undefined) + const store = { + getSshRemotePtyLeases: vi.fn(() => []), + markSshRemotePtyShutdownRequested: vi.fn(() => false), + markSshRemotePtyLease: vi.fn() + } + handlers.clear() + registerPtyHandlers( + mainWindow as never, + undefined, + undefined, + undefined, + undefined, + store as never + ) + registerSshPtyProvider('ssh-1', { + shutdown: oldProviderShutdown, + onExit: vi.fn(() => () => {}) + } as never) + const id = 'ssh:ssh-1@@boot-1@@lease-less-pty' + + const kill = handlers.get('pty:kill')!(null, { + id, + expectedPaneKey: 'tab-a:leaf-a', + expectedTabId: 'tab-a' + }) as Promise + expect(oldProviderShutdown).toHaveBeenCalledOnce() + unregisterSshPtyProvider('ssh-1') + registerSshPtyProvider('ssh-1', { + shutdown: newProviderShutdown, + onExit: vi.fn(() => () => {}) + } as never) + oldShutdown.resolve() + + await expect(kill).resolves.toBeUndefined() + expect(newProviderShutdown).toHaveBeenCalledOnce() + expect(store.markSshRemotePtyLease).toHaveBeenCalledWith('ssh-1', id, 'terminated') + unregisterSshPtyProvider('ssh-1') + await vi.advanceTimersByTimeAsync(30_000) + expect(vi.getTimerCount()).toBe(0) + vi.useRealTimers() + }) + it('rebinds a persisted local shutdown after async daemon startup replaces the provider', async () => { vi.useFakeTimers() const startup = makeDeferred() diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index 8cadfbc63fe..d8981da95f6 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -180,6 +180,7 @@ type RetainedPtyShutdown = { options: PtyShutdownOptions provider: IPtyProvider | null store: Store | null + durable: boolean attempts: number nextRetryAt: number inFlight: Promise | null @@ -759,6 +760,7 @@ function retainLocalPtyShutdown( options, provider: null, store: activePtyStore, + durable: true, attempts, nextRetryAt: deferUntilProviderReady ? Number.POSITIVE_INFINITY @@ -804,6 +806,7 @@ function retainSshPtyShutdown( options, provider, store: activePtyStore, + durable: persisted, attempts: 0, nextRetryAt: provider ? 0 : Number.POSITIVE_INFINITY, inFlight: null, @@ -811,6 +814,7 @@ function retainSshPtyShutdown( } pendingSshShutdownRetries.set(id, retained) } else { + retained.durable ||= persisted retained.options = mergePtyShutdownOptions(retained.options, options) if (retained.provider !== provider) { retained.provider = provider @@ -1341,15 +1345,31 @@ function routesFreshSpawnsToLocalProvider( /** Register an SSH PTY provider for a connection. */ export function registerSshPtyProvider(connectionId: string, provider: IPtyProvider): void { sshProviders.set(connectionId, provider) + // Durable leases reconcile generation first; a stale boot-qualified owner + // must be dropped before any replacement provider can observe it. retryPersistedSshShutdowns(connectionId, provider) + for (const retained of pendingSshShutdownRetries.values()) { + if (retained.connectionId !== connectionId || retained.durable) { + continue + } + retained.provider = provider + retained.nextRetryAt = 0 + if (!retained.inFlight) { + void attemptRetainedPtyShutdown(retained) + } + } + scheduleRetainedShutdownRetries() } /** Remove an SSH PTY provider when a connection is closed. */ export function unregisterSshPtyProvider(connectionId: string): void { sshProviders.delete(connectionId) - for (const [id, retained] of pendingSshShutdownRetries) { + for (const retained of pendingSshShutdownRetries.values()) { if (retained.connectionId === connectionId) { - pendingSshShutdownRetries.delete(id) + // Why: provider teardown is often a reconnect, not proof the remote PTY + // exited. Keep even lease-less accepted kills for the replacement owner. + retained.provider = null + retained.nextRetryAt = Number.POSITIVE_INFINITY } } scheduleRetainedShutdownRetries() @@ -1386,7 +1406,7 @@ function retryPersistedSshShutdowns(connectionId: string, provider: IPtyProvider retainedLeases.map((lease) => toAppSshPtyId(connectionId, lease.ptyId, lease.relayInstanceId)) ) for (const [id, retained] of pendingSshShutdownRetries) { - if (retained.connectionId === connectionId && !retainedIds.has(id)) { + if (retained.connectionId === connectionId && retained.durable && !retainedIds.has(id)) { pendingSshShutdownRetries.delete(id) } } @@ -1406,6 +1426,7 @@ function retryPersistedSshShutdowns(connectionId: string, provider: IPtyProvider connectionId, provider, store, + durable: true, attempts: 0, nextRetryAt: 0, inFlight: null, @@ -1414,6 +1435,7 @@ function retryPersistedSshShutdowns(connectionId: string, provider: IPtyProvider } pendingSshShutdownRetries.set(id, retained) } else { + retained.durable = true retained.options = mergePtyShutdownOptions(retained.options, options) if (retained.provider !== provider) { // Why: reconnect replaces the transport owner for the same generation-qualified id. diff --git a/src/main/providers/local-pty-provider.test.ts b/src/main/providers/local-pty-provider.test.ts index 6cf14f593fc..715c2670ddb 100644 --- a/src/main/providers/local-pty-provider.test.ts +++ b/src/main/providers/local-pty-provider.test.ts @@ -942,6 +942,31 @@ describe('LocalPtyProvider', () => { }) describe('shutdown', () => { + it('rejects a stale pane identity before killing the local PTY', async () => { + const killSpy = mockProc.kill + const { id } = await provider.spawn({ + cols: 80, + rows: 24, + env: { ORCA_PANE_KEY: 'tab-b:leaf-b', ORCA_TAB_ID: 'tab-b' } + }) + + await expect( + provider.shutdown(id, { + immediate: true, + expectedPaneKey: 'tab-a:leaf-a', + expectedTabId: 'tab-a' + }) + ).rejects.toThrow('PTY identity mismatch') + expect(killSpy).not.toHaveBeenCalled() + expect(provider.hasPty(id)).toBe(true) + + await provider.shutdown(id, { + immediate: true, + expectedPaneKey: 'tab-b:leaf-b', + expectedTabId: 'tab-b' + }) + }) + it('kills the PTY process', async () => { // Why: capture the spy reference before shutdown triggers onExit → // POSIX kill neutralization. After neutralization, mockProc.kill is diff --git a/src/main/providers/local-pty-provider.ts b/src/main/providers/local-pty-provider.ts index e8231d11c20..4bb6ced28a3 100644 --- a/src/main/providers/local-pty-provider.ts +++ b/src/main/providers/local-pty-provider.ts @@ -64,6 +64,11 @@ import { readWindowsConptyProcessIds } from './windows-conpty-process-membership import { shouldUseShellReadyStartupDelivery } from '../../shared/codex-startup-delivery' import { assertSafeAgentStartupCwd, resolveSafePtyDefaultCwd } from './pty-default-cwd' import { ORCA_HERMES_STARTUP_QUERY_ENV } from '../../shared/hermes-startup-query' +import { + assertPtyPaneIdentity, + getPtyPaneIdentityFromEnv, + type PtyPaneIdentity +} from '../pty/pty-shutdown-identity' const PANE_IDENTITY_ENV_KEYS = [ 'ORCA_PANE_KEY', @@ -77,6 +82,7 @@ const ptyProcesses = new Map() const ptyShellName = new Map() const ptyAgentForegroundContextPaths = new Map() const ptyTerminalHandle = new Map() +const ptyPaneIdentity = new Map() // Why: node-pty's onData/onExit register native NAPI ThreadSafeFunction // callbacks. If the PTY is killed without disposing these listeners, the // stale callbacks survive into node::FreeEnvironment() where NAPI attempts @@ -188,6 +194,7 @@ function clearPtyState(id: string): void { ptyShellName.delete(id) ptyAgentForegroundContextPaths.delete(id) ptyTerminalHandle.delete(id) + ptyPaneIdentity.delete(id) ptyLoadGeneration.delete(id) ptyShutdownInProgress.delete(id) ptyExitDuringShutdown.delete(id) @@ -696,6 +703,7 @@ export class LocalPtyProvider implements IPtyProvider { const proc = spawnResult.process ptyProcesses.set(id, proc) + ptyPaneIdentity.set(id, getPtyPaneIdentityFromEnv(finalEnv)) ptyShellName.set(id, getSpawnedShellName(shellPath)) if (finalEnv.ORCA_TERMINAL_HANDLE) { ptyTerminalHandle.set(id, finalEnv.ORCA_TERMINAL_HANDLE) @@ -897,11 +905,12 @@ export class LocalPtyProvider implements IPtyProvider { return { cols: proc.cols, rows: proc.rows } } - async shutdown(id: string, _opts: PtyShutdownOptions): Promise { + async shutdown(id: string, opts: PtyShutdownOptions): Promise { const proc = ptyProcesses.get(id) if (!proc) { return } + assertPtyPaneIdentity(id, ptyPaneIdentity.get(id) ?? { paneKey: null, tabId: null }, opts) // Why: keep listeners live until native shutdown is accepted so a thrown // kill retains retry ownership. The in-progress marker makes a synchronous // onExit an independent death proof without racing duplicate cleanup. diff --git a/src/main/pty/pty-shutdown-identity.ts b/src/main/pty/pty-shutdown-identity.ts new file mode 100644 index 00000000000..76f52dfaf84 --- /dev/null +++ b/src/main/pty/pty-shutdown-identity.ts @@ -0,0 +1,34 @@ +import type { PtyShutdownOptions } from '../providers/types' + +export type PtyPaneIdentity = { + paneKey: string | null + tabId: string | null +} + +export type PtyPaneIdentityFields = { paneKey?: string; tabId?: string } +export type PtyExpectedPaneIdentityFields = Pick< + PtyShutdownOptions, + 'expectedPaneKey' | 'expectedTabId' +> + +export function getPtyPaneIdentityFromEnv( + env: Record | undefined +): PtyPaneIdentity { + return { + paneKey: env?.ORCA_PANE_KEY?.trim() || null, + tabId: env?.ORCA_TAB_ID?.trim() || null + } +} + +export function assertPtyPaneIdentity( + id: string, + actual: PtyPaneIdentity, + expected: PtyExpectedPaneIdentityFields +): void { + if ( + (expected.expectedPaneKey !== undefined && actual.paneKey !== expected.expectedPaneKey) || + (expected.expectedTabId !== undefined && actual.tabId !== expected.expectedTabId) + ) { + throw new Error(`PTY identity mismatch for "${id}"`) + } +} diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 56d25285c1e..4bfd8ff299c 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -22624,6 +22624,51 @@ describe('OrcaRuntimeService', () => { expect(spawn).not.toHaveBeenCalled() }) + it('allows workspace resolution on an unrelated host during host-scoped removal', async () => { + const runtime = new OrcaRuntimeService(store) + const runtimeWithResolver = runtime as unknown as { + resolveTerminalWorkspaceLaunchScope: () => Promise<{ + id: string + path: string + hostId: 'ssh:target-1' + connectionId: 'target-1' + repo: ReturnType + folderWorkspace: null + }> + } + vi.spyOn(runtimeWithResolver, 'resolveTerminalWorkspaceLaunchScope').mockResolvedValue({ + id: TEST_WORKTREE_ID, + path: '/home/dev/orca', + hostId: 'ssh:target-1', + connectionId: 'target-1', + repo: store.getRepo(TEST_REPO_ID), + folderWorkspace: null + }) + const spawn = vi.fn().mockResolvedValue({ id: 'ssh-pty' }) + runtime.setPtyController({ + spawn, + write: () => true, + kill: () => false, + getForegroundProcess: async () => null, + listProcesses: async () => [] + }) + const holdRemoval = deferred() + const removal = runtime.runWithWorktreeRemovalAdmission( + TEST_WORKTREE_ID, + () => holdRemoval.promise, + 'local' + ) + await Promise.resolve() + + await expect(runtime.createTerminal(`id:${TEST_WORKTREE_ID}`)).resolves.toMatchObject({ + ptyId: 'ssh-pty' + }) + expect(spawn).toHaveBeenCalledWith(expect.objectContaining({ connectionId: 'target-1' })) + + holdRemoval.resolve() + await removal + }) + it('drains a create-first managed worktree before project removal snapshots ownership', async () => { const removeProject = vi.fn() const runtime = new OrcaRuntimeService({ ...store, removeProject } as never) diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 537232cfe07..45e41381eba 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -12351,14 +12351,35 @@ export class OrcaRuntimeService { ): Promise<{ workspace: TerminalWorkspaceLaunchScope; release: () => void }> { // Why: workspace resolution can await SSH/runtime discovery. Keep removal // aware of the unresolved create, then transfer ownership without a gap. - const releaseResolutionAdmission = this.beginTerminalAdmission(GLOBAL_TERMINAL_ADMISSION_KEY) + let releaseResolutionAdmission: (() => void) | null = null + let removalsActiveAtResolutionStart: ReadonlySet | null = null + try { + releaseResolutionAdmission = this.beginTerminalAdmission(GLOBAL_TERMINAL_ADMISSION_KEY) + } catch (error) { + if ( + !(error instanceof Error) || + error.message !== 'terminal_admission_blocked_for_project_removal' + ) { + throw error + } + // Why: an unresolved selector might belong to another execution host. + // Resolve it, but remember the active removals so a fast completion + // cannot reopen the exact stale scope before the atomic transfer. + removalsActiveAtResolutionStart = new Set(this.terminalAdmissionBlockedWorktreeIds) + } try { const workspace = await this.resolveTerminalWorkspaceLaunchScope(selector, hostId) + if ( + removalsActiveAtResolutionStart && + this.isAdmissionBlocked(removalsActiveAtResolutionStart, workspace.id, workspace.hostId) + ) { + throw new Error('terminal_admission_blocked_for_project_removal') + } const releaseWorkspaceAdmission = this.beginTerminalAdmission(workspace.id, workspace.hostId) - releaseResolutionAdmission() + releaseResolutionAdmission?.() return { workspace, release: releaseWorkspaceAdmission } } catch (error) { - releaseResolutionAdmission() + releaseResolutionAdmission?.() throw error } }