mirror of
https://github.com/morluto/rea.git
synced 2026-11-03 21:36:13 +00:00
fix!: preserve feature traces when owner lookup is unavailable (#8368)
* fix!: preserve feature traces when owner lookup is unavailable * test: strengthen feature trace cancellation assertions * fix: preserve feature trace provider readiness errors * test: verify all feature trace reference sources * test: verify IDA guidance at its owning boundary * style: format feature trace regression
This commit is contained in:
1 parent
f72638f6b1
commit
6e9ee97a69
16 files changed
+440
-37
No files matched your search
@@ -3,7 +3,10 @@ import { afterEach, describe, expect, it } from "vitest";
|
||||
import type { AnalysisOperationPort } from "../../../src/application/AnalysisProvider.js";
|
||||
import { createAnalysisExecution } from "../../../src/application/AnalysisProvider.js";
|
||||
import { EnhancedTools } from "../../../src/application/EnhancedTools.js";
|
||||
import { AnalysisOutputError } from "../../../src/domain/analysisErrorCore.js";
|
||||
import {
|
||||
AnalysisCapabilityUnavailableError,
|
||||
AnalysisOutputError,
|
||||
} from "../../../src/domain/analysisErrorCore.js";
|
||||
import { err, ok } from "../../../src/domain/result.js";
|
||||
|
||||
import { closeEnhancedToolResources, connect } from "./enhancedToolsHarness.js";
|
||||
@@ -137,6 +140,195 @@ const trace = async (analysis: AnalysisOperationPort) => {
|
||||
return result.structuredContent as { normalized_result: unknown };
|
||||
};
|
||||
|
||||
it("preserves references and explicitly unknown owners without claiming truncation", async () => {
|
||||
const baseline = provider();
|
||||
const output = await trace({
|
||||
execute: (operation, input, options) =>
|
||||
operation === "resolve_containing_procedure" && input.address === "0x20"
|
||||
? Promise.resolve(
|
||||
err(
|
||||
new AnalysisCapabilityUnavailableError(
|
||||
"fixture",
|
||||
operation,
|
||||
"not supported",
|
||||
),
|
||||
),
|
||||
)
|
||||
: baseline.execute(operation, input, options),
|
||||
});
|
||||
expect(output.normalized_result).toMatchObject({
|
||||
truncated: false,
|
||||
residual_unknowns: [expect.stringContaining("0x20")],
|
||||
references: [
|
||||
{
|
||||
target_address: "0x10",
|
||||
source_address: "0x20",
|
||||
containing_procedure: {
|
||||
query_address: "0x20",
|
||||
status: "unavailable",
|
||||
provider_id: "fixture",
|
||||
reason: "not supported",
|
||||
},
|
||||
},
|
||||
{
|
||||
target_address: "0x11",
|
||||
source_address: "0x21",
|
||||
containing_procedure: containing("0x21"),
|
||||
},
|
||||
],
|
||||
});
|
||||
expect(output.normalized_result).toMatchObject({
|
||||
residual_unknowns: [expect.stringContaining("not supported")],
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps distinct targets when a capped reference source has an unavailable owner", async () => {
|
||||
const calls: string[] = [];
|
||||
const baseline = provider({ duplicateProcedure: true, calls });
|
||||
const output = await trace({
|
||||
execute: (operation, input, options) => {
|
||||
if (operation === "xrefs") {
|
||||
calls.push(`xrefs:${String(input.address)}`);
|
||||
return Promise.resolve(
|
||||
ok(
|
||||
createAnalysisExecution(["0x20"], PROVIDER, {
|
||||
referencesTruncated: input.address === "0x10",
|
||||
}),
|
||||
),
|
||||
);
|
||||
}
|
||||
if (operation === "resolve_containing_procedure") {
|
||||
calls.push(`resolve_containing_procedure:${String(input.address)}`);
|
||||
return Promise.resolve(
|
||||
err(
|
||||
new AnalysisCapabilityUnavailableError(
|
||||
"fixture",
|
||||
operation,
|
||||
"not supported",
|
||||
),
|
||||
),
|
||||
);
|
||||
}
|
||||
return baseline.execute(operation, input, options);
|
||||
},
|
||||
});
|
||||
|
||||
expect(calls.filter((call) => call.startsWith("xrefs:"))).toEqual([
|
||||
"xrefs:0x10",
|
||||
"xrefs:0x11",
|
||||
]);
|
||||
expect(
|
||||
calls.filter((call) => call.startsWith("resolve_containing_procedure:")),
|
||||
).toEqual(["resolve_containing_procedure:0x20"]);
|
||||
expect(output.normalized_result).toMatchObject({
|
||||
truncated: true,
|
||||
residual_unknowns: [
|
||||
expect.stringContaining("0x10"),
|
||||
expect.stringContaining("0x20"),
|
||||
],
|
||||
references: [
|
||||
{
|
||||
target_address: "0x10",
|
||||
source_address: "0x20",
|
||||
containing_procedure: {
|
||||
status: "unavailable",
|
||||
reason: "not supported",
|
||||
},
|
||||
},
|
||||
{
|
||||
target_address: "0x11",
|
||||
source_address: "0x20",
|
||||
containing_procedure: {
|
||||
status: "unavailable",
|
||||
reason: "not supported",
|
||||
},
|
||||
},
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
it("does not swallow an unavailable owner with incomplete cleanup", async () => {
|
||||
const baseline = provider();
|
||||
const result = await new EnhancedTools({
|
||||
execute: (operation, input, options) =>
|
||||
operation === "resolve_containing_procedure"
|
||||
? Promise.resolve(
|
||||
err(
|
||||
new AnalysisCapabilityUnavailableError(
|
||||
"fixture",
|
||||
operation,
|
||||
"cleanup failed",
|
||||
{
|
||||
cleanup: {
|
||||
reason: "cleanup failed",
|
||||
resources: ["owned worker"],
|
||||
},
|
||||
},
|
||||
),
|
||||
),
|
||||
)
|
||||
: baseline.execute(operation, input, options),
|
||||
}).execute("trace_feature", { query: "hello" });
|
||||
expect(result.ok).toBe(false);
|
||||
if (!result.ok) expect(result.error.cleanupIncomplete).toBe(true);
|
||||
});
|
||||
|
||||
it("preserves cancellation that arrives during an unavailable owner lookup", async () => {
|
||||
const baseline = provider();
|
||||
const controller = new AbortController();
|
||||
const reason = { request: "cancel during owner lookup" };
|
||||
const result = await new EnhancedTools({
|
||||
execute: (operation, input, options) => {
|
||||
if (operation !== "resolve_containing_procedure")
|
||||
return baseline.execute(operation, input, options);
|
||||
controller.abort(reason);
|
||||
return Promise.resolve(
|
||||
err(
|
||||
new AnalysisCapabilityUnavailableError(
|
||||
"fixture",
|
||||
operation,
|
||||
"not supported",
|
||||
),
|
||||
),
|
||||
);
|
||||
},
|
||||
}).execute("trace_feature", { query: "hello" }, controller.signal);
|
||||
expect(result.ok).toBe(false);
|
||||
if (!result.ok) {
|
||||
expect(result.error._tag).toBe("AnalysisCancelledError");
|
||||
expect(result.error.cause).toBe(reason);
|
||||
}
|
||||
});
|
||||
|
||||
it("retains incomplete cleanup when cancellation arrives during owner lookup", async () => {
|
||||
const baseline = provider();
|
||||
const controller = new AbortController();
|
||||
const result = await new EnhancedTools({
|
||||
execute: (operation, input, options) => {
|
||||
if (operation !== "resolve_containing_procedure")
|
||||
return baseline.execute(operation, input, options);
|
||||
controller.abort();
|
||||
return Promise.resolve(
|
||||
err(
|
||||
new AnalysisCapabilityUnavailableError(
|
||||
"fixture",
|
||||
operation,
|
||||
"cleanup failed",
|
||||
{
|
||||
cleanup: {
|
||||
reason: "cleanup failed",
|
||||
resources: ["owned worker"],
|
||||
},
|
||||
},
|
||||
),
|
||||
),
|
||||
);
|
||||
},
|
||||
}).execute("trace_feature", { query: "hello" }, controller.signal);
|
||||
expect(result.ok).toBe(false);
|
||||
if (!result.ok) expect(result.error.cleanupIncomplete).toBe(true);
|
||||
});
|
||||
|
||||
describe("trace_feature reference coverage", () => {
|
||||
it("marks the trace truncated for the target whose references the provider capped", async () => {
|
||||
const output = await trace(
|
||||
|
||||
@@ -0,0 +1,87 @@
|
||||
import { rm } from "node:fs/promises";
|
||||
import { afterEach, expect, it } from "vitest";
|
||||
import { AnalysisCapabilityUnavailableError } from "../../../src/domain/analysisErrorCore.js";
|
||||
import { IdaSessionClient } from "../../../src/ida/IdaSessionClient.js";
|
||||
import { parseMcpToolError } from "../../fixtures/mcpToolError.js";
|
||||
import { createIdaTarget, RecordingIdaMcp } from "../../fixtures/idaMcp.js";
|
||||
import { closeEnhancedToolResources, connect } from "./enhancedToolsHarness.js";
|
||||
|
||||
afterEach(closeEnhancedToolResources);
|
||||
|
||||
it.each([2, 3])(
|
||||
"preserves unknown IDA readiness at owner health check %s",
|
||||
async (failureCheck) => {
|
||||
const { root, target } = await createIdaTarget("arm64");
|
||||
const producer = new RecordingIdaMcp(target, "headless");
|
||||
let xrefsSeen = false;
|
||||
let healthAfterXrefs = 0;
|
||||
producer.overrideCall = (name) => {
|
||||
if (name === "xrefs_to") xrefsSeen = true;
|
||||
if (
|
||||
name === "server_health" &&
|
||||
xrefsSeen &&
|
||||
++healthAfterXrefs >= failureCheck
|
||||
)
|
||||
return {
|
||||
status: "ok",
|
||||
input_path: producer.inputPath,
|
||||
idb_path: `${producer.inputPath}.i64`,
|
||||
module: "target.elf",
|
||||
imagebase: producer.imageBase,
|
||||
auto_analysis_ready: null,
|
||||
};
|
||||
return undefined;
|
||||
};
|
||||
const ida = new IdaSessionClient(
|
||||
{
|
||||
command: "fixture",
|
||||
args: [],
|
||||
env: {},
|
||||
mode: "headless",
|
||||
timeoutMs: 1000,
|
||||
workspaceRoot: root,
|
||||
},
|
||||
target,
|
||||
producer,
|
||||
);
|
||||
let refusal: AnalysisCapabilityUnavailableError | undefined;
|
||||
const mcp = await connect({
|
||||
execute: async (operation, input, options) => {
|
||||
const result = await ida.execute(operation, input, options);
|
||||
if (
|
||||
operation === "resolve_containing_procedure" &&
|
||||
!result.ok &&
|
||||
result.error instanceof AnalysisCapabilityUnavailableError
|
||||
)
|
||||
refusal = result.error;
|
||||
return result;
|
||||
},
|
||||
});
|
||||
try {
|
||||
const result = await mcp.callTool({
|
||||
name: "trace_feature",
|
||||
arguments: { query: "a.b" },
|
||||
});
|
||||
const error = parseMcpToolError(result).error;
|
||||
expect(error).toMatchObject({
|
||||
code: "capability_unavailable",
|
||||
details: {
|
||||
operation: "server_health",
|
||||
diagnostics: { auto_analysis_ready: null },
|
||||
},
|
||||
});
|
||||
expect(refusal?.remediationAction).toBeDefined();
|
||||
expect(error.remediation?.action).toBe(refusal?.remediationAction);
|
||||
expect(healthAfterXrefs).toBe(failureCheck);
|
||||
if (failureCheck === 3) {
|
||||
expect(error.details?.partial_observation).toMatchObject({
|
||||
operation: "resolve_containing_procedure",
|
||||
provider: { id: "ida" },
|
||||
});
|
||||
}
|
||||
} finally {
|
||||
expect((await ida.close()).ok).toBe(true);
|
||||
await rm(root, { recursive: true, force: true });
|
||||
}
|
||||
},
|
||||
);
|
||||
@@ -71,6 +71,8 @@ export interface FakeKunaBehavior {
|
||||
/** Bytes of an extra root member in the graph JSON. */
|
||||
readonly graphPadding?: number;
|
||||
readonly xrefs?: "ok" | "hang" | "not-json";
|
||||
/** Additional exact-target incoming call-site addresses. */
|
||||
readonly extraXrefSources?: readonly number[];
|
||||
readonly referenceDelayMs?: number;
|
||||
readonly referencePadding?: number;
|
||||
/** `invalid-then-hang` writes a contradictory first row, then never finishes. */
|
||||
@@ -155,7 +157,7 @@ if (args[0] === "--version") {
|
||||
const identity = (name, value) => ({ name, address: value, address_hex: "0x" + value.toString(16) });
|
||||
const aliases = target === "0x2000" ? [identity("puts.slot", 12288)] : [];
|
||||
const row = (from, to, kind) => ({ address: from, address_hex: "0x" + from.toString(16), from_address: from, from_address_hex: "0x" + from.toString(16), to_address: to, to_address_hex: "0x" + to.toString(16), kind, from_function: identity("main", 4096), to_function: null, instruction: "fixture reference" + "x".repeat(behavior.referencePadding ?? 0) });
|
||||
const rows = [row(4100, parseInt(target, 16), "call")];
|
||||
const rows = [4100, ...(behavior.extraXrefSources ?? [])].map((source) => row(source, parseInt(target, 16), "call"));
|
||||
if (aliases.length) rows.push(row(4164, 12288, "call"));
|
||||
setTimeout(() => process.stdout.write(JSON.stringify({ binary: args[1], direction: "to", count: rows.length, target: { ...identity(null, parseInt(target, 16)), aliases }, xrefs: rows })), behavior.referenceDelayMs ?? 0);
|
||||
}
|
||||
|
||||
@@ -146,6 +146,7 @@ it.each([
|
||||
async ({ rejectClose, rejectRead, wrongXrefs, wrongStrings }) => {
|
||||
const root = await createTestTempDirectory("rea-kuna-verifier-cleanup-");
|
||||
const kuna = await writeFakeKuna(root, {
|
||||
extraXrefSources: [4104],
|
||||
stringRows: [
|
||||
{ address: 12288, text: "REA fixture prompt" },
|
||||
{
|
||||
|
||||
Reference in new issue
Block a user