fix(rest): serve the producer's derived discovery version, not config.api.version (#11292)

.changeset/rest-discovery-version-producer.md

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

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

@@ -4,9 +4,10 @@
 // matters most, because this is the shape a browser client actually receives.
 //
 // It is a COMPOSED shape: `getDiscovery()` (metadata-protocol) builds the base,
-// then `registerDiscoveryEndpoints` overrides `version` and `routes`, ANDs
+// then `registerDiscoveryEndpoints` overrides `routes`, ANDs
 // `capabilities.transactionalBatch` with its own `api.enableBatch`, and attaches
-// `scoping`. Neither producer alone could be checked against the schema and be
+// `scoping`. It no longer overrides `version` — see the #11292 suite at the
+// bottom, which pins the served value's PROVENANCE. Neither producer alone could be checked against the schema and be
 // meaningful here, so this test drives the REAL protocol implementation through
 // the REAL handler rather than the `createMockProtocol()` double the other
 // rest tests use — a mock would only prove the mock's shape conforms.
@@ -71,7 +72,7 @@ function createMockServer() {
  * deployment) or the environment-scoped `/api/v1/environments/:environmentId`
  * one, which is the only mount that can report `scoped: true`.
  */
-function discoveryHandler(opts: { scoped?: boolean } = {}) {
+function buildDiscovery(opts: { scoped?: boolean; apiVersion?: string } = {}) {
  const engine = {
  registry: {
  getObject: (_n: string) => undefined,
@@ -82,18 +83,30 @@ function discoveryHandler(opts: { scoped?: boolean } = {}) {
  const config: any = {
  api: {
  requireAuth: false,
+ ...(opts.apiVersion ? { version: opts.apiVersion } : {}),
  ...(opts.scoped ? { enableProjectScoping: true, projectResolution: 'auto' } : {}),
  },
  };
  const rest = new RestServer(createMockServer() as any, protocol as any, config);
  rest.registerRoutes();
 
+ // The mounted segment is `api.version`'s job (`getApiBasePath()`), which is
+ // exactly why it is not the identity answer — see the #11292 suite below.
+ const version = opts.apiVersion ?? 'v1';
  const path = opts.scoped
- ? '/api/v1/environments/:environmentId/discovery'
- : '/api/v1/discovery';
+ ? `/api/${version}/environments/:environmentId/discovery`
+ : `/api/${version}/discovery`;
  const entry = rest.getRouteManager().get('GET', path);
  if (!entry) throw new Error(`discovery route not registered at ${path}`);
- return entry.handler as (req: any, res: any) => Promise<void>;
+ return {
+ handler: entry.handler as (req: any, res: any) => Promise<void>,
+ /** The REAL producer this server composes over — the provenance the wire answer must track. */
+ protocol,
+ };
+}
+
+function discoveryHandler(opts: { scoped?: boolean; apiVersion?: string } = {}) {
+ return buildDiscovery(opts).handler;
 }
 
 async function invoke(
@@ -318,4 +331,87 @@ describe('[#4828] the REST /discovery live shape conforms to DiscoverySchema', (
  ).toBeUndefined();
  });
  });
+
+ // ═══════════════════════════════════════════════════════════════════════════
+ // [#11292] `version` is the PRODUCER's — provenance, pinned without a literal
+ // ═══════════════════════════════════════════════════════════════════════════
+ //
+ // This seam used to run `discovery.version = this.config.api.version` one
+ // line after calling the producer, so the wire answer was the MOUNTED PATH
+ // SEGMENT (`'v1'` by default) — the string the caller had just typed to reach
+ // the endpoint. `DiscoverySchema` declares `version` under "System Identity"
+ // next to `name` and `environment`, and the #10993 ruling (reaffirmed by
+ // #11235/#11242) settled that as the SERVING ARTIFACT's version.
+ //
+ // Every assertion below pins PROVENANCE, never a literal version string: the
+ // wire answer is compared against the producer's own answer, or against a
+ // stamp this test injects. A pin spelling `'1.0.0'` would rot at the next
+ // release and would re-create the class #11295 is filed against.
+ describe("[#11292] the served `version` is the producer's, not the mounted API version", () => {
+ it('tracks the producer when the deployment stamps OS_RUNTIME_VERSION', async () => {
+ // `resolveDiscoveryVersion()` reads the stamp LIVE (documented in
+ // `metadata-protocol/src/discovery-version.ts`), so a value injected here
+ // must appear on the wire. This is the provenance assertion: the sentinel
+ // exists nowhere in the REST layer, so it can only have come through
+ // `getDiscovery()`.
+ const old = process.env.OS_RUNTIME_VERSION;
+ process.env.OS_RUNTIME_VERSION = '9.9.9-provenance-sentinel';
+ try {
+ const body = await invoke(discoveryHandler());
+
+ expect(body.version).toBe('9.9.9-provenance-sentinel');
+ expect(DiscoverySchema.safeParse(body).success).toBe(true);
+ } finally {
+ if (old === undefined) delete process.env.OS_RUNTIME_VERSION;
+ else process.env.OS_RUNTIME_VERSION = old;
+ }
+ });
+
+ it('agrees with the producer called directly, whatever the producer derives', async () => {
+ // No stamp: the producer falls through to its own package version. The
+ // assertion still names no literal — it asks only that the two answers
+ // are the SAME answer, which is the whole content of "the REST seam does
+ // not rewrite this field".
+ const { handler, protocol } = buildDiscovery();
+
+ const body = await invoke(handler);
+ const direct: any = await (protocol as any).getDiscovery();
+
+ expect(body.version).toBe(direct.version);
+ expect(typeof body.version).toBe('string');
+ expect(body.version.length).toBeGreaterThan(0);
+ });
+
+ it('answers the producer even when `api.version` is set to something else', async () => {
+ // The sharp edge, stated as a measurement. `api.version` still does its
+ // real job — it builds the mount — and that job is visible in the SAME
+ // document, on `routes`. What it no longer does is answer the identity
+ // question.
+ const { handler, protocol } = buildDiscovery({ apiVersion: 'v9' });
+
+ const body = await invoke(handler);
+ const direct: any = await (protocol as any).getDiscovery();
+
+ // Anti-vacuity: `api.version` really is 'v9' on this server, and really
+ // does drive the mounted path. Without this, the assertion below could
+ // pass on a server where the option was silently ignored.
+ expect(body.routes.data).toBe('/api/v9/data');
+
+ expect(body.version).toBe(direct.version);
+ expect(body.version).not.toBe('v9');
+ });
+
+ it('does not answer the mounted segment on the scoped mount either', async () => {
+ // The scoped mount runs the same closure but resolves a per-request
+ // protocol, so it is a second path to the same field.
+ const { handler, protocol } = buildDiscovery({ scoped: true, apiVersion: 'v9' });
+
+ const body = await invoke(handler, { environmentId: 'env_alpha' });
+ const direct: any = await (protocol as any).getDiscovery();
+
+ expect(body.routes.data).toBe('/api/v9/environments/env_alpha/data');
+ expect(body.version).toBe(direct.version);
+ expect(body.version).not.toBe('v9');
+ });
+ });
 });

packages/rest/src/rest-server.ts

@@ -3215,8 +3215,32 @@ export class RestServer {
  const protocol = await this.resolveProtocol(environmentId, req);
  const discovery = await protocol.getDiscovery();
 
- // Override discovery information with actual server configuration
- discovery.version = this.config.api.version;
+ // [#11292] `version` is the PRODUCER's, and is deliberately
+ // NOT overwritten here. `DiscoverySchema` declares the field
+ // under "System Identity", grouped with `name` and
+ // `environment` — the "what server is this" question, settled
+ // by the #10993 ruling and reaffirmed by #11235/#11242.
+ //
+ // This line used to read `discovery.version =
+ // this.config.api.version`, which is a different fact
+ // entirely: `normalizeConfig()` defaults it to `'v1'` and the
+ // SAME value builds the mounted path (`${basePath}/${version}`
+ // → `/api/v1`), so `GET /api/v1/discovery` answered with the
+ // path segment the caller had just typed to get there. On the
+ // one producer most clients actually hit, the identity field
+ // carried no identity.
+ //
+ // It also masked the producer. `getDiscovery()` derives the
+ // value from `OS_RUNTIME_VERSION` (#11235) — the same stamp
+ // `/health` and the runtime dispatcher's own `/discovery`
+ // read (#10993/#11242) — so after #11297 this overwrote a
+ // value that already AGREED with the other producer, turning
+ // one answer back into two dialects of one field.
+ //
+ // The API-version fact is not lost: every entry in `routes`
+ // below is prefixed with the mounted base path, which is
+ // built from `api.version`. It is recoverable from the same
+ // document, in the field that means it.
 
  // Substitute the resolved environmentId into the advertised routes so
  // clients can consume them verbatim (e.g. /api/v1/environments/abc/data).