diff --git a/server/src/__tests__/execution-workspaces-service.test.ts b/server/src/__tests__/execution-workspaces-service.test.ts index 858ed344f2..80af2fd5aa 100644 --- a/server/src/__tests__/execution-workspaces-service.test.ts +++ b/server/src/__tests__/execution-workspaces-service.test.ts @@ -631,6 +631,228 @@ describeEmbeddedPostgres("executionWorkspaceService.getCloseReadiness", () => { }); }, 20_000); + it("reconciles forward when the recorded branch has no resolvable commit and the worktree is clean", async () => { + const repoRoot = await createTempRepo(); + tempDirs.add(repoRoot); + const worktreePath = path.join(path.dirname(repoRoot), `paperclip-missing-recorded-${randomUUID()}`); + tempDirs.add(worktreePath); + + await runGit(repoRoot, ["worktree", "add", "-b", "feature/current", worktreePath, "HEAD"]); + await fs.writeFile(path.join(worktreePath, "feature.txt"), "current branch\n", "utf8"); + await runGit(worktreePath, ["add", "feature.txt"]); + await runGit(worktreePath, ["commit", "-m", "Current branch work"]); + + const companyId = randomUUID(); + const projectId = randomUUID(); + const issueId = randomUUID(); + const executionWorkspaceId = randomUUID(); + + await db.insert(companies).values({ + id: companyId, + name: "Paperclip", + issuePrefix: "PAP", + requireBoardApprovalForNewAgents: false, + }); + await db.insert(projects).values({ + id: projectId, + companyId, + name: "Missing recorded branch", + status: "in_progress", + }); + await db.insert(issues).values({ + id: issueId, + companyId, + projectId, + title: "Source task", + identifier: "PAP-124", + status: "blocked", + priority: "medium", + }); + await db.insert(executionWorkspaces).values({ + id: executionWorkspaceId, + companyId, + projectId, + sourceIssueId: issueId, + mode: "isolated_workspace", + strategyType: "git_worktree", + name: "feature/never-created", + status: "idle", + providerType: "git_worktree", + cwd: worktreePath, + providerRef: worktreePath, + branchName: "feature/never-created", + baseRef: "main", + }); + + const result = await svc.reconcileExecutionWorkspaceBranch(executionWorkspaceId, { + mode: "forward", + reason: null, + actor: { + actorType: "user", + actorId: "local-board", + agentId: null, + runId: null, + }, + }); + + expect(result.workspace.branchName).toBe("feature/current"); + expect(result.workspace.name).toBe("feature/current"); + expect(result.inspection).toMatchObject({ + fromBranch: "feature/never-created", + toBranch: "feature/current", + fromSha: null, + ancestryVerdict: "unknown", + cleanliness: "clean", + }); + + const [comment] = await db + .select() + .from(issueComments) + .where(eq(issueComments.issueId, issueId)); + expect(comment?.body).toContain("Execution workspace branch reconciled."); + expect(comment?.body).toContain("- From branch: `feature/never-created`"); + expect(comment?.body).toContain("- To branch: `feature/current`"); + }, 20_000); + + it("keeps forward reconciliation fail-closed when the recorded branch is missing but the worktree is dirty", async () => { + const repoRoot = await createTempRepo(); + tempDirs.add(repoRoot); + const worktreePath = path.join(path.dirname(repoRoot), `paperclip-missing-recorded-dirty-${randomUUID()}`); + tempDirs.add(worktreePath); + + await runGit(repoRoot, ["worktree", "add", "-b", "feature/current", worktreePath, "HEAD"]); + await fs.writeFile(path.join(worktreePath, "uncommitted.txt"), "dirty work\n", "utf8"); + + const companyId = randomUUID(); + const projectId = randomUUID(); + const issueId = randomUUID(); + const executionWorkspaceId = randomUUID(); + + await db.insert(companies).values({ + id: companyId, + name: "Paperclip", + issuePrefix: "PAP", + requireBoardApprovalForNewAgents: false, + }); + await db.insert(projects).values({ + id: projectId, + companyId, + name: "Missing recorded branch dirty", + status: "in_progress", + }); + await db.insert(issues).values({ + id: issueId, + companyId, + projectId, + title: "Source task", + identifier: "PAP-125", + status: "blocked", + priority: "medium", + }); + await db.insert(executionWorkspaces).values({ + id: executionWorkspaceId, + companyId, + projectId, + sourceIssueId: issueId, + mode: "isolated_workspace", + strategyType: "git_worktree", + name: "feature/never-created", + status: "idle", + providerType: "git_worktree", + cwd: worktreePath, + providerRef: worktreePath, + branchName: "feature/never-created", + baseRef: "main", + }); + + await expect(svc.reconcileExecutionWorkspaceBranch(executionWorkspaceId, { + mode: "forward", + reason: null, + actor: { + actorType: "user", + actorId: "local-board", + agentId: null, + runId: null, + }, + })).rejects.toMatchObject({ + status: 422, + message: expect.stringContaining("requires the recorded branch to be an ancestor"), + }); + }, 20_000); + + it("keeps forward reconciliation fail-closed when the checked-out branch ref does not resolve either", async () => { + const repoRoot = await createTempRepo(); + tempDirs.add(repoRoot); + const worktreePath = path.join(path.dirname(repoRoot), `paperclip-missing-both-refs-${randomUUID()}`); + tempDirs.add(worktreePath); + + // An empty tree keeps the worktree clean even after its branch ref is + // deleted, so this exercises the adoption gate rather than cleanliness. + const emptyTreeSha = (await readGit(repoRoot, ["hash-object", "-t", "tree", "/dev/null"]))!; + const emptyCommitSha = (await readGit(repoRoot, ["commit-tree", emptyTreeSha, "-m", "empty base"]))!; + await runGit(repoRoot, ["branch", "empty-base", emptyCommitSha]); + await runGit(repoRoot, ["worktree", "add", "-b", "feature/current", worktreePath, "empty-base"]); + // Deleting the local ref while it is checked out leaves symbolic-ref still + // reporting the branch name even though nothing resolves to a commit. + await runGit(repoRoot, ["update-ref", "-d", "refs/heads/feature/current"]); + + const companyId = randomUUID(); + const projectId = randomUUID(); + const issueId = randomUUID(); + const executionWorkspaceId = randomUUID(); + + await db.insert(companies).values({ + id: companyId, + name: "Paperclip", + issuePrefix: "PAP", + requireBoardApprovalForNewAgents: false, + }); + await db.insert(projects).values({ + id: projectId, + companyId, + name: "Missing both branch refs", + status: "in_progress", + }); + await db.insert(issues).values({ + id: issueId, + companyId, + projectId, + title: "Source task", + identifier: "PAP-126", + status: "blocked", + priority: "medium", + }); + await db.insert(executionWorkspaces).values({ + id: executionWorkspaceId, + companyId, + projectId, + sourceIssueId: issueId, + mode: "isolated_workspace", + strategyType: "git_worktree", + name: "feature/never-created", + status: "idle", + providerType: "git_worktree", + cwd: worktreePath, + providerRef: worktreePath, + branchName: "feature/never-created", + baseRef: "main", + }); + + await expect(svc.reconcileExecutionWorkspaceBranch(executionWorkspaceId, { + mode: "forward", + reason: null, + actor: { + actorType: "user", + actorId: "local-board", + agentId: null, + runId: null, + }, + })).rejects.toMatchObject({ + status: 422, + message: expect.stringContaining("requires the recorded branch to be an ancestor"), + }); + }, 20_000); + it("quarantine_restore rescues dirty live-branch work, resolves recovery, and returns the source issue to todo", async () => { const repoRoot = await createTempRepo(); tempDirs.add(repoRoot); @@ -2042,81 +2264,6 @@ describeEmbeddedPostgres("executionWorkspaceService.getCloseReadiness", () => { expect(comments).toHaveLength(0); }, 20_000); - it("rejects forward branch reconciliation when branch ancestry is unknown", async () => { - const repoRoot = await createTempRepo(); - tempDirs.add(repoRoot); - const worktreePath = path.join(path.dirname(repoRoot), `paperclip-unknown-${randomUUID()}`); - tempDirs.add(worktreePath); - - await runGit(repoRoot, ["branch", "feature/current"]); - await runGit(repoRoot, ["worktree", "add", worktreePath, "feature/current"]); - - const companyId = randomUUID(); - const projectId = randomUUID(); - const issueId = randomUUID(); - const executionWorkspaceId = randomUUID(); - - await db.insert(companies).values({ - id: companyId, - name: "Paperclip", - issuePrefix: "PAP", - requireBoardApprovalForNewAgents: false, - }); - await db.insert(projects).values({ - id: projectId, - companyId, - name: "Branch reconcile", - status: "in_progress", - }); - await db.insert(issues).values({ - id: issueId, - companyId, - projectId, - title: "Source task", - status: "blocked", - priority: "medium", - }); - await db.insert(executionWorkspaces).values({ - id: executionWorkspaceId, - companyId, - projectId, - sourceIssueId: issueId, - mode: "isolated_workspace", - strategyType: "git_worktree", - name: "Unknown workspace", - status: "idle", - providerType: "git_worktree", - cwd: worktreePath, - providerRef: worktreePath, - branchName: "feature/missing-recorded", - baseRef: "main", - }); - - await expect(svc.reconcileExecutionWorkspaceBranch(executionWorkspaceId, { - mode: "forward", - reason: null, - actor: { - actorType: "user", - actorId: "local-board", - agentId: null, - runId: null, - }, - })).rejects.toMatchObject({ - status: 422, - details: { - inspection: expect.objectContaining({ - ancestryVerdict: "unknown", - fromBranch: "feature/missing-recorded", - toBranch: "feature/current", - fromSha: null, - }), - }, - }); - - const comments = await db.select().from(issueComments).where(eq(issueComments.issueId, issueId)); - expect(comments).toHaveLength(0); - }, 20_000); - it("returns a bounded company-scoped workspace overview with service and linked issue summaries", async () => { const companyId = randomUUID(); const otherCompanyId = randomUUID(); diff --git a/server/src/__tests__/workspace-runtime.test.ts b/server/src/__tests__/workspace-runtime.test.ts index 8a00355965..10dcb37507 100644 --- a/server/src/__tests__/workspace-runtime.test.ts +++ b/server/src/__tests__/workspace-runtime.test.ts @@ -2690,7 +2690,7 @@ describe("realizeExecutionWorkspace", () => { }); }, 15_000); - it("classifies persisted git worktree branch incoherence as unknown when the recorded branch was deleted", async () => { + it("routes a deleted recorded branch with a clean worktree to forward adoption when reconcile-forward is enabled", async () => { const repoRoot = await createTempRepo(); const expectedBranch = "PAP-458-deleted-recorded-branch"; const actualBranch = "PAP-458-actual-work"; @@ -2740,6 +2740,8 @@ describe("realizeExecutionWorkspace", () => { error = err; } + // Without a database the adoption cannot be audited, so it still fails closed — + // but through the forward-adoption path rather than "expected branch does not exist". expect(error).toMatchObject({ code: "workspace_validation_failed", resultJson: { @@ -2759,6 +2761,76 @@ describe("realizeExecutionWorkspace", () => { ancestryVerdict: "unknown", plainLanguageReason: expect.stringContaining("missing a resolvable HEAD commit"), }), + safeRepair: expect.objectContaining({ + attempted: false, + succeeded: false, + reason: "forward reconciliation adoption requires database access to audit after workspace realization", + }), + }), + }, + }); + }, 15_000); + + it("keeps a deleted recorded branch fail-closed when reconcile-forward is disabled", async () => { + const repoRoot = await createTempRepo(); + const expectedBranch = "PAP-458-deleted-recorded-branch-flag-off"; + const actualBranch = "PAP-458-actual-work-flag-off"; + const worktreePath = path.join(repoRoot, ".paperclip", "worktrees", expectedBranch); + + await fs.mkdir(path.dirname(worktreePath), { recursive: true }); + await runGit(repoRoot, ["branch", expectedBranch]); + await runGit(repoRoot, ["worktree", "add", "-b", actualBranch, worktreePath, "HEAD"]); + await runGit(repoRoot, ["branch", "-D", expectedBranch]); + + let error: unknown = null; + try { + await ensurePersistedExecutionWorkspaceAvailable({ + base: { + baseCwd: repoRoot, + source: "project_primary", + projectId: "project-1", + workspaceId: "workspace-1", + repoUrl: null, + repoRef: "HEAD", + }, + workspace: { + id: "execution-workspace-deleted-branch-flag-off", + mode: "isolated_workspace", + strategyType: "git_worktree", + cwd: worktreePath, + providerRef: worktreePath, + projectId: "project-1", + projectWorkspaceId: "workspace-1", + repoUrl: null, + baseRef: "HEAD", + branchName: expectedBranch, + }, + issue: { + id: "issue-deleted-branch-flag-off", + identifier: "PAP-458", + title: "Classify deleted branch ancestry", + }, + agent: { + id: "agent-1", + name: "Codex Coder", + companyId: "company-1", + }, + enableWorkspaceBranchReconcileForward: false, + }); + } catch (err) { + error = err; + } + + expect(error).toMatchObject({ + code: "workspace_validation_failed", + resultJson: { + workspaceValidation: expect.objectContaining({ + cleanliness: "clean", + provenance: expect.objectContaining({ + expectedBranchExists: false, + actualBranchExists: true, + ancestryVerdict: "unknown", + }), safeRepair: expect.objectContaining({ eligible: false, attempted: false, diff --git a/server/src/services/execution-workspaces.ts b/server/src/services/execution-workspaces.ts index d47d81801e..d7e12172f3 100644 --- a/server/src/services/execution-workspaces.ts +++ b/server/src/services/execution-workspaces.ts @@ -56,6 +56,8 @@ export type ExecutionWorkspaceBranchReconcileActor = { runId: string | null; }; +export type ExecutionWorkspaceBranchRefResolution = "resolved" | "missing" | "error"; + export type ExecutionWorkspaceBranchReconcileInspection = { fingerprint: string; worktreePath: string; @@ -64,6 +66,8 @@ export type ExecutionWorkspaceBranchReconcileInspection = { toBranch: string; fromSha: string | null; toSha: string | null; + fromBranchRefStatus: ExecutionWorkspaceBranchRefResolution; + toBranchRefStatus: ExecutionWorkspaceBranchRefResolution; ancestryVerdict: GitWorktreeBranchAncestryVerdict; cleanliness: "clean" | "dirty" | "unknown"; statusEntryCount: number | null; @@ -224,6 +228,24 @@ function fingerprintWorkspaceBranchIncoherence(input: { return `workspace_incoherence:v1:sha256:${digest}`; } +async function resolveLocalBranchCommit( + repoRoot: string, + branch: string, +): Promise<{ status: ExecutionWorkspaceBranchRefResolution; sha: string | null }> { + try { + // --quiet makes an absent ref exit 1 with empty output instead of exiting + // 128 with a fatal message, so a missing branch stays distinguishable from + // git failing to inspect the repository at all. + const sha = await readGitStdout(["rev-parse", "--verify", "--quiet", `refs/heads/${branch}^{commit}`], repoRoot); + return sha ? { status: "resolved", sha } : { status: "missing", sha: null }; + } catch (error) { + const code = typeof error === "object" && error && "code" in error + ? (error as { code?: unknown }).code + : null; + return { status: code === 1 ? "missing" : "error", sha: null }; + } +} + async function getGitWorktreeBranchAncestryVerdict(input: { repoRoot: string; expectedHeadSha: string | null; @@ -296,8 +318,9 @@ async function inspectExecutionWorkspaceBranchForReconcile( const cleanliness: ExecutionWorkspaceBranchReconcileInspection["cleanliness"] = status === null ? "unknown" : status.trim().length > 0 ? "dirty" : "clean"; - const fromSha = await readGitStdout(["rev-parse", "--verify", `refs/heads/${fromBranch}^{commit}`], repoRoot) - .catch(() => null); + const fromRef = await resolveLocalBranchCommit(repoRoot, fromBranch); + const toRef = await resolveLocalBranchCommit(repoRoot, toBranch); + const fromSha = fromRef.sha; const toSha = await readGitStdout(["rev-parse", "HEAD"], worktreePath).catch(() => null); const ancestryVerdict = await getGitWorktreeBranchAncestryVerdict({ repoRoot, @@ -322,6 +345,8 @@ async function inspectExecutionWorkspaceBranchForReconcile( toBranch, fromSha, toSha, + fromBranchRefStatus: fromRef.status, + toBranchRefStatus: toRef.status, ancestryVerdict, cleanliness, statusEntryCount: statusLines?.length ?? null, @@ -1627,7 +1652,18 @@ export function executionWorkspaceService(db: Db) { } const inspection = await inspectExecutionWorkspaceBranchForReconcile(existing); - if (input.mode === "forward" && inspection.ancestryVerdict !== "ancestor") { + // A recorded branch whose ref is confirmed absent (not merely unreadable) + // has nothing to lose, so adopting the clean checked-out branch is + // trivially forward-only — provided the adopted branch's own local ref + // resolves, so a nonexistent branch name is never persisted. + const recordedBranchAdoptable = + inspection.fromBranchRefStatus === "missing" && + inspection.toBranchRefStatus === "resolved"; + if ( + input.mode === "forward" && + inspection.ancestryVerdict !== "ancestor" && + !(recordedBranchAdoptable && inspection.cleanliness === "clean") + ) { throw unprocessable( "Forward branch reconciliation requires the recorded branch to be an ancestor of the checked-out branch", { inspection }, diff --git a/server/src/services/workspace-runtime.ts b/server/src/services/workspace-runtime.ts index 3814635ae7..71ee337052 100644 --- a/server/src/services/workspace-runtime.ts +++ b/server/src/services/workspace-runtime.ts @@ -1758,14 +1758,24 @@ export async function ensureGitWorktreeBranchCoherent(input: { }; } + // A recorded branch that no longer exists anywhere has no commits to lose, so + // adopting the clean checked-out branch is trivially forward-only. This is the + // steady state left behind when an agent renames its task branch (e.g. to a + // feat/* PR branch) and the recorded branch was never created or was deleted. + const recordedBranchMissingButAdoptable = + !evidence.provenance.expectedBranchExists && + evidence.provenance.actualBranchExists === true && + evidence.provenance.registeredBranchMatchesHead; if ( input.enableWorkspaceBranchReconcileForward === true && - evidence.provenance.ancestryVerdict === "ancestor" && - !evidence.provenance.sameHead && evidence.cleanliness === "clean" && - currentBranch + currentBranch && + ((evidence.provenance.ancestryVerdict === "ancestor" && !evidence.provenance.sameHead) || + recordedBranchMissingButAdoptable) ) { - const reason = "Automatic forward reconciliation: recorded branch is an ancestor of the checked-out branch."; + const reason = evidence.provenance.expectedBranchExists + ? "Automatic forward reconciliation: recorded branch is an ancestor of the checked-out branch." + : "Automatic forward reconciliation: the recorded branch no longer exists, so Paperclip adopted the clean checked-out branch."; if (input.executionWorkspaceId && input.persistForwardReconcile !== false) { if (!input.db) { evidence.safeRepair.reason = "forward reconciliation requires database access to update the execution workspace record";