From b912990becfc9369516fd4ef9a58b78e8cf20008 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 12 Aug 2026 01:03:59 -0700 Subject: [PATCH] fix(terminal): keep a DA1 answerer for parked panes with raw-byte sidecars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fish re-emits DA1 at every prompt paint and blocks up to 10s on the reply. Main's query authority yielded whenever the hidden-delivery gate delivered the chunk, on the assumption that delivery implies a renderer xterm will answer. A raw-byte sidecar (background agent launch, automation observer, draft-ready probe) claims delivery interest, and parking unmounts the pane's xterm — so a parked pane with a live sidecar had NOBODY answering, and every prompt took 10.0s indefinitely (fish's give-up latch only arms on the startup probe). Hidden now means the view is suppressed regardless of delivery interest: bytes a sidecar still needs are sent flagged `sidecarOnly`, the dispatcher routes them past the primary handler, and main owns their replies. The flag is captured at ingestion and rides the chunk through batching, so a reveal landing before the flush cannot hand an already-answered chunk to a freshly mounted xterm. Also: a paired stream that pauses output receives nothing, so it now swaps its view presence for raw presence — the host model answers while paused instead of yielding to a client that cannot. --- .github/workflows/pr.yml | 2 + src/main/ipc/pty-hidden-delivery-gate.test.ts | 35 +++ src/main/ipc/pty-hidden-delivery-gate.ts | 41 +++- .../ipc/pty-pending-data-drain-contract.ts | 3 + src/main/ipc/pty.test.ts | 65 ++++-- src/main/ipc/pty.ts | 49 +++- ...arked-pane-prompt-latency.node-pty.test.ts | 215 ++++++++++++++++++ src/main/runtime/rpc/methods/terminal.ts | 40 +++- .../runtime/rpc/terminal-multiplex.test.ts | 46 ++++ .../terminal-model-query-authority.test.ts | 9 +- .../runtime/terminal-model-query-authority.ts | 24 +- .../runtime/terminal-query-responder.test.ts | 12 +- src/preload/api-types.ts | 3 + src/preload/index.ts | 1 + .../pty-data-delivery-routing.test.ts | 64 ++++++ .../pty-data-delivery-routing.ts | 110 +++++++++ .../terminal-pane/pty-dispatcher.ts | 77 +------ .../pty-shutdown-data-suspension.ts | 6 +- 18 files changed, 678 insertions(+), 124 deletions(-) create mode 100644 src/main/runtime/fish-parked-pane-prompt-latency.node-pty.test.ts create mode 100644 src/renderer/src/components/terminal-pane/pty-data-delivery-routing.test.ts create mode 100644 src/renderer/src/components/terminal-pane/pty-data-delivery-routing.ts diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 0c1789bf3ea..e6b6422bd75 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -247,6 +247,7 @@ jobs: src/main/providers/__tests__/shell-ready-framework-example.test.ts \ src/main/pty/omp-shell-wrapper.node-pty.test.ts \ src/renderer/src/components/terminal-pane/fish-color-scheme-child-stdin.node-pty.test.ts \ + src/main/runtime/fish-parked-pane-prompt-latency.node-pty.test.ts \ src/shared/posix-command-path-lookup.test.ts test: @@ -283,6 +284,7 @@ jobs: --exclude=src/main/providers/__tests__/shell-ready-framework-example.test.ts \ --exclude=src/main/pty/omp-shell-wrapper.node-pty.test.ts \ --exclude=src/renderer/src/components/terminal-pane/fish-color-scheme-child-stdin.node-pty.test.ts \ + --exclude=src/main/runtime/fish-parked-pane-prompt-latency.node-pty.test.ts \ --exclude=src/shared/posix-command-path-lookup.test.ts \ --exclude=tests/e2e/cross-version-wire/** \ --shard=${{ matrix.shard }}/${{ matrix.shard_total }} diff --git a/src/main/ipc/pty-hidden-delivery-gate.test.ts b/src/main/ipc/pty-hidden-delivery-gate.test.ts index c85c4e2c8e0..e4358812d5b 100644 --- a/src/main/ipc/pty-hidden-delivery-gate.test.ts +++ b/src/main/ipc/pty-hidden-delivery-gate.test.ts @@ -6,9 +6,12 @@ import { isHiddenPtyDeliveryGateEnabled, markHiddenRendererPty, recordHiddenRendererPtyDataDrop, + recordHiddenRendererPtyViewGap, resetRendererScopedHiddenPtyDeliveryState, setRendererPtyDeliveryInterest, + shouldDeliverHiddenRendererPtyDataToSidecarsOnly, shouldDropHiddenRendererPtyData, + shouldSuppressHiddenRendererPtyView, unmarkHiddenRendererPty } from './pty-hidden-delivery-gate' @@ -41,6 +44,38 @@ describe('pty hidden delivery gate', () => { expect(shouldDropHiddenRendererPtyData(PTY_ID, {})).toBe(true) }) + it('suppresses the view for every hidden PTY, delivery interest or not', () => { + expect(shouldSuppressHiddenRendererPtyView(PTY_ID, {})).toBe(false) + markHiddenRendererPty(PTY_ID) + expect(shouldSuppressHiddenRendererPtyView(PTY_ID, {})).toBe(true) + setRendererPtyDeliveryInterest(PTY_ID, true) + expect(shouldSuppressHiddenRendererPtyView(PTY_ID, {})).toBe(true) + expect( + shouldSuppressHiddenRendererPtyView(PTY_ID, { terminalHiddenDeliveryGate: false }) + ).toBe(false) + }) + + it('routes a suppressed PTY sidecar-only exactly while interest is registered', () => { + markHiddenRendererPty(PTY_ID) + expect(shouldDeliverHiddenRendererPtyDataToSidecarsOnly(PTY_ID, {})).toBe(false) + setRendererPtyDeliveryInterest(PTY_ID, true) + expect(shouldDeliverHiddenRendererPtyDataToSidecarsOnly(PTY_ID, {})).toBe(true) + // Exactly one of drop / sidecar-only ever holds, so bytes are never both dropped and sent. + expect(shouldDropHiddenRendererPtyData(PTY_ID, {})).toBe(false) + unmarkHiddenRendererPty(PTY_ID) + expect(shouldDeliverHiddenRendererPtyDataToSidecarsOnly(PTY_ID, {})).toBe(false) + }) + + it('latches the restore marker for sidecar-only sends too (the view missed those bytes)', () => { + markHiddenRendererPty(PTY_ID) + setRendererPtyDeliveryInterest(PTY_ID, true) + expect(recordHiddenRendererPtyViewGap(PTY_ID).shouldEmitRestoreMarker).toBe(true) + expect(recordHiddenRendererPtyViewGap(PTY_ID).shouldEmitRestoreMarker).toBe(false) + expect(unmarkHiddenRendererPty(PTY_ID).droppedWhileHidden).toBe(true) + // Why: the sidecar send is not a drop — the diagnostics counters must stay clean. + expect(getHiddenRendererPtyDeliveryDebug().hiddenDeliveryDroppedChunks).toBe(0) + }) + it('requests the restore marker exactly once per drop episode, re-armed by unmark', () => { markHiddenRendererPty(PTY_ID) expect(recordHiddenRendererPtyDataDrop(PTY_ID, 10).shouldEmitRestoreMarker).toBe(true) diff --git a/src/main/ipc/pty-hidden-delivery-gate.ts b/src/main/ipc/pty-hidden-delivery-gate.ts index 43fa8567b88..25f53d915e5 100644 --- a/src/main/ipc/pty-hidden-delivery-gate.ts +++ b/src/main/ipc/pty-hidden-delivery-gate.ts @@ -6,8 +6,13 @@ * main then drops renderer-bound delivery AFTER model ingestion — the runtime * already parsed the chunk, and reveal restores from the model snapshot via * the existing seq-guarded machinery. Any renderer party that still needs raw - * bytes (dispatcher sidecars) registers delivery - * interest, which suppresses the gate for that PTY. + * bytes (dispatcher sidecars) registers delivery interest; those bytes are + * still sent, but flagged sidecar-only so no view renders or replies to them. + * + * The view suppression, not the send, is what main's query authority keys on: + * a sidecar is not an xterm, so inferring "delivered ⇒ a renderer xterm will + * answer" left parked panes with a live sidecar unanswered (fish blocks ~10s + * on DA1 at every prompt paint). */ import type { GlobalSettings } from '../../shared/types' @@ -77,17 +82,35 @@ export function setRendererPtyDeliveryInterest(id: string, interested: boolean): } } +/** No renderer VIEW may render or reply to this PTY's bytes: hidden panes are + * starved by design and parked panes have no xterm at all. Delivery interest + * does not lift this — it only decides whether the bytes still travel (to + * sidecars) or are dropped outright. */ +export function shouldSuppressHiddenRendererPtyView( + id: string, + settings: HiddenPtyDeliveryGateSettings | null | undefined +): boolean { + return isHiddenPtyDeliveryGateEnabled(settings) && hiddenRendererPtys.has(id) +} + export function shouldDropHiddenRendererPtyData( id: string, settings: HiddenPtyDeliveryGateSettings | null | undefined ): boolean { return ( - isHiddenPtyDeliveryGateEnabled(settings) && - hiddenRendererPtys.has(id) && - !deliveryInterestRendererPtys.has(id) + shouldSuppressHiddenRendererPtyView(id, settings) && !deliveryInterestRendererPtys.has(id) ) } +/** Bytes a sidecar still needs while the view is suppressed: sent, but flagged + * so the dispatcher routes them past the primary (xterm) handler. */ +export function shouldDeliverHiddenRendererPtyDataToSidecarsOnly( + id: string, + settings: HiddenPtyDeliveryGateSettings | null | undefined +): boolean { + return shouldSuppressHiddenRendererPtyView(id, settings) && deliveryInterestRendererPtys.has(id) +} + /** Record one gated drop. Returns whether the caller should emit the one-shot * empty restore-marker chunk (first drop since this PTY went hidden). */ export function recordHiddenRendererPtyDataDrop( @@ -96,6 +119,14 @@ export function recordHiddenRendererPtyDataDrop( ): { shouldEmitRestoreMarker: boolean } { droppedHiddenDeliveryChars += chars droppedHiddenDeliveryChunks += 1 + return recordHiddenRendererPtyViewGap(id) +} + +/** Same restore latch as a drop, for bytes that were sent sidecar-only: the + * view missed them either way, so reveal must repaint from the model. */ +export function recordHiddenRendererPtyViewGap(id: string): { + shouldEmitRestoreMarker: boolean +} { if (droppedSinceHiddenPtys.has(id)) { return { shouldEmitRestoreMarker: false } } diff --git a/src/main/ipc/pty-pending-data-drain-contract.ts b/src/main/ipc/pty-pending-data-drain-contract.ts index d1d4c5be1a8..0a9874239d6 100644 --- a/src/main/ipc/pty-pending-data-drain-contract.ts +++ b/src/main/ipc/pty-pending-data-drain-contract.ts @@ -7,6 +7,9 @@ export type PendingPtyData = { transformed?: true containsBackgroundOutput?: boolean droppedOutput?: true + /** Ingested while no renderer view could see this PTY, so main answered its + * queries: the send must stay sidecar-only even if the pane reveals first. */ + viewSuppressed?: true droppedMode2031Data?: string droppedMode2031ScanState?: Mode2031ReplyScanState projectionAdmissionIds?: readonly string[] diff --git a/src/main/ipc/pty.test.ts b/src/main/ipc/pty.test.ts index ae9166148af..5736be18c99 100644 --- a/src/main/ipc/pty.test.ts +++ b/src/main/ipc/pty.test.ts @@ -16715,7 +16715,7 @@ describe('registerPtyHandlers', () => { } }) - it('suppresses the gate while renderer delivery interest is registered', async () => { + it('routes hidden bytes sidecar-only while renderer delivery interest is registered', async () => { vi.useFakeTimers() const mockProc = createMockProc() spawnMock.mockReturnValue(mockProc.proc) @@ -16735,20 +16735,59 @@ describe('registerPtyHandlers', () => { setInterest(null, { id: spawnResult.id, interested: true }) mockProc.emitData('sidecar bytes') vi.advanceTimersByTime(2) + // Why the flag: the sidecar gets the bytes, the view does not — so main + // stays the query answerer for a pane whose xterm is unmounted. expect(mainWindow.webContents.send).toHaveBeenCalledWith('pty:data', { id: spawnResult.id, - data: 'sidecar bytes' + data: 'sidecar bytes', + sidecarOnly: true + }) + expect(mainWindow.webContents.send).toHaveBeenCalledWith('pty:modelRestoreNeeded', { + id: spawnResult.id, + reason: 'hidden-drop' }) setInterest(null, { id: spawnResult.id, interested: false }) mainWindow.webContents.send.mockClear() mockProc.emitData('gated bytes') vi.advanceTimersByTime(2) - expect(mainWindow.webContents.send).toHaveBeenCalledTimes(1) - expect(mainWindow.webContents.send).toHaveBeenCalledWith('pty:modelRestoreNeeded', { - id: spawnResult.id, - reason: 'hidden-drop' - }) + // Nothing reaches the renderer now, and the restore latch was already + // consumed by the sidecar-only send, so no second marker. + expect(mainWindow.webContents.send).not.toHaveBeenCalled() + } finally { + vi.useRealTimers() + } + }) + + it('keeps queued sidecar-only bytes sidecar-only when the pane reveals before the flush', async () => { + vi.useFakeTimers() + const mockProc = createMockProc() + spawnMock.mockReturnValue(mockProc.proc) + + try { + registerPtyHandlers(mainWindow as never) + const spawnResult = (await handlers.get('pty:spawn')!(null, { + cols: 80, + rows: 24, + cwd: '/tmp' + })) as { id: string } + const setHidden = getPtySetHiddenRendererPtyListener() + const setInterest = getPtySetDeliveryInterestListener() + + setHidden(null, { id: spawnResult.id, hidden: true }) + setInterest(null, { id: spawnResult.id, interested: true }) + mockProc.emitData('parked bytes') + mainWindow.webContents.send.mockClear() + // Reveal lands between ingestion and flush; main already answered this + // chunk's queries, so the now-mounted xterm must never parse it. + setHidden(null, { id: spawnResult.id, hidden: false }) + vi.advanceTimersByTime(50) + + const dataSends = mainWindow.webContents.send.mock.calls.filter( + (call: unknown[]) => call[0] === 'pty:data' + ) + expect(dataSends).toHaveLength(1) + expect(dataSends[0][1]).toMatchObject({ data: 'parked bytes', sidecarOnly: true }) } finally { vi.useRealTimers() } @@ -17135,7 +17174,7 @@ describe('registerPtyHandlers', () => { vi.advanceTimersByTime(50) expect(mainWindow.webContents.send).toHaveBeenLastCalledWith( 'pty:data', - expect.objectContaining({ id: result.id, data: 'sidecar bytes' }) + expect.objectContaining({ id: result.id, data: 'sidecar bytes', sidecarOnly: true }) ) // Why: the renderer reload killed the sidecar's ref count without a release IPC — the leaked hold must not force-feed the PTY forever. @@ -17145,12 +17184,10 @@ describe('registerPtyHandlers', () => { daemon.emitData(result.id, 'gated after reload') vi.advanceTimersByTime(50) - expect(mainWindow.webContents.send).toHaveBeenCalledTimes(1) - expect(mainWindow.webContents.send).toHaveBeenCalledWith('pty:modelRestoreNeeded', { - id: result.id, - reason: 'hidden-drop', - markerSeq: 42 - }) + // Fully dropped now, not merely sidecar-routed. The marker latch is + // deliberately preserved across a reload (the new renderer's first + // unmark re-emits), so the drop itself is silent. + expect(mainWindow.webContents.send).not.toHaveBeenCalled() } finally { vi.useRealTimers() } diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index 67c0cb04edc..16168ad5cc9 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -170,9 +170,11 @@ import { isHiddenRendererPty, markHiddenRendererPty, recordHiddenRendererPtyDataDrop, + recordHiddenRendererPtyViewGap, resetHiddenRendererPtyDeliveryDebugCounters, resetRendererScopedHiddenPtyDeliveryState, setRendererPtyDeliveryInterest, + shouldDeliverHiddenRendererPtyDataToSidecarsOnly, shouldDropHiddenRendererPtyData, unmarkHiddenRendererPty } from './pty-hidden-delivery-gate' @@ -2494,6 +2496,7 @@ export function registerPtyHandlers( transformed?: boolean background?: boolean droppedOutput?: boolean + sidecarOnly?: boolean } // Why: bounded batch windows amortize renderer IPC; keystroke echo/redraws bypass them below. @@ -2910,9 +2913,13 @@ export function registerPtyHandlers( startSeq: number | undefined, containsBackgroundOutput: boolean | undefined, rawLength = data.length, - transformed = false + transformed = false, + viewSuppressed = false ): PtyDataPayload { const payload: PtyDataPayload = { id, data } + if (viewSuppressed) { + payload.sidecarOnly = true + } if (typeof startSeq === 'number') { payload.seq = startSeq + rawLength } @@ -3074,6 +3081,12 @@ export function registerPtyHandlers( payload: PtyDataPayload, projectionAdmissionIds?: readonly string[] ): { sent: boolean; projectionsTransferred: boolean } { + // Why the caller decides: ownership was captured at ingestion (same tick and + // module state as the runtime's reply decision), so a reveal that lands + // before the flush cannot hand these bytes to an xterm main already answered for. + if (payload.sidecarOnly === true && recordHiddenRendererPtyViewGap(id).shouldEmitRestoreMarker) { + sendModelRestoreNeededMarker(id, 'hidden-drop', runtime?.getPtyOutputSequence(id)) + } const charCount = getPtyPayloadCharCount(payload) const accounting = rendererDeliveryAccountingByPty.get(id) const hadAccounting = accounting !== undefined @@ -3253,6 +3266,7 @@ export function registerPtyHandlers( return { data: extractDroppedPtyQueryBytes(pending.data).slice(0, DROPPED_QUERY_SALVAGE_MAX_CHARS), droppedOutput: true, + ...(pending.viewSuppressed === true ? { viewSuppressed: true as const } : {}), droppedMode2031Data: mode2031.data, droppedMode2031ScanState: mode2031.state } @@ -3300,7 +3314,8 @@ export function registerPtyHandlers( containsBackgroundOutput: boolean, rawLength = data.length, transformed = false, - projectionSemanticsId?: string + projectionSemanticsId?: string, + viewSuppressed = false ): PendingPtyData { // Why stay dropped at O(1): once over the cap the restore sentinel supersedes interim bytes; queries still get carved out (bounded) so replies survive the whole episode. if (existing?.droppedOutput === true) { @@ -3326,13 +3341,17 @@ export function registerPtyHandlers( const projectionState = compactPendingProjectionState(existing ?? {}, projectionSemanticsId) const nextContainsBackgroundOutput = existing?.containsBackgroundOutput === true || containsBackgroundOutput + // Why sticky: one byte main already answered for taints the whole coalesced + // entry — the view heals from the model snapshot, a double reply does not. + const nextViewSuppressed = existing?.viewSuppressed === true || viewSuppressed if (!existing) { const pending: PendingPtyData = { data, ...(typeof startSeq === 'number' ? { startSeq } : {}), ...(rawLength !== data.length ? { rawLength } : {}), ...(transformed ? { transformed: true } : {}), - ...(nextContainsBackgroundOutput ? { containsBackgroundOutput: true } : {}) + ...(nextContainsBackgroundOutput ? { containsBackgroundOutput: true } : {}), + ...(nextViewSuppressed ? { viewSuppressed: true as const } : {}) } updatePendingProjectionAdmissions(pending, projectionState) return dropOversizedPendingPtyData(id, pending) @@ -3343,7 +3362,8 @@ export function registerPtyHandlers( ...(!preservesSeq || existing.transformed || transformed ? { rawLength: existingRawLength + rawLength, transformed: true as const } : {}), - ...(nextContainsBackgroundOutput ? { containsBackgroundOutput: true } : {}) + ...(nextContainsBackgroundOutput ? { containsBackgroundOutput: true } : {}), + ...(nextViewSuppressed ? { viewSuppressed: true as const } : {}) } updatePendingProjectionAdmissions(next, projectionState) if (typeof existing.startSeq === 'number') { @@ -3452,7 +3472,8 @@ export function registerPtyHandlers( { id, data: pending.data + getDroppedMode2031RendererData(pending), - droppedOutput: true + droppedOutput: true, + ...(pending.viewSuppressed === true ? { sidecarOnly: true } : {}) }, pending.projectionAdmissionIds ).sent @@ -3476,6 +3497,9 @@ export function registerPtyHandlers( if (pending.containsBackgroundOutput === true) { nextPending.containsBackgroundOutput = true } + if (pending.viewSuppressed === true) { + nextPending.viewSuppressed = true + } if (pending.projectionAdmissionIds) { nextPending.projectionAdmissionIds = pending.projectionAdmissionIds } @@ -3496,7 +3520,8 @@ export function registerPtyHandlers( pending.startSeq, pending.containsBackgroundOutput, pending.rawLength, - pending.transformed + pending.transformed, + pending.viewSuppressed === true ), pending.projectionAdmissionIds ) @@ -3608,7 +3633,8 @@ export function registerPtyHandlers( { id: payload.id, data: remaining.data, - droppedOutput: true + droppedOutput: true, + ...(remaining.viewSuppressed === true ? { sidecarOnly: true } : {}) }, remaining.projectionAdmissionIds ) @@ -3621,7 +3647,8 @@ export function registerPtyHandlers( remaining.startSeq, remaining.containsBackgroundOutput, remaining.rawLength, - remaining.transformed + remaining.transformed, + remaining.viewSuppressed === true ), remaining.projectionAdmissionIds ) @@ -3762,7 +3789,8 @@ export function registerPtyHandlers( containsBackgroundOutput, rawLength, payload.transformed === true, - projectionId + projectionId, + shouldDeliverHiddenRendererPtyDataToSidecarsOnly(payload.id, getSettings?.()) ) const shouldEmitPendingCapRestoreMarker = pending.droppedOutput === true && @@ -3804,7 +3832,8 @@ export function registerPtyHandlers( : {}), ...(pending.transformed ? { transformed: true } : {}), ...(pending.containsBackgroundOutput === true ? { background: true } : {}), - ...(pending.droppedOutput === true ? { droppedOutput: true } : {}) + ...(pending.droppedOutput === true ? { droppedOutput: true } : {}), + ...(pending.viewSuppressed === true ? { sidecarOnly: true } : {}) }, pending.projectionAdmissionIds ) diff --git a/src/main/runtime/fish-parked-pane-prompt-latency.node-pty.test.ts b/src/main/runtime/fish-parked-pane-prompt-latency.node-pty.test.ts new file mode 100644 index 00000000000..9d71e89f362 --- /dev/null +++ b/src/main/runtime/fish-parked-pane-prompt-latency.node-pty.test.ts @@ -0,0 +1,215 @@ +/** + * Real-fish regression for the parked-pane DA1 stall. + * + * fish re-emits DA1 (`ESC [ 0 c`) at EVERY prompt paint and BLOCKS on the reply + * for up to 10s. Its "give up after one timeout" latch only arms when the + * timeout happens during the startup probe, so a reply source that disappears + * later stalls every subsequent prompt, indefinitely. + * + * The configuration under test is a PARKED pane (xterm unmounted, so no + * renderer responder exists) whose PTY still has a raw-byte sidecar registered + * — a background agent launch, an automation observer, a draft-readiness + * probe. Before the fix, that sidecar's delivery interest made main's query + * authority yield ("the chunk was delivered, so a renderer xterm will answer + * it") to a renderer that had no xterm at all, and nobody answered. + * + * Real production code under test, in production's two stages: + * 1. the daemon Session's startup DA1 responder + query filter, released at + * shell-ready (session.ts) — WITHOUT it fish times out on its startup + * probe, latches "give up on DA1", and no later stall is observable; + * 2. the main-side reply-ownership predicate (shouldModelAnswerHiddenPtyQueries) + * driving a runtime HeadlessEmulator wired as createPtyHeadlessTerminalState + * wires it. + * Nothing here answers DA1 out of band — if stage 2's predicate says "not + * mine", fish waits out its 10s cap. + * + * The assertion is PROMPT LATENCY, the observable symptom: a user's parked fish + * pane taking 10s to redraw its prompt after every command. + */ +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import path from 'node:path' +import { afterEach, describe, expect, it } from 'vitest' +import { fishRequirementViolation, resolveFishBinary } from '../../shared/fish-binary-requirement' +import type { TerminalViewAttributes } from '../../shared/terminal-view-attributes' +import { HeadlessEmulator } from '../daemon/headless-emulator' +import { + installDeviceAttributesResponder, + StartupDeviceAttributesQueryFilter, + STARTUP_DA1_RESPONSE +} from '../daemon/startup-device-attributes-responder' +import { + _resetHiddenRendererPtyDeliveryGateForTest, + markHiddenRendererPty, + setRendererPtyDeliveryInterest +} from '../ipc/pty-hidden-delivery-gate' +import { shouldModelAnswerHiddenPtyQueries } from './terminal-model-query-authority' + +const FISH = resolveFishBinary(4) +const itWithFish = FISH.available ? it : it.skip + +const PTY_ID = 'fish-parked-pty' +const PROMPT_MARK = 'ORCA-DA1> ' +// Well under fish's 10s DA1 cap, and far above the ~30ms an answered prompt takes. +const PROMPT_LATENCY_BUDGET_MS = 1_000 +const PROMPT_WAIT_MS = 15_000 +const SETTINGS = { + terminalMainSideEffectAuthority: true, + terminalHiddenDeliveryGate: true, + terminalModelQueryAuthority: true +} as const + +const VIEW_ATTRIBUTES: TerminalViewAttributes = { + foreground: [238, 238, 238], + background: [17, 17, 17], + cursor: [238, 238, 238], + ansi: Array.from({ length: 256 }, () => [128, 128, 128] as [number, number, number]), + colorSchemeMode: 'dark', + cursorStyle: 'block', + cursorBlink: false +} + +const sleep = (ms: number): Promise => new Promise((resolve) => setTimeout(resolve, ms)) + +describe('a parked fish pane keeps painting prompts (DA1 must always have an answerer)', () => { + let configHome: string | null = null + let cleanup: (() => void) | null = null + + afterEach(() => { + cleanup?.() + cleanup = null + _resetHiddenRendererPtyDeliveryGateForTest() + if (configHome) { + rmSync(configHome, { recursive: true, force: true }) + configHome = null + } + }) + + // Always runs, so the CI lane cannot report green with the regression below skipped. + it('has the fish this suite needs when CI requires one', () => { + expect(fishRequirementViolation(FISH)).toBeNull() + }) + + itWithFish( + 'answers DA1 from the model while a sidecar holds delivery interest', + async () => { + const nodePty = await import('node-pty') + + configHome = mkdtempSync(path.join(tmpdir(), 'orca-fish-da1-')) + mkdirSync(path.join(configHome, 'fish'), { recursive: true }) + // Why a bare prompt: fish core re-probes DA1 on every paint regardless of + // what the prompt function prints, so nothing here shapes the measurement. + writeFileSync( + path.join(configHome, 'fish/config.fish'), + [ + 'set -g fish_greeting ""', + `function fish_prompt; printf '${PROMPT_MARK}'; end`, + 'function fish_right_prompt; end', + '' + ].join('\n') + ) + + const term = nodePty.spawn(FISH.path as string, ['-l', '-i'], { + name: 'xterm-256color', + cols: 120, + rows: 30, + cwd: configHome, + env: { + PATH: process.env.PATH ?? '/usr/bin:/bin', + HOME: configHome, + TERM: 'xterm-256color', + COLORTERM: 'truecolor', + LANG: 'en_US.UTF-8', + XDG_CONFIG_HOME: configHome, + XDG_DATA_HOME: path.join(configHome, 'data') + } + }) + + // Parked tab: the pane's xterm is unmounted (hidden mark) but a background + // agent / automation sidecar still consumes raw bytes (delivery interest). + markHiddenRendererPty(PTY_ID) + setRendererPtyDeliveryInterest(PTY_ID, true) + + // Stage 1 — daemon Session: answers DA1 past the shell-ready input queue and + // strips the query from downstream output, then hands DA1 back at ready. + const daemonEmulator = new HeadlessEmulator({ cols: 120, rows: 30 }) + const startupFilter = new StartupDeviceAttributesQueryFilter() + let releaseStartupResponder: (() => void) | null = installDeviceAttributesResponder({ + parser: daemonEmulator.responderParser, + response: STARTUP_DA1_RESPONSE, + reply: (data) => term.write(data) + }) + + // Stage 2 — main runtime model, the only post-startup answerer for a pane + // with no xterm. + const emulator = new HeadlessEmulator({ + cols: 120, + rows: 30, + onQueryReply: (reply) => term.write(reply) + }) + emulator.installViewAttributeResponder(() => VIEW_ATTRIBUTES) + emulator.applyPushedViewAttributes(VIEW_ATTRIBUTES) + + let promptCount = 0 + let writeChain: Promise = Promise.resolve() + term.onData((chunk) => { + void daemonEmulator.write(chunk) + const downstream = releaseStartupResponder ? startupFilter.accept(chunk) : chunk + // Mirrors OrcaRuntimeService.onPtyData: ownership is captured per chunk at + // ingestion and rides that chunk's write-chain link. + const forwardQueryReplies = shouldModelAnswerHiddenPtyQueries({ + ptyId: PTY_ID, + settings: SETTINGS, + hasRemoteViewSubscriber: false + }) + writeChain = writeChain.then(() => emulator.write(downstream, { forwardQueryReplies })) + promptCount += chunk.split(PROMPT_MARK).length - 1 + if (promptCount > 0 && releaseStartupResponder) { + // Shell-ready: the startup barrier is done, so DA1 goes back to whoever + // owns the view. Production releases here too (session.ts). + releaseStartupResponder() + releaseStartupResponder = null + } + }) + + cleanup = () => { + try { + term.kill() + } catch { + /* already gone */ + } + releaseStartupResponder?.() + emulator.dispose() + daemonEmulator.dispose() + } + + const waitForPromptCount = async (target: number): Promise => { + const deadline = Date.now() + PROMPT_WAIT_MS + while (Date.now() < deadline) { + if (promptCount >= target) { + return true + } + await sleep(10) + } + return false + } + + expect(await waitForPromptCount(1)).toBe(true) + + // Why three: the pre-fix failure is not the first prompt (the startup probe + // has its own responder in production) but every prompt after it. + const latenciesMs: number[] = [] + for (let round = 0; round < 3; round += 1) { + const target = promptCount + 1 + const startedAt = Date.now() + term.write('\r') + const painted = await waitForPromptCount(target) + latenciesMs.push(Date.now() - startedAt) + expect(painted).toBe(true) + } + + expect(Math.max(...latenciesMs)).toBeLessThan(PROMPT_LATENCY_BUDGET_MS) + }, + 120_000 + ) +}) diff --git a/src/main/runtime/rpc/methods/terminal.ts b/src/main/runtime/rpc/methods/terminal.ts index 32c31149f3f..be4dcf4f3e2 100644 --- a/src/main/runtime/rpc/methods/terminal.ts +++ b/src/main/runtime/rpc/methods/terminal.ts @@ -140,6 +140,11 @@ type TerminalMultiplexStream = { supportsOutputPause: boolean supportsWriteUnavailable: boolean outputPaused: boolean + /** Releases whichever presence this stream currently holds — view (query + * authority) while it receives output, raw-only while it is paused. */ + releaseViewPresence: () => void + /** null until this stream registers presence at all. */ + holdsViewAuthority: boolean | null supportsDesktopViewportClaims: boolean desktopClaimTail: Promise // Whether THIS stream registered the width driver, so detach won't release a peer stream's floor. @@ -414,6 +419,30 @@ function assertTerminalSendExactPtyBinding( throw new Error('terminal_guard_not_writable') } +/** + * Query authority follows what the client can actually SEE. A paused stream + * receives no bytes, so its remote xterm cannot answer DA1/DSR/OSC probes — + * holding view presence there leaves nobody answering and fish blocks ~10s at + * every prompt paint. Raw presence still pins the provider so unpausing + * resumes the same PTY. Both flags flip synchronously in main, so the model + * takes over exactly for the chunks the client never receives (no double + * answer, no gap). + */ +function setRemoteStreamViewAuthority( + runtime: OrcaRuntimeService, + stream: TerminalMultiplexStream, + holdsViewAuthority: boolean +): void { + if (stream.holdsViewAuthority === holdsViewAuthority) { + return + } + stream.holdsViewAuthority = holdsViewAuthority + stream.releaseViewPresence() + stream.releaseViewPresence = holdsViewAuthority + ? runtime.registerRemoteTerminalViewSubscriber(stream.ptyId) + : runtime.registerRawTerminalViewSubscriber(stream.ptyId) +} + function appendPendingMultiplexOutput( stream: TerminalMultiplexStream, data: string, @@ -2195,6 +2224,7 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ stream.ackPendingOutputBytes = 0 stream.ackPendingOutputOverflowed = false } + setRemoteStreamViewAuthority(runtime, stream, !stream.outputPaused) return } if (frame.opcode === TerminalStreamOpcode.Resize && stream.client) { @@ -2522,6 +2552,8 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ supportsOutputPause: request.capabilities?.outputPause === 1, supportsWriteUnavailable: request.capabilities?.writeUnavailable === 1, outputPaused: false, + releaseViewPresence: () => {}, + holdsViewAuthority: null, supportsDesktopViewportClaims: request.capabilities?.desktopViewportClaims === 1, desktopClaimTail: Promise.resolve(true), registeredRemoteDesktopDriver: false, @@ -2577,10 +2609,12 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ } stream.outputBatcher.push(data, meta) }) - // Why: a multiplexed stream feeds a remote xterm view with query authority, so the main model responder yields while attached (terminal-query-authority.md). - const releaseViewSubscriber = runtime.registerRemoteTerminalViewSubscriber(ptyId) + // Why: a multiplexed stream feeds a remote xterm view with query authority, so the main model responder yields while attached (terminal-query-authority.md) — until the client pauses output, which hands authority back. + setRemoteStreamViewAuthority(runtime, stream, !stream.outputPaused) stream.unsubscribeData = () => { - releaseViewSubscriber() + stream.releaseViewPresence() + stream.releaseViewPresence = () => {} + stream.holdsViewAuthority = null unsubscribeStreamData() } diff --git a/src/main/runtime/rpc/terminal-multiplex.test.ts b/src/main/runtime/rpc/terminal-multiplex.test.ts index 95ad62ec18c..bb488bb5ab7 100644 --- a/src/main/runtime/rpc/terminal-multiplex.test.ts +++ b/src/main/runtime/rpc/terminal-multiplex.test.ts @@ -34,6 +34,8 @@ function stubRuntime(overrides: Partial = {}): OrcaRuntimeSe // Why: every multiplex stream registers as a remote view subscriber for // Phase-5 query-authority suppression (terminal-query-authority.md). registerRemoteTerminalViewSubscriber: () => () => {}, + // Why: a paused stream swaps view presence for raw presence — it can no longer answer queries. + registerRawTerminalViewSubscriber: () => () => {}, // Why: the multiplex subscribe path resolves handles via // resolveLiveLeafForHandle (#7718). Default to a live pty so tests that // only stub the legacy resolveLeafForHandle still bind; tests that need a @@ -4556,6 +4558,50 @@ describe('terminal multiplex RPC', () => { await harness.dispatchPromise }) + // Why: a paused stream receives no bytes, so its remote xterm cannot answer + // DA1/DSR/OSC probes. Holding view presence there left NOBODY answering and a + // remote fish pane blocked ~10s at every prompt paint. + it('hands query authority back to the model while a stream pauses output', async () => { + const releaseView = vi.fn() + const releaseRaw = vi.fn() + const registerRemoteTerminalViewSubscriber = vi.fn().mockReturnValue(releaseView) + const registerRawTerminalViewSubscriber = vi.fn().mockReturnValue(releaseRaw) + const harness = startDesktopMultiplexSubscribe({ + registerRemoteTerminalViewSubscriber, + registerRawTerminalViewSubscriber + }) + await vi.waitFor(() => + expect(harness.messages.some((msg) => JSON.parse(msg).result?.type === 'ready')).toBe(true) + ) + sendDesktopMultiplexSubscribe(harness.handlers, { ackOutput: 1, outputPause: 1 }) + await vi.waitFor(() => expect(registerRemoteTerminalViewSubscriber).toHaveBeenCalledTimes(1)) + + const setPaused = (paused: boolean, seq: number): void => { + harness.handlers.get(7)?.( + decodeTerminalStreamFrame( + encodeTerminalStreamFrame({ + opcode: SET_OUTPUT_PAUSED_OPCODE, + streamId: 7, + seq, + payload: encodeTerminalStreamJson({ paused }) + }) + )! + ) + } + + setPaused(true, 2) + expect(releaseView).toHaveBeenCalledTimes(1) + // Raw presence still pins the provider, without claiming reply ownership. + expect(registerRawTerminalViewSubscriber).toHaveBeenCalledTimes(1) + + setPaused(false, 3) + expect(releaseRaw).toHaveBeenCalledTimes(1) + expect(registerRemoteTerminalViewSubscriber).toHaveBeenCalledTimes(2) + + harness.cleanups.get('terminal-multiplex:conn-desktop-first-paint')?.() + await harness.dispatchPromise + }) + it('cancels a pending desktop PTY wait when its multiplex slot unsubscribes', async () => { let resolvePty: (ptyId: string) => void = () => {} let waitSignal: AbortSignal | undefined diff --git a/src/main/runtime/terminal-model-query-authority.test.ts b/src/main/runtime/terminal-model-query-authority.test.ts index 3217a98022e..6741c6733af 100644 --- a/src/main/runtime/terminal-model-query-authority.test.ts +++ b/src/main/runtime/terminal-model-query-authority.test.ts @@ -57,17 +57,20 @@ describe('shouldModelAnswerHiddenPtyQueries', () => { hasRemoteViewSubscriber: false }) - it('answers only for hidden-marked PTYs (the delivery decision is the reply decision)', () => { + it('answers only for hidden-marked PTYs (the view-delivery decision is the reply decision)', () => { expect(answer('pty-1')).toBe(false) markHiddenRendererPty('pty-1') expect(answer('pty-1')).toBe(true) expect(answer('pty-other')).toBe(false) }) - it('yields to registered renderer delivery interest (chunk is delivered to a sidecar)', () => { + // Regression: a sidecar is not an xterm. Yielding to delivery interest left a + // parked pane (xterm unmounted) with a live sidecar unanswered — fish then + // blocked ~10s on DA1 at every prompt paint, forever. + it('keeps answering when only a raw-byte sidecar holds delivery interest', () => { markHiddenRendererPty('pty-1') setRendererPtyDeliveryInterest('pty-1', true) - expect(answer('pty-1')).toBe(false) + expect(answer('pty-1')).toBe(true) setRendererPtyDeliveryInterest('pty-1', false) expect(answer('pty-1')).toBe(true) }) diff --git a/src/main/runtime/terminal-model-query-authority.ts b/src/main/runtime/terminal-model-query-authority.ts index df8f05798a4..ccfef6c60d0 100644 --- a/src/main/runtime/terminal-model-query-authority.ts +++ b/src/main/runtime/terminal-model-query-authority.ts @@ -2,17 +2,19 @@ * Phase 5 of the terminal model/view architecture: main-side terminal query * authority. * - * The delivery decision is the reply decision: main answers a query iff the - * hidden-delivery gate dropped the chunk that carried it. This module owns - * the responder kill-switch predicate and the main-side mirror of the - * renderer's native-Windows-ConPTY determination, recorded per PTY at spawn - * so the runtime emulator can register the DA1 override before byte zero. + * The VIEW delivery decision is the reply decision: main answers a query iff + * the hidden-delivery gate withheld the chunk from every renderer view — a + * hidden pane's xterm is starved, a parked pane has none, and a sidecar-only + * chunk reaches raw-byte watchers that never reply. This module owns the + * responder kill-switch predicate and the main-side mirror of the renderer's + * native-Windows-ConPTY determination, recorded per PTY at spawn so the + * runtime emulator can register the DA1 override before byte zero. */ import type { GlobalSettings } from '../../shared/types' import { isWslUncPath } from '../../shared/wsl-paths' import { isHiddenPtyDeliveryGateEnabled, - shouldDropHiddenRendererPtyData + shouldSuppressHiddenRendererPtyView } from '../ipc/pty-hidden-delivery-gate' export type TerminalModelQueryAuthoritySettings = Pick< @@ -30,10 +32,10 @@ export function isTerminalModelQueryAuthorityEnabled( /** Per-chunk reply-ownership predicate, evaluated once at ingestion in * OrcaRuntimeService.onPtyData — the same module state and tick as the - * hidden-gate drop sites, so "chunk dropped" and "main answers" cannot - * diverge for live chunks. Remote view subscribers (mobile/web/remote - * desktop xterms on the multiplexed stream) keep view authority, so main - * yields while one is attached. */ + * hidden-gate suppression sites, so "no view saw the chunk" and "main + * answers" cannot diverge for live chunks. Remote view subscribers + * (mobile/web/remote desktop xterms on the multiplexed stream) keep view + * authority, so main yields while one is attached and unpaused. */ export function shouldModelAnswerHiddenPtyQueries(opts: { ptyId: string settings: TerminalModelQueryAuthoritySettings | null | undefined @@ -42,7 +44,7 @@ export function shouldModelAnswerHiddenPtyQueries(opts: { return ( isTerminalModelQueryAuthorityEnabled(opts.settings) && !opts.hasRemoteViewSubscriber && - shouldDropHiddenRendererPtyData(opts.ptyId, opts.settings) + shouldSuppressHiddenRendererPtyView(opts.ptyId, opts.settings) ) } diff --git a/src/main/runtime/terminal-query-responder.test.ts b/src/main/runtime/terminal-query-responder.test.ts index cdcd10dfbc3..ddee0f11ca8 100644 --- a/src/main/runtime/terminal-query-responder.test.ts +++ b/src/main/runtime/terminal-query-responder.test.ts @@ -190,7 +190,10 @@ describe('reply ownership matrix', () => { expect(replies).toEqual([]) }) - it('never answers while renderer delivery interest holds the chunk delivered', async () => { + // Regression: delivery interest routes bytes to raw-byte sidecars, never to a + // view — a parked pane has no xterm at all, so yielding here left DA1 + // unanswered and fish blocked ~10s at every prompt paint. + it('still answers while renderer delivery interest holds — a sidecar is not an xterm', async () => { const { runtime, replies } = createResponderRuntime() markHiddenRendererPty('pty-i') setRendererPtyDeliveryInterest('pty-i', true) @@ -198,7 +201,7 @@ describe('reply ownership matrix', () => { runtime.onPtyData('pty-i', DA1, Date.now()) await settle(runtime, 'pty-i') - expect(replies).toEqual([]) + expect(replies).toEqual([{ ptyId: 'pty-i', data: '\x1b[?1;2c' }]) }) it.each([ @@ -793,7 +796,7 @@ describe('view-attribute replay guard and suppression', () => { expect(replies).toEqual([]) }) - it('never answers while renderer delivery interest holds the chunk delivered', async () => { + it('answers a view-attribute query while only a sidecar holds delivery interest', async () => { const { runtime, replies } = createResponderRuntime() markHiddenRendererPty('pty-vint') setRendererPtyDeliveryInterest('pty-vint', true) @@ -802,7 +805,8 @@ describe('view-attribute replay guard and suppression', () => { runtime.onPtyData('pty-vint', '\x1b]11;?\x07', Date.now()) await settle(runtime, 'pty-vint') - expect(replies).toEqual([]) + expect(replies).toHaveLength(1) + expect(replies[0]?.ptyId).toBe('pty-vint') }) it('yields view-attribute replies while a remote view subscriber is attached', async () => { diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index 0f90182d053..24555ca239e 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -1627,6 +1627,9 @@ export type PreloadApi = { transformed?: boolean background?: boolean droppedOutput?: boolean + /** Hidden-gated bytes for raw-byte sidecars only; the view must not + * render or reply to them (main answered their queries). */ + sidecarOnly?: boolean }) => void ) => () => void onReplay: (callback: (data: { id: string; data: string }) => void) => () => void diff --git a/src/preload/index.ts b/src/preload/index.ts index 58aec899f03..394dcfac7eb 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -1175,6 +1175,7 @@ const api = { transformed?: boolean background?: boolean droppedOutput?: boolean + sidecarOnly?: boolean } ) => callback(data) ipcRenderer.on('pty:data', listener) diff --git a/src/renderer/src/components/terminal-pane/pty-data-delivery-routing.test.ts b/src/renderer/src/components/terminal-pane/pty-data-delivery-routing.test.ts new file mode 100644 index 00000000000..2104ed1b1d9 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/pty-data-delivery-routing.test.ts @@ -0,0 +1,64 @@ +// Why: a sidecar-flagged chunk must reach raw-byte watchers and NOTHING else — +// the view already restores from main's model snapshot, and letting a mounted +// xterm parse it would put a second DA1/OSC reply on the shell's stdin. +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { routeDispatchedPtyData } from './pty-data-delivery-routing' +import { ptyDataHandlers, ptyDataSidecars } from './pty-shutdown-data-suspension' +import { drainPreHandlerPtyData, clearPreHandlerPtyState } from './pty-pre-handler-buffer' + +const PTY_ID = 'pty-sidecar-only' +const originalWindow = (globalThis as { window?: typeof window }).window + +describe('routeDispatchedPtyData', () => { + beforeEach(() => { + ;(globalThis as { window: unknown }).window = { + api: { pty: { ackData: vi.fn() } } + } + }) + + afterEach(() => { + ptyDataHandlers.delete(PTY_ID) + ptyDataSidecars.delete(PTY_ID) + clearPreHandlerPtyState(PTY_ID) + if (originalWindow) { + ;(globalThis as { window: typeof window }).window = originalWindow + } else { + delete (globalThis as { window?: unknown }).window + } + }) + + it('feeds both the primary handler and sidecars for ordinary chunks', () => { + const primary = vi.fn() + const sidecar = vi.fn() + ptyDataHandlers.set(PTY_ID, primary) + ptyDataSidecars.set(PTY_ID, new Set([sidecar])) + + routeDispatchedPtyData({ id: PTY_ID, data: 'hello' }) + + expect(primary).toHaveBeenCalledWith('hello', undefined) + expect(sidecar).toHaveBeenCalledWith('hello') + }) + + it('feeds only sidecars when main flags the chunk sidecar-only', () => { + const primary = vi.fn() + const sidecar = vi.fn() + ptyDataHandlers.set(PTY_ID, primary) + ptyDataSidecars.set(PTY_ID, new Set([sidecar])) + + routeDispatchedPtyData({ id: PTY_ID, data: '\x1b[0c', sidecarOnly: true }) + + expect(primary).not.toHaveBeenCalled() + expect(sidecar).toHaveBeenCalledWith('\x1b[0c') + }) + + it('does not stash a sidecar-only chunk for a pane that mounts later', () => { + const sidecar = vi.fn() + ptyDataSidecars.set(PTY_ID, new Set([sidecar])) + + routeDispatchedPtyData({ id: PTY_ID, data: '\x1b[0c', sidecarOnly: true }) + + const lateHandler = vi.fn() + drainPreHandlerPtyData(PTY_ID, lateHandler) + expect(lateHandler).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/components/terminal-pane/pty-data-delivery-routing.ts b/src/renderer/src/components/terminal-pane/pty-data-delivery-routing.ts new file mode 100644 index 00000000000..f0b9cb92d41 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/pty-data-delivery-routing.ts @@ -0,0 +1,110 @@ +/** + * Routes one inbound `pty:data` payload to its renderer consumers: the primary + * (xterm / eager-buffer) handler and the raw-byte sidecars. + * + * Split from the dispatcher because the two consumers no longer receive the + * same chunks — main flags hidden-gated bytes `sidecarOnly`, and the view must + * neither render nor reply to those (main answered their queries). + */ +import { deliverPtyDataWithDeferredAck } from './terminal-pty-ack-gate' +import { bufferPreHandlerPtyData } from './pty-pre-handler-buffer' +import { recordPtyDataReceived } from './terminal-delivery-watchdog' +import { + bufferPtyShutdownData, + isPtyDataHandlerShutdownPending, + ptyDataHandlers, + ptyDataSidecars +} from './pty-shutdown-data-suspension' + +export type PtyDataMeta = { + seq?: number + rawLength?: number + transformed?: boolean + background?: boolean + /** Main dropped this PTY's buffered output at the pending cap; repaint from the main-owned snapshot, not the live stream. */ + droppedOutput?: boolean + /** Hidden-gated bytes forwarded for raw-byte sidecars only; no view may render or reply to them (main owns the query reply). */ + sidecarOnly?: boolean +} + +export type PtyDataPushPayload = { + id: string + data: string + seq?: number + rawLength?: number + transformed?: boolean + background?: boolean + droppedOutput?: boolean + sidecarOnly?: boolean +} + +function ptyDataMetaFromPayload(payload: PtyDataPushPayload): PtyDataMeta | undefined { + let meta: PtyDataMeta | undefined + if (typeof payload.seq === 'number') { + meta ??= {} + meta.seq = payload.seq + } + if (typeof payload.rawLength === 'number') { + meta ??= {} + meta.rawLength = payload.rawLength + } + if (payload.transformed === true) { + meta ??= {} + meta.transformed = true + } + if (payload.background === true) { + meta ??= {} + meta.background = true + } + if (payload.droppedOutput === true) { + meta ??= {} + meta.droppedOutput = true + } + if (payload.sidecarOnly === true) { + meta ??= {} + meta.sidecarOnly = true + } + return meta +} + +function dispatchToPrimaryPtyDataHandler( + ptyId: string, + data: string, + meta: PtyDataMeta | undefined +): void { + const handler = ptyDataHandlers.get(ptyId) + if (handler) { + handler(data, meta) + return + } + bufferPreHandlerPtyData(ptyId, data, meta) +} + +export function routeDispatchedPtyData(payload: PtyDataPushPayload): void { + const meta = ptyDataMetaFromPayload(payload) + const chars = payload.rawLength ?? payload.data.length + const dispatch = (): void => { + if (isPtyDataHandlerShutdownPending(payload.id)) { + // Why: teardown output is speculative until the owner verifies sleep; retain it (sidecars included) so a failed attempt resumes without losing terminal data. + bufferPtyShutdownData(payload.id, payload.data, meta) + return + } + // Why skipped: main withheld these bytes from every view and answered their + // queries itself, so feeding a (hidden or stale) xterm would repaint a gapped + // stream and put a second DA1/OSC reply on the shell's stdin. + if (payload.sidecarOnly !== true) { + dispatchToPrimaryPtyDataHandler(payload.id, payload.data, meta) + } + const sidecars = ptyDataSidecars.get(payload.id) + if (sidecars && sidecars.size > 0) { + // Why: snapshot before iterating — watchers often unsubscribe (or subscribe siblings) mid-iteration, and mutating the live Set would skip or double-fire. + const snapshot = Array.from(sidecars) + for (const watcher of snapshot) { + watcher(payload.data) + } + } + } + recordPtyDataReceived(payload.id, chars) + // Why deferred: main budgets by bytes PARSED not received; ACK fires when xterm consumes, and undelivered chunks settle at return so no PTY stays backpressured. + deliverPtyDataWithDeferredAck(payload.id, chars, dispatch) +} diff --git a/src/renderer/src/components/terminal-pane/pty-dispatcher.ts b/src/renderer/src/components/terminal-pane/pty-dispatcher.ts index 73f756c0857..93d3d838e12 100644 --- a/src/renderer/src/components/terminal-pane/pty-dispatcher.ts +++ b/src/renderer/src/components/terminal-pane/pty-dispatcher.ts @@ -2,13 +2,11 @@ import { TERMINAL_SCROLLBACK_SESSION_BUFFER_BYTE_LIMIT } from '../../../../shared/terminal-scrollback-limits' import { clearProcessedPtyCharTotal, - deliverPtyDataWithDeferredAck, exposeE2eTerminalPtyAckGate, getProcessedPtyCharTotals } from './terminal-pty-ack-gate' import { clampUtf8Tail, type EagerBufferChunk } from './pty-eager-buffer-clamp' import { - bufferPreHandlerPtyData, clearPreHandlerPtyState, drainPreHandlerPtyData, drainPreHandlerPtyExit @@ -17,22 +15,21 @@ import { deliverPtyExitToHandlers } from './pty-exit-delivery' import { clearReceivedPtyCharTotal, isPtyPushDeliveryBlackholed, - recordPtyDataReceived, startTerminalDeliveryWatchdog } from './terminal-delivery-watchdog' import { recordTerminalFreezeBreadcrumb } from './terminal-freeze-breadcrumbs' import { installTerminalFreezeReport } from './terminal-freeze-report' import { - bufferPtyShutdownData, bufferPtyShutdownReplayData, - isPtyDataHandlerShutdownPending, ptyDataHandlers, - ptyDataSidecars, ptyExitHandlers, ptyReplayHandlers } from './pty-shutdown-data-suspension' +import { routeDispatchedPtyData } from './pty-data-delivery-routing' import { markCommittedPtyShutdowns } from './pty-shutdown-exit-deferral' +export { type PtyDataMeta } from './pty-data-delivery-routing' + export { ptyDataHandlers, ptyDataSidecars, @@ -49,15 +46,6 @@ export { // ── Singleton PTY event dispatcher ─────────────────────────────────── // One global IPC listener per channel (routed by PTY ID) avoids the N-listener MaxListenersExceededWarning with many panes. -export type PtyDataMeta = { - seq?: number - rawLength?: number - transformed?: boolean - background?: boolean - /** Main dropped this PTY's buffered output at the pending cap; repaint from the main-owned snapshot, not the live stream. */ - droppedOutput?: boolean -} - /** Sidecar PTY-data observers, invoked AFTER the primary handler so a side-effect-only watcher can't delay xterm rendering. */ /** Per-PTY replay handlers on a dedicated pty:replay channel so the renderer can engage the replay guard and suppress xterm auto-replies. */ const ptyExitSidecars = new Map< @@ -104,69 +92,12 @@ function attachPtyPushListeners(): void { if (isPtyPushDeliveryBlackholed()) { return } - handleDispatchedPtyData(payload) + routeDispatchedPtyData(payload) }) ) attachPtySecondaryPushListeners(unsubscribes) } -function handleDispatchedPtyData(payload: { - id: string - data: string - seq?: number - rawLength?: number - transformed?: boolean - background?: boolean - droppedOutput?: boolean -}): void { - let meta: PtyDataMeta | undefined - if (typeof payload.seq === 'number') { - meta ??= {} - meta.seq = payload.seq - } - if (typeof payload.rawLength === 'number') { - meta ??= {} - meta.rawLength = payload.rawLength - } - if (payload.transformed === true) { - meta ??= {} - meta.transformed = true - } - if (payload.background === true) { - meta ??= {} - meta.background = true - } - if (payload.droppedOutput === true) { - meta ??= {} - meta.droppedOutput = true - } - const chars = payload.rawLength ?? payload.data.length - const dispatch = (): void => { - if (isPtyDataHandlerShutdownPending(payload.id)) { - // Why: teardown output is speculative until the owner verifies sleep; retain it so a failed attempt resumes without losing terminal data. - bufferPtyShutdownData(payload.id, payload.data, meta) - return - } - const handler = ptyDataHandlers.get(payload.id) - if (handler) { - handler(payload.data, meta) - } else { - bufferPreHandlerPtyData(payload.id, payload.data, meta) - } - const sidecars = ptyDataSidecars.get(payload.id) - if (sidecars && sidecars.size > 0) { - // Why: snapshot before iterating — watchers often unsubscribe (or subscribe siblings) mid-iteration, and mutating the live Set would skip or double-fire. - const snapshot = Array.from(sidecars) - for (const watcher of snapshot) { - watcher(payload.data) - } - } - } - recordPtyDataReceived(payload.id, chars) - // Why deferred: main budgets by bytes PARSED not received; ACK fires when xterm consumes, and undelivered chunks settle at return so no PTY stays backpressured. - deliverPtyDataWithDeferredAck(payload.id, chars, dispatch) -} - function attachPtySecondaryPushListeners(unsubscribes: (() => void)[]): void { const unsubscribeWriteUnavailable = window.api.pty.onWriteUnavailable?.((payload) => { ptyWriteUnavailableHandlers.get(payload.id)?.() diff --git a/src/renderer/src/components/terminal-pane/pty-shutdown-data-suspension.ts b/src/renderer/src/components/terminal-pane/pty-shutdown-data-suspension.ts index 508ea88b2be..abd3ef5d8cd 100644 --- a/src/renderer/src/components/terminal-pane/pty-shutdown-data-suspension.ts +++ b/src/renderer/src/components/terminal-pane/pty-shutdown-data-suspension.ts @@ -189,7 +189,11 @@ function deliverShutdownEvent( replayHandler(event.data) return } - dataHandler(event.data, event.meta) + // Why conditional: a sidecar-only chunk was withheld from the view and already + // answered by main; a rollback must not hand it to the pane's xterm. + if (event.meta?.sidecarOnly !== true) { + dataHandler(event.data, event.meta) + } for (const sidecar of Array.from(ptyDataSidecars.get(ptyId) ?? [])) { sidecar(event.data) }