Skip to content

Commit e7c103c

Browse files
committed
fix(rest): serve the producer's derived discovery version, not config.api.version (#11292)
`registerDiscoveryEndpoints` called `protocol.getDiscovery()` and then unconditionally overwrote the result's `version` with `this.config.api.version` — the API version identifier that `normalizeConfig()` defaults to `'v1'` and that `getApiBasePath()` uses to build the mount. So `GET /api/v1/discovery` answered with the path segment the caller had just typed to get there. `DiscoverySchema` declares `version` under "System Identity" alongside `name` and `environment`; the #10993 ruling settled that as the serving artifact's version and #11235/#11242 reaffirmed it. Dropping the override lets the producer's derivation (`OS_RUNTIME_VERSION` → package version → `'unknown'`) reach the wire, which is the same stamp `/health` and the runtime dispatcher's own `/discovery` read. The API-version fact stays recoverable from the same document: every `routes` entry is prefixed with the mounted base path. No schema change and no new field. The pin in `discovery-schema-conformance.test.ts` drives the real producer through the real handler and asserts PROVENANCE, not a literal — an injected `OS_RUNTIME_VERSION` stamp must reach the wire, and the served value must equal the producer's own answer even on a server mounted at a different `api.version`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR
1 parent daacc10 commit e7c103c

3 files changed

Lines changed: 181 additions & 8 deletions

File tree

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
---
2+
'@objectstack/rest': patch
3+
---
4+
5+
`GET /api/v1/discovery` reports the serving artifact's version instead of the
6+
URL path segment the caller just typed
7+
8+
`registerDiscoveryEndpoints` called the producer and overwrote the answer one
9+
line later:
10+
11+
```ts
12+
const discovery = await protocol.getDiscovery();
13+
14+
// Override discovery information with actual server configuration
15+
discovery.version = this.config.api.version;
16+
```
17+
18+
`config.api.version` is the **API version identifier**, not an artifact
19+
identity. `normalizeConfig()` defaults it to `'v1'`, `packages/spec`'s
20+
`plugin-rest-api.zod.ts` describes it as "API version identifier", and the same
21+
value builds the mount — `getApiBasePath()` returns
22+
``api.apiPath ?? `${api.basePath}/${api.version}` `` → `/api/v1`. So on every
23+
REST-served host, `GET /api/v1/discovery` answered `version: "v1"`: the segment
24+
the caller had already typed to reach the endpoint, on every build of every
25+
release, forever.
26+
27+
`DiscoverySchema` declares `version` under **System Identity**, grouped with
28+
`name` and `environment` — the "what server is this" question. The #10993
29+
ruling settled that reading and #11235/#11242 reaffirmed it. The override is
30+
now gone and the producer's derived value reaches the wire.
31+
32+
**What changes on the wire.** `version` on this endpoint was `"v1"` and is now
33+
the value `getDiscovery()` derives: `OS_RUNTIME_VERSION` when a deployment or
34+
build pipeline stamps one, else the resolved `@objectstack/metadata-protocol`
35+
package version, else `"unknown"`. That is the same stamp `/health` and the
36+
runtime dispatcher's own `/discovery` already read, so the two discovery
37+
producers now give one answer rather than two dialects of one field. Before
38+
#11297 this override masked two producers that genuinely disagreed (`'1.0.0'`
39+
vs `'1.0'`); after it, it was overwriting a value that already agreed.
40+
41+
**The API-version fact is not lost.** Every entry in the same document's
42+
`routes` is prefixed with the mounted base path, which is built from
43+
`api.version` — recoverable from the same response, in the field that means it.
44+
No schema change, no new field: the accept set and the public surface are
45+
unchanged, and `api.version` still does its real job of building the mount.
46+
47+
Pinned in `packages/rest/src/discovery-schema-conformance.test.ts`, which drives
48+
the **real** producer through the **real** handler. The assertions pin
49+
provenance, never a literal version string — a stamp injected by the test must
50+
appear on the wire, and the served value must equal what the producer answers
51+
when called directly, including on a server configured with a different
52+
`api.version` (where `routes.data` is asserted to still carry that segment). A
53+
pin spelling a literal would rot at the next release.

‎packages/rest/src/discovery-schema-conformance.test.ts‎

Lines changed: 102 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,10 @@
44
// matters most, because this is the shape a browser client actually receives.
55
//
66
// It is a COMPOSED shape: `getDiscovery()` (metadata-protocol) builds the base,
7-
// then `registerDiscoveryEndpoints` overrides `version` and `routes`, ANDs
7+
// then `registerDiscoveryEndpoints` overrides `routes`, ANDs
88
// `capabilities.transactionalBatch` with its own `api.enableBatch`, and attaches
9-
// `scoping`. Neither producer alone could be checked against the schema and be
9+
// `scoping`. It no longer overrides `version` — see the #11292 suite at the
10+
// bottom, which pins the served value's PROVENANCE. Neither producer alone could be checked against the schema and be
1011
// meaningful here, so this test drives the REAL protocol implementation through
1112
// the REAL handler rather than the `createMockProtocol()` double the other
1213
// rest tests use — a mock would only prove the mock's shape conforms.
@@ -71,7 +72,7 @@ function createMockServer() {
7172
* deployment) or the environment-scoped `/api/v1/environments/:environmentId`
7273
* one, which is the only mount that can report `scoped: true`.
7374
*/
74-
function discoveryHandler(opts: { scoped?: boolean } = {}) {
75+
function buildDiscovery(opts: { scoped?: boolean; apiVersion?: string } = {}) {
7576
const engine = {
7677
registry: {
7778
getObject: (_n: string) => undefined,
@@ -82,18 +83,30 @@ function discoveryHandler(opts: { scoped?: boolean } = {}) {
8283
const config: any = {
8384
api: {
8485
requireAuth: false,
86+
...(opts.apiVersion ? { version: opts.apiVersion } : {}),
8587
...(opts.scoped ? { enableProjectScoping: true, projectResolution: 'auto' } : {}),
8688
},
8789
};
8890
const rest = new RestServer(createMockServer() as any, protocol as any, config);
8991
rest.registerRoutes();
9092

93+
// The mounted segment is `api.version`'s job (`getApiBasePath()`), which is
94+
// exactly why it is not the identity answer — see the #11292 suite below.
95+
const version = opts.apiVersion ?? 'v1';
9196
const path = opts.scoped
92-
? '/api/v1/environments/:environmentId/discovery'
93-
: '/api/v1/discovery';
97+
? `/api/${version}/environments/:environmentId/discovery`
98+
: `/api/${version}/discovery`;
9499
const entry = rest.getRouteManager().get('GET', path);
95100
if (!entry) throw new Error(`discovery route not registered at ${path}`);
96-
return entry.handler as (req: any, res: any) => Promise<void>;
101+
return {
102+
handler: entry.handler as (req: any, res: any) => Promise<void>,
103+
/** The REAL producer this server composes over — the provenance the wire answer must track. */
104+
protocol,
105+
};
106+
}
107+
108+
function discoveryHandler(opts: { scoped?: boolean; apiVersion?: string } = {}) {
109+
return buildDiscovery(opts).handler;
97110
}
98111

99112
async function invoke(
@@ -318,4 +331,87 @@ describe('[#4828] the REST /discovery live shape conforms to DiscoverySchema', (
318331
).toBeUndefined();
319332
});
320333
});
334+
335+
// ═══════════════════════════════════════════════════════════════════════════
336+
// [#11292] `version` is the PRODUCER's — provenance, pinned without a literal
337+
// ═══════════════════════════════════════════════════════════════════════════
338+
//
339+
// This seam used to run `discovery.version = this.config.api.version` one
340+
// line after calling the producer, so the wire answer was the MOUNTED PATH
341+
// SEGMENT (`'v1'` by default) — the string the caller had just typed to reach
342+
// the endpoint. `DiscoverySchema` declares `version` under "System Identity"
343+
// next to `name` and `environment`, and the #10993 ruling (reaffirmed by
344+
// #11235/#11242) settled that as the SERVING ARTIFACT's version.
345+
//
346+
// Every assertion below pins PROVENANCE, never a literal version string: the
347+
// wire answer is compared against the producer's own answer, or against a
348+
// stamp this test injects. A pin spelling `'1.0.0'` would rot at the next
349+
// release and would re-create the class #11295 is filed against.
350+
describe("[#11292] the served `version` is the producer's, not the mounted API version", () => {
351+
it('tracks the producer when the deployment stamps OS_RUNTIME_VERSION', async () => {
352+
// `resolveDiscoveryVersion()` reads the stamp LIVE (documented in
353+
// `metadata-protocol/src/discovery-version.ts`), so a value injected here
354+
// must appear on the wire. This is the provenance assertion: the sentinel
355+
// exists nowhere in the REST layer, so it can only have come through
356+
// `getDiscovery()`.
357+
const old = process.env.OS_RUNTIME_VERSION;
358+
process.env.OS_RUNTIME_VERSION = '9.9.9-provenance-sentinel';
359+
try {
360+
const body = await invoke(discoveryHandler());
361+
362+
expect(body.version).toBe('9.9.9-provenance-sentinel');
363+
expect(DiscoverySchema.safeParse(body).success).toBe(true);
364+
} finally {
365+
if (old === undefined) delete process.env.OS_RUNTIME_VERSION;
366+
else process.env.OS_RUNTIME_VERSION = old;
367+
}
368+
});
369+
370+
it('agrees with the producer called directly, whatever the producer derives', async () => {
371+
// No stamp: the producer falls through to its own package version. The
372+
// assertion still names no literal — it asks only that the two answers
373+
// are the SAME answer, which is the whole content of "the REST seam does
374+
// not rewrite this field".
375+
const { handler, protocol } = buildDiscovery();
376+
377+
const body = await invoke(handler);
378+
const direct: any = await (protocol as any).getDiscovery();
379+
380+
expect(body.version).toBe(direct.version);
381+
expect(typeof body.version).toBe('string');
382+
expect(body.version.length).toBeGreaterThan(0);
383+
});
384+
385+
it('answers the producer even when `api.version` is set to something else', async () => {
386+
// The sharp edge, stated as a measurement. `api.version` still does its
387+
// real job — it builds the mount — and that job is visible in the SAME
388+
// document, on `routes`. What it no longer does is answer the identity
389+
// question.
390+
const { handler, protocol } = buildDiscovery({ apiVersion: 'v9' });
391+
392+
const body = await invoke(handler);
393+
const direct: any = await (protocol as any).getDiscovery();
394+
395+
// Anti-vacuity: `api.version` really is 'v9' on this server, and really
396+
// does drive the mounted path. Without this, the assertion below could
397+
// pass on a server where the option was silently ignored.
398+
expect(body.routes.data).toBe('/api/v9/data');
399+
400+
expect(body.version).toBe(direct.version);
401+
expect(body.version).not.toBe('v9');
402+
});
403+
404+
it('does not answer the mounted segment on the scoped mount either', async () => {
405+
// The scoped mount runs the same closure but resolves a per-request
406+
// protocol, so it is a second path to the same field.
407+
const { handler, protocol } = buildDiscovery({ scoped: true, apiVersion: 'v9' });
408+
409+
const body = await invoke(handler, { environmentId: 'env_alpha' });
410+
const direct: any = await (protocol as any).getDiscovery();
411+
412+
expect(body.routes.data).toBe('/api/v9/environments/env_alpha/data');
413+
expect(body.version).toBe(direct.version);
414+
expect(body.version).not.toBe('v9');
415+
});
416+
});
321417
});

‎packages/rest/src/rest-server.ts‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3215,8 +3215,32 @@ export class RestServer {
32153215
const protocol = await this.resolveProtocol(environmentId, req);
32163216
const discovery = await protocol.getDiscovery();
32173217

3218-
// Override discovery information with actual server configuration
3219-
discovery.version = this.config.api.version;
3218+
// [#11292] `version` is the PRODUCER's, and is deliberately
3219+
// NOT overwritten here. `DiscoverySchema` declares the field
3220+
// under "System Identity", grouped with `name` and
3221+
// `environment` — the "what server is this" question, settled
3222+
// by the #10993 ruling and reaffirmed by #11235/#11242.
3223+
//
3224+
// This line used to read `discovery.version =
3225+
// this.config.api.version`, which is a different fact
3226+
// entirely: `normalizeConfig()` defaults it to `'v1'` and the
3227+
// SAME value builds the mounted path (`${basePath}/${version}`
3228+
// → `/api/v1`), so `GET /api/v1/discovery` answered with the
3229+
// path segment the caller had just typed to get there. On the
3230+
// one producer most clients actually hit, the identity field
3231+
// carried no identity.
3232+
//
3233+
// It also masked the producer. `getDiscovery()` derives the
3234+
// value from `OS_RUNTIME_VERSION` (#11235) — the same stamp
3235+
// `/health` and the runtime dispatcher's own `/discovery`
3236+
// read (#10993/#11242) — so after #11297 this overwrote a
3237+
// value that already AGREED with the other producer, turning
3238+
// one answer back into two dialects of one field.
3239+
//
3240+
// The API-version fact is not lost: every entry in `routes`
3241+
// below is prefixed with the mounted base path, which is
3242+
// built from `api.version`. It is recoverable from the same
3243+
// document, in the field that means it.
32203244

32213245
// Substitute the resolved environmentId into the advertised routes so
32223246
// clients can consume them verbatim (e.g. /api/v1/environments/abc/data).

0 commit comments

Comments
 (0)