From 696a10c061b561f6ad453d56587a0f499299df05 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 14:38:36 +0000 Subject: [PATCH 1/3] Plans: gh-v1 vendors the SDK; C1 in gh-v1#33 gh installs a script extension with git clone alone, so the SDK ships in gh-v1's vendor/ with the commit it came from, pinned as a devDependency. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016NSWj9gL1oCubj5e2anVkb --- plans/001-sdk-adoption-campaign.md | 7 ++++--- plans/002-adopt-sdk-gh-v1.md | 23 +++++++++++++++++------ 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/plans/001-sdk-adoption-campaign.md b/plans/001-sdk-adoption-campaign.md index 7eea774..30dd3ae 100644 --- a/plans/001-sdk-adoption-campaign.md +++ b/plans/001-sdk-adoption-campaign.md @@ -225,8 +225,9 @@ instance: same requests out, same rows in. - **C1 Seam swap.** Re-implement `lib/api.js`'s exported surface (`query`, `queries`, `asset`, `attempt`, `rest`, `meta`, `localize`, `raw`, `blob`, - `permalink`) on `AgilityApiClient`, `require`d from the CJS build. Callers - unchanged; all 99 tests green. Needs A2, A3 (loc2), gap 9. + `permalink`) on the SDK, imported from a vendored copy of its ESM build + (gh installs extensions without npm; see plan 002, *Dependency*). Callers + unchanged; all tests green. Needs A2, A3 (loc2). - **C2 Delete the decodes.** With rows translated by the schema, `isTrue` and `oidOf` drop out of `meta.js`, `resolve.js`, `board.js`; `momentless` comes from the SDK. @@ -361,5 +362,5 @@ ESM" here means finishing the edges: - [ ] S1–S4 slack-bot migrations (slack-bot session, `bolt-migration`) - [ ] B1–B4 slack bot SDK swap — brief: `plans/003-adopt-sdk-k6i-agility-slack-bot.md` - [x] C0 gh-v1 surveyed (2026-11-04) -- [ ] C1–C4 gh-v1 — brief: `plans/002-adopt-sdk-gh-v1.md` +- [ ] C1–C4 gh-v1 — brief: `plans/002-adopt-sdk-gh-v1.md`; C1 in versionone/gh-v1#33 (SDK vendored) - [ ] D1–D4 agility-rooms diff --git a/plans/002-adopt-sdk-gh-v1.md b/plans/002-adopt-sdk-gh-v1.md index c1697ab..f76e335 100644 --- a/plans/002-adopt-sdk-gh-v1.md +++ b/plans/002-adopt-sdk-gh-v1.md @@ -25,16 +25,27 @@ output stay the same; the code that duplicates the SDK goes. in `lib/api.js`, `meta.js`, `resolve.js`, `board.js`, `batch.js`, and `commands/apply.js`, `bulk.js`, `board.js`. -## Dependency +## Dependency: vendored -No registry package yet. Add the first runtime dependency as a pinned commit: -`"@versionone/sdk": "github:versionone/VersionOne.SDK.JavaScript#"`. -The SDK's `prepare` script builds `dist` on install. gh-v1 is ESM now, so use -`import` (the ESM build), not `require`. +gh installs a script extension with a plain `git clone` and never runs npm, +so gh-v1 cannot rely on `node_modules` at run time. The SDK is still a +runtime dependency, but its code ships inside the repo: + +- `vendor/versionone-sdk/` holds the SDK's ESM build (`index.js`, which + imports nothing) and types (`index.d.ts`), plus `SOURCE.json` naming + the commit they came from. `lib/api.js` imports from there. +- `package.json` lists `@versionone/sdk` under `devDependencies`, pinned + to a full commit, so the lockfile records exactly what was vendored. +- To move the pin: change it in `package.json`, then + `npm install && npm run vendor:sdk`, and commit the copy. + `test/vendor.test.js` fails if the copy, `SOURCE.json` and the pin + disagree. +- Publishing the SDK to a registry (plan 001, *Decisions needed*) changes + only where the pin points; the copy stays. ## Steps, one PR each, on branch `sdk-adoption` -1. **C1 Seam swap** (`lib/api.js`). Keep `api(cfg)` and the `Api` typedef. +1. **C1 Seam swap** (`lib/api.js`) — done in versionone/gh-v1#33. Keep `api(cfg)` and the `Api` typedef. Inside, build one `AgilityApiClient({ baseUrl: cfg.v1Url, accessToken })`. Map: `query`/`queries` → `client.assets.query`/`queries`; `asset` → `client.assets.send(cmds, { throwOnFailure: true })` From 966501dfa23001373423e389c114e3323d504809 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 15:35:00 +0000 Subject: [PATCH 2/3] queryV1: a read-only wire module and client.queryV1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit api/asset can run every query query.v1 can, but query.v1 cannot write and answers with rows alone, so it is the safe, simple place to explore a query. Rows are spelled as api/asset spells them (probed 2026-11-06), so they translate through the same schema. - wire/queryV1: query/queries; a page required; POSTs marked idempotent (the endpoint cannot write); a refused query throws QUERY_REJECTED with the server's exception messages, never an empty result. - client.queryV1.query/queries, translated through the client's schema. - Integration (read-only): query.v1 and api/asset give the same rows; a miss is []; an unknown attribute fails the request. - Plans: A4 done; the slack bot keeps query.v1 reads (S2 is now "page every read", B1 maps query to client.queryV1). Spec §6 and README. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016NSWj9gL1oCubj5e2anVkb --- README.md | 1 + plans/001-sdk-adoption-campaign.md | 36 +++++----- plans/003-adopt-sdk-k6i-agility-slack-bot.md | 13 ++-- plans/handoffs/k6i-agility-slack-bot.md | 16 ++--- specs/000-layers.md | 6 +- src/client.ts | 17 +++++ src/index.ts | 2 +- src/wire/index.ts | 1 + src/wire/queryV1.ts | 66 ++++++++++++++++++ test/integration/queryV1.test.ts | 37 ++++++++++ test/unit/queryV1.test.ts | 71 ++++++++++++++++++++ 11 files changed, 230 insertions(+), 36 deletions(-) create mode 100644 src/wire/queryV1.ts create mode 100644 test/integration/queryV1.test.ts create mode 100644 test/unit/queryV1.test.ts diff --git a/README.md b/README.md index c660fbd..1d72302 100644 --- a/README.md +++ b/README.md @@ -22,6 +22,7 @@ npm test ## Public API - `AgilityApiClient` - top-level SDK client (`meta`, `data` for rest-1, `assets` for api/asset, `localize` for loc-2, `schema`), and the services on it: + - `queryV1.query` / `queryV1.queries` - reads through query.v1: the same query bodies as `assets.query`, rows translated the same way, but the endpoint is read-only and answers with rows alone, so it is the safe place to explore a query - `resolve.numbers` / `resolve.ref` - asset numbers and OID tokens to assets, in one request - `assets.sendGuarded` - a batch as a compare-and-set on each asset's ChangeDate; changed assets come back `skipped` - `batch.run` / `batch.reversible` - preview, snapshot, (confirm,) send guarded, and a receipt with a reverse batch diff --git a/plans/001-sdk-adoption-campaign.md b/plans/001-sdk-adoption-campaign.md index 30dd3ae..829ea14 100644 --- a/plans/001-sdk-adoption-campaign.md +++ b/plans/001-sdk-adoption-campaign.md @@ -134,8 +134,11 @@ Found by reading the code above against `src/`: return failures typed (it does) and offer `throwOnFailure` producing an `ApiError` subclass carrying `sourceCommandIndex` per failure. 5. **Missing wire modules** named in spec 000 but not built: `loc2` (both - consumers), `queryV1` (slack's main read path — or migrate slack to - api/asset queries instead; see B3), `attachment`/`EmbeddedImage` (rooms). + consumers), `queryV1` (slack's main read path), `attachment`/ + `EmbeddedImage` (rooms). `queryV1` is built (2026-11-06) and kept on + purpose even though api/asset can run the same queries: query.v1 is + read-only by construction and answers with rows alone, so it is the safe, + simple place to explore a query (and its requests may be retried). 6. **rest-1 writes — not needed.** Settled from the docs (apis/asset-bulk/updating-assets, creating-assets): api/asset carries a change comment as `ChangeComment` inside an update's `update` node, or @@ -166,11 +169,11 @@ moves. - **A2 Command-failure ergonomics** — gap 4: `CommandFailureError`, `send(..., { throwOnFailure })`, `sourceCommandIndex` preserved. - **A3 loc2 wire module** — gap 5, smallest consumer-shared endpoint. -- **A4 Slack-shaped reads** — decide queryV1-module vs. api/asset-only by - porting `lib/queries.js` bodies into integration tests against the dev - instance; whichever reads translate cleanly wins. (No `rest1` writes: - gap 6 is settled.) If the bot's S2 lands first, this is only the - translation check. +- **A4 Slack-shaped reads** — done: a `queryV1` wire module and + `client.queryV1.query`/`queries`, rows translated through the same + schema as api/asset (the integration tier proves both endpoints give the + same rows). The bot keeps query.v1 for its reads. (No `rest1` writes: + gap 6 is settled.) - **A5 Services port** — gap 7. `services/resolve` first (gh-v1's `resolveAssets`; slack's `queryForAssetByNumber` needs it too), then `services/guard` (`guarded()`), then `services/batch` (preview, describe, @@ -200,12 +203,12 @@ bodies in `lib/queries.js`), api/asset (`bulk`, `createWithNumber`, - **B0 Pre-SDK migrations in the bot's own session** (S1–S4 under *Consumer migrations* below). They shrink what the SDK must support: - once S2 moves reads to api/asset, the SDK needs no `queryV1` module and - A4 collapses to "verify the bot's query bodies translate" — and A4's - rest-1-writes question is closed (gap 6). + once S2 pages every read, B1's reads move to `client.queryV1` as they + are; A4's rest-1-writes question is closed (gap 6). - **B1 Client swap.** `createV1Client` keeps its surface; inside, one - `AgilityApiClient({ baseUrl: url, accessToken })`. `query`/`bulk`/`exec` - → `client.assets.*`; `fetchLocalizations` → `loc2` (A3). + `AgilityApiClient({ baseUrl: url, accessToken })`. `query` → + `client.queryV1.queries`; `bulk`/`exec` → `client.assets.*`; + `fetchLocalizations` → `loc2` (A3). `test/v1-service.test.js` stays the parity proof. - **B2 Delete the decodes.** Rows arrive translated: `"True"` comparisons, `IsKanban.toLowerCase()`, `toMomentless` and positional @@ -301,10 +304,9 @@ ESM" here means finishing the edges: - **S1 Finish ESM tooling.** `jsconfig.json` still says `module: commonJS`, `target: es6`; set `NodeNext`/`ES2022` and turn on `checkJs` so editors and `tsc --noEmit` see the real module system (and, later, SDK types). -- **S2 query.v1 → api/asset reads.** Same query bodies; the endpoint is - `api/asset` and results come from `queryResult.results` (positional, one - array per query). Add an explicit `page` to every body. Removes the - SDK's need for a `queryV1` module. +- **S2 Page every read.** Reads stay on query.v1 (read-only, simplest + responses). Add an explicit `page` to every body in `lib/queries.js`, + as the SDK's `queryV1` refuses an unpaged query. - **S3 Close Bolt Phase 2.** Drop `botkit`, `botbuilder-*`, `botkit-plugin-cms` and `axios` from `package.json` once nothing imports them; delete `lib/v1api.js` and the `plugins/` Botkit modules that @@ -355,7 +357,7 @@ ESM" here means finishing the edges: - [x] A1 transport: credentials, onUnauthorized (idempotent requests resent once; writes never), signal (`RequestOptions.signal`, `client.withSignal`) - [x] A2 command-failure error (`CommandFailureError`, `throwOnFailure`, reads always throw) - [x] A3 loc2 (`loc2.localize`, `client.localize`; GET, POST past 2000 encoded chars, culture via Accept-Language) -- [ ] A4 slack-shaped reads (queryV1 or api/asset) +- [x] A4 slack-shaped reads — `queryV1` wire module, `client.queryV1.query`/`queries` (read-only, idempotent, same row translation as api/asset) - [x] A5 services port from gh-v1 — `resolve` (`client.resolve.numbers`/`.ref`), `guard` (`client.assets.sendGuarded`), `batch` (`client.batch.run`/`.reversible`) - [ ] A6 CI + prerelease publish — CI (`.github/workflows/ci.yml`: verify on Node 20/22/24, `npm pack` artifact) and git-SHA installs (`prepare`) done; publish waits on the registry decision - [ ] G0–G3 gh-v1 migrations (gh-v1 session) diff --git a/plans/003-adopt-sdk-k6i-agility-slack-bot.md b/plans/003-adopt-sdk-k6i-agility-slack-bot.md index b9f8131..353b998 100644 --- a/plans/003-adopt-sdk-k6i-agility-slack-bot.md +++ b/plans/003-adopt-sdk-k6i-agility-slack-bot.md @@ -21,7 +21,7 @@ hand-decoding and rest-1 write path go. `fetchLocalizations` (loc-2), and `queryFor*` helpers built on `lib/queries.js`. `test/v1-service.test.js` stubs global `fetch`. - **S1–S4 are not done yet**: `jsconfig.json` still says `commonJS`/`es6`; - reads still post to `query.v1`; `axios`, `botkit*`, `botbuilder-*` are + query bodies have no explicit page; `axios`, `botkit*`, `botbuilder-*` are still dependencies and `lib/v1api.js` still exists. These come first. ## Dependency @@ -34,12 +34,12 @@ The SDK's `prepare` script builds `dist` on install. The bot is ESM, so ## Steps, one PR each 0. **S1–S4 first**, as written in `plans/handoffs/k6i-agility-slack-bot.md`. - B1 needs at least S2 (reads on api/asset) and S3 (axios and Botkit gone). + B1 needs at least S2 (every read paged) and S3 (axios and Botkit gone). If the bot's own session already merged them, skip. 1. **B1 Client swap** (`services/v1.js`). Inside `createV1Client`, build one `new AgilityApiClient({ baseUrl: url, accessToken })`. `query` → - `client.assets.queries` (positional; a failed query now throws instead - of shifting later results); `bulk`/`exec`/`createWithNumber` → + `client.queryV1.queries` (the bot's reads stay on query.v1: read-only, + simplest responses; a refused query throws with the server's message); `bulk`/`exec`/`createWithNumber` → `client.assets.send`; `fetchLocalizations` → `client.localize`; `findAssets` → `client.data.list("BaseAsset", { find, findin, where, sort, page, deleted: false })`. Map `ApiError` with `status === 401` @@ -72,8 +72,9 @@ The SDK's `prepare` script builds `dist` on install. The bot is ESM, so ## Done when - `npm test` passes on each PR; S1's `npm run typecheck` passes. -- `services/v1.js` makes no `fetch` call of its own; nothing posts to - `query.v1` or writes through rest-1. +- `services/v1.js` makes no `fetch` call of its own; reads go through + `client.queryV1` (or `client.data` for `find`), and nothing writes through + rest-1. - No string-boolean comparisons or local `toMomentless` remain. - Sandbox-workspace smoke passes: asset mention, `/agility find`, backlog-item status change, webhook notification. diff --git a/plans/handoffs/k6i-agility-slack-bot.md b/plans/handoffs/k6i-agility-slack-bot.md index 819123b..1dcd988 100644 --- a/plans/handoffs/k6i-agility-slack-bot.md +++ b/plans/handoffs/k6i-agility-slack-bot.md @@ -21,16 +21,12 @@ no `require`). `jsconfig.json` still declares `module: commonJS`, `npm run typecheck` (`tsc -p jsconfig.json`; `typescript` is already a devDependency). Start non-strict; fix or annotate what it finds. -**S2 — Reads from query.v1 to api/asset.** In `services/v1.js`, `query()` -posts to `query.v1`. Post the same bodies to `api/asset` instead and read -`queryResult.results` (one array per query, positional, an empty array -for a query that matched nothing) so callers still receive an array of -result arrays. Add an explicit `page: { size, start }` to every body in -`lib/queries.js` (api/asset reads should never be unbounded). Note that -api/asset returns every scalar as a string (`"True"`, `"5"`) and a -single relation selected with a sub-select as a one-element list — check -each call site's comparisons against a real response. Update -`test/v1-service.test.js`. +**S2 — Page every read.** Reads stay on `query.v1`: it is read-only and +answers with rows alone, and the SDK has a `queryV1` module for it. Add an +explicit `page: { size, start }` to every body in `lib/queries.js` (the +SDK refuses an unpaged query; no read should be unbounded), choosing sizes +that match what each caller actually shows. Update +`test/v1-service.test.js` to assert the pages sent. **S3 — Close Bolt Phase 2.** AGENTS.md says axios is dropped in Phase 2. Once nothing imports them, remove `axios`, `botkit`, diff --git a/specs/000-layers.md b/specs/000-layers.md index 16aa7ed..ea14231 100644 --- a/specs/000-layers.md +++ b/specs/000-layers.md @@ -132,9 +132,11 @@ expect-zero control beside every "it returned rows". ## 6. Politeness is in the layer -No read is unbounded: `assetApi.queries` refuses a query without a page, +No read is unbounded: `assetApi.queries` and `queryV1.queries` refuse a +query without a page, and the rest-1 module will refuse a Data read without one. The transport -retries only idempotent requests (GET by default), only on a network +retries only idempotent requests (GET by default, and every query.v1 +POST, since that endpoint cannot write), only on a network failure or a 5xx, never a 4xx; a batch write is never retried unless the caller has made it idempotent with a guard and says so. diff --git a/src/client.ts b/src/client.ts index 619a9fb..3dcc65e 100644 --- a/src/client.ts +++ b/src/client.ts @@ -10,6 +10,7 @@ import type { Metamodel } from "./types/metamodel"; import * as assetApi from "./wire/assetApi"; import * as loc2 from "./wire/loc2"; import * as meta from "./wire/meta"; +import * as queryV1 from "./wire/queryV1"; import * as rest1 from "./wire/rest1"; export interface ClientOptions { @@ -113,6 +114,22 @@ export class AgilityApiClient { history: (target: string, params: rest1.ReadParams) => rest1.history(this.transport, target, params) }; + /** + * query.v1: reads only, the same query bodies as assets.query, rows + * translated the same way. The endpoint cannot write, so it is the safe + * place to explore a query; its requests may be retried like a GET. + */ + readonly queryV1 = { + query: async (q: assetApi.QueryBody) => { + await this.schema.ensureFor([q]); + return queryV1.query(this.transport, q, { lookup: this.schema }); + }, + queries: async (qs: assetApi.QueryBody[]) => { + await this.schema.ensureFor(qs); + return queryV1.queries(this.transport, qs, { lookup: this.schema }); + } + }; + /** api/asset: batches and queries, rows translated through the schema. */ readonly assets = { send: async (commands: assetApi.Command[], options?: Omit) => { diff --git a/src/index.ts b/src/index.ts index 22a6e3d..0bf0ebf 100644 --- a/src/index.ts +++ b/src/index.ts @@ -15,7 +15,7 @@ export type { AssetType, AttributeDefinition, AttributeLookup, AttributeType, At export type * from "./types/assets/index"; // The wire layer: one module per endpoint. -export { assetApi, rest1, meta, loc2 } from "./wire"; +export { assetApi, rest1, meta, loc2, queryV1 } from "./wire"; export * as select from "./wire/select"; export type { AssetApiResponse, Command, CommandBody, CommandFailureDetail, Outcome, CommandValue, CreateBody, ExecuteBody, QueryBody, UpdateBody, ReadOptions, RowContext, SendOptions } from "./wire/assetApi"; export { CommandFailureError } from "./wire/assetApi"; diff --git a/src/wire/index.ts b/src/wire/index.ts index 37a8e1a..5aede19 100644 --- a/src/wire/index.ts +++ b/src/wire/index.ts @@ -7,6 +7,7 @@ export * as assetApi from "./assetApi"; export * as rest1 from "./rest1"; export * as meta from "./meta"; export * as loc2 from "./loc2"; +export * as queryV1 from "./queryV1"; export * as select from "./select"; export type { AssetDocument, AssetsDocument, ReadParams, ReadResult } from "./rest1"; export type { AssetApiResponse, Command, CommandBody, CommandValue, CreateBody, ExecuteBody, QueryBody, UpdateBody, CommandFailure, CommandError, OidTokens, SendOptions } from "./assetApi"; diff --git a/src/wire/queryV1.ts b/src/wire/queryV1.ts new file mode 100644 index 0000000..e1ebfb6 --- /dev/null +++ b/src/wire/queryV1.ts @@ -0,0 +1,66 @@ +/** + * The query.v1 wire module: reads only. The endpoint takes the same query + * bodies as api/asset's query commands and answers with nothing but the + * rows — an array with one array of rows per query, in request order — so + * it is the simplest place to explore what a query does. api/asset can do + * everything query.v1 does (and write); query.v1 can never write. + * + * Verified against the API-testing instance (2026-11-06): + * - POST a query body, or an array of them (or YAML documents separated by + * ---); the answer is [[row, …], …], [] for a query that matched nothing. + * - rows are spelled exactly as api/asset spells them: every scalar a + * string ("2", "True"), a relation { _oid }, a multi-value relation a + * list, a sub-selected single relation a list. So they translate through + * the same schema the same way. + * - one bad query fails the whole request: 400 with rest-1's error shape, + * { error: "Bad Request", exceptions: [{ type, message }, …] }, and no + * rows at all. The useful text is in exceptions, not error. + * - a filter literal with an em dash fails here too; a `with` variable + * carrying the same value works. + * + * Because the endpoint cannot change anything, a request is sent idempotent: + * the transport may retry it after a network failure or a 5xx like a GET. + */ +import { ApiError } from "../core/errors"; +import type { Transport } from "../core/transport"; +import type { Asset } from "../types/asset"; +import { fromWireRow, type QueryBody, type ReadOptions, type RowContext } from "./assetApi"; +import { fromTypeOf } from "./select"; + +/** The error query.v1 sends for a query it refused: the exceptions' messages, joined. */ +function rejected(error: ApiError): ApiError { + const details = error.details as { exceptions?: { message?: unknown }[] } | undefined; + const messages = (details?.exceptions ?? []).map((e) => e?.message).filter((m): m is string => typeof m === "string" && !!m); + return messages.length ? new ApiError(messages.join("; "), error.status, "QUERY_REJECTED", error.details) : error; +} + +/** + * Several queries in one request; one array of rows per query, in order, [] + * where a query matched nothing. A page is required on every query. A + * query the server refuses throws (an ApiError coded QUERY_REJECTED, with + * the server's messages) — never an empty result. + */ +export async function queries(transport: Transport, qs: QueryBody[], options: ReadOptions = {}): Promise { + for (const q of qs) if (!q.page) throw new Error(`query of ${q.from} without a page: every query.v1 read must be bounded`); + if (!qs.length) return []; + let response: unknown; + try { + response = await transport.request({ method: "POST", path: "/query.v1", body: qs, responseType: "json", idempotent: true }); + } catch (error) { + throw error instanceof ApiError && error.status === 400 ? rejected(error) : error; + } + if (!Array.isArray(response) || response.length !== qs.length || !response.every(Array.isArray)) { + throw new ApiError(`query.v1 answered with ${Array.isArray(response) ? `${response.length} results` : "no result list"} for ${qs.length} queries`, 200, "BAD_RESPONSE", response); + } + return qs.map((q, i) => { + const type = options.lookup ? fromTypeOf(q.from, qs) : null; + const context: RowContext | undefined = type ? { lookup: options.lookup!, type } : undefined; + return (response as Record[][])[i]!.map((row) => fromWireRow(row, context)); + }); +} + +/** One query, its rows. */ +export async function query(transport: Transport, q: QueryBody, options: ReadOptions = {}): Promise { + const [rows] = await queries(transport, [q], options); + return rows ?? []; +} diff --git a/test/integration/queryV1.test.ts b/test/integration/queryV1.test.ts new file mode 100644 index 0000000..3aaf6fa --- /dev/null +++ b/test/integration/queryV1.test.ts @@ -0,0 +1,37 @@ +import { expect, it } from "vitest"; +import { AgilityApiClient } from "../../src/client"; +import { describeInstance, instance } from "./instance"; + +/** + * query.v1 against a real instance. Read-only, small pages. What the fakes + * cannot prove: that query.v1 and api/asset really spell rows the same way + * (so one translation serves both), that a miss is [] and not an error, and + * that a refused query fails the whole request with the server's message. + */ +describeInstance("queryV1 (live)", () => { + const client = new AgilityApiClient({ baseUrl: instance!.url }, { transport: instance!.transport }); + const q = { from: "Story", select: ["Name", "Number", "Estimate", "IsClosed", "Scope", "Owners", { from: "Status", select: ["Name"] }], sort: ["ID"], page: { size: 3, start: 0 } }; + + it("the same query through query.v1 and api/asset translates to the same rows", async () => { + const [viaQueryV1, viaAssets] = await Promise.all([client.queryV1.query(q), client.assets.query(q)]); + expect(viaQueryV1.length).toBeGreaterThan(0); + expect(viaQueryV1).toEqual(viaAssets); + }); + + it("a query that matches nothing is [] in its own slot — the expect-zero control — and a `with` variable carries an em dash", async () => { + const [none, scopes] = await client.queryV1.queries([ + { from: "Story", select: ["Name"], filter: ["Name=$n"], with: { $n: "sdk-integration — no such story" }, page: { size: 1, start: 0 } }, + { from: "Scope", select: ["Name"], page: { size: 1, start: 0 } } + ]); + expect(none).toEqual([]); + expect(scopes).toHaveLength(1); + }); + + it("an unknown attribute fails the request with the server's message", async () => { + await expect(client.queryV1.queries([q, { from: "Story", select: ["NoSuchAttribute"], page: { size: 1, start: 0 } }])).rejects.toMatchObject({ + status: 400, + code: "QUERY_REJECTED", + message: expect.stringContaining("Story.NoSuchAttribute") + }); + }); +}); diff --git a/test/unit/queryV1.test.ts b/test/unit/queryV1.test.ts new file mode 100644 index 0000000..48a6c61 --- /dev/null +++ b/test/unit/queryV1.test.ts @@ -0,0 +1,71 @@ +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import { describe, expect, it, vi } from "vitest"; +import { AgilityApiClient } from "../../src/client"; +import { ApiError } from "../../src/core/errors"; +import type { RequestOptions, Transport } from "../../src/core/transport"; +import { Schema } from "../../src/services/schema"; +import { toMetamodel, type MetamodelDocument } from "../../src/wire/meta"; +import { queries, query } from "../../src/wire/queryV1"; + +const schema = Schema.fromMetamodel(toMetamodel(JSON.parse(readFileSync(join(__dirname, "..", "..", "metamodel.json"), "utf8")) as MetamodelDocument)); + +function fakeTransport(...answers: unknown[]): Transport & { calls: RequestOptions[] } { + const calls: RequestOptions[] = []; + const request = vi.fn(async (options: RequestOptions) => { + calls.push(options); + const next = answers.shift(); + if (next instanceof Error) throw next; + return next; + }); + return { request: request as Transport["request"], calls }; +} + +// A row as query.v1 sent it from the API-testing instance (2026-11-06). +const wireStory = { _oid: "Story:1055", Name: "Sample: Logon", Number: "S-01001", Estimate: "2", IsClosed: "True", Scope: { _oid: "Scope:1121" }, Owners: [{ _oid: "Member:1008" }, { _oid: "Member:11576" }], Status: [] }; +const storyQuery = { from: "Story", select: ["Name", "Number", "Estimate", "IsClosed", "Scope", "Owners", { from: "Status", select: ["Name"] }], page: { size: 2, start: 0 } }; + +describe("queryV1", () => { + it("POSTs the queries to query.v1 as a list, marked idempotent (the endpoint cannot write), and returns one array per query", async () => { + const t = fakeTransport([[wireStory], []]); + const [stories, scopes] = await queries(t, [storyQuery, { from: "Scope", select: ["Name"], filter: ["Name=$n"], with: { $n: "a—b" }, page: { size: 1, start: 0 } }]); + expect(t.calls[0]).toMatchObject({ method: "POST", path: "/query.v1", idempotent: true, responseType: "json" }); + expect(Array.isArray(t.calls[0]!.body)).toBe(true); + expect(stories).toHaveLength(1); + expect(scopes).toEqual([]); + }); + + it("without a schema, rows keep the endpoint's spelling; with one, they arrive in the model's shape", async () => { + const [raw] = await query(fakeTransport([[wireStory]]), storyQuery); + expect(raw).toMatchObject({ Estimate: "2", IsClosed: "True", Status: [] }); + const [row] = await query(fakeTransport([[wireStory]]), storyQuery, { lookup: schema }); + expect(row).toMatchObject({ _oid: "Story:1055", Estimate: 2, IsClosed: true, Scope: { _oid: "Scope:1121" }, Owners: [{ _oid: "Member:1008" }, { _oid: "Member:11576" }], Status: null }); + }); + + it("refuses an unpaged query before sending anything", async () => { + const t = fakeTransport(); + await expect(queries(t, [{ from: "Story", select: ["Name"] }])).rejects.toThrow(/without a page/); + expect(t.calls).toHaveLength(0); + }); + + it("a refused query throws with the server's messages, not 'Bad Request' — never an empty result", async () => { + // As sent for select: ["Nope"] (2026-11-06): the whole request fails, no rows for the good queries. + const t = fakeTransport(new ApiError("Bad Request", 400, undefined, { error: "Bad Request", exceptions: [{ type: "MetaException", message: "Unknown AttributeDefinition: Story.Nope" }] })); + const err = (await queries(t, [storyQuery, { from: "Story", select: ["Nope"], page: { size: 1, start: 0 } }]).catch((e: unknown) => e)) as ApiError; + expect(err).toBeInstanceOf(ApiError); + expect(err).toMatchObject({ status: 400, code: "QUERY_REJECTED", message: "Unknown AttributeDefinition: Story.Nope" }); + }); + + it("a response that does not hold one array per query is an error, not a guess", async () => { + await expect(queries(fakeTransport([[wireStory]]), [storyQuery, storyQuery])).rejects.toMatchObject({ code: "BAD_RESPONSE" }); + await expect(queries(fakeTransport({ oops: true }), [storyQuery])).rejects.toMatchObject({ code: "BAD_RESPONSE" }); + }); + + it("client.queryV1 loads the types its queries touch and translates through the client's schema", async () => { + const t = fakeTransport([[wireStory]]); + const client = new AgilityApiClient({ baseUrl: "https://x" }, { transport: t, schema }); + const [row] = await client.queryV1.query(storyQuery); + expect(row).toMatchObject({ Estimate: 2, IsClosed: true }); + expect(t.calls.map((c) => c.path)).toEqual(["/query.v1"]); + }); +}); From a7460db7561cbe1692af4dfda44111dbc665daf0 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 15:44:56 +0000 Subject: [PATCH 3/3] assetApi.send: a failure without a command index is reported, not a count error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a failure carried no sourceCommandIndex and the query results did not line up, send threw "cannot tell which result belongs to which query" and the real failure was lost. Now every query slot is null (no row is placed when any query may be the one that failed) and the failures are the answer: a CommandFailureError with the server's messages for reads and with throwOnFailure, or in the envelope otherwise. A count mismatch with no unattributed failure is still its own error. Spec 000 §6 updated. Found adopting the SDK in gh-v1 (C1). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016NSWj9gL1oCubj5e2anVkb --- specs/000-layers.md | 5 ++++- src/wire/assetApi.ts | 26 +++++++++++++++++++------- test/unit/assetApi.test.ts | 26 ++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 8 deletions(-) diff --git a/specs/000-layers.md b/specs/000-layers.md index ea14231..9df040c 100644 --- a/specs/000-layers.md +++ b/specs/000-layers.md @@ -158,7 +158,10 @@ always throw: a query the server rejected must not come back as `[]`. The server drops a failed query's slot from `queryResult.results` (later results shift up); `send` restores it as `null` by `sourceCommandIndex`, so positions always match the queries sent, and refuses to guess when the -counts disagree. +counts disagree. If a failure carries no sourceCommandIndex it may be any +of the queries, so every query slot is null and the failures are the +answer (thrown for reads and with throwOnFailure); a disagreement with no +such failure is an error of its own. The guard (`services/guard`) is what makes a write batch safe to resend: each per-asset command of the server's projection is sent scoped to its diff --git a/src/wire/assetApi.ts b/src/wire/assetApi.ts index 6ef46bb..814f1ee 100644 --- a/src/wire/assetApi.ts +++ b/src/wire/assetApi.ts @@ -283,12 +283,13 @@ function queriesIn(commands: Command[]): { body: QueryBody; index: number }[] { * The server's results, one slot per query again. It sends a slot for every * query that ran (an empty match included) and none for a query that * failed; a failure names its command by sourceCommandIndex, so the failed - * queries are known and the rest fill in order. If the counts disagree the - * positions cannot be trusted, and that is an error rather than a guess. + * queries are known and the rest fill in order. Null when the counts + * disagree: the positions cannot be trusted, and the caller decides what + * that means rather than this guessing. */ -function realign(sent: { index: number }[], results: T[], failed: Set): (T | null)[] { +function realign(sent: { index: number }[], results: T[], failed: Set): (T | null)[] | null { const expected = sent.filter((q) => !failed.has(q.index)).length; - if (results.length !== expected) throw new Error(`api/asset returned ${results.length} query results for ${expected} queries that did not fail; cannot tell which result belongs to which query`); + if (results.length !== expected) return null; let next = 0; return sent.map((q) => (failed.has(q.index) ? null : results[next++]!)); } @@ -311,13 +312,24 @@ export async function send(transport: Transport, comm // A preview answers with the projection, not an envelope. if (options.previewOnly) return response; const queries = queriesIn(commands); - const failed = new Set(failuresOf(response).flatMap((f) => (f.index === null ? [] : [f.index]))); - const aligned = realign(queries, response.queryResult?.results ?? [], failed); + const failures = failuresOf(response); + const failed = new Set(failures.flatMap((f) => (f.index === null ? [] : [f.index]))); + const results = response.queryResult?.results ?? []; + let aligned = realign(queries, results, failed); + if (!aligned) { + // A failure without a sourceCommandIndex may be one of the queries, so + // no row can be placed with confidence: every slot is null, never a + // misplaced row, and the failures themselves are the answer (thrown + // with throwOnFailure, as every read asks). With no such failure, the + // server's answer is inconsistent, and that is an error of its own. + if (!failures.some((f) => f.index === null)) throw new Error(`api/asset returned ${results.length} query results for ${queries.filter((q) => !failed.has(q.index)).length} queries that did not fail; cannot tell which result belongs to which query`); + aligned = queries.map(() => null); + } const contextOf = (i: number): RowContext | undefined => { const type = options.lookup ? fromTypeOf(queries[i]!.body.from, commands) : null; return type ? { lookup: options.lookup!, type } : undefined; }; - const translated: AssetApiResponse = { ...response, queryResult: { ...response.queryResult, results: aligned.map((rows, i) => rows && rows.map((r) => fromWireRow(r as Record, contextOf(i)))) } }; + const translated: AssetApiResponse = { ...response, queryResult: { ...response.queryResult, results: aligned!.map((rows, i) => rows && rows.map((r) => fromWireRow(r as Record, contextOf(i)))) } }; if (options.throwOnFailure && translated.commandFailures?.count) throw new CommandFailureError(translated); return translated; } diff --git a/test/unit/assetApi.test.ts b/test/unit/assetApi.test.ts index 4d195e5..bb03ef1 100644 --- a/test/unit/assetApi.test.ts +++ b/test/unit/assetApi.test.ts @@ -186,4 +186,30 @@ describe("assetApi: command failures", () => { const t = fakeTransport(envelope({ queryResult: { results: [[], []], resultMetaData: [], count: -1 } })); await expect(send(t, [{ from: "Story", select: ["Name"], page: { size: 1, start: 0 } }])).rejects.toThrow(/cannot tell which result belongs to which query/); }); + + it("a failure without a command index makes every query slot null and reports the failure itself — not a 'cannot tell' error", async () => { + // A create fails, and a follow-up query on its alias fails without saying which command it was. + const unattributed = () => envelope({ + commandFailures: { count: 2, commands: [ + { "@new": { AssetType: "Story" }, error: { message: "Resolution'Ambiguous'Story.Scope'Scope", sourceCommandIndex: 0 } }, + { error: { message: "Unknown AssetType: @new" } } + ] }, + queryResult: { results: [[{ _oid: "Member:20" }]], resultMetaData: [], count: -1 } + }); + const batch: Command[] = [ + { "@new": { AssetType: "Story", Name: "x" } }, + { from: "@new", select: ["Number"], page: { size: 1, start: 0 } }, + { from: "Member", select: ["Name"], page: { size: 1, start: 0 } } + ]; + const res = await send(fakeTransport(unattributed()), batch); + expect(res.queryResult.results).toEqual([null, null]); // neither row placed: either query may be the one that failed + expect(failuresOf(res).map((f) => f.index)).toEqual([0, null]); + const err = (await send(fakeTransport(unattributed()), batch, { throwOnFailure: true }).catch((e: unknown) => e)) as CommandFailureError; + expect(err).toBeInstanceOf(CommandFailureError); + expect(err.message).toBe("Resolution'Ambiguous'Story.Scope'Scope; Unknown AssetType: @new"); + // A read throws the failure too, root cause first. + await expect(queries(fakeTransport(envelope({ commandFailures: { count: 1, commands: [{ error: { message: "Unknown AttributeDefinition: Story.Nope" } }] }, queryResult: { results: [], resultMetaData: [], count: -1 } })), [ + { from: "Story", select: ["Nope"], page: { size: 1, start: 0 } } + ])).rejects.toMatchObject({ name: "CommandFailureError", message: "Unknown AttributeDefinition: Story.Nope" }); + }); });