mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
perf(worktree): batch remote conflict probes, re-arm the prepared checkout (#17829)
* perf(worktree): batch remote conflict probes, re-arm the prepared checkout A repo with many remotes paid one `git show-ref --verify` subprocess per remote on every branch-conflict check during create. Ask one `git cat-file --batch-check` over stdin instead; it reports a missing ref as data rather than a failed exit, so a batch stays as decidable as the per-ref probe. Hosts that cannot feed stdin, and undecided batches, still fall back to the per-ref path. The prepared checkout was single-use, so the second create in a row paid the full cold `git worktree add`. Re-arm it in the background after one is consumed; the existing TTL and preparation limit still bound it. The create timing recorder existed but its phases were never emitted and did not cover preflight, leaving a multi-second gap in the trace with no attribution. Add `resolve_name`/`prepare_push_target` phases and record the breakdown, plus the unattributed remainder, on the create span. * fix(worktree): format the conflicting review number eagerly for the create error * perf(worktree): re-arm a prepared checkout only for a burst of creates Re-arming after every consumed preparation spends a full checkout and ~200MB of disk on a user who created one worktree and stopped, then pays an unexplained delete when the TTL expires five minutes later. Track when each preparation key was last consumed and only replace it when a second create lands inside the burst window, so the warm second create is still free and an isolated create costs nothing. * fix(worktree): address review findings on the create-path batching Three findings from PR review: The `batched.found` fallback in the remote-conflict probe was unreachable — a present ref is decisive, so `found` never survives with `unknown` set, and the guard above already returns that case. `rearmPreparation` checked for an existing preparation before recording the consume, so a prefetch that re-armed the key while create finalized swallowed the timestamp and made the next create look isolated when it was really mid-burst. Create runs some phases concurrently, so summing phase durations double-counted overlap and understated `unattributed_ms` — the one number that matters when a create is slow for no visible reason. Measure the union of the phase intervals instead. * refactor(worktree): move stale-preparation cleanup into its own module The preparation module crossed the 300-line budget. Crash recovery is a separate concern from the pool itself — it discards preparations another process left registered, single-flighted per repo and runtime so a burst of arming calls shares one worktree listing. * test(worktree): make the re-arm test able to fail The burst test armed a preparation manually after the second consume, so the third checkout appeared whether or not the re-arm produced it — the assertion passed with re-arming disabled. Drop that arming call so the third checkout can only come from the re-arm, and assert the consume results rather than discarding them.
This commit is contained in:
@@ -19,6 +19,8 @@ export type ExactRefProbeSetResult = {
|
||||
type ExactRefPresence = 'present' | 'absent' | 'unknown'
|
||||
|
||||
const EXACT_REF_PROBE_CONCURRENCY = 8
|
||||
// SHA-1 and SHA-256 repositories both report a full object id here.
|
||||
const OBJECT_ID_PATTERN = /^[0-9a-f]{40}(?:[0-9a-f]{24})?$/
|
||||
|
||||
export function isShowRefNoMatchError(error: unknown): boolean {
|
||||
const record = error && typeof error === 'object' ? (error as Record<string, unknown>) : undefined
|
||||
@@ -126,3 +128,50 @@ export async function probeAnyExactRef(
|
||||
await Promise.all(Array.from({ length: workerCount }, () => probeNext()))
|
||||
return { found, unknown }
|
||||
}
|
||||
|
||||
/** Runs Git with a stdin payload. Only hosts that can feed a child's stdin supply one. */
|
||||
export type ExactRefProbeStdinExec = (
|
||||
argv: string[],
|
||||
options: ExactRefProbeExecOptions & { stdin: string }
|
||||
) => Promise<{ stdout: string }>
|
||||
|
||||
/** `cat-file --batch-check` reports every ref from one child, and reports a missing ref as data
|
||||
* rather than a failed exit — so a batch stays as decidable as a per-ref `show-ref --verify`.
|
||||
* A repo with many remotes otherwise pays one subprocess per remote on every conflict check. */
|
||||
export async function probeAnyExactRefBatched(
|
||||
runGit: ExactRefProbeStdinExec,
|
||||
refs: readonly string[],
|
||||
options: ExactRefProbeExecOptions = {}
|
||||
): Promise<{ found: boolean; unknown: boolean }> {
|
||||
const uniqueRefs = [...new Set(refs)]
|
||||
const safeRefs = uniqueRefs.filter((ref) => isSafeGitRefName(ref))
|
||||
if (safeRefs.length === 0) {
|
||||
return { found: false, unknown: uniqueRefs.length > 0 }
|
||||
}
|
||||
let stdout: string
|
||||
try {
|
||||
;({ stdout } = await runGit(['cat-file', '--batch-check'], {
|
||||
...options,
|
||||
stdin: `${safeRefs.join('\n')}\n`
|
||||
}))
|
||||
} catch {
|
||||
return { found: false, unknown: true }
|
||||
}
|
||||
const lines = stdout.split('\n').filter((line) => line.trim().length > 0)
|
||||
// One line per input, in order; a short read means the batch never answered for the rest.
|
||||
if (lines.length !== safeRefs.length) {
|
||||
return { found: false, unknown: true }
|
||||
}
|
||||
let unknown = safeRefs.length !== uniqueRefs.length
|
||||
for (const line of lines) {
|
||||
const [head, type] = line.split(' ')
|
||||
if (OBJECT_ID_PATTERN.test(head) && type !== undefined && type !== 'missing') {
|
||||
return { found: true, unknown: false }
|
||||
}
|
||||
if (type !== 'missing') {
|
||||
// `ambiguous`, or a spelling this Git reports differently; neither proves absence.
|
||||
unknown = true
|
||||
}
|
||||
}
|
||||
return { found: false, unknown }
|
||||
}
|
||||
|
||||
@@ -0,0 +1,49 @@
|
||||
import { execFileSync } from 'node:child_process'
|
||||
import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { afterEach, describe, expect, it } from 'vitest'
|
||||
import { getBranchConflictKind } from './repo-branch-conflict'
|
||||
|
||||
describe('branch conflict real Git contract', () => {
|
||||
const tempPaths: string[] = []
|
||||
|
||||
afterEach(() => {
|
||||
for (const path of tempPaths.splice(0)) {
|
||||
rmSync(path, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
it('decides remote conflicts from one batched probe across many remotes', async () => {
|
||||
const repoPath = mkdtempSync(join(tmpdir(), 'orca-branch-conflict-'))
|
||||
tempPaths.push(repoPath)
|
||||
const git = (...args: string[]): string =>
|
||||
execFileSync('git', args, { cwd: repoPath, encoding: 'utf8' })
|
||||
|
||||
git('init', '--quiet')
|
||||
git('config', 'user.name', 'Orca Test')
|
||||
git('config', 'user.email', 'orca@example.test')
|
||||
git('config', 'commit.gpgSign', 'false')
|
||||
git('config', 'core.hooksPath', '.git/no-hooks')
|
||||
writeFileSync(join(repoPath, 'fixture.txt'), 'base\n')
|
||||
git('add', 'fixture.txt')
|
||||
git('commit', '--quiet', '-m', 'base')
|
||||
const head = git('rev-parse', 'HEAD').trim()
|
||||
|
||||
// Many remotes is the shape that used to cost one subprocess each.
|
||||
for (let index = 0; index < 12; index += 1) {
|
||||
git('remote', 'add', `remote${index}`, 'https://example.test/repo.git')
|
||||
}
|
||||
git('update-ref', 'refs/remotes/remote7/taken', head)
|
||||
|
||||
await expect(getBranchConflictKind(repoPath, 'taken')).resolves.toBe('remote')
|
||||
await expect(getBranchConflictKind(repoPath, 'free')).resolves.toBeNull()
|
||||
// The allowed base ref is the one remote spelling that is not a conflict.
|
||||
await expect(
|
||||
getBranchConflictKind(repoPath, 'taken', 'refs/remotes/remote7/taken')
|
||||
).resolves.toBeNull()
|
||||
|
||||
git('branch', 'local-only', head)
|
||||
await expect(getBranchConflictKind(repoPath, 'local-only')).resolves.toBe('local')
|
||||
})
|
||||
})
|
||||
@@ -134,3 +134,123 @@ describe('getBranchConflictKindViaExec', () => {
|
||||
expect(exec).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
describe('getBranchConflictKindViaExec batched remote probe', () => {
|
||||
function remoteNames(count: number): string {
|
||||
return `${Array.from({ length: count }, (_, index) => `remote${index}`).join('\n')}\n`
|
||||
}
|
||||
|
||||
function baseExec(calls: string[][]): (argv: string[]) => Promise<{ stdout: string }> {
|
||||
return async (argv) => {
|
||||
calls.push(argv)
|
||||
if (argv[0] === 'rev-parse') {
|
||||
throw new Error('local branch is absent')
|
||||
}
|
||||
if (argv[0] === 'remote') {
|
||||
return { stdout: remoteNames(3) }
|
||||
}
|
||||
throw new Error(`unexpected git command: ${argv.join(' ')}`)
|
||||
}
|
||||
}
|
||||
|
||||
it('asks one batched child instead of one probe per remote', async () => {
|
||||
const calls: string[][] = []
|
||||
const stdinPayloads: (string | undefined)[] = []
|
||||
const exec = baseExec(calls)
|
||||
const batched = async (
|
||||
argv: string[],
|
||||
options: { stdin: string }
|
||||
): Promise<{ stdout: string }> => {
|
||||
calls.push(argv)
|
||||
stdinPayloads.push(options.stdin)
|
||||
return {
|
||||
stdout: [
|
||||
'refs/remotes/remote0/feature missing',
|
||||
'refs/remotes/remote1/feature missing',
|
||||
'refs/remotes/remote2/feature missing'
|
||||
].join('\n')
|
||||
}
|
||||
}
|
||||
|
||||
await expect(
|
||||
getBranchConflictKindViaExec(exec, 'feature', undefined, {}, batched)
|
||||
).resolves.toBeNull()
|
||||
expect(calls).toEqual([
|
||||
['rev-parse', '--verify', 'refs/heads/feature'],
|
||||
['remote'],
|
||||
['cat-file', '--batch-check']
|
||||
])
|
||||
expect(stdinPayloads).toEqual([
|
||||
'refs/remotes/remote0/feature\nrefs/remotes/remote1/feature\nrefs/remotes/remote2/feature\n'
|
||||
])
|
||||
})
|
||||
|
||||
it('reports a remote conflict from the batched answer', async () => {
|
||||
const calls: string[][] = []
|
||||
const exec = baseExec(calls)
|
||||
const batched = async (): Promise<{ stdout: string }> => ({
|
||||
stdout: [
|
||||
'refs/remotes/remote0/feature missing',
|
||||
`${'a'.repeat(40)} commit 214`,
|
||||
'refs/remotes/remote2/feature missing'
|
||||
].join('\n')
|
||||
})
|
||||
|
||||
await expect(
|
||||
getBranchConflictKindViaExec(exec, 'feature', undefined, {}, batched)
|
||||
).resolves.toBe('remote')
|
||||
})
|
||||
|
||||
it('falls back to per-ref probes when the batch cannot answer', async () => {
|
||||
const calls: string[][] = []
|
||||
const exec = async (argv: string[]): Promise<{ stdout: string }> => {
|
||||
calls.push(argv)
|
||||
if (argv[0] === 'rev-parse') {
|
||||
throw new Error('local branch is absent')
|
||||
}
|
||||
if (argv[0] === 'remote') {
|
||||
return { stdout: remoteNames(3) }
|
||||
}
|
||||
if (argv[0] === 'show-ref') {
|
||||
if (argv[4] === 'refs/remotes/remote1/feature') {
|
||||
return { stdout: 'abc refs/remotes/remote1/feature\n' }
|
||||
}
|
||||
throw Object.assign(new Error('missing'), { code: 1, stderr: '' })
|
||||
}
|
||||
throw new Error(`unexpected git command: ${argv.join(' ')}`)
|
||||
}
|
||||
const batched = async (): Promise<{ stdout: string }> => {
|
||||
throw new Error('cat-file is unavailable')
|
||||
}
|
||||
|
||||
await expect(
|
||||
getBranchConflictKindViaExec(exec, 'feature', undefined, {}, batched)
|
||||
).resolves.toBe('remote')
|
||||
expect(calls.filter((argv) => argv[0] === 'show-ref')).toHaveLength(3)
|
||||
})
|
||||
|
||||
it('treats a short batch read as undecided rather than as absence', async () => {
|
||||
const calls: string[][] = []
|
||||
const exec = async (argv: string[]): Promise<{ stdout: string }> => {
|
||||
calls.push(argv)
|
||||
if (argv[0] === 'rev-parse') {
|
||||
throw new Error('local branch is absent')
|
||||
}
|
||||
if (argv[0] === 'remote') {
|
||||
return { stdout: remoteNames(3) }
|
||||
}
|
||||
if (argv[0] === 'show-ref') {
|
||||
throw Object.assign(new Error('missing'), { code: 1, stderr: '' })
|
||||
}
|
||||
throw new Error(`unexpected git command: ${argv.join(' ')}`)
|
||||
}
|
||||
const batched = async (): Promise<{ stdout: string }> => ({
|
||||
stdout: 'refs/remotes/remote0/feature missing'
|
||||
})
|
||||
|
||||
await expect(
|
||||
getBranchConflictKindViaExec(exec, 'feature', undefined, {}, batched)
|
||||
).resolves.toBeNull()
|
||||
expect(calls.filter((argv) => argv[0] === 'show-ref')).toHaveLength(3)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -4,8 +4,10 @@ import { gitExecFileAsync } from './runner'
|
||||
import { isSafeGitRefName } from '../../shared/git-status-upstream-ref'
|
||||
import {
|
||||
probeAnyExactRef,
|
||||
probeAnyExactRefBatched,
|
||||
type ExactRefProbeExec,
|
||||
type ExactRefProbeExecOptions
|
||||
type ExactRefProbeExecOptions,
|
||||
type ExactRefProbeStdinExec
|
||||
} from './exact-ref-probe'
|
||||
|
||||
export type BranchConflictKind = 'local' | 'remote'
|
||||
@@ -79,12 +81,31 @@ function buildRemoteBranchConflictRefs(
|
||||
return [...refs]
|
||||
}
|
||||
|
||||
/** One batched child answers for every remote; the per-ref probes only run when the host cannot
|
||||
* feed stdin, or when the batch came back undecided. */
|
||||
async function probeAnyRemoteConflictRef(
|
||||
exec: ExactRefProbeExec,
|
||||
batchedExec: ExactRefProbeStdinExec | undefined,
|
||||
candidateRefs: readonly string[],
|
||||
probeOptions: ExactRefProbeExecOptions
|
||||
): Promise<{ found: boolean }> {
|
||||
if (batchedExec) {
|
||||
// A present ref is always decisive, so `found` never survives with `unknown` set.
|
||||
const batched = await probeAnyExactRefBatched(batchedExec, candidateRefs, probeOptions)
|
||||
if (!batched.unknown) {
|
||||
return { found: batched.found }
|
||||
}
|
||||
}
|
||||
return probeAnyExactRef(exec, candidateRefs, probeOptions)
|
||||
}
|
||||
|
||||
/** Run branch-conflict policy through the host that owns Git execution. */
|
||||
export async function getBranchConflictKindViaExec(
|
||||
exec: ExactRefProbeExec,
|
||||
branchName: string,
|
||||
allowedBaseRef?: string,
|
||||
options: ExactRefProbeExecOptions = {}
|
||||
options: ExactRefProbeExecOptions = {},
|
||||
batchedExec?: ExactRefProbeStdinExec
|
||||
): Promise<BranchConflictKind | null> {
|
||||
if (!canQueryRemoteBranchName(branchName)) {
|
||||
return null
|
||||
@@ -104,7 +125,12 @@ export async function getBranchConflictKindViaExec(
|
||||
return null
|
||||
}
|
||||
|
||||
const { found: hasRemoteConflict } = await probeAnyExactRef(exec, candidateRefs, probeOptions)
|
||||
const { found: hasRemoteConflict } = await probeAnyRemoteConflictRef(
|
||||
exec,
|
||||
batchedExec,
|
||||
candidateRefs,
|
||||
probeOptions
|
||||
)
|
||||
|
||||
return hasRemoteConflict ? 'remote' : null
|
||||
} catch {
|
||||
@@ -119,15 +145,22 @@ export function getBranchConflictKind(
|
||||
options: LocalGitExecOptions = {}
|
||||
): Promise<BranchConflictKind | null> {
|
||||
const execOptions = gitExecOptions(path, options)
|
||||
const runLocalGit = (
|
||||
argv: string[],
|
||||
commandOptions?: ExactRefProbeExecOptions & { stdin?: string }
|
||||
): Promise<{ stdout: string }> =>
|
||||
gitExecFileAsync(argv, {
|
||||
...execOptions,
|
||||
...(commandOptions?.maxBuffer === undefined ? {} : { maxBuffer: commandOptions.maxBuffer }),
|
||||
...(commandOptions?.timeoutMs === undefined ? {} : { timeout: commandOptions.timeoutMs }),
|
||||
...(commandOptions?.stdin === undefined ? {} : { stdin: commandOptions.stdin })
|
||||
})
|
||||
return getBranchConflictKindViaExec(
|
||||
(argv, commandOptions) =>
|
||||
gitExecFileAsync(argv, {
|
||||
...execOptions,
|
||||
...(commandOptions?.maxBuffer === undefined ? {} : { maxBuffer: commandOptions.maxBuffer }),
|
||||
...(commandOptions?.timeoutMs === undefined ? {} : { timeout: commandOptions.timeoutMs })
|
||||
}),
|
||||
runLocalGit,
|
||||
branchName,
|
||||
allowedBaseRef
|
||||
allowedBaseRef,
|
||||
{},
|
||||
(argv, commandOptions) => runLocalGit(argv, commandOptions)
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
+131
-119
@@ -2189,130 +2189,139 @@ export async function createLocalWorktree(
|
||||
let lastExistingReviewNumber: number | null = null
|
||||
const shouldRetireGeneratedName =
|
||||
args.nameWasGenerated === true && isGeneratedWorktreeCreateName(sanitizedName)
|
||||
const retiredNameRegistry = shouldRetireGeneratedName
|
||||
? await getRetiredNameRegistryForRepo(store, repo, store.getRepos(), settings)
|
||||
: null
|
||||
const isRetiredName = retiredNameRegistry ? createRetiredNameLookup(retiredNameRegistry) : null
|
||||
// Why: a create-from-review branch override may already exist locally; suffix both branch and path instead of blocking the user.
|
||||
for (let suffix = 1, attempts = 0; attempts < WORKTREE_CREATE_MAX_SUFFIX_ATTEMPTS; suffix += 1) {
|
||||
effectiveSanitizedName = shouldRetireGeneratedName
|
||||
? getGeneratedWorktreeCreateCandidate(
|
||||
sanitizedName,
|
||||
suffix,
|
||||
retiredNameRegistry?.exhaustedTiers
|
||||
)
|
||||
: getWorktreeCreateCandidate(sanitizedName, suffix)
|
||||
effectiveRequestedName = shouldRetireGeneratedName
|
||||
? effectiveSanitizedName
|
||||
: requestedName.trim()
|
||||
? getWorktreeCreateCandidate(requestedName, suffix)
|
||||
: effectiveSanitizedName
|
||||
if (isRetiredName?.(effectiveSanitizedName)) {
|
||||
continue
|
||||
}
|
||||
attempts += 1
|
||||
lastExistingReviewNumber = null
|
||||
await timing.time('resolve_name', async () => {
|
||||
const retiredNameRegistry = shouldRetireGeneratedName
|
||||
? await getRetiredNameRegistryForRepo(store, repo, store.getRepos(), settings)
|
||||
: null
|
||||
const isRetiredName = retiredNameRegistry ? createRetiredNameLookup(retiredNameRegistry) : null
|
||||
// Why: a create-from-review branch override may already exist locally; suffix both branch and path instead of blocking the user.
|
||||
for (
|
||||
let suffix = 1, attempts = 0;
|
||||
attempts < WORKTREE_CREATE_MAX_SUFFIX_ATTEMPTS;
|
||||
suffix += 1
|
||||
) {
|
||||
effectiveSanitizedName = shouldRetireGeneratedName
|
||||
? getGeneratedWorktreeCreateCandidate(
|
||||
sanitizedName,
|
||||
suffix,
|
||||
retiredNameRegistry?.exhaustedTiers
|
||||
)
|
||||
: getWorktreeCreateCandidate(sanitizedName, suffix)
|
||||
effectiveRequestedName = shouldRetireGeneratedName
|
||||
? effectiveSanitizedName
|
||||
: requestedName.trim()
|
||||
? getWorktreeCreateCandidate(requestedName, suffix)
|
||||
: effectiveSanitizedName
|
||||
if (isRetiredName?.(effectiveSanitizedName)) {
|
||||
continue
|
||||
}
|
||||
attempts += 1
|
||||
lastExistingReviewNumber = null
|
||||
|
||||
branchName = await resolveCreateBranchName(
|
||||
repo.path,
|
||||
selectedExistingLocalBranchName
|
||||
? selectedExistingLocalBranchName
|
||||
: getBranchNameOverrideCandidate(args.branchNameOverride, suffix),
|
||||
effectiveSanitizedName,
|
||||
settings,
|
||||
username,
|
||||
localWorktreeGitOptions
|
||||
)
|
||||
checkoutExistingBranch = await canCheckoutExistingLocalBranch(
|
||||
repo.path,
|
||||
branchName,
|
||||
baseBranch,
|
||||
localWorktreeGitOptions
|
||||
)
|
||||
if (checkoutExistingBranch && !selectedExistingLocalBranchName) {
|
||||
// Why: suffix retries may need a new path, but an existing-branch checkout must keep the user-selected branch, not a sibling.
|
||||
selectedExistingLocalBranchName = branchName
|
||||
}
|
||||
lastBranchConflictKind = checkoutExistingBranch
|
||||
? null
|
||||
: await getBranchConflictKind(repo.path, branchName, baseBranch, localWorktreeGitOptions)
|
||||
const allowedPushTargetRemoteConflict =
|
||||
lastBranchConflictKind &&
|
||||
isAllowedPushTargetRemoteConflict(lastBranchConflictKind, branchName, args)
|
||||
if (lastBranchConflictKind) {
|
||||
if (allowedPushTargetRemoteConflict) {
|
||||
lastExistingPR = null
|
||||
let lookupFailed = false
|
||||
const selectedReview = getSelectedReviewBranch(args)
|
||||
if (selectedReview?.provider === 'github') {
|
||||
try {
|
||||
lastExistingPR = await getLocalGitHubPrForBranch(
|
||||
repo.path,
|
||||
branchName,
|
||||
localWorktreeGitOptions
|
||||
)
|
||||
} catch {
|
||||
lookupFailed = true
|
||||
}
|
||||
if (!lookupFailed && isMatchingSelectedGitHubPr(lastExistingPR, args, branchName)) {
|
||||
lastBranchConflictKind = null
|
||||
} else if (lastExistingPR) {
|
||||
lastExistingReviewNumber = lastExistingPR.number
|
||||
}
|
||||
} else if (selectedReview) {
|
||||
let hostedReview: Awaited<ReturnType<typeof getSelectedHostedReviewForBranch>> = null
|
||||
try {
|
||||
hostedReview = await getSelectedHostedReviewForBranch(repo, branchName, args)
|
||||
} catch {
|
||||
lookupFailed = true
|
||||
}
|
||||
if (!lookupFailed && hostedReview?.matchesSelected) {
|
||||
lastBranchConflictKind = null
|
||||
} else if (hostedReview) {
|
||||
lastExistingReviewNumber = hostedReview.number
|
||||
branchName = await resolveCreateBranchName(
|
||||
repo.path,
|
||||
selectedExistingLocalBranchName
|
||||
? selectedExistingLocalBranchName
|
||||
: getBranchNameOverrideCandidate(args.branchNameOverride, suffix),
|
||||
effectiveSanitizedName,
|
||||
settings,
|
||||
username,
|
||||
localWorktreeGitOptions
|
||||
)
|
||||
checkoutExistingBranch = await canCheckoutExistingLocalBranch(
|
||||
repo.path,
|
||||
branchName,
|
||||
baseBranch,
|
||||
localWorktreeGitOptions
|
||||
)
|
||||
if (checkoutExistingBranch && !selectedExistingLocalBranchName) {
|
||||
// Why: suffix retries may need a new path, but an existing-branch checkout must keep the user-selected branch, not a sibling.
|
||||
selectedExistingLocalBranchName = branchName
|
||||
}
|
||||
lastBranchConflictKind = checkoutExistingBranch
|
||||
? null
|
||||
: await getBranchConflictKind(repo.path, branchName, baseBranch, localWorktreeGitOptions)
|
||||
const allowedPushTargetRemoteConflict =
|
||||
lastBranchConflictKind &&
|
||||
isAllowedPushTargetRemoteConflict(lastBranchConflictKind, branchName, args)
|
||||
if (lastBranchConflictKind) {
|
||||
if (allowedPushTargetRemoteConflict) {
|
||||
lastExistingPR = null
|
||||
let lookupFailed = false
|
||||
const selectedReview = getSelectedReviewBranch(args)
|
||||
if (selectedReview?.provider === 'github') {
|
||||
try {
|
||||
lastExistingPR = await getLocalGitHubPrForBranch(
|
||||
repo.path,
|
||||
branchName,
|
||||
localWorktreeGitOptions
|
||||
)
|
||||
} catch {
|
||||
lookupFailed = true
|
||||
}
|
||||
if (!lookupFailed && isMatchingSelectedGitHubPr(lastExistingPR, args, branchName)) {
|
||||
lastBranchConflictKind = null
|
||||
} else if (lastExistingPR) {
|
||||
lastExistingReviewNumber = lastExistingPR.number
|
||||
}
|
||||
} else if (selectedReview) {
|
||||
let hostedReview: Awaited<ReturnType<typeof getSelectedHostedReviewForBranch>> = null
|
||||
try {
|
||||
hostedReview = await getSelectedHostedReviewForBranch(repo, branchName, args)
|
||||
} catch {
|
||||
lookupFailed = true
|
||||
}
|
||||
if (!lookupFailed && hostedReview?.matchesSelected) {
|
||||
lastBranchConflictKind = null
|
||||
} else if (hostedReview) {
|
||||
lastExistingReviewNumber = hostedReview.number
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
if (lastBranchConflictKind) {
|
||||
continue
|
||||
}
|
||||
|
||||
// Why: gh pr list is a ~1–3s network call; only probe PR conflicts after a branch collision (suffix > 1) so the common no-collision path skips it.
|
||||
if (suffix > 1 && !checkoutExistingBranch) {
|
||||
lastExistingPR = null
|
||||
try {
|
||||
lastExistingPR = await getLocalGitHubPrForBranch(
|
||||
repo.path,
|
||||
branchName,
|
||||
localWorktreeGitOptions
|
||||
)
|
||||
} catch {
|
||||
// GitHub API may be unreachable, rate-limited, or token missing
|
||||
}
|
||||
if (lastExistingPR && !isMatchingSelectedGitHubPr(lastExistingPR, args, branchName)) {
|
||||
lastExistingReviewNumber = lastExistingPR.number
|
||||
if (lastBranchConflictKind) {
|
||||
continue
|
||||
}
|
||||
}
|
||||
|
||||
worktreePath = ensurePathWithinWorkspace(
|
||||
computeWorktreePath(effectiveSanitizedName, repo.path, worktreePathSettings),
|
||||
workspaceRoot
|
||||
)
|
||||
if (existsSync(worktreePath)) {
|
||||
continue
|
||||
}
|
||||
// Why: gh pr list is a ~1–3s network call; only probe PR conflicts after a branch collision (suffix > 1) so the common no-collision path skips it.
|
||||
if (suffix > 1 && !checkoutExistingBranch) {
|
||||
lastExistingPR = null
|
||||
try {
|
||||
lastExistingPR = await getLocalGitHubPrForBranch(
|
||||
repo.path,
|
||||
branchName,
|
||||
localWorktreeGitOptions
|
||||
)
|
||||
} catch {
|
||||
// GitHub API may be unreachable, rate-limited, or token missing
|
||||
}
|
||||
if (lastExistingPR && !isMatchingSelectedGitHubPr(lastExistingPR, args, branchName)) {
|
||||
lastExistingReviewNumber = lastExistingPR.number
|
||||
continue
|
||||
}
|
||||
}
|
||||
|
||||
resolved = true
|
||||
break
|
||||
}
|
||||
worktreePath = ensurePathWithinWorkspace(
|
||||
computeWorktreePath(effectiveSanitizedName, repo.path, worktreePathSettings),
|
||||
workspaceRoot
|
||||
)
|
||||
if (existsSync(worktreePath)) {
|
||||
continue
|
||||
}
|
||||
|
||||
resolved = true
|
||||
break
|
||||
}
|
||||
})
|
||||
|
||||
if (!resolved) {
|
||||
// Why: every suffix collided; reject with a specific reason so the user sees why create failed instead of a generic error or hung spinner.
|
||||
if (lastExistingReviewNumber !== null) {
|
||||
// Read once and format eagerly: the suffix loop assigns this from a callback, so the `let`'s
|
||||
// narrowing does not reach the message.
|
||||
const existingReviewNumber = lastExistingReviewNumber
|
||||
if (existingReviewNumber !== null) {
|
||||
throw new Error(
|
||||
`Branch "${branchName}" already has PR #${lastExistingReviewNumber}. Pick a different ${branchConflictSubject}.`
|
||||
`Branch "${branchName}" already has PR #${String(existingReviewNumber)}. Pick a different ${branchConflictSubject}.`
|
||||
)
|
||||
}
|
||||
if (lastBranchConflictKind) {
|
||||
@@ -2361,14 +2370,17 @@ export async function createLocalWorktree(
|
||||
emitCreateWorktreeProgress(mainWindow, 'creating', args.creationId)
|
||||
|
||||
let preparedPushTarget: GitPushTarget | undefined
|
||||
if (args.pushTarget) {
|
||||
const requestedPushTarget = args.pushTarget
|
||||
if (requestedPushTarget) {
|
||||
// Why: validate/fetch the contributor remote before create so a failure doesn't leave a half-created worktree with conflicts on retry.
|
||||
preparedPushTarget = await prepareWorktreePushTarget(
|
||||
repo.path,
|
||||
args.pushTarget,
|
||||
store,
|
||||
repo.id,
|
||||
localWorktreeGitOptions
|
||||
preparedPushTarget = await timing.time('prepare_push_target', () =>
|
||||
prepareWorktreePushTarget(
|
||||
repo.path,
|
||||
requestedPushTarget,
|
||||
store,
|
||||
repo.id,
|
||||
localWorktreeGitOptions
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
@@ -4,7 +4,10 @@ import type {
|
||||
CreateWorktreeResult,
|
||||
AdoptProvisionedRootArgs
|
||||
} from '../../../../shared/worktree/create-types'
|
||||
import { withWorktreeSpan } from '../../../observability/instrumentation'
|
||||
import {
|
||||
addWorktreeCreatePhaseAttributes,
|
||||
withWorktreeSpan
|
||||
} from '../../../observability/instrumentation'
|
||||
import { workspaceSourceSchema } from '../../../../shared/telemetry-events'
|
||||
import type { WorkspaceSource } from '../../../../shared/telemetry-events'
|
||||
import {
|
||||
@@ -36,7 +39,7 @@ export function registerWorktreeCreateHandlers(context: WorktreeIpcContext): voi
|
||||
async (_event, rawArgs: CreateWorktreeArgs): Promise<CreateWorktreeResult> => {
|
||||
const args = normalizeLinkedWorkItemFields(rawArgs)
|
||||
// Why span here: parent the child git spans for the trace tree; don't attach branch name/remote URL (user content) — repo ID is the safer correlator.
|
||||
return withWorktreeSpan({ stage: 'create' }, async () => {
|
||||
return withWorktreeSpan({ stage: 'create' }, async (span) => {
|
||||
const repo = store.getRepo(args.repoId)
|
||||
if (!repo) {
|
||||
throw new Error(`Repo not found: ${args.repoId}`)
|
||||
@@ -74,6 +77,9 @@ export function registerWorktreeCreateHandlers(context: WorktreeIpcContext): voi
|
||||
throw error
|
||||
}
|
||||
finishAutomationWorkspaceProvenanceRequest(args.automationProvenanceRequest)
|
||||
if (result.timing) {
|
||||
addWorktreeCreatePhaseAttributes(span, result.timing)
|
||||
}
|
||||
|
||||
// Why: reaching here means create succeeded (helpers throw); skip a separate workspace_initialized (telemetry-plan.md§Deferred); never send the branch name.
|
||||
track('workspace_created', {
|
||||
|
||||
@@ -3,6 +3,7 @@ import { _resetTracerForTests, setActiveSink, type TracerSink } from './tracer'
|
||||
import {
|
||||
_gitSpanSamplingBucketCountForTests,
|
||||
_resetGitSpanSamplingForTests,
|
||||
addWorktreeCreatePhaseAttributes,
|
||||
withGitSpan
|
||||
} from './instrumentation'
|
||||
|
||||
@@ -167,3 +168,50 @@ describe('withGitSpan sampling', () => {
|
||||
expect(_gitSpanSamplingBucketCountForTests()).toBe(1)
|
||||
})
|
||||
})
|
||||
|
||||
describe('addWorktreeCreatePhaseAttributes', () => {
|
||||
function capture(): {
|
||||
attributes: Record<string, unknown>
|
||||
span: Parameters<typeof addWorktreeCreatePhaseAttributes>[0]
|
||||
} {
|
||||
const attributes: Record<string, unknown> = {}
|
||||
const span = {
|
||||
setAttribute: (key: string, value: unknown) => {
|
||||
attributes[key] = value
|
||||
}
|
||||
} as unknown as Parameters<typeof addWorktreeCreatePhaseAttributes>[0]
|
||||
return { attributes, span }
|
||||
}
|
||||
|
||||
it('counts concurrent phases once when measuring unattributed time', () => {
|
||||
const { attributes, span } = capture()
|
||||
// Create resolves shared directories and .worktreeinclude concurrently; summing their
|
||||
// durations would claim 400ms of coverage for a 200ms window.
|
||||
addWorktreeCreatePhaseAttributes(span, {
|
||||
totalDurationMs: 1000,
|
||||
phases: [
|
||||
{ phase: 'resolve_shared_directories', startedAtMs: 100, durationMs: 200 },
|
||||
{ phase: 'resolve_worktreeinclude', startedAtMs: 150, durationMs: 150 }
|
||||
]
|
||||
})
|
||||
|
||||
expect(attributes['worktree.create.phase.resolve_shared_directories_ms']).toBe(200)
|
||||
expect(attributes['worktree.create.phase.resolve_worktreeinclude_ms']).toBe(150)
|
||||
// Covered wall clock is 100..300, so 800ms is genuinely unaccounted for.
|
||||
expect(attributes['worktree.create.unattributed_ms']).toBe(800)
|
||||
})
|
||||
|
||||
it('sums disjoint phases and never reports negative unattributed time', () => {
|
||||
const { attributes, span } = capture()
|
||||
addWorktreeCreatePhaseAttributes(span, {
|
||||
totalDurationMs: 500,
|
||||
phases: [
|
||||
{ phase: 'resolve_name', startedAtMs: 0, durationMs: 100 },
|
||||
{ phase: 'git_worktree_add', startedAtMs: 300, durationMs: 200 }
|
||||
]
|
||||
})
|
||||
|
||||
expect(attributes['worktree.create.total_ms']).toBe(500)
|
||||
expect(attributes['worktree.create.unattributed_ms']).toBe(200)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -202,10 +202,11 @@ export type WorktreeSpanArgs = {
|
||||
readonly path?: string
|
||||
}
|
||||
|
||||
/** Wrap a worktree-setup phase in a `worktree.<stage>` span. */
|
||||
/** Wrap a worktree-setup phase in a `worktree.<stage>` span. The callback receives the span so a
|
||||
* create can attach its own phase breakdown; the git children alone leave the waits invisible. */
|
||||
export async function withWorktreeSpan<T>(
|
||||
meta: WorktreeSpanArgs,
|
||||
fn: () => Promise<T>
|
||||
fn: (span: ActiveSpan) => Promise<T>
|
||||
): Promise<T> {
|
||||
return withSpan(
|
||||
`worktree.${meta.stage}`,
|
||||
@@ -214,12 +215,64 @@ export async function withWorktreeSpan<T>(
|
||||
if (meta.path) {
|
||||
span.setAttribute('worktree.path', meta.path)
|
||||
}
|
||||
return await fn()
|
||||
return await fn(span)
|
||||
},
|
||||
{ attributes: { kind: 'worktree' } }
|
||||
)
|
||||
}
|
||||
|
||||
type WorktreeCreatePhaseTiming = {
|
||||
readonly phase: string
|
||||
readonly startedAtMs: number
|
||||
readonly durationMs: number
|
||||
}
|
||||
|
||||
/** Wall-clock span covered by at least one phase. Create runs some phases concurrently, so summing
|
||||
* durations double-counts and would report overlap as coverage the phases never had. */
|
||||
function measuredWallClockMs(phases: readonly WorktreePhaseInterval[]): number {
|
||||
const intervals = [...phases]
|
||||
.map((phase) => [phase.startedAtMs, phase.startedAtMs + phase.durationMs] as const)
|
||||
.sort((left, right) => left[0] - right[0])
|
||||
let covered = 0
|
||||
let openedAt: number | null = null
|
||||
let closesAt = 0
|
||||
for (const [start, end] of intervals) {
|
||||
if (openedAt === null) {
|
||||
openedAt = start
|
||||
closesAt = end
|
||||
continue
|
||||
}
|
||||
if (start <= closesAt) {
|
||||
closesAt = Math.max(closesAt, end)
|
||||
continue
|
||||
}
|
||||
covered += closesAt - openedAt
|
||||
openedAt = start
|
||||
closesAt = end
|
||||
}
|
||||
return openedAt === null ? 0 : covered + (closesAt - openedAt)
|
||||
}
|
||||
|
||||
type WorktreePhaseInterval = Pick<WorktreeCreatePhaseTiming, 'startedAtMs' | 'durationMs'>
|
||||
|
||||
/** Records a create's phase breakdown on its span. Phase names are already a closed vocabulary in
|
||||
* the recorder, so they are safe to key on; nothing here carries a branch name or a path. */
|
||||
export function addWorktreeCreatePhaseAttributes(
|
||||
span: ActiveSpan,
|
||||
timing: { totalDurationMs: number; phases: readonly WorktreeCreatePhaseTiming[] }
|
||||
): void {
|
||||
span.setAttribute('worktree.create.total_ms', Math.round(timing.totalDurationMs))
|
||||
for (const phase of timing.phases) {
|
||||
span.setAttribute(`worktree.create.phase.${phase.phase}_ms`, Math.round(phase.durationMs))
|
||||
}
|
||||
// What the phases do not cover is the number that matters when create feels slow for no visible
|
||||
// reason, so name it rather than leaving it to subtraction.
|
||||
span.setAttribute(
|
||||
'worktree.create.unattributed_ms',
|
||||
Math.max(0, Math.round(timing.totalDurationMs - measuredWallClockMs(timing.phases)))
|
||||
)
|
||||
}
|
||||
|
||||
/** Closed set so a typo can't silently mint an orphan span name. */
|
||||
export type WorktreeRemoveStage =
|
||||
| 'archive_hook'
|
||||
|
||||
@@ -0,0 +1,21 @@
|
||||
import { setBoundedMapEntry } from './runtime/runtime-async-boundaries'
|
||||
|
||||
/** Two creates this close together mean more are likely; an isolated create earns no replacement. */
|
||||
export const WORKTREE_CREATE_BURST_MS = 5 * 60_000
|
||||
const WORKTREE_CREATE_PREPARATION_CONSUME_MAX = 64
|
||||
|
||||
/** When each preparation key was last consumed, so a burst can be told from an isolated create. */
|
||||
const lastConsumedAt = new Map<string, number>()
|
||||
|
||||
/** Records this consume and reports whether it continues a burst. A replacement checkout costs a
|
||||
* full tree and holds disk until its TTL, so only a user who is already creating repeatedly earns
|
||||
* one; the first create of a session pays nothing for a spare nobody claims. */
|
||||
export function recordPreparationConsume(key: string, now = Date.now()): boolean {
|
||||
const previous = lastConsumedAt.get(key)
|
||||
setBoundedMapEntry(lastConsumedAt, key, now, WORKTREE_CREATE_PREPARATION_CONSUME_MAX)
|
||||
return previous !== undefined && now - previous <= WORKTREE_CREATE_BURST_MS
|
||||
}
|
||||
|
||||
export function resetPreparationConsumeHistoryForTests(): void {
|
||||
lastConsumedAt.clear()
|
||||
}
|
||||
@@ -0,0 +1,79 @@
|
||||
import {
|
||||
isWorktreeCreatePreparation,
|
||||
parseWorktreePreparationOwnerPid,
|
||||
parseWorktreePreparationPathOwnerPid
|
||||
} from '../shared/worktree/create-preparation'
|
||||
import type { AddWorktreeOptions } from './git/worktree'
|
||||
import { listWorktreeGraph } from './git/worktree'
|
||||
import { discardPreparedWorktree, unlockPreparedWorktree } from './git/worktree-create-preparation'
|
||||
import { retryPendingPreparationDiscards } from './worktree-preparation-discard-retry'
|
||||
|
||||
const STALE_PREPARATION_CLEANUP_CONCURRENCY = 4
|
||||
|
||||
const staleCleanupInFlight = new Map<string, Promise<void>>()
|
||||
|
||||
function isProcessAlive(pid: number): boolean {
|
||||
try {
|
||||
process.kill(pid, 0)
|
||||
return true
|
||||
} catch (error) {
|
||||
return (error as NodeJS.ErrnoException).code !== 'ESRCH'
|
||||
}
|
||||
}
|
||||
|
||||
/** Reclaims preparations a crashed process left registered. Single-flighted per host key so a burst
|
||||
* of arming calls shares one worktree listing. */
|
||||
export async function cleanupStalePreparations(
|
||||
cleanupKey: string,
|
||||
repoPath: string,
|
||||
options: AddWorktreeOptions
|
||||
): Promise<void> {
|
||||
const existing = staleCleanupInFlight.get(cleanupKey)
|
||||
if (existing) {
|
||||
await existing.catch(() => {})
|
||||
return
|
||||
}
|
||||
const cleanup = (async () => {
|
||||
// Not awaited: the create path awaits this cleanup, and one stranded discard costs an unlock plus
|
||||
// a `worktree remove --force` bounded at 30s each. Reclaiming leaked scratch must not delay create.
|
||||
void retryPendingPreparationDiscards(cleanupKey)
|
||||
const worktrees = await listWorktreeGraph(repoPath, {
|
||||
...options,
|
||||
includeCreatePreparations: true
|
||||
})
|
||||
const staleWorktrees = worktrees.filter(isWorktreeCreatePreparation)
|
||||
let nextIndex = 0
|
||||
async function discardNextStalePreparation(): Promise<void> {
|
||||
while (nextIndex < staleWorktrees.length) {
|
||||
const worktree = staleWorktrees[nextIndex]
|
||||
nextIndex += 1
|
||||
const lockOwnerPid = parseWorktreePreparationOwnerPid(worktree.lockReason)
|
||||
const pathOwnerPid = parseWorktreePreparationPathOwnerPid(worktree.path)
|
||||
if (!lockOwnerPid || isProcessAlive(lockOwnerPid)) {
|
||||
continue
|
||||
}
|
||||
// Preserve a branch-attached final path after a crash; only detached or
|
||||
// still-hidden preparations are safe to discard automatically.
|
||||
if (worktree.branch && pathOwnerPid === null) {
|
||||
await unlockPreparedWorktree(repoPath, worktree.path, options).catch(() => {})
|
||||
} else if (pathOwnerPid === lockOwnerPid) {
|
||||
await discardPreparedWorktree(repoPath, worktree.path, options).catch(() => {})
|
||||
}
|
||||
}
|
||||
}
|
||||
const workerCount = Math.min(STALE_PREPARATION_CLEANUP_CONCURRENCY, staleWorktrees.length)
|
||||
await Promise.all(Array.from({ length: workerCount }, () => discardNextStalePreparation()))
|
||||
})()
|
||||
staleCleanupInFlight.set(cleanupKey, cleanup)
|
||||
try {
|
||||
await cleanup.catch(() => {})
|
||||
} finally {
|
||||
if (staleCleanupInFlight.get(cleanupKey) === cleanup) {
|
||||
staleCleanupInFlight.delete(cleanupKey)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
export function resetStalePreparationCleanupForTests(): void {
|
||||
staleCleanupInFlight.clear()
|
||||
}
|
||||
@@ -466,4 +466,51 @@ describe('worktree create preparation registry', () => {
|
||||
expect(mocks.mkdir).toHaveBeenCalledWith('/workspace', { recursive: true })
|
||||
expect(mocks.discard).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
function consumeOnce(name: string): ReturnType<typeof consumePreparedWorktreeCreate> {
|
||||
return 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 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()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -7,17 +7,12 @@ import { isFolderRepo } from '../shared/repo-kind'
|
||||
import { isWindowsAbsolutePathLike } from '../shared/cross-platform-path'
|
||||
import {
|
||||
WORKTREE_CREATE_PREPARATION_DIRECTORY,
|
||||
createWorktreePreparationLockReason,
|
||||
isWorktreeCreatePreparation,
|
||||
parseWorktreePreparationOwnerPid,
|
||||
parseWorktreePreparationPathOwnerPid
|
||||
createWorktreePreparationLockReason
|
||||
} from '../shared/worktree/create-preparation'
|
||||
import type { AddWorktreeOptions, AddWorktreeResult } from './git/worktree'
|
||||
import { listWorktreeGraph } from './git/worktree'
|
||||
import {
|
||||
discardPreparedWorktree,
|
||||
finalizePreparedWorktree,
|
||||
unlockPreparedWorktree,
|
||||
prepareWorktreeCreateCheckout
|
||||
} from './git/worktree-create-preparation'
|
||||
import {
|
||||
@@ -25,17 +20,23 @@ import {
|
||||
getWorktreeMirrorDistro
|
||||
} from './project-runtime-git-options'
|
||||
import { computeWorkspaceRootAsync, getWorktreePathSettings } from './ipc/worktree-logic'
|
||||
import {
|
||||
recordPreparationConsume,
|
||||
resetPreparationConsumeHistoryForTests
|
||||
} from './worktree-create-preparation-burst'
|
||||
import {
|
||||
cleanupStalePreparations,
|
||||
resetStalePreparationCleanupForTests
|
||||
} from './worktree-create-preparation-stale-cleanup'
|
||||
import { toHostFilesystemPath } from './host-tree-removal'
|
||||
import {
|
||||
discardPreparationWithRetry,
|
||||
resetPendingPreparationDiscardsForTests,
|
||||
retryPendingPreparationDiscards,
|
||||
trackPreparationDiscard
|
||||
} from './worktree-preparation-discard-retry'
|
||||
|
||||
export const WORKTREE_CREATE_PREPARATION_TTL_MS = 5 * 60_000
|
||||
export const WORKTREE_CREATE_PREPARATION_LIMIT = 3
|
||||
const STALE_PREPARATION_CLEANUP_CONCURRENCY = 4
|
||||
|
||||
type PreparationEntry = {
|
||||
key: string
|
||||
@@ -59,7 +60,6 @@ type ConsumePreparedWorktreeArgs = {
|
||||
}
|
||||
|
||||
const preparations = new Map<string, PreparationEntry>()
|
||||
const staleCleanupInFlight = new Map<string, Promise<void>>()
|
||||
|
||||
function pathOps(path: string): Pick<typeof posix, 'dirname' | 'join' | 'normalize'> {
|
||||
return isWindowsAbsolutePathLike(path) ? win32 : posix
|
||||
@@ -79,15 +79,6 @@ function preparationKey(
|
||||
return `${pathKey(repoPath)}\0${pathKey(workspaceRoot)}\0${baseBranch}\0${options.wslDistro ?? ''}`
|
||||
}
|
||||
|
||||
function isProcessAlive(pid: number): boolean {
|
||||
try {
|
||||
process.kill(pid, 0)
|
||||
return true
|
||||
} catch (error) {
|
||||
return (error as NodeJS.ErrnoException).code !== 'ESRCH'
|
||||
}
|
||||
}
|
||||
|
||||
function preparationHostKey(repoPath: string, options: AddWorktreeOptions): string {
|
||||
return `${pathKey(repoPath)}\0${options.wslDistro ?? ''}`
|
||||
}
|
||||
@@ -131,57 +122,6 @@ function enforcePreparationLimit(): void {
|
||||
}
|
||||
}
|
||||
|
||||
async function cleanupStalePreparations(
|
||||
repoPath: string,
|
||||
options: AddWorktreeOptions
|
||||
): Promise<void> {
|
||||
const cleanupKey = preparationHostKey(repoPath, options)
|
||||
const existing = staleCleanupInFlight.get(cleanupKey)
|
||||
if (existing) {
|
||||
await existing.catch(() => {})
|
||||
return
|
||||
}
|
||||
const cleanup = (async () => {
|
||||
// Not awaited: the create path awaits this cleanup, and one stranded discard costs an unlock plus
|
||||
// a `worktree remove --force` bounded at 30s each. Reclaiming leaked scratch must not delay create.
|
||||
void retryPendingPreparationDiscards(cleanupKey)
|
||||
const worktrees = await listWorktreeGraph(repoPath, {
|
||||
...options,
|
||||
includeCreatePreparations: true
|
||||
})
|
||||
const staleWorktrees = worktrees.filter(isWorktreeCreatePreparation)
|
||||
let nextIndex = 0
|
||||
async function discardNextStalePreparation(): Promise<void> {
|
||||
while (nextIndex < staleWorktrees.length) {
|
||||
const worktree = staleWorktrees[nextIndex]
|
||||
nextIndex += 1
|
||||
const lockOwnerPid = parseWorktreePreparationOwnerPid(worktree.lockReason)
|
||||
const pathOwnerPid = parseWorktreePreparationPathOwnerPid(worktree.path)
|
||||
if (!lockOwnerPid || isProcessAlive(lockOwnerPid)) {
|
||||
continue
|
||||
}
|
||||
// Preserve a branch-attached final path after a crash; only detached or
|
||||
// still-hidden preparations are safe to discard automatically.
|
||||
if (worktree.branch && pathOwnerPid === null) {
|
||||
await unlockPreparedWorktree(repoPath, worktree.path, options).catch(() => {})
|
||||
} else if (pathOwnerPid === lockOwnerPid) {
|
||||
await discardPreparedWorktree(repoPath, worktree.path, options).catch(() => {})
|
||||
}
|
||||
}
|
||||
}
|
||||
const workerCount = Math.min(STALE_PREPARATION_CLEANUP_CONCURRENCY, staleWorktrees.length)
|
||||
await Promise.all(Array.from({ length: workerCount }, () => discardNextStalePreparation()))
|
||||
})()
|
||||
staleCleanupInFlight.set(cleanupKey, cleanup)
|
||||
try {
|
||||
await cleanup.catch(() => {})
|
||||
} finally {
|
||||
if (staleCleanupInFlight.get(cleanupKey) === cleanup) {
|
||||
staleCleanupInFlight.delete(cleanupKey)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
export async function prepareWorktreeCreateForRepo(
|
||||
store: Store,
|
||||
repo: Repo,
|
||||
@@ -205,6 +145,16 @@ export async function prepareWorktreeCreateForRepo(
|
||||
return existing.ready
|
||||
}
|
||||
|
||||
return startPreparation(key, repo.path, workspaceRoot, baseBranch, options)
|
||||
}
|
||||
|
||||
function startPreparation(
|
||||
key: string,
|
||||
repoPath: string,
|
||||
workspaceRoot: string,
|
||||
baseBranch: string,
|
||||
options: AddWorktreeOptions
|
||||
): Promise<void> {
|
||||
enforcePreparationLimit()
|
||||
const preparationId = `${process.pid}-${randomUUID()}`
|
||||
const lockReason = createWorktreePreparationLockReason(preparationId)
|
||||
@@ -218,21 +168,21 @@ export async function prepareWorktreeCreateForRepo(
|
||||
expiration.unref()
|
||||
Object.assign(entry, {
|
||||
key,
|
||||
repoPath: repo.path,
|
||||
repoPath,
|
||||
workspaceRoot,
|
||||
preparedPath,
|
||||
options,
|
||||
createdAt: Date.now(),
|
||||
expiration,
|
||||
ready: (async () => {
|
||||
await cleanupStalePreparations(repo.path, options)
|
||||
await cleanupStalePreparations(preparationHostKey(repoPath, options), repoPath, options)
|
||||
await mkdir(
|
||||
toHostFilesystemPath(
|
||||
pathOps(workspaceRoot).join(workspaceRoot, WORKTREE_CREATE_PREPARATION_DIRECTORY)
|
||||
),
|
||||
{ recursive: true }
|
||||
)
|
||||
await prepareWorktreeCreateCheckout(repo.path, preparedPath, baseBranch, lockReason, options)
|
||||
await prepareWorktreeCreateCheckout(repoPath, preparedPath, baseBranch, lockReason, options)
|
||||
})()
|
||||
} satisfies PreparationEntry)
|
||||
preparations.set(key, entry)
|
||||
@@ -266,6 +216,28 @@ async function claimPreparedWorktree(
|
||||
}
|
||||
}
|
||||
|
||||
/** 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 {
|
||||
// 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) {
|
||||
return
|
||||
}
|
||||
void startPreparation(
|
||||
entry.key,
|
||||
entry.repoPath,
|
||||
entry.workspaceRoot,
|
||||
baseBranch,
|
||||
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<AddWorktreeResult | null> {
|
||||
@@ -283,7 +255,7 @@ export async function consumePreparedWorktreeCreate(
|
||||
await mkdir(toHostFilesystemPath(pathOps(args.worktreePath).dirname(args.worktreePath)), {
|
||||
recursive: true
|
||||
})
|
||||
return await finalizePreparedWorktree(
|
||||
const result = await finalizePreparedWorktree(
|
||||
args.repoPath,
|
||||
entry.preparedPath,
|
||||
args.worktreePath,
|
||||
@@ -292,6 +264,10 @@ export async function consumePreparedWorktreeCreate(
|
||||
args.refreshLocalBaseRef,
|
||||
options
|
||||
)
|
||||
// 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
|
||||
} catch (error) {
|
||||
await discardPreparedWorktree(args.repoPath, entry.preparedPath, options).catch(() => {})
|
||||
console.warn(
|
||||
@@ -305,7 +281,8 @@ export async function consumePreparedWorktreeCreate(
|
||||
export async function _resetWorktreeCreatePreparationsForTests(): Promise<void> {
|
||||
const entries = [...preparations.values()]
|
||||
preparations.clear()
|
||||
staleCleanupInFlight.clear()
|
||||
resetPreparationConsumeHistoryForTests()
|
||||
resetStalePreparationCleanupForTests()
|
||||
await Promise.all(
|
||||
entries.map(async (entry) => {
|
||||
clearTimeout(entry.expiration)
|
||||
|
||||
Reference in New Issue
Block a user