diff --git a/docs/specs/agent-environments/state-writes/spec.md b/docs/specs/agent-environments/state-writes/spec.md index e15d9a9689..d0350e41f2 100644 --- a/docs/specs/agent-environments/state-writes/spec.md +++ b/docs/specs/agent-environments/state-writes/spec.md @@ -195,8 +195,9 @@ refuses to answer with. - A check exercises the write tools that changed its entity or collection 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 + whose every changed field, sent or derived, it overwrites (an increment adds + to its field rather than overwriting it), 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. @@ -266,7 +267,8 @@ 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; a collection keeps its spelling after its last entity is deleted. `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, until +a reset or load starts from the spellings of the state it installs. `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. diff --git a/docs/specs/agent-environments/state-writes/verification.md b/docs/specs/agent-environments/state-writes/verification.md index 4b5b1351b0..1aaa975381 100644 --- a/docs/specs/agent-environments/state-writes/verification.md +++ b/docs/specs/agent-environments/state-writes/verification.md @@ -235,3 +235,12 @@ 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`). + +A seventh round settled two more findings. An update keeps its confirmation +when a later write overwrites only part of what it changed: a derived change, +such as an incremented version, counts as part of the update's effect, and a +later increment adds to the field rather than replacing it. A reset or load +forgets the collection spellings of the state it replaced. Both new tests fail +on the previous head. The verdicts over the 50 captures are unchanged plan by +plan, the in-process smoke applies all 31 served write tools, and the gates +passed again. diff --git a/src/application/environment/environmentStateStore.ts b/src/application/environment/environmentStateStore.ts index 8505da5d6b..dce3dae2a9 100644 --- a/src/application/environment/environmentStateStore.ts +++ b/src/application/environment/environmentStateStore.ts @@ -63,7 +63,7 @@ export class EnvironmentStateStore { 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. */ + /** Every collection spelling a type's entities have been held under since the last reset or load, kept when a collection empties. */ readonly #knownCollections = new Map(); constructor(readonly state: EnvironmentState) { @@ -133,6 +133,10 @@ export class EnvironmentStateStore { this.#writes = 0; this.#sequences.clear(); this.#started = this.#content.entities; + // A reset or load starts as a fresh store would, without the spellings of the state it replaced. + this.#knownCollections.clear(); + for (const [type, named] of this.#typeCollections) + this.#knownCollections.set(type, [...named]); } /** The entities of a type the state held when it was last reset or loaded. */ @@ -272,7 +276,7 @@ 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. */ + /** Every collection spelling a type's entities have been held under since the last reset or load, including ones that have since emptied. */ knownCollectionsOf(type: string): readonly string[] { return this.#knownCollections.get(type) ?? []; } diff --git a/src/application/environment/environmentStateWriteAnswer.test.ts b/src/application/environment/environmentStateWriteAnswer.test.ts index a3740d790f..58df89e182 100644 --- a/src/application/environment/environmentStateWriteAnswer.test.ts +++ b/src/application/environment/environmentStateWriteAnswer.test.ts @@ -499,50 +499,80 @@ describe("values the state started with", () => { 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 orgBook = (id: number, org: string, name: string): Book => ({ + id, + org, + name, + }); + /** Books under two organisations, one spelled `first` in its paths, with creates and a delete the lists confirm. */ + const twoOrgs = (first: string, org: string) => { + const alpha = orgBook(1, org, "Alpha"); const beta = orgBook(2, "acme2", "Beta"); - const gamma = orgBook(3, "my org1", "Gamma"); + const gamma = orgBook(3, org, "Gamma"); const delta = orgBook(4, "acme2", "Delta"); const books = servedFrom([ - ["GET", "/api/orgs/my%20org1/books", [alpha]], + ["GET", `/api/orgs/${first}/books`, [alpha]], ["GET", "/api/orgs/acme2/books", [beta]], - ["GET", "/api/orgs/my%20org1/books/1", alpha], + ["GET", `/api/orgs/${first}/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/${first}/books`, gamma, 201, { name: "Gamma" }], + ["GET", `/api/orgs/${first}/books`, [alpha, gamma]], ["POST", "/api/orgs/acme2/books", delta, 201, { name: "Delta" }], ["GET", "/api/orgs/acme2/books", [beta, delta]], + ["DELETE", `/api/orgs/${first}/books/3`, "", 204], + ["GET", `/api/orgs/${first}/books`, [alpha]], ]); + const toolOf = (method: string) => + books.source.manifest.tools.find( + (candidate) => candidate.method === method, + )!; + const create = (value: string, name: string): string | undefined => { + const tool = toolOf("POST"); + const [parent] = environmentToolArguments(tool).path.values(); + books.answer(tool.name, { [parent!]: value, body: { name } }); + return books.store + .snapshot() + .rea_state.entities.find((entity) => entity.fields["name"] === name) + ?.collection; + }; + return { books, toolOf, create }; + }; + + it("creates in a collection's recorded spelling after the collection empties", () => { + const { books, toolOf, create } = twoOrgs("my%20org1", "my org1"); + const remove = toolOf("DELETE"); + const [parent, id] = environmentToolArguments(remove).path.values(); + for (const held of ["1", "3"]) + books.answer(remove.name, { [parent!]: "my%20org1", [id!]: held }); + expect(books.store.collectionsOf("/api/orgs/{org}/books")).not.toContain( + "/api/orgs/my%20org1/books", + ); + expect(create("my%20org1", "Epsilon")).toBe("/api/orgs/my%20org1/books"); + }); + + it("forgets the spellings of a loaded state once reset", () => { + const { books, create } = twoOrgs("%61cme1", "acme1"); const { rea_state: held } = books.store.snapshot(); - // Load a state in which the encoded organisation holds no book. + // A loaded state spells the first organisation plainly; the Evidence's state spells it encoded. expect( books.store.replace({ rea_state: { version: held.version, - entities: held.entities.filter( - (entity) => !entity.collection.includes("%20"), + // Moved to another spelling, the entity cites none of the Evidence's records. + entities: held.entities.map((entity) => + entity.collection.includes("%61cme1") + ? { + ...entity, + collection: entity.collection.replace("%61cme1", "acme1"), + record_ordinals: [], + } + : entity, ), 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"); + books.store.reset(); + expect(create("acme1", "Epsilon")).toBe("/api/orgs/%61cme1/books"); }); }); diff --git a/src/domain/analysisView/EnvironmentStateWriteSimulation.test.ts b/src/domain/analysisView/EnvironmentStateWriteSimulation.test.ts index 67a846556b..a5701732b9 100644 --- a/src/domain/analysisView/EnvironmentStateWriteSimulation.test.ts +++ b/src/domain/analysisView/EnvironmentStateWriteSimulation.test.ts @@ -406,4 +406,35 @@ describe("reads the served answer no longer matches", () => { ]; expect(gated(rows, "post_books")).toMatchObject({ served: false }); }); + + it("keeps an update confirmed when a later one rewrites only part of what it changed", () => { + const versioned = (entry: Book, version: number): Book => ({ + ...entry, + version, + }); + const again = versioned(renamedBook(FIRST_BOOK, "Again Book", 4), 2); + const rows: TimedCaptureRow[] = [ + [ + "GET", + "/api/books", + [versioned(FIRST_BOOK, 1), versioned(SECOND_BOOK, 1)], + ], + ["GET", "/api/books/1", versioned(FIRST_BOOK, 1)], + ["GET", "/api/books/2", versioned(SECOND_BOOK, 1)], + [ + "PATCH", + "/api/books/1", + versioned(RENAMED, 2), + 200, + { name: "Renamed Book" }, + ], + ["PUT", "/api/books/1", again, 200, { name: "Again Book" }], + ["GET", "/api/books/1", again], + ]; + // The PUT keeps the version the PATCH incremented, so the read still shows the PATCH's effect. + expect(gated(rows, "patch_books_by_id")).toMatchObject({ + served: true, + checked_record_ordinals: [5], + }); + }); }); diff --git a/src/domain/analysisView/EnvironmentStateWriteSimulation.ts b/src/domain/analysisView/EnvironmentStateWriteSimulation.ts index bb443e1a68..8fb4f2a2e7 100644 --- a/src/domain/analysisView/EnvironmentStateWriteSimulation.ts +++ b/src/domain/analysisView/EnvironmentStateWriteSimulation.ts @@ -24,13 +24,12 @@ import { isAbsence, servedLine, succeeded, - supersede, withBodies, writtenFieldChecks, SimulatedState, + WriteEffects, type CheckOutcome, type CheckedLines, - type WrittenEntities, type TouchedTargets, type ServedLine, } from "./environmentStateWriteChecks.js"; @@ -90,7 +89,7 @@ export class EnvironmentStateWriteSimulation { readonly #observations = new Set(); readonly #taint: WriteTaint; readonly #touched: TouchedTargets = new Map(); - readonly #writtenSince: WrittenEntities = new Map(); + readonly #effects = new WriteEffects(); readonly #lastCheck = new Map(); /** The earliest modelled write to each target since its last check. */ readonly #touchedSince = new Map(); @@ -231,7 +230,7 @@ export class EnvironmentStateWriteSimulation { result.entity.id, ); if (result.kind === "applied" && result.entity !== undefined) - supersede(this.#touched, this.#writtenSince, write.tool, result); + this.#effects.supersede(this.#touched, 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") { @@ -377,7 +376,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.#effects.checked(key); this.#touchedSince.delete(key); this.#lastCheck.set(key, index); } diff --git a/src/domain/analysisView/environmentStateWriteChecks.ts b/src/domain/analysisView/environmentStateWriteChecks.ts index 489e8556b2..d56689671f 100644 --- a/src/domain/analysisView/environmentStateWriteChecks.ts +++ b/src/domain/analysisView/environmentStateWriteChecks.ts @@ -128,63 +128,98 @@ export const servedLine = ( /** 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. */ +/** The fields a write changed on an entity: a create or delete all of them (null), an update those it sent or derived. */ +type ChangedFields = readonly string[] | null; + +/** Whether a later write replaces an earlier one's effect: it changes the whole entity, or overwrites every field the earlier changed (an increment adds to its field instead). */ const replaces = ( - write: { - readonly whole: boolean; - readonly fields: readonly string[] | undefined; - }, - earlier: readonly string[] | null | undefined, + write: { readonly whole: boolean; readonly overwrites: ReadonlySet }, + earlier: ChangedFields | undefined, ): boolean => earlier !== undefined && (write.whole || (earlier !== null && - earlier.every((field) => write.fields?.includes(field) === true))); + earlier.every((field) => write.overwrites.has(field)))); + +/** Add a write's changed fields under a key: all of them once any write there changed the whole entity. */ +const record = ( + map: Map, + key: string, + whole: boolean, + changed: readonly string[], +): Map => { + const before = map.get(key); + return map.set( + key, + whole || before === null + ? null + : [...new Set([...(before ?? []), ...changed])], + ); +}; /** - * 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. + * What each write changed since the targets it touched were last checked, so a later write that replaces its effect + * drops it: from the entity's target, and from its collection's once no entity it changed there is left. */ -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); +export class WriteEffects { + /** Per entity target, the fields each tool changed there. */ + readonly #changed = new Map>(); + /** Per collection target, the entities each tool's claim there rests on, with the fields it changed on each. */ + readonly #members = new Map< + string, + Map> + >(); + + /** Forget a checked target's writes, as the touched targets do. */ + checked(key: string): void { + this.#changed.delete(key); + this.#members.delete(key); } - 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); -}; + + supersede( + touched: TouchedTargets, + writer: string, + effect: { + readonly targets: readonly string[]; + readonly fields?: readonly string[]; + readonly changed?: { + readonly fields: readonly string[]; + readonly accumulated: readonly string[]; + }; + }, + ): void { + const [entity, collection] = effect.targets as [string, string?]; + const changed = effect.changed?.fields ?? effect.fields ?? []; + const write = { + whole: effect.fields === undefined, + overwrites: new Set( + changed.filter((field) => !effect.changed?.accumulated.includes(field)), + ), + }; + const own = this.#changed.get(entity) ?? new Map(); + for (const [tool, fields] of own) + if (tool !== writer && replaces(write, fields)) { + own.delete(tool); + touched.get(entity)?.delete(tool); + } + this.#changed.set(entity, record(own, writer, write.whole, changed)); + if (collection === undefined) return; + const byTool = this.#members.get(collection) ?? new Map(); + 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); + } + byTool.set( + writer, + record(byTool.get(writer) ?? new Map(), entity, write.whole, changed), + ); + this.#members.set(collection, byTool); + } +} export const entityTarget = (type: string, collection: string, id: string) => `e${compositeKey([type, collection, id])}`; diff --git a/src/domain/analysisView/environmentStateWriteEffects.ts b/src/domain/analysisView/environmentStateWriteEffects.ts index 66c0853f8c..7538f0d0ad 100644 --- a/src/domain/analysisView/environmentStateWriteEffects.ts +++ b/src/domain/analysisView/environmentStateWriteEffects.ts @@ -112,6 +112,11 @@ export type EffectResult = readonly entity?: WrittenEntity; /** The fields an update wrote; a create writes the whole entity. */ readonly fields?: readonly string[]; + /** The fields an update sent or changed, derived ones included, and those of them its increments add to. */ + readonly changed?: { + readonly fields: readonly string[]; + readonly accumulated: readonly string[]; + }; readonly inserted?: string; } | { @@ -406,7 +411,16 @@ export const applyUpdate = ( const sent = Object.keys(body).filter( (member) => member !== draft.plan.body?.identity, ); - return { kind: "applied", targets, entity: target, fields: sent }; + const derived = Object.keys(written.value).filter( + (field) => !same(written.value[field], entity.fields[field]), + ); + const changed = { + fields: [...new Set([...sent, ...derived])], + accumulated: draft.plan.rules.flatMap((rule) => + rule.rule === "increment" ? [rule.field] : [], + ), + }; + return { kind: "applied", targets, entity: target, fields: sent, changed }; }; /** Apply a recorded delete: the held entity leaves its collection. */