From 801d9548e3283259552faa1e05b09ce9a1f5d3ff Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Wed, 4 Nov 2026 16:47:29 -0600 Subject: [PATCH] test(server): cover competing recovery action inserts (#18773) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Recovery owners record one active action for each task source. > - Two owners can read an empty source before either inserts its action. > - PostgreSQL rejects one insert, and Drizzle wraps that conflict. > - The production fix for wrapped conflicts already landed in #18885. > - This pull request adds a real database race test for that fix. ## Linked Issues or Issue Description Refs #18885 and #18544. Related recovery work: #18402 and #18705. **What happened?** [CI run 41585908789](https://github.com/paperclipai/paperclip/actions/runs/41585908789/job/125752385968) failed with `issue_recovery_actions_active_source_uq`. The recovery worker did not handle the wrapped PostgreSQL conflict. #18885 now provides the production fix. **Expected behavior** Both competing recovery owners return the same active action. The existing bounded retry handles the exact wrapped unique constraint. **Steps to reproduce** 1. Create a task in real PostgreSQL. 2. Hold both recovery inserts until both owners read an empty source. 3. Release both writes. 4. Check that the losing transaction records a wrapped `23505` conflict and both service calls return one action. ## What Changed - Retain the newer bounded cause-chain handling from #18885. - Add a deterministic database race test. Check the actual wrapped conflict, both returned action IDs, the saved action count, and the attempt count. - Keep the original real-clock settlement and idle-window assertions. ## Verification - The two owning suites passed **98/98 tests** after resolving the conflict: `pnpm exec vitest run server/src/__tests__/issue-recovery-dependencies.test.ts server/src/__tests__/issue-recovery-actions.test.ts`. - Workspace typechecks passed across the root run and a successful server rerun. The first server attempt could not resolve the local plugin SDK from provider sources. Restoring that checkout-only SDK link fixed resolution. The server typecheck rerun passed. - Full local `pnpm test:run` has not run. The current local full build stopped after the host exhausted disk space during native compilation. Generated caches were removed from this checkout; no source or test setting changed. Fresh CI is complete: 53 successful checks, two expected skips and one neutral security scan. The branch is mergeable, and the exact-head Greptile review is 5/5 with zero unresolved threads. - Greptile scored current head `dbaee78272a5b0f1db87ba33340a63571ef16948` **5/5** with no actionable findings. - Historical red proof: before wrapped-conflict handling, this race returned one rejection. The losing transaction carried PostgreSQL code `23505` and constraint `issue_recovery_actions_active_source_uq`. - Preserve prior CI evidence: an [old serialized job](https://github.com/paperclipai/paperclip/actions/runs/41649267044/job/125964435813) hit an agent-audit setup timeout and a heartbeat wait failure. The exact timing cause remained unproven. Its [single rerun](https://github.com/paperclipai/paperclip/actions/runs/41649267044/job/125972675510) passed. This PR changes no waits or assertions in those tests. ## Risks - Low risk. The final diff adds one database regression test. It changes no production code, schema, API, permissions, runner routing, or workflows. ## Model Used - OpenAI GPT-6.1 Sol (`gpt-6.1-sol`), xhigh reasoning, tool use, and code execution. ## Checklist - [x] I have included a thinking path and specified the model used. - [x] I have searched for related work and linked the production fix. - [x] I have included only public issue and PR references. - [x] The changed test passes against real PostgreSQL. - [x] I have documented verification limits and risks. - [x] All Paperclip CI gates are green. - [x] Greptile is 5/5 with no unresolved comments on the final head. - [x] I will address review comments before requesting merge. Co-authored-by: Paperclip Co-authored-by: Devin Foley --- .../issue-recovery-dependencies.test.ts | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/server/src/__tests__/issue-recovery-dependencies.test.ts b/server/src/__tests__/issue-recovery-dependencies.test.ts index 90537715210..1c46edb89ba 100644 --- a/server/src/__tests__/issue-recovery-dependencies.test.ts +++ b/server/src/__tests__/issue-recovery-dependencies.test.ts @@ -41,6 +41,37 @@ describeIssueRecovery("issue recovery production producers and dependencies", () sql`${heartbeatRuns.contextSnapshot}->>'wakeReason' = 'issue_blockers_resolved'`))).toHaveLength(1); expect(await state.db.select().from(heartbeatRuns).where(eq(heartbeatRuns.companyId, f.companyId))).toHaveLength(1); }, 90_000); + it("settles competing recovery inserts after both owners read an empty source", async () => { + const f = await fixture(); + const issue = await issueService(state.db).create(f.companyId, { title: "Concurrent recovery", status: "todo" }); + const transaction = state.db.transaction.bind(state.db); + let arrivals = 0, release: () => void = () => {}; + const barrier = new Promise(resolve => { release = resolve; }); + const conflicts: unknown[] = []; + vi.spyOn(state.db, "transaction").mockImplementation(async (callback, config) => { + if (++arrivals <= 2) { + if (arrivals === 2) release(); + await barrier; + } + try { return await transaction(callback, config); } + catch (error) { conflicts.push(error); throw error; } + }); + const input = { companyId: f.companyId, sourceIssueId: issue.id, kind: "stranded_assigned_issue" as const, + ownerType: "agent" as const, ownerAgentId: f.agentId, cause: "stranded_assigned_issue", + fingerprint: "concurrent-source", nextAction: "Restore work", evidence: {} }; + const results = await Promise.allSettled([ + issueRecoveryActionService(state.db).upsertSourceScoped(input), + issueRecoveryActionService(state.db).upsertSourceScoped(input), + ]); + expect(conflicts).toHaveLength(1); + expect(conflicts[0]).toMatchObject({ cause: { code: "23505", constraint_name: "issue_recovery_actions_active_source_uq" } }); + expect(results.map(result => result.status)).toEqual(["fulfilled", "fulfilled"]); + const actions = await state.db.select().from(issueRecoveryActions).where(and( + eq(issueRecoveryActions.companyId, f.companyId), eq(issueRecoveryActions.sourceIssueId, issue.id))); + expect(actions).toHaveLength(1); + expect(actions[0]).toMatchObject({ status: "active", attemptCount: 2, fingerprint: input.fingerprint }); + for (const result of results) if (result.status === "fulfilled") expect(result.value.id).toBe(actions[0].id); + }); it("settles saved disposition debt on its original real-clock timers before measuring idle", async () => { const f = await fixture(); const blocker = await issueService(state.db).create(f.companyId, { title: "Saved debt blocker", status: "todo" });