fix(actions): surface inner result envelope (no more false-success on failed actions)

__tests__/lib/record-actions.test.ts

@@ -0,0 +1,45 @@
+/**
+ * Tests for interpretEnvelope — the server-response interpreter behind object
+ * actions / flow triggers. The key case: a transport 200 whose body wraps an
+ * INNER { success: false } must be reported as a failure, not a false success.
+ */
+import { interpretEnvelope } from "~/lib/record-actions";
+
+const ok = { ok: true, status: 200 };
+const notFound = { ok: false, status: 404 };
+
+describe("interpretEnvelope", () => {
+  it("treats a clean 200 as success and lifts the inner data payload", () => {
+    const r = interpretEnvelope(ok, { success: true, data: { id: "1" } }, "fallback", true);
+    expect(r).toEqual({ success: true, data: { id: "1" }, reload: true });
+  });
+
+  it("fails on a non-ok HTTP status", () => {
+    const r = interpretEnvelope(notFound, null, "fallback err", false);
+    expect(r.success).toBe(false);
+    expect(r.error).toBe("fallback err");
+  });
+
+  it("fails when the outer envelope says success:false", () => {
+    const r = interpretEnvelope(ok, { success: false, error: "denied" }, "fallback", false);
+    expect(r).toMatchObject({ success: false, error: "denied" });
+  });
+
+  it("fails when the INNER data envelope says success:false (the bug)", () => {
+    // What the action route actually returns for an unknown handler.
+    const body = { success: true, data: { success: false, error: "Action 'x' not found" } };
+    const r = interpretEnvelope(ok, body, "fallback", true);
+    expect(r.success).toBe(false);
+    expect(r.error).toBe("Action 'x' not found");
+  });
+
+  it("falls back to the default error when the inner failure has no message", () => {
+    const r = interpretEnvelope(ok, { success: true, data: { success: false } }, "fallback", true);
+    expect(r.success).toBe(false);
+    expect(r.error).toBe("fallback");
+  });
+
+  it("honours the reload flag on success", () => {
+    expect(interpretEnvelope(ok, { success: true, data: {} }, "f", false).reload).toBe(false);
+  });
+});

lib/record-actions.ts

@@ -98,6 +98,35 @@ async function readJson(res: Response): Promise<Record<string, unknown> | null>
   }
 }
 
+/**
+ * Interpret a server response envelope into an {@link ActionResult}.
+ *
+ * Automation / action routes wrap the handler's own result, so a transport-level
+ * 200 with `{ success: true }` can still carry an INNER `{ success: false,
+ * error }` in `data` (e.g. "Action 'x' not found"). Checking only the outer
+ * flag reports those failures as success — so we unwrap one level and honour the
+ * inner `success` too, lifting the inner `error` for display.
+ */
+export function interpretEnvelope(
+  res: Pick<Response, "ok" | "status">,
+  json: Record<string, unknown> | null,
+  fallbackError: string,
+  reload: boolean,
+): ActionResult {
+  if (!res.ok || json?.success === false) {
+    return { success: false, error: (json?.error as string) ?? fallbackError };
+  }
+  const inner = json?.data;
+  if (inner && typeof inner === "object" && (inner as { success?: unknown }).success === false) {
+    const innerObj = inner as { error?: unknown };
+    return {
+      success: false,
+      error: typeof innerObj.error === "string" ? innerObj.error : fallbackError,
+    };
+  }
+  return { success: true, data: inner, reload };
+}
+
 /* ---- Per-type dispatch ------------------------------------------------- */
 
 async function dispatchUrl(
@@ -136,13 +165,12 @@ async function dispatchFlow(
     },
   );
   const json = await readJson(res);
-  if (!res.ok || json?.success === false) {
-    return {
-      success: false,
-      error: (json?.error as string) ?? `Flow "${flowName}" failed (HTTP ${res.status})`,
-    };
-  }
-  return { success: true, data: json?.data, reload: action.refreshAfter !== false };
+  return interpretEnvelope(
+    res,
+    json,
+    `Flow "${flowName}" failed (HTTP ${res.status})`,
+    action.refreshAfter !== false,
+  );
 }
 
 async function dispatchApi(
@@ -160,13 +188,12 @@ async function dispatchApi(
       body: JSON.stringify(params),
     });
     const json = await readJson(res);
-    if (!res.ok || json?.success === false) {
-      return {
-        success: false,
-        error: (json?.error as string) ?? `Request failed (HTTP ${res.status})`,
-      };
-    }
-    return { success: true, data: json?.data, reload: action.refreshAfter !== false };
+    return interpretEnvelope(
+      res,
+      json,
+      `Request failed (HTTP ${res.status})`,
+      action.refreshAfter !== false,
+    );
   }
   // Generic record mutation with the collected params.
   if (Object.keys(params).length > 0) {
@@ -191,13 +218,12 @@ async function dispatchServerAction(
     },
   );
   const json = await readJson(res);
-  if (!res.ok || json?.success === false) {
-    return {
-      success: false,
-      error: (json?.error as string) ?? `Action "${target}" failed (HTTP ${res.status})`,
-    };
-  }
-  return { success: true, data: json?.data, reload: action.refreshAfter === true };
+  return interpretEnvelope(
+    res,
+    json,
+    `Action "${target}" failed (HTTP ${res.status})`,
+    action.refreshAfter === true,
+  );
 }
 
 /* ---- Lifecycle --------------------------------------------------------- */