From 82f7a15c9fb9928f6549e70c85e4b2d2df4007cd Mon Sep 17 00:00:00 2001 From: Dotta Date: Fri, 24 Apr 2026 07:37:29 -0500 Subject: [PATCH] Fix subtree cancel status application Co-Authored-By: Paperclip --- .../issue-tree-control-routes.test.ts | 55 +++++++++++++ server/src/routes/issue-tree-control.ts | 81 +++++++++++++++---- 2 files changed, 119 insertions(+), 17 deletions(-) diff --git a/server/src/__tests__/issue-tree-control-routes.test.ts b/server/src/__tests__/issue-tree-control-routes.test.ts index efdf6b9b17a..08dc0d412f2 100644 --- a/server/src/__tests__/issue-tree-control-routes.test.ts +++ b/server/src/__tests__/issue-tree-control-routes.test.ts @@ -195,6 +195,61 @@ describe("issue tree control routes", () => { ); }); + it("still marks affected issues cancelled when run interruption fails", async () => { + const app = await createApp({ + type: "board", + userId: "user-1", + companyIds: ["company-2"], + source: "session", + isInstanceAdmin: false, + }); + mockTreeControlService.createHold.mockResolvedValue({ + hold: { + id: "33333333-3333-4333-8333-333333333333", + mode: "cancel", + reason: "cancel subtree", + }, + preview: { + mode: "cancel", + totals: { affectedIssues: 1 }, + warnings: [], + activeRuns: [ + { + id: "44444444-4444-4444-8444-444444444444", + issueId: "11111111-1111-4111-8111-111111111111", + }, + ], + }, + }); + mockTreeControlService.cancelIssueStatusesForHold.mockResolvedValue({ + updatedIssueIds: ["11111111-1111-4111-8111-111111111111"], + updatedIssues: [], + }); + mockHeartbeatService.cancelRun.mockRejectedValue(new Error("adapter process did not exit")); + + const res = await request(app) + .post("/api/issues/11111111-1111-4111-8111-111111111111/tree-holds") + .send({ mode: "cancel", reason: "cancel subtree" }); + + expect(res.status).toBe(201); + expect(mockHeartbeatService.cancelRun).toHaveBeenCalledWith("44444444-4444-4444-8444-444444444444"); + expect(mockTreeControlService.cancelIssueStatusesForHold).toHaveBeenCalledWith( + "company-2", + "11111111-1111-4111-8111-111111111111", + "33333333-3333-4333-8333-333333333333", + ); + expect(mockLogActivity).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + action: "issue.tree_hold_run_interrupt_failed", + entityId: "44444444-4444-4444-8444-444444444444", + details: expect.objectContaining({ + error: "adapter process did not exit", + }), + }), + ); + }); + it("restores affected issues and can request explicit wakeups", async () => { const app = await createApp({ type: "board", diff --git a/server/src/routes/issue-tree-control.ts b/server/src/routes/issue-tree-control.ts index 1a0ff062c23..917c4454264 100644 --- a/server/src/routes/issue-tree-control.ts +++ b/server/src/routes/issue-tree-control.ts @@ -10,6 +10,26 @@ import { validate } from "../middleware/validate.js"; import { heartbeatService, issueService, issueTreeControlService, logActivity } from "../services/index.js"; import { assertBoard, assertCompanyAccess, getActorInfo } from "./authz.js"; +const TREE_RUN_CANCELLATION_RESPONSE_WAIT_MS = 1_000; + +function errorToMessage(error: unknown) { + return error instanceof Error ? error.message : String(error); +} + +async function waitForRunCancellationTasks(tasks: Promise[]) { + let timeout: ReturnType | null = null; + try { + await Promise.race([ + Promise.all(tasks), + new Promise((resolve) => { + timeout = setTimeout(resolve, TREE_RUN_CANCELLATION_RESPONSE_WAIT_MS); + }), + ]); + } finally { + if (timeout) clearTimeout(timeout); + } +} + export function issueTreeControlRoutes(db: Db) { const router = Router(); const issuesSvc = issueService(db); @@ -91,25 +111,48 @@ export function issueTreeControlRoutes(db: Db) { }, }); + const runCancellationTasks: Promise[] = []; if (result.hold.mode === "pause" || result.hold.mode === "cancel") { const interruptedRunIds = [...new Set(result.preview.activeRuns.map((run) => run.id))]; - for (const runId of interruptedRunIds) { - await heartbeat.cancelRun(runId); - await logActivity(db, { - companyId: root.companyId, - actorType: actor.actorType, - actorId: actor.actorId, - agentId: actor.agentId, - runId: actor.runId, - action: "issue.tree_hold_run_interrupted", - entityType: "heartbeat_run", - entityId: runId, - details: { - holdId: result.hold.id, - rootIssueId: root.id, - reason: result.hold.mode === "pause" ? "active_subtree_pause_hold" : "subtree_cancel_operation", - }, - }); + for (const heartbeatRunId of interruptedRunIds) { + const cancellationTask = (async () => { + try { + await heartbeat.cancelRun(heartbeatRunId); + await logActivity(db, { + companyId: root.companyId, + actorType: actor.actorType, + actorId: actor.actorId, + agentId: actor.agentId, + runId: actor.runId, + action: "issue.tree_hold_run_interrupted", + entityType: "heartbeat_run", + entityId: heartbeatRunId, + details: { + holdId: result.hold.id, + rootIssueId: root.id, + reason: result.hold.mode === "pause" ? "active_subtree_pause_hold" : "subtree_cancel_operation", + }, + }); + } catch (error) { + await Promise.resolve(logActivity(db, { + companyId: root.companyId, + actorType: actor.actorType, + actorId: actor.actorId, + agentId: actor.agentId, + runId: actor.runId, + action: "issue.tree_hold_run_interrupt_failed", + entityType: "heartbeat_run", + entityId: heartbeatRunId, + details: { + holdId: result.hold.id, + rootIssueId: root.id, + reason: result.hold.mode === "pause" ? "active_subtree_pause_hold" : "subtree_cancel_operation", + error: errorToMessage(error), + }, + })).catch(() => null); + } + })(); + runCancellationTasks.push(cancellationTask); } const cancelledWakeups = await treeControlSvc.cancelUnclaimedWakeupsForTree( @@ -158,6 +201,10 @@ export function issueTreeControlRoutes(db: Db) { }); } + if (runCancellationTasks.length > 0) { + await waitForRunCancellationTasks(runCancellationTasks); + } + if (result.hold.mode === "restore") { let statusUpdate; try {