From 238914974d17729bcdcc2e6cb012745d70d98a5e Mon Sep 17 00:00:00 2001 From: Wenyu Chiou <162016108+WenyuChiou@users.noreply.github.com> Date: Sat, 24 Oct 2026 18:35:12 -0400 Subject: [PATCH] fix(analysis-view): preserve current observation paths in summaries (#7039) --- .../analysisView/AnalysisViewService.test.ts | 102 +++++++++++++++++- .../analysisView/AnalysisViewService.ts | 63 ++++++++--- .../mcp/analyzeFunctionDetail.test.ts | 67 ++++++++++-- 3 files changed, 209 insertions(+), 23 deletions(-) diff --git a/src/application/analysisView/AnalysisViewService.test.ts b/src/application/analysisView/AnalysisViewService.test.ts index 9b8476673a..84307150b5 100644 --- a/src/application/analysisView/AnalysisViewService.test.ts +++ b/src/application/analysisView/AnalysisViewService.test.ts @@ -11,7 +11,11 @@ import { import { describe, expect, it } from "vitest"; import { JAVASCRIPT_APPLICATION_EVIDENCE_EXAMPLE } from "../../contracts/javascript/javascriptRuntimeReconciliationExample.js"; -import { createEvidence, parseEvidence } from "../../domain/evidence.js"; +import { + createEvidence, + parseEvidence, + rebindEvidenceToTargetPath, +} from "../../domain/evidence.js"; import { inspectAnalysisViewInputSchema } from "../../domain/analysisView/analysisView.js"; import { functionDossierSchema } from "../../domain/hopperValues.js"; import { ghidraFunctionDossier } from "../../domain/ghidraValues.fixture.js"; @@ -24,6 +28,7 @@ import { inspectAnalysisView, inspectAnalysisViewValidated, summarizeRetainedAnalysis, + projectRetainedAnalysis, } from "./AnalysisViewService.js"; import type { Evidence } from "../../domain/evidence.js"; import { EvidenceIntegrityError } from "../../domain/evidenceErrors.js"; @@ -39,6 +44,84 @@ import { } from "../../../tests/fixtures/browserScenarioCapture.js"; import { projectAnalysisError } from "../../domain/analysisErrorProjection.js"; +it.each([false, true])( + "summarizes the selected observation path rather than older retained display metadata (history=%s)", + (history) => { + const makeParent = (path: string) => + createEvidence( + { path, format: "pe", sha256: "c".repeat(64) }, + { id: "ghidra", name: "Ghidra", version: "12.1.4" }, + { + operation: "analyze_function", + parameters: {}, + result: functionDossierSchema.parse(ghidraFunctionDossier()), + }, + ); + const previous = makeParent("/old/fixture.exe"); + const selected = makeParent("/selected/fixture.exe"); + expect(selected.evidence_id).toBe(previous.evidence_id); + const ledger = new EvidenceLedger(); + if (history) expect(ledger.record(previous).ok).toBe(true); + const summary = summarizeRetainedAnalysis(selected, { + recordEvidence: (evidence) => ledger.record(evidence), + evidenceById: (id) => ledger.forAnalysis(id), + }); + if (!summary.ok) throw summary.error; + expect(summary.value.evidence_links).toEqual([selected.evidence_id]); + expect(summary.value.subject?.local_path).toBe( + selected.subject?.local_path, + ); + expect(ledger.get(selected.evidence_id)?.subject?.local_path).toBe( + history ? previous.subject?.local_path : selected.subject?.local_path, + ); + }, +); + +it("keeps retained admission and payload ownership for a large capture when only its display path changes", async () => { + const parent = analysisViewCaptureEvidence( + analysisViewObjectDenseCapture(40_000), + ); + const ledger = new EvidenceLedger(); + expect(ledger.record(parent).ok).toBe(true); + const retained = ledger.forAnalysis(parent.evidence_id)!; + const payload = retained.normalized_result; + const subject = retained.subject!; + const selected = rebindEvidenceToTargetPath(retained, { + path: "/selected/capture.har", + sha256: subject.digest.sha256, + format: subject.format, + ...(subject.architecture === null + ? {} + : { architecture: subject.architecture }), + }); + const retention = { + recordEvidence: (evidence: Evidence) => ledger.record(evidence), + evidenceById: (id: string) => ledger.forAnalysis(id), + }; + const view = inspectAnalysisViewInputSchema.parse({ + source: { kind: "retained-evidence", evidence_id: parent.evidence_id }, + view: { kind: "api-surface" }, + }).view; + for (const evidence of [retained, selected]) { + const result = projectRetainedAnalysis(evidence, retention, view); + if (!result.ok) throw result.error; + expect(result.value.subject?.local_path).toBe(evidence.subject?.local_path); + expect(result.value.evidence_links).toEqual([parent.evidence_id]); + } + expect(ledger.forAnalysis(parent.evidence_id)).toBe(retained); + expect(retained.normalized_result).toBe(payload); + expect(retained.subject?.local_path).toBe(parent.subject?.local_path); + expect( + await inspectAnalysisView({ + source: { kind: "inline", evidence: selected }, + view: { kind: "api-surface" }, + }), + ).toMatchObject({ + ok: false, + error: { _tag: "AnalysisResourceConstraintError", resource: "memory" }, + }); +}); + it.each(["layout", "javascript"])( "carries the retained %s source snapshot into repeatable shared views", async (kind) => { @@ -410,6 +493,23 @@ describe("summarizeRetainedAnalysis", () => { }), ).toEqual(err(conflict)); }); + + it("still rejects missing or mismatched retained identities after a successful writer reply", () => { + for (const [retained, reason] of [ + [undefined, "missing"], + [analysisViewLayoutEvidence(), "identity_mismatch"], + ] as const) { + expect( + summarizeRetainedAnalysis(parent, { + recordEvidence: () => ok("added"), + evidenceById: () => retained, + }), + ).toMatchObject({ + ok: false, + error: { _tag: "EvidenceIntegrityError", reason }, + }); + } + }); }); it("seals inferred and literal HAR pages, including a null artifact digest", async () => { diff --git a/src/application/analysisView/AnalysisViewService.ts b/src/application/analysisView/AnalysisViewService.ts index 55e614be31..0867b8051a 100644 --- a/src/application/analysisView/AnalysisViewService.ts +++ b/src/application/analysisView/AnalysisViewService.ts @@ -16,7 +16,12 @@ import { import { analysisInputErrorFromIssues } from "../../domain/inputIssueProjection.js"; import { EvidenceIntegrityError } from "../../domain/evidenceErrors.js"; import { safeParseEvidence } from "../../domain/evidenceValidation.js"; -import { createEvidence, type Evidence } from "../../domain/evidence.js"; +import { + createEvidence, + rebindEvidenceToTargetPath, + type Evidence, + type EvidenceSubjectTarget, +} from "../../domain/evidence.js"; import { inspectAnalysisViewInputSchema, prepareAnalysisViewProjection, @@ -179,7 +184,20 @@ export const inspectAnalysisViewValidated = ( } const resolved = resolveAnalysisViewSource(input.source, lookup); if (!resolved.ok) return resolved; - const parent = resolved.value; + return projectResolvedAnalysisView(input, projection, resolved.value); +}; + +/** Keep payload ownership separate from this delivery's display-only subject. */ +const projectResolvedAnalysisView = ( + input: ReadyAnalysisViewInput, + projection: ReturnType, + parent: Evidence, + selectedTarget?: EvidenceSubjectTarget, +): Result => { + const presentation = + selectedTarget === undefined + ? parent + : rebindEvidenceToTargetPath(parent, selectedTarget); const projected = projection.project({ evidenceId: parent.evidence_id, operation: parent.operation, @@ -190,15 +208,15 @@ export const inspectAnalysisViewValidated = ( parent.normalized_result, // Sealing an Evidence root deep-freezes it; only the root is registered. sealedResult: isImmutableJsonSnapshot(parent), - subject: parent.subject, + subject: presentation.subject, limitations: parent.limitations, parameters: parent.parameters, artifact: - parent.subject === null + presentation.subject === null ? null : { - path: parent.subject.local_path, - sha256: parent.subject.digest.sha256, + path: presentation.subject.local_path, + sha256: presentation.subject.digest.sha256, }, }); if (!projected.ok) return projected; @@ -211,7 +229,7 @@ export const inspectAnalysisViewValidated = ( `Parent subject SHA-256 ${parent.subject.digest.sha256} differs from analysis artifact SHA-256 ${projected.value.artifact?.sha256 ?? "unknown"}.`, ), ); - return ok(createAnalysisViewEvidence(input, parent, projected.value)); + return ok(createAnalysisViewEvidence(input, presentation, projected.value)); }; /** Session Evidence ownership required to deliver a view instead of a complete record. */ @@ -254,7 +272,7 @@ export const requireAnalysisViewRetention = ( /** * Retain complete analysis Evidence in the session, then project its summary - * view from the retained record. Callers page or select the rest through + * with this observation's subject path. Callers page or select the rest through * inspect_analysis_view with the view's parent Evidence ID. */ export const summarizeRetainedAnalysis = ( @@ -273,12 +291,29 @@ export const projectRetainedAnalysis = ( if (!ready.ok) return ready; const recorded = ready.value.recordEvidence(evidence); if (!recorded.ok) return recorded; - return inspectAnalysisViewValidated( - { - source: { kind: "retained-evidence", evidence_id: evidence.evidence_id }, - view, - }, - ready.value.evidenceById, + const source = { + kind: "retained-evidence" as const, + evidence_id: evidence.evidence_id, + }; + const retained = resolveEvidenceInput(source, ready.value.evidenceById); + if (!retained.ok) return retained; + // Duplicate semantic records retain their original ledger metadata. Detail + // selection changes presentation, not the historical lookup or payload owner. + const subject = evidence.subject; + return projectResolvedAnalysisView( + { source, view }, + prepareAnalysisViewProjection(view), + retained.value, + subject === null + ? undefined + : { + path: subject.local_path, + sha256: subject.digest.sha256, + format: subject.format, + ...(subject.architecture === null + ? {} + : { architecture: subject.architecture }), + }, ); }; diff --git a/tests/boundary/mcp/analyzeFunctionDetail.test.ts b/tests/boundary/mcp/analyzeFunctionDetail.test.ts index 4cb3632f44..af674bae64 100644 --- a/tests/boundary/mcp/analyzeFunctionDetail.test.ts +++ b/tests/boundary/mcp/analyzeFunctionDetail.test.ts @@ -13,6 +13,8 @@ import { ok } from "../../../src/domain/result.js"; import { functionDossierSchema } from "../../../src/domain/hopperValues.js"; import { ghidraFunctionDossier } from "../../../src/domain/ghidraValues.fixture.js"; import { parseGhidraFunctionInput } from "../../../src/ghidra/GhidraFunctionValues.js"; +import { EvidenceLedger } from "../../../src/application/investigation/EvidenceLedger.js"; +import { registerAnalysisViewTool } from "../../../src/server/registerAnalysisViewTool.js"; const dossier = functionDossierSchema.parse(ghidraFunctionDossier()); const partialCallees = { @@ -23,13 +25,14 @@ const partialCallees = { // A fixed provider that returns the fixture dossier for any analyze_function // call, after validating the request exactly as the production Ghidra client does. -const connect = async (retain = true, result = dossier) => { - const ledger = new Map(); - const record = (evidence: Evidence) => { - ledger.set(evidence.evidence_id, evidence); - return ok("added" as const); - }; - const lookup = (id: string) => ledger.get(id); +const connect = async ( + retain = true, + result = dossier, + path = () => "/fixtures/native.exe", +) => { + const ledger = new EvidenceLedger(); + const record = (evidence: Evidence) => ledger.record(evidence); + const lookup = (id: string) => ledger.forAnalysis(id); const execute = vi.fn( async (operation: string, parameters: Record) => { if (operation !== "analyze_function") @@ -43,7 +46,7 @@ const connect = async (retain = true, result = dossier) => { limitations: [], locations: [], subject: { - path: "/fixtures/native.exe", + path: path(), format: "pe" as const, sha256: "a".repeat(64), }, @@ -70,6 +73,12 @@ const connect = async (retain = true, result = dossier) => { allowsSnapshotReplay: undefined, recordWorkflowSnapshot: undefined, }); + registerAnalysisViewTool( + server, + pino({ enabled: false }), + retain ? lookup : undefined, + retain ? record : undefined, + ); const client = new Client({ name: "fn-detail-client", version: "1" }); onTestFinished(async () => { await client.close(); @@ -82,6 +91,48 @@ const connect = async (retain = true, result = dossier) => { return { client, ledger, execute }; }; +it("keeps complete and summary delivery on the current observed path despite retained same-ID history", async () => { + let selected = "/old/native.exe"; + const { client, ledger } = await connect(true, dossier, () => selected); + const call = async (detail: "complete" | "summary") => { + const reply = await client.callTool({ + name: "analyze_function", + arguments: { procedure: "main", detail }, + }); + expect(reply.isError).not.toBe(true); + return parseEvidence(reply.structuredContent); + }; + const old = await call("complete"); + selected = "/selected/native.exe"; + const complete = await call("complete"); + expect(complete.evidence_id).toBe(old.evidence_id); + expect(complete.subject?.local_path).toBe(selected); + const summary = await call("summary"); + expect(summary.evidence_links).toEqual([complete.evidence_id]); + expect(summary.subject?.local_path).toBe(complete.subject?.local_path); + expect(summary.normalized_result).toMatchObject({ + artifact: { path: selected }, + }); + expect(summary.locations).toContainEqual({ + kind: "artifact-path", + path: selected, + }); + expect(ledger.get(complete.evidence_id)?.subject?.local_path).toBe( + "/old/native.exe", + ); + const historicalReply = await client.callTool({ + name: "inspect_analysis_view", + arguments: { + source: { kind: "retained-evidence", evidence_id: complete.evidence_id }, + view: { kind: "summary" }, + }, + }); + expect(historicalReply.isError).not.toBe(true); + expect( + parseEvidence(historicalReply.structuredContent).subject?.local_path, + ).toBe("/old/native.exe"); +}); + it("defaults to a retained summary and strips delivery detail before the provider", async () => { const { client, ledger, execute } = await connect(); const progress: number[] = [];