diff --git a/README.md b/README.md index ce042f4..d228285 100644 --- a/README.md +++ b/README.md @@ -25,7 +25,7 @@ npm test - `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. `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 + - `batch.run` / `batch.reversible` - preview, snapshot, (confirm,) send guarded, and a receipt with a reverse batch. Undo (`batch.reversible`) reactivates a closed asset before deleting it, and a second command on one asset is sent as a second guarded pass. `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`) diff --git a/plans/001-sdk-adoption-campaign.md b/plans/001-sdk-adoption-campaign.md index 6000e8f..bd575db 100644 --- a/plans/001-sdk-adoption-campaign.md +++ b/plans/001-sdk-adoption-campaign.md @@ -65,6 +65,13 @@ Findings from the testing instance worth carrying into later milestones: applies. - A StoryStatus create requires `RollupState` (`"Active"` and `64` both accepted). +- An api/asset update that gives a multi-value relation as a plain list + **adds** to it (like rest-1's `act: "add"`); it does not replace the list + (slack bot, 2026-11-07). +- **Agility refuses to Delete a closed asset** + (`Violation'AssetCannotDeleteClosed`); reactivate first. The batch + service's reverse does this (`reversible` reports `reactivated`), and the + guard sends a second command on one asset as a second pass. ## What the consumers do today (surveyed 2026-11-04) @@ -91,6 +98,37 @@ hand-decodes string booleans in 48 places (including `getApiResults` patching `IsReadOnly`), and `src/metaAssetStore/` keeps its own per-type field knowledge that overlaps the SDK's `schema` service. +Re-surveyed 2026-11-07 at `main` `0874590` (read-only): + +- **HTTP.** One default axios instance (`src/utils/axios.tsx`) plus + `getAxiosInstance(...)` variants, a bare `meta` instance and a plain + `axios.create` in `src/navigation/routes.tsx`; several skip the + auth-refresh and snackbar interceptors. 88 production files import axios. + Cookie auth only. On 401/403 an SSO popup (`Account.mvc/Login`), then + replay. Cancellation by `AbortController` in a few places; no retries. + The commandFailures snackbar runs only when the URL is exactly + `/api/asset`; 9 calls written `api/asset` (no slash) bypass it. +- **Endpoints.** api/asset (111 call sites, 57 files), query.v1 (16), + rest-1.v1 reads (4) **and writes** (5 `Data/{T}/{id}` posts with + `{ Attributes: { X: { value, act } } }`, plus 2 `?op=Copy`), meta.v1, + loc-2.v1, `loc.v1`, `config.v1`, `attachment.img`, `embedded.img`, + `EmbeddedImage.mvc/Upload`, about 15 other `*.mvc` routes and about 20 + custom `api/*` routes (reports, tags, watch, rooms2config, LLM). +- **Writes.** `metaAssetSlice` updates multi-value fields as + `{ X: { [value]: "add" | "remove" } }`, sends a cleared relation as + `"NULL"`, updates optimistically and reverts on error. Execute ops with + and without `args` (Convert). No change comments, no ChangeDate checks. +- **Reads.** 34 explicit pages, but some board and `ID=$Oids` reads are + unbounded. Positional `results[n]` with index constants + (`metaAssetStore/constants.ts`). No `group` queries. +- **Decoding.** 46 `"True"`/`"False"` comparisons; `NULL_OID` referenced + 118 times; `massageApiAssetResponseObject` unwraps single relations by + meta. meta.v1 is loaded by `store/metaDefinition/useMetaDefinitionStore.ts` + (per type, in-flight dedupe, ETag revalidation), not by `metaAssetStore/`. +- **Tests.** Jest (222 files) mocks modules; no MSW. The AI domain's + injected `IHttpClient` / `FakeHttpClient` is the cleanest seam for a swap. + Playwright (79 specs) runs against a live server, including attachment CRUD. + **gh-v1** (surveyed 2026-11-04 at `41fe333`). A `gh` extension in plain CommonJS Node (≥20), no runtime dependencies, `node --test` with 99 passing tests. One seam, `lib/api.js` (native `fetch`, `Authorization` header from @@ -213,6 +251,25 @@ moves. Integration tests on v1sdktesting with `sdk-integration` fixtures, deleted by OID. + What rooms does today (survey 2026-11-07), which A7 must cover: + - **Upload** is one api/asset create with the bytes inline: `{ AssetType: + "Attachment", Asset, ContentType, Filename, Name, Content: }` + (`MultiValueRelations/Attachments.tsx`). The UI caps a file at 4 MB and + has no 413 handling. So `add` needs both modes: inline Base64 for small + files (one request, what rooms does) and create-then-POST-bytes for + large ones (what the docs and gh-v1 do), chosen by size. + - **Preview** of an image re-reads `Content` for one row and builds a + `data:` URL; **download** opens `attachment.img/` from the app root. + So `get` returns bytes and type, and the service exposes the + `attachment.img` URL for a browser to open. + - **Delete** is `execute: "Delete"` on the Attachment. + - **Embedded images** (TinyMCE paste) post multipart to + `EmbeddedImage.mvc/Upload` with fields `image` and `oidToken`; the + answer is HTML with ``. Stored HTML + references `embedded.img/`, rewritten to app-root URLs. So: + `embeddedImages.upload(asset, blob)` → `{ id, url }`, parsing that + response, and a helper that normalises `embedded.img` URLs. + ## Track B — k6i-agility-slack-bot (fast, on `bolt-migration`) Target branch: `bolt-migration`, not `main`. There, Bolt Phase 2 has already @@ -281,6 +338,13 @@ SDK behaviour. ## Track D — agility-rooms (slow) +Open questions from the 2026-11-07 survey: rooms still writes through +rest-1 (5 attribute posts, 2 `?op=Copy`) and reads `rest-1.v1/New/{T}` +templates, which gap 6's "no rest-1 writes" did not foresee; decide whether +those move to api/asset (Copy as an execute?) or the SDK grows a narrow +rest-1 write path. Its commandFailures policy should apply to every +api/asset call, not only URLs spelled `/api/asset`. + Strangler, starting below the call sites so nothing user-visible moves: - **D1 SDK under the existing axios.** Implement a `Transport` that @@ -404,7 +468,8 @@ ESM" here means finishing the edges: - [x] S1–S4 slack-bot migrations — digital-ai/k6i-agility-slack-bot#21–#24 (on `bolt-migration`) - [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] B3 slack bot writes on `client.assets.send` (rest-1 writes gone) — digital-ai/k6i-agility-slack-bot#32 +- [x] B4 slack bot number lookups on `client.resolve.numbers` — digital-ai/k6i-agility-slack-bot#34 (SDK #44 adopted in #33: pin `785b4c9`, a `TypeCache` per instance) - [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 - [x] C2 gh-v1 rows translated through the schema; `isTrue`/`oidOf` gone — versionone/gh-v1#36 diff --git a/specs/000-layers.md b/specs/000-layers.md index ada832b..ed2dabf 100644 --- a/specs/000-layers.md +++ b/specs/000-layers.md @@ -176,3 +176,8 @@ exception to "values travel as `with` variables", because a ChangeDate variable never compares equal — after a strict timestamp check. Moment is not a stamp: a filter on an earlier Moment selects the asset as it was then. A guarded batch with no creates is sent idempotent; a plain one never is. +Two commands on one asset are sent in passes: the second goes only if the +first applied, guarded on the ChangeDate read just after it (a shared +pre-batch stamp would make the second match nothing). A reverse batch +reactivates a closed asset before deleting it, since Agility refuses to +delete a closed asset. diff --git a/src/client.ts b/src/client.ts index ec2ef46..ad3a2cf 100644 --- a/src/client.ts +++ b/src/client.ts @@ -99,8 +99,8 @@ export class AgilityApiClient { * as is. */ readonly resolve = { - numbers: (numbers: readonly string[]) => resolve.resolveNumbers(this.transport, numbers), - ref: (spec: string) => resolve.resolveRef(this.transport, spec) + numbers: (numbers: readonly string[]) => resolve.resolveNumbers(this.transport, numbers, { types: this.schema.cache }), + ref: (spec: string) => resolve.resolveRef(this.transport, spec, { types: this.schema.cache }) }; /** diff --git a/src/index.ts b/src/index.ts index 4bba368..504b76f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -8,7 +8,7 @@ export type { RequestBody, RequestOptions, ResponseType, Transport } from "./cor // The model: what things are. export type { Asset, AttributeValue, CustomAttributes, DateString, Oid, Ref, AssetStateValue } from "./types/asset"; -export { ASSET_STATE_CODES } from "./types/asset"; +export { ASSET_STATE_CODES, assetStateOf } from "./types/asset"; export { isOidToken, momentless, parseOid, toOid, typeOfOid } from "./types/oid"; export type { OidParts } from "./types/oid"; export type { AssetType, AttributeDefinition, AttributeLookup, AttributeType, AttributeTokenRef, AssetTypeToken, Metamodel } from "./types/metamodel"; diff --git a/src/services/batch.ts b/src/services/batch.ts index 7b809d5..ff81f9f 100644 --- a/src/services/batch.ts +++ b/src/services/batch.ts @@ -22,7 +22,7 @@ import type { AssetTypeToken } from "../types/metamodel"; import { momentless, typeOfOid } from "../types/oid"; import * as assetApi from "../wire/assetApi"; import type { Command, CommandValue } from "../wire/assetApi"; -import { aliased, guard, stampsOf, touchedBy, unguardable, UnguardableError } from "./guard"; +import { aliased, guard, GuardedPassError, passesOf, sendPasses, stampsOf, touchedBy, unguardable, UnguardableError } from "./guard"; import type { Schema } from "./schema"; /** OIDs per lookup query: bounded, never one giant filter. */ @@ -275,18 +275,30 @@ export async function runBatch(transport: Transport, schema: Schema, commands: C if (options.dryRun) return receipt; if (options.confirm && !(await options.confirm(receipt))) return receipt; - let toSend = commands, unstamped: Oid[] = []; - if (useGuard) { - const stamps = Object.fromEntries(Object.entries(receipt.before).flatMap(([oid, v]) => (typeof v.ChangeDate === "string" ? [[oid, v.ChangeDate]] : []))); - ({ commands: toSend, unstamped } = guard(projected, stamps)); - } + let unstamped: Oid[] = []; receipt.guarded = useGuard; - receipt.sentCommands = toSend; receipt.sentAt = now(); let done: assetApi.Outcome; try { - done = assetApi.outcomeOf(await assetApi.send(transport, toSend, { idempotent: useGuard && !toSend.some((c) => "AssetType" in bodyOf(c)) })); + if (useGuard) { + // Guarded pass by pass: a second command on the same asset (Reactivate + // then Delete in a reverse) goes only if the first applied (guard.ts). + const stamps = Object.fromEntries(Object.entries(receipt.before).flatMap(([oid, v]) => (typeof v.ChangeDate === "string" ? [[oid, v.ChangeDate]] : []))); + receipt.sentCommands = guard(passesOf(projected)[0]!, stamps).commands; // what the first pass sends; replaced below by everything sent + const result = await sendPasses(transport, projected, stamps); + ({ outcome: done, unstamped } = result); + receipt.sentCommands = result.sent; + } else { + receipt.sentCommands = commands; + done = assetApi.outcomeOf(await assetApi.send(transport, commands)); + } } catch (error) { + if (error instanceof GuardedPassError) { + // Earlier passes applied: the receipt says so, and what failed after. + receipt.sent = true; + receipt.outcome = error.outcome; + receipt.sentCommands = error.sent; + } throw new BatchSendError(error, receipt); } receipt.sent = true; @@ -315,15 +327,43 @@ export async function runBatch(transport: Transport, schema: Schema, commands: C return receipt; } +/** Each asset's AssetState by name ("Active", "Closed"), one bounded query per type and chunk. An asset that no longer exists is absent. */ +async function statesOf(transport: Transport, oids: Oid[]): Promise> { + const out: Record = {}; + for (const [type, list] of byType(oids)) { + for (const part of chunks(list)) { + const rows = await assetApi.query(transport, { from: type, select: ["AssetState"], filter: ["ID=$oids"], with: { $oids: part }, page: { size: part.length, start: 0 } }); + for (const r of rows) if (r.AssetState != null) out[momentless(r._oid)] = String(r.AssetState); + } + } + return out; +} + +/** AssetState as the endpoint spells a closed asset: by name, or by Core's code (128). */ +const isClosed = (state: string | undefined): boolean => state === "Closed" || state === "128"; + /** * A receipt's reverse batch, minus the assets someone changed since the * batch (their ChangeDate is no longer the one recorded just after it), * unless `force`. Send what it returns with runBatch like any batch. + * + * Agility refuses to Delete a closed asset (Violation'AssetCannotDeleteClosed), + * so a Delete of an asset that is closed now gets a Reactivate just before + * it: two commands on one asset, which runBatch sends in two guarded passes + * (the Delete only if the Reactivate applied). `reactivated` lists them. */ -export async function reversible(transport: Transport, receipt: Pick, options: { force?: boolean } = {}): Promise<{ commands: Command[]; drifted: { oid: Oid; was: string; now: string | null }[] }> { +export async function reversible(transport: Transport, receipt: Pick, options: { force?: boolean } = {}): Promise<{ commands: Command[]; drifted: { oid: Oid; was: string; now: string | null }[]; reactivated: Oid[] }> { const targets = receipt.reverse.commands.map((c) => momentless(String(bodyOf(c).from))); const now = await stampsOf(transport, targets.filter((oid) => receipt.after[oid] !== undefined)); const drifted = targets.flatMap((oid) => (receipt.after[oid] !== undefined && now[oid] !== receipt.after[oid] ? [{ oid, was: receipt.after[oid]!, now: now[oid] ?? null }] : [])); const skip = new Set(options.force ? [] : drifted.map((d) => d.oid)); - return { commands: receipt.reverse.commands.filter((c) => !skip.has(momentless(String(bodyOf(c).from)))), drifted }; + const kept = receipt.reverse.commands.filter((c) => !skip.has(momentless(String(bodyOf(c).from)))); + const deletes = kept.flatMap((c) => (bodyOf(c).execute === "Delete" ? [momentless(String(bodyOf(c).from))] : [])); + const states = deletes.length ? await statesOf(transport, deletes) : {}; + const reactivated = deletes.filter((oid) => isClosed(states[oid])); + const commands = kept.flatMap((c) => { + const body = bodyOf(c), oid = momentless(String(body.from)); + return body.execute === "Delete" && reactivated.includes(oid) ? [{ from: oid, execute: "Reactivate" } as Command, c] : [c]; + }); + return { commands, drifted, reactivated }; } diff --git a/src/services/guard.ts b/src/services/guard.ts index 76aa12c..28db6dc 100644 --- a/src/services/guard.ts +++ b/src/services/guard.ts @@ -136,6 +136,80 @@ export function guard(projected: Command[], stamps: Readonly return { commands, unstamped }; } +// ---- passes -------------------------------------------------------------------- + +/** + * A projection split into passes: the k-th per-asset command for each asset + * goes in pass k; creates and queries ride in pass 0. One pass is the usual + * case. Two commands on one asset (a reverse batch's Reactivate then Delete, + * say) cannot share a pass: both would be stamped with the ChangeDate read + * before the batch, the first would advance it, and the second would match + * nothing — silently skipped. + */ +export function passesOf(projected: Command[]): Command[][] { + const seen = new Map(); + const passes: Command[][] = [[]]; + for (const p of projected) { + const x = perAsset(p); + const k = x ? seen.get(x.oid) ?? 0 : 0; + if (x) seen.set(x.oid, k + 1); + (passes[k] ??= []).push(p); + } + return passes; +} + +/** A guarded send that failed after some passes applied: `outcome` and `sent` say what did. */ +export class GuardedPassError extends Error { + constructor(readonly cause: unknown, readonly outcome: assetApi.Outcome, readonly sent: Command[], readonly pass: number) { + super(`guarded send failed in pass ${pass + 1}${pass ? " (earlier passes applied)" : ""}: ${cause instanceof Error ? cause.message : String(cause)}`, { cause }); + this.name = "GuardedPassError"; + } +} + +/** + * Send a projection guarded, pass by pass. Pass 0 is guarded on `stamps` + * (read before the batch). A later pass sends an asset's next command only + * if its previous one applied, guarded on the ChangeDate read again just + * after it — so the chain stays a compare-and-set, with one narrow window + * between our write and that read. An asset whose earlier command was + * skipped is skipped for the rest of the batch. Failure indexes are + * positions in `sent` (all passes, in order). A send that fails in the + * first pass throws as it would alone; in a later pass it throws a + * GuardedPassError carrying what the earlier passes did. + */ +export async function sendPasses(transport: Transport, projected: Command[], stamps: Readonly>, options: Pick = {}): Promise<{ outcome: assetApi.Outcome; sent: Command[]; unstamped: Oid[] }> { + const passes = passesOf(projected); + const outcome = assetApi.emptyOutcome(); + const sent: Command[] = [], unstamped: Oid[] = []; + let applied = new Set(); + for (const [k, pass] of passes.entries()) { + let current: { commands: Command[]; unstamped: Oid[] }; + if (k === 0) current = guard(pass, stamps); + else { + // Only assets whose previous command applied go on, stamped afresh. + const eligible = pass.filter((p) => applied.has(perAsset(p)!.oid)); + current = guard(eligible, await stampsOf(transport, eligible.map((p) => perAsset(p)!.oid))); + } + const commands = current.commands; + unstamped.push(...current.unstamped); + if (!commands.length) break; + const offset = sent.length; + let done: assetApi.Outcome; + try { + const creates = commands.some((c) => "AssetType" in bodyOf(c)); + done = assetApi.outcomeOf(await assetApi.send(transport, commands, { ...options, idempotent: !creates })); + } catch (error) { + // Nothing applied yet: the error is the whole story, as for a single send. + if (k === 0) throw error; + throw new GuardedPassError(error, outcome, sent, k); + } + sent.push(...commands); + assetApi.mergeOutcome(outcome, done, offset); + applied = new Set([...done.modified, ...done.operated].map(momentless)); + } + return { outcome, sent, unstamped: [...new Set(unstamped)] }; +} + // ---- the whole move -------------------------------------------------------------- export interface GuardedResult { @@ -169,9 +243,7 @@ export async function sendGuarded(transport: Transpor const ideas = touched.filter((oid) => UNGUARDABLE_TYPES.includes(typeOfOid(oid))); if (ideas.length) throw new UnguardableError(ideas.map((oid) => ({ index: -1, reason: `${oid}: ${typeOfOid(oid)} has no ChangeDate` }))); const stamps = await stampsOf(transport, touched); - const { commands: sent, unstamped } = guard(projected, stamps); - const creates = sent.some((c) => "AssetType" in bodyOf(c)); - const outcome = assetApi.outcomeOf(await assetApi.send(transport, sent, { ...options, idempotent: !creates })); + const { outcome, sent, unstamped } = await sendPasses(transport, projected, stamps, options); const done = new Set([...outcome.modified, ...outcome.operated]); return { outcome, projected, sent, touched, stamps, skipped: [...new Set([...unstamped, ...touched.filter((oid) => !done.has(oid))])] }; } diff --git a/src/services/resolve.ts b/src/services/resolve.ts index d84c29d..11ed493 100644 --- a/src/services/resolve.ts +++ b/src/services/resolve.ts @@ -18,7 +18,8 @@ * matching on the server is case-insensitive. */ import type { Transport } from "../core/transport"; -import type { Asset, AssetStateValue, Oid } from "../types/asset"; +import { assetStateOf, type Asset, type AssetStateValue, type Oid } from "../types/asset"; +import type { TypeCache } from "./schema"; import type { AssetTypeToken } from "../types/metamodel"; import { isOidToken, momentless, typeOfOid } from "../types/oid"; import * as assetApi from "../wire/assetApi"; @@ -76,15 +77,25 @@ export interface Resolution { missing: string[]; } -// Roots an instance turned out not to have, per transport: asked once, then left out. +// Roots an instance turned out not to have, asked once, then left out. With a +// TypeCache (the client passes its schema's) that is the cache's own record of +// types the instance lacks (null), so clients that share a cache share it; +// without one, it is remembered per transport. const absentRoots = new WeakMap>(); + +interface Absent { has(root: string): boolean; add(root: string): void } +const absentFor = (transport: Transport, types?: TypeCache): Absent => { + if (types) return { has: (root) => types.types.get(root) === null, add: (root) => { types.types.set(root, null); } }; + const set = absentRoots.get(transport) ?? new Set(); + absentRoots.set(transport, set); + return set; +}; const UNKNOWN_TYPE = /^Unknown AssetType: (\S+)/; -type Row = Asset & { Name?: string | null; Number?: string; AssetState?: AssetStateValue | null }; +type Row = Asset & { Name?: string | null; Number?: string; AssetState?: unknown }; -async function askRoots(transport: Transport, numbers: string[]): Promise { - const absent = absentRoots.get(transport) ?? new Set(); - absentRoots.set(transport, absent); +async function askRoots(transport: Transport, numbers: string[], types?: TypeCache): Promise { + const absent = absentFor(transport, types); for (let attempt = 0; ; attempt += 1) { const roots = Object.keys(NUMBER_ROOTS).filter((r) => !absent.has(r)); try { @@ -108,7 +119,7 @@ async function askRoots(transport: Transport, numbers: string[]): Promise } /** Resolve asset numbers in one request per hundred numbers. Not found is not an error: it is `missing`. */ -export async function resolveNumbers(transport: Transport, numbers: readonly string[]): Promise { +export async function resolveNumbers(transport: Transport, numbers: readonly string[], options: { types?: TypeCache } = {}): Promise { const firsts = new Map(); for (const n of numbers) if (!firsts.has(n.trim().toUpperCase())) firsts.set(n.trim().toUpperCase(), n.trim()); const asked = [...firsts.values()]; @@ -116,10 +127,10 @@ export async function resolveNumbers(transport: Transport, numbers: readonly str if (invalid.length) throw new TypeError(`not asset numbers (expected a prefix and digits, S-1234): ${invalid.join(", ")}`); const byNumber = new Map(); for (let i = 0; i < asked.length; i += CHUNK) { - for (const r of await askRoots(transport, asked.slice(i, i + CHUNK))) { + for (const r of await askRoots(transport, asked.slice(i, i + CHUNK), options.types)) { if (!r.Number || byNumber.has(r.Number.toUpperCase())) continue; const oid = momentless(r._oid); - byNumber.set(r.Number.toUpperCase(), { number: r.Number, oid, type: typeOfOid(oid), name: r.Name ?? null, assetState: r.AssetState ?? null }); + byNumber.set(r.Number.toUpperCase(), { number: r.Number, oid, type: typeOfOid(oid), name: r.Name ?? null, assetState: assetStateOf(r.AssetState) as AssetStateValue | null }); } } return { @@ -133,9 +144,9 @@ export async function resolveNumbers(transport: Transport, numbers: readonly str * used as is, no request, so `number` and `name` are null) or a number * (resolved; null when no root knows it). Anything else is a TypeError. */ -export async function resolveRef(transport: Transport, spec: string): Promise { +export async function resolveRef(transport: Transport, spec: string, options: { types?: TypeCache } = {}): Promise { const text = spec.trim(); if (isOidToken(text)) { const oid = momentless(text); return { number: null, oid, type: typeOfOid(oid), name: null, assetState: null }; } - if (isAssetNumber(text)) return (await resolveNumbers(transport, [text])).found[0] ?? null; + if (isAssetNumber(text)) return (await resolveNumbers(transport, [text], options)).found[0] ?? null; throw new TypeError(`"${spec}" is neither an asset number (S-1234) nor an OID token (Story:1234)`); } diff --git a/src/types/asset.ts b/src/types/asset.ts index 168bf22..c95aea9 100644 --- a/src/types/asset.ts +++ b/src/types/asset.ts @@ -19,10 +19,24 @@ export type DateString = string; * OldBrokenDownStory 208, Deprecated 254, Deleted 255). rest-1 renders the * number and query.v1 the name; the wire layer normalises to the name. */ -export type AssetStateValue = "Future" | "Active" | "Closed" | "Dead" | "OldBrokenDownStory" | "Deprecated" | "Deleted"; +export type AssetStateValue = "Future" | "Active" | "Closed" | "Dead" | "Template" | "OldBrokenDownStory" | "Deprecated" | "Deleted"; /** Core's AssetState codes, by name — the rest-1 rendering. */ -export const ASSET_STATE_CODES: Record = { Future: 0, Active: 64, Closed: 128, Dead: 192, OldBrokenDownStory: 208, Deprecated: 254, Deleted: 255 }; +export const ASSET_STATE_CODES: Record = { Future: 0, Active: 64, Closed: 128, Dead: 192, Template: 200, OldBrokenDownStory: 208, Deprecated: 254, Deleted: 255 }; + +const STATE_OF_CODE: Record = Object.fromEntries(Object.entries(ASSET_STATE_CODES).map(([name, code]) => [code, name as AssetStateValue])); + +/** + * An AssetState by name, whichever way the endpoint spelled it: api/asset + * and query.v1 send the name ("Closed") for most states but the code for a + * few ("200", a template), and rest-1 always sends the number. An unknown + * code is left as sent. + */ +export function assetStateOf(value: unknown): AssetStateValue | string | null { + if (value == null) return null; + const code = typeof value === "number" ? value : typeof value === "string" && /^\d+$/.test(value) ? Number(value) : NaN; + return Number.isNaN(code) ? String(value) : STATE_OF_CODE[code] ?? String(value); +} /** * A reference to another asset: always at least its OID, plus whatever a diff --git a/src/wire/assetApi.ts b/src/wire/assetApi.ts index 737a6de..07db990 100644 --- a/src/wire/assetApi.ts +++ b/src/wire/assetApi.ts @@ -53,7 +53,22 @@ export interface QueryBody { /** 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 })[]; } +/** + * An update. Each key of `update` is an attribute; its value replaces a + * scalar or a single relation (null clears it). A multi-value relation is + * changed with `{ add: [...], remove: [...] }`. A plain list does NOT + * replace a multi-value relation: the server ADDS its members, like rest-1's + * `act: "add"` (seen live on v1sdktesting, 2026-11-07). To set one exactly, + * remove what should go and add what should come. A change comment goes in + * the update node as `ChangeComment`. + */ export interface UpdateBody { from: string; filter?: string[]; where?: Record; with?: Record; update: Record } +/** + * An operation (QuickClose, Inactivate, Reactivate, Delete, …), optionally + * with arguments. It cannot carry a change comment. Agility refuses to + * Delete a closed asset (Violation'AssetCannotDeleteClosed): Reactivate it + * first, as batch.reversible does. + */ export interface ExecuteBody { from: string; filter?: string[]; where?: Record; with?: Record; execute: string | { op: string; args?: Record; list?: Record[] } } export type CommandBody = CreateBody | QueryBody | UpdateBody | ExecuteBody; @@ -242,6 +257,24 @@ export function outcomeOf(response: AssetApiResponse< }; } +/** An outcome with nothing in it, to merge several sends into. */ +export const emptyOutcome = (): Outcome => ({ created: [], modified: [], operated: [], failures: [], queryResults: [], durationSeconds: null }); + +/** + * Add one send's outcome to another's, for a batch sent in several + * requests. `offset` is how many commands were sent before this one, so a + * failure's index stays a position in everything sent. + */ +export function mergeOutcome(into: Outcome, done: Outcome, offset: number): Outcome { + into.created.push(...done.created); + into.modified.push(...done.modified); + into.operated.push(...done.operated); + into.failures.push(...done.failures.map((f) => ({ ...f, index: f.index === null ? null : f.index + offset }))); + into.queryResults.push(...done.queryResults); + if (done.durationSeconds !== null) into.durationSeconds = (into.durationSeconds ?? 0) + done.durationSeconds; + return into; +} + /** * A batch in which at least one command failed. api/asset answers 200 and * commits per command, so this is not an HTTP failure and not all-or- diff --git a/src/wire/rest1.ts b/src/wire/rest1.ts index ed59db3..74065d2 100644 --- a/src/wire/rest1.ts +++ b/src/wire/rest1.ts @@ -23,7 +23,7 @@ * - a page is required by this module: no Data read is unbounded. */ import type { Transport } from "../core/transport"; -import { ASSET_STATE_CODES, type Asset, type AssetStateValue, type Oid, type Ref } from "../types/asset"; +import { assetStateOf, type Asset, type Oid, type Ref } from "../types/asset"; import { momentless } from "../types/oid"; export { momentless }; @@ -57,7 +57,6 @@ export interface ReadParams { // ---- translation ----------------------------------------------------------- -const CODE_TO_STATE: Record = Object.fromEntries(Object.entries(ASSET_STATE_CODES).map(([name, code]) => [code, name as AssetStateValue])); const toRef = (doc: AssetRefDocument): Ref => ({ _oid: momentless(doc.idref) }); @@ -76,7 +75,7 @@ export function fromAssetDocument(doc: AssetDocument, if (a._type === "Relation") { out[key] = Array.isArray(a.value) ? a.value.map(toRef) : a.value ? toRef(a.value) : null; } else if (key === "AssetState" && typeof a.value === "number") { - out[key] = CODE_TO_STATE[a.value] ?? a.value; + out[key] = assetStateOf(a.value); } else { out[key] = a.value; } diff --git a/test/integration/batch.test.ts b/test/integration/batch.test.ts index 05b926d..9b6c646 100644 --- a/test/integration/batch.test.ts +++ b/test/integration/batch.test.ts @@ -79,4 +79,18 @@ describeInstance("batch (live)", () => { const [row] = await assetApi.query<{ _oid: string; AssetState: string }>(t, { from: story, select: ["AssetState"], page: { size: 1, start: 0 } }); expect(row!.AssetState).toBe("Closed"); }); + it("undoing a batch whose created asset was closed since reactivates it first, then deletes it (Agility refuses to delete a closed asset)", async () => { + const receipt = await client.batch.run([{ "@s": { AssetType: "Story", Name: fixtureName("batch undo closed"), Scope: { _oid: "Scope:0" } } }]); + const story = receipt.created[0]!.oid; + created.push(story); + await assetApi.send(t, [{ from: story, execute: "QuickClose" }], { throwOnFailure: true }); + // Closing it changed it since the batch, so the reverse is forced. + const undo = await client.batch.reversible(receipt, { force: true }); + expect(undo.reactivated).toEqual([story]); + expect(undo.commands).toEqual([{ from: story, execute: "Reactivate" }, { from: story, execute: "Delete" }]); + const reversed = await client.batch.run(undo.commands); + expect(reversed.outcome!.failures).toEqual([]); + expect(reversed.skipped).toEqual([]); + expect(await assetApi.query(t, { from: "Story", select: ["Name"], filter: ["ID=$s"], with: { $s: story }, page: { size: 1, start: 0 } })).toEqual([]); + }); }); diff --git a/test/unit/batch.test.ts b/test/unit/batch.test.ts index 6b363ff..65963b5 100644 --- a/test/unit/batch.test.ts +++ b/test/unit/batch.test.ts @@ -163,6 +163,17 @@ group("batch", () => { expect(e.receipt.after).toEqual({}); }); + it("reversible puts a Reactivate before deleting an asset that is closed now: Agility refuses to delete a closed asset", async () => { + const receipt = { after: { "Task:50": "2026-11-04T17:00:00.5", "Story:1": "2026-11-04T17:00:00.1" }, reverse: { notes: [], commands: [{ from: "Story:1", update: { Estimate: "3" } }, { from: "Task:50", execute: "Delete" }] as Command[] } }; + const t = server({ preview: [], lookup: (q) => q.with!.$oids!.map((oid) => (q.select![0] === "AssetState" ? { _oid: oid, AssetState: "Closed" } : { _oid: oid, ChangeDate: receipt.after[oid as "Task:50"] })), send: () => envelope() }); + const r = await reversible(t, receipt); + expect(r.reactivated).toEqual(["Task:50"]); + expect(r.commands).toEqual([{ from: "Story:1", update: { Estimate: "3" } }, { from: "Task:50", execute: "Reactivate" }, { from: "Task:50", execute: "Delete" }]); + // An open one is deleted as is. + const open = server({ preview: [], lookup: (q) => q.with!.$oids!.map((oid) => (q.select![0] === "AssetState" ? { _oid: oid, AssetState: "Active" } : { _oid: oid, ChangeDate: receipt.after[oid as "Task:50"] })), send: () => envelope() }); + expect((await reversible(open, receipt)).reactivated).toEqual([]); + }); + it("reversible drops assets changed since the batch (their ChangeDate is not the one recorded after it) unless forced", async () => { const receipt = { after: { "Story:1": "2026-11-04T17:00:00.1", "Epic:9": "2026-11-04T17:00:00.3" }, reverse: { notes: [], commands: [{ from: "Story:1", update: { Estimate: "3" } }, { from: "Epic:9", execute: "Reactivate" }] as Command[] } }; const now: Record = { "Story:1": "2026-11-04T18:00:00.9", "Epic:9": "2026-11-04T17:00:00.3" }; diff --git a/test/unit/guard.test.ts b/test/unit/guard.test.ts index 3a23a03..01a6507 100644 --- a/test/unit/guard.test.ts +++ b/test/unit/guard.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it, vi } from "vitest"; import type { RequestOptions, Transport } from "../../src/core/transport"; -import { UnguardableError, guard, sendGuarded, touchedBy, unguardable } from "../../src/services/guard"; +import { UnguardableError, guard, passesOf, sendGuarded, sendPasses, touchedBy, unguardable } from "../../src/services/guard"; import type { Command } from "../../src/wire/assetApi"; const envelope = (over: Record = {}) => ({ @@ -81,3 +81,32 @@ describe("guard", () => { expect(ideas.calls).toHaveLength(1); // the preview only }); }); + +describe("guard passes: two commands on one asset", () => { + const P = (oid: string, cmd: Record) => ({ [`@${oid.replace(":", "")}${Object.keys(cmd)[0]}`]: { oid, ...cmd } }) as unknown as Command; + const env = (over: Record = {}) => ({ requestId: "r", assetsCreated: { oidTokens: [], count: 0 }, assetsModified: { oidTokens: [], count: 0 }, assetsOperatedOn: { oidTokens: [], count: 0 }, commandFailures: { commands: [], count: 0 }, queryResult: { results: [], resultMetaData: [], count: -1 }, ...over }); + + it("passesOf puts an asset's second command in a second pass; creates ride in the first", () => { + const create = { "@t": { AssetType: "Task", Name: "x" } } as Command; + const passes = passesOf([create, P("Story:1", { execute: { op: "Reactivate", args: {} } }), P("Story:2", { execute: { op: "Delete", args: {} } }), P("Story:1", { execute: { op: "Delete", args: {} } })]); + expect(passes.map((p) => p.length)).toEqual([3, 1]); + expect(passes[1]![0]).toEqual(P("Story:1", { execute: { op: "Delete", args: {} } })); + }); + + it("the second pass goes only for assets whose first command applied, guarded on the ChangeDate read just after it", async () => { + const sent: unknown[][] = []; + const transport = { request: vi.fn(async (o: { body: unknown[] }) => { + const body = o.body as Record[]; + if (body.length === 1 && (body[0]!.select as string[] | undefined)?.[0] === "ChangeDate") return env({ queryResult: { results: [[{ _oid: "Story:1", ChangeDate: "2026-11-07T12:00:00.5" }]], resultMetaData: [], count: 1 } }); + sent.push(body); + // Pass 1: Story:1 applies, Story:2 was changed by someone else and matches nothing. + return sent.length === 1 ? env({ assetsOperatedOn: { oidTokens: ["Story:1:77"], count: 1 } }) : env({ assetsOperatedOn: { oidTokens: ["Story:1"], count: 1 } }); + }) } as unknown as Parameters[0]; + const projected = [P("Story:1", { execute: { op: "Reactivate", args: {} } }), P("Story:2", { execute: { op: "Reactivate", args: {} } }), P("Story:1", { execute: { op: "Delete", args: {} } }), P("Story:2", { execute: { op: "Delete", args: {} } })]; + const r = await sendPasses(transport, projected, { "Story:1": "2026-11-07T11:00:00.1", "Story:2": "2026-11-07T11:00:00.2" }); + expect(sent).toHaveLength(2); + expect(sent[1]).toEqual([{ from: "Story", filter: ["ID=$oid", "ChangeDate='2026-11-07T12:00:00.5'"], with: { $oid: "Story:1" }, execute: { op: "Delete", args: {} } }]); // Story:2's Delete never sent + expect(r.sent).toHaveLength(3); + expect(r.outcome.operated).toEqual(["Story:1", "Story:1"]); + }); +}); diff --git a/test/unit/oid.test.ts b/test/unit/oid.test.ts index f1b0e14..650d155 100644 --- a/test/unit/oid.test.ts +++ b/test/unit/oid.test.ts @@ -25,3 +25,14 @@ describe("oid", () => { expect(momentless("@story")).toBe("@story"); }); }); + +import { assetStateOf } from "../../src/types/asset"; +describe("assetStateOf", () => { + it("reads a state by name however the endpoint spelled it: a name as is, a code (string or number) as its name, an unknown code as sent", () => { + expect(assetStateOf("Closed")).toBe("Closed"); + expect(assetStateOf("200")).toBe("Template"); + expect(assetStateOf(128)).toBe("Closed"); + expect(assetStateOf("199")).toBe("199"); + expect(assetStateOf(null)).toBeNull(); + }); +}); diff --git a/test/unit/resolve.test.ts b/test/unit/resolve.test.ts index 864ec2a..32c2fb2 100644 --- a/test/unit/resolve.test.ts +++ b/test/unit/resolve.test.ts @@ -3,6 +3,7 @@ import { join } from "node:path"; import { describe, expect, it, vi } from "vitest"; import type { RequestOptions, Transport } from "../../src/core/transport"; import { CHUNK, NUMBER_ROOTS, isAssetNumber, resolveNumbers, resolveRef } from "../../src/services/resolve"; +import { TypeCache } from "../../src/services/schema"; import type { MetamodelDocument } from "../../src/wire/meta"; const envelope = (results: unknown[][], failures: unknown[] = []) => ({ @@ -70,6 +71,22 @@ describe("resolve", () => { expect((t.calls[2]!.body as Sent).map((q) => q.from)).toEqual(roots); }); + it("with a TypeCache, an absent root is the cache's record (null), so a new client and transport sharing the cache never asks it again", async () => { + const roots = ROOTS.filter((r) => r !== "Okr"); + const missingOkr = envelope(roots.map(() => []), [{ "@x$auto-aliased": { from: "Okr" }, error: { message: "Unknown AssetType: Okr", sourceCommandIndex: ROOTS.indexOf("Okr") } }]); + const types = new TypeCache(); + const alice = fakeTransport(missingOkr, answer({}, roots)); + await resolveNumbers(alice, ["S-1"], { types }); + expect(types.types.get("Okr")).toBeNull(); + // Bob: another user, another transport, the same instance. + const bob = fakeTransport(answer({ Workitem: [{ _oid: "Task:1096", Name: "Template task", Number: "TK-01001", AssetState: "200" }] }, roots)); + const found = await resolveNumbers(bob, ["TK-01001"], { types }); + expect(bob.calls).toHaveLength(1); + expect((bob.calls[0]!.body as Sent).map((q) => q.from)).toEqual(roots); + // A template's state comes back as its code; it reads by name. + expect(found.found[0]!.assetState).toBe("Template"); + }); + it("any other failure is the caller's to see", async () => { const t = fakeTransport(envelope(ROOTS.slice(1).map(() => []), [{ "@x": {}, error: { message: "Invalid QueryFilter2 token Number", sourceCommandIndex: 0 } }])); await expect(resolveNumbers(t, ["S-1"])).rejects.toThrow(/Invalid QueryFilter2/);