fix(adapter-utils): give sync-created merge commits a deterministic git identity (#11637)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Runs execute in transported workspaces; at finalize, the host syncs the sandbox git history back into the local worktree > - When both sides advanced, `integrateImportedGitHead` reconciles them with `git merge-tree` plus `git commit-tree` on the host > - Execution hosts are often containers with no git config and no resolvable hostname, so `commit-tree` fails with "Author identity unknown" > - That one local command failure marks the whole run as failed, even though the run's work succeeded > - This pull request gives sync-created merge commits an explicit, deterministic identity at the call site > - The benefit is that workspace finalize no longer depends on ambient host git configuration ## Linked Issues or Issue Description No existing issue found. I searched issues and PRs for "Author identity unknown", "unrelated histories", and "commit-tree identity". **What happened?** A run finished its work, but workspace finalize failed. The host-side sync ran `git commit-tree <tree> -p <localHead> -p <importedHead> -m "Paperclip remote git sync merge <sha>"`. Git exited with `Author identity unknown ... fatal: unable to auto-detect email address (got 'node@<container-id>.(none)')`. The adapter recorded the whole run as failed, and the host worktree kept the stale head. Any container deployment without a global gitconfig reproduces this; I observed it on a Paperclip Cloud stack. **Expected behavior** Commits that the sync machinery itself creates must not depend on ambient host git configuration. The merge commit is machine-authored, so it should carry a deterministic Paperclip identity. **Steps to reproduce** 1. Run the Paperclip server in a container with no `user.name`/`user.email` git config and a hostname git cannot turn into an email. 2. Let a run's sandbox branch diverge from the host worktree, so both sides advance. 3. Workspace finalize calls `integrateImportedGitHead`. The `git commit-tree` step fails with "Author identity unknown" and the run fails. ## What Changed - `git-workspace-sync.ts`: new exported `GIT_SYNC_COMMIT_IDENTITY_ARGS` (`-c user.name=Paperclip -c user.email=noreply@paperclip.ing`), applied to the `commit-tree` call in `integrateImportedGitHead`. - `ssh.ts`: the SSH-sync copy of `integrateImportedGitHead` applies the same identity args to its `commit-tree` call. - New regression test: builds divergent histories in a repo with no configured identity and asserts the sync merge commit is created with the deterministic identity, correct parents, and merged tree. ## Verification - `pnpm vitest run packages/adapter-utils/src/git-workspace-sync.test.ts` — 18/18 pass. - `pnpm --filter @paperclipai/adapter-utils typecheck` — clean. - Negative proof: with the source fix stashed, the new test fails on the identity assertion. - 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 Low risk. The change only adds `-c` identity flags to two machine-generated commit invocations. `GIT_AUTHOR_*` / `GIT_COMMITTER_*` environment variables still take precedence over `-c` when an operator sets them, so existing deployments that configure an identity keep their behavior. ## 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 - [ ] All Paperclip CI gates are green - [ ] 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:
parent
0e9b03832d
commit
9ea8143c87
|
|
@ -11,6 +11,7 @@ import {
|
|||
createRemoteGitExportRef,
|
||||
deleteLocalGitRef,
|
||||
fetchGitBundleIntoLocalRef,
|
||||
integrateImportedGitHead,
|
||||
isMissingGitPrerequisiteError,
|
||||
readGitWorkspaceSnapshot,
|
||||
runLocalGit,
|
||||
|
|
@ -422,6 +423,60 @@ describe("git workspace sync", () => {
|
|||
await deleteLocalGitRef({ localDir: host, ref: importedRef });
|
||||
}
|
||||
});
|
||||
|
||||
it("creates the concurrent-history merge commit with a deterministic identity", async () => {
|
||||
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-git-merge-identity-"));
|
||||
cleanupDirs.push(rootDir);
|
||||
// No repo-local user.name/user.email on purpose: execution hosts are
|
||||
// containers without git config, where commit-tree cannot auto-detect an
|
||||
// identity. Setup commits pass their identity inline so only the merge
|
||||
// commit under test depends on the sync-supplied identity.
|
||||
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"]);
|
||||
|
||||
await git(repo, ["checkout", "-b", "imported", baseHead]);
|
||||
await writeFile(path.join(repo, "imported.txt"), "imported\n", "utf8");
|
||||
await git(repo, ["add", "imported.txt"]);
|
||||
await git(repo, [...setupIdentity, "commit", "-m", "sandbox change"]);
|
||||
const importedHead = await git(repo, ["rev-parse", "HEAD"]);
|
||||
await git(repo, ["checkout", "main"]);
|
||||
|
||||
// Ambient identity env vars would override the `-c` flags and make the
|
||||
// assertion machine-dependent, so clear them for the call under test.
|
||||
const identityEnvKeys = ["GIT_AUTHOR_NAME", "GIT_AUTHOR_EMAIL", "GIT_COMMITTER_NAME", "GIT_COMMITTER_EMAIL", "EMAIL"];
|
||||
const savedEnv = new Map(identityEnvKeys.map((key) => [key, process.env[key]]));
|
||||
for (const key of identityEnvKeys) delete process.env[key];
|
||||
try {
|
||||
await integrateImportedGitHead({ localDir: repo, importedHead });
|
||||
} finally {
|
||||
for (const [key, value] of savedEnv) {
|
||||
if (value === undefined) delete process.env[key];
|
||||
else process.env[key] = value;
|
||||
}
|
||||
}
|
||||
|
||||
const parents = (await git(repo, ["rev-list", "--parents", "-1", "HEAD"])).split(" ");
|
||||
expect(parents.slice(1)).toEqual([currentHead, importedHead]);
|
||||
expect(await git(repo, ["log", "-1", "--format=%an|%ae|%cn|%ce"]))
|
||||
.toBe("Paperclip|noreply@paperclip.ing|Paperclip|noreply@paperclip.ing");
|
||||
expect(await git(repo, ["log", "-1", "--format=%s"]))
|
||||
.toBe(`Paperclip remote git sync merge ${importedHead.slice(0, 12)}`);
|
||||
const mergedTree = await git(repo, ["ls-tree", "--name-only", "HEAD"]);
|
||||
expect(mergedTree).toContain("local.txt");
|
||||
expect(mergedTree).toContain("imported.txt");
|
||||
});
|
||||
});
|
||||
|
||||
describe("sanitizeGitRemoteUrl", () => {
|
||||
|
|
|
|||
|
|
@ -42,6 +42,23 @@ export function setExpensiveWorkspaceGitExecutor(executor: ExpensiveWorkspaceGit
|
|||
|
||||
export const GIT_ARCHIVE_EXCLUDES = [".git", ".git/*"] as const;
|
||||
|
||||
/**
|
||||
* Identity flags for commits the sync machinery itself creates (the merge
|
||||
* commits that reconcile concurrent histories). Execution hosts are often
|
||||
* containers with no git config and no resolvable hostname, so git cannot
|
||||
* auto-detect an identity there and `commit-tree` hard-fails with "Author
|
||||
* identity unknown" — which fails the whole run at finalize. Passing the
|
||||
* identity per invocation keeps every deployment working without host
|
||||
* configuration; `GIT_AUTHOR_*` / `GIT_COMMITTER_*` environment variables
|
||||
* still take precedence over `-c` when an operator sets them.
|
||||
*/
|
||||
export const GIT_SYNC_COMMIT_IDENTITY_ARGS = [
|
||||
"-c",
|
||||
"user.name=Paperclip",
|
||||
"-c",
|
||||
"user.email=noreply@paperclip.ing",
|
||||
] as const;
|
||||
|
||||
function shellQuote(value: string) {
|
||||
return `'${value.replace(/'/g, `'\"'\"'`)}'`;
|
||||
}
|
||||
|
|
@ -476,6 +493,7 @@ export async function integrateImportedGitHead(input: {
|
|||
const mergeCommit = await runLocalGit(
|
||||
input.localDir,
|
||||
[
|
||||
...GIT_SYNC_COMMIT_IDENTITY_ARGS,
|
||||
"commit-tree",
|
||||
mergedTreeId,
|
||||
"-p",
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@ import os from "node:os";
|
|||
import path from "node:path";
|
||||
import { Transform } from "node:stream";
|
||||
import type { CommandManagedRuntimeRunner } from "./command-managed-runtime.js";
|
||||
import { readSanitizedOriginRemoteUrl } from "./git-workspace-sync.js";
|
||||
import { 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";
|
||||
|
|
@ -974,6 +974,7 @@ async function integrateImportedGitHead(input: {
|
|||
const mergeCommit = await runLocalGit(
|
||||
input.localDir,
|
||||
[
|
||||
...GIT_SYNC_COMMIT_IDENTITY_ARGS,
|
||||
"commit-tree",
|
||||
mergedTreeId,
|
||||
"-p",
|
||||
|
|
|
|||
Loading…
Reference in New Issue