fix(plugin-approvals): report a run that failed mid-resume as stranded, and measure the resume-ordering fork (#13909)#13934

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
os-steve merged 4 commits into from
Sep 1, 2026
+346 −12
Merged

fix(plugin-approvals): report a run that failed mid-resume as stranded, and measure the resume-ordering fork (#13909) #13934

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8e2ec8f
fix(plugin-approvals): report a run that failed mid-resume as strande…
claude Aug 31, 2026
e138bf9
docs(permissions): re-anchor the system-context census rows this PR's…
claude Aug 31, 2026
d987bbb
docs(plugin-approvals): fix the runId field doc the widening falsifie…
claude Sep 1, 2026
d3f53f0
docs(permissions): re-anchor the 8 approval-service census rows the +…
claude Sep 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions .changeset/approvals-inspector-sees-failed-mid-resume.md
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
---
"@objectstack/plugin-approvals": patch
---

fix(plugin-approvals): the stranded-request inspection now sees a run that FAILED mid-resume (#13909)

`ApprovalService.inspectStrandedRequests` was structurally blind to the shape
#13909 owns, and reported `0` for it — the one shape an operator most needs to
see.

**The mechanism.** `AutomationEngine.resumeInternal` consumes the suspension
*before* running the downstream nodes: `forgetSuspendedRun(run, 'resumed')`
precedes `traverseNext`. A downstream node that merely THREW therefore threw
with the pause already gone, the catch arm recorded the run `failed`, and
nothing can resume it again (`resume` answers `RUN_NOT_FOUND`, `cancelRun` is a
no-op). The decision is durable and the flow stopped half-way.

**Why the inspection could not see it.** Its second oracle was
`if (terminal) continue` — the existence of ANY run-history row ended the check,
on the reading "the run ran to a terminal state, it is not dangling". But the
terminal row here is written BY the failure that stranded the request, so the
evidence of the defect was read as evidence of health. `releaseDeadRunRequests`
cannot see it either: it scans `status: 'pending'`, and the decision is what
took the row out of `pending`.

**The widening, and its limits.** The second oracle now classifies the run
instead of merely detecting it. A `failed` run is reported; `completed`,
`cancelled` and `paused` are each still skipped, one named reason at a time, and
a status this code does not recognise is skipped too — the spec's
`ExecutionStatus` vocabulary is wider than the four statuses the engine writes,
and a future status must not become a silent false positive. `paused` in
particular stays skipped because "the suspension is gone but no terminal row is
written yet" is exactly what a resume IN FLIGHT looks like. The first oracle is
unchanged: a run the suspension store still holds is alive, and an unreadable
store is still counted `undetermined`, never condemned.

Reported rows now carry `runState: 'missing' | 'failed'` (new exported type
`StrandedRunState`), because the two shapes need different remedies: a `missing`
run has no history to read, a `failed` one has a step log and an error naming
the node that threw. The sweep's own warning splits its counts the same way.
`StrandedApprovalRequest` is an output-only reporting shape the service
produces; the added field is not constructed by any caller in this repo.

**Still read-only, and still not a census.** No status is changed and no run is
cancelled — the decision genuinely happened. This makes the condition *visible*
in a deployment; how many runs are already in it can only be answered against
that deployment's own tables. Nothing here changes the resume ordering, which
is #13909's own next slice.
2 changes: 1 addition & 1 deletion content/docs/permissions/system-context.mdx
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -145,7 +145,7 @@ The largest single consumer — **20 of the 109 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:333` |
| 41 | Delegation write guard bypassed | plugin-approvals | Get: service / seed / import may write delegation rows naming another delegator | `lifecycle-hooks.ts:440` |
| 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:850`, `:959`, `:2916`, `:3062`, `:3229`, `:3300`, `:3489`, `:3529` |
| 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:931`, `:1040`, `:2997`, `:3143`, `:3310`, `:3381`, `:3570`, `:3610` |
| 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` |
Expand Down
138 changes: 128 additions & 10 deletions packages/plugins/plugin-approvals/src/approval-service.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -247,6 +247,76 @@ const TERMINAL_RUN_STATUSES: ReadonlySet<string> = new Set([
*/
const STRANDABLE_REQUEST_STATUSES = ['approved', 'rejected', 'returned'] as const;

/**
* The second oracle's verdict: which unrecoverable shape this run is in, or
* `undefined` for every run that must NOT be reported (#13909).
*
* Written as an explicit switch, not `status !== 'completed'`, because the
* negatives are the load-bearing half — a widening that reports everything the
* old `if (terminal) continue` used to skip would bury the finding it exists to
* surface. Each skip below is a distinct reason, and each is pinned by its own
* test.
*
* The engine writes exactly four run statuses (`paused`, `completed`, `failed`,
* `cancelled`); the spec's `ExecutionStatus` vocabulary is wider (`timed_out`,
* `retrying`, …) and nothing in the engine writes the rest today. The `default`
* arm therefore stays SILENT rather than reporting: a status this code does not
* recognise is not evidence a decision was stranded, and condemning on it would
* make every future status a false positive until someone noticed.
*/
function classifyStrandedRunState(run: { status?: string } | null | undefined): StrandedRunState | undefined {
// No history row at all — the #4469 shape this inspection was built for.
if (!run) return 'missing';
switch (run.status) {
// The resume consumed the pause and a downstream node threw. Reported.
case 'failed':
return 'failed';

// ── The negatives, each for its own reason ──────────────────────────────
// The decision advanced the flow and the flow finished. Healthy.
case 'completed':
return undefined;
// Deliberately terminated by an operator (`cancelRun`, ADR-0044). The run
// stopping is the intended outcome, exactly as `recalled` is on the request
// side — reporting it would bury the real findings under expected ones.
case 'cancelled':
return undefined;
// The history's last row says `paused` while the suspension store says no
// live pause. That is AMBIGUOUS, and the ambiguity is not resolvable from
// one scan: a resume in flight right now has consumed the suspension and
// not yet written its terminal row, and reads exactly like a process that
// died in the same window. Condemning it would name every concurrently
// resuming approval — so this stays SKIPPED, the conservative arm this
// whole method is built on.
case 'paused':
return undefined;
default:
return undefined;
}
}

/**
* WHY a terminal request's run is unrecoverable — the two shapes the inspection
* reports, which have different causes and different remedies (#13909).
*
* - `missing` — `getRun` finds no history row at all (#4469's original shape):
* the run was lost before it could record anything, typically a pause that
* never reached a durable store and did not survive a restart.
* - `failed` — the run DID record a terminal `failed` row. The engine consumes
* a suspension *before* running the downstream nodes
* (`AutomationEngine.resumeInternal`: `forgetSuspendedRun(run, 'resumed')`
* precedes `traverseNext`), so a downstream node that merely THREW threw with
* the pause already gone — the catch arm recorded `failed` and there is no
* suspension left to resume. The decision is durable, the flow stopped
* mid-continuation, and no verb moves the run out of that state.
*
* ⚠️ This names the shapes for the REPORT only. It is not a run state: the
* engine's own vocabulary is still `'completed' | 'paused' | 'failed'`
* (`AutomationResult.status`) and nothing persists or queries "stranded".
* Giving the condition a platform-level name is #13909's own deliverable.
*/
export type StrandedRunState = 'missing' | 'failed';

/**
* One terminal request whose owning flow run is unrecoverable (#4469) — the
* decision was recorded and the flow never moved. Reporting shape only: the
Expand All@@ -257,8 +327,19 @@ export interface StrandedApprovalRequest {
requestId: string;
/** Terminal status the request reached — the decision that WAS recorded. */
status: string;
/** The `flow_run_id` that resolves to neither a suspension nor a run history row. */
/**
* The `flow_run_id` that resolves to no live suspension and no recoverable
* run — see `runState`: no history row at all (`missing`), or a terminal
* `failed` row (`failed`).
*/
runId: string;
/**
* Which unrecoverable shape this is — see {@link StrandedRunState}. Carried
* because the two need different remedies: a `missing` run has no history to
* read, while a `failed` one has a step log and an error message naming the
* node that threw.
*/
runState: StrandedRunState;
flowName?: string;
/** Approval node the run should have continued from. */
nodeId?: string;
Expand DownExpand Up@@ -3679,9 +3760,35 @@ export class ApprovalService implements IApprovalService {
* live pause exists. It THROWS when the store cannot be read, and that
* case is SKIPPED, never counted as dead: an unreadable store means
* "unknown", and a storage outage must not be published as a lost run.
* - `getRun(runId) == null` — no terminal history row either (the `run_`
* prefixed rows in `sys_automation_run`). A run that merely finished is
* not stranded; a request whose run neither waits nor ever completed is.
* - `classifyStrandedRunState` over `getRun(runId)` — the run's own
* history row (the `run_` prefixed rows in `sys_automation_run`). A run
* that merely finished is not stranded; a request whose run neither waits
* nor completed is.
*
* **The second oracle was widened (#13909), and this is the whole point of
* that card's first slice.** It used to be `if (terminal) continue` — the
* existence of ANY history row ended the check, on the reading "the run ran to
* a terminal state, it is not dangling". That is true of a run that COMPLETED
* and false of one that FAILED: the engine consumes a suspension *before*
* running the downstream nodes (`AutomationEngine.resumeInternal` calls
* `forgetSuspendedRun(run, 'resumed')` and only then `traverseNext`), so a
* downstream node that merely threw threw with the pause already gone, and the
* catch arm wrote a terminal `failed` row. The decision is durable, the
* continuation stopped half-way, `resume` answers `RUN_NOT_FOUND` and
* `cancelRun` is a no-op — and the terminal row this oracle used to read as
* health is written BY the very failure that stranded it. So this inspection
* reported `0` for the one shape an operator most needs to see.
*
* ⚠️ The widening does NOT reverse the conservatism: `completed`, `cancelled`
* and `paused` are each still skipped, for reasons named one at a time in
* `classifyStrandedRunState`, and an unrecognised status is skipped too.
* What the widening buys is that a `failed` run is now reported with
* `runState: 'failed'` instead of counted as healthy.
*
* ⚠️ **What this can and cannot size.** It makes the condition *visible* in a
* deployment; it is not itself a census, and it says nothing about this
* repository. How many runs are already in this state can only be answered
* against a real deployment's tables — see the card.
*
* **Reports; never rewrites.** No status is changed and no run is cancelled.
* The decision genuinely happened — a human approved or rejected — and
Expand DownExpand Up@@ -3748,10 +3855,15 @@ export class ApprovalService implements IApprovalService {
});
continue;
}
if (terminal) continue; // the run ran to a terminal state — it is not dangling

// Neither suspended nor ever finished: the run this decision was supposed
// to advance is genuinely gone.
// #13909 — the widened verdict. `undefined` means "not a shape this
// reports": healthy, deliberate, or unresolvable. See
// `classifyStrandedRunState` for which, and why each one.
const runState = classifyStrandedRunState(terminal);
if (!runState) continue;

// Neither suspended nor recoverable: the run this decision was supposed to
// advance is gone (`missing`) or terminally failed mid-continuation with
// its pause already consumed (`failed`).
const config = parseJson<ApprovalNodeConfig>(
raw.node_config_json, { approvers: [], behavior: 'first_response' } as any,
);
Expand All@@ -3770,6 +3882,7 @@ export class ApprovalService implements IApprovalService {
requestId: String(raw.id),
status: raw.status,
runId,
runState,
flowName: typeof raw.process_name === 'string' ? raw.process_name.replace(/^flow:/, '') : undefined,
nodeId: raw.flow_node_id ?? raw.current_step ?? undefined,
objectName: raw.object_name,
Expand All@@ -3782,9 +3895,14 @@ export class ApprovalService implements IApprovalService {
}

if (stranded.length || undetermined) {
this.logger?.warn?.('[approvals] stranded terminal requests (decision recorded, flow run gone)', {
// The two shapes are counted separately: they have different causes and
// different remedies, and an operator reading one number could not tell a
// pre-existing #4469 zombie from a run that failed mid-resume (#13909).
this.logger?.warn?.('[approvals] stranded terminal requests (decision recorded, flow run unrecoverable)', {
scanned: rows.length, stranded: stranded.length, undetermined,
requests: stranded.map(s => `${s.requestId}@${s.nodeId ?? '?'} → run ${s.runId}`),
runMissing: stranded.filter(s => s.runState === 'missing').length,
runFailed: stranded.filter(s => s.runState === 'failed').length,
requests: stranded.map(s => `${s.requestId}@${s.nodeId ?? '?'} → run ${s.runId} (${s.runState})`),
});
}
return { scanned: rows.length, stranded, undetermined };
Expand Down
2 changes: 2 additions & 0 deletions packages/plugins/plugin-approvals/src/index.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -25,6 +25,8 @@ export {
type ApprovalNodeAutoOutcome,
// #4469 — the read-only stranded-request inspection's report shape.
type StrandedApprovalRequest,
// #13909 — which unrecoverable shape a reported row is in.
type StrandedRunState,
} from './approval-service.js';
export {
ApprovalsServicePlugin,
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
fix(plugin-approvals): report a run that failed mid-resume as stranded, and measure the resume-ordering fork (#13909)#13934

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

os-steve
merged 4 commits into

from

Sep 1, 2026

+346

−12

 Merged


fix(plugin-approvals): report a run that failed mid-resume as stranded, and measure the resume-ordering fork (#13909)
#13934
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


4 commits


Select commit
Hold shift + click to select a range
8e2ec8f

fix(plugin-approvals): report a run that failed mid-resume as strande…


claude Aug 31, 2026
e138bf9

docs(permissions): re-anchor the system-context census rows this PR's…


claude Aug 31, 2026
d987bbb

docs(plugin-approvals): fix the runId field doc the widening falsifie…


claude Sep 1, 2026
d3f53f0

docs(permissions): re-anchor the 8 approval-service census rows the +…


claude Sep 1, 2026

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


48 changes: 48 additions & 0 deletions
.changeset/approvals-inspector-sees-failed-mid-resume.md

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
---
"@objectstack/plugin-approvals": patch
---

fix(plugin-approvals): the stranded-request inspection now sees a run that FAILED mid-resume (#13909)

`ApprovalService.inspectStrandedRequests` was structurally blind to the shape
#13909 owns, and reported `0` for it — the one shape an operator most needs to
see.

**The mechanism.** `AutomationEngine.resumeInternal` consumes the suspension
*before* running the downstream nodes: `forgetSuspendedRun(run, 'resumed')`
precedes `traverseNext`. A downstream node that merely THREW therefore threw
with the pause already gone, the catch arm recorded the run `failed`, and
nothing can resume it again (`resume` answers `RUN_NOT_FOUND`, `cancelRun` is a
no-op). The decision is durable and the flow stopped half-way.

**Why the inspection could not see it.** Its second oracle was
`if (terminal) continue` — the existence of ANY run-history row ended the check,
on the reading "the run ran to a terminal state, it is not dangling". But the
terminal row here is written BY the failure that stranded the request, so the
evidence of the defect was read as evidence of health. `releaseDeadRunRequests`
cannot see it either: it scans `status: 'pending'`, and the decision is what
took the row out of `pending`.

**The widening, and its limits.** The second oracle now classifies the run
instead of merely detecting it. A `failed` run is reported; `completed`,
`cancelled` and `paused` are each still skipped, one named reason at a time, and
a status this code does not recognise is skipped too — the spec's
`ExecutionStatus` vocabulary is wider than the four statuses the engine writes,
and a future status must not become a silent false positive. `paused` in
particular stays skipped because "the suspension is gone but no terminal row is
written yet" is exactly what a resume IN FLIGHT looks like. The first oracle is
unchanged: a run the suspension store still holds is alive, and an unreadable
store is still counted `undetermined`, never condemned.

Reported rows now carry `runState: 'missing' | 'failed'` (new exported type
`StrandedRunState`), because the two shapes need different remedies: a `missing`
run has no history to read, a `failed` one has a step log and an error naming
the node that threw. The sweep's own warning splits its counts the same way.
`StrandedApprovalRequest` is an output-only reporting shape the service
produces; the added field is not constructed by any caller in this repo.

**Still read-only, and still not a census.** No status is changed and no run is
cancelled — the decision genuinely happened. This makes the condition *visible*
in a deployment; how many runs are already in it can only be answered against
that deployment's own tables. Nothing here changes the resume ordering, which
is #13909's own next slice.

2 changes: 1 addition & 1 deletion
content/docs/permissions/system-context.mdx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -145,7 +145,7 @@ The largest single consumer — **20 of the 109 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:333` |
| 41 | Delegation write guard bypassed | plugin-approvals | Get: service / seed / import may write delegation rows naming another delegator | `lifecycle-hooks.ts:440` |
| 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:850`, `:959`, `:2916`, `:3062`, `:3229`, `:3300`, `:3489`, `:3529` |
| 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:931`, `:1040`, `:2997`, `:3143`, `:3310`, `:3381`, `:3570`, `:3610` |
| 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` |
Expand Down

138 changes: 128 additions & 10 deletions
packages/plugins/plugin-approvals/src/approval-service.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -247,6 +247,76 @@ const TERMINAL_RUN_STATUSES: ReadonlySet<string> = new Set([
 */
const STRANDABLE_REQUEST_STATUSES = ['approved', 'rejected', 'returned'] as const;

/**
 * The second oracle's verdict: which unrecoverable shape this run is in, or
 * `undefined` for every run that must NOT be reported (#13909).
 *
 * Written as an explicit switch, not `status !== 'completed'`, because the
 * negatives are the load-bearing half — a widening that reports everything the
 * old `if (terminal) continue` used to skip would bury the finding it exists to
 * surface. Each skip below is a distinct reason, and each is pinned by its own
 * test.
 *
 * The engine writes exactly four run statuses (`paused`, `completed`, `failed`,
 * `cancelled`); the spec's `ExecutionStatus` vocabulary is wider (`timed_out`,
 * `retrying`, …) and nothing in the engine writes the rest today. The `default`
 * arm therefore stays SILENT rather than reporting: a status this code does not
 * recognise is not evidence a decision was stranded, and condemning on it would
 * make every future status a false positive until someone noticed.
 */
function classifyStrandedRunState(run: { status?: string } | null | undefined): StrandedRunState | undefined {
 // No history row at all — the #4469 shape this inspection was built for.
 if (!run) return 'missing';
 switch (run.status) {
 // The resume consumed the pause and a downstream node threw. Reported.
 case 'failed':
 return 'failed';

 // ── The negatives, each for its own reason ──────────────────────────────
 // The decision advanced the flow and the flow finished. Healthy.
 case 'completed':
 return undefined;
 // Deliberately terminated by an operator (`cancelRun`, ADR-0044). The run
 // stopping is the intended outcome, exactly as `recalled` is on the request
 // side — reporting it would bury the real findings under expected ones.
 case 'cancelled':
 return undefined;
 // The history's last row says `paused` while the suspension store says no
 // live pause. That is AMBIGUOUS, and the ambiguity is not resolvable from
 // one scan: a resume in flight right now has consumed the suspension and
 // not yet written its terminal row, and reads exactly like a process that
 // died in the same window. Condemning it would name every concurrently
 // resuming approval — so this stays SKIPPED, the conservative arm this
 // whole method is built on.
 case 'paused':
 return undefined;
 default:
 return undefined;
 }
}

/**
 * WHY a terminal request's run is unrecoverable — the two shapes the inspection
 * reports, which have different causes and different remedies (#13909).
 *
 * - `missing` — `getRun` finds no history row at all (#4469's original shape):
 * the run was lost before it could record anything, typically a pause that
 * never reached a durable store and did not survive a restart.
 * - `failed` — the run DID record a terminal `failed` row. The engine consumes
 * a suspension *before* running the downstream nodes
 * (`AutomationEngine.resumeInternal`: `forgetSuspendedRun(run, 'resumed')`
 * precedes `traverseNext`), so a downstream node that merely THREW threw with
 * the pause already gone — the catch arm recorded `failed` and there is no
 * suspension left to resume. The decision is durable, the flow stopped
 * mid-continuation, and no verb moves the run out of that state.
 *
 * ⚠️ This names the shapes for the REPORT only. It is not a run state: the
 * engine's own vocabulary is still `'completed' | 'paused' | 'failed'`
 * (`AutomationResult.status`) and nothing persists or queries "stranded".
 * Giving the condition a platform-level name is #13909's own deliverable.
 */
export type StrandedRunState = 'missing' | 'failed';

/**
 * One terminal request whose owning flow run is unrecoverable (#4469) — the
 * decision was recorded and the flow never moved. Reporting shape only: the
Expand All@@ -257,8 +327,19 @@ export interface StrandedApprovalRequest {
 requestId: string;
 /** Terminal status the request reached — the decision that WAS recorded. */
 status: string;
 /** The `flow_run_id` that resolves to neither a suspension nor a run history row. */
 /**
 * The `flow_run_id` that resolves to no live suspension and no recoverable
 * run — see `runState`: no history row at all (`missing`), or a terminal
 * `failed` row (`failed`).
 */
 runId: string;
 /**
 * Which unrecoverable shape this is — see {@link StrandedRunState}. Carried
 * because the two need different remedies: a `missing` run has no history to
 * read, while a `failed` one has a step log and an error message naming the
 * node that threw.
 */
 runState: StrandedRunState;
 flowName?: string;
 /** Approval node the run should have continued from. */
 nodeId?: string;
Expand DownExpand Up@@ -3679,9 +3760,35 @@ export class ApprovalService implements IApprovalService {
 * live pause exists. It THROWS when the store cannot be read, and that
 * case is SKIPPED, never counted as dead: an unreadable store means
 * "unknown", and a storage outage must not be published as a lost run.
 * - `getRun(runId) == null` — no terminal history row either (the `run_`
 * prefixed rows in `sys_automation_run`). A run that merely finished is
 * not stranded; a request whose run neither waits nor ever completed is.
 * - `classifyStrandedRunState` over `getRun(runId)` — the run's own
 * history row (the `run_` prefixed rows in `sys_automation_run`). A run
 * that merely finished is not stranded; a request whose run neither waits
 * nor completed is.
 *
 * **The second oracle was widened (#13909), and this is the whole point of
 * that card's first slice.** It used to be `if (terminal) continue` — the
 * existence of ANY history row ended the check, on the reading "the run ran to
 * a terminal state, it is not dangling". That is true of a run that COMPLETED
 * and false of one that FAILED: the engine consumes a suspension *before*
 * running the downstream nodes (`AutomationEngine.resumeInternal` calls
 * `forgetSuspendedRun(run, 'resumed')` and only then `traverseNext`), so a
 * downstream node that merely threw threw with the pause already gone, and the
 * catch arm wrote a terminal `failed` row. The decision is durable, the
 * continuation stopped half-way, `resume` answers `RUN_NOT_FOUND` and
 * `cancelRun` is a no-op — and the terminal row this oracle used to read as
 * health is written BY the very failure that stranded it. So this inspection
 * reported `0` for the one shape an operator most needs to see.
 *
 * ⚠️ The widening does NOT reverse the conservatism: `completed`, `cancelled`
 * and `paused` are each still skipped, for reasons named one at a time in
 * `classifyStrandedRunState`, and an unrecognised status is skipped too.
 * What the widening buys is that a `failed` run is now reported with
 * `runState: 'failed'` instead of counted as healthy.
 *
 * ⚠️ **What this can and cannot size.** It makes the condition *visible* in a
 * deployment; it is not itself a census, and it says nothing about this
 * repository. How many runs are already in this state can only be answered
 * against a real deployment's tables — see the card.
 *
 * **Reports; never rewrites.** No status is changed and no run is cancelled.
 * The decision genuinely happened — a human approved or rejected — and
Expand DownExpand Up@@ -3748,10 +3855,15 @@ export class ApprovalService implements IApprovalService {
 });
 continue;
 }
 if (terminal) continue; // the run ran to a terminal state — it is not dangling

 // Neither suspended nor ever finished: the run this decision was supposed
 // to advance is genuinely gone.
 // #13909 — the widened verdict. `undefined` means "not a shape this
 // reports": healthy, deliberate, or unresolvable. See
 // `classifyStrandedRunState` for which, and why each one.
 const runState = classifyStrandedRunState(terminal);
 if (!runState) continue;

 // Neither suspended nor recoverable: the run this decision was supposed to
 // advance is gone (`missing`) or terminally failed mid-continuation with
 // its pause already consumed (`failed`).
 const config = parseJson<ApprovalNodeConfig>(
 raw.node_config_json, { approvers: [], behavior: 'first_response' } as any,
 );
Expand All@@ -3770,6 +3882,7 @@ export class ApprovalService implements IApprovalService {
 requestId: String(raw.id),
 status: raw.status,
 runId,
 runState,
 flowName: typeof raw.process_name === 'string' ? raw.process_name.replace(/^flow:/, '') : undefined,
 nodeId: raw.flow_node_id ?? raw.current_step ?? undefined,
 objectName: raw.object_name,
Expand All@@ -3782,9 +3895,14 @@ export class ApprovalService implements IApprovalService {
 }

 if (stranded.length || undetermined) {
 this.logger?.warn?.('[approvals] stranded terminal requests (decision recorded, flow run gone)', {
 // The two shapes are counted separately: they have different causes and
 // different remedies, and an operator reading one number could not tell a
 // pre-existing #4469 zombie from a run that failed mid-resume (#13909).
 this.logger?.warn?.('[approvals] stranded terminal requests (decision recorded, flow run unrecoverable)', {
 scanned: rows.length, stranded: stranded.length, undetermined,
 requests: stranded.map(s => `${s.requestId}@${s.nodeId ?? '?'} → run ${s.runId}`),
 runMissing: stranded.filter(s => s.runState === 'missing').length,
 runFailed: stranded.filter(s => s.runState === 'failed').length,
 requests: stranded.map(s => `${s.requestId}@${s.nodeId ?? '?'} → run ${s.runId} (${s.runState})`),
 });
 }
 return { scanned: rows.length, stranded, undetermined };
Expand Down

2 changes: 2 additions & 0 deletions
packages/plugins/plugin-approvals/src/index.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -25,6 +25,8 @@ export {
 type ApprovalNodeAutoOutcome,
 // #4469 — the read-only stranded-request inspection's report shape.
 type StrandedApprovalRequest,
 // #13909 — which unrecoverable shape a reported row is in.
 type StrandedRunState,
} from './approval-service.js';
export {
 ApprovalsServicePlugin,
Expand Down
Loading
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix(plugin-approvals): report a run that failed mid-resume as stranded, and measure the resume-ordering fork (#13909)#13934

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

os-steve
merged 4 commits into

from

Sep 1, 2026

+346

−12

 Merged


fix(plugin-approvals): report a run that failed mid-resume as stranded, and measure the resume-ordering fork (#13909)
#13934
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


4 commits


Select commit
Hold shift + click to select a range
8e2ec8f

fix(plugin-approvals): report a run that failed mid-resume as strande…


claude Aug 31, 2026
e138bf9

docs(permissions): re-anchor the system-context census rows this PR's…


claude Aug 31, 2026
d987bbb

docs(plugin-approvals): fix the runId field doc the widening falsifie…


claude Sep 1, 2026
d3f53f0

docs(permissions): re-anchor the 8 approval-service census rows the +…


claude Sep 1, 2026

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


48 changes: 48 additions & 0 deletions
.changeset/approvals-inspector-sees-failed-mid-resume.md

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
---
"@objectstack/plugin-approvals": patch
---

fix(plugin-approvals): the stranded-request inspection now sees a run that FAILED mid-resume (#13909)

`ApprovalService.inspectStrandedRequests` was structurally blind to the shape
#13909 owns, and reported `0` for it — the one shape an operator most needs to
see.

**The mechanism.** `AutomationEngine.resumeInternal` consumes the suspension
*before* running the downstream nodes: `forgetSuspendedRun(run, 'resumed')`
precedes `traverseNext`. A downstream node that merely THREW therefore threw
with the pause already gone, the catch arm recorded the run `failed`, and
nothing can resume it again (`resume` answers `RUN_NOT_FOUND`, `cancelRun` is a
no-op). The decision is durable and the flow stopped half-way.

**Why the inspection could not see it.** Its second oracle was
`if (terminal) continue` — the existence of ANY run-history row ended the check,
on the reading "the run ran to a terminal state, it is not dangling". But the
terminal row here is written BY the failure that stranded the request, so the
evidence of the defect was read as evidence of health. `releaseDeadRunRequests`
cannot see it either: it scans `status: 'pending'`, and the decision is what
took the row out of `pending`.

**The widening, and its limits.** The second oracle now classifies the run
instead of merely detecting it. A `failed` run is reported; `completed`,
`cancelled` and `paused` are each still skipped, one named reason at a time, and
a status this code does not recognise is skipped too — the spec's
`ExecutionStatus` vocabulary is wider than the four statuses the engine writes,
and a future status must not become a silent false positive. `paused` in
particular stays skipped because "the suspension is gone but no terminal row is
written yet" is exactly what a resume IN FLIGHT looks like. The first oracle is
unchanged: a run the suspension store still holds is alive, and an unreadable
store is still counted `undetermined`, never condemned.

Reported rows now carry `runState: 'missing' | 'failed'` (new exported type
`StrandedRunState`), because the two shapes need different remedies: a `missing`
run has no history to read, a `failed` one has a step log and an error naming
the node that threw. The sweep's own warning splits its counts the same way.
`StrandedApprovalRequest` is an output-only reporting shape the service
produces; the added field is not constructed by any caller in this repo.

**Still read-only, and still not a census.** No status is changed and no run is
cancelled — the decision genuinely happened. This makes the condition *visible*
in a deployment; how many runs are already in it can only be answered against
that deployment's own tables. Nothing here changes the resume ordering, which
is #13909's own next slice.

2 changes: 1 addition & 1 deletion
content/docs/permissions/system-context.mdx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -145,7 +145,7 @@ The largest single consumer — **20 of the 109 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:333` |
| 41 | Delegation write guard bypassed | plugin-approvals | Get: service / seed / import may write delegation rows naming another delegator | `lifecycle-hooks.ts:440` |
| 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:850`, `:959`, `:2916`, `:3062`, `:3229`, `:3300`, `:3489`, `:3529` |
| 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:931`, `:1040`, `:2997`, `:3143`, `:3310`, `:3381`, `:3570`, `:3610` |
| 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` |
Expand Down

138 changes: 128 additions & 10 deletions
packages/plugins/plugin-approvals/src/approval-service.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -247,6 +247,76 @@ const TERMINAL_RUN_STATUSES: ReadonlySet<string> = new Set([
 */
const STRANDABLE_REQUEST_STATUSES = ['approved', 'rejected', 'returned'] as const;

/**
 * The second oracle's verdict: which unrecoverable shape this run is in, or
 * `undefined` for every run that must NOT be reported (#13909).
 *
 * Written as an explicit switch, not `status !== 'completed'`, because the
 * negatives are the load-bearing half — a widening that reports everything the
 * old `if (terminal) continue` used to skip would bury the finding it exists to
 * surface. Each skip below is a distinct reason, and each is pinned by its own
 * test.
 *
 * The engine writes exactly four run statuses (`paused`, `completed`, `failed`,
 * `cancelled`); the spec's `ExecutionStatus` vocabulary is wider (`timed_out`,
 * `retrying`, …) and nothing in the engine writes the rest today. The `default`
 * arm therefore stays SILENT rather than reporting: a status this code does not
 * recognise is not evidence a decision was stranded, and condemning on it would
 * make every future status a false positive until someone noticed.
 */
function classifyStrandedRunState(run: { status?: string } | null | undefined): StrandedRunState | undefined {
 // No history row at all — the #4469 shape this inspection was built for.
 if (!run) return 'missing';
 switch (run.status) {
 // The resume consumed the pause and a downstream node threw. Reported.
 case 'failed':
 return 'failed';

 // ── The negatives, each for its own reason ──────────────────────────────
 // The decision advanced the flow and the flow finished. Healthy.
 case 'completed':
 return undefined;
 // Deliberately terminated by an operator (`cancelRun`, ADR-0044). The run
 // stopping is the intended outcome, exactly as `recalled` is on the request
 // side — reporting it would bury the real findings under expected ones.
 case 'cancelled':
 return undefined;
 // The history's last row says `paused` while the suspension store says no
 // live pause. That is AMBIGUOUS, and the ambiguity is not resolvable from
 // one scan: a resume in flight right now has consumed the suspension and
 // not yet written its terminal row, and reads exactly like a process that
 // died in the same window. Condemning it would name every concurrently
 // resuming approval — so this stays SKIPPED, the conservative arm this
 // whole method is built on.
 case 'paused':
 return undefined;
 default:
 return undefined;
 }
}

/**
 * WHY a terminal request's run is unrecoverable — the two shapes the inspection
 * reports, which have different causes and different remedies (#13909).
 *
 * - `missing` — `getRun` finds no history row at all (#4469's original shape):
 * the run was lost before it could record anything, typically a pause that
 * never reached a durable store and did not survive a restart.
 * - `failed` — the run DID record a terminal `failed` row. The engine consumes
 * a suspension *before* running the downstream nodes
 * (`AutomationEngine.resumeInternal`: `forgetSuspendedRun(run, 'resumed')`
 * precedes `traverseNext`), so a downstream node that merely THREW threw with
 * the pause already gone — the catch arm recorded `failed` and there is no
 * suspension left to resume. The decision is durable, the flow stopped
 * mid-continuation, and no verb moves the run out of that state.
 *
 * ⚠️ This names the shapes for the REPORT only. It is not a run state: the
 * engine's own vocabulary is still `'completed' | 'paused' | 'failed'`
 * (`AutomationResult.status`) and nothing persists or queries "stranded".
 * Giving the condition a platform-level name is #13909's own deliverable.
 */
export type StrandedRunState = 'missing' | 'failed';

/**
 * One terminal request whose owning flow run is unrecoverable (#4469) — the
 * decision was recorded and the flow never moved. Reporting shape only: the
Expand All@@ -257,8 +327,19 @@ export interface StrandedApprovalRequest {
 requestId: string;
 /** Terminal status the request reached — the decision that WAS recorded. */
 status: string;
 /** The `flow_run_id` that resolves to neither a suspension nor a run history row. */
 /**
 * The `flow_run_id` that resolves to no live suspension and no recoverable
 * run — see `runState`: no history row at all (`missing`), or a terminal
 * `failed` row (`failed`).
 */
 runId: string;
 /**
 * Which unrecoverable shape this is — see {@link StrandedRunState}. Carried
 * because the two need different remedies: a `missing` run has no history to
 * read, while a `failed` one has a step log and an error message naming the
 * node that threw.
 */
 runState: StrandedRunState;
 flowName?: string;
 /** Approval node the run should have continued from. */
 nodeId?: string;
Expand DownExpand Up@@ -3679,9 +3760,35 @@ export class ApprovalService implements IApprovalService {
 * live pause exists. It THROWS when the store cannot be read, and that
 * case is SKIPPED, never counted as dead: an unreadable store means
 * "unknown", and a storage outage must not be published as a lost run.
 * - `getRun(runId) == null` — no terminal history row either (the `run_`
 * prefixed rows in `sys_automation_run`). A run that merely finished is
 * not stranded; a request whose run neither waits nor ever completed is.
 * - `classifyStrandedRunState` over `getRun(runId)` — the run's own
 * history row (the `run_` prefixed rows in `sys_automation_run`). A run
 * that merely finished is not stranded; a request whose run neither waits
 * nor completed is.
 *
 * **The second oracle was widened (#13909), and this is the whole point of
 * that card's first slice.** It used to be `if (terminal) continue` — the
 * existence of ANY history row ended the check, on the reading "the run ran to
 * a terminal state, it is not dangling". That is true of a run that COMPLETED
 * and false of one that FAILED: the engine consumes a suspension *before*
 * running the downstream nodes (`AutomationEngine.resumeInternal` calls
 * `forgetSuspendedRun(run, 'resumed')` and only then `traverseNext`), so a
 * downstream node that merely threw threw with the pause already gone, and the
 * catch arm wrote a terminal `failed` row. The decision is durable, the
 * continuation stopped half-way, `resume` answers `RUN_NOT_FOUND` and
 * `cancelRun` is a no-op — and the terminal row this oracle used to read as
 * health is written BY the very failure that stranded it. So this inspection
 * reported `0` for the one shape an operator most needs to see.
 *
 * ⚠️ The widening does NOT reverse the conservatism: `completed`, `cancelled`
 * and `paused` are each still skipped, for reasons named one at a time in
 * `classifyStrandedRunState`, and an unrecognised status is skipped too.
 * What the widening buys is that a `failed` run is now reported with
 * `runState: 'failed'` instead of counted as healthy.
 *
 * ⚠️ **What this can and cannot size.** It makes the condition *visible* in a
 * deployment; it is not itself a census, and it says nothing about this
 * repository. How many runs are already in this state can only be answered
 * against a real deployment's tables — see the card.
 *
 * **Reports; never rewrites.** No status is changed and no run is cancelled.
 * The decision genuinely happened — a human approved or rejected — and
Expand DownExpand Up@@ -3748,10 +3855,15 @@ export class ApprovalService implements IApprovalService {
 });
 continue;
 }
 if (terminal) continue; // the run ran to a terminal state — it is not dangling

 // Neither suspended nor ever finished: the run this decision was supposed
 // to advance is genuinely gone.
 // #13909 — the widened verdict. `undefined` means "not a shape this
 // reports": healthy, deliberate, or unresolvable. See
 // `classifyStrandedRunState` for which, and why each one.
 const runState = classifyStrandedRunState(terminal);
 if (!runState) continue;

 // Neither suspended nor recoverable: the run this decision was supposed to
 // advance is gone (`missing`) or terminally failed mid-continuation with
 // its pause already consumed (`failed`).
 const config = parseJson<ApprovalNodeConfig>(
 raw.node_config_json, { approvers: [], behavior: 'first_response' } as any,
 );
Expand All@@ -3770,6 +3882,7 @@ export class ApprovalService implements IApprovalService {
 requestId: String(raw.id),
 status: raw.status,
 runId,
 runState,
 flowName: typeof raw.process_name === 'string' ? raw.process_name.replace(/^flow:/, '') : undefined,
 nodeId: raw.flow_node_id ?? raw.current_step ?? undefined,
 objectName: raw.object_name,
Expand All@@ -3782,9 +3895,14 @@ export class ApprovalService implements IApprovalService {
 }

 if (stranded.length || undetermined) {
 this.logger?.warn?.('[approvals] stranded terminal requests (decision recorded, flow run gone)', {
 // The two shapes are counted separately: they have different causes and
 // different remedies, and an operator reading one number could not tell a
 // pre-existing #4469 zombie from a run that failed mid-resume (#13909).
 this.logger?.warn?.('[approvals] stranded terminal requests (decision recorded, flow run unrecoverable)', {
 scanned: rows.length, stranded: stranded.length, undetermined,
 requests: stranded.map(s => `${s.requestId}@${s.nodeId ?? '?'} → run ${s.runId}`),
 runMissing: stranded.filter(s => s.runState === 'missing').length,
 runFailed: stranded.filter(s => s.runState === 'failed').length,
 requests: stranded.map(s => `${s.requestId}@${s.nodeId ?? '?'} → run ${s.runId} (${s.runState})`),
 });
 }
 return { scanned: rows.length, stranded, undetermined };
Expand Down

2 changes: 2 additions & 0 deletions
packages/plugins/plugin-approvals/src/index.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -25,6 +25,8 @@ export {
 type ApprovalNodeAutoOutcome,
 // #4469 — the read-only stranded-request inspection's report shape.
 type StrandedApprovalRequest,
 // #13909 — which unrecoverable shape a reported row is in.
 type StrandedRunState,
} from './approval-service.js';
export {
 ApprovalsServicePlugin,
Expand Down
Loading
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix(plugin-approvals): report a run that failed mid-resume as stranded, and measure the resume-ordering fork (#13909)#13934

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

os-steve
merged 4 commits into

from

Sep 1, 2026

+346

−12

 Merged


fix(plugin-approvals): report a run that failed mid-resume as stranded, and measure the resume-ordering fork (#13909)
#13934
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


4 commits


Select commit
Hold shift + click to select a range
8e2ec8f

fix(plugin-approvals): report a run that failed mid-resume as strande…


claude Aug 31, 2026
e138bf9

docs(permissions): re-anchor the system-context census rows this PR's…


claude Aug 31, 2026
d987bbb

docs(plugin-approvals): fix the runId field doc the widening falsifie…


claude Sep 1, 2026
d3f53f0

docs(permissions): re-anchor the 8 approval-service census rows the +…


claude Sep 1, 2026

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


48 changes: 48 additions & 0 deletions
.changeset/approvals-inspector-sees-failed-mid-resume.md

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
---
"@objectstack/plugin-approvals": patch
---

fix(plugin-approvals): the stranded-request inspection now sees a run that FAILED mid-resume (#13909)

`ApprovalService.inspectStrandedRequests` was structurally blind to the shape
#13909 owns, and reported `0` for it — the one shape an operator most needs to
see.

**The mechanism.** `AutomationEngine.resumeInternal` consumes the suspension
*before* running the downstream nodes: `forgetSuspendedRun(run, 'resumed')`
precedes `traverseNext`. A downstream node that merely THREW therefore threw
with the pause already gone, the catch arm recorded the run `failed`, and
nothing can resume it again (`resume` answers `RUN_NOT_FOUND`, `cancelRun` is a
no-op). The decision is durable and the flow stopped half-way.

**Why the inspection could not see it.** Its second oracle was
`if (terminal) continue` — the existence of ANY run-history row ended the check,
on the reading "the run ran to a terminal state, it is not dangling". But the
terminal row here is written BY the failure that stranded the request, so the
evidence of the defect was read as evidence of health. `releaseDeadRunRequests`
cannot see it either: it scans `status: 'pending'`, and the decision is what
took the row out of `pending`.

**The widening, and its limits.** The second oracle now classifies the run
instead of merely detecting it. A `failed` run is reported; `completed`,
`cancelled` and `paused` are each still skipped, one named reason at a time, and
a status this code does not recognise is skipped too — the spec's
`ExecutionStatus` vocabulary is wider than the four statuses the engine writes,
and a future status must not become a silent false positive. `paused` in
particular stays skipped because "the suspension is gone but no terminal row is
written yet" is exactly what a resume IN FLIGHT looks like. The first oracle is
unchanged: a run the suspension store still holds is alive, and an unreadable
store is still counted `undetermined`, never condemned.

Reported rows now carry `runState: 'missing' | 'failed'` (new exported type
`StrandedRunState`), because the two shapes need different remedies: a `missing`
run has no history to read, a `failed` one has a step log and an error naming
the node that threw. The sweep's own warning splits its counts the same way.
`StrandedApprovalRequest` is an output-only reporting shape the service
produces; the added field is not constructed by any caller in this repo.

**Still read-only, and still not a census.** No status is changed and no run is
cancelled — the decision genuinely happened. This makes the condition *visible*
in a deployment; how many runs are already in it can only be answered against
that deployment's own tables. Nothing here changes the resume ordering, which
is #13909's own next slice.

2 changes: 1 addition & 1 deletion
content/docs/permissions/system-context.mdx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -145,7 +145,7 @@ The largest single consumer — **20 of the 109 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:333` |
| 41 | Delegation write guard bypassed | plugin-approvals | Get: service / seed / import may write delegation rows naming another delegator | `lifecycle-hooks.ts:440` |
| 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:850`, `:959`, `:2916`, `:3062`, `:3229`, `:3300`, `:3489`, `:3529` |
| 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:931`, `:1040`, `:2997`, `:3143`, `:3310`, `:3381`, `:3570`, `:3610` |
| 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` |
Expand Down

138 changes: 128 additions & 10 deletions
packages/plugins/plugin-approvals/src/approval-service.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -247,6 +247,76 @@ const TERMINAL_RUN_STATUSES: ReadonlySet<string> = new Set([
 */
const STRANDABLE_REQUEST_STATUSES = ['approved', 'rejected', 'returned'] as const;

/**
 * The second oracle's verdict: which unrecoverable shape this run is in, or
 * `undefined` for every run that must NOT be reported (#13909).
 *
 * Written as an explicit switch, not `status !== 'completed'`, because the
 * negatives are the load-bearing half — a widening that reports everything the
 * old `if (terminal) continue` used to skip would bury the finding it exists to
 * surface. Each skip below is a distinct reason, and each is pinned by its own
 * test.
 *
 * The engine writes exactly four run statuses (`paused`, `completed`, `failed`,
 * `cancelled`); the spec's `ExecutionStatus` vocabulary is wider (`timed_out`,
 * `retrying`, …) and nothing in the engine writes the rest today. The `default`
 * arm therefore stays SILENT rather than reporting: a status this code does not
 * recognise is not evidence a decision was stranded, and condemning on it would
 * make every future status a false positive until someone noticed.
 */
function classifyStrandedRunState(run: { status?: string } | null | undefined): StrandedRunState | undefined {
 // No history row at all — the #4469 shape this inspection was built for.
 if (!run) return 'missing';
 switch (run.status) {
 // The resume consumed the pause and a downstream node threw. Reported.
 case 'failed':
 return 'failed';

 // ── The negatives, each for its own reason ──────────────────────────────
 // The decision advanced the flow and the flow finished. Healthy.
 case 'completed':
 return undefined;
 // Deliberately terminated by an operator (`cancelRun`, ADR-0044). The run
 // stopping is the intended outcome, exactly as `recalled` is on the request
 // side — reporting it would bury the real findings under expected ones.
 case 'cancelled':
 return undefined;
 // The history's last row says `paused` while the suspension store says no
 // live pause. That is AMBIGUOUS, and the ambiguity is not resolvable from
 // one scan: a resume in flight right now has consumed the suspension and
 // not yet written its terminal row, and reads exactly like a process that
 // died in the same window. Condemning it would name every concurrently
 // resuming approval — so this stays SKIPPED, the conservative arm this
 // whole method is built on.
 case 'paused':
 return undefined;
 default:
 return undefined;
 }
}

/**
 * WHY a terminal request's run is unrecoverable — the two shapes the inspection
 * reports, which have different causes and different remedies (#13909).
 *
 * - `missing` — `getRun` finds no history row at all (#4469's original shape):
 * the run was lost before it could record anything, typically a pause that
 * never reached a durable store and did not survive a restart.
 * - `failed` — the run DID record a terminal `failed` row. The engine consumes
 * a suspension *before* running the downstream nodes
 * (`AutomationEngine.resumeInternal`: `forgetSuspendedRun(run, 'resumed')`
 * precedes `traverseNext`), so a downstream node that merely THREW threw with
 * the pause already gone — the catch arm recorded `failed` and there is no
 * suspension left to resume. The decision is durable, the flow stopped
 * mid-continuation, and no verb moves the run out of that state.
 *
 * ⚠️ This names the shapes for the REPORT only. It is not a run state: the
 * engine's own vocabulary is still `'completed' | 'paused' | 'failed'`
 * (`AutomationResult.status`) and nothing persists or queries "stranded".
 * Giving the condition a platform-level name is #13909's own deliverable.
 */
export type StrandedRunState = 'missing' | 'failed';

/**
 * One terminal request whose owning flow run is unrecoverable (#4469) — the
 * decision was recorded and the flow never moved. Reporting shape only: the
Expand All@@ -257,8 +327,19 @@ export interface StrandedApprovalRequest {
 requestId: string;
 /** Terminal status the request reached — the decision that WAS recorded. */
 status: string;
 /** The `flow_run_id` that resolves to neither a suspension nor a run history row. */
 /**
 * The `flow_run_id` that resolves to no live suspension and no recoverable
 * run — see `runState`: no history row at all (`missing`), or a terminal
 * `failed` row (`failed`).
 */
 runId: string;
 /**
 * Which unrecoverable shape this is — see {@link StrandedRunState}. Carried
 * because the two need different remedies: a `missing` run has no history to
 * read, while a `failed` one has a step log and an error message naming the
 * node that threw.
 */
 runState: StrandedRunState;
 flowName?: string;
 /** Approval node the run should have continued from. */
 nodeId?: string;
Expand DownExpand Up@@ -3679,9 +3760,35 @@ export class ApprovalService implements IApprovalService {
 * live pause exists. It THROWS when the store cannot be read, and that
 * case is SKIPPED, never counted as dead: an unreadable store means
 * "unknown", and a storage outage must not be published as a lost run.
 * - `getRun(runId) == null` — no terminal history row either (the `run_`
 * prefixed rows in `sys_automation_run`). A run that merely finished is
 * not stranded; a request whose run neither waits nor ever completed is.
 * - `classifyStrandedRunState` over `getRun(runId)` — the run's own
 * history row (the `run_` prefixed rows in `sys_automation_run`). A run
 * that merely finished is not stranded; a request whose run neither waits
 * nor completed is.
 *
 * **The second oracle was widened (#13909), and this is the whole point of
 * that card's first slice.** It used to be `if (terminal) continue` — the
 * existence of ANY history row ended the check, on the reading "the run ran to
 * a terminal state, it is not dangling". That is true of a run that COMPLETED
 * and false of one that FAILED: the engine consumes a suspension *before*
 * running the downstream nodes (`AutomationEngine.resumeInternal` calls
 * `forgetSuspendedRun(run, 'resumed')` and only then `traverseNext`), so a
 * downstream node that merely threw threw with the pause already gone, and the
 * catch arm wrote a terminal `failed` row. The decision is durable, the
 * continuation stopped half-way, `resume` answers `RUN_NOT_FOUND` and
 * `cancelRun` is a no-op — and the terminal row this oracle used to read as
 * health is written BY the very failure that stranded it. So this inspection
 * reported `0` for the one shape an operator most needs to see.
 *
 * ⚠️ The widening does NOT reverse the conservatism: `completed`, `cancelled`
 * and `paused` are each still skipped, for reasons named one at a time in
 * `classifyStrandedRunState`, and an unrecognised status is skipped too.
 * What the widening buys is that a `failed` run is now reported with
 * `runState: 'failed'` instead of counted as healthy.
 *
 * ⚠️ **What this can and cannot size.** It makes the condition *visible* in a
 * deployment; it is not itself a census, and it says nothing about this
 * repository. How many runs are already in this state can only be answered
 * against a real deployment's tables — see the card.
 *
 * **Reports; never rewrites.** No status is changed and no run is cancelled.
 * The decision genuinely happened — a human approved or rejected — and
Expand DownExpand Up@@ -3748,10 +3855,15 @@ export class ApprovalService implements IApprovalService {
 });
 continue;
 }
 if (terminal) continue; // the run ran to a terminal state — it is not dangling

 // Neither suspended nor ever finished: the run this decision was supposed
 // to advance is genuinely gone.
 // #13909 — the widened verdict. `undefined` means "not a shape this
 // reports": healthy, deliberate, or unresolvable. See
 // `classifyStrandedRunState` for which, and why each one.
 const runState = classifyStrandedRunState(terminal);
 if (!runState) continue;

 // Neither suspended nor recoverable: the run this decision was supposed to
 // advance is gone (`missing`) or terminally failed mid-continuation with
 // its pause already consumed (`failed`).
 const config = parseJson<ApprovalNodeConfig>(
 raw.node_config_json, { approvers: [], behavior: 'first_response' } as any,
 );
Expand All@@ -3770,6 +3882,7 @@ export class ApprovalService implements IApprovalService {
 requestId: String(raw.id),
 status: raw.status,
 runId,
 runState,
 flowName: typeof raw.process_name === 'string' ? raw.process_name.replace(/^flow:/, '') : undefined,
 nodeId: raw.flow_node_id ?? raw.current_step ?? undefined,
 objectName: raw.object_name,
Expand All@@ -3782,9 +3895,14 @@ export class ApprovalService implements IApprovalService {
 }

 if (stranded.length || undetermined) {
 this.logger?.warn?.('[approvals] stranded terminal requests (decision recorded, flow run gone)', {
 // The two shapes are counted separately: they have different causes and
 // different remedies, and an operator reading one number could not tell a
 // pre-existing #4469 zombie from a run that failed mid-resume (#13909).
 this.logger?.warn?.('[approvals] stranded terminal requests (decision recorded, flow run unrecoverable)', {
 scanned: rows.length, stranded: stranded.length, undetermined,
 requests: stranded.map(s => `${s.requestId}@${s.nodeId ?? '?'} → run ${s.runId}`),
 runMissing: stranded.filter(s => s.runState === 'missing').length,
 runFailed: stranded.filter(s => s.runState === 'failed').length,
 requests: stranded.map(s => `${s.requestId}@${s.nodeId ?? '?'} → run ${s.runId} (${s.runState})`),
 });
 }
 return { scanned: rows.length, stranded, undetermined };
Expand Down

2 changes: 2 additions & 0 deletions
packages/plugins/plugin-approvals/src/index.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -25,6 +25,8 @@ export {
 type ApprovalNodeAutoOutcome,
 // #4469 — the read-only stranded-request inspection's report shape.
 type StrandedApprovalRequest,
 // #13909 — which unrecoverable shape a reported row is in.
 type StrandedRunState,
} from './approval-service.js';
export {
 ApprovalsServicePlugin,
Expand Down
Loading
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
fix(plugin-approvals): report a run that failed mid-resume as stranded, and measure the resume-ordering fork (#13909)#13934

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

os-steve
merged 4 commits into

from

Sep 1, 2026

+346

−12

 Merged


fix(plugin-approvals): report a run that failed mid-resume as stranded, and measure the resume-ordering fork (#13909)
#13934
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


4 commits


Select commit
Hold shift + click to select a range
8e2ec8f

fix(plugin-approvals): report a run that failed mid-resume as strande…


claude Aug 31, 2026
e138bf9

docs(permissions): re-anchor the system-context census rows this PR's…


claude Aug 31, 2026
d987bbb

docs(plugin-approvals): fix the runId field doc the widening falsifie…


claude Sep 1, 2026
d3f53f0

docs(permissions): re-anchor the 8 approval-service census rows the +…


claude Sep 1, 2026

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


48 changes: 48 additions & 0 deletions
.changeset/approvals-inspector-sees-failed-mid-resume.md

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
---
"@objectstack/plugin-approvals": patch
---

fix(plugin-approvals): the stranded-request inspection now sees a run that FAILED mid-resume (#13909)

`ApprovalService.inspectStrandedRequests` was structurally blind to the shape
#13909 owns, and reported `0` for it — the one shape an operator most needs to
see.

**The mechanism.** `AutomationEngine.resumeInternal` consumes the suspension
*before* running the downstream nodes: `forgetSuspendedRun(run, 'resumed')`
precedes `traverseNext`. A downstream node that merely THREW therefore threw
with the pause already gone, the catch arm recorded the run `failed`, and
nothing can resume it again (`resume` answers `RUN_NOT_FOUND`, `cancelRun` is a
no-op). The decision is durable and the flow stopped half-way.

**Why the inspection could not see it.** Its second oracle was
`if (terminal) continue` — the existence of ANY run-history row ended the check,
on the reading "the run ran to a terminal state, it is not dangling". But the
terminal row here is written BY the failure that stranded the request, so the
evidence of the defect was read as evidence of health. `releaseDeadRunRequests`
cannot see it either: it scans `status: 'pending'`, and the decision is what
took the row out of `pending`.

**The widening, and its limits.** The second oracle now classifies the run
instead of merely detecting it. A `failed` run is reported; `completed`,
`cancelled` and `paused` are each still skipped, one named reason at a time, and
a status this code does not recognise is skipped too — the spec's
`ExecutionStatus` vocabulary is wider than the four statuses the engine writes,
and a future status must not become a silent false positive. `paused` in
particular stays skipped because "the suspension is gone but no terminal row is
written yet" is exactly what a resume IN FLIGHT looks like. The first oracle is
unchanged: a run the suspension store still holds is alive, and an unreadable
store is still counted `undetermined`, never condemned.

Reported rows now carry `runState: 'missing' | 'failed'` (new exported type
`StrandedRunState`), because the two shapes need different remedies: a `missing`
run has no history to read, a `failed` one has a step log and an error naming
the node that threw. The sweep's own warning splits its counts the same way.
`StrandedApprovalRequest` is an output-only reporting shape the service
produces; the added field is not constructed by any caller in this repo.

**Still read-only, and still not a census.** No status is changed and no run is
cancelled — the decision genuinely happened. This makes the condition *visible*
in a deployment; how many runs are already in it can only be answered against
that deployment's own tables. Nothing here changes the resume ordering, which
is #13909's own next slice.

2 changes: 1 addition & 1 deletion
content/docs/permissions/system-context.mdx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -145,7 +145,7 @@ The largest single consumer — **20 of the 109 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:333` |
| 41 | Delegation write guard bypassed | plugin-approvals | Get: service / seed / import may write delegation rows naming another delegator | `lifecycle-hooks.ts:440` |
| 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:850`, `:959`, `:2916`, `:3062`, `:3229`, `:3300`, `:3489`, `:3529` |
| 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:931`, `:1040`, `:2997`, `:3143`, `:3310`, `:3381`, `:3570`, `:3610` |
| 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` |
Expand Down

138 changes: 128 additions & 10 deletions
packages/plugins/plugin-approvals/src/approval-service.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -247,6 +247,76 @@ const TERMINAL_RUN_STATUSES: ReadonlySet<string> = new Set([
 */
const STRANDABLE_REQUEST_STATUSES = ['approved', 'rejected', 'returned'] as const;

/**
 * The second oracle's verdict: which unrecoverable shape this run is in, or
 * `undefined` for every run that must NOT be reported (#13909).
 *
 * Written as an explicit switch, not `status !== 'completed'`, because the
 * negatives are the load-bearing half — a widening that reports everything the
 * old `if (terminal) continue` used to skip would bury the finding it exists to
 * surface. Each skip below is a distinct reason, and each is pinned by its own
 * test.
 *
 * The engine writes exactly four run statuses (`paused`, `completed`, `failed`,
 * `cancelled`); the spec's `ExecutionStatus` vocabulary is wider (`timed_out`,
 * `retrying`, …) and nothing in the engine writes the rest today. The `default`
 * arm therefore stays SILENT rather than reporting: a status this code does not
 * recognise is not evidence a decision was stranded, and condemning on it would
 * make every future status a false positive until someone noticed.
 */
function classifyStrandedRunState(run: { status?: string } | null | undefined): StrandedRunState | undefined {
 // No history row at all — the #4469 shape this inspection was built for.
 if (!run) return 'missing';
 switch (run.status) {
 // The resume consumed the pause and a downstream node threw. Reported.
 case 'failed':
 return 'failed';

 // ── The negatives, each for its own reason ──────────────────────────────
 // The decision advanced the flow and the flow finished. Healthy.
 case 'completed':
 return undefined;
 // Deliberately terminated by an operator (`cancelRun`, ADR-0044). The run
 // stopping is the intended outcome, exactly as `recalled` is on the request
 // side — reporting it would bury the real findings under expected ones.
 case 'cancelled':
 return undefined;
 // The history's last row says `paused` while the suspension store says no
 // live pause. That is AMBIGUOUS, and the ambiguity is not resolvable from
 // one scan: a resume in flight right now has consumed the suspension and
 // not yet written its terminal row, and reads exactly like a process that
 // died in the same window. Condemning it would name every concurrently
 // resuming approval — so this stays SKIPPED, the conservative arm this
 // whole method is built on.
 case 'paused':
 return undefined;
 default:
 return undefined;
 }
}

/**
 * WHY a terminal request's run is unrecoverable — the two shapes the inspection
 * reports, which have different causes and different remedies (#13909).
 *
 * - `missing` — `getRun` finds no history row at all (#4469's original shape):
 * the run was lost before it could record anything, typically a pause that
 * never reached a durable store and did not survive a restart.
 * - `failed` — the run DID record a terminal `failed` row. The engine consumes
 * a suspension *before* running the downstream nodes
 * (`AutomationEngine.resumeInternal`: `forgetSuspendedRun(run, 'resumed')`
 * precedes `traverseNext`), so a downstream node that merely THREW threw with
 * the pause already gone — the catch arm recorded `failed` and there is no
 * suspension left to resume. The decision is durable, the flow stopped
 * mid-continuation, and no verb moves the run out of that state.
 *
 * ⚠️ This names the shapes for the REPORT only. It is not a run state: the
 * engine's own vocabulary is still `'completed' | 'paused' | 'failed'`
 * (`AutomationResult.status`) and nothing persists or queries "stranded".
 * Giving the condition a platform-level name is #13909's own deliverable.
 */
export type StrandedRunState = 'missing' | 'failed';

/**
 * One terminal request whose owning flow run is unrecoverable (#4469) — the
 * decision was recorded and the flow never moved. Reporting shape only: the
Expand All@@ -257,8 +327,19 @@ export interface StrandedApprovalRequest {
 requestId: string;
 /** Terminal status the request reached — the decision that WAS recorded. */
 status: string;
 /** The `flow_run_id` that resolves to neither a suspension nor a run history row. */
 /**
 * The `flow_run_id` that resolves to no live suspension and no recoverable
 * run — see `runState`: no history row at all (`missing`), or a terminal
 * `failed` row (`failed`).
 */
 runId: string;
 /**
 * Which unrecoverable shape this is — see {@link StrandedRunState}. Carried
 * because the two need different remedies: a `missing` run has no history to
 * read, while a `failed` one has a step log and an error message naming the
 * node that threw.
 */
 runState: StrandedRunState;
 flowName?: string;
 /** Approval node the run should have continued from. */
 nodeId?: string;
Expand DownExpand Up@@ -3679,9 +3760,35 @@ export class ApprovalService implements IApprovalService {
 * live pause exists. It THROWS when the store cannot be read, and that
 * case is SKIPPED, never counted as dead: an unreadable store means
 * "unknown", and a storage outage must not be published as a lost run.
 * - `getRun(runId) == null` — no terminal history row either (the `run_`
 * prefixed rows in `sys_automation_run`). A run that merely finished is
 * not stranded; a request whose run neither waits nor ever completed is.
 * - `classifyStrandedRunState` over `getRun(runId)` — the run's own
 * history row (the `run_` prefixed rows in `sys_automation_run`). A run
 * that merely finished is not stranded; a request whose run neither waits
 * nor completed is.
 *
 * **The second oracle was widened (#13909), and this is the whole point of
 * that card's first slice.** It used to be `if (terminal) continue` — the
 * existence of ANY history row ended the check, on the reading "the run ran to
 * a terminal state, it is not dangling". That is true of a run that COMPLETED
 * and false of one that FAILED: the engine consumes a suspension *before*
 * running the downstream nodes (`AutomationEngine.resumeInternal` calls
 * `forgetSuspendedRun(run, 'resumed')` and only then `traverseNext`), so a
 * downstream node that merely threw threw with the pause already gone, and the
 * catch arm wrote a terminal `failed` row. The decision is durable, the
 * continuation stopped half-way, `resume` answers `RUN_NOT_FOUND` and
 * `cancelRun` is a no-op — and the terminal row this oracle used to read as
 * health is written BY the very failure that stranded it. So this inspection
 * reported `0` for the one shape an operator most needs to see.
 *
 * ⚠️ The widening does NOT reverse the conservatism: `completed`, `cancelled`
 * and `paused` are each still skipped, for reasons named one at a time in
 * `classifyStrandedRunState`, and an unrecognised status is skipped too.
 * What the widening buys is that a `failed` run is now reported with
 * `runState: 'failed'` instead of counted as healthy.
 *
 * ⚠️ **What this can and cannot size.** It makes the condition *visible* in a
 * deployment; it is not itself a census, and it says nothing about this
 * repository. How many runs are already in this state can only be answered
 * against a real deployment's tables — see the card.
 *
 * **Reports; never rewrites.** No status is changed and no run is cancelled.
 * The decision genuinely happened — a human approved or rejected — and
Expand DownExpand Up@@ -3748,10 +3855,15 @@ export class ApprovalService implements IApprovalService {
 });
 continue;
 }
 if (terminal) continue; // the run ran to a terminal state — it is not dangling

 // Neither suspended nor ever finished: the run this decision was supposed
 // to advance is genuinely gone.
 // #13909 — the widened verdict. `undefined` means "not a shape this
 // reports": healthy, deliberate, or unresolvable. See
 // `classifyStrandedRunState` for which, and why each one.
 const runState = classifyStrandedRunState(terminal);
 if (!runState) continue;

 // Neither suspended nor recoverable: the run this decision was supposed to
 // advance is gone (`missing`) or terminally failed mid-continuation with
 // its pause already consumed (`failed`).
 const config = parseJson<ApprovalNodeConfig>(
 raw.node_config_json, { approvers: [], behavior: 'first_response' } as any,
 );
Expand All@@ -3770,6 +3882,7 @@ export class ApprovalService implements IApprovalService {
 requestId: String(raw.id),
 status: raw.status,
 runId,
 runState,
 flowName: typeof raw.process_name === 'string' ? raw.process_name.replace(/^flow:/, '') : undefined,
 nodeId: raw.flow_node_id ?? raw.current_step ?? undefined,
 objectName: raw.object_name,
Expand All@@ -3782,9 +3895,14 @@ export class ApprovalService implements IApprovalService {
 }

 if (stranded.length || undetermined) {
 this.logger?.warn?.('[approvals] stranded terminal requests (decision recorded, flow run gone)', {
 // The two shapes are counted separately: they have different causes and
 // different remedies, and an operator reading one number could not tell a
 // pre-existing #4469 zombie from a run that failed mid-resume (#13909).
 this.logger?.warn?.('[approvals] stranded terminal requests (decision recorded, flow run unrecoverable)', {
 scanned: rows.length, stranded: stranded.length, undetermined,
 requests: stranded.map(s => `${s.requestId}@${s.nodeId ?? '?'} → run ${s.runId}`),
 runMissing: stranded.filter(s => s.runState === 'missing').length,
 runFailed: stranded.filter(s => s.runState === 'failed').length,
 requests: stranded.map(s => `${s.requestId}@${s.nodeId ?? '?'} → run ${s.runId} (${s.runState})`),
 });
 }
 return { scanned: rows.length, stranded, undetermined };
Expand Down

2 changes: 2 additions & 0 deletions
packages/plugins/plugin-approvals/src/index.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -25,6 +25,8 @@ export {
 type ApprovalNodeAutoOutcome,
 // #4469 — the read-only stranded-request inspection's report shape.
 type StrandedApprovalRequest,
 // #13909 — which unrecoverable shape a reported row is in.
 type StrandedRunState,
} from './approval-service.js';
export {
 ApprovalsServicePlugin,
Expand Down
Loading
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix(plugin-approvals): report a run that failed mid-resume as stranded, and measure the resume-ordering fork (#13909)#13934

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

os-steve
merged 4 commits into

from

Sep 1, 2026

+346

−12

 Merged


fix(plugin-approvals): report a run that failed mid-resume as stranded, and measure the resume-ordering fork (#13909)
#13934
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


4 commits


Select commit
Hold shift + click to select a range
8e2ec8f

fix(plugin-approvals): report a run that failed mid-resume as strande…


claude Aug 31, 2026
e138bf9

docs(permissions): re-anchor the system-context census rows this PR's…


claude Aug 31, 2026
d987bbb

docs(plugin-approvals): fix the runId field doc the widening falsifie…


claude Sep 1, 2026
d3f53f0

docs(permissions): re-anchor the 8 approval-service census rows the +…


claude Sep 1, 2026

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


48 changes: 48 additions & 0 deletions
.changeset/approvals-inspector-sees-failed-mid-resume.md

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
---
"@objectstack/plugin-approvals": patch
---

fix(plugin-approvals): the stranded-request inspection now sees a run that FAILED mid-resume (#13909)

`ApprovalService.inspectStrandedRequests` was structurally blind to the shape
#13909 owns, and reported `0` for it — the one shape an operator most needs to
see.

**The mechanism.** `AutomationEngine.resumeInternal` consumes the suspension
*before* running the downstream nodes: `forgetSuspendedRun(run, 'resumed')`
precedes `traverseNext`. A downstream node that merely THREW therefore threw
with the pause already gone, the catch arm recorded the run `failed`, and
nothing can resume it again (`resume` answers `RUN_NOT_FOUND`, `cancelRun` is a
no-op). The decision is durable and the flow stopped half-way.

**Why the inspection could not see it.** Its second oracle was
`if (terminal) continue` — the existence of ANY run-history row ended the check,
on the reading "the run ran to a terminal state, it is not dangling". But the
terminal row here is written BY the failure that stranded the request, so the
evidence of the defect was read as evidence of health. `releaseDeadRunRequests`
cannot see it either: it scans `status: 'pending'`, and the decision is what
took the row out of `pending`.

**The widening, and its limits.** The second oracle now classifies the run
instead of merely detecting it. A `failed` run is reported; `completed`,
`cancelled` and `paused` are each still skipped, one named reason at a time, and
a status this code does not recognise is skipped too — the spec's
`ExecutionStatus` vocabulary is wider than the four statuses the engine writes,
and a future status must not become a silent false positive. `paused` in
particular stays skipped because "the suspension is gone but no terminal row is
written yet" is exactly what a resume IN FLIGHT looks like. The first oracle is
unchanged: a run the suspension store still holds is alive, and an unreadable
store is still counted `undetermined`, never condemned.

Reported rows now carry `runState: 'missing' | 'failed'` (new exported type
`StrandedRunState`), because the two shapes need different remedies: a `missing`
run has no history to read, a `failed` one has a step log and an error naming
the node that threw. The sweep's own warning splits its counts the same way.
`StrandedApprovalRequest` is an output-only reporting shape the service
produces; the added field is not constructed by any caller in this repo.

**Still read-only, and still not a census.** No status is changed and no run is
cancelled — the decision genuinely happened. This makes the condition *visible*
in a deployment; how many runs are already in it can only be answered against
that deployment's own tables. Nothing here changes the resume ordering, which
is #13909's own next slice.

2 changes: 1 addition & 1 deletion
content/docs/permissions/system-context.mdx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -145,7 +145,7 @@ The largest single consumer — **20 of the 109 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:333` |
| 41 | Delegation write guard bypassed | plugin-approvals | Get: service / seed / import may write delegation rows naming another delegator | `lifecycle-hooks.ts:440` |
| 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:850`, `:959`, `:2916`, `:3062`, `:3229`, `:3300`, `:3489`, `:3529` |
| 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:931`, `:1040`, `:2997`, `:3143`, `:3310`, `:3381`, `:3570`, `:3610` |
| 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` |
Expand Down

138 changes: 128 additions & 10 deletions
packages/plugins/plugin-approvals/src/approval-service.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -247,6 +247,76 @@ const TERMINAL_RUN_STATUSES: ReadonlySet<string> = new Set([
 */
const STRANDABLE_REQUEST_STATUSES = ['approved', 'rejected', 'returned'] as const;

/**
 * The second oracle's verdict: which unrecoverable shape this run is in, or
 * `undefined` for every run that must NOT be reported (#13909).
 *
 * Written as an explicit switch, not `status !== 'completed'`, because the
 * negatives are the load-bearing half — a widening that reports everything the
 * old `if (terminal) continue` used to skip would bury the finding it exists to
 * surface. Each skip below is a distinct reason, and each is pinned by its own
 * test.
 *
 * The engine writes exactly four run statuses (`paused`, `completed`, `failed`,
 * `cancelled`); the spec's `ExecutionStatus` vocabulary is wider (`timed_out`,
 * `retrying`, …) and nothing in the engine writes the rest today. The `default`
 * arm therefore stays SILENT rather than reporting: a status this code does not
 * recognise is not evidence a decision was stranded, and condemning on it would
 * make every future status a false positive until someone noticed.
 */
function classifyStrandedRunState(run: { status?: string } | null | undefined): StrandedRunState | undefined {
 // No history row at all — the #4469 shape this inspection was built for.
 if (!run) return 'missing';
 switch (run.status) {
 // The resume consumed the pause and a downstream node threw. Reported.
 case 'failed':
 return 'failed';

 // ── The negatives, each for its own reason ──────────────────────────────
 // The decision advanced the flow and the flow finished. Healthy.
 case 'completed':
 return undefined;
 // Deliberately terminated by an operator (`cancelRun`, ADR-0044). The run
 // stopping is the intended outcome, exactly as `recalled` is on the request
 // side — reporting it would bury the real findings under expected ones.
 case 'cancelled':
 return undefined;
 // The history's last row says `paused` while the suspension store says no
 // live pause. That is AMBIGUOUS, and the ambiguity is not resolvable from
 // one scan: a resume in flight right now has consumed the suspension and
 // not yet written its terminal row, and reads exactly like a process that
 // died in the same window. Condemning it would name every concurrently
 // resuming approval — so this stays SKIPPED, the conservative arm this
 // whole method is built on.
 case 'paused':
 return undefined;
 default:
 return undefined;
 }
}

/**
 * WHY a terminal request's run is unrecoverable — the two shapes the inspection
 * reports, which have different causes and different remedies (#13909).
 *
 * - `missing` — `getRun` finds no history row at all (#4469's original shape):
 * the run was lost before it could record anything, typically a pause that
 * never reached a durable store and did not survive a restart.
 * - `failed` — the run DID record a terminal `failed` row. The engine consumes
 * a suspension *before* running the downstream nodes
 * (`AutomationEngine.resumeInternal`: `forgetSuspendedRun(run, 'resumed')`
 * precedes `traverseNext`), so a downstream node that merely THREW threw with
 * the pause already gone — the catch arm recorded `failed` and there is no
 * suspension left to resume. The decision is durable, the flow stopped
 * mid-continuation, and no verb moves the run out of that state.
 *
 * ⚠️ This names the shapes for the REPORT only. It is not a run state: the
 * engine's own vocabulary is still `'completed' | 'paused' | 'failed'`
 * (`AutomationResult.status`) and nothing persists or queries "stranded".
 * Giving the condition a platform-level name is #13909's own deliverable.
 */
export type StrandedRunState = 'missing' | 'failed';

/**
 * One terminal request whose owning flow run is unrecoverable (#4469) — the
 * decision was recorded and the flow never moved. Reporting shape only: the
Expand All@@ -257,8 +327,19 @@ export interface StrandedApprovalRequest {
 requestId: string;
 /** Terminal status the request reached — the decision that WAS recorded. */
 status: string;
 /** The `flow_run_id` that resolves to neither a suspension nor a run history row. */
 /**
 * The `flow_run_id` that resolves to no live suspension and no recoverable
 * run — see `runState`: no history row at all (`missing`), or a terminal
 * `failed` row (`failed`).
 */
 runId: string;
 /**
 * Which unrecoverable shape this is — see {@link StrandedRunState}. Carried
 * because the two need different remedies: a `missing` run has no history to
 * read, while a `failed` one has a step log and an error message naming the
 * node that threw.
 */
 runState: StrandedRunState;
 flowName?: string;
 /** Approval node the run should have continued from. */
 nodeId?: string;
Expand DownExpand Up@@ -3679,9 +3760,35 @@ export class ApprovalService implements IApprovalService {
 * live pause exists. It THROWS when the store cannot be read, and that
 * case is SKIPPED, never counted as dead: an unreadable store means
 * "unknown", and a storage outage must not be published as a lost run.
 * - `getRun(runId) == null` — no terminal history row either (the `run_`
 * prefixed rows in `sys_automation_run`). A run that merely finished is
 * not stranded; a request whose run neither waits nor ever completed is.
 * - `classifyStrandedRunState` over `getRun(runId)` — the run's own
 * history row (the `run_` prefixed rows in `sys_automation_run`). A run
 * that merely finished is not stranded; a request whose run neither waits
 * nor completed is.
 *
 * **The second oracle was widened (#13909), and this is the whole point of
 * that card's first slice.** It used to be `if (terminal) continue` — the
 * existence of ANY history row ended the check, on the reading "the run ran to
 * a terminal state, it is not dangling". That is true of a run that COMPLETED
 * and false of one that FAILED: the engine consumes a suspension *before*
 * running the downstream nodes (`AutomationEngine.resumeInternal` calls
 * `forgetSuspendedRun(run, 'resumed')` and only then `traverseNext`), so a
 * downstream node that merely threw threw with the pause already gone, and the
 * catch arm wrote a terminal `failed` row. The decision is durable, the
 * continuation stopped half-way, `resume` answers `RUN_NOT_FOUND` and
 * `cancelRun` is a no-op — and the terminal row this oracle used to read as
 * health is written BY the very failure that stranded it. So this inspection
 * reported `0` for the one shape an operator most needs to see.
 *
 * ⚠️ The widening does NOT reverse the conservatism: `completed`, `cancelled`
 * and `paused` are each still skipped, for reasons named one at a time in
 * `classifyStrandedRunState`, and an unrecognised status is skipped too.
 * What the widening buys is that a `failed` run is now reported with
 * `runState: 'failed'` instead of counted as healthy.
 *
 * ⚠️ **What this can and cannot size.** It makes the condition *visible* in a
 * deployment; it is not itself a census, and it says nothing about this
 * repository. How many runs are already in this state can only be answered
 * against a real deployment's tables — see the card.
 *
 * **Reports; never rewrites.** No status is changed and no run is cancelled.
 * The decision genuinely happened — a human approved or rejected — and
Expand DownExpand Up@@ -3748,10 +3855,15 @@ export class ApprovalService implements IApprovalService {
 });
 continue;
 }
 if (terminal) continue; // the run ran to a terminal state — it is not dangling

 // Neither suspended nor ever finished: the run this decision was supposed
 // to advance is genuinely gone.
 // #13909 — the widened verdict. `undefined` means "not a shape this
 // reports": healthy, deliberate, or unresolvable. See
 // `classifyStrandedRunState` for which, and why each one.
 const runState = classifyStrandedRunState(terminal);
 if (!runState) continue;

 // Neither suspended nor recoverable: the run this decision was supposed to
 // advance is gone (`missing`) or terminally failed mid-continuation with
 // its pause already consumed (`failed`).
 const config = parseJson<ApprovalNodeConfig>(
 raw.node_config_json, { approvers: [], behavior: 'first_response' } as any,
 );
Expand All@@ -3770,6 +3882,7 @@ export class ApprovalService implements IApprovalService {
 requestId: String(raw.id),
 status: raw.status,
 runId,
 runState,
 flowName: typeof raw.process_name === 'string' ? raw.process_name.replace(/^flow:/, '') : undefined,
 nodeId: raw.flow_node_id ?? raw.current_step ?? undefined,
 objectName: raw.object_name,
Expand All@@ -3782,9 +3895,14 @@ export class ApprovalService implements IApprovalService {
 }

 if (stranded.length || undetermined) {
 this.logger?.warn?.('[approvals] stranded terminal requests (decision recorded, flow run gone)', {
 // The two shapes are counted separately: they have different causes and
 // different remedies, and an operator reading one number could not tell a
 // pre-existing #4469 zombie from a run that failed mid-resume (#13909).
 this.logger?.warn?.('[approvals] stranded terminal requests (decision recorded, flow run unrecoverable)', {
 scanned: rows.length, stranded: stranded.length, undetermined,
 requests: stranded.map(s => `${s.requestId}@${s.nodeId ?? '?'} → run ${s.runId}`),
 runMissing: stranded.filter(s => s.runState === 'missing').length,
 runFailed: stranded.filter(s => s.runState === 'failed').length,
 requests: stranded.map(s => `${s.requestId}@${s.nodeId ?? '?'} → run ${s.runId} (${s.runState})`),
 });
 }
 return { scanned: rows.length, stranded, undetermined };
Expand Down

2 changes: 2 additions & 0 deletions
packages/plugins/plugin-approvals/src/index.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -25,6 +25,8 @@ export {
 type ApprovalNodeAutoOutcome,
 // #4469 — the read-only stranded-request inspection's report shape.
 type StrandedApprovalRequest,
 // #13909 — which unrecoverable shape a reported row is in.
 type StrandedRunState,
} from './approval-service.js';
export {
 ApprovalsServicePlugin,
Expand Down
Loading
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix(plugin-approvals): report a run that failed mid-resume as stranded, and measure the resume-ordering fork (#13909)#13934

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

os-steve
merged 4 commits into

from

Sep 1, 2026

+346

−12

 Merged


fix(plugin-approvals): report a run that failed mid-resume as stranded, and measure the resume-ordering fork (#13909)
#13934
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


4 commits


Select commit
Hold shift + click to select a range
8e2ec8f

fix(plugin-approvals): report a run that failed mid-resume as strande…


claude Aug 31, 2026
e138bf9

docs(permissions): re-anchor the system-context census rows this PR's…


claude Aug 31, 2026
d987bbb

docs(plugin-approvals): fix the runId field doc the widening falsifie…


claude Sep 1, 2026
d3f53f0

docs(permissions): re-anchor the 8 approval-service census rows the +…


claude Sep 1, 2026

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


48 changes: 48 additions & 0 deletions
.changeset/approvals-inspector-sees-failed-mid-resume.md

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
---
"@objectstack/plugin-approvals": patch
---

fix(plugin-approvals): the stranded-request inspection now sees a run that FAILED mid-resume (#13909)

`ApprovalService.inspectStrandedRequests` was structurally blind to the shape
#13909 owns, and reported `0` for it — the one shape an operator most needs to
see.

**The mechanism.** `AutomationEngine.resumeInternal` consumes the suspension
*before* running the downstream nodes: `forgetSuspendedRun(run, 'resumed')`
precedes `traverseNext`. A downstream node that merely THREW therefore threw
with the pause already gone, the catch arm recorded the run `failed`, and
nothing can resume it again (`resume` answers `RUN_NOT_FOUND`, `cancelRun` is a
no-op). The decision is durable and the flow stopped half-way.

**Why the inspection could not see it.** Its second oracle was
`if (terminal) continue` — the existence of ANY run-history row ended the check,
on the reading "the run ran to a terminal state, it is not dangling". But the
terminal row here is written BY the failure that stranded the request, so the
evidence of the defect was read as evidence of health. `releaseDeadRunRequests`
cannot see it either: it scans `status: 'pending'`, and the decision is what
took the row out of `pending`.

**The widening, and its limits.** The second oracle now classifies the run
instead of merely detecting it. A `failed` run is reported; `completed`,
`cancelled` and `paused` are each still skipped, one named reason at a time, and
a status this code does not recognise is skipped too — the spec's
`ExecutionStatus` vocabulary is wider than the four statuses the engine writes,
and a future status must not become a silent false positive. `paused` in
particular stays skipped because "the suspension is gone but no terminal row is
written yet" is exactly what a resume IN FLIGHT looks like. The first oracle is
unchanged: a run the suspension store still holds is alive, and an unreadable
store is still counted `undetermined`, never condemned.

Reported rows now carry `runState: 'missing' | 'failed'` (new exported type
`StrandedRunState`), because the two shapes need different remedies: a `missing`
run has no history to read, a `failed` one has a step log and an error naming
the node that threw. The sweep's own warning splits its counts the same way.
`StrandedApprovalRequest` is an output-only reporting shape the service
produces; the added field is not constructed by any caller in this repo.

**Still read-only, and still not a census.** No status is changed and no run is
cancelled — the decision genuinely happened. This makes the condition *visible*
in a deployment; how many runs are already in it can only be answered against
that deployment's own tables. Nothing here changes the resume ordering, which
is #13909's own next slice.

2 changes: 1 addition & 1 deletion
content/docs/permissions/system-context.mdx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -145,7 +145,7 @@ The largest single consumer — **20 of the 109 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:333` |
| 41 | Delegation write guard bypassed | plugin-approvals | Get: service / seed / import may write delegation rows naming another delegator | `lifecycle-hooks.ts:440` |
| 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:850`, `:959`, `:2916`, `:3062`, `:3229`, `:3300`, `:3489`, `:3529` |
| 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:931`, `:1040`, `:2997`, `:3143`, `:3310`, `:3381`, `:3570`, `:3610` |
| 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` |
Expand Down

138 changes: 128 additions & 10 deletions
packages/plugins/plugin-approvals/src/approval-service.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -247,6 +247,76 @@ const TERMINAL_RUN_STATUSES: ReadonlySet<string> = new Set([
 */
const STRANDABLE_REQUEST_STATUSES = ['approved', 'rejected', 'returned'] as const;

/**
 * The second oracle's verdict: which unrecoverable shape this run is in, or
 * `undefined` for every run that must NOT be reported (#13909).
 *
 * Written as an explicit switch, not `status !== 'completed'`, because the
 * negatives are the load-bearing half — a widening that reports everything the
 * old `if (terminal) continue` used to skip would bury the finding it exists to
 * surface. Each skip below is a distinct reason, and each is pinned by its own
 * test.
 *
 * The engine writes exactly four run statuses (`paused`, `completed`, `failed`,
 * `cancelled`); the spec's `ExecutionStatus` vocabulary is wider (`timed_out`,
 * `retrying`, …) and nothing in the engine writes the rest today. The `default`
 * arm therefore stays SILENT rather than reporting: a status this code does not
 * recognise is not evidence a decision was stranded, and condemning on it would
 * make every future status a false positive until someone noticed.
 */
function classifyStrandedRunState(run: { status?: string } | null | undefined): StrandedRunState | undefined {
 // No history row at all — the #4469 shape this inspection was built for.
 if (!run) return 'missing';
 switch (run.status) {
 // The resume consumed the pause and a downstream node threw. Reported.
 case 'failed':
 return 'failed';

 // ── The negatives, each for its own reason ──────────────────────────────
 // The decision advanced the flow and the flow finished. Healthy.
 case 'completed':
 return undefined;
 // Deliberately terminated by an operator (`cancelRun`, ADR-0044). The run
 // stopping is the intended outcome, exactly as `recalled` is on the request
 // side — reporting it would bury the real findings under expected ones.
 case 'cancelled':
 return undefined;
 // The history's last row says `paused` while the suspension store says no
 // live pause. That is AMBIGUOUS, and the ambiguity is not resolvable from
 // one scan: a resume in flight right now has consumed the suspension and
 // not yet written its terminal row, and reads exactly like a process that
 // died in the same window. Condemning it would name every concurrently
 // resuming approval — so this stays SKIPPED, the conservative arm this
 // whole method is built on.
 case 'paused':
 return undefined;
 default:
 return undefined;
 }
}

/**
 * WHY a terminal request's run is unrecoverable — the two shapes the inspection
 * reports, which have different causes and different remedies (#13909).
 *
 * - `missing` — `getRun` finds no history row at all (#4469's original shape):
 * the run was lost before it could record anything, typically a pause that
 * never reached a durable store and did not survive a restart.
 * - `failed` — the run DID record a terminal `failed` row. The engine consumes
 * a suspension *before* running the downstream nodes
 * (`AutomationEngine.resumeInternal`: `forgetSuspendedRun(run, 'resumed')`
 * precedes `traverseNext`), so a downstream node that merely THREW threw with
 * the pause already gone — the catch arm recorded `failed` and there is no
 * suspension left to resume. The decision is durable, the flow stopped
 * mid-continuation, and no verb moves the run out of that state.
 *
 * ⚠️ This names the shapes for the REPORT only. It is not a run state: the
 * engine's own vocabulary is still `'completed' | 'paused' | 'failed'`
 * (`AutomationResult.status`) and nothing persists or queries "stranded".
 * Giving the condition a platform-level name is #13909's own deliverable.
 */
export type StrandedRunState = 'missing' | 'failed';

/**
 * One terminal request whose owning flow run is unrecoverable (#4469) — the
 * decision was recorded and the flow never moved. Reporting shape only: the
Expand All@@ -257,8 +327,19 @@ export interface StrandedApprovalRequest {
 requestId: string;
 /** Terminal status the request reached — the decision that WAS recorded. */
 status: string;
 /** The `flow_run_id` that resolves to neither a suspension nor a run history row. */
 /**
 * The `flow_run_id` that resolves to no live suspension and no recoverable
 * run — see `runState`: no history row at all (`missing`), or a terminal
 * `failed` row (`failed`).
 */
 runId: string;
 /**
 * Which unrecoverable shape this is — see {@link StrandedRunState}. Carried
 * because the two need different remedies: a `missing` run has no history to
 * read, while a `failed` one has a step log and an error message naming the
 * node that threw.
 */
 runState: StrandedRunState;
 flowName?: string;
 /** Approval node the run should have continued from. */
 nodeId?: string;
Expand DownExpand Up@@ -3679,9 +3760,35 @@ export class ApprovalService implements IApprovalService {
 * live pause exists. It THROWS when the store cannot be read, and that
 * case is SKIPPED, never counted as dead: an unreadable store means
 * "unknown", and a storage outage must not be published as a lost run.
 * - `getRun(runId) == null` — no terminal history row either (the `run_`
 * prefixed rows in `sys_automation_run`). A run that merely finished is
 * not stranded; a request whose run neither waits nor ever completed is.
 * - `classifyStrandedRunState` over `getRun(runId)` — the run's own
 * history row (the `run_` prefixed rows in `sys_automation_run`). A run
 * that merely finished is not stranded; a request whose run neither waits
 * nor completed is.
 *
 * **The second oracle was widened (#13909), and this is the whole point of
 * that card's first slice.** It used to be `if (terminal) continue` — the
 * existence of ANY history row ended the check, on the reading "the run ran to
 * a terminal state, it is not dangling". That is true of a run that COMPLETED
 * and false of one that FAILED: the engine consumes a suspension *before*
 * running the downstream nodes (`AutomationEngine.resumeInternal` calls
 * `forgetSuspendedRun(run, 'resumed')` and only then `traverseNext`), so a
 * downstream node that merely threw threw with the pause already gone, and the
 * catch arm wrote a terminal `failed` row. The decision is durable, the
 * continuation stopped half-way, `resume` answers `RUN_NOT_FOUND` and
 * `cancelRun` is a no-op — and the terminal row this oracle used to read as
 * health is written BY the very failure that stranded it. So this inspection
 * reported `0` for the one shape an operator most needs to see.
 *
 * ⚠️ The widening does NOT reverse the conservatism: `completed`, `cancelled`
 * and `paused` are each still skipped, for reasons named one at a time in
 * `classifyStrandedRunState`, and an unrecognised status is skipped too.
 * What the widening buys is that a `failed` run is now reported with
 * `runState: 'failed'` instead of counted as healthy.
 *
 * ⚠️ **What this can and cannot size.** It makes the condition *visible* in a
 * deployment; it is not itself a census, and it says nothing about this
 * repository. How many runs are already in this state can only be answered
 * against a real deployment's tables — see the card.
 *
 * **Reports; never rewrites.** No status is changed and no run is cancelled.
 * The decision genuinely happened — a human approved or rejected — and
Expand DownExpand Up@@ -3748,10 +3855,15 @@ export class ApprovalService implements IApprovalService {
 });
 continue;
 }
 if (terminal) continue; // the run ran to a terminal state — it is not dangling

 // Neither suspended nor ever finished: the run this decision was supposed
 // to advance is genuinely gone.
 // #13909 — the widened verdict. `undefined` means "not a shape this
 // reports": healthy, deliberate, or unresolvable. See
 // `classifyStrandedRunState` for which, and why each one.
 const runState = classifyStrandedRunState(terminal);
 if (!runState) continue;

 // Neither suspended nor recoverable: the run this decision was supposed to
 // advance is gone (`missing`) or terminally failed mid-continuation with
 // its pause already consumed (`failed`).
 const config = parseJson<ApprovalNodeConfig>(
 raw.node_config_json, { approvers: [], behavior: 'first_response' } as any,
 );
Expand All@@ -3770,6 +3882,7 @@ export class ApprovalService implements IApprovalService {
 requestId: String(raw.id),
 status: raw.status,
 runId,
 runState,
 flowName: typeof raw.process_name === 'string' ? raw.process_name.replace(/^flow:/, '') : undefined,
 nodeId: raw.flow_node_id ?? raw.current_step ?? undefined,
 objectName: raw.object_name,
Expand All@@ -3782,9 +3895,14 @@ export class ApprovalService implements IApprovalService {
 }

 if (stranded.length || undetermined) {
 this.logger?.warn?.('[approvals] stranded terminal requests (decision recorded, flow run gone)', {
 // The two shapes are counted separately: they have different causes and
 // different remedies, and an operator reading one number could not tell a
 // pre-existing #4469 zombie from a run that failed mid-resume (#13909).
 this.logger?.warn?.('[approvals] stranded terminal requests (decision recorded, flow run unrecoverable)', {
 scanned: rows.length, stranded: stranded.length, undetermined,
 requests: stranded.map(s => `${s.requestId}@${s.nodeId ?? '?'} → run ${s.runId}`),
 runMissing: stranded.filter(s => s.runState === 'missing').length,
 runFailed: stranded.filter(s => s.runState === 'failed').length,
 requests: stranded.map(s => `${s.requestId}@${s.nodeId ?? '?'} → run ${s.runId} (${s.runState})`),
 });
 }
 return { scanned: rows.length, stranded, undetermined };
Expand Down

2 changes: 2 additions & 0 deletions
packages/plugins/plugin-approvals/src/index.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -25,6 +25,8 @@ export {
 type ApprovalNodeAutoOutcome,
 // #4469 — the read-only stranded-request inspection's report shape.
 type StrandedApprovalRequest,
 // #13909 — which unrecoverable shape a reported row is in.
 type StrandedRunState,
} from './approval-service.js';
export {
 ApprovalsServicePlugin,
Expand Down
Loading
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
fix(plugin-approvals): report a run that failed mid-resume as stranded, and measure the resume-ordering fork (#13909)#13934

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

os-steve
merged 4 commits into

from

Sep 1, 2026

+346

−12

 Merged


fix(plugin-approvals): report a run that failed mid-resume as stranded, and measure the resume-ordering fork (#13909)
#13934
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


4 commits


Select commit
Hold shift + click to select a range
8e2ec8f

fix(plugin-approvals): report a run that failed mid-resume as strande…


claude Aug 31, 2026
e138bf9

docs(permissions): re-anchor the system-context census rows this PR's…


claude Aug 31, 2026
d987bbb

docs(plugin-approvals): fix the runId field doc the widening falsifie…


claude Sep 1, 2026
d3f53f0

docs(permissions): re-anchor the 8 approval-service census rows the +…


claude Sep 1, 2026

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


48 changes: 48 additions & 0 deletions
.changeset/approvals-inspector-sees-failed-mid-resume.md

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
---
"@objectstack/plugin-approvals": patch
---

fix(plugin-approvals): the stranded-request inspection now sees a run that FAILED mid-resume (#13909)

`ApprovalService.inspectStrandedRequests` was structurally blind to the shape
#13909 owns, and reported `0` for it — the one shape an operator most needs to
see.

**The mechanism.** `AutomationEngine.resumeInternal` consumes the suspension
*before* running the downstream nodes: `forgetSuspendedRun(run, 'resumed')`
precedes `traverseNext`. A downstream node that merely THREW therefore threw
with the pause already gone, the catch arm recorded the run `failed`, and
nothing can resume it again (`resume` answers `RUN_NOT_FOUND`, `cancelRun` is a
no-op). The decision is durable and the flow stopped half-way.

**Why the inspection could not see it.** Its second oracle was
`if (terminal) continue` — the existence of ANY run-history row ended the check,
on the reading "the run ran to a terminal state, it is not dangling". But the
terminal row here is written BY the failure that stranded the request, so the
evidence of the defect was read as evidence of health. `releaseDeadRunRequests`
cannot see it either: it scans `status: 'pending'`, and the decision is what
took the row out of `pending`.

**The widening, and its limits.** The second oracle now classifies the run
instead of merely detecting it. A `failed` run is reported; `completed`,
`cancelled` and `paused` are each still skipped, one named reason at a time, and
a status this code does not recognise is skipped too — the spec's
`ExecutionStatus` vocabulary is wider than the four statuses the engine writes,
and a future status must not become a silent false positive. `paused` in
particular stays skipped because "the suspension is gone but no terminal row is
written yet" is exactly what a resume IN FLIGHT looks like. The first oracle is
unchanged: a run the suspension store still holds is alive, and an unreadable
store is still counted `undetermined`, never condemned.

Reported rows now carry `runState: 'missing' | 'failed'` (new exported type
`StrandedRunState`), because the two shapes need different remedies: a `missing`
run has no history to read, a `failed` one has a step log and an error naming
the node that threw. The sweep's own warning splits its counts the same way.
`StrandedApprovalRequest` is an output-only reporting shape the service
produces; the added field is not constructed by any caller in this repo.

**Still read-only, and still not a census.** No status is changed and no run is
cancelled — the decision genuinely happened. This makes the condition *visible*
in a deployment; how many runs are already in it can only be answered against
that deployment's own tables. Nothing here changes the resume ordering, which
is #13909's own next slice.

2 changes: 1 addition & 1 deletion
content/docs/permissions/system-context.mdx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -145,7 +145,7 @@ The largest single consumer — **20 of the 109 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:333` |
| 41 | Delegation write guard bypassed | plugin-approvals | Get: service / seed / import may write delegation rows naming another delegator | `lifecycle-hooks.ts:440` |
| 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:850`, `:959`, `:2916`, `:3062`, `:3229`, `:3300`, `:3489`, `:3529` |
| 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:931`, `:1040`, `:2997`, `:3143`, `:3310`, `:3381`, `:3570`, `:3610` |
| 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` |
Expand Down

138 changes: 128 additions & 10 deletions
packages/plugins/plugin-approvals/src/approval-service.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -247,6 +247,76 @@ const TERMINAL_RUN_STATUSES: ReadonlySet<string> = new Set([
 */
const STRANDABLE_REQUEST_STATUSES = ['approved', 'rejected', 'returned'] as const;

/**
 * The second oracle's verdict: which unrecoverable shape this run is in, or
 * `undefined` for every run that must NOT be reported (#13909).
 *
 * Written as an explicit switch, not `status !== 'completed'`, because the
 * negatives are the load-bearing half — a widening that reports everything the
 * old `if (terminal) continue` used to skip would bury the finding it exists to
 * surface. Each skip below is a distinct reason, and each is pinned by its own
 * test.
 *
 * The engine writes exactly four run statuses (`paused`, `completed`, `failed`,
 * `cancelled`); the spec's `ExecutionStatus` vocabulary is wider (`timed_out`,
 * `retrying`, …) and nothing in the engine writes the rest today. The `default`
 * arm therefore stays SILENT rather than reporting: a status this code does not
 * recognise is not evidence a decision was stranded, and condemning on it would
 * make every future status a false positive until someone noticed.
 */
function classifyStrandedRunState(run: { status?: string } | null | undefined): StrandedRunState | undefined {
 // No history row at all — the #4469 shape this inspection was built for.
 if (!run) return 'missing';
 switch (run.status) {
 // The resume consumed the pause and a downstream node threw. Reported.
 case 'failed':
 return 'failed';

 // ── The negatives, each for its own reason ──────────────────────────────
 // The decision advanced the flow and the flow finished. Healthy.
 case 'completed':
 return undefined;
 // Deliberately terminated by an operator (`cancelRun`, ADR-0044). The run
 // stopping is the intended outcome, exactly as `recalled` is on the request
 // side — reporting it would bury the real findings under expected ones.
 case 'cancelled':
 return undefined;
 // The history's last row says `paused` while the suspension store says no
 // live pause. That is AMBIGUOUS, and the ambiguity is not resolvable from
 // one scan: a resume in flight right now has consumed the suspension and
 // not yet written its terminal row, and reads exactly like a process that
 // died in the same window. Condemning it would name every concurrently
 // resuming approval — so this stays SKIPPED, the conservative arm this
 // whole method is built on.
 case 'paused':
 return undefined;
 default:
 return undefined;
 }
}

/**
 * WHY a terminal request's run is unrecoverable — the two shapes the inspection
 * reports, which have different causes and different remedies (#13909).
 *
 * - `missing` — `getRun` finds no history row at all (#4469's original shape):
 * the run was lost before it could record anything, typically a pause that
 * never reached a durable store and did not survive a restart.
 * - `failed` — the run DID record a terminal `failed` row. The engine consumes
 * a suspension *before* running the downstream nodes
 * (`AutomationEngine.resumeInternal`: `forgetSuspendedRun(run, 'resumed')`
 * precedes `traverseNext`), so a downstream node that merely THREW threw with
 * the pause already gone — the catch arm recorded `failed` and there is no
 * suspension left to resume. The decision is durable, the flow stopped
 * mid-continuation, and no verb moves the run out of that state.
 *
 * ⚠️ This names the shapes for the REPORT only. It is not a run state: the
 * engine's own vocabulary is still `'completed' | 'paused' | 'failed'`
 * (`AutomationResult.status`) and nothing persists or queries "stranded".
 * Giving the condition a platform-level name is #13909's own deliverable.
 */
export type StrandedRunState = 'missing' | 'failed';

/**
 * One terminal request whose owning flow run is unrecoverable (#4469) — the
 * decision was recorded and the flow never moved. Reporting shape only: the
Expand All@@ -257,8 +327,19 @@ export interface StrandedApprovalRequest {
 requestId: string;
 /** Terminal status the request reached — the decision that WAS recorded. */
 status: string;
 /** The `flow_run_id` that resolves to neither a suspension nor a run history row. */
 /**
 * The `flow_run_id` that resolves to no live suspension and no recoverable
 * run — see `runState`: no history row at all (`missing`), or a terminal
 * `failed` row (`failed`).
 */
 runId: string;
 /**
 * Which unrecoverable shape this is — see {@link StrandedRunState}. Carried
 * because the two need different remedies: a `missing` run has no history to
 * read, while a `failed` one has a step log and an error message naming the
 * node that threw.
 */
 runState: StrandedRunState;
 flowName?: string;
 /** Approval node the run should have continued from. */
 nodeId?: string;
Expand DownExpand Up@@ -3679,9 +3760,35 @@ export class ApprovalService implements IApprovalService {
 * live pause exists. It THROWS when the store cannot be read, and that
 * case is SKIPPED, never counted as dead: an unreadable store means
 * "unknown", and a storage outage must not be published as a lost run.
 * - `getRun(runId) == null` — no terminal history row either (the `run_`
 * prefixed rows in `sys_automation_run`). A run that merely finished is
 * not stranded; a request whose run neither waits nor ever completed is.
 * - `classifyStrandedRunState` over `getRun(runId)` — the run's own
 * history row (the `run_` prefixed rows in `sys_automation_run`). A run
 * that merely finished is not stranded; a request whose run neither waits
 * nor completed is.
 *
 * **The second oracle was widened (#13909), and this is the whole point of
 * that card's first slice.** It used to be `if (terminal) continue` — the
 * existence of ANY history row ended the check, on the reading "the run ran to
 * a terminal state, it is not dangling". That is true of a run that COMPLETED
 * and false of one that FAILED: the engine consumes a suspension *before*
 * running the downstream nodes (`AutomationEngine.resumeInternal` calls
 * `forgetSuspendedRun(run, 'resumed')` and only then `traverseNext`), so a
 * downstream node that merely threw threw with the pause already gone, and the
 * catch arm wrote a terminal `failed` row. The decision is durable, the
 * continuation stopped half-way, `resume` answers `RUN_NOT_FOUND` and
 * `cancelRun` is a no-op — and the terminal row this oracle used to read as
 * health is written BY the very failure that stranded it. So this inspection
 * reported `0` for the one shape an operator most needs to see.
 *
 * ⚠️ The widening does NOT reverse the conservatism: `completed`, `cancelled`
 * and `paused` are each still skipped, for reasons named one at a time in
 * `classifyStrandedRunState`, and an unrecognised status is skipped too.
 * What the widening buys is that a `failed` run is now reported with
 * `runState: 'failed'` instead of counted as healthy.
 *
 * ⚠️ **What this can and cannot size.** It makes the condition *visible* in a
 * deployment; it is not itself a census, and it says nothing about this
 * repository. How many runs are already in this state can only be answered
 * against a real deployment's tables — see the card.
 *
 * **Reports; never rewrites.** No status is changed and no run is cancelled.
 * The decision genuinely happened — a human approved or rejected — and
Expand DownExpand Up@@ -3748,10 +3855,15 @@ export class ApprovalService implements IApprovalService {
 });
 continue;
 }
 if (terminal) continue; // the run ran to a terminal state — it is not dangling

 // Neither suspended nor ever finished: the run this decision was supposed
 // to advance is genuinely gone.
 // #13909 — the widened verdict. `undefined` means "not a shape this
 // reports": healthy, deliberate, or unresolvable. See
 // `classifyStrandedRunState` for which, and why each one.
 const runState = classifyStrandedRunState(terminal);
 if (!runState) continue;

 // Neither suspended nor recoverable: the run this decision was supposed to
 // advance is gone (`missing`) or terminally failed mid-continuation with
 // its pause already consumed (`failed`).
 const config = parseJson<ApprovalNodeConfig>(
 raw.node_config_json, { approvers: [], behavior: 'first_response' } as any,
 );
Expand All@@ -3770,6 +3882,7 @@ export class ApprovalService implements IApprovalService {
 requestId: String(raw.id),
 status: raw.status,
 runId,
 runState,
 flowName: typeof raw.process_name === 'string' ? raw.process_name.replace(/^flow:/, '') : undefined,
 nodeId: raw.flow_node_id ?? raw.current_step ?? undefined,
 objectName: raw.object_name,
Expand All@@ -3782,9 +3895,14 @@ export class ApprovalService implements IApprovalService {
 }

 if (stranded.length || undetermined) {
 this.logger?.warn?.('[approvals] stranded terminal requests (decision recorded, flow run gone)', {
 // The two shapes are counted separately: they have different causes and
 // different remedies, and an operator reading one number could not tell a
 // pre-existing #4469 zombie from a run that failed mid-resume (#13909).
 this.logger?.warn?.('[approvals] stranded terminal requests (decision recorded, flow run unrecoverable)', {
 scanned: rows.length, stranded: stranded.length, undetermined,
 requests: stranded.map(s => `${s.requestId}@${s.nodeId ?? '?'} → run ${s.runId}`),
 runMissing: stranded.filter(s => s.runState === 'missing').length,
 runFailed: stranded.filter(s => s.runState === 'failed').length,
 requests: stranded.map(s => `${s.requestId}@${s.nodeId ?? '?'} → run ${s.runId} (${s.runState})`),
 });
 }
 return { scanned: rows.length, stranded, undetermined };
Expand Down

2 changes: 2 additions & 0 deletions
packages/plugins/plugin-approvals/src/index.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -25,6 +25,8 @@ export {
 type ApprovalNodeAutoOutcome,
 // #4469 — the read-only stranded-request inspection's report shape.
 type StrandedApprovalRequest,
 // #13909 — which unrecoverable shape a reported row is in.
 type StrandedRunState,
} from './approval-service.js';
export {
 ApprovalsServicePlugin,
Expand Down
Loading
Loading