test(e2e): cover worktree lifecycle and tab-close navigation (#846)

This commit is contained in:
Brennan Benson 2026-04-19 15:50:19 -07:00 committed by GitHub
parent a8fe60932f
commit 08b0a3058a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 654 additions and 0 deletions

View file

@ -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<typeof getActiveWorktreeId>[0],
worktreeId: string,
relativePaths: string[]
): Promise<string[]> {
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<typeof getActiveWorktreeId>[0],
fileId: string
): Promise<void> {
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<typeof getActiveWorktreeId>[0],
fileId: string
): Promise<void> {
await page.evaluate((id) => {
window.__store?.getState().closeFile(id)
}, fileId)
}
async function getActiveFileId(
page: Parameters<typeof getActiveWorktreeId>[0]
): Promise<string | null> {
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()
})
})

View file

@ -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<typeof getActiveWorktreeId>[0]
): Promise<string> {
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<typeof getActiveWorktreeId>[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([])
})
})