diff --git a/.changeset/discovery-version-third-producer.md b/.changeset/discovery-version-third-producer.md new file mode 100644 index 0000000000..3670aa940a --- /dev/null +++ b/.changeset/discovery-version-third-producer.md @@ -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. diff --git a/packages/metadata-protocol/src/discovery-version.test.ts b/packages/metadata-protocol/src/discovery-version.test.ts new file mode 100644 index 0000000000..c7d197cd51 --- /dev/null +++ b/packages/metadata-protocol/src/discovery-version.test.ts @@ -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 { + 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'); + }); +}); diff --git a/packages/metadata-protocol/src/discovery-version.ts b/packages/metadata-protocol/src/discovery-version.ts new file mode 100644 index 0000000000..03a3c22ec7 --- /dev/null +++ b/packages/metadata-protocol/src/discovery-version.ts @@ -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'; +} diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 4600d7ab48..31f4462a08 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -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 { @@ -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, diff --git a/packages/metadata-protocol/tsup.config.ts b/packages/metadata-protocol/tsup.config.ts index 086365e59b..b83882928b 100644 --- a/packages/metadata-protocol/tsup.config.ts +++ b/packages/metadata-protocol/tsup.config.ts @@ -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'], });