From ef49dca6d7cb5becaf8302f9781fe466f5ab4c71 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Tue, 20 Oct 2026 08:37:47 -0700 Subject: [PATCH] fix(environment): keep changed documents out of the state; mark OPTIONS and TRACE safe Every non-entity GET read became a document from its latest initial recording, bounded only by writes under related roots, so GET /api/me read before and after POST /api/orders published the post-write value as initial state (entities already leave such a type recorded). A document whose value changed between two of its line's reads across any recorded write is now recorded as fields-differ; an unchanged value is still a document. MCP hints treated OPTIONS and TRACE as non-idempotent writes, so a CORS preflight tool fell to the destructive default. The RFC 9110 safe methods (GET, HEAD, OPTIONS, TRACE) are read-only and idempotent. Co-Authored-By: Claude Opus 5.5 --- docs/agent-environments.md | 10 +++---- .../agent-environments/state-coverage/spec.md | 8 ++++-- docs/web-network-captures.md | 2 +- .../environment/environmentTools.test.ts | 19 +++++++++++++ .../environment/environmentTools.ts | 12 ++++++-- .../EnvironmentStateExtraction.ts | 28 ++++++++++++++----- .../analysisView/environmentStateView.test.ts | 27 ++++++++++++++++++ 7 files changed, 87 insertions(+), 19 deletions(-) diff --git a/docs/agent-environments.md b/docs/agent-environments.md index 8158a4d0ca..26444e0d35 100644 --- a/docs/agent-environments.md +++ b/docs/agent-environments.md @@ -326,12 +326,12 @@ records and the evidence ID, and the whole catalog stays within its serialization reservation, which the summaries and MCP annotations are charged to. -Over MCP, every tool also carries annotations: `readOnlyHint` is true for `GET` -and `HEAD` tools and for query `POST`s (a path segment such as `search` or -`query`); `destructiveHint` is true for `DELETE` and left unset for other +Over MCP, every tool also carries annotations: `readOnlyHint` is true for the +safe methods (`GET`, `HEAD`, `OPTIONS` and `TRACE`) and for query `POST`s (a +path segment such as `search` or `query`); `destructiveHint` is true for `DELETE` and left unset for other writes, so the MCP default (destructive) applies to them, and read-only tools -omit it because clients ignore it there; `idempotentHint` is true for `GET`, -`HEAD`, `PUT` and `DELETE`; and `openWorldHint` is false for every tool, +omit it because clients ignore it there; `idempotentHint` is true for the safe +methods, `PUT` and `DELETE`; and `openWorldHint` is false for every tool, because answers come from the capture rather than the captured application. The hints describe the captured method; replay itself changes nothing. The card keeps the `name`, `description` and `inputSchema` of the echo example's tool diff --git a/docs/specs/agent-environments/state-coverage/spec.md b/docs/specs/agent-environments/state-coverage/spec.md index fd0b28cd0b..a18f09061c 100644 --- a/docs/specs/agent-environments/state-coverage/spec.md +++ b/docs/specs/agent-environments/state-coverage/spec.md @@ -17,7 +17,8 @@ tool, view kind, provider, network activity or write effect. - A `GET` list tool (its template does not end in a parameter) that yields no item or list plan holds its latest initial value as a `document` when its initial reads have one request line, whether or not the capture writes its - path. More request lines give `path-varies` when they differ by path (a + path, unless its value changed between two of those reads across any + recorded write (even under another root), which leaves it `fields-differ`. More request lines give `path-varies` when they differ by path (a template with a parameter before its end) and `query-varies` when they share one path. With every such read now a document, `path-varies`, `query-varies` or `body-unusable`, the unreachable `not-entity` reason leaves the result @@ -58,10 +59,11 @@ tool, view kind, provider, network activity or write effect. One owner in the environment source classifies each retained body as none, base64, cut, JSON or text; replay answers from that classification and descriptions read only its JSON case. -- MCP tools carry `readOnlyHint` (true for `GET`, `HEAD` and query `POST`s), +- MCP tools carry `readOnlyHint` (true for the safe methods `GET`, `HEAD`, + `OPTIONS` and `TRACE`, and for query `POST`s), `destructiveHint: true` for `DELETE` (unset for other writes, so the MCP default applies, and for reads, where clients ignore it), `idempotentHint` - (true for `GET`, `HEAD`, `PUT` and `DELETE`) and `openWorldHint: false`. The + (true for the safe methods, `PUT` and `DELETE`) and `openWorldHint: false`. The tool definitions own the hints and charge them to the catalog reservation; the environment card keeps its `name`, `description`, `inputSchema` tool shape. diff --git a/docs/web-network-captures.md b/docs/web-network-captures.md index 9345e8e0cd..eb95a988cf 100644 --- a/docs/web-network-captures.md +++ b/docs/web-network-captures.md @@ -583,7 +583,7 @@ Every entity cites its `record_ordinals`. | ---------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `item` | The entity object at `pointer` (the root, one member, or `/0` or `//0`), the ordered `fields` the view shows and the `required_fields` every recording had, and `envelope_ordinal`: the recording whose response, with the entity replaced, every item response shares | | `list` | The array at `pointer`, its `fields`, `membership` (`all`: every entity of the collection in state order, when each recording listed exactly that, had no query, no item-total member (`total`, `count`, `totalCount`, `item_count`, `totalcount`, `itemcount`, but not `total_pages` or `per_page`; a number or a string of digits) differing from the number listed, and no `has_more`, `next` or next-page token member set; `recorded`: the entities each recording listed) and `counters` that equal the number listed | -| `document` | The latest initial value of a list tool whose responses hold no identified list, when its initial reads share one request line; held in `documents` with that `path`, whether or not the capture writes it | +| `document` | The latest initial value of a list tool whose responses hold no identified list, when its initial reads share one request line; held in `documents` with that `path`, whether or not the capture writes it, unless its value changed across any recorded write between those reads (`fields-differ`) | | `recorded` | A `reason` the tool replays its recordings instead | An item or list plan is kept only when it reproduces the latest initial diff --git a/src/application/environment/environmentTools.test.ts b/src/application/environment/environmentTools.test.ts index ebd4afd58b..527a55cdce 100644 --- a/src/application/environment/environmentTools.test.ts +++ b/src/application/environment/environmentTools.test.ts @@ -521,3 +521,22 @@ describe("environmentCard", () => { expect(environmentCard("renamed", definitions).name).toBe("renamed"); }); }); + +describe("environment tool annotations", () => { + it("marks OPTIONS and TRACE safe and idempotent, as GET is", () => { + const get = tool("get_users_by_id"); + const hints = environmentToolDefinitions( + withTools( + (["OPTIONS", "TRACE"] as const).map((method) => ({ + ...get, + name: `${method.toLowerCase()}_users_by_id`, + method, + })), + ), + ).map((definition) => definition.annotations); + expect(hints).toEqual([ + { readOnlyHint: true, idempotentHint: true, openWorldHint: false }, + { readOnlyHint: true, idempotentHint: true, openWorldHint: false }, + ]); + }); +}); diff --git a/src/application/environment/environmentTools.ts b/src/application/environment/environmentTools.ts index e1dcb2848f..17fc288860 100644 --- a/src/application/environment/environmentTools.ts +++ b/src/application/environment/environmentTools.ts @@ -56,9 +56,16 @@ export interface EnvironmentToolDefinition { readonly tool: EnvironmentManifestTool; } -const IDEMPOTENT_METHODS: ReadonlySet = new Set([ +/** RFC 9110 safe methods: a preflight `OPTIONS` reads, as `GET` does. */ +const SAFE_METHODS: ReadonlySet = new Set([ "GET", "HEAD", + "OPTIONS", + "TRACE", +]); + +const IDEMPOTENT_METHODS: ReadonlySet = new Set([ + ...SAFE_METHODS, "PUT", "DELETE", ]); @@ -67,8 +74,7 @@ const environmentToolAnnotations = ({ method, path_template: path, }: EnvironmentManifestTool): EnvironmentToolAnnotations => ({ - readOnlyHint: - method === "GET" || method === "HEAD" || isQueryPost(method, path), + readOnlyHint: SAFE_METHODS.has(method) || isQueryPost(method, path), ...(method === "DELETE" ? { destructiveHint: true as const } : {}), idempotentHint: IDEMPOTENT_METHODS.has(method), openWorldHint: false, diff --git a/src/domain/analysisView/EnvironmentStateExtraction.ts b/src/domain/analysisView/EnvironmentStateExtraction.ts index 8a25199c26..047febe554 100644 --- a/src/domain/analysisView/EnvironmentStateExtraction.ts +++ b/src/domain/analysisView/EnvironmentStateExtraction.ts @@ -4,26 +4,27 @@ import { compositeKey, } from "../unicodeCodePointOrder.js"; import { - IDENTITY_NAMES, + apiBaseLengths, at, collectionPathOf, + hasPrototypeKey, identifies, + IDENTITY_NAMES, identityCandidates, isObject, isParam, isQueryPost, isStateWrite, jsonBytes, - hasPrototypeKey, lastSegmentValue, listItems, - normalizedPath, locateItem, locateList, + normalizedPath, rootOf, + same, scalarText, segmentsOf, - apiBaseLengths, writeSpanCheck, type ExtractedState, type JsonObject, @@ -518,9 +519,8 @@ export class EnvironmentStateExtraction { const documents: StateDocument[] = []; for (const { tool, item } of candidates) { if (item || this.#views.has(tool.name)) continue; - const latest = this.#latestPerLine( - this.#initRecordings(tool, tool.path_template), - ); + const recordings = this.#initRecordings(tool, tool.path_template); + const latest = this.#latestPerLine(recordings); if (latest.length !== 1) { const paths = new Set(latest.map((record) => record.path)); this.#recorded( @@ -534,6 +534,20 @@ export class EnvironmentStateExtraction { continue; } const record = latest[0]!; + // A value that changed across a write, even under another root, is not one initial state. + if ( + recordings.some( + (after, index) => + index > 0 && + this.#writeWithin(recordings[index - 1]!, after) && + this.#withBody(recordings[index - 1]!.ordinal, (before) => + this.#withBody(after.ordinal, (body) => !same(before, body)), + ), + ) + ) { + this.#recorded(tool, "fields-differ"); + continue; + } const value = this.#withBody(record.ordinal, (body) => body); if (value === undefined) { this.#recorded(tool, "body-unusable"); diff --git a/src/domain/analysisView/environmentStateView.test.ts b/src/domain/analysisView/environmentStateView.test.ts index 1d1869997b..1bb6d7c632 100644 --- a/src/domain/analysisView/environmentStateView.test.ts +++ b/src/domain/analysisView/environmentStateView.test.ts @@ -1097,3 +1097,30 @@ it("reads a sparse list once per candidate field when inferring identity", () => // Each identity name and suffix field stops at the first sparse item. expect(reads).toBeLessThanOrEqual(fields + 20); }); + +it("keeps a document out of the state when its value changed across a write under another root", () => { + const changed = stateOf( + captureOf([ + ["GET", "/api/me", { credits: 10, plan: "free" }], + ["POST", "/api/orders", { id: "o1" }], + ["GET", "/api/me", { credits: 9, plan: "free" }], + ]), + ); + expect(changed.documents).toEqual([]); + expect(changed.views).toContainEqual({ + tool: "get_me", + shape: "recorded", + reason: "fields-differ", + }); + // A value the write left unchanged is still one initial state. + const unchanged = stateOf( + captureOf([ + ["GET", "/api/me", { credits: 10, plan: "free" }], + ["POST", "/api/orders", { id: "o1" }], + ["GET", "/api/me", { credits: 10, plan: "free" }], + ]), + ); + expect(unchanged.documents).toMatchObject([ + { tool: "get_me", value: { credits: 10, plan: "free" }, record_ordinal: 2 }, + ]); +});