diff --git a/docs/specs/agent-environments/state-writes/spec.md b/docs/specs/agent-environments/state-writes/spec.md index a2d757320a..e15d9a9689 100644 --- a/docs/specs/agent-environments/state-writes/spec.md +++ b/docs/specs/agent-environments/state-writes/spec.md @@ -185,12 +185,18 @@ or a list read's members as serving would list them (so a member the copy no longer holds fails the check) rendered into its line's base recording. It is compared with the recording, as the read gate compares, with volatile fields taking the observed values. A recorded 400, 404 or 410, or a 2xx with a JSON `null` -body, for an entity the copy does not hold agrees with it. +body, for an entity the copy does not hold agrees with it. A successful read +whose usable body no longer holds the entity or the list fails, as does one of +an entity lacking a field every recording of its view carried, which serving +refuses to answer with. **Attribution.** - A check exercises the write tools that changed its entity or collection - since the previous check of it. A pass confirms them and a mismatch + since the previous check of it, less those a later write replaced: a create + or delete replaces an entity's earlier writes, an update the earlier updates + whose fields it rewrites, and a write left with no changed entity in a + collection no longer counts there. A pass confirms them and a mismatch contradicts them. - A mismatch after an unmodelled write that followed those writes is checked only on the members they sent: such a write may explain the rest. @@ -260,7 +266,7 @@ recording instead. A create in a collection held under more than one spelling (`acme1` and `%61cme1`) goes to the one the call spells verbatim, as item calls choose their -entity. `data/add` refuses a document that `data/get` would write back past the +entity; a collection keeps its spelling after its last entity is deleted. `data/add` refuses a document that `data/get` would write back past the 32 MiB intake limit once it adds the members a document may omit, such as `types` and the state Evidence id. @@ -268,9 +274,10 @@ entity. `data/add` refuses a document that `data/get` would write back past the advances one second before each served write; clock fields take its value in their recorded format. -- **Sequences:** one more than both the largest current value in their scope - and the largest value they generated, so a deleted entity's value is not - reused, as database sequences do not reuse values. +- **Sequences:** one more than the largest current value in their scope, the + largest the state started with at its last reset or load, and the largest + value they generated, so a deleted entity's value is not reused, as database + sequences do not reuse values. - **UUIDs and tokens:** derived from a SHA-256 of the state Evidence id, the tool, the field and a write counter, skipping values the type already holds. diff --git a/docs/specs/agent-environments/state-writes/verification.md b/docs/specs/agent-environments/state-writes/verification.md index 6d827d6a43..4b5b1351b0 100644 --- a/docs/specs/agent-environments/state-writes/verification.md +++ b/docs/specs/agent-environments/state-writes/verification.md @@ -223,3 +223,15 @@ fails on the previous head. The gates passed again (domain and services 7475 with 5 pre-existing skips, boundaries 75, conformance, `jscpd` 3.46%), the verdicts over the 50 captures were unchanged plan by plan, and the smoke applied all 33 served write tools. + +A sixth round, after merging the serving and coverage branches and main +335c042d, settled five more findings: a read whose usable body no longer holds +the entity or list fails its check, an entity lacking a field its view always +carried confirms nothing, a write a later write replaced is not confirmed by +reads of the replacement, sequences never reuse a value the state started +with, and a collection keeps its spelling after it empties. Each new test fails +on the previous head. Over the 50 captures one capture's verdicts change: +Linkding's bundle create and patch, confirmed only by a list read after the +same bundle was deleted, are now unconfirmed, so 31 write tools are served (10 +creates, 11 updates, 10 deletes) and the in-process smoke applies all 31. The +gates passed again (domain and services, boundaries, conformance and `jscpd`). diff --git a/src/application/environment/environmentStateAnswer.ts b/src/application/environment/environmentStateAnswer.ts index 4e44dc58e5..14e95b90a1 100644 --- a/src/application/environment/environmentStateAnswer.ts +++ b/src/application/environment/environmentStateAnswer.ts @@ -3,6 +3,7 @@ import { collectionPathOf, listItems, pagesOnly, + missingRequiredFields, renderStateEntity, renderStateList, replaceAt, @@ -104,12 +105,9 @@ const shownEntity = ( entity: StateEntity, volatile: ReadonlySet, ): Result => { - const shown = renderStateEntity(entity, view, volatile); - const missing = view.required_fields.filter( - (name) => !Object.hasOwn(shown, name), - ); + const missing = missingRequiredFields(entity, view, volatile); return missing.length === 0 - ? ok(shown) + ? ok(renderStateEntity(entity, view, volatile)) : err( refused( tool, diff --git a/src/application/environment/environmentStateStore.ts b/src/application/environment/environmentStateStore.ts index 7e6539db3b..8505da5d6b 100644 --- a/src/application/environment/environmentStateStore.ts +++ b/src/application/environment/environmentStateStore.ts @@ -61,6 +61,10 @@ export class EnvironmentStateStore { #writes = 0; /** The largest value each sequence has generated since the last reset or load, so a deleted entity's value is not reused. */ readonly #sequences = new Map(); + /** The entities the state held when it was last reset or loaded, whose values no sequence reuses either. */ + #started: readonly StateEntity[] = []; + /** Every collection spelling a type's entities have been held under, kept when a collection empties. */ + readonly #knownCollections = new Map(); constructor(readonly state: EnvironmentState) { this.#types = new Map( @@ -128,6 +132,12 @@ export class EnvironmentStateStore { #restart(): void { this.#writes = 0; this.#sequences.clear(); + this.#started = this.#content.entities; + } + + /** The entities of a type the state held when it was last reset or loaded. */ + startedWith(type: string): readonly StateEntity[] { + return this.#started.filter((entity) => entity.type === type); } /** Every held entity of a type, in state order. */ @@ -262,6 +272,11 @@ export class EnvironmentStateStore { return this.#typeCollections.get(type) ?? []; } + /** Every collection spelling a type's entities have been held under, including ones that have since emptied. */ + knownCollectionsOf(type: string): readonly string[] { + return this.#knownCollections.get(type) ?? []; + } + document(tool: string): HeldDocument | undefined { return this.#documents.get(tool); } @@ -324,6 +339,12 @@ export class EnvironmentStateStore { this.#entities = entities; this.#collections = collections; this.#typeCollections = typeCollections; + for (const [type, named] of typeCollections) { + const known = this.#knownCollections.get(type) ?? []; + for (const collection of named) + if (!known.includes(collection)) known.push(collection); + this.#knownCollections.set(type, known); + } this.#documents = new Map( content.documents.map((document) => [document.tool, document]), ); diff --git a/src/application/environment/environmentStateWriteAnswer.test.ts b/src/application/environment/environmentStateWriteAnswer.test.ts index 678f764060..a3740d790f 100644 --- a/src/application/environment/environmentStateWriteAnswer.test.ts +++ b/src/application/environment/environmentStateWriteAnswer.test.ts @@ -489,3 +489,60 @@ describe("missing entities after a delete", () => { }); }); }); + +describe("values the state started with", () => { + it("never reuses a seeded id for a create, even after its entity is deleted first", () => { + const books = servedFrom(); + books.answer("delete_books_by_id", { id: "2" }); + books.answer("post_books", CREATE_NEW_BOOK); + expect(entityOf(books, "2")).toBeUndefined(); + expect(entityOf(books, "3")?.fields).toMatchObject({ name: "New Book" }); + }); + + it("creates in a collection's recorded spelling after the collection empties", () => { + const orgBook = (id: number, org: string, name: string): Book => ({ + id, + org, + name, + }); + const alpha = orgBook(1, "my org1", "Alpha"); + const beta = orgBook(2, "acme2", "Beta"); + const gamma = orgBook(3, "my org1", "Gamma"); + const delta = orgBook(4, "acme2", "Delta"); + const books = servedFrom([ + ["GET", "/api/orgs/my%20org1/books", [alpha]], + ["GET", "/api/orgs/acme2/books", [beta]], + ["GET", "/api/orgs/my%20org1/books/1", alpha], + ["GET", "/api/orgs/acme2/books/2", beta], + ["POST", "/api/orgs/my%20org1/books", gamma, 201, { name: "Gamma" }], + ["GET", "/api/orgs/my%20org1/books", [alpha, gamma]], + ["POST", "/api/orgs/acme2/books", delta, 201, { name: "Delta" }], + ["GET", "/api/orgs/acme2/books", [beta, delta]], + ]); + const { rea_state: held } = books.store.snapshot(); + // Load a state in which the encoded organisation holds no book. + expect( + books.store.replace({ + rea_state: { + version: held.version, + entities: held.entities.filter( + (entity) => !entity.collection.includes("%20"), + ), + documents: held.documents, + }, + }).ok, + ).toBe(true); + const tool = books.source.manifest.tools.find( + (candidate) => candidate.method === "POST", + )!; + const [org] = environmentToolArguments(tool).path.values(); + books.answer(tool.name, { [org!]: "my%20org1", body: { name: "Epsilon" } }); + expect( + books.store + .snapshot() + .rea_state.entities.find( + (entity) => entity.fields["name"] === "Epsilon", + )?.collection, + ).toBe("/api/orgs/my%20org1/books"); + }); +}); diff --git a/src/application/environment/environmentStateWriteAnswer.ts b/src/application/environment/environmentStateWriteAnswer.ts index e7c22f227f..1bea1b2cfe 100644 --- a/src/application/environment/environmentStateWriteAnswer.ts +++ b/src/application/environment/environmentStateWriteAnswer.ts @@ -159,6 +159,9 @@ const generator = ( return value; }; const ofType = store.ofType(request.plan.target ?? ""); + // A sequence never reuses a value the state started with, even after its entity was deleted. + const started = store.startedWith(request.plan.target ?? ""); + const startedIn = started.filter((held) => held.collection === target.name); const unused = (field: string, make: (attempt: number) => string): string => { const held = new Set( ofType.map((entity) => scalarText(entity.fields[field])), @@ -182,7 +185,9 @@ const generator = ( sequence( `${request.tool.name} ${rule.field} ${rule.scope === "collection" ? target.name : ""}`, nextInteger( - rule.scope === "collection" ? target.collection : ofType, + rule.scope === "collection" + ? [...target.collection, ...startedIn] + : [...ofType, ...started], rule.field, ), ), @@ -197,7 +202,7 @@ const generator = ( exemplar, sequence( `${request.plan.target ?? ""} ${rule.field} ${target.name}`, - nextInteger(target.collection, rule.field), + nextInteger([...target.collection, ...startedIn], rule.field), ), ); return unused(rule.field, (attempt) => @@ -241,10 +246,10 @@ const createdCollection = ( ): string | undefined => { const segments = callSegments(request.tool, request.args); if (segments === undefined) return undefined; - // The spelling the call supplies verbatim wins, as item calls choose their entity. + // The spelling the call supplies verbatim wins, as item calls choose their entity; an emptied collection keeps its spelling. let held: string | undefined; let verbatim = -1; - for (const collection of serving.store.collectionsOf( + for (const collection of serving.store.knownCollectionsOf( request.plan.target ?? "", )) { const matched = collectionMatch(collection, segments); diff --git a/src/domain/analysisView/EnvironmentStateWriteSimulation.test.ts b/src/domain/analysisView/EnvironmentStateWriteSimulation.test.ts index 6ed155b780..67a846556b 100644 --- a/src/domain/analysisView/EnvironmentStateWriteSimulation.test.ts +++ b/src/domain/analysisView/EnvironmentStateWriteSimulation.test.ts @@ -357,3 +357,53 @@ describe("insert rules", () => { }); }); }); + +describe("reads the served answer no longer matches", () => { + it("contradicts an update when a later read's body no longer holds the entity", () => { + const rows: TimedCaptureRow[] = [ + ["GET", "/api/books", [FIRST_BOOK, SECOND_BOOK]], + ["GET", "/api/books/1", { data: FIRST_BOOK }], + ["GET", "/api/books/2", { data: SECOND_BOOK }], + ["PATCH", "/api/books/1", RENAMED, 200, { name: "Renamed Book" }], + ["GET", "/api/books/1", { data: null }], + ["GET", "/api/books/1", { data: RENAMED }], + ]; + // Serving still answers the renamed book, so the read that shows none is a check it fails, not one it skips. + expect(gated(rows, "patch_books_by_id")).toMatchObject({ + served: false, + reason: "contradicted", + contradicted_by: 4, + }); + }); + + it("confirms no create by the absence of the entity a later delete removed", () => { + const rows: TimedCaptureRow[] = [ + ...BOOK_INIT_ROWS, + ["POST", "/api/books", THIRD_BOOK, 201, { name: "Third Book" }], + ["DELETE", "/api/books/3", "", 204], + ["GET", "/api/books/3", BOOK_NOT_FOUND, 404], + ["GET", "/api/books", [FIRST_BOOK, SECOND_BOOK]], + ]; + expect(gated(rows, "post_books")).toMatchObject({ + served: false, + reason: "unconfirmed", + checked_record_ordinals: [], + }); + expect(gated(rows, "delete_books_by_id")).toMatchObject({ + served: true, + checked_record_ordinals: [5, 6], + }); + }); + + it("confirms no create whose entity lacks a field every recording of its item view carried", () => { + const detailed = (entry: Book): Book => ({ ...entry, detail: "x" }); + const rows: TimedCaptureRow[] = [ + ["GET", "/api/books", [FIRST_BOOK, SECOND_BOOK]], + ["GET", "/api/books/1", detailed(FIRST_BOOK)], + ["GET", "/api/books/2", detailed(SECOND_BOOK)], + ["POST", "/api/books", THIRD_BOOK, 201, { name: "Third Book" }], + ["GET", "/api/books/3", THIRD_BOOK], + ]; + expect(gated(rows, "post_books")).toMatchObject({ served: false }); + }); +}); diff --git a/src/domain/analysisView/EnvironmentStateWriteSimulation.ts b/src/domain/analysisView/EnvironmentStateWriteSimulation.ts index c2755a5d4f..bb443e1a68 100644 --- a/src/domain/analysisView/EnvironmentStateWriteSimulation.ts +++ b/src/domain/analysisView/EnvironmentStateWriteSimulation.ts @@ -24,11 +24,14 @@ import { isAbsence, servedLine, succeeded, + supersede, withBodies, writtenFieldChecks, SimulatedState, type CheckOutcome, type CheckedLines, + type WrittenEntities, + type TouchedTargets, type ServedLine, } from "./environmentStateWriteChecks.js"; import { applyCreate } from "./environmentStateWriteCreate.js"; @@ -86,8 +89,8 @@ export class EnvironmentStateWriteSimulation { readonly #checkedLines: ReadonlyMap; readonly #observations = new Set(); readonly #taint: WriteTaint; - /** Modelled writes per target since its last check, with the fields each wrote (null: all). */ - readonly #touched = new Map>(); + readonly #touched: TouchedTargets = new Map(); + readonly #writtenSince: WrittenEntities = new Map(); readonly #lastCheck = new Map(); /** The earliest modelled write to each target since its last check. */ readonly #touchedSince = new Map(); @@ -227,6 +230,8 @@ export class EnvironmentStateWriteSimulation { result.entity.collection, result.entity.id, ); + if (result.kind === "applied" && result.entity !== undefined) + supersede(this.#touched, this.#writtenSince, write.tool, result); for (const target of result.targets) { // A recorded outcome replaces the entity's values, not its collection's membership. if (result.kind === "observed") { @@ -372,6 +377,7 @@ export class EnvironmentStateWriteSimulation { if (!pass && view.shape === "item") this.#adopt(view, record); for (const key of keys) { this.#touched.delete(key); + this.#writtenSince.delete(key); this.#touchedSince.delete(key); this.#lastCheck.set(key, index); } diff --git a/src/domain/analysisView/environmentStateRules.ts b/src/domain/analysisView/environmentStateRules.ts index 160c117c7b..cb0a05bc5d 100644 --- a/src/domain/analysisView/environmentStateRules.ts +++ b/src/domain/analysisView/environmentStateRules.ts @@ -535,6 +535,16 @@ export const renderStateEntity = ( ); }; +/** The fields every recording of a view carried that the entity, as serving renders it, lacks; serving refuses to answer with such an entity. */ +export const missingRequiredFields = ( + entity: StateEntity, + view: Pick, + volatileFields: ReadonlySet, +): string[] => { + const shown = renderStateEntity(entity, view, volatileFields); + return view.required_fields.filter((name) => !Object.hasOwn(shown, name)); +}; + /** A list response rendered from `base`: its array replaced by `items` and its counters set to their number. */ export const renderStateList = ( view: Pick, diff --git a/src/domain/analysisView/environmentStateWriteChecks.ts b/src/domain/analysisView/environmentStateWriteChecks.ts index ea26974b24..489e8556b2 100644 --- a/src/domain/analysisView/environmentStateWriteChecks.ts +++ b/src/domain/analysisView/environmentStateWriteChecks.ts @@ -12,6 +12,7 @@ import { isParam, lastSegmentValue, listItems, + missingRequiredFields, renderStateEntity, renderStateList, replaceAt, @@ -125,6 +126,66 @@ export const servedLine = ( return best?.line; }; +/** Modelled writes per target since its last check, with the fields each wrote (null: all). */ +export type TouchedTargets = Map>; +/** Per collection target, the entities each write changed there since its last check, with the fields it wrote (null: all). */ +export type WrittenEntities = Map< + string, + Map> +>; + +/** Whether a write replaces an earlier one's effect: a create or delete replaces every field, an update those it rewrites. */ +const replaces = ( + write: { + readonly whole: boolean; + readonly fields: readonly string[] | undefined; + }, + earlier: readonly string[] | null | undefined, +): boolean => + earlier !== undefined && + (write.whole || + (earlier !== null && + earlier.every((field) => write.fields?.includes(field) === true))); + +/** + * Drop the writes whose effect on an entity a later write replaces, so a read checks only writes whose effect it can + * see: from the entity's target, and from its collection's, where a write that has no entity left loses its claim. + */ +export const supersede = ( + touched: TouchedTargets, + written: WrittenEntities, + writer: string, + effect: { + readonly targets: readonly string[]; + readonly fields?: readonly string[]; + }, +): void => { + const [entity, collection] = effect.targets; + const write = { whole: effect.fields === undefined, fields: effect.fields }; + const own = touched.get(entity!); + for (const [tool, fields] of own ?? []) + if (tool !== writer && replaces(write, fields)) own!.delete(tool); + if (collection === undefined) return; + const byTool = written.get(collection) ?? new Map(); + written.set(collection, byTool); + for (const [tool, entities] of byTool) { + if (tool === writer || !replaces(write, entities.get(entity!))) continue; + entities.delete(entity!); + if (entities.size > 0) continue; + byTool.delete(tool); + touched.get(collection)?.delete(tool); + } + const entities = byTool.get(writer) ?? new Map(); + const before = entities.get(entity!); + entities.set( + entity!, + write.whole || before === null + ? null + : [...new Set([...(before ?? []), ...(write.fields ?? [])])], + ); + byTool.set(writer, entities); +}; + export const entityTarget = (type: string, collection: string, id: string) => `e${compositeKey([type, collection, id])}`; export const collectionTarget = (type: string, collection: string) => @@ -250,8 +311,15 @@ export const checkItemRead = ( if (!succeeded(record) || check.base === undefined) return undefined; const object = check.body === undefined ? undefined : at(check.body, view.pointer); - if (!isObject(object)) return undefined; + // A usable body that no longer holds the entity is not what serving answers for one it holds. + if (!isObject(object)) + return check.body === undefined || entity === undefined + ? undefined + : { pass: false, targets, unheld: [] }; if (entity === undefined) return { pass: false, targets, unheld: targets }; + // Serving refuses an entity lacking a field every recording carried, so no read can confirm one. + if (missingRequiredFields(entity, view, check.volatile).length > 0) + return { pass: false, targets, unheld: [] }; const shown = renderStateEntity(entity, view, check.volatile, object); return { pass: same(replaceAt(check.base, view.pointer, shown), check.body), @@ -278,6 +346,13 @@ const listCheck = ( check: ListBodies, members: readonly StateEntity[], ): boolean => { + if ( + members.some( + (entity) => + missingRequiredFields(entity, view, check.volatile).length > 0, + ) + ) + return false; const observed = new Map( check.items.map((item) => [scalarText(item[check.idField]) ?? "", item]), ); @@ -342,20 +417,18 @@ export const checkListRead = ( ): CheckOutcome | undefined => { if (!succeeded(record)) return undefined; const { body, base } = check; - const items = body === undefined ? undefined : listItems(body, view.pointer); const baseItems = base === undefined ? undefined : listItems(base, view.pointer); - if ( - body === undefined || - base === undefined || - items === undefined || - baseItems === undefined - ) + if (body === undefined || base === undefined || baseItems === undefined) return undefined; - const bodies = { ...check, body, base, items, baseItems }; - return view.membership === "all" - ? checkAllList(state, view, bodies) - : checkRecordedList(state, view, bodies); + // A usable body that no longer holds the list is not what serving answers on this line. + const items = listItems(body, view.pointer); + const bodies = { ...check, body, base, items: items ?? [], baseItems }; + const outcome = + view.membership === "all" + ? checkAllList(state, view, bodies) + : checkRecordedList(state, view, bodies); + return items === undefined ? { ...outcome, pass: false } : outcome; }; /** A document read on its checked line must equal the held document. */ diff --git a/src/domain/analysisView/environmentStateWrites.test.ts b/src/domain/analysisView/environmentStateWrites.test.ts index 1a19e968cb..dd852e2d8b 100644 --- a/src/domain/analysisView/environmentStateWrites.test.ts +++ b/src/domain/analysisView/environmentStateWrites.test.ts @@ -98,7 +98,8 @@ describe("served write plans", () => { collection: null, insert: { rule: "sorted", field: "id", direction: "ascending" }, merge: null, - checked_record_ordinals: [4, 9], + // The final list follows the delete of book 3, so it shows nothing of the create. + checked_record_ordinals: [4], contradicted_by: null, }); // PATCH requires no member; the slug follows the sent name and the update stamps its clock.