Skip to content

fix(metadata-protocol): derive discovery version instead of the hardcoded '1.0' literal - #11297

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-11235-discovery-version-third-producer
Aug 23, 2026
Merged

fix(metadata-protocol): derive discovery version instead of the hardcoded '1.0' literal#11297
os-zhuang merged 2 commits into
mainfrom
claude/issue-11235-discovery-version-third-producer

Conversation

@os-zhuang

@os-zhuangos-zhuang commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Fixes#11235

Premise check

Confirmed at origin/main (1fa05a69e): packages/metadata-protocol/src/protocol.ts:5095 carried version: '1.0' inside getDiscovery()'s return. Premise holds for the producer.

One part of the card's framing does not survive the check, and it matters — reported rather than quietly worked around. The card describes this producer as "served over REST by packages/rest/src/rest-server.ts:3196". It is called there, and its version is overwritten on the next line:

constdiscovery=awaitprotocol.getDiscovery();// Override discovery information with actual server configurationdiscovery.version=this.config.api.version;// rest-server.ts:3219

config.api.version defaults to 'v1' (normalizeConfig()) and is the API version identifier that builds the mounted URL (api.apiPath ?? \${api.basePath}/${api.version}`; spec declares it z.string().default('v1').describe('API version identifier')). So the '1.0'` literal never reached the REST wire, and neither does its replacement.

That does not make this card moot, and it is deliberately not fixed here:

The evidence, which needs no opinion about what version should be

DiscoverySchema has exactly two producers (the spec's own census, packages/spec/src/api/discovery.zod.ts:421). They filled the sameversion: z.string() "System Identity" field with different constants — '1.0.0' in HttpDispatcher.getDiscoveryInfo() (until #10993), '1.0' here. Both literals are therefore provably not meaningful contract values: if version were a contract, two producers would not each invent their own; if it is not, it should not be hardcoded.

Chosen shape — matched to what #10993 actually landed, not to the card's wording

Read #11242 (the merged #10993 fix) rather than the card's wording, per dispatch. Same three-step resolution, same env var, same 'unknown' last resort:

  1. OS_RUNTIME_VERSION — the operator/build-pipeline stamp. The same variable, not a new one: cloud-connection-plugin.ts already read it and [finding] /api/v1/health reports a hardcoded version: '1.0.0' — a field that exists and lies, so no consumer can use it for artifact identity #10993 made it /health's source. One stamp, one meaning — a stamped deployment now gets the same answer from both discovery producers and /health, which is the producer disagreement closed at its source. AGENTS.md [WIP] Create a new release version #9's OS_{DOMAIN}_{NAME} config-value shape.
  2. This package's own resolved package.json version, via createRequire(import.meta.url)('../package.json').
  3. 'unknown' only if both are unavailable — honest about not knowing rather than a plausible-looking literal.

Two deliberate differences from the sibling, each forced:

  • Package-local resolver (packages/metadata-protocol/src/discovery-version.ts, not exported from the package index). @objectstack/runtime depends on @objectstack/metadata-protocol, not the reverse, so resolveRuntimeVersion() cannot be imported here; hoisting a shared helper to types/core was declined at triage. Named resolveDiscoveryVersion() rather than duplicating resolveRuntimeVersion so it cannot be confused with the sibling or with PROTOCOL_VERSION (the unrelated wire-protocol number in spec/kernel).
  • Resolved per call, not memoized at construction.[finding] /api/v1/health reports a hardcoded version: '1.0.0' — a field that exists and lies, so no consumer can use it for artifact identity #10993 resolves once in HttpDispatcher's constructor because the dispatcher is long-lived; getDiscovery() builds a fresh document per call and has no construction moment to freeze against.

tsup.config.ts gains shims: true, exactly as packages/runtime/tsup.config.ts did for the sibling.

Reverse verification — both legs, on-disk proof, restored by trap

Fix committed first (88fb4b6ce), so every restore is a git checkout HEAD -- back to a known state. Each ablation script installs an EXIT INT TERM trap whose handler restores the mutated file (spelled without angle brackets here on purpose — GitHub's body sanitizer eats short bracketed placeholders, which is how the previous revision of this paragraph rendered as an EMPTY trap handler and read as though the restore were a no-op), and each mutation is proven on disk by anchored counts of the specific text, not by an editor's exit code or a bare git diff --stat.

Ablation 1 — restore the '1.0' literal on the producer. Predicted direction: red.

--- BEFORE mutation (anchored counts) --- literal count: 0 derived count: 1
--- AFTER mutation (anchored counts) --- literal count: 1 derived count: 0
MUTATION_LANDED: YES
FAIL src/discovery-version.test.ts > serves the injected OS_RUNTIME_VERSION stamp verbatim
FAIL src/discovery-version.test.ts > TRACKS the stamp across two distinct values — a constant cannot do this
AssertionError: expected '1.0' not to be '1.0'
FAIL src/discovery-version.test.ts > falls back to the resolved package version …
AssertionError: expected '1.0' to be '17.2.0'
Test Files 1 failed (1) Tests 3 failed | 1 passed (4)
RESTORED (trap): injected=0 derived=1

The fourth case stayed green, correctly: '1.0'is a non-empty string that is not 'unknown', which is what that case asserts. It is not an anti-literal pin and does not pretend to be one. No build was needed for this leg — the test imports ./index.js, a package-relative specifier vitest resolves to src/, so no dist/ is in the path.

Ablation 2 — drop shims: true and rebuild. Predicted direction: the CJS consumer degrades to 'unknown'. Observed: worse than predicted, and reported as observed.

--- AFTER mutation (anchored count) --- shims count: 0
MUTATION_LANDED: YES MUTATION-LEG REBUILD exit=0
getImportMetaUrl occurrences in dist/*.cjs: 0 (the shim really left the built output)
DIST_PROBE_VERDICT: ERROR
dist/index.cjs:20 const require2 = _module.createRequire.call(void 0, import.meta.url);
SyntaxError: Cannot use 'import.meta' outside a module

At this target esbuild does not empty import.meta in the CJS output, it emits it verbatim — so without the shim require('@objectstack/metadata-protocol') throws at load time and the package is unimportable from CJS, not merely degraded. Both comments were rewritten to state the measured behaviour rather than the sibling's prediction (23d2831b8). Restore leg rebuilt too, and re-verified — no mutated dist/ survived the ablation:

RESTORED (trap): shims count now 1 RESTORE-LEG REBUILD exit=0 shim back in dist/chunk-XJQEL33S.cjs
git status --short → (empty) # byte-identical restore

Both built formats verified against a real getDiscovery() call

Not a unit test: constructed ObjectStackProtocolImplementation from the builtdist/index.cjs and dist/index.js and read the served field.

package.json version : 17.2.0
CJS build, no stamp : 17.2.0
ESM build, no stamp : 17.2.0
CJS build, stamped : dist-probe-stamp-11235
DIST_PROBE_VERDICT: PASS

Pin design — a property, not a string

packages/metadata-protocol/src/discovery-version.test.ts, four cases, every one driving the real getDiscovery() (never the resolver in isolation, never source text) so a fix that stops being wired in fails here.

The load-bearing case names no expected string at all: it asserts that two different injected stamps produce two different served values. No constant can satisfy that, whichever constant someone picks — which is precisely what a specific-string assertion cannot promise. The value-level cases sit beside it for the ordinary failure (stamp read but mangled), not in place of it, and they name both '1.0' and '1.0.0' so a restoration of either literal is caught twice.

Consumer-by-value survey — with the controls that prove the queries were live

Dispatch asked whether discovery's version is consumed by value (version comparison, feature gating, cache key), because that would make this more than a bug fix. I found no such consumer in objectstack, objectui or cloud.

Controls, because a zero-hit query is not a reading until a known-present one proves it ran:

queryresultcontrol
discovery(Info|Result|Data)?\.version / data.version (repo)17 hits, none a comparisonsame regex family on apiName → 5 hits
semver comparison against a discovery version (repo)0semver anywhere → 10+ files
cache key built from a version (repo)0cacheKey → 5+ files
org-wide "version ===" near discovery4 hits, none about this fieldorg-wide x-objectstack-build-sha10 hits across objectstack AND cloud, so org search reaches both sibling repos

What the by-value hits actually are: packages/adapters/hono/src/hono.test.ts:95 pins '1.0' from its own mock of getDiscoveryInfo (the other producer — untouched here); packages/client/tests/integration/01-discovery.test.ts:38 asserts /^v?\d+/ against the REST-served value, which is config.api.version ('v1') and unaffected; packages/spec/src/api/discovery.test.ts:207 is a schema-parse fixture. packages/metadata-core/src/protocol-handshake.ts compares PROTOCOL_VERSION against a package manifest — a different field entirely.

Stated at its real strength: "I found no consumer" is not "there is no consumer." Embedder code outside these three repos is not visible from here.

Tier

Default tier holds, on content: no schema shape change, no new field, no widened export (discovery-version.ts is package-internal — not re-exported from src/index.ts, mirroring runtime-version.ts). Only where one field's value comes from. packages/spec untouched.

Gate verdicts — exit captured before any pipe, each quoted from the gate's own output

Run at 23d2831b8 (this PR's head, git status --porcelain empty at run time). The list was re-derived with node scripts/pm/dispatch-gates.mjsafter the changeset existed, and every path-derived and convention-triggered family it named was run except the one noted at the end.

  • pnpm lint (repo-wide eslint . --no-inline-config) — LINT_EXIT=0. Full farm, no narrowing to declare.
  • pnpm --filter @objectstack/metadata-protocol testTest Files 138 passed | 2 skipped (140) / Tests 1883 passed | 10 skipped (1893).
  • pnpm --filter @objectstack/metadata-protocol buildESM ⚡️ Build success / DTS ⚡️ Build success, both formats.
  • pnpm check:nul-bytescheck-nul-bytes: OK (scanned 6336 text file(s) …; no raw ASCII control bytes).
  • pnpm check:cross-package-test-inputsOK: 14 package(s) read outside themselves, all declared, and turbo.json hashes every declared glob.
  • pnpm check:published-files✓ 69 publishable package(s) of 78 workspace member(s) declare a \files` whitelist …`
  • pnpm check:test-source-aliascheck-test-source-alias OK — 72 packages with tests scanned; …
  • pnpm check:where-matcher✓ where-matcher conformance holds: 282 matcher(s) discovered … baseline key set verified against 1fa05a6: no files added.
  • pnpm check:query-options-erasure✓ query-options-erasure ratchet holds: 67 unswept non-test site(s) in 17 file(s), none new … no files added.
  • pnpm check:engine-double-contractOK — 384 pinned, 133 in the DEBT ledger, 2 exempt.
  • pnpm check:type-check-coverage — exit 0 (structural layer).
  • node scripts/check-empty-changeset.mjs✓ No empty-frontmatter changeset introduced by this diff (1 declaring changeset(s) added).
  • node scripts/check-changeset-no-major.mjs✓ This diff introduces no \major` bump.`
  • node scripts/check-adr-0087-registration.mjs✓ this PR adds no declared-breaking changeset (1 non-breaking changeset(s) seen).
  • Also exit 0: check:durability-log-level, check:filter-alias-parity, check:slot-lookup, check:type-source-resolution, check:objectui-changeset, check:changeset-gate-self-tests, scripts/check-ci-filter-parity.mjs, scripts/check-plugin-teardown-shape.mjs, scripts/docs-audit/check-affected-docs.mjs.

One declared narrowing.pnpm check:type-check-debt (--re-measure) requires the whole workspace build closure and re-runs tsc across all 33 ledger entries; that is CI's run. What it would judge about this diff was measured directly instead: tsc --noEmit -p packages/metadata-protocol/tsconfig.json reports 63 errors — exactly this package's recorded ledger number (errors: 63 in scripts/check-type-check-coverage.mjs) — with 0 attributed to src/discovery-version.ts, 0 to src/discovery-version.test.ts and 0 to src/protocol.ts. The count landing on the ledger figure is the before/after comparison; the ratchet cannot move.

Out of scope, filed not fixed

No fourth DiscoverySchema producer exists: the spec's own census names exactly two, and the only non-test caller of getDiscovery() in the tree is rest-server.ts.


Generated by Claude Code

….0'` literal (#11235)
`getDiscovery()` filled `DiscoverySchema`'s "System Identity" `version` with the
constant `'1.0'`, while the other producer of the same field
(`HttpDispatcher.getDiscoveryInfo()` in `@objectstack/runtime`) filled it with
its own constant `'1.0.0'` until #10993. Two producers disagreeing on one field
is what proves neither literal was a contract value.
It now resolves an injected `OS_RUNTIME_VERSION` stamp, falling back to this
package's own installed version, then `'unknown'` — the same shape #10993
landed, with a package-local resolver because the dependency direction forbids
importing runtime's. `tsup.config.ts` gains `shims: true` so the CJS build's
`import.meta.url` is real, exactly as `packages/runtime/tsup.config.ts` does.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RfyXxZ2WPjcjhuXpiQQc3y
…`shims: true` (#11235)
Reverse verification removed `shims: true` and rebuilt: 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 —
`require('@objectstack/metadata-protocol')` fails outright rather than
degrading to `'unknown'`. Both comments now state what was measured on this
package instead of what the sibling package's comment predicts.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RfyXxZ2WPjcjhuXpiQQc3y
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/metadata-protocol, touching 5 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/kernel/services-checklist.mdx(via getDiscovery (symbol))
What this run could not see
  • the SDK route bridge reached 45 of 221 client-bound route-ledger rows — the other 176 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 7 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 764dbbccbd00bbf61936bf128be524e8d8ad1bedpackageMentionDocs.

Which tree this was computed on

This run read content/docs from 02409c23fbcd71478133ce33e511838148edb9de — the merge of head 23d2831b8eec03c365051740a57f72d829fd52f3 into base 764dbbccbd00bbf61936bf128be524e8d8ad1bed, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 02409c23fbcd71478133ce33e511838148edb9de && git checkout 02409c23fbcd71478133ce33e511838148edb9de
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 764dbbccbd00bbf61936bf128be524e8d8ad1bed 23d2831b8eec03c365051740a57f72d829fd52f3 && git checkout -B drift-repro 764dbbccbd00bbf61936bf128be524e8d8ad1bed && git merge --no-ff 23d2831b8eec03c365051740a57f72d829fd52f3
node scripts/docs-audit/affected-docs.mjs --json 764dbbccbd00bbf61936bf128be524e8d8ad1bed

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 764dbbccbd00bbf61936bf128be524e8d8ad1bed → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@os-zhuang@claude