Uh oh!
There was an error while loading. Please reload this page.
fix(app-shell): no Retry for an api-disabled attachments list - #4707
Merged
Conversation
RecordAttachmentsPanel offered a Retry for OBJECT_API_DISABLED (404) and
OBJECT_API_METHOD_NOT_ALLOWED (405) sys_attachment reads, both pure functions
of the object's `enable` block — no persona and no retry changes the answer.
The panel gains a fifth status, api-unavailable: no Retry, honest copy ("The
attachments list is not available on this object.", new
detail.attachmentsApiUnavailable key in all ten locale packs). denied and
unavailable keep their pre-existing meaning and affordances.
Lifts ListView.classifyLoadError out of plugin-list's module scope into
@object-ui/react (classifyLoadError, LoadErrorKind) so both surfaces consume
one classification instead of re-deriving it; ListView's own behavior is
unchanged (same function, now imported). The classifier now delegates its
api-disabled check to isApiAccessDeniedError (@object-ui/data-objectstack),
removing a second, independently-maintained copy of the same code list.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RnQd8iMMUwXQEV1crFmQiQThe latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
yinlianghui
commented
Aug 15, 2026
CollaboratorAuthor
Review verdict: ACCEPT (reviewer of record, PM session Verified against this PR, not the report: all 17 changed files account for exactly the classifier lift (3 files in Non-blocking nit, recorded rather than actioned (out of this diff's files): Will flip ready + enable auto-merge once Lint, Type Check, and Test shards 1–4 conclude Generated by Claude Code |
yinlianghui
marked this pull request as ready for review
August 15, 2026 13:10
Uh oh!
There was an error while loading. Please reload this page.
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 freeto 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#4693
What
RecordAttachmentsPaneloffered a Retry for asys_attachmentlist read thatfailed with
OBJECT_API_DISABLED(404,enable.apiEnabled: false) orOBJECT_API_METHOD_NOT_ALLOWED(405, the operation absent fromenable.apiMethods). Both are pure functions of the object's metadata — nouser, no session, no request body — so every retry of every persona
re-fetches the identical refusal. The button was the same wrong advice
("try again") that
ListView's error panel already stops giving for listreads, one surface over (issue #4408).
The panel gains a fifth status,
api-unavailable: no Retry button, honestcopy ("The attachments list is not available on this object.", new
detail.attachmentsApiUnavailablekey, synced to all ten locale packs). Thepre-existing
denied(authorization, issue #4269 / PR #4685) andunavailable(network/5xx/expired-session, issue #4684) states keep theirexact meaning and affordances — this only inserts one new fork ahead of them.
Classifier home:
@object-ui/react, not a new dependency edge either wayFollowed the ruling's preferred half of the card's suggested shape: lifted
ListView.classifyLoadErrorout ofpackages/plugin-list/src/ListView.tsx'smodule scope into a shared home, rather than growing a second,
independently-hand-rolled
api-disabledcheck on the panel.Checked the function itself before picking a home: it is a pure classifier —
takes an
unknownerror, returns aLoadErrorKindstring, never rendersanything. All the i18n/copy is owned separately by each caller (
ListView'sown title/message-per-kind ternaries; the panel's own single sentence per
status). So there is no list-specific text tangled into the function, and the
whole thing could move as-is — no split into "kernel vs. copy" was needed.
Landed it in
@object-ui/react(packages/react/src/utils/error-message.ts,next to the existing
isPermissionErrorthe panel already imports fromthere), not
@object-ui/data-objectstackor a new export fromplugin-list:@object-ui/plugin-listand@object-ui/app-shellalready depend on@object-ui/react(confirmed in bothpackage.jsons before touchinganything) —
plugin-listalready importsSchemaRenderer/useNavigationOverlayfrom it,app-shell'sRecordAttachmentsPanelalready imports
isPermissionErrorfrom it. Landing in@object-ui/data-objectstackwould have needed a brand-new dependencyedge from
plugin-list(not currently a consumer — several siblingplugin-*packages are, but not this one), for zero benefit over reusingan edge both sides already have.
isPermissionErroris thesame shape of thing — classify an error into a UI-relevant verdict — and
already lives there;
RecordAttachmentsPanelwas already calling it fromthe same import line I'm extending.
@object-ui/reactalready depends on@object-ui/data-objectstack(declared inpackage.json, and already usedby
AppShellContext.tsx), soclassifyLoadErrorcan delegate itsapi-disabledcheck to the already-exportedisApiAccessDeniedErrorthere — removing a second, independently-maintained copy of the same
OBJECT_API_DISABLED/OBJECT_API_METHOD_NOT_ALLOWEDcode list thatListViewused to hand-roll as its own localSet.ListView's own behavior is byte-for-byte unchanged — it now imports thefunction it used to define locally, and its existing
ListView.loadErrorKind.test.tsxsuite (unedited) is the pin for that.Truth table (what I actually ran, not what the card presumed)
All four via
RecordAttachmentsPanel.test.tsx, realdataSource.find()rejections (recorded-double style — no global fetch mock, no swallowed
console error):
PERMISSION_DENIEDdeniedTypeError('Failed to fetch')unavailableOBJECT_API_DISABLEDapi-unavailableOBJECT_API_METHOD_NOT_ALLOWEDapi-unavailablePlus: code-alone classification (no numeric status), no-leak assertions (no
status code / server text in the rendered sentence, matching the sibling
states' bar from issue #2532), and Upload-affordance withdrawal under the new
state.
classifyLoadErroritself gets a direct unit-level pin at its new homein
error-message.test.ts(api-disabled / forbidden / unauthorized /rejected / network, plus the message-embedded-status and 403-over-400
ordering cases already pinned for
ListView).Locales
detail.attachmentsApiUnavailableadded to all ten packs (en,zh,ar,de,es,fr,ja,ko,pt,ru), following the pattern the siblingattachmentsAccessDenied/attachmentsLoadFailedkeys already use in thesame file.
all-locales-key-parity,check-i18n-call-site-keysandcheck-i18n-en-driftall green (output quoted below).Changeset
One changeset,
patchfor all four touched publishable packages(
@object-ui/app-shell,@object-ui/i18n,@object-ui/plugin-list,@object-ui/react) — judged by publish status (none isprivate), not byhow user-visible each individual package's slice looks.
Gates (all at
7e1701bbd, the commit this PR carries)pnpm exec turbo run build --filter='@object-ui/app-shell...' --filter='@object-ui/plugin-list...'— 29/29 tasks successful (full dependency closure: types, core, i18n, data-objectstack, react, components, plugin-list, app-shell, …).pnpm exec turbo run type-check --filter='@object-ui/app-shell' --filter='@object-ui/plugin-list' --filter='@object-ui/react' --filter='@object-ui/i18n' --filter='@object-ui/data-objectstack'— 34/34 successful.pnpm exec turbo run lint --filter='@object-ui/app-shell' --filter='@object-ui/plugin-list' --filter='@object-ui/react' --filter='@object-ui/i18n'— 5/5 successful, 0 errors (pre-existing warnings only, none in the files this PR touches beyond a pre-existingreact-hooks/set-state-in-effectwarning on an untouched line the diff merely shifted).pnpm exec vitest run packages/react/— 593 passed (44 files).pnpm exec vitest run packages/plugin-list/— 549 passed (40 files).pnpm exec vitest run packages/app-shell/src/views/__tests__/— 106 passed (11 files).pnpm exec vitest run packages/i18n/— 804 passed (45 files).node scripts/check-i18n-call-site-keys.mjs— every call-site key resolves, every inline default matches itsenvalue.node scripts/check-i18n-en-drift.mjs— 0envalues changed, 1 key added (parity-covered).node scripts/check-control-bytes.mjs— OK.node scripts/check-phantom-dependencies.mjs— every import declared by the package that publishes it (confirms the "zero new dependency edges" claim above).node scripts/check-changeset-presence.mjs/check-changeset-no-major.mjs/check-changeset-fixed.mjs— all green.node scripts/check-lint-coverage.mjs/check-type-check-coverage.mjs— all green.Generated by Claude Code
Generated by Claude Code