Skip to content

Commit 1483bb8

Browse files
fix: pass plugin workers to issue tree resume (#13757)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Issue tree controls can resume tasks and wake their assigned agents. > - A sandbox-backed run needs the shared plugin worker manager to acquire its lease. > - The issue tree route constructed a heartbeat service without that manager. > - A ready sandbox provider therefore appeared offline and the resumed run failed setup. > - This pull request passes the existing manager to the route and distinguishes missing wiring from a stopped worker. ## Linked Issues or Issue Description Related implementation: #10262 identified this wiring defect in several dispatch paths. This PR applies the issue-tree resume fix to current master and adds coverage in the current route and runtime suites. The other dispatch paths remain in the scope of that earlier PR. **What happened?** Resuming a paused issue subtree can fail to acquire a sandbox lease even while its provider plugin is ready and its worker is running. The error says the worker is not running because this route's heartbeat service never received the process's worker manager. **Expected behavior** Resumed work uses the same worker manager as normal issue dispatch. Missing wiring and an actually stopped worker produce distinct diagnostic errors. **Steps to reproduce** 1. Configure an agent with a plugin-backed sandbox environment and start the provider worker. 2. Pause a subtree assigned to that agent. 3. Release the hold with `metadata.wakeAgents: true`. 4. The old route constructs an unwired heartbeat service and the resumed run fails during setup. **Paperclip version or commit** Reproduced by source inspection and regression coverage against `9d19f98b50`. **Deployment mode** Server with a plugin-backed sandbox environment. ## What Changed - Pass the process's plugin worker manager from `createApp` through issue tree controls to heartbeat dispatch. - Check for a missing manager before reporting the sandbox worker as stopped. - Exercise the resume request with a shared manager and cover both runtime failure cases. Model a stopped worker explicitly in the existing infrastructure-retry fixture. Existing board and company access checks remain covered. ## Verification - Targeted Vitest route/runtime/recovery suites: 17 passed; 384 native database tests skipped because embedded Postgres cannot start on this host. - Direct server `tsc --noEmit` passed. - `pnpm -r typecheck`, server typecheck wrapper, and `pnpm build` were attempted. Their runner dependency requires `cargo`, which is absent on this host. CI must pass these checks before merge. - Full `pnpm test:run` was attempted. The general server stage reported 8,146 passed, 4,747 skipped, and 14 failed tests across 35 failed suites. Failures were embedded Postgres startup errors (with related teardown errors) and 10 cache-directory rename failures on this macOS host. The local run stopped there; Linux CI must pass the complete suite before merge. - All CI gates are green, including the native database recovery suite, workspace typecheck/build, runner checks, and browser tests. Greptile reviewed the current commit at 5/5 with no unresolved threads. - No live provider operations were performed by these tests. ## Risks Low risk: dependency forwarding and error classification only. The route retains board authorization, company boundaries, hold semantics, and existing cancellation/replay guards. The change does not replace a sandbox or discard a retained lease. No schema or API shape changes. ## Model Used OpenAI GPT-6 (Codex), with reasoning, repository tooling, and code execution. ## 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] This fixes existing behavior and does not add planned core features - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have described the issue in-PR following the bug issue template - [x] I have not referenced internal/instance-local Paperclip issues or links - [x] My branch name describes the change and contains no private ticket or instance data - [x] Targeted local tests pass; full-suite and toolchain limits are recorded above - [x] I have added or updated tests where applicable - [x] No documentation changes are needed for dependency forwarding - [x] I have considered and documented 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 reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
1 parent e8c8ba3 commit 1483bb8

6 files changed

Lines changed: 95 additions & 14 deletions

File tree

server/src/__tests__/environment-runtime.test.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,29 @@ describeEmbeddedPostgres("environmentRuntimeService", () => {
494494
},
495495
);
496496

497+
it("reports an unwired manager separately from a stopped sandbox worker", async () => {
498+
const { companyId, environment, runId } = await seedReusablePluginSandboxLease();
499+
const input = {
500+
companyId,
501+
environment,
502+
issueId: null,
503+
heartbeatRunId: runId,
504+
persistedExecutionWorkspace: null,
505+
};
506+
const runtimeWithoutManager = environmentRuntimeService(db);
507+
await expect(runtimeWithoutManager.acquireRunLease(input))
508+
.rejects.toThrow("sandbox plugin workers are unavailable in this server process");
509+
510+
const offlineManager = { isRunning: () => false, call: vi.fn() } as unknown as PluginWorkerManager;
511+
const runtimeWithStoppedWorker = environmentRuntimeService(db, {
512+
pluginWorkerManager: offlineManager,
513+
pluginWorkerReadyTimeoutMs: 0,
514+
});
515+
await expect(runtimeWithStoppedWorker.acquireRunLease(input))
516+
.rejects.toThrow("its worker is not running");
517+
expect(offlineManager.call).not.toHaveBeenCalled();
518+
});
519+
497520
it("retains a successful reusable sandbox lease without stopping the provider resource", async () => {
498521
const { pluginId, runId, reusableLease } = await seedReusablePluginSandboxLease();
499522
const workerManager = {

server/src/__tests__/heartbeat-process-recovery.test.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,7 @@ import {
212212
redactDetectedSuccessfulRunProgressSummaryForBoard,
213213
redactSuccessfulRunHandoffEvidence,
214214
} from "../services/heartbeat.ts";
215+
import type { PluginWorkerManager } from "../services/plugin-worker-manager.ts";
215216
import {
216217
claimNativeRestartRecoveries,
217218
currentNativeControllerIdentity,
@@ -4221,8 +4222,9 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => {
42214222
});
42224223

42234224
it("schedules an infra retry for a setup failure caused by a transient sandbox provider worker restart", async () => {
4224-
// Reproduces the production incident: the "Kubernetes Sandbox" plugin
4225-
// worker was mid-restart when a run tried to acquire a lease. The lease
4225+
// Model a configured plugin manager whose worker is mid-restart when
4226+
// a run tries to acquire a lease. A missing manager is a wiring error,
4227+
// not a worker restart. The lease
42264228
// acquisition fails BEFORE the adapter is ever dispatched (no call to
42274229
// mockAdapterExecute), so this hits the setup-failure catch (errorCode
42284230
// "setup_failed") rather than the adapter-failure catch. The condition is
@@ -4342,16 +4344,23 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => {
43424344
.set({ status: "in_progress" })
43434345
.where(eq(issues.id, issueId));
43444346

4345-
const heartbeat = heartbeatService(db);
4347+
const heartbeat = heartbeatService(db, {
4348+
pluginWorkerManager: {
4349+
isRunning: () => false,
4350+
call: vi.fn(),
4351+
} as unknown as PluginWorkerManager,
4352+
});
43464353
await heartbeat.resumeQueuedRuns();
43474354

4355+
// The real manager waits up to five seconds for worker readiness before
4356+
// lease acquisition fails and the heartbeat can schedule its retry.
43484357
const runs = await waitForValue(async () => {
43494358
const rows = await db
43504359
.select()
43514360
.from(heartbeatRuns)
43524361
.where(eq(heartbeatRuns.agentId, agentId));
43534362
return rows.length >= 2 ? rows : null;
4354-
});
4363+
}, 10_000);
43554364
expect(runs).toHaveLength(2);
43564365

43574366
const failedRun = runs?.find((row) => row.id === runId);

server/src/__tests__/issue-tree-control-routes.test.ts

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import express from "express";
2+
import type { PluginWorkerManager } from "../services/plugin-worker-manager.js";
23
import { PgDialect } from "drizzle-orm/pg-core";
34
import request from "supertest";
45
import { beforeEach, describe, expect, it, vi } from "vitest";
@@ -28,14 +29,19 @@ const mockHeartbeatService = vi.hoisted(() => ({
2829
wakeup: vi.fn(),
2930
}));
3031

32+
const mockHeartbeatFactory = vi.hoisted(() => vi.fn());
33+
3134
vi.mock("../services/index.js", () => ({
32-
heartbeatService: () => mockHeartbeatService,
35+
heartbeatService: mockHeartbeatFactory,
3336
issueService: () => mockIssueService,
3437
issueTreeControlService: () => mockTreeControlService,
3538
logActivity: mockLogActivity,
3639
}));
3740

38-
async function createApp(actor: Record<string, unknown>) {
41+
async function createApp(
42+
actor: Record<string, unknown>,
43+
pluginWorkerManager?: PluginWorkerManager,
44+
) {
3945
const [{ errorHandler }, { issueTreeControlRoutes }] = await Promise.all([
4046
import("../middleware/index.js"),
4147
import("../routes/issue-tree-control.js"),
@@ -52,14 +58,15 @@ async function createApp(actor: Record<string, unknown>) {
5258
where: (predicate: unknown) => { mockReplayWhere(predicate); return mockReplayBlocks(); },
5359
limit: mockReplayBlocks,
5460
};
55-
app.use("/api", issueTreeControlRoutes({ select: () => query } as any));
61+
app.use("/api", issueTreeControlRoutes({ select: () => query } as any, { pluginWorkerManager }));
5662
app.use(errorHandler);
5763
return app;
5864
}
5965

6066
describe("issue tree control routes", () => {
6167
beforeEach(() => {
6268
vi.clearAllMocks();
69+
mockHeartbeatFactory.mockReturnValue(mockHeartbeatService);
6370
mockReplayBlocks.mockResolvedValue([]);
6471
mockExecutionBlocker.mockResolvedValue(null);
6572
mockTreeControlService.getHold.mockResolvedValue(null);
@@ -185,6 +192,39 @@ describe("issue tree control routes", () => {
185192
},
186193
);
187194

195+
it("resumes sandbox-backed work through the shared plugin worker manager", async () => {
196+
const rootId = "11111111-1111-4111-8111-111111111111";
197+
const holdId = "33333333-3333-4333-8333-333333333333";
198+
const pluginWorkerManager = { isRunning: () => true } as unknown as PluginWorkerManager;
199+
mockHeartbeatFactory.mockImplementation((_db, options) => ({
200+
...mockHeartbeatService,
201+
wakeup: async (...args: unknown[]) => {
202+
// Lease acquisition needs the process's manager, even when its worker
203+
// is already running. A service without it cannot dispatch this run.
204+
expect(options?.pluginWorkerManager).toBe(pluginWorkerManager);
205+
return mockHeartbeatService.wakeup(...args);
206+
},
207+
}));
208+
mockIssueService.getById.mockResolvedValue({
209+
id: rootId, companyId: "company-2", status: "todo", assigneeAgentId: "agent-parent",
210+
});
211+
mockTreeControlService.releaseHold.mockResolvedValue({
212+
id: holdId, mode: "pause", status: "released", members: [{ issueId: rootId }],
213+
});
214+
const app = await createApp({
215+
type: "board", userId: "user-1", companyIds: ["company-2"], source: "session",
216+
}, pluginWorkerManager);
217+
const response = await request(app)
218+
.post(`/api/issues/${rootId}/tree-holds/${holdId}/release`)
219+
.send({ metadata: { wakeAgents: true } });
220+
expect(response.status).toBe(200);
221+
expect(response.body.wakeFailures).toBeUndefined();
222+
expect(mockHeartbeatService.wakeup).toHaveBeenCalledWith("agent-parent", expect.objectContaining({
223+
reason: "issue_tree_resumed",
224+
contextSnapshot: expect.objectContaining({ issueId: rootId, source: "issue.tree_resume" }),
225+
}));
226+
});
227+
188228
it("reports wake failures without undoing release or skipping other assignees", async () => {
189229
const rootId = "11111111-1111-4111-8111-111111111111";
190230
const holdId = "33333333-3333-4333-8333-333333333333";

server/src/app.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -740,7 +740,7 @@ export async function createApp(
740740
api.use(projectToolRoutes(db));
741741
api.use(projectRoutes(db));
742742
api.use(caseRoutes(db, opts.storageService));
743-
api.use(issueTreeControlRoutes(db));
743+
api.use(issueTreeControlRoutes(db, { pluginWorkerManager: workerManager }));
744744
api.use(fileResourceRoutes(db));
745745
api.use(routineRoutes(db, { pluginWorkerManager: workerManager }));
746746
api.use(pipelineRoutes(db));

server/src/routes/issue-tree-control.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ import {
2222
} from "../services/index.js";
2323
import { assertBoard, getAccessibleResource, getActorInfo } from "./authz.js";
2424

25+
import type { PluginWorkerManager } from "../services/plugin-worker-manager.js";
26+
2527
const TREE_RUN_CANCELLATION_RESPONSE_WAIT_MS = 1_000;
2628
const RESUME_EXECUTABLE_STATUSES = ["todo", "in_progress", "in_review"];
2729

@@ -43,11 +45,16 @@ async function waitForRunCancellationTasks(tasks: Promise<void>[]) {
4345
}
4446
}
4547

46-
export function issueTreeControlRoutes(db: Db) {
48+
export function issueTreeControlRoutes(
49+
db: Db,
50+
options: { pluginWorkerManager?: PluginWorkerManager } = {},
51+
) {
4752
const router = Router();
4853
const issuesSvc = issueService(db);
4954
const treeControlSvc = issueTreeControlService(db);
50-
const heartbeat = heartbeatService(db);
55+
const heartbeat = heartbeatService(db, {
56+
pluginWorkerManager: options.pluginWorkerManager,
57+
});
5158

5259
async function resolveRootIssue(req: Request) {
5360
const rootIssueId = req.params.id as string;

server/src/services/environment-runtime.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1785,14 +1785,16 @@ function createSandboxEnvironmentDriver(
17851785
`Sandbox provider "${parsed.config.provider}" is installed via plugin "${pluginProvider.resolved.plugin.pluginKey}", but that plugin is currently ${pluginProvider.resolved.plugin.status}.`,
17861786
);
17871787
}
1788-
if (pluginProvider.state === "worker_unavailable") {
1788+
// A missing manager is a wiring failure, even when the plugin worker
1789+
// is healthy elsewhere in this process. Check it before worker state.
1790+
if (!pluginWorkerManager) {
17891791
throw new Error(
1790-
`Sandbox provider "${parsed.config.provider}" is installed via plugin "${pluginProvider.resolved.plugin.pluginKey}", but its worker is not running.`,
1792+
`Sandbox provider "${parsed.config.provider}" is installed, but sandbox plugin workers are unavailable in this server process.`,
17911793
);
17921794
}
1793-
if (!pluginWorkerManager) {
1795+
if (pluginProvider.state === "worker_unavailable") {
17941796
throw new Error(
1795-
`Sandbox provider "${parsed.config.provider}" is installed, but sandbox plugin workers are unavailable in this server process.`,
1797+
`Sandbox provider "${parsed.config.provider}" is installed via plugin "${pluginProvider.resolved.plugin.pluginKey}", but its worker is not running.`,
17961798
);
17971799
}
17981800

0 commit comments

Comments
 (0)