Repository navigation
Commit e87070e
fix(objectql): a find/findOne projection that names a formula field returns that projection, not every stored column (#22337)
Fixes #22300
Clause-②: no
## What changes
`planFormulaProjection` (`packages/objectql/src/engine.ts`) widens a
`fields` projection that names a formula field to every stored column
plus `id`, so the formula's CEL `record.FIELD` lookups see the full row.
`find` and `findOne` never cut that widening back off their results. A
projection that named a formula field therefore returned every column of
the record.
- `planFormulaProjection` now also returns `widened`: the columns it
added beyond what the caller named. `id` is one of them when the caller
did not name it.
- New `withoutFormulaWidening` / `rowsWithoutFormulaWidening` remove
exactly those columns from the result. `find` and `findOne` call them at
their `return`, after `executeWithMiddleware`.
- The driver read is unchanged. The formula still sees the full row.
## Where the cut sits (M4), and what still reads the widened row
Every internal reader of the widened rows runs before the cut, and they
run in this order:
1. the formula pass (`resolveFormulaPermissions`, `applyFormulaPlan`),
including the unevaluated-formula fault sink;
2. `expandRelatedRecords`, which reads only the foreign-key keys named
in `expand`;
3. `resolveFileReferences`;
4. the `afterFind` hooks;
5. `maskSecretFields` / `omitInternalFields` and
`stripSearchCompanionFromRead`;
6. the middleware post-phase. This includes the field-level-security
result mask, and the audit redactions that add their judged columns to
the projection and remove them afterwards.
Cutting any earlier would starve one of these of a column it reads
today. The cut removes the widened columns rather than keeping a list. A
key an `afterFind` hook derives survives, and so does an expanded
relation the caller named. Rows are copied, never mutated, in their own
key order.
Post-hoc sorting of formula fields is refused before planning
(`assertOrderByIsMaterializable`), so it reads no row.
## Measurements
### M1: the widening, reproduced on `main` before the change
- Engine: the new `engine-formula-projection-trim.test.ts`, at base
`f2626c71db`: 8 failed and 4 passed of 12. The 4 that passed are the
controls. Each failing case received 12 unrequested keys.
- Door: a flow `get_record` on the real stack, `driver-sql` on
better-sqlite3. That is
`get-record-formula-projection.integration.test.ts`, at base
`f2626c71db`: 8 failed of 8, covering both node branches and both
`runAs`. Each served row carried 11 unrequested keys.
- Also measured on the same base: the REST data read, `POST
/api/v1/data/:object/query` with `fields`. It returns the same 13 keys
for a formula projection. The same cut covers it.
### M2: field-level security against the widening
The FLS result mask is `SecurityPlugin.maskOperationResult` →
`FieldMasker.maskResults`. It is step 4 of the security middleware and
runs in the middleware post-phase. That is after `planFormulaProjection`
widened the driver read and after `applyFormulaPlan` ran, on the rows
the read returns.
Measured on base `f2626c71db` with the real `SecurityPlugin` over
`driver-sql`. A member's permission set withheld one field (`readable:
false`) that the projection did not name. The read was `POST
/api/v1/data/:object/query` plus direct `find` / `findOne` with the
member context:
| read (member) | keys returned on base | withheld field present |
|---|---|---|
| `fields: [name, total_price]` | 13: name, total_price, quantity,
unit_price, note, id, organization_id, owner_id,
owning_business_unit_id, created_by, updated_by, created_at, updated_at
| no |
| `fields: [name]` | name | no |
| no projection | the same 13 | no |
**Answer: only columns the caller may read leave.** The widened read
never carried the withheld field. It carried exactly the no-projection
read's column set. That is the contract breach this card names, with no
field-permission bypass. The seat may relay this to triage to drop
`security`. A system-identity caller (for example a flow with `runAs:
system`) has no field mask, so every column is readable to it. Its
widened read is the same contract breach, not a permission bypass.
### M3: what a projection without a formula returns
- `driver-sql`, measured through the door pin's control: `fields: [name,
quantity]` returns exactly `name, quantity`. There is no `id` unless it
is named.
- The engine adds no key to a projection. `PLATFORM_PROVISIONED_COLUMNS`
only admits names in the unknown-plain-field filter.
- So the cut restores exactly the named columns plus the formula value.
The pins assert this as row equality: the formula read equals the same
read without the formula, plus the formula value, on `driver-sql` and on
the driver-sql-shaped double.
- `driver-memory` and `driver-mongodb` behave differently, and that is
not engine behaviour: each adds `id` to every projection at the driver
layer. See the measured reading in Acceptance notes.
### M5: other read paths
`planFormulaProjection` has four callers:
- `find` and `findOne` pass the caller's projection.
- `hydrateWriteFormulas`, the write-result hydration, passes no
projection.
- `evaluateFormulaField` passes one field and evaluates on a copy.
`count`, `aggregate` (its `driver.find` fallback included) and the write
paths' pre-image reads never plan a formula projection.
## Base vs head: keys returned per case
Engine table, driver-sql-shaped double, `find` and `findOne` alike. The
stored row also carries `note`, `parent_id` and the seven provisioned
columns:
| projection | base `f2626c71db` | head |
|---|---|---|
| `[name, total_price]` | 14 keys (every stored column + `total_price`)
| `name, total_price` |
| `[id, owner_id, total_price]` | 14 keys | `id, owner_id, total_price`
|
| `[name, total_price]` + an `afterFind` hook adding a key | 15 keys |
`name, total_price` + the hook's key |
| `[name, quantity]` (control) | `name, quantity` | `name, quantity` |
| none (control) | every declared column + `total_price` | unchanged |
Door, flow `get_record` on `driver-sql`, both branches × both `runAs`:
| `config.fields` | base `f2626c71db` | head |
|---|---|---|
| `[name, total_price]` | 13 keys | `name, total_price` |
| `[name, quantity, total_price]` | 13 keys | `name, quantity,
total_price` = control + `total_price` |
| `[name, quantity]` (control) | `name, quantity` | `name, quantity` |
## Tests
Runs are at head `99ffeb8fa5` unless noted. The one exception is the
full objectql suite, which ran at `aaa5e4deb2`. Between that commit and
head, the only change is the reshaped test double in the new test file,
and that file was re-run at head.
| what | command | result |
|---|---|---|
| objectql, full | `pnpm --filter @objectstack/objectql exec vitest run
--project local --maxWorkers=2` (at `aaa5e4deb2`) | 386 files, 7600
tests passed |
| objectql, repo project | `… --project repo` (at `aaa5e4deb2`) | 1
file, 5 tests passed |
| the new engine pin | `… src/engine-formula-projection-trim.test.ts` |
12 passed |
| objectql typecheck | `pnpm --filter @objectstack/objectql typecheck` |
exit 0. Test-layer debt is unchanged (40 files, 234 errors, 65
signatures), and the new test file is in the `tsconfig.test.json`
program (`--listFilesOnly`) |
| service-automation, full | `pnpm --filter
@objectstack/service-automation exec vitest run --maxWorkers=2` | 177
files, 2165 tests passed |
| service-automation typecheck | `pnpm --filter
@objectstack/service-automation typecheck` | exit 0. Test-layer debt is
0, and the door pin is in the program |
| rest | `pnpm --filter @objectstack/rest exec vitest run --project
local` / `--project repo` | 263 files, 4951 passed and 326 skipped / 5
files, 191 passed and 1 skipped |
**Reverse verification**, at head, through
`scripts/ablation-replace.mjs`:
- **Mutation.** `withoutFormulaWidening` was changed to return every row
untouched (anchor 1 → 0, blob `42aff651c70c` → `45700267e608`).
- **Engine pin** (imports source): 8 failed, 4 passed. The 4 controls
stay green.
- **Built artifact.** The mutant JS was emitted with the marker in 4
built files (`ablation-dist-preflight` exit 0). The mutant's DTS step
failed on the type narrowing the mutation removed (`TS18048`). The JS
the suite consumes does not depend on that step.
- **Door pin** (imports objectql `dist`): 8 failed of 8.
- **Restore.** The blob equals HEAD, `git diff HEAD` is empty, the
rebuild exits 0, the marker is absent from all 14 built files, and the
tree is clean. Afterwards the engine pin passed 12 and the door pin
passed 8.
- **Direction:** red, as expected.
**Gates.** `node scripts/pm/dispatch-gates.mjs --commands` at head
derived 69 commands. All 69 ran and each exited 0.
- `--ran` reconciliation: 69 derived, 69 run, 0 NOT-MEASURED, 0 UNRUN.
The zero is derived from the recorded exit codes.
- `check:objectql-double-limit` first failed on the new test double,
because its probe could not drive it. The double was reshaped in
`99ffeb8fa5`, and the gate now passes.
**Lint, narrowed.** `npx eslint --no-inline-config --format json` on the
three touched `.ts` files at head returned 3 files, 0 errors and 0
warnings, exit 0.
1. **Scope.** `eslint.config.mjs`'s `**/*.{ts,…}` and
`packages/**/*.{ts,…}` blocks cover all three files, and all three come
back in the JSON with results, so none was ignored.
2. **Count.** The JSON shows 3 files.
3. **Why other files cannot change.** `eslint.config.mjs` never enables
type-aware linting: there is no `parserOptions.project` and no typed
rule, as its own comment states. This diff therefore cannot change a
verdict on a file it does not touch.
Repo-wide `pnpm lint` is left to CI.
**NOT MEASURED locally, left to CI:**
- the Test Core shards;
- Dogfood;
- Temporal Conformance;
- Build Core;
- the workspace type-check lanes;
- `driver-mongodb`'s projection, which was read from source only.
## Clause-②
`Clause-②: no`, measured. The built entry declarations of
`@objectstack/objectql` are byte-identical between the merge base
`fe98cc63a4` and head `99ffeb8fa5`. The table gives each file's sha256
prefix, which is the same on both sides:
| file | sha256 prefix |
|---|---|
| `dist/index.d.ts` | `edb995cf7c2baa9f` |
| `dist/core.d.ts` | `3b5e05652e32b56e` |
| `dist/util-BuqCJOyg.d.ts` | `357f6c243d69e450` |
The new helpers are module-private. What changes is runtime behaviour: a
projection that names a formula field now returns that projection, which
is the direction the card asks for.
## Acceptance notes
- **`id` on two drivers.** `driver-memory` and `driver-mongodb` add `id`
to every projection at the driver layer (`InMemoryDriver.projectFields`,
`MongoDBDriver.buildFindOptions`). The `driver-sql` family does not, and
no driver contract declares either behaviour.
- Measured at head with `driver-memory` through the built engine:
`fields: [name]` returns `name, id`, while `fields: [name, total_price]`
returns `name, total_price`.
- So on those two drivers, a formula projection now omits the `id` that
the same projection without a formula carries. On base it carried `id`
together with every other column.
- Carrier: none for the driver-level divergence (no contract declares
either behaviour; the contract review 6066975867 answered A).
- **Formula evaluation and field-level security.** The formula pass
evaluates against the row before the field-level-security result mask
runs, because the mask runs in the middleware post-phase. That holds on
every read, with or without a projection.
- Whether a formula over a withheld field should be evaluated for that
caller is not measured here, and this PR does not change it. Carrier:
none.
- **What `afterFind` hooks see.** `afterFind` hooks still read the
widened row, as before. If a hook re-assigns a key that the widening
added, the cut removes it, because the caller never named it.
## Files
- `packages/objectql/src/engine.ts`: the read path's formula projection
region only, meaning `planFormulaProjection`, the new cut helpers and
the two `return`s.
- `packages/objectql/src/engine-formula-projection-trim.test.ts`: new.
-
`packages/services/service-automation/src/builtin/get-record-formula-projection.integration.test.ts`:
new, the door pin. It is cross-lane (`domain:services`) and only adds a
test.
- `.changeset/22300-formula-projection-trim.md`: `@objectstack/objectql`
patch.
## Patch round 1
Head `b6dbe39576` adds one commit on top of `99ffeb8fa5`. It changes one
sentence in `.changeset/22300-formula-projection-trim.md` and nothing
else.
- **Why.** The old sentence said `id` is returned "when it is named, as
for any other projection". That is false on `driver-memory` and
`driver-mongodb`, which add `id` to every projection at the driver
layer.
- **New text:** "`id` is returned only when it is named. `driver-memory`
and `driver-mongodb` add `id` to every projection at the driver layer,
so on those two drivers a projection that names a formula field but not
`id` no longer carries `id`, while the same projection without the
formula still does: name `id` when you need it."
- **Each clause, checked against the code:**
- On every driver, the cut removes `id` whenever the widening added it,
which is whenever the caller did not name it.
- `InMemoryDriver.projectFields` and `MongoDBDriver.buildFindOptions`
add `id` to any non-empty projection, on `find` and `findOne` alike.
- Measured on `driver-memory`: `[name]` returns `name, id`, and `[name,
total_price]` returns `name, total_price`.
- **Unchanged.** No code or test changed, so the test runs and the
`.d.ts` comparison above still hold: a changeset is not part of any
build.
- **Gates for the touched path, at `b6dbe39576`.** `dispatch-gates
--commands .changeset/22300-formula-projection-trim.md` derived 20
commands, including `check-changeset-no-major --base origin/main`,
`check-empty-changeset --base origin/main` and
`check-adr-0087-registration --base origin/main`. All 20 exited 0.
`--ran` reconciliation: 20 derived, 20 run, 0 NOT-MEASURED, 0 UNRUN.
- **Merge check.** `git merge-tree --write-tree` against `origin/main`
`3599fef123` is clean: exit 0, no conflicts. `main` has touched none of
this PR's four files since the merge base, so nothing was merged.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01EUBvqtauTDmHi2ZgY759p2)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 35afb15 commit e87070e
4 files changed
Lines changed: 443 additions & 4 deletions
File tree
- .changeset
- packages
- objectql/src
- services/service-automation/src/builtin
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
Lines changed: 205 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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1534 | 1534 | | |
1535 | 1535 | | |
1536 | 1536 | | |
| 1537 | + | |
| 1538 | + | |
| 1539 | + | |
| 1540 | + | |
| 1541 | + | |
| 1542 | + | |
| 1543 | + | |
| 1544 | + | |
| 1545 | + | |
| 1546 | + | |
| 1547 | + | |
| 1548 | + | |
1537 | 1549 | | |
1538 | 1550 | | |
1539 | 1551 | | |
1540 | | - | |
| 1552 | + | |
1541 | 1553 | | |
1542 | 1554 | | |
1543 | 1555 | | |
| |||
1578 | 1590 | | |
1579 | 1591 | | |
1580 | 1592 | | |
1581 | | - | |
| 1593 | + | |
| 1594 | + | |
| 1595 | + | |
| 1596 | + | |
| 1597 | + | |
1582 | 1598 | | |
1583 | 1599 | | |
1584 | 1600 | | |
1585 | 1601 | | |
1586 | 1602 | | |
1587 | 1603 | | |
| 1604 | + | |
| 1605 | + | |
| 1606 | + | |
| 1607 | + | |
| 1608 | + | |
| 1609 | + | |
| 1610 | + | |
| 1611 | + | |
| 1612 | + | |
| 1613 | + | |
| 1614 | + | |
| 1615 | + | |
| 1616 | + | |
| 1617 | + | |
| 1618 | + | |
| 1619 | + | |
| 1620 | + | |
| 1621 | + | |
| 1622 | + | |
| 1623 | + | |
| 1624 | + | |
| 1625 | + | |
| 1626 | + | |
| 1627 | + | |
| 1628 | + | |
| 1629 | + | |
| 1630 | + | |
| 1631 | + | |
| 1632 | + | |
| 1633 | + | |
| 1634 | + | |
| 1635 | + | |
| 1636 | + | |
| 1637 | + | |
| 1638 | + | |
| 1639 | + | |
| 1640 | + | |
| 1641 | + | |
| 1642 | + | |
| 1643 | + | |
| 1644 | + | |
| 1645 | + | |
| 1646 | + | |
| 1647 | + | |
| 1648 | + | |
| 1649 | + | |
| 1650 | + | |
| 1651 | + | |
| 1652 | + | |
| 1653 | + | |
| 1654 | + | |
| 1655 | + | |
1588 | 1656 | | |
1589 | 1657 | | |
1590 | 1658 | | |
| |||
12097 | 12165 | | |
12098 | 12166 | | |
12099 | 12167 | | |
| 12168 | + | |
| 12169 | + | |
| 12170 | + | |
12100 | 12171 | | |
12101 | 12172 | | |
12102 | 12173 | | |
| |||
12233 | 12304 | | |
12234 | 12305 | | |
12235 | 12306 | | |
12236 | | - | |
| 12307 | + | |
| 12308 | + | |
| 12309 | + | |
12237 | 12310 | | |
12238 | 12311 | | |
12239 | 12312 | | |
| |||
12400 | 12473 | | |
12401 | 12474 | | |
12402 | 12475 | | |
| 12476 | + | |
| 12477 | + | |
12403 | 12478 | | |
12404 | 12479 | | |
12405 | 12480 | | |
| |||
12502 | 12577 | | |
12503 | 12578 | | |
12504 | 12579 | | |
12505 | | - | |
| 12580 | + | |
| 12581 | + | |
12506 | 12582 | | |
12507 | 12583 | | |
12508 | 12584 | | |
| |||
0 commit comments