Repository navigation
Commit 5b5e83f
fix(objectql)!: a write's returned row and its prior read serve the declared fields, never an orphaned column (#21631)
Fixes #21613
Clause-②: no (narrowing)
A write now answers with the object's declared fields plus the
platform's system columns. A column no metadata declares, such as a
field retired in an upgrade whose column additive sync leaves behind, no
longer leaves the engine through a write. The same holds for the prior
read a write binds as a hook's `previous`. This completes #21571's read
rule for writes. It is decided in the same place, the engine
(`packages/objectql`), with the same helper
(`declared-read-columns.ts`), on the rows as the driver returned them:
before formulas, hooks, events and the door's own ingress strip. No
driver source and no `metadata-protocol` source is edited. There is no
per-door strip, no allow-list and no flag.
## Measured first (before any fix)
The probes were two temporary tests on this branch: `9c73f3702b` (rest)
and `b923112e06` (plugin-audit), reverted by `dfbb80ec2e`. The net diff
carries none of them. Harness: the composed REST harness of #21571's
reach pin (`RestServer`, then `ObjectStackProtocolImplementation`, then
`ObjectQL`, then a real `SqlDriver` on better-sqlite3), two boots, at
`5c9138b4b6`.
| door | before | after |
| --- | --- | --- |
| `PATCH /data/:object/:id` | 200, `record.mailing_street = "1 Retired
Way"` | no retired key |
| `POST /data/:object` | 201, both retired keys, `null` | no retired key
|
| `POST /data/:object/:id/clone` | 201, both retired keys, `null` | no
retired key |
| `POST /data/:object/createMany` | 201, every record carries both,
`null` | none |
| `POST /data/:object/batch` update / upsert (update arm) | values |
none |
| `POST /data/:object/batch` create / upsert (create arm) | `null` |
none |
| `POST /data/:object/updateMany` | values | none |
| `DELETE /data/:object/:id` | no record in the body | unchanged |
| `engine.update` by id, `engine.insert`, `engine.insertMany` outcomes |
values or `null` | none |
| `data.record.created` / `data.record.updated` events, `after` | 13 of
13 carried them (16 events in all) | 0 of 16 events carry one |
| hook contexts, `previous` or `result` | 28 of 28 carried them | 0 of
28 carry one |
Bulk `data.records.*` events carry a count and no row, and `changes`
carries the input patch only.
**The audit ledger serves the prior read.** Measured with a plugin-audit
probe (real `ObjectQL`, a SQL-shaped store, `installAuditWriters`):
- a delete's `old_value` recorded `mailing_street: "c2 Retired Way"`,
and a create's `new_value` recorded both columns as `null`;
- an update recorded nothing of them, only because both sides of the
diff carried them;
- with ONLY the write result shaped (the prior read left whole), every
update of such a row recorded `mailing_street: "c1 Retired Way"` in
`old_value` against `null` in `new_value`. That is a phantom change, and
it puts the stored value into a served row on every write.
`sys_audit_log` is read back through the data door, so the prior read is
a served row. That puts it in this card's class (H3), and shaping the
result alone would have made the ledger worse. So the prior reads are
shaped too.
### Hypothesis verdicts
- **H1 (confirmed):** `SqlDriver.update` returns its `select *`
readback, and create and `bulkCreate` return `returning('*')`.
`ObjectQL` returned both as they came.
- **H2 (confirmed):** the engine publishes `data.record.*` from the same
`result` variable the hooks get, after the driver. The webhook enqueuer
copies the event's `payload` verbatim (`payload: { ...payload, ... }`).
With the shaping placed after the driver, the event and the webhook
payload are clean, with no edit in `plugin-webhooks` or
`service-realtime`. The pins below drive the real `AutoEnqueuer`.
- **H3 (served, in class, shaped):** see the audit ledger above. The
four prior reads are shaped at the read:
- update by id (`driver.findOne`);
- update by predicate (the matched rows the per-row hooks compose
`result` from);
- delete by id;
- delete by predicate.
The delete event reads only the tenant column off its pre-image, which
is declared.
- **H4 (enumerated):** the write verbs that return a row are:
- `insert` (one row and a batch);
- `insertMany` (its `ok` outcomes carry `rowHookContexts[i].result`);
- `update` by id.
Predicate `update` and `delete` return a count, and `delete` by id
returns the driver's boolean. The engine has no upsert verb: the batch
door's upsert is `update` or `insert`. `ObjectRepository` and the scoped
context delegate to these verbs. Every REST and protocol face above
reaches one of the three, so shaping them covers every door.
- **H5 (pinned):** an `internal: true` field stays whole on the
engine-level write result (the A-prime ruling) and is still stripped by
the data door. The rest pin and the conformance matrix both assert it.
### In-process readers of undeclared columns
The suites ran with the shaping in place: objectql at `10d7f333d9`, the
others at `f082789802` (the probes were still on the branch):
| suite | result |
| --- | --- |
| objectql | 4 failures, all fixtures, see below |
| rest | 260 files, all pass |
| metadata-protocol | 208 files pass, 3 skipped |
| plugin-audit | 39 files pass |
| plugin-webhooks | 13 files pass |
| plugin-sharing | 38 files pass |
| plugin-auth | 118 files pass |
| service-automation | 166 files pass |
| runtime | 318 files pass |
The 4 objectql failures were fixtures whose `readonlyWhen` or validation
rule reads a field the fixture never declared (`record.locked`,
`record.limit`), off the prior read. #21571 saw the same shape in trim
mode. The fixtures now declare those fields (`engine.test.ts` × 3,
`plugin.integration.test.ts` × 1). No assertion changed. At authoring,
the formula validator's `unknown field` check
(`packages/formula/src/validate.ts`) flags a production rule that names
no field.
The operator reads that legitimately need a retired column's values go
through the driver, so they are unaffected:
- `os migrate plan`'s `unmapped_column` detection;
- `os migrate account-issuer`.
## Changes
- `packages/objectql/src/engine.ts`, one shaping per site, each with
`declaredColumnSet(registry object)`:
- `insert`: the driver's result rows, after the one-row-per-input guard;
- `update` by id: the driver readback;
- the update by-id prior read, and the update predicate's matched rows;
- the delete by-id pre-image, and the delete predicate's doomed rows.
- Tests:
- `packages/rest/src/data-write-result-declared-fields.test.ts`: the
reach pin, on the composed harness.
-
`packages/objectql/src/write-result-declared-fields-conformance.test.ts`:
the engine matrix.
-
`packages/plugins/plugin-webhooks/src/webhook-payload-declared-fields.test.ts`:
the webhook payload through the real `AutoEnqueuer`.
-
`packages/plugins/plugin-audit/src/audit-ledger-declared-fields.test.ts`:
the served ledger.
- The objectql fixtures above.
- `.changeset/21613-write-result-declared-fields.md`:
`@objectstack/objectql` minor, BREAKING (narrowing), ADR-0087
`not-required (no-migration-prescription)`, and the interim route
(convert before upgrading, or #21573's operator read once it lands).
**File surface beyond the claim, test-only:** the webhook and audit pins
live in their own packages. The composed REST package has no dependency
on either plugin. Adding one would need a devDependency plus a source
alias, because `KNOWN_UNALIASED_TEST_IMPORTS` is shrink-only. So the
event both of them consume is pinned on the composed harness, and each
consumer is pinned where it lives.
## Pins
- **Reach pin (rest, SqlDriver, two boots, 8 cases):**
- the fixture is real: a raw driver read still finds the stored values;
- PATCH, POST and clone carry no retired key;
- the bulk faces: createMany, batch upsert / create / update,
updateMany;
- every `data.record.created` / `updated` event's `after`;
- the update and delete `previous`;
- declared fields and the eight system columns are served, and every key
is declared or provisioned;
- `internal`: stripped at the door, whole on `engine.update`.
- **Conformance matrix (objectql, SQL-shaped store, 12 cases):**
- insert one row, a batch, `insertMany`, and update by id;
- the hooks' `previous` and `result`, for by id, per-row predicate
update, and delete by id and by predicate;
- the events;
- `createData`, `updateData` and `cloneData`;
- the declared treatment: `internal` whole on the engine result and
stripped at the door, formula hydrated;
- the driver's stored row is not mutated.
- **Webhook pin:** a real `ObjectQL` write, then the realtime publish,
then the real `AutoEnqueuer`. The enqueued `payload.after` carries no
retired key, for created and for updated.
- **Audit pin:** the update diff is exactly `{ name }`, and the delete
`old_value` and create `new_value` carry no retired key.
- **Pin sweep:** a repo-wide `git grep` over `*.test.ts` for orphaned,
retired, unmapped or undeclared column readings, plus the suite runs
above. No test asserted that a write's returned row or prior read
carries an undeclared column, so no pin flipped. Each new pin asserts
the declared values as well as the absence: ids, names and system
columns.
## Reverse verification (fix committed first)
Each shaping was removed with `node scripts/ablation-replace.mjs`: the
anchor went from 1 hit to 0 and the blob changed. objectql was then
rebuilt (exit 0), and `ablation-dist-preflight --absent` confirmed the
call is absent from all 14 built files (exit 0 each). The pristine build
carries every marker. Final run at `bfbeffed43`:
| removed | conformance (12) | reach (8) | webhook (1) | audit (3) |
| --- | --- | --- | --- | --- |
| insert result | 5 red: insert, batch, `insertMany`, events, doors | 4
red: POST, clone, bulk faces, events | red | create red |
| update by-id result | 4 red: update, hooks, events, doors | 4 red:
PATCH, bulk faces, events, system columns | red | update red (inverse
phantom diff) |
| update prior reads | 2 red: by-id and per-row hooks | 1 red:
`previous` | green | update red (the phantom diff) |
| delete prior reads | 1 red: delete hooks | 1 red: `previous` | green |
delete red |
Restore: `ablation-replace` restored with `git checkout HEAD -- PATH`
each time. The blob `6008f2bc` equals the HEAD blob, `git diff HEAD` is
empty and `git status --porcelain` is clean. After a rebuild, the
preflight finds all five markers present, and the pins are 12/12, 8/8,
1/1 and 3/3. Each mutation was replaced with a type-valid spelling so
the DTS step builds. A first C1 run with an arity-breaking spelling
failed the DTS step and was redone; its red set was the same.
## Local verification
- objectql: `test` 368 files, 7427 passed, and `test:repo` 5 passed, at
`bfb7b28a49` (after merging `origin/main` `e367002e11`). `typecheck`
(tsc, scripts and test-typecheck) exits 0 at `bfbeffed43`.
- rest:
- `test` 260 files, 4897 passed, 326 skipped;
- `test:repo` 5 files, 177 passed;
- `typecheck` exits 0;
- all at `bfb7b28a49`. The rest test file is unchanged since.
- plugin-audit and plugin-webhooks:
- full suites at `bfb7b28a49`: 39 files / 621 tests and 14 / 161;
- `typecheck` exits 0 at `bfbeffed43`;
- the two later commits touched only the new pin files, which were rerun
green.
- **Gates:** `node scripts/pm/dispatch-gates.mjs --commands` (no paths)
at `bfbeffed43` derived 71 commands. All 71 were run on that head and
each exited 0. `--ran` reconciles with "71 derived, 71 run, 0
NOT-MEASURED, 0 UNRUN", with every exit code recorded.
- Added beyond the dispatch's list, by this diff's paths:
- `check-adr-0087-registration` ×2, `check-empty-changeset` ×2 and
`check-tenant-audit-census` ×2;
- the `release-rehearsal-clone` and `release-pending-publish`
self-tests;
- `check:engine-double-contract`, `check:i18n`, `check:i18n-stale-fill`
and `check:objectql-double-limit`;
- `check:objectui-changeset`, `check:pm-changeset-deadline-census` and
`check:query-options-erasure`;
- `check:type-check-coverage`, `check:type-check-debt` and
`check:where-matcher`.
- `check:objectql-double-limit` first flagged the three new store
doubles as limit-blind. They now apply the caller's bound
(`bfbeffed43`).
- `check:dual-build-cjs-loads` and `check:i18n` first answered
PREREQUISITE NOT MET (exit 3), which is not a measurement. Both were
rerun after the build they name, and both exit 0.
- **Lint, narrowed and proven:**
- Population, from eslint's own config: the 8 changed files went to
`eslint --no-inline-config --format json`. eslint reports the changeset
as "File ignored because no matching configuration was supplied".
- Count, from the JSON: 8 files in the report, 7 TypeScript files
linted, 0 errors. The 1 warning is that ignore notice.
- Invariance: `eslint.config.mjs` enables no type-aware linting (no
`parserOptions.project`, no typed rules), so this diff cannot move an
untouched file's verdict.
- **Declared to CI:** the remaining downstream consumers of
`@objectstack/objectql`. The suites above are the ones that read write
results (rest, metadata-protocol, runtime, plugin-auth, plugin-sharing,
plugin-audit, plugin-webhooks, service-automation).
## Acceptance notes
- `declared-read-columns.ts`'s header still describes the read side
only. The write-side seats carry their own comments in `engine.ts`. It
was not edited, because the claim names it only for a new entry point
and none was needed. Carrier: none.
- `content/docs/data-modeling/queries.mdx` states the default projection
for reads. A write now follows the same rule, but no sentence says so.
`check-affected-docs` is green. Carrier: none.
- `engine.aggregate`'s undeclared `groupBy` name in process is unchanged
(already noted on #21571). Carrier: none.
---
_Generated by [Claude
Code](https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 5d095a0 commit 5b5e83f
8 files changed
Lines changed: 963 additions & 6 deletions
File tree
- .changeset
- packages
- objectql/src
- plugins
- plugin-audit/src
- plugin-webhooks/src
- rest/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 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1406 | 1406 | | |
1407 | 1407 | | |
1408 | 1408 | | |
| 1409 | + | |
| 1410 | + | |
| 1411 | + | |
| 1412 | + | |
1409 | 1413 | | |
1410 | 1414 | | |
1411 | 1415 | | |
| |||
1466 | 1470 | | |
1467 | 1471 | | |
1468 | 1472 | | |
1469 | | - | |
| 1473 | + | |
| 1474 | + | |
| 1475 | + | |
1470 | 1476 | | |
1471 | 1477 | | |
1472 | 1478 | | |
| |||
1483 | 1489 | | |
1484 | 1490 | | |
1485 | 1491 | | |
1486 | | - | |
| 1492 | + | |
| 1493 | + | |
| 1494 | + | |
1487 | 1495 | | |
1488 | 1496 | | |
1489 | 1497 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
13628 | 13628 | | |
13629 | 13629 | | |
13630 | 13630 | | |
| 13631 | + | |
| 13632 | + | |
| 13633 | + | |
| 13634 | + | |
| 13635 | + | |
| 13636 | + | |
| 13637 | + | |
| 13638 | + | |
| 13639 | + | |
| 13640 | + | |
| 13641 | + | |
| 13642 | + | |
| 13643 | + | |
| 13644 | + | |
| 13645 | + | |
| 13646 | + | |
| 13647 | + | |
| 13648 | + | |
| 13649 | + | |
13631 | 13650 | | |
13632 | 13651 | | |
13633 | 13652 | | |
| |||
14433 | 14452 | | |
14434 | 14453 | | |
14435 | 14454 | | |
14436 | | - | |
| 14455 | + | |
| 14456 | + | |
| 14457 | + | |
| 14458 | + | |
| 14459 | + | |
| 14460 | + | |
| 14461 | + | |
| 14462 | + | |
| 14463 | + | |
| 14464 | + | |
14437 | 14465 | | |
14438 | 14466 | | |
14439 | 14467 | | |
| |||
14556 | 14584 | | |
14557 | 14585 | | |
14558 | 14586 | | |
14559 | | - | |
| 14587 | + | |
| 14588 | + | |
| 14589 | + | |
| 14590 | + | |
| 14591 | + | |
| 14592 | + | |
| 14593 | + | |
14560 | 14594 | | |
14561 | 14595 | | |
14562 | 14596 | | |
| |||
15082 | 15116 | | |
15083 | 15117 | | |
15084 | 15118 | | |
| 15119 | + | |
| 15120 | + | |
| 15121 | + | |
| 15122 | + | |
| 15123 | + | |
| 15124 | + | |
| 15125 | + | |
| 15126 | + | |
| 15127 | + | |
15085 | 15128 | | |
15086 | 15129 | | |
15087 | 15130 | | |
| |||
16735 | 16778 | | |
16736 | 16779 | | |
16737 | 16780 | | |
| 16781 | + | |
| 16782 | + | |
| 16783 | + | |
| 16784 | + | |
| 16785 | + | |
16738 | 16786 | | |
16739 | 16787 | | |
16740 | 16788 | | |
16741 | | - | |
| 16789 | + | |
| 16790 | + | |
| 16791 | + | |
| 16792 | + | |
16742 | 16793 | | |
16743 | 16794 | | |
16744 | 16795 | | |
| |||
16878 | 16929 | | |
16879 | 16930 | | |
16880 | 16931 | | |
16881 | | - | |
| 16932 | + | |
| 16933 | + | |
| 16934 | + | |
| 16935 | + | |
| 16936 | + | |
16882 | 16937 | | |
16883 | 16938 | | |
16884 | 16939 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1725 | 1725 | | |
1726 | 1726 | | |
1727 | 1727 | | |
| 1728 | + | |
| 1729 | + | |
| 1730 | + | |
1728 | 1731 | | |
1729 | 1732 | | |
1730 | 1733 | | |
| |||
0 commit comments