From ca9deedb915d529933d8d893042f3d06348ba7b2 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 12 Aug 2026 20:21:44 -0700 Subject: [PATCH] fix(terminal): preserve query authority across pauses --- .../ipc/pty-pending-data-drain-contract.ts | 7 + src/main/ipc/pty.test.ts | 152 ++++++++++++ src/main/ipc/pty.ts | 225 ++++++++++++++---- src/main/runtime/orca-runtime.ts | 19 +- src/main/runtime/rpc/methods/terminal.ts | 29 +-- .../runtime/rpc/terminal-multiplex.test.ts | 120 ++++++++++ .../terminal-model-query-authority.test.ts | 21 ++ .../runtime/terminal-model-query-authority.ts | 5 +- .../runtime/terminal-query-responder.test.ts | 15 ++ 9 files changed, 526 insertions(+), 67 deletions(-) diff --git a/src/main/ipc/pty-pending-data-drain-contract.ts b/src/main/ipc/pty-pending-data-drain-contract.ts index 0a9874239d6..39bb597d31c 100644 --- a/src/main/ipc/pty-pending-data-drain-contract.ts +++ b/src/main/ipc/pty-pending-data-drain-contract.ts @@ -10,6 +10,13 @@ export type PendingPtyData = { /** 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 + /** Authority transitions within data/rawLength; delivery never crosses one. */ + viewSuppressionBoundaries?: { + dataOffset: number + rawOffset: number + viewSuppressed: boolean + }[] + emitViewGapRestoreMarker?: 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 5736be18c99..7f9593b9ff3 100644 --- a/src/main/ipc/pty.test.ts +++ b/src/main/ipc/pty.test.ts @@ -14755,6 +14755,45 @@ describe('registerPtyHandlers', () => { } }) + it('keeps a visible dropped sentinel out of xterm after hidden authority takes over', async () => { + vi.useFakeTimers() + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + const mockProc = createMockProc() + spawnMock.mockReturnValue(mockProc.proc) + + try { + registerPtyHandlers(mainWindow as never) + const spawn = (await handlers.get('pty:spawn')!(null, { + cols: 80, + rows: 24, + cwd: '/tmp' + })) as { id: string } + const ackData = getPtyAckDataListener() + const setHidden = getPtySetHiddenRendererPtyListener() + const setInterest = getPtySetDeliveryInterestListener() + + mockProc.emitData('x'.repeat(600 * 1024)) + vi.advanceTimersByTime(34) + mockProc.emitData(`${'y'.repeat(3 * 1024 * 1024)}\x1b[6n`) + setInterest(null, { id: spawn.id, interested: true }) + setHidden(null, { id: spawn.id, hidden: true }) + mockProc.emitData('\x1b[0c') + + mainWindow.webContents.send.mockClear() + ackData(null, { id: spawn.id, charCount: 512 * 1024 }) + vi.advanceTimersByTime(2) + expect(mainWindow.webContents.send).toHaveBeenCalledWith('pty:data', { + id: spawn.id, + data: '\x1b[6n\x1b[0c', + droppedOutput: true, + sidecarOnly: true + }) + } finally { + errorSpy.mockRestore() + vi.useRealTimers() + } + }) + it('scales the pending-output cap with the scrollback setting', async () => { vi.useFakeTimers() const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) @@ -16788,6 +16827,119 @@ describe('registerPtyHandlers', () => { ) expect(dataSends).toHaveLength(1) expect(dataSends[0][1]).toMatchObject({ data: 'parked bytes', sidecarOnly: true }) + + mainWindow.webContents.send.mockClear() + setHidden(null, { id: spawnResult.id, hidden: true }) + setHidden(null, { id: spawnResult.id, hidden: false }) + expect(mainWindow.webContents.send).not.toHaveBeenCalledWith( + 'pty:modelRestoreNeeded', + expect.anything() + ) + } finally { + vi.useRealTimers() + } + }) + + it.each([ + { + direction: 'hidden to visible', + arrange: ( + setHidden: ReturnType, + setInterest: ReturnType, + id: string + ) => { + setHidden(null, { id, hidden: true }) + setInterest(null, { id, interested: true }) + }, + transition: ( + setHidden: ReturnType, + _setInterest: ReturnType, + id: string + ) => setHidden(null, { id, hidden: false }), + firstSidecarOnly: true, + secondSidecarOnly: false + }, + { + direction: 'visible to hidden', + arrange: () => {}, + transition: ( + setHidden: ReturnType, + setInterest: ReturnType, + id: string + ) => { + setInterest(null, { id, interested: true }) + setHidden(null, { id, hidden: true }) + }, + firstSidecarOnly: false, + secondSidecarOnly: true + } + ])('splits queued $direction view authority', async (scenario) => { + 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() + scenario.arrange(setHidden, setInterest, spawnResult.id) + mainWindow.webContents.send.mockClear() + + mockProc.emitData('first-query\x1b[c') + scenario.transition(setHidden, setInterest, spawnResult.id) + mockProc.emitData('second-query\x1b[c') + vi.advanceTimersByTime(50) + + const payloads = mainWindow.webContents.send.mock.calls + .filter((call: unknown[]) => call[0] === 'pty:data') + .map((call: unknown[]) => call[1] as { data: string; sidecarOnly?: boolean }) + expect(payloads.map(({ data }) => data)).toEqual([ + 'first-query\x1b[c', + 'second-query\x1b[c' + ]) + expect(payloads.map(({ sidecarOnly }) => sidecarOnly === true)).toEqual([ + scenario.firstSidecarOnly, + scenario.secondSidecarOnly + ]) + } finally { + vi.useRealTimers() + } + }) + + it('bounds queued view-authority transitions with the snapshot sentinel', 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() + getPtySetDeliveryInterestListener()(null, { id: spawnResult.id, interested: true }) + + for (let index = 0; index < 258; index++) { + setHidden(null, { id: spawnResult.id, hidden: index % 2 === 0 }) + mockProc.emitData('x') + } + mainWindow.webContents.send.mockClear() + vi.advanceTimersByTime(50) + + expect(mainWindow.webContents.send).toHaveBeenCalledTimes(1) + expect(mainWindow.webContents.send).toHaveBeenCalledWith('pty:data', { + id: spawnResult.id, + data: '', + droppedOutput: true, + sidecarOnly: true + }) } finally { vi.useRealTimers() } diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index 16168ad5cc9..b0a9b6c95c7 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -2568,6 +2568,8 @@ export function registerPtyHandlers( const PTY_BATCH_DRAIN_CONTINUE_MS = 1 const PTY_BATCH_FLUSH_CHUNK_CHARS = 16 * 1024 const PTY_BATCH_FLUSH_MAX_WRITES = 2 + // Bounds metadata under pathological visibility churn; overflow heals through the snapshot sentinel. + const PTY_PENDING_VIEW_AUTHORITY_BOUNDARY_MAX = 256 const PTY_RENDERER_IN_FLIGHT_HIGH_WATER_CHARS = 512 * 1024 const PTY_RENDERER_TOTAL_IN_FLIGHT_HIGH_WATER_CHARS = 8 * 1024 * 1024 // Why: cap unbounded pendingData growth when the renderer can't receive (frozen/reloading); beyond it bytes drop and the pane heals from the main buffer snapshot via the droppedOutput sentinel (#7630). @@ -3081,12 +3083,6 @@ 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 @@ -3238,20 +3234,22 @@ export function registerPtyHandlers( return (pending.droppedMode2031Data ?? '') + pendingSubscribe + state.tail } - function dropOversizedPendingPtyData(id: string, pending: PendingPtyData): PendingPtyData { + function dropPendingPtyData( + id: string, + pending: PendingPtyData, + reason: 'pending-cap' | 'view-authority-churn' + ): PendingPtyData { const capChars = pendingDataCapChars() - if (pending.droppedOutput === true || pending.data.length <= capChars) { - return pending - } if (!pendingDataDropWarnedPtys.has(id)) { pendingDataDropWarnedPtys.add(id) console.error( - `[pty] dropped ${pending.data.length} buffered chars for ${id}: renderer not receiving and per-PTY pending cap exceeded; pane will restore from the main-owned snapshot` + `[pty] dropped ${pending.data.length} buffered chars for ${id}: ${reason}; pane will restore from the main-owned snapshot` ) // Why: field visibility for cap tuning (issue #2836 / #7017); no pty id since session ids can embed workspace paths. recordCrashBreadcrumb('terminal_pending_output_dropped', { droppedChars: pending.data.length, - capChars + capChars, + reason }) } if (isHiddenPtyDeliveryGateEnabled(getSettings?.()) && !pendingOverflowMarkedPtys.has(id)) { @@ -3266,12 +3264,22 @@ export function registerPtyHandlers( return { data: extractDroppedPtyQueryBytes(pending.data).slice(0, DROPPED_QUERY_SALVAGE_MAX_CHARS), droppedOutput: true, - ...(pending.viewSuppressed === true ? { viewSuppressed: true as const } : {}), + ...(pending.viewSuppressed === true || + pending.viewSuppressionBoundaries?.some((boundary) => boundary.viewSuppressed) + ? { viewSuppressed: true as const } + : {}), droppedMode2031Data: mode2031.data, droppedMode2031ScanState: mode2031.state } } + function dropOversizedPendingPtyData(id: string, pending: PendingPtyData): PendingPtyData { + if (pending.droppedOutput === true || pending.data.length <= pendingDataCapChars()) { + return pending + } + return dropPendingPtyData(id, pending, 'pending-cap') + } + function updatePendingProjectionAdmissions( pending: PendingPtyData, state: PendingProjectionAdmissions @@ -3315,7 +3323,8 @@ export function registerPtyHandlers( rawLength = data.length, transformed = false, projectionSemanticsId?: string, - viewSuppressed = false + viewSuppressed = false, + emitViewGapRestoreMarker = 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) { @@ -3334,6 +3343,8 @@ export function registerPtyHandlers( return { ...existing, data: existing.data + salvaged, + ...(viewSuppressed ? { viewSuppressed: true as const } : {}), + ...(emitViewGapRestoreMarker ? { emitViewGapRestoreMarker: true as const } : {}), droppedMode2031Data: mode2031.data || existing.droppedMode2031Data, droppedMode2031ScanState: mode2031.state } @@ -3341,9 +3352,6 @@ 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, @@ -3351,27 +3359,120 @@ export function registerPtyHandlers( ...(rawLength !== data.length ? { rawLength } : {}), ...(transformed ? { transformed: true } : {}), ...(nextContainsBackgroundOutput ? { containsBackgroundOutput: true } : {}), - ...(nextViewSuppressed ? { viewSuppressed: true as const } : {}) + ...(viewSuppressed ? { viewSuppressed: true as const } : {}), + ...(emitViewGapRestoreMarker ? { emitViewGapRestoreMarker: true as const } : {}) } updatePendingProjectionAdmissions(pending, projectionState) return dropOversizedPendingPtyData(id, pending) } const existingRawLength = existing.rawLength ?? existing.data.length + const boundaries = existing.viewSuppressionBoundaries + ? existing.viewSuppressionBoundaries.slice() + : [] + const previousViewSuppressed = + boundaries.at(-1)?.viewSuppressed ?? existing.viewSuppressed === true + if (previousViewSuppressed !== viewSuppressed) { + boundaries.push({ + dataOffset: existing.data.length, + rawOffset: existingRawLength, + viewSuppressed + }) + } const next: PendingPtyData = { data: existing.data + data, ...(!preservesSeq || existing.transformed || transformed ? { rawLength: existingRawLength + rawLength, transformed: true as const } : {}), ...(nextContainsBackgroundOutput ? { containsBackgroundOutput: true } : {}), - ...(nextViewSuppressed ? { viewSuppressed: true as const } : {}) + ...(existing.viewSuppressed === true ? { viewSuppressed: true as const } : {}), + ...(boundaries.length > 0 ? { viewSuppressionBoundaries: boundaries } : {}), + ...(existing.emitViewGapRestoreMarker === true || emitViewGapRestoreMarker + ? { emitViewGapRestoreMarker: true as const } + : {}) } updatePendingProjectionAdmissions(next, projectionState) if (typeof existing.startSeq === 'number') { next.startSeq = existing.startSeq } + if (boundaries.length > PTY_PENDING_VIEW_AUTHORITY_BOUNDARY_MAX) { + return dropPendingPtyData(id, next, 'view-authority-churn') + } return dropOversizedPendingPtyData(id, next) } + function takePendingPtyDeliverySlice(pending: PendingPtyData): { + data: string + rawLength: number + transformed: boolean + viewSuppressed: boolean + remainder: PendingPtyData | undefined + } { + const boundary = pending.viewSuppressionBoundaries?.[0] + const dataLimit = boundary?.dataOffset ?? pending.data.length + const dataLength = pending.transformed + ? dataLimit + : Math.min(dataLimit, PTY_BATCH_FLUSH_CHUNK_CHARS) + const rawLength = + boundary && dataLength === boundary.dataOffset + ? boundary.rawOffset + : pending.transformed + ? (pending.rawLength ?? pending.data.length) + : dataLength + const data = pending.data.slice(0, dataLength) + const remainingData = pending.data.slice(dataLength) + if (!remainingData) { + return { + data, + rawLength, + transformed: pending.transformed === true, + viewSuppressed: pending.viewSuppressed === true, + remainder: undefined + } + } + const remainingRawLength = (pending.rawLength ?? pending.data.length) - rawLength + const remainingBoundaries = pending.viewSuppressionBoundaries + ?.filter((entry) => entry.dataOffset > dataLength) + .map((entry) => ({ + dataOffset: entry.dataOffset - dataLength, + rawOffset: entry.rawOffset - rawLength, + viewSuppressed: entry.viewSuppressed + })) + const nextViewSuppressed = + boundary && boundary.dataOffset === dataLength + ? boundary.viewSuppressed + : pending.viewSuppressed === true + const remainder: PendingPtyData = { + data: remainingData, + ...(typeof pending.startSeq === 'number' ? { startSeq: pending.startSeq + rawLength } : {}), + ...(remainingRawLength !== remainingData.length ? { rawLength: remainingRawLength } : {}), + ...(pending.transformed === true ? { transformed: true as const } : {}), + ...(pending.containsBackgroundOutput === true ? { containsBackgroundOutput: true } : {}), + ...(nextViewSuppressed ? { viewSuppressed: true as const } : {}), + ...(remainingBoundaries && remainingBoundaries.length > 0 + ? { viewSuppressionBoundaries: remainingBoundaries } + : {}) + } + if (pending.projectionAdmissionIds) { + remainder.projectionAdmissionIds = pending.projectionAdmissionIds + } + if (pending.projectionAdmissionsTransferred) { + remainder.projectionAdmissionsTransferred = true + } + return { + data, + rawLength, + transformed: pending.transformed === true, + viewSuppressed: pending.viewSuppressed === true, + remainder + } + } + + function emitPendingViewGapRestoreMarker(id: string, pending: PendingPtyData): void { + if (pending.emitViewGapRestoreMarker === true && isHiddenRendererPty(id)) { + sendModelRestoreNeededMarker(id, 'hidden-drop', runtime?.getPtyOutputSequence(id)) + } + } + function schedulePendingDataFlush(delayMs: number): void { if (flushTimer) { return @@ -3448,6 +3549,7 @@ export function registerPtyHandlers( pendingData.remove(selection) pendingOverflowMarkedPtys.delete(id) updateProducerFlowControl(id) + emitPendingViewGapRestoreMarker(id, pending) const drop = recordHiddenRendererPtyDataDrop(id, pending.data.length) if (pending.projectionAdmissionIds) { sshOutputIntake?.transferProjections(pending.projectionAdmissionIds, 'hidden-drop') @@ -3465,6 +3567,7 @@ export function registerPtyHandlers( if (pending.droppedOutput === true) { pendingData.remove(selection) updateProducerFlowControl(id) + emitPendingViewGapRestoreMarker(id, pending) // Why droppedOutput sentinel: pending-cap drop means the pane must repaint from the snapshot, not continue a gapped stream (data = carved query bytes only). if ( !sendPtyDataToRenderer( @@ -3484,44 +3587,26 @@ export function registerPtyHandlers( writes++ continue } - const { data } = pending - const indivisible = pending.transformed === true - const chunk = indivisible ? data : data.slice(0, PTY_BATCH_FLUSH_CHUNK_CHARS) - const remaining = indivisible ? '' : data.slice(PTY_BATCH_FLUSH_CHUNK_CHARS) - let nextPending: PendingPtyData | undefined - if (remaining) { - nextPending = { data: remaining } - if (typeof pending.startSeq === 'number') { - nextPending.startSeq = pending.startSeq + chunk.length - } - if (pending.containsBackgroundOutput === true) { - nextPending.containsBackgroundOutput = true - } - if (pending.viewSuppressed === true) { - nextPending.viewSuppressed = true - } - if (pending.projectionAdmissionIds) { - nextPending.projectionAdmissionIds = pending.projectionAdmissionIds - } - if (pending.projectionAdmissionsTransferred) { - nextPending.projectionAdmissionsTransferred = true - } + const slice = takePendingPtyDeliverySlice(pending) + const nextPending = slice.remainder + if (nextPending) { pendingData.replaceWithRemainder(selection, nextPending) } else { pendingData.remove(selection) pendingOverflowMarkedPtys.delete(id) } updateProducerFlowControl(id) + emitPendingViewGapRestoreMarker(id, pending) const delivery = sendPtyDataToRenderer( id, makePtyDataPayload( id, - chunk, + slice.data, pending.startSeq, pending.containsBackgroundOutput, - pending.rawLength, - pending.transformed, - pending.viewSuppressed === true + slice.rawLength, + slice.transformed, + slice.viewSuppressed ), pending.projectionAdmissionIds ) @@ -3626,6 +3711,7 @@ export function registerPtyHandlers( const remaining = pendingData.delete(payload.id) clearFlushTimerIfIdle() if (remaining) { + emitPendingViewGapRestoreMarker(payload.id, remaining) if (remaining.droppedOutput === true) { // Sentinel entry: only salvaged query bytes remain; keep the flag so the renderer knows the span was dropped. sendPtyDataToRenderer( @@ -3638,7 +3724,7 @@ export function registerPtyHandlers( }, remaining.projectionAdmissionIds ) - } else { + } else if (!remaining.viewSuppressionBoundaries?.length) { sendPtyDataToRenderer( payload.id, makePtyDataPayload( @@ -3652,6 +3738,35 @@ export function registerPtyHandlers( ), remaining.projectionAdmissionIds ) + } else { + let unsettled: PendingPtyData | undefined = remaining + while (unsettled) { + const slice = takePendingPtyDeliverySlice(unsettled) + const delivery = sendPtyDataToRenderer( + payload.id, + makePtyDataPayload( + payload.id, + slice.data, + unsettled.startSeq, + unsettled.containsBackgroundOutput, + slice.rawLength, + slice.transformed, + slice.viewSuppressed + ), + unsettled.projectionAdmissionIds + ) + if (slice.remainder) { + updatePendingProjectionAdmissions( + slice.remainder, + propagatePendingProjectionRemainder( + slice.remainder, + delivery, + pendingProjectionAdmissionOptions() + ) + ) + } + unsettled = slice.remainder + } } } return release @@ -3780,6 +3895,12 @@ export function registerPtyHandlers( if (projection?.desktopSpan) { sourceCreditPendingPtys.add(payload.id) } + const viewSuppressed = shouldDeliverHiddenRendererPtyDataToSidecarsOnly( + payload.id, + getSettings?.() + ) + const emitViewGapRestoreMarker = + viewSuppressed && recordHiddenRendererPtyViewGap(payload.id).shouldEmitRestoreMarker const pending = appendPendingPtyData( payload.id, pendingData.get(payload.id), @@ -3790,7 +3911,8 @@ export function registerPtyHandlers( rawLength, payload.transformed === true, projectionId, - shouldDeliverHiddenRendererPtyDataToSidecarsOnly(payload.id, getSettings?.()) + viewSuppressed, + emitViewGapRestoreMarker ) const shouldEmitPendingCapRestoreMarker = pending.droppedOutput === true && @@ -3802,7 +3924,11 @@ export function registerPtyHandlers( nextData, performance.now() ) - if (isInteractiveOutput && rendererPtyDispatcherReady) { + if ( + isInteractiveOutput && + rendererPtyDispatcherReady && + !pending.viewSuppressionBoundaries?.length + ) { if (!canSendPtyDataToRenderer(payload.id, { interactive: true })) { setPendingPtyData(payload.id, pending) if (shouldEmitPendingCapRestoreMarker) { @@ -3819,6 +3945,7 @@ export function registerPtyHandlers( } pendingOverflowMarkedPtys.delete(payload.id) try { + emitPendingViewGapRestoreMarker(payload.id, pending) sendPtyDataToRenderer( payload.id, { @@ -4155,6 +4282,7 @@ export function registerPtyHandlers( const resetRendererPtyDeliveryGateState = (): void => { const gateDebug = getHiddenRendererPtyDeliveryDebug() resetRendererScopedHiddenPtyDeliveryState() + runtime?.resetLocalRendererTerminalViews?.() if (gateDebug.hiddenDeliveryGatedPtyCount > 0 || gateDebug.deliveryInterestPtyCount > 0) { invalidatePendingPtyDrainPolicy() } @@ -7423,6 +7551,7 @@ export function registerPtyHandlers( } else { visibleRendererPtys.delete(args.id) } + runtime?.setLocalRendererTerminalViewVisible?.(args.id, args.visible) syncPtyBackgroundedDelivery(args.id, 'visibility-report') }) diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 7cad4ad94bc..080eacd43ad 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -3039,6 +3039,8 @@ export class OrcaRuntimeService { // Preview windows consume the raw stream but deliberately leave terminal // query replies to main's headless emulator. private rawTerminalViewSubscriberCounts = new Map() + // Positive presence distinguishes a visible desktop xterm from a headless host. + private localRendererTerminalViewPtys = new Set() // Why a sticky promise per PTY: the daemon only emits data for sessions this // app has attached, so the first remote view subscriber of a never-attached // local daemon session triggers a main-side attach. The map dedupes @@ -11035,6 +11037,18 @@ export class OrcaRuntimeService { return (this.mobileSubscribers.get(ptyId)?.size ?? 0) > 0 } + setLocalRendererTerminalViewVisible(ptyId: string, visible: boolean): void { + if (visible) { + this.localRendererTerminalViewPtys.add(ptyId) + } else { + this.localRendererTerminalViewPtys.delete(ptyId) + } + } + + resetLocalRendererTerminalViews(): void { + this.localRendererTerminalViewPtys.clear() + } + isMobileTerminalQueryReplyAuthority(ptyId: string, clientId: string): boolean { // Why: a passive phone watching desktop-sized output must not race the // desktop xterm. Mobile becomes reply authority only with the mobile floor. @@ -11438,7 +11452,9 @@ export class OrcaRuntimeService { return shouldModelAnswerHiddenPtyQueries({ ptyId, settings: this.store?.getSettings(), - hasRemoteViewSubscriber: this.hasRemoteTerminalViewSubscriber(ptyId) + hasRemoteViewSubscriber: this.hasRemoteTerminalViewSubscriber(ptyId), + hasRawViewSubscriber: this.hasRawTerminalViewSubscriber(ptyId), + hasLocalRendererView: this.localRendererTerminalViewPtys.has(ptyId) }) } @@ -13637,6 +13653,7 @@ export class OrcaRuntimeService { this.mobileSubscribers.delete(ptyId) this.remoteTerminalViewSubscriberCounts.delete(ptyId) this.rawTerminalViewSubscriberCounts.delete(ptyId) + this.localRendererTerminalViewPtys.delete(ptyId) this.mobileDisplayModes.delete(ptyId) this.resizeListeners.delete(ptyId) this.lastRendererSizes.delete(ptyId) diff --git a/src/main/runtime/rpc/methods/terminal.ts b/src/main/runtime/rpc/methods/terminal.ts index be4dcf4f3e2..ef738416f41 100644 --- a/src/main/runtime/rpc/methods/terminal.ts +++ b/src/main/runtime/rpc/methods/terminal.ts @@ -140,8 +140,7 @@ 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. */ + /** Releases the stream's current view or raw-only presence. */ releaseViewPresence: () => void /** null until this stream registers presence at all. */ holdsViewAuthority: boolean | null @@ -419,15 +418,7 @@ 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). - */ +// Why: a paused client cannot answer terminal probes; raw presence retains its PTY without claiming view authority. function setRemoteStreamViewAuthority( runtime: OrcaRuntimeService, stream: TerminalMultiplexStream, @@ -2217,14 +2208,18 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ if (typeof payload?.paused !== 'boolean' || stream.outputPaused === payload.paused) { return } - stream.outputPaused = payload.paused - if (stream.outputPaused) { + if (payload.paused) { + // Pre-pause bytes were ingested under view authority, so retire the + // batch before the pause guard and retain ACK-blocked bytes for resume. stream.outputBatcher.flush() - stream.ackPendingOutput = [] - stream.ackPendingOutputBytes = 0 - stream.ackPendingOutputOverflowed = false + flushAckPendingOutput(stream) + stream.outputPaused = true + setRemoteStreamViewAuthority(runtime, stream, false) + } else { + stream.outputPaused = false + setRemoteStreamViewAuthority(runtime, stream, true) + flushAckPendingOutput(stream) } - setRemoteStreamViewAuthority(runtime, stream, !stream.outputPaused) return } if (frame.opcode === TerminalStreamOpcode.Resize && stream.client) { diff --git a/src/main/runtime/rpc/terminal-multiplex.test.ts b/src/main/runtime/rpc/terminal-multiplex.test.ts index bb488bb5ab7..16a728add3b 100644 --- a/src/main/runtime/rpc/terminal-multiplex.test.ts +++ b/src/main/runtime/rpc/terminal-multiplex.test.ts @@ -4602,6 +4602,126 @@ describe('terminal multiplex RPC', () => { await harness.dispatchPromise }) + it('settles the pre-pause batch and preserves ACK-pending bytes for resume', async () => { + let dataListener: ((data: string, meta?: RuntimeTerminalDataMeta) => void) | undefined + const releaseView = vi.fn() + const sendBinary = vi.fn(() => true) + const harness = startDesktopMultiplexSubscribe( + { + registerRemoteTerminalViewSubscriber: vi.fn(() => releaseView), + registerRawTerminalViewSubscriber: vi.fn(() => vi.fn()), + subscribeToTerminalData: vi.fn((_ptyId, listener) => { + dataListener = listener + return vi.fn() + }) + }, + undefined, + sendBinary + ) + await vi.waitFor(() => expect(harness.handlers.has(0)).toBe(true)) + sendDesktopMultiplexSubscribe(harness.handlers, { ackOutput: 1, outputPause: 1 }) + await vi.waitFor(() => expect(dataListener).toBeDefined()) + await vi.waitFor(() => + expect( + harness.messages.some((message) => JSON.parse(message).result?.type === 'subscribed') + ).toBe(true) + ) + harness.binaryFrames.splice(0) + sendBinary.mockClear() + + dataListener?.('five-ms-batch', { seq: 13, rawLength: 13 }) + harness.handlers.get(7)?.( + decodeTerminalStreamFrame( + encodeTerminalStreamFrame({ + opcode: SET_OUTPUT_PAUSED_OPCODE, + streamId: 7, + seq: 2, + payload: encodeTerminalStreamJson({ paused: true }) + }) + )! + ) + expect( + harness.binaryFrames + .map(decodeTerminalStreamFrame) + .filter((frame) => frame?.opcode === TerminalStreamOpcode.Output) + .map((frame) => (frame ? decodeTerminalStreamText(frame.payload) : '')) + .join('') + ).toBe('five-ms-batch') + expect(sendBinary.mock.invocationCallOrder.at(-1)).toBeLessThan( + releaseView.mock.invocationCallOrder[0]! + ) + + harness.handlers.get(7)?.( + decodeTerminalStreamFrame( + encodeTerminalStreamFrame({ + opcode: SET_OUTPUT_PAUSED_OPCODE, + streamId: 7, + seq: 3, + payload: encodeTerminalStreamJson({ paused: false }) + }) + )! + ) + harness.binaryFrames.splice(0) + dataListener?.('x'.repeat(TERMINAL_MULTIPLEX_ACK_STREAM_INITIAL_WINDOW_BYTES), { + seq: 13 + TERMINAL_MULTIPLEX_ACK_STREAM_INITIAL_WINDOW_BYTES, + rawLength: TERMINAL_MULTIPLEX_ACK_STREAM_INITIAL_WINDOW_BYTES + }) + dataListener?.('ack-pending', { + seq: 24 + TERMINAL_MULTIPLEX_ACK_STREAM_INITIAL_WINDOW_BYTES, + rawLength: 11 + }) + harness.handlers.get(7)?.( + decodeTerminalStreamFrame( + encodeTerminalStreamFrame({ + opcode: SET_OUTPUT_PAUSED_OPCODE, + streamId: 7, + seq: 4, + payload: encodeTerminalStreamJson({ paused: true }) + }) + )! + ) + harness.handlers.get(7)?.( + decodeTerminalStreamFrame( + encodeTerminalStreamFrame({ + opcode: TerminalStreamOpcode.Ack, + streamId: 7, + seq: 5, + payload: encodeTerminalStreamJson({ + bytes: TERMINAL_MULTIPLEX_ACK_STREAM_INITIAL_WINDOW_BYTES + }) + }) + )! + ) + expect( + harness.binaryFrames + .map(decodeTerminalStreamFrame) + .filter((frame) => frame?.opcode === TerminalStreamOpcode.Output) + .map((frame) => (frame ? decodeTerminalStreamText(frame.payload) : '')) + .join('') + ).not.toContain('ack-pending') + + harness.handlers.get(7)?.( + decodeTerminalStreamFrame( + encodeTerminalStreamFrame({ + opcode: SET_OUTPUT_PAUSED_OPCODE, + streamId: 7, + seq: 6, + payload: encodeTerminalStreamJson({ paused: false }) + }) + )! + ) + expect( + harness.binaryFrames + .map(decodeTerminalStreamFrame) + .filter((frame) => frame?.opcode === TerminalStreamOpcode.Output) + .map((frame) => (frame ? decodeTerminalStreamText(frame.payload) : '')) + .join('') + ).toContain('ack-pending') + + 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 6741c6733af..098723fd933 100644 --- a/src/main/runtime/terminal-model-query-authority.test.ts +++ b/src/main/runtime/terminal-model-query-authority.test.ts @@ -86,6 +86,27 @@ describe('shouldModelAnswerHiddenPtyQueries', () => { ).toBe(false) }) + it('answers for a raw-only headless stream but yields to a visible local xterm', () => { + expect( + shouldModelAnswerHiddenPtyQueries({ + ptyId: 'pty-headless', + settings: ALL_ON, + hasRemoteViewSubscriber: false, + hasRawViewSubscriber: true, + hasLocalRendererView: false + }) + ).toBe(true) + expect( + shouldModelAnswerHiddenPtyQueries({ + ptyId: 'pty-headless', + settings: ALL_ON, + hasRemoteViewSubscriber: false, + hasRawViewSubscriber: true, + hasLocalRendererView: true + }) + ).toBe(false) + }) + it('stays silent under any kill switch', () => { markHiddenRendererPty('pty-1') expect(answer('pty-1', { terminalModelQueryAuthority: false })).toBe(false) diff --git a/src/main/runtime/terminal-model-query-authority.ts b/src/main/runtime/terminal-model-query-authority.ts index ccfef6c60d0..15e4189a263 100644 --- a/src/main/runtime/terminal-model-query-authority.ts +++ b/src/main/runtime/terminal-model-query-authority.ts @@ -40,11 +40,14 @@ export function shouldModelAnswerHiddenPtyQueries(opts: { ptyId: string settings: TerminalModelQueryAuthoritySettings | null | undefined hasRemoteViewSubscriber: boolean + hasRawViewSubscriber?: boolean + hasLocalRendererView?: boolean }): boolean { return ( isTerminalModelQueryAuthorityEnabled(opts.settings) && !opts.hasRemoteViewSubscriber && - shouldSuppressHiddenRendererPtyView(opts.ptyId, opts.settings) + (shouldSuppressHiddenRendererPtyView(opts.ptyId, opts.settings) || + (opts.hasRawViewSubscriber === true && opts.hasLocalRendererView !== true)) ) } diff --git a/src/main/runtime/terminal-query-responder.test.ts b/src/main/runtime/terminal-query-responder.test.ts index ddee0f11ca8..f93bb6cf782 100644 --- a/src/main/runtime/terminal-query-responder.test.ts +++ b/src/main/runtime/terminal-query-responder.test.ts @@ -265,6 +265,21 @@ describe('reply ownership matrix', () => { expect(runtime.hasRawTerminalViewSubscriber('pty-preview')).toBe(false) }) + it('gives a raw-only headless stream exactly one answerer', async () => { + const { runtime, replies } = createResponderRuntime() + const release = runtime.registerRawTerminalViewSubscriber('pty-headless') + + runtime.onPtyData('pty-headless', DA1, Date.now()) + await settle(runtime, 'pty-headless') + expect(replies).toEqual([{ ptyId: 'pty-headless', data: '\x1b[?1;2c' }]) + + runtime.setLocalRendererTerminalViewVisible('pty-headless', true) + runtime.onPtyData('pty-headless', DA1, Date.now()) + await settle(runtime, 'pty-headless') + expect(replies).toHaveLength(1) + release() + }) + it('treats mobile subscriber records as remote view subscribers', async () => { const { runtime } = createResponderRuntime() await runtime.handleMobileSubscribe('pty-mob', 'client-1', { cols: 40, rows: 20 })