From 8ed3d1df72bfae2104197e7ec894db4ccb1730f0 Mon Sep 17 00:00:00 2001 From: Kaoru0822 Date: Thu, 15 Oct 2026 05:19:25 -0700 Subject: [PATCH] fix(kuna): list a cold target from the export's own inventory (#4062) * fix(kuna): list a cold target from the export's own inventory On a cold cache the session ran `kuna functions --json` for listings while the background export ran the same inventory as its first step, so every cold target was inventoried twice at once. On a 254 MB Mach-O each run took about 66 s of CPU. The quick run was also bound by the single-function deadline. The export's status now carries its parsed inventory, the cache replays the latest status to requests that join a running build, and listings wait for that inventory. The session runs its own inventory only when the export ends without one. Refs #1957 Co-Authored-By: Claude Opus 5.5 * fix(kuna): replay a running build's status after its waiter can cancel The cache replayed the latest status to a joining request before it registered that request's abort listener, so a listener that cancelled on the replayed status was ignored and the request waited for the export to finish. Replay now happens once cancellation is registered. Co-Authored-By: Claude Opus 5.5 * perf(kuna): publish the export's inventory without parsing it twice The export parsed `functions.json` for its status and then parsed the file it had just written again to publish, so two expanded inventories coexisted while the export finished. Publishing now reuses the inventory parsed from that same text. Cached lookups still parse the published file. Co-Authored-By: Claude Opus 5.5 * test(kuna): exercise export inventory sharing from pseudocode requests Inventory-only requests no longer start the export, so a cold request that shares the export's inventory is now a text request. The tests and the guide describe that trigger. Co-Authored-By: Claude Opus 5.5 * fix(kuna): let a listing wait for an export that is still starting A listing that arrived while a concurrent text request was still looking up the cache saw no export inventory yet and started its own `kuna functions`, and the export then started another. The listing now waits for that startup decision before falling back to its own run. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- docs/kuna-provider.md | 4 +- src/kuna/KunaProjectCache.ts | 8 ++ src/kuna/KunaProjectExport.ts | 60 ++++++++---- src/kuna/KunaSessionClient.ts | 33 ++++++- .../kuna/kunaExportInventory.test.ts | 98 +++++++++++++++++++ .../providers/kuna/kunaProjectCache.test.ts | 15 ++- .../kuna/kunaProjectConcurrency.test.ts | 37 +++++++ 7 files changed, 224 insertions(+), 31 deletions(-) create mode 100644 tests/boundary/providers/kuna/kunaExportInventory.test.ts diff --git a/docs/kuna-provider.md b/docs/kuna-provider.md index c938e37695..d7b909052f 100644 --- a/docs/kuna-provider.md +++ b/docs/kuna-provider.md @@ -19,7 +19,7 @@ Select it explicitly with `--provider kuna`, MCP's `open_binary.provider_id: "ku | `REA_KUNA_CACHE_DIR` | `~/Library/Caches/rea/kuna` (macOS), `$XDG_CACHE_HOME/rea/kuna` (Linux; `~/.cache/rea/kuna` when `XDG_CACHE_HOME` is unset or relative) | Absolute directory REA owns for project exports. Kuna's own spec cache also lives here, not in your Kuna cache. | | `REA_KUNA_CACHE_MAX_BYTES` | 4 GiB | Size cap; least recently used exports are evicted above it. | | `REA_KUNA_PROJECT_TIMEOUT_MS` | 30 minutes | Deadline for one project export. | -| `REA_KUNA_TIMEOUT_MS` | 120 s | Deadline for one single-function decompile. | +| `REA_KUNA_TIMEOUT_MS` | 120 s | Deadline for one single-function decompile, and for the inventory run REA makes when the export ends without one. | | `REA_KUNA_JOBS` | 1 | `decompile-project --jobs`. 1 is Kuna's reference answer; more jobs finish sooner but lose callee-first prototypes, and change about 40–60% of function bodies on the measured binaries. | Windows is not supported yet. Supported targets are ELF and thin Mach-O executables for x86-64, AArch64 and ARM; PE and universal Mach-O are rejected with a typed reason. @@ -45,7 +45,7 @@ The first text or assembly request in a session looks for this target's export i - **Warm cache:** analysis reads reuse the verified export. A fresh server verifies its file digests once, even for concurrent first requests; that cost scales with export size. - **Cold cache:** - REA starts `kuna decompile-project` in the background on a private copy of the target. - - Meanwhile, pseudocode comes from one `kuna decompile --addr` run, and listings from `kuna functions --json`. + - Meanwhile, pseudocode comes from one `kuna decompile --addr` run. Name and address lookups, and listings, use the inventory the export takes with `kuna functions --json` before it decompiles. The session runs `kuna functions` itself, under `REA_KUNA_TIMEOUT_MS`, only when no export is building (an inventory-only request does not start one) or the export ends without an inventory. - Requests for the same function share one run; it stops when its last waiting request is cancelled. Each run analyzes the whole image again in its own process, so one REA process runs at most as many at once as the host has cores beyond `REA_KUNA_JOBS` (at least one). Further requests wait in order, and a waiting request is answered from the export if it finishes first. A run whose stop REA cannot prove keeps its slot until `close_binary` proves it stopped. - Assembly and data names wait for the export; progress notifications report the wait every ten seconds. - A single-function answer says so in its limitations. It lacks the export's whole-program prototypes, so a later request can return different text for the same function. diff --git a/src/kuna/KunaProjectCache.ts b/src/kuna/KunaProjectCache.ts index 29a0f1e959..e137243423 100644 --- a/src/kuna/KunaProjectCache.ts +++ b/src/kuna/KunaProjectCache.ts @@ -150,6 +150,8 @@ interface InflightBuild { readonly promise: Promise>; readonly controller: AbortController; readonly listeners: Set<(status: KunaExportStatus) => void>; + /** The latest status, replayed to requests that join after it was reported. */ + status: KunaExportStatus | undefined; waiters: number; } @@ -334,7 +336,9 @@ export class KunaProjectCache { return this.#await(running, request); const controller = new AbortController(); const listeners = new Set<(status: KunaExportStatus) => void>(); + // #produce reports its first status only after an await, once `created` exists. const promise = this.#produce(key, request, controller.signal, (status) => { + created.status = status; for (const listener of listeners) listener(status); }); const created: InflightBuild = { @@ -342,6 +346,7 @@ export class KunaProjectCache { promise, controller, listeners, + status: undefined, waiters: 0, }; this.#inflight.set(key, created); @@ -465,6 +470,9 @@ export class KunaProjectCache { leave(); resolve(result); }); + // Replay only once cancellation is registered, so a listener may abort. + if (listener !== undefined && entry.status !== undefined) + listener(entry.status); }); } diff --git a/src/kuna/KunaProjectExport.ts b/src/kuna/KunaProjectExport.ts index a782cdeccb..25b25606d9 100644 --- a/src/kuna/KunaProjectExport.ts +++ b/src/kuna/KunaProjectExport.ts @@ -67,8 +67,11 @@ export interface KunaExport { /** The step an export is in; Kuna itself reports no progress until it finishes. */ export interface KunaExportStatus { readonly phase: "inventory" | "decompiling" | "sealing"; - /** Callable entries found by the inventory, once known. */ - readonly inventoryFunctions: number | null; + /** + * Every callable entry the export's inventory found, once known. Listings + * use it while the export still decompiles, so no second inventory runs. + */ + readonly inventory: readonly KunaInventoryFunction[] | null; } /** @@ -207,7 +210,7 @@ export const exportKunaProject = async ( }; }; - request.onStatus({ phase: "inventory", inventoryFunctions: null }); + request.onStatus({ phase: "inventory", inventory: null }); const listed = await run( ["functions", input, "--json"], KUNA_INVENTORY_OUTPUT_BYTES, @@ -223,10 +226,7 @@ export const exportKunaProject = async ( { mode: 0o600 }, ); - request.onStatus({ - phase: "decompiling", - inventoryFunctions: inventory.value.length, - }); + request.onStatus({ phase: "decompiling", inventory: inventory.value }); const exported = await run( [ "decompile-project", @@ -240,10 +240,7 @@ export const exportKunaProject = async ( ); if (!exported.ok) return exported.outcome; - request.onStatus({ - phase: "sealing", - inventoryFunctions: inventory.value.length, - }); + request.onStatus({ phase: "sealing", inventory: inventory.value }); const summary = parseKunaExportSummary(exported.snapshot.stdout.text); if (summary === undefined) return unusable( @@ -272,22 +269,44 @@ export const exportKunaProject = async ( files[role] = { path: name, ...(await hashStableFile(path, signal)) }; } const complete = files as KunaExportFiles; - const parsed = await readKunaExport( + const functions = await readKunaExportSource( request.output, - complete, + complete.c, summary, signal, ); - if (!parsed.ok) return unusable(parsed.error); + if (!functions.ok) return unusable(functions.error); const identity = await verifyKunaExecutable(installation, signal); if (!identity.ok) return { kind: "done", result: identity }; + // `functions.json` holds exactly the text this inventory was parsed from, so + // publishing reuses it rather than holding a second parsed copy. return { kind: "done", - result: ok({ files: complete, summary, ...parsed.value }), + result: ok({ + files: complete, + summary, + functions: functions.value, + inventory: inventory.value, + }), }; }; -/** Parse an export's function blocks and inventory from its own files. */ +/** Split an export's `.c` into its function blocks without buffering the file. */ +const readKunaExportSource = ( + directory: string, + file: KunaExportFile, + summary: KunaExportSummary, + signal: AbortSignal | undefined, +): Promise> => + scanKunaProjectSource( + createReadStream( + join(directory, file.path), + signal === undefined ? {} : { signal }, + ), + summary, + ); + +/** Parse a published export's function blocks and inventory from its own files. */ export const readKunaExport = async ( directory: string, files: KunaExportFiles, @@ -302,12 +321,11 @@ export const readKunaExport = async ( string > > => { - const functions = await scanKunaProjectSource( - createReadStream( - join(directory, files.c.path), - signal === undefined ? {} : { signal }, - ), + const functions = await readKunaExportSource( + directory, + files.c, summary, + signal, ); if (!functions.ok) return functions; const inventory = parseKunaInventory( diff --git a/src/kuna/KunaSessionClient.ts b/src/kuna/KunaSessionClient.ts index af8d017156..60f07e213a 100644 --- a/src/kuna/KunaSessionClient.ts +++ b/src/kuna/KunaSessionClient.ts @@ -147,7 +147,7 @@ const procedureSelectorSchema = z.string().min(1); * One Kuna analysis session for one target. * * Reads come from the verified project export in REA's cache. Until that - * export exists, listings use a quick `kuna functions` inventory and + * export exists, listings use the inventory the export takes first and * pseudocode a single-function `kuna decompile`, while the export builds in * the background; assembly waits for it. Kuna always runs on a private copy * whose digest matches the selected target. @@ -173,6 +173,10 @@ export class KunaSessionClient implements AnalysisClient { #project: Promise> | undefined; /** The cache lookup that decides #project, shared by concurrent first requests. */ #starting: Promise | undefined; + /** The building export's inventory; undefined when it settles without one. */ + #exportInventory: + | Promise + | undefined; #reader: KunaProjectReader | undefined; #inventoryIndex: InventoryIndex | undefined; #status: KunaExportStatus | undefined; @@ -451,7 +455,7 @@ export class KunaSessionClient implements AnalysisClient { return { state: "building", phase: this.#status?.phase ?? null, - inventory_functions: this.#status?.inventoryFunctions ?? null, + inventory_functions: this.#status?.inventory?.length ?? null, }; } @@ -1172,6 +1176,12 @@ export class KunaSessionClient implements AnalysisClient { this.#project = Promise.resolve(ok(cached)); return; } + let listed!: ( + inventory: readonly KunaInventoryFunction[] | undefined, + ) => void; + this.#exportInventory = new Promise((resolve) => { + listed = resolve; + }); this.#project = this.#cache .build({ installation, @@ -1180,9 +1190,11 @@ export class KunaSessionClient implements AnalysisClient { signal: this.#session.signal, onStatus: (status) => { this.#status = status; + if (status.inventory !== null) listed(status.inventory); }, }) .then((result) => { + listed(result.ok ? result.value.inventory : undefined); if (result.ok && !this.#closing) this.#adopt(result.value); else if (!result.ok && !this.#closing) this.#exportFailure = @@ -1281,27 +1293,40 @@ export class KunaSessionClient implements AnalysisClient { } } - /** The export's inventory when ready, otherwise one quick `kuna functions` run. */ + /** The published or building export's inventory, otherwise one `kuna functions` run. */ async #ensureInventory( installation: KunaInstallation, options: ExecutionOptions, ): Promise> { if (this.#reader !== undefined) return ok(this.#reader.project.inventory); + const exported = this.#status?.inventory; + if (exported !== undefined && exported !== null) return ok(exported); if (this.#inventory === undefined) { // The session owns the shared run; each caller only stops waiting. const run = this.#listInventory(installation); this.#inventory = run; void run.then((listed) => { - // A failed quick listing is retried by the next request. + // A failed listing is retried by the next request. if (!listed.ok && this.#inventory === run) this.#inventory = undefined; }); } return waitFor(this.#inventory, options.signal, "list_procedures"); } + /** + * The export lists every callable entry before it decompiles, so listings + * wait for that inventory. Only an export that ends without one, or none at + * all, leads to a separate `kuna functions` run. + */ async #listInventory( installation: KunaInstallation, ): Promise> { + // A concurrent text request may still be choosing between the cache and a + // build; its outcome decides whether an inventory already exists. + await this.#starting?.catch(() => undefined); + if (this.#reader !== undefined) return ok(this.#reader.project.inventory); + const exported = await this.#exportInventory; + if (exported !== undefined) return ok(exported); const run = await this.#commands.run({ installation, arguments: (input) => ["functions", input, "--json"], diff --git a/tests/boundary/providers/kuna/kunaExportInventory.test.ts b/tests/boundary/providers/kuna/kunaExportInventory.test.ts new file mode 100644 index 0000000000..f83e826b4a --- /dev/null +++ b/tests/boundary/providers/kuna/kunaExportInventory.test.ts @@ -0,0 +1,98 @@ +import { expect, it as test } from "vitest"; + +import { fakeKunaCalls } from "./fakeKuna.js"; +import { exportsOf, run, session } from "./kunaSessionFixture.js"; + +const it = test.skipIf(process.platform === "win32"); + +const inventoriesOf = async (kuna: string) => + (await fakeKunaCalls(kuna)).filter(({ argv }) => argv[0] === "functions"); + +it("resolves a cold request from the export's inventory without a second run", async () => { + const { kuna, open } = await session("rea-kuna-export-inventory-", { + project: "hang", + inventoryDelayMs: 200, + }); + const client = open(); + try { + // Pseudocode starts the export, then resolves its name from the inventory. + expect( + (await run(client, "procedure_pseudo_code", { procedure: "parse" })) + .rawResult, + ).toMatchObject({ source: { kind: "single-function" } }); + expect( + (await run(client, "search_procedures", { pattern: "main" })).result, + ).toEqual([{ address: "0x1000", value: "main" }]); + // The export goes on to decompile; its inventory answered both requests. + await expect.poll(() => exportsOf(kuna)).toHaveLength(1); + expect(await inventoriesOf(kuna)).toHaveLength(1); + } finally { + expect(await client.close()).toEqual({ ok: true, value: null }); + } +}); + +it("gives a session that joins a running export the inventory it already listed", async () => { + const { kuna, open } = await session("rea-kuna-joined-inventory-", { + project: "hang", + }); + const first = open(); + const second = open(); + try { + await run(first, "procedure_pseudo_code", { procedure: "main" }); + await expect.poll(() => exportsOf(kuna)).toHaveLength(1); + expect( + (await run(second, "procedure_pseudo_code", { procedure: "parse" })) + .rawResult, + ).toMatchObject({ source: { kind: "single-function" } }); + expect(await inventoriesOf(kuna)).toHaveLength(1); + } finally { + expect(await second.close()).toEqual({ ok: true, value: null }); + expect(await first.close()).toEqual({ ok: true, value: null }); + } +}); + +it("runs its own inventory when the export ends before listing", async () => { + const { kuna, open } = await session("rea-kuna-inventory-fallback-", { + inventory: "not-json", + }); + const client = open(); + try { + expect( + await client.execute("procedure_pseudo_code", { procedure: "main" }), + ).toMatchObject({ + ok: false, + error: { + userMessage: expect.stringContaining( + "Kuna's function inventory is not usable", + ), + }, + }); + // The export's listing failed first; the session's own run reports why. + expect(await inventoriesOf(kuna)).toHaveLength(2); + } finally { + expect(await client.close()).toEqual({ ok: true, value: null }); + } +}); + +it("lets a listing that arrives while the export starts wait for its inventory", async () => { + const { kuna, open } = await session("rea-kuna-starting-inventory-", { + project: "hang", + }); + const client = open(); + try { + // The pseudocode request is still deciding between the cache and a build + // when the search arrives; the search must not start its own inventory. + const [text, search] = await Promise.all([ + run(client, "procedure_pseudo_code", { procedure: "main" }), + run(client, "search_procedures", { pattern: "parse" }), + ]); + expect(text.rawResult).toMatchObject({ + source: { kind: "single-function" }, + }); + expect(search.result).toEqual([{ address: "0x1040", value: "parse" }]); + await expect.poll(() => exportsOf(kuna)).toHaveLength(1); + expect(await inventoriesOf(kuna)).toHaveLength(1); + } finally { + expect(await client.close()).toEqual({ ok: true, value: null }); + } +}); diff --git a/tests/boundary/providers/kuna/kunaProjectCache.test.ts b/tests/boundary/providers/kuna/kunaProjectCache.test.ts index 0885029ea4..b445169e09 100644 --- a/tests/boundary/providers/kuna/kunaProjectCache.test.ts +++ b/tests/boundary/providers/kuna/kunaProjectCache.test.ts @@ -463,11 +463,18 @@ it("reports each build phase to every waiting request", async () => { }); expect(result.ok).toBe(true); - expect(statuses).toEqual([ - { phase: "inventory", inventoryFunctions: null }, - { phase: "decompiling", inventoryFunctions: 3 }, - { phase: "sealing", inventoryFunctions: 3 }, + expect( + statuses.map(({ phase, inventory }) => [phase, inventory?.length ?? null]), + ).toEqual([ + ["inventory", null], + ["decompiling", 3], + ["sealing", 3], ]); + // Waiters receive the export's own parsed inventory, and publishing keeps + // that one array rather than parsing a second copy. + expect(statuses.at(-1)?.inventory).toBe( + result.ok ? result.value.inventory : undefined, + ); }); it("sweeps retired entries but preserves staging with unresolved process ownership", async () => { diff --git a/tests/boundary/providers/kuna/kunaProjectConcurrency.test.ts b/tests/boundary/providers/kuna/kunaProjectConcurrency.test.ts index ccb3c50c32..700ef91620 100644 --- a/tests/boundary/providers/kuna/kunaProjectConcurrency.test.ts +++ b/tests/boundary/providers/kuna/kunaProjectConcurrency.test.ts @@ -250,3 +250,40 @@ it("keeps a shared progress callback registered for the remaining waiter", async ).toHaveLength(1); expect(await cache.close()).toEqual({ ok: true, value: null }); }); + +it("lets a joining request cancel from the status replayed to it", async () => { + const { cache, request } = await fixture(); + const input = await request("program"); + const decompiling = barrier(); + const first = cache.build({ + ...input, + onStatus: (status) => { + if (status.phase === "decompiling") decompiling.release(); + }, + }); + let firstSettled = false; + void first.then(() => { + firstSettled = true; + }); + await decompiling.promise; + + const leaving = new AbortController(); + const replayed: KunaExportStatus[] = []; + const second = await cache.build({ + ...input, + signal: leaving.signal, + onStatus: (status) => { + replayed.push(status); + leaving.abort(); + }, + }); + expect(replayed.map(({ phase }) => phase)).toEqual(["decompiling"]); + expect(second).toMatchObject({ + ok: false, + error: { _tag: "AnalysisCancelledError" }, + }); + // The second request left without waiting for, or stopping, the first. + expect(firstSettled).toBe(false); + expect((await first).ok).toBe(true); + expect(await cache.close()).toEqual({ ok: true, value: null }); +});