From c6ea5e9fcaf757222bf1560260f47cbbd185566f Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Thu, 23 Jul 2026 14:23:58 +0100 Subject: [PATCH 01/12] Support host multi-repo team workers Co-authored-by: bobbit-ai --- src/server/agent/session-manager.ts | 20 ++- src/server/agent/team-manager.ts | 179 ++++++++++++++++++++++---- src/server/agent/team-store.ts | 2 + src/server/skills/git.ts | 191 ++++++++++++++++++---------- 4 files changed, 298 insertions(+), 94 deletions(-) diff --git a/src/server/agent/session-manager.ts b/src/server/agent/session-manager.ts index 5ccf87a62..bd7f290a3 100755 --- a/src/server/agent/session-manager.ts +++ b/src/server/agent/session-manager.ts @@ -10822,8 +10822,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) { @@ -11254,8 +11255,21 @@ 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 fsp.rmdir(ps.worktreePath); + } catch (err: any) { + if (err?.code !== "ENOENT") { + 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/team-manager.ts b/src/server/agent/team-manager.ts index 7afd1e564..b1d31116e 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 } 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,40 @@ 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, + 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 fs.promises.rmdir(result.container); + } catch (err: any) { + if (err?.code !== "ENOENT") { + 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 +281,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 +615,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 +1042,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 +2107,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 +2131,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 +2143,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 +2159,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 +2237,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 { @@ -2195,6 +2303,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 +2316,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 +2338,7 @@ export class TeamManager { role, kind: "worker", worktreePath: actualWorktreePath, + repoWorktrees: workerRepoWorktrees, branch: branchName, baseSha, task, @@ -2298,8 +2414,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 +2642,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..4b2e19dbe 100644 --- a/src/server/skills/git.ts +++ b/src/server/skills/git.ts @@ -607,12 +607,56 @@ 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 WorktreeSetRollbackEntry { + repo: string; + repoPath: string; + worktreePath: string; + deleteBranch: boolean; +} + +async function rollbackWorktreeSet( + entries: WorktreeSetRollbackEntry[], + 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, + entry.deleteBranch, + 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 fs.promises.rmdir(container); + } catch (err: any) { + if (err?.code !== "ENOENT") { + failures.push(`branch container cleanup failed at ${container}: ${err instanceof Error ? err.message : String(err)}`); + } + } + return failures; +} + export interface WorktreeResult { worktreePath: string; branchName: string; @@ -811,6 +855,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 +887,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); } @@ -867,76 +918,84 @@ export async function createWorktreeSet( 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 }); - } else { - await runGit(["worktree", "add", "-b", branchName, wtPath, startPoint], { cwd: repoSrc }); + const rollbackEntries: WorktreeSetRollbackEntry[] = []; + 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}`); } - } 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. - 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.`, - ); + // 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 { + startPoint = await resolveRemotePrimary(repoSrc, commandRunner); } - 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) { + // Branch may already exist from a prior partial attempt. + let branchExists = false; try { - await runGit(["branch", `--set-upstream-to=${configuredBaseRefTrimmed}`, branchName], { - cwd: wtPath, - timeout: 10_000, - }); + await runGit(["rev-parse", "--verify", branchName], { cwd: repoSrc }); + branchExists = true; + } catch { /* not present */ } + rollbackEntries.push({ repo, repoPath: repoSrc, worktreePath: wtPath, deleteBranch: !branchExists }); + + try { + 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}`); + } + + // 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.`, + ); + } } - } - out.push({ repo, repoPath: repoSrc, worktreePath: wtPath }); + out.push({ repo, repoPath: repoSrc, worktreePath: wtPath }); + } + } catch (err) { + const rollbackFailures = await rollbackWorktreeSet(rollbackEntries, 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 }; From 47a7a1839f072a1ec9112f1aa8e1cf1d71a4d1b5 Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Thu, 23 Jul 2026 14:32:54 +0100 Subject: [PATCH 02/12] Clean nested multi-repo containers Co-authored-by: bobbit-ai --- src/server/agent/session-manager.ts | 10 +++---- src/server/agent/team-manager.ts | 13 +++++---- src/server/skills/git.ts | 45 +++++++++++++++++++++++++---- 3 files changed, 51 insertions(+), 17 deletions(-) diff --git a/src/server/agent/session-manager.ts b/src/server/agent/session-manager.ts index bd7f290a3..2dc209466 100755 --- a/src/server/agent/session-manager.ts +++ b/src/server/agent/session-manager.ts @@ -11242,7 +11242,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). @@ -11264,11 +11264,9 @@ export class SessionManager { } }); try { - await fsp.rmdir(ps.worktreePath); - } catch (err: any) { - if (err?.code !== "ENOENT") { - console.error(`[session-manager] Failed to remove multi-repo branch container for ${ps.id}: ${ps.worktreePath}`, err); - } + 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); diff --git a/src/server/agent/team-manager.ts b/src/server/agent/team-manager.ts index b1d31116e..835552549 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, createWorktreeSet, 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"; @@ -191,11 +191,12 @@ async function cleanupCreatedWorktreeSet( } } try { - await fs.promises.rmdir(result.container); - } catch (err: any) { - if (err?.code !== "ENOENT") { - console.error(`[team-manager] Failed to remove worker branch container ${result.container}:`, err); - } + 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); } } diff --git a/src/server/skills/git.ts b/src/server/skills/git.ts index 4b2e19dbe..8836a7499 100644 --- a/src/server/skills/git.ts +++ b/src/server/skills/git.ts @@ -622,6 +622,43 @@ interface WorktreeSetRollbackEntry { deleteBranch: boolean; } +/** + * 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: WorktreeSetRollbackEntry[], container: string, @@ -648,11 +685,9 @@ async function rollbackWorktreeSet( } } try { - await fs.promises.rmdir(container); - } catch (err: any) { - if (err?.code !== "ENOENT") { - failures.push(`branch container cleanup failed at ${container}: ${err instanceof Error ? err.message : String(err)}`); - } + 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; } From 6dc61cf3d91ff26a01ae49c84813b88d2b5ae1d5 Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Thu, 23 Jul 2026 14:33:28 +0100 Subject: [PATCH 03/12] test multi-repo team spawn Co-authored-by: bobbit-ai --- scripts/testing-v2/test-map-execution.mjs | 1 + tests2/core/test-map-execution.test.ts | 3 + .../team-spawn-multi-repo-real-git.test.ts | 305 ++++++++++++++++++ tests2/tests-map.json | 9 + 4 files changed, 318 insertions(+) create mode 100644 tests2/integration/team-spawn-multi-repo-real-git.test.ts 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/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/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..9c04e1557 --- /dev/null +++ b/tests2/integration/team-spawn-multi-repo-real-git.test.ts @@ -0,0 +1,305 @@ +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 COMPONENTS = ["alpha", "beta"] 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]); + expect(new Set(COMPONENTS.map(repo => normalized(dirname(repoWorktrees[repo]))))).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 create coordinated workers from local component HEADs and roll back partial creation", 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 directWorkerId: string | undefined; + let restWorkerId: string | undefined; + let originalExecFile: CommandRunner["execFile"] | undefined; + + 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, 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]); + 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.alpha).not.toBe(goalHeads.beta); + + 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 alpha's real worktree add has completed. The + // rejected direct spawn must remove alpha's worktree, branch, and container. + originalExecFile = runner.execFile; + let failedBranch: string | undefined; + let failedContainer: string | undefined; + let firstComponentWorktree: string | undefined; + (runner as any).execFile = async (file: string, args: string[], options: any) => { + const gitCommand = file.toLowerCase().replace(/\.exe$/, "") === "git"; + 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, "alpha"))) { + failedBranch = branch; + firstComponentWorktree = target; + failedContainer = dirname(target); + } + if (workerAdd && normalized(String(options?.cwd ?? "")) === normalized(join(projectRoot, "beta"))) { + 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 && existsSync(firstComponentWorktree), "alpha partial worktree must be rolled back").toBe(false); + expect(failedContainer && existsSync(failedContainer), "empty worker branch container must be rolled back").toBe(false); + expect(await gitRefExists(runner, join(projectRoot, "alpha"), `refs/heads/${failedBranch}`), "alpha partial 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]); + } + + // Direct production call: this is the primary host-side regression boundary. + const directResult = await gateway.teamManager.spawnRole(goalId!, "coder", "Direct coordinated spawn"); + 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!; + for (const repo of COMPONENTS) { + 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" }, + ); + 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; + for (const workerId of [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); + if (goalId) await deleteGoal(goalId).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..da2f838be 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 local goal-component HEADs, verifies one common worker branch container and repo-key layout, and injects a second-component failure to prove first-component rollback.", + "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.", From cfa95be9e412ea34335834b731329be275c765f7 Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Thu, 23 Jul 2026 14:37:47 +0100 Subject: [PATCH 04/12] Restrict worktree set rollback targets Co-authored-by: bobbit-ai --- src/server/skills/git.ts | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/server/skills/git.ts b/src/server/skills/git.ts index 8836a7499..fb0a366c4 100644 --- a/src/server/skills/git.ts +++ b/src/server/skills/git.ts @@ -615,11 +615,10 @@ export interface CreateWorktreeSetOptions { skipPush?: boolean; } -interface WorktreeSetRollbackEntry { +interface WorktreeSetEntry { repo: string; repoPath: string; worktreePath: string; - deleteBranch: boolean; } /** @@ -660,7 +659,8 @@ export async function removeEmptyWorktreeSetContainer( } async function rollbackWorktreeSet( - entries: WorktreeSetRollbackEntry[], + entries: WorktreeSetEntry[], + createdBranchRepos: ReadonlySet, container: string, branchName: string, commandRunner: CommandRunner, @@ -673,7 +673,7 @@ async function rollbackWorktreeSet( entry.repoPath, entry.worktreePath, branchName, - entry.deleteBranch, + createdBranchRepos.has(entry.repo), commandRunner, { ...remotePolicy, skipRemotePush: true }, ); @@ -952,8 +952,8 @@ 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 }> = []; - const rollbackEntries: WorktreeSetRollbackEntry[] = []; + const out: WorktreeSetEntry[] = []; + const createdBranchRepos = new Set(); try { for (const repo of repoList) { const repoSrc = path.join(rootPath, repo); @@ -987,7 +987,6 @@ export async function createWorktreeSet( await runGit(["rev-parse", "--verify", branchName], { cwd: repoSrc }); branchExists = true; } catch { /* not present */ } - rollbackEntries.push({ repo, repoPath: repoSrc, worktreePath: wtPath, deleteBranch: !branchExists }); try { if (branchExists) { @@ -1006,6 +1005,12 @@ export async function createWorktreeSet( throw new Error(`createWorktreeSet: git worktree add failed for component "${repo}" at ${wtPath}: ${err instanceof Error ? err.message : err}`); } + // 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) { @@ -1023,10 +1028,9 @@ export async function createWorktreeSet( } } - out.push({ repo, repoPath: repoSrc, worktreePath: wtPath }); } } catch (err) { - const rollbackFailures = await rollbackWorktreeSet(rollbackEntries, container, branchName, commandRunner, remotePolicy); + 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("; ")}`); } From 57097b8959dd0e551e30ffbf8b7fb785d7b139c0 Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Thu, 23 Jul 2026 14:53:24 +0100 Subject: [PATCH 05/12] test nested multi-repo cleanup rollback Co-authored-by: bobbit-ai --- .../team-spawn-multi-repo-real-git.test.ts | 57 ++++++++++++++----- 1 file changed, 42 insertions(+), 15 deletions(-) diff --git a/tests2/integration/team-spawn-multi-repo-real-git.test.ts b/tests2/integration/team-spawn-multi-repo-real-git.test.ts index 9c04e1557..c9b04042f 100644 --- a/tests2/integration/team-spawn-multi-repo-real-git.test.ts +++ b/tests2/integration/team-spawn-multi-repo-real-git.test.ts @@ -8,7 +8,9 @@ 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 COMPONENTS = ["alpha", "beta"] as const; +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 { @@ -81,10 +83,11 @@ function assertWorkerShape(opts: { 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]); - expect(new Set(COMPONENTS.map(repo => normalized(dirname(repoWorktrees[repo]))))).toEqual( - new Set([normalized(worktreePath)]), - ); + 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( @@ -126,7 +129,7 @@ test("direct and REST team spawn create coordinated workers from local component 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, repo })), + components: COMPONENTS.map(repo => ({ name: repo.split("/").at(-1)!, repo })), workflows: { general: { name: "General", @@ -164,7 +167,7 @@ test("direct and REST team spawn create coordinated workers from local component 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]); + 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; @@ -175,21 +178,28 @@ test("direct and REST team spawn create coordinated workers from local component 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.alpha).not.toBe(goalHeads.beta); + 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 alpha's real worktree add has completed. The - // rejected direct spawn must remove alpha's worktree, branch, and container. + // 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]; @@ -197,12 +207,13 @@ test("direct and REST team spawn create coordinated workers from local component && args[0] === "worktree" && args[1] === "add" && branch?.startsWith(`goal/${goalId!.slice(0, 8)}/coder-`); - if (workerAdd && normalized(String(options?.cwd ?? "")) === normalized(join(projectRoot, "alpha"))) { + if (workerAdd && normalized(String(options?.cwd ?? "")) === normalized(join(projectRoot, NESTED_COMPONENT))) { failedBranch = branch; firstComponentWorktree = target; - failedContainer = dirname(target); + failedContainer = dirname(dirname(target)); } - if (workerAdd && normalized(String(options?.cwd ?? "")) === normalized(join(projectRoot, "beta"))) { + 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; @@ -224,9 +235,21 @@ test("direct and REST team spawn create coordinated workers from local component 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 && existsSync(firstComponentWorktree), "alpha partial worktree must be rolled back").toBe(false); + 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(await gitRefExists(runner, join(projectRoot, "alpha"), `refs/heads/${failedBranch}`), "alpha partial branch 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"]), @@ -264,6 +287,10 @@ test("direct and REST team spawn create coordinated workers from local component 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 From 819f0d0d19a5cb162bb1b28848070283c6940565 Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Thu, 23 Jul 2026 15:04:59 +0100 Subject: [PATCH 06/12] test: pin multi-repo spawn E2E lane Co-authored-by: bobbit-ai --- tests2/core/unit-lanes-scheduling.test.ts | 1 + 1 file changed, 1 insertion(+) 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, }, From d781e44622a06e276044dc3ac67a4e4f19b164ed Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Thu, 23 Jul 2026 15:55:02 +0100 Subject: [PATCH 07/12] test multi-repo spawn cleanup ownership Co-authored-by: bobbit-ai --- .../team-spawn-multi-repo-real-git.test.ts | 164 +++++++++++++++++- tests2/tests-map.json | 2 +- 2 files changed, 164 insertions(+), 2 deletions(-) diff --git a/tests2/integration/team-spawn-multi-repo-real-git.test.ts b/tests2/integration/team-spawn-multi-repo-real-git.test.ts index c9b04042f..cfd737783 100644 --- a/tests2/integration/team-spawn-multi-repo-real-git.test.ts +++ b/tests2/integration/team-spawn-multi-repo-real-git.test.ts @@ -118,6 +118,10 @@ test("direct and REST team spawn create coordinated workers from local component let directWorkerId: string | undefined; let restWorkerId: string | undefined; let originalExecFile: CommandRunner["execFile"] | undefined; + let originalCreateSession: any; + let originalUpdateSessionMeta: any; + let originalSessionStorePut: any; + let interceptedSessionStore: any; try { for (const repo of COMPONENTS) copyGitTemplate(join(projectRoot, repo)); @@ -257,8 +261,153 @@ test("direct and REST team spawn create coordinated workers from local component ).toBe(goalHeads[repo]); } + // A later session-creation failure owns only resources created by that + // attempt. Attach the nested component to a branch that already existed, + // let beta create the same branch freshly, then fail after both real Git + // worktrees exist. Cleanup must remove both attachments while preserving + // the pre-existing nested-component branch and never deleting remotely. + originalExecFile = runner.execFile; + originalCreateSession = (gateway.sessionManager as any).createSession; + let preExistingBranch: string | undefined; + let failedSessionContainer: string | undefined; + const attachedWorktrees: Partial> = {}; + const localBranchDeleteCalls: Array<{ cwd: string; branch: string }> = []; + const remoteBranchDeleteCalls: 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 ?? ""); + const candidateBranch = args[0] === "rev-parse" && args[1] === "--verify" + ? String(args[2] ?? "") + : ""; + if ( + gitCommand + && !preExistingBranch + && candidateBranch.startsWith(`goal/${goalId!.slice(0, 8)}/coder-`) + && normalized(cwd) === normalized(join(projectRoot, NESTED_COMPONENT)) + ) { + preExistingBranch = candidateBranch; + await originalExecFile!.call(runner, "git", ["branch", preExistingBranch, goalHeads[NESTED_COMPONENT]], { + cwd, + encoding: "utf-8", + timeout: 10_000, + }); + } + + 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 === preExistingBranch) { + if (normalized(cwd) === normalized(join(projectRoot, NESTED_COMPONENT))) { + attachedWorktrees[NESTED_COMPONENT] = target; + failedSessionContainer = resolve( + target, + ...NESTED_COMPONENT.split("/").map(() => ".."), + ); + } + if (normalized(cwd) === normalized(join(projectRoot, FAILED_COMPONENT))) { + attachedWorktrees[FAILED_COMPONENT] = target; + } + } + } + if (gitCommand && args[0] === "branch" && args[1] === "-D") { + localBranchDeleteCalls.push({ cwd, branch: String(args[2]) }); + } + if (gitCommand && args[0] === "push" && args.includes("--delete")) { + remoteBranchDeleteCalls.push({ cwd, args: [...args] }); + } + return originalExecFile!.call(runner, file, args, options); + }; + (gateway.sessionManager as any).createSession = async (...args: any[]) => { + const opts = args[4]; + if (args[2] === goalId && opts?.roleName === "coder") { + throw new Error("injected worker session creation failure"); + } + return originalCreateSession.apply(gateway.sessionManager, args); + }; + + let sessionCreationFailure: unknown; + try { + await gateway.teamManager.spawnRole(goalId!, "coder", "Preserve a pre-existing branch on later failure"); + } catch (error) { + sessionCreationFailure = error; + } finally { + (runner as any).execFile = originalExecFile; + originalExecFile = undefined; + (gateway.sessionManager as any).createSession = originalCreateSession; + originalCreateSession = undefined; + } + expect(sessionCreationFailure).toBeInstanceOf(Error); + expect((sessionCreationFailure as Error).message).toContain("injected worker session creation failure"); + expect(preExistingBranch, "fixture must create the worker branch before its nested-component attach").toBeTruthy(); + for (const repo of COMPONENTS) { + expect(attachedWorktrees[repo], `${repo} worker attachment must exist before session creation fails`).toBeTruthy(); + expect(existsSync(attachedWorktrees[repo]!), `${repo} newly attached worktree must be cleaned`).toBe(false); + } + expect(existsSync(dirname(attachedWorktrees[NESTED_COMPONENT]!)), "nested repo-key directory must be cleaned after session failure").toBe(false); + expect(failedSessionContainer && existsSync(failedSessionContainer), "branch container must be cleaned after session failure").toBe(false); + let preservedBranchHead: string | undefined; + try { + preservedBranchHead = await git(runner, join(projectRoot, NESTED_COMPONENT), ["rev-parse", preExistingBranch!]); + } catch { /* assertion below owns the missing-ref failure */ } + expect.soft( + preservedBranchHead, + "cleanup must preserve the exact pre-existing local branch", + ).toBe(goalHeads[NESTED_COMPONENT]); + expect.soft( + localBranchDeleteCalls.some(call => normalized(call.cwd) === normalized(join(projectRoot, NESTED_COMPONENT)) && call.branch === preExistingBranch), + "cleanup must not ask Git to delete the pre-existing component branch", + ).toBe(false); + expect( + await gitRefExists(runner, join(projectRoot, FAILED_COMPONENT), `refs/heads/${preExistingBranch}`), + "cleanup must still delete the beta branch created by this failed attempt", + ).toBe(false); + expect(remoteBranchDeleteCalls, "failed local provisioning/session creation must never delete a remote branch").toEqual([]); + // Direct production call: this is the primary host-side regression boundary. - const directResult = await gateway.teamManager.spawnRole(goalId!, "coder", "Direct coordinated spawn"); + // 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({ @@ -271,7 +420,17 @@ test("direct and REST team spawn create coordinated workers from local component 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); } @@ -318,6 +477,9 @@ test("direct and REST team spawn create coordinated workers from local component }); } finally { if (originalExecFile) (runner as any).execFile = originalExecFile; + if (originalCreateSession) (gateway.sessionManager as any).createSession = originalCreateSession; + if (originalUpdateSessionMeta) (gateway.sessionManager as any).updateSessionMeta = originalUpdateSessionMeta; + if (originalSessionStorePut && interceptedSessionStore) interceptedSessionStore.put = originalSessionStorePut; for (const workerId of [directWorkerId, restWorkerId]) { if (!workerId || !goalId) continue; await gateway.teamManager.dismissRoleForGoal(goalId, workerId).catch(() => undefined); diff --git a/tests2/tests-map.json b/tests2/tests-map.json index da2f838be..6e1d0c6a2 100644 --- a/tests2/tests-map.json +++ b/tests2/tests-map.json @@ -22,7 +22,7 @@ "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 local goal-component HEADs, verifies one common worker branch container and repo-key layout, and injects a second-component failure to prove first-component rollback.", + "reason": "Focused real-Git regression for a non-Git two-component project: drives direct TeamManager.spawnRole and one REST team/spawn request from unpublished local goal-component HEADs; verifies common branch-container layout, partial rollback, preservation of a pre-existing component branch without remote deletion after later failure, and cleanup coordinates in createSession's first durable write.", "execution": { "runner": "vitest", "tier": "e2e", From 4d4a7b9d4d49a58d9aca13ad8e0c5a7983b58a42 Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Thu, 23 Jul 2026 15:55:23 +0100 Subject: [PATCH 08/12] Preserve multi-repo cleanup ownership Co-authored-by: bobbit-ai --- src/server/agent/session-manager.ts | 9 ++++++++- src/server/agent/session-setup.ts | 12 ++++++++++++ src/server/agent/team-manager.ts | 11 ++++++++++- src/server/skills/git.ts | 9 +++++++-- 4 files changed, 37 insertions(+), 4 deletions(-) diff --git a/src/server/agent/session-manager.ts b/src/server/agent/session-manager.ts index 2dc209466..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, diff --git a/src/server/agent/session-setup.ts b/src/server/agent/session-setup.ts index 2a571eaf8..69b7bfca1 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, diff --git a/src/server/agent/team-manager.ts b/src/server/agent/team-manager.ts index 835552549..c8c80fbbe 100644 --- a/src/server/agent/team-manager.ts +++ b/src/server/agent/team-manager.ts @@ -178,7 +178,7 @@ async function cleanupCreatedWorktreeSet( worktree.repoPath, worktree.worktreePath, branchName, - true, + result.createdBranchRepos?.has(worktree.repo) === true, commandRunner, { skipRemotePush: true }, ); @@ -2279,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, diff --git a/src/server/skills/git.ts b/src/server/skills/git.ts index fb0a366c4..e5d87c761 100644 --- a/src/server/skills/git.ts +++ b/src/server/skills/git.ts @@ -875,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. @@ -1037,7 +1042,7 @@ export async function createWorktreeSet( throw err; } - return { container, worktrees: out }; + return { container, worktrees: out, createdBranchRepos }; } /** From 5cca9a1ffaf84970651b84753c43d09d58f7ea19 Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Thu, 23 Jul 2026 16:28:54 +0100 Subject: [PATCH 09/12] Reject exact-start branch reuse Co-authored-by: bobbit-ai --- src/server/skills/git.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/server/skills/git.ts b/src/server/skills/git.ts index e5d87c761..d2f4ddc5b 100644 --- a/src/server/skills/git.ts +++ b/src/server/skills/git.ts @@ -993,6 +993,17 @@ export async function createWorktreeSet( 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( + `createWorktreeSet: validate exact start failed for component "${repo}": branch "${branchName}" already exists`, + ); + } + try { if (branchExists) { await runGit(["worktree", "add", wtPath, branchName], { cwd: repoSrc }); From e983d786cc5ea15517152471df8a1c0467e3ce41 Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Thu, 23 Jul 2026 16:30:44 +0100 Subject: [PATCH 10/12] test exact-start branch collision rollback Co-authored-by: bobbit-ai --- .../team-spawn-multi-repo-real-git.test.ts | 188 ++++++++++-------- tests2/tests-map.json | 2 +- 2 files changed, 110 insertions(+), 80 deletions(-) diff --git a/tests2/integration/team-spawn-multi-repo-real-git.test.ts b/tests2/integration/team-spawn-multi-repo-real-git.test.ts index cfd737783..9ebb9fbee 100644 --- a/tests2/integration/team-spawn-multi-repo-real-git.test.ts +++ b/tests2/integration/team-spawn-multi-repo-real-git.test.ts @@ -106,7 +106,7 @@ function assertWorkerShape(opts: { // 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 create coordinated workers from local component HEADs and roll back partial creation", async ({ gateway }) => { +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"); @@ -115,10 +115,10 @@ test("direct and REST team spawn create coordinated workers from local component let projectId: string | undefined; let goalId: string | undefined; let teamLeadSessionId: string | undefined; + let collisionWorkerId: string | undefined; let directWorkerId: string | undefined; let restWorkerId: string | undefined; let originalExecFile: CommandRunner["execFile"] | undefined; - let originalCreateSession: any; let originalUpdateSessionMeta: any; let originalSessionStorePut: any; let interceptedSessionStore: any; @@ -261,108 +261,139 @@ test("direct and REST team spawn create coordinated workers from local component ).toBe(goalHeads[repo]); } - // A later session-creation failure owns only resources created by that - // attempt. Attach the nested component to a branch that already existed, - // let beta create the same branch freshly, then fail after both real Git - // worktrees exist. Cleanup must remove both attachments while preserving - // the pre-existing nested-component branch and never deleting remotely. + // 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; - originalCreateSession = (gateway.sessionManager as any).createSession; - let preExistingBranch: string | undefined; - let failedSessionContainer: string | undefined; - const attachedWorktrees: Partial> = {}; + 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 remoteBranchDeleteCalls: Array<{ cwd: string; args: 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 ?? ""); - const candidateBranch = args[0] === "rev-parse" && args[1] === "--verify" - ? String(args[2] ?? "") - : ""; - if ( - gitCommand - && !preExistingBranch - && candidateBranch.startsWith(`goal/${goalId!.slice(0, 8)}/coder-`) - && normalized(cwd) === normalized(join(projectRoot, NESTED_COMPONENT)) - ) { - preExistingBranch = candidateBranch; - await originalExecFile!.call(runner, "git", ["branch", preExistingBranch, goalHeads[NESTED_COMPONENT]], { - cwd, - encoding: "utf-8", - timeout: 10_000, - }); + 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]); - if (branch === preExistingBranch) { - if (normalized(cwd) === normalized(join(projectRoot, NESTED_COMPONENT))) { - attachedWorktrees[NESTED_COMPONENT] = target; - failedSessionContainer = resolve( - target, - ...NESTED_COMPONENT.split("/").map(() => ".."), - ); - } - if (normalized(cwd) === normalized(join(projectRoot, FAILED_COMPONENT))) { - attachedWorktrees[FAILED_COMPONENT] = target; + 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 (gitCommand && args[0] === "branch" && args[1] === "-D") { - localBranchDeleteCalls.push({ cwd, branch: String(args[2]) }); - } - if (gitCommand && args[0] === "push" && args.includes("--delete")) { - remoteBranchDeleteCalls.push({ cwd, args: [...args] }); + if (workerAdd && normalized(cwd) === normalized(join(projectRoot, FAILED_COMPONENT))) { + collidingWorktree = target; + collisionWorktreeAddTargets.push(target); + } } return originalExecFile!.call(runner, file, args, options); }; - (gateway.sessionManager as any).createSession = async (...args: any[]) => { - const opts = args[4]; - if (args[2] === goalId && opts?.roleName === "coder") { - throw new Error("injected worker session creation failure"); - } - return originalCreateSession.apply(gateway.sessionManager, args); - }; - let sessionCreationFailure: unknown; + let collisionFailure: unknown; try { - await gateway.teamManager.spawnRole(goalId!, "coder", "Preserve a pre-existing branch on later failure"); + const result = await gateway.teamManager.spawnRole(goalId!, "coder", "Reject a divergent exact-start branch collision"); + collisionWorkerId = result.sessionId; } catch (error) { - sessionCreationFailure = error; + collisionFailure = error; } finally { (runner as any).execFile = originalExecFile; originalExecFile = undefined; - (gateway.sessionManager as any).createSession = originalCreateSession; - originalCreateSession = undefined; } - expect(sessionCreationFailure).toBeInstanceOf(Error); - expect((sessionCreationFailure as Error).message).toContain("injected worker session creation failure"); - expect(preExistingBranch, "fixture must create the worker branch before its nested-component attach").toBeTruthy(); - for (const repo of COMPONENTS) { - expect(attachedWorktrees[repo], `${repo} worker attachment must exist before session creation fails`).toBeTruthy(); - expect(existsSync(attachedWorktrees[repo]!), `${repo} newly attached worktree must be cleaned`).toBe(false); + 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(existsSync(dirname(attachedWorktrees[NESTED_COMPONENT]!)), "nested repo-key directory must be cleaned after session failure").toBe(false); - expect(failedSessionContainer && existsSync(failedSessionContainer), "branch container must be cleaned after session failure").toBe(false); - let preservedBranchHead: string | undefined; - try { - preservedBranchHead = await git(runner, join(projectRoot, NESTED_COMPONENT), ["rev-parse", preExistingBranch!]); - } catch { /* assertion below owns the missing-ref failure */ } - expect.soft( - preservedBranchHead, - "cleanup must preserve the exact pre-existing local branch", - ).toBe(goalHeads[NESTED_COMPONENT]); - expect.soft( - localBranchDeleteCalls.some(call => normalized(call.cwd) === normalized(join(projectRoot, NESTED_COMPONENT)) && call.branch === preExistingBranch), - "cleanup must not ask Git to delete the pre-existing component branch", + 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( - await gitRefExists(runner, join(projectRoot, FAILED_COMPONENT), `refs/heads/${preExistingBranch}`), - "cleanup must still delete the beta branch created by this failed attempt", + 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(remoteBranchDeleteCalls, "failed local provisioning/session creation must never delete a remote branch").toEqual([]); + 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!]); // Direct production call: this is the primary host-side regression boundary. // Observe the project SessionStore boundary so the first durable worker @@ -477,10 +508,9 @@ test("direct and REST team spawn create coordinated workers from local component }); } finally { if (originalExecFile) (runner as any).execFile = originalExecFile; - if (originalCreateSession) (gateway.sessionManager as any).createSession = originalCreateSession; if (originalUpdateSessionMeta) (gateway.sessionManager as any).updateSessionMeta = originalUpdateSessionMeta; if (originalSessionStorePut && interceptedSessionStore) interceptedSessionStore.put = originalSessionStorePut; - for (const workerId of [directWorkerId, restWorkerId]) { + 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); diff --git a/tests2/tests-map.json b/tests2/tests-map.json index 6e1d0c6a2..aa0def1da 100644 --- a/tests2/tests-map.json +++ b/tests2/tests-map.json @@ -22,7 +22,7 @@ "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 local goal-component HEADs; verifies common branch-container layout, partial rollback, preservation of a pre-existing component branch without remote deletion after later failure, and cleanup coordinates in createSession's first durable write.", + "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, common branch-container layout, injected partial rollback, and cleanup coordinates in createSession's first durable write.", "execution": { "runner": "vitest", "tier": "e2e", From 204cb87009930ccc50fcdef20476a00b8832f289 Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Thu, 23 Jul 2026 17:09:45 +0100 Subject: [PATCH 11/12] Skip container cleanup after multi-repo setup failure Co-authored-by: bobbit-ai --- src/server/agent/session-setup.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/server/agent/session-setup.ts b/src/server/agent/session-setup.ts index 69b7bfca1..510f092ac 100644 --- a/src/server/agent/session-setup.ts +++ b/src/server/agent/session-setup.ts @@ -1817,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(() => {}); From 7fc725bc990e6f29fbe76936284ec53873d3790f Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Thu, 23 Jul 2026 17:18:08 +0100 Subject: [PATCH 12/12] test post-spawn multi-repo cleanup ownership Co-authored-by: bobbit-ai --- .../team-spawn-multi-repo-real-git.test.ts | 133 ++++++++++++++++++ tests2/tests-map.json | 2 +- 2 files changed, 134 insertions(+), 1 deletion(-) diff --git a/tests2/integration/team-spawn-multi-repo-real-git.test.ts b/tests2/integration/team-spawn-multi-repo-real-git.test.ts index 9ebb9fbee..7122b0ca6 100644 --- a/tests2/integration/team-spawn-multi-repo-real-git.test.ts +++ b/tests2/integration/team-spawn-multi-repo-real-git.test.ts @@ -116,9 +116,11 @@ test("direct and REST team spawn preserve exact local component HEADs across col 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; @@ -395,6 +397,132 @@ test("direct and REST team spawn preserve exact local component HEADs across col } 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. @@ -508,6 +636,7 @@ test("direct and REST team spawn preserve exact local component HEADs across col }); } 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]) { @@ -517,7 +646,11 @@ test("direct and REST team spawn preserve exact local component HEADs across col } 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 aa0def1da..a6c8bda7d 100644 --- a/tests2/tests-map.json +++ b/tests2/tests-map.json @@ -22,7 +22,7 @@ "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, common branch-container layout, injected partial rollback, and cleanup coordinates in createSession's first durable write.", + "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",