fix(service-storage,service-i18n): emit the declared error envelope, not a bare { error } (#3675)

#3636 aligned the SUCCESS bodies of the autonomously-mounted service routes,
because those were the ones breaking `ObjectStackClient.unwrapResponse`. The
error bodies were left alone and stayed a bare `{ error: '<message>' }` — with
the code, where one existed at all, as a SIBLING of `error` rather than a field
of it — against a contract (`BaseResponseSchema` + `ApiErrorSchema`) declaring
`{ success: false, error: { code, message } }`. So the same SDK method returned
two different error shapes depending on which provider mounted the route.

All 32 sites (27 in `storage-routes.ts`, 5 in `i18n-service-plugin.ts`) now go
through one `sendError` helper per module: the nested-`error` shape the sibling
services already use (`settings-routes.ts`, `share-link-routes.ts`), plus the
`success` flag those two still omit and the contract requires.

The codes moved, and that is the breaking part. `AUTH_REQUIRED`,
`ATTACHMENT_DOWNLOAD_DENIED` and `FILE_DOWNLOAD_DENIED` were at `body.code` and
are now at `body.error.code`. The SDK is unaffected — it already reads
`errorBody?.code || errorBody?.error?.code`, one of four shapes its error path
sniffs for, which is the consumer-side shim Prime Directive #12 says to cure at
the producer. The console's attachment panel was NOT: it read the top level
only, so every gated download would have degraded to "Download failed (403)".
Fixed in objectui to read both dialects, since a console build ships
independently of the server it talks to.

Guarded both ways. A new `error-envelope.conformance.test.ts` in each service
drives every distinct error branch through the real registrar and parses the
body against the real `BaseResponseSchema` imported from `packages/spec` — not
a local restatement that could drift from the schema it claims to check — and
scans the module source so a new route cannot quietly reintroduce the bare
shape. Without the second half the suite would only ever cover the branches
that existed the day it was written. Both halves are mutation-checked.

The route ledgers (#3563 → #3656) could not have caught this: they audit which
routes exist and whether the SDK can address them, not what comes back.
Recorded as §12 of the audit doc, with the four error dialects found live.

Measured and deliberately left alone: the dispatcher does not conform either —
it puts the HTTP status in `error.code` where the contract declares a semantic
string, and parks the real code in `details` to work around its own occupied
field. Now pinned to exactly one deviating field by a test rather than prose.
Also unchanged: service-storage's success bodies are still three shapes of
their own, none carrying `success: true` — non-additive, so it gets its own
issue instead of a quiet ride along with this one.

Co-Authored-By: Claude <noreply@anthropic.com>

Author: claude

.changeset/service-error-envelope-conformance.md

@@ -0,0 +1,50 @@
+---
+"@objectstack/service-storage": patch
+"@objectstack/service-i18n": patch
+---
+
+fix(service-storage,service-i18n): emit the declared error envelope, not a bare `{ error }` (#3675)
+
+#3636 aligned the **success** bodies of the autonomously-mounted service
+routes because those were the ones breaking `ObjectStackClient.unwrapResponse`.
+The error bodies were left alone and stayed a bare `{ error: '<message>' }` —
+with the code, where one existed at all, as a *sibling* of `error` rather than
+a field of it — against a contract (`BaseResponseSchema` + `ApiErrorSchema`)
+that declares `{ success: false, error: { code, message } }`.
+
+So the same SDK method returned two different error shapes depending on which
+provider mounted the route: a caller reading `body.error.message` got the real
+message from the dispatcher and `undefined` from these services. All 32 sites
+(27 in `storage-routes.ts`, 5 in `i18n-service-plugin.ts`) now go through a
+single `sendError` helper per module — the nested-`error` shape the sibling
+services already use (`settings-routes.ts`, `share-link-routes.ts`), plus the
+`success` flag those two still omit and the contract requires.
+
+**Codes moved, and that is the breaking part.** `AUTH_REQUIRED`,
+`ATTACHMENT_DOWNLOAD_DENIED` and `FILE_DOWNLOAD_DENIED` used to sit at
+`body.code`; they now sit at `body.error.code`. The SDK is unaffected — it
+already reads `errorBody?.code || errorBody?.error?.code`, one of the four
+shapes its error path sniffs for, which is the consumer-side shim Prime
+Directive #12 says to cure at the producer. The console's attachment panel
+was NOT: it read the top level only, so every gated download would have
+degraded from "You don't have access to download this attachment." to
+"Download failed (403)". Fixed in objectui to read both dialects, since a
+console build ships independently of the server it talks to.
+
+**Guarded both ways.** New `error-envelope.conformance.test.ts` in each
+service drives every distinct error branch through the real registrar and
+parses the body against the real `BaseResponseSchema` imported from
+`packages/spec` — not a local restatement of it — and scans the module source
+so a new route cannot quietly reintroduce the bare shape. The route ledgers
+(#3563 → #3656) could never have caught this: they audit which routes exist
+and whether the SDK can address them, not what comes back.
+
+Measured and left alone: the dispatcher does not conform either — it puts the
+HTTP status in `error.code`, where the contract declares a semantic string,
+and parks the real code in `details` to work around its own occupied field.
+That deviation is now pinned to exactly one field by a test in
+`http-dispatcher.test.ts` rather than described in prose. Also unchanged:
+service-storage's success bodies are still three shapes of their own
+(`{ data }`, bare `{ url }`, `{ ok, key }`, none with `success: true`) — a
+non-additive change that needs its own issue, not a quiet ride along with this
+one.

docs/audits/2026-07-dispatcher-client-route-coverage.md

@@ -271,6 +271,37 @@ capstone also had to prefer exact rows over wildcard families when matching —
 otherwise every `/auth/*` URL would still have been absorbed by `* /auth/**`
 and the new ledger would have changed nothing.
 
+## 12. What the ledgers cannot see: response bodies (#3675)
+
+Worth recording as a **boundary of this audit family**, not an oversight in it.
+Every guard from §1 to §11 answers one question — does this route exist, and
+can the SDK address it? None of them looks at what comes back. That is how
+service-i18n and service-storage carried green `sdk` rows for surfaces that
+emitted a bare `{ error: '<message>' }` against a contract declaring
+`{ success: false, error: { code, message } }`.
+
+Four error dialects were live in-repo at the time:
+
+| Producer | Shape |
+|---|---|
+| `contract.zod.ts` (declared) | `{ success, error: { code: string, message, … } }` |
+| `http-dispatcher.ts` | `{ success: false, error: { message, code: <HTTP number>, details } }` |
+| `rest-server.ts` | `{ error: <string>, code: <SEMANTIC_STRING> }` |
+| `settings-routes.ts`, `share-link-routes.ts` | `{ error: { code, message } }` — no `success` |
+| service-i18n, service-storage | `{ error: <string> }`, sometimes `+ code` at the top level |
+
+#3675 moved the last row to the contract. The rest are unchanged, and the
+dispatcher's deviation is now pinned to exactly one field (`error.code` carries
+the HTTP status where a semantic string is declared) by a test rather than by
+prose — it parks the real code in `details` to work around its own occupied
+field, which is the tell.
+
+The generalisable lesson matches §11's: a guard only covers the question it
+asks. "The route exists" and "the route answers in the declared shape" are two
+questions, and the second one needs the contract imported into the assertion —
+`BaseResponseSchema.safeParse(body)`, not a hand-copied restatement that drifts
+from the schema it claims to check.
+
 ## Follow-up slicing (proposed)
 
 1. **`client.actions.invoke(...)`** — closes the largest hole (3 routes).
@@ -286,6 +317,9 @@ and the new ledger would have changed nothing.
 11. **Control-plane surface** (§10) — #3655, needs a ledger in the `cloud` repo.
 12. **Enumerate `/auth/**`** (§11) — done in #3656; wildcard ratchet 60 → 3.
 13. **Enumerate `/ai/**`** — the last dynamic family, 3 SDK methods.
+14. **Response-shape conformance** (§12) — error path done in #3675 for both
+    services; the storage **success** bodies (three shapes, none carrying
+    `success: true`) and the dispatcher's numeric `error.code` remain.
 
 Each gap closed must flip its ledger row to `sdk` and lower the ratchet bound
 in the conformance test — the guard enforces both directions from PR-1 onward.

packages/qa/dogfood/test/attachments-permission-matrix.dogfood.test.ts

@@ -157,7 +157,7 @@ describe('attachments permission matrix (#2755)', () => {
       body: JSON.stringify({ filename: 'x.txt', mimeType: 'text/plain', size: 1, scope: 'attachments' }),
     });
     expect(res.status).toBe(401);
-    expect(((await res.json()) as any).code).toBe('AUTH_REQUIRED');
+    expect(((await res.json()) as any).error?.code).toBe('AUTH_REQUIRED');
   });
 
   it('(e) authenticated upload succeeds and sys_file.owner_id is server-stamped', async () => {
@@ -319,13 +319,13 @@ describe('attachments permission matrix (#2755)', () => {
     // Anonymous → 401 (was a 200 capability URL before #2970).
     const anon = await stack.api(`/storage/files/${adminFile}/url`);
     expect(anon.status).toBe(401);
-    expect(((await anon.json()) as any).code).toBe('AUTH_REQUIRED');
+    expect(((await anon.json()) as any).error?.code).toBe('AUTH_REQUIRED');
 
     // memberB is authenticated but cannot read att_secret and is not the
     // owner → 403.
     const denied = await stack.apiAs(memberBTok, 'GET', `/storage/files/${adminFile}/url`);
     expect(denied.status).toBe(403);
-    expect(((await denied.json()) as any).code).toBe('ATTACHMENT_DOWNLOAD_DENIED');
+    expect(((await denied.json()) as any).error?.code).toBe('ATTACHMENT_DOWNLOAD_DENIED');
 
     // The owner (admin) → 200 with a signed URL.
     const owner = await stack.apiAs(adminTok, 'GET', `/storage/files/${adminFile}/url`);

packages/runtime/src/http-dispatcher.test.ts

@@ -2,6 +2,7 @@
 import { describe, it, expect, vi, beforeEach } from 'vitest';
 import { HttpDispatcher } from './http-dispatcher.js';
 import { ObjectKernel } from '@objectstack/core';
+import { ApiErrorSchema } from '@objectstack/spec/api';
 
 describe('HttpDispatcher', () => {
     let kernel: ObjectKernel;
@@ -1837,6 +1838,35 @@ describe('HttpDispatcher', () => {
             expect(result.response?.body?.error?.message).toBe('Missing locale parameter');
         });
 
+        /**
+         * The dispatcher's error body is the SHAPE the autonomously-mounted
+         * i18n/storage services were aligned to in #3675 — nested `error`, with
+         * the `success` flag. It is not yet the CONTRACT: `ApiErrorSchema`
+         * declares `code` as a semantic string ('validation_error'), and this
+         * emits the HTTP status as a number. The dispatcher already works
+         * around its own field being occupied — `this.error(msg, 403, { code:
+         * 'PERMISSION_DENIED' })` parks the real code in `details` — which is
+         * the tell that the number is in the wrong place.
+         *
+         * Pinned rather than fixed: `error.code` is read across the SDK, the
+         * console and the dogfood suite, so moving it is its own change.
+         * `toEqual(['code'])` is deliberately exact — if a second field starts
+         * deviating this fails, and if the dispatcher is fixed it also fails
+         * and this pin should be deleted.
+         */
+        it('pins the ONE field where the dispatcher deviates from ApiErrorSchema (#3675)', async () => {
+            const result = await dispatcher.handleI18n('/translations', 'GET', {}, { request: {} });
+            const body = result.response?.body as { success?: boolean; error?: unknown };
+
+            expect(body.success).toBe(false);
+            expect(typeof body.error).toBe('object');
+
+            const parsed = ApiErrorSchema.safeParse(body.error);
+            expect(parsed.success).toBe(false);
+            expect(parsed.error!.issues.map((i) => i.path.join('.'))).toEqual(['code']);
+            expect((body.error as { code: unknown }).code).toBe(400);
+        });
+
         it('should fallback to deriving labels from translations when getFieldLabels is missing', async () => {
             delete mockI18nService.getFieldLabels;
             mockI18nService.getTranslations.mockReturnValue({

packages/services/service-i18n/src/error-envelope.conformance.test.ts

@@ -0,0 +1,192 @@
+// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * Error-envelope conformance for the autonomously-mounted `/api/v1/i18n/*`
+ * routes (#3675) — the error-path twin of the success-path fix in #3636.
+ *
+ * #3636 aligned the SUCCESS bodies because those were the ones breaking
+ * `ObjectStackClient.unwrapResponse`. The error bodies stayed a bare
+ * `{ error: '<message>' }` while the dispatcher's `/i18n` domain — the OTHER
+ * provider of these same three shapes — emitted
+ * `{ success: false, error: { … } }` for the identical condition. Same SDK
+ * method, two error shapes, decided by which plugin mounted the route.
+ *
+ * Both directions are guarded: every error branch is driven and parsed against
+ * the real `BaseResponseSchema`, and the module source is scanned so a new
+ * route cannot quietly reintroduce the bare shape.
+ */
+
+import { describe, it, expect, vi } from 'vitest';
+import { readFileSync } from 'node:fs';
+import { BaseResponseSchema } from '@objectstack/spec/api';
+import type { IHttpRequest, IHttpResponse, RouteHandler } from '@objectstack/spec/contracts';
+import { I18nServicePlugin } from './i18n-service-plugin';
+
+const BASE = '/api/v1/i18n';
+
+/**
+ * Mount the plugin's routes through its REAL lifecycle and hand back the
+ * handlers, so these assertions run against the routes the plugin actually
+ * registers rather than a hand-copied table.
+ */
+async function mount(i18nOverrides: Record<string, unknown> = {}) {
+  const routes = new Map<string, RouteHandler>();
+  const server = {
+    get: (p: string, h: RouteHandler) => { routes.set(`GET:${p}`, h); },
+    post: vi.fn(), put: vi.fn(), delete: vi.fn(), patch: vi.fn(), use: vi.fn(),
+    listen: vi.fn().mockResolvedValue(undefined),
+    close: vi.fn().mockResolvedValue(undefined),
+  };
+  const hooks = new Map<string, Array<(...a: unknown[]) => Promise<void>>>();
+  const services = new Map<string, unknown>();
+  const ctx = {
+    registerService: vi.fn((name: string, svc: unknown) => { services.set(name, svc); }),
+    replaceService: vi.fn(),
+    getService: vi.fn((name: string) => {
+      if (name === 'http-server') return server;
+      if (services.has(name)) return services.get(name);
+      throw new Error(`Service '${name}' not found`);
+    }),
+    getServices: vi.fn(() => new Map()),
+    getKernel: vi.fn(),
+    hook: vi.fn((name: string, h: (...a: unknown[]) => Promise<void>) => {
+      if (!hooks.has(name)) hooks.set(name, []);
+      hooks.get(name)!.push(h);
+    }),
+    trigger: vi.fn(async (name: string, ...a: unknown[]) => {
+      for (const h of hooks.get(name) ?? []) await h(...a);
+    }),
+    logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() },
+  };
+
+  const plugin = new I18nServicePlugin();
+  await plugin.init!(ctx as never);
+  // Sabotage the registered service where a test needs the `catch` arm — the
+  // routes close over whatever `init` registered.
+  const svc = services.get('i18n') as Record<string, unknown> | undefined;
+  if (svc) Object.assign(svc, i18nOverrides);
+  await plugin.start!(ctx as never);
+  await ctx.trigger('kernel:ready');
+  return routes;
+}
+
+async function drive(
+  routes: Map<string, RouteHandler>,
+  path: string,
+  req: Partial<IHttpRequest> = {},
+): Promise<{ status: number; body: any }> {
+  const handler = routes.get(`GET:${path}`);
+  if (!handler) throw new Error(`no handler for GET ${path} (have: ${[...routes.keys()].join(', ')})`);
+  const captured = { status: 200, body: undefined as any };
+  const res: any = {
+    json(data: any) { captured.body = data; },
+    send() {},
+    status(code: number) { captured.status = code; return res; },
+    header() { return res; },
+  };
+  await handler(
+    { params: {}, query: {}, body: undefined, headers: {}, method: 'GET', path, ...req } as IHttpRequest,
+    res as IHttpResponse,
+  );
+  return captured;
+}
+
+describe('i18n error envelope (#3675)', () => {
+  const CASES: Array<{
+    name: string;
+    status: number;
+    code: string;
+    run: () => Promise<{ status: number; body: any }>;
+  }> = [
+    {
+      name: 'translations without a locale',
+      status: 400,
+      code: 'INVALID_REQUEST',
+      run: async () => drive(await mount(), `${BASE}/translations/:locale`, { params: {} }),
+    },
+    {
+      name: 'labels without an object or locale',
+      status: 400,
+      code: 'INVALID_REQUEST',
+      run: async () => drive(await mount(), `${BASE}/labels/:object/:locale`, { params: {} }),
+    },
+    {
+      name: 'a throwing i18n service on /locales',
+      status: 500,
+      code: 'INTERNAL',
+      run: async () =>
+        drive(
+          await mount({ getLocales: () => { throw new Error('adapter exploded'); } }),
+          `${BASE}/locales`,
+        ),
+    },
+    {
+      name: 'a throwing i18n service on /translations/:locale',
+      status: 500,
+      code: 'INTERNAL',
+      run: async () =>
+        drive(
+          await mount({ getTranslations: () => { throw new Error('adapter exploded'); } }),
+          `${BASE}/translations/:locale`,
+          { params: { locale: 'en' } },
+        ),
+    },
+    {
+      name: 'a throwing i18n service on /labels/:object/:locale',
+      status: 500,
+      code: 'INTERNAL',
+      run: async () =>
+        drive(
+          await mount({
+            getFieldLabels: () => { throw new Error('adapter exploded'); },
+            getTranslations: () => { throw new Error('adapter exploded'); },
+          }),
+          `${BASE}/labels/:object/:locale`,
+          { params: { object: 'customer', locale: 'en' } },
+        ),
+    },
+  ];
+
+  for (const c of CASES) {
+    it(`${c.name} → ${c.status} ${c.code}, in the declared envelope`, async () => {
+      const { status, body } = await c.run();
+      expect(status).toBe(c.status);
+
+      const parsed = BaseResponseSchema.safeParse(body);
+      expect(parsed.success, `body is not a BaseResponse: ${JSON.stringify(body)}`).toBe(true);
+
+      expect(body.success).toBe(false);
+      expect(body.error.code).toBe(c.code);
+      expect(typeof body.error.message).toBe('string');
+      expect(body.error.message.length).toBeGreaterThan(0);
+
+      // The pre-#3675 shape, explicitly dead.
+      expect(typeof body.error).not.toBe('string');
+    });
+  }
+
+  it('a 500 still carries a message when the thrown value has none', async () => {
+    // `error.message` is REQUIRED by ApiErrorSchema, so passing a raw thrown
+    // value straight through would emit an invalid body for anything that is
+    // not an Error.
+    const routes = await mount({ getLocales: () => { throw 'a bare string'; } });
+    const { status, body } = await drive(routes, `${BASE}/locales`);
+    expect(status).toBe(500);
+    expect(BaseResponseSchema.safeParse(body).success).toBe(true);
+    expect(body.error.message).toBe('Internal error');
+  });
+
+  it('routes every error through `sendError` — no route may reintroduce the bare shape', () => {
+    // Comments stripped first: this module's prose quotes both shapes, and a
+    // doc comment is not a code path.
+    const source = readFileSync(new URL('./i18n-service-plugin.ts', import.meta.url), 'utf8')
+      .replace(/\/\*[\s\S]*?\*\//g, '')
+      .replace(/\/\/[^\n]*/g, '');
+
+    const bare = [...source.matchAll(/res\s*\.\s*status\([^)]*\)\s*\.\s*json\(\s*\{\s*error/g)];
+    expect(bare, `bare error bodies found: ${bare.map((m) => m[0]).join(', ')}`).toHaveLength(0);
+
+    // The envelope is built in exactly one place.
+    expect([...source.matchAll(/success:\s*false/g)]).toHaveLength(1);
+  });
+});

packages/services/service-i18n/src/i18n-service-plugin.test.ts

@@ -234,7 +234,10 @@ describe('I18nServicePlugin', () => {
       await handler(req, res);
 
       expect(res._status).toBe(400);
-      expect(res._data).toEqual({ error: 'Missing locale parameter' });
+      expect(res._data).toEqual({
+        success: false,
+        error: { code: 'INVALID_REQUEST', message: 'Missing locale parameter' },
+      });
     });
 
     it('GET /labels/:object/:locale should derive field labels from translation bundle', async () => {
@@ -266,7 +269,10 @@ describe('I18nServicePlugin', () => {
       await handler(req, res);
 
       expect(res._status).toBe(400);
-      expect(res._data).toEqual({ error: 'Missing object or locale parameter' });
+      expect(res._data).toEqual({
+        success: false,
+        error: { code: 'INVALID_REQUEST', message: 'Missing object or locale parameter' },
+      });
     });
   });