Commit 6530e04
fix(approvals): give a restored approval suspension an issuer, so it can be decided and not only cancelled (#15966)
* fix(approvals): give a restored approval suspension an issuer, so it can be decided and not only cancelled
`AutomationEngine.restoreConsumedSuspension` re-arms the pause of a run that
stranded mid-resume and tells the operator to re-issue the continuation. For an
`approval` suspension nobody could. Every approvals door that stamps the resume
marker — `decide`, `recall`, `sendBack`, `resubmit` — guards on a `pending`
request, and the row is terminal, written by the very call that stranded the
run; the generic engine door refuses an `approval` pause outright because that
node declares `resumeAuthority: 'service'`. The only verb left was `cancelRun`,
which discards the branch's downstream work, so the advertised repair produced a
run that looked resumable and was not decidable.
Measured against the real engine and the real decision door: the restored
suspension lacks nothing. A `resumeAuthority`-marked resume walks the restored
pause to completion. What was missing is an ISSUER on the approvals side, which
is what this adds — `ApprovalService.continueRestoredRun(requestId, options?)`.
The failing door now journals the signal it was carrying, on the repairable exit
only (the engine's own `status: 'stranded'` discriminator, the one exit that
journals a repair snapshot), under `__strandedContinuation` in the request's
`node_config_json` — beside the `__decisionOutputs` side-channel already there,
so no column and no authored config moves. It is awaited but can never replace
the `RESUME_FAILED` throw the decision's caller is owed.
Runs stranded before this shipped are served too: with no journal, the signal is
rebuilt from the recorded outcome, which is exact for each accepted status
because each is written by exactly one door (`approved` → `approve`, `rejected`
→ `reject`, `returned` → `revise`, `recalled` → `reject`). The one shape it
cannot rebuild — a `rejected` row also carrying a `revise` action, possibly
ADR-0044's revision-limit auto-rejection whose resume carried `autoRejected` —
is refused rather than guessed.
The verb replays a decision and rewrites nothing: all four `pending` guards are
untouched, no status, mirror field or audit row is written, `resumeAuthority`
stays as it is, and `ApprovalDecisionResult` is not touched. Like the engine
verb it completes, it is an in-process operator repair with no REST route and no
entry in the spec `ApprovalService` contract.
Part of #15389
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
* test(approvals): pin the engine-side measurement directly, and fix the fake engine's insert arity
The raw `resumeAuthority`-marked resume against a restored suspension is the
measurement that places this card's fix on the approvals side rather than in
`engine.ts`, so it is pinned in its own right and not only through the new
verb. Driven on a second stranded-and-restored run so the cancel measurement
above it is undisturbed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
* chore(gates): teach three ledgers about this change's new double, new write site and shifted lines
Each repair is the one the failing gate itself prescribes, and each grows or
re-anchors a ledger rather than loosening one. A base control at the merge-base
(924f0fe) confirms all three are caused by this branch and were green before it.
- `engine-double-contract.pinned.json` learns the two doubles (delete, update)
in the new pin file. +10 lines, nothing removed, no baseline lowered: 791
pinned / 133 debt / 3 exempt, up from 789 at base.
- `content/docs/permissions/system-context.mdx` re-anchored by the census's own
`--fix`: inserting methods into `approval-service.ts` shifted eight cited line
numbers. Pure line rot, no elevation behaviour added or removed — this branch
introduces no `context.isSystem` read.
- The tenant-audit census re-measured: `journalStrandedContinuation` adds one
write call site, so 221 -> 222, 147 -> 148 decidable, 103 -> 104 decidably
elevated. `--write` regenerates the tables; the eight prose figures the gate
names as hand-written are corrected to match, each verified against the
regenerated table rather than arithmetic.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
* chore(gates): re-anchor the system-context census against the merged tree
Merging origin/main auto-merged content/docs/permissions/system-context.mdx
textually clean and semantically stale: the census is green at origin/main
(904e707, measured) and green on this branch before the merge, but red on the
merge product with 26 problems, every one an anchor into plugin-security or
rest — files this branch never touches and the merge brought in.
Repaired with the census's own --fix, which re-anchors against the merged code.
15 anchors rewritten; the result reports 105 elevation read sites across 44
files and 140 resolving anchors, identical to what origin/main reports, so this
restores the upstream reading rather than inventing one. No elevation behaviour
is added or removed anywhere on this branch.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
* fix(approvals): tie the restored continuation to this request's own pause, and refuse the rebuilds that cannot be proved
Two defects found by contract review of the first revision, both measured at the
real door, and both making the repair verb advance a flow with no decision behind
it - strictly worse than the dead end it exists to open.
GUARDS. `hasSuspendedRun(runId)` answers a boolean and cannot tell a re-armed
pause from any other live pause on the same run. Four shapes were measured
advancing something they should not: a terminal request driving a LATER approval
node's pending pause with no decision recorded on it; a plain RETRY of the verb
doing the same after the run re-suspended; a next-round `returned` row orphaning
round two; and a `recalled` request resurrected into a new pending round from a
pause it never gated. The retry is the one that settles the severity: repeating a
repair verb is the ordinary shape of using it, not a hostile construction.
Three guards now stand, each with its own reverse-control pin: `assertLatestForRun`
(already in this file, used by recall and resubmit for the same reason) refuses a
superseded row; the strict `hasSuspendedRun` refuses a run with no pause and
throws rather than read a store outage as "not suspended"; and a node-identity
check refuses a pause parked anywhere but this request's own node. The last is
fail-closed in every direction - no reader, no listing, no entry, or an entry at
another node all refuse - because a false refusal costs a retry while a false
admission advances a step nobody decided.
It needs NO new automation-engine surface. `listSuspendedRunsDurable` is already
public on AutomationEngine and already names the parked node; the approvals-side
resume interface simply declares the method it already has.
REBUILD. "Each status is written by exactly one door" was false, and it was the
load-bearing premise of the no-journal path that serves runs stranded before this
ships. `returned` has one writer but two issuers, so a stranded resubmit was
rebuilt as a send-back and walked the `revise` edge with `{decision:'revise'}`
where `{resubmitted:true}` was owed - proceeding, rather than failing loudly,
through the engine's unmatched-label fallback. It is now discriminated by the
`resubmit` action row, whose sole writer in this file is `resubmit`. `recalled`
has two writers across three behaviours, two issuing no continuation at all, and
nothing on the row tells them apart - so it is REFUSED on the rebuild path, with
a message naming what an operator can do instead. Both stay journal-recoverable.
HARNESS. The test double ignored `orderBy`, so it returned the OLDEST row where
`assertLatestForRun` asks for the newest - the guard passed on every input and a
pin naming it would have measured nothing. The double now honours it, and guard 1
is additionally driven in isolation on a row whose `created_at` is controlled, so
the ordering is what is under test rather than the clock's resolution.
PIN 3 previously asserted `decide` alone while claiming all four `pending` guards;
it now asserts all four by name.
The system-context census is regenerated from the merged tree with
`pnpm gen:system-context-census` (9 anchors), discharging the deferral the merge
commit took; it reports the same 105 sites / 44 files / 140 anchors as before.
Part of #15389
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
* fix(approvals): make guard 3 ask where the signal was ISSUED FROM, and replace a phantom pin
Two things this PR claimed were not what it said they were. Both were in the
safe direction — nothing escalated — but a false claim is not shippable.
1. Guard 3 compared the run's parked node against the request row's own
`flow_node_id`. That is right for `approve`, `reject`, `revise` and
`recall`, all of which are issued at the request's own approval node. It is
WRONG for `resubmit`: a resubmit is only reachable after a send-back moved
the run to the revise window, and it resumes THAT pause down the
`resubmit` back-edge. So a stranded resubmit re-arms at the revise window
while the row still records the approval node, and guard 3 refused it — on
both the journal and the rebuild paths — with a message telling the
operator the pause was not this request's when it was exactly this
request's. The expected node is now signal-aware
(`ApprovalService.expectedPauseNode`), derived from the flow definition the
same way `assertReviseEdge` already derives the revise window, fail-closed
in every direction, and needing no automation surface that method did not
already use. The refusal message now names the node the signal was issued
from rather than "this request's own node".
1 parent dcad825 commit 6530e04
8 files changed
Lines changed: 1614 additions & 24 deletions
File tree
- .changeset
- content/docs/permissions
- docs/audits
- packages/plugins/plugin-approvals/src
- scripts
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
145 | 145 | | |
146 | 146 | | |
147 | 147 | | |
148 | | - | |
| 148 | + | |
149 | 149 | | |
150 | 150 | | |
151 | 151 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
98 | 98 | | |
99 | 99 | | |
100 | 100 | | |
101 | | - | |
| 101 | + | |
102 | 102 | | |
103 | 103 | | |
104 | 104 | | |
| |||
147 | 147 | | |
148 | 148 | | |
149 | 149 | | |
150 | | - | |
| 150 | + | |
151 | 151 | | |
152 | | - | |
153 | | - | |
| 152 | + | |
| 153 | + | |
154 | 154 | | |
155 | 155 | | |
156 | 156 | | |
| |||
167 | 167 | | |
168 | 168 | | |
169 | 169 | | |
170 | | - | |
| 170 | + | |
171 | 171 | | |
172 | 172 | | |
173 | 173 | | |
174 | | - | |
| 174 | + | |
175 | 175 | | |
176 | 176 | | |
177 | 177 | | |
| |||
183 | 183 | | |
184 | 184 | | |
185 | 185 | | |
186 | | - | |
187 | | - | |
| 186 | + | |
| 187 | + | |
188 | 188 | | |
189 | | - | |
| 189 | + | |
190 | 190 | | |
191 | | - | |
| 191 | + | |
192 | 192 | | |
193 | 193 | | |
194 | 194 | | |
195 | 195 | | |
196 | | - | |
| 196 | + | |
197 | 197 | | |
198 | 198 | | |
199 | 199 | | |
200 | 200 | | |
201 | 201 | | |
202 | | - | |
| 202 | + | |
203 | 203 | | |
204 | 204 | | |
205 | 205 | | |
206 | 206 | | |
207 | | - | |
| 207 | + | |
208 | 208 | | |
209 | 209 | | |
210 | 210 | | |
| |||
224 | 224 | | |
225 | 225 | | |
226 | 226 | | |
227 | | - | |
| 227 | + | |
228 | 228 | | |
229 | 229 | | |
230 | 230 | | |
231 | | - | |
| 231 | + | |
232 | 232 | | |
233 | 233 | | |
234 | 234 | | |
| |||
Lines changed: 8 additions & 8 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
29 | 29 | | |
30 | 30 | | |
31 | 31 | | |
32 | | - | |
33 | | - | |
| 32 | + | |
| 33 | + | |
34 | 34 | | |
35 | | - | |
| 35 | + | |
36 | 36 | | |
37 | | - | |
| 37 | + | |
38 | 38 | | |
39 | 39 | | |
40 | 40 | | |
41 | 41 | | |
42 | | - | |
| 42 | + | |
43 | 43 | | |
44 | 44 | | |
45 | 45 | | |
| |||
52 | 52 | | |
53 | 53 | | |
54 | 54 | | |
55 | | - | |
| 55 | + | |
56 | 56 | | |
57 | 57 | | |
58 | 58 | | |
59 | | - | |
| 59 | + | |
60 | 60 | | |
61 | 61 | | |
62 | 62 | | |
| |||
70 | 70 | | |
71 | 71 | | |
72 | 72 | | |
73 | | - | |
| 73 | + | |
74 | 74 | | |
75 | 75 | | |
76 | 76 | | |
| |||
0 commit comments