From d4500b486dff19ee4c2b5d8dfa14e64eac7f8b69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Antonio=20Muriano=20Criado?= Date: Mon, 26 Oct 2026 17:59:09 +0100 Subject: [PATCH] fix(pty): feed the runtime once for in-process panes behind a degraded daemon (#33119) When the daemon's PTY spawn check fails, the DegradedDaemonPtyProvider routes fresh terminals to the in-process LocalPtyProvider. bindProviderListeners decided whether to feed the runtime via `instanceof LocalPtyProvider` on the registered provider, so behind the degraded wrapper every in-process chunk and exit was ingested twice (once through configure().onData/onExit, once here), doubling output sequence numbers and exit handling. Tag each data/exit event with `runtimeFed` when the emitting provider already reported it through configure(), and gate the listener's runtime feed on that per-event flag instead of the provider's class. Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com> --- ...ty-degraded-fallback-single-ingest.test.ts | 136 ++++++++++++++++++ src/main/ipc/pty/provider/bind-listeners.ts | 9 +- .../local-pty-provider-io-events.test.ts | 14 +- .../providers/local-pty-provider-state.ts | 18 +-- .../providers/local-pty-session-activation.ts | 16 ++- src/main/providers/pty-provider-contract.ts | 2 + src/main/providers/pty-provider-events.ts | 2 + 7 files changed, 168 insertions(+), 29 deletions(-) create mode 100644 src/main/ipc/pty-degraded-fallback-single-ingest.test.ts diff --git a/src/main/ipc/pty-degraded-fallback-single-ingest.test.ts b/src/main/ipc/pty-degraded-fallback-single-ingest.test.ts new file mode 100644 index 00000000000..86cf5ffab86 --- /dev/null +++ b/src/main/ipc/pty-degraded-fallback-single-ingest.test.ts @@ -0,0 +1,136 @@ +import { describe, expect, it, vi } from 'vitest' +import { spawnMock } from './pty-ipc-mock-registry' +import { setupPtyIpcSuite } from './pty-ipc-test-harness' +import { registerPtyHandlers } from './pty/register-handlers' +import { getLocalPtyProvider, setLocalPtyProvider } from './pty/provider/registry' +import { rebindLocalProviderListeners } from './pty/provider/listener-lifecycle' +import { DegradedDaemonPtyProvider } from '../daemon/degraded-daemon-pty-provider' +import type { DaemonPtyAdapter } from '../daemon/daemon-pty-adapter' +import type { PtyDataEvent } from '../providers/pty-provider-events' +import { OrcaRuntimeService } from '../runtime/orca-runtime' + +vi.mock('electron', () => import('./pty-ipc-mock-registry').then((m) => m.electronModuleMock())) +vi.mock('fs', () => import('./pty-ipc-mock-registry').then((m) => m.fsModuleMock())) +vi.mock('node-pty', () => import('./pty-ipc-mock-registry').then((m) => m.nodePtyModuleMock())) +vi.mock('node:child_process', async (importOriginal) => + (await import('./pty-ipc-mock-registry')).childProcessModuleMock(await importOriginal()) +) +vi.mock('../opencode/hook-service', () => + import('./pty-ipc-mock-registry').then((m) => m.openCodeHookServiceModuleMock()) +) +vi.mock('../mimo/hook-service', () => + import('./pty-ipc-mock-registry').then((m) => m.mimoHookServiceModuleMock()) +) +vi.mock('../agent-hooks/server', () => + import('./pty-ipc-mock-registry').then((m) => m.agentHookServerModuleMock()) +) +vi.mock('../pi/titlebar-extension-service', () => + import('./pty-ipc-mock-registry').then((m) => m.piTitlebarExtensionModuleMock()) +) +vi.mock('../pwsh', () => import('./pty-ipc-mock-registry').then((m) => m.pwshModuleMock())) +vi.mock('../wsl', async (importOriginal) => + (await import('./pty-ipc-mock-registry')).wslModuleMock(await importOriginal()) +) +vi.mock('../telemetry/client', () => + import('./pty-ipc-mock-registry').then((m) => m.telemetryClientModuleMock()) +) +vi.mock('../telemetry/classify-error', () => + import('./pty-ipc-mock-registry').then((m) => m.classifyErrorModuleMock()) +) +vi.mock('../cli/linux-terminal-orca-cli-shim', () => + import('./pty-ipc-mock-registry').then((m) => m.linuxCliShimModuleMock()) +) +vi.mock('../memory/pty-registry', () => + import('./pty-ipc-mock-registry').then((m) => m.ptyRegistryModuleMock()) +) +vi.mock('../agent-hooks/migration-unsupported-pty-state', () => + import('./pty-ipc-mock-registry').then((m) => m.migrationUnsupportedPtyModuleMock()) +) +vi.mock('../codex/codex-pane-account-registry', () => + import('./pty-ipc-mock-registry').then((m) => m.codexPaneAccountRegistryModuleMock()) +) +vi.mock('../codex/codex-state-db-backfill-recovery', () => + import('./pty-ipc-mock-registry').then((m) => m.codexBackfillRecoveryModuleMock()) +) + +// A daemon whose PTY spawn check failed: fresh terminals go to the in-process fallback. +function createUnhealthyDaemonAdapter(daemonData: ((payload: PtyDataEvent) => void)[]) { + return { + spawn: vi.fn(async () => { + throw new Error('fresh spawns must not reach the unhealthy daemon') + }), + listProcesses: vi.fn(async () => []), + onData: vi.fn((cb: (payload: PtyDataEvent) => void) => { + daemonData.push(cb) + return () => {} + }), + onReplay: vi.fn(() => () => {}), + onExit: vi.fn(() => () => {}), + onWriteUnavailable: vi.fn(() => () => {}) + } +} + +describe('degraded daemon with in-process fallback', () => { + const { handlers, mainWindow, createMockProc } = setupPtyIpcSuite() + + it('feeds each fallback PTY chunk and exit to the runtime once', async () => { + vi.useFakeTimers() + const mockProc = createMockProc() + spawnMock.mockReturnValue(mockProc.proc) + const runtime = new OrcaRuntimeService() + const onPtyData = vi.spyOn(runtime, 'onPtyData') + const onPtyExit = vi.spyOn(runtime, 'onPtyExit') + const daemonData: ((payload: PtyDataEvent) => void)[] = [] + try { + handlers.clear() + // The window registers first, so the in-process provider is configured to feed the runtime. + registerPtyHandlers(mainWindow, runtime) + setLocalPtyProvider( + new DegradedDaemonPtyProvider({ + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the degraded provider reaches only these adapter members on this spawn/data/exit path. + current: createUnhealthyDaemonAdapter(daemonData) as unknown as DaemonPtyAdapter, + legacy: [], + fallback: getLocalPtyProvider() + }) + ) + rebindLocalProviderListeners() + + const spawned: unknown = await handlers.get('pty:spawn')!(null, { + cols: 80, + rows: 24, + cwd: '/tmp' + }) + if ( + !spawned || + typeof spawned !== 'object' || + !('id' in spawned) || + typeof spawned.id !== 'string' + ) { + throw new Error('pty:spawn returned no id') + } + const { id } = spawned + mockProc.emitData('frame') + daemonData.forEach((cb) => cb({ id: 'daemon-pty', data: 'daemon' })) + await vi.advanceTimersByTimeAsync(50) + + expect(onPtyData.mock.calls).toEqual([ + [id, 'frame', expect.any(Number), 5, undefined], + ['daemon-pty', 'daemon', expect.any(Number), 6, undefined] + ]) + expect(runtime.getPtyOutputSequence(id)).toBe(5) + expect(runtime.getPtyOutputSequence('daemon-pty')).toBe(6) + expect(mainWindow.webContents.send).toHaveBeenCalledWith('pty:data', { + id, + data: 'frame', + seq: 5, + rawLength: 5 + }) + + mockProc.emitExit(0) + await vi.advanceTimersByTimeAsync(50) + expect(onPtyExit).toHaveBeenCalledTimes(1) + } finally { + vi.useRealTimers() + } + }) +}) diff --git a/src/main/ipc/pty/provider/bind-listeners.ts b/src/main/ipc/pty/provider/bind-listeners.ts index be350e64141..0323d6995ea 100644 --- a/src/main/ipc/pty/provider/bind-listeners.ts +++ b/src/main/ipc/pty/provider/bind-listeners.ts @@ -1,4 +1,3 @@ -import { LocalPtyProvider } from '../../../providers/local-pty-provider' import { isCurrentPtyExit, ptyOwnership } from './ownership-state' import { localBackgroundStreamUnsub, @@ -66,13 +65,11 @@ export function bindProviderListeners(session: PtyIpcSession): void { }) ?? null ) - // Daemon providers lack configure().onData; feed runtime records/streams here. - const isLocalProvider = localProvider instanceof LocalPtyProvider - + // Feed the runtime unless the emitting provider already did via configure(). setLocalDataUnsub( localProvider.onData((payload) => { const rawLength = payload.sequenceChars ?? payload.data.length - const outputSeq = isLocalProvider + const outputSeq = payload.runtimeFed ? session.runtime?.getPtyOutputSequence(payload.id) : session.runtime?.onPtyData( payload.id, @@ -99,7 +96,7 @@ export function bindProviderListeners(session: PtyIpcSession): void { ) : undefined try { - if (!isLocalProvider) { + if (!payload.runtimeFed) { const viewShown = ORCA_LAYOUT_OWNER_IS_MAIN && visibleRendererPtys.has(payload.id) clearProviderPtyState(payload.id) ptyOwnership.delete(payload.id) diff --git a/src/main/providers/local-pty-provider-io-events.test.ts b/src/main/providers/local-pty-provider-io-events.test.ts index 030cada0c2a..85ff15914e4 100644 --- a/src/main/providers/local-pty-provider-io-events.test.ts +++ b/src/main/providers/local-pty-provider-io-events.test.ts @@ -248,15 +248,23 @@ describe('LocalPtyProvider', () => { ['prompt', expect.any(Number)] ]) expect(dataHandler.mock.calls.map(([payload]) => payload)).toEqual([ - { id, data: '', sequenceChars: query.length, seq: query.length, transformed: true }, + { + id, + data: '', + sequenceChars: query.length, + seq: query.length, + transformed: true, + runtimeFed: true + }, { id, data: '', sequenceChars: echo.length, seq: query.length + echo.length, - transformed: true + transformed: true, + runtimeFed: true }, - { id, data: 'prompt' } + { id, data: 'prompt', runtimeFed: true } ]) }) diff --git a/src/main/providers/local-pty-provider-state.ts b/src/main/providers/local-pty-provider-state.ts index a0dfda57646..956934904e4 100644 --- a/src/main/providers/local-pty-provider-state.ts +++ b/src/main/providers/local-pty-provider-state.ts @@ -1,7 +1,8 @@ import type * as pty from 'node-pty' import type { PhysicalExitTracker } from '../../shared/physical-exit-tracker' import type { PtyStartupIngress } from '../../shared/pty-startup-ingress' -import type { TerminalExitCause } from '../../shared/terminal-exit-cause' +import type { IPtyProvider } from './pty-provider-contract' +import type { PtyDataEvent } from './pty-provider-events' import { getSpawnedShellName, normalizeLocalCallerSessionId } from './local-pty-launch-helpers' export type PtyShutdownOperation = { @@ -15,20 +16,9 @@ export type PendingLocalPtySpawn = { cancellation: AbortController } -export type DataCallback = (payload: { - id: string - data: string - sequenceChars?: number - transformed?: boolean - seq?: number -}) => void +export type DataCallback = (payload: PtyDataEvent) => void -export type ExitCallback = (payload: { - id: string - code: number - incarnationId?: string - cause?: TerminalExitCause -}) => void +export type ExitCallback = Parameters[0] let ptyCounter = 0 export const ptyProcesses = new Map() diff --git a/src/main/providers/local-pty-session-activation.ts b/src/main/providers/local-pty-session-activation.ts index 9bebe81de13..5e4e805cc84 100644 --- a/src/main/providers/local-pty-session-activation.ts +++ b/src/main/providers/local-pty-session-activation.ts @@ -81,11 +81,13 @@ export function activateLocalPtySession(args: { const emitIngressData = (emission: PtyIngressEmission): void => { const sequenceChars = emission.rawEndSeq - emission.rawStartSeq + const { onData } = getOptions() if (emission.transformed || sequenceChars !== emission.data.length) { - getOptions().onData?.(id, emission.data, Date.now(), sequenceChars, true) + onData?.(id, emission.data, Date.now(), sequenceChars, true) } else { - getOptions().onData?.(id, emission.data, Date.now()) + onData?.(id, emission.data, Date.now()) } + const runtimeFed = onData ? true : undefined for (const cb of dataListeners) { cb( emission.transformed || sequenceChars !== emission.data.length @@ -94,9 +96,10 @@ export function activateLocalPtySession(args: { data: emission.data, sequenceChars, seq: emission.rawEndSeq, - transformed: true + transformed: true, + runtimeFed } - : { id, data: emission.data } + : { id, data: emission.data, runtimeFed } ) } } @@ -153,9 +156,10 @@ export function activateLocalPtySession(args: { // Release the natural-exit ptmx fd now instead of waiting for GC (docs/fix-pty-fd-leak.md). destroyPtyProcess(proc, { alreadyKilled: wasTerminationRequested }) ptyReportsChildExitStatus.delete(id) - getOptions().onExit?.(id, exitCode, incarnationId, cause) + const { onExit } = getOptions() + onExit?.(id, exitCode, incarnationId, cause) for (const cb of exitListeners) { - cb({ id, code: exitCode, incarnationId, cause }) + cb({ id, code: exitCode, incarnationId, cause, runtimeFed: onExit ? true : undefined }) } }) if (onExitDisposable) { diff --git a/src/main/providers/pty-provider-contract.ts b/src/main/providers/pty-provider-contract.ts index 3c404fa56a1..930636c9461 100644 --- a/src/main/providers/pty-provider-contract.ts +++ b/src/main/providers/pty-provider-contract.ts @@ -221,6 +221,8 @@ export type IPtyProvider = { cause?: TerminalExitCause /** Main ended the session to restart the daemon; the pane stays for its next process. */ replacedByRestart?: true + /** The emitting provider already reported this exit to the runtime through configure().onExit. */ + runtimeFed?: true }) => void ): () => void } diff --git a/src/main/providers/pty-provider-events.ts b/src/main/providers/pty-provider-events.ts index a8a90f55693..4cca06f67f4 100644 --- a/src/main/providers/pty-provider-events.ts +++ b/src/main/providers/pty-provider-events.ts @@ -6,6 +6,8 @@ export type PtyDataEvent = { sequenceChars?: number transformed?: boolean seq?: number + /** The emitting provider already fed this chunk to the runtime through configure().onData. */ + runtimeFed?: true } /** Notification fact observed while the thinning transport owned scan authority for this PTY. */