fix(workflows): stop masking blocked returned statuses as completed

packages/coding-agent/docs/workflows.md

@@ -246,6 +246,8 @@ Goal worker/reviewer prompts treat the objective and acceptance criteria as the
 
 The worker may claim readiness, but it cannot finalize completion. Workers and reviewers are prompted to verify user-visible behavior end-to-end when practical, using `playwright-cli`-skilled subagents for web/frontend flows that may depend on backend/API behavior and tmux-skilled subagents for TUI or terminal-app scenarios. They must assume credentials/auth/environment access exists until concrete checks plus an actual app/flow launch attempt prove otherwise; skipped E2E is valid only when exact attempted commands and observed failure output are recorded. Goal reviewers also look for any QA E2E video referenced by the ledger or receipt and must inspect the actual video before treating it as proof. Three reviewers independently inspect the ledger, worker receipt, repository state, and diff against `base_branch`; each returns structured JSON with findings, evidence, verification still remaining, and an optional blocker. A TypeScript reducer marks the goal complete only when reviewer quorum approves, marks blocked only when the same dependency/tool blocker repeats for the blocker threshold, continues when evidence is missing, and returns `needs_human` when `max_turns` is exhausted or worker execution fails.
 
+When Goal's reducer returns `needs_human`, `blocked`, or another incomplete status, the top-level workflow run is not reported as a successful completion. `/workflow status` and lifecycle notices surface it as blocked/failed according to the run's terminal condition. Atomic also preserves structured recoverable failure metadata from the run's blocking stage (`failedStageId`) or run-level failure metadata, so auth, rate-limit, and provider fallback exhaustion remains blocked/resumable even if the workflow later returns ordinary outputs instead of a reserved `status` value. Tolerated branch failures from non-fail-fast parallel work do not reclassify an otherwise completed run.
+
 Every Goal review round also persists an explicit convergence summary. Each reviewer record and review artifact distinguishes schema-parse status from the review verdict with `parsed`, `approved`, `stopReviewLoop`, `nextAction`, `finalActionRemaining`, and `diagnostics` fields; malformed or missing structured reviewer output is reported as a parse failure rather than as an ordinary finding/rejection. When `create_pr=true`, reviewers are told that PR/MR/review creation is a post-approval final action: if implementation and validation requirements are proven and only PR creation remains, the implementation can approve with `finalActionRemaining: true` and `nextAction: "pull-request"` instead of consuming another worker turn. The ledger's reducer decision repeats the same concise fields for the controller outcome, so a successful quorum records `approved: true`, `stopReviewLoop: true`, and `nextAction: "pull-request"` when `create_pr=true` (otherwise `"finish"`) before any final handoff runs.
 
 Result fields:
@@ -1324,7 +1326,7 @@ Workflow outputs are runtime contracts for completed workflow runs and for paren
 
 **Return convention:** outputs are return-object keys. Atomic never infers child workflow outputs from stage names, stage order, or the final assistant message. If a parent should read `child.outputs.foo`, the child workflow's `run` must both declare `outputs: { foo: schema }` and return `{ foo: value }`. `result` is not special and is never added for you: to expose `result`, declare it in `outputs` and return `{ result }` exactly like any other output. Returning a key that is not declared in `outputs` fails the run with `atomic-workflows: workflow "<name>" returned undeclared output "<key>"; declare it in outputs or remove it from the run return`.
 
-**Reserved `status` output convention:** if a workflow declares and returns a top-level `status` output with the string value `"failed"` or `"blocked"`, Atomic treats that as the workflow's terminal run status instead of recording a successful completion. When present, a non-empty top-level `summary` string becomes the run error/reason shown in lifecycle notices and status surfaces. Use this convention only when the workflow is intentionally reporting its own terminal state (for example, a deterministic release gate that returns `{ status: "blocked", summary: "required checks are pending" }`). Do not use a top-level `status` field for unrelated external state such as a deployment/check you merely inspected; choose a domain-specific name like `deployment_status` or `gate_status` instead.
+**Reserved `status` output convention and structured failures:** if a workflow declares and returns a top-level `status` output with the string value `"failed"`, Atomic treats the run as failed instead of recording a successful completion. Returned `"blocked"`, `"needs_human"`, `"incomplete"`, `"active"`, and `"auth_blocked"` statuses are treated as blocked/incomplete terminal states rather than successful completions. Independently of that convention, Atomic uses structured failure metadata captured from the run's blocking stage (`failedStageId`) or run-level failure metadata to keep recoverable auth, rate-limit, and provider fallback exhaustion blocked/resumable even when the workflow did not declare a `status` output. Atomic does not infer failure state by scanning arbitrary output text or by scanning every failed stage in an otherwise completed non-fail-fast branch. When a reserved status is returned, a non-empty top-level `summary` string becomes the run reason shown in lifecycle notices and status surfaces; if it is absent, Atomic falls back to non-empty top-level `remaining_work` and then `result` text. Use the reserved `status` convention only when the workflow is intentionally reporting its own terminal state (for example, a deterministic release gate that returns `{ status: "blocked", summary: "required checks are pending" }`, or a reviewer-gated workflow that returns `{ status: "needs_human", remaining_work: "provider credentials are missing" }`). Do not use a top-level `status` field for unrelated external state such as a deployment/check you merely inspected; choose a domain-specific name like `deployment_status` or `gate_status` instead.
 
 The `outputs` object is a schema contract, not an automatic stage selector. To expose values from any stage, capture the stage/task/child result in normal TypeScript and return it from `run` under the desired key:
 

packages/workflows/CHANGELOG.md

@@ -8,6 +8,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 
 ### Fixed
 
+- Fixed workflow failure finalization so structured recoverable auth, rate-limit, and provider-exhaustion metadata captured on the run or its blocking stage (`failedStageId`) no longer allows the top-level run, `/workflow status`, durable status, or lifecycle notice to appear as a successful `completed` state. Goal-like `needs_human`, incomplete, or auth-blocked reducer outputs are still treated as blocked, structured provider/auth fallback exhaustion now preserves an actionable blocked/failure message and resumable metadata, legacy completed snapshots with incomplete returned statuses or structured blocking-stage failures render as blocked, tolerated non-fail-fast branch failures no longer reclassify completed runs, and Goal reviewer-batch fallback exhaustion stops promptly as `needs_human` instead of launching another worker turn.
 - Fixed the fullscreen workflow graph statusline to mirror non-workflow extension statuses, so async/background subagent progress and completion remain visible while the graph overlay is active.
 - Fixed `/workflow resume` for workflows that use reusable `gitWorktreeDir`/`git_worktree_dir` worktrees by persisting the original invocation cwd and resolved reusable-worktree metadata, replaying durable resumes from that original repository context instead of the resumed interactive session cwd, and hardening Git subprocess timeouts so slow filesystem timeouts are reported as Git timeouts rather than “not inside a Git repository”.
 

packages/workflows/builtin/goal-runner.ts

@@ -8,6 +8,7 @@ import {
   type GoalWorkflowInputs,
   type GoalWorkflowOutputs,
   type ReviewRecord,
+  type ReducerDecision,
 } from "./goal-types.js";
 import { artifactSafeName, writeReviewArtifact, writeReviewRoundArtifact } from "./goal-artifacts.js";
 import { appendLifecycleEvent, createGoalLedger, writeGoalLedger } from "./goal-ledger.js";
@@ -75,6 +76,27 @@ type GoalWorkflowOptions = {
   readonly workflowStartCwd: string;
 };
 
+function reviewerExecutionFailedDecision(input: {
+  readonly turn: number;
+  readonly reviewQuorum: number;
+  readonly reviews: readonly ReviewRecord[];
+  readonly reason: string;
+}): ReducerDecision {
+  return {
+    turn: input.turn,
+    decision: "needs_human",
+    reason: input.reason,
+    complete_votes: input.reviews.filter((review) => review.decision === "complete").length,
+    review_quorum: input.reviewQuorum,
+    parsed: input.reviews.every((review) => review.parsed),
+    approved: false,
+    stopReviewLoop: false,
+    nextAction: "needs_human",
+    finalActionRemaining: false,
+    diagnostics: input.reviews.flatMap((review) => review.parse_diagnostics),
+  };
+}
+
 export async function runGoalWorkflow(ctx: GoalRunnerContext, options: GoalWorkflowOptions): Promise<GoalWorkflowOutputs> {
     const inputs = ctx.inputs;
     const createPr = options.createPr;
@@ -255,12 +277,14 @@ export async function runGoalWorkflow(ctx: GoalRunnerContext, options: GoalWorkf
       ];
 
       let reviewResults: WorkflowTaskResult[];
+      let reviewerBatchFailed = false;
       try {
         reviewResults = await ctx.parallel(reviewerSteps, {
           task: objective,
-          failFast: false,
+          failFast: true,
         });
       } catch (err) {
+        reviewerBatchFailed = true;
         reviewResults = [
           {
             name: "reviewer-error",
@@ -308,6 +332,20 @@ export async function runGoalWorkflow(ctx: GoalRunnerContext, options: GoalWorkf
         `Recorded ${latestReviews.length} reviewer decisions.`,
         turn,
       );
+      if (reviewerBatchFailed) {
+        terminalRemainingWork = collectRemainingWork(latestReviews);
+        const reason = `Reviewer execution failed before quorum could be established. Remaining work: ${terminalRemainingWork}`;
+        ledger.decisions.push(reviewerExecutionFailedDecision({
+          turn,
+          reviewQuorum,
+          reviews: latestReviews,
+          reason,
+        }));
+        ledger.status = "needs_human";
+        appendLifecycleEvent(ledger, "status_decided", reason, turn);
+        await writeGoalLedger(ledgerPath, ledger);
+        break;
+      }
 
       const reducerOutcome = reduceGoalDecision(ledger, latestReviews, {
         turn,

packages/workflows/src/engine/run-durable-finalize.ts

@@ -38,8 +38,7 @@ export async function finalizeDurableTerminalStatus(input: DurableTerminalFinali
 
   const durableStatus = toDurableStatus(status);
   if (durableStatus !== undefined) {
-    const resumable = status === "blocked" ? false : input.runSnapshot.resumable;
-    input.durableBackend.setWorkflowStatus(input.runId, durableStatus, undefined, resumable);
+    input.durableBackend.setWorkflowStatus(input.runId, durableStatus, undefined, input.runSnapshot.resumable);
   }
   try {
     await input.durableBackend.flush?.();

packages/workflows/src/engine/run-returned-status.ts

@@ -1,29 +1,58 @@
 import type { RunEndMetadata } from "../shared/store-public-types.js";
 import type { WorkflowOutputValues } from "../shared/types.js";
+import type { RunSnapshot } from "../shared/store-types.js";
+import {
+  actionableReturnedStatusText,
+  isReturnedBlockedWorkflowStatus,
+  isReturnedResumableBlockedWorkflowStatus,
+  normalizeReturnedWorkflowStatus,
+  structuredRecoverableWorkflowFailure,
+} from "../shared/returned-run-status.js";
 
 export interface ReturnedRunStatus {
   readonly status: "completed" | "failed" | "blocked";
   readonly error?: string;
   readonly metadata?: RunEndMetadata;
 }
 
-export function classifyReturnedRunStatus(result: WorkflowOutputValues | undefined): ReturnedRunStatus {
-  const returnedStatus = result?.["status"];
-  if (returnedStatus !== "failed" && returnedStatus !== "blocked") {
+export function classifyReturnedRunStatus(result: WorkflowOutputValues | undefined, runSnapshot?: RunSnapshot): ReturnedRunStatus {
+  const structuredFailure = runSnapshot !== undefined ? structuredRecoverableWorkflowFailure(runSnapshot) : undefined;
+  if (structuredFailure !== undefined) {
+    return {
+      status: "blocked",
+      error: structuredFailure.error,
+      metadata: structuredFailure.metadata,
+    };
+  }
+  const returnedStatus = normalizeReturnedWorkflowStatus(result?.["status"]);
+  if (returnedStatus === undefined || returnedStatus === "completed" || returnedStatus === "complete") {
     return { status: "completed" };
   }
 
-  const summary = result?.["summary"];
-  const error = typeof summary === "string" && summary.trim().length > 0
-    ? summary.trim()
-    : `Workflow returned status ${JSON.stringify(returnedStatus)}.`;
-  return {
-    status: returnedStatus,
-    error,
-    metadata: returnedStatus === "failed"
-      ? returnedFailureMetadata(error)
-      : returnedBlockedMetadata(),
-  };
+  const error = returnedStatusError(result, returnedStatus);
+  if (returnedStatus === "failed") {
+    return {
+      status: "failed",
+      error,
+      metadata: returnedFailureMetadata(error),
+    };
+  }
+  if (isReturnedBlockedWorkflowStatus(returnedStatus)) {
+    const metadata = isReturnedResumableBlockedWorkflowStatus(returnedStatus)
+      ? returnedRecoverableBlockedMetadata(error)
+      : { resumable: false };
+    return {
+      status: "blocked",
+      error,
+      metadata,
+    };
+  }
+
+  return { status: "completed" };
+}
+
+function returnedStatusError(result: WorkflowOutputValues | undefined, returnedStatus: string): string {
+  return actionableReturnedStatusText(result) ?? `Workflow returned status ${JSON.stringify(returnedStatus)}.`;
 }
 
 function returnedFailureMetadata(error: string): RunEndMetadata {
@@ -36,6 +65,11 @@ function returnedFailureMetadata(error: string): RunEndMetadata {
   };
 }
 
-function returnedBlockedMetadata(): RunEndMetadata {
-  return { resumable: false };
+function returnedRecoverableBlockedMetadata(error: string): RunEndMetadata {
+  return {
+    failureRecoverability: "recoverable",
+    failureDisposition: "active_blocked",
+    failureMessage: error,
+    resumable: true,
+  };
 }

packages/workflows/src/engine/run.ts

@@ -422,7 +422,7 @@ export async function run<
     assertWorkflowRunOutputs(def.name, result, def.outputs);
     assertWorkflowCreatedStage(runSnapshot);
     await durableBackend.flush?.();
-    const returned = classifyReturnedRunStatus(result);
+    const returned = classifyReturnedRunStatus(result, runSnapshot);
     const recorded = activeStore.recordRunEnd(runId, returned.status, result, returned.error, returned.metadata);
     appendRunEndWhenRecorded(opts.persistence, recorded, { runId, status: returned.status, result, ...(returned.error !== undefined ? { error: returned.error } : {}), ...(returned.metadata ?? {}), ...(runSnapshot.endedAt !== undefined ? { endedAt: runSnapshot.endedAt } : {}), ...(runSnapshot.durationMs !== undefined ? { durationMs: runSnapshot.durationMs } : {}), ts: Date.now() });
     durableBackend.setWorkflowStatus(runId, returned.status, undefined, returned.metadata?.resumable);

packages/workflows/src/extension/lifecycle-notifications.ts

@@ -15,6 +15,13 @@ import type {
   StoreSnapshot,
 } from "../shared/store-types.js";
 import { isTopLevelWorkflowRun } from "../shared/run-visibility.js";
+import {
+  actionableReturnedStatusText,
+  effectiveRunStatus,
+  isReturnedBlockedWorkflowStatus,
+  normalizeReturnedWorkflowStatus,
+  structuredRecoverableWorkflowFailureText,
+} from "../shared/returned-run-status.js";
 import { deriveGraphThemeFromPiTheme, type GraphTheme } from "../tui/graph-theme.js";
 import { renderWorkflowNoticeCard, type WorkflowNoticeTone } from "../tui/workflow-notice-card.js";
 
@@ -299,13 +306,13 @@ function makeTerminalNotice(
   const failedStage = run.failedStageId
     ? run.stages.find((stage) => stage.id === run.failedStageId)
     : undefined;
-  const error = run.error ?? (kind === "blocked" ? run.exitReason : undefined);
+  const error = run.error ?? returnedNoticeError(run, kind) ?? (kind === "blocked" ? run.exitReason : undefined);
   return {
     kind,
     scope: "run",
     runId: run.id,
     workflowName: run.name,
-    status: run.status,
+    status: effectiveRunStatus(run),
     ...(error ? { error: truncateSnippet(error) } : {}),
     ...(run.failedStageId ? { failedStageId: run.failedStageId } : {}),
     ...(failedStage ? { stageId: failedStage.id, stageName: failedStage.name } : {}),
@@ -330,13 +337,22 @@ function jsonString(value: string): string {
 }
 
 function terminalNoticeKind(run: RunSnapshot): "completed" | "failed" | "blocked" | undefined {
-  if (run.status === "failed" || run.status === "blocked") return run.status;
-  if (run.status !== "completed") return undefined;
-  const returnedStatus = run.result?.["status"];
-  if (returnedStatus === "failed" || returnedStatus === "blocked") return returnedStatus;
+  const status = effectiveRunStatus(run);
+  if (status === "failed" || status === "blocked") return status;
+  if (status !== "completed") return undefined;
   return "completed";
 }
 
+function returnedNoticeError(run: RunSnapshot, kind: "completed" | "failed" | "blocked"): string | undefined {
+  const structuredFailureText = structuredRecoverableWorkflowFailureText(run);
+  if (kind === "blocked" && structuredFailureText !== undefined) return structuredFailureText;
+  const returnedStatus = normalizeReturnedWorkflowStatus(run.result?.["status"]);
+  if (returnedStatus === undefined) return undefined;
+  if (kind === "failed" && returnedStatus === "failed") return actionableReturnedStatusText(run.result);
+  if (kind === "blocked" && isReturnedBlockedWorkflowStatus(returnedStatus)) return actionableReturnedStatusText(run.result);
+  return undefined;
+}
+
 function terminalRunKey(kind: "completed" | "failed" | "blocked", runId: string): string {
   return `${kind}:${runId}`;
 }

packages/workflows/src/runs/background/status.ts

@@ -18,6 +18,7 @@ import { stageControlRegistry as defaultStageControlRegistry } from "../foregrou
 import { appendRunEnd } from "../../shared/persistence-session-entries.js";
 import { expandWorkflowGraph } from "../../shared/expanded-workflow-graph.js";
 import { topLevelWorkflowRuns } from "../../shared/run-visibility.js";
+import { actionableReturnedStatusText, effectiveRunStatus, structuredRecoverableWorkflowFailureText } from "../../shared/returned-run-status.js";
 
 // ---------------------------------------------------------------------------
 // Types
@@ -115,7 +116,7 @@ export function statusRuns(opts?: { all?: boolean; store?: Store }): RunStatusEn
   return topLevelWorkflowRuns(snapshot.runs).map((run) => ({
     runId: run.id,
     name: run.name,
-    status: run.status,
+    status: effectiveRunStatus(run),
     startedAt: run.startedAt,
     durationMs: run.durationMs,
     stageCount: expandWorkflowGraph(snapshot, run.id).stages.length,
@@ -470,7 +471,7 @@ export function inspectRun(
   const detail: RunDetail = {
     runId: copy.id,
     name: copy.name,
-    status: copy.status,
+    status: effectiveRunStatus(copy),
     mode: expandedStages.length > 1 ? "chain" : "single",
     startedAt: copy.startedAt,
     endedAt: copy.endedAt,
@@ -481,7 +482,7 @@ export function inspectRun(
     inputs: copy.inputs,
     stages: expandedStages.map((stage) => structuredClone(stage)),
     result: copy.result,
-    error: copy.error,
+    error: copy.error ?? (effectiveRunStatus(copy) === copy.status ? undefined : (structuredRecoverableWorkflowFailureText(copy) ?? actionableReturnedStatusText(copy.result))),
     exited: copy.exited,
     exitReason: copy.exitReason,
     failureKind: copy.failureKind,