fix(app-shell): unwrap the declared response envelope on the datasource page and the api-action runner (objectstack#3843)

framework#3843 moves four more route modules onto BaseResponseSchema, so
/api/v1/datasources/*, /api/v1/packages/* and /api/settings/* now answer
{ success, data } / { success: false, error: { code, message } }.

Two consumers here were not envelope-aware:

  DatasourceResourcePage — its shared api() helper returned the raw body, so
  all nine payload reads (data.datasources, data.drivers, data.tables,
  { draft }, …) would have gone undefined, and its error path did
  `body.message || body.error` — which stringifies the new nested error object
  to "[object Object]".

  MetadataTypeActions — the generic `type: 'api'` action runner. Same
  "[object Object]" on any nested error, plus a resultDialog would have bound
  its `path` fields against the envelope wrapper instead of the payload.

Both now unwrap `{ success, data }` centrally and read error.message. The
packages readers needed no change: parsePackages, apiJson and
fetchFullPackage were already tolerant (`payload?.data ?? payload`).

Both shapes are accepted on purpose. The framework converts these modules one
at a time (#3675 → #3689 → #3843), so a console build must talk to a backend
on either side of that sequence — the same tolerance the attachment openers
carry for #3689. It is a migration device, not a second contract; the producer
stays the authority.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CYbS3kS8xzsHNXFTzp4e2z

Author: claude

packages/app-shell/src/views/metadata-admin/MetadataTypeActions.test.tsx

@@ -13,6 +13,11 @@ const mockFetch = vi.fn(async () => ({
 }));
 vi.mock('@object-ui/auth', () => ({ createAuthenticatedFetch: () => mockFetch }));
 
+// Toasts are the only observable output of the error path.
+const toastError = vi.fn();
+const toastSuccess = vi.fn();
+vi.mock('sonner', () => ({ toast: { error: (...a: unknown[]) => toastError(...a), success: (...a: unknown[]) => toastSuccess(...a) } }));
+
 // Stub the param dialog: when open, expose a button that resolves with values —
 // lets us drive the collect-params promise without the heavy field renderers.
 vi.mock('../ActionParamDialog', () => ({
@@ -31,7 +36,11 @@ vi.mock('../ActionResultDialog', () => ({
 
 import { MetadataTypeActions } from './MetadataTypeActions';
 
-beforeEach(() => mockFetch.mockClear());
+beforeEach(() => {
+  mockFetch.mockClear();
+  toastError.mockClear();
+  toastSuccess.mockClear();
+});
 
 describe('MetadataTypeActions', () => {
   it('runs an api action without params directly (no dialog)', async () => {
@@ -79,4 +88,72 @@ describe('MetadataTypeActions', () => {
     await waitFor(() => expect(screen.getByTestId('result-dialog')).toBeTruthy());
     expect(screen.getByTestId('result-dialog').textContent).toContain('done');
   });
+
+  // ── framework#3843: the declared `{ success, data }` envelope ──────────────
+  // Service route modules (`/api/v1/datasources`, `/api/v1/packages`,
+  // `/api/settings`, …) answer `BaseResponseSchema`, so an action's payload
+  // arrives under `data`. Both shapes are accepted so this repo is not coupled
+  // to the framework's module-by-module merge order.
+
+  it('unwraps the { success, data } envelope before binding the result dialog', async () => {
+    mockFetch.mockResolvedValueOnce({
+      ok: true,
+      status: 200,
+      statusText: 'OK',
+      json: async () => ({ success: true, data: { message: 'connection ok', latencyMs: 7 } }),
+    } as never);
+    render(
+      <MetadataTypeActions
+        location="record_header"
+        recordId="ds1"
+        entry={{ actions: [{ name: 'probe', label: 'Probe', type: 'api', target: '/api/v1/x', locations: ['record_header'], resultDialog: { fields: [{ path: 'message' }] } }] }}
+      />,
+    );
+    fireEvent.click(screen.getByTitle('Probe'));
+    await waitFor(() => expect(screen.getByTestId('result-dialog')).toBeTruthy());
+    const shown = screen.getByTestId('result-dialog').textContent ?? '';
+    expect(shown).toContain('connection ok');
+    // The wrapper itself must not reach the dialog's `path` bindings.
+    expect(JSON.parse(shown)).toEqual({ message: 'connection ok', latencyMs: 7 });
+  });
+
+  it('reads the failure message out of the nested error object, not "[object Object]"', async () => {
+    mockFetch.mockResolvedValueOnce({
+      ok: false,
+      status: 400,
+      statusText: 'Bad Request',
+      json: async () => ({ success: false, error: { code: 'datasource_admin_error', message: 'duplicate name' } }),
+    } as never);
+    render(
+      <MetadataTypeActions
+        location="record_header"
+        recordId="ds1"
+        entry={{ actions: [{ name: 'probe', label: 'Probe', type: 'api', target: '/api/v1/x', locations: ['record_header'] }] }}
+      />,
+    );
+    fireEvent.click(screen.getByTitle('Probe'));
+    await waitFor(() => expect(toastError).toHaveBeenCalled());
+    const msg = String(toastError.mock.calls[0][0]);
+    expect(msg).toContain('duplicate name');
+    expect(msg).not.toContain('[object Object]');
+  });
+
+  it('still reads a pre-envelope bare-string error — merge order is not a coupling', async () => {
+    mockFetch.mockResolvedValueOnce({
+      ok: false,
+      status: 503,
+      statusText: 'Service Unavailable',
+      json: async () => ({ error: 'datasource_admin_unavailable' }),
+    } as never);
+    render(
+      <MetadataTypeActions
+        location="record_header"
+        recordId="ds1"
+        entry={{ actions: [{ name: 'probe', label: 'Probe', type: 'api', target: '/api/v1/x', locations: ['record_header'] }] }}
+      />,
+    );
+    fireEvent.click(screen.getByTitle('Probe'));
+    await waitFor(() => expect(toastError).toHaveBeenCalled());
+    expect(String(toastError.mock.calls[0][0])).toContain('datasource_admin_unavailable');
+  });
 });

packages/app-shell/src/views/metadata-admin/MetadataTypeActions.tsx

@@ -155,16 +155,39 @@ export function MetadataTypeActions({ entry, location, recordId, onAfter }: Meta
       if (method !== 'GET' && method !== 'DELETE') init.body = JSON.stringify(params);
 
       const res = await authFetch(url, init);
-      let data: Record<string, unknown> | null = null;
+      let body: Record<string, unknown> | null = null;
       try {
-        data = (await res.json()) as Record<string, unknown>;
+        body = (await res.json()) as Record<string, unknown>;
       } catch {
         /* non-JSON / empty body — fall back to status text */
       }
 
-      if (!res.ok || (data && data.success === false)) {
+      // framework#3843: the service route modules now answer the declared
+      // `{ success, data }` envelope (`BaseResponseSchema`), so the payload a
+      // `resultDialog` binds to — and the `message` the success toast reads —
+      // lives under `data` rather than at the top level. Unwrap it here, in the
+      // one place every `type: 'api'` action passes through.
+      //
+      // Deliberately tolerant of an already-unwrapped body: endpoints an action
+      // may target are converted module by module (framework#3675 → #3689 →
+      // #3843), so this repo must not be coupled to the merge order of that
+      // sequence. Same reason the two attachment openers read
+      // `body?.url ?? body?.data?.url` for framework#3689.
+      const data =
+        body && typeof body.success === 'boolean' && 'data' in body
+          ? (body.data as Record<string, unknown> | null)
+          : body;
+
+      if (!res.ok || (body && body.success === false)) {
+        // `error` is `{ code, message }` in the envelope, and was a bare string
+        // before it — read both so a partially-converted backend still explains
+        // itself instead of toasting "[object Object]".
+        const err = body?.error as { message?: string } | string | undefined;
         const detail =
-          (data?.error as string) || (data?.message as string) || `HTTP ${res.status} ${res.statusText}`.trim();
+          (typeof err === 'object' && err !== null ? err.message : undefined) ||
+          (typeof err === 'string' ? err : undefined) ||
+          (body?.message as string) ||
+          `HTTP ${res.status} ${res.statusText}`.trim();
         toast.error(`${action.errorMessage ? `${action.errorMessage}: ` : ''}${title}: ${detail}`);
         return;
       }

packages/app-shell/src/views/metadata-admin/datasource/DatasourceResourcePage.tsx

@@ -172,14 +172,40 @@ export function DatasourceResourcePage(_props: { type?: string }): React.ReactEl
   const [loading, setLoading] = React.useState(true);
   const [busy, setBusy] = React.useState<string | null>(null);
 
+  /**
+   * Fetch + unwrap the declared response envelope.
+   *
+   * framework#3843 moved `/api/v1/datasources/*` onto `BaseResponseSchema`, so
+   * every body is now `{ success, data }` / `{ success: false, error: { code,
+   * message } }`. Unwrapping here keeps all nine call sites below reading their
+   * payload key unchanged (`data.datasources`, `data.drivers`, `data.tables`,
+   * `{ draft }`, …) — the keys did not change, only their depth.
+   *
+   * Both shapes are accepted on purpose, error and success alike. This is the
+   * same tolerance the console already carries for framework#3689 (the
+   * attachment openers' `body?.url ?? body?.data?.url`), and it exists so the
+   * two repos are not coupled by merge order: a console on either side of the
+   * framework change talks to a backend on either side of it. It is a migration
+   * device, not a permanent second contract — the producer is the authority
+   * (framework Prime Directive #12).
+   */
   const api = React.useCallback(
     async (path: string, init?: RequestInit) => {
       const res = await authFetch(`${SERVER}${path}`, { credentials: 'include', headers: { 'Content-Type': 'application/json' }, ...init });
       const text = await res.text();
       let body: any = text;
       try { body = JSON.parse(text); } catch { /* keep text */ }
-      if (!res.ok) throw new Error((body && (body.message || body.error)) || `HTTP ${res.status}`);
-      return body;
+      if (!res.ok) {
+        // `error` is `{ code, message }` post-#3843 and was a bare string
+        // before it; without the first read this threw "[object Object]".
+        const detail =
+          (body && typeof body.error === 'object' && body.error?.message) ||
+          (body && typeof body.error === 'string' ? body.error : undefined) ||
+          (body && body.message) ||
+          `HTTP ${res.status}`;
+        throw new Error(detail);
+      }
+      return body && typeof body.success === 'boolean' && 'data' in body ? body.data : body;
     },
     [authFetch],
   );