From 9b2adf4e9707b407deeb8699da13bbbdf71b6809 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Thu, 15 Oct 2026 23:13:43 -0700 Subject: [PATCH] fix(runtime): route a CLI close of a paired host's terminal to that host (#29455) * fix(runtime): route a CLI close of a paired host's terminal to that host orca terminal list on a desktop lists terminals a paired runtime runs, but orca terminal close on one of those handles stopped the remote PTY id on this machine, which cannot own it, and answered terminal_stop_unverifiable while the process kept running on the host. closeTerminal and closeTerminalTab now send the close to the owning runtime first and return its receipt; when that host is unreachable the close fails and the terminal stays visible. * fix(runtime): revalidate the pane after a paired-host close, and read the owner's stale-handle answer Codex review: a pane rebound while the host call was pending had its new occupant retired, and an owner that no longer knows the handle answers terminal_handle_stale, which rejected instead of retiring the mirror. * fix(runtime): refuse a pinned paired-host tab before asking its host to close it Codex review: the renderer's pin guard runs at the local commit, after the host had already stopped the process. --- .../paired-host-terminal-close.test.ts | 256 ++++++++++++++++++ .../explicit-terminal-close-stops.ts | 46 +++- .../paired-host-terminal-close.ts | 146 ++++++++++ .../workspace-session-terminal-tab-close.ts | 2 +- 4 files changed, 448 insertions(+), 2 deletions(-) create mode 100644 src/main/runtime/paired-host-terminal-close.test.ts create mode 100644 src/main/runtime/session-tabs/paired-host-terminal-close.ts diff --git a/src/main/runtime/paired-host-terminal-close.test.ts b/src/main/runtime/paired-host-terminal-close.test.ts new file mode 100644 index 00000000000..d59f964acbd --- /dev/null +++ b/src/main/runtime/paired-host-terminal-close.test.ts @@ -0,0 +1,256 @@ +// STA-7826: a CLI close of a terminal a paired runtime runs must reach that runtime, the only +// machine that can stop its process, instead of "stopping" the remote id on this machine. +import { describe, expect, it, vi } from 'vitest' +import { toRemoteRuntimePtyId } from '../../shared/remote-runtime-pty-id' +import { OrchestrationError } from './orchestration/orchestration-error' +import { getDefaultWorkspaceSession } from '../../shared/constants' + +const { OrcaRuntimeService } = await import('./orca-runtime-test-mocks.spec') +await import('./orca-runtime-test-lifecycle.spec') +const { store, makeRuntimeStoreWithWorkspaceSession } = + await import('./orca-runtime-test-fixtures.spec') + +const WORKTREE_ID = 'repo-1::/tmp/worktree-a' + +type HostCall = (environmentId: string, method: string, params: unknown) => Promise + +function pairedHostLeafRuntime(hostCall: HostCall, runtimeStore: typeof store = store) { + const runtime = new OrcaRuntimeService(runtimeStore) + const localStops: string[] = [] + runtime.setPtyController({ + write: () => true, + kill: (ptyId) => { + localStops.push(ptyId) + return false + }, + stopAndWait: async (ptyId) => { + localStops.push(ptyId) + return false + }, + getForegroundProcess: async () => null, + listProcesses: async () => [] + }) + const closeTerminalTab = vi.fn(async () => {}) + runtime.setNotifier({ + worktreesChanged: vi.fn(), + reposChanged: vi.fn(), + activateWorktree: vi.fn(), + createTerminal: vi.fn(), + splitTerminal: vi.fn(), + renameTerminal: vi.fn(), + focusTerminal: vi.fn(), + closeTerminal: vi.fn(), + closeTerminalTab, + sleepWorktree: vi.fn(), + terminalFitOverrideChanged: vi.fn(), + terminalDriverChanged: vi.fn() + }) + const call = vi.spyOn(runtime, 'callOrchestrationWorkerServer').mockImplementation(hostCall) + runtime.attachWindow(1) + runtime.syncWindowGraph(1, { + tabs: [ + { + tabId: 'tab-1', + worktreeId: WORKTREE_ID, + title: 'Shell', + activeLeafId: 'pane:1', + layout: null + } + ], + leaves: [ + { + tabId: 'tab-1', + worktreeId: WORKTREE_ID, + leafId: 'pane:1', + paneRuntimeId: 1, + ptyId: toRemoteRuntimePtyId('term_on_host', 'env-1') + } + ] + }) + return { runtime, call, closeTerminalTab, localStops } +} + +async function listedHandle(runtime: InstanceType): Promise { + const [terminal] = (await runtime.listTerminals()).terminals + expect(terminal?.executionHostId).toBe('runtime:env-1') + return terminal!.handle +} + +describe('closing a terminal that a paired runtime runs', () => { + it("returns the owning host's stop receipt and retires the local mirror", async () => { + const { runtime, call, closeTerminalTab, localStops } = pairedHostLeafRuntime(async () => ({ + close: { handle: 'term_on_host', tabId: 'host-tab', ptyKilled: true } + })) + const handle = await listedHandle(runtime) + + await expect(runtime.closeTerminal(handle)).resolves.toEqual({ + handle, + tabId: 'tab-1', + ptyKilled: true + }) + + expect(call).toHaveBeenCalledWith( + 'env-1', + 'terminal.close', + { terminal: 'term_on_host' }, + expect.any(Number) + ) + expect(closeTerminalTab).toHaveBeenCalledWith('tab-1', { + localPtyTeardownOwnedExternally: true + }) + // This machine cannot stop the paired host's process, so it must not pretend to. + expect(localStops).toEqual([]) + }) + + it("keeps the host's unconfirmed-stop verdict instead of minting its own", async () => { + const { runtime } = pairedHostLeafRuntime(async () => ({ + close: { + handle: 'term_on_host', + tabId: 'host-tab', + ptyKilled: false, + ptyStopVerdict: 'unverifiable', + ptyStopReason: 'its SSH host could not be reached', + pendingKillRecorded: true + } + })) + const handle = await listedHandle(runtime) + + await expect(runtime.closeTerminal(handle)).resolves.toEqual({ + handle, + tabId: 'tab-1', + ptyKilled: false, + ptyStopVerdict: 'unverifiable', + ptyStopReason: 'its SSH host could not be reached', + pendingKillRecorded: true + }) + }) + + it('leaves the terminal in place when the owning host cannot be reached', async () => { + const { runtime, closeTerminalTab, localStops } = pairedHostLeafRuntime(async () => { + throw new Error('connect ETIMEDOUT') + }) + const handle = await listedHandle(runtime) + + await expect(runtime.closeTerminal(handle)).rejects.toThrow('connect ETIMEDOUT') + + expect(closeTerminalTab).not.toHaveBeenCalled() + expect(localStops).toEqual([]) + expect((await runtime.listTerminals()).terminals.map((terminal) => terminal.handle)).toEqual([ + handle + ]) + }) + + it('retires the mirror of a terminal the owning host no longer knows, without claiming a stop', async () => { + // The owner is a real runtime that never issued this handle; the RPC layer passes its + // refusal code through, as `mapRuntimeError` does for `terminal_handle_stale`. + const owner = new OrcaRuntimeService(store) + owner.syncWindowGraph(1, { tabs: [], leaves: [] }) + const { runtime, closeTerminalTab } = pairedHostLeafRuntime(async (_env, _method, params) => { + const terminal = + typeof params === 'object' && params !== null && 'terminal' in params + ? String(params.terminal) + : '' + try { + return { close: await owner.closeTerminal(terminal) } + } catch (error) { + const message = error instanceof Error ? error.message : String(error) + throw new OrchestrationError(message, message) + } + }) + const handle = await listedHandle(runtime) + + await expect(runtime.closeTerminal(handle)).resolves.toEqual({ + handle, + tabId: 'tab-1', + ptyKilled: false, + ptyStopVerdict: 'unverifiable', + ptyStopReason: 'its owning host no longer recognizes this terminal handle' + }) + expect(closeTerminalTab).toHaveBeenCalledOnce() + }) + + it('refuses a pinned tab before the owning host stops anything', async () => { + const { runtimeStore } = makeRuntimeStoreWithWorkspaceSession({ + ...getDefaultWorkspaceSession(), + tabsByWorktree: { + [WORKTREE_ID]: [ + { + id: 'tab-1', + ptyId: toRemoteRuntimePtyId('term_on_host', 'env-1'), + worktreeId: WORKTREE_ID, + title: 'Shell', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 0, + isPinned: true + } + ] + } + }) + const { runtime, call } = pairedHostLeafRuntime( + async () => ({ close: { handle: 'term_on_host', tabId: 'host-tab', ptyKilled: true } }), + runtimeStore + ) + const handle = await listedHandle(runtime) + + await expect(runtime.closeTerminal(handle)).rejects.toThrow('terminal_tab_pinned') + await expect(runtime.closeTerminalTab(handle)).rejects.toThrow('terminal_tab_pinned') + expect(call).not.toHaveBeenCalled() + }) + + it('leaves a pane rebound during the host call to its new occupant', async () => { + let answerHost: ((value: unknown) => void) | null = null + const { runtime, closeTerminalTab } = pairedHostLeafRuntime( + () => + new Promise((resolve) => { + answerHost = resolve + }) + ) + const handle = await listedHandle(runtime) + + const closing = runtime.closeTerminal(handle) + await vi.waitFor(() => expect(answerHost).not.toBeNull()) + runtime.syncWindowGraph(1, { + tabs: [ + { + tabId: 'tab-1', + worktreeId: WORKTREE_ID, + title: 'Shell', + activeLeafId: 'pane:1', + layout: null + } + ], + leaves: [ + { + tabId: 'tab-1', + worktreeId: WORKTREE_ID, + leafId: 'pane:1', + paneRuntimeId: 1, + ptyId: toRemoteRuntimePtyId('term_replacement', 'env-1') + } + ] + }) + answerHost!({ close: { handle: 'term_on_host', tabId: 'host-tab', ptyKilled: true } }) + + await expect(closing).resolves.toEqual({ handle, tabId: 'tab-1', ptyKilled: true }) + expect(closeTerminalTab).not.toHaveBeenCalled() + }) + + it('routes a whole-tab close to the owning host too', async () => { + const { runtime, call } = pairedHostLeafRuntime(async () => ({ + close: { handle: 'term_on_host', tabId: 'host-tab', closeMode: 'tab', ptyKilled: false } + })) + const handle = await listedHandle(runtime) + + // This harness publishes no session snapshot, so the local tab commit after it answers + // tab_not_found; the host close must already have been sent. + await runtime.closeTerminalTab(handle).catch(() => undefined) + expect(call).toHaveBeenCalledWith( + 'env-1', + 'terminal.closeTab', + { terminal: 'term_on_host' }, + expect.any(Number) + ) + }) +}) diff --git a/src/main/runtime/session-tabs/explicit-terminal-close-stops.ts b/src/main/runtime/session-tabs/explicit-terminal-close-stops.ts index e74f00a1428..32c9fa839ea 100644 --- a/src/main/runtime/session-tabs/explicit-terminal-close-stops.ts +++ b/src/main/runtime/session-tabs/explicit-terminal-close-stops.ts @@ -6,6 +6,10 @@ import type { RuntimePtyTabCloseAuthority } from '../runtime-terminal-state-reco import { parsePaneKey } from '../../../shared/stable-pane-id' import { errorMessage } from '../../../shared/error-message' import type { ResolvedWorktree } from '../runtime-worktree-path-identity' +import { + closePairedHostLeafFirst, + isTerminalTabPinnedInSession +} from './paired-host-terminal-close' /** How an explicit close's stop of its addressed PTY ended. */ type ExplicitCloseStop = { stopped: boolean } @@ -107,6 +111,25 @@ export abstract class OrcaRuntimeWithStopExplicitlyClosedTabPtys extends OrcaRun return close } + /** null when this machine owns the PTY; see `closePairedHostLeafFirst` otherwise. */ + private closeOnOwningPairedHost( + handle: string, + leaf: { worktreeId: string; tabId: string; leafId: string; ptyId: string | null }, + method: 'terminal.close' | 'terminal.closeTab', + closesTab: boolean + ): ReturnType { + return closePairedHostLeafFirst(leaf, method, { + call: (environmentId, hostMethod, params, timeoutMs) => + this.callOrchestrationWorkerServer(environmentId, hostMethod, params, timeoutMs), + isTabPinned: () => + closesTab && + this.getWorkspaceSessionHostIdsForTab(leaf.worktreeId, leaf.tabId).some((hostId) => + isTerminalTabPinnedInSession(this.store?.getWorkspaceSession?.(hostId), leaf) + ), + readBoundLeaf: () => this.getLiveLeafForHandle(handle).leaf + }) + } + async closeTerminal(handle: string): Promise { const pty = this.getLivePtyForHandle(handle) this.claudeAgentTeams.removeTeamForLeaderHandle(handle) @@ -218,6 +241,15 @@ export abstract class OrcaRuntimeWithStopExplicitlyClosedTabPtys extends OrcaRun this.assertGraphReady() const { leaf } = this.getLiveLeafForHandle(handle) const paneTarget = { kind: 'pane' as const, tabId: leaf.tabId, leafId: leaf.leafId } + const hostClose = await this.closeOnOwningPairedHost( + handle, + leaf, + 'terminal.close', + this.resolveTerminalCloseTarget(leaf.worktreeId, paneTarget) === 'last-pane' + ) + if (hostClose && !hostClose.stillBound) { + return { handle, tabId: leaf.tabId, ...hostClose.close } + } const closesTab = this.resolveTerminalCloseTarget(leaf.worktreeId, paneTarget) === 'last-pane' const ptyIdsToKill = closesTab ? this.getPtyIdsForExplicitTabClose(leaf.worktreeId, leaf.tabId) @@ -227,6 +259,14 @@ export abstract class OrcaRuntimeWithStopExplicitlyClosedTabPtys extends OrcaRun if (closesTab && this.notifier?.closeTerminalTab) { await this.notifier.closeTerminalTab(leaf.tabId, { localPtyTeardownOwnedExternally: true }) } + if (hostClose) { + if (!closesTab) { + await this.closeTerminalPane(leaf.worktreeId, paneTarget) + } else if (!this.notifier?.closeTerminalTab) { + this.notifier?.closeTerminal(leaf.tabId) + } + return { handle, tabId: leaf.tabId, ...hostClose.close } + } const stop = leaf.ptyId ? await this.stopExplicitlyClosedTabPtys(ptyIdsToKill, leaf.ptyId) : NO_STOP @@ -264,8 +304,12 @@ export abstract class OrcaRuntimeWithStopExplicitlyClosedTabPtys extends OrcaRun } this.assertGraphReady() const { leaf } = this.getLiveLeafForHandle(handle) + const hostClose = await this.closeOnOwningPairedHost(handle, leaf, 'terminal.closeTab', true) + if (hostClose && !hostClose.stillBound) { + return { handle, tabId: leaf.tabId, ...hostClose.close, closeMode: 'tab' } + } await this.closeMobileSessionTab(`id:${leaf.worktreeId}`, leaf.tabId, { reason: 'user' }) this.claudeAgentTeams.removeTeamForLeaderHandle(handle) - return { handle, tabId: leaf.tabId, closeMode: 'tab', ptyKilled: false } + return { handle, tabId: leaf.tabId, ptyKilled: false, ...hostClose?.close, closeMode: 'tab' } } } diff --git a/src/main/runtime/session-tabs/paired-host-terminal-close.ts b/src/main/runtime/session-tabs/paired-host-terminal-close.ts new file mode 100644 index 00000000000..6cf1d7b6379 --- /dev/null +++ b/src/main/runtime/session-tabs/paired-host-terminal-close.ts @@ -0,0 +1,146 @@ +import { parseRemoteRuntimePtyId } from '../../../shared/remote-runtime-pty-id' +import type { RuntimeTerminalClose } from '../../../shared/runtime-types' +import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' +import { findUnifiedTerminalTabs } from '../../../shared/workspace-session-terminal-tab-close' + +const PAIRED_HOST_TERMINAL_CLOSE_TIMEOUT_MS = 15_000 + +export type PairedHostTerminal = { environmentId: string; handle: string } + +export type PairedHostTerminalCall = ( + environmentId: string, + method: 'terminal.close' | 'terminal.closeTab', + params: { terminal: string }, + timeoutMs: number +) => Promise + +/** The paired runtime that runs this PTY, or null when the PTY is not a paired runtime's. */ +export function getPairedHostTerminal(ptyId: string | null | undefined): PairedHostTerminal | null { + const parsed = ptyId ? parseRemoteRuntimePtyId(ptyId) : null + const environmentId = parsed?.environmentId?.trim() + return parsed && environmentId && parsed.handle ? { environmentId, handle: parsed.handle } : null +} + +function readHostClose(result: unknown): Omit | null { + const close = + typeof result === 'object' && result !== null && 'close' in result ? result.close : null + if (typeof close !== 'object' || close === null || !('ptyKilled' in close)) { + return null + } + if (typeof close.ptyKilled !== 'boolean') { + return null + } + const verdict = 'ptyStopVerdict' in close ? close.ptyStopVerdict : undefined + const reason = 'ptyStopReason' in close ? close.ptyStopReason : undefined + return { + ptyKilled: close.ptyKilled, + ...(verdict === 'live' || verdict === 'unverifiable' ? { ptyStopVerdict: verdict } : {}), + ...(typeof reason === 'string' ? { ptyStopReason: reason } : {}), + ...('pendingKillRecorded' in close && close.pendingKillRecorded === true + ? { pendingKillRecorded: true as const } + : {}) + } +} + +function errorCode(error: unknown): string | null { + return typeof error === 'object' && error !== null && 'code' in error + ? typeof error.code === 'string' + ? error.code + : null + : null +} + +/** + * Asks the paired runtime that owns the PTY to close it. This machine cannot stop that process, + * so the owner's receipt is the only evidence of the stop. A failure to reach the owner rejects, + * so the caller keeps its local mirror rather than hiding a terminal that may still run. + */ +export async function closeTerminalOnPairedHost( + call: PairedHostTerminalCall, + terminal: PairedHostTerminal, + method: 'terminal.close' | 'terminal.closeTab' +): Promise> { + let result: unknown + try { + result = await call( + terminal.environmentId, + method, + { terminal: terminal.handle }, + PAIRED_HOST_TERMINAL_CLOSE_TIMEOUT_MS + ) + } catch (error) { + const code = errorCode(error) + if (code === 'terminal_not_found') { + // The owner has no such terminal, so nothing runs there to stop. + return { ptyKilled: false } + } + if (code === 'terminal_handle_stale') { + // Why not absence: a host whose renderer reloaded answers this for a PTY that still runs. + return { + ptyKilled: false, + ptyStopVerdict: 'unverifiable', + ptyStopReason: 'its owning host no longer recognizes this terminal handle' + } + } + throw error + } + const close = readHostClose(result) + if (!close) { + // Why unverifiable: the owner answered, but not with a receipt this build can read. + return { + ptyKilled: false, + ptyStopVerdict: 'unverifiable', + ptyStopReason: 'its owning host returned no close receipt' + } + } + return close +} + +/** The pin guard `closeTerminalTabInWorkspaceSession` applies, for a caller that must check first. */ +export function isTerminalTabPinnedInSession( + session: WorkspaceSessionState | null | undefined, + tab: { worktreeId: string; tabId: string } +): boolean { + return session + ? session.tabsByWorktree[tab.worktreeId]?.find((row) => row.id === tab.tabId)?.isPinned === + true || + findUnifiedTerminalTabs(session, tab.worktreeId, tab.tabId).some((row) => row.isPinned) + : false +} + +type PairedHostLeaf = { tabId: string; leafId: string; ptyId: string | null } + +/** + * Closes a leaf's PTY on the paired runtime that owns it, before any local commit: only that host + * can stop the process, and a failure must reject while the terminal is still visible to retry. + * null when this machine owns the PTY. `stillBound` is false when the pane was rebound or removed + * during the host call, so its new occupant must not be retired by this close. + */ +export async function closePairedHostLeafFirst( + leaf: PairedHostLeaf, + method: 'terminal.close' | 'terminal.closeTab', + deps: { + call: PairedHostTerminalCall + /** The renderer's pin guard runs only at the local commit, after the host stopped it. */ + isTabPinned: () => boolean + readBoundLeaf: () => PairedHostLeaf + } +): Promise<{ close: Omit; stillBound: boolean } | null> { + const terminal = getPairedHostTerminal(leaf.ptyId) + if (!terminal) { + return null + } + if (deps.isTabPinned()) { + throw new Error('terminal_tab_pinned') + } + const close = await closeTerminalOnPairedHost(deps.call, terminal, method) + let stillBound = false + try { + const current = deps.readBoundLeaf() + stillBound = + current.tabId === leaf.tabId && current.leafId === leaf.leafId && current.ptyId === leaf.ptyId + } catch { + // The handle no longer resolves, so there is nothing local left to retire. + } + return { close, stillBound } +} diff --git a/src/shared/workspace-session-terminal-tab-close.ts b/src/shared/workspace-session-terminal-tab-close.ts index e7e8d297ae7..21dfe62f741 100644 --- a/src/shared/workspace-session-terminal-tab-close.ts +++ b/src/shared/workspace-session-terminal-tab-close.ts @@ -74,7 +74,7 @@ function collectTabPtyIds( return ids } -function findUnifiedTerminalTabs( +export function findUnifiedTerminalTabs( session: WorkspaceSessionState, worktreeId: string, tabId: string