Skip to content

Commit 032452a

Browse files
claude[bot]claude
andauthored
fix(client): the scoped SDK reads metadata.prefix off the advertised routes instead of restating /meta (#17122)
* 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8 * fix(client): scoped SDK reads metadata.prefix off the advertised routes (#16675) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8 * chore: changeset for the scoped metadata.prefix fix (#16675) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9c8b497 commit 032452a

3 files changed

Lines changed: 424 additions & 6 deletions

File tree

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
---
2+
'@objectstack/client': patch
3+
---
4+
5+
fix(client): the scoped SDK reads `metadata.prefix` off the advertised routes instead of restating `/meta`
6+
7+
`metadata.prefix` is a live `RestServerConfig` key: REST mounts every metadata
8+
route under `metaPath = ${basePath}${metadata.prefix}` and the discovery handler
9+
advertises the same value as `routes.metadata = ${realBase}${metadata.prefix}`.
10+
Three surfaces describe one set of paths — the mounts, the discovery document,
11+
and this SDK.
12+
13+
`ScopedEnvironmentClient` restated `/meta` as a literal in all six of its
14+
metadata methods — `getTypes`, `getItems`, `getItem`, `saveItem`, `deleteItem`,
15+
`getHistory` — so on a deployment that moved the prefix, every one of them
16+
called a path the server does not mount. The unscoped twin of each method was
17+
already correct (it builds `${baseUrl}${getRoute('metadata')}`), so one SDK
18+
disagreed with itself: the unscoped half read the advertised value while the
19+
scoped half guessed. Measured on a live server booted at
20+
`metadata: { prefix: '/metadata' }`, all six went to
21+
`/api/v1/environments/<id>/meta`, which that deployment answers 404.
22+
23+
The six now build through `metaUrl()`, which takes its base from `_apiBase()`
24+
and its prefix from the new `_metaPrefix()` — the exact sibling of the
25+
`_dataPrefix()` derivation that fixed `crud.dataPrefix`, fallback discipline
26+
included. `_metaPrefix()` prefers the advertised `routes.metadata`, recovers the
27+
prefix from `routes.data` as a second equation over the same `realBase` when the
28+
advertised value is not the conventional one, and **declines to `/meta`**
29+
whenever the document does not determine the answer: an SDK must not become
30+
unusable because a server's discovery document is missing a key.
31+
32+
Deployments on the default prefix are unaffected, by construction and by
33+
measurement: the conventional-suffix rule is taken first, so a default
34+
deployment is answered from `routes.metadata` alone, and a client that never
35+
connected never reaches a rule at all. The pinned negative control asserts the
36+
six request URLs of a default deployment byte for byte, for a connected client
37+
and for an unconnected one, and that the unconnected client puts no discovery
38+
request on the wire.
39+
40+
The unscoped metadata methods are untouched.
Lines changed: 263 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,263 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* `metadata.prefix` is honoured by the SDK, not restated by it (#16675).
5+
*
6+
* THE CONTRACT. `metadata.prefix` is a live `RestServerConfig` key, the exact
7+
* sibling of the `crud.dataPrefix` #14879 fixed: REST mounts every metadata
8+
* route under `metaPath = ${basePath}${metadata.prefix}` and the discovery
9+
* handler advertises the same value as
10+
* `routes.metadata = ${realBase}${metadata.prefix}`. Three surfaces describe
11+
* one set of paths — the mounts, the discovery document, and this SDK — so the
12+
* SDK must READ the value rather than restate it.
13+
*
14+
* WHAT WAS WRONG. The SDK's scoped surface restated `/meta` as a literal in
15+
* all SIX of its metadata methods (`getTypes` / `getItems` / `getItem` /
16+
* `saveItem` / `deleteItem` / `getHistory`), so on a deployment that moved the
17+
* prefix it called paths the server does not mount. The unscoped twin of each
18+
* of those methods was already correct — it builds
19+
* `${baseUrl}${getRoute('metadata')}` — so ONE SDK disagreed with itself: the
20+
* unscoped half read the advertised value while the scoped half guessed.
21+
*
22+
* WHY THE FIXTURE CREATES THE CONDITION. Measured on `origin/main`, no in-repo
23+
* caller sets a non-default `metadata.prefix`, so no existing fixture
24+
* exercises this and nothing in the tree is broken today; the exposure is
25+
* external deployments. So this suite BOOTS a server on a non-default prefix
26+
* rather than looking for one.
27+
*
28+
* WHY A LIVE SERVER AND A RECORDED URL. A mock that answers 200 to whatever it
29+
* is asked cannot tell a mounted path from an unmounted one — it would go
30+
* green against the very bug this pins. So the server is real, and the suite
31+
* asserts BOTH halves of the claim: that the URL the client puts on the wire
32+
* is the one the server actually mounts, and (`serves nothing at /meta`) that
33+
* the old hard-coded path is genuinely dead on this deployment, which is what
34+
* makes the first assertion mean something.
35+
*
36+
* ⚠️ EVERY URL ASSERTION IS FULL-STRING EQUALITY, NEVER `toContain`. The
37+
* realistic non-default prefix `/metadata` CONTAINS the conventional `/meta`
38+
* as a prefix, so `expect(url).not.toContain('/meta')` would fail on the
39+
* CORRECT url and `toContain('/meta')` would pass on it — a substring probe
40+
* over this pair of values answers noise. Equality is immune to that, and it
41+
* is also what the negative control below needs to mean anything.
42+
*
43+
* THE NEGATIVE CONTROL (the acceptance criterion of #16675). The same drive
44+
* runs against a default-prefix server built by the same helper, and against a
45+
* client that never connected at all. It is what distinguishes "the SDK
46+
* follows the advertised prefix" from "the SDK now always rebuilds its paths
47+
* out of discovery": a default deployment must be reached at exactly the URLs
48+
* it was reached at before, byte for byte, and an unconnected client must
49+
* still produce them WITHOUT putting a discovery round-trip on the wire. An
50+
* implementation that always derives from discovery passes the non-default
51+
* case above while making every default deployment slower and more fragile —
52+
* these two describes are the only thing that catches it.
53+
*/
54+
55+
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
56+
import { LiteKernel } from '@objectstack/core';
57+
import { ObjectQL, ObjectQLPlugin } from '@objectstack/objectql';
58+
import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm';
59+
import { HonoServerPlugin } from '@objectstack/plugin-hono-server';
60+
import { createRestApiPlugin } from '@objectstack/runtime';
61+
import { ObjectStackClient } from './index';
62+
import type { IHttpServer } from '@objectstack/spec/contracts';
63+
64+
const ENV_ID = 'proj-alpha';
65+
/**
66+
* Deliberately the value `packages/spec`'s own `MetadataEndpointsConfigSchema`
67+
* test uses for "custom prefix" — and deliberately one that has the
68+
* conventional `/meta` as a string prefix, so the substring trap above is
69+
* exercised rather than side-stepped.
70+
*/
71+
const CUSTOM_PREFIX = '/metadata';
72+
73+
interface Fixture {
74+
baseUrl: string;
75+
kernel: LiteKernel;
76+
}
77+
78+
/**
79+
* One boot recipe, two prefixes — so the non-default case and the control
80+
* differ in exactly the key under test and nothing else.
81+
*/
82+
async function bootServer(metadataPrefix?: string): Promise<Fixture> {
83+
const kernel = new LiteKernel();
84+
kernel.use(new ObjectQLPlugin());
85+
// Same reason as the sibling scoping suite (#3963): the anonymous-deny
86+
// gate is unconditional, so a live-server client suite needs a session.
87+
kernel.use({
88+
metadata: { name: 'test-auth', version: '1.0.0' },
89+
async init(ctx: any) {
90+
ctx.registerService('auth', {
91+
api: { getSession: async () => ({ user: { id: 'test-user' } }) },
92+
});
93+
},
94+
} as any);
95+
96+
const honoPlugin = new HonoServerPlugin({ port: 0 });
97+
kernel.use(honoPlugin);
98+
99+
kernel.use(
100+
createRestApiPlugin({
101+
api: {
102+
api: {
103+
// Routing test, no auth stack mounted (ADR-0056 D2).
104+
requireAuth: false,
105+
enableProjectScoping: true,
106+
projectResolution: 'auto',
107+
} as any,
108+
// The key under test. Omitted entirely for the control, so the
109+
// control runs the schema's own `.default('/meta')` rather
110+
// than a second literal written here.
111+
...(metadataPrefix ? { metadata: { prefix: metadataPrefix } as any } : {}),
112+
},
113+
}),
114+
);
115+
116+
await kernel.bootstrap();
117+
118+
const ql = kernel.getService<ObjectQL>('objectql');
119+
ql.registerDriver(new SqliteWasmDriver({ filename: ':memory:' }) as never, true);
120+
ql.registerObject({
121+
name: 'task',
122+
label: 'Task',
123+
fields: { title: { type: 'text', label: 'Title' } },
124+
});
125+
// Registered after bootstrap, so nothing has issued the DDL yet (#4065).
126+
await ql.syncObjectSchema('task');
127+
128+
const httpServer = kernel.getService<IHttpServer>('http.server');
129+
const port = httpServer.getPort!();
130+
return { baseUrl: `http://localhost:${port}`, kernel };
131+
}
132+
133+
async function shutdown(fixture: Fixture | undefined): Promise<void> {
134+
if (!fixture?.kernel) return;
135+
await Promise.race([
136+
fixture.kernel.shutdown(),
137+
new Promise<void>((resolve) => setTimeout(resolve, 10_000)),
138+
]);
139+
}
140+
141+
/**
142+
* A client whose every request URL is recorded. The recorder DELEGATES to the
143+
* real fetch, so the recorded URL and the server's real answer are the same
144+
* exchange — the assertion cannot pass on a URL that was never served.
145+
*/
146+
function recordingClient(baseUrl: string): { client: ObjectStackClient; urls: string[] } {
147+
const urls: string[] = [];
148+
const client = new ObjectStackClient({
149+
baseUrl,
150+
fetch: (input: RequestInfo | URL, init?: RequestInit) => {
151+
urls.push(typeof input === 'string' ? input : String(input));
152+
return globalThis.fetch(input as any, init);
153+
},
154+
} as any);
155+
return { client, urls };
156+
}
157+
158+
/**
159+
* Drive all SIX scoped metadata methods and return the URL each one put on the
160+
* wire, in call order. The writes are allowed to reject — a 404 on an
161+
* unmounted path and a refusal from a live write door are both rejections, and
162+
* this suite is about the URL, not the answer. Recording happens before the
163+
* request is made, so a rejected call still contributes its URL.
164+
*/
165+
async function driveAllSix(client: ObjectStackClient, urls: string[]): Promise<string[]> {
166+
const scoped = client.environment(ENV_ID);
167+
const before = urls.length;
168+
const swallow = (p: Promise<unknown>) => p.then(() => undefined, () => undefined);
169+
await swallow(scoped.meta.getTypes());
170+
await swallow(scoped.meta.getItems('object'));
171+
await swallow(scoped.meta.getItem('object', 'task'));
172+
await swallow(scoped.meta.saveItem('object', 'task', { name: 'task', label: 'Task' }));
173+
await swallow(scoped.meta.deleteItem('object', 'task'));
174+
await swallow(scoped.meta.getHistory('object', 'task'));
175+
return urls.slice(before);
176+
}
177+
178+
/** The six URLs a deployment on `prefix` must be called at. */
179+
function expectedSix(baseUrl: string, prefix: string): string[] {
180+
const root = `${baseUrl}/api/v1/environments/${ENV_ID}${prefix}`;
181+
return [
182+
`${root}`,
183+
`${root}/object`,
184+
`${root}/object/task`,
185+
`${root}/object/task`,
186+
`${root}/object/task`,
187+
`${root}/object/task/history`,
188+
];
189+
}
190+
191+
describe('SDK honours metadata.prefix (#16675)', () => {
192+
describe(`non-default prefix (${CUSTOM_PREFIX})`, () => {
193+
let fx: Fixture;
194+
195+
beforeAll(async () => { fx = await bootServer(CUSTOM_PREFIX); }, 30_000);
196+
afterAll(async () => { await shutdown(fx); }, 30_000);
197+
198+
it('mounts scoped metadata under the configured prefix, and serves nothing at /meta', async () => {
199+
const mounted = await fetch(`${fx.baseUrl}/api/v1/environments/${ENV_ID}${CUSTOM_PREFIX}`);
200+
expect(mounted.status).toBe(200);
201+
202+
// The half that makes this fixture worth anything: the path the
203+
// SDK used to hard-code is genuinely not mounted here.
204+
const hardCoded = await fetch(`${fx.baseUrl}/api/v1/environments/${ENV_ID}/meta`);
205+
expect(hardCoded.status).toBe(404);
206+
});
207+
208+
it('advertises the prefix on the discovery document', async () => {
209+
const res = await fetch(`${fx.baseUrl}/api/v1/discovery`);
210+
expect(res.status).toBe(200);
211+
const body = await res.json();
212+
const routes = (body?.data ?? body)?.routes;
213+
expect(routes?.metadata).toBe(`/api/v1${CUSTOM_PREFIX}`);
214+
});
215+
216+
it('all six scoped meta methods call the mounted path, not /meta', async () => {
217+
const { client, urls } = recordingClient(fx.baseUrl);
218+
await client.connect();
219+
220+
const called = await driveAllSix(client, urls);
221+
expect(called).toEqual(expectedSix(fx.baseUrl, CUSTOM_PREFIX));
222+
});
223+
});
224+
225+
describe('negative control — default prefix, byte-identical URLs', () => {
226+
let fx: Fixture;
227+
228+
beforeAll(async () => { fx = await bootServer(); }, 30_000);
229+
afterAll(async () => { await shutdown(fx); }, 30_000);
230+
231+
it('advertises /api/v1/meta', async () => {
232+
const res = await fetch(`${fx.baseUrl}/api/v1/discovery`);
233+
const body = await res.json();
234+
const routes = (body?.data ?? body)?.routes;
235+
expect(routes?.metadata).toBe('/api/v1/meta');
236+
});
237+
238+
it('a CONNECTED client still calls the six /meta URLs, byte for byte', async () => {
239+
const { client, urls } = recordingClient(fx.baseUrl);
240+
await client.connect();
241+
242+
const called = await driveAllSix(client, urls);
243+
expect(called).toEqual(expectedSix(fx.baseUrl, '/meta'));
244+
});
245+
246+
it('an UNCONNECTED client calls the same six URLs and puts NO discovery request on the wire', async () => {
247+
// No `connect()`, so there is no advertised document to read. The
248+
// derivation must decline to the conventional literal rather than
249+
// reach for one -- this is the leg that fails on an
250+
// "always rebuild the path out of discovery" implementation, which
251+
// would make every default deployment pay a round-trip it does not
252+
// pay today.
253+
const { client, urls } = recordingClient(fx.baseUrl);
254+
255+
const called = await driveAllSix(client, urls);
256+
expect(called).toEqual(expectedSix(fx.baseUrl, '/meta'));
257+
258+
// Exactly six requests, and none of them is a discovery read.
259+
expect(urls).toHaveLength(6);
260+
expect(urls.filter((u) => u.includes('/discovery'))).toEqual([]);
261+
});
262+
});
263+
});

0 commit comments

Comments
 (0)