From 42c73562c5c18cb4e01d60f3d89295ff13992a36 Mon Sep 17 00:00:00 2001 From: Michael Nguyen <13559011+nguyenm7@users.noreply.github.com> Date: Fri, 7 Aug 2026 09:06:36 -0700 Subject: [PATCH] fix(heartbeat): backfill projectWorkspaceId when restoring a reused execution workspace (#10171) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Each run executes inside a persisted **execution workspace** (a row in `execution_workspaces`) that is either freshly created or **restored/reused** across runs of the same issue > - Before adapter launch, a guard rejects a restored workspace whose `projectWorkspaceId` is null while the issue resolves a concrete project workspace (`persisted_workspace_missing_project_workspace_id`) — a safety check against binding a run to a workspace with no project-workspace link > - The reuse/**restore** path updated the existing row (cwd, branch, status, metadata…) but **never set `projectWorkspaceId`**, so a row persisted with a null value stayed null on every restore > - Result: for an issue that resolves a project workspace, the guard fires, `reuse_existing` re-selects and re-binds the *same* stale null row on the next attempt, and the run crash-loops forever with no self-heal > - This pull request backfills `projectWorkspaceId` during restore (prefer the existing binding, fall back to the resolved one) so the row heals on first reuse and the guard stops firing > - The benefit is that reused workspaces created before their project had a primary project workspace self-repair on next use instead of crash-looping, while genuine mismatches are still surfaced by the guard ## Linked Issues or Issue Description No public GitHub issue exists — describing the bug inline per the bug report template (`.github/ISSUE_TEMPLATE/bug_report.yml`): ### What happened? In `heartbeatService`, the execution-workspace reuse/restore branch calls `executionWorkspacesSvc.update(reusableExistingExecutionWorkspace.id, { … })` without a `projectWorkspaceId` field. Only the sibling CREATE branch sets `projectWorkspaceId`. So an execution workspace that was persisted with a null `projectWorkspaceId` (e.g. created before its project had a primary project workspace) is never backfilled on restore. When such a workspace is later reused for a run whose issue resolves a concrete project workspace, the pre-launch guard throws `persisted_workspace_missing_project_workspace_id`, the run fails, and `reuse_existing` re-binds the identical stale row on the next attempt — an unbounded crash-loop with no self-heal. ### Expected behavior On restore, the reused workspace's `projectWorkspaceId` is backfilled from the resolved project workspace when it is currently null, so the guard passes and the run launches. An existing non-null binding is never overwritten (a genuine mismatch is still surfaced by the separate `project_workspace_mismatch` guard). ### Steps to reproduce 1. Have an `execution_workspaces` row with `project_workspace_id = NULL` that is eligible for reuse. 2. Give its project a primary project workspace (so the issue now resolves a concrete `projectWorkspaceId`). 3. Dispatch a run for an issue in that project that reuses the workspace. The restore `update()` leaves `project_workspace_id` null, the launch guard throws `persisted_workspace_missing_project_workspace_id`, and every subsequent reuse re-binds the same null row and fails identically. ### Paperclip version or commit `master` (branched from `14f20be92`); reproduced on a live self-hosted instance. ### Deployment mode Self-hosted, embedded Postgres, local adapters. ## What Changed - New exported pure helper `reconcileReusedExecutionWorkspaceProjectWorkspaceId(existing, resolved)` in `server/src/services/heartbeat.ts`, returning `existing ?? resolved ?? null`. It prefers an existing binding (never nulls out a good value or silently rebinds a genuine mismatch — the guard still surfaces those), backfills a null binding from the resolved value, and stays null when neither is present. - Wire the helper into the reuse/restore `executionWorkspacesSvc.update(...)` call so the restored row's `projectWorkspaceId` is set to `reconcileReusedExecutionWorkspaceProjectWorkspaceId(reusableExistingExecutionWorkspace.projectWorkspaceId, resolvedProjectWorkspaceId)`. The CREATE branch already set `projectWorkspaceId`; this brings the restore branch to parity. ## Verification - Added 3-case unit coverage in `server/src/__tests__/heartbeat-workspace-session.test.ts` for the helper: (a) backfills a null existing binding from the resolved value, (b) never overwrites an existing binding even when a resolved value is present, (c) returns null when both existing and resolved are absent (null and undefined inputs). - Confirmed the `update()` patch type accepts the field: `executionWorkspacesSvc.update` takes `Partial`, and `projectWorkspaceId` is a column on that table; both `reusableExistingExecutionWorkspace.projectWorkspaceId` and `resolvedProjectWorkspaceId` are `string | null`, matching the helper's `string | null | undefined` params / `string | null` return. - Live-instance exposure check (embedded Postgres): 354 `execution_workspaces` rows carry a null `project_workspace_id`; all of them belong to projects with **no** project workspace, so `expectedProjectWorkspaceId` currently resolves null and the guard does not fire today. The fix is durable heal-on-reuse protection for the moment any such project gains a primary project workspace (or a null row is reused for an issue that resolves one). - CI (full pnpm workspace install) runs the authoritative test + typecheck for this change on this PR. ## Risks - Low risk; scoped to the execution-workspace restore path, no schema or API change. - The helper only ever *adds* a `projectWorkspaceId` where the row had none; it never overwrites an existing binding, so it cannot mask a real `project_workspace_mismatch` (that guard still runs after). - Complementary to (not overlapping with) #10130, which escalates a terminal `workspace_validation_failed` run to `blocked` from the recovery side; this PR prevents the guard from firing on reuse in the first place. Neither depends on the other. ## Model Used Claude Opus 4.8 (`claude-opus-4-8`), extended thinking, with tool use / code execution (repo edit, embedded-Postgres exposure query, unit-logic 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 (searched open PRs touching heartbeat / execution-workspace / reuse; only #10130 is related, and it is complementary) - [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 - [ ] I have updated relevant documentation to reflect my changes (no user-facing docs affected) - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green (in progress) - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups (pending) - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Paperclip --- .../heartbeat-workspace-session.test.ts | 27 +++++++++++++++++++ server/src/services/heartbeat.ts | 24 +++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/server/src/__tests__/heartbeat-workspace-session.test.ts b/server/src/__tests__/heartbeat-workspace-session.test.ts index 26734c687d..a969382862 100644 --- a/server/src/__tests__/heartbeat-workspace-session.test.ts +++ b/server/src/__tests__/heartbeat-workspace-session.test.ts @@ -25,6 +25,7 @@ import { prioritizeProjectWorkspaceCandidatesForRun, parseSessionCompactionPolicy, provisionExecutionWorkspaceForFreshnessDecision, + reconcileReusedExecutionWorkspaceProjectWorkspaceId, resolveExecutionWorkspaceConfigFreshness, resolveExecutionWorkspaceReuseRequestForIssue, resolveExecutionWorkspaceReuseProvisioningPolicy, @@ -2884,3 +2885,29 @@ describe("isWorkspaceSyncConflictFailure", () => { expect(isWorkspaceSyncConflictFailure("")).toBe(false); }); }); + +describe("reconcileReusedExecutionWorkspaceProjectWorkspaceId", () => { + it("backfills a null existing binding from the resolved value", () => { + expect( + reconcileReusedExecutionWorkspaceProjectWorkspaceId(null, "resolved-workspace"), + ).toBe("resolved-workspace"); + }); + + it("never overwrites an existing binding, even when a resolved value is present", () => { + expect( + reconcileReusedExecutionWorkspaceProjectWorkspaceId("existing-workspace", "resolved-workspace"), + ).toBe("existing-workspace"); + }); + + it("returns null when both existing and resolved are absent", () => { + expect(reconcileReusedExecutionWorkspaceProjectWorkspaceId(null, null)).toBeNull(); + expect(reconcileReusedExecutionWorkspaceProjectWorkspaceId(undefined, undefined)).toBeNull(); + expect(reconcileReusedExecutionWorkspaceProjectWorkspaceId(null, undefined)).toBeNull(); + }); + + it("backfills when existing is undefined and resolved is present", () => { + expect( + reconcileReusedExecutionWorkspaceProjectWorkspaceId(undefined, "resolved-workspace"), + ).toBe("resolved-workspace"); + }); +}); diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 11e9e24725..f9eddcbd5b 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -1979,6 +1979,26 @@ export async function assertPushCapabilityCheckoutValid(input: { ); } +/** + * Reconcile the `projectWorkspaceId` for a reused execution workspace. + * + * A `reuse_existing` workspace can have been persisted with a null + * `projectWorkspaceId` (e.g. it was created before its project had a primary + * project workspace). When we later restore it for a run whose issue now + * expects a concrete project workspace, backfill the column so the launch + * guard (`persisted_workspace_missing_project_workspace_id`) stops rejecting + * it on every requeue — otherwise `reuse_existing` re-binds the same stale + * record forever and the run crash-loops. Prefer the existing binding when + * present so we never null out a good value or silently rebind a genuine + * mismatch (which the guard still surfaces). + */ +export function reconcileReusedExecutionWorkspaceProjectWorkspaceId( + existingProjectWorkspaceId: string | null | undefined, + resolvedProjectWorkspaceId: string | null | undefined, +): string | null { + return existingProjectWorkspaceId ?? resolvedProjectWorkspaceId ?? null; +} + export async function assertGitSensitiveAdapterWorkspaceValid(input: { adapterType: string; agentId: string; @@ -14578,6 +14598,10 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {}) status: "active", lastUsedAt: new Date(), metadata: nextExecutionWorkspaceMetadata, + projectWorkspaceId: reconcileReusedExecutionWorkspaceProjectWorkspaceId( + reusableExistingExecutionWorkspace.projectWorkspaceId, + resolvedProjectWorkspaceId, + ), }) : resolvedProjectId ? await executionWorkspacesSvc.create({