fix serialize git worktree mutations and forward teammate PATH (#721)
This commit is contained in:
33
src/utils/swarm/spawnUtils.test.ts
Normal file
33
src/utils/swarm/spawnUtils.test.ts
Normal file
@@ -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')
|
||||||
|
})
|
||||||
@@ -141,6 +141,9 @@ const TEAMMATE_ENV_VARS = [
|
|||||||
'NODE_EXTRA_CA_CERTS',
|
'NODE_EXTRA_CA_CERTS',
|
||||||
'REQUESTS_CA_BUNDLE',
|
'REQUESTS_CA_BUNDLE',
|
||||||
'CURL_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
|
] as const
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -149,7 +152,13 @@ const TEAMMATE_ENV_VARS = [
|
|||||||
* plus any provider/config env vars that are set in the current process.
|
* plus any provider/config env vars that are set in the current process.
|
||||||
*/
|
*/
|
||||||
export function buildInheritedEnvVars(): string {
|
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) {
|
for (const key of TEAMMATE_ENV_VARS) {
|
||||||
const value = process.env[key]
|
const value = process.env[key]
|
||||||
|
|||||||
69
src/utils/worktree.test.ts
Normal file
69
src/utils/worktree.test.ts
Normal file
@@ -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<void>(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<void>(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])
|
||||||
|
})
|
||||||
@@ -192,6 +192,36 @@ type WorktreeCreateResult =
|
|||||||
existed: false
|
existed: false
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const gitWorktreeMutationLocks = new Map<string, Promise<void>>()
|
||||||
|
|
||||||
|
export async function withGitWorktreeMutationLock<T>(
|
||||||
|
repoRoot: string,
|
||||||
|
fn: () => Promise<T>,
|
||||||
|
): Promise<T> {
|
||||||
|
const previous = gitWorktreeMutationLocks.get(repoRoot) ?? Promise.resolve()
|
||||||
|
let releaseCurrent!: () => void
|
||||||
|
const current = new Promise<void>(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).
|
// 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_TERMINAL_PROMPT=0 prevents git from opening /dev/tty for credential prompts.
|
||||||
// GIT_ASKPASS='' disables askpass GUI programs.
|
// GIT_ASKPASS='' disables askpass GUI programs.
|
||||||
@@ -254,6 +284,17 @@ async function getOrCreateWorktree(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
return withGitWorktreeMutationLock(repoRoot, async () => {
|
||||||
|
const lockedExistingHead = await readWorktreeHeadSha(worktreePath)
|
||||||
|
if (lockedExistingHead) {
|
||||||
|
return {
|
||||||
|
worktreePath,
|
||||||
|
worktreeBranch,
|
||||||
|
headCommit: lockedExistingHead,
|
||||||
|
existed: true,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// New worktree: fetch base branch then add
|
// New worktree: fetch base branch then add
|
||||||
await mkdir(worktreesDir(repoRoot), { recursive: true })
|
await mkdir(worktreesDir(repoRoot), { recursive: true })
|
||||||
|
|
||||||
@@ -372,6 +413,7 @@ async function getOrCreateWorktree(
|
|||||||
baseBranch,
|
baseBranch,
|
||||||
existed: false,
|
existed: false,
|
||||||
}
|
}
|
||||||
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -984,6 +1026,7 @@ export async function removeAgentWorktree(
|
|||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
|
|
||||||
|
return withGitWorktreeMutationLock(gitRoot, async () => {
|
||||||
// Run from the main repo root, not the worktree (which we're about to delete)
|
// Run from the main repo root, not the worktree (which we're about to delete)
|
||||||
const { code: removeCode, stderr: removeError } =
|
const { code: removeCode, stderr: removeError } =
|
||||||
await execFileNoThrowWithCwd(
|
await execFileNoThrowWithCwd(
|
||||||
@@ -1017,6 +1060,7 @@ export async function removeAgentWorktree(
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
return true
|
return true
|
||||||
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
Reference in New Issue
Block a user