Merge pull request #44 from versionone/claude/admiring-ride-vsq4sa

Group rows, TypeCache, dependency audit; plan ticks and decisions

Author: jallers

README.md

@@ -26,6 +26,7 @@ npm test
   - `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. `confirm` sees the touched assets' names; aliased updates/executes are sent as written with `guard: false`; a failure after the send throws `BatchRecordError` carrying the receipt
+- `TypeCache` - one instance's loaded type definitions, shared by many clients (`new AgilityApiClient(config, { types })`): a server that builds a client per user fetches each type once
 - `createFetchTransport` - configurable HTTP transport: `credentials` for same-origin cookie auth, `onUnauthorized` to re-authenticate on a 401 (resends idempotent requests once), and a per-request `signal`; `client.withSignal(signal)` ties a whole client to one
 - `ApiError` - typed API error; `CommandFailureError` - an api/asset batch in which commands failed (`failures` by index, plus the `response`)
 

package-lock.json

@@ -46,9 +46,9 @@
   "devDependencies": {
     "@types/node": "^22.13.10",
     "rimraf": "^6.0.1",
-    "tsup": "^8.4.0",
+    "tsup": "^9.0.0",
     "typescript": "^5.8.2",
-    "vitest": "^3.0.8"
+    "vitest": "^5.1.0"
   },
   "volta": {
     "node": "24.14.0"

package.json

@@ -11,6 +11,12 @@ tests green) to a package that three surfaces depend on:
 | Messaging | `digital-ai/k6i-agility-slack-bot` | Slack app (Node 24, plain ESM JS) | first |
 | Rooms | `digital-ai/agility-rooms` | React/Vite SPA, same-origin cookie auth | slow, later |
 
+The SDK's goal is one abstraction around every part of changing an asset —
+its attributes, its relations, its operations, and the files attached to
+it (attachments, embedded images) — so no consumer talks to an endpoint
+directly for any of them. gh-v1 (`gh v1 attach`) and agility-rooms both
+handle attachments today, each on its own.
+
 Specs (`specs/`) hold invariants; plans (`plans/`) hold sequencing and are
 allowed to go stale once done.
 
@@ -184,6 +190,28 @@ moves.
   publish prereleases (`0.x`, `next` tag) to GitHub Packages under the
   `@versionone` scope (or npmjs — decision below). Until then consumers pin a
   git SHA or a tarball from `npm pack`.
+- **A7 Attachments and embedded images** — gap 5's `attachment`/
+  `EmbeddedImage`, as part of the asset-update abstraction, not a side
+  channel. Two consumers to serve: gh-v1's `gh v1 attach` (list, get, add,
+  put) and agility-rooms' attachment handling (survey it first: where it
+  uploads, which `*.mvc` routes it uses, how rich text embeds images).
+  Shape, from the docs and gh-v1's working code:
+  - wire: `attachment.img/<id>` (and the embedded-image endpoint) — bytes
+    in and out, addressed by the numeric id, not the OID token; a body as
+    `Uint8Array`, `Blob` or `FormData` so it works in Node and the browser;
+    the endpoint's statuses (404 missing, 413 too large) as typed errors.
+  - service: `client.attachments.list(asset)` (metadata only — never
+    select `Content` across many: it Base64s every blob into one
+    response), `get(id)` → bytes + content type, `add(asset, bytes,
+    { filename, contentType, name, description })` as the docs prescribe
+    (create the Attachment with empty `Content`, then POST the bytes; on a
+    failed upload, report the created-but-empty asset rather than hide it),
+    `put(id, bytes)` to replace.
+  - batches: an Attachment create inside `assets.send`/`batch.run` stays an
+    ordinary create; the bytes are a second step the service owns, and a
+    receipt's reverse deletes the asset.
+  Integration tests on v1sdktesting with `sdk-integration` fixtures,
+  deleted by OID.
 
 ## Track B — k6i-agility-slack-bot (fast, on `bolt-migration`)
 
@@ -244,8 +272,9 @@ instance: same requests out, same rows in.
   `guard: false` (gh-v1 sent it as written with a note — keep that UX by
   checking `unguardable()` first), and its number roots are wider than
   gh-v1's list.
-- **C4 Passthrough.** `raw`/`blob` stay CLI-local unless an `attachment`
-  wire module (gap 5) is built for rooms — then `gh v1 attach` uses it.
+- **C4 Attachments on the SDK.** Once A7 lands, `gh v1 attach` moves onto
+  `client.attachments` and `blob` leaves `lib/api.js`. `raw` stays CLI-local:
+  it is the `gh v1 api` passthrough.
 
 `lib/guard.js` stays in gh-v1: it is policy for a terminal passthrough, not
 SDK behaviour.
@@ -330,7 +359,8 @@ ESM" here means finishing the edges:
   (proposed: `sdk-adoption`), never on their default branches, PRs only on
   request.
 - **Order here:** A2 → A3 → C1 → C2 → B1 → B2 → A5 → C3 → B3/B4 → A6 → A1 →
-  D1 → D2+. gh-v1 first: no framework migration, and stubbed-fetch tests
+  A7 → C4 → D1 → D2+. A7 starts with the rooms attachment survey, so
+  rooms' needs shape the module before gh-v1 moves onto it. gh-v1 first: no framework migration, and stubbed-fetch tests
   prove parity offline. A1 (browser credentials, onUnauthorized) waits for
   rooms; the bot maps `ApiError.status === 401` itself.
 - **In parallel, elsewhere:** G0–G3 in the gh-v1 session, S1–S4 in the
@@ -344,9 +374,18 @@ ESM" here means finishing the edges:
 
 ## Decisions needed
 
-- Publish target: GitHub Packages (`@versionone` scope, org auth required
-  for consumers in CI) vs. npmjs public. Rooms and slack both build in GitHub
-  Actions under `digital-ai`, so cross-org package read access matters.
+- ~~Publish target~~ — decided 2026-11-07: public npmjs, as
+  `@versionone/sdk` (the `@versionone` org already publishes there), starting
+  at `1.0.0-alpha.0` on the `alpha` dist-tag so `latest` is never a
+  prerelease. Not `v1sdk` 3.x: the 2.x package (`v1sdk`, last published
+  2022) stays as it is, because a new major of the same name would imply a
+  compatibility this SDK does not aim for. Publishing needs an `@versionone`
+  org owner to set up a token or trusted publisher.
+- Dependency audit before the first publish — done 2026-11-07: no runtime
+  dependencies, and the built bundle imports nothing. `npm audit` found 13
+  issues, all in dev tooling (vitest/vite, tinypool, tsup/esbuild,
+  rimraf/glob); fixed by in-range updates plus vitest 3 → 5 and tsup 8 → 9.
+  `npm audit` is clean; unit and integration tiers pass on the new tools.
 - ~~Live instance for the integration tier from a cloud session~~ — done
   2026-11-04: `V1_URL`/`V1_ACCESS_TOKEN` point at the API-testing instance
   `https://www14.v1host.com/v1sdktesting`; `npm run test:integration` is
@@ -359,11 +398,16 @@ ESM" here means finishing the edges:
 - [x] A3 loc2 (`loc2.localize`, `client.localize`; GET, POST past 2000 encoded chars, culture via Accept-Language)
 - [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
+- [ ] A7 attachments and embedded images (`attachment` wire module, `client.attachments`; survey rooms first)
+- [ ] A6 CI + prerelease publish — CI (`.github/workflows/ci.yml`: verify on Node 20/22/24, `npm pack` artifact), git-SHA installs (`prepare`) and the dependency audit done; publish `@versionone/sdk@1.0.0-alpha.0` to npmjs with `--tag alpha` next (needs an org owner's token or trusted publisher)
 - [x] G0–G3 gh-v1 migrations — versionone/gh-v1#29–#32
 - [x] S1–S4 slack-bot migrations — digital-ai/k6i-agility-slack-bot#21–#24 (on `bolt-migration`)
-- [ ] B1–B4 slack bot SDK swap — brief: `plans/003-adopt-sdk-k6i-agility-slack-bot.md` (unblocked: S2, S3 and SDK #42 merged)
+- [x] B1 slack bot client swap — digital-ai/k6i-agility-slack-bot#29
+- [x] B2 slack bot decodes deleted; reads through `client.queryV1` with a schema — digital-ai/k6i-agility-slack-bot#30
+- [ ] B3–B4 slack bot writes on api/asset, number resolution — brief: `docs/handoffs/slack-bot-sdk-b3-b4.md` in the bot repo
 - [x] C0 gh-v1 surveyed (2026-11-04)
 - [x] C1 gh-v1 seam swap — versionone/gh-v1#33, with the vendored SDK's hashes, CI and Node 22 floor in #35
-- [ ] C2–C4 gh-v1 — brief: `plans/002-adopt-sdk-gh-v1.md` (C2–C3 in progress in a gh-v1 session)
+- [x] C2 gh-v1 rows translated through the schema; `isTrue`/`oidOf` gone — versionone/gh-v1#36
+- [x] C3 gh-v1 resolve and batch on `client.resolve`/`client.batch` — versionone/gh-v1#37 (with the SDK fixes from #43)
+- [ ] C4 gh-v1 attachments on `client.attachments` (after A7); `raw` stays CLI-local
 - [ ] D1–D4 agility-rooms

plans/001-sdk-adoption-campaign.md

@@ -66,8 +66,11 @@ runtime dependency, but its code ships inside the repo:
    `force`). Keep the CLI's UX for an unguardable batch by checking
    `unguardable()` first, then sending with `guard: false` and the existing
    note. Prompting, rendering, permalinks and receipt files stay in gh-v1.
-4. **C4 Passthrough.** `raw`/`blob` stay as they are. `lib/guard.js` stays:
-   it is CLI policy, not SDK behaviour.
+4. **C4 Attachments on the SDK** (after plan 001's A7). `gh v1 attach`
+   (`list`, `get`, `add`, `put`) moves onto `client.attachments`; `blob`
+   leaves `lib/api.js`. File I/O, `--out`, the content-type guess and the
+   CLI's messages stay in gh-v1. `raw` (the `gh v1 api` passthrough) and
+   `lib/guard.js` stay: they are CLI policy, not SDK behaviour.
 
 ## Rules
 

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

@@ -64,7 +64,10 @@ So the `assetApi` wire module translates a row against an `AttributeLookup`
 to the attribute it lands on; Numeric, LongInt and Rank become numbers, Boolean
 booleans, a single relation one Ref or null (the endpoint sends an unset
 one as `{ _oid: "NULL" }`), an aggregate a number, and a
-sub-select's rows are translated by their own type. A key the schema does
+sub-select's rows are translated by their own type. A `group` query's
+rows are groups (`_oid` the group key, `null` for the unset group; rows
+under `_children`), and the children translate by the query's own type,
+recursively for nested groups. A key the schema does
 not know is left as sent. Without a lookup the row keeps the endpoint's
 spelling, OIDs normalised and a `NULL` relation null — never silently
 guessed.

specs/000-layers.md

@@ -4,7 +4,7 @@ import type { Transport } from "./core/transport";
 import * as batch from "./services/batch";
 import * as guard from "./services/guard";
 import * as resolve from "./services/resolve";
-import { Schema } from "./services/schema";
+import { Schema, type TypeCache } from "./services/schema";
 import type { Asset } from "./types/asset";
 import type { Metamodel } from "./types/metamodel";
 import * as assetApi from "./wire/assetApi";
@@ -25,6 +25,13 @@ export interface ClientOptions {
   metamodel?: Metamodel;
   /** Share an existing schema (and its cache) instead of starting a new one. */
   schema?: Schema;
+  /**
+   * Share loaded type definitions with other clients of the same instance,
+   * whatever their credentials: this client's schema keeps its types in
+   * `types` and loads anything missing through this client's transport.
+   * A server that builds a client per user keeps one TypeCache per instance.
+   */
+  types?: TypeCache;
 }
 
 /**
@@ -53,7 +60,7 @@ export class AgilityApiClient {
       unauthorizedStatuses: config.unauthorizedStatuses
     });
     this.config = config;
-    this.schema = opts.schema ?? new Schema(this.transport, opts.metamodel);
+    this.schema = opts.schema ?? new Schema(this.transport, opts.metamodel, opts.types);
   }
 
   private readonly config: ClientConfig;

src/client.ts

@@ -23,7 +23,7 @@ export type { AssetDocument, AssetsDocument, ReadParams, ReadResult } from "./wi
 export type { Localizations, LocalizeOptions } from "./wire/loc2";
 
 // Services: what to do.
-export { Schema } from "./services/schema";
+export { Schema, TypeCache } from "./services/schema";
 export { CHUNK as RESOLVE_CHUNK, NUMBER_ROOTS, isAssetNumber, resolveNumbers, resolveRef } from "./services/resolve";
 export type { Resolution, ResolvedAsset } from "./services/resolve";
 export { UNGUARDABLE_TYPES, UnguardableError, guard, sendGuarded, stampsOf, touchedBy, unguardable } from "./services/guard";

src/index.ts

@@ -17,13 +17,42 @@ import type { AssetType, AssetTypeToken, AttributeDefinition, AttributeLookup, M
 import * as meta from "../wire/meta";
 import { attributeOf, fromTypeOf, typesNeeded, type SelectShape } from "../wire/select";
 
-export class Schema implements AttributeLookup {
+/**
+ * The type definitions loaded from one instance, shareable by any number of
+ * schemas. A definition does not depend on who read it, so a server that
+ * builds a client per user (each with that user's token) can give them all
+ * one cache per instance: each type is fetched once, through whichever
+ * client needed it first, and concurrent loads of a type share one request.
+ */
+export class TypeCache {
   /** Loaded types; null records that the instance has no such type, so it is not asked for again. */
-  private readonly types = new Map<AssetTypeToken, AssetType | null>();
-  private readonly loading = new Map<AssetTypeToken, Promise<AssetType | null>>();
+  readonly types = new Map<AssetTypeToken, AssetType | null>();
+  readonly loading = new Map<AssetTypeToken, Promise<AssetType | null>>();
+
+  /** A cache seeded from a metamodel document. */
+  static fromMetamodel(metamodel: Metamodel): TypeCache {
+    const cache = new TypeCache();
+    for (const [token, type] of Object.entries(metamodel.AssetTypes)) cache.types.set(token, type);
+    return cache;
+  }
+}
+
+export class Schema implements AttributeLookup {
+  private readonly types: Map<AssetTypeToken, AssetType | null>;
+  private readonly loading: Map<AssetTypeToken, Promise<AssetType | null>>;
+  /** Where this schema keeps what it loads; pass it to another schema to share it. */
+  readonly cache: TypeCache;
+
+  constructor(private readonly transport?: Transport, seed?: Metamodel, cache: TypeCache = new TypeCache()) {
+    this.cache = cache;
+    this.types = cache.types;
+    this.loading = cache.loading;
+    if (seed) for (const [token, type] of Object.entries(seed.AssetTypes)) if (!this.types.has(token)) this.types.set(token, type);
+  }
 
-  constructor(private readonly transport?: Transport, seed?: Metamodel) {
-    if (seed) for (const [token, type] of Object.entries(seed.AssetTypes)) this.types.set(token, type);
+  /** The same cache, loading anything missing through another transport (another user's credentials). */
+  withTransport(transport: Transport): Schema {
+    return new Schema(transport, undefined, this.cache);
   }
 
   /** A complete schema from a metamodel document; nothing is ever fetched. */

src/services/schema.ts

@@ -50,6 +50,8 @@ export interface QueryBody {
   find?: string;
   findin?: string[];
   asof?: string;
+  /** Group the rows by a relation: each result row is then a group, its rows under `_children` (translated like any row of `from`). */
+  group?: (string | { from: string; [key: string]: unknown })[];
 }
 export interface UpdateBody { from: string; filter?: string[]; where?: Record<string, string>; with?: Record<string, CommandValue>; update: Record<string, CommandValue> }
 export interface ExecuteBody { from: string; filter?: string[]; where?: Record<string, string>; with?: Record<string, CommandValue>; execute: string | { op: string; args?: Record<string, CommandValue>; list?: Record<string, CommandValue>[] } }
@@ -128,8 +130,17 @@ export interface RowContext { lookup: AttributeLookup; type: AssetTypeToken }
  */
 export function fromWireRow<Row extends Asset = Asset>(row: Record<string, unknown>, context?: RowContext): Row {
   const out: Record<string, unknown> = {};
+  // A `group` query answers with group rows: the group's key as `_oid` and
+  // the grouped rows under `_children`. The children are rows of the query's
+  // own type (or group rows again, for nested groups), so they translate
+  // with the same context. No attribute is called _children, so its presence
+  // is what marks a group row; a "NULL" key is the group of rows with the
+  // relation unset, and reads as null like any unset relation.
+  const grouped = Array.isArray(row._children);
   for (const [k, v] of Object.entries(row)) {
-    if (k === "_oid" && typeof v === "string") out[k] = momentless(v);
+    if (grouped && k === "_children") out[k] = (v as unknown[]).map((child) => (child && typeof child === "object" ? fromWireRow(child as Record<string, unknown>, context) : child));
+    else if (grouped && k === "_oid" && v === "NULL") out[k] = null;
+    else if (k === "_oid" && typeof v === "string") out[k] = momentless(v);
     else out[k] = context ? translate(v, resolvePath(context.lookup, context.type, k), context.lookup) : fromWireValue(v);
   }
   return out as Row;