diff --git a/docs/specs/agent-environments/state-writes/spec.md b/docs/specs/agent-environments/state-writes/spec.md index ebd713b38b..8b927d8383 100644 --- a/docs/specs/agent-environments/state-writes/spec.md +++ b/docs/specs/agent-environments/state-writes/spec.md @@ -110,8 +110,9 @@ explained as one of: - a slug of an echoed member. Any other change is `unexplained-change`. An increment or a slug applies to -every call that could take it, so a call that leaves such a field as it was -while another call of the tool changes it is also `unexplained-change`. +every call that could take it, so the plan is also `unexplained-change` when +such a rule would give a call of the tool another value than the one it +recorded. **Deletes.** A delete removes the entity. Its response is the exemplar's response, with the entity re-rendered when the response carries it. @@ -172,8 +173,9 @@ from the state: - an item read of a state-backed item view; - a list or document read on one of its view's checked request lines, - matched as serving matches a call to a line: path and query values - decoded, the query in any order. + matched as serving matches a call to a line: the path as spelled, since + replay prefers a verbatim spelling, and the query values decoded, in any + order. **Comparison.** The read's answer is the full response serving would give from the copy: an item read's entity rendered into its item view's base recording, diff --git a/docs/specs/agent-environments/state-writes/verification.md b/docs/specs/agent-environments/state-writes/verification.md index e8c9834493..3f74acdd94 100644 --- a/docs/specs/agent-environments/state-writes/verification.md +++ b/docs/specs/agent-environments/state-writes/verification.md @@ -205,3 +205,8 @@ fails on the previous head and passes here. The same gates passed again boundary set 75, conformance 279, `jscpd` 3.47%), the write verdicts over the 50 captures were again unchanged plan by plan, and the smoke applied all 33 served write tools. + +A third round kept a checked line's path spelling (replay prefers a verbatim +one, so two spellings of one parent stay separate lines) and compared a later +slug rule with the values earlier calls recorded rather than only their member +names; both new tests fail on the previous head. diff --git a/src/domain/analysisView/EnvironmentStateWriteSimulation.test.ts b/src/domain/analysisView/EnvironmentStateWriteSimulation.test.ts index ab961da8bc..30b029cb4b 100644 --- a/src/domain/analysisView/EnvironmentStateWriteSimulation.test.ts +++ b/src/domain/analysisView/EnvironmentStateWriteSimulation.test.ts @@ -16,6 +16,7 @@ import { renamedBook, type Book, } from "../../../tests/fixtures/environmentStateWrites.js"; +import type { JsonValue } from "../jsonValue.js"; import { ok } from "../result.js"; import { updatedFields } from "./environmentStateWriteApply.js"; @@ -234,6 +235,78 @@ describe("update rules learned from a later call", () => { }); }); +describe("slug rules learned from a later call", () => { + /** A patch of the first book sending `name`, answered with `answered`, then a rename of the second. */ + const renames = (name: string, answered: Book): TimedCaptureRow[] => { + const second = renamedBook(SECOND_BOOK, "Second Edition", 5); + return [ + ...BOOK_INIT_ROWS, + ["PATCH", "/api/books/1", answered, 200, { name }], + ["GET", "/api/books/1", answered], + ["PATCH", "/api/books/2", second, 200, { name: "Second Edition" }], + ["GET", "/api/books/2", second], + ]; + }; + + it("serves a slug rule an earlier call agrees with, and not one it contradicts", () => { + const kept = { ...FIRST_BOOK, updated_at: bookClock(3) }; + expect( + gated(renames("First Book", kept), "patch_books_by_id"), + ).toMatchObject({ + served: true, + rules: expect.arrayContaining([ + { field: "slug", rule: "slug", from: "name" }, + ]), + }); + // The application renamed the first book without following its slug. + expect( + gated( + renames("Renamed Book", { ...kept, name: "Renamed Book" }), + "patch_books_by_id", + ), + ).toMatchObject({ served: false, reason: "unexplained-change" }); + }); +}); + +describe("checked lines", () => { + /** A list page that echoes its own request path, as some APIs do. */ + const page = (path: string, items: JsonValue[]): JsonValue => ({ + path, + items, + }); + const PLAIN = "/api/orgs/acme1/books"; + const ENCODED = "/api/orgs/%61cme1/books"; + const OTHER = "/api/orgs/beta2/books"; + /** Three organisations' books under one templated tool, two spelling one organisation differently, then deletes each confirmed by its list. */ + const ROWS: TimedCaptureRow[] = [ + ["GET", PLAIN, page(PLAIN, [FIRST_BOOK])], + ["GET", ENCODED, page(ENCODED, [SECOND_BOOK])], + ["GET", OTHER, page(OTHER, [THIRD_BOOK])], + ["GET", `${PLAIN}/1`, FIRST_BOOK], + ["GET", `${ENCODED}/2`, SECOND_BOOK], + ["GET", `${OTHER}/3`, THIRD_BOOK], + ["DELETE", `${ENCODED}/2`, "", 204], + ["GET", ENCODED, page(ENCODED, [])], + ["DELETE", `${OTHER}/3`, "", 204], + ["GET", OTHER, page(OTHER, [])], + ]; + + it("keeps a line's path spelling, as replay prefers the verbatim one", () => { + const view = environmentStateViewOf(timedCaptureEvidence(ROWS), [ + "sibling-variance", + ]); + expect(view.views).toContainEqual( + expect.objectContaining({ + shape: "list", + checked_record_ordinals: [0, 1, 2], + }), + ); + expect( + view.writes.find((candidate) => candidate.effect === "delete"), + ).toMatchObject({ served: true, checked_record_ordinals: [7, 9] }); + }); +}); + describe("insert rules", () => { it("still learns where a list places a create after an item read confirmed it", () => { const alpha = book(3, "Alpha", bookClock(3)); diff --git a/src/domain/analysisView/environmentStateRules.ts b/src/domain/analysisView/environmentStateRules.ts index 160c117c7b..cb18769f8e 100644 --- a/src/domain/analysisView/environmentStateRules.ts +++ b/src/domain/analysisView/environmentStateRules.ts @@ -204,8 +204,9 @@ export const same = ( : canonicalJson(left, "Environment state") === canonicalJson(right, "Environment state"); -/** A path segment percent-decoded, or as written when it does not decode. */ -export const decodedSegment = (segment: string): string => { +/** The decoded final path segment, which an item request names its entity by. */ +export const lastSegmentValue = (path: string): string => { + const segment = segmentsOf(path).at(-1) ?? ""; try { return decodeURIComponent(segment); } catch { @@ -213,10 +214,6 @@ export const decodedSegment = (segment: string): string => { } }; -/** The decoded final path segment, which an item request names its entity by. */ -export const lastSegmentValue = (path: string): string => - decodedSegment(segmentsOf(path).at(-1) ?? ""); - /** A path's non-empty segments, so a trailing slash names the same collection. */ export const normalizedPath = (path: string): string => `/${segmentsOf(path).join("/")}`; diff --git a/src/domain/analysisView/environmentStateWriteChecks.ts b/src/domain/analysisView/environmentStateWriteChecks.ts index 0bf41bff59..ef64460329 100644 --- a/src/domain/analysisView/environmentStateWriteChecks.ts +++ b/src/domain/analysisView/environmentStateWriteChecks.ts @@ -7,7 +7,6 @@ import { import { at, collectionPathOf, - decodedSegment, isObject, lastSegmentValue, listItems, @@ -16,7 +15,6 @@ import { replaceAt, same, scalarText, - segmentsOf, type JsonObject, type StateDocument, type StateEntity, @@ -28,15 +26,16 @@ import { } from "./environmentStateRules.js"; /** - * A read's request line as serving names one: its path segments and query values decoded, the - * query in any order. A query repeating a name is no call's arguments, so it keeps its spelling. + * A read's request line as serving names one: its path as spelled, since replay prefers a + * verbatim spelling, and its query values decoded in any order, as replay compares them. A + * query repeating a name is no call's arguments, so it keeps its spelling. */ export const requestLine = (record: StateRecordFacts): string => { const query = [...iterateUrlQueryParameters(record.query ?? "")]; if (new Set(query.map(([name]) => name)).size !== query.length) return compositeKey([record.path, record.query ?? ""]); return compositeKey([ - segmentsOf(record.path).map(decodedSegment), + record.path, query.sort(([left], [right]) => compare(left, right)), ]); }; diff --git a/src/domain/analysisView/environmentStateWriteEffects.ts b/src/domain/analysisView/environmentStateWriteEffects.ts index 032fbbacaf..9200ba106b 100644 --- a/src/domain/analysisView/environmentStateWriteEffects.ts +++ b/src/domain/analysisView/environmentStateWriteEffects.ts @@ -33,6 +33,7 @@ import { laterObservation, type Observed, } from "./environmentStateWriteObserve.js"; +import { integerOf, slugOf } from "./environmentStateWriteValues.js"; import { pathValuesOf, type FieldRule, @@ -54,10 +55,10 @@ export interface WriteDraft { contradictedBy: number | undefined; sideEffect: boolean; calls: number; - /** The fields each modelled update call showed and sent, which later rules must agree with. */ + /** The scalar values each modelled update call showed and sent, which later rules must reproduce. */ readonly updates: { - readonly shown: ReadonlySet; - readonly sent: ReadonlySet; + readonly shown: ReadonlyMap; + readonly sent: ReadonlyMap; }[]; } @@ -246,7 +247,15 @@ const refineUpdate = ( const unsentChange = (rule: FieldRule): boolean => rule.rule === "increment" || rule.rule === "slug"; -/** Whether a rule a later call showed would have changed a field an earlier call showed unchanged without sending. */ +/** A call's scalar members by name, as much of it as later rules are checked against. */ +const scalars = (object: JsonObject): Map => + new Map( + Object.entries(object).filter( + ([, value]) => value === null || typeof value !== "object", + ), + ); + +/** Whether a rule a later call showed gives an earlier call a value other than the one it showed for a field it did not send. */ const contradictsEarlierCalls = ( draft: WriteDraft, rules: readonly FieldRule[], @@ -256,11 +265,14 @@ const contradictsEarlierCalls = ( (rule) => unsentChange(rule) && !ruled.has(rule.field) && - draft.updates.some( - ({ shown, sent }) => - shown.has(rule.field) && - (rule.rule !== "slug" || sent.has(rule.from)), - ), + draft.updates.some(({ shown, sent }) => { + // An unruled field an earlier call showed is one it left as it was. + const value = shown.get(rule.field); + if (value === undefined) return false; + if (rule.rule === "increment") return integerOf(value) !== undefined; + const from = rule.rule === "slug" ? sent.get(rule.from) : undefined; + return typeof from === "string" && slugOf(from) !== value; + }), ); }; @@ -380,10 +392,11 @@ export const applyUpdate = ( if (!written.ok || (disagrees && !unknown)) refuse(draft, step, "unexplained-change"); if (!written.ok || disagrees || unknown) return recorded(); - const shown = Object.keys(after.object); - const sentMembers = Object.keys(body); - context.io.retain(64 + [...shown, ...sentMembers].join("").length * 2); - draft.updates.push({ shown: new Set(shown), sent: new Set(sentMembers) }); + const facts = { shown: scalars(after.object), sent: scalars(body) }; + context.io.retain( + 64 + JSON.stringify([...facts.shown, ...facts.sent]).length * 2, + ); + draft.updates.push(facts); context.io.retain(64 + JSON.stringify(written.value).length * 2); context.state.put(updatedEntity(entity, written.value)); // A later read that also shows an unmodelled change checks only the members this call sent. diff --git a/tests/fixtures/environmentStateWrites.ts b/tests/fixtures/environmentStateWrites.ts index a2c1afc840..0880edbd0d 100644 --- a/tests/fixtures/environmentStateWrites.ts +++ b/tests/fixtures/environmentStateWrites.ts @@ -2,6 +2,7 @@ import { projectAnalysisView, type AnalysisViewResult, } from "../../src/domain/analysisView/analysisView.js"; +import type { TemplateRule } from "../../src/domain/analysisView/environmentManifestRules.js"; import type { Evidence } from "../../src/domain/evidence.js"; import type { JsonValue } from "../../src/domain/jsonValue.js"; import { @@ -145,6 +146,7 @@ type EnvironmentStateView = Extract< /** The environment-state view a capture projects with no opt-in template rules. */ export const environmentStateViewOf = ( capture: Evidence, + templateRules: readonly TemplateRule[] = [], ): EnvironmentStateView => { const result = projectAnalysisView( { @@ -155,7 +157,7 @@ export const environmentStateViewOf = ( limitations: [], parameters: capture.parameters, }, - { kind: "environment-state", template_rules: [] }, + { kind: "environment-state", template_rules: [...templateRules] }, ); if (!result.ok || result.value.kind !== "environment-state") throw new Error("Expected an environment state");