From bf818280f2806366f23013e127232bbc1597fd62 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Mon, 9 Nov 2026 17:28:23 -0600 Subject: [PATCH] perf(workspaces): reserve cohort members concurrently (#20401) ## Thinking Path > - Paperclip controls agent work and preserves its workspaces. > - The online importer moves up to eight shared-source leaves under one source fence. > - Each member must have a durable reservation before capture starts. > - Serial reservations repeat the same validation cost before any archive work. > - This pull request reserves members in parallel through their existing independent transactions. > - Every reservation finishes before an error can release the fence or start capture. ## Linked Issues or Issue Description Refs: #19873, #20387, #20394. **Current behavior** A measured six-member cohort spent about 110 seconds in serial reservations. Each reservation keeps its existing source, task, recovery and journal checks. **Proposed behavior** Start the bounded member reservations together. Join all outcomes, retain member order, and refuse capture if any member fails. Keep the exact partial-journal resume contract. **Reason and benefit** This removes serial metadata work before capture. Hosted speedup has not been measured for this change. ## What Changed - Replace only the cohort reservation caller loop with `Promise.allSettled`. - Preserve the existing per-member transaction, preassigned operation ID, source fingerprint and all source fences. - Extend the real PostgreSQL failure fixture to require overlapping reservations and complete peer closure before fence release. - Document the unchanged partial-journal and atomic publication contract. ## Verification - Canonical cached worktree preflight and plugin SDK prerequisites passed on Node 24.22.2. - The unchanged production baseline failed the new reservation overlap assertion: one reader entered instead of two. The canonical PostgreSQL wrapper cleaned up. The other 21 tests were intentionally not selected. - The first baseline command failed before PostgreSQL because root-level `tsx` resolution was absent. The corrected command uses the checkout's explicit `cli/node_modules/tsx/dist/loader.mjs`. - The four focused PostgreSQL controls passed before the final rebase: overlap/join failure, exact partial-journal resume, parallel capture with both cold restores, and atomic outer-commit rollback. Eighteen tests were intentionally not selected. Server TypeScript passed. - The final branch is based on merged #20394 at `302ada90dfda0e8fbcd8a514d506d310c9843bcb`. The two directly affected PostgreSQL reservation/resume controls passed again on final head `06d436e45705da3b616a2b91725960f6e0ea52c0` (2 passed, 20 intentionally not selected; 100.56 seconds). Server TypeScript passed again. The canonical wrapper completed cleanup. - The separate Ponytail review remains 4/5. Its nonblocking fan-out warning was resolved as the documented phase contract; no source change was required. Optional multi-error aggregation is outside this change. - Fresh Greptile review is 5/5 on `06d436e45705da3b616a2b91725960f6e0ea52c0`, with no actionable findings or unresolved threads: https://github.com/paperclipai/paperclip/pull/20401#issuecomment-6754025480. - Exact-head CI is complete: 56 success, 4 skipped and 1 neutral, with no failed or pending checks. Aggregate `ci / verify` passed: https://github.com/paperclipai/paperclip/actions/runs/42718439355. - Reproduce the final focused cases on Node 24: `node --import ./cli/node_modules/tsx/dist/loader.mjs packages/db/src/test-postgres-command.ts pnpm exec vitest run server/src/services/remote-workspace-cohort-migration.test.ts -t "joins concurrent reservation validation readers|resumes the exact membership after reservation stops halfway"`. Run `pnpm --filter @paperclipai/server exec tsc --noEmit` for TypeScript. ## Risks - API phase contract: `concurrency` limits capture workers. Preflight, reservation and post-finalizer validation fan out over the exact selected member set, with the canonical maximum of eight. The original DB pool bounds connections. `concurrency: 1` does not promise one metadata query or transaction at a time. - Up to eight reservations can use independent pool connections at once. Members that pass can leave durable journals when a peer fails. The existing exact-member resume path retains those journals; this change does not retry automatically. - Capture and publication remain blocked until every reservation settles. Publication stays serial in one transaction. Saved journal schemas, singleton behavior and recovery proof checks do not change. - This does not change concurrency inside the publication transaction. One PostgreSQL connection serializes that SQL, and changing its row-lock order has no measured benefit here. ## Model Used OpenAI GPT-6 (Codex), with tool use and code execution. The runtime does not expose a more specific model identifier or context-window size. ## 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 --- doc/plans/2026-11-07-online-git-source-fence.md | 4 ++++ .../remote-workspace-cohort-migration.test.ts | 7 ++++--- server/src/services/remote-workspace-migration.ts | 11 +++++++---- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/doc/plans/2026-11-07-online-git-source-fence.md b/doc/plans/2026-11-07-online-git-source-fence.md index 434287d7276..e67165c3406 100644 --- a/doc/plans/2026-11-07-online-git-source-fence.md +++ b/doc/plans/2026-11-07-online-git-source-fence.md @@ -38,6 +38,10 @@ their existing reconstruction path. Preparation, lifecycle, runtime-service, expiration, and reopen owners retain the same physical keys before filesystem mutation or lifecycle exclusion. The importer holds all cohort task/project and lifecycle keys through publication. +Each cohort reserves its at most eight members in independent transactions in +parallel. It joins every reservation before starting capture or releasing the +source fence, including on failure. Partial journals keep the full exact member +and operation set for resume; publication stays serial in one transaction. It also holds the existing per-run physical finalizer keys for each exact native recovery proof. It re-inspects the complete source proof before capture and publication. Changed descriptors, allocations, owners, aliases, or Git metadata diff --git a/server/src/services/remote-workspace-cohort-migration.test.ts b/server/src/services/remote-workspace-cohort-migration.test.ts index 25fe135d197..21cb2de46ba 100644 --- a/server/src/services/remote-workspace-cohort-migration.test.ts +++ b/server/src/services/remote-workspace-cohort-migration.test.ts @@ -313,10 +313,10 @@ describe.skipIf(!support.supported)("bounded workspace cohort migration", () => expect.soft(await db.select().from(workspaceOperations)).toHaveLength(1); await assertLocal(f); }); - it.each(["preflight", "recovery"] as const)("joins concurrent %s validation readers before releasing the fence", async phase => { + it.each(["preflight", "reservation", "recovery"] as const)("joins concurrent %s validation readers before releasing the fence", async phase => { const f = await fixture(), held = await recoverable(f), inspect = onlineWorkspaceSource.inspectOnlineWorkspaceSource; const counts = new Map(), selected = new Set(f.leaves.map(row => row.id)); - const phaseCall = phase === "preflight" ? 2 : 4; + const phaseCall = phase === "preflight" ? 2 : phase === "reservation" ? 3 : 4; let entered = 0, settled = false, bothEntered!: () => void, release!: () => void, firstFailed!: () => void; const both = new Promise(resolve => { bothEntered = resolve; }), peer = new Promise(resolve => { release = resolve; }); const failed = new Promise(resolve => { firstFailed = resolve; }), timer = setTimeout(() => bothEntered(), 5000); @@ -344,7 +344,8 @@ describe.skipIf(!support.supported)("bounded workspace cohort migration", () => await expect(withWorkspaceSourceLocks(db, { companyId: f.companyId, projectIds: [f.projectId] }, async () => true, true)).resolves.toBe(true); expect((await db.select().from(nativeRunFinalizations).where(eq(nativeRunFinalizations.runId, held.runId)))[0]).toEqual(held.owner); await assertLocal(f); - expect(await db.select().from(workspaceOperations)).toHaveLength(phase === "preflight" ? 0 : 2); + expect(await db.select().from(workspaceOperations)).toHaveLength(phase === "preflight" ? 0 : phase === "reservation" ? 1 : 2); + expect(await db.select().from(executionWorkspaceSnapshots)).toHaveLength(0); expect(await db.select().from(activityLog)).toHaveLength(0); }); it.each(["failure", "cancel"] as const)("joins active capture peers and keeps the fence on %s", async mode => { diff --git a/server/src/services/remote-workspace-migration.ts b/server/src/services/remote-workspace-migration.ts index 6819091b8ac..0b686588bf5 100644 --- a/server/src/services/remote-workspace-migration.ts +++ b/server/src/services/remote-workspace-migration.ts @@ -773,13 +773,16 @@ export async function migrateRemoteWorkspaceCohort(input: { if (!await tryAcquireExecutionWorkspaceLifecycleLock(lock, workspaceId)) throw new IsolatedRepositorySourceBusy("workspace_source_busy"); } await preflight(input.db); - const plans: MigrationPlan[] = []; - for (const member of members) { + const reservations = await Promise.allSettled(members.map(async member => { const reserved = await reserveMigration(member.input, member.onlineSource, cohort, fingerprint(initial.find(workspace => workspace.id === member.input.workspaceId)!)); if (reserved.authority) throw new Error("workspace_cohort_membership_changed"); - plans.push({ ...member, reserved }); - } + return { ...member, reserved }; + })); + const plans: MigrationPlan[] = reservations.map(reservation => { + if (reservation.status === "rejected") throw reservation.reason; + return reservation.value; + }); await lockMigrationRecovery(lock, plans, cohort); const captures: Array = Array(plans.length); const childVerifier = createMigrationChildVerifier(plans[0].input, plans[0].onlineSource, lock);