fix(objectql): droppedFields on a create names only keys the caller sent, never a middleware fill (#21682) - #21701
Conversation
…no organization_id (#21682) The pins come first so the red on unmodified main is measured. The fix follows. Claude-Session: https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi Co-authored-by: Claude <noreply@anthropic.com>
…ent, never a middleware fill (#21682) ObjectQL.insert took its caller snapshot (suppliedPerRow) inside the middleware chain's innermost step, after a write middleware had already filled the payload. On a walled posture @objectstack/organizations fills an absent organization_id, so the static-readonly strip took the platform's own value and droppedFields reported it on every create that named no organization. The console shows every non-empty droppedFields as a warning toast. The keys each row carries are now recorded at insert's entry, before the chain runs, keyed by the row object. The snapshot keeps only those keys. The values stay as the chain handed them on, so every key the caller did send is judged as before. There is no name list. Claude-Session: https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi Co-authored-by: Claude <noreply@anthropic.com>
… no organization (#21682) Claude-Session: https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi Co-authored-by: Claude <noreply@anthropic.com>
…opped-fields-platform-stamp
📓 Docs Drift Check2 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run. What this run could not see
Coarse fallback — 17 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin af9afeec9034ccc9a908f87a570fb1a629334d21 && git checkout af9afeec9034ccc9a908f87a570fb1a629334d21
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 16d241a6af00be4acce9883190fc333a5f560825 b7d2799c8d7d941362a5790bba5c56cf8186780c && git checkout -B drift-repro 16d241a6af00be4acce9883190fc333a5f560825 && git merge --no-ff b7d2799c8d7d941362a5790bba5c56cf8186780c
node scripts/docs-audit/affected-docs.mjs --json 16d241a6af00be4acce9883190fc333a5f560825 |
ACCEPT — PR #21701 at head
|
⛔ merge queue 构建失败 — 先分诊,再决定要不要重排队列构建 37186085048 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集), 失败的 job(日志抽取,best effort):
跨 PR 相同签名(24h,按失败测试文件聚合):
历史信号:
分诊清单:
Generated by Claude Code · merge-queue-triage workflow (#4859) |
…opped-fields-platform-stamp
…h a valid instant (#21682) The readonly value shape check that landed on main judges every readonly value left on an insert row. The pin's fill of a readonly datetime was the string 'stamp', which that check refuses with invalid_date. It now fills a valid ISO-8601 instant. What the pin proves is unchanged: a middleware fill lands, and it is not reported. Claude-Session: https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi Co-authored-by: Claude <noreply@anthropic.com>
ACCEPT addendum — PR #21701, patch round 1, head
|
⛔ merge queue 构建失败 — 先分诊,再决定要不要重排队列构建 37191123449 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集), 失败的 job(日志抽取,best effort):
跨 PR 相同签名(24h,按失败测试文件聚合):
历史信号:
分诊清单:
Generated by Claude Code · merge-queue-triage workflow (#4859) |
Fixes #21682
Clause-②: no
What changes
This applies on a walled posture with the real
@objectstack/organizationsruntime mounted:organization_idanswered 201 withdroppedFields: [{ object, fields: ['organization_id'], reason: 'readonly' }]. The caller never sent that key.droppedFields. The row is still stored in the active organization.The fix sits where triage routed it:
ObjectQL.insert's caller-payload snapshot inpackages/objectql/src/engine.ts.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).suppliedPerRow(line 12983) now keeps only those keys (callerSuppliedRow, line 2113).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:packages/data-objectstack/src/index.ts:ObjectStackAdapter.createcallsnotifyDroppedFields('create', resource, result, id, data)on every create.notifyDroppedFields(line 3767) runs the entries throughwithoutNoOpDrops(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.packages/app-shell/src/providers/AdapterProvider.tsxline 80 subscribesonWriteWarningand passes sonner'stoasttoemitWriteWarning. For any non-empty list,emitWriteWarning(writeWarningToast.ts) callssink.warningwith 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.dataSource.create:plugin-form'sObjectForm.tsx:1281,ModalForm.tsx:660,DrawerForm.tsx:574,SplitForm.tsx:399,TabbedForm.tsx:501andWizardForm.tsx:978, plusObjectCalendar.tsx:1190and the grid'sImportWizard.tsx:1931.withoutNoOpDrops,sameWireValueandstableStringifyverbatim and ran them on the card's wire shape:[{ object: 'qa_ledger', fields: ['organization_id'], reason: 'readonly' }];{ name };organization_id.toast fires: true).apps/console/src/components/FormPage.tsxposts its internal submit with rawfetch(line 1375) and never readsdroppedFields.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)
251a7dd4b4,inserttooksuppliedPerRow(line 12905) inside the callback it handsexecuteWithMiddleware(line 12828). By then every middleware had already run.organizations-plugin.ts:344) fills an absent or emptyorganization_idin place, so the snapshot recorded the fill as a caller key.readonly: true, and reported it. Further down, the driver'sinjectTenantOnInsertfilled the column again from the context. That is why no stored data was wrong.[{ fields: ['organization_id'], reason: 'readonly' }]for both a member and a platform administrator.a0fdc564eb) narrowed the update report by a provenance fact (a payloadidequal to the bound row), not by a field name.suppliedValuessnapshot before its chain (engine.ts:14054, chain at line 14200). Insert now matches it for keys.!Array.isArray(opCtx.data)). So acreateManyrow carries no fill and never reported one. The driver fills the column for it.single) there is no fill at all.opCtx.dataare two:@objectstack/organizationsMiddleware A, fororganization_id(single row only);@objectstack/plugin-securitystep 3.5, forowner_id(every row,security-plugin.ts:3230).next(), or (the anonymous public-form branch) delete keys rather than fill them.owner_idis notreadonly, so it never reacheddroppedFields. It did reach the referential-integrity check (see Behaviour notes).droppedFieldsis 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/organizationsruntime, the realSecurityPlugin, a realObjectQLandSqliteWasmDriver. The block is added to #21666's harness inpackages/plugins/organizations/src/create-explicit-organization-wall.test.ts. The drops are collected throughonFieldsDropped, the listener the REST create doors (createData,createManyData) answerdroppedFieldsfrom.organization_idreports no dropped field and is stored in the active organization. Run for a member and for a platform administrator.organization_iditself, sent explicitly, is reported;created_by, sent beside the fill, is reported alone.createManyand thesingleposture are unchanged: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":readonlycolumn lands and is not reported;Reverse verification
Run from committed state, with HEAD
81ad21efc1carrying the fix:scripts/ablation-replace.mjschangedcallerSuppliedRow(row, callerKeysPerRow[i])tocallerSuppliedRow(row, undefined). That restores the full post-middleware copy, which is the pre-fix snapshot.16159cd42cbfto9fae7a2c1837.@objectstack/objectqltosrc, and the engine test imports./engine.js.expected undefined to be 'stamp';expected [ undefined, undefined ] to deeply equal [ 'stamp', 'stamp' ]). The rewritten-value case stayed green.git checkout HEAD -- packages/objectql/src/engine.ts, run by the tool's trap. Blob after restore16159cd42cbfequals HEAD, andgit diff HEADis empty.Behaviour notes
assertReferencesResolvereadssuppliedPerRowto decide what the caller sent. Its docblock already scopes it to caller-supplied keys and namesowner_idandorganization_idas stamps it must not judge.lookupwith a dangling id on a create that did not name it.VALIDATION_FAILED. After: created.owner_idfill: its stamp was checked as if the caller had sent it. I did not measure a principal that the check actually refused for it.organization_idthat 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 commit81ad21efc1. The merge oforigin/main(994ec65025) brought in only aservice-analyticstest andscripts/pm/git-history.mjs, neither inobjectqlnororganizations, so the suites were not rerun after it. Every run went throughscripts/pm/os-verify-lock.shwith--maxWorkers=2.Before the fix, with the pins at
e95548845cover the unmodified engine:pnpm --filter @objectstack/organizations exec vitest run src/create-explicit-organization-wall.test.tsgave 3 failed and 18 passed. The received value was[{ fields: ['organization_id'], object: 'qa_ledger', reason: 'readonly' }].After the fix:
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'stestscript): 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-typecheckreports 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:
turbo run build --filter='@objectstack/organizations^...': 27 of 27 tasks;objectqlandorganizations, rebuilt for the gates that readdist/(the fix is present indist/index.mjs);--filter='!@objectstack/docs'build, 72 of 72 tasks (71 cached), forcheck:dual-build-cjs-loads.Lint, a declared narrowing. I ran
eslint --no-inline-config --format jsonon the 3 TypeScript files this diff touches: 3 files linted, 0 errors, 0 warnings, none ignored (--print-configexits 0 for each). Why the narrowing excludes nothing:eslint.config.mjsenables no type-aware linting (its own note at lines 327-328: noparserOptions.project, no typed rules);scripts/slot-lookup-baseline.jsonandscripts/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 lintstays with CI.Gates
The final run was at
994ec65025, after the merge.node scripts/pm/dispatch-gates.mjs --commandswas run with no paths. It derives 70 commands from the 4-path change set against merge base38bef8cf9, the same 70 as before the merge.dispatch-gates --ranreports "70 derived, 70 run, 0 NOT-MEASURED, 0 UNRUN".check:dual-build-cjs-loadsexited 3 (PREREQUISITE NOT MET, 39 packages withoutdist) in the first run at47cbb883b3. After the full build it exited 0, and it exited 0 again at994ec65025.check:nul-bytesOK (10060 files, no control bytes);check:engine-double-contractOK (936 pinned);check:test-source-aliasOK;check:cross-package-test-inputsOK;check-adr-0087-registration(1 non-breaking changeset);check-changeset-no-major(no major);check-empty-changesetOK;check:type-check-coverageOK.check-closing-target-claim,check-partof-closing-keywordandcheck-single-claim-paths.check-partof-closing-keywordwas also run on this body withPR_BODYset, and exited 0.Acceptance notes
These are noted, not filed: none is a reproducible defect, a contract violation or an authoring trap that this card measured.
injectTenantOnInsertgive the same value. Before, the fill was stripped and the driver filled the column again. Now the fill is kept.FormPage.tsxdrops the report. Its internal submit posts with rawfetchand never readsdroppedFields, so a strip on that path is silent. That is at objectui pinab187972; no PR or person will carry it.withoutNoOpDropskeeps 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