From 9a033c9ee297f226b2d002b1ecc557f9e19e1dd2 Mon Sep 17 00:00:00 2001 From: brennanb2025 Date: Mon, 27 Apr 2026 00:07:20 -0700 Subject: [PATCH] test(sidebar): add listAll coverage and tighten recent-sort e2e - Add a unit test for worktrees:listAll to guard the stamp path parallel to worktrees:list. - Scope the e2e fixture tracking inside the describe block and clean up tmpdirs in afterEach. - Replace the cross-process clock window with `toBeGreaterThan(0)`, which captures the exact pre-fix regression (lastActivityAt === 0) without introducing NTP/clock-skew flakiness in CI. - Narrow the helper return type from ReturnType<...> to WorktreeMeta. Co-authored-by: Orca --- src/main/ipc/worktrees.test.ts | 31 +++++++++++++++++ src/main/ipc/worktrees.ts | 5 +-- tests/e2e/worktree-recent-sort.spec.ts | 46 ++++++++++++++++++-------- 3 files changed, 64 insertions(+), 18 deletions(-) diff --git a/src/main/ipc/worktrees.test.ts b/src/main/ipc/worktrees.test.ts index 8e8ee2ce698..36c642dd3d3 100644 --- a/src/main/ipc/worktrees.test.ts +++ b/src/main/ipc/worktrees.test.ts @@ -415,6 +415,37 @@ describe('registerWorktreeHandlers', () => { ) }) + it('stamps lastActivityAt on first discovery via worktrees:listAll', async () => { + // Why: the stamping logic lives in both worktrees:list and worktrees:listAll. + // Without a dedicated test, a regression in the listAll loop would silently + // bury newly-discovered worktrees from the multi-repo sidebar view. + listWorktreesMock.mockResolvedValue([ + { + path: '/workspace/discovered-wt', + head: 'abc123', + branch: 'refs/heads/feature', + isBare: false, + isMainWorktree: false + } + ]) + store.getWorktreeMeta.mockReturnValue(undefined) + store.setWorktreeMeta.mockReturnValue({ lastActivityAt: 1_700_000_000_000 }) + + const listed = (await handlers['worktrees:listAll'](null, undefined)) as { + id: string + lastActivityAt: number + }[] + + expect(store.setWorktreeMeta).toHaveBeenCalledWith( + 'repo-1::/workspace/discovered-wt', + expect.objectContaining({ lastActivityAt: expect.any(Number) }) + ) + expect(listed[0]).toMatchObject({ + id: 'repo-1::/workspace/discovered-wt', + lastActivityAt: 1_700_000_000_000 + }) + }) + it('skips past a suffix that already belongs to a PR after an initial branch conflict', async () => { // Why: `gh pr list` is network-bound and previously fired on every single // create, adding 1–3s to the happy path. We now only probe PR conflicts diff --git a/src/main/ipc/worktrees.ts b/src/main/ipc/worktrees.ts index 838d449db01..3e016118d1f 100644 --- a/src/main/ipc/worktrees.ts +++ b/src/main/ipc/worktrees.ts @@ -38,10 +38,7 @@ import { rebuildAuthorizedRootsCache, ensureAuthorizedRootsCache } from './files // added the repo / folder. Stamp discovery time the first time we see a // worktree so its very existence counts as a recency signal. Subsequent // list calls find the persisted meta and skip the stamp. -function resolveWorktreeMetaWithDiscoveryStamp( - store: Store, - worktreeId: string -): ReturnType { +function resolveWorktreeMetaWithDiscoveryStamp(store: Store, worktreeId: string): WorktreeMeta { const existing = store.getWorktreeMeta(worktreeId) if (existing) { return existing diff --git a/tests/e2e/worktree-recent-sort.spec.ts b/tests/e2e/worktree-recent-sort.spec.ts index 1dca434cd6c..82e24ca6e7b 100644 --- a/tests/e2e/worktree-recent-sort.spec.ts +++ b/tests/e2e/worktree-recent-sort.spec.ts @@ -14,20 +14,13 @@ * behavior in end-to-end. */ -import { mkdtempSync, mkdirSync, writeFileSync } from 'fs' +import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from 'fs' import os from 'os' import path from 'path' import type { Page } from '@stablyai/playwright-test' import { test, expect } from './helpers/orca-app' import { waitForSessionReady, waitForActiveWorktree } from './helpers/store' -function createFolderFixture(): string { - const dir = mkdtempSync(path.join(os.tmpdir(), 'orca-e2e-folder-')) - mkdirSync(path.join(dir, 'src'), { recursive: true }) - writeFileSync(path.join(dir, 'README.md'), '# folder fixture\n') - return dir -} - async function addFolderRepo(page: Page, folderPath: string): Promise { return page.evaluate(async (p) => { const store = window.__store @@ -61,25 +54,50 @@ async function readFolderWorktreeLastActivity(page: Page, repoId: string): Promi } test.describe('Worktree Recent Sort', () => { + // Why: keep fixture tracking scoped to this describe block. Module-level + // shared arrays race if the file ever flips to parallel mode or another + // describe is added. + const createdFolderFixtures: string[] = [] + + function createFolderFixture(): string { + const dir = mkdtempSync(path.join(os.tmpdir(), 'orca-e2e-folder-')) + createdFolderFixtures.push(dir) + mkdirSync(path.join(dir, 'src'), { recursive: true }) + writeFileSync(path.join(dir, 'README.md'), '# folder fixture\n') + return dir + } + test.beforeEach(async ({ orcaPage }) => { await waitForSessionReady(orcaPage) await waitForActiveWorktree(orcaPage) }) + test.afterEach(() => { + // Why: mkdtempSync fixtures leak unless we clean them up explicitly — + // matches the mkdtempSync/rmSync pairing used in helpers/orca-app.ts + // and helpers/orca-restart.ts. + while (createdFolderFixtures.length) { + const dir = createdFolderFixtures.pop() + if (dir) { + rmSync(dir, { recursive: true, force: true }) + } + } + }) + test('stamps lastActivityAt on a newly-added folder repo so it sorts to the top of Recent', async ({ orcaPage }) => { const folderPath = createFolderFixture() - const beforeAdd = Date.now() const repoId = await addFolderRepo(orcaPage, folderPath) const lastActivityAt = await readFolderWorktreeLastActivity(orcaPage, repoId) - // The synthetic folder worktree had no persisted meta, so the list - // handler must have stamped it on first discovery. Before the fix this - // would have been 0 (epoch), sorting the folder to the bottom of Recent. - expect(lastActivityAt).toBeGreaterThanOrEqual(beforeAdd) - expect(lastActivityAt).toBeLessThanOrEqual(Date.now()) + // Why: the exact failure mode before the fix was `lastActivityAt === 0` + // (the fallback in mergeWorktree when meta is undefined). Asserting + // `> 0` captures that regression precisely without coupling to the + // wall-clock of the main process, which would introduce cross-process + // clock-skew flakiness in CI. + expect(lastActivityAt).toBeGreaterThan(0) }) test('leaves lastActivityAt stable across repeated list refreshes', async ({ orcaPage }) => {