From 39ac32f279f96104e19d130c0712e4f5f6b5e611 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Thu, 27 Aug 2026 17:25:24 -0700 Subject: [PATCH 01/10] Extract terminal output scheduler subsystems (#16775) --- .../pane-terminal-cursor-sequencing.ts | 168 ++++++ .../pane-terminal-output-queue-chunks.ts | 181 +++++++ .../pane-terminal-output-scheduler-debug.ts | 110 ++++ .../pane-terminal-output-scheduler.ts | 477 +----------------- 4 files changed, 478 insertions(+), 458 deletions(-) create mode 100644 src/renderer/src/lib/pane-manager/pane-terminal-cursor-sequencing.ts create mode 100644 src/renderer/src/lib/pane-manager/pane-terminal-output-queue-chunks.ts create mode 100644 src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler-debug.ts diff --git a/src/renderer/src/lib/pane-manager/pane-terminal-cursor-sequencing.ts b/src/renderer/src/lib/pane-manager/pane-terminal-cursor-sequencing.ts new file mode 100644 index 00000000000..7a27ae1ea15 --- /dev/null +++ b/src/renderer/src/lib/pane-manager/pane-terminal-cursor-sequencing.ts @@ -0,0 +1,168 @@ +type CursorQueueEntry = { + chunks: { data: string }[] + chunkIndex: number +} + +export const SYNC_FOREGROUND_FLUSH_CHARS = 256 * 1024 +const CURSOR_SHOW_SEQUENCE = '\x1b[?25h' +const CURSOR_HIDE_SEQUENCE = '\x1b[?25l' +const SYNCHRONIZED_OUTPUT_END_SEQUENCE = '\x1b[?2026l' + +function findCursorPositionSequenceEnd( + data: string, + fromIndex: number, + toIndex = data.length +): number { + let offset = data.indexOf('\x1b[', fromIndex) + while (offset !== -1 && offset < toIndex) { + let index = offset + 2 + while (index < toIndex) { + const char = data[index] + if (char === 'G' || char === 'H' || char === 'f') { + return index + 1 + } + if ((char < '0' || char > '9') && char !== ';') { + break + } + index += 1 + } + offset = data.indexOf('\x1b[', offset + 2) + } + return -1 +} + +export function removeTransientCursorShowSequences(data: string): string { + let result = '' + let offset = 0 + let showIndex = data.indexOf(CURSOR_SHOW_SEQUENCE) + while (showIndex !== -1) { + const nextHideIndex = data.indexOf( + CURSOR_HIDE_SEQUENCE, + showIndex + CURSOR_SHOW_SEQUENCE.length + ) + const nextPositionEnd = findCursorPositionSequenceEnd( + data, + showIndex + CURSOR_SHOW_SEQUENCE.length, + nextHideIndex === -1 ? data.length : nextHideIndex + ) + if (nextHideIndex === -1) { + if (nextPositionEnd === -1) { + const synchronizedEndIndex = data.indexOf( + SYNCHRONIZED_OUTPUT_END_SEQUENCE, + showIndex + CURSOR_SHOW_SEQUENCE.length + ) + if (synchronizedEndIndex === -1) { + break + } + // Why: keep the cursor hidden through the synchronized repaint, restoring it after the frame ends so Windows never paints it in the transient draw position. + result += data.slice(offset, showIndex) + result += data.slice( + showIndex + CURSOR_SHOW_SEQUENCE.length, + synchronizedEndIndex + SYNCHRONIZED_OUTPUT_END_SEQUENCE.length + ) + result += CURSOR_SHOW_SEQUENCE + offset = synchronizedEndIndex + SYNCHRONIZED_OUTPUT_END_SEQUENCE.length + showIndex = data.indexOf(CURSOR_SHOW_SEQUENCE, offset) + continue + } + // Why: Codex can show the cursor before its final synchronized-frame placement. Place first so xterm cannot rasterize the stale cell. + result += data.slice(offset, showIndex) + result += data.slice(showIndex + CURSOR_SHOW_SEQUENCE.length, nextPositionEnd) + result += CURSOR_SHOW_SEQUENCE + offset = nextPositionEnd + showIndex = data.indexOf(CURSOR_SHOW_SEQUENCE, offset) + continue + } + result += data.slice(offset, showIndex) + offset = showIndex + CURSOR_SHOW_SEQUENCE.length + showIndex = data.indexOf(CURSOR_SHOW_SEQUENCE, offset) + } + return offset === 0 ? data : result + data.slice(offset) +} + +function containsCursorPositionSequence(data: string): boolean { + let offset = data.indexOf('\x1b[') + while (offset !== -1) { + let index = offset + 2 + while (index < data.length) { + const char = data[index] + if (char === 'G' || char === 'H' || char === 'f') { + return true + } + if ((char < '0' || char > '9') && char !== ';') { + break + } + index += 1 + } + offset = data.indexOf('\x1b[', offset + 2) + } + return false +} + +function containsCursorRestore(data: string): boolean { + const hideIndex = data.indexOf(CURSOR_HIDE_SEQUENCE) + const showIndex = data.lastIndexOf(CURSOR_SHOW_SEQUENCE) + return hideIndex !== -1 && showIndex > hideIndex && containsCursorPositionSequence(data) +} + +export function containsDrainableCursorRestore(data: string): boolean { + const synchronizedEndIndex = data.lastIndexOf(SYNCHRONIZED_OUTPUT_END_SEQUENCE) + if (synchronizedEndIndex === -1) { + return containsCursorRestore(data) + } + return containsCursorRestore( + data.slice(synchronizedEndIndex + SYNCHRONIZED_OUTPUT_END_SEQUENCE.length) + ) +} + +export function containsFinalCursorPlacementBeforeSynchronizedEnd(data: string): boolean { + const synchronizedEndIndex = data.lastIndexOf(SYNCHRONIZED_OUTPUT_END_SEQUENCE) + if (synchronizedEndIndex === -1) { + return false + } + const lastShowIndex = data.lastIndexOf(CURSOR_SHOW_SEQUENCE, synchronizedEndIndex) + if (lastShowIndex === -1) { + return false + } + const lastHideIndex = data.lastIndexOf(CURSOR_HIDE_SEQUENCE, synchronizedEndIndex) + if (lastHideIndex > lastShowIndex) { + return false + } + return ( + findCursorPositionSequenceEnd( + data, + lastShowIndex + CURSOR_SHOW_SEQUENCE.length, + synchronizedEndIndex + ) !== -1 + ) +} + +function previewQueuedData(entry: CursorQueueEntry, limit: number): string { + let data = '' + for (let index = entry.chunkIndex; index < entry.chunks.length; index += 1) { + const chunk = entry.chunks[index] + const remaining = limit - data.length + if (remaining <= 0) { + break + } + data += chunk.data.slice(0, remaining) + } + return data +} + +export function coalescedQueuedDataNeedsCursorRestore(entry: CursorQueueEntry): boolean { + const data = previewQueuedData(entry, SYNC_FOREGROUND_FLUSH_CHARS) + const synchronizedEndIndex = data.lastIndexOf(SYNCHRONIZED_OUTPUT_END_SEQUENCE) + if (synchronizedEndIndex === -1) { + return false + } + const synchronizedFrame = data.slice( + 0, + synchronizedEndIndex + SYNCHRONIZED_OUTPUT_END_SEQUENCE.length + ) + return ( + containsCursorRestore(synchronizedFrame) && + !containsFinalCursorPlacementBeforeSynchronizedEnd(synchronizedFrame) && + !containsDrainableCursorRestore(data) + ) +} diff --git a/src/renderer/src/lib/pane-manager/pane-terminal-output-queue-chunks.ts b/src/renderer/src/lib/pane-manager/pane-terminal-output-queue-chunks.ts new file mode 100644 index 00000000000..0142c3528be --- /dev/null +++ b/src/renderer/src/lib/pane-manager/pane-terminal-output-queue-chunks.ts @@ -0,0 +1,181 @@ +import { flattenRetainedSlice } from '@/lib/flatten-retained-slice' +import type { + QueueEntry, + QueuedWrite, + TerminalOutputBeforeWrite, + TerminalOutputParsedCallback +} from './pane-terminal-output-scheduler' +import { recordTerminalOutputQueueDebugPressure as recordQueueDebugPressure } from './pane-terminal-output-scheduler-debug' + +type ForegroundRefreshSyncResolver = () => boolean +const ALWAYS_REFRESH_FOREGROUND_SYNCHRONOUSLY = (): boolean => true + +export function takeQueuedChunk(entry: QueueEntry, limit: number): QueuedWrite | null { + let remaining = limit + let data = '' + let foreground: boolean | null = null + let forceForegroundRefresh = false + let followupForegroundRefresh = false + let shouldRefreshForegroundSynchronously: ForegroundRefreshSyncResolver | null = null + let additionalRefreshSyncResolvers: ForegroundRefreshSyncResolver[] | null = null + let stripTransientCursorShows = false + let beforeWrite: TerminalOutputBeforeWrite | undefined + let additionalBeforeWriteCallbacks: TerminalOutputBeforeWrite[] | null = null + const parsedCallbacks: TerminalOutputParsedCallback[] = [] + const ackCredits: (() => void)[] = [] + + while (remaining > 0 && entry.chunkIndex < entry.chunks.length) { + const chunk = entry.chunks[entry.chunkIndex] + if (foreground !== null && chunk.foreground !== foreground) { + break + } + foreground ??= chunk.foreground + forceForegroundRefresh ||= chunk.forceForegroundRefresh + followupForegroundRefresh ||= chunk.followupForegroundRefresh + // Why: one drained write can combine chunks from different renderer states or producers; preserve every forced policy and prep hook. + if (chunk.forceForegroundRefresh) { + if (shouldRefreshForegroundSynchronously === null) { + shouldRefreshForegroundSynchronously = chunk.shouldRefreshForegroundSynchronously + } else if ( + chunk.shouldRefreshForegroundSynchronously !== shouldRefreshForegroundSynchronously && + !additionalRefreshSyncResolvers?.includes(chunk.shouldRefreshForegroundSynchronously) + ) { + additionalRefreshSyncResolvers ??= [] + additionalRefreshSyncResolvers.push(chunk.shouldRefreshForegroundSynchronously) + } + } + stripTransientCursorShows ||= chunk.stripTransientCursorShows + if (!beforeWrite) { + beforeWrite = chunk.beforeWrite + } else if ( + chunk.beforeWrite && + chunk.beforeWrite !== beforeWrite && + !additionalBeforeWriteCallbacks?.includes(chunk.beforeWrite) + ) { + additionalBeforeWriteCallbacks ??= [] + additionalBeforeWriteCallbacks.push(chunk.beforeWrite) + } + if (chunk.data.length <= remaining) { + data += chunk.data + remaining -= chunk.data.length + entry.queuedChars -= chunk.data.length + // Clear drained slots before the 64-chunk compaction can release them. + entry.chunks[entry.chunkIndex] = { + data: '', + retainedChars: 0, + foreground: chunk.foreground, + forceForegroundRefresh: false, + followupForegroundRefresh: false, + shouldRefreshForegroundSynchronously: ALWAYS_REFRESH_FOREGROUND_SYNCHRONOUSLY, + stripTransientCursorShows: false + } + entry.chunkIndex += 1 + if (chunk.onParsed) { + parsedCallbacks.push(chunk.onParsed) + } + if (chunk.ackCredit) { + ackCredits.push(chunk.ackCredit) + } + continue + } + + data += chunk.data.slice(0, remaining) + const residual = chunk.data.slice(remaining) + // Geometric flattening bounds retained parents while keeping total copy work linear. + const flatten = residual.length * 2 <= chunk.retainedChars + entry.chunks[entry.chunkIndex] = { + ...chunk, + data: flatten ? flattenRetainedSlice(residual) : residual, + retainedChars: flatten ? residual.length : chunk.retainedChars + } + entry.queuedChars -= remaining + remaining = 0 + } + + compactConsumedChunks(entry) + if (entry.queuedChars < 0) { + entry.queuedChars = 0 + } + recordQueueDebugPressure() + return data + ? { + data, + foreground: foreground === true, + forceForegroundRefresh, + followupForegroundRefresh, + shouldRefreshForegroundSynchronously: + additionalRefreshSyncResolvers && shouldRefreshForegroundSynchronously + ? () => + shouldRefreshForegroundSynchronously() || + additionalRefreshSyncResolvers.some((resolve) => resolve()) + : (shouldRefreshForegroundSynchronously ?? ALWAYS_REFRESH_FOREGROUND_SYNCHRONOUSLY), + stripTransientCursorShows, + beforeWrite: + additionalBeforeWriteCallbacks && beforeWrite + ? (queuedData) => { + beforeWrite(queuedData) + for (const callback of additionalBeforeWriteCallbacks) { + callback(queuedData) + } + } + : beforeWrite, + onParsed: + parsedCallbacks.length > 0 + ? () => { + for (const callback of parsedCallbacks) { + callback() + } + } + : undefined, + ackCredits + } + : null +} + +export function compactConsumedChunks(entry: QueueEntry): void { + if (entry.chunkIndex === 0) { + return + } + if (entry.chunkIndex === entry.chunks.length) { + entry.chunks.length = 0 + entry.chunkIndex = 0 + return + } + if (entry.chunkIndex >= 64) { + entry.chunks.splice(0, entry.chunkIndex) + entry.chunkIndex = 0 + } +} + +export function enqueueChunk( + entry: QueueEntry, + data: string, + options?: { + foreground?: boolean + forceForegroundRefresh?: boolean + followupForegroundRefresh?: boolean + shouldRefreshForegroundSynchronously?: ForegroundRefreshSyncResolver + stripTransientCursorShows?: boolean + beforeWrite?: TerminalOutputBeforeWrite + onParsed?: TerminalOutputParsedCallback + ackCredit?: () => void + } +): void { + // Own queued data so producer slices cannot pin larger PTY or restore buffers. + const owned = flattenRetainedSlice(data) + entry.chunks.push({ + data: owned, + retainedChars: owned.length, + foreground: options?.foreground === true, + forceForegroundRefresh: options?.forceForegroundRefresh === true, + followupForegroundRefresh: options?.followupForegroundRefresh === true, + shouldRefreshForegroundSynchronously: + options?.shouldRefreshForegroundSynchronously ?? ALWAYS_REFRESH_FOREGROUND_SYNCHRONOUSLY, + stripTransientCursorShows: options?.stripTransientCursorShows === true, + beforeWrite: options?.beforeWrite, + onParsed: options?.onParsed, + ackCredit: options?.ackCredit + }) + entry.queuedChars += data.length + recordQueueDebugPressure() +} diff --git a/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler-debug.ts b/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler-debug.ts new file mode 100644 index 00000000000..fbe34176f65 --- /dev/null +++ b/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler-debug.ts @@ -0,0 +1,110 @@ +import { e2eConfig } from '@/lib/e2e-config' + +type QueueDebugEntry = { queuedChars: number } + +// Why the cap is lossy: a backgrounded Chromium document throttles timers while PTYs keep writing, so unbounded hidden scrollback would grow renderer memory until the app crashes. +type TerminalOutputSchedulerDebugSnapshot = { + backgroundEnqueueCount: number + deferredForegroundEnqueueCount: number + foregroundWriteCount: number + backgroundWriteCount: number + deferredForegroundWriteCount: number + flushWriteCount: number + scheduledDrainCount: number + queuedTerminalCount: number + queuedChars: number + peakQueuedTerminalCount: number + peakQueuedChars: number + peakQueuedCharsByTerminal: number + droppedBacklogCount: number + drainWrites: number[] + drainHighPriority: boolean[] +} + +export const terminalOutputSchedulerDebugEnabled = e2eConfig.exposeStore +export const terminalOutputSchedulerDebugState: TerminalOutputSchedulerDebugSnapshot = { + backgroundEnqueueCount: 0, + deferredForegroundEnqueueCount: 0, + foregroundWriteCount: 0, + backgroundWriteCount: 0, + deferredForegroundWriteCount: 0, + flushWriteCount: 0, + scheduledDrainCount: 0, + queuedTerminalCount: 0, + queuedChars: 0, + peakQueuedTerminalCount: 0, + peakQueuedChars: 0, + peakQueuedCharsByTerminal: 0, + droppedBacklogCount: 0, + drainWrites: [], + drainHighPriority: [] +} + +let readQueueEntries: () => Iterable = () => [] + +export function setTerminalOutputDebugQueueReader(reader: () => Iterable): void { + readQueueEntries = reader +} + +function resetDebugState(): void { + const state = terminalOutputSchedulerDebugState + state.backgroundEnqueueCount = 0 + state.deferredForegroundEnqueueCount = 0 + state.foregroundWriteCount = 0 + state.backgroundWriteCount = 0 + state.deferredForegroundWriteCount = 0 + state.flushWriteCount = 0 + state.scheduledDrainCount = 0 + state.queuedTerminalCount = 0 + state.queuedChars = 0 + state.peakQueuedTerminalCount = 0 + state.peakQueuedChars = 0 + state.peakQueuedCharsByTerminal = 0 + state.droppedBacklogCount = 0 + state.drainWrites = [] + state.drainHighPriority = [] +} + +export function recordTerminalOutputQueueDebugPressure(): void { + if (!terminalOutputSchedulerDebugEnabled) { + return + } + let queuedTerminalCount = 0 + let queuedChars = 0 + let queuedCharsByTerminal = 0 + for (const entry of readQueueEntries()) { + queuedTerminalCount++ + queuedChars += entry.queuedChars + queuedCharsByTerminal = Math.max(queuedCharsByTerminal, entry.queuedChars) + } + const state = terminalOutputSchedulerDebugState + state.queuedTerminalCount = queuedTerminalCount + state.queuedChars = queuedChars + state.peakQueuedTerminalCount = Math.max(state.peakQueuedTerminalCount, queuedTerminalCount) + state.peakQueuedChars = Math.max(state.peakQueuedChars, queuedChars) + state.peakQueuedCharsByTerminal = Math.max(state.peakQueuedCharsByTerminal, queuedCharsByTerminal) +} + +export function exposeTerminalOutputSchedulerDebugApi(): void { + if (!terminalOutputSchedulerDebugEnabled || typeof window === 'undefined') { + return + } + // Why: the e2e repro must prove background output used the shared drain, but production must not accumulate diagnostic counters indefinitely. + const target = window as unknown as { + __terminalOutputSchedulerDebug?: { + reset: () => void + snapshot: () => TerminalOutputSchedulerDebugSnapshot + } + } + target.__terminalOutputSchedulerDebug ??= { + reset: resetDebugState, + snapshot: () => { + recordTerminalOutputQueueDebugPressure() + return { + ...terminalOutputSchedulerDebugState, + drainWrites: [...terminalOutputSchedulerDebugState.drainWrites], + drainHighPriority: [...terminalOutputSchedulerDebugState.drainHighPriority] + } + } + } +} diff --git a/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.ts b/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.ts index 0177692d26e..402b847d421 100644 --- a/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.ts +++ b/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.ts @@ -1,5 +1,4 @@ /* oxlint-disable max-lines -- Why: output ordering, foreground settle, queue state, and e2e diagnostics form one state machine; splitting it would make backlog/resume guarantees harder to audit. */ -import { e2eConfig } from '@/lib/e2e-config' import { discardForegroundRenderSettle, writeForegroundTerminalChunk, @@ -7,7 +6,6 @@ import { } from './pane-terminal-foreground-render-settle' import { runGuardedWriteCompletionStep } from './xterm-write-callback-guard' import { recordRendererCrashBreadcrumb } from '@/lib/crash-breadcrumb-recorder' -import { flattenRetainedSlice } from '@/lib/flatten-retained-slice' import { discardInFlightTerminalOutputAckCredits, registerTerminalOutputAckCredits @@ -24,12 +22,26 @@ import { TERMINAL_OUTPUT_BACKLOG_MIN_CAP_CHARS, terminalOutputBacklogCapChars } from '../../../../shared/terminal-scrollback-policy' +import { + coalescedQueuedDataNeedsCursorRestore, + containsDrainableCursorRestore, + removeTransientCursorShowSequences, + SYNC_FOREGROUND_FLUSH_CHARS +} from './pane-terminal-cursor-sequencing' +import { + exposeTerminalOutputSchedulerDebugApi as exposeDebugApi, + recordTerminalOutputQueueDebugPressure as recordQueueDebugPressure, + setTerminalOutputDebugQueueReader, + terminalOutputSchedulerDebugEnabled as debugEnabled, + terminalOutputSchedulerDebugState as debugState +} from './pane-terminal-output-scheduler-debug' +import { enqueueChunk, takeQueuedChunk } from './pane-terminal-output-queue-chunks' type TerminalOutputTarget = ForegroundTerminalOutputTarget -type TerminalOutputBeforeWrite = (data: string) => void +export type TerminalOutputBeforeWrite = (data: string) => void type TerminalBacklogRecoveryRequest = () => boolean -type TerminalOutputParsedCallback = () => void +export type TerminalOutputParsedCallback = () => void type ForegroundRefreshSyncResolver = () => boolean type WriteTerminalOutputOptions = { @@ -62,7 +74,7 @@ type QueueChunk = { ackCredit?: () => void } -type QueuedWrite = { +export type QueuedWrite = { data: string foreground: boolean forceForegroundRefresh: boolean @@ -74,7 +86,7 @@ type QueuedWrite = { ackCredits: (() => void)[] } -type QueueEntry = { +export type QueueEntry = { terminal: TerminalOutputTarget chunks: QueueChunk[] chunkIndex: number @@ -103,7 +115,6 @@ const MAX_WRITES_PER_DRAIN = 2 const HIGH_PRIORITY_MAX_WRITES_PER_DRAIN = 8 const DRAIN_TIME_BUDGET_MS = 8 const LARGE_BACKLOG_CHARS = 512 * 1024 -const SYNC_FOREGROUND_FLUSH_CHARS = 256 * 1024 // Why mutable: the cap scales with the user's scrollback setting (terminalOutputBacklogCapChars), configured when settings apply; the chunk-count cap stays fixed. let maxQueueChars = TERMINAL_OUTPUT_BACKLOG_MIN_CAP_CHARS const MAX_BACKGROUND_QUEUE_CHUNKS = 4096 @@ -117,9 +128,6 @@ const FOREGROUND_HOLD_SAFETY_DELAY_MS = 250 // Why: key repeat can tick every 30-50ms; one frame catches split restores without batching multiple typed-character redraws behind the fallback. const LATENCY_SENSITIVE_FOREGROUND_COALESCE_DELAY_MS = 16 const LATENCY_SENSITIVE_FOREGROUND_HOLD_SAFETY_DELAY_MS = 32 -const CURSOR_SHOW_SEQUENCE = '\x1b[?25h' -const CURSOR_HIDE_SEQUENCE = '\x1b[?25l' -const SYNCHRONIZED_OUTPUT_END_SEQUENCE = '\x1b[?2026l' // Why: leading CAN aborts any partial escape sequence before the style reset so the backlog warning renders cleanly. const BACKGROUND_BACKLOG_WARNING = '\x18\x1b[0m\r\n[Orca skipped hidden terminal output because the backlog grew too large.]\r\n' @@ -129,6 +137,7 @@ const FOREGROUND_BACKLOG_WARNING = const ALWAYS_REFRESH_FOREGROUND_SYNCHRONOUSLY = (): boolean => true const queuedByTerminal = new Map() +setTerminalOutputDebugQueueReader(() => queuedByTerminal.values()) const backlogRecoveryByTerminal = new WeakMap< TerminalOutputTarget, TerminalBacklogRecoveryRequest @@ -169,125 +178,6 @@ export function setUseMessageChannelDrainForTesting(value: boolean | null): void cancelImmediateDrain() useMessageChannelDrain = value ?? (typeof MessageChannel !== 'undefined' && !isVitestEnv()) } -const debugEnabled = e2eConfig.exposeStore - -// Why the cap is lossy: a backgrounded Chromium document throttles timers while PTYs keep writing, so unbounded hidden scrollback would grow renderer memory until the app crashes. - -type TerminalOutputSchedulerDebugSnapshot = { - backgroundEnqueueCount: number - deferredForegroundEnqueueCount: number - foregroundWriteCount: number - backgroundWriteCount: number - deferredForegroundWriteCount: number - flushWriteCount: number - scheduledDrainCount: number - queuedTerminalCount: number - queuedChars: number - peakQueuedTerminalCount: number - peakQueuedChars: number - peakQueuedCharsByTerminal: number - droppedBacklogCount: number - drainWrites: number[] - drainHighPriority: boolean[] -} - -type TerminalOutputSchedulerDebugApi = { - reset: () => void - snapshot: () => TerminalOutputSchedulerDebugSnapshot -} - -const debugState: TerminalOutputSchedulerDebugSnapshot = { - backgroundEnqueueCount: 0, - deferredForegroundEnqueueCount: 0, - foregroundWriteCount: 0, - backgroundWriteCount: 0, - deferredForegroundWriteCount: 0, - flushWriteCount: 0, - scheduledDrainCount: 0, - queuedTerminalCount: 0, - queuedChars: 0, - peakQueuedTerminalCount: 0, - peakQueuedChars: 0, - peakQueuedCharsByTerminal: 0, - droppedBacklogCount: 0, - drainWrites: [], - drainHighPriority: [] -} - -function resetDebugState(): void { - debugState.backgroundEnqueueCount = 0 - debugState.deferredForegroundEnqueueCount = 0 - debugState.foregroundWriteCount = 0 - debugState.backgroundWriteCount = 0 - debugState.deferredForegroundWriteCount = 0 - debugState.flushWriteCount = 0 - debugState.scheduledDrainCount = 0 - debugState.queuedTerminalCount = 0 - debugState.queuedChars = 0 - debugState.peakQueuedTerminalCount = 0 - debugState.peakQueuedChars = 0 - debugState.peakQueuedCharsByTerminal = 0 - debugState.droppedBacklogCount = 0 - debugState.drainWrites = [] - debugState.drainHighPriority = [] -} - -function readQueueDebugSnapshot(): { - queuedTerminalCount: number - queuedChars: number - queuedCharsByTerminal: number -} { - let queuedChars = 0 - let queuedCharsByTerminal = 0 - for (const entry of queuedByTerminal.values()) { - queuedChars += entry.queuedChars - queuedCharsByTerminal = Math.max(queuedCharsByTerminal, entry.queuedChars) - } - return { - queuedTerminalCount: queuedByTerminal.size, - queuedChars, - queuedCharsByTerminal - } -} - -function recordQueueDebugPressure(): void { - if (!debugEnabled) { - return - } - const current = readQueueDebugSnapshot() - debugState.queuedTerminalCount = current.queuedTerminalCount - debugState.queuedChars = current.queuedChars - debugState.peakQueuedTerminalCount = Math.max( - debugState.peakQueuedTerminalCount, - current.queuedTerminalCount - ) - debugState.peakQueuedChars = Math.max(debugState.peakQueuedChars, current.queuedChars) - debugState.peakQueuedCharsByTerminal = Math.max( - debugState.peakQueuedCharsByTerminal, - current.queuedCharsByTerminal - ) -} - -function exposeDebugApi(): void { - if (!debugEnabled || typeof window === 'undefined') { - return - } - // Why: the e2e repro must prove background output used the shared drain, but production must not accumulate diagnostic counters indefinitely. - const target = window as unknown as { - __terminalOutputSchedulerDebug?: TerminalOutputSchedulerDebugApi - } - target.__terminalOutputSchedulerDebug ??= { - reset: resetDebugState, - snapshot: () => { - recordQueueDebugPressure() - return { - ...debugState, - drainWrites: [...debugState.drainWrites], - drainHighPriority: [...debugState.drainHighPriority] - } - } - } -} function scheduleDrain(delayMs: number): void { if (drainImmediatePending) { @@ -430,335 +320,6 @@ function isEntryDrainable(entry: QueueEntry): boolean { return !entry.foregroundHold && !entry.foregroundCoalesce } -function findCursorPositionSequenceEnd( - data: string, - fromIndex: number, - toIndex = data.length -): number { - let offset = data.indexOf('\x1b[', fromIndex) - while (offset !== -1 && offset < toIndex) { - let index = offset + 2 - while (index < toIndex) { - const char = data[index] - if (char === 'G' || char === 'H' || char === 'f') { - return index + 1 - } - if ((char < '0' || char > '9') && char !== ';') { - break - } - index += 1 - } - offset = data.indexOf('\x1b[', offset + 2) - } - return -1 -} - -function removeTransientCursorShowSequences(data: string): string { - let result = '' - let offset = 0 - let showIndex = data.indexOf(CURSOR_SHOW_SEQUENCE) - while (showIndex !== -1) { - const nextHideIndex = data.indexOf( - CURSOR_HIDE_SEQUENCE, - showIndex + CURSOR_SHOW_SEQUENCE.length - ) - const nextPositionEnd = findCursorPositionSequenceEnd( - data, - showIndex + CURSOR_SHOW_SEQUENCE.length, - nextHideIndex === -1 ? data.length : nextHideIndex - ) - if (nextHideIndex === -1) { - if (nextPositionEnd === -1) { - const synchronizedEndIndex = data.indexOf( - SYNCHRONIZED_OUTPUT_END_SEQUENCE, - showIndex + CURSOR_SHOW_SEQUENCE.length - ) - if (synchronizedEndIndex === -1) { - break - } - // Why: keep the cursor hidden through the synchronized repaint, restoring it after the frame ends so Windows never paints it in the transient draw position. - result += data.slice(offset, showIndex) - result += data.slice( - showIndex + CURSOR_SHOW_SEQUENCE.length, - synchronizedEndIndex + SYNCHRONIZED_OUTPUT_END_SEQUENCE.length - ) - result += CURSOR_SHOW_SEQUENCE - offset = synchronizedEndIndex + SYNCHRONIZED_OUTPUT_END_SEQUENCE.length - showIndex = data.indexOf(CURSOR_SHOW_SEQUENCE, offset) - continue - } - // Why: Codex can show the cursor before its final synchronized-frame placement. Place first so xterm cannot rasterize the stale cell. - result += data.slice(offset, showIndex) - result += data.slice(showIndex + CURSOR_SHOW_SEQUENCE.length, nextPositionEnd) - result += CURSOR_SHOW_SEQUENCE - offset = nextPositionEnd - showIndex = data.indexOf(CURSOR_SHOW_SEQUENCE, offset) - continue - } - result += data.slice(offset, showIndex) - offset = showIndex + CURSOR_SHOW_SEQUENCE.length - showIndex = data.indexOf(CURSOR_SHOW_SEQUENCE, offset) - } - return offset === 0 ? data : result + data.slice(offset) -} - -function containsCursorPositionSequence(data: string): boolean { - let offset = data.indexOf('\x1b[') - while (offset !== -1) { - let index = offset + 2 - while (index < data.length) { - const char = data[index] - if (char === 'G' || char === 'H' || char === 'f') { - return true - } - if ((char < '0' || char > '9') && char !== ';') { - break - } - index += 1 - } - offset = data.indexOf('\x1b[', offset + 2) - } - return false -} - -function containsCursorRestore(data: string): boolean { - const hideIndex = data.indexOf(CURSOR_HIDE_SEQUENCE) - const showIndex = data.lastIndexOf(CURSOR_SHOW_SEQUENCE) - return hideIndex !== -1 && showIndex > hideIndex && containsCursorPositionSequence(data) -} - -function containsDrainableCursorRestore(data: string): boolean { - const synchronizedEndIndex = data.lastIndexOf(SYNCHRONIZED_OUTPUT_END_SEQUENCE) - if (synchronizedEndIndex === -1) { - return containsCursorRestore(data) - } - return containsCursorRestore( - data.slice(synchronizedEndIndex + SYNCHRONIZED_OUTPUT_END_SEQUENCE.length) - ) -} - -function containsFinalCursorPlacementBeforeSynchronizedEnd(data: string): boolean { - const synchronizedEndIndex = data.lastIndexOf(SYNCHRONIZED_OUTPUT_END_SEQUENCE) - if (synchronizedEndIndex === -1) { - return false - } - const lastShowIndex = data.lastIndexOf(CURSOR_SHOW_SEQUENCE, synchronizedEndIndex) - if (lastShowIndex === -1) { - return false - } - const lastHideIndex = data.lastIndexOf(CURSOR_HIDE_SEQUENCE, synchronizedEndIndex) - if (lastHideIndex > lastShowIndex) { - return false - } - return ( - findCursorPositionSequenceEnd( - data, - lastShowIndex + CURSOR_SHOW_SEQUENCE.length, - synchronizedEndIndex - ) !== -1 - ) -} - -function previewQueuedData(entry: QueueEntry, limit: number): string { - let data = '' - for (let index = entry.chunkIndex; index < entry.chunks.length; index += 1) { - const chunk = entry.chunks[index] - const remaining = limit - data.length - if (remaining <= 0) { - break - } - data += chunk.data.slice(0, remaining) - } - return data -} - -function coalescedQueuedDataNeedsCursorRestore(entry: QueueEntry): boolean { - const data = previewQueuedData(entry, SYNC_FOREGROUND_FLUSH_CHARS) - const synchronizedEndIndex = data.lastIndexOf(SYNCHRONIZED_OUTPUT_END_SEQUENCE) - if (synchronizedEndIndex === -1) { - return false - } - const synchronizedFrame = data.slice( - 0, - synchronizedEndIndex + SYNCHRONIZED_OUTPUT_END_SEQUENCE.length - ) - return ( - containsCursorRestore(synchronizedFrame) && - !containsFinalCursorPlacementBeforeSynchronizedEnd(synchronizedFrame) && - !containsDrainableCursorRestore(data) - ) -} - -function takeQueuedChunk(entry: QueueEntry, limit: number): QueuedWrite | null { - let remaining = limit - let data = '' - let foreground: boolean | null = null - let forceForegroundRefresh = false - let followupForegroundRefresh = false - let shouldRefreshForegroundSynchronously: ForegroundRefreshSyncResolver | null = null - let additionalRefreshSyncResolvers: ForegroundRefreshSyncResolver[] | null = null - let stripTransientCursorShows = false - let beforeWrite: TerminalOutputBeforeWrite | undefined - let additionalBeforeWriteCallbacks: TerminalOutputBeforeWrite[] | null = null - const parsedCallbacks: TerminalOutputParsedCallback[] = [] - const ackCredits: (() => void)[] = [] - - while (remaining > 0 && entry.chunkIndex < entry.chunks.length) { - const chunk = entry.chunks[entry.chunkIndex] - if (foreground !== null && chunk.foreground !== foreground) { - break - } - foreground ??= chunk.foreground - forceForegroundRefresh ||= chunk.forceForegroundRefresh - followupForegroundRefresh ||= chunk.followupForegroundRefresh - // Why: one drained write can combine chunks from different renderer states or producers; preserve every forced policy and prep hook. - if (chunk.forceForegroundRefresh) { - if (shouldRefreshForegroundSynchronously === null) { - shouldRefreshForegroundSynchronously = chunk.shouldRefreshForegroundSynchronously - } else if ( - chunk.shouldRefreshForegroundSynchronously !== shouldRefreshForegroundSynchronously && - !additionalRefreshSyncResolvers?.includes(chunk.shouldRefreshForegroundSynchronously) - ) { - additionalRefreshSyncResolvers ??= [] - additionalRefreshSyncResolvers.push(chunk.shouldRefreshForegroundSynchronously) - } - } - stripTransientCursorShows ||= chunk.stripTransientCursorShows - if (!beforeWrite) { - beforeWrite = chunk.beforeWrite - } else if ( - chunk.beforeWrite && - chunk.beforeWrite !== beforeWrite && - !additionalBeforeWriteCallbacks?.includes(chunk.beforeWrite) - ) { - additionalBeforeWriteCallbacks ??= [] - additionalBeforeWriteCallbacks.push(chunk.beforeWrite) - } - if (chunk.data.length <= remaining) { - data += chunk.data - remaining -= chunk.data.length - entry.queuedChars -= chunk.data.length - // Clear drained slots before the 64-chunk compaction can release them. - entry.chunks[entry.chunkIndex] = { - data: '', - retainedChars: 0, - foreground: chunk.foreground, - forceForegroundRefresh: false, - followupForegroundRefresh: false, - shouldRefreshForegroundSynchronously: ALWAYS_REFRESH_FOREGROUND_SYNCHRONOUSLY, - stripTransientCursorShows: false - } - entry.chunkIndex += 1 - if (chunk.onParsed) { - parsedCallbacks.push(chunk.onParsed) - } - if (chunk.ackCredit) { - ackCredits.push(chunk.ackCredit) - } - continue - } - - data += chunk.data.slice(0, remaining) - const residual = chunk.data.slice(remaining) - // Geometric flattening bounds retained parents while keeping total copy work linear. - const flatten = residual.length * 2 <= chunk.retainedChars - entry.chunks[entry.chunkIndex] = { - ...chunk, - data: flatten ? flattenRetainedSlice(residual) : residual, - retainedChars: flatten ? residual.length : chunk.retainedChars - } - entry.queuedChars -= remaining - remaining = 0 - } - - compactConsumedChunks(entry) - if (entry.queuedChars < 0) { - entry.queuedChars = 0 - } - recordQueueDebugPressure() - return data - ? { - data, - foreground: foreground === true, - forceForegroundRefresh, - followupForegroundRefresh, - shouldRefreshForegroundSynchronously: - additionalRefreshSyncResolvers && shouldRefreshForegroundSynchronously - ? () => - shouldRefreshForegroundSynchronously() || - additionalRefreshSyncResolvers.some((resolve) => resolve()) - : (shouldRefreshForegroundSynchronously ?? ALWAYS_REFRESH_FOREGROUND_SYNCHRONOUSLY), - stripTransientCursorShows, - beforeWrite: - additionalBeforeWriteCallbacks && beforeWrite - ? (queuedData) => { - beforeWrite(queuedData) - for (const callback of additionalBeforeWriteCallbacks) { - callback(queuedData) - } - } - : beforeWrite, - onParsed: - parsedCallbacks.length > 0 - ? () => { - for (const callback of parsedCallbacks) { - callback() - } - } - : undefined, - ackCredits - } - : null -} - -function compactConsumedChunks(entry: QueueEntry): void { - if (entry.chunkIndex === 0) { - return - } - if (entry.chunkIndex === entry.chunks.length) { - entry.chunks.length = 0 - entry.chunkIndex = 0 - return - } - if (entry.chunkIndex >= 64) { - entry.chunks.splice(0, entry.chunkIndex) - entry.chunkIndex = 0 - } -} - -function enqueueChunk( - entry: QueueEntry, - data: string, - options?: { - foreground?: boolean - forceForegroundRefresh?: boolean - followupForegroundRefresh?: boolean - shouldRefreshForegroundSynchronously?: ForegroundRefreshSyncResolver - stripTransientCursorShows?: boolean - beforeWrite?: TerminalOutputBeforeWrite - onParsed?: TerminalOutputParsedCallback - ackCredit?: () => void - } -): void { - // Own queued data so producer slices cannot pin larger PTY or restore buffers. - const owned = flattenRetainedSlice(data) - entry.chunks.push({ - data: owned, - retainedChars: owned.length, - foreground: options?.foreground === true, - forceForegroundRefresh: options?.forceForegroundRefresh === true, - followupForegroundRefresh: options?.followupForegroundRefresh === true, - shouldRefreshForegroundSynchronously: - options?.shouldRefreshForegroundSynchronously ?? ALWAYS_REFRESH_FOREGROUND_SYNCHRONOUSLY, - stripTransientCursorShows: options?.stripTransientCursorShows === true, - beforeWrite: options?.beforeWrite, - onParsed: options?.onParsed, - ackCredit: options?.ackCredit - }) - entry.queuedChars += data.length - recordQueueDebugPressure() -} - // Why: every discard path MUST fire these before clearing/replacing the queue — a dropped chunk still counts as consumed, or main's in-flight window shrinks permanently and the PTY wedges. function fireQueuedAckCredits(entry: QueueEntry): void { for (let index = entry.chunkIndex; index < entry.chunks.length; index += 1) { From e16f3943fd9171e3ac8875c0a074ca50ecde6183 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Thu, 27 Aug 2026 17:39:04 -0700 Subject: [PATCH 02/10] Extract daemon launch path helpers (#16777) --- src/main/daemon/daemon-init.ts | 113 +++---------------------- src/main/daemon/daemon-launch-paths.ts | 93 ++++++++++++++++++++ 2 files changed, 103 insertions(+), 103 deletions(-) create mode 100644 src/main/daemon/daemon-launch-paths.ts diff --git a/src/main/daemon/daemon-init.ts b/src/main/daemon/daemon-init.ts index 77edbed6027..0c09836f7fe 100644 --- a/src/main/daemon/daemon-init.ts +++ b/src/main/daemon/daemon-init.ts @@ -1,11 +1,9 @@ /* eslint-disable max-lines -- Why: owns the full daemon lifecycle (init, launch, adapter wiring, restart, teardown); the "swap the provider atomically" invariant keeps restart + singletons co-located. */ -import { join } from 'node:path' import { randomUUID } from 'node:crypto' import { getAppEnvironment } from '../../shared/app-environment' -import { mkdirSync, existsSync, readFileSync, unlinkSync } from 'node:fs' +import { readFileSync, unlinkSync } from 'node:fs' import { fork, type ChildProcess } from 'node:child_process' -import { connect } from 'node:net' import { DaemonSpawner, getDaemonPidPath, @@ -52,13 +50,21 @@ import { rebindLocalProviderListeners } from '../ipc/pty' import { isStartupDiagnosticsEnabled, logStartupDiagnostic } from '../startup/startup-diagnostics' -import { getDaemonLogFilePath } from '../observability/logs-directory' import { confirmSeededClaudeLivePtys, hasSeededUnconfirmedClaudePtys } from '../claude-accounts/live-pty-gate' import { parseDaemonReadyIdentity, readDaemonProcessIncarnation } from './daemon-ready-identity' import type { DaemonEndpointIdentity } from './daemon-hello-protocol' +import { + daemonLogArgs, + getAliveDaemonSessionCount, + getDaemonEntryPath, + getDaemonHistoryDir as getHistoryDir, + getDaemonRuntimeDir as getRuntimeDir, + probeDaemonSocket as probeSocket, + resolvePackagedDarwinAppVersion +} from './daemon-launch-paths' // Why: daemon init runs concurrent with window load, so an in-process t timestamp (not harness stderr timing) measures cold-start. function logDaemonMilestone(event: string, details: Record = {}): void { @@ -101,105 +107,6 @@ let adapter: DaemonProvider | null = null // Why: coalesce concurrent restartDaemon() calls so two entries can't race the 7-step sequence against a half-spawned replacement. let restartInFlight: Promise | null = null -function getRuntimeDir(): string { - const dir = join(getAppEnvironment().getPath('userData'), 'daemon') - mkdirSync(dir, { recursive: true }) - return dir -} - -function getHistoryDir(): string { - const dir = join(getAppEnvironment().getPath('userData'), 'terminal-history') - mkdirSync(dir, { recursive: true }) - return dir -} - -function getDaemonEntryPath(): string { - const appPath = getAppEnvironment().getAppPath() - // Why: packaged getAppPath() points at app.asar, so redirect to app.asar.unpacked where daemon-entry.js is fork-executable. - // Why asar and not isPackaged: orcad is a packaged non-Electron host whose bundle root holds - // orcad.js and daemon-entry.js side by side with no asar to redirect (see parcel-watcher-entry-path.ts). - const basePath = appPath.includes('app.asar') - ? appPath.replace('app.asar', 'app.asar.unpacked') - : appPath - const directEntryPath = join(basePath, 'daemon-entry.js') - if (existsSync(directEntryPath)) { - return directEntryPath - } - return join(basePath, 'out', 'main', 'daemon-entry.js') -} - -// macOS TCC attribution pins the daemon to a packaged app bundle; there is none on a Node host. -function resolvePackagedDarwinAppVersion(): string | null { - const environment = getAppEnvironment() - return process.platform === 'darwin' && environment.isPackaged() ? environment.getVersion() : null -} - -// Why: pass a log-file arg so field failures are diagnosable, but honor the ORCA_DIAGNOSTICS_DISABLED privacy switch. -function daemonLogArgs(): string[] { - const disabled = (process.env.ORCA_DIAGNOSTICS_DISABLED ?? '').trim().toLowerCase() - if (disabled === '1' || disabled === 'true') { - return [] - } - return ['--log-file', getDaemonLogFilePath()] -} - -// Why: a socket that accepts a connection proves a daemon survived a previous app session and can be reused. -function probeSocket(socketPath: string): Promise { - return new Promise((resolve) => { - if (process.platform !== 'win32' && !existsSync(socketPath)) { - resolve(false) - return - } - const sock = connect({ path: socketPath }) - let settled = false - let timer: ReturnType - function finish(alive: boolean, options?: { destroy?: boolean }): void { - if (settled) { - return - } - settled = true - clearTimeout(timer) - sock.removeListener('connect', onConnect) - sock.removeListener('error', onError) - if (options?.destroy) { - sock.destroy() - } - resolve(alive) - } - - function onConnect(): void { - finish(true, { destroy: true }) - } - - function onError(): void { - finish(false) - } - - timer = setTimeout(() => { - finish(false, { destroy: true }) - }, 1000) - sock.on('connect', onConnect) - sock.on('error', onError) - }) -} - -async function getAliveDaemonSessionCount( - socketPath: string, - tokenPath: string, - protocolVersion = PROTOCOL_VERSION -): Promise { - const client = new DaemonClient({ socketPath, tokenPath, protocolVersion }) - try { - await client.ensureConnected() - const result = await client.request('listSessions', undefined) - return result.sessions.filter((session) => session.isAlive).length - } catch { - return null - } finally { - client.disconnect() - } -} - function createPreservedDaemonHandle( runtimeDir: string, protocolVersion = PROTOCOL_VERSION, diff --git a/src/main/daemon/daemon-launch-paths.ts b/src/main/daemon/daemon-launch-paths.ts new file mode 100644 index 00000000000..9f01d2770e9 --- /dev/null +++ b/src/main/daemon/daemon-launch-paths.ts @@ -0,0 +1,93 @@ +import { existsSync, mkdirSync } from 'node:fs' +import { connect } from 'node:net' +import { join } from 'node:path' +import { getAppEnvironment } from '../../shared/app-environment' +import { getDaemonLogFilePath } from '../observability/logs-directory' +import { DaemonClient } from './client' +import { PROTOCOL_VERSION, type ListSessionsResult } from './types' + +export function getDaemonRuntimeDir(): string { + const dir = join(getAppEnvironment().getPath('userData'), 'daemon') + mkdirSync(dir, { recursive: true }) + return dir +} + +export function getDaemonHistoryDir(): string { + const dir = join(getAppEnvironment().getPath('userData'), 'terminal-history') + mkdirSync(dir, { recursive: true }) + return dir +} + +export function getDaemonEntryPath(): string { + const appPath = getAppEnvironment().getAppPath() + // Why: packaged getAppPath() points at app.asar, so redirect to app.asar.unpacked where daemon-entry.js is fork-executable. + // Why asar and not isPackaged: orcad is a packaged non-Electron host whose bundle root holds + // orcad.js and daemon-entry.js side by side with no asar to redirect (see parcel-watcher-entry-path.ts). + const basePath = appPath.includes('app.asar') + ? appPath.replace('app.asar', 'app.asar.unpacked') + : appPath + const directEntryPath = join(basePath, 'daemon-entry.js') + return existsSync(directEntryPath) + ? directEntryPath + : join(basePath, 'out', 'main', 'daemon-entry.js') +} + +// macOS TCC attribution pins the daemon to a packaged app bundle; there is none on a Node host. +export function resolvePackagedDarwinAppVersion(): string | null { + const environment = getAppEnvironment() + return process.platform === 'darwin' && environment.isPackaged() ? environment.getVersion() : null +} + +// Why: pass a log-file arg so field failures are diagnosable, but honor the ORCA_DIAGNOSTICS_DISABLED privacy switch. +export function daemonLogArgs(): string[] { + const disabled = (process.env.ORCA_DIAGNOSTICS_DISABLED ?? '').trim().toLowerCase() + return disabled === '1' || disabled === 'true' ? [] : ['--log-file', getDaemonLogFilePath()] +} + +// Why: a socket that accepts a connection proves a daemon survived a previous app session and can be reused. +export function probeDaemonSocket(socketPath: string): Promise { + const { promise, resolve } = Promise.withResolvers() + if (process.platform !== 'win32' && !existsSync(socketPath)) { + resolve(false) + return promise + } + const socket = connect({ path: socketPath }) + let settled = false + let timer: ReturnType + const finish = (alive: boolean, destroy = false): void => { + if (settled) { + return + } + settled = true + clearTimeout(timer) + socket.removeListener('connect', onConnect) + socket.removeListener('error', onError) + if (destroy) { + socket.destroy() + } + resolve(alive) + } + const onConnect = (): void => finish(true, true) + const onError = (): void => finish(false) + timer = setTimeout(() => finish(false, true), 1000) + socket.on('connect', onConnect) + socket.on('error', onError) + return promise +} + +export async function getAliveDaemonSessionCount( + socketPath: string, + tokenPath: string, + protocolVersion = PROTOCOL_VERSION +): Promise { + const client = new DaemonClient({ socketPath, tokenPath, protocolVersion }) + try { + await client.ensureConnected() + const result = await client.request('listSessions', undefined) + return result.sessions.filter((session) => session.isAlive).length + } catch { + return null + } finally { + client.disconnect() + } +} From f48c038a93b1650f18ae29be49d12c2d9a36f736 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Thu, 27 Aug 2026 17:46:36 -0700 Subject: [PATCH 03/10] Extract local PTY launch helpers (#16778) --- .../providers/local-pty-launch-helpers.ts | 98 ++++++++++++ src/main/providers/local-pty-provider.ts | 140 ++---------------- 2 files changed, 111 insertions(+), 127 deletions(-) create mode 100644 src/main/providers/local-pty-launch-helpers.ts diff --git a/src/main/providers/local-pty-launch-helpers.ts b/src/main/providers/local-pty-launch-helpers.ts new file mode 100644 index 00000000000..220669d7cb7 --- /dev/null +++ b/src/main/providers/local-pty-launch-helpers.ts @@ -0,0 +1,98 @@ +import { basename, delimiter, win32 as pathWin32 } from 'node:path' +import { splitWorktreeIdForFilesystem } from '../../shared/worktree/id' +import { parseWslPath } from '../wsl' +import { resolvePathEnvKey } from '../pty/windows-environment-path' +import { expandWindowsEnvironmentVariables } from '../../shared/windows-environment-expansion' +import { resolveSafePtyDefaultCwd } from './pty-default-cwd' + +const PANE_IDENTITY_ENV_KEYS = [ + 'ORCA_PANE_KEY', + 'ORCA_TAB_ID', + 'ORCA_WORKTREE_ID', + 'ORCA_AGENT_LAUNCH_TOKEN' +] as const + +export function getDefaultCwd(): string { + return resolveSafePtyDefaultCwd() +} + +export function removeUnspecifiedPaneIdentityEnv( + env: Record, + explicitEnv: Record | undefined +): void { + for (const key of PANE_IDENTITY_ENV_KEYS) { + if (!explicitEnv || !Object.hasOwn(explicitEnv, key)) { + delete env[key] + } + } +} + +export function promoteAgentTeamsShimPath( + env: Record, + requestedPath: string | undefined +): void { + if (!env.ORCA_AGENT_TEAMS_TEAM_ID || !requestedPath) { + return + } + const normalizedRequestedPath = + process.platform === 'win32' + ? expandWindowsEnvironmentVariables(requestedPath, env) + : requestedPath + const pathDelimiter = process.platform === 'win32' ? ';' : delimiter + const shimDir = normalizedRequestedPath.split(pathDelimiter)[0] + if (!shimDir) { + return + } + const pathKey = resolvePathEnvKey(env, process.platform) + const currentParts = env[pathKey]?.split(pathDelimiter).filter(Boolean) ?? [] + env[pathKey] = [shimDir, ...currentParts.filter((part) => part !== shimDir)].join(pathDelimiter) +} + +export function getWslContextFromWorktreeId( + worktreeId: string | undefined +): { distro: string; treatPosixCwdAsWsl: true } | undefined { + // Why: strip any synthetic `::workspace:` suffix so WSL detection parses the real path, not a nonexistent identifier. + const worktreePath = worktreeId + ? splitWorktreeIdForFilesystem(worktreeId)?.worktreePath + : undefined + const wslInfo = worktreePath ? parseWslPath(worktreePath) : null + return wslInfo ? { distro: wslInfo.distro, treatPosixCwdAsWsl: true } : undefined +} + +export function getWslContextFromPreferredDistro( + distro: string | null | undefined +): { distro: string; treatPosixCwdAsWsl: true } | undefined { + const trimmed = distro?.trim() + return trimmed ? { distro: trimmed, treatPosixCwdAsWsl: true } : undefined +} + +export function normalizeLocalCallerSessionId( + sessionId: string | undefined, + allowNumeric = false +): string | null { + const requested = sessionId?.trim() + return !requested || (!allowNumeric && /^\d+$/.test(requested)) ? null : requested +} + +export function normalizeForegroundProcessName( + processName: string | null | undefined +): string | null { + const trimmed = processName?.trim().replace(/^["']|["']$/g, '') ?? '' + return !trimmed || trimmed === 'xterm-256color' ? null : trimmed.split(/[\\/]/).pop() || null +} + +export function resolveForegroundFallbackProcess( + processName: string | null | undefined, + shellName: string | undefined +): string | null { + if (process.platform !== 'win32' || normalizeForegroundProcessName(processName)) { + return processName || null + } + return shellName ?? processName ?? null +} + +/** Basename of the spawned shell path, parsed for the *target* platform. + * Why: POSIX `basename` won't split a Windows `\` path (non-Windows host/CI), so it'd break the foreground comparison. */ +export function getSpawnedShellName(shellPath: string): string { + return process.platform === 'win32' ? pathWin32.basename(shellPath) : basename(shellPath) +} diff --git a/src/main/providers/local-pty-provider.ts b/src/main/providers/local-pty-provider.ts index 223168b65c0..1ad88aaa2ba 100644 --- a/src/main/providers/local-pty-provider.ts +++ b/src/main/providers/local-pty-provider.ts @@ -1,5 +1,5 @@ /* eslint-disable max-lines -- Why: splitting spawn() would scatter tightly coupled PTY lifecycle logic (scan → ready → write → exit) with no cleaner ownership seam. */ -import { basename, delimiter, win32 as pathWin32 } from 'node:path' +import { basename, win32 as pathWin32 } from 'node:path' import { randomUUID } from 'node:crypto' import { ORCA_CODEX_LAUNCH_PREFLIGHT_CMD_QUOTE_ENV, @@ -15,7 +15,6 @@ import { resolveProcessCwd } from './process-cwd' import { existsSync } from 'node:fs' import * as pty from 'node-pty' import { getDefaultWslDistro, parseWslPath, isWslAvailableAsync } from '../wsl' -import { splitWorktreeIdForFilesystem } from '../../shared/worktree/id' import { isBracketedPasteSafeShell } from '../../shared/startup-command-submission' import { injectHistoryEnv, @@ -76,7 +75,7 @@ import { } from './windows-cached-agent-revalidation' import { forceKillPosixPtyProcessGroups } from '../pty/posix-pty-process-groups' import { shouldUseShellReadyStartupDelivery } from '../../shared/codex-startup-delivery' -import { assertSafeAgentStartupCwd, resolveSafePtyDefaultCwd } from './pty-default-cwd' +import { assertSafeAgentStartupCwd } from './pty-default-cwd' import { ORCA_HERMES_STARTUP_QUERY_ENV } from '../../shared/hermes-startup-query' import { PhysicalExitTracker } from '../../shared/physical-exit-tracker' import { mergeGitConfigEnvProtocol } from '../../shared/git-credential-prompt-env' @@ -94,18 +93,19 @@ import { createShellPromptReadinessProbe, type ShellPromptReadinessProbe } from '../shell-prompt-readiness-probe' -import { - expandWindowsEnvironmentVariables, - expandWindowsPathEnvironmentVariables -} from '../../shared/windows-environment-expansion' +import { expandWindowsPathEnvironmentVariables } from '../../shared/windows-environment-expansion' import { resolveProcessExitCause, type TerminalExitCause } from '../../shared/terminal-exit-cause' -const PANE_IDENTITY_ENV_KEYS = [ - 'ORCA_PANE_KEY', - 'ORCA_TAB_ID', - 'ORCA_WORKTREE_ID', - 'ORCA_AGENT_LAUNCH_TOKEN' -] as const +import { + getDefaultCwd, + getSpawnedShellName, + getWslContextFromPreferredDistro, + getWslContextFromWorktreeId, + normalizeLocalCallerSessionId, + promoteAgentTeamsShimPath, + removeUnspecifiedPaneIdentityEnv, + resolveForegroundFallbackProcess +} from './local-pty-launch-helpers' let ptyCounter = 0 const ptyProcesses = new Map() @@ -174,51 +174,6 @@ const dataListeners = new Set() const exitListeners = new Set() const startupIngressByPty = new Map() -/** - * Returns a stable default cwd for locally spawned PTYs. - */ -function getDefaultCwd(): string { - return resolveSafePtyDefaultCwd() -} - -/** - * Removes inherited pane identity unless this PTY explicitly supplies it. - */ -function removeUnspecifiedPaneIdentityEnv( - env: Record, - explicitEnv: Record | undefined -): void { - for (const key of PANE_IDENTITY_ENV_KEYS) { - if (!explicitEnv || !Object.hasOwn(explicitEnv, key)) { - delete env[key] - } - } -} - -/** - * Promotes the agent-teams shim path ahead of inherited PATH entries. - */ -function promoteAgentTeamsShimPath( - env: Record, - requestedPath: string | undefined -): void { - if (!env.ORCA_AGENT_TEAMS_TEAM_ID || !requestedPath) { - return - } - const normalizedRequestedPath = - process.platform === 'win32' - ? expandWindowsEnvironmentVariables(requestedPath, env) - : requestedPath - const pathDelimiter = process.platform === 'win32' ? ';' : delimiter - const shimDir = normalizedRequestedPath.split(pathDelimiter)[0] - if (!shimDir) { - return - } - const pathKey = resolvePathEnvKey(env, process.platform) - const currentParts = env[pathKey]?.split(pathDelimiter).filter(Boolean) ?? [] - env[pathKey] = [shimDir, ...currentParts.filter((part) => part !== shimDir)].join(pathDelimiter) -} - /** * Disposes native node-pty listeners registered for a PTY id. */ @@ -254,30 +209,6 @@ function runPtyCleanup(id: string): void { cleanup() } -/** - * Resolves a WSL context from a worktree id whose path is already a WSL path. - */ -function getWslContextFromWorktreeId( - worktreeId: string | undefined -): { distro: string; treatPosixCwdAsWsl: true } | undefined { - // Why: strip any synthetic `::workspace:` suffix so WSL detection parses the real path, not a nonexistent identifier. - const worktreePath = worktreeId - ? splitWorktreeIdForFilesystem(worktreeId)?.worktreePath - : undefined - const wslInfo = worktreePath ? parseWslPath(worktreePath) : null - return wslInfo ? { distro: wslInfo.distro, treatPosixCwdAsWsl: true } : undefined -} - -/** - * Resolves a WSL launch context from a user-selected distro name. - */ -function getWslContextFromPreferredDistro( - distro: string | null | undefined -): { distro: string; treatPosixCwdAsWsl: true } | undefined { - const trimmed = distro?.trim() - return trimmed ? { distro: trimmed, treatPosixCwdAsWsl: true } : undefined -} - /** * Removes all local tracking state for a PTY id after teardown. */ @@ -413,20 +344,6 @@ function cancelAllPendingLocalPtySpawns(): void { } } -/** - * Normalizes renderer session ids that should be reused for local PTY reattach. - */ -function normalizeLocalCallerSessionId( - sessionId: string | undefined, - allowNumeric = false -): string | null { - const requested = sessionId?.trim() - if (!requested || (!allowNumeric && /^\d+$/.test(requested))) { - return null - } - return requested -} - function reattachLocalPty(id: string, cols: number, rows: number): PtySpawnResult | null { const existing = ptyProcesses.get(id) if (!existing) { @@ -445,37 +362,6 @@ function reattachLocalPty(id: string, cols: number, rows: number): PtySpawnResul } } -/** - * Normalizes node-pty foreground process strings to executable basenames. - */ -function normalizeForegroundProcessName(processName: string | null | undefined): string | null { - const trimmed = processName?.trim().replace(/^["']|["']$/g, '') ?? '' - if (!trimmed || trimmed === 'xterm-256color') { - return null - } - return trimmed.split(/[\\/]/).pop() || null -} - -/** - * Falls back to the spawned Windows shell when node-pty reports a terminal name. - */ -function resolveForegroundFallbackProcess( - processName: string | null | undefined, - shellName: string | undefined -): string | null { - if (process.platform !== 'win32' || normalizeForegroundProcessName(processName)) { - return processName || null - } - // Why: Windows node-pty may expose only the terminal name (`xterm-256color`); the spawned shell is the best foreground fallback. - return shellName ?? processName ?? null -} - -/** Basename of the spawned shell path, parsed for the *target* platform. - * Why: POSIX `basename` won't split a Windows `\` path (non-Windows host/CI), so it'd break the foreground comparison. */ -function getSpawnedShellName(shellPath: string): string { - return process.platform === 'win32' ? pathWin32.basename(shellPath) : basename(shellPath) -} - /** * Disposes the native PTY handle while avoiding recycled-pid signals on POSIX. */ From bdabb09a07af4eca7d50314e9dcbe25e77017fa6 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Thu, 27 Aug 2026 17:50:11 -0700 Subject: [PATCH 04/10] Extract mobile host and browser helpers (#16779) --- mobile/app/h/[hostId]/index.tsx | 312 +------------- mobile/src/browser/MobileBrowserPane.tsx | 395 +----------------- .../src/browser/mobile-browser-frame-state.ts | 210 ++++++++++ .../src/browser/mobile-browser-pane-styles.ts | 171 ++++++++ .../host-screen/host-screen-primary-styles.ts | 175 ++++++++ .../host-screen-secondary-styles.ts | 139 ++++++ mobile/src/host-screen/host-screen-styles.ts | 7 + 7 files changed, 728 insertions(+), 681 deletions(-) create mode 100644 mobile/src/browser/mobile-browser-frame-state.ts create mode 100644 mobile/src/browser/mobile-browser-pane-styles.ts create mode 100644 mobile/src/host-screen/host-screen-primary-styles.ts create mode 100644 mobile/src/host-screen/host-screen-secondary-styles.ts create mode 100644 mobile/src/host-screen/host-screen-styles.ts diff --git a/mobile/app/h/[hostId]/index.tsx b/mobile/app/h/[hostId]/index.tsx index 3400fd3694b..b211f5cc92f 100644 --- a/mobile/app/h/[hostId]/index.tsx +++ b/mobile/app/h/[hostId]/index.tsx @@ -1,5 +1,5 @@ import { useState, useEffect, useCallback, useMemo, useRef } from 'react' -import { View, Text, StyleSheet, SectionList, Pressable, Alert, RefreshControl } from 'react-native' +import { View, Text, SectionList, Pressable, Alert, RefreshControl } from 'react-native' import { SafeAreaView, useSafeAreaInsets } from 'react-native-safe-area-context' import { useFocusEffect, useLocalSearchParams, usePathname, useRouter } from 'expo-router' import { @@ -69,8 +69,9 @@ import { MobileSearchField } from '../../../src/components/MobileSearchField' import { WorkspaceDetailPlaceholder } from '../../../src/components/WorkspaceDetailPlaceholder' import { getCachedWorktrees, setCachedWorktrees } from '../../../src/cache/worktree-cache' import { setCachedRepos } from '../../../src/cache/repo-cache' -import { colors, radii, spacing, typography } from '../../../src/theme/mobile-theme' +import { colors, spacing } from '../../../src/theme/mobile-theme' import { useResponsiveLayout } from '../../../src/layout/responsive-layout' +import { hostScreenStyles as styles } from '../../../src/host-screen/host-screen-styles' import { leaveHostRoute } from '../../../src/host-route-exit' import { loadPinnedIds, savePinnedIds } from '../../../src/storage/preferences' import { @@ -1410,310 +1411,3 @@ export default function HostWorktreeRoute() { function ListSeparator() { return } - -const styles = StyleSheet.create({ - container: { - flex: 1, - backgroundColor: colors.bgBase - }, - topChrome: { - backgroundColor: colors.bgPanel, - borderBottomWidth: 1, - borderBottomColor: colors.borderSubtle - }, - statusBar: { - flexDirection: 'row', - alignItems: 'center', - justifyContent: 'space-between', - minHeight: 34, - paddingTop: spacing.xs, - paddingHorizontal: spacing.lg - }, - backButton: { - width: 32, - height: 32, - alignItems: 'center', - justifyContent: 'center', - marginRight: spacing.xs - }, - sidebarCollapseButton: { - width: 24, - height: 24, - alignItems: 'center', - justifyContent: 'center', - borderRadius: radii.button, - marginLeft: spacing.xs - }, - hostIdentity: { - flex: 1, - flexDirection: 'row', - alignItems: 'center', - minWidth: 0, - marginRight: spacing.md - }, - hostNameText: { - flex: 1, - fontSize: 15, - fontWeight: '600', - color: colors.textPrimary - }, - reconnectButton: { - paddingVertical: 4, - paddingHorizontal: spacing.sm, - borderRadius: radii.button, - backgroundColor: colors.bgPanel, - borderWidth: 1, - borderColor: colors.borderSubtle - }, - reconnectButtonText: { - color: colors.textPrimary, - fontSize: typography.metaSize, - fontWeight: '600' - }, - toolbar: { - flexDirection: 'row', - alignItems: 'center', - paddingVertical: spacing.xs + 2, - paddingHorizontal: spacing.md, - gap: spacing.sm, - borderBottomWidth: 1, - borderBottomColor: colors.borderSubtle - }, - embeddedToolbar: { - paddingVertical: spacing.xs + 2, - paddingHorizontal: spacing.sm, - gap: spacing.xs, - borderBottomWidth: 1, - borderBottomColor: colors.borderSubtle - }, - embeddedToolbarRow: { - flexDirection: 'row', - alignItems: 'center', - gap: spacing.sm - }, - embeddedFilterChip: { - flex: 1, - minWidth: 0, - height: 30, - justifyContent: 'center', - paddingHorizontal: spacing.xs, - paddingVertical: 0 - }, - embeddedModeButton: { - flex: 1, - minWidth: 0, - height: 30, - justifyContent: 'center', - paddingHorizontal: spacing.xs, - paddingVertical: 0 - }, - filterChip: { - flexDirection: 'row', - alignItems: 'center', - gap: 4, - paddingHorizontal: spacing.sm + 2, - paddingVertical: spacing.xs, - borderRadius: 12, - borderWidth: 1, - borderColor: colors.borderSubtle - }, - filterChipActive: { - borderColor: colors.textSecondary, - backgroundColor: colors.bgRaised - }, - filterChipText: { - fontSize: 12, - color: colors.textSecondary - }, - filterChipTextActive: { - color: colors.textPrimary - }, - modeButton: { - flexDirection: 'row', - alignItems: 'center', - flexShrink: 1, - minWidth: 0, - gap: 4, - paddingHorizontal: spacing.sm, - paddingVertical: spacing.xs - }, - sortLabel: { - flexShrink: 1, - minWidth: 0, - fontSize: 12, - color: colors.textSecondary - }, - toolbarSpacer: { - flex: 1 - }, - floatingWorkspaceHeaderButton: { - width: 32, - height: 32, - alignItems: 'center', - justifyContent: 'center', - marginLeft: spacing.xs - }, - embeddedToolbarIconButton: { - flex: 1, - height: 28, - alignItems: 'center', - justifyContent: 'center', - borderRadius: radii.button - }, - toolbarIconDisabled: { - opacity: 0.6 - }, - searchToggle: { - padding: spacing.xs - }, - searchBar: { - paddingHorizontal: spacing.md, - paddingVertical: spacing.sm, - borderBottomWidth: StyleSheet.hairlineWidth, - borderBottomColor: colors.borderSubtle, - backgroundColor: colors.bgPanel - }, - centered: { - flex: 1, - alignItems: 'center', - justifyContent: 'center' - }, - emptyText: { - color: colors.textSecondary, - fontSize: typography.bodySize - }, - errorText: { - color: colors.statusRed, - fontSize: typography.bodySize - }, - list: { - paddingBottom: spacing.lg - }, - sectionHeader: { - flexDirection: 'row', - alignItems: 'center', - paddingHorizontal: spacing.lg, - paddingTop: spacing.md, - paddingBottom: spacing.xs - }, - sectionIcon: { - marginRight: spacing.xs - }, - sectionRepoIcon: { - marginRight: spacing.xs - }, - sectionTitle: { - fontSize: 11, - fontWeight: '600', - color: colors.textMuted, - textTransform: 'uppercase', - letterSpacing: 0.5 - }, - sectionCount: { - fontSize: 11, - color: colors.textMuted, - marginLeft: spacing.xs - }, - separator: { - height: 1, - backgroundColor: colors.borderSubtle, - marginLeft: spacing.lg + 24, - marginRight: spacing.lg - }, - filterModalHeader: { - flexDirection: 'row', - alignItems: 'center', - justifyContent: 'space-between', - paddingHorizontal: spacing.xs, - marginBottom: spacing.md - }, - filterModalTitle: { - fontSize: 15, - fontWeight: '600', - color: colors.textPrimary - }, - clearFiltersText: { - fontSize: 13, - color: colors.textSecondary - }, - filterSectionLabel: { - fontSize: 11, - fontWeight: '600', - color: colors.textMuted, - textTransform: 'uppercase', - letterSpacing: 0.5, - marginBottom: spacing.xs, - paddingHorizontal: spacing.xs - }, - filterGroup: { - backgroundColor: colors.bgPanel, - borderRadius: 12, - overflow: 'hidden', - marginBottom: spacing.md - }, - filterRow: { - flexDirection: 'row', - alignItems: 'center', - paddingVertical: spacing.md, - paddingHorizontal: spacing.md + 2, - gap: spacing.sm - }, - filterRowText: { - flex: 1, - fontSize: typography.bodySize, - color: colors.textPrimary - }, - filterSeparator: { - height: StyleSheet.hairlineWidth, - backgroundColor: colors.borderSubtle, - marginHorizontal: spacing.md - }, - filterRepoDot: { - width: 8, - height: 8, - borderRadius: 4 - }, - confirmContent: { - paddingBottom: spacing.lg - }, - confirmTitle: { - fontSize: 16, - fontWeight: '700', - color: colors.textPrimary - }, - confirmMessage: { - fontSize: typography.bodySize, - color: colors.textSecondary, - marginTop: spacing.xs, - lineHeight: 20 - }, - confirmButtons: { - flexDirection: 'row', - gap: spacing.sm - }, - confirmBtn: { - flex: 1, - paddingVertical: spacing.sm + 2, - borderRadius: 10, - alignItems: 'center' - }, - confirmBtnCancel: { - backgroundColor: colors.bgPanel - }, - confirmBtnDestructive: { - backgroundColor: colors.statusRed - }, - confirmBtnPressed: { - opacity: 0.7 - }, - confirmBtnCancelText: { - fontSize: typography.bodySize, - fontWeight: '600', - color: colors.textSecondary - }, - confirmBtnDestructiveText: { - fontSize: typography.bodySize, - fontWeight: '600', - color: '#fff' - } -}) diff --git a/mobile/src/browser/MobileBrowserPane.tsx b/mobile/src/browser/MobileBrowserPane.tsx index 6397f69c549..f823f309630 100644 --- a/mobile/src/browser/MobileBrowserPane.tsx +++ b/mobile/src/browser/MobileBrowserPane.tsx @@ -1,7 +1,4 @@ /* oxlint-disable react-doctor/no-adjust-state-on-prop-change -- Why: mobile browser state mirrors a remote desktop screencast session and CDP dialogs, which are external systems that cannot be derived during render. */ -// Why: import from 'buffer' (the npm polyfill), not 'node:buffer' — Metro -// can't resolve Node's builtin in a React Native bundle. -import { Buffer } from 'buffer' import { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react' import { ActivityIndicator, @@ -10,7 +7,6 @@ import { PanResponder, PixelRatio, Pressable, - StyleSheet, Text, TextInput, View, @@ -24,7 +20,7 @@ import type { BrowserScreencastFrame, BrowserScreencastFrameMetadata } from '../transport/browser-screencast-protocol' -import { colors, radii, spacing, typography } from '../theme/mobile-theme' +import { colors } from '../theme/mobile-theme' import { MOBILE_BROWSER_FRAME_MIN_INTERVAL_MS, buildMobileBrowserScreencastRequest, @@ -47,11 +43,32 @@ import { computeBrowserTouchClickRadiusCss, mapScreenToBrowserPoint, readLocalTouchPoint, - type BrowserFrameGeometry, type BrowserPoint, type BrowserTouchLayout, type BrowserZoomState } from './browser-touch-geometry' +import { + MAX_ZOOM, + MIN_ZOOM, + assertRpcOk, + browserErrorMessage, + browserFrameMetadataEqual, + buttonColor, + cacheBrowserFrame, + clearCachedBrowserFramesForWorktree, + createBrowserFrameDataUri, + createPinchGesture, + getCachedBrowserFrame, + makeBrowserFrameCacheKey, + peekCachedBrowserFrame, + shouldSurfaceBrowserError, + updateBrowserImageSource, + updateBrowserLayerVisibility, + updatePinchZoom, + type FrameLayer, + type PinchGesture +} from './mobile-browser-frame-state' +import { mobileBrowserPaneStyles as styles } from './mobile-browser-pane-styles' import { displayBrowserUrl, normalizeBrowserUrl } from './browser-url' import { MobileBrowserAddressField } from './MobileBrowserAddressField' import { resolveMobileBrowserAddressSync } from './mobile-browser-address-sync' @@ -79,15 +96,6 @@ type MobileBrowserPaneProps = { onToast: (message: string, durationMs?: number) => void } -type FrameLayer = 0 | 1 - -type PinchGesture = { - distance: number - scale: number - anchorX: number - anchorY: number -} - type PanGesture = { x: number y: number @@ -105,17 +113,7 @@ const SCROLL_START_SLOP = 22 const LONG_PRESS_MS = 550 const WHEEL_INTERVAL_MS = 70 const TOUCH_CLICK_RADIUS_DIP = 14 -const MIN_ZOOM = 1 -const MAX_ZOOM = 3.5 const DEFAULT_ZOOM: BrowserZoomState = { scale: 1, offsetX: 0, offsetY: 0 } -const BROWSER_FRAME_CACHE_LIMIT = 4 - -type BrowserFrameCacheEntry = { - uri: string - metadata: BrowserScreencastFrameMetadata -} - -const browserFrameCache = new Map() type BrowserPageParams = { worktree: string @@ -1268,350 +1266,3 @@ export function MobileBrowserPane({ ) } - -function buttonColor(enabled: boolean): string { - return enabled ? colors.textSecondary : colors.textMuted -} - -function createBrowserFrameDataUri(frame: BrowserScreencastFrame): string { - return `data:image/${frame.format};base64,${Buffer.from(frame.image).toString('base64')}` -} - -function makeBrowserFrameCacheKey( - worktreeId: string, - browserPageId: string | null, - viewMode: MobileBrowserViewMode -): string | null { - return browserPageId ? `${worktreeId}:${browserPageId}:${viewMode}` : null -} - -function clearCachedBrowserFramesForWorktree(worktreeId: string): void { - const prefix = `${worktreeId}:` - for (const key of browserFrameCache.keys()) { - if (key.startsWith(prefix)) { - browserFrameCache.delete(key) - } - } -} - -function getCachedBrowserFrame(cacheKey: string | null): BrowserFrameCacheEntry | null { - if (!cacheKey) { - return null - } - const cached = browserFrameCache.get(cacheKey) - if (!cached) { - return null - } - browserFrameCache.delete(cacheKey) - browserFrameCache.set(cacheKey, cached) - return cached -} - -function peekCachedBrowserFrame(cacheKey: string | null): BrowserFrameCacheEntry | null { - return cacheKey ? (browserFrameCache.get(cacheKey) ?? null) : null -} - -function cacheBrowserFrame(cacheKey: string | null, entry: BrowserFrameCacheEntry): void { - if (!cacheKey) { - return - } - browserFrameCache.delete(cacheKey) - browserFrameCache.set(cacheKey, entry) - while (browserFrameCache.size > BROWSER_FRAME_CACHE_LIMIT) { - const oldestKey = browserFrameCache.keys().next().value - if (typeof oldestKey !== 'string') { - break - } - browserFrameCache.delete(oldestKey) - } -} - -function updateBrowserLayerVisibility( - layers: [View | null, View | null], - visible: FrameLayer -): void { - for (const [index, layer] of layers.entries()) { - layer?.setNativeProps({ style: { opacity: index === visible ? 1 : 0 } }) - } -} - -function updateBrowserImageSource(image: Image | null, uri: string): void { - // Why: browser frames are large strings; mutating only the native Image - // source avoids re-rendering the whole tab view for every streamed frame. - const source = [{ uri }] - image?.setNativeProps({ source, src: source }) -} - -function assertRpcOk( - response: RpcSuccess | RpcFailure, - fallbackMessage: string -): asserts response is RpcSuccess { - if (!response.ok) { - throw new Error(response.error.message || fallbackMessage) - } -} - -function browserFrameMetadataEqual( - a: BrowserScreencastFrameMetadata | null, - b: BrowserScreencastFrameMetadata -): boolean { - return ( - a?.deviceWidth === b.deviceWidth && - a?.deviceHeight === b.deviceHeight && - a?.pageScaleFactor === b.pageScaleFactor - ) -} - -function browserErrorMessage(error: unknown, fallback: string): string { - return error instanceof Error && error.message ? error.message : fallback -} - -function shouldSurfaceBrowserError(message: string): boolean { - const normalized = message.toLowerCase() - // Why: selector_not_found can be emitted by in-flight page automation while - // the browser is still usable; replacing the frame with it feels like a crash. - return !normalized.includes('selector_not_found') && !normalized.includes('selector not found') -} - -function touchPair(event: GestureResponderEvent): { a: BrowserPoint; b: BrowserPoint } | null { - const touches = event.nativeEvent.touches - if (!touches || touches.length < 2) { - return null - } - const a = readLocalTouchPoint(touches[0]) - const b = readLocalTouchPoint(touches[1]) - return a && b ? { a, b } : null -} - -function pointDistance(a: BrowserPoint, b: BrowserPoint): number { - return Math.hypot(a.x - b.x, a.y - b.y) -} - -function createPinchGesture( - event: GestureResponderEvent, - geometry: BrowserFrameGeometry | null, - zoom: BrowserZoomState -): PinchGesture | null { - if (!geometry) { - return null - } - const pair = touchPair(event) - if (!pair) { - return null - } - const distance = pointDistance(pair.a, pair.b) - if (distance < 8) { - return null - } - const centerX = (pair.a.x + pair.b.x) / 2 - const centerY = (pair.a.y + pair.b.y) / 2 - const frameCenterX = geometry.offsetX + geometry.renderedWidth / 2 + zoom.offsetX - const frameCenterY = geometry.offsetY + geometry.renderedHeight / 2 + zoom.offsetY - return { - distance, - scale: zoom.scale, - anchorX: (centerX - frameCenterX) / zoom.scale, - anchorY: (centerY - frameCenterY) / zoom.scale - } -} - -function updatePinchZoom( - event: GestureResponderEvent, - geometry: BrowserFrameGeometry | null, - pinch: PinchGesture -): BrowserZoomState | null { - if (!geometry) { - return null - } - const pair = touchPair(event) - if (!pair) { - return null - } - const nextScale = Math.min( - MAX_ZOOM, - Math.max(MIN_ZOOM, (pinch.scale * pointDistance(pair.a, pair.b)) / pinch.distance) - ) - const centerX = (pair.a.x + pair.b.x) / 2 - const centerY = (pair.a.y + pair.b.y) / 2 - const baseCenterX = geometry.offsetX + geometry.renderedWidth / 2 - const baseCenterY = geometry.offsetY + geometry.renderedHeight / 2 - return clampBrowserZoomState( - { - scale: nextScale, - offsetX: centerX - baseCenterX - pinch.anchorX * nextScale, - offsetY: centerY - baseCenterY - pinch.anchorY * nextScale - }, - geometry, - MIN_ZOOM, - MAX_ZOOM - ) -} - -const styles = StyleSheet.create({ - root: { - flex: 1, - minHeight: 0, - backgroundColor: colors.bgBase - }, - toolbar: { - minHeight: 32, - flexDirection: 'row', - alignItems: 'center', - gap: spacing.xs, - paddingHorizontal: spacing.sm, - paddingVertical: 2, - borderBottomWidth: 1, - borderBottomColor: colors.borderSubtle, - backgroundColor: colors.bgPanel - }, - viewport: { - flex: 1, - minHeight: 0, - overflow: 'hidden', - backgroundColor: colors.bgBase - }, - browserImageHost: { - ...StyleSheet.absoluteFillObject, - alignItems: 'center', - justifyContent: 'center', - overflow: 'hidden' - }, - browserImageFill: { - width: '100%', - height: '100%' - }, - browserImageLayer: { - ...StyleSheet.absoluteFillObject, - alignItems: 'center', - justifyContent: 'center' - }, - browserImageLayerHidden: { - opacity: 0 - }, - browserZoomOffset: { - alignItems: 'center', - justifyContent: 'center' - }, - browserFrameBox: { - alignItems: 'center', - justifyContent: 'center', - overflow: 'hidden' - }, - browserImage: { - backgroundColor: colors.bgBase - }, - overlay: { - ...StyleSheet.absoluteFillObject, - alignItems: 'center', - justifyContent: 'center', - padding: spacing.xl, - gap: spacing.sm, - backgroundColor: 'rgba(13, 15, 24, 0.2)' - }, - errorText: { - color: colors.textPrimary, - backgroundColor: colors.bgPanel, - borderWidth: 1, - borderColor: colors.borderSubtle, - borderRadius: radii.button, - paddingHorizontal: spacing.md, - paddingVertical: spacing.sm, - fontSize: 13, - textAlign: 'center', - overflow: 'hidden' - }, - dialogOverlay: { - ...StyleSheet.absoluteFillObject, - zIndex: 30, - alignItems: 'center', - justifyContent: 'center', - padding: spacing.xl, - backgroundColor: 'rgba(13, 15, 24, 0.5)' - }, - dialogCard: { - width: '100%', - maxWidth: 360, - borderRadius: radii.card, - borderWidth: 1, - borderColor: colors.borderSubtle, - backgroundColor: colors.bgPanel, - padding: spacing.lg - }, - dialogTitle: { - color: colors.textPrimary, - fontSize: 16, - fontWeight: '600' - }, - dialogMessage: { - color: colors.textSecondary, - fontSize: typography.bodySize, - lineHeight: 20, - marginTop: spacing.sm - }, - dialogActions: { - flexDirection: 'row', - justifyContent: 'flex-end', - gap: spacing.sm, - marginTop: spacing.lg - }, - dialogButton: { - minHeight: 34, - borderRadius: radii.button, - backgroundColor: colors.bgRaised, - paddingHorizontal: spacing.md, - alignItems: 'center', - justifyContent: 'center' - }, - dialogButtonPrimary: { - backgroundColor: colors.textPrimary - }, - dialogButtonPressed: { - opacity: 0.75 - }, - dialogButtonText: { - color: colors.textSecondary, - fontSize: typography.bodySize, - fontWeight: '600' - }, - dialogButtonPrimaryText: { - color: colors.bgBase - }, - keyboardDock: { - zIndex: 20, - borderTopWidth: 1, - borderTopColor: colors.borderSubtle, - backgroundColor: colors.bgPanel - }, - inputRow: { - flexDirection: 'row', - alignItems: 'center', - paddingHorizontal: spacing.md, - paddingTop: spacing.xs, - paddingBottom: spacing.xs + 2 - }, - keyboardInput: { - flex: 1, - height: 34, - backgroundColor: colors.bgRaised, - color: colors.textPrimary, - borderRadius: radii.input, - paddingHorizontal: spacing.md, - fontSize: 14, - fontFamily: typography.monoFamily, - marginRight: spacing.sm - }, - sendButton: { - width: 34, - height: 34, - borderRadius: 17, - alignItems: 'center', - justifyContent: 'center', - backgroundColor: colors.bgRaised - }, - disabled: { - opacity: 0.35 - }, - disabledText: { - color: colors.textMuted - } -}) diff --git a/mobile/src/browser/mobile-browser-frame-state.ts b/mobile/src/browser/mobile-browser-frame-state.ts new file mode 100644 index 00000000000..9609c2ffda4 --- /dev/null +++ b/mobile/src/browser/mobile-browser-frame-state.ts @@ -0,0 +1,210 @@ +import { Buffer } from 'buffer' +import type { GestureResponderEvent, Image, View } from 'react-native' +import type { RpcFailure, RpcSuccess } from '../transport/types' +import type { + BrowserScreencastFrame, + BrowserScreencastFrameMetadata +} from '../transport/browser-screencast-protocol' +import { colors } from '../theme/mobile-theme' +import { + clampBrowserZoomState, + readLocalTouchPoint, + type BrowserFrameGeometry, + type BrowserPoint, + type BrowserZoomState +} from './browser-touch-geometry' +import type { MobileBrowserViewMode } from './browser-screencast-request' + +export type FrameLayer = 0 | 1 +export type PinchGesture = { + distance: number + scale: number + anchorX: number + anchorY: number +} +export type BrowserFrameCacheEntry = { + uri: string + metadata: BrowserScreencastFrameMetadata +} +export const MIN_ZOOM = 1 +export const MAX_ZOOM = 3.5 +const BROWSER_FRAME_CACHE_LIMIT = 4 +const browserFrameCache = new Map() + +export function buttonColor(enabled: boolean): string { + return enabled ? colors.textSecondary : colors.textMuted +} + +export function createBrowserFrameDataUri(frame: BrowserScreencastFrame): string { + return `data:image/${frame.format};base64,${Buffer.from(frame.image).toString('base64')}` +} + +export function makeBrowserFrameCacheKey( + worktreeId: string, + browserPageId: string | null, + viewMode: MobileBrowserViewMode +): string | null { + return browserPageId ? `${worktreeId}:${browserPageId}:${viewMode}` : null +} + +export function clearCachedBrowserFramesForWorktree(worktreeId: string): void { + const prefix = `${worktreeId}:` + for (const key of browserFrameCache.keys()) { + if (key.startsWith(prefix)) { + browserFrameCache.delete(key) + } + } +} + +export function getCachedBrowserFrame(cacheKey: string | null): BrowserFrameCacheEntry | null { + if (!cacheKey) { + return null + } + const cached = browserFrameCache.get(cacheKey) + if (!cached) { + return null + } + browserFrameCache.delete(cacheKey) + browserFrameCache.set(cacheKey, cached) + return cached +} + +export function peekCachedBrowserFrame(cacheKey: string | null): BrowserFrameCacheEntry | null { + return cacheKey ? (browserFrameCache.get(cacheKey) ?? null) : null +} + +export function cacheBrowserFrame(cacheKey: string | null, entry: BrowserFrameCacheEntry): void { + if (!cacheKey) { + return + } + browserFrameCache.delete(cacheKey) + browserFrameCache.set(cacheKey, entry) + while (browserFrameCache.size > BROWSER_FRAME_CACHE_LIMIT) { + const oldestKey = browserFrameCache.keys().next().value + if (typeof oldestKey !== 'string') { + break + } + browserFrameCache.delete(oldestKey) + } +} + +export function updateBrowserLayerVisibility( + layers: [View | null, View | null], + visible: FrameLayer +): void { + for (const [index, layer] of layers.entries()) { + layer?.setNativeProps({ style: { opacity: index === visible ? 1 : 0 } }) + } +} + +export function updateBrowserImageSource(image: Image | null, uri: string): void { + // Why: browser frames are large strings; mutating only the native Image + // source avoids re-rendering the whole tab view for every streamed frame. + const source = [{ uri }] + image?.setNativeProps({ source, src: source }) +} + +export function assertRpcOk( + response: RpcSuccess | RpcFailure, + fallbackMessage: string +): asserts response is RpcSuccess { + if (!response.ok) { + throw new Error(response.error.message || fallbackMessage) + } +} + +export function browserFrameMetadataEqual( + a: BrowserScreencastFrameMetadata | null, + b: BrowserScreencastFrameMetadata +): boolean { + return ( + a?.deviceWidth === b.deviceWidth && + a?.deviceHeight === b.deviceHeight && + a?.pageScaleFactor === b.pageScaleFactor + ) +} + +export function browserErrorMessage(error: unknown, fallback: string): string { + return error instanceof Error && error.message ? error.message : fallback +} + +export function shouldSurfaceBrowserError(message: string): boolean { + const normalized = message.toLowerCase() + // Why: selector_not_found can be emitted by in-flight page automation while + // the browser is still usable; replacing the frame with it feels like a crash. + return !normalized.includes('selector_not_found') && !normalized.includes('selector not found') +} + +function touchPair(event: GestureResponderEvent): { a: BrowserPoint; b: BrowserPoint } | null { + const touches = event.nativeEvent.touches + if (!touches || touches.length < 2) { + return null + } + const a = readLocalTouchPoint(touches[0]) + const b = readLocalTouchPoint(touches[1]) + return a && b ? { a, b } : null +} + +function pointDistance(a: BrowserPoint, b: BrowserPoint): number { + return Math.hypot(a.x - b.x, a.y - b.y) +} + +export function createPinchGesture( + event: GestureResponderEvent, + geometry: BrowserFrameGeometry | null, + zoom: BrowserZoomState +): PinchGesture | null { + if (!geometry) { + return null + } + const pair = touchPair(event) + if (!pair) { + return null + } + const distance = pointDistance(pair.a, pair.b) + if (distance < 8) { + return null + } + const centerX = (pair.a.x + pair.b.x) / 2 + const centerY = (pair.a.y + pair.b.y) / 2 + const frameCenterX = geometry.offsetX + geometry.renderedWidth / 2 + zoom.offsetX + const frameCenterY = geometry.offsetY + geometry.renderedHeight / 2 + zoom.offsetY + return { + distance, + scale: zoom.scale, + anchorX: (centerX - frameCenterX) / zoom.scale, + anchorY: (centerY - frameCenterY) / zoom.scale + } +} + +export function updatePinchZoom( + event: GestureResponderEvent, + geometry: BrowserFrameGeometry | null, + pinch: PinchGesture +): BrowserZoomState | null { + if (!geometry) { + return null + } + const pair = touchPair(event) + if (!pair) { + return null + } + const nextScale = Math.min( + MAX_ZOOM, + Math.max(MIN_ZOOM, (pinch.scale * pointDistance(pair.a, pair.b)) / pinch.distance) + ) + const centerX = (pair.a.x + pair.b.x) / 2 + const centerY = (pair.a.y + pair.b.y) / 2 + const baseCenterX = geometry.offsetX + geometry.renderedWidth / 2 + const baseCenterY = geometry.offsetY + geometry.renderedHeight / 2 + return clampBrowserZoomState( + { + scale: nextScale, + offsetX: centerX - baseCenterX - pinch.anchorX * nextScale, + offsetY: centerY - baseCenterY - pinch.anchorY * nextScale + }, + geometry, + MIN_ZOOM, + MAX_ZOOM + ) +} diff --git a/mobile/src/browser/mobile-browser-pane-styles.ts b/mobile/src/browser/mobile-browser-pane-styles.ts new file mode 100644 index 00000000000..b41428516ba --- /dev/null +++ b/mobile/src/browser/mobile-browser-pane-styles.ts @@ -0,0 +1,171 @@ +import { StyleSheet } from 'react-native' +import { colors, radii, spacing, typography } from '../theme/mobile-theme' + +export const mobileBrowserPaneStyles = StyleSheet.create({ + root: { + flex: 1, + minHeight: 0, + backgroundColor: colors.bgBase + }, + toolbar: { + minHeight: 32, + flexDirection: 'row', + alignItems: 'center', + gap: spacing.xs, + paddingHorizontal: spacing.sm, + paddingVertical: 2, + borderBottomWidth: 1, + borderBottomColor: colors.borderSubtle, + backgroundColor: colors.bgPanel + }, + viewport: { + flex: 1, + minHeight: 0, + overflow: 'hidden', + backgroundColor: colors.bgBase + }, + browserImageHost: { + ...StyleSheet.absoluteFillObject, + alignItems: 'center', + justifyContent: 'center', + overflow: 'hidden' + }, + browserImageFill: { + width: '100%', + height: '100%' + }, + browserImageLayer: { + ...StyleSheet.absoluteFillObject, + alignItems: 'center', + justifyContent: 'center' + }, + browserImageLayerHidden: { + opacity: 0 + }, + browserZoomOffset: { + alignItems: 'center', + justifyContent: 'center' + }, + browserFrameBox: { + alignItems: 'center', + justifyContent: 'center', + overflow: 'hidden' + }, + browserImage: { + backgroundColor: colors.bgBase + }, + overlay: { + ...StyleSheet.absoluteFillObject, + alignItems: 'center', + justifyContent: 'center', + padding: spacing.xl, + gap: spacing.sm, + backgroundColor: 'rgba(13, 15, 24, 0.2)' + }, + errorText: { + color: colors.textPrimary, + backgroundColor: colors.bgPanel, + borderWidth: 1, + borderColor: colors.borderSubtle, + borderRadius: radii.button, + paddingHorizontal: spacing.md, + paddingVertical: spacing.sm, + fontSize: 13, + textAlign: 'center', + overflow: 'hidden' + }, + dialogOverlay: { + ...StyleSheet.absoluteFillObject, + zIndex: 30, + alignItems: 'center', + justifyContent: 'center', + padding: spacing.xl, + backgroundColor: 'rgba(13, 15, 24, 0.5)' + }, + dialogCard: { + width: '100%', + maxWidth: 360, + borderRadius: radii.card, + borderWidth: 1, + borderColor: colors.borderSubtle, + backgroundColor: colors.bgPanel, + padding: spacing.lg + }, + dialogTitle: { + color: colors.textPrimary, + fontSize: 16, + fontWeight: '600' + }, + dialogMessage: { + color: colors.textSecondary, + fontSize: typography.bodySize, + lineHeight: 20, + marginTop: spacing.sm + }, + dialogActions: { + flexDirection: 'row', + justifyContent: 'flex-end', + gap: spacing.sm, + marginTop: spacing.lg + }, + dialogButton: { + minHeight: 34, + borderRadius: radii.button, + backgroundColor: colors.bgRaised, + paddingHorizontal: spacing.md, + alignItems: 'center', + justifyContent: 'center' + }, + dialogButtonPrimary: { + backgroundColor: colors.textPrimary + }, + dialogButtonPressed: { + opacity: 0.75 + }, + dialogButtonText: { + color: colors.textSecondary, + fontSize: typography.bodySize, + fontWeight: '600' + }, + dialogButtonPrimaryText: { + color: colors.bgBase + }, + keyboardDock: { + zIndex: 20, + borderTopWidth: 1, + borderTopColor: colors.borderSubtle, + backgroundColor: colors.bgPanel + }, + inputRow: { + flexDirection: 'row', + alignItems: 'center', + paddingHorizontal: spacing.md, + paddingTop: spacing.xs, + paddingBottom: spacing.xs + 2 + }, + keyboardInput: { + flex: 1, + height: 34, + backgroundColor: colors.bgRaised, + color: colors.textPrimary, + borderRadius: radii.input, + paddingHorizontal: spacing.md, + fontSize: 14, + fontFamily: typography.monoFamily, + marginRight: spacing.sm + }, + sendButton: { + width: 34, + height: 34, + borderRadius: 17, + alignItems: 'center', + justifyContent: 'center', + backgroundColor: colors.bgRaised + }, + disabled: { + opacity: 0.35 + }, + disabledText: { + color: colors.textMuted + } +}) diff --git a/mobile/src/host-screen/host-screen-primary-styles.ts b/mobile/src/host-screen/host-screen-primary-styles.ts new file mode 100644 index 00000000000..c499ad2afe6 --- /dev/null +++ b/mobile/src/host-screen/host-screen-primary-styles.ts @@ -0,0 +1,175 @@ +import { StyleSheet } from 'react-native' +import { colors, radii, spacing, typography } from '../theme/mobile-theme' + +export const hostScreenPrimaryStyles = StyleSheet.create({ + container: { + flex: 1, + backgroundColor: colors.bgBase + }, + topChrome: { + backgroundColor: colors.bgPanel, + borderBottomWidth: 1, + borderBottomColor: colors.borderSubtle + }, + statusBar: { + flexDirection: 'row', + alignItems: 'center', + justifyContent: 'space-between', + minHeight: 34, + paddingTop: spacing.xs, + paddingHorizontal: spacing.lg + }, + backButton: { + width: 32, + height: 32, + alignItems: 'center', + justifyContent: 'center', + marginRight: spacing.xs + }, + sidebarCollapseButton: { + width: 24, + height: 24, + alignItems: 'center', + justifyContent: 'center', + borderRadius: radii.button, + marginLeft: spacing.xs + }, + hostIdentity: { + flex: 1, + flexDirection: 'row', + alignItems: 'center', + minWidth: 0, + marginRight: spacing.md + }, + hostNameText: { + flex: 1, + fontSize: 15, + fontWeight: '600', + color: colors.textPrimary + }, + reconnectButton: { + paddingVertical: 4, + paddingHorizontal: spacing.sm, + borderRadius: radii.button, + backgroundColor: colors.bgPanel, + borderWidth: 1, + borderColor: colors.borderSubtle + }, + reconnectButtonText: { + color: colors.textPrimary, + fontSize: typography.metaSize, + fontWeight: '600' + }, + toolbar: { + flexDirection: 'row', + alignItems: 'center', + paddingVertical: spacing.xs + 2, + paddingHorizontal: spacing.md, + gap: spacing.sm, + borderBottomWidth: 1, + borderBottomColor: colors.borderSubtle + }, + embeddedToolbar: { + paddingVertical: spacing.xs + 2, + paddingHorizontal: spacing.sm, + gap: spacing.xs, + borderBottomWidth: 1, + borderBottomColor: colors.borderSubtle + }, + embeddedToolbarRow: { + flexDirection: 'row', + alignItems: 'center', + gap: spacing.sm + }, + embeddedFilterChip: { + flex: 1, + minWidth: 0, + height: 30, + justifyContent: 'center', + paddingHorizontal: spacing.xs, + paddingVertical: 0 + }, + embeddedModeButton: { + flex: 1, + minWidth: 0, + height: 30, + justifyContent: 'center', + paddingHorizontal: spacing.xs, + paddingVertical: 0 + }, + filterChip: { + flexDirection: 'row', + alignItems: 'center', + gap: 4, + paddingHorizontal: spacing.sm + 2, + paddingVertical: spacing.xs, + borderRadius: 12, + borderWidth: 1, + borderColor: colors.borderSubtle + }, + filterChipActive: { + borderColor: colors.textSecondary, + backgroundColor: colors.bgRaised + }, + filterChipText: { + fontSize: 12, + color: colors.textSecondary + }, + filterChipTextActive: { + color: colors.textPrimary + }, + modeButton: { + flexDirection: 'row', + alignItems: 'center', + flexShrink: 1, + minWidth: 0, + gap: 4, + paddingHorizontal: spacing.sm, + paddingVertical: spacing.xs + }, + sortLabel: { + flexShrink: 1, + minWidth: 0, + fontSize: 12, + color: colors.textSecondary + }, + toolbarSpacer: { + flex: 1 + }, + floatingWorkspaceHeaderButton: { + width: 32, + height: 32, + alignItems: 'center', + justifyContent: 'center', + marginLeft: spacing.xs + }, + embeddedToolbarIconButton: { + flex: 1, + height: 28, + alignItems: 'center', + justifyContent: 'center', + borderRadius: radii.button + }, + toolbarIconDisabled: { + opacity: 0.6 + }, + searchToggle: { + padding: spacing.xs + }, + searchBar: { + paddingHorizontal: spacing.md, + paddingVertical: spacing.sm, + borderBottomWidth: StyleSheet.hairlineWidth, + borderBottomColor: colors.borderSubtle, + backgroundColor: colors.bgPanel + }, + centered: { + flex: 1, + alignItems: 'center', + justifyContent: 'center' + }, + emptyText: { + color: colors.textSecondary, + fontSize: typography.bodySize + } +}) diff --git a/mobile/src/host-screen/host-screen-secondary-styles.ts b/mobile/src/host-screen/host-screen-secondary-styles.ts new file mode 100644 index 00000000000..af14aa2b97a --- /dev/null +++ b/mobile/src/host-screen/host-screen-secondary-styles.ts @@ -0,0 +1,139 @@ +import { StyleSheet } from 'react-native' +import { colors, spacing, typography } from '../theme/mobile-theme' + +export const hostScreenSecondaryStyles = StyleSheet.create({ + errorText: { + color: colors.statusRed, + fontSize: typography.bodySize + }, + list: { + paddingBottom: spacing.lg + }, + sectionHeader: { + flexDirection: 'row', + alignItems: 'center', + paddingHorizontal: spacing.lg, + paddingTop: spacing.md, + paddingBottom: spacing.xs + }, + sectionIcon: { + marginRight: spacing.xs + }, + sectionRepoIcon: { + marginRight: spacing.xs + }, + sectionTitle: { + fontSize: 11, + fontWeight: '600', + color: colors.textMuted, + textTransform: 'uppercase', + letterSpacing: 0.5 + }, + sectionCount: { + fontSize: 11, + color: colors.textMuted, + marginLeft: spacing.xs + }, + separator: { + height: 1, + backgroundColor: colors.borderSubtle, + marginLeft: spacing.lg + 24, + marginRight: spacing.lg + }, + filterModalHeader: { + flexDirection: 'row', + alignItems: 'center', + justifyContent: 'space-between', + paddingHorizontal: spacing.xs, + marginBottom: spacing.md + }, + filterModalTitle: { + fontSize: 15, + fontWeight: '600', + color: colors.textPrimary + }, + clearFiltersText: { + fontSize: 13, + color: colors.textSecondary + }, + filterSectionLabel: { + fontSize: 11, + fontWeight: '600', + color: colors.textMuted, + textTransform: 'uppercase', + letterSpacing: 0.5, + marginBottom: spacing.xs, + paddingHorizontal: spacing.xs + }, + filterGroup: { + backgroundColor: colors.bgPanel, + borderRadius: 12, + overflow: 'hidden', + marginBottom: spacing.md + }, + filterRow: { + flexDirection: 'row', + alignItems: 'center', + paddingVertical: spacing.md, + paddingHorizontal: spacing.md + 2, + gap: spacing.sm + }, + filterRowText: { + flex: 1, + fontSize: typography.bodySize, + color: colors.textPrimary + }, + filterSeparator: { + height: StyleSheet.hairlineWidth, + backgroundColor: colors.borderSubtle, + marginHorizontal: spacing.md + }, + filterRepoDot: { + width: 8, + height: 8, + borderRadius: 4 + }, + confirmContent: { + paddingBottom: spacing.lg + }, + confirmTitle: { + fontSize: 16, + fontWeight: '700', + color: colors.textPrimary + }, + confirmMessage: { + fontSize: typography.bodySize, + color: colors.textSecondary, + marginTop: spacing.xs, + lineHeight: 20 + }, + confirmButtons: { + flexDirection: 'row', + gap: spacing.sm + }, + confirmBtn: { + flex: 1, + paddingVertical: spacing.sm + 2, + borderRadius: 10, + alignItems: 'center' + }, + confirmBtnCancel: { + backgroundColor: colors.bgPanel + }, + confirmBtnDestructive: { + backgroundColor: colors.statusRed + }, + confirmBtnPressed: { + opacity: 0.7 + }, + confirmBtnCancelText: { + fontSize: typography.bodySize, + fontWeight: '600', + color: colors.textSecondary + }, + confirmBtnDestructiveText: { + fontSize: typography.bodySize, + fontWeight: '600', + color: '#fff' + } +}) diff --git a/mobile/src/host-screen/host-screen-styles.ts b/mobile/src/host-screen/host-screen-styles.ts new file mode 100644 index 00000000000..f4fdb7bed5d --- /dev/null +++ b/mobile/src/host-screen/host-screen-styles.ts @@ -0,0 +1,7 @@ +import { hostScreenPrimaryStyles } from './host-screen-primary-styles' +import { hostScreenSecondaryStyles } from './host-screen-secondary-styles' + +export const hostScreenStyles = { + ...hostScreenPrimaryStyles, + ...hostScreenSecondaryStyles +} From c90d9c4780b7323b3c413b2b3a18fcdcccb42d26 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Thu, 27 Aug 2026 17:57:29 -0700 Subject: [PATCH 05/10] Extract GitLab dialog presentation parts (#16780) --- .../src/components/GitLabItemDialog.tsx | 288 +----------------- .../components/gitlab-item-dialog-parts.tsx | 285 +++++++++++++++++ 2 files changed, 299 insertions(+), 274 deletions(-) create mode 100644 src/renderer/src/components/gitlab-item-dialog-parts.tsx diff --git a/src/renderer/src/components/GitLabItemDialog.tsx b/src/renderer/src/components/GitLabItemDialog.tsx index aeddf718975..1d32d254084 100644 --- a/src/renderer/src/components/GitLabItemDialog.tsx +++ b/src/renderer/src/components/GitLabItemDialog.tsx @@ -47,11 +47,23 @@ import type { GitLabMRUpdate, GitLabPipelineJob, GitLabWorkItem, - GitLabWorkItemDetails, - MRComment + GitLabWorkItemDetails } from '../../../shared/gitlab-types' import type { TaskSourceContext } from '../../../shared/task-source-context' import { translate } from '@/i18n/i18n' +import { + CommentCard, + PipelineJobRow, + StateBadge, + dedupeGitLabUsers, + formatGitLabLabelDraft, + gitLabUserKey, + normalizeGitLabLabels, + parseGitLabLabelDraft, + showGitLabMutationError, + toggleGitLabLabelDraft, + type JobTraceState +} from './gitlab-item-dialog-parts' type Props = { item: GitLabWorkItem | null @@ -68,278 +80,6 @@ type GitLabDialogRepoSelector = { sourceContext?: TaskSourceContext | null } -type JobTraceState = { - loading: boolean - trace?: string - error?: string -} - -// Why: GitLab MR / issue states map onto a coarser palette than GitHub. -const STATE_TONE: Record = { - opened: 'bg-emerald-500/15 text-emerald-700 dark:text-emerald-300', - closed: 'bg-rose-500/15 text-rose-700 dark:text-rose-300', - merged: 'bg-violet-500/15 text-violet-700 dark:text-violet-300', - locked: 'bg-rose-500/15 text-rose-700 dark:text-rose-300', - draft: 'bg-amber-500/15 text-amber-700 dark:text-amber-300' -} - -// Why: pipeline job statuses map to one of four visual buckets — keep -// the mapping local so the renderer doesn't depend on the backend's -// shared mapper module (which is main-process only). -function jobStatusTone(status: string): string { - switch (status) { - case 'success': - return 'bg-emerald-500/15 text-emerald-700 dark:text-emerald-300' - case 'failed': - return 'bg-rose-500/15 text-rose-700 dark:text-rose-300' - case 'running': - case 'pending': - case 'created': - case 'preparing': - case 'waiting_for_resource': - case 'scheduled': - return 'bg-sky-500/15 text-sky-700 dark:text-sky-300' - case 'manual': - return 'bg-amber-500/15 text-amber-700 dark:text-amber-300' - case 'canceled': - case 'skipped': - default: - return 'bg-muted text-muted-foreground' - } -} - -function showGitLabMutationError(error: unknown): void { - const message = error instanceof Error && error.message ? error.message : String(error) - toast.error( - message === 'undefined' || message === 'null' - ? translate('auto.components.GitLabItemDialog.gitlabActionFailed', 'GitLab action failed.') - : message - ) -} - -function StateBadge({ state }: { state: GitLabWorkItem['state'] }): React.JSX.Element { - return ( - - {state} - - ) -} - -function normalizeGitLabLabels(labels: readonly string[]): string[] { - const seen = new Set() - const normalized: string[] = [] - for (const label of labels) { - const trimmed = label.trim() - const key = trimmed.toLowerCase() - if (!trimmed || seen.has(key)) { - continue - } - seen.add(key) - normalized.push(trimmed) - } - return normalized -} - -function parseGitLabLabelDraft(value: string): string[] { - return normalizeGitLabLabels(value.split(',')) -} - -function formatGitLabLabelDraft(labels: readonly string[]): string { - return normalizeGitLabLabels(labels).join(', ') -} - -function toggleGitLabLabelDraft(value: string, label: string): string { - const labels = parseGitLabLabelDraft(value) - const key = label.trim().toLowerCase() - const next = labels.some((item) => item.toLowerCase() === key) - ? labels.filter((item) => item.toLowerCase() !== key) - : [...labels, label] - return formatGitLabLabelDraft(next) -} - -function gitLabUserKey(user: GitLabAssignableUser): string { - return typeof user.id === 'number' ? `id:${user.id}` : `username:${user.username.toLowerCase()}` -} - -function dedupeGitLabUsers(users: readonly GitLabAssignableUser[]): GitLabAssignableUser[] { - const byKey = new Map() - for (const user of users) { - byKey.set(gitLabUserKey(user), user) - } - return Array.from(byKey.values()).sort((a, b) => a.username.localeCompare(b.username)) -} - -function CommentCard({ - comment, - canResolve, - resolving, - onResolve -}: { - comment: MRComment - canResolve?: boolean - resolving?: boolean - onResolve?: (threadId: string, resolved: boolean) => void -}): React.JSX.Element { - const hasThread = Boolean(comment.threadId) - return ( -
-
-
- {comment.authorAvatarUrl ? ( - { - e.currentTarget.style.display = 'none' - }} - /> - ) : null} - {comment.author} - {comment.isResolved ? ( - - {translate('auto.components.GitLabItemDialog.f23ea85341', 'resolved')} - - ) : null} -
-
- {canResolve && hasThread && onResolve ? ( - - ) : null} - {comment.createdAt ? new Date(comment.createdAt).toLocaleDateString() : ''} -
-
- {comment.path ? ( -
- {comment.path} - {comment.line ? `:${comment.line}` : ''} -
- ) : null} - -
- ) -} - -function PipelineJobRow({ - job, - expanded, - traceState, - retrying, - onToggleTrace, - onRetry -}: { - job: GitLabPipelineJob - expanded: boolean - traceState?: JobTraceState - retrying: boolean - onToggleTrace: (job: GitLabPipelineJob) => void - onRetry: (job: GitLabPipelineJob) => void -}): React.JSX.Element { - const canRetry = ['failed', 'canceled', 'cancelled'].includes(job.status) - return ( -
-
- - {job.stage} - - {job.status} - - - {/* Why: durations come back as seconds; show "Nm Ns" for >60s - and "Ns" otherwise. null = job hasn't finished. */} - {typeof job.duration === 'number' - ? job.duration >= 60 - ? `${Math.floor(job.duration / 60)}m ${Math.floor(job.duration % 60)}s` - : `${Math.floor(job.duration)}s` - : '—'} - -
- {canRetry ? ( - - ) : null} - {job.webUrl ? ( - - ) : null} -
-
- {expanded ? ( -
-
- {translate('auto.components.GitLabItemDialog.2f9b27f838', 'Job log')} - -
- {traceState?.loading ? ( -
- - {translate('auto.components.GitLabItemDialog.d600c2619a', 'Loading log')} -
- ) : traceState?.error ? ( -
{traceState.error}
- ) : ( -
-              {traceState?.trace?.trim()
-                ? traceState.trace
-                : translate('auto.components.GitLabItemDialog.32f8bef818', 'No log output.')}
-            
- )} -
- ) : null} -
- ) -} - export default function GitLabItemDialog({ item, repoPath, diff --git a/src/renderer/src/components/gitlab-item-dialog-parts.tsx b/src/renderer/src/components/gitlab-item-dialog-parts.tsx new file mode 100644 index 00000000000..fe16ff10e39 --- /dev/null +++ b/src/renderer/src/components/gitlab-item-dialog-parts.tsx @@ -0,0 +1,285 @@ +import React from 'react' +import { ExternalLink, LoaderCircle } from 'lucide-react' +import { toast } from 'sonner' +import { Button } from '@/components/ui/button' +import CommentMarkdown from '@/components/sidebar/CommentMarkdown' +import { cn } from '@/lib/utils' +import { translate } from '@/i18n/i18n' +import type { + GitLabAssignableUser, + GitLabPipelineJob, + GitLabWorkItem, + MRComment +} from '../../../shared/gitlab-types' + +export type JobTraceState = { + loading: boolean + trace?: string + error?: string +} + +// Why: GitLab MR / issue states map onto a coarser palette than GitHub. +export const STATE_TONE: Record = { + opened: 'bg-emerald-500/15 text-emerald-700 dark:text-emerald-300', + closed: 'bg-rose-500/15 text-rose-700 dark:text-rose-300', + merged: 'bg-violet-500/15 text-violet-700 dark:text-violet-300', + locked: 'bg-rose-500/15 text-rose-700 dark:text-rose-300', + draft: 'bg-amber-500/15 text-amber-700 dark:text-amber-300' +} + +// Why: pipeline job statuses map to one of four visual buckets — keep +// the mapping local so the renderer doesn't depend on the backend's +// shared mapper module (which is main-process only). +export function jobStatusTone(status: string): string { + switch (status) { + case 'success': + return 'bg-emerald-500/15 text-emerald-700 dark:text-emerald-300' + case 'failed': + return 'bg-rose-500/15 text-rose-700 dark:text-rose-300' + case 'running': + case 'pending': + case 'created': + case 'preparing': + case 'waiting_for_resource': + case 'scheduled': + return 'bg-sky-500/15 text-sky-700 dark:text-sky-300' + case 'manual': + return 'bg-amber-500/15 text-amber-700 dark:text-amber-300' + case 'canceled': + case 'skipped': + default: + return 'bg-muted text-muted-foreground' + } +} + +export function showGitLabMutationError(error: unknown): void { + const message = error instanceof Error && error.message ? error.message : String(error) + toast.error( + message === 'undefined' || message === 'null' + ? translate('auto.components.GitLabItemDialog.gitlabActionFailed', 'GitLab action failed.') + : message + ) +} + +export function StateBadge({ state }: { state: GitLabWorkItem['state'] }): React.JSX.Element { + return ( + + {state} + + ) +} + +export function normalizeGitLabLabels(labels: readonly string[]): string[] { + const seen = new Set() + const normalized: string[] = [] + for (const label of labels) { + const trimmed = label.trim() + const key = trimmed.toLowerCase() + if (!trimmed || seen.has(key)) { + continue + } + seen.add(key) + normalized.push(trimmed) + } + return normalized +} + +export function parseGitLabLabelDraft(value: string): string[] { + return normalizeGitLabLabels(value.split(',')) +} + +export function formatGitLabLabelDraft(labels: readonly string[]): string { + return normalizeGitLabLabels(labels).join(', ') +} + +export function toggleGitLabLabelDraft(value: string, label: string): string { + const labels = parseGitLabLabelDraft(value) + const key = label.trim().toLowerCase() + const next = labels.some((item) => item.toLowerCase() === key) + ? labels.filter((item) => item.toLowerCase() !== key) + : [...labels, label] + return formatGitLabLabelDraft(next) +} + +export function gitLabUserKey(user: GitLabAssignableUser): string { + return typeof user.id === 'number' ? `id:${user.id}` : `username:${user.username.toLowerCase()}` +} + +export function dedupeGitLabUsers(users: readonly GitLabAssignableUser[]): GitLabAssignableUser[] { + const byKey = new Map() + for (const user of users) { + byKey.set(gitLabUserKey(user), user) + } + return Array.from(byKey.values()).sort((a, b) => a.username.localeCompare(b.username)) +} + +export function CommentCard({ + comment, + canResolve, + resolving, + onResolve +}: { + comment: MRComment + canResolve?: boolean + resolving?: boolean + onResolve?: (threadId: string, resolved: boolean) => void +}): React.JSX.Element { + const hasThread = Boolean(comment.threadId) + return ( +
+
+
+ {comment.authorAvatarUrl ? ( + { + e.currentTarget.style.display = 'none' + }} + /> + ) : null} + {comment.author} + {comment.isResolved ? ( + + {translate('auto.components.GitLabItemDialog.f23ea85341', 'resolved')} + + ) : null} +
+
+ {canResolve && hasThread && onResolve ? ( + + ) : null} + {comment.createdAt ? new Date(comment.createdAt).toLocaleDateString() : ''} +
+
+ {comment.path ? ( +
+ {comment.path} + {comment.line ? `:${comment.line}` : ''} +
+ ) : null} + +
+ ) +} + +export function PipelineJobRow({ + job, + expanded, + traceState, + retrying, + onToggleTrace, + onRetry +}: { + job: GitLabPipelineJob + expanded: boolean + traceState?: JobTraceState + retrying: boolean + onToggleTrace: (job: GitLabPipelineJob) => void + onRetry: (job: GitLabPipelineJob) => void +}): React.JSX.Element { + const canRetry = ['failed', 'canceled', 'cancelled'].includes(job.status) + return ( +
+
+ + {job.stage} + + {job.status} + + + {/* Why: durations come back as seconds; show "Nm Ns" for >60s + and "Ns" otherwise. null = job hasn't finished. */} + {typeof job.duration === 'number' + ? job.duration >= 60 + ? `${Math.floor(job.duration / 60)}m ${Math.floor(job.duration % 60)}s` + : `${Math.floor(job.duration)}s` + : '—'} + +
+ {canRetry ? ( + + ) : null} + {job.webUrl ? ( + + ) : null} +
+
+ {expanded ? ( +
+
+ {translate('auto.components.GitLabItemDialog.2f9b27f838', 'Job log')} + +
+ {traceState?.loading ? ( +
+ + {translate('auto.components.GitLabItemDialog.d600c2619a', 'Loading log')} +
+ ) : traceState?.error ? ( +
{traceState.error}
+ ) : ( +
+              {traceState?.trace?.trim()
+                ? traceState.trace
+                : translate('auto.components.GitLabItemDialog.32f8bef818', 'No log output.')}
+            
+ )} +
+ ) : null} +
+ ) +} From 3ed7796624c4d8ffaf82bb8e13d562c3119e00a7 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Thu, 27 Aug 2026 18:16:06 -0700 Subject: [PATCH 06/10] fix(fork-sync): address runtime repos by main worktree id, not repo id (#16876) Repo-level fork sync (Safe Auto and the Sync Now button) passed `repo.id` as the runtime worktree selector, so runtime-hosted repos always failed with `worktree_id_requires_full_path`. Compose the repo's main worktree id (`::`) via a new shared `getRepoMainWorktreeId`. Fixes #16447 --- .../RepositoryForkSyncSection.test.tsx | 54 ++++++++++ .../settings/RepositoryForkSyncSection.tsx | 3 +- .../store/repos/safe-auto-fork-sync.test.ts | 98 +++++++++++++++++++ .../src/store/repos/safe-auto-fork-sync.ts | 3 +- src/shared/worktree/id.test.ts | 14 +++ src/shared/worktree/id.ts | 9 ++ 6 files changed, 179 insertions(+), 2 deletions(-) create mode 100644 src/renderer/src/components/settings/RepositoryForkSyncSection.test.tsx create mode 100644 src/renderer/src/store/repos/safe-auto-fork-sync.test.ts diff --git a/src/renderer/src/components/settings/RepositoryForkSyncSection.test.tsx b/src/renderer/src/components/settings/RepositoryForkSyncSection.test.tsx new file mode 100644 index 00000000000..53545eaf740 --- /dev/null +++ b/src/renderer/src/components/settings/RepositoryForkSyncSection.test.tsx @@ -0,0 +1,54 @@ +// @vitest-environment happy-dom +import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { Repo } from '../../../../shared/repo-types' +import { RepositoryForkSyncSection } from './RepositoryForkSyncSection' + +const syncRuntimeGitForkDefaultBranch = vi.fn() + +vi.mock('../../runtime/runtime-git-client', () => ({ + syncRuntimeGitForkDefaultBranch: (...args: unknown[]) => syncRuntimeGitForkDefaultBranch(...args) +})) + +vi.mock('../../store', () => ({ + useAppStore: (selector: (state: Record) => unknown) => + selector({ settingsSearchQuery: '', settings: { activeRuntimeEnvironmentId: 'env-1' } }) +})) + +vi.mock('sonner', () => ({ + toast: Object.assign(vi.fn(), { message: vi.fn(), success: vi.fn(), error: vi.fn() }) +})) + +const RUNTIME_REPO: Repo = { + id: 'repo-1', + path: '/srv/repo-1', + displayName: 'repo-1', + badgeColor: '#000000', + addedAt: 0, + kind: 'git', + executionHostId: 'runtime:env-1', + forkSyncMode: 'ask', + upstream: { owner: 'up', repo: 'r' } +} + +describe('RepositoryForkSyncSection', () => { + afterEach(() => { + cleanup() + syncRuntimeGitForkDefaultBranch.mockReset() + }) + + it('syncs a runtime-hosted repo by its main worktree id, not the bare repo id', () => { + // Why: the runtime rejects `id:` with worktree_id_requires_full_path (#16447). + syncRuntimeGitForkDefaultBranch.mockResolvedValue({ status: 'up-to-date', behind: 0 }) + render( + + ) + + fireEvent.click(screen.getByRole('button', { name: /sync now/i })) + + expect(syncRuntimeGitForkDefaultBranch).toHaveBeenCalledWith( + expect.objectContaining({ worktreeId: 'repo-1::/srv/repo-1', worktreePath: '/srv/repo-1' }), + { owner: 'up', repo: 'r' } + ) + }) +}) diff --git a/src/renderer/src/components/settings/RepositoryForkSyncSection.tsx b/src/renderer/src/components/settings/RepositoryForkSyncSection.tsx index 3a399f16e38..a541a82dd24 100644 --- a/src/renderer/src/components/settings/RepositoryForkSyncSection.tsx +++ b/src/renderer/src/components/settings/RepositoryForkSyncSection.tsx @@ -3,6 +3,7 @@ import { RefreshCw } from 'lucide-react' import { toast } from 'sonner' import type { ForkSyncMode, GitForkSyncResult } from '../../../../shared/git-fork-sync' import type { Repo } from '../../../../shared/repo-types' +import { getRepoMainWorktreeId } from '../../../../shared/worktree/id' import { Button } from '../ui/button' import { SearchableSetting } from './SearchableSetting' import { SettingsSegmentedControl } from './SettingsFormControls' @@ -128,7 +129,7 @@ export function RepositoryForkSyncSection({ const result = await syncRuntimeGitForkDefaultBranch( { settings: getRepoOwnerRoutedSettings(settings, repo), - worktreeId: repo.id, + worktreeId: getRepoMainWorktreeId(repo), worktreePath: repo.path, connectionId: repo.connectionId ?? undefined }, diff --git a/src/renderer/src/store/repos/safe-auto-fork-sync.test.ts b/src/renderer/src/store/repos/safe-auto-fork-sync.test.ts new file mode 100644 index 00000000000..ec30e09944f --- /dev/null +++ b/src/renderer/src/store/repos/safe-auto-fork-sync.test.ts @@ -0,0 +1,98 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { AppState } from '../types' +import type { Repo } from '../../../../shared/repo-types' +import { + createCompatibleRuntimeStatusResponseIfNeeded, + type RuntimeEnvironmentCallRequest +} from '../../runtime/runtime-compatibility-test-fixture' +import { clearRuntimeCompatibilityCacheForTests } from '../../runtime/runtime-rpc-client' +import { safeAutoForkSyncAttempts, scheduleSafeAutoForkSync } from './safe-auto-fork-sync' + +const runtimeEnvironmentCall = vi.fn() +const runtimeEnvironmentTransportCall = vi.fn() +const gitSyncFork = vi.fn() + +const RUNTIME_REPO: Repo = { + id: 'repo-1', + path: '/srv/repo-1', + displayName: 'repo-1', + badgeColor: '#000000', + addedAt: 0, + kind: 'git', + executionHostId: 'runtime:env-1', + forkSyncMode: 'safe-auto', + upstream: { owner: 'up', repo: 'r' } +} + +function stateWith(repo: Repo): AppState { + return { + repos: [repo], + settings: { activeRuntimeEnvironmentId: 'env-1' } + } as unknown as AppState +} + +async function flushScheduledSyncs(): Promise { + await Promise.all([...safeAutoForkSyncAttempts.values()].map((attempt) => attempt.promise)) +} + +beforeEach(() => { + clearRuntimeCompatibilityCacheForTests() + safeAutoForkSyncAttempts.clear() + runtimeEnvironmentCall.mockReset() + runtimeEnvironmentTransportCall.mockReset() + gitSyncFork.mockReset() + gitSyncFork.mockResolvedValue({ status: 'up-to-date', behind: 0 }) + runtimeEnvironmentCall.mockResolvedValue({ + id: 'rpc-1', + ok: true, + result: { status: 'up-to-date', behind: 0 }, + _meta: { runtimeId: 'remote-runtime' } + }) + runtimeEnvironmentTransportCall.mockImplementation((args: RuntimeEnvironmentCallRequest) => { + return createCompatibleRuntimeStatusResponseIfNeeded(args) ?? runtimeEnvironmentCall(args) + }) + vi.stubGlobal('window', { + api: { + git: { syncFork: gitSyncFork }, + runtimeEnvironments: { call: runtimeEnvironmentTransportCall } + } + }) +}) + +describe('scheduleSafeAutoForkSync', () => { + it('addresses a runtime-hosted repo by its main worktree id, not the bare repo id', async () => { + // Why: the runtime rejects `id:` with worktree_id_requires_full_path (#16447). + scheduleSafeAutoForkSync(() => stateWith(RUNTIME_REPO), [RUNTIME_REPO]) + await flushScheduledSyncs() + + expect(runtimeEnvironmentCall).toHaveBeenCalledWith( + expect.objectContaining({ + method: 'git.forkSync', + params: expect.objectContaining({ worktree: 'id:repo-1::/srv/repo-1' }) + }) + ) + }) + + it('still runs local repos against the repo path over local git IPC', async () => { + const localRepo: Repo = { + ...RUNTIME_REPO, + id: 'repo-2', + path: '/home/me/repo-2', + executionHostId: 'local' + } + + scheduleSafeAutoForkSync( + () => + ({ ...stateWith(localRepo), settings: { activeRuntimeEnvironmentId: null } }) as AppState, + [localRepo] + ) + await flushScheduledSyncs() + + expect(gitSyncFork).toHaveBeenCalledWith({ + worktreePath: '/home/me/repo-2', + connectionId: undefined, + expectedUpstream: { owner: 'up', repo: 'r' } + }) + expect(runtimeEnvironmentCall).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/store/repos/safe-auto-fork-sync.ts b/src/renderer/src/store/repos/safe-auto-fork-sync.ts index d2167a9296a..3db51383ee5 100644 --- a/src/renderer/src/store/repos/safe-auto-fork-sync.ts +++ b/src/renderer/src/store/repos/safe-auto-fork-sync.ts @@ -2,6 +2,7 @@ import type { AppState } from '../types' import type { Repo } from '../../../../shared/repo-types' import { syncRuntimeGitForkDefaultBranch } from '../../runtime/runtime-git-client' import { getRepoExecutionHostId } from '../../../../shared/execution-host' +import { getRepoMainWorktreeId } from '../../../../shared/worktree/id' import { settingsForRepoOwner } from './owner-routing' export const SAFE_AUTO_FORK_SYNC_COOLDOWN_MS = 10 * 60 * 1000 @@ -32,7 +33,7 @@ export function scheduleSafeAutoForkSync(get: () => AppState, repos: readonly Re const promise = syncRuntimeGitForkDefaultBranch( { settings: settingsForRepoOwner(get(), repo.id), - worktreeId: repo.id, + worktreeId: getRepoMainWorktreeId(repo), worktreePath: repo.path, connectionId: repo.connectionId ?? undefined }, diff --git a/src/shared/worktree/id.test.ts b/src/shared/worktree/id.test.ts index 726e53abe5a..156c78e9959 100644 --- a/src/shared/worktree/id.test.ts +++ b/src/shared/worktree/id.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest' import { WORKTREE_ID_SEPARATOR, getRepoIdFromWorktreeId, + getRepoMainWorktreeId, getWorktreePathBasenameFromId, splitWorktreeId, splitWorktreeIdForFilesystem, @@ -14,6 +15,19 @@ describe('WORKTREE_ID_SEPARATOR', () => { }) }) +describe('getRepoMainWorktreeId', () => { + it('round-trips through the id parsers on posix and Windows paths', () => { + for (const repo of [ + { id: 'repo-123', path: '/abs/path' }, + { id: 'repo-123', path: 'C:\\Users\\me\\repo' } + ]) { + const worktreeId = getRepoMainWorktreeId(repo) + expect(worktreeId).toBe(`${repo.id}${WORKTREE_ID_SEPARATOR}${repo.path}`) + expect(splitWorktreeId(worktreeId)).toEqual({ repoId: repo.id, worktreePath: repo.path }) + } + }) +}) + describe('getRepoIdFromWorktreeId', () => { it('returns the repo id for a canonical worktree id', () => { expect(getRepoIdFromWorktreeId('repo-123::/abs/path')).toBe('repo-123') diff --git a/src/shared/worktree/id.ts b/src/shared/worktree/id.ts index 478ac6bfa76..82220328eb3 100644 --- a/src/shared/worktree/id.ts +++ b/src/shared/worktree/id.ts @@ -1,5 +1,6 @@ import { normalizeRuntimePathForComparison } from '../cross-platform-path' import { WORKTREE_ID_SEPARATOR } from '../pty-session-id-format' +import type { Repo } from '../repo-types' export { WORKTREE_ID_SEPARATOR } from '../pty-session-id-format' @@ -13,6 +14,14 @@ const FOLDER_WORKSPACE_INSTANCE_SUFFIX = new RegExp( `${FOLDER_WORKSPACE_INSTANCE_SEPARATOR.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}[0-9a-f-]{36}$` ) +/** + * Worktree id of the repo's own checkout. A bare repo id is never a valid worktree id — + * runtimes reject it with `worktree_id_requires_full_path` (#16447). + */ +export function getRepoMainWorktreeId(repo: Pick): string { + return `${repo.id}${WORKTREE_ID_SEPARATOR}${repo.path}` +} + export function getRepoIdFromWorktreeId(worktreeId: string): string { const separatorIdx = worktreeId.indexOf(WORKTREE_ID_SEPARATOR) return separatorIdx === -1 ? worktreeId : worktreeId.slice(0, separatorIdx) From f96dfd58103cd822a7d399a377421591094dc8d5 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Thu, 27 Aug 2026 18:18:50 -0700 Subject: [PATCH 07/10] Extract CDP auxiliary commands (#16781) --- src/main/browser/cdp-auxiliary-commands.ts | 213 ++++++++++++++++ src/main/browser/cdp-bridge.ts | 268 +++++---------------- 2 files changed, 271 insertions(+), 210 deletions(-) create mode 100644 src/main/browser/cdp-auxiliary-commands.ts diff --git a/src/main/browser/cdp-auxiliary-commands.ts b/src/main/browser/cdp-auxiliary-commands.ts new file mode 100644 index 00000000000..88a02d3f43c --- /dev/null +++ b/src/main/browser/cdp-auxiliary-commands.ts @@ -0,0 +1,213 @@ +import type { WebContents } from 'electron' +import type { + BrowserCaptureStartResult, + BrowserCaptureStopResult, + BrowserConsoleEntry, + BrowserConsoleResult, + BrowserCookie, + BrowserCookieDeleteResult, + BrowserCookieGetResult, + BrowserCookieSetResult, + BrowserGeolocationResult, + BrowserInterceptDisableResult, + BrowserInterceptEnableResult, + BrowserInterceptedRequest, + BrowserNetworkEntry, + BrowserNetworkLogResult, + BrowserViewportResult +} from '../../shared/runtime-types' +import type { CdpCommandSender, SnapshotResult } from './snapshot-engine' + +export type CdpTabState = { + navigationId: string | null + snapshotResult: SnapshotResult | null + debuggerAttached: boolean + debuggerDetachListener: (() => void) | null + debuggerMessageListener: ((_event: unknown, method: string, params: unknown) => void) | null + iframeSessions: Map + capturing: boolean + consoleLog: BrowserConsoleEntry[] + networkLog: BrowserNetworkEntry[] + intercepting: boolean + interceptPatterns: string[] + pausedRequests: Map + networkRequestMap: Map +} + +type ActiveCdpContext = { + guest: WebContents + sender: CdpCommandSender + state: CdpTabState +} + +export type CdpAuxiliaryCommandHost = { + run: (operation: (context: ActiveCdpContext) => Promise) => Promise + // Why: captureStop must not attach the debugger — stopping a capture on a tab whose + // debugger detached (DevTools opened) would otherwise throw before clearing state. + runOnState: ( + operation: (context: Pick) => Promise + ) => Promise + current: () => Pick +} + +export class CdpAuxiliaryCommands { + constructor(private readonly host: CdpAuxiliaryCommandHost) {} + + cookieGet(url?: string): Promise { + return this.host.run(async ({ sender }) => { + const params: Record = {} + if (url) { + params.urls = [url] + } + const { cookies } = (await sender('Network.getCookies', params)) as { + cookies: BrowserCookie[] + } + return { cookies } + }) + } + + cookieSet(cookie: { + name: string + value: string + domain?: string + path?: string + secure?: boolean + httpOnly?: boolean + sameSite?: string + expires?: number + }): Promise { + return this.host.run(async ({ sender }) => { + let domain = cookie.domain + if (!domain) { + const { result } = (await sender('Runtime.evaluate', { + expression: 'location.hostname', + returnByValue: true + })) as { result: { value: string } } + domain = result.value + } + const params: Record = { + name: cookie.name, + value: cookie.value, + domain, + path: cookie.path ?? '/', + secure: cookie.secure ?? false, + httpOnly: cookie.httpOnly ?? false, + sameSite: cookie.sameSite ?? 'Lax' + } + if (cookie.expires !== undefined) { + params.expires = cookie.expires + } + const { success } = (await sender('Network.setCookie', params)) as { success: boolean } + return { success } + }) + } + + cookieDelete(name: string, domain?: string, url?: string): Promise { + return this.host.run(async ({ sender }) => { + const params: Record = { name } + if (domain) { + params.domain = domain + } + if (url) { + params.url = url + } + if (!domain && !url) { + const { result } = (await sender('Runtime.evaluate', { + expression: 'location.href', + returnByValue: true + })) as { result: { value: string } } + params.url = result.value + } + await sender('Network.deleteCookies', params) + return { deleted: true } + }) + } + + setViewport( + width: number, + height: number, + deviceScaleFactor = 1, + mobile = false + ): Promise { + return this.host.run(async ({ sender }) => { + await sender('Emulation.setDeviceMetricsOverride', { + width, + height, + deviceScaleFactor, + mobile + }) + await Promise.resolve(sender('Emulation.setVisibleSize', { width, height })).catch(() => {}) + return { width, height, deviceScaleFactor, mobile } + }) + } + + setGeolocation( + latitude: number, + longitude: number, + accuracy = 1 + ): Promise { + return this.host.run(async ({ sender }) => { + await sender('Emulation.setGeolocationOverride', { latitude, longitude, accuracy }) + return { latitude, longitude, accuracy } + }) + } + + interceptEnable(patterns: string[] = ['*']): Promise { + return this.host.run(async ({ sender, state }) => { + await sender('Fetch.enable', { patterns: patterns.map((urlPattern) => ({ urlPattern })) }) + state.intercepting = true + state.interceptPatterns = patterns + return { enabled: true, patterns } + }) + } + + interceptDisable(): Promise { + return this.host.run(async ({ sender, state }) => { + await sender('Fetch.disable') + state.intercepting = false + state.interceptPatterns = [] + state.pausedRequests.clear() + return { disabled: true } + }) + } + + interceptList(): { requests: BrowserInterceptedRequest[] } { + const { state } = this.host.current() + return { requests: [...state.pausedRequests.values()] } + } + + captureStart(): Promise { + return this.host.run(async ({ sender, state }) => { + await sender('Runtime.enable') + state.capturing = true + state.consoleLog = [] + state.networkLog = [] + state.networkRequestMap.clear() + return { capturing: true } + }) + } + + captureStop(): Promise { + return this.host.runOnState(async ({ state }) => { + state.capturing = false + state.networkRequestMap.clear() + return { stopped: true } + }) + } + + consoleLog(limit = 100): BrowserConsoleResult { + const { state } = this.host.current() + return { + entries: state.consoleLog.slice(-limit), + truncated: state.consoleLog.length > limit + } + } + + networkLog(limit = 100): BrowserNetworkLogResult { + const { state } = this.host.current() + return { + entries: state.networkLog.slice(-limit), + truncated: state.networkLog.length > limit + } + } +} diff --git a/src/main/browser/cdp-bridge.ts b/src/main/browser/cdp-bridge.ts index fc5991924a9..73dec07f2d2 100644 --- a/src/main/browser/cdp-bridge.ts +++ b/src/main/browser/cdp-bridge.ts @@ -6,9 +6,7 @@ import type { BrowserCheckResult, BrowserClearResult, BrowserClickResult, - BrowserConsoleEntry, BrowserConsoleResult, - BrowserCookie, BrowserCookieDeleteResult, BrowserCookieGetResult, BrowserCookieSetResult, @@ -39,16 +37,12 @@ import type { BrowserViewportResult, BrowserWaitResult } from '../../shared/runtime-types' -import { - buildSnapshot, - type CdpCommandSender, - type RefEntry, - type SnapshotResult -} from './snapshot-engine' +import { buildSnapshot, type CdpCommandSender, type RefEntry } from './snapshot-engine' import { insertTextThroughCdp } from './browser-text-insertion' import type { BrowserManager } from './browser-manager' import { ANTI_DETECTION_SCRIPT } from './anti-detection' import { BrowserError } from './browser-error' +import { CdpAuxiliaryCommands, type CdpTabState } from './cdp-auxiliary-commands' const CAPTURE_LOG_LIMIT = 1000 @@ -56,25 +50,6 @@ const CAPTURE_LOG_LIMIT = 1000 // pulling Chromium in. Existing importers of this path keep working. export { BrowserError } from './browser-error' -type TabState = { - navigationId: string | null - snapshotResult: SnapshotResult | null - debuggerAttached: boolean - debuggerDetachListener: (() => void) | null - debuggerMessageListener: ((_event: unknown, method: string, params: unknown) => void) | null - iframeSessions: Map - // Why: capture state is per-tab so one tab's console/network events don't pollute another's buffer. - capturing: boolean - consoleLog: BrowserConsoleEntry[] - networkLog: BrowserNetworkEntry[] - // Why: interception state lets the agent selectively continue or block individual requests. - intercepting: boolean - interceptPatterns: string[] - pausedRequests: Map - // Why: maps CDP requestId → networkLog entry so loadingFinished attributes size to the right overlapping response. - networkRequestMap: Map -} - type QueuedCommand = { execute: () => Promise resolve: (value: unknown) => void @@ -83,13 +58,45 @@ type QueuedCommand = { export class CdpBridge { private activeWebContentsId: number | null = null - private readonly tabState = new Map() + private readonly tabState = new Map() private readonly commandQueues = new Map() private readonly processingQueues = new Set() private readonly browserManager: BrowserManager + private readonly auxiliaryCommands: CdpAuxiliaryCommands constructor(browserManager: BrowserManager) { this.browserManager = browserManager + this.auxiliaryCommands = new CdpAuxiliaryCommands({ + run: ( + operation: (context: { + guest: Electron.WebContents + sender: CdpCommandSender + state: CdpTabState + }) => Promise + ): Promise => + this.enqueueCommand(async () => { + const guest = this.getActiveGuest() + const sender = this.makeCdpSender(guest) + await this.ensureDebuggerAttached(guest) + const state = this.getOrCreateTabState(this.resolveTabId(guest.id)) + return operation({ guest, sender, state }) + }), + runOnState: ( + operation: (context: { guest: Electron.WebContents; state: CdpTabState }) => Promise + ): Promise => + this.enqueueCommand(async () => { + const guest = this.getActiveGuest() + const state = this.getOrCreateTabState(this.resolveTabId(guest.id)) + return operation({ guest, state }) + }), + current: () => { + const guest = this.getActiveGuest() + return { + guest, + state: this.getOrCreateTabState(this.resolveTabId(guest.id)) + } + } + }) } setActiveTab(webContentsId: number): void { @@ -636,25 +643,11 @@ export class CdpBridge { // ── Cookie management ── - async cookieGet(url?: string): Promise { - return this.enqueueCommand(async () => { - const guest = this.getActiveGuest() - const sender = this.makeCdpSender(guest) - await this.ensureDebuggerAttached(guest) - - const params: Record = {} - if (url) { - params.urls = [url] - } - const { cookies } = (await sender('Network.getCookies', params)) as { - cookies: BrowserCookie[] - } - - return { cookies } - }) + cookieGet(url?: string): Promise { + return this.auxiliaryCommands.cookieGet(url) } - async cookieSet(cookie: { + cookieSet(cookie: { name: string value: string domain?: string @@ -664,211 +657,66 @@ export class CdpBridge { sameSite?: string expires?: number }): Promise { - return this.enqueueCommand(async () => { - const guest = this.getActiveGuest() - const sender = this.makeCdpSender(guest) - await this.ensureDebuggerAttached(guest) - - // Why: Network.setCookie needs a domain or url to scope the cookie; infer the domain from the current page when omitted. - let domain = cookie.domain - if (!domain) { - const { result: urlResult } = (await sender('Runtime.evaluate', { - expression: 'location.hostname', - returnByValue: true - })) as { result: { value: string } } - domain = urlResult.value - } - - const params: Record = { - name: cookie.name, - value: cookie.value, - domain, - path: cookie.path ?? '/', - secure: cookie.secure ?? false, - httpOnly: cookie.httpOnly ?? false, - sameSite: cookie.sameSite ?? 'Lax' - } - if (cookie.expires !== undefined) { - params.expires = cookie.expires - } - - const { success } = (await sender('Network.setCookie', params)) as { success: boolean } - return { success } - }) + return this.auxiliaryCommands.cookieSet(cookie) } - async cookieDelete( - name: string, - domain?: string, - url?: string - ): Promise { - return this.enqueueCommand(async () => { - const guest = this.getActiveGuest() - const sender = this.makeCdpSender(guest) - await this.ensureDebuggerAttached(guest) - - const params: Record = { name } - if (domain) { - params.domain = domain - } - if (url) { - params.url = url - } - // Why: Network.deleteCookies needs a domain or url; infer from the current page if neither was given. - if (!domain && !url) { - const { result: urlResult } = (await sender('Runtime.evaluate', { - expression: 'location.href', - returnByValue: true - })) as { result: { value: string } } - params.url = urlResult.value - } - - await sender('Network.deleteCookies', params) - return { deleted: true } - }) + cookieDelete(name: string, domain?: string, url?: string): Promise { + return this.auxiliaryCommands.cookieDelete(name, domain, url) } // ── Viewport emulation ── - async setViewport( + setViewport( width: number, height: number, deviceScaleFactor = 1, mobile = false ): Promise { - return this.enqueueCommand(async () => { - const guest = this.getActiveGuest() - const sender = this.makeCdpSender(guest) - await this.ensureDebuggerAttached(guest) - - await sender('Emulation.setDeviceMetricsOverride', { - width, - height, - deviceScaleFactor, - mobile - }) - // Why: metrics-only resize can leave the compositor surface at the old size, cropping remote screencast clients. - await Promise.resolve(sender('Emulation.setVisibleSize', { width, height })).catch(() => {}) - - return { width, height, deviceScaleFactor, mobile } - }) + return this.auxiliaryCommands.setViewport(width, height, deviceScaleFactor, mobile) } // ── Geolocation ── - async setGeolocation( + setGeolocation( latitude: number, longitude: number, accuracy = 1 ): Promise { - return this.enqueueCommand(async () => { - const guest = this.getActiveGuest() - const sender = this.makeCdpSender(guest) - await this.ensureDebuggerAttached(guest) - - await sender('Emulation.setGeolocationOverride', { latitude, longitude, accuracy }) - return { latitude, longitude, accuracy } - }) + return this.auxiliaryCommands.setGeolocation(latitude, longitude, accuracy) } // ── Request interception ── - async interceptEnable(patterns: string[] = ['*']): Promise { - return this.enqueueCommand(async () => { - const guest = this.getActiveGuest() - const sender = this.makeCdpSender(guest) - await this.ensureDebuggerAttached(guest) - - const tabId = this.resolveTabId(guest.id) - const state = this.getOrCreateTabState(tabId) - - const requestPatterns = patterns.map((p) => ({ urlPattern: p })) - await sender('Fetch.enable', { patterns: requestPatterns }) - - state.intercepting = true - state.interceptPatterns = patterns - - return { enabled: true, patterns } - }) + interceptEnable(patterns: string[] = ['*']): Promise { + return this.auxiliaryCommands.interceptEnable(patterns) } - async interceptDisable(): Promise { - return this.enqueueCommand(async () => { - const guest = this.getActiveGuest() - const sender = this.makeCdpSender(guest) - await this.ensureDebuggerAttached(guest) - - const tabId = this.resolveTabId(guest.id) - const state = this.getOrCreateTabState(tabId) - - await sender('Fetch.disable') - state.intercepting = false - state.interceptPatterns = [] - state.pausedRequests.clear() - - return { disabled: true } - }) + interceptDisable(): Promise { + return this.auxiliaryCommands.interceptDisable() } interceptList(): { requests: BrowserInterceptedRequest[] } { - const guest = this.getActiveGuest() - const tabId = this.resolveTabId(guest.id) - const state = this.getOrCreateTabState(tabId) - return { requests: [...state.pausedRequests.values()] } + return this.auxiliaryCommands.interceptList() } // TODO: Add interceptContinue/interceptBlock once agent-browser supports per-request decisions (CLI is URL-pattern-only). // ── Console/network capture ── - async captureStart(): Promise { - return this.enqueueCommand(async () => { - const guest = this.getActiveGuest() - const sender = this.makeCdpSender(guest) - await this.ensureDebuggerAttached(guest) - - const tabId = this.resolveTabId(guest.id) - const state = this.getOrCreateTabState(tabId) - - await sender('Runtime.enable') - state.capturing = true - state.consoleLog = [] - state.networkLog = [] - state.networkRequestMap.clear() - - return { capturing: true } - }) + captureStart(): Promise { + return this.auxiliaryCommands.captureStart() } - async captureStop(): Promise { - return this.enqueueCommand(async () => { - const guest = this.getActiveGuest() - const tabId = this.resolveTabId(guest.id) - const state = this.getOrCreateTabState(tabId) - - state.capturing = false - state.networkRequestMap.clear() - - return { stopped: true } - }) + captureStop(): Promise { + return this.auxiliaryCommands.captureStop() } consoleLog(limit = 100): BrowserConsoleResult { - const guest = this.getActiveGuest() - const tabId = this.resolveTabId(guest.id) - const state = this.getOrCreateTabState(tabId) - - const entries = state.consoleLog.slice(-limit) - return { entries, truncated: state.consoleLog.length > limit } + return this.auxiliaryCommands.consoleLog(limit) } networkLog(limit = 100): BrowserNetworkLogResult { - const guest = this.getActiveGuest() - const tabId = this.resolveTabId(guest.id) - const state = this.getOrCreateTabState(tabId) - - const entries = state.networkLog.slice(-limit) - return { entries, truncated: state.networkLog.length > limit } + return this.auxiliaryCommands.networkLog(limit) } async back(): Promise<{ url: string; title: string }> { @@ -1077,7 +925,7 @@ export class CdpBridge { return null } - private getOrCreateTabState(tabId: string): TabState { + private getOrCreateTabState(tabId: string): CdpTabState { let state = this.tabState.get(tabId) if (!state) { state = { @@ -1100,7 +948,7 @@ export class CdpBridge { return state } - private removeDebuggerListeners(guest: Electron.WebContents, state: TabState): void { + private removeDebuggerListeners(guest: Electron.WebContents, state: CdpTabState): void { const detachListener = state.debuggerDetachListener const messageListener = state.debuggerMessageListener state.debuggerDetachListener = null From ebe6b559eff3385db7e456a0c76c02ea1ecca3d2 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Thu, 27 Aug 2026 18:34:03 -0700 Subject: [PATCH 08/10] Prime changed native caches before test fanout (#16918) --- .../install-node-dependencies/action.yml | 5 +-- .github/workflows/pr.yml | 35 ++++++++++++++++--- config/scripts/pr-code-change-scope.mjs | 23 +++++++++++- config/scripts/pr-code-change-scope.test.mjs | 31 ++++++++++++++-- .../scripts/pr-workflow-parallelism.test.mjs | 12 +++++++ ...ws-pty-native-capability-workflow.test.mjs | 5 +++ 6 files changed, 101 insertions(+), 10 deletions(-) diff --git a/.github/actions/install-node-dependencies/action.yml b/.github/actions/install-node-dependencies/action.yml index e6d6f8f15e3..b67ce13cc1b 100644 --- a/.github/actions/install-node-dependencies/action.yml +++ b/.github/actions/install-node-dependencies/action.yml @@ -30,6 +30,7 @@ runs: - name: Setup pnpm uses: pnpm/action-setup@v6 with: + version: 10.24.0 run_install: false - name: Setup Node.js @@ -117,7 +118,7 @@ runs: node_modules/.pnpm/node-pty@*/node_modules/node-pty/build node_modules/.pnpm/windows-native-registry@*/node_modules/windows-native-registry/build node_modules/.pnpm/@vscode+windows-process-tree@*/node_modules/@vscode/windows-process-tree/build - key: native-modules-${{ runner.os }}-${{ steps.native-cache-scope.outputs.scope }}-${{ runner.arch }}-${{ inputs.native-runtime }}-node${{ steps.default-node.outputs.node-version }}${{ steps.requested-node.outputs.node-version }}-${{ hashFiles('pnpm-lock.yaml', 'config/patches/node-pty@1.1.0.patch', 'config/patches/@vscode__windows-process-tree@0.8.0.patch') }} + key: native-modules-${{ runner.os }}-${{ steps.native-cache-scope.outputs.scope }}-${{ runner.arch }}-${{ inputs.native-runtime }}-node${{ steps.default-node.outputs.node-version }}${{ steps.requested-node.outputs.node-version }}-${{ hashFiles('pnpm-lock.yaml', '.github/actions/install-node-dependencies/action.yml', 'config/scripts/ensure-native-runtime.mjs', 'config/scripts/rebuild-native-deps.mjs', 'config/patches/node-pty@1.1.0.patch', 'config/patches/@vscode__windows-process-tree@0.8.0.patch') }} - name: Restore compiled native modules without saving if: inputs.native-runtime != 'none' && inputs.persist-native-cache == 'false' @@ -127,7 +128,7 @@ runs: node_modules/.pnpm/node-pty@*/node_modules/node-pty/build node_modules/.pnpm/windows-native-registry@*/node_modules/windows-native-registry/build node_modules/.pnpm/@vscode+windows-process-tree@*/node_modules/@vscode/windows-process-tree/build - key: native-modules-${{ runner.os }}-${{ steps.native-cache-scope.outputs.scope }}-${{ runner.arch }}-${{ inputs.native-runtime }}-node${{ steps.default-node.outputs.node-version }}${{ steps.requested-node.outputs.node-version }}-${{ hashFiles('pnpm-lock.yaml', 'config/patches/node-pty@1.1.0.patch', 'config/patches/@vscode__windows-process-tree@0.8.0.patch') }} + key: native-modules-${{ runner.os }}-${{ steps.native-cache-scope.outputs.scope }}-${{ runner.arch }}-${{ inputs.native-runtime }}-node${{ steps.default-node.outputs.node-version }}${{ steps.requested-node.outputs.node-version }}-${{ hashFiles('pnpm-lock.yaml', '.github/actions/install-node-dependencies/action.yml', 'config/scripts/ensure-native-runtime.mjs', 'config/scripts/rebuild-native-deps.mjs', 'config/patches/node-pty@1.1.0.patch', 'config/patches/@vscode__windows-process-tree@0.8.0.patch') }} - name: Prepare native runtime if: inputs.native-runtime != 'none' diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 5eadeb9be81..9fbe35a173f 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -27,6 +27,7 @@ jobs: runs-on: ubuntu-latest outputs: should_run: ${{ steps.filter.outputs.should_run }} + native_cache_changed: ${{ steps.filter.outputs.native_cache_changed }} static_analysis: ${{ steps.filter.outputs.static_analysis }} typecheck: ${{ steps.filter.outputs.typecheck }} git_compatibility: ${{ steps.filter.outputs.git_compatibility }} @@ -442,10 +443,36 @@ jobs: src/shared/startup-shell-portability.live-shell.test.ts \ src/shared/posix-command-path-lookup.test.ts + # Cache-key input changes would otherwise make every shard compile the same + # native addon concurrently. Prime each Node ABI once before the matrix fans out. + test_native_cache: + name: prepare test native cache node ${{ matrix.node }} + needs: [code_paths] + if: needs.code_paths.outputs.native_cache_changed == 'true' + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + node: ['24', '26'] + + steps: + - name: Checkout + uses: actions/checkout@v6 + with: + persist-credentials: false + + - uses: ./.github/actions/install-node-dependencies + with: + native-runtime: node + node-version: ${{ matrix.node }} + test: name: tests node ${{ matrix.node }} ${{ matrix.shard }}/${{ matrix.shard_total }} - needs: [code_paths] - if: needs.code_paths.outputs.test == 'true' + needs: [code_paths, test_native_cache] + if: >- + always() && + needs.code_paths.outputs.test == 'true' && + (needs.test_native_cache.result == 'success' || needs.test_native_cache.result == 'skipped') runs-on: ubuntu-latest strategy: fail-fast: false @@ -695,7 +722,7 @@ jobs: node_modules/.pnpm/node-pty@*/node_modules/node-pty/build node_modules/.pnpm/windows-native-registry@*/node_modules/windows-native-registry/build node_modules/.pnpm/@vscode+windows-process-tree@*/node_modules/@vscode/windows-process-tree/build - key: native-modules-${{ runner.os }}-${{ steps.deps.outputs.native-cache-scope }}-${{ runner.arch }}-node-node${{ steps.deps.outputs.node-version }}-${{ hashFiles('pnpm-lock.yaml', 'config/patches/node-pty@1.1.0.patch', 'config/patches/@vscode__windows-process-tree@0.8.0.patch') }} + key: native-modules-${{ runner.os }}-${{ steps.deps.outputs.native-cache-scope }}-${{ runner.arch }}-node-node${{ steps.deps.outputs.node-version }}-${{ hashFiles('pnpm-lock.yaml', '.github/actions/install-node-dependencies/action.yml', 'config/scripts/ensure-native-runtime.mjs', 'config/scripts/rebuild-native-deps.mjs', 'config/patches/node-pty@1.1.0.patch', 'config/patches/@vscode__windows-process-tree@0.8.0.patch') }} - name: Test Windows-specific boundaries run: >- @@ -748,7 +775,7 @@ jobs: node_modules/.pnpm/node-pty@*/node_modules/node-pty/build node_modules/.pnpm/windows-native-registry@*/node_modules/windows-native-registry/build node_modules/.pnpm/@vscode+windows-process-tree@*/node_modules/@vscode/windows-process-tree/build - key: native-modules-${{ runner.os }}-${{ steps.deps.outputs.native-cache-scope }}-${{ runner.arch }}-electron-node${{ steps.deps.outputs.node-version }}-${{ hashFiles('pnpm-lock.yaml', 'config/patches/node-pty@1.1.0.patch', 'config/patches/@vscode__windows-process-tree@0.8.0.patch') }} + key: native-modules-${{ runner.os }}-${{ steps.deps.outputs.native-cache-scope }}-${{ runner.arch }}-electron-node${{ steps.deps.outputs.node-version }}-${{ hashFiles('pnpm-lock.yaml', '.github/actions/install-node-dependencies/action.yml', 'config/scripts/ensure-native-runtime.mjs', 'config/scripts/rebuild-native-deps.mjs', 'config/patches/node-pty@1.1.0.patch', 'config/patches/@vscode__windows-process-tree@0.8.0.patch') }} - name: Prepare Electron native runtime run: node config/scripts/ensure-native-runtime.mjs --runtime=electron diff --git a/config/scripts/pr-code-change-scope.mjs b/config/scripts/pr-code-change-scope.mjs index 8ccc864aabb..c51b1c18a5c 100644 --- a/config/scripts/pr-code-change-scope.mjs +++ b/config/scripts/pr-code-change-scope.mjs @@ -116,6 +116,19 @@ const NATIVE_RUNTIME_PREFIXES = [ 'config/patches/@vscode__windows-process-tree' ] +const NATIVE_CACHE_FILES = new Set([ + 'package.json', + 'pnpm-lock.yaml', + '.github/actions/install-node-dependencies/action.yml', + 'config/scripts/ensure-native-runtime.mjs', + 'config/scripts/rebuild-native-deps.mjs' +]) + +const NATIVE_CACHE_PREFIXES = [ + 'config/patches/node-pty@', + 'config/patches/@vscode__windows-process-tree' +] + const SHARED_PACKAGE_PREFIXES = [ 'electron.vite.config.ts', 'config/electron-builder', @@ -222,7 +235,11 @@ export function classifyPrJobs(changedFiles) { shouldRun && (forceAll || ALWAYS_ON_CODE_JOBS.has(job) || jobDetector(job)(changedFiles)) ]) ) - return { should_run: shouldRun, ...jobs } + return { + should_run: shouldRun, + native_cache_changed: shouldRun && (emptyDiff || changedFiles.some(isNativeCacheInputPath)), + ...jobs + } } function jobDetector(job) { @@ -274,6 +291,10 @@ function isDesktopIrrelevantPath(file) { return matchesPrefix(file, DESKTOP_IRRELEVANT_PREFIXES) } +function isNativeCacheInputPath(file) { + return NATIVE_CACHE_FILES.has(file) || matchesPrefix(file, NATIVE_CACHE_PREFIXES) +} + function isGlobalForcePath(file) { return GLOBAL_FORCE_FILES.has(file) || matchesPrefix(file, GLOBAL_FORCE_PREFIXES) } diff --git a/config/scripts/pr-code-change-scope.test.mjs b/config/scripts/pr-code-change-scope.test.mjs index 0ce697d1511..223929fc213 100644 --- a/config/scripts/pr-code-change-scope.test.mjs +++ b/config/scripts/pr-code-change-scope.test.mjs @@ -207,6 +207,22 @@ describe('per-job path classification', () => { expect(classifyPrJobs(['pnpm-lock.yaml']).git_compatibility).toBe(true) }) + it('primes native caches only when their immutable inputs change', () => { + expect(classifyPrJobs([]).native_cache_changed).toBe(true) + expect(classifyPrJobs(['README.md']).native_cache_changed).toBe(false) + expect(classifyPrJobs(['src/main/index.ts']).native_cache_changed).toBe(false) + for (const file of [ + 'package.json', + 'pnpm-lock.yaml', + '.github/actions/install-node-dependencies/action.yml', + 'config/scripts/ensure-native-runtime.mjs', + 'config/scripts/rebuild-native-deps.mjs', + 'config/patches/node-pty@1.1.0.patch' + ]) { + expect(classifyPrJobs([file]).native_cache_changed, file).toBe(true) + } + }) + it('keeps unit-test-only diffs out of packaging', () => { expectClassification(['src/main/git/git-status.test.ts'], { git_compatibility: true @@ -238,7 +254,7 @@ describe('PR Checks skip wiring', () => { expect(classify.run).toContain('--merge-base "$BASE_SHA" "$HEAD_SHA"') expect(classify.run).toContain('node config/scripts/pr-code-change-scope.mjs') expect(classify.run).toContain('tee -a "$GITHUB_OUTPUT"') - for (const jobName of ['should_run', ...expensiveJobs]) { + for (const jobName of ['should_run', 'native_cache_changed', ...expensiveJobs]) { expect(prWorkflow.jobs.code_paths.outputs[jobName], jobName).toBe( `\${{ steps.filter.outputs.${jobName} }}` ) @@ -250,13 +266,22 @@ describe('PR Checks skip wiring', () => { expect(prWorkflow.jobs.root_directory_guard.needs).toBeUndefined() }) - it('gates each expensive job on its own classifier output', () => { - for (const jobName of expensiveJobs) { + it('gates each expensive job on its classifier and cache prerequisite', () => { + for (const jobName of expensiveJobs.filter((jobName) => jobName !== 'test')) { expect(prWorkflow.jobs[jobName].needs, jobName).toEqual(['code_paths']) expect(prWorkflow.jobs[jobName].if, jobName).toBe( `needs.code_paths.outputs.${jobName} == 'true'` ) } + expect(prWorkflow.jobs.test.needs).toEqual(['code_paths', 'test_native_cache']) + expect(prWorkflow.jobs.test.if).toContain("needs.code_paths.outputs.test == 'true'") + expect(prWorkflow.jobs.test.if).toContain("needs.test_native_cache.result == 'success'") + expect(prWorkflow.jobs.test.if).toContain("needs.test_native_cache.result == 'skipped'") + expect(prWorkflow.jobs.test_native_cache.needs).toEqual(['code_paths']) + expect(prWorkflow.jobs.test_native_cache.if).toBe( + "needs.code_paths.outputs.native_cache_changed == 'true'" + ) + expect(prWorkflow.jobs.test_native_cache.strategy.matrix.node).toEqual(['24', '26']) }) it('skips e2e detection on docs-only PRs without dropping the draft gate', () => { diff --git a/config/scripts/pr-workflow-parallelism.test.mjs b/config/scripts/pr-workflow-parallelism.test.mjs index a61244d42d3..88bf4165d63 100644 --- a/config/scripts/pr-workflow-parallelism.test.mjs +++ b/config/scripts/pr-workflow-parallelism.test.mjs @@ -63,6 +63,13 @@ describe('PR workflow parallelism', () => { for (const testFile of nativeShellContractFiles) { expect(testStep.run).toContain(`--exclude=${testFile}`) } + const primerInstall = workflow.jobs.test_native_cache.steps.find( + (step) => step.uses === './.github/actions/install-node-dependencies' + ) + expect(workflow.jobs.test_native_cache.strategy.matrix.node).toEqual(['24', '26']) + expect(primerInstall.with['native-runtime']).toBe('node') + expect(primerInstall.with['node-version']).toBe('${{ matrix.node }}') + expect(workflow.jobs.test.needs).toContain('test_native_cache') }) it('runs real-shell coverage once outside the general shards', () => { @@ -208,6 +215,8 @@ describe('PR workflow parallelism', () => { expect(pnpmIndex).toBeLessThan(nodeIndex) expect(pnpmIndex).toBeLessThan(requestedNodeIndex) + const packageManagerVersion = /^pnpm@([^+]+)/.exec(packageJson.packageManager)?.[1] + expect(steps[pnpmIndex].with.version).toBe(packageManagerVersion) expect(steps[nodeIndex].with.cache).toBe('pnpm') expect(steps[nodeIndex].if).toBe("inputs.node-version == ''") expect(steps[requestedNodeIndex].if).toBe("inputs.node-version != ''") @@ -316,6 +325,9 @@ describe('PR workflow parallelism', () => { expect(cacheStep.with.key).toContain( 'config/patches/@vscode__windows-process-tree@0.8.0.patch' ) + expect(cacheStep.with.key).toContain('.github/actions/install-node-dependencies/action.yml') + expect(cacheStep.with.key).toContain('config/scripts/ensure-native-runtime.mjs') + expect(cacheStep.with.key).toContain('config/scripts/rebuild-native-deps.mjs') expect(cacheStep.with.path).toContain('node-pty@*/node_modules/node-pty/build') expect(cacheStep.with.path).toContain('windows-native-registry@') expect(cacheStep.with.path).toContain('@vscode+windows-process-tree@') diff --git a/config/scripts/windows-pty-native-capability-workflow.test.mjs b/config/scripts/windows-pty-native-capability-workflow.test.mjs index 8d4b7d568fd..e1742a43bb0 100644 --- a/config/scripts/windows-pty-native-capability-workflow.test.mjs +++ b/config/scripts/windows-pty-native-capability-workflow.test.mjs @@ -50,6 +50,11 @@ describe('packaged Windows PTY native capability routing', () => { expect(nodeCacheSave.uses).toBe('actions/cache/save@v5') expect(nodeCacheSave.with.key).toContain('-node-node') expect(electronCache.with.key).toContain('-electron-node') + for (const cache of [nodeCacheSave, electronCache]) { + expect(cache.with.key).toContain('.github/actions/install-node-dependencies/action.yml') + expect(cache.with.key).toContain('config/scripts/ensure-native-runtime.mjs') + expect(cache.with.key).toContain('config/scripts/rebuild-native-deps.mjs') + } expect(ensureNativeRuntime).toContain("runPnpm(['exec', 'node-gyp', 'rebuild']") expect(ensureNativeRuntime).toContain("resolve(moduleDir, 'scripts', 'post-install.js')") expect(build.run).toBe('pnpm run build:release:parallel') From 3b4a15e609dd7dcb294795d0cc25751c16c187e8 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Thu, 27 Aug 2026 18:44:26 -0700 Subject: [PATCH 09/10] test(editor): add copy button tests for check run details (#16925) Add tests verifying that copy buttons render and correctly copy content to clipboard for the output, annotations, and jobs sections of the check run details panel. --- .../editor/CheckRunDetailsPanel.copy.test.tsx | 134 ++++++++++++++++++ 1 file changed, 134 insertions(+) create mode 100644 src/renderer/src/components/editor/CheckRunDetailsPanel.copy.test.tsx diff --git a/src/renderer/src/components/editor/CheckRunDetailsPanel.copy.test.tsx b/src/renderer/src/components/editor/CheckRunDetailsPanel.copy.test.tsx new file mode 100644 index 00000000000..23bc3d51c59 --- /dev/null +++ b/src/renderer/src/components/editor/CheckRunDetailsPanel.copy.test.tsx @@ -0,0 +1,134 @@ +// @vitest-environment happy-dom + +import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { TooltipProvider } from '@/components/ui/tooltip' +import type { PRCheckDetail, PRCheckRunDetails } from '../../../../shared/github/check-types' +import { CheckRunDetailsPanel } from './CheckRunDetailsPanel' + +const writeClipboardText = vi.fn<(text: string) => Promise>() + +vi.mock('@/i18n/i18n', () => ({ + translate: (_key: string, fallback: string) => fallback +})) + +vi.mock('@/components/sidebar/CommentMarkdown', () => ({ + default: ({ content }: { content: string }) => content +})) + +vi.mock('@/components/right-sidebar/source-control-fix-split-button', () => ({ + SourceControlFixSplitButton: () => null +})) + +vi.mock('./check-run-details-fix-with-ai', () => ({ + useCheckRunDetailsFixWithAI: () => ({ + canFixWithAI: false, + disabledReason: null, + isFixing: false, + fixPrompt: '', + repoId: null, + connectionId: null, + launchPlatform: null, + savedAgentId: null, + savedCommandInputTemplate: null, + savedAgentArgs: [], + saveLaunchActionDefault: vi.fn(), + openSourceControlAiSettings: vi.fn(), + fixWithAI: vi.fn() + }) +})) + +afterEach(cleanup) + +beforeEach(() => { + writeClipboardText.mockReset().mockResolvedValue(undefined) + Object.assign(window, { api: { ui: { writeClipboardText } } }) +}) + +const check: PRCheckDetail = { + name: 'package', + status: 'completed', + conclusion: 'failure', + url: null, + checkRunId: 42 +} + +const details: PRCheckRunDetails = { + name: 'package', + status: 'completed', + conclusion: 'failure', + url: null, + detailsUrl: null, + startedAt: null, + completedAt: null, + title: 'Build failed', + summary: 'One test failed.', + text: 'Run the suite locally.', + annotations: [ + { + path: 'src/main.ts', + startLine: 10, + endLine: 10, + annotationLevel: 'failure', + title: 'Type error', + message: 'Expected string', + rawDetails: 'Actual: number' + } + ], + jobs: [ + { + id: 1, + name: 'unit', + status: 'completed', + conclusion: 'failure', + startedAt: null, + completedAt: null, + url: null, + logTail: 'AssertionError: expected true', + steps: [ + { + name: 'test', + status: 'completed', + conclusion: 'failure', + startedAt: null, + completedAt: null + } + ] + } + ] +} + +describe('CheckRunDetailsPanel card copy actions', () => { + it('renders a copy action for every card and copies each card payload', async () => { + render( + + + + ) + + fireEvent.click(screen.getByRole('button', { name: 'Copy output' })) + fireEvent.click(screen.getByRole('button', { name: 'Copy annotations' })) + fireEvent.click(screen.getByRole('button', { name: 'Copy jobs' })) + + await waitFor(() => expect(writeClipboardText).toHaveBeenCalledTimes(3)) + expect(writeClipboardText).toHaveBeenNthCalledWith( + 1, + 'Build failed\n\nOne test failed.\n\nRun the suite locally.' + ) + expect(writeClipboardText).toHaveBeenNthCalledWith( + 2, + 'src/main.ts:10\nfailure\nType error\nExpected string\nActual: number' + ) + expect(writeClipboardText).toHaveBeenNthCalledWith( + 3, + 'unit: failure\ntest: failure\nAssertionError: expected true' + ) + }) +}) From 4f14d6d757c0d3eee7bf52f6704682f5c02be270 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 27 Aug 2026 19:06:08 -0700 Subject: [PATCH 10/10] fix(mobile): stop held accessory keys after release under latency (#14219) * fix(mobile): backpressure accessory key repeats * fix(mobile): keep accessory repeats on the pressed terminal * fix(mobile): serialize accessory key presses * fix(mobile): preserve queued accessory taps * fix(mobile): fence queued taps across reconnects * fix(mobile): recheck queued tap delivery context * fix(mobile): stop accessory repeats after IME send failure * fix(mobile): pace repeated live-input edits * fix(mobile): dispatch accessory taps without ack delay --- .../app/h/[hostId]/session/[worktreeId].tsx | 73 ++++--- .../terminal-accessory-repeat.test.ts | 192 ++++++++++++++++++ .../src/terminal/terminal-accessory-repeat.ts | 76 +++++++ .../terminal-live-accessory-raw-send.test.ts | 38 +++- .../terminal-live-accessory-raw-send.ts | 12 +- ...rminal-live-accessory-input-commit.test.ts | 16 +- ...se-terminal-live-accessory-input-commit.ts | 21 +- .../use-terminal-live-input-commit.test.ts | 14 ++ .../use-terminal-live-input-commit.ts | 2 +- .../use-terminal-live-pending-input-flush.ts | 11 +- 10 files changed, 398 insertions(+), 57 deletions(-) create mode 100644 mobile/src/terminal/terminal-accessory-repeat.test.ts create mode 100644 mobile/src/terminal/terminal-accessory-repeat.ts diff --git a/mobile/app/h/[hostId]/session/[worktreeId].tsx b/mobile/app/h/[hostId]/session/[worktreeId].tsx index 14a94513507..0d36fa96020 100644 --- a/mobile/app/h/[hostId]/session/[worktreeId].tsx +++ b/mobile/app/h/[hostId]/session/[worktreeId].tsx @@ -117,6 +117,10 @@ import { } from '../../../../src/terminal/terminal-accessory-layout' import { createTerminalLiveAccessoryInput } from '../../../../src/terminal/terminal-live-accessory-input' import { sendTerminalLiveAccessoryRawBytes } from '../../../../src/terminal/terminal-live-accessory-raw-send' +import { + createTerminalAccessoryRepeatController, + createTerminalAccessoryRepeatSender +} from '../../../../src/terminal/terminal-accessory-repeat' import { clearTerminalLiveInputFocusTimer, isTerminalLiveInputWithinByteLimit, @@ -2993,16 +2997,22 @@ export default function SessionScreen() { } } - async function handleAccessoryKey(input: ReturnType) { - if (!client || !activeHandle || !canSend) { - return + async function handleAccessoryKey( + input: ReturnType, + targetHandle = activeHandleRef.current, + isDeliveryTargetCurrent: () => boolean = () => targetHandle === activeHandleRef.current + ) { + if (!client || !targetHandle || !isDeliveryTargetCurrent() || !canSend) { + return false } - const targetHandle = activeHandle const accessoryCommit = await handleLiveInputAccessoryBytes(input) - if (accessoryCommit.kind !== 'allow-raw') { - return + if (!isDeliveryTargetCurrent()) { + return false } - await sendTerminalLiveAccessoryRawBytes({ + if (accessoryCommit.kind !== 'allow-raw') { + return accessoryCommit.kind === 'handled' + } + return sendTerminalLiveAccessoryRawBytes({ client: clientRef.current, targetHandle, activeHandle: activeHandleRef.current, @@ -3399,32 +3409,37 @@ export default function SessionScreen() { } } - // Why: hold-to-repeat matches iOS cadence (400ms then 45ms); non-repeatable keys fire once (holding is destructive). - const repeatTimeoutRef = useRef | null>(null) - const repeatIntervalRef = useRef | null>(null) - // Why: ref keeps repeat firing the current callback; else a mid-hold tab switch/reconnect routes bytes to a stale terminal. + const accessoryRepeatRef = useRef( + createTerminalAccessoryRepeatController>() + ) + // Why: the current callback observes reconnect state while the sender pins the press to its original terminal. const handleAccessoryKeyRef = useRef(handleAccessoryKey) handleAccessoryKeyRef.current = handleAccessoryKey const stopAccessoryRepeat = useCallback(() => { - if (repeatTimeoutRef.current) { - clearTimeout(repeatTimeoutRef.current) - repeatTimeoutRef.current = null - } - if (repeatIntervalRef.current) { - clearInterval(repeatIntervalRef.current) - repeatIntervalRef.current = null - } + accessoryRepeatRef.current.stop() }, []) const startAccessoryRepeat = useCallback( (input: ReturnType) => { - stopAccessoryRepeat() - repeatTimeoutRef.current = setTimeout(() => { - repeatIntervalRef.current = setInterval(() => { - void handleAccessoryKeyRef.current(input) - }, 45) - }, 400) + // Why: a held repeat must not resume through a replacement client or connection. + const targetClient = clientRef.current + const targetConnectedAt = targetClient?.getLastConnectedAt() ?? null + const isDeliveryTargetCurrent = (targetHandle: string) => + activeHandleRef.current === targetHandle && + targetClient !== null && + clientRef.current === targetClient && + connStateRef.current === 'connected' && + targetClient.getLastConnectedAt() === targetConnectedAt + accessoryRepeatRef.current.start( + input, + createTerminalAccessoryRepeatSender( + activeHandleRef.current, + isDeliveryTargetCurrent, + (nextInput, targetHandle, isTargetCurrent) => + handleAccessoryKeyRef.current(nextInput, targetHandle, isTargetCurrent) + ) + ) }, - [stopAccessoryRepeat] + [] ) const setMobileSessionRootRef = useCallback( (node: View | null): void => { @@ -3440,15 +3455,14 @@ export default function SessionScreen() { clearPendingLiveInputCommit() sessionTabActionSheetRequestSeqRef.current += 1 clearSessionTabActionSheetKeyboardListener() - stopAccessoryRepeat() + accessoryRepeatRef.current.cancel() }, [ clearPendingLiveInputCommit, clearDelayedActionTimers, clearSessionTabActionSheetKeyboardListener, clearTerminalCache, - clearToastHideTimer, - stopAccessoryRepeat + clearToastHideTimer ] ) @@ -4848,7 +4862,6 @@ export default function SessionScreen() { return } const input = createTerminalLiveAccessoryInput(key) - void handleAccessoryKey(input) startAccessoryRepeat(input) }} onPressOut={() => { diff --git a/mobile/src/terminal/terminal-accessory-repeat.test.ts b/mobile/src/terminal/terminal-accessory-repeat.test.ts new file mode 100644 index 00000000000..eab103eac15 --- /dev/null +++ b/mobile/src/terminal/terminal-accessory-repeat.test.ts @@ -0,0 +1,192 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { + createTerminalAccessoryRepeatController, + createTerminalAccessoryRepeatSender, + TERMINAL_ACCESSORY_REPEAT_DELAY_MS, + TERMINAL_ACCESSORY_REPEAT_INTERVAL_MS +} from './terminal-accessory-repeat' + +function deferred() { + let resolve: () => void = () => {} + const promise = new Promise((done) => { + resolve = done + }) + return { promise, resolve } +} + +describe('terminal accessory repeat', () => { + afterEach(() => vi.useRealTimers()) + + it('keeps one send in flight and preserves repeat order under host latency', async () => { + vi.useFakeTimers() + const first = deferred() + const second = deferred() + const sent: string[] = [] + const send = vi.fn((input: string) => { + sent.push(input) + return (sent.length === 1 ? first.promise : second.promise).then(() => true) + }) + const repeat = createTerminalAccessoryRepeatController() + + repeat.start('down', send) + expect(sent).toEqual(['down']) + + await vi.advanceTimersByTimeAsync(TERMINAL_ACCESSORY_REPEAT_DELAY_MS + 500) + expect(sent).toEqual(['down']) + + first.resolve() + await vi.advanceTimersByTimeAsync(0) + expect(sent).toEqual(['down', 'down']) + + await vi.advanceTimersByTimeAsync(TERMINAL_ACCESSORY_REPEAT_INTERVAL_MS + 500) + expect(sent).toEqual(['down', 'down']) + + second.resolve() + await vi.advanceTimersByTimeAsync(TERMINAL_ACCESSORY_REPEAT_INTERVAL_MS) + expect(sent).toEqual(['down', 'down', 'down']) + }) + + it('does not schedule another repeat after release while a send is pending', async () => { + vi.useFakeTimers() + const pending = deferred() + const send = vi.fn(() => pending.promise.then(() => true)) + const repeat = createTerminalAccessoryRepeatController() + + repeat.start('down', send) + repeat.stop() + pending.resolve() + await vi.runAllTimersAsync() + + expect(send).toHaveBeenCalledTimes(1) + }) + + it('dispatches distinct taps without waiting for prior acknowledgements', async () => { + vi.useFakeTimers() + const pending = Array.from({ length: 5 }, () => deferred()) + const sent: string[] = [] + const send = vi.fn((input: string) => { + sent.push(input) + return pending[sent.length - 1]!.promise.then(() => true) + }) + const repeat = createTerminalAccessoryRepeatController() + + for (let index = 0; index < pending.length; index += 1) { + repeat.start(`tap-${index}`, send) + repeat.stop() + } + + expect(sent).toEqual(['tap-0', 'tap-1', 'tap-2', 'tap-3', 'tap-4']) + + pending.forEach(({ resolve }) => resolve()) + await vi.runAllTimersAsync() + expect(send).toHaveBeenCalledTimes(5) + }) + + it('cancels repeats without dropping a distinct tap already dispatched by the user', async () => { + vi.useFakeTimers() + const first = deferred() + const sent: string[] = [] + const send = vi.fn((input: string) => { + sent.push(input) + return input === 'down' ? first.promise.then(() => true) : Promise.resolve(true) + }) + const repeat = createTerminalAccessoryRepeatController() + + repeat.start('down', send) + repeat.stop() + repeat.start('up', send) + repeat.cancel() + + first.resolve() + await vi.runAllTimersAsync() + + expect(sent).toEqual(['down', 'up']) + expect(send).toHaveBeenCalledTimes(2) + }) + + it('stops repeating when the transport rejects a send', async () => { + vi.useFakeTimers() + const send = vi.fn(() => Promise.reject(new Error('disconnected'))) + const repeat = createTerminalAccessoryRepeatController() + + repeat.start('down', send) + await vi.runAllTimersAsync() + + expect(send).toHaveBeenCalledTimes(1) + }) + + it('stops repeating when the send is dropped before reaching the transport', async () => { + vi.useFakeTimers() + const send = vi.fn(async () => false) + const repeat = createTerminalAccessoryRepeatController() + + repeat.start('down', send) + await vi.runAllTimersAsync() + + expect(send).toHaveBeenCalledTimes(1) + }) + + it('stops sending when the terminal active at press time is no longer active', async () => { + let activeTerminal = 'terminal-a' + const sendToTerminal = vi.fn(async () => true) + const send = createTerminalAccessoryRepeatSender( + activeTerminal, + (targetHandle) => activeTerminal === targetHandle, + sendToTerminal + ) + + await expect(send('down')).resolves.toBe(true) + activeTerminal = 'terminal-b' + await expect(send('down')).resolves.toBe(false) + + expect(sendToTerminal).toHaveBeenCalledTimes(1) + expect(sendToTerminal).toHaveBeenCalledWith('down', 'terminal-a', expect.any(Function)) + }) + + it('provides a live target guard for async send preflight', async () => { + let targetCurrent = true + const preflight = deferred() + const send = createTerminalAccessoryRepeatSender( + 'terminal-a', + () => targetCurrent, + async (_input, _targetHandle, isDeliveryTargetCurrent) => { + await preflight.promise + return isDeliveryTargetCurrent() + } + ) + + const result = send('down') + targetCurrent = false + preflight.resolve() + + await expect(result).resolves.toBe(false) + }) + + it('stops held repeats when the press-time connection is no longer current', async () => { + vi.useFakeTimers() + let connectionGeneration = 1 + const first = deferred() + const sent: string[] = [] + const sendToTerminal = vi.fn((input: string) => { + sent.push(input) + return input === 'down' ? first.promise.then(() => true) : Promise.resolve(true) + }) + const createSender = () => { + const pressedConnectionGeneration = connectionGeneration + return createTerminalAccessoryRepeatSender( + 'terminal-a', + () => connectionGeneration === pressedConnectionGeneration, + sendToTerminal + ) + } + const repeat = createTerminalAccessoryRepeatController() + + repeat.start('down', createSender()) + + connectionGeneration = 2 + first.resolve() + await vi.runAllTimersAsync() + + expect(sent).toEqual(['down']) + }) +}) diff --git a/mobile/src/terminal/terminal-accessory-repeat.ts b/mobile/src/terminal/terminal-accessory-repeat.ts new file mode 100644 index 00000000000..2b354b42d59 --- /dev/null +++ b/mobile/src/terminal/terminal-accessory-repeat.ts @@ -0,0 +1,76 @@ +export const TERMINAL_ACCESSORY_REPEAT_DELAY_MS = 400 +export const TERMINAL_ACCESSORY_REPEAT_INTERVAL_MS = 45 + +type TerminalAccessoryRepeatSender = (input: TInput) => Promise +type TerminalAccessoryRepeatSendToTerminal = ( + input: TInput, + targetHandle: string, + isDeliveryTargetCurrent: () => boolean +) => Promise + +export function createTerminalAccessoryRepeatSender( + targetHandle: string | null, + isDeliveryTargetCurrent: (targetHandle: string) => boolean, + sendToTerminal: TerminalAccessoryRepeatSendToTerminal +): TerminalAccessoryRepeatSender { + return (input) => { + if (!targetHandle || !isDeliveryTargetCurrent(targetHandle)) { + return Promise.resolve(false) + } + return sendToTerminal(input, targetHandle, () => isDeliveryTargetCurrent(targetHandle)) + } +} + +export function createTerminalAccessoryRepeatController() { + let generation = 0 + let timer: ReturnType | null = null + + const stop = () => { + generation += 1 + if (timer !== null) { + clearTimeout(timer) + timer = null + } + } + + const cancel = stop + + const start = (input: TInput, send: TerminalAccessoryRepeatSender) => { + stop() + const activeGeneration = generation + const pressedAt = Date.now() + + const schedule = (delayMs: number) => { + timer = setTimeout(() => { + timer = null + if (generation !== activeGeneration) { + return + } + void send(input).then( + (sent) => { + if (sent && generation === activeGeneration) { + schedule(TERMINAL_ACCESSORY_REPEAT_INTERVAL_MS) + } + }, + () => undefined + ) + }, delayMs) + } + + void send(input).then( + (sent) => { + if (sent && generation === activeGeneration) { + schedule( + Math.min( + TERMINAL_ACCESSORY_REPEAT_DELAY_MS, + Math.max(0, TERMINAL_ACCESSORY_REPEAT_DELAY_MS - (Date.now() - pressedAt)) + ) + ) + } + }, + () => undefined + ) + } + + return { cancel, start, stop } +} diff --git a/mobile/src/terminal/terminal-live-accessory-raw-send.test.ts b/mobile/src/terminal/terminal-live-accessory-raw-send.test.ts index 7a1110b407b..4ecab7dd10c 100644 --- a/mobile/src/terminal/terminal-live-accessory-raw-send.test.ts +++ b/mobile/src/terminal/terminal-live-accessory-raw-send.test.ts @@ -1,8 +1,16 @@ import { describe, expect, it, vi } from 'vitest' import { sendTerminalLiveAccessoryRawBytes } from './terminal-live-accessory-raw-send' import type { RpcClient } from '../transport/rpc-client' +import type { RpcResponse } from '../transport/types' -function captureClient(result: Promise = Promise.resolve({ ok: true })) { +function captureClient( + result: Promise = Promise.resolve({ + id: 'send', + ok: true, + result: { send: { handle: 'terminal-a', accepted: true, bytesWritten: 1 } }, + _meta: { runtimeId: 'test-runtime' } + }) +) { const sendRequest = vi.fn(() => result) return { client: { sendRequest } as unknown as Pick, sendRequest } } @@ -20,7 +28,7 @@ describe('terminal live accessory raw send', () => { it('sends raw bytes now-or-never with the device presence tag', async () => { const { client, sendRequest } = captureClient() - await sendTerminalLiveAccessoryRawBytes({ ...BASE_ARGS, client }) + await expect(sendTerminalLiveAccessoryRawBytes({ ...BASE_ARGS, client })).resolves.toBe(true) expect(sendRequest).toHaveBeenCalledWith( 'terminal.send', @@ -33,7 +41,9 @@ describe('terminal live accessory raw send', () => { it('drops the bytes instead of sending while disconnected', async () => { const { client, sendRequest } = captureClient() - await sendTerminalLiveAccessoryRawBytes({ ...BASE_ARGS, client, connState: 'reconnecting' }) + await expect( + sendTerminalLiveAccessoryRawBytes({ ...BASE_ARGS, client, connState: 'reconnecting' }) + ).resolves.toBe(false) expect(sendRequest).not.toHaveBeenCalled() }) @@ -41,16 +51,28 @@ describe('terminal live accessory raw send', () => { it('drops the bytes when the terminal selection went stale mid-flush', async () => { const { client, sendRequest } = captureClient() - await sendTerminalLiveAccessoryRawBytes({ ...BASE_ARGS, client, activeHandle: 'terminal-b' }) + await expect( + sendTerminalLiveAccessoryRawBytes({ ...BASE_ARGS, client, activeHandle: 'terminal-b' }) + ).resolves.toBe(false) expect(sendRequest).not.toHaveBeenCalled() }) - it('swallows a rejected send so accessory taps never surface transport errors', async () => { + it('reports a rejected send without surfacing transport errors', async () => { const { client } = captureClient(Promise.reject(new Error('Not connected: terminal.send'))) - await expect( - sendTerminalLiveAccessoryRawBytes({ ...BASE_ARGS, client }) - ).resolves.toBeUndefined() + await expect(sendTerminalLiveAccessoryRawBytes({ ...BASE_ARGS, client })).resolves.toBe(false) + }) + + it('reports a fulfilled RPC failure as a failed send', async () => { + const response: RpcResponse = { + id: 'send', + ok: false, + error: { code: 'terminal_error', message: 'failed' }, + _meta: { runtimeId: 'test-runtime' } + } + const { client } = captureClient(Promise.resolve(response)) + + await expect(sendTerminalLiveAccessoryRawBytes({ ...BASE_ARGS, client })).resolves.toBe(false) }) }) diff --git a/mobile/src/terminal/terminal-live-accessory-raw-send.ts b/mobile/src/terminal/terminal-live-accessory-raw-send.ts index 792491f6c63..3824d750792 100644 --- a/mobile/src/terminal/terminal-live-accessory-raw-send.ts +++ b/mobile/src/terminal/terminal-live-accessory-raw-send.ts @@ -1,4 +1,5 @@ import { getTerminalLiveAccessoryRawSendTarget } from './terminal-live-accessory-raw-send-target' +import { isTerminalSendRpcAccepted } from './terminal-send-rpc-response' import { buildTerminalSendParams, TERMINAL_INPUT_SEND_OPTIONS } from './terminal-send-request' import type { RpcClient } from '../transport/rpc-client' import type { ConnectionState } from '../transport/types' @@ -15,7 +16,7 @@ type TerminalLiveAccessoryRawSendArgs = { export async function sendTerminalLiveAccessoryRawBytes( args: TerminalLiveAccessoryRawSendArgs -): Promise { +): Promise { // Why: async IME flushing can outlive the original terminal selection. const rawSendTarget = getTerminalLiveAccessoryRawSendTarget({ targetHandle: args.targetHandle, @@ -23,9 +24,9 @@ export async function sendTerminalLiveAccessoryRawBytes( activeSessionTabType: args.activeSessionTabType }) if (!args.client || !rawSendTarget || args.connState !== 'connected') { - return + return false } - await args.client + return args.client .sendRequest( 'terminal.send', buildTerminalSendParams({ @@ -36,8 +37,5 @@ export async function sendTerminalLiveAccessoryRawBytes( }), TERMINAL_INPUT_SEND_OPTIONS ) - .then( - () => undefined, - () => undefined - ) + .then(isTerminalSendRpcAccepted, () => false) } diff --git a/mobile/src/terminal/use-terminal-live-accessory-input-commit.test.ts b/mobile/src/terminal/use-terminal-live-accessory-input-commit.test.ts index 4474f043ebe..ad1d1726798 100644 --- a/mobile/src/terminal/use-terminal-live-accessory-input-commit.test.ts +++ b/mobile/src/terminal/use-terminal-live-accessory-input-commit.test.ts @@ -70,7 +70,7 @@ function createAccessoryInputCommitHarness({ } } const applyLiveInputMirror = vi.fn( - (_handle: string, _fieldText: string, _composing?: boolean) => {} + async (_handle: string, _fieldText: string, _composing?: boolean) => true ) const clearPendingLiveInputCommit = vi.fn(() => {}) const flushPendingLiveInputText = vi.fn(async (_expectedHandle: string | null) => flushResult) @@ -171,6 +171,20 @@ describe('terminal live accessory input commit hook', () => { expect(result).toEqual({ kind: 'handled' }) }) + it('Given held text with a failed control send When committed Then reports the accessory input as suppressed', async () => { + const harness = createAccessoryInputCommitHarness({ + heldText: '한', + pendingHandle: 'terminal-a', + sendResult: false + }) + + const result = await harness.commit({ bytes: '\x1b' }) + + expect(harness.flushPendingLiveInputText).toHaveBeenCalledWith('terminal-a') + expect(harness.sent).toEqual(['\x1b']) + expect(result).toEqual({ kind: 'suppress-raw' }) + }) + it('Given raw accessory bytes with no held text When committed Then allows the raw send without flushing', async () => { // Given const harness = createAccessoryInputCommitHarness({ pendingHandle: null }) diff --git a/mobile/src/terminal/use-terminal-live-accessory-input-commit.ts b/mobile/src/terminal/use-terminal-live-accessory-input-commit.ts index ef6a2339822..80a8422d085 100644 --- a/mobile/src/terminal/use-terminal-live-accessory-input-commit.ts +++ b/mobile/src/terminal/use-terminal-live-accessory-input-commit.ts @@ -21,7 +21,11 @@ export async function getTerminalLiveAccessoryInactiveInputCommitResult( type TerminalLiveAccessoryInputCommitOptions = { readonly activeHandle: string | null - readonly applyLiveInputMirror: (handle: string, fieldText: string, composing?: boolean) => void + readonly applyLiveInputMirror: ( + handle: string, + fieldText: string, + composing?: boolean + ) => Promise readonly clearPendingLiveInputCommit: () => void readonly flushPendingLiveInputText: (expectedHandle: string | null) => Promise readonly heldLiveInputTextRef: RefObject @@ -84,15 +88,20 @@ export function useTerminalLiveAccessoryInputCommit({ setLiveInputCapture(editedText) liveInputRef.current?.setNativeProps({ text: editedText }) // Preserve undefined so Android's heuristic hold still settles on its timer. - applyLiveInputMirror(activeHandle, editedText, liveInputComposingRef.current) - return { kind: 'handled' } + const sent = await applyLiveInputMirror( + activeHandle, + editedText, + liveInputComposingRef.current + ) + return sent ? { kind: 'handled' } : { kind: 'suppress-raw' } } - case 'commit-held-then-send': - await sendTerminalLiveControlAfterPendingFlush( + case 'commit-held-then-send': { + const sent = await sendTerminalLiveControlAfterPendingFlush( () => flushPendingLiveInputText(activeHandle), () => sendLiveTerminalInputRef.current(activeHandle, decision.bytes) ) - return { kind: 'handled' } + return sent ? { kind: 'handled' } : { kind: 'suppress-raw' } + } default: decision satisfies never return { kind: 'handled' } diff --git a/mobile/src/terminal/use-terminal-live-input-commit.test.ts b/mobile/src/terminal/use-terminal-live-input-commit.test.ts index b24636114a9..6077ccb4033 100644 --- a/mobile/src/terminal/use-terminal-live-input-commit.test.ts +++ b/mobile/src/terminal/use-terminal-live-input-commit.test.ts @@ -190,6 +190,20 @@ describe('terminal live input commit hook', () => { await vi.waitFor(() => expect(sent).toEqual(['한'])) }) + it('Given a failed mirrored Backspace When accessory input commits Then reports failure', async () => { + const { handlers, sent } = createTerminalLiveInputCommitHarness({ sendResult: false }) + changeLiveInput(handlers, 'a', false) + await vi.waitFor(() => expect(sent).toEqual(['a'])) + + const result = await handlers.handleLiveInputAccessoryBytes({ + bytes: '\x7f', + localEdit: 'backspace' + }) + + expect(sent).toEqual(['a', '\x7f']) + expect(result).toEqual({ kind: 'suppress-raw' }) + }) + it('Given a held syllable When the settle timer elapses Then commits it to the terminal', async () => { // Given vi.useFakeTimers() diff --git a/mobile/src/terminal/use-terminal-live-input-commit.ts b/mobile/src/terminal/use-terminal-live-input-commit.ts index 25bbf1ae002..952612f0a87 100644 --- a/mobile/src/terminal/use-terminal-live-input-commit.ts +++ b/mobile/src/terminal/use-terminal-live-input-commit.ts @@ -133,7 +133,7 @@ export function useTerminalLiveInputCommit({ // that differs from the native field text, so the controlled capture must // echo the field verbatim; only the PTY mirror sees normalized text. setLiveInputCapture(nativeEvent.text) - applyLiveInputMirror( + void applyLiveInputMirror( activeHandle, normalizeTerminalTextInput(nativeEvent.text), nativeEvent.isComposing diff --git a/mobile/src/terminal/use-terminal-live-pending-input-flush.ts b/mobile/src/terminal/use-terminal-live-pending-input-flush.ts index c01aa5af213..ff0c09c1c51 100644 --- a/mobile/src/terminal/use-terminal-live-pending-input-flush.ts +++ b/mobile/src/terminal/use-terminal-live-pending-input-flush.ts @@ -30,7 +30,11 @@ type RunTerminalLiveMirrorStep = ( ) => Promise type TerminalLivePendingInputFlush = { - readonly applyLiveInputMirror: (handle: string, fieldText: string, composing?: boolean) => void + readonly applyLiveInputMirror: ( + handle: string, + fieldText: string, + composing?: boolean + ) => Promise readonly clearPendingLiveInputCommit: () => void readonly flushPendingLiveInputText: (expectedHandle: string | null) => Promise readonly heldLiveInputTextRef: RefObject @@ -153,9 +157,8 @@ export function useTerminalLivePendingInputFlush({ }, [runMirrorStep]) const applyLiveInputMirror = useCallback( - (handle: string, fieldText: string, composing?: boolean): void => { - void runMirrorStep(handle, fieldText, false, composing) - }, + (handle: string, fieldText: string, composing?: boolean): Promise => + runMirrorStep(handle, fieldText, false, composing), [runMirrorStep] )