mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-08 01:57:31 +01:00
test(server): remove two more server test races (#19330)
## 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 <noreply@anthropic.com>
This commit is contained in:
1 parent
be5709a25c
commit
b86f55f0ba
2 files changed
+11
-5
No files matched your search
@@ -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" });
|
||||
|
||||
@@ -627,6 +627,12 @@ describe("automatic retained workspace recovery", () => {
|
||||
return app;
|
||||
}
|
||||
|
||||
function receiptOnlyRecoveryApp(actor?: Parameters<typeof boardRecoveryApp>[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,
|
||||
|
||||
Reference in new issue
Block a user