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); +});