Skip to content

Commit 4df6292

Browse files
fix(core): os test --url with a trailing slash addresses the served routes (#28999)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"} --> _Requested by **Leehom** · [project thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1XFBxyXPUfh7hY87x8YuedEk)_ Fixes #28946 Clause-②: no — no schema, type, export or route change; a `--url` that answered every HTTP scenario 404 now reaches the served routes. Metadata-protocol: none — nothing a metadata author writes is read differently; `qa/*.test.json` suites are unchanged. ## 维护者速读 - 改了什么:`HttpTestAdapter` 在构造函数里把 base URL 末尾的斜杠一次性去掉,于是发现探针、记录动作 URL、`api_call` 相对路径和报错里的 URL 都读同一个规整后的 base。`api_call` 遇到 404/405 时,报错句末尾追加它实际请求的 `方法 + URL`,和记录动作已有的写法一致。 - 为什么改:`os test --url http://localhost:PORT/`(带末尾斜杠)会请求 `//api/v1/...`,服务端回 404 `ENDPOINT_NOT_FOUND`,每个 HTTP 场景都失败,而且 `api_call` 的报错不说它请求了哪个 URL,看起来像应用坏了。 - 风险与回滚:只动一个类的构造函数和一处报错文案;不加参数、不做 URL 校验,畸形 `--url` 行为不变。回滚即 revert。 - 先看:`packages/core/src/qa/http-adapter.ts` 的构造函数与 `rawApiCall` 末尾;`packages/core/src/qa/http-adapter.test.ts` 末尾新增的 describe 与改过的 `[#7983] an api_call failure ...` 用例。 - 两分钟验证:`pnpm --filter @objectstack/core exec vitest run src/qa/http-adapter.test.ts`,预期 `Tests 52 passed (52)`。 - 需要你做的:无,席位落地。 ## What changes - **`packages/core/src/qa/http-adapter.ts`**: the constructor stores `baseUrl` with its trailing slashes stripped (`private readonly baseUrl`), once. Every URL the adapter builds reads that field: the discovery probe, `collectionUrl` / `recordUrl`, `api_call`'s relative target, and the record-actions provenance clause. `HttpTestAdapter` has one caller (`packages/cli/src/commands/test.ts`, which hands it `--url` verbatim), so normalising in the adapter covers it and any future caller; `Test.run` is unchanged. No flag, no URL validation: a malformed `--url` behaves as it did. - `rawApiCall` hands `handleResponse` the clause `api_call addressed METHOD URL`, so an `api_call` that fails 404 or 405 ends its existing `HTTP Error STATUS: BODY` sentence with one added clause naming the URL it addressed. The 404/405 gate is the one record actions already use: other statuses reached a route, so the server's own message stays undecorated. `handleResponse`'s parameter is renamed `mountNote` → `addressNote`, since it now carries either. - **`.changeset/28946-qa-base-url-trailing-slash.md`**: `@objectstack/core` patch with both declaration lines. The CLI is unchanged (its dist imports `@objectstack/core` externally), so it carries no changeset. ## Tests - New describe at the end of `http-adapter.test.ts`, three rows (`http://localhost:3000/`, `http://localhost:3000//`, `http://localhost:3000/prefix/`) × four cases, against a captured `fetch`: the probe plus the record collection URL carry one slash; an `api_call` relative target carries one slash; the record-actions clause names the normalised base and probe; an `api_call` 404 names the normalised URL. - The existing `[#7983] an api_call failure is not decorated with a mount it never used` pinned the exact undecorated 404 message. Per the order's ruling it now asserts the added URL clause and still asserts no mount clause. A new case pins that an `api_call` 422 stays undecorated. - **Ablation 1** (`scripts/ablation-replace.mjs`, anchor hit once, blob `3c016586` → `f4f31939`): the constructor kept `baseUrl` verbatim. 12 failed / 40 passed; for example, expected `http://localhost:3000/api/v1/objects/crm_account`, received `http://localhost:3000//api/v1/data/crm_account` (the doubled-slash probe misses discovery, too). Restored: blob equals HEAD's, `git diff HEAD` empty. - **Ablation 2** (blob `3c016586` → `43092308`): `rawApiCall` passed no clause. 4 failed / 48 passed (the rewritten #7983 case and the three per-row `api_call` 404 cases). Restored the same way. - The test imports `./http-adapter.js` from source, so no build sits on the ablation path. ## Acceptance notes - Measured on a stock showcase boot (`objectstack dev --seed-admin --fresh`, admin token from `sign-in/email`) before and after, with the showcase's `qa/platform-smoke.test.json` and a two-scenario suite (one `create_record`, one `api_call`): | `--url` | base `135054eb` | head `4ff51b55` | |---|---|---| | `http://localhost:PORT` | 2/2 pass | 2/2 pass | | `http://localhost:PORT/` | 0/2, 404 | 2/2 pass | | `http://localhost:PORT//` | 0/2, 404 | 2/2 pass | | `http://localhost:PORT/prefix/` | 0/2, 404, record clause names `.../prefix//api/v1/data` | 0/2, 404 (the server is not under a prefix), and each failure names its URL: `api_call addressed GET http://localhost:PORT/prefix/api/v1/health`; `record actions addressed http://localhost:PORT/prefix/api/v1/data` | - Before the fix, an `api_call` 404 printed only `HTTP Error 404: {"success":false,"error":{"code":"ENDPOINT_NOT_FOUND","message":"Not found"}}`, with no URL. ## Verification All at head `4ff51b55` (base `135054eb`, `origin/main` when the branch was cut). <details><summary>Package suite, typecheck, lint (under the verify lock)</summary> - `pnpm --filter @objectstack/core test`: Test Files 129 passed (129), Tests 2905 passed (2905). - `pnpm --filter @objectstack/core typecheck` (`tsc --noEmit` plus `check:test-typecheck` over `tsconfig.test.json`, which covers the test file): exit 0. - `pnpm exec eslint --no-inline-config packages/core`: exit 0. - `test:repo` (core's `repo` vitest project) was not run locally: the diff touches no census pin and the derivation does not name it. CI Test Core runs it. </details> <details><summary>Derived gates (dispatch-gates --commands, 59 families)</summary> - 58 run with exit 0, 1 NOT MEASURED. `dispatch-gates.mjs --ran`: "59 derived famil(ies) accounted for — 58 run, 1 NOT-MEASURED (1 DERIVED from a recorded exit 3)". - NOT MEASURED: `pnpm check:dual-build-cjs-loads`, exit 3 (PREREQUISITE NOT MET: packages outside the showcase closure have no `dist`). CI runs it. - `check-plugin-teardown-shape.mjs --self-test` ran after `git fetch --depth=1 origin 621a487` (the shallow-clone prerequisite). </details> CI check-runs: read by the seat on this head. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01FEkQ5nPQ5s2XpToMJtoBDm --- _Generated by [Claude Code](https://claude.ai/code/session_01FEkQ5nPQ5s2XpToMJtoBDm)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent d8c611d commit 4df6292

3 files changed

Lines changed: 104 additions & 7 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@objectstack/core': patch
3+
---
4+
5+
fix(core): `os test --url http://host:PORT/` addresses the same endpoints as the URL without the slash, and a failed `api_call` names the URL it addressed (#28946)
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
`HttpTestAdapter` now strips trailing slashes from its base URL once, in its constructor, so every URL it builds (the discovery probe, the record collection URLs, an `api_call` relative target and the provenance clauses that name them) reads the same base. Before, each builder appended a path starting with `/` to the base as given, so a `--url` copied with its trailing slash addressed `//api/v1/...` and a stock server answered every HTTP scenario `404 ENDPOINT_NOT_FOUND`. A base with a path prefix (`http://host/prefix/`) and repeated slashes are normalised the same way. Nothing else about `--url` is validated, so a malformed value behaves as it did.
11+
12+
An `api_call` step that fails 404 or 405 now appends ` — api_call addressed METHOD URL` to its `HTTP Error STATUS: BODY` sentence, as record actions already append the mount they addressed. Other statuses stay undecorated, so the server's own message stands alone.

‎packages/core/src/qa/http-adapter.test.ts‎

Lines changed: 72 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -361,13 +361,22 @@ describe('[#7848] the three non-record action types are unchanged', () => {
361361
expect(calls[0].url).toBe(`${BASE_URL}/api/2026-01/data/crm_account`);
362362
});
363363

364-
it('[#7983] an api_call failure is not decorated with a mount it never used', async () => {
364+
it('[#7983] an api_call failure is not decorated with a mount it never used — it names the URL it addressed', async () => {
365365
fetchMock.mockImplementation(async () => jsonResponse({ error: 'Not found' }, 404));
366366
const adapter = new HttpTestAdapter(BASE_URL);
367367
const err = await adapter
368368
.execute(action('api_call', '/nope', { method: 'GET' }), {})
369369
.then(() => undefined, (e: Error) => e);
370-
expect(err?.message).toBe('HTTP Error 404: {"error":"Not found"}');
370+
expect(err?.message).toBe(`HTTP Error 404: {"error":"Not found"} — api_call addressed GET ${BASE_URL}/nope`);
371+
expect(err?.message).not.toContain('record actions addressed');
372+
});
373+
374+
it('an api_call 422 stays undecorated — it reached a route and the server\'s message stands alone', async () => {
375+
fetchMock.mockImplementation(async () => jsonResponse({ error: 'name is required' }, 422));
376+
const err = await new HttpTestAdapter(BASE_URL)
377+
.execute(action('api_call', '/api/v1/data/crm_account', { method: 'POST', body: {} }), {})
378+
.then(() => undefined, (e: Error) => e);
379+
expect(err?.message).toBe('HTTP Error 422: {"error":"name is required"}');
371380
});
372381

373382
it('wait resolves without touching the network', async () => {
@@ -452,3 +461,64 @@ describe('[#20289] readTargetServices hands the runner the SAME probe\'s `servic
452461
expect(warnings).toEqual([]);
453462
});
454463
});
464+
465+
/**
466+
* PIN — the base URL is normalised once, so a trailing slash on `--url` does
467+
* not double the slash in front of every path the adapter builds.
468+
*
469+
* Each URL builder appends a path that starts with `/`; `os test --url
470+
* http://host:PORT/` kept the slash and addressed `//api/v1/...`, which a stock
471+
* server answers `404 ENDPOINT_NOT_FOUND` — every HTTP scenario failed while the
472+
* same URL without the slash passed. Pinned per builder (discovery probe, record
473+
* collection URL, `api_call` relative target, the provenance clause) against a
474+
* captured `fetch`, for one slash, several, and a base that carries a path.
475+
*/
476+
describe('a trailing slash on the base URL addresses the same endpoints', () => {
477+
const rows: Array<[label: string, given: string, normalised: string]> = [
478+
['one trailing slash', `${BASE_URL}/`, BASE_URL],
479+
['two trailing slashes', `${BASE_URL}//`, BASE_URL],
480+
['a path prefix with a trailing slash', 'http://localhost:3000/prefix/', 'http://localhost:3000/prefix'],
481+
];
482+
483+
for (const [label, given, normalised] of rows) {
484+
describe(label, () => {
485+
const discoveryUrl = `${normalised}${EXPECTED_API_BASE}/discovery`;
486+
487+
beforeEach(() => {
488+
fetchMock.mockImplementation(async (input: unknown, init?: RequestInit) => {
489+
calls.push({ url: String(input), method: init?.method ?? 'GET', body: undefined, headers: {} });
490+
if (String(input) === discoveryUrl) return discoveryDocument({ data: ADVERTISED_DATA_PATH });
491+
return jsonResponse({ ok: true });
492+
});
493+
});
494+
495+
it('the discovery probe and the record collection URL carry one slash', async () => {
496+
await new HttpTestAdapter(given).execute(action('create_record', 'crm_account', { name: 'Acme' }), {});
497+
expect(calls.map((c) => c.url)).toEqual([discoveryUrl, `${normalised}${ADVERTISED_DATA_PATH}/crm_account`]);
498+
expect(warnings).toEqual([]);
499+
});
500+
501+
it('an api_call relative target carries one slash', async () => {
502+
await new HttpTestAdapter(given).execute(action('api_call', '/api/v1/health', { method: 'GET' }), {});
503+
expect(calls.map((c) => c.url)).toEqual([`${normalised}/api/v1/health`]);
504+
});
505+
506+
it('the record-action provenance clause names the normalised base', async () => {
507+
fetchMock.mockImplementation(async () => jsonResponse({ error: 'Not found' }, 404));
508+
const err = await new HttpTestAdapter(given)
509+
.execute(action('read_record', 'crm_account', { id: 'rec_1' }), {})
510+
.then(() => undefined, (e: Error) => e);
511+
expect(err?.message).toContain(`record actions addressed ${normalised}${EXPECTED_DATA_PATH} (`);
512+
expect(err?.message).toContain(`GET ${discoveryUrl} answered 404`);
513+
});
514+
515+
it('an api_call 404 names the normalised URL it addressed', async () => {
516+
fetchMock.mockImplementation(async () => jsonResponse({ error: 'Not found' }, 404));
517+
const err = await new HttpTestAdapter(given)
518+
.execute(action('api_call', '/api/v1/health', { method: 'GET' }), {})
519+
.then(() => undefined, (e: Error) => e);
520+
expect(err?.message).toBe(`HTTP Error 404: {"error":"Not found"} — api_call addressed GET ${normalised}/api/v1/health`);
521+
});
522+
});
523+
}
524+
});

‎packages/core/src/qa/http-adapter.ts‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,20 @@ export class HttpTestAdapter implements TestExecutionAdapter {
100100
/** The data mount derived from that probe, memoised so its fallback warns once. */
101101
private mountPromise?: Promise<ResolvedDataMount>;
102102

103-
constructor(private baseUrl: string, private authToken?: string) {}
103+
/**
104+
* The target origin (plus any path prefix) with its trailing slashes
105+
* stripped, normalised ONCE here so every URL this adapter builds — the
106+
* discovery probe, the record collection URLs, `api_call`'s relative target
107+
* and the provenance clauses that name them — reads the same base. Each
108+
* builder appends a path that starts with `/`, so `--url http://host:PORT/`
109+
* kept verbatim addressed `//api/v1/...`, which the server answers 404.
110+
* Nothing else is validated: a malformed `--url` behaves as it did.
111+
*/
112+
private readonly baseUrl: string;
113+
114+
constructor(baseUrl: string, private authToken?: string) {
115+
this.baseUrl = baseUrl.replace(/\/+$/, '');
116+
}
104117

105118
/** The run's discovery read; probes at most once per adapter. */
106119
private discovery(): Promise<DiscoveryRead> {
@@ -370,15 +383,17 @@ export class HttpTestAdapter implements TestExecutionAdapter {
370383
headers,
371384
body
372385
});
373-
return this.handleResponse(response);
386+
// The URL it addressed, never a mount: `api_call` consulted none.
387+
return this.handleResponse(response, `api_call addressed ${method} ${url}`);
374388
}
375389

376-
private async handleResponse(response: Response, mountNote?: string) {
390+
private async handleResponse(response: Response, addressNote?: string) {
377391
if (!response.ok) {
378392
const text = await response.text();
379393
// The `HTTP Error <status>: <body>` prefix is unchanged — a suite author
380394
// (and every transcript predating #7983) reads the same first clause;
381-
// the mount provenance is APPENDED, never substituted.
395+
// the mount provenance (or, for `api_call`, the URL it addressed) is
396+
// APPENDED, never substituted.
382397
//
383398
// Appended on 404/405 only: those are the statuses a wrong mount
384399
// produces (nothing registered at the path / registered under another
@@ -388,7 +403,7 @@ export class HttpTestAdapter implements TestExecutionAdapter {
388403
// under a URL the author has no reason to doubt.
389404
const wrongUrlShaped = response.status === 404 || response.status === 405;
390405
throw new Error(
391-
`HTTP Error ${response.status}: ${text}${wrongUrlShaped && mountNote ? ` — ${mountNote}` : ''}`,
406+
`HTTP Error ${response.status}: ${text}${wrongUrlShaped && addressNote ? ` — ${addressNote}` : ''}`,
392407
);
393408
}
394409
const contentType = response.headers.get('content-type');

0 commit comments

Comments
 (0)