[PR #7017] fix(discovery): advertise attached IDA navigation effects #7577

Open
opened 2026-11-03 13:38:50 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/7017
Author: @dhruvac29
Created: 10/24/2026
Status: 🔄 Open

Base: main ← Head: fix/5829-ida-tool-effects


📝 Commits (10+)

  • 279c7fb fix(discovery): advertise attached IDA navigation effects
  • ef32dc2 fix(discovery): cover IDA provider effects across tools and workflows
  • d9035ee test(discovery): isolate IDA metadata from host tool probes
  • f5de206 test(discovery): compare live identity with session effect bases
  • 38c53d7 fix(discovery): audit complete IDA provider effect union
  • 2497500 fix(discovery): exclude unreachable provider workflow effects
  • 3cbf3ce Merge origin/main into fix/5829-ida-tool-effects
  • 197388a fix(discovery): propagate IDA effects through composed tools
  • f6f5047 Merge origin/main into fix/5829-ida-tool-effects
  • b61201f fix(artifacts): deduplicate keyed archive path diagnostics

📊 Changes

9 files changed (+644 additions, -58 deletions)

View changed files

📝 src/application/AnalysisContextQueries.ts (+10 -7)
📝 src/application/CapabilityInventory.ts (+48 -11)
📝 src/contracts/toolEffects.test.ts (+61 -0)
📝 src/contracts/toolEffects.ts (+105 -32)
➕ tests/boundary/mcp/idaToolEffects.test.ts (+162 -0)
📝 tests/boundary/mcp/serverIdentity.test.ts (+11 -5)
📝 tests/boundary/mcp/toolSchemaValidity.test.ts (+1 -1)
📝 tests/boundary/providers/ida/idaCapabilityInventory.test.ts (+240 -1)
📝 website/wrangler.toml (+6 -1)

📄 Description

Summary

Fixes #5829. tools/list now advertises possible IDA UI, network, process, and filesystem effects, while binary_session reports effects composed from the selected provider. The affected attached IDA navigation tools no longer present openWorldHint: false when their supported IDA route may access the network.

Problem and expected behavior

With the recording producer in an attached legacy IDA session, binary_session reported UI and network effects for procedure_pseudo_code, procedure_assembly, read_function_instructions, and analyze_function; the canonical tool audit omitted them. A caller reading SDK discovery before invocation therefore received contradictory side-effect metadata. Composed workflows and ordinary IDA operations had similar possible-network omissions.

Change and scope

  • toolEffects.ts keeps each affected tool's provider-neutral session base with conservative global additions for supported IDA routes. toolSessionEffects derives the base from that entry; the separate navigation override list is removed.
  • CapabilityInventory.ts composes the session base with selected-provider descriptors. Requirement groups prevent unreachable workflow steps from lending effects. AnalysisContextQueries.ts shares the address-context facet list used by this composition and retains current cancellation handling.
  • Source, provider-boundary, and MCP tests cover global versus selected-provider effects, attached/native/headless distinctions, direct and composed workflows, and the four reported operations through the recording producer.
  • Lifecycle cleanup can call IDA's configured MCP transport while opening another target, opening bundle resources, closing a binary, or resetting the investigation. The canonical audit now includes network access for those four tools, even when cleanup belongs to a prior provider.

This is an ordinary discovery-metadata fix within existing provider boundaries. It adds no target authority or IDA execution path; a maintainer scope decision is not applicable. The branch is merged with main at b912ec184. The diff contains eight discovery and regression-test files plus a formatting-only fix to website/wrangler.toml, whose new main version failed the repository formatter in merged CI.

Contract and boundary impact

  • Semantic owner and earliest changed stage: canonical effects in src/contracts/toolEffects.ts; session composition in src/application/CapabilityInventory.ts.
  • Contribution scope fit and linked maintainer decision: ordinary fix; not applicable.
  • CLI and MCP/tool-catalog contract: no schema change. SDK openWorldHint derives from conservative possible effects; binary_session retains selected-provider facts.
  • Provider, bridge, target-format, or platform compatibility: attached legacy IDA exposes UI on the four navigation operations; native attached and headless profiles retain their profile-specific UI/process behavior. IDA transport may access the network.
  • Evidence, artifact, provenance, or reconstruction contract: unchanged.
  • Target selection, authority, upstream behavior, and cleanup execution: unchanged; only the advertised effects of existing cleanup paths changed.
  • Generated metadata, package, or installation impact: none.

Expected effects:

  • Read existing files or captured data.
  • Launch or attach to a process.
  • Inject instrumentation or run scripts/code.
  • Change process memory, registers, files, or configuration.
  • Contact network services.
  • Recover protected code or work around protections.
  • Documentation or usability only; no target interaction.
  • Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

  • Tests added or updated: src/contracts/toolEffects.test.ts, tests/boundary/providers/ida/idaCapabilityInventory.test.ts, tests/boundary/mcp/idaToolEffects.test.ts, tests/boundary/mcp/serverIdentity.test.ts, and tests/boundary/mcp/toolSchemaValidity.test.ts.
  • Base evidence: the issue's executed reproduction recorded false UI/network tool effects and openWorldHint: false for the four attached operations. I did not rerun the base revision. The new MCP assertions would fail against that base audit.
  • User-visible MCP output: the production server with a recording IDA producer returned provider may_show_ui: true, tool changesUiState: true, provider may_access_network: true, tool accessesNetwork: true, and tools/list openWorldHint: true for all four operations.
  • Relevant boundary coverage: SDK tools/list and binary_session on the same recording session, direct calls, selected-provider capabilities, composed workflow effects, and lifecycle cleanup metadata. No lifecycle execution behavior changed.
  • Remaining proof gap: no real IDA engine, GUI movement, or network traffic was observed. The tests establish discovery metadata, not real-provider behavior.

The existing Evidence contract is unchanged; tests preserve unavailable and profile-specific effects rather than inferring them from a global audit.

Validation performed

  • npm run test:local -- src/contracts/toolEffects.test.ts src/application/CapabilityInventory.test.ts src/application/AnalysisContextQueries.test.ts src/application/javascript/JavaScriptArtifactGraphStructure.test.ts — 105 passed on the final merged tree.
  • npm run test:focused -- tests/boundary/providers/ida/idaCapabilityInventory.test.ts tests/boundary/mcp/idaToolEffects.test.ts tests/boundary/mcp/serverIdentity.test.ts — 22 passed on the final merged tree.
  • npm run test:focused -- src/contracts/toolEffects.test.ts tests/boundary/providers/ida/idaCapabilityInventory.test.ts tests/boundary/mcp/idaToolEffects.test.ts — 22 passed after the lifecycle audit fix.
  • npm run test:focused -- tests/boundary/mcp/toolSchemaValidity.test.ts — 16 passed after aligning its close_binary authority hint with the lifecycle audit.
  • npm run test:focused -- src/contracts/toolEffects.test.ts tests/boundary/providers/ida/idaCapabilityInventory.test.ts tests/boundary/mcp/idaToolEffects.test.ts tests/boundary/mcp/toolSchemaValidity.test.ts — 38 passed after merging current main.
  • npm run test:focused -- tests/boundary/filesystem/javascript-applications/electronApplicationAnalysis.test.ts — 10 passed after removing the superseded JavaScript change from the final diff.
  • Seven additional boundary fixture files passed 35 tests during conflict resolution; their files were retained from current main.
  • npm run check:ci:fast — typecheck, lint, module-boundary verification, and repository-wide formatting passed after the lifecycle audit fix.
  • git diff --check origin/main...HEAD — passed.
  • GitHub CI on edd11d036 found one stale close_binary hint assertion in shard 7; shards 1, 5, and 6 were canceled after that failure. The assertion is fixed in d14295e84, and full CI on this current head is pending.
  • GitHub CI on d14295e84 found website/wrangler.toml unformatted in the newly merged main. The one-line list is formatted in a113717c4; npm run check:ci:fast passed locally on the merged tree. Full CI on the current head passed, including all eight test shards, source tests, static checks, and website checks.

Compatibility, safety, and release

  • Breaking changes or migration steps: none; this corrects advertised possible effects without changing calls or configuration.
  • Real provider coverage: recording IDA producer only; no IDA engine or GUI.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: no additional authority. The advertised network and UI effects already exist in the IDA provider descriptors.

Author readiness

Human owner: @dhruvac29

  • I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
  • I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
  • I declare this PR ready for initial maintainer review.

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🔄 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/7017 **Author:** [@dhruvac29](https://github.com/dhruvac29) **Created:** 10/24/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/5829-ida-tool-effects` --- ### 📝 Commits (10+) - [`279c7fb`](https://github.com/morluto/rea/commit/279c7fba4c342bc2d7de1b9f7cd1de820a8f4eb9) fix(discovery): advertise attached IDA navigation effects - [`ef32dc2`](https://github.com/morluto/rea/commit/ef32dc2be8f0d615edd7b5d680b9b3f6908f851a) fix(discovery): cover IDA provider effects across tools and workflows - [`d9035ee`](https://github.com/morluto/rea/commit/d9035eec99212d6ed24f7d21f5732820b3c7f1b5) test(discovery): isolate IDA metadata from host tool probes - [`f5de206`](https://github.com/morluto/rea/commit/f5de2061893e5b4f8cecd97864cd45a456abf997) test(discovery): compare live identity with session effect bases - [`38c53d7`](https://github.com/morluto/rea/commit/38c53d74bedc221499394f86eac6ceb0e963d80d) fix(discovery): audit complete IDA provider effect union - [`2497500`](https://github.com/morluto/rea/commit/2497500ddfc8726fb035d445790854a190f19e18) fix(discovery): exclude unreachable provider workflow effects - [`3cbf3ce`](https://github.com/morluto/rea/commit/3cbf3ce4f7d973f519d6d81d6eb58c49d04f0216) Merge origin/main into fix/5829-ida-tool-effects - [`197388a`](https://github.com/morluto/rea/commit/197388adc5c41e453e6e41263776e05152858c79) fix(discovery): propagate IDA effects through composed tools - [`f6f5047`](https://github.com/morluto/rea/commit/f6f50475daefd724430a51126e45851171cd2335) Merge origin/main into fix/5829-ida-tool-effects - [`b61201f`](https://github.com/morluto/rea/commit/b61201f3c39905934ce7d94244e9c773a466a510) fix(artifacts): deduplicate keyed archive path diagnostics ### 📊 Changes **9 files changed** (+644 additions, -58 deletions) <details> <summary>View changed files</summary> 📝 `src/application/AnalysisContextQueries.ts` (+10 -7) 📝 `src/application/CapabilityInventory.ts` (+48 -11) 📝 `src/contracts/toolEffects.test.ts` (+61 -0) 📝 `src/contracts/toolEffects.ts` (+105 -32) ➕ `tests/boundary/mcp/idaToolEffects.test.ts` (+162 -0) 📝 `tests/boundary/mcp/serverIdentity.test.ts` (+11 -5) 📝 `tests/boundary/mcp/toolSchemaValidity.test.ts` (+1 -1) 📝 `tests/boundary/providers/ida/idaCapabilityInventory.test.ts` (+240 -1) 📝 `website/wrangler.toml` (+6 -1) </details> ### 📄 Description ## Summary Fixes #5829. `tools/list` now advertises possible IDA UI, network, process, and filesystem effects, while `binary_session` reports effects composed from the selected provider. The affected attached IDA navigation tools no longer present `openWorldHint: false` when their supported IDA route may access the network. ## Problem and expected behavior With the recording producer in an attached legacy IDA session, `binary_session` reported UI and network effects for `procedure_pseudo_code`, `procedure_assembly`, `read_function_instructions`, and `analyze_function`; the canonical tool audit omitted them. A caller reading SDK discovery before invocation therefore received contradictory side-effect metadata. Composed workflows and ordinary IDA operations had similar possible-network omissions. ## Change and scope - `toolEffects.ts` keeps each affected tool's provider-neutral session base with conservative global additions for supported IDA routes. `toolSessionEffects` derives the base from that entry; the separate navigation override list is removed. - `CapabilityInventory.ts` composes the session base with selected-provider descriptors. Requirement groups prevent unreachable workflow steps from lending effects. `AnalysisContextQueries.ts` shares the address-context facet list used by this composition and retains current cancellation handling. - Source, provider-boundary, and MCP tests cover global versus selected-provider effects, attached/native/headless distinctions, direct and composed workflows, and the four reported operations through the recording producer. - Lifecycle cleanup can call IDA's configured MCP transport while opening another target, opening bundle resources, closing a binary, or resetting the investigation. The canonical audit now includes network access for those four tools, even when cleanup belongs to a prior provider. This is an ordinary discovery-metadata fix within existing provider boundaries. It adds no target authority or IDA execution path; a maintainer scope decision is not applicable. The branch is merged with `main` at `b912ec184`. The diff contains eight discovery and regression-test files plus a formatting-only fix to `website/wrangler.toml`, whose new `main` version failed the repository formatter in merged CI. ## Contract and boundary impact - Semantic owner and earliest changed stage: canonical effects in `src/contracts/toolEffects.ts`; session composition in `src/application/CapabilityInventory.ts`. - Contribution scope fit and linked maintainer decision: ordinary fix; not applicable. - CLI and MCP/tool-catalog contract: no schema change. SDK `openWorldHint` derives from conservative possible effects; `binary_session` retains selected-provider facts. - Provider, bridge, target-format, or platform compatibility: attached legacy IDA exposes UI on the four navigation operations; native attached and headless profiles retain their profile-specific UI/process behavior. IDA transport may access the network. - Evidence, artifact, provenance, or reconstruction contract: unchanged. - Target selection, authority, upstream behavior, and cleanup execution: unchanged; only the advertised effects of existing cleanup paths changed. - Generated metadata, package, or installation impact: none. Expected effects: - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: `src/contracts/toolEffects.test.ts`, `tests/boundary/providers/ida/idaCapabilityInventory.test.ts`, `tests/boundary/mcp/idaToolEffects.test.ts`, `tests/boundary/mcp/serverIdentity.test.ts`, and `tests/boundary/mcp/toolSchemaValidity.test.ts`. - Base evidence: the issue's executed reproduction recorded false UI/network tool effects and `openWorldHint: false` for the four attached operations. I did not rerun the base revision. The new MCP assertions would fail against that base audit. - User-visible MCP output: the production server with a recording IDA producer returned provider `may_show_ui: true`, tool `changesUiState: true`, provider `may_access_network: true`, tool `accessesNetwork: true`, and `tools/list` `openWorldHint: true` for all four operations. - Relevant boundary coverage: SDK `tools/list` and `binary_session` on the same recording session, direct calls, selected-provider capabilities, composed workflow effects, and lifecycle cleanup metadata. No lifecycle execution behavior changed. - Remaining proof gap: no real IDA engine, GUI movement, or network traffic was observed. The tests establish discovery metadata, not real-provider behavior. The existing Evidence contract is unchanged; tests preserve unavailable and profile-specific effects rather than inferring them from a global audit. ## Validation performed - `npm run test:local -- src/contracts/toolEffects.test.ts src/application/CapabilityInventory.test.ts src/application/AnalysisContextQueries.test.ts src/application/javascript/JavaScriptArtifactGraphStructure.test.ts` — 105 passed on the final merged tree. - `npm run test:focused -- tests/boundary/providers/ida/idaCapabilityInventory.test.ts tests/boundary/mcp/idaToolEffects.test.ts tests/boundary/mcp/serverIdentity.test.ts` — 22 passed on the final merged tree. - `npm run test:focused -- src/contracts/toolEffects.test.ts tests/boundary/providers/ida/idaCapabilityInventory.test.ts tests/boundary/mcp/idaToolEffects.test.ts` — 22 passed after the lifecycle audit fix. - `npm run test:focused -- tests/boundary/mcp/toolSchemaValidity.test.ts` — 16 passed after aligning its `close_binary` authority hint with the lifecycle audit. - `npm run test:focused -- src/contracts/toolEffects.test.ts tests/boundary/providers/ida/idaCapabilityInventory.test.ts tests/boundary/mcp/idaToolEffects.test.ts tests/boundary/mcp/toolSchemaValidity.test.ts` — 38 passed after merging current `main`. - `npm run test:focused -- tests/boundary/filesystem/javascript-applications/electronApplicationAnalysis.test.ts` — 10 passed after removing the superseded JavaScript change from the final diff. - Seven additional boundary fixture files passed 35 tests during conflict resolution; their files were retained from current `main`. - `npm run check:ci:fast` — typecheck, lint, module-boundary verification, and repository-wide formatting passed after the lifecycle audit fix. - `git diff --check origin/main...HEAD` — passed. - GitHub CI on `edd11d036` found one stale `close_binary` hint assertion in shard 7; shards 1, 5, and 6 were canceled after that failure. The assertion is fixed in `d14295e84`, and full CI on this current head is pending. - GitHub CI on `d14295e84` found `website/wrangler.toml` unformatted in the newly merged `main`. The one-line list is formatted in `a113717c4`; `npm run check:ci:fast` passed locally on the merged tree. [Full CI on the current head](https://github.com/morluto/rea/actions/runs/41564809466) passed, including all eight test shards, source tests, static checks, and website checks. ## Compatibility, safety, and release - Breaking changes or migration steps: none; this corrects advertised possible effects without changing calls or configuration. - Real provider coverage: recording IDA producer only; no IDA engine or GUI. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no additional authority. The advertised network and UI effects already exist in the IDA provider descriptors. ## Author readiness Human owner: @dhruvac29 - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
Sign in to join this conversation.
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#7577
No description provided.