morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #2513] [CLOSED] fix(ida): keep binary_overview available without segment enumeration #5288

New issue

Closed

opened 2026-10-31 01:05:28 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:05:28 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2513
Author: @Jackzigen
Created: 10/13/2026
Status: ❌ Closed

Base: main ← Head: fix/ida-binary-overview-documents

📝 Commits (9)

72e4698 fix(ida): keep binary_overview available without segment enumeration
f60fab6 fix(ida): merge main and preserve public overview document identity
e768bab docs(skill): describe degraded IDA binary overview
8c87b46 fix(ida): reconcile public document identity with containment
e22c443 test(ida): cover overview contracts in attached workflows
f8b5880 fix(ida): validate workflow overview evidence separately
997023e fix(ida): compare binary overview with inventories
4766147 Merge branch 'main' into fix/ida-binary-overview-documents
70f20f4 fix(ci): verify IDA verifier cleanup and parity

📊 Changes

18 files changed (+393 additions, -59 deletions)

View changed files

📝 .agents/skills/reverse-engineer-anything/SKILL.md (+4 -2)
📝 docs/ida-provider.md (+2 -2)
➕ scripts/lib/ida-verifier-workspaces.mjs (+22 -0)
📝 scripts/verify-real-ida.mjs (+110 -4)
📝 src/application/CapabilityInventory.ts (+22 -6)
📝 src/application/EnhancedTools.ts (+17 -3)
📝 src/contracts/enhancedToolContracts.ts (+1 -1)
📝 src/contracts/toolOutputSchemaGroups.ts (+13 -11)
➕ src/ida/BinaryOverviewRequirement.test.ts (+59 -0)
📝 src/ida/IdaContainment.test.ts (+8 -11)
📝 src/ida/IdaNativeProtocol.test.ts (+5 -3)
📝 src/ida/IdaOperationRunner.ts (+5 -2)
📝 src/ida/IdaProviderCapabilities.ts (+1 -2)
📝 src/ida/IdaSessionClient.test.ts (+22 -0)
📝 tests/boundary/mcp/enhancedTools.04-uses-complete-inventories-inline.test.ts (+23 -0)
➕ tests/boundary/providers/ida/idaVerifierWorkspaces.test.ts (+26 -0)
📝 tests/boundary/providers/ida/verifierObservations.test.ts (+53 -11)
📝 tests/conformance/ida/providerContracts.test.ts (+0 -1)

📄 Description

Summary

binary_overview failed for every IDA configuration because provider selection could not be satisfied:

list_documents was not declared by the IDA adapter, so resolveAnalysisDocument fell back to current_document, which the IDA compatibility profiles also do not expose — document resolution failed before any analysis ran.
list_segments was a hard requirement, but none of the three upstream IDA MCP profiles expose a segment enumeration tool, so no IDA configuration could ever satisfy the composed requirement.

Changes

Declare and implement list_documents (IdaProviderCapabilities / IdaOperationRunner / IdaSessionClient): an IDA session binds exactly one target, so the adapter reports the bound input identity — the observed attached-input path, or the headless workspace input path — the same way Ghidra reports its single imported Program.
Mark list_segments optional in the binary_overview requirements (CapabilityInventory): requirement entries may end with ?; optional facets do not block provider selection.
Degrade gracefully in the composed workflow (EnhancedTools#binaryOverview): when list_segments reports capability-unavailable, the tool still returns document/procedure_count/string_count and omits segments/segment_count (unobserved), mirroring the existing degradation in #findXrefs. Every other segment failure still propagates.
Contract/docs: segments and segment_count are optional in the output schema; the tool description now states that missing segment fields mean the provider lacks that facet, not that the binary has no segments.

Design notes

I considered the two directions raised in the issue:

Hard-declaring both operations would make list_segments a lie: no upstream profile can enumerate segments, so the adapter would have to synthesize or fail at run time. Rejected.
Optional facet + omission keeps the tool truthful and available. The omitted fields read as unobserved, consistent with how the codebase treats empty facets elsewhere.

Testing

New src/ida/BinaryOverviewRequirement.test.ts (co-located with the adapter per repo convention):
- IDA headless declares list_documents;
- binary_overview requirements are satisfied when list_segments? is absent;
- a missing required operation still reports provider_missing.
Full test:fast: 426 files / 5528 tests passing. tsc --noEmit, lint, format:check, knip, metadata:check, verify:module-boundaries, verify:architecture-guards all clean.

Closes #2460

Summary by CodeRabbit

New Features
- Binary overviews now remain available when segment enumeration is unsupported. In that case, the overview includes procedure and string counts while omitting segment details; missing segment fields indicate that the information was not observed.
- IDA can now report the document associated with the active analysis session.
Bug Fixes
- Missing optional segment support no longer makes binary overview unavailable, while failures in required analysis operations continue to be reported.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/2513 **Author:** [@Jackzigen](https://github.com/Jackzigen) **Created:** 10/13/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/ida-binary-overview-documents` --- ### 📝 Commits (9) - [`72e4698`](https://github.com/morluto/rea/commit/72e469890f1eafb6a576027001e82ab334f8ab36) fix(ida): keep binary_overview available without segment enumeration - [`f60fab6`](https://github.com/morluto/rea/commit/f60fab62af646bbe732fa6fb839cf0eb469de393) fix(ida): merge main and preserve public overview document identity - [`e768bab`](https://github.com/morluto/rea/commit/e768bab89904e3e3088e3052abbcd75ed6141481) docs(skill): describe degraded IDA binary overview - [`8c87b46`](https://github.com/morluto/rea/commit/8c87b46a79bc8e75d6bc7aef3267c8188c386c9d) fix(ida): reconcile public document identity with containment - [`e22c443`](https://github.com/morluto/rea/commit/e22c4436fd593e0c5e1d4d0e3bde345fbfbd50db) test(ida): cover overview contracts in attached workflows - [`f8b5880`](https://github.com/morluto/rea/commit/f8b58806607d4d05df486fbd2e95ab4cae3313fa) fix(ida): validate workflow overview evidence separately - [`997023e`](https://github.com/morluto/rea/commit/997023ee41dc8222bc03c72bc8f2869d5239efca) fix(ida): compare binary overview with inventories - [`4766147`](https://github.com/morluto/rea/commit/476614785e64388a1561bd513a7da67074d6b875) Merge branch 'main' into fix/ida-binary-overview-documents - [`70f20f4`](https://github.com/morluto/rea/commit/70f20f4d87bfe6ca09d4c2c0ee898ddc8882a012) fix(ci): verify IDA verifier cleanup and parity ### 📊 Changes **18 files changed** (+393 additions, -59 deletions) <details> <summary>View changed files</summary> 📝 `.agents/skills/reverse-engineer-anything/SKILL.md` (+4 -2) 📝 `docs/ida-provider.md` (+2 -2) ➕ `scripts/lib/ida-verifier-workspaces.mjs` (+22 -0) 📝 `scripts/verify-real-ida.mjs` (+110 -4) 📝 `src/application/CapabilityInventory.ts` (+22 -6) 📝 `src/application/EnhancedTools.ts` (+17 -3) 📝 `src/contracts/enhancedToolContracts.ts` (+1 -1) 📝 `src/contracts/toolOutputSchemaGroups.ts` (+13 -11) ➕ `src/ida/BinaryOverviewRequirement.test.ts` (+59 -0) 📝 `src/ida/IdaContainment.test.ts` (+8 -11) 📝 `src/ida/IdaNativeProtocol.test.ts` (+5 -3) 📝 `src/ida/IdaOperationRunner.ts` (+5 -2) 📝 `src/ida/IdaProviderCapabilities.ts` (+1 -2) 📝 `src/ida/IdaSessionClient.test.ts` (+22 -0) 📝 `tests/boundary/mcp/enhancedTools.04-uses-complete-inventories-inline.test.ts` (+23 -0) ➕ `tests/boundary/providers/ida/idaVerifierWorkspaces.test.ts` (+26 -0) 📝 `tests/boundary/providers/ida/verifierObservations.test.ts` (+53 -11) 📝 `tests/conformance/ida/providerContracts.test.ts` (+0 -1) </details> ### 📄 Description ## Summary `binary_overview` failed for every IDA configuration because provider selection could not be satisfied: 1. **`list_documents` was not declared** by the IDA adapter, so `resolveAnalysisDocument` fell back to `current_document`, which the IDA compatibility profiles also do not expose — document resolution failed before any analysis ran. 2. **`list_segments` was a hard requirement**, but none of the three upstream IDA MCP profiles expose a segment enumeration tool, so no IDA configuration could ever satisfy the composed requirement. ## Changes - **Declare and implement `list_documents`** (`IdaProviderCapabilities` / `IdaOperationRunner` / `IdaSessionClient`): an IDA session binds exactly one target, so the adapter reports the bound input identity — the observed attached-input path, or the headless workspace input path — the same way Ghidra reports its single imported Program. - **Mark `list_segments` optional** in the `binary_overview` requirements (`CapabilityInventory`): requirement entries may end with `?`; optional facets do not block provider selection. - **Degrade gracefully in the composed workflow** (`EnhancedTools#binaryOverview`): when `list_segments` reports capability-unavailable, the tool still returns `document`/`procedure_count`/`string_count` and omits `segments`/`segment_count` (unobserved), mirroring the existing degradation in `#findXrefs`. Every other segment failure still propagates. - **Contract/docs**: `segments` and `segment_count` are optional in the output schema; the tool description now states that missing segment fields mean the provider lacks that facet, not that the binary has no segments. ## Design notes I considered the two directions raised in the issue: - *Hard-declaring both operations* would make `list_segments` a lie: no upstream profile can enumerate segments, so the adapter would have to synthesize or fail at run time. Rejected. - *Optional facet + omission* keeps the tool truthful and available. The omitted fields read as unobserved, consistent with how the codebase treats empty facets elsewhere. ## Testing - New `src/ida/BinaryOverviewRequirement.test.ts` (co-located with the adapter per repo convention): - IDA headless declares `list_documents`; - `binary_overview` requirements are satisfied when `list_segments?` is absent; - a missing **required** operation still reports `provider_missing`. - Full `test:fast`: 426 files / 5528 tests passing. `tsc --noEmit`, `lint`, `format:check`, `knip`, `metadata:check`, `verify:module-boundaries`, `verify:architecture-guards` all clean. Closes #2460  ## Summary by CodeRabbit * **New Features** * Binary overviews now remain available when segment enumeration is unsupported. In that case, the overview includes procedure and string counts while omitting segment details; missing segment fields indicate that the information was not observed. * IDA can now report the document associated with the active analysis session. * **Bug Fixes** * Missing optional segment support no longer makes binary overview unavailable, while failures in required analysis operations continue to be reported.  --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:05:28 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:43:35 +00:00

[PR #5288] [CLOSED] fix(server): refuse unretained layout and JavaScript summaries before analysis #6859

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#5288

No description provided.

Rows
Columns