Prioritize workspace opening over replacement checkout preparation (#23013)

* Prioritize workspace opening over replacement checkout preparation

* Preserve Git hook semantics and exercise preparation edge cases
This commit is contained in:
Neil
2026-09-25 21:31:25 -07:00
committed by GitHub
parent f6eab381ce
commit e13631ee53
10 changed files with 892 additions and 70 deletions
@@ -55,6 +55,137 @@ afterEach(async () => {
})
describe('prepared worktree creation with real Git', () => {
it.each([false, true])(
'attaches the prepared HEAD and runs the hook (base advanced: %s)',
async (advanceBase) => {
const { repoPath, root } = await createRepo()
const preparedPath = join(root, 'prepared checkout')
const finalPath = join(root, 'final checkout')
const hooksPath = join(root, 'hooks')
await mkdir(hooksPath)
await writeFile(
join(hooksPath, 'post-checkout'),
'#!/bin/sh\nprintf \'%s\\n\' "$@" >> checkout-hook.txt\ngit symbolic-ref --short HEAD >> checkout-hook.txt\n',
{ mode: 0o755 }
)
git(repoPath, ['config', 'core.hooksPath', hooksPath])
git(repoPath, ['config', 'branch.autoSetupMerge', 'always'])
await prepareWorktreeCreateCheckout(
repoPath,
preparedPath,
'main',
createWorktreePreparationLockReason('attach-with-hook')
)
expect(existsSync(join(preparedPath, 'checkout-hook.txt'))).toBe(false)
if (advanceBase) {
await writeFile(join(repoPath, 'version.txt'), 'advanced\n')
git(repoPath, ['commit', '--quiet', '-am', 'advance base'])
}
const targetHead = git(repoPath, ['rev-parse', 'HEAD'])
await finalizePreparedWorktree(repoPath, preparedPath, finalPath, 'feature/attached', 'main')
expect(git(finalPath, ['rev-parse', 'HEAD'])).toBe(targetHead)
expect(git(finalPath, ['symbolic-ref', '--short', 'HEAD'])).toBe('feature/attached')
expect(
git(finalPath, ['for-each-ref', '--format=%(upstream)', 'refs/heads/feature/attached'])
).toBe('')
expect(await readFile(join(finalPath, 'checkout-hook.txt'), 'utf8')).toBe(
`${targetHead}\n${targetHead}\n1\nfeature/attached\n`
)
await rm(join(finalPath, 'checkout-hook.txt'))
expect(await readFile(join(finalPath, 'version.txt'), 'utf8')).toBe(
advanceBase ? 'advanced\n' : 'one\n'
)
expect(git(finalPath, ['status', '--porcelain'])).toBe('')
}
)
it('never publishes a branch at a HEAD changed before attachment', async () => {
const { repoPath, root } = await createRepo()
const preparedPath = join(root, 'prepared-race')
const finalPath = join(root, 'final-race')
await prepareWorktreeCreateCheckout(
repoPath,
preparedPath,
'main',
createWorktreePreparationLockReason('head-race')
)
const expectedHead = git(repoPath, ['rev-parse', 'HEAD'])
git(repoPath, ['checkout', '--quiet', '-b', 'other'])
await writeFile(join(repoPath, 'version.txt'), 'other\n')
git(repoPath, ['commit', '--quiet', '-am', 'other commit'])
const otherHead = git(repoPath, ['rev-parse', 'HEAD'])
git(repoPath, ['checkout', '--quiet', 'main'])
const original = gitRunner.gitExecFileAsync
const spy = vi.spyOn(gitRunner, 'gitExecFileAsync').mockImplementation((args, options) => {
if (args.includes('checkout') || args.includes('switch')) {
git(finalPath, ['reset', '--hard', otherHead])
}
return original(args, options)
})
try {
await finalizePreparedWorktree(repoPath, preparedPath, finalPath, 'feature/race', 'main')
} finally {
spy.mockRestore()
}
expect(git(repoPath, ['rev-parse', 'main'])).toBe(expectedHead)
expect(git(finalPath, ['rev-parse', 'HEAD'])).toBe(expectedHead)
expect(git(finalPath, ['symbolic-ref', '--short', 'HEAD'])).toBe('feature/race')
expect(await readFile(join(finalPath, 'version.txt'), 'utf8')).toBe('one\n')
})
it('accepts a commit made by the post-checkout hook during attachment', async () => {
const { repoPath, root } = await createRepo()
const preparedPath = join(root, 'prepared-hook-commit')
const finalPath = join(root, 'final-hook-commit')
const hooksPath = join(root, 'hooks')
await mkdir(hooksPath)
await writeFile(
join(hooksPath, 'post-checkout'),
'#!/bin/sh\nprintf "invoked\\n" >> hook-invocations.txt\ngit add hook-invocations.txt\ngit commit --quiet -m "hook commit"\n',
{ mode: 0o755 }
)
git(repoPath, ['config', 'core.hooksPath', hooksPath])
await prepareWorktreeCreateCheckout(
repoPath,
preparedPath,
'main',
createWorktreePreparationLockReason('hook-commit')
)
const baseHead = git(repoPath, ['rev-parse', 'HEAD'])
await finalizePreparedWorktree(repoPath, preparedPath, finalPath, 'feature/hook-commit', 'main')
expect(git(finalPath, ['symbolic-ref', '--short', 'HEAD'])).toBe('feature/hook-commit')
expect(git(finalPath, ['rev-parse', 'HEAD^'])).toBe(baseHead)
expect(git(finalPath, ['show', '-s', '--format=%s', 'HEAD'])).toBe('hook commit')
expect(await readFile(join(finalPath, 'hook-invocations.txt'), 'utf8')).toBe('invoked\n')
expect(git(finalPath, ['status', '--porcelain'])).toBe('')
})
it('cleans up a branch when post-checkout rejects the attachment', async () => {
const { repoPath, root } = await createRepo()
const preparedPath = join(root, 'prepared-hook-failure')
const finalPath = join(root, 'final-hook-failure')
const hooksPath = join(root, 'hooks')
await mkdir(hooksPath)
await writeFile(join(hooksPath, 'post-checkout'), '#!/bin/sh\nexit 1\n', { mode: 0o755 })
git(repoPath, ['config', 'core.hooksPath', hooksPath])
await prepareWorktreeCreateCheckout(
repoPath,
preparedPath,
'main',
createWorktreePreparationLockReason('hook-failure')
)
await expect(
finalizePreparedWorktree(repoPath, preparedPath, finalPath, 'feature/hook-failure', 'main')
).rejects.toThrow()
expect(existsSync(finalPath)).toBe(false)
expect(git(repoPath, ['branch', '--list', 'feature/hook-failure'])).toBe('')
})
it('retains preparation ownership when the removal command cannot start', async () => {
const fixture = await createRepo()
const repoPath = await realpath(fixture.repoPath)
@@ -65,7 +196,7 @@ describe('prepared worktree creation with real Git', () => {
await prepareWorktreeCreateCheckout(repoPath, preparedPath, 'main', lockReason)
const original = gitRunner.gitExecFileAsync
const spy = vi.spyOn(gitRunner, 'gitExecFileAsync').mockImplementation((args, options) => {
if (args.includes('remove') && args.includes(preparedPath)) {
if (args.includes('remove') && args.some((arg) => areWorktreePathsEqual(arg, preparedPath))) {
return Promise.reject(new Error('injected removal launch failure'))
}
return original(args, options)
@@ -113,7 +244,7 @@ describe('prepared worktree creation with real Git', () => {
const spy = vi
.spyOn(gitRunner, 'gitExecFileAsync')
.mockImplementation(async (args, options) => {
if (args.includes('remove') && args.includes(stalePath)) {
if (args.includes('remove') && args.some((arg) => areWorktreePathsEqual(arg, stalePath))) {
markRemovalStarted()
await removalGate
}
@@ -145,7 +276,8 @@ describe('prepared worktree creation with real Git', () => {
expect(existsSync(stalePath)).toBe(false)
const remaining = await listWorktrees(repoPath, { includeCreatePreparations: true })
expect(remaining).toHaveLength(2)
expect(remaining.map((w) => w.path)).toEqual(expect.arrayContaining([repoPath, finalPath]))
expect(remaining.some((w) => areWorktreePathsEqual(w.path, repoPath))).toBe(true)
expect(remaining.some((w) => areWorktreePathsEqual(w.path, finalPath))).toBe(true)
expect(hasPendingStalePreparationCleanup()).toBe(false)
} finally {
releaseRemoval()
+5 -1
View File
@@ -2741,7 +2741,8 @@ async function performLocalWorktreeCreate(
branch: branchName,
baseBranch,
refreshLocalBaseRef: settings.refreshLocalBaseRefOnWorktreeCreate,
options: preparedWorktreeOptions
options: preparedWorktreeOptions,
timing
})
timing.recordPreparedCheckout(
prepared.status === 'hit'
@@ -2754,6 +2755,9 @@ async function performLocalWorktreeCreate(
rearm.fire = prepared.rearm
return prepared.result
}
if (prepared.rearm) {
rearm.fire = prepared.rearm
}
} else {
timing.recordPreparedCheckout({
status: 'miss',
@@ -152,6 +152,9 @@ export async function createRuntimeLocalGitWorktree(args: {
})
: null
// This path has no create-span recorder, so the miss reason is only observable on the IPC path.
if (preparedAttempt?.status === 'miss' && preparedAttempt.rearm) {
args.rearm.fire = preparedAttempt.rearm
}
if (preparedAttempt?.status === 'hit') {
addResult = preparedAttempt.result
// Deferred, not fired: re-arming is a full `reset --hard`, and the caller still has
@@ -1,4 +1,5 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { resolve } from 'node:path'
import type { Store } from '../persistence'
import type { WorktreeMeta } from '../../shared/worktree/meta-types'
import type { RuntimeManagedWorktreeCreateArgs } from './runtime-managed-worktree-create-types'
@@ -32,7 +33,7 @@ const mocks = vi.hoisted(() => ({
resolveInclude: vi.fn<() => Promise<string[]>>(),
copyPaths: vi.fn<() => Promise<string[]>>(),
created: {
path: '/worktrees/app',
path: '',
head: 'abc123',
branch: 'app',
isBare: false,
@@ -82,6 +83,8 @@ vi.mock('../ipc/worktree-symlinks', () => ({
import { createRuntimeLocalManagedWorktree } from './runtime-local-worktree-create'
import type { PreparationRearmHolder } from '../worktree-create-preparation'
const worktreePath = resolve('/worktrees', 'app')
function createWorktree(
request: Partial<RuntimeManagedWorktreeCreateArgs> = {},
rearm: PreparationRearmHolder = { fire: () => {} }
@@ -112,6 +115,7 @@ function createWorktree(
beforeEach(() => {
vi.resetAllMocks()
mocks.created.path = worktreePath
mocks.routing.mockReturnValue({})
mocks.defaultBase.mockImplementation(async () => {
expect(resolveGitAdmissionTier()).toBe('interactive')
@@ -142,6 +146,36 @@ beforeEach(() => {
})
describe('runtime prepared-worktree replenishment', () => {
it('keeps a failed claim reserved through the normal-add fallback', async () => {
mocks.consume.mockResolvedValue({
status: 'miss',
reason: 'finalize_failed',
rearm: mocks.rearm
})
const rearm: PreparationRearmHolder = { fire: () => {} }
await createWorktree({}, rearm)
expect(mocks.add).toHaveBeenCalledOnce()
expect(mocks.rearm).not.toHaveBeenCalled()
rearm.fire()
expect(mocks.rearm).toHaveBeenCalledOnce()
})
it('keeps the failed claim release available when the fallback also fails', async () => {
mocks.consume.mockResolvedValue({
status: 'miss',
reason: 'prepare_failed',
rearm: mocks.rearm
})
mocks.add.mockRejectedValue(new Error('normal add failed'))
const rearm: PreparationRearmHolder = { fire: () => {} }
await expect(createWorktree({}, rearm)).rejects.toThrow('normal add failed')
expect(mocks.rearm).not.toHaveBeenCalled()
rearm.fire()
expect(mocks.rearm).toHaveBeenCalledOnce()
})
it('leaves the re-arm holder armed but unfired once probes and include copies finish', async () => {
const rearm: PreparationRearmHolder = { fire: () => {} }
let finishProbe!: (paths: string[]) => void
@@ -207,8 +241,8 @@ describe('runtime create Git priority', () => {
expect(mocks.remoteBase).toHaveBeenCalledWith('/repo', 'main', options)
expect(mocks.hasBase).toHaveBeenCalledWith('/repo', 'main', options)
expect(mocks.consume).toHaveBeenCalledWith(expect.objectContaining({ options }))
expect(mocks.pushTarget).toHaveBeenCalledWith('/worktrees/app', 'app', target, options)
expect(mocks.listing).toHaveBeenCalledWith('/repo', '/worktrees/app', 'app', options)
expect(mocks.pushTarget).toHaveBeenCalledWith(worktreePath, 'app', target, options)
expect(mocks.listing).toHaveBeenCalledWith('/repo', worktreePath, 'app', options)
expect(mocks.resolveShared).toHaveBeenCalledWith('/repo', options)
expect(mocks.resolveInclude).toHaveBeenCalledWith('/repo', options)
}
@@ -240,7 +274,7 @@ describe('runtime create Git priority', () => {
}
)
try {
await expect(createWorktree()).resolves.toHaveProperty('worktreePath', '/worktrees/app')
await expect(createWorktree()).resolves.toHaveProperty('worktreePath', worktreePath)
expect(mocks.add).toHaveBeenCalledOnce()
} finally {
blocker.release()
@@ -262,7 +296,7 @@ describe('runtime create Git priority', () => {
expect(mocks.refresh).toHaveBeenCalledWith('/repo', base, options)
expect(mocks.addSparse).toHaveBeenCalledWith(
'/repo',
'/worktrees/app',
worktreePath,
'app',
['src'],
'origin/main',
@@ -0,0 +1,369 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import type { Store } from './persistence'
import type { Repo } from '../shared/repo-types'
import type { AddWorktreeResult } from './git/worktree'
const mocks = vi.hoisted(() => ({
mkdir: vi.fn(),
listWorktreeGraph: vi.fn(),
prepare: vi.fn(),
finalize: vi.fn(),
discard: vi.fn(),
computeWorkspaceRootAsync: vi.fn(),
getWorktreeOptions: vi.fn(),
resolveBaseRef: vi.fn(),
measureDivergence: vi.fn()
}))
vi.mock('node:fs/promises', () => ({ mkdir: mocks.mkdir }))
vi.mock('./git/worktree', () => ({ listWorktreeGraph: mocks.listWorktreeGraph }))
vi.mock('./git/worktree-create-preparation', () => ({
prepareWorktreeCreateCheckout: mocks.prepare,
finalizePreparedWorktree: mocks.finalize,
discardPreparedWorktree: mocks.discard,
unlockPreparedWorktree: vi.fn()
}))
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
}))
vi.mock('./ipc/worktree-logic', () => ({
computeWorkspaceRootAsync: mocks.computeWorkspaceRootAsync,
getWorktreePathSettings: () => ({ workspaceDir: '/workspace', nestWorkspaces: false })
}))
import {
_resetWorktreeCreatePreparationsForTests,
consumePreparedWorktreeCreate,
hasPendingWorktreeCreatePreparations,
prepareWorktreeCreateForRepo
} from './worktree-create-preparation'
import {
listPreparations,
releasePreparationClaim,
startPreparation,
takePreparation
} from './worktree-create-preparation-pool'
const repo: Repo = {
id: 'repo-1',
path: '/repo',
displayName: 'Repo',
badgeColor: 'blue',
addedAt: 0
}
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The tested path reads only getSettings from Store.
const store = { getSettings: () => ({}) } as unknown as Store
const flushBackgroundWork = (): Promise<void> => new Promise((resolve) => setTimeout(resolve, 0))
function consume(baseBranch = 'origin/main') {
return consumePreparedWorktreeCreate({
repoPath: repo.path,
workspaceRoot: '/workspace',
worktreePath: '/workspace/new-worktree',
branch: 'feature/new-worktree',
baseBranch
})
}
beforeEach(() => {
mocks.mkdir.mockReset().mockResolvedValue(undefined)
mocks.listWorktreeGraph.mockReset().mockResolvedValue([])
mocks.prepare.mockReset().mockResolvedValue(undefined)
mocks.finalize.mockReset().mockResolvedValue({})
mocks.discard.mockReset().mockResolvedValue(undefined)
mocks.computeWorkspaceRootAsync.mockReset().mockResolvedValue('/workspace')
mocks.getWorktreeOptions.mockReset().mockReturnValue({})
mocks.resolveBaseRef
.mockReset()
.mockImplementation(async (_path: string, base: string) =>
base === 'main'
? 'refs/heads/main'
: base === 'other/main'
? 'refs/remotes/other/main'
: 'refs/remotes/origin/main'
)
mocks.measureDivergence.mockReset().mockResolvedValue('within')
})
afterEach(async () => {
await _resetWorktreeCreatePreparationsForTests()
})
describe('claimed worktree preparation', () => {
it('defers prefetch through checkout, finalization, and the remaining create work', async () => {
const checkout = Promise.withResolvers<void>()
const checkoutStarted = Promise.withResolvers<void>()
const finalize = Promise.withResolvers<AddWorktreeResult>()
const finalizeStarted = Promise.withResolvers<void>()
mocks.prepare.mockImplementationOnce(() => {
checkoutStarted.resolve()
return checkout.promise
})
mocks.finalize.mockImplementationOnce(() => {
finalizeStarted.resolve()
return finalize.promise
})
const preparation = prepareWorktreeCreateForRepo(store, repo, 'origin/main')
await checkoutStarted.promise
const create = consume()
await prepareWorktreeCreateForRepo(store, repo, 'origin/main')
expect(mocks.prepare).toHaveBeenCalledTimes(1)
checkout.resolve()
await preparation
await finalizeStarted.promise
await prepareWorktreeCreateForRepo(store, repo, 'origin/main')
expect(mocks.prepare).toHaveBeenCalledTimes(1)
finalize.resolve({})
const result = await create
expect(result.status).toBe('hit')
await prepareWorktreeCreateForRepo(store, repo, 'origin/main')
expect(mocks.prepare).toHaveBeenCalledTimes(1)
result.rearm?.()
await flushBackgroundWork()
expect(mocks.prepare).toHaveBeenCalledTimes(2)
})
it('coalesces mid-create prefetches and releases once at create completion', async () => {
await prepareWorktreeCreateForRepo(store, repo, 'origin/main')
const result = await consume()
expect(result.status).toBe('hit')
expect(hasPendingWorktreeCreatePreparations()).toBe(true)
await Promise.all([
prepareWorktreeCreateForRepo(store, repo, 'origin/main'),
prepareWorktreeCreateForRepo(store, repo, 'origin/main')
])
expect(mocks.prepare).toHaveBeenCalledTimes(1)
if (result.status === 'hit') {
result.rearm()
result.rearm()
}
await flushBackgroundWork()
expect(mocks.prepare).toHaveBeenCalledTimes(2)
expect(hasPendingWorktreeCreatePreparations()).toBe(true)
})
it('does not let two creates claim the same prepared checkout', async () => {
await prepareWorktreeCreateForRepo(store, repo, 'origin/main')
const [first, second] = await Promise.all([consume(), consume()])
expect([first.status, second.status].sort()).toEqual(['hit', 'miss'])
expect(mocks.finalize).toHaveBeenCalledOnce()
expect(hasPendingWorktreeCreatePreparations()).toBe(true)
first.rearm?.()
second.rearm?.()
expect(hasPendingWorktreeCreatePreparations()).toBe(false)
})
it('keeps an explicit prefetch ahead of an automatic replacement for the same key', async () => {
await prepareWorktreeCreateForRepo(store, repo, 'origin/main')
const entry = listPreparations()[0]
if (!entry) {
throw new Error('expected a prepared checkout')
}
const claim = takePreparation(entry)
const base = {
repoPath: repo.path,
workspaceRoot: '/workspace',
baseBranch: 'origin/main',
canonicalBase: 'refs/remotes/origin/main'
}
await startPreparation({ ...base, options: {} }, 'automatic')
await startPreparation({ ...base, options: { admissionTier: 'background' } })
await startPreparation({ ...base, options: {} }, 'automatic')
const released = releasePreparationClaim(claim)
expect(released.pendingPreparations).toEqual([
{
kind: 'explicit',
args: { ...base, options: { admissionTier: 'background' } }
}
])
expect(releasePreparationClaim(claim)).toEqual({ released: false, pendingPreparations: [] })
})
it('allows a fresh prefetch after an isolated create completes', async () => {
await prepareWorktreeCreateForRepo(store, repo, 'origin/main')
const result = await consume()
expect(result.status).toBe('hit')
if (result.status === 'hit') {
result.rearm()
}
expect(hasPendingWorktreeCreatePreparations()).toBe(false)
await prepareWorktreeCreateForRepo(store, repo, 'origin/main')
expect(mocks.prepare).toHaveBeenCalledTimes(2)
})
it('does not repeat a burst replacement when release runs twice', async () => {
await prepareWorktreeCreateForRepo(store, repo, 'origin/main')
const first = await consume()
if (first.status === 'hit') {
first.rearm()
}
await prepareWorktreeCreateForRepo(store, repo, 'origin/main')
const second = await consume()
expect(second.status).toBe('hit')
if (second.status === 'hit') {
second.rearm()
second.rearm()
}
await flushBackgroundWork()
expect(mocks.prepare).toHaveBeenCalledTimes(3)
})
it('reserves both the prepared and requested canonical bases on a retarget', async () => {
await prepareWorktreeCreateForRepo(store, repo, 'origin/main')
const result = await consume('main')
expect(result).toMatchObject({ status: 'hit', retargeted: true })
await prepareWorktreeCreateForRepo(store, repo, 'main')
expect(mocks.prepare).toHaveBeenCalledTimes(1)
if (result.status === 'hit') {
result.rearm()
}
await flushBackgroundWork()
expect(mocks.prepare).toHaveBeenCalledTimes(2)
expect(mocks.prepare.mock.calls[1]?.[2]).toBe('refs/heads/main')
})
it('preserves distinct prefetch bases while a retargeted create finishes', async () => {
await prepareWorktreeCreateForRepo(store, repo, 'origin/main')
const result = await consume('main')
expect(result.status).toBe('hit')
await prepareWorktreeCreateForRepo(store, repo, 'origin/main')
await prepareWorktreeCreateForRepo(store, repo, 'main')
expect(mocks.prepare).toHaveBeenCalledTimes(1)
if (result.status === 'hit') {
result.rearm()
}
await flushBackgroundWork()
expect(mocks.prepare).toHaveBeenCalledTimes(3)
})
it('passes a pending prefetch to another create claiming the same requested base', async () => {
await prepareWorktreeCreateForRepo(store, repo, 'origin/main')
await prepareWorktreeCreateForRepo(store, repo, 'other/main')
const first = await consume('main')
const second = await consume('main')
expect(first.status).toBe('hit')
expect(second.status).toBe('hit')
await prepareWorktreeCreateForRepo(store, repo, 'main')
expect(mocks.prepare).toHaveBeenCalledTimes(2)
if (second.status === 'hit') {
second.rearm()
}
await flushBackgroundWork()
expect(mocks.prepare).toHaveBeenCalledTimes(2)
if (first.status === 'hit') {
first.rearm()
}
await flushBackgroundWork()
expect(mocks.prepare).toHaveBeenCalledTimes(3)
})
it('keeps a burst replacement when the remaining claim is isolated', async () => {
let timestamp = 1_000
const now = vi.spyOn(Date, 'now').mockImplementation(() => timestamp++)
try {
await prepareWorktreeCreateForRepo(store, repo, 'other/main')
const seed = await consume('other/main')
expect(seed.status).toBe('hit')
seed.rearm?.()
await prepareWorktreeCreateForRepo(store, repo, 'origin/main')
await prepareWorktreeCreateForRepo(store, repo, 'other/main')
const burst = await consume('main')
const isolated = await consume('main')
expect(burst).toMatchObject({ status: 'hit', retargeted: true })
expect(isolated).toMatchObject({ status: 'hit', retargeted: true })
expect(mocks.prepare).toHaveBeenCalledTimes(3)
burst.rearm?.()
await flushBackgroundWork()
expect(mocks.prepare).toHaveBeenCalledTimes(3)
isolated.rearm?.()
await flushBackgroundWork()
expect(mocks.prepare).toHaveBeenCalledTimes(4)
expect(mocks.prepare.mock.calls[3]?.[2]).toBe('refs/heads/main')
} finally {
now.mockRestore()
}
})
it('does not hold another repo, workspace root, or Git host behind the claim', async () => {
await prepareWorktreeCreateForRepo(store, repo, 'origin/main')
const result = await consume()
expect(result.status).toBe('hit')
await prepareWorktreeCreateForRepo(store, { ...repo, path: '/other-repo' }, 'origin/main')
mocks.computeWorkspaceRootAsync.mockResolvedValueOnce('/other-workspace')
await prepareWorktreeCreateForRepo(store, repo, 'origin/main')
mocks.getWorktreeOptions.mockReturnValue({ wslDistro: 'Ubuntu' })
await prepareWorktreeCreateForRepo(store, repo, 'origin/main')
expect(mocks.prepare).toHaveBeenCalledTimes(4)
if (result.status === 'hit') {
result.rearm()
}
})
it('releases after failed finalization has discarded the claimed checkout', async () => {
await prepareWorktreeCreateForRepo(store, repo, 'origin/main')
let failFinalization!: (error: Error) => void
mocks.finalize.mockReturnValueOnce(
new Promise((_resolve, reject) => {
failFinalization = reject
})
)
const create = consume()
await flushBackgroundWork()
await prepareWorktreeCreateForRepo(store, repo, 'origin/main')
expect(mocks.prepare).toHaveBeenCalledTimes(1)
failFinalization(new Error('finalize failed'))
const result = await create
expect(result).toMatchObject({ status: 'miss', reason: 'finalize_failed' })
expect(mocks.discard).toHaveBeenCalledTimes(1)
await flushBackgroundWork()
expect(mocks.prepare).toHaveBeenCalledTimes(1)
result.rearm?.()
await flushBackgroundWork()
expect(mocks.prepare).toHaveBeenCalledTimes(2)
})
it('holds an explicit prefetch through a claimed checkout failure', async () => {
let failPreparation!: (error: Error) => void
mocks.prepare.mockReturnValueOnce(
new Promise((_resolve, reject) => {
failPreparation = reject
})
)
const initialPreparation = prepareWorktreeCreateForRepo(store, repo, 'origin/main')
const preparationFailure = initialPreparation.catch(() => {})
await flushBackgroundWork()
const create = consume()
await flushBackgroundWork()
await prepareWorktreeCreateForRepo(store, repo, 'origin/main')
expect(mocks.prepare).toHaveBeenCalledTimes(1)
failPreparation(new Error('checkout failed'))
const result = await create
await preparationFailure
expect(result).toMatchObject({ status: 'miss', reason: 'prepare_failed' })
expect(mocks.prepare).toHaveBeenCalledTimes(1)
result.rearm?.()
await flushBackgroundWork()
expect(mocks.prepare).toHaveBeenCalledTimes(2)
})
})
+86 -3
View File
@@ -51,7 +51,18 @@ export type StartPreparationArgs = {
options: AddWorktreeOptions
}
export type DeferredPreparation = {
args: StartPreparationArgs
kind: 'explicit' | 'automatic'
}
const preparations = new Map<string, PreparationEntry>()
export type PreparationClaim = {
entry: PreparationEntry
requestedKey: string
pendingPreparations: Map<string, DeferredPreparation>
}
const claims = new Set<PreparationClaim>()
/** One repo on one Git host: the scope a stranded discard is retried under. */
function preparationHostKey(repoPathKey: string, wslDistro: string): string {
@@ -60,7 +71,7 @@ function preparationHostKey(repoPathKey: string, wslDistro: string): string {
/** A prepared checkout is a create that is either in flight or imminent. */
export function hasPendingPreparations(): boolean {
return preparations.size > 0 || hasPendingStalePreparationCleanup()
return preparations.size > 0 || claims.size > 0 || hasPendingStalePreparationCleanup()
}
function pathOps(path: string): Pick<typeof posix, 'dirname' | 'join'> {
@@ -147,12 +158,83 @@ export function findPreparation(
/** 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 {
export function takePreparation(
entry: PreparationEntry,
requestedCanonicalBase = entry.canonicalBase
): PreparationClaim {
preparations.delete(entry.key)
clearTimeout(entry.expiration)
const requestedKey = preparationEntryKey(
entry.repoPathKey,
entry.workspaceRootKey,
requestedCanonicalBase,
entry.wslDistro
)
const claim = { entry, requestedKey, pendingPreparations: new Map<string, DeferredPreparation>() }
claims.add(claim)
return claim
}
export function startPreparation(args: StartPreparationArgs): Promise<void> {
function matchingClaim(args: StartPreparationArgs): PreparationClaim | undefined {
const key = preparationEntryKey(
preparationPathKey(args.repoPath),
preparationPathKey(args.workspaceRoot),
args.canonicalBase,
args.options.wslDistro ?? ''
)
return [...claims]
.toReversed()
.find((claim) => claim.entry.key === key || claim.requestedKey === key)
}
/** Preserve one request per canonical key, with explicit prefetch taking precedence. */
function deferPreparationForClaim(
args: StartPreparationArgs,
kind: DeferredPreparation['kind']
): boolean {
const matching = matchingClaim(args)
if (!matching) {
return false
}
const key = preparationEntryKey(
preparationPathKey(args.repoPath),
preparationPathKey(args.workspaceRoot),
args.canonicalBase,
args.options.wslDistro ?? ''
)
if (kind === 'explicit' || !matching.pendingPreparations.has(key)) {
matching.pendingPreparations.set(key, { args, kind })
}
return true
}
/** A second release is inert, including after a test reset. */
export function releasePreparationClaim(claim: PreparationClaim): {
released: boolean
pendingPreparations: DeferredPreparation[]
} {
if (!claims.delete(claim)) {
return { released: false, pendingPreparations: [] }
}
return { released: true, pendingPreparations: [...claim.pendingPreparations.values()] }
}
export function startPreparation(
args: StartPreparationArgs,
kind: DeferredPreparation['kind'] = 'explicit'
): Promise<void> {
const existing = findPreparation(
preparationPathKey(args.repoPath),
preparationPathKey(args.workspaceRoot),
args.canonicalBase,
args.options.wslDistro ?? ''
)
if (existing) {
return existing.ready
}
if (deferPreparationForClaim(args, kind)) {
return Promise.resolve()
}
return worktreePreparationGit.run(() => startBackgroundPreparation(args))
}
@@ -227,6 +309,7 @@ function startBackgroundPreparation({
export async function _resetPreparationPoolForTests(): Promise<void> {
const entries = [...preparations.values()]
preparations.clear()
claims.clear()
await resetStalePreparationCleanupForTests()
await Promise.all(
entries.map(async (entry) => {
@@ -0,0 +1,168 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import type { AddWorktreeResult } from './git/worktree'
const mocks = vi.hoisted(() => ({
mkdir: vi.fn(),
prepare: vi.fn(),
finalize: vi.fn(),
discard: vi.fn(),
listWorktreeGraph: vi.fn()
}))
vi.mock('node:fs/promises', () => ({ mkdir: mocks.mkdir }))
vi.mock('./git/worktree', () => ({ listWorktreeGraph: mocks.listWorktreeGraph }))
vi.mock('./git/worktree-create-preparation', () => ({
prepareWorktreeCreateCheckout: mocks.prepare,
finalizePreparedWorktree: mocks.finalize,
discardPreparedWorktree: mocks.discard,
unlockPreparedWorktree: vi.fn()
}))
vi.mock('./git/worktree-base-ref-probe', () => ({ resolveLocalWorktreeBaseRef: vi.fn() }))
vi.mock('./git/worktree-base-divergence', () => ({ measureRetargetDivergence: vi.fn() }))
vi.mock('./project-runtime-git-options', () => ({
getLocalProjectWorktreeGitOptions: vi.fn(),
getWorktreeMirrorDistro: vi.fn()
}))
vi.mock('./ipc/worktree-logic', () => ({
computeWorkspaceRootAsync: vi.fn(),
getWorktreePathSettings: vi.fn()
}))
import {
_resetWorktreeCreatePreparationsForTests,
consumePreparedWorktreeCreate
} from './worktree-create-preparation'
import { startPreparation } from './worktree-create-preparation-pool'
import { createWorktreeCreateTimingRecorder } from './worktree-create-timing'
const request = {
repoPath: '/repo',
workspaceRoot: '/workspace',
worktreePath: '/workspace/feature',
branch: 'feature',
baseBranch: 'origin/main'
}
function prepare() {
return startPreparation({
repoPath: request.repoPath,
workspaceRoot: request.workspaceRoot,
baseBranch: request.baseBranch,
canonicalBase: 'refs/remotes/origin/main',
options: {}
})
}
beforeEach(() => {
mocks.mkdir.mockReset().mockResolvedValue(undefined)
mocks.prepare.mockReset().mockResolvedValue(undefined)
mocks.finalize.mockReset().mockResolvedValue({})
mocks.discard.mockReset().mockResolvedValue(undefined)
mocks.listWorktreeGraph.mockReset().mockResolvedValue([])
})
afterEach(async () => {
await _resetWorktreeCreatePreparationsForTests()
vi.restoreAllMocks()
})
describe('prepared checkout create timing', () => {
it('separates the remaining preparation wait from finalization', async () => {
const checkoutStarted = Promise.withResolvers<void>()
const checkout = Promise.withResolvers<void>()
const finalizeStarted = Promise.withResolvers<void>()
const finalize = Promise.withResolvers<AddWorktreeResult>()
mocks.prepare.mockImplementation(() => {
checkoutStarted.resolve()
return checkout.promise
})
mocks.finalize.mockImplementation(() => {
finalizeStarted.resolve()
return finalize.promise
})
const preparation = prepare()
await checkoutStarted.promise
let now = 40
const timing = createWorktreeCreateTimingRecorder(() => now)
const create = timing.time('git_worktree_add', () =>
consumePreparedWorktreeCreate({ ...request, timing })
)
now = 150
checkout.resolve()
await finalizeStarted.promise
now = 180
finalize.resolve({})
expect(await create).toMatchObject({ status: 'hit', retargeted: false })
await preparation
expect(timing.finish()).toEqual({
totalDurationMs: 140,
phases: [
{ phase: 'prepared_checkout_wait', startedAtMs: 0, durationMs: 110 },
{ phase: 'prepared_checkout_finalize', startedAtMs: 110, durationMs: 30 },
{ phase: 'git_worktree_add', startedAtMs: 0, durationMs: 140 }
]
})
})
it('records the wait when preparation fails and the create must fall back', async () => {
const checkoutStarted = Promise.withResolvers<void>()
const checkout = Promise.withResolvers<void>()
mocks.prepare.mockImplementation(() => {
checkoutStarted.resolve()
return checkout.promise
})
const preparation = Promise.allSettled([prepare()])
await checkoutStarted.promise
let now = 0
const timing = createWorktreeCreateTimingRecorder(() => now)
const create = consumePreparedWorktreeCreate({ ...request, timing })
now = 250
checkout.reject(new Error('checkout failed'))
expect(await create).toEqual({
status: 'miss',
reason: 'prepare_failed',
rearm: expect.any(Function)
})
await preparation
expect(timing.finish().phases).toEqual([
{ phase: 'prepared_checkout_wait', startedAtMs: 0, durationMs: 250 }
])
expect(mocks.finalize).not.toHaveBeenCalled()
})
it('records a failed finalization before its fallback cleanup', async () => {
vi.spyOn(console, 'warn').mockImplementation(() => {})
await prepare()
let now = 0
const timing = createWorktreeCreateTimingRecorder(() => now)
mocks.finalize.mockImplementation(async () => {
now = 80
throw new Error('move failed')
})
mocks.discard.mockImplementation(async () => {
now = 100
})
expect(await consumePreparedWorktreeCreate({ ...request, timing })).toEqual({
status: 'miss',
reason: 'finalize_failed',
rearm: expect.any(Function)
})
expect(timing.finish().phases).toEqual([
{ phase: 'prepared_checkout_wait', startedAtMs: 0, durationMs: 0 },
{ phase: 'prepared_checkout_finalize', startedAtMs: 0, durationMs: 80 }
])
})
it('does not report a preparation wait when no checkout was armed', async () => {
const timing = createWorktreeCreateTimingRecorder(() => 0)
expect(await consumePreparedWorktreeCreate({ ...request, timing })).toEqual({
status: 'miss',
reason: 'none_armed'
})
expect(timing.finish().phases).toEqual([])
})
})
+4 -5
View File
@@ -555,7 +555,7 @@ describe('worktree create preparation registry', () => {
branch: 'feature/test',
baseBranch: 'origin/main'
})
).resolves.toEqual({ status: 'miss', reason: 'finalize_failed' })
).resolves.toMatchObject({ status: 'miss', reason: 'finalize_failed' })
expect(mocks.mkdir).toHaveBeenCalledWith('/workspace', { recursive: true })
expect(mocks.discard).toHaveBeenCalledTimes(1)
})
@@ -637,9 +637,7 @@ describe('worktree create preparation registry', () => {
expect(mocks.prepareCheckout).toHaveBeenCalledTimes(1)
})
// `startPreparation` overwrites the map entry outright, so a thunk that armed over a prefetch
// would leave that prefetch's locked checkout on disk with nothing holding a reference to it.
it('skips the deferred re-arm when a prefetch armed the same key mid-create', async () => {
it('starts one explicit prefetch after the create completes', async () => {
await prepareWorktreeCreateForRepo(store, repo, 'origin/main')
await consumeOnce('first')
await prepareWorktreeCreateForRepo(store, repo, 'origin/main')
@@ -655,6 +653,7 @@ describe('worktree create preparation registry', () => {
// The user reopens the composer while the create is still finishing.
await prepareWorktreeCreateForRepo(store, repo, 'origin/main')
expect(mocks.prepareCheckout).toHaveBeenCalledTimes(2)
mocks.prepareCheckout.mockClear()
if (attempt.status === 'hit') {
@@ -662,7 +661,7 @@ describe('worktree create preparation registry', () => {
}
await flushBackgroundWork()
expect(mocks.prepareCheckout).not.toHaveBeenCalled()
expect(mocks.prepareCheckout).toHaveBeenCalledTimes(1)
})
it('does not re-arm when finalization failed', async () => {
+78 -52
View File
@@ -12,11 +12,13 @@ import { resolveLocalWorktreeBaseRef } from './git/worktree-base-ref-probe'
import { preparationPathKey, selectPreparationForCreate } from './worktree-create-preparation-claim'
import {
_resetPreparationPoolForTests,
findPreparation,
hasPendingPreparations,
listPreparations,
releasePreparationClaim,
startPreparation,
takePreparation,
type DeferredPreparation,
type PreparationClaim,
type PreparationEntry
} from './worktree-create-preparation-pool'
import {
@@ -33,6 +35,7 @@ import {
resetPreparationConsumeHistoryForTests
} from './worktree-create-preparation-burst'
import { toHostFilesystemPath } from './host-tree-removal'
import type { WorktreeCreateTimingRecorder } from './worktree-create-timing'
export {
WORKTREE_CREATE_PREPARATION_LIMIT,
@@ -56,7 +59,7 @@ export type PreparedWorktreeCreateAttempt =
/** Run after materialization/startup completes, before returning the create result. */
rearm: () => void
}
| { status: 'miss'; reason: PreparedCheckoutMissReason }
| { status: 'miss'; reason: PreparedCheckoutMissReason; rearm?: () => void }
type ConsumePreparedWorktreeArgs = {
repoPath: string
@@ -66,6 +69,7 @@ type ConsumePreparedWorktreeArgs = {
baseBranch: string
refreshLocalBaseRef?: boolean
options?: AddWorktreeOptions
timing?: Pick<WorktreeCreateTimingRecorder, 'time'>
}
function canonicalBaseRef(
@@ -107,16 +111,6 @@ async function prepareWorktreeCreateInBackground(
getWorktreePathSettings(repo, store.getSettings(), getWorktreeMirrorDistro(store, repo))
)
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({
repoPath: repo.path,
workspaceRoot,
@@ -127,8 +121,14 @@ async function prepareWorktreeCreateInBackground(
}
type ClaimedPreparation =
| { status: 'claimed'; entry: PreparationEntry; retargeted: boolean; canonicalBase: string }
| { status: 'miss'; reason: PreparedCheckoutMissReason }
| {
status: 'claimed'
entry: PreparationEntry
reservation: PreparationClaim
retargeted: boolean
canonicalBase: string
}
| { status: 'miss'; reason: PreparedCheckoutMissReason; rearm?: () => void }
async function claimPreparedWorktree(
args: ConsumePreparedWorktreeArgs,
@@ -187,17 +187,34 @@ async function claimPreparedWorktree(
}
}
const entry = selection.candidate
takePreparation(entry)
const reservation = takePreparation(entry, selection.canonicalBase)
try {
await entry.ready
await (args.timing
? args.timing.time('prepared_checkout_wait', () => entry.ready)
: entry.ready)
return {
status: 'claimed',
entry,
reservation,
retargeted: selection.kind === 'retarget',
canonicalBase: selection.canonicalBase
}
} catch {
return { status: 'miss', reason: 'prepare_failed' }
return { status: 'miss', reason: 'prepare_failed', rearm: releaseClaimAfterCreate(reservation) }
}
}
function startDeferredPreparation(preparation: DeferredPreparation): void {
void startPreparation(preparation.args, preparation.kind).catch(() => {
// A later create still has the normal add path if speculative preparation fails.
})
}
function releaseClaimAfterCreate(reservation: PreparationClaim): () => void {
return () => {
for (const preparation of releasePreparationClaim(reservation).pendingPreparations) {
startDeferredPreparation(preparation)
}
}
}
@@ -209,37 +226,38 @@ async function claimPreparedWorktree(
* Returns a thunk rather than launching: the replacement is a full `reset --hard`, which on a
* large repo holds a general admission slot for tens of seconds. Started mid-create it competes
* with the create's own git, so the caller runs it after materialization/startup completes. The burst
* bookkeeping still happens here — 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. */
* bookkeeping still happens here so a later create is recognized as part of a burst. An explicit
* prefetch during this create takes precedence over the burst replacement at release. */
function deferRearmPreparation(
entry: PreparationEntry,
reservation: PreparationClaim,
baseBranch: string,
canonicalBase: string
): () => void {
const continuesBurst = recordPreparationConsume(entry.key)
const alreadyArmed = (): boolean =>
findPreparation(entry.repoPathKey, entry.workspaceRootKey, canonicalBase, entry.wslDistro) !==
undefined
if (!continuesBurst || alreadyArmed()) {
return () => {}
}
return () => {
// Re-checked here, not only at consume time: `startPreparation` overwrites the map entry
// outright, so arming over a prefetch that landed during the create would strand its
// checkout on disk with no owner to discard it.
if (alreadyArmed()) {
const { released, pendingPreparations } = releasePreparationClaim(reservation)
if (!released) {
return
}
void startPreparation({
repoPath: entry.repoPath,
workspaceRoot: entry.workspaceRoot,
baseBranch,
canonicalBase,
options: entry.options
}).catch(() => {
// Why: a warm-up failure is recovered by the normal add on the next create.
})
const requestedBaseArmed = pendingPreparations.some(
(preparation) => preparation.args.canonicalBase === canonicalBase
)
for (const preparation of pendingPreparations) {
startDeferredPreparation(preparation)
}
if (continuesBurst && !requestedBaseArmed) {
startDeferredPreparation({
args: {
repoPath: entry.repoPath,
workspaceRoot: entry.workspaceRoot,
baseBranch,
canonicalBase,
options: entry.options
},
kind: 'automatic'
})
}
}
}
@@ -249,9 +267,9 @@ export async function consumePreparedWorktreeCreate(
const options = args.options ?? {}
const claim = await claimPreparedWorktree(args, options)
if (claim.status === 'miss') {
return { status: 'miss', reason: claim.reason }
return { status: 'miss', reason: claim.reason, ...(claim.rearm ? { rearm: claim.rearm } : {}) }
}
const { entry } = claim
const { entry, reservation } = claim
try {
const parentDir = isWindowsAbsolutePathLike(args.worktreePath)
? win32.dirname(args.worktreePath)
@@ -259,18 +277,22 @@ export async function consumePreparedWorktreeCreate(
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,
args.worktreePath,
args.branch,
args.baseBranch,
args.refreshLocalBaseRef,
options
)
const finalize = (): Promise<AddWorktreeResult> =>
finalizePreparedWorktree(
args.repoPath,
entry.preparedPath,
args.worktreePath,
args.branch,
args.baseBranch,
args.refreshLocalBaseRef,
options
)
const result = args.timing
? await args.timing.time('prepared_checkout_finalize', finalize)
: await finalize()
// 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.
const rearm = deferRearmPreparation(entry, args.baseBranch, claim.canonicalBase)
const rearm = deferRearmPreparation(entry, reservation, args.baseBranch, claim.canonicalBase)
return { status: 'hit', retargeted: claim.retargeted, result, rearm }
} catch (error) {
await discardPreparedWorktree(args.repoPath, entry.preparedPath, options).catch(() => {})
@@ -278,7 +300,11 @@ export async function consumePreparedWorktreeCreate(
'[worktree-create] prepared checkout could not be finalized; using normal add',
error
)
return { status: 'miss', reason: 'finalize_failed' }
return {
status: 'miss',
reason: 'finalize_failed',
rearm: releaseClaimAfterCreate(reservation)
}
}
}
+5 -1
View File
@@ -216,6 +216,7 @@ describeBinaryCompatibility('real Git binary compatibility', () => {
})
it('supports prepared worktree creation and finalization', async () => {
const head = (await runGit(['rev-parse', 'HEAD'])).stdout.trim()
await runGit(['worktree', 'add', '--detach', '--no-checkout', 'compat-prepared', 'HEAD'])
await runGit(['-C', 'compat-prepared', 'reset', '--hard', 'HEAD'])
await runGit([
@@ -234,12 +235,15 @@ describeBinaryCompatibility('real Git binary compatibility', () => {
'--no-track',
'-b',
'compat-prepared-final',
'HEAD'
head
])
await expect(runGit(['-C', 'compat-final', 'branch', '--show-current'])).resolves.toMatchObject(
{ stdout: 'compat-prepared-final\n' }
)
await expect(runGit(['-C', 'compat-final', 'rev-parse', 'HEAD'])).resolves.toMatchObject({
stdout: `${head}\n`
})
await runGit(['worktree', 'unlock', 'compat-final'])
await runGit(['worktree', 'remove', '--force', 'compat-final'])
await runGit(['branch', '-D', 'compat-prepared-final'])