[PR #7140] [MERGED] fix(ghidra): scope rejected-input diagnostics to their request #7668

Closed
opened 2026-11-03 13:39:31 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/7140
Author: @gensokyo-dev
Created: 10/25/2026
Status: ✅ Merged
Merged: 10/25/2026
Merged by: @gensokyo-dev

Base: main ← Head: fix/6426-ghidra-input-diagnostics


📝 Commits (1)

  • 2b0aa21 fix(ghidra): scope rejected-input diagnostics to their request

📊 Changes

10 files changed (+260 additions, -52 deletions)

View changed files

📝 docs/ghidra-provider.md (+11 -0)
📝 src/ghidra/GhidraClient.ts (+17 -3)
📝 src/ghidra/GhidraDiagnostics.test.ts (+58 -0)
📝 src/ghidra/GhidraDiagnostics.ts (+25 -9)
📝 src/ghidra/GhidraProviderClient.ts (+2 -6)
📝 src/ghidra/GhidraSessionError.test.ts (+33 -0)
📝 src/ghidra/GhidraSessionError.ts (+16 -3)
📝 tests/boundary/providers/ghidra/ghidraClient.fixture.ts (+1 -0)
📝 tests/boundary/providers/ghidra/ghidraClient.test.ts (+65 -22)
📝 tests/fixtures/fakeGhidra.mjs (+32 -9)

📄 Description

Summary

Refs #6426: this handles rejected-input diagnostics; normal primitive context and graph limitation deduplication remain separate work.

An unknown Ghidra symbol copied cumulative launcher output into its input error. Expected input failures now request operation-scoped diagnostics. The client requires a completed authenticated handshake and an owned active output checkpoint before selecting that scope. Target/profile identity, runtime/log coordinates, error meaning and the request's failure_output survive; cumulative stdout/stderr and launch-support output remain owned by the client instead of being repeated in the reply. Startup, process, protocol and other remote failures retain full session diagnostics.

The failure factory and domain error projection share the input-code classifier. CLI and MCP use the same provider implementation. No provider, authority, target execution, mutation or resource limit is added; a new capability scope decision is not applicable.

Validation performed

  • npm run check:fast, formatting and npm run docs:check passed; docs checks included nine README cases.
  • 184 source cases passed across diagnostics, failure construction and provider behavior. The complete client boundary file passed 90 cases; nine process/OOM-retention cases and six address-restoration cases passed. The two changed client cases passed again after test relocation.
  • The request-scope boundary regression fails on the original implementation. It verifies actual authenticated process/socket traffic, retained cumulative output, request-local output, subsequent successful work and cleanup. A startup rejection still carries launcher output. Unknown remote codes retain full diagnostics. The JVM is simulated in these deterministic boundary cases.
  • Real Ghidra 12.1.4, Linux x64, Node 24.22.1: two fresh stdio MCP sessions compared the same cold/warm task. Each phase obtains the 652-function inventory, then runs 21 functions × callers/callees/containing-procedure, two graphs and one unknown-symbol rejection. Forward/backward graphs contain 327/10 nodes. All successful-query Evidence IDs and complete facts agree with the control, including cold/warm fact digest c5479d25b2368b2115e7f9d3eac1bc0aced95287a734dfa2d5a026e4ff5c575e; error category, code, retryability, issues, target/profile identity and remote meaning agree. Cleanup passed.
  • Real CLI rea function /usr/bin/ls rea_unknown_symbol_6426 --provider ghidra --detail complete --json returned the expected invalid_request (exit 1), with request scope, the correct source digest and no launcher streams. Its caller-visible error retained empty-but-complete request output. This verifies the shared CLI path; it is an expected rejection, not an analysis success.

Target SHA-256: cb30d69b24245bf2ecdc9e7f53bbad19159999970b6d82c0c00c7d32d9e37aa4. Baseline a0a301a5c has the same tree as main f69c52a1c; implementation 2b0aa2121. Each session used two CPUs, a 1.5 GiB Ghidra heap, a 3 GiB Node heap and an 8 GiB sampled process-family stop guard. No guard stopped a run.

Measurement Before After
Unknown-symbol RPC reply 13,170 B 2,209 B
Error content text, cl100k_base estimate 3,873 tokens 538 tokens
Whole two-phase task + discovery, content estimate 1,251,052 tokens 1,244,382 tokens
Elapsed / CPU 52.55 s / 91.37 s 33.18 s / 57.82 s
Sampled family RSS peak 1.47 GiB 1.45 GiB

Token estimates include the unchanged 919,115-token raw catalog, all arguments and reply content text, inventory, export and close calls. They are serialization estimates, not model usage; prompts, framing and client rendering were not measured. Counting complete raw RPC envelopes instead gives 1,595,918 → 1,589,088 estimated tokens. One pair with warm OS caches does not establish a time or memory improvement; the demonstrated benefit is smaller input-error context with unchanged facts and provenance. Cold/warm task phases were 43.16/1.42 s before and 27.85/0.77 s after. RSS summed the family every 100 ms, can double-count shared pages and miss short peaks. Other engines/platforms were not verified.

CI and review

CI static checks, build, docs and review passed. Source tests fail only the two inline-Evidence advertisement assertions; shard 5 fails three analysis-view advertisement assertions and the Godot full-profile field assertion. These are the existing main schema-fitting regression documented in #7135. This change does not modify tool contracts or schema presentation; before/after catalogs are byte-identical. Other shards were cancelled by fail-fast, so a complete CI suite pass is not claimed. The requester authorizes bypass for existing main failures; human readiness declarations remain unchecked.

Author readiness

Human owner: @gensokyo-dev

  • 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.

🔄 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/7140 **Author:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Created:** 10/25/2026 **Status:** ✅ Merged **Merged:** 10/25/2026 **Merged by:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Base:** `main` ← **Head:** `fix/6426-ghidra-input-diagnostics` --- ### 📝 Commits (1) - [`2b0aa21`](https://github.com/morluto/rea/commit/2b0aa212156fe125c60aea8fd73092329e25dd09) fix(ghidra): scope rejected-input diagnostics to their request ### 📊 Changes **10 files changed** (+260 additions, -52 deletions) <details> <summary>View changed files</summary> 📝 `docs/ghidra-provider.md` (+11 -0) 📝 `src/ghidra/GhidraClient.ts` (+17 -3) 📝 `src/ghidra/GhidraDiagnostics.test.ts` (+58 -0) 📝 `src/ghidra/GhidraDiagnostics.ts` (+25 -9) 📝 `src/ghidra/GhidraProviderClient.ts` (+2 -6) 📝 `src/ghidra/GhidraSessionError.test.ts` (+33 -0) 📝 `src/ghidra/GhidraSessionError.ts` (+16 -3) 📝 `tests/boundary/providers/ghidra/ghidraClient.fixture.ts` (+1 -0) 📝 `tests/boundary/providers/ghidra/ghidraClient.test.ts` (+65 -22) 📝 `tests/fixtures/fakeGhidra.mjs` (+32 -9) </details> ### 📄 Description ## Summary Refs #6426: this handles rejected-input diagnostics; normal primitive context and graph limitation deduplication remain separate work. An unknown Ghidra symbol copied cumulative launcher output into its input error. Expected input failures now request operation-scoped diagnostics. The client requires a completed authenticated handshake and an owned active output checkpoint before selecting that scope. Target/profile identity, runtime/log coordinates, error meaning and the request's `failure_output` survive; cumulative stdout/stderr and launch-support output remain owned by the client instead of being repeated in the reply. Startup, process, protocol and other remote failures retain full session diagnostics. The failure factory and domain error projection share the input-code classifier. CLI and MCP use the same provider implementation. No provider, authority, target execution, mutation or resource limit is added; a new capability scope decision is not applicable. ## Validation performed - `npm run check:fast`, formatting and `npm run docs:check` passed; docs checks included nine README cases. - 184 source cases passed across diagnostics, failure construction and provider behavior. The complete client boundary file passed 90 cases; nine process/OOM-retention cases and six address-restoration cases passed. The two changed client cases passed again after test relocation. - The request-scope boundary regression fails on the original implementation. It verifies actual authenticated process/socket traffic, retained cumulative output, request-local output, subsequent successful work and cleanup. A startup rejection still carries launcher output. Unknown remote codes retain full diagnostics. The JVM is simulated in these deterministic boundary cases. - Real Ghidra 12.1.4, Linux x64, Node 24.22.1: two fresh stdio MCP sessions compared the same cold/warm task. Each phase obtains the 652-function inventory, then runs 21 functions × callers/callees/containing-procedure, two graphs and one unknown-symbol rejection. Forward/backward graphs contain 327/10 nodes. All successful-query Evidence IDs and complete facts agree with the control, including cold/warm fact digest `c5479d25b2368b2115e7f9d3eac1bc0aced95287a734dfa2d5a026e4ff5c575e`; error category, code, retryability, issues, target/profile identity and remote meaning agree. Cleanup passed. - Real CLI `rea function /usr/bin/ls rea_unknown_symbol_6426 --provider ghidra --detail complete --json` returned the expected `invalid_request` (exit 1), with request scope, the correct source digest and no launcher streams. Its caller-visible error retained empty-but-complete request output. This verifies the shared CLI path; it is an expected rejection, not an analysis success. Target SHA-256: `cb30d69b24245bf2ecdc9e7f53bbad19159999970b6d82c0c00c7d32d9e37aa4`. Baseline `a0a301a5c` has the same tree as main `f69c52a1c`; implementation `2b0aa2121`. Each session used two CPUs, a 1.5 GiB Ghidra heap, a 3 GiB Node heap and an 8 GiB sampled process-family stop guard. No guard stopped a run. | Measurement | Before | After | | --- | ---: | ---: | | Unknown-symbol RPC reply | 13,170 B | 2,209 B | | Error content text, cl100k_base estimate | 3,873 tokens | 538 tokens | | Whole two-phase task + discovery, content estimate | 1,251,052 tokens | 1,244,382 tokens | | Elapsed / CPU | 52.55 s / 91.37 s | 33.18 s / 57.82 s | | Sampled family RSS peak | 1.47 GiB | 1.45 GiB | Token estimates include the unchanged 919,115-token raw catalog, all arguments and reply content text, inventory, export and close calls. They are serialization estimates, not model usage; prompts, framing and client rendering were not measured. Counting complete raw RPC envelopes instead gives 1,595,918 → 1,589,088 estimated tokens. One pair with warm OS caches does not establish a time or memory improvement; the demonstrated benefit is smaller input-error context with unchanged facts and provenance. Cold/warm task phases were 43.16/1.42 s before and 27.85/0.77 s after. RSS summed the family every 100 ms, can double-count shared pages and miss short peaks. Other engines/platforms were not verified. ## CI and review CI static checks, build, docs and review passed. Source tests fail only the two inline-Evidence advertisement assertions; shard 5 fails three analysis-view advertisement assertions and the Godot full-profile field assertion. These are the existing main schema-fitting regression documented in #7135. This change does not modify tool contracts or schema presentation; before/after catalogs are byte-identical. Other shards were cancelled by fail-fast, so a complete CI suite pass is not claimed. The requester authorizes bypass for existing main failures; human readiness declarations remain unchecked. ## Author readiness Human owner: @gensokyo-dev - [ ] 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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-11-03 13:39:31 +00:00
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#7668
No description provided.