From 992d687608f4f0d296deb704cfeccc43d1f3072a Mon Sep 17 00:00:00 2001 From: Dotta Date: Wed, 19 Aug 2026 08:19:06 +0000 Subject: [PATCH] feat(workspaces): pin PR-prep issues to an exact existing branch PR-preparation tasks could never attach an isolated execution workspace on a pre-existing branch: child-issue creation inherited the source issue's workspace binding (often the shared primary checkout), and the worktree runtime only knew template-derived branch names, silently creating a fresh branch off base when the rendered name did not exist. Add workspaceStrategy.existingBranch as the smallest explicit contract: - Shared type + issue validator accept the pin only with isolated mode and a git_worktree strategy, reject branchTemplate alongside it, and validate the branch-name shape. - realizeExecutionWorkspace gains an exact-branch mode: verify the branch exists (fail closed otherwise), reuse a registered worktree that already has it checked out (including legacy .worktrees/ paths), otherwise attach it under the managed worktree parent without -b. Exact mode never creates, renames, fast-forwards, resets, or reconciles the requested branch, and never falls back to project_primary or a derived branch. - Dispatch ignores an inherited reuse_existing workspace binding whose recorded branch does not equal the pinned branch and realizes the pinned branch instead. - scripts/repair-pr-prep-workspace-attachment.mjs converts stranded PR-prep issues to the new contract via the company-scoped API without touching any git branch. Co-authored-by: Paperclip Co-Authored-By: Claude Fable 5 --- doc/DEVELOPING.md | 2 + .../shared/src/types/workspace-runtime.ts | 6 + packages/shared/src/validators/issue.ts | 46 +++++- .../repair-pr-prep-workspace-attachment.mjs | 87 ++++++++++ .../execution-workspace-policy.test.ts | 48 ++++++ .../heartbeat-workspace-session.test.ts | 59 +++++++ .../src/__tests__/workspace-runtime.test.ts | 148 +++++++++++++++++ .../services/execution-workspace-policy.ts | 3 + server/src/services/heartbeat.ts | 16 +- server/src/services/issues.ts | 1 + server/src/services/workspace-runtime.ts | 156 +++++++++++++++++- 11 files changed, 560 insertions(+), 12 deletions(-) create mode 100644 scripts/repair-pr-prep-workspace-attachment.mjs diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index 82889da8e39..75f0c09e556 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -688,6 +688,8 @@ eval "$(npx paperclipai worktree env)" For project execution worktrees, Paperclip can also run a project-defined provision command after it creates or reuses an isolated git worktree. Configure this on the project's execution workspace policy (`workspaceStrategy.provisionCommand`). The command runs inside the derived worktree and receives `PAPERCLIP_WORKSPACE_*`, `PAPERCLIP_PROJECT_ID`, `PAPERCLIP_AGENT_ID`, and `PAPERCLIP_ISSUE_*` environment variables so each repo can bootstrap itself however it wants. +An issue can pin its isolated worktree to an exact pre-existing branch instead of a template-derived one — the contract PR-preparation tasks use. Set the issue's `executionWorkspaceSettings` to `{ "mode": "isolated_workspace", "workspaceStrategy": { "type": "git_worktree", "existingBranch": "" } }`. The validator requires isolated mode plus a `git_worktree` strategy and rejects `branchTemplate` alongside `existingBranch`. At dispatch the runtime attaches (never creates, renames, fast-forwards, or resets) that branch: it reuses a registered worktree that already has the branch checked out (including legacy `.worktrees/` paths), otherwise it attaches the branch under the managed worktree parent. A missing branch, an occupied worktree path on another branch, or a non-worktree strategy fails closed with a `workspace_validation_failed` error instead of falling back to the shared checkout or a derived branch, and an inherited `reuse_existing` workspace binding on a different branch is ignored in favor of realizing the pinned branch. + Heavier setup that is only needed by a managed runtime service can use `workspaceStrategy.runtimeProvisionCommand`. Paperclip runs this command lazily before spawning the first service in a start batch, serializes concurrent provisioning for the same workspace, and records the attempt as `workspace_runtime_provision`. The command receives the same workspace environment as `provisionCommand` and should be idempotent because later service-start batches invoke it again. Managed runtime control actions (`start`, `stop`, `restart`, and job `run`) are mutually exclusive per execution workspace. An overlapping control is rejected with `409 workspace_runtime_control_in_progress` instead of racing the active operation, and authorization is still checked first, so the conflict never widens who may control a workspace. diff --git a/packages/shared/src/types/workspace-runtime.ts b/packages/shared/src/types/workspace-runtime.ts index fcde0ef0fc3..a36fbaf162c 100644 --- a/packages/shared/src/types/workspace-runtime.ts +++ b/packages/shared/src/types/workspace-runtime.ts @@ -83,6 +83,12 @@ export interface ExecutionWorkspaceStrategy { type: ExecutionWorkspaceStrategyType; baseRef?: string | null; branchTemplate?: string | null; + /** + * Pin the worktree to this exact pre-existing branch instead of rendering + * `branchTemplate`. Realization attaches (never creates) the branch and + * fails closed when the branch does not exist or is not safely attachable. + */ + existingBranch?: string | null; worktreeParentDir?: string | null; provisionCommand?: string | null; runtimeProvisionCommand?: string | null; diff --git a/packages/shared/src/validators/issue.ts b/packages/shared/src/validators/issue.ts index 8eed625341c..28e3cb50c74 100644 --- a/packages/shared/src/validators/issue.ts +++ b/packages/shared/src/validators/issue.ts @@ -111,17 +111,50 @@ export const ISSUE_EXECUTION_WORKSPACE_PREFERENCES = [ "agent_default", ] as const; +// Mirrors git check-ref-format for a single branch name. The runtime verifies +// the branch actually exists before attaching, so this only rejects values +// that could never be a branch (path escapes, option injection, ref syntax). +export function isValidExistingBranchName(value: string): boolean { + if (value.length === 0 || value.length > 255) return false; + if (value.startsWith("-") || value.startsWith("/") || value.startsWith(".")) return false; + if (value.endsWith("/") || value.endsWith(".") || value.endsWith(".lock")) return false; + if (value.includes("..") || value.includes("//") || value.includes("@{") || value.includes("/.")) return false; + // eslint-disable-next-line no-control-regex + if (/[\x00-\x20\x7f~^:?*[\\]/.test(value)) return false; + return true; +} + const executionWorkspaceStrategySchema = z .object({ type: z.enum(["project_primary", "git_worktree", "adapter_managed", "cloud_sandbox"]).optional(), baseRef: z.string().optional().nullable(), branchTemplate: z.string().optional().nullable(), + existingBranch: z.string().trim().refine(isValidExistingBranchName, { + message: "existingBranch must be a valid git branch name", + }).optional().nullable(), worktreeParentDir: z.string().optional().nullable(), provisionCommand: z.string().optional().nullable(), runtimeProvisionCommand: z.string().optional().nullable(), teardownCommand: z.string().optional().nullable(), }) - .strict(); + .strict() + .superRefine((strategy, ctx) => { + if (!strategy.existingBranch) return; + if (strategy.type !== "git_worktree") { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ["existingBranch"], + message: "existingBranch requires workspaceStrategy.type \"git_worktree\"", + }); + } + if (strategy.branchTemplate) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ["existingBranch"], + message: "existingBranch and branchTemplate are mutually exclusive", + }); + } + }); const ipv4CidrPattern = /^(?:(?:25[0-5]|2[0-4]\d|[01]?\d\d?)\.){3}(?:25[0-5]|2[0-4]\d|[01]?\d\d?)\/(?:3[0-2]|[12]?\d)$/; const protectedTaskEgressCidrs = [ @@ -175,7 +208,16 @@ export const issueExecutionWorkspaceSettingsSchema = z )).max(100).optional(), }).strict().optional().nullable(), }) - .strict(); + .strict() + .superRefine((settings, ctx) => { + if (settings.workspaceStrategy?.existingBranch && settings.mode !== "isolated_workspace") { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ["workspaceStrategy", "existingBranch"], + message: "existingBranch requires mode \"isolated_workspace\"", + }); + } + }); export const issueAssigneeAdapterOverridesSchema = z .object({ diff --git a/scripts/repair-pr-prep-workspace-attachment.mjs b/scripts/repair-pr-prep-workspace-attachment.mjs new file mode 100644 index 00000000000..b442b80d32e --- /dev/null +++ b/scripts/repair-pr-prep-workspace-attachment.mjs @@ -0,0 +1,87 @@ +#!/usr/bin/env node +// Bounded repair for PR-preparation issues stranded on a shared or stale +// execution-workspace binding (PAP-17617). For each explicit issue=branch +// pair, this pins the issue's executionWorkspaceSettings to the exact-branch +// isolated-worktree contract: +// { mode: "isolated_workspace", +// workspaceStrategy: { type: "git_worktree", existingBranch: "" } } +// Dispatch then attaches a git worktree on exactly that branch and ignores a +// mismatched inherited reuse_existing binding. The script only PATCHes issues +// through the company-scoped API; it never runs git and never mutates or +// deletes any branch. +// +// Usage: +// node scripts/repair-pr-prep-workspace-attachment.mjs PAP-16555=PAP-14380-salvage-pap-9514 [more ISSUE=BRANCH ...] [--apply] +// +// Without --apply it reports the current binding and the patch it would send. +// Requires PAPERCLIP_API_URL and PAPERCLIP_API_KEY in the environment. + +const args = process.argv.slice(2); +const apply = args.includes("--apply"); +const pairs = args.filter((arg) => !arg.startsWith("--")).map((arg) => { + const separator = arg.indexOf("="); + if (separator <= 0 || separator === arg.length - 1) { + console.error(`Expected ISSUE=BRANCH, got "${arg}"`); + process.exit(1); + } + return { issue: arg.slice(0, separator), branch: arg.slice(separator + 1) }; +}); +if (pairs.length === 0) { + console.error("No ISSUE=BRANCH pairs given. Nothing to repair."); + process.exit(1); +} + +const apiUrl = process.env.PAPERCLIP_API_URL; +const apiKey = process.env.PAPERCLIP_API_KEY; +if (!apiUrl || !apiKey) { + console.error("PAPERCLIP_API_URL and PAPERCLIP_API_KEY are required."); + process.exit(1); +} +const apiBase = apiUrl.replace(/\/$/, "").replace(/\/api$/, ""); +const headers = { + Authorization: `Bearer ${apiKey}`, + "Content-Type": "application/json", + ...(process.env.PAPERCLIP_RUN_ID ? { "X-Paperclip-Run-Id": process.env.PAPERCLIP_RUN_ID } : {}), +}; + +let failed = false; +for (const { issue, branch } of pairs) { + const current = await fetch(`${apiBase}/api/issues/${issue}`, { headers }); + if (!current.ok) { + console.error(`${issue}: fetch failed (${current.status})`); + failed = true; + continue; + } + const existing = await current.json(); + const settings = { + mode: "isolated_workspace", + workspaceStrategy: { type: "git_worktree", existingBranch: branch }, + }; + console.log( + `${issue}: status=${existing.status} preference=${existing.executionWorkspacePreference ?? "null"} ` + + `workspace=${existing.executionWorkspaceId ?? "null"} settings=${JSON.stringify(existing.executionWorkspaceSettings)}`, + ); + if (!apply) { + console.log(`${issue}: would PATCH executionWorkspaceSettings=${JSON.stringify(settings)}`); + continue; + } + const response = await fetch(`${apiBase}/api/issues/${issue}`, { + method: "PATCH", + headers, + body: JSON.stringify({ + executionWorkspaceSettings: settings, + comment: + `Repair for PAP-17617/PAP-17618: pinned this PR-preparation task to an isolated git worktree on the exact ` + + `existing branch \`${branch}\` via workspaceStrategy.existingBranch. Dispatch will attach that branch ` + + `(never create or reset it) and will not reuse the stale inherited workspace binding.`, + }), + }); + const body = await response.text(); + if (!response.ok) { + console.error(`${issue}: PATCH failed (${response.status}): ${body.slice(0, 500)}`); + failed = true; + continue; + } + console.log(`${issue}: PATCHED — executionWorkspaceSettings now pins existingBranch=${branch}`); +} +process.exit(failed ? 1 : 0); diff --git a/server/src/__tests__/execution-workspace-policy.test.ts b/server/src/__tests__/execution-workspace-policy.test.ts index 3669dd4e2fe..2509d02994e 100644 --- a/server/src/__tests__/execution-workspace-policy.test.ts +++ b/server/src/__tests__/execution-workspace-policy.test.ts @@ -82,6 +82,54 @@ describe("execution workspace policy helpers", () => { }).success).toBe(false); }); + it("accepts an existing-branch pin only with isolated mode and a git_worktree strategy", () => { + expect(issueExecutionWorkspaceSettingsSchema.parse({ + mode: "isolated_workspace", + workspaceStrategy: { + type: "git_worktree", + existingBranch: "PAP-14380-salvage-pap-9514", + }, + }).workspaceStrategy?.existingBranch).toBe("PAP-14380-salvage-pap-9514"); + + // Fail closed at the contract layer: an exact-branch pin outside an + // isolated git worktree could silently land in the shared checkout. + expect(issueExecutionWorkspaceSettingsSchema.safeParse({ + workspaceStrategy: { type: "git_worktree", existingBranch: "some-branch" }, + }).success).toBe(false); + expect(issueExecutionWorkspaceSettingsSchema.safeParse({ + mode: "shared_workspace", + workspaceStrategy: { type: "git_worktree", existingBranch: "some-branch" }, + }).success).toBe(false); + expect(issueExecutionWorkspaceSettingsSchema.safeParse({ + mode: "isolated_workspace", + workspaceStrategy: { type: "project_primary", existingBranch: "some-branch" }, + }).success).toBe(false); + expect(issueExecutionWorkspaceSettingsSchema.safeParse({ + mode: "isolated_workspace", + workspaceStrategy: { + type: "git_worktree", + existingBranch: "some-branch", + branchTemplate: "{{issue.identifier}}-{{slug}}", + }, + }).success).toBe(false); + + for (const invalidBranch of ["-leading-dash", "a..b", "has space", "ends/", "back\\slash", "a.lock", "../escape"]) { + expect(issueExecutionWorkspaceSettingsSchema.safeParse({ + mode: "isolated_workspace", + workspaceStrategy: { type: "git_worktree", existingBranch: invalidBranch }, + }).success).toBe(false); + } + }); + + it("carries the existing-branch pin through issue settings parsing", () => { + expect( + parseIssueExecutionWorkspaceSettings({ + mode: "isolated_workspace", + workspaceStrategy: { type: "git_worktree", existingBranch: " PAP-14754-run-redaction " }, + })?.workspaceStrategy, + ).toEqual({ type: "git_worktree", existingBranch: "PAP-14754-run-redaction" }); + }); + it("centralizes unrunnable isolated worktree detection", () => { expect( isUnrunnableWorktreeCombo({ diff --git a/server/src/__tests__/heartbeat-workspace-session.test.ts b/server/src/__tests__/heartbeat-workspace-session.test.ts index 963c10ad2fb..0f934ef5794 100644 --- a/server/src/__tests__/heartbeat-workspace-session.test.ts +++ b/server/src/__tests__/heartbeat-workspace-session.test.ts @@ -1676,6 +1676,65 @@ describe("effective run execution workspace config freshness", () => { expect(realizeWorkspace).not.toHaveBeenCalled(); }); + it.each([ + { name: "a different branch", branchName: "PAP-9001-derived-child-branch" }, + { name: "no recorded branch", branchName: null }, + ])( + "realizes the pinned existing branch instead of reusing an inherited workspace on $name", + async ({ branchName }) => { + const reuseRequest = resolveExecutionWorkspaceReuseRequestForIssue({ + issueExecutionWorkspaceId: "workspace-old", + issueExecutionWorkspacePreference: "reuse_existing", + existingExecutionWorkspaceStatus: "active", + requestedExistingBranch: "PAP-14380-salvage-pap-9514", + existingExecutionWorkspaceBranchName: branchName, + }); + + expect(reuseRequest).toEqual({ + requestedExecutionWorkspaceId: "workspace-old", + requestedShouldReuseExisting: false, + existingExecutionWorkspaceAvailable: false, + }); + + const metadata = buildWorkspaceConfigMetadata(); + const decision = resolveExecutionWorkspaceConfigFreshness({ + hasExistingWorkspace: false, + existingWorkspaceMetadata: null, + nextMetadata: metadata, + }); + const realizeWorkspace = vi.fn(async () => ({ id: "pinned-branch-workspace", warnings: [] })); + const restoreExistingWorkspace = vi.fn(async () => ({ id: "workspace-old", warnings: [] })); + + const result = await provisionExecutionWorkspaceForFreshnessDecision({ + requestedShouldReuseExisting: reuseRequest.requestedShouldReuseExisting, + existingExecutionWorkspaceId: reuseRequest.requestedExecutionWorkspaceId, + issueRef: { id: "issue-1", identifier: "PAP-42" }, + runId: "run-1", + workspaceConfigFreshness: decision, + restoreExistingWorkspace, + realizeWorkspace, + }); + + expect(result.executionWorkspace).toEqual({ id: "pinned-branch-workspace", warnings: [] }); + expect(result.reusedExecutionWorkspace).toBeNull(); + expect(restoreExistingWorkspace).not.toHaveBeenCalled(); + }, + ); + + it("keeps reusing an inherited workspace whose branch matches the pinned existing branch", () => { + expect(resolveExecutionWorkspaceReuseRequestForIssue({ + issueExecutionWorkspaceId: "workspace-old", + issueExecutionWorkspacePreference: "reuse_existing", + existingExecutionWorkspaceStatus: "active", + requestedExistingBranch: "PAP-14380-salvage-pap-9514", + existingExecutionWorkspaceBranchName: "PAP-14380-salvage-pap-9514", + })).toEqual({ + requestedExecutionWorkspaceId: "workspace-old", + requestedShouldReuseExisting: true, + existingExecutionWorkspaceAvailable: true, + }); + }); + it("fails loudly when explicit reuse restore returns no workspace", async () => { const metadata = buildWorkspaceConfigMetadata(); const decision = resolveExecutionWorkspaceConfigFreshness({ diff --git a/server/src/__tests__/workspace-runtime.test.ts b/server/src/__tests__/workspace-runtime.test.ts index 8b122f78640..2fd730ae4df 100644 --- a/server/src/__tests__/workspace-runtime.test.ts +++ b/server/src/__tests__/workspace-runtime.test.ts @@ -8314,3 +8314,151 @@ describe("workspace realization request additionalSources", () => { expect(parsed?.runtimeOverlay.runtimeProvisionCommand).toBeNull(); }); }); + +describe("realizeExecutionWorkspace with an exact existing branch", () => { + function realizeExistingBranch( + repoRoot: string, + existingBranch: string, + strategyOverrides: Record = {}, + ) { + return realizeExecutionWorkspace({ + base: { + baseCwd: repoRoot, + source: "project_primary", + projectId: "project-1", + workspaceId: "workspace-1", + repoUrl: null, + repoRef: "HEAD", + }, + config: { + workspaceStrategy: { + type: "git_worktree", + existingBranch, + ...strategyOverrides, + }, + }, + issue: { + id: "issue-pr-prep", + identifier: "PAP-9001", + title: "Prepare pull request for an existing branch", + }, + agent: { + id: "agent-1", + name: "Codex Coder", + companyId: "company-1", + }, + }); + } + + async function createBranchWithCommit(repoRoot: string, branchName: string, fileName: string) { + const baseBranch = await readGit(repoRoot, ["rev-parse", "--abbrev-ref", "HEAD"]); + await runGit(repoRoot, ["checkout", "-b", branchName]); + await fs.writeFile(path.join(repoRoot, fileName), `${fileName}\n`, "utf8"); + await runGit(repoRoot, ["add", fileName]); + await runGit(repoRoot, ["commit", "-m", `Add ${fileName}`]); + await runGit(repoRoot, ["checkout", baseBranch]); + return readGit(repoRoot, ["rev-parse", branchName]); + } + + it("attaches an isolated worktree checked out on exactly the requested branch", async () => { + const repoRoot = await createTempRepo(); + const branchTip = await createBranchWithCommit(repoRoot, "feature/preexisting-work", "existing.txt"); + + const workspace = await realizeExistingBranch(repoRoot, "feature/preexisting-work"); + + expect(workspace.strategy).toBe("git_worktree"); + expect(workspace.branchName).toBe("feature/preexisting-work"); + 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"); + expect(await readGit(workspace.cwd, ["rev-parse", "HEAD"])).toBe(branchTip); + expect(await readGit(repoRoot, ["rev-parse", "feature/preexisting-work"])).toBe(branchTip); + }); + + it("reuses a registered legacy worktree that already has the branch checked out", async () => { + const repoRoot = await createTempRepo(); + const branchTip = await createBranchWithCommit(repoRoot, "feature/legacy-checkout", "legacy.txt"); + const legacyPath = path.join(repoRoot, ".worktrees", "legacy-checkout"); + await runGit(repoRoot, ["worktree", "add", legacyPath, "feature/legacy-checkout"]); + + const workspace = await realizeExistingBranch(repoRoot, "feature/legacy-checkout"); + + expect(workspace.cwd).toBe(path.resolve(legacyPath)); + expect(workspace.branchName).toBe("feature/legacy-checkout"); + expect(workspace.created).toBe(false); + expect(await readGit(workspace.cwd, ["rev-parse", "HEAD"])).toBe(branchTip); + expect(await readGit(repoRoot, ["rev-parse", "feature/legacy-checkout"])).toBe(branchTip); + }); + + it("fails closed when the requested branch does not exist and creates nothing", async () => { + const repoRoot = await createTempRepo(); + + await expect(realizeExistingBranch(repoRoot, "feature/never-created")).rejects.toMatchObject({ + code: "workspace_validation_failed", + resultJson: { + workspaceValidation: expect.objectContaining({ + reason: "existing_branch_not_found", + requestedExistingBranch: "feature/never-created", + }), + }, + }); + + expect(await readGit(repoRoot, ["branch", "--list", "feature/never-created"])).toBe(""); + expect( + existsSync(path.join(repoRoot, ".paperclip", "worktrees", "feature", "never-created")), + ).toBe(false); + }); + + it("fails closed instead of reconciling when the managed worktree path holds another branch", async () => { + const repoRoot = await createTempRepo(); + const branchTip = await createBranchWithCommit(repoRoot, "feature/pinned", "pinned.txt"); + const managedPath = path.join(repoRoot, ".paperclip", "worktrees", "feature/pinned"); + await runGit(repoRoot, ["worktree", "add", "-b", "stale-occupant", managedPath]); + + await expect(realizeExistingBranch(repoRoot, "feature/pinned")).rejects.toMatchObject({ + code: "workspace_validation_failed", + resultJson: { + workspaceValidation: expect.objectContaining({ + reason: "existing_branch_worktree_not_reusable", + requestedExistingBranch: "feature/pinned", + }), + }, + }); + + expect(await readGit(repoRoot, ["rev-parse", "feature/pinned"])).toBe(branchTip); + expect(await readGit(managedPath, ["branch", "--show-current"])).toBe("stale-occupant"); + }); + + it("never fast-forwards an unstarted exact branch to a newer base ref", async () => { + const { sourceRepo, remotePath, repoRoot } = await createClonedRepoWithRemote(); + const oldMaster = await readGit(repoRoot, ["rev-parse", "origin/master"]); + await runGit(repoRoot, ["branch", "release/frozen", oldMaster]); + + const first = await realizeExistingBranch(repoRoot, "release/frozen", { baseRef: "origin/master" }); + expect(await readGit(first.cwd, ["rev-parse", "HEAD"])).toBe(oldMaster); + + const newMaster = await advanceRemoteMaster(sourceRepo, remotePath, "advance.txt"); + const reused = await realizeExistingBranch(repoRoot, "release/frozen", { baseRef: "origin/master" }); + + expect(reused.cwd).toBe(first.cwd); + expect(await readGit(reused.cwd, ["rev-parse", "HEAD"])).toBe(oldMaster); + expect(await readGit(repoRoot, ["rev-parse", "release/frozen"])).toBe(oldMaster); + expect(newMaster).not.toBe(oldMaster); + }); + + 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"); + + await expect( + realizeExistingBranch(repoRoot, "feature/wrong-mode", { type: "project_primary" }), + ).rejects.toMatchObject({ + code: "workspace_validation_failed", + resultJson: { + workspaceValidation: expect.objectContaining({ + reason: "existing_branch_requires_git_worktree", + }), + }, + }); + }); +}); diff --git a/server/src/services/execution-workspace-policy.ts b/server/src/services/execution-workspace-policy.ts index 1c6b3ccc93e..0205950fc18 100644 --- a/server/src/services/execution-workspace-policy.ts +++ b/server/src/services/execution-workspace-policy.ts @@ -40,6 +40,9 @@ function parseExecutionWorkspaceStrategy(raw: unknown): ExecutionWorkspaceStrate type, ...(typeof parsed.baseRef === "string" ? { baseRef: parsed.baseRef } : {}), ...(typeof parsed.branchTemplate === "string" ? { branchTemplate: parsed.branchTemplate } : {}), + ...(typeof parsed.existingBranch === "string" && parsed.existingBranch.trim().length > 0 + ? { existingBranch: parsed.existingBranch.trim() } + : {}), ...(typeof parsed.worktreeParentDir === "string" ? { worktreeParentDir: parsed.worktreeParentDir } : {}), ...(typeof parsed.provisionCommand === "string" ? { provisionCommand: parsed.provisionCommand } : {}), ...(typeof parsed.runtimeProvisionCommand === "string" diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index cbeb0978f29..82ae608a41c 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -4389,10 +4389,22 @@ export function resolveExecutionWorkspaceReuseRequestForIssue(input: { issueExecutionWorkspaceId?: string | null; issueExecutionWorkspacePreference?: string | null; existingExecutionWorkspaceStatus?: string | null; + requestedExistingBranch?: string | null; + existingExecutionWorkspaceBranchName?: string | null; }): ExecutionWorkspaceReuseRequestForIssue { const requestedExecutionWorkspaceId = readNonEmptyString(input.issueExecutionWorkspaceId); + // An explicitly pinned existing branch outranks an inherited reuse_existing + // binding: a persisted workspace on any other branch (or with no recorded + // branch) is stale for this issue, so dispatch realizes the pinned branch + // instead of restoring the mismatched workspace. + const requestedExistingBranch = readNonEmptyString(input.requestedExistingBranch); + const existingWorkspaceMatchesRequestedBranch = + requestedExistingBranch === null || + readNonEmptyString(input.existingExecutionWorkspaceBranchName) === requestedExistingBranch; const requestedShouldReuseExisting = - input.issueExecutionWorkspacePreference === "reuse_existing" && requestedExecutionWorkspaceId !== null; + input.issueExecutionWorkspacePreference === "reuse_existing" && + requestedExecutionWorkspaceId !== null && + existingWorkspaceMatchesRequestedBranch; return { requestedExecutionWorkspaceId, @@ -14345,6 +14357,8 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {}) issueExecutionWorkspaceId: requestedExecutionWorkspaceId, issueExecutionWorkspacePreference: issueRef?.executionWorkspacePreference ?? null, existingExecutionWorkspaceStatus: existingExecutionWorkspace?.status ?? null, + requestedExistingBranch: issueExecutionWorkspaceSettings?.workspaceStrategy?.existingBranch ?? null, + existingExecutionWorkspaceBranchName: existingExecutionWorkspace?.branchName ?? null, }); const requestedShouldReuseExisting = workspaceReuseRequest.requestedShouldReuseExisting; const reusableExistingExecutionWorkspace = workspaceReuseRequest.existingExecutionWorkspaceAvailable diff --git a/server/src/services/issues.ts b/server/src/services/issues.ts index 93a57137dcd..00d0c088b42 100644 --- a/server/src/services/issues.ts +++ b/server/src/services/issues.ts @@ -338,6 +338,7 @@ function buildPreRealizationExecutionWorkspaceSettings(raw: unknown): Record { const rawStrategy = parseObject(input.config.workspaceStrategy); const strategyType = asString(rawStrategy.type, "project_primary"); + const requestedExistingBranch = asString(rawStrategy.existingBranch, "").trim(); if (strategyType !== "git_worktree") { + if (requestedExistingBranch) { + throw new WorkspaceRuntimeValidationFailure( + `Workspace strategy pins existing branch "${requestedExistingBranch}" but has type "${strategyType}"; an exact-branch workspace requires strategy type "git_worktree". Set workspaceStrategy.type to "git_worktree" or remove existingBranch.`, + { + workspaceValidation: { + reason: "existing_branch_requires_git_worktree", + requestedExistingBranch, + strategyType, + }, + }, + ); + } return { ...input.base, strategy: "project_primary", @@ -3055,19 +3068,56 @@ export async function realizeExecutionWorkspace(input: { } const repoRoot = await resolveGitOwnerRepoRoot(input.base.baseCwd); - const branchTemplate = asString(rawStrategy.branchTemplate, "{{issue.identifier}}-{{slug}}"); - const renderedBranch = renderWorkspaceTemplate(branchTemplate, { - issue: input.issue, - agent: input.agent, - projectId: input.base.projectId, - repoRef: input.base.repoRef, - }); - let branchName = sanitizeBranchName(renderedBranch); + let branchName: string; + if (requestedExistingBranch) { + // Exact-branch mode: attach the requested pre-existing branch verbatim. + // The branch must already exist; realization never creates, renames, or + // resets it, and any mismatch below fails closed instead of falling back + // to a derived branch or the shared checkout. + const existingBranchSha = await runGit( + ["rev-parse", "--verify", "--quiet", `refs/heads/${requestedExistingBranch}`], + repoRoot, + ).catch(() => null); + if (!existingBranchSha) { + throw new WorkspaceRuntimeValidationFailure( + `Workspace strategy pins existing branch "${requestedExistingBranch}", but no local branch with that name exists in "${repoRoot}". Create or fetch the branch first, or remove workspaceStrategy.existingBranch; exact-branch realization never creates a branch.`, + { + workspaceValidation: { + reason: "existing_branch_not_found", + requestedExistingBranch, + repoRoot, + }, + }, + ); + } + branchName = requestedExistingBranch; + } else { + const branchTemplate = asString(rawStrategy.branchTemplate, "{{issue.identifier}}-{{slug}}"); + const renderedBranch = renderWorkspaceTemplate(branchTemplate, { + issue: input.issue, + agent: input.agent, + projectId: input.base.projectId, + repoRef: input.base.repoRef, + }); + branchName = sanitizeBranchName(renderedBranch); + } const configuredParentDir = asString(rawStrategy.worktreeParentDir, ""); const worktreeParentDir = configuredParentDir ? resolveConfiguredPath(configuredParentDir, repoRoot) : path.join(repoRoot, ".paperclip", "worktrees"); const worktreePath = path.join(worktreeParentDir, branchName); + if (path.relative(worktreeParentDir, worktreePath).startsWith("..")) { + throw new WorkspaceRuntimeValidationFailure( + `Workspace branch "${branchName}" resolves to a worktree path outside the managed worktree parent directory "${worktreeParentDir}".`, + { + workspaceValidation: { + reason: "worktree_path_escapes_parent_dir", + branchName, + worktreeParentDir, + }, + }, + ); + } let pendingForwardBranchReconcile: PendingForwardBranchReconcile | null = null; const configuredBaseRef = typeof rawStrategy.baseRef === "string" && rawStrategy.baseRef.length > 0 ? rawStrategy.baseRef @@ -3086,7 +3136,9 @@ export async function realizeExecutionWorkspace(input: { await fs.mkdir(worktreeParentDir, { recursive: true }); async function reuseExistingWorktree(reusablePath: string, effectiveBranchName = branchName, extraWarnings: string[] = []) { - const refresh = currentBaseRefSha + // An exact-branch attach must never move the requested branch, so skip + // the unstarted-worktree fast-forward that template-derived reuse gets. + const refresh = currentBaseRefSha && !requestedExistingBranch ? await refreshUnstartedWorktreeToBase({ repoRoot, worktreePath: reusablePath, @@ -3157,6 +3209,11 @@ export async function realizeExecutionWorkspace(input: { expectedBranchName: branchName, }).catch(() => null); if (validation && !validation.valid && validation.reasonCode === "branch_mismatch") { + if (requestedExistingBranch) { + // Exact-branch mode never reconciles a mismatched checkout onto + // another branch; the caller fails closed with the mismatch reason. + return { validation, branchName, warnings: [] }; + } const coherence = await ensureGitWorktreeBranchCoherent({ db: input.db ?? null, repoRoot, @@ -3198,6 +3255,19 @@ export async function realizeExecutionWorkspace(input: { } const validation = reusable.validation; const reason = validation && !validation.valid ? ` (${validation.reason})` : ""; + if (requestedExistingBranch) { + throw new WorkspaceRuntimeValidationFailure( + `Workspace strategy pins existing branch "${requestedExistingBranch}", but the worktree path "${worktreePath}" already exists and is not a reusable checkout of that branch${reason}. Repair or remove that worktree, then retry; exact-branch realization never reconciles it onto another branch.`, + { + workspaceValidation: { + reason: "existing_branch_worktree_not_reusable", + reasonCode: validation && !validation.valid ? validation.reasonCode : null, + requestedExistingBranch, + worktreePath, + }, + }, + ); + } throw new Error(`Configured worktree path "${worktreePath}" already exists and is not a reusable git worktree${reason}.`); } @@ -3209,9 +3279,77 @@ export async function realizeExecutionWorkspace(input: { } const validation = reusable.validation; const reason = validation && !validation.valid ? ` (${validation.reason})` : ""; + if (requestedExistingBranch) { + throw new WorkspaceRuntimeValidationFailure( + `Workspace strategy pins existing branch "${requestedExistingBranch}", which is already checked out at "${registeredBranchWorktree}", but that worktree is not reusable${reason}. Repair or remove that worktree, then retry.`, + { + workspaceValidation: { + reason: "existing_branch_worktree_not_reusable", + reasonCode: validation && !validation.valid ? validation.reasonCode : null, + requestedExistingBranch, + worktreePath: registeredBranchWorktree, + }, + }, + ); + } throw new Error(`Registered worktree for branch "${branchName}" at "${registeredBranchWorktree}" is not reusable${reason}.`); } + if (requestedExistingBranch) { + try { + await recordGitOperation(input.recorder, { + phase: "worktree_prepare", + args: ["worktree", "add", worktreePath, branchName], + cwd: repoRoot, + metadata: { + repoRoot, + worktreePath, + branchName, + baseRef, + baseRefSha: currentBaseRefSha, + created: false, + attachedExistingBranch: true, + }, + successMessage: `Attached existing branch ${branchName} at ${worktreePath}\n`, + failureLabel: `git worktree add ${worktreePath}`, + }); + } catch (attachError) { + const message = attachError instanceof Error ? attachError.message : String(attachError); + throw new WorkspaceRuntimeValidationFailure( + `Could not attach existing branch "${requestedExistingBranch}" as a git worktree at "${worktreePath}": ${message}`, + { + workspaceValidation: { + reason: "existing_branch_attach_failed", + requestedExistingBranch, + worktreePath, + }, + }, + ); + } + await provisionExecutionWorktree({ + strategy: rawStrategy, + base: input.base, + repoRoot, + worktreePath, + branchName, + issue: input.issue, + agent: input.agent, + created: true, + recorder: input.recorder ?? null, + }); + return { + ...input.base, + repoRef: baseRef, + strategy: "git_worktree", + cwd: worktreePath, + branchName, + worktreePath, + warnings: baseRefreshWarnings, + created: true, + baseRefSha: currentBaseRefSha, + }; + } + try { await recordGitOperation(input.recorder, { phase: "worktree_prepare",