Skip to content

Commit 79b5b48

Browse files
authored
Merge pull request #42 from versionone/claude/admiring-ride-vsq4sa
queryV1 read module; plans for the vendored SDK and query.v1 reads
2 parents 32454a7 + a7460db commit 79b5b48

14 files changed

Lines changed: 300 additions & 53 deletions

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ npm test
2222
## Public API
2323

2424
- `AgilityApiClient` - top-level SDK client (`meta`, `data` for rest-1, `assets` for api/asset, `localize` for loc-2, `schema`), and the services on it:
25+
- `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
2526
- `resolve.numbers` / `resolve.ref` - asset numbers and OID tokens to assets, in one request
2627
- `assets.sendGuarded` - a batch as a compare-and-set on each asset's ChangeDate; changed assets come back `skipped`
2728
- `batch.run` / `batch.reversible` - preview, snapshot, (confirm,) send guarded, and a receipt with a reverse batch

‎plans/001-sdk-adoption-campaign.md‎

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,11 @@ Found by reading the code above against `src/`:
134134
return failures typed (it does) and offer `throwOnFailure` producing an
135135
`ApiError` subclass carrying `sourceCommandIndex` per failure.
136136
5. **Missing wire modules** named in spec 000 but not built: `loc2` (both
137-
consumers), `queryV1` (slack's main read path — or migrate slack to
138-
api/asset queries instead; see B3), `attachment`/`EmbeddedImage` (rooms).
137+
consumers), `queryV1` (slack's main read path), `attachment`/
138+
`EmbeddedImage` (rooms). `queryV1` is built (2026-11-06) and kept on
139+
purpose even though api/asset can run the same queries: query.v1 is
140+
read-only by construction and answers with rows alone, so it is the safe,
141+
simple place to explore a query (and its requests may be retried).
139142
6. **rest-1 writes — not needed.** Settled from the docs
140143
(apis/asset-bulk/updating-assets, creating-assets): api/asset carries a
141144
change comment as `ChangeComment` inside an update's `update` node, or
@@ -166,11 +169,11 @@ moves.
166169
- **A2 Command-failure ergonomics** — gap 4: `CommandFailureError`,
167170
`send(..., { throwOnFailure })`, `sourceCommandIndex` preserved.
168171
- **A3 loc2 wire module** — gap 5, smallest consumer-shared endpoint.
169-
- **A4 Slack-shaped reads** — decide queryV1-module vs. api/asset-only by
170-
porting `lib/queries.js` bodies into integration tests against the dev
171-
instance; whichever reads translate cleanly wins. (No `rest1` writes:
172-
gap 6 is settled.) If the bot's S2 lands first, this is only the
173-
translation check.
172+
- **A4 Slack-shaped reads** — done: a `queryV1` wire module and
173+
`client.queryV1.query`/`queries`, rows translated through the same
174+
schema as api/asset (the integration tier proves both endpoints give the
175+
same rows). The bot keeps query.v1 for its reads. (No `rest1` writes:
176+
gap 6 is settled.)
174177
- **A5 Services port** — gap 7. `services/resolve` first (gh-v1's
175178
`resolveAssets`; slack's `queryForAssetByNumber` needs it too), then
176179
`services/guard` (`guarded()`), then `services/batch` (preview, describe,
@@ -200,12 +203,12 @@ bodies in `lib/queries.js`), api/asset (`bulk`, `createWithNumber`,
200203

201204
- **B0 Pre-SDK migrations in the bot's own session** (S1–S4 under
202205
*Consumer migrations* below). They shrink what the SDK must support:
203-
once S2 moves reads to api/asset, the SDK needs no `queryV1` module and
204-
A4 collapses to "verify the bot's query bodies translate" — and A4's
205-
rest-1-writes question is closed (gap 6).
206+
once S2 pages every read, B1's reads move to `client.queryV1` as they
207+
are; A4's rest-1-writes question is closed (gap 6).
206208
- **B1 Client swap.** `createV1Client` keeps its surface; inside, one
207-
`AgilityApiClient({ baseUrl: url, accessToken })`. `query`/`bulk`/`exec`
208-
→ `client.assets.*`; `fetchLocalizations` → `loc2` (A3).
209+
`AgilityApiClient({ baseUrl: url, accessToken })`. `query` →
210+
`client.queryV1.queries`; `bulk`/`exec` → `client.assets.*`;
211+
`fetchLocalizations` → `loc2` (A3).
209212
`test/v1-service.test.js` stays the parity proof.
210213
- **B2 Delete the decodes.** Rows arrive translated: `"True"` comparisons,
211214
`IsKanban.toLowerCase()`, `toMomentless` and positional
@@ -225,8 +228,9 @@ instance: same requests out, same rows in.
225228

226229
- **C1 Seam swap.** Re-implement `lib/api.js`'s exported surface (`query`,
227230
`queries`, `asset`, `attempt`, `rest`, `meta`, `localize`, `raw`, `blob`,
228-
`permalink`) on `AgilityApiClient`, `require`d from the CJS build. Callers
229-
unchanged; all 99 tests green. Needs A2, A3 (loc2), gap 9.
231+
`permalink`) on the SDK, imported from a vendored copy of its ESM build
232+
(gh installs extensions without npm; see plan 002, *Dependency*). Callers
233+
unchanged; all tests green. Needs A2, A3 (loc2).
230234
- **C2 Delete the decodes.** With rows translated by the schema, `isTrue`
231235
and `oidOf` drop out of `meta.js`, `resolve.js`, `board.js`; `momentless`
232236
comes from the SDK.
@@ -300,10 +304,9 @@ ESM" here means finishing the edges:
300304
- **S1 Finish ESM tooling.** `jsconfig.json` still says `module: commonJS`,
301305
`target: es6`; set `NodeNext`/`ES2022` and turn on `checkJs` so editors
302306
and `tsc --noEmit` see the real module system (and, later, SDK types).
303-
- **S2 query.v1 → api/asset reads.** Same query bodies; the endpoint is
304-
`api/asset` and results come from `queryResult.results` (positional, one
305-
array per query). Add an explicit `page` to every body. Removes the
306-
SDK's need for a `queryV1` module.
307+
- **S2 Page every read.** Reads stay on query.v1 (read-only, simplest
308+
responses). Add an explicit `page` to every body in `lib/queries.js`,
309+
as the SDK's `queryV1` refuses an unpaged query.
307310
- **S3 Close Bolt Phase 2.** Drop `botkit`, `botbuilder-*`,
308311
`botkit-plugin-cms` and `axios` from `package.json` once nothing imports
309312
them; delete `lib/v1api.js` and the `plugins/` Botkit modules that
@@ -354,12 +357,12 @@ ESM" here means finishing the edges:
354357
- [x] A1 transport: credentials, onUnauthorized (idempotent requests resent once; writes never), signal (`RequestOptions.signal`, `client.withSignal`)
355358
- [x] A2 command-failure error (`CommandFailureError`, `throwOnFailure`, reads always throw)
356359
- [x] A3 loc2 (`loc2.localize`, `client.localize`; GET, POST past 2000 encoded chars, culture via Accept-Language)
357-
- [ ] A4 slack-shaped reads (queryV1 or api/asset)
360+
- [x] A4 slack-shaped reads — `queryV1` wire module, `client.queryV1.query`/`queries` (read-only, idempotent, same row translation as api/asset)
358361
- [x] A5 services port from gh-v1 — `resolve` (`client.resolve.numbers`/`.ref`), `guard` (`client.assets.sendGuarded`), `batch` (`client.batch.run`/`.reversible`)
359362
- [ ] 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
360363
- [ ] G0–G3 gh-v1 migrations (gh-v1 session)
361364
- [ ] S1–S4 slack-bot migrations (slack-bot session, `bolt-migration`)
362365
- [ ] B1–B4 slack bot SDK swap — brief: `plans/003-adopt-sdk-k6i-agility-slack-bot.md`
363366
- [x] C0 gh-v1 surveyed (2026-11-04)
364-
- [ ] C1–C4 gh-v1 — brief: `plans/002-adopt-sdk-gh-v1.md`
367+
- [ ] C1–C4 gh-v1 — brief: `plans/002-adopt-sdk-gh-v1.md`; C1 in versionone/gh-v1#33 (SDK vendored)
365368
- [ ] D1–D4 agility-rooms

‎plans/002-adopt-sdk-gh-v1.md‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,16 +25,27 @@ output stay the same; the code that duplicates the SDK goes.
2525
in `lib/api.js`, `meta.js`, `resolve.js`, `board.js`, `batch.js`, and
2626
`commands/apply.js`, `bulk.js`, `board.js`.
2727

28-
## Dependency
28+
## Dependency: vendored
2929

30-
No registry package yet. Add the first runtime dependency as a pinned commit:
31-
`"@versionone/sdk": "github:versionone/VersionOne.SDK.JavaScript#<merge sha>"`.
32-
The SDK's `prepare` script builds `dist` on install. gh-v1 is ESM now, so use
33-
`import` (the ESM build), not `require`.
30+
gh installs a script extension with a plain `git clone` and never runs npm,
31+
so gh-v1 cannot rely on `node_modules` at run time. The SDK is still a
32+
runtime dependency, but its code ships inside the repo:
33+
34+
- `vendor/versionone-sdk/` holds the SDK's ESM build (`index.js`, which
35+
imports nothing) and types (`index.d.ts`), plus `SOURCE.json` naming
36+
the commit they came from. `lib/api.js` imports from there.
37+
- `package.json` lists `@versionone/sdk` under `devDependencies`, pinned
38+
to a full commit, so the lockfile records exactly what was vendored.
39+
- To move the pin: change it in `package.json`, then
40+
`npm install && npm run vendor:sdk`, and commit the copy.
41+
`test/vendor.test.js` fails if the copy, `SOURCE.json` and the pin
42+
disagree.
43+
- Publishing the SDK to a registry (plan 001, *Decisions needed*) changes
44+
only where the pin points; the copy stays.
3445

3546
## Steps, one PR each, on branch `sdk-adoption`
3647

37-
1. **C1 Seam swap** (`lib/api.js`). Keep `api(cfg)` and the `Api` typedef.
48+
1. **C1 Seam swap** (`lib/api.js`) — done in versionone/gh-v1#33. Keep `api(cfg)` and the `Api` typedef.
3849
Inside, build one `AgilityApiClient({ baseUrl: cfg.v1Url, accessToken })`.
3950
Map: `query`/`queries` → `client.assets.query`/`queries`;
4051
`asset` → `client.assets.send(cmds, { throwOnFailure: true })`

‎plans/003-adopt-sdk-k6i-agility-slack-bot.md‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ hand-decoding and rest-1 write path go.
2121
`fetchLocalizations` (loc-2), and `queryFor*` helpers built on
2222
`lib/queries.js`. `test/v1-service.test.js` stubs global `fetch`.
2323
- **S1–S4 are not done yet**: `jsconfig.json` still says `commonJS`/`es6`;
24-
reads still post to `query.v1`; `axios`, `botkit*`, `botbuilder-*` are
24+
query bodies have no explicit page; `axios`, `botkit*`, `botbuilder-*` are
2525
still dependencies and `lib/v1api.js` still exists. These come first.
2626

2727
## Dependency
@@ -34,12 +34,12 @@ The SDK's `prepare` script builds `dist` on install. The bot is ESM, so
3434
## Steps, one PR each
3535

3636
0. **S1–S4 first**, as written in `plans/handoffs/k6i-agility-slack-bot.md`.
37-
B1 needs at least S2 (reads on api/asset) and S3 (axios and Botkit gone).
37+
B1 needs at least S2 (every read paged) and S3 (axios and Botkit gone).
3838
If the bot's own session already merged them, skip.
3939
1. **B1 Client swap** (`services/v1.js`). Inside `createV1Client`, build one
4040
`new AgilityApiClient({ baseUrl: url, accessToken })`. `query` →
41-
`client.assets.queries` (positional; a failed query now throws instead
42-
of shifting later results); `bulk`/`exec`/`createWithNumber` →
41+
`client.queryV1.queries` (the bot's reads stay on query.v1: read-only,
42+
simplest responses; a refused query throws with the server's message); `bulk`/`exec`/`createWithNumber` →
4343
`client.assets.send`; `fetchLocalizations` → `client.localize`;
4444
`findAssets` → `client.data.list("BaseAsset", { find, findin, where,
4545
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
7272
## Done when
7373

7474
- `npm test` passes on each PR; S1's `npm run typecheck` passes.
75-
- `services/v1.js` makes no `fetch` call of its own; nothing posts to
76-
`query.v1` or writes through rest-1.
75+
- `services/v1.js` makes no `fetch` call of its own; reads go through
76+
`client.queryV1` (or `client.data` for `find`), and nothing writes through
77+
rest-1.
7778
- No string-boolean comparisons or local `toMomentless` remain.
7879
- Sandbox-workspace smoke passes: asset mention, `/agility find`,
7980
backlog-item status change, webhook notification.

‎plans/handoffs/k6i-agility-slack-bot.md‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,12 @@ no `require`). `jsconfig.json` still declares `module: commonJS`,
2121
`npm run typecheck` (`tsc -p jsconfig.json`; `typescript` is already a
2222
devDependency). Start non-strict; fix or annotate what it finds.
2323

24-
**S2 — Reads from query.v1 to api/asset.** In `services/v1.js`, `query()`
25-
posts to `query.v1`. Post the same bodies to `api/asset` instead and read
26-
`queryResult.results` (one array per query, positional, an empty array
27-
for a query that matched nothing) so callers still receive an array of
28-
result arrays. Add an explicit `page: { size, start }` to every body in
29-
`lib/queries.js` (api/asset reads should never be unbounded). Note that
30-
api/asset returns every scalar as a string (`"True"`, `"5"`) and a
31-
single relation selected with a sub-select as a one-element list — check
32-
each call site's comparisons against a real response. Update
33-
`test/v1-service.test.js`.
24+
**S2 — Page every read.** Reads stay on `query.v1`: it is read-only and
25+
answers with rows alone, and the SDK has a `queryV1` module for it. Add an
26+
explicit `page: { size, start }` to every body in `lib/queries.js` (the
27+
SDK refuses an unpaged query; no read should be unbounded), choosing sizes
28+
that match what each caller actually shows. Update
29+
`test/v1-service.test.js` to assert the pages sent.
3430

3531
**S3 — Close Bolt Phase 2.** AGENTS.md says axios is dropped in Phase 2.
3632
Once nothing imports them, remove `axios`, `botkit`,

‎specs/000-layers.md‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -132,9 +132,11 @@ expect-zero control beside every "it returned rows".
132132

133133
## 6. Politeness is in the layer
134134

135-
No read is unbounded: `assetApi.queries` refuses a query without a page,
135+
No read is unbounded: `assetApi.queries` and `queryV1.queries` refuse a
136+
query without a page,
136137
and the rest-1 module will refuse a Data read without one. The transport
137-
retries only idempotent requests (GET by default), only on a network
138+
retries only idempotent requests (GET by default, and every query.v1
139+
POST, since that endpoint cannot write), only on a network
138140
failure or a 5xx, never a 4xx; a batch write is never retried unless the
139141
caller has made it idempotent with a guard and says so.
140142

@@ -156,7 +158,10 @@ always throw: a query the server rejected must not come back as `[]`.
156158
The server drops a failed query's slot from `queryResult.results` (later
157159
results shift up); `send` restores it as `null` by `sourceCommandIndex`, so
158160
positions always match the queries sent, and refuses to guess when the
159-
counts disagree.
161+
counts disagree. If a failure carries no sourceCommandIndex it may be any
162+
of the queries, so every query slot is null and the failures are the
163+
answer (thrown for reads and with throwOnFailure); a disagreement with no
164+
such failure is an error of its own.
160165

161166
The guard (`services/guard`) is what makes a write batch safe to resend:
162167
each per-asset command of the server's projection is sent scoped to its

‎src/client.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import type { Metamodel } from "./types/metamodel";
1010
import * as assetApi from "./wire/assetApi";
1111
import * as loc2 from "./wire/loc2";
1212
import * as meta from "./wire/meta";
13+
import * as queryV1 from "./wire/queryV1";
1314
import * as rest1 from "./wire/rest1";
1415

1516
export interface ClientOptions {
@@ -113,6 +114,22 @@ export class AgilityApiClient {
113114
history: <Row extends Asset = Asset>(target: string, params: rest1.ReadParams) => rest1.history<Row>(this.transport, target, params)
114115
};
115116

117+
/**
118+
* query.v1: reads only, the same query bodies as assets.query, rows
119+
* translated the same way. The endpoint cannot write, so it is the safe
120+
* place to explore a query; its requests may be retried like a GET.
121+
*/
122+
readonly queryV1 = {
123+
query: async <Row extends Asset = Asset>(q: assetApi.QueryBody) => {
124+
await this.schema.ensureFor([q]);
125+
return queryV1.query<Row>(this.transport, q, { lookup: this.schema });
126+
},
127+
queries: async <Row extends Asset = Asset>(qs: assetApi.QueryBody[]) => {
128+
await this.schema.ensureFor(qs);
129+
return queryV1.queries<Row>(this.transport, qs, { lookup: this.schema });
130+
}
131+
};
132+
116133
/** api/asset: batches and queries, rows translated through the schema. */
117134
readonly assets = {
118135
send: async <Row extends Asset = Asset>(commands: assetApi.Command[], options?: Omit<assetApi.SendOptions, "lookup">) => {

‎src/index.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ export type { AssetType, AttributeDefinition, AttributeLookup, AttributeType, At
1515
export type * from "./types/assets/index";
1616

1717
// The wire layer: one module per endpoint.
18-
export { assetApi, rest1, meta, loc2 } from "./wire";
18+
export { assetApi, rest1, meta, loc2, queryV1 } from "./wire";
1919
export * as select from "./wire/select";
2020
export type { AssetApiResponse, Command, CommandBody, CommandFailureDetail, Outcome, CommandValue, CreateBody, ExecuteBody, QueryBody, UpdateBody, ReadOptions, RowContext, SendOptions } from "./wire/assetApi";
2121
export { CommandFailureError } from "./wire/assetApi";

‎src/wire/assetApi.ts‎

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -283,12 +283,13 @@ function queriesIn(commands: Command[]): { body: QueryBody; index: number }[] {
283283
* The server's results, one slot per query again. It sends a slot for every
284284
* query that ran (an empty match included) and none for a query that
285285
* failed; a failure names its command by sourceCommandIndex, so the failed
286-
* queries are known and the rest fill in order. If the counts disagree the
287-
* positions cannot be trusted, and that is an error rather than a guess.
286+
* queries are known and the rest fill in order. Null when the counts
287+
* disagree: the positions cannot be trusted, and the caller decides what
288+
* that means rather than this guessing.
288289
*/
289-
function realign<T>(sent: { index: number }[], results: T[], failed: Set<number>): (T | null)[] {
290+
function realign<T>(sent: { index: number }[], results: T[], failed: Set<number>): (T | null)[] | null {
290291
const expected = sent.filter((q) => !failed.has(q.index)).length;
291-
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`);
292+
if (results.length !== expected) return null;
292293
let next = 0;
293294
return sent.map((q) => (failed.has(q.index) ? null : results[next++]!));
294295
}
@@ -311,13 +312,24 @@ export async function send<Row extends Asset = Asset>(transport: Transport, comm
311312
// A preview answers with the projection, not an envelope.
312313
if (options.previewOnly) return response;
313314
const queries = queriesIn(commands);
314-
const failed = new Set(failuresOf(response).flatMap((f) => (f.index === null ? [] : [f.index])));
315-
const aligned = realign(queries, response.queryResult?.results ?? [], failed);
315+
const failures = failuresOf(response);
316+
const failed = new Set(failures.flatMap((f) => (f.index === null ? [] : [f.index])));
317+
const results = response.queryResult?.results ?? [];
318+
let aligned = realign(queries, results, failed);
319+
if (!aligned) {
320+
// A failure without a sourceCommandIndex may be one of the queries, so
321+
// no row can be placed with confidence: every slot is null, never a
322+
// misplaced row, and the failures themselves are the answer (thrown
323+
// with throwOnFailure, as every read asks). With no such failure, the
324+
// server's answer is inconsistent, and that is an error of its own.
325+
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`);
326+
aligned = queries.map(() => null);
327+
}
316328
const contextOf = (i: number): RowContext | undefined => {
317329
const type = options.lookup ? fromTypeOf(queries[i]!.body.from, commands) : null;
318330
return type ? { lookup: options.lookup!, type } : undefined;
319331
};
320-
const translated: AssetApiResponse<Row> = { ...response, queryResult: { ...response.queryResult, results: aligned.map((rows, i) => rows && rows.map((r) => fromWireRow<Row>(r as Record<string, unknown>, contextOf(i)))) } };
332+
const translated: AssetApiResponse<Row> = { ...response, queryResult: { ...response.queryResult, results: aligned!.map((rows, i) => rows && rows.map((r) => fromWireRow<Row>(r as Record<string, unknown>, contextOf(i)))) } };
321333
if (options.throwOnFailure && translated.commandFailures?.count) throw new CommandFailureError(translated);
322334
return translated;
323335
}

‎src/wire/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ export * as assetApi from "./assetApi";
77
export * as rest1 from "./rest1";
88
export * as meta from "./meta";
99
export * as loc2 from "./loc2";
10+
export * as queryV1 from "./queryV1";
1011
export * as select from "./select";
1112
export type { AssetDocument, AssetsDocument, ReadParams, ReadResult } from "./rest1";
1213
export type { AssetApiResponse, Command, CommandBody, CommandValue, CreateBody, ExecuteBody, QueryBody, UpdateBody, CommandFailure, CommandError, OidTokens, SendOptions } from "./assetApi";

0 commit comments

Comments
 (0)