diff --git a/docs/specs/agent-environments/state-writes/spec.md b/docs/specs/agent-environments/state-writes/spec.md index 70ed04dd57..a2d757320a 100644 --- a/docs/specs/agent-environments/state-writes/spec.md +++ b/docs/specs/agent-environments/state-writes/spec.md @@ -120,9 +120,10 @@ response, with the entity re-rendered when the response carries it. **Documents.** A document write merges the body's members (replaces them, for `PUT`) into the document object that holds most of them: the root or one member. Document reads answer only on their recorded line, so a write is -served only at the path the state holds the document at. A recorded write -whose body serving would refuse (a member of another JSON type than the -document holds) is `unexplained-change`. +served only at the path the state holds the document at, and the gate treats a +recorded write at another path as an unmodelled write. A recorded write whose +body serving would refuse (a member of another JSON type than the document +holds) is `unexplained-change`. **Responses.** A write answers with the status and content type of one recorded response: @@ -257,6 +258,12 @@ When the tool, or else an item view of the type, recorded a 400, 404 or 410 response, an update or delete of a missing entity answers with the latest such 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 +32 MiB intake limit once it adds the members a document may omit, such as +`types` and the state Evidence id. + **Clock and generators.** The store's clock starts at `write_clock` and advances one second before each served write; clock fields take its value in their recorded format. @@ -271,8 +278,9 @@ their recorded format. sequences. **Missing entities.** An item read of an entity the state does not hold -answers with the tool's latest recorded 400, 404 or 410 response when it has -one, labelled `state.view: "missing"`. Otherwise it stays a tool error. +answers with the tool's latest recorded 400, 404 or 410 response, or 2xx JSON +`null` response, the absences the gate accepts, when it has one, labelled +`state.view: "missing"`. Otherwise it stays a tool error. **Lists of a whole collection.** Written entities matter only if lists show them, so the read gate's `membership: "all"` also covers a line that only pages diff --git a/docs/specs/agent-environments/state-writes/verification.md b/docs/specs/agent-environments/state-writes/verification.md index 55307df97b..6d827d6a43 100644 --- a/docs/specs/agent-environments/state-writes/verification.md +++ b/docs/specs/agent-environments/state-writes/verification.md @@ -213,3 +213,13 @@ names; both new tests fail on the previous head. A fourth round chose a read's line as replay does, so a read spelled plainly is checked against the encoded line that answers it, and its list against that line's collection; its test fails on the previous head. + +A review of the fidelity branch built on this one found four more places: a +document write at another path is now unmodelled in the gate, a create picks +the collection the call spells verbatim, a missing entity is answered with a +recorded 2xx `null` the gate accepted as an absence, and `data/add` refuses a +document whose `data/get` form would exceed the intake limit. Each new test +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. diff --git a/src/application/environment/environmentStateAnswer.ts b/src/application/environment/environmentStateAnswer.ts index 22b1f64196..b99379f58c 100644 --- a/src/application/environment/environmentStateAnswer.ts +++ b/src/application/environment/environmentStateAnswer.ts @@ -264,12 +264,18 @@ export const missingAnswer = ( readonly id: string; }, ): Result => { + // The write gate takes a 2xx JSON null as an absence too, so a delete it confirmed that way can be answered. const latestAbsence = (ordinals: readonly number[]) => ordinals .map((ordinal) => serving.source.exchanges.get(ordinal)) .filter( (exchange): exchange is RecordedExchange => - exchange !== undefined && isAbsence(exchange.status), + exchange !== undefined && + (isAbsence(exchange.status) || + (exchange.status !== null && + exchange.status >= 200 && + exchange.status < 300 && + recordedJson(exchange) === null)), ) .at(-1); // The tool's own recorded absence first, else that of an item view of the type. diff --git a/src/application/environment/environmentStateStore.ts b/src/application/environment/environmentStateStore.ts index 70938c7e3e..7e6539db3b 100644 --- a/src/application/environment/environmentStateStore.ts +++ b/src/application/environment/environmentStateStore.ts @@ -3,8 +3,8 @@ import type { StateType, } from "../../domain/analysisView/environmentStateRules.js"; import { + AnalysisInputError, AnalysisResourceConstraintError, - type AnalysisInputError, } from "../../domain/analysisErrorCore.js"; import type { JsonValue } from "../../domain/jsonValue.js"; import { err, ok, type Result } from "../../domain/result.js"; @@ -83,7 +83,26 @@ export class EnvironmentStateStore { replace(document: unknown): Result { const content = parseStateDocument(this.state, document); if (!content.ok) return content; - this.#install(content.value, this.#contentBytes(content.value)); + // data/get writes members a document may omit, so the state it reports must still be one data/add loads. + const bytes = this.#contentBytes(content.value); + if (bytes > STATE_INTAKE_LIMIT_BYTES) + return err( + new AnalysisInputError( + ENVIRONMENT_SERVE_OPERATION, + { + userMessage: `The state would exceed ${String(STATE_INTAKE_LIMIT_BYTES)} bytes as data/get writes it.`, + }, + [ + { + path: [], + reason: "out_of_range", + message: `would exceed ${String(STATE_INTAKE_LIMIT_BYTES)} bytes as data/get writes it`, + maximum: STATE_INTAKE_LIMIT_BYTES, + }, + ], + ), + ); + this.#install(content.value, bytes); this.#restart(); return ok(undefined); } diff --git a/src/application/environment/environmentStateWriteAnswer.test.ts b/src/application/environment/environmentStateWriteAnswer.test.ts index a2cf59c0b9..b74b86402a 100644 --- a/src/application/environment/environmentStateWriteAnswer.test.ts +++ b/src/application/environment/environmentStateWriteAnswer.test.ts @@ -408,3 +408,68 @@ describe("path values", () => { } }); }); + +describe("collections a create chooses", () => { + const orgBook = (id: number, name: string): Book => ({ id, name }); + /** One organisation listed under two spellings and another organisation, each with a create. */ + const ROWS: readonly TimedCaptureRow[] = [ + ["GET", "/api/orgs/%61cme1/books", [orgBook(1, "Alpha")]], + ["GET", "/api/orgs/acme1/books", [orgBook(2, "Beta")]], + ["GET", "/api/orgs/beta2/books", [orgBook(3, "Gamma")]], + [ + "POST", + "/api/orgs/acme1/books", + orgBook(4, "Delta"), + 201, + { name: "Delta" }, + ], + ["GET", "/api/orgs/acme1/books", [orgBook(2, "Beta"), orgBook(4, "Delta")]], + [ + "POST", + "/api/orgs/beta2/books", + orgBook(5, "Epsilon"), + 201, + { name: "Epsilon" }, + ], + [ + "GET", + "/api/orgs/beta2/books", + [orgBook(3, "Gamma"), orgBook(5, "Epsilon")], + ], + ]; + + it("creates in the collection the call spells verbatim, as item calls choose their entity", () => { + const books = servedFrom(ROWS); + const tool = books.source.manifest.tools.find( + (candidate) => candidate.method === "POST", + )!; + expect(books.state.writes.get(tool.name)).toMatchObject({ served: true }); + const [org] = environmentToolArguments(tool).path.values(); + books.answer(tool.name, { [org!]: "acme1", body: { name: "Zeta" } }); + expect( + books.store + .snapshot() + .rea_state.entities.find((entity) => entity.fields["name"] === "Zeta") + ?.collection, + ).toBe("/api/orgs/acme1/books"); + }); +}); + +describe("missing entities after a delete", () => { + it("answers with a recorded 2xx null, which the gate also took as an absence", () => { + const books = servedFrom([ + ...BOOK_INIT_ROWS, + ["DELETE", "/api/books/2", "", 204], + ["GET", "/api/books/2", null], + ]); + expect(books.state.writes.get("delete_books_by_id")).toMatchObject({ + served: true, + }); + books.answer("delete_books_by_id", { id: "1" }); + expect(books.answer("get_books_by_id", { id: "1" })).toMatchObject({ + status: 200, + body: null, + state: { view: "missing", id: "1" }, + }); + }); +}); diff --git a/src/application/environment/environmentStateWriteAnswer.ts b/src/application/environment/environmentStateWriteAnswer.ts index 72cf4824d7..e7c22f227f 100644 --- a/src/application/environment/environmentStateWriteAnswer.ts +++ b/src/application/environment/environmentStateWriteAnswer.ts @@ -241,9 +241,18 @@ const createdCollection = ( ): string | undefined => { const segments = callSegments(request.tool, request.args); if (segments === undefined) return undefined; - const held = serving.store - .collectionsOf(request.plan.target ?? "") - .find((collection) => collectionMatch(collection, segments) !== undefined); + // The spelling the call supplies verbatim wins, as item calls choose their entity. + let held: string | undefined; + let verbatim = -1; + for (const collection of serving.store.collectionsOf( + request.plan.target ?? "", + )) { + const matched = collectionMatch(collection, segments); + if (matched !== undefined && matched > verbatim) { + held = collection; + verbatim = matched; + } + } return ( held ?? `/${segments.map(({ text, supplied }) => (supplied ? encodeURIComponent(text) : text)).join("/")}` diff --git a/src/application/environment/environmentStateWriteRefusals.test.ts b/src/application/environment/environmentStateWriteRefusals.test.ts index 5fedbadc0f..c989c6777c 100644 --- a/src/application/environment/environmentStateWriteRefusals.test.ts +++ b/src/application/environment/environmentStateWriteRefusals.test.ts @@ -95,6 +95,17 @@ describe("served document writes", () => { }); }); + it("does not confirm a document write at another path, which serving refuses", () => { + const settings = servedFrom([ + ["GET", "/api/orgs/1/settings", DARK], + ["PATCH", "/api/orgs/2/settings", LIGHT, 200, LIGHT], + ["GET", "/api/orgs/1/settings", LIGHT], + ]); + expect(settings.state.writes.get(PATCH_SETTINGS)).toMatchObject({ + served: false, + }); + }); + it("changes the document only at the path the state holds it", () => { const settings = servedFrom(SETTINGS_ROWS); expectRefusedUnchanged(settings, [ @@ -219,4 +230,40 @@ describe("the state size limit", () => { books.answer("delete_books_by_id", { id: "3" }); expect(books.answer("post_books", create).status).toBe(201); }); + + it("refuses a document that data/get would write past what data/add loads", () => { + const books = servedFrom(); + const { rea_state: state } = books.store.snapshot(); + // Without the members data/get adds back, the document fits only until they return. + const document = { + rea_state: { + version: state.version, + entities: state.entities.map(({ type, collection, id, fields }) => ({ + type, + collection, + id, + fields: { ...fields }, + })), + documents: state.documents, + }, + }; + const first = document.rea_state.entities[0]!.fields; + first["name"] = `${String(first["name"])}${"a".repeat( + STATE_INTAKE_LIMIT_BYTES - + Buffer.byteLength(JSON.stringify(document), "utf8"), + )}`; + expect(Buffer.byteLength(JSON.stringify(document), "utf8")).toBe( + STATE_INTAKE_LIMIT_BYTES, + ); + const before = books.snapshot(); + const replaced = books.store.replace(document); + expect(replaced.ok).toBe(false); + expect(!replaced.ok && replaced.error.issues).toEqual([ + expect.objectContaining({ + reason: "out_of_range", + maximum: STATE_INTAKE_LIMIT_BYTES, + }), + ]); + expect(books.snapshot()).toBe(before); + }); }); diff --git a/src/domain/analysisView/EnvironmentStateWriteSimulation.ts b/src/domain/analysisView/EnvironmentStateWriteSimulation.ts index 1e63aa24a5..c2755a5d4f 100644 --- a/src/domain/analysisView/EnvironmentStateWriteSimulation.ts +++ b/src/domain/analysisView/EnvironmentStateWriteSimulation.ts @@ -178,7 +178,7 @@ export class EnvironmentStateWriteSimulation { unknown: (type, collection, id) => this.#taint.unknown(type, collection, id), initialDocument: (tool) => - this.#input.documents.find((document) => document.tool === tool)?.value, + this.#input.documents.find((document) => document.tool === tool), }; } diff --git a/src/domain/analysisView/environmentStateWriteEffects.ts b/src/domain/analysisView/environmentStateWriteEffects.ts index 9200ba106b..66c0853f8c 100644 --- a/src/domain/analysisView/environmentStateWriteEffects.ts +++ b/src/domain/analysisView/environmentStateWriteEffects.ts @@ -1,13 +1,16 @@ import type { JsonValue } from "../jsonValue.js"; import { collectionPathOf, + decodedSegment, isObject, isStateWrite, lastSegmentValue, locateItem, at, same, + segmentsOf, type JsonObject, + type StateDocument, type StateRecordFacts, type StateToolFacts, type StateView, @@ -85,8 +88,8 @@ export interface EffectContext { readonly observed: (ordinal: number) => void; /** Whether an unmodelled write may have changed the entity since the state last saw it. */ readonly unknown: (type: string, collection: string, id: string) => boolean; - /** A document's value in the initial state, which serving also reads member types from. */ - readonly initialDocument: (tool: string) => JsonValue | undefined; + /** A document as the initial state holds it: its path, and the value serving also reads member types from. */ + readonly initialDocument: (tool: string) => StateDocument | undefined; } /** One recorded write: the record, its timeline index, its tool, and whether its plan only takes the recorded outcome. */ @@ -443,6 +446,20 @@ export const applyDelete = ( }; }; +/** Whether a write's path names the held document's, a segment verbatim or once percent-decoded, as serving matches a call. */ +const namesDocument = (path: string, held: string): boolean => { + const written = segmentsOf(path); + const recorded = segmentsOf(held); + return ( + written.length === recorded.length && + recorded.every( + (segment, index) => + segment === written[index] || + decodedSegment(segment) === written[index], + ) + ); +}; + /** Apply a recorded document write: the body merged into, or replacing, the object holding most of its members. */ export const applyDocument = ( context: EffectContext, @@ -450,6 +467,10 @@ export const applyDocument = ( step: WriteStep, ): EffectResult => { const tool = draft.plan.target!; + const initial = context.initialDocument(tool); + // Serving refuses a write at another path, so one here changes no document the state holds. + if (initial !== undefined && !namesDocument(step.record.path, initial.path)) + return UNKNOWN; const value = context.state.document(tool); const body = requestObject(context, step.record); const pointer = @@ -482,7 +503,7 @@ export const applyDocument = ( // Serving refuses a body its check refuses, so such a call cannot confirm the plan. if ( documentBodyProblem(draft.plan, call, { - initial: context.initialDocument(tool), + initial: initial?.value, current: value, }) !== undefined ) {