From 0e365768f032a472807b5720da8431f92d2f07ce Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Wed, 28 Oct 2026 13:13:36 -0500 Subject: [PATCH] fix(server): preserve sandbox workers when task drain resumes (#18065) ## Thinking Path > - Paperclip coordinates agents and preserves queued work during maintenance. > - Stopping a task drain resumes that queued work. > - Sandbox runs need the configured plugin worker manager to acquire their environment. > - The instance-settings route received that manager but omitted it from its heartbeat service. > - The route could claim queued work and then fail before provider execution. > - This PR forwards the existing manager and tests the actual dispatch boundary. ## Linked Issues or Issue Description Follow-up to #17967, which resumes queued work after maintenance. #18037 adds readiness reporting to the same route but does not repair this dependency. **What happened?** After a task drain stopped, a queued Daytona task failed with `setup_failed`: the provider was installed, but sandbox plugin workers were unavailable in that server process. The application had supplied the manager to `instanceSettingsRoutes`. That route constructed `heartbeatService(db)` without it, then dispatched through that separate service. The environment runtime therefore reached its missing-manager guard before any provider work. **Expected behavior** Drain-release dispatch must use the manager supplied to the route. Queue eligibility, scope and ownership checks must still apply. **Steps to reproduce** 1. Configure a sandbox provider plugin and an agent that uses its environment. 2. Start a task drain and queue work for that agent. 3. Stop the drain through `DELETE /api/instance/task-drain`. 4. Observe dispatch fail before it calls the configured plugin manager. **Paperclip version or commit** Observed at `2e5bbde8cc7eba0eacce00649f17c331203e56d9`. Reproduced against `7f10cc058568e3c1ac133cf24f3b4000cbdfca0e`. **Deployment mode** Hosted Linux with a Daytona sandbox plugin. The local regression uses real PostgreSQL 18 and the actual route, heartbeat, queue, orchestrator and environment runtime. ## What Changed - Pass the supplied plugin worker manager to the route's heartbeat service. - Add a database regression that releases genuinely queued sandbox work through the real HTTP route and checks the scoped acquisition call. ## Verification - Red: the new test executed the route and recorded zero manager calls. The queued run failed with the same missing-manager error as the incident. - Green: the route reached exactly one `environmentAcquireLease` call bound to the expected plugin, company, task, agent, environment and queued run. The fixture then threw a unique sentinel before any external allocation; that sentinel was persisted on the same run. - All 8 database drain-release cases pass. All 101 existing instance-settings and task-drain unit checks pass. `git diff --check` passes. - Local database verification used an owned PostgreSQL 18 container with all 396 migrations. A temporary Vitest configuration replaced only embedded database startup/cleanup. Database queries and dispatch services remained real. The override and credentials are not in this PR. The owned container stopped after the tests. - Preserved setup failures include the initial temporary mock-bootstrap import, an unbuilt SDK entry and a reused synthetic sponsor row. A first fixed-path run also exposed an unnecessary expectation about an optional RPC timeout. That assumption was removed; the run identity, exact call count and scope assertions remain. These failures are not claimed as the behavioral red. - The initial frozen install found existing UI lockfile drift. Setup used the repository's CI resolution fallback. The original lockfile was restored and is unchanged in this PR. - Full `pnpm -r typecheck` and `pnpm build` pass at `be18d8ec1f84af20bad2b6bd97677b1e60c3626f`. Current-head CI passes: 53 successful checks and two intended Storybook skips, none pending or failing. Greptile is 5/5 on that exact head with no open review threads. CI ran the standard embedded PostgreSQL bootstrap and passed all 8 drain-release cases, including the new regression. The full standard local `pnpm test:run` is still in progress; no local full-suite pass is claimed. The earlier targeted database suite used the isolated external fixture. - No post-fix production drain cycle is claimed. The repair is not deployed. An ordinary retry of the observed legacy preparation failure was separately refused by the existing internal retry-safety classifier before admission. This PR does not broaden that classifier or replay the request. The initial CI server-7 shard failed the unchanged decision-expiry restart test: the global coordinator reported a wake at +30 seconds rather than the decision expiry at +120 seconds. The test failed before checking early expiration. A bounded probe of the real work signals, coordinator and decision workers reproduces that earlier wake after a failed transaction is reconciled during startup, without database or provider calls. The captured CI log does not establish that exact trigger. One diagnostic failed-job rerun passed all 1,625 shard tests and the dependent verification gate on unchanged source. The original failure, probe and retry receipts are retained; no scheduler fix or infrastructure cause is claimed. ## Risks The route can now reach the configured sandbox provider when it resumes eligible work. Existing queue claims, company scope, pause and new-drain gates remain in place. The change does not start a second manager, add a wake loop or change the successful drain-release response. Installations without a manager retain the existing missing-manager behavior. No schema, provider binary, credential or retry-authorization contract changes. ## Model Used OpenAI GPT-6 through Codex, with reasoning, tool use, code execution and collaborating agents. The session exposes GPT-6 as its model version; a more specific model ID and context-window size are unavailable. ## 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 --- ...tbeat-task-drain-admission-release.test.ts | 72 +++++++++++++++++++ server/src/routes/instance-settings.ts | 2 +- 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/server/src/__tests__/heartbeat-task-drain-admission-release.test.ts b/server/src/__tests__/heartbeat-task-drain-admission-release.test.ts index 72053aac40c..69d793fd87d 100644 --- a/server/src/__tests__/heartbeat-task-drain-admission-release.test.ts +++ b/server/src/__tests__/heartbeat-task-drain-admission-release.test.ts @@ -17,9 +17,11 @@ import { companyMemberships, companySkills, createDb, + environments, heartbeatRunEvents, heartbeatRuns, issues, + plugins, } from "@paperclipai/db"; import { getEmbeddedPostgresTestSupport, @@ -31,6 +33,7 @@ import express from "express"; import request from "supertest"; import { instanceSettingsRoutes } from "../routes/instance-settings.js"; import { errorHandler } from "../middleware/error-handler.js"; +import type { PluginWorkerManager } from "../services/plugin-worker-manager.js"; const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; @@ -298,6 +301,75 @@ describeEmbeddedPostgres("heartbeat task-drain admission release", () => { } }, 20_000); + it("preserves the plugin worker manager when DELETE releases a queued sandbox run", async () => { + const { companyId, agentId, issueId } = await seedAgentAndIssue(); + const environmentId = randomUUID(); + const pluginId = randomUUID(); + const pluginKey = `test.drain-sandbox.${pluginId}`; + const provider = "drain-sandbox"; + const acquisitionFailure = "drain_release_plugin_acquisition_reached"; + const calls: Array<{ pluginId: string; method: string; params: unknown }> = []; + const pluginWorkers = { + isRunning: (id: string) => id === pluginId, + getWorker: () => ({ supportedMethods: ["environmentAcquireLease"] }), + call: async (id: string, method: string, params: unknown) => { + calls.push({ pluginId: id, method, params }); + throw new Error(acquisitionFailure); + }, + } as unknown as PluginWorkerManager; + const heartbeat = heartbeatService(db, { pluginWorkerManager: pluginWorkers }); + try { + await db.insert(plugins).values({ + id: pluginId, pluginKey, packageName: "@test/drain-sandbox", version: "1.0.0", apiVersion: 1, + categories: ["automation"], status: "ready", + manifestJson: { + id: pluginKey, apiVersion: 1, version: "1.0.0", displayName: "Drain sandbox", + description: "Task-drain release fixture", author: "Paperclip", categories: ["automation"], + capabilities: ["environment.drivers.register"], entrypoints: { worker: "worker.js" }, + environmentDrivers: [{ driverKey: provider, kind: "sandbox_provider", displayName: "Drain sandbox", configSchema: { type: "object" } }], + }, + }); + await db.insert(environments).values({ id: environmentId, name: `Drain sandbox ${environmentId}`, + driver: "sandbox", status: "active", config: { provider } }); + await db.update(agents).set({ defaultEnvironmentId: environmentId }).where(eq(agents.id, agentId)); + startTaskDrain({}); + await heartbeat.wakeup(agentId, { + source: "assignment", triggerDetail: "system", reason: "issue_assigned", + payload: { issueId }, contextSnapshot: { issueId, wakeReason: "issue_assigned" }, + requestedByActorType: "system", requestedByActorId: "issue_assignment", + }); + await heartbeat.drainActiveRunExecutions(); + const [queued] = await db.select().from(heartbeatRuns); + expect(queued).toMatchObject({ status: "queued", startedAt: null }); + expect(calls).toEqual([]); + + const app = express(); + app.use(express.json()); + app.use((req, _res, next) => { + req.actor = { type: "board", userId: "local-board", source: "local_implicit", isInstanceAdmin: true }; + next(); + }); + app.use("/api", instanceSettingsRoutes(db, pluginWorkers)); + app.use(errorHandler); + await request(app).delete("/api/instance/task-drain").expect(200); + await heartbeat.drainActiveRunExecutions(); + const runs = await db.select({ id: heartbeatRuns.id, status: heartbeatRuns.status, error: heartbeatRuns.error }) + .from(heartbeatRuns); + expect(runs).toHaveLength(1); + expect.soft(runs[0]).toMatchObject({ id: queued!.id, status: "failed", error: expect.stringContaining(acquisitionFailure) }); + expect.soft(calls).toEqual([{ + pluginId, method: "environmentAcquireLease", + params: expect.objectContaining({ driverKey: provider, companyId, environmentId, + issueId, agentId, runId: queued!.id, adapterType: "process" }), + }]); + } finally { + await heartbeat.drainActiveRunExecutions(); + await db.update(agents).set({ defaultEnvironmentId: null }).where(eq(agents.id, agentId)); + await db.delete(environments).where(eq(environments.id, environmentId)); + await db.delete(plugins).where(eq(plugins.id, pluginId)); + } + }, 20_000); + it("dispatches existing queued work at the drain deadline", async () => { const { agentId, issueId } = await seedAgentAndIssue(); const heartbeat = heartbeatService(db); diff --git a/server/src/routes/instance-settings.ts b/server/src/routes/instance-settings.ts index 2a8e78cb99f..b8072b8b308 100644 --- a/server/src/routes/instance-settings.ts +++ b/server/src/routes/instance-settings.ts @@ -136,7 +136,7 @@ export function instanceSettingsRoutes(db: Db, pluginWorkers?: PluginWorkerManag const router = Router(); const svc = instanceSettingsService(db); const environments = environmentService(db); - const heartbeat = heartbeatService(db); + const heartbeat = heartbeatService(db, { pluginWorkerManager: pluginWorkers }); router.get("/instance/settings", async (req, res) => { assertBoardOrgAccess(req);