Repository navigation
Commit f966f01
fix(objectql)!: a hook's own write's stale-roll-up refusal never answers for the outer write; the unlanded-write refusal re-leads, declares 500, and never claims a failed rollback (#29519)
<!-- claude-projects-attribution: {"github_login":"os-steve"} -->
_Requested by **Steve** · [project
thread](https://claude.ai/code/project/chan_01FdL1XeUyvVNxLreh3F4Qi9?thread=cmsg_01FdL1XeUyvVNxLreh3F4Qi9Dsf26PgoqraAnprbfyBEiC)_
Fixes #29422
Fixes #29421
Clause-②: no (narrowing)
Metadata-protocol: none
Skills-docs: none — no file under `skills/**` or the hand-written docs
teaches what a write answers when a hook's own write, a cascade or a
transaction meets a roll-up that cannot be recomputed; the generated
reference pages only list the code
Narrowing note: the seat amended the claim from `Clause-②: no` once the
fix shape was chosen (option A on #29422, option B on #29421 item 2). A
write whose hook let its own write's stale-roll-up refusal escape
answered success with the hook's record. It now refuses, and an aborting
`after*` hook's unit rolls both rows back. The action door also answers
these refusals `500` instead of `400`. So the title carries `!`, full
dogfood ran locally, and the seat runs a contract review at tier.
## 维护者速读
- 改了什么:钩子(hook)通过 `ctx.api`
自己写的记录如果汇总重算失败(`ERR_SUMMARY_RECOMPUTE`),这个错误不再冒给外层写入的调用方当作"外层写入成功"。外层写入现在报内部错误(500
INTERNAL_ERROR),文案点名外层写入和钩子,原错误挂在 `cause`
上。另外三处顺带修正:级联多层时文案点名最外层对象;这类错误显式声明 `status: 500`、`code:
INTERNAL_ERROR`;事务回滚本身失败时不再说"已回滚"。
- 为什么改:主干实测 `POST /api/v1/data/case` 答 `201`,可返回的 id 和记录是钩子写的那条 line,不是
case。带 abort 钩子时整个单元还会把两行都提交。
- 风险与回滚:只改"嵌套写入的汇总失败、外层写入因此没完成"这一类答复。其它钩子错误照旧;`onError: 'log'`
的钩子照旧不让外层失败;外层写入自己的汇总失败照旧答 `ERR_SUMMARY_RECOMPUTE`。动作入口对这类错误从 400 改为
500,数据入口的响应体字节不变。没有新错误码,没有数据迁移,回滚就是整体 revert。
-
先看:`packages/objectql/src/summary-errors.ts`(`hookStoppedRefusal`、`UnlandedWriteRefusal`
类与重新领头、`rollbackFailedRefusal`);`packages/objectql/src/engine.ts` 的
`triggerHooks` 和 `dispatchUnscopedMultiWriteHooks` 两处 handler 调用,以及
`ObjectQL.transaction` 的回滚 catch。
- 两分钟验证:`pnpm --filter @objectstack/objectql exec vitest run
src/engine-hook-stale-summary-refusal.test.ts
src/engine-unlanded-refusal-lead.test.ts`,应看到 14 个测试全部通过。
- 需要你做的:无,席位落地(契约复核后)。
## What changes
**#29422: a hook's own write.** The one seam a hook handler's error
leaves through is `triggerHooks` (`await entry.handler(ctx)`); the
unscoped multi-write dispatch is its twin. At that seam a stale-roll-up
refusal is converted with `hookStoppedRefusal`. Before, the refusal
crossed it carrying the HOOK's write as `written`, so the outer door
answered the hook's record as its own result. Now the outer write
refuses with an internal failure that names it and the hook (`The insert
of "case" did not complete: its afterInsert hook "line_writer" stopped.
…`), and the refusal is kept as `cause`. Consequences:
- An aborting `after*` hook's unit (`inAfterAbortUnit`) no longer sees
`SummaryRecomputeError`, so it rolls both rows back, as
`HookSchema.onError` `abort` says. Only the outer write's OWN refusal
still reaches it and commits.
- With no unit (a code-registered hook, a driver without transactions, a
caller's unit), the text says that what was written before that point
stays written.
- `before*` hooks go through the same seam: the outer write never ran.
- Bounds the seat set: only the `ERR_SUMMARY_RECOMPUTE` family is
converted, and every other hook error passes through unchanged (pinned).
A hook declared `onError: 'log'` swallows it before the seam, so the
outer write still succeeds (pinned).
**#29421: the unlanded-write refusal** (`summary-errors.ts`):
1. **Lead.** The converted error is now a module-private class,
`UnlandedWriteRefusal`. When a cascade walk or a hook meets an earlier
conversion, it re-leads with its own write and keeps the original
refusal as `cause`. A three-level cascade's caller reads the root
object. A transaction does not re-lead: what escapes it already names
the write that did not complete.
2. **Structural status.** The class declares `status: 500` and `code:
'INTERNAL_ERROR'`. The data door's body is byte-identical (`500 { error:
"Internal server error", code: "INTERNAL_ERROR" }`), now declared
instead of reached by falling through every heuristic. The action door
now answers `500 INTERNAL_ERROR` instead of `400`, with the same
sentence. `status` alone was measured and rejected: a declared 5xx with
no code drops `code` from the data door's body.
3. **Rollback failure.** When `ObjectQL.transaction`'s rollback throws,
the refusal is `rollbackFailedRefusal` ("…its rollback failed too, so
what was written inside it may or may not be stored"), not "rolled
back". `ScopedContext.transaction` does not swallow a rollback failure,
so it is unchanged.
## Tests
- New `packages/objectql/src/engine-hook-stale-summary-refusal.test.ts`
(7): aborting `afterInsert`, `afterUpdate` and `afterDelete` (refused,
rolled back, nothing stored); a code-registered hook with no unit
(refused with the hedged text, both rows stored); `beforeInsert`; and
three CONTROLS: `onError: 'log'` succeeds, the outer write's own refusal
is still `ERR_SUMMARY_RECOMPUTE` and commits, and another hook error
passes through unchanged.
- New `packages/objectql/src/engine-unlanded-refusal-lead.test.ts` (7):
a three-level cascade where the root's caller reads the root; the middle
level's own caller; re-lead at every depth through cascade and hook
seams with `cause` kept; the rollback-throws hedge. CONTROLS: an
unrelated code-less error, the leaf's own refusal, and a successful
rollback.
- New `packages/rest/src/data-write-hook-stale-summary-refusal.test.ts`
(3, real SqlDriver): POST with a metadata-bound hook answers `500` with
the exact body and stores neither row; POST with a code hook answers the
same and stores both rows; CONTROL a line's own POST answers `201` with
its own row.
- **Forced test edits** (named):
`packages/objectql/src/engine-cascade-summary-rollback.test.ts`
(#29381's pin), where five `err.code` assertions go from undefined to
`'INTERNAL_ERROR'` plus `status` 500;
`packages/runtime/src/action-delete-cascade-summary-rollback.test.ts`,
where the action door goes 400 → 500 with `code: INTERNAL_ERROR` (the
seat accepted this); and
`packages/rest/src/data-delete-cascade-summary-rollback.test.ts`,
tightened from status + code to the whole body (the byte-identity pin
the seat asked for).
## Acceptance notes
- The pending #29381 changeset says the converted error has "no `code`".
This PR's changeset says it supersedes that sentence; the #29381
changeset was not edited.
- `ui/**` and `packages/client` were grepped for a branch on an action's
`400`. None reads it for these refusals. `classifyLoadError` is for data
fetches, `flowResponse.ts` keys on `FLOW_FAILED`, and the analytics
`400` arm is another door.
- Commit shape: one fix commit per card (61f49fa #29422, c594fd0
#29421), plus one REST door-pin commit (54eb565), the changeset commit
and the main merge.
## Verification
All readings on `objectstack-ai/objectstack` at head `b81dcfa4` (the
branch's three commits plus the changeset, merged with origin/main
`04e0c2c0`), after `pnpm install --frozen-lockfile` and a full `turbo
run build --filter='./packages/**'` (78/78).
<details><summary>Measurement on main before the fix (#29422)</summary>
Harness: RestServer → protocol → ObjectQL → SqlDriver (better-sqlite3),
on main `8ca9dd7b`, with a scratch test that was not committed. An
`afterInsert` hook on `case` writes a `line` through `ctx.api`, and the
line's roll-up refuses.
- `POST /data/case` with a metadata-bound hook answered `201
{object:"case", id: LINE_ID, record: LINE_ROW}`, where LINE_ID and
LINE_ROW are the id and row of the line the hook wrote (name
"from-hook"). Both rows were stored, because the unit committed.
- `POST /data/codecase` with a code-registered hook (no unit) gave the
same shape. Both rows were stored.
- The engine's read: `code ERR_SUMMARY_RECOMPUTE`, `written` = the line
row, failures `line -> bucket.memo_sum`.
- CONTROL `POST /data/line` answered `201` with its own row.
</details>
<details><summary>Item 2: door readings for the three shapes of the
converted error</summary>
| declares | data door `DELETE` | action door `POST` |
|---|---|---|
| nothing (#29381 shape) | 500 `{error, code: INTERNAL_ERROR}` | 400
`{error:{code: VALIDATION_ERROR, message, httpStatus:400}}` |
| `status: 500` only | 500 `{error}`, with `code` dropped | 500
`INTERNAL_ERROR` + sentence |
| `status: 500` + `code: INTERNAL_ERROR` (this PR) | 500 `{error, code:
INTERNAL_ERROR}`, byte-identical | 500 `INTERNAL_ERROR` + sentence |
</details>
<details><summary>Tests</summary>
- `pnpm --filter @objectstack/objectql test && pnpm --filter
@objectstack/objectql typecheck` (under the lock): VERDICT command-exit
0. Test Files 536 passed (536), Tests 10398 passed (10398); typecheck
exit 0.
- rest `src/data-delete-cascade-summary-rollback.test.ts
src/data-write-hook-stale-summary-refusal.test.ts
src/data-write-summary-recompute-refused.test.ts`: 3 files, 10 tests
passed. runtime `src/action-delete-cascade-summary-rollback.test.ts`: 2
tests passed.
- Full dogfood, `pnpm --filter @objectstack/dogfood test` under the
lock, with no shard and no path: VERDICT command-exit 0. Test Files 397
passed | 3 skipped (400), Tests 3277 passed | 28 skipped (3305).
- Lint: root eslint `--no-inline-config` on `packages/objectql/src` plus
the three door test files: exit 0, no output.
</details>
<details><summary>Ablations (each restored, with git diff HEAD empty, by
scripts/ablation-replace.mjs)</summary>
- **Hook seam** (`triggerHooks`: `throw hookStoppedRefusal(…)` → `throw
e`), engine pin: 4 failed | 3 passed. `AssertionError: expected
'ERR_SUMMARY_RECOMPUTE' to be undefined` on the three after* cases, the
code-hook case and `beforeInsert`. The three controls stay green.
- **Re-lead** (`relead && e instanceof UnlandedWriteRefusal ? e.cause :
e` → `e`): 2 failed | 5 passed. The three-level case reads `The delete
of "line" did not complete…`.
- **Rollback hedge** (`rollbackFailed ? rollbackFailedRefusal(err) :
rolledBackRefusal(err)` → `rolledBackRefusal(err)`): 1 failed | 6
passed. It reads `The transaction rolled back, so nothing written inside
it was stored.`
- **Declared code dropped** (status kept), objectql dist rebuilt, with
`ablation-dist-preflight` showing the marker present: data-door pins 3
failed (`expected { error: 'Internal server error' } to deeply equal {
error, code }`); the action pin stays green.
- **Status and code both dropped** (the #29381 shape), dist rebuilt,
marker present: data-door pins 5/5 green, which shows the body is
byte-identical to the old heuristic path. Action pin: 1 failed,
`expected 400 to be 500`, body code `VALIDATION_ERROR`.
- Restore leg: objectql dist rebuilt, and `ablation-dist-preflight
--absent` passed for both markers.
</details>
<details><summary>Gates: dispatch-gates --commands, 63 derived</summary>
- All 63 ran; `--ran` reconciliation: "63 derived famil(ies) accounted
for — 63 run, 0 NOT-MEASURED".
- Two first-pass exit 3s were re-run green. `check-plugin-teardown-shape
--self-test` needed its pinned fixture commit fetched.
`check:type-check-debt` had queued behind this branch's own dogfood lock
past its budget; on the re-run it was OK, 0 raw errors.
- CI families: see this PR's check-runs.
</details>
---
_Generated by [Claude
Code](https://claude.ai/code/session_01ADHkmPCSqt8uDw17RmX9hu)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent bd68248 commit f966f01
9 files changed
Lines changed: 754 additions & 29 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 | |
|---|---|---|---|
| |||
144 | 144 | | |
145 | 145 | | |
146 | 146 | | |
147 | | - | |
| 147 | + | |
| 148 | + | |
148 | 149 | | |
149 | 150 | | |
150 | 151 | | |
| |||
156 | 157 | | |
157 | 158 | | |
158 | 159 | | |
159 | | - | |
| 160 | + | |
| 161 | + | |
160 | 162 | | |
161 | 163 | | |
162 | 164 | | |
| |||
167 | 169 | | |
168 | 170 | | |
169 | 171 | | |
170 | | - | |
| 172 | + | |
| 173 | + | |
171 | 174 | | |
172 | 175 | | |
173 | 176 | | |
| |||
178 | 181 | | |
179 | 182 | | |
180 | 183 | | |
181 | | - | |
| 184 | + | |
| 185 | + | |
182 | 186 | | |
183 | 187 | | |
184 | 188 | | |
| |||
191 | 195 | | |
192 | 196 | | |
193 | 197 | | |
194 | | - | |
| 198 | + | |
| 199 | + | |
195 | 200 | | |
196 | 201 | | |
197 | 202 | | |
| |||
| 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 | + | |
0 commit comments