From ef4e9c40ab05a58f714451b8ecb3b86dd334f1ca Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 2 Sep 2026 14:03:21 -0700 Subject: [PATCH 1/5] fix(terminal): replay paired-runtime snapshots at the host's grid (#18132) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(terminal): replay paired-runtime snapshots at the host's grid A paired remote pane parsed the host's authoritative terminal image at whatever grid its own xterm happened to have. The host dimensions every snapshot it publishes, but only the REQUESTED snapshot path ever read `cols`/`rows` back — both PUSH paths (initial subscribe and server recovery) dropped them, so `onSnapshot` handed the transport an image with no grid and the drain wrote it as-is. Serialized frames are grid-relative: rows are newline-fed and the frame ends in an absolute CUP. Parsed at a different grid they re-wrap and clip, and because an alternate-screen TUI has no scrollback the rows scrolled off the top are gone. An idle agent never repaints, so the pane stays wrong until the next byte arrives — which for a finished Claude Code session is never. Carry the grid the host already publishes through the multiplexer and transport, then reuse the choreography the reattach payload already follows: resize to the source grid, replay, fit back to the pane, and push the resulting grid to the PTY. A host that publishes no dimensions reads as unknown and keeps today's behaviour, so no wire change and no capability negotiation is involved. * fix(terminal): keep the source-grid fit correct under mobile fit overrides Two follow-ups on the source-grid replay: - A mobile fit override skipped the post-replay fit entirely, stranding the pane at the host's replay grid. Fit without the PTY grid push instead, matching applyMainBufferSnapshot. - Reset the source-grid flag when a drain is scheduled: a transaction whose restore was skipped never runs afterRestore, and the stale flag would fit a later drain that never left the pane's own grid. * perf(terminal): clear the replay buffer before the source-grid resize The drain resized xterm to the host's serialization grid and only then wrote the clearing `2J`/`3J`/`H`. `clearBeforeReplay` is true for every pushed remote snapshot, so a column change reflowed a full scrollback that the next sequence discarded microseconds later — on the recovery push that lands under output flood, when the renderer is already loaded. The clear is grid-independent, so running it first is equivalent: the resize then operates on an empty buffer. Verified identical end state (content, cursor, buffer type, baseY) across cols-change, rows-change, alt-screen, no-scrollback and equal-grid shapes. Interleaved 25-run medians on a 10k-line scrollback: 6.19ms -> 2.48ms on the normal buffer, unchanged on the alternate screen (where `3J` cannot free the normal buffer's history, so the reflow is paid either way). --- ...ection-remote-snapshot-source-grid.test.ts | 242 ++++++++++++++++++ .../deferred-cold-restore-and-snapshot.ts | 5 +- .../pty-connection/replay-data-drain.ts | 91 ++++++- .../terminal-pane/pty-output-processor.ts | 7 +- .../terminal-pane/pty-transport-types.ts | 4 + ...e-runtime-pty-snapshot-source-grid.test.ts | 138 ++++++++++ ...time-pty-transport-snapshot-replay.test.ts | 6 +- .../remote-runtime-pty-transport.ts | 6 + ...emote-runtime-terminal-binary-snapshots.ts | 11 +- ...mote-runtime-terminal-multiplexer-types.ts | 4 + .../runtime/runtime-terminal-stream.test.ts | 13 +- 11 files changed, 516 insertions(+), 11 deletions(-) create mode 100644 src/renderer/src/components/terminal-pane/pty-connection-remote-snapshot-source-grid.test.ts create mode 100644 src/renderer/src/components/terminal-pane/remote-runtime-pty-snapshot-source-grid.test.ts diff --git a/src/renderer/src/components/terminal-pane/pty-connection-remote-snapshot-source-grid.test.ts b/src/renderer/src/components/terminal-pane/pty-connection-remote-snapshot-source-grid.test.ts new file mode 100644 index 00000000000..f4e8d8bd2bb --- /dev/null +++ b/src/renderer/src/components/terminal-pane/pty-connection-remote-snapshot-source-grid.test.ts @@ -0,0 +1,242 @@ +import type * as React from 'react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { flushAsyncTicks, renderHeadlessBuffer } from './pty-connection-test-async' +import { createMockTransport, createPane, createManager } from './pty-connection-test-pane-fixtures' +import type { ConnectCallbacks, MockTransport } from './pty-connection-test-pane-fixtures' +import { buildPaneConnectionDeps } from './pty-connection-test-deps' +import { + createInitialStoreState, + buildActiveRuntimeEnvironmentState +} from './pty-connection-test-store-fixtures' +import type { StoreState } from './pty-connection-test-store-state' +import { + installTerminalTestGlobals, + restoreTerminalTestGlobals +} from './pty-connection-test-environment' + +const { + resetAndRefreshAllTerminalWebglAtlases, + scheduleTerminalWebglAtlasRecovery, + scheduleRuntimeGraphSync, + shouldSeedCacheTimerOnInitialTitle, + toastInfo, + notifyCodexPaneBoundForStaleSweep +} = vi.hoisted(() => ({ + resetAndRefreshAllTerminalWebglAtlases: vi.fn(), + scheduleTerminalWebglAtlasRecovery: vi.fn(), + scheduleRuntimeGraphSync: vi.fn(), + shouldSeedCacheTimerOnInitialTitle: vi.fn(() => false), + toastInfo: vi.fn(), + notifyCodexPaneBoundForStaleSweep: vi.fn() +})) + +let mockStoreState: StoreState +let transportFactoryQueue: MockTransport[] = [] +let createdTransportOptions: Record[] = [] +let storeSubscribers: ((state: StoreState) => void)[] = [] + +vi.mock('@/runtime/sync-runtime-graph', () => ({ + scheduleRuntimeGraphSync +})) + +vi.mock('@/lib/pane-manager/pane-manager-registry', async (importOriginal) => ({ + ...(await importOriginal>()), + resetAndRefreshAllTerminalWebglAtlases +})) + +vi.mock('./terminal-webgl-atlas-recovery', () => ({ + scheduleTerminalWebglAtlasRecovery +})) + +vi.mock('@/store', () => ({ + useAppStore: { + getState: () => mockStoreState, + subscribe: (listener: (state: StoreState) => void) => { + storeSubscribers.push(listener) + return () => { + storeSubscribers = storeSubscribers.filter((candidate) => candidate !== listener) + } + } + } +})) + +vi.mock('@/lib/agent-status', async (importOriginal) => { + const { buildAgentStatusModuleMock } = await import('./pty-connection-test-environment') + return buildAgentStatusModuleMock(await importOriginal>()) +}) + +vi.mock('./cache-timer-seeding', () => ({ + shouldSeedCacheTimerOnInitialTitle +})) + +vi.mock('sonner', () => ({ + toast: { info: toastInfo } +})) + +vi.mock('@/lib/codex-stale-pane-sweep', () => ({ + notifyCodexPaneBoundForStaleSweep +})) + +vi.mock('react', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + useCallback: unknown>(fn: T): T => fn + } +}) + +vi.mock('./pty-transport', () => ({ + createIpcPtyTransport: vi.fn((options: Record) => { + createdTransportOptions.push(options) + const nextTransport = transportFactoryQueue.shift() + if (!nextTransport) { + throw new Error('No mock transport queued') + } + return nextTransport + }) +})) + +vi.mock('./remote-runtime-pty-transport', () => ({ + createRemoteRuntimePtyTransport: vi.fn( + (_environmentId: string, options: Record) => { + createdTransportOptions.push(options) + const nextTransport = transportFactoryQueue.shift() + if (!nextTransport) { + throw new Error('No mock transport queued') + } + return nextTransport + } + ) +})) + +vi.mock('./pty-dispatcher', async (importOriginal) => { + const actual = await importOriginal>() + return { + ...actual, + getEagerPtyBufferHandle: vi.fn(() => undefined) + } +}) + +const HOST_COLS = 143 +const HOST_ROWS = 12 +const PANE_COLS = 120 +const PANE_ROWS = 40 + +// A serialized TUI frame the way @xterm/addon-serialize emits one: newline-fed +// rows plus a trailing absolute CUP. Both are grid-relative. +const HOST_FRAME = `\x1b[?1049h\x1b[2J\x1b[H${Array.from( + { length: HOST_ROWS }, + (_unused, index) => `host row ${index + 1}` +).join('\r\n')}\x1b[${HOST_ROWS};3H` + +function createDeps(overrides: Record = {}) { + return buildPaneConnectionDeps(() => mockStoreState, overrides) +} + +async function connectRemotePane(): Promise<{ + operations: { kind: 'resize' | 'write'; value: string }[] + pane: ReturnType + transport: MockTransport + replay: (data: string, meta?: Record) => void + dispose: () => void +}> { + const { connectPanePty } = await import('./pty-connection') + mockStoreState = buildActiveRuntimeEnvironmentState(mockStoreState, 'env-1') + const transport = createMockTransport('remote:env-1@@terminal-1') + const captured: { current: ConnectCallbacks['onReplayData'] | null } = { current: null } + transport.connect.mockImplementation(async ({ callbacks }: { callbacks: ConnectCallbacks }) => { + captured.current = callbacks.onReplayData ?? null + return { id: 'remote:env-1@@terminal-1', replay: '' } + }) + transportFactoryQueue.push(transport) + + const pane = createPane(1) + pane.terminal.cols = PANE_COLS + pane.terminal.rows = PANE_ROWS + const operations: { kind: 'resize' | 'write'; value: string }[] = [] + pane.terminal.write = vi.fn((data: string, callback?: () => void) => { + operations.push({ kind: 'write', value: data }) + callback?.() + }) + pane.terminal.resize = vi.fn((cols: number, rows: number) => { + operations.push({ kind: 'resize', value: `${cols}x${rows}` }) + pane.terminal.cols = cols + pane.terminal.rows = rows + }) + pane.fitAddon.proposeDimensions = vi.fn(() => ({ cols: PANE_COLS, rows: PANE_ROWS })) + pane.fitAddon.fit = vi.fn(() => { + pane.terminal.resize(PANE_COLS, PANE_ROWS) + }) + + const manager = createManager(1) + const disposable = connectPanePty(pane as never, manager as never, createDeps() as never) + await flushAsyncTicks(6) + transport.resize.mockClear() + + return { + operations, + pane, + transport, + replay: (data, meta) => captured.current?.(data, meta as never), + dispose: () => disposable.dispose() + } +} + +describe('pushed remote snapshot replay grid', () => { + beforeEach(() => { + vi.resetModules() + vi.clearAllMocks() + transportFactoryQueue = [] + createdTransportOptions = [] + storeSubscribers = [] + mockStoreState = createInitialStoreState(() => mockStoreState) + installTerminalTestGlobals() + }) + + afterEach(async () => { + await restoreTerminalTestGlobals() + }) + + it('replays at the host grid and then pushes the pane grid back to the PTY', async () => { + const session = await connectRemotePane() + + session.replay(HOST_FRAME, { snapshotCols: HOST_COLS, snapshotRows: HOST_ROWS }) + await flushAsyncTicks(20) + + const frameWriteIndex = session.operations.findIndex( + (operation) => operation.kind === 'write' && operation.value === HOST_FRAME + ) + const sourceResizeIndex = session.operations.findIndex( + (operation) => operation.kind === 'resize' && operation.value === `${HOST_COLS}x${HOST_ROWS}` + ) + expect(sourceResizeIndex).toBeGreaterThanOrEqual(0) + expect(frameWriteIndex).toBeGreaterThan(sourceResizeIndex) + // Why the PTY push matters: the pane must not be left driving the host at + // the replay geometry once the destination fit has run. + expect(session.transport.resize).toHaveBeenCalledWith(PANE_COLS, PANE_ROWS) + expect(session.transport.resize).not.toHaveBeenCalledWith(HOST_COLS, HOST_ROWS) + session.dispose() + }) + + it('keeps the pane grid when the host published no snapshot dimensions', async () => { + const session = await connectRemotePane() + + session.replay(HOST_FRAME) + await flushAsyncTicks(20) + + expect(session.pane.terminal.resize).not.toHaveBeenCalledWith(HOST_COLS, HOST_ROWS) + session.dispose() + }) + + it('only reproduces the host frame when it is parsed at the host grid', async () => { + const atHostGrid = await renderHeadlessBuffer([HOST_FRAME], HOST_COLS, HOST_ROWS) + const atPaneGrid = await renderHeadlessBuffer([HOST_FRAME], PANE_COLS, HOST_ROWS - 4) + + // Why this is the user-visible failure: the alternate screen has no + // scrollback, so rows scrolled off by a shorter grid are gone for good and + // an idle TUI never repaints them. + expect(atHostGrid.filter((line) => line.startsWith('host row'))).toHaveLength(HOST_ROWS) + expect(atPaneGrid.filter((line) => line.startsWith('host row')).length).toBeLessThan(HOST_ROWS) + expect(atPaneGrid).not.toContain('host row 1') + }) +}) diff --git a/src/renderer/src/components/terminal-pane/pty-connection/deferred-cold-restore-and-snapshot.ts b/src/renderer/src/components/terminal-pane/pty-connection/deferred-cold-restore-and-snapshot.ts index 28e0f95b9f7..6c3a0628f79 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/deferred-cold-restore-and-snapshot.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/deferred-cold-restore-and-snapshot.ts @@ -156,7 +156,10 @@ export function bindDeferredColdRestoreAndSnapshot(session: ConnectPanePtySessio } : {}), ...(meta.terminalOwner ? { terminalOwner: meta.terminalOwner } : {}), - ...(meta.alternateScreen !== undefined ? { alternateScreen: meta.alternateScreen } : {}) + ...(meta.alternateScreen !== undefined ? { alternateScreen: meta.alternateScreen } : {}), + ...(meta.snapshotCols !== undefined && meta.snapshotRows !== undefined + ? { snapshotCols: meta.snapshotCols, snapshotRows: meta.snapshotRows } + : {}) } session.scheduleReplayDataDrain() } diff --git a/src/renderer/src/components/terminal-pane/pty-connection/replay-data-drain.ts b/src/renderer/src/components/terminal-pane/pty-connection/replay-data-drain.ts index 87a2b6039ce..81ac85ed752 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/replay-data-drain.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/replay-data-drain.ts @@ -1,4 +1,8 @@ import { waitForTerminalOutputParsed } from '@/lib/pane-manager/pane-terminal-output-scheduler' +import { safeFit, safeFitAndThen } from '@/lib/pane-manager/pane-tree-ops' +import { getFitOverrideForPty } from '@/lib/pane-manager/mobile-fit-overrides' + +import { resolvePositiveTerminalDimensions } from '../terminal-snapshot-replay-paint' import { CURSOR_SHOW_SEQUENCE, @@ -66,6 +70,9 @@ export function bindReplayDataDrain(session: ConnectPanePtySession): void { session.pendingReplayData = null session.replayPayloadGeneration = 0 let replayDrainQueued = false + // Why: a payload replayed at a foreign grid leaves xterm sized to the source, + // so the destination fit belongs after the whole transaction parses. + let replayedAtSourceGrid = false const drainReplayDataQueue = async ( expectedPtyId: string | null, expectedStreamGeneration: number @@ -86,8 +93,15 @@ export function bindReplayDataDrain(session: ConnectPanePtySession): void { return false } const payload = session.pendingReplayData - const { data, clearBeforeReplay, pendingEscapeTailAnsi, alternateScreen, terminalOwner } = - payload + const { + data, + clearBeforeReplay, + pendingEscapeTailAnsi, + alternateScreen, + terminalOwner, + snapshotCols, + snapshotRows + } = payload session.pendingReplayData = null const isCurrentPayload = (): boolean => !session.disposed && @@ -100,12 +114,35 @@ export function bindReplayDataDrain(session: ConnectPanePtySession): void { // Relay replay buffers may overlap with content already rendered in // xterm. Local eager replay decides this earlier so metadata-only frames // can keep restored scrollback while still using the replay guard. + // Why ahead of the source-grid resize: the clear is grid-independent, so + // dropping the scrollback first spares a reflow of history the very next + // sequence discards (see use-terminal-container-fit-sync.ts on its cost). if (clearBeforeReplay) { await session.writeReplayDataAsync('\x1b[2J\x1b[3J\x1b[H') if (!isCurrentPayload()) { continue } } + // Why before the frame: the payload's wraps and cursor moves are relative + // to the grid the host serialized it at. Parsing it at the pane's own grid + // clips or re-wraps the image, and an idle TUI never repaints to correct + // it — the pane stays blank until the next byte arrives. + const sourceGrid = resolvePositiveTerminalDimensions(snapshotCols, snapshotRows) + if ( + sourceGrid && + (session.pane.terminal.cols !== sourceGrid.cols || + session.pane.terminal.rows !== sourceGrid.rows) + ) { + // Why suppressed: this resize is a layout step for parsing, not the + // pane's real geometry — the destination fit below owns the PTY grid. + session.suppressStructuralReplayPtyResize = true + try { + session.pane.terminal.resize(sourceGrid.cols, sourceGrid.rows) + } finally { + session.suppressStructuralReplayPtyResize = false + } + replayedAtSourceGrid = true + } if (clearBeforeReplay || data.length > 0) { // Why: an empty clearing frame is still an authoritative repaint and // must clear a stale agent signal from an earlier payload. @@ -148,12 +185,59 @@ export function bindReplayDataDrain(session: ConnectPanePtySession): void { } return appliedCurrentPayload } + // Why the same helper the reattach payload uses: a source-grid replay leaves + // xterm at the host's geometry, so the pane must fit back and push the + // resulting grid to the PTY before live bytes resume. + const fitAfterSourceGridReplay = async ( + scheduledPtyId: string | null, + scheduledStreamGeneration: number + ): Promise => { + if (!replayedAtSourceGrid) { + return + } + replayedAtSourceGrid = false + if ( + session.disposed || + !scheduledPtyId || + session.transport.getPtyId() !== scheduledPtyId || + session.transportStreamGeneration !== scheduledStreamGeneration + ) { + return + } + if (getFitOverrideForPty(scheduledPtyId)) { + // Why fit without the grid push: a mobile driver owns the PTY geometry, + // but the pane must still leave the host's replay grid. + safeFit(session.pane) + return + } + const gridPush = session.createReattachGridPush(scheduledStreamGeneration, scheduledPtyId) + const fit = safeFitAndThen(session.pane, 'replay-source-grid-fit', gridPush.continuation, { + shouldContinue: gridPush.shouldContinue, + retryIfUnmeasurable: true, + // Why: a hidden or parked pane must still leave the source grid once it + // is revealed, or the PTY stays pinned to the host's replay geometry. + deferIfHidden: true + }) + session.pendingReattachFit = fit + try { + await fit.completion + } finally { + if (session.pendingReattachFit === fit) { + session.pendingReattachFit = null + } + } + } + session.scheduleReplayDataDrain = (): void => { if (replayDrainQueued) { return } const scheduledPtyId = session.pendingReplayData?.ptyId ?? null replayDrainQueued = true + // Why reset here: a transaction whose restore was skipped never ran its + // afterRestore, and a stale flag would fit a later drain that never left + // the pane's own grid. + replayedAtSourceGrid = false // Why: live bytes are newer than the authoritative replay frame. Hold // them until clear + replay + reset have all parsed, or replay can erase them. const scheduledStreamGeneration = @@ -171,7 +255,8 @@ export function bindReplayDataDrain(session: ConnectPanePtySession): void { shouldRestore: () => !session.disposed && session.transport.getPtyId() === scheduledPtyId && - session.transportStreamGeneration === scheduledStreamGeneration + session.transportStreamGeneration === scheduledStreamGeneration, + afterRestore: () => fitAfterSourceGridReplay(scheduledPtyId, scheduledStreamGeneration) } ) ) diff --git a/src/renderer/src/components/terminal-pane/pty-output-processor.ts b/src/renderer/src/components/terminal-pane/pty-output-processor.ts index ac611e8ed0c..983b619b3bd 100644 --- a/src/renderer/src/components/terminal-pane/pty-output-processor.ts +++ b/src/renderer/src/components/terminal-pane/pty-output-processor.ts @@ -37,6 +37,8 @@ export type ProcessPtyOutputOptions = { snapshotSeq?: number alternateScreen?: boolean terminalOwner?: 'shell' + snapshotCols?: number + snapshotRows?: number } function removeSuppressedCursorNativeTitles( @@ -217,7 +219,10 @@ export function createPtyOutputProcessor({ ...(options.alternateScreen !== undefined ? { alternateScreen: options.alternateScreen } : {}), - ...(options.terminalOwner ? { terminalOwner: options.terminalOwner } : {}) + ...(options.terminalOwner ? { terminalOwner: options.terminalOwner } : {}), + ...(options.snapshotCols !== undefined && options.snapshotRows !== undefined + ? { snapshotCols: options.snapshotCols, snapshotRows: options.snapshotRows } + : {}) } if (Object.keys(replayMeta).length > 0) { callbacks.onReplayData(data, replayMeta) diff --git a/src/renderer/src/components/terminal-pane/pty-transport-types.ts b/src/renderer/src/components/terminal-pane/pty-transport-types.ts index f041f495c5c..b3b3d0700d8 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport-types.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport-types.ts @@ -60,6 +60,10 @@ export type PtyReplayDataMeta = { snapshotSeq?: number alternateScreen?: boolean terminalOwner?: 'shell' + /** Grid the payload was serialized at. Present only when the producer proved + * it; the drain replays there and fits back to the pane afterwards. */ + snapshotCols?: number + snapshotRows?: number } export type LocalPtySessionMetadata = { diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-snapshot-source-grid.test.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-snapshot-source-grid.test.ts new file mode 100644 index 00000000000..3f49e6e0b7c --- /dev/null +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-snapshot-source-grid.test.ts @@ -0,0 +1,138 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { + TerminalStreamOpcode, + decodeTerminalStreamFrame, + decodeTerminalStreamJson, + encodeTerminalStreamFrame, + encodeTerminalStreamJson, + encodeTerminalStreamText +} from '../../../../shared/terminal-stream-protocol' + +// Client-side wire regression: the host dimensions every snapshot it publishes, +// but only the REQUESTED snapshot path ever read `cols`/`rows` back. Both PUSH +// paths (initial subscribe, server recovery) dropped them, so the pane parsed a +// host-grid image at its own grid and an idle TUI never repainted the damage. +// This drives REAL binary frames through the REAL multiplexer +// (decodeSnapshotInfo → onSnapshot meta) into the REAL transport +// (processData → onReplayData meta). One stream carries every case: the +// multiplexer is a module-level singleton, so separate cases would need +// separate module registries. + +describe('remote transport snapshot source-grid threading', () => { + const runtimeCall = vi.fn() + const runtimeSubscribe = vi.fn() + const subscriptionSendBinary = vi.fn() + let subscriptionCallbacks: { + onResponse: (response: unknown) => void + onBinary?: (bytes: Uint8Array) => void + onError?: (error: { code: string; message: string }) => void + onClose?: () => void + } | null = null + + beforeEach(() => { + vi.resetModules() + vi.doUnmock('../../runtime/remote-runtime-terminal-multiplexer') + vi.clearAllMocks() + subscriptionCallbacks = null + subscriptionSendBinary.mockReset() + runtimeCall.mockResolvedValue({ + ok: true, + result: { + terminal: { + handle: 'terminal-1', + tabId: 'tab-1', + leafId: 'pane:1', + worktreeId: 'wt-1' + } + } + }) + runtimeSubscribe.mockImplementation( + async (_args: unknown, callbacks: typeof subscriptionCallbacks) => { + subscriptionCallbacks = callbacks + return { unsubscribe: vi.fn(), sendBinary: subscriptionSendBinary } + } + ) + vi.stubGlobal('window', { + api: { + runtimeEnvironments: { call: runtimeCall, subscribe: runtimeSubscribe } + } + }) + }) + + it('carries the host grid on pushed snapshots and omits it when the host has none', async () => { + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const transport = createRemoteRuntimePtyTransport('env-1', { + worktreeId: 'wt-1', + tabId: 'tab-1', + leafId: 'pane:1' + }) + const onReplayData = vi.fn() + transport.attach({ + existingPtyId: 'remote:env-1@@terminal-1', + cols: 80, + rows: 24, + callbacks: { onReplayData } + }) + + await expect.poll(() => subscriptionCallbacks !== null, { timeout: 5000 }).toBe(true) + subscriptionCallbacks?.onResponse({ ok: true, result: { type: 'ready' } }) + await expect + .poll(() => subscriptionSendBinary.mock.calls.length, { timeout: 5000 }) + .toBeGreaterThan(0) + const subscribeFrame = subscriptionSendBinary.mock.calls + .map((call) => decodeTerminalStreamFrame(call[0] as Uint8Array)) + .find((frame) => frame?.opcode === TerminalStreamOpcode.Subscribe) + expect(subscribeFrame).toBeDefined() + const streamId = decodeTerminalStreamJson<{ streamId: number }>( + subscribeFrame!.payload + )!.streamId + + const deliverSnapshot = (start: Record, body: string): void => { + for (const frame of [ + encodeTerminalStreamFrame({ + opcode: TerminalStreamOpcode.SnapshotStart, + streamId, + seq: 0, + payload: encodeTerminalStreamJson(start) + }), + encodeTerminalStreamFrame({ + opcode: TerminalStreamOpcode.SnapshotChunk, + streamId, + seq: 0, + payload: encodeTerminalStreamText(body) + }), + encodeTerminalStreamFrame({ + opcode: TerminalStreamOpcode.SnapshotEnd, + streamId, + seq: 0, + payload: new Uint8Array(0) + }) + ]) { + subscriptionCallbacks?.onBinary?.(frame) + } + } + + // Initial subscribe push: the host's 143x43 grid must reach the restorer. + deliverSnapshot({ cols: 143, rows: 43, seq: 7, source: 'headless' }, 'restored TUI frame') + await expect.poll(() => onReplayData.mock.calls.length, { timeout: 5000 }).toBe(1) + expect(onReplayData).toHaveBeenLastCalledWith( + 'restored TUI frame', + expect.objectContaining({ snapshotCols: 143, snapshotRows: 43 }) + ) + + // Server-pushed recovery: untagged, after the initial snapshot landed. + deliverSnapshot({ cols: 154, rows: 68, seq: 9, source: 'headless' }, 'recovered') + await expect.poll(() => onReplayData.mock.calls.length, { timeout: 5000 }).toBe(2) + expect(onReplayData).toHaveBeenLastCalledWith( + '\x1b[2J\x1b[3J\x1b[Hrecovered', + expect.objectContaining({ snapshotCols: 154, snapshotRows: 68 }) + ) + + // A host that publishes no dimensions must read as unknown, not as a grid. + deliverSnapshot({ seq: 11, source: 'headless' }, 'undimensioned') + await expect.poll(() => onReplayData.mock.calls.length, { timeout: 5000 }).toBe(3) + const [, meta] = onReplayData.mock.calls[2] as [string, Record | undefined] + expect(meta?.snapshotCols).toBeUndefined() + expect(meta?.snapshotRows).toBeUndefined() + }) +}) diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport-snapshot-replay.test.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport-snapshot-replay.test.ts index c6d1b8508f7..b6fc15f6413 100644 --- a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport-snapshot-replay.test.ts +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport-snapshot-replay.test.ts @@ -127,7 +127,11 @@ describe('createRemoteRuntimePtyTransport', () => { emitOutput(streamId, liveOutput, liveSeq) expect(onReplayData).toHaveBeenCalledOnce() - expect(onReplayData).toHaveBeenCalledWith('AUTHORITATIVE_INITIAL_MARKER') + expect(onReplayData).toHaveBeenCalledWith( + 'AUTHORITATIVE_INITIAL_MARKER', + // The host's grid rides the snapshot so the pane replays it there. + expect.objectContaining({ snapshotCols: 80, snapshotRows: 24 }) + ) expect(onConnect).toHaveBeenCalledOnce() expect(onData).toHaveBeenCalledWith(liveOutput, expect.objectContaining({ seq: liveSeq })) await vi.waitFor(() => { diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts index a4290c5dbd5..1e5782fe496 100644 --- a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts @@ -1922,6 +1922,12 @@ export function createRemoteRuntimePtyTransport( : {}), ...(meta?.alternateScreen !== undefined && meta.seq !== undefined ? { alternateScreen: meta.alternateScreen } + : {}), + // Why unconditional on seq: the grid describes the image itself, + // not a stream boundary, so it is valid for every snapshot the + // host dimensions. Absent/zero degrades to the pane's own grid. + ...(meta?.cols !== undefined && meta.rows !== undefined + ? { snapshotCols: meta.cols, snapshotRows: meta.rows } : {}) }) } diff --git a/src/renderer/src/runtime/remote-runtime-terminal-binary-snapshots.ts b/src/renderer/src/runtime/remote-runtime-terminal-binary-snapshots.ts index c90c00fe00b..61cb242a856 100644 --- a/src/renderer/src/runtime/remote-runtime-terminal-binary-snapshots.ts +++ b/src/renderer/src/runtime/remote-runtime-terminal-binary-snapshots.ts @@ -93,7 +93,12 @@ export abstract class RemoteRuntimeTerminalBinarySnapshots extends RemoteRuntime seq: info?.seq, kittyKeyboardFlags: info?.kittyKeyboardFlags, alternateScreen: info?.alternateScreen, - terminalOwner: info?.terminalOwner + terminalOwner: info?.terminalOwner, + // Why: the image encodes wraps and cursor moves against the host's + // grid, so the restorer must replay it there — the request path has + // always carried these; the pushes silently dropped them. + cols: info?.cols, + rows: info?.rows }) } else if (target === 'recovery') { // Why: a server-pushed recovery snapshot replaces terminal state @@ -105,7 +110,9 @@ export abstract class RemoteRuntimeTerminalBinarySnapshots extends RemoteRuntime seq: info?.seq, kittyKeyboardFlags: info?.kittyKeyboardFlags, alternateScreen: info?.alternateScreen, - terminalOwner: info?.terminalOwner + terminalOwner: info?.terminalOwner, + cols: info?.cols, + rows: info?.rows }) } } else if (matchesPendingRequest) { diff --git a/src/renderer/src/runtime/remote-runtime-terminal-multiplexer-types.ts b/src/renderer/src/runtime/remote-runtime-terminal-multiplexer-types.ts index e8c6a286718..4b3a94aaf97 100644 --- a/src/renderer/src/runtime/remote-runtime-terminal-multiplexer-types.ts +++ b/src/renderer/src/runtime/remote-runtime-terminal-multiplexer-types.ts @@ -41,6 +41,10 @@ export type RemoteRuntimeMultiplexedTerminalCallbacks = { kittyKeyboardFlags?: number alternateScreen?: boolean terminalOwner?: 'shell' + /** Grid the host serialized this image at. Absent from hosts that omit + * it, which must read as unknown so replay keeps the pane's own grid. */ + cols?: number + rows?: number } ) => void onSubscribed?: () => void diff --git a/src/renderer/src/runtime/runtime-terminal-stream.test.ts b/src/renderer/src/runtime/runtime-terminal-stream.test.ts index 39cd8ce829a..2fcc8b2e293 100644 --- a/src/renderer/src/runtime/runtime-terminal-stream.test.ts +++ b/src/renderer/src/runtime/runtime-terminal-stream.test.ts @@ -572,7 +572,10 @@ describe('remote runtime terminal multiplex ACK gate', () => { injectSnapshot({ kind: 'scrollback', cols: 120, rows: 40, truncated: false }, 'initial state') expect(onSnapshot).toHaveBeenCalledWith('initial state', { - pendingEscapeTailAnsi: undefined + pendingEscapeTailAnsi: undefined, + // The host's serialization grid; the restorer replays there, not at the pane's own. + cols: 120, + rows: 40 }) expect(onSubscribed).toHaveBeenCalledTimes(1) @@ -590,7 +593,9 @@ describe('remote runtime terminal multiplex ACK gate', () => { // clears screen and scrollback first and must not replay the subscribe // lifecycle. expect(onSnapshot).toHaveBeenCalledWith(`\x1b[2J\x1b[3J\x1b[H${'recovered state'}`, { - pendingEscapeTailAnsi: undefined + pendingEscapeTailAnsi: undefined, + cols: 120, + rows: 40 }) expect(onSubscribed).toHaveBeenCalledTimes(1) @@ -607,7 +612,9 @@ describe('remote runtime terminal multiplex ACK gate', () => { '' ) expect(onSnapshot).toHaveBeenCalledWith('\x1b[2J\x1b[3J\x1b[H', { - pendingEscapeTailAnsi: undefined + pendingEscapeTailAnsi: undefined, + cols: 120, + rows: 40 }) expect(onSubscribed).toHaveBeenCalledTimes(1) From d8ca420cddc0a49df629484436ac7a54b137e74d Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 2 Sep 2026 14:03:46 -0700 Subject: [PATCH 2/5] perf(remote): apply the no-evidence inspection cadence to remote panes (#18146) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A visible remote/SSH terminal that has never run an agent inspected its execution host every ~2s forever for a strictly negative answer — ~30 RPC round trips per minute per pane, each a network hop plus a host-side foreground process scan. The `no-evidence` 15s cadence tier exists to bound exactly that volume, but `isProcessInspectionCostly` gated it on local Windows only and explicitly excluded remote-execution-host PTYs — the most expensive inspection shape in the codebase. Extract the predicate to `agent-process-inspection-cost.ts` and treat a remote-execution-host PTY as costly on every client platform. The local branch (Windows costly, POSIX cheap) is byte-identical. Client-side timer choice only: no wire change, no new field, no opcode. Activity (output/title/hook) re-arms the 2s cadence, agent evidence returns the tier to active/idle, and the `unavailable` branch and its error backoff are untouched. --- .../agent-completion-coordinator-types.ts | 8 +- ...ent-completion-no-evidence-cadence.test.ts | 85 +++++++++++++++++-- .../agent-process-inspection-cost.ts | 27 ++++++ .../pty-connection/terminal-keydown-fit.ts | 15 +--- 4 files changed, 112 insertions(+), 23 deletions(-) create mode 100644 src/renderer/src/components/terminal-pane/agent-process-inspection-cost.ts diff --git a/src/renderer/src/components/terminal-pane/agent-completion-coordinator-types.ts b/src/renderer/src/components/terminal-pane/agent-completion-coordinator-types.ts index c65fec024ad..43389f59ce2 100644 --- a/src/renderer/src/components/terminal-pane/agent-completion-coordinator-types.ts +++ b/src/renderer/src/components/terminal-pane/agent-completion-coordinator-types.ts @@ -46,10 +46,10 @@ export type AgentCompletionCoordinatorOptions = { // this renderer CONSUMES that evidence and can tell "no evidence published" // from "host too old to publish it" — mixed-version hosts omit the field. shouldPollNoEvidenceProcessCadence?: () => boolean - // Why: on hosts where one inspection forks a whole-process-table scan (local - // Windows PowerShell/CIM), panes without agent evidence relax to a slow - // cadence; remote authorities can disable no-evidence polling entirely and - // re-arm from output/title activity instead. + // Why: where one inspection is a whole-process-table scan (local Windows + // PowerShell/CIM) or a host round trip plus a host-side scan (remote/SSH), + // panes without agent evidence relax to a slow cadence and re-arm from + // output/title/hook activity. See agent-process-inspection-cost.ts. isProcessInspectionCostly?: () => boolean shouldSuppressHookCompletion?: (payload: AgentCompletionStatusSnapshot) => boolean } diff --git a/src/renderer/src/components/terminal-pane/agent-completion-no-evidence-cadence.test.ts b/src/renderer/src/components/terminal-pane/agent-completion-no-evidence-cadence.test.ts index a0a35c36057..238f073fd38 100644 --- a/src/renderer/src/components/terminal-pane/agent-completion-no-evidence-cadence.test.ts +++ b/src/renderer/src/components/terminal-pane/agent-completion-no-evidence-cadence.test.ts @@ -1,20 +1,27 @@ // Regression guard: bound the volume of cadence process inspections a visible, // idle terminal with NO agent evidence drives on hosts where each inspection is -// a whole-process-table scan (local Windows forks powershell.exe/CIM — the -// scan-cost analogue of #6288). Pre-fix a single visible idle shell inspected -// every 2s forever (~30 scans/min); with the no-evidence tier it inspects every -// 15s, and pane activity (output/title/hook) or agent evidence re-arms the hot -// cadence so agent-start detection stays event-driven and agent-finish -// detection is unchanged. +// expensive — local Windows forks a powershell.exe/CIM whole-process-table scan +// (the scan-cost analogue of #6288), and a remote/SSH pane pays a host round +// trip plus a host-side foreground scan. Pre-fix a single visible idle shell +// inspected every 2s forever (~30 scans/min); with the no-evidence tier it +// inspects every 15s, and pane activity (output/title/hook) or agent evidence +// re-arms the hot cadence so agent-start detection stays event-driven and +// agent-finish detection is unchanged. import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { createAgentCompletionCoordinator, resetAgentCompletionCoordinatorIdentitiesForTest } from './agent-completion-coordinator' import { resetAgentProcessInspectionQueueForTests } from './agent-process-inspection-queue' +import { isAgentProcessInspectionCostly } from './agent-process-inspection-cost' +import { toRemoteRuntimePtyId } from '../../../../shared/remote-runtime-pty-id' +import { toAppSshPtyId } from '../../../../shared/ssh-pty-id' import type { RuntimeTerminalProcessInspection } from '@/runtime/runtime-terminal-inspection' import type { AgentCompletionCoordinatorOptions } from './agent-completion-coordinator-types' +const MAC_UA = 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7)' +const WINDOWS_UA = 'Mozilla/5.0 (Windows NT 10.0; Win64; x64)' + function processResult( foregroundProcess: string | null, hasChildProcesses = foregroundProcess !== null @@ -67,6 +74,43 @@ describe('agent completion no-evidence inspection cadence', () => { expect(inspectProcess).toHaveBeenCalledTimes(4) }) + it('bounds a visible idle remote pane through the shipped cost predicate', async () => { + // Why: a remote inspection is an RPC round trip to the execution host plus a + // host-side foreground scan — the costliest inspection shape here — yet it + // was excluded from the no-evidence tier on every client platform. + const sshPtyId = toAppSshPtyId('target-1', 'pty-1') + const inspectProcess = vi.fn(async () => processResult(null, false)) + const { coordinator } = createCoordinator(inspectProcess, { + getPtyId: () => sshPtyId, + isProcessInspectionCostly: () => isAgentProcessInspectionCostly(MAC_UA, sshPtyId) + }) + + coordinator.startProcessTracking() + await vi.advanceTimersByTimeAsync(60_000) + + // 60s / 15s = 4 host round trips. Pre-fix (2s idle cadence) this was 30. + expect(inspectProcess).toHaveBeenCalledTimes(4) + }) + + it('re-arms the remote pane to the 2s cadence on the first byte of PTY output', async () => { + // Why: agent-start detection on a remote pane must stay event-driven, not + // wait out the relaxed interval. + const runtimePtyId = toRemoteRuntimePtyId('term_1', 'env-a') + const inspectProcess = vi.fn(async () => processResult(null, false)) + const { coordinator } = createCoordinator(inspectProcess, { + getPtyId: () => runtimePtyId, + isProcessInspectionCostly: () => isAgentProcessInspectionCostly(MAC_UA, runtimePtyId) + }) + + coordinator.startProcessTracking() + await vi.advanceTimersByTimeAsync(14_000) + expect(inspectProcess).not.toHaveBeenCalled() + + coordinator.observeOutputActivity() + await vi.advanceTimersByTimeAsync(2_000) + expect(inspectProcess).toHaveBeenCalledTimes(1) + }) + it('keeps the full 2s idle cadence on hosts where inspection is cheap', async () => { const inspectProcess = vi.fn(async () => processResult(null, false)) const { coordinator } = createCoordinator(inspectProcess, { @@ -76,7 +120,7 @@ describe('agent completion no-evidence inspection cadence', () => { coordinator.startProcessTracking() await vi.advanceTimersByTimeAsync(60_000) - // 60s / 2s = 30: POSIX/SSH/remote panes must not be relaxed. + // 60s / 2s = 30: local POSIX panes (cheap `ps`) must not be relaxed. expect(inspectProcess).toHaveBeenCalledTimes(30) }) @@ -275,3 +319,30 @@ describe('agent completion no-evidence inspection cadence', () => { }) }) }) + +describe('isAgentProcessInspectionCostly', () => { + it('treats remote-execution-host ptys as costly on every client platform', () => { + for (const userAgent of [MAC_UA, WINDOWS_UA]) { + expect(isAgentProcessInspectionCostly(userAgent, toAppSshPtyId('target-1', 'pty-1'))).toBe( + true + ) + expect( + isAgentProcessInspectionCostly(userAgent, toRemoteRuntimePtyId('term_1', 'env-a')) + ).toBe(true) + expect(isAgentProcessInspectionCostly(userAgent, toRemoteRuntimePtyId('term_1'))).toBe(true) + } + }) + + it('leaves the local branch unchanged: Windows costly, POSIX cheap', () => { + expect(isAgentProcessInspectionCostly(WINDOWS_UA, 'worktree-1|pane-1')).toBe(true) + expect(isAgentProcessInspectionCostly(WINDOWS_UA, null)).toBe(false) + expect(isAgentProcessInspectionCostly(MAC_UA, 'worktree-1|pane-1')).toBe(false) + expect(isAgentProcessInspectionCostly(MAC_UA, null)).toBe(false) + }) + + // Why: a bare "ssh:" id names no connection, so it is not evidence the + // inspection crosses a link (see remote-execution-host-pty.test.ts). + it('does not relax a POSIX pane for an ssh-prefixed id carrying no relay pty id', () => { + expect(isAgentProcessInspectionCostly(MAC_UA, 'ssh:target-1')).toBe(false) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/agent-process-inspection-cost.ts b/src/renderer/src/components/terminal-pane/agent-process-inspection-cost.ts new file mode 100644 index 00000000000..aeeea606bc0 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/agent-process-inspection-cost.ts @@ -0,0 +1,27 @@ +import { isRemoteExecutionHostPtyId } from './remote-execution-host-pty' + +/** + * Whether one cadence process inspection for this pane is expensive enough that + * a pane with no agent evidence should relax to the `no-evidence` tier. + * + * Why remote first: a remote inspection is a `terminal.inspectProcess` / + * `pty.inspectProcess` round trip to the execution host plus a host-side + * foreground scan there — the costliest shape in this codebase, on every client + * platform. Local Windows is costly for a different reason: it forks a + * powershell.exe whole-process-table CIM scan per poll (~10-40x POSIX `ps`). + * Local POSIX (and daemon/WSL panes on it) stays on the full cadence. + * + * Relaxing is the interim measure: once this renderer consumes the batched + * foreground evidence direct-SSH/remote authorities already publish with their + * PTY inventory (#17525), those panes can drop to `shouldPollNoEvidenceProcessCadence` + * and stop scheduling idle host reads altogether. + */ +export function isAgentProcessInspectionCostly(userAgent: string, ptyId: string | null): boolean { + if (ptyId !== null && isRemoteExecutionHostPtyId(ptyId)) { + return true + } + if (!userAgent.includes('Windows')) { + return false + } + return ptyId !== null +} diff --git a/src/renderer/src/components/terminal-pane/pty-connection/terminal-keydown-fit.ts b/src/renderer/src/components/terminal-pane/pty-connection/terminal-keydown-fit.ts index 947ead70c31..387e4362e4b 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/terminal-keydown-fit.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/terminal-keydown-fit.ts @@ -11,7 +11,7 @@ import { resolveCompatibleAgentTypeForOwner } from '../../../../../shared/agent- import { registerTerminalSideEffectFactConsumer } from '../terminal-side-effect-facts-handler' import { isAgentTaskCompleteTrackingEnabled } from './agent-task-complete-settings' -import { isRemoteExecutionHostPtyId } from '../remote-execution-host-pty' +import { isAgentProcessInspectionCostly } from '../agent-process-inspection-cost' import { isRemoteRuntimePtyId } from './paired-parked-terminal-restore' import type { ConnectPanePtySession } from './connect-pane-pty-session' @@ -229,17 +229,8 @@ export function installTerminalKeydownFit(session: ConnectPanePtySession): void }), shouldPollProcessCadence: () => isAgentTaskCompleteTrackingEnabled() && session.deps.isVisibleRef.current, - isProcessInspectionCostly: () => { - // Why: local Windows inspection forks a powershell.exe whole-process-table - // CIM scan per poll (~10-40x heavier than POSIX `ps`). Keep the no-evidence - // cadence enabled until inventory evidence is consumed by this renderer; - // mixed-version relays may omit the optional field. - if (!navigator.userAgent.includes('Windows')) { - return false - } - const ptyId = session.transport.getPtyId() - return ptyId !== null && !isRemoteExecutionHostPtyId(ptyId) - }, + isProcessInspectionCostly: () => + isAgentProcessInspectionCostly(navigator.userAgent, session.transport.getPtyId()), isLive: () => { if (session.disposed) { return false From 4ff96df2b527b74e2d1001d4ef7c5073743ce099 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Wed, 2 Sep 2026 14:05:29 -0700 Subject: [PATCH 3/5] Auto e2e tests autofix scheduled ci 1h run 32 20260902T0700 (#18227) * Fix flaky e2e tests with improved locators and synchronization Add explicit waits, use more robust element selectors, and simplify test setup to reduce race conditions. Replace file-based fixtures with programmatic browser creation, use parent-scoped locators for menu interactions, and poll for stable state before assertions. * Add E2E failure triage report for run 33564563164 - Reconciles 14 failed tests against job logs and trace artifacts - Categorizes failures: 8 product bugs, 2 flaky tests, 4 test updates - Documents test-maintenance fixes and diagnostic findings - Files 8 Linear issues with owners and fresh recurrence evidence - Provides next actions for product owners and repository maintenance * rm artifact notes * Refactor browser creation E2E test to use UI interactions - Click through menu instead of manipulating internal store state - Use Playwright's locator and toBeVisible() assertion patterns * Record E2E browser creation pageId before barrier check Move createdPageId assignment before the barrier arm/fire checks. This ensures the pageId is recorded unconditionally when tracking is enabled, allowing tests to distinguish between creations rejected before the host attempt vs those that failed after creation. * Remove browser page reclamation assertion from restart test Simplifies test by removing page ID tracking and poll checking if pages persist after paired runtime restart. --- .../web-runtime-browser-creation-e2e-fault.ts | 7 +- .../issue-12656-terminal-link-tooltip.spec.ts | 9 ++- ...er-creation-reconciliation-failure.spec.ts | 78 +++++++++---------- .../source-control-large-file-count.spec.ts | 4 +- tests/e2e/tabs.spec.ts | 4 +- ...tree-active-delete-scroll-position.spec.ts | 22 +----- 6 files changed, 58 insertions(+), 66 deletions(-) diff --git a/src/renderer/src/runtime/web-runtime-browser-creation-e2e-fault.ts b/src/renderer/src/runtime/web-runtime-browser-creation-e2e-fault.ts index db26cfa3d67..44a422a5d6a 100644 --- a/src/renderer/src/runtime/web-runtime-browser-creation-e2e-fault.ts +++ b/src/renderer/src/runtime/web-runtime-browser-creation-e2e-fault.ts @@ -179,10 +179,15 @@ export function throwIfE2eWebRuntimeBrowserCapabilityUnavailable(): void { } export async function pauseAfterE2eWebRuntimeBrowserCreate(remotePageId: string): Promise { - if (!e2eConfig.exposeStore || !armed || !createdPageBarrier) { + if (!e2eConfig.exposeStore) { return } + // Recorded before the arm check so a journey that never arms the barrier can still prove no host + // page was created — a null id is only evidence if a real create would have set one. createdPageId = remotePageId + if (!armed || !createdPageBarrier) { + return + } await createdPageBarrier } diff --git a/tests/e2e/issue-12656-terminal-link-tooltip.spec.ts b/tests/e2e/issue-12656-terminal-link-tooltip.spec.ts index 06fc725ee36..879c8305266 100644 --- a/tests/e2e/issue-12656-terminal-link-tooltip.spec.ts +++ b/tests/e2e/issue-12656-terminal-link-tooltip.spec.ts @@ -147,8 +147,13 @@ test.describe('Issue #12656 terminal link tooltip', () => { expect(Math.abs(idle.paneBottom - idle.terminalBottom)).toBeLessThanOrEqual(1) await expect .poll(async () => { - await moveToLink(orcaPage, probe) - return readTooltipState(orcaPage, probe.tabId) + const currentProbe = await locateUrl(orcaPage, url) + if (!currentProbe) { + return { display: 'none', text: '' } + } + probe = currentProbe + await moveToLink(orcaPage, currentProbe) + return readTooltipState(orcaPage, currentProbe.tabId) }) .toMatchObject({ display: '', text: expect.stringContaining(url) }) diff --git a/tests/e2e/paired-browser-creation-reconciliation-failure.spec.ts b/tests/e2e/paired-browser-creation-reconciliation-failure.spec.ts index 27bbfff03fb..be280fcff13 100644 --- a/tests/e2e/paired-browser-creation-reconciliation-failure.spec.ts +++ b/tests/e2e/paired-browser-creation-reconciliation-failure.spec.ts @@ -1,10 +1,7 @@ -import { writeFileSync } from 'node:fs' -import path from 'node:path' import type { Page, TestInfo } from '@stablyai/playwright-test' import { RuntimeClient } from '../../src/cli/runtime/client' import { expect, test } from './helpers/orca-app' import { readHostBrowserPageIds, readHostTabs } from './helpers/host-session-tabs' -import { openFileExplorer } from './helpers/file-explorer' import { launchHeadlessPairedRuntimeHost, type HeadlessPairedRuntimeHost @@ -17,8 +14,6 @@ import { } from './helpers/paired-electron-client' import { ensureTerminalVisible, waitForActiveWorktree, waitForSessionReady } from './helpers/store' -const FIXTURE_NAME = 'paired-browser-reconcile-failure.html' - type FaultSnapshot = { armed: boolean capabilityRejectionArmed: boolean @@ -36,6 +31,22 @@ type FaultWindow = Window & { } } +// Drives the real create menu so the failure surfaces through handleNewBrowserTab's toast. +async function startBrowserCreate(page: Page): Promise { + await page.evaluate(() => window.__store?.getState().setBrowserDefaultUrl('about:blank')) + await page.getByRole('button', { name: 'New tab' }).first().click() + const newBrowserTab = page.getByRole('menuitem', { name: /New Browser Tab/i }) + await expect(newBrowserTab).toBeVisible({ timeout: 30_000 }) + await newBrowserTab.click() +} + +async function readStableHostTabs(hostClient: RuntimeClient, repoPath: string) { + const { publicationEpoch, snapshotVersion, ...state } = await readHostTabs(hostClient, repoPath) + expect(publicationEpoch).not.toBe('') + expect(snapshotVersion).toBeGreaterThan(0) + return state +} + type ClientTabState = { browserTabIds: string[] browserWorkspaceIds: string[] @@ -115,14 +126,7 @@ async function runReconciliationFailureJourney(args: { }) .toMatchObject({ terminalTabIds: expect.arrayContaining([expect.any(String)]) }) - await openFileExplorer(page) - const fixtureRow = page.locator('[data-file-explorer-row]').filter({ hasText: FIXTURE_NAME }) - await expect(fixtureRow).toBeVisible({ timeout: 30_000 }) - await fixtureRow.click() - const openPreviewToSide = page.getByRole('button', { name: 'Open Preview to the Side' }) - await expect(openPreviewToSide).toBeVisible({ timeout: 30_000 }) const baselineClient = await readClientTabs(page, worktreeId) - expect(baselineClient.editorTabIds).not.toHaveLength(0) expect(baselineClient.terminalTabIds).not.toHaveLength(0) const baselineHostBrowserIds = await readHostBrowserPageIds(args.hostClient, args.repoPath) @@ -133,7 +137,7 @@ async function runReconciliationFailureJourney(args: { } fault.arm() }) - await openPreviewToSide.click() + await startBrowserCreate(page) const faultSnapshot = await expect .poll( @@ -155,9 +159,8 @@ async function runReconciliationFailureJourney(args: { } expect(await readHostBrowserPageIds(args.hostClient, args.repoPath)).toContain(createdPageId) - // Why: the tab is staged on click, so while the create is held the user already sees it — - // exactly one of it, in the new split. The rollback assertions after release are what prove - // the optimism is unwound rather than stranded. + // The managed-browser action stages one tab in the active group while the host create is held. + // The rollback assertions prove that optimism is unwound rather than stranded. const heldClient = await readClientTabs(page, worktreeId) const addedSince = (baseline: string[], held: string[]): string[] => { expect(held).toEqual(expect.arrayContaining(baseline)) @@ -169,7 +172,7 @@ async function runReconciliationFailureJourney(args: { ).toHaveLength(1) expect(heldClient.editorTabIds).toEqual(baselineClient.editorTabIds) expect(heldClient.terminalTabIds).toEqual(baselineClient.terminalTabIds) - expect(addedSince(baselineClient.groupIds, heldClient.groupIds)).toHaveLength(1) + expect(heldClient.groupIds).toEqual(baselineClient.groupIds) await page.screenshot({ path: args.testInfo.outputPath(`${args.topology}-browser-reconciliation-held.png`), @@ -181,9 +184,9 @@ async function runReconciliationFailureJourney(args: { ) ).toBe(true) - await expect(page.getByText('Unable to open this file in Orca Browser.')).toBeVisible({ - timeout: 30_000 - }) + await expect( + page.getByText('The paired runtime could not create a managed browser tab.') + ).toBeVisible({ timeout: 30_000 }) await expect .poll(() => readHostBrowserPageIds(args.hostClient, args.repoPath), { timeout: 30_000, @@ -261,14 +264,8 @@ async function runCapabilityFailureJourney(args: { }) .toMatchObject({ terminalTabIds: expect.arrayContaining([expect.any(String)]) }) - await openFileExplorer(page) - const fixtureRow = page.locator('[data-file-explorer-row]').filter({ hasText: FIXTURE_NAME }) - await expect(fixtureRow).toBeVisible({ timeout: 30_000 }) - await fixtureRow.click() - const openPreviewToSide = page.getByRole('button', { name: 'Open Preview to the Side' }) - await expect(openPreviewToSide).toBeVisible({ timeout: 30_000 }) const baselineClient = await readClientTabs(page, worktreeId) - const baselineHost = await readHostTabs(args.hostClient, args.repoPath) + const baselineHost = await readStableHostTabs(args.hostClient, args.repoPath) await page.evaluate(() => { const fault = (window as FaultWindow).__webRuntimeBrowserCreationFault @@ -277,18 +274,25 @@ async function runCapabilityFailureJourney(args: { } fault.armCapabilityRejection() }) - await openPreviewToSide.click() + await startBrowserCreate(page) - await expect(page.getByText('Unable to open this file in Orca Browser.')).toBeVisible({ + await expect(page.getByText(/E2E forced browser capability rejection/)).toBeVisible({ timeout: 30_000 }) + // Why: baseline equality alone also holds for a create that was rolled back. A null page id is + // what separates rejecting before the host create from undoing one afterwards. + expect( + await page.evaluate( + () => (window as FaultWindow).__webRuntimeBrowserCreationFault?.snapshot() ?? null + ) + ).toMatchObject({ createdPageId: null }) await expect .poll(() => readClientTabs(page, worktreeId), { timeout: 30_000, message: 'client split state did not settle after capability rejection' }) .toEqual(baselineClient) - expect(await readHostTabs(args.hostClient, args.repoPath)).toEqual(baselineHost) + expect(await readStableHostTabs(args.hostClient, args.repoPath)).toEqual(baselineHost) await page.screenshot({ path: args.testInfo.outputPath(`${args.topology}-browser-capability-rejected.png`), fullPage: true @@ -305,10 +309,6 @@ test('rolls back a headed-host browser when client reconciliation times out @hea testRepoPath }, testInfo) => { test.setTimeout(300_000) - writeFileSync( - path.join(testRepoPath, FIXTURE_NAME), - '

browser reconciliation fault

\n' - ) await waitForSessionReady(orcaPage) await waitForActiveWorktree(orcaPage) await ensureTerminalVisible(orcaPage) @@ -326,16 +326,12 @@ test('rolls back a headed-host browser when client reconciliation times out @hea }) }) -test('cleans up a headed-host preview when capability rejects after preflight @headful', async ({ +test('cleans up a headed-host browser when capability rejects before create @headful', async ({ electronApp, orcaPage, testRepoPath }, testInfo) => { test.setTimeout(300_000) - writeFileSync( - path.join(testRepoPath, FIXTURE_NAME), - '

browser capability fault

\n' - ) await waitForSessionReady(orcaPage) await waitForActiveWorktree(orcaPage) await ensureTerminalVisible(orcaPage) @@ -355,10 +351,6 @@ test('cleans up a headed-host preview when capability rejects after preflight @h test('keeps browser failure cleanup on a headless host', async ({ testRepoPath }, testInfo) => { test.setTimeout(300_000) - writeFileSync( - path.join(testRepoPath, FIXTURE_NAME), - '

headless capability fault

\n' - ) const host: HeadlessPairedRuntimeHost = await launchHeadlessPairedRuntimeHost() try { await host.client.call('repo.add', { path: testRepoPath, kind: 'git' }) diff --git a/tests/e2e/source-control-large-file-count.spec.ts b/tests/e2e/source-control-large-file-count.spec.ts index fb98eb9a123..28a5e7758f2 100644 --- a/tests/e2e/source-control-large-file-count.spec.ts +++ b/tests/e2e/source-control-large-file-count.spec.ts @@ -438,7 +438,9 @@ test.describe('Source Control large file count (#8013)', () => { // explicit recovery path after the underlying change count drops. removeLargeFileCountUntrackedTree(fixture.repoPath) await expect(tooManyChangesBanner).toBeVisible() - await orcaPage.getByRole('button', { name: 'Retry' }).click() + const retryButton = tooManyChangesBanner.locator('..').getByRole('button', { name: 'Retry' }) + await expect(retryButton).toBeVisible() + await retryButton.click() await expect(tooManyChangesBanner).not.toBeVisible() await expect .poll(() => diff --git a/tests/e2e/tabs.spec.ts b/tests/e2e/tabs.spec.ts index b89e47d4079..2faabafc1cc 100644 --- a/tests/e2e/tabs.spec.ts +++ b/tests/e2e/tabs.spec.ts @@ -29,7 +29,8 @@ import { getActiveTabType, getWorktreeTabs, getTabBarOrder, - ensureTerminalVisible + ensureTerminalVisible, + waitForStartupWorktreeRefresh } from './helpers/store' const SORTABLE_TAB = '[data-testid="sortable-tab"]' @@ -69,6 +70,7 @@ async function getFocusedTerminalTabId(page: Page): Promise { test.describe('Tabs', () => { test.beforeEach(async ({ orcaPage }) => { await waitForSessionReady(orcaPage) + await waitForStartupWorktreeRefresh(orcaPage) await waitForActiveWorktree(orcaPage) await ensureTerminalVisible(orcaPage) }) diff --git a/tests/e2e/worktree-active-delete-scroll-position.spec.ts b/tests/e2e/worktree-active-delete-scroll-position.spec.ts index 1b9d4bc3283..5e2f15f5b75 100644 --- a/tests/e2e/worktree-active-delete-scroll-position.spec.ts +++ b/tests/e2e/worktree-active-delete-scroll-position.spec.ts @@ -237,24 +237,10 @@ test('deleting the active scrolled worktree preserves position and closes the ro `[data-worktree-sidebar] [data-worktree-id=${JSON.stringify(belowId)}]` ) await pauseForVisualProof(orcaPage) - await target.evaluate((element) => { - const scope = element.querySelector( - '[data-worktree-context-menu-scope="worktree"]' - ) - if (!scope) { - throw new Error('Worktree context-menu scope is unavailable') - } - scope.dispatchEvent( - new MouseEvent('contextmenu', { - bubbles: true, - button: 2, - cancelable: true, - clientX: scope.getBoundingClientRect().left + 10, - clientY: scope.getBoundingClientRect().top + 10 - }) - ) - }) - const deleteItem = orcaPage.getByRole('menuitem', { name: 'Delete', exact: true }) + const contextMenuScope = target.locator('[data-worktree-context-menu-scope="worktree"]') + await expect(contextMenuScope).toBeVisible() + await contextMenuScope.click({ button: 'right' }) + const deleteItem = orcaPage.getByRole('menuitem', { name: /^Delete(?:\s|$)/ }) await expect(deleteItem).toBeVisible() await expect(deleteItem).toBeInViewport() await pauseForVisualProof(orcaPage) From 817827be5b98d593184d62b9561e8b17acf87498 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 2 Sep 2026 14:06:45 -0700 Subject: [PATCH 4/5] perf(renderer): drop react-markdown and the emoji catalog off the boot path (#18149) The sidebar pulled react-markdown, remark/rehype and DOMPurify onto the eager module graph through two static importers -- WorktreeCardMeta's hover-card notes and DashboardAgentRowMessage's inline agent preview -- and built a 3,979-key emoji shortcode catalog at module scope in both the renderer and the main process. Neither is needed before first paint. Route both markdown surfaces through one shared lazyWithRetry boundary that preloads on pointer-enter (250ms hover open delay) and on agent-row mount, with a same-box raw-text Suspense fallback so a pre-load paint cannot shift layout. Memoize the emoji catalog behind loadCatalog() so import costs nothing. Eager renderer JS: 5,569,446 B / 331 chunks -> 5,198,787 B / 325 chunks (-370,659 B, -6.7%). Emoji catalog module eval: ~19.6 ms median -> 0 ms, paid once on renderer boot and once on main boot. --- .../dashboard/DashboardAgentRowMessage.tsx | 11 +- .../components/sidebar/WorktreeCardMeta.tsx | 18 ++- .../sidebar/comment-markdown-lazy.tsx | 49 +++++++++ .../worktree-card-markdown-isolation.test.ts | 103 ++++++++++++++++++ .../workspace-emoji-shortcodes.lazy.test.ts | 30 +++++ .../src/lib/workspace-emoji-shortcodes.ts | 40 ++++--- .../emoji-shortcode-catalog.lazy.test.ts | 44 ++++++++ src/shared/emoji-shortcode-catalog.ts | 63 ++++++++--- 8 files changed, 320 insertions(+), 38 deletions(-) create mode 100644 src/renderer/src/components/sidebar/comment-markdown-lazy.tsx create mode 100644 src/renderer/src/components/sidebar/worktree-card-markdown-isolation.test.ts create mode 100644 src/renderer/src/lib/workspace-emoji-shortcodes.lazy.test.ts create mode 100644 src/shared/emoji-shortcode-catalog.lazy.test.ts diff --git a/src/renderer/src/components/dashboard/DashboardAgentRowMessage.tsx b/src/renderer/src/components/dashboard/DashboardAgentRowMessage.tsx index 6b107086523..74d36300436 100644 --- a/src/renderer/src/components/dashboard/DashboardAgentRowMessage.tsx +++ b/src/renderer/src/components/dashboard/DashboardAgentRowMessage.tsx @@ -1,5 +1,9 @@ +import { useEffect } from 'react' import { cn } from '@/lib/utils' -import CommentMarkdown from '@/components/sidebar/CommentMarkdown' +import { + CommentMarkdownAsync, + preloadCommentMarkdown +} from '@/components/sidebar/comment-markdown-lazy' import { translate } from '@/i18n/i18n' type DashboardAgentRowMessageProps = { @@ -13,6 +17,9 @@ export function DashboardAgentRowMessage({ isInterrupted, lastAssistantMessage }: DashboardAgentRowMessageProps): React.JSX.Element | null { + // These rows are the sidebar's only boot-visible markdown, so warm the chunk as + // soon as one mounts rather than waiting for text to arrive. + useEffect(preloadCommentMarkdown, []) // Why: message slot is always reserved in collapsed view so the row height // stays fixed as assistant text arrives or clears. if (!isInterrupted && !lastAssistantMessage) { @@ -38,7 +45,7 @@ export function DashboardAgentRowMessage({ ) : null} {lastAssistantMessage ? ( - - {children} + + {children} + - diff --git a/src/renderer/src/components/sidebar/comment-markdown-lazy.tsx b/src/renderer/src/components/sidebar/comment-markdown-lazy.tsx new file mode 100644 index 00000000000..81de8362505 --- /dev/null +++ b/src/renderer/src/components/sidebar/comment-markdown-lazy.tsx @@ -0,0 +1,49 @@ +import React from 'react' +import { cn } from '@/lib/utils' +import { lazyWithRetry } from '@/lib/lazy-with-retry' + +// Boot-path split: react-markdown + remark/rehype/DOMPurify is ~356 KB of JS that +// only the sidebar's two markdown surfaces pull onto the eager graph. One lazy() +// identity for both, so they share a component type and a single chunk fetch. +const LazyCommentMarkdown = lazyWithRetry(() => import('./CommentMarkdown'), { + reloadKey: 'comment-markdown' +}) + +/** Warms the chunk ahead of render so the fallback is never actually shown. */ +export function preloadCommentMarkdown(): void { + void import('./CommentMarkdown') +} + +type CommentMarkdownAsyncProps = React.ComponentProps & { + /** Extra classes for the pre-load fallback only, e.g. to mirror remark-breaks. */ + fallbackClassName?: string +} + +/** + * Renders the markdown body, falling back to the raw text in an identically + * classed box while the chunk loads — same width and wrapping constraints, so a + * paint before the chunk lands cannot shift layout. + */ +export function CommentMarkdownAsync({ + fallbackClassName, + ...props +}: CommentMarkdownAsyncProps): React.JSX.Element { + return ( + + {props.content} + + } + > + + + ) +} diff --git a/src/renderer/src/components/sidebar/worktree-card-markdown-isolation.test.ts b/src/renderer/src/components/sidebar/worktree-card-markdown-isolation.test.ts new file mode 100644 index 00000000000..28925fc89dc --- /dev/null +++ b/src/renderer/src/components/sidebar/worktree-card-markdown-isolation.test.ts @@ -0,0 +1,103 @@ +import { readFileSync, existsSync, statSync } from 'node:fs' +import { dirname, join, resolve } from 'node:path' +import { describe, expect, it } from 'vitest' + +const rendererSrc = join(__dirname, '../..') +const entry = join(rendererSrc, 'main.tsx') +const COMMENT_MARKDOWN = join(rendererSrc, 'components/sidebar/CommentMarkdown.tsx') + +function source(relativePath: string): string { + return readFileSync(join(rendererSrc, relativePath), 'utf8') +} + +const MODULE_EXTENSIONS = ['.ts', '.tsx', '.js', '.jsx'] + +function resolveImport(specifier: string, fromFile: string): string | null { + const base = specifier.startsWith('@/') + ? join(rendererSrc, specifier.slice(2)) + : specifier.startsWith('.') + ? resolve(dirname(fromFile), specifier) + : null + if (base === null) { + return null + } + for (const extension of ['', ...MODULE_EXTENSIONS]) { + const candidate = base + extension + if (existsSync(candidate) && statSync(candidate).isFile()) { + return candidate + } + } + for (const extension of MODULE_EXTENSIONS) { + const candidate = join(base, `index${extension}`) + if (existsSync(candidate) && statSync(candidate).isFile()) { + return candidate + } + } + return null +} + +// Static `from '...'` edges only; `import('...')` and `import type` do not ship +// code onto the eager graph. +const STATIC_IMPORT = + /(?:^|[\n;])\s*(?:import|export)(?:(?!\bfrom\b)[\s\S])*?\bfrom\s*['"]([^'"]+)['"]/g + +/** Walks the renderer entry's static import graph, recording how each module was reached. */ +function eagerModuleGraph(): Map { + const parents = new Map([[entry, null]]) + const queue = [entry] + while (queue.length > 0) { + const current = queue.shift() as string + const contents = readFileSync(current, 'utf8') + for (const match of contents.matchAll(STATIC_IMPORT)) { + if (/^\s*(?:import|export)\s+type\b/.test(match[0].replace(/^[\n;]/, ''))) { + continue + } + const resolved = resolveImport(match[1], current) + if (resolved === null || parents.has(resolved)) { + continue + } + parents.set(resolved, current) + queue.push(resolved) + } + } + return parents +} + +function importChain(parents: Map, module: string): string[] { + const chain: string[] = [] + let cursor: string | null | undefined = module + while (cursor) { + chain.push(cursor.slice(rendererSrc.length + 1)) + cursor = parents.get(cursor) + } + return chain.toReversed() +} + +describe('worktree card markdown performance isolation', () => { + it('keeps CommentMarkdown off the renderer boot graph entirely', () => { + const parents = eagerModuleGraph() + + // Names the offending chain when this regresses, instead of a bare boolean. + const chain = parents.has(COMMENT_MARKDOWN) ? importChain(parents, COMMENT_MARKDOWN) : [] + expect(chain).toEqual([]) + expect(parents.size).toBeGreaterThan(1000) + }) + + it('routes both sidebar markdown surfaces through the shared lazy boundary', () => { + const lazyBoundary = source('components/sidebar/comment-markdown-lazy.tsx') + expect(lazyBoundary).toContain("import('./CommentMarkdown')") + // A fallback in the same box keeps first paint from shifting layout. + expect(lazyBoundary).toContain('React.Suspense') + + for (const file of [ + 'components/sidebar/WorktreeCardMeta.tsx', + 'components/dashboard/DashboardAgentRowMessage.tsx' + ]) { + const contents = source(file) + expect(contents).not.toMatch(/^import CommentMarkdown from/m) + expect(contents).toContain('CommentMarkdownAsync') + // The chunk must be warmed before the surface renders, not on demand. + expect(contents).toContain('preloadCommentMarkdown') + } + }) +}) diff --git a/src/renderer/src/lib/workspace-emoji-shortcodes.lazy.test.ts b/src/renderer/src/lib/workspace-emoji-shortcodes.lazy.test.ts new file mode 100644 index 00000000000..eafc3e3b2b8 --- /dev/null +++ b/src/renderer/src/lib/workspace-emoji-shortcodes.lazy.test.ts @@ -0,0 +1,30 @@ +import { readFileSync } from 'node:fs' +import { join } from 'node:path' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +describe('workspace emoji shortcode index laziness', () => { + beforeEach(() => { + vi.resetModules() + }) + + it('does not build the shared catalog when the renderer index is imported', async () => { + const shortcodeIndex = await import('./workspace-emoji-shortcodes') + const catalog = await import('../../../shared/emoji-shortcode-catalog') + + expect(catalog.isEmojiShortcodeCatalogBuiltForTest()).toBe(false) + + // Cursor/regex-only paths must stay off the catalog too. + expect(shortcodeIndex.getActiveWorkspaceEmojiShortcode('hi :tad', 7)).not.toBeNull() + expect(catalog.isEmojiShortcodeCatalogBuiltForTest()).toBe(false) + + expect(shortcodeIndex.searchWorkspaceEmojiShortcodes('tada')[0]?.emoji).toBe('🎉') + expect(catalog.isEmojiShortcodeCatalogBuiltForTest()).toBe(true) + }) + + it('keeps the exact-shortcode index out of module scope', () => { + const indexSource = readFileSync(join(__dirname, 'workspace-emoji-shortcodes.ts'), 'utf8') + + expect(indexSource).not.toMatch(/^const \w+ = new Map\(/m) + expect(indexSource).not.toContain('STANDARD_EMOJI_SHORTCODE_ENTRIES') + }) +}) diff --git a/src/renderer/src/lib/workspace-emoji-shortcodes.ts b/src/renderer/src/lib/workspace-emoji-shortcodes.ts index 29e06dc74c9..a3d65c18907 100644 --- a/src/renderer/src/lib/workspace-emoji-shortcodes.ts +++ b/src/renderer/src/lib/workspace-emoji-shortcodes.ts @@ -1,5 +1,5 @@ import { - STANDARD_EMOJI_SHORTCODE_ENTRIES, + getStandardEmojiShortcodeEntries, type StandardEmojiShortcodeEntry } from '../../../shared/emoji-shortcode-catalog' @@ -16,9 +16,19 @@ export type WorkspaceEmojiReplacement = { value: string } -const EXACT_SHORTCODE = new Map( - STANDARD_EMOJI_SHORTCODE_ENTRIES.map(({ emoji, shortcode }) => [shortcode, { emoji, shortcode }]) -) +// Lazy for the same reason as the shared catalog it indexes: nothing needs it +// until a `:` shortcode is completed. +let exactShortcode: ReadonlyMap | null = null + +function exactShortcodeIndex(): ReadonlyMap { + exactShortcode ??= new Map( + getStandardEmojiShortcodeEntries().map(({ emoji, shortcode }) => [ + shortcode, + { emoji, shortcode } + ]) + ) + return exactShortcode +} // Lower tiers rank first, so `korea` surfaces `south_korea` above `dishwasher`-style incidental hits. const MATCH_TIER = { exact: 0, prefix: 1, wordStart: 2, substring: 3 } as const @@ -43,15 +53,17 @@ export function searchWorkspaceEmojiShortcodes( return [] } - const matches = STANDARD_EMOJI_SHORTCODE_ENTRIES.flatMap((entry) => { - const tier = matchTier(entry.shortcode, normalizedQuery) - return tier === null ? [] : [{ ...entry, tier }] - }).sort( - (left, right) => - left.tier - right.tier || - left.shortcode.length - right.shortcode.length || - left.shortcode.localeCompare(right.shortcode) - ) + const matches = getStandardEmojiShortcodeEntries() + .flatMap((entry) => { + const tier = matchTier(entry.shortcode, normalizedQuery) + return tier === null ? [] : [{ ...entry, tier }] + }) + .sort( + (left, right) => + left.tier - right.tier || + left.shortcode.length - right.shortcode.length || + left.shortcode.localeCompare(right.shortcode) + ) const seenEmoji = new Set() const suggestions: WorkspaceEmojiSuggestion[] = [] for (const { emoji, shortcode } of matches) { @@ -96,7 +108,7 @@ export function replaceCompletedWorkspaceEmojiShortcode( if (!match) { return null } - const suggestion = EXACT_SHORTCODE.get(match[2].toLowerCase()) + const suggestion = exactShortcodeIndex().get(match[2].toLowerCase()) if (!suggestion) { return null } diff --git a/src/shared/emoji-shortcode-catalog.lazy.test.ts b/src/shared/emoji-shortcode-catalog.lazy.test.ts new file mode 100644 index 00000000000..e838eef765e --- /dev/null +++ b/src/shared/emoji-shortcode-catalog.lazy.test.ts @@ -0,0 +1,44 @@ +import { readFileSync } from 'node:fs' +import { join } from 'node:path' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +describe('emoji shortcode catalog laziness', () => { + beforeEach(() => { + vi.resetModules() + }) + + it('does not build the catalog when the shared module is imported', async () => { + const catalog = await import('./emoji-shortcode-catalog.js') + + expect(catalog.isEmojiShortcodeCatalogBuiltForTest()).toBe(false) + + expect(catalog.getStandardEmojiShortcodeEntries().length).toBeGreaterThan(1000) + expect(catalog.isEmojiShortcodeCatalogBuiltForTest()).toBe(true) + }) + + it('builds on first use and keeps the main process off the eager path', async () => { + const catalog = await import('./emoji-shortcode-catalog.js') + + expect(catalog.replaceKnownEmojiWithShortcodes('ship \u{1F389}')).toBe('ship party ') + expect(catalog.isEmojiShortcodeCatalogBuiltForTest()).toBe(true) + }) + + it('leaves the main-process worktree namer importing only the deferred entry point', () => { + // A cross-project import would drag src/main into the shared tsconfig, so assert on source. + const worktreeLogic = readFileSync(join(__dirname, '../main/ipc/worktree-logic.ts'), 'utf8') + const catalogImport = worktreeLogic.match( + /import \{([^}]*)\} from '[^']*emoji-shortcode-catalog'/ + ) + + expect(catalogImport?.[1].trim()).toBe('replaceKnownEmojiWithShortcodes') + }) + + it('keeps the catalog build out of module scope', () => { + const sharedSource = readFileSync(join(__dirname, 'emoji-shortcode-catalog.ts'), 'utf8') + + // A module-scope `const X = ` is the regression this guards. + expect(sharedSource).not.toMatch(/^const \w+ = Object\.entries\(/m) + expect(sharedSource).not.toMatch(/^const \w+ = new (?:Map|Intl\.Segmenter)\(/m) + expect(sharedSource).toContain('function loadCatalog()') + }) +}) diff --git a/src/shared/emoji-shortcode-catalog.ts b/src/shared/emoji-shortcode-catalog.ts index 63477e2069a..b583a0c617a 100644 --- a/src/shared/emoji-shortcode-catalog.ts +++ b/src/shared/emoji-shortcode-catalog.ts @@ -8,22 +8,50 @@ export type StandardEmojiShortcodeEntry = { // Skin-tone aliases (`wave_tone3`) are ~40% of the dataset and would drown the suggestion list. const SKIN_TONE_SHORTCODE = /_tone\d(?:-\d)?$/ -const CATALOG = Object.entries(emojiShortcodes).flatMap(([hexcode, value]) => { - const shortcodes = (typeof value === 'string' ? [value] : value).filter( - (shortcode) => !SKIN_TONE_SHORTCODE.test(shortcode) - ) - return shortcodes.length > 0 ? [{ emoji: hexcodeToEmoji(hexcode), shortcodes }] : [] -}) +type EmojiShortcodeCatalog = { + entries: readonly StandardEmojiShortcodeEntry[] + primaryShortcodeByEmoji: ReadonlyMap + segmenter: Intl.Segmenter +} -export const STANDARD_EMOJI_SHORTCODE_ENTRIES: readonly StandardEmojiShortcodeEntry[] = - CATALOG.flatMap(({ emoji, shortcodes }) => shortcodes.map((shortcode) => ({ emoji, shortcode }))) +let catalog: EmojiShortcodeCatalog | null = null -const PRIMARY_SHORTCODE_BY_EMOJI = new Map( - CATALOG.map(({ emoji, shortcodes }) => [ - normalizeEmojiLookup(emoji), - primaryShortcode(shortcodes) - ]) -) +// Why lazy: this walks ~3,900 shortcodes and is only needed once a `:` is typed +// or a worktree name is sanitized, but at module scope every renderer and main +// boot paid for it. Memoized so the first caller builds it exactly once. +function loadCatalog(): EmojiShortcodeCatalog { + if (catalog) { + return catalog + } + const grouped = Object.entries(emojiShortcodes).flatMap(([hexcode, value]) => { + const shortcodes = (typeof value === 'string' ? [value] : value).filter( + (shortcode) => !SKIN_TONE_SHORTCODE.test(shortcode) + ) + return shortcodes.length > 0 ? [{ emoji: hexcodeToEmoji(hexcode), shortcodes }] : [] + }) + catalog = { + entries: grouped.flatMap(({ emoji, shortcodes }) => + shortcodes.map((shortcode) => ({ emoji, shortcode })) + ), + primaryShortcodeByEmoji: new Map( + grouped.map(({ emoji, shortcodes }) => [ + normalizeEmojiLookup(emoji), + primaryShortcode(shortcodes) + ]) + ), + segmenter: new Intl.Segmenter('en', { granularity: 'grapheme' }) + } + return catalog +} + +export function getStandardEmojiShortcodeEntries(): readonly StandardEmojiShortcodeEntry[] { + return loadCatalog().entries +} + +/** Test-only probe for the lazy-boundary guard; never branch on this in product code. */ +export function isEmojiShortcodeCatalogBuiltForTest(): boolean { + return catalog !== null +} /** * Pick the alias that reads best as a branch or directory name: skip `+1`/`-1` so the name @@ -40,11 +68,10 @@ function primaryShortcode(shortcodes: readonly string[]): string { ) } -const EMOJI_SEGMENTER = new Intl.Segmenter('en', { granularity: 'grapheme' }) - export function replaceKnownEmojiWithShortcodes(input: string): string { - return Array.from(EMOJI_SEGMENTER.segment(input), ({ segment }) => { - const shortcode = PRIMARY_SHORTCODE_BY_EMOJI.get(normalizeEmojiLookup(segment)) + const { primaryShortcodeByEmoji, segmenter } = loadCatalog() + return Array.from(segmenter.segment(input), ({ segment }) => { + const shortcode = primaryShortcodeByEmoji.get(normalizeEmojiLookup(segment)) return shortcode ? ` ${shortcode.replaceAll('_', '-')} ` : segment }).join('') } From 1d34d76f28414c774564ffc7158a515e1dc3a03a Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 2 Sep 2026 14:29:21 -0700 Subject: [PATCH 5/5] fix(i18n): add the three activity keys #18245 left out of en.json (#18250) verify:localization-catalog and verify:localization-extraction both exited 1 on main. The failure was masked: the Lint step failed first on max-lines, so every later static-analysis step was skipped. --- src/renderer/src/i18n/locales/en.json | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index b546a930830..1ffa6eaf9ce 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -15962,7 +15962,8 @@ "none": "None", "search": "Search", "showUnreadOnly": "Show unread only", - "showChildAgents": "Show child agents" + "showChildAgents": "Show child agents", + "activityOptions": "Activity options" }, "clearCompleted": { "clearedOne": "Cleared 1 completed agent", @@ -17259,7 +17260,9 @@ "dashboard": { "sidebar": { "label": "Agents", - "dashboardLabel": "Agent Dashboard" + "dashboardLabel": "Agent Dashboard", + "openActivity": "View activity", + "closeActivity": "Turn off activity view" } }, "runtimeRpc": {