fix(environment)!: report recorded reads and target uncertainty (#7834)
* fix(environment)!: report recorded reads and target uncertainty BREAKING CHANGE: Environment state and coverage can report method-replay for non-GET safe reads; exhaustive reason consumers must handle it. * docs(environment): include safe-method replay reason
This commit is contained in:
1 parent
cc5a4062ff
commit
1e088c19ad
14 files changed
+205
-37
No files matched your search
@@ -235,17 +235,17 @@ Re-record a withheld write at most twice, and only for a recording gap. Stop at
|
||||
once when the reason is application behaviour, and report the write as
|
||||
withheld with its hint:
|
||||
|
||||
| Reason | What to do |
|
||||
| --------------------- | --------------------------------------------------------------------------------------------- |
|
||||
| `no-state-target` | Re-record: read the collection or item the write changes. |
|
||||
| `no-observation` | Re-record: read the entity back by id after the write; read it first for an update or delete. |
|
||||
| `unconfirmed` | Re-record: read the entity and its list after the write. |
|
||||
| `incomplete-entities` | Re-record: read the entity through every view that lists it. |
|
||||
| `unexplained-change` | Re-record once with a second, different call of the same write. |
|
||||
| `no-call` | Re-record once with a write that succeeds. |
|
||||
| `side-effect` | Stop: the application changes other entities. |
|
||||
| `contradicted` | Stop: a later read disagrees with the modelled effect. |
|
||||
| `body-unusable` | Re-record a UI write once with `request` steps (its body was not exposed); else stop. |
|
||||
| Reason | What to do |
|
||||
| --------------------- | --------------------------------------------------------------------------------------------------------------------------- |
|
||||
| `no-state-target` | Check the path template and recorded views; record missing reads. The route form may not be supported as a mutation target. |
|
||||
| `no-observation` | Re-record: read the entity back by id after the write; read it first for an update or delete. |
|
||||
| `unconfirmed` | Re-record: read the entity and its list after the write. |
|
||||
| `incomplete-entities` | Re-record: read the entity through every view that lists it. |
|
||||
| `unexplained-change` | Re-record once with a second, different call of the same write. |
|
||||
| `no-call` | Re-record once with a write that succeeds. |
|
||||
| `side-effect` | Stop: the application changes other entities. |
|
||||
| `contradicted` | Stop: a later read disagrees with the modelled effect. |
|
||||
| `body-unusable` | Re-record a UI write once with `request` steps (its body was not exposed); else stop. |
|
||||
|
||||
Use new scratch names on each attempt, and delete what an earlier attempt
|
||||
created before recording again (through the UI, or a `request` step with the
|
||||
|
||||
@@ -158,12 +158,12 @@ assigned by the app.
|
||||
|
||||
Use the report's hints to decide what to record next:
|
||||
|
||||
| Reason | Next step |
|
||||
| ------------------------------- | ---------------------------------------------------------------------------------------- |
|
||||
| `unconfirmed` | Read the item and its list after the change. |
|
||||
| `no-state-target` | Record reads of the collection or item the operation changes. |
|
||||
| `no-observation` | Read back a created item. For an edit or delete, also read it before changing it. |
|
||||
| `contradicted` or `side-effect` | Inspect the reported disagreement. The operation may need behavior REA cannot model yet. |
|
||||
| Reason | Next step |
|
||||
| ------------------------------- | --------------------------------------------------------------------------------------------------------------------------- |
|
||||
| `unconfirmed` | Read the item and its list after the change. |
|
||||
| `no-state-target` | Check the path template and recorded views; record missing reads. The route form may not be supported as a mutation target. |
|
||||
| `no-observation` | Read back a created item. For an edit or delete, also read it before changing it. |
|
||||
| `contradicted` or `side-effect` | Inspect the reported disagreement. The operation may need behavior REA cannot model yet. |
|
||||
|
||||
See the [full reason table](agent-environments.md#build-a-bundle-in-one-step)
|
||||
for the other cases. Rebuild from the new recording and check the report
|
||||
|
||||
@@ -174,7 +174,7 @@ what to record next; hints never change what is served:
|
||||
|
||||
| Reason | Hint |
|
||||
| --------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
|
||||
| `no-state-target` | The write's path names no collection or item the capture reads. Read the collection or item it changes. |
|
||||
| `no-state-target` | The write could not be linked to a modelled target. Check the path template and recorded views; record missing reads. Some route forms may not be supported. |
|
||||
| `no-call` | No recorded call of this tool was modelled, for example because every call failed. Record a successful call. |
|
||||
| `body-unusable` | The retained request or response cannot be modelled. Record a successful call with a complete supported request and reproducible response. |
|
||||
| `no-observation` | Nothing showed the entity or document this write changed. Read a created entity back by id right after the write; for an update or delete the state must already hold the entity, so read it before the write or record the create that made it. |
|
||||
@@ -723,6 +723,8 @@ and `POST` tools the capture shows only reading (a lookup answering as a `GET`
|
||||
did, an RPC tunnel naming read operations, a session renewal), stay served and
|
||||
answer from their recordings with the reason `query-body`; they are not write
|
||||
tools.
|
||||
`HEAD`, `OPTIONS` and `TRACE` tools also replay their captured responses; their
|
||||
state views report `method-replay` and coverage counts them as recorded reads.
|
||||
|
||||
A line establishes whole-collection membership only from the beginning:
|
||||
`offset`, `start`, `_start` and `page` must be zero when present. Positive or
|
||||
|
||||
@@ -14,6 +14,9 @@ cannot. This increment adds the view kind only; serving from the state, loading
|
||||
another state through the AgentEnv data plane and modelling write effects are
|
||||
later increments.
|
||||
|
||||
Safe `HEAD`, `OPTIONS` and `TRACE` tools have recorded views with reason
|
||||
`method-replay`; state rendering currently applies only to `GET` reads.
|
||||
|
||||
## Requirements
|
||||
|
||||
- Accept `inspect_web_network_capture` HAR parents and
|
||||
|
||||
@@ -689,7 +689,8 @@ with a `value` or `absent: true`), and serving replays calls matching them and
|
||||
calls asking for fields the checked lines do not show.
|
||||
Otherwise the tool is a recorded view
|
||||
with reason `envelope-varies`, `fields-differ` or `list-differs`; the other
|
||||
reasons are `query-body` (a query `POST`, or one the capture shows only reading), `no-init-recording`,
|
||||
reasons are `query-body` (a query `POST`, or one the capture shows only reading),
|
||||
`method-replay` (a safe `HEAD`, `OPTIONS` or `TRACE` call), `no-init-recording`,
|
||||
`body-unusable`, `no-identity` (an item read without a proved identity),
|
||||
`path-varies` (another read under more than one path, as a template with a
|
||||
parameter before its end), `query-varies` (another read of one path under
|
||||
|
||||
@@ -2,6 +2,7 @@ import {
|
||||
ownMember,
|
||||
isObject,
|
||||
isQueryPost,
|
||||
isSafeHttpMethod,
|
||||
listItems,
|
||||
locateList,
|
||||
pointerTokens,
|
||||
@@ -64,27 +65,14 @@ export interface EnvironmentToolDefinition {
|
||||
readonly tool: EnvironmentManifestTool;
|
||||
}
|
||||
|
||||
/** 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",
|
||||
]);
|
||||
|
||||
const environmentToolAnnotations = ({
|
||||
method,
|
||||
path_template: path,
|
||||
}: EnvironmentManifestTool): EnvironmentToolAnnotations => ({
|
||||
readOnlyHint: SAFE_METHODS.has(method) || isQueryPost(method, path),
|
||||
readOnlyHint: isSafeHttpMethod(method) || isQueryPost(method, path),
|
||||
...(method === "DELETE" ? { destructiveHint: true as const } : {}),
|
||||
idempotentHint: IDEMPOTENT_METHODS.has(method),
|
||||
idempotentHint:
|
||||
isSafeHttpMethod(method) || method === "PUT" || method === "DELETE",
|
||||
openWorldHint: false,
|
||||
});
|
||||
|
||||
|
||||
@@ -16,6 +16,7 @@ import {
|
||||
identityCandidates,
|
||||
isObject,
|
||||
isQueryPost,
|
||||
isSafeHttpMethod,
|
||||
stateValueRetentionBytes,
|
||||
lastSegmentValue,
|
||||
STATE_BODY_LIMIT_BYTES,
|
||||
@@ -331,7 +332,7 @@ export class EnvironmentStateExtraction {
|
||||
this.#views.set(tool.name, { tool: tool.name, shape: "recorded", reason });
|
||||
}
|
||||
|
||||
/** GET tools may render from state; query POSTs stay recorded. */
|
||||
/** GET tools may render from state; other safe methods and query POSTs replay. */
|
||||
#candidates(): Candidate[] {
|
||||
const candidates: Candidate[] = [];
|
||||
for (const tool of this.#tools) {
|
||||
@@ -340,6 +341,8 @@ export class EnvironmentStateExtraction {
|
||||
this.#readOnly.has(tool.name)
|
||||
)
|
||||
this.#recorded(tool, "query-body");
|
||||
else if (tool.method !== "GET" && isSafeHttpMethod(tool.method))
|
||||
this.#recorded(tool, "method-replay");
|
||||
if (tool.method !== "GET") continue;
|
||||
candidates.push({ tool, ...readType(tool.path_template) });
|
||||
}
|
||||
|
||||
@@ -2,6 +2,7 @@ import { expect, it } from "vitest";
|
||||
import {
|
||||
analysisViewCaptureEvidence,
|
||||
environmentStateCaptureFixture,
|
||||
timedCaptureEvidence,
|
||||
} from "../../../tests/fixtures/analysisView.js";
|
||||
import {
|
||||
SCENARIO_EXCHANGES,
|
||||
@@ -169,6 +170,40 @@ it("summarizes the state the environment-state view derives with the same rules"
|
||||
);
|
||||
});
|
||||
|
||||
it("does not claim reads are missing when a literal delete has no modelled target", () => {
|
||||
const evidence = timedCaptureEvidence([
|
||||
["GET", "/api/folders", [{ uid: "f1", title: "General" }]],
|
||||
["GET", "/api/folders/f1", { uid: "f1", title: "General" }],
|
||||
["DELETE", "/api/folders/f1", null, 204],
|
||||
["GET", "/api/folders/f1", { message: "not found" }, 404],
|
||||
["GET", "/api/folders", []],
|
||||
]);
|
||||
const parent = parentOf(evidence);
|
||||
const state = project("environment-state", parent);
|
||||
const coverage = project("environment-coverage", parent);
|
||||
if (
|
||||
state.kind !== "environment-state" ||
|
||||
coverage.kind !== "environment-coverage"
|
||||
)
|
||||
throw new Error("Expected state and coverage views");
|
||||
expect(state.views).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({ tool: "get_folders", shape: "list" }),
|
||||
expect.objectContaining({ tool: "get_folders_f1", shape: "document" }),
|
||||
]),
|
||||
);
|
||||
expect(coverage.writes.withheld).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
reason: "no-state-target",
|
||||
hint: expect.stringContaining(
|
||||
"could not be linked to a modelled state target",
|
||||
),
|
||||
}),
|
||||
]),
|
||||
);
|
||||
});
|
||||
|
||||
it("says an untimed capture learns no clock, and only for an untimed capture", () => {
|
||||
const untimed = /learns no clock/u;
|
||||
const scenarioParent = parentOf(
|
||||
|
||||
@@ -29,7 +29,7 @@ export const environmentCoverageViewRequestSchema = z.strictObject({
|
||||
/** What to record next for each reason a write tool is withheld; advice only, it never changes what is served. */
|
||||
export const WITHHELD_WRITE_HINTS = {
|
||||
"no-state-target":
|
||||
"The write's path names no collection or item the capture reads. Read the collection or item it changes.",
|
||||
"The write could not be linked to a modelled state target. Check its path template and the recorded collection or item views; if the relevant reads are missing, record them. Some route forms may not be supported as mutation targets.",
|
||||
"no-call":
|
||||
"No recorded call of this tool was modelled, for example because every call failed. Record a successful call.",
|
||||
"body-unusable":
|
||||
|
||||
@@ -73,6 +73,7 @@ export interface StateBudget {
|
||||
|
||||
export type RecordedViewReason =
|
||||
| "query-body"
|
||||
| "method-replay"
|
||||
| "no-init-recording"
|
||||
| "body-unusable"
|
||||
| "no-identity"
|
||||
@@ -268,6 +269,13 @@ export interface ExtractedState {
|
||||
}
|
||||
|
||||
const WRITE_METHODS = new Set(["POST", "PUT", "PATCH", "DELETE"]);
|
||||
|
||||
/** HTTP safe methods share read-only classification across state and MCP views. */
|
||||
export const isSafeHttpMethod = (method: string): boolean =>
|
||||
method === "GET" ||
|
||||
method === "HEAD" ||
|
||||
method === "OPTIONS" ||
|
||||
method === "TRACE";
|
||||
/** A POST whose path names one of these segments queries state rather than changing it. */
|
||||
export const QUERY_SEGMENTS = new Set([
|
||||
"search",
|
||||
|
||||
@@ -220,6 +220,7 @@ const viewSchema = z.discriminatedUnion("shape", [
|
||||
shape: z.literal("recorded"),
|
||||
reason: z.enum([
|
||||
"query-body",
|
||||
"method-replay",
|
||||
"no-init-recording",
|
||||
"body-unusable",
|
||||
"no-identity",
|
||||
|
||||
@@ -920,6 +920,30 @@ it("loads a padded inline state file and keeps the RPC result compact", async ()
|
||||
});
|
||||
|
||||
describe("serveEnvironment with a state", () => {
|
||||
it("replays safe non-GET methods with the recorded-view reason", async () => {
|
||||
const environment = await serveState(
|
||||
environmentStateServeInputs(
|
||||
environmentStateServeFixture(
|
||||
timedCaptureEvidence([
|
||||
["GET", "/api/items", [{ id: 1 }]],
|
||||
["HEAD", "/api/items"],
|
||||
["OPTIONS", "/api/items", { allow: "GET, HEAD, OPTIONS" }],
|
||||
["TRACE", "/api/items", { trace: true }],
|
||||
]),
|
||||
),
|
||||
),
|
||||
);
|
||||
await withClient(environment, async (client) => {
|
||||
for (const name of ["head_items", "options_items", "trace_items"]) {
|
||||
const response = await client.callTool({ name, arguments: {} });
|
||||
expect(response.isError).not.toBe(true);
|
||||
expect(response.structuredContent).toMatchObject({
|
||||
state: { source: "recording", reason: "method-replay" },
|
||||
});
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
it("declares the data plane, serves read tools from the state and reflects data/add in the next answer", async () => {
|
||||
const environment = await serveState({ source, state });
|
||||
const url = new URL(environment.url);
|
||||
|
||||
@@ -1771,6 +1771,72 @@ it("reports environment coverage identically from retained, portable and file MC
|
||||
await client.ping();
|
||||
});
|
||||
|
||||
it("accounts for every safe HTTP method in environment state and coverage", async () => {
|
||||
const { client } = await connect();
|
||||
const { ajv, outputSchema } = await advertisedMcpToolSchemas(
|
||||
client,
|
||||
"inspect_analysis_view",
|
||||
schemaValidation.assertions,
|
||||
);
|
||||
const entries = ["GET", "HEAD", "OPTIONS", "TRACE"].map((method) => ({
|
||||
request: { method, url: "https://sample.test/api/items" },
|
||||
response: {
|
||||
status: 200,
|
||||
content:
|
||||
method === "GET"
|
||||
? {
|
||||
mimeType: "application/json",
|
||||
text: '{"items":[{"id":"a"}]}',
|
||||
size: 22,
|
||||
}
|
||||
: { mimeType: "application/json", size: 0 },
|
||||
},
|
||||
}));
|
||||
const parent = analysisViewCaptureEvidence(
|
||||
environmentManifestCaptureFixture(entries),
|
||||
);
|
||||
const results = [];
|
||||
for (const kind of [
|
||||
"environment-manifest",
|
||||
"environment-state",
|
||||
"environment-coverage",
|
||||
]) {
|
||||
const response = await client.callTool({
|
||||
name: "inspect_analysis_view",
|
||||
arguments: {
|
||||
source: { kind: "inline", evidence: parent },
|
||||
view: { kind, template_rules: [] },
|
||||
},
|
||||
});
|
||||
expect(response.isError).not.toBe(true);
|
||||
expect(
|
||||
ajv.validate(outputSchema, response.structuredContent),
|
||||
JSON.stringify(ajv.errors),
|
||||
).toBe(true);
|
||||
results.push(parseEvidence(response.structuredContent).normalized_result);
|
||||
}
|
||||
expect(results[0]).toMatchObject({
|
||||
tools: expect.arrayContaining([
|
||||
expect.objectContaining({ method: "GET" }),
|
||||
expect.objectContaining({ method: "HEAD" }),
|
||||
expect.objectContaining({ method: "OPTIONS" }),
|
||||
expect.objectContaining({ method: "TRACE" }),
|
||||
]),
|
||||
});
|
||||
expect(results[1]).toMatchObject({
|
||||
views: expect.arrayContaining([
|
||||
{ tool: "head_items", shape: "recorded", reason: "method-replay" },
|
||||
{ tool: "options_items", shape: "recorded", reason: "method-replay" },
|
||||
{ tool: "trace_items", shape: "recorded", reason: "method-replay" },
|
||||
]),
|
||||
});
|
||||
expect(results[2]).toMatchObject({
|
||||
reads: { state: 1, recorded: 3, recorded_reasons: { "method-replay": 3 } },
|
||||
writes: { served: [], withheld: [] },
|
||||
coverage: { examined: 4, total: 4, exhausted: true },
|
||||
});
|
||||
});
|
||||
|
||||
it("projects an environment manifest identically from retained, portable and file MCP Evidence", async () => {
|
||||
const { client, session } = await connect();
|
||||
const { ajv, inputSchema, outputSchema } = await advertisedMcpToolSchemas(
|
||||
|
||||
@@ -17,6 +17,7 @@ import { advertisedMcpToolSchemas } from "../../fixtures/advertisedMcpTool.js";
|
||||
import {
|
||||
analysisViewCaptureEvidence,
|
||||
environmentManifestCaptureFixture,
|
||||
timedCaptureEvidence,
|
||||
} from "../../fixtures/analysisView.js";
|
||||
import { createTestBinarySession } from "../../fixtures/binarySession.js";
|
||||
import { parseMcpToolError } from "../../fixtures/mcpToolError.js";
|
||||
@@ -68,6 +69,42 @@ const loopbackCapture = (): Evidence =>
|
||||
) as ReturnType<typeof environmentManifestCaptureFixture>,
|
||||
);
|
||||
|
||||
it("reports an unmodelled literal delete without claiming recorded reads are absent", async () => {
|
||||
const { client, session } = await connect();
|
||||
const capture = timedCaptureEvidence([
|
||||
["GET", "/api/folders", [{ uid: "f1", title: "General" }]],
|
||||
["GET", "/api/folders/f1", { uid: "f1", title: "General" }],
|
||||
["DELETE", "/api/folders/f1", null, 204],
|
||||
["GET", "/api/folders/f1", { message: "not found" }, 404],
|
||||
["GET", "/api/folders", []],
|
||||
]);
|
||||
expect(session.recordEvidence(capture).ok).toBe(true);
|
||||
const root = await createTestTempDirectory("rea-bundle-mcp-target-");
|
||||
const response = await client.callTool({
|
||||
name: TOOL,
|
||||
arguments: {
|
||||
source: { kind: "retained-evidence", evidence_id: capture.evidence_id },
|
||||
output_directory: join(root, "bundle"),
|
||||
dry_run: true,
|
||||
},
|
||||
});
|
||||
expect(response.isError, JSON.stringify(response)).not.toBe(true);
|
||||
const report = toolContract(TOOL).outputSchema.parse(
|
||||
response.structuredContent,
|
||||
);
|
||||
if (report.coverage === null) throw new Error("Expected coverage");
|
||||
expect(report.coverage.writes.withheld).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
reason: "no-state-target",
|
||||
hint: expect.stringContaining(
|
||||
"could not be linked to a modelled state target",
|
||||
),
|
||||
}),
|
||||
]),
|
||||
);
|
||||
});
|
||||
|
||||
it("writes the bundle environment-build writes from retained capture Evidence", async () => {
|
||||
const { client, session } = await connect();
|
||||
const { ajv, inputSchema, outputSchema } = await advertisedMcpToolSchemas(
|
||||
|
||||
Reference in new issue
Block a user