mirror of
https://github.com/morluto/rea
synced 2026-10-28 15:07:26 -04:00
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -583,7 +583,7 @@ Every entity cites its `record_ordinals`.
|
||||
| ---------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||||
| `item` | The entity object at `pointer` (the root, one member, or `/0` or `/<member>/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
|
||||
|
||||
@@ -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 },
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -56,9 +56,16 @@ export interface EnvironmentToolDefinition {
|
||||
readonly tool: EnvironmentManifestTool;
|
||||
}
|
||||
|
||||
const IDEMPOTENT_METHODS: ReadonlySet<string> = new Set([
|
||||
/** RFC 9110 safe methods: a preflight `OPTIONS` reads, as `GET` does. */
|
||||
const SAFE_METHODS: ReadonlySet<string> = new Set([
|
||||
"GET",
|
||||
"HEAD",
|
||||
"OPTIONS",
|
||||
"TRACE",
|
||||
]);
|
||||
|
||||
const IDEMPOTENT_METHODS: ReadonlySet<string> = 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,
|
||||
|
||||
@@ -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");
|
||||
|
||||
@@ -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 },
|
||||
]);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user