From 95162721963ec711fa01f28227cf8232cf20e640 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 17 Aug 2026 21:12:02 +0000 Subject: [PATCH 1/2] fix(qa): HttpTestAdapter resolves the data mount from discovery, and degrades loudly (#7983) The record action types built their URLs from the DEFAULTS of RestApiConfigSchema.apiPath and CrudEndpointsConfigSchema.dataPrefix, so a deployment that moved the mount got a 404 that reads like the suite author's own URL mistake. The adapter now probes `{apiBase}/discovery` once per run and addresses whatever `routes.data` advertises, following the getRoute precedent in @objectstack/client; the schema-derived convention stays as the fallback and taking it is announced, naming the mount, the evidence and the remedy. Co-Authored-By: Claude --- packages/core/src/qa/http-adapter.test.ts | 252 ++++++++++++++++++++-- packages/core/src/qa/http-adapter.ts | 227 ++++++++++++++++--- 2 files changed, 425 insertions(+), 54 deletions(-) diff --git a/packages/core/src/qa/http-adapter.test.ts b/packages/core/src/qa/http-adapter.test.ts index fd6331e8a4..81053d9211 100644 --- a/packages/core/src/qa/http-adapter.test.ts +++ b/packages/core/src/qa/http-adapter.test.ts @@ -25,11 +25,19 @@ * own URL rather than a platform defect. * * What these tests pin is the URL and the VERB per action type, against a - * captured `fetch` — the wire statement, without needing a server. The base - * path is asserted against the spec schemas the server itself resolves from - * (`RestApiConfigSchema` + `CrudEndpointsConfigSchema`), never against a second - * copy of the literal `/api/v1/data`: a pin that hard-codes the string it is - * guarding goes green the day the schema moves and the adapter does not. + * captured `fetch` — the wire statement, without needing a server. + * + * PIN (#7983) — the mount is now RESOLVED from the server, not assumed. + * + * The adapter asks `{apiBase}/discovery` once per run and addresses whatever + * `routes.data` advertises; the schema-derived convention is the fallback, and + * taking it is announced loudly (Route & surface ownership §3). So the record + * URLs below are pinned against a discovery document that deliberately + * advertises a NON-default mount — a pin written against the literal + * `/api/v1/data` would pass while the probe was ignored entirely, and would go + * red for #9292's reasons rather than this adapter's. The convention pins keep + * asserting the schemas (`RestApiConfigSchema` + `CrudEndpointsConfigSchema`), + * never a second copy of the literal they produce. */ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; @@ -39,13 +47,27 @@ import { HttpTestAdapter } from './http-adapter.js'; const BASE_URL = 'http://localhost:3000'; +/** What `RestServer.getApiBasePath()` evaluates. */ +const EXPECTED_API_BASE = (() => { + const api = RestApiConfigSchema.parse({}); + return api.apiPath ?? `${api.basePath}/${api.version}`; +})(); + /** What `RestServer` composes: `getApiBasePath()` + `crud.dataPrefix`. */ const EXPECTED_DATA_PATH = (() => { - const api = RestApiConfigSchema.parse({}); const crud = CrudEndpointsConfigSchema.parse({}); - return `${api.apiPath ?? `${api.basePath}/${api.version}`}${crud.dataPrefix}`; + return `${EXPECTED_API_BASE}${crud.dataPrefix}`; })(); +/** Where `registerDiscoveryEndpoints` mounts the document the adapter probes. */ +const EXPECTED_DISCOVERY_URL = `${BASE_URL}${EXPECTED_API_BASE}/discovery`; + +/** + * The mount this suite's server advertises — deliberately NOT the convention, + * so every record-URL assertion below fails if the probe is ignored. + */ +const ADVERTISED_DATA_PATH = '/api/v1/objects'; + interface Call { url: string; method: string; @@ -55,6 +77,10 @@ interface Call { let calls: Call[]; let fetchMock: ReturnType; +let warnings: string[]; + +/** What the discovery probe gets back; per-test overridable. */ +let discoveryReply: () => Response | Promise; function jsonResponse(body: unknown, status = 200): Response { return new Response(JSON.stringify(body), { @@ -63,8 +89,15 @@ function jsonResponse(body: unknown, status = 200): Response { }); } +/** A discovery document shaped like the one `@objectstack/rest` answers. */ +function discoveryDocument(routes: Record): Response { + return jsonResponse({ version: 'v1', apiName: 'ObjectStack', routes }); +} + beforeEach(() => { calls = []; + warnings = []; + discoveryReply = () => discoveryDocument({ data: ADVERTISED_DATA_PATH, metadata: '/api/v1/meta' }); // `input` is deliberately `unknown`: this package's tsc program has no DOM lib, // so `RequestInfo` does not resolve here — and the assertions want the URL as a // string anyway. @@ -75,49 +108,61 @@ beforeEach(() => { body: typeof init?.body === 'string' ? JSON.parse(init.body) : undefined, headers: (init?.headers ?? {}) as Record, }); + if (String(input) === EXPECTED_DISCOVERY_URL) return discoveryReply(); return jsonResponse({ ok: true }); }); vi.stubGlobal('fetch', fetchMock); + vi.spyOn(console, 'warn').mockImplementation((...args: unknown[]) => { + warnings.push(args.map(String).join(' ')); + }); }); afterEach(() => { vi.unstubAllGlobals(); + vi.restoreAllMocks(); }); function action(type: QA.TestActionType, target: string, payload?: Record): QA.TestAction { return { type, target, ...(payload ? { payload } : {}) } as QA.TestAction; } +/** Every call the adapter made that was not the one-per-run discovery probe. */ +function actionCalls(): Call[] { + return calls.filter((c) => c.url !== EXPECTED_DISCOVERY_URL); +} + async function run(a: QA.TestAction): Promise { const adapter = new HttpTestAdapter(BASE_URL); await adapter.execute(a, {}); - expect(calls).toHaveLength(1); - return calls[0]; + const acted = actionCalls(); + expect(acted).toHaveLength(1); + return acted[0]; } describe('[#7848] HttpTestAdapter record action types reach the served route', () => { - it('derives the data path from the schemas the server resolves from', () => { + it('derives the convention from the schemas the server resolves from', () => { // Not a tautology: this is the one assertion that would fail if the // adapter went back to writing the prefix out by hand. expect(EXPECTED_DATA_PATH).toBe('/api/v1/data'); + expect(EXPECTED_API_BASE).toBe('/api/v1'); }); it('create_record POSTs the collection URL', async () => { const call = await run(action('create_record', 'crm_account', { name: 'Acme' })); - expect(call.url).toBe(`${BASE_URL}${EXPECTED_DATA_PATH}/crm_account`); + expect(call.url).toBe(`${BASE_URL}${ADVERTISED_DATA_PATH}/crm_account`); expect(call.method).toBe('POST'); expect(call.body).toEqual({ name: 'Acme' }); }); it('read_record GETs the record URL', async () => { const call = await run(action('read_record', 'crm_account', { id: 'rec_1' })); - expect(call.url).toBe(`${BASE_URL}${EXPECTED_DATA_PATH}/crm_account/rec_1`); + expect(call.url).toBe(`${BASE_URL}${ADVERTISED_DATA_PATH}/crm_account/rec_1`); expect(call.method).toBe('GET'); }); it('update_record PATCHes the record URL — the route has no PUT sibling', async () => { const call = await run(action('update_record', 'crm_account', { id: 'rec_1', name: 'Acme II' })); - expect(call.url).toBe(`${BASE_URL}${EXPECTED_DATA_PATH}/crm_account/rec_1`); + expect(call.url).toBe(`${BASE_URL}${ADVERTISED_DATA_PATH}/crm_account/rec_1`); expect(call.method).toBe('PATCH'); // `id` addressed the record; the body is the field patch, not a column write. expect(call.body).toEqual({ name: 'Acme II' }); @@ -125,20 +170,20 @@ describe('[#7848] HttpTestAdapter record action types reach the served route', ( it('delete_record DELETEs the record URL', async () => { const call = await run(action('delete_record', 'crm_account', { id: 'rec_1' })); - expect(call.url).toBe(`${BASE_URL}${EXPECTED_DATA_PATH}/crm_account/rec_1`); + expect(call.url).toBe(`${BASE_URL}${ADVERTISED_DATA_PATH}/crm_account/rec_1`); expect(call.method).toBe('DELETE'); }); it('query_records POSTs the QueryAST to the collection query URL', async () => { const call = await run(action('query_records', 'crm_account', { filters: [['name', '=', 'Acme']] })); - expect(call.url).toBe(`${BASE_URL}${EXPECTED_DATA_PATH}/crm_account/query`); + expect(call.url).toBe(`${BASE_URL}${ADVERTISED_DATA_PATH}/crm_account/query`); expect(call.method).toBe('POST'); expect(call.body).toEqual({ filters: [['name', '=', 'Acme']] }); }); it('percent-encodes the record id so an id with a slash cannot forge a path', async () => { const call = await run(action('read_record', 'crm_account', { id: 'a/b c' })); - expect(call.url).toBe(`${BASE_URL}${EXPECTED_DATA_PATH}/crm_account/a%2Fb%20c`); + expect(call.url).toBe(`${BASE_URL}${ADVERTISED_DATA_PATH}/crm_account/a%2Fb%20c`); }); it('no record action type addresses the old unversioned `/api/data` path', async () => { @@ -146,16 +191,162 @@ describe('[#7848] HttpTestAdapter record action types reach the served route', ( calls = []; const adapter = new HttpTestAdapter(BASE_URL); await adapter.execute(action(type, 'crm_account', { id: 'rec_1' }), {}); - expect(calls[0].url.startsWith(`${BASE_URL}/api/data/`)).toBe(false); + expect(actionCalls()[0].url.startsWith(`${BASE_URL}/api/data/`)).toBe(false); } }); }); +describe('[#7983] the data mount is resolved from the server, once per run', () => { + it('probes the discovery document at the base path the server mounts it under', async () => { + await run(action('create_record', 'crm_account', { name: 'Acme' })); + const probes = calls.filter((c) => c.url === EXPECTED_DISCOVERY_URL); + expect(probes).toHaveLength(1); + expect(probes[0].method).toBe('GET'); + }); + + it('addresses whatever routes.data advertises — a re-prefixed mount is reached', async () => { + discoveryReply = () => discoveryDocument({ data: '/api/2026-01/records' }); + const call = await run(action('create_record', 'crm_account', { name: 'Acme' })); + expect(call.url).toBe(`${BASE_URL}/api/2026-01/records/crm_account`); + }); + + it('probes ONCE per adapter, however many record actions run', async () => { + const adapter = new HttpTestAdapter(BASE_URL); + await adapter.execute(action('create_record', 'crm_account', { name: 'Acme' }), {}); + await adapter.execute(action('read_record', 'crm_account', { id: 'rec_1' }), {}); + await adapter.execute(action('query_records', 'crm_account', {}), {}); + expect(calls.filter((c) => c.url === EXPECTED_DISCOVERY_URL)).toHaveLength(1); + expect(actionCalls()).toHaveLength(3); + }); + + it('probes once even when record actions start concurrently', async () => { + const adapter = new HttpTestAdapter(BASE_URL); + await Promise.all([ + adapter.execute(action('create_record', 'crm_account', { name: 'A' }), {}), + adapter.execute(action('create_record', 'crm_account', { name: 'B' }), {}), + adapter.execute(action('read_record', 'crm_account', { id: 'rec_1' }), {}), + ]); + expect(calls.filter((c) => c.url === EXPECTED_DISCOVERY_URL)).toHaveLength(1); + }); + + it('sends the bearer token on the probe — a host that gates discovery still answers', async () => { + const adapter = new HttpTestAdapter(BASE_URL, 'tok_123'); + await adapter.execute(action('read_record', 'crm_account', { id: 'rec_1' }), {}); + const probe = calls.find((c) => c.url === EXPECTED_DISCOVERY_URL)!; + expect(probe.headers['Authorization']).toBe('Bearer tok_123'); + }); + + it('reads the dispatcher bridge\'s `{ data: … }` envelope as well as the bare document', async () => { + discoveryReply = () => jsonResponse({ data: { routes: { data: '/api/v1/things' } } }); + const call = await run(action('read_record', 'crm_account', { id: 'rec_1' })); + expect(call.url).toBe(`${BASE_URL}/api/v1/things/crm_account/rec_1`); + }); + + it('says nothing when the probe answered — no false alarm on a stock host', async () => { + await run(action('create_record', 'crm_account', { name: 'Acme' })); + expect(warnings).toEqual([]); + }); +}); + +describe('[#7983] falling back to the convention degrades LOUDLY', () => { + /** Each row is a way the probe fails to settle the mount. */ + const failures: Array<[label: string, reply: () => Response, evidence: RegExp]> = [ + ['discovery is not mounted (404)', () => jsonResponse({ error: 'Not found' }, 404), /answered 404/], + ['discovery is disabled (503)', () => jsonResponse({ error: 'nope' }, 503), /answered 503/], + ['the document carries no routes.data', () => discoveryDocument({ metadata: '/api/v1/meta' }), /carried no routes\.data/], + ['routes.data is not a usable path', () => discoveryDocument({ data: '' }), /carried no routes\.data/], + ]; + + for (const [label, reply, evidence] of failures) { + it(`falls back and names the mount when ${label}`, async () => { + discoveryReply = reply; + const call = await run(action('create_record', 'crm_account', { name: 'Acme' })); + expect(call.url).toBe(`${BASE_URL}${EXPECTED_DATA_PATH}/crm_account`); + expect(warnings).toHaveLength(1); + // The mount it resolved… + expect(warnings[0]).toContain(`${BASE_URL}${EXPECTED_DATA_PATH}`); + // …why it resolved that way… + expect(warnings[0]).toMatch(evidence); + expect(warnings[0]).toContain(EXPECTED_DISCOVERY_URL); + // …and the remedy, which is the part a bare 404 never carries. + expect(warnings[0]).toContain('api_call'); + expect(warnings[0]).toContain('api.apiPath'); + }); + } + + it('falls back when the host cannot be reached at all', async () => { + discoveryReply = () => { throw new Error('ECONNREFUSED'); }; + const call = await run(action('read_record', 'crm_account', { id: 'rec_1' })); + expect(call.url).toBe(`${BASE_URL}${EXPECTED_DATA_PATH}/crm_account/rec_1`); + expect(warnings[0]).toContain('ECONNREFUSED'); + }); + + it('warns ONCE per run, not once per action', async () => { + discoveryReply = () => jsonResponse({ error: 'Not found' }, 404); + const adapter = new HttpTestAdapter(BASE_URL); + await adapter.execute(action('create_record', 'crm_account', { name: 'Acme' }), {}); + await adapter.execute(action('read_record', 'crm_account', { id: 'rec_1' }), {}); + expect(warnings).toHaveLength(1); + }); + + it('⛔ never probes /.well-known/objectstack — it advertises the dispatcher prefix, not the REST mount', async () => { + discoveryReply = () => jsonResponse({ error: 'Not found' }, 404); + await run(action('create_record', 'crm_account', { name: 'Acme' })); + expect(calls.some((c) => c.url.includes('/.well-known/'))).toBe(false); + }); +}); + +describe('[#7983] a failed record action carries the mount it addressed', () => { + it('appends the mount and its provenance to a 404', async () => { + discoveryReply = () => jsonResponse({ error: 'Not found' }, 404); + fetchMock.mockImplementation(async (input: unknown) => { + if (String(input) === EXPECTED_DISCOVERY_URL) return jsonResponse({ error: 'Not found' }, 404); + return jsonResponse({ error: 'Not found' }, 404); + }); + const adapter = new HttpTestAdapter(BASE_URL); + await expect(adapter.execute(action('create_record', 'crm_account', { name: 'Acme' }), {})).rejects.toThrow( + /HTTP Error 404: .*record actions addressed http:\/\/localhost:3000\/api\/v1\/data \(data mount assumed by convention/, + ); + }); + + it('names discovery as the source when discovery answered', async () => { + fetchMock.mockImplementation(async (input: unknown) => { + if (String(input) === EXPECTED_DISCOVERY_URL) { + return discoveryDocument({ data: ADVERTISED_DATA_PATH }); + } + return jsonResponse({ error: 'Not found' }, 404); + }); + const adapter = new HttpTestAdapter(BASE_URL); + await expect(adapter.execute(action('read_record', 'crm_account', { id: 'rec_1' }), {})).rejects.toThrow( + /record actions addressed http:\/\/localhost:3000\/api\/v1\/objects \(data mount resolved from discovery/, + ); + }); + + it('leaves a non-routing failure undecorated — the server\'s own message stands alone', async () => { + fetchMock.mockImplementation(async (input: unknown) => { + if (String(input) === EXPECTED_DISCOVERY_URL) { + return discoveryDocument({ data: ADVERTISED_DATA_PATH }); + } + return jsonResponse({ error: 'name is required' }, 422); + }); + const adapter = new HttpTestAdapter(BASE_URL); + const err = await adapter + .execute(action('create_record', 'crm_account', {}), {}) + .then(() => undefined, (e: Error) => e); + expect(err?.message).toBe('HTTP Error 422: {"error":"name is required"}'); + }); +}); + describe('[#7848] the three non-record action types are unchanged', () => { it('api_call resolves a relative target against the base URL', async () => { - const call = await run(action('api_call', '/api/v1/discovery', { method: 'GET' })); - expect(call.url).toBe(`${BASE_URL}/api/v1/discovery`); - expect(call.method).toBe('GET'); + // Not routed through `run()`: this target IS the discovery path, and the + // helper filters that URL out as the probe. `api_call` never probes, so + // the single recorded call is the action's own — which is the point. + const adapter = new HttpTestAdapter(BASE_URL); + await adapter.execute(action('api_call', `${EXPECTED_API_BASE}/discovery`, { method: 'GET' }), {}); + expect(calls).toHaveLength(1); + expect(calls[0].url).toBe(EXPECTED_DISCOVERY_URL); + expect(calls[0].method).toBe('GET'); }); it('api_call leaves an absolute target alone', async () => { @@ -163,6 +354,22 @@ describe('[#7848] the three non-record action types are unchanged', () => { expect(call.url).toBe('http://example.test/health'); }); + it('[#7983] api_call issues NO discovery probe — it takes the path it is given', async () => { + const adapter = new HttpTestAdapter(BASE_URL); + await adapter.execute(action('api_call', '/api/2026-01/data/crm_account', { method: 'GET' }), {}); + expect(calls).toHaveLength(1); + expect(calls[0].url).toBe(`${BASE_URL}/api/2026-01/data/crm_account`); + }); + + it('[#7983] an api_call failure is not decorated with a mount it never used', async () => { + fetchMock.mockImplementation(async () => jsonResponse({ error: 'Not found' }, 404)); + const adapter = new HttpTestAdapter(BASE_URL); + const err = await adapter + .execute(action('api_call', '/nope', { method: 'GET' }), {}) + .then(() => undefined, (e: Error) => e); + expect(err?.message).toBe('HTTP Error 404: {"error":"Not found"}'); + }); + it('wait resolves without touching the network', async () => { const adapter = new HttpTestAdapter(BASE_URL); const result = await adapter.execute(action('wait', 'n/a', { duration: 1 }), {}); @@ -185,7 +392,8 @@ describe('[#7848] auth and impersonation headers still ride along', () => { { type: 'create_record', target: 'crm_account', payload: { name: 'Acme' }, user: 'alice' } as QA.TestAction, {}, ); - expect(calls[0].headers['Authorization']).toBe('Bearer tok_123'); - expect(calls[0].headers['X-Run-As']).toBe('alice'); + const call = actionCalls()[0]; + expect(call.headers['Authorization']).toBe('Bearer tok_123'); + expect(call.headers['X-Run-As']).toBe('alice'); }); }); diff --git a/packages/core/src/qa/http-adapter.ts b/packages/core/src/qa/http-adapter.ts index 0e5c5cbe0c..c1daa841f9 100644 --- a/packages/core/src/qa/http-adapter.ts +++ b/packages/core/src/qa/http-adapter.ts @@ -4,11 +4,12 @@ import * as QA from '@objectstack/spec/qa'; import { RestApiConfigSchema, CrudEndpointsConfigSchema } from '@objectstack/spec/api'; import { TestExecutionAdapter } from './adapter.js'; -/** Memoised {@link defaultDataPath} — the schemas are `lazySchema`, so build them once. */ -let dataPathCache: string | undefined; +/** Memoised {@link conventionMounts} — the schemas are `lazySchema`, so build them once. */ +let conventionCache: { apiBase: string; dataPath: string } | undefined; /** - * The path prefix a stock ObjectStack server serves the Data Protocol under. + * The paths a STOCK ObjectStack server serves the API base and the Data + * Protocol under, derived from the two schemas `RestServer` resolves from. * * ## [#7848] Why this is derived and not written down * @@ -28,32 +29,172 @@ let dataPathCache: string | undefined; * - `CrudEndpointsConfigSchema.dataPrefix` — what `RestServer` appends to it * to get `dataPath` (`/data`). * - * Defaults only: this adapter is handed an origin, not a deployment's config, - * so a host that overrides `api.apiPath` or `crud.dataPrefix` is still out of - * reach here (tracked separately — the `api_call` action type is the escape - * hatch until then). What the derivation buys is that the DEFAULT can never - * again disagree with the schema that declares it. + * These are the CONVENTION: what the schemas default to, which is what a + * deployment serves until it says otherwise. `HttpTestAdapter` is handed an + * origin and nothing else, so the convention is also all it can know before it + * asks the server — which is what {@link HttpTestAdapter.resolveDataMount} + * does (#7983). The `/discovery` path is derived here for the same reason the + * data path is: `RestServer.registerDiscoveryEndpoints` mounts it at + * `${getApiBasePath()}/discovery`, so a second literal would be a second place + * to forget. */ -function defaultDataPath(): string { - if (dataPathCache === undefined) { +function conventionMounts(): { apiBase: string; dataPath: string } { + if (conventionCache === undefined) { const api = RestApiConfigSchema.parse({}); const crud = CrudEndpointsConfigSchema.parse({}); - dataPathCache = `${api.apiPath ?? `${api.basePath}/${api.version}`}${crud.dataPrefix}`; + const apiBase = api.apiPath ?? `${api.basePath}/${api.version}`; + conventionCache = { apiBase, dataPath: `${apiBase}${crud.dataPrefix}` }; } - return dataPathCache; + return conventionCache; +} + +/** + * Where this adapter decided the Data Protocol is mounted, and on what evidence. + * + * `why` is not decoration — it is the whole difference between a 404 that reads + * as the suite author's own URL mistake and one that names the mount it used + * and where that mount came from (Route & surface ownership §3). + */ +interface ResolvedDataMount { + /** The path prefix record actions address, e.g. `/api/v1/data`. */ + readonly path: string; + /** `discovery` = the server told us; `convention` = the schemas' defaults. */ + readonly source: 'discovery' | 'convention'; + /** One clause naming the probe and its outcome. */ + readonly why: string; } export class HttpTestAdapter implements TestExecutionAdapter { + /** + * The single discovery probe of a run, memoised as the in-flight promise so + * concurrent record actions share one request rather than racing N. + * + * `os test` builds ONE adapter for the whole run (`packages/cli/src/commands/ + * test.ts`) and hands it to every suite, so instance scope IS run scope. + */ + private mountPromise?: Promise; + constructor(private baseUrl: string, private authToken?: string) {} - /** `{baseUrl}{apiBasePath}{dataPrefix}/{object}` — the collection URL. */ - private collectionUrl(objectName: string): string { - return `${this.baseUrl}${defaultDataPath()}/${encodeURIComponent(objectName)}`; + /** The resolved data mount; probes at most once per adapter. */ + private dataMount(): Promise { + if (this.mountPromise === undefined) { + this.mountPromise = this.resolveDataMount(); + } + return this.mountPromise; + } + + /** + * Ask the server where it serves the Data Protocol, and fall back to the + * convention — loudly — when it cannot say. + * + * ## [#7983] What the probe recovers, measured rather than assumed + * + * `@objectstack/client` answers the same question through discovery + * (`getRoute`, `packages/client/src/index.ts`), and this follows it: prefer + * the server's own `routes.data`, fall back to the convention. Measured on a + * booted stack (REST generator + dispatcher bridge, three configs): + * + * | deployment | `{apiBase}/discovery` | serves | + * |--------------------------------|-----------------------|---------------| + * | stock | 200 `/api/v1/data` | `/api/v1/data`| + * | `crud.dataPrefix: '/objects'` | 200 `/api/v1/objects` | `/api/v1/objects` | + * | `api.apiPath: '/api/2026-01'` | **404** | `/api/2026-01/data` | + * + * So the probe closes the `dataPrefix` row exactly: `RestServer`'s discovery + * handler substitutes the configured prefix into `routes.data`, and reading + * it is strictly better than recomputing it here. The `apiPath` row it cannot + * close, and the reason is structural rather than an oversight — `apiPath` + * moves the base that discovery itself is mounted under, so the document that + * would name the new mount is behind the very prefix we are missing. + * + * ⛔ And the one discovery document at a FIXED path does not rescue it: + * `/.well-known/objectstack` is mounted at the site root by the dispatcher + * bridge, but its `routes.data` is the DISPATCHER's own `${prefix}/data` — + * measured as `/api/v1/data` under all three configs above, including the two + * where the server serves elsewhere. Falling back to it would turn "we could + * not resolve the mount" into "discovery told us `/api/v1/data`": the same + * 404, now with a false provenance attached. Not probed, deliberately. + * + * Hence: one probe, then a diagnostic that NAMES the mount, the evidence and + * the remedy. `api_call` takes the path it is given and is unaffected either + * way — it stays the escape hatch for a host this cannot reach. + */ + private async resolveDataMount(): Promise { + const { apiBase, dataPath } = conventionMounts(); + const probeUrl = `${this.baseUrl}${apiBase}/discovery`; + let why: string; + + try { + const headers: Record = {}; + if (this.authToken) { + headers['Authorization'] = `Bearer ${this.authToken}`; + } + const response = await fetch(probeUrl, { method: 'GET', headers }); + if (response.ok) { + const body = await response.json(); + // `@objectstack/rest` answers the document bare; the dispatcher bridge + // wraps it as `{ data: … }`. Pick the object that actually carries + // `routes` rather than `body.data || body` — the document's own + // `routes.data` key makes the looser test ambiguous to read here. + const doc = (body && typeof body === 'object' && 'routes' in body) + ? (body as { routes?: Record }) + : ((body as { data?: { routes?: Record } } | null)?.data); + const advertised = doc?.routes?.data; + if (typeof advertised === 'string' && advertised.length > 0) { + return { + path: advertised, + source: 'discovery', + why: `GET ${probeUrl} advertised routes.data`, + }; + } + why = `GET ${probeUrl} answered ${response.status} but carried no routes.data`; + } else { + why = `GET ${probeUrl} answered ${response.status}`; + } + } catch (error) { + why = `GET ${probeUrl} could not be reached (${(error as Error).message})`; + } + + const mount: ResolvedDataMount = { path: dataPath, source: 'convention', why }; + // Absence must be loud (Route & surface ownership §3): state the mount that + // will be addressed, the evidence for it, and what to do about a host this + // cannot reach — never leave the bare 404 to be diagnosed. + console.warn( + `[HttpTestAdapter] Data Protocol mount NOT resolved from discovery. ` + + `Record actions (create_record, read_record, update_record, delete_record, query_records) ` + + `will address ${this.baseUrl}${dataPath}, the convention declared by RestApiConfigSchema ` + + `(apiPath ?? basePath/version) + CrudEndpointsConfigSchema.dataPrefix. Evidence: ${why}. ` + + `A deployment that sets crud.dataPrefix is normally recovered by this probe; one that sets ` + + `api.apiPath moves discovery itself out from under it, and no fixed-path document reports ` + + `the REST mount — write those steps as api_call, which takes the path you give it.`, + ); + return mount; + } + + /** `{baseUrl}{dataMount}/{object}` — the collection URL. */ + private collectionUrl(mount: ResolvedDataMount, objectName: string): string { + return `${this.baseUrl}${mount.path}/${encodeURIComponent(objectName)}`; } /** `{collection}/{id}` — the single-record URL. */ - private recordUrl(objectName: string, id: unknown): string { - return `${this.collectionUrl(objectName)}/${encodeURIComponent(String(id))}`; + private recordUrl(mount: ResolvedDataMount, objectName: string, id: unknown): string { + return `${this.collectionUrl(mount, objectName)}/${encodeURIComponent(String(id))}`; + } + + /** + * The provenance clause appended to a failed record action's error. + * + * The card this closes is about a 404 that reads like the author's own URL + * mistake; the mount is the one fact that distinguishes the two, so it rides + * on the failure itself rather than only on a warning printed earlier in the + * transcript. + */ + private mountNote(mount: ResolvedDataMount): string { + const how = mount.source === 'discovery' + ? 'resolved from discovery' + : 'assumed by convention — the server was not able to state it'; + return `record actions addressed ${this.baseUrl}${mount.path} (data mount ${how}: ${mount.why})`; } async execute(action: QA.TestAction, _context: Record): Promise { @@ -90,65 +231,74 @@ export class HttpTestAdapter implements TestExecutionAdapter { } private async createRecord(objectName: string, data: Record, headers: Record) { - const response = await fetch(this.collectionUrl(objectName), { + const mount = await this.dataMount(); + const response = await fetch(this.collectionUrl(mount, objectName), { method: 'POST', headers, body: JSON.stringify(data) }); - return this.handleResponse(response); + return this.handleResponse(response, this.mountNote(mount)); } private async updateRecord(objectName: string, data: Record, headers: Record) { const { id, ...fields } = data; if (!id) throw new Error('Update record requires id in payload'); - // PATCH, not PUT: `PATCH {apiPath}/data/:object/:id` is the route the server + // PATCH, not PUT: `PATCH {dataMount}/:object/:id` is the route the server // registers, and there is no PUT sibling — the old verb 404'd even once the // path was right (#7848). The body is the field patch, so `id` is peeled off // rather than posted back as a column write. - const response = await fetch(this.recordUrl(objectName, id), { + const mount = await this.dataMount(); + const response = await fetch(this.recordUrl(mount, objectName, id), { method: 'PATCH', headers, body: JSON.stringify(fields) }); - return this.handleResponse(response); + return this.handleResponse(response, this.mountNote(mount)); } private async deleteRecord(objectName: string, data: Record, headers: Record) { const id = data.id; if (!id) throw new Error('Delete record requires id in payload'); - const response = await fetch(this.recordUrl(objectName, id), { + const mount = await this.dataMount(); + const response = await fetch(this.recordUrl(mount, objectName, id), { method: 'DELETE', headers }); - return this.handleResponse(response); + return this.handleResponse(response, this.mountNote(mount)); } private async readRecord(objectName: string, data: Record, headers: Record) { const id = data.id; if (!id) throw new Error('Read record requires id in payload'); - const response = await fetch(this.recordUrl(objectName, id), { + const mount = await this.dataMount(); + const response = await fetch(this.recordUrl(mount, objectName, id), { method: 'GET', headers }); - return this.handleResponse(response); + return this.handleResponse(response, this.mountNote(mount)); } private async queryRecords(objectName: string, data: Record, headers: Record) { - // `POST {apiPath}/data/:object/query` — the spec-shape advanced query + // `POST {dataMount}/:object/query` — the spec-shape advanced query // (QueryAST in the body), the same route `client.data.query()` posts to. - const response = await fetch(`${this.collectionUrl(objectName)}/query`, { + const mount = await this.dataMount(); + const response = await fetch(`${this.collectionUrl(mount, objectName)}/query`, { method: 'POST', headers, body: JSON.stringify(data) }); - return this.handleResponse(response); + return this.handleResponse(response, this.mountNote(mount)); } private async rawApiCall(endpoint: string, data: Record, headers: Record) { + // Deliberately does NOT consult the resolved mount — `api_call` takes the + // path it is given, which is what makes it the escape hatch for a host + // the discovery probe cannot reach (#7983). It probes nothing either: a + // suite written entirely in `api_call` steps issues no discovery request. const method = (data.method as string) || 'GET'; const body = data.body ? JSON.stringify(data.body) : undefined; const url = endpoint.startsWith('http') ? endpoint : `${this.baseUrl}${endpoint}`; - + const response = await fetch(url, { method, headers, @@ -157,10 +307,23 @@ export class HttpTestAdapter implements TestExecutionAdapter { return this.handleResponse(response); } - private async handleResponse(response: Response) { + private async handleResponse(response: Response, mountNote?: string) { if (!response.ok) { const text = await response.text(); - throw new Error(`HTTP Error ${response.status}: ${text}`); + // The `HTTP Error : ` prefix is unchanged — a suite author + // (and every transcript predating #7983) reads the same first clause; + // the mount provenance is APPENDED, never substituted. + // + // Appended on 404/405 only: those are the statuses a wrong mount + // produces (nothing registered at the path / registered under another + // verb), and they are exactly the ones this card calls unreadable. A + // 400 or 422 reached the right route and got a real answer from it — + // decorating those with the mount would bury the server's own message + // under a URL the author has no reason to doubt. + const wrongUrlShaped = response.status === 404 || response.status === 405; + throw new Error( + `HTTP Error ${response.status}: ${text}${wrongUrlShaped && mountNote ? ` — ${mountNote}` : ''}`, + ); } const contentType = response.headers.get('content-type'); if (contentType && contentType.includes('application/json')) { From 99592d0ab526c600aef809b2a59d541125452129 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 17 Aug 2026 21:49:08 +0000 Subject: [PATCH 2/2] docs(qa): narrow the HttpTestAdapter mount gap to the apiPath row (#7983) The knownGaps entry and the deployment/cli.mdx paragraph both said the record action types address the DEFAULT mount only. The discovery probe closes the crud.dataPrefix row, so both are NARROWED rather than deleted: api.apiPath moves the discovery document itself out from under the probe and is still out of reach, and that row must keep saying so. Adds the changeset with the measured before/after table. Co-Authored-By: Claude --- .changeset/qa-http-adapter-mount-discovery.md | 38 +++++++++++++++++++ content/docs/deployment/cli.mdx | 21 +++++++--- docs/qa/platform-checklist/areas/cli.json | 9 +++-- 3 files changed, 59 insertions(+), 9 deletions(-) create mode 100644 .changeset/qa-http-adapter-mount-discovery.md diff --git a/.changeset/qa-http-adapter-mount-discovery.md b/.changeset/qa-http-adapter-mount-discovery.md new file mode 100644 index 0000000000..91f05bb943 --- /dev/null +++ b/.changeset/qa-http-adapter-mount-discovery.md @@ -0,0 +1,38 @@ +--- +"@objectstack/core": patch +--- + +fix(qa): `HttpTestAdapter` resolves the Data Protocol mount from the server's `/discovery`, and falls back to the convention loudly (#7983) + +The record-shaped `os test` action types (`create_record`, `read_record`, +`update_record`, `delete_record`, `query_records`) built their URLs from the +**defaults** of `RestApiConfigSchema.apiPath` and +`CrudEndpointsConfigSchema.dataPrefix`, because the adapter is handed an origin +and nothing else. A deployment that moved the mount got a 404 that reads like the +suite author's own URL mistake rather than a platform limitation. + +The adapter now asks the server, following the `getRoute` precedent in +`@objectstack/client`: **one memoised `GET {apiBase}/discovery` per run** (`os +test` builds one adapter for the whole run), addressing whatever `routes.data` +advertises, with the schema-derived convention as the fallback. Measured on a +booted stack (REST route generator + dispatcher bridge), before and after: + +| deployment | before | after | +|---|---|---| +| stock | created | created | +| `crud.dataPrefix: '/objects'` | `HTTP Error 404` | created | +| `api.apiPath: '/api/2026-01'` | `HTTP Error 404` | `HTTP Error 404`, now naming the mount | + +The `apiPath` row is **not** closed, and the reason is structural: `apiPath` +moves the base that `/discovery` is itself mounted under, so the document that +would name the new mount sits behind the prefix that is missing. The one +discovery document at a fixed path does not rescue it — `/.well-known/objectstack` +advertises the **dispatcher's** `${prefix}/data`, measured as `/api/v1/data` +under all three configs above — so it is deliberately not probed: trusting it +would attach a false provenance ("discovery told us") to the same 404. + +Instead that case degrades loudly. Falling back to the convention prints a +warning naming the mount it will address, the probe that failed and the remedy, +and every 404/405 from a record action now carries the mount it addressed and +where that mount came from. `api_call` is unchanged, issues no probe, and remains +the escape hatch for a host the probe cannot reach. diff --git a/content/docs/deployment/cli.mdx b/content/docs/deployment/cli.mdx index 3d2fd4ed93..36a0a77f9a 100644 --- a/content/docs/deployment/cli.mdx +++ b/content/docs/deployment/cli.mdx @@ -1054,11 +1054,22 @@ success forever. Pass **`--fail-on-empty`** to opt into the strict reading, wher an empty match exits 1 (#7848). The **record-shaped** action types — `create_record`, `read_record`, -`update_record`, `delete_record`, `query_records` — address the Data Protocol at -its default mount (`/api/v1/data`, i.e. `{apiPath}{crud.dataPrefix}`). A -deployment that moves that mount by setting `api.apiPath` or `crud.dataPrefix` is -out of reach for them; write those steps as `api_call`, which takes the path you -give it. `run_script` has no adapter branch at all and fails by name. +`update_record`, `delete_record`, `query_records` — **ask the server where the +Data Protocol is mounted** instead of assuming it. Once per run, `os test` +fetches `{apiBase}/discovery` and addresses whatever `routes.data` advertises, +so a deployment that moves the mount with `crud.dataPrefix` is reached without +you telling it anything. When the probe cannot answer, the run falls back to the +convention `{apiPath}{crud.dataPrefix}` (`/api/v1/data`) and **says so**: a +warning naming the mount it will address and the probe that failed, and the same +statement appended to every 404 a record step gets — so a wrong mount reads as a +wrong mount, not as your own URL mistake. + +One case survives that fallback by construction: setting **`api.apiPath`** moves +the discovery document itself out from under the probe, and no fixed-path +document reports the REST mount (`/.well-known/objectstack` advertises the +dispatcher's own prefix, not this one). Against such a host, write those steps as +`api_call`, which takes the path you give it. `run_script` has no adapter branch +at all and fails by name. #### `os doctor` diff --git a/docs/qa/platform-checklist/areas/cli.json b/docs/qa/platform-checklist/areas/cli.json index ac5fa89ce8..a1adc61bfb 100644 --- a/docs/qa/platform-checklist/areas/cli.json +++ b/docs/qa/platform-checklist/areas/cli.json @@ -433,7 +433,7 @@ "title": "os test: a Quality Protocol suite is validated at LOAD, executed against a booted app, and its verdict is the exit code — capture/interpolation thread state, an unevaluable assertion FAILS", "since": "v17", "status": "active", - "revision": 2, + "revision": 3, "priority": "P1", "surface": "cli", "personas": ["operator (local shell)", "suite author (writes qa/*.test.json)"], @@ -447,7 +447,7 @@ ], "knownGaps": [ "`run_script` has no adapter branch and fails by name (`Unsupported action type in HttpAdapter: run_script`) — the variant sweep records ONE refusal and that is the honest verdict, not a fixture gap to work around. The other seven members execute since #7848; the five record-shaped ones did not until then (see `negative`), so a sweep transcript predating that fix shows five 404s and is not comparable", - "the record action types address the Data Protocol at its DEFAULT mount only (`{apiPath}{crud.dataPrefix}` = `/api/v1/data`). A deployment that sets `api.apiPath` or `crud.dataPrefix` moves the mount out from under them; the fixture boot uses stock config, so this does not bite here — a run against a re-prefixed host must write those steps as `api_call`", + "the record action types resolve the Data Protocol mount from the server's own `/discovery` (`routes.data`) since #7983, so a deployment that sets `crud.dataPrefix` IS reached; what remains out of reach is `api.apiPath`, which moves the discovery document itself out from under the probe (measured: `{apiBase}/discovery` 404s, and `/.well-known/objectstack` reports the DISPATCHER's `/api/v1/data`, not the REST mount). Against such a host the adapter falls back to the convention and says so — it names the mount it addressed, the probe that failed and the remedy, on the warning AND on every 404 — and those steps must still be written as `api_call`. The fixture boot is stock, so neither case bites here", "no scenario SELECTION exists: `os test` has exactly two flags (--url, --token), `scenario.tags` filters nothing and `scenario.requires` is never checked (packages/spec/liveness/qa.json rows `tags`/`requires`), so the whole glob always runs and a suite cannot declare a precondition it will be skipped for" ] }, @@ -541,7 +541,7 @@ "source": [ "packages/cli/src/commands/test.ts (the shipped `os test`: the #7363 lazy segment-directed glob with its prune list, `loadTestSuite`'s #6247 boundary parse, the per-scenario report, and the exit 0/1 summary)", "packages/core/src/qa/runner.ts (scenario sequencing, `capture` + `{{var}}` interpolation, the assertion operators, setup/teardown semantics, the #7256 unevaluable-`contains` fix)", - "packages/core/src/qa/http-adapter.ts (the action-type switch — its case labels ARE the enum values; the record routes derive their prefix from RestApiConfigSchema + CrudEndpointsConfigSchema rather than spelling `/api/v1/data` out, #7848)", + "packages/core/src/qa/http-adapter.ts (the action-type switch — its case labels ARE the enum values; the record routes take their prefix from the one memoised `/discovery` probe per run, falling back to the RestApiConfigSchema + CrudEndpointsConfigSchema convention with a diagnostic that names the mount, #7848 / #7983)", "packages/spec/src/qa/testing.zod.ts (TestSuiteSchema — the shape enforced at load; TestActionTypeSchema pinned above)", "packages/spec/liveness/qa.json (the ADR-0049 ledger whose existence this item is coverage.json's mapping for — its dead `tags`/`requires` rows are why no scenario selection exists)", "packages/cli/test/qa-suite-schema-load.test.ts, packages/cli/test/resolve-glob-lazy-walk.test.ts, packages/core/src/qa/runner.test.ts (the three unit pins — cited so a run knows what is already covered, NOT a substitute for driving a booted app)", @@ -550,7 +550,8 @@ ], "history": [ { "revision": 1, "date": "2026-08-11", "change": "new item: the `qa` capability's coverage.json mapping, authored rather than waived (#7347 triage ruling). `os test` is a shipped, documented CLI command, so the honest mapping is a surface:cli item that authors a real qa/*.test.json suite and drives it against a booted app — the fixture suite examples/app-showcase/qa/platform-smoke.test.json lands with this item and is the repo's first Quality Protocol suite. Every clause was measured on showcase before it was written: the green path, capture/interpolation, the #6247 load refusal, the #7256 unevaluable-contains failure, teardown-after-failure, the 8-member action-type sweep and the #7363 glob. `since: v17` records the release in which the surface became GOVERNED (liveness ledger seeded + TestSuiteSchema enforced at the load site, #6247 / PR #7255); the command itself predates it. No `automated` entry: the three unit pins cover pieces, none of them proves a suite reaches a real server", "ref": "#7347" }, - {"revision": 2, "date": "2026-08-12", "change": "the adapter was repaired, which this item's own `negative` clause declared to be a revision rather than a silent green (#7848). Item 1: the five record-shaped action types now round-trip against a stock server — the `${baseUrl}/api/data/:object` literal became a prefix DERIVED from the two schemas RestServer itself resolves from (RestApiConfigSchema `apiPath ?? {basePath}/{version}` + CrudEndpointsConfigSchema.dataPrefix), and `update_record` PATCHes where it used to PUT a route that has no PUT sibling. Re-measured on a booted showcase, one scenario per member with NO shared setup so no member's verdict is inferred from a sibling: 7 of 8 execute and assert, `run_script` still refuses by name. The `negative` clause is inverted accordingly — a 404 from a record action type is the regression now — and the knownGap it rested on is replaced by the narrower one that survives: the record types address the DEFAULT mount only, so a host that moves it with `api.apiPath`/`crud.dataPrefix` still needs `api_call`. Item 2: a zero-match glob still exits 0, deliberately (a repo that legitimately ships no suites must not start failing CI), but the posture is now DECLARED — stated in `--help`, opt out with the new `--fail-on-empty`, and `Found N test suites.` is emitted on EVERY run including `Found 0 test suites.`, which is the line this item's first acceptance clause already asks a run record to quote", "ref": "#7848"} + {"revision": 2, "date": "2026-08-12", "change": "the adapter was repaired, which this item's own `negative` clause declared to be a revision rather than a silent green (#7848). Item 1: the five record-shaped action types now round-trip against a stock server — the `${baseUrl}/api/data/:object` literal became a prefix DERIVED from the two schemas RestServer itself resolves from (RestApiConfigSchema `apiPath ?? {basePath}/{version}` + CrudEndpointsConfigSchema.dataPrefix), and `update_record` PATCHes where it used to PUT a route that has no PUT sibling. Re-measured on a booted showcase, one scenario per member with NO shared setup so no member's verdict is inferred from a sibling: 7 of 8 execute and assert, `run_script` still refuses by name. The `negative` clause is inverted accordingly — a 404 from a record action type is the regression now — and the knownGap it rested on is replaced by the narrower one that survives: the record types address the DEFAULT mount only, so a host that moves it with `api.apiPath`/`crud.dataPrefix` still needs `api_call`. Item 2: a zero-match glob still exits 0, deliberately (a repo that legitimately ships no suites must not start failing CI), but the posture is now DECLARED — stated in `--help`, opt out with the new `--fail-on-empty`, and `Found N test suites.` is emitted on EVERY run including `Found 0 test suites.`, which is the line this item's first acceptance clause already asks a run record to quote", "ref": "#7848"}, + {"revision": 3, "date": "2026-08-17", "change": "the record action types stopped ASSUMING the mount (#7983). They now resolve it from the server: one memoised `GET {apiBase}/discovery` per run, addressing whatever `routes.data` advertises, with the RestApiConfigSchema + CrudEndpointsConfigSchema convention as the fallback — the `@objectstack/client` `getRoute` pattern, copied rather than re-invented. Measured on a booted stack (REST generator + dispatcher bridge) before and after, three configs: stock stays green; `crud.dataPrefix: '/objects'` went from `HTTP Error 404` to a created record, so that row of the gap is CLOSED; `api.apiPath: '/api/2026-01'` still 404s and is NOT closed — `apiPath` moves the discovery document itself, and the one fixed-path document (`/.well-known/objectstack`) advertises the dispatcher's `/api/v1/data` under all three configs, so trusting it would attach a false provenance to the same 404. That row is narrowed instead: the fallback is announced (a warning naming the mount, the failed probe and the remedy) and every 404 from a record action now carries the mount it addressed and where that mount came from, so the failure can no longer read as the suite author's own URL mistake. `api_call` is unchanged and probes nothing — it remains the escape hatch for a host the probe cannot reach", "ref": "#7983"} ] }, {