From 7de20bc9eaf08eca6540d6c2c929266e0ad84948 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 21 Oct 2026 22:49:11 +0000 Subject: [PATCH] fix(environment): credit sent fields, negative sequences, and encoded targets A later overwrite copied surviving derived fields into the sent-member claim, so a members-only check rejected an update whose sent field still matched. Serving floored sequences at zero and stored missing history as zero, so a collection sequence learned from negative ranks skipped the next value. An absence after a delete forgot the resolved collection, and an encoded read could not observe an update spelled with another collection encoding. Regression tests cover each case. Co-authored-by: morluto --- .../environment/environmentStateStore.ts | 17 +++- .../environmentStateWriteAnswer.test.ts | 36 +++++++ .../environmentStateWriteAnswer.ts | 25 ++--- .../EnvironmentStateWriteSimulation.test.ts | 85 +++++++++++++++++ .../environmentStateWriteAttribution.ts | 13 ++- .../environmentStateWriteChecks.ts | 95 +++++++++++++++---- .../environmentStateWriteCreate.ts | 13 ++- .../environmentStateWriteEffects.ts | 33 ++++--- .../environmentStateWriteObserve.ts | 15 +-- .../environmentStateWriteValues.ts | 11 +++ 10 files changed, 282 insertions(+), 61 deletions(-) diff --git a/src/application/environment/environmentStateStore.ts b/src/application/environment/environmentStateStore.ts index dc712b4c02..89732f50ac 100644 --- a/src/application/environment/environmentStateStore.ts +++ b/src/application/environment/environmentStateStore.ts @@ -2,6 +2,7 @@ import type { StateEntity, StateType, } from "../../domain/analysisView/environmentStateRules.js"; +import { nextSequence } from "../../domain/analysisView/environmentStateWriteValues.js"; import { AnalysisInputError, AnalysisResourceConstraintError, @@ -191,13 +192,19 @@ export class EnvironmentStateStore { /** Count an applied write and keep the sequence values it took, so a refused call takes none. */ commitWrite(sequences: ReadonlyMap): void { this.#writes += 1; - for (const [key, value] of sequences) - this.#sequences.set(key, Math.max(value, this.#sequences.get(key) ?? 0)); + for (const [key, value] of sequences) { + const taken = this.#sequences.get(key); + // No earlier value leaves this one in place, including when it is negative. + this.#sequences.set( + key, + taken === undefined ? value : Math.max(taken, value), + ); + } } - /** The next value of a sequence: one past both the largest held value and the largest an applied write took. */ - sequenceAfter(key: string, held: number): number { - return Math.max(held, (this.#sequences.get(key) ?? 0) + 1); + /** The next value of a sequence: the next held integer, else one past the later of that and the last value taken. */ + sequenceAfter(key: string, held: number | undefined): number { + return nextSequence(held, this.#sequences.get(key)); } #restart(): void { diff --git a/src/application/environment/environmentStateWriteAnswer.test.ts b/src/application/environment/environmentStateWriteAnswer.test.ts index c4caa15de9..aef6853dd4 100644 --- a/src/application/environment/environmentStateWriteAnswer.test.ts +++ b/src/application/environment/environmentStateWriteAnswer.test.ts @@ -337,6 +337,24 @@ describe("refusals", () => { }); }); +/** Items whose `rank` is a collection sequence, with a recorded create at `recorded`. */ +const rankedItems = (initial: readonly [number, number], recorded: number) => { + const item = (id: number, rank: number) => ({ + id, + name: `n${String(id)}`, + rank, + }); + const [first, second] = initial; + return servedFrom([ + ["GET", "/api/items", [item(1, first), item(2, second)]], + ["GET", "/api/items/1", item(1, first)], + ["GET", "/api/items/2", item(2, second)], + ["POST", "/api/items", item(3, recorded), 201, { name: "n3" }], + ["GET", "/api/items/3", item(3, recorded)], + ["GET", "/api/items", [item(1, first), item(2, second), item(3, recorded)]], + ]); +}; + describe("generated values", () => { it("answers the same calls identically after a reset, never reusing a deleted id", () => { const books = servedFrom(); @@ -362,6 +380,24 @@ describe("generated values", () => { }); }); + it("serves the next rank when a collection sequence runs through negative values", () => { + const acrossZero = rankedItems([-2, -1], 0); + expect( + acrossZero.answer("post_items", { body: { name: "n4" } }).body, + ).toMatchObject({ id: 3, rank: 0 }); + expect( + acrossZero.answer("post_items", { body: { name: "n5" } }).body, + ).toMatchObject({ id: 4, rank: 1 }); + // The value after a stored negative rank stays on that sequence. + const negative = rankedItems([-4, -3], -2); + expect( + negative.answer("post_items", { body: { name: "n4" } }).body, + ).toMatchObject({ id: 3, rank: -2 }); + expect( + negative.answer("post_items", { body: { name: "n5" } }).body, + ).toMatchObject({ id: 4, rank: -1 }); + }); + it("restarts the clock and the sequences when data/add loads a state", () => { const books = servedFrom(); const initial: unknown = JSON.parse(books.snapshot()); diff --git a/src/application/environment/environmentStateWriteAnswer.ts b/src/application/environment/environmentStateWriteAnswer.ts index 00dda491f7..bb40c3a4d2 100644 --- a/src/application/environment/environmentStateWriteAnswer.ts +++ b/src/application/environment/environmentStateWriteAnswer.ts @@ -8,7 +8,10 @@ import { writtenResponse, type GenerateValue, } from "../../domain/analysisView/environmentStateWriteApply.js"; -import { insertIndex } from "../../domain/analysisView/environmentStateWriteExplain.js"; +import { + insertIndex, + nextInteger, +} from "../../domain/analysisView/environmentStateWriteExplain.js"; import { pathValuesOf, type GeneratedRule, @@ -17,7 +20,6 @@ import { } from "../../domain/analysisView/environmentStateWriteRules.js"; import { formatClock, - integerOf, isUuid, seededToken, seededUuid, @@ -126,13 +128,12 @@ const observedEntity = ( return isObject(object) ? object : undefined; }; -/** The largest integer the entities hold in a field, plus one. */ -const nextInteger = (entities: readonly StateEntity[], field: string): number => - entities.reduce( - (largest, entity) => - Math.max(largest, integerOf(entity.fields[field]) ?? 0), - 0, - ) + 1; +/** One past the largest integer these entities hold in the field, or undefined when none holds one. */ +const nextHeld = (entities: readonly StateEntity[], field: string) => + nextInteger( + entities.map((entity) => entity.fields), + field, + ); /** * Values one served write generates: its clock instant, sequences one past the largest held @@ -154,7 +155,7 @@ const generator = ( const { store } = serving; const write = store.pendingWrite(); const taken = new Map(); - const sequence = (key: string, held: number): number => { + const sequence = (key: string, held: number | undefined): number => { const value = store.sequenceAfter(key, held); taken.set(key, value); return value; @@ -191,7 +192,7 @@ const generator = ( exemplar, sequence( `${request.tool.name} ${rule.field} ${rule.scope === "collection" ? target.name : ""}`, - nextInteger( + nextHeld( rule.scope === "collection" ? [...target.collection, ...startedIn] : [...ofType, ...started], @@ -211,7 +212,7 @@ const generator = ( exemplar, sequence( `${request.plan.target ?? ""} ${rule.field} ${target.name}`, - nextInteger([...target.collection, ...startedIn], rule.field), + nextHeld([...target.collection, ...startedIn], rule.field), ), ), ); diff --git a/src/domain/analysisView/EnvironmentStateWriteSimulation.test.ts b/src/domain/analysisView/EnvironmentStateWriteSimulation.test.ts index c48989893b..4c86779656 100644 --- a/src/domain/analysisView/EnvironmentStateWriteSimulation.test.ts +++ b/src/domain/analysisView/EnvironmentStateWriteSimulation.test.ts @@ -740,6 +740,60 @@ describe("reads named as serving names them", () => { }); }); + it("confirms a delete by a plain absence of an entity held under the encoded collection", () => { + const alpha = orgBook(1, "acme1", "Alpha"); + const beta = orgBook(2, "beta2", "Beta"); + const rows: TimedCaptureRow[] = [ + ["GET", "/api/orgs/%61cme1/books", [alpha]], + ["GET", "/api/orgs/beta2/books", [beta]], + ["GET", "/api/orgs/%61cme1/books/1", alpha], + ["GET", "/api/orgs/beta2/books/2", beta], + ["DELETE", "/api/orgs/acme1/books/1", "", 204], + ["GET", "/api/orgs/acme1/books/1", BOOK_NOT_FOUND, 404], + ["DELETE", "/api/orgs/beta2/books/2", "", 204], + ["GET", "/api/orgs/beta2/books/2", BOOK_NOT_FOUND, 404], + ]; + const { writes } = environmentStateViewOf(timedCaptureEvidence(rows), [ + "sibling-variance", + ]); + expect(writes.find((plan) => plan.effect === "delete")).toMatchObject({ + served: true, + checked_record_ordinals: expect.arrayContaining([5]), + }); + }); + + it("learns a plain-path update from a later read under the encoded collection", () => { + const alpha = orgBook(1, "acme1", "Alpha"); + const renamed = orgBook(1, "acme1", "Renamed"); + const beta = orgBook(2, "beta2", "Beta"); + const renamedBeta = orgBook(2, "beta2", "Renamed Beta"); + const rows: TimedCaptureRow[] = [ + ["GET", "/api/orgs/%61cme1/books", [alpha]], + ["GET", "/api/orgs/beta2/books", [beta]], + ["GET", "/api/orgs/%61cme1/books/1", alpha], + ["GET", "/api/orgs/beta2/books/2", beta], + ["PATCH", "/api/orgs/acme1/books/1", "", 204, { name: "Renamed" }], + ["GET", "/api/orgs/%61cme1/books/1", renamed], + ["GET", "/api/orgs/%61cme1/books/1", renamed], + [ + "PATCH", + "/api/orgs/beta2/books/2", + renamedBeta, + 200, + { name: "Renamed Beta" }, + ], + ["GET", "/api/orgs/beta2/books/2", renamedBeta], + ]; + const { writes } = environmentStateViewOf(timedCaptureEvidence(rows), [ + "sibling-variance", + ]); + expect(writes.find((plan) => plan.effect === "update")).toMatchObject({ + served: true, + observation: { ordinal: 5 }, + checked_record_ordinals: expect.arrayContaining([6]), + }); + }); + it("checks a line whose query repeats a name with one value, which a call still names", () => { const rows: TimedCaptureRow[] = [ ["GET", "/api/books?tag=a&tag=a", [FIRST_BOOK]], @@ -870,6 +924,37 @@ describe("what a check leaves pending", () => { }); }); + it("checks a surviving sent member after a derived field was bumped by an unmodelled write", () => { + const entry = ( + name: string, + description: string, + version: number, + ): Book => ({ id: 1, name, description, version }); + const other = { id: 2, name: "B", description: "y", version: 1 }; + const rows: TimedCaptureRow[] = [ + ["GET", "/api/books", [entry("A", "x", 1), other]], + ["GET", "/api/books/1", entry("A", "x", 1)], + ["GET", "/api/books/2", other], + [ + "PATCH", + "/api/books/1", + entry("A2", "new", 2), + 200, + { name: "A2", description: "new" }, + ], + ["PUT", "/api/books/1", entry("A3", "new", 2), 200, { name: "A3" }], + ["POST", "/api/books/1/archive", { archived: true }, 204, {}], + // The read still shows the description the patch sent. The version bump is not that patch. + ["GET", "/api/books/1", entry("A3", "new", 9)], + ]; + expect(gated(rows, "patch_books_by_id")).toMatchObject({ + served: true, + rules: expect.arrayContaining([{ field: "version", rule: "increment" }]), + checked_record_ordinals: [6], + contradicted_by: null, + }); + }); + it("neither resynchronises nor checks an entity by a read begun while a write to it still ran", () => { const rows: TimedCaptureRow[] = [ ["GET", "/api/books", [FIRST_BOOK, SECOND_BOOK]], diff --git a/src/domain/analysisView/environmentStateWriteAttribution.ts b/src/domain/analysisView/environmentStateWriteAttribution.ts index 49b551f1be..71c3970a1d 100644 --- a/src/domain/analysisView/environmentStateWriteAttribution.ts +++ b/src/domain/analysisView/environmentStateWriteAttribution.ts @@ -222,7 +222,18 @@ export class WriteEffects { const left = tool === at.writer ? claim : remaining(claim, write); if (left !== undefined) { own!.set(tool, left); - if (tool !== at.writer) touched.get(at.entity)?.set(tool, left); + if (tool !== at.writer) { + const tools = touched.get(at.entity); + const sent = tools?.get(tool); + // touched keeps sent members. A members check compares those; a surviving derived field stays on the effect. + if (tools !== undefined && sent !== undefined) + tools.set( + tool, + sent !== null && left !== null + ? sent.filter((field) => left.includes(field)) + : left, + ); + } } else { own!.delete(tool); touched.get(at.entity)?.delete(tool); diff --git a/src/domain/analysisView/environmentStateWriteChecks.ts b/src/domain/analysisView/environmentStateWriteChecks.ts index 759ce4bd4f..774d46f5fa 100644 --- a/src/domain/analysisView/environmentStateWriteChecks.ts +++ b/src/domain/analysisView/environmentStateWriteChecks.ts @@ -147,11 +147,19 @@ export const collectionTarget = (type: string, collection: string) => `c${compositeKey([type, collection])}`; export const documentTarget = (tool: string) => `d${compositeKey([tool])}`; +/** A target `remove` dropped, so a later absence can still name the collection and id serving resolved. */ +interface RemovedTarget { + readonly type: string; + readonly collection: string; + readonly id: string; +} + /** The gate's copy of the state: entities in collection order and documents, changed by modelled writes. */ export class SimulatedState { readonly #entities = new Map(); readonly #collections = new Map(); readonly #documents = new Map(); + readonly #removed: RemovedTarget[] = []; constructor( entities: readonly StateEntity[], @@ -203,6 +211,27 @@ export class SimulatedState { collection, order.filter((member) => member !== key), ); + const index = this.#removed.findIndex( + (target) => + target.type === entity.type && + target.collection === entity.collection && + target.id === entity.id, + ); + if (index >= 0) this.#removed.splice(index, 1); + this.#removed.push({ + type: entity.type, + collection: entity.collection, + id: entity.id, + }); + } + + /** Removed targets of this type the state does not hold again. */ + removedTargets(type: string): readonly RemovedTarget[] { + return this.#removed.filter( + (target) => + target.type === type && + !this.#entities.has(entityTarget(type, target.collection, target.id)), + ); } document(tool: string): JsonValue | undefined { @@ -264,9 +293,34 @@ const segmentMatch = ( return verbatim; }; +/** A held target an item path names, preferring the path's own collection, then a verbatim spelling. */ +const matchingTarget = ( + path: { + readonly read: readonly string[]; + readonly ids: readonly string[]; + readonly written: string; + }, + collections: readonly string[], + held: (collection: string, id: string) => boolean, +): { readonly collection: string; readonly id: string } | undefined => { + for (const id of path.ids) + if (held(path.written, id)) return { collection: path.written, id }; + const ranked = [...new Set(collections)] + .flatMap((collection) => { + const verbatim = segmentMatch(segmentsOf(collection), path.read); + return verbatim === undefined ? [] : [{ collection, verbatim }]; + }) + .sort((left, right) => right.verbatim - left.verbatim); + for (const { collection } of ranked) + for (const id of path.ids) + if (held(collection, id)) return { collection, id }; + return undefined; +}; + /** * The entity an item read or write names, as serving names it: in a held collection its path matches, verbatim - * spellings first, the id percent-decoded once, then as written; else the collection and decoded id as written. + * spellings first, the id percent-decoded once, then as written; else a removed target still named that way, so + * an absence keeps the collection and id a delete resolved; else the collection and decoded id as written. */ export const itemTarget = ( state: SimulatedState, @@ -275,24 +329,27 @@ export const itemTarget = ( ): { readonly collection: string; readonly id: string } => { const read = segmentsOf(record.path); const raw = read.pop() ?? ""; - const ids = [...new Set([decodedSegment(raw), raw])]; - const written = `/${read.join("/")}`; - for (const id of ids) - if (state.entity(type, written, id) !== undefined) - return { collection: written, id }; - const collections = [ - ...new Set(state.ofType(type).map((entity) => entity.collection)), - ] - .flatMap((collection) => { - const verbatim = segmentMatch(segmentsOf(collection), read); - return verbatim === undefined ? [] : [{ collection, verbatim }]; - }) - .sort((left, right) => right.verbatim - left.verbatim); - for (const { collection } of collections) - for (const id of ids) - if (state.entity(type, collection, id) !== undefined) - return { collection, id }; - return { collection: written, id: ids[0]! }; + const path = { + read, + ids: [...new Set([decodedSegment(raw), raw])], + written: `/${read.join("/")}`, + }; + const removed = state.removedTargets(type); + return ( + matchingTarget( + path, + state.ofType(type).map((entity) => entity.collection), + (collection, id) => state.entity(type, collection, id) !== undefined, + ) ?? + matchingTarget( + path, + removed.map((target) => target.collection), + (collection, id) => + removed.some( + (target) => target.collection === collection && target.id === id, + ), + ) ?? { collection: path.written, id: path.ids[0]! } + ); }; export const checkItemRead = ( diff --git a/src/domain/analysisView/environmentStateWriteCreate.ts b/src/domain/analysisView/environmentStateWriteCreate.ts index 05e516d8f5..1ceb53597e 100644 --- a/src/domain/analysisView/environmentStateWriteCreate.ts +++ b/src/domain/analysisView/environmentStateWriteCreate.ts @@ -41,7 +41,12 @@ import { type Observed, } from "./environmentStateWriteObserve.js"; import type { FieldRule, WriteCall } from "./environmentStateWriteRules.js"; -import { integerOf, isUuid, readClock } from "./environmentStateWriteValues.js"; +import { + integerOf, + isUuid, + nextSequence, + readClock, +} from "./environmentStateWriteValues.js"; /** The id a response names alone: a scalar body, or an object's id field or single scalar member. */ const respondedId = ( @@ -180,9 +185,9 @@ const followsSequences = ( (entity) => rule.scope === "type" || entity.collection === created.collection, ); - const next = Math.max( - nextInteger(fieldsOf(scope), rule.field) ?? 1, - (draft.sequences.get(key) ?? 0) + 1, + const next = nextSequence( + nextInteger(fieldsOf(scope), rule.field), + draft.sequences.get(key), ); return integerOf(created.fields[rule.field]) === next; }); diff --git a/src/domain/analysisView/environmentStateWriteEffects.ts b/src/domain/analysisView/environmentStateWriteEffects.ts index da9f4b0bc7..6af7c7ab7c 100644 --- a/src/domain/analysisView/environmentStateWriteEffects.ts +++ b/src/domain/analysisView/environmentStateWriteEffects.ts @@ -1,10 +1,8 @@ import type { JsonValue } from "../jsonValue.js"; import { ok } from "../result.js"; import { - collectionPathOf, isObject, isStateWrite, - lastSegmentValue, locateItem, at, same, @@ -201,39 +199,46 @@ export const searchFrom = (context: EffectContext, step: WriteStep) => ({ withBody: context.io.withBody, }); -/** The entity an update's response shows, located by its identity, else a later item read of it spelled as the call was. */ +/** The entity an update's response shows, located by its identity, else a later item read of the target the write resolved. */ const updatedObservation = ( context: EffectContext, step: WriteStep, - entity: { readonly type: string; readonly idField: string }, + entity: { + readonly type: string; + readonly idField: string; + readonly collection: string; + readonly id: string; + }, ): Observed | undefined => { - const target = { - ...entity, - collection: collectionPathOf(step.record, true), - id: lastSegmentValue(step.record.path), - }; const own = context.io.withBody( step.record.ordinal, (body): Observed | undefined => { const located = - body === undefined ? undefined : locateItem(body, target.id); + body === undefined ? undefined : locateItem(body, entity.id); const object = located === undefined ? undefined : at(body!, located.pointer); - return located?.keys.includes(target.idField) === true && isObject(object) + return located?.keys.includes(entity.idField) === true && isObject(object) ? { ordinal: step.record.ordinal, pointer: located.pointer, object, - id: target.id, + id: entity.id, } : undefined; }, ); if (own !== undefined) return own; const later = laterObservation(searchFrom(context, step), { - ...target, + type: entity.type, + collection: entity.collection, + idField: entity.idField, + id: entity.id, held: () => true, isWrite: isStateWrite, + sameItem: (record) => { + const found = itemTarget(context.state, entity.type, record); + return found.collection === entity.collection && found.id === entity.id; + }, }); if (later !== undefined) context.observed(later.ordinal); return later; @@ -365,7 +370,7 @@ export const applyUpdate = ( const after = entity === undefined ? undefined - : updatedObservation(context, step, { type, idField }); + : updatedObservation(context, step, { type, idField, collection, id }); if (entity === undefined || after === undefined) { fail(draft, step, "no-observation"); return UNKNOWN; diff --git a/src/domain/analysisView/environmentStateWriteObserve.ts b/src/domain/analysisView/environmentStateWriteObserve.ts index dcfa0a6d4e..cfda4a2918 100644 --- a/src/domain/analysisView/environmentStateWriteObserve.ts +++ b/src/domain/analysisView/environmentStateWriteObserve.ts @@ -85,6 +85,8 @@ export const laterObservation = ( readonly id?: string; readonly held: (id: string) => boolean; readonly isWrite: (record: StateRecordFacts) => boolean; + /** An item read matches by this instead of equal collection strings. */ + readonly sameItem?: (record: StateRecordFacts) => boolean; }, ): Observed | undefined => { for (const record of search.records.slice(search.from + 1)) { @@ -111,12 +113,13 @@ const itemObservation = ( pointer: string, target: Parameters[1], ): Observed | undefined => { - const id = lastSegmentValue(record.path); - if ( - collectionPathOf(record, true) !== target.collection || - (target.id !== undefined ? id !== target.id : target.held(id)) - ) - return undefined; + const pathId = lastSegmentValue(record.path); + const named = + target.sameItem?.(record) ?? + (collectionPathOf(record, true) === target.collection && + (target.id === undefined ? !target.held(pathId) : pathId === target.id)); + if (!named) return undefined; + const id = target.id ?? pathId; return search.withBody(record.ordinal, (body) => { const object = body === undefined diff --git a/src/domain/analysisView/environmentStateWriteValues.ts b/src/domain/analysisView/environmentStateWriteValues.ts index 3be510fe03..10e814f22a 100644 --- a/src/domain/analysisView/environmentStateWriteValues.ts +++ b/src/domain/analysisView/environmentStateWriteValues.ts @@ -107,6 +107,17 @@ export const integerOf = (value: JsonValue | undefined): number | undefined => ? Number(value) : undefined; +/** + * The next value of a sequence. With nothing taken yet, that is the next held integer, or 1 when + * the scope holds none. Otherwise it is one past the later of that and the last value taken. + * A missing side leaves a negative value negative. + */ +export const nextSequence = ( + held: number | undefined, + taken: number | undefined, +): number => + taken === undefined ? (held ?? 1) : Math.max(held ?? taken + 1, taken + 1); + /** An integer spelled as the recorded value spells integers: a number, or a string of digits. */ export const spelledLike = (exemplar: JsonValue, value: number): JsonValue => typeof exemplar === "string" ? String(value) : value;