fix(data-objectstack): parse a dropped-fields entry's fields elements and object instead of asserting them - #7159
Merged
Conversation
…ts and `object` instead of asserting them
The write-warning boundary's structural gate read `Array.isArray(fields) &&
fields.length > 0` and asserted the entry into `Omit<DroppedFieldsEvent,
'reason'>`, which claims a `string[]` of field names and a REQUIRED
`object: string`. It read neither. `fields: [42]` reached every subscriber
typed as a field name, and an entry with no `object` arrived claiming a
string that was not there.
Same discipline the sibling `reason` fix applied: parse rather than assert.
- `WireDroppedFieldsEntry` now declares exactly what the gate establishes:
`{ object?: unknown; fields: unknown[]; reason?: unknown }`.
- The gate is named (`isWireDroppedFieldsEntry`), shared by the single-record
and batch paths, and requires at least one string element — an entry that
names no field has nothing truthful to say.
- `asDroppedFieldsNotice` narrows `fields` to its string elements and takes
`object` from the wire when it is a string, else from the object the write
targeted. It now BUILDS the notice, so the last cast in this seam is gone.
- The batch path computes the object name once, for both the notice and the
event's `resource`; those were two separate expressions that could disagree.
No published type changes: `DroppedFieldsNotice`, `WriteWarningEvent` and
`UnrecognizedDropReasonEvent` keep their declared shapes, and a subscriber's
`fields: string[]` is now true rather than asserted.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012wwHa4aaFybxXrfmfHioDM
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012wwHa4aaFybxXrfmfHioDM
Contributor
✅ 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
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #6889
Measured on
origin/mainatfd11e1644(fd11e1644601d261ec00e371f8bf0b4ce8a7455a). The card is from 2026-08-30 and its snippets were not trusted; the filter, the type and the reader were all re-derived at that head.1. The gating measurement — driven, not reasoned
The card left the severity question open and pointed at the one reader. That reader was executed, not read: a scratch probe drove the real production chain end to end —
ObjectStackAdapter.create()toonWriteWarningtoapp-shell'semitWriteWarning, with the realtfromuseObjectTranslation()and the realfieldLabelfromuseSafeFieldLabel()(exactly whatAdapterProviderpasses) and a recording sink.fields['type','source_method'](control)... : Andon type, Source method[42]... : 42[{ name: 'salary' }]... : [object Object][null, 'type']... : , Andon type[['type','source_method']]... : type,source_method[true]... : true['type', 42, {}]... : Andon type, 42, [object Object]objectkey absentWriteWarningEvent.resourceNo throw on any JSON-representable non-string element. The reader degrades to a wrong label, or to an empty entry for
null. Nothing a user would act on as a real field name.The one throw observed came from a deliberately non-JSON probe (an element whose
toStringthrows), and it did not reach the reader at all — it threw inside the adapter, inwithoutNoOpDrops, onObject.prototype.hasOwnProperty.call(sent, f).droppedFieldsarrives as parsed JSON, so that value is not reachable from the wire. Reported as a curiosity, not as evidence of a defect.Verdict: severity is cosmetic. Per the card's own ruling that licenses the cheaper honest repair — never no repair.
2. Which branch that selected, and why it is the opposite of the cheap one
The card offered two shapes: narrow the filter to check element types, or keep the filter permissive and let the declared type say
unknown.The second one — the "cheaper" branch — cannot be taken without moving a published type.
WireDroppedFieldsEntryfeedsasDroppedFieldsNotice, which returnsDroppedFieldsNotice, whosefieldsisstring[]on both arms and which IS re-exported. Declaring the wirefieldsasunknown[]and stopping there only relocates the split one call down, into the cast; carried to its honest conclusion it makes a subscriber's declaredstring[]becomeunknown[]. That is the contract tier, and it is not what this PR does.So this PR takes the filter-narrowing branch, and additionally makes the internal wire type honest — which is free, because
WireDroppedFieldsEntryis not published (evidence below). The two together give the shape the card asked for: what the code establishes and what the declaration claims are now the same thing at every step.3. Both over-claims held, independently
fieldselements — the gate readArray.isArray(fields) && fields.length > 0and nothing about the elements, while the asserted type declaresstring[]. Real.object— the installed spec pin (@objectstack/spec@17.2.0) declaresobject: z.ZodString, i.e. required.Omitcarried that requiredstringthrough, and the gate never readobject. Real, and independent of the first.This falsifies the dispatch's A2.3 hypothesis, which guessed that a required
objectmight mean "no gap". Required is precisely what makes it a gap — an optionalobjectwould have had nothing to over-claim. The adapter's own batch path already knew this: it wrotee.object ?? op?.object ?? '', a runtime defence against an absence the type denied.4. What changed
WireDroppedFieldsEntrynow declares only what the gate establishes:{ object?: unknown; fields: unknown[]; reason?: unknown }.isWireDroppedFieldsEntry) and shared by the single-record and batch paths, which previously spelled it twice. It requires at least one string element: an entry naming no field has nothing truthful to tell the user, which is the same disposition the boundary already gavefields: [].asDroppedFieldsNoticenarrowsfieldsto its string elements, and takesobjectfrom the wire when it is a string, else from the object the write targeted. It now builds the notice, so the last cast in this seam is gone.resource; those were two separate expressions that could disagree.The repair is deliberately asymmetric with
reason's, and the code says why: a reason from the future is the expected direction of version skew, so it earns an explicit skew arm carrying the wire value verbatim.fieldsisz.array(z.string())in the spec and cannot grow a non-string element without a breaking change, so a non-string element is off-spec input — refused here, fixed at the producer (AGENTS.md #0.1), never given an invented rendering.objectis neither: the adapter knows which object it wrote to, so a missing one is healed rather than dropped, and no entry is ever silenced for it (objectui#3484).Bounded, per the card's own scope
No widening into PR #6884's territory (
reasonuntouched) or objectui#4934's. objectui#3160's canonical arm is not widened tostring.app-shell'swriteWarningToast.tswas read only — it is unchanged, and the probe that drove it was scratch and is not in this diff.5. Evidence
Published-surface probe — the claim that no published type moved. Built the package at
fd11e1644and at this branch's head and compared the emitteddist/index.d.ts:privatemethod's JSDoc. So the zero is a reading, not a broken comparison.git diff fd11e1644over the source adds and removes zero lines beginning withexport.WireDroppedFieldsEntryappears 0 times in the builtdist/index.d.ts; positive controlDroppedFieldsNoticeappears 4 times on the same grep. The two private methods emit signature-less (private notifyDroppedFields;).Reader census. Repo-wide grep for
onWriteWarning,WriteWarningEventandDroppedFieldsNotice: exactly one non-test subscriber,packages/app-shell/src/providers/AdapterProvider.tsx:78, which callsemitWriteWarninginwriteWarningToast.ts. Nothing inapps/. Control: the same instrument returned dozens of hits across the test suites, so the single production hit is a reading.No test pinned the permissive behaviour. Every
fields:array in the write-warning suites is all-strings or empty; none pins a non-string element or a missingobject. So the fix's shape was not constrained by an existing pin.Ablation. Direction predicted before running: reverting only
index.tstofd11e1644, keeping the new test file, turns 8 of the 9 new tests red and leaves exactly one green — theobjectpass-through control — while the sibling suites stay green.Mutation proven on disk by blob hash, not by an editor's exit code: worktree
c48540398871a4a9bb4e4e4a049598f267080affequals the base blob and differs from the HEAD blob; marker countsisWireDroppedFieldsEntry4 to 0 andfallbackObject3 to 0, withfields.length > 0back at 2. Observed: 8 failed, 34 passed across 4 files — exactly the predicted split, including the single green control. Restore proven by state:git diff HEAD,git diff --cachedandgit status --shortall empty, and the worktree blob hash back to1b93160c80b2d6ebd3a01366a2d05ba5a48c5d14.No rebuild was needed for the ablation to be valid:
vitest.config.mtsaliases every workspace package to itssrc, and the suites import./indexrelatively, so the mutation reached the run from source rather than from a staledist.Gate union, re-run after the final commit, at
e268018b4(exit code captured by redirect-then-capture, never across a pipe; verdicts quoted from each gate's own line):pnpm --filter @object-ui/data-objectstack type-checktsc --noEmitcleanpnpm exec vitest run(5 files, from the repo root)Test Files 5 passed (5)/Tests 56 passed (56)pnpm --filter @object-ui/data-objectstack lint419 problems (0 errors, 419 warnings)check-changeset-presence.mjs2 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)pnpm changeset:checkNo changeset declares a major bump.pnpm check:control-bytesOK (scanned 5931 tracked text file(s); skipped 85 binary)pnpm check:self-importNo package names itself inside its own src/.pnpm check:phantom-depsEvery in-scope import is declared by the package that publishes it.pnpm check:vi-mock-specifiersOKpnpm check:vi-mock-inheritOKThe package
type-checkreally does cover the new test file:tsc --listFilesnamesdroppedFieldsShape.boundary.test.tsonce, withsrc/index.tsas the control.Declared narrowing. The repo-wide
pnpm lintis CI's run; this branch ran the per-package form instead, and the narrowing is measured rather than skipped: the population came from eslint's own config resolution (eslint .in the package dir, not a hand-picked list);--format jsonreports 58 files linted, 0 errors, and both touched files are in that list; andeslint.config.jsdeclares noprojectServiceand noparserOptions(control:rulesmatches 10 times on the same grep), so type-aware linting is off and this diff cannot move the verdict on any file it did not touch.Not measured. Whether any real server emits a non-string
fieldselement or omitsobject. That is unanswerable from these two repos — a host application outside them is invisible to any measurement runnable here — and no inference about it was made.Generated by Claude Code