mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-29 04:25:52 +01:00
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 <noreply@paperclip.ing>
This commit is contained in:
1 parent
21137db805
commit
0e365768f0
2 files changed
+73
-1
No files matched your search
@@ -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);
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in new issue
Block a user