mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-22 16:51:28 +02:00
## Thinking Path > - Paperclip controls agent work across service deployments. > - A run can provision a remote sandbox before a process or invocation event exists. > - Each container previously treated its own missing process handle as proof that the run was orphaned. > - Overlapping deployments could therefore fail a run owned by another container. > - This pull request records and renews a controller lease before provisioning. > - A recovery worker must revoke an expired owner before it finalizes the run. ## Linked Issues or Issue Description Merged PR #13272 records startup adapter identity and restores explicit user continuation. This PR adds controller ownership on top of current master. Refs #7997 and #10442 for related replica and ownership problems. Related #13138 addresses silence and detached local processes; this change does not infer death from silence. **What happened?** During an overlapping hosted service deployment, a new container reaped a legacy conversation run that another container was provisioning. The run had no PID or adapter invocation yet. **Expected behavior** A live controller keeps its run. After controller loss, one recovery worker takes cleanup authority and the old controller cannot dispatch further work. **Steps to reproduce** Claim a legacy run in controller A. Start controller B against the same database before A finishes provisioning. Run the startup reaper in B. **Paperclip version or commit** Observed on `663c44cb2b9c28336d38d0b4a6971f4f1964bce6` in a hosted Railway deployment with a Daytona environment. ## What Changed - Add nullable controller boot ID, lease deadline, and execution stage columns. Claim ownership in the queued-to-running update. - Renew ownership independently of run output. Abort and reject dispatch if renewal fails. - Serialize reaper revocation against renewal. Let unfinished recovery claims expire after a restart. - Restrict graceful shutdown to legacy runs owned by the current controller. - Hand ownership back to the existing native coordinator when runtime selection becomes native. - Add twelve database regressions and document the lease contract. Update the task-drain regression to require controller expiry before reaping. ## Verification - `pnpm exec vitest run server/src/services/legacy-controller-lease.test.ts server/src/__tests__/heartbeat-task-drain-admission-release.test.ts`: 14 passed after rebasing onto master (`f12b647ae`). - Queue-interruption regressions in `heartbeat-process-recovery.test.ts`: 2 passed after preserving the new cleanup promotion from #13275. - `pnpm --filter @paperclipai/server exec tsc --noEmit`: passed after rebuilding runner TypeScript outputs for the updated master. Broad local tests are omitted at the maintainer’s request; CI owns broad coverage. - Latest-head CI passed on `f255e8e4d5ab2b24b12638a02434e6aa8a2285c5`: [run 34658248569](https://github.com/paperclipai/paperclip/actions/runs/34658248569). All test shards, browser suites, typecheck, build, canary, and security checks passed. Greptile is 5/5 with no unresolved review threads. ## Risks - Additive, idempotent migration; historical rows retain the previous recovery behavior. - Database unavailability aborts new dispatch rather than permitting an unfenced controller to continue. - Lease expiry is permission to clean up, not evidence that remote inference stopped. Follow-up PRs add persistent cleanup and automatic continuation. - Mixed-version deployment still includes old binaries whose reapers do not understand controller leases. ## Model Used OpenAI GPT-6 through Codex, using reasoning, repository inspection, code execution, and test tools. The precise backend revision and context-window size are not exposed in this session. ## 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 <noreply@paperclip.ing>
121 lines
6.8 KiB
TypeScript
121 lines
6.8 KiB
TypeScript
import { randomUUID } from "node:crypto";
|
|
import { eq, sql } from "drizzle-orm";
|
|
import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
|
|
import { agents, companies, createDb, heartbeatRuns } from "@paperclipai/db";
|
|
import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "../__tests__/helpers/embedded-postgres.js";
|
|
import { heartbeatService } from "./heartbeat.js";
|
|
import { hasLiveLegacyController, legacyControllerBootId, legacyControllerClaim,
|
|
renewLegacyControllerLease, revokeExpiredLegacyController, watchLegacyControllerLease } from "./legacy-controller-lease.js";
|
|
|
|
const support = await getEmbeddedPostgresTestSupport();
|
|
(support.supported ? describe : describe.skip)("durable legacy controller ownership", () => {
|
|
let database: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>>;
|
|
let db: ReturnType<typeof createDb>;
|
|
beforeAll(async () => {
|
|
database = await startEmbeddedPostgresTestDatabase("legacy-controller-");
|
|
db = createDb(database.connectionString);
|
|
}, 30000);
|
|
afterAll(async () => { await database?.cleanup(); });
|
|
async function seed() {
|
|
const companyId = randomUUID(), agentId = randomUUID();
|
|
await db.insert(companies).values({ id: companyId, name: "Controller test", issuePrefix: `C${companyId.slice(0, 7)}` });
|
|
await db.insert(agents).values({ id: agentId, companyId, name: "Agent", role: "general", adapterType: "claude_local", status: "idle" });
|
|
const [queued] = await db.insert(heartbeatRuns).values({ companyId, agentId }).returning();
|
|
const [run] = await db.update(heartbeatRuns).set({ status: "running", ...legacyControllerClaim("legacy") })
|
|
.where(eq(heartbeatRuns.id, queued.id)).returning();
|
|
return run;
|
|
}
|
|
async function expire(id: string) {
|
|
await db.update(heartbeatRuns).set({ controllerLeaseExpiresAt: sql`clock_timestamp() - interval '1 second'` })
|
|
.where(eq(heartbeatRuns.id, id));
|
|
}
|
|
it("commits ownership with the queued claim before provisioning or logs exist", async () => {
|
|
const run = await seed();
|
|
expect(run).toMatchObject({ status: "running", controllerBootId: legacyControllerBootId, executionStage: "preparing", processPid: null });
|
|
expect(await hasLiveLegacyController(db, run)).toBe(true);
|
|
expect(await revokeExpiredLegacyController(db, run)).toBe(false);
|
|
});
|
|
it("another deployment's startup reaper preserves an unexpired controller", async () => {
|
|
const run = await seed();
|
|
await db.update(heartbeatRuns).set({ controllerBootId: randomUUID() }).where(eq(heartbeatRuns.id, run.id));
|
|
await heartbeatService(db).reapOrphanedRuns({ staleThresholdMs: 0 });
|
|
const [saved] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, run.id));
|
|
expect(saved.status).toBe("running");
|
|
expect(saved.errorCode).toBeNull();
|
|
});
|
|
it.each([false, true])("shutdown preserves a foreign controller (expired: %s)", async expired => {
|
|
const run = await seed();
|
|
await db.update(heartbeatRuns).set({ controllerBootId: randomUUID() }).where(eq(heartbeatRuns.id, run.id));
|
|
if (expired) await expire(run.id);
|
|
const result = await heartbeatService(db).drainRunningRunsForShutdown("SIGTERM", new Date(), [run.id]);
|
|
expect(result.interruptedRunIds).toEqual([]);
|
|
const [saved] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, run.id));
|
|
expect(saved.status).toBe("running");
|
|
});
|
|
it("a current controller renews and records the dispatch boundary", async () => {
|
|
const run = await seed();
|
|
expect(await renewLegacyControllerLease(db, run, "dispatching")).toBe(true);
|
|
const [saved] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, run.id));
|
|
expect(saved.executionStage).toBe("dispatching");
|
|
expect(await revokeExpiredLegacyController(db, run)).toBe(false);
|
|
});
|
|
it("an expired controller cannot renew or dispatch even before a reaper claims it", async () => {
|
|
const run = await seed();
|
|
await expire(run.id);
|
|
expect(await renewLegacyControllerLease(db, run, "dispatching")).toBe(false);
|
|
const controller = new AbortController();
|
|
const watch = watchLegacyControllerLease(db, run, controller);
|
|
try {
|
|
await expect(watch.assertOwned("dispatching")).rejects.toThrow("lease lost");
|
|
expect(controller.signal.aborted).toBe(true);
|
|
} finally { watch.stop(); }
|
|
});
|
|
it("only one competing recovery revokes the observed expired owner", async () => {
|
|
const run = await seed();
|
|
await expire(run.id);
|
|
const attempts = await Promise.all([revokeExpiredLegacyController(db, run), revokeExpiredLegacyController(db, run)]);
|
|
expect(attempts.filter(Boolean)).toHaveLength(1);
|
|
expect(await renewLegacyControllerLease(db, run)).toBe(false);
|
|
});
|
|
it("a crash after revocation permits a later recovery claim", async () => {
|
|
const run = await seed();
|
|
await expire(run.id);
|
|
expect(await revokeExpiredLegacyController(db, run)).toBe(true);
|
|
const [claimed] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, run.id));
|
|
expect(await hasLiveLegacyController(db, claimed)).toBe(true);
|
|
expect(await revokeExpiredLegacyController(db, claimed)).toBe(false);
|
|
await expire(run.id);
|
|
const [saved] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, run.id));
|
|
expect(await revokeExpiredLegacyController(db, saved)).toBe(true);
|
|
});
|
|
it("rejects foreign company renewal and revocation", async () => {
|
|
const run = await seed();
|
|
expect(await renewLegacyControllerLease(db, { ...run, companyId: randomUUID() })).toBe(false);
|
|
await expire(run.id);
|
|
expect(await revokeExpiredLegacyController(db, { ...run, companyId: randomUUID() })).toBe(false);
|
|
});
|
|
it("never turns a terminal run back into owned execution", async () => {
|
|
const run = await seed();
|
|
await db.update(heartbeatRuns).set({ status: "succeeded" }).where(eq(heartbeatRuns.id, run.id));
|
|
expect(await renewLegacyControllerLease(db, run)).toBe(false);
|
|
expect(await revokeExpiredLegacyController(db, run)).toBe(false);
|
|
});
|
|
it("rejects dispatch at the lease deadline even if the database query never settles", async () => {
|
|
const run = await seed();
|
|
const hungDb = { update: () => ({ set: () => ({ where: () => ({ returning: () => new Promise(() => {}) }) }) }) } as unknown as typeof db;
|
|
vi.useFakeTimers();
|
|
const controller = new AbortController();
|
|
const watch = watchLegacyControllerLease(hungDb, { ...run, controllerLeaseExpiresAt: new Date(Date.now() + 100) }, controller);
|
|
try {
|
|
const checked = expect(watch.assertOwned("dispatching")).rejects.toThrow("lease lost");
|
|
await vi.advanceTimersByTimeAsync(101);
|
|
await checked;
|
|
expect(controller.signal.aborted).toBe(true);
|
|
} finally { watch.stop(); vi.useRealTimers(); }
|
|
});
|
|
|
|
it("leaves native controller ownership to the native coordinator", () => {
|
|
expect(legacyControllerClaim("native")).toEqual({});
|
|
});
|
|
});
|