Repository navigation
Commit 5259a35
Fixes #21682
Clause-②: no
## What changes
This applies on a walled posture with the real
`@objectstack/organizations` runtime mounted:
- **Before:** a create that named no `organization_id` answered 201 with
`droppedFields: [{ object, fields: ['organization_id'], reason:
'readonly' }]`. The caller never sent that key.
- **After:** the same create answers 201 with no `droppedFields`. The
row is still stored in the active organization.
The fix sits where triage routed it: `ObjectQL.insert`'s caller-payload
snapshot in `packages/objectql/src/engine.ts`.
- At `insert`'s entry, before the middleware chain runs, the engine
records the keys each row carries. The record is keyed by the row object
(`callerKeySets`, line 2094; called at line 12879).
- The snapshot `suppliedPerRow` (line 12983) now keeps only those keys
(`callerSuppliedRow`, line 2113).
- Values are unchanged, so every key the caller does send is judged as
before.
- No field is named anywhere. A key counts as the platform's because of
WHEN it appeared on the payload, not because of its name.
## The console reading (H4), measured first: the console warns, so this
is the p2 raise reading
objectui at its pin `ab187972159583b595facdcae3c73b50f6f312e9`
(`.objectui-sha`), read from a shallow clone that has since been
deleted:
- **Every adapter create reports.**
`packages/data-objectstack/src/index.ts`: `ObjectStackAdapter.create`
calls `notifyDroppedFields('create', resource, result, id, data)` on
every create.
- **The filter keeps a field the caller never sent.**
`notifyDroppedFields` (line 3767) runs the entries through
`withoutNoOpDrops(valid, sent, stored)` (line 2794). That filter keeps
such a field: `if (!Object.prototype.hasOwnProperty.call(sent, f))
return true;`. It drops only a field whose sent value equals the stored
value.
- **The shell turns it into a toast.**
`packages/app-shell/src/providers/AdapterProvider.tsx` line 80
subscribes `onWriteWarning` and passes sonner's `toast` to
`emitWriteWarning`. For any non-empty list, `emitWriteWarning`
(`writeWarningToast.ts`) calls `sink.warning` with the title "Saved —
but some fields did not take effect" and the line "Read-only, so it did
not take effect: " followed by the field's label.
- **The record forms go through that adapter.** These call
`dataSource.create`: `plugin-form`'s `ObjectForm.tsx:1281`,
`ModalForm.tsx:660`, `DrawerForm.tsx:574`, `SplitForm.tsx:399`,
`TabbedForm.tsx:501` and `WizardForm.tsx:978`, plus
`ObjectCalendar.tsx:1190` and the grid's `ImportWizard.tsx:1931`.
- **Measured as well as read.** I extracted the pinned
`withoutNoOpDrops`, `sameWireValue` and `stableStringify` verbatim and
ran them on the card's wire shape:
- entries: `[{ object: 'qa_ledger', fields: ['organization_id'], reason:
'readonly' }]`;
- sent: `{ name }`;
- stored: a record carrying `organization_id`.
- Output: the entry is kept (`toast fires: true`).
- **One console path does not toast.**
`apps/console/src/components/FormPage.tsx` posts its internal submit
with raw `fetch` (line 1375) and never reads `droppedFields`.
So an ordinary walled create from the console's record forms shows the
amber "Saved — but some fields did not take effect" toast, naming
Organization. That is #8093's symptom, on create. The seat raises the
grade on this reading.
## Mechanism, measured (Partition 2)
- **H1, confirmed by measurement.**
- At `251a7dd4b4`, `insert` took `suppliedPerRow` (line 12905) inside
the callback it hands `executeWithMiddleware` (line 12828). By then
every middleware had already run.
- Middleware A (`organizations-plugin.ts:344`) fills an absent or empty
`organization_id` in place, so the snapshot recorded the fill as a
caller key.
- The static-readonly strip took it, because the injected column is
`readonly: true`, and reported it. Further down, the driver's
`injectTenantOnInsert` filled the column again from the context. That is
why no stored data was wrong.
- The new pins, run against the unmodified engine, returned `[{ fields:
['organization_id'], reason: 'readonly' }]` for both a member and a
platform administrator.
- **H2, the precedent applied.**
- #8093 (commit `a0fdc564eb`) narrowed the update report by a provenance
fact (a payload `id` equal to the bound row), not by a field name.
- This change takes the first arm of triage's direction: the caller key
set is recorded before any platform middleware runs.
- The update path already takes its `suppliedValues` snapshot before its
chain (`engine.ts:14054`, chain at line 14200). Insert now matches it
for keys.
- I did not take the other arm (marking each stamped key). It would have
needed a recording Proxy handed to every middleware, and to every batch
row inside it.
- **H3, explained.**
- Middleware A fills only a non-array payload
(`!Array.isArray(opCtx.data)`). So a `createMany` row carries no fill
and never reported one. The driver fills the column for it.
- Without the runtime (`single`) there is no fill at all.
- Both controls are pinned. They are green before and after.
- **H5, measured.**
- On insert, the in-tree middlewares that write a value into
`opCtx.data` are two:
- `@objectstack/organizations` Middleware A, for `organization_id`
(single row only);
- `@objectstack/plugin-security` step 3.5, for `owner_id` (every row,
`security-plugin.ts:3230`).
- The rest filter reads, act after `next()`, or (the anonymous
public-form branch) delete keys rather than fill them.
- `owner_id` is not `readonly`, so it never reached `droppedFields`. It
did reach the referential-integrity check (see Behaviour notes).
- The walled pins assert `droppedFields` is empty, which covers both
fills. The engine pins use an arbitrary author column, so no name is
involved.
## Pins
These are triage's three, on a walled boot built from the real
`@objectstack/organizations` runtime, the real `SecurityPlugin`, a real
`ObjectQL` and `SqliteWasmDriver`. The block is added to #21666's
harness in
`packages/plugins/organizations/src/create-explicit-organization-wall.test.ts`.
The drops are collected through `onFieldsDropped`, the listener the REST
create doors (`createData`, `createManyData`) answer `droppedFields`
from.
1. A walled create with no `organization_id` reports no dropped field
and is stored in the active organization. Run for a member and for a
platform administrator.
2. A caller that sends a readonly key still sees it reported:
- `organization_id` itself, sent explicitly, is reported;
- `created_by`, sent beside the fill, is reported alone.
3. `createMany` and the `single` posture are unchanged:
- an array row naming no organization reports nothing;
- under `single`, booted without the runtime, a create naming no
organization reports nothing, and one naming an organization still
reports it.
There are three engine pins in
`packages/objectql/src/engine-insert-static-readonly-strip.test.ts`,
beside the existing exemption "a beforeInsert hook's OWN stamp
survives":
- a middleware fill on an author `readonly` column lands and is not
reported;
- the same holds per row on a batch, beside a key the caller did send,
which is still taken and reported;
- a key the caller sent is judged as before even when a middleware
rewrote its value.
## Reverse verification
Run from committed state, with HEAD `81ad21efc1` carrying the fix:
- **The mutation.** `scripts/ablation-replace.mjs` changed
`callerSuppliedRow(row, callerKeysPerRow[i])` to `callerSuppliedRow(row,
undefined)`. That restores the full post-middleware copy, which is the
pre-fix snapshot.
- **It landed on disk.** The anchor count went 1 to 0 and the
replacement count 0 to 1. The blob went from `16159cd42cbf` to
`9fae7a2c1837`.
- **No build was needed.** Both suites resolve the subject from source:
the organizations config aliases `@objectstack/objectql` to `src`, and
the engine test imports `./engine.js`.
- **Predicted, then observed:**
- organizations file: 3 failed, 18 passed. The failures are pin 1 for
both callers, plus pin 2's "beside the fill" case, which also asserts
pin 1's absence. Pin 2's explicit case and every pin 3 case stayed
green.
- engine file: 2 failed, 19 passed. The failures are the two fill cases
(`expected undefined to be 'stamp'`; `expected [ undefined, undefined ]
to deeply equal [ 'stamp', 'stamp' ]`). The rewritten-value case stayed
green.
- **Restored.** `git checkout HEAD -- packages/objectql/src/engine.ts`,
run by the tool's trap. Blob after restore `16159cd42cbf` equals HEAD,
and `git diff HEAD` is empty.
## Behaviour notes
- **The referential-integrity check moves with the snapshot.**
- `assertReferencesResolve` reads `suppliedPerRow` to decide what the
caller sent. Its docblock already scopes it to caller-supplied keys and
names `owner_id` and `organization_id` as stamps it must not judge.
- A middleware fill used to be judged anyway. I measured this once with
a scratch probe (deleted, not committed): a middleware fills a `lookup`
with a dangling id on a create that did not name it.
- Before: refused `VALIDATION_FAILED`. After: created.
- The in-tree fill this reaches is plugin-security's `owner_id` fill:
its stamp was checked as if the caller had sent it. I did not measure a
principal that the check actually refused for it.
- **Values are unchanged for keys the caller sent.** That includes a
caller-sent empty `organization_id` that Middleware A fills. It is still
reported, as before. The update path's snapshot also captures values
before its chain; this change aligns the KEY set only.
## Tests (at `47cbb883b3`)
The test runs were taken at `47cbb883b3`, whose code equals the fix
commit `81ad21efc1`. The merge of `origin/main` (`994ec65025`) brought
in only a `service-analytics` test and `scripts/pm/git-history.mjs`,
neither in `objectql` nor `organizations`, so the suites were not rerun
after it. Every run went through `scripts/pm/os-verify-lock.sh` with
`--maxWorkers=2`.
- **Before the fix**, with the pins at `e95548845c` over the unmodified
engine: `pnpm --filter @objectstack/organizations exec vitest run
src/create-explicit-organization-wall.test.ts` gave 3 failed and 18
passed. The received value was `[{ fields: ['organization_id'], object:
'qa_ledger', reason: 'readonly' }]`.
- **After the fix:**
- the same file: 21 passed;
- `pnpm --filter @objectstack/objectql exec vitest run
src/engine-insert-static-readonly-strip.test.ts`: 21 passed (18
existing, 3 new);
- `pnpm --filter @objectstack/objectql exec vitest run --project local`
(the package's `test` script): 371 files, 7450 tests passed;
- `pnpm --filter @objectstack/organizations test`: 9 files, 130 passed
(123 existing, 7 new);
- `pnpm --filter @objectstack/objectql typecheck`: exit 0.
`check:test-typecheck` reports OK, with 40 files, 234 errors and 65
signatures held in the ledger, unchanged;
- `pnpm --filter @objectstack/organizations typecheck`: exit 0, test
layer 0 errors.
- **Builds:**
- the dependency closure, `turbo run build
--filter='@objectstack/organizations^...'`: 27 of 27 tasks;
- `objectql` and `organizations`, rebuilt for the gates that read
`dist/` (the fix is present in `dist/index.mjs`);
- a full `--filter='!@objectstack/docs'` build, 72 of 72 tasks (71
cached), for `check:dual-build-cjs-loads`.
- **Lint, a declared narrowing.** I ran `eslint --no-inline-config
--format json` on the 3 TypeScript files this diff touches: 3 files
linted, 0 errors, 0 warnings, none ignored (`--print-config` exits 0 for
each). Why the narrowing excludes nothing:
- `eslint.config.mjs` enables no type-aware linting (its own note at
lines 327-328: no `parserOptions.project`, no typed rules);
- its plugins are local per-file AST rules;
- its only external reads are `scripts/slot-lookup-baseline.json` and
`scripts/query-options-erasure-baseline.json`, which this diff does not
touch.
So this diff cannot move a verdict on an untouched file. The repo-wide
`pnpm lint` stays with CI.
## Gates
The final run was at `994ec65025`, after the merge. `node
scripts/pm/dispatch-gates.mjs --commands` was run with no paths. It
derives 70 commands from the 4-path change set against merge base
`38bef8cf9`, the same 70 as before the merge.
- **The 70 derived:** all exit 0. `dispatch-gates --ran` reports "70
derived, 70 run, 0 NOT-MEASURED, 0 UNRUN".
- `check:dual-build-cjs-loads` exited 3 (PREREQUISITE NOT MET, 39
packages without `dist`) in the first run at `47cbb883b3`. After the
full build it exited 0, and it exited 0 again at `994ec65025`.
- Some verdict lines: `check:nul-bytes` OK (10060 files, no control
bytes); `check:engine-double-contract` OK (936 pinned);
`check:test-source-alias` OK; `check:cross-package-test-inputs` OK;
`check-adr-0087-registration` (1 non-breaking changeset);
`check-changeset-no-major` (no major); `check-empty-changeset` OK;
`check:type-check-coverage` OK.
- **The artifact-roster block printed outside the total:** 54 families,
53 of them not in the derived set. 51 exit 0.
- 3 exit 2 NOT WIRED, because they need pull-request context:
`check-closing-target-claim`, `check-partof-closing-keyword` and
`check-single-claim-paths`.
- `check-partof-closing-keyword` was also run on this body with
`PR_BODY` set, and exited 0.
- The other two can only run once this PR exists. Their readings are in
the os-dev report on the card.
## Acceptance notes
These are noted, not filed: none is a reproducible defect, a contract
violation or an authoring trap that this card measured.
- **Values still differ from the update path.** Insert now matches
update for the caller KEY set. Update also captures VALUES before its
chain; insert keeps the value the chain handed on. So a caller-sent key
whose value a middleware rewrote is judged on the middleware's value on
insert, and on the caller's value on update. No in-tree insert
middleware rewrites a non-empty caller value since #21666: Middleware A
only fills an absent or empty value, and plugin-security refuses a
forged owner rather than rewriting it. This is unexercised drift, and no
PR or person will carry it.
- **Middleware A's fill now reaches the driver.** On a single-row
create, Middleware A's fill and the driver's `injectTenantOnInsert` give
the same value. Before, the fill was stripped and the driver filled the
column again. Now the fill is kept.
- **objectui's `FormPage.tsx` drops the report.** Its internal submit
posts with raw `fetch` and never reads `droppedFields`, so a strip on
that path is silent. That is at objectui pin `ab187972`; no PR or person
will carry it.
- **objectui keeps never-sent fields.** objectui's `withoutNoOpDrops`
keeps a reported field the caller never sent. With this change the
server no longer reports a middleware fill, so that branch no longer
fires for one.
---
_Generated by [Claude
Code](https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi)_
---------
Co-authored-by: Claude <noreply@anthropic.com>
1 parent 5ac2ba1 commit 5259a35
4 files changed
Lines changed: 256 additions & 11 deletions
File tree
- .changeset
- packages
- objectql/src
- plugins/organizations/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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
228 | 228 | | |
229 | 229 | | |
230 | 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 | + | |
231 | 278 | | |
232 | 279 | | |
233 | 280 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2063 | 2063 | | |
2064 | 2064 | | |
2065 | 2065 | | |
| 2066 | + | |
| 2067 | + | |
| 2068 | + | |
| 2069 | + | |
| 2070 | + | |
| 2071 | + | |
| 2072 | + | |
| 2073 | + | |
| 2074 | + | |
| 2075 | + | |
| 2076 | + | |
| 2077 | + | |
| 2078 | + | |
| 2079 | + | |
| 2080 | + | |
| 2081 | + | |
| 2082 | + | |
| 2083 | + | |
| 2084 | + | |
| 2085 | + | |
| 2086 | + | |
| 2087 | + | |
| 2088 | + | |
| 2089 | + | |
| 2090 | + | |
| 2091 | + | |
| 2092 | + | |
| 2093 | + | |
| 2094 | + | |
| 2095 | + | |
| 2096 | + | |
| 2097 | + | |
| 2098 | + | |
| 2099 | + | |
| 2100 | + | |
| 2101 | + | |
| 2102 | + | |
| 2103 | + | |
| 2104 | + | |
| 2105 | + | |
| 2106 | + | |
| 2107 | + | |
| 2108 | + | |
| 2109 | + | |
| 2110 | + | |
| 2111 | + | |
| 2112 | + | |
| 2113 | + | |
| 2114 | + | |
| 2115 | + | |
| 2116 | + | |
| 2117 | + | |
| 2118 | + | |
| 2119 | + | |
| 2120 | + | |
| 2121 | + | |
| 2122 | + | |
2066 | 2123 | | |
2067 | 2124 | | |
2068 | 2125 | | |
| |||
12831 | 12888 | | |
12832 | 12889 | | |
12833 | 12890 | | |
| 12891 | + | |
| 12892 | + | |
| 12893 | + | |
| 12894 | + | |
12834 | 12895 | | |
12835 | 12896 | | |
12836 | 12897 | | |
| |||
12858 | 12919 | | |
12859 | 12920 | | |
12860 | 12921 | | |
| 12922 | + | |
| 12923 | + | |
| 12924 | + | |
| 12925 | + | |
| 12926 | + | |
| 12927 | + | |
| 12928 | + | |
| 12929 | + | |
12861 | 12930 | | |
12862 | 12931 | | |
12863 | 12932 | | |
| |||
12893 | 12962 | | |
12894 | 12963 | | |
12895 | 12964 | | |
12896 | | - | |
12897 | | - | |
12898 | | - | |
12899 | | - | |
| 12965 | + | |
| 12966 | + | |
| 12967 | + | |
| 12968 | + | |
| 12969 | + | |
| 12970 | + | |
| 12971 | + | |
| 12972 | + | |
| 12973 | + | |
| 12974 | + | |
| 12975 | + | |
12900 | 12976 | | |
12901 | 12977 | | |
12902 | 12978 | | |
| |||
12918 | 12994 | | |
12919 | 12995 | | |
12920 | 12996 | | |
12921 | | - | |
| 12997 | + | |
12922 | 12998 | | |
12923 | 12999 | | |
12924 | 13000 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
42 | 42 | | |
43 | 43 | | |
44 | 44 | | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
45 | 54 | | |
46 | 55 | | |
47 | 56 | | |
48 | 57 | | |
49 | 58 | | |
50 | 59 | | |
51 | 60 | | |
| 61 | + | |
52 | 62 | | |
53 | 63 | | |
54 | 64 | | |
| |||
108 | 118 | | |
109 | 119 | | |
110 | 120 | | |
111 | | - | |
| 121 | + | |
112 | 122 | | |
113 | 123 | | |
114 | 124 | | |
| |||
125 | 135 | | |
126 | 136 | | |
127 | 137 | | |
128 | | - | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
129 | 143 | | |
130 | 144 | | |
131 | 145 | | |
| |||
170 | 184 | | |
171 | 185 | | |
172 | 186 | | |
173 | | - | |
| 187 | + | |
174 | 188 | | |
175 | 189 | | |
176 | | - | |
| 190 | + | |
177 | 191 | | |
178 | | - | |
| 192 | + | |
179 | 193 | | |
180 | | - | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
181 | 199 | | |
182 | 200 | | |
183 | 201 | | |
| |||
326 | 344 | | |
327 | 345 | | |
328 | 346 | | |
| 347 | + | |
| 348 | + | |
| 349 | + | |
| 350 | + | |
| 351 | + | |
| 352 | + | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
| 372 | + | |
| 373 | + | |
| 374 | + | |
| 375 | + | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
| 383 | + | |
| 384 | + | |
| 385 | + | |
| 386 | + | |
| 387 | + | |
| 388 | + | |
| 389 | + | |
| 390 | + | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
| 394 | + | |
| 395 | + | |
| 396 | + | |
| 397 | + | |
| 398 | + | |
| 399 | + | |
| 400 | + | |
| 401 | + | |
| 402 | + | |
| 403 | + | |
| 404 | + | |
| 405 | + | |
| 406 | + | |
| 407 | + | |
| 408 | + | |
| 409 | + | |
| 410 | + | |
| 411 | + | |
| 412 | + | |
| 413 | + | |
| 414 | + | |
| 415 | + | |
| 416 | + | |
| 417 | + | |
| 418 | + | |
| 419 | + | |
| 420 | + | |
| 421 | + | |
| 422 | + | |
| 423 | + | |
| 424 | + | |
| 425 | + | |
| 426 | + | |
| 427 | + | |
| 428 | + | |
0 commit comments