mirror of
https://github.com/stablyai/orca.git
synced 2026-11-02 10:08:13 +00:00
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>
This commit is contained in:
co-authored by
Neil
parent
fd26aef37c
commit
d4500b486d
@@ -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()
|
||||
}
|
||||
})
|
||||
})
|
||||
@@ -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)
|
||||
|
||||
@@ -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 }
|
||||
])
|
||||
})
|
||||
|
||||
|
||||
@@ -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<IPtyProvider['onExit']>[0]
|
||||
|
||||
let ptyCounter = 0
|
||||
export const ptyProcesses = new Map<string, pty.IPty>()
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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. */
|
||||
|
||||
Reference in New Issue
Block a user