From 0001a4514e1305b7bddea4fcbf509e1da6406b1a Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Fri, 23 Oct 2026 10:25:59 -0700 Subject: [PATCH] fix: keep stale pending restore points from blocking retries (#17344) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Git cache expiration saves restore points and tracks them in `execution_workspace_restore_points` > - A unique index allows one row per workspace, generation and checkout unless the row is deleted > - A pending row left by an interrupted attempt therefore blocked every retry for an hour, and superseded rows blocked retries until retention deleted their objects > - This pull request narrows the index to live rows and supersedes stale pending rows before inserting > - The benefit is that an interrupted expiration no longer stalls a workspace's cleanup ## Linked Issues or Issue Description Refs #17257. Bug report described in-PR (bug report template). **What happened?** On one instance, a closed workspace with a nested checkout failed expiration with `inspection_failed` on every sweep. An earlier attempt had left a `pending` row for the nested checkout at the workspace's next generation. Each retry stored the main worktree's restore point, then failed to insert the nested row because the unique index on `(execution_workspace_id, generation, checkout_name) WHERE status <> 'deleted'` still held the stale row. Running the expiration by hand reproduced the failing insert. **Expected behavior** A retry after an interrupted attempt replaces the stale row and expires the workspace. **Steps to reproduce** 1. Leave a `pending` restore point row for a workspace's next generation and a nested checkout, as an interrupted attempt does. 2. Run expiration with restore points: it fails with `inspection_failed` until retention supersedes the row after the one-hour pending grace period. **Paperclip version or commit:** `master` at 0fb152ee949. **Deployment mode:** any. **Agent adapter(s) involved:** not adapter-specific (core). ## What Changed - **Schema** (`packages/db/src/schema/execution_workspace_restore_points.ts`, migration `0375`): the unique index is renamed `execution_workspace_restore_points_live_status_uniq` and covers only `pending`, `active` and `expiring`. Superseded, delete-requested and deleted rows no longer block a new row. - **Storage** (`storeGitRestorePoint`): in the transaction that inserts the new `pending` row, any `pending` row for the same workspace, generation and checkout is first marked `superseded`. The expiration holds the workspace lifecycle lock, so such a row can only come from an attempt that already ended. Retention then deletes its object as usual. No code comments were added. ## Verification - `pnpm exec vitest run src/__tests__/git-workspace-expiration.test.ts -t "stale pending restore point"` (in `server/`): 1 passed. A workspace with a stale `pending` nested row and a `superseded` main row from an earlier attempt expires with a nested restore point, and the stale row is superseded. - **Mutation checks:** removing the supersede step, or keeping the old index scope in the migration, each reproduces the instance's `inspection_failed`. - `pnpm exec vitest run git-restore-point git-workspace-expiration git-workspace-contents git-workspace-restoration git-reproducible-managed-checkouts execution-workspaces heartbeat-git-restore-point-resume` (in `server/`): 274 passed. - `pnpm run check:migrations` (in `packages/db`): pass. - `pnpm run typecheck` (in `server/`): pass. - Not run: server build and boot smoke. ## Risks - **Migration:** drops and recreates one partial unique index on a small table. It cannot fail on existing data, because the old index was stricter. The index is briefly absent while the migration runs. - **Superseding a pending row:** only rows for the same workspace, generation and checkout, while the current attempt holds the workspace lifecycle lock. - **Not addressed:** why the earlier attempt left the row behind. #17341 logs the underlying error of `inspection_failed` so the next occurrence can be diagnosed. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), 1M context, through Claude Code with tool use (shell, file edits, tests, read-only and dry-run inspection of 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 --- .../db/src/migrations/0375_short_mercury.sql | 2 + ...{0370_snapshot.json => 0375_snapshot.json} | 404 +++++++++++++++++- packages/db/src/migrations/meta/_journal.json | 7 + .../execution_workspace_restore_points.ts | 4 +- .../git-workspace-expiration.test.ts | 18 + .../src/services/git-restore-point-storage.ts | 5 + 6 files changed, 436 insertions(+), 4 deletions(-) create mode 100644 packages/db/src/migrations/0375_short_mercury.sql rename packages/db/src/migrations/meta/{0370_snapshot.json => 0375_snapshot.json} (99%) diff --git a/packages/db/src/migrations/0375_short_mercury.sql b/packages/db/src/migrations/0375_short_mercury.sql new file mode 100644 index 00000000000..eac51f532dc --- /dev/null +++ b/packages/db/src/migrations/0375_short_mercury.sql @@ -0,0 +1,2 @@ +DROP INDEX "execution_workspace_restore_points_live_checkout_uniq";--> statement-breakpoint +CREATE UNIQUE INDEX "execution_workspace_restore_points_live_status_uniq" ON "execution_workspace_restore_points" USING btree ("execution_workspace_id","generation","checkout_name") WHERE "execution_workspace_restore_points"."status" in ('pending', 'active', 'expiring'); \ No newline at end of file diff --git a/packages/db/src/migrations/meta/0370_snapshot.json b/packages/db/src/migrations/meta/0375_snapshot.json similarity index 99% rename from packages/db/src/migrations/meta/0370_snapshot.json rename to packages/db/src/migrations/meta/0375_snapshot.json index d6870986af0..7bd1861bd2c 100644 --- a/packages/db/src/migrations/meta/0370_snapshot.json +++ b/packages/db/src/migrations/meta/0375_snapshot.json @@ -1,6 +1,6 @@ { - "id": "a7c9681c-fb33-47c3-8139-3d88d7bbfe19", - "prevId": "9fa514b5-5cd1-4350-9b77-f60924655298", + "id": "3e17203b-ec95-41e9-8363-685982450199", + "prevId": "945c31ed-f39f-4272-a91d-85de925f5361", "version": "7", "dialect": "postgresql", "tables": { @@ -8252,6 +8252,24 @@ "name": "case_handoffs", "schema": "", "columns": { + "workflow_transition": { + "name": "workflow_transition", + "type": "jsonb", + "primaryKey": false, + "notNull": false + }, + "workflow_prompt": { + "name": "workflow_prompt", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "base_workflow_document_revision_id": { + "name": "base_workflow_document_revision_id", + "type": "uuid", + "primaryKey": false, + "notNull": false + }, "id": { "name": "id", "type": "uuid", @@ -9067,6 +9085,12 @@ "primaryKey": false, "notNull": false }, + "workflow_id": { + "name": "workflow_id", + "type": "uuid", + "primaryKey": false, + "notNull": false + }, "handoff_version": { "name": "handoff_version", "type": "integer", @@ -9410,6 +9434,19 @@ "onDelete": "restrict", "onUpdate": "no action" }, + "cases_workflow_id_workflows_id_fk": { + "name": "cases_workflow_id_workflows_id_fk", + "tableFrom": "cases", + "tableTo": "workflows", + "columnsFrom": [ + "workflow_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "restrict", + "onUpdate": "no action" + }, "cases_parent_case_id_cases_id_fk": { "name": "cases_parent_case_id_cases_id_fk", "tableFrom": "cases", @@ -28436,6 +28473,250 @@ "checkConstraints": {}, "isRLSEnabled": false }, + "public.execution_workspace_restore_points": { + "name": "execution_workspace_restore_points", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "uuid", + "primaryKey": true, + "notNull": true, + "default": "gen_random_uuid()" + }, + "company_id": { + "name": "company_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "execution_workspace_id": { + "name": "execution_workspace_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "source_issue_id": { + "name": "source_issue_id", + "type": "uuid", + "primaryKey": false, + "notNull": false + }, + "generation": { + "name": "generation", + "type": "integer", + "primaryKey": false, + "notNull": true + }, + "checkout_name": { + "name": "checkout_name", + "type": "text", + "primaryKey": false, + "notNull": true, + "default": "''" + }, + "object_key": { + "name": "object_key", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "sha256": { + "name": "sha256", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "byte_size": { + "name": "byte_size", + "type": "bigint", + "primaryKey": false, + "notNull": true + }, + "status": { + "name": "status", + "type": "text", + "primaryKey": false, + "notNull": true, + "default": "'pending'" + }, + "activated_at": { + "name": "activated_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": false + }, + "tree_closed_at": { + "name": "tree_closed_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": false + }, + "delete_after": { + "name": "delete_after", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": false + }, + "review_generation": { + "name": "review_generation", + "type": "integer", + "primaryKey": false, + "notNull": true, + "default": 0 + }, + "deleted_at": { + "name": "deleted_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": false + }, + "last_error": { + "name": "last_error", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": { + "execution_workspace_restore_points_object_key_uniq": { + "name": "execution_workspace_restore_points_object_key_uniq", + "columns": [ + { + "expression": "object_key", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": true, + "concurrently": false, + "method": "btree", + "with": {} + }, + "execution_workspace_restore_points_live_status_uniq": { + "name": "execution_workspace_restore_points_live_status_uniq", + "columns": [ + { + "expression": "execution_workspace_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "generation", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "checkout_name", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": true, + "where": "\"execution_workspace_restore_points\".\"status\" in ('pending', 'active', 'expiring')", + "concurrently": false, + "method": "btree", + "with": {} + }, + "execution_workspace_restore_points_status_delete_after_idx": { + "name": "execution_workspace_restore_points_status_delete_after_idx", + "columns": [ + { + "expression": "status", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "delete_after", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + }, + "execution_workspace_restore_points_company_idx": { + "name": "execution_workspace_restore_points_company_idx", + "columns": [ + { + "expression": "company_id", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + }, + "execution_workspace_restore_points_source_issue_idx": { + "name": "execution_workspace_restore_points_source_issue_idx", + "columns": [ + { + "expression": "company_id", + "isExpression": false, + "asc": true, + "nulls": "last" + }, + { + "expression": "source_issue_id", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + }, + "execution_workspace_restore_points_active_source_idx": { + "name": "execution_workspace_restore_points_active_source_idx", + "columns": [ + { + "expression": "source_issue_id", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "where": "\"execution_workspace_restore_points\".\"status\" = 'active'", + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, "public.execution_workspace_runtime_leases": { "name": "execution_workspace_runtime_leases", "schema": "", @@ -38590,6 +38871,18 @@ "primaryKey": false, "notNull": false }, + "dismissed_at": { + "name": "dismissed_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": false + }, + "dismissed_by_user_id": { + "name": "dismissed_by_user_id", + "type": "text", + "primaryKey": false, + "notNull": false + }, "created_at": { "name": "created_at", "type": "timestamp with time zone", @@ -59279,6 +59572,113 @@ }, "isRLSEnabled": false }, + "public.workflows": { + "name": "workflows", + "schema": "", + "columns": { + "id": { + "name": "id", + "type": "uuid", + "primaryKey": true, + "notNull": true, + "default": "gen_random_uuid()" + }, + "company_id": { + "name": "company_id", + "type": "uuid", + "primaryKey": false, + "notNull": true + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "prompt": { + "name": "prompt", + "type": "text", + "primaryKey": false, + "notNull": true + }, + "created_by_agent_id": { + "name": "created_by_agent_id", + "type": "uuid", + "primaryKey": false, + "notNull": false + }, + "created_by_user_id": { + "name": "created_by_user_id", + "type": "text", + "primaryKey": false, + "notNull": false + }, + "created_at": { + "name": "created_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + }, + "updated_at": { + "name": "updated_at", + "type": "timestamp with time zone", + "primaryKey": false, + "notNull": true, + "default": "now()" + } + }, + "indexes": { + "workflows_company_idx": { + "name": "workflows_company_idx", + "columns": [ + { + "expression": "company_id", + "isExpression": false, + "asc": true, + "nulls": "last" + } + ], + "isUnique": false, + "concurrently": false, + "method": "btree", + "with": {} + } + }, + "foreignKeys": { + "workflows_company_id_companies_id_fk": { + "name": "workflows_company_id_companies_id_fk", + "tableFrom": "workflows", + "tableTo": "companies", + "columnsFrom": [ + "company_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "workflows_created_by_agent_id_agents_id_fk": { + "name": "workflows_created_by_agent_id_agents_id_fk", + "tableFrom": "workflows", + "tableTo": "agents", + "columnsFrom": [ + "created_by_agent_id" + ], + "columnsTo": [ + "id" + ], + "onDelete": "set null", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "policies": {}, + "checkConstraints": {}, + "isRLSEnabled": false + }, "public.workspace_operations": { "name": "workspace_operations", "schema": "", diff --git a/packages/db/src/migrations/meta/_journal.json b/packages/db/src/migrations/meta/_journal.json index 2e91dea8c03..801be2f3d07 100644 --- a/packages/db/src/migrations/meta/_journal.json +++ b/packages/db/src/migrations/meta/_journal.json @@ -2612,6 +2612,13 @@ "when": 1792713110842, "tag": "0374_rare_wolfpack", "breakpoints": true + }, + { + "idx": 375, + "version": "7", + "when": 1792775097887, + "tag": "0375_short_mercury", + "breakpoints": true } ] } \ No newline at end of file diff --git a/packages/db/src/schema/execution_workspace_restore_points.ts b/packages/db/src/schema/execution_workspace_restore_points.ts index afb5e0aeea4..fd1f6dc8195 100644 --- a/packages/db/src/schema/execution_workspace_restore_points.ts +++ b/packages/db/src/schema/execution_workspace_restore_points.ts @@ -35,9 +35,9 @@ export const executionWorkspaceRestorePoints = pgTable( }, (table) => ({ objectKeyUniqueIdx: uniqueIndex("execution_workspace_restore_points_object_key_uniq").on(table.objectKey), - liveGenerationUniqueIdx: uniqueIndex("execution_workspace_restore_points_live_checkout_uniq") + liveGenerationUniqueIdx: uniqueIndex("execution_workspace_restore_points_live_status_uniq") .on(table.executionWorkspaceId, table.generation, table.checkoutName) - .where(sql`${table.status} <> 'deleted'`), + .where(sql`${table.status} in ('pending', 'active', 'expiring')`), statusDeleteAfterIdx: index("execution_workspace_restore_points_status_delete_after_idx").on(table.status, table.deleteAfter), companyIdx: index("execution_workspace_restore_points_company_idx").on(table.companyId), sourceIssueIdx: index("execution_workspace_restore_points_source_issue_idx").on(table.companyId, table.sourceIssueId), diff --git a/server/src/__tests__/git-workspace-expiration.test.ts b/server/src/__tests__/git-workspace-expiration.test.ts index 4a5b6a7d307..8e9db5c0b8c 100644 --- a/server/src/__tests__/git-workspace-expiration.test.ts +++ b/server/src/__tests__/git-workspace-expiration.test.ts @@ -389,6 +389,24 @@ describeEmbeddedPostgres("reconstructable Git expiration", () => { vi.unstubAllEnvs(); }); + it("expires despite a stale pending restore point left by an earlier attempt", async () => { + vi.stubEnv("PAPERCLIP_SECRETS_MASTER_KEY", "31".repeat(32)); + const f = await nestedFixture(false); + await unpushedNestedWork(f.nested.cwd); + const generation = Number((await f.row()).metadata?.lifecycleGeneration ?? 0) + 1; + const base = { companyId: f.companyId, executionWorkspaceId: f.workspaceId, sourceIssueId: f.issueId, generation, + sha256: "0".repeat(64), byteSize: 1 }; + const [stale] = await db.insert(executionWorkspaceRestorePoints).values([ + { ...base, checkoutName: path.basename(f.nested.cwd), objectKey: `stale/${randomUUID()}.pcrp`, status: "pending" }, + { ...base, checkoutName: "", objectKey: `stale/${randomUUID()}.pcrp`, status: "superseded" }, + ]).returning(); + expect(await f.expire({ expireBase: false, restorePoint: restorePoint() })).toMatchObject({ expired: true, restorePoint: true, skipReason: null }); + const rows = await ledger(f.workspaceId); + expect(rows.find(row => row.id === stale!.id)).toMatchObject({ status: "superseded" }); + expect(rows.filter(row => row.status === "active").map(row => row.checkoutName)).toEqual([path.basename(f.nested.cwd)]); + vi.unstubAllEnvs(); + }); + it("keeps a workspace whose nested checkout has unpushed work on another branch", async () => { vi.stubEnv("PAPERCLIP_SECRETS_MASTER_KEY", "31".repeat(32)); const f = await nestedFixture(false); diff --git a/server/src/services/git-restore-point-storage.ts b/server/src/services/git-restore-point-storage.ts index 1b7fb4bc9b9..32fb3dc15df 100644 --- a/server/src/services/git-restore-point-storage.ts +++ b/server/src/services/git-restore-point-storage.ts @@ -101,6 +101,11 @@ export async function storeGitRestorePoint(input: { const objectKey = restorePointObjectKey(input.workspace.companyId, input.workspace.id, input.generation, id, input.checkout); const [ledger] = await input.db.transaction(async (tx) => { await notifyGitRestorePointWork(tx, `workspace:${input.workspace.id}`); + await tx.update(executionWorkspaceRestorePoints).set({ status: "superseded", updatedAt: new Date() }) + .where(and(eq(executionWorkspaceRestorePoints.executionWorkspaceId, input.workspace.id), + eq(executionWorkspaceRestorePoints.generation, input.generation), + eq(executionWorkspaceRestorePoints.checkoutName, input.checkout ?? ""), + eq(executionWorkspaceRestorePoints.status, "pending"))); return tx.insert(executionWorkspaceRestorePoints).values({ companyId: input.workspace.companyId, executionWorkspaceId: input.workspace.id,