From 7b327544f3cf898bf6ee81371b9d65f0b117cbbe Mon Sep 17 00:00:00 2001 From: N0zoM1z0 <161784452+N0zoM1z0@users.noreply.github.com> Date: Wed, 14 Oct 2026 00:26:42 +0800 Subject: [PATCH] fix(workflows): retain upstream symbol coverage caveats Keep limitations from exactly the provider observations used by a composed CLI/MCP call. Reuse the authenticated Ghidra PDB handshake owner, preserve caller options and failure identity, and deduplicate caveats without extra engine queries. Verified on actual Windows with Ghidra 12.1.4 PUBLIC, JDK21.0.12.1 and Node24.18.0, using the host where.exe and its matching Microsoft public PDB. Public CLI and MCP composed Evidence retain the referenced GUID/age and unknown PDB load state. Engine processes used two CPU affinity slots and 512MiB heaps; the CLI test needed a900s startup budget. The initial600s startup timeout is retained as performance evidence. No paired speedup or complete-symbol-coverage claim is made. Refs #2259 --- src/application/DirectAnalysis.ts | 10 +-- src/application/WorkflowEvidence.test.ts | 71 ++++++++++++++++++- src/application/WorkflowEvidence.ts | 56 ++++++++++++++- src/server/registerEnhancedTools.ts | 17 +++-- .../enhancedTools.callPathsAndBatches.test.ts | 11 ++- .../mcp/enhancedTools.limitations.test.ts | 42 +++++++++++ .../directAnalysisLimitations.test.ts | 54 ++++++++++++++ 7 files changed, 245 insertions(+), 16 deletions(-) create mode 100644 tests/boundary/mcp/enhancedTools.limitations.test.ts create mode 100644 tests/composition/directAnalysisLimitations.test.ts diff --git a/src/application/DirectAnalysis.ts b/src/application/DirectAnalysis.ts index 8a7874598..5e3799b0d 100644 --- a/src/application/DirectAnalysis.ts +++ b/src/application/DirectAnalysis.ts @@ -42,6 +42,7 @@ import { workflowAnalysisProfile, } from "./InvestigationProviders.js"; import { + executeComposedWorkflow, createWorkflowEvidence, workflowSnapshotRecord, recordWorkflowUnknowns, @@ -401,18 +402,17 @@ const executeAnalysisTool = async (input: { tool === "trace_native_ui_action" ) { const { EnhancedTools } = await import("./EnhancedTools.js"); - const result = await new EnhancedTools(session).execute( - tool, - input.arguments, - signal, + const result = await executeComposedWorkflow(session, (observed) => + new EnhancedTools(observed).execute(tool, input.arguments, signal), ); if (!result.ok) return { output: cliError(result.error) }; const evidence = createWorkflowEvidence({ target: input.openedTarget, operation: tool, parameters: input.arguments, - result: result.value, + result: result.value.result, upstreamProfile: evidenceProfile, + upstreamLimitations: result.value.limitations, }); return { output: evidence, evidence }; } diff --git a/src/application/WorkflowEvidence.test.ts b/src/application/WorkflowEvidence.test.ts index dab61a79c..39fcfdcce 100644 --- a/src/application/WorkflowEvidence.test.ts +++ b/src/application/WorkflowEvidence.test.ts @@ -3,15 +3,80 @@ import { describe, expect, it } from "vitest"; import { recordWorkflowUnknowns, createWorkflowEvidence, + executeComposedWorkflow, } from "./WorkflowEvidence.js"; import { InvestigationRecords } from "./investigation/InvestigationRecords.js"; import { UnknownRegistryError } from "../domain/unknownRegistryError.js"; -import { err } from "../domain/result.js"; +import { err, ok } from "../domain/result.js"; +import { + createAnalysisExecution, + type AnalysisOperationPort, +} from "./AnalysisProvider.js"; +import { AnalysisCancelledError } from "../domain/analysisErrorCore.js"; import { nativeApiBoundarySchema } from "../domain/native/nativeApiBoundary.js"; import { projectNativeApiInspection } from "./native/NativeApiInspection.js"; import { functionDossierSchema } from "../domain/hopperValues.js"; import { ghidraFunctionDossier } from "../domain/ghidraValues.fixture.js"; +const provider = { id: "fixture", name: "Fixture", version: "1" }; + +it("retains unique upstream limits in invocation order across concurrent observations", async () => { + let release: () => void = () => { + throw new Error("gate not initialized"); + }; + const gate = new Promise((resolve) => { + release = resolve; + }); + let calls = 0; + const options = { signal: new AbortController().signal }; + const analysis: AnalysisOperationPort = { + execute: async (_, parameters, forwarded) => { + expect(parameters).toEqual({ marker: true }); + expect(forwarded).toBe(options); + const call = calls++; + if (call === 0) await gate; + else release(); + return ok( + createAnalysisExecution([], provider, { + limitations: call === 0 ? ["first", "shared"] : ["second", "shared"], + }), + ); + }, + }; + const result = await executeComposedWorkflow(analysis, async (observed) => { + await Promise.all([ + observed.execute("list_strings", { marker: true }, options), + observed.execute("list_procedures", { marker: true }, options), + ]); + return ok({ count: 0 }); + }); + expect(calls).toBe(2); + expect(result).toEqual( + ok({ result: { count: 0 }, limitations: ["first", "shared", "second"] }), + ); +}); + +it("preserves a failed composed operation without an extra provider read", async () => { + const failure = err(new AnalysisCancelledError("list_strings")); + let calls = 0; + const result = await executeComposedWorkflow( + { + execute: async () => { + calls++; + return failure; + }, + }, + async (observed) => { + const observedResult = await observed.execute("list_strings", {}); + return observedResult.ok + ? ok(observedResult.value.result) + : observedResult; + }, + ); + expect(result).toBe(failure); + expect(calls).toBe(1); +}); + describe("workflow residual question retention", () => { it("keeps questions linked to workflow Evidence and records repeats idempotently", () => { const records = new InvestigationRecords(); @@ -24,6 +89,7 @@ describe("workflow residual question retention", () => { parameters: { procedure: "0x1000" }, result, upstreamProfile: undefined, + upstreamLimitations: [], }); expect(records.recordEvidence(evidence).ok).toBe(true); const input = { @@ -61,6 +127,7 @@ describe("workflow residual question retention", () => { parameters: {}, result, upstreamProfile: undefined, + upstreamLimitations: [], }); expect(records.recordEvidence(evidence).ok).toBe(true); @@ -88,6 +155,7 @@ describe("workflow residual question retention", () => { parameters: {}, result, upstreamProfile: undefined, + upstreamLimitations: [], }); expect(records.recordEvidence(evidence).ok).toBe(true); @@ -139,6 +207,7 @@ it.each([ parameters: { procedure: "0x1000" }, result, upstreamProfile: undefined, + upstreamLimitations: [], }); const records = new InvestigationRecords(); expect(records.recordEvidence(evidence).ok).toBe(true); diff --git a/src/application/WorkflowEvidence.ts b/src/application/WorkflowEvidence.ts index bf2530080..d79ff8405 100644 --- a/src/application/WorkflowEvidence.ts +++ b/src/application/WorkflowEvidence.ts @@ -2,7 +2,12 @@ import { REA_WORKFLOW_PROVIDER, workflowAnalysisProfile, } from "./InvestigationProviders.js"; -import type { AnalysisOperation } from "./AnalysisProvider.js"; +import type { + AnalysisOperation, + AnalysisOperationPort, +} from "./AnalysisProvider.js"; +import type { AnalysisError } from "../domain/analysisErrorBase.js"; +import { ok, type Result } from "../domain/result.js"; import type { AnalysisProfileCommitment } from "../domain/analysisProfile.js"; import type { WorkflowSnapshotRecordInput } from "./binary/BinarySessionRecords.js"; import { @@ -19,6 +24,49 @@ import type { JsonValue } from "../domain/jsonValue.js"; import type { UnknownRegistryPort } from "./investigation/InvestigationRecordPort.js"; import { UnknownRegistryError } from "../domain/unknownRegistryError.js"; +/** Retain the limits of exactly the observations used by one composed call. */ +export const executeComposedWorkflow = async ( + analysis: AnalysisOperationPort, + execute: ( + observed: AnalysisOperationPort, + ) => Promise>, +): Promise< + Result< + { readonly result: JsonValue; readonly limitations: readonly string[] }, + AnalysisError + > +> => { + let nextObservation = 0; + const limitations = new Map< + string, + { observation: number; position: number } + >(); + const result = await execute({ + execute: async (operation, parameters, options) => { + const observation = nextObservation++; + const execution = await analysis.execute(operation, parameters, options); + if (execution.ok) + execution.value.limitations.forEach((limitation, position) => { + const previous = limitations.get(limitation); + if (previous === undefined || observation < previous.observation) + limitations.set(limitation, { observation, position }); + }); + return execution; + }, + }); + if (!result.ok) return result; + return ok({ + result: result.value, + limitations: [...limitations] + .sort( + ([, left], [, right]) => + left.observation - right.observation || + left.position - right.position, + ) + .map(([limitation]) => limitation), + }); +}; + /** Build the shared Evidence record for a successful composed binary workflow. */ export const createWorkflowEvidence = (input: { readonly target: BinaryTarget | undefined; @@ -26,6 +74,7 @@ export const createWorkflowEvidence = (input: { readonly parameters: Readonly>; readonly result: JsonValue; readonly upstreamProfile: AnalysisProfileCommitment | undefined; + readonly upstreamLimitations: readonly string[]; }): Evidence => createEvidence(input.target, REA_WORKFLOW_PROVIDER, { operation: input.operation, @@ -40,7 +89,10 @@ export const createWorkflowEvidence = (input: { ), }), confidence: "derived", - limitations: ["Derived by an REA composed workflow."], + limitations: [ + "Derived by an REA composed workflow.", + ...input.upstreamLimitations, + ], ...(input.operation === "batch_decompile" ? { locations: batchProcedureLocations(input.result) } : {}), diff --git a/src/server/registerEnhancedTools.ts b/src/server/registerEnhancedTools.ts index e1e27c127..32cc4b6a1 100644 --- a/src/server/registerEnhancedTools.ts +++ b/src/server/registerEnhancedTools.ts @@ -14,6 +14,7 @@ import type { BinarySessionPort } from "../application/binary/BinarySessionPort. import { EnhancedTools } from "../application/EnhancedTools.js"; import type { ValidatedEnhancedCall } from "../application/EnhancedToolTypes.js"; import { + executeComposedWorkflow, createWorkflowEvidence, workflowSnapshotRecord, recordWorkflowUnknowns, @@ -231,15 +232,20 @@ const executeEnhancedTool = async ( context, parameters, }); - const services = new EnhancedTools({ + const analysis: AnalysisOperationPort = { execute: (operation, operationParameters, executionOptions) => admittedAnalysis.execute(operation, operationParameters, { ...executionOptions, progress, }), - }); + }; const result = await logToolExecution(registration.logger, name, () => - services.executeValidated(validatedCall, context.mcpReq.signal), + executeComposedWorkflow(analysis, (observed) => + new EnhancedTools(observed).executeValidated( + validatedCall, + context.mcpReq.signal, + ), + ), ); await progress.report({ phase: name, @@ -253,8 +259,9 @@ const executeEnhancedTool = async ( target: registration.activeTarget?.(), operation: name, parameters, - result: result.value, + result: result.value.result, upstreamProfile: registration.analysisProfile?.(), + upstreamLimitations: result.value.limitations, }); const recorded = registration.recordEvidence?.(evidence); if (recorded !== undefined && !recorded.ok) @@ -273,7 +280,7 @@ const executeEnhancedTool = async ( } const unknowns = recordWorkflowUnknowns({ name, - result: result.value, + result: result.value.result, evidenceId: evidence.evidence_id, recordUnknown: registration.recordUnknown, }); diff --git a/tests/boundary/mcp/enhancedTools.callPathsAndBatches.test.ts b/tests/boundary/mcp/enhancedTools.callPathsAndBatches.test.ts index f4915afd1..6f2837b4d 100644 --- a/tests/boundary/mcp/enhancedTools.callPathsAndBatches.test.ts +++ b/tests/boundary/mcp/enhancedTools.callPathsAndBatches.test.ts @@ -9,6 +9,7 @@ import { AnalysisOutputError, } from "../../../src/domain/analysisErrorCore.js"; import type { RelationshipCoverage } from "../../../src/domain/relationshipCoverage.js"; +import { parseEvidence } from "../../../src/domain/evidence.js"; import { err, ok as resultOk } from "../../../src/domain/result.js"; import { closeEnhancedToolResources, connect } from "./enhancedToolsHarness.js"; @@ -125,9 +126,13 @@ describe("enhanced MCP tools", () => { limitations: [caveat], }, }); - expect(JSON.stringify(graph.structuredContent)?.split(caveat)).toHaveLength( - 2, - ); + const evidence = parseEvidence(graph.structuredContent); + expect( + JSON.stringify(evidence.normalized_result).split(caveat), + ).toHaveLength(2); + expect( + evidence.limitations.filter((limit) => limit === caveat), + ).toHaveLength(1); const trace = await client.callTool({ name: "trace_call_path", diff --git a/tests/boundary/mcp/enhancedTools.limitations.test.ts b/tests/boundary/mcp/enhancedTools.limitations.test.ts new file mode 100644 index 000000000..bf05bb20c --- /dev/null +++ b/tests/boundary/mcp/enhancedTools.limitations.test.ts @@ -0,0 +1,42 @@ +import { afterEach, expect, it } from "vitest"; +import { createAnalysisExecution } from "../../../src/application/AnalysisProvider.js"; +import { ok } from "../../../src/domain/result.js"; +import { parseEvidence } from "../../../src/domain/evidence.js"; +import { connect, closeEnhancedToolResources } from "./enhancedToolsHarness.js"; + +afterEach(closeEnhancedToolResources); + +it("retains provider coverage caveats in a composed MCP overview without repeated reads", async () => { + const calls: string[] = []; + const limitation = "Companion PDB symbol coverage is unverified."; + const client = await connect({ + execute: async (operation) => { + calls.push(operation); + const value = operation === "current_document" ? "fixture" : []; + return ok( + createAnalysisExecution( + value, + { id: "fixture", name: "Fixture", version: "1" }, + { limitations: [limitation] }, + ), + ); + }, + }); + const response = await client.callTool({ + name: "binary_overview", + arguments: {}, + }); + expect(response.isError).not.toBe(true); + const evidence = parseEvidence(response.structuredContent); + expect(evidence.limitations).toEqual([ + "Artifact identity is unavailable for this observation.", + "Derived by an REA composed workflow.", + limitation, + ]); + expect(evidence.normalized_result).toMatchObject({ + document: "fixture", + procedure_count: 0, + string_count: 0, + }); + expect(calls.length).toBe(new Set(calls).size); +}); diff --git a/tests/composition/directAnalysisLimitations.test.ts b/tests/composition/directAnalysisLimitations.test.ts new file mode 100644 index 000000000..7c22ecea4 --- /dev/null +++ b/tests/composition/directAnalysisLimitations.test.ts @@ -0,0 +1,54 @@ +import { expect, it } from "vitest"; +import { parseConfig } from "../../src/config/parseConfig.js"; +import { runDirectAnalysis } from "../../src/application/DirectAnalysis.js"; +import { createAnalysisExecution } from "../../src/application/AnalysisProvider.js"; +import { ok } from "../../src/domain/result.js"; +import { + createBinarySessionTargets, + createTestBinarySession, +} from "../fixtures/binarySession.js"; + +it("retains provider coverage caveats in a composed CLI result and closes the session", async () => { + const [path] = await createBinarySessionTargets(); + const limitation = "Companion PDB symbol coverage is unverified."; + let reads = 0; + let closed = 0; + const createSession = () => + createTestBinarySession(() => ({ + execute: async (operation) => { + reads++; + return ok( + createAnalysisExecution( + operation === "current_document" ? "fixture" : [], + { id: "fixture", name: "Fixture", version: "1" }, + { limitations: [limitation] }, + ), + ); + }, + close: async () => { + closed++; + return ok(null); + }, + })); + const output = await runDirectAnalysis( + { + readConfiguration: () => parseConfig({}), + createBinarySession: createSession, + createManagedBinarySession: createSession, + }, + path, + "binary_overview", + {}, + ); + expect(output).toMatchObject({ + limitations: ["Derived by an REA composed workflow.", limitation], + normalized_result: { + document: "fixture", + segment_count: 0, + procedure_count: 0, + string_count: 0, + }, + }); + expect(reads).toBe(5); + expect(closed).toBe(1); +});