Repository navigation
Commit 5c9138b
fix(objectql)!: an unprojected read serves the declared fields, never an orphaned column (#21612)
Fixes #21571
Clause-②: no (narrowing)
An unprojected read now serves 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. The decision is made once, in the engine
(`packages/objectql`), so every driver and every door gets the same
answer. No driver source and no `metadata-protocol` source is edited.
## Measured first (before any fix)
The probe was a temporary, env-gated recorder at the engine's read sites
(commit 920cecf, reverted by a7500b3; the net diff carries none of
it). In trim mode it deleted every undeclared key at `find`, `findOne`,
the by-id update result and the update/delete prior reads, a superset of
this fix.
| suite (tree) | files | under trim | undeclared-key hits |
| --- | --- | --- | --- |
| objectql (a7ab047) | 366 | 8 failed, 7374 passed | 50 |
| rest (a7ab047) | 258 | all pass | 0 |
| plugin-auth (a7ab047) | 118 | all pass | 1 (a mock `sys_account` row)
|
| service-automation, plugin-sharing, plugin-audit (a7ab047) | all |
all pass | 0 |
| runtime (778bf5c, with #21545 merged) | 318 | all pass | 0 |
The 8 objectql failures, by cause:
- 2 in `engine.test.ts` (#3617): the `sys_migration` double declared
only `id`. The production object declares `blocking`, `verified_at` and
`last_run_at`, and `readMigrationFlagVerified` reads those. The fixture
now declares what production declares.
- 5 in `engine.test.ts` / `plugin.integration.test.ts`: red only at the
prior-read probe sites (a fixture's `readonlyWhen` reads an undeclared
`locked`). This fix does not touch prior reads.
- 1 in `driver-fault-boundary-redaction.test.ts`: the mock driver hands
back live references to its store, and an in-place delete corrupted it.
So the fix never mutates a driver row: a row carrying an undeclared key
is copied without it.
In-process readers of undeclared columns, by call site:
- `os migrate plan`'s `unmapped_column` detection introspects the table
through the driver (`driver-sql/src/schema-drift.ts`). Unaffected.
- `os migrate account-issuer` reads `sys_account.issuer` through the
driver (`cli/src/commands/migrate/account-issuer.ts`). Unaffected.
- The `os migrate apply` account preflight
(`cli/src/commands/migrate/apply.ts`, then plugin-auth
`account-identity-preflight.ts`) calls `engine.find` with an explicit
projection naming the retired `issuer`. The engine's unknown-plain
filter already drops that name today. Unchanged by this PR (see
Acceptance notes).
- The metadata column migrations (`packages/metadata/src/migrations/*`)
use driver raw DDL. Unaffected.
- `cloneData` copies every key of its `findOne` source into an insert.
Before this fix, an orphaned column made the clone fail at the write
door (`Unknown field 'mailing_street'`, `INVALID_FIELD`); it now
succeeds. This reader was hurt by the orphaned column, it did not rely
on it.
- Export, search, the RPC dispatcher, `getData` and `findData` all read
through `find`/`findOne`. They narrow with the door, which is intended.
- hotcrm's `scripts/backfill-contact-mailing-address.ts` is external and
reads through REST. The changeset names its interim route.
Hypothesis verdicts:
- **H1 (confirmed):** `find` reached the driver with `fields` undefined,
and driver-sql answered with `select('*')`.
- **H2 (confirmed, and it decided the shape):** on the composed REST
harness, an explicit projection naming a declared field with no column
fails, and driver-sql's ladder returns the whole row, retired column
included (the reach pin's ladder case, red before the fix). So the
engine shapes the rows the driver returns instead of pushing a
projection down. That holds whichever rung answered, and needs no driver
edit.
- **H3:** one authority, the registry field map plus
`PLATFORM_PROVISIONED_COLUMNS`. That list moved into
`declared-read-columns.ts`. The explicit-projection filter
(`find`/`findOne`), the new default projection and the write door's
`undeclaredWriteFieldErrors` all read it. No second list.
- **H4:** the shaping runs before formulas, `expand`, file references
and the hooks. Internal omission, credential masking and the `__search`
strip still run after the hooks. The conformance matrix pins formula,
password mask, `internal`, and each system column.
- **H6:** only `find` and `findOne` return row bodies. `count` returns a
number. `aggregate` returns aggregates, and its `groupBy` names are
gated at the door. `findStream` was retired in 17.0. `expand` re-enters
`this.find`.
## Changes
- `packages/objectql/src/declared-read-columns.ts` (new):
`PLATFORM_PROVISIONED_COLUMNS`, `declaredColumnSet` (no opinion for an
absent, array or empty field map, the rule the read and write doors
already share), and the non-mutating `withDeclaredColumnsOnly` /
`rowsWithDeclaredColumnsOnly`.
- `packages/objectql/src/engine.ts`: `find` and `findOne` shape the
driver's rows right after the driver call. The explicit filter and the
write door read the shared list.
- `packages/objectql/src/no-operator-object-door.ts`: a comment pointer
to the list's new home.
- Tests:
`packages/rest/src/data-query-unprojected-declared-fields.test.ts` (the
reach pin),
`packages/objectql/src/unprojected-read-declared-fields-conformance.test.ts`
(the conformance matrix), and the `sys_migration` fixture in
`engine.test.ts`.
- `.changeset/21571-unprojected-read-declared-fields.md`:
`@objectstack/objectql` minor, BREAKING narrowing, ADR-0087
`not-required (no-migration-prescription)`, and the interim route.
- `content/docs/data-modeling/queries.mdx`: one paragraph stating the
default projection.
## Pins
- **Reach pin.** The harness is the composed REST harness this package
already uses: `RestServer`, then `ObjectStackProtocolImplementation`,
then `ObjectQL`, then a real `SqlDriver` on better-sqlite3 with a file
database. Boot one declares two mailing fields and writes values. Boot
two retires them, the card's upgrade. Then `POST
/api/v1/data/rq_contact/query` with no `fields`.
- Before the fix: 4 failed, 2 passed. The retired columns were present
on the query and on `GET /data/:object/:id`, every-key-declared was red,
and the ladder rung was red. The system-columns pin and the explicit
`INVALID_FIELD` / 400 pin were green.
- After the fix: 6 passed.
- **Conformance matrix.** `objectql`, three driver behaviours (`whole
row`, `projection`, `ladder`) × doors (`find`, bare `find`, `findOne`,
ladder projection, retired-only projection, `expand`, `findData`,
`getData`, `cloneData`), plus declared-treatment and store-not-mutated
pins. A future driver shape is one entry.
- **Explicit projection** of a retired column still answers
`INVALID_FIELD` / 400. The test asserts `code` and `status`.
- **Pin sweep.** Repo-wide `git grep` over tests for orphaned, unmapped,
retired or undeclared column readings, plus the trim-mode runs above: no
test asserted that an unprojected read returns an undeclared column. The
only fallout was the `sys_migration` fixture, which now declares its
columns. No refusal assertion was changed.
## Reverse verification (fix committed first, at 9593fbd)
Each shaping call was replaced in turn with `node
scripts/ablation-replace.mjs` (anchor hit 1 to 0, blob changed). Then
objectql was rebuilt (exit 0), and `ablation-dist-preflight --absent`
confirmed the call is absent from all 14 built files.
- `find` call removed: reach pin 3 failed, 3 passed (query,
every-key-declared, ladder). Matrix 17 failed, 19 passed.
- `findOne` call removed: reach pin 1 failed, 5 passed (GET by id).
Matrix 9 failed, 27 passed (`findOne`, `getData`, `cloneData` × 3
shapes). The clone's failure reads `Unknown field 'mailing_street' on
object 'rq_contact'`.
- Restore: `git checkout HEAD -- packages/objectql/src/engine.ts`. Blob
aadf401 equals the HEAD blob, `git diff HEAD` is empty and `git status
--porcelain` is clean. After a rebuild, the preflight finds both calls
present in dist. Reach pin 6/6 and matrix 36/36.
## Local verification, at 4bc22fe (the PR head, after merging main,
which includes #21545)
- `pnpm --filter @objectstack/objectql test`: 367 files, 7415 passed.
`test:repo`: 1 file, 5 passed. `typecheck` (tsc plus test-typecheck):
exit 0.
- The reach pin: 6 passed. `pnpm --filter @objectstack/rest run
typecheck`: exit 0.
- The full rest suite ran at 99033a9: 259 files, 4889 passed, 326
skipped. The last merge (3 commits) touched neither `rest` nor
`objectql`, so that run was not repeated.
- Gates: `node scripts/pm/dispatch-gates.mjs --commands` (no paths)
re-derived the same 96 commands at this head. All 96 were run and
reconciled with `--ran` (96 run, 0 unrun).
- 95 exit 0.
- `node scripts/check-engine-split-ratio.mjs --days 90` answered exit 2:
NOT MEASURED. The clone is shallow inside the 90-day window, and this is
a report-only metric.
- 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
`.md`/`.mdx` pair as "File ignored because no matching configuration was
supplied".
- Count, from the JSON: 8 files in the report, 6 TypeScript files
linted, 0 errors on them. The 2 warnings are the two ignore notices.
- 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.
- Not run locally, declared to CI: the remaining downstream consumers of
`@objectstack/objectql`. The trim-mode measurement above covered rest,
runtime, plugin-auth, plugin-sharing, plugin-audit and
service-automation under a superset of this change.
## Acceptance notes
- **Write responses still carry orphaned columns. This is a finding for
the seat, outside this card's read ruling.**
- Measured after this fix on the same harness: `PATCH
/api/v1/data/rq_contact/c1` with `{ name }` answers 200, and its
`record` carries `mailing_street: "1 Retired Way"`.
- Cause: the engine's by-id update returns driver-sql's readback
(`select *`), and `updateData` strips only `internal` fields from it.
- The create 201 and clone 201 bodies are built from driver-sql's
`returning('*')` too. That is not measured here; on a new row the
orphaned columns would read null.
- The A-prime ruling keeps engine write results whole for privileged
writers, so where to cut this is a decision. It is not taken here.
- Prior reads (the update/delete `previous` rows handed to hooks) still
carry orphaned columns. In process only, no public door measured.
Carrier: none.
- In process, `engine.aggregate` does not refuse an undeclared `groupBy`
name. The data door does (`assertGroupByFieldsExist`). Carrier: none.
- The `os migrate apply` account preflight's `issuers` label reads
`(none)` on the engine path, because the engine already drops the
retired `issuer` from an explicit projection. The collision verdict keys
on provider and account ids and is unaffected. `os migrate
account-issuer` reads through the driver and shows real issuers.
Carrier: none.
- #21573 (the operator-only `os migrate` read of unmapped columns) is
not addressed here. It is the interim route the changeset names.
---
_Generated by [Claude
Code](https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 440cd32 commit 5c9138b
8 files changed
Lines changed: 717 additions & 20 deletions
File tree
- .changeset
- content/docs/data-modeling
- packages
- objectql/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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
285 | 285 | | |
286 | 286 | | |
287 | 287 | | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
288 | 295 | | |
289 | 296 | | |
290 | 297 | | |
| |||
| 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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2565 | 2565 | | |
2566 | 2566 | | |
2567 | 2567 | | |
| 2568 | + | |
| 2569 | + | |
| 2570 | + | |
| 2571 | + | |
| 2572 | + | |
| 2573 | + | |
| 2574 | + | |
| 2575 | + | |
| 2576 | + | |
| 2577 | + | |
| 2578 | + | |
| 2579 | + | |
| 2580 | + | |
| 2581 | + | |
2568 | 2582 | | |
2569 | 2583 | | |
2570 | 2584 | | |
| |||
2594 | 2608 | | |
2595 | 2609 | | |
2596 | 2610 | | |
2597 | | - | |
| 2611 | + | |
2598 | 2612 | | |
2599 | 2613 | | |
2600 | 2614 | | |
| |||
2636 | 2650 | | |
2637 | 2651 | | |
2638 | 2652 | | |
2639 | | - | |
| 2653 | + | |
2640 | 2654 | | |
2641 | 2655 | | |
2642 | 2656 | | |
| |||
2736 | 2750 | | |
2737 | 2751 | | |
2738 | 2752 | | |
2739 | | - | |
| 2753 | + | |
2740 | 2754 | | |
2741 | 2755 | | |
2742 | 2756 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
290 | 290 | | |
291 | 291 | | |
292 | 292 | | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
293 | 301 | | |
294 | 302 | | |
295 | 303 | | |
| |||
1912 | 1920 | | |
1913 | 1921 | | |
1914 | 1922 | | |
1915 | | - | |
1916 | | - | |
1917 | 1923 | | |
1918 | 1924 | | |
1919 | 1925 | | |
1920 | 1926 | | |
1921 | 1927 | | |
1922 | 1928 | | |
1923 | | - | |
1924 | | - | |
1925 | | - | |
1926 | | - | |
1927 | | - | |
| 1929 | + | |
| 1930 | + | |
| 1931 | + | |
| 1932 | + | |
| 1933 | + | |
1928 | 1934 | | |
1929 | 1935 | | |
1930 | 1936 | | |
| |||
11894 | 11900 | | |
11895 | 11901 | | |
11896 | 11902 | | |
11897 | | - | |
11898 | 11903 | | |
11899 | 11904 | | |
11900 | 11905 | | |
11901 | | - | |
11902 | | - | |
11903 | | - | |
| 11906 | + | |
| 11907 | + | |
| 11908 | + | |
11904 | 11909 | | |
11905 | 11910 | | |
11906 | 11911 | | |
| |||
11939 | 11944 | | |
11940 | 11945 | | |
11941 | 11946 | | |
| 11947 | + | |
| 11948 | + | |
| 11949 | + | |
| 11950 | + | |
| 11951 | + | |
| 11952 | + | |
| 11953 | + | |
| 11954 | + | |
| 11955 | + | |
| 11956 | + | |
| 11957 | + | |
| 11958 | + | |
| 11959 | + | |
11942 | 11960 | | |
11943 | 11961 | | |
11944 | 11962 | | |
| |||
12174 | 12192 | | |
12175 | 12193 | | |
12176 | 12194 | | |
12177 | | - | |
12178 | 12195 | | |
12179 | | - | |
12180 | | - | |
12181 | | - | |
12182 | | - | |
| 12196 | + | |
| 12197 | + | |
12183 | 12198 | | |
12184 | 12199 | | |
12185 | 12200 | | |
| |||
12215 | 12230 | | |
12216 | 12231 | | |
12217 | 12232 | | |
| 12233 | + | |
| 12234 | + | |
| 12235 | + | |
| 12236 | + | |
12218 | 12237 | | |
12219 | 12238 | | |
12220 | 12239 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
206 | 206 | | |
207 | 207 | | |
208 | 208 | | |
209 | | - | |
| 209 | + | |
210 | 210 | | |
211 | 211 | | |
212 | 212 | | |
| |||
0 commit comments