mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 13:24:55 +01:00
Add auto-forward execution workspace branch reconciliation (#9172)
## Thinking Path > - Paperclip manages execution workspaces for AI agent runs, each associated with a git branch so the agent always works in a known code state. > - Workspace runtime reconciles an agent's checkout branch against the workspace's recorded branch when a heartbeat resumes; without forward-ancestry detection, any divergence fails closed and blocks the run. > - When the workspace branch has moved forward (e.g. after a feature merge), the recorded branch is an ancestor of the current HEAD — a safe, forward-only case that the previous implementation refused even though it carries no safety risk. > - The gap means legitimate forward-advancing deployments require manual operator intervention to unblock agents every time, creating operational friction and interrupting automated workflows. > - This pull request adds a flag-gated reconcile-forward path that detects when the current branch is a strict forward descendant of the workspace branch and auto-reconciles, while preserving fail-closed behavior for all non-forward or flag-off cases. > - It also threads the active execution workspace id through restore and finalize call sites so the reconcile verdict can be persisted durably across heartbeats. > - The benefit is that agents resume automatically from forward-advancing workspace branches without operator intervention, while adversarial and backward branch changes continue to fail closed. ## Linked Issues or Issue Description This PR adds an auto-forward reconcile path for execution workspace branch tracking. When a workspace's recorded branch is a strict ancestor of the current HEAD (a forward-only advancement), the runtime now auto-reconciles rather than hard-blocking. The feature is gated behind an explicit runtime flag, defaults to off, and falls back to fail-closed behavior for all non-forward or flag-off cases. The prior implementation treated all branch divergences identically: any mismatch between the recorded workspace branch and the current HEAD failed closed. This prevented agents from resuming after routine forward deployments (e.g. after a feature branch merges into the workspace branch), requiring manual operator action to unblock every affected run. ## What Changed - Added `reconcileForward` flag-gated path in workspace runtime reconciliation logic that allows auto-reconciliation when the workspace branch is a strict ancestor of the current HEAD. - Threaded active execution workspace id through `restore` and `finalize` call sites so reconcile verdicts are persisted durably. - Added `plainLanguageReason` and `ancestryVerdict` evidence fields to the reconciliation result structure for operator visibility. - Stabilized a branch containment test that exposed a late run-linked activity FK cleanup race during the focused Vitest rerun. - All new paths remain fail-closed when the flag is off or when the branch relationship is not strictly forward. ## Verification ```bash pnpm --filter @paperclipai/shared typecheck pnpm --filter @paperclipai/server typecheck pnpm exec vitest run \ server/src/__tests__/workspace-runtime.test.ts \ server/src/__tests__/heartbeat-workspace-branch-containment.test.ts \ server/src/__tests__/execution-workspaces-service.test.ts git diff --check origin/master..HEAD ``` All 3 test files / 98 tests pass. Typecheck passes for both shared and server packages. > **Note:** This is a stacked PR on top of PR #9170 (Add execution workspace branch reconciliation route). The diff shown targets that branch; the combined change builds on the reconciliation route infrastructure it provides. ## Risks - **Flag-off default:** The reconcile-forward path is off by default. No behavior change for existing workspaces unless the flag is explicitly enabled by an operator. - **Ancestry check correctness:** The forward-only guard uses git ancestry verification; a branch that is not a strict ancestor of HEAD remains fail-closed. Adversarial or concurrent branch resets are not auto-reconciled. - **FK cleanup race (stabilized):** A late run-linked activity FK cleanup race in the containment test was exposed during the Vitest rerun. The stabilization commit addresses the non-deterministic ordering without changing production behavior. - **Stacking dependency:** This PR must not be merged before PR #9170 merges, as it is built on top of the reconciliation route infrastructure. ## Model Used - Provider: Anthropic - Model: Claude Sonnet 4.6 (`claude-sonnet-4-6`) - Context: 200k token context window - Mode: Agentic tool use with code execution and git operations ## 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 - [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 --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
f17202b571
commit
bb87047fe6
5 files changed
+852
-19
No files matched your search
@@ -122,6 +122,78 @@ async function createTempRepo(defaultBranch = "main") {
|
||||
return repoRoot;
|
||||
}
|
||||
|
||||
async function expectPersistedBranchMismatchRejected(input: {
|
||||
repoRoot: string;
|
||||
worktreePath: string;
|
||||
expectedBranch: string;
|
||||
actualBranch: string;
|
||||
issueId: string;
|
||||
executionWorkspaceId: string;
|
||||
expectedAncestryVerdict: "diverged" | "unknown";
|
||||
expectedReason?: string;
|
||||
}) {
|
||||
let error: unknown = null;
|
||||
try {
|
||||
await ensurePersistedExecutionWorkspaceAvailable({
|
||||
base: {
|
||||
baseCwd: input.repoRoot,
|
||||
source: "project_primary",
|
||||
projectId: "project-1",
|
||||
workspaceId: "workspace-1",
|
||||
repoUrl: null,
|
||||
repoRef: "HEAD",
|
||||
},
|
||||
workspace: {
|
||||
id: input.executionWorkspaceId,
|
||||
mode: "isolated_workspace",
|
||||
strategyType: "git_worktree",
|
||||
cwd: input.worktreePath,
|
||||
providerRef: input.worktreePath,
|
||||
projectId: "project-1",
|
||||
projectWorkspaceId: "workspace-1",
|
||||
repoUrl: null,
|
||||
baseRef: "HEAD",
|
||||
branchName: input.expectedBranch,
|
||||
},
|
||||
issue: {
|
||||
id: input.issueId,
|
||||
identifier: "PAP-459",
|
||||
title: "Reject unsafe forward branch reconciliation",
|
||||
},
|
||||
agent: {
|
||||
id: "agent-1",
|
||||
name: "Codex Coder",
|
||||
companyId: "company-1",
|
||||
},
|
||||
enableWorkspaceBranchReconcileForward: true,
|
||||
});
|
||||
} catch (err) {
|
||||
error = err;
|
||||
}
|
||||
|
||||
expect(error).toMatchObject({
|
||||
code: "workspace_validation_failed",
|
||||
resultJson: {
|
||||
workspaceValidation: expect.objectContaining({
|
||||
reason: "git_worktree_branch_incoherence",
|
||||
sourceIssueId: input.issueId,
|
||||
executionWorkspaceId: input.executionWorkspaceId,
|
||||
expectedBranch: input.expectedBranch,
|
||||
actualBranch: input.actualBranch,
|
||||
provenance: expect.objectContaining({
|
||||
ancestryVerdict: input.expectedAncestryVerdict,
|
||||
}),
|
||||
safeRepair: expect.objectContaining({
|
||||
eligible: false,
|
||||
attempted: false,
|
||||
succeeded: false,
|
||||
...(input.expectedReason ? { reason: input.expectedReason } : {}),
|
||||
}),
|
||||
}),
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
async function createClonedRepoWithRemote() {
|
||||
const sourceRepo = await createTempRepo("master");
|
||||
const remoteDir = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-worktree-remote-"));
|
||||
@@ -2558,6 +2630,7 @@ describe("realizeExecutionWorkspace", () => {
|
||||
name: "Codex Coder",
|
||||
companyId: "company-1",
|
||||
},
|
||||
enableWorkspaceBranchReconcileForward: true,
|
||||
});
|
||||
} catch (err) {
|
||||
error = err;
|
||||
@@ -2630,6 +2703,7 @@ describe("realizeExecutionWorkspace", () => {
|
||||
name: "Codex Coder",
|
||||
companyId: "company-1",
|
||||
},
|
||||
enableWorkspaceBranchReconcileForward: true,
|
||||
});
|
||||
} catch (err) {
|
||||
error = err;
|
||||
@@ -2665,6 +2739,93 @@ describe("realizeExecutionWorkspace", () => {
|
||||
});
|
||||
}, 15_000);
|
||||
|
||||
it("keeps forward reconciliation fail-closed for same-content rewritten history", async () => {
|
||||
const repoRoot = await createTempRepo();
|
||||
const expectedBranch = "PAP-459-recorded-content";
|
||||
const actualBranch = "PAP-459-rewritten-content";
|
||||
const worktreePath = path.join(repoRoot, ".paperclip", "worktrees", expectedBranch);
|
||||
|
||||
await runGit(repoRoot, ["checkout", "-b", expectedBranch]);
|
||||
await fs.writeFile(path.join(repoRoot, "same-content.txt"), "same content\n", "utf8");
|
||||
await runGit(repoRoot, ["add", "same-content.txt"]);
|
||||
await runGit(repoRoot, ["commit", "-m", "Add content on recorded branch"]);
|
||||
await runGit(repoRoot, ["checkout", "main"]);
|
||||
|
||||
await fs.mkdir(path.dirname(worktreePath), { recursive: true });
|
||||
await runGit(repoRoot, ["worktree", "add", "-b", actualBranch, worktreePath, "main"]);
|
||||
await fs.writeFile(path.join(worktreePath, "same-content.txt"), "same content\n", "utf8");
|
||||
await runGit(worktreePath, ["add", "same-content.txt"]);
|
||||
await runGit(worktreePath, ["commit", "-m", "Add content on rewritten branch"]);
|
||||
|
||||
await expectPersistedBranchMismatchRejected({
|
||||
repoRoot,
|
||||
worktreePath,
|
||||
expectedBranch,
|
||||
actualBranch,
|
||||
issueId: "issue-rewritten-history",
|
||||
executionWorkspaceId: "execution-workspace-rewritten-history",
|
||||
expectedAncestryVerdict: "diverged",
|
||||
expectedReason: "expected branch and current HEAD differ",
|
||||
});
|
||||
}, 15_000);
|
||||
|
||||
it("keeps forward reconciliation fail-closed for an unrelated task branch", async () => {
|
||||
const repoRoot = await createTempRepo();
|
||||
const expectedBranch = "PAP-459-recorded-task";
|
||||
const actualBranch = "PAP-999-unrelated-task";
|
||||
const worktreePath = path.join(repoRoot, ".paperclip", "worktrees", expectedBranch);
|
||||
|
||||
await runGit(repoRoot, ["checkout", "-b", expectedBranch]);
|
||||
await fs.writeFile(path.join(repoRoot, "recorded-task.txt"), "recorded task work\n", "utf8");
|
||||
await runGit(repoRoot, ["add", "recorded-task.txt"]);
|
||||
await runGit(repoRoot, ["commit", "-m", "Add recorded task work"]);
|
||||
await runGit(repoRoot, ["checkout", "main"]);
|
||||
|
||||
await fs.mkdir(path.dirname(worktreePath), { recursive: true });
|
||||
await runGit(repoRoot, ["worktree", "add", "-b", actualBranch, worktreePath, "main"]);
|
||||
await fs.writeFile(path.join(worktreePath, "unrelated-task.txt"), "unrelated task work\n", "utf8");
|
||||
await runGit(worktreePath, ["add", "unrelated-task.txt"]);
|
||||
await runGit(worktreePath, ["commit", "-m", "Add unrelated task work"]);
|
||||
|
||||
await expectPersistedBranchMismatchRejected({
|
||||
repoRoot,
|
||||
worktreePath,
|
||||
expectedBranch,
|
||||
actualBranch,
|
||||
issueId: "issue-unrelated-task",
|
||||
executionWorkspaceId: "execution-workspace-unrelated-task",
|
||||
expectedAncestryVerdict: "diverged",
|
||||
expectedReason: "expected branch and current HEAD differ",
|
||||
});
|
||||
}, 15_000);
|
||||
|
||||
it("keeps forward reconciliation fail-closed when the live branch is behind the recorded branch", async () => {
|
||||
const repoRoot = await createTempRepo();
|
||||
const expectedBranch = "PAP-459-recorded-ahead";
|
||||
const actualBranch = "PAP-459-live-behind";
|
||||
const worktreePath = path.join(repoRoot, ".paperclip", "worktrees", expectedBranch);
|
||||
|
||||
await fs.mkdir(path.dirname(worktreePath), { recursive: true });
|
||||
await runGit(repoRoot, ["branch", expectedBranch]);
|
||||
await runGit(repoRoot, ["worktree", "add", "-b", actualBranch, worktreePath, expectedBranch]);
|
||||
|
||||
await runGit(repoRoot, ["checkout", expectedBranch]);
|
||||
await fs.writeFile(path.join(repoRoot, "recorded-ahead.txt"), "recorded branch moved ahead\n", "utf8");
|
||||
await runGit(repoRoot, ["add", "recorded-ahead.txt"]);
|
||||
await runGit(repoRoot, ["commit", "-m", "Move recorded branch ahead"]);
|
||||
|
||||
await expectPersistedBranchMismatchRejected({
|
||||
repoRoot,
|
||||
worktreePath,
|
||||
expectedBranch,
|
||||
actualBranch,
|
||||
issueId: "issue-live-behind",
|
||||
executionWorkspaceId: "execution-workspace-live-behind",
|
||||
expectedAncestryVerdict: "diverged",
|
||||
expectedReason: "expected branch and current HEAD differ",
|
||||
});
|
||||
}, 15_000);
|
||||
|
||||
it("does not reuse a missing persisted local filesystem workspace", async () => {
|
||||
const baseCwd = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-workspace-base-"));
|
||||
const missingCwd = path.join(baseCwd, "missing-workspace");
|
||||
|
||||
Reference in new issue
Block a user