From 6677ae4e5ee8d3ea3452d1f0daa7069e4871c9e3 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 30 Aug 2026 11:45:21 -0700 Subject: [PATCH] test: correct 8 stale specs surfaced by the test-detected-bugs sweep (#17434) --- ...n-tab-browser-placement-projection.test.ts | 22 +- ...tdown-checkpoint-restart-lifecycle.test.ts | 39 ++- .../worktree-manual-order-store-write.test.ts | 115 ++++++++ .../lib/browser-palette-page-entries.test.ts | 34 +++ tests/e2e/alternate-screen-fixture-script.ts | 15 + ...ternate-screen-fixture-script.unit.test.ts | 91 ++++++ ...ificial-opencode-hidden-pressure-script.ts | 7 +- .../artificial-opencode-terminal-load.spec.ts | 2 +- tests/e2e/helpers/alt-screen-frame.ts | 64 ++-- .../e2e/helpers/alt-screen-frame.unit.test.ts | 111 +++++++ .../helpers/renderer-console-forwarding.ts | 26 ++ tests/e2e/helpers/store.ts | 16 + ...d-remote-browser-link-open-routing.spec.ts | 274 +++++++++++++++--- ...inal-duplicate-pty-renderer-reveal.spec.ts | 116 ++++++-- ...terminal-hidden-tui-visual-restore.spec.ts | 104 +++---- .../e2e/terminal-hidden-view-parking.spec.ts | 22 +- .../terminal-osc8-cold-park-restore.spec.ts | 90 +++--- ...stall-renderer-checkpoint-recovery.spec.ts | 31 +- tests/e2e/worktree-card-downward-drag.spec.ts | 12 +- .../e2e/worktree-jump-palette-filter.spec.ts | 58 +++- 20 files changed, 1031 insertions(+), 218 deletions(-) create mode 100644 src/renderer/src/components/sidebar/worktree-manual-order-store-write.test.ts create mode 100644 tests/e2e/alternate-screen-fixture-script.ts create mode 100644 tests/e2e/alternate-screen-fixture-script.unit.test.ts create mode 100644 tests/e2e/helpers/alt-screen-frame.unit.test.ts create mode 100644 tests/e2e/helpers/renderer-console-forwarding.ts diff --git a/src/main/runtime/rpc/methods/session-tab-browser-placement-projection.test.ts b/src/main/runtime/rpc/methods/session-tab-browser-placement-projection.test.ts index 1337ce504d6..e5f309bcd4e 100644 --- a/src/main/runtime/rpc/methods/session-tab-browser-placement-projection.test.ts +++ b/src/main/runtime/rpc/methods/session-tab-browser-placement-projection.test.ts @@ -1,8 +1,13 @@ import { describe, expect, it } from 'vitest' -import { BROWSER_CLIENT_HOST_RUNTIME_CAPABILITY } from '../../../../shared/protocol-version' +import { + BROWSER_CLIENT_HOST_RUNTIME_CAPABILITY, + ELECTRON_REMOTE_RUNTIME_CLIENT_CAPABILITIES, + NATIVE_REMOTE_RUNTIME_CLIENT_CAPABILITIES +} from '../../../../shared/protocol-version' import type { RuntimeMobileSessionTabsResult } from '../../../../shared/runtime-types' import { assertProjectedSessionTabVisible, + clientCanObserveClientHostedBrowserPages, projectSessionTabBrowserPlacements, translateProjectedSessionTabMove } from './session-tab-browser-placement-projection' @@ -36,6 +41,21 @@ describe('projectSessionTabBrowserPlacements', () => { expect(projected.tabGroupLayout).toEqual({ type: 'leaf', groupId: 'group-terminal' }) }) + // Why named here: a CLI socket sends no capabilities at all, so anything asking it for browser + // tabs — an e2e oracle included — is blind to every client-placed page by design. + it('hides client-placed pages from a native peer and from a caller with no capabilities', () => { + expect( + clientCanObserveClientHostedBrowserPages(NATIVE_REMOTE_RUNTIME_CLIENT_CAPABILITIES) + ).toBe(false) + expect( + clientCanObserveClientHostedBrowserPages(ELECTRON_REMOTE_RUNTIME_CLIENT_CAPABILITIES) + ).toBe(true) + expect(clientCanObserveClientHostedBrowserPages(undefined)).toBe(false) + expect(projectSessionTabBrowserPlacements(makeSnapshot(), undefined).tabs).toEqual([ + expect.objectContaining({ id: 'terminal-leaf' }) + ]) + }) + it('preserves hidden raw slots while translating an old-client reorder', () => { const raw = mixedGroupSnapshot() const projected = projectSessionTabBrowserPlacements(raw, []) diff --git a/src/renderer/src/app-shell/shutdown-checkpoint-restart-lifecycle.test.ts b/src/renderer/src/app-shell/shutdown-checkpoint-restart-lifecycle.test.ts index 55f5c0e51bf..8253932f376 100644 --- a/src/renderer/src/app-shell/shutdown-checkpoint-restart-lifecycle.test.ts +++ b/src/renderer/src/app-shell/shutdown-checkpoint-restart-lifecycle.test.ts @@ -19,7 +19,10 @@ import { isIntentionalAppRestartInProgress, registerUpdaterBeforeUnloadBypass } from '../lib/updater-beforeunload' -import { createShutdownCheckpointPersist } from './shutdown-checkpoint-persist' +import { + createShutdownCheckpointPersist, + type ShutdownCheckpointPersistDeps +} from './shutdown-checkpoint-persist' type LifecycleHarness = { cleanup: () => void @@ -27,9 +30,14 @@ type LifecycleHarness = { stageBeforeUnloadSync: ReturnType } +type LifecycleHarnessOverrides = Partial< + Pick +> + function createLifecycleHarness( startedEventName: string, - abortedEventName: string + abortedEventName: string, + overrides: LifecycleHarnessOverrides = {} ): LifecycleHarness { const stageBeforeUnloadSync = vi.fn((args: { sessions: unknown[] }) => { if (args.sessions.length > 0) { @@ -44,7 +52,8 @@ function createLifecycleHarness( buildUiPatch: () => ({ activeView: 'workspace' }) as never, hasDirtyOpenFiles: () => false, isDegradableShutdownInProgress: isIntentionalAppRestartInProgress, - stageBeforeUnloadSync + stageBeforeUnloadSync, + ...overrides }) const guard = createShutdownCheckpointGuard(persist.run, persist.abandonAttempt) const checkpoint = createShutdownCheckpointBeforeUnloadHandler(guard) @@ -130,4 +139,28 @@ describe('shutdown checkpoint restart lifecycle', () => { expect(harness.stageBeforeUnloadSync).toHaveBeenCalledTimes(2) }) + + // Mirrors the e2e fixture in tests/e2e/update-install-renderer-checkpoint-recovery.spec.ts, + // which asserted the pre-STA-5505 bare message long after the cause suffix landed (STA-5668). + it('names the snapshot-build cause when dirty drafts block the checkpoint', async () => { + const snapshotFailure = "Cannot read properties of null (reading 'toLowerCase')" + vi.spyOn(console, 'error').mockImplementation(() => {}) + const harness = createLifecycleHarness( + ORCA_UPDATER_QUIT_AND_INSTALL_STARTED_EVENT, + ORCA_UPDATER_QUIT_AND_INSTALL_ABORTED_EVENT, + { + buildSessionSnapshots: () => { + throw new Error(snapshotFailure) + }, + hasDirtyOpenFiles: () => true + } + ) + cleanupFns.push(harness.cleanup) + + await expect(harness.prepare()).rejects.toThrow( + new Error(`Renderer shutdown checkpoint was not completed: ${snapshotFailure}`) + ) + // Dirty drafts must block before any durable-only degrade stages over them. + expect(harness.stageBeforeUnloadSync).not.toHaveBeenCalled() + }) }) diff --git a/src/renderer/src/components/sidebar/worktree-manual-order-store-write.test.ts b/src/renderer/src/components/sidebar/worktree-manual-order-store-write.test.ts new file mode 100644 index 00000000000..6789c95df1b --- /dev/null +++ b/src/renderer/src/components/sidebar/worktree-manual-order-store-write.test.ts @@ -0,0 +1,115 @@ +// @vitest-environment happy-dom + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { act, cleanup, renderHook } from '@testing-library/react' +import { useAppStore } from '@/store' +import type { Worktree } from '../../../../shared/worktree/types' +import { makeWorktree } from '../worktree-jump-palette-test-fixtures' +import { buildWorktreeManualOrderCatalog } from './worktree-manual-order-catalog' +import { useWorktreeStatusMutations } from './worktree-list/drag/use-status-mutations' + +/** + * The sidebar drop only reorders if the payload `reorderWorktrees` builds is the + * one `updateWorktreesMeta` consumes. The e2e that covered this replaced the store + * action with a fake, so a payload-shape change (#16691, Map -> batch array) showed + * up as a red drag spec instead of a red contract. This runs the real action. + */ + +const initialState = useAppStore.getInitialState() +const REPO_ID = 'repo-manual-order' +const GROUP_KEY = `repo:${REPO_ID}` + +const updateMeta = vi.fn().mockResolvedValue(undefined) + +function seedManualOrderedRows(count: number): Worktree[] { + const worktrees = Array.from({ length: count }, (_, index) => + // The store routes a metadata write by the repo id embedded in the worktree id. + makeWorktree(`${REPO_ID}::manual-${String(index).padStart(2, '0')}`, `Manual ${index}`, { + repoId: REPO_ID, + manualOrder: 100_000 - index + }) + ) + useAppStore.setState({ + sortBy: 'smart', + worktreesByRepo: { [REPO_ID]: worktrees } + }) + return worktrees +} + +function renderReorder(worktrees: readonly Worktree[]) { + return renderHook(() => + useWorktreeStatusMutations({ + worktreeMap: new Map(worktrees.map((worktree) => [worktree.id, worktree])), + manualOrderCatalog: buildWorktreeManualOrderCatalog({ + worktrees, + folderWorkspaces: [] + }), + workspaceStatuses: [], + sortBy: 'smart' + }) + ).result +} + +function manualOrderedIds(): readonly string[] { + return buildWorktreeManualOrderCatalog({ + worktrees: useAppStore.getState().worktreesByRepo[REPO_ID] ?? [], + folderWorkspaces: [] + }).orderedIds +} + +describe('sidebar manual-order drop', () => { + beforeEach(() => { + useAppStore.setState(initialState, true) + updateMeta.mockClear() + Object.assign(window, { api: { worktrees: { updateMeta } } }) + }) + + afterEach(() => { + cleanup() + useAppStore.setState(initialState, true) + }) + + it('moves the dragged row past its neighbor in the store', async () => { + const worktrees = seedManualOrderedRows(3) + const ids = worktrees.map((worktree) => worktree.id) + const reorder = renderReorder(worktrees) + + await act(async () => { + reorder.current.reorderWorktrees({ + groups: [{ key: GROUP_KEY, worktreeIds: ids }], + sourceGroupKey: GROUP_KEY, + draggedIds: [ids[0]!], + dropIndex: 2 + }) + }) + + expect(manualOrderedIds()).toEqual([ids[1], ids[0], ids[2]]) + expect(useAppStore.getState().sortBy).toBe('manual') + expect(updateMeta).toHaveBeenCalledWith({ + worktreeId: ids[0], + executionHostId: 'local', + updates: { manualOrder: expect.any(Number) } + }) + }) + + it('leaves the order alone when the drop lands where the row already is', async () => { + const worktrees = seedManualOrderedRows(3) + const ids = worktrees.map((worktree) => worktree.id) + const reorder = renderReorder(worktrees) + const epochBeforeDrop = useAppStore.getState().sortEpoch + + await act(async () => { + reorder.current.reorderWorktrees({ + groups: [{ key: GROUP_KEY, worktreeIds: ids }], + sourceGroupKey: GROUP_KEY, + draggedIds: [ids[0]!], + dropIndex: 1 + }) + }) + + expect(manualOrderedIds()).toEqual(ids) + expect(useAppStore.getState().sortBy).toBe('smart') + expect(useAppStore.getState().sortEpoch).toBe(epochBeforeDrop) + expect(updateMeta).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/lib/browser-palette-page-entries.test.ts b/src/renderer/src/lib/browser-palette-page-entries.test.ts index 05cb8d45f3f..c05a29a40bd 100644 --- a/src/renderer/src/lib/browser-palette-page-entries.test.ts +++ b/src/renderer/src/lib/browser-palette-page-entries.test.ts @@ -210,6 +210,40 @@ describe('buildSearchableBrowserPages', () => { ]) }) + it('re-hosts a same-id page entry when the sibling row is missing from the catalog', () => { + // Why: host qualification is gated on both same-id rows being present. With one reaped, a + // local-stamped tab still renders but carries the surviving row's host — so a wrong-host + // Cmd-J activation means the catalog lost a row, not that host qualification regressed. + // This characterizes today's fallback, it does not bless it: overriding a tab's own 'local' + // stamp may be the wrong answer, and changing it is tracked as the unified-tab-host-ownership + // follow-up. Update this expectation with that change rather than treating it as a contract. + const sharedId = 'repo-shared::/workspace' + const remote = makeWorktree({ id: sharedId, hostId: 'runtime:host-b' }) + const entries = buildSearchableBrowserPages({ + worktrees: [remote], + repoMap, + worktreeOrder: new Map([[getWorktreeHostIdentity(remote), 0]]), + browserTabsByWorktree: { + [sharedId]: [ + makeWorkspace({ id: 'ws-local', worktreeId: sharedId, activePageId: 'page-local' }) + ] + }, + browserPagesByWorkspace: { + 'ws-local': [makePage({ id: 'page-local', workspaceId: 'ws-local', worktreeId: sharedId })] + }, + unifiedTabsByWorktree: { + [sharedId]: [browserUnifiedTab('tab-local', 'ws-local', sharedId, 'local')] + }, + activeBrowserTabId: null, + activeWorktreeId: null, + activeTabType: 'terminal' + }) + + expect(entries.map((entry) => [entry.page.id, entry.executionHostId])).toEqual([ + ['page-local', 'runtime:host-b'] + ]) + }) + it('does not route one ambiguous legacy browser bucket to both hosts', () => { const sharedId = 'repo-shared::/workspace' const workspace = makeWorkspace({ worktreeId: sharedId }) diff --git a/tests/e2e/alternate-screen-fixture-script.ts b/tests/e2e/alternate-screen-fixture-script.ts new file mode 100644 index 00000000000..f6a62247e07 --- /dev/null +++ b/tests/e2e/alternate-screen-fixture-script.ts @@ -0,0 +1,15 @@ +// Why an alternate-screen fixture must outlive the assertions that read it: +// main's dead-TUI recovery barrier (src/main/daemon/terminal-shell-recovery-barrier.ts) +// injects `\x1b[?1049l` as soon as a shell prompt proves an alternate-screen owner +// exited without its own cleanup, so a fixture that paints and exits has its frames +// discarded before a spec can observe them. Specs Ctrl-C them once done reading. + +/** Node statement that keeps a fixture — and so its alternate screen — the live PTY foreground. */ +export const HOLD_ALTERNATE_SCREEN_OPEN = 'setInterval(() => {}, 1000)' + +/** Fixture program: paint `payload`, then stay the live alternate-screen owner. */ +export function alternateScreenFixtureScript(payload: string, delayMs = 0): string { + const write = `process.stdout.write(${JSON.stringify(payload)})` + const paint = delayMs > 0 ? `setTimeout(() => ${write}, ${delayMs})` : write + return `${paint}\n${HOLD_ALTERNATE_SCREEN_OPEN}\n` +} diff --git a/tests/e2e/alternate-screen-fixture-script.unit.test.ts b/tests/e2e/alternate-screen-fixture-script.unit.test.ts new file mode 100644 index 00000000000..56718af3620 --- /dev/null +++ b/tests/e2e/alternate-screen-fixture-script.unit.test.ts @@ -0,0 +1,91 @@ +import { readFileSync } from 'node:fs' +import { describe, expect, it } from 'vitest' +import { + HOLD_ALTERNATE_SCREEN_OPEN, + alternateScreenFixtureScript +} from './alternate-screen-fixture-script' +import { + type HiddenPressureOutputMode, + pressureOutputScript +} from './artificial-opencode-hidden-pressure-script' + +// The escape as it appears in generated source, where it is still a JS string escape. +const ALTERNATE_SCREEN_ENTER_SOURCE = '\\x1b[?1049h' + +const PRESSURE_MODES: HiddenPressureOutputMode[] = ['tui', 'plain', 'title', 'latin', 'rich-model'] + +describe('alternateScreenFixtureScript', () => { + it('holds the painting process open so the alternate screen survives the assertions', () => { + const source = alternateScreenFixtureScript('\x1b[?1049hFRAME') + + expect(source).toContain(HOLD_ALTERNATE_SCREEN_OPEN) + expect(source).toContain('process.stdout.write("\\u001b[?1049hFRAME")') + expect(source).not.toContain('setTimeout') + }) + + it('defers the paint by the requested delay and still holds open', () => { + const source = alternateScreenFixtureScript('FRAME', 750) + + expect(source).toContain('setTimeout(() => process.stdout.write("FRAME"), 750)') + expect(source).toContain(HOLD_ALTERNATE_SCREEN_OPEN) + }) +}) + +describe('hidden pressure fixture', () => { + // Ratchet: entering the alternate screen and exiting is the dead-TUI shape the + // recovery barrier deliberately dismisses, which silently emptied these panes. + it.each(PRESSURE_MODES)( + 'keeps mode %s alive exactly when it enters the alternate screen', + (mode) => { + const source = pressureOutputScript('run-id', mode) + + expect(source.includes(HOLD_ALTERNATE_SCREEN_OPEN)).toBe( + source.includes(ALTERNATE_SCREEN_ENTER_SOURCE) + ) + } + ) + + it('holds the rich-model alternate screen open past its done marker', () => { + const source = pressureOutputScript('run-id', 'rich-model') + + expect(source).toContain(ALTERNATE_SCREEN_ENTER_SOURCE) + expect(source.indexOf(HOLD_ALTERNATE_SCREEN_OPEN)).toBeGreaterThan( + source.indexOf('OPENCODE_PRESSURE_DONE_') + ) + }) +}) + +// Ratchet: the builder is only worth anything while the specs still route through it, +// and a fixture hand-rolled back to paint-and-exit would surface days later in a +// scheduled Electron run — the detection channel that produced this ticket. +describe('spec alternate-screen fixtures', () => { + const SPEC_FIXTURE_BUILDERS: [spec: string, builder: string][] = [ + ['terminal-hidden-view-parking.spec.ts', 'writeParkedFrameScript'], + ['terminal-hidden-view-parking.spec.ts', 'writeCycleReferenceScript'], + ['terminal-hidden-tui-visual-restore.spec.ts', 'writeHiddenFrameScript'] + ] + + function readSpec(spec: string): string { + return readFileSync(new URL(spec, import.meta.url), 'utf8') + } + + function topLevelFunctionBody(source: string, name: string): string { + const start = source.indexOf(`function ${name}(`) + expect(start, `${name} was renamed or removed; re-point this ratchet`).toBeGreaterThan(-1) + // Why '\n}' and a bound: '\n}\n' misses CRLF checkouts, and an unresolved indexOf slices to EOF, + // letting a later builder call in the same file satisfy the assertion vacuously. + const end = source.indexOf('\n}', start) + expect(end, `${name} has no closing brace; re-point this ratchet`).toBeGreaterThan(start) + return source.slice(start, end) + } + + it.each(SPEC_FIXTURE_BUILDERS)('%s writes %s through the shared builder', (spec, builder) => { + expect(topLevelFunctionBody(readSpec(spec), builder)).toContain('alternateScreenFixtureScript(') + }) + + it('the OSC 8 spec stages its link fixture through the shared builder', () => { + expect(readSpec('terminal-osc8-cold-park-restore.spec.ts')).toContain( + 'stageNodeScriptForTerminal(alternateScreenFixtureScript(' + ) + }) +}) diff --git a/tests/e2e/artificial-opencode-hidden-pressure-script.ts b/tests/e2e/artificial-opencode-hidden-pressure-script.ts index 1eb94b3de89..2fae1387027 100644 --- a/tests/e2e/artificial-opencode-hidden-pressure-script.ts +++ b/tests/e2e/artificial-opencode-hidden-pressure-script.ts @@ -1,5 +1,6 @@ import { mkdirSync, writeFileSync } from 'node:fs' import path from 'node:path' +import { HOLD_ALTERNATE_SCREEN_OPEN } from './alternate-screen-fixture-script' export type HiddenPressureOutputMode = 'tui' | 'plain' | 'title' | 'latin' | 'rich-model' @@ -16,6 +17,10 @@ export function pressureOutputScript(runId: string, mode: HiddenPressureOutputMo : mode === 'rich-model' ? "'\\x1b[?2026h\\x1b[?1049h\\x1b[2J\\x1b[H\\x1b[?25l\\x1b[2;36m╭────────────────────────────────────────╮\\x1b[0m\\r\\n\\x1b[2;36m│ rich model pane=' + paneIndex + ' frame=' + frame + ' 😀 ███░ │\\x1b[0m\\r\\n\\x1b[2;36m│ ' + chunkBody + ' │\\x1b[0m\\r\\n\\x1b[2;36m╰────────────────────────────────────────╯\\x1b[0m\\x1b[6;4H\\x1b[?25h\\x1b[?2026l\\n'" : "'\\x1b[?2026h\\x1b[1;1Hpressure pane=' + paneIndex + ' frame=' + frame + ' ' + chunkBody + '\\x1b[?2026l\\n'" + // Why only rich-model: it is the one mode that enters the alternate screen, and an + // alt-screen owner that exits is exactly the dead TUI main's recovery barrier + // discards — frames and done marker with it. Scenario cleanup Ctrl-Cs the pane. + const holdOpen = mode === 'rich-model' ? `\n${HOLD_ALTERNATE_SCREEN_OPEN}` : '' return ` const paneIndex = process.argv[2] ?? '0' const targetChars = Number(process.argv[3] ?? '0') @@ -38,7 +43,7 @@ function writeMore() { } process.stdout.write('${donePrefix}OPENCODE_PRESSURE_DONE_${runId}_' + paneIndex + '\\n') } -setTimeout(writeMore, Number.isFinite(delayMs) && delayMs > 0 ? delayMs : 0) +setTimeout(writeMore, Number.isFinite(delayMs) && delayMs > 0 ? delayMs : 0)${holdOpen} ` } diff --git a/tests/e2e/artificial-opencode-terminal-load.spec.ts b/tests/e2e/artificial-opencode-terminal-load.spec.ts index 63780c2ae64..bf7b7d2c7db 100644 --- a/tests/e2e/artificial-opencode-terminal-load.spec.ts +++ b/tests/e2e/artificial-opencode-terminal-load.spec.ts @@ -447,7 +447,7 @@ async function measureCrossWorkspaceTypingDuringHiddenLoad({ scheduler, mainPressure ) - expect(scheduler?.rendererDroppedBacklogs ?? 0).toBe(0) + expect(scheduler?.droppedBacklogCount ?? Number.POSITIVE_INFINITY).toBe(0) expect(measurement.medianLatencyMs).toBeLessThan(MAX_MEDIAN_KEY_LATENCY_MS) expect(measurement.worstLatencyMs).toBeLessThan(MAX_WORST_KEY_LATENCY_UNDER_LOAD_MS) expect(measurement.maxTimerDriftMs).toBeLessThan(MAX_TIMER_DRIFT_UNDER_LOAD_MS) diff --git a/tests/e2e/helpers/alt-screen-frame.ts b/tests/e2e/helpers/alt-screen-frame.ts index 6292b0c84e1..7ef3b334652 100644 --- a/tests/e2e/helpers/alt-screen-frame.ts +++ b/tests/e2e/helpers/alt-screen-frame.ts @@ -17,6 +17,47 @@ export function buildAltScreenFrame(marker: string, frame: number): string { ].join('\r\n') } +export type ActiveScreen = { + bufferType: 'normal' | 'alternate' + rows: string[] +} + +// Why: what the pane shows is the active buffer's viewport. `serializeAddon.serialize()` +// dumps the whole normal buffer (scrollback included) before the alt frame, so a stale +// marker there is indistinguishable from the live one (STA-5208). Null on a missing pane +// because callers poll this and `expect.poll` aborts on a generator throw. +export async function readActiveScreen(page: Page, tabId: string): Promise { + return page.evaluate( + ({ tabId }) => { + const manager = window.__paneManagers?.get(tabId) + const pane = manager?.getActivePane?.() ?? manager?.getPanes?.()[0] ?? null + if (!pane) { + return null + } + const buffer = pane.terminal.buffer.active + const rows: string[] = [] + for (let row = 0; row < pane.terminal.rows; row += 1) { + rows.push(buffer.getLine(buffer.viewportY + row)?.translateToString(true) ?? '') + } + return { bufferType: buffer.type, rows } + }, + { tabId } + ) +} + +// Why the highest match rather than the first: a repaint can leave an older marker line +// beside the live one, and only the newest frame says which paint landed last. +export function findMarkerFrame(text: string, marker: string): number | null { + // marker is a literal, so escape it rather than letting `[`/`.`/`+` act as regex syntax. + const pattern = new RegExp(`${marker.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')} frame (\\d+)`, 'g') + let latest: number | null = null + for (const match of text.matchAll(pattern)) { + const frame = Number(match[1]) + latest = latest === null ? frame : Math.max(latest, frame) + } + return latest +} + // Why: the live-write and the reveal restore paint the same layout, so the frame // number is the only thing on screen that says which of the two landed last. export async function readRenderedAltScreenFrame( @@ -24,27 +65,8 @@ export async function readRenderedAltScreenFrame( tabId: string, marker: string ): Promise { - return page.evaluate( - ({ tabId, marker }) => { - const manager = window.__paneManagers?.get(tabId) - const pane = manager?.getActivePane?.() ?? manager?.getPanes?.()[0] - if (!pane) { - throw new Error(`No terminal pane for tab ${tabId}`) - } - // marker is a literal, so escape it rather than letting `[`/`.`/`+` act as regex syntax. - const pattern = new RegExp(`${marker.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')} frame (\\d{3})`) - const buffer = pane.terminal.buffer.active - for (let row = 0; row < pane.terminal.rows; row += 1) { - const line = buffer.getLine(buffer.viewportY + row)?.translateToString(true) ?? '' - const match = pattern.exec(line) - if (match) { - return Number(match[1]) - } - } - return null - }, - { tabId, marker } - ) + const screen = await readActiveScreen(page, tabId) + return screen ? findMarkerFrame(screen.rows.join('\n'), marker) : null } export function describeAltScreenRenderPath( diff --git a/tests/e2e/helpers/alt-screen-frame.unit.test.ts b/tests/e2e/helpers/alt-screen-frame.unit.test.ts new file mode 100644 index 00000000000..1cab97f334c --- /dev/null +++ b/tests/e2e/helpers/alt-screen-frame.unit.test.ts @@ -0,0 +1,111 @@ +// STA-5208: the duplicate-PTY reveal oracle parsed `serializeAddon.serialize()`, which +// concatenates the whole normal buffer (scrollback included) ahead of the alt frame, so a +// stale marker left behind by the pre-hide paint was read as the revealed pane's current +// frame. These pin the replacement oracle: read the frame off the active buffer's viewport. +import '../../../src/main/daemon/xterm-env-polyfill' +import { describe, expect, it } from 'vitest' +import { Terminal } from '@xterm/headless' +import { SerializeAddon } from '@xterm/addon-serialize' +import type { Page } from '@stablyai/playwright-test' +import { findMarkerFrame, readActiveScreen, readRenderedAltScreenFrame } from './alt-screen-frame' + +const MARKER = 'DUPLICATE_PTY_REVEAL_TEST' +const TAB_ID = 'tab-under-test' + +type Harness = { page: Page; terminal: Terminal; serialize: () => string } + +// Matches the streaming TUI fixture in terminal-duplicate-pty-renderer-reveal.spec.ts. +function frameLine(frame: number): string { + return `${MARKER} frame ${String(frame).padStart(6, '0')}` +} + +function write(terminal: Terminal, data: string): Promise { + return new Promise((resolve) => terminal.write(data, () => resolve())) +} + +// Runs the helper's real in-page closure against a headless terminal so the test covers +// the code the browser executes rather than a copy of it. +function createHarness(rows: number): Harness { + const terminal = new Terminal({ cols: 80, rows, scrollback: 100, allowProposedApi: true }) + const serializeAddon = new SerializeAddon() + terminal.loadAddon(serializeAddon) + const pane = { terminal, serializeAddon } + ;(globalThis as Record).__paneManagers = new Map([ + [TAB_ID, { getPanes: () => [pane] }] + ]) + const page = { + evaluate: (fn: (arg: Arg) => Result, arg: Arg): Promise => + Promise.resolve(fn(arg)) + } as unknown as Page + return { page, terminal, serialize: () => serializeAddon.serialize() } +} + +// The oracle this replaces: a positional scan over every buffer serialize() emits. +function parseSerializedFrame(content: string, pick: 'first' | 'last'): number | null { + const prefix = `${MARKER} frame ` + const start = pick === 'first' ? content.indexOf(prefix) : content.lastIndexOf(prefix) + if (start < 0) { + return null + } + const digits = content.slice(start + prefix.length).match(/^\d+/)?.[0] + return digits ? Number(digits) : null +} + +describe('readRenderedAltScreenFrame', () => { + it('reads the live alt frame, not the stale copy left in the normal buffer', async () => { + const harness = createHarness(8) + await write(harness.terminal, `${frameLine(390)}\r\n`) + await write(harness.terminal, `\x1b[?1049h\x1b[H${frameLine(400)}\x1b[J`) + + const serialized = harness.serialize() + expect(parseSerializedFrame(serialized, 'first')).toBe(390) + expect(parseSerializedFrame(serialized, 'last')).toBe(400) + + await expect(readActiveScreen(harness.page, TAB_ID)).resolves.toMatchObject({ + bufferType: 'alternate' + }) + await expect(readRenderedAltScreenFrame(harness.page, TAB_ID, MARKER)).resolves.toBe(400) + }) + + it('reports no frame when the freshest marker scrolled off the visible rows', async () => { + const harness = createHarness(8) + await write(harness.terminal, `${frameLine(400)}\r\n`) + for (let row = 0; row < 12; row += 1) { + await write(harness.terminal, `filler row ${row}\r\n`) + } + + // serialize() still contains the marker from scrollback, so it cannot see that nothing + // correct is on screen; the viewport read can. + expect(parseSerializedFrame(harness.serialize(), 'last')).toBe(400) + await expect(readActiveScreen(harness.page, TAB_ID)).resolves.toMatchObject({ + bufferType: 'normal' + }) + await expect(readRenderedAltScreenFrame(harness.page, TAB_ID, MARKER)).resolves.toBeNull() + }) + + // expect.poll aborts on a generator throw, so a pane mid-remount has to read as + // "not converged yet" rather than ending the poll. + it('returns null when the tab has no pane', async () => { + const harness = createHarness(8) + await expect(readActiveScreen(harness.page, 'tab-without-pane')).resolves.toBeNull() + await expect( + readRenderedAltScreenFrame(harness.page, 'tab-without-pane', MARKER) + ).resolves.toBeNull() + }) +}) + +describe('findMarkerFrame', () => { + it('reads frame numbers of any width', () => { + expect(findMarkerFrame(`${MARKER} frame 000400`, MARKER)).toBe(400) + expect(findMarkerFrame(`| ${MARKER} frame 024 |`, MARKER)).toBe(24) + }) + + it('takes the highest frame when several are on screen', () => { + expect(findMarkerFrame([frameLine(400), frameLine(390)].join('\n'), MARKER)).toBe(400) + }) + + it('treats the marker as a literal', () => { + expect(findMarkerFrame('A[B frame 7', 'A[B')).toBe(7) + expect(findMarkerFrame('AxB frame 7', 'A[B')).toBeNull() + }) +}) diff --git a/tests/e2e/helpers/renderer-console-forwarding.ts b/tests/e2e/helpers/renderer-console-forwarding.ts new file mode 100644 index 00000000000..13277597d6b --- /dev/null +++ b/tests/e2e/helpers/renderer-console-forwarding.ts @@ -0,0 +1,26 @@ +import type { Page, TestInfo } from '@stablyai/playwright-test' + +/** + * Renderer-side counterpart of `forwardElectronProcessLogs`, sharing its + * `ORCA_E2E_FORWARD_APP_LOGS` gate. + * + * Why: a contained render crash only ever reaches the renderer console + * (`RecoverableRenderErrorBoundary` logs the error plus its component stack + * there), so without this a boundary failure leaves nothing but a screenshot of + * the dialog and the stack that would localize the first bad render is lost. + */ +export function forwardRendererConsole(page: Page, testInfo: TestInfo): void { + if (process.env.ORCA_E2E_FORWARD_APP_LOGS !== '1') { + return + } + + const prefix = `[renderer:${testInfo.title}]` + page.on('console', (message) => { + if (message.type() === 'error' || message.type() === 'warning') { + console.error(`${prefix} ${message.type()}: ${message.text()}`) + } + }) + page.on('pageerror', (error) => { + console.error(`${prefix} pageerror: ${error.stack ?? error.message}`) + }) +} diff --git a/tests/e2e/helpers/store.ts b/tests/e2e/helpers/store.ts index e11c3921f82..dad165494d4 100644 --- a/tests/e2e/helpers/store.ts +++ b/tests/e2e/helpers/store.ts @@ -154,6 +154,22 @@ export async function waitForSessionReady(page: Page, timeoutMs = 30_000): Promi .toBe(true) } +/** + * Wait until the deferred startup worktree scan has completed. + * + * Why: hydration fires an unawaited full catalog refresh after + * `workspaceSessionReady`; a fixture seeded before it lands is silently + * overwritten when it does. + */ +export async function waitForStartupWorktreeRefresh(page: Page, timeoutMs = 60_000): Promise { + await expect + .poll(async () => getStoreState(page, 'startupWorktreeRefreshCompleted'), { + timeout: timeoutMs, + message: 'startupWorktreeRefreshCompleted did not become true' + }) + .toBe(true) +} + /** Wait until a worktree is active and return its ID. */ export async function waitForActiveWorktree(page: Page, timeoutMs = 30_000): Promise { let activeWorktreeId: string | null = null diff --git a/tests/e2e/paired-remote-browser-link-open-routing.spec.ts b/tests/e2e/paired-remote-browser-link-open-routing.spec.ts index 6da4965703e..695d39c251b 100644 --- a/tests/e2e/paired-remote-browser-link-open-routing.spec.ts +++ b/tests/e2e/paired-remote-browser-link-open-routing.spec.ts @@ -1,19 +1,26 @@ import { createServer, type IncomingMessage, type Server, type ServerResponse } from 'node:http' import type { AddressInfo } from 'node:net' import type { Page } from '@stablyai/playwright-test' +import { parseBrowserNetworkExecutionHostKey } from '../../src/main/browser/browser-network-execution-route' import { LOCAL_EXECUTION_HOST_ID } from '../../src/shared/execution-host' +import { readOwnedPageUrls } from './helpers/client-hosted-browser-observer' import { launchHeadlessPairedRuntimeHost, type HeadlessPairedRuntimeHost } from './helpers/headless-paired-runtime-host' +import { readHostBrowserPageUrls } from './helpers/host-session-tabs' import { expect, test } from './helpers/orca-app' import { launchPairedElectronClient, type PairedElectronClient } from './helpers/paired-electron-client' -// The link is a dev-server URL, so a client-local fallback would silently load a *different -// machine's* server while looking successful. +// The link is a dev-server URL on the pane runtime's network, so a client-local fallback would +// silently load a *different machine's* server. Which machine renders the pixels no longer answers +// that: under client-hosted placement the guest paints on this desktop while its network is still +// pinned to the host at creation. So each act below pins the placement it was written for and reads +// the host's own record — the page row's placement and executionHostKey — instead of inferring +// routing from where a appeared. const PANE_PATH = '/remote-pane' const LINK_PATH = '/remote-link-target' @@ -77,8 +84,14 @@ async function startLinkFixtureServer(): Promise { } } -/** Asked over the host's own connection, not proxied through the client under test. */ -async function readHostBrowserUrls( +/** + * The host's session-tab view, asked over the host's own CLI socket rather than proxied through the + * client under test. + * + * Server-placed pages only: this socket advertises no `BROWSER_CLIENT_HOST_RUNTIME_CAPABILITY`, so + * the host strips every client-placed page from the snapshot before answering. + */ +async function readHostServerPlacedBrowserUrls( host: HeadlessPairedRuntimeHost, worktreeId: string ): Promise { @@ -90,7 +103,57 @@ async function readHostBrowserUrls( return response.result.tabs.filter((tab) => tab.type === 'browser').map((tab) => tab.url ?? '') } -/** A remote pane is a screencast image; a means the page really loaded on this machine. */ +type HostBrowserRow = { + executionHostKey: string | null + placementKind: string | null + url: string +} + +/** + * The host's rows for one URL, asked through the paired client's connection. + * + * Why through the client: an Electron peer advertises the client-host capability, so the host + * answers it with client-placed pages intact and with the placement and network pin it minted at + * creation. The host still authors every field; the client is only the transport. + */ +async function readHostBrowserRows( + page: Page, + environmentId: string, + worktreeId: string, + urlPrefix: string +): Promise { + return page.evaluate( + async ({ environmentId, urlPrefix, worktreeId }) => { + const response = await window.api.runtimeEnvironments.call({ + selector: environmentId, + method: 'session.tabs.list', + params: { worktree: `id:${worktreeId}` }, + timeoutMs: 15_000 + }) + if (!response.ok) { + throw new Error('host session tab inventory unavailable') + } + const { tabs } = response.result as { + tabs: { + type: string + url?: string + executionHostKey?: string + placement?: { kind: string } + }[] + } + return tabs + .filter((tab) => tab.type === 'browser' && (tab.url ?? '').startsWith(urlPrefix)) + .map((tab) => ({ + executionHostKey: tab.executionHostKey ?? null, + placementKind: tab.placement?.kind ?? null, + url: tab.url ?? '' + })) + }, + { environmentId, urlPrefix, worktreeId } + ) +} + +/** Under server placement the client renders nothing itself, so any is a local fallback. */ async function readLocalBrowserViewUrls(page: Page): Promise { return page.evaluate(() => Array.from(document.querySelectorAll('webview')).map( @@ -117,17 +180,22 @@ async function findMirroredPage( page: Page, worktreeId: string, url: string -): Promise<{ handleEnvironmentId: string | null; pageId: string } | null> { +): Promise<{ + handleEnvironmentId: string | null + pageId: string + placementKind: string | null +} | null> { return page.evaluate( ({ url, worktreeId }) => { const state = window.__store?.getState() for (const workspace of state?.browserTabsByWorktree[worktreeId] ?? []) { for (const browserPage of state?.browserPagesByWorkspace[workspace.id] ?? []) { if (browserPage.url.startsWith(url)) { + const handle = state?.remoteBrowserPageHandlesByPageId[browserPage.id] return { - handleEnvironmentId: - state?.remoteBrowserPageHandlesByPageId[browserPage.id]?.environmentId ?? null, - pageId: browserPage.id + handleEnvironmentId: handle?.environmentId ?? null, + pageId: browserPage.id, + placementKind: handle?.placement?.kind ?? null } } } @@ -149,13 +217,50 @@ async function focusMirroredPage(page: Page, worktreeId: string, pageId: string) ) } +/** Placement is a user setting whose default has already flipped once, so every act pins its own. */ +async function pinClientHostedPlacement(page: Page, enabled: boolean): Promise { + await page.evaluate(async (enabled) => { + await window.__store?.getState().updateSettings({ browserClientHostedRemoteEnabled: enabled }) + }, enabled) + expect( + await page.evaluate( + () => window.__store?.getState().settings?.browserClientHostedRemoteEnabled ?? null + ), + 'the placement setting this act is written for did not take' + ).toBe(enabled) +} + +/** Leaves the workspace holding only the screencast pane the next act right-clicks. */ +async function closeBrowserTabsExceptPane( + page: Page, + worktreeId: string, + paneUrl: string +): Promise { + await page.evaluate( + ({ paneUrl, worktreeId }) => { + const state = window.__store?.getState() + for (const workspace of state?.browserTabsByWorktree[worktreeId] ?? []) { + const pages = state?.browserPagesByWorkspace[workspace.id] ?? [] + if (!pages.some((browserPage) => browserPage.url.startsWith(paneUrl))) { + state?.closeBrowserTab(workspace.id) + } + } + }, + { paneUrl, worktreeId } + ) +} + type LinkOpenOutcome = 'opened on this machine' | 'pending' | 'refused' -async function readLinkOpenOutcome(page: Page, linkUrl: string): Promise { - if ((await readLocalBrowserViewUrls(page)).some((url) => url.startsWith(linkUrl))) { +/** The previous act's guest is settled away before this runs, so any page here is the fallback. */ +async function readLinkOpenOutcome( + client: PairedElectronClient, + linkUrl: string +): Promise { + if ((await readOwnedPageUrls(client.app, linkUrl)).length > 0) { return 'opened on this machine' } - const notice = page.getByTestId('remote-browser-stream-error') + const notice = client.page.getByTestId('remote-browser-stream-error') const text = (await notice.count()) > 0 ? ((await notice.first().textContent()) ?? '') : '' return text.includes('Unable to open URL.') ? 'refused' : 'pending' } @@ -172,7 +277,7 @@ async function openLinkFromRemotePaneContextMenu(page: Page): Promise { await openInOrca.click() } -test('opens a remote pane link on the pane runtime and refuses to fall back to the client', async ({ +test('opens a remote pane link on the pane runtime under either placement and refuses to fall back to the client', async ({ testRepoPath }, testInfo) => { test.setTimeout(300_000) @@ -181,7 +286,8 @@ test('opens a remote pane link on the pane runtime and refuses to fall back to t let client: PairedElectronClient | null = null try { - await host.client.call('repo.add', { path: testRepoPath, kind: 'git' }) + const hostRuntimeId = (await host.client.call('repo.add', { path: testRepoPath, kind: 'git' })) + ._meta.runtimeId client = await launchPairedElectronClient(host.offer, testInfo, 'Remote browser link routing') const page = client.page const environmentId = client.environmentId @@ -199,6 +305,8 @@ test('opens a remote pane link on the pane runtime and refuses to fall back to t throw new Error('paired client did not receive the host worktree') } + const worktreeSelector = `id:${worktreeId}` + // The workspace runs on the paired runtime, the way it does when the user picks that host. await page.evaluate( ({ environmentId, worktreeId }) => { @@ -227,13 +335,15 @@ test('opens a remote pane link on the pane runtime and refuses to fall back to t await focusMirroredPage(page, worktreeId, pane.pageId) const paneCountBeforeOpen = await page.getByTestId('remote-browser-pane').count() - // Act 1: the healthy path must land on the runtime, end to end. + // Act 1: server placement. The user asked for pages to live on the server, so the link must + // land on the runtime and be streamed back — nothing renders here. + await pinClientHostedPlacement(page, false) await openLinkFromRemotePaneContextMenu(page) await expect .poll( async () => - (await readHostBrowserUrls(host, worktreeId)).filter((url) => + (await readHostServerPlacedBrowserUrls(host, worktreeId)).filter((url) => url.startsWith(fixture.linkUrl) ).length, { timeout: 60_000, message: 'the link never opened as a browser tab on the host runtime' } @@ -241,6 +351,12 @@ test('opens a remote pane link on the pane runtime and refuses to fall back to t .toBe(1) // The host's browser really fetched it; a tab record alone would not prove a load. expect(fixture.linkLoadCount()).toBeGreaterThan(0) + await expect + .poll(() => readOwnedPageUrls(host.app, fixture.linkUrl), { + timeout: 60_000, + message: 'the runtime process never held a page for the link' + }) + .toHaveLength(1) // One more remote pane, and still nothing rendered by this machine's own browser. await expect(page.getByTestId('remote-browser-pane')).toHaveCount(paneCountBeforeOpen + 1, { timeout: 60_000 @@ -250,34 +366,118 @@ test('opens a remote pane link on the pane runtime and refuses to fall back to t ) expect(await readLocalBrowserViewUrls(page)).toHaveLength(0) - // Drop every tab except the pane's, so the second act drives the pane it started with. - await page.evaluate( - ({ paneUrl, worktreeId }) => { - const state = window.__store?.getState() - for (const workspace of state?.browserTabsByWorktree[worktreeId] ?? []) { - const pages = state?.browserPagesByWorkspace[workspace.id] ?? [] - if (!pages.some((browserPage) => browserPage.url.startsWith(paneUrl))) { - state?.closeBrowserTab(workspace.id) - } - } - }, - { paneUrl: fixture.paneUrl, worktreeId } - ) + // Drop every tab except the pane's, so the next act drives the pane it started with against a + // host that no longer holds the link. + await closeBrowserTabsExceptPane(page, worktreeId, fixture.paneUrl) await expect(page.getByTestId('remote-browser-pane')).toHaveCount(paneCountBeforeOpen, { timeout: 60_000 }) + await expect + .poll( + async () => + (await readHostServerPlacedBrowserUrls(host, worktreeId)).filter((url) => + url.startsWith(fixture.linkUrl) + ).length, + { timeout: 60_000, message: 'the runtime kept the closed browser tab' } + ) + .toBe(0) + await focusMirroredPage(page, worktreeId, pane.pageId) + const linkLoadsBeforeClientAct = fixture.linkLoadCount() - // Act 2: the user moves this workspace onto their own machine while the runtime's page is + // Act 2: client-hosted placement, the default. The page is hosted by this desktop, so the + // proof of correct routing is the host's record of it, not where it painted. + await pinClientHostedPlacement(page, true) + await openLinkFromRemotePaneContextMenu(page) + + await expect + .poll( + async () => (await findMirroredPage(page, worktreeId, fixture.linkUrl))?.placementKind, + { + timeout: 60_000, + message: 'the link never became a client-hosted browser page on this desktop' + } + ) + .toBe('client') + // The host's own connection, unprojected: browser.tabList reads the page registry, which is + // where a client-hosted page lives. + await expect + .poll( + async () => + (await readHostBrowserPageUrls(host.client, worktreeSelector)).filter((url) => + url.startsWith(fixture.linkUrl) + ).length, + { timeout: 60_000, message: 'the link never became a browser page on the host runtime' } + ) + .toBe(1) + const hostRows = await readHostBrowserRows(page, environmentId, worktreeId, fixture.linkUrl) + expect(hostRows).toHaveLength(1) + expect(hostRows[0]?.placementKind).toBe('client') + // The routing invariant, structurally: the host pinned this page's network to its own runtime + // when it created it, so the dev server was reached through the runtime and not through this + // machine — which CI cannot tell apart by watching the fixture, since both ends are loopback. + const executionHostKey = hostRows[0]?.executionHostKey + if (!executionHostKey) { + // Narrowed before parsing: an unpinned page would otherwise surface as a parse crash rather + // than as the missing network pin it is. + throw new Error('the host minted no network pin for the client-hosted page') + } + expect(parseBrowserNetworkExecutionHostKey(executionHostKey)).toMatchObject({ + runtimeId: hostRuntimeId + }) + expect(fixture.linkLoadCount()).toBeGreaterThan(linkLoadsBeforeClientAct) + // Hosted here, streamed from nowhere: this desktop holds the page and the runtime holds none. + await expect + .poll(() => readOwnedPageUrls(client!.app, fixture.linkUrl), { + timeout: 60_000, + message: 'the client-hosted guest never loaded the link on this desktop' + }) + .toHaveLength(1) + expect(await readOwnedPageUrls(host.app, fixture.linkUrl)).toHaveLength(0) + await expect(page.getByTestId('remote-browser-pane')).toHaveCount(paneCountBeforeOpen) + + // The store drops the tab synchronously and only then fires browser.tabClose, so the mirror + // going empty proves nothing about the host or the guest. Settle both before act 3 baselines + // them, or act 3 reads this teardown landing mid-act as its own doing. + await closeBrowserTabsExceptPane(page, worktreeId, fixture.paneUrl) + await expect + .poll(() => findMirroredPage(page, worktreeId, fixture.linkUrl), { + timeout: 60_000, + message: 'the client kept the closed link tab' + }) + .toBeNull() + await expect + .poll( + async () => + (await readHostBrowserPageUrls(host.client, worktreeSelector)).filter((url) => + url.startsWith(fixture.linkUrl) + ).length, + { timeout: 60_000, message: 'the runtime kept the closed client-hosted page' } + ) + .toBe(0) + await expect + .poll(() => readOwnedPageUrls(client!.app, fixture.linkUrl), { + timeout: 60_000, + message: 'the client-hosted guest outlived the tab that owned it' + }) + .toHaveLength(0) + await focusMirroredPage(page, worktreeId, pane.pageId) + + // Act 3: the user moves this workspace onto their own machine while the runtime's page is // still on screen. Opening the link must fail in the pane, not load the runtime's dev server // here — the client has no business serving a page for a workspace it does not run. await page.evaluate( ({ localHostId, worktreeId }) => { window.__store?.getState().setActiveWorktree(worktreeId, localHostId) }, - { localHostId: LOCAL_EXECUTION_HOST_ID, worktreeId } + // `as const` keeps the host id a literal through serialization; widened to string it stops + // being an ExecutionHostId. + { localHostId: LOCAL_EXECUTION_HOST_ID, worktreeId } as const ) await focusMirroredPage(page, worktreeId, pane.pageId) - const hostUrlsBefore = await readHostBrowserUrls(host, worktreeId) + // The refusal is about who owns the workspace, not where pages render, so it must hold under + // the placement the user most likely has on. + await pinClientHostedPlacement(page, true) + const hostPagesBefore = await readHostBrowserPageUrls(host.client, worktreeSelector) const linkLoadsBefore = fixture.linkLoadCount() await openLinkFromRemotePaneContextMenu(page) @@ -285,20 +485,20 @@ test('opens a remote pane link on the pane runtime and refuses to fall back to t // Wait for the click to produce an outcome — refusal or a local page — so the assertion below // reports which one happened instead of racing past a fallback that lands a moment later. await expect - .poll(() => readLinkOpenOutcome(page, fixture.linkUrl), { + .poll(() => readLinkOpenOutcome(client!, fixture.linkUrl), { timeout: 30_000, message: 'the link open produced neither a refusal nor a page' }) .not.toBe('pending') - expect(await readLinkOpenOutcome(page, fixture.linkUrl)).toBe('refused') + expect(await readLinkOpenOutcome(client, fixture.linkUrl)).toBe('refused') // The workspace must still be the local one, or the refusal above proved nothing. expect( await page.evaluate(() => window.__store?.getState().activeWorkspaceExecutionHostId ?? null) ).toBe(LOCAL_EXECUTION_HOST_ID) - // Nothing rendered here, nothing new on the host, and nobody fetched the link anywhere. - expect(await readLocalBrowserViewUrls(page)).toHaveLength(0) - expect(await readHostBrowserUrls(host, worktreeId)).toEqual(hostUrlsBefore) + // Nothing new rendered here, nothing new on the host, and nobody fetched the link anywhere. + expect(await readOwnedPageUrls(client.app, fixture.linkUrl)).toHaveLength(0) + expect(await readHostBrowserPageUrls(host.client, worktreeSelector)).toEqual(hostPagesBefore) expect(fixture.linkLoadCount()).toBe(linkLoadsBefore) } finally { if (client) { diff --git a/tests/e2e/terminal-duplicate-pty-renderer-reveal.spec.ts b/tests/e2e/terminal-duplicate-pty-renderer-reveal.spec.ts index 1792076de56..2eec7356f65 100644 --- a/tests/e2e/terminal-duplicate-pty-renderer-reveal.spec.ts +++ b/tests/e2e/terminal-duplicate-pty-renderer-reveal.spec.ts @@ -5,6 +5,12 @@ import type { ElectronApplication, Page } from '@stablyai/playwright-test' import type { TerminalLayoutSnapshot } from '../../src/shared/terminal-tab-types' import { DEFAULT_LOCAL_ORCA_PROFILE_ID } from '../../src/shared/orca-profiles' import { test, expect } from './helpers/orca-app' +import { + findMarkerFrame, + readActiveScreen, + readRenderedAltScreenFrame, + type ActiveScreen +} from './helpers/alt-screen-frame' import { attachRepoAndOpenTerminal, createRestartSession } from './helpers/orca-restart' import { stageNodeScriptForTerminal } from './helpers/run-node-script-in-terminal' import { @@ -124,18 +130,7 @@ async function readRendererOwnership( }, tabId) } -async function readStreamingFrame( - page: Page, - tabId: string, - marker: string -): Promise { - const content = await page.evaluate((tabId) => { - const pane = window.__paneManagers?.get(tabId)?.getPanes?.()[0] - return pane?.serializeAddon?.serialize?.() ?? null - }, tabId) - return parseStreamingFrame(content, marker) -} - +// Viewport-only, so it is the same projection the revealed pane is read with. async function readMainStreamingFrame( page: Page, ptyId: string, @@ -145,17 +140,47 @@ async function readMainStreamingFrame( const snapshot = await window.api.pty.getMainBufferSnapshot(ptyId, { scrollbackRows: 0 }) return snapshot?.data ?? null }, ptyId) - return parseStreamingFrame(content, marker) + return findMarkerFrame(content ?? '', marker) } -function parseStreamingFrame(content: string | null, marker: string): number | null { +type RevealFrameDiagnostics = { + bufferType: ActiveScreen['bufferType'] | null + screenFrame: number | null + serializedFrame: number | null + serializedLength: number + markerOffsets: number[] + screenRows: string[] +} + +// Why: if the revealed pane ever fails to converge, whether the freshest marker is on +// screen, only in normal-buffer scrollback, or absent is what separates a stalled +// renderer from stale residue left by the restore replay (STA-5208). +async function readRevealFrameDiagnostics( + page: Page, + tabId: string, + marker: string +): Promise { + const screen = await readActiveScreen(page, tabId) + const serialized = + (await page.evaluate((tabId) => { + // Same pane resolution as readActiveScreen, so both halves describe one pane. + const manager = window.__paneManagers?.get(tabId) + const pane = manager?.getActivePane?.() ?? manager?.getPanes?.()[0] + return pane?.serializeAddon?.serialize?.() ?? null + }, tabId)) ?? '' const prefix = `${marker} frame ` - const start = content?.lastIndexOf(prefix) ?? -1 - if (!content || start < 0) { - return null + const markerOffsets: number[] = [] + for (let at = serialized.indexOf(prefix); at >= 0; at = serialized.indexOf(prefix, at + 1)) { + markerOffsets.push(at) + } + return { + bufferType: screen?.bufferType ?? null, + screenFrame: screen ? findMarkerFrame(screen.rows.join('\n'), marker) : null, + serializedFrame: findMarkerFrame(serialized, marker), + serializedLength: serialized.length, + markerOffsets, + screenRows: screen?.rows ?? [] } - const digits = content.slice(start + prefix.length).match(/^\d+/)?.[0] - return digits ? Number(digits) : null } test('repairs duplicate persisted PTY renderers before streaming tab reveal', async (// oxlint-disable-next-line no-empty-pattern -- this restart test owns its Electron launches. @@ -231,13 +256,8 @@ test('repairs duplicate persisted PTY renderers before streaming tab reveal', as await expect .poll(() => getActiveTabId(secondLaunch.page), { timeout: 10_000 }) .toBe(restoredTabId) - await expect - .poll(() => readStreamingFrame(secondLaunch.page, restoredTabId, marker), { - timeout: 20_000, - message: 'Revealed renderer did not catch up to hidden authoritative output' - }) - .toBeGreaterThanOrEqual(hiddenFrame) - + // Ownership first: the frame is read off the single repaired pane, so the repair has + // to have settled before that read means anything. await expect .poll(() => readRendererOwnership(secondLaunch.page, restoredTabId), { timeout: 10_000 }) .toEqual({ @@ -247,10 +267,46 @@ test('repairs duplicate persisted PTY renderers before streaming tab reveal', as ptyBindingCount: 1, uniquePtyCount: 1 }) - await testInfo.attach('duplicate-pty-renderer-after-reveal.png', { - body: await secondLaunch.page.screenshot(), - contentType: 'image/png' - }) + let revealFailure: unknown = null + try { + await expect + .poll(() => readRenderedAltScreenFrame(secondLaunch.page, restoredTabId, marker), { + timeout: 20_000, + message: 'Revealed renderer did not catch up to hidden authoritative output' + }) + .toBeGreaterThanOrEqual(hiddenFrame) + // The fixture enters the alternate buffer once at startup and repaints in place, so a + // revealed pane parked on the normal buffer is painting the TUI into scrollback. + const revealedScreen = await readActiveScreen(secondLaunch.page, restoredTabId) + expect( + revealedScreen?.bufferType, + 'revealed pane was missing or painting outside the alternate buffer' + ).toBe('alternate') + } catch (error) { + // Diagnostics are more page reads, so a dead page has to degrade to a note in the + // attachment rather than replacing the failure the attachment exists to explain. + revealFailure = error + const diagnostics = await readRevealFrameDiagnostics( + secondLaunch.page, + restoredTabId, + marker + ).catch((diagnosticsError: unknown) => ({ diagnosticsError: String(diagnosticsError) })) + await testInfo.attach('duplicate-pty-reveal-frame-diagnostics.json', { + body: JSON.stringify(diagnostics, null, 2), + contentType: 'application/json' + }) + } + // Evidence for both outcomes; a capture that fails must not become the verdict. + const screenshot = await secondLaunch.page.screenshot().catch(() => null) + if (screenshot) { + await testInfo.attach('duplicate-pty-renderer-after-reveal.png', { + body: screenshot, + contentType: 'image/png' + }) + } + if (revealFailure) { + throw revealFailure + } } finally { tui.cleanup() if (secondApp) { diff --git a/tests/e2e/terminal-hidden-tui-visual-restore.spec.ts b/tests/e2e/terminal-hidden-tui-visual-restore.spec.ts index a78f4e81fa1..2bbba4b9117 100644 --- a/tests/e2e/terminal-hidden-tui-visual-restore.spec.ts +++ b/tests/e2e/terminal-hidden-tui-visual-restore.spec.ts @@ -2,6 +2,7 @@ import type { Page, TestInfo } from '@stablyai/playwright-test' import { randomUUID } from 'node:crypto' import { mkdirSync, rmSync, writeFileSync } from 'node:fs' import path from 'node:path' +import { alternateScreenFixtureScript } from './alternate-screen-fixture-script' import { test, expect } from './helpers/orca-app' import { runNodeScriptInTerminal } from './helpers/run-node-script-in-terminal' import { @@ -87,7 +88,7 @@ function writeHiddenFrameScript(scriptPath: string, runId: string): void { mkdirSync(path.dirname(scriptPath), { recursive: true }) writeFileSync( scriptPath, - `setTimeout(() => process.stdout.write(${JSON.stringify(frames.join(''))}), ${HIDDEN_FRAME_SCRIPT_DELAY_MS})\n` + alternateScreenFixtureScript(frames.join(''), HIDDEN_FRAME_SCRIPT_DELAY_MS) ) } @@ -246,59 +247,63 @@ test.describe('Hidden terminal TUI visual restore', () => { const scriptPath = path.join(testRepoPath, `.orca-hidden-tui-visual-${runId}.mjs`) writeHiddenFrameScript(scriptPath, runId) await resetHiddenDebug(orcaPage) - await writeHiddenFrames(orcaPage, hiddenPane.ptyId, scriptPath) - await resetHiddenDebug(orcaPage) + try { + await writeHiddenFrames(orcaPage, hiddenPane.ptyId, scriptPath) + await resetHiddenDebug(orcaPage) - // Why: hidden-delivery gate contract — the bulk TUI frames must be - // withheld in main (dropped after model ingestion), not delivered and - // skipped renderer-side. - await expect - .poll(() => readMainHiddenDeliveryDroppedChars(orcaPage), { - timeout: 10_000, - message: 'visually rich hidden TUI output was not withheld from the renderer' - }) - .toBeGreaterThan(1024) - await expect - .poll(() => readMainSnapshotSource(orcaPage, hiddenPane.ptyId!), { - timeout: 10_000, - message: 'visually rich hidden TUI source did not come from headless model' - }) - .toBe('headless') + // Why: hidden-delivery gate contract — the bulk TUI frames must be + // withheld in main (dropped after model ingestion), not delivered and + // skipped renderer-side. + await expect + .poll(() => readMainHiddenDeliveryDroppedChars(orcaPage), { + timeout: 10_000, + message: 'visually rich hidden TUI output was not withheld from the renderer' + }) + .toBeGreaterThan(1024) + await expect + .poll(() => readMainSnapshotSource(orcaPage, hiddenPane.ptyId!), { + timeout: 10_000, + message: 'visually rich hidden TUI source did not come from headless model' + }) + .toBe('headless') - await switchToWorktree(orcaPage, secondWorktreeId) - await ensureTerminalVisible(orcaPage) - await waitForActiveTerminalManager(orcaPage, 30_000) + await switchToWorktree(orcaPage, secondWorktreeId) + await ensureTerminalVisible(orcaPage) + await waitForActiveTerminalManager(orcaPage, 30_000) - await expect - .poll(() => getTerminalContent(orcaPage, 12_000), { - timeout: 10_000, - message: 'hidden TUI final frame did not restore when the workspace became visible' - }) - .toContain(finalMarker) + await expect + .poll(() => getTerminalContent(orcaPage, 12_000), { + timeout: 10_000, + message: 'hidden TUI final frame did not restore when the workspace became visible' + }) + .toContain(finalMarker) - const content = await getTerminalContent(orcaPage, 12_000) - expect(content).toContain(`Frame 024`) - expect(content).toContain('╭') - expect(content).toContain('├') - expect(content).toContain('█') - expect(content).not.toContain('Orca skipped hidden terminal output') - await expect - .poll(() => readTuiCursorState(orcaPage), { - timeout: 5_000, - message: 'restored TUI cursor stayed hidden after final frame' - }) - .toMatchObject({ - hidden: false, - initialized: true - }) + const content = await getTerminalContent(orcaPage, 12_000) + expect(content).toContain(`Frame 024`) + expect(content).toContain('╭') + expect(content).toContain('├') + expect(content).toContain('█') + expect(content).not.toContain('Orca skipped hidden terminal output') + await expect + .poll(() => readTuiCursorState(orcaPage), { + timeout: 5_000, + message: 'restored TUI cursor stayed hidden after final frame' + }) + .toMatchObject({ + hidden: false, + initialized: true + }) - const screenshotPath = testInfo.outputPath('hidden-tui-restore-final.png') - await orcaPage.screenshot({ path: screenshotPath, fullPage: true }) - await testInfo.attach('hidden-tui-restore-final.png', { - path: screenshotPath, - contentType: 'image/png' - }) - rmSync(scriptPath, { force: true }) + const screenshotPath = testInfo.outputPath('hidden-tui-restore-final.png') + await orcaPage.screenshot({ path: screenshotPath, fullPage: true }) + await testInfo.attach('hidden-tui-restore-final.png', { + path: screenshotPath, + contentType: 'image/png' + }) + } finally { + await sendToTerminal(orcaPage, hiddenPane.ptyId, '\x03').catch(() => undefined) + rmSync(scriptPath, { force: true }) + } }) test('keeps newer live output correct after plain hidden output restores', async ({ @@ -480,6 +485,7 @@ test.describe('Hidden terminal TUI visual restore', () => { contentType: 'image/png' }) } finally { + await sendToTerminal(orcaPage, hiddenPane.ptyId, '\x03').catch(() => undefined) rmSync(scriptPath, { force: true }) } }) diff --git a/tests/e2e/terminal-hidden-view-parking.spec.ts b/tests/e2e/terminal-hidden-view-parking.spec.ts index 19fc02e8486..9aba68407f5 100644 --- a/tests/e2e/terminal-hidden-view-parking.spec.ts +++ b/tests/e2e/terminal-hidden-view-parking.spec.ts @@ -2,6 +2,7 @@ import type { Page, TestInfo } from '@stablyai/playwright-test' import { randomUUID } from 'node:crypto' import { mkdirSync, rmSync, writeFileSync } from 'node:fs' import path from 'node:path' +import { alternateScreenFixtureScript } from './alternate-screen-fixture-script' import { test, expect } from './helpers/orca-app' import { runNodeScriptInTerminal } from './helpers/run-node-script-in-terminal' import { @@ -18,6 +19,7 @@ import { waitForPaneIdentitySnapshot } from './helpers/terminal' import { parkHiddenTabBehindDecoy, waitForTabParked } from './helpers/terminal-hidden-parking' +import { waitForPtyShellEcho } from './terminal-pty-readiness' import { TERMINAL_TAB_PARK_FLIP_BURST_WINDOW_MS } from '../../src/renderer/src/components/terminal-pane/terminal-park-verdict-flip-telemetry' // Why: the parking wiring registers this handle (dev/exposeStore builds only) @@ -73,7 +75,7 @@ function writeParkedFrameScript(scriptPath: string, runId: string): void { mkdirSync(path.dirname(scriptPath), { recursive: true }) writeFileSync( scriptPath, - `setTimeout(() => process.stdout.write(${JSON.stringify(frames.join(''))}), ${PARKED_FRAME_SCRIPT_DELAY_MS})\n` + alternateScreenFixtureScript(frames.join(''), PARKED_FRAME_SCRIPT_DELAY_MS) ) } @@ -103,12 +105,7 @@ function cycleReferenceFrame(runId: string): string { function writeCycleReferenceScript(scriptPath: string, runId: string): void { mkdirSync(path.dirname(scriptPath), { recursive: true }) - // Paint the frame once, then hold the process open so the alt-screen TUI - // stays on screen (and the parkable PTY session stays alive) across cycles. - writeFileSync( - scriptPath, - `process.stdout.write(${JSON.stringify(cycleReferenceFrame(runId))}); setInterval(() => {}, 1000)\n` - ) + writeFileSync(scriptPath, alternateScreenFixtureScript(cycleReferenceFrame(runId))) } // Why: serialize() re-emits the buffer with cursor-restore trailer sequences @@ -335,6 +332,17 @@ test.describe('Terminal hidden view parking', () => { expect(content).toContain('█') expect(content).not.toContain('Orca skipped hidden terminal output') + // Why: the fixture TUI still owns the PTY foreground after the reveal, so + // interrupt it and wait for the shell to take input back before probing. + await sendToTerminal(orcaPage, tabAPtyId, '\x03') + // Why rethrow: the readiness failure reads as a dead shell, but the only new + // dependency here is Ctrl-C reaching the foreground TUI (ConPTY translates it). + await waitForPtyShellEcho(orcaPage, tabAPtyId, 15_000).catch((error: unknown) => { + throw new Error( + `Ctrl-C did not hand the PTY back from the fixture TUI: ${error instanceof Error ? error.message : String(error)}` + ) + }) + // Why: the typed marker only appears joined in command *output*, so this // proves the revealed terminal accepts input end-to-end, not just echo. const typedMarker = `PARKED_TYPED_OK_${runId}` diff --git a/tests/e2e/terminal-osc8-cold-park-restore.spec.ts b/tests/e2e/terminal-osc8-cold-park-restore.spec.ts index 8a31afe83da..fddcb24b23d 100644 --- a/tests/e2e/terminal-osc8-cold-park-restore.spec.ts +++ b/tests/e2e/terminal-osc8-cold-park-restore.spec.ts @@ -1,6 +1,8 @@ import { randomUUID } from 'node:crypto' import type { Page } from '@stablyai/playwright-test' +import { alternateScreenFixtureScript } from './alternate-screen-fixture-script' import { expect, test } from './helpers/orca-app' +import { stageNodeScriptForTerminal } from './helpers/run-node-script-in-terminal' import { parkHiddenTabBehindDecoy } from './helpers/terminal-hidden-parking' import { ensureTerminalVisible, @@ -136,48 +138,56 @@ test('restores and opens an OSC 8 link after its terminal is cold-parked', async const label = `#${randomUUID().slice(0, 6)}` const url = `https://example.com/orca-osc8-${randomUUID()}` const linkedOutput = `\x1b[?1049h\x1b[2J\x1b[H\x1b]8;id=cold-park;${url}\x1b\\${label}\x1b]8;;\x1b\\\n` - await sendToTerminal( - orcaPage, - ptyId, - `${nodeTerminalCommand(['-e', `process.stdout.write(${JSON.stringify(linkedOutput)})`])}\r` - ) - await expect.poll(() => getTerminalContent(orcaPage, 4_000)).toContain(label) - - const baselineProbe = await locateLink(orcaPage, label) - await orcaPage.mouse.move(baselineProbe.clientX, baselineProbe.clientY) - await expect - .poll(() => readLinkState(orcaPage, tabId, label)) - .toMatchObject({ - bufferType: 'alternate', - serializedUri: true, - underlined: true, - uri: url - }) - - await parkHiddenTabBehindDecoy(orcaPage, worktreeId, tabId, { - parkDelayMs: PARKING_DELAY_MS + // Why staged rather than `node -e`: PowerShell mangles the escapes (#8521), and it + // keeps the label out of the command line so the readiness poll below cannot be + // satisfied by the shell's own echo. `staged.command` is bypassed because it runs a + // bare `node`; nodeTerminalCommand pins process.execPath for Windows CI's PATH. + const staged = stageNodeScriptForTerminal(alternateScreenFixtureScript(linkedOutput), { + prefix: 'orca-osc8-cold-park' }) - await activateTerminalTab(orcaPage, tabId) - await expect.poll(() => getTerminalContent(orcaPage, 4_000)).toContain(label) + try { + await sendToTerminal(orcaPage, ptyId, `${nodeTerminalCommand([staged.scriptPath])}\r`) + await expect.poll(() => getTerminalContent(orcaPage, 4_000)).toContain(label) - const restoredProbe = await locateLink(orcaPage, label) - await orcaPage.mouse.move(restoredProbe.clientX, restoredProbe.clientY) - await expect - .poll(() => readLinkState(orcaPage, tabId, label)) - .toMatchObject({ - bufferType: 'alternate', - serializedUri: true, - underlined: true, - uri: url + const baselineProbe = await locateLink(orcaPage, label) + await orcaPage.mouse.move(baselineProbe.clientX, baselineProbe.clientY) + await expect + .poll(() => readLinkState(orcaPage, tabId, label)) + .toMatchObject({ + bufferType: 'alternate', + serializedUri: true, + underlined: true, + uri: url + }) + + await parkHiddenTabBehindDecoy(orcaPage, worktreeId, tabId, { + parkDelayMs: PARKING_DELAY_MS }) + await activateTerminalTab(orcaPage, tabId) + await expect.poll(() => getTerminalContent(orcaPage, 4_000)).toContain(label) - const isMac = await orcaPage.evaluate(() => navigator.userAgent.includes('Mac')) - const modifier = isMac ? 'Meta' : 'Control' - await orcaPage.keyboard.down(modifier) - await orcaPage.mouse.down() - await orcaPage.mouse.up() - await orcaPage.keyboard.up(modifier) - await expect - .poll(async () => (await getBrowserTabs(orcaPage, worktreeId)).some((tab) => tab.url === url)) - .toBe(true) + const restoredProbe = await locateLink(orcaPage, label) + await orcaPage.mouse.move(restoredProbe.clientX, restoredProbe.clientY) + await expect + .poll(() => readLinkState(orcaPage, tabId, label)) + .toMatchObject({ + bufferType: 'alternate', + serializedUri: true, + underlined: true, + uri: url + }) + + const isMac = await orcaPage.evaluate(() => navigator.userAgent.includes('Mac')) + const modifier = isMac ? 'Meta' : 'Control' + await orcaPage.keyboard.down(modifier) + await orcaPage.mouse.down() + await orcaPage.mouse.up() + await orcaPage.keyboard.up(modifier) + await expect + .poll(async () => (await getBrowserTabs(orcaPage, worktreeId)).some((tab) => tab.url === url)) + .toBe(true) + } finally { + await sendToTerminal(orcaPage, ptyId, '\x03').catch(() => undefined) + staged.cleanup() + } }) diff --git a/tests/e2e/update-install-renderer-checkpoint-recovery.spec.ts b/tests/e2e/update-install-renderer-checkpoint-recovery.spec.ts index 9a521926f19..b1bb0c0f997 100644 --- a/tests/e2e/update-install-renderer-checkpoint-recovery.spec.ts +++ b/tests/e2e/update-install-renderer-checkpoint-recovery.spec.ts @@ -1,7 +1,14 @@ import path from 'node:path' import { test, expect } from './helpers/orca-app' -const CHECKPOINT_ERROR = 'Renderer shutdown checkpoint was not completed.' +// Only the prefix is contract: the checkpoint error appends the swallowed persist +// cause (STA-5505), whose wording belongs to whatever threw. +const CHECKPOINT_ERROR_PREFIX = 'Renderer shutdown checkpoint was not completed: ' + +// The null url is injected, never hydrated: the desktop session schema drops such a row, and +// no other arrival path carries browserUrlHistory at all (paired web reads it unvalidated but +// has no producer — STA-5668 follow-up). It is just a deterministic snapshot-build failure. +const CORRUPT_HISTORY_ENTRY = { url: null, title: 'corrupt persisted history', lastVisitedAt: 0 } test('recovers update install from a corrupt clean session but preserves dirty drafts', async ({ orcaPage, @@ -15,16 +22,14 @@ test('recovers update install from a corrupt clean session but preserves dirty d }) const dirtyResult = await orcaPage.evaluate( - async ({ filePath, worktreeId }) => { + async ({ filePath, worktreeId, corruptEntry }) => { const store = window.__store if (!store) { throw new Error('window.__store is not available') } const state = store.getState() const originalHistory = state.browserUrlHistory - state.browserUrlHistory = [ - { url: null, title: 'corrupt persisted history', lastVisitedAt: 0 } - ] as unknown as typeof state.browserUrlHistory + state.browserUrlHistory = [corruptEntry] as unknown as typeof state.browserUrlHistory const fileId = state.openFile({ filePath, relativePath: 'checkpoint-draft.txt', @@ -48,22 +53,24 @@ test('recovers update install from a corrupt clean session but preserves dirty d }, { filePath: path.join(testRepoPath, 'checkpoint-draft.txt'), - worktreeId: await orcaPage.evaluate(() => window.__store?.getState().activeWorktreeId ?? '') + worktreeId: await orcaPage.evaluate(() => window.__store?.getState().activeWorktreeId ?? ''), + corruptEntry: CORRUPT_HISTORY_ENTRY } ) - expect(dirtyResult).toBe(CHECKPOINT_ERROR) + expect(dirtyResult).toContain(CHECKPOINT_ERROR_PREFIX) + // Pin the cause to the corrupt row, not just any named failure; only the member name + // survives V8 rewording of "Cannot read properties of null". + expect(dirtyResult).toContain('toLowerCase') - const cleanResult = await orcaPage.evaluate(async () => { + const cleanResult = await orcaPage.evaluate(async (corruptEntry) => { const store = window.__store if (!store) { throw new Error('window.__store is not available') } const state = store.getState() const originalHistory = state.browserUrlHistory - state.browserUrlHistory = [ - { url: null, title: 'corrupt persisted history', lastVisitedAt: 0 } - ] as unknown as typeof state.browserUrlHistory + state.browserUrlHistory = [corruptEntry] as unknown as typeof state.browserUrlHistory try { await window.api.updater.quitAndInstall() return 'continued' @@ -72,7 +79,7 @@ test('recovers update install from a corrupt clean session but preserves dirty d } finally { state.browserUrlHistory = originalHistory } - }) + }, CORRUPT_HISTORY_ENTRY) expect(cleanResult).toBe('continued') expect(fallbackLogs).toHaveLength(1) diff --git a/tests/e2e/worktree-card-downward-drag.spec.ts b/tests/e2e/worktree-card-downward-drag.spec.ts index b5cb685a4a6..8ee77548aee 100644 --- a/tests/e2e/worktree-card-downward-drag.spec.ts +++ b/tests/e2e/worktree-card-downward-drag.spec.ts @@ -78,7 +78,17 @@ async function seedVirtualizedManualWorktrees(page: Page): Promise<{ ...state.worktreesByRepo, [repoId]: [...worktrees, ...seededWorktrees] }, - updateWorktreesMeta: async (updatesByWorktreeId) => { + // Stands in for the real action only to skip its IPC persistence tail: these 60 rows + // are store-only, so persisting them would fail and refetch them away mid-drag. + updateWorktreesMeta: async (batchUpdates) => { + if (batchUpdates.length === 0) { + return + } + // executionHostId is ignored on purpose: every seeded row is host-less, which the + // real action treats as local (worktree-meta-host-match.ts). + const updatesByWorktreeId = new Map( + batchUpdates.map((batchUpdate) => [batchUpdate.worktreeId, batchUpdate.updates]) + ) store.setState((current) => ({ sortEpoch: current.sortEpoch + 1, worktreesByRepo: Object.fromEntries( diff --git a/tests/e2e/worktree-jump-palette-filter.spec.ts b/tests/e2e/worktree-jump-palette-filter.spec.ts index b4841cf059e..9c5d6146ba1 100644 --- a/tests/e2e/worktree-jump-palette-filter.spec.ts +++ b/tests/e2e/worktree-jump-palette-filter.spec.ts @@ -1,4 +1,4 @@ -import type { Page } from '@stablyai/playwright-test' +import type { Locator, Page } from '@stablyai/playwright-test' import { expect, test } from './helpers/orca-app' import { waitForActiveWorktree, waitForSessionReady } from './helpers/store' @@ -132,6 +132,23 @@ async function selectRemoteHost(page: Page, useKeyboard = false): Promise await filterTrigger(page).click() } +async function openComposerFromTypedName(page: Page): Promise { + await openPalette(page) + const input = palette(page).getByPlaceholder(SEARCH_PLACEHOLDER) + await input.fill(`cmd-j-enter-${Date.now()}`) + await expect(palette(page).locator('[cmdk-item][data-value="__create_worktree__"]')).toBeVisible() + + await input.press('Enter') + + const createDialog = page.getByRole('dialog', { name: /Create (Workspace|Worktree)/i }) + await expect(createDialog).toBeVisible() + // Why assert focus: the composer auto-focuses the name field, so Escape always + // lands on an input the user never chose. A page-style "blur the field first" + // handler reachable from here would silently cost a second press. + await expect(createDialog.locator('[data-workspace-name-input="true"]')).toBeFocused() + return createDialog +} + test.describe('Worktree jump-palette filters', () => { test.beforeEach(async ({ orcaPage }) => { await waitForSessionReady(orcaPage) @@ -182,22 +199,33 @@ test.describe('Worktree jump-palette filters', () => { }) test('pressing Enter creates a worktree from a typed name', async ({ orcaPage }) => { - await openPalette(orcaPage) - const input = palette(orcaPage).getByPlaceholder(SEARCH_PLACEHOLDER) - await input.fill(`cmd-j-enter-${Date.now()}`) - await expect( - palette(orcaPage).locator('[cmdk-item][data-value="__create_worktree__"]') - ).toBeVisible() + const createDialog = await openComposerFromTypedName(orcaPage) - await input.press('Enter') - - const createDialog = orcaPage.getByRole('dialog', { name: /Create (Workspace|Worktree)/i }) - await expect(createDialog).toBeVisible() - // Why assert focus first: the composer auto-focuses the name field, so Escape - // always lands on an input the user never chose. A page-style "blur the field - // first" handler here would silently cost a second press. - await expect(createDialog.locator('[data-workspace-name-input="true"]')).toBeFocused() await orcaPage.keyboard.press('Escape') + await expect(createDialog).toBeHidden() }) + + test('Escape closes the composer opened over the Automations page', async ({ orcaPage }) => { + // Why this view: Cmd+J has no view guard, and a page mounted under the palette + // keeps its own capture-phase Escape listener registered. Window capture runs + // before Radix's document capture, so a preventDefault there vetoes dismissal. + await orcaPage.evaluate(() => window.__store?.getState().openAutomationsPage()) + const automationsHeading = orcaPage.getByRole('heading', { name: 'Automations', level: 1 }) + await expect(automationsHeading).toBeVisible() + + const createDialog = await openComposerFromTypedName(orcaPage) + + await orcaPage.keyboard.press('Escape') + + await expect(createDialog).toBeHidden() + // The page declined the press rather than consuming it, so it is still open. + await expect(automationsHeading).toBeVisible() + + // Why a second press: with nothing layered above, the real page chrome must not + // trip the overlay check, or Escape would never close Automations again. + await orcaPage.keyboard.press('Escape') + + await expect(automationsHeading).toBeHidden() + }) })