From 82fbb2606a3775cd0428d3699d6862a66c9d976b Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 08:53:22 +0000 Subject: [PATCH 1/2] fix(core,cli): os test record action types reach the served route; zero-match glob states its posture (#7848) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Item 1 — `HttpTestAdapter` built `${baseUrl}/api/data/:object` while a stock server serves `{apiPath}/data/:object` with `apiPath` = `/api/v1`, so all five record-shaped `TestActionTypeSchema` members 404'd, and `update_record` issued `PUT` where the route is `PATCH`. The prefix is now derived from the two schemas `RestServer` itself resolves from (`RestApiConfigSchema` + `CrudEndpointsConfigSchema.dataPrefix`) rather than written down a second time, `update_record` PATCHes with `id` peeled off the body, and record ids are percent-encoded. Item 2 — a zero-match glob still exits 0 (a repo that legitimately ships no suites must not start failing CI), but the posture is now declared: `--help` states it, `--fail-on-empty` opts into the strict reading, and `Found N test suites.` is emitted on every run including `Found 0 test suites.` Both exit-code arms are asserted over a real child process; the adapter's URLs and verbs are pinned against the spec schemas rather than a copied literal. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01D1z19epfecapa69CqwZ3Mm --- ...a-http-adapter-base-path-and-empty-glob.md | 55 +++++ content/docs/deployment/cli.mdx | 17 ++ packages/cli/src/commands/test.ts | 48 ++++- .../test/qa-empty-glob-exit-code.e2e.test.ts | 122 ++++++++++++ packages/core/src/qa/http-adapter.test.ts | 188 ++++++++++++++++++ packages/core/src/qa/http-adapter.ts | 73 ++++++- packages/spec/liveness/qa.json | 2 +- 7 files changed, 492 insertions(+), 13 deletions(-) create mode 100644 .changeset/qa-http-adapter-base-path-and-empty-glob.md create mode 100644 packages/cli/test/qa-empty-glob-exit-code.e2e.test.ts create mode 100644 packages/core/src/qa/http-adapter.test.ts diff --git a/.changeset/qa-http-adapter-base-path-and-empty-glob.md b/.changeset/qa-http-adapter-base-path-and-empty-glob.md new file mode 100644 index 0000000000..970f9c7ccb --- /dev/null +++ b/.changeset/qa-http-adapter-base-path-and-empty-glob.md @@ -0,0 +1,55 @@ +--- +"@objectstack/core": patch +"@objectstack/cli": minor +--- + +fix(core,cli): `os test`'s record action types reach the served route, and a zero-match glob states its posture (#7848) + +Two defects on the same surface, both measured on a booted showcase while +authoring the `qa` platform-checklist item. + +## 5 of the 8 declared action types could not reach a stock server + +`HttpTestAdapter` built `${baseUrl}/api/data/:object`. A stock server serves +`{apiPath}/data/:object` with `apiPath` = `/api/v1`, so every record-shaped +member of `TestActionTypeSchema` was one version segment short and answered +`HTTP Error 404: {"error":"Not found"}` — `create_record`, `read_record`, +`update_record`, `delete_record` and `query_records`. `update_record` was wrong +twice: it issued `PUT` where the route is `PATCH`, and there is no `PUT` +sibling to fall back on. Only `api_call` and `wait` executed, which is why the +gap survived — everything the Quality Protocol had been used for so far was +expressible through `api_call`. + +All five now address the route the server registers, and `update_record` uses +`PATCH` with `id` peeled off the body (the body is the field patch, not a +column write). The prefix is no longer written down: it is derived from the two +schemas `RestServer` itself resolves from — `RestApiConfigSchema` +(`apiPath ?? {basePath}/{version}`) and `CrudEndpointsConfigSchema.dataPrefix` +— so the adapter's default cannot drift from the declaration again. Defaults +only: a deployment that overrides `api.apiPath` or `crud.dataPrefix` is still +out of reach for the record action types, and `api_call` remains the escape +hatch there. + +`run_script` still has no adapter branch and still throws by name; nothing here +implements it. + +## A run that loaded no suite reported success silently + +`os test 'qa/nothing-matches-*.test.json'` exited **0** after executing nothing, +so a CI step whose glob stopped matching (a renamed directory, a moved suite) +reported success forever. + +The default exit status is deliberately unchanged — a repository that +legitimately ships no suites must not begin failing CI. What changes is that the +posture is now **declared** rather than accidental: + +- `os test --help` states it: a pattern matching no suite prints + `Found 0 test suites.` and exits 0; +- **new flag `--fail-on-empty`** opts into the strict reading and exits 1 on an + empty match; +- `Found N test suites.` is emitted on **every** run, `Found 0 test suites.` + included. It was previously printed only when the count was positive — absent + from exactly the run where a caller needs it to tell "every suite passed" from + "there were no suites". + +Both exit-code arms now carry explicit assertions over a real child process. diff --git a/content/docs/deployment/cli.mdx b/content/docs/deployment/cli.mdx index ebabbfd44a..4d1bc518fe 100644 --- a/content/docs/deployment/cli.mdx +++ b/content/docs/deployment/cli.mdx @@ -1018,6 +1018,7 @@ os test qa/my-test.json # Specific test file os test --url http://localhost:4000 # Custom server URL os test --token my-api-key # With authentication os test 'qa/**/*.test.json' # Recursive — quote it, or the shell expands it first +os test --fail-on-empty # Matching no suite is a failure, not a pass ``` The pattern accepts `*` (one path segment) and `**` (any number of segments); @@ -1044,6 +1045,22 @@ switch and reported ✅, so a `contains` against a missing path was a test that silently deleted itself. Assert absence with `is_null`; compare a scalar with `equals`. +**A pattern that matches no suite is not a failure by default.** The run prints +`Found 0 test suites.` — the same machine-readable line a full run prints, so a +caller can tell "every suite passed" from "there were no suites" — and exits +**0**, because a project that legitimately ships no suites should not fail its +build. That is a posture, not an oversight, and it has the cost you would expect: +a CI step whose glob stops matching (a renamed directory, a moved suite) reports +success forever. Pass **`--fail-on-empty`** to opt into the strict reading, where +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. + #### `os doctor` Checks your development environment and reports issues: diff --git a/packages/cli/src/commands/test.ts b/packages/cli/src/commands/test.ts index fa146a36ea..0365c7deec 100644 --- a/packages/cli/src/commands/test.ts +++ b/packages/cli/src/commands/test.ts @@ -231,8 +231,39 @@ export function loadTestSuite(file: string): QA.TestSuite { return result.data as QA.TestSuite; } +/** + * The suite-count line, in the ONE spelling a caller may match on (#7848). + * + * `Found N test suites.` was already printed — but only when N was positive, so + * the single number a CI step needs in order to tell "every suite passed" from + * "there were no suites" was missing from exactly the run where it mattered. + * It is emitted on every run now, `Found 0 test suites.` included, and the + * wording lives here rather than inline so an edit to the prose has to notice + * it is editing a machine-readable surface. + */ +export function foundSuitesLine(count: number): string { + return `Found ${count} test suites.`; +} + export default class Test extends Command { - static override description = 'Run Quality Protocol test scenarios against a running server'; + /** + * The empty-match posture is stated in the help text on purpose (#7848). + * + * `os test 'qa/nothing-matches-*.test.json'` exits 0 — a green exit from a run + * that executed nothing, which is the #7347 `coverage.json` shape (a check that + * checked nothing, reporting success). The exit status is nevertheless kept as + * it is: a repository that legitimately ships no suites must not start failing + * CI because we changed our minds about a number nobody documented. So the + * posture becomes DECLARED, and `--fail-on-empty` makes the strict reading + * available to the callers that want it. + */ + static override description = + 'Run Quality Protocol test scenarios against a running server.\n' + + 'A pattern that matches no suite is NOT a failure by default: the run prints ' + + '"Found 0 test suites." and exits 0, so a repository that legitimately ships no ' + + 'suites does not fail CI. Pass --fail-on-empty for the strict reading, where a ' + + 'pattern that has stopped matching (a renamed directory, a moved suite) fails the ' + + 'step instead of reporting success forever.'; static override args = { files: Args.string({ description: 'Glob pattern for test files (e.g. "qa/*.test.json")', required: false, default: 'qa/*.test.json' }), @@ -241,6 +272,10 @@ export default class Test extends Command { static override flags = { url: Flags.string({ description: 'Target base URL', default: 'http://localhost:3000' }), token: Flags.string({ description: 'Authentication token' }), + 'fail-on-empty': Flags.boolean({ + description: 'Exit non-zero when the pattern matches no test suite (default: matching nothing exits 0)', + default: false, + }), }; async run(): Promise { @@ -259,12 +294,19 @@ export default class Test extends Command { const testFiles: string[] = resolveGlob(filesPattern); if (testFiles.length === 0) { + // The count line comes FIRST and unconditionally: a caller parsing stdout + // gets the same statement on an empty run as on a full one. + console.log(foundSuitesLine(0)); console.warn(chalk.yellow(`No test files found matching: ${filesPattern}`)); - // Create a demo test file if none exist? + if (flags['fail-on-empty']) { + console.error(chalk.red(`--fail-on-empty: a run that loaded no suite is a failed run.`)); + process.exit(1); + } + console.log(chalk.dim(`Exiting 0 — an empty match is not a failure. Pass --fail-on-empty to make it one.`)); return; } - console.log(`Found ${testFiles.length} test suites.`); + console.log(foundSuitesLine(testFiles.length)); // 3. Run Tests let totalPassed = 0; diff --git a/packages/cli/test/qa-empty-glob-exit-code.e2e.test.ts b/packages/cli/test/qa-empty-glob-exit-code.e2e.test.ts new file mode 100644 index 0000000000..e47285b576 --- /dev/null +++ b/packages/cli/test/qa-empty-glob-exit-code.e2e.test.ts @@ -0,0 +1,122 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * PIN (#7848) — what the SHELL sees when `os test` matches no suite. + * + * ``` + * $ os test 'qa/nothing-matches-*.test.json' + * No test files found matching: qa/nothing-matches-*.test.json + * $ echo $? + * 0 + * ``` + * + * A green exit from a run that executed nothing — the #7347 `coverage.json` + * shape, where a check that checked nothing kept reporting success. The exit + * status is deliberately KEPT at 0: a repository that legitimately ships no + * suites must not begin failing CI because we changed our minds about a number + * nobody had written down. What changes is that the posture is now declared + * (`--help`), the count is machine-readable on every run (`Found 0 test + * suites.`, previously printed only when the count was positive), and + * `--fail-on-empty` makes the strict reading available. + * + * Both arms are asserted here, and "exits 0" is the one that had to be: an exit + * status nobody asserts is exactly the behaviour that changes by accident, and + * the whole reason this issue exists is that a green nobody looked at survived. + * + * It has to be a real child process. `process.exit(1)` inside a vitest worker + * is not an exit status — the number a shell, a `set -e` script or a CI step + * reads only exists once Node has exited. Spawned through `bin/run-dev.js` + + * tsx (the pattern `migrate-exit-code.e2e.test.ts` and `emit-json-pipe.test.ts` + * already use) so the suite does not depend on `packages/cli/dist` having been + * built. No server is contacted on this path — the command resolves the glob + * before it sends anything — so the run needs neither a boot nor a config. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { execFile } from 'node:child_process'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const HERE = resolve(fileURLToPath(import.meta.url), '..'); +const CLI = resolve(HERE, '../bin/run-dev.js'); +const TSX = resolve(HERE, '../../../node_modules/.bin/tsx'); + +/** Matches nothing, in a temp dir that contains nothing. */ +const EMPTY_PATTERN = 'qa/nothing-matches-*.test.json'; + +/** oclif + tsx cold start; a healthy run here is ~2-5 s. */ +const RUN_TIMEOUT_MS = 120_000; + +interface Run { + code: number; + stdout: string; + stderr: string; +} + +function runCli(args: string[], cwd: string): Promise { + return new Promise((resolvePromise) => { + execFile( + TSX, + [CLI, ...args], + { cwd, maxBuffer: 8 * 1024 * 1024, env: { ...process.env, NO_COLOR: '1' } }, + (err, stdout, stderr) => { + resolvePromise({ + // `err.code` is the real exit status; `null`/undefined means the child + // was signalled — a failure of a different kind, never reported as 0. + code: err ? (typeof (err as { code?: unknown }).code === 'number' ? (err as unknown as { code: number }).code : 1) : 0, + stdout: String(stdout), + stderr: String(stderr), + }); + }, + ); + }); +} + +let dir: string; +let lenient: Run; +let strict: Run; + +beforeAll(async () => { + dir = mkdtempSync(join(tmpdir(), 'os-qa-empty-glob-')); + lenient = await runCli(['test', EMPTY_PATTERN], dir); + strict = await runCli(['test', EMPTY_PATTERN, '--fail-on-empty'], dir); +}, RUN_TIMEOUT_MS * 2); + +afterAll(() => { + rmSync(dir, { recursive: true, force: true }); +}); + +describe('[#7848] `os test` with a zero-match glob', () => { + it('exits 0 without the flag — the declared posture, now asserted', () => { + expect(lenient.code).toBe(0); + }); + + it('exits non-zero with --fail-on-empty', () => { + expect(strict.code).not.toBe(0); + expect(strict.code).toBe(1); + }); + + it('prints the machine-readable count on the empty run, in both arms', () => { + // The number a CI step needs to tell "all suites passed" from "there were + // no suites" — absent, before this change, from exactly that run. + expect(lenient.stdout).toContain('Found 0 test suites.'); + expect(strict.stdout).toContain('Found 0 test suites.'); + }); + + it('still names the pattern that matched nothing', () => { + expect(`${lenient.stdout}${lenient.stderr}`).toContain(`No test files found matching: ${EMPTY_PATTERN}`); + }); + + it('says out loud that the zero exit is a posture, not an oversight', () => { + expect(lenient.stdout).toContain('--fail-on-empty'); + }); + + it('states the posture in --help', async () => { + const help = await runCli(['test', '--help'], dir); + expect(help.code).toBe(0); + expect(`${help.stdout}${help.stderr}`).toContain('--fail-on-empty'); + expect(`${help.stdout}${help.stderr}`).toContain('Found 0 test suites.'); + }, RUN_TIMEOUT_MS); +}); diff --git a/packages/core/src/qa/http-adapter.test.ts b/packages/core/src/qa/http-adapter.test.ts new file mode 100644 index 0000000000..a486fe7cca --- /dev/null +++ b/packages/core/src/qa/http-adapter.test.ts @@ -0,0 +1,188 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * PIN (#7848) — the record action types address the route the server ACTUALLY + * serves. + * + * `HttpTestAdapter` built `${baseUrl}/api/data/:object`. A stock server serves + * `{apiPath}/data/:object` with `apiPath` = `/api/v1`, so every record-shaped + * member of `TestActionTypeSchema` was one version segment short. Measured + * verbatim against a booted showcase while authoring the `qa` checklist item + * for #7347: + * + * create_record → HTTP Error 404: {"error":"Not found"} + * read_record → 404 + * update_record → 404, and `PUT` where the route is `PATCH` — wrong twice + * delete_record → 404 + * query_records → 404 + * api_call / wait → executed (which is why the gap survived: everything the + * Quality Protocol had been used for was expressible through + * `api_call`) + * run_script → no adapter branch, throws loudly (liveness ledger) + * + * So 5 of 8 declared action types could not do the thing their name promises, + * and the suite author reading that 404 has every reason to think it is their + * 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. + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { RestApiConfigSchema, CrudEndpointsConfigSchema } from '@objectstack/spec/api'; +import type * as QA from '@objectstack/spec/qa'; +import { HttpTestAdapter } from './http-adapter.js'; + +const BASE_URL = 'http://localhost:3000'; + +/** 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}`; +})(); + +interface Call { + url: string; + method: string; + body: unknown; + headers: Record; +} + +let calls: Call[]; +let fetchMock: ReturnType; + +function jsonResponse(body: unknown, status = 200): Response { + return new Response(JSON.stringify(body), { + status, + headers: { 'content-type': 'application/json' }, + }); +} + +beforeEach(() => { + calls = []; + fetchMock = vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + calls.push({ + url: String(input), + method: init?.method ?? 'GET', + body: typeof init?.body === 'string' ? JSON.parse(init.body) : undefined, + headers: (init?.headers ?? {}) as Record, + }); + return jsonResponse({ ok: true }); + }); + vi.stubGlobal('fetch', fetchMock); +}); + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +function action(type: QA.TestActionType, target: string, payload?: Record): QA.TestAction { + return { type, target, ...(payload ? { payload } : {}) } as QA.TestAction; +} + +async function run(a: QA.TestAction): Promise { + const adapter = new HttpTestAdapter(BASE_URL); + await adapter.execute(a, {}); + expect(calls).toHaveLength(1); + return calls[0]; +} + +describe('[#7848] HttpTestAdapter record action types reach the served route', () => { + it('derives the data path 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'); + }); + + 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.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.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.method).toBe('PATCH'); + // `id` addressed the record; the body is the field patch, not a column write. + expect(call.body).toEqual({ name: 'Acme II' }); + }); + + 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.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.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`); + }); + + it('no record action type addresses the old unversioned `/api/data` path', async () => { + for (const type of ['create_record', 'read_record', 'update_record', 'delete_record', 'query_records'] as const) { + 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); + } + }); +}); + +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'); + }); + + it('api_call leaves an absolute target alone', async () => { + const call = await run(action('api_call', 'http://example.test/health', { method: 'GET' })); + expect(call.url).toBe('http://example.test/health'); + }); + + 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 }), {}); + expect(result).toEqual({ waited: 1 }); + expect(calls).toHaveLength(0); + }); + + it('run_script still throws — no adapter branch, and this PR does not add one', async () => { + const adapter = new HttpTestAdapter(BASE_URL); + await expect(adapter.execute(action('run_script', 'doThing'), {})).rejects.toThrow( + /Unsupported action type in HttpAdapter: run_script/, + ); + }); +}); + +describe('[#7848] auth and impersonation headers still ride along', () => { + it('sends the bearer token and X-Run-As', async () => { + const adapter = new HttpTestAdapter(BASE_URL, 'tok_123'); + await adapter.execute( + { 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'); + }); +}); diff --git a/packages/core/src/qa/http-adapter.ts b/packages/core/src/qa/http-adapter.ts index 943ccc0f28..0e5c5cbe0c 100644 --- a/packages/core/src/qa/http-adapter.ts +++ b/packages/core/src/qa/http-adapter.ts @@ -1,11 +1,61 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. 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; + +/** + * The path prefix a stock ObjectStack server serves the Data Protocol under. + * + * ## [#7848] Why this is derived and not written down + * + * Every record-shaped action type used to build `${baseUrl}/api/data/:object` — + * a literal, one version segment short of the route the server registers, so + * `create_record`, `read_record`, `update_record`, `delete_record` and + * `query_records` (5 of the 8 declared `TestActionTypeSchema` members) answered + * `HTTP Error 404: {"error":"Not found"}` against a stock boot. The suite author + * reading that 404 has every reason to think it is their own URL. + * + * Replacing one literal with a corrected literal only moves the drift: the + * server composes this path out of two declared pieces, and both of them are + * configurable. So this asks the SAME schemas the server's own resolution asks: + * + * - `RestApiConfigSchema` → `apiPath ?? `${basePath}/${version}`` — the exact + * expression `RestServer.getApiBasePath()` evaluates (`/api` + `v1`); + * - `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. + */ +function defaultDataPath(): string { + if (dataPathCache === undefined) { + const api = RestApiConfigSchema.parse({}); + const crud = CrudEndpointsConfigSchema.parse({}); + dataPathCache = `${api.apiPath ?? `${api.basePath}/${api.version}`}${crud.dataPrefix}`; + } + return dataPathCache; +} + export class HttpTestAdapter implements TestExecutionAdapter { 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)}`; + } + + /** `{collection}/{id}` — the single-record URL. */ + private recordUrl(objectName: string, id: unknown): string { + return `${this.collectionUrl(objectName)}/${encodeURIComponent(String(id))}`; + } + async execute(action: QA.TestAction, _context: Record): Promise { const headers: Record = { 'Content-Type': 'application/json', @@ -40,7 +90,7 @@ export class HttpTestAdapter implements TestExecutionAdapter { } private async createRecord(objectName: string, data: Record, headers: Record) { - const response = await fetch(`${this.baseUrl}/api/data/${objectName}`, { + const response = await fetch(this.collectionUrl(objectName), { method: 'POST', headers, body: JSON.stringify(data) @@ -49,12 +99,16 @@ export class HttpTestAdapter implements TestExecutionAdapter { } private async updateRecord(objectName: string, data: Record, headers: Record) { - const id = data.id; + const { id, ...fields } = data; if (!id) throw new Error('Update record requires id in payload'); - const response = await fetch(`${this.baseUrl}/api/data/${objectName}/${id}`, { - method: 'PUT', + // PATCH, not PUT: `PATCH {apiPath}/data/: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), { + method: 'PATCH', headers, - body: JSON.stringify(data) + body: JSON.stringify(fields) }); return this.handleResponse(response); } @@ -62,7 +116,7 @@ export class HttpTestAdapter implements TestExecutionAdapter { 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.baseUrl}/api/data/${objectName}/${id}`, { + const response = await fetch(this.recordUrl(objectName, id), { method: 'DELETE', headers }); @@ -72,7 +126,7 @@ export class HttpTestAdapter implements TestExecutionAdapter { 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.baseUrl}/api/data/${objectName}/${id}`, { + const response = await fetch(this.recordUrl(objectName, id), { method: 'GET', headers }); @@ -80,8 +134,9 @@ export class HttpTestAdapter implements TestExecutionAdapter { } private async queryRecords(objectName: string, data: Record, headers: Record) { - // Assuming query via POST or GraphQL-like endpoint - const response = await fetch(`${this.baseUrl}/api/data/${objectName}/query`, { + // `POST {apiPath}/data/: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`, { method: 'POST', headers, body: JSON.stringify(data) diff --git a/packages/spec/liveness/qa.json b/packages/spec/liveness/qa.json index 2b054a3027..bbe0631104 100644 --- a/packages/spec/liveness/qa.json +++ b/packages/spec/liveness/qa.json @@ -33,7 +33,7 @@ "status": "dead", "verifiedAt": "2026-08-10", "evidenceScope": "in-repo", - "note": "The sharpest row in this file. Its describe() promises 'Tags for filtering and categorization (e.g. \"critical\", \"regression\", \"crm\")' and NOTHING filters on it: `os test` has exactly two flags, `--url` and `--token` (packages/cli/src/commands/test.ts:62-65), the runner never reads `scenario.tags`, and the only selection the command offers is the file glob. So `os test --tags critical` is not a narrower run, it is an unknown-flag error, and a suite tagged `regression` runs on every invocation. This is the entry that would carry `authorWarn` if the type had a channel for one (see `_authorWarnSkipped` in the file note): an author tagging scenarios is buying a filter that does not exist. Enforce-or-remove worklist — the enforce route is a `--tags` filter in the command, the remove route drops the key; either is a decision, not a cleanup." + "note": "The sharpest row in this file. Its describe() promises 'Tags for filtering and categorization (e.g. \"critical\", \"regression\", \"crm\")' and NOTHING filters on it: `os test` has three flags — `--url`, `--token` and `--fail-on-empty` (#7848) — and not one of them selects scenarios, the runner never reads `scenario.tags`, and the only selection the command offers is the file glob. So `os test --tags critical` is not a narrower run, it is an unknown-flag error, and a suite tagged `regression` runs on every invocation. This is the entry that would carry `authorWarn` if the type had a channel for one (see `_authorWarnSkipped` in the file note): an author tagging scenarios is buying a filter that does not exist. Enforce-or-remove worklist — the enforce route is a `--tags` filter in the command, the remove route drops the key; either is a decision, not a cleanup." }, "setup": { "status": "live", From f0e6427c6a4342a2d73f7a549bfe5976364dbff8 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 09:17:18 +0000 Subject: [PATCH 2/2] docs(qa): revise the cli.qa-suite-execution checklist item for the repaired adapter (#7848) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The item's own `negative` clause said a future run finding the record action types PASSING "means the adapter was repaired, which is a revision of this item rather than a silent green" — so this is that revision (revision 2, with its history entry). The five-404s knownGap 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`), the variants carry their new routes and response shapes, and the zero-match negative records the now-declared posture plus `--fail-on-empty`. Also: the core test's fetch mock takes `input: unknown` — this package's tsc program has no DOM lib, so `RequestInfo` does not resolve and the debt ratchet catches it (it compiles *.test.ts, which the package `typecheck` skips). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01D1z19epfecapa69CqwZ3Mm --- docs/qa/platform-checklist/areas/cli.json | 24 ++++++++++++----------- packages/core/src/qa/http-adapter.test.ts | 5 ++++- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/docs/qa/platform-checklist/areas/cli.json b/docs/qa/platform-checklist/areas/cli.json index 1cdcdd5f95..e730b92f59 100644 --- a/docs/qa/platform-checklist/areas/cli.json +++ b/docs/qa/platform-checklist/areas/cli.json @@ -432,7 +432,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": 1, + "revision": 2, "priority": "P1", "surface": "cli", "personas": ["operator (local shell)", "suite author (writes qa/*.test.json)"], @@ -445,7 +445,8 @@ "a scratch directory with its own qa/ for the negative and variant probes — deliberately broken suites are STAGED per run, never committed, so the repo's own suite stays green" ], "knownGaps": [ - "five of the eight action types cannot reach a stock server at all (see `negative`): `HttpTestAdapter` builds `${baseUrl}/api/data/:object` while the server serves `{basePath}/data/:object` with basePath `/api/v1`. The variant sweep therefore records five refusals, and that is the honest verdict — not a fixture gap to work around by rewriting the probes as `api_call`", + "`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`", "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" ] }, @@ -514,20 +515,20 @@ } ], "negative": [ - "a zero-match glob prints `No test files found matching: ` and exits **0** (measured 2026-08-11) — a green exit from a run that loaded no suite. A run record that ticks this item without quoting a `Found N test suites.` line with N > 0 is the false positive this item exists to prevent, and a CI job wired to `os test` with a typo'd path would report success forever", + "a zero-match glob prints `Found 0 test suites.` + `No test files found matching: ` and exits **0** — a green exit from a run that loaded no suite. Since #7848 that is a DECLARED posture (stated in `--help`, opt out with `--fail-on-empty`, which exits 1) rather than an accident, but the default is unchanged and so is the trap: a CI job wired to `os test` with a typo'd path still reports success forever unless it passes the flag. A run record that ticks this item without quoting a `Found N test suites.` line with N > 0 is the false positive this item exists to prevent", "a malformed suite that REACHES the runner — reported passed having executed nothing, or dying as a TypeError inside runSuite with no file named — is the #6247 regression", "a `contains` against a path the result does not carry reporting ✅ is the #7256 regression", - "the five record-shaped action types (`create_record`, `read_record`, `update_record`, `delete_record`, `query_records`) answer **HTTP 404** against a stock server: `HttpTestAdapter` builds `${baseUrl}/api/data/:object` while the server serves `{basePath}/data/:object` with basePath `/api/v1` (and `update_record` issues PUT where the route is PATCH). Measured 2026-08-11 on showcase. A run must record those four-oh-fours as the measured verdict — recording them as 'not applicable' hides the finding, and a future run that finds them PASSING means the adapter was repaired, which is a revision of this item rather than a silent green", + "the five record-shaped action types (`create_record`, `read_record`, `update_record`, `delete_record`, `query_records`) **round-trip** against a stock server, and an **HTTP 404** from any of them is now the regression this clause names. They answered 404 until #7848 — `HttpTestAdapter` built `${baseUrl}/api/data/:object` where the server serves `{apiPath}/data/:object` with `apiPath` = `/api/v1`, and `update_record` issued PUT where the route is PATCH. Both are fixed and the prefix is derived from `RestApiConfigSchema` + `CrudEndpointsConfigSchema` rather than written down, so this cannot regress by a literal drifting. Re-measured 2026-08-12 on showcase, one scenario per member with no shared setup: 7 of 8 execute and assert, `run_script` refuses by name", "satisfying this item by executing the unit pins in `source` instead of driving a booted app is not a run: those pins cover the load boundary, the glob resolver and one assertion operator, and none of them proves a suite reaches a real server — which is why this item carries no `automated` entry" ], "variants": [ "api_call — the only action type that reaches the real API surface (target is a path or absolute URL; `payload.method`/`payload.body` shape the request)", "wait — local, no HTTP; resolves `{ waited: }` from `payload.duration`", - "create_record — refused: POST ${baseUrl}/api/data/:object → 404 (see `negative`)", - "read_record — refused: GET ${baseUrl}/api/data/:object/:id → 404", - "update_record — refused: PUT ${baseUrl}/api/data/:object/:id → 404 (and PUT, where the route is PATCH)", - "delete_record — refused: DELETE ${baseUrl}/api/data/:object/:id → 404", - "query_records — refused: POST ${baseUrl}/api/data/:object/query → 404", + "create_record — POST {apiPath}/data/:object → 201 `{ object, id, record }` (404 before #7848)", + "read_record — GET {apiPath}/data/:object/:id → `{ object, id, record }`; the id comes from `payload.id`", + "update_record — PATCH {apiPath}/data/:object/:id with `payload.id` peeled off the body (it addresses the record, it is not a column write); issued PUT to a route that has no PUT sibling before #7848", + "delete_record — DELETE {apiPath}/data/:object/:id → `{ object, id, success }`", + "query_records — POST {apiPath}/data/:object/query with the QueryAST as the body → `{ object, records }`", "run_script — declared in the enum with NO adapter branch: `Unsupported action type in HttpAdapter: run_script` (loud, recorded in packages/spec/liveness/qa.json)" ], "enumSource": { @@ -539,7 +540,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 `/api/data/...` record routes this item's variants measure against the server's `/api/v1/data/...`)", + "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/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)", @@ -547,7 +548,8 @@ "examples/app-showcase/qa/platform-smoke.test.json (the fixture suite this item drives)" ], "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": 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"} ] }, { diff --git a/packages/core/src/qa/http-adapter.test.ts b/packages/core/src/qa/http-adapter.test.ts index a486fe7cca..fd6331e8a4 100644 --- a/packages/core/src/qa/http-adapter.test.ts +++ b/packages/core/src/qa/http-adapter.test.ts @@ -65,7 +65,10 @@ function jsonResponse(body: unknown, status = 200): Response { beforeEach(() => { calls = []; - fetchMock = vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + // `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. + fetchMock = vi.fn(async (input: unknown, init?: RequestInit) => { calls.push({ url: String(input), method: init?.method ?? 'GET',