diff --git a/src/utils/swarm/spawnUtils.test.ts b/src/utils/swarm/spawnUtils.test.ts new file mode 100644 index 00000000..08bdd2cb --- /dev/null +++ b/src/utils/swarm/spawnUtils.test.ts @@ -0,0 +1,33 @@ +import { afterEach, beforeEach, expect, test } from 'bun:test' + +import { buildInheritedEnvVars } from './spawnUtils.js' + +const ORIGINAL_ENV = { ...process.env } + +beforeEach(() => { + for (const key of Object.keys(process.env)) { + delete process.env[key] + } +}) + +afterEach(() => { + for (const key of Object.keys(process.env)) { + delete process.env[key] + } + Object.assign(process.env, ORIGINAL_ENV) +}) + +test('buildInheritedEnvVars marks spawned teammates as host-managed for provider routing', () => { + const envVars = buildInheritedEnvVars() + + expect(envVars).toContain('CLAUDE_CODE_PROVIDER_MANAGED_BY_HOST=1') +}) + +test('buildInheritedEnvVars forwards PATH for source-built teammate tool lookups', () => { + process.env.PATH = '/custom/bin:/usr/bin' + + const envVars = buildInheritedEnvVars() + + expect(envVars).toContain('PATH=') + expect(envVars).toContain('/custom/bin\\:/usr/bin') +}) diff --git a/src/utils/swarm/spawnUtils.ts b/src/utils/swarm/spawnUtils.ts index e156383d..7b7e2299 100644 --- a/src/utils/swarm/spawnUtils.ts +++ b/src/utils/swarm/spawnUtils.ts @@ -141,6 +141,9 @@ const TEAMMATE_ENV_VARS = [ 'NODE_EXTRA_CA_CERTS', 'REQUESTS_CA_BUNDLE', 'CURL_CA_BUNDLE', + // Source builds may rely on user shell PATH for rg/node/bun and other tools. + // Forward it so teammates resolve the same toolchain as the parent session. + 'PATH', ] as const /** @@ -149,7 +152,13 @@ const TEAMMATE_ENV_VARS = [ * plus any provider/config env vars that are set in the current process. */ export function buildInheritedEnvVars(): string { - const envVars = ['CLAUDECODE=1', 'CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS=1'] + const envVars = [ + 'CLAUDECODE=1', + 'CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS=1', + // Teammates should inherit the leader-selected provider route instead of + // replaying persisted ~/.claude or settings.env provider defaults. + 'CLAUDE_CODE_PROVIDER_MANAGED_BY_HOST=1', + ] for (const key of TEAMMATE_ENV_VARS) { const value = process.env[key] diff --git a/src/utils/worktree.test.ts b/src/utils/worktree.test.ts new file mode 100644 index 00000000..53b2fef0 --- /dev/null +++ b/src/utils/worktree.test.ts @@ -0,0 +1,69 @@ +import { afterEach, expect, test } from 'bun:test' + +import { + _resetGitWorktreeMutationLocksForTesting, + withGitWorktreeMutationLock, +} from './worktree.js' + +afterEach(() => { + _resetGitWorktreeMutationLocksForTesting() +}) + +test('withGitWorktreeMutationLock serializes mutations for the same repo', async () => { + const order: string[] = [] + let releaseFirst!: () => void + const firstGate = new Promise(resolve => { + releaseFirst = resolve + }) + + const first = withGitWorktreeMutationLock('/repo', async () => { + order.push('first:start') + await firstGate + order.push('first:end') + }) + + const second = withGitWorktreeMutationLock('/repo', async () => { + order.push('second:start') + order.push('second:end') + }) + + await Promise.resolve() + await Promise.resolve() + expect(order).toEqual(['first:start']) + + releaseFirst() + await Promise.all([first, second]) + + expect(order).toEqual([ + 'first:start', + 'first:end', + 'second:start', + 'second:end', + ]) +}) + +test('withGitWorktreeMutationLock does not serialize different repos', async () => { + const order: string[] = [] + let releaseFirst!: () => void + const firstGate = new Promise(resolve => { + releaseFirst = resolve + }) + + const first = withGitWorktreeMutationLock('/repo-a', async () => { + order.push('a:start') + await firstGate + order.push('a:end') + }) + + const second = withGitWorktreeMutationLock('/repo-b', async () => { + order.push('b:start') + order.push('b:end') + }) + + await Promise.resolve() + await Promise.resolve() + expect(order).toEqual(['a:start', 'b:start', 'b:end']) + + releaseFirst() + await Promise.all([first, second]) +}) diff --git a/src/utils/worktree.ts b/src/utils/worktree.ts index 5e07ba7f..2181017e 100644 --- a/src/utils/worktree.ts +++ b/src/utils/worktree.ts @@ -192,6 +192,36 @@ type WorktreeCreateResult = existed: false } +const gitWorktreeMutationLocks = new Map>() + +export async function withGitWorktreeMutationLock( + repoRoot: string, + fn: () => Promise, +): Promise { + const previous = gitWorktreeMutationLocks.get(repoRoot) ?? Promise.resolve() + let releaseCurrent!: () => void + const current = new Promise(resolve => { + releaseCurrent = resolve + }) + const next = previous.catch(() => {}).then(() => current) + gitWorktreeMutationLocks.set(repoRoot, next) + + await previous.catch(() => {}) + + try { + return await fn() + } finally { + releaseCurrent() + if (gitWorktreeMutationLocks.get(repoRoot) === next) { + gitWorktreeMutationLocks.delete(repoRoot) + } + } +} + +export function _resetGitWorktreeMutationLocksForTesting(): void { + gitWorktreeMutationLocks.clear() +} + // Env vars to prevent git/SSH from prompting for credentials (which hangs the CLI). // GIT_TERMINAL_PROMPT=0 prevents git from opening /dev/tty for credential prompts. // GIT_ASKPASS='' disables askpass GUI programs. @@ -254,124 +284,136 @@ async function getOrCreateWorktree( } } - // New worktree: fetch base branch then add - await mkdir(worktreesDir(repoRoot), { recursive: true }) - - const fetchEnv = { ...process.env, ...GIT_NO_PROMPT_ENV } - - let baseBranch: string - let baseSha: string | null = null - if (options?.prNumber) { - const { code: prFetchCode, stderr: prFetchStderr } = - await execFileNoThrowWithCwd( - gitExe(), - ['fetch', 'origin', `pull/${options.prNumber}/head`], - { cwd: repoRoot, stdin: 'ignore', env: fetchEnv }, - ) - if (prFetchCode !== 0) { - throw new Error( - `Failed to fetch PR #${options.prNumber}: ${prFetchStderr.trim() || 'PR may not exist or the repository may not have a remote named "origin"'}`, - ) + return withGitWorktreeMutationLock(repoRoot, async () => { + const lockedExistingHead = await readWorktreeHeadSha(worktreePath) + if (lockedExistingHead) { + return { + worktreePath, + worktreeBranch, + headCommit: lockedExistingHead, + existed: true, + } } - baseBranch = 'FETCH_HEAD' - } else { - // If origin/ already exists locally, skip fetch. In large repos - // (210k files, 16M objects) fetch burns ~6-8s on a local commit-graph - // scan before even hitting the network. A slightly stale base is fine — - // the user can pull in the worktree if they want latest. - // resolveRef reads the loose/packed ref directly; when it succeeds we - // already have the SHA, so the later rev-parse is skipped entirely. - const [defaultBranch, gitDir] = await Promise.all([ - getDefaultBranch(), - resolveGitDir(repoRoot), - ]) - const originRef = `origin/${defaultBranch}` - const originSha = gitDir - ? await resolveRef(gitDir, `refs/remotes/origin/${defaultBranch}`) - : null - if (originSha) { - baseBranch = originRef - baseSha = originSha + + // New worktree: fetch base branch then add + await mkdir(worktreesDir(repoRoot), { recursive: true }) + + const fetchEnv = { ...process.env, ...GIT_NO_PROMPT_ENV } + + let baseBranch: string + let baseSha: string | null = null + if (options?.prNumber) { + const { code: prFetchCode, stderr: prFetchStderr } = + await execFileNoThrowWithCwd( + gitExe(), + ['fetch', 'origin', `pull/${options.prNumber}/head`], + { cwd: repoRoot, stdin: 'ignore', env: fetchEnv }, + ) + if (prFetchCode !== 0) { + throw new Error( + `Failed to fetch PR #${options.prNumber}: ${prFetchStderr.trim() || 'PR may not exist or the repository may not have a remote named "origin"'}`, + ) + } + baseBranch = 'FETCH_HEAD' } else { - const { code: fetchCode } = await execFileNoThrowWithCwd( - gitExe(), - ['fetch', 'origin', defaultBranch], - { cwd: repoRoot, stdin: 'ignore', env: fetchEnv }, - ) - baseBranch = fetchCode === 0 ? originRef : 'HEAD' + // If origin/ already exists locally, skip fetch. In large repos + // (210k files, 16M objects) fetch burns ~6-8s on a local commit-graph + // scan before even hitting the network. A slightly stale base is fine — + // the user can pull in the worktree if they want latest. + // resolveRef reads the loose/packed ref directly; when it succeeds we + // already have the SHA, so the later rev-parse is skipped entirely. + const [defaultBranch, gitDir] = await Promise.all([ + getDefaultBranch(), + resolveGitDir(repoRoot), + ]) + const originRef = `origin/${defaultBranch}` + const originSha = gitDir + ? await resolveRef(gitDir, `refs/remotes/origin/${defaultBranch}`) + : null + if (originSha) { + baseBranch = originRef + baseSha = originSha + } else { + const { code: fetchCode } = await execFileNoThrowWithCwd( + gitExe(), + ['fetch', 'origin', defaultBranch], + { cwd: repoRoot, stdin: 'ignore', env: fetchEnv }, + ) + baseBranch = fetchCode === 0 ? originRef : 'HEAD' + } } - } - // For the fetch/PR-fetch paths we still need the SHA — the fs-only resolveRef - // above only covers the "origin/ already exists locally" case. - if (!baseSha) { - const { stdout, code: shaCode } = await execFileNoThrowWithCwd( - gitExe(), - ['rev-parse', baseBranch], - { cwd: repoRoot }, - ) - if (shaCode !== 0) { - throw new Error( - `Failed to resolve base branch "${baseBranch}": git rev-parse failed`, - ) - } - baseSha = stdout.trim() - } - - const sparsePaths = getInitialSettings().worktree?.sparsePaths - const addArgs = ['worktree', 'add'] - if (sparsePaths?.length) { - addArgs.push('--no-checkout') - } - // -B (not -b): reset any orphan branch left behind by a removed worktree dir. - // Saves a `git branch -D` subprocess (~15ms spawn overhead) on every create. - addArgs.push('-B', worktreeBranch, worktreePath, baseBranch) - - const { code: createCode, stderr: createStderr } = - await execFileNoThrowWithCwd(gitExe(), addArgs, { cwd: repoRoot }) - if (createCode !== 0) { - throw new Error(`Failed to create worktree: ${createStderr}`) - } - - if (sparsePaths?.length) { - // If sparse-checkout or checkout fail after --no-checkout, the worktree - // is registered and HEAD is set but the working tree is empty. Next run's - // fast-resume (rev-parse HEAD) would succeed and present a broken worktree - // as "resumed". Tear it down before propagating the error. - const tearDown = async (msg: string): Promise => { - await execFileNoThrowWithCwd( + // For the fetch/PR-fetch paths we still need the SHA — the fs-only resolveRef + // above only covers the "origin/ already exists locally" case. + if (!baseSha) { + const { stdout, code: shaCode } = await execFileNoThrowWithCwd( gitExe(), - ['worktree', 'remove', '--force', worktreePath], + ['rev-parse', baseBranch], { cwd: repoRoot }, ) - throw new Error(msg) + if (shaCode !== 0) { + throw new Error( + `Failed to resolve base branch "${baseBranch}": git rev-parse failed`, + ) + } + baseSha = stdout.trim() } - const { code: sparseCode, stderr: sparseErr } = - await execFileNoThrowWithCwd( + + const sparsePaths = getInitialSettings().worktree?.sparsePaths + const addArgs = ['worktree', 'add'] + if (sparsePaths?.length) { + addArgs.push('--no-checkout') + } + // -B (not -b): reset any orphan branch left behind by a removed worktree dir. + // Saves a `git branch -D` subprocess (~15ms spawn overhead) on every create. + addArgs.push('-B', worktreeBranch, worktreePath, baseBranch) + + const { code: createCode, stderr: createStderr } = + await execFileNoThrowWithCwd(gitExe(), addArgs, { cwd: repoRoot }) + if (createCode !== 0) { + throw new Error(`Failed to create worktree: ${createStderr}`) + } + + if (sparsePaths?.length) { + // If sparse-checkout or checkout fail after --no-checkout, the worktree + // is registered and HEAD is set but the working tree is empty. Next run's + // fast-resume (rev-parse HEAD) would succeed and present a broken worktree + // as "resumed". Tear it down before propagating the error. + const tearDown = async (msg: string): Promise => { + await execFileNoThrowWithCwd( + gitExe(), + ['worktree', 'remove', '--force', worktreePath], + { cwd: repoRoot }, + ) + throw new Error(msg) + } + const { code: sparseCode, stderr: sparseErr } = + await execFileNoThrowWithCwd( + gitExe(), + ['sparse-checkout', 'set', '--cone', '--', ...sparsePaths], + { cwd: worktreePath }, + ) + if (sparseCode !== 0) { + await tearDown(`Failed to configure sparse-checkout: ${sparseErr}`) + } + const { code: coCode, stderr: coErr } = await execFileNoThrowWithCwd( gitExe(), - ['sparse-checkout', 'set', '--cone', '--', ...sparsePaths], + ['checkout', 'HEAD'], { cwd: worktreePath }, ) - if (sparseCode !== 0) { - await tearDown(`Failed to configure sparse-checkout: ${sparseErr}`) + if (coCode !== 0) { + await tearDown(`Failed to checkout sparse worktree: ${coErr}`) + } } - const { code: coCode, stderr: coErr } = await execFileNoThrowWithCwd( - gitExe(), - ['checkout', 'HEAD'], - { cwd: worktreePath }, - ) - if (coCode !== 0) { - await tearDown(`Failed to checkout sparse worktree: ${coErr}`) - } - } - return { - worktreePath, - worktreeBranch, - headCommit: baseSha, - baseBranch, - existed: false, - } + return { + worktreePath, + worktreeBranch, + headCommit: baseSha, + baseBranch, + existed: false, + } + }) } /** @@ -984,39 +1026,41 @@ export async function removeAgentWorktree( return false } - // Run from the main repo root, not the worktree (which we're about to delete) - const { code: removeCode, stderr: removeError } = - await execFileNoThrowWithCwd( - gitExe(), - ['worktree', 'remove', '--force', worktreePath], - { cwd: gitRoot }, - ) + return withGitWorktreeMutationLock(gitRoot, async () => { + // Run from the main repo root, not the worktree (which we're about to delete) + const { code: removeCode, stderr: removeError } = + await execFileNoThrowWithCwd( + gitExe(), + ['worktree', 'remove', '--force', worktreePath], + { cwd: gitRoot }, + ) - if (removeCode !== 0) { - logForDebugging(`Failed to remove agent worktree: ${removeError}`, { - level: 'error', - }) - return false - } - logForDebugging(`Removed agent worktree at: ${worktreePath}`) + if (removeCode !== 0) { + logForDebugging(`Failed to remove agent worktree: ${removeError}`, { + level: 'error', + }) + return false + } + logForDebugging(`Removed agent worktree at: ${worktreePath}`) - if (!worktreeBranch) { + if (!worktreeBranch) { + return true + } + + // Delete the temporary worktree branch from the main repo + const { code: deleteBranchCode, stderr: deleteBranchError } = + await execFileNoThrowWithCwd(gitExe(), ['branch', '-D', worktreeBranch], { + cwd: gitRoot, + }) + + if (deleteBranchCode !== 0) { + logForDebugging( + `Could not delete agent worktree branch: ${deleteBranchError}`, + { level: 'error' }, + ) + } return true - } - - // Delete the temporary worktree branch from the main repo - const { code: deleteBranchCode, stderr: deleteBranchError } = - await execFileNoThrowWithCwd(gitExe(), ['branch', '-D', worktreeBranch], { - cwd: gitRoot, - }) - - if (deleteBranchCode !== 0) { - logForDebugging( - `Could not delete agent worktree branch: ${deleteBranchError}`, - { level: 'error' }, - ) - } - return true + }) } /**