fix(attachments): read the storage service's new error envelope so gated downloads keep their friendly copy (objectstack#3675) (#2869)

`RecordAttachmentsPanel` maps the server's fail-closed 40x codes
(`AUTH_REQUIRED`, `ATTACHMENT_DOWNLOAD_DENIED`) to human copy by reading `code`
off the error body. It read the TOP LEVEL only. The storage service has moved
that code into the envelope its contract declares — `{ success: false, error:
{ code, message } }` — so the top-level read now returns `undefined` and every
gated download would have degraded from "You don't have access to download this
attachment." to the generic "Download failed (403)".

The download handler now reads `body?.error?.code ?? body?.code`, mirroring how
the success branch two lines below already reads `body?.url ?? body?.data?.url`.
Both dialects on purpose: the console ships independently of the server it
talks to, so a current console must keep understanding an older one — tolerance
is the right answer here, not a synchronized cutover.

The two existing tests now mock what the server actually sends, and a third
covers the legacy shape so the tolerance is an asserted property rather than an
accident. Mutation-checked: dropping the nested read fails the two
new-envelope cases.

The upload path needed no change — it stringifies the whole body into the error
message, so its substring match survives either shape.

Co-authored-by: Claude <noreply@anthropic.com>

Author: os-zhuang

.changeset/attachment-error-envelope.md

@@ -0,0 +1,21 @@
+---
+"@object-ui/app-shell": patch
+---
+
+fix(attachments): read the storage service's new error envelope so gated downloads keep their friendly copy (objectstack#3675)
+
+`RecordAttachmentsPanel` mapped the server's fail-closed 40x codes
+(`AUTH_REQUIRED`, `ATTACHMENT_DOWNLOAD_DENIED`) to human copy by reading
+`code` off the top level of the error body. The storage service has moved that
+code into the envelope its contract declares —
+`{ success: false, error: { code, message } }` — so the top-level read now
+returns `undefined`, and every gated download would have degraded from
+"You don't have access to download this attachment." to the generic
+"Download failed (403)".
+
+The download handler now reads `body?.error?.code ?? body?.code`, mirroring how
+the success branch two lines below already reads `body?.url ?? body?.data?.url`.
+Both dialects on purpose: the console ships independently of the server it
+talks to, so a current console must keep understanding an older one. A test
+covers each shape, and the fix is mutation-checked — dropping the nested read
+fails the two new-envelope cases.

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

@@ -211,7 +211,17 @@ export const RecordAttachmentsPanel: React.FC<RecordAttachmentsPanelProps> = ({
         if (!res.ok) {
           let code: string | undefined;
           try {
-            code = (await res.json())?.code;
+            // Read BOTH error dialects, the same way the success branch below
+            // reads both `url` shapes. The storage service moved its error
+            // code from a sibling of `error` into the declared
+            // `{ success: false, error: { code, message } }` envelope
+            // (objectstack#3675); a console build is deployed independently of
+            // the server it talks to, so it has to keep understanding the
+            // older top-level `code` too. Without the nested branch the 401/403
+            // downgrade silently to "Download failed (403)" and the friendly
+            // copy below never fires.
+            const body = (await res.json()) as { code?: string; error?: { code?: string } } | null;
+            code = body?.error?.code ?? body?.code;
           } catch {
             /* non-JSON body */
           }

packages/app-shell/src/views/__tests__/RecordAttachmentsPanel.test.tsx

@@ -124,10 +124,13 @@ describe('RecordAttachmentsPanel — authenticated signed-URL download (#2970)',
   it('maps a 403 ATTACHMENT_DOWNLOAD_DENIED to friendly copy and does not open a tab', async () => {
     const openSpy = vi.spyOn(window, 'open').mockImplementation(() => null);
     vi.spyOn(globalThis, 'fetch').mockResolvedValue(
-      new Response(JSON.stringify({ code: 'ATTACHMENT_DOWNLOAD_DENIED' }), {
-        status: 403,
-        headers: { 'Content-Type': 'application/json' },
-      }),
+      new Response(
+        JSON.stringify({
+          success: false,
+          error: { code: 'ATTACHMENT_DOWNLOAD_DENIED', message: 'You do not have access…' },
+        }),
+        { status: 403, headers: { 'Content-Type': 'application/json' } },
+      ),
     );
     setup(makeDataSource());
     await waitFor(() => expect(screen.getByText('report.pdf')).toBeInTheDocument());
@@ -145,8 +148,33 @@ describe('RecordAttachmentsPanel — authenticated signed-URL download (#2970)',
   it('maps a 401 AUTH_REQUIRED to friendly copy', async () => {
     vi.spyOn(window, 'open').mockImplementation(() => null);
     vi.spyOn(globalThis, 'fetch').mockResolvedValue(
-      new Response(JSON.stringify({ code: 'AUTH_REQUIRED' }), {
-        status: 401,
+      new Response(
+        JSON.stringify({
+          success: false,
+          error: { code: 'AUTH_REQUIRED', message: 'Authentication required to download this file' },
+        }),
+        { status: 401, headers: { 'Content-Type': 'application/json' } },
+      ),
+    );
+    setup(makeDataSource());
+    await waitFor(() => expect(screen.getByText('report.pdf')).toBeInTheDocument());
+
+    await userEvent.setup().click(screen.getByRole('button', { name: 'Download' }));
+
+    await waitFor(() =>
+      expect(screen.getByRole('alert')).toHaveTextContent('Please sign in to download this attachment.'),
+    );
+  });
+
+  // The console ships independently of the server it talks to, so it must keep
+  // reading the PRE-objectstack#3675 body — code as a sibling of `error`, not a
+  // field of it. Without this, pointing a new console at an older server turns
+  // every gated download into the generic "Download failed (403)".
+  it('still maps the legacy top-level `code` shape (older server)', async () => {
+    const openSpy = vi.spyOn(window, 'open').mockImplementation(() => null);
+    vi.spyOn(globalThis, 'fetch').mockResolvedValue(
+      new Response(JSON.stringify({ error: 'You do not have access…', code: 'ATTACHMENT_DOWNLOAD_DENIED' }), {
+        status: 403,
         headers: { 'Content-Type': 'application/json' },
       }),
     );
@@ -156,7 +184,10 @@ describe('RecordAttachmentsPanel — authenticated signed-URL download (#2970)',
     await userEvent.setup().click(screen.getByRole('button', { name: 'Download' }));
 
     await waitFor(() =>
-      expect(screen.getByRole('alert')).toHaveTextContent('Please sign in to download this attachment.'),
+      expect(screen.getByRole('alert')).toHaveTextContent(
+        "You don't have access to download this attachment.",
+      ),
     );
+    expect(openSpy).not.toHaveBeenCalled();
   });
 });