test(spec): envelopeViolations — the conformance check BaseResponseSchema cannot express (#4090)

* test(spec): envelopeViolations — the conformance check BaseResponseSchema cannot express

Every conformance suite from the #3843 line leads with
`BaseResponseSchema.safeParse(body)` under a comment calling it "the contract
itself, imported — not a restatement of it". That overclaimed, and the gap is
demonstrable:

  BaseResponseSchema.safeParse({ success: true })                    // passes
  BaseResponseSchema.safeParse({ success: true, data: link, link })  // passes

The schema declares no `data` (each response type adds its own via
`.extend({ data })`) and a plain z.object strips unknown keys rather than
rejecting them. So it catches the one drift it was added for — a missing or
non-boolean `success`, the flag `unwrapResponse` keys on — and nothing else. The
second body is exactly the duplicate-payload drift /share-links shipped until
#4038/#4049 removed it, and safeParse passed it the whole time. What was actually
catching that drift was the assertions each suite hand-wrote beside it, which
works only for as long as whoever writes the next suite remembers to.

`envelopeViolations(body)` returns every departure as readable reasons, empty when
conformant: `success` must be a boolean; a success body must carry `data`
(`undefined` only — null/[]/{}/0/'' are payloads, not absences); a failure body
must carry `error` with a string code and message; and no top-level key outside
success/data/error/meta, which is the general form of the duplicate-payload drift.
It deliberately does NOT check the shape of `data` — that is each route's own
payload schema, and conflating the two is what let SettingsNamespacePayload
describe a whole body before #3843 and only `data` after it.

Ten suites now assert it beside safeParse, and their comments say what each of the
two actually proves. Verified the pairing is load-bearing: reintroducing the
/share-links duplicate key fails the new assertion and still passes the old one.
Verified the rules are not too strict by running every existing suite unchanged —
spec 6951, rest 505, runtime 914, storage 220, settings 189, datasource 154,
i18n 62, sharing 225, client 200, all green, nothing flagged.

Placed in contract.zod.ts beside the schema it completes, alongside the other
plain predicates spec/api already exports (standardErrorCodeForHttpStatus,
readServiceSelfInfo). No new package.

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

* chore(spec): regenerate the API-surface snapshot for envelopeViolations

`check:api-surface` is a ratchet on `@objectstack/spec`'s public exports, and the
new predicate is an addition to it:

    ./api
      + envelopeViolations (function)
    0 breaking (removed/narrowed), 1 added.

One line, alphabetically beside the other predicates that module already exports.

Two things this caught that are worth writing down.

The gate lives in the TypeScript Type Check job, which runs TEN steps — I had
found four of them by grepping a window of lint.yml that cut off before the rest,
and ran only those. The full list is: spec tsc, check:docs, check:skill-refs,
check:react-blocks, a workspace build, examples typecheck, downstream-contract
typecheck, check:api-surface, check:skill-examples, and the two i18n checks. All
ten pass locally now.

And `gen:api-surface` reads `dist/index.d.ts`, so it fails outright under
`OS_SKIP_DTS=1` — the flag I had been using to keep local builds fast. Rebuilding
spec with declarations is a prerequisite for regenerating the snapshot, not an
optional speed-up.

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

---------

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

Author: os-zhuang

.changeset/envelope-violations-predicate.md

@@ -0,0 +1,46 @@
+---
+"@objectstack/spec": minor
+---
+
+**`envelopeViolations` — the conformance check `BaseResponseSchema` cannot express.**
+
+Every conformance suite from the #3843 line leads with
+`BaseResponseSchema.safeParse(body)`, under a comment claiming it is "the contract
+itself, imported — not a restatement of it". That overclaimed, and the gap is
+demonstrable:
+
+```ts
+BaseResponseSchema.safeParse({ success: true })                        // passes
+BaseResponseSchema.safeParse({ success: true, data: link, link })      // passes
+```
+
+The schema declares no `data` — each response type adds its own via
+`.extend({ data })` — and a plain `z.object` strips unknown keys rather than
+rejecting them. So it catches the one drift it was added for (a missing or
+non-boolean `success`, the flag `unwrapResponse` keys on) and nothing else. The
+second body above is exactly the duplicate-payload drift `/share-links` shipped
+until #4038 / #4049 removed it, and `safeParse` passed it the whole time.
+
+`envelopeViolations(body)` returns every departure from the declared envelope as
+readable reasons, empty when conformant:
+
+- `success` must be a **boolean**
+- a success body must carry `data` (`undefined` only — `null`, `[]`, `{}`, `0`
+  and `''` are payloads, not absences)
+- a failure body must carry `error` with a string `code` and `message` — the
+  nested form, not the pre-#3675 bare string
+- no top-level key outside `success` / `data` / `error` / `meta`, which is the
+  general form of the duplicate-payload drift
+
+It deliberately does **not** check the shape of `data`: that is each route's own
+payload schema, and conflating the two is what let `SettingsNamespacePayload`
+describe a whole body before #3843 and only `data` after it.
+
+The ten conformance suites now assert it beside `safeParse`, and their comments
+say what each of the two actually proves. Reintroducing the `/share-links`
+duplicate key is caught by the new assertion and still passes the old one — which
+is the demonstration that the pairing is the point.
+
+Placed in `contract.zod.ts` beside the schema it completes, alongside the other
+plain predicates `spec/api` already exports (`standardErrorCodeForHttpStatus`,
+`readServiceSelfInfo`). No new package.

packages/plugins/plugin-sharing/src/share-link-envelope.conformance.test.ts

@@ -34,7 +34,7 @@
  */
 
 import { describe, expect, it, vi } from 'vitest';
-import { BaseResponseSchema } from '@objectstack/spec/api';
+import { BaseResponseSchema, envelopeViolations } from '@objectstack/spec/api';
 import type { IHttpServer, IHttpRequest, IHttpResponse, RouteHandler } from '@objectstack/spec/contracts';
 import { registerShareLinkRoutes } from './share-link-routes';
 import type { ShareLinkService } from './share-link-service';
@@ -214,9 +214,15 @@ describe('share-link envelope (#3983) — success bodies', () => {
       const { status, body } = await c.run();
       expect(status).toBe(c.status);
 
-      // The contract itself, imported — not a restatement of it.
+      // The envelope SKELETON, imported. It is not the whole contract: it declares
+      // no `data` and strips unknown keys, so on its own it passes `{ success: true }`
+      // and passes a payload duplicated into a stray top-level key. What it DOES
+      // catch is the missing `success` flag — the drift this line was added for.
       const parsed = BaseResponseSchema.safeParse(body);
       expect(parsed.success, `body is not a BaseResponse: ${JSON.stringify(body)}`).toBe(true);
+      // The declared envelope in full — `safeParse` alone passes a body with no
+      // `data`, or a payload duplicated into a stray top-level key (#4049).
+      expect(envelopeViolations(body), `not the declared envelope: ${JSON.stringify(body)}`).toEqual([]);
       expect(body.success).toBe(true);
       expect(body.error).toBeUndefined();
       c.expectData(body.data);
@@ -424,6 +430,9 @@ describe('share-link envelope (#3983) — error bodies', () => {
 
       const parsed = BaseResponseSchema.safeParse(body);
       expect(parsed.success, `body is not a BaseResponse: ${JSON.stringify(body)}`).toBe(true);
+      // The declared envelope in full — `safeParse` alone passes a body with no
+      // `data`, or a payload duplicated into a stray top-level key (#4049).
+      expect(envelopeViolations(body), `not the declared envelope: ${JSON.stringify(body)}`).toEqual([]);
 
       expect(body.success).toBe(false);
       expect(body.error.code).toBe(c.code);

packages/rest/src/external-datasource-envelope.conformance.test.ts

@@ -33,7 +33,7 @@
  */
 
 import { describe, it, expect, vi } from 'vitest';
-import { BaseResponseSchema } from '@objectstack/spec/api';
+import { BaseResponseSchema, envelopeViolations } from '@objectstack/spec/api';
 import type { IHttpServer, RouteHandler } from '@objectstack/spec/contracts';
 import { registerExternalDatasourceRoutes } from './external-datasource-routes.js';
 
@@ -127,9 +127,15 @@ describe('external-datasource envelope (#3843) — success bodies', () => {
       const { status, body } = await c.run();
       expect(status).toBe(c.status);
 
-      // The contract itself, imported — not a restatement of it.
+      // The envelope SKELETON, imported. It is not the whole contract: it declares
+      // no `data` and strips unknown keys, so on its own it passes `{ success: true }`
+      // and passes a payload duplicated into a stray top-level key. What it DOES
+      // catch is the missing `success` flag — the drift this line was added for.
       const parsed = BaseResponseSchema.safeParse(body);
       expect(parsed.success, `body is not a BaseResponse: ${JSON.stringify(body)}`).toBe(true);
+      // The declared envelope in full — `safeParse` alone passes a body with no
+      // `data`, or a payload duplicated into a stray top-level key (#4049).
+      expect(envelopeViolations(body), `not the declared envelope: ${JSON.stringify(body)}`).toEqual([]);
       expect(body.success).toBe(true);
       expect(body.error).toBeUndefined();
 
@@ -205,6 +211,9 @@ describe('external-datasource envelope (#3843) — error bodies', () => {
 
       const parsed = BaseResponseSchema.safeParse(body);
       expect(parsed.success, `body is not a BaseResponse: ${JSON.stringify(body)}`).toBe(true);
+      // The declared envelope in full — `safeParse` alone passes a body with no
+      // `data`, or a payload duplicated into a stray top-level key (#4049).
+      expect(envelopeViolations(body), `not the declared envelope: ${JSON.stringify(body)}`).toEqual([]);
 
       expect(body.success).toBe(false);
       expect(body.error.code).toBe(c.code);

packages/rest/src/package-envelope.conformance.test.ts

@@ -36,7 +36,7 @@
  */
 
 import { describe, it, expect } from 'vitest';
-import { BaseResponseSchema } from '@objectstack/spec/api';
+import { BaseResponseSchema, envelopeViolations } from '@objectstack/spec/api';
 import type { RouteHandler } from '@objectstack/spec/contracts';
 import { registerPackageRoutes } from './package-routes.js';
 
@@ -164,9 +164,15 @@ describe('packages envelope (#3843) — success bodies', () => {
       const { status, body } = await c.run();
       expect(status).toBe(c.status);
 
-      // The contract itself, imported — not a restatement of it.
+      // The envelope SKELETON, imported. It is not the whole contract: it declares
+      // no `data` and strips unknown keys, so on its own it passes `{ success: true }`
+      // and passes a payload duplicated into a stray top-level key. What it DOES
+      // catch is the missing `success` flag — the drift this line was added for.
       const parsed = BaseResponseSchema.safeParse(body);
       expect(parsed.success, `body is not a BaseResponse: ${JSON.stringify(body)}`).toBe(true);
+      // The declared envelope in full — `safeParse` alone passes a body with no
+      // `data`, or a payload duplicated into a stray top-level key (#4049).
+      expect(envelopeViolations(body), `not the declared envelope: ${JSON.stringify(body)}`).toEqual([]);
       expect(body.success).toBe(true);
       expect(body.error).toBeUndefined();
 
@@ -297,6 +303,9 @@ describe('packages envelope (#3843) — error bodies', () => {
 
       const parsed = BaseResponseSchema.safeParse(body);
       expect(parsed.success, `body is not a BaseResponse: ${JSON.stringify(body)}`).toBe(true);
+      // The declared envelope in full — `safeParse` alone passes a body with no
+      // `data`, or a payload duplicated into a stray top-level key (#4049).
+      expect(envelopeViolations(body), `not the declared envelope: ${JSON.stringify(body)}`).toEqual([]);
 
       expect(body.success).toBe(false);
       expect(body.error.code).toBe(c.code);

packages/runtime/src/error-envelope.conformance.test.ts

@@ -27,7 +27,7 @@
 
 import { describe, it, expect, vi } from 'vitest';
 import { readFileSync } from 'node:fs';
-import { ApiErrorSchema, BaseResponseSchema, DispatcherErrorCode } from '@objectstack/spec/api';
+import { ApiErrorSchema, BaseResponseSchema, DispatcherErrorCode, envelopeViolations } from '@objectstack/spec/api';
 import { HttpDispatcher } from './http-dispatcher.js';
 import { buildApiError, splitSemanticCode } from './error-envelope.js';
 
@@ -45,6 +45,7 @@ function expectConformantError(response: { status: number; body: any } | undefin
     const body = response!.body;
 
     expect(BaseResponseSchema.safeParse(body).success).toBe(true);
+    expect(envelopeViolations(body), `not the declared envelope: ${JSON.stringify(body)}`).toEqual([]);
     expect(body.success).toBe(false);
 
     const parsed = ApiErrorSchema.safeParse(body.error);

packages/runtime/src/i18n-success-envelope.conformance.test.ts

@@ -41,7 +41,7 @@
 
 import { describe, it, expect, vi, beforeEach } from 'vitest';
 import { HttpDispatcher } from './http-dispatcher.js';
-import { BaseResponseSchema } from '@objectstack/spec/api';
+import { BaseResponseSchema, envelopeViolations } from '@objectstack/spec/api';
 import {
   GetLocalesResponseSchema,
   GetTranslationsResponseSchema,
@@ -86,6 +86,9 @@ describe('/i18n success-envelope conformance (dispatcher domain)', () => {
   function expectEnvelope(body: unknown) {
     const parsed = BaseResponseSchema.safeParse(body);
     expect(parsed.success, `body is not a BaseResponse: ${JSON.stringify(body)}`).toBe(true);
+    // The declared envelope in full — `safeParse` alone passes a body with no
+    // `data`, or a payload duplicated into a stray top-level key (#4049).
+    expect(envelopeViolations(body), `not the declared envelope: ${JSON.stringify(body)}`).toEqual([]);
     expect((body as { success?: boolean }).success).toBe(true);
   }
 

packages/services/service-datasource/src/__tests__/envelope.conformance.test.ts

@@ -35,7 +35,7 @@
  */
 
 import { describe, it, expect, vi } from 'vitest';
-import { BaseResponseSchema } from '@objectstack/spec/api';
+import { BaseResponseSchema, envelopeViolations } from '@objectstack/spec/api';
 import { HonoHttpServer } from '@objectstack/plugin-hono-server';
 import { registerDatasourceAdminRoutes } from '../admin-routes.js';
 
@@ -128,9 +128,15 @@ describe('datasource-admin envelope (#3843) — success bodies', () => {
       const { status, body } = await c.run();
       expect(status).toBe(c.status);
 
-      // The contract itself, imported — not a restatement of it.
+      // The envelope SKELETON, imported. It is not the whole contract: it declares
+      // no `data` and strips unknown keys, so on its own it passes `{ success: true }`
+      // and passes a payload duplicated into a stray top-level key. What it DOES
+      // catch is the missing `success` flag — the drift this line was added for.
       const parsed = BaseResponseSchema.safeParse(body);
       expect(parsed.success, `body is not a BaseResponse: ${JSON.stringify(body)}`).toBe(true);
+      // The declared envelope in full — `safeParse` alone passes a body with no
+      // `data`, or a payload duplicated into a stray top-level key (#4049).
+      expect(envelopeViolations(body), `not the declared envelope: ${JSON.stringify(body)}`).toEqual([]);
       expect(body.success).toBe(true);
       expect(body.error).toBeUndefined();
 
@@ -205,6 +211,9 @@ describe('datasource-admin envelope (#3843) — error bodies', () => {
 
       const parsed = BaseResponseSchema.safeParse(body);
       expect(parsed.success, `body is not a BaseResponse: ${JSON.stringify(body)}`).toBe(true);
+      // The declared envelope in full — `safeParse` alone passes a body with no
+      // `data`, or a payload duplicated into a stray top-level key (#4049).
+      expect(envelopeViolations(body), `not the declared envelope: ${JSON.stringify(body)}`).toEqual([]);
 
       expect(body.success).toBe(false);
       expect(body.error.code).toBe(c.code);

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

@@ -31,7 +31,7 @@
  */
 
 import { describe, it, expect, vi } from 'vitest';
-import { BaseResponseSchema } from '@objectstack/spec/api';
+import { BaseResponseSchema, envelopeViolations } from '@objectstack/spec/api';
 import type { IHttpRequest, IHttpResponse, RouteHandler } from '@objectstack/spec/contracts';
 import { I18nServicePlugin } from './i18n-service-plugin';
 
@@ -167,6 +167,9 @@ describe('i18n error envelope (#3675)', () => {
 
       const parsed = BaseResponseSchema.safeParse(body);
       expect(parsed.success, `body is not a BaseResponse: ${JSON.stringify(body)}`).toBe(true);
+      // The declared envelope in full — `safeParse` alone passes a body with no
+      // `data`, or a payload duplicated into a stray top-level key (#4049).
+      expect(envelopeViolations(body), `not the declared envelope: ${JSON.stringify(body)}`).toEqual([]);
 
       expect(body.success).toBe(false);
       expect(body.error.code).toBe(c.code);
@@ -186,6 +189,7 @@ describe('i18n error envelope (#3675)', () => {
     const { status, body } = await drive(routes, `${BASE}/locales`);
     expect(status).toBe(500);
     expect(BaseResponseSchema.safeParse(body).success).toBe(true);
+    expect(envelopeViolations(body), `not the declared envelope: ${JSON.stringify(body)}`).toEqual([]);
     expect(body.error.message).toBe('Internal error');
   });
 });

packages/services/service-settings/src/envelope.conformance.test.ts

@@ -32,7 +32,7 @@
  */
 
 import { describe, expect, it, vi } from 'vitest';
-import { BaseResponseSchema } from '@objectstack/spec/api';
+import { BaseResponseSchema, envelopeViolations } from '@objectstack/spec/api';
 import { SettingsNamespacePayloadSchema } from '@objectstack/spec/system';
 import type { IHttpServer, IHttpRequest, IHttpResponse, RouteHandler } from '@objectstack/spec/contracts';
 import { SettingsService } from './settings-service';
@@ -152,9 +152,15 @@ describe('settings envelope (#3843) — success bodies', () => {
       const { status, body } = await c.run();
       expect(status).toBe(200);
 
-      // The contract itself, imported — not a restatement of it.
+      // The envelope SKELETON, imported. It is not the whole contract: it declares
+      // no `data` and strips unknown keys, so on its own it passes `{ success: true }`
+      // and passes a payload duplicated into a stray top-level key. What it DOES
+      // catch is the missing `success` flag — the drift this line was added for.
       const parsed = BaseResponseSchema.safeParse(body);
       expect(parsed.success, `body is not a BaseResponse: ${JSON.stringify(body)}`).toBe(true);
+      // The declared envelope in full — `safeParse` alone passes a body with no
+      // `data`, or a payload duplicated into a stray top-level key (#4049).
+      expect(envelopeViolations(body), `not the declared envelope: ${JSON.stringify(body)}`).toEqual([]);
       expect(body.success).toBe(true);
       expect(body.error).toBeUndefined();
 
@@ -269,6 +275,9 @@ describe('settings envelope (#3843) — error bodies', () => {
 
       const parsed = BaseResponseSchema.safeParse(body);
       expect(parsed.success, `body is not a BaseResponse: ${JSON.stringify(body)}`).toBe(true);
+      // The declared envelope in full — `safeParse` alone passes a body with no
+      // `data`, or a payload duplicated into a stray top-level key (#4049).
+      expect(envelopeViolations(body), `not the declared envelope: ${JSON.stringify(body)}`).toEqual([]);
 
       expect(body.success).toBe(false);
       expect(body.error.code).toBe(c.code);

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

@@ -38,7 +38,7 @@ import { describe, it, expect } from 'vitest';
 import { promises as fs } from 'node:fs';
 import { join } from 'node:path';
 import { tmpdir } from 'node:os';
-import { BaseResponseSchema } from '@objectstack/spec/api';
+import { BaseResponseSchema, envelopeViolations } from '@objectstack/spec/api';
 import type { IHttpRequest, IHttpResponse, RouteHandler } from '@objectstack/spec/contracts';
 import { LocalStorageAdapter } from './local-storage-adapter';
 import { StorageMetadataStore } from './metadata-store';
@@ -286,9 +286,15 @@ describe('storage error envelope (#3675)', () => {
       const { status, body } = await c.run();
       expect(status).toBe(c.status);
 
-      // The contract itself, imported — not a restatement of it.
+      // The envelope SKELETON, imported. It is not the whole contract: it declares
+      // no `data` and strips unknown keys, so on its own it passes `{ success: true }`
+      // and passes a payload duplicated into a stray top-level key. What it DOES
+      // catch is the missing `success` flag — the drift this line was added for.
       const parsed = BaseResponseSchema.safeParse(body);
       expect(parsed.success, `body is not a BaseResponse: ${JSON.stringify(body)}`).toBe(true);
+      // The declared envelope in full — `safeParse` alone passes a body with no
+      // `data`, or a payload duplicated into a stray top-level key (#4049).
+      expect(envelopeViolations(body), `not the declared envelope: ${JSON.stringify(body)}`).toEqual([]);
 
       expect(body.success).toBe(false);
       expect(body.error.code).toBe(c.code);