From 43d98ea9411ed0547579cb4425929b1da48aec4f Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Wed, 21 Oct 2026 09:35:41 -0700 Subject: [PATCH] fix(environment): pin a create's observation to the id it answered, and keep later identity, wide bodies and fixed values in check Greptile raised these on the list-lines branch against this branch's code. - A create that answers with its id alone (a scalar, the identity field or one member) is observed only through a read of that id. A read of another, unlisted book can no longer supply the created entity. Lists may supply it when they show that id. - A later update call that is the first to send the path's id in its body adds the identity member to the plan, instead of making the tool unexplained. - The copy search walks a request body with an index rather than shifting it. 500,000 zeros took 63 s before and 74 ms after. - The sent values of fixed members a plan keeps are counted against the view's memory allowance, on the first call and when a later call adds more. The verdicts over the 50 captures are unchanged plan by plan, and the smoke applies all 31 served write tools. Co-Authored-By: Claude Opus 5.5 --- .../agent-environments/state-writes/spec.md | 6 +- .../state-writes/verification.md | 11 ++++ .../EnvironmentStateWriteSimulation.test.ts | 60 +++++++++++++++++++ .../environmentStateWriteCreate.ts | 21 +++++++ .../environmentStateWriteEffects.ts | 24 ++++++-- .../environmentStateWriteExplain.test.ts | 13 ++++ .../environmentStateWriteExplain.ts | 4 +- .../environmentStateWriteObserve.ts | 10 ++-- 8 files changed, 135 insertions(+), 14 deletions(-) diff --git a/docs/specs/agent-environments/state-writes/spec.md b/docs/specs/agent-environments/state-writes/spec.md index 8f31e69d28..589f5b34a7 100644 --- a/docs/specs/agent-environments/state-writes/spec.md +++ b/docs/specs/agent-environments/state-writes/spec.md @@ -56,7 +56,9 @@ the exemplar. one-element array, as item views locate it. When the response does not carry it, the entity is observed in the first later read that shows it: an item read of that entity, or for a create, the one entity a collection read newly - lists. With neither, the reason is `no-observation`. + lists. A create whose response names its id alone (a scalar, the identity + field, or one member) is observed only through a read of that id. With + neither, the reason is `no-observation`. **Field rules for creates.** Each field of the observed entity gets the first rule that explains its value. Rules that read the call come first, so an @@ -233,7 +235,7 @@ refuses to answer with. becomes the plan's rule, unless an earlier list check relied on the rule (an item read's confirmation does not fix it). - The read that supplied a create's or update's observation does not check - it. + it, but checks the other writes it exercises. **Verdicts.** A plan is served when at least one check confirms it and none contradicts it. Otherwise the reason, in this precedence, is: diff --git a/docs/specs/agent-environments/state-writes/verification.md b/docs/specs/agent-environments/state-writes/verification.md index 4a6b82dcbf..d3fd6172c9 100644 --- a/docs/specs/agent-environments/state-writes/verification.md +++ b/docs/specs/agent-environments/state-writes/verification.md @@ -286,3 +286,14 @@ An eleventh round settled six more findings, five raised on this branch and one Each new test fails on the previous head. The verdicts over the 50 captures are unchanged plan by plan, the smoke applies all 31 served write tools, and the gates passed again. + +A twelfth round settled four findings Greptile raised on the list-lines branch against this branch's code: + +- a create that answers with its id alone is observed only through a read of that id; +- a later call that is the first to send the path's id in its body adds the identity member to the plan; +- the copy search walks a request body with an index, so a wide body no longer takes quadratic time (500,000 zeros: 63 s + before, 74 ms after); +- the sent values of fixed members a plan keeps are counted against the view's allowance. + +Each new test fails on the previous head. The verdicts over the 50 captures are unchanged plan by plan, the smoke +applies all 31 served write tools, and the gates passed again. diff --git a/src/domain/analysisView/EnvironmentStateWriteSimulation.test.ts b/src/domain/analysisView/EnvironmentStateWriteSimulation.test.ts index 82974b0a02..0dbcff8c77 100644 --- a/src/domain/analysisView/EnvironmentStateWriteSimulation.test.ts +++ b/src/domain/analysisView/EnvironmentStateWriteSimulation.test.ts @@ -19,6 +19,10 @@ import { import type { JsonValue } from "../jsonValue.js"; import { ok } from "../result.js"; import { updatedFields } from "./environmentStateWriteApply.js"; +import { + retainFixed, + type EffectContext, +} from "./environmentStateWriteEffects.js"; /** The plan the gate gives `tool` over a capture of `rows`. */ const gated = (rows: readonly TimedCaptureRow[], tool: string) => { @@ -752,3 +756,59 @@ describe("reads named as serving names them", () => { }); }); }); + +describe("what a write's own answer pins down", () => { + it("observes a create that answers with its id alone only through a read of that id", () => { + const fourth = book(4, "Fourth Book", "2026-10-01T11:00:00Z"); + const rows: TimedCaptureRow[] = [ + ["GET", "/api/books", [FIRST_BOOK, SECOND_BOOK]], + ["GET", "/api/books/1", FIRST_BOOK], + ["GET", "/api/books/2", SECOND_BOOK], + ["POST", "/api/books", { id: 3 }, 201, { name: "Third Book" }], + // A book the capture never listed is not the one the create made. + ["GET", "/api/books/4", fourth], + ["GET", "/api/books/4", fourth], + ["GET", "/api/books/3", THIRD_BOOK], + ["GET", "/api/books/3", THIRD_BOOK], + ]; + expect(gated(rows, "post_books")).toMatchObject({ + observation: { ordinal: 6, pointer: "" }, + checked_record_ordinals: [7], + }); + }); + + it("keeps an update served when a later call is the first to send the path's id in its body", () => { + const renamedSecond = renamedBook(SECOND_BOOK, "Renamed Second", 5); + const rows: TimedCaptureRow[] = [ + ["GET", "/api/books", [FIRST_BOOK, SECOND_BOOK]], + ["GET", "/api/books/1", FIRST_BOOK], + ["GET", "/api/books/2", SECOND_BOOK], + ["PATCH", "/api/books/1", RENAMED, 200, { name: "Renamed Book" }], + ["GET", "/api/books/1", RENAMED], + [ + "PATCH", + "/api/books/2", + renamedSecond, + 200, + { id: 2, name: "Renamed Second" }, + ], + ["GET", "/api/books/2", renamedSecond], + ]; + expect(gated(rows, "patch_books_by_id")).toMatchObject({ + served: true, + body: { identity: "id" }, + }); + }); +}); + +it("counts the sent values a plan keeps for fixed members against the view's allowance", () => { + let retained = 0; + const context = { + io: { retain: (bytes: number) => (retained += bytes) }, + } as unknown as EffectContext; + retainFixed(context, [{ sent: "x".repeat(10_000) }]); + expect(retained).toBeGreaterThan(20_000); + retained = 0; + retainFixed(context, []); + expect(retained).toBe(0); +}); diff --git a/src/domain/analysisView/environmentStateWriteCreate.ts b/src/domain/analysisView/environmentStateWriteCreate.ts index c84b3bf4a6..a51607cc9d 100644 --- a/src/domain/analysisView/environmentStateWriteCreate.ts +++ b/src/domain/analysisView/environmentStateWriteCreate.ts @@ -1,4 +1,6 @@ +import type { JsonValue } from "../jsonValue.js"; import { + isObject, isStateWrite, normalizedPath, scalarText, @@ -16,6 +18,7 @@ import { fail, refuse, requestObject, + retainFixed, searchFrom, startOf, UNKNOWN, @@ -37,6 +40,18 @@ import { } from "./environmentStateWriteObserve.js"; import type { WriteCall } from "./environmentStateWriteRules.js"; +/** The id a response names alone: a scalar body, or an object's id field or single scalar member. */ +const respondedId = ( + body: JsonValue | undefined, + idField: string, +): string | undefined => { + if (body === undefined) return undefined; + if (!isObject(body)) return scalarText(body); + if (Object.hasOwn(body, idField)) return scalarText(body[idField]); + const members = Object.values(body); + return members.length === 1 ? scalarText(members[0]) : undefined; +}; + /** The written entity a create's response or a later read shows. */ const createdObservation = ( context: EffectContext, @@ -53,8 +68,13 @@ const createdObservation = ( newEntityIn(step.record.ordinal, body, target.idField, held), ); if (own !== undefined) return own; + // A response that names the created id alone ties the later read to that id. + const id = context.io.withBody(step.record.ordinal, (body) => + respondedId(body, target.idField), + ); const later = laterObservation(searchFrom(context, step), { ...target, + ...(id === undefined ? {} : { id }), held, isWrite: isStateWrite, }); @@ -99,6 +119,7 @@ const buildCreate = ( draft.candidates = insertCandidates(members, [ ...new Set([idField, ...clocks, ...scalars]), ]); + retainFixed(context, explained.body.fixed); Object.assign(draft.plan, { exemplar_ordinal: step.record.ordinal, observation: { diff --git a/src/domain/analysisView/environmentStateWriteEffects.ts b/src/domain/analysisView/environmentStateWriteEffects.ts index c9b846f7c7..273d512bd4 100644 --- a/src/domain/analysisView/environmentStateWriteEffects.ts +++ b/src/domain/analysisView/environmentStateWriteEffects.ts @@ -227,23 +227,27 @@ const updatedObservation = ( /** Add what a later call of an update showed to its plan: members, fixed values and rules not seen before. */ const refineUpdate = ( + context: EffectContext, plan: WriteDraft["plan"], explained: ReturnType, ): void => { const body = plan.body!; const fixed = new Set(body.fixed.map((entry) => entry.member)); + const added = explained.body.fixed.filter( + (entry) => !fixed.has(entry.member), + ); + retainFixed(context, added); plan.body = { ...body, + // A later call may be the first to show the body repeating the path's id. + identity: body.identity ?? explained.body.identity, accepted: [ ...body.accepted, ...explained.body.accepted.filter( (member) => !body.accepted.includes(member), ), ], - fixed: [ - ...body.fixed, - ...explained.body.fixed.filter((entry) => !fixed.has(entry.member)), - ], + fixed: [...body.fixed, ...added], }; const ruled = new Set(plan.rules.map((rule) => rule.field)); plan.rules = [ @@ -252,6 +256,15 @@ const refineUpdate = ( ]; }; +/** Count the sent values of fixed members a plan keeps, which outlive the request body they were read from. */ +export const retainFixed = ( + context: EffectContext, + fixed: readonly { readonly sent: JsonValue }[], +): void => { + if (fixed.length > 0) + context.io.retain(64 + JSON.stringify(fixed).length * 2); +}; + /** Whether a rule changes its field on calls that do not send it: serving applies it to every call that could. */ const unsentChange = (rule: FieldRule): boolean => rule.rule === "increment" || rule.rule === "slug"; @@ -297,9 +310,10 @@ const buildUpdate = ( }, ): void => { if (built(draft)) { - refineUpdate(draft.plan, evidence.explained); + refineUpdate(context, draft.plan, evidence.explained); return; } + retainFixed(context, evidence.explained.body.fixed); const { ordinal } = step.record; Object.assign(draft.plan, { exemplar_ordinal: ordinal, diff --git a/src/domain/analysisView/environmentStateWriteExplain.test.ts b/src/domain/analysisView/environmentStateWriteExplain.test.ts index 5a12b3ec7c..7e897b70bf 100644 --- a/src/domain/analysisView/environmentStateWriteExplain.test.ts +++ b/src/domain/analysisView/environmentStateWriteExplain.test.ts @@ -552,3 +552,16 @@ it.each<[string, string, boolean]>([ expect(prefixRelated(left, right)).toBe(related); expect(prefixRelated(right, left)).toBe(related); }); + +it( + "searches a wide request body for a copied value in linear time", + { timeout: 5_000 }, + () => { + // Shifting each value off the queue made one fruitless search over 500,000 zeros take about a minute. + const wide = Array.from({ length: 500_000 }, () => 0); + const result = explainCreate( + created({ id: 1, note: "not anywhere in the body" }, { items: wide }), + ); + expect(ruleFor(result.rules, "note")).toBeUndefined(); + }, +); diff --git a/src/domain/analysisView/environmentStateWriteExplain.ts b/src/domain/analysisView/environmentStateWriteExplain.ts index ff426dd63a..88edf89ae8 100644 --- a/src/domain/analysisView/environmentStateWriteExplain.ts +++ b/src/domain/analysisView/environmentStateWriteExplain.ts @@ -46,7 +46,9 @@ const bodyPointerOf = (body: JsonValue, value: string): string | undefined => { const pending: { readonly value: JsonValue; readonly pointer: string }[] = [ { value: body, pointer: "" }, ]; - for (let next = pending.shift(); next !== undefined; next = pending.shift()) { + // An index walks the queue: shifting a wide body's values one by one would take quadratic time. + for (let at = 0; at < pending.length; at += 1) { + const next = pending[at]!; if (next.value === value) return next.pointer; const children: [string, JsonValue][] = Array.isArray(next.value) ? next.value.map((item, index) => [String(index), item]) diff --git a/src/domain/analysisView/environmentStateWriteObserve.ts b/src/domain/analysisView/environmentStateWriteObserve.ts index 0bd99018c7..dcfa0a6d4e 100644 --- a/src/domain/analysisView/environmentStateWriteObserve.ts +++ b/src/domain/analysisView/environmentStateWriteObserve.ts @@ -136,11 +136,7 @@ const listObservation = ( pointer: string, target: Parameters[1], ): Observed | undefined => { - if ( - target.id !== undefined || - collectionPathOf(record, false) !== target.collection - ) - return undefined; + if (collectionPathOf(record, false) !== target.collection) return undefined; return search.withBody(record.ordinal, (body) => { const items = body === undefined ? undefined : listItems(body, pointer); const unheld = (items ?? []) @@ -151,7 +147,9 @@ const listObservation = ( })) .filter( (entry): entry is { object: JsonObject; index: number; id: string } => - entry.id !== undefined && !target.held(entry.id), + entry.id !== undefined && + !target.held(entry.id) && + (target.id === undefined || entry.id === target.id), ); return unheld.length === 1 ? {