fix(data-objectstack): parse a write-strip's reason against the spec enum at the boundary - #6884
Merged
Merged
Conversation
`notifyDroppedFields` filtered a create/update response's `droppedFields` on SHAPE alone — a hand-written `e is DroppedFieldsEvent` guard checking only `Array.isArray(fields)` — so a `reason` outside the spec enum reached every subscriber typed as though it were inside the union. A deployed client normally runs behind the server it talks to, so a reason from the future is the expected skew direction. `notifyBatchDroppedFields` did the same through its cast. Both paths now read `reason` against `DroppedFieldsEventSchema.shape.reason`, keep every entry, and route an unrecognized one to a named skew arm carrying the wire value verbatim. The spec type stays the canonical arm — no widening to `string`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB
Contributor
ℹ️ Console Performance Budget — not measuredThis run did not produce a console bundle to measure, so there is no pass/fail verdict for the performance budget. This is not a budget violation. Nothing was measured — the numbers a real violation would carry are simply absent.
See the workflow run for details. No package size report: it is only generated from a complete package build, so a partial one is never shown. |
…union The boundary now parses a write-strip's `reason` against the spec enum (objectui#4934), so `WriteWarningEvent['droppedFields']` is the two-arm `DroppedFieldsNotice[]` and a skew `reason` is deliberately NOT assignable to `DroppedFieldsEvent['reason']`. Two annotations in `writeWarningToast.ts` were pinned to the spec union and stopped compiling. Both were already narrower than the file's own documented contract. `lineFor` looks the reason up through a widened view of `STRIPPED_LINE`, and its docstring says outright that the runtime value may sit outside that union — so the `undefined` this branch handles is reachable, not dead. Only the parameter and the `Map` key had been left on the union; this corrects them rather than admitting a new case. `STRIPPED_LINE` keeps its `Record<DroppedFieldsEvent['reason'], StrippedLine>` declaration, so objectui#3935's guarantee — a new SPEC arm fails `type-check` unworded — survives untouched. Widen the lookups, not the table. Zero runtime change, measured rather than asserted: the emitted JavaScript is byte-identical across the diff (tsc transpile, sha256 6a5c5a95…, 6686 bytes each) and the toast's 13 wording tests pass unchanged. The changeset now states the blast radius: a consumer branching exhaustively on `reason` must widen its annotation, and that compile error is the intended signal. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB
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
|
… falsified `strippedLineUnknownReason` still described the PRE-fix boundary — the adapter "asserts the entry into `DroppedFieldsEvent` without ever checking the value against the spec enum". It checks it now: `notifyDroppedFields` parses `reason` and routes an unrecognized value to the named skew arm. The conclusion the sentence supports is unchanged — `UNRECOGNIZED_DROP_REASON` is still not a key of `STRIPPED_LINE`, so this fallback is still reachable — only the mechanism was stale. `lineFor`'s docstring opened "The PARAMETER carries the spec union", which it no longer does, and hung `STRIPPED_LINE`'s exhaustiveness on that parameter. That causal claim was already loose before this PR: exhaustiveness comes from the table's own `Record<DroppedFieldsEvent['reason'], StrippedLine>` declaration, never from this signature. Both are now stated truthfully. Comments only, and bounded: `STRIPPED_LINE`'s declaration does not move, and the sentence about the widened lookup being reachable rather than dead is untouched to the byte. The comment-stripped transpile is identical before and after (sha256 f9ff9b85b9cf39e28caf81a4bd99708bfc36b903b190af35fe56bca731e91bf8, both sides, 0 diagnostics); every changed line of the comment-preserving emit is a comment line. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB
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
|
The changeset said of the app shell "Its emitted JavaScript is byte-identical". That was true when written, at `c01e5a2` — both emit modes were identical there. The docstring commit `3ec17deb` falsified its comment-kept half: `tsconfig.base.json:22` sets `removeComments: false` and `@object-ui/app-shell` builds with a bare `tsc`, so docstring bytes do reach `dist/index.js` and `dist/index.d.ts`, and the real build's emit differs (6726 B -> 6854 B, sha256 `6a5c5a95...` -> `393f0180...`), every differing line a comment line. The EXECUTABLE emit is unchanged and measured so: comments-stripped, sha256 `f9ff9b85...` / 2181 B on both sides. One word, so the sentence says that. This one is worth a commit where the identical slip in the PR body and in the ruling was answered with a comment: a changeset is a release-notes input. It is compiled into published notes and read by consumers who cannot see this thread, so an unqualified claim here becomes the record rather than costing a reader a moment. Bump level, structure and every other line are untouched, and no `@object-ui/app-shell` entry is added: the executable emit is unchanged, and a comment-only difference in dist is not a published behaviour change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB
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
1 participant
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.
Part of #4934 — the boundary and the one consumer it moves, together. No
closing keyword: the card closes when the fable-tier review lands this PR.
What changed — the boundary (
@object-ui/data-objectstack)notifyDroppedFieldsfiltered a create/update response'sdroppedFieldsonSHAPE alone — a hand-written
e is DroppedFieldsEventguard that checkedArray.isArray(fields)and nothing else.reasonwas never read, never comparedagainst
DroppedFieldsEventSchema.shape.reason, and the predicate asserted theentry into the spec type regardless.
notifyBatchDroppedFieldsdid the samethrough
entry as DroppedFieldsEvent & { index?: number }.Both paths now parse
reasonagainst the enum the installed@objectstack/specpin declares, derived from
DroppedFieldsEventSchema.shape.reason.optionsratherthan restated, so a pin bump that adds an arm widens the accept set on its own.
Per the maintainer ruling of 2026-08-22 (option A, confirming the triage
adjudication), all four parts:
notifyDroppedFieldsandnotifyBatchDroppedFields.silence console 编辑表单对 readonlyWhen 只认一半:previous.* 口径 fail-open 致锁定字段可改且被静默丢弃;record.* 口径虽渲染成只读却仍被整表回传,触发「部分字段未保存」假告警 #3484 removed.
reasongets a named, documented skew arm —UnrecognizedDropReasonEvent, carryingUNRECOGNIZED_DROP_REASONplus thewire value verbatim in
unrecognizedReason. It is not asserted into theunion and the surface is not blanket-widened to
string: the spec type staysthe canonical arm (台账燃尽批次 6/8 · data-objectstack + plugin-chatbot + plugin-list,12 符号(objectstack#4115) #3160), and
WriteWarningEvent['droppedFields']becomesthe two-arm
DroppedFieldsNotice.packages/data-objectstack/src/droppedFieldsReason.boundary.test.ts.The shape filter's guard now asserts only what it actually checks
(
WireDroppedFieldsEntry, whosereasonisunknown) — that is where theoriginal lie lived, so it is deleted rather than worked around.
What changed — the consumer (
@object-ui/app-shell)The first round stopped and reported a fork, as the 2026-08-22 ruling required:
the sole consumer,
packages/app-shell/src/providers/writeWarningToast.ts, keptits runtime behaviour exactly but no longer compiled. That fork was ruled on
2026-08-30 (option A, comment 5468185263): the two annotations ride in this PR.
They are now applied — and nothing else in
packages/app-shell.This is not a widening — the file was already written for this value
writeWarningToast.tshandles the off-unionreasonat runtime on purpose,and said so before this card existed.
lineFor's body is a deliberately widenedlookup with a fallback:
and the docstring immediately above it says so in as many words — quoted verbatim
from the tree:
So this PR does not widen two annotations to admit a new case. It corrects
two annotations that were already narrower than the file's own documented
contract. The author anticipated exactly this value and wrote
strippedLineUnknownReasonfor it; only the parameter and theMapkey had beenleft pinned to the spec union.
STRIPPED_LINEkeeps itsRecorddeclaration over the SPEC union — line 108after this diff, unmoved. That is #3935's guarantee that a new SPEC arm fails
type-checkunworded, and it is why the annotations nameDroppedFieldsNotice['reason']rather thanstring. Widen the lookups, not thetable.
The first round's clause-② self-check concluded "does not fire" on the grounds
that nothing was added to the published
@objectstack/spectype. That reading istoo narrow: it scopes "public" to the spec's surface, and the gate does not.
Clause ② is judged from card CONTENT, and this card does both of the things it
names, on objectui's own published surface:
reasonwhere it previously asserted it; andWriteWarningEvent['droppedFields']goesfrom
DroppedFieldsEvent[]to the two-armDroppedFieldsNotice[], which isexactly what broke a consumer.
This PR therefore stays in draft for a fable-tier review.
The narrow part of the original check is still true and still worth stating:
nothing was added to the published
@objectstack/spectype,DroppedFieldsEventis re-exported untouched, and
spec-symbol-batch6.test.ts's pin that it IS thespec type still passes.
Verification
All readings in this section are on head
c01e5a2(the previous round), re-runafter that round's last commit. This round's readings are on
3ec17deb, above.pnpm --filter @object-ui/app-shell type-checktsc --noEmit && tsc -p tsconfig.test.json, exit 0pnpm exec turbo run build --filter='!@object-ui/site' --concurrency=2Tasks: 43 successful, 43 total— zero failurespnpm exec vitest run packages/app-shell/src/providers/writeWarningToast.test.tsTests 13 passed (13)pnpm exec vitest run packages/data-objectstack/Test Files 49 passed (49)/Tests 669 passed (669)— unmoved by this roundpnpm --filter @object-ui/app-shell lint2798 problems (0 errors, 2798 warnings), exit 0check:control-bytes·spec-symbols·phantom-deps·self-import·esm-specifiers·readme-exports·spec-floorscheck-changeset-no-major·-presence·-fixed·-overwritecheck:i18n-keys·i18n-drift·i18n-dead-keysThe build reading is 43 of 43, not "40 of 40". The first round measured
Tasks: 39 successful, 40 totalwith@object-ui/app-shell#buildfailing — andthat denominator existed because it was failing.
app-shell#buildhas threedirect dependents in the turbo graph (
@object-ui/console,@object-ui/example-console-starter,@object-ui/example-byo-backend-console);when it fails they are never attempted and never counted. With app-shell green
they run, and the total is 43. Three more packages build than were even attempted
before.
check:readme-exportsandcheck:spec-floorsreaddist. Their readings hereare post-build and real, not
no-artifact: readme-exports3282 export symbol(s) read from 37 of 40 package(s) ... 0 unbuilt, spec-floors274 (subpath, symbol) pair(s) judged.@object-ui/data-objectstackwas rebuiltbefore app-shell read it and the marker was confirmed to have reached the
artifact (
grep -c UNRECOGNIZED_DROP_REASON dist/index.d.ts dist/index.js→ 5, 3).check-changeset-overwriteand the three i18n gates are not in the dispatchedclue list — they were derived from the actual diff. This round edits an
existing
.changeset/*.md, andwriteWarningToast.tsis a wording table whoseown docstring says the i18n gate is watching it. Both families are green.
Zero runtime change — measured, not asserted
The claim is that the consumer's behaviour is untouched, so it is proven rather
than inferred from a passing suite. Transpiling the file before and after the
diff (TypeScript API, ES2022/ESNext, 0 diagnostics) emits byte-identical
JavaScript — 6686 bytes each, sha256
6a5c5a957517a0489699ff85054a5edd7409f8809d5fe0084ad96f2a16655e9don both sides.The 13 wording tests passing unchanged is corroboration, not the proof.
Reverse verification — the annotations are load-bearing
Predicted before running: reverting only the
lineForparameter, leaving theMapkey widened, must produce exactly oneTS2345, and at the call siteinside
emitWriteWarningrather than at the declaration, becausebyReason's keyis by then the four-arm union. Observed, after committing the fix so the revert
had a real restore point:
One error, that code, that site. The mutation was confirmed on disk before the
run (injected text present once, removed text absent,
Mapkey still widened asa control), and the restore was proven by observed state rather than an exit
code: on-disk blob
c4560fd222fd8f5d9d961ea3534793f57e45c165equalsgit rev-parse HEAD:<path>, withgit diff HEADandgit status --porcelainboth empty.
type-checkreturns to exit 0 on the restored tree.Stale prose this PR created — now FIXED in this PR
The first round left two sentences in
writeWarningToast.tsthat this diff hadjust made false. It reported them instead of editing them, because that order was
bounded to two annotations. The maintainer ruled option B on 2026-08-30
(comment 5468298926):
correcting prose this diff falsifies is finishing the change, not widening it —
"a stale comment about a type contract is the same defect class as the card."
Both are now corrected, in commit
3ec17deb, and nothing else in that file moved.strippedLineUnknownReason's docstring said the adapter "asserts the entryinto
DroppedFieldsEventwithout ever checking the value against the specenum". It now says the boundary PARSES
reasonagainst the spec enum androutes a value the enum does not name onto its explicit skew arm, whose
UNRECOGNIZED_DROP_REASONis by construction not a key of the table above —so a server ahead of the pin still arrives at this line. Only the mechanism
was stale; the conclusion it supports was true before and is true now.
lineFor's docstring opened "The PARAMETER carries the spec union — that iswhat makes
STRIPPED_LINEexhaustive-checked at its declaration above." Theparameter now carries the two-arm notice union, and the causal half was
already loose before this PR. It now says exhaustiveness has never come from
this signature: it comes from that table's own
Recorddeclaration abovebeing keyed by
DroppedFieldsEvent['reason'].Bounds honoured — checked, not asserted
STRIPPED_LINE's declaration did not move. Line 108, unchanged; still keyedby the SPEC union, which is writeWarningToast branches on
DroppedFieldsEvent.reasonwith a binary ternary, so every future reason is labelled "Read-only" #3935's guarantee that a new spec arm failstype-checkunworded.the one about the widened lookup and the
undefinedbeing reachable, not dead.Its three lines occur exactly once in both
HEADand the edited tree, and theyappear in the diff as context lines, never as changed lines.
comment line in a single file.
@object-ui/app-shellchangeset entry, and the pre-existing changeset wasnot touched —
check-changeset-overwritereads "1 changeset(s) added, 0modified, 0 deleted", and
check-changeset-presenceis satisfied by thechangeset this PR already carries.
Readings for this round (head
3ec17deb)pnpm --filter @object-ui/app-shell type-checktsc --noEmit && tsc -p tsconfig.test.json, exit 0pnpm exec vitest run packages/app-shell/src/providers/writeWarningToast.test.tsTest Files 1 passed (1)/Tests 13 passed (13)— messages unchangedcheck:i18n-keyscheck:i18n-driftcheck:i18n-dead-keyscheck:control-bytescheck-changeset-presence·-fixed·-no-major·-overwritecheck:shell-escape-residue·check-type-check-coverage·check-lint-coverageeslinton the changed file--format json: 1 file linted, 0 errors, 0 warningsEvery exit code above was captured before any pipe, and each row quotes the gate's
own verdict line rather than a bare
$?.The lint reading is a declared narrowing, not a repo scan. It carries the three
things a narrowing owes: the population came from eslint's own resolution (the file
resolves a config and is linted, not ignored — 1 file in the JSON report, not 0);
the count came from
--format json; and the invariance holds because this configis not type-aware —
--print-configshowsparserOptionswith noprojectandno
projectService, so no cross-file type information exists for a comment-onlyedit in one module to move. The full repo lint is CI's run either way.
Gate families deliberately not re-run, with the reason: everything whose subject is
the export surface, the
distartifacts, the module graph or the registrationtables —
spec-symbols,spec-floors,readme-exports,phantom-deps,self-import,esm-specifiers,published-dist,vi-mock-specifiers. Theexecutable emit is proven unchanged (below), so none of them can move.
Correction to the ruling: "byte-identical emit" needs its comment mode stated
The ruling's ground for adding no changeset entry was that "the emitted JavaScript
is byte-identical ... The prose edits B adds cannot change that." Measured on this
round, that holds only with comments stripped, and this repo does not strip
them:
tsconfig.base.jsonsetsremoveComments: false, and@object-ui/app-shellbuilds with a bare
tsc. Docstring bytes therefore do reachdist/index.jsanddist/index.d.ts.Both modes were measured before and after the edit, each leg carrying a success
control — 0 diagnostics, real JS emitted, three known markers present — because a
digest taken from a command that failed identically twice is a perfect false
identity proof, which is exactly the trap the last round caught itself in:
f9ff9b85· 2181 Bf9ff9b85· 2181 BremoveComments: false, the real build6a5c5a95· 6726 B393f0180· 6854 BFiltering the comment-kept emit diff for changed lines that are not comment lines
returns nothing: every byte that moved is docstring prose. So the ruling's
conclusion survives on stronger ground than the phrase it used — zero executable
change, with the only published bytes that move being documentation. Read it as
"byte-identical executable emit".
Footnote on the earlier number: the previous round's
6686is this same emitcounted in UTF-16 code units; the file carries CJK and em-dashes, so its length in
bytes is 6726. The sha256 is unchanged and was reproduced here exactly —
6a5c5a957517a0489699ff85054a5edd7409f8809d5fe0084ad96f2a16655e9d.What was verified against the tree rather than taken from the order
origin/mainand matched exactly. After the added import they are 108 / 158 / 191.
before being repeated here; the ruling's rendering of
lineFor's body had itsgeneric arguments eaten by GitHub's body sanitizer, the tree has them.
@objectstack/specis 17.2.0 andDroppedFieldsEventSchema.shape.reason.optionsis exactly["readonly","readonly_when","primary_key"].Generated by Claude Code
Generated by Claude Code