Repository navigation
fix(data-objectstack): read the flat REST error envelope through one reader (objectui#9594) - #9812
Merged
Merged
Conversation
…reader (objectui#9594)
A denied CSV export reached the operator as the single word `Forbidden` while
the server was answering a sentence naming the object, the user and the axis.
The adapter's failure ladder knew two envelope dialects -- the ADR-0112 nested
`error: { message }` and the flat shape carrying a `message` key -- and the REST
export gate writes a third: flat, sentence in a bare string `error`, no
`message` key at all. Every rung missed and the ladder fell through to
`res.statusText`.
That ladder was hand-copied at four sites (`searchAll`, `rawFindWithPopulate`,
`exportDownload`, `fetchObjectSchemaFresh`), with a narrower variant at the two
upload paths, so all six shared the blind spot. Hoist one internal
`readErrorEnvelope` and route all six through it.
The rung order is load-bearing: `message` is consulted before a string-valued
`error`, because the REST 401 body is flat AND carries both, with the code word
in `error` and the sentence in `message`. The 401/403 pair is pinned as a
control against that inversion.
The four `res.ok` ladders now also attach the server's `code` to the thrown
error, matching what `rawFindWithPopulate` already did. The code is read from
`code` or a nested `error.code` and never promoted out of a string `error`.
The reader is deliberately not exported from the package entry point; the
published face is unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xm4WFhEe5mwcgyqHjxR2hn
…ort-error-envelope-reader
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
|
os-sales
marked this pull request as ready for review
September 18, 2026 06:59
Collaborator
Author
|
Reviewed, green and ready at 2026-09-18T06:59Z — ACCEPT recorded on objectui#9594 (comment ⛔ This seat cannot put it in the merge queue, for the same reason and with the same refusal already recorded in full on PR objectui#9804: ⇒ This is a pointer, not a second report: the blocker, what this seat will not do about it (⛔ no MCP fallback, ⛔ no direct merge, ⛔ no retry through another door) and the two ways to unblock it are all on objectui#9804 and were reported to the maintainer there. ⛔ No auto-merge attempt was made on this PR: the refusal is identity-based and therefore predictable, and retrying a denied action class unchanged is not a reading, it is repetition. Nothing else is outstanding on this PR. It stays watched until it is merged or closed. Posted by the Generated by Claude Code |
This was referenced Sep 18, 2026
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 #9594
Clause-②: no
A console operator clicking "export to CSV" without the
allowExportgrant was shown oneword:
Forbidden. The server had answered a sentence naming the object, the user and theaxis. This routes six hand-copied failure ladders in
ObjectStackAdapterthrough oneinternal envelope reader that understands every dialect on the wire.
Per the seat ruling on the card (option 2 carrying option 1's behaviour), the rung order is
error.messagethenmessagethen a bare-stringerrorthenstatusText.What changed
One new internal module,
packages/data-objectstack/src/error-envelope.ts, exportingreadErrorEnvelope(body)which answers a message and a code. Six sites now call it:searchAllstatusTextcoderawFindWithPopulatecodecodeprecedence preserved exactlyexportDownloadstatusTextcodefetchObjectSchemaFreshstatusTextcodeuploadFileerror.messageonly, own fallback textuploadFileserror.messageonly, own fallback textThe reader is deliberately not re-exported from the package entry point. Verified
mechanically: the export list of
src/index.tsis byte-identical to base, 64 export lineson both sides, so
Clause-②: noholds.Mechanism assumptions, each re-measured on my own base
1. The six sites are one ladder — PARTLY FALSIFIED, and the brief predicted where.
The four
res.okladders really are one shape, and the card's line numbers were all exact onmy base. The
statusTextpair is not the same shape:uploadFile/uploadFilesreadonly
error.message, fall back to a template rather thanstatusText, and throwObjectStackErrorwith a hard-codedUPLOAD_ERRORcode instead of a bareErrorcarryingstatus. So they share the dialect reading and keep their own envelope — forcing theirlast rung and their code through one function would have changed behaviour nobody asked for.
The card's phrase "the identical ladder is copied at five more sites" is accurate for four of
the six and loose for the upload pair.
2. The flat dialect is still on the wire — CONFIRMED.
@objectstack/restis not adependency of this repo, so I read the producer source in the sibling checkout, which
declares version
17.4.0— the same version the card measured.enforceExportPermissionstill writes
codeplus a stringerrorplusobject, with nomessagekey.3. The affected-code count — RE-MEASURED INDEPENDENTLY, and it reproduces. My scan walked
the producer's source tree rather than the shipped bundle the card used, brace-matching every
.json(body and keeping those withcodeanderrorand nomessage: 28 files, 68 matchingbodies, 34 distinct literal codes. Diffed against the card's list: identical, zero
difference either way.
EXPORT_NOT_PERMITTEDwas the known-hit control and it fired. Adifferent instrument reaching the same population is corroboration rather than a restatement.
Also measured: of those bodies, zero spell
errorequal tocode, so the bare-string rungalways yields a real sentence for that family.
4. The 401-vs-403 control — CONFIRMED, and it turned out to be load-bearing. The 401 body
is
{ error: 'UNAUTHENTICATED', code: 'UNAUTHENTICATED', message: ... }: flat, and itserrorkey holds the code word while the sentence lives in
message. That is exactly why theruled rung order puts
messagebefore a stringerror. Inverting those two rungs would makeevery unauthenticated response render the word
UNAUTHENTICATEDinstead of its sentence. Thepair is pinned as a control at both the reader and the call site.
Not a lenient fallback
Reading this dialect is conformance, not tolerance, and the producer's own text says so.
ADR-0112's 2026-07-30 amendment records the flat and wrapped envelopes as the two live,
sanctioned shapes, D5 leaves the flat one's fate an open maintainer question, and
@objectstack/core's guidance besideANONYMOUS_DENY_BODYstates the rule: read the envelopethe seam you called DECLARES — flat from
/dataand/meta. Every site routed through thereader calls a
/dataroute. The server envelope is untouched; that belongs to objectstack#9559.That same guidance names
body.error?.code ?? body.erroras precisely the chain an enveloperegression hides inside, so the reader takes
codefromcodeor a nestederror.codeandnever promotes a string
errorinto that slot. A sentence is not a code.Decision on the card's optional extra (Zone 3)
The card floated appending
codeto the message text so an operator can search forEXPORT_NOT_PERMITTED. Taken, but as a structured field rather than in the text. The fourres.okladders attach the server code to the thrown error. Reasons:rawFindWithPopulatealready did exactly this, so the other three now match it instead of drifting from it;
concatenating into
messagewould corrupt the string every toolbar and i18n surface displaysand make the message unstable to assert on; and it gives the refusal tests a real
codetoassert. The upload pair keeps
UPLOAD_ERROR— putting a second, differently-sourced code onone error with no declared precedence is the second de-facto contract AGENTS.md #0.1 bans.
Tests, and proof they are not vacuous
pnpm exec vitest run packages/data-objectstack/— 67 files, 912 tests, all passing, readat
9cf9e0c6(the merge commit, so after the last content change). Refusal cases assertcodeand
statustogether, never a bare throw.Two ablations, in opposite directions, each mutated on disk through an anchor that had to
hit, with the injected and deleted text counted by
grep -con both sides and the restoreproven by
git diff HEADbeing empty and the blob hash matching HEAD:flat-dialect case, across all three test files — and the 401 control stayed green.
messageand the stringerror: 3 tests red, every one a 401 case, and every403 case stayed green.
The two mutations turn disjoint sets red, which is what shows the 401 case is a genuine
control rather than a second copy of the 403 assertion. The call-site tests going red from a
mutation inside the reader module is also what proves the wiring is real at those sites.
No build was needed for either leg and none was run: the root vitest config aliases the
workspace packages to
src, and the tests import the reader and the adapter by relativepath, so vitest compiles this tree's source directly. If source had not been what ran, no
mutation to it could have changed a verdict.
Gates, exit code captured before any pipe
Derived from this repo's
package.jsonscripts and.github/workflows/, against the realdiff.
scripts/pm/dispatch-gates.mjswas not used: it lives in the sibling repo andderives families from the tree it runs in, so it cannot answer for this one.
check:new-line-citations(Line Citation Gate)check:control-bytescheck:changeset-presencecheck:changeset-no-major/-overwrite/-fixed/-claimscheck:pending-changeset-literalscheck:unreferenced-sourcescheck:self-importcheck:test-path-rootscheck:vi-mock-specifiers/-inherit/-override-shapecheck:readme-exportscheck:dist-completeness --allcheck:published-distcheck:published-tsconfig-excludecheck:esm-specifierscheck:entry-guardcheck:side-effects-arraytype-check(this package)lint(this package)check:node-esm-loadpnpm build(whole workspace): 43 of 43 tasks successful.check:readme-exportsfirst answered 1 and that reading was discarded, not reported. Itsown output said the population had collapsed and named the cause — 34 unbuilt packages, all
four findings in
plugin-calendar/plugin-gantt/plugin-kanbanREADMEs, none in thispackage. After a full build it is 0 over a real population (58 keys compared, 0 unbuilt). A
prerequisite-not-met exit is not a failed measurement.
check:node-esm-loadanswers 1 for a reason outside this branch. Turbo shares one cacheacross every worktree of a checkout, so the build replayed a sibling agent worktree's
artifacts (
objectui-issue-9568) for five packages —core,i18n,providers,react-runtime,types— and the gate refuses to grade what this tree did not produce ratherthan greening over another tree's output.
@object-ui/data-objectstackis not among therefused: it was graded and its ESM entry loaded clean. The five are untouched by this diff, and
a clean CI checkout has no sibling worktree to inherit from. I force-rebuilt those five through
pnpm and the marker persisted, because the provenance marker is turbo's own task log and a
filtered pnpm build does not go through turbo; clearing it needs a full
TURBO_FORCErebuildof the workspace, which I judged not worth several minutes of exclusive hold on this shared
container's verify lock to re-derive a fact about packages this branch does not touch.
The repo-wide
pnpm lintandpnpm testare CI's runs. The lint narrowing here is declared andmeasured: the root
eslint.config.jsis the only config (this package declares none), thepackage's own
eslint .— the exact per-package unitturbo run lintinvokes — linted 76files, 0 errors, 446 warnings by its own
--format jsoncount, and type-aware linting isnot enabled (
tseslint.configs.recommended, noprojectorprojectService), so nothingin this diff can move the verdict of a file it does not touch. Every warning is the
pre-existing
no-explicit-anypopulation; my appended export-test block adds none, and thesearchAllinsert adds exactly one, in that file's establishedfetchImpl as anyidiom.Acceptance notes
Observed and deliberately not filed:
app-shell,and if both land, one reader shared across packages is the honest end state. Out of scope
here by the card's own fence, and recorded rather than built. Successor: whichever of
objectui#7959 / objectui#7938 / objectui#7368 lands next in this family.
statusTextsites that are not ladders and were left alone. The discovery probebuilds a
statusplusstatusTextstring and never opens the body at all; the dataset-querythrow already derives its detail through a separate classifier. Neither is an envelope read,
so neither belongs to this reader. Successor: none — noted so the next reader of this module
does not mistake the count of
statusTextmentions for the count of ladders.which names it and cites objectui#7276 — a condition of running several agents in one
checkout, not a defect.
Generated by Claude Code