RecordAttachmentsPanel renders "No attachments yet" for a network/5xx failure it never loaded

Finding

Found while implementing #4269 (a 403 attachment list rendered as the empty state). That card's ruling scoped the fix to the denied-vs-empty distinction and explicitly left non-authorization failures at their current handling, so this is filed rather than fixed there.

RecordAttachmentsPanel.refresh() catches every failed sys_attachment list read and falls back to setRows([]). After #4269 the authorization branch is split out, but everything else still lands in the empty state:

a network failure (server unreachable, DNS, aborted request)
a 5xx
a 401 (AUTH_REQUIRED — session expired), which is authentication, not authorization, so the denied predicate deliberately does not claim it

All three render "No attachments yet. Upload a file to get started." — an affirmative assertion that the record holds nothing, made by a panel that never got an answer. It is the same defect class as #4269, one status over: the panel states as fact something it does not know.

Why it is a real defect, not polish

The house rule already exists elsewhere in this repo and was landed twice as a bug fix:

HomeActionCenter (The console HOME header renders a second, different inbox polling the wrong source — sys_inbox_message instead of /api/v1/notifications, no badge, "You're all caught up" with 9 unread #4235): "You're all caught up" is an ASSERTION about the inbox, so it may only be made once the inbox has answered — an unanswered read renders a quiet notice instead.
The app list (fix(console): an unloadable app list is UNKNOWN, not "no default app"; wire the Applications page's writes (#4233) #4300): "an unloadable app list is UNKNOWN, not 'no default app'".
ListView.classifyLoadError carries the full five-way vocabulary (api-disabled / forbidden / unauthorized / rejected / network).

The attachments panel is the surface that still lacks it.

Suggested shape

Give the panel the same status vocabulary the siblings use — loading / loaded / denied / unavailable — and reserve the empty state for a genuine 200-with-zero-rows. A failed-to-load state should say the list could not be loaded (and, unlike the denied state, may keep a retry).

Note that #4269's test file already pins TODAY's behaviour for a non-authz failure (pins today's behaviour for a NON-authz failure: still the empty state), deliberately, so that changing it is a conscious edit rather than an accident. That test is the one to rewrite when this is picked up.

Scope note

Touches the same catch block as #4269, so it is best sequenced after that PR lands. #4269 is not addressed by this issue and remains open on its own.

File: packages/app-shell/src/views/RecordAttachmentsPanel.tsx

Filed unassigned from the #4269 dispatch, for PM triage.

First-touch grading (seat repo:objectui; session session_01RnQd8iMMUwXQEV1crFmQiQ, 2026-08-15): promoted — pm:queue, type Bug, size S–M. The direction needs no new ruling: "an unanswered read must not assert emptiness" has landed twice as a bug fix (#4235, #4300) and ListView.classifyLoadError already carries the vocabulary — the eventual dispatch is mechanism-only (give the panel the sibling loading/loaded/denied/unavailable status set; empty reserved for a real 200-with-zero-rows; failed-to-load keeps a retry, unlike denied). Sequenced after PR #4685 lands — same catch block — and the dispatch must rewrite the deliberate pin test #4685 planted (pins today's behaviour for a NON-authz failure), which exists precisely so that change is conscious.

Generated by Claude Code

Claim: PM loop round 9 (sequencing cleared — PR #4685 is MERGED, the shared catch block is free)
Session: session_01RnQd8iMMUwXQEV1crFmQiQ
Branch: claude/issue-4684-attachments-unavailable-state
Worktree: objectui-issue-4684
Domain: repo:objectui
File surface: packages/app-shell/src/views/RecordAttachmentsPanel.tsx (the same refresh() catch #4685 split) + its test file (rewriting #4685's deliberate non-authz pin) + the new unavailable-state i18n key (en + all ten locale packs) (stop on breach; explain in the report)
Container & model: S–M, mode:subagent, model: opus
Serial constraints cleared: #4296 holds plugin-grid; #4687 holds types/field-types.ts; #4672-b holds plugin-charts/core chart-series/DatasetWidget; #8830 is objectstack-side — all disjoint; locale packs free (#4685 merged); no open dev PR touches the attachments panel (checked this round).

Generated by Claude Code

{
  "issue": 4684,
  "status": "done",
  "branch": "claude/issue-4684-attachments-unavailable-state",
  "pr": "https://github.com/objectstack-ai/objectui/pull/4691",
  "premise_still_valid": true,
  "summary": "All three premises verified at source before building. Premise A: on post-#4685 main the refresh() catch splits denied via isPermissionError and folds every other failure into setRows([]). Premise B: isPermissionError matches only 403 / PERMISSION_DENIED / FORBIDDEN / an RLS text denial, so a 401 AUTH_REQUIRED is genuinely not claimed by it, exactly as the issue body said. Premise C: wiring a retry to refresh() cannot double-fetch, because refresh() sets status to 'loading' synchronously and rows is already empty in the unavailable state, so the loading branch wins on the next render and unmounts the only button that calls it. Implemented the sibling four-value vocabulary loading/loaded/denied/unavailable as one enum replacing the loading and listDenied booleans; a non-authz failure now renders a distinct unavailable state with retry and no Upload, leaking nothing from the error; the denied state's JSX is untouched (only its condition changed, so its render is byte-identical) and the empty state is now reachable only with status==='loaded'. #4685's deliberate pin was rewritten in place, not deleted: same TypeError('Failed to fetch') input, now asserting the unavailable state AND that the denied state has not crept outward. Two new i18n keys in all ten packs. One judgement worth the PM's eye: an OBJECT_API_DISABLED / OBJECT_API_METHOD_NOT_ALLOWED refusal folds into unavailable rather than getting a fifth state, so it shows a retry that can never succeed — honest but less precise than ListView's api-disabled kind; filed as #4693 rather than widened here.",
  "tests": "Union run AFTER the final commit, at HEAD 37d57c64c with a clean working tree (git status empty, so the union ran on the tree that is HEAD). Build closure first: pnpm --workspace-concurrency=2 --filter '@object-ui/app-shell^...' build -> exit 0. Panel suite: pnpm exec vitest run packages/app-shell/src/views/__tests__/RecordAttachmentsPanel.test.tsx -> 'Test Files 1 passed (1) / Tests 24 passed (24)'. Full package: pnpm exec vitest run packages/app-shell/ -> 'Test Files 401 passed (401) / Tests 3810 passed | 1 skipped (3811)' (note: pnpm --filter <pkg> test is blocked by a repo guard as false-green-prone; the root form above is the one it names). i18n: pnpm exec vitest run packages/i18n/ -> '45 passed (45) / 804 passed (804)'. type-check for app-shell + i18n -> 'Scope: 2 of 47 workspace projects', both Done — filters matched, not a silent zero-match. eslint --quiet on all 12 changed source files -> clean, exit 0. check:i18n-keys -> exit 0, 'every literal inline defaultValue matches the value the pack serves' (both new keys). check:i18n-drift -> exit 0, '0 en value(s) changed (2 key(s) added)'. check:control-bytes -> OK, 4215 tracked text files. check-changeset-presence.mjs -> OK, '12 source file(s) of 2 released package(s) changed, and this change declares 1 changeset(s)'. REVERSE VERIFICATION, direction predicted before running: reverting only the component (tests kept) should redden exactly the 8 tests requiring record-attachments-unavailable and leave 16 green. Observed exactly that — 'Tests 8 failed | 16 passed (24)', the 8 being the 7 new unavailable cases plus the rewritten pin, while #4685's five denied tests, the empty-state test, the loaded test and the #2755/#2970 suites stayed green. The fix was committed BEFORE the ablation, so restoring was git checkout of the branch ref; git diff HEAD came back empty (byte-identical) and the suite re-ran 24/24 green from that restored state. No ablation on dist/ was involved — this is a source-resolved vitest suite, not a dogfood run.",
  "open_questions": [],
  "out_of_scope_findings": [
    "filed as #4693: RecordAttachmentsPanel offers a Retry for an api-disabled sys_attachment (OBJECT_API_DISABLED / OBJECT_API_METHOD_NOT_ALLOWED), which no retry can fix — observation-class copy/affordance precision, labelled `finding`, no pm:queue; suggests lifting ListView.classifyLoadError out of its module-local scope so both surfaces share one classifier"
  ]
}

Generated by Claude Code

ACCEPT — PR #4691 (reviewer of record; readings taken on GitHub at head 37d57c64c, matching the report's gate-union HEAD, tree clean at union time).

Shape: draft → main, first line Fixes #4684 — correct, full deliverable. No closing keyword near any other card's number (RecordAttachmentsPanel offers a Retry for an api-disabled sys_attachment, which no retry can fix #4693 referenced bare).
Scope (13 changed files): the panel + its test + two i18n keys × ten packs + changeset — the claimed surface exactly.
Direction conformance: the four-way enum replaces both booleans, and the render chain enforces the assertion discipline — the empty state is reachable ONLY through loaded. The denied/unavailable affordance split is reasoned correctly (retry fixes outages and lapsed sessions, never a 403; Upload withdrawn under both because the presigned flow fails on the same outage). The 401 case — the sharpest one — verified at the predicate source rather than assumed.
Pin discipline exactly right: fix(app-shell): a denied attachment list is not an empty one #4685's deliberate pin was rewritten in place as a stricter assertion (same input, now asserting unavailable AND that denied has not crept outward to claim outages) — what the pin protected survives, stronger.
Edges handled honestly: the A list request that 404s with OBJECT_API_DISABLED renders as the generic empty state — "this page cannot work" reads as "you have no records" #4408 bare-404 degradation still resolves through the success path (verified, not assumed); the no-dataSource mount stays loading rather than asserting anything from zero evidence; the api-disabled fold into unavailable is named as a precision loss and filed (RecordAttachmentsPanel offers a Retry for an api-disabled sys_attachment, which no retry can fix #4693) instead of growing a fifth state here.
Classifier reuse measured: classifyLoadError is module-local and unimportable — the reuse verdict is a reading of the source, not a preference; no third classifier written.
Evidence: 24/24 panel + 401 files full app-shell + i18n parity gates green (2 keys added, 0 drift); reverse verification predicted 8 red / 16 green — observed exactly, with fix(app-shell): a denied attachment list is not an empty one #4685's five denied tests among the survivors, proving the byte-identical claim about the denied branch.

CI on this head is converging — ready + auto-merge flips when the gates conclude green (folded into the standing timer round). Card closes on merge; pm:dispatched comes off then. #4693 graded on its own card.

Generated by Claude Code

Navigation Menu

Repository navigation

RecordAttachmentsPanel renders "No attachments yet" for a network/5xx failure it never loaded #4684

Finding

Why it is a real defect, not polish

Suggested shape

Scope note

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

RecordAttachmentsPanel renders "No attachments yet" for a network/5xx failure it never loaded #4684

Description

Finding

Why it is a real defect, not polish

Suggested shape

Scope note

Activity

yinlianghui commented on Aug 15, 2026

yinlianghui commented on Aug 15, 2026

yinlianghui commented on Aug 15, 2026

yinlianghui commented on Aug 15, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions