Repository navigation
Commit b4ce5e2
feat(verify): hooks.run takes the system principal on an insert and a delete (#22596)
Part of #22301
Clause-②: yes (widening)
Item 5 of the card: a user-less record trigger on an insert or a delete,
and a flow over a record the engine no longer holds. Items 6 and 7 and
item 1's remaining composition gap are not in this PR, and the card
stays open for them.
## What changes
- `hooks.run(object, 'insert' | 'delete', input, { system: true })` now
runs. It is the same engine call a person's write makes
(`ql.insert(object, input, { context })`, `ql.delete(object, { where: {
id }, context })`), with `context` set to `{ isSystem: true }` (the
handle's existing `SYSTEM_CONTEXT`). This is the write an integration or
a system job makes. Until now `hooks.run` refused `{ system: true }` on
anything but `update`, with `INVALID_REQUEST` / `400`, and that refusal
is removed.
- The `AsSystem` overload of `hooks.run` widens from `'update'` to
`'insert' | 'update' | 'delete'`. Its JSDoc says what runs: the hooks
(they see `session.isSystem` and no `userId`), the declared validations,
and the record-change trigger with no trigger user. It also says what
does not run: the permission gate. It says what a delete-triggered flow
is handed, and how this door differs from `seed`, which sets
`skipTriggers`.
- A call naming two callers (`{ as, system: true }`) is still refused
with `INVALID_REQUEST` / `400` before the engine is touched, on every
operation.
- `packages/verify/README.md`: the handle example and the caller and
refusal bullets.
- `.changeset/22301-verify-system-insert-delete-door.md`:
`@objectstack/verify` `minor`, `Clause-②: yes (widening)`.
- **Declared deviation:** two pending release notes from PR #22517
(items 2 and 3, not yet released) said this PR's door was refused, and
both are corrected in this change.
`.changeset/22301-verify-update-doors.md` said "Only `update` takes it.
`{ system: true }` on `insert` or `delete` … is refused"; it now keeps
only the two-caller refusal and points at this entry.
`.changeset/22301-spec-ledger-verify-provenance.md` gave "`{ system:
true }` on an insert or delete" as an `INVALID_REQUEST` example; it now
names a call naming two callers (`{ as: token, system: true }`) and
calls `hooks.run` and `hooks.updateWhere` the two write doors (patch
round, after the contract review FAIL `6093381495`).
`check-empty-changeset` and `Check Changeset` are RED on these two `M`
rows by design (the "DELIBERATE CORRECTION" class). The same-head
contract review PASS `6093488852` names both notes and judges each
rewritten sentence, which is the confirmation the landing rule takes.
## What was measured before the door was opened
These were measured on a booted stack (`requires: ['automation',
'triggers']`) at `d6c37919c7`. A scratch probe (since deleted) called
the booted `objectql` with `{ isSystem: true }`, which is exactly the
call the door would make.
1. On `origin/main`, the refusal sat at `handle.ts:486`–`:491`, and the
JSDoc said so at `:201`–`:203`. The `insert` and `delete` arms already
made the engine calls above, so lifting the refusal hands those calls
`SYSTEM_CONTEXT`. Confirmed.
2. **A system insert and a system delete each fire the record-change
trigger.** No dispatch was invented: the engine's own hook chain fired
it.
- On a system insert, `beforeInsert` and `afterInsert` see `isSystem:
true` and no `userId`. A `record-after-create` flow runs (`completed`),
with trigger `{ type: 'record-after-create', object, recordId }` and no
`userId`.
- On a system delete, `beforeDelete` and `afterDelete` see `isSystem:
true`, no `userId`, and the pre-image as `previous`. A
`record-after-delete` flow runs (`completed`) with no trigger `userId`.
- Only `skipTriggers` suppresses the trigger
(`trigger-record-change/src/record-change-trigger.ts:291`); `isSystem`
does not.
3. **A record the engine no longer holds.** A `record-after-delete` flow
is handed the pre-image as `record` and as `previous` (both read
`status: 'closing'`), and its `get_record` of `{record.id}` answers no
row. On the same delete, a `record-before-delete` flow's identical
`get_record` finds the row. The system delete door reaches this with no
further door.
4. `seed` is unchanged and still skips record-change flows (pinned
below).
Other readings from the same probe:
- `ql.delete` answers `true` for a system delete and for a person's
delete alike. See the Acceptance notes.
- A `readonly: true` field is KEPT by a system insert and stripped from
an admin's insert.
## Pins: `packages/verify/src/handle.system-insert-delete.test.ts` (7
tests)
The fixture is neutral. `sid_case` can be read, but not created or
deleted, by a fresh member. It has one capture hook and three
record-change flows: on create, before a delete, and after a delete. The
two delete flows read the case back with `get_record` and write `held`
or `gone` to a ledger.
- **System insert:**
- The member's insert is refused `PERMISSION_DENIED` / `403` and leaves
no row; the system insert writes the row.
- The hooks see the system and no user. The `record-after-create` flow
runs with no trigger user and writes its ledger row.
- Controls: an admin's insert carries `adminId` at the hook and on the
run, and `seed` of the same shape fires no run.
- **Validation:** a declared validation rule still refuses a system
insert with `VALIDATION_FAILED` and leaves no row. Control: an in-range
value is written.
- **System delete:**
- The member's delete is refused `PERMISSION_DENIED` / `403` and leaves
the row; the system delete removes it.
- The hooks see the system, no user and the pre-image. The
`record-after-delete` flow runs with no trigger user.
- Control: an admin's delete carries `adminId` on the run.
- **Deleted-record read:** the after-delete flow's ledger reads
`record_status: 'closing'`, `previous_status: 'closing'`, `held:
'gone'`. The before-delete flow on the same delete reads `held: 'held'`,
the control showing `gone` is the engine's answer about a deleted row.
- **Two callers** (`{ as, system: true }`) on an insert and on a delete:
refused `INVALID_REQUEST` / `400`, with no row written or deleted, no
hook dispatched and no flow run.
`handle.update-doors.test.ts`: the old pin "the system is accepted on
update only" asserted the refusal this PR removes. Its insert and delete
legs are dropped, since they are now doors pinned above. Its two-caller
leg on `update` stays.
## Ablations
Both ran on the committed head `915f88e526`, through `node
scripts/ablation-replace.mjs` (anchor hit 1 → 0, blob changed). Each was
restored to `HEAD` blob `a6e175d1c0d3` with `git diff HEAD` empty. The
direction of each was predicted before the run.
- **A1, the update-only refusal put back** (`throw callShapeRefusal(…)`
when `system && operation !== 'update'`). Predicted: the 6 system insert
and delete pins red, the two-caller pin and all 9 update-door pins
green. Observed: 6 failed, 10 passed, and each failure is the ablation's
own refusal (6 hits of its message in the log).
- The first attempt was refused by the tool before anything ran: its
replacement contained the anchor. It was re-anchored and re-run, and
that first attempt is not counted.
- A second attempt never acquired the verify lock (`queue-timeout`) and
is NOT MEASURED. It was restored by the tool.
- **A2, the seed context handed instead of the system context** in
`hooks.run`. Predicted: the 3 flow-run pins red here, plus the item 2
flow pin in `handle.update-doors.test.ts`; the row-level pins green.
Observed: 4 failed, 12 passed, exactly those four.
## Verification
All at `84a4a1e07f`, the final head (a merge of `origin/main`
`25be87612d`, which touches none of this PR's paths), unless named
otherwise.
- **Dependency closure:** `pnpm exec turbo run build
--filter='@objectstack/verify^...' --concurrency=2` rebuilt after the
merge, 46/46 tasks.
- **`@objectstack/verify` full suite:** `vitest run --maxWorkers=2`
passed, 27 files / 216 tests.
- **`pnpm --filter @objectstack/verify typecheck`** (`tsc --noEmit` and
`check:test-typecheck`) exited 0, at `915f88e526` and again at
`84a4a1e07f`. `tsc -p tsconfig.test.json --listFiles` lists 27/27 of the
package's test files, the new one included.
- **Consumer typecheck (`packages/qa/dogfood`):** the
`@objectstack/dogfood^...` closure was built (63/63;
`@objectstack/verify` rebuilt as a cache miss, and its `dist/index.d.ts`
carries the widened overload 1 time and the old one 0 times), then `pnpm
--filter @objectstack/dogfood typecheck` exited 0 at `915f88e526`. The
dogfood fake stack types `hooks` as `never`, so an overload change
cannot break it.
- **Reverse check against the rebuilt `.d.ts`, from dogfood:** a scratch
file calling `hooks.run('o', 'insert' | 'delete', …, { system: true })`
compiled (exit 0). Adding `'upsert'` turned `tsc` red with TS2769 "No
overload matches this call". The file was removed and `git status` is
clean.
- **Gates:** `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` with no paths gives 63 commands; the 4 extra
families from the dispatch-time list (spec `check:empty-state`,
`check:liveness`, `check:strictness-ledger`, `check:variant-docs`) also
ran.
- 61 exited 0. One is red: `check-empty-changeset`, the deliberate
correction above.
- `check:dual-build-cjs-loads` is NOT MEASURED: it needs a
whole-workspace build, which this dispatch excludes, so it is left to
CI.
- `--ran` reconciliation over the exit-coded record: 63 derived, 62 run,
1 NOT-MEASURED, 0 UNRUN.
- **The artifact-roster block:** see the report on the card.
## Acceptance notes
- **The `delete` arm's return type.** `hooks.run(…, 'delete', …)` is
typed as a promise of `EngineRow`, and the engine answers `true`,
measured for a person's delete and a system delete. The type is older
than this PR on the person overload, and the system overload repeats it.
Nothing reads a field of it: in hotcrm `f0afcbd`, no `hooks.run(…,
'delete', …)` call site reads the answer beyond asserting that it is
defined. Noted, not filed.
- **Carried from items 2 and 3, unchanged here:**
- the engine's `ValidationError` has no `status` at the engine door;
- `requireId`'s and the bad-operation call-shape errors carry no `code`.
- **Not served by this door:** an UPDATE-triggered flow over a row that
vanished between the trigger and the flow's `get_record`. The engine
runs a record-change flow inside the write, while it still holds the row
(the synchronous run log), so no write produces that state. The card's
report carries this as an open question with options.
## For hotcrm (`runRecordFlow`, `test/helpers/verify-stack.ts:255`; read
at hotcrm `f0afcbd`)
The helper has 8 call sites in 2 files:
- **Served by the system insert door:**
`test/flow-record-change.test.ts:633`, `:647`, `:684` and `:689`. Each
is an integration-written lead or contact whose record-after-create flow
must run with no trigger user.
- **Likely served, not measured on hotcrm:**
- `:249`, an ownerless contact. The owner stamp is the person's; a
system insert leaves an unset owner null in the fixture.
- `:500` and `:501`, a deal born `pending`. If `approval_status` is held
by `readonly: true`, the fixture reading above says a system insert
keeps it.
- **Not served:** `test/flow-escalation-ownerless-case.test.ts:233`, the
ghost case: an update-triggered flow over a row the engine does not
hold. See the note above.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01KNKBCRDJCu5tGy3TEbvtrF)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 73d7001 commit b4ce5e2
7 files changed
Lines changed: 456 additions & 46 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
9 | | - | |
| 9 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
12 | | - | |
| 12 | + | |
13 | 13 | | |
14 | 14 | | |
15 | 15 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
100 | 100 | | |
101 | 101 | | |
102 | 102 | | |
103 | | - | |
104 | | - | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
105 | 106 | | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
106 | 111 | | |
107 | 112 | | |
108 | 113 | | |
| |||
144 | 149 | | |
145 | 150 | | |
146 | 151 | | |
147 | | - | |
148 | | - | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
149 | 156 | | |
150 | 157 | | |
151 | 158 | | |
152 | 159 | | |
153 | 160 | | |
154 | 161 | | |
155 | | - | |
156 | | - | |
157 | | - | |
158 | | - | |
159 | | - | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
160 | 167 | | |
161 | 168 | | |
162 | 169 | | |
| |||
0 commit comments