Repository navigation
Commit a3009cb
fix(plugin-approvals)!: refuse a decision whose paused run lost its flow before recording it (409 RESUME_TARGET_LOST, not 500 after the write) (#26544)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"}
-->
_Requested by **Leehom** · [project
thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1XW7vnyaZVQ7e4bbVPxZpnVF)_
Fixes #26477
Clause-②: yes (narrowing: an approve, reject or send-back on a request
whose paused run can never resume, because its flow was deleted or the
node it parked on was edited away, is refused 409 `RESUME_TARGET_LOST`
with nothing written, where it was recorded as finalized and answered
500 `RESUME_FAILED`; widened: `@objectstack/service-automation`'s
`AutomationEngine` gains the read-only public method
`unresumableSuspensionCause(runId)`)
Metadata-protocol: none
Skills-docs: none — no skill or hand-written docs page teaches what a
decision answers when its run's flow is gone; the answer it now gets is
the existing `RESUME_TARGET_LOST` ledger row, whose docs page is
generated
## 维护者速读
- **改了什么**:流程在审批暂停期间被删除(或其暂停节点被编辑掉)后,审批人点"批准"不再先记下决定再报 500,而是在写入任何东西之前就答
409 `RESUME_TARGET_LOST`,与"先取消运行再批准"的对照情形同形。
- **为什么改**:原来的前置检查只问"暂停记录还在不在",而暂停记录比流程活得久,于是决定被记成已批准、流程却永远不再推进,也无法修复。
- **风险与回滚**:风险低。只多了一次只读查询;读不到存储时照旧放行(fail open)。回滚即还原本 PR 的两个包。
- **先看**:`packages/plugins/plugin-approvals/src/approval-service.ts` 的
`assertRunResumable`;`packages/services/service-automation/src/run-resume.ts`
的 `resolveResumeTarget`;`engine.ts` 的 `unresumableSuspensionCause`。
- **两分钟验证**:`pnpm --filter @objectstack/plugin-approvals exec vitest run
src/deleted-flow-decision-refusal.test.ts`,预期 5 个用例全绿。
- **需要你做的**:无,席位落地。
## What changes
- **`@objectstack/service-automation`.** The resume's two
never-dispatched exits (the run's flow is not registered, or the node it
parked on is gone) are now one pure function, `resolveResumeTarget` in
`run-resume.ts`. `resumeInternal` calls it with byte-identical cause
texts, so the resume and settle behaviour from #23963 is unchanged.
`AutomationEngine.unresumableSuspensionCause(runId)` is a new read-only
public method that asks the same function without acting on the answer.
It returns the cause for an existing suspension that can never be
dispatched, and `undefined` when the run can continue or when no
suspension exists (that answer belongs to `hasSuspendedRun`). It rejects
when the store cannot be read, as `hasSuspendedRun` does. It settles and
records nothing, so it may answer before the boot seal (AGENTS.md
"Startup registry reads": a read-only probe is legal).
- **`@objectstack/plugin-approvals`.** `ApprovalResumeSurface` declares
the optional member. `assertRunResumable`, the pre-flight that every
decision door (approve, reject, send back, resubmit) runs before its
first write, asks it after the existence check. A run whose flow or
parked node is gone now gets the 409 `RESUME_TARGET_LOST` that a
cancelled run already gets, naming the cause, with nothing recorded. The
request stays `pending` with its approvers. A probe that cannot read the
store fails open, the same posture as the existence check. An engine
without the member gets the existence check alone.
- One `patch` changeset for both packages, with a `**BREAKING**` banner
and ADR-0087 `not-required (no-migration-prescription)`.
Door (b) of triage `6434168375`. Door (a), refusing `DELETE
/automation/:name` while runs are paused, is not added: (b) holds
whatever removed the flow (a delete, an uninstall, an overlay removal),
and the order allows (a) only at the same size with its own pin.
## Why
A suspended run outlives the flow it belongs to. The pre-flight asked
only `hasSuspendedRun`, which answers `true` for the orphaned row, so
the decision was written and the request flipped to `approved`. Only the
resume after that hit the engine's flow-gone exit (`RUN_NOT_FOUND`), and
the decision door answered 500 `RESUME_FAILED` with `finalized: true,
repairable: false`. The approver was told an error happened, but the
decision stood and the flow never ran its approve branch.
**Ordering, not atomicity.** The check and the decision write are
ordered: the check runs first, and nothing is written when it refuses. A
flow deleted in the window between the check and the resume still
reaches the post-resume `RESUME_FAILED` report. That is the same
residual race the cancelled-run pre-flight has carried since #4420, and
`approval-restart-resume.test.ts` already pins it. This PR does not
widen that window.
## Tests
- **New: `plugin-approvals/src/deleted-flow-decision-refusal.test.ts`**
(5 cases). It uses a real `AutomationEngine` over a durable in-memory
suspended-run store, sealed the way the plugin seals it, plus the real
`approval` node and the real `ApprovalService`:
- the flow is deleted while paused (`unregisterFlow`, the `DELETE
/automation/:name` door): the answer is `RESUME_TARGET_LOST` naming
`Flow 'scratch_approval' not found for run …`, no approve row is
written, the request stays `pending` with `u1` still pending, no branch
runs, and the run row is left alone;
- the parked node is edited away: the same refusal, with the node-gone
cause;
- **contrast**: the run is cancelled first, then approved. This gives
the 409 `RESUME_TARGET_LOST` it already gave, with nothing recorded. It
was green on `main` and stays green;
- a probe that cannot read the store fails open, and the decision
proceeds and resumes;
- the refusal's remedy holds: a recall afterwards releases the request
(`recalled`), and its resume settles the orphaned run.
- **New: `service-automation/src/unresumable-suspension-cause.test.ts`**
(6 cases). For both causes, the probe's cause equals byte for byte the
`error` that the resume then answers `RUN_NOT_FOUND` with, and the
parked row is untouched until that resume. A live suspension and a
missing one both answer `undefined`. Before the seal the probe still
answers, records nothing, and the run resumes once its flow registers. A
store that cannot be read rejects.
- **Red first on `main`** (base `45de8c6cba`, only the new approvals pin
applied, closure built): 3 failed and 1 passed. The flow-gone, node-gone
and remedy cases each received `RESUME_FAILED: the approve decision was
recorded on request …, but its flow run '…' could not be resumed and is
now stranded: … [RUN_NOT_FOUND]: Flow 'scratch_approval' not found for
run '…'`. That is the card's reproduction. The contrast case passed.
- **Ablation** (fix committed, `scripts/ablation-replace.mjs`; the
approvals pin imports `approval-service.ts` from source, so no build
sits in between): `if (lostCause !== undefined) {` → `if (false &&
lostCause !== undefined) {`. The anchor went 1 → 0 and the blob
`2fa79298581b` → `1f41d1e74334`. Result: 3 failed and 2 passed, the same
three cases. Restored: the blob equals HEAD and `git diff HEAD` is
empty.
## Acceptance notes
- The wire status comes from the message prefix at the REST door
(`approvals-routes.ts`: `RESUME_TARGET_LOST` → 409, `RESUME_FAILED` →
500), which `rest-approvals-wire-codes.test.ts` pins. These pins assert
the thrown code.
- After the 409 the orphaned run row still reads `paused`, as it did
before any decision. The refusal is a read. Settling a gone-for-good run
stays with the resume (#23963): any later resume of the run, including
the recall's, settles it.
- Open PR #26337 also edits `approval-service.ts`, in other hunks. A
local `git merge-tree` of the two heads reports no conflict.
## Verification
All readings below are at head `3fb9ba3199` unless a line names another
commit.
- **Derived gates:** `node scripts/pm/dispatch-gates.mjs --commands
--repo objectstack-ai/objectstack` derived 72 commands for this diff.
Each one was run, with its exit code captured before any pipe. `--ran`
reconciliation: `72 derived famil(ies) accounted for — 72 run, 0
NOT-MEASURED`. All 72 exited 0. The first pass, at `bd15ba3d1`, caught
three reds, all fixed before this head. `check:engine-double-contract`
and `check:objectql-double-limit` flagged the hand-copied fake engine in
the new approvals pin, which now uses the pinned double from
`decision-strand-envelope.test.ts` and is recorded in
`scripts/engine-double-contract.pinned.json` by the gate's own
`--write`. `check-plugin-teardown-shape --self-test` exited 3 because
the container lacked its positive-control commit, and it is green once
that commit is fetched.
- **Package suites:** `pnpm --filter @objectstack/plugin-approvals
test`: 102 files and 1295 tests passed. `pnpm --filter
@objectstack/service-automation test` (at `ccc86788`; nothing in that
package changed after it): 211 files and 2725 tests passed.
- **Typecheck:** `pnpm --filter @objectstack/service-automation
typecheck` and `pnpm --filter @objectstack/plugin-approvals typecheck`
both exit 0, including `check:test-typecheck: OK` with the ledger
unchanged.
- **Lint:** `eslint --no-inline-config packages/plugins/plugin-approvals
packages/services/service-automation` exit 0.
- **Full build:** `pnpm build` exit 0 (13m09s under the shared lock).
- **Dogfood, full battery** (owed by the `(narrowing)` arm), at
`bd15ba3d1`. The one later commit changes only the new approvals test
file and the double ledger, neither of which dogfood reads. `pnpm
--filter @objectstack/dogfood test` under the lock gave `VERDICT
command-exit 0`, with Test Files 300 passed and 3 skipped (303), and
Tests 2532 passed and 18 skipped (2550).
- **CI:** every family CI runs is read from this PR's check-runs. They
are not re-run locally.
<details><summary>--ran sheet (72 commands, exit codes)</summary>
```text
node scripts/check-adr-0087-registration.mjs --base origin/main :: exit 0
node scripts/check-adr-0087-registration.mjs --self-test :: exit 0
node scripts/check-changeset-no-major.mjs --base origin/main :: exit 0
node scripts/check-closing-keyword-parity.mjs :: exit 0
node scripts/check-closing-keyword-parity.mjs --self-test :: exit 0
node scripts/check-comment-mask-adoption.mjs :: exit 0
node scripts/check-comment-mask-adoption.mjs --self-test :: exit 0
node scripts/check-comment-mask-corpus.mjs :: exit 0
node scripts/check-dts-emitted.mjs --self-test :: exit 0
node scripts/check-empty-changeset.mjs --base origin/main :: exit 0
node scripts/check-empty-changeset.mjs --self-test :: exit 0
node scripts/check-issue-citations.mjs :: exit 0
node scripts/check-plugin-teardown-shape.mjs :: exit 0
node scripts/check-plugin-teardown-shape.mjs --self-test :: exit 0
node scripts/check-tenant-audit-census.mjs :: exit 0
node scripts/check-tenant-audit-census.mjs --self-test :: exit 0
node scripts/ci/assert-turbo-remote-cache.mjs --self-test :: exit 0
node scripts/ci/plan-test-core-shards.mjs --self-test :: exit 0
node scripts/ci/prune-turbo-cache.mjs --self-test :: exit 0
node scripts/docs-audit/check-affected-docs.mjs :: exit 0
node scripts/docs-audit/check-drift-comment.mjs :: exit 0
node scripts/pm/release-rehearsal-clone.mjs --self-test :: exit 0
node scripts/release-pending-publish.mjs --self-test :: exit 0
pnpm --filter @objectstack/spec run check:duration-unit-keys :: exit 0
pnpm check:agent-test-spelling :: exit 0
pnpm check:bash32-floor :: exit 0
pnpm check:changeset-gate-self-tests :: exit 0
pnpm check:ci-filter-parity :: exit 0
pnpm check:cli-command-ids :: exit 0
pnpm check:closed-ceremony-text :: exit 0
pnpm check:cross-package-test-inputs :: exit 0
pnpm check:dispatcher-error-vocabulary :: exit 0
pnpm check:doc-authoring :: exit 0
pnpm check:driver-memory-census :: exit 0
pnpm check:dts-closure :: exit 0
pnpm check:dual-build-cjs-loads :: exit 0
pnpm check:durability-log-level :: exit 0
pnpm check:engine-double-contract :: exit 0
pnpm check:entry-guard :: exit 0
pnpm check:error-status-conformance :: exit 0
pnpm check:gitlink-declared :: exit 0
pnpm check:i18n :: exit 0
pnpm check:i18n-stale-fill :: exit 0
pnpm check:issue-citations :: exit 0
pnpm check:keyed-text-bounds :: exit 0
pnpm check:lean-entry-closure :: exit 0
pnpm check:logger-receiver-detach :: exit 0
pnpm check:nul-bytes :: exit 0
pnpm check:objectql-double-limit :: exit 0
pnpm check:org-identifier :: exit 0
pnpm check:page-declaration-shape :: exit 0
pnpm check:parse-guard :: exit 0
pnpm check:platform-object-tenancy-census :: exit 0
pnpm check:pm-changeset-deadline-census :: exit 0
pnpm check:pnpm-filter-targets :: exit 0
pnpm check:pnpm-flag-spellings :: exit 0
pnpm check:published-files :: exit 0
pnpm check:query-options-erasure :: exit 0
pnpm check:refd-timer-probe :: exit 0
pnpm check:registry-log-declared :: exit 0
pnpm check:rest-log-spy-declared :: exit 0
pnpm check:scripts-symbol-anchors :: exit 0
pnpm check:slot-lookup :: exit 0
pnpm check:sourcemap-no-sources-content :: exit 0
pnpm check:system-context-census :: exit 0
pnpm check:test-source-alias :: exit 0
pnpm check:tier-file-adoption :: exit 0
pnpm check:type-check-coverage :: exit 0
pnpm check:type-check-debt :: exit 0
pnpm check:undeclared-dep-imports :: exit 0
pnpm check:watch-hint-literal :: exit 0
pnpm check:where-matcher :: exit 0
```
</details>
## Human Minimum
- [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic.
(No `packages/spec` change.)
- [x] 2. Every name I added uses the casing for its kind.
- [x] 3. No formatter ran; `pnpm lint` passes (eslint over both touched
packages, exit 0).
- [x] 4. One issue, one branch off an up-to-date `main`, one pull
request.
- [x] 5. A change that publishes carries a changeset, and both
declaration lines are filled in here and in the changeset.
- [x] 6. `packages/spec` touched: not touched.
- [x] 7. No hand edits to `content/docs/references/`,
`packages/*/CHANGELOG.md` or `content/docs/releases/`.
- [x] 8. Title, body and comments are in English (the 维护者速读 block is the
maintainer channel).
- [x] The *Before Submitting* list in CONTRIBUTING.md is done.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01FEkQ5nQQbWD6Bz9UduzNSs)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent d9bf777 commit a3009cb
7 files changed
Lines changed: 527 additions & 11 deletions
File tree
- .changeset
- packages
- plugins/plugin-approvals/src
- services/service-automation/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 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
251 | 251 | | |
252 | 252 | | |
253 | 253 | | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
254 | 270 | | |
255 | 271 | | |
256 | 272 | | |
| |||
4551 | 4567 | | |
4552 | 4568 | | |
4553 | 4569 | | |
| 4570 | + | |
| 4571 | + | |
| 4572 | + | |
| 4573 | + | |
| 4574 | + | |
| 4575 | + | |
| 4576 | + | |
| 4577 | + | |
4554 | 4578 | | |
4555 | 4579 | | |
4556 | 4580 | | |
| 4581 | + | |
| 4582 | + | |
4557 | 4583 | | |
4558 | 4584 | | |
4559 | 4585 | | |
| |||
4563 | 4589 | | |
4564 | 4590 | | |
4565 | 4591 | | |
| 4592 | + | |
4566 | 4593 | | |
4567 | 4594 | | |
| 4595 | + | |
| 4596 | + | |
| 4597 | + | |
4568 | 4598 | | |
4569 | 4599 | | |
4570 | 4600 | | |
| |||
4578 | 4608 | | |
4579 | 4609 | | |
4580 | 4610 | | |
| 4611 | + | |
| 4612 | + | |
| 4613 | + | |
| 4614 | + | |
| 4615 | + | |
| 4616 | + | |
| 4617 | + | |
4581 | 4618 | | |
4582 | 4619 | | |
4583 | 4620 | | |
| |||
| 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 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
0 commit comments