fix(app-shell): every envelope reader on the package surfaces renders the producer-marked userMessage - #7981
Merged
Conversation
… the producer-marked userMessage `fetchPackages` answered a refusal with `HTTP <status>` and never opened the body, so `message`, `code` and `userMessage` were discarded together — a 403 whose envelope named the capability to grant reached the author as four characters. `apiJson` on the package admin page read `error.message` and never the mark, and rendered no code. `duplicatePackage`, in the same module, read `error.message` alone. Those were three independent implementations of "read the ADR-0112 failure envelope" on one endpoint family, already drifted into three different answers for the same body. They now ask one shared rule: a producer-marked `error.userMessage` outranks the diagnostic `error.message` at any status — presence of the field IS the producer's marking — with `error.code` appended to whichever prose won. Each reader keeps its own fallback and, for `apiJson`, its own legacy rungs; those are not the envelope. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KbJQ1y1J12nZxYzFWhP8Q3
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 was referenced Sep 6, 2026
os-sam
marked this pull request as ready for review
September 6, 2026 06:44
This was referenced Sep 6, 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
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.
Fixes #7959
Clause-②:
no— this reads a few fields that are already on the wire and renders them. Noschema's accept/reject behaviour changes, no public surface widens, no gate's scan population
moves.
What was lost
fetchPackages(views/studio-design/packages-io.ts)throw new Error('HTTP ' + status)— the body was never opened, somessage,codeanduserMessagewere discarded togetherapiJson(views/metadata-admin/PackagesPage.tsx)error.messageladder — nouserMessage, nocodeduplicatePackage(same module asfetchPackages)error.messagealoneThe measured cost:
GET /api/v1/packagesanswers a 403 whose message isReading packages requires the studio.access or setup.access capability.— a sentence that names thecapability to grant — and
fetchPackagesreported it as the four charactersHTTP 403.Every caller of that read already displays words: the Studio switcher, the writability
courtesy gate and the namespace lookup all render
formatMetadataError(e)(which returnserr.message) onto the sharedstudio-package-listsonner id (#7368's posture), and thebuilder landing page puts the same string in its error banner. The plumbing was wired; there
was nothing to put in it.
The decision the card asked for: can one signature serve the call sites?
Measured first, then extracted. The three sites differ in four ways, and only one of the
four is the envelope read:
fetchPackagesResponse, body unread!res.okHTTP+ STATUSapiJson(generic in T)res.text()+JSON.parse)!res.okorpayload.success === falseRequest failed (+ STATUS +)error, top-levelmessageduplicatePackageres.json().catch(() => null))!res.okHTTP+ STATUSSo a helper that owns the whole failure cannot serve them —
apiJsonhas already consumedthe body as text and its trigger is not
!res.ok, and folding the fallbacks in would haveneeded a
fallbackparameter plus alegacyRungsflag: three different things pressed intoone signature, harder to read than the copies it replaces.
A helper over the parsed body does serve all three, exactly:
Envelope in, the person's prose out — a producer-marked
error.userMessageoutranking thediagnostic
error.message, witherror.codeappended to whichever prose won — ornullwhenthe body carried no prose, in which case each caller states its own fallback on its own line.
apiJson's two legacy rungs stay atapiJson, deliberately: a bare-stringerrorand atop-level
messageare older runtimes' shapes and are not this envelope; folding them inwould hand every other consumer a tolerant dialect it never asked for.
New file:
packages/app-shell/src/utils/apiErrorEnvelope.ts. Not exported from the package'spublic entry — the surface stays where it was.
1.
duplicatePackage, the fourth reader, in the same file. It readerror.messagealone,dropping the mark and the code, on a route (
POST /packages/:id/duplicate) served by thedispatcher door — the twin that has emitted the marked channel since #9934. It is the same
defect class, in this card's declared file surface, and the correct shape was already pinned
by #7938 / PR #7960. Leaving a hand-rolled copy of the rule a hundred lines below the import
would have been the exact drift this extraction exists to stop. Pinned in
packages-io.envelopeUserMessage.test.ts; its existingpackages-io.duplicateEnvelope.test.ts(which is about the operation's verdict inside a 200) is untouched and still green.
2.
data-testid="packages-load-error"on the PackagesPage error banner. The pins need astable handle: the words in that banner are the server's, so a test that located the banner
by those words could not assert what is absent from it (
not.toContain(GENERIC)).Out of scope and untouched:
fetchFullPackageinStudioDesignSurface.tsx. PR #7960 ischanging that exact function and has not merged. Migrating it onto this helper is a follow-up
once #7960 lands — its landed expression and this helper already agree behaviourally, so the
migration is a deletion.
Not scoped to 5xx, deliberately
The producing door applies no status condition to this channel — "a marked text is the
producer's deliberate statement to the caller at any status" — so honouring it in one band
only would re-create, on the reading end, the divergence that door refused to create on the
writing end (ruled on #7938). The helper makes that structural rather than merely stated: the
rule takes the body and nothing else, and a pin asserts its arity, so an "only in the 5xx
band" variant cannot be written without changing a line that is pinned.
Pins — 51 new assertions across the four combinations
Three files, because "the rule is right" and "this reader actually asks it" are different
claims:
utils/apiErrorEnvelope.test.ts(21) — the rule where it is defined.views/studio-design/packages-io.envelopeUserMessage.test.ts(17) —fetchPackagesandduplicatePackagethrough real fetch mocks.views/metadata-admin/PackagesPage.envelopeUserMessage.test.tsx(13) —apiJsondriventhrough the rendered page, because
apiJsonis module-private and what the card is about iswhat the person reads.
All four combinations (
messageonly /userMessageonly / both / neither) are pinned at eachsite, plus the
codeinteractions (appended to the winning prose; bare sentence when no code;still the bare status when a code arrives with no prose; a non-string or empty-string
userMessageis not a mark and falls through tomessage).fetchPackagesadditionally pinsthat
messageandcodenow arrive at all — its loss was the whole body, not one field.Forward control — the fix reverted, predicted before it was run
The cut reverts only the two call-site reads, keeping the helper and every pin file in
place. Deleting the helper instead would have turned every pin into
MODULE_NOT_FOUND— a NOTMEASURED reading, not a red one — and keeping it gives the run a negative control.
PackagesPage.tsxwas mutated surgically (its top ladder rung only) rather than revertedwhole, because a whole-file revert would also have removed the
data-testidhandle and turnedevery PackagesPage pin red for a reason that is not the defect.
apiErrorEnvelope.test.ts(negative control)packages-io.envelopeUserMessage.test.tsPackagesPage.envelopeUserMessage.test.tsxTests 20 failed/31 passed (51)Prediction/measurement delta: zero. The green-when-reverted guards all held: PackagesPage
§1 (
messageonly, no code) stayed green — the pin that stops "preferuserMessage" frombeing implemented as "read
userMessageinstead", which would blank every unmarked refusalthe platform serves today — as did §4, §6's three legacy-rung cases, and on the
packages-ioside the unparseable-body and successful-read controls.
fetchPackages'smessage-only pingoes RED on revert, unlike #7938's. That is the defect, not a weaker pin: the pre-fix
reader opened no body, so the unmarked refusal was as lost as the marked one. The
green-when-reverted guard role is carried there by the negative controls instead, and the
apiJsontwin does hold the classic form. Both files say so in their docblocks.The control also found a defect in this PR's own tests: §7 (a 200 declaring
success: false) was sitting under a describe headed "GREEN with the fix reverted" whilebeing red — a claim the run does not support. It now has its own block, with a note saying
why.
Restore proved by state, not by exit code:
git diff HEADempty,git status --shortempty, and each file's worktree blob hash compared equal to its
HEADblob (an empty hashwould have been read as failure). The script carried a
trap … EXIT INT TERMrestore usingabsolute paths, and
git checkout HEAD --PATH rather than the bare form, which wouldhave taken the file from the index the mutation leg wrote.
Verification — all at final commit
f82ae88ad,git rev-parse --short HEADRun from the repository root (
RUN v4.1.10 /home/user/objectui-issue-7959), exit codescaptured before any pipe.
pnpm --filter '@object-ui/app-shell^...' buildpnpm exec vitest run packages/app-shell/Test Files 631 passed (631),Tests 6062 passed, 1 skipped (6063)pnpm --filter @object-ui/app-shell run type-checktsc --noEmit && tsc -p tsconfig.test.jsonpnpm --filter @object-ui/app-shell run lintnode scripts/check-changeset-presence.mjs✅ 6 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s): .changeset/olive-crabs-shave.md.node scripts/check-changeset-no-major.mjs✅ No changeset declares a major bump.node scripts/check-control-bytes.mjsnode scripts/check-unreferenced-sources.mjsnode scripts/check-vi-mock-specifiers.mjsnode scripts/check-vi-mock-inherit.mjsnode scripts/check-phantom-dependencies.mjsnode scripts/check-package-self-import.mjsThe type-check green is not vacuous:
tsc -p tsconfig.test.json --listFilesputs all sixchanged/added files in a program (the three test files appear in the test program, the three
sources in both).
The lint run is a declared narrowing, and a measured one. Scope is the
@object-ui/app-shellpackage rather than the repo-wide
pnpm lint;--format jsonreports 1086 files judged, 0errors, all six changed files among them. Nothing untouched can have moved:
eslint.config.jsconfigures no type-aware linting (no
project/projectService), so each file's verdictdepends only on its own bytes plus the shared config, which this diff does not touch. CI runs
the full farm regardless.
Out of scope, filed rather than fixed
PackageFormDialogholds a fourth copy of the envelope ladder and still dropserror.userMessageanderror.code#7979 —PackageFormDialog.tsxholds a fourth copy of the identical ladder and stilldrops
userMessageandcode. Same package, outside this card's declared file surface.error.message, so a markeduserMessageand thecodenever reach the developer #7980 —apps/console'sAgentConnectSectionreads onlyerror.message, and its!data?.key-on-200 arm reportsRequest failed (200)for something that was not a transportfailure. Different app, different route, wants its own decision.
Neither is a duplicate: checked against all 407 open issues in this repository.
Angle-bracket placeholders are spelled as capitalised words throughout this body on purpose:
GitHub's body sanitizer eats tag-shaped fragments on save, and backticks and fenced blocks do
not protect them (AGENTS.md, "GitHub rewrites the bytes you put in an issue/PR body").
Authored by the
os-devseat, sessionhttps://claude.ai/code/session_01KbJQ1y1J12nZxYzFWhP8Q3.🤖 Generated with Claude Code
https://claude.ai/code/session_01KbJQ1y1J12nZxYzFWhP8Q3
Generated by Claude Code