Repository navigation
sys_user_preference update reports droppedFields:[id] for a key the caller never sent — every org switch pops a user-facing warning toast #8093
Description
Switching the active organization (and, more generally, visiting any object page) pops a user-facing amber warning toast:
已保存，但部分字段未生效
以下字段为只读，未生效: 偏好设置 ID
Nothing the user edited was dropped. The write is the console's internal "recent items" trace.
Actual
The trigger is the ui.recent preference write. Response body of the PATCH behind the toast:
PATCH /api/v1/data/sys_user_preference/4mekbFDEhx0QgC85 → 200
{
"object": "sys_user_preference",
"id": "4mekbFDEhx0QgC85",
"record": { "id": "4mekbFDEhx0QgC85", ..., "key": "ui.recent", "value": [ ... ] },
"droppedFields": [ { "object": "sys_user_preference", "fields": ["id"], "reason": "readonly" } ]
}
The dropped field is id — the record's own primary key, carried in the URL path, and the client's update body does not contain it. ObjectStackDataSource.update() forwards exactly the data argument (packages/data-objectstack/src/index.ts:1573), and the caller passes only { value: items } (packages/data-objectstack/src/userState.ts:169 — dataSource.update(resource, cachedRowId, { value: items })). So the server appears to fold the path id into the candidate write set, judge it read-only, and then report it as a dropped field.
(The request body was not captured on the wire; the above is read off the client source in the current main checkout. If the running build does send id, this is a client bug instead — either way the toast is wrong.)
Expected
droppedFields reports fields the caller supplied and the engine refused. The primary key addressing the row is not a supplied field, and an internal preference write must never surface a warning to the user.
Both invariants are already written down:
packages/data-objectstack/src/userState.ts:41— "save()resolves without throwing, so UI mutations never surface a toast."- the same file, on why
updated_atis no longer sent (rest: PATCH /data 对被静默剥离的写入字段无任何回传 — onFieldsDropped 通道未接线(follow-up #3407/#3413) #3431/审批场景下记录可写性的反馈全线失真:能改的显示「已锁定」,改不了的提示「更新成功」 #3794) — "Sending it made every recents/favorites write pop a scary warning about a field the user never touched, drowning the real signal the toast exists for."
This is that same drowning, one field over, and it is the loudest thing a user sees after switching workspaces.
Suggested fix
Server side: exclude the path/primary key from the read-only drop accounting on single-record update (it is addressing, not payload). Optionally also let a write opt out of drop reporting for internal preference traces.
Environment
Local dev server http://localhost:8080, multi-org enabled, zh locale, observed 2026-08-12 (reproduced on every org switch). The server was started by the maintainer and its exact commit is not verified.
Activity
baozhoutao commentedon Aug 12, 2026
Found in the same multi-org walkthrough (sign-up → create workspace → invite → second user accepts → second workspace → switch → isolation/authz probes): #8090, #8091, #8092, #8093, #8094.
baozhoutao commentedon Aug 12, 2026
Two further findings from the same pass, filed for the platform to rule on: #8095, #8096.
hotlong commentedon Aug 12, 2026
Triage:
pm:queue,domain:engine-coreconfirmed (the suggested fix — exclude the path/primary key from read-only drop accounting on single-record update — lands in the engine's write path; prior art #6437/#3431/#3455 all lived there or adjacent). Type: Bug (violates the declared meaning ofdroppedFields— fields the caller supplied and the engine refused — and theuserState.tsno-toast invariant). Dedup: searched; no open twin, only the closed #3431/#3794/#6437 lineage the card already cites.Adopting the engine-core seat's flag from the seat-post note as a dispatch requirement: the card's own fork (request body not captured on the wire) is measured FIRST — if the running build's client does send
id, this becomes a client-side card instead and must come back for re-routing rather than be fixed server-side on an unverified premise. Either way the toast is wrong, but which repo eats the fix depends on that capture.Vocabulary note, not a re-label:
priority:p2is left in place because a human filer applied it, but no seat's selection currently reads any priority tier exceptpriority:p0— the ladder question is raised in this round's report to the maintainer.Size/model suggestion: M /
opus,mode:subagent.本评论来自分诊座位 Routine。
Generated by Claude Code
os-zhuang commentedon Aug 12, 2026
Claim: PM loop round 2⚠️
Session:
session_014C8pAprWdmtecFsEprZax4Branch:
claude/issue-8093-dropped-fields-path-idWorktree:
objectstack-issue-8093Domain:
domain:engine-coreFile surface: not yet determined — that is step 1 of the card. The card's own suggested fix names the read-only drop accounting on single-record update; the producer of
droppedFieldshas to be located by symbol before anything is declared. Expected candidates only:packages/objectql/src, possiblypackages/metadata-protocol/src/protocol.ts(stop on breach; explain in the report)Container & model:
M,mode:subagent,model: opusSerial constraints cleared: in-flight 0 in this lane at claim time.
packages/objectql/src/registry.tsandmetadata-facade.tsboth had cards land or start today — re-derive any line number; three different addresses were measured for one seam inregistry.tsin a single day. Sibling card #7378 is dispatched in the same round and holdsmetadata-facade.ts— ⛔ if this card's producer turns out to live there, stop and report the collision rather than editing around it.id, this is a client bug and the server half is not the fix. That measurement is step 1 — ⛔ do not assume the server half.Note on the label set: this card arrived carrying
priority:p2, which is outside this repo's pm vocabulary (onlypriority:p0exists; the P1–P5 ladder is on record as tried-and-rejected). ⛔ Not this seat's to remove — raised with triage on #6015 and preserved verbatim in this write rather than silently dropped.Seat #6019,
domain:engine-core.Generated by Claude Code
os-zhuang commentedon Aug 12, 2026
{ "issue": 8093, "status": "done", "branch": "claude/issue-8093-dropped-fields-path-id", "pr": "https://github.com/objectstack-ai/objectstack/pull/8143", "premise_still_valid": true, "summary": "STEP 1 SETTLED BY EXECUTION, NOT BY GREP: the server folds the path id into the candidate write set, so this is ours and nothing moves to repo:objectui. Drove the real ingress (real ObjectQL + real ObjectStackProtocolImplementation + the real registered PATCH route) with a body this side constructs — `client body keys: [\"value\"]`, `hasOwnProperty('id'): false` — and the response still reproduced the card's payload byte for byte: droppedFields:[{object:'sys_user_preference',fields:['id'],reason:'readonly'}]. Chain: metadata-protocol's updateData folds `{...request.data, id: request.id}` (#6479, the cross-row-write fix, correct and UNCHANGED here) BEFORE the engine snapshots suppliedValues, after which the address is indistinguishable from caller payload, and the static-readonly strip drops and reports it because sys_user_preference.id is readonly:true. Fix is in packages/objectql/src/engine.ts — engine-core's OWN package, located by symbol as the card required, NOT metadata-facade.ts (no collision with sibling #7378): a by-id `id` equal to the bound key is excluded from the drop accounting. It narrows the REPORT, not the strip — the driver's SET clause is byte-identical before and after (measured), and it must be, since a same-value primary-key write is rejected outright by stores with immutable primary keys and #6435 already ruled widening the strip a separate decision. Self-scopes to single-record update by keying on equality with the bound key, so predicate/multi writes and both #6437 primary_key strips are untouched. The invariant is now stated at the contract (WriteObservabilityOptions, packages/spec/src/contracts/data-engine.ts), including what is deliberately still reported. ⚠️ CROSS-SEAT, FYI ONLY: the TRIGGER lives in packages/metadata-protocol/src/protocol.ts (seat #6367, domain:metadata) — that file is NOT modified by this PR, deliberately; fixing the engine's accounting instead also fixes batch/GraphQL/flow ingresses in one place. Only other package touched is packages/spec, doc comment only.", "tests": "New: packages/rest/src/rest-update-path-id-not-a-dropped-field.test.ts (3 cases, whole ingress) + packages/objectql/src/engine-update-addressing-id-not-dropped.test.ts (10 cases, engine seam). BOTH DIRECTIONS PINNED: body omitting the PK => no droppedFields key and no X-ObjectStack-Dropped-Fields header; a read-only field the caller DID supply => still reported unchanged; both at once => only the real refusal, `id` does not ride along; plus driver SET clause unchanged, both primary_key strips unchanged, predicate/multi still reports a supplied read-only field, non-readonly `id` unchanged in both channels, strictReadonlyWrites consistent. PRE-FIX MEASUREMENT (the step-1 red): 'Tests 3 failed | 1671 passed' with case 2's diff showing fields: [\"created_at\", + \"id\"]. POST-FIX: objectql full 192 files/3397 passed; rest full 102 files/1674 passed; spec full 385 files/10195 passed; service-automation (named consumer of this seam) 79 files/940 passed; typecheck objectql+rest+spec clean. REVERSE VERIFICATION — direction predicted first, and the prediction was WRONG in an informative way: predicted the 4 'NONE/ONLY' rows red and both strict rows green; MEASURED 6 failed / 4 passed — both strict rows ALSO went red, because reverting the report also restores the refusal and puts 'id' back in the refusal's `drops`. That is the #6437 derived-coverage contract (strict covers every drop onFieldsDropped reports), so the quiet and loud halves cannot move apart; predicting them independent was the error. The 4 that stayed green are exactly the neighbours the change claims not to touch. NAMED DELIVERABLES: `pnpm check:query-options-erasure` green (ratchet holds, 67 unswept sites, none new); `pnpm check:type-check-debt --re-measure` green on a fully built closure (33 entries re-measured, none above recorded) — it was RED first at +1 objectql / +2 rest from my two new test files, fixed AT SOURCE (missing packageId arg to registry.registerObject; missing .js extension under nodenext); NO baseline raised and `--lower` NOT run. Gates derived at implementation time via `node scripts/pm/dispatch-gates.mjs` on the real file surface, all green: adr-anchors, authz-resolver, changeset-gate-self-tests, cross-package-test-inputs, doc-formula-expressions, docs-audit-scope, durability-log-level, engine-double-contract, i18n (needed the CLI built first — it self-reports 'nothing was checked' otherwise), merge-driver, meta-type-normalized, objectui-changeset, release-body, spec-parsed-alias, stack-collection-maps, nul-bytes, check-changeset-no-major.mjs, check-engine-split-ratio.mjs. ⚠️ ONE DERIVED GATE IS RED AND IS NOT MINE: check:objectui-pin-fresh — .objectui-sha is stale vs objectui main. Not in my diff (last touched by #7916); it matched only because this PR adds a changeset. Remedy is a maintainer pin bump (scripts/bump-objectui.sh); the recurring task was last filed as the now-closed #6089, so I reported it here rather than filing a duplicate — your call.", "open_questions": [], "out_of_scope_findings": [ "filed as #8141: the read-only strip's server-side WARN still calls the same addressing `id` a forged caller write, on every single-record update of every platform object — measured AFTER this fix (droppedFields: null, warn count: 1), so the toast is gone but the same untruth remains in the log channel, and its remedy prose wrongly advises passing { context: { isSystem: true } }. Not fixed here because it means changing stripReadonlyFields, a helper shared by the by-id branch, the multi branch and the insert-side sibling, and the tidier alternative is #6435's explicitly-separate decision. Filed unassigned, unlabeled, with three options and a recommendation for triage." ] }Draft PR: #8143. Reporting at draft-PR time per the 2026-08-10 maintainer decision — CI convergence is the PM's wait, not this lane's; no gate job conclusions are claimed above beyond the local runs listed.
Seat #6019 dev lane, session
session_014C8pAprWdmtecFsEprZax4.Generated by Claude Code
os-zhuang commentedon Aug 12, 2026
CI note on PR #8143 —
Check Documentation Linksfailed, and it is not this diff. Re-run queued.Engine-core seat #6019,
session_014C8pAprWdmtecFsEprZax4. Recording the diagnosis rather than a verdict; the dev's final report has not landed yet and ⛔ this is not a review.The signature, read from the job's own log
Exit 22 is curl's code for an HTTP error response — the lychee binary download failed. The consequence is the part that settles it:
⭐ The link checker never ran. Not one link in this PR was evaluated, so the failure carries no information about the diff. And the causal direction is closed: a TypeScript doc comment plus two test files cannot make a third-party release download return 4xx.
Applying the two-part discriminator honestly
This seat's rule is that how a job died is not whether your diff caused it — the second half is whether it reproduces here and not on siblings. Four PRs from this lane earlier today (#8079, #8085, #8105, #8115) all show
Check Documentation Linkssuccess, so by the reproduces-here-only test alone this would look like it could be ours.Action taken:
rerun_failed_jobson run31624984731. ⛔ No code change, no push, no branch touched — the dev is still live on this card and this is a job re-run, not an intervention on its work.Check Documentation Linksfails atlychee-setupwith curl exit 22;Install lycheeandRun Lycheebothskipped. Transient third-party download; plain re-run. The discriminator is the skipped outcome on the two later steps — a red insideRun Lycheeis a different verdict entirely and means real broken links.⛔ Filed here as a proposal on the anchor card rather than added to a flake table: only a human escalates a signature into the queue steward's ledger, and one sighting is not a pattern. If it recurs, it belongs there; if it does not, this comment is the whole record and costs nothing.
Generated by Claude Code
os-zhuang commentedon Aug 12, 2026
ACCEPTED on substance — PR #8143. ⛔ Ready-flip held on CI plus two cross-seat windows.
Engine-core seat #6019,
session_014C8pAprWdmtecFsEprZax4. Read from the diff and the card's own record, ⛔ not from the report.Step 1 did what step 1 was for
The dispatch said the measurement could end the card — that if the running build sent
id, this belonged inrepo:objectuiandpremise_still_valid: falsewas the right terminal state. It was settled by executing the real ingress, not by reading source: body keys["value"],hasOwnProperty(body,'id') === false, and the response still reproduced the card's payload byte for byte.⭐ And the third block of that measurement is what made the fix's shape obvious: the driver's SET clause never contained
id. The strip was always doing its job — only the report was untrue. That is why this is a report-only change, and it is a conclusion the measurement produced rather than one the dispatch assumed.The reverse verification was wrong, said so, and is better for it
Predicted: four rows red, both
strictReadonlyWritesrows green. Measured: 6 failed / 4 passed — both strict rows went red too.The explanation is the valuable part:
strictReadonlyWritesis contracted as covering "every droponFieldsDroppedreports" — coverage derived from the reported set (#6437) — so the quiet and loud halves cannot move apart; un-reporting the address un-refuses it in the same edit. ⭐ "Predicting them independent was the error, not the code." A prediction that fails and is diagnosed teaches more than one that passes, and the four rows that did stay green are exactly the neighbours the change claims not to touch.One deviation from my dispatch, and it is correct
I wrote that rejection tests assert
code+statusat minimum. The strict-refusal test assertscode+dropsinstead, with the reason in the file:strictReadonlyWriteslives onWriteObservabilityOptions, which is not the serializable options bag, so no wire caller can reach this refusal and it carries no HTTP status to assert — anddropsis the documented breakdown a caller reads after catching the one stable code.⭐ That is a measured, cited deviation, not a shortcut. ⛔ Asserting a
statusthere would have meant inventing one. The rule stands; this seam is outside it, and the file says why.Verified independently
get_files)objectql/src/engine.ts· two new test files · a doc comment inspec/src/contracts/data-engine.ts— ⛔ nodocs/adr/**, no skills root. CleanidAddressesThisRowkeyed onObject.is(suppliedValues.id, id)and read from the snapshot, so it survives a hook rewriting the key mid-write (#5591). Self-scopes to by-id by constructionpackageIdarg, missing.jsundernodenext) — ⛔ no baseline raised,--lowernot runWhy it is not flipped
Check Documentation Linksfailed — diagnosed from the job log as the lychee binary download returning 4xx (curl exit 22) withInstall lycheeandRun Lycheebothskipped: the checker never ran, so it evaluated nothing in this diff.rerun_failed_jobsqueued. ⛔ The rest of the run must still be read per job before any flip.packages/spec/src/contracts/data-engine.ts⇒ [PM seat] domain:spec — ⏳ vacant #6017, andpackages/rest⇒ [PM seat] domain:cli — 🟢 os-sales · 由Sales账户的项目负责 #6024. ⛔ Neither is the dev's drift; the dispatch declined to pre-declare a producer it had not located, which was right, and the consequence is that the declarations are filed after the fact rather than before.check:objectui-pin-fresh(stale.objectui-sha) matched here only because the PR adds a changeset — the #7865 dev hit the identical thing. Its remedy is a maintainer pin bump; the recurring card (#6089) is closed, so ⛔ nothing open tracks it. Raised to the queue steward rather than filed here, since pin-chain observation is that seat's standing duty.Generated by Claude Code
baozhoutao commentedon Aug 13, 2026
Same object, adjacent defect:
sys_user_preference's(user_id, key)unique index is global rather than per-organization, so a user in a second org cannot persist a key they already use in the first — filed separately.fix(objectql): the addressed row's primary key is not a dropped field (…
fix(objectql): the read-only strip stops calling the addressed row's …
fix(objectql): droppedFields on a create names only keys the caller s…