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
33 changes: 33 additions & 0 deletions .changeset/discovery-version-third-producer.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
---
"@objectstack/metadata-protocol": patch
---

fix(metadata-protocol): `getDiscovery()` serves a derived `version`, not the hardcoded `'1.0'` literal (#11235)

`ObjectStackProtocolImplementation.getDiscovery()` filled `DiscoverySchema`'s "System
Identity" `version` field with the constant `'1.0'`. The other `DiscoverySchema` producer
— `HttpDispatcher.getDiscoveryInfo()` in `@objectstack/runtime` — filled the *same* field
with its own constant `'1.0.0'` until #10993 derived it. Two producers of one field
disagreeing with each other is what proves neither literal was ever a contract value: if
`version` were a contract, two producers would not each invent their own constant; if it
is not, it should not be hardcoded. That argument needs no opinion about what `version`
"should" be.

It now resolves the same way its sibling does: an injected `OS_RUNTIME_VERSION` build
stamp, falling back to this package's own installed version, and `'unknown'` only if both
are unavailable — honest about not knowing rather than a plausible-looking constant. One
stamp, one meaning: a deployment that sets `OS_RUNTIME_VERSION` now gets the same answer
from both discovery producers and from `GET /health`, so the two can no longer drift.

The resolver is a package-local ~10-line copy of `packages/runtime/src/runtime-version.ts`
rather than a shared import: `@objectstack/runtime` depends on
`@objectstack/metadata-protocol`, not the reverse, so importing it would invert the
dependency direction, and hoisting a helper into `@objectstack/types`/`@objectstack/core`
would widen two packages' published surface for two call sites (declined at #11235
triage). `tsup.config.ts` gains `shims: true` for the same reason
`packages/runtime/tsup.config.ts` carries it — esbuild empties `import.meta` in a CJS
bundle, so without the shim `require('@objectstack/metadata-protocol')` would have fallen
through to `'unknown'` on every consumer.

No schema shape changed, no field was added, and no export was widened — only where one
field's value comes from. Patch, matching the sibling fix.
129 changes: 129 additions & 0 deletions packages/metadata-protocol/src/discovery-version.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* #11235 — the `version` field `getDiscovery()` serves must be DERIVED: an
* injected `OS_RUNTIME_VERSION` stamp, falling back to the resolved
* `@objectstack/metadata-protocol` package version — never the `'1.0'` literal
* this producer hardcoded before the fix, and never any other constant.
*
* ## What these cases are built to catch, and why they are shaped this way
*
* The regression this file exists to prevent is a literal creeping back into
* the producer. A test that asserts one specific expected string is weak
* against exactly that: whoever restores a literal only has to restore the
* string the test names. So the load-bearing case here
* ("tracks the stamp across two distinct values") asserts a PROPERTY no
* constant can satisfy — that two different injected stamps produce two
* different served values — and it names no expected string at all. The
* value-level cases sit beside it for the more ordinary failure (the stamp is
* read but mangled), not in place of it.
*
* Every assertion drives the REAL `ObjectStackProtocolImplementation
* .getDiscovery()` path, never `resolveDiscoveryVersion()` in isolation and
* never the source text: a fix that stops being wired into the producer has to
* fail here. That is the same anti-vacuity requirement #10993's sibling test
* (`packages/runtime/src/http-dispatcher.runtime-version.test.ts`) states, and
* this file is deliberately its counterpart one package over.
*/

import { describe, it, expect, afterEach } from 'vitest';
import { createRequire } from 'node:module';
import { ObjectStackProtocolImplementation } from './index.js';

/**
* The real fallback value, read the same way `resolveDiscoveryVersion()` reads
* it — so this assertion tracks the production code instead of independently
* guessing a version string that would drift from it at the next release.
*/
const PACKAGE_VERSION = (
createRequire(import.meta.url)('../package.json') as { version: string }
).version;

/**
* A protocol impl over a minimal engine — the harness
* `discovery-schema-conformance.test.ts` already uses in this package.
* `getDiscovery()` reads `engine.registry`, `engine.transaction` and the
* services registry; nothing here touches `version`, which is the point.
*/
function makeImpl() {
const engine = {
registry: {
getObject: (_name: string) => undefined,
getRegisteredTypes: () => [],
},
};
return new ObjectStackProtocolImplementation(engine as any, () => new Map());
}

async function servedVersion(): Promise<unknown> {
const discovery: any = await makeImpl().getDiscovery();
return discovery.version;
}

describe('[#11235] getDiscovery() serves a DERIVED `version`, not a literal', () => {
const ORIGINAL_STAMP = process.env.OS_RUNTIME_VERSION;

afterEach(() => {
if (ORIGINAL_STAMP === undefined) delete process.env.OS_RUNTIME_VERSION;
else process.env.OS_RUNTIME_VERSION = ORIGINAL_STAMP;
});

// Values with no plausible relationship to '1.0', '1.0.0' or the package
// version — anything else served back would prove the producer is not
// actually reading the injected stamp.
const STAMP_A = 'stamp-a-6d1e0b4-issue11235-could-not-be-a-coincidence';
const STAMP_B = 'stamp-b-c72f593-issue11235-could-not-be-a-coincidence';

it('serves the injected OS_RUNTIME_VERSION stamp verbatim', async () => {
process.env.OS_RUNTIME_VERSION = STAMP_A;

expect(await servedVersion()).toBe(STAMP_A);
});

it('TRACKS the stamp across two distinct values — a constant cannot do this', async () => {
// The anti-literal pin. It names no expected string: it asserts only
// that the served value FOLLOWS its source. Restoring `version: '1.0'`
// — or any other hardcode — fails this case no matter which string the
// hardcode picks, which is precisely what a specific-string assertion
// cannot promise.
process.env.OS_RUNTIME_VERSION = STAMP_A;
const first = await servedVersion();

process.env.OS_RUNTIME_VERSION = STAMP_B;
const second = await servedVersion();

expect(first).not.toBe(second);
expect(first).toBe(STAMP_A);
expect(second).toBe(STAMP_B);
});

it('falls back to the resolved package version — not a literal, not undefined — when no stamp is injected', async () => {
delete process.env.OS_RUNTIME_VERSION;

const version = await servedVersion();

expect(version).toBe(PACKAGE_VERSION);
expect(version).not.toBeUndefined();
// The two literals this defect family produced, named so a restoration
// of EITHER is caught here as well as by the tracking case above:
// `'1.0'` was this producer's, `'1.0.0'` the runtime dispatcher's
// (#10993). Their disagreement is the evidence neither was a contract
// value.
expect(version).not.toBe('1.0');
expect(version).not.toBe('1.0.0');
});

it('reports the fallback as a real, non-empty identity string', async () => {
delete process.env.OS_RUNTIME_VERSION;

const version = await servedVersion();

expect(typeof version).toBe('string');
expect((version as string).length).toBeGreaterThan(0);
// `'unknown'` is the honest last resort when the package's own
// `package.json` cannot be read. In this repo it always can be, so
// seeing it here would mean the resolver's read path is broken —
// failing loudly instead of passing on a plausible-looking string.
expect(version).not.toBe('unknown');
});
});
110 changes: 110 additions & 0 deletions packages/metadata-protocol/src/discovery-version.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* Resolves the value `ObjectStackProtocolImplementation.getDiscovery()` serves
* as the `DiscoverySchema` "System Identity" `version` field (`./protocol.ts`).
*
* ## #11235 — why the literal it replaces was provably not a contract value
*
* This producer hardcoded `version: '1.0'`. It is the SECOND
* `DiscoverySchema`-conforming producer; the first
* (`HttpDispatcher.getDiscoveryInfo()`, plus `GET /health`, both in
* `@objectstack/runtime`) hardcoded `'1.0.0'` until #10993 derived it. The two
* producers of the SAME `version: z.string()` field therefore disagreed with
* each other — and that, not anyone's opinion about what `version` "should"
* be, is the argument: if the field were a contract, two producers would not
* each invent their own constant; if it is not a contract, it should not be
* hardcoded at all.
*
* ## Why a package-local copy rather than an import
*
* `packages/runtime/src/runtime-version.ts` holds the identical resolver and
* CANNOT be imported here: `@objectstack/runtime` depends on
* `@objectstack/metadata-protocol`, not the reverse, so importing it would
* invert the dependency direction. Hoisting a shared helper into
* `@objectstack/types` or `@objectstack/core` (both already dependencies of
* this package) was considered and declined at #11235 triage: a hoist widens
* two packages' published surface for ~10 lines serving two call sites.
* Consolidation rides a later card if a third caller ever appears.
*
* ## Why `OS_RUNTIME_VERSION` — the SAME variable the first producer reads
*
* One stamp, one meaning. A deployment that injects `OS_RUNTIME_VERSION` now
* gets the same answer from both discovery producers and from `/health`, which
* is exactly the disagreement above, closed at its source: the two producers
* can no longer drift on a stamped host because there is nothing left for
* either of them to invent. The variable is not new — `cloud-connection-
* plugin.ts` already reads it, and #10993 made it `/health`'s source — and it
* matches AGENTS.md Prime Directive #9's `OS_{DOMAIN}_{NAME}` config-value
* shape (`RUNTIME` is the domain, `VERSION` the value; no `_ENABLED` /
* `_ALLOW_` / `_SKIP_` shape applies, this being a value rather than a flag or
* an escape hatch).
*
* The fallback is the one place this resolver differs from its sibling, and
* necessarily so: it resolves THIS package's own installed version, because
* `@objectstack/metadata-protocol` is the artifact whose identity this
* producer can honestly report when no stamp was injected.
*/

import { createRequire } from 'node:module';
import { getEnv } from '@objectstack/core';

/**
* `null` = not yet resolved. `undefined` is a legitimate resolved outcome (the
* read failed), so it cannot double as the "unset" sentinel.
*/
let cachedPackageVersion: string | undefined | null = null;

/**
* `@objectstack/metadata-protocol`'s own installed version, read from its
* `package.json`.
*
* That file sits one directory above both `src/` (tests running against
* source) and the built `dist/` output, so `../package.json` resolves the same
* `package.json` from either shape. This package's `tsup.config.ts` uses
* `splitting: true`, which emits `dist/chunk-*.js` alongside `dist/index.js` —
* siblings at the same depth, so a chunked build resolves identically.
*
* `createRequire` (not a static `import … with { type: "json" }`) matches the
* resolution style already used for this exact kind of read elsewhere in the
* repo (`packages/runtime/src/runtime-version.ts`,
* `packages/cli/src/utils/spec-version.ts`) and needs no JSON-module-assertion
* support from the build target. Its CJS half depends on `shims: true` in this
* package's tsup config, which is load-bearing rather than defensive: measured
* without it, `dist/index.cjs` carries `createRequire(import.meta.url)`
* verbatim and throws `SyntaxError: Cannot use 'import.meta' outside a module`
* at load, so `require('@objectstack/metadata-protocol')` fails outright — see
* the comment there.
*/
function resolvePackageVersion(): string | undefined {
if (cachedPackageVersion !== null) return cachedPackageVersion;
try {
const require = createRequire(import.meta.url);
const pkg = require('../package.json') as { version?: unknown };
cachedPackageVersion = typeof pkg.version === 'string' && pkg.version.length > 0
? pkg.version
: undefined;
} catch {
cachedPackageVersion = undefined;
}
return cachedPackageVersion;
}

/**
* The version this producer should report as the serving system's identity.
*
* 1. `OS_RUNTIME_VERSION` — an operator/build-pipeline-injected stamp (image
* tag, git sha, release version). Read live, not memoized: `getDiscovery()`
* builds a fresh document per call, so there is no construction moment to
* freeze against, and tests that set/unset the variable around a call must
* see it take effect without a stale cache.
* 2. The resolved `@objectstack/metadata-protocol` package version, when no
* stamp was injected.
* 3. `'unknown'` — only if BOTH of the above are unavailable (the package's own
* `package.json` is unreadable). Honest about not knowing, rather than a
* plausible-looking literal a caller could mistake for real identity — the
* exact failure mode #10993 and #11235 exist to close.
*/
export function resolveDiscoveryVersion(): string {
return getEnv('OS_RUNTIME_VERSION') || resolvePackageVersion() || 'unknown';
}
16 changes: 15 additions & 1 deletion packages/metadata-protocol/src/protocol.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,6 +10,9 @@ import { readEnvWithDeprecation, resolveTenancyPosture, resolveThrownHttpError }
// this question with it is a bug (cloud#1020, #5233) — so the posture, and only
// the posture, is what the runtime authoring gate is told.
import { postureEnforcesWall } from '@objectstack/spec/security';
// [#11235] The derived `version` this file's `getDiscovery()` serves as the
// `DiscoverySchema` "System Identity" field — never a literal again.
import { resolveDiscoveryVersion } from './discovery-version.js';
import type { MetadataHostEngine } from './host-engine.js';
import { omitInternalFieldsFromWriteResponse } from './write-response-internal-fields.js';
import {
Expand DownExpand Up@@ -5092,7 +5095,18 @@ export class ObjectStackProtocolImplementation implements
const name = 'ObjectStack API';

return {
version: '1.0',
// [#11235] The serving system's identity, DERIVED — an injected
// `OS_RUNTIME_VERSION` stamp, falling back to this package's own
// resolved version. It was the literal `'1.0'` while the other
// `DiscoverySchema` producer (`HttpDispatcher.getDiscoveryInfo()`
// in `@objectstack/runtime`) carried its own literal `'1.0.0'` —
// two producers of the same field disagreeing with each other,
// which is what proves neither constant was ever a contract value.
// #10993 fixed that producer the same way; see
// {@link resolveDiscoveryVersion} for the derivation, the fallback,
// and why the resolver is a package-local copy rather than an
// import.
version: resolveDiscoveryVersion(),
name,
/** @deprecated Use `name`. Removed in protocol 18 (#4828). */
apiName: name,
Expand Down
20 changes: 20 additions & 0 deletions packages/metadata-protocol/tsup.config.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,5 +10,25 @@ export default defineConfig({
dts: !process.env.OS_SKIP_DTS,
format: ['esm', 'cjs'],
target: 'es2020',
// [#11235] LOAD-BEARING, and measured rather than assumed. `discovery-
// version.ts` reads its own `package.json` via
// `createRequire(import.meta.url)` — correct as written for the ESM output.
// `shims: true` makes tsup rewrite `import.meta.url` in the CJS build to a
// real `__filename`-derived value (its `assets/cjs_shims.js`), so both
// formats resolve the SAME package.json; `packages/runtime/tsup.config.ts`
// carries it for #10993's resolver, the sibling of this one.
//
// What removing it does HERE was measured on this package, and it is worse
// than the degradation the sibling's comment anticipates: at this `target`
// esbuild does not empty `import.meta` in the CJS output, it emits
// `createRequire(import.meta.url)` verbatim — so `dist/index.cjs` throws
// `SyntaxError: Cannot use 'import.meta' outside a module` at LOAD time and
// `require('@objectstack/metadata-protocol')` fails outright. Not a version
// that degrades to `'unknown'`; a package a CJS consumer cannot import at
// all. Do not drop this line while `discovery-version.ts` exists.
//
// Need-based injection — nothing else here references
// `__dirname`/`__filename`, so the ESM build's shim path is a no-op.
shims: true,
external: ['vitest'],
});
Loading