Address subtree hold review findings

packages/db/src/migrations/meta/_journal.json

@@ -467,7 +467,7 @@
     {
       "idx": 66,
       "version": "7",
-      "when": 1776786127000,
+      "when": 1776903901000,
       "tag": "0066_issue_tree_holds",
       "breakpoints": true
     }

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

@@ -256,4 +256,48 @@ describe("issue tree control routes", () => {
     );
     expect(res.body.hold.status).toBe("released");
   });
+
+  it("releases a restore hold if the restore application fails", async () => {
+    const app = await createApp({
+      type: "board",
+      userId: "user-1",
+      companyIds: ["company-2"],
+      source: "session",
+      isInstanceAdmin: false,
+    });
+    mockTreeControlService.createHold.mockResolvedValue({
+      hold: {
+        id: "66666666-6666-4666-8666-666666666666",
+        mode: "restore",
+        reason: "restore subtree",
+      },
+      preview: {
+        mode: "restore",
+        totals: { affectedIssues: 1 },
+        warnings: [],
+        activeRuns: [],
+      },
+    });
+    mockTreeControlService.restoreIssueStatusesForHold.mockRejectedValue(new Error("restore failed"));
+    mockTreeControlService.releaseHold.mockResolvedValue({
+      id: "66666666-6666-4666-8666-666666666666",
+      mode: "restore",
+      status: "released",
+    });
+
+    const res = await request(app)
+      .post("/api/issues/11111111-1111-4111-8111-111111111111/tree-holds")
+      .send({ mode: "restore", reason: "restore subtree" });
+
+    expect(res.status).toBe(500);
+    expect(mockTreeControlService.releaseHold).toHaveBeenCalledWith(
+      "company-2",
+      "11111111-1111-4111-8111-111111111111",
+      "66666666-6666-4666-8666-666666666666",
+      expect.objectContaining({
+        reason: "Restore operation failed before subtree status updates completed",
+        metadata: { cleanup: "restore_failed_before_apply" },
+      }),
+    );
+  });
 });

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

@@ -159,10 +159,22 @@ export function issueTreeControlRoutes(db: Db) {
     }
 
     if (result.hold.mode === "restore") {
-      const statusUpdate = await treeControlSvc.restoreIssueStatusesForHold(root.companyId, root.id, result.hold.id, {
-        reason: result.hold.reason,
-        actor: actorInput,
-      });
+      let statusUpdate;
+      try {
+        statusUpdate = await treeControlSvc.restoreIssueStatusesForHold(root.companyId, root.id, result.hold.id, {
+          reason: result.hold.reason,
+          actor: actorInput,
+        });
+      } catch (error) {
+        await treeControlSvc.releaseHold(root.companyId, root.id, result.hold.id, {
+          reason: "Restore operation failed before subtree status updates completed",
+          metadata: {
+            cleanup: "restore_failed_before_apply",
+          },
+          actor: actorInput,
+        }).catch(() => null);
+        throw error;
+      }
       if (statusUpdate.restoreHold) {
         result = { ...result, hold: statusUpdate.restoreHold };
       }

server/src/services/heartbeat.ts

@@ -80,7 +80,10 @@ import {
   sanitizeRuntimeServiceBaseEnv,
 } from "./workspace-runtime.js";
 import { issueService } from "./issues.js";
-import { issueTreeControlService } from "./issue-tree-control.js";
+import {
+  ISSUE_TREE_CONTROL_INTERACTION_WAKE_REASONS,
+  issueTreeControlService,
+} from "./issue-tree-control.js";
 import {
   getIssueContinuationSummaryDocument,
   refreshIssueContinuationSummary,
@@ -1252,17 +1255,11 @@ function shouldRequireIssueCommentForWake(
   );
 }
 
-const ISSUE_INTERACTION_WAKE_REASONS = new Set([
-  "issue_commented",
-  "issue_reopened_via_comment",
-  "issue_comment_mentioned",
-]);
-
 function allowsIssueInteractionWake(
   contextSnapshot: Record<string, unknown> | null | undefined,
 ) {
   const wakeReason = readNonEmptyString(contextSnapshot?.wakeReason);
-  if (!wakeReason || !ISSUE_INTERACTION_WAKE_REASONS.has(wakeReason)) return false;
+  if (!wakeReason || !ISSUE_TREE_CONTROL_INTERACTION_WAKE_REASONS.has(wakeReason)) return false;
   return Boolean(deriveCommentId(contextSnapshot, null));
 }
 

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

@@ -70,6 +70,12 @@ type RestoreTreeStatusResult = TreeStatusUpdateResult & {
 const TERMINAL_ISSUE_STATUSES = new Set<IssueStatus>(["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<string> = new Set([
+  "issue_commented",
+  "issue_reopened_via_comment",
+  "issue_comment_mentioned",
+] as const);
 
 function normalizeReleasePolicy(
   releasePolicy: IssueTreeHoldReleasePolicy | null | undefined,
@@ -421,8 +427,9 @@ export function issueTreeControlService(db: Db) {
     const holdByRootIssueId = new Map(activePauseHolds.map((hold) => [hold.rootIssueId, hold]));
     let currentIssueId: string | null = issueId;
     const visited = new Set<string>();
+    let depth = 0;
 
-    while (currentIssueId && !visited.has(currentIssueId)) {
+    while (currentIssueId && !visited.has(currentIssueId) && depth < MAX_PAUSE_HOLD_GATE_DEPTH) {
       visited.add(currentIssueId);
       const hold = holdByRootIssueId.get(currentIssueId);
       if (hold) {
@@ -443,6 +450,7 @@ 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;

server/src/services/issues.ts

@@ -37,6 +37,7 @@ import { redactCurrentUserText } from "../log-redaction.js";
 import { resolveIssueGoalId, resolveNextIssueGoalId } from "./issue-goal-fallback.js";
 import { getDefaultCompanyGoal } from "./goals.js";
 import {
+  ISSUE_TREE_CONTROL_INTERACTION_WAKE_REASONS,
   issueTreeControlService,
   type ActiveIssueTreePauseHoldGate,
 } from "./issue-tree-control.js";
@@ -49,12 +50,6 @@ const ISSUE_LIST_RELATED_QUERY_CHUNK_SIZE = 500;
 export const MAX_CHILD_ISSUES_CREATED_BY_HELPER = 25;
 const MAX_CHILD_COMPLETION_SUMMARIES = 20;
 const CHILD_COMPLETION_SUMMARY_BODY_MAX_CHARS = 500;
-const ISSUE_INTERACTION_WAKE_REASONS = new Set([
-  "issue_commented",
-  "issue_reopened_via_comment",
-  "issue_comment_mentioned",
-]);
-
 function assertTransition(from: string, to: string) {
   if (from === to) return;
   if (!ALL_ISSUE_STATUSES.includes(to)) {
@@ -968,7 +963,7 @@ export function issueService(db: Db) {
       readStringFromRecord(run?.contextSnapshot, "reason");
     return Boolean(
       wakeReason &&
-      ISSUE_INTERACTION_WAKE_REASONS.has(wakeReason) &&
+      ISSUE_TREE_CONTROL_INTERACTION_WAKE_REASONS.has(wakeReason) &&
       readLatestWakeCommentId(run?.contextSnapshot),
     );
   }