Skip to content

fix(metadata-protocol): a plain clone does not report the source's empty readonly columns as dropped fields - #29290

Merged
objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/project-thread-v06ygi
Nov 2, 2026
Merged

objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/project-thread-v06ygi

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by Steve · project thread

Fixes #29200

Clause-②: no
Metadata-protocol: none

维护者速读

  • 改了什么:克隆记录时,源记录里为空的只读字段不再被复制进新记录的插入数据,所以一次什么都没改的克隆不会再在响应里报告"字段被丢弃",X-ObjectStack-Dropped-Fields 响应头也不再出现。
  • 为什么改:控制台每次克隆一条只读字段为空的记录(如 showcase 联系人的 lead_score),都会提示"某些字段被丢弃";集成方读 droppedFields 或响应头,会把每次克隆都当成丢了数据。用户其实什么都没发。
  • 风险与回滚:存下来的新记录与修复前逐字节相同(空值本来就会被剥掉、插入时空值与缺省同样取默认值)。只读字段有值时照旧复制、剥离并报告(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 裁决),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 held null. 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 (no overrides) of a row whose read-only column was empty therefore answered droppedFields naming that column, and since #29034 the X-ObjectStack-Dropped-Fields header 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 readonly whose copied value is null/undefined is now left out of the insert payload. Nothing changes in packages/rest/src/error-response.ts; the header still mirrors the body.

clone body droppedFields before → after header before → after
no overrides, source's read-only column empty names the column → absent names the column → absent
no overrides, source's read-only column holds a value names the column → names the column ro;reason=readonly → same
overrides sets the read-only field names the column → names the column ro;reason=readonly → same

Why the stored row is unchanged. For a non-system caller the strip removed the empty key anyway. For every caller, the insert's applyFieldDefaults treats an explicit null exactly as an omitted key (out[f.name] != null is its only skip), so the copy takes the same default either way. Injected platform columns (organization_id, owner, audit) are system: true and were already removed from the copy by the existing branch.

Acceptance 3 (a read-only column that holds a value) — measured, kept. On main at 45e9b2f1, a plain clone of a row whose ro holds 'seeded' answers 201 with droppedFields: [{ object, fields: ['ro'], reason: 'readonly' }] and header ro;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 null key) was not taken: it changes the #3431 "fields stripped from THIS write" report on every insert path, and a caller that really sends ro: null on 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

Forced test edits (assertions that accommodated the defect):

  • dogfood clone with overrides.lead_score: the header assertion moves from toContain('lead_score;reason=readonly') to toBe('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.
  • rest test file: the comment above CLONE_OBJECT ("a clone of a PROJ row also reports the source's ro2") 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

Verification

Head: 85ebba60 (merge of origin/main at f6a04e3a over fix f825d6bd; red-first base 45e9b2f1).

Red first, fix, ablation
  • Red on main (45e9b2f1, tests only, @objectstack/rest^... built): rest-dropped-fields-every-write-door.test.ts Tests 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 on main.
  • Fix (f825d6bd, metadata-protocol rebuilt): Tests 18 passed (18).
  • Ablation through scripts/ablation-replace.mjs (anchor hit 1 -> 0, blob changed), the new line made dead by a name === '__ABLATION_29200__' guard; ablation-dist-preflight.mjs found the marker in dist/index.js and dist/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 named lead_score). Restore leg each time: blob == HEAD, git diff HEAD empty, rebuilt, preflight --absent exit 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 (at f825d6bd).
  • pnpm --filter @objectstack/rest test && typecheck: Test Files 367 passed (367); Tests 6966 passed, 332 skipped; VERDICT command-exit 0 (at f825d6bd).
  • After merging 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, dogfood showcase-static-readonly Tests 9/9.
  • node scripts/pm/dispatch-gates.mjs --commands: 65 derived; all 65 recorded exit 0 at 85ebba60 (check:dual-build-cjs-loads first exit 3, PREREQUISITE NOT MET, re-run exit 0 after building ./packages/**). --ran: 65 run, 0 NOT-MEASURED, 0 UNRUN.
  • eslint (--no-inline-config) on the three touched source and test files: exit 0.
  • Full dogfood battery: not owed (no !, no narrowing arm).

Human Minimum

  • 1. Schemas start as Zod; packages/spec holds no runtime logic.
  • 2. Every name I added uses the casing for its kind.
  • 3. No formatter ran; pnpm lint passes.
  • 4. One issue, one branch off an up-to-date main, one pull request.
  • 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset.
  • 6. packages/spec touched: its generated artifacts are regenerated and committed. (not touched)
  • 7. No hand edits to content/docs/references/, packages/*/CHANGELOG.md or content/docs/releases/.
  • 8. Title, body and comments are in English.
  • The Before Submitting list in CONTRIBUTING.md is done.

Generated by Claude Code

…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>
@github-actions github-actions Bot added size/s documentation Improvements or additions to documentation tests tooling labels Nov 2, 2026
@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/metadata-protocol, touching 3 documentable anchor(s).

2 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/data-api.mdx (via /:object/:id/clone (route, bridged from symbol cloneData — its route source's handler names it))
  • content/docs/kernel/contracts/data-engine.mdx (via cloneData (symbol, a method of class ObjectStackProtocolImplementation))

⛔ 3 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/implementation-status.mdx (via /:object/:id/clone (route, bridged from symbol cloneData — its route source's handler names it))
  • content/docs/releases/v17/17-4.mdx (via cloneData (symbol, a method of class ObjectStackProtocolImplementation))
  • content/docs/releases/v17/17-6.mdx (via /:object/:id/clone (route, bridged from symbol cloneData — its route source's handler names it))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • the SDK route bridge reached 60 of 216 client-bound route-ledger rows — the other 156 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 156: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 101 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 10 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 39951d2a26590ff93226d3c0cf4ecfdc4136031e → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 23659f4722b6bf7feb054594e278a91c4348343f — the merge of head 85ebba609a7f0120e5f5855c0ea93cdc6f5ba7e7 into base 39951d2a26590ff93226d3c0cf4ecfdc4136031e, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# 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

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 39951d2a26590ff93226d3c0cf4ecfdc4136031e → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#2 · os-steve · session_01ADHkmQxwoGb5fSnrFkpxmt

席位核对

ACCEPT · domain:server seat 2 · os-steve · session_01ADHkmQxwoGb5fSnrFkpxmt · 2026-11-02T11:12Z · head 85ebba60 · claimed by seat 2 on #29200 (comment 6574488796)

The maintainer's words for this seat, verbatim: 「接手所有开发,包括跟进之前未完成的开发落地。」

  1. Shape: a draft against main, Fixes #29200, Clause-②: no and Metadata-protocol: none at line start. check-governed-merges --pr 29290: NOT governed. Four files, +78 / -6: cloneData only in protocol.ts, the rest pin, the one dogfood file, and the changeset. No change to error-response.ts, no ui/**, no packages/spec, as the order required.
  2. Route, read against the order: the fix is on the producer side, route (a), as fix(rest): every 2xx write answer whose body reports droppedFields also sends the X-ObjectStack-Dropped-Fields header #29034's contract review ruled. A readonly field whose copied value is null/undefined is left out of the clone's insert payload. The stored row is unchanged: the strip removed the empty key anyway, and applyFieldDefaults treats null as omitted. Route (b), the engine strip, was not taken, for the reason the order gave.
  3. Acceptance 3, a readonly column holding a value: measured. It is reported on main and still reported after. The dev kept it on purpose, citing the maintainer's ruling on 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 (option 1, record 5578819560): a clone reports a copied-and-stripped value so the caller learns why it reads the default back. The order asked the dev to decide whether this is the same defect and to say so. The dev judged it is not, and the seat accepts that reading on the ruling's wording. It is a deviation from the order's "fix it under (a) if so" only in that the "if so" did not hold. Declared in the body.
  4. Evidence: red first on main (1 failed of 18), then 18 of 18. The overrides control and the valued-column control are green on both trees. Ablation reds the rest pin and the dogfood file, with dist preflight, and restores clean. metadata-protocol 22100 tests and rest 6966 tests pass, both typechecks are green, and the 65 derived gates all exit 0. Forced test edits are named: the dogfood clone-with-overrides header toContain becomes toBe, and the rest test comment plus one seed row.
  5. Gates on this head at ACCEPT time: 13 success, 13 skipped, 0 red, 20 still running. Auto-merge waits for them, and the merge queue re-runs CI against current main.
  6. Serial: open PR fix(metadata-protocol): the by-id OCC comparison answers only for a row the caller reads #28731 edits protocol.ts at probeRecord, disjoint from cloneData; whichever lands second merges main. Landing: pr_ready + automerge_enable through the relay.

Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review November 2, 2026 11:13
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge November 2, 2026 11:13
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Nov 2, 2026
Merged via the queue into main with commit 2626bdd Nov 2, 2026
50 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/project-thread-v06ygi branch November 2, 2026 12:17
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#2 · os-steve · session_01ADHkmQxwoGb5fSnrFkpxmt

Landing record · PR #29290

domain:server seat 2 · session_01ADHkmQxwoGb5fSnrFkpxmt · 2026-11-02T13:07Z · card #29200

Reading Probe Result
Merged PR #29290 state merged, merge commit 2626bdd3a0
On main git merge-base --is-ancestor 2626bdd3a0 origin/main yes
Queue refs git ls-remote origin refs/heads/gh-readonly-queue/* matching 29290 0
Content git show 2626bdd3a0 --stat names packages/metadata-protocol/src/protocol.ts present

Release: ships patch for metadata-protocol at the next release.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/s tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

REST clone: a plain clone reports the source row's empty readonly columns as dropped fields (body and header)

1 participant