diff --git a/server/src/__tests__/workspace-runtime.test.ts b/server/src/__tests__/workspace-runtime.test.ts index 2fd730ae4df..35016b84fe7 100644 --- a/server/src/__tests__/workspace-runtime.test.ts +++ b/server/src/__tests__/workspace-runtime.test.ts @@ -3469,6 +3469,8 @@ describe("realizeExecutionWorkspace", () => { }, }); + expect(workspace.branchCreatedByRuntime).toBe(true); + const cleanup = await cleanupExecutionWorkspaceArtifacts({ workspace: { id: "execution-workspace-1", @@ -3482,7 +3484,7 @@ describe("realizeExecutionWorkspace", () => { projectWorkspaceId: workspace.workspaceId, sourceIssueId: "issue-1", metadata: { - createdByRuntime: true, + createdByRuntime: workspace.branchCreatedByRuntime, }, }, projectWorkspace: { @@ -8368,6 +8370,10 @@ describe("realizeExecutionWorkspace with an exact existing branch", () => { expect(workspace.strategy).toBe("git_worktree"); expect(workspace.branchName).toBe("feature/preexisting-work"); + // The worktree is freshly created, but the pinned branch pre-existed and + // stays operator-owned so terminal cleanup never deletes it. + expect(workspace.created).toBe(true); + expect(workspace.branchCreatedByRuntime).toBe(false); expect(workspace.cwd).not.toBe(repoRoot); expect(workspace.cwd).toContain(path.join(".paperclip", "worktrees")); expect(await readGit(workspace.cwd, ["branch", "--show-current"])).toBe("feature/preexisting-work"); @@ -8446,6 +8452,146 @@ describe("realizeExecutionWorkspace with an exact existing branch", () => { expect(newMaster).not.toBe(oldMaster); }); + function cleanupWorkspaceInput( + workspace: RealizedExecutionWorkspace, + repoRoot: string, + expectedBranchHeadSha: string, + ) { + return { + workspace: { + id: "execution-workspace-1", + cwd: workspace.cwd, + providerType: "git_worktree", + providerRef: workspace.worktreePath, + branchName: workspace.branchName, + repoUrl: workspace.repoUrl, + baseRef: workspace.repoRef, + projectId: workspace.projectId, + projectWorkspaceId: workspace.workspaceId, + sourceIssueId: "issue-pr-prep", + // Persist ownership the way the heartbeat does: from the branch + // ownership signal, never from worktree creation. + metadata: { createdByRuntime: workspace.branchCreatedByRuntime }, + }, + projectWorkspace: { + cwd: repoRoot, + cleanupCommand: null, + }, + expectedBranchHeadSha, + }; + } + + it("terminal cleanup removes the worktree but preserves the attached pre-existing branch at its tip", async () => { + const repoRoot = await createTempRepo(); + const branchTip = await createBranchWithCommit(repoRoot, "feature/operator-owned", "operator.txt"); + + const workspace = await realizeExistingBranch(repoRoot, "feature/operator-owned"); + expect(workspace.created).toBe(true); + expect(workspace.branchCreatedByRuntime).toBe(false); + + const cleanup = await cleanupExecutionWorkspaceArtifacts( + cleanupWorkspaceInput(workspace, repoRoot, branchTip), + ); + + expect(cleanup.cleaned).toBe(true); + expect(cleanup.warnings).toEqual([]); + await expect(fs.stat(workspace.cwd)).rejects.toThrow(); + expect(await readGit(repoRoot, ["rev-parse", "refs/heads/feature/operator-owned"])).toBe(branchTip); + }); + + it("terminal cleanup preserves a pre-existing branch pinned via a literal branchTemplate", async () => { + const repoRoot = await createTempRepo(); + const branchTip = await createBranchWithCommit(repoRoot, "feature/literal-pin", "literal.txt"); + + const workspace = await realizeExecutionWorkspace({ + base: { + baseCwd: repoRoot, + source: "project_primary", + projectId: "project-1", + workspaceId: "workspace-1", + repoUrl: null, + repoRef: "HEAD", + }, + config: { + workspaceStrategy: { + type: "git_worktree", + branchTemplate: "feature/literal-pin", + }, + }, + issue: { + id: "issue-pr-prep", + identifier: "PAP-9002", + title: "Prepare pull request via a literal branch template", + }, + agent: { + id: "agent-1", + name: "Codex Coder", + companyId: "company-1", + }, + }); + + expect(workspace.branchName).toBe("feature/literal-pin"); + expect(workspace.created).toBe(true); + expect(workspace.branchCreatedByRuntime).toBe(false); + expect(await readGit(workspace.cwd, ["rev-parse", "HEAD"])).toBe(branchTip); + + const cleanup = await cleanupExecutionWorkspaceArtifacts( + cleanupWorkspaceInput(workspace, repoRoot, branchTip), + ); + + expect(cleanup.cleaned).toBe(true); + expect(cleanup.warnings).toEqual([]); + await expect(fs.stat(workspace.cwd)).rejects.toThrow(); + expect(await readGit(repoRoot, ["rev-parse", "refs/heads/feature/literal-pin"])).toBe(branchTip); + }); + + it("restoring a persisted workspace keeps the recorded branch ownership", async () => { + const repoRoot = await createTempRepo(); + await createBranchWithCommit(repoRoot, "feature/persisted-ownership", "persisted-ownership.txt"); + const first = await realizeExistingBranch(repoRoot, "feature/persisted-ownership"); + + async function restoreWithMetadata(metadata: Record) { + return await ensurePersistedExecutionWorkspaceAvailable({ + base: { + baseCwd: repoRoot, + source: "project_primary", + projectId: "project-1", + workspaceId: "workspace-1", + repoUrl: null, + repoRef: "HEAD", + }, + workspace: { + mode: "isolated_workspace", + strategyType: "git_worktree", + cwd: first.cwd, + providerRef: first.worktreePath, + projectId: "project-1", + projectWorkspaceId: "workspace-1", + repoUrl: null, + baseRef: "HEAD", + branchName: first.branchName, + metadata, + }, + issue: { + id: "issue-pr-prep", + identifier: "PAP-9003", + title: "Restore keeps branch ownership", + }, + agent: { + id: "agent-1", + name: "Codex Coder", + companyId: "company-1", + }, + }); + } + + const operatorOwned = await restoreWithMetadata({ createdByRuntime: false }); + expect(operatorOwned?.branchCreatedByRuntime).toBe(false); + + const runtimeOwned = await restoreWithMetadata({ createdByRuntime: true }); + expect(runtimeOwned?.branchCreatedByRuntime).toBe(true); + }); + it("fails closed when an existing branch is pinned on a non-worktree strategy", async () => { const repoRoot = await createTempRepo(); await createBranchWithCommit(repoRoot, "feature/wrong-mode", "wrong-mode.txt"); diff --git a/server/src/routes/projects.ts b/server/src/routes/projects.ts index 50b4be27853..c01a028ddb6 100644 --- a/server/src/routes/projects.ts +++ b/server/src/routes/projects.ts @@ -489,6 +489,7 @@ export function projectRoutes(db: Db) { worktreePath: null, warnings: [], created: false, + branchCreatedByRuntime: false, }, command: workspaceCommand.rawConfig, adapterEnv: {}, @@ -544,6 +545,7 @@ export function projectRoutes(db: Db) { worktreePath: null, warnings: [], created: false, + branchCreatedByRuntime: false, }, config: { workspaceRuntime: runtimeConfig }, adapterEnv: {}, diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 82ae608a41c..ded8b0e5d69 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -14951,7 +14951,10 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {}) ? reusableExistingExecutionWorkspace?.metadata ?? null : null, source: executionWorkspace.source, - createdByRuntime: executionWorkspace.created, + // Branch ownership, not worktree freshness: attaching a worktree to a + // pre-existing branch reports created=true but must never make terminal + // cleanup delete that operator-owned branch. + createdByRuntime: executionWorkspace.branchCreatedByRuntime, configSnapshot, shouldReuseExisting: resolvedWorkspaceReusePolicy.shouldRestoreExistingWorkspace, shouldRefreshConfigSnapshot: resolvedWorkspaceReusePolicy.shouldRefreshWorkspaceConfigSnapshot, diff --git a/server/src/services/workspace-realization.ts b/server/src/services/workspace-realization.ts index f4938decc39..8f908d33003 100644 --- a/server/src/services/workspace-realization.ts +++ b/server/src/services/workspace-realization.ts @@ -344,6 +344,7 @@ export function buildWorkspaceRealizationRecordFromDriverInput(input: { worktreePath: null, warnings: [], created: false, + branchCreatedByRuntime: false, }, workspaceConfig: null, }); diff --git a/server/src/services/workspace-runtime-exposure.test.ts b/server/src/services/workspace-runtime-exposure.test.ts index 4120e18f57e..4392f4a3c22 100644 --- a/server/src/services/workspace-runtime-exposure.test.ts +++ b/server/src/services/workspace-runtime-exposure.test.ts @@ -241,6 +241,7 @@ function startInput(options?: { worktreePath: null, warnings: [], created: false, + branchCreatedByRuntime: false, }, executionWorkspaceId: EXECUTION_WORKSPACE_ID, config: { diff --git a/server/src/services/workspace-runtime-ready-comment.test.ts b/server/src/services/workspace-runtime-ready-comment.test.ts index 214f5be79ed..c669b71ee3a 100644 --- a/server/src/services/workspace-runtime-ready-comment.test.ts +++ b/server/src/services/workspace-runtime-ready-comment.test.ts @@ -23,6 +23,7 @@ function workspace( worktreePath: "/repo/.paperclip/worktrees/PAP-16051", warnings: [], created: true, + branchCreatedByRuntime: true, ...overrides, }; } diff --git a/server/src/services/workspace-runtime.ts b/server/src/services/workspace-runtime.ts index 5e64b52ea82..765fdfcd0d1 100644 --- a/server/src/services/workspace-runtime.ts +++ b/server/src/services/workspace-runtime.ts @@ -163,6 +163,12 @@ export interface RealizedExecutionWorkspace extends ExecutionWorkspaceInput { worktreePath: string | null; warnings: string[]; created: boolean; + // Branch ownership, distinct from `created` (which reports a fresh worktree + // checkout). True only when this realization created the branch ref itself; + // attaching a worktree to a pre-existing branch keeps the branch + // operator-owned. Persisted as metadata.createdByRuntime, which terminal + // cleanup uses to decide whether the branch may be deleted. + branchCreatedByRuntime: boolean; baseRefSha?: string | null; pendingForwardBranchReconcile?: PendingForwardBranchReconcile | null; } @@ -3063,6 +3069,7 @@ export async function realizeExecutionWorkspace(input: { worktreePath: null, warnings: [], created: false, + branchCreatedByRuntime: false, baseRefSha: null, }; } @@ -3197,6 +3204,7 @@ export async function realizeExecutionWorkspace(input: { worktreePath: reusablePath, warnings: [...extraWarnings, ...baseRefreshWarnings, ...baseDrift.warnings], created: false, + branchCreatedByRuntime: false, baseRefSha: refresh.baseRefSha ?? baseDrift.branchBaseRefSha ?? baseDrift.currentBaseRefSha, pendingForwardBranchReconcile, }; @@ -3345,11 +3353,15 @@ export async function realizeExecutionWorkspace(input: { branchName, worktreePath, warnings: baseRefreshWarnings, + // The worktree is new, but the pinned branch pre-existed: it stays + // operator-owned so terminal cleanup never deletes it. created: true, + branchCreatedByRuntime: false, baseRefSha: currentBaseRefSha, }; } + let branchCreatedByRuntime = true; try { await recordGitOperation(input.recorder, { phase: "worktree_prepare", @@ -3387,6 +3399,9 @@ export async function realizeExecutionWorkspace(input: { successMessage: `Attached existing branch ${branchName} at ${worktreePath}\n`, failureLabel: `git worktree add ${worktreePath}`, }); + // The template rendered to a branch that already existed, so this + // attach did not create the branch and cleanup must not delete it. + branchCreatedByRuntime = false; } catch (attachError) { if (!gitErrorIncludes(attachError, "already checked out")) { throw attachError; @@ -3419,6 +3434,7 @@ export async function realizeExecutionWorkspace(input: { worktreePath, warnings: baseRefreshWarnings, created: true, + branchCreatedByRuntime, baseRefSha: currentBaseRefSha, }; } @@ -3469,6 +3485,10 @@ export async function ensurePersistedExecutionWorkspaceAvailable(input: { worktreePath: strategy === "git_worktree" ? (input.workspace.providerRef ?? cwd) : null, warnings: [], created: false, + // Restoring a persisted workspace keeps its recorded branch ownership, so + // a runtime-created branch stays cleanup-eligible across reuse heartbeats + // and an attached pre-existing branch stays operator-owned. + branchCreatedByRuntime: input.workspace.metadata?.createdByRuntime === true, baseRefSha: readRecordedBaseRefSha(input.workspace.metadata), }; const provisionCommand = asString(input.workspace.config?.provisionCommand, "").trim(); @@ -3677,6 +3697,7 @@ export async function ensurePersistedExecutionWorkspaceAvailable(input: { worktreePath, warnings: [...restoreRefreshWarnings, ...baseDrift.warnings], created, + branchCreatedByRuntime: realized.branchCreatedByRuntime || created, baseRefSha: recordedBaseRefSha ?? (created ? restoreCurrentBaseRefSha : baseDrift.branchBaseRefSha) @@ -3837,6 +3858,10 @@ export async function cleanupExecutionWorkspaceArtifacts(input: { warnings.push(`Could not read worktree instance pointer: ${err instanceof Error ? err.message : String(err)}`); } } + // Ownership signal persisted from RealizedExecutionWorkspace.branchCreatedByRuntime: + // true only when the runtime created the branch ref (or local_fs directory) + // itself. A worktree attached to a pre-existing branch persists false, so + // cleanup below removes the worktree but leaves the branch untouched. const createdByRuntime = input.workspace.metadata?.createdByRuntime === true; const cleanupCommands = input.runCleanupCommands === false ? [] @@ -7896,6 +7921,7 @@ export async function restartDesiredRuntimeServicesOnStartup(db: Db) { worktreePath: null, warnings: [], created: false, + branchCreatedByRuntime: false, }, config: { workspaceRuntime: runtimeConfig.workspaceRuntime, @@ -7950,6 +7976,7 @@ export async function restartDesiredRuntimeServicesOnStartup(db: Db) { worktreePath: row.strategyType === "git_worktree" ? row.cwd : null, warnings: [], created: false, + branchCreatedByRuntime: (row.metadata as Record | null)?.createdByRuntime === true, }, executionWorkspaceId: row.id, config: {