From b84683bdcaeee35968df4fa28214ebcc74a1f175 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 10:51:15 +0000 Subject: [PATCH 1/4] Undo reactivates closed assets before deleting; guard sends passes Agility refuses to Delete a closed asset (Violation'AssetCannotDeleteClosed), so batch.reversible now reads the state of every asset its reverse deletes and puts a Reactivate before the Delete of each closed one (`reactivated`). That puts two commands on one asset, which a guarded batch could never send: both were stamped with the ChangeDate read before the batch, so the second always matched nothing and was silently skipped. The guard now sends an asset's later commands as later passes, each only if the previous one applied and guarded on the ChangeDate read just after it (sendPasses, passesOf); runBatch and sendGuarded use it. A failure after an earlier pass applied throws GuardedPassError / BatchSendError with what applied. Live (v1sdktesting): a batch-created Story, closed afterwards, is reactivated and deleted by its undo. Plan 001 records the finding, the multi-value-list-adds finding from the slack bot, and the agility-rooms survey (A7 requirements, Track D questions). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016NSWj9gL1oCubj5e2anVkb --- README.md | 2 +- plans/001-sdk-adoption-campaign.md | 64 ++++++++++++++++++++++++ specs/000-layers.md | 5 ++ src/services/batch.ts | 60 +++++++++++++++++++---- src/services/guard.ts | 78 ++++++++++++++++++++++++++++-- src/wire/assetApi.ts | 18 +++++++ test/integration/batch.test.ts | 14 ++++++ test/unit/batch.test.ts | 11 +++++ test/unit/guard.test.ts | 31 +++++++++++- 9 files changed, 268 insertions(+), 15 deletions(-) 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..fae0bbe 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 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/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/wire/assetApi.ts b/src/wire/assetApi.ts index 737a6de..8f4db58 100644 --- a/src/wire/assetApi.ts +++ b/src/wire/assetApi.ts @@ -242,6 +242,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/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"]); + }); +}); From b48ace9f2dd4ec7bbc37095c313598a46b3364eb Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 11:00:26 +0000 Subject: [PATCH 2/4] Plan 001: tick B3 and B4 (k6i-agility-slack-bot#32, #34) Track B is done on the bot side. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016NSWj9gL1oCubj5e2anVkb --- plans/001-sdk-adoption-campaign.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plans/001-sdk-adoption-campaign.md b/plans/001-sdk-adoption-campaign.md index fae0bbe..bd575db 100644 --- a/plans/001-sdk-adoption-campaign.md +++ b/plans/001-sdk-adoption-campaign.md @@ -468,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 From c1dc18ed7ac26b488da452850d80fb6298af500a Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 11:02:29 +0000 Subject: [PATCH 3/4] Absent number roots live in the TypeCache; state 200 reads as Template From the slack bot's B4: - resolve.numbers remembered roots an instance lacks per transport, so a server building a client per user paid the failed-then-retry round trip on every lookup. With a TypeCache (the client passes its schema's) the cache's own record of a missing type (null) is that memory, shared by every client of the instance. Without one, it stays per transport. - AssetState 200 (a template) arrives as the code "200". ASSET_STATE_CODES gains Template: 200, and assetStateOf reads any spelling by name; rest-1 and resolve use it. Live: TK-01001 on v1sdktesting is "Template". Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016NSWj9gL1oCubj5e2anVkb --- src/client.ts | 4 ++-- src/index.ts | 2 +- src/services/resolve.ts | 33 ++++++++++++++++++++++----------- src/types/asset.ts | 18 ++++++++++++++++-- src/wire/rest1.ts | 5 ++--- test/unit/oid.test.ts | 11 +++++++++++ test/unit/resolve.test.ts | 17 +++++++++++++++++ 7 files changed, 71 insertions(+), 19 deletions(-) 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/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/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/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/); From 10e7cc52b7ee27df58d1a5bf98ba617ce5aa55c2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 11:03:29 +0000 Subject: [PATCH 4/4] Document two api/asset behaviours on the command types From the slack bot's live checks (2026-11-07): an update giving a multi-value relation a plain list adds its members rather than replacing the list, and Agility refuses to Delete a closed asset. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016NSWj9gL1oCubj5e2anVkb --- src/wire/assetApi.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/wire/assetApi.ts b/src/wire/assetApi.ts index 8f4db58..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;