Merge 2f0625e8f8 into c9e3bb7ca4

doc/execution-semantics.md

@@ -521,7 +521,7 @@ tasks hide both waiting surfaces even if a stale schedule remains in the respons
 
 Because `serviceName` and `notes` remain visible in issue activity and wake context, operators should keep them short and non-secret. Put enough context for the assignee to know what to inspect, but do not include signed URLs, bearer tokens, customer secrets, tenant-private identifiers, or provider links with embedded credentials.
 
-Monitor bounds are enforced. Paperclip rejects attempts to re-arm a monitor whose `timeoutAt` or `maxAttempts` is already exhausted. When a scheduled monitor reaches an exhausted bound at trigger time, Paperclip clears it and follows `recoveryPolicy`: `wake_owner` queues a bounded recovery wake for the assignee, `create_recovery_issue` opens visible issue-backed recovery work, and `escalate_to_board` records a board-visible escalation comment/activity.
+Monitor bounds are enforced, and they bound one monitor rather than the issue. `maxAttempts` counts the attempts of the monitor that is currently scheduled, so a `maxAttempts` below the attempts that monitor already spent is rejected. A monitor that has fired or been cleared has completed its lifecycle: the next `nextCheckAt` is a new monitor and starts at attempt zero. A `timeoutAt` that has already passed is rejected, because that monitor can never fire. Every rejection names the bound that stopped it, the attempts already spent, and the minimum `maxAttempts` that is accepted. When a scheduled monitor reaches an exhausted bound at trigger time, Paperclip clears it and follows `recoveryPolicy`: `wake_owner` queues a bounded recovery wake for the assignee, `create_recovery_issue` opens visible issue-backed recovery work, and `escalate_to_board` records a board-visible escalation comment/activity.
 
 Use `blocked` instead of a monitor when no Paperclip assignee owns a responsible polling path. In that case, name the external owner/action or create first-class recovery/blocker work.
 

server/src/__tests__/issue-execution-policy-routes.test.ts

@@ -708,6 +708,80 @@ describe("issue execution policy routes", () => {
     );
   });
 
+  it("schedules a fresh monitor on an issue whose previous monitor exhausted its attempts", async () => {
+    const issue = {
+      id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
+      companyId: "company-1",
+      status: "in_review",
+      assigneeAgentId: "33333333-3333-4333-8333-333333333333",
+      assigneeUserId: null,
+      createdByUserId: "local-board",
+      identifier: "PAP-1008",
+      title: "Monitor re-armed after exhaustion",
+      executionPolicy: null,
+      executionState: {
+        status: "idle",
+        currentStageId: null,
+        currentStageIndex: null,
+        currentStageType: null,
+        currentParticipant: null,
+        returnAssignee: null,
+        completedStageIds: [],
+        lastDecisionId: null,
+        lastDecisionOutcome: null,
+        monitor: {
+          status: "cleared",
+          nextCheckAt: null,
+          lastTriggeredAt: "2026-11-01T12:00:00.000Z",
+          attemptCount: 3,
+          maxAttempts: 3,
+          notes: null,
+          scheduledBy: "assignee",
+          clearedAt: "2026-11-01T12:00:00.000Z",
+          clearReason: "max_attempts_exhausted",
+        },
+      },
+      monitorAttemptCount: 3,
+      monitorNextCheckAt: null,
+      monitorLastTriggeredAt: new Date("2026-11-01T12:00:00.000Z"),
+      monitorNotes: null,
+      monitorScheduledBy: "assignee",
+    };
+    mockIssueService.getById.mockResolvedValue(issue);
+    mockIssueService.update.mockImplementation(async (_id: string, patch: Record<string, unknown>) => ({
+      ...issue,
+      ...patch,
+      updatedAt: new Date(),
+    }));
+
+    const res = await request(await createApp({
+      type: "agent",
+      agentId: "33333333-3333-4333-8333-333333333333",
+      companyId: "company-1",
+      runId: "55555555-5555-4555-8555-555555555555",
+    }))
+      .patch("/api/issues/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa")
+      .send({
+        executionPolicy: {
+          monitor: {
+            nextCheckAt: "2026-12-01T12:00:00.000Z",
+            scheduledBy: "assignee",
+            maxAttempts: 3,
+            notes: "Wait for the follow-up review.",
+          },
+        },
+      });
+
+    expect(res.status).toBe(200);
+    expect(mockIssueService.update).toHaveBeenCalledWith(
+      "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",
+      expect.objectContaining({
+        monitorNextCheckAt: new Date("2026-12-01T12:00:00.000Z"),
+        monitorAttemptCount: 0,
+      }),
+    );
+  });
+
   it("allows board-authored in_review repair updates without a review path", async () => {
     const issue = {
       id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa",

server/src/__tests__/issue-execution-policy.test.ts

@@ -1760,7 +1760,7 @@ describe("issue execution policy transitions", () => {
       ).toThrow("Monitor can only be scheduled");
     });
 
-    it("rejects explicitly re-arming a monitor after max attempts are exhausted", () => {
+    it("starts a new monitor at attempt zero once the previous monitor is spent", () => {
       const policy = normalizeIssueExecutionPolicy({
         stages: [],
         monitor: {
@@ -1770,6 +1770,102 @@ describe("issue execution policy transitions", () => {
         },
       })!;
 
+      const result = applyIssueExecutionPolicyTransition({
+        issue: {
+          status: "in_review",
+          assigneeAgentId: coderAgentId,
+          assigneeUserId: null,
+          executionPolicy: null,
+          executionState: null,
+          monitorAttemptCount: 1,
+          monitorNextCheckAt: null,
+          monitorLastTriggeredAt: new Date("2026-04-11T12:30:00.000Z"),
+          monitorNotes: null,
+          monitorScheduledBy: "assignee",
+        },
+        policy,
+        previousPolicy: null,
+        requestedAssigneePatch: {},
+        actor: { agentId: coderAgentId },
+        monitorExplicitlyUpdated: true,
+      });
+
+      expect(result.patch.monitorNextCheckAt).toEqual(new Date("2099-04-11T12:30:00.000Z"));
+      expect(result.patch.monitorAttemptCount).toBe(0);
+      expect(result.patch.executionState).toMatchObject({
+        monitor: {
+          status: "scheduled",
+          attemptCount: 0,
+          maxAttempts: 1,
+        },
+      });
+    });
+
+    it("still rejects a maxAttempts below the attempts the scheduled monitor already spent", () => {
+      const policy = normalizeIssueExecutionPolicy({
+        stages: [],
+        monitor: {
+          nextCheckAt: "2099-04-11T12:30:00.000Z",
+          maxAttempts: 2,
+          scheduledBy: "assignee",
+        },
+      })!;
+
+      expect(() =>
+        applyIssueExecutionPolicyTransition({
+          issue: {
+            status: "in_review",
+            assigneeAgentId: coderAgentId,
+            assigneeUserId: null,
+            executionPolicy: null,
+            executionState: {
+              status: "idle",
+              currentStageId: null,
+              currentStageIndex: null,
+              currentStageType: null,
+              currentParticipant: null,
+              returnAssignee: null,
+              completedStageIds: [],
+              lastDecisionId: null,
+              lastDecisionOutcome: null,
+              monitor: {
+                status: "scheduled",
+                nextCheckAt: "2098-04-11T12:30:00.000Z",
+                lastTriggeredAt: "2026-04-11T12:30:00.000Z",
+                attemptCount: 2,
+                notes: null,
+                scheduledBy: "assignee",
+                clearedAt: null,
+                clearReason: null,
+              },
+            },
+            monitorAttemptCount: 2,
+            monitorNextCheckAt: new Date("2098-04-11T12:30:00.000Z"),
+            monitorLastTriggeredAt: new Date("2026-04-11T12:30:00.000Z"),
+            monitorNotes: null,
+            monitorScheduledBy: "assignee",
+          },
+          policy,
+          previousPolicy: null,
+          requestedAssigneePatch: {},
+          actor: { agentId: coderAgentId },
+          monitorExplicitlyUpdated: true,
+        }),
+      ).toThrow(
+        "Monitor bounds are already exhausted: the scheduled monitor has already used 2 attempt(s) and maxAttempts is 2. Supply maxAttempts of at least 3.",
+      );
+    });
+
+    it("names the offending timeoutAt when the monitor deadline has already passed", () => {
+      const policy = normalizeIssueExecutionPolicy({
+        stages: [],
+        monitor: {
+          nextCheckAt: "2099-04-11T12:30:00.000Z",
+          timeoutAt: "2020-04-11T12:30:00.000Z",
+          scheduledBy: "assignee",
+        },
+      })!;
+
       expect(() =>
         applyIssueExecutionPolicyTransition({
           issue: {
@@ -1778,11 +1874,6 @@ describe("issue execution policy transitions", () => {
             assigneeUserId: null,
             executionPolicy: null,
             executionState: null,
-            monitorAttemptCount: 1,
-            monitorNextCheckAt: null,
-            monitorLastTriggeredAt: null,
-            monitorNotes: null,
-            monitorScheduledBy: "assignee",
           },
           policy,
           previousPolicy: null,
@@ -1790,7 +1881,7 @@ describe("issue execution policy transitions", () => {
           actor: { agentId: coderAgentId },
           monitorExplicitlyUpdated: true,
         }),
-      ).toThrow("Monitor bounds are already exhausted");
+      ).toThrow("timeoutAt 2020-04-11T12:30:00.000Z has already passed");
     });
   });
 });

server/src/services/issue-execution-policy.ts

@@ -210,6 +210,25 @@ function derivePersistedMonitorState(input: {
   return null;
 }
 
+/**
+ * Attempts carried into a newly scheduled monitor.
+ *
+ * `maxAttempts` bounds one monitor, not the issue. Only a monitor that is still
+ * `scheduled` carries its attempts forward, so lowering `maxAttempts` below the
+ * attempts the live monitor already spent is still rejected. Once a monitor has
+ * fired (`triggered`) or been cleared its lifecycle is over — the policy monitor
+ * is stripped on trigger — so the next `nextCheckAt` is a new monitor and starts
+ * at zero.
+ *
+ * Without the reset the count is a lifetime cap on the issue: every monitor the
+ * issue ever held is charged against whatever `maxAttempts` the next caller
+ * supplies, so one exhausted monitor permanently blocks every later one and
+ * strands `in_review` issues with no wake path at all.
+ */
+function carriedMonitorAttemptCount(previous: IssueExecutionMonitorState | null | undefined): number {
+  return previous?.status === "scheduled" ? previous.attemptCount ?? 0 : 0;
+}
+
 function buildScheduledMonitorState(
   previous: IssueExecutionMonitorState | null,
   monitor: IssueExecutionMonitorPolicy,
@@ -218,7 +237,7 @@ function buildScheduledMonitorState(
     status: "scheduled",
     nextCheckAt: monitor.nextCheckAt,
     lastTriggeredAt: previous?.lastTriggeredAt ?? null,
-    attemptCount: previous?.attemptCount ?? 0,
+    attemptCount: carriedMonitorAttemptCount(previous),
     notes: monitor.notes ?? null,
     scheduledBy: monitor.scheduledBy,
     ...monitorMetadataFromPolicy(monitor),
@@ -302,6 +321,36 @@ function exhaustedMonitorClearReason(input: {
   return null;
 }
 
+/**
+ * Name the bound that rejected the monitor, the count it was compared against,
+ * and the value that would be accepted. "Monitor bounds are already exhausted"
+ * on its own tells the caller neither which bound tripped nor how to satisfy it.
+ */
+function monitorBoundsExhaustedError(input: {
+  clearReason: IssueExecutionMonitorClearReason;
+  monitor: IssueExecutionMonitorPolicy;
+  attemptCount: number;
+}) {
+  const maxAttempts = input.monitor.maxAttempts ?? null;
+  const details: Record<string, unknown> = {
+    clearReason: input.clearReason,
+    attemptCount: input.attemptCount,
+    maxAttempts,
+    timeoutAt: input.monitor.timeoutAt ?? null,
+  };
+  if (input.clearReason === "timeout_exceeded") {
+    return unprocessable(
+      `${MONITOR_BOUNDS_EXHAUSTED_MESSAGE}: timeoutAt ${input.monitor.timeoutAt} has already passed. Supply a timeoutAt in the future, or omit it.`,
+      details,
+    );
+  }
+  const minimumMaxAttempts = input.attemptCount + 1;
+  return unprocessable(
+    `${MONITOR_BOUNDS_EXHAUSTED_MESSAGE}: the scheduled monitor has already used ${input.attemptCount} attempt(s) and maxAttempts is ${maxAttempts}. Supply maxAttempts of at least ${minimumMaxAttempts}.`,
+    { ...details, minimumMaxAttempts },
+  );
+}
+
 function nextAssigneeIds(input: {
   issue: IssueLike;
   requestedAssigneePatch: RequestedAssigneePatch;
@@ -1086,14 +1135,19 @@ function applyMonitorTransition(input: TransitionInput, stagePatch: Record<strin
         clearedAt: new Date(),
       });
     } else {
+      const carriedAttemptCount = carriedMonitorAttemptCount(currentMonitorState);
       const exhaustedReason = exhaustedMonitorClearReason({
         monitor: input.policy.monitor,
-        attemptCount: currentMonitorState?.attemptCount ?? 0,
+        attemptCount: carriedAttemptCount,
         now: new Date(),
       });
       if (exhaustedReason) {
         if (input.monitorExplicitlyUpdated) {
-          throw unprocessable(MONITOR_BOUNDS_EXHAUSTED_MESSAGE, { clearReason: exhaustedReason });
+          throw monitorBoundsExhaustedError({
+            clearReason: exhaustedReason,
+            monitor: input.policy.monitor,
+            attemptCount: carriedAttemptCount,
+          });
         }
         patch.executionPolicy = stripMonitorFromExecutionPolicy(input.policy);
         patch.monitorNextCheckAt = null;
@@ -1108,6 +1162,10 @@ function applyMonitorTransition(input: TransitionInput, stagePatch: Record<strin
         patch.monitorWakeRequestedAt = null;
         patch.monitorNotes = input.policy.monitor.notes ?? null;
         patch.monitorScheduledBy = input.policy.monitor.scheduledBy;
+        // The column is what the dispatcher and the liveness classifier read, so
+        // it has to be reset alongside the state or the new monitor inherits the
+        // previous monitor's attempts.
+        patch.monitorAttemptCount = carriedAttemptCount;
         targetMonitorState = buildScheduledMonitorState(currentMonitorState, input.policy.monitor);
       }
     }
@@ -1147,7 +1205,11 @@ export function buildInitialIssueMonitorFields(input: {
     now: new Date(),
   });
   if (exhaustedReason) {
-    throw unprocessable(MONITOR_BOUNDS_EXHAUSTED_MESSAGE, { clearReason: exhaustedReason });
+    throw monitorBoundsExhaustedError({
+      clearReason: exhaustedReason,
+      monitor: input.policy.monitor,
+      attemptCount: 0,
+    });
   }
 
   const monitorState = buildScheduledMonitorState(null, input.policy.monitor);

server/src/services/recovery/review-path-recovery.test.ts

@@ -4,6 +4,7 @@ import {
   buildIssueReviewPathLostIdempotencyKey,
   decideIssueReviewPathRecovery,
   isReviewPathRecoveryIdempotencyConflict,
+  reviewPathConsumedRefFromRun,
 } from "./review-path-recovery.js";
 
 const stalled = {
@@ -96,6 +97,28 @@ describe("review-path recovery", () => {
     expect(decision).toEqual({ kind: "skip", reason: "review issue still has a maintained path" });
   });
 
+  it("fingerprints monitor wakes per monitor, not per attempt count", () => {
+    const refFor = (nextCheckAt: string) => reviewPathConsumedRefFromRun({
+      runId: "run-1",
+      issueId: "issue-1",
+      contextSnapshot: {
+        wakeReason: "issue_monitor",
+        nextCheckAt,
+        monitorAttemptCount: 1,
+      },
+    });
+
+    // Two monitors, each on its first attempt: distinct fingerprints.
+    expect(refFor("2026-12-01T12:00:00.000Z")).not.toEqual(refFor("2026-12-08T12:00:00.000Z"));
+
+    // Contexts without a scheduled instant still fall back to the attempt count.
+    expect(reviewPathConsumedRefFromRun({
+      runId: "run-1",
+      issueId: "issue-1",
+      contextSnapshot: { wakeReason: "issue_monitor", monitorAttemptCount: 2 },
+    })).toBe("monitor:issue-1:cleared:2");
+  });
+
   it("recognizes wrapped atomic deduplication conflicts without swallowing unrelated uniqueness errors", () => {
     expect(isReviewPathRecoveryIdempotencyConflict({
       cause: {

server/src/services/recovery/review-path-recovery.ts

@@ -21,7 +21,13 @@ export function reviewPathConsumedRefFromRun(input: {
     ?? readNonEmptyString(context.interactionId)
     ?? readNonEmptyString(context.approvalId)
     ?? (readNonEmptyString(context.wakeReason)?.startsWith("issue_monitor")
-      ? `monitor:${input.issueId}:${readNonEmptyString(context.clearReason) ?? "cleared"}:${String(context.monitorAttemptCount ?? "unknown")}`
+      // Prefer the monitor's scheduled instant. The attempt count restarts at
+      // zero for each new monitor, so counting alone would let two monitors on
+      // the same issue share a fingerprint and silently suppress the second
+      // one's recovery wake.
+      ? `monitor:${input.issueId}:${readNonEmptyString(context.clearReason) ?? "cleared"}:${
+        readNonEmptyString(context.nextCheckAt) ?? String(context.monitorAttemptCount ?? "unknown")
+      }`
       : null)
     ?? input.runId;
 }