Skip to content

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

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-11292-discovery-version-override
Aug 24, 2026
Merged

fix(rest): serve the producer's derived discovery version, not config.api.version (#11292)#11548
os-zhuang merged 1 commit into
mainfrom
claude/issue-11292-discovery-version-override

Conversation

@claude

@claudeclaudeBot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Fixes#11292

registerDiscoveryEndpoints called the producer and overwrote its answer one line later:

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

config.api.version is the API version identifier, not an artifact identity: normalizeConfig() defaults it to 'v1', packages/spec/src/api/plugin-rest-api.zod.ts describes it as 'API version identifier', and getApiBasePath() builds the mount from it (api.apiPath ?? ${api.basePath}/${api.version}/api/v1). So GET /api/v1/discovery answered version: "v1" — the path segment the caller had just typed to reach the endpoint.

DiscoverySchema declares version under System Identity, grouped with name and environment. The #10993 ruling settled that as the serving artifact's version and #11235/#11242 reaffirmed it. Direction A per triage (5385327999): drop the override, let the producer's derived value through. No schema change, no new field.

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.

Clause-②: no

A served value pulled back to its declared semantics. The accept set is unchanged (no schema, no parser, no validation touched — the diff is packages/rest only), and the public surface is unchanged (no field added, removed or renamed; DiscoverySchema is untouched). What changes is the value on one existing field of one endpoint, in the direction the schema already declared. api.version keeps its real job of building the mount.

Premise-first — both second-hand readings re-measured

Triage flagged two load-bearing readings as unverified (engine seat's note 5385245138 says so in as many words). Re-measured on this branch, with the zero-hit discipline — control term getDiscoveryInfo returns 92 hits across 17 files over the same population and tooling, so the queries were live.

1. "rest-server.ts is getDiscovery()'s only non-test caller" — CONFIRMED. Scanning the spelling family (not one spelling) across *.ts/tsx/mts/mjs/js, the only non-test call is rest-server.ts:3216. Every other non-test occurrence is one of: the definition (metadata-protocol/src/protocol.ts:4808), the interface declaration (spec/src/api/protocol.zod.ts:2516), prose in comments, or handler-name string literals in spec/src/api/plugin-rest-api.zod.ts. Those strings are not an indirect caller: packages/rest/src/route-manager.ts:69-77 explicitly refuses a string handler (Received handler identifier "…"), so the manifest is declarative. No destructured or aliased reference form exists ({ … getDiscovery … } = / getDiscovery as → zero, under a scan whose substring would have caught either).

2. "DiscoverySchema has exactly two producers" — CONFIRMED.getDiscovery() in @objectstack/metadata-protocol and HttpDispatcher.getDiscoveryInfo() in @objectstack/runtime; packages/spec/src/api/discovery.zod.ts:421 names the same pair. @objectstack/rest is not a third producer but a serving seam composing over the first, which is exactly how its own conformance suite frames it. The other DiscoverySchema mentions are route-ledger responseSchema metadata and prose.

3. PR #11297 (#11235) is on the ref branched from — CONFIRMED.376c70f9 is an ancestor, and resolveDiscoveryVersion() reads getEnv('OS_RUNTIME_VERSION') || resolvePackageVersion() || 'unknown'. This matters to what the override was doing: before #11297 it masked two producers that disagreed ('1.0.0' vs '1.0'); after it, it overwrote a value that already agreed.

The one real fork did not trigger. No by-value consumer of 'v1' — no version comparison, feature gate or cache key. The 'v1' by-value assertions that do exist (packages/spec/src/api/rest-server.test.ts, plugin-rest-api.test.ts, versioning.test.ts) all assert the config default, a different fact this diff does not touch. packages/client/tests/integration/01-discovery.test.ts is a shape assertion (toMatch(/^v?\d+/)), which passes both before and after. As the card itself says, "I found no consumer" is not "there is no consumer" — embedder code outside this repo is invisible from here.

The pin: provenance, not a literal

Added to packages/rest/src/discovery-schema-conformance.test.ts, which drives the real producer through the real handler. Four assertions, none naming a version literal:

  1. a OS_RUNTIME_VERSION stamp injected by the test reaches the wire (the sentinel exists nowhere in the REST layer, so it can only have come through getDiscovery());
  2. the served value equals the producer's own answer when called directly, whatever it derives;
  3. same, on a server configured with api.version: 'v9' — with routes.data === '/api/v9/data' asserted as anti-vacuity, so the option provably took effect — and the served version is not 'v9';
  4. same again on the scoped mount, a second path to the field.

A pin spelling '1.0.0' would rot at the next release and would re-create the class #11295 is filed against.

Anti-vacuity: shown failing without the fix

Ablation carried trap … EXIT INT TERM, and the mutation was proven on disk each leg by grepping the injected and the removed text (an editor exit code proves nothing; a zero-hit sed exits 0):

legon-disk proofresult
override restoredinjected marker 1, discovery.version = this.config.api.version;1Tests 4 failed | 17 passed (21) — exactly the 4 new tests
restored to HEADboth 0, git diff emptyTests 21 passed (21)

Predicted direction before running was red-by-4; that is what was observed.

No rebuild was needed between legs, and this is measured rather than assumed: the mutated subject is imported relatively (./rest-server.js → source), and packages/rest has no dist/ at all in this worktree, yet the suite runs — so it provably reads source. The producer is the dist-resolved half (@objectstack/metadata-protocol is a bare specifier and sits in KNOWN_UNALIASED_TEST_IMPORTS for @objectstack/rest, so it resolves through exports into dist/); its closure was built first and verified to carry the #11235 derivation — version: resolveDiscoveryVersion() present, the '1.0' literal absent from dist/index.js.

Verification, at e7c103c1

Union re-run after the final commit, on that exact tree.

  • Dependency closurepnpm --workspace-concurrency=2 --filter '@objectstack/rest^...' buildos-verify-lock: VERDICT command-exit 0
  • Affected package suitepnpm --filter @objectstack/rest exec vitest run --maxWorkers=2Test Files 140 passed (140), Tests 2228 passed (2228), REST_TEST_EXIT=0
  • Typecheckpnpm --filter @objectstack/rest typecheckREST_TSC_EXIT=0

Gate set derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (which reported gate list derived from the tree of 'objectstack-ai/objectstack' at commit e7c103c1), not a hand-built path list. Every exit captured before any pipe. All green (exit=0): check:authz-resolver, check:changeset-gate-self-tests, check:cross-package-test-inputs, check:dispatcher-error-vocabulary, check:objectui-changeset, check:published-files, check:route-envelope, check:slot-lookup, check:test-source-alias, check:type-source-resolution, check:nul-bytes, check:query-options-erasure, check:type-check-coverage, check:engine-double-contract, check:where-matcher, check-adr-0087-registration.mjs, check-changeset-no-major.mjs, check-ci-filter-parity.mjs, check-cross-package-test-inputs.mjs, check-empty-changeset.mjs, check-plugin-teardown-shape.mjs, docs-audit/check-affected-docs.mjs.

pnpm check:type-check-debt — NOT MEASURED, not passed. Its two cheap legs printed clean (✓ check:type-check-coverage --self-test — 47 semantic case(s) … hold. and check-type-check-coverage: OK — 65/78 workspace packages type-checked …), but the --re-measureratchet leg did not reach a verdict: the first attempt refused outright on an unbuilt closure (--re-measure cannot run: 32 workspace dependenc(ies) … have no built type entry point on disk), and after building the full closure (pnpm exec turbo run build --filter='./packages/*' --filter='./packages/*/*'Tasks: 70 successful, 70 total) the re-measure was killed by this container's ~10-minute foreground limit before emitting one. Flagging it explicitly because it is genuinely reachable from this diff: @objectstack/rest carries a TEST_DEBT entry (errors: 155, shrink-only) and this PR adds test code. CI runs it on the real tree.

Lint — a declared narrowing, with its three evidences.pnpm lint (eslint . --no-inline-config) is CI's repo-wide run and was not run here. Targeted instead, and the narrowing is a measurement rather than a gap: (1) population membership read from eslint's own config, not guessed — eslint --print-config resolves 6 and 5 rules for the two changed files respectively, so both are in the linted population and neither is ignored; (2) counts read from --format jsonfiles linted: 2, errors: 0, warnings: 0, exit 0; (3) invariance for untouched files — this repo "never enables type-aware linting (no parserOptions.project, no typed @typescript-eslint rules) for ANY file" (eslint.config.mjs:327-328, and every parserOptions in the config is { ecmaVersion, sourceType }), so each file's verdict is a function of that file plus the config alone, and this diff touches no config and therefore cannot move any untouched file's verdict.

Out of scope

Filed #11546 (unassigned): the sibling site at rest-server.ts:3622-3629 sets openapi.json's info.version to the same config.api.version under a comment promising "the runtime version" — comment and code disagree there. Deliberately not fixed here: OpenAPI's info.version legitimately may be an API version, so the correct shape is a genuine design question rather than a mechanical extension of this card, and it is outside this card's declared file surface behaviour.

#11295 is Blocked-by: this card and unlocks on close; its docs are not touched here.


Generated by Claude Code

…fig.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
@github-actionsgithub-actionsBot added size/m documentation Improvements or additions to documentation tests tooling labels Aug 24, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/rest, touching 2 documentable anchor(s).

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

  • content/docs/ai/connect-mcp.mdx(via /api/v1/discovery (route))
  • content/docs/api/client-sdk.mdx(via /api/v1/discovery (route))
  • content/docs/api/index.mdx(via /api/v1/discovery (route))
  • content/docs/kernel/runtime-services/storage-service.mdx(via /api/v1/discovery (route))
  • content/docs/protocol/kernel/http-protocol.mdx(via /api/v1/discovery (route))

1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/implementation-status.mdx(via registerDiscoveryEndpoints (symbol), /api/v1/discovery (route))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 anchor(s) matched too much of the corpus to be a work list: /api/v1 (route, 81 pages)
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 45 of 222 client-bound route-ledger rows — the other 177 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 — 13 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 4c9780c7ace66a9ded339c5b261465ad89765b1fpackageMentionDocs.

Which tree this was computed on

This run read content/docs from 45ba760eeccd0f3b5954c7250258ab54343f5e68 — the merge of head e7c103c122f10f50829d6d5b030252e3cb264e91 into base 4c9780c7ace66a9ded339c5b261465ad89765b1f, 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 45ba760eeccd0f3b5954c7250258ab54343f5e68 && git checkout 45ba760eeccd0f3b5954c7250258ab54343f5e68
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 4c9780c7ace66a9ded339c5b261465ad89765b1f e7c103c122f10f50829d6d5b030252e3cb264e91 && git checkout -B drift-repro 4c9780c7ace66a9ded339c5b261465ad89765b1f && git merge --no-ff e7c103c122f10f50829d6d5b030252e3cb264e91
node scripts/docs-audit/affected-docs.mjs --json 4c9780c7ace66a9ded339c5b261465ad89765b1f

⚠️ 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 4c9780c7ace66a9ded339c5b261465ad89765b1f → 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