fix(workflows): an approval must not repeat an outward call, finish a run that is waiting, or hide what it is approving (#846)

frontend/src/lib/language.ts

@@ -266,13 +266,50 @@ const TOOL_LABELS: Readonly<Record<string, string>> = {
  * left alone so no other surface shifts under this change.
  */
 export function approvalAction(a: ApprovalSummary): string {
+  // Issue #846: a paused workflow gate is named by the CALL it is stopping, not
+  // by the mechanism that stopped it. "Continue a paused workflow" is true of
+  // every one of these cards and therefore tells an operator nothing — it is
+  // #372's "Glob" complaint, one surface over: the chat path was fixed by #375
+  // and this path was not, because its label came from `EFFECT_LABELS` (which
+  // keys on the effect kind, and the kind here is always `workflow.approve`)
+  // rather than from the tool.
+  //
+  // The tool is on the wire whenever the host could classify the node's call.
+  // When it could not — an authored gate on a step that calls nothing — the
+  // glossary entry below is still the honest answer, so this promotes and never
+  // hides.
+  const workflowTool = workflowGateTool(a);
+  if (workflowTool) return toolAction(workflowTool);
   return (
     labelFor(EFFECT_LABELS, a.kind) ??
     labelFor(TOOL_LABELS, a.kind) ??
     (a.agent ? "Use one of its tools" : "Do something that needs your sign-off")
   );
 }
 
+/** The effect kind a paused workflow gate parks as — mirrors
+ * `WORKFLOW_APPROVE_KIND` in `src/runtime/workflow_resume.rs`. */
+const WORKFLOW_APPROVE_KIND = "workflow.approve";
+
+/**
+ * The tool a paused workflow gate is stopping, when the host named one
+ * (issue #846) — `null` for every other kind, and for a gate whose node makes
+ * no classifiable call.
+ *
+ * Reads `payload.tool`, which the host writes for a policy-raised gate (#460)
+ * and, since #846, for an authored one too. Absence is meaningful and is
+ * preserved as such: an **older host** omits the key entirely, and this then
+ * returns `null` so the card renders exactly the pre-#846 line rather than an
+ * empty label.
+ */
+function workflowGateTool(a: ApprovalSummary): string | null {
+  if (a.kind !== WORKFLOW_APPROVE_KIND) return null;
+  const payload = a.payload;
+  if (payload == null || typeof payload !== "object" || Array.isArray(payload)) return null;
+  const tool = (payload as Record<string, unknown>).tool;
+  return typeof tool === "string" && tool !== "" ? tool : null;
+}
+
 /**
  * What a tool does, in plain language, from its identifier alone (#374).
  *
@@ -324,7 +361,10 @@ export function payloadLines(a: ApprovalSummary): PayloadLine[] {
   if (typeof payload !== "object") return [{ label: "value", value: renderValue(payload) }];
   if (Array.isArray(payload)) return [{ label: "items", value: renderValue(payload) }];
 
-  const entries = Object.entries(payload as Record<string, unknown>);
+  const entries =
+    a.kind === WORKFLOW_APPROVE_KIND
+      ? workflowGateEntries(payload as Record<string, unknown>)
+      : Object.entries(payload as Record<string, unknown>);
   if (entries.length === 0) return [];
 
   // Preferred ordering for the kinds whose argument names we know. Unlisted
@@ -340,6 +380,66 @@ export function payloadLines(a: ApprovalSummary): PayloadLine[] {
     .map(([label, value]) => ({ label, value: renderValue(value) }));
 }
 
+/**
+ * The payload of a paused workflow gate, as the lines an operator needs
+ * (issue #846).
+ *
+ * A `workflow.approve` payload is not a tool call's arguments — it is the host's
+ * **resume record**, and every other card's rule ("show the payload, it is what
+ * you are consenting to") produces exactly the wrong thing here. What an
+ * operator saw was `input: {"items":[{"json":{}}],"port":null}`: the engine's
+ * seed payload, verbatim, as the sole description of a decision.
+ *
+ * So this promotes the call's own arguments to the top level, where every other
+ * card carries them, and drops the machinery. The dropped keys are dropped for
+ * stated reasons, not for tidiness:
+ *
+ * * `input` — the resume payload. It is engine seed data, it is the thing that
+ *   read as a description and was not one, and it also carries an `approvals`
+ *   list that accumulates down a lineage: an operator seeing
+ *   `{"approvals":["fetch_bbc","fetch_espn"]}` reads it as a card covering all
+ *   of them, which it is not. (Consolidating several gates into one card is
+ *   real and is issue #842 — not something to imply by accident here.)
+ * * `delivered`, `performed` — the #438/#846 ledgers. They say what will NOT
+ *   happen again, which is a property of the mechanism and is already stated in
+ *   prose by `note`.
+ * * `content` — rendered in full by its own block (`WorkflowContentReview`,
+ *   #596); repeating it as one clamped JSON line would be strictly worse.
+ * * `note` — prose, rendered as prose elsewhere on the card, not as a
+ *   `key: value` pair in a monospace block.
+ *
+ * Everything else survives, including any key a **newer host** adds that this
+ * console has never heard of. Dropping unknown keys would make an old console
+ * silently hide new information, which is the failure mode this whole file is
+ * written against; the denylist is closed and the allowlist is not.
+ */
+function workflowGateEntries(payload: Record<string, unknown>): [string, unknown][] {
+  const args = payload.args;
+  const argEntries: [string, unknown][] =
+    args != null && typeof args === "object" && !Array.isArray(args)
+      ? Object.entries(args as Record<string, unknown>)
+      : [];
+  const rest = Object.entries(payload).filter(([key]) => !WORKFLOW_GATE_HIDDEN.has(key));
+  // The call first, then where and what stopped it. An operator decides on the
+  // call; the node id is how they find it afterwards. This order survives
+  // `payloadLines`' sort because no `PAYLOAD_KEY_ORDER` entry exists for this
+  // kind — every rank is equal, and the sort is stable.
+  return [...argEntries, ...rest];
+}
+
+/** Payload keys of a `workflow.approve` card that are machinery, not the
+ * decision — see {@link workflowGateEntries} for why each one is here. */
+const WORKFLOW_GATE_HIDDEN: ReadonlySet<string> = new Set([
+  "input",
+  "delivered",
+  "performed",
+  "content",
+  "note",
+  // Unwrapped one level above, so keeping it would print the same arguments
+  // twice — once readably and once as a JSON blob.
+  "args",
+]);
+
 /** Which arguments lead the preview, per tool. Presentation only. */
 const PAYLOAD_KEY_ORDER: Readonly<Record<string, string[]>> = {
   shell: ["command", "cwd", "timeout"],

frontend/src/views/workflows/RunHistoryPanel.tsx

@@ -16,7 +16,7 @@ import type {
 } from "@/api/workflows";
 
 import { failedNodeOf } from "./graph";
-import { pendingCount, relativeTime, runTone, undeliveredCount } from "./run-health";
+import { awaitingCount, relativeTime, runTone, undeliveredCount } from "./run-health";
 
 /** Badge styling per delivery outcome. A report that did NOT go out must not
  * look like one that did — `denied` and `failed` are the two an operator has to
@@ -82,7 +82,9 @@ export function DeliveryRows({ deliveries }: { deliveries: DeliveryReport[] }) {
 export function LastRunChip({ run }: { run: WorkflowRunOutcome }) {
   const tone = runTone(run);
   const undelivered = undeliveredCount(run.deliveries);
-  const pending = pendingCount(run.deliveries);
+  // Issue #846: gates and parked reports together. The chip said "Manual run"
+  // and a green dot for a run whose first node was still waiting on a person.
+  const awaiting = awaitingCount(run);
   return (
     <Badge
       variant="outline"
@@ -111,8 +113,8 @@ export function LastRunChip({ run }: { run: WorkflowRunOutcome }) {
             ? " stopped"
             : undelivered > 0
             ? ` · ${undelivered} not delivered`
-            : pending > 0
-              ? ` · ${pending} awaiting approval`
+            : awaiting > 0
+              ? ` · ${awaiting} awaiting approval`
               : ""}
       <span className="text-muted-foreground">· {relativeTime(run.atMillis)}</span>
     </Badge>
@@ -291,6 +293,36 @@ function RunHistoryRow({
         <p className="text-2xs text-muted-foreground">
           Still running — reports are routed when it finishes.
         </p>
+      ) : run.pendingApprovals.length > 0 ? (
+        <>
+          {/* A paused run can still have routed reports — the output nodes it
+              reached BEFORE the gate. Those rows are shown as they always were,
+              with the waiting line above rather than instead of them: replacing
+              them would trade one silent omission for another. */}
+          {run.deliveries.length > 0 && <DeliveryRows deliveries={run.deliveries} />}
+        // Issue #846. This is the arm that was missing, and its absence is how a
+        // run waiting on a human came to report success: a paused run has no
+        // error, no cancellation, is not `running` (the engine settled it) and
+        // routed nothing, so it fell through every branch to the "Finished" line
+        // below — while its gate sat undecided on the Approvals page.
+        //
+        // "Not finished" is the claim, stated in the operator's terms rather
+        // than the engine's. The run object really is settled; what has not
+        // happened is the work, and the work is what the operator is asking
+        // about. Naming the nodes matters as much as the state: a scheduled run
+        // that silently did nothing is exactly the failure this reads as, and
+        // the fix is a click, so the row says which click.
+        <p
+          className="text-2xs text-[var(--status-blocked-text)]"
+          data-testid="workflow-run-awaiting"
+        >
+          Not finished — waiting for your approval on{" "}
+          {run.pendingApprovals.map((node) => `“${node}”`).join(", ")}. Nothing past{" "}
+          {run.pendingApprovals.length === 1 ? "it" : "them"} has run
+          {run.deliveries.length === 0 ? ", and no reports were routed" : ""}. Approve or decline
+          it in Approvals to carry the run on.
+        </p>
+        </>
       ) : run.deliveries.length > 0 ? (
         // Deliberately the SAME component the live run drawer uses, so a report
         // reads identically whether it's on screen now or a week old.

frontend/src/views/workflows/run-health.ts

@@ -33,6 +33,25 @@ export function pendingCount(deliveries: DeliveryReport[]): number {
   return deliveries.filter((d) => d.status === PENDING_STATUS).length;
 }
 
+/**
+ * Everything about this run that is waiting on a person: the gates it paused at
+ * **and** the reports it parked (issue #846).
+ *
+ * The two were never read together, and that is what let a run report success
+ * while a human had not answered it. `pendingCount` sees only `deliveries`, so a
+ * run that paused at a `requires_approval` node and therefore never reached an
+ * `output` node at all — the exact shape of a gated workflow — has an empty
+ * `deliveries` array and scored as a clean run.
+ *
+ * `pendingApprovals` has been on the wire since #395 and the history row has
+ * badged its count since; nothing read it for the run's *state*. This is that
+ * read, in one place, so the tone, the chip and the row cannot disagree about
+ * whether somebody is being waited on.
+ */
+export function awaitingCount(run: WorkflowRunOutcome): number {
+  return (run.pendingApprovals?.length ?? 0) + pendingCount(run.deliveries);
+}
+
 /** A compact "N minutes ago" for a run timestamp — enough to tell last night's
  * scheduled run from the one just clicked, without a date library. */
 export function relativeTime(atMillis: number): string {
@@ -77,8 +96,12 @@ export function runTone(run: WorkflowRunOutcome): { dot: string; label: string }
   // Blocked, not running. This was the running colour, which said "the machine
   // is working on it" about the one state that means the opposite: it is
   // parked until a human decides. Amber is the colour that gets looked at.
-  if (pendingCount(run.deliveries) > 0)
-    return { dot: "bg-status-blocked", label: "awaiting approval" };
+  //
+  // Issue #846: `awaitingCount`, not `pendingCount`. A run that paused at a gate
+  // parked no report — it never reached an output node — so the delivery-only
+  // read scored it green and the operator was told a run that did none of its
+  // work had succeeded.
+  if (awaitingCount(run) > 0) return { dot: "bg-status-blocked", label: "awaiting approval" };
   return { dot: "bg-status-done", label: "ok" };
 }