Repository navigation
fix(objectql): an aborting after* hook rolls its write back on the default write door - #22819
Conversation
A plain write opened no unit of work, so an after* hook whose onError is abort (the default) refused the caller while the row stayed stored. The write door now opens a unit for exactly that case. Claude-Session: https://claude.ai/code/session_01JfJfBUC3cQ6hhgm9MQK76T Co-authored-by: Claude <noreply@anthropic.com>
Work a unit starts and does not await (an async: true hook) inherited the finished handle and lost its write. Measured on a booted stack. Claude-Session: https://claude.ai/code/session_01JfJfBUC3cQ6hhgm9MQK76T Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JfJfBUC3cQ6hhgm9MQK76T Co-authored-by: Claude <noreply@anthropic.com>
…e door re-enters its own unit The system-context census anchors elevation reads by symbol, so the bodies keep their names: the door asks opensAfterAbortUnit, and the unit re-enters the same method, which joins it. Claude-Session: https://claude.ai/code/session_01JfJfBUC3cQ6hhgm9MQK76T Co-authored-by: Claude <noreply@anthropic.com>
…caller's limit Claude-Session: https://claude.ai/code/session_01JfJfBUC3cQ6hhgm9MQK76T Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 2 package(s): 14 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 4 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 140 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 9616b56e772038ab4430ceeb37d5baf4d19fd3c8 && git checkout 9616b56e772038ab4430ceeb37d5baf4d19fd3c8
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 31b5a5f7f51b7a7fd1131299a5b1dbe95910bc4f a1ed7388631a7376b9df230401550a1be5b8de6f && git checkout -B drift-repro 31b5a5f7f51b7a7fd1131299a5b1dbe95910bc4f && git merge --no-ff a1ed7388631a7376b9df230401550a1be5b8de6f
node scripts/docs-audit/affected-docs.mjs --json 31b5a5f7f51b7a7fd1131299a5b1dbe95910bc4f
|
Text only: HookEvent's JSDoc gains the write door's unit as the fourth way a write ends up inside a unit of work; onError's TSDoc and describe() say where abort rolls back and what it means elsewhere. No shape, key, default or enum change. Claude-Session: https://claude.ai/code/session_01JfJfBUC3cQ6hhgm9MQK76T Co-authored-by: Claude <noreply@anthropic.com>
…for the onError text Regenerated with check:generated --fix (gen:docs only, the one artifact it proved stale). Claude-Session: https://claude.ai/code/session_01JfJfBUC3cQ6hhgm9MQK76T Co-authored-by: Claude <noreply@anthropic.com>
…door's unit A sandboxed afterInsert body declaring no onError aborts, so the write door opens the unit, the body's ctx.api.transaction joins it and the abort rolls the outer row back too. The #6406 subject stays asserted: one begin, the body's write on that handle, one rollback (the owner's). The sibling case's comment says the begin is now the door's; its assertions are unchanged. Claude-Session: https://claude.ai/code/session_01JfJfBUC3cQ6hhgm9MQK76T Co-authored-by: Claude <noreply@anthropic.com>
Contract reviewServed-tier: Inputs read: card #22782 (body; triage 6106449072; claim 6106744769; os-dev-reports 6107830381, 6108597601, 6108994502), PR #22819 (body, 9-file list, net diff Check-runs on this head: 42, all ① Derived judgmentsThe unit-opening predicate (
Re-entry design (
Nesting — right. Inside a caller-opened Q1 — cross-datasource refusal — right (ruled A, declared 6107851318/6108611359 context). No new rule: Spec text against the code — matches. The runtime re-pin — a contract change, not a weakening; right. The old case made 4 observable assertions (begins 1, rollbacks 1, commits 0, Hand-written docs named by the docs-drift comment — one page still states the OLD behaviour, and that is wrong. Of the 14 pages listed in 6107821481, three state
② Semver level
③ Boundary flagsDeviations and open questions across the three reports — all answered or escalated:
The four out-of-scope findings of round 1:
The 444-line count: +444/−19 against the 300 suggestion. Source 72 ≤ 80. The excess is 286 test lines over two layers (the triage's four pins plus the measurement pins, and the real-door file with the regression the unit introduced), 36 for the runtime re-pin, 31 of changeset text. Proportionate to a stored-outcome change on a published door.
Unmeasured edge, for the seat's note (not a FAIL carrier): inside the door's unit a summary recompute runs enrolled; on a SQL driver a STATEMENT-level recompute failure (a deadlock or serialization failure on PostgreSQL) leaves the transaction aborted, so the subsequent Remediation for round 4 (the verdict rests on R1 and R3):
Everything else judged above is right: the predicate, the re-entry, the commit-then-rethrow, the ambient close, the nesting, Q1, the spec text, the re-pin, both semver levels and both Implemented-by: VERDICT: FAIL Read at 2026-10-11T13:25Z by the reviewer, read-only, from the shared checkout at Generated by Claude Code |
…-flow, events); changeset cites ADR-0087's launch-window exemption data-flow.mdx's lifecycle table said an after* hook cannot abort (record is saved); events.mdx gains where the rollback holds. The changeset's provenance for the minor-for-breaking convention named ADR-0131, which decides organization ownership; it now names ADR-0087's ratified launch-window exemption (amended 2026-09-13). The runtime sibling case's title states the door's unit; its assertions are unchanged. Claude-Session: https://claude.ai/code/session_01JfJfBUC3cQ6hhgm9MQK76T Co-authored-by: Claude <noreply@anthropic.com>
Contract reviewServed-tier: Re-review after the FAIL record 6109504950 on Check-runs on this head: 42, all ① Derived judgmentsThe delta is the four one-line changes the report names, and nothing else — right. R1 — R2 — R3 — the changeset citation — right. Line 11 now reads "(ADR-0087, the ratified pre-launch launch-window exemption, as amended 2026-09-13, #18003)". On R4 — the sibling case's title — right in what it names; it says more than the case pins. The title is now "with NO host transaction the door's unit owns the one begin, the sandbox body joins it, and the door commits both rows (#22782)". Its four assertions are unchanged: Re-check of the docs-drift pages (6107821481) — no hand-written page still states the old ② Semver levelUnchanged. The delta touches no package source and no test assertion, and only the citation parenthetical in the objectql changeset. ③ Boundary flagsRound 4's deviations: none reported, and none found. The delta is text-only (4 lines); no rebase or force. The PR body was edited once through the relay: it now carries the AGENTS.md footer form (rule, one Round 1's (b) and (c), measured in round 4 and filed as #22855 / #22856 (not read here):
R6, recommended (from R4): add to the sibling case the three handle-identity lines its neighbour already has —
Standing from the prior record, not re-judged: the unmeasured SQL statement-level recompute edge inside the door's unit (a PG probe for the seat's note); round 1–3 deviations and the Q1–Q3 rulings. Governance: the 11-file list touches no governed surface ( Implemented-by: VERDICT: PASS Read at 2026-10-11T14:24Z by the reviewer, read-only, from the shared checkout at |
…nization's administrators; retire the unstaffed arm (#22850) Fixes #22824 Clause-②: yes An approval node whose approvers resolve to no concrete person now opens, under the default `onEmptyApprovers: 'admin_rescue'`, on the request organization's administrators, as ordinary approvers in the ordinary inbox. The `unstaffed` queue arm that #22775 added (never published) retires in the same stroke, since the named slate makes it obsolete. Route P1, as ruled (maintainer 「同意 P1」, recorded as `6108375797` on objectstack-ai/hotcrm#2057); triage `6108769161`; claim `6108865702`. ## FROM → TO: who receives an `admin_rescue` request - **FROM:** the request opened on a slate that named no one (the literal `position:NAME`). Measured on this branch's base `efcbac73c`, on a real boot (`bootStack`, the seeded admin owning the default organization): the stored slate was `["position:unstaffed_arm_nobody"]`, the admin's My Pending (`status=pending&approverId=ADMIN_ID,EMAIL`) answered `data: [], total: 0`, their `GET /notifications?type=approval.requested` answered `notifications: [], unreadCount: 0`, and no `sys_inbox_message` row was written. Only the #3424 override could decide it. - **TO:** the request opens on the holders of the `TENANT_ADMIN` rung of the request's organization: the same authority the tenant arm of `isOverrideActor` trusts (ADR-0095 D3, `ORGANIZATION_ADMIN_GRANTS`), resolved to people. There is no second notion of "admin". ## What changed **`@objectstack/plugin-approvals`** (`approval-service.ts`) - `organizationAdministrators(org)`: the candidates are the organization's members (`sys_member.organization_id`, this file's existing answer to "who is in this organization"). Each is asked the rung question over the grants `resolveUserAuthzGrants` (`@objectstack/core`) resolves for them with that organization as the active tenant: the resolver every authorization envelope comes from, so grant windows, deactivated sets, other organizations' scoped grants and position-bound sets answer exactly as when the person acts. No `plugin-security` edit; no better-auth role is read as authority. - `holdsTenantAdminRung(permissions)` is now the one spelling of the grant read, shared by the tenant arm of `isOverrideActor` (a mechanical swap of the identical expression; its authority does not change) and by the slate. - Platform administrators are not added for being platform administrators. One who also holds the organization's rung (its owner, say) is on the slate for that rung. - At node entry, an empty slate under `admin_rescue` (and a `fallback` that found no one either, which already degraded to `admin_rescue`) goes through the SAME `expandApprovers` as `approvers`, as `user` entries in one synthetic group (`#0`). So OOO delegation, the `approverId` index, the `approval.requested` fan-out and `decide` apply unchanged, and a decision by one of them is a slot decision recorded with `via_override: false`. - The request says why. The abandoned slate and the reason are kept in `__resolvedFrom` as `admin_rescue:abandoned` / `admin_rescue:reason` (the `fallback` branch's flat, policy-prefixed shape), and the timeline carries one system note (`action: 'comment'`, `actor_id: null`, the shape the OOO hop row already uses), for example "Routed to this organization's administrators because the approver slate named no one: position 'sales_manager' has no holder." - Break-glass stays break-glass. An organization with no administrator, a request with no organization, or an administrator read that fails opens on the slate that names no one, logged as a warning that names which of the three and the fix. A read failure answers a failure, never an invented "nobody". - The `unstaffed` arm retires: `awaitingRequestIds` and `unstaffedOverridableRequestIds` are removed, and `listRequests` / `countRequests` are byte-identical to their pre-#22775 bodies (compared function by function). `slateNamesAPerson` stays (the open path reads it); its docblock no longer names the arm. **`@objectstack/spec`**: `IApprovalService.listRequests`' `unstaffed?: boolean` member retires with its docblock. The `limit` docblock keeps #22775's correction minus the arm. The `onEmptyApprovers` `.describe()` text and TSDoc say that `admin_rescue` opens on the organization's administrators and that `fallback` names who decides instead. `content/docs/references/automation/approval.mdx` is regenerated (`check:generated --fix`, `check:docs` only). **`@objectstack/rest`**: `unstaffed` leaves `APPROVAL_REQUEST_LIST_PARAMS` and the multiplicity set, and `readDeclaredQueryBoolean` (its only reader) goes. The file equals the exact reverse-apply of #22775's hunks onto `efcbac73c` (`git apply -R`, then `cmp`). `?unstaffed=` is now refused as an unknown name, `400` with nested `error.code: VALIDATION_ERROR`, before the service runs. **`@objectstack/lint`**: `approval-approvers-may-resolve-empty` no longer says "waits forever". Both arms say the request opens on the organization's administrators. The hints name `onEmptyApprovers: 'fallback'` as the way to name who decides, and the `os explain` paragraph says the same. Severity, firing conditions and rule ids are unchanged. **Docs** (`approvals.mdx`): a new "When a slate names no one" section. The manager callout, the `onEmptyApprovers` list, the fallback text, the notification callout and the admin-override callout now describe the administrators' slate. The `unstaffed` filter paragraph and curl are gone. ## The census, first No shipped consumer of the `unstaffed` member or `?unstaffed` exists outside #22775's own files: - this repository at `efcbac73c`: every code hit of `unstaffed:`, `?unstaffed`, `.unstaffed` and `'unstaffed'` is in #22775's file list. The only non-test callers of `listRequests` / `countRequests` are the REST handler's two calls; - objectstack-ai/objectui `origin/main` `1d1cfe0` (read only): `approvalsApi.ts` builds the list query from `status / object / recordId / approverId / submitterId / q / limit / offset` and never sends `unstaffed`. Every other `unstaffed` hit there is the console's own seat label (`approverIdentity.ts`'s `unstaffed: boolean` chip state), not the filter. - Not published: `npm pack @objectstack/{spec,rest,plugin-approvals}@17.7.0` (npm `latest` is `17.7.0`) holds 0 files matching `unstaffed?: boolean | unstaffedOverridableRequestIds | readDeclaredQueryBoolean`; control: each tarball holds 3–5 files naming `listRequests`. ## Mechanism assumptions, measured 1. **Holds.** See FROM above: the real boot at `efcbac73c` stored literals only, listed and counted nothing for the admin, and notified no one. 2. **Partly.** `ORGANIZATION_ADMIN_GRANTS` is declared in `packages/spec/src/identity/eval-user.zod.ts`. NO reader resolves "the users holding that rung in organization X": every reader is per user (`resolveUserAuthzGrants`, `derivePosture`, the explain engine). So the slate composes the per-user resolver over the organization's members. Nothing in `plugin-security` was edited. 3. **Partly false.** The `fallback` branch keeps the primary slate's resolution INPUTS (`field:NAME`, `expression#N`) and namespaces its own as `fallback:KEY`. It keeps neither the abandoned slate nor a reason, and a position-only slate writes nothing there (measured: `__resolvedFrom` absent on the boot above). The new keys mirror that flat, prefixed shape (`admin_rescue:abandoned`, `admin_rescue:reason`); there is no second snapshot shape. 4. **Holds.** The member never shipped (the tarball reading above). `check:adr-0087-registration --base origin/main`: exit 0. The new changeset declares no breaking arm, so no disposition marker is owed, and the deleted 22725 row is not a judged status. See Changesets for the one gate that stays red. ## Pins `src/admin-rescue-named-slate.integration.test.ts`: the real `ApprovalService` on a real `ObjectQL` over `SqlDriver` (better-sqlite3), the real `resolveUserAuthzGrants` over a bound security catalog, and the real `listRequests` / `countRequests` / `getRequest` / `decide` / messaging fan-out: - a position nobody holds opens on the two administrators (`organization_admin`, `organization_admin_no_bypass`), each one's My Pending lists it with `total: 1` and `viewer.can_act: true`, and each is told once; - the scene anchored on that slate: a member sees nothing and is told nothing, and a platform admin (unscoped `admin_full_access`) and another organization's administrator are not on it; - a tenant administrator's decision: `finalized: true`, `approved`, `acted_as` their id, `via_override` false; - the timeline note and `__resolvedFrom` (exact keys and reason); - a `fallback` that resolves to no one degrades to the administrators and says so in the reason; - an administrator out of office is covered by their delegate (`approval.ooo_substituted`), and is not told twice; - CONTROLS: a staffed slate is unchanged (no administrator, no note); `fallback` opens on its own list; `auto_approve` opens nothing and `fail` refuses (`NO_APPROVERS`); an organization with no administrator gets the slate that names no one, logs it, tells no one, and the platform admin's decision there is an override (`via_override` true). `packages/qa/dogfood/test/approval-admin-rescue-slate.dogfood.test.ts` (the #22775 dogfood pin, rewritten to the named slate) on a real boot, through the REST door: the seeded admin owns the default organization, and a second member is promoted by a `sys_member` role write, which provisions their capability. Both administrators find the request in My Pending (`total: 1`, `can_act: true`) and hold one `approval.requested` notification. A member sees and holds nothing. The staffed control is unchanged. An administrator's `POST …/approve` is an approval with `via_override` false and the routing note on the timeline. `?unstaffed=true` answers `400 VALIDATION_ERROR`. **Retired with the code they pinned**, ⛔ none skipped or disabled: the 9-case `ApprovalService — the unstaffed arm (#22725)` block in `approval-service.test.ts` (the file equals the exact reverse-apply of #22775's hunk), the REST §4 `unstaffed` block (replaced by one refusal pin: `?unstaffed=true|false` answers the full ADR-0112 unknown-name refusal and neither service method runs), and the dogfood arm pin (rewritten above). ## Ablation (one-time, from committed `f3ff683a1`) `node scripts/ablation-replace.mjs` replaced `if (administrators.length) {` with `if (false && administrators.length) {` (anchor 1 → 0, blob `259e3e834b00` → `551f2027d03b`), wrapped in a `trap` restoring `git checkout HEAD --` on an absolute path. The integration file then had **6 failed / 4 passed**: every named-slate case red, and the four controls (staffed, fallback, auto_approve / fail, and the leaderless organization with its override) green. Restore proven: blob `259e3e834b00` equals HEAD's, and `git diff HEAD` is 0 bytes. The integration test imports the service from `src`, so no `dist` hop applies. The dogfood pin was not ablated (it reads `plugin-approvals` through `dist`). ## Suites (final runs at `06f9948c8` unless noted) - `@objectstack/plugin-approvals`: `test` 73 files / 1035 passed; `typecheck` exit 0 (`check:test-typecheck` OK, the test layer included). - dogfood, 12 approval-reading files (the new pin, override-composite, my-pending, position-address-readers, flow-node-config, notification-body, activity-text-predicate, both snapshot pins, lookup-title-exposure, seed-ownership-claim-dispatch, audit-log-admin-search): 12 files / 43 passed. `dogfood` `typecheck` exit 0, and its `tsc --listFiles` lists the new pin and fixtures. - `@objectstack/rest` (at `23a63a18a`; `rest` is unchanged since): `test` 272 files / 5239 passed, 326 skipped; `typecheck` exit 0. - `@objectstack/lint` (at `23a63a18a`; unchanged since): `test` 137 files / 6416 passed; `typecheck` exit 0. - `@objectstack/spec` (at `f3ff683a1`; unchanged since): `typecheck` exit 0; `test` 648 files / 19342 passed; `check:generated` all artifacts up to date after the one `gen:docs` regeneration. - The Dogfood Regression Gate's full population is left to CI. ## Gates (at `06f9948c8`, the final commit) - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived **116** families (32 paths against merge base `efcbac73c`). Each was run with its exit code recorded to disk before any pipe: **115 exit 0, 1 exit 1**. The one red is `check-empty-changeset.mjs --base origin/main`, the deliberate correction below. `--ran` reconciliation: "116 derived, 116 run, 0 NOT-MEASURED, 0 UNRUN". - Two families first answered `PREREQUISITE NOT MET` (exit 3) on unbuilt packages: `check:skill-examples` (client-react) and `check:dual-build-cjs-loads`. After those packages were built, both exit 0 in the final run. In an earlier run, `check:query-options-erasure` caught two `as any` option erasures this branch had added, and `check-tenant-audit-census` caught the new write call site. Both were fixed (commits `06f9948c8`, `8af84ef80`) and both exit 0 in the final run. - Lint, narrowed and proven: eslint's population is `**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}` (`eslint.config.mjs`). `eslint --no-inline-config --format json` over the 24 changed TypeScript files reports 24 files, 0 errors, 0 warnings. The config sets no `parserOptions.project` (no type-aware linting), so this diff cannot move any untouched file's verdict. The Markdown and MDX files are outside eslint's population. - The 49 artifact-roster families, the CI-only jobs and the type-check lanes are left to CI. ## Changesets, and one deliberately red check - `.changeset/22824-admin-rescue-named-slate.md`: `@objectstack/plugin-approvals` minor, `@objectstack/spec` patch (description text), `@objectstack/lint` patch (verdict text). `Clause-②: yes`, stating FROM → TO. `@objectstack/rest` is not listed: against the published 17.7.0, its shipped content does not move. - **`.changeset/22725-approvals-unstaffed-arm.md` is withdrawn (deleted).** It announces an `unstaffed` arm that this PR retires before it ever ships, so it would publish a false release note. This is the DELIBERATE CORRECTION class `check-empty-changeset.mjs` names. `check:empty-changeset --base origin/main` stays **red by design**: Check Changeset is not a required context, and per ruling D the `skip-changeset` label is not applied. **Confirmation requested here:** the 22725 note is withdrawn because the behaviour it describes does not ship. ## File surface beyond the claim's list (declared) My change turned these red, so each is updated to keep what it pins: - `packages/lint/src/rule-explanations.ts` (the rule's `os explain` paragraph) and the rule's hints in `validate-approval-approvers.ts`. The dispatch ruled "the message text only"; the explanation and hints said "waits forever" / "recoverable only by override", which this change makes false, and the os-dev standard clause makes a text this change falsifies a must-fix. The rule's logic is untouched. The conflict is named in the report, not silently resolved. - `approval-requested-notification.integration.test.ts`: one test name and the header now say "with no organization administrator". - Dogfood pins whose scene was a slate the administrators' slate now replaces: - `approval-override-composite-pin` + fixture route to a named approver who never acts (the override's remaining break-glass case); - `my-pending-position-address` and `position-address-readers` + fixtures add a named co-approver, so the `position:NAME` literal stays on the slate for the late holder; - `flow-node-config-values-at-registration` asserts the administrators' slate; - `activity-text-predicate` and both approval-snapshot pins stamp the holder's organization on the `sys_user_position` rows they seed. The routing read never resolved an unstamped (unknown-tenancy) row, and the literal fallback had masked that. - `content/docs/permissions/tenant-audit-census.mdx` and `docs/audits/2026-08-tenant-audit-write-call-sites.counts.md`: the routing note's insert is one more write call site (238 → 239, decidable 159 → 160, elevated 126 → 127). Regenerated with `node scripts/tenant-audit-census.mjs --write`, and the census-held prose figures were updated by hand. ## Acceptance notes - **The abandoned slate is replaced, not kept beside the administrators**, as the ruling's "abandoned primary slate" and the `fallback` branch do. A holder staffed into a position after an all-literal request opened no longer receives it (an administrator can reassign it). A `position:NAME` literal beside a named approver is untouched, and so is the late holder's path to it. - **Unstamped position assignments.** `expandPositionUsers` matches `sys_user_position.organization_id` by strict equality (the platform's stated fail-closed reading of unknown tenancy, shared with `plugin-sharing`), so an organization-less assignment row never put its holder on a slate. Before, the literal stayed and that holder could still act through the `position:NAME` address. Now an all-literal slate opens on the administrators. Rows written through a door are stamped, and the showcase seed stamps them. Noted, not filed; carrier: none. - **Cost.** One grant resolution per member of the request's organization, eight at a time, on the empty-slate path only. - **The submitter is not excluded** from the administrators' slate (a one-admin organization decides its own request), the same as any slate that names its submitter. - `docs/qa/platform-checklist/areas/approvals.json` (the empty-slate fork item) still says `admin_rescue` is "decidable only via the #3424 privileged override". It is internal QA text, not a published surface; carrier: none. - The `ApproverType` describe text ("a manager step resolves to nobody and the request waits") is still literally true (it waits on the administrators). It was left alone under the describe-only scope. ## Seat append: patch round 1 (the dev's note, appended by the `domain:services` seat 1; the dev never edits this body) - `origin/main` `d48931871` was merged in by merge commit `9152dfdf5`, made by `scripts/pm/os-regen-merge.sh` (no rebase, no amend, no force-push). - The census conflict took `main`'s bytes in the merge. `9e24d7812` then regenerates both census files with `scripts/tenant-audit-census.mjs --write` and re-holds the page's prose figures (233 → 234: this branch's one new write call site). - **Gates at `9e24d7812`:** 116 derived, 116 run. The one expected red is `check-empty-changeset`, the deletion of the `22725` note, which the maintainer confirmed in `6109850105`. - **Tests:** `plugin-approvals` 1035/1035, lint 6455/6455, approval dogfood 43/43. ## Seat append: patch round 2 (the dev's note, appended by the `domain:services` seat 1) - The merge queue dequeued this PR at 2026-10-11T15:18Z (`MERGE_CONFLICT`, the same two census files). - `origin/main` `6364375b5` was merged in by merge commit `5d0a4a152` (`os-regen-merge.sh`), and `e25e6c27f` regenerates both census files with `scripts/tenant-audit-census.mjs --write` (231 → 232: this branch's one write call site). - **Gates at `e25e6c27f`:** 116 derived, 116 run. The one expected red is `check-empty-changeset`, the `22725` deletion confirmed in `6109850105`. - **Tests:** `plugin-approvals` 1035/1035, lint 6461/6461, approval dogfood 43/43, on dists rebuilt from the merged tree (which carries #22854 and #22819). - **The earlier contract review PASS (`6110255223`, on `9e24d7812e`) does NOT carry to this head.** The hop moved paths this PR owns that are not `merge=os-regen` regenerations: the two census files, plus `rest-server.ts` and `rule-explanations.ts` from `main`'s side (`unexplainedPathsBetween`, machine-read on the committed trees). So a fresh at-tier review runs on `e25e6c27f`. --- _Generated by [Claude Code](https://claude.ai/code/session_01CBAfsWMSfM3EToQGVStEcp)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
Fixes #22782
Clause-②: yes (narrowing)
What changed
HookSchema.onErrorsaysabort: Rollback transaction (if blocking), defaultabort. A plain write opened no unit of work. So a throwingafterInsertrefused the caller while the row stayed stored, and so did the rows earlierafter*hooks had written throughctx.api. A client that retried the refused create made a duplicate.The write door (
insert/update/delete,packages/objectql/src/engine.ts) now opens a unit of work for exactly the case that needs one. The abort then rolls back the row and the hook chain's engine writes together.opensAfterAbortUnit): the object has a metadata-bound hook (meta, the only hook entries that carry a declaredonError) on the write'safter*event. That hook is blocking (notasync) and itsonErrorisabortor omitted. Automations are not skipped, no unit is open yet (neither ambient nor threaded), the object is on the default driver, and that driver declares transactions.inAfterAbortUnit): it openstransaction()and re-enters the same door method, which joins the unit. Hook count and order are unchanged (ObjectQL.delete's single-id cascade is not transactional — a refusal mid-cascade leaves earlier children deleted while the response says the delete failed #7413). Only the boundary around the hooks moves.SummaryRecomputeErrormeans "the records WERE written". It is rethrown after the commit, never through the rollback.transaction()closes its ambient entry when the callback settles, before the commit. Work the unit starts and does not await (anasync: truehook) used to inherit the finished handle. Measured on the booted stack, that work failed withTransaction query already completeand its write was lost. It now runs on its own connection.No new authorable key. No change to #7413's hook count or order. No commit-time trigger and no
timingkey (#7477's guardrails).Step 1 measurements (base
9f5eca52b3)Which drivers implement transactions. Measured by symbol (
beginTransaction/commit/rollbackanddriverSupportsTransactions), not by atransaction(grep. That grep found only driver-sql and driver-turso. By symbol, driver-memory and driver-mongodb implement the trio too, and driver-sqlite-wasm inherits it.SqlDriversubclass, the verify stack's default)supports.transactionsUnsupported, so no unit opens. This is (B).beginTransactiondeep-copies every table androllbackrestores that copy. The cost is O(rows) per unit, and a rollback also drops writes other requests made in the meantime. It is not a boot store and is frozen as a test backend.transaction(), the atomic cascade and the atomic batch. See the open questions in the report.driverSupportsTransactionsis false, so no unit opens and nothing is warned.Cost, on the verify stack (sqlite-wasm, in-process). 5 rounds of 100 inserts per object, interleaved, on a shared box, so read the ratios rather than the absolute times:
aborthook (unit opened);onError: 'log'hook (no unit);That makes the unit about 1.36x. An object without such a hook pays one hook-map lookup.
Nesting. Inside a caller-opened
transaction()the door sees the handle and opens nothing.transaction()itself joins an ambient one (ADR-0067 D2). Pinned: exactly onebeginTransaction, and the caller's commit decides.Where (A) does not hold, so (B) applies, and the spec text it needs
transaction()covers the default driver only (ADR-0119 D1), and opening one would refuse the object's own write. This is the same verdict as ObjectQL.delete's single-id cascade is not transactional — a refusal mid-cascade leaves earlier children deleted while the response says the delete failed #7413's'split'cascade. After an abort, the row stays stored, as before.packages/spec/src/data/hook.zod.tschanges in text only.HookEvent's JSDoc now lists four ways a write ends up inside a unit, the write door's own being the new one.onError's TSDoc anddescribe()now say where the rollback holds and that elsewhere anafter*hook's abort refuses the caller while the row stays stored. No shape, key, default or enum changes.content/docs/references/data/hook.mdxwas regenerated (check:generated --fix, which rangen:docsfor the one artifact it proved stale). An@objectstack/specpatchchangeset covers it.content/docs/automation/hooks.mdx, which the claim declares, carries the same fourth path.Behaviour inside the new unit (stated in the changeset)
aborthook on such an object that writes a business row to another datasource is now refused withCrossDatasourceTransactionWriteError, as it would be inside any unit (the [spec] engine.transaction 契约收紧:opts.requirefail-closed、跨驱动拒绝、owned-vs-joined 信号(#4619 的契约半边,维护者已批 P2) #5696 ruling). The abort turns that refusal into a refused write. In-tree reach: zero. The one first-party blockingabortafter*hook, app-todo'stask_logic, only logs.onError: 'log',async: true, or one datasource.Tests
The final head is
1c880cd1cb. Its only change sinced572e29ec8is a test file: the unit test's driver double now honourslimit, forcheck:objectql-double-limit. The source runs below were taken atd572e29ec8, except where a run says1c880cd1cb.pnpm --filter @objectstack/objectql exec vitest run src/engine-after-hook-abort-unit.test.ts src/engine-cascade-delete-atomic.test.ts src/engine-summary-retry.test.ts: 3 files, 25 passed (at1c880cd1cb).pnpm --filter @objectstack/dogfood exec vitest run test/hook-after-abort-rollback.dogfood.test.ts test/hook-error-format.dogfood.test.ts: 2 files, 10 passed.pnpm --filter @objectstack/verify exec vitest run src/handle.system-insert-delete.test.ts src/handle.exemplar-deal-lifecycle.test.ts: 2 files, 19 passed. This is the canary: a blockingafterInsert/afterDeletecapture hook plus record-change flows, now inside the unit.@objectstack/objectqllocalproject (vitest run --project local): 399 files, 7803 passed, atd572e29ec8.repoproject: 1 file, 5 passed.pnpm --filter @objectstack/objectql run typecheck: clean at1c880cd1cb.check:test-typecheckholds the ledger, and the new test file compiles clean.eslint --no-inline-config --format json). The configuration ignores the.mdand.mdxfiles ("no matching configuration"). The three.tsfiles report 0 errors and 0 warnings.eslint.config.mjsenables no type-aware linting, so this diff cannot move a verdict on an untouched file. The repo-widepnpm lintrun is left to CI.pnpm --filter @objectstack/dogfood run typecheck: clean.--listFilesincludes the new file.The pins. These are the triage's, on two layers:
packages/objectql/src/engine-after-hook-abort-unit.test.ts).POST /data/:object(createData) and the engine doorhooks.runon a booted stack (packages/qa/dogfood/test/hook-after-abort-rollback.dogfood.test.ts).What they assert:
afterInsertwith the defaultonError: the caller is refused; the row and thectx.apinote are absent.afterUpdate/afterDelete: the prior row is intact.onError: 'log': the row is stored and no unit opens.transaction(): one unit, and the caller keeps the row it committed.beforeInsert: refused, nothing stored, no unit.async, code-registered orskipAutomationswrites, for an object off the default driver, or for a driver with no transactions, and nothing is warned.async: truehook's late write lands after the unit commits.A plain
Errorfrom a function hook answers REST's sanitised500 INTERNAL_ERRORon both sides of the fix. The pins assert that envelope plus what is stored.Gates (head
1c880cd1cb)The dispatch's 80-gate list together with this diff's own derivation (
dispatch-gates.mjs --commands, 97 families) comes to 113 commands, and every one exited 0.dispatch-gates --ranreports 97 derived, 97 run, 0 NOT-MEASURED and 0 UNRUN, with every exit code recorded.check:skill-examplesandcheck:dual-build-cjs-loadsfirst answeredPREREQUISITE NOT MET(exit 3) because packages they read were not built. Both went green once those packages were built.check:objectql-double-limitflagged the new driver double as limit-blind. The double was fixed in1c880cd1cb.origin/main. Inpackages/objectqlthose commits touch onlyfilter-comparand-shape.tsand one new test, andgit merge-treeis clean. CI's merge ref is the joint check.Reverse verification and ablations
Each ablation leg was run with
scripts/ablation-replace.mjs(anchor hit, blob changed), a rebuild andablation-dist-preflight.mjswheredist/is read, and a restore proven by blob == HEAD and an emptygit diff HEAD, plus--absentafter the rebuild. Legs A, C and E were re-run on the final code shape atd572e29ec8.hooks.runpins (expected 1 to be +0, the row and the note both stored). This is also the reproduction of the card on a published door. The measured direction was the expected one: red.expected +0 to be 1, the async hook's late write lost). The first attempt at B was a no-op. Its marker (void 'ABLATION_22782_B') was eliminated by the build, the dist preflight refused it, and the leg was re-run with a marker that survives (B2).SummaryRecomputeErrorgoes through the rollback. Red: the roll-up case (one rollback, and the child row gone).beforeInsertveto never matches (fixture). Red: thebefore*control ({ status: 201 }).asyncandonError. Red: thelogcontrol and the no-unit case (one unit opened).Line budget (final head
a1ed738863)447 additions and 22 deletions over 11 files, against a 300-line suggestion. Measured from the base, round 4 is 3 additions and 3 deletions: text only, in two docs pages and one test title. Its changeset edit rewrites a line the PR already adds.
Round 2 — the seat's rulings and their verification
The seat ruled on the report's three open questions (verbatim):
What this PR does with each:
@objectstack/objectqlchangeset names the affected hooks and the remedy (onError: 'log',async: true, or one datasource).@objectstack/specpatchchangeset.Verification at
ad05283877:pnpm --filter @objectstack/spec runwithcheck:generated,check:docs,check:authorable-surfaceandcheck:api-surface: each exited 0.check:generatedreported all 14 artifacts up to date, against adistthatcheck:generated --fixhad just built.pnpm --filter @objectstack/spec run typecheck: clean, andcheck:test-typecheckholds the ledger.@objectstack/speclocalproject: 645 files; 19311 passed, 1 todo.objectqldistrebuilt and preflight-checked (no ablation marker).origin/mainis not merged. It is 18 commits ahead, touches none of this PR's files, andgit merge-treeis clean.--ranreconciliation are reported in the round-2os-dev-reportcomment on objectql: anafterInserthook withonError: 'abort'(the default) that throws rejects the write, but the row stays stored —HookSchema.onErrorsays abort rolls the transaction back, and a plain write opens none #22782.Round 3 — the ruling on the runtime test and its verification
CI on
ad05283877was red onTest Core (5/6)inpackages/runtime/src/sandbox/transaction-ambient-join.integration.test.ts. The case "with NO host transaction a throwing body still ROLLS BACK its own — unchanged" pinnedcommittedNames('thing')as['outer']. Its sandboxedafterInsertbody declares noonError, so it takesabort, and the case pinned exactly the defect this card fixes. Both readings were measured. With this PR the result is[]. With the door's unit ablated the file passes 6 of 6, and this PR's own pins go red. The seat ruled (verbatim):What changed in round 3. Only that test file and one changeset line changed; no runtime source and no other case.
afterInserthook withonError: 'abort'(the default) that throws rejects the write, but the row stays stored —HookSchema.onErrorsays abort rolls the transaction back, and a plain write opens none #22782 contract. It is renamed "with NO host transaction the door's unit owns the begin, the body joins it, and the abort rolls the outer row back too (objectql: anafterInserthook withonError: 'abort'(the default) that throws rejects the write, but the row stays stored —HookSchema.onErrorsays abort rolls the transaction back, and a plain write opens none #22782)", and its comment states FROM and TO. It now makes 9 assertions where it made 5:/body boom/);committedNamesis[].@objectstack/objectqlchangeset gains one line. A sandboxed body's ownctx.api.transaction, with no transaction open around the write, now joins the door's unit, and the abort rolls back the triggering row too.Verification at
0b6b8275d1(objectqldisthas the change, and the preflight finds no ablation marker in it):ablation-replacelanded the mutation, the rebuild was preflight-checked (marker in 4 files), and the restore was proven (blob equals HEAD,--absent). The re-pinned case goes red withexpected undefined to be { __trx: 1 }: the outer write is outside any unit. The other 5 cases pass.localproject, this PR's unit and dogfood pins, and the gate re-derivation with its--ranreconciliation are reported in the round-3os-dev-reportcomment on objectql: anafterInserthook withonError: 'abort'(the default) that throws rejects the write, but the row stays stored —HookSchema.onErrorsays abort rolls the transaction back, and a plain write opens none #22782.Round 4 — the contract review's remediation (review 6109504950, FAIL on
0b6b8275d1)Round 4 changes text only: no engine source and no assertion.
content/docs/api/data-flow.mdxlifecycle table, theafterInsert/Update/Deleterow. It said "Can Abort? ❌ No (record is saved)". It now states this PR's contract: a blockingonError: 'abort'hook (the default) refuses the write and rolls it back; on an object outside the default datasource, or on a driver with no transactions, the caller is refused but the row stays stored. It links to the hooks page's unit-of-work section. Declared on the devx seat post [PM seat] domain:devx @ objectstack — 🟢 os-warren · session_01FM29ZqLTrnPNXxijioZwMp #6023 (6109534840).content/docs/kernel/events.mdx, theonError: 'abort'bullet. It gains where the rollback holds, in the same wording as R1.@objectstack/objectqlchangeset's provenance for shipping a breaking change asminornamed ADR-0131 (organization ownership), copied from another changeset. It now names ADR-0087's ratified pre-launch launch-window exemption, as amended 2026-09-13. The level staysminor.afterInserthook withonError: 'abort'(the default) that throws rejects the write, but the row stays stored —HookSchema.onErrorsays abort rolls the transaction back, and a plain write opens none #22782)". Its assertions are unchanged. Declared in the [PM seat] domain:cli — ⏳ vacant #6024 addendum (6109539378).Verification at
a1ed738863:objectqldisthas the change, and the preflight finds no ablation marker in it.check:docs,check:doc-authoring, both docs-audit scripts,check-adr-0087-registration,check-changeset-no-major,check-empty-changesetand the rest) and its--ranreconciliation are in the round-4os-dev-reportcomment on objectql: anafterInserthook withonError: 'abort'(the default) that throws rejects the write, but the row stays stored —HookSchema.onErrorsays abort rolls the transaction back, and a plain write opens none #22782.Acceptance notes
Each note below is unmeasured or out of scope, and none was filed as a card.
registerHook, nometa) that throws in anafter*event still refuses the caller with the row stored. It declares noonError, so it is outside the contract this card enforces. Carrier: none.transaction()threads its handle explicitly (trxCtx). Work that escapes such a callback still carries the finished handle; the close here covers the ambient entry only.ScopedContext.transaction()publishes an ambient entry that is not closed either. Read from the code, not measured. Carrier: none.after*hook withonError: 'log'whose condition cannot be evaluated raisesHookConditionError, whichonErrornever softens (hook 的condition求不出值时:全局 fail loud —— 抛错并中断该次操作(方案 B 已拍板;Blocked-by #4770) #4775). Such a hook opens no unit, so the caller is refused while the row stays stored. Read from the code, not measured. Carrier: none.rollbackrestores a whole-store snapshot, which drops concurrent writes. This predates this change and is reachable from any unit. Carrier: none.Authored by session
session_01JfJfBUC3cQ6hhgm9MQK76T(os-dev, dispatched by thedomain:engineseat 1); rounds 2 to 4 edited this body through the fleet relay.Generated by Claude Code