diff --git a/tests/e2e/tab-close-navigation.spec.ts b/tests/e2e/tab-close-navigation.spec.ts new file mode 100644 index 00000000..abdd445e --- /dev/null +++ b/tests/e2e/tab-close-navigation.spec.ts @@ -0,0 +1,313 @@ +/** + * E2E tests for what happens when tabs are closed: which neighbor becomes + * active, and how the app returns to Landing when the last tab is gone. + * + * Why these flows: + * - PR #693 (`close editor/diff tabs should navigate to visual neighbor tab`) + * fixed a regression where closing the active editor tab jumped to an + * arbitrary file. The existing `tabs.spec.ts` only covers terminal tab + * close; the editor/diff close path has no E2E guard today. + * - PR #677 (`return to Orca landing screen after closing last terminal`) + * plus editor.ts's `shouldDeactivateWorktree` branch (also hardened in + * tabs.ts's `closeUnifiedTab`) require that when a worktree's last visible + * surface closes, the app clears `activeWorktreeId` instead of leaving a + * selected worktree with nothing to render. Any regression here shows up + * as a blank workspace. + * - PR #532 had to be patched because `closeFile` forgot to keep + * `activeFileIdByWorktree` honest. This spec covers the user-visible + * invariant: after closing the active editor tab, the replacement active + * file is one that is still open. + */ + +import { test, expect } from './helpers/orca-app' +import { + waitForSessionReady, + waitForActiveWorktree, + getActiveWorktreeId, + getActiveTabType, + getOpenFiles, + ensureTerminalVisible +} from './helpers/store' + +/** + * Why: take the worktreeId explicitly instead of reading activeWorktreeId + * inside the page. Tests in this file deliberately drain other surfaces + * (terminals/browser tabs) which can trigger the `shouldDeactivateWorktree` + * cascade and clear activeWorktreeId. A helper that silently depends on + * activeWorktreeId would then return `[]` and fail in a confusing way. Taking + * the id as an argument keeps each test's setup self-contained and order- + * independent. + */ +async function openSeededEditorTabs( + page: Parameters[0], + worktreeId: string, + relativePaths: string[] +): Promise { + return page.evaluate( + ({ wId, relPaths }) => { + const store = window.__store + if (!store) { + return [] + } + + const state = store.getState() + const worktree = Object.values(state.worktreesByRepo) + .flat() + .find((entry) => entry.id === wId) + if (!worktree) { + return [] + } + + const separator = worktree.path.includes('\\') ? '\\' : '/' + const ids: string[] = [] + for (const relPath of relPaths) { + const filePath = `${worktree.path}${separator}${relPath}` + state.openFile({ + filePath, + relativePath: relPath, + worktreeId: wId, + language: relPath.endsWith('.md') + ? 'markdown' + : relPath.endsWith('.json') + ? 'json' + : relPath.endsWith('.ts') + ? 'typescript' + : 'plaintext', + mode: 'edit' + }) + const latest = store.getState().openFiles.find((f) => f.filePath === filePath) + if (latest) { + ids.push(latest.id) + } + } + return ids + }, + { wId: worktreeId, relPaths: relativePaths } + ) +} + +async function setActiveFile( + page: Parameters[0], + fileId: string +): Promise { + await page.evaluate((id) => { + const store = window.__store + if (!store) { + return + } + + const state = store.getState() + state.setActiveFile(id) + state.setActiveTabType('editor') + }, fileId) +} + +async function closeFile( + page: Parameters[0], + fileId: string +): Promise { + await page.evaluate((id) => { + window.__store?.getState().closeFile(id) + }, fileId) +} + +async function getActiveFileId( + page: Parameters[0] +): Promise { + return page.evaluate(() => window.__store?.getState().activeFileId ?? null) +} + +test.describe('Tab Close Navigation', () => { + test.beforeEach(async ({ orcaPage }) => { + await waitForSessionReady(orcaPage) + await waitForActiveWorktree(orcaPage) + await ensureTerminalVisible(orcaPage) + }) + + /** + * Covers PR #693: closing the active editor tab should activate the visual + * neighbor in the same worktree, not the first file in the list. + */ + test('closing the active editor tab activates its visual neighbor', async ({ orcaPage }) => { + const worktreeId = await waitForActiveWorktree(orcaPage) + + const fileIds = await openSeededEditorTabs(orcaPage, worktreeId, [ + 'package.json', + 'README.md', + 'tsconfig.json' + ]) + expect(fileIds.length).toBe(3) + + // Activate the middle tab and close it. The neighbor-picking logic in + // closeFile should pick the file that sat immediately after the closed + // one in the worktree's openFiles slice. + await setActiveFile(orcaPage, fileIds[1]) + await expect.poll(async () => getActiveFileId(orcaPage), { timeout: 3_000 }).toBe(fileIds[1]) + + await closeFile(orcaPage, fileIds[1]) + + const openFilesAfter = await getOpenFiles(orcaPage, worktreeId) + const remainingIds = new Set(openFilesAfter.map((f) => f.id)) + expect(remainingIds.has(fileIds[1])).toBe(false) + + // Why tsconfig.json specifically: `closeFile` picks `worktreeFiles[closedIdx]` + // from the post-close list (editor.ts:681-684). For a middle close on + // [pkg, README, tsconfig], closedIdx=1 and the post-close list is + // [pkg, tsconfig], so the neighbor is tsconfig.json (fileIds[2]). PR #693 + // regressed this by leaving `activeFileId` pointing at a closed file — a + // laxer assertion like "some open file is active" would have missed that + // specific regression, since any order-agnostic fallback would still pass. + await expect + .poll(async () => getActiveFileId(orcaPage), { + timeout: 5_000, + message: 'expected the visual neighbor (tsconfig.json) to become active after close' + }) + .toBe(fileIds[2]) + + // And the workspace must still be showing an editor, not silently flipping + // back to terminal while editors remain open. + await expect.poll(async () => getActiveTabType(orcaPage), { timeout: 3_000 }).toBe('editor') + }) + + /** + * Same visual-neighbor invariant but for diff tabs — they share the + * openFiles list with editor tabs (contentType='diff') and route through + * the same closeFile path, which is where #693 regressed. + */ + test('closing the active diff tab activates a still-open neighbor', async ({ orcaPage }) => { + const worktreeId = await waitForActiveWorktree(orcaPage) + + // Seed two editor tabs + one diff tab in the same worktree. + const editorIds = await openSeededEditorTabs(orcaPage, worktreeId, [ + 'package.json', + 'README.md' + ]) + expect(editorIds.length).toBe(2) + + const diffId = await orcaPage.evaluate((wId) => { + const store = window.__store + if (!store) { + return null + } + + const state = store.getState() + const worktree = Object.values(state.worktreesByRepo) + .flat() + .find((entry) => entry.id === wId) + if (!worktree) { + return null + } + + const separator = worktree.path.includes('\\') ? '\\' : '/' + state.openDiff( + wId, + `${worktree.path}${separator}src${separator}index.ts`, + `src${separator}index.ts`, + 'typescript', + false + ) + return store.getState().activeFileId + }, worktreeId) + + expect(diffId).not.toBeNull() + await expect.poll(async () => getActiveFileId(orcaPage), { timeout: 3_000 }).toBe(diffId) + + await closeFile(orcaPage, diffId!) + + const openFilesAfter = await getOpenFiles(orcaPage, worktreeId) + const remainingIds = new Set(openFilesAfter.map((f) => f.id)) + expect(remainingIds.has(diffId!)).toBe(false) + expect(remainingIds.size).toBe(2) + + // Why README.md specifically: the diff tab was appended last + // (index 2 in openFiles). Closing it hits the + // `closedWorktreeIdx >= worktreeFiles.length` branch in closeFile + // (editor.ts:681-683) which picks `worktreeFiles.at(-1)` — the last + // remaining file, README.md (editorIds[1]). Asserting the exact ID makes + // this a real guard against #693 instead of a tautology. + await expect + .poll(async () => getActiveFileId(orcaPage), { + timeout: 5_000, + message: 'expected README.md (last remaining) to become active after closing the diff tab' + }) + .toBe(editorIds[1]) + }) + + /** + * Covers PR #677 and the `shouldDeactivateWorktree` branch in closeFile: + * when the last editor closes and no terminal/browser surface remains for + * the worktree, the app must return to Landing (activeWorktreeId === null). + */ + test('closing the last visible surface returns the app to Landing', async ({ orcaPage }) => { + const worktreeId = await waitForActiveWorktree(orcaPage) + + // Prepare the worktree so only a single editor tab is present as a + // visible surface: no browser tabs and no terminal tabs. + // + // Why re-activate at the end: closing the last unified tab for a worktree + // that has no editor/browser surfaces triggers the `shouldDeactivateWorktree` + // cascade in closeUnifiedTab, which clears activeWorktreeId. That's the + // exact behavior this test eventually asserts — but we need the worktree + // active again in order to seed the editor that we will close. Re-selecting + // it here keeps the helpers below self-contained and the test's setup + // order-independent instead of depending on whichever surface-close + // happens to leave activeWorktreeId untouched. + await orcaPage.evaluate((wId) => { + const store = window.__store + if (!store) { + return + } + + const state = store.getState() + // Close every terminal tab in this worktree so removing the last editor + // leaves nothing visible. Terminal tabs persist in tabsByWorktree even + // when activeTabType flips to 'editor'. + for (const tab of state.tabsByWorktree[wId] ?? []) { + state.closeTab(tab.id) + } + + // Drop any browser tabs too, for the same reason. + for (const bt of state.browserTabsByWorktree[wId] ?? []) { + state.closeBrowserTab(bt.id) + } + + // Re-select the worktree if surface-close cascades deactivated it. + if (store.getState().activeWorktreeId !== wId) { + store.getState().setActiveWorktree(wId) + } + }, worktreeId) + + const editorIds = await openSeededEditorTabs(orcaPage, worktreeId, ['package.json']) + expect(editorIds.length).toBe(1) + + await setActiveFile(orcaPage, editorIds[0]) + await expect.poll(async () => getActiveFileId(orcaPage), { timeout: 3_000 }).toBe(editorIds[0]) + + // Sanity: confirm the worktree has no backing terminal/browser surfaces + // before we close the last editor. Otherwise the deactivate branch would + // not trigger for reasons unrelated to this regression. + const surfaceCounts = await orcaPage.evaluate((wId) => { + const store = window.__store + if (!store) { + throw new Error('window.__store is not available') + } + const state = store.getState() + return { + terminals: (state.tabsByWorktree[wId] ?? []).length, + browserTabs: (state.browserTabsByWorktree[wId] ?? []).length + } + }, worktreeId) + expect(surfaceCounts).toEqual({ terminals: 0, browserTabs: 0 }) + + await closeFile(orcaPage, editorIds[0]) + + // The worktree should be deselected. Landing renders when + // activeWorktreeId === null. + await expect + .poll(async () => getActiveWorktreeId(orcaPage), { + timeout: 5_000, + message: 'activeWorktreeId was not cleared after closing the last visible surface' + }) + .toBeNull() + }) +}) diff --git a/tests/e2e/worktree-lifecycle.spec.ts b/tests/e2e/worktree-lifecycle.spec.ts new file mode 100644 index 00000000..1f5bed38 --- /dev/null +++ b/tests/e2e/worktree-lifecycle.spec.ts @@ -0,0 +1,341 @@ +/** + * E2E tests for the full worktree lifecycle: removal cleanup, switching with + * the right sidebar open, and cross-worktree tab isolation. + * + * Why these flows: + * - PR #532 (`clean up editor/terminal state when removing a worktree`) showed + * that removeWorktree must drop the tabs/editors/browser tabs owned by the + * removed worktree. A regression here silently leaks a deleted worktree's + * IDs into tabsByWorktree / openFiles and breaks the UI the next time the + * user opens another worktree. + * - PR #628 (`resolve Windows freeze when switching worktrees with right + * sidebar open`) + PR #598 (`resolve right sidebar freeze on Windows`) + + * PR #726 (`prevent split-group container teardown when switching + * worktrees`) all changed behavior on the same path: activating a different + * worktree while the right sidebar is showing. Assert that the switch lands + * cleanly with the sidebar still open, because the prior regressions left + * the UI hung. + * - PR #542 / #554 (`terminal shortcuts firing in wrong worktree`) regressed + * twice. Cover the invariant directly: a terminal tab created in worktree A + * must not appear in worktree B's tab list. + */ + +import { test, expect } from './helpers/orca-app' +import { + waitForSessionReady, + waitForActiveWorktree, + getActiveWorktreeId, + getAllWorktreeIds, + getWorktreeTabs, + getOpenFiles, + getBrowserTabs, + switchToWorktree, + ensureTerminalVisible +} from './helpers/store' +import { clickFileInExplorer, openFileExplorer } from './helpers/file-explorer' + +async function createIsolatedWorktree( + page: Parameters[0] +): Promise { + const name = `e2e-lifecycle-${Date.now()}` + return page.evaluate(async (worktreeName) => { + const store = window.__store + if (!store) { + throw new Error('window.__store is not available') + } + + const state = store.getState() + const activeWorktreeId = state.activeWorktreeId + if (!activeWorktreeId) { + throw new Error('No active worktree to derive repo from') + } + + const activeWorktree = Object.values(state.worktreesByRepo) + .flat() + .find((worktree) => worktree.id === activeWorktreeId) + if (!activeWorktree) { + throw new Error(`Active worktree ${activeWorktreeId} not found`) + } + + const result = await state.createWorktree(activeWorktree.repoId, worktreeName) + await state.fetchWorktrees(activeWorktree.repoId) + return result.worktree.id + }, name) +} + +async function removeWorktreeViaStore( + page: Parameters[0], + worktreeId: string +): Promise<{ ok: boolean; error?: string }> { + return page.evaluate(async (id) => { + const store = window.__store + if (!store) { + return { ok: false as const, error: 'store unavailable' } + } + + const result = await store.getState().removeWorktree(id, true) + return result + }, worktreeId) +} + +test.describe('Worktree Lifecycle', () => { + // Why: `createIsolatedWorktree` materializes a real on-disk worktree in the + // worker-scoped seed repo. If a mid-test assertion fails, that branch + + // working directory leaks across subsequent tests in the same worker. Track + // the ID here and best-effort remove it in afterEach so fixture state stays + // clean even when a test aborts before its own cleanup runs. + let createdWorktreeId: string | null = null + + test.beforeEach(async ({ orcaPage }) => { + await waitForSessionReady(orcaPage) + await waitForActiveWorktree(orcaPage) + await ensureTerminalVisible(orcaPage) + }) + + test.afterEach(async ({ orcaPage }) => { + if (!createdWorktreeId) { + return + } + const idToClean = createdWorktreeId + createdWorktreeId = null + await orcaPage + .evaluate(async (id) => { + try { + await window.__store?.getState().removeWorktree(id, true) + } catch { + /* best-effort cleanup */ + } + }, idToClean) + .catch(() => undefined) + }) + + /** + * Covers PR #532: removing a worktree must drop its tab/editor/browser state + * from the store, not leak IDs into the next render. + */ + test('removing a worktree clears its tabs, open files, and browser tabs', async ({ + orcaPage + }) => { + const originalWorktreeId = await waitForActiveWorktree(orcaPage) + + createdWorktreeId = await createIsolatedWorktree(orcaPage) + const newWorktreeId = createdWorktreeId + await switchToWorktree(orcaPage, newWorktreeId) + await expect + .poll(async () => getActiveWorktreeId(orcaPage), { timeout: 10_000 }) + .toBe(newWorktreeId) + await ensureTerminalVisible(orcaPage) + + // Seed one of each surface on the new worktree so removeWorktree has to + // clean up all three in a single atomic set(). + await orcaPage.evaluate((worktreeId) => { + const store = window.__store + if (!store) { + return + } + + const state = store.getState() + state.createTab(worktreeId) + state.createBrowserTab(worktreeId, 'about:blank', { + title: 'lifecycle-test', + activate: false + }) + }, newWorktreeId) + + await openFileExplorer(orcaPage) + await clickFileInExplorer(orcaPage, ['README.md', 'package.json']) + + // Baseline: the new worktree now has tabs/browser tabs/open files. + expect((await getWorktreeTabs(orcaPage, newWorktreeId)).length).toBeGreaterThan(0) + expect((await getBrowserTabs(orcaPage, newWorktreeId)).length).toBeGreaterThan(0) + expect((await getOpenFiles(orcaPage, newWorktreeId)).length).toBeGreaterThan(0) + + // Switch away before removing so we're not deleting the active worktree — + // that's an easier code path and hides the cleanup regression this spec + // is protecting. + await switchToWorktree(orcaPage, originalWorktreeId) + await expect + .poll(async () => getActiveWorktreeId(orcaPage), { timeout: 10_000 }) + .toBe(originalWorktreeId) + + const result = await removeWorktreeViaStore(orcaPage, newWorktreeId) + expect(result.ok).toBe(true) + // Successful removal — afterEach hook no longer needs to clean this up. + createdWorktreeId = null + + // Tabs / open files / browser tabs keyed by the removed worktree must all + // be dropped. A regression that leaves any of these behind will show up + // in the sidebar as a worktree-less tab strip. + await expect + .poll(async () => (await getWorktreeTabs(orcaPage, newWorktreeId)).length, { + timeout: 10_000, + message: 'tabsByWorktree still holds entries for the removed worktree' + }) + .toBe(0) + await expect + .poll(async () => (await getBrowserTabs(orcaPage, newWorktreeId)).length, { timeout: 5_000 }) + .toBe(0) + await expect + .poll(async () => (await getOpenFiles(orcaPage, newWorktreeId)).length, { timeout: 5_000 }) + .toBe(0) + + const allIds = await getAllWorktreeIds(orcaPage) + expect(allIds).not.toContain(newWorktreeId) + }) + + /** + * Worktree switching preserves per-worktree state — specifically + * `layoutByWorktree`, `openFiles`, and the right-sidebar UI state across a + * round-trip. + * + * Why a narrowed claim: the original #598 / #628 regressions were renderer + * freezes, and #726 was split-group container teardown. Those are + * *renderer-side* bugs — a store-level test can't observe a frozen React + * render loop (if the renderer hung, `page.evaluate` would hang too, which + * looks identical to any other timeout). #726 in particular is already + * guarded at the unit level by `anyMountedWorktreeHasLayout` tests per its + * PR summary. + * + * What this test *does* catch: regressions that wipe per-worktree store + * state during a switch — e.g. a cascading reducer that clears + * `layoutByWorktree[oldWorktreeId]` when activating a new worktree, or a + * sidebar-reset side effect attached to `setActiveWorktree`. That's a + * smaller claim than "doesn't hang," but it's one this layer can actually + * verify. + */ + test('switching worktrees preserves per-worktree state across a round-trip', async ({ + orcaPage + }) => { + const allIds = await getAllWorktreeIds(orcaPage) + expect( + allIds.length, + 'fixture should provide primary + e2e-secondary worktrees' + ).toBeGreaterThanOrEqual(2) + + const originalWorktreeId = await waitForActiveWorktree(orcaPage) + + await openFileExplorer(orcaPage) + await clickFileInExplorer(orcaPage, ['README.md', 'package.json']) + + // Snapshot the original worktree's state so we can assert preservation + // after the round-trip. An empty `openFiles` here would make the second + // assertion tautological, so guard that expectation up-front. + const originalState = await orcaPage.evaluate((wId) => { + const store = window.__store + if (!store) { + // Surface a store-unavailable failure via a clear empty baseline + // rather than a null-deref inside page.evaluate. + return { openFileIds: [] as string[], hasLayout: false } + } + const state = store.getState() + return { + openFileIds: state.openFiles.filter((f) => f.worktreeId === wId).map((f) => f.id), + hasLayout: Boolean(state.layoutByWorktree?.[wId]) + } + }, originalWorktreeId) + expect( + originalState.openFileIds.length, + 'expected seeded openFiles on original worktree' + ).toBeGreaterThan(0) + + const otherWorktreeId = allIds.find((id) => id !== originalWorktreeId)! + await switchToWorktree(orcaPage, otherWorktreeId) + await expect + .poll(async () => getActiveWorktreeId(orcaPage), { timeout: 10_000 }) + .toBe(otherWorktreeId) + + // Sidebar UI state must survive the switch — user shouldn't have to + // re-open the explorer after every worktree change. + await expect + .poll( + async () => + orcaPage.evaluate(() => { + const state = window.__store?.getState() + return Boolean(state?.rightSidebarOpen && state?.rightSidebarTab === 'explorer') + }), + { timeout: 5_000, message: 'Right sidebar state was lost during worktree switch' } + ) + .toBe(true) + + await switchToWorktree(orcaPage, originalWorktreeId) + await expect + .poll(async () => getActiveWorktreeId(orcaPage), { timeout: 10_000 }) + .toBe(originalWorktreeId) + + // Original worktree's state must be intact: the openFiles it had before + // the switch are all still present, and its layout entry (if any) was + // not torn down. A regression that clears these on setActiveWorktree + // would fail here even though `activeWorktreeId` round-tripped cleanly. + const afterRoundTrip = await orcaPage.evaluate((wId) => { + const store = window.__store + if (!store) { + // Match the originalState guard so assertion failures point at + // "store gone" instead of a null-deref stack. + return { openFileIds: [] as string[], hasLayout: false } + } + const state = store.getState() + return { + openFileIds: state.openFiles.filter((f) => f.worktreeId === wId).map((f) => f.id), + hasLayout: Boolean(state.layoutByWorktree?.[wId]) + } + }, originalWorktreeId) + expect(new Set(afterRoundTrip.openFileIds)).toEqual(new Set(originalState.openFileIds)) + expect(afterRoundTrip.hasLayout).toBe(originalState.hasLayout) + }) + + /** + * Covers PR #542 / #554: a regression caused terminal tab membership to + * leak across worktrees (the wrong worktree's tab reacted to shortcuts). + * Guard the underlying invariant — tabsByWorktree[A] and tabsByWorktree[B] + * do not share IDs — at the model layer where the bug actually lived. + */ + test('terminal tabs stay scoped to the worktree that created them', async ({ orcaPage }) => { + const allIds = await getAllWorktreeIds(orcaPage) + expect( + allIds.length, + 'fixture should provide primary + e2e-secondary worktrees' + ).toBeGreaterThanOrEqual(2) + + const worktreeA = await waitForActiveWorktree(orcaPage) + const worktreeB = allIds.find((id) => id !== worktreeA)! + + // Create an extra tab on A so it has a distinctive tab ID set. + await orcaPage.evaluate((worktreeId) => { + const store = window.__store + if (!store) { + return + } + + store.getState().createTab(worktreeId) + }, worktreeA) + await expect + .poll(async () => (await getWorktreeTabs(orcaPage, worktreeA)).length, { timeout: 5_000 }) + .toBeGreaterThanOrEqual(2) + + // Switch to B and create a tab there too. + await switchToWorktree(orcaPage, worktreeB) + await expect + .poll(async () => getActiveWorktreeId(orcaPage), { timeout: 10_000 }) + .toBe(worktreeB) + await ensureTerminalVisible(orcaPage) + await orcaPage.evaluate((worktreeId) => { + const store = window.__store + if (!store) { + return + } + + store.getState().createTab(worktreeId) + }, worktreeB) + await expect + .poll(async () => (await getWorktreeTabs(orcaPage, worktreeB)).length, { timeout: 5_000 }) + .toBeGreaterThanOrEqual(2) + + const tabsA = await getWorktreeTabs(orcaPage, worktreeA) + const tabsB = await getWorktreeTabs(orcaPage, worktreeB) + const idsA = new Set(tabsA.map((tab) => tab.id)) + const idsB = new Set(tabsB.map((tab) => tab.id)) + + const overlap = [...idsA].filter((id) => idsB.has(id)) + expect(overlap, 'tabsByWorktree leaked tab IDs across worktrees').toEqual([]) + }) +})