From d8734fa5fd9f7ee0b7e25e9b986e4af1d437fac8 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Wed, 5 Aug 2026 04:33:39 -0400 Subject: [PATCH] fix(terminal): carry the rejected paired-runtime write into an actual remount (STA-2830) The WriteUnavailable signal reached the renderer and then died. `onWriteUnavailable` routed to `requestTerminalPaneRecovery` with `requireAuthoritativeLiveness: true`, which probes `pty:hasPty`. For a paired pane the id is `remote:@@`: `ptyOwnership` never holds it and `parseAppSshPtyId` ignores it, so the lookup falls through to the local provider, whose "not in my table" came back as an authoritative `false`. Recovery returned early and no remount ever happened. Two fixes, both about not inventing evidence: - `pty:hasPty` now returns null for a `remote:` id instead of routing it to the local provider, mirroring the guard `pty:kill` already has. Main cannot know another host's PTY state, so it says so. - A host rejection now takes a distinct recovery reason, `input-rejected-by-host`, that consults no local probe. The probe is not made to lie: the frame itself is the evidence, it came from the process that owns the PTY over a live connection, and the remount preserves the session rather than destroying it. Local ids keep the probe, where a `false` really does mean the shell died. Tests: a cross-boundary contract runs the real dispatcher, multiplexer, transport and pty-connection together and requires the remount for every answer the probe can give; an un-negotiated legacy binary subscriber must never receive opcode 17. --- config/reliability-gates.jsonc | 55 ++- src/main/ipc/pty.test.ts | 35 ++ src/main/ipc/pty.ts | 7 + .../runtime/rpc/terminal-multiplex.test.ts | 105 +++++ .../terminal-pane/pty-connection.ts | 9 +- .../terminal-pane-recovery.test.ts | 50 ++ .../terminal-pane/terminal-pane-recovery.ts | 17 +- ...untime-rejected-input-remount.unit.test.ts | 436 ++++++++++++++++++ 8 files changed, 696 insertions(+), 18 deletions(-) create mode 100644 tests/e2e/paired-runtime-rejected-input-remount.unit.test.ts diff --git a/config/reliability-gates.jsonc b/config/reliability-gates.jsonc index b1ffe66cf58..4d521b06bec 100644 --- a/config/reliability-gates.jsonc +++ b/config/reliability-gates.jsonc @@ -5973,7 +5973,7 @@ }, { "id": "terminal-input.remote-write-rejection-recovery", - "title": "Rejected paired-runtime terminal input reaches pane recovery", + "title": "Rejected paired-runtime terminal input remounts the pane", "maturity": "experimental", "protection": "partial", "owner": "terminal-runtime", @@ -5982,33 +5982,38 @@ "terminal multiplex input", "legacy binary terminal input", "one-shot terminal.send fallback", - "pane recovery" + "pane recovery", + "pty:hasPty liveness routing" ], "platforms": ["macos", "linux", "windows"], "providers": ["paired-runtime"], "coveredPlatforms": ["macos"], "coveredProviders": ["paired-runtime"], - "coverageNotes": "Deterministic dispatcher and renderer contracts cover rejected authoritative writes, capability negotiation, legacy clients, legacy binary subscriptions, stream-id reuse, pane lifecycle reuse, and the one-shot JSON fallback. Live headed/headless paired-runtime and mixed installed releases remain uncollected.", + "coverageNotes": "One end-to-end contract runs the real dispatcher, renderer multiplexer, remote transport, and pty-connection together and requires the tab remount, so the signal is proven past the transport callback it used to die behind. Focused contracts cover capability negotiation, legacy binary subscriptions, stream-id reuse, pane lifecycle reuse, the one-shot JSON fallback, and main refusing to answer liveness for a `remote:` id. Live headed/headless paired-runtime and mixed installed releases remain uncollected.", "motivatingLinks": [ "https://linear.app/stably/issue/STA-2830", "https://github.com/stablyai/orca/issues/11124" ], - "invariant": "When a paired-runtime client accepts terminal input locally but the authoritative host rejects the PTY write, a capability-compatible stream must notify only that current pane generation so its existing write-unavailable recovery runs. A host must never send the new opcode to a legacy client, and a late rejection must never recover a replacement stream or pane lifecycle.", - "oracle": "Subscribe through the real dispatcher, deliver an Input frame, reject the authoritative runtime send before any process write, and require one WriteUnavailable frame only for a capability-declaring client. Reuse the stream id before releasing a held rejection and require no signal. Decode the signal through the renderer multiplexer and require the current pane callback, then replace the stream or detach and reattach the same handle before releasing held failures and require no stale recovery. Repeat for the legacy binary stream and rejected terminal.send fallback.", + "invariant": "When a paired-runtime client accepts terminal input locally but the authoritative host rejects the PTY write, a capability-compatible stream must notify only that current pane generation and that notification must end in an actual tab remount — no local liveness probe may veto it, because main owns no registry entry for a `remote:` id and must answer unknown for one. A host must never send the new opcode to a legacy or un-negotiated client, and a late rejection must never recover a replacement stream or pane lifecycle.", + "oracle": "Wire the real dispatcher to the real renderer multiplexer, remote transport, and pty-connection over one bridged subscription; type into the pane, reject the authoritative runtime send before any process write, and require both the WriteUnavailable frame and a remountTerminalTabForRecovery call — repeated for every answer main can produce for a `remote:` id (fabricated dead, unknown, thrown). Separately: require one frame only for a capability-declaring client by driving an un-negotiated legacy binary subscriber first and a capable one second on the same runtime, so the capable frame proves the rejection had already been processed for both. Reuse the stream id before releasing a held rejection and require no signal; replace the stream or detach and reattach the same handle before releasing held failures and require no stale recovery. Require pty:hasPty to answer null for a `remote:` id without consulting the local provider.", "commands": [ - "pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/rpc/terminal-multiplex.test.ts src/renderer/src/runtime/runtime-terminal-stream.test.ts src/renderer/src/runtime/remote-runtime-terminal-parse-backpressure.test.ts src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts --reporter=dot" + "pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/rpc/terminal-multiplex.test.ts src/renderer/src/runtime/runtime-terminal-stream.test.ts src/renderer/src/runtime/remote-runtime-terminal-parse-backpressure.test.ts src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts tests/e2e/paired-runtime-rejected-input-remount.unit.test.ts src/renderer/src/components/terminal-pane/terminal-pane-recovery.test.ts src/main/ipc/pty.test.ts --reporter=dot" ], "testFiles": [ "src/main/runtime/rpc/terminal-multiplex.test.ts", "src/renderer/src/runtime/runtime-terminal-stream.test.ts", "src/renderer/src/runtime/remote-runtime-terminal-parse-backpressure.test.ts", - "src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts" + "src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts", + "tests/e2e/paired-runtime-rejected-input-remount.unit.test.ts", + "src/renderer/src/components/terminal-pane/terminal-pane-recovery.test.ts", + "src/main/ipc/pty.test.ts" ], "assertionRefs": [ { "file": "src/main/runtime/rpc/terminal-multiplex.test.ts", "assertions": [ "rejected authoritative input emits WriteUnavailable only for capable multiplex and legacy binary clients", + "an un-negotiated legacy binary subscriber never receives the rejection opcode", "a late rejection cannot target a replacement stream with the same id" ] }, @@ -6019,6 +6024,26 @@ "superseded streams and same-handle pane lifecycles ignore delayed rejections", "a rejected one-shot runtime fallback invokes pane recovery" ] + }, + { + "file": "tests/e2e/paired-runtime-rejected-input-remount.unit.test.ts", + "assertions": [ + "a host-rejected write travels dispatcher to multiplexer to transport to pty-connection and remounts the tab", + "no answer the local liveness probe can give for a `remote:` id blocks that remount" + ] + }, + { + "file": "src/renderer/src/components/terminal-pane/terminal-pane-recovery.test.ts", + "assertions": [ + "input-rejected-by-host recovery consults no liveness probe", + "input-rejected-by-host still coalesces under the shared recovery cooldown" + ] + }, + { + "file": "src/main/ipc/pty.test.ts", + "assertions": [ + "pty:hasPty answers unknown for a paired-runtime handle instead of the local provider's fabricated dead" + ] } ], "evidenceRuns": [ @@ -6026,15 +6051,15 @@ "date": "2026-08-05", "runner": "local", "platform": "macos", - "command": "pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/rpc/terminal-multiplex.test.ts src/renderer/src/runtime/runtime-terminal-stream.test.ts src/renderer/src/runtime/remote-runtime-terminal-parse-backpressure.test.ts src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts --reporter=dot", + "command": "pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/rpc/terminal-multiplex.test.ts src/renderer/src/runtime/runtime-terminal-stream.test.ts src/renderer/src/runtime/remote-runtime-terminal-parse-backpressure.test.ts src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts tests/e2e/paired-runtime-rejected-input-remount.unit.test.ts src/renderer/src/components/terminal-pane/terminal-pane-recovery.test.ts src/main/ipc/pty.test.ts --reporter=dot", "result": "passed", - "durationSeconds": 16, - "summary": "Deterministic host and renderer contracts passed capability, rejection, mixed-version, and stale-generation coverage." + "durationSeconds": 15, + "summary": "678 passed / 0 failed with the end-to-end remount contract, the un-negotiated legacy binary gate, and the pty:hasPty liveness-routing contract added." } ], "runtimeBudget": { - "p95Seconds": 30, - "scope": "focused paired-runtime host and renderer contracts" + "p95Seconds": 45, + "scope": "focused paired-runtime host and renderer contracts plus the end-to-end remount chain" }, "flakeHistory": { "status": "unknown", @@ -6042,11 +6067,11 @@ }, "redGreenEvidence": { "status": "complete", - "evidence": "On origin/main e5f49e0e1d, the test-only dispatcher oracle subscribed, accepted client input, invoked the authoritative host send once, recorded no process write, and failed only because no WriteUnavailable frame returned. Independent host, renderer, and fallback mutations each make their focused oracle red; the integrated implementation is green." + "evidence": "On origin/main e5f49e0e1d, the test-only dispatcher oracle subscribed, accepted client input, invoked the authoritative host send once, recorded no process write, and failed only because no WriteUnavailable frame returned. The end-to-end contract was then red at the last hop on the delivery-only implementation — the host frame arrived and no remount followed — for all three liveness answers (3 failed / 0 passed), and green after the recovery routing fix (3 passed). Deleting the legacy-binary capability gate makes terminal-multiplex red (1 failed / 62 passed); deleting the pty:hasPty `remote:` guard makes the main contract red with the fabricated `false`; reverting either half of the renderer routing makes the end-to-end contract red (3 failed). Every mutation was restored by re-applying the edit and re-verified green." }, "performanceBudget": { "required": true, - "evidence": "The change adds one optional capability field, one constant-time outcome classification per existing write, and one rejection-only frame/callback. It adds no polling, timer, provider listing, subprocess, retained payload, or cross-pane fanout." + "evidence": "The change adds one optional capability field, one constant-time outcome classification per existing write, and one rejection-only frame/callback. The recovery routing adds one string comparison per rejection and removes an IPC round-trip on that path; the pty:hasPty guard is a prefix test that short-circuits a provider lookup. It adds no polling, timer, provider listing, subprocess, retained payload, or cross-pane fanout." }, "promotionCriteria": [ "Collect headed and headless paired-runtime journeys with a rejected host write.", @@ -6058,7 +6083,7 @@ "Linux, Windows, mobile, and mixed installed-release runs are not collected.", "Dead-record connected-state correction remains tracked separately in STA-2896." ], - "demotionRule": "Keep experimental or demote if rejected input can remain silent, a legacy client receives an unknown opcode, a stale failure recovers a replacement pane, or the deterministic contract flakes without an identified product or harness bug." + "demotionRule": "Keep experimental or demote if rejected input can remain silent, the signal stops short of a remount, a legacy or un-negotiated client receives an unknown opcode, a liveness probe fabricates an answer for a `remote:` id, a stale failure recovers a replacement pane, or the deterministic contract flakes without an identified product or harness bug." }, { "id": "terminal-provider.wsl-restore-contract", diff --git a/src/main/ipc/pty.test.ts b/src/main/ipc/pty.test.ts index f4074b38539..c91c7d23dfd 100644 --- a/src/main/ipc/pty.test.ts +++ b/src/main/ipc/pty.test.ts @@ -7071,6 +7071,41 @@ describe('registerPtyHandlers', () => { await expect(handlers.get('pty:hasPty')!(null, { id: 'maybe-pty' })).resolves.toBe(null) }) + it('never answers liveness for a paired-runtime handle from the local registry', async () => { + const hasPty = vi.fn(() => false) + setLocalPtyProvider({ + spawn: vi.fn(), + write: vi.fn(), + resize: vi.fn(), + shutdown: vi.fn(), + sendSignal: vi.fn(), + getCwd: vi.fn(), + getInitialCwd: vi.fn(), + clearBuffer: vi.fn(), + acknowledgeDataEvent: vi.fn(), + hasChildProcesses: vi.fn(), + getForegroundProcess: vi.fn(), + serialize: vi.fn(), + revive: vi.fn(), + onData: vi.fn(() => () => {}), + onReplay: vi.fn(() => () => {}), + onExit: vi.fn(() => () => {}), + listProcesses: vi.fn(async () => []), + attach: vi.fn(), + hasPty, + getDefaultShell: vi.fn(), + getProfiles: vi.fn() + } as never) + registerPtyHandlers(mainWindow as never) + + // The local provider would happily report `false` here — it just doesn't + // hold the id. Callers read that as "the shell died" (STA-2830). + await expect( + handlers.get('pty:hasPty')!(null, { id: 'remote:env-1@@terminal-1' }) + ).resolves.toBe(null) + expect(hasPty).not.toHaveBeenCalled() + }) + it('lists duplicate SSH relay session ids as distinct app sessions', async () => { registerPtyHandlers(mainWindow as never) const shutdownA = vi.fn(async () => undefined) diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index 582e6659324..62448ffd517 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -7159,6 +7159,13 @@ export function registerPtyHandlers( ) ipcMain.handle('pty:hasPty', async (_event, args: { id: string }): Promise => { + if (typeof args?.id !== 'string' || args.id.startsWith('remote:')) { + // Why: same routing hazard pty:kill guards against — ptyOwnership never holds + // a runtime terminal handle and parseAppSshPtyId ignores it, so the lookup + // falls through to the local provider and its "not in my table" reads as an + // authoritative dead. That is a fabricated answer about another host's PTY. + return null + } const ownedConnectionId = ptyOwnership.get(args.id) const parsedSshId = ownedConnectionId === undefined ? parseAppSshPtyId(args.id) : null const provider = parsedSshId diff --git a/src/main/runtime/rpc/terminal-multiplex.test.ts b/src/main/runtime/rpc/terminal-multiplex.test.ts index 3a7bb5704ac..dd46b36650e 100644 --- a/src/main/runtime/rpc/terminal-multiplex.test.ts +++ b/src/main/runtime/rpc/terminal-multiplex.test.ts @@ -3832,6 +3832,111 @@ describe('terminal multiplex RPC', () => { await dispatchPromise }) + it('never sends the rejection opcode to an un-negotiated legacy binary stream', async () => { + // The mobile client is exactly this subscriber: it declares + // terminalBinaryStream and nothing else, and its vendored opcode enum knows + // nothing past 12, so an unsolicited 17 is an unknown opcode on that wire. + // A capable desktop subscriber shares the runtime and is driven second, so + // its frame proves the rejection had already been processed for both. + const cleanups = new Map void>() + const sendTerminal = vi.fn().mockRejectedValue(new Error('terminal_not_writable')) + const runtime = stubRuntime({ + resolveLeafForHandle: vi.fn().mockReturnValue({ ptyId: 'pty-1' }), + readTerminal: vi.fn().mockResolvedValue({ tail: [], truncated: false }), + serializeTerminalBuffer: vi.fn().mockResolvedValue(null), + getTerminalSize: vi.fn().mockReturnValue({ cols: 120, rows: 40 }), + getMobileDisplayMode: vi.fn().mockReturnValue('auto'), + getLayout: vi.fn().mockReturnValue({ seq: 1 }), + subscribeToTerminalData: vi.fn().mockReturnValue(vi.fn()), + subscribeToTerminalResize: vi.fn().mockReturnValue(vi.fn()), + subscribeToFitOverrideChanges: vi.fn().mockReturnValue(vi.fn()), + getDriver: vi.fn().mockReturnValue({ kind: 'idle' }), + registerSubscriptionCleanup: vi.fn((id: string, cleanup: () => void) => { + cleanups.set(id, cleanup) + }), + cleanupSubscription: vi.fn((id: string) => cleanups.get(id)?.()), + waitForTerminal: vi.fn(() => new Promise(() => {})), + sendTerminal: sendTerminal as unknown as OrcaRuntimeService['sendTerminal'], + updateDesktopViewport: vi.fn().mockResolvedValue(true), + handleMobileSubscribe: vi.fn(), + handleMobileUnsubscribe: vi.fn(), + updateMobileViewport: vi.fn().mockResolvedValue(true) + }) + const dispatcher = new RpcDispatcher({ runtime, methods: TERMINAL_METHODS }) + + async function subscribeLegacyBinary( + client: { id: string; type: 'mobile' | 'desktop' }, + capabilities: Record + ) { + const messages: string[] = [] + const binaryFrames: Uint8Array[] = [] + const handlers = new Map< + number, + (frame: NonNullable>) => void + >() + const dispatchPromise = dispatcher.dispatchStreaming( + makeRequest('terminal.subscribe', { terminal: 'terminal-1', client, capabilities }), + (message) => messages.push(message), + { + connectionId: `conn-legacy-${client.id}`, + sendBinary: (bytes) => { + binaryFrames.push(bytes) + }, + registerBinaryStreamHandler: (streamId, handler) => { + handlers.set(streamId, handler) + return () => handlers.delete(streamId) + } + } + ) + await vi.waitFor(() => + expect(messages.some((message) => JSON.parse(message).result?.type === 'subscribed')).toBe( + true + ) + ) + const streamId = JSON.parse( + messages.find((message) => JSON.parse(message).result?.type === 'subscribed')! + ).result.streamId as number + binaryFrames.splice(0) + return { + binaryFrames, + dispatchPromise, + sendInput: () => + handlers.get(streamId)?.( + decodeTerminalStreamFrame( + encodeTerminalStreamFrame({ + opcode: TerminalStreamOpcode.Input, + streamId, + seq: 1, + payload: encodeTerminalStreamText('x') + }) + )! + ) + } + } + + const legacy = await subscribeLegacyBinary( + { id: 'mobile-1', type: 'mobile' }, + { terminalBinaryStream: 1 } + ) + const capable = await subscribeLegacyBinary( + { id: 'desktop-1', type: 'desktop' }, + { terminalBinaryStream: 1, writeUnavailable: 1 } + ) + + legacy.sendInput() + capable.sendInput() + + await vi.waitFor(() => expect(sendTerminal).toHaveBeenCalledTimes(2)) + await vi.waitFor(() => + expect(capable.binaryFrames.some((frame) => frame[2] === WRITE_UNAVAILABLE_OPCODE)).toBe(true) + ) + expect(legacy.binaryFrames.some((frame) => frame[2] === WRITE_UNAVAILABLE_OPCODE)).toBe(false) + + runtime.cleanupSubscription('terminal-1:mobile-1') + runtime.cleanupSubscription('terminal-1:desktop-1') + await Promise.all([legacy.dispatchPromise, capable.dispatchPromise]) + }) + it('owns and releases a viewport floor for legacy JSON desktop streams', async () => { const messages: string[] = [] const cleanups = new Map void>() diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 90fe243210d..38f705e3386 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -3987,10 +3987,17 @@ export function connectPanePty( } const storePtyId = useAppStore.getState().ptyIdsByTabId?.[deps.tabId]?.[0] ?? null const undeliverablePtyId = transport.getPtyId() ?? storePtyId + // Why the split: for a local (daemon/app-SSH) id main's registry can answer, + // and a `false` there means the shell really died — the dead-session + // reconcile owns that teardown and a remount would race it. For a `remote:` + // id main owns no registry entry, so `pty:hasPty` routes to the local + // provider and fabricates "dead"; that answer blocked every recovery this + // signal exists to trigger (STA-2830). The host's own rejection replaces it. + const hostRejectedRemoteInput = providerRejected && isRemoteRuntimePtyId(undeliverablePtyId) void requestTerminalPaneRecovery({ tabId: deps.tabId, ptyId: undeliverablePtyId, - reason: 'input-undeliverable', + reason: hostRejectedRemoteInput ? 'input-rejected-by-host' : 'input-undeliverable', terminalRecoveryGeneration, terminalRecoveryInstanceId: terminalRecoveryInstance.id, // Why: pty:hasPty answers null for ids the local registry doesn't own, diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-recovery.test.ts b/src/renderer/src/components/terminal-pane/terminal-pane-recovery.test.ts index 52d82ead7bb..50d3d693f7d 100644 --- a/src/renderer/src/components/terminal-pane/terminal-pane-recovery.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-pane-recovery.test.ts @@ -443,6 +443,56 @@ describe('requestTerminalPaneRecovery', () => { expect(mocks.remountTerminalTabForRecovery).not.toHaveBeenCalled() }) + // A `remote:` id has no entry in main's registry, so pty:hasPty routes it to + // the local provider. Every answer that path can produce blocked the remount + // this signal exists to trigger (STA-2830); none of them is evidence. + describe('host-rejected input', () => { + for (const [label, liveness] of [ + ['a fabricated dead answer', async () => false], + ['an explicit unknown', async () => null], + [ + 'a failed probe', + async () => { + throw new Error('ipc down') + } + ] + ] as [string, () => Promise][]) { + it(`recovers even though the local probe would give ${label}`, async () => { + mocks.hasPty.mockImplementation(liveness) + + const result = await requestTerminalPaneRecovery({ + tabId: 'tab-1', + ptyId: 'remote:env-1@@terminal-1', + reason: 'input-rejected-by-host', + requireAuthoritativeLiveness: true, + endpointReplaced: true + }) + + expect(result).toBe(true) + expect(mocks.hasPty).not.toHaveBeenCalled() + expect(mocks.remountTerminalTabForRecovery).toHaveBeenCalledWith('tab-1') + }) + } + + it('still coalesces under the shared cooldown', async () => { + expect( + await requestTerminalPaneRecovery({ + tabId: 'tab-1', + ptyId: 'remote:env-1@@terminal-1', + reason: 'input-rejected-by-host' + }) + ).toBe(true) + expect( + await requestTerminalPaneRecovery({ + tabId: 'tab-1', + ptyId: 'remote:env-1@@terminal-1', + reason: 'input-rejected-by-host' + }) + ).toBe(false) + expect(mocks.remountTerminalTabForRecovery).toHaveBeenCalledTimes(1) + }) + }) + it('never throws when the store surface is partial (timer/callback contexts)', async () => { // Regression: recovery fires from stall-watch timers and write callbacks; // an environment with a partial store (mocked suites, teardown races) must diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-recovery.ts b/src/renderer/src/components/terminal-pane/terminal-pane-recovery.ts index cecf4497fb0..9800e95d2ac 100644 --- a/src/renderer/src/components/terminal-pane/terminal-pane-recovery.ts +++ b/src/renderer/src/components/terminal-pane/terminal-pane-recovery.ts @@ -20,6 +20,13 @@ export type TerminalPaneRecoveryReason = | 'write-stalled' | 'replay-wedged' | 'input-undeliverable' + // The paired runtime that owns the PTY refused this write and said so on the + // wire. Distinct from 'input-undeliverable' because it skips the liveness + // probe: main's registry holds no entry for a `remote:` id, so `pty:hasPty` + // routes it to the local provider and answers a fabricated "dead". The + // rejection frame is the evidence instead — it came from the process that + // owns the PTY, over a connection that is by construction still up. + | 'input-rejected-by-host' // A restore was requested for a certified-dead pipeline (reveal path). | 'restore-blocked' @@ -36,7 +43,9 @@ type RecoveryRequest = { /** Remote panes (runtime mirrors, app-SSH) must prove the PTY alive before * an input-undeliverable remount: pty:hasPty answers null for ids the local * registry doesn't own, and treating null as "proceed" would let a - * disconnected remote pane churn reconnects on every cooldown window. */ + * disconnected remote pane churn reconnects on every cooldown window. That + * churn needs a *disconnected* pane, which is why 'input-rejected-by-host' + * is exempt: its evidence arrives over a live connection. */ requireAuthoritativeLiveness?: boolean /** The provider rejected the write because its endpoint stopped accepting * writes, so re-attach MAY land on a *fresh* shell (a respawn; a transient @@ -183,7 +192,9 @@ function cancelPendingRecoveryRetry(tabId: string): void { * * For 'input-undeliverable' the PTY is liveness-checked first: a dead PTY is * the dead-session reconcile's job (it tears down and reports "Process - * exited"), and remounting there would race it. + * exited"), and remounting there would race it. 'input-rejected-by-host' skips + * that probe — see the reason's declaration. Nothing here destroys a session + * either way: a remount rebuilds the renderer over the PTY it already had. */ export async function requestTerminalPaneRecovery(request: RecoveryRequest): Promise { if (!isCurrentTerminalRecoveryRequest(request)) { @@ -199,6 +210,8 @@ export async function requestTerminalPaneRecovery(request: RecoveryRequest): Pro } return false } + // 'input-rejected-by-host' is deliberately absent: no local probe can speak + // for the id it carries, and its evidence already came from the PTY's owner. if (request.reason === 'input-undeliverable') { if (!request.ptyId) { return false diff --git a/tests/e2e/paired-runtime-rejected-input-remount.unit.test.ts b/tests/e2e/paired-runtime-rejected-input-remount.unit.test.ts new file mode 100644 index 00000000000..9a358c65b54 --- /dev/null +++ b/tests/e2e/paired-runtime-rejected-input-remount.unit.test.ts @@ -0,0 +1,436 @@ +/** + * Carries a host-rejected paired-runtime write the whole way: the real + * terminal.multiplex dispatcher rejects the authoritative PTY write, the real + * renderer multiplexer and remote transport decode the WriteUnavailable frame, + * and pty-connection must turn it into an actual tab remount. + * + * Every other test for this signal stops at a transport callback, so the last + * hop was unproven — and that hop is where it died: pane recovery probes + * `pty:hasPty`, which owns no registry entry for a `remote:` id. The parametrized + * liveness answers below cover every reply main can produce for one. + */ +import { beforeEach, afterEach, describe, expect, it, vi } from 'vitest' +import { RpcDispatcher } from '../../src/main/runtime/rpc/dispatcher' +import { TERMINAL_METHODS } from '../../src/main/runtime/rpc/methods/terminal' +import type { OrcaRuntimeService } from '../../src/main/runtime/orca-runtime' +import { + TerminalStreamOpcode, + decodeTerminalStreamFrame +} from '../../src/shared/terminal-stream-protocol' + +const ENVIRONMENT_ID = 'env-1' +const TERMINAL_HANDLE = 'terminal-1' +const REMOTE_PTY_ID = `remote:${ENVIRONMENT_ID}@@${TERMINAL_HANDLE}` +const LEAF_ID = '11111111-1111-4111-8111-111111111111' + +type StoreState = Record + +let mockStoreState: StoreState +let storeSubscribers: ((state: StoreState) => void)[] = [] +const remountTerminalTabForRecovery = vi.fn<(tabId: string) => boolean>(() => true) + +vi.mock('@/store', () => ({ + useAppStore: { + getState: () => mockStoreState, + subscribe: (listener: (state: StoreState) => void) => { + storeSubscribers.push(listener) + return () => { + storeSubscribers = storeSubscribers.filter((candidate) => candidate !== listener) + } + } + } +})) + +vi.mock('@/runtime/sync-runtime-graph', () => ({ scheduleRuntimeGraphSync: vi.fn() })) +vi.mock('@/components/terminal-pane/terminal-webgl-atlas-recovery', () => ({ + scheduleTerminalWebglAtlasRecovery: vi.fn() +})) +vi.mock('sonner', () => ({ toast: { info: vi.fn() } })) +vi.mock('@/lib/codex-stale-pane-sweep', () => ({ notifyCodexPaneBoundForStaleSweep: vi.fn() })) +vi.mock('@/runtime/web-runtime-session', () => ({ + refreshWebRuntimeSessionTabsSnapshot: vi.fn(async () => {}) +})) + +/** One live paired host: the real dispatcher, wired to a runtime that refuses the write. */ +function startHost(): { + bridge: { + subscribe: ( + args: { method: string }, + callbacks: { + onResponse: (response: unknown) => void + onBinary?: (bytes: Uint8Array) => void + onClose?: () => void + } + ) => Promise<{ unsubscribe: () => void; sendBinary: (bytes: Uint8Array) => void }> + call: (request: { method: string; params?: unknown }) => Promise + } + sendTerminal: ReturnType + /** Opcodes the host pushed to this client, in order. */ + hostOpcodes: number[] +} { + const hostOpcodes: number[] = [] + // The host's whole reason to emit the opcode: the PTY refused the bytes. + const sendTerminal = vi.fn().mockResolvedValue({ accepted: false }) + const runtime = { + getRuntimeId: () => 'test-runtime', + registerRemoteTerminalViewSubscriber: () => () => {}, + resolveLiveLeafForHandle: vi.fn().mockReturnValue({ ptyId: 'pty-1' }), + requestRendererTerminalTabMount: vi.fn().mockReturnValue(true), + updateRemoteDesktopViewer: vi.fn().mockResolvedValue(true), + unregisterRemoteDesktopViewer: vi.fn().mockResolvedValue(true), + unregisterRemoteDesktopViewers: vi.fn().mockResolvedValue(true), + isPtyResizeDrivenRemotely: vi.fn().mockReturnValue(false), + getRemoteDesktopFitHold: vi.fn().mockReturnValue({ mode: 'desktop-fit', cols: 120, rows: 40 }), + isRemoteDesktopViewerOwner: vi.fn().mockReturnValue(false), + getPtyOutputSequence: vi.fn().mockReturnValue(0), + attachRemoteTerminalSourceRangeConsumer: vi.fn().mockReturnValue(false), + detachRemoteTerminalSourceRangeConsumer: vi.fn(), + getRendererTerminalSerializerGeneration: vi.fn().mockReturnValue(0), + sendTerminal, + readTerminal: vi.fn().mockResolvedValue({ tail: [], truncated: false }), + serializeTerminalBuffer: vi.fn().mockResolvedValue({ data: 'snapshot', cols: 120, rows: 40 }), + serializeAuthoritativeTerminalBuffer: vi + .fn() + .mockResolvedValue({ data: 'snapshot', cols: 120, rows: 40 }), + getTerminalSize: vi.fn().mockReturnValue({ cols: 120, rows: 40 }), + getMobileDisplayMode: vi.fn().mockReturnValue('auto'), + getLayout: vi.fn().mockReturnValue({ seq: 1 }), + subscribeToTerminalData: vi.fn().mockReturnValue(vi.fn()), + subscribeToTerminalResize: vi.fn().mockReturnValue(vi.fn()), + subscribeToFitOverrideChanges: vi.fn().mockReturnValue(vi.fn()), + subscribeToDriverChanges: vi.fn().mockReturnValue(vi.fn()), + getTerminalFitOverride: vi.fn().mockReturnValue(null), + getDriver: vi.fn().mockReturnValue({ kind: 'idle' }), + registerSubscriptionCleanup: vi.fn(), + cleanupSubscription: vi.fn(), + waitForTerminal: vi.fn(() => new Promise(() => {})) + } as unknown as OrcaRuntimeService + const dispatcher = new RpcDispatcher({ runtime, methods: TERMINAL_METHODS }) + + const bridge = { + async subscribe( + args: { method: string }, + callbacks: { + onResponse: (response: unknown) => void + onBinary?: (bytes: Uint8Array) => void + onClose?: () => void + } + ) { + const handlers = new Map< + number, + (frame: NonNullable>) => void + >() + void dispatcher.dispatchStreaming( + { id: 'req-1', authToken: 'tok', method: args.method, params: {} }, + (message) => callbacks.onResponse(JSON.parse(message)), + { + connectionId: 'conn-e2e', + sendBinary: (bytes) => { + const opcode = decodeTerminalStreamFrame(bytes)?.opcode + if (opcode !== undefined) { + hostOpcodes.push(opcode) + } + callbacks.onBinary?.(bytes) + return true + }, + registerBinaryStreamHandler: (streamId, handler) => { + handlers.set(streamId, handler) + return () => { + if (handlers.get(streamId) === handler) { + handlers.delete(streamId) + } + } + } + } + ) + return { + unsubscribe: vi.fn(), + sendBinary: (bytes: Uint8Array) => { + const frame = decodeTerminalStreamFrame(bytes) + if (frame) { + handlers.get(frame.streamId)?.(frame) + } + } + } + }, + async call(request: { method: string; params?: unknown }) { + if (request.method === 'terminal.resolvePane') { + const params = request.params as { paneKey: string; worktreeId: string } + const separator = params.paneKey.indexOf(':') + return { + ok: true, + result: { + terminal: { + handle: TERMINAL_HANDLE, + tabId: params.paneKey.slice(0, separator), + leafId: params.paneKey.slice(separator + 1), + worktreeId: params.worktreeId + } + } + } + } + return { ok: true, result: { terminal: { handle: TERMINAL_HANDLE } } } + } + } + return { bridge, sendTerminal, hostOpcodes } +} + +function createPane() { + const activeBuffer = { type: 'normal' as const, viewportY: 0, baseY: 0, cursorY: 0, cursorX: 0 } + const container = new EventTarget() as HTMLElement + Object.defineProperty(container, 'dataset', { configurable: true, value: {} }) + const terminal = { + cols: 120, + rows: 40, + element: {}, + buffer: { active: activeBuffer }, + modes: { bracketedPasteMode: false, sendFocusMode: false }, + options: { scrollback: 5_000, ignoreBracketedPasteMode: false, theme: {} }, + write: vi.fn((data: string, callback?: () => void) => { + if (data === '' || callback?.name === 'runParsedSteps') { + callback?.() + } + }), + resize: vi.fn(), + clear: vi.fn(), + scrollToBottom: vi.fn(), + scrollToLine: vi.fn(), + scrollLines: vi.fn(), + paste: vi.fn(), + onData: vi.fn(() => ({ dispose: vi.fn() })), + onResize: vi.fn(() => ({ dispose: vi.fn() })), + onRender: vi.fn(() => ({ dispose: vi.fn() })), + onTitleChange: vi.fn(() => ({ dispose: vi.fn() })), + hasSelection: vi.fn(() => false), + parser: { + registerCsiHandler: vi.fn(() => ({ dispose: vi.fn() })), + registerOscHandler: vi.fn(() => ({ dispose: vi.fn() })) + } + } + return { id: 1, leafId: LEAF_ID, stablePaneId: LEAF_ID, terminal, container } +} + +function createManager() { + const panes = [{ id: 1, leafId: LEAF_ID }] + return { + setPaneGpuRendering: vi.fn(), + markPaneHasComplexScriptOutput: vi.fn(), + rebuildPaneWebgl: vi.fn(), + hasWebglRenderer: vi.fn(() => false), + getPanes: vi.fn(() => panes), + closePane: vi.fn(), + getActivePane: vi.fn(() => panes[0]), + getNumericIdForLeaf: vi.fn(() => 1), + setActivePane: vi.fn() + } +} + +function createDeps() { + return { + tabId: 'tab-1', + worktreeId: 'wt-1', + cwd: '/tmp/wt-1', + startup: null, + restoredLeafId: LEAF_ID, + restoredPtyIdByLeafId: { [LEAF_ID]: REMOTE_PTY_ID }, + paneTransportsRef: { current: new Map() }, + paneMode2031Ref: { current: new Map() }, + paneKittyKeyboardModesRef: { current: new Map() }, + paneLastThemeModeRef: { current: new Map() }, + replayingPanesRef: { current: new Map() }, + isActiveRef: { current: true }, + isVisibleRef: { current: true }, + onPtyExitRef: { current: vi.fn() }, + onAgentExitedRef: { current: vi.fn() }, + onPtyErrorRef: { current: vi.fn() }, + clearTabPtyId: vi.fn(), + consumeSuppressedPtyExit: vi.fn(() => false), + isPtyShutdownPending: vi.fn(() => false), + updateTabTitle: vi.fn(), + setRuntimePaneTitle: vi.fn(), + clearRuntimePaneTitle: vi.fn(), + updateTabPtyId: vi.fn(), + markWorktreeUnread: vi.fn(), + markTerminalTabUnread: vi.fn(), + markTerminalPaneUnread: vi.fn(), + clearWorktreeUnread: vi.fn(), + clearTerminalTabUnread: vi.fn(), + clearTerminalPaneUnread: vi.fn(), + dispatchNotification: vi.fn(), + onShowSessionRestoredBanner: vi.fn(), + setCacheTimerStartedAt: vi.fn(), + syncPanePtyLayoutBinding: vi.fn(), + clearExitedPanePtyLayoutBinding: vi.fn() + } +} + +/** Every answer main can give for a `remote:` id it owns no registry entry for. */ +const LIVENESS_ANSWERS: [string, () => Promise][] = [ + ['a fabricated dead answer from the local registry', async () => false], + ['an explicit unknown', async () => null], + [ + 'a failed probe', + async () => { + throw new Error('ipc unavailable') + } + ] +] + +describe('host-rejected paired-runtime input reaches a pane remount', () => { + beforeEach(() => { + vi.resetModules() + vi.clearAllMocks() + storeSubscribers = [] + remountTerminalTabForRecovery.mockReturnValue(true) + mockStoreState = { + activeWorktreeId: 'wt-1', + activeWorkspaceExecutionHostId: `runtime:${ENVIRONMENT_ID}`, + tabsByWorktree: { 'wt-1': [{ id: 'tab-1', ptyId: REMOTE_PTY_ID }] }, + ptyIdsByTabId: { 'tab-1': [REMOTE_PTY_ID] }, + terminalLayoutsByTabId: { + 'tab-1': { + root: { type: 'leaf', leafId: LEAF_ID }, + activeLeafId: LEAF_ID, + expandedLeafId: null, + ptyIdsByLeafId: { [LEAF_ID]: REMOTE_PTY_ID } + } + }, + unreadTerminalTabs: {}, + deleteStateByWorktreeId: {}, + worktreesByRepo: { + repo1: [{ id: 'wt-1', repoId: 'repo1', path: '/tmp/wt-1', hostId: 'local' }] + }, + runtimeStatusByEnvironmentId: new Map(), + repos: [{ id: 'repo1', connectionId: null, displayName: 'orca' }], + projects: [], + sshConnectionStates: new Map(), + transientClearedAgentStatusConnectionIds: {}, + cacheTimerByKey: {}, + settings: { terminalMainSideEffectAuthority: false }, + codexRestartNoticeByPtyId: {}, + deferredSshReconnectTargets: [], + deferredSshSessionIdsByTabId: {}, + removeDeferredSshReconnectTarget: vi.fn(), + removeDeferredSshSessionId: vi.fn(), + consumePendingColdRestore: vi.fn(() => null), + consumePendingSnapshot: vi.fn(() => null), + runtimePaneTitlesByTabId: {}, + agentStatusByPaneKey: {}, + retainedAgentsByPaneKey: {}, + paneForegroundAgentByPaneKey: {}, + sleepingAgentSessionsByPaneKey: {}, + suppressedPtyExitIds: {}, + agentLaunchConfigByPaneKey: {}, + getAgentLaunchConfigForStatusEntry: vi.fn(), + getAgentLaunchConfigForStatusMetadata: vi.fn(), + clearSleepingAgentSession: vi.fn(), + registerAgentLaunchConfig: vi.fn(), + clearAgentLaunchConfig: vi.fn(), + markWorktreeUnread: vi.fn(), + observeTerminalGitHubPullRequestLink: vi.fn(), + recordTerminalInput: vi.fn(), + setAgentStatus: vi.fn(), + removeAgentStatus: vi.fn(), + dropAgentStatus: vi.fn(), + retireAgentPaneAuthority: vi.fn(), + setPaneForegroundAgent: vi.fn(), + clearPaneForegroundAgent: vi.fn(), + markTerminalTabUnread: vi.fn(), + markTerminalPaneUnread: vi.fn(), + markAgentCompletionPaneUnread: vi.fn(), + remountTerminalTabForRecovery + } + globalThis.requestAnimationFrame = vi.fn((callback: FrameRequestCallback) => { + callback(0) + return 1 + }) + globalThis.cancelAnimationFrame = vi.fn() + }) + + afterEach(() => { + delete (globalThis as { window?: unknown }).window + }) + + for (const [label, hasPty] of LIVENESS_ANSWERS) { + it(`remounts the tab when the pane liveness probe returns ${label}`, async () => { + const { bridge, sendTerminal, hostOpcodes } = startHost() + ;(globalThis as unknown as { window: unknown }).window = { + api: { + runtimeEnvironments: { call: bridge.call, subscribe: bridge.subscribe }, + pty: { + hasPty: vi.fn(hasPty), + kill: vi.fn(), + signal: vi.fn(), + listSessions: vi.fn().mockResolvedValue([]), + getSize: vi.fn().mockResolvedValue(null), + reportGeometry: vi.fn(), + getMainBufferSnapshot: vi.fn().mockResolvedValue(null), + getForegroundProcess: vi.fn().mockResolvedValue(null), + inspectProcess: vi.fn().mockResolvedValue({ + foregroundProcess: null, + hasChildProcesses: false + }), + confirmForegroundProcess: vi.fn().mockResolvedValue(null), + hasChildProcesses: vi.fn().mockResolvedValue(false), + write: vi.fn(), + writeAccepted: vi.fn().mockResolvedValue(true), + setHiddenRendererPty: vi.fn(), + setPtyDeliveryInterest: vi.fn(), + ackColdRestore: vi.fn(), + onClearBufferRequest: vi.fn(() => vi.fn()), + onSerializeBufferRequest: vi.fn(() => vi.fn()), + sendSerializedBuffer: vi.fn(), + declarePendingPaneSerializer: vi.fn().mockResolvedValue(1), + settlePaneSerializer: vi.fn().mockResolvedValue(undefined), + clearPendingPaneSerializer: vi.fn().mockResolvedValue(undefined), + reportRendererSerializerReady: vi.fn().mockResolvedValue(undefined) + }, + platform: { get: vi.fn(() => ({ platform: 'darwin', osRelease: '25.0.0' })) }, + notifications: { + dispatch: vi.fn().mockResolvedValue({ delivered: true }), + playSound: vi.fn().mockResolvedValue({ played: true }) + }, + runtime: { restoreTerminalFit: vi.fn().mockResolvedValue({ restored: true }) }, + agentStatus: { inferInterrupt: vi.fn().mockResolvedValue(false) }, + ssh: { + connect: vi.fn().mockResolvedValue({ status: 'connected' }), + needsPassphrasePrompt: vi.fn().mockResolvedValue(false) + } + }, + dispatchEvent: vi.fn(), + addEventListener: vi.fn(), + removeEventListener: vi.fn() + } + + const { connectPanePty } = await import('@/components/terminal-pane/pty-connection') + const { _resetTerminalPaneRecoveryForTests } = + await import('@/components/terminal-pane/terminal-pane-recovery') + _resetTerminalPaneRecoveryForTests() + + const pane = createPane() + const deps = createDeps() + const binding = connectPanePty(pane as never, createManager() as never, deps as never) + await vi.waitFor(() => { + expect(deps.paneTransportsRef.current.get(1)?.getPtyId()).toBe(REMOTE_PTY_ID) + }) + + // The user types; the host accepts the frame and the PTY refuses the bytes. + sendTerminalInput(pane, 'ls\r') + await vi.waitFor(() => expect(sendTerminal).toHaveBeenCalled()) + // Hop 1: the host turned the refusal into the negotiated frame. + await vi.waitFor(() => expect(hostOpcodes).toContain(TerminalStreamOpcode.WriteUnavailable)) + // Hop 2 (the one that was missing): it survives pane recovery as a remount. + await vi.waitFor(() => expect(remountTerminalTabForRecovery).toHaveBeenCalledWith('tab-1')) + + binding.dispose() + _resetTerminalPaneRecoveryForTests() + }) + } +}) + +function sendTerminalInput(pane: ReturnType, data: string): void { + const calls = pane.terminal.onData.mock.calls as unknown as [(data: string) => void][] + const handler = calls[0]?.[0] + expect(handler).toBeTypeOf('function') + handler?.(data) +}