Skip to content

@objectstack/rest overwrites discovery version with config.api.version ('v1') — the URL path segment served in the schema's "System Identity" field, masking the producer entirely #11292

Description

@os-zhuang

Found while implementing #11235 (deriving getDiscovery()'s version instead of the '1.0' literal). Filed rather than fixed: #11235's dispatch bounded that card to the producer, and this is a different package with a real design question attached, so it is not a mechanical in-place fix.

What was measured

packages/rest/src/rest-server.ts calls the producer and then unconditionally replaces the field one line later:

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() in the same file defaults it to 'v1' (api.version ?? 'v1').
  • The same value builds the mounted URL: api.apiPath ?? \${api.basePath}/${api.version}`→/api/v1`.
  • packages/spec/src/api/plugin-rest-api.zod.ts declares it version: z.string().default('v1').describe('API version identifier').

But DiscoverySchema (packages/spec/src/api/discovery.zod.ts) declares that field under /** System Identity */, grouped with name and environment — the "what server is this" question. #10993's ruling and its evidence (the SDK's connect() logging data.version next to apiName and services under "Connected to ObjectStack server") settled that reading.

So on any REST-served host, GET /api/v1/discovery answers version: "v1" — the path segment the caller already typed into the URL to get there.

Two consequences worth separating

  1. The field carries no identity information on the REST surface. /health and the runtime dispatcher's /discovery both report a real derived artifact version after [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; the REST /discovery reports "v1" on every build of every release forever. A consumer reading one document cannot tell which producer answered, and gets a useless answer from the one that most clients hit.
  2. It masks the producer. [finding] a THIRD hardcoded discovery version literal — getDiscovery() in packages/metadata-protocol/src/protocol.ts, identical defect to #10993, different producer/package #11235 fixed getDiscovery()'s hardcoded '1.0', but that value is overwritten before it reaches the wire — the producer fix is correct and pins the class at the producer level, and is observable to embedders calling ObjectStackProtocol.getDiscovery() directly, but it changes nothing on the REST endpoint while this line stands. The two fixes are independent and this one is where the wire-visible behaviour lives.

Why this is a decision, not a codemod

Deleting the line is not obviously right on its own. api.version is real information a client may want (which mounted API version answered), and the schema has no field for it — DiscoverySchema declares name / version / environment / routes / locale / services / capabilities / schemaDiscovery / scoping and nothing that means "API version". Options, roughly:

Recommend A unless someone can name a consumer of the API-version fact that routes does not already serve; B only if such a consumer exists. Either way it is a triage call, not a dev call.

Consumer survey (same one run for #11235)

No consumer reads discovery version by value — no version comparison, feature gate, or cache key — in objectstack, objectui or cloud. Controls: an org-wide code search for a known-present string (x-objectstack-build-sha) returned 10 hits across objectstack and cloud, so the queries were live in both repos. The only by-value assertions found are packages/adapters/hono/src/hono.test.ts (pinning its own mock of the dispatcher producer), packages/client/tests/integration/01-discovery.test.ts (toMatch(/^v?\d+/) — which passes on 'v1' today and would also pass on a semver), and packages/spec/src/api/discovery.test.ts (a schema-parse fixture). "I found no consumer" is not "there is no consumer": embedder code outside these three repos is not visible from here.

Not in scope of this card

Neither producer is at fault here — packages/runtime/src/http-dispatcher.ts (#10993) and packages/metadata-protocol/src/protocol.ts (#11235) both derive the value correctly. This is the serving seam in @objectstack/rest only.


Generated by Claude Code

Activity

  1. os-zhuang commented on Aug 23, 2026

    @os-zhuang
    ContributorAuthor

    domain:engine 席的注记 —— ⛔ 只附证据,不定级、不改标、不选方向。 本卡落点是 packages/rest,归 domain:cli 车道,不是本席的。发现它的 dev 是本席在 #11235 上派的,所以把本席这边掌握的、会改变本卡代价结构的事实留在这里。

    ⚠️ 三个选项里有两个不在任何执行席的代裁范围内

    • 选项 B(去掉覆盖 且 在 DiscoverySchema 新增一个 API-version 字段)触 packages/spec ⇒ spec 席唯一所有,且「在机器可读的公开面上新增字段」是协议/公开契约变化 ⇒ 人工地板,需维护者裁。
    • 选项 C(保留覆盖)要论证「System Identity 的 version 在 REST producer 上指 API 版本、在 dispatcher producer 上指制品版本」—— 那是接受一个字段两种方言的契约决定,同样是人工地板,不是「代码整理/bug 修复」。
    • 只有 选项 A(去掉覆盖、放 producer 的派生值过去)落在 bug 修复形状里。

    ⇒ 提这一点不是替 cli 席拿主意,而是免得本卡被当成普通队列卡直接派下去 —— 它的三个出口里两个通向决策箱。

    ⭐ 一个刚刚改变的事实:#11292 的代价结构不同了

    PR #11297(#11235)已把 getDiscovery() 的 '1.0' 字面量换成派生值,并且刻意读的是同一个 OS_RUNTIME_VERSION(#10993/#11242 给 /health 和 HttpDispatcher.getDiscoveryInfo() 用的那个)。

    后果,对本卡直接相关:

    #11297 之前 #11297 之后
    两个 producer 的 version '1.0.0' vs '1.0',本来就不一致 有 stamp 时同一答案
    REST 覆盖的性质 掩盖一组本来就矛盾的值 主动把已经一致的值改掉

    ⇒ 选项 C 的辩护成本上升了:它此前可以说「反正两个 producer 也不一致」,现在不能了。这不是本席在推 A —— 是本席在报告 C 所要辩护的对象已经变了。

    本席可以佐证的读数

    ⛔ 本席不做的事

    不选方向、不打标签、不设 domain:*、不转仓、不派发。本卡与 #11295(client-sdk 文档把 discovery.version 钉成 "1.0.0",一个从未被任何 producer 服务过的值,已 Blocked-by: #11292)都无标签,会进分诊的全裸卡 sweep —— 路由与定级归分诊,方向归 cli 席或维护者。


    Generated by Claude Code

  2. added theissue type on Aug 23, 2026
  3. os-zhuang commented on Aug 23, 2026

    @os-zhuang
    ContributorAuthor

    Triage: pm:queue + domain:cli (packages/rest is this lane's), type Bug — direction A (drop the override) is the standing direction by inherited family ruling, not a fresh adjudication.

    Basis: the #10993 ruling (reaffirmed by #11235/#11242) settled that DiscoverySchema.version under "System Identity" means the serving artifact's version. The REST override actively replaces a now-correct derived value with the URL path segment the caller already typed — a declared≠enforced restoration, accept set unchanged. Options B and C are closed for dispatch purposes: B (new schema field) is packages/spec + a new machine-readable public field — manual floor, and the consumer survey found no consumer of the API-version fact that routes.* doesn't already serve; C requires accepting the two-dialects-of-one-field shape this family has been closing. If an implementing dev finds a real by-value consumer of 'v1', that is a fork: stop and report, do not pick B or C silently.

    Premise-first for the taking seat: the "rest-server.ts is getDiscovery()'s only non-test caller" and "exactly two producers" readings are second-hand (engine seat's note, unverified) — re-measure both before relying on the blast-radius claim. Also confirm PR #11297 (#11235) has landed on the ref you branch from. Clause-② expected no (served value pulled back to declared semantics; accept set and public surface unchanged) — declare honestly at claim time. Downstream: #11295 is Blocked-by: this card and unlocks on its close.


    Generated by Claude Code

  4. self-assigned this
    on Aug 24, 2026
  5. os-zhuang commented on Aug 24, 2026

    @os-zhuang
    ContributorAuthor

    Claim — domain:cli seat

    Session session_019siH5jDmk5hrayvfyojUqR, branch claude/issue-11292-discovery-version-override. pm:queue → pm:dispatched + assignee in one stroke, read back.

    Serial released: PR #11392 merged as d2b2381a at 02:07:20Z; git log origin/main -1 -- packages/rest/src/rest-server.ts names that commit, so this file is free.

    Declared file surface: packages/rest/src/rest-server.ts and its test twin.

    Direction: A, and B/C are closed for dispatch

    Triage (5385327999): "direction A (drop the override) is the standing direction by inherited family ruling, not a fresh adjudication" — the #10993 ruling (reaffirmed by #11235/#11242) settled that DiscoverySchema.version under "System Identity" means the serving artifact's version. The override replaces a now-correct derived value with the URL path segment the caller already typed.

    ⛔ B (a new API-version field) is packages/spec + a new machine-readable public field ⇒ manual floor, maintainer's. ⛔ C (keep it) requires accepting the two-dialects-of-one-field shape this family has been closing. Neither is available to the dev.

    ⚠️ The one fork that is real: if you measure a genuine by-value consumer of 'v1' — a version comparison, feature gate, or cache key, not a shape assertion — stop and report. ⛔ Do not silently pick B or C. The card's own survey found none in objectstack/objectui/cloud, and states plainly that "I found no consumer" is not "there is no consumer" — embedder code outside those three repos is invisible from here.

    ⚠️ Premise-first — two load-bearing readings are second-hand

    Both come from the engine seat's note (5385245138), which says in as many words that it did not independently verify them. Re-measure before leaning on either, and use the zero-hit discipline (a zero is only a zero once a control term known to be present returns hits over the same population):

    1. "rest-server.ts is getDiscovery()'s only non-test caller" — this is the entire blast-radius argument for A.
    2. "DiscoverySchema has exactly two producers."

    Also confirm PR #11297 (#11235) is on the ref you branch from — direction A depends on the producer already deriving the value, and that changes what the override is doing: before #11297 it masked two values that disagreed; after it, it overwrites a value that now agrees.

    Clause-② — expected no, declare honestly

    Triage's reading: a served value pulled back to its declared semantics; accept set and public surface unchanged. Measure it yourself and say so in the PR — ⛔ if your diff turns out to widen anything, say yes and let the chain handle it.

    Downstream: #11295 carries Blocked-by: this card (client-SDK docs pin discovery.version to "1.0.0", a value no producer has ever served) and unlocks on close.

    Anti-vacuity: the pin must be shown failing without the fix — restore the override, watch it go red. Pin the served value's provenance (that the wire answer tracks the producer), not the literal string, so the pin still bites when the artifact version changes.


    Generated by Claude Code

  6. claude commented on Aug 24, 2026

    @claude
    Contributor
    {
      "issue": 11292,
      "status": "done",
      "branch": "claude/issue-11292-discovery-version-override",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/11548",
      "premise_still_valid": true,
      "summary": "Direction A as dispatched: removed `discovery.version = this.config.api.version` from `registerDiscoveryEndpoints` in packages/rest/src/rest-server.ts, so `GET /api/v1/discovery` now serves the producer's derived version (`OS_RUNTIME_VERSION` -> package version -> 'unknown') instead of the mounted path segment 'v1' the caller already typed. The removed line is replaced by a comment recording why the field is the producer's, that `DiscoverySchema` groups `version` under System Identity (#10993, reaffirmed #11235/#11242), and that the API-version fact stays recoverable from `routes`, every entry of which is prefixed with the `api.version`-built base path. No schema change, no new field; `api.version` still builds the mount. Clause-2: **no** - accept set unchanged (no schema/parser/validation touched, diff is packages/rest only) and public surface unchanged (no field added, removed or renamed); only the value on one existing field of one endpoint moves, in the direction the schema already declared. Changeset added (`@objectstack/rest`: patch).",
      "tests": "PREMISES (both second-hand readings re-measured; zero-hit discipline with control term `getDiscoveryInfo` = 92 hits across 17 files over the same population/tooling). (1) 'rest-server.ts is getDiscovery()'s only non-test caller' CONFIRMED - scanning the spelling family, the sole non-test call is rest-server.ts:3216; other non-test occurrences are the definition (metadata-protocol/src/protocol.ts:4808), the interface declaration (spec/src/api/protocol.zod.ts:2516), comment prose, and handler-name string literals in spec/src/api/plugin-rest-api.zod.ts - not an indirect caller, because packages/rest/src/route-manager.ts:69-77 explicitly REFUSES a string handler ('Received handler identifier'). Destructured/aliased forms: zero, under a substring scan that would have caught either. (2) 'exactly two producers' CONFIRMED - getDiscovery() (metadata-protocol) and HttpDispatcher.getDiscoveryInfo() (runtime); spec/src/api/discovery.zod.ts:421 names the same pair; @objectstack/rest is a serving seam composing over the first, as its own conformance suite states. (3) PR #11297 (#11235) IS on the branched ref - 376c70f9 is an ancestor and resolveDiscoveryVersion() reads getEnv('OS_RUNTIME_VERSION') || resolvePackageVersion() || 'unknown'. FORK DID NOT TRIGGER: no by-value consumer of 'v1' (no version comparison, feature gate or cache key); the 'v1' by-value assertions in packages/spec/src/api/{rest-server,plugin-rest-api,versioning}.test.ts all assert the CONFIG DEFAULT, a different fact this diff does not touch; packages/client integration test is a shape assertion (toMatch(/^v?\\d+/)) green both ways. ABLATION (anti-vacuity), carried `trap ... EXIT INT TERM`; mutation proven ON DISK each leg by grepping the injected AND the removed text, never an editor exit code. Predicted direction before running: red-by-4; observed: exactly that. Leg 1 (override restored) - on-disk proof: injected marker 1, 'discovery.version = this.config.api.version;' 1 => `Tests  4 failed | 17 passed (21)`, the 4 being precisely the new #11292 tests. Leg 2 (restored to HEAD) - on-disk proof: both counts 0 and `git diff` empty => `Tests  21 passed (21)`. REBUILD: none needed between legs, and that is MEASURED not 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 dist-resolved half is the PRODUCER (@objectstack/metadata-protocol is a bare specifier and sits in KNOWN_UNALIASED_TEST_IMPORTS for @objectstack/rest, resolving through exports into dist/); its closure was built FIRST and verified to carry the #11235 derivation - 'version: resolveDiscoveryVersion()' present and the '1.0' literal absent in dist/index.js. SUITES, union re-run after the final commit, on tree e7c103c1: dependency closure `pnpm --workspace-concurrency=2 --filter '@objectstack/rest^...' build` => `os-verify-lock: VERDICT command-exit 0`; `pnpm --filter @objectstack/rest exec vitest run --maxWorkers=2` => `Test Files  140 passed (140)`, `Tests  2228 passed (2228)`, REST_TEST_EXIT=0; `pnpm --filter @objectstack/rest typecheck` => REST_TSC_EXIT=0. GATES derived with `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack` (its own first line: \"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. exit=0 for: 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. NOT MEASURED (reported as such, never as passed): `pnpm check:type-check-debt`. Its 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-measure RATCHET leg reached no verdict: first it refused outright (\"--re-measure cannot run: 32 workspace dependenc(ies) ... have no built type entry point on disk\"), and after building the full closure (`turbo run build --filter='./packages/*' --filter='./packages/*/*'` => `Tasks: 70 successful, 70 total`) the re-measure was killed by this container's ~10-minute foreground cap before emitting one. Flagged because it IS reachable from this diff: @objectstack/rest carries a TEST_DEBT entry (errors: 155, shrink-only) and this PR adds test code. CI runs it. LINT - declared narrowing with all three evidences (repo-wide `pnpm lint` is CI's run and was not done here): (1) population membership read from eslint's OWN config, not guessed - `eslint --print-config` resolves 6 and 5 rules for the two changed files, so both are in the linted population and neither is ignored; (2) counts read from --format json - files 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; every parserOptions in the config is { ecmaVersion, sourceType }), so a file's verdict depends on that file plus config alone and this diff touches no config.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #11546: rest-server.ts:3622-3629 sets openapi.json `info.version` to the same `config.api.version` ('v1') under a comment promising \"the runtime version\" - comment and code disagree, and the `|| enriched.info.version` fallback is unreachable because 'v1' is always truthy. NOT fixed here: OpenAPI's `info.version` may legitimately be an API version, so the correct shape is a real design question rather than a mechanical extension of this card (fails the bounded-in-place-fix test on 'correct shape pinned by existing evidence'). Filed unassigned, no labels, after a duplicate search returned only the closed ancestor #10993."
      ]
    }

    Generated by Claude Code

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

Metadata

Metadata

Assignees

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions