Repository navigation
Commit 3291213
fix(objectql)!: a nested write's stale-roll-up refusal never answers for a delete or transaction that did not land (#29381)
<!-- claude-projects-attribution: {"github_login":"os-steve"} -->
_Requested by **Steve** · [project
thread](https://claude.ai/code/project/chan_01FdL1XeUyvVNxLreh3F4Qi9?thread=cmsg_01FdL1XeUyvVNxLreh3F4Qi92cLDdfrzbfFte3TG7C44xC)_
Fixes #29291
Clause-②: no (narrowing)
Metadata-protocol: none
Skills-docs: none — no file under `skills/**` or the hand-written docs
teaches what a cascade delete or a rolled-back transaction answers when
a dependent's roll-up cannot be recomputed; the two generated reference
pages only list the code
Narrowing note: a delete or a transaction that answered success after
rolling back now refuses (`!` in the title, full dogfood, seat contract
review); claim amendment 6577647622 on #29291.
## 维护者速读
-
改了什么:级联删除里,某个子记录的汇总重算失败(`ERR_SUMMARY_RECOMPUTE`,意思是"这次写入已落库,只是汇总没算出来")不再原样冒给父记录删除的调用方;事务因这个错误回滚时也一样。两处都改成一个不带错误码的内部错误(数据入口答
500),原错误挂在 `cause` 上。
-
为什么改:数据入口、MCP、动作/沙箱/流程都把这个错误码当作"写入成功"。原子级联里子记录一失败整个事务回滚,父记录和子记录都还在,可调用方被告知删除成功(主干实测:`DELETE`
答 `200 { success: true }`,记录仍在)。
- 风险与回滚:只在"嵌套写入的汇总失败、外层写入没落地"时改答复;子记录自己的删除、事务里自己捕获后提交、加入外层事务的写入照旧答
`ERR_SUMMARY_RECOMPUTE`。无新错误码、无数据迁移,回滚即整体 revert。
- 先看:`packages/objectql/src/summary-errors.ts`(新增 `unlandedWriteRefusal`
及两个包装);`packages/objectql/src/engine.ts`(`runByIdDelete` /
`runPredicateDelete` 的级联调用,以及 `ObjectQL.transaction` 与
`ScopedContext.transaction` 的回滚 catch)。
- 两分钟验证:`pnpm --filter @objectstack/objectql exec vitest run
src/engine-cascade-summary-rollback.test.ts`,应看到 7 个测试全部通过;动作入口另见
`packages/runtime/src/action-delete-cascade-summary-rollback.test.ts`(2
个,动作体内 `ctx.engine.delete` 答 400 拒绝而非成功)。
- 需要你做的:无,席位落地。
## What changes
`SummaryRecomputeError` (`ERR_SUMMARY_RECOMPUTE`) means "this write is
stored, only a parent roll-up recompute failed", and every answering
surface turns it into the write's own success. Two engine units carried
a nested write's refusal past the point where it stopped being true.
Both now convert it at the engine, so no answering surface changes:
- **The cascade walk** (`packages/objectql/src/engine.ts`, the
`cascadeDeleteRelations` call in `runByIdDelete` and in
`runPredicateDelete`). A dependent's refusal stops the walk before the
parent's own row is removed, so it is never the parent delete's answer.
It becomes `cascadeStoppedRefusal(e, object)`. This covers every plan:
the atomic plan (the owning transaction then rolls back), a joined
transaction, the `'split'` plan and a runtime that cannot roll back.
- **An owned transaction that rolls back** (`ObjectQL.transaction` and
`ScopedContext.transaction`, the catch after the rollback). A refusal
escaping the callback is no longer true once every write in it is
undone. It becomes `rolledBackRefusal(e)`.
- **One meaning, one place**
(`packages/objectql/src/summary-errors.ts`). `unlandedWriteRefusal` is
the single conversion: a code-less `Error` (an unrecomputable roll-up is
an internal failure, so the data doors answer `500 INTERNAL_ERROR`). Its
message names the unit that did not complete and the stale summary
(`PARENT.FIELD`), and the original refusal is kept as `cause`. Every
other error passes through unchanged. It matches by `code`, as every
other reader of the refusal does. Nothing new is exported, and there is
no new error code or member.
- **Unchanged, by design:** the refusal anywhere it is still true. That
covers a dependent's own delete, a write outside any unit, a transaction
that catches it and commits (the `inAfterAbortUnit` commit-then-rethrow
included), and a joined transaction, whose owner decides.
- **No source change outside the engine.** PR #29283 (merged as
`153869d7`) made the action, sandbox and flow writers answer a landed
write's refusal as success; they read the engine's answer, so the
conversion reaches them with no edit there.
- **Changeset:**
`.changeset/29291-cascade-summary-refusal-rolled-back.md`,
`@objectstack/objectql` patch, BREAKING (narrowing), `adr-0087:
not-required (no-migration-prescription)`.
## Why
Measured on `origin/main` `a19759ac` before the fix, with
`engine-cascade-summary-rollback.test.ts` (a stub driver that really
rolls back; a two-level `master_detail` cascade where each line's delete
recomputes a roll-up on another parent, and that parent's update is
refused):
| case | stored after | caller was told |
|---|---|---|
| atomic plan, delete the account | account and both lines (rolled back,
1 begin / 1 rollback) | `ERR_SUMMARY_RECOMPUTE`, `written: true` (the
line's result) |
| runtime that cannot roll back | account and line 2; **line 1 deleted**
| `ERR_SUMMARY_RECOMPUTE`, `written: true` |
| caller's own `transaction()` around a line delete | both lines (rolled
back) | `ERR_SUMMARY_RECOMPUTE`, `written: true` |
At the REST door on a real `SqlDriver` (better-sqlite3), `DELETE
/api/v1/data/qcr_account/a1` answered `200
{"object":"qcr_account","id":"a1","success":true}` while the account and
all three lines were still stored.
**The non-atomic plan, as the delta asked.** On a runtime that cannot
roll back, the dependent the walk reached first stays deleted (its own
write landed) and the parent and every later dependent stay stored. The
parent's own write did not land, so the ruling's first sentence covers
it without any reading of my own: the parent's caller now gets the same
refusal. The message says that dependents written before the stop stay
written unless a transaction rolls them back. What lands is unchanged.
Whether the walk should instead tolerate a dependent's stale summary and
finish the delete is a widening the ruling does not cover; see
Acceptance notes.
## Tests
Three new files, red first (no existing test was edited; no forced test
edits):
- `packages/objectql/src/engine-cascade-summary-rollback.test.ts` (7).
Refusal pins assert `code` undefined and `cause.code ===
'ERR_SUMMARY_RECOMPUTE'`, plus what is stored:
- atomic plan by id: refused, 1 rollback, account and lines stored;
- a runtime that cannot roll back, by id: refused, account stored, line
1 gone (the measured reading);
- the same by predicate (`multi: true`): refused, account stored;
- a caller's engine `transaction()` the refusal rolls back: refused,
nothing stored;
- the same through a scoped context's `transaction()` (`ctx.api`);
- CONTROL the line's own delete: `ERR_SUMMARY_RECOMPUTE`, `written:
true`, line gone;
- CONTROL a caller's transaction that catches the refusal and commits:
the refusal stays, 0 rollbacks, line gone.
- Red on base: 3 of the first 5 (the predicate and scoped-context cases
were added after the fix; each is shown red by its own ablation below).
- `packages/rest/src/data-delete-cascade-summary-rollback.test.ts` (2).
The composed REST harness of
`data-write-summary-recompute-refused.test.ts` on a real `SqlDriver`:
`DELETE` of the account answers `500 INTERNAL_ERROR` with the account
and all lines stored; CONTROL `DELETE` of a line answers `200 { success:
true }` and the line is gone. Red on base (objectql `dist` proven to be
main's): 1 of 2, `expected 200 to be 500`.
- `packages/runtime/src/action-delete-cascade-summary-rollback.test.ts`
(2), added after PR #29283 merged. The real stack of
`dispatcher-unique-violation-wire-code.test.ts` (`HttpDispatcher` →
`POST /api/v1/actions/:object/:action` → `ctx.engine` → `ObjectQL` →
`SqlDriver` on better-sqlite3), same cascade shape: an action whose body
runs `ctx.engine.delete('qca_account', 'a1')` answers the action door's
`400` refusal (`success: false`, the engine's message), the body's next
line never runs, and the account and all three lines are still stored.
CONTROL an action deleting a line itself answers `200` with its body's
result and the line is gone. Red with the conversion ablated (AB0-action
below): `expected 200 to be 400`, body
`{"success":true,"data":{"deleted":"a1"}}`, the bug as reported. The
door answers `400` because it answers every throw out of an action body
that way (a hook's or body's own `throw` is a deliberate 4xx); this PR
changes nothing there.
<details><summary>Ablations: each mutation landed with
<code>ablation-replace</code>, red, restored to blob == HEAD</summary>
| # | mutation | file | red |
|---|---|---|---|
| AB0 | `unlandedWriteRefusal` passes every error through (built;
`ablation-dist-preflight` marker present in 4 dist files; restored,
rebuilt, `--absent` and tree clean) | `summary-errors.ts` | engine 5 of
7 (every refusal pin, both controls green); REST 1 of 2 |
| AB0-action | the same mutation, on `2af3815d` (built; marker present
in 4 dist files; restored, rebuilt, `--absent` and tree clean) |
`summary-errors.ts` | action pin 1 of 2 (`expected 200 to be 400`);
CONTROL green; green again after restore |
| AB1 | by-id walk without `cascadeStoppedRefusal` | `engine.ts` | 1 of
7 (no-rollback by id; the atomic case stays green through the
transaction conversion) |
| AB2 | predicate walk without it | `engine.ts` | 1 of 7 (predicate) |
| AB3 | `ObjectQL.transaction` rollback without `rolledBackRefusal` |
`engine.ts` | 1 of 7 (caller's transaction) |
| AB4 | `ScopedContext.transaction` rollback without it | `engine.ts` |
1 of 7 (scoped context) |
AB1 to AB4 ran on `a5bc9c90` plus the predicate pin (committed as the
next commit), and AB0 on that commit. The first AB0 attempt used a
comment marker, which tsup strips, so the dist preflight could not see
it (the red reading was the same). It was rerun with a string-literal
marker, and the counts above are from that rerun.
</details>
## Acceptance notes
- **Declared as narrowing** (the declaration line above; claim amendment
6577647622). A delete whose cascade a dependent's failing roll-up
stopped, and a transaction that refusal rolled back, answered success
(`ERR_SUMMARY_RECOMPUTE`, read as success by every door) and now answer
`500 INTERNAL_ERROR` at the data door and `400` at the action door. The
published `@objectstack/objectql` behaves differently on that answer;
the data that lands is unchanged.
- **The 500 body is the door's sanitised `{"error":"Internal server
error","code":"INTERNAL_ERROR"}`.** The message (what did not complete,
which summary is stale, the fix) reaches the server log and `cause`. It
names the object in double quotes on purpose: a single-quoted object
name beside a "not" is read by the data door's unknown-object heuristic
(`data-error-classification.ts`) as `404 OBJECT_NOT_FOUND`. The first
green leg measured exactly that (`404 OBJECT_NOT_FOUND`), and the quotes
were changed before the pin was finalised.
- **Out of reach on purpose:** the discrete `beginTransaction` /
`commit` / `rollback` trio on `ScopedContext` (the QuickJS sandbox's
`ctx.api.transaction`). The caller drives the rollback there, so the
engine never sees the refusal leave the unit. Since PR #29283 (merged as
`153869d7`), a sandbox body's write already answers its own success
inside such a unit. Also unchanged is the degrade path of
`transaction()` on a runtime that cannot roll back, where every write
before the refusing one, and the refusing one itself, did land.
- **Noted, not filed (read-only inference, not measured):** an `after*`
hook whose `ctx.api` write raises the refusal. The outer write's abort
unit (`inAfterAbortUnit`) catches it, commits and rethrows, so both
writes did land and the code is true. The `written` the caller reads,
though, is the hook write's result, not the outer write's.
- **The non-atomic alternative left open:** tolerating a dependent's
stale summary, so that the walk continues and the parent delete lands
with `ERR_SUMMARY_RECOMPUTE`, would turn refused deletes into landed
ones (widening). The ruling does not cover it, so it is not done here;
see `open_questions` in the card report.
- **PR #29283 (#28861) merged as `153869d7`** while this PR was open.
`main` is merged in (`8fc7f886`) and the one pin through an action
surface the seat asked for is added (the runtime file above).
- No `packages/spec/**`, `ui/**`, governed or workflow file.
## Verification
<details><summary>Suites and typecheck</summary>
- `@objectstack/objectql` full local suite on `f1eec6fd` (main merged at
`504ba510`): 529 of 529 files, 10266 of 10266 tests; `typecheck` (`tsc
--noEmit` + `check:test-typecheck`): exit 0. `os-verify-lock` VERDICT
command-exit 0 for the whole chain.
- `@objectstack/rest` full local suite on `f1eec6fd`: 368 of 368 files,
6976 passed and 332 skipped; `typecheck`: exit 0.
- Final head `2af3815d` (main merged at `8fc7f886`, then the action
pin): the three new files re-run green (objectql 7 of 7, rest 2 of 2,
runtime 2 of 2), and `@objectstack/runtime` `typecheck` (`tsc --noEmit`
+ `check:test-typecheck`) exit 0. The objectql and rest full suites
above are not re-run: since `f1eec6fd` this branch changed only test
files and the changeset, and main's own changes are covered by the
dogfood below.
- **Full dogfood on the final head `2af3815d`** (rerun, because merging
main changed code): closure built (`turbo run build
--filter='@objectstack/dogfood^...'`, 68 of 68), then `pnpm --filter
@objectstack/dogfood test` under the lock: VERDICT command-exit 0
(34m19s, shared box), Test Files 393 passed and 3 skipped (396), Tests
3243 passed and 28 skipped (3271). The earlier run on `f1eec6fd` was
also green (391 files, 3223 tests).
</details>
<details><summary>Gates</summary>
On the final head `2af3815d`, run after the dogfood closure build
(unlocked):
- `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` derived 63 commands (the same 63 on
`f1eec6fd`, `4dd59050` and `2af3815d`). Each ran with `< /dev/null`.
`--ran` reconciles **63 derived, 63 run, 0 NOT-MEASURED, 0 UNRUN** (exit
0), and all 63 exit 0. `check:dual-build-cjs-loads` first exited 3
(prerequisite: some package dists missing) and is recorded from its
re-run after `turbo run build --filter='./packages/**'` (78 of 78, all
cache hits): exit 0. On round 1 the four changeset-reading gates
(`check-adr-0087-registration`, `check-changeset-no-major`,
`check-empty-changeset`, `check:pm-changeset-deadline-census`) were
re-run after the last changeset edit, and all exit 0.
- First pass, on `f1eec6fd`: two real reds, both in my new test files,
fixed in `4dd59050` rather than by any baseline entry.
`check:query-options-erasure` (test surface 236 → 237: an `as any` on
the REST pin's `engine.find` options; typed now, and the engine pin's
`delete` options are no longer cast). `check:rest-log-spy-declared` (the
REST pin spied on `console` with no `OS_REST_LOG` level; the spies are
removed, since the pin asserts no log). Two prerequisite exit-3s cleared
on re-run: `check-plugin-teardown-shape --self-test` after `git fetch
--depth=1 origin 621a487…`, and `check:dual-build-cjs-loads` after the
build.
- `pnpm lint`: exit 0 on `2af3815d` (`eslint . --no-inline-config`).
- PR body offline: `check-changeset-no-major --base origin/main --event`
(this body and title) exit 0, LEVEL AXIS `no (narrowing)` + `none` at
`patch`. `check-closing-keyword-parity --body`: only `Fixes #29291`
binds.
</details>
## Human Minimum
- [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic.
(no spec edit)
- [x] 2. Every name I added uses the casing for its kind.
- [x] 3. No formatter ran; `pnpm lint` passes.
- [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: n/a.
- [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.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01ADHkmQQoWsHzMSsiaM89mr)_
<!-- signed: domain:server#3 · os-steve ·
session_01ADHkmQQoWsHzMSsiaM89mr -->
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 0e0ddac commit 3291213
6 files changed
Lines changed: 618 additions & 8 deletions
File tree
- .changeset
- packages
- objectql/src
- rest/src
- runtime/src
| 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 | + | |
| 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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
201 | 201 | | |
202 | 202 | | |
203 | 203 | | |
204 | | - | |
| 204 | + | |
205 | 205 | | |
206 | 206 | | |
207 | 207 | | |
| |||
20968 | 20968 | | |
20969 | 20969 | | |
20970 | 20970 | | |
| 20971 | + | |
| 20972 | + | |
| 20973 | + | |
| 20974 | + | |
20971 | 20975 | | |
20972 | | - | |
| 20976 | + | |
| 20977 | + | |
20973 | 20978 | | |
20974 | 20979 | | |
20975 | 20980 | | |
| |||
21061 | 21066 | | |
21062 | 21067 | | |
21063 | 21068 | | |
21064 | | - | |
21065 | | - | |
| 21069 | + | |
| 21070 | + | |
| 21071 | + | |
| 21072 | + | |
| 21073 | + | |
21066 | 21074 | | |
21067 | 21075 | | |
21068 | 21076 | | |
| |||
21915 | 21923 | | |
21916 | 21924 | | |
21917 | 21925 | | |
21918 | | - | |
21919 | | - | |
| 21926 | + | |
| 21927 | + | |
| 21928 | + | |
21920 | 21929 | | |
21921 | 21930 | | |
21922 | 21931 | | |
| |||
22906 | 22915 | | |
22907 | 22916 | | |
22908 | 22917 | | |
22909 | | - | |
22910 | | - | |
| 22918 | + | |
| 22919 | + | |
| 22920 | + | |
22911 | 22921 | | |
22912 | 22922 | | |
22913 | 22923 | | |
| |||
0 commit comments