fix(metadata-admin): strip read decorations before the client edit gate - #8182
Conversation
`client.getDraft()` serves a decorated body — `decorateMetadataItem` attaches `_diagnostics` whenever the type has a registered Zod schema, and `_draft` rides the preview-draft branch. `ResourceEditPage` merged that body over the layered baseline and handed the result to `validateMetadataDraft`, so every wired type with a `.strict()` schema reported a body the SERVER ACCEPTS as `unrecognized_keys` — a false "this item is invalid" banner plus inline field errors on an author who is mid-edit. The layered half is clean (`getMetaItemLayered` serves RAW layers), so the misfire needed a pending draft to exist; an item without one passes today, which is why this stayed invisible. Strip in `extractDraftBody` — the one function that turns a served draft envelope into a body, and the chokepoint all three merge sites read (the load effect, the post-save refresh, the post-publish refresh). It reuses the spec's exported `stripReadDecorations`, the same helper `MetadataService.saveFields` uses on the write side; no key list is restated here, and no schema is loosened. The strip runs after the presence verdict so it can never turn a served draft into "no draft". Refs objectui#7603 (the closing declaration lives in the PR body, once). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
Contract review — PASS. And the dev corrected a false premise in my own dispatch order.PM seat, session My Zone 1 was wrong, and the correction is rightI told the dev that objectui "already has a correct read-time That is the write path, inside Where the error came from, since it matters for the next dispatch: triage's note said "decoration strip". I wrote "read-time decoration strip" and turned a true statement into a false one by adding a qualifier I had not measured. A dev following my order literally would have gone looking for a read-time strip to lift and found nothing. The adjudication survives unharmed — the reusable asset is the spec's exported Clause-② — YES on accept/reject, NO on the public face, verified independentlyThe client edit gate now accepts bodies it previously rejected, so the accept/reject leg fires and The public-face leg does not fire, measured rather than assumed: What earns the PASSIt fixed the chokepoint, not the symptom the card named. The card names one merge site; The pre-change RED is a real reproduction, with the mutation proven on disk by It caught a tsc-only defect in its own pin — Also worth recording, because it reads like a red and is not: One addition to the out-of-scope card#8181 names three consumers; reviewing this PR I measured a fourth. Held only on CI convergence. Generated by Claude Code |
2 participants
Fixes #7603
What was wrong
client.getDraft()serves a decorated body: the strict draft branch returnsitem: decorateMetadataItem(type, ...), which attaches_diagnosticswhenever the type has aregistered Zod schema, and
_draftrides the preview-draft branch.ResourceEditPagemergedthat body over the layered baseline (
{ ...baseline, ...draftReal }) and handed the resultstraight to
validateMetadataDraft.The layered half is clean —
getMetaItemLayeredserves RAW layers — so the misfire needed onecondition: the item has a pending draft. That is why it stayed invisible; an item without a
draft passes today and cannot catch it.
Result: every wired metadata type whose schema is
.strict()reported a body the serveraccepts as
unrecognized_keys— a false "this item is invalid" banner plus inline fielderrors, on exactly the workflow where an author is mid-edit.
The fix
extractDraftBodynow passes the body through the spec's exportedstripReadDecorations.That function is the chokepoint: it is the one place a served draft envelope becomes a
body, and all three merge sites read it — the load effect (
:950), the post-save refresh(
:1440) and the post-publish refresh (:1592). The card names only the first; fixing thechokepoint closes all three and leaves no fourth site to forget.
The strip runs after the presence verdict, never before it: what counts as a pending draft
is
getDraft's answer, and removing our own decorations must not be able to turn a serveddraft into "no draft".
⛔ No schema was loosened, and no key list is restated in this repo. The list is the spec's,
reached through its own helper. The ADR-0010 protection envelope (
_lock,_provenance, …) isdeliberately not on that list — those keys are allowlisted by the closed schemas so
provenance survives a re-parse — and this strip leaves them alone.
Corrections to the dispatch's premises (both measured)
origin/main9bfd61848:MetadataService.ts617 / 658 / 665 ✅;ResourceEditPage.tsx939 / 949 / 950 /951 / 429 ✅. Nothing had moved.
both describe
MetadataService.ts:658-665as a read-time strip. Measured: it sits insidesaveFieldsand strips on the way out to the server, and its own comment says so("the objectui#4644 strip-on-load shape applied on the write side"). Before this PR there
was exactly one
stripReadDecorationscall in the whole repo —MetadataService.ts:679.The adjudication is not void: the reusable asset is the spec's exported helper, and that
is what this PR reuses. Only the direction was described wrongly.
Evidence
The bug, proven on the pre-change tree.
ResourceEditPage.tsxwas reverted to itsorigin/mainbytes (blob75680b79, verified bygit hash-object, and thestripReadDecorationsanchor count went 2 → 0 on disk before the run). Both pins go RED, forexactly the stated reason:
Restore proven by state, not by an exit code.
git hash-objectreturnedd5c59d3a,equal to
git rev-parse HEAD:PATHfor the same file, withgit diff HEADempty and the anchorcount back to 2.
The fixed tree.
Test Files 1 passed (1) · Tests 2 passed (2). Re-run after the finalcommit alongside the two suites nearest this behaviour (
ResourceEditPage.schemaAdvisory,clientValidation.optOuts):Test Files 3 passed (3) · Tests 25 passed (25).Schema-level measurement (bundled spec,
PageSchema):+ _diagnosticsunrecognized_keys: ["_diagnostics"]+ _draftunrecognized_keys: ["_draft"]+ bothunrecognized_keys: ["_diagnostics","_draft"]+ zzBogusunrecognized_keys: ["zzBogus"]stripReadDecorations(+ both)stripReadDecorations(+ zzBogus)["zzBogus"]— the strip does not over-reachTests
Two directions, as required — and the second is also the first's control, so "no issues" is
a measured verdict rather than a validator that never fired:
_diagnosticsand_draftnow passes the gate;with neither decoration named alongside it, and the banner fires.
The pins assert the gate's own verdict (via a recording wrapper around the real
validateMetadataDraft), not merely a rendered banner.All readings below were taken on the final commit
d16f2845c.Gates run locally
--filter '@object-ui/app-shell^...' build)@object-ui/app-shelltype-checkmetadata-adminsuitecheck:control-bytes·check:vi-mock-specifiers·check:vi-mock-inheritcheck:phantom-deps·check:spec-symbolscheck-changeset-presence·check-changeset-no-majorcheck-governed-queue-guard --testAGENTS.md→ governed)Lint — a declared narrowing, measured. Repo-wide
turbo run lintis CI's run. Locally thetwo changed source files were linted with the package's own command (
eslint ., no extraflags): exit 0, 0 errors, 2 files linted per
--format json. The 55 warnings inResourceEditPage.tsxare pre-existing — the first is at line 322 and my changed ranges are130and219-262, so 0 warnings fall inside them. The narrowing excludes nothing:eslint.config.jsenables no type-aware linting (noprojectService,parserOptions.projector
*TypeCheckedconfig anywhere in it — zero hits, against a control of 10 hits forrules),so this diff cannot move the verdict on any file it does not touch.
Contract review
needs:contract-reviewis attached. The diff changes accept/reject behaviour on a contractgate: the client edit gate now accepts bodies it previously rejected. It does so by removing
the framework's own read-time decorations before a re-parse — which is what the spec prescribes
— and not by loosening any schema, so server parity moves toward the server rather than away
from it. No published face is widened:
extractDraftBodyis module-local, and this PR adds noexported symbol at all (one import, one call).
Scope
Three files: the fix, its pin, one changeset. ⛔ The hot files held by #8164
(
ConditionalFormattingEditor.tsx/.test.tsx) are untouched — 0 hits in the changed-filelist. Out-of-scope finding filed unassigned as #8181 (three other draft-envelope consumers
take
getDraft().itemverbatim with no strip); ⛔ not folded in here.⛔
sharing_rulewas not re-gated — its immunity is an accident ofAUTHOR_SHAPE_ONLY_TYPESthat #6982 preserves deliberately for this measurement.
Generated by Claude Code