From 692f7fb15b89102706c0d8788a7324bccb2317e6 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 10:14:13 +0000 Subject: [PATCH 1/3] test(client): probe metadata.prefix against the scoped SDK surface (#16675) The card's mandated first step: reuse #14879's data-prefix fixture with metadata.prefix in place of crud.dataPrefix, and record the reading before touching the implementation. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8 --- .../client/src/client.metadata-prefix.test.ts | 263 ++++++++++++++++++ 1 file changed, 263 insertions(+) create mode 100644 packages/client/src/client.metadata-prefix.test.ts diff --git a/packages/client/src/client.metadata-prefix.test.ts b/packages/client/src/client.metadata-prefix.test.ts new file mode 100644 index 0000000000..9d2acaf75b --- /dev/null +++ b/packages/client/src/client.metadata-prefix.test.ts @@ -0,0 +1,263 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `metadata.prefix` is honoured by the SDK, not restated by it (#16675). + * + * THE CONTRACT. `metadata.prefix` is a live `RestServerConfig` key, the exact + * sibling of the `crud.dataPrefix` #14879 fixed: REST mounts every metadata + * route under `metaPath = ${basePath}${metadata.prefix}` and the discovery + * handler advertises the same value as + * `routes.metadata = ${realBase}${metadata.prefix}`. Three surfaces describe + * one set of paths — the mounts, the discovery document, and this SDK — so the + * SDK must READ the value rather than restate it. + * + * WHAT WAS WRONG. The SDK's scoped surface restated `/meta` as a literal in + * all SIX of its metadata methods (`getTypes` / `getItems` / `getItem` / + * `saveItem` / `deleteItem` / `getHistory`), so on a deployment that moved the + * prefix it called paths the server does not mount. The unscoped twin of each + * of those methods was already correct — it builds + * `${baseUrl}${getRoute('metadata')}` — so ONE SDK disagreed with itself: the + * unscoped half read the advertised value while the scoped half guessed. + * + * WHY THE FIXTURE CREATES THE CONDITION. Measured on `origin/main`, no in-repo + * caller sets a non-default `metadata.prefix`, so no existing fixture + * exercises this and nothing in the tree is broken today; the exposure is + * external deployments. So this suite BOOTS a server on a non-default prefix + * rather than looking for one. + * + * WHY A LIVE SERVER AND A RECORDED URL. A mock that answers 200 to whatever it + * is asked cannot tell a mounted path from an unmounted one — it would go + * green against the very bug this pins. So the server is real, and the suite + * asserts BOTH halves of the claim: that the URL the client puts on the wire + * is the one the server actually mounts, and (`serves nothing at /meta`) that + * the old hard-coded path is genuinely dead on this deployment, which is what + * makes the first assertion mean something. + * + * ⚠️ EVERY URL ASSERTION IS FULL-STRING EQUALITY, NEVER `toContain`. The + * realistic non-default prefix `/metadata` CONTAINS the conventional `/meta` + * as a prefix, so `expect(url).not.toContain('/meta')` would fail on the + * CORRECT url and `toContain('/meta')` would pass on it — a substring probe + * over this pair of values answers noise. Equality is immune to that, and it + * is also what the negative control below needs to mean anything. + * + * THE NEGATIVE CONTROL (the acceptance criterion of #16675). The same drive + * runs against a default-prefix server built by the same helper, and against a + * client that never connected at all. It is what distinguishes "the SDK + * follows the advertised prefix" from "the SDK now always rebuilds its paths + * out of discovery": a default deployment must be reached at exactly the URLs + * it was reached at before, byte for byte, and an unconnected client must + * still produce them WITHOUT putting a discovery round-trip on the wire. An + * implementation that always derives from discovery passes the non-default + * case above while making every default deployment slower and more fragile — + * these two describes are the only thing that catches it. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { LiteKernel } from '@objectstack/core'; +import { ObjectQL, ObjectQLPlugin } from '@objectstack/objectql'; +import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm'; +import { HonoServerPlugin } from '@objectstack/plugin-hono-server'; +import { createRestApiPlugin } from '@objectstack/runtime'; +import { ObjectStackClient } from './index'; +import type { IHttpServer } from '@objectstack/spec/contracts'; + +const ENV_ID = 'proj-alpha'; +/** + * Deliberately the value `packages/spec`'s own `MetadataEndpointsConfigSchema` + * test uses for "custom prefix" — and deliberately one that has the + * conventional `/meta` as a string prefix, so the substring trap above is + * exercised rather than side-stepped. + */ +const CUSTOM_PREFIX = '/metadata'; + +interface Fixture { + baseUrl: string; + kernel: LiteKernel; +} + +/** + * One boot recipe, two prefixes — so the non-default case and the control + * differ in exactly the key under test and nothing else. + */ +async function bootServer(metadataPrefix?: string): Promise { + const kernel = new LiteKernel(); + kernel.use(new ObjectQLPlugin()); + // Same reason as the sibling scoping suite (#3963): the anonymous-deny + // gate is unconditional, so a live-server client suite needs a session. + kernel.use({ + metadata: { name: 'test-auth', version: '1.0.0' }, + async init(ctx: any) { + ctx.registerService('auth', { + api: { getSession: async () => ({ user: { id: 'test-user' } }) }, + }); + }, + } as any); + + const honoPlugin = new HonoServerPlugin({ port: 0 }); + kernel.use(honoPlugin); + + kernel.use( + createRestApiPlugin({ + api: { + api: { + // Routing test, no auth stack mounted (ADR-0056 D2). + requireAuth: false, + enableProjectScoping: true, + projectResolution: 'auto', + } as any, + // The key under test. Omitted entirely for the control, so the + // control runs the schema's own `.default('/meta')` rather + // than a second literal written here. + ...(metadataPrefix ? { metadata: { prefix: metadataPrefix } as any } : {}), + }, + }), + ); + + await kernel.bootstrap(); + + const ql = kernel.getService('objectql'); + ql.registerDriver(new SqliteWasmDriver({ filename: ':memory:' }) as never, true); + ql.registerObject({ + name: 'task', + label: 'Task', + fields: { title: { type: 'text', label: 'Title' } }, + }); + // Registered after bootstrap, so nothing has issued the DDL yet (#4065). + await ql.syncObjectSchema('task'); + + const httpServer = kernel.getService('http.server'); + const port = httpServer.getPort!(); + return { baseUrl: `http://localhost:${port}`, kernel }; +} + +async function shutdown(fixture: Fixture | undefined): Promise { + if (!fixture?.kernel) return; + await Promise.race([ + fixture.kernel.shutdown(), + new Promise((resolve) => setTimeout(resolve, 10_000)), + ]); +} + +/** + * A client whose every request URL is recorded. The recorder DELEGATES to the + * real fetch, so the recorded URL and the server's real answer are the same + * exchange — the assertion cannot pass on a URL that was never served. + */ +function recordingClient(baseUrl: string): { client: ObjectStackClient; urls: string[] } { + const urls: string[] = []; + const client = new ObjectStackClient({ + baseUrl, + fetch: (input: RequestInfo | URL, init?: RequestInit) => { + urls.push(typeof input === 'string' ? input : String(input)); + return globalThis.fetch(input as any, init); + }, + } as any); + return { client, urls }; +} + +/** + * Drive all SIX scoped metadata methods and return the URL each one put on the + * wire, in call order. The writes are allowed to reject — a 404 on an + * unmounted path and a refusal from a live write door are both rejections, and + * this suite is about the URL, not the answer. Recording happens before the + * request is made, so a rejected call still contributes its URL. + */ +async function driveAllSix(client: ObjectStackClient, urls: string[]): Promise { + const scoped = client.environment(ENV_ID); + const before = urls.length; + const swallow = (p: Promise) => p.then(() => undefined, () => undefined); + await swallow(scoped.meta.getTypes()); + await swallow(scoped.meta.getItems('object')); + await swallow(scoped.meta.getItem('object', 'task')); + await swallow(scoped.meta.saveItem('object', 'task', { name: 'task', label: 'Task' })); + await swallow(scoped.meta.deleteItem('object', 'task')); + await swallow(scoped.meta.getHistory('object', 'task')); + return urls.slice(before); +} + +/** The six URLs a deployment on `prefix` must be called at. */ +function expectedSix(baseUrl: string, prefix: string): string[] { + const root = `${baseUrl}/api/v1/environments/${ENV_ID}${prefix}`; + return [ + `${root}`, + `${root}/object`, + `${root}/object/task`, + `${root}/object/task`, + `${root}/object/task`, + `${root}/object/task/history`, + ]; +} + +describe('SDK honours metadata.prefix (#16675)', () => { + describe(`non-default prefix (${CUSTOM_PREFIX})`, () => { + let fx: Fixture; + + beforeAll(async () => { fx = await bootServer(CUSTOM_PREFIX); }, 30_000); + afterAll(async () => { await shutdown(fx); }, 30_000); + + it('mounts scoped metadata under the configured prefix, and serves nothing at /meta', async () => { + const mounted = await fetch(`${fx.baseUrl}/api/v1/environments/${ENV_ID}${CUSTOM_PREFIX}`); + expect(mounted.status).toBe(200); + + // The half that makes this fixture worth anything: the path the + // SDK used to hard-code is genuinely not mounted here. + const hardCoded = await fetch(`${fx.baseUrl}/api/v1/environments/${ENV_ID}/meta`); + expect(hardCoded.status).toBe(404); + }); + + it('advertises the prefix on the discovery document', async () => { + const res = await fetch(`${fx.baseUrl}/api/v1/discovery`); + expect(res.status).toBe(200); + const body = await res.json(); + const routes = (body?.data ?? body)?.routes; + expect(routes?.metadata).toBe(`/api/v1${CUSTOM_PREFIX}`); + }); + + it('all six scoped meta methods call the mounted path, not /meta', async () => { + const { client, urls } = recordingClient(fx.baseUrl); + await client.connect(); + + const called = await driveAllSix(client, urls); + expect(called).toEqual(expectedSix(fx.baseUrl, CUSTOM_PREFIX)); + }); + }); + + describe('negative control — default prefix, byte-identical URLs', () => { + let fx: Fixture; + + beforeAll(async () => { fx = await bootServer(); }, 30_000); + afterAll(async () => { await shutdown(fx); }, 30_000); + + it('advertises /api/v1/meta', async () => { + const res = await fetch(`${fx.baseUrl}/api/v1/discovery`); + const body = await res.json(); + const routes = (body?.data ?? body)?.routes; + expect(routes?.metadata).toBe('/api/v1/meta'); + }); + + it('a CONNECTED client still calls the six /meta URLs, byte for byte', async () => { + const { client, urls } = recordingClient(fx.baseUrl); + await client.connect(); + + const called = await driveAllSix(client, urls); + expect(called).toEqual(expectedSix(fx.baseUrl, '/meta')); + }); + + it('an UNCONNECTED client calls the same six URLs and puts NO discovery request on the wire', async () => { + // No `connect()`, so there is no advertised document to read. The + // derivation must decline to the conventional literal rather than + // reach for one -- this is the leg that fails on an + // "always rebuild the path out of discovery" implementation, which + // would make every default deployment pay a round-trip it does not + // pay today. + const { client, urls } = recordingClient(fx.baseUrl); + + const called = await driveAllSix(client, urls); + expect(called).toEqual(expectedSix(fx.baseUrl, '/meta')); + + // Exactly six requests, and none of them is a discovery read. + expect(urls).toHaveLength(6); + expect(urls.filter((u) => u.includes('/discovery'))).toEqual([]); + }); + }); +}); From 3c5c0602f8a6c994807d1367ce956b03b47720ae Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 10:16:29 +0000 Subject: [PATCH 2/3] fix(client): scoped SDK reads metadata.prefix off the advertised routes (#16675) Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8 --- packages/client/src/index.ts | 127 +++++++++++++++++++++++++++++++++-- 1 file changed, 121 insertions(+), 6 deletions(-) diff --git a/packages/client/src/index.ts b/packages/client/src/index.ts index 1ba8bd165c..a69454eac1 100644 --- a/packages/client/src/index.ts +++ b/packages/client/src/index.ts @@ -1426,6 +1426,18 @@ export interface OrganizationTeamMemberRemovedReceipt { */ const DEFAULT_DATA_PREFIX = '/data'; +/** + * The conventional metadata prefix — `MetadataEndpointsConfigSchema.prefix` + * in `packages/spec` declares `.default('/meta')`, and REST mounts every + * metadata route under `${basePath}${metadata.prefix}`. + * + * The exact sibling of {@link DEFAULT_DATA_PREFIX}, and the same kind of + * value: what the SDK falls back to when discovery has not told it otherwise, + * NOT a competing source of truth. `_metaPrefix()` prefers the advertised + * value in every case where it can read one. + */ +const DEFAULT_META_PREFIX = '/meta'; + export class ObjectStackClient { private baseUrl: string; private token?: string; @@ -3176,6 +3188,83 @@ export class ObjectStackClient { return derived.length > 1 ? derived : DEFAULT_DATA_PREFIX; } + /** + * @internal The metadata prefix this client's server actually mounts, read + * off the advertised routes (#16675). + * + * The same defect as #14879 one key over, so deliberately the same + * derivation shape as {@link ObjectStackClient._dataPrefix}, fallback + * discipline included. `metadata.prefix` moves the mounted metadata paths + * and the advertised discovery document TOGETHER — REST builds every + * metadata route as `${basePath}${metadata.prefix}` and then advertises the + * same value as `routes.metadata = ${realBase}${metadata.prefix}`. The SDK + * is the third surface that has to describe those same paths, so it must + * read the value rather than restate it: a deployment on a non-default + * prefix mounts nothing at `/meta`, and a client that assumes `/meta` calls + * paths that do not exist. + * + * The advertised `routes.metadata` is `{realBase}{metadata.prefix}` — ONE + * string carrying TWO unknowns, and no discovery key carries either half + * alone. The split is recovered in the order below, and where it cannot be + * recovered this DECLINES to the conventional `/meta` rather than guess. An + * SDK must not become unusable because a server's discovery document is + * missing a key: + * + * 1. If the advertised value already ends with the conventional `/meta`, + * that IS the prefix. Taking this first is what makes the change + * incapable of regressing a deployment that works today: every rule + * below can only run in the branch where the current code — which + * knows the single literal `/meta` — is ALREADY wrong. It is also what + * keeps a DEFAULT deployment free of any new dependency: the answer is + * reached from `routes.metadata` alone, and an unconnected client + * never reaches a rule at all. + * 2. Otherwise `routes.data` supplies the missing equation. It is + * `{realBase}{crud.dataPrefix}` over the SAME `realBase` (both are + * substituted from one `realBase` in the same discovery handler), so + * the two advertised routes share exactly `realBase` plus whatever + * their two prefixes happen to share. Cutting their common run back to + * its last `/` therefore lands on the `realBase` boundary, and the + * remainder of `routes.metadata` is the prefix. This is also correct + * when the document was served from the environment-scoped mount, + * where both routes carry the same `/environments/{id}` segment and it + * simply becomes part of the shared run. + * + * A derived prefix of `/` or empty is not a prefix this understands, so it + * declines too, as does the case where the two routes share nothing but the + * leading `/` and therefore share no base at all. The one shape that + * survives all of it — a deployment that moved `metadata.prefix` off + * `/meta` AND whose `routes.data` is not substituted from the same base + * (i.e. `api.enableCrud` is off) — is one the SDK cannot serve today + * either; it keeps today's answer. + */ + _metaPrefix(): string { + const meta = this.discoveryInfo?.routes?.metadata; + if (typeof meta !== 'string' || !meta) return DEFAULT_META_PREFIX; + + // (1) The conventional prefix — today's entire rule, kept first. + if (meta.endsWith(DEFAULT_META_PREFIX)) return DEFAULT_META_PREFIX; + + // (2) `routes.data` as the second equation over the same `realBase`. + const data = this.discoveryInfo?.routes?.data; + if (typeof data !== 'string' || !data || data === meta) return DEFAULT_META_PREFIX; + + let shared = 0; + while (shared < meta.length && shared < data.length + && meta.charCodeAt(shared) === data.charCodeAt(shared)) shared++; + + // `boundary === 0` means the two advertised routes have nothing in common + // but the leading `/` — so they do NOT share a `realBase`, and the whole + // of `routes.metadata` would be mistaken for the prefix. That happens when + // `routes.data` was never substituted from this deployment's base (its + // endpoints are off, so it still carries the conventional literal) while + // `routes.metadata` was. Decline: a wrong prefix is worse than today's. + const boundary = meta.lastIndexOf('/', shared - 1); + if (boundary <= 0) return DEFAULT_META_PREFIX; + + const derived = meta.slice(boundary); + return derived.length > 1 ? derived : DEFAULT_META_PREFIX; + } + /** * @internal The unscoped API base this client's server actually serves, * derived from the advertised routes (#6714 face 3). @@ -6869,19 +6958,45 @@ export class ScopedEnvironmentClient { return `${this.parent._baseUrl()}${this.scope()}${this.parent._dataPrefix()}${suffix}`; } + /** + * URL for a route mounted under the deployment's metadata prefix (#16675). + * + * The exact sibling of {@link ScopedEnvironmentClient.dataUrl}, for the + * other half of the same defect. Every route reached through here is + * mounted by REST as `${metaPath}/...` with + * `metaPath = ${basePath}${metadata.prefix}`, so the prefix is deployment + * state, not a constant. The unscoped twin of each of these methods already + * reads it — it builds `${baseUrl}${getRoute('metadata')}` and + * `routes.metadata` IS `{realBase}{metadata.prefix}`. This surface restated + * `/meta` as a literal instead, so on a deployment that moved + * `metadata.prefix` the scoped half of one SDK called paths the server does + * not mount while the unscoped half of the same SDK called the right ones. + * + * The scoped form cannot consume `routes.metadata` verbatim the way the + * unscoped form does: the environment segment goes BETWEEN the API base and + * the prefix (`{base}/environments/{id}{metadata.prefix}`), and the id is + * this client's, which need not be the one discovery resolved. So the two + * halves are taken separately — `_apiBase()` for the base, `_metaPrefix()` + * for the prefix — and both decline to today's conventions when the + * advertised document does not determine them. + */ + private metaUrl(suffix: string): string { + return `${this.parent._baseUrl()}${this.scope()}${this.parent._metaPrefix()}${suffix}`; + } + /** * Metadata operations scoped to this project. */ meta = { getTypes: async (): Promise => { - const res = await this.parent._fetch(this.url('/meta')); + const res = await this.parent._fetch(this.metaUrl('')); return this.parent._unwrap(res); }, getItems: async (type: string, options?: { packageId?: string }): Promise => { const params = new URLSearchParams(); if (options?.packageId) params.set('package', options.packageId); const qs = params.toString(); - const res = await this.parent._fetch(this.url(`/meta/${type}${qs ? `?${qs}` : ''}`)); + const res = await this.parent._fetch(this.metaUrl(`/${type}${qs ? `?${qs}` : ''}`)); return this.parent._unwrap(res); }, /** Same `{ type, name, item }` envelope as the unscoped surface (#5563). */ @@ -6889,7 +7004,7 @@ export class ScopedEnvironmentClient { const params = new URLSearchParams(); if (options?.packageId) params.set('package', options.packageId); const qs = params.toString(); - const res = await this.parent._fetch(this.url(`/meta/${encodeURIComponent(type)}/${encodeURIComponent(name)}${qs ? `?${qs}` : ''}`)); + const res = await this.parent._fetch(this.metaUrl(`/${encodeURIComponent(type)}/${encodeURIComponent(name)}${qs ? `?${qs}` : ''}`)); return this.parent._unwrap(res); }, /** @@ -6916,7 +7031,7 @@ export class ScopedEnvironmentClient { // Header half of the same bag, through the same one builder the twin // calls — see {@link metaSaveHeaders}. const headers = metaSaveHeaders(options); - const res = await this.parent._fetch(this.url(`/meta/${encodeURIComponent(type)}/${encodeURIComponent(name)}${query}`), { + const res = await this.parent._fetch(this.metaUrl(`/${encodeURIComponent(type)}/${encodeURIComponent(name)}${query}`), { method: 'PUT', body: JSON.stringify(item), ...(headers ? { headers } : {}), @@ -6951,7 +7066,7 @@ export class ScopedEnvironmentClient { // Header half of the same bag, through the same one builder the twin // calls — see {@link metaDeleteHeaders}. const headers = metaDeleteHeaders(options); - const res = await this.parent._fetch(this.url(`/meta/${encodeURIComponent(type)}/${encodeURIComponent(name)}${query}`), { + const res = await this.parent._fetch(this.metaUrl(`/${encodeURIComponent(type)}/${encodeURIComponent(name)}${query}`), { method: 'DELETE', ...(headers ? { headers } : {}), }); @@ -6983,7 +7098,7 @@ export class ScopedEnvironmentClient { if (options?.limit !== undefined) params.set('limit', String(options.limit)); const qs = params.toString(); const res = await this.parent._fetch( - this.url(`/meta/${encodeURIComponent(type)}/${encodeURIComponent(name)}/history${qs ? `?${qs}` : ''}`), + this.metaUrl(`/${encodeURIComponent(type)}/${encodeURIComponent(name)}/history${qs ? `?${qs}` : ''}`), ); return this.parent._unwrap(res); }, From 772ef3519ccb56b15b5db9b39e0b40d9ce54c958 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 10:48:56 +0000 Subject: [PATCH 3/3] chore: changeset for the scoped metadata.prefix fix (#16675) Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8 --- .../scoped-sdk-honours-metadata-prefix.md | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 .changeset/scoped-sdk-honours-metadata-prefix.md diff --git a/.changeset/scoped-sdk-honours-metadata-prefix.md b/.changeset/scoped-sdk-honours-metadata-prefix.md new file mode 100644 index 0000000000..590236786a --- /dev/null +++ b/.changeset/scoped-sdk-honours-metadata-prefix.md @@ -0,0 +1,40 @@ +--- +'@objectstack/client': patch +--- + +fix(client): the scoped SDK reads `metadata.prefix` off the advertised routes instead of restating `/meta` + +`metadata.prefix` is a live `RestServerConfig` key: REST mounts every metadata +route under `metaPath = ${basePath}${metadata.prefix}` and the discovery handler +advertises the same value as `routes.metadata = ${realBase}${metadata.prefix}`. +Three surfaces describe one set of paths — the mounts, the discovery document, +and this SDK. + +`ScopedEnvironmentClient` restated `/meta` as a literal in all six of its +metadata methods — `getTypes`, `getItems`, `getItem`, `saveItem`, `deleteItem`, +`getHistory` — so on a deployment that moved the prefix, every one of them +called a path the server does not mount. The unscoped twin of each method was +already correct (it builds `${baseUrl}${getRoute('metadata')}`), so one SDK +disagreed with itself: the unscoped half read the advertised value while the +scoped half guessed. Measured on a live server booted at +`metadata: { prefix: '/metadata' }`, all six went to +`/api/v1/environments//meta`, which that deployment answers 404. + +The six now build through `metaUrl()`, which takes its base from `_apiBase()` +and its prefix from the new `_metaPrefix()` — the exact sibling of the +`_dataPrefix()` derivation that fixed `crud.dataPrefix`, fallback discipline +included. `_metaPrefix()` prefers the advertised `routes.metadata`, recovers the +prefix from `routes.data` as a second equation over the same `realBase` when the +advertised value is not the conventional one, and **declines to `/meta`** +whenever the document does not determine the answer: an SDK must not become +unusable because a server's discovery document is missing a key. + +Deployments on the default prefix are unaffected, by construction and by +measurement: the conventional-suffix rule is taken first, so a default +deployment is answered from `routes.metadata` alone, and a client that never +connected never reaches a rule at all. The pinned negative control asserts the +six request URLs of a default deployment byte for byte, for a connected client +and for an unconnected one, and that the unconnected client puts no discovery +request on the wire. + +The unscoped metadata methods are untouched.