From 9d2a57fd8d9c02346f98f4ddbd62c494b61eabbc Mon Sep 17 00:00:00 2001 From: gensokyo-dev Date: Sat, 14 Nov 2026 09:12:29 +0000 Subject: [PATCH] fix(decompile): keep each successful batch item's own limitations A successful batch_decompile item carried no item-level provenance: only the batch Evidence aggregated limitations, so when a provider such as Kuna answered repeated selectors from different sources, the caveat did not say which occurrence it applied to. Successful items now keep their own observation's limitations (exactOptional, so retained records still parse), and the retained items view projects them. The batch unavailable item also dropped the provider's typed pseudocodeAbsence reason; it now carries pseudocode_absence. Fixes #8350 Co-Authored-By: gensokyo-dev <342580529+gensokyo-dev@users.noreply.github.com> --- docs/guides/native/native-investigation.md | 6 ++- docs/reference/mcp-contracts.md | 5 +- src/application/EnhancedTools.ts | 25 +++++++--- src/domain/analysisView/batchDecompileView.ts | 11 ++++- src/domain/native/batchDecompile.ts | 7 ++- .../enhancedTools.callPathsAndBatches.test.ts | 14 ++++-- tests/boundary/mcp/ghidraRoutingMcp.test.ts | 17 ++++++- .../kuna/kunaBatchDecompileMcp.test.ts | 47 ++++++++++++++++++- 8 files changed, 113 insertions(+), 19 deletions(-) diff --git a/docs/guides/native/native-investigation.md b/docs/guides/native/native-investigation.md index 73436f6851..23f0acb5ef 100644 --- a/docs/guides/native/native-investigation.md +++ b/docs/guides/native/native-investigation.md @@ -247,8 +247,10 @@ observation's own `limitations`, so a reason the provider reported for that entry, such as a Kuna export failure or a missing body, stays with the item. Null pseudocode is `capability_unavailable`; empty or non-text pseudocode is `unreadable_output`. Provider failures keep their typed error. Successful items -do not repeat limitations; the batch Evidence aggregates every observation's -limitations by provider and operation. +also keep their own observation's limitations, so repeated selectors that the +provider answered differently stay distinguishable, and an unavailable item +keeps the provider's `pseudocode_absence` reason. The batch Evidence still +aggregates every observation's limitations by provider and operation. Resolved entries also appear in the composed Evidence locations. New batch Evidence commits the revised workflow profile; retained historical Evidence remains valid. This tool diff --git a/docs/reference/mcp-contracts.md b/docs/reference/mcp-contracts.md index 9e0fb726df..892c51824a 100644 --- a/docs/reference/mcp-contracts.md +++ b/docs/reference/mcp-contracts.md @@ -866,8 +866,9 @@ and provider identities: Page `coverage` counts the selection only: exhausting it establishes nothing about functions outside the batch. Evidence `limitations` aggregate every item's -provider observations and are not attributed to one successful item; unavailable and error -items keep their own. A page over the response budget offers a smaller selection +provider observations; every item, successful or not, also keeps its own +observation's limitations, and an unavailable item keeps the provider's +`pseudocode_absence` reason when one was reported. A page over the response budget offers a smaller selection from the same `source`: fewer rows, or one body's `pseudocode` pages when a single row does not fit. An inline source offers none, because the record cannot be repeated in that reply; resend it with a smaller selection. diff --git a/src/application/EnhancedTools.ts b/src/application/EnhancedTools.ts index fe38bd8cfe..f6b4000708 100644 --- a/src/application/EnhancedTools.ts +++ b/src/application/EnhancedTools.ts @@ -506,21 +506,32 @@ export class EnhancedTools { }); continue; } - const { result: pseudocode } = observed.value; + const { result: pseudocode, pseudocodeAbsence } = observed.value; + // Each item keeps its own observation's caveats, so repeated selectors + // that the provider answered differently stay distinguishable; the batch + // Evidence still aggregates them. + const limitations = [...new Set(observed.value.limitations)]; + const itemLimitations = limitations.length === 0 ? {} : { limitations }; if (typeof pseudocode === "string" && pseudocode.length > 0) { - items.push({ address, procedure, status: "ok", pseudocode }); + items.push({ + address, + procedure, + status: "ok", + pseudocode, + ...itemLimitations, + }); continue; } - // Only this observation can explain its missing text, so its caveats - // stay with the item; the batch Evidence still aggregates them. - const limitations = [...new Set(observed.value.limitations)]; if (pseudocode === null) { items.push({ address, procedure, status: "unavailable", reason: "no_pseudocode", - ...(limitations.length === 0 ? {} : { limitations }), + ...(pseudocodeAbsence === undefined + ? {} + : { pseudocode_absence: pseudocodeAbsence }), + ...itemLimitations, }); continue; } @@ -529,7 +540,7 @@ export class EnhancedTools { procedure, status: "error", error: projectAnalysisError(missingPseudocodeError(pseudocode)), - ...(limitations.length === 0 ? {} : { limitations }), + ...itemLimitations, }); } return ok(batchDecompileResult(items)); diff --git a/src/domain/analysisView/batchDecompileView.ts b/src/domain/analysisView/batchDecompileView.ts index 29ee452596..bf2e62cc31 100644 --- a/src/domain/analysisView/batchDecompileView.ts +++ b/src/domain/analysisView/batchDecompileView.ts @@ -4,6 +4,7 @@ import { type CommittedProviderIdentity, } from "../analysisProfile.js"; import { digestSchema } from "../digests.js"; +import { pseudocodeAbsenceSchema } from "../native/pseudocodeAbsence.js"; import { providerSchema } from "../evidence.js"; import { batchDecompileResultSchema, @@ -114,11 +115,13 @@ const batchDecompileRowSchema = z.discriminatedUnion("status", [ status: z.literal("ok"), pseudocode_code_units: z.int().positive(), pseudocode: z.string().min(1).exactOptional(), + limitations: batchItemLimitationsSchema, }), z.strictObject({ ...itemIdentity, status: z.literal("unavailable"), reason: z.literal("no_pseudocode"), + pseudocode_absence: pseudocodeAbsenceSchema.exactOptional(), limitations: batchItemLimitationsSchema, }), z.strictObject({ @@ -253,7 +256,7 @@ const batchUnknowns = (items: readonly BatchDecompileItem[]): string[] => { : [ `${String(unknownIdentity)} items have unknown procedure identity; their address is the caller's selector, not an observed entry.`, ]), - "Evidence limitations aggregate every item's provider observations and are not attributed to one successful item. A batch covers only its selected procedures, and exhausting a page establishes nothing outside it.", + "Evidence limitations aggregate every item's provider observations; each item row also carries its own observation's limitations. A batch covers only its selected procedures, and exhausting a page establishes nothing outside it.", ]; }; @@ -276,12 +279,18 @@ const itemRow = ( status: "ok", pseudocode_code_units: item.pseudocode.length, ...(includePseudocode ? { pseudocode: item.pseudocode } : {}), + ...(item.limitations === undefined + ? {} + : { limitations: item.limitations }), }; if (item.status === "unavailable") return { ...identity, status: "unavailable", reason: item.reason, + ...(item.pseudocode_absence === undefined + ? {} + : { pseudocode_absence: item.pseudocode_absence }), ...(item.limitations === undefined ? {} : { limitations: item.limitations }), diff --git a/src/domain/native/batchDecompile.ts b/src/domain/native/batchDecompile.ts index 839f640ee2..fa85c9ba70 100644 --- a/src/domain/native/batchDecompile.ts +++ b/src/domain/native/batchDecompile.ts @@ -1,5 +1,6 @@ import { z } from "zod"; import { analysisErrorProjectionSchema } from "../../contracts/errorSchemas.js"; +import { pseudocodeAbsenceSchema } from "./pseudocodeAbsence.js"; /** Identity facts attached to one requested batch selector. */ export const batchProcedureIdentitySchema = z.discriminatedUnion("status", [ @@ -27,7 +28,7 @@ export const batchItemLimitationsSchema = z .array(z.string()) .exactOptional() .describe( - "Caveats of this item's own provider observation when it supplied no pseudocode, including any reason the provider reported.", + "Caveats of this item's own provider observation, including any reason the provider reported when it supplied no pseudocode.", ); /** One ordered batch outcome; array position keeps duplicate selectors distinct. */ @@ -37,12 +38,16 @@ export const batchDecompileItemSchema = z.discriminatedUnion("status", [ procedure: batchProcedureIdentitySchema, status: z.literal("ok"), pseudocode: z.string().min(1), + limitations: batchItemLimitationsSchema, }), z.object({ address: batchSelectorSchema, procedure: batchProcedureIdentitySchema, status: z.literal("unavailable"), reason: z.literal("no_pseudocode"), + pseudocode_absence: pseudocodeAbsenceSchema + .exactOptional() + .describe("The provider's reported reason for null pseudocode."), limitations: batchItemLimitationsSchema, }), z.object({ diff --git a/tests/boundary/mcp/enhancedTools.callPathsAndBatches.test.ts b/tests/boundary/mcp/enhancedTools.callPathsAndBatches.test.ts index 55eafbd7cc..bba99d4650 100644 --- a/tests/boundary/mcp/enhancedTools.callPathsAndBatches.test.ts +++ b/tests/boundary/mcp/enhancedTools.callPathsAndBatches.test.ts @@ -678,7 +678,13 @@ describe("enhanced MCP batch answers without pseudocode", () => { const evidence = contract.outputSchema.parse(result.structuredContent); expect(evidence.normalized_result).toMatchObject({ items: [ - { address: "0x1", status: "ok", pseudocode: answers["0x1"]?.result }, + // Successful text keeps its own observation's caveats (#8350). + { + address: "0x1", + status: "ok", + pseudocode: answers["0x1"]?.result, + limitations: answers["0x1"]?.limitations, + }, { address: "0x2", status: "unavailable", @@ -725,9 +731,9 @@ describe("enhanced MCP batch answers without pseudocode", () => { const { items } = batchDecompileResultSchema.parse( evidence.normalized_result, ); - // Successful text, a caveat-free null and a typed provider failure carry - // no item caveats; the batch Evidence aggregates every observation's. - for (const index of [0, 2, 5]) + // A caveat-free null and a typed provider failure carry no item caveats; + // the batch Evidence aggregates every observation's. + for (const index of [2, 5]) expect(items[index]).not.toHaveProperty("limitations"); expect(evidence.limitations).toEqual( expect.arrayContaining([ diff --git a/tests/boundary/mcp/ghidraRoutingMcp.test.ts b/tests/boundary/mcp/ghidraRoutingMcp.test.ts index 7edc1b6801..a80bc7ee9f 100644 --- a/tests/boundary/mcp/ghidraRoutingMcp.test.ts +++ b/tests/boundary/mcp/ghidraRoutingMcp.test.ts @@ -118,7 +118,22 @@ describe("Ghidra MCP capability routing", () => { batch.structuredContent, ).normalized_result, ); - expect(result.items[0]).not.toHaveProperty("limitations"); + // The successful item keeps its own observation's caveats (#8350). + const answered = parseEvidence( + ( + await harness.mcp.callTool({ + name: "procedure_pseudo_code", + arguments: { procedure: "0x401000" }, + }) + ).structuredContent, + ); + const answeredLimitations = [...new Set(answered.limitations)]; + if (answeredLimitations.length === 0) + expect(result.items[0]).not.toHaveProperty("limitations"); + else + expect(result.items[0]).toMatchObject({ + limitations: answeredLimitations, + }); expect(result).toEqual({ items: [ expect.objectContaining({ address: "0x401000", status: "ok" }), diff --git a/tests/boundary/providers/kuna/kunaBatchDecompileMcp.test.ts b/tests/boundary/providers/kuna/kunaBatchDecompileMcp.test.ts index 577e5dddd3..0a7b3dd3a9 100644 --- a/tests/boundary/providers/kuna/kunaBatchDecompileMcp.test.ts +++ b/tests/boundary/providers/kuna/kunaBatchDecompileMcp.test.ts @@ -4,6 +4,7 @@ import { writeFile } from "node:fs/promises"; import { join } from "node:path"; import { expect } from "vitest"; +import { z } from "zod"; import { AnalysisProviderRegistry } from "../../../../src/application/binary/AnalysisProviderRegistry.js"; import { composeBinarySession } from "../../../../src/application/binary/BinarySessionComposition.js"; @@ -38,6 +39,40 @@ const getEvidenceBundle = SESSION_TOOL_CONTRACTS.find( > => contract.name === "get_evidence_bundle", ); +/** Read the retained batch's items view and check the per-item provider facts. */ +const expectRetainedItems = async ( + call: ( + name: string, + arguments_: Record, + ) => Promise<{ structuredContent?: unknown }>, + evidenceId: string, + broken: { readonly limitations: readonly string[] }, + puts: { readonly limitations: readonly string[] }, +) => { + const items = z + .looseObject({ items: z.array(z.looseObject({ address: z.string() })) }) + .parse( + parseEvidence( + ( + await call("inspect_analysis_view", { + source: { kind: "retained-evidence", evidence_id: evidenceId }, + view: { kind: "batch-decompile", collection: "items" }, + }) + ).structuredContent, + ).normalized_result, + ).items; + const row = (address: string) => + items.find((item) => item.address === address); + expect(row("broken")).toMatchObject({ + pseudocode_absence: { kind: "provider-failure" }, + limitations: broken.limitations, + }); + expect(row("puts")).toMatchObject({ + pseudocode_absence: { kind: "not-in-export" }, + limitations: puts.limitations, + }); +}; + test("keeps each Kuna no-pseudocode observation's limitations on its batch item", async ({ mcp, }) => { @@ -75,6 +110,7 @@ test("keeps each Kuna no-pseudocode observation's limitations on its batch item" parseEvidence( (await call("procedure_pseudo_code", { procedure })).structuredContent, ); + const mainDirect = await direct("main"); const broken = await direct("broken"); const importSlot = await direct("puts"); expect(broken.normalized_result).toBeNull(); @@ -136,7 +172,11 @@ test("keeps each Kuna no-pseudocode observation's limitations on its batch item" expect(result).toMatchObject({ total: 4, succeeded: 1, failed: 3 }); const [main, failed, imported, unknown] = result.items; expect(main).toMatchObject({ address: "main", status: "ok" }); - expect(main).not.toHaveProperty("limitations"); + // A successful item keeps its own observation's caveats too, so a + // repeated selector answered differently stays distinguishable. + if (mainDirect.limitations.length === 0) + expect(main).not.toHaveProperty("limitations"); + else expect(main).toMatchObject({ limitations: mainDirect.limitations }); const noPseudocode = { status: "unavailable", reason: "no_pseudocode", @@ -147,12 +187,14 @@ test("keeps each Kuna no-pseudocode observation's limitations on its batch item" address: "broken", procedure: { status: "resolved", address: "0x1080", name: "broken" }, ...noPseudocode, + pseudocode_absence: { kind: "provider-failure" }, limitations: broken.limitations, }); expect(imported).toMatchObject({ address: "puts", procedure: { status: "resolved", address: "0x2000" }, ...noPseudocode, + pseudocode_absence: { kind: "not-in-export" }, limitations: importSlot.limitations, }); expect(imported).not.toEqual( @@ -174,6 +216,9 @@ test("keeps each Kuna no-pseudocode observation's limitations on its batch item" ), ).toBe(true); + // The retained items view projects the same per-item facts. + await expectRetainedItems(call, batch.evidence_id, broken, importSlot); + // A batch-only bundle keeps the association without another provider read. const bundle = evidenceBundleSchema.parse( getEvidenceBundle.outputSchema.parse(