From dae1e55381f0d6cfee7f2082d5d27b938e55704c Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 14 Oct 2026 17:49:13 +0800 Subject: [PATCH 01/20] fix(javascript): preserve lexical binding and default export capture semantics Resolve CommonJS wrapper bindings from the parser-owned source type. Capture default identifier export values at the export statement while preserving live named bindings. Refs: #3261, #3297 Co-authored-by: hhang <169027046+xu91102@users.noreply.github.com> --- ...riptSemanticAnalysis.require_links.test.ts | 17 ++++++++ .../javascript/javascriptSemanticAnalysis.ts | 1 + .../javascriptSemanticProjection.ts | 20 ++++++--- .../javascript/javascriptSemanticReturns.ts | 38 +++++++++++++---- .../javascript/javascriptSemanticState.ts | 9 +++- .../exportedFunctionBindingsCli.test.ts | 42 +++++++++++++++++++ 6 files changed, 111 insertions(+), 16 deletions(-) diff --git a/src/domain/javascript/javascriptSemanticAnalysis.require_links.test.ts b/src/domain/javascript/javascriptSemanticAnalysis.require_links.test.ts index 831c9ebc41..f05d7a7587 100644 --- a/src/domain/javascript/javascriptSemanticAnalysis.require_links.test.ts +++ b/src/domain/javascript/javascriptSemanticAnalysis.require_links.test.ts @@ -62,6 +62,23 @@ describe("TypeScript import-equals links", () => { }); describe("CommonJS wrapper parameter redeclarations", () => { + it("does not infer a CommonJS wrapper from resolution mode in an explicit module", () => { + const path = "app/main.mjs"; + const parsed = parseJavaScriptSource( + 'var require; require("./dependency.cjs");', + path, + ); + expect(parsed).not.toBeNull(); + expect(parsed?.program.sourceType).toBe("module"); + const ir = analyzeParsedJavaScriptSemantics( + parsed!, + moduleResolutionModeForJavaScriptReference(path, "static-import"), + ); + expect(ir.moduleLinks).not.toContainEqual( + expect.objectContaining({ specifier: "./dependency.cjs" }), + ); + }); + it("retains ambient exports and require through uninitialized top-level vars", () => { const path = "app/main.cjs"; const parsed = parseJavaScriptSource( diff --git a/src/domain/javascript/javascriptSemanticAnalysis.ts b/src/domain/javascript/javascriptSemanticAnalysis.ts index 9947191e80..147d05f608 100644 --- a/src/domain/javascript/javascriptSemanticAnalysis.ts +++ b/src/domain/javascript/javascriptSemanticAnalysis.ts @@ -687,6 +687,7 @@ const createState = ( moduleLinks: [], linkedRequireCalls: new WeakSet(), moduleLinkBindings: new WeakMap(), + defaultExportExpressions: new WeakMap(), conditionallySkippedExpressions: new WeakSet(), requiredModuleEffects: new Map(), ambientBindingWrites: new Map(), diff --git a/src/domain/javascript/javascriptSemanticProjection.ts b/src/domain/javascript/javascriptSemanticProjection.ts index 59e92b98dd..2493a05c65 100644 --- a/src/domain/javascript/javascriptSemanticProjection.ts +++ b/src/domain/javascript/javascriptSemanticProjection.ts @@ -102,7 +102,7 @@ export const collectSemanticModuleLink = ( kind: "export", specifier: null, importedName: null, - localName: defaultDeclarationName(node.declaration), + localName: defaultExportLocalName(node.declaration), exportedName: "default", callableId: semanticCallableIdForNode(node.declaration), location: range(node), @@ -557,6 +557,11 @@ const addModuleLink = ( callableId: link.callableId ?? null, }; state.moduleLinks.push(collected); + if ( + t.isExportDefaultDeclaration(referenceNode) && + t.isExpression(referenceNode.declaration) + ) + state.defaultExportExpressions.set(collected, referenceNode.declaration); if (link.localName !== null) { const binding = resolveSemanticBindingState( state, @@ -816,13 +821,16 @@ const declarationNames = (declaration: t.Declaration): string[] => { return []; }; -const defaultDeclarationName = ( +const defaultExportLocalName = ( declaration: t.ExportDefaultDeclaration["declaration"], -): string | null => - (t.isFunctionDeclaration(declaration) || t.isClassDeclaration(declaration)) && - t.isIdentifier(declaration.id) - ? declaration.id.name +): string | null => { + const value = unwrapJavaScriptExpression(declaration).node; + if (t.isIdentifier(value)) return value.name; + return (t.isFunctionDeclaration(value) || t.isClassDeclaration(value)) && + t.isIdentifier(value.id) + ? value.id.name : null; +}; const commonJsExportName = ( node: t.Node, diff --git a/src/domain/javascript/javascriptSemanticReturns.ts b/src/domain/javascript/javascriptSemanticReturns.ts index 11aaddc09d..50adf38ef1 100644 --- a/src/domain/javascript/javascriptSemanticReturns.ts +++ b/src/domain/javascript/javascriptSemanticReturns.ts @@ -28,6 +28,7 @@ import { evaluateSemanticExpression } from "./javascriptSemanticValues.js"; import { range } from "./javascriptStaticAnalysisHelpers.js"; import { semanticReturnCoverage } from "./javascriptSemanticCoverage.js"; import { semanticCallableResultExpressions } from "./javascriptSemanticCallableResults.js"; +import { semanticReadInitializers } from "./javascriptSemanticMutationInitializers.js"; interface ReturnExpression { readonly node: t.Node | null; @@ -104,15 +105,18 @@ export const resolveSemanticModuleCallables = ( if (link.kind === "type-import") return { ...link, callableId: null }; // Named exports follow their lexical binding, including every assignment. // A declaration's ID or a callable's display name cannot prove that link. - if (link.localName === null) + const captured = state.defaultExportExpressions.get(link); + if (link.localName === null && captured === undefined) return link.callableId !== null && callableIds.has(link.callableId) ? link : { ...link, callableId: null }; const binding = state.moduleLinkBindings.get(link); const resolved = - binding === undefined - ? [] - : callableIdsForBinding(binding, state, callableIds, new Set()); + captured !== undefined + ? callableIdsForNode(captured, state, callableIds, new Set(), true) + : binding === undefined + ? [] + : callableIdsForBinding(binding, state, callableIds, new Set()); const provenance = binding === undefined || link.specifier !== null ? undefined @@ -139,21 +143,29 @@ const callableIdsForBinding = ( state: JavaScriptSemanticAnalysisState, admitted: ReadonlySet, seen: ReadonlySet, + read?: t.Node, ): string[] => { // A single initializer does not prove an assignment executes or replace // the unknown incoming value of a parameter/catch binding. if ( seen.has(binding.bindingId) || - binding.initializers.length !== 1 || + (read === undefined && binding.initializers.length !== 1) || binding.definitions.some( ({ kind }) => - kind === "assignment" || kind === "parameter" || kind === "catch", + (read === undefined && kind === "assignment") || + kind === "parameter" || + kind === "catch", ) ) return []; - const initializer = binding.initializers[0]; + const selected = + read === undefined + ? binding + : semanticReadInitializers(binding, read, state.parentsByNode); + const initializer = selected.initializers[0]; if ( initializer === undefined || + selected.initializers.length !== 1 || initializer.projection.length > 0 || state.conditionallySkippedExpressions.has(initializer.node) ) @@ -163,6 +175,7 @@ const callableIdsForBinding = ( state, admitted, new Set([...seen, binding.bindingId]), + read !== undefined, ); }; @@ -171,17 +184,24 @@ const callableIdsForNode = ( state: JavaScriptSemanticAnalysisState, admitted: ReadonlySet, seen: ReadonlySet, + snapshot = false, ): string[] => { const unwrapped = unwrapJavaScriptExpression(node).node; if (unwrapped !== node) - return callableIdsForNode(unwrapped, state, admitted, seen); + return callableIdsForNode(unwrapped, state, admitted, seen, snapshot); const direct = semanticCallableIdForNode(node); if (direct !== null && admitted.has(direct)) return [direct]; if (t.isIdentifier(node)) { const binding = resolveSemanticBindingState(state, node, node.name); return binding === undefined ? [] - : callableIdsForBinding(binding, state, admitted, seen); + : callableIdsForBinding( + binding, + state, + admitted, + seen, + snapshot ? node : undefined, + ); } return []; }; diff --git a/src/domain/javascript/javascriptSemanticState.ts b/src/domain/javascript/javascriptSemanticState.ts index 2d38c45964..4f2b091a11 100644 --- a/src/domain/javascript/javascriptSemanticState.ts +++ b/src/domain/javascript/javascriptSemanticState.ts @@ -94,6 +94,11 @@ export interface JavaScriptSemanticAnalysisState { JavaScriptSemanticModuleLink, JavaScriptSemanticBindingState >; + /** Default expressions capture their value at the export statement. */ + readonly defaultExportExpressions: WeakMap< + JavaScriptSemanticModuleLink, + t.Node + >; /** Initializer expressions whose write can be skipped by enclosing control flow. */ readonly conditionallySkippedExpressions: WeakSet; /** Unbound identifier writes retain their source sites for capture ordering. */ @@ -263,8 +268,10 @@ export const isUnmodifiedCommonJsWrapperBinding = ( name: string, state: JavaScriptSemanticAnalysisState, ): boolean => { + // Parser sourceType separates explicit modules from script/CommonJS source; + // specifier resolution mode does not establish that distinction. if ( - state.staticImportResolutionMode !== "require" || + state.program.sourceType !== "script" || !["module", "exports", "require"].includes(name) || !t.isIdentifier(node, { name }) ) diff --git a/tests/acceptance/applications/exportedFunctionBindingsCli.test.ts b/tests/acceptance/applications/exportedFunctionBindingsCli.test.ts index d7fac06987..0271a016f8 100644 --- a/tests/acceptance/applications/exportedFunctionBindingsCli.test.ts +++ b/tests/acceptance/applications/exportedFunctionBindingsCli.test.ts @@ -462,6 +462,48 @@ describe("stable exported function bindings through stdio MCP", () => { const fixture = await analyzeVersions(source, "default"); expectCountChange(await compareThroughMcp(fixture.input)); }); + + it.each([ + { + name: "a local callable captured by an identifier default export", + source: (count: number) => + `const run = () => ({ kind: "result", count: ${String(count)} });\nexport default run;`, + runtimeValue: (count: number) => ({ kind: "result", count }), + expected: "known", + }, + { + name: "the latest callable assigned before the default export", + source: (count: number) => + `let run = () => ({ count: 0 });\nrun = () => ({ kind: "result", count: ${String(count)} });\nexport default run;`, + runtimeValue: (count: number) => ({ kind: "result", count }), + expected: "known", + }, + { + name: "a callable assigned only after the default export capture", + source: (count: number) => + `var run;\nexport default run;\nrun = () => ({ kind: "result", count: ${String(count)} });`, + runtimeValue: () => undefined, + expected: "unknown", + }, + { + name: "a reassignment after the default export capture", + source: (count: number) => + `let run = () => ({ kind: "result", count: ${String(count)} });\nexport default run;\nrun = () => ({ count: 99 });`, + runtimeValue: (count: number) => ({ kind: "result", count }), + expected: "known", + }, + ] as const)( + "uses the value at $name", + async ({ source, runtimeValue, expected }) => { + const fixture = await analyzeVersions(source, "default"); + await expectRuntimeExports(fixture, runtimeValue); + const mcp = await compareThroughMcp(fixture.input); + const cli = await compareThroughCli(fixture); + expect(cli).toEqual(mcp); + if (expected === "known") expectCountChange(mcp); + else expectUncertainComparison(mcp); + }, + ); }); describe("export bindings with loop and assignment-only writes", () => { From 28f57619738e41cc5e9c8b8e50692231d238a6bb Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 14 Oct 2026 17:49:13 +0800 Subject: [PATCH 02/20] test(mcp): isolate target-free sessions from host provider discovery Inject explicit unavailable provider ports into the fake-session harness, forbid accidental execution, and retain public availability coverage. This removes host-dependent timeouts without increasing deadlines. --- tests/boundary/mcp/sessionMcp.test.ts | 27 +++++++++++++++++ tests/boundary/mcp/sessionMcpHarness.ts | 39 +++++++++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/tests/boundary/mcp/sessionMcp.test.ts b/tests/boundary/mcp/sessionMcp.test.ts index 2c36275029..0bad24e576 100644 --- a/tests/boundary/mcp/sessionMcp.test.ts +++ b/tests/boundary/mcp/sessionMcp.test.ts @@ -217,6 +217,33 @@ describe("public provider availability", () => { }); }); +describe("target-free MCP fixture host isolation", () => { + it("uses fixed unavailable ports instead of probing installed Android tools", async () => { + directory = await createTestTempDirectory("rea-mcp-host-isolation-"); + const { mcp } = await createSessionMcpHarness( + directory, + provider, + resources, + ); + + const status = structured( + await mcp.callTool({ name: "binary_session", arguments: {} }), + ); + const availability = toolAvailabilityInventory.parse( + (status.result as { tool_availability: unknown }).tool_availability, + ); + for (const name of [ + "inspect_android_package", + "inspect_adb_client", + "inspect_apktool_client", + ]) { + expect( + availability.groups.find((group) => group.names.includes(name)), + ).toMatchObject({ available: false, reason: "provider_missing" }); + } + }); +}); + describe("target-free MCP lifecycle", () => { it("takes the session snapshot after Android readiness inspection completes", async () => { directory = await createTestTempDirectory("rea-mcp-status-readiness-"); diff --git a/tests/boundary/mcp/sessionMcpHarness.ts b/tests/boundary/mcp/sessionMcpHarness.ts index 45f298c765..f1bca7b54c 100644 --- a/tests/boundary/mcp/sessionMcpHarness.ts +++ b/tests/boundary/mcp/sessionMcpHarness.ts @@ -10,6 +10,10 @@ import { z } from "zod"; import { createTestBinarySession } from "../../fixtures/binarySession.js"; import { createAnalysisProfile } from "../../../src/domain/analysisProfile.js"; import { createEvidence } from "../../../src/domain/evidence.js"; +import type { ProviderAvailability } from "../../../src/application/AnalysisProvider.js"; +import type { AndroidAnalysisPort } from "../../../src/application/android/AndroidAnalysisPort.js"; +import type { AdbDeviceAnalysisPort } from "../../../src/application/adb/AdbDeviceAnalysisPort.js"; +import type { ApktoolResourceAnalysisPort } from "../../../src/application/apktool/ApktoolResourceAnalysisPort.js"; import { createEvidenceBundle, evidenceBundleSchema, @@ -24,6 +28,38 @@ const SNAPSHOT_PROFILE = createAnalysisProfile( { fixture: true }, ); +const HOST_TOOLS_NOT_CONFIGURED: ProviderAvailability = { + status: "unavailable", + code: "not_configured", + reason: "The target-free MCP fixture does not probe host tools.", + diagnostics: { source: "test_fixture" }, +}; + +const unusedHostToolExecution = async (): Promise => { + throw new Error("Host-tool execution is outside the target-free fixture"); +}; + +// These fixed ports keep this fake-session harness independent of whichever +// Android tools happen to be installed on the test host. The dedicated public +// availability test below continues to exercise status projection separately. +const androidAnalysis: AndroidAnalysisPort = { + inspectAvailability: async () => HOST_TOOLS_NOT_CONFIGURED, + close: async () => {}, + execute: unusedHostToolExecution, +}; + +const adbDeviceAnalysis: AdbDeviceAnalysisPort = { + inspectAvailability: async () => HOST_TOOLS_NOT_CONFIGURED, + close: async () => {}, + execute: unusedHostToolExecution, +}; + +const apktoolAnalysis: ApktoolResourceAnalysisPort = { + inspectAvailability: async () => HOST_TOOLS_NOT_CONFIGURED, + close: async () => {}, + execute: unusedHostToolExecution, +}; + export interface SessionMcpHarness { readonly mcp: Client; readonly first: string; @@ -47,6 +83,9 @@ export const createSessionMcpHarness = async ( const server = createServer( { kind: "session", session }, { + androidAnalysis, + adbDeviceAnalysis, + apktoolAnalysis, logger: silentLogger, }, ); From 503109995f9dcf8a955585d39a344fc846d2f59e Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 14 Oct 2026 17:49:14 +0800 Subject: [PATCH 03/20] fix(javascript): retain early-return conditions in wrapped call paths Share sequential fallthrough interpretation between block traversal and isolated seed context. Refs: #3196 --- .../javascript/electronWrappedCallPaths.ts | 62 ++++++++----- .../cli/electronWrappedCallPaths.test.ts | 87 +++++++++++++++++++ 2 files changed, 127 insertions(+), 22 deletions(-) diff --git a/src/domain/javascript/electronWrappedCallPaths.ts b/src/domain/javascript/electronWrappedCallPaths.ts index deb922ea69..3d38bbd484 100644 --- a/src/domain/javascript/electronWrappedCallPaths.ts +++ b/src/domain/javascript/electronWrappedCallPaths.ts @@ -108,6 +108,32 @@ export const collectElectronWrappedCallPaths = ( ], }; }; + const sequentialFallthrough = ( + statement: t.Statement, + env: SymbolicEnvironment, + context: PathContext, + ): { readonly continues: boolean; readonly context: PathContext } => { + if (symbolicDefiniteExit(statement)) return { continues: false, context }; + if (!t.isIfStatement(statement)) return { continues: true, context }; + const truth = symbolicTruth(resolver.evaluate(statement.test, env)); + if (truth !== undefined) + return symbolicDefiniteExit( + truth ? statement.consequent : statement.alternate, + ) + ? { continues: false, context } + : { continues: true, context }; + if (symbolicDefiniteExit(statement.consequent)) + return { + continues: true, + context: condition(context, statement.test, false), + }; + if (symbolicDefiniteExit(statement.alternate)) + return { + continues: true, + context: condition(context, statement.test), + }; + return { continues: true, context }; + }; const metadataContext = ( context: PathContext, value: SymbolicValue, @@ -534,21 +560,9 @@ export const collectElectronWrappedCallPaths = ( let next = context; for (const statement of node.body) { walk(statement, env, next); - if (symbolicDefiniteExit(statement)) break; - if (t.isIfStatement(statement)) { - const truth = symbolicTruth(resolver.evaluate(statement.test, env)); - if (truth !== undefined) { - if ( - symbolicDefiniteExit( - truth ? statement.consequent : statement.alternate, - ) - ) - break; - } else if (symbolicDefiniteExit(statement.consequent)) - next = condition(next, statement.test, false); - else if (symbolicDefiniteExit(statement.alternate)) - next = condition(next, statement.test); - } + const fallthrough = sequentialFallthrough(statement, env, next); + if (!fallthrough.continues) break; + next = fallthrough.context; } return; } @@ -639,13 +653,17 @@ export const collectElectronWrappedCallPaths = ( const index = parent.body.findIndex( (statement) => statement === child, ); - if ( - index >= 0 && - parent.body - .slice(0, index) - .some((statement) => symbolicDefiniteExit(statement)) - ) - return; + if (index >= 0) { + for (const statement of parent.body.slice(0, index)) { + const fallthrough = sequentialFallthrough( + statement, + env, + context, + ); + if (!fallthrough.continues) return; + context = fallthrough.context; + } + } } } call(node, env, context, isolatedSeed?.callee); diff --git a/tests/boundary/cli/electronWrappedCallPaths.test.ts b/tests/boundary/cli/electronWrappedCallPaths.test.ts index 128b8188e9..b7ed2b67c5 100644 --- a/tests/boundary/cli/electronWrappedCallPaths.test.ts +++ b/tests/boundary/cli/electronWrappedCallPaths.test.ts @@ -9,6 +9,19 @@ import { javascriptApplicationAnalysisResultSchema } from "../../../src/domain/j import { applicationFeatureTraceResultSchema } from "../../../src/domain/javascript/javascriptFeatureTraceSchemas.js"; import { analyzeJavaScriptDirectory } from "../../support/cli/javascriptApplication.js"; +const sourcePositionSchema = z.object({ line: z.number(), column: z.number() }); +const callPathConditionSchema = z.object({ + expression: z.string(), + location: z.object({ + start: sourcePositionSchema, + end: sourcePositionSchema, + }), +}); +const callPathTerminalObservationSchema = z.object({ + properties: z.object({ conditions: z.array(callPathConditionSchema) }), + evidence: z.object({ coverage: z.object({ status: z.string() }) }), +}); + const analyze = async (cli: TestCli, extra = "") => { const root = await createTestTempDirectory("rea-electron-wrapped-paths-"); await writeFixtureFiles(root, { @@ -123,6 +136,80 @@ cliTest( }, ); +cliTest( + "preserves early-return predicates on recovered bridge paths through CLI and stdio MCP", + async ({ cli }) => { + const cases = [ + { + name: "conditional early return", + body: 'if(x)return;e.contextBridge.exposeInMainWorld("api",{copy:()=>e.ipcRenderer.send("x")});', + conditions: ["!(x)"], + coverage: "partial", + }, + { + name: "equivalent nested branch", + body: 'if(!x){e.contextBridge.exposeInMainWorld("api",{copy:()=>e.ipcRenderer.send("x")});}', + conditions: ["!x"], + coverage: "partial", + }, + { + name: "definite early return", + body: 'return;e.contextBridge.exposeInMainWorld("api",{copy:()=>e.ipcRenderer.send("x")});', + conditions: [], + coverage: "complete", + }, + { + name: "unguarded exposure", + body: 'e.contextBridge.exposeInMainWorld("api",{copy:()=>e.ipcRenderer.send("x")});', + conditions: [], + coverage: "complete", + }, + ] as const; + + for (const surface of ["CLI", "stdio MCP"] as const) + for (const fixture of cases) { + const root = await createTestTempDirectory( + "rea-electron-early-return-", + ); + await writeFixtureFiles(root, { + "index.js": `const e=require("electron");function f(x){${fixture.body}}f(globalThis.blocked);`, + }); + const application = await analyzeJavaScriptDirectory( + surface, + cli, + root, + ); + const observations = application.graph.nodes.flatMap( + (node) => node.observations, + ); + const terminal = observations.find( + (observation) => + observation.properties.entry === "api.copy" && + observation.properties.step_kind === "terminal", + ); + if (fixture.name === "definite early return") { + expect(terminal).toBeUndefined(); + continue; + } + const terminalEvidence = + callPathTerminalObservationSchema.parse(terminal); + expect( + terminalEvidence.properties.conditions.map( + ({ expression }) => expression, + ), + ).toEqual(fixture.conditions); + expect(terminalEvidence.evidence.coverage.status).toBe( + fixture.coverage, + ); + expect(application.graph.coverage.status).toBe(fixture.coverage); + const source = observations.find( + (observation) => observation.properties.file_kind === "javascript", + ); + expect(source?.properties.parse_status).toBe(fixture.coverage); + } + }, +); + cliTest( "traces a nested contextBridge member through a unique IPC channel", async ({ cli }) => { From 44c9d2924d5b95d7a86595e8d7b7cf4fd4f0f6ac Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 14 Oct 2026 17:49:14 +0800 Subject: [PATCH 04/20] fix(javascript): match feature seeds against their owning source values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Apply exact, contains, and case options to node IDs. Route seeds consume retained route values rather than generated display labels. Refs: #3189, #3190, #3298 Co-authored-by: João Pedro de Faria --- .../javascript/javascriptFeatureSeed.ts | 40 +-- ...scriptFeatureTraceSeedSelectionCli.test.ts | 256 ++++++++++++++++++ 2 files changed, 279 insertions(+), 17 deletions(-) create mode 100644 tests/acceptance/applications/javascriptFeatureTraceSeedSelectionCli.test.ts diff --git a/src/domain/javascript/javascriptFeatureSeed.ts b/src/domain/javascript/javascriptFeatureSeed.ts index d5f06b8172..2cd238487d 100644 --- a/src/domain/javascript/javascriptFeatureSeed.ts +++ b/src/domain/javascript/javascriptFeatureSeed.ts @@ -40,17 +40,9 @@ const seedMatch = ( node: ApplicationNode, seed: ApplicationFeatureSeed, ): ApplicationFeatureSeedMatch[] => { - if (seed.kind === "node-id") - return node.node_id === seed.value - ? [ - { - node_id: node.node_id, - kind: node.kind, - basis: "node-id", - field: "node_id", - }, - ] - : []; + const mode = featureSeedMatchMode(seed); + const caseSensitive = seed.case_sensitive; + const query = normalized(seed.value, caseSensitive); if (!kindMatches(node, seed.kind)) return []; const requestedEntryPath = seed.kind === "api" && "entry_path" in seed ? seed.entry_path : undefined; @@ -69,9 +61,6 @@ const seedMatch = ( }) ) return []; - const mode = featureSeedMatchMode(seed); - const caseSensitive = seed.case_sensitive; - const query = normalized(seed.value, caseSensitive); const match = candidateFields(node, seed.kind).find((candidate) => { const value = normalized(candidate.value, caseSensitive); return mode === "exact" ? value === query : value.includes(query); @@ -90,9 +79,9 @@ const seedMatch = ( const kindMatches = ( node: ApplicationNode, - kind: Exclude, + kind: ApplicationFeatureSeed["kind"], ): boolean => { - if (kind === "string") return true; + if (kind === "string" || kind === "node-id") return true; if (kind === "route") return ( node.kind === "endpoint" && @@ -116,8 +105,25 @@ const kindMatches = ( const candidateFields = ( node: ApplicationNode, - seedKind: Exclude, + seedKind: ApplicationFeatureSeed["kind"], ): CandidateField[] => { + if (seedKind === "node-id") + return [{ basis: "node-id", field: "node_id", value: node.node_id }]; + if (seedKind === "route") + return node.observations.flatMap((observation, index) => + (["value", "path"] as const).flatMap((key) => { + const value = observation.properties[key]; + return typeof value === "string" + ? [ + { + basis: "property" as const, + field: `observations[${String(index)}].properties.${key}`, + value, + }, + ] + : []; + }), + ); const fields: CandidateField[] = [ ...identityFields(node), ...node.observations.flatMap((observation, index) => [ diff --git a/tests/acceptance/applications/javascriptFeatureTraceSeedSelectionCli.test.ts b/tests/acceptance/applications/javascriptFeatureTraceSeedSelectionCli.test.ts new file mode 100644 index 0000000000..a0438556d7 --- /dev/null +++ b/tests/acceptance/applications/javascriptFeatureTraceSeedSelectionCli.test.ts @@ -0,0 +1,256 @@ +import { Client } from "@modelcontextprotocol/client"; +import { StdioClientTransport } from "@modelcontextprotocol/client/stdio"; +import { writeFile } from "node:fs/promises"; +import { join, resolve } from "node:path"; +import { expect } from "vitest"; +import { z } from "zod"; + +import { toolContract } from "../../../src/contracts/toolContracts.js"; +import { + applicationFeatureSeedSchema, + applicationFeatureTraceResultSchema, +} from "../../../src/domain/javascript/javascriptFeatureTraceSchemas.js"; +import { javascriptApplicationAnalysisResultSchema } from "../../../src/domain/javascript/javascriptApplicationAnalysis.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { cliTest, type TestCli } from "../../support/cli/cliFixture.js"; + +type FeatureSeed = z.input; +type Trace = z.infer; +type SeedScenario = { + readonly name: string; + readonly seed: FeatureSeed; + readonly expected: { + readonly nodeId: string; + readonly basis: "node-id" | "property"; + } | null; +}; + +const createSeedScenarios = ( + graph: z.infer["graph"], +): SeedScenario[] => { + const notificationRoute = graph.nodes.find( + (node) => + node.kind === "endpoint" && + node.observations.some( + ({ properties }) => + properties.endpoint_kind === "route" && + properties.value === "/accounts/preferences/notifications", + ), + ); + const tinyRoute = graph.nodes.find( + (node) => + node.kind === "endpoint" && + node.observations.some( + ({ properties }) => + properties.endpoint_kind === "route" && properties.value === "/tiny", + ), + ); + const labeledNode = graph.nodes.find((node) => + node.observations.some( + ({ label }) => label !== null && label !== node.node_id, + ), + ); + if (notificationRoute === undefined || tinyRoute === undefined) + throw new Error( + "Expected literal route endpoints in the application graph", + ); + if (labeledNode === undefined) + throw new Error("Expected a labeled node in the application graph"); + const label = labeledNode.observations.find( + ({ label: value }) => value !== null && value !== labeledNode.node_id, + )?.label; + if (label === undefined || label === null) + throw new Error("Expected the selected graph node to have a display label"); + + const id = labeledNode.node_id; + return [ + { + name: "node ID contains prefix", + seed: { kind: "node-id", value: id.slice(0, 24), match: "contains" }, + expected: { nodeId: id, basis: "node-id" }, + }, + { + name: "node ID exact ignores case by default", + seed: { kind: "node-id", value: id.toUpperCase(), match: "exact" }, + expected: { nodeId: id, basis: "node-id" }, + }, + { + name: "node ID exact can be case sensitive", + seed: { + kind: "node-id", + value: id.toUpperCase(), + match: "exact", + case_sensitive: true, + }, + expected: null, + }, + { + name: "node ID never falls back to a display label", + seed: { kind: "node-id", value: label, match: "exact" }, + expected: null, + }, + { + name: "generated route label exact", + seed: { + kind: "route", + value: "route endpoint", + match: "exact", + case_sensitive: true, + }, + expected: null, + }, + { + name: "generated route label substring", + seed: { kind: "route", value: "route", match: "contains" }, + expected: null, + }, + { + name: "preserved long route exact", + seed: { + kind: "route", + value: "/accounts/preferences/notifications", + match: "exact", + }, + expected: { nodeId: notificationRoute.node_id, basis: "property" }, + }, + { + name: "preserved route substring", + seed: { + kind: "route", + value: "/accounts/preferences", + match: "contains", + }, + expected: { nodeId: notificationRoute.node_id, basis: "property" }, + }, + { + name: "short preserved route exact", + seed: { kind: "route", value: "/tiny", match: "exact" }, + expected: { nodeId: tinyRoute.node_id, basis: "property" }, + }, + ]; +}; + +const analyzeFixture = async (cli: TestCli, root: string) => { + const response = await cli.run({ + arguments: [ + "analyze-javascript-application", + root, + "--artifact-format", + "directory", + "--json", + ], + }); + expect(response.exitCode).toBe(0); + const evidence = toolContract( + "analyze_javascript_application", + ).outputSchema.parse(response.json); + return { + evidence, + result: javascriptApplicationAnalysisResultSchema.parse( + evidence.normalized_result, + ), + }; +}; + +const traceThroughCli = async ( + cli: TestCli, + application: unknown, + scenario: SeedScenario, +): Promise => { + const response = await cli.run({ + arguments: [ + "trace-application-feature", + JSON.stringify({ + application, + seed: scenario.seed, + direction: "both", + }), + "--json", + ], + }); + expect(response.exitCode, `${scenario.name} via CLI`).toBe(0); + const evidence = toolContract("trace_application_feature").outputSchema.parse( + response.json, + ); + return applicationFeatureTraceResultSchema.parse(evidence.normalized_result); +}; + +const traceThroughMcp = async ( + client: Client, + application: unknown, + scenario: SeedScenario, +): Promise => { + const response = await client.callTool({ + name: "trace_application_feature", + arguments: { + application, + seed: scenario.seed, + direction: "both", + }, + }); + expect(response.isError, `${scenario.name} via MCP`).not.toBe(true); + const evidence = toolContract("trace_application_feature").outputSchema.parse( + response.structuredContent, + ); + return applicationFeatureTraceResultSchema.parse(evidence.normalized_result); +}; + +const expectTrace = (trace: Trace, scenario: SeedScenario): void => { + if (scenario.expected === null) { + expect(trace.seed_matches).toEqual([]); + expect(trace.summary.matched_seeds).toBe(0); + expect(trace.coverage.status).toBe("no-match"); + return; + } + expect(trace.seed_matches).toHaveLength(1); + expect(trace.seed_matches[0]).toMatchObject({ + node_id: scenario.expected.nodeId, + basis: scenario.expected.basis, + }); + expect(trace.summary.matched_seeds).toBe(1); + expect(trace.coverage.status).toBe("complete-within-source"); + if (scenario.seed.kind === "node-id") + expect(trace.seed_matches[0]).toMatchObject({ + basis: "node-id", + field: "node_id", + }); + else + expect(trace.seed_matches[0]?.field).toMatch( + /\.properties\.(value|path)$/u, + ); +}; + +cliTest( + "matches feature seeds against node IDs and preserved route values through CLI and MCP", + async ({ cli }) => { + const root = await createTestTempDirectory("rea-feature-seed-selection-"); + await writeFile( + join(root, "routes.js"), + `const notificationRoute = { path: "/accounts/preferences/notifications" };\nconst tinyRoute = { path: "/tiny" };\nmodule.exports = { notificationRoute, tinyRoute };\n`, + ); + const { evidence, result } = await analyzeFixture(cli, root); + const client = new Client({ name: "feature-seed-selection", version: "1" }); + const transport = new StdioClientTransport({ + command: process.execPath, + args: [resolve("scripts/rea.mjs"), "mcp"], + cwd: process.cwd(), + env: { PATH: process.env.PATH ?? "", REA_LOG_LEVEL: "silent" }, + stderr: "pipe", + }); + transport.stderr?.on("data", () => undefined); + try { + await client.connect(transport); + for (const scenario of createSeedScenarios(result.graph)) { + const cliTrace = await traceThroughCli(cli, evidence, scenario); + const mcpTrace = await traceThroughMcp(client, evidence, scenario); + expectTrace(cliTrace, scenario); + expectTrace(mcpTrace, scenario); + expect(mcpTrace.seed_matches).toEqual(cliTrace.seed_matches); + expect(mcpTrace.coverage).toEqual(cliTrace.coverage); + } + } finally { + await client.close(); + await transport.close(); + } + }, +); From 5e79d468e5864204cbe79a7ca2898541cee5e012 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 14 Oct 2026 17:49:15 +0800 Subject: [PATCH 05/20] fix(javascript): retain runtime candidate ambiguity Keep referenced ambiguous candidates unknown instead of classifying them as absent. Refs: #3192 --- .../javascript/javascriptRuntimeLoadState.ts | 11 ++ .../javascriptRuntimeReconciliationSchemas.ts | 1 + tests/boundary/cli/electronCli.test.ts | 121 ++++++++++++++++++ 3 files changed, 133 insertions(+) diff --git a/src/domain/javascript/javascriptRuntimeLoadState.ts b/src/domain/javascript/javascriptRuntimeLoadState.ts index 838f606f64..d2d2cab43a 100644 --- a/src/domain/javascript/javascriptRuntimeLoadState.ts +++ b/src/domain/javascript/javascriptRuntimeLoadState.ts @@ -46,6 +46,7 @@ const classifyLayer = ( ): { readonly states: JavaScriptStaticLoadState[] } => { const nodes = layer.graph.nodes.filter(isLoadStateNode); const direct = new Map(); + const ambiguous = new Set(); for (const match of matches) if ( match.status === "matched" && @@ -53,6 +54,10 @@ const classifyLayer = ( match.static_node_id !== null ) append(direct, match.static_node_id, match.runtime_node_id); + else if (match.status === "ambiguous") + for (const candidate of match.candidate_static_nodes) + if (candidate.static_layer_id === layer.layerId) + ambiguous.add(candidate.static_node_id); const resident = residentNodes(layer, new Set(direct.keys())); const runtimeScope = createStaticRuntimeScope(layer, runtimeEntities); const scopedEntities = runtimeEntities.filter( @@ -77,6 +82,12 @@ const classifyLayer = ( runtimeNodeIds: [], reason: "containing-asset-was-loaded", }); + if (ambiguous.has(node.node_id)) + return state(layer, node, { + status: "unknown", + runtimeNodeIds: [], + reason: "ambiguous-runtime-candidate", + }); const nodeWithinScope = staticNodeWithinRuntimeScope(node, runtimeScope); if (layerComplete && nodeWithinScope) return state(layer, node, { diff --git a/src/domain/javascript/javascriptRuntimeReconciliationSchemas.ts b/src/domain/javascript/javascriptRuntimeReconciliationSchemas.ts index 684a4a4d2c..976b82dbe5 100644 --- a/src/domain/javascript/javascriptRuntimeReconciliationSchemas.ts +++ b/src/domain/javascript/javascriptRuntimeReconciliationSchemas.ts @@ -228,6 +228,7 @@ const staticLoadStateSchema = z.strictObject({ "not-observed-in-bounded-capture", "layer-outside-runtime-scope", "static-or-runtime-coverage-incomplete", + "ambiguous-runtime-candidate", ]), }); diff --git a/tests/boundary/cli/electronCli.test.ts b/tests/boundary/cli/electronCli.test.ts index 6ba962e9b4..8898ad9004 100644 --- a/tests/boundary/cli/electronCli.test.ts +++ b/tests/boundary/cli/electronCli.test.ts @@ -13,6 +13,12 @@ import { type FakeCdpBrowser, } from "../../fixtures/fakeCdpBrowser.js"; import { writeElectronBoundaryFixture } from "../../fixtures/electronBoundaryApplication.js"; +import { + SOURCE, + applicationFixture, + analyzeFixture, + electronRuntimeEvidence, +} from "../../fixtures/javascriptRuntimeReconciliation.js"; import { JAVASCRIPT_RUNTIME_RECONCILIATION_EXAMPLE } from "../../../src/contracts/javascript/javascriptRuntimeReconciliationExample.js"; const execute = promisify(execFile); @@ -219,8 +225,123 @@ describe("Electron CLI runtime reconciliation", () => { }, INTEGRATION_TEST_TIMEOUT_MS, ); + + it( + "keeps ambiguous runtime candidates unknown while unrelated scoped assets remain absent", + async () => { + const application = await applicationFixture(); + const assets = await createTestTempDirectory("rea-runtime-cli-assets-"); + const runtime = await createTestTempDirectory("rea-runtime-cli-live-"); + const inputPath = join(assets, "reconciliation.json"); + temporary.push(application, assets, runtime); + await Promise.all([ + writeFile(join(assets, "app.js"), SOURCE), + writeFile(join(assets, "absent.js"), "export const absent = true;\n"), + writeFile(join(runtime, "app.js"), SOURCE), + ]); + const rootMapping = [{ kind: "file-root", root: runtime }] as const; + const input = { + static_layers: [ + { + role: "application", + analysis: await analyzeFixture(application), + runtime_mappings: rootMapping, + }, + { + role: "assets", + analysis: await analyzeFixture(assets), + runtime_mappings: rootMapping, + }, + ], + runtime_observations: [ + electronRuntimeEvidence(runtime, SOURCE, { includeWorker: false }), + ], + }; + await writeFile(inputPath, JSON.stringify(input)); + const reconciled = await runCli( + ["reconcile-javascript-runtime", inputPath, "--json"], + process.env, + ); + + const normalized = + isRecord(reconciled) && isRecord(reconciled.normalized_result) + ? reconciled.normalized_result + : undefined; + const reconciliations = normalized?.reconciliations; + const loadStates = normalized?.static_load_states; + if (!Array.isArray(reconciliations) || !Array.isArray(loadStates)) + throw new Error("Missing runtime reconciliation output"); + const ambiguous = reconciliations.find( + (item) => + isRecord(item) && + item.status === "ambiguous" && + item.entity_kind === "script", + ); + if ( + !isRecord(ambiguous) || + !Array.isArray(ambiguous.candidate_static_nodes) + ) + throw new Error("Missing ambiguous runtime script"); + const candidateIds = ambiguous.candidate_static_nodes.flatMap((item) => + isRecord(item) && typeof item.static_node_id === "string" + ? [item.static_node_id] + : [], + ); + expect(candidateIds).toHaveLength(2); + const candidateStates = loadStates.filter( + (item) => + isRecord(item) && + typeof item.static_node_id === "string" && + candidateIds.includes(item.static_node_id), + ); + expect(candidateStates).toHaveLength(2); + expect( + candidateStates.every( + (item) => + isRecord(item) && + item.status === "unknown" && + item.reason === "ambiguous-runtime-candidate", + ), + ).toBe(true); + expect(loadStates).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + status: "not-observed-in-capture", + reason: "not-observed-in-bounded-capture", + }), + ]), + ); + + const singleLayerPath = join(application, "single-reconciliation.json"); + await writeFile( + singleLayerPath, + JSON.stringify({ + static_layers: [input.static_layers[0]], + runtime_observations: input.runtime_observations, + }), + ); + const uniquelyMatched = await runCli( + ["reconcile-javascript-runtime", singleLayerPath, "--json"], + process.env, + ); + expect(uniquelyMatched).toMatchObject({ + normalized_result: { + static_load_states: expect.arrayContaining([ + expect.objectContaining({ + status: "loaded", + reason: "runtime-script-correspondence", + }), + ]), + }, + }); + }, + INTEGRATION_TEST_TIMEOUT_MS, + ); }); +const isRecord = (value: unknown): value is Record => + typeof value === "object" && value !== null && !Array.isArray(value); + const runCli = async ( arguments_: readonly string[], environment: NodeJS.ProcessEnv, From 2b60e097f281f7708c69f1f7ba9263694547bcc0 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 14 Oct 2026 17:49:15 +0800 Subject: [PATCH 06/20] fix(browser): preserve uncertainty and compare redirect observations Retain scenario selection limitations independently of known changes. Compare known status, MIME and byte observations for each redirect hop without inventing missing facts. Refs: #3172, #3173 --- src/domain/browserScenarioDiff.test.ts | 59 ++++++ src/domain/browserScenarioDiff.ts | 2 +- src/domain/webCaptureDiff.ts | 82 ++++++-- tests/boundary/browser/webCaptureDiff.test.ts | 83 ++++++++ tests/boundary/cli/browserCli.test.ts | 177 ++++++++++++++++++ 5 files changed, 384 insertions(+), 19 deletions(-) diff --git a/src/domain/browserScenarioDiff.test.ts b/src/domain/browserScenarioDiff.test.ts index aaf3ab9d8b..0eb5e593dd 100644 --- a/src/domain/browserScenarioDiff.test.ts +++ b/src/domain/browserScenarioDiff.test.ts @@ -539,6 +539,65 @@ describe("browser scenario network comparison stability", () => { }); }); +describe("scenario comparison recovery guidance", () => { + it("retains network guidance beside a changed console event", () => { + const before = scenarioCapture({}); + const afterBase = scenarioCapture({}); + const consoleEvent = afterBase.events.items[0]; + if (consoleEvent?.kind !== "console") + throw new Error("Expected console fixture"); + const after = browserScenarioCaptureSchema.parse({ + ...afterBase, + scenario: { + ...afterBase.scenario, + network_content: { + ...afterBase.scenario.network_content, + response_body: true, + }, + }, + events: { + ...afterBase.events, + items: [{ ...consoleEvent, text: "settings-ready-again" }], + }, + }); + const compare = (left: typeof before, right: typeof before) => + compareBrowserScenarios( + compareBrowserScenariosInputSchema.parse({ + before_scenario: left, + after_scenario: right, + }), + ); + + const changed = compare(before, after); + expect(changed.overall_status).toBe("changed"); + expect(changed.steps.flatMap((step) => step.artifact_diffs)).toContainEqual( + expect.objectContaining({ + artifact: "events", + status: "changed", + reason: expect.stringContaining("Network content selections differ"), + }), + ); + const sameSelection = compare(before, scenarioCapture({})); + expect(sameSelection.overall_status).toBe("unchanged"); + expect( + sameSelection.steps.flatMap((step) => step.artifact_diffs), + ).not.toContainEqual(expect.objectContaining({ artifact: "events" })); + const quietMismatch = compare( + before, + browserScenarioCaptureSchema.parse({ ...after, events: before.events }), + ); + expect( + quietMismatch.steps.flatMap((step) => step.artifact_diffs), + ).toContainEqual( + expect.objectContaining({ + artifact: "events", + status: "unknown", + reason: expect.stringContaining("Network content selections differ"), + }), + ); + }); +}); + describe("browser scenario comparison", () => { it("records canonical literal normalization and ignores declared volatile fields", () => { const before = scenarioCapture({ diff --git a/src/domain/browserScenarioDiff.ts b/src/domain/browserScenarioDiff.ts index 55c8901bce..e2a0fe0a5a 100644 --- a/src/domain/browserScenarioDiff.ts +++ b/src/domain/browserScenarioDiff.ts @@ -425,7 +425,7 @@ const compareScenarioEvents = ( after_state: afterState, before_sha256: beforeSha256, after_sha256: afterSha256, - reason: null, + reason: input.incomparableReason ?? null, }, }; if ( diff --git a/src/domain/webCaptureDiff.ts b/src/domain/webCaptureDiff.ts index e062582a5b..da4533690f 100644 --- a/src/domain/webCaptureDiff.ts +++ b/src/domain/webCaptureDiff.ts @@ -1,5 +1,8 @@ import { digestCanonicalValue } from "./canonicalDigest.js"; -import { compareUnicodeCodePoints } from "./unicodeCodePointOrder.js"; +import { + compareUnicodeCodePoints, + compositeKey, +} from "./unicodeCodePointOrder.js"; import type { WebPageInspection } from "./browserObservationSchemas.js"; import { webCaptureDiffSchema, @@ -258,14 +261,34 @@ const sharedNetworkResponseFields = ( for (const inspection of [before, after]) for (const request of inspection.network.requests) { const identity = networkIdentity(request); - const fields = shared.get(identity) ?? new Set(networkResponseFields); - for (const field of networkResponseFields) - if (request[field] === null) fields.delete(field); - shared.set(identity, fields); + retainSharedResponseFields(shared, identity, request); + for (const [index, redirect] of (request.redirects ?? []).entries()) + retainSharedResponseFields( + shared, + redirectNetworkIdentity(identity, index), + redirect, + ); } return shared; }; +const retainSharedResponseFields = ( + shared: Map>, + identity: string, + response: Pick< + WebPageInspection["network"]["requests"][number], + NetworkResponseField + >, +): void => { + const fields = shared.get(identity) ?? new Set(networkResponseFields); + for (const field of networkResponseFields) + if (response[field] === null) fields.delete(field); + shared.set(identity, fields); +}; + +const redirectNetworkIdentity = (identity: string, index: number): string => + compositeKey([identity, "redirect", index]); + const networkDimension = ( before: WebPageInspection, after: WebPageInspection, @@ -359,14 +382,19 @@ const networkDimension = ( "Network capture or selected HTTP/WebSocket payload-shape coverage is attach-window limited, unavailable, or incomplete.", inventoryComplete, ); - return compared.status === "unchanged" && !responseFieldsComplete - ? { - ...compared, - status: "unknown", - reason: - "HTTP response status, MIME type, or byte-count evidence is incomplete.", - } - : compared; + if (responseFieldsComplete) return compared; + const responseEvidenceReason = + "HTTP response or redirect status, MIME type, or byte-count evidence is incomplete."; + if (compared.status === "changed") return compared; + return { + status: "unknown", + total_changes: 0, + changes: [], + reason: + compared.reason === null + ? responseEvidenceReason + : `${responseEvidenceReason} ${compared.reason}`, + }; }; const networkMap = ( @@ -385,11 +413,29 @@ const networkMap = ( values.push({ ...response, redirects: (request.redirects ?? []).map( - ({ - request_timestamp: _requestTimestamp, - redirect_event_timestamp: _redirectEventTimestamp, - ...redirect - }) => redirect, + ( + { + request_timestamp: _requestTimestamp, + redirect_event_timestamp: _redirectEventTimestamp, + ...redirect + }, + index, + ) => { + const response: Partial< + Record + > = {}; + for (const field of responseFields.get( + redirectNetworkIdentity(identity, index), + ) ?? []) + if (redirect[field] !== null) response[field] = redirect[field]; + return { + url: redirect.url, + response_url: redirect.response_url, + method: redirect.method, + resource_type: redirect.resource_type, + ...response, + }; + }, ), initiator: request.initiator, ...(shapes?.request === true diff --git a/tests/boundary/browser/webCaptureDiff.test.ts b/tests/boundary/browser/webCaptureDiff.test.ts index 11d992fa4d..955b63714d 100644 --- a/tests/boundary/browser/webCaptureDiff.test.ts +++ b/tests/boundary/browser/webCaptureDiff.test.ts @@ -869,6 +869,89 @@ describe("web capture redirect comparison", () => { changes: [expect.objectContaining({ change: "modified" })], }); }); + + it.each([ + ["status", 302], + ["mime_type", "text/plain"], + ["encoded_data_length", 12], + ] as const)( + "keeps unavailable redirect %s evidence unknown without suppressing known changes", + async (field, knownValue) => { + const browser = await startFakeCdpBrowser(); + browsers.push(browser); + const captured = await new CdpBrowserProvider().inspectPage( + inspectWebPageInputSchema.parse({ + cdp_endpoint: browser.endpoint, + allowed_origins: [browser.allowedOrigin], + target_id: "allowed-page", + observation_ms: 0, + }), + ); + if (!captured.ok) throw captured.error; + const before = structuredClone(captured.value); + const beforeRequest = before.network.requests[0]; + if (beforeRequest === undefined) + throw new Error("Missing network request"); + beforeRequest.redirects = [ + { + url: `${browser.allowedOrigin}/prior`, + response_url: `${browser.allowedOrigin}/prior`, + method: "GET", + resource_type: "Fetch", + status: 302, + mime_type: "text/plain", + encoded_data_length: 12, + request_timestamp: 1, + redirect_event_timestamp: 2, + }, + ]; + markSectionsComplete(before, [ + "network_requests", + "websocket_connections", + "websocket_frames", + ]); + + const compare = (left: typeof before, right: typeof before) => + compareWebCaptures( + compareWebCapturesInputSchema.parse({ + before: { inspection: left }, + after: { inspection: right }, + }), + ).dimensions.network; + for (const missingOnRight of [false, true]) { + const known = structuredClone(before); + const missing = structuredClone(before); + const knownHop = known.network.requests[0]?.redirects?.[0]; + const missingHop = missing.network.requests[0]?.redirects?.[0]; + if (knownHop === undefined || missingHop === undefined) + throw new Error("Missing redirect fixture"); + Reflect.set(knownHop, field, knownValue); + Reflect.set(missingHop, field, null); + const result = missingOnRight + ? compare(known, missing) + : compare(missing, known); + expect(result).toMatchObject({ + status: "unknown", + total_changes: 0, + reason: expect.stringContaining("redirect"), + }); + expect(compare(missing, missing)).toMatchObject({ + status: "unknown", + total_changes: 0, + reason: expect.stringContaining("redirect"), + }); + } + + const changed = structuredClone(before); + const changedHop = changed.network.requests[0]?.redirects?.[0]; + if (changedHop === undefined) throw new Error("Missing redirect fixture"); + changedHop.status = 307; + expect(compare(before, changed)).toMatchObject({ + status: "changed", + changes: [expect.objectContaining({ change: "modified" })], + }); + }, + ); }); describe("web capture diff incomplete inventories", () => { diff --git a/tests/boundary/cli/browserCli.test.ts b/tests/boundary/cli/browserCli.test.ts index 56daccafb1..01fc53b53b 100644 --- a/tests/boundary/cli/browserCli.test.ts +++ b/tests/boundary/cli/browserCli.test.ts @@ -183,6 +183,51 @@ it( INTEGRATION_TEST_TIMEOUT_MS, ); +it( + "compares redirect metadata uncertainty through the public CLI", + async () => { + const browser = await startFakeCdpBrowser(); + browsers.push(browser); + const inspected = await runCli( + [ + "inspect-web-page", + browser.endpoint, + "allowed-page", + "--observation-ms", + "0", + "--json", + ], + process.env, + ); + const inspection = normalizedResult(inspected); + const missingVsKnown = await compareCapturesCli( + redirectCapture(inspection, browser.allowedOrigin, null), + redirectCapture(inspection, browser.allowedOrigin, 302), + ); + expect(missingVsKnown).toMatchObject({ + normalized_result: { + dimensions: { + network: { + status: "unknown", + total_changes: 0, + reason: expect.stringContaining("redirect"), + }, + }, + }, + }); + const knownDifference = await compareCapturesCli( + redirectCapture(inspection, browser.allowedOrigin, 302), + redirectCapture(inspection, browser.allowedOrigin, 307), + ); + expect(knownDifference).toMatchObject({ + normalized_result: { + dimensions: { network: { status: "changed", total_changes: 1 } }, + }, + }); + }, + INTEGRATION_TEST_TIMEOUT_MS, +); + it( "browser CLI attaches through a browser socket and records it redacted", async () => { @@ -344,6 +389,68 @@ describe("scenario comparison CLI diagnostics", () => { ); }); +describe("scenario comparison recovery guidance", () => { + it("reports independent network recovery alongside console differences", async () => { + const base = completeScenarioCapture(); + const matchingSelection = scenarioCaptureWithConsole( + "before", + base.scenario.network_content, + ); + const differentSelection = scenarioCaptureWithConsole("after", { + ...base.scenario.network_content, + response_body: true, + }); + const changed = await compareCapturesCli( + matchingSelection, + differentSelection, + ); + expect(changed).toMatchObject({ + normalized_result: { + overall_status: "changed", + steps: expect.arrayContaining([ + expect.objectContaining({ + step_id: "open-settings", + artifact_diffs: expect.arrayContaining([ + expect.objectContaining({ + artifact: "events", + status: "changed", + reason: expect.stringContaining( + "Network content selections differ", + ), + }), + ]), + }), + ]), + }, + }); + const quietMismatch = await compareCapturesCli( + matchingSelection, + scenarioCaptureWithConsole( + "before", + differentSelection.scenario.network_content, + ), + ); + expect(quietMismatch).toMatchObject({ + normalized_result: { + steps: expect.arrayContaining([ + expect.objectContaining({ + step_id: "open-settings", + artifact_diffs: expect.arrayContaining([ + expect.objectContaining({ + artifact: "events", + status: "unknown", + reason: expect.stringContaining( + "Network content selections differ", + ), + }), + ]), + }), + ]), + }, + }); + }); +}); + describe("browser CLI capture parity", () => { it( "runs analysis, session, WebMCP, screenshot, and comparison from explicit request scope", @@ -468,6 +575,43 @@ const normalizedResult = (value: unknown): Record => { return value.normalized_result; }; +const redirectCapture = ( + inspection: Record, + origin: string, + status: number | null, +) => { + const copy = structuredClone(inspection); + if (!isRecord(copy.network) || !Array.isArray(copy.network.requests)) + throw new Error("Missing network capture"); + const request = copy.network.requests[0]; + if (!isRecord(request)) throw new Error("Missing network request"); + request.redirects = [ + { + url: `${origin}/prior`, + response_url: `${origin}/prior`, + method: "GET", + resource_type: "Fetch", + status, + mime_type: "text/plain", + encoded_data_length: 12, + request_timestamp: 1, + redirect_event_timestamp: 2, + }, + ]; + return { inspection: copy }; +}; + +const compareCapturesCli = (before: unknown, after: unknown) => + runCli( + [ + "compare-web-captures", + JSON.stringify(before), + JSON.stringify(after), + "--json", + ], + process.env, + ); + const completeScenarioCapture = () => { const url = { url: "https://app.example.test/", @@ -533,5 +677,38 @@ const completeScenarioCapture = () => { }; }; +const scenarioCaptureWithConsole = ( + text: string, + networkContent: { + readonly request_body: boolean; + readonly response_body: boolean; + readonly header_values: boolean; + }, +) => { + const capture = completeScenarioCapture(); + return { + ...capture, + scenario: { + ...capture.scenario, + network_content: networkContent, + event_families: ["console"], + }, + events: { + retained: 1, + dropped: 0, + items: [ + { + sequence: 1, + step_index: 1, + kind: "console", + level: "info", + text, + url: null, + }, + ], + }, + }; +}; + const isRecord = (value: unknown): value is Record => typeof value === "object" && value !== null && !Array.isArray(value); From 1250aaedd8b4d1e471d8a382f76418bde65c05d2 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 14 Oct 2026 17:49:16 +0800 Subject: [PATCH 07/20] fix(browser): decode SAZ XML through typed encoding validation Reuse the format-aware XML text owner, preserve decoder causes, and reject contradictory declarations even in unselected records. Refs: #3273 --- src/browser/history/CaptureFormatError.ts | 3 +- src/browser/history/SazMetadata.test.ts | 44 +++++++++++++++++++ src/browser/history/SazMetadata.ts | 36 ++++++++++++---- src/domain/propertyListXmlText.test.ts | 25 +++++++++++ src/domain/propertyListXmlText.ts | 39 +++++++++++++++-- tests/boundary/browser/sazWebCapture.test.ts | 45 ++++++++++++++++++++ 6 files changed, 179 insertions(+), 13 deletions(-) create mode 100644 src/domain/propertyListXmlText.test.ts diff --git a/src/browser/history/CaptureFormatError.ts b/src/browser/history/CaptureFormatError.ts index c3e4da9b3e..0fdc026909 100644 --- a/src/browser/history/CaptureFormatError.ts +++ b/src/browser/history/CaptureFormatError.ts @@ -9,7 +9,8 @@ export class CaptureFormatError extends Error { | "limit", message: string, readonly pointer = "", + cause?: unknown, ) { - super(message); + super(message, cause === undefined ? undefined : { cause }); } } diff --git a/src/browser/history/SazMetadata.test.ts b/src/browser/history/SazMetadata.test.ts index 4aa482a7a4..54126937c1 100644 --- a/src/browser/history/SazMetadata.test.ts +++ b/src/browser/history/SazMetadata.test.ts @@ -2,6 +2,50 @@ import { expect, it } from "vitest"; import { CaptureFormatError } from "./CaptureFormatError.js"; import { decodeSazMetadata } from "./SazMetadata.js"; +it.each([ + ["UTF-8", Buffer.from('')], + [ + "BOM-marked UTF-16LE", + Buffer.concat([ + Buffer.from([0xff, 0xfe]), + Buffer.from( + '', + "utf16le", + ), + ]), + ], +] as const)("accepts matching %s XML declarations", (_label, bytes) => { + expect(() => decodeSazMetadata(bytes)).not.toThrow(); +}); + +it.each([ + [ + "UTF-16 declaration on UTF-8 bytes", + Buffer.from(''), + "format", + ], + [ + "UTF-8 declaration on UTF-16 bytes", + Buffer.concat([ + Buffer.from([0xff, 0xfe]), + Buffer.from( + '', + "utf16le", + ), + ]), + "format", + ], + [ + "unsupported declared encoding", + Buffer.from(''), + "unsupported", + ], +] as const)("rejects %s", (_label, bytes, reason) => { + expect(() => decodeSazMetadata(bytes)).toThrow( + expect.objectContaining({ reason }), + ); +}); + it("retains U+FFFD in well-formed metadata as an ordinary XML character", () => { const decoded = JSON.stringify( decodeSazMetadata( diff --git a/src/browser/history/SazMetadata.ts b/src/browser/history/SazMetadata.ts index a5dd239bd0..5ae8d950b7 100644 --- a/src/browser/history/SazMetadata.ts +++ b/src/browser/history/SazMetadata.ts @@ -2,23 +2,43 @@ import { DOMParser, ParseError, type Node } from "@xmldom/xmldom"; import { SaxesParser } from "saxes"; import type { JsonValue } from "../../domain/jsonValue.js"; import { WEB_NETWORK_CAPTURE_LIMITS } from "../../domain/webNetworkCapture.js"; +import { + decodeXmlPlistText, + XmlPlistTextDecodeError, +} from "../../domain/propertyListXmlText.js"; import { CaptureFormatError } from "./CaptureFormatError.js"; /** Preserve XML node order, attribute names and text without interpreting producer-specific flags. */ export const decodeSazMetadata = (bytes: Buffer): JsonValue => { let xml: string; try { - const encoding = - bytes[0] === 0xff && bytes[1] === 0xfe - ? "utf-16le" - : bytes[0] === 0xfe && bytes[1] === 0xff - ? "utf-16be" - : "utf-8"; - xml = new TextDecoder(encoding, { fatal: true }).decode(bytes); - } catch { + xml = decodeXmlPlistText(bytes); + } catch (cause: unknown) { + if ( + cause instanceof XmlPlistTextDecodeError && + cause.reason === "unsupported-encoding" + ) + throw new CaptureFormatError( + "unsupported", + `SAZ metadata declares unsupported XML encoding ${cause.declaredEncoding}.`, + "", + cause, + ); + if ( + cause instanceof XmlPlistTextDecodeError && + cause.reason === "encoding-mismatch" + ) + throw new CaptureFormatError( + "format", + "SAZ metadata encoding declaration disagrees with its bytes.", + "", + cause, + ); throw new CaptureFormatError( "unsupported", "SAZ metadata must use UTF-8 or BOM-marked UTF-16.", + "", + cause, ); } validateSazXml(xml); diff --git a/src/domain/propertyListXmlText.test.ts b/src/domain/propertyListXmlText.test.ts new file mode 100644 index 0000000000..8cae86b390 --- /dev/null +++ b/src/domain/propertyListXmlText.test.ts @@ -0,0 +1,25 @@ +import { expect, it } from "vitest"; + +import { + decodeXmlPlistText, + XmlPlistTextDecodeError, +} from "./propertyListXmlText.js"; + +it.each([ + [ + Buffer.from(''), + "encoding-mismatch", + ], + [ + Buffer.from(''), + "unsupported-encoding", + ], + [Buffer.from([0xff, 0xfe, 0x3c]), "invalid-bytes"], +] as const)("tags XML decoding failures as %s", (bytes, reason) => { + expect(() => decodeXmlPlistText(bytes)).toThrow( + expect.objectContaining({ + name: "XmlPlistTextDecodeError", + reason, + } satisfies Partial), + ); +}); diff --git a/src/domain/propertyListXmlText.ts b/src/domain/propertyListXmlText.ts index 2d6820120f..8cf52d772d 100644 --- a/src/domain/propertyListXmlText.ts +++ b/src/domain/propertyListXmlText.ts @@ -1,4 +1,19 @@ -/** Decode BOM-aware XML plist text with fatal byte validation. */ +export class XmlPlistTextDecodeError extends TypeError { + constructor( + readonly reason: + | "invalid-bytes" + | "unsupported-encoding" + | "encoding-mismatch", + message: string, + readonly declaredEncoding?: string, + options?: ErrorOptions, + ) { + super(message, options); + this.name = "XmlPlistTextDecodeError"; + } +} + +/** Decode BOM-aware XML plist text and verify any declaration against its bytes. */ export const decodeXmlPlistText = (bytes: Uint8Array): string => { const encoding = bytes[0] === 0xff && bytes[1] === 0xfe @@ -6,7 +21,17 @@ export const decodeXmlPlistText = (bytes: Uint8Array): string => { : bytes[0] === 0xfe && bytes[1] === 0xff ? "utf-16be" : "utf-8"; - const text = new TextDecoder(encoding, { fatal: true }).decode(bytes); + let text: string; + try { + text = new TextDecoder(encoding, { fatal: true }).decode(bytes); + } catch (cause: unknown) { + throw new XmlPlistTextDecodeError( + "invalid-bytes", + "XML text bytes are invalid for the detected encoding", + undefined, + { cause }, + ); + } const declaration = /^<\?xml\s[^?]*\bencoding\s*=\s*["']([^"']+)["']/u.exec( text, )?.[1]; @@ -15,14 +40,20 @@ export const decodeXmlPlistText = (bytes: Uint8Array): string => { declared !== undefined && !["utf-8", "utf-16", "utf-16le", "utf-16be"].includes(declared) ) - throw new TypeError( + throw new XmlPlistTextDecodeError( + "unsupported-encoding", `Unsupported XML encoding declaration: ${declaration}. Supported encodings are UTF-8 and BOM-marked UTF-16 in either byte order.`, + declaration, ); if ( declared !== undefined && declared !== encoding && !(encoding !== "utf-8" && declared === "utf-16") ) - throw new TypeError("XML encoding declaration disagrees with its bytes"); + throw new XmlPlistTextDecodeError( + "encoding-mismatch", + "XML encoding declaration disagrees with its bytes", + declaration, + ); return text; }; diff --git a/tests/boundary/browser/sazWebCapture.test.ts b/tests/boundary/browser/sazWebCapture.test.ts index d6192e2e69..3e16fac4c3 100644 --- a/tests/boundary/browser/sazWebCapture.test.ts +++ b/tests/boundary/browser/sazWebCapture.test.ts @@ -342,6 +342,51 @@ it("keeps CRLF separators inside a body after LF-only HTTP headers", async () => expect(await readFile(path)).toEqual(bytes); }); +it("validates SAZ XML declarations against archive member bytes at the CLI boundary", async () => { + const withMetadata = (metadata: Buffer): [string, Buffer][] => + members().map(([name, bytes]) => + name === "raw/0042_m.xml" ? [name, metadata] : [name, bytes], + ); + const utf8 = Buffer.from( + '', + ); + const utf16le = Buffer.concat([ + Buffer.from([0xff, 0xfe]), + Buffer.from( + '', + "utf16le", + ), + ]); + for (const metadata of [utf8, utf16le]) { + const result = await cli( + await fixture(await archive(withMetadata(metadata))), + ); + expect( + webNetworkCaptureSchema.parse(result.normalized_result).records, + ).toHaveLength(2); + } + const mismatch = Buffer.from( + '', + ); + const rejected = await cliResponse( + await fixture(await archive(withMetadata(mismatch))), + ["--record", "1"], + ); + expect(rejected.ok).toBe(false); + expect(JSON.parse(rejected.stdout)).toMatchObject({ + category: "invalid_input", + details: { + issues: [ + expect.objectContaining({ + message: expect.stringContaining( + "SAZ metadata encoding declaration disagrees with its bytes", + ), + }), + ], + }, + }); +}); + it("excludes explicit literals from XML, HTTP byte views, member coordinates and binary sidecars", async () => { const path = await fixture(await archive()); const result = await cli(path, [ From 3a34ad37bdb7b8debba0f6515ba85b80141f2716 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 14 Oct 2026 17:49:16 +0800 Subject: [PATCH 08/20] fix(evidence): classify noncanonical bundles at the input boundary Use a dedicated canonical-projection error so unrelated TypeErrors retain their protocol failure meaning. Refs: #3219 --- .../ReconstructionObligationLedgerService.ts | 3 + src/domain/evidenceBundle.test.ts | 18 +++++ src/domain/evidenceBundle.ts | 10 ++- .../reconstructionObligationLedgerCli.test.ts | 75 +++++++++++++++++++ .../reconstructionObligationLedgerMcp.test.ts | 68 +++++++++++++++++ 5 files changed, 173 insertions(+), 1 deletion(-) diff --git a/src/application/ReconstructionObligationLedgerService.ts b/src/application/ReconstructionObligationLedgerService.ts index e0b1f160a2..cffee63304 100644 --- a/src/application/ReconstructionObligationLedgerService.ts +++ b/src/application/ReconstructionObligationLedgerService.ts @@ -2,6 +2,7 @@ import { z } from "zod"; import { AnalysisProtocolError } from "../domain/analysisErrorCore.js"; import type { AnalysisError } from "../domain/analysisErrorBase.js"; +import { NonCanonicalEvidenceBundleError } from "../domain/evidenceBundle.js"; import { createEvidence, type Evidence } from "../domain/evidence.js"; import { jsonObjectSchema, jsonValueSchema } from "../domain/jsonValue.js"; import { @@ -45,6 +46,8 @@ export const buildReconstructionObligationLedgerEvidenceValidated = ( }); return ok(createLedgerEvidence(input, ledger)); } catch (cause: unknown) { + if (cause instanceof NonCanonicalEvidenceBundleError) + return err(workflowInputError(OPERATION, cause)); if (cause instanceof z.ZodError) return err(workflowInputError(OPERATION, cause)); return err( diff --git a/src/domain/evidenceBundle.test.ts b/src/domain/evidenceBundle.test.ts index d2f90cbf6c..1936e50464 100644 --- a/src/domain/evidenceBundle.test.ts +++ b/src/domain/evidenceBundle.test.ts @@ -5,6 +5,7 @@ import { ANALYSIS_SNAPSHOT_TARGET } from "./analysisSnapshot.fixture.js"; import { createEvidenceBundle, createImmutableEvidenceBundle, + NonCanonicalEvidenceBundleError, describeEvidenceBundleFailure, evidenceBundleForTarget, parseEvidenceBundle, @@ -20,6 +21,23 @@ const targetDigest = "a".repeat(64); const foreignDigest = "b".repeat(64); describe("evidence bundle failure classification", () => { + it("tags valid-shaped bundles that fail canonical ordering", () => { + const records = ["first", "second"].map((label) => + createEvidence(undefined, provider, { + operation: "observe", + parameters: { label }, + result: { label }, + }), + ); + const canonical = createEvidenceBundle(records); + expect(() => + parseEvidenceBundle({ + ...canonical, + records: [...canonical.records].reverse(), + }), + ).toThrow(NonCanonicalEvidenceBundleError); + }); + it("treats only authenticated standalone Evidence as the wrong input kind", () => { const record = createEvidence(undefined, provider, { operation: "observe", diff --git a/src/domain/evidenceBundle.ts b/src/domain/evidenceBundle.ts index a70969b8ef..6df730f0e8 100644 --- a/src/domain/evidenceBundle.ts +++ b/src/domain/evidenceBundle.ts @@ -52,6 +52,14 @@ export const evidenceBundleSchema = z.strictObject({ export type EvidenceBundle = z.infer; const immutableBundles = new WeakMap(); +/** Caller input has valid shape but is not in authenticated canonical form. */ +export class NonCanonicalEvidenceBundleError extends TypeError { + constructor() { + super("Evidence bundle manifests are not canonical"); + this.name = "NonCanonicalEvidenceBundleError"; + } +} + /** Build and authenticate a sealed bundle while reusing immutable Evidence records. */ export const createImmutableEvidenceBundle = ( records: readonly Evidence[], @@ -307,7 +315,7 @@ const authenticateBundle = ( parsed.unknowns, ); if (JSON.stringify(parsed) !== JSON.stringify(canonical)) - throw new TypeError("Evidence bundle manifests are not canonical"); + throw new NonCanonicalEvidenceBundleError(); if (seal) { freezeJsonSnapshot(canonical); immutableBundles.set(canonical, canonical); diff --git a/tests/boundary/cli/reconstructionObligationLedgerCli.test.ts b/tests/boundary/cli/reconstructionObligationLedgerCli.test.ts index 63f3e94c41..eee6b3df1a 100644 --- a/tests/boundary/cli/reconstructionObligationLedgerCli.test.ts +++ b/tests/boundary/cli/reconstructionObligationLedgerCli.test.ts @@ -3,11 +3,86 @@ import { promisify } from "node:util"; import { describe, expect, it } from "vitest"; +import { createEvidence } from "../../../src/domain/evidence.js"; import { createEvidenceBundle } from "../../../src/domain/evidenceBundle.js"; const execute = promisify(execFile); describe("reconstruction obligation ledger CLI parity", () => { + it("reports non-canonical authenticated bundles as caller input", async () => { + const records = ["first", "second"].map((label) => + createEvidence( + undefined, + { id: "fixture", name: "Fixture", version: "1" }, + { + operation: "observe", + parameters: { label }, + result: { label }, + }, + ), + ); + const canonical = createEvidenceBundle(records); + const input = { + evidence_bundle: canonical, + reviewed_obligations: [], + manifest: { bindings: [], contradictions: [] }, + }; + const accepted = await execute( + process.execPath, + [ + "scripts/rea.mjs", + "build-reconstruction-obligation-ledger", + JSON.stringify(input), + "--json", + ], + { cwd: process.cwd(), env: process.env, maxBuffer: 16 * 1_024 * 1_024 }, + ); + expect(JSON.parse(accepted.stdout)).toMatchObject({ + normalized_result: { status: "unknown", summary: { total: 0 } }, + }); + input.evidence_bundle = { + ...canonical, + records: [...canonical.records].reverse(), + }; + let stdout: string; + try { + await execute( + process.execPath, + [ + "scripts/rea.mjs", + "build-reconstruction-obligation-ledger", + JSON.stringify(input), + "--json", + ], + { cwd: process.cwd(), env: process.env, maxBuffer: 16 * 1_024 * 1_024 }, + ); + throw new Error("Expected non-canonical bundle input to fail"); + } catch (cause: unknown) { + if ( + !(cause instanceof Error) || + !("stdout" in cause) || + typeof cause.stdout !== "string" + ) + throw cause; + stdout = cause.stdout; + } + expect(JSON.parse(stdout)).toMatchObject({ + code: "invalid_request", + category: "invalid_input", + retryable: true, + details: { + operation: "build_reconstruction_obligation_ledger", + issues: [ + expect.objectContaining({ + message: expect.stringContaining( + "Evidence bundle manifests are not canonical", + ), + }), + ], + }, + }); + }, 20_000); + it("builds the same Evidence-backed empty ledger through the CLI", async () => { const input = { evidence_bundle: createEvidenceBundle([]), diff --git a/tests/boundary/mcp/reconstructionObligationLedgerMcp.test.ts b/tests/boundary/mcp/reconstructionObligationLedgerMcp.test.ts index 653977a8fa..f9b61b7514 100644 --- a/tests/boundary/mcp/reconstructionObligationLedgerMcp.test.ts +++ b/tests/boundary/mcp/reconstructionObligationLedgerMcp.test.ts @@ -3,11 +3,79 @@ import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; import { describe, expect, it } from "vitest"; import { createTestBinarySession } from "../../fixtures/binarySession.js"; +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { createEvidenceBundle } from "../../../src/domain/evidenceBundle.js"; +import { createEvidence } from "../../../src/domain/evidence.js"; import { createServer } from "../../../src/server/createServer.js"; import { observed } from "../../fixtures/analysisExecution.js"; describe("reconstruction obligation ledger MCP parity", () => { + it("returns canonical bundle failures as caller input errors", async () => { + const records = ["first", "second"].map((label) => + createEvidence( + undefined, + { id: "fixture", name: "Fixture", version: "1" }, + { + operation: "observe", + parameters: { label }, + result: { label }, + }, + ), + ); + const canonical = createEvidenceBundle(records); + const session = createTestBinarySession(() => ({ + execute: () => Promise.resolve(observed(null)), + close: () => Promise.resolve(resultOk(null)), + })); + const server = createServer({ kind: "session", session }); + const client = new Client({ name: "obligation-ledger-test", version: "1" }); + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + try { + await server.connect(serverTransport); + await client.connect(clientTransport); + const request = (evidenceBundle: unknown) => + client.callTool({ + name: "build_reconstruction_obligation_ledger", + arguments: { + evidence_bundle: evidenceBundle, + reviewed_obligations: [], + manifest: { bindings: [], contradictions: [] }, + }, + }); + const valid = await request(canonical); + expect(valid.isError).not.toBe(true); + expect(valid.structuredContent).toMatchObject({ + normalized_result: { status: "unknown", summary: { total: 0 } }, + }); + const result = await request({ + ...canonical, + records: [...canonical.records].reverse(), + }); + expect(result.isError).toBe(true); + expect(parseMcpToolError(result)).toMatchObject({ + error: { + code: "invalid_request", + category: "invalid_input", + details: { + operation: "build_reconstruction_obligation_ledger", + issues: [ + expect.objectContaining({ + message: expect.stringContaining( + "Evidence bundle manifests are not canonical", + ), + }), + ], + }, + }, + }); + } finally { + await client.close(); + await server.close(); + await session.close(); + } + }); + it("returns the complete Evidence-backed ledger inline", async () => { const session = createTestBinarySession(() => ({ execute: () => Promise.resolve(observed(null)), From 030c47732833c9cedcc31db1309eb3a1ca2c62e8 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 14 Oct 2026 17:49:17 +0800 Subject: [PATCH 09/20] fix(evidence): derive bundle comparison status from classified changes Keep digests as provenance while status follows examined deltas and unresolved coverage. Refs: #3216 --- src/domain/bundleComparison.test.ts | 33 ++++++ src/domain/bundleComparison.ts | 6 +- .../boundary/cli/bundleComparisonCli.test.ts | 105 ++++++++++++++++++ .../boundary/mcp/bundleComparisonMcp.test.ts | 103 +++++++++++++++++ 4 files changed, 242 insertions(+), 5 deletions(-) create mode 100644 tests/boundary/cli/bundleComparisonCli.test.ts diff --git a/src/domain/bundleComparison.test.ts b/src/domain/bundleComparison.test.ts index 9743c3525a..866c09be23 100644 --- a/src/domain/bundleComparison.test.ts +++ b/src/domain/bundleComparison.test.ts @@ -76,6 +76,39 @@ describe("bundle comparison explicit cross-pairs", () => { }); describe("bundle comparison", () => { + it("does not report display-only bundle differences as unexplained changes", () => { + const target = { + path: "/left/artifact.elf", + sha256: "a".repeat(64), + format: "elf" as const, + architecture: "x86_64" as const, + }; + const record = createEvidence(target, PROVIDER, { + operation: "observe", + parameters: {}, + result: { observed: true }, + confidence: "observed", + authority: "shipped-artifact", + }); + const left = createEvidenceBundle([record]); + const rightRecord = { + ...record, + subject: { + ...record.subject!, + name: "renamed.elf", + local_path: "/right/renamed.elf", + }, + }; + const right = { ...left, records: [rightRecord] }; + const result = compareBundles(left, right); + expect(result).toMatchObject({ + status: "unchanged", + summary: { records_unchanged: 1, unresolved: 0 }, + changes: [], + }); + expect(result.left_bundle_sha256).not.toBe(result.right_bundle_sha256); + }); + it("accepts long caller-selected bundle paths", () => { const path = `/${"deep/".repeat(1_000)}bundle.json`; expect( diff --git a/src/domain/bundleComparison.ts b/src/domain/bundleComparison.ts index cb01980a47..3a00c0cb32 100644 --- a/src/domain/bundleComparison.ts +++ b/src/domain/bundleComparison.ts @@ -102,11 +102,7 @@ export const compareBundles = ( ).length; return bundleComparisonResultSchema.parse({ status: - leftDigest === rightDigest && changes.length === 0 - ? "unchanged" - : unresolved > 0 - ? "unknown" - : "changed", + unresolved > 0 ? "unknown" : changes.length > 0 ? "changed" : "unchanged", left_bundle_sha256: leftDigest, right_bundle_sha256: rightDigest, summary: { diff --git a/tests/boundary/cli/bundleComparisonCli.test.ts b/tests/boundary/cli/bundleComparisonCli.test.ts new file mode 100644 index 0000000000..620cd0eaf2 --- /dev/null +++ b/tests/boundary/cli/bundleComparisonCli.test.ts @@ -0,0 +1,105 @@ +import { writeFile } from "node:fs/promises"; +import { join } from "node:path"; + +import { expect } from "vitest"; + +import { createEvidence } from "../../../src/domain/evidence.js"; +import { bundleComparisonResultSchema } from "../../../src/domain/bundleComparison.js"; +import { + createEvidenceBundle, + serializeEvidenceBundle, +} from "../../../src/domain/evidenceBundle.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { cliTest } from "../../support/cli/cliFixture.js"; + +cliTest( + "compares Evidence bundles without unexplained display-only changes", + async ({ cli }) => { + const root = await createTestTempDirectory("rea-bundle-comparison-cli-"); + const leftPath = join(root, "left.json"); + const displayPath = join(root, "display.json"); + const changedPath = join(root, "changed.json"); + const target = { + path: join(root, "left.elf"), + sha256: "a".repeat(64), + format: "elf" as const, + architecture: "x86_64" as const, + }; + const record = createEvidence( + target, + { id: "fixture", name: "Fixture", version: "1" }, + { + operation: "observe", + parameters: {}, + result: { observed: true }, + confidence: "observed", + authority: "shipped-artifact", + }, + ); + const left = createEvidenceBundle([record]); + const displayOnly = { + ...left, + records: [ + { + ...record, + subject: { + ...record.subject!, + name: "renamed.elf", + local_path: join(root, "renamed.elf"), + }, + }, + ], + }; + const different = createEvidenceBundle([ + createEvidence( + target, + { id: "fixture", name: "Fixture", version: "1" }, + { + operation: "observe", + parameters: {}, + result: { observed: false }, + confidence: "observed", + authority: "shipped-artifact", + }, + ), + ]); + await Promise.all([ + writeFile(leftPath, serializeEvidenceBundle(left)), + writeFile(displayPath, serializeEvidenceBundle(displayOnly)), + writeFile(changedPath, serializeEvidenceBundle(different)), + ]); + const compare = async (path: string) => + cli.run({ arguments: ["compare", leftPath, path, "--json"] }); + const same = await compare(leftPath); + expect(same.exitCode).toBe(0); + expect(bundleComparisonResultSchema.parse(same.json)).toMatchObject({ + status: "unchanged", + changes: [], + }); + const display = await compare(displayPath); + expect(display.exitCode).toBe(0); + const displayResult = bundleComparisonResultSchema.parse(display.json); + expect(displayResult).toMatchObject({ status: "unchanged", changes: [] }); + expect(displayResult.left_bundle_sha256).not.toBe( + displayResult.right_bundle_sha256, + ); + const differentResult = await compare(changedPath); + expect(differentResult.exitCode).toBe(0); + expect( + bundleComparisonResultSchema.parse(differentResult.json), + ).toMatchObject({ + status: "unknown", + summary: { unresolved: 2 }, + changes: [ + expect.objectContaining({ + entity: "evidence", + conclusion_kind: "unresolved_branch", + }), + expect.objectContaining({ + entity: "evidence", + conclusion_kind: "unresolved_branch", + }), + ], + }); + }, +); diff --git a/tests/boundary/mcp/bundleComparisonMcp.test.ts b/tests/boundary/mcp/bundleComparisonMcp.test.ts index ab8838287d..24dd42dbfe 100644 --- a/tests/boundary/mcp/bundleComparisonMcp.test.ts +++ b/tests/boundary/mcp/bundleComparisonMcp.test.ts @@ -9,6 +9,8 @@ import { afterEach, describe, expect, it } from "vitest"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; import { createTestBinarySession } from "../../fixtures/binarySession.js"; +import { toolContract } from "../../../src/contracts/toolContracts.js"; +import { bundleComparisonResultSchema } from "../../../src/domain/bundleComparison.js"; import { createEvidence } from "../../../src/domain/evidence.js"; import { createEvidenceBundle, @@ -37,6 +39,107 @@ const sourceEvidence = (label: string) => }, ); +describe("bundle comparison MCP semantic status", () => { + it("keeps display-only metadata out of deltas and preserves semantic changes", async () => { + const root = await createTestTempDirectory("rea-bundle-metadata-mcp-"); + roots.push(root); + const leftPath = join(root, "left.json"); + const displayPath = join(root, "display.json"); + const changedPath = join(root, "changed.json"); + const target = { + path: join(root, "left.elf"), + sha256: "a".repeat(64), + format: "elf" as const, + architecture: "x86_64" as const, + }; + const record = createEvidence( + target, + { id: "fixture", name: "Fixture", version: "1" }, + { + operation: "observe", + parameters: {}, + result: { observed: true }, + confidence: "observed", + authority: "shipped-artifact", + }, + ); + const left = createEvidenceBundle([record]); + const displayRecord = { + ...record, + subject: { + ...record.subject!, + name: "renamed.elf", + local_path: join(root, "renamed.elf"), + }, + }; + const displayOnly = { ...left, records: [displayRecord] }; + const changedRecord = createEvidence( + target, + { id: "fixture", name: "Fixture", version: "1" }, + { + operation: "observe", + parameters: {}, + result: { observed: false }, + confidence: "observed", + authority: "shipped-artifact", + }, + ); + await Promise.all([ + writeFile(leftPath, serializeEvidenceBundle(left)), + writeFile(displayPath, serializeEvidenceBundle(displayOnly)), + writeFile( + changedPath, + serializeEvidenceBundle(createEvidenceBundle([changedRecord])), + ), + ]); + const connected = await connect(); + try { + const compare = async ( + right: string, + recordPairs: readonly { + left_evidence_id: string; + right_evidence_id: string; + }[] = [], + ) => { + const result = await connected.client.callTool({ + name: "compare_bundles", + arguments: { + left_bundle_path: leftPath, + right_bundle_path: right, + record_pairs: recordPairs, + }, + }); + expect(result.isError).not.toBe(true); + const evidence = toolContract("compare_bundles").outputSchema.parse( + result.structuredContent, + ); + return bundleComparisonResultSchema.parse(evidence.normalized_result); + }; + const same = await compare(leftPath); + expect(same).toMatchObject({ + status: "unchanged", + changes: [], + }); + const display = await compare(displayPath); + expect(display).toMatchObject({ + status: "unchanged", + changes: [], + }); + expect(display.left_bundle_sha256).not.toBe(display.right_bundle_sha256); + const changed = await compare(changedPath, [ + { + left_evidence_id: record.evidence_id, + right_evidence_id: changedRecord.evidence_id, + }, + ]); + expect(changed.status).toBe("changed"); + expect(changed.changes).toHaveLength(1); + } finally { + await connected.close(); + } + }); +}); + describe("bundle comparison MCP integration", () => { it("reads caller-selected bundle files and returns complete Evidence", async () => { const root = await createTestTempDirectory("rea-bundle-mcp-"); From e67c3642c8920a2a9c0e231ee2f16f50b245df31 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 14 Oct 2026 17:49:17 +0800 Subject: [PATCH 10/20] fix(process): retain deletions after observed roots become obstructed Carry initial root observations into final capture. Treat ENOTDIR as absence only for a previously observed root, preserving initial obstruction and cancellation failures. Refs: #3238, #3263 Co-authored-by: hhang <169027046+xu91102@users.noreply.github.com> --- src/process/capture/FilesystemSnapshot.ts | 35 +++++++++++-- src/process/capture/ProcessHarness.ts | 6 ++- .../process/processCaptureLifecycle.test.ts | 51 ++++++++++++++++++- 3 files changed, 85 insertions(+), 7 deletions(-) diff --git a/src/process/capture/FilesystemSnapshot.ts b/src/process/capture/FilesystemSnapshot.ts index 3ec2e0716a..5defd2574b 100644 --- a/src/process/capture/FilesystemSnapshot.ts +++ b/src/process/capture/FilesystemSnapshot.ts @@ -47,12 +47,26 @@ const fileUnavailableSystemCode = (cause: unknown): string | undefined => { : undefined; }; -const lstatIfPresent = async (path: string): Promise => { +const lstatIfPresent = async ( + path: string, + previouslyObservedRoot = false, + signal?: AbortSignal, +): Promise => { + signal?.throwIfAborted(); try { - return await lstat(path); + const stats = await lstat(path); + signal?.throwIfAborted(); + return stats; } catch (cause: unknown) { - if (cause instanceof Error && "code" in cause && cause.code === "ENOENT") + if ( + cause instanceof Error && + "code" in cause && + (cause.code === "ENOENT" || + (previouslyObservedRoot && cause.code === "ENOTDIR")) + ) { + signal?.throwIfAborted(); return undefined; + } throw cause; } }; @@ -169,7 +183,11 @@ export const hashFile = async ( export const snapshotRoots = async ( scenario: ProcessScenario, signal?: AbortSignal, + previous?: ProcessFilesystemSnapshot, ): Promise => { + // A prior root entry establishes same-run admission. Preserve initial + // obstruction errors; a later non-directory ancestor makes it absent. + const previousPaths = new Set(previous?.files.map(({ path }) => path)); const entries: FileState[] = []; const completeRoots: string[] = []; let remainingBytes = scenario.limits.file_bytes; @@ -198,7 +216,11 @@ export const snapshotRoots = async ( enumerationReasons.add("depth_limit"); return false; } - const stats = await lstatIfPresent(path); + const stats = await lstatIfPresent( + path, + previousPaths.has(`${rootAlias}:.`), + signal, + ); if (stats === undefined) return true; const relativePath = relative(root, path) || "."; if (stats.isSymbolicLink()) { @@ -326,7 +348,10 @@ export const snapshotRoots = async ( }; for (const [index, root] of scenario.filesystem_observation_paths.entries()) { const alias = `root_${String(index)}`; - if ((await lstatIfPresent(root)) === undefined) { + if ( + (await lstatIfPresent(root, previousPaths.has(`${alias}:.`), signal)) === + undefined + ) { completeRoots.push(alias); continue; } diff --git a/src/process/capture/ProcessHarness.ts b/src/process/capture/ProcessHarness.ts index 948c24a835..ef3c844087 100644 --- a/src/process/capture/ProcessHarness.ts +++ b/src/process/capture/ProcessHarness.ts @@ -508,7 +508,11 @@ const completeCapture = async (options: { assertNotCancelled(options.signal); let after: Awaited>; try { - after = await options.captureSnapshot(scenario, options.signal); + after = await options.captureSnapshot( + scenario, + options.signal, + options.before, + ); } catch (cause: unknown) { options.observationBuffer.filesystem_snapshots = { ...options.observationBuffer.filesystem_snapshots, diff --git a/tests/boundary/process/processCaptureLifecycle.test.ts b/tests/boundary/process/processCaptureLifecycle.test.ts index 01d8eba13c..bf566ec029 100644 --- a/tests/boundary/process/processCaptureLifecycle.test.ts +++ b/tests/boundary/process/processCaptureLifecycle.test.ts @@ -1,6 +1,13 @@ import { createHash } from "node:crypto"; import { execFile } from "node:child_process"; -import { access, readFile, rm, symlink, writeFile } from "node:fs/promises"; +import { + access, + mkdir, + readFile, + rm, + symlink, + writeFile, +} from "node:fs/promises"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; import { promisify } from "node:util"; @@ -123,6 +130,48 @@ const terminateFixtureProcess = async (pid: number): Promise => { await waitForProcessExit(pid); }; +itWithCaptureCapability( + "retains a deleted observed file when its parent becomes a regular file", + async () => { + const root = await createTestTempDirectory("rea-observed-parent-type-"); + const payload = '{"result":42}\n'; + await mkdir(join(root, "reports")); + await writeFile(join(root, "reports", "final.json"), payload); + const result = await captureProcessScenario( + parseProcessScenario({ + executable: process.execPath, + arguments: [ + "-e", + `const fs = require('node:fs'); + fs.renameSync('reports', 'saved-reports'); + fs.writeFileSync('reports', 'regular file'); console.log('finished');`, + ], + working_directory: root, + filesystem_observation_paths: [join(root, "reports", "final.json")], + timeout_ms: 5_000, + idle_timeout_ms: 5_000, + settle_ms: 100, + }), + ); + if (!result.ok) throw result.error; + expect(result.value.exit).toMatchObject({ code: 0, reason: "exited" }); + expect(result.value.filesystem_effects).toContainEqual( + expect.objectContaining({ + path: "root_0:.", + status: "deleted", + before: expect.objectContaining({ + type: "file", + sha256: createHash("sha256").update(payload).digest("hex"), + }), + after: null, + }), + ); + expect( + await readFile(join(root, "saved-reports", "final.json"), "utf8"), + ).toBe(payload); + }, +); + itWithCaptureCapability( "keeps the process deadline bounded when the wall clock moves backward", async () => { From 2daeff4b243e1cd137c591ca394e18a1c05c8bbc Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 14 Oct 2026 17:49:18 +0800 Subject: [PATCH 11/20] fix(application): normalize owned target admission cancellation Normalize only the exact aborted signal reason, retaining unrelated failures and descriptor ownership after cleanup failures. Refs: #3197, #3265 Co-authored-by: SII-YDD <38822679+SII-YDD@users.noreply.github.com> --- src/application/BinaryTargetResolver.ts | 4 + .../binaryTargetAdmissionCancellation.test.ts | 256 ++++++++++++++++++ 2 files changed, 260 insertions(+) create mode 100644 tests/process-global/binaryTargetAdmissionCancellation.test.ts diff --git a/src/application/BinaryTargetResolver.ts b/src/application/BinaryTargetResolver.ts index 355c7c12b5..41383ff250 100644 --- a/src/application/BinaryTargetResolver.ts +++ b/src/application/BinaryTargetResolver.ts @@ -223,6 +223,10 @@ export const parseBinaryTarget = async ( } } catch (cause: unknown) { if (cause instanceof AnalysisCancelledError) return err(cause); + if (signal?.aborted === true && cause === signal.reason) + return err( + new AnalysisCancelledError("resolve_binary_target", { cause }), + ); if (cause instanceof BinaryTargetError) return err(cause); if (!(cause instanceof Error)) throw cause; const failure = classifyBinaryTargetFilesystemFailure(candidate, cause); diff --git a/tests/process-global/binaryTargetAdmissionCancellation.test.ts b/tests/process-global/binaryTargetAdmissionCancellation.test.ts new file mode 100644 index 0000000000..ef5c66f51e --- /dev/null +++ b/tests/process-global/binaryTargetAdmissionCancellation.test.ts @@ -0,0 +1,256 @@ +import type { FileHandle } from "node:fs/promises"; +import { readFile, writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { afterEach, expect, it, onTestFinished, vi } from "vitest"; + +const admission = vi.hoisted(() => ({ + path: "", + statCount: 0, + abortAt: 0, + controller: undefined as AbortController | undefined, + reason: undefined as unknown, + statFailure: undefined as Error | undefined, + closeFailure: undefined as Error | undefined, + handles: [] as FileHandle[], + afterStat: undefined as ((count: number) => Promise) | undefined, +})); + +// Only control when cancellation arrives; preserve real open/stat/close IO. +vi.mock("node:fs/promises", async (importOriginal) => { + const fs = await importOriginal(); + return { + ...fs, + open: async (...args: Parameters) => { + const handle = await fs.open(...args); + if (args[0] !== admission.path) return handle; + admission.handles.push(handle); + const stat = handle.stat.bind(handle); + const close = handle.close.bind(handle); + handle.stat = (async (...options: Parameters) => { + const metadata = await stat(...options); + admission.statCount += 1; + await admission.afterStat?.(admission.statCount); + if (admission.statCount === admission.abortAt) { + admission.controller?.abort(admission.reason); + if (admission.statFailure !== undefined) throw admission.statFailure; + } + return metadata; + }) as FileHandle["stat"]; + handle.close = async () => { + if (admission.closeFailure !== undefined) { + const failure = admission.closeFailure; + admission.closeFailure = undefined; + throw failure; + } + await close(); + }; + return handle; + }, + }; +}); + +import { parseBinaryTarget } from "../../src/application/BinaryTargetResolver.js"; +import { projectAnalysisError } from "../../src/domain/analysisErrorProjection.js"; +import { RegularFileAdmissionFailure } from "../../src/filesystem/RegularFile.js"; +import { createApplicationMcpHarness } from "../fixtures/applicationMcpHarness.js"; +import { createTestBinarySession } from "../fixtures/binarySession.js"; +import { advertisedMcpToolSchemas } from "../fixtures/advertisedMcpTool.js"; +import { parseMcpToolError } from "../fixtures/mcpToolError.js"; +import { createTestTempDirectory } from "../fixtures/temporaryDirectory.js"; + +const fixture = async (abortAt: number, reason?: unknown) => { + const root = await createTestTempDirectory("rea-target-admission-cancel-"); + const path = join(root, "sample.bin"); + await writeFile(path, "sample-bytes"); + const controller = new AbortController(); + Object.assign(admission, { + path, + abortAt, + reason, + statCount: 0, + controller, + }); + return { path, controller }; +}; + +afterEach(async () => { + admission.closeFailure = undefined; + for (const handle of admission.handles) + if (handle.fd >= 0) await handle.close(); + Object.assign(admission, { + path: "", + statCount: 0, + abortAt: 0, + controller: undefined, + reason: undefined, + statFailure: undefined, + afterStat: undefined, + handles: [], + }); +}); + +it.each([ + { checkpoint: 1, reason: undefined, name: "default" }, + { checkpoint: 1, reason: new Error("caller cancelled"), name: "Error" }, + { checkpoint: 1, reason: "caller cancelled", name: "string" }, + { checkpoint: 2, reason: undefined, name: "default" }, + { checkpoint: 2, reason: new Error("caller cancelled"), name: "Error" }, + { checkpoint: 2, reason: "caller cancelled", name: "string" }, +])( + "classifies $name cancellation at descriptor stat $checkpoint and closes the file", + async ({ checkpoint, reason }) => { + const { path, controller } = await fixture(checkpoint, reason); + const resolved = await parseBinaryTarget(path, { + signal: controller.signal, + }); + expect(resolved.ok).toBe(false); + if (resolved.ok) throw new Error("cancelled target was admitted"); + expect(projectAnalysisError(resolved.error)).toMatchObject({ + code: "cancelled", + category: "cancelled", + retryable: true, + details: { operation: "resolve_binary_target", cleanup: "complete" }, + }); + expect(admission.handles[0]?.fd).toBe(-1); + expect(await readFile(path, "utf8")).toBe("sample-bytes"); + }, +); + +it("preserves unsupported-format diagnostics when admission is not cancelled", async () => { + const { path, controller } = await fixture(0); + const resolved = await parseBinaryTarget(path, { + signal: controller.signal, + }); + expect(resolved).toMatchObject({ + ok: false, + error: { _tag: "BinaryTargetError", constraint: "file_format_rejected" }, + }); + expect(admission.handles[0]?.fd).toBe(-1); +}); + +it("does not replace an unrelated stat failure with cancellation", async () => { + const { path, controller } = await fixture(1); + const failure = Object.assign(new Error("disk read failed"), { code: "EIO" }); + admission.statFailure = failure; + expect( + await parseBinaryTarget(path, { signal: controller.signal }), + ).toMatchObject({ + ok: false, + error: { + _tag: "BinaryTargetError", + cause: failure, + reason: expect.stringContaining("(EIO)"), + }, + }); + expect(admission.handles[0]?.fd).toBe(-1); +}); + +it("retains the cleanup failure and its file owner when cancelled admission cannot close", async () => { + const reason = new Error("caller cancelled"); + const { path, controller } = await fixture(1, reason); + const cleanup = new Error("descriptor close failed"); + admission.closeFailure = cleanup; + const failure = await parseBinaryTarget(path, { + signal: controller.signal, + }).catch((cause: unknown) => cause); + expect(failure).toBeInstanceOf(RegularFileAdmissionFailure); + if (!(failure instanceof RegularFileAdmissionFailure)) throw failure; + expect(failure.cause).toBe(reason); + expect(failure.cleanupCause).toBe(cleanup); + expect(failure.owner.handle).toBe(admission.handles[0]); + expect(failure.owner.handle.fd).toBeGreaterThanOrEqual(0); + await failure.owner.close(); + expect(failure.owner.handle.fd).toBe(-1); +}); + +it("settles SDK cancellation at the admission boundary and reuses the same MCP connection", async () => { + const { path, controller } = await fixture(0); + const createClient = vi.fn(() => { + throw new Error("native provider must not start during target admission"); + }); + const session = createTestBinarySession(createClient); + const harness = await createApplicationMcpHarness({ session }); + let resume = () => {}; + const resumed = new Promise((resolve) => { + resume = resolve; + }); + let reached = () => {}; + const firstStat = new Promise((resolve) => { + reached = resolve; + }); + let finished = (_result: unknown) => {}; + const completed = new Promise((resolve) => { + finished = resolve; + }); + let cancelled = () => {}; + const serverCancelled = new Promise((resolve) => { + cancelled = resolve; + }); + const open = session.open.bind(session); + vi.spyOn(session, "open").mockImplementation((input, options) => { + options?.signal?.addEventListener("abort", cancelled, { once: true }); + return open(input, options).then( + (result) => { + finished(result); + return result; + }, + (cause: unknown) => { + finished(cause); + throw cause; + }, + ); + }); + onTestFinished(async () => { + resume(); + vi.restoreAllMocks(); + await harness.close(); + }); + admission.afterStat = async (count) => { + if (count === 1) { + reached(); + await resumed; + } + }; + const { client } = harness; + const { name, ajv, inputSchema } = await advertisedMcpToolSchemas( + client, + "open_binary", + ); + const args = { path }; + expect(ajv.validate(inputSchema, args)).toBe(true); + const request = client + .callTool({ name, arguments: args }, { signal: controller.signal }) + .then( + () => "completed", + (cause: unknown) => cause, + ); + await firstStat; + await client.ping(); + controller.abort(new Error("SDK target cancellation")); + expect(await request).toMatchObject({ + message: expect.stringContaining("SDK target cancellation"), + }); + await serverCancelled; + resume(); + const settled = await completed; + expect(settled).toMatchObject({ + ok: false, + error: { + _tag: "AnalysisCancelledError", + operation: "resolve_binary_target", + }, + }); + expect(admission.handles[0]?.fd).toBe(-1); + expect(createClient).not.toHaveBeenCalled(); + await client.ping(); + const retry = await client.callTool({ name, arguments: args }); + expect(parseMcpToolError(retry)).toMatchObject({ + error: { + code: "target_unavailable", + details: { constraint: "file_format_rejected" }, + }, + }); + await client.ping(); + expect(await readFile(path, "utf8")).toBe("sample-bytes"); + expect(admission.handles.every((handle) => handle.fd === -1)).toBe(true); +}); From 45b6a7fe8f51a89951aaa823ffa627f6f5453a33 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 14 Oct 2026 17:49:18 +0800 Subject: [PATCH 12/20] fix(native): distinguish proven pre-launch failure from unknown target ownership Emit explicit target-creation facts only when known. Invalid post-launch process handles remain unknown and retain cleanup ownership. Refs: #3185 Validation uses bridge fakes and provider-boundary tests; no real LLDB target launch was performed. --- bridge/native/rea_lldb_tracer.py | 46 ++++- bridge/native/test_rea_lldb_tracer.py | 37 ++++ src/native/LldbCallTracer.ts | 8 +- .../native/lldbCallTracerFailures.test.ts | 164 +++++++++++++++++- 4 files changed, 245 insertions(+), 10 deletions(-) diff --git a/bridge/native/rea_lldb_tracer.py b/bridge/native/rea_lldb_tracer.py index b99123bf4b..fcec3ea92b 100644 --- a/bridge/native/rea_lldb_tracer.py +++ b/bridge/native/rea_lldb_tracer.py @@ -567,18 +567,27 @@ def trace(debugger, config): before_digest = _hash_file(config["executable"]) before_identity = _file_identity(config["executable"]) except OSError as error: - return {"status": "target-integrity-error", "error": str(error)} + return { + "status": "target-integrity-error", + "error": str(error), + "target_process_created": False, + } if before_digest != config["expected_sha256"]: return { "status": "target-integrity-error", "error": "selected executable digest changed before LLDB target creation", + "target_process_created": False, } error = lldb.SBError() target = debugger.CreateTarget( config["executable"], config["architecture"], None, False, error ) if not target.IsValid(): - return {"status": "target-error", "error": error.GetCString()} + return { + "status": "target-error", + "error": error.GetCString(), + "target_process_created": False, + } module_path_before, module_file_identity_before, module_uuid_before = ( _module_identity(target) ) @@ -589,6 +598,7 @@ def trace(debugger, config): return { "status": "target-integrity-error", "error": "LLDB executable module resolved to a different pre-launch file identity", + "target_process_created": False, } created = [_create_breakpoints(target, spec) for spec in config["breakpoints"]] breakpoint_index = {} @@ -622,12 +632,24 @@ def trace(debugger, config): if not error.Success() or not process.IsValid(): stdout.close() stderr.close() - return {"status": "launch-error", "error": error.GetCString()} + return { + "status": "launch-error", + "error": error.GetCString(), + **( + {"target_process_created": True} + if process.IsValid() + else {} + ), + } launch_failure = _failed_launch_description(process) if launch_failure is not None: stdout.close(expected_exit=True) stderr.close(expected_exit=True) - return {"status": "launch-error", "error": launch_failure} + return { + "status": "launch-error", + "error": launch_failure, + "target_process_created": True, + } stdout.mark_launched() stderr.mark_launched() # Publish the PID before any post-launch file or module inspection. The @@ -644,6 +666,7 @@ def trace(debugger, config): return { "status": "target-identity-unavailable", "error": "host could not establish the launched process identity; target was stopped", + "target_process_created": True, } try: after_digest = _hash_file(config["executable"]) @@ -652,7 +675,11 @@ def trace(debugger, config): process.Kill() stdout.close(expected_exit=process.GetState() == lldb.eStateExited) stderr.close(expected_exit=process.GetState() == lldb.eStateExited) - return {"status": "target-integrity-error", "error": str(identity_error)} + return { + "status": "target-integrity-error", + "error": str(identity_error), + "target_process_created": True, + } if after_digest != config["expected_sha256"] or after_identity != before_identity: process.Kill() _wait_for_exit(listener, process, 5) @@ -661,6 +688,7 @@ def trace(debugger, config): return { "status": "target-integrity-error", "error": "selected executable digest or file identity changed during LLDB launch", + "target_process_created": True, } module_path_after, module_file_identity_after, module_uuid_after = ( _module_identity(target) @@ -676,6 +704,7 @@ def trace(debugger, config): return { "status": "target-integrity-error", "error": "LLDB executable module resolved to a different file identity", + "target_process_created": True, } if ( module_uuid_before is not None @@ -689,6 +718,7 @@ def trace(debugger, config): return { "status": "target-integrity-error", "error": "LLDB executable module identity changed during launch", + "target_process_created": True, } context = { "target": target, @@ -748,7 +778,11 @@ def trace(debugger, config): # exit code; preserve the debugger's concrete launch denial. launch_failure = _failed_launch_description(process) if launch_failure is not None: - return {"status": "launch-error", "error": launch_failure} + return { + "status": "launch-error", + "error": launch_failure, + "target_process_created": True, + } launch_identity_stable = ( module_path_before is not None and module_path_after == module_path_before diff --git a/bridge/native/test_rea_lldb_tracer.py b/bridge/native/test_rea_lldb_tracer.py index 7cd95d5038..308ed00336 100644 --- a/bridge/native/test_rea_lldb_tracer.py +++ b/bridge/native/test_rea_lldb_tracer.py @@ -8,6 +8,7 @@ import sys import tempfile import types import unittest +from unittest.mock import patch fake_lldb = sys.modules.setdefault("lldb", types.ModuleType("lldb")) @@ -22,6 +23,42 @@ spec.loader.exec_module(tracer) class NativeTraceBudgetTests(unittest.TestCase): + def test_pre_target_integrity_failure_reports_no_process_created(self): + original_debugger = getattr(tracer.lldb, "SBDebugger", None) + + class DebuggerApi: + @staticmethod + def SetInternalVariable(*_arguments): + return None + + class Debugger: + def SetAsync(self, _enabled): + return None + + def GetInstanceName(self): + return "fixture" + + def CreateTarget(self, *_arguments): + raise AssertionError("integrity failure precedes target creation") + + tracer.lldb.SBDebugger = DebuggerApi + try: + with patch.object(tracer, "_hash_file", return_value="changed"), patch.object( + tracer, "_file_identity", return_value=(1, 2) + ): + result = tracer.trace( + Debugger(), + {"executable": "fixture", "expected_sha256": "expected"}, + ) + finally: + if original_debugger is None: + del tracer.lldb.SBDebugger + else: + tracer.lldb.SBDebugger = original_debugger + + self.assertEqual(result["status"], "target-integrity-error") + self.assertIs(result["target_process_created"], False) + def test_journal_checks_exact_byte_budget_before_writing(self): record = {"kind": "other-stop", "reason": "signal"} encoded_size = len((tracer.json.dumps(record) + "\n").encode("utf-8")) diff --git a/src/native/LldbCallTracer.ts b/src/native/LldbCallTracer.ts index 9fa5813b86..a7d1e0a3f0 100644 --- a/src/native/LldbCallTracer.ts +++ b/src/native/LldbCallTracer.ts @@ -123,6 +123,7 @@ const tracerOutputSchema = z.discriminatedUnion("status", [ "tracer-error", ]), error: z.string().nullable(), + target_process_created: z.boolean().optional(), }), ]); @@ -423,11 +424,12 @@ export class LldbCallTracer implements NativeCallTracer { let targetTermination: TargetTermination = exited.targetLaunched ? await ensureTerminated(paths.pid, exited.targetIdentity, true) : "terminated"; - // These two bridge outcomes are emitted before a target process exists. - // Other missing-PID cases, including a missing result, remain unknown. + // Only an explicit bridge phase fact may prove that no target process was + // created. A missing PID or an integrity-error status alone is ambiguous. if ( output !== undefined && - (output.status === "target-error" || output.status === "launch-error") && + output.status !== "traced" && + output.target_process_created === false && (await readTargetPid(paths.pid)) === undefined ) targetTermination = "terminated"; diff --git a/tests/boundary/providers/native/lldbCallTracerFailures.test.ts b/tests/boundary/providers/native/lldbCallTracerFailures.test.ts index a8d65dedbd..a365f67733 100644 --- a/tests/boundary/providers/native/lldbCallTracerFailures.test.ts +++ b/tests/boundary/providers/native/lldbCallTracerFailures.test.ts @@ -1,4 +1,4 @@ -import { access, readFile, writeFile } from "node:fs/promises"; +import { access, readFile, rm, writeFile } from "node:fs/promises"; import { dirname } from "node:path"; import { describe, expect, it } from "vitest"; @@ -215,3 +215,165 @@ describe("LLDB failure retention through the production tracer", () => { }); }); }); + +describe("LLDB pre-target phase proof", () => { + it("releases a proven pre-target integrity rejection and allows tracer reuse", async () => { + let launches = 0; + let firstRuntime: string | undefined; + const tracer = new LldbCallTracer( + {}, + async (_executable, arguments_) => { + const { config } = await configFromArguments(arguments_); + launches += 1; + if (firstRuntime === undefined) + firstRuntime = dirname(config.result_path); + await writeFile(config.observation_path, ""); + await writeFile(config.stdout_capture_path, ""); + await writeFile( + config.result_path, + JSON.stringify( + launches === 1 + ? { + status: "target-integrity-error", + error: + "selected executable digest changed before LLDB target creation", + target_process_created: false, + } + : { + status: "target-error", + error: "second launch reached the bridge", + }, + ), + ); + return { + kind: "exited", + targetLaunched: launches === 1, + exitCode: 1, + output: "", + targetIdentity: undefined, + cleanupFailure: undefined, + }; + }, + async () => ok(fakeTool), + ); + + const rejected = await tracer.trace(request); + if (rejected.ok) throw new Error("expected pre-target integrity failure"); + expect(rejected.error).toBeInstanceOf(EvidenceIntegrityError); + expect(projectAnalysisError(rejected.error).code).toBe( + "evidence_integrity_mismatch", + ); + if (firstRuntime === undefined) throw new Error("LLDB was not launched"); + await expect(access(firstRuntime)).rejects.toThrow(); + + const retried = await tracer.trace(request); + expect(launches).toBe(2); + if (retried.ok) throw new Error("expected the second bridge result"); + expect(retried.error).toMatchObject({ + _tag: "AnalysisCapabilityUnavailableError", + reason: expect.stringContaining("second launch reached the bridge"), + }); + }); +}); + +describe("LLDB retains ambiguous target ownership", () => { + it("keeps a post-target integrity rejection unresolved when its PID is missing", async () => { + let runtime: string | undefined; + const tracer = new LldbCallTracer( + {}, + async (_executable, arguments_) => { + const { config } = await configFromArguments(arguments_); + runtime = dirname(config.result_path); + await writeFile(config.observation_path, ""); + await writeFile(config.stdout_capture_path, ""); + await writeFile( + config.result_path, + JSON.stringify({ + status: "target-integrity-error", + error: "selected executable changed during LLDB launch", + target_process_created: true, + }), + ); + return { + kind: "exited", + targetLaunched: true, + exitCode: 1, + output: "", + targetIdentity: undefined, + cleanupFailure: undefined, + }; + }, + async () => ok(fakeTool), + ); + + try { + const result = await tracer.trace(request); + if (result.ok) throw new Error("expected unresolved target identity"); + expect(projectAnalysisError(result.error)).toMatchObject({ + code: "cleanup_incomplete", + details: { + resources: expect.arrayContaining(["native-target:unknown"]), + }, + }); + expect(runtime).toBeDefined(); + if (runtime !== undefined) await access(runtime); + const retry = await tracer.trace(request); + if (retry.ok) throw new Error("expected retained target cleanup"); + expect(projectAnalysisError(retry.error).code).toBe("cleanup_incomplete"); + } finally { + if (runtime !== undefined) + await rm(runtime, { recursive: true, force: true }); + } + }); + + it("keeps an invalid process after Launch unresolved without producer proof", async () => { + let launches = 0; + let runtime: string | undefined; + const tracer = new LldbCallTracer( + {}, + async (_executable, arguments_) => { + launches += 1; + const { config } = await configFromArguments(arguments_); + runtime = dirname(config.result_path); + await writeFile(config.observation_path, ""); + await writeFile(config.stdout_capture_path, ""); + await writeFile( + config.result_path, + JSON.stringify({ + status: "launch-error", + error: "LLDB returned an invalid process after Launch", + }), + ); + return { + kind: "exited", + targetLaunched: true, + exitCode: 1, + output: "", + targetIdentity: undefined, + cleanupFailure: undefined, + }; + }, + async () => ok(fakeTool), + ); + + try { + const result = await tracer.trace(request); + if (result.ok) throw new Error("expected unresolved launch cleanup"); + expect(projectAnalysisError(result.error)).toMatchObject({ + code: "cleanup_incomplete", + details: { + resources: expect.arrayContaining(["native-target:unknown"]), + }, + }); + expect(runtime).toBeDefined(); + if (runtime !== undefined) await access(runtime); + const retry = await tracer.trace(request); + if (retry.ok) throw new Error("expected retained target cleanup"); + expect(projectAnalysisError(retry.error).code).toBe("cleanup_incomplete"); + expect(launches).toBe(1); + } finally { + if (runtime !== undefined) + await rm(runtime, { recursive: true, force: true }); + } + }); +}); From 3f6b5755b3e699bd9373641acffe0f2c5d2ab094 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 14 Oct 2026 17:49:18 +0800 Subject: [PATCH 13/20] fix(managed)!: share member list facts and retain verification uncertainty Parse TypeDef member lists once at their owning boundary, retain malformed ranges as unknown, and admit representation storage before allocation. Reuse parsed facts across member ownership, inventory, attributes, and native boundaries. Preserve constant operands in structural matching. Refs: #3202, #3201, #3199 BREAKING CHANGE: unresolved managed/native verification results may retain a non-null matched_native and candidate facts when module identity is unknown. Consumers must use status and reason rather than assume unresolved observations are empty. --- docs/managed-code-analysis.md | 8 +- .../managed/managedWorkflowToolContracts.ts | 2 +- .../managed/managedMemberComparisonMatch.ts | 3 +- .../managed/managedNativeVerificationMatch.ts | 82 ++++++--- .../managedNativeVerificationSchemas.ts | 10 +- src/dotnet/ManagedArtifactInspector.ts | 3 + src/dotnet/ManagedMemberInspector.test.ts | 169 +++++++++++++++++- src/dotnet/ManagedMemberInspector.ts | 16 +- src/dotnet/ManagedMemberInspectorCore.ts | 73 ++++---- src/dotnet/ManagedMemberOwnership.test.ts | 73 ++++---- src/dotnet/ManagedMemberOwnership.ts | 58 +++--- src/dotnet/ManagedMemberPointers.ts | 16 +- src/dotnet/ManagedMemberRows.ts | 8 +- src/dotnet/ManagedMetadataInventory.ts | 16 +- src/dotnet/ManagedMetadataInventoryRows.ts | 25 ++- src/dotnet/ManagedNativeBoundaryInspector.ts | 3 + src/dotnet/ManagedTypeMemberLists.ts | 147 +++++++++++++++ ...anagedMemberComparisonFromMetadata.test.ts | 69 +++++++ .../managed/managedNativeVerification.test.ts | 61 +++++-- 19 files changed, 683 insertions(+), 159 deletions(-) create mode 100644 src/dotnet/ManagedTypeMemberLists.ts diff --git a/docs/managed-code-analysis.md b/docs/managed-code-analysis.md index f2d211d4ea..27b7c7e54b 100644 --- a/docs/managed-code-analysis.md +++ b/docs/managed-code-analysis.md @@ -323,6 +323,9 @@ function implements a specific managed method. `verify_managed_native_boundaries` compares declared P/Invoke module and entry point identities with supplied native export or function evidence. Name/module matches establish declaration links; they do not establish that a call executed. +When either side lacks a usable module identity, a matching name remains +unresolved and retains the candidate evidence; REA reports a module mismatch +only when both identities are available and differ. Metadata tokens are not native addresses. Native body mapping for C++/CLI, ReadyToRun, NativeAOT, and IL2CPP requires separately verified format and provider @@ -360,8 +363,9 @@ these indirections for member ownership and local attribute-constructor types; field and method tokens still identify the original definition rows. `field_list` and `method_list` retain the metadata's list coordinates, which refer to the pointer table when present rather than necessarily contiguous -definition rows. Malformed or ambiguous pointers leave the owner unknown with -partial coverage while preserving readable member and attribute facts. +definition rows. Invalid or nonmonotonic list coordinates and malformed or +ambiguous pointers leave affected owners unknown with partial coverage while +preserving readable member and attribute facts. The retained inverse indexes are admitted against the existing representation budget before allocation. diff --git a/src/contracts/managed/managedWorkflowToolContracts.ts b/src/contracts/managed/managedWorkflowToolContracts.ts index 18d78a3102..a668e4a008 100644 --- a/src/contracts/managed/managedWorkflowToolContracts.ts +++ b/src/contracts/managed/managedWorkflowToolContracts.ts @@ -116,7 +116,7 @@ export const MANAGED_WORKFLOW_TOOL_CONTRACTS = [ name: "verify_managed_native_boundaries", ...toolContractMetadata("verify_managed_native_boundaries"), description: - "Verify managed P/Invoke/native-boundary declarations against authenticated native export or function Evidence without executing managed code or translating managed metadata tokens into native addresses. The workflow preserves declaration-only, verified, inferred, contradicted, and unresolved states.", + "Verify managed P/Invoke/native-boundary declarations against authenticated native export or function Evidence without executing managed code or translating managed metadata tokens into native addresses. The workflow preserves declaration-only, verified, inferred, contradicted, and unresolved states; a name candidate with missing managed or native module identity remains unresolved with its candidate evidence retained.", kind: "application", inputSchema: managedNativeVerificationInputSchema, outputSchema: nativeVerificationOutputSchema, diff --git a/src/domain/managed/managedMemberComparisonMatch.ts b/src/domain/managed/managedMemberComparisonMatch.ts index 2cbe32a4a6..e18da9ef59 100644 --- a/src/domain/managed/managedMemberComparisonMatch.ts +++ b/src/domain/managed/managedMemberComparisonMatch.ts @@ -87,9 +87,10 @@ const keyMethod = (item: Method): Keyed => ({ item.body.max_stack, item.body.init_locals, item.body.opcode_counts, - item.body.anchors.map(({ opcode, operand_kind }) => [ + item.body.anchors.map(({ opcode, operand_kind, operand }) => [ opcode, operand_kind, + operand_kind === "constant" ? operand : null, ]), item.body.exception_regions.map((region) => [ region.flags, diff --git a/src/domain/managed/managedNativeVerificationMatch.ts b/src/domain/managed/managedNativeVerificationMatch.ts index bbe886a708..49cb0cdf34 100644 --- a/src/domain/managed/managedNativeVerificationMatch.ts +++ b/src/domain/managed/managedNativeVerificationMatch.ts @@ -124,13 +124,21 @@ const moduleCompatible = ( scope: string | null, symbol: NativeSymbol, ): boolean => { - if (scope === null) return true; + if (scope === null || scope.trim().length === 0) return false; const expected = normalizeModule(scope); return [symbol.module_name, symbol.module_path] - .filter((value): value is string => value !== null) + .filter( + (value): value is string => + value !== null && normalizeModule(value).length > 0, + ) .some((value) => nativeModuleIdentities(value).has(expected)); }; +const hasModuleIdentity = (symbol: NativeSymbol): boolean => + [symbol.module_name, symbol.module_path].some( + (value) => value !== null && normalizeModule(value).length > 0, + ); + const candidateNames = ( managed: Pick< PinvokeImport, @@ -190,6 +198,9 @@ const matchPinvoke = ( }; const candidates: [NativeSymbol, ...NativeSymbol[]] = [first, ...remaining]; + const scopeKnown = + managed.import_scope_name !== null && + managed.import_scope_name.trim().length > 0; const exact = candidates.find( (symbol) => managed.import_name === symbol.name && @@ -198,17 +209,13 @@ const matchPinvoke = ( const decorated = candidates.find((symbol) => moduleCompatible(managed.import_scope_name, symbol), ); - const selected = exact ?? decorated ?? first; - if (!moduleCompatible(managed.import_scope_name, selected)) - return { - match: { - status: "contradicted", - basis: "module-mismatch", - confidence: "unknown", - matched_native: selected, - candidates, - }, - }; + const selected = + exact ?? + decorated ?? + (scopeKnown + ? candidates.find((symbol) => !hasModuleIdentity(symbol)) + : undefined) ?? + first; if (exact !== undefined) return { match: { @@ -222,11 +229,31 @@ const matchPinvoke = ( candidates, }, }; + if (decorated !== undefined) + return { + match: { + basis: "decorated-name-candidate", + status: "inferred", + confidence: "inferred", + matched_native: selected, + candidates, + }, + }; + if (!scopeKnown || !hasModuleIdentity(selected)) + return { + match: { + status: "unresolved", + basis: "module-identity-unknown", + confidence: "unknown", + matched_native: selected, + candidates, + }, + }; return { match: { - basis: "decorated-name-candidate", - status: "inferred", - confidence: "inferred", + status: "contradicted", + basis: "module-mismatch", + confidence: "unknown", matched_native: selected, candidates, }, @@ -234,26 +261,31 @@ const matchPinvoke = ( }; const pinvokeLimitations = ( - status: PinvokeVerification["status"], + match: PinvokeMatch, managed: PinvokeImport, ): readonly string[] => [ - ...(managed.import_scope_name === null + ...(managed.import_scope_name === null || + managed.import_scope_name.trim().length === 0 ? ["Managed declaration does not name an import scope/module."] : []), - ...(status === "inferred" + ...(match.status === "inferred" ? [ "Native symbol matched a decorated candidate rather than the exact declared import name.", ] : []), - ...(status === "contradicted" + ...(match.status === "contradicted" ? [ "A symbol name candidate exists, but the supplied native Evidence module identity does not match the managed import scope.", ] : []), - ...(status === "unresolved" - ? [ - "No supplied native export/function Evidence matched this declared import.", - ] + ...(match.status === "unresolved" + ? match.basis === "module-identity-unknown" + ? [ + "A native name candidate exists, but a required module identity is unavailable; module correspondence is unknown.", + ] + : [ + "No supplied native export/function Evidence matched this declared import.", + ] : []), ]; @@ -283,7 +315,7 @@ export const verifyPinvoke = ({ declaration_verification: item.verification, }; const selection = matchPinvoke(item, native.symbols, native.accepted); - const limitations = pinvokeLimitations(selection.match.status, item); + const limitations = pinvokeLimitations(selection.match, item); return { verification: pinvokeVerificationSchema.parse({ item_id: `mnv_pinvoke_${digest({ diff --git a/src/domain/managed/managedNativeVerificationSchemas.ts b/src/domain/managed/managedNativeVerificationSchemas.ts index 16afe4048a..c83488a50d 100644 --- a/src/domain/managed/managedNativeVerificationSchemas.ts +++ b/src/domain/managed/managedNativeVerificationSchemas.ts @@ -51,7 +51,7 @@ const observedCandidatesSchema = z .tuple([nativeSymbolSchema]) .rest(nativeSymbolSchema); -export const pinvokeVerificationSchema = z.discriminatedUnion("status", [ +export const pinvokeVerificationSchema = z.union([ z.strictObject({ ...pinvokeVerificationContextShape, status: z.literal("verified"), @@ -76,6 +76,14 @@ export const pinvokeVerificationSchema = z.discriminatedUnion("status", [ matched_native: z.null(), candidates: emptyArraySchema, }), + z.strictObject({ + ...pinvokeVerificationContextShape, + status: z.literal("unresolved"), + basis: z.literal("module-identity-unknown"), + confidence: z.literal("unknown"), + matched_native: nativeSymbolSchema, + candidates: observedCandidatesSchema, + }), z.strictObject({ ...pinvokeVerificationContextShape, status: z.literal("contradicted"), diff --git a/src/dotnet/ManagedArtifactInspector.ts b/src/dotnet/ManagedArtifactInspector.ts index 625969aba8..ab40afa9f8 100644 --- a/src/dotnet/ManagedArtifactInspector.ts +++ b/src/dotnet/ManagedArtifactInspector.ts @@ -24,6 +24,7 @@ import { readManagedMetadataLayout, type ManagedMetadataLayout, } from "./ManagedMetadataLayout.js"; +import { readTypeMemberListFacts } from "./ManagedTypeMemberLists.js"; import { ManagedReaderFailure } from "./ManagedReaderFailure.js"; const flagNames = (flags: number): string[] => { @@ -384,10 +385,12 @@ export const inspectManagedArtifactBytes = ( resourceIssues, ); try { + const memberLists = readTypeMemberListFacts(bytes, metadata); const inventory = readManagedMetadataInventory( bytes, metadata, resourceDirectory, + memberLists, ); const issues = [...resourceIssues, ...inventory.issues]; const limitations = [ diff --git a/src/dotnet/ManagedMemberInspector.test.ts b/src/dotnet/ManagedMemberInspector.test.ts index ea4c41ac7b..69fadc6290 100644 --- a/src/dotnet/ManagedMemberInspector.test.ts +++ b/src/dotnet/ManagedMemberInspector.test.ts @@ -11,6 +11,7 @@ import { type ManagedMetadataLayout, } from "./ManagedMetadataLayout.js"; import { readManagedPeLayout } from "./ManagedPeReader.js"; +import { memberListIndexSize } from "./ManagedMemberPointers.js"; import { edges } from "./ManagedMemberRows.js"; import { parseUserStrings } from "./ManagedUserStrings.js"; import { signature, type ManagedMethod } from "./ManagedMemberInspectorCore.js"; @@ -1224,7 +1225,173 @@ describe("managed derived caller names", () => { }); }); -describe("managed member reference bounds", () => { +describe("managed TypeDef list ownership bounds", () => { + it("retains rows but leaves ownership unknown for zero TypeDef list starts", () => { + const bytes = buildManagedPeFixture(); + const pe = readManagedPeLayout(bytes); + if (pe.cli === null) throw new Error("fixture has no CLI header"); + const { rva, size } = pe.cli.metadata; + const layout = readManagedMetadataLayout( + bytes, + pe.rvaToOffset(rva, size, "cli.metadata"), + size, + ); + const listOffset = + metadataRowOffset(layout, 2, 1) + + 4 + + 2 * layout.stringIndexSize + + layout.codedIndexSize("TypeDefOrRef"); + const listIndexWidth = Math.max( + layout.tableIndexSize(3), + layout.tableIndexSize(4), + layout.tableIndexSize(5), + layout.tableIndexSize(6), + ); + const writeIndex = (offset: number) => + listIndexWidth === 2 + ? bytes.writeUInt16LE(0, offset) + : bytes.writeUInt32LE(0, offset); + writeIndex(listOffset); + writeIndex(listOffset + listIndexWidth); + + const result = inspectManagedMembersBytes( + bytes, + managedPeFixtureTarget(bytes), + ); + + expect(result.types[0]).toMatchObject({ + field_list: { first_row: null, last_row: null, count: 0 }, + method_list: { first_row: null, last_row: null, count: 0 }, + }); + expect(result.fields[0]).toMatchObject({ + token: "0x04000001", + name: "counter", + declaring_type: null, + }); + expect(result.methods[0]).toMatchObject({ + token: "0x06000001", + name: "Main", + declaring_type: null, + }); + expect(result.coverage).toMatchObject({ + state: "partial", + issues: [ + expect.objectContaining({ + code: "invalid-row", + detail: expect.stringContaining("TypeDef FieldList start 0"), + }), + expect.objectContaining({ + code: "invalid-row", + detail: expect.stringContaining("TypeDef MethodList start 0"), + }), + ], + }); + }); + + it("does not infer an earlier owner from an invalid next list endpoint", () => { + const bytes = buildManagedPeFixture({ additionalEmptyTypes: 1 }); + const pe = readManagedPeLayout(bytes); + if (pe.cli === null) throw new Error("fixture has no CLI header"); + const { rva, size } = pe.cli.metadata; + const layout = readManagedMetadataLayout( + bytes, + pe.rvaToOffset(rva, size, "cli.metadata"), + size, + ); + const methodStartOffset = + metadataRowOffset(layout, 2, 2) + + 4 + + 2 * layout.stringIndexSize + + layout.codedIndexSize("TypeDefOrRef") + + memberListIndexSize(layout, 4); + if (memberListIndexSize(layout, 6) === 2) + bytes.writeUInt16LE(0, methodStartOffset); + else bytes.writeUInt32LE(0, methodStartOffset); + + const result = inspectManagedMembersBytes( + bytes, + managedPeFixtureTarget(bytes), + ); + + expect(result.methods[0]).toMatchObject({ + token: "0x06000001", + declaring_type: null, + }); + expect(result.types[0]?.method_list).toMatchObject({ + first_row: null, + last_row: null, + count: 0, + }); + expect(result.coverage.state).toBe("partial"); + }); +}); + +describe("managed TypeDef list ordering", () => { + it("leaves adjacent ownership unknown when valid list starts decrease", () => { + const bytes = buildManagedPeFixture({ + additionalEmptyTypes: 1, + methodRowCount: 2, + }); + const pe = readManagedPeLayout(bytes); + if (pe.cli === null) throw new Error("fixture has no CLI header"); + const { rva, size } = pe.cli.metadata; + const layout = readManagedMetadataLayout( + bytes, + pe.rvaToOffset(rva, size, "cli.metadata"), + size, + ); + const methodOffset = + 4 + + 2 * layout.stringIndexSize + + layout.codedIndexSize("TypeDefOrRef") + + memberListIndexSize(layout, 4); + const writeMethodStart = (row: number, value: number): void => { + const offset = metadataRowOffset(layout, 2, row) + methodOffset; + if (memberListIndexSize(layout, 6) === 2) + bytes.writeUInt16LE(value, offset); + else bytes.writeUInt32LE(value, offset); + }; + writeMethodStart(1, 2); + writeMethodStart(2, 1); + + const result = inspectManagedMembersBytes( + bytes, + managedPeFixtureTarget(bytes), + ); + + expect(result.methods.map(({ declaring_type }) => declaring_type)).toEqual([ + null, + null, + ]); + expect(result.coverage).toMatchObject({ + state: "partial", + issues: [ + expect.objectContaining({ + code: "invalid-row", + detail: expect.stringContaining( + "MethodList start 1 decreases from preceding start 2", + ), + }), + ], + }); + }); + + it("accepts the one-based start for an empty ordinary member table", () => { + const bytes = buildManagedPeFixture({ methodRowCount: 0 }); + const result = inspectManagedMembersBytes( + bytes, + managedPeFixtureTarget(bytes), + ); + expect(result.types[0]?.method_list).toEqual({ + first_row: null, + last_row: null, + count: 0, + }); + expect(result.coverage).toMatchObject({ state: "complete", issues: [] }); + }); +}); + +describe("managed metadata reference bounds", () => { it("retains members and reports an out-of-range TypeDef coded index as partial", () => { const bytes = buildManagedPeFixture({ extendsRaw: 12 }); const result = inspectManagedMembersBytes( diff --git a/src/dotnet/ManagedMemberInspector.ts b/src/dotnet/ManagedMemberInspector.ts index d6464b09d7..ab074d146b 100644 --- a/src/dotnet/ManagedMemberInspector.ts +++ b/src/dotnet/ManagedMemberInspector.ts @@ -31,6 +31,7 @@ import { import { edgeTargets } from "./ManagedMethodSpecs.js"; import { typeNameLookup } from "./ManagedTypeNames.js"; import { parseUserStrings } from "./ManagedUserStrings.js"; +import { readTypeMemberListFacts } from "./ManagedTypeMemberLists.js"; import { edges, parseExportedTypes, @@ -113,12 +114,14 @@ const readMemberInventory = ( rootOffset, cli.metadata.size, ); + const memberLists = readTypeMemberListFacts(bytes, layout); const inventory = readManagedMetadataInventory( bytes, layout, readManagedResourceDirectory(pe, issues), + memberLists, ); - return { layout, inventory }; + return { layout, inventory, memberLists }; }; const selectTypes = ( @@ -185,9 +188,13 @@ export const inspectManagedMembersBytes = ( return unavailable(target, bytes, pe.cliIssue, typeNames); const issues: ManagedParseIssue[] = []; try { - const { layout, inventory } = readMemberInventory(bytes, pe, issues); + const { layout, inventory, memberLists } = readMemberInventory( + bytes, + pe, + issues, + ); appendManagedParseIssues(issues, inventory.issues); - const ranges = typeRanges(bytes, layout, issues); + const ranges = typeRanges(bytes, layout, issues, memberLists); const { tokens: selected, selection, @@ -197,7 +204,7 @@ export const inspectManagedMembersBytes = ( appendManagedParseIssues(issues, parsedTypes.issues); const exportedTypes = parseExportedTypes(bytes, layout); appendManagedParseIssues(issues, exportedTypes.issues); - const fields = parseFields(bytes, layout, ranges, selected); + const fields = parseFields(bytes, layout, ranges, memberLists, selected); const memberRefs = parseMemberRefs( bytes, layout, @@ -209,6 +216,7 @@ export const inspectManagedMembersBytes = ( layout, pe, ranges, + memberLists, selected, methodOffset, methodLimit, diff --git a/src/dotnet/ManagedMemberInspectorCore.ts b/src/dotnet/ManagedMemberInspectorCore.ts index caac2e66eb..858a6c2fbc 100644 --- a/src/dotnet/ManagedMemberInspectorCore.ts +++ b/src/dotnet/ManagedMemberInspectorCore.ts @@ -20,10 +20,8 @@ import { readMetadataString, sha256Bytes, } from "./ManagedMetadataHeaps.js"; -import { - memberListIndexSize, - memberListTable, -} from "./ManagedMemberPointers.js"; +import { memberListTable } from "./ManagedMemberPointers.js"; +import type { TypeMemberListFacts } from "./ManagedTypeMemberLists.js"; export type ManagedType = ManagedMemberInspection["types"][number]; export type ManagedExportedType = NonNullable< @@ -47,10 +45,10 @@ export type ManagedExceptionRegion = export interface TypeRange { readonly token: string; readonly fullName: string | null; - readonly fieldStart: number; - readonly fieldEnd: number; - readonly methodStart: number; - readonly methodEnd: number; + readonly fieldStart: number | null; + readonly fieldEnd: number | null; + readonly methodStart: number | null; + readonly methodEnd: number | null; } /** @@ -80,18 +78,23 @@ export interface ParsedInstruction { const rowRange = ( table: MetadataTableLayout | undefined, - start: number, - nextStart: number, + start: number | null, + end: number | null, ): { readonly first: number | null; readonly last: number | null; readonly count: number; } => { const total = table?.rowCount ?? 0; - if (total === 0 || start === 0) return { first: null, last: null, count: 0 }; - const end = Math.min(nextStart === 0 ? total + 1 : nextStart, total + 1); - if (start >= end) return { first: null, last: null, count: 0 }; - return { first: start, last: end - 1, count: end - start }; + if (start === null || end === null) + return { first: null, last: null, count: 0 }; + const lastExclusive = Math.min(end, total + 1); + if (start >= lastExclusive) return { first: null, last: null, count: 0 }; + return { + first: start, + last: lastExclusive - 1, + count: lastExclusive - start, + }; }; export const fullName = (namespace: string, name: string): string => @@ -102,9 +105,8 @@ const readTypeRange = ( layout: ManagedMetadataLayout, row: number, issues: ManagedParseIssue[], + memberLists: TypeMemberListFacts, ): TypeRange => { - const methods = memberListTable(layout, 6); - const fields = memberListTable(layout, 4); const cursor = metadataRowCursor(bytes, layout, 2, row); cursor.readUInt32(); const nameIndex = cursor.readIndex(layout.stringIndexSize); @@ -124,27 +126,14 @@ const readTypeRange = ( issues, ) ?? null; cursor.readIndex(layout.codedIndexSize("TypeDefOrRef")); - const fieldStart = cursor.readIndex(memberListIndexSize(layout, 4)); - const methodStart = cursor.readIndex(memberListIndexSize(layout, 6)); - let nextField = (fields?.rowCount ?? 0) + 1; - let nextMethod = (methods?.rowCount ?? 0) + 1; - const typeTable = layout.table(2); - if (typeTable !== undefined && row < typeTable.rowCount) { - const next = metadataRowCursor(bytes, layout, 2, row + 1); - next.readUInt32(); - next.readIndex(layout.stringIndexSize); - next.readIndex(layout.stringIndexSize); - next.readIndex(layout.codedIndexSize("TypeDefOrRef")); - nextField = next.readIndex(memberListIndexSize(layout, 4)); - nextMethod = next.readIndex(memberListIndexSize(layout, 6)); - } + const current = memberLists.entries[row - 1]; return { token: metadataToken(2, row), fullName: declaredName, - fieldStart, - fieldEnd: nextField, - methodStart, - methodEnd: nextMethod, + fieldStart: current?.field.start.value ?? null, + fieldEnd: current?.field.end ?? null, + methodStart: current?.method.start.value ?? null, + methodEnd: current?.method.end ?? null, }; }; @@ -152,11 +141,13 @@ export const typeRanges = ( bytes: Buffer, layout: ManagedMetadataLayout, issues: ManagedParseIssue[], + memberLists: TypeMemberListFacts, ): readonly TypeRange[] => { + issues.push(...memberLists.issues); const typeTable = layout.table(2); const ranges: TypeRange[] = []; for (let row = 1; row <= (typeTable?.rowCount ?? 0); row += 1) - ranges.push(readTypeRange(bytes, layout, row, issues)); + ranges.push(readTypeRange(bytes, layout, row, issues, memberLists)); return ranges; }; @@ -193,18 +184,16 @@ export const parseTypes = ( const extendsRaw = cursor.readIndex( layout.codedIndexSize("TypeDefOrRef"), ); - const fieldStart = cursor.readIndex(memberListIndexSize(layout, 4)); - const methodStart = cursor.readIndex(memberListIndexSize(layout, 6)); - const nextRange = ranges[row] ?? null; + const currentRange = ranges[row - 1] ?? null; const fieldRange = rowRange( fields, - fieldStart, - nextRange?.fieldStart ?? 0, + currentRange?.fieldStart ?? null, + currentRange?.fieldEnd ?? null, ); const methodRange = rowRange( methods, - methodStart, - nextRange?.methodStart ?? 0, + currentRange?.methodStart ?? null, + currentRange?.methodEnd ?? null, ); const extendsReason = metadataCodedTokenInvalidReason( extendsRaw, diff --git a/src/dotnet/ManagedMemberOwnership.test.ts b/src/dotnet/ManagedMemberOwnership.test.ts index 9e570a195e..cee5e985a7 100644 --- a/src/dotnet/ManagedMemberOwnership.test.ts +++ b/src/dotnet/ManagedMemberOwnership.test.ts @@ -6,6 +6,7 @@ import { inspectManagedMembersBytes } from "./ManagedMemberInspector.js"; import { managedDecodeBudget } from "./ManagedDecodeBudget.js"; import { memberDeclaringTypeRowLookup } from "./ManagedMemberOwnership.js"; import { memberListIndexSize } from "./ManagedMemberPointers.js"; +import { readTypeMemberListFacts } from "./ManagedTypeMemberLists.js"; import { readManagedMetadataLayout } from "./ManagedMetadataLayout.js"; import { buildManagedMemberPointerFixture, @@ -28,15 +29,15 @@ const metadataLayout = (bytes: Buffer) => { describe("managed declaring type ownership", () => { it.prop([ fc.tuple( - fc.tuple( - ...Array.from({ length: 3 }, () => fc.integer({ min: 0, max: 6 })), - ), - fc.tuple( - ...Array.from({ length: 3 }, () => fc.integer({ min: 0, max: 6 })), - ), + fc + .array(fc.integer({ min: 1, max: 3 }), { minLength: 3, maxLength: 3 }) + .map((starts) => starts.sort((left, right) => left - right)), + fc + .array(fc.integer({ min: 1, max: 3 }), { minLength: 3, maxLength: 3 }) + .map((starts) => starts.sort((left, right) => left - right)), ), ])( - "retains first-match field and method owners for arbitrary metadata lists", + "matches literal ownership ranges for valid nondecreasing list starts", ([fields, methods]) => { const bytes = buildManagedMemberPointerFixture(); const layout = metadataLayout(bytes); @@ -64,11 +65,16 @@ describe("managed declaring type ownership", () => { ] as const) { // The fixture's pointers reorder physical rows 1,2 to list positions 2,1. const expected = [2, 1].map((position) => { - const owner = starts.findIndex( - (start, index) => - position >= start && position < (starts[index + 1] ?? 3), - ); - return owner < 0 ? null : members.types[owner]?.full_name; + let owner: number | null = null; + for (let index = 0; index < starts.length; index += 1) { + const start = starts[index]; + const end = starts[index + 1] ?? 3; + if (start !== undefined && position >= start && position < end) { + owner = index; + break; + } + } + return owner === null ? null : members.types[owner]?.full_name; }); expect(members[table].map((member) => member.declaring_type)).toEqual( expected, @@ -80,10 +86,10 @@ describe("managed declaring type ownership", () => { }, ); - it("indexes large unordered member lists once per layout", () => { - const count = 100_000; + it("indexes wide valid member lists once per layout", () => { + const count = 65_538; const bytes = buildManagedPeFixture({ - additionalEmptyTypes: count - 1, + additionalEmptyTypes: count - 3, methodRowCount: count, }); const layout = metadataLayout(bytes); @@ -95,24 +101,25 @@ describe("managed declaring type ownership", () => { 2 * layout.stringIndexSize + layout.codedIndexSize("TypeDefOrRef") + memberListIndexSize(layout, 4); - // The first reversed range forces first-match semantics, with the owner last. - // 100,000 MethodDef rows require a four-byte method-list index. - bytes.writeUInt32LE(2, methodOffset); - const start = performance.now(); - const lookup = memberDeclaringTypeRowLookup(bytes, layout, 6); - expect( - Array.from({ length: count }, (_, row) => lookup(row + 1)).every( - (owner) => owner === count, - ), - ).toBe(true); - expect(performance.now() - start).toBeLessThan(2000); - expect(memberDeclaringTypeRowLookup(bytes, layout, 6)).toBe(lookup); + // Each preceding TypeDef owns one row; later TypeDefs have empty intervals. + // This exercises the four-byte list-index boundary without malformed starts. + for (let row = 0; row < count; row += 1) + bytes.writeUInt32LE( + Math.min(row + 1, 65_537), + methodOffset + row * types.rowSize, + ); + const facts = readTypeMemberListFacts(bytes, layout); + const lookup = memberDeclaringTypeRowLookup(bytes, layout, 6, facts); + expect(lookup(65_536)).toBe(65_536); + expect(lookup(65_535)).toBe(65_535); + expect(memberDeclaringTypeRowLookup(bytes, layout, 6, facts)).toBe(lookup); expect([lookup(0), lookup(count + 1)]).toEqual([0, 0]); }); it("admits ownership storage before allocation and retains failed admission", () => { const bytes = buildManagedPeFixture(); const layout = metadataLayout(bytes); + const facts = readTypeMemberListFacts(bytes, layout); const budget = managedDecodeBudget(layout); const capacityFailure = new ManagedReaderFailure({ code: "resource-limit", @@ -134,12 +141,12 @@ describe("managed declaring type ownership", () => { }), ); try { - expect(() => memberDeclaringTypeRowLookup(bytes, layout, 6)).toThrow( - capacityFailure, - ); - expect(() => memberDeclaringTypeRowLookup(bytes, layout, 6)).toThrow( - capacityFailure, - ); + expect(() => + memberDeclaringTypeRowLookup(bytes, layout, 6, facts), + ).toThrow(capacityFailure); + expect(() => + memberDeclaringTypeRowLookup(bytes, layout, 6, facts), + ).toThrow(capacityFailure); expect(admission).toHaveBeenCalledTimes(1); expect(allocations).toBe(0); } finally { diff --git a/src/dotnet/ManagedMemberOwnership.ts b/src/dotnet/ManagedMemberOwnership.ts index 0292f5e0ab..6119b214f5 100644 --- a/src/dotnet/ManagedMemberOwnership.ts +++ b/src/dotnet/ManagedMemberOwnership.ts @@ -1,33 +1,35 @@ import { managedDecodeBudget } from "./ManagedDecodeBudget.js"; -import { metadataRowCursor } from "./ManagedMetadataHeaps.js"; import type { ManagedMetadataLayout } from "./ManagedMetadataLayout.js"; import { - memberListIndexSize, memberListRowLookup, memberListTable, } from "./ManagedMemberPointers.js"; import { ManagedReaderFailure } from "./ManagedReaderFailure.js"; +import type { + MemberDefinitionTable, + TypeMemberListFacts, +} from "./ManagedTypeMemberLists.js"; -type MemberTable = 4 | 6; type OwnerLookup = (definitionRow: number) => number; const lookups = new WeakMap< - ManagedMetadataLayout, - Map + TypeMemberListFacts, + Map >(); /** One first-match TypeDef owner per physical member, shared by every inspection facet. */ export const memberDeclaringTypeRowLookup = ( bytes: Buffer, layout: ManagedMetadataLayout, - table: MemberTable, + table: MemberDefinitionTable, + facts: TypeMemberListFacts, ): OwnerLookup => { - let byTable = lookups.get(layout); + let byTable = lookups.get(facts); const cached = byTable?.get(table); if (cached instanceof ManagedReaderFailure) throw cached; if (cached !== undefined) return cached; if (byTable === undefined) { byTable = new Map(); - lookups.set(layout, byTable); + lookups.set(facts, byTable); } try { const positionOf = memberListRowLookup(bytes, layout, table); @@ -50,30 +52,26 @@ export const memberDeclaringTypeRowLookup = ( } return root; }; - const startForType = (row: number): number => { - const cursor = metadataRowCursor(bytes, layout, 2, row); - cursor.readUInt32(); - cursor.readIndex(layout.stringIndexSize); - cursor.readIndex(layout.stringIndexSize); - cursor.readIndex(layout.codedIndexSize("TypeDefOrRef")); - const fieldStart = cursor.readIndex(memberListIndexSize(layout, 4)); - return table === 4 - ? fieldStart - : cursor.readIndex(memberListIndexSize(layout, 6)); - }; - let start = types === 0 ? 0 : startForType(1); for (let row = 1; row <= types; row += 1) { - const end = row === types ? positions + 1 : startForType(row + 1); - const limit = Math.min(end, positions + 1); - for ( - let p = unowned(Math.min(Math.max(start, 1), positions + 1)); - p < limit; - p = unowned(p + 1) - ) { - owners[p] = row; - next[p] = p + 1; + const memberStart = + table === 4 + ? facts.entries[row - 1]?.field + : facts.entries[row - 1]?.method; + const start = memberStart?.start.value ?? null; + const end = memberStart?.end ?? null; + // A malformed endpoint makes this entire interval uncertain. In + // particular, zero must not become row one through owner-side clamping. + if (start !== null && end !== null) { + const limit = Math.min(end, positions + 1); + for ( + let p = unowned(Math.min(start, positions + 1)); + p < limit; + p = unowned(p + 1) + ) { + owners[p] = row; + next[p] = p + 1; + } } - start = end; } const definitions = layout.table(table)?.rowCount ?? 0; const lookup: OwnerLookup = (definition) => diff --git a/src/dotnet/ManagedMemberPointers.ts b/src/dotnet/ManagedMemberPointers.ts index 3517d0a111..876440c78b 100644 --- a/src/dotnet/ManagedMemberPointers.ts +++ b/src/dotnet/ManagedMemberPointers.ts @@ -1,5 +1,8 @@ import { managedDecodeBudget } from "./ManagedDecodeBudget.js"; -import type { ManagedMetadataLayout } from "./ManagedMetadataLayout.js"; +import type { + ManagedMetadataLayout, + MetadataTableLayout, +} from "./ManagedMetadataLayout.js"; import { managedFailure, ManagedReaderFailure, @@ -32,6 +35,17 @@ export const memberListIndexSize = ( ? 4 : 2; +/** Normalize a TypeDef list coordinate once for parsing and owner lookup. */ +export const validatedMemberListStart = ( + start: number, + table: MetadataTableLayout | undefined, +): number | null => { + const total = table?.rowCount ?? 0; + return Number.isSafeInteger(start) && start >= 1 && start <= total + 1 + ? start + : null; +}; + /** Resolve physical definition rows to list positions without changing token identity. */ export const memberListRowLookup = ( bytes: Buffer, diff --git a/src/dotnet/ManagedMemberRows.ts b/src/dotnet/ManagedMemberRows.ts index 0f2bcc6336..0eb03c5425 100644 --- a/src/dotnet/ManagedMemberRows.ts +++ b/src/dotnet/ManagedMemberRows.ts @@ -30,6 +30,7 @@ import { import { readManagedValue } from "./ManagedReaderFailure.js"; import { methodBody } from "./ManagedMethodBodyReader.js"; import { memberDeclaringTypeRowLookup } from "./ManagedMemberOwnership.js"; +import type { TypeMemberListFacts } from "./ManagedTypeMemberLists.js"; /** * Declaring-type tokens whose members are decoded in full; null selects every @@ -47,6 +48,7 @@ export const parseFields = ( bytes: Buffer, layout: ManagedMetadataLayout, ranges: readonly TypeRange[], + memberLists: TypeMemberListFacts, selected: SelectedTypeTokens = null, ): { readonly fields: readonly ManagedField[]; @@ -58,7 +60,7 @@ export const parseFields = ( const issues: ManagedParseIssue[] = []; const table = layout.table(4); const ownerRow = readManagedValue( - () => memberDeclaringTypeRowLookup(bytes, layout, 4), + () => memberDeclaringTypeRowLookup(bytes, layout, 4, memberLists), issues, ); for (let row = 1; row <= (table?.rowCount ?? 0); row += 1) { @@ -256,6 +258,7 @@ interface ParseMethodsInput { readonly layout: ManagedMetadataLayout; readonly pe: ManagedPeLayout; readonly ranges: readonly TypeRange[]; + readonly memberLists: TypeMemberListFacts; readonly selected?: SelectedTypeTokens; readonly methodOffset?: number; readonly methodLimit?: number; @@ -266,6 +269,7 @@ export const parseMethods = ({ layout, pe, ranges, + memberLists, selected = null, methodOffset = 0, methodLimit = Number.MAX_SAFE_INTEGER, @@ -285,7 +289,7 @@ export const parseMethods = ({ let unscannedMethods = 0; const table = layout.table(6); const ownerRow = readManagedValue( - () => memberDeclaringTypeRowLookup(bytes, layout, 6), + () => memberDeclaringTypeRowLookup(bytes, layout, 6, memberLists), issues, ); for (let row = 1; row <= (table?.rowCount ?? 0); row += 1) { diff --git a/src/dotnet/ManagedMetadataInventory.ts b/src/dotnet/ManagedMetadataInventory.ts index c31059d575..f2d5a90452 100644 --- a/src/dotnet/ManagedMetadataInventory.ts +++ b/src/dotnet/ManagedMetadataInventory.ts @@ -15,6 +15,7 @@ import { readResource, } from "./ManagedMetadataInventoryRows.js"; import { metadataToken } from "./ManagedMetadataHeaps.js"; +import type { TypeMemberListFacts } from "./ManagedTypeMemberLists.js"; import { readManagedValue, uniqueManagedParseIssues, @@ -149,13 +150,22 @@ const readAttributes = ( bytes: Buffer, layout: ManagedMetadataLayout, issues: ManagedParseIssue[], + memberLists: TypeMemberListFacts, ): { readonly attributes: readonly CustomAttribute[]; readonly targetFrameworks: readonly string[]; } => { const attributes = readRows(layout.table(12), (row) => readManagedValue( - () => readCustomAttribute(bytes, layout, row, heapExtent(layout), issues), + () => + readCustomAttribute( + bytes, + layout, + row, + heapExtent(layout), + issues, + memberLists, + ), issues, ), ); @@ -177,8 +187,9 @@ export const readManagedMetadataInventory = ( bytes: Buffer, layout: ManagedMetadataLayout, resourceDirectory: ManagedResourceDirectory | null, + memberLists: TypeMemberListFacts, ): ManagedMetadataInventory => { - const issues: ManagedParseIssue[] = []; + const issues: ManagedParseIssue[] = [...memberLists.issues]; validateIdentityTableCounts(layout, issues); const module = readManagedValue( @@ -201,6 +212,7 @@ export const readManagedMetadataInventory = ( bytes, layout, issues, + memberLists, ); return { module, diff --git a/src/dotnet/ManagedMetadataInventoryRows.ts b/src/dotnet/ManagedMetadataInventoryRows.ts index 30f9e8e5a3..3eb54cf50b 100644 --- a/src/dotnet/ManagedMetadataInventoryRows.ts +++ b/src/dotnet/ManagedMetadataInventoryRows.ts @@ -1,5 +1,6 @@ import { managedDecodeBudget } from "./ManagedDecodeBudget.js"; import { memberDeclaringTypeRowLookup } from "./ManagedMemberOwnership.js"; +import type { TypeMemberListFacts } from "./ManagedTypeMemberLists.js"; import type { ManagedArtifactInspection, @@ -248,8 +249,14 @@ const declaringTypeForMethod = ( layout: ManagedMetadataLayout, methodRow: number, heapExtent: number, + memberLists: TypeMemberListFacts, ): string | null => { - const owner = memberDeclaringTypeRowLookup(bytes, layout, 6)(methodRow); + const owner = memberDeclaringTypeRowLookup( + bytes, + layout, + 6, + memberLists, + )(methodRow); if (owner === 0) return null; return readTypeName({ bytes, layout, table: 2, row: owner, heapExtent }); }; @@ -259,11 +266,13 @@ const attributeTypeName = ( layout: ManagedMetadataLayout, rawType: number, heapExtent: number, + memberLists: TypeMemberListFacts, ): string | null => { const tag = rawType & 7; const row = Math.floor(rawType / 8); if (row === 0) return null; - if (tag === 2) return declaringTypeForMethod(bytes, layout, row, heapExtent); + if (tag === 2) + return declaringTypeForMethod(bytes, layout, row, heapExtent, memberLists); if (tag !== 3) return null; const memberRefs = layout.table(10); if (memberRefs === undefined || row > memberRefs.rowCount) return null; @@ -288,7 +297,13 @@ const attributeTypeName = ( heapExtent, }); if (parentTag === 3) - return declaringTypeForMethod(bytes, layout, parentRow, heapExtent); + return declaringTypeForMethod( + bytes, + layout, + parentRow, + heapExtent, + memberLists, + ); return null; }; @@ -349,6 +364,7 @@ export const readCustomAttribute = ( row: number, heapExtent: number, issues: ManagedParseIssue[], + memberLists: TypeMemberListFacts, ): CustomAttribute => { const cursor = metadataRowCursor(bytes, layout, 12, row); const parentRaw = cursor.readIndex( @@ -411,7 +427,8 @@ export const readCustomAttribute = ( if (constructor !== null) typeName = readManagedValue( - () => attributeTypeName(bytes, layout, typeRaw, heapExtent), + () => + attributeTypeName(bytes, layout, typeRaw, heapExtent, memberLists), issues, ) ?? null; return { diff --git a/src/dotnet/ManagedNativeBoundaryInspector.ts b/src/dotnet/ManagedNativeBoundaryInspector.ts index d2b84be049..ef0b62cf19 100644 --- a/src/dotnet/ManagedNativeBoundaryInspector.ts +++ b/src/dotnet/ManagedNativeBoundaryInspector.ts @@ -13,6 +13,7 @@ import { readManagedMetadataLayout, type ManagedMetadataLayout, } from "./ManagedMetadataLayout.js"; +import { readTypeMemberListFacts } from "./ManagedTypeMemberLists.js"; import { readManagedPeLayout, type ManagedPeLayout, @@ -108,10 +109,12 @@ const readBoundaryInventory = ( metadataOffset, cli.metadata.size, ); + const memberLists = readTypeMemberListFacts(bytes, layout); const inventory = readManagedMetadataInventory( bytes, layout, readManagedResourceDirectory(pe, issues), + memberLists, ); return { layout, inventory }; }; diff --git a/src/dotnet/ManagedTypeMemberLists.ts b/src/dotnet/ManagedTypeMemberLists.ts new file mode 100644 index 0000000000..1db5674c64 --- /dev/null +++ b/src/dotnet/ManagedTypeMemberLists.ts @@ -0,0 +1,147 @@ +import type { ManagedParseIssue } from "../domain/managed/managedArtifact.js"; +import { managedDecodeBudget } from "./ManagedDecodeBudget.js"; +import type { ManagedMetadataLayout } from "./ManagedMetadataLayout.js"; +import { metadataRowCursor, metadataToken } from "./ManagedMetadataHeaps.js"; +import { + memberListIndexSize, + memberListTable, + validatedMemberListStart, +} from "./ManagedMemberPointers.js"; + +export type MemberDefinitionTable = 4 | 6; + +export interface TypeMemberListStart { + readonly raw: number; + readonly value: number | null; + readonly offset: number; +} + +export interface TypeMemberListRange { + readonly start: TypeMemberListStart; + /** Next TypeDef's validated start, or the one-past-end sentinel. */ + readonly end: number | null; +} + +export interface TypeMemberListEntry { + readonly field: TypeMemberListRange; + readonly method: TypeMemberListRange; +} + +/** Parsed TypeDef list coordinates for one metadata-layout lifetime. */ +export interface TypeMemberListFacts { + readonly entries: readonly TypeMemberListEntry[]; + readonly issues: readonly ManagedParseIssue[]; +} + +export const readTypeMemberListFacts = ( + bytes: Buffer, + layout: ManagedMetadataLayout, +): TypeMemberListFacts => { + const fields = memberListTable(layout, 4); + const methods = memberListTable(layout, 6); + const rows: { field: TypeMemberListStart; method: TypeMemberListStart }[] = + []; + const issues: ManagedParseIssue[] = []; + const types = layout.table(2); + const typeCount = types?.rowCount ?? 0; + // Peak retained shape includes both the parsed-start and range arrays, their + // nested records, plus each eventual JSON object/key projection. Malformed + // row diagnostics are admitted separately before they are allocated. + managedDecodeBudget(layout).reserve( + typeCount * 512 + 256, + types?.offset ?? null, + ); + for (let row = 1; row <= typeCount; row += 1) { + const cursor = metadataRowCursor(bytes, layout, 2, row); + cursor.readUInt32(); + cursor.readIndex(layout.stringIndexSize); + cursor.readIndex(layout.stringIndexSize); + cursor.readIndex(layout.codedIndexSize("TypeDefOrRef")); + const field = { + offset: cursor.offset, + raw: cursor.readIndex(memberListIndexSize(layout, 4)), + }; + const method = { + offset: cursor.offset, + raw: cursor.readIndex(memberListIndexSize(layout, 6)), + }; + const entry = { + field: { ...field, value: validatedMemberListStart(field.raw, fields) }, + method: { + ...method, + value: validatedMemberListStart(method.raw, methods), + }, + }; + rows.push(entry); + for (const [name, start, table] of [ + ["FieldList", entry.field, fields], + ["MethodList", entry.method, methods], + ] as const) + if (start.value === null) { + managedDecodeBudget(layout).reserve(512, start.offset); + issues.push({ + code: "invalid-row", + scope: `metadata.TypeDef:${metadataToken(2, row)}`, + offset: start.offset, + detail: `TypeDef ${name} start ${String(start.raw)} is outside the one-based list range 1..${String((table?.rowCount ?? 0) + 1)}`, + }); + } + } + const nonmonotonicFields = new Set(); + const nonmonotonicMethods = new Set(); + for (let index = 0; index + 1 < rows.length; index += 1) { + const current = rows[index]; + const next = rows[index + 1]; + if (current === undefined || next === undefined) continue; + for (const [name, before, after, affected] of [ + ["FieldList", current.field, next.field, nonmonotonicFields], + ["MethodList", current.method, next.method, nonmonotonicMethods], + ] as const) + if ( + before.value !== null && + after.value !== null && + after.value < before.value + ) { + managedDecodeBudget(layout).reserve(512, after.offset); + affected.add(index); + affected.add(index + 1); + issues.push({ + code: "invalid-row", + scope: `metadata.TypeDef:${metadataToken(2, index + 2)}`, + offset: after.offset, + detail: `TypeDef ${name} start ${String(after.raw)} decreases from preceding start ${String(before.raw)}; adjacent member ranges are unknown`, + }); + } + } + const entries: TypeMemberListEntry[] = rows.map((entry, index) => { + const next = rows[index + 1]; + return { + field: { + start: { + ...entry.field, + value: nonmonotonicFields.has(index) ? null : entry.field.value, + }, + end: + next === undefined + ? (fields?.rowCount ?? 0) + 1 + : nonmonotonicFields.has(index) || nonmonotonicFields.has(index + 1) + ? null + : next.field.value, + }, + method: { + start: { + ...entry.method, + value: nonmonotonicMethods.has(index) ? null : entry.method.value, + }, + end: + next === undefined + ? (methods?.rowCount ?? 0) + 1 + : nonmonotonicMethods.has(index) || + nonmonotonicMethods.has(index + 1) + ? null + : next.method.value, + }, + }; + }); + return { entries, issues }; +}; diff --git a/tests/boundary/providers/managed/managedMemberComparisonFromMetadata.test.ts b/tests/boundary/providers/managed/managedMemberComparisonFromMetadata.test.ts index 5ffabdd59a..0b3a69f6fd 100644 --- a/tests/boundary/providers/managed/managedMemberComparisonFromMetadata.test.ts +++ b/tests/boundary/providers/managed/managedMemberComparisonFromMetadata.test.ts @@ -12,6 +12,75 @@ import { managedPeFixtureTarget, } from "../../../../src/dotnet/ManagedPe.fixture.js"; +describe("managed member structural matching", () => { + it("keeps unequal constants from becoming a structural remap", () => { + const observe = ( + typeName: string, + methodName: string, + constant: number, + branchOffset: number, + ) => + inspect( + buildManagedPeFixture({ + typeName, + methodName, + ilBody: Buffer.from([ + 0x1e, + 0x1f, + constant, + 0x26, + 0x2b, + branchOffset, + 0x00, + 0x2a, + ]), + }), + `/tmp/${typeName}-${constant}.dll`, + ); + const left = observe("OldType", "OldMethod", 5, 1); + const differentConstant = observe("NewType", "NewMethod", 7, 1); + const sameConstant = observe("NewType", "NewMethod", 5, 0); + for (const [observed, constant] of [ + [left, 5], + [differentConstant, 7], + [sameConstant, 5], + ] as const) { + expect(observed.result.methods[0]?.body).toMatchObject({ + status: "present", + normalized_il_sha256: expect.any(String), + anchors: expect.arrayContaining([ + expect.objectContaining({ + operand_kind: "constant", + operand: String(constant), + }), + ]), + }); + } + + const different = compareManagedMembers(left, differentConstant); + expect(different.methods.map(({ status }) => status)).toEqual([ + "removed", + "added", + ]); + expect( + different.methods.some( + ({ match }) => match.basis === "structural-method-shape", + ), + ).toBe(false); + + expect(compareManagedMembers(left, sameConstant).methods).toContainEqual( + expect.objectContaining({ + status: "changed", + match: expect.objectContaining({ + status: "matched", + basis: "structural-method-shape", + confidence: "high", + }), + }), + ); + }); +}); + describe("managed member comparison", () => { it("remaps renamed methods by exact CIL/signature without using names", () => { const leftBytes = buildManagedPeFixture(); diff --git a/tests/conformance/providers/managed/managedNativeVerification.test.ts b/tests/conformance/providers/managed/managedNativeVerification.test.ts index bca7dbfdce..6427853bfb 100644 --- a/tests/conformance/providers/managed/managedNativeVerification.test.ts +++ b/tests/conformance/providers/managed/managedNativeVerification.test.ts @@ -16,21 +16,27 @@ const exampleInput = () => MANAGED_NATIVE_VERIFICATION_EXAMPLE, ); -const nativeEvidenceWithExports = (names: readonly string[], path?: string) => { +const nativeEvidenceWithExports = ( + names: readonly string[], + path?: string, + subjectless = false, +) => { const input = exampleInput(); const native = input.native_observations[0]; if (native === undefined || native.subject === null) throw new Error("missing native example"); const macho = inspectMachoSchema.parse(native.normalized_result); return createEvidence( - { - path: path ?? native.subject.local_path, - sha256: native.subject.digest.sha256, - format: native.subject.format, - ...(native.subject.architecture === null - ? {} - : { architecture: native.subject.architecture }), - }, + subjectless + ? undefined + : { + path: path ?? native.subject.local_path, + sha256: native.subject.digest.sha256, + format: native.subject.format, + ...(native.subject.architecture === null + ? {} + : { architecture: native.subject.architecture }), + }, native.provider, { operation: native.operation, @@ -60,7 +66,7 @@ const nativeEvidenceWithExports = (names: readonly string[], path?: string) => { ); }; -const inputWithImportScope = (scope: string) => { +const inputWithImportScope = (scope: string | null) => { const input = exampleInput(); const managed = input.managed_boundaries; const normalized = managedNativeBoundaryInspectionSchema.parse( @@ -226,6 +232,41 @@ it("keeps literal module names distinct and accepts the shared-library lib alias }); }); +it("keeps name candidates unresolved when a required module identity is absent", () => { + for (const scope of [null, "", " "]) { + const result = verifyManagedNativeBoundaries({ + ...inputWithImportScope(scope), + native_observations: [nativeEvidenceWithExports(["open_native"])], + }); + expect(result.pinvoke_imports[0]).toMatchObject({ + status: "unresolved", + basis: "module-identity-unknown", + matched_native: { name: "open_native" }, + candidates: [{ name: "open_native" }], + }); + expect(result.pinvoke_imports[0]?.limitations).toContain( + "A native name candidate exists, but a required module identity is unavailable; module correspondence is unknown.", + ); + } + + const subjectless = verifyManagedNativeBoundaries({ + ...inputWithImportScope("nativehelper.dll"), + native_observations: [ + nativeEvidenceWithExports(["open_native"], undefined, true), + ], + }); + expect(subjectless.pinvoke_imports[0]).toMatchObject({ + status: "unresolved", + basis: "module-identity-unknown", + matched_native: { + name: "open_native", + module_name: null, + module_path: null, + }, + candidates: [{ name: "open_native" }], + }); +}); + it("treats supported native Evidence with no symbols as unresolved", () => { const result = verifyManagedNativeBoundaries({ ...exampleInput(), From 0930e94e077ab7fb4d695f62785919a8a25c2e21 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 14 Oct 2026 17:49:19 +0800 Subject: [PATCH 14/20] fix(setup): validate registered MCP output schema profiles Reuse the shared profile schema before setup writes and preserve supported values through registration and startup. Refs: #3205, #3277 Co-authored-by: PaoloValletta --- src/config/environment.ts | 2 ++ tests/acceptance/setup/piClientSetup.test.ts | 14 ++++++++++---- .../clientConfigurationEnvironment.test.ts | 11 +++++++++++ 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/config/environment.ts b/src/config/environment.ts index f5cc0373dc..69e0f0ed85 100644 --- a/src/config/environment.ts +++ b/src/config/environment.ts @@ -16,6 +16,7 @@ import { providerStartupTimeoutSchema, } from "./providerStartupTimeout.js"; import { mcpInputSchemaProfileSchema } from "./mcpInputSchemaProfile.js"; +import { mcpOutputSchemaProfileSchema } from "./mcpOutputSchemaProfile.js"; import { mcpResponseBudgetSchema } from "./mcpResponseBudget.js"; import { parseLoaderArgs, parseStringArray } from "./parsers.js"; @@ -23,6 +24,7 @@ const environmentSchema = z.object({ REA_ANALYSIS_PROVIDER: analysisProviderSelectorSchema.default("auto"), REA_MCP_MAX_RESPONSE_BYTES: mcpResponseBudgetSchema.optional(), REA_MCP_INPUT_SCHEMA_PROFILE: mcpInputSchemaProfileSchema.optional(), + REA_MCP_OUTPUT_SCHEMA_PROFILE: mcpOutputSchemaProfileSchema.optional(), REA_IDA_MCP_CONFIG: z .string() .min(1) diff --git a/tests/acceptance/setup/piClientSetup.test.ts b/tests/acceptance/setup/piClientSetup.test.ts index 736e6a5088..afc24ae7c7 100644 --- a/tests/acceptance/setup/piClientSetup.test.ts +++ b/tests/acceptance/setup/piClientSetup.test.ts @@ -11,9 +11,14 @@ import { COMPACT_INPUT_SCHEMA_BUDGET_BYTES } from "../../../src/config/mcpInputS import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; import { cliTest } from "../../support/cli/cliFixture.js"; -for (const profile of ["", "bogus"]) +for (const [setting, profile] of [ + ["REA_MCP_INPUT_SCHEMA_PROFILE", ""], + ["REA_MCP_INPUT_SCHEMA_PROFILE", "bogus"], + ["REA_MCP_OUTPUT_SCHEMA_PROFILE", ""], + ["REA_MCP_OUTPUT_SCHEMA_PROFILE", "bogus"], +] as const) cliTest( - `setup and doctor refuse an invalid retained schema profile ${JSON.stringify(profile)}`, + `setup and doctor refuse an invalid retained ${setting} value ${JSON.stringify(profile)}`, async ({ cli }) => { const home = await createTestTempDirectory("rea-pi-invalid-profile-cli-"); const agent = join(home, ".pi", "agent"); @@ -24,7 +29,7 @@ for (const profile of ["", "bogus"]) type: "stdio", command: "rea", args: ["mcp"], - env: { REA_MCP_INPUT_SCHEMA_PROFILE: profile }, + env: { [setting]: profile }, }, }, }); @@ -52,7 +57,7 @@ for (const profile of ["", "bogus"]) status: "needs_human", plannedActions: [], appliedActions: [], - remediation: expect.stringContaining("REA_MCP_INPUT_SCHEMA_PROFILE"), + remediation: expect.stringContaining(setting), }); const doctor = await cli.run({ arguments: ["doctor", "--client", "pi", "--json"], @@ -83,6 +88,7 @@ cliTest( const config = join(agent, "mcp.json"); const custom = { REA_MCP_INPUT_SCHEMA_PROFILE: "compact", + REA_MCP_OUTPUT_SCHEMA_PROFILE: "off", REA_LOG_LEVEL: "silent", }; const original = JSON.stringify({ diff --git a/tests/boundary/filesystem/clientConfigurationEnvironment.test.ts b/tests/boundary/filesystem/clientConfigurationEnvironment.test.ts index 74cbf612d8..5c5340b02d 100644 --- a/tests/boundary/filesystem/clientConfigurationEnvironment.test.ts +++ b/tests/boundary/filesystem/clientConfigurationEnvironment.test.ts @@ -29,6 +29,7 @@ it.each([ const client = { name, format, configPath }; const custom = { REA_MCP_INPUT_SCHEMA_PROFILE: "compact", + REA_MCP_OUTPUT_SCHEMA_PROFILE: "off", REA_MCP_MAX_RESPONSE_BYTES: "33554432", REA_BROWSER_EXECUTABLE: join(root, "custom-browser"), GHIDRA_INSTALL_DIR: join(root, "old-ghidra"), @@ -143,6 +144,16 @@ it.each([ { REA_MCP_INPUT_SCHEMA_PROFILE: "bogus" }, "REA_MCP_INPUT_SCHEMA_PROFILE", ], + [ + "empty output profile", + { REA_MCP_OUTPUT_SCHEMA_PROFILE: "" }, + "REA_MCP_OUTPUT_SCHEMA_PROFILE", + ], + [ + "invalid output profile", + { REA_MCP_OUTPUT_SCHEMA_PROFILE: "bogus" }, + "REA_MCP_OUTPUT_SCHEMA_PROFILE", + ], [ "invalid response budget", { REA_MCP_MAX_RESPONSE_BYTES: "1" }, From aade9bbcb79aa8504ab6921b25f8ab082aba78e7 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 14 Oct 2026 17:49:19 +0800 Subject: [PATCH 15/20] fix(setup): evaluate Gemini workspace trust before settings content Treat only ENOENT as absence. Preserve unreadable metadata errors and ignore untrusted workspace content before parsing it. Refs: #3206, #3319 Co-authored-by: haoran20100322 --- src/application/GeminiClientSettings.ts | 29 ++- .../setup/geminiWorkspaceTrust.test.ts | 220 ++++++++++++++++++ 2 files changed, 242 insertions(+), 7 deletions(-) create mode 100644 tests/acceptance/setup/geminiWorkspaceTrust.test.ts diff --git a/src/application/GeminiClientSettings.ts b/src/application/GeminiClientSettings.ts index 440ccf9d6e..ae503ccf57 100644 --- a/src/application/GeminiClientSettings.ts +++ b/src/application/GeminiClientSettings.ts @@ -1,4 +1,4 @@ -import { realpath } from "node:fs/promises"; +import { realpath, stat } from "node:fs/promises"; import { dirname, resolve } from "node:path"; import { stripComments } from "jsonc-parser"; import { z } from "zod"; @@ -149,12 +149,27 @@ export const readClientPolicyBlock = async ( ".gemini", "settings.json", ); - const document = await readConfigurationFile(path, "json", resources); - if (!document.ok) return document; - if (document.value !== undefined) { - const trusted = await workspaceTrusted(settings, sources, resources); - if (!trusted.ok) return trusted; - if (trusted.value) { + // Only ENOENT proves absence; preserve other metadata failures as unreadable. + const present = await stat(path).then( + () => ok(true), + (cause: unknown) => + cause instanceof Error && "code" in cause && cause.code === "ENOENT" + ? ok(false) + : err({ + kind: "unreadable" as const, + path, + detail: `Configuration ${path} could not be read: ${cause instanceof Error ? cause.message : String(cause)}`, + }), + ); + if (!present.ok) return present; + const includeWorkspace = present.value + ? await workspaceTrusted(settings, sources, resources) + : ok(false); + if (!includeWorkspace.ok) return includeWorkspace; + if (includeWorkspace.value) { + const document = await readConfigurationFile(path, "json", resources); + if (!document.ok) return document; + if (document.value !== undefined) { const source = { path, document: document.value }; const systemIndex = sources.findIndex( (item) => item.path === settings.systemPath, diff --git a/tests/acceptance/setup/geminiWorkspaceTrust.test.ts b/tests/acceptance/setup/geminiWorkspaceTrust.test.ts new file mode 100644 index 0000000000..a1d19c0125 --- /dev/null +++ b/tests/acceptance/setup/geminiWorkspaceTrust.test.ts @@ -0,0 +1,220 @@ +import { access, mkdir, readFile, writeFile } from "node:fs/promises"; +import { dirname, join } from "node:path"; +import { expect } from "vitest"; + +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { cliTest, type TestCli } from "../../support/cli/cliFixture.js"; + +const setupArguments = ["setup", "--client", "gemini_cli", "--yes", "--json"]; +const doctorArguments = ["doctor", "--client", "gemini_cli", "--json"]; + +const geminiWorkspace = async (cli: TestCli) => { + const home = await createTestTempDirectory("rea-gemini-workspace-trust-"); + const project = join(home, "project"); + const workspace = join(project, ".gemini/settings.json"); + const user = join(home, ".gemini/settings.json"); + await mkdir(dirname(workspace), { recursive: true }); + const environment: NodeJS.ProcessEnv = { + HOME: home, + USERPROFILE: home, + GEMINI_CLI_HOME: home, + GEMINI_CLI_SYSTEM_SETTINGS_PATH: join(home, "system.json"), + GEMINI_CLI_SYSTEM_DEFAULTS_PATH: join(home, "defaults.json"), + GEMINI_CLI_TRUSTED_FOLDERS_PATH: join(home, "trust.json"), + }; + const initial = await cli.run({ + arguments: setupArguments, + cwd: home, + environment, + }); + expect(initial.exitCode, initial.stdout).toBe(0); + const originalUser = await readFile(user); + const run = (arguments_: readonly string[]) => + cli.run({ arguments: arguments_, cwd: project, environment }); + return { workspace, user, originalUser, environment, run }; +}; + +cliTest( + "Gemini does not require workspace trust until workspace settings exist", + async ({ cli }) => { + const fixture = await geminiWorkspace(cli); + const trust = join(dirname(fixture.user), "trustedFolders.json"); + fixture.environment.GEMINI_CLI_TRUSTED_FOLDERS_PATH = trust; + for (const text of ["{}", "{ not json"]) { + await writeFile(trust, text); + const setup = await fixture.run(setupArguments); + expect(setup.exitCode, setup.stdout).toBe(0); + expect(setup.json).toMatchObject({ status: "ready", appliedActions: [] }); + const doctor = await fixture.run(doctorArguments); + expect(doctor.exitCode, doctor.stdout).toBe(0); + expect(doctor.json).toMatchObject({ + healthy: true, + identity: { + registrations: expect.arrayContaining([ + expect.objectContaining({ + client: "gemini_cli", + state: "aligned", + remediation: null, + }), + ]), + }, + }); + await expect(access(fixture.workspace)).rejects.toMatchObject({ + code: "ENOENT", + }); + expect(await readFile(trust, "utf8")).toBe(text); + expect(await readFile(fixture.user)).toEqual(fixture.originalUser); + } + + const workspaceText = "{ not json"; + await writeFile(fixture.workspace, workspaceText); + const setup = await fixture.run(setupArguments); + expect(setup.exitCode, setup.stdout).toBe(1); + expect(setup.json).toMatchObject({ + status: "needs_human", + plannedActions: [], + appliedActions: [], + remediation: expect.stringContaining(trust), + }); + const doctor = await fixture.run(doctorArguments); + expect(doctor.exitCode, doctor.stdout).toBe(1); + expect(doctor.json).toMatchObject({ + healthy: false, + identity: { + registrations: expect.arrayContaining([ + expect.objectContaining({ + client: "gemini_cli", + state: "invalid", + remediation: expect.stringContaining(trust), + }), + ]), + }, + }); + expect(await readFile(fixture.workspace, "utf8")).toBe(workspaceText); + expect(await readFile(trust, "utf8")).toBe("{ not json"); + expect(await readFile(fixture.user)).toEqual(fixture.originalUser); + }, +); + +for (const scenario of [ + { name: "an unlisted workspace", environment: {} }, + { + name: "an explicitly untrusted workspace", + environment: { GEMINI_CLI_TRUST_WORKSPACE: "false" }, + }, + { + name: "a restricted workspace with an explicit trust opt-in", + environment: { + GEMINI_CLI_TRUST_WORKSPACE: "true", + GEMINI_RESTRICTED_MODE: "true", + }, + }, +]) + cliTest( + `Gemini setup and doctor ignore malformed settings in ${scenario.name}`, + async ({ cli }) => { + const fixture = await geminiWorkspace(cli); + Object.assign(fixture.environment, scenario.environment); + const malformed = Buffer.from("{ not json"); + await writeFile(fixture.workspace, malformed); + + const setup = await fixture.run(setupArguments); + expect(setup.exitCode, setup.stdout).toBe(0); + expect(setup.json).toMatchObject({ status: "ready", appliedActions: [] }); + const doctor = await fixture.run(doctorArguments); + expect(doctor.exitCode, doctor.stdout).toBe(0); + expect(doctor.json).toMatchObject({ + healthy: true, + identity: { + registrations: expect.arrayContaining([ + expect.objectContaining({ + client: "gemini_cli", + state: "aligned", + remediation: null, + }), + ]), + }, + }); + expect(await readFile(fixture.workspace)).toEqual(malformed); + expect(await readFile(fixture.user)).toEqual(fixture.originalUser); + await expect( + access(`${fixture.workspace}.rea.backup`), + ).rejects.toMatchObject({ + code: "ENOENT", + }); + + if (scenario.name === "an explicitly untrusted workspace") { + const excluded = Buffer.from( + JSON.stringify({ mcp: { excluded: ["rea"] } }), + ); + await writeFile(fixture.workspace, excluded); + const excludedSetup = await fixture.run(setupArguments); + expect(excludedSetup.exitCode, excludedSetup.stdout).toBe(0); + const excludedDoctor = await fixture.run(doctorArguments); + expect(excludedDoctor.exitCode, excludedDoctor.stdout).toBe(0); + expect(excludedDoctor.json).toMatchObject({ + identity: { + registrations: expect.arrayContaining([ + expect.objectContaining({ + client: "gemini_cli", + state: "aligned", + }), + ]), + }, + }); + expect(await readFile(fixture.workspace)).toEqual(excluded); + } + }, + ); + +cliTest( + "Gemini still rejects malformed settings and exclusions in a trusted workspace", + async ({ cli }) => { + const fixture = await geminiWorkspace(cli); + fixture.environment.GEMINI_CLI_TRUST_WORKSPACE = "true"; + for (const scenario of [ + { + text: "{ not json", + state: "invalid", + diagnostic: "Invalid JSON/JSONC", + }, + { + text: JSON.stringify({ mcp: { excluded: ["rea"] } }), + state: "stale", + diagnostic: "mcp.excluded", + }, + ]) { + const original = Buffer.from(scenario.text); + await writeFile(fixture.workspace, original); + const setup = await fixture.run(setupArguments); + expect(setup.exitCode, setup.stdout).toBe(1); + expect(setup.json).toMatchObject({ + status: "needs_human", + plannedActions: [], + appliedActions: [], + remediation: expect.stringContaining(scenario.diagnostic), + }); + const doctor = await fixture.run(doctorArguments); + expect(doctor.exitCode, doctor.stdout).toBe(1); + expect(doctor.json).toMatchObject({ + healthy: false, + identity: { + registrations: expect.arrayContaining([ + expect.objectContaining({ + client: "gemini_cli", + state: scenario.state, + remediation: expect.stringContaining(scenario.diagnostic), + }), + ]), + }, + }); + expect(await readFile(fixture.workspace)).toEqual(original); + expect(await readFile(fixture.user)).toEqual(fixture.originalUser); + await expect( + access(`${fixture.workspace}.rea.backup`), + ).rejects.toMatchObject({ + code: "ENOENT", + }); + } + }, +); From ba33c706d26b63e43308be50c0249c72cff62cc6 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 14 Oct 2026 17:49:20 +0800 Subject: [PATCH 16/20] fix(setup): inspect installed skill identity for unqualified doctor Select installed skill destinations in the existing inspection pass, retaining malformed or foreign identity facts while ignoring empty marker-only roots. Refs: #3208, #3296 Co-authored-by: msaimshafique --- src/application/Doctor.ts | 7 +- src/application/SetupSkill.ts | 48 +++++-- .../setup/unqualifiedDoctorSkill.test.ts | 121 ++++++++++++++++++ 3 files changed, 161 insertions(+), 15 deletions(-) create mode 100644 tests/acceptance/setup/unqualifiedDoctorSkill.test.ts diff --git a/src/application/Doctor.ts b/src/application/Doctor.ts index c1e465a5e3..40b9cbadd3 100644 --- a/src/application/Doctor.ts +++ b/src/application/Doctor.ts @@ -108,8 +108,9 @@ export interface DoctorHost { /** Validated provider settings preserved by setup, independent of readiness. */ providerRegistrationEnvironment?(): Readonly>; installationPaths?(): Promise; + /** Undefined checks installed skill roots; explicit selections stay scoped. */ installedSkillIdentity?( - clientIds: readonly string[], + clientIds: readonly string[] | undefined, ): Promise; close?(): Promise; clientRegistrations?(): Promise; @@ -292,9 +293,7 @@ const collectDoctorIdentity = async ( const registrations = (await host.clientRegistrations?.()) ?? []; const skillClientIds = scope.skill_clients ?? - (scope.mode === "audit-wide" - ? registrations.map(({ client }) => client) - : scope.clients); + (scope.clients.length > 0 ? scope.clients : undefined); const installedSkillIdentity = await host.installedSkillIdentity?.(skillClientIds); const skillAligned = skillIdentityAligned(installedSkillIdentity); diff --git a/src/application/SetupSkill.ts b/src/application/SetupSkill.ts index 7dd28dee0d..eb7b4b138f 100644 --- a/src/application/SetupSkill.ts +++ b/src/application/SetupSkill.ts @@ -361,6 +361,7 @@ const canonicalSkillFiles = async ( interface CanonicalSkillInspection { readonly canonical: boolean; readonly manifest: string | undefined; + readonly installedDestination: boolean; } interface SkillInspectionFailure { @@ -372,7 +373,7 @@ interface SkillInspectionFailure { const inspectCanonicalSkill = async ( home: string, - clientIds: readonly string[], + clientIds: readonly string[] | undefined, environment: Readonly, platform: NodeJS.Platform, resources?: ArtifactResourceScope, @@ -383,7 +384,10 @@ const inspectCanonicalSkill = async ( environment, platform, ); - const issues = [...(await skillDirectoryIssues(destinations))]; + const issues = + clientIds === undefined + ? [] + : [...(await skillDirectoryIssues(destinations))]; if (issues.length > 0) return err({ _tag: "SkillInspectionFailure", @@ -394,7 +398,21 @@ const inspectCanonicalSkill = async ( let canonical = true; let manifest: string | undefined; let failureCause: unknown; - for (const { path: root } of destinations) { + let installedDestination = false; + for (const destination of destinations) { + const { path: root } = destination; + const pathIssues = + clientIds === undefined ? await skillDirectoryIssues([destination]) : []; + if (pathIssues.length > 0) { + issues.push(...pathIssues); + if (failureCause === undefined) failureCause = pathIssues; + installedDestination = true; + canonical = false; + continue; + } + let rootCanonical = true; + let rootHasFiles = false; + const rootIssues: SkillPathIssue[] = []; for (const relativePath of MANAGED_SKILL_FILES) { const destination = join(root, relativePath); let original: SkillFileState | undefined; @@ -406,27 +424,34 @@ const inspectCanonicalSkill = async ( cause instanceof ArtifactReaderFailure ) throw cause; - issues.push({ + rootIssues.push({ path: destination, kind: "unreadable", detail: causeReason(cause), }); - if (issues.length === 1) failureCause = cause; - canonical = false; + if (failureCause === undefined) failureCause = cause; + rootCanonical = false; + rootHasFiles = true; continue; } + if (original !== undefined) rootHasFiles = true; if (relativePath === "SKILL.md" && manifest === undefined) manifest = original?.bytes.toString("utf8"); // A mismatch settles freshness, but not coverage of later read failures. - if (original === undefined) canonical = false; - else if (canonical) { + if (original === undefined) rootCanonical = false; + else if (rootCanonical) { const content = await canonicalSkillContent(relativePath); - canonical = original.bytes.equals(Buffer.from(content, "utf8")); + rootCanonical = original.bytes.equals(Buffer.from(content, "utf8")); } } + if (clientIds !== undefined || rootHasFiles || rootIssues.length > 0) { + installedDestination = true; + canonical = canonical && rootCanonical; + issues.push(...rootIssues); + } } return issues.length === 0 - ? ok({ canonical, manifest }) + ? ok({ canonical, manifest, installedDestination }) : err({ _tag: "SkillInspectionFailure", issues, @@ -604,7 +629,7 @@ export const installCanonicalSkill = async ( /** Read installed skill metadata and compare every managed location. */ export const readInstalledSkillIdentity = async ( home: string, - clientIds: readonly string[], + clientIds: readonly string[] | undefined, environment: Readonly = {}, platform: NodeJS.Platform = process.platform, resources?: ArtifactResourceScope, @@ -616,6 +641,7 @@ export const readInstalledSkillIdentity = async ( platform, resources, ); + if (inspection.ok && !inspection.value.installedDestination) return undefined; const content = inspection.ok ? inspection.value.manifest : inspection.error.manifest; diff --git a/tests/acceptance/setup/unqualifiedDoctorSkill.test.ts b/tests/acceptance/setup/unqualifiedDoctorSkill.test.ts new file mode 100644 index 0000000000..014467dc3f --- /dev/null +++ b/tests/acceptance/setup/unqualifiedDoctorSkill.test.ts @@ -0,0 +1,121 @@ +import { mkdir, readFile, writeFile } from "node:fs/promises"; +import { dirname, join } from "node:path"; +import { expect } from "vitest"; + +import { PRODUCT_IDENTITY } from "../../../src/identity.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { cliTest } from "../../support/cli/cliFixture.js"; + +cliTest( + "unqualified doctor checks the installed managed skill and preserves its files", + async ({ cli }) => { + const home = await createTestTempDirectory("rea-unqualified-doctor-skill-"); + const skill = join( + home, + ".claude/skills/reverse-engineer-anything/SKILL.md", + ); + const run = (arguments_: readonly string[]) => + cli.run({ + arguments: [...arguments_, "--json"], + cwd: home, + environment: { HOME: home, USERPROFILE: home }, + }); + + const install = await run([ + "setup", + "--skill-client", + "claude_code", + "--yes", + ]); + expect(install.exitCode, install.stdout).toBe(0); + const original = await readFile(skill); + + const skillCheck = await run(["doctor", "--skill"]); + expect(skillCheck.exitCode, skillCheck.stdout).toBe(0); + expect(skillCheck.json).toMatchObject({ + healthy: true, + identity: { + skill: { + state: "aligned", + installed_version: PRODUCT_IDENTITY.skillVersion, + installed_tool_count: expect.any(Number), + }, + }, + }); + expect(await readFile(skill)).toEqual(original); + + const audit = await run(["doctor"]); + expect(audit.json).toMatchObject({ + identity: { + skill: { + state: "aligned", + installed_version: PRODUCT_IDENTITY.skillVersion, + installed_tool_count: expect.any(Number), + }, + }, + }); + expect(await readFile(skill)).toEqual(original); + + await mkdir(join(home, ".cursor")); + const withMarker = await run(["doctor"]); + expect(withMarker.json).toMatchObject({ + identity: { + skill: { + state: "aligned", + installed_version: PRODUCT_IDENTITY.skillVersion, + installed_tool_count: expect.any(Number), + }, + }, + }); + expect(await readFile(skill)).toEqual(original); + + await writeFile( + skill, + Buffer.concat([original, Buffer.from("\nlocal edit\n")]), + ); + const changed = await run(["doctor", "--skill"]); + expect(changed.exitCode).toBe(1); + expect(changed.json).toMatchObject({ + healthy: false, + identity: { skill: { state: "stale" } }, + }); + + const emptyHome = await createTestTempDirectory( + "rea-unqualified-doctor-empty-", + ); + const empty = await cli.run({ + arguments: ["doctor", "--skill", "--json"], + cwd: emptyHome, + environment: { HOME: emptyHome, USERPROFILE: emptyHome }, + }); + expect(empty.exitCode).toBe(1); + expect(empty.json).toMatchObject({ + healthy: false, + identity: { skill: { state: "missing" } }, + }); + + const foreignHome = await createTestTempDirectory( + "rea-unqualified-doctor-foreign-skill-", + ); + const foreignSkill = join( + foreignHome, + ".claude/skills/reverse-engineer-anything/SKILL.md", + ); + const foreignBytes = Buffer.from( + "---\nname: another-skill\n---\nA caller-owned workflow.\n", + ); + await mkdir(dirname(foreignSkill), { recursive: true }); + await writeFile(foreignSkill, foreignBytes); + const foreign = await cli.run({ + arguments: ["doctor", "--skill", "--json"], + cwd: foreignHome, + environment: { HOME: foreignHome, USERPROFILE: foreignHome }, + }); + expect(foreign.exitCode).toBe(1); + expect(foreign.json).toMatchObject({ + healthy: false, + identity: { skill: { state: "stale", installed_version: null } }, + }); + expect(await readFile(foreignSkill)).toEqual(foreignBytes); + }, +); From 44d98de2367b99808d78db732ec7d72533266233 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 14 Oct 2026 17:55:22 +0800 Subject: [PATCH 17/20] test: align malformed XML and route seed assertions with owning contracts --- .../apple/KeyedArchiveReader.test.ts | 4 ++- ...avascriptFeatureSeed.array_members.test.ts | 25 ++++++++++++++++++- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/artifacts/apple/KeyedArchiveReader.test.ts b/src/artifacts/apple/KeyedArchiveReader.test.ts index a0b0f1b736..71f62c8136 100644 --- a/src/artifacts/apple/KeyedArchiveReader.test.ts +++ b/src/artifacts/apple/KeyedArchiveReader.test.ts @@ -33,7 +33,9 @@ describe("inert keyed archive decoding", () => { decodeKeyedArchiveBytes(bytes, { offset: 0, limit: 1 }), ).toThrow( expect.objectContaining({ - code: "ERR_ENCODING_INVALID_ENCODED_DATA", + name: "XmlPlistTextDecodeError", + reason: "invalid-bytes", + cause: expect.any(TypeError), }), ); } diff --git a/src/domain/javascript/javascriptFeatureSeed.array_members.test.ts b/src/domain/javascript/javascriptFeatureSeed.array_members.test.ts index 50f6ceb095..f8a57c53ea 100644 --- a/src/domain/javascript/javascriptFeatureSeed.array_members.test.ts +++ b/src/domain/javascript/javascriptFeatureSeed.array_members.test.ts @@ -165,7 +165,6 @@ describe("feature seed selection controls", () => { { kind: "module", nodeKind: "javascript-module", field: "original_source" }, { kind: "native-export", nodeKind: "native-export", field: "label" }, { kind: "native-export", nodeKind: "native-export", field: "key" }, - { kind: "route", nodeKind: "endpoint", field: "label" }, { kind: "route", nodeKind: "endpoint", field: "value" }, { kind: "route", nodeKind: "endpoint", field: "path" }, { kind: "channel", nodeKind: "ipc-channel", field: "label" }, @@ -192,6 +191,30 @@ describe("feature seed selection controls", () => { }, ); + it("excludes route labels while preserving generic scalar and array matching", () => { + const scalar = memberNode("endpoint", { + endpoint_kind: "route", + label: "route endpoint", + }); + const array = memberNode("endpoint", { + endpoint_kind: "route", + label: ["route endpoint"], + }); + + expect( + findApplicationFeatureSeeds([scalar], seed("route", "route endpoint")), + ).toEqual([]); + expect( + findApplicationFeatureSeeds([array], seed("route", "route endpoint")), + ).toEqual([]); + expect( + findApplicationFeatureSeeds([scalar], seed("string", "route endpoint")), + ).toEqual([propertyMatch(scalar, "label")]); + expect( + findApplicationFeatureSeeds([array], seed("string", "route endpoint")), + ).toEqual([propertyMatch(array, "label[0]")]); + }); + it("keeps generic matching and scalar label and identity selection", () => { const node = memberNode("context-bridge-api", { members: ["openProject"] }); expect(findApplicationFeatureSeeds([node], seed("string"))).toEqual([ From 7435e79d6524e81fc17e49469d8d284e0b569707 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 14 Oct 2026 18:14:01 +0800 Subject: [PATCH 18/20] test(doctor): assert platform-specific Hopper readiness and exit status --- .../cli/doctorHopperDiscovery.test.ts | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/tests/boundary/cli/doctorHopperDiscovery.test.ts b/tests/boundary/cli/doctorHopperDiscovery.test.ts index d14715a958..a637f73914 100644 --- a/tests/boundary/cli/doctorHopperDiscovery.test.ts +++ b/tests/boundary/cli/doctorHopperDiscovery.test.ts @@ -6,8 +6,8 @@ import { expect } from "vitest"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; import { cliTest } from "../../support/cli/cliFixture.js"; -cliTest.skipIf(process.platform === "win32")( - "preserves a configured Hopper launcher through public CLI doctor without launching it", +cliTest.skipIf(!["darwin", "linux"].includes(process.platform))( + "retains a configured Hopper path in host diagnostics without launching it", async ({ cli }) => { const home = await createTestTempDirectory("rea-doctor-hopper-home-"); const launcher = `${home}/bin/../bin/hopper`; @@ -27,20 +27,31 @@ cliTest.skipIf(process.platform === "win32")( USERPROFILE: home, HOPPER_LAUNCHER_PATH: launcher, REA_HOPPER_TEST_MARKER: launchMarker, + GHIDRA_INSTALL_DIR: join(home, "missing-ghidra"), }, timeoutMs: 20_000, }); + const executableOnHost = process.platform === "darwin"; + expect(result.signal).toBeNull(); + expect(result.exitCode).toBe(executableOnHost ? 0 : 1); expect(result.json).toMatchObject({ - hopperPath: launcher, + healthy: executableOnHost, + environment_healthy: false, + ...(executableOnHost ? { hopperPath: launcher } : {}), checks: expect.arrayContaining([ expect.objectContaining({ name: "hopper", - ok: true, + ok: executableOnHost, detail: launcher, + classification: executableOnHost ? "healthy" : "config_drift", + ...(executableOnHost + ? {} + : { remediation: expect.stringContaining("HOPPER_LAUNCHER_PATH") }), }), ]), }); + if (!executableOnHost) expect(result.json).not.toHaveProperty("hopperPath"); await expect(access(launchMarker)).rejects.toMatchObject({ code: "ENOENT", }); From 60c08802a7b6d2195121114a9e4224492f138185 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 14 Oct 2026 18:14:01 +0800 Subject: [PATCH 19/20] test(native): validate demangling through owned contracts --- tests/boundary/cli/nativeDemangleCli.test.ts | 56 +++++++++++--------- tests/boundary/mcp/nativeDemangleMcp.test.ts | 12 ++--- 2 files changed, 35 insertions(+), 33 deletions(-) diff --git a/tests/boundary/cli/nativeDemangleCli.test.ts b/tests/boundary/cli/nativeDemangleCli.test.ts index fe905cbbf7..431cf02eff 100644 --- a/tests/boundary/cli/nativeDemangleCli.test.ts +++ b/tests/boundary/cli/nativeDemangleCli.test.ts @@ -18,10 +18,10 @@ const evidenceTarget = async () => { return path; }; -describe.skipIf(process.platform !== "darwin")( - "native Swift demangling CLI with line-terminator characters", - () => { - cliTest("keeps a carriage return in the symbol", async ({ cli }) => { +describe("Swift demangling CLI with line-terminator characters", () => { + cliTest.skipIf(process.platform !== "darwin")( + "keeps a carriage return in the symbol", + async ({ cli }) => { const result = await cli.run({ arguments: [ "demangle-swift", @@ -41,26 +41,32 @@ describe.skipIf(process.platform !== "darwin")( ], }, }); - }); + }, + ); - cliTest("rejects a symbol spanning lines as input", async ({ cli }) => { - const missingTarget = join( - await createTestTempDirectory("rea-demangle-invalid-input-"), - "absent", - ); - const result = await cli.run({ - arguments: [ - "demangle-swift", - missingTarget, - "$s4main3FooV", - "$s4main\n3FooV", - "--json", - ], - environment, - timeoutMs: 20_000, - }); - expect(result.exitCode).not.toBe(0); - expect(result.json).toMatchObject({ code: "VALIDATION_ERROR" }); + cliTest("rejects a symbol spanning lines as input", async ({ cli }) => { + const missingTarget = join( + await createTestTempDirectory("rea-demangle-invalid-input-"), + "absent", + ); + const result = await cli.run({ + arguments: [ + "demangle-swift", + missingTarget, + "$s4main3FooV", + "$s4main\n3FooV", + "--json", + ], + environment, + timeoutMs: 20_000, }); - }, -); + expect(result.signal).toBeNull(); + expect(result.exitCode).toBe(1); + expect(result.json).toMatchObject({ + code: "VALIDATION_ERROR", + fieldErrors: expect.arrayContaining([ + expect.objectContaining({ path: "symbols.1", missing: false }), + ]), + }); + }); +}); diff --git a/tests/boundary/mcp/nativeDemangleMcp.test.ts b/tests/boundary/mcp/nativeDemangleMcp.test.ts index 7bff5fa037..3130d09bba 100644 --- a/tests/boundary/mcp/nativeDemangleMcp.test.ts +++ b/tests/boundary/mcp/nativeDemangleMcp.test.ts @@ -2,19 +2,15 @@ import { copyFile } from "node:fs/promises"; import { join } from "node:path"; import { describe, expect } from "vitest"; -import { z } from "zod"; + +import { demangleSwiftSchema } from "../../../src/domain/native/nativeInspection.js"; +import { evidenceResultOf } from "../../../src/contracts/toolOutputSchemaPrimitives.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; import { mcpTest } from "../../support/mcp/mcpFixture.js"; import { openProductionTarget } from "../../support/mcp/productionSession.js"; -const demangledSchema = z.object({ - normalized_result: z.object({ - symbols: z.array( - z.object({ input: z.string(), output: z.string(), status: z.string() }), - ), - }), -}); +const demangledSchema = evidenceResultOf(demangleSwiftSchema); describe.skipIf(process.platform !== "darwin")( "native Swift demangling through MCP", From a32b4b6ac54a02a8fe9a7cb62861e54473ba5eb0 Mon Sep 17 00:00:00 2001 From: Ysopwnable <161784452+N0zoM1z0@users.noreply.github.com> Date: Wed, 14 Oct 2026 18:42:33 +0800 Subject: [PATCH 20/20] test(ci): correct Rizin readiness and Hopper launcher fixtures (#3299) Correct two independently reproduced CI fixture preconditions: close the real Rizin child's stdin before its readiness frame, and provide scoped Linux loader evidence for the Hopper doctor shell fixture. Preserve original timeouts, rejection checks, path spelling, launch prevention and cleanup. Current-head CI passed, including both complete reverse-tools mutation passes and 1,273 unique portable test files (14,521 passed, 126 skipped, zero failures). Each issue retains its own branch/commit; the validated integration tree includes the Hopper fix from #3410. That duplicate PR can close after the same code is verified on main. Fixes #3295 Fixes #3406 Co-authored-by: Kaoru0822 <208281322+kaoru0822-kitauji@users.noreply.github.com> --- .../cli/doctorHopperDiscovery.test.ts | 30 +++++++++++++++++-- .../RizinDebugSessionCoverage.part2.test.ts | 3 +- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/tests/boundary/cli/doctorHopperDiscovery.test.ts b/tests/boundary/cli/doctorHopperDiscovery.test.ts index a637f73914..0e7fdeacb9 100644 --- a/tests/boundary/cli/doctorHopperDiscovery.test.ts +++ b/tests/boundary/cli/doctorHopperDiscovery.test.ts @@ -1,5 +1,5 @@ -import { access, chmod, mkdir, writeFile } from "node:fs/promises"; -import { join } from "node:path"; +import { access, chmod, mkdir, readFile, writeFile } from "node:fs/promises"; +import { delimiter, join } from "node:path"; import { expect } from "vitest"; @@ -18,11 +18,35 @@ cliTest.skipIf(!["darwin", "linux"].includes(process.platform))( '#!/bin/sh\nprintf launched > "$REA_HOPPER_TEST_MARKER"\nexit 99\n', ); await chmod(launcher, 0o700); + const loaderMarker = join(home, "loader-checked-launcher"); + if (process.platform === "linux") { + // A shell fixture needs controlled loader diagnostics on Linux. This + // checks discovery and path preservation, not actual Hopper compatibility. + const loader = join(home, "bin", "ldd"); + await writeFile( + loader, + [ + "#!/bin/sh", + 'test "$#" -eq 1 || exit 87', + 'test "$1" = "$HOPPER_LAUNCHER_PATH" || exit 88', + 'printf "%s" "$1" > "$REA_HOPPER_LDD_TEST_MARKER"', + 'printf "%s\\n" "libc.so.6 => /fixture/libc.so.6 (0x1)"', + "", + ].join("\n"), + ); + await chmod(loader, 0o700); + } const result = await cli.run({ arguments: ["doctor", "--provider", "hopper", "--json"], cwd: home, environment: { + ...(process.platform === "linux" + ? { + PATH: [join(home, "bin"), process.env.PATH ?? ""].join(delimiter), + REA_HOPPER_LDD_TEST_MARKER: loaderMarker, + } + : {}), HOME: home, USERPROFILE: home, HOPPER_LAUNCHER_PATH: launcher, @@ -55,5 +79,7 @@ cliTest.skipIf(!["darwin", "linux"].includes(process.platform))( await expect(access(launchMarker)).rejects.toMatchObject({ code: "ENOENT", }); + if (process.platform === "linux") + expect(await readFile(loaderMarker, "utf8")).toBe(launcher); }, ); diff --git a/tests/boundary/providers/RizinDebugSessionCoverage.part2.test.ts b/tests/boundary/providers/RizinDebugSessionCoverage.part2.test.ts index 13b6c6dcf1..ba63901845 100644 --- a/tests/boundary/providers/RizinDebugSessionCoverage.part2.test.ts +++ b/tests/boundary/providers/RizinDebugSessionCoverage.part2.test.ts @@ -19,7 +19,8 @@ it.skipIf(process.platform === "win32")( "stops the session when the provider closes its stdin pipe", async () => { const provider = await makeProvider( - 'printf "\\000"; exec 0<&-; while :; do :; done', + // Readiness must guarantee that the child's stdin is already closed. + 'exec 0<&-; printf "\\000"; while :; do :; done', "#!/bin/sh", ); const { manager, sessionId } = await start(provider, {