Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions .changeset/envelope-violations-predicate.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
13 changes: 11 additions & 2 deletions packages/rest/src/external-datasource-envelope.conformance.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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);
Expand Down
13 changes: 11 additions & 2 deletions packages/rest/src/package-envelope.conformance.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion packages/runtime/src/error-envelope.conformance.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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);
Expand All @@ -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');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,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 {
PresignedUrlResponseSchema,
FileUploadResponseSchema,
Expand Down Expand Up @@ -276,9 +276,13 @@ describe('storage success envelope (#3689)', () => {
const { status, body } = await c.run();
expect(status).toBe(200);

// The shared envelope, imported — not a restatement of it.
// The shared envelope skeleton, imported. Note what it does NOT prove: it
// declares no `data` and strips unknown keys, so it passes `{ success: true }`
// and passes a payload duplicated into a stray top-level key (#4049).
const base = BaseResponseSchema.safeParse(body);
expect(base.success, `body is not a BaseResponse: ${JSON.stringify(body)}`).toBe(true);
// …so this is the assertion that actually says "the declared envelope".
expect(envelopeViolations(body), `not the declared envelope: ${JSON.stringify(body)}`).toEqual([]);
expect(body.success).toBe(true);
expect(body.error).toBeUndefined();

Expand Down
1 change: 1 addition & 0 deletions packages/spec/api-surface.json
Original file line number Diff line number Diff line change
Expand Up @@ -3088,6 +3088,7 @@
"WorkflowTransitionRequestSchema (const)",
"WorkflowTransitionResponse (type)",
"WorkflowTransitionResponseSchema (const)",
"envelopeViolations (function)",
"getAuthEndpointUrl (function)",
"getDefaultRouteRegistrations (function)",
"readServiceSelfInfo (function)",
Expand Down
Loading
Loading