fix(adapter-utils): graft unrelated imported histories instead of failing the run (#11638)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - At run finalize, the host imports the sandbox git history and reconciles it with the local worktree in `integrateImportedGitHead` > - Transported workspaces are depth-1 shallow clones, so the boundary commit reads as parentless inside the sandbox > - A `git commit --amend` there rewrites the boundary commit into a root commit, and the re-imported history no longer connects to the host history > - `git merge-tree` has no common base to merge against, so the sync throws "Failed to merge concurrent remote git histories" and the run fails with its work stranded in the sandbox > - This pull request grafts the imported tree onto the current head as a single commit instead of failing > - The benefit is that a history rewrite inside the sandbox can no longer lose a run's work ## Linked Issues or Issue Description No existing issue found. I searched issues and PRs for "unrelated histories", "Failed to merge concurrent", and "shallow". Depends on #11637 (merged; the graft commit reuses its identity constant). This PR is now rebased onto `master`. **What happened?** An agent run amended a commit inside its sandbox workspace to address review feedback. The sandbox clone is depth-1 shallow, so git treated the boundary commit as parentless and the amend produced a root commit. At finalize, the host-side sync failed with `Failed to merge concurrent remote git histories for <sha>` and the run was marked failed. A follow-up run had to repair the branch by hand: fetch the true parent from origin and rebuild the commit with `git commit-tree`. **Expected behavior** The sync must never strand completed work. When the imported history shares no ancestor with the local one, the imported tree should still land on the current head, with the imported message preserved and the graft recorded. **Steps to reproduce** 1. Start a run whose workspace transport uses the shallow clone path (`withShallowGitWorkspaceClone`, depth 1). 2. Inside the sandbox workspace, run `git commit --amend` on the boundary commit. The result is a parentless root commit. 3. Finish the run. The host-side `integrateImportedGitHead` finds no merge base, `merge-tree` fails, and the run fails. ## What Changed - `git-workspace-sync.ts`: new exported `createUnrelatedHistoryGraftCommit` helper. It reads the imported head's tree and message, and creates one commit on top of the current head with the deterministic sync identity and a trailer that records the graft and both shas. - `integrateImportedGitHead` (both the remote-git-sync version and the SSH copy in `ssh.ts`): when `merge-base` reports no common ancestor, graft instead of throwing. The ref update keeps the same compare-and-swap and concurrent-retry semantics as the merge path. - The graft is gated on `git merge-base` exiting with status 1 — the no-ancestor signal. Operational failures (timeout, missing object, repository error) keep the loud merge failure instead of rewriting the tip. - New regression tests: one builds the exact shallow-amend shape (a root commit rebuilt from the base tree) and asserts the graft lands on the current head with the imported tree, subject, and graft trailer; one integrates a well-formed sha the repository does not hold and asserts the integration still throws with the branch tip unchanged. ## Verification - `pnpm vitest run packages/adapter-utils/src/git-workspace-sync.test.ts` — 19/19 pass (includes the new graft test and the merge-base failure-discrimination test). - `pnpm --filter @paperclipai/adapter-utils typecheck` — clean. - Full `pnpm vitest run packages/adapter-utils`: every file passes except `local-process-sandbox.test.ts`, which fails identically on an untouched `master` checkout on macOS (bubblewrap-dependent, pre-existing, unrelated). ## Risks - Behavioral shift: unrelated imported histories previously failed the integration; now they land as a squash-graft. In this degenerate case there is no base to merge against, so the imported tree is taken wholesale and concurrent local-only tree changes are superseded at the tip. The local commits keep their place in the graft's ancestry, and the trailer records both shas, so nothing is unrecoverable. The old behavior lost the imported work instead, which is the worse failure for an autonomous run. - The graft reuses the imported head's commit message, so branch history still reads naturally after a sandbox rewrite. ## Model Used - Claude Fable 5 (`claude-fable-5`), extended thinking, via Claude Code CLI (tool use for code exploration, test runs, and verification). ## 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 (no doc surface describes this internal sync path) - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge
This commit is contained in:
Devin Foley
GitHub
parent
9ea8143c87
commit
393da0f67c
|
|
@ -477,6 +477,63 @@ describe("git workspace sync", () => {
|
|||
expect(mergedTree).toContain("local.txt");
|
||||
expect(mergedTree).toContain("imported.txt");
|
||||
});
|
||||
|
||||
it("grafts an imported head onto the current head when histories share no ancestor", async () => {
|
||||
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-git-graft-"));
|
||||
cleanupDirs.push(rootDir);
|
||||
const setupIdentity = ["-c", "user.name=Setup", "-c", "user.email=setup@paperclip.dev"];
|
||||
const repo = path.join(rootDir, "repo");
|
||||
await mkdir(repo, { recursive: true });
|
||||
await git(repo, ["init"]);
|
||||
await git(repo, ["checkout", "-b", "main"]);
|
||||
await writeFile(path.join(repo, "tracked.txt"), "base\n", "utf8");
|
||||
await git(repo, ["add", "tracked.txt"]);
|
||||
await git(repo, [...setupIdentity, "commit", "-m", "base"]);
|
||||
const baseHead = await git(repo, ["rev-parse", "HEAD"]);
|
||||
|
||||
await writeFile(path.join(repo, "local.txt"), "local\n", "utf8");
|
||||
await git(repo, ["add", "local.txt"]);
|
||||
await git(repo, [...setupIdentity, "commit", "-m", "local advance"]);
|
||||
const currentHead = await git(repo, ["rev-parse", "HEAD"]);
|
||||
|
||||
// The shape a depth-1 shallow clone produces after `git commit --amend`:
|
||||
// a parentless root commit that shares no ancestor with the host history.
|
||||
const importedTree = await git(repo, ["rev-parse", `${baseHead}^{tree}`]);
|
||||
const importedHead = await git(repo, [...setupIdentity, "commit-tree", importedTree, "-m", "sandbox rewrite"]);
|
||||
|
||||
await integrateImportedGitHead({ localDir: repo, importedHead });
|
||||
|
||||
const parents = (await git(repo, ["rev-list", "--parents", "-1", "HEAD"])).split(" ");
|
||||
expect(parents.slice(1)).toEqual([currentHead]);
|
||||
// The imported tree is taken wholesale: no base exists to merge against.
|
||||
expect(await git(repo, ["rev-parse", "HEAD^{tree}"])).toBe(importedTree);
|
||||
expect(await git(repo, ["log", "-1", "--format=%s"])).toBe("sandbox rewrite");
|
||||
const body = await git(repo, ["log", "-1", "--format=%B"]);
|
||||
expect(body).toContain(`Paperclip remote git sync graft ${importedHead.slice(0, 12)}`);
|
||||
expect(body).toContain("shares no ancestor");
|
||||
});
|
||||
|
||||
it("does not graft when merge-base fails for a reason other than missing ancestry", async () => {
|
||||
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-git-no-graft-"));
|
||||
cleanupDirs.push(rootDir);
|
||||
const setupIdentity = ["-c", "user.name=Setup", "-c", "user.email=setup@paperclip.dev"];
|
||||
const repo = path.join(rootDir, "repo");
|
||||
await mkdir(repo, { recursive: true });
|
||||
await git(repo, ["init"]);
|
||||
await git(repo, ["checkout", "-b", "main"]);
|
||||
await writeFile(path.join(repo, "tracked.txt"), "base\n", "utf8");
|
||||
await git(repo, ["add", "tracked.txt"]);
|
||||
await git(repo, [...setupIdentity, "commit", "-m", "base"]);
|
||||
const currentHead = await git(repo, ["rev-parse", "HEAD"]);
|
||||
|
||||
// A well-formed sha the repository does not hold: merge-base fails with an
|
||||
// object error (exit 128), not the no-ancestor signal (exit 1). The graft
|
||||
// must not fire, and the integration keeps its loud failure.
|
||||
const missingHead = "0123456789abcdef0123456789abcdef01234567";
|
||||
await expect(integrateImportedGitHead({ localDir: repo, importedHead: missingHead }))
|
||||
.rejects.toThrow(/Failed to merge concurrent remote git histories/);
|
||||
expect(await git(repo, ["rev-parse", "HEAD"])).toBe(currentHead);
|
||||
});
|
||||
});
|
||||
|
||||
describe("sanitizeGitRemoteUrl", () => {
|
||||
|
|
|
|||
|
|
@ -433,6 +433,51 @@ export function buildRemoteGitDeltaBundleScript(input: {
|
|||
].filter(Boolean).join("\n");
|
||||
}
|
||||
|
||||
/**
|
||||
* Preserve imported work whose history does not connect to the local one.
|
||||
*
|
||||
* The dominant real-world cause is a history rewrite inside a transported
|
||||
* workspace: transported clones are depth-1 shallow, so the boundary commit
|
||||
* reads as parentless there and `git commit --amend` rewrites it into a root
|
||||
* commit that shares no ancestor with the host history. A tree merge is
|
||||
* impossible without a common ancestor, and failing the integration would
|
||||
* discard the run's work. Instead, squash-graft the imported tree onto the
|
||||
* current head as a single commit that reuses the imported head's message,
|
||||
* with a trailer recording the graft. Concurrent local-only commits keep
|
||||
* their place in history as the graft's ancestry; the imported tree is taken
|
||||
* wholesale because no base exists to merge against. The caller advances the
|
||||
* branch ref to the returned commit.
|
||||
*/
|
||||
export async function createUnrelatedHistoryGraftCommit(input: {
|
||||
localDir: string;
|
||||
currentHead: string;
|
||||
importedHead: string;
|
||||
syncLabel: string;
|
||||
}): Promise<string> {
|
||||
const importedTree = (await runLocalGit(input.localDir, ["rev-parse", `${input.importedHead}^{tree}`], {
|
||||
timeout: 10_000,
|
||||
maxBuffer: 16 * 1024,
|
||||
})).stdout.trim();
|
||||
const importedMessage = (await runLocalGit(input.localDir, ["log", "-1", "--format=%B", input.importedHead], {
|
||||
timeout: 10_000,
|
||||
maxBuffer: 256 * 1024,
|
||||
})).stdout;
|
||||
const message = [
|
||||
importedMessage.trim(),
|
||||
"",
|
||||
`(${input.syncLabel} graft ${input.importedHead.slice(0, 12)}: imported history shares no ancestor with ${input.currentHead.slice(0, 12)})`,
|
||||
].join("\n");
|
||||
const graftCommit = await runLocalGit(
|
||||
input.localDir,
|
||||
[...GIT_SYNC_COMMIT_IDENTITY_ARGS, "commit-tree", importedTree, "-p", input.currentHead, "-m", message],
|
||||
{
|
||||
timeout: 60_000,
|
||||
maxBuffer: 64 * 1024,
|
||||
},
|
||||
);
|
||||
return graftCommit.stdout.trim();
|
||||
}
|
||||
|
||||
export async function integrateImportedGitHead(input: {
|
||||
localDir: string;
|
||||
importedHead: string;
|
||||
|
|
@ -450,10 +495,18 @@ export async function integrateImportedGitHead(input: {
|
|||
if (!currentHead || currentHead === input.importedHead) return;
|
||||
|
||||
const headRef = snapshot.branchName ? `refs/heads/${snapshot.branchName}` : "HEAD";
|
||||
// `git merge-base` exits 1 when the commits share no ancestor — the only
|
||||
// outcome that authorizes the graft fallback below. Every other failure
|
||||
// (timeout, missing object, repository error) must keep failing the
|
||||
// integration instead of silently rewriting the tip.
|
||||
let noCommonAncestor = false;
|
||||
const mergeBase = await runLocalGit(input.localDir, ["merge-base", currentHead, input.importedHead], {
|
||||
timeout: 10_000,
|
||||
maxBuffer: 16 * 1024,
|
||||
}).catch(() => null);
|
||||
}).catch((error: unknown) => {
|
||||
noCommonAncestor = (error as { code?: unknown } | null)?.code === 1;
|
||||
return null;
|
||||
});
|
||||
const mergeBaseHead = mergeBase?.stdout.trim() ?? "";
|
||||
|
||||
if (mergeBaseHead === input.importedHead) {
|
||||
|
|
@ -473,6 +526,28 @@ export async function integrateImportedGitHead(input: {
|
|||
}
|
||||
}
|
||||
|
||||
if (noCommonAncestor) {
|
||||
// No common ancestor — merging is impossible and failing here would
|
||||
// discard the imported work. Graft it onto the current head instead;
|
||||
// see createUnrelatedHistoryGraftCommit.
|
||||
const graftCommit = await createUnrelatedHistoryGraftCommit({
|
||||
localDir: input.localDir,
|
||||
currentHead,
|
||||
importedHead: input.importedHead,
|
||||
syncLabel: "Paperclip remote git sync",
|
||||
});
|
||||
try {
|
||||
await runLocalGit(input.localDir, ["update-ref", headRef, graftCommit, currentHead], {
|
||||
timeout: 10_000,
|
||||
maxBuffer: 16 * 1024,
|
||||
});
|
||||
return;
|
||||
} catch (error) {
|
||||
if (isConcurrentRefUpdateError(error) && attempt < 4) continue;
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
let mergedTree;
|
||||
try {
|
||||
mergedTree = await runLocalGit(input.localDir, ["merge-tree", "--write-tree", currentHead, input.importedHead], {
|
||||
|
|
|
|||
|
|
@ -6,7 +6,11 @@ import os from "node:os";
|
|||
import path from "node:path";
|
||||
import { Transform } from "node:stream";
|
||||
import type { CommandManagedRuntimeRunner } from "./command-managed-runtime.js";
|
||||
import { GIT_SYNC_COMMIT_IDENTITY_ARGS, readSanitizedOriginRemoteUrl } from "./git-workspace-sync.js";
|
||||
import {
|
||||
createUnrelatedHistoryGraftCommit,
|
||||
GIT_SYNC_COMMIT_IDENTITY_ARGS,
|
||||
readSanitizedOriginRemoteUrl,
|
||||
} from "./git-workspace-sync.js";
|
||||
import type { RunProcessResult } from "./server-utils.js";
|
||||
import type { DirectorySnapshot } from "./workspace-restore-merge.js";
|
||||
import { mergeDirectoryWithBaseline } from "./workspace-restore-merge.js";
|
||||
|
|
@ -931,10 +935,18 @@ async function integrateImportedGitHead(input: {
|
|||
if (!currentHead || currentHead === input.importedHead) return;
|
||||
|
||||
const headRef = snapshot.branchName ? `refs/heads/${snapshot.branchName}` : "HEAD";
|
||||
// `git merge-base` exits 1 when the commits share no ancestor — the only
|
||||
// outcome that authorizes the graft fallback below. Every other failure
|
||||
// (timeout, missing object, repository error) must keep failing the
|
||||
// integration instead of silently rewriting the tip.
|
||||
let noCommonAncestor = false;
|
||||
const mergeBase = await runLocalGit(input.localDir, ["merge-base", currentHead, input.importedHead], {
|
||||
timeout: 10_000,
|
||||
maxBuffer: 16 * 1024,
|
||||
}).catch(() => null);
|
||||
}).catch((error: unknown) => {
|
||||
noCommonAncestor = (error as { code?: unknown } | null)?.code === 1;
|
||||
return null;
|
||||
});
|
||||
const mergeBaseHead = mergeBase?.stdout.trim() ?? "";
|
||||
|
||||
if (mergeBaseHead === input.importedHead) {
|
||||
|
|
@ -954,6 +966,28 @@ async function integrateImportedGitHead(input: {
|
|||
}
|
||||
}
|
||||
|
||||
if (noCommonAncestor) {
|
||||
// No common ancestor — merging is impossible and failing here would
|
||||
// discard the imported work. Graft it onto the current head instead;
|
||||
// see createUnrelatedHistoryGraftCommit.
|
||||
const graftCommit = await createUnrelatedHistoryGraftCommit({
|
||||
localDir: input.localDir,
|
||||
currentHead,
|
||||
importedHead: input.importedHead,
|
||||
syncLabel: "Paperclip SSH sync",
|
||||
});
|
||||
try {
|
||||
await runLocalGit(input.localDir, ["update-ref", headRef, graftCommit, currentHead], {
|
||||
timeout: 10_000,
|
||||
maxBuffer: 16 * 1024,
|
||||
});
|
||||
return;
|
||||
} catch (error) {
|
||||
if (isConcurrentRefUpdateError(error) && attempt < 4) continue;
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
let mergedTree;
|
||||
try {
|
||||
mergedTree = await runLocalGit(input.localDir, ["merge-tree", "--write-tree", currentHead, input.importedHead], {
|
||||
|
|
|
|||
Loading…
Reference in New Issue