fix(worker): close post-merge reliability gaps (#353)

* fix(worker): close post-merge reliability gaps

* test(worker): stabilize sandbox and health fixtures

Author: outof-place

apps/worker/src/lib/reconcile.test.ts

@@ -399,6 +399,56 @@ describe("reconcileRuns owner-CAS recovery", () => {
     },
   );
 
+  it("evicts a terminal manual ticket when the capped AI snapshot omits it but Jira still reports AI", async () => {
+    const manual = entry({ kind: "manual_ticket" });
+    const runRegistry = registry([manual]);
+    const tracker = issueTracker("AI");
+    mockGetRun.mockReturnValue({ status: Promise.resolve("completed") });
+    const onReleased = vi.fn();
+    const { reconcileRuns } = await import("./reconcile.js");
+
+    await expect(
+      reconcileRuns(
+        new Set(),
+        runRegistry,
+        tracker,
+        undefined,
+        onReleased,
+        undefined,
+        mockDb,
+      ),
+    ).resolves.toEqual({ cancelled: 0, cleaned: 1 });
+    expect(mockAssertActiveRunOwnerState).toHaveBeenCalledWith(
+      mockDb,
+      manual,
+      "bound",
+    );
+    expect(tracker.moveTicket).toHaveBeenCalledTimes(1);
+    expect(tracker.moveTicket).toHaveBeenCalledWith("PROJ-1", "Backlog");
+    expect(runRegistry.release).toHaveBeenCalledTimes(1);
+    expect(runRegistry.release).toHaveBeenCalledWith(
+      manual.subjectKey,
+      manual.ownerToken,
+      manual.runId,
+    );
+    expect(onReleased).toHaveBeenCalledTimes(1);
+    expect(onReleased).toHaveBeenCalledWith(manual.subjectKey);
+  });
+
+  it("retains a manual claim omitted from the snapshot when Jira's live read is uncertain", async () => {
+    const manual = entry({ kind: "manual_ticket" });
+    const runRegistry = registry([manual]);
+    const tracker = issueTracker();
+    vi.mocked(tracker.fetchTicket).mockRejectedValue(new Error("Jira unavailable"));
+    const { reconcileRuns } = await import("./reconcile.js");
+
+    await expect(
+      reconcileRuns(new Set(), runRegistry, tracker, undefined, undefined, undefined, mockDb),
+    ).resolves.toEqual({ cancelled: 0, cleaned: 0 });
+    expect(tracker.moveTicket).not.toHaveBeenCalled();
+    expect(runRegistry.release).not.toHaveBeenCalled();
+  });
+
   it("retains a manual claim when stale-snapshot withdrawal cannot be confirmed", async () => {
     const manual = entry({ kind: "manual_ticket" });
     const runRegistry = registry([manual]);

apps/worker/src/lib/reconcile.ts

@@ -266,7 +266,21 @@ export async function reconcileRuns(
       continue;
     }
     const departure = await verifyTicketLeftAiColumn(ticketKey, issueTracker);
-    if (!departure.left) continue;
+    if (!departure.left) {
+      // The Jira poll is capped, so a manual claim can be absent from its
+      // snapshot even though the authoritative read still finds AI. Reuse the
+      // owner-fenced terminal cleanup; an uncertain read retains the claim.
+      if (entry.kind === "manual_ticket" && departure.trackerStatus !== null) {
+        cleaned += await cleanFinishedManualTicket(
+          boundEntry,
+          runRegistry,
+          issueTracker,
+          onSubjectReleased,
+          db,
+        );
+      }
+      continue;
+    }
     const reviewDestination =
       departure.trackerStatus !== null &&
       (await isAiReviewDestination({

apps/worker/src/mcp/run-diagnosis.test.ts

@@ -4,6 +4,21 @@ import { diagnoseRun } from "./run-diagnosis.js";
 import { WORKSPACE_GATE_NOT_RECORDED_MESSAGE } from "../workflow-definition/interpreter.js";
 
 describe("diagnoseRun", () => {
+  it("classifies the stable watchdog stalled-engine reason instead of unknown", () => {
+    const result = diagnoseRun({
+      status: "failed",
+      error: {
+        code: "AIW-DIAG-wrun_1-watchdog",
+        message: 'Run engine stalled: step "collectPhase" has been running for 32 minutes',
+      },
+      steps: [],
+    });
+    expect(result.category).toBe("engine_stalled");
+    expect(result.confidence).toBe("low");
+    expect(result.evidenceRefs).toEqual(["AIW-DIAG-wrun_1-watchdog"]);
+    expect(result.nextActions.join(" ")).toMatch(/watchdog|step|worker/i);
+  });
+
   it("hands over the evidence it has even when no rule matched", () => {
     // An unmatched run used to return evidenceRefs: [] unconditionally, so the
     // diagnosis was strictly worse than its neighbours: runs.result showed a
@@ -392,6 +407,31 @@ describe("diagnoseRun", () => {
     });
   });
 
+  // Real production shape from wrun_01M13WTS2KV1ZX7ZKAFAHM5F7J: the provider
+  // failure was already reduced to this stable, client-safe sentence, but the
+  // diagnosis table had no matching rule and returned unknown. A provider
+  // project spend limit is a dependency failure, not the workflow's own
+  // budget_exhausted category, and needs billing guidance rather than a blind
+  // retry/status-page suggestion.
+  it("classifies the curated provider spend-limit message with billing remediation and preserves its diagnostic evidence", () => {
+    const diagnosticId =
+      "AIW-DIAG-wrun_01M13WTS2KV1ZX7ZKAFAHM5F7J-call_llm-1";
+    const result = diagnoseRun({
+      status: "failed",
+      error: {
+        code: diagnosticId,
+        message:
+          "The AI provider rejected the request: the account has reached its configured spend limit. Raise or remove the spend limit in the provider's billing settings, then rerun.",
+      },
+      steps: [],
+    });
+    expect(result.category).toBe("dependency_unavailable");
+    expect(result.confidence).toBe("low");
+    expect(result.evidenceRefs).toEqual([diagnosticId]);
+    expect(result.nextActions.join(" ")).toMatch(/spend limit|billing/i);
+    expect(result.nextActions.join(" ")).not.toMatch(/status page/i);
+  });
+
   // Real shape: PROVIDER_CAUSES rate-limit entry (workflow-definition/
   // failure-message.ts:96-99), reached the same way as the auth case above.
   it("classifies the curated AI-provider rate-limit message as dependency_unavailable, with low confidence", () => {

apps/worker/src/mcp/run-diagnosis.ts

@@ -48,6 +48,7 @@ export type RunDiagnosisCategory =
   | "source_pull_request_moved"
   | "validation_failed"
   | "budget_exhausted"
+  | "engine_stalled"
   | "engine_error"
   | "step_failed"
   | "unknown";
@@ -142,6 +143,10 @@ const NEXT_ACTIONS: Record<RunDiagnosisCategory, string[]> = {
     "The run stopped after exhausting its configured budget, not from a failure.",
     "Raise the workflow's budget limit or narrow the ticket's scope before retrying.",
   ],
+  engine_stalled: [
+    "The watchdog marked this run failed after a workflow step stopped making progress.",
+    "Inspect the named step and worker or sandbox health before retrying the run.",
+  ],
   engine_error: [
     "Check the workflow definition graph for an unresolvable trigger, node, or edge.",
   ],
@@ -237,6 +242,10 @@ const SOURCE_PULL_REQUEST_MOVED_KEYWORDS = [
 // stopped by a budget check (workflows/agent.ts:2537-2543).
 const BUDGET_EXHAUSTED_PREFIX = "Run stopped on budget:";
 
+// WATCHDOG_FAILURE_REASON_PREFIX (lib/telemetry/run-telemetry.ts:116), written
+// only by the engine-stall watchdog as a durable failed-run reason.
+const ENGINE_STALLED_PREFIX = "Run engine stalled:";
+
 // fallbackTerminalError's "blocked" lead (lib/overview/sanitize-run-detail.ts:
 // 104-113): the observed face of three silent stop paths that record no
 // statusReason: markRunBlockedOnCancel and sweepOrphanedAwaitingRuns
@@ -265,6 +274,19 @@ const VALIDATION_FAILED_PREFIXES = [
 const DEPENDENCY_AUTH_PREFIX =
   "The AI provider rejected the credentials (authentication failed).";
 
+// Curated provider sentence for an account/project spend-limit rejection
+// (workflow-definition/failure-message.ts:148-151). This is deliberately a
+// whole trusted lead rather than a raw `spend limit` search: runs.diagnose
+// receives the already-sanitized run reason, and only this code-owned sentence
+// is safe to route to billing remediation. It stays distinct from
+// `budget_exhausted`, which describes the workflow's own configured budget.
+const PROVIDER_SPEND_LIMIT_PREFIX =
+  "The AI provider rejected the request: the account has reached its configured spend limit.";
+const PROVIDER_SPEND_LIMIT_ACTIONS = [
+  "Raise or remove the provider project's configured spend limit in billing settings before retrying.",
+  "Confirm the intended provider project/account is selected and that the new limit has propagated before rerunning.",
+] as const;
+
 // The other PROVIDER_CAUSES sentences (workflow-definition/failure-message.ts:
 // 90-113): billing/credit, rate limit, model unavailable, and overloaded. Plus
 // SAFE_EXECUTION_ERROR_MESSAGES.provider (interpreter.ts:89), the uncurated
@@ -390,6 +412,15 @@ const RULES: readonly Rule[] = [
       return { confidence: "low", evidenceRefs: evidenceFrom(input) };
     },
   },
+  {
+    category: "engine_stalled",
+    match: (input) => {
+      if (input.status !== "failed") return null;
+      const message = input.error?.message;
+      if (!message || !message.startsWith(ENGINE_STALLED_PREFIX)) return null;
+      return { confidence: "low", evidenceRefs: evidenceFrom(input) };
+    },
+  },
   {
     category: "cancelled",
     match: (input) => {
@@ -476,6 +507,22 @@ const RULES: readonly Rule[] = [
       return { confidence: "low", evidenceRefs: evidenceFrom(input) };
     },
   },
+  {
+    // This is a provider dependency failure, but the operator action is
+    // billing remediation rather than a blind retry or a status-page check.
+    // Keep it ahead of the generic dependency_unavailable rule below so the
+    // stable curated spend-limit sentence gets its specific guidance.
+    category: "dependency_unavailable",
+    match: (input) => {
+      const message = input.error?.message;
+      if (!message || !message.startsWith(PROVIDER_SPEND_LIMIT_PREFIX)) return null;
+      return {
+        confidence: "low",
+        evidenceRefs: evidenceFrom(input),
+        nextActions: PROVIDER_SPEND_LIMIT_ACTIONS,
+      };
+    },
+  },
   {
     category: "dependency_unavailable",
     match: (input) => {