Repository navigation
fix(metadata-protocol): a plain clone does not report the source's empty readonly columns as dropped fields - #29290
Conversation
…n reports no drop Red first on main: cloneData copies the source's null readonly columns into the insert and the engine's strip reports them. Claude-Session: https://claude.ai/code/session_01ADHkmQZyRQCzui7zekMYY7 Co-authored-by: Claude <noreply@anthropic.com>
…pty readonly columns as dropped cloneData leaves a readonly column whose source value is null out of the copy, so engine.insert's strip no longer reports a key nobody supplied. A readonly column holding a value is still copied, stripped and reported (#15703), and so is one set through overrides. Claude-Session: https://claude.ai/code/session_01ADHkmQZyRQCzui7zekMYY7 Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 1 package(s): 2 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 3 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 10 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 23659f4722b6bf7feb054594e278a91c4348343f && git checkout 23659f4722b6bf7feb054594e278a91c4348343f
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 39951d2a26590ff93226d3c0cf4ecfdc4136031e 85ebba609a7f0120e5f5855c0ea93cdc6f5ba7e7 && git checkout -B drift-repro 39951d2a26590ff93226d3c0cf4ecfdc4136031e && git merge --no-ff 85ebba609a7f0120e5f5855c0ea93cdc6f5ba7e7
node scripts/docs-audit/affected-docs.mjs --json 39951d2a26590ff93226d3c0cf4ecfdc4136031e
|
|
domain:server#2 · os-steve · session_01ADHkmQxwoGb5fSnrFkpxmt 席位核对ACCEPT · The maintainer's words for this seat, verbatim: 「接手所有开发,包括跟进之前未完成的开发落地。」
Generated by Claude Code |
|
domain:server#2 · os-steve · session_01ADHkmQxwoGb5fSnrFkpxmt Landing record · PR #29290
Release: ships Generated by Claude Code |
Requested by Steve · project thread
Fixes #29200
Clause-②: no
Metadata-protocol: none
维护者速读
X-ObjectStack-Dropped-Fields响应头也不再出现。lead_score),都会提示"某些字段被丢弃";集成方读droppedFields或响应头,会把每次克隆都当成丢了数据。用户其实什么都没发。overrides里带只读字段也照旧报告。回滚即 revert 本 PR。packages/metadata-protocol/src/protocol.ts的cloneData(新增一行与注释);packages/rest/src/rest-dropped-fields-every-write-door.test.ts的 clone 用例(新增 3 个)。pnpm --filter @objectstack/rest exec vitest run src/rest-dropped-fields-every-write-door.test.ts,预期 18 个用例全过(需先构建@objectstack/metadata-protocol)。What changes
cloneData(packages/metadata-protocol/src/protocol.ts) copied every column of the source row into the insert, read-only ones included, even where the source heldnull.engine.insert's static-readonly strip judges every present key as caller-supplied, so it took that empty key and reported it. A plain clone (nooverrides) of a row whose read-only column was empty therefore answereddroppedFieldsnaming that column, and since #29034 theX-ObjectStack-Dropped-Fieldsheader repeated it.The fix is on the producer side, as the contract review on #29034 ruled: in the loop that already removes engine-owned columns from the copy, a field declared
readonlywhose copied value isnull/undefinedis now left out of the insert payload. Nothing changes inpackages/rest/src/error-response.ts; the header still mirrors the body.droppedFieldsbefore → afterro;reason=readonly→ sameoverridessets the read-only fieldro;reason=readonly→ sameWhy the stored row is unchanged. For a non-system caller the strip removed the empty key anyway. For every caller, the insert's
applyFieldDefaultstreats an explicitnullexactly as an omitted key (out[f.name] != nullis its only skip), so the copy takes the same default either way. Injected platform columns (organization_id, owner, audit) aresystem: trueand were already removed from the copy by the existing branch.Acceptance 3 (a read-only column that holds a value) — measured, kept. On
mainat45e9b2f1, a plain clone of a row whoseroholds'seeded'answers 201 withdroppedFields: [{ object, fields: ['ro'], reason: 'readonly' }]and headerro;reason=readonly; after the fix it answers the same (the new control case passes on both trees). This is not the same defect: the value was really copied from the source and really stripped, so the caller reads the default back, and the maintainer ruling on #15703 (option 1, record 5578819560) is exactly that the clone reports this strip so the caller is told why. The card's route (a) is therefore applied to EMPTY read-only values only; dropping valued read-only columns from the copy would silence a report that ruling requires.Route (b) (the engine strip not reporting a present
nullkey) was not taken: it changes the #3431 "fields stripped from THIS write" report on every insert path, and a caller that really sendsro: nullon a create did send that key.Why
The console announces every non-empty
droppedFields, so users saw a "fields dropped" notice on a clone where they changed nothing, and integrations reading the body or the header would treat every such clone as data loss.Tests
packages/rest/src/rest-dropped-fields-every-write-door.test.ts(realObjectQL+ protocol +RestServerover better-sqlite3), three new clone cases on a new seed rowsrc_empty(read-onlyroleft empty):src_empty: 201, nodroppedFields, no header,record.ronull (red onmain);overrides: { ro: 'forged' }onsrc_empty: still reported, body and header (control, acceptance 2);src1(ro: 'seeded'): still reported, body and header (control, acceptance 3, finding: cloneData is the one create face whose response contract carries no droppedFields — a clone that copies or overrides a readonly column is stripped and warned, but not reported on the wire #15703).packages/qa/dogfood/test/showcase-static-readonly.dogfood.test.ts: new case, a plain clone of a stock showcase contact whoselead_scoreis empty answers nodroppedFieldsand no header.Forced test edits (assertions that accommodated the defect):
clone with overrides.lead_score: the header assertion moves fromtoContain('lead_score;reason=readonly')totoBe('lead_score;reason=readonly'), and its comment ("a plain clone copies the source's other readonly columns too, so the header may name more than lead_score") is rewritten, since the copy no longer carries empty read-only columns.CLONE_OBJECT("a clone of a PROJ row also reports the source'sro2") is rewritten to say only valued read-only columns are copied; no assertion moved. The seed gains one row (src_empty); no case counts that object's rows.Acceptance notes
protocol.tsatprobeRecord; this diff touches onlycloneData.lead_score, cannot be given a value by a non-system caller); the real-stack rest test carries it.Verification
Head:
85ebba60(merge oforigin/mainatf6a04e3aover fixf825d6bd; red-first base45e9b2f1).Red first, fix, ablation
main(45e9b2f1, tests only,@objectstack/rest^...built):rest-dropped-fields-every-write-door.test.tsTests 1 failed | 17 passed (18); the failure is the new plain-clone case,expected { object: 'hdr_drops_clone', ... } to not have property "droppedFields". The valued-column and overrides controls passed onmain.f825d6bd,metadata-protocolrebuilt): Tests 18 passed (18).scripts/ablation-replace.mjs(anchor hit 1 -> 0, blob changed), the new line made dead by aname === '__ABLATION_29200__'guard;ablation-dist-preflight.mjsfound the marker indist/index.jsanddist/index.cjs. Rest pin: Tests 1 failed | 17 passed (the plain-clone case). Dogfood file: Tests 1 failed | 8 passed (the new plain-clone case, body namedlead_score). Restore leg each time: blob == HEAD,git diff HEADempty, rebuilt, preflight--absentexit 0, Tests 18/18 and 9/9.Package suites and gates
pnpm --filter @objectstack/metadata-protocol test && typecheck: Test Files 300 passed, 4 skipped (304); Tests 22100 passed, 26 skipped; VERDICT command-exit 0 (atf825d6bd).pnpm --filter @objectstack/rest test && typecheck: Test Files 367 passed (367); Tests 6966 passed, 332 skipped; VERDICT command-exit 0 (atf825d6bd).origin/main(85ebba60):pnpm install --frozen-lockfile, build of the@objectstack/rest^...and@objectstack/dogfood^...closures,pnpm --filter @objectstack/spec check:generated: VERDICT command-exit 0. Rest pin Tests 18/18, dogfoodshowcase-static-readonlyTests 9/9.node scripts/pm/dispatch-gates.mjs --commands: 65 derived; all 65 recordedexit 0at85ebba60(check:dual-build-cjs-loadsfirst exit 3, PREREQUISITE NOT MET, re-run exit 0 after building./packages/**).--ran: 65 run, 0 NOT-MEASURED, 0 UNRUN.--no-inline-config) on the three touched source and test files: exit 0.!, no narrowing arm).Human Minimum
packages/specholds no runtime logic.pnpm lintpasses.main, one pull request.packages/spectouched: its generated artifacts are regenerated and committed. (not touched)content/docs/references/,packages/*/CHANGELOG.mdorcontent/docs/releases/.Generated by Claude Code