diff --git a/src/main/git/worktree-base-divergence-real-git.test.ts b/src/main/git/worktree-base-divergence-real-git.test.ts new file mode 100644 index 00000000000..e17192cf914 --- /dev/null +++ b/src/main/git/worktree-base-divergence-real-git.test.ts @@ -0,0 +1,99 @@ +import { execFileSync } from 'node:child_process' +import { mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, it } from 'vitest' +import { + measureRetargetDivergence, + RETARGET_MAX_COMMIT_DIVERGENCE +} from './worktree-base-divergence' + +const tempRoots: string[] = [] + +function git(cwd: string, args: string[]): string { + return execFileSync('git', args, { + cwd, + encoding: 'utf8', + stdio: ['pipe', 'pipe', 'pipe'] + }).trim() +} + +async function createRepo(): Promise { + const root = await mkdtemp(join(tmpdir(), 'orca-base-divergence-')) + tempRoots.push(root) + const repoPath = join(root, 'repo') + execFileSync('git', ['init', '--quiet', repoPath]) + git(repoPath, ['symbolic-ref', 'HEAD', 'refs/heads/main']) + git(repoPath, ['config', 'user.email', 'test@example.com']) + git(repoPath, ['config', 'user.name', 'Test User']) + await writeFile(join(repoPath, 'version.txt'), 'one\n') + git(repoPath, ['add', 'version.txt']) + git(repoPath, ['commit', '--quiet', '-m', 'initial']) + return repoPath +} + +function commitEmpty(repoPath: string, count: number): void { + for (let index = 0; index < count; index += 1) { + git(repoPath, ['commit', '--quiet', '--allow-empty', '-m', `commit ${index}`]) + } +} + +afterEach(async () => { + await Promise.all(tempRoots.splice(0).map((root) => rm(root, { recursive: true, force: true }))) +}) + +describe('measureRetargetDivergence with real Git', () => { + it('allows the drift between a local branch and its remote-tracking copy', async () => { + const repoPath = await createRepo() + git(repoPath, ['update-ref', 'refs/remotes/origin/main', 'HEAD']) + commitEmpty(repoPath, 5) + git(repoPath, ['update-ref', 'refs/remotes/origin/main', 'HEAD']) + git(repoPath, ['reset', '--hard', '--quiet', 'HEAD~3']) + + await expect( + measureRetargetDivergence(repoPath, 'refs/heads/main', 'refs/remotes/origin/main') + ).resolves.toBe('within') + }) + + it('counts drift in both directions', async () => { + const repoPath = await createRepo() + const forkPoint = git(repoPath, ['rev-parse', 'HEAD']) + commitEmpty(repoPath, RETARGET_MAX_COMMIT_DIVERGENCE) + git(repoPath, ['update-ref', 'refs/remotes/origin/main', 'HEAD']) + git(repoPath, ['reset', '--hard', '--quiet', forkPoint]) + commitEmpty(repoPath, 1) + + // 100 ahead + 1 behind is over the cap even though neither side alone exceeds it. + await expect( + measureRetargetDivergence(repoPath, 'refs/heads/main', 'refs/remotes/origin/main') + ).resolves.toBe('exceeded') + }) + + it('refuses a base that has drifted past the cap', async () => { + const repoPath = await createRepo() + git(repoPath, ['update-ref', 'refs/remotes/origin/main', 'HEAD']) + commitEmpty(repoPath, RETARGET_MAX_COMMIT_DIVERGENCE + 1) + + await expect( + measureRetargetDivergence(repoPath, 'refs/remotes/origin/main', 'refs/heads/main') + ).resolves.toBe('exceeded') + }) + + it('refuses unrelated histories, which share no commits at all', async () => { + const repoPath = await createRepo() + git(repoPath, ['checkout', '--quiet', '--orphan', 'unrelated']) + git(repoPath, ['commit', '--quiet', '--allow-empty', '-m', 'unrelated root']) + + await expect( + measureRetargetDivergence(repoPath, 'refs/heads/main', 'refs/heads/unrelated') + ).resolves.toBe('exceeded') + }) + + it('reports an unreadable ref as unverifiable, not as excess drift', async () => { + const repoPath = await createRepo() + + await expect( + measureRetargetDivergence(repoPath, 'refs/heads/main', 'refs/heads/missing') + ).resolves.toBe('unknown') + }) +}) diff --git a/src/main/git/worktree-base-divergence.test.ts b/src/main/git/worktree-base-divergence.test.ts new file mode 100644 index 00000000000..88eec6a6b11 --- /dev/null +++ b/src/main/git/worktree-base-divergence.test.ts @@ -0,0 +1,181 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ gitExecFileAsync: vi.fn() })) + +vi.mock('./runner', () => ({ gitExecFileAsync: mocks.gitExecFileAsync })) + +import { GIT_READ_TIMEOUT_MS } from './command-runner/git-command-timeout' +import { WSL_GIT_READ_ENVIRONMENT_WAIT_MS } from './wsl-git-read-environment' +import { + measureRetargetDivergence, + RETARGET_DIVERGENCE_BUDGET_MS +} from './worktree-base-divergence' + +type ExecOptions = { cwd: string; timeout?: number; wslDistro?: string; signal?: AbortSignal } + +function callOptions(): ExecOptions[] { + return mocks.gitExecFileAsync.mock.calls.map((call) => call[1] as ExecOptions) +} + +function subcommands(): string[] { + return mocks.gitExecFileAsync.mock.calls.map((call) => (call[0] as string[])[0]!) +} + +function answerProbes(count: string, mergeBase = 'abc123\n') { + mocks.gitExecFileAsync.mockImplementation(async (args: string[]) => + args[0] === 'merge-base' ? { stdout: mergeBase } : { stdout: count } + ) +} + +function exitCodeError(code: number): Error & { code: number } { + return Object.assign(new Error('git exited'), { code }) +} + +beforeEach(() => { + mocks.gitExecFileAsync.mockReset() +}) + +describe('measureRetargetDivergence deadlines', () => { + it('puts every probe under one shared budget, not a budget each', async () => { + answerProbes('3\n') + + await expect( + measureRetargetDivergence('/repo', 'refs/heads/main', 'refs/remotes/origin/main') + ).resolves.toBe('within') + + expect(subcommands()).toEqual(['rev-list', 'rev-list', 'merge-base']) + const signals = callOptions().map((options) => options.signal) + // One signal object across all three: the counts and merge-base are staged, so per-probe + // budgets would let the check cost the sum of them. + expect(new Set(signals).size).toBe(1) + expect(signals[0]).toBeInstanceOf(AbortSignal) + }) + + it('also gives each probe a command timeout well below git default read deadline', async () => { + answerProbes('3\n') + + await measureRetargetDivergence('/repo', 'refs/heads/main', 'refs/remotes/origin/main') + + // The signal covers admission queueing and the WSL environment wait, which start before a + // command timeout exists; the timeout still covers a hung spawn. + for (const options of callOptions()) { + expect(options.timeout).toBe(RETARGET_DIVERGENCE_BUDGET_MS) + } + expect(RETARGET_DIVERGENCE_BUDGET_MS).toBeLessThan(GIT_READ_TIMEOUT_MS) + }) + + it('really aborts the in-flight probes when the shared budget expires', async () => { + // A probe that behaves like a slow walk: it produces nothing on its own and only settles when + // its signal fires. If the budget never fired, or never reached the probe, this hangs and the + // test fails on its own timeout rather than passing on a signal that does nothing. + mocks.gitExecFileAsync.mockImplementation( + (_args: string[], options: ExecOptions) => + new Promise((_resolve, reject) => { + options.signal?.addEventListener( + 'abort', + () => + reject( + Object.assign(new Error('The operation was aborted.'), { name: 'AbortError' }) + ), + { once: true } + ) + }) + ) + + await expect( + measureRetargetDivergence('/repo', 'refs/heads/main', 'refs/remotes/origin/main', { + budgetMsForTest: 25 + }) + ).resolves.toBe('unknown') + }) + + it('clears the WSL read-environment wait, which starts before any command timeout', () => { + // Equal to it would make the first WSL-routed create of a session `unknown` by construction, + // and the WSL numbers meaningless. + expect(RETARGET_DIVERGENCE_BUDGET_MS).toBeGreaterThan(WSL_GIT_READ_ENVIRONMENT_WAIT_MS) + expect(RETARGET_DIVERGENCE_BUDGET_MS).toBeLessThan(GIT_READ_TIMEOUT_MS) + }) + + it('stops the probes when the create itself is cancelled, without waiting for the budget', async () => { + mocks.gitExecFileAsync.mockImplementation( + (_args: string[], options: ExecOptions) => + new Promise((_resolve, reject) => { + options.signal?.addEventListener( + 'abort', + () => + reject( + Object.assign(new Error('The operation was aborted.'), { name: 'AbortError' }) + ), + { once: true } + ) + }) + ) + const controller = new AbortController() + const pending = measureRetargetDivergence( + '/repo', + 'refs/heads/main', + 'refs/remotes/origin/main', + // A budget long enough that only the caller's cancellation can end this in time. + { signal: controller.signal, budgetMsForTest: 60_000 } + ) + controller.abort() + + await expect(pending).resolves.toBe('unknown') + }) + + it('reports a blown deadline as unverifiable rather than as excess drift', async () => { + mocks.gitExecFileAsync.mockRejectedValue(new Error('git timed out.')) + + await expect( + measureRetargetDivergence('/repo', 'refs/heads/main', 'refs/remotes/origin/main') + ).resolves.toBe('unknown') + // A count that never answered must not go on to spend a merge-base walk. + expect(subcommands()).not.toContain('merge-base') + }) + + it('separates merge-base saying no from merge-base failing', async () => { + mocks.gitExecFileAsync.mockImplementation(async (args: string[]) => { + if (args[0] === 'merge-base') { + // Exit 1 is Git's answer for unrelated histories. + throw exitCodeError(1) + } + return { stdout: '2\n' } + }) + await expect( + measureRetargetDivergence('/repo', 'refs/heads/main', 'refs/remotes/origin/main') + ).resolves.toBe('exceeded') + + mocks.gitExecFileAsync.mockReset() + mocks.gitExecFileAsync.mockImplementation(async (args: string[]) => { + if (args[0] === 'merge-base') { + // A timeout carries no exit code and must not be read as "no common ancestor". + throw new Error('git timed out.') + } + return { stdout: '2\n' } + }) + await expect( + measureRetargetDivergence('/repo', 'refs/heads/main', 'refs/remotes/origin/main') + ).resolves.toBe('unknown') + }) + + it('reports drift past the cap without spending a merge-base walk', async () => { + answerProbes('101\n') + + await expect( + measureRetargetDivergence('/repo', 'refs/heads/main', 'refs/remotes/origin/main') + ).resolves.toBe('exceeded') + expect(subcommands()).not.toContain('merge-base') + }) + + it('routes every probe to the caller-named WSL distro', async () => { + answerProbes('1\n') + + await measureRetargetDivergence('/repo', 'refs/heads/main', 'refs/remotes/origin/main', { + wslDistro: 'Ubuntu' + }) + + for (const options of callOptions()) { + expect(options).toMatchObject({ cwd: '/repo', wslDistro: 'Ubuntu' }) + } + }) +}) diff --git a/src/main/git/worktree-base-divergence.ts b/src/main/git/worktree-base-divergence.ts new file mode 100644 index 00000000000..c7c924c2f24 --- /dev/null +++ b/src/main/git/worktree-base-divergence.ts @@ -0,0 +1,165 @@ +import { WSL_GIT_READ_ENVIRONMENT_WAIT_MS } from './wsl-git-read-environment' +import { gitExecFileAsync } from './runner' + +export type RetargetDivergenceOptions = { + wslDistro?: string + /** The create's own cancellation signal. Without it a cancelled create leaves these probes + * running until the budget expires. */ + signal?: AbortSignal + /** Shortens only the end-to-end budget so a test can observe a real abort; production always + * uses the constant. Mirrors `timeoutMsForTest` on the git exec options. */ + budgetMsForTest?: number +} + +/** `unknown` is deliberately not folded into `exceeded`: "the bound says no" and "the bound could + * not be evaluated" have different causes and different fixes, and only the second one means a + * retarget that would have been cheap was skipped. `unknown` covers a blown deadline, a + * cancelled create, and an ordinary Git failure alike — it is "no answer", not "slow". */ +export type RetargetDivergence = 'within' | 'exceeded' | 'unknown' + +/** + * How far two bases may drift and still be worth retargeting a prepared checkout between. + * + * Measured on a 21,715-file repo: a local `main` and its `origin/main` were 5 commits and 74 + * files apart, while an abandoned fork's `main` — same branch name, so the same base family — + * was 8,173 commits and 21,708 files from `origin/main`, i.e. a whole-tree checkout. A commit + * count separates those by three orders of magnitude, so it is the cheap proxy for the tree diff + * the retarget reset would have to write. + */ +export const RETARGET_MAX_COMMIT_DIVERGENCE = 100 + +/** + * Headroom for the walk itself, on top of the worst pre-spawn wait. + * + * ~3x the slowest walk measured on the 12GB/80k-ref repo (180ms to reject, 57ms to allow), so a + * cold WSL environment probe cannot eat the whole budget and make the answer `unknown` by + * construction. + */ +const RETARGET_DIVERGENCE_WALK_HEADROOM_MS = 500 + +/** + * End-to-end deadline for the whole check, not per probe. + * + * Derived from the WSL read-environment wait rather than picked: `git-exec-file` awaits that probe + * before a command timeout even exists, so a budget merely equal to it would guarantee `unknown` + * on the first WSL-routed create of a session and make the WSL numbers meaningless. Deriving it + * keeps that relationship explicit instead of coincidental. + * + * Sized against what it competes with: this exists only to decide whether to skip a ~4.1s p50 cold + * `worktree add`, so when it expires the create pays the budget and then does that add anyway. The + * total stays under that add even on Windows, where a killed probe also awaits `taskkill /t`. + * + * It must be a signal, not just a per-command timeout, because a per-command timeout starts only + * after `git-exec-file` has awaited admission and the WSL read-environment probe, and because the + * counts and `merge-base` are staged — two per-probe budgets in sequence would be twice the number + * written here. + */ +export const RETARGET_DIVERGENCE_BUDGET_MS = + WSL_GIT_READ_ENVIRONMENT_WAIT_MS + RETARGET_DIVERGENCE_WALK_HEADROOM_MS + +function probeOptions( + repoPath: string, + options: RetargetDivergenceOptions, + signal: AbortSignal +): { cwd: string; wslDistro?: string; signal: AbortSignal; timeout: number } { + // Built field by field rather than spread: the caller's bag carries a test-only key that must + // never reach git's exec options. + // Both bounds: the signal covers the pre-spawn waits (admission queue, WSL environment) that a + // command timeout cannot see, and the timeout keeps the bounded tree-kill path for a hung spawn. + return { + cwd: repoPath, + ...(options.wslDistro ? { wslDistro: options.wslDistro } : {}), + signal, + timeout: RETARGET_DIVERGENCE_BUDGET_MS + } +} + +/** Commits reachable from `toRef` but not `fromRef`, capped; null when the probe was unusable. */ +async function countCommitsAhead( + repoPath: string, + fromRef: string, + toRef: string, + options: RetargetDivergenceOptions, + signal: AbortSignal +): Promise { + try { + // `--max-count` stops the walk, so an unrelated history costs a bounded number of commits + // rather than a full traversal. Both flags predate the Git 2.25 baseline. + // `--end-of-options` (Git 2.24) because a range whose left side began with `-` would + // otherwise parse as an option; callers only pass `refs/`-qualified names today, and this + // keeps that from being load-bearing. + const { stdout } = await gitExecFileAsync( + [ + 'rev-list', + '--count', + `--max-count=${RETARGET_MAX_COMMIT_DIVERGENCE + 1}`, + '--end-of-options', + `${fromRef}..${toRef}` + ], + probeOptions(repoPath, options, signal) + ) + const count = Number.parseInt(stdout.trim(), 10) + return Number.isNaN(count) ? null : count + } catch { + return null + } +} + +/** True/false when Git decided, null when the probe was unusable. */ +async function hasCommonHistory( + repoPath: string, + leftRef: string, + rightRef: string, + options: RetargetDivergenceOptions, + signal: AbortSignal +): Promise { + try { + const { stdout } = await gitExecFileAsync( + ['merge-base', '--end-of-options', leftRef, rightRef], + probeOptions(repoPath, options, signal) + ) + return stdout.trim().length > 0 + } catch (error) { + // Exit 1 is `merge-base` reporting no common ancestor, which is an answer. A timeout or abort + // carries no exit code and must not be read as one. + return (error as { code?: unknown }).code === 1 ? false : null + } +} + +/** + * Whether retargeting a checkout prepared at `preparedBase` onto `targetBase` stays cheap. + * + * Fails closed on error, slowness, and cancellation alike: only a positive `within` authorizes + * reusing the checkout, so every other outcome lands on the cold create path. + */ +export async function measureRetargetDivergence( + repoPath: string, + preparedBase: string, + targetBase: string, + options: RetargetDivergenceOptions = {} +): Promise { + const budget = AbortSignal.timeout(options.budgetMsForTest ?? RETARGET_DIVERGENCE_BUDGET_MS) + // Combined so cancelling the create stops the probes immediately rather than at the deadline. + const signal = options.signal ? AbortSignal.any([options.signal, budget]) : budget + // Both directions: commits the target adds decide what the reset writes, commits only the + // preparation has decide what it must delete. + const [ahead, behind] = await Promise.all([ + countCommitsAhead(repoPath, preparedBase, targetBase, options, signal), + countCommitsAhead(repoPath, targetBase, preparedBase, options, signal) + ]) + if (ahead === null || behind === null) { + return 'unknown' + } + if (ahead + behind > RETARGET_MAX_COMMIT_DIVERGENCE) { + return 'exceeded' + } + // Only now: `merge-base` has no `--max-count`, so on unrelated histories it would walk both of + // them in full. Reaching here already proved neither side is more than the cap ahead of the + // other, which bounds that walk — and unrelated histories of any size fail the counts first. + // Required because unrelated histories replace the whole tree however few commits they carry. + const shareHistory = await hasCommonHistory(repoPath, preparedBase, targetBase, options, signal) + if (shareHistory === null) { + return 'unknown' + } + return shareHistory ? 'within' : 'exceeded' +} diff --git a/src/main/git/worktree-base-ref-probe.ts b/src/main/git/worktree-base-ref-probe.ts index 8e44877ab76..87c761ebbcc 100644 --- a/src/main/git/worktree-base-ref-probe.ts +++ b/src/main/git/worktree-base-ref-probe.ts @@ -42,6 +42,21 @@ export async function hasWorktreeBaseCommitRef( return (await resolveWorktreeBaseCommitOid(repoPath, qualifiedRef, options)) !== null } +/** + * The qualified ref a worktree base names in this repo, or the base unchanged when nothing + * matches. Callers that key on a base must compare this, not the raw string, or `main` and + * `refs/heads/main` look like different bases. + */ +export function resolveLocalWorktreeBaseRef( + repoPath: string, + baseRef: string, + options: GitExecOptions = {} +): Promise { + return resolveWorktreeAddBaseRef(baseRef, (qualifiedRef) => + hasWorktreeBaseCommitRef(repoPath, qualifiedRef, options) + ) +} + /** * Whether a worktree base — a qualified ref, a short branch or remote name, or a * full commit id — already resolves in this repo's own object/ref store. diff --git a/src/main/git/worktree-create-preparation-real-git.test.ts b/src/main/git/worktree-create-preparation-real-git.test.ts index bdb1e9fcc4b..63f8021a7ae 100644 --- a/src/main/git/worktree-create-preparation-real-git.test.ts +++ b/src/main/git/worktree-create-preparation-real-git.test.ts @@ -68,6 +68,55 @@ describe('prepared worktree creation with real Git', () => { expect(await listWorktrees(repoPath, { includeCreatePreparations: true })).toHaveLength(1) }) + it('lands a cross-base retarget on exactly the requested commit', async () => { + const { repoPath, root } = await createRepo() + const preparationRoot = join(root, WORKTREE_CREATE_PREPARATION_DIRECTORY) + const preparedPath = join(preparationRoot, `${process.pid}-retarget`) + const finalPath = join(root, 'retargeted-worktree') + await mkdir(preparationRoot, { recursive: true }) + + await writeFile(join(repoPath, 'shared.txt'), 'kept\n') + git(repoPath, ['add', 'shared.txt']) + git(repoPath, ['commit', '--quiet', '-m', 'local main']) + const localMainHead = git(repoPath, ['rev-parse', 'HEAD']) + + // A remote-tracking `main` that diverged: different content, an extra file, and one deletion. + git(repoPath, ['checkout', '--quiet', '-b', 'upstream-main']) + await writeFile(join(repoPath, 'version.txt'), 'two\n') + await writeFile(join(repoPath, 'only-upstream.txt'), 'upstream\n') + git(repoPath, ['rm', '--quiet', 'shared.txt']) + git(repoPath, ['add', 'version.txt', 'only-upstream.txt']) + git(repoPath, ['commit', '--quiet', '-m', 'upstream main']) + git(repoPath, ['update-ref', 'refs/remotes/origin/main', 'HEAD']) + git(repoPath, ['checkout', '--quiet', 'main']) + git(repoPath, ['branch', '--quiet', '-D', 'upstream-main']) + + await prepareWorktreeCreateCheckout( + repoPath, + preparedPath, + 'refs/remotes/origin/main', + createWorktreePreparationLockReason('retarget-test') + ) + expect(git(preparedPath, ['rev-parse', 'HEAD'])).not.toBe(localMainHead) + + await finalizePreparedWorktree(repoPath, preparedPath, finalPath, 'feature/retargeted', 'main') + + expect(git(finalPath, ['rev-parse', 'HEAD'])).toBe(localMainHead) + // A retarget that left stale files behind would be a wrong checkout, not just a slow one. + expect(git(finalPath, ['status', '--porcelain'])).toBe('') + expect((await readFile(join(finalPath, 'version.txt'), 'utf8')).replaceAll('\r\n', '\n')).toBe( + 'one\n' + ) + expect((await readFile(join(finalPath, 'shared.txt'), 'utf8')).replaceAll('\r\n', '\n')).toBe( + 'kept\n' + ) + await expect(readFile(join(finalPath, 'only-upstream.txt'), 'utf8')).rejects.toThrow() + expect(git(finalPath, ['branch', '--show-current'])).toBe('feature/retargeted') + expect(git(finalPath, ['config', '--get', 'branch.feature/retargeted.base'])).toBe( + 'refs/heads/main' + ) + }) + it('hides the preparation, retargets an advanced base, and attaches the final branch', async () => { const { repoPath, root } = await createRepo() const preparationRoot = join(root, WORKTREE_CREATE_PREPARATION_DIRECTORY) diff --git a/src/main/ipc/worktree-remote.ts b/src/main/ipc/worktree-remote.ts index 7d28f38f2db..ce6ee7b3394 100644 --- a/src/main/ipc/worktree-remote.ts +++ b/src/main/ipc/worktree-remote.ts @@ -2402,7 +2402,7 @@ export async function createLocalWorktree( addResult = (await timing.time('git_worktree_add', async () => { if (sparseDirectories.length === 0 && !checkoutExistingBranch) { - const preparedResult = await consumePreparedWorktreeCreate({ + const prepared = await consumePreparedWorktreeCreate({ repoPath: repo.path, workspaceRoot, worktreePath, @@ -2411,9 +2411,19 @@ export async function createLocalWorktree( refreshLocalBaseRef: settings.refreshLocalBaseRefOnWorktreeCreate, ...(preparedWorktreeOptions ? { options: preparedWorktreeOptions } : {}) }) - if (preparedResult) { - return preparedResult + timing.recordPreparedCheckout( + prepared.status === 'hit' + ? { status: 'hit', retargeted: prepared.retargeted } + : { status: 'miss', reason: prepared.reason } + ) + if (prepared.status === 'hit') { + return prepared.result } + } else { + timing.recordPreparedCheckout({ + status: 'miss', + reason: sparseDirectories.length > 0 ? 'sparse_checkout' : 'checkout_existing_branch' + }) } if (sparseDirectories.length > 0) { if (checkoutExistingBranch) { diff --git a/src/main/ipc/worktrees-local-create-flow.test.ts b/src/main/ipc/worktrees-local-create-flow.test.ts index d01efc489da..abc711bbd9b 100644 --- a/src/main/ipc/worktrees-local-create-flow.test.ts +++ b/src/main/ipc/worktrees-local-create-flow.test.ts @@ -633,7 +633,10 @@ describe('registerWorktreeHandlers', () => { })) as { setup?: unknown startupTerminal?: { spawned: boolean; surface?: string } - timing?: { phases: { phase: string }[] } + timing?: { + phases: { phase: string }[] + preparedCheckout?: { status: string; reason?: string } + } } expect(createSetupRunnerScriptMock).toHaveBeenCalledWith( expect.objectContaining({ id: 'repo-1' }), @@ -695,6 +698,8 @@ describe('registerWorktreeHandlers', () => { 'spawn_startup_terminal' ]) ) + // Nothing warmed this repo, so the create must report the cold path rather than stay silent. + expect(result.timing?.preparedCheckout).toEqual({ status: 'miss', reason: 'none_armed' }) }) it('returns the wrapped setup command when startup spawned but setup creation failed', async () => { diff --git a/src/main/ipc/worktrees-setup-launch-sparse-checkout.test.ts b/src/main/ipc/worktrees-setup-launch-sparse-checkout.test.ts index 0996df03cb4..725f2df8c1f 100644 --- a/src/main/ipc/worktrees-setup-launch-sparse-checkout.test.ts +++ b/src/main/ipc/worktrees-setup-launch-sparse-checkout.test.ts @@ -205,14 +205,14 @@ describe('registerWorktreeHandlers', () => { } ]) - const result = await handlers['worktrees:create'](null, { + const result = (await handlers['worktrees:create'](null, { repoId: 'repo-1', name: 'improve-dashboard', sparseCheckout: { directories: [' packages/web ', 'apps\\api\\', 'packages/web/'], presetId: 'preset-1' } - }) + })) as { timing?: { preparedCheckout?: { status: string; reason?: string } } } expect(addWorktreeMock).not.toHaveBeenCalled() expect(addSparseWorktreeMock).toHaveBeenCalledWith( @@ -240,6 +240,11 @@ describe('registerWorktreeHandlers', () => { sparsePresetId: 'preset-1' }) }) + // A sparse create can never claim a prepared checkout; say so rather than looking like a miss. + expect(result.timing?.preparedCheckout).toEqual({ + status: 'miss', + reason: 'sparse_checkout' + }) }) it('retires a generated sparse name when creation rollback also fails', async () => { diff --git a/src/main/observability/instrumentation.test.ts b/src/main/observability/instrumentation.test.ts index 978854897e4..452b5cd155c 100644 --- a/src/main/observability/instrumentation.test.ts +++ b/src/main/observability/instrumentation.test.ts @@ -214,4 +214,38 @@ describe('addWorktreeCreatePhaseAttributes', () => { expect(attributes['worktree.create.total_ms']).toBe(500) expect(attributes['worktree.create.unattributed_ms']).toBe(200) }) + + it('records a prepared-checkout hit and whether it had to be retargeted', () => { + const { attributes, span } = capture() + addWorktreeCreatePhaseAttributes(span, { + totalDurationMs: 900, + phases: [{ phase: 'git_worktree_add', startedAtMs: 0, durationMs: 400 }], + preparedCheckout: { status: 'hit', retargeted: true } + }) + + expect(attributes['worktree.create.prepared_checkout']).toBe('hit') + expect(attributes['worktree.create.prepared_checkout_retargeted']).toBe(true) + expect(attributes['worktree.create.prepared_checkout_miss']).toBeUndefined() + expect(attributes['worktree.create.unattributed_ms']).toBe(500) + }) + + it('records why a create missed the prepared checkout', () => { + const { attributes, span } = capture() + addWorktreeCreatePhaseAttributes(span, { + totalDurationMs: 8_000, + phases: [], + preparedCheckout: { status: 'miss', reason: 'base_mismatch' } + }) + + expect(attributes['worktree.create.prepared_checkout']).toBe('miss') + expect(attributes['worktree.create.prepared_checkout_miss']).toBe('base_mismatch') + expect(attributes['worktree.create.prepared_checkout_retargeted']).toBeUndefined() + }) + + it('stays silent on paths that never consult the prepared checkout', () => { + const { attributes, span } = capture() + addWorktreeCreatePhaseAttributes(span, { totalDurationMs: 10, phases: [] }) + + expect(attributes['worktree.create.prepared_checkout']).toBeUndefined() + }) }) diff --git a/src/main/observability/instrumentation.ts b/src/main/observability/instrumentation.ts index fb57aa5a870..8569ab6ef04 100644 --- a/src/main/observability/instrumentation.ts +++ b/src/main/observability/instrumentation.ts @@ -21,6 +21,7 @@ // itself becomes a `noopSpan` that swallows all calls — call sites do not // need to branch on whether tracing is on. +import type { PreparedCheckoutOutcome } from '../../shared/worktree/create-types' import { startSpan, withSpan, type ActiveSpan } from './tracer' const GIT_FAST_SUCCESS_THRESHOLD_MS = 250 @@ -259,9 +260,25 @@ type WorktreePhaseInterval = Pick 0) { addResult = (await (addOptions diff --git a/src/main/worktree-create-preparation-claim.test.ts b/src/main/worktree-create-preparation-claim.test.ts new file mode 100644 index 00000000000..8abc42f30fe --- /dev/null +++ b/src/main/worktree-create-preparation-claim.test.ts @@ -0,0 +1,152 @@ +import { describe, expect, it } from 'vitest' +import { + preparationPathKey, + selectPreparationForCreate, + type PreparationCandidate, + type PreparationRequest +} from './worktree-create-preparation-claim' + +function candidate(overrides: Partial = {}): PreparationCandidate { + return { + repoPathKey: '/repo', + workspaceRootKey: '/workspace', + wslDistro: '', + baseBranch: 'origin/main', + canonicalBase: 'refs/remotes/origin/main', + createdAt: 1_000, + ...overrides + } +} + +function request(overrides: Partial = {}): PreparationRequest { + return { + repoPathKey: '/repo', + workspaceRootKey: '/workspace', + wslDistro: '', + baseBranch: 'origin/main', + canonicalBase: 'refs/remotes/origin/main', + ...overrides + } +} + +describe('selectPreparationForCreate', () => { + it('matches the identical base before any ref probe has run', () => { + const selection = selectPreparationForCreate([candidate()], request({ canonicalBase: null })) + + expect(selection).toEqual({ + kind: 'exact', + candidate: candidate(), + canonicalBase: 'refs/remotes/origin/main' + }) + }) + + it('asks for a canonical base only when something is armed under another spelling', () => { + expect( + selectPreparationForCreate( + [candidate()], + request({ baseBranch: 'main', canonicalBase: null }) + ) + ).toEqual({ kind: 'needs-canonical-base' }) + // Nothing armed for this repo, so the create must not pay a probe to learn that. + expect( + selectPreparationForCreate([], request({ baseBranch: 'main', canonicalBase: null })) + ).toEqual({ kind: 'miss', reason: 'none_armed' }) + }) + + it('matches when the two sides spell the same ref differently', () => { + const selection = selectPreparationForCreate( + [candidate()], + request({ baseBranch: 'refs/remotes/origin/main' }) + ) + + expect(selection).toEqual({ + kind: 'exact', + candidate: candidate(), + canonicalBase: 'refs/remotes/origin/main' + }) + }) + + it('retargets a local base onto the armed remote-tracking base of the same branch', () => { + const selection = selectPreparationForCreate( + [candidate()], + request({ baseBranch: 'main', canonicalBase: 'refs/heads/main' }) + ) + + expect(selection).toEqual({ + kind: 'retarget', + candidate: candidate(), + canonicalBase: 'refs/heads/main' + }) + }) + + it('prefers the freshest armed entry when several share the family', () => { + const older = candidate({ canonicalBase: 'refs/remotes/origin/main', createdAt: 1 }) + const newer = candidate({ canonicalBase: 'refs/remotes/upstream/main', createdAt: 2 }) + + const selection = selectPreparationForCreate( + [older, newer], + request({ baseBranch: 'main', canonicalBase: 'refs/heads/main' }) + ) + + expect(selection).toMatchObject({ kind: 'retarget', candidate: newer }) + }) + + it('refuses to retarget onto a different branch', () => { + const selection = selectPreparationForCreate( + [candidate()], + request({ baseBranch: 'origin/release', canonicalBase: 'refs/remotes/origin/release' }) + ) + + expect(selection).toEqual({ kind: 'miss', reason: 'base_mismatch' }) + }) + + it('refuses to retarget onto a bare commit id, whose divergence is unbounded', () => { + const selection = selectPreparationForCreate( + [candidate()], + request({ baseBranch: '1f2e3d4c5b6a7988', canonicalBase: '1f2e3d4c5b6a7988' }) + ) + + expect(selection).toEqual({ kind: 'miss', reason: 'base_mismatch' }) + }) + + it('names the key field that disagreed', () => { + expect(selectPreparationForCreate([], request())).toEqual({ + kind: 'miss', + reason: 'none_armed' + }) + // Something is warm, just not for this repo — the shape of a size-cap eviction. + expect( + selectPreparationForCreate([candidate()], request({ repoPathKey: '/other-repo' })) + ).toEqual({ kind: 'miss', reason: 'repo_mismatch' }) + expect(selectPreparationForCreate([candidate()], request({ wslDistro: 'Ubuntu' }))).toEqual({ + kind: 'miss', + reason: 'wsl_distro_mismatch' + }) + expect( + selectPreparationForCreate([candidate()], request({ workspaceRootKey: '/other' })) + ).toEqual({ kind: 'miss', reason: 'workspace_root_mismatch' }) + }) + + it('never crosses hosts to satisfy a family retarget', () => { + const selection = selectPreparationForCreate( + [candidate({ wslDistro: 'Ubuntu' })], + request({ baseBranch: 'main', canonicalBase: 'refs/heads/main' }) + ) + + expect(selection).toEqual({ kind: 'miss', reason: 'wsl_distro_mismatch' }) + }) +}) + +describe('preparationPathKey', () => { + it('normalizes a posix path without folding case', () => { + expect(preparationPathKey('/workspace/./repo/')).toBe('/workspace/repo/') + expect(preparationPathKey('/Workspace/Repo')).toBe('/Workspace/Repo') + }) + + it('folds case for Windows drive and UNC paths, which compare case-insensitively', () => { + expect(preparationPathKey('C:\\Workspace\\Repo')).toBe('c:\\workspace\\repo') + expect(preparationPathKey('\\\\wsl.localhost\\Ubuntu\\home\\jin')).toBe( + '\\\\wsl.localhost\\ubuntu\\home\\jin' + ) + }) +}) diff --git a/src/main/worktree-create-preparation-claim.ts b/src/main/worktree-create-preparation-claim.ts new file mode 100644 index 00000000000..e329282982d --- /dev/null +++ b/src/main/worktree-create-preparation-claim.ts @@ -0,0 +1,130 @@ +import { posix, win32 } from 'node:path' +import { isWindowsAbsolutePathLike } from '../shared/cross-platform-path' +import { worktreeBaseRefFamily } from '../shared/worktree/base-ref' +import type { PreparedCheckoutMissReason } from '../shared/worktree/create-types' + +/** The subset of miss reasons this selection can produce; the rest are decided by the caller + * (sparse/existing-branch skips) or by the finalize step. */ +export type PreparationSelectionMissReason = Extract< + PreparedCheckoutMissReason, + | 'none_armed' + | 'repo_mismatch' + | 'base_mismatch' + | 'workspace_root_mismatch' + | 'wsl_distro_mismatch' +> + +export type PreparationCandidate = { + repoPathKey: string + workspaceRootKey: string + wslDistro: string + /** The base exactly as the prefetch handler armed it. */ + baseBranch: string + /** That base after `resolveWorktreeAddBaseRef`, so `main` and `refs/heads/main` compare equal. */ + canonicalBase: string + createdAt: number +} + +export type PreparationRequest = { + repoPathKey: string + workspaceRootKey: string + wslDistro: string + baseBranch: string + /** `null` until the caller has paid the ref probe. A raw-base match resolves without it, so the + * common hit spawns no git at all. */ + canonicalBase: string | null +} + +/** Case-folded on Windows, so the arming and claiming sides key on the same path. */ +export function preparationPathKey(path: string): string { + if (isWindowsAbsolutePathLike(path)) { + return win32.normalize(path).toLowerCase() + } + return posix.normalize(path) +} + +/** Keyed on the canonical base so the prefetch and the create agree when they spell the same ref + * differently; a genuinely different ref still gets its own entry. */ +export function preparationEntryKey( + repoPathKey: string, + workspaceRootKey: string, + canonicalBase: string, + wslDistro: string +): string { + return `${repoPathKey}\0${workspaceRootKey}\0${canonicalBase}\0${wslDistro}` +} + +export type PreparationSelection = + /** `canonicalBase` is echoed back so the caller can re-arm on the create's own base without + * paying the ref probe a second time. */ + | { kind: 'exact'; candidate: T; canonicalBase: string } + | { kind: 'retarget'; candidate: T; canonicalBase: string } + /** Something is armed for this repo but not under this raw base; only a resolved canonical base + * can decide between a hit and a miss. */ + | { kind: 'needs-canonical-base' } + | { kind: 'miss'; reason: PreparationSelectionMissReason } + +/** + * Picks the armed preparation a create may claim. + * + * The two sides of the pool disagree in practice — the prefetch arms `origin/main` while the + * create resolves `main`, or vice versa — and an exact-string key turns every such disagreement + * into a silent cold create. Canonicalizing catches the spelling differences; the base-family + * retarget catches the local-vs-remote-tracking ones, where finalize's existing drift reset lands + * the checkout on the requested commit for far less than a cold add plus a full materialize. + * + * The bound matters: refs outside the same branch family are rejected, because a retarget across + * unrelated history degenerates into a full checkout and wins nothing. + * + * Synchronous on purpose: the caller claims the returned entry in the same run, so two concurrent + * creates cannot both walk away with the same prepared checkout. + */ +export function selectPreparationForCreate( + candidates: readonly T[], + request: PreparationRequest +): PreparationSelection { + if (candidates.length === 0) { + return { kind: 'miss', reason: 'none_armed' } + } + const sameRepo = candidates.filter((candidate) => candidate.repoPathKey === request.repoPathKey) + if (sameRepo.length === 0) { + // Separate from `none_armed`: this is what a size-cap eviction looks like from the create side. + return { kind: 'miss', reason: 'repo_mismatch' } + } + // Distro before root: the distro decides which filesystem the root is even on. + const sameHost = sameRepo.filter((candidate) => candidate.wslDistro === request.wslDistro) + if (sameHost.length === 0) { + return { kind: 'miss', reason: 'wsl_distro_mismatch' } + } + const sameRoot = sameHost.filter( + (candidate) => candidate.workspaceRootKey === request.workspaceRootKey + ) + if (sameRoot.length === 0) { + return { kind: 'miss', reason: 'workspace_root_mismatch' } + } + + const { canonicalBase } = request + if (canonicalBase === null) { + const rawMatch = sameRoot.find((candidate) => candidate.baseBranch === request.baseBranch) + // Same spelling, so the armed entry already holds this request's canonical form. + return rawMatch + ? { kind: 'exact', candidate: rawMatch, canonicalBase: rawMatch.canonicalBase } + : { kind: 'needs-canonical-base' } + } + + const canonicalMatch = sameRoot.find((candidate) => candidate.canonicalBase === canonicalBase) + if (canonicalMatch) { + return { kind: 'exact', candidate: canonicalMatch, canonicalBase } + } + + const family = worktreeBaseRefFamily(canonicalBase) + if (family) { + const retarget = sameRoot + .filter((candidate) => worktreeBaseRefFamily(candidate.canonicalBase) === family) + .sort((left, right) => right.createdAt - left.createdAt)[0] + if (retarget) { + return { kind: 'retarget', candidate: retarget, canonicalBase } + } + } + return { kind: 'miss', reason: 'base_mismatch' } +} diff --git a/src/main/worktree-create-preparation-pool.ts b/src/main/worktree-create-preparation-pool.ts new file mode 100644 index 00000000000..26ee1c41aad --- /dev/null +++ b/src/main/worktree-create-preparation-pool.ts @@ -0,0 +1,210 @@ +import { randomUUID } from 'node:crypto' +import { mkdir } from 'node:fs/promises' +import { posix, win32 } from 'node:path' +import { isWindowsAbsolutePathLike } from '../shared/cross-platform-path' +import { + WORKTREE_CREATE_PREPARATION_DIRECTORY, + createWorktreePreparationLockReason +} from '../shared/worktree/create-preparation' +import type { AddWorktreeOptions } from './git/worktree' +import { prepareWorktreeCreateCheckout } from './git/worktree-create-preparation' +import { toHostFilesystemPath } from './host-tree-removal' +import { preparationEntryKey, preparationPathKey } from './worktree-create-preparation-claim' +import { + cleanupStalePreparations, + hasPendingStalePreparationCleanup, + resetStalePreparationCleanupForTests +} from './worktree-create-preparation-stale-cleanup' +import { + discardPreparationWithRetry, + resetPendingPreparationDiscardsForTests, + trackPreparationDiscard +} from './worktree-preparation-discard-retry' + +export const WORKTREE_CREATE_PREPARATION_TTL_MS = 5 * 60_000 +export const WORKTREE_CREATE_PREPARATION_LIMIT = 3 + +export type PreparationEntry = { + key: string + repoPath: string + repoPathKey: string + workspaceRoot: string + workspaceRootKey: string + wslDistro: string + baseBranch: string + canonicalBase: string + preparedPath: string + options: AddWorktreeOptions + createdAt: number + ready: Promise + expiration: NodeJS.Timeout +} + +export type StartPreparationArgs = { + repoPath: string + workspaceRoot: string + baseBranch: string + canonicalBase: string + options: AddWorktreeOptions +} + +const preparations = new Map() + +/** One repo on one Git host: the scope a stranded discard is retried under. */ +function preparationHostKey(repoPathKey: string, wslDistro: string): string { + return `${repoPathKey}\0${wslDistro}` +} + +/** A prepared checkout is a create that is either in flight or imminent. */ +export function hasPendingPreparations(): boolean { + return preparations.size > 0 || hasPendingStalePreparationCleanup() +} + +function pathOps(path: string): Pick { + return isWindowsAbsolutePathLike(path) ? win32 : posix +} + +async function discardEntry(entry: PreparationEntry): Promise { + // A failed checkout self-discards, but that self-discard is best-effort too, so it can strand the + // registration for the same reason the discard here can. Enrol either way. + await entry.ready.catch(() => {}) + await discardPreparationWithRetry({ + hostKey: preparationHostKey(entry.repoPathKey, entry.wslDistro), + repoPath: entry.repoPath, + preparedPath: entry.preparedPath, + options: entry.options + }) +} + +function discardEntryInBackground(entry: PreparationEntry): void { + // Tracked, not bare `void`: the test reset must be able to settle it before dropping the registry. + trackPreparationDiscard(discardEntry(entry)) +} + +function expireEntry(entry: PreparationEntry): void { + if (preparations.get(entry.key) !== entry) { + return + } + preparations.delete(entry.key) + discardEntryInBackground(entry) +} + +/** + * Frees a slot for an incoming preparation, preferring one the same workspace already owns. + * + * The cap is a disk bound — a prepared checkout is a full tree, ~200 MB of tracked content in the + * repo this was measured against — so it stays small. But flipping through the composer's base + * picker arms several preparations for one repo, and a plain oldest-first eviction let that churn + * throw away another project's warm checkout, which is a structural miss for anyone working across + * several repos. Evict the incoming workspace's own oldest entry first; only reach across + * workspaces when this one holds none. + */ +function enforcePreparationLimit( + repoPathKey: string, + workspaceRootKey: string, + wslDistro: string +): void { + while (preparations.size >= WORKTREE_CREATE_PREPARATION_LIMIT) { + const byAge = [...preparations.values()].sort((left, right) => left.createdAt - right.createdAt) + const victim = + byAge.find( + (entry) => + entry.repoPathKey === repoPathKey && + entry.workspaceRootKey === workspaceRootKey && + entry.wslDistro === wslDistro + ) ?? byAge[0] + if (!victim) { + return + } + preparations.delete(victim.key) + clearTimeout(victim.expiration) + discardEntryInBackground(victim) + } +} + +export function listPreparations(): PreparationEntry[] { + return [...preparations.values()] +} + +export function findPreparation( + repoPathKey: string, + workspaceRootKey: string, + canonicalBase: string, + wslDistro: string +): PreparationEntry | undefined { + return preparations.get( + preparationEntryKey(repoPathKey, workspaceRootKey, canonicalBase, wslDistro) + ) +} + +/** Removes an entry from the pool so no other create can claim it. Callers must run this in the + * same synchronous turn as the selection that produced `entry`. */ +export function takePreparation(entry: PreparationEntry): void { + preparations.delete(entry.key) + clearTimeout(entry.expiration) +} + +export function startPreparation({ + repoPath, + workspaceRoot, + baseBranch, + canonicalBase, + options +}: StartPreparationArgs): Promise { + const repoPathKey = preparationPathKey(repoPath) + const workspaceRootKey = preparationPathKey(workspaceRoot) + const wslDistro = options.wslDistro ?? '' + const key = preparationEntryKey(repoPathKey, workspaceRootKey, canonicalBase, wslDistro) + enforcePreparationLimit(repoPathKey, workspaceRootKey, wslDistro) + const preparationId = `${process.pid}-${randomUUID()}` + const lockReason = createWorktreePreparationLockReason(preparationId) + const preparationRoot = pathOps(workspaceRoot).join( + workspaceRoot, + WORKTREE_CREATE_PREPARATION_DIRECTORY + ) + const preparedPath = pathOps(workspaceRoot).join(preparationRoot, preparationId) + const entry = {} as PreparationEntry + const expiration = setTimeout(() => expireEntry(entry), WORKTREE_CREATE_PREPARATION_TTL_MS) + expiration.unref() + Object.assign(entry, { + key, + repoPath, + repoPathKey, + workspaceRoot, + workspaceRootKey, + wslDistro, + baseBranch, + canonicalBase, + preparedPath, + options, + createdAt: Date.now(), + expiration, + ready: (async () => { + await cleanupStalePreparations(preparationHostKey(repoPathKey, wslDistro), repoPath, options) + await mkdir(toHostFilesystemPath(preparationRoot), { recursive: true }) + // Already canonical, so the add re-resolves nothing. + await prepareWorktreeCreateCheckout(repoPath, preparedPath, canonicalBase, lockReason, options) + })() + } satisfies PreparationEntry) + preparations.set(key, entry) + void entry.ready.catch(() => { + if (preparations.get(key) === entry) { + preparations.delete(key) + clearTimeout(entry.expiration) + } + }) + return entry.ready +} + +export async function _resetPreparationPoolForTests(): Promise { + const entries = [...preparations.values()] + preparations.clear() + resetStalePreparationCleanupForTests() + await Promise.all( + entries.map(async (entry) => { + clearTimeout(entry.expiration) + await discardEntry(entry) + }) + ) + await resetPendingPreparationDiscardsForTests() +} diff --git a/src/main/worktree-create-preparation-wsl-root.test.ts b/src/main/worktree-create-preparation-wsl-root.test.ts index f0c8e664298..2c752b74a80 100644 --- a/src/main/worktree-create-preparation-wsl-root.test.ts +++ b/src/main/worktree-create-preparation-wsl-root.test.ts @@ -16,7 +16,8 @@ const mocks = vi.hoisted(() => ({ getWorktreeOptions: vi.fn(), getMirrorDistro: vi.fn(), getWslHome: vi.fn(), - getWslHomeAsync: vi.fn() + getWslHomeAsync: vi.fn(), + resolveBaseRef: vi.fn() })) vi.mock('node:fs/promises', () => ({ mkdir: mocks.mkdir })) @@ -27,6 +28,9 @@ vi.mock('./git/worktree-create-preparation', () => ({ discardPreparedWorktree: mocks.discard, unlockPreparedWorktree: mocks.unlock })) +vi.mock('./git/worktree-base-ref-probe', () => ({ + resolveLocalWorktreeBaseRef: mocks.resolveBaseRef +})) vi.mock('./project-runtime-git-options', () => ({ getLocalProjectWorktreeGitOptions: mocks.getWorktreeOptions, getWorktreeMirrorDistro: mocks.getMirrorDistro @@ -86,6 +90,9 @@ beforeEach(() => { throw new Error('the blocking wsl.exe home probe must not run while preparing') }) mocks.getWslHomeAsync.mockReset().mockResolvedValue(WSL_HOME) + mocks.resolveBaseRef + .mockReset() + .mockImplementation(async (_repoPath: string, baseRef: string) => `refs/remotes/${baseRef}`) }) afterEach(async () => { diff --git a/src/main/worktree-create-preparation.test.ts b/src/main/worktree-create-preparation.test.ts index eae3b59081f..06818fec422 100644 --- a/src/main/worktree-create-preparation.test.ts +++ b/src/main/worktree-create-preparation.test.ts @@ -2,6 +2,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { Store } from './persistence' import type { Repo } from '../shared/repo-types' import { WORKTREE_CREATE_PREPARATION_DIRECTORY } from '../shared/worktree/create-preparation' +import { resolveWorktreeAddBaseRef } from '../shared/worktree/base-ref' const mocks = vi.hoisted(() => ({ mkdir: vi.fn(), @@ -12,7 +13,9 @@ const mocks = vi.hoisted(() => ({ unlock: vi.fn(), getWorktreeOptions: vi.fn(), computeWorkspaceRoot: vi.fn(), - computeWorkspaceRootAsync: vi.fn() + computeWorkspaceRootAsync: vi.fn(), + resolveBaseRef: vi.fn(), + measureDivergence: vi.fn() })) vi.mock('node:fs/promises', () => ({ mkdir: mocks.mkdir })) @@ -23,6 +26,12 @@ vi.mock('./git/worktree-create-preparation', () => ({ discardPreparedWorktree: mocks.discard, unlockPreparedWorktree: mocks.unlock })) +vi.mock('./git/worktree-base-ref-probe', () => ({ + resolveLocalWorktreeBaseRef: mocks.resolveBaseRef +})) +vi.mock('./git/worktree-base-divergence', () => ({ + measureRetargetDivergence: mocks.measureDivergence +})) vi.mock('./project-runtime-git-options', () => ({ getLocalProjectWorktreeGitOptions: mocks.getWorktreeOptions, getWorktreeMirrorDistro: () => undefined @@ -48,6 +57,11 @@ function flushBackgroundWork(ms = 0): Promise { return new Promise((resolve) => setTimeout(resolve, ms)) } +const EXISTING_REFS = new Set([ + 'refs/heads/main', + 'refs/remotes/origin/main', + 'refs/remotes/origin/release' +]) const repo = { id: 'repo-1', path: '/repo' } as Repo const store = { getSettings: () => ({}) } as unknown as Store @@ -59,6 +73,12 @@ beforeEach(() => { mocks.discard.mockReset().mockResolvedValue(undefined) mocks.unlock.mockReset().mockResolvedValue(undefined) mocks.getWorktreeOptions.mockReset().mockReturnValue({}) + mocks.measureDivergence.mockReset().mockResolvedValue('within') + mocks.resolveBaseRef + .mockReset() + .mockImplementation((_repoPath: string, baseRef: string) => + resolveWorktreeAddBaseRef(baseRef, async (candidate) => EXISTING_REFS.has(candidate)) + ) mocks.computeWorkspaceRoot.mockReset().mockImplementation(() => { throw new Error('synchronous workspace-root lookup must not run on the main thread') }) @@ -135,7 +155,7 @@ describe('worktree create preparation registry', () => { expect(mocks.prepareCheckout).toHaveBeenCalledTimes(1) }) - it('does not claim a preparation after the selected base changes', async () => { + it('does not claim a preparation after the selected base changes to another branch', async () => { await prepareWorktreeCreateForRepo(store, repo, 'origin/main') await expect( @@ -146,10 +166,185 @@ describe('worktree create preparation registry', () => { branch: 'feature/test', baseBranch: 'origin/release' }) - ).resolves.toBeNull() + ).resolves.toEqual({ status: 'miss', reason: 'base_mismatch' }) expect(mocks.finalize).not.toHaveBeenCalled() }) + it('claims across the local/remote spelling of the same base and reports the retarget', async () => { + await prepareWorktreeCreateForRepo(store, repo, 'origin/main') + + // `main` has no local ref here, so the canonical forms differ and only the base family matches. + await expect( + consumePreparedWorktreeCreate({ + repoPath: repo.path, + workspaceRoot: '/workspace', + worktreePath: '/workspace/final', + branch: 'feature/test', + baseBranch: 'main' + }) + ).resolves.toEqual({ status: 'hit', retargeted: true, result: {} }) + // Finalize still receives the requested base, so it resets onto the requested commit. + expect(mocks.finalize).toHaveBeenCalledWith( + repo.path, + expect.any(String), + '/workspace/final', + 'feature/test', + 'main', + undefined, + {} + ) + }) + + it('refuses a same-family retarget whose bases have drifted too far apart', async () => { + await prepareWorktreeCreateForRepo(store, repo, 'origin/main') + // An abandoned fork's `main` is the same base family but a whole-tree checkout away. + mocks.measureDivergence.mockResolvedValue('exceeded') + + await expect( + consumePreparedWorktreeCreate({ + repoPath: repo.path, + workspaceRoot: '/workspace', + worktreePath: '/workspace/final', + branch: 'feature/test', + baseBranch: 'main' + }) + ).resolves.toEqual({ status: 'miss', reason: 'retarget_too_divergent' }) + expect(mocks.finalize).not.toHaveBeenCalled() + // The preparation is left armed for the base it actually holds. + expect(mocks.discard).not.toHaveBeenCalled() + }) + + it('separates a drift check that said no from one that could not answer', async () => { + await prepareWorktreeCreateForRepo(store, repo, 'origin/main') + // A timed-out or aborted walk skipped a retarget that may well have been cheap; that is a + // tuning signal, not the bound working as intended, so it must not report as excess drift. + mocks.measureDivergence.mockResolvedValue('unknown') + + await expect( + consumePreparedWorktreeCreate({ + repoPath: repo.path, + workspaceRoot: '/workspace', + worktreePath: '/workspace/final', + branch: 'feature/test', + baseBranch: 'main' + }) + ).resolves.toEqual({ status: 'miss', reason: 'retarget_unverifiable' }) + expect(mocks.finalize).not.toHaveBeenCalled() + expect(mocks.discard).not.toHaveBeenCalled() + }) + + it('does not spend a divergence walk when the base matches exactly', async () => { + await prepareWorktreeCreateForRepo(store, repo, 'origin/main') + + await consumePreparedWorktreeCreate({ + repoPath: repo.path, + workspaceRoot: '/workspace', + worktreePath: '/workspace/final', + branch: 'feature/test', + baseBranch: 'origin/main' + }) + + expect(mocks.measureDivergence).not.toHaveBeenCalled() + }) + + it('claims when the two sides spell the same ref differently', async () => { + await prepareWorktreeCreateForRepo(store, repo, 'origin/main') + + await expect( + consumePreparedWorktreeCreate({ + repoPath: repo.path, + workspaceRoot: '/workspace', + worktreePath: '/workspace/final', + branch: 'feature/test', + baseBranch: 'refs/remotes/origin/main' + }) + ).resolves.toEqual({ status: 'hit', retargeted: false, result: {} }) + }) + + it('never hands the same prepared checkout to two concurrent creates', async () => { + await prepareWorktreeCreateForRepo(store, repo, 'origin/main') + + // `main` needs the ref probe, so the claim has to await mid-flight — the window where a + // second create could otherwise walk away with the same preparation. + const [first, second] = await Promise.all([ + consumePreparedWorktreeCreate({ + repoPath: repo.path, + workspaceRoot: '/workspace', + worktreePath: '/workspace/first', + branch: 'feature/first', + baseBranch: 'main' + }), + consumePreparedWorktreeCreate({ + repoPath: repo.path, + workspaceRoot: '/workspace', + worktreePath: '/workspace/second', + branch: 'feature/second', + baseBranch: 'main' + }) + ]) + + expect([first.status, second.status]).toContain('hit') + const preparedPaths = mocks.finalize.mock.calls.map((call) => call[1]) + expect(new Set(preparedPaths).size).toBe(preparedPaths.length) + }) + + it('reports which part of the claim key disagreed', async () => { + await prepareWorktreeCreateForRepo(store, repo, 'origin/main') + + await expect( + consumePreparedWorktreeCreate({ + repoPath: repo.path, + workspaceRoot: '/other-workspace', + worktreePath: '/other-workspace/final', + branch: 'feature/test', + baseBranch: 'origin/main' + }) + ).resolves.toEqual({ status: 'miss', reason: 'workspace_root_mismatch' }) + + await expect( + consumePreparedWorktreeCreate({ + repoPath: repo.path, + workspaceRoot: '/workspace', + worktreePath: '/workspace/final', + branch: 'feature/test', + baseBranch: 'origin/main', + options: { wslDistro: 'Ubuntu' } + }) + ).resolves.toEqual({ status: 'miss', reason: 'wsl_distro_mismatch' }) + + await expect( + consumePreparedWorktreeCreate({ + repoPath: '/other-repo', + workspaceRoot: '/workspace', + worktreePath: '/workspace/final', + branch: 'feature/test', + baseBranch: 'origin/main' + }) + ).resolves.toEqual({ status: 'miss', reason: 'repo_mismatch' }) + expect(mocks.finalize).not.toHaveBeenCalled() + }) + + it("evicts a repo's own stale preparation before another repo's", async () => { + const otherRepo = { id: 'repo-2', path: '/other-repo' } as Repo + await prepareWorktreeCreateForRepo(store, otherRepo, 'origin/main') + // Fill the pool from one repo, as flipping the composer's base picker does, until the next + // arm has to evict something. + await prepareWorktreeCreateForRepo(store, repo, 'origin/main') + await prepareWorktreeCreateForRepo(store, repo, 'origin/release') + await prepareWorktreeCreateForRepo(store, repo, 'main') + + // The eviction must cost `repo` a slot, not `otherRepo` its warm checkout. + await expect( + consumePreparedWorktreeCreate({ + repoPath: otherRepo.path, + workspaceRoot: '/workspace', + worktreePath: '/workspace/other', + branch: 'feature/other', + baseBranch: 'origin/main' + }) + ).resolves.toMatchObject({ status: 'hit' }) + }) + it('routes preparation and finalization through the selected WSL runtime', async () => { const options = { wslDistro: 'Ubuntu' } mocks.getWorktreeOptions.mockReturnValue(options) @@ -167,7 +362,7 @@ describe('worktree create preparation registry', () => { expect(mocks.prepareCheckout).toHaveBeenCalledWith( repo.path, expect.any(String), - 'origin/main', + 'refs/remotes/origin/main', expect.any(String), options ) @@ -247,6 +442,76 @@ describe('worktree create preparation registry', () => { ) }) + it('cleans up and reports a finalize miss so normal add can run', async () => { + await prepareWorktreeCreateForRepo(store, repo, 'origin/main') + mocks.finalize.mockRejectedValueOnce(new Error('submodules prevent worktree move')) + + await expect( + consumePreparedWorktreeCreate({ + repoPath: repo.path, + workspaceRoot: '/workspace', + worktreePath: '/workspace/final', + branch: 'feature/test', + baseBranch: 'origin/main' + }) + ).resolves.toEqual({ status: 'miss', reason: 'finalize_failed' }) + expect(mocks.mkdir).toHaveBeenCalledWith('/workspace', { recursive: true }) + expect(mocks.discard).toHaveBeenCalledTimes(1) + }) + + async function consumeOnce(name: string): Promise { + await consumePreparedWorktreeCreate({ + repoPath: repo.path, + workspaceRoot: '/workspace', + worktreePath: `/workspace/${name}`, + branch: `feature/${name}`, + baseBranch: 'origin/main' + }) + } + + it('does not re-arm after an isolated create', async () => { + await prepareWorktreeCreateForRepo(store, repo, 'origin/main') + await consumeOnce('only') + + // Why: a lone create would otherwise leave a full spare checkout on disk for the whole TTL. + expect(mocks.prepareCheckout).toHaveBeenCalledTimes(1) + }) + + it('re-arms a preparation once creates arrive in a burst', async () => { + await prepareWorktreeCreateForRepo(store, repo, 'origin/main') + await consumeOnce('first') + expect(mocks.prepareCheckout).toHaveBeenCalledTimes(1) + + await prepareWorktreeCreateForRepo(store, repo, 'origin/main') + await consumeOnce('second') + await prepareWorktreeCreateForRepo(store, repo, 'origin/main') + + expect(mocks.prepareCheckout).toHaveBeenCalledTimes(3) + // The replacement is claimable, so a third create still skips the cold add. + await expect( + consumePreparedWorktreeCreate({ + repoPath: repo.path, + workspaceRoot: '/workspace', + worktreePath: '/workspace/third', + branch: 'feature/third', + baseBranch: 'origin/main' + }) + ).resolves.toEqual({ status: 'hit', retargeted: false, result: {} }) + expect(mocks.finalize).toHaveBeenCalledTimes(3) + }) + + it('does not re-arm when finalization failed', async () => { + await prepareWorktreeCreateForRepo(store, repo, 'origin/main') + await consumeOnce('first') + await prepareWorktreeCreateForRepo(store, repo, 'origin/main') + mocks.prepareCheckout.mockClear() + mocks.finalize.mockRejectedValueOnce(new Error('submodules prevent worktree move')) + + await consumeOnce('second') + + expect(mocks.prepareCheckout).not.toHaveBeenCalled() + }) + it('retries a discard that failed while this process is still alive', async () => { await prepareWorktreeCreateForRepo(store, repo, 'origin/main') const leakedPath = mocks.prepareCheckout.mock.calls[0][1] as string @@ -286,10 +551,14 @@ describe('worktree create preparation registry', () => { unremovable.add(leakedHere) unremovable.add(leakedElsewhere) - // Evict both, oldest first, so each host has one recorded discard failure. - for (const base of ['origin/one', 'origin/two', 'origin/three']) { + // Evict through each host's own arming: eviction prefers the incoming workspace's oldest + // entry, so preparing for `repo` no longer reaches across and takes `otherRepo`'s. + for (const base of ['origin/one', 'origin/two']) { await prepareWorktreeCreateForRepo(store, repo, base) } + for (const base of ['origin/one', 'origin/two']) { + await prepareWorktreeCreateForRepo(store, otherRepo, base) + } await flushBackgroundWork() expect(mocks.discard).toHaveBeenCalledWith(repo.path, leakedHere, {}) expect(mocks.discard).toHaveBeenCalledWith(otherRepo.path, leakedElsewhere, {}) @@ -379,11 +648,15 @@ describe('worktree create preparation registry', () => { unremovable.add(leakedOnUbuntu) unremovable.add(leakedOnDebian) - // Evict both, oldest first, so each distro has one recorded discard failure. + // Evict through each distro's own arming: the eviction scope includes the distro, so arming + // under Ubuntu no longer reaches across and takes the Debian entry. mocks.getWorktreeOptions.mockReturnValue({ wslDistro: 'Ubuntu' }) - for (const base of ['origin/one', 'origin/two', 'origin/three']) { + for (const base of ['origin/one', 'origin/two']) { await prepareWorktreeCreateForRepo(store, repo, base) } + mocks.getWorktreeOptions.mockReturnValue({ wslDistro: 'Debian' }) + await prepareWorktreeCreateForRepo(store, repo, 'origin/one') + mocks.getWorktreeOptions.mockReturnValue({ wslDistro: 'Ubuntu' }) await flushBackgroundWork() expect(mocks.discard).toHaveBeenCalledWith(repo.path, leakedOnUbuntu, { wslDistro: 'Ubuntu' }) expect(mocks.discard).toHaveBeenCalledWith(repo.path, leakedOnDebian, { wslDistro: 'Debian' }) @@ -451,33 +724,6 @@ describe('worktree create preparation registry', () => { expect(mocks.discard).not.toHaveBeenCalledWith(repo.path, leakedPath, {}) }) - it('cleans up and returns null so normal add can run when finalization fails', async () => { - await prepareWorktreeCreateForRepo(store, repo, 'origin/main') - mocks.finalize.mockRejectedValueOnce(new Error('submodules prevent worktree move')) - - await expect( - consumePreparedWorktreeCreate({ - repoPath: repo.path, - workspaceRoot: '/workspace', - worktreePath: '/workspace/final', - branch: 'feature/test', - baseBranch: 'origin/main' - }) - ).resolves.toBeNull() - expect(mocks.mkdir).toHaveBeenCalledWith('/workspace', { recursive: true }) - expect(mocks.discard).toHaveBeenCalledTimes(1) - }) - - function consumeOnce(name: string): ReturnType { - return consumePreparedWorktreeCreate({ - repoPath: repo.path, - workspaceRoot: '/workspace', - worktreePath: `/workspace/${name}`, - branch: `feature/${name}`, - baseBranch: 'origin/main' - }) - } - it('reports a pending create while a stale-cleanup scan is running', async () => { let releaseListing!: () => void mocks.listWorktreeGraph.mockReturnValueOnce( @@ -498,41 +744,4 @@ describe('worktree create preparation registry', () => { releaseListing() await arming }) - - it('does not re-arm after an isolated create', async () => { - await prepareWorktreeCreateForRepo(store, repo, 'origin/main') - await expect(consumeOnce('only')).resolves.toEqual({}) - - // Why: a lone create would otherwise leave a full spare checkout on disk for the whole TTL. - expect(mocks.prepareCheckout).toHaveBeenCalledTimes(1) - }) - - it('re-arms a preparation once creates arrive in a burst', async () => { - await prepareWorktreeCreateForRepo(store, repo, 'origin/main') - await expect(consumeOnce('first')).resolves.toEqual({}) - expect(mocks.prepareCheckout).toHaveBeenCalledTimes(1) - - await prepareWorktreeCreateForRepo(store, repo, 'origin/main') - expect(mocks.prepareCheckout).toHaveBeenCalledTimes(2) - - // No arming call follows this consume: the third checkout can only come from the re-arm. - await expect(consumeOnce('second')).resolves.toEqual({}) - expect(mocks.prepareCheckout).toHaveBeenCalledTimes(3) - - // The replacement is claimable, so a third create still skips the cold add. - await expect(consumeOnce('third')).resolves.toEqual({}) - expect(mocks.finalize).toHaveBeenCalledTimes(3) - }) - - it('does not re-arm when finalization failed', async () => { - await prepareWorktreeCreateForRepo(store, repo, 'origin/main') - await expect(consumeOnce('first')).resolves.toEqual({}) - await prepareWorktreeCreateForRepo(store, repo, 'origin/main') - mocks.prepareCheckout.mockClear() - mocks.finalize.mockRejectedValueOnce(new Error('submodules prevent worktree move')) - - await expect(consumeOnce('second')).resolves.toBeNull() - - expect(mocks.prepareCheckout).not.toHaveBeenCalled() - }) }) diff --git a/src/main/worktree-create-preparation.ts b/src/main/worktree-create-preparation.ts index 8ddb2a5e69c..b13916194ca 100644 --- a/src/main/worktree-create-preparation.ts +++ b/src/main/worktree-create-preparation.ts @@ -1,19 +1,26 @@ -import { randomUUID } from 'node:crypto' import { mkdir } from 'node:fs/promises' import { posix, win32 } from 'node:path' import type { Store } from './persistence' import type { Repo } from '../shared/repo-types' import { isFolderRepo } from '../shared/repo-kind' import { isWindowsAbsolutePathLike } from '../shared/cross-platform-path' -import { - WORKTREE_CREATE_PREPARATION_DIRECTORY, - createWorktreePreparationLockReason -} from '../shared/worktree/create-preparation' +import type { PreparedCheckoutMissReason } from '../shared/worktree/create-types' import type { AddWorktreeOptions, AddWorktreeResult } from './git/worktree' +import { measureRetargetDivergence } from './git/worktree-base-divergence' +import { resolveLocalWorktreeBaseRef } from './git/worktree-base-ref-probe' +import { preparationPathKey, selectPreparationForCreate } from './worktree-create-preparation-claim' +import { + _resetPreparationPoolForTests, + findPreparation, + hasPendingPreparations, + listPreparations, + startPreparation, + takePreparation, + type PreparationEntry +} from './worktree-create-preparation-pool' import { discardPreparedWorktree, - finalizePreparedWorktree, - prepareWorktreeCreateCheckout + finalizePreparedWorktree } from './git/worktree-create-preparation' import { getLocalProjectWorktreeGitOptions, @@ -24,32 +31,22 @@ import { recordPreparationConsume, resetPreparationConsumeHistoryForTests } from './worktree-create-preparation-burst' -import { - cleanupStalePreparations, - hasPendingStalePreparationCleanup, - resetStalePreparationCleanupForTests -} from './worktree-create-preparation-stale-cleanup' import { toHostFilesystemPath } from './host-tree-removal' -import { - discardPreparationWithRetry, - resetPendingPreparationDiscardsForTests, - trackPreparationDiscard -} from './worktree-preparation-discard-retry' -export const WORKTREE_CREATE_PREPARATION_TTL_MS = 5 * 60_000 -export const WORKTREE_CREATE_PREPARATION_LIMIT = 3 +export { + WORKTREE_CREATE_PREPARATION_LIMIT, + WORKTREE_CREATE_PREPARATION_TTL_MS +} from './worktree-create-preparation-pool' -type PreparationEntry = { - key: string - repoPath: string - workspaceRoot: string - preparedPath: string - options: AddWorktreeOptions - createdAt: number - ready: Promise - expiration: NodeJS.Timeout +/** A prepared checkout is a create that is either in flight or imminent. */ +export function hasPendingWorktreeCreatePreparations(): boolean { + return hasPendingPreparations() } +export type PreparedWorktreeCreateAttempt = + | { status: 'hit'; retargeted: boolean; result: AddWorktreeResult } + | { status: 'miss'; reason: PreparedCheckoutMissReason } + type ConsumePreparedWorktreeArgs = { repoPath: string workspaceRoot: string @@ -60,72 +57,16 @@ type ConsumePreparedWorktreeArgs = { options?: AddWorktreeOptions } -const preparations = new Map() - -/** A prepared checkout is a create that is either in flight or imminent. */ -export function hasPendingWorktreeCreatePreparations(): boolean { - return preparations.size > 0 || hasPendingStalePreparationCleanup() -} - -function pathOps(path: string): Pick { - return isWindowsAbsolutePathLike(path) ? win32 : posix -} - -function pathKey(path: string): string { - const normalized = pathOps(path).normalize(path) - return isWindowsAbsolutePathLike(path) ? normalized.toLowerCase() : normalized -} - -function preparationKey( +function canonicalBaseRef( repoPath: string, - workspaceRoot: string, baseBranch: string, options: AddWorktreeOptions -): string { - return `${pathKey(repoPath)}\0${pathKey(workspaceRoot)}\0${baseBranch}\0${options.wslDistro ?? ''}` -} - -function preparationHostKey(repoPath: string, options: AddWorktreeOptions): string { - return `${pathKey(repoPath)}\0${options.wslDistro ?? ''}` -} - -async function discardEntry(entry: PreparationEntry): Promise { - // A failed checkout self-discards, but that self-discard is best-effort too, so it can strand the - // registration for the same reason the discard here can. Enrol either way. - await entry.ready.catch(() => {}) - await discardPreparationWithRetry({ - hostKey: preparationHostKey(entry.repoPath, entry.options), - repoPath: entry.repoPath, - preparedPath: entry.preparedPath, - options: entry.options - }) -} - -function discardEntryInBackground(entry: PreparationEntry): void { - // Tracked, not bare `void`: the test reset must be able to settle it before dropping the registry. - trackPreparationDiscard(discardEntry(entry)) -} - -function expireEntry(entry: PreparationEntry): void { - if (preparations.get(entry.key) !== entry) { - return - } - preparations.delete(entry.key) - discardEntryInBackground(entry) -} - -function enforcePreparationLimit(): void { - while (preparations.size >= WORKTREE_CREATE_PREPARATION_LIMIT) { - const oldest = [...preparations.values()].sort( - (left, right) => left.createdAt - right.createdAt - )[0] - if (!oldest) { - return - } - preparations.delete(oldest.key) - clearTimeout(oldest.expiration) - discardEntryInBackground(oldest) - } +): Promise { + return resolveLocalWorktreeBaseRef( + repoPath, + baseBranch, + options.wslDistro ? { wslDistro: options.wslDistro } : {} + ) } export async function prepareWorktreeCreateForRepo( @@ -145,122 +86,146 @@ export async function prepareWorktreeCreateForRepo( repo.path, getWorktreePathSettings(repo, store.getSettings(), getWorktreeMirrorDistro(store, repo)) ) - const key = preparationKey(repo.path, workspaceRoot, baseBranch, options) - const existing = preparations.get(key) + const canonicalBase = await canonicalBaseRef(repo.path, baseBranch, options) + const existing = findPreparation( + preparationPathKey(repo.path), + preparationPathKey(workspaceRoot), + canonicalBase, + options.wslDistro ?? '' + ) if (existing) { return existing.ready } - return startPreparation(key, repo.path, workspaceRoot, baseBranch, options) + return startPreparation({ + repoPath: repo.path, + workspaceRoot, + baseBranch, + canonicalBase, + options + }) } -function startPreparation( - key: string, - repoPath: string, - workspaceRoot: string, - baseBranch: string, - options: AddWorktreeOptions -): Promise { - enforcePreparationLimit() - const preparationId = `${process.pid}-${randomUUID()}` - const lockReason = createWorktreePreparationLockReason(preparationId) - const preparedPath = pathOps(workspaceRoot).join( - workspaceRoot, - WORKTREE_CREATE_PREPARATION_DIRECTORY, - preparationId - ) - const entry = {} as PreparationEntry - const expiration = setTimeout(() => expireEntry(entry), WORKTREE_CREATE_PREPARATION_TTL_MS) - expiration.unref() - Object.assign(entry, { - key, - repoPath, - workspaceRoot, - preparedPath, - options, - createdAt: Date.now(), - expiration, - ready: (async () => { - await cleanupStalePreparations(preparationHostKey(repoPath, options), repoPath, options) - await mkdir( - toHostFilesystemPath( - pathOps(workspaceRoot).join(workspaceRoot, WORKTREE_CREATE_PREPARATION_DIRECTORY) - ), - { recursive: true } - ) - await prepareWorktreeCreateCheckout(repoPath, preparedPath, baseBranch, lockReason, options) - })() - } satisfies PreparationEntry) - preparations.set(key, entry) - void entry.ready.catch(() => { - if (preparations.get(key) === entry) { - preparations.delete(key) - clearTimeout(entry.expiration) - } - }) - return entry.ready -} +type ClaimedPreparation = + | { status: 'claimed'; entry: PreparationEntry; retargeted: boolean; canonicalBase: string } + | { status: 'miss'; reason: PreparedCheckoutMissReason } async function claimPreparedWorktree( - repoPath: string, - workspaceRoot: string, - baseBranch: string, + args: ConsumePreparedWorktreeArgs, options: AddWorktreeOptions -): Promise { - const key = preparationKey(repoPath, workspaceRoot, baseBranch, options) - const entry = preparations.get(key) - if (!entry) { - return null +): Promise { + const request = { + repoPathKey: preparationPathKey(args.repoPath), + workspaceRootKey: preparationPathKey(args.workspaceRoot), + wslDistro: options.wslDistro ?? '', + baseBranch: args.baseBranch } - preparations.delete(key) - clearTimeout(entry.expiration) + let selection = selectPreparationForCreate(listPreparations(), { + ...request, + canonicalBase: null + }) + if (selection.kind === 'needs-canonical-base') { + // The probe is the only await here, and the pool is re-read after it, so the select-and-take + // below stays one synchronous run and no other create can hold the same entry. + const canonicalBase = await canonicalBaseRef(args.repoPath, args.baseBranch, options) + selection = selectPreparationForCreate(listPreparations(), { ...request, canonicalBase }) + } + if (selection.kind !== 'exact' && selection.kind !== 'retarget') { + return { + status: 'miss', + reason: selection.kind === 'miss' ? selection.reason : 'base_mismatch' + } + } + if (selection.kind === 'retarget') { + const candidate = selection.candidate + const { canonicalBase } = selection + const divergence = await measureRetargetDivergence( + args.repoPath, + candidate.canonicalBase, + canonicalBase, + { + ...(options.wslDistro ? { wslDistro: options.wslDistro } : {}), + // Why forward it: a cancelled create must stop these probes now, not at the deadline. + ...(options.signal ? { signal: options.signal } : {}) + } + ) + if (divergence !== 'within') { + return { + status: 'miss', + reason: divergence === 'exceeded' ? 'retarget_too_divergent' : 'retarget_unverifiable' + } + } + // Re-select after the walk: the pool may have gained an exact match or lost this entry. A + // different retarget candidate is left for the next create rather than claimed unverified. + selection = selectPreparationForCreate(listPreparations(), { ...request, canonicalBase }) + if (selection.kind === 'miss' || selection.kind === 'needs-canonical-base') { + return { status: 'miss', reason: 'base_mismatch' } + } + if (selection.kind === 'retarget' && selection.candidate !== candidate) { + return { status: 'miss', reason: 'base_mismatch' } + } + } + const entry = selection.candidate + takePreparation(entry) try { await entry.ready - return entry + return { + status: 'claimed', + entry, + retargeted: selection.kind === 'retarget', + canonicalBase: selection.canonicalBase + } } catch { - return null + return { status: 'miss', reason: 'prepare_failed' } } } -/** Replaces a just-consumed preparation, but only once the user has shown they are creating in a - * burst. A replacement costs a full checkout and ~5 minutes of disk until its TTL, so arming one - * after an isolated create spends that on nobody. Never awaited: create has already returned by - * the time the replacement checkout finishes. */ -function rearmPreparation(entry: PreparationEntry, baseBranch: string): void { +/** Replaces a just-consumed preparation, re-armed on the base the create actually used so the + * next one hits exactly — but only once the user has shown they are creating in a burst. A + * replacement costs a full checkout and ~5 minutes of disk until its TTL, so arming one after an + * isolated create spends that on nobody. Never awaited: create has already returned by the time + * the replacement checkout finishes. */ +function rearmPreparation( + entry: PreparationEntry, + baseBranch: string, + canonicalBase: string +): void { // Record first: a prefetch that re-armed this key while we finalized would otherwise swallow the // consume, and the next create would look isolated when it is really the middle of a burst. const continuesBurst = recordPreparationConsume(entry.key) - if (preparations.has(entry.key) || !continuesBurst) { + if ( + !continuesBurst || + findPreparation(entry.repoPathKey, entry.workspaceRootKey, canonicalBase, entry.wslDistro) + ) { return } - void startPreparation( - entry.key, - entry.repoPath, - entry.workspaceRoot, + void startPreparation({ + repoPath: entry.repoPath, + workspaceRoot: entry.workspaceRoot, baseBranch, - entry.options - ).catch(() => { + canonicalBase, + options: entry.options + }).catch(() => { // Why: a warm-up failure is recovered by the normal add on the next create. }) } export async function consumePreparedWorktreeCreate( args: ConsumePreparedWorktreeArgs -): Promise { +): Promise { const options = args.options ?? {} - const entry = await claimPreparedWorktree( - args.repoPath, - args.workspaceRoot, - args.baseBranch, - options - ) - if (!entry) { - return null + const claim = await claimPreparedWorktree(args, options) + if (claim.status === 'miss') { + return { status: 'miss', reason: claim.reason } } + const { entry } = claim try { - await mkdir(toHostFilesystemPath(pathOps(args.worktreePath).dirname(args.worktreePath)), { - recursive: true - }) + const parentDir = isWindowsAbsolutePathLike(args.worktreePath) + ? win32.dirname(args.worktreePath) + : posix.dirname(args.worktreePath) + await mkdir(toHostFilesystemPath(parentDir), { recursive: true }) + // Finalize resolves the requested base itself and resets the prepared checkout onto that + // commit, so a retargeted claim is handed over at the requested commit or not at all. const result = await finalizePreparedWorktree( args.repoPath, entry.preparedPath, @@ -272,28 +237,19 @@ export async function consumePreparedWorktreeCreate( ) // Consuming the only prepared checkout leaves the next create cold. Re-arm for a user who is // creating in a burst; the TTL and the preparation limit still bound an unused replacement. - rearmPreparation(entry, args.baseBranch) - return result + rearmPreparation(entry, args.baseBranch, claim.canonicalBase) + return { status: 'hit', retargeted: claim.retargeted, result } } catch (error) { await discardPreparedWorktree(args.repoPath, entry.preparedPath, options).catch(() => {}) console.warn( '[worktree-create] prepared checkout could not be finalized; using normal add', error ) - return null + return { status: 'miss', reason: 'finalize_failed' } } } export async function _resetWorktreeCreatePreparationsForTests(): Promise { - const entries = [...preparations.values()] - preparations.clear() resetPreparationConsumeHistoryForTests() - resetStalePreparationCleanupForTests() - await Promise.all( - entries.map(async (entry) => { - clearTimeout(entry.expiration) - await discardEntry(entry) - }) - ) - await resetPendingPreparationDiscardsForTests() + await _resetPreparationPoolForTests() } diff --git a/src/main/worktree-create-timing.ts b/src/main/worktree-create-timing.ts index 433a9ac0098..bc1e49f4842 100644 --- a/src/main/worktree-create-timing.ts +++ b/src/main/worktree-create-timing.ts @@ -1,4 +1,5 @@ import type { + PreparedCheckoutOutcome, WorktreeCreateTiming, WorktreeCreateTimingPhase } from '../shared/worktree/create-types' @@ -8,6 +9,7 @@ type TimingClock = () => number export type WorktreeCreateTimingRecorder = { time(phase: string, operation: () => Promise): Promise timeSync(phase: string, operation: () => T): T + recordPreparedCheckout(outcome: PreparedCheckoutOutcome): void finish(): WorktreeCreateTiming } @@ -37,6 +39,7 @@ export function createWorktreeCreateTimingRecorder( ): WorktreeCreateTimingRecorder { const startedAt = clock() const phases: WorktreeCreateTimingPhase[] = [] + let preparedCheckout: PreparedCheckoutOutcome | undefined const recordPhase = (phase: string, operationStartedAt: number): void => { phases.push(createPhase(phase, operationStartedAt, clock(), startedAt)) @@ -59,10 +62,14 @@ export function createWorktreeCreateTimingRecorder( recordPhase(phase, operationStartedAt) } }, + recordPreparedCheckout(outcome: PreparedCheckoutOutcome): void { + preparedCheckout = outcome + }, finish() { return { totalDurationMs: clampDuration(clock() - startedAt), - phases: [...phases] + phases: [...phases], + ...(preparedCheckout ? { preparedCheckout } : {}) } } } diff --git a/src/shared/git-binary-compatibility.test.ts b/src/shared/git-binary-compatibility.test.ts index 2861c447788..a61e64464da 100644 --- a/src/shared/git-binary-compatibility.test.ts +++ b/src/shared/git-binary-compatibility.test.ts @@ -182,6 +182,38 @@ describeBinaryCompatibility('real Git binary compatibility', () => { await runGit(['branch', '-D', 'compat-prepared-final']) }) + // Why pin this: the prepared-checkout retarget bound reads these as data, and it fails closed, + // so a version that printed a different shape would silently stop every retarget rather than + // error. Built with `commit-tree` so the check leaves no ref, branch, or worktree behind. + it('measures retarget drift identically on every supported Git', async () => { + const tree = (await runGit(['rev-parse', 'HEAD^{tree}'])).stdout.trim() + const head = (await runGit(['rev-parse', 'HEAD'])).stdout.trim() + const ahead1 = (await runGit(['commit-tree', tree, '-p', head, '-m', 'drift 1'])).stdout.trim() + const ahead2 = ( + await runGit(['commit-tree', tree, '-p', ahead1, '-m', 'drift 2']) + ).stdout.trim() + + await expect( + runGit(['rev-list', '--count', '--max-count=101', '--end-of-options', `${head}..${ahead2}`]) + ).resolves.toMatchObject({ stdout: '2\n' }) + // `--max-count` must report the capped number, not the full one: the bound reads it as a + // ceiling, so a Git that returned the true count would reject every retarget instead. + await expect( + runGit(['rev-list', '--count', '--max-count=1', '--end-of-options', `${head}..${ahead2}`]) + ).resolves.toMatchObject({ stdout: '1\n' }) + await expect( + runGit(['rev-list', '--count', '--max-count=101', '--end-of-options', `${ahead2}..${head}`]) + ).resolves.toMatchObject({ stdout: '0\n' }) + + await expect(runGit(['merge-base', '--end-of-options', head, ahead2])).resolves.toMatchObject({ + stdout: `${head}\n` + }) + // A parentless commit shares no history, which is the case the bound must reject however few + // commits each side carries. + const unrelated = (await runGit(['commit-tree', tree, '-m', 'unrelated root'])).stdout.trim() + await expect(runGit(['merge-base', '--end-of-options', head, unrelated])).rejects.toBeDefined() + }) + it('recognizes ref and merge-tree compatibility boundaries', async () => { const fetchHeadPath = join(repoPath, '.git', 'FETCH_HEAD') await writeFile(fetchHeadPath, 'sentinel\n') diff --git a/src/shared/worktree/base-ref.test.ts b/src/shared/worktree/base-ref.test.ts index 062d840b1cd..de87a1b0880 100644 --- a/src/shared/worktree/base-ref.test.ts +++ b/src/shared/worktree/base-ref.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from 'vitest' -import { resolveWorktreeAddBaseRef } from './base-ref' +import { resolveWorktreeAddBaseRef, worktreeBaseRefFamily } from './base-ref' describe('resolveWorktreeAddBaseRef', () => { it('leaves fully qualified refs unchanged', async () => { @@ -62,3 +62,31 @@ describe('resolveWorktreeAddBaseRef', () => { await expect(resolveWorktreeAddBaseRef('abc1234', refExists)).resolves.toBe('abc1234') }) }) + +describe('worktreeBaseRefFamily', () => { + it('gives a local branch and its remote-tracking copies the same family', () => { + expect(worktreeBaseRefFamily('refs/heads/main')).toBe('main') + expect(worktreeBaseRefFamily('refs/remotes/origin/main')).toBe('main') + expect(worktreeBaseRefFamily('refs/remotes/upstream/main')).toBe('main') + }) + + it('keeps the full branch path for slash-containing branches', () => { + expect(worktreeBaseRefFamily('refs/heads/release/24.1')).toBe('release/24.1') + expect(worktreeBaseRefFamily('refs/remotes/origin/release/24.1')).toBe('release/24.1') + }) + + it('separates different branches', () => { + expect(worktreeBaseRefFamily('refs/heads/main')).not.toBe( + worktreeBaseRefFamily('refs/heads/release') + ) + }) + + it('has no family for anything that is not a branch ref', () => { + expect(worktreeBaseRefFamily('abc1234')).toBeNull() + expect(worktreeBaseRefFamily('main')).toBeNull() + expect(worktreeBaseRefFamily('refs/tags/v1.0.0')).toBeNull() + expect(worktreeBaseRefFamily('refs/pull/123/head')).toBeNull() + expect(worktreeBaseRefFamily('refs/remotes/origin/HEAD')).toBeNull() + expect(worktreeBaseRefFamily('refs/remotes/origin')).toBeNull() + }) +}) diff --git a/src/shared/worktree/base-ref.ts b/src/shared/worktree/base-ref.ts index 6b4b840910f..08b39f6ef27 100644 --- a/src/shared/worktree/base-ref.ts +++ b/src/shared/worktree/base-ref.ts @@ -23,3 +23,30 @@ export async function resolveWorktreeAddBaseRef( return baseRef } + +/** + * The branch identity two base refs share when one is the local branch and the + * other is a remote-tracking copy of it: `refs/heads/main` and + * `refs/remotes/origin/main` both return `main`. + * + * Bounds the prepared-checkout retarget. A prepared checkout may only be reused + * for a different base when both name the same branch, so the retarget reset is + * bounded by that branch's drift across remotes rather than by an arbitrary + * divergence. Anything unqualified — a bare name, a commit id — has no family. + */ +export function worktreeBaseRefFamily(qualifiedRef: string): string | null { + if (qualifiedRef.startsWith('refs/heads/')) { + return qualifiedRef.slice('refs/heads/'.length) || null + } + if (qualifiedRef.startsWith('refs/remotes/')) { + const withoutRemote = qualifiedRef.slice('refs/remotes/'.length) + const separator = withoutRemote.indexOf('/') + if (separator <= 0) { + return null + } + const branch = withoutRemote.slice(separator + 1) + // `refs/remotes//HEAD` is a symbolic pointer, not a branch identity. + return branch && branch !== 'HEAD' ? branch : null + } + return null +} diff --git a/src/shared/worktree/create-types.ts b/src/shared/worktree/create-types.ts index cb773336db1..d8f3cfc81c5 100644 --- a/src/shared/worktree/create-types.ts +++ b/src/shared/worktree/create-types.ts @@ -31,9 +31,40 @@ export type WorktreeCreateTimingPhase = { durationMs: number } +/** Closed vocabulary: these values reach span attributes, so none of them may ever + * be derived from a branch name, a ref, or a path. */ +export type PreparedCheckoutMissReason = + | 'none_armed' + /** Preparations exist, but none for this repo — it was never warmed, or the pool's size cap + * evicted it for another repo. Distinguished from `none_armed` because it is the signal that + * the cap is thrashing for a multi-project user. */ + | 'repo_mismatch' + | 'base_mismatch' + | 'retarget_too_divergent' + /** The drift check returned no answer. Distinct from `retarget_too_divergent` because that one + * is the bound working as intended, while this one means a possibly cheap retarget was skipped + * anyway. Deliberately a mixed bucket — a blown deadline, a cancelled create, and an ordinary + * Git failure such as a missing ref all land here — so treat a rise as "look at why", not as a + * direct readout of the budget being too small. */ + | 'retarget_unverifiable' + | 'workspace_root_mismatch' + | 'wsl_distro_mismatch' + | 'prepare_failed' + | 'finalize_failed' + | 'checkout_existing_branch' + | 'sparse_checkout' + +/** Whether a create reused a prewarmed checkout, and when it did not, which part of + * the claim key disagreed. `retargeted` marks a hit that had to reset the prepared + * checkout onto a different ref in the same base family. */ +export type PreparedCheckoutOutcome = + | { status: 'hit'; retargeted: boolean } + | { status: 'miss'; reason: PreparedCheckoutMissReason } + export type WorktreeCreateTiming = { totalDurationMs: number phases: WorktreeCreateTimingPhase[] + preparedCheckout?: PreparedCheckoutOutcome } export type CreateSparseCheckoutRequest = {