From d2416ca5425905992a66903692b09db2769cab59 Mon Sep 17 00:00:00 2001 From: gensokyo-dev Date: Mon, 19 Oct 2026 21:44:50 +0000 Subject: [PATCH] fix(environment): admit scenario text expansion before decoding --- .../scenario-capture/spec.md | 6 ++ .../scenario-capture/verification.md | 52 ++++++++++++++ scripts/ci/scopes.mjs | 2 +- .../environment/environmentExport.test.ts | 9 ++- .../environment/environmentSource.ts | 2 +- .../WebNetworkProjectionAccumulator.ts | 12 +++- .../analysisView/analysisViewDataAdmission.ts | 10 ++- .../scenarioCaptureRecords.test.ts | 68 +++++++++++++++++-- .../analysisView/scenarioCaptureRecords.ts | 43 ++++++------ .../boundary/cli/environmentExportCli.test.ts | 7 +- .../boundary/filesystem/ciChangeScope.test.ts | 12 ++++ 11 files changed, 188 insertions(+), 35 deletions(-) diff --git a/docs/specs/agent-environments/scenario-capture/spec.md b/docs/specs/agent-environments/scenario-capture/spec.md index 843fccd77f..243782969e 100644 --- a/docs/specs/agent-environments/scenario-capture/spec.md +++ b/docs/specs/agent-environments/scenario-capture/spec.md @@ -31,6 +31,12 @@ change. unrepresented records, as HAR records without a usable request line are. - The response media type is the captured body's `media_type`, or the `content-type` header value when only headers were captured. +- The existing projection representation budget accounts for copied and + transformed strings before schema body validation or request-body decoding, + including retained Evidence parents. Over-budget captures return + `resource_constraint` before those allocations. Replay projection omits the + request text used only for manifest body-shape inference; retained response + bytes and their integrity checks are unchanged. - The manifest result names `parent_operation: "capture_browser_scenario"`, carries an empty `artifact` (`path: ""`, `sha256: null`) and states, as limitations, the ordinal rule, any dropped capture events and how many diff --git a/docs/specs/agent-environments/scenario-capture/verification.md b/docs/specs/agent-environments/scenario-capture/verification.md index 76531bd09e..3383efe38b 100644 --- a/docs/specs/agent-environments/scenario-capture/verification.md +++ b/docs/specs/agent-environments/scenario-capture/verification.md @@ -88,3 +88,55 @@ These were observed during the runs and are outside this increment: cancelled. Every scenario above sets `timeout_ms` on its actions. - A redirect is its own transaction; Django's trailing-slash 301 produced a second tool (`get_pins_by_id` and `get_pins_by_id_2`) for one resource. + +## Linux followup verification (2026-10-19) + +The original PR was merged with current main `7a0c47bfa`, after #4955, #4975 +and #4999 merged. The export filesystem ownership, mode, tarball, cleanup and +caller-path fixes from main are retained. Default MCP remains stdio. + +The retained-parent projection previously decoded every request body before +per-record budget checks. It now reserves copied/transformed text through the +shared admission owner before schema validation or decoding, including retained +parents. Replay projection omits request text used only for body-shape inference; +response bytes still undergo the producer schema's integrity validation. The +intermediate projected-array copy and unused capture wrapper were removed. +Windows test inventories normalize filesystem separators to the portable bundle +path representation. CI selects the agentenv lane for the new projection and its +shared admission dependencies. + +Executed on Linux x64, Node 24.18.0 / npm 11.16.0, Chrome for Testing +156.0.8078.4: + +- `turbo run typecheck lint:built format:check docs:check:built knip artifacts:skills artifacts:mcp-catalog --concurrency=1 --summarize`: eleven tasks passed, one cached, 210.023 s elapsed / 209.860 s CPU / 4562.07 MiB peak process-family RSS. This first complete gate preceded the final lint cleanup and CI-selection additions; later checks are reported on the PR. +- Initial source verification passed 111 tests in 13 files, including HAR/manifest regressions, scenario loading/replay/export and new capacity/replay-mode tests. CI-selection verification passed 235 cases. +- Real public CLI capture of the source-owned interactive browser network site: ten transactions, fifty events, two tools, no dropped events. Its POST response replayed twice through the official HTTP SDK with exact body/status/Evidence ordinal and untrusted-content provenance. Declared secrets and transport credentials were excluded; the ordinary selected `password` field value remained in retained body bytes. +- Real public CLI capture of three live GitHub API responses in Chromium, without a HAR: three transactions, fifteen events, three tools, no dropped events. Every response replayed twice through the HTTP SDK with exact body/status/Evidence/provenance. Explicit navigation waits let response-body capture settle. Complete two-scenario capture/projection/serve task: **22.527 s / 16.678 s CPU / 1397.93 MiB** peak process-family RSS, including owned browser processes. Both listeners exited 143 and newly owned browser profiles were gone. +- The same two actual captures produced identical manifest results through CLI and official SDK **stdio** MCP, with HTTP environment variables present: **2.795 s / 2.778 s CPU / 476.67 MiB**. This verifies adapter parity and default transport independently of explicit environment HTTP. + +The first local probe mistakenly searched base64 body content for plaintext; its +assertion was corrected to inspect decoded retained bytes. A later SDK probe +initially imported stdio transport from the SDK root instead of its documented +subpath; the corrected probe passed. An early live navigation also reported one +`body-read-failed` response, which the projection preserved as unavailable. No +browser-provider behavior was changed; the successful run used explicit waits. +These earlier attempts are not counted as passing runs. + +A matched retained-scenario projection comparison bundles the original helper +from `3aae3579e` and the followup helper, supplies the same 72,771-byte capture +with three JSON writes, and completes one cold plus 100 warm full manifest +projections. Both outputs have the same SHA-256 +`990ac431670374ff68c77a20d77985f626e4681bb0e2f491e5af18f45ce79991`. +Before/after process totals: **0.970/0.876 s elapsed, 0.934/0.865 s CPU, +184.89/184.39 MiB** RSS. First task: **17.378/13.073 ms**; 100 warm tasks: +**261.419/236.131 ms**. Single samples do not establish a speed or memory +improvement. Bundle preparation is excluded; process totals include fixture +startup. CLI, provider and transport costs are excluded from this comparison. + +Heavy work is serialized on one CPU with one test worker. RSS sums sampled +process descendants every 50 ms and can double-count shared pages. Agent token +usage is unmeasured. These followups do not rerun the author's five application +reconstruction/model tasks, nor establish Windows or macOS behavior. Historical +response replay does not reconstruct application state. + +A retained-parent counterexample used a 128 KiB projection budget and one 16 KiB JSON request: the original helper admitted it and decoded once; the repaired helper returned `AnalysisResourceConstraintError` with zero UTF-8 decodes. The test intentionally uses a small budget without inducing an OOM. Final targeted verification passed 242 tests (including 235 CI-selection cases): 41.042 s / 40.666 s CPU / 644.37 MiB peak process-family RSS. diff --git a/scripts/ci/scopes.mjs b/scripts/ci/scopes.mjs index aeda08504e..8cd4a99533 100644 --- a/scripts/ci/scopes.mjs +++ b/scripts/ci/scopes.mjs @@ -448,7 +448,7 @@ export const SCOPES = { "src/server/http/**", "src/{cli,cliCommandNames,cli/environmentCommands}.ts", "src/config/{environmentName,mcpHttpEndpoint}.ts", - "src/domain/analysisView/{e,E}nvironmentManifest*.ts", + "src/domain/analysisView/{environmentManifest*,EnvironmentManifest*,scenarioCaptureRecords,WebNetworkProjectionAccumulator,analysisViewDataAdmission}.ts", "src/process/{OwnedCommand,ProviderProcess,PrivateRuntimeRoot}.ts", "src/process/ProcessOwnership*.ts", "tests/fixtures/historicalHar.ts", diff --git a/src/application/environment/environmentExport.test.ts b/src/application/environment/environmentExport.test.ts index c7d15a5076..bb053ef326 100644 --- a/src/application/environment/environmentExport.test.ts +++ b/src/application/environment/environmentExport.test.ts @@ -1,5 +1,5 @@ import { mkdir, readdir, readFile, stat, writeFile } from "node:fs/promises"; -import { join } from "node:path"; +import { join, sep } from "node:path"; import * as v8 from "node:v8"; import { describe, expect, it, vi } from "vitest"; @@ -26,7 +26,12 @@ vi.mock("../RegularFileRead.js", { spy: true }); const listFiles = async (root: string): Promise => (await readdir(root, { recursive: true, withFileTypes: true })) .filter((entry) => entry.isFile()) - .map((entry) => join(entry.parentPath, entry.name).slice(root.length + 1)) + .map((entry) => + join(entry.parentPath, entry.name) + .slice(root.length + 1) + .split(sep) + .join("/"), + ) .sort(); const prepare = async (prefix: string) => { diff --git a/src/application/environment/environmentSource.ts b/src/application/environment/environmentSource.ts index 03ef40858b..569c2299ea 100644 --- a/src/application/environment/environmentSource.ts +++ b/src/application/environment/environmentSource.ts @@ -231,7 +231,7 @@ const parseCaptureEvidence = ( .safeParse(evidence.value.parameters); if (scenario.success) return ok({ - records: scenarioCaptureRecords(scenario.data).records, + records: scenarioCaptureRecords(scenario.data, "replay").records, evidenceId, captureKind: "browser-scenario", captureSha256: definition.success diff --git a/src/domain/analysisView/WebNetworkProjectionAccumulator.ts b/src/domain/analysisView/WebNetworkProjectionAccumulator.ts index 153832d5f8..aa867eb267 100644 --- a/src/domain/analysisView/WebNetworkProjectionAccumulator.ts +++ b/src/domain/analysisView/WebNetworkProjectionAccumulator.ts @@ -63,9 +63,15 @@ export abstract class WebNetworkProjectionAccumulator { } /** Reserve schema output and validation scratch while the borrowed parent stays live. */ - admitCaptureClone(value: unknown): Result { - return admitAnalysisViewData(value, false, (bytes) => - this.retain(bytes) ? ok(undefined) : this.overBudget(), + admitCaptureClone( + value: unknown, + options?: { readonly copiedStrings: boolean }, + ): Result { + return admitAnalysisViewData( + value, + false, + (bytes) => (this.retain(bytes) ? ok(undefined) : this.overBudget()), + options?.copiedStrings, ); } diff --git a/src/domain/analysisView/analysisViewDataAdmission.ts b/src/domain/analysisView/analysisViewDataAdmission.ts index ad39d23418..a4847c0ae2 100644 --- a/src/domain/analysisView/analysisViewDataAdmission.ts +++ b/src/domain/analysisView/analysisViewDataAdmission.ts @@ -10,12 +10,14 @@ const MEMBER_BYTES = 256; const CONTAINER_BYTES = 512; const textBytes = (length: number): number => 32 + length * 14; -/** Inspect borrowed data without invoking accessors; only API-surface reserves bytes. */ +/** Inspect borrowed JSON without accessors and reserve copied or transformed representations. */ export const admitAnalysisViewData = ( value: unknown, inline: boolean, reserve?: (bytes: number) => Result, + copiedStrings = false, ): Result => { + const countStrings = inline || copiedStrings; const ancestors = new Set(); // A single bounded stack preserves diagnostics without per-node path copies. const path: (string | number)[] = inline @@ -43,7 +45,9 @@ export const admitAnalysisViewData = ( return invalid(); const member = reserve?.( MEMBER_BYTES + - (inline && typeof item === "string" ? textBytes(item.length) : 0) + + (countStrings && typeof item === "string" + ? textBytes(item.length) + : 0) + (typeof item === "object" && item !== null ? CONTAINER_BYTES : 0), ); if (member !== undefined && !member.ok) return member; @@ -83,7 +87,7 @@ export const admitAnalysisViewData = ( // Zod reads named fields even when they are non-enumerable. for (const key of Object.getOwnPropertyNames(item)) { // Authentication hashes strings and keys alongside the schema clone. - const keyAdmission = inline + const keyAdmission = countStrings ? reserve?.(textBytes(key.length)) : undefined; if (keyAdmission !== undefined && !keyAdmission.ok) return keyAdmission; diff --git a/src/domain/analysisView/scenarioCaptureRecords.test.ts b/src/domain/analysisView/scenarioCaptureRecords.test.ts index 769937d03c..a43ccb26b2 100644 --- a/src/domain/analysisView/scenarioCaptureRecords.test.ts +++ b/src/domain/analysisView/scenarioCaptureRecords.test.ts @@ -1,14 +1,21 @@ -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { SCENARIO_EXCHANGES, scenarioCaptureFixture, } from "../../../tests/fixtures/browserScenarioCapture.js"; -import { scenarioCaptureRecords } from "./scenarioCaptureRecords.js"; +import { + admitScenarioCaptureParent, + scenarioCaptureRecords, +} from "./scenarioCaptureRecords.js"; +import { EnvironmentManifestAccumulator } from "./EnvironmentManifestAccumulator.js"; describe("scenarioCaptureRecords", () => { it("projects each transaction, in request order, as a HAR-shaped record with its retained bytes", () => { - const projected = scenarioCaptureRecords(scenarioCaptureFixture()); + const projected = scenarioCaptureRecords( + scenarioCaptureFixture(), + "manifest", + ); expect(projected.totalRecords).toBe(SCENARIO_EXCHANGES.length); expect(projected.records.map((record) => record.ordinal)).toEqual([ 0, 1, 2, 3, 4, 5, 6, 7, @@ -66,6 +73,59 @@ describe("scenarioCaptureRecords", () => { items: [...capture.events.items, { ...response, sequence: 99 }], }, }; - expect(scenarioCaptureRecords(duplicated).records[0]?.reported).toEqual({}); + expect( + scenarioCaptureRecords(duplicated, "manifest").records[0]?.reported, + ).toEqual({}); }); }); + +it("reserves retained request-body expansion before schema decoding or projection", () => { + const capture = scenarioCaptureFixture([ + { + method: "POST", + url: "/api/items", + status: 201, + mediaType: "application/json", + body: "{}", + requestMediaType: "application/json", + requestBody: JSON.stringify({ value: "x".repeat(16 * 1024) }), + }, + ]); + const decoder = vi.spyOn(TextDecoder.prototype, "decode"); + try { + const projected = admitScenarioCaptureParent( + { + evidenceId: `ev_${"a".repeat(64)}`, + operation: "capture_browser_scenario", + normalizedResult: capture, + limitations: [], + }, + new EnvironmentManifestAccumulator(128 * 1024), + ); + expect(projected).toMatchObject({ + ok: false, + error: { _tag: "AnalysisResourceConstraintError", resource: "memory" }, + }); + expect(decoder).not.toHaveBeenCalled(); + } finally { + decoder.mockRestore(); + } +}); + +it("does not decode request text when projecting replay records", () => { + const capture = scenarioCaptureFixture(); + const decoder = vi.spyOn(TextDecoder.prototype, "decode"); + try { + const projected = scenarioCaptureRecords(capture, "replay"); + expect(projected.records[4]?.reported).toMatchObject({ + request: { method: "POST", url: "http://127.0.0.1:3101/api/folders" }, + }); + expect(projected.records[4]?.reported).not.toHaveProperty( + "request.postData", + ); + expect(decoder).not.toHaveBeenCalled(); + expect(projected.records[4]?.binary_fields).toHaveLength(1); + } finally { + decoder.mockRestore(); + } +}); diff --git a/src/domain/analysisView/scenarioCaptureRecords.ts b/src/domain/analysisView/scenarioCaptureRecords.ts index 91c604e123..f6e2d1ae3c 100644 --- a/src/domain/analysisView/scenarioCaptureRecords.ts +++ b/src/domain/analysisView/scenarioCaptureRecords.ts @@ -115,8 +115,9 @@ const mediaType = (content: ContentEvent | undefined): string | undefined => const reportedRequest = ( request: RequestEvent, content: ContentEvent | undefined, + purpose: "manifest" | "replay", ): { readonly [key: string]: JsonValue } => { - const body = captured(content); + const body = purpose === "manifest" ? captured(content) : undefined; const text = body === undefined ? undefined : utf8Text(body); const mimeType = mediaType(content); return { @@ -130,10 +131,11 @@ const reportedRequest = ( const record = ( ordinal: number, - item: number, - request: RequestEvent, + observed: TransactionIndex["requests"][number], index: TransactionIndex, + purpose: "manifest" | "replay", ): { readonly record: WebNetworkCaptureRecord; readonly body: boolean } => { + const { item, event: request } = observed; const location = { kind: "json-pointer" as const, pointer: `/events/items/${String(item)}`, @@ -168,7 +170,7 @@ const record = ( ordinal, location, reported: { - request: reportedRequest(request, requestContent), + request: reportedRequest(request, requestContent, purpose), ...(response === undefined ? {} : { @@ -203,15 +205,19 @@ const record = ( /** Project each network transaction, in request order, as one record; the ordinal is its zero-based position. */ export const scenarioCaptureRecords = ( capture: BrowserScenarioCapture, + purpose: "manifest" | "replay", ): ScenarioCaptureRecords => { const index = indexTransactions(capture.events.items); - const projected = index.requests.map(({ event, item }, ordinal) => - record(ordinal, item, event, index), - ); - const withoutBody = projected.filter((entry) => !entry.body).length; + const records: WebNetworkCaptureRecord[] = []; + let withoutBody = 0; + for (const [ordinal, observed] of index.requests.entries()) { + const projected = record(ordinal, observed, index, purpose); + records.push(projected.record); + if (!projected.body) withoutBody += 1; + } return { - records: projected.map((entry) => entry.record), - totalRecords: projected.length, + records, + totalRecords: records.length, limitations: [ "Scenario records are the capture's network transactions in request order; each record ordinal is the transaction's zero-based position.", ...(capture.events.dropped > 0 @@ -221,24 +227,23 @@ export const scenarioCaptureRecords = ( : []), ...(withoutBody > 0 ? [ - `${String(withoutBody)} of ${String(projected.length)} scenario transactions retained no response body; their tools replay status and content type only.`, + `${String(withoutBody)} of ${String(records.length)} scenario transactions retained no response body; their tools replay status and content type only.`, ] : []), ], }; }; -/** A scenario capture parent in the shape the environment manifest projects from. */ -export interface ScenarioCaptureParent extends ScenarioCaptureRecords { - readonly capture: BrowserScenarioCapture; -} - /** Admit a capture_browser_scenario parent whose network content identifies response media types. */ export const admitScenarioCaptureParent = ( parent: AnalysisViewParent, accumulator: WebNetworkProjectionAccumulator, -): Result => { - const admitted = accumulator.admitCaptureClone(parent.normalizedResult); +): Result => { + // Body validation decodes base64, and projection constructs UTF-8 request text. + // Reserve their expanded strings before either allocation, even for retained parents. + const admitted = accumulator.admitCaptureClone(parent.normalizedResult, { + copiedStrings: true, + }); if (!admitted.ok) return admitted; const parsed = browserScenarioCaptureSchema.safeParse( parent.normalizedResult, @@ -264,5 +269,5 @@ export const admitScenarioCaptureParent = ( "environment-manifest needs a capture_browser_scenario capture with capture.network.response_body or header_values selected.", ), ); - return ok({ capture: parsed.data, ...scenarioCaptureRecords(parsed.data) }); + return ok(scenarioCaptureRecords(parsed.data, "manifest")); }; diff --git a/tests/boundary/cli/environmentExportCli.test.ts b/tests/boundary/cli/environmentExportCli.test.ts index 064a8358dd..7f6dae529f 100644 --- a/tests/boundary/cli/environmentExportCli.test.ts +++ b/tests/boundary/cli/environmentExportCli.test.ts @@ -6,7 +6,7 @@ import { symlink, writeFile, } from "node:fs/promises"; -import { join } from "node:path"; +import { join, sep } from "node:path"; import { describe, expect } from "vitest"; @@ -106,7 +106,10 @@ describe("rea environment-export", () => { ) .filter((entry) => entry.isFile()) .map((entry) => - join(entry.parentPath, entry.name).slice(directory.length + 1), + join(entry.parentPath, entry.name) + .slice(directory.length + 1) + .split(sep) + .join("/"), ) .sort(); expect(written).toEqual(BUNDLE_FILES); diff --git a/tests/boundary/filesystem/ciChangeScope.test.ts b/tests/boundary/filesystem/ciChangeScope.test.ts index 107eb1449d..874b4ade29 100644 --- a/tests/boundary/filesystem/ciChangeScope.test.ts +++ b/tests/boundary/filesystem/ciChangeScope.test.ts @@ -806,6 +806,18 @@ it.each([ "scripts/verify-agentenv-environment.mjs", { code: true, agentenv: true, runtime: false, package: false }, ], + [ + "src/domain/analysisView/scenarioCaptureRecords.ts", + { code: true, agentenv: true, runtime: true, package: false }, + ], + [ + "src/domain/analysisView/WebNetworkProjectionAccumulator.ts", + { code: true, agentenv: true, runtime: true, package: false }, + ], + [ + "src/domain/analysisView/analysisViewDataAdmission.ts", + { code: true, agentenv: true, runtime: true, package: false }, + ], [ "src/application/OwnedJsonInput.ts", { code: true, agentenv: true, runtime: true, package: false },