From ac15d883f7baeda25753aa7631c83a8d30d3d1e5 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Sat, 24 Oct 2026 19:26:27 -0700 Subject: [PATCH] fix: preserve remote-tracking history when expiring nested worktrees (#17535) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Nested project repositories are now worktrees of one shared clone per project repository, and existing standalone clones are converted in place > - Conversion keeps the clone's remote-tracking refs as task-owned branches (`paperclip//remotes/...`) so commits only they hold survive > - Expiration then required every task-owned branch, including those, to be on the remote > - Remote-tracking refs for branches deleted or force-pushed upstream never are, so converted checkouts were kept forever > - This pull request treats those refs the way the standalone-clone path already does: commits missing upstream go into the restore point instead of blocking > - The benefit is that converted workspaces can expire and free disk without losing any history ## Linked Issues or Issue Description Refs #17407, #17382, #17289. Bug report described in-PR (bug report template). **What happened?** On one instance, 17 closed workspaces in a two-repository project were kept on every expiration sweep with `unpushed_managed_checkout_branches`. Each nested checkout is a converted worktree whose task-owned refs include `paperclip//remotes/origin/` for upstream branches since deleted or force-pushed (dependabot, squashed feature branches). Their commits are on no remote, so `provenNestedWorktreeHistory` refused. The task's own branch was fully pushed. **Expected behavior** As in the standalone-clone path (#17289), remote-tracking commits missing upstream are preserved in the restore point, and only genuinely local branches block expiration. **Steps to reproduce** 1. Fetch an upstream branch into a standalone nested clone, then delete that branch upstream. 2. Convert the clone into a worktree (nested clone migration). 3. Close the issue and let expiration inspect it: kept with `unpushed_managed_checkout_branches`. **Paperclip version or commit:** `master` at cb47dbb2da. **Deployment mode:** any. **Agent adapter(s) involved:** not adapter-specific (core). ## What Changed - `provenNestedWorktreeHistory` in `server/src/services/git-workspace-restoration.ts`: - splits task-owned refs into those under `remotes/` and the rest; - still requires the rest (and their reflogs) to be on the remote; - returns commits under `remotes/` that are missing upstream, and their reflogs, as `preserve`, which the restore point bundles. This matches the standalone-clone path. - `restorePointHistoryDigest` in `server/src/services/git-restore-point.ts`: for a linked worktree, it fingerprints the task's own refs and their reflogs instead of the shared clone's `refs/remotes`. Other tasks change those whenever they fetch, and they never covered the task-owned remote refs. It loads `nested-repository-worktrees` lazily to avoid an import cycle. No code comments were added. ## Verification - `pnpm exec vitest run nested-clone-migration` (in `server/`). New cases: - A converted checkout whose only extra history is a remote-tracking commit deleted upstream expires with a restore point. After its refs are removed and the shared clone is garbage-collected, reopening restores the commit. - The digest ignores the shared clone's remote refs and changes when the task's own refs move. - **Mutation checks**, each failing its test: - reverting the prover split; - dropping the preserve list; - reverting the digest change. - The existing case where a converted checkout with a genuinely unpushed branch is kept still passes. - `pnpm exec vitest run git-restore-point nested-repository-worktrees nested-clone-migration git-workspace-expiration git-reproducible-managed-checkouts isolated-task-directory isolated-repository-rehome git-credentials execution-workspace` (in `server/`): 591 passed. - `pnpm run typecheck` (in `server/`): pass. - Not run: server build and boot smoke. ## Risks - **Misclassified branches:** a local branch literally named `remotes/...` in a standalone clone becomes `remotes/...` after conversion and is now preserved in the restore point instead of blocking expiration. Its commits are still saved, not lost. - **Leftover refs:** after expiry, the task's other owned refs stay in the shared clone, as they did before this change, so their commits stay in two places. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), 1M context, through Claude Code with tool use (shell, file edits, tests, read-only diagnosis on a live instance). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [ ] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- .../__tests__/nested-clone-migration.test.ts | 37 ++++++++++++++++++- server/src/services/git-restore-point.ts | 18 +++++++-- .../src/services/git-workspace-restoration.ts | 15 +++++--- .../services/nested-repository-worktrees.ts | 1 + 4 files changed, 62 insertions(+), 9 deletions(-) diff --git a/server/src/__tests__/nested-clone-migration.test.ts b/server/src/__tests__/nested-clone-migration.test.ts index e77222c4b9e..b417a4ce897 100644 --- a/server/src/__tests__/nested-clone-migration.test.ts +++ b/server/src/__tests__/nested-clone-migration.test.ts @@ -14,7 +14,7 @@ import { realizeExecutionWorkspace } from "../services/workspace-runtime.js"; import { executionWorkspaceService } from "../services/execution-workspaces.js"; import { prepareProjectRepositoryWorkspaces } from "../services/heartbeat/workspaces.js"; import { expireReconstructableGitWorkspace } from "../services/git-workspace-expiration.js"; -import { currentRestorePointSnapshot } from "../services/git-restore-point.js"; +import { currentRestorePointSnapshot, restorePointHistoryDigest } from "../services/git-restore-point.js"; import { nestedRepositoryBaseCandidates, nestedWorktreeBranchPrefix, readLinkedNestedCheckout, withNestedRepositoryBaseLock } from "../services/nested-repository-worktrees.js"; import { listNestedCloneMigrationNames, migrateNestedRepositoryClone, nextNestedCloneMigrationBatch, type NestedCloneMigrationOptions } from "../services/nested-clone-migration.js"; import { getStorageService } from "../storage/index.js"; @@ -225,6 +225,41 @@ describeEmbeddedPostgres("nested repository clone migration", () => { expect(git(f.checkout, "cat-file", "-t", feature)).toBe("commit"); }); + it("expires a converted checkout whose only extra history is remote-tracking and restores that history", async () => { + const f = await fixture({ issueStatus: "done" }); + git(f.nested.author, "checkout", "-q", "-b", "squashed-away"); + await fs.writeFile(path.join(f.nested.author, "feature.txt"), "feature\n"); + git(f.nested.author, "add", "."); git(f.nested.author, "commit", "-qm", "Feature"); + const feature = git(f.nested.author, "rev-parse", "HEAD"); + git(f.nested.author, "push", "-q", f.nested.remote, "squashed-away"); + git(f.checkout, "fetch", "-q", "origin"); + git(f.nested.remote, "branch", "-D", "squashed-away"); + git(f.nested.remote, "reflog", "expire", "--expire=now", "--all"); + git(f.nested.remote, "gc", "-q", "--prune=now"); + expect(await f.migrate()).toMatchObject({ outcome: "migrated" }); + + expect(await f.expire()).toMatchObject({ expired: true, restorePoint: true, skipReason: null }); + for (const line of ownedRefs(f.base, f.prefix).split("\n").filter(Boolean)) git(f.base, "update-ref", "-d", line.split(" ")[0]!); + git(f.base, "reflog", "expire", "--expire=now", "--all"); + git(f.base, "gc", "-q", "--prune=now"); + expect(() => git(f.base, "cat-file", "-e", `${feature}^{commit}`)).toThrow(); + expect(await f.reopen()).toMatchObject({ ok: true, reopened: true }); + expect(git(f.checkout, "cat-file", "-t", feature)).toBe("commit"); + }); + + it("fingerprints a converted checkout's own history rather than the shared clone's remote refs", async () => { + const f = await fixture(); + expect(await f.migrate()).toMatchObject({ outcome: "migrated" }); + const head = git(f.checkout, "rev-parse", "HEAD"); + const before = await restorePointHistoryDigest(f.checkout); + git(f.base, "update-ref", "refs/remotes/origin/another-task", head); + expect(await restorePointHistoryDigest(f.checkout)).toBe(before); + git(f.base, "update-ref", `refs/heads/${f.prefix}remotes/origin/extra`, head); + expect(await restorePointHistoryDigest(f.checkout)).not.toBe(before); + for (let index = 0; index < 64; index++) git(f.base, "update-ref", `refs/heads/${f.prefix}remotes/origin/bulk-${index}`, head); + await expect(restorePointHistoryDigest(f.checkout)).rejects.toThrow("managed_checkout_local_tip_budget"); + }); + it("finishes an interrupted conversion after the workspace is archived and forgets one whose workspace is gone", async () => { const f = await fixture(); const { unpushed } = await nestedWork(f.checkout); diff --git a/server/src/services/git-restore-point.ts b/server/src/services/git-restore-point.ts index 698b1395ade..3af9997b693 100644 --- a/server/src/services/git-restore-point.ts +++ b/server/src/services/git-restore-point.ts @@ -595,9 +595,21 @@ export async function restorePointHistoryDigest(input: string, signal?: AbortSig const cwd = await fs.realpath(input); const reflog = await gitText(cwd, ["reflog", "show", "--format=%H", "HEAD"], { signal }).catch(() => ""); const original = await gitText(cwd, ["rev-parse", "--verify", "-q", "ORIG_HEAD^{commit}"], { signal }).catch(() => ""); - const remotes = await gitText(cwd, ["for-each-ref", "--format=%(objectname) %(refname)", "refs/remotes"], { signal }).catch(() => ""); - const remoteReflogs = (await gitText(cwd, ["reflog", "show", "--all", "--format=%H %gD"], { signal }).catch(() => "")) - .split("\n").filter((line) => / refs\/remotes\//.test(line)); + const linked = (await fs.lstat(path.join(cwd, ".git")).catch(() => null))?.isFile() ?? false; + const nested = linked ? await import("./nested-repository-worktrees.js") : null; + const scope = nested ? `refs/heads/${await nested.nestedWorktreeBranchPrefix(cwd)}` : "refs/remotes"; + const remotes = await gitText(cwd, ["for-each-ref", ...nested ? [`--count=${nested.OWNED_NESTED_REF_LIMIT + 1}`] : [], + "--format=%(objectname) %(refname)", scope], { signal }).catch(() => ""); + const remoteReflogs: string[] = []; + if (nested) { + if (remotes.split("\n").filter(Boolean).length > nested.OWNED_NESTED_REF_LIMIT) throw new Error("managed_checkout_local_tip_budget"); + for (const ref of remotes.split("\n").map((line) => line.split(" ")[1]).filter(Boolean)) { + remoteReflogs.push(...(await gitText(cwd, ["reflog", "show", "--format=%H", ref!], { signal }).catch(() => "")).split("\n").filter((line) => SHA.test(line)).map((sha) => `${sha} ${ref}`)); + } + } else { + remoteReflogs.push(...(await gitText(cwd, ["reflog", "show", "--all", "--format=%H %gD"], { signal }).catch(() => "")) + .split("\n").filter((line) => / refs\/remotes\//.test(line))); + } return createHash("sha256").update(JSON.stringify([reflog.split("\n").filter((line) => SHA.test(line)), SHA.test(original) ? original : null, remotes.split("\n").filter(Boolean), remoteReflogs])).digest("hex"); } diff --git a/server/src/services/git-workspace-restoration.ts b/server/src/services/git-workspace-restoration.ts index f71c1851e7c..5aacb475a80 100644 --- a/server/src/services/git-workspace-restoration.ts +++ b/server/src/services/git-workspace-restoration.ts @@ -18,7 +18,7 @@ import { ensureGitRestorePointCommit, validateGitRestorePointRef, type GitRestor import { getStorageService } from "../storage/index.js"; import type { StorageService } from "../storage/types.js"; import { PROJECT_REPOSITORIES_DIR } from "@paperclipai/adapter-utils/git-workspace-sync"; -import { forgetExpiredNestedRepositoryWorktree, nestedRepositoryBaseCandidates, readLinkedNestedCheckout, recordedNestedRepositoryBase, type LinkedNestedCheckout } from "./nested-repository-worktrees.js"; +import { OWNED_NESTED_REF_LIMIT, forgetExpiredNestedRepositoryWorktree, nestedRepositoryBaseCandidates, readLinkedNestedCheckout, recordedNestedRepositoryBase, type LinkedNestedCheckout } from "./nested-repository-worktrees.js"; const execFile = promisify(execFileCallback); const shaPattern = /^[a-f0-9]{40}$/; @@ -249,7 +249,6 @@ async function assertCommitMessageSaved(cwd: string, gitDir: string, signal?: Ab if (!saved.includes(message.trim())) throw new Error("uncommitted_git_message"); } -const OWNED_NESTED_REF_LIMIT = 64; async function ownedNestedRefs(linked: LinkedNestedCheckout, signal?: AbortSignal) { const refs = (await gitCacheCommand(linked.checkout, ["for-each-ref", `--count=${OWNED_NESTED_REF_LIMIT + 1}`, @@ -380,9 +379,13 @@ async function provenNestedWorktreeHistory(linked: LinkedNestedCheckout, repoUrl const covered = new Set([head, ...shas(await gitCacheCommand(checkout, ["reflog", "show", "--format=%H", "HEAD"], signal).catch(() => "")), ...shas(await gitCacheCommand(checkout, ["rev-parse", "--verify", "-q", "ORIG_HEAD^{commit}"], signal).catch(() => ""))]); - const owned = await ownedNestedRefs(linked, signal); + const allOwned = await ownedNestedRefs(linked, signal); + const remotePrefix = `refs/heads/${linked.branchPrefix}remotes/`; + const owned = allOwned.filter(([, ref]) => !ref.startsWith(remotePrefix)); + const remoteTracked = [...new Set([...allOwned.filter(([, ref]) => ref.startsWith(remotePrefix)).map(([sha]) => sha), + ...await ownedNestedReflogs(linked, allOwned.filter(([, ref]) => ref.startsWith(remotePrefix)), signal)])]; const recorded = new Set([...owned.map(([sha]) => sha), ...await ownedNestedReflogs(linked, owned, signal)]); - if (recorded.size + covered.size > MANAGED_CHECKOUT_LOCAL_TIP_LIMIT) throw new Error("managed_checkout_local_tip_budget"); + if (recorded.size + covered.size + remoteTracked.length > MANAGED_CHECKOUT_LOCAL_TIP_LIMIT) throw new Error("managed_checkout_local_tip_budget"); const current = (await gitCacheCommand(checkout, ["symbolic-ref", "--quiet", "HEAD"], signal).catch(() => "")).trim(); const otherTips = owned.filter(([, ref]) => ref !== current).map(([sha]) => sha); if (otherTips.length && await hasCommitsMissingUpstream(checkout, otherTips, repoUrl, resolveGitAuth, signal, [head])) { @@ -393,7 +396,9 @@ async function provenNestedWorktreeHistory(linked: LinkedNestedCheckout, repoUrl throw new Error("unpushed_managed_checkout_branches"); } const headOnRemote = !await hasCommitsMissingUpstream(checkout, [head], repoUrl, resolveGitAuth, signal); - return { headOnRemote, preserve: [] }; + const preserve = remoteTracked.length + ? await tipsMissingUpstream(checkout, remoteTracked, repoUrl, resolveGitAuth, signal, [...covered, ...recorded]) : []; + return { headOnRemote, preserve }; } async function tipsMissingUpstream(checkout: string, tips: string[], repoUrl: string, diff --git a/server/src/services/nested-repository-worktrees.ts b/server/src/services/nested-repository-worktrees.ts index 8a2969ec2f3..ad907159cc3 100644 --- a/server/src/services/nested-repository-worktrees.ts +++ b/server/src/services/nested-repository-worktrees.ts @@ -15,6 +15,7 @@ import { sanitizeRuntimeServiceBaseEnv } from "./runtime-service-env.js"; const execFile = promisify(execFileCallback); const NESTED_WORKTREE_GIT_TIMEOUT_MS = 10 * 60 * 1000; const NESTED_WORKTREE_BRANCH = /^paperclip\/[a-f0-9]{12}\/./; +export const OWNED_NESTED_REF_LIMIT = 64; export const DETACHED_REPOSITORIES_DIR = path.join(".paperclip-runtime", "detached-repositories"); export type LinkedNestedCheckout = { checkout: string; base: string; gitDir: string; admin: string; branchPrefix: string };