From b86f55f0ba858fa5495e0822bc28573a03c58221 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Thu, 5 Nov 2026 14:34:06 -0600 Subject: [PATCH] test(server): remove two more server test races (#19330) 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 > - The server Vitest shards gate every pull request > - Two more server tests failed on pull requests that do not touch them (#19309, #19265) > - #19314 fixed four other flakes. These two have different causes > - Random CI failures slow every contributor and hide real regressions > - This pull request removes both root causes without retries or skips > - The benefit is fewer false CI failures ## Linked Issues or Issue Description Refs #19309, #19265, #19314 **What happened?** - #19309, job 127459915333: `workspace-recovery.test.ts` > "keeps ancestor Retry held when current proof changes: sibling_hold" received `requestId: null` from a second Retry request. - #19265, job 127481105563: `paperclip-control-plane-port.test.ts` > "retains a durably recorded governed terminal while its postcommit observer is pending: checkpoint timeout" failed with "native session checkpoint timed out after 100ms" before the terminal was recorded. **Root causes** 1. Workspace recovery (test race). `POST /issues/:id/retry` returns 202 and then starts `heartbeat.resumeTaskRetry` without waiting. The proof-change tests change state after the receipt. When the background nudge runs first, it admits the Retry and queues a run before the change. The new queued run is then the latest run, so the second Retry in `sibling_hold` is stale. I proved this locally: a 1.5 s pause after the receipt made every gate in the family fail with a queued run in the issue history. 2. Control-plane checkpoint timeout (budget too tight). `checkpointTimeoutMs: 100` applies to every checkpoint in the session, including the normal checkpoints before the terminal. On a loaded runner, one of those took more than 100 ms. The test expects only the checkpoint that the pending observer blocks to time out. ## What Changed - `server/src/services/workspace-recovery.test.ts`: add `receiptOnlyRecoveryApp`. It builds the same route app, but the route nudge does nothing. The two proof-change families ("keeps ancestor Retry held…" and "holds a failed-preparation HTTP Retry…") use it and call `resumeTaskRetry` explicitly, as before. - `server/src/services/native-runtime/paperclip-control-plane-port.test.ts`: the checkpoint budget is 1 s. The timeout case now waits for the run to fail with the timeout while the observer is still blocked, instead of a fixed 700 ms sleep. This also proves that the timeout fires while the observer is pending. ## Verification - With the forced 1.5 s pause and the fix applied: both workspace-recovery families pass (80 passed). - `npx vitest run src/services/workspace-recovery.test.ts -t "keeps ancestor Retry held when current proof changes|holds a failed-preparation HTTP Retry when current proof changes"`: 80 passed. - `npx vitest run src/services/native-runtime/paperclip-control-plane-port.test.ts`: 94 passed, run 3 times. ## Risks - Low risk. The changes are only in tests. The route nudge is a latency hint ("Maintenance redelivers it"). Other tests still cover it. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edit, and GitHub CLI). ## 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 - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 --- .../paperclip-control-plane-port.test.ts | 6 +++--- server/src/services/workspace-recovery.test.ts | 10 ++++++++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/server/src/services/native-runtime/paperclip-control-plane-port.test.ts b/server/src/services/native-runtime/paperclip-control-plane-port.test.ts index bae43dd7f39..1de7c9a6b37 100644 --- a/server/src/services/native-runtime/paperclip-control-plane-port.test.ts +++ b/server/src/services/native-runtime/paperclip-control-plane-port.test.ts @@ -390,7 +390,7 @@ describe("PaperclipControlPlanePort conformance", () => { }; }, }; const options = { backend, controlPlane: port, timeoutMs: 2_000, - ...(mode === "checkpoint timeout" ? { checkpointTimeoutMs: 100 } : {}), + ...(mode === "checkpoint timeout" ? { checkpointTimeoutMs: 1_000 } : {}), runnerInstanceId: runnerId, controlPlaneInstanceId: "park-control", input: parseNativeExecutionInput({ schema: "paperclip.native-execution-input.v1", binding: { companyId: identity.companyId, runId: identity.runId, issueId: identity.issueId, agentId: identity.agentId, executionWorkspaceId: "park-workspace" }, @@ -414,13 +414,13 @@ describe("PaperclipControlPlanePort conformance", () => { expect(rows.some(row => row.eventType === "turn.interrupted")).toBe(true); expect(triggerConsumed).toBe(true); if (mode !== "unscoped terminal") { - await new Promise(resolve => setTimeout(resolve, 700)); + if (mode === "settles") await new Promise(resolve => setTimeout(resolve, 700)); + else expect(await outcome).toMatchObject({ error: { message: "native session checkpoint timed out after 1000ms" } }); expect(await db.select().from(nativeRunResults).where(eq(nativeRunResults.runId, identity.runId))).toEqual([]); releaseObserver(); await observerDone; if (mode === "settles") expect(await outcome).not.toHaveProperty("error"); else { - expect(await outcome).toMatchObject({ error: { message: "native session checkpoint timed out after 100ms" } }); expect(await db.select().from(nativeRunResults).where(eq(nativeRunResults.runId, identity.runId))).toEqual([]); const restored = new PaperclipControlPlanePort(db, { ...identity, completionContractId: fixtureContractId, completionContractSha256: contractSha, sourceInstanceId: runnerId, controlPlaneSourceInstanceId: "park-control" }); diff --git a/server/src/services/workspace-recovery.test.ts b/server/src/services/workspace-recovery.test.ts index 580a10b3263..b99542a255b 100644 --- a/server/src/services/workspace-recovery.test.ts +++ b/server/src/services/workspace-recovery.test.ts @@ -627,6 +627,12 @@ describe("automatic retained workspace recovery", () => { return app; } + function receiptOnlyRecoveryApp(actor?: Parameters[0]) { + const routeHeartbeat = heartbeatService(db); + vi.spyOn(routeHeartbeat, "resumeTaskRetry").mockImplementation(async () => undefined as never); + return boardRecoveryApp(actor, routeHeartbeat); + } + type RecoverySourceLayout = "retained" | "published" | "raw"; async function acceptedRemoteExport(sourceLayout: RecoverySourceLayout = "retained") { @@ -1425,7 +1431,7 @@ describe("automatic retained workspace recovery", () => { "keeps ancestor Retry held when current proof changes: %s", async gate => { const f = await recoveredPreparationRetryChain(["warning_reason", "provider_warning", "extra_after_warning"].includes(gate) ? "historical_warning" : "inherited_session"); - const heartbeat = heartbeatService(db), app = boardRecoveryApp(); + const heartbeat = heartbeatService(db), app = receiptOnlyRecoveryApp(); const original = readSystemRecovery((await action(f.actionId)).evidence)!; const receipt = (await request(app).post(`/api/issues/${f.issueId}/retry`) .send({ observedRunId: f.failed.id, idempotencyKey: randomUUID() }).expect(202)).body; @@ -1607,7 +1613,7 @@ describe("automatic retained workspace recovery", () => { await db.insert(authUsers).values({ id: actorId, name: "Retry owner", email: `${actorId}@example.invalid`, createdAt: new Date(), updatedAt: new Date() }); await db.insert(companyMemberships).values({ companyId: f.companyId, principalType: "user", principalId: actorId, membershipRole: "owner" }); } - const app = boardRecoveryApp({ userId: actorId, source: gate === "actor_permission_revoked" ? "session" : "local_implicit", companyIds: [f.companyId] }); + const app = receiptOnlyRecoveryApp({ userId: actorId, source: gate === "actor_permission_revoked" ? "session" : "local_implicit", companyIds: [f.companyId] }); const receipt = (await request(app).post(`/api/issues/${f.issueId}/retry`) .send({ observedRunId: f.runId, idempotencyKey: randomUUID() }).expect(202)).body; if (gate === "delayed_provider_event") await appendHeartbeatRunEvent(db, { companyId: f.companyId, runId: f.runId,