fix(approvals,rest,types): a stranded decision publishes finalized / decision / runId / repairable beside its 500

.changeset/truthful-stranded-decision-envelope.md

@@ -0,0 +1,17 @@
+---
+"@objectstack/types": minor
+"@objectstack/plugin-approvals": minor
+"@objectstack/rest": minor
+---
+
+An approval decision that lands while its flow run strands now says so in fields, not only in prose.
+
+`POST /api/v1/approvals/requests/{id}/reject` — and its sibling decision doors — could produce three coexisting outcomes from one call: the caller read HTTP 500, the request row **was** in its terminal status and had left the pending inbox, and the workflow run was stranded. A caller reading 500 has one honest inference available — "the rejection did not happen" — and it was the wrong one, so scripts and operators retried or escalated against a decision that was already durable. The only carrier of the truth was English prose in `error`, so finding the affected run meant regexing a run id out of a sentence, and nothing said whether that run could be repaired at all.
+
+The 500 stays. A recorded decision whose flow never advances is still a failure and is still reported as one; the door does not become atomic and no decision is ever rolled back. What changed is that it stops discarding what the engine already said:
+
+- **The `RESUME_FAILED` body gains four fields**, additively — `finalized` (always `true`: the decision stands), `decision`, `runId`, and `repairable`. Existing consumers see the same `code`, the same `error` and the same status.
+- **`repairable` carries the engine's own discriminator** — `AutomationResult.status === 'stranded'`, the state stamped on exactly the exit that journals a repair snapshot. `false` is the answer for every other failure, including a lost run: absence of the signal is not repairability, and a repair verb that would refuse is worse than no promise.
+- **`serviceResume` carries `status`** through to the door. It previously read only `success` / `code` / `error`, and the stranded exit reports a `status` and no `code` at all — so the platform's own repairability signal died one line before the envelope was built.
+
+`@objectstack/types` gains `strandedDecisionFailure` / `strandedDecisionDetails` and the `StrandedDecisionDetails` type — the constructor and its recogniser in one module, so the producing service and the REST door cannot drift. A `RESUME_FAILED` raised without that carrier answers exactly the body it always did; the door never synthesises the envelope.

content/docs/permissions/system-context.mdx

@@ -64,7 +64,7 @@ not on any flag.
 ## How the flag is set
 
 `isSystem` is **server-constructed and never client-supplied**. Inbound HTTP
-cannot set it (`packages/rest/src/rest-server.ts:1548`, `:1577`), and neither
+cannot set it (`packages/rest/src/rest-server.ts:1553`, `:1582`), and neither
 can an action body (`packages/runtime/src/domains/actions.ts:414`). It is
 written by internal callers only, as an option on the engine call:
 
@@ -103,7 +103,7 @@ that silently does not happen.
 | 14 | MCP stdio bridge skips the object API-exposure gate | mcp | Get: the bridge reaches objects whose `apiEnabled` / `apiMethods` would refuse an external caller | `stdio-data-bridge.ts:246` |
 | 15 | **Read-audit rows are not written** | plugin-audit | Lose: the "a person opened this record" trail. `sudo()` keeps the caller's `userId`, so this flag is the only thing separating a human read from a platform one | `read-audit.ts:556` |
 | 16 | Approval snapshot payload redaction skipped | plugin-approvals | Get: the whole snapshot on `find` / `findOne` — the audit/replay channel. Lose: field-visibility redaction over approval payloads | `payload-redaction-middleware.ts:115` |
-| 17 | REST anonymous-deny seam satisfied | rest | Get: `enforceAuth` passes with no `userId`. Not reachable from the wire — `isSystem` is never set on an inbound request | `rest-server.ts:1580` |
+| 17 | REST anonymous-deny seam satisfied | rest | Get: `enforceAuth` passes with no `userId`. Not reachable from the wire — `isSystem` is never set on an inbound request | `rest-server.ts:1585` |
 
 ### 2. Write pipeline and data integrity
 
@@ -145,7 +145,7 @@ The largest single consumer — **17 of the 106 sites**.
 |:--|:---|:---|:---|:---|
 | 40 | **Approval record lock released** — a locked record is writable | plugin-approvals | Get: engine self-writes (the status mirror) pass. Lose: the lock that stops edits while an approval is live. Note there is deliberately **no admin exemption** here — only `isSystem` | `lifecycle-hooks.ts:347` |
 | 41 | Delegation write guard bypassed | plugin-approvals | Get: service / seed / import may write delegation rows naming another delegator | `lifecycle-hooks.ts:570` |
-| 42 | Approval actor / submitter / pending-approver checks bypassed (8 sites) | plugin-approvals | Get: approve, reject, recall, reassign without being a pending approver or the submitter | `plugin-approvals/src/approval-service.ts:963`, `:1072`, `:3196`, `:3342`, `:3509`, `:3580`, `:3769`, `:3809` |
+| 42 | Approval actor / submitter / pending-approver checks bypassed (8 sites) | plugin-approvals | Get: approve, reject, recall, reassign without being a pending approver or the submitter | `plugin-approvals/src/approval-service.ts:963`, `:1072`, `:3248`, `:3396`, `:3564`, `:3635`, `:3824`, `:3864` |
 | 43 | Saved-report ownership is **assignable**, and an update may reassign it | plugin-reports | Get: `ownerId` from input is honoured. A non-system caller always owns what it creates and can never reassign | `plugin-reports/src/report-service.ts:404`, `:425` |
 | 44 | Saved-report access / export / mutation gates bypassed | plugin-reports | Get: read, bulk-export and overwrite any report | `plugin-reports/src/report-service.ts:343`, `:372`, `:447`, `:684` |
 | 45 | Attachment access hooks return early (insert + update + delete, and the read AST) | service-storage | Lose: attachment visibility scoping | `attachment-access-hooks.ts:300`, `:349`, `:448`, `:524` |
@@ -158,7 +158,7 @@ The largest single consumer — **17 of the 106 sites**.
 |:--|:---|:---|:---|:---|
 | 48 | Object API-exposure gate bypassed (`apiEnabled` / `apiMethods`) | runtime | Get: internal self-writes ignore exposure declarations — these govern **external** exposure, not engine self-writes | `action-execution.ts:138` |
 | 49 | Action `requiredPermissions` bypassed | runtime | Get: engine self-invocation runs any action | `action-execution.ts:401` |
-| 50 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability | `domains/meta.ts:471`, `:874`, `rest-server.ts:4967`, `:6381`, `:6629`, `:7060`, `:7253` |
+| 50 | `manage_metadata` bypassed on metadata writes | runtime, rest | Get: schema writes without the capability | `domains/meta.ts:471`, `:874`, `rest-server.ts:4972`, `:6386`, `:6634`, `:7065`, `:7258` |
 | 51 | The shared metadata-write verdict itself returns `allowed` | metadata-core | Get: the one function all of row 50's doors consult answers yes before any capability is examined | `meta-write-capability.ts:134` |
 | 52 | Anonymous-deny seam satisfied on the domain dispatchers and the package/federation routes | runtime, rest | Get: passes with no `userId` | `domains/actions.ts:421`, `domains/ai.ts:60`, `domains/automation.ts:989`, `domains/meta.ts:232`, `domains/security.ts:78`, `domains/packages.ts:422`, `external-datasource-routes.ts:302`, `package-routes.ts:97` |
 | 53 | MCP principal check satisfied | runtime | Get: MCP surface reachable with no user | `domains/mcp.ts:61` |
@@ -199,7 +199,7 @@ assuming `isSystem` covers it is a documented source of bugs.
 | "It preserves a supplied `updated_at` / `updated_by`" | **No.** That is `preserveAudit`, a separate opt-in — and an UPDATE-path exemption only | `field.zod.ts:1580` (#3493 / #6640) |
 | "It stamps `created_by`" | **No.** Audit stamping reads `userId` from the context. A user-less system write stamps nothing — that is today's behaviour, not an error | `runtime-identity.ts:280`–`281` |
 | "It bypasses every guard" | **No.** The last-admin guard applies to **every** context, `isSystem` included — the deprovision path that actually locks an org out is the system one | `last-admin-guard.ts:299` |
-| "A client can request it" | **No.** Never settable from inbound HTTP or from an action body | `rest-server.ts:1548`, `:1577`; `domains/actions.ts:414` |
+| "A client can request it" | **No.** Never settable from inbound HTTP or from an action body | `rest-server.ts:1553`, `:1582`; `domains/actions.ts:414` |
 
 ---
 

packages/plugins/plugin-approvals/src/approval-service.ts

@@ -18,7 +18,7 @@ import { ExpressionEngine, collectCelRootIdentifiers } from '@objectstack/formul
 // writer-local re-derivation here was rejected by name (Option B): it would be
 // a third answer to a question the codebase already answered two ways.
 import { createRecordOrganizationResolver, type RecordOrganizationResolver } from '@objectstack/metadata-core';
-import { keysetWalk } from '@objectstack/types';
+import { keysetWalk, strandedDecisionFailure } from '@objectstack/types';
 import {
   ADMIN_FULL_ACCESS,
   ORGANIZATION_ADMIN_GRANTS,
@@ -2665,15 +2665,17 @@ export class ApprovalService implements IApprovalService {
     signal: { output?: Record<string, unknown>; branchLabel?: string },
   ): Promise<void> {
     const result = await this.automation!.resume!(runId, { ...signal, [RESUME_AUTHORITY_SERVICE]: true });
-    const reported = result as { success?: boolean; code?: string; error?: string } | undefined;
+    const reported = result as
+      { success?: boolean; code?: string; error?: string; status?: string } | undefined;
     // Only an explicit `success: false` is a failure. An engine (or a test
     // double) that returns nothing is reporting nothing, and has always meant
     // "it ran".
     if (reported && typeof reported === 'object' && reported.success === false) {
       const err = new Error(
         `resume of run '${runId}' failed${reported.code ? ` [${reported.code}]` : ''}: ${reported.error ?? 'unknown error'}`,
-      ) as Error & { resumeCode?: string };
+      ) as Error & { resumeCode?: string; resumeStatus?: string };
       err.resumeCode = reported.code;
+      err.resumeStatus = reported.status;
       throw err;
     }
   }
@@ -2683,6 +2685,23 @@ export class ApprovalService implements IApprovalService {
     return (err as { resumeCode?: string } | undefined)?.resumeCode;
   }
 
+  /**
+   * The engine's own run-state discriminator behind a {@link serviceResume}
+   * rejection — `AutomationResult.status` — if the engine reported one
+   * (#13807).
+   *
+   * Read as a SIBLING of {@link resumeCodeOf}, never as a substitute: the two
+   * answer different questions and the stranded exit proves they are not
+   * interchangeable. It reports `status: 'stranded'` and **no `code` at all**
+   * (`service-automation` `engine.ts`, the resume catch arm), so a door that
+   * reads only the code sees an unnamed failure and cannot tell a repairable
+   * strand from a dead run — which is how the platform's own repairability
+   * signal had a producer and zero consumers until this call site.
+   */
+  private static resumeStatusOf(err: unknown): string | undefined {
+    return (err as { resumeStatus?: string } | undefined)?.resumeStatus;
+  }
+
   /**
    * Refuse an operation whose whole point is to advance a flow run when that
    * run no longer exists — BEFORE anything is written down (#4420).
@@ -2788,14 +2807,38 @@ export class ApprovalService implements IApprovalService {
    * which cannot throw without breaking every standalone deployment — it
    * reports through `resumeError` instead.
    *
+   * ## The throw is truthful, not merely loud (#13807)
+   *
+   * Maintainer ruling 2026-09-04 (decision batch #37, option B): this door
+   * KEEPS its status code — the effect landing while the run strands is still
+   * a failure and must still be reported as one — and stops discarding what
+   * the engine said. ⛔ Not "return 200", which the card forbids; ⛔ not
+   * atomic, because rolling a real human decision back is excluded by the
+   * #13937 shape-4 ruling, which binds this door's own writes too (a machine
+   * that re-armed strandings by itself would re-run the node that threw,
+   * forever, with nobody deciding it should).
+   *
+   * So the error carries {@link StrandedDecisionDetails} beside its prose:
+   * `finalized` (the decision stands), `decision`, `runId`, and `repairable`
+   * derived from the engine's `'stranded'` discriminator. Before this a caller
+   * had a 500 and a sentence — and 500 alone reads as "the rejection did not
+   * happen", which is the misreading that makes a caller retry or escalate
+   * against a decision that IS durable.
+   *
    * @param what - how the recorded outcome reads in the error, e.g.
    *   `"the approve decision"`.
+   * @param decision - the outcome label for the machine-readable envelope
+   *   (`'approve'` / `'reject'` / `'revise'` / `'resubmit'`). Passed
+   *   explicitly rather than parsed back out of `what` or the signal: the
+   *   prose is for humans and `output` is the flow's, and neither is a place
+   *   to keep a wire value.
    */
   private async resumeRecordedOutcome(
     runId: string,
     requestId: string,
     what: string,
     signal: { output?: Record<string, unknown>; branchLabel?: string },
+    decision: string,
   ): Promise<{ resumed: boolean; resumeError?: string }> {
     const missing = this.missingRunCapability(runId, requestId, what, 'resume');
     if (missing) return { resumed: false, resumeError: missing };
@@ -2810,12 +2853,20 @@ export class ApprovalService implements IApprovalService {
         });
         return { resumed: false, resumeError: reason };
       }
+      // #13807: the engine's own discriminator decides `repairable`, never
+      // this door and never the message text. `'stranded'` is the ONE exit
+      // that journalled a repair snapshot, so it is the one exit an operator
+      // can act on; every other failure (a lost run, an engine too old to
+      // report a status) is honestly `false`.
+      const status = ApprovalService.resumeStatusOf(err);
+      const repairable = status === 'stranded';
       this.logger?.error?.('[approvals] resume failed — the run is stranded', {
-        request: requestId, run: runId, outcome: what, error: reason,
+        request: requestId, run: runId, outcome: what, error: reason, status, repairable,
       });
-      throw new Error(
+      throw strandedDecisionFailure(
         `RESUME_FAILED: ${what} was recorded on request ${requestId}, but its flow run '${runId}' ` +
         `could not be resumed and is now stranded: ${reason}`,
+        { finalized: true, decision, runId, repairable },
       );
     }
   }
@@ -2862,6 +2913,7 @@ export class ApprovalService implements IApprovalService {
           // whitelist already rejects them; this is defense in depth).
           output: { ...(result.outputs ?? {}), decision: result.decision, requestId },
         },
+        result.decision,
       );
       resumed = outcome.resumed;
       resumeError = outcome.resumeError;
@@ -3254,6 +3306,7 @@ export class ApprovalService implements IApprovalService {
             branchLabel: APPROVAL_BRANCH_LABELS.reject,
             output: { decision: 'reject', autoRejected: true, requestId },
           },
+          'reject',
         );
         resumed = outcome.resumed;
         resumeError = outcome.resumeError;
@@ -3295,6 +3348,7 @@ export class ApprovalService implements IApprovalService {
           branchLabel: APPROVAL_BRANCH_LABELS.revise,
           output: { decision: 'revise', requestId },
         },
+        'revise',
       );
       resumed = outcome.resumed;
       resumeError = outcome.resumeError;
@@ -3384,6 +3438,7 @@ export class ApprovalService implements IApprovalService {
           branchLabel: APPROVAL_BRANCH_LABELS.resubmit,
           output: { resubmitted: true, requestId },
         },
+        'resubmit',
       );
       resumed = outcome.resumed;
       resumeError = outcome.resumeError;