Address PR review feedback round 2 (#1683)

- monitor-store: add releaseIncidentFixTaskClaim (guarded UPDATE that only
  clears an in-flight sentinel, never a real attached task id) and make
  countRecentAutoFixTasks ignore sentinel placeholders, so a claim stranded by
  a failed createTask can't permanently absorb/suppress future regressions
- monitor-trait: release the claim if createTask throws after a successful
  claim, returning an error outcome instead of stranding the sentinel
- tests: release-vs-real-id, sentinel-excluded count, createTask-failure-then-reopen
- fix a type-unsound narrowing in the concurrency test (cast to the full union
  exposed the error variant's missing incidentId); use a discriminated guard
- add FNXC annotations on the new release/count paths and the concurrency harness

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

packages/dashboard/src/__tests__/monitor-store.test.ts

@@ -18,9 +18,13 @@ import {
   resolveIncident,
   getOpenIncidentByGroupingKey,
   attachFixTask,
+  claimIncidentForFixTask,
+  releaseIncidentFixTaskClaim,
+  getIncident,
   decideStormGuard,
   countRecentAutoFixTasks,
   DEFAULT_STORM_GUARD,
+  FIX_TASK_CLAIM_SENTINEL_PREFIX,
   type Incident,
 } from "../monitor-store.js";
 
@@ -168,5 +172,46 @@ describe("monitor-store (U13)", () => {
       attachFixTask(db, incident.incidentId, "FN-1");
       expect(countRecentAutoFixTasks(db)).toBe(1);
     });
+
+    // FNXC:Monitor 2026-06-16-15:40: the breaker count must ignore in-flight /
+    // stranded sentinel placeholders and only count real fix-task links.
+    it("ignores sentinel placeholders but counts real fix-task links", () => {
+      const { incident: a } = ingestIncidentSignal(db, { groupingKey: "ga", title: "a" });
+      const { incident: b } = ingestIncidentSignal(db, { groupingKey: "gb", title: "b" });
+      // a is only claimed (sentinel) → must NOT count.
+      expect(claimIncidentForFixTask(db, a.incidentId)).toBe(true);
+      expect(countRecentAutoFixTasks(db)).toBe(0);
+      // b gets a real fix task → counts.
+      attachFixTask(db, b.incidentId, "FN-2");
+      expect(countRecentAutoFixTasks(db)).toBe(1);
+    });
+  });
+
+  describe("releaseIncidentFixTaskClaim", () => {
+    // FNXC:Monitor 2026-06-16-15:40: a claim must be releasable back to NULL when
+    // task creation fails, but the release must never clobber a real attached id.
+    it("clears a sentinel claim back to NULL", () => {
+      const { incident } = ingestIncidentSignal(db, { groupingKey: "g-rel", title: "t" });
+      expect(claimIncidentForFixTask(db, incident.incidentId)).toBe(true);
+      const claimed = getIncident(db, incident.incidentId);
+      expect(claimed?.fixTaskId).toBe(`${FIX_TASK_CLAIM_SENTINEL_PREFIX}${incident.incidentId}`);
+
+      expect(releaseIncidentFixTaskClaim(db, incident.incidentId)).toBe(true);
+      const released = getIncident(db, incident.incidentId);
+      expect(released?.fixTaskId).toBeNull();
+
+      // Releasing again is a no-op (nothing to clear).
+      expect(releaseIncidentFixTaskClaim(db, incident.incidentId)).toBe(false);
+    });
+
+    it("does NOT clear a real attached fix task id", () => {
+      const { incident } = ingestIncidentSignal(db, { groupingKey: "g-real", title: "t" });
+      claimIncidentForFixTask(db, incident.incidentId);
+      attachFixTask(db, incident.incidentId, "FN-99");
+
+      // The release guard (fixTaskId = sentinel) must reject an attached row.
+      expect(releaseIncidentFixTaskClaim(db, incident.incidentId)).toBe(false);
+      expect(getIncident(db, incident.incidentId)?.fixTaskId).toBe("FN-99");
+    });
   });
 });

packages/dashboard/src/__tests__/monitor-trait.test.ts

@@ -13,6 +13,7 @@ import {
   claimIncidentForFixTask,
   ingestIncidentSignal,
   getIncident,
+  getOpenIncidentByGroupingKey,
 } from "../monitor-store.js";
 
 /**
@@ -147,6 +148,12 @@ describe("monitor-trait runMonitorOnRegression (U13)", () => {
     // that exact yield point so they overlap; only the claim-holder should win.
     const created: Task[] = [];
     let seq = 0;
+    // FNXC:Monitor 2026-06-16-15:40: the gate (a Promise both createTask calls
+    // await) holds both concurrent callers suspended at the createTask yield
+    // point so the claim race is reproduced deterministically rather than by
+    // chance scheduling. With both callers parked there, releaseGate() unblocks
+    // them together, proving the atomic claim lets exactly ONE fix task open
+    // (the loser absorbs on the lost claim, not on scheduling luck).
     let releaseGate: () => void = () => {};
     const gate = new Promise<void>((resolve) => {
       releaseGate = resolve;
@@ -188,11 +195,57 @@ describe("monitor-trait runMonitorOnRegression (U13)", () => {
     expect(kinds).toEqual(["absorbed", "fix-task-opened"]);
 
     // The incident is linked to the single real task, not a sentinel.
-    const incidentId = (ra.kind === "fix-task-opened" ? ra : (rb as typeof ra)).incidentId;
-    const incident = getIncident(db, incidentId);
+    const openedOutcome = ra.kind === "fix-task-opened" ? ra : rb;
+    if (openedOutcome.kind !== "fix-task-opened") {
+      throw new Error(`expected exactly one fix-task-opened outcome, got ${ra.kind} + ${rb.kind}`);
+    }
+    const incident = getIncident(db, openedOutcome.incidentId);
     expect(incident?.fixTaskId).toBe(created[0].id);
   });
 
+  // FNXC:Monitor 2026-06-16-15:40: if createTask throws AFTER the claim, the
+  // claim must be released so the sentinel can't permanently absorb/suppress
+  // future regressions for the same incident.
+  it("a createTask failure after claim releases the claim so a later regression can open a fix task", async () => {
+    let failNext = true;
+    const created: Task[] = [];
+    let seq = 0;
+    const store = {
+      getDatabase: () => db,
+      async createTask(input: TaskCreateInput): Promise<Task> {
+        if (failNext) {
+          failNext = false;
+          throw new Error("task store unavailable");
+        }
+        const task = {
+          id: `FN-${++seq}`,
+          title: input.title,
+          column: input.column,
+          source: input.source,
+        } as unknown as Task;
+        created.push(task);
+        return task;
+      },
+    } as unknown as TaskStore;
+
+    // Prime an open incident past the gate so the guard decides open-fix-task.
+    for (let i = 0; i < DEFAULT_STORM_GUARD.threshold; i += 1) {
+      ingestIncidentSignal(db, { groupingKey: "g-fail", title: "Boom" });
+    }
+
+    // First open-fix-task attempt: createTask throws → claim released, error out.
+    const failed = await runMonitorOnRegression({ groupingKey: "g-fail", title: "Boom" }, { store });
+    expect(failed.kind).toBe("error");
+    expect(created).toHaveLength(0);
+    const incident = getOpenIncidentByGroupingKey(db, "g-fail");
+    expect(incident?.fixTaskId).toBeNull(); // claim released, not stranded
+
+    // A later regression can now open a fix task again (not absorbed by a sentinel).
+    const reopened = await runMonitorOnRegression({ groupingKey: "g-fail", title: "Boom" }, { store });
+    expect(reopened.kind).toBe("fix-task-opened");
+    expect(created).toHaveLength(1);
+  });
+
   it("the atomic claim step prevents a second create once an incident is claimed/linked", () => {
     const { incident } = ingestIncidentSignal(db, { groupingKey: "g-claim", title: "Claim me" });
     // First claim wins.

packages/dashboard/src/monitor-store.ts

@@ -341,6 +341,33 @@ export function attachFixTask(db: Database, incidentId: string, fixTaskId: strin
   db.bumpLastModified();
 }
 
+/**
+ * FNXC:Monitor 2026-06-16-15:40: a fix-task claim must be released if task
+ * creation fails so a stranded sentinel can't permanently absorb/suppress
+ * future regressions. {@link claimIncidentForFixTask} writes a non-null sentinel
+ * to `fixTaskId`; if {@link attachFixTask} never runs (createTask threw after the
+ * claim), the incident would stay pseudo-linked forever — every later regression
+ * would absorb against the sentinel and the circuit-breaker count would include
+ * it. This releases the claim back to NULL, but ONLY when the value is STILL the
+ * exact sentinel, so it can never clobber a real attached task id (the
+ * `WHERE fixTaskId = <sentinel>` guard rejects any already-attached row).
+ *
+ * Returns true if a sentinel was actually cleared.
+ */
+export function releaseIncidentFixTaskClaim(db: Database, incidentId: string): boolean {
+  const now = new Date().toISOString();
+  const sentinel = `${FIX_TASK_CLAIM_SENTINEL_PREFIX}${incidentId}`;
+  const result = db
+    .prepare(
+      `UPDATE incidents SET fixTaskId = NULL, updatedAt = ?
+       WHERE incidentId = ? AND fixTaskId = ?`,
+    )
+    .run(now, incidentId, sentinel) as { changes?: number | bigint };
+  const released = Number(result.changes ?? 0) > 0;
+  if (released) db.bumpLastModified();
+  return released;
+}
+
 // ── Storm guard ───────────────────────────────────────────────────────────────
 
 export interface StormGuardConfig {
@@ -421,6 +448,16 @@ export function decideStormGuard(
  * task is one linked to an incident (fixTaskId set) whose incident updatedAt is
  * within the window. This is a deliberately coarse proxy that does not require a
  * separate audit table.
+ *
+ * FNXC:Monitor 2026-06-16-15:40: the circuit-breaker count must ignore in-flight
+ * and stranded sentinel placeholders. {@link claimIncidentForFixTask} writes a
+ * `${FIX_TASK_CLAIM_SENTINEL_PREFIX}…` sentinel into `fixTaskId` BEFORE the real
+ * task exists; the real id overwrites it synchronously right after createTask, so
+ * excluding sentinels here only discounts the brief in-flight window and the
+ * stranded-claim case (creation failed) — exactly the rows that should not count
+ * against the breaker. Loser-absorption is unaffected: a loser absorbs because
+ * {@link decideStormGuard} sees the SPECIFIC incident's non-null `fixTaskId`, or
+ * because its claim attempt lost — never because of this window count.
  */
 export function countRecentAutoFixTasks(
   db: Database,
@@ -431,8 +468,8 @@ export function countRecentAutoFixTasks(
   const row = db
     .prepare(
       `SELECT COUNT(*) AS count FROM incidents
-       WHERE fixTaskId IS NOT NULL AND updatedAt >= ?`,
+       WHERE fixTaskId IS NOT NULL AND fixTaskId NOT LIKE ? AND updatedAt >= ?`,
     )
-    .get(cutoff) as { count: number };
+    .get(`${FIX_TASK_CLAIM_SENTINEL_PREFIX}%`, cutoff) as { count: number };
   return row.count;
 }

packages/dashboard/src/monitor-trait.ts

@@ -12,6 +12,7 @@ import {
   countRecentAutoFixTasks,
   decideStormGuard,
   ingestIncidentSignal,
+  releaseIncidentFixTaskClaim,
   type IncidentSignalInput,
   type StormGuardConfig,
 } from "./monitor-store.js";
@@ -173,7 +174,27 @@ export async function runMonitorOnRegression(
         reason: "fix-task-claimed-concurrently",
       };
     }
-    const task = await store.createTask(buildFixTaskInput(signal, incidentId));
+    // FNXC:Monitor 2026-06-16-15:40: a fix-task claim must be released if task
+    // creation fails so a stranded sentinel can't permanently absorb/suppress
+    // future regressions. The claim wrote a non-null sentinel to fixTaskId; if
+    // createTask throws here, attachFixTask never overwrites it, leaving the
+    // incident pseudo-linked forever. Release the claim (back to NULL, only when
+    // still the sentinel) before surfacing an error outcome so a later regression
+    // can open a fix task again.
+    let task: Task;
+    try {
+      task = await store.createTask(buildFixTaskInput(signal, incidentId));
+    } catch (createErr) {
+      releaseIncidentFixTaskClaim(db, incidentId);
+      diagnostics.errorFromException("Monitor fix-task creation failed; released claim", createErr, {
+        groupingKey: signal.groupingKey,
+        incidentId,
+      });
+      return {
+        kind: "error",
+        reason: createErr instanceof Error ? createErr.message : String(createErr),
+      };
+    }
     attachFixTask(db, incidentId, task.id);
     return { kind: "fix-task-opened", taskId: task.id, incidentId };
   } catch (err) {