diff --git a/scripts/testing-v2/test-map-execution.mjs b/scripts/testing-v2/test-map-execution.mjs index 1fb730ecd..99cccfbf3 100644 --- a/scripts/testing-v2/test-map-execution.mjs +++ b/scripts/testing-v2/test-map-execution.mjs @@ -12,6 +12,7 @@ export const APPROVED_E2E_VITEST_PATHS = Object.freeze([ "tests2/core/marketplace-install.test.ts", "tests2/core/orphan-tool-result-rehydration-boundaries.test.ts", "tests2/core/team-manager.test.ts", + "tests2/integration/team-spawn-multi-repo-real-git.test.ts", ]); export const ISOLATED_VITEST_FILES = Object.freeze({ diff --git a/src/server/agent/session-manager.ts b/src/server/agent/session-manager.ts index 5ccf87a62..08a21df46 100755 --- a/src/server/agent/session-manager.ts +++ b/src/server/agent/session-manager.ts @@ -7729,7 +7729,7 @@ export class SessionManager { } } - async createSession(cwd: string, agentArgs?: string[], goalId?: string, assistantType?: string, opts?: { rolePrompt?: string; roleName?: string; role?: string; teamGoalId?: string; teamLeadSessionId?: string; accessory?: string; nonInteractive?: boolean; env?: Record; taskId?: string; staffId?: string; allowedTools?: string[]; workflowContext?: string; worktreeOpts?: { repoPath: string }; reattemptGoalId?: string; sandboxed?: boolean; projectId?: string; sessionId?: string; allowSessionReuse?: boolean; sandboxBranch?: string; sandboxBaseBranch?: string; sandboxCwdOffset?: string; skipAutoModel?: boolean; skipAutoThinking?: boolean; initialModel?: string; initialThinkingLevel?: string; preExistingAgentSessionFile?: string; preExistingAgentSessionOldCwds?: string[]; parentSessionId?: string; childKind?: string; readOnly?: boolean; title?: string; awaitWorktreeSetup?: boolean; bypassWorktreePool?: boolean }): Promise { + async createSession(cwd: string, agentArgs?: string[], goalId?: string, assistantType?: string, opts?: { rolePrompt?: string; roleName?: string; role?: string; teamGoalId?: string; teamLeadSessionId?: string; accessory?: string; nonInteractive?: boolean; env?: Record; taskId?: string; staffId?: string; allowedTools?: string[]; workflowContext?: string; worktreeOpts?: { repoPath: string }; worktreePath?: string; repoPath?: string; branch?: string; repoWorktrees?: Record; reattemptGoalId?: string; sandboxed?: boolean; projectId?: string; sessionId?: string; allowSessionReuse?: boolean; sandboxBranch?: string; sandboxBaseBranch?: string; sandboxCwdOffset?: string; skipAutoModel?: boolean; skipAutoThinking?: boolean; initialModel?: string; initialThinkingLevel?: string; preExistingAgentSessionFile?: string; preExistingAgentSessionOldCwds?: string[]; parentSessionId?: string; childKind?: string; readOnly?: boolean; title?: string; awaitWorktreeSetup?: boolean; bypassWorktreePool?: boolean }): Promise { const id = opts?.sessionId || randomUUID(); // Guard against silently clobbering an existing session's transcript. A // caller-supplied sessionId that already maps to a LIVE session (or an @@ -7977,6 +7977,13 @@ export class SessionManager { childKind: opts?.childKind, readOnly: opts?.readOnly, sessionScopedAllowedTools, + // Prebuilt host multi-repo worktrees already have all ordinary-cleanup + // coordinates. Carry them into persistOnce instead of adding them only + // after createSession returns. + worktreePath: opts?.worktreePath, + repoPath: opts?.repoPath, + branch: opts?.branch, + repoWorktrees: opts?.repoWorktrees, // Load-bearing wire: same contract as the worktree branch above. // Pinned by `tests/staff-session-staffid-persistence.test.ts`. staffId: opts?.staffId, @@ -10822,8 +10829,9 @@ export class SessionManager { for (const team of projectCtx.teamStore.getAll()) { const ownerGoal = goalsById.get(team.goalId); for (const agent of team.agents) { - teamRefs.push({ id: agent.sessionId, repoPath: ownerGoal?.repoPath ?? projectCtx.project.rootPath, worktreePath: agent.worktreePath, branch: agent.branch }); - addBranchGuard(ownerGoal?.repoPath ?? projectCtx.project.rootPath, agent.branch); + const repoPath = ownerGoal?.repoPath ?? projectCtx.project.rootPath; + teamRefs.push({ id: agent.sessionId, repoPath, worktreePath: agent.worktreePath, branch: agent.branch, repoWorktrees: agent.repoWorktrees }); + addRepoBranches(repoPath, agent.branch, agent.repoWorktrees); } const lead = team.teamLeadSessionId ? projectCtx.sessionStore.get(team.teamLeadSessionId) : undefined; if (lead) { @@ -11241,7 +11249,7 @@ export class SessionManager { // Skip delegates — they share the parent's worktree and must never remove it. if (ps.worktreePath && ps.repoPath && !ps.worktreePath.startsWith("/workspace") && !ps.delegateOf) { try { - const { cleanupWorktree } = await import("../skills/git.js"); + const { cleanupWorktree, removeEmptyWorktreeSetContainer } = await import("../skills/git.js"); const allPersisted = this.getAllPersistedSessionsForWorktreeGuard(); // Multi-repo: clean each repo's worktree with the shared background-I/O // ceiling + delete the shared branch from each repo's remote (Phase 4a). @@ -11254,8 +11262,19 @@ export class SessionManager { const repoPath = repo === "." ? ps.repoPath! : path.join(ps.repoPath!, repo); try { await cleanupWorktree(repoPath, wt, ps.branch, true, this.commandRunner, this.remoteGitPolicy); - } catch { /* preserve per-repo all-settled isolation */ } + try { + await fsp.access(wt); + console.error(`[session-manager] Component "${repo}" cleanup left worktree for ${ps.id}: ${wt}`); + } catch { /* removed */ } + } catch (err) { + console.error(`[session-manager] Failed to clean up component "${repo}" worktree for ${ps.id}:`, err); + } }); + try { + await removeEmptyWorktreeSetContainer(ps.worktreePath, Object.values(ps.repoWorktrees)); + } catch (err) { + console.error(`[session-manager] Failed to remove multi-repo branch container for ${ps.id}: ${ps.worktreePath}`, err); + } } else if (!isWorktreePathReferencedByLiveSession(ps.worktreePath, allPersisted, { ignoreSessionId: ps.id })) { await cleanupWorktree(ps.repoPath, ps.worktreePath, ps.branch, true, this.commandRunner, this.remoteGitPolicy); } else { diff --git a/src/server/agent/session-setup.ts b/src/server/agent/session-setup.ts index 2a571eaf8..510f092ac 100644 --- a/src/server/agent/session-setup.ts +++ b/src/server/agent/session-setup.ts @@ -267,6 +267,7 @@ export interface SessionSetupPlan { worktreePath?: string; repoPath?: string; branch?: string; + repoWorktrees?: Record; sandboxed?: boolean; role?: string; staffId?: string; @@ -1059,6 +1060,7 @@ export function persistOnce(session: SessionInfo, plan: SessionSetupPlan, store: worktreePath: plan.worktreePath, repoPath: plan.repoPath, branch: plan.branch, + repoWorktrees: plan.repoWorktrees, taskId: plan.taskId, staffId: plan.staffId, accessory: plan.accessory, @@ -1589,6 +1591,16 @@ async function spawnAgent(plan: SessionSetupPlan, ctx: PipelineContext): Promise role: plan.role ?? plan.roleName, accessory: plan.accessory, nonInteractive: plan.nonInteractive, + ...(plan.repoWorktrees && plan.repoPath ? { + worktreePath: plan.worktreePath, + repoPath: plan.repoPath, + branch: plan.branch, + repoWorktrees: Object.entries(plan.repoWorktrees).map(([repo, worktreePath]) => ({ + repo, + repoPath: repo === "." ? plan.repoPath! : path.join(plan.repoPath!, repo), + worktreePath, + })), + } : {}), promptQueue: new PromptQueue(), spawnPinnedModel, spawnPinnedThinkingLevel, @@ -1805,7 +1817,9 @@ export function handleSetupFailure( broadcastStatus(session, "terminated"); // 6. Background worktree cleanup (slow, non-blocking) - if (plan.worktreePath && plan.repoPath && plan.branch) { + // Pre-provisioned multi-repo workers are cleaned component-by-component by + // TeamManager after createSession rejects; their flat paths are non-Git containers. + if ((!plan.repoWorktrees || Object.keys(plan.repoWorktrees).length === 0) && plan.worktreePath && plan.repoPath && plan.branch) { const persistedSessions = ctx.listPersistedSessionsForWorktreeGuard?.() ?? ctx.store.getAll(); if (!isWorktreePathReferencedByLiveSession(plan.worktreePath, persistedSessions, { ignoreSessionId: session.id })) { cleanupWorktree(plan.repoPath, plan.worktreePath, plan.branch, true, ctx.commandRunner, ctx.remoteGitPolicy).catch(() => {}); diff --git a/src/server/agent/team-manager.ts b/src/server/agent/team-manager.ts index 7afd1e564..c8c80fbbe 100644 --- a/src/server/agent/team-manager.ts +++ b/src/server/agent/team-manager.ts @@ -9,7 +9,7 @@ import type { PromptSource, SessionManager, SessionInfo } from "./session-manage import { isNonRetryableAgentError, isProviderBackoffError, isRetryableGenericAgentError, isTransientReviewError } from "./verification-logic.js"; import { GoalManager } from "./goal-manager.js"; import { GoalStore, type PersistedGoal } from "./goal-store.js"; -import { createWorktree, cleanupWorktree } from "../skills/git.js"; +import { createWorktree, createWorktreeSet, cleanupWorktree, removeEmptyWorktreeSetContainer } from "../skills/git.js"; import { applyPromptConditionals } from "./prompt-conditionals.js"; import type { RoleStore, Role } from "./role-store.js"; import { resolveRole, listAvailableRoles, type RoleSource } from "./resolve-role.js"; @@ -165,6 +165,41 @@ async function resolveTeamMemberStartPoint(goal: PersistedGoal): Promise>; + +async function cleanupCreatedWorktreeSet( + result: CreatedWorktreeSet, + branchName: string | undefined, + commandRunner: CommandRunner, +): Promise { + for (const worktree of [...result.worktrees].reverse()) { + try { + await cleanupWorktree( + worktree.repoPath, + worktree.worktreePath, + branchName, + result.createdBranchRepos?.has(worktree.repo) === true, + commandRunner, + { skipRemotePush: true }, + ); + try { + await fs.promises.access(worktree.worktreePath); + console.error(`[team-manager] Component "${worktree.repo}" cleanup left worktree at ${worktree.worktreePath}`); + } catch { /* removed */ } + } catch (err) { + console.error(`[team-manager] Failed to clean up component "${worktree.repo}" worktree ${worktree.worktreePath}:`, err); + } + } + try { + await removeEmptyWorktreeSetContainer( + result.container, + result.worktrees.map(worktree => worktree.worktreePath), + ); + } catch (err) { + console.error(`[team-manager] Failed to remove worker branch container ${result.container}:`, err); + } +} + function splitWorkerResultSummary(resultSummary: string): { summary?: string; branch?: string; commit?: string; checks?: string } { let rest = resultSummary.trim(); let branch: string | undefined; @@ -247,6 +282,8 @@ export interface TeamAgent { /** In-memory marker for pre-kind reviewer records that need legacy safeguards. */ legacyMissingKind?: boolean; worktreePath?: string; + /** Per-component worktree paths for a multi-repo worker. */ + repoWorktrees?: Record; branch?: string; baseSha?: string; task: string; @@ -579,6 +616,7 @@ export class TeamManager { role: a.role, kind: a.kind, worktreePath: a.worktreePath, + repoWorktrees: a.repoWorktrees, branch: a.branch, baseSha: a.baseSha, task: a.task, @@ -1005,6 +1043,7 @@ export class TeamManager { kind: (a.kind === "reviewer" ? "reviewer" : "worker"), legacyMissingKind: !hasPersistedKind && a.role === "reviewer" ? true : undefined, worktreePath: a.worktreePath, + repoWorktrees: a.repoWorktrees, branch: a.branch, baseSha: a.baseSha, task: a.task, @@ -2069,6 +2108,11 @@ export class TeamManager { // Headquarters never gets member worktrees, even if a legacy goal record // still has repo/branch fields from the pre-split implementation. const useWorktree = !isHeadquartersProject(goal.projectId) && !!goal.repoPath && !!goal.branch; + const goalRepoWorktrees = goal.repoWorktrees; + const distinctGoalRepoWorktrees = goalRepoWorktrees + ? new Set(Object.values(goalRepoWorktrees).map(worktreePath => path.resolve(worktreePath))) + : new Set(); + const isMultiRepoGoal = distinctGoalRepoWorktrees.size >= 2; // Enforce gate dependency check: upstream gates must be passed before spawning for a gate const resolvedWorkflowGateId = opts?.workflowGateId ?? this.extractWorkflowGateId(task, goalId); @@ -2088,6 +2132,9 @@ export class TeamManager { // so the on-disk directory is `goal---`. const shortId = randomUUID().slice(0, 4); let worktreeResult: { worktreePath: string; branchName: string } | undefined; + let multiRepoWorktreeResult: CreatedWorktreeSet | undefined; + let workerRepoWorktrees: Record | undefined; + let multiRepoStartPoints: Record | undefined; let branchName: string | undefined; let agentCwd: string; let memberStartPoint: string | undefined; @@ -2097,7 +2144,9 @@ export class TeamManager { const goalId8 = goalId.slice(0, 8); branchName = `goal/${goalId8}/${role}-${shortId}`; - memberStartPoint = await resolveTeamMemberStartPoint(goal); + if (!isMultiRepoGoal) { + memberStartPoint = await resolveTeamMemberStartPoint(goal); + } // Compute subdirectory offset from the goal's worktree root to its cwd. // If the project rootPath is a subdirectory of the repo, goal.cwd includes @@ -2111,6 +2160,59 @@ export class TeamManager { // Sandboxed: worktree created inside the container by applySandboxWiring // via ProjectSandbox.createWorktree(). Use goal.cwd as placeholder. agentCwd = goal.cwd; // placeholder — sandbox wiring overrides this + } else if (isMultiRepoGoal && goalRepoWorktrees) { + // A true multi-repo goal is identified by its persisted component + // worktrees, not by probing the non-Git project container. + const projectContext = this.config.projectContextManager?.getContextForGoal(goalId); + if (!projectContext) { + throw new Error(`Cannot resolve project components for multi-repo team member goal "${goalId}"`); + } + const configuredComponents = projectContext.projectConfigStore.getComponents(); + const configuredRepos = new Set(configuredComponents.map(component => component.repo)); + for (const repo of Object.keys(goalRepoWorktrees)) { + if (!configuredRepos.has(repo)) { + throw new Error(`Cannot resolve project component "${repo}" for multi-repo team member goal "${goalId}"`); + } + } + const components = configuredComponents.filter(component => + Object.prototype.hasOwnProperty.call(goalRepoWorktrees, component.repo), + ); + multiRepoStartPoints = {}; + for (const [repo, goalWorktreePath] of Object.entries(goalRepoWorktrees)) { + try { + const { stdout } = await this.commandRunner.execFile("git", ["rev-parse", "HEAD"], { + cwd: goalWorktreePath, + timeout: 5_000, + }); + const head = stdout.toString().trim(); + if (!head) throw new Error("git returned an empty commit SHA"); + multiRepoStartPoints[repo] = head; + } catch (err) { + throw new Error( + `Cannot run git rev-parse HEAD for multi-repo team member component "${repo}" at ${goalWorktreePath}: ` + + `${err instanceof Error ? err.message : String(err)}`, + ); + } + } + + multiRepoWorktreeResult = await createWorktreeSet(goal.repoPath!, components, branchName, undefined, { + worktreeRoot: projectContext.projectConfigStore.get("worktree_root") || undefined, + configuredBaseRef: projectContext.projectConfigStore.get("base_ref") || undefined, + commandRunner: this.commandRunner, + startPointsByRepo: multiRepoStartPoints, + }); + const createdRepos = new Set(multiRepoWorktreeResult.worktrees.map(worktree => worktree.repo)); + for (const repo of Object.keys(multiRepoStartPoints)) { + if (!createdRepos.has(repo)) { + await cleanupCreatedWorktreeSet(multiRepoWorktreeResult, branchName, this.commandRunner); + throw new Error(`createWorktreeSet did not create multi-repo team member component "${repo}"`); + } + } + workerRepoWorktrees = Object.fromEntries( + multiRepoWorktreeResult.worktrees.map(worktree => [worktree.repo, worktree.worktreePath]), + ); + worktreeResult = { worktreePath: multiRepoWorktreeResult.container, branchName }; + agentCwd = multiRepoWorktreeResult.container; } else { // Non-sandboxed: create the member worktree from local goal state. Goal // branches may be unpublished, so prefer local refs before origin refs. @@ -2136,13 +2238,20 @@ export class TeamManager { // and is created later by applySandboxWiring, which fires the hook itself // (with the actual container worktree path). Non-fatal. if (worktreeResult) { - await this.sessionManager.dispatchGoalProvisionedForWorktree({ - goalId, - projectId: goal.projectId, - worktreePath: worktreeResult.worktreePath, - cwd: agentCwd, - branch: branchName, - }); + try { + await this.sessionManager.dispatchGoalProvisionedForWorktree({ + goalId, + projectId: goal.projectId, + worktreePath: worktreeResult.worktreePath, + cwd: agentCwd, + branch: branchName, + }); + } catch (err) { + if (multiRepoWorktreeResult) { + await cleanupCreatedWorktreeSet(multiRepoWorktreeResult, branchName, this.commandRunner); + } + throw err; + } } try { @@ -2170,6 +2279,15 @@ export class TeamManager { undefined, { rolePrompt, roleName: role, workflowContext, sandboxed: memberSandboxed, + // A host multi-repo worker is already fully provisioned. Thread its + // ordinary-cleanup coordinates into createSession so the initial + // persisted session row owns them before createSession returns. + ...(workerRepoWorktrees ? { + worktreePath: worktreeResult?.worktreePath, + repoPath: goal.repoPath, + branch: branchName, + repoWorktrees: workerRepoWorktrees, + } : {}), // Pass branch info so applySandboxWiring creates the worktree inside the container. // The base branch is local-ref-first because sandbox goal branches may be unpublished. sandboxBranch: memberSandboxed && branchName ? branchName : undefined, @@ -2195,6 +2313,11 @@ export class TeamManager { role, teamGoalId: goalId, worktreePath: actualWorktreePath, + ...(workerRepoWorktrees ? { + repoPath: goal.repoPath, + branch: branchName, + repoWorktrees: workerRepoWorktrees, + } : {}), accessory: roleAccessory, teamLeadSessionId: entry.teamLeadSessionId ?? undefined, }; @@ -2203,19 +2326,21 @@ export class TeamManager { // Resolve baseSha from the agent's working directory. // For sandboxed sessions, run git inside the container. let baseSha: string | undefined; - try { - const effectiveCwd = actualWorktreePath || session.cwd || agentCwd; - if (memberSandboxed && this.sessionManager.getSandboxManager()) { - const sandbox = this.sessionManager.getSandboxManager()!.get(goal.projectId || ""); - if (sandbox) { - const output = await sandbox.exec(["git", "rev-parse", "HEAD"], { cwd: effectiveCwd }); - baseSha = output.trim() || undefined; + if (!multiRepoStartPoints) { + try { + const effectiveCwd = actualWorktreePath || session.cwd || agentCwd; + if (memberSandboxed && this.sessionManager.getSandboxManager()) { + const sandbox = this.sessionManager.getSandboxManager()!.get(goal.projectId || ""); + if (sandbox) { + const output = await sandbox.exec(["git", "rev-parse", "HEAD"], { cwd: effectiveCwd }); + baseSha = output.trim() || undefined; + } + } else { + const { stdout } = await execFile("git", ["rev-parse", "HEAD"], { cwd: effectiveCwd, timeout: 5_000 }); + baseSha = stdout.trim() || undefined; } - } else { - const { stdout } = await execFile("git", ["rev-parse", "HEAD"], { cwd: effectiveCwd, timeout: 5_000 }); - baseSha = stdout.trim() || undefined; - } - } catch { /* non-fatal — baseSha stays undefined */ } + } catch { /* non-fatal — baseSha stays undefined */ } + } // Track the agent const agent: TeamAgent = { @@ -2223,6 +2348,7 @@ export class TeamManager { role, kind: "worker", worktreePath: actualWorktreePath, + repoWorktrees: workerRepoWorktrees, branch: branchName, baseSha, task, @@ -2298,8 +2424,10 @@ export class TeamManager { return { sessionId: session.id, worktreePath: worktreeResult?.worktreePath }; } catch (err) { - // Clean up the orphaned worktree on failure (only if one was created) - if (worktreeResult && goal.repoPath) { + // Clean up orphaned worktrees if worker-session creation fails. + if (multiRepoWorktreeResult) { + await cleanupCreatedWorktreeSet(multiRepoWorktreeResult, branchName, this.commandRunner); + } else if (worktreeResult && goal.repoPath) { try { await cleanupWorktree(goal.repoPath, worktreeResult.worktreePath, branchName, true); console.log(`[team-manager] Cleaned up orphaned worktree after spawnRole failure: ${worktreeResult.worktreePath}`); @@ -2524,13 +2652,24 @@ export class TeamManager { this.sessionManager.updateSessionMeta(sessionId, { worktreePath: agent.worktreePath, teamGoalId: goalId, + ...(agent.repoWorktrees ? { + repoPath: goal.repoPath, + branch: agent.branch, + repoWorktrees: agent.repoWorktrees, + } : {}), } as any); - // Store repoPath and branch in the session store for later purge cleanup + // Store repo coordinates for later purge cleanup. Multi-repo workers + // retain their component map so purge never probes the branch container. const projectCtx = goalId && this.config.projectContextManager ? this.config.projectContextManager.getContextForGoal(goalId) : null; if (projectCtx) { - projectCtx.sessionStore.update(sessionId, { repoPath: goal.repoPath, branch: agent.branch, teamGoalId: goalId } as any); + projectCtx.sessionStore.update(sessionId, { + repoPath: goal.repoPath, + branch: agent.branch, + teamGoalId: goalId, + ...(agent.repoWorktrees ? { repoWorktrees: agent.repoWorktrees } : {}), + } as any); } } diff --git a/src/server/agent/team-store.ts b/src/server/agent/team-store.ts index bad9cbfc1..05f5f673c 100644 --- a/src/server/agent/team-store.ts +++ b/src/server/agent/team-store.ts @@ -15,6 +15,8 @@ export interface PersistedTeamEntry { */ kind?: "worker" | "reviewer"; worktreePath?: string; + /** Per-component worktree paths for a multi-repo worker. */ + repoWorktrees?: Record; branch?: string; baseSha?: string; task: string; diff --git a/src/server/skills/git.ts b/src/server/skills/git.ts index 80e500cce..d2f4ddc5b 100644 --- a/src/server/skills/git.ts +++ b/src/server/skills/git.ts @@ -607,12 +607,91 @@ export interface CreateWorktreeSetOptions { configuredBaseRef?: string; commandRunner?: CommandRunner; remotePolicy?: RemoteGitPolicy; + /** Exact authoritative start commit/ref for each repository key. */ + startPointsByRepo?: Record; /** @deprecated Ignored. Worktree creation is always local-only. */ pushPolicy?: "local-only" | "publish"; /** @deprecated Ignored. Worktree creation is always local-only. */ skipPush?: boolean; } +interface WorktreeSetEntry { + repo: string; + repoPath: string; + worktreePath: string; +} + +/** + * Remove only the known empty repo-key ancestors and branch container left + * after a multi-repo worktree set has been cleaned. This deliberately uses + * non-recursive rmdir calls so unexpected files are never removed. + */ +export async function removeEmptyWorktreeSetContainer( + container: string, + worktreePaths: Iterable, +): Promise { + const containerPath = path.resolve(container); + const ancestors = new Set(); + for (const worktreePath of worktreePaths) { + const relative = path.relative(containerPath, path.resolve(worktreePath)); + if (!relative || relative === ".." || relative.startsWith(`..${path.sep}`) || path.isAbsolute(relative)) continue; + const parts = relative.split(path.sep); + for (let i = 1; i < parts.length; i++) { + ancestors.add(path.join(containerPath, ...parts.slice(0, i))); + } + } + + const failures: string[] = []; + const targets = [...ancestors].sort((a, b) => b.length - a.length); + targets.push(containerPath); + for (const target of targets) { + try { + await fs.promises.rmdir(target); + } catch (err: any) { + if (err?.code !== "ENOENT") { + failures.push(`${target}: ${err instanceof Error ? err.message : String(err)}`); + } + } + } + if (failures.length > 0) { + throw new Error(failures.join("; ")); + } +} + +async function rollbackWorktreeSet( + entries: WorktreeSetEntry[], + createdBranchRepos: ReadonlySet, + container: string, + branchName: string, + commandRunner: CommandRunner, + remotePolicy: RemoteGitPolicy, +): Promise { + const failures: string[] = []; + for (const entry of [...entries].reverse()) { + try { + await cleanupWorktree( + entry.repoPath, + entry.worktreePath, + branchName, + createdBranchRepos.has(entry.repo), + commandRunner, + { ...remotePolicy, skipRemotePush: true }, + ); + if (await pathExists(entry.worktreePath)) { + failures.push(`component "${entry.repo}" cleanup left worktree at ${entry.worktreePath}`); + } + } catch (err) { + failures.push(`component "${entry.repo}" cleanup failed: ${err instanceof Error ? err.message : String(err)}`); + } + } + try { + await removeEmptyWorktreeSetContainer(container, entries.map(entry => entry.worktreePath)); + } catch (err) { + failures.push(`branch container cleanup failed at ${container}: ${err instanceof Error ? err.message : String(err)}`); + } + return failures; +} + export interface WorktreeResult { worktreePath: string; branchName: string; @@ -796,7 +875,12 @@ export async function createWorktreeSet( branchName: string, baseBranch?: string, opts?: CreateWorktreeSetOptions, -): Promise<{ container: string; worktrees: Array<{ repo: string; repoPath: string; worktreePath: string }> }> { +): Promise<{ + container: string; + worktrees: Array<{ repo: string; repoPath: string; worktreePath: string }>; + /** Multi-repo components whose local branch was created by this call. */ + createdBranchRepos?: ReadonlySet; +}> { // Per-component worktree setup is the caller's responsibility — invoke // `runComponentSetups()` after this function returns. // Distinct repos in declared order. @@ -811,6 +895,7 @@ export async function createWorktreeSet( const configuredBaseRefTrimmed = (opts?.configuredBaseRef ?? "").trim(); const commandRunner = opts?.commandRunner ?? realCommandRunner; const remotePolicy = opts?.remotePolicy ?? DEFAULT_REMOTE_GIT_POLICY; + const startPointsByRepo = opts?.startPointsByRepo; const runGit = (args: readonly string[], options?: any) => execGit(args, options, commandRunner); // Single-repo path collapses to existing behavior. `configuredBaseRef` @@ -842,11 +927,17 @@ export async function createWorktreeSet( const repoList: string[] = []; for (const repo of repos) { const repoSrc = path.join(rootPath, repo === "." ? "" : repo); - if (!(await isGitRepoRoot(repoSrc, commandRunner))) continue; + const hasExactStart = !!startPointsByRepo && Object.prototype.hasOwnProperty.call(startPointsByRepo, repo); + if (!(await isGitRepoRoot(repoSrc, commandRunner))) { + if (hasExactStart) { + throw new Error(`createWorktreeSet: validate source repo failed for component "${repo}": ${repoSrc} is not a Git repository root`); + } + continue; + } // When no explicit start point/base_ref is configured, this component would // fall back to literal HEAD. Skip unborn repos before git worktree sees an - // invalid start point; explicit base refs still surface their normal errors. - if (!baseBranch && !configuredBaseRefTrimmed && !(await hasResolvedHead(repoSrc, commandRunner))) continue; + // invalid start point; explicit per-repo starts remain authoritative. + if (!hasExactStart && !baseBranch && !configuredBaseRefTrimmed && !(await hasResolvedHead(repoSrc, commandRunner))) continue; repoList.push(repo); } @@ -866,80 +957,103 @@ export async function createWorktreeSet( // Operation-first creation is idempotent and avoids an exists/mkdir race. await fs.promises.mkdir(container, { recursive: true }); - const out: Array<{ repo: string; repoPath: string; worktreePath: string }> = []; - for (const repo of repoList) { - const repoSrc = path.join(rootPath, repo); - const wtPath = path.join(container, repo); - // Keep the existing actionable result if the source disappears after the - // canonical repo-root scan and before this component is created. - if (!await pathExists(repoSrc)) { - throw new Error(`createWorktreeSet: source repo not found: ${repoSrc}`); - } - // Start-point precedence mirrors `createWorktree`: - // 1. explicit `baseBranch` arg (per-call override) - // 2. `opts.configuredBaseRef` (non-empty) (project's `base_ref` setting) - // 3. `resolveRemotePrimary(repoSrc)` (today's fallback) - let startPoint: string; - let startPointFromConfiguredBase = false; - if (baseBranch) { - startPoint = baseBranch; - } else if (configuredBaseRefTrimmed) { - startPoint = parseBaseRef(configuredBaseRefTrimmed).ref; - startPointFromConfiguredBase = true; - } else { - startPoint = await resolveRemotePrimary(repoSrc, commandRunner); - } - - // Branch may already exist from a prior partial attempt. - let branchExists = false; - try { - await runGit(["rev-parse", "--verify", branchName], { cwd: repoSrc }); - branchExists = true; - } catch { /* not present */ } - - try { - if (branchExists) { - await runGit(["worktree", "add", wtPath, branchName], { cwd: repoSrc }); + const out: WorktreeSetEntry[] = []; + const createdBranchRepos = new Set(); + try { + for (const repo of repoList) { + const repoSrc = path.join(rootPath, repo); + const wtPath = path.join(container, repo); + // Keep the existing actionable result if the source disappears after the + // canonical repo-root scan and before this component is created. + if (!await pathExists(repoSrc)) { + throw new Error(`createWorktreeSet: validate source repo failed for component "${repo}": source repo not found at ${repoSrc}`); + } + // An exact per-repo start takes precedence over every shared fallback. + const exactStart = startPointsByRepo && Object.prototype.hasOwnProperty.call(startPointsByRepo, repo) + ? startPointsByRepo[repo].trim() + : undefined; + let startPoint: string; + let startPointFromConfiguredBase = false; + if (exactStart !== undefined) { + if (!exactStart) throw new Error(`createWorktreeSet: resolve exact start failed for component "${repo}": start point is empty`); + startPoint = exactStart; + } else if (baseBranch) { + startPoint = baseBranch; + } else if (configuredBaseRefTrimmed) { + startPoint = parseBaseRef(configuredBaseRefTrimmed).ref; + startPointFromConfiguredBase = true; } else { - await runGit(["worktree", "add", "-b", branchName, wtPath, startPoint], { cwd: repoSrc }); + startPoint = await resolveRemotePrimary(repoSrc, commandRunner); } - } catch (err) { - if (startPointFromConfiguredBase && !branchExists) { - // Configured base ref disappeared between save and creation in this - // component repo. Emit the design-spec'd actionable message naming - // the repo so the user knows which one to `git fetch origin` in. + + // Branch may already exist from a prior partial attempt. + let branchExists = false; + try { + await runGit(["rev-parse", "--verify", branchName], { cwd: repoSrc }); + branchExists = true; + } catch { /* not present */ } + + // Exact per-repo starts are authoritative and identify a fresh coordinated + // worker allocation. Reusing an existing branch could silently attach a + // stale commit and later delete a branch this attempt did not create. Reject + // it before attaching a worktree or changing its upstream; the outer rollback + // removes only earlier components created by this attempt. + if (exactStart !== undefined && branchExists) { throw new Error( - `Failed to create worktree: base_ref '${configuredBaseRefTrimmed}' no longer exists in repo '${repo}'. ` + - `It may have been deleted on the remote since the project was configured. ` + - `Run 'git fetch origin' to refresh, then update the base_ref setting if the branch was renamed.`, + `createWorktreeSet: validate exact start failed for component "${repo}": branch "${branchName}" already exists`, ); } - throw new Error(`createWorktreeSet: git worktree add failed for repo "${repo}" at ${wtPath}: ${err instanceof Error ? err.message : err}`); - } - // When the project has a configured `base_ref`, set it as the per-branch - // upstream so each component's `@{u}` reflects the integration target. - // Single-repo path delegates to `createWorktree` above, which handles - // this; the multi-repo loop must do it explicitly per worktree. - if (configuredBaseRefTrimmed) { try { - await runGit(["branch", `--set-upstream-to=${configuredBaseRefTrimmed}`, branchName], { - cwd: wtPath, - timeout: 10_000, - }); + if (branchExists) { + await runGit(["worktree", "add", wtPath, branchName], { cwd: repoSrc }); + } else { + await runGit(["worktree", "add", "-b", branchName, wtPath, startPoint], { cwd: repoSrc }); + } } catch (err) { - const stderr = err instanceof Error ? err.message : String(err); - throw new Error( - `Failed to set upstream for branch '${branchName}' to '${configuredBaseRefTrimmed}' in repo '${repo}': ${stderr}. ` + - `Check that the ref is still a valid branch.`, - ); + if (startPointFromConfiguredBase && !branchExists) { + throw new Error( + `Failed to create worktree for component "${repo}": base_ref '${configuredBaseRefTrimmed}' no longer exists. ` + + `It may have been deleted on the remote since the project was configured. ` + + `Run 'git fetch origin' to refresh, then update the base_ref setting if the branch was renamed.`, + ); + } + throw new Error(`createWorktreeSet: git worktree add failed for component "${repo}" at ${wtPath}: ${err instanceof Error ? err.message : err}`); } - } - out.push({ repo, repoPath: repoSrc, worktreePath: wtPath }); + // Only successfully-created worktrees are eligible for rollback. Recording + // the entry after `git worktree add` prevents cleanup from probing the + // failed component path while still covering later upstream setup failures. + out.push({ repo, repoPath: repoSrc, worktreePath: wtPath }); + if (!branchExists) createdBranchRepos.add(repo); + + // When the project has a configured `base_ref`, set it as the per-branch + // upstream so each component's `@{u}` reflects the integration target. + if (configuredBaseRefTrimmed) { + try { + await runGit(["branch", `--set-upstream-to=${configuredBaseRefTrimmed}`, branchName], { + cwd: wtPath, + timeout: 10_000, + }); + } catch (err) { + const stderr = err instanceof Error ? err.message : String(err); + throw new Error( + `Failed to set upstream for branch '${branchName}' to '${configuredBaseRefTrimmed}' in component "${repo}": ${stderr}. ` + + `Check that the ref is still a valid branch.`, + ); + } + } + + } + } catch (err) { + const rollbackFailures = await rollbackWorktreeSet(out, createdBranchRepos, container, branchName, commandRunner, remotePolicy); + if (rollbackFailures.length > 0) { + throw new Error(`${err instanceof Error ? err.message : String(err)}; rollback failures: ${rollbackFailures.join("; ")}`); + } + throw err; } - return { container, worktrees: out }; + return { container, worktrees: out, createdBranchRepos }; } /** diff --git a/tests2/core/test-map-execution.test.ts b/tests2/core/test-map-execution.test.ts index d818e87b6..90fcfab49 100644 --- a/tests2/core/test-map-execution.test.ts +++ b/tests2/core/test-map-execution.test.ts @@ -27,6 +27,7 @@ const MATERIALIZED_PATHS = [ "tests2/core/marketplace-install.test.ts", "tests2/core/orphan-tool-result-rehydration-boundaries.test.ts", "tests2/core/team-manager.test.ts", + "tests2/integration/team-spawn-multi-repo-real-git.test.ts", ] as const; let root: string; let mapPath: string; @@ -87,6 +88,7 @@ describe("tests-map execution metadata", () => { "tests2/core/marketplace-install.test.ts", "tests2/core/orphan-tool-result-rehydration-boundaries.test.ts", "tests2/core/team-manager.test.ts", + "tests2/integration/team-spawn-multi-repo-real-git.test.ts", ]); }); @@ -98,6 +100,7 @@ describe("tests-map execution metadata", () => { ["tests2/core/git-lifecycle-no-publication-real-git.test.ts", { runner: "vitest", tier: "unit", project: "core" }], ["tests2/core/orphan-tool-result-rehydration-boundaries.test.ts", { runner: "vitest", tier: "unit", project: "core" }], ["tests2/core/team-manager.test.ts", { runner: "vitest", tier: "unit", project: "core" }], + ["tests2/integration/team-spawn-multi-repo-real-git.test.ts", { runner: "vitest", tier: "unit", project: "integration" }], ])("rejects cross-tagging %s", (path, execution) => { const { root, mapPath, map } = makeFixture(); record(map, path).execution = execution; diff --git a/tests2/core/unit-lanes-scheduling.test.ts b/tests2/core/unit-lanes-scheduling.test.ts index 8442b0224..c2344581a 100644 --- a/tests2/core/unit-lanes-scheduling.test.ts +++ b/tests2/core/unit-lanes-scheduling.test.ts @@ -264,6 +264,7 @@ describe("direct unit-stage scheduling", () => { "tests2/core/marketplace-install.test.ts", "tests2/core/orphan-tool-result-rehydration-boundaries.test.ts", "tests2/core/team-manager.test.ts", + "tests2/integration/team-spawn-multi-repo-real-git.test.ts", ], setupFiles: undefined, }, diff --git a/tests2/integration/team-spawn-multi-repo-real-git.test.ts b/tests2/integration/team-spawn-multi-repo-real-git.test.ts new file mode 100644 index 000000000..7122b0ca6 --- /dev/null +++ b/tests2/integration/team-spawn-multi-repo-real-git.test.ts @@ -0,0 +1,657 @@ +import { existsSync, mkdtempSync, rmSync } from "node:fs"; +import { dirname, join, resolve } from "node:path"; + +import type { CommandRunner } from "../../src/server/gateway-deps.js"; +import { pollUntil } from "../../tests/e2e/test-utils/cleanup.js"; +import { copyGitTemplate, prepareGitTemplate } from "../harness/git-template.js"; +import { test, expect } from "./_e2e/in-process-harness.js"; +import { apiFetch, deleteGoal, registerProject, teardownTeam } from "./_e2e/e2e-setup.js"; + +const REPRO = "MULTI_REPO_TEAM_SPAWN_REGRESSION"; +const NESTED_COMPONENT = "packages/alpha" as const; +const FAILED_COMPONENT = "beta" as const; +const COMPONENTS = [NESTED_COMPONENT, FAILED_COMPONENT] as const; +type ComponentName = typeof COMPONENTS[number]; + +async function git(runner: CommandRunner, cwd: string, args: string[]): Promise { + const result = await runner.execFile("git", args, { cwd, encoding: "utf-8", timeout: 10_000 }); + return String(result.stdout).trim(); +} + +async function gitRefExists(runner: CommandRunner, cwd: string, ref: string): Promise { + try { + await git(runner, cwd, ["show-ref", "--verify", "--quiet", ref]); + return true; + } catch { + return false; + } +} + +async function readJson(response: Response): Promise { + const text = await response.text(); + try { + return text ? JSON.parse(text) : {}; + } catch { + return { raw: text }; + } +} + +async function waitForGoalReady(goalId: string): Promise { + return pollUntil( + async () => { + const response = await apiFetch(`/api/goals/${goalId}`); + const body = await readJson(response); + return body.setupStatus === "ready" || body.setupStatus === "error" ? body : null; + }, + { timeoutMs: 30_000, intervalMs: 50, label: "multi-repo goal provisioning" }, + ); +} + +function normalized(filePath: string): string { + const absolute = resolve(filePath); + return process.platform === "win32" ? absolute.toLowerCase() : absolute; +} + +function liveRepoWorktrees(session: any): Record { + return Object.fromEntries( + (session?.repoWorktrees ?? []).map((entry: { repo: string; worktreePath: string }) => [ + entry.repo, + entry.worktreePath, + ]), + ); +} + +function assertWorkerShape(opts: { + gateway: any; + runner: CommandRunner; + projectRoot: string; + goalId: string; + goalHeads: Record; + sessionId: string; + worktreePath: string; +}): Record { + const { gateway, runner, projectRoot, goalId, goalHeads, sessionId, worktreePath } = opts; + const session = gateway.sessionManager.getSession(sessionId); + const agent = gateway.teamManager.findAgentBySessionId(sessionId); + const repoWorktrees = liveRepoWorktrees(session) as Record; + + expect(session, `${REPRO}: spawned worker session must exist`).toBeTruthy(); + expect(agent?.branch, `${REPRO}: TeamManager must retain the worker branch`).toMatch( + new RegExp(`^goal/${goalId.slice(0, 8)}/coder-[0-9a-f]{4}$`), + ); + expect(normalized(session.cwd), "worker cwd must be its non-Git branch container").toBe(normalized(worktreePath)); + expect(normalized(session.worktreePath), "flat worker worktreePath must be the branch container").toBe(normalized(worktreePath)); + expect(normalized(session.repoPath), "worker cleanup must retain the non-Git project root as its repo container").toBe(normalized(projectRoot)); + expect(existsSync(join(worktreePath, ".git")), "worker branch container must remain non-Git").toBe(false); + expect(Object.keys(repoWorktrees).sort()).toEqual([...COMPONENTS].sort()); + expect(new Set(COMPONENTS.map(repo => normalized(resolve( + repoWorktrees[repo], + ...repo.split("/").map(() => ".."), + ))))).toEqual(new Set([normalized(worktreePath)])); + + for (const repo of COMPONENTS) { + expect(normalized(repoWorktrees[repo]), `${repo} must retain the configured repo-key layout`).toBe( + normalized(join(worktreePath, repo)), + ); + expect(existsSync(repoWorktrees[repo]), `${repo} worker worktree must exist`).toBe(true); + } + + // Keep this helper synchronous for structural assertions; callers perform the + // real Git HEAD/branch assertions with the same canonical runner. + void runner; + void goalHeads; + return repoWorktrees; +} + +// Real-Git fidelity owner. The project root is deliberately not a repository; +// GoalManager provisions one goal worktree per configured component first, then +// TeamManager must use those persisted component paths as authoritative starts. +test("direct and REST team spawn preserve exact local component HEADs across collisions, rollback, and cleanup", async ({ gateway }) => { + await prepareGitTemplate(); + const fixtureRoot = mkdtempSync(join(gateway.bobbitDir, "team-multi-minimal-")); + const projectRoot = join(fixtureRoot, "project"); + const worktreeRoot = join(fixtureRoot, "worktrees"); + const runner = gateway.sessionManager.commandRunner as CommandRunner; + let projectId: string | undefined; + let goalId: string | undefined; + let teamLeadSessionId: string | undefined; + let collisionWorkerId: string | undefined; + let postSpawnFailedSessionId: string | undefined; + let directWorkerId: string | undefined; + let restWorkerId: string | undefined; + let originalExecFile: CommandRunner["execFile"] | undefined; + let originalTryAutoSelectModel: any; + let originalUpdateSessionMeta: any; + let originalSessionStorePut: any; + let interceptedSessionStore: any; + + try { + for (const repo of COMPONENTS) copyGitTemplate(join(projectRoot, repo)); + expect(existsSync(join(projectRoot, ".git")), "fixture root must be a non-Git container").toBe(false); + for (const repo of COMPONENTS) { + expect(await git(runner, join(projectRoot, repo), ["rev-parse", "HEAD"])).toMatch(/^[0-9a-f]{40}$/); + } + + const project = await registerProject({ + name: `team-multi-minimal-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`, + rootPath: projectRoot, + components: COMPONENTS.map(repo => ({ name: repo.split("/").at(-1)!, repo })), + workflows: { + general: { + name: "General", + description: "Focused multi-repo team-spawn regression", + gates: [{ id: "implementation", name: "Implementation", depends_on: [] }], + }, + }, + seedWorkflows: false, + }); + projectId = project.id; + + const configResponse = await apiFetch(`/api/projects/${projectId}/config`, { + method: "PUT", + body: JSON.stringify({ worktree_root: worktreeRoot }), + }); + expect(configResponse.status, `${REPRO}: fixture worktree_root configuration must succeed`).toBe(200); + + const goalResponse = await apiFetch("/api/goals", { + method: "POST", + body: JSON.stringify({ + projectId, + title: "Minimal coordinated worker spawn", + spec: "Spawn workers from unpublished alpha and beta goal commits.", + workflowId: "general", + cwd: projectRoot, + worktree: true, + team: true, + autoStartTeam: false, + }), + }); + const createdGoal = await readJson(goalResponse); + expect(goalResponse.status, `${REPRO}: goal creation failed: ${JSON.stringify(createdGoal)}`).toBe(201); + goalId = createdGoal.id; + + const goal = await waitForGoalReady(goalId!); + expect(goal.setupStatus, `${REPRO}: goal setup failed: ${goal.setupError ?? "unknown"}`).toBe("ready"); + expect(normalized(goal.repoPath)).toBe(normalized(projectRoot)); + expect(Object.keys(goal.repoWorktrees ?? {}).sort()).toEqual([...COMPONENTS].sort()); + expect(existsSync(join(goal.worktreePath, ".git")), "goal branch container must remain non-Git").toBe(false); + + const goalHeads = {} as Record; + for (const repo of COMPONENTS) { + const goalWorktree = goal.repoWorktrees[repo]; + expect(existsSync(goalWorktree), `${repo} goal worktree must exist`).toBe(true); + await git(runner, goalWorktree, ["commit", "--quiet", "--allow-empty", "-m", `Unpublished ${repo} goal head`]); + goalHeads[repo] = await git(runner, goalWorktree, ["rev-parse", "HEAD"]); + expect(await git(runner, join(projectRoot, repo), ["remote"]), `${repo} goal commit must be unpublished`).toBe(""); + } + expect(goalHeads[NESTED_COMPONENT]).not.toBe(goalHeads[FAILED_COMPONENT]); + + const startResponse = await apiFetch(`/api/goals/${goalId}/team/start`, { method: "POST" }); + const startBody = await readJson(startResponse); + expect(startResponse.status, `${REPRO}: team lead start failed: ${JSON.stringify(startBody)}`).toBe(201); + teamLeadSessionId = startBody.sessionId; + + // Inject failure only after the nested component's real worktree add has + // completed. The rejected direct spawn must remove that worktree, its + // intermediate repo-key directory, branch, and container. It must never + // attempt cleanup against beta's failed, never-created target. + originalExecFile = runner.execFile; + let failedBranch: string | undefined; + let failedContainer: string | undefined; + let firstComponentWorktree: string | undefined; + let failedComponentWorktree: string | undefined; + const rollbackWorktreeRemoveTargets: string[] = []; + (runner as any).execFile = async (file: string, args: string[], options: any) => { + const gitCommand = file.toLowerCase().replace(/\.exe$/, "") === "git"; + if (gitCommand && args[0] === "worktree" && args[1] === "remove") { + rollbackWorktreeRemoveTargets.push(String(args[2])); + } + const branchIndex = args.indexOf("-b"); + const branch = branchIndex >= 0 ? args[branchIndex + 1] : undefined; + const target = branchIndex >= 0 ? args[branchIndex + 2] : args[2]; + const workerAdd = gitCommand + && args[0] === "worktree" + && args[1] === "add" + && branch?.startsWith(`goal/${goalId!.slice(0, 8)}/coder-`); + if (workerAdd && normalized(String(options?.cwd ?? "")) === normalized(join(projectRoot, NESTED_COMPONENT))) { + failedBranch = branch; + firstComponentWorktree = target; + failedContainer = dirname(dirname(target)); + } + if (workerAdd && normalized(String(options?.cwd ?? "")) === normalized(join(projectRoot, FAILED_COMPONENT))) { + failedComponentWorktree = target; + const error = new Error("injected beta git worktree add failure"); + (error as any).stderr = "injected beta git worktree add failure"; + throw error; + } + return originalExecFile!.call(runner, file, args, options); + }; + + let injectedFailure: unknown; + try { + await gateway.teamManager.spawnRole(goalId!, "coder", "Prove partial multi-repo rollback"); + } catch (error) { + injectedFailure = error; + } finally { + (runner as any).execFile = originalExecFile; + originalExecFile = undefined; + } + const failureMessage = injectedFailure instanceof Error ? injectedFailure.message : String(injectedFailure ?? "spawn succeeded"); + if (!/beta.*(?:git )?worktree add|(?:git )?worktree add.*beta/i.test(failureMessage)) { + throw new Error(`${REPRO}: expected beta git worktree add failure, received: ${failureMessage}`); + } + expect(failedBranch, "injection must run after the first component worktree add").toBeTruthy(); + expect(firstComponentWorktree, "nested first-component target must be observed").toBeTruthy(); + expect(failedComponentWorktree, "failed second-component target must be observed").toBeTruthy(); + expect(firstComponentWorktree && existsSync(firstComponentWorktree), "nested first-component worktree must be rolled back").toBe(false); + expect(firstComponentWorktree && existsSync(dirname(firstComponentWorktree)), "nested repo-key intermediate directory must be rolled back").toBe(false); + expect(failedContainer && existsSync(failedContainer), "empty worker branch container must be rolled back").toBe(false); + expect(failedComponentWorktree && existsSync(failedComponentWorktree), "failed second-component target must never be created").toBe(false); + expect(rollbackWorktreeRemoveTargets.map(normalized)).toContain(normalized(firstComponentWorktree!)); + expect( + rollbackWorktreeRemoveTargets.map(normalized), + "rollback must not invoke git worktree remove for the failed, never-created component", + ).not.toContain(normalized(failedComponentWorktree!)); + expect( + await gitRefExists(runner, join(projectRoot, NESTED_COMPONENT), `refs/heads/${failedBranch}`), + "nested first-component branch must be rolled back", + ).toBe(false); + for (const repo of COMPONENTS) { + expect( + await git(runner, goal.repoWorktrees[repo], ["rev-parse", "HEAD"]), + `${repo} goal component must remain usable after worker rollback`, + ).toBe(goalHeads[repo]); + } + + // Pin the exact-start collision boundary after one earlier component has + // already been created. The second component owns a worker-named branch at + // its source HEAD, deliberately divergent from its unpublished goal HEAD. + // Provisioning must reject before attaching or mutating that branch, roll + // back only the first component it created, and perform no remote mutation. + const projectContext = gateway.projectContextManager.getContextForGoal(goalId!); + expect(projectContext, "multi-repo goal must retain its project context").toBeTruthy(); + projectContext!.projectConfigStore.set("base_ref", "master"); + originalExecFile = runner.execFile; + let collidingBranch: string | undefined; + let collidingBranchHead: string | undefined; + let collisionContainer: string | undefined; + let firstCreatedWorktree: string | undefined; + let collidingWorktree: string | undefined; + const collisionWorktreeAddTargets: string[] = []; + const rollbackWorktreeRemoveTargetsForCollision: string[] = []; + const upstreamMutationCalls: Array<{ cwd: string; args: string[] }> = []; + const localBranchDeleteCalls: Array<{ cwd: string; branch: string }> = []; + const remoteMutationCalls: Array<{ cwd: string; args: string[] }> = []; + (runner as any).execFile = async (file: string, args: string[], options: any) => { + const gitCommand = file.toLowerCase().replace(/\.exe$/, "") === "git"; + const cwd = String(options?.cwd ?? ""); + if (gitCommand && args[0] === "worktree" && args[1] === "remove") { + rollbackWorktreeRemoveTargetsForCollision.push(String(args[2])); + } + if (gitCommand && args[0] === "branch" && args.some(arg => arg.startsWith("--set-upstream-to="))) { + upstreamMutationCalls.push({ cwd, args: [...args] }); + } + if (gitCommand && args[0] === "branch" && args[1] === "-D") { + localBranchDeleteCalls.push({ cwd, branch: String(args[2]) }); + } + if (gitCommand && args[0] === "push") { + remoteMutationCalls.push({ cwd, args: [...args] }); + } + + if (gitCommand && args[0] === "worktree" && args[1] === "add") { + const branchIndex = args.indexOf("-b"); + const branch = String(branchIndex >= 0 ? args[branchIndex + 1] : args[3]); + const target = String(branchIndex >= 0 ? args[branchIndex + 2] : args[2]); + const workerAdd = branch.startsWith(`goal/${goalId!.slice(0, 8)}/coder-`); + if (workerAdd && normalized(cwd) === normalized(join(projectRoot, NESTED_COMPONENT))) { + firstCreatedWorktree = target; + collisionContainer = resolve( + target, + ...NESTED_COMPONENT.split("/").map(() => ".."), + ); + if (!collidingBranch) { + collidingBranch = branch; + const result = await originalExecFile!.call(runner, "git", ["rev-parse", "HEAD"], { + cwd: join(projectRoot, FAILED_COMPONENT), + encoding: "utf-8", + timeout: 10_000, + }); + collidingBranchHead = String(result.stdout).trim(); + await originalExecFile!.call(runner, "git", ["branch", collidingBranch, collidingBranchHead], { + cwd: join(projectRoot, FAILED_COMPONENT), + encoding: "utf-8", + timeout: 10_000, + }); + } + } + if (workerAdd && normalized(cwd) === normalized(join(projectRoot, FAILED_COMPONENT))) { + collidingWorktree = target; + collisionWorktreeAddTargets.push(target); + } + } + return originalExecFile!.call(runner, file, args, options); + }; + + let collisionFailure: unknown; + try { + const result = await gateway.teamManager.spawnRole(goalId!, "coder", "Reject a divergent exact-start branch collision"); + collisionWorkerId = result.sessionId; + } catch (error) { + collisionFailure = error; + } finally { + (runner as any).execFile = originalExecFile; + originalExecFile = undefined; + } + const collisionFailureMessage = collisionFailure instanceof Error + ? collisionFailure.message + : String(collisionFailure ?? "spawn succeeded"); + if ( + !/component\s+["']?beta["']?/i.test(collisionFailureMessage) + || !/(?:exact start|does not match|mismatch|collision|differs)/i.test(collisionFailureMessage) + ) { + throw new Error(`${REPRO}: expected beta exact-start branch collision rejection, received: ${collisionFailureMessage}`); + } + expect(collidingBranch, "fixture must create beta's colliding worker branch after alpha starts").toBeTruthy(); + expect(collidingBranchHead, "fixture must retain the divergent beta branch HEAD").toBeTruthy(); + expect(collidingBranchHead, "beta collision must differ from its authoritative unpublished goal HEAD").not.toBe(goalHeads[FAILED_COMPONENT]); + expect(firstCreatedWorktree, "collision must be detected only after the first component was created").toBeTruthy(); + expect(collisionContainer, "first component must reveal the worker branch container").toBeTruthy(); + const expectedCollidingWorktree = join(collisionContainer!, FAILED_COMPONENT); + expect(firstCreatedWorktree && existsSync(firstCreatedWorktree), "earlier nested component worktree must be rolled back").toBe(false); + expect(firstCreatedWorktree && existsSync(dirname(firstCreatedWorktree)), "earlier nested repo-key directory must be rolled back").toBe(false); + expect(existsSync(collisionContainer!), "empty collision-attempt container must be rolled back").toBe(false); + expect(rollbackWorktreeRemoveTargetsForCollision.map(normalized)).toContain(normalized(firstCreatedWorktree!)); + expect(collisionWorktreeAddTargets, "beta collision must be rejected before git worktree add can attach it").toEqual([]); + expect(collidingWorktree, "beta collision must never produce a worker worktree target").toBeUndefined(); + expect(existsSync(expectedCollidingWorktree), "beta collision path must never be created").toBe(false); + expect( + await git(runner, join(projectRoot, FAILED_COMPONENT), ["rev-parse", `refs/heads/${collidingBranch}`]), + "collision rejection must preserve beta's divergent local branch HEAD exactly", + ).toBe(collidingBranchHead); + expect( + await git(runner, join(projectRoot, FAILED_COMPONENT), ["worktree", "list", "--porcelain"]), + "collision rejection must not leave beta's branch attached to any worktree", + ).not.toContain(`branch refs/heads/${collidingBranch}`); + expect( + await git(runner, join(projectRoot, FAILED_COMPONENT), ["for-each-ref", "--format=%(upstream)", `refs/heads/${collidingBranch}`]), + "collision rejection must not assign an upstream to beta's pre-existing branch", + ).toBe(""); + expect( + upstreamMutationCalls.some(call => normalized(call.cwd) === normalized(expectedCollidingWorktree)), + "collision rejection must occur before beta's upstream mutation step", + ).toBe(false); + expect( + localBranchDeleteCalls.some(call => normalized(call.cwd) === normalized(join(projectRoot, FAILED_COMPONENT)) && call.branch === collidingBranch), + "rollback must not ask Git to delete beta's pre-existing colliding branch", + ).toBe(false); + expect(remoteMutationCalls, "collision rejection and rollback must not mutate any remote").toEqual([]); + expect( + await gitRefExists(runner, join(projectRoot, NESTED_COMPONENT), `refs/heads/${collidingBranch}`), + "rollback must delete the earlier alpha branch created by this attempt", + ).toBe(false); + for (const repo of COMPONENTS) { + expect( + await git(runner, goal.repoWorktrees[repo], ["rev-parse", "HEAD"]), + `${repo} goal component must remain at its authoritative HEAD after collision rollback`, + ).toBe(goalHeads[repo]); + } + await git(runner, join(projectRoot, FAILED_COMPONENT), ["branch", "-D", collidingBranch!]); + + // Drive the real createSession pipeline through agent spawn, then fail its + // awaited post-spawn model setup. SessionManager must not treat either + // non-Git container as a single-repo cleanup owner; TeamManager retains sole + // ownership of the already-provisioned component worktrees and container. + originalExecFile = runner.execFile; + originalTryAutoSelectModel = (gateway.sessionManager as any).tryAutoSelectModel; + const postSpawnGitCalls: Array<{ cwd: string; args: string[] }> = []; + const postSpawnWorktrees: Partial> = {}; + let postSpawnBranch: string | undefined; + let postSpawnContainer: string | undefined; + let postSpawnAgentState: any; + let postSpawnSessionShape: any; + let postSpawnSessionWasLive = false; + let componentsExistedDuringPostSpawn: Partial> = {}; + (runner as any).execFile = async (file: string, args: string[], options: any) => { + const gitCommand = file.toLowerCase().replace(/\.exe$/, "") === "git"; + const cwd = String(options?.cwd ?? ""); + if (gitCommand) postSpawnGitCalls.push({ cwd, args: [...args] }); + const result = await originalExecFile!.call(runner, file, args, options); + if (gitCommand && args[0] === "worktree" && args[1] === "add") { + const branchIndex = args.indexOf("-b"); + const branch = String(branchIndex >= 0 ? args[branchIndex + 1] : args[3]); + const target = String(branchIndex >= 0 ? args[branchIndex + 2] : args[2]); + if (branch.startsWith(`goal/${goalId!.slice(0, 8)}/coder-`)) { + postSpawnBranch = branch; + for (const repo of COMPONENTS) { + if (normalized(cwd) === normalized(join(projectRoot, repo))) { + postSpawnWorktrees[repo] = target; + } + } + if (normalized(cwd) === normalized(join(projectRoot, NESTED_COMPONENT))) { + postSpawnContainer = resolve( + target, + ...NESTED_COMPONENT.split("/").map(() => ".."), + ); + } + } + } + return result; + }; + (gateway.sessionManager as any).tryAutoSelectModel = async (session: any) => { + if (session.goalId === goalId && session.role === "coder") { + postSpawnFailedSessionId = session.id; + postSpawnSessionWasLive = gateway.sessionManager.getSession(session.id) === session; + postSpawnSessionShape = { + cwd: session.cwd, + worktreePath: session.worktreePath, + repoPath: session.repoPath, + branch: session.branch, + repoWorktrees: liveRepoWorktrees(session), + }; + componentsExistedDuringPostSpawn = Object.fromEntries( + COMPONENTS.map(repo => [repo, existsSync(postSpawnSessionShape.repoWorktrees[repo])]), + ); + postSpawnAgentState = await session.rpcClient.getState(); + throw new Error(`${REPRO}_POST_SPAWN_SETUP_FAILURE`); + } + return originalTryAutoSelectModel.call(gateway.sessionManager, session); + }; + + let postSpawnFailure: unknown; + try { + await gateway.teamManager.spawnRole(goalId!, "coder", "Fail only after the real worker agent spawns"); + } catch (error) { + postSpawnFailure = error; + } finally { + (runner as any).execFile = originalExecFile; + originalExecFile = undefined; + (gateway.sessionManager as any).tryAutoSelectModel = originalTryAutoSelectModel; + originalTryAutoSelectModel = undefined; + } + const postSpawnFailureMessage = postSpawnFailure instanceof Error + ? postSpawnFailure.message + : String(postSpawnFailure ?? "spawn succeeded"); + if (!postSpawnFailureMessage.includes(`${REPRO}_POST_SPAWN_SETUP_FAILURE`)) { + throw new Error(`${REPRO}: expected actual post-spawn setup failure, received: ${postSpawnFailureMessage}`); + } + expect(postSpawnFailedSessionId, "post-spawn injection must observe the real worker session").toBeTruthy(); + expect(postSpawnSessionWasLive, "worker must enter SessionManager's live map before post-spawn setup").toBe(true); + expect(postSpawnAgentState?.success, "worker RPC must answer get_state after agent spawn").toBe(true); + expect(postSpawnContainer, "real component provisioning must create one worker branch container").toBeTruthy(); + expect(postSpawnBranch, "real component provisioning must use a worker branch").toBeTruthy(); + expect(normalized(postSpawnSessionShape?.cwd), "worker cwd must be the pre-provisioned branch container").toBe(normalized(postSpawnContainer!)); + expect(normalized(postSpawnSessionShape?.worktreePath), "flat setup worktreePath must remain the branch container").toBe(normalized(postSpawnContainer!)); + expect(normalized(postSpawnSessionShape?.repoPath), "flat setup repoPath must remain the non-Git project root").toBe(normalized(projectRoot)); + expect(postSpawnSessionShape?.branch).toBe(postSpawnBranch); + expect(Object.keys(postSpawnSessionShape?.repoWorktrees ?? {}).sort()).toEqual([...COMPONENTS].sort()); + for (const repo of COMPONENTS) { + expect(postSpawnWorktrees[repo], `${repo} must be provisioned before createSession post-spawn setup`).toBeTruthy(); + expect(postSpawnSessionShape?.repoWorktrees?.[repo], `${repo} cleanup coordinate must reach the live session`).toBe(postSpawnWorktrees[repo]); + expect(componentsExistedDuringPostSpawn[repo], `${repo} must exist when post-spawn setup fails`).toBe(true); + } + expect(existsSync(join(projectRoot, ".git")), "post-spawn failure must leave the project root non-Git").toBe(false); + expect(existsSync(join(postSpawnContainer!, ".git")), "post-spawn failure container must never become a Git repository").toBe(false); + + const invalidContainerGitCalls = postSpawnGitCalls.filter(call => { + const cwd = normalized(call.cwd); + return cwd === normalized(projectRoot) || cwd === normalized(postSpawnContainer!); + }); + if (invalidContainerGitCalls.length > 0) { + throw new Error( + `${REPRO}_POST_SPAWN_NON_GIT_CLEANUP: session setup invoked Git against a non-Git root/container: ` + + invalidContainerGitCalls.map(call => `${call.cwd} :: git ${call.args.join(" ")}`).join("; "), + ); + } + const worktreeRemoveCalls = postSpawnGitCalls.filter(call => call.args[0] === "worktree" && call.args[1] === "remove"); + for (const repo of COMPONENTS) { + const componentCleanupCalls = worktreeRemoveCalls.filter(call => + normalized(call.cwd) === normalized(join(projectRoot, repo)) + && normalized(String(call.args[2])) === normalized(postSpawnWorktrees[repo]!), + ); + expect( + componentCleanupCalls, + `${repo}: TeamManager must issue exactly one component-owned cleanup with component-specific diagnostics`, + ).toHaveLength(1); + expect(existsSync(postSpawnWorktrees[repo]!), `${repo}: TeamManager must remove the created component worktree`).toBe(false); + expect( + await gitRefExists(runner, join(projectRoot, repo), `refs/heads/${postSpawnBranch}`), + `${repo}: TeamManager must remove the branch created by this failed attempt`, + ).toBe(false); + expect(await git(runner, goal.repoWorktrees[repo], ["rev-parse", "HEAD"]), `${repo} goal HEAD must survive post-spawn rollback`).toBe(goalHeads[repo]); + } + expect(existsSync(dirname(postSpawnWorktrees[NESTED_COMPONENT]!)), "post-spawn rollback must remove the nested repo-key directory").toBe(false); + expect(existsSync(postSpawnContainer!), "TeamManager must remove the empty worker branch container").toBe(false); + expect(gateway.sessionManager.getSession(postSpawnFailedSessionId!), "setup-failed worker must leave the live session map").toBeUndefined(); + + // Direct production call: this is the primary host-side regression boundary. + // Observe the project SessionStore boundary so the first durable worker + // record can be distinguished from TeamManager's later metadata update. + const sessionWriteEvents: Array<{ kind: "put" | "update"; id: string; value: any }> = []; + interceptedSessionStore = gateway.projectContextManager.getContextForGoal(goalId!)!.sessionStore; + originalSessionStorePut = interceptedSessionStore.put; + originalUpdateSessionMeta = (gateway.sessionManager as any).updateSessionMeta; + interceptedSessionStore.put = (persisted: any) => { + if (persisted.goalId === goalId && persisted.role === "coder") { + sessionWriteEvents.push({ + kind: "put", + id: persisted.id, + value: { + ...persisted, + repoWorktrees: persisted.repoWorktrees ? { ...persisted.repoWorktrees } : undefined, + }, + }); + } + return originalSessionStorePut.call(interceptedSessionStore, persisted); + }; + (gateway.sessionManager as any).updateSessionMeta = (id: string, updates: any) => { + if (updates?.role === "coder" && updates?.teamGoalId === goalId) { + sessionWriteEvents.push({ + kind: "update", + id, + value: { + ...updates, + repoWorktrees: updates.repoWorktrees ? { ...updates.repoWorktrees } : undefined, + }, + }); + } + return originalUpdateSessionMeta.call(gateway.sessionManager, id, updates); + }; + + let directResult: Awaited>; + try { + directResult = await gateway.teamManager.spawnRole(goalId!, "coder", "Direct coordinated spawn"); + } finally { + interceptedSessionStore.put = originalSessionStorePut; + originalSessionStorePut = undefined; + (gateway.sessionManager as any).updateSessionMeta = originalUpdateSessionMeta; + originalUpdateSessionMeta = undefined; + interceptedSessionStore = undefined; + } + directWorkerId = directResult.sessionId; + expect(directResult.worktreePath).toBeTruthy(); + const directPaths = assertWorkerShape({ + gateway, + runner, + projectRoot, + goalId: goalId!, + goalHeads, + sessionId: directWorkerId!, + worktreePath: directResult.worktreePath!, + }); + const directBranch = gateway.teamManager.findAgentBySessionId(directWorkerId!)!.branch!; + const firstDurableWriteIndex = sessionWriteEvents.findIndex(event => event.kind === "put" && event.id === directWorkerId); + const laterTeamMetaIndex = sessionWriteEvents.findIndex(event => event.kind === "update" && event.id === directWorkerId); + expect(firstDurableWriteIndex, "createSession must make a durable worker write").toBeGreaterThanOrEqual(0); + expect(laterTeamMetaIndex, "TeamManager must retain its later worker metadata update").toBeGreaterThan(firstDurableWriteIndex); + const firstDurableWorker = firstDurableWriteIndex >= 0 ? sessionWriteEvents[firstDurableWriteIndex].value : {}; + expect.soft(firstDurableWorker.worktreePath, "first durable write must already own the branch container").toBe(directResult.worktreePath!); + expect.soft(firstDurableWorker.repoPath, "first durable write must already own the non-Git repo container").toBe(projectRoot); + expect.soft(firstDurableWorker.branch, "first durable write must already own the worker branch").toBe(directBranch); + expect.soft(Object.keys(firstDurableWorker.repoWorktrees ?? {}).sort(), "first durable write must already own both component cleanup paths").toEqual([...COMPONENTS].sort()); + for (const repo of COMPONENTS) { + expect.soft(firstDurableWorker.repoWorktrees?.[repo], `${repo} cleanup coordinate must exist in the first durable write`).toBe(directPaths[repo]); + expect(await git(runner, directPaths[repo], ["rev-parse", "HEAD"]), `${repo} must start at its exact local goal HEAD`).toBe(goalHeads[repo]); + expect(await git(runner, directPaths[repo], ["branch", "--show-current"]), `${repo} must use the common worker branch`).toBe(directBranch); + } + expect(existsSync(join(projectRoot, ".git")), "worker spawn must not make the project container a repository").toBe(false); + + // Ordinary dismissal + purge must consume the worker's known component map, + // not probe the non-Git branch container as though it were a repository. + const dismissed = await gateway.teamManager.dismissRoleForGoal(goalId!, directWorkerId!); + expect(dismissed.status).toBe("dismissed"); + const purgeResponse = await apiFetch(`/api/sessions/${directWorkerId}?purge=true`, { method: "DELETE" }); + expect(purgeResponse.status, `${REPRO}: direct worker purge must succeed`).toBe(200); + await pollUntil( + async () => COMPONENTS.every(repo => !existsSync(directPaths[repo])) && !existsSync(directResult.worktreePath!) ? true : null, + { timeoutMs: 15_000, intervalMs: 50, label: "direct worker component purge" }, + ); + expect( + existsSync(dirname(directPaths[NESTED_COMPONENT])), + "ordinary purge must remove the nested repo-key intermediate directory", + ).toBe(false); + directWorkerId = undefined; + + // Exactly one REST team/spawn request proves the HTTP route reaches the same + // fixed TeamManager path and returns the worker branch container. + const spawnResponse = await apiFetch(`/api/goals/${goalId}/team/spawn`, { + method: "POST", + body: JSON.stringify({ role: "coder", task: "REST coordinated spawn" }), + }); + const spawnBody = await readJson(spawnResponse); + expect( + spawnResponse.status, + `${REPRO}: POST team/spawn must accept a non-Git multi-repo container: ${JSON.stringify(spawnBody)}`, + ).toBe(201); + restWorkerId = spawnBody.sessionId; + expect(restWorkerId).toBeTruthy(); + expect(spawnBody.worktreePath).toBeTruthy(); + assertWorkerShape({ + gateway, + runner, + projectRoot, + goalId: goalId!, + goalHeads, + sessionId: restWorkerId!, + worktreePath: spawnBody.worktreePath, + }); + } finally { + if (originalExecFile) (runner as any).execFile = originalExecFile; + if (originalTryAutoSelectModel) (gateway.sessionManager as any).tryAutoSelectModel = originalTryAutoSelectModel; + if (originalUpdateSessionMeta) (gateway.sessionManager as any).updateSessionMeta = originalUpdateSessionMeta; + if (originalSessionStorePut && interceptedSessionStore) interceptedSessionStore.put = originalSessionStorePut; + for (const workerId of [collisionWorkerId, directWorkerId, restWorkerId]) { + if (!workerId || !goalId) continue; + await gateway.teamManager.dismissRoleForGoal(goalId, workerId).catch(() => undefined); + await apiFetch(`/api/sessions/${workerId}?purge=true`, { method: "DELETE" }).catch(() => undefined); + } + if (goalId) await teardownTeam(goalId).catch(() => undefined); + if (teamLeadSessionId) await apiFetch(`/api/sessions/${teamLeadSessionId}?purge=true`, { method: "DELETE" }).catch(() => undefined); + // The failed session retains worker cleanup coordinates for diagnostics. Purge + // it only after GoalManager has removed the goal component worktrees, because + // both nested worktree basenames are intentionally identical in this fixture. + if (goalId) await deleteGoal(goalId).catch(() => undefined); + if (postSpawnFailedSessionId) await apiFetch(`/api/sessions/${postSpawnFailedSessionId}?purge=true`, { method: "DELETE" }).catch(() => undefined); + if (projectId) await apiFetch(`/api/projects/${encodeURIComponent(projectId)}`, { method: "DELETE" }).catch(() => undefined); + rmSync(fixtureRoot, { recursive: true, force: true, maxRetries: 5, retryDelay: 50 }); + } +}); diff --git a/tests2/tests-map.json b/tests2/tests-map.json index 3be48d41c..a6c8bda7d 100644 --- a/tests2/tests-map.json +++ b/tests2/tests-map.json @@ -20,6 +20,15 @@ "vitest-e2e": 2 }, "v2Native": [ + { + "path": "tests2/integration/team-spawn-multi-repo-real-git.test.ts", + "reason": "Focused real-Git regression for a non-Git two-component project: drives direct TeamManager.spawnRole and one REST team/spawn request from unpublished divergent local goal-component HEADs; verifies exact-start branch-collision rejection before worktree attachment/upstream or remote mutation, rollback of an earlier component while preserving the divergent colliding branch, actual createSession agent-spawn then post-spawn setup failure without Git probes against either non-Git container, TeamManager-owned component/container cleanup, common branch-container layout, injected partial rollback, and cleanup coordinates in createSession's first durable write.", + "execution": { + "runner": "vitest", + "tier": "e2e", + "project": "e2e" + } + }, { "path": "tests2/core/inline-html-theme-bridge-repro.test.ts", "reason": "Fast failing-first regression proving completed srcdoc and streaming document.write inline HTML payloads are prepared through the canonical preview theme bridge in the compiled UI dependency graph.",