From 9465d04755061b8ab66466cbcbdcb35a20e9c900 Mon Sep 17 00:00:00 2001 From: Dotta Date: Tue, 28 Apr 2026 13:23:56 -0500 Subject: [PATCH] Fix subtree resume pause release Co-Authored-By: Paperclip --- .../heartbeat-dependency-scheduling.test.ts | 27 +-- .../issue-tree-control-routes.test.ts | 40 +++++ .../issue-tree-control-service.test.ts | 167 +++++++++++++----- server/src/routes/issue-tree-control.ts | 4 +- server/src/services/issue-tree-control.ts | 110 +++++++++++- 5 files changed, 289 insertions(+), 59 deletions(-) diff --git a/server/src/__tests__/heartbeat-dependency-scheduling.test.ts b/server/src/__tests__/heartbeat-dependency-scheduling.test.ts index 5da58a69951..ec5430e2a0c 100644 --- a/server/src/__tests__/heartbeat-dependency-scheduling.test.ts +++ b/server/src/__tests__/heartbeat-dependency-scheduling.test.ts @@ -672,7 +672,8 @@ describeEmbeddedPostgres("heartbeat dependency-aware queued run selection", () = const companyId = randomUUID(); const agentId = randomUUID(); const rootIssueId = randomUUID(); - const childIssueId = randomUUID(); + const issueChain = Array.from({ length: 17 }, () => randomUUID()); + const deepDescendantIssueId = issueChain.at(-1)!; await db.insert(companies).values({ id: companyId, @@ -705,15 +706,15 @@ describeEmbeddedPostgres("heartbeat dependency-aware queued run selection", () = priority: "medium", assigneeAgentId: agentId, }, - { - id: childIssueId, + ...issueChain.map((issueId, index) => ({ + id: issueId, companyId, - parentId: rootIssueId, - title: "Paused child", + parentId: index === 0 ? rootIssueId : issueChain[index - 1], + title: `Paused desc ${index + 1}`, status: "todo", priority: "medium", assigneeAgentId: agentId, - }, + })), ]); const [hold] = await db .insert(issueTreeHolds) @@ -731,8 +732,8 @@ describeEmbeddedPostgres("heartbeat dependency-aware queued run selection", () = source: "automation", triggerDetail: "system", reason: "issue_blockers_resolved", - payload: { issueId: childIssueId }, - contextSnapshot: { issueId: childIssueId, wakeReason: "issue_blockers_resolved" }, + payload: { issueId: deepDescendantIssueId }, + contextSnapshot: { issueId: deepDescendantIssueId, wakeReason: "issue_blockers_resolved" }, }); expect(blockedWake).toBeNull(); @@ -742,7 +743,7 @@ describeEmbeddedPostgres("heartbeat dependency-aware queued run selection", () = reason: agentWakeupRequests.reason, }) .from(agentWakeupRequests) - .where(sql`${agentWakeupRequests.payload} ->> 'issueId' = ${childIssueId}`) + .where(sql`${agentWakeupRequests.payload} ->> 'issueId' = ${deepDescendantIssueId}`) .then((rows) => rows[0] ?? null); expect(skippedWake).toMatchObject({ status: "skipped", reason: "issue_tree_hold_active" }); @@ -750,7 +751,7 @@ describeEmbeddedPostgres("heartbeat dependency-aware queued run selection", () = await db.insert(issueComments).values({ id: childCommentId, companyId, - issueId: childIssueId, + issueId: deepDescendantIssueId, authorUserId: "board-user", body: "Please respond while this hold is active.", }); @@ -759,7 +760,7 @@ describeEmbeddedPostgres("heartbeat dependency-aware queued run selection", () = source: "on_demand", triggerDetail: "manual", reason: "issue_commented", - payload: { issueId: childIssueId, commentId: childCommentId }, + payload: { issueId: deepDescendantIssueId, commentId: childCommentId }, requestedByActorType: "agent", requestedByActorId: agentId, }); @@ -769,11 +770,11 @@ describeEmbeddedPostgres("heartbeat dependency-aware queued run selection", () = source: "automation", triggerDetail: "system", reason: "issue_commented", - payload: { issueId: childIssueId, commentId: childCommentId }, + payload: { issueId: deepDescendantIssueId, commentId: childCommentId }, requestedByActorType: "user", requestedByActorId: "board-user", contextSnapshot: { - issueId: childIssueId, + issueId: deepDescendantIssueId, commentId: childCommentId, wakeCommentId: childCommentId, wakeReason: "issue_commented", diff --git a/server/src/__tests__/issue-tree-control-routes.test.ts b/server/src/__tests__/issue-tree-control-routes.test.ts index 08dc0d412f2..b2de69b1764 100644 --- a/server/src/__tests__/issue-tree-control-routes.test.ts +++ b/server/src/__tests__/issue-tree-control-routes.test.ts @@ -355,4 +355,44 @@ describe("issue tree control routes", () => { }), ); }); + + it("returns resume operations as released holds and avoids cancellation side effects", async () => { + const app = await createApp({ + type: "board", + userId: "user-1", + companyIds: ["company-2"], + source: "session", + isInstanceAdmin: false, + }); + mockTreeControlService.createHold.mockResolvedValue({ + hold: { + id: "77777777-7777-4777-8777-777777777777", + mode: "resume", + status: "released", + reason: "resume subtree", + }, + preview: { + mode: "resume", + totals: { + affectedIssues: 1, + }, + warnings: [], + activeRuns: [], + }, + resumedPauseHoldIds: ["33333333-3333-4333-8333-333333333333"], + }); + + const res = await request(app) + .post("/api/issues/11111111-1111-4111-8111-111111111111/tree-holds") + .send({ mode: "resume", reason: "resume subtree" }); + + expect(res.status).toBe(200); + expect(res.body.hold.mode).toBe("resume"); + expect(res.body.hold.status).toBe("released"); + expect(res.body.resumedPauseHoldIds).toEqual(["33333333-3333-4333-8333-333333333333"]); + expect(mockHeartbeatService.cancelRun).not.toHaveBeenCalled(); + expect(mockTreeControlService.cancelUnclaimedWakeupsForTree).not.toHaveBeenCalled(); + expect(mockTreeControlService.cancelIssueStatusesForHold).not.toHaveBeenCalled(); + expect(mockTreeControlService.restoreIssueStatusesForHold).not.toHaveBeenCalled(); + }); }); diff --git a/server/src/__tests__/issue-tree-control-service.test.ts b/server/src/__tests__/issue-tree-control-service.test.ts index 60953c7c05c..17c26f52a5e 100644 --- a/server/src/__tests__/issue-tree-control-service.test.ts +++ b/server/src/__tests__/issue-tree-control-service.test.ts @@ -337,19 +337,20 @@ describeEmbeddedPostgres("issueTreeControlService", () => { }); }); - it("blocks normal checkout but allows comment interaction checkout under a pause hold", async () => { + it("walks pause-hold ancestry beyond 15 levels for checkout and interaction waives", async () => { const companyId = randomUUID(); const agentId = randomUUID(); - const rootIssueId = randomUUID(); - const childIssueId = randomUUID(); + const issuePath = Array.from({ length: 17 }, () => randomUUID()); + const rootIssueId = issuePath[0]; + const deepDescendantIssueId = issuePath.at(-1)!; const rootRunId = randomUUID(); - const childRunId = randomUUID(); + const deepDescendantRunId = randomUUID(); const forgedRunId = randomUUID(); const rootWakeupRequestId = randomUUID(); - const childWakeupRequestId = randomUUID(); + const deepDescendantWakeupRequestId = randomUUID(); const forgedWakeupRequestId = randomUUID(); const rootCommentId = randomUUID(); - const childCommentId = randomUUID(); + const deepDescendantCommentId = randomUUID(); await db.insert(companies).values({ id: companyId, @@ -368,25 +369,17 @@ describeEmbeddedPostgres("issueTreeControlService", () => { runtimeConfig: {}, permissions: {}, }); - await db.insert(issues).values([ - { - id: rootIssueId, + await db.insert(issues).values( + issuePath.map((issueId, index) => ({ + id: issueId, companyId, - title: "Paused root", + parentId: index > 0 ? issuePath[index - 1] : null, + title: `Issue ${index}`, status: "todo", priority: "medium", assigneeAgentId: agentId, - }, - { - id: childIssueId, - companyId, - parentId: rootIssueId, - title: "Paused child", - status: "todo", - priority: "medium", - assigneeAgentId: agentId, - }, - ]); + })), + ); await db.insert(issueComments).values([ { id: rootCommentId, @@ -396,11 +389,11 @@ describeEmbeddedPostgres("issueTreeControlService", () => { body: "Please answer this root issue question.", }, { - id: childCommentId, + id: deepDescendantCommentId, companyId, - issueId: childIssueId, + issueId: deepDescendantIssueId, authorUserId: "board-user", - body: "Please answer this child issue question.", + body: "Please answer this deep descendant issue question.", }, ]); await db.insert(agentWakeupRequests).values([ @@ -424,24 +417,24 @@ describeEmbeddedPostgres("issueTreeControlService", () => { source: "on_demand", triggerDetail: "manual", reason: "issue_commented", - payload: { issueId: childIssueId, commentId: childCommentId }, + payload: { issueId: deepDescendantIssueId, commentId: deepDescendantCommentId }, status: "queued", requestedByActorType: "agent", requestedByActorId: agentId, runId: forgedRunId, }, { - id: childWakeupRequestId, + id: deepDescendantWakeupRequestId, companyId, agentId, source: "automation", triggerDetail: "system", reason: "issue_commented", - payload: { issueId: childIssueId, commentId: childCommentId }, + payload: { issueId: deepDescendantIssueId, commentId: deepDescendantCommentId }, status: "queued", requestedByActorType: "user", requestedByActorId: "board-user", - runId: childRunId, + runId: deepDescendantRunId, }, ]); await db.insert(heartbeatRuns).values([ @@ -470,25 +463,25 @@ describeEmbeddedPostgres("issueTreeControlService", () => { status: "queued", wakeupRequestId: forgedWakeupRequestId, contextSnapshot: { - issueId: childIssueId, + issueId: deepDescendantIssueId, wakeReason: "issue_commented", - commentId: childCommentId, - wakeCommentId: childCommentId, + commentId: deepDescendantCommentId, + wakeCommentId: deepDescendantCommentId, }, }, { - id: childRunId, + id: deepDescendantRunId, companyId, agentId, invocationSource: "automation", triggerDetail: "system", status: "queued", - wakeupRequestId: childWakeupRequestId, + wakeupRequestId: deepDescendantWakeupRequestId, contextSnapshot: { - issueId: childIssueId, + issueId: deepDescendantIssueId, wakeReason: "issue_commented", - commentId: childCommentId, - wakeCommentId: childCommentId, + commentId: deepDescendantCommentId, + wakeCommentId: deepDescendantCommentId, source: "issue.comment", }, }, @@ -500,16 +493,28 @@ describeEmbeddedPostgres("issueTreeControlService", () => { reason: "operator requested pause", actor: { actorType: "user", actorId: "board-user", userId: "board-user" }, }); + const deepDescendantGate = await treeSvc.getActivePauseHoldGate(companyId, deepDescendantIssueId); + expect(deepDescendantGate).toMatchObject({ + holdId: expect.any(String), + rootIssueId, + issueId: deepDescendantIssueId, + isRoot: false, + mode: "pause", + }); const issueSvc = issueService(db); - await expect(issueSvc.checkout(childIssueId, agentId, ["todo"], randomUUID())).rejects.toMatchObject({ + await expect( + issueSvc.checkout(deepDescendantIssueId, agentId, ["todo"], randomUUID()), + ).rejects.toMatchObject({ status: 409, details: expect.objectContaining({ rootIssueId, mode: "pause", }), }); - await expect(issueSvc.checkout(childIssueId, agentId, ["todo"], forgedRunId)).rejects.toMatchObject({ + await expect( + issueSvc.checkout(deepDescendantIssueId, agentId, ["todo"], forgedRunId), + ).rejects.toMatchObject({ status: 409, details: expect.objectContaining({ rootIssueId, @@ -517,9 +522,9 @@ describeEmbeddedPostgres("issueTreeControlService", () => { }), }); - const checkedOutChild = await issueSvc.checkout(childIssueId, agentId, ["todo"], childRunId); + const checkedOutChild = await issueSvc.checkout(deepDescendantIssueId, agentId, ["todo"], deepDescendantRunId); expect(checkedOutChild.status).toBe("in_progress"); - expect(checkedOutChild.checkoutRunId).toBe(childRunId); + expect(checkedOutChild.checkoutRunId).toBe(deepDescendantRunId); const checkedOutRoot = await issueSvc.checkout(rootIssueId, agentId, ["todo"], rootRunId); expect(checkedOutRoot.status).toBe("in_progress"); @@ -552,4 +557,86 @@ describeEmbeddedPostgres("issueTreeControlService", () => { expect(checkedOutLegacyFullPauseRoot.status).toBe("in_progress"); expect(checkedOutLegacyFullPauseRoot.checkoutRunId).toBe(rootRunId); }); + + it("resumes subtree pauses by releasing matching pause holds", async () => { + const companyId = randomUUID(); + const rootIssueId = randomUUID(); + const childIssueId = randomUUID(); + const nonSubtreeIssueId = randomUUID(); + + await db.insert(companies).values({ + id: companyId, + name: "Paperclip", + issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`, + requireBoardApprovalForNewAgents: false, + }); + await db.insert(issues).values([ + { + id: rootIssueId, + companyId, + title: "Root", + status: "todo", + priority: "medium", + }, + { + id: childIssueId, + companyId, + parentId: rootIssueId, + title: "Child", + status: "todo", + priority: "medium", + }, + { + id: nonSubtreeIssueId, + companyId, + title: "Unrelated", + status: "todo", + priority: "medium", + }, + ]); + + const treeSvc = issueTreeControlService(db); + const subtreePause = await treeSvc.createHold(companyId, childIssueId, { + mode: "pause", + reason: "pause child only", + actor: { actorType: "user", actorId: "board-user", userId: "board-user" }, + }); + const nonSubtreePause = await treeSvc.createHold(companyId, nonSubtreeIssueId, { + mode: "pause", + reason: "pause unrelated issue", + actor: { actorType: "user", actorId: "board-user", userId: "board-user" }, + }); + + const resumed = await treeSvc.createHold(companyId, rootIssueId, { + mode: "resume", + reason: "resume subtree", + actor: { actorType: "user", actorId: "board-user", userId: "board-user" }, + }); + + expect(resumed.hold.mode).toBe("resume"); + expect(resumed.hold.status).toBe("released"); + expect(resumed.resumedPauseHoldIds).toEqual([subtreePause.hold.id]); + + const rows = await db + .select({ id: issueTreeHolds.id, status: issueTreeHolds.status, releaseMetadata: issueTreeHolds.releaseMetadata }) + .from(issueTreeHolds) + .where(eq(issueTreeHolds.companyId, companyId)); + const byId = new Map(rows.map((row) => [row.id, row] as const)); + expect(byId.get(subtreePause.hold.id)?.status).toBe("released"); + expect(byId.get(nonSubtreePause.hold.id)?.status).toBe("active"); + expect(byId.get(resumed.hold.id)?.status).toBe("released"); + + const releaseMetadata = byId.get(subtreePause.hold.id)?.releaseMetadata as + | Record + | null; + expect(releaseMetadata).toMatchObject({ + resumedByResumeHoldId: resumed.hold.id, + resumeHoldMode: "tree_resume", + resumedPauseHoldId: subtreePause.hold.id, + }); + expect((byId.get(resumed.hold.id)?.releaseMetadata as Record | null)).toMatchObject({ + resumedPauseHoldIds: [subtreePause.hold.id], + resumeMode: "subtree", + }); + }); }); diff --git a/server/src/routes/issue-tree-control.ts b/server/src/routes/issue-tree-control.ts index 917c4454264..44212128c59 100644 --- a/server/src/routes/issue-tree-control.ts +++ b/server/src/routes/issue-tree-control.ts @@ -291,7 +291,9 @@ export function issueTreeControlRoutes(db: Db) { } } - res.status(result.hold.mode === "restore" ? 200 : 201).json(result); + res + .status(result.hold.mode === "restore" || result.hold.mode === "resume" ? 200 : 201) + .json(result); }); router.get("/issues/:id/tree-control/state", async (req, res) => { diff --git a/server/src/services/issue-tree-control.ts b/server/src/services/issue-tree-control.ts index 04de642e1d5..37c45683604 100644 --- a/server/src/services/issue-tree-control.ts +++ b/server/src/services/issue-tree-control.ts @@ -71,7 +71,6 @@ type RestoreTreeStatusResult = TreeStatusUpdateResult & { const TERMINAL_ISSUE_STATUSES = new Set(["done", "cancelled"]); const ACTIVE_RUN_STATUSES = ["queued", "running"] as const; const DEFAULT_RELEASE_POLICY: IssueTreeHoldReleasePolicy = { strategy: "manual" }; -const MAX_PAUSE_HOLD_GATE_DEPTH = 15; export const ISSUE_TREE_CONTROL_INTERACTION_WAKE_REASONS: ReadonlySet = new Set([ "issue_commented", "issue_reopened_via_comment", @@ -548,6 +547,22 @@ export function issueTreeControlService(db: Db) { return byIssueId; } + async function activePauseHoldsForIssueIds(companyId: string, issueIds: string[]) { + if (issueIds.length === 0) return []; + return db + .select() + .from(issueTreeHolds) + .where( + and( + eq(issueTreeHolds.companyId, companyId), + eq(issueTreeHolds.status, "active"), + eq(issueTreeHolds.mode, "pause"), + inArray(issueTreeHolds.rootIssueId, issueIds), + ), + ) + .orderBy(asc(issueTreeHolds.createdAt), asc(issueTreeHolds.id)); + } + async function getActivePauseHoldGate( companyId: string, issueId: string, @@ -573,9 +588,8 @@ export function issueTreeControlService(db: Db) { const holdByRootIssueId = new Map(activePauseHolds.map((hold) => [hold.rootIssueId, hold])); let currentIssueId: string | null = issueId; const visited = new Set(); - let depth = 0; - while (currentIssueId && !visited.has(currentIssueId) && depth < MAX_PAUSE_HOLD_GATE_DEPTH) { + while (currentIssueId && !visited.has(currentIssueId)) { visited.add(currentIssueId); const hold = holdByRootIssueId.get(currentIssueId); if (hold) { @@ -596,7 +610,6 @@ export function issueTreeControlService(db: Db) { .where(and(eq(issues.id, currentIssueId), eq(issues.companyId, companyId))) .then((rows) => rows[0] ?? null); currentIssueId = parent?.parentId ?? null; - depth += 1; } return null; @@ -690,13 +703,100 @@ export function issueTreeControlService(db: Db) { releasePolicy?: IssueTreeHoldReleasePolicy | null; actor: ActorInput; }, - ) { + ): Promise<{ + hold: IssueTreeHold; + preview: IssueTreeControlPreview; + resumedPauseHoldIds?: string[]; + }> { const holdReleasePolicy = normalizeReleasePolicy(input.releasePolicy); const holdPreview = await preview(companyId, rootIssueId, { mode: input.mode, releasePolicy: holdReleasePolicy, }); + if (input.mode === "resume") { + const issueIds = [...new Set(holdPreview.issues.map((issue) => issue.id))]; + const activePauseHolds = await activePauseHoldsForIssueIds(companyId, issueIds); + const releaseReason = input.reason ?? "Subtree resume applied."; + + const { hold: resumeHold } = await db.transaction(async (tx) => { + const [createdHold] = await tx + .insert(issueTreeHolds) + .values({ + companyId, + rootIssueId, + mode: input.mode, + status: "active", + reason: input.reason ?? null, + releasePolicy: holdReleasePolicy as unknown as Record, + createdByActorType: input.actor.actorType, + createdByAgentId: input.actor.agentId ?? null, + createdByUserId: input.actor.userId ?? (input.actor.actorType === "user" ? input.actor.actorId : null), + createdByRunId: input.actor.runId ?? null, + }) + .returning(); + + const memberRows = holdPreview.issues.map((issue) => ({ + companyId, + holdId: createdHold.id, + issueId: issue.id, + parentIssueId: issue.parentId, + depth: issue.depth, + issueIdentifier: issue.identifier, + issueTitle: issue.title, + issueStatus: issue.status, + assigneeAgentId: issue.assigneeAgentId, + assigneeUserId: issue.assigneeUserId, + activeRunId: issue.activeRun?.id ?? null, + activeRunStatus: issue.activeRun?.status ?? null, + skipped: issue.skipped, + skipReason: issue.skipReason, + })); + + const createdMembers = memberRows.length > 0 + ? await tx + .insert(issueTreeHoldMembers) + .values(memberRows) + .returning() + : []; + + return { hold: toHold(createdHold, createdMembers) }; + }); + + const resumedPauseHoldIds = activePauseHolds.map((hold) => hold.id); + if (resumedPauseHoldIds.length > 0) { + await Promise.all( + activePauseHolds.map((pauseHold) => + releaseHold(companyId, pauseHold.rootIssueId, pauseHold.id, { + reason: releaseReason, + metadata: { + resumedByResumeHoldId: resumeHold.id, + resumeHoldMode: "tree_resume", + resumedPauseHoldId: pauseHold.id, + }, + actor: input.actor, + }), + ), + ); + } + + const releasedResumeHold = await releaseHold(companyId, rootIssueId, resumeHold.id, { + reason: releaseReason, + metadata: { + resumedPauseHoldIds, + resumeMode: "subtree", + ...(input.releasePolicy ? { releasePolicy: holdReleasePolicy } : {}), + }, + actor: input.actor, + }); + + return { + hold: releasedResumeHold, + preview: holdPreview, + resumedPauseHoldIds, + }; + } + const { hold, members } = await db.transaction(async (tx) => { const [createdHold] = await tx .insert(issueTreeHolds)