Repository navigation
Commit c8d06a9
fix(objectql): skipAutomations never skips the builtin audit stamps (#22106)
Fixes #22070
Clause-②: no
## What changed
`ExecutionContext.skipAutomations` skips the lifecycle hooks bound from
metadata. Its own description says it "must never bypass security or
audit", and so does the engine's comment at the skip. ObjectQL's builtin
audit stamps (`sys_stamp_audit_insert` / `sys_stamp_audit_update`) were
registered through the hook binder, which puts `meta` on every
registration. `meta` is exactly what the engine's skip keys on, so the
opt-out skipped the audit stamps along with the app's hooks.
- `packages/objectql/src/plugin.ts`, `registerAuditHooks`: every
`sys:audit` builtin is now registered in code with
`ql.registerHook(event, wrapDeclarativeHook(h, h.handler, { logger,
metrics: ql.getHookMetricsRecorder() }), { object, priority, packageId:
'sys:audit' })` and no `meta`, after
`unregisterHooksByPackage('sys:audit')`.
- It uses the same wrapper and the same metrics recorder that
`bindHooks` reads at bind time. AppPlugin installs the Server-Timing
`hooks` feed in its `init`, ahead of this `start`, so that span keeps
counting the builtins.
- The unregister-by-package keeps a second ObjectQLPlugin on a shared
engine idempotent. The constructor accepts an existing `ql`.
- So the opt-out is the only thing that changes for a builtin. Priority
and package id are unchanged.
- `packages/objectql/src/hook-binder.ts`: the comment that called `meta`
inert ("The engine ignores unknown options today; this is forward-only")
now says what `meta` is: the binding the opt-out keys on.
- `engine.ts` is unchanged. Its one rule stays "`meta` means bound from
metadata", and no dispatch site gains a second criterion. The rejected
alternative was exempting `sys:audit` at the engine's two dispatch
sites. It would add a magic package id to every dispatch site, and it
contradicts the spec text "bound from metadata (`bindHooksToEngine`) are
skipped".
- `.changeset/22070-audit-stamp-under-skip-automations.md`:
`@objectstack/objectql` patch.
## Measurement: which stamped column is lost under the flag
The door is the import runner: `runImport`, then
`ObjectStackProtocolImplementation`, then the engine, then driver-sql on
SQLite, with a session user and an organization. Base is `3d9188502e`.
| column | automations on (base) | automations off (base) | automations
off (head) |
|---|---|---|---|
| `created_by` | written | **lost** (NULL) | written |
| `updated_by`, insert and update | written | **lost** (NULL) | written
|
| a declared plain `tenant_id` field | written | **lost** (NULL) |
written |
| `created_at`, `updated_at` | written | written (driver-sql stamps
them) | written |
| `organization_id`, or a declared `tenancy.tenantField` (the wall
column) | written | written, by the driver from `DriverOptions.tenantId`
| written |
- The tenancy wall held throughout. Triage kept p2 on this measurement
(6040619736).
- The head column was read with the same probe after this change (the
same probe at head `5da266a319`, after a workspace rebuild: `Tests 4
passed (4)`; with automations off the app hook still does not run).
- On a driver that stamps nothing (the in-package pin's stub),
`created_at` / `updated_at` are lost too at base, and written at head.
## Pins
`packages/objectql/src/audit-stamp-skip-automations.test.ts`. They were
committed red at base in `8e0bfab19d`. They boot a real ObjectKernel
with ObjectQLPlugin over a store that stamps nothing, so every audit
value on a stored row is the hook's.
- **(a)** Under `skipAutomations` with a session user, an insert stamps
`created_by`, `updated_by`, `created_at`, `updated_at` and the declared
`tenant_id`. An update stamps `updated_by` and `updated_at`. The answers
are the same as without the flag.
- **(b)** Control: a metadata-bound app hook (`bindHooks`, `packageId:
'test-app'`) is still skipped under the flag, and runs without it.
- **(c)** The import door: `runImport` with `runAutomations: false`,
through the real `ObjectStackProtocolImplementation`, stamps the session
user on the row it creates and on the row it updates.
- **(d)** Every `sys:audit` entry on the engine carries no `meta`, so a
builtin added to the list later cannot reach the opt-out.
## Reverse verification (committed head `b97585c76f`)
- **Mutation:** `node scripts/ablation-replace.mjs` (WRAP mode, plus a
shell trap on EXIT/INT/TERM) replaced the anchor
`this.ql.unregisterHooksByPackage('sys:audit');` with the old binder
call `(this.ql as any).bindHooks(builtinHooks, { packageId: 'sys:audit'
}); return;`.
- The tool reported: anchor 1 to 0, replacement 0 to 1, blob
`9a41af3ad873` to `4d1c8b04da7c`.
- **Mutated run:** `Tests 3 failed | 1 passed (4)`. (a), (c) and (d) are
red, and (b) is green.
- **Restore:** blob after restore `9a41af3ad873` equals the HEAD blob
`9a41af3ad873`, `git diff HEAD` is empty, and `git status --porcelain`
is empty.
- **Restored run:** `Tests 4 passed (4)`.
- The subject is imported from `src` by relative path, so no `dist/` leg
was needed for the pins.
## Clause-② measured on the built declarations
- `@objectstack/objectql` was built from the pre-fix source (`HEAD~1`
blobs restored in place, trap-guarded) and from head.
- `core.d.ts` is byte-identical.
- `index.d.ts` differs only in the JSDoc of the private member
`registerAuditHooks` (6 comment lines). Pre-fix sha256 is
`b0e8cffd9ce1`, the same as base `3d9188502e`. Head is `c0cfc96e72f3`.
- No export, key, option or type moves.
- The head `dist/` was rebuilt afterwards. `ablation-dist-preflight
--absent` for the old JSDoc text exited 0, and the tree is clean.
## Local verification (head `5da266a319`, `origin/main` `bafb58bb08`
merged)
- `node scripts/pm/dispatch-gates.mjs --commands` (no paths; merge base
`bafb58bb0`, 4 paths): all 65 derived commands exit 0.
- `--ran` reconcile: `✓ dispatch-gates --ran: 65 derived famil(ies)
accounted for — 65 run, 0 NOT-MEASURED (a DERIVED zero — all 65 recorded
an exit code and none of them is 3)`.
- Artifact-roster block: all 54 run. 51 exit 0.
- 3 exit 2, `NOT WIRED` / `NOT MEASURED` with no PR context:
`check-closing-target-claim.mjs`, `check-partof-closing-keyword.mjs`,
`check-single-claim-paths.mjs`. They are left to CI on this PR.
- `check-partof-closing-keyword.mjs` was also run with `PR_BODY` set to
this body (exit 0: `this PR carries no Part-of/closing-keyword
contradiction`).
- Symbol-anchor sweeps: `check:adr-symbol-anchors`,
`check:scripts-symbol-anchors`, `check:spec-docblock-symbol-anchors` and
`check:adr-anchors` all exit 0.
- `@objectstack/objectql` full suite, both vitest projects (`vitest run
--maxWorkers=2`): `Test Files 380 passed (380)`, `Tests 7517 passed
(7517)`, lock VERDICT command-exit 0.
- `@objectstack/objectql` typecheck: `tsc --noEmit` is clean, and
`check:test-typecheck` reports `OK — 40 file(s) / 234 error(s) / 65
pinned signature(s) held`.
- `check:objectql-double-limit` was red on the first pass: the new pin's
stub `find` ignored the caller's `limit`. It was fixed in `86ee151e64`,
and the gate is green at head.
- The workspace was rebuilt at head before the dist-reading gates ran
(`turbo run build --filter=!@objectstack/docs`: 72/72).
- `plugin-security`'s suite was not run, because its file is not edited
here.
## Acceptance notes
- **`plugin.ts` `tenant_id` reading (not changed here; a read-only
inference):** `record.tenant_id = record.tenant_id ??
session.organizationId` keeps a caller-supplied `tenant_id` with no
flag.
- That is the client-preferred shape that #15964 and #16311 removed for
`created_at` / `created_by`.
- Not measured. No shipped object declares a plain `tenant_id` the hook
stamps: the one shipped `tenant_id` field, `sys_audit_log`'s, is stamped
by plugin-audit's writer.
- **For the services lane:
`packages/plugins/plugin-security/src/claim-seed-ownership.ts` lines 69
to 72 become stale with this change.**
- It says: "ObjectQL's own `sys_stamp_audit_*` builtins are bound
through the hook binder, so they carry metadata and are skipped as well;
for this write that changes nothing — the claim has no user to stamp
into `updated_by`, and the drivers stamp `updated_at` themselves."
- Proposed replacement: "ObjectQL's own `sys_stamp_audit_*` builtins are
registered in code too (#22070), so they also run on every claimed row;
for this write they stamp `updated_at` only, because the claim has no
user to stamp into `updated_by`."
- Left out of this PR on the seat's instruction. No change to #22067's
code.
- **#22067's `reown`:** on driver-sql, its `updated_at` was already
written under the flag (the driver stamps it). It carries no
`session.userId`, so it writes no `updated_by` before or after this
change.
- **H2:** the two stamps are the only platform builtins that carried
`meta`. The binder's other callers are metadata sources: `plugin.ts`
metadata-service binding and hook reload, and `runtime` app artifacts.
No `registerHook` call under `packages/` passes `meta`.
---
_Generated by [Claude
Code](https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 51290bc commit c8d06a9
4 files changed
Lines changed: 356 additions & 22 deletions
File tree
- .changeset
- packages/objectql/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 | + | |
| 22 | + | |
Lines changed: 288 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
305 | 305 | | |
306 | 306 | | |
307 | 307 | | |
308 | | - | |
309 | | - | |
310 | | - | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
311 | 315 | | |
312 | 316 | | |
313 | 317 | | |
| |||
0 commit comments