test(server): fix onboarding first-task teardown foreign-key race (#11284)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The server coordinates issue work and heartbeat runs. > - The onboarding first-task route sends an assignment wake in the background. > - The route test removes related database rows during teardown. > - A late heartbeat run can keep foreign-key child rows alive during teardown. > - This pull request drains the wake and deletes run rows in foreign-key order. > - The benefit is a stable test that keeps the onboarding behavior unchanged. ## Linked Issues or Issue Description **What happened?** The onboarding first-task route sent a background assignment wake. The test teardown removed parent rows before the wake-created heartbeat rows finished. **Expected behavior** The test teardown should wait for the background wake and remove heartbeat rows before it removes their parent rows. **Steps to reproduce** 1. Run the onboarding first-task route test. 2. Repeat the test many times. 3. Observe an intermittent foreign-key error during teardown. **Paperclip version or commit** Commit `c30fe965920eeb7e7fb88e17574a65bed8fc01a4`. **Deployment mode** Local dev (pnpm dev). **Installation method** Built from source (pnpm dev / pnpm build). **Agent adapter(s) involved** Not adapter-specific (core bug). **Database mode** Embedded PGlite (default — DATABASE_URL unset). ## What Changed - Stub the server adapter in the route test so the dispatched run finishes at once. - Drain heartbeat runs to quiescence before teardown. - Delete heartbeat runs and child rows before their parent rows. - Delete runtime state and company skill rows in foreign-key order. - Keep the route behavior and all three test assertions unchanged. ## Verification - Run `pnpm exec vitest run src/__tests__/issue-onboarding-first-task-routes.test.ts` from the `server` package. - The author ran the suite 25 times with 25 passes. - The suite reproduced the teardown foreign-key error before this change. ## Risks Low risk. This change affects one test file and does not change product code or route behavior. ## Model Used OpenAI GPT-5. The model used tool calls and code review assistance. The exact context window and reasoning mode were not exposed in this run. ## 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] 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 <noreply@paperclip.ing>
This commit is contained in:
parent
f9bd0438e1
commit
6a5b293240
|
|
@ -2,13 +2,17 @@ import { randomUUID } from "node:crypto";
|
|||
import express from "express";
|
||||
import request from "supertest";
|
||||
import { and, eq } from "drizzle-orm";
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest";
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
activityLog,
|
||||
agentRuntimeState,
|
||||
agents,
|
||||
agentWakeupRequests,
|
||||
companies,
|
||||
companySkills,
|
||||
createDb,
|
||||
heartbeatRunEvents,
|
||||
heartbeatRuns,
|
||||
issueComments,
|
||||
issues,
|
||||
} from "@paperclipai/db";
|
||||
|
|
@ -19,7 +23,38 @@ import {
|
|||
} from "./helpers/embedded-postgres.js";
|
||||
import { actorMiddleware } from "../middleware/auth.js";
|
||||
import { errorHandler } from "../middleware/index.js";
|
||||
import { runningProcesses } from "../adapters/index.ts";
|
||||
import { issueRoutes } from "../routes/issues.js";
|
||||
import { heartbeatService } from "../services/heartbeat.js";
|
||||
import { drainHeartbeatRunsToQuiescence } from "./helpers/drain-heartbeat-runs.js";
|
||||
|
||||
// The issue-create route dispatches an assignment wake fire-and-forget for an
|
||||
// assigned agent. The wake dispatches a heartbeat run that calls the server
|
||||
// adapter. Stub the adapter so the run finishes at once and does not start a
|
||||
// real agent session. The stub keeps the wake path realistic and fast, so the
|
||||
// afterEach drain reaches quiescence without a long real run.
|
||||
const mockAdapterExecute = vi.hoisted(() =>
|
||||
vi.fn(async () => ({
|
||||
exitCode: 0,
|
||||
signal: null,
|
||||
timedOut: false,
|
||||
errorMessage: null,
|
||||
summary: "Onboarding first-task route test run.",
|
||||
provider: "test",
|
||||
model: "test-model",
|
||||
})),
|
||||
);
|
||||
|
||||
vi.mock("../adapters/index.ts", async () => {
|
||||
const actual = await vi.importActual<typeof import("../adapters/index.ts")>("../adapters/index.ts");
|
||||
return {
|
||||
...actual,
|
||||
getServerAdapter: vi.fn(() => ({
|
||||
supportsLocalAgentJwt: false,
|
||||
execute: mockAdapterExecute,
|
||||
})),
|
||||
};
|
||||
});
|
||||
|
||||
const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport();
|
||||
const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip;
|
||||
|
|
@ -42,11 +77,26 @@ describeEmbeddedPostgres("issue create onboarding first-task routes", () => {
|
|||
}, 20_000);
|
||||
|
||||
afterEach(async () => {
|
||||
// Wait for the background assignment wake to reach quiescence before the
|
||||
// deletes. The wake inserts agent_wakeup_requests and heartbeat_runs rows
|
||||
// asynchronously, and a late row races the teardown deletes. The module
|
||||
// global promise sets in heartbeat.ts are shared, so a fresh heartbeatService
|
||||
// drains the route wake that this suite started.
|
||||
mockAdapterExecute.mockClear();
|
||||
runningProcesses.clear();
|
||||
await drainHeartbeatRunsToQuiescence(db, heartbeatService(db));
|
||||
// Delete heartbeat_runs and its child rows before agents and
|
||||
// agent_wakeup_requests. heartbeat_runs references both, so a completed run
|
||||
// row blocks a parent delete with a foreign-key violation.
|
||||
await db.delete(activityLog);
|
||||
await db.delete(agentWakeupRequests);
|
||||
await db.delete(issueComments);
|
||||
await db.delete(heartbeatRunEvents);
|
||||
await db.delete(heartbeatRuns);
|
||||
await db.delete(agentWakeupRequests);
|
||||
await db.delete(agentRuntimeState);
|
||||
await db.delete(issues);
|
||||
await db.delete(agents);
|
||||
await db.delete(companySkills);
|
||||
await db.delete(companies);
|
||||
});
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue