morluto/rea

Fork 0

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

[PR #5046] fix(mcp): support retained pseudocode summaries and paging #6756

New issue

Open

opened 2026-11-01 21:42:50 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:42:50 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5046
Author: @L13an
Created: 10/18/2026
Status: 🔄 Open

Base: main ← Head: codex/fix-pseudocode-retained-pages

📝 Commits (4)

21933a8 refactor(mcp): share official tool evidence registration
688f24a fix(mcp): support retained pseudocode summaries and paging
20d4357 Merge origin/main into codex/fix-pseudocode-retained-pages
ef3d100 chore: merge main into pseudocode delivery fix

📊 Changes

14 files changed (+682 additions, -5 deletions)

View changed files

📝 .agents/skills/reverse-engineer-anything/references/native-and-artifacts.md (+11 -0)
📝 docs/cli.md (+9 -0)
📝 docs/native-investigation.md (+11 -0)
📝 src/application/analysisView/AnalysisViewService.ts (+9 -0)
📝 src/contracts/analysisViewToolContracts.ts (+1 -1)
📝 src/contracts/officialToolContracts.ts (+3 -2)
📝 src/contracts/toolOutputSchemaGroups.ts (+5 -1)
📝 src/domain/analysisView/analysisView.ts (+5 -0)
📝 src/domain/analysisView/analysisViewResult.ts (+10 -0)
➕ src/domain/analysisView/procedurePseudocodeView.test.ts (+113 -0)
➕ src/domain/analysisView/procedurePseudocodeView.ts (+136 -0)
📝 src/server/toolResult.ts (+2 -1)
📝 tests/boundary/cli/analysisViewCli.test.ts (+44 -0)
➕ tests/boundary/mcp/pseudocodeDeliveryMcp.test.ts (+323 -0)

📄 Description

Summary

Fixes #5032.

Add optional detail: "summary" to procedure_pseudo_code, retaining complete Evidence for exact native/pseudocode pages through inspect_analysis_view. Omitted or complete detail preserves the complete response. Derived summaries and pages retain the authenticated producer locations, including resolved file offsets and addresses.

Problem and expected behavior

Large pseudocode was always returned in full, and retained scalar pseudocode could not be paged. A host may save that result to a file despite it fitting REA's wire budget. Callers should be able to request a small first response and page the original observation without another analysis request.

Change and scope

Add the delivery contracts and scalar pseudocode view owner; reuse existing UTF-16 paging, preserving selectors, provenance, limitations and null/empty results.
Reuse shared Evidence registration for official tools, removing their duplicated pipeline while preserving relationship coverage. Strip detail from provider parameters and source identity; reject unavailable retention before analysis.
Make overflow continuation usable and update CLI/native/Skill guidance.

Contract and boundary impact

Owner: scalar view projection and shared MCP delivery. This is an ordinary existing-workflow fix; a separate maintainer scope decision is not applicable.
MCP adds an optional input and summary result variant. CLI can project saved scalar Evidence; default decompile behavior is unchanged.
Engine/bridge implementations, authority, dependencies and versions are unchanged. Complete Evidence remains retained; views link to it. Authored contracts/docs are updated; generated outputs stay ignored.

Expected effects of the added delivery/projection behavior:

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.

The manual native check exercised the existing owned Ghidra headless startup and ephemeral database. The fixture executable was never run or modified.

Evidence and regression coverage

The first four regressions failed on unchanged base 23f984b1a. Two added MCP cases reproduced lost function locations on 4e84920ca; the shared Evidence writer now carries parent locations for both scalar summary and page views instead of reinterpreting selectors. Committed tests cover exact reconstruction, full retention, stable parent identity, Unicode/CRLF, null/empty output, malformed parents, no repeated provider calls during paging, retention preflight, callable overflow recovery, compiled CLI projection and SDK-converted schemas.

Real-provider verification at 688f24a40: production scripts/rea.mjs mcp over SDK stdio, macOS arm64, Node 24.18.0, JDK 21.0.7 and Ghidra 12.1.4. Its native arm64 decompiler was built from the source shipped in the SHA-256-verified official release; doctor then passed.

Native fixture	Pseudocode UTF-16 units	Complete response JSON bytes	Summary response JSON bytes	Exact pages
Small: `value * 7 + 13`	73	7,135	8,223	5
Large: 1,600 mix calls/volatile writes	102,670	427,123	8,231	6

All pages reconstructed the complete text exactly. Omitted/explicit complete Evidence matched the summary parent ID; subjects and source links were preserved. The pagination phase made only retained-view requests; native internal invocation counts were not instrumented. Real file-offset:0x3c0 and 0x3c0 fallback calls both retained offset 960 and resolved address 0x1000003c0 in every summary/page. Saved parents for both selectors were also projected through the compiled CLI offline with the same locations.

After close: open: false, cleanup_pending: false, successful ping, unchanged artifact digest, and no surviving observed child processes. Sizes describe first responses only; later pages cost additional exchanges, and summary costs more for the small function. No model-token, speed or memory improvement is claimed.

Validation performed

Current head 688f24a40 retains two feature commits on 39275420e, including the resolved-location follow-up for the automated review finding.

Focused source/MCP regressions: 154 passed / 6 files; compiled CLI: 16 passed — 170 total. The new cases failed before the identity fix and cover explicit file offsets, hexadecimal fallback, null output and retained/portable Evidence.
npm run check, npm run docs:check, git diff --check and the normal pre-push npm run check:fast hook — passed. Documentation checks also passed all 5 README consistency tests.
Real Ghidra 12.1.4 over production stdio MCP — passed for direct small/large functions and both file-offset selector forms; exact text, parent identity, source locations, default/explicit complete parity, immutable target digest and owned-process cleanup were checked. Offline CLI projection passed for both saved file-offset observations.
Selectors are preserved as caller input. Resolved positions come from authenticated parent locations; generic address locations do not identify a canonical-entry role, so the scalar view does not guess procedure_address from their order. Other view types retain their existing projection behavior.
Full current-head upstream CI remains pending after this update; the platform/provider limitations below still apply.

Executed focused regression commands

npm run test:focused -- \
  src/ghidra/GhidraFunctionValues.test.ts \
  src/domain/analysisView/procedurePseudocodeView.test.ts \
  src/application/analysisView/AnalysisViewService.test.ts \
  tests/boundary/mcp/pseudocodeDeliveryMcp.test.ts \
  tests/boundary/mcp/analysisViewMcp.test.ts \
  tests/boundary/mcp/toolSchemaValidity.test.ts

npm run test:focused -- tests/boundary/cli/analysisViewCli.test.ts

Native fixture construction and verification scope

Use Node 24.18.0, the existing C compiler and a compatible Ghidra/JDK installation. The exact source used for this check can be generated as follows; compile it for static analysis only:

node --input-type=module <<'NODE'
import { writeFileSync } from 'node:fs';
const lines = [
  'typedef unsigned int u32;',
  'volatile u32 rea_issue5032_sink[64];',
  '__attribute__((noinline,used)) u32 rea_issue5032_small(u32 value) { return value * 7u + 13u; }',
  '__attribute__((noinline,used)) u32 rea_issue5032_mix(u32 value, u32 salt) { return (value * 33u) ^ salt; }',
  '__attribute__((noinline,used)) u32 rea_issue5032_large(u32 value) {'
];
for (let i = 0; i < 1600; i++) {
  const salt = (Math.imul(i + 1, 0x9e3779b1) >>> 0).toString(16);
  lines.push(`  value = rea_issue5032_mix(value, 0x${salt}u);`, `  rea_issue5032_sink[${i % 64}] = value;`);
}
lines.push('  return value;', '}', 'int main(void) { return (int)(rea_issue5032_large(5u) ^ rea_issue5032_small(3u)); }', '');
writeFileSync('fixture.c', lines.join('\n'));
NODE
clang -O0 -g -fno-inline -fno-stack-protector fixture.c -o fixture

Source SHA-256: 4d17f1dd7ac434caca0496e93a33764fd6c2192eb364446e60e72a2dfc871937. Local Mach-O SHA-256: 29597ddfcac805d51ef5c8b7b501116fb39d82868a8680d748152563e4bc50e0; paths/debug information/UUIDs can change another build's bytes. Open the artifact through open_binary with provider_id: "ghidra", select each observed entry, request summary, page to exhaustion, then compare with omitted and explicit complete calls. Save complete Evidence and use inspect-analysis-view with an inline source after closing the session.

The successful four-case native correctness workflow had 37 public tool calls and took 21.009s; wait4 root/reaped-descendant CPU was 25.002s; 20ms sampled summed family RSS peaked at 1396.3 MiB. Shared pages may be counted twice and short peaks missed. This excludes fixture preparation and offline CLI and is not a before/after benchmark; no model-token measurement was performed. The first function call followed inventory/import, not a cold filesystem run.

Earlier failures and corrective attempts

An existing HAR test timed out in an earlier combined regression run; an intervening diagnostic overlapped dist rebuilding. The final serial CLI file passed without changing deadlines/skips/retries; the original timeout cause remains unproven. Native doctor initially lacked the arm64 decompiler; building the shipped component resolved that. The first manual native driver asserted the derived summary provider incorrectly; only the script was corrected, and the complete rerun passed. Both native attempts cleaned up their observed child processes.

Compatibility, safety, and release

No migration or breaking change; summary requires session retention. The native fixture was analyzed but never executed or modified. Original iTunes/Claude Code, real Hopper/IDA, Windows/Linux, full native-provider lanes, upstream CI and real-model investigations remain unverified. Local drivers, binaries, raw logs and generated outputs are excluded from the commits.

Author readiness

Human owner: @L13an

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/5046 **Author:** [@L13an](https://github.com/L13an) **Created:** 10/18/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `codex/fix-pseudocode-retained-pages` --- ### 📝 Commits (4) - [`21933a8`](https://github.com/morluto/rea/commit/21933a853c1c222ad3cb9d18943a35f439a20e71) refactor(mcp): share official tool evidence registration - [`688f24a`](https://github.com/morluto/rea/commit/688f24a409cfe970d2845c9aec4a7a9d74d22759) fix(mcp): support retained pseudocode summaries and paging - [`20d4357`](https://github.com/morluto/rea/commit/20d43579d8fa331e760f917702012926b3dd683b) Merge origin/main into codex/fix-pseudocode-retained-pages - [`ef3d100`](https://github.com/morluto/rea/commit/ef3d1000207c33b1a38fcf42e928f0809988f5de) chore: merge main into pseudocode delivery fix ### 📊 Changes **14 files changed** (+682 additions, -5 deletions) <details> <summary>View changed files</summary> 📝 `.agents/skills/reverse-engineer-anything/references/native-and-artifacts.md` (+11 -0) 📝 `docs/cli.md` (+9 -0) 📝 `docs/native-investigation.md` (+11 -0) 📝 `src/application/analysisView/AnalysisViewService.ts` (+9 -0) 📝 `src/contracts/analysisViewToolContracts.ts` (+1 -1) 📝 `src/contracts/officialToolContracts.ts` (+3 -2) 📝 `src/contracts/toolOutputSchemaGroups.ts` (+5 -1) 📝 `src/domain/analysisView/analysisView.ts` (+5 -0) 📝 `src/domain/analysisView/analysisViewResult.ts` (+10 -0) ➕ `src/domain/analysisView/procedurePseudocodeView.test.ts` (+113 -0) ➕ `src/domain/analysisView/procedurePseudocodeView.ts` (+136 -0) 📝 `src/server/toolResult.ts` (+2 -1) 📝 `tests/boundary/cli/analysisViewCli.test.ts` (+44 -0) ➕ `tests/boundary/mcp/pseudocodeDeliveryMcp.test.ts` (+323 -0) </details> ### 📄 Description ## Summary Fixes #5032. Add optional `detail: "summary"` to `procedure_pseudo_code`, retaining complete Evidence for exact native/pseudocode pages through `inspect_analysis_view`. Omitted or `complete` detail preserves the complete response. Derived summaries and pages retain the authenticated producer locations, including resolved file offsets and addresses. ## Problem and expected behavior Large pseudocode was always returned in full, and retained scalar pseudocode could not be paged. A host may save that result to a file despite it fitting REA's wire budget. Callers should be able to request a small first response and page the original observation without another analysis request. ## Change and scope - Add the delivery contracts and scalar pseudocode view owner; reuse existing UTF-16 paging, preserving selectors, provenance, limitations and null/empty results. - Reuse shared Evidence registration for official tools, removing their duplicated pipeline while preserving relationship coverage. Strip detail from provider parameters and source identity; reject unavailable retention before analysis. - Make overflow continuation usable and update CLI/native/Skill guidance. ## Contract and boundary impact - Owner: scalar view projection and shared MCP delivery. This is an ordinary existing-workflow fix; a separate maintainer scope decision is not applicable. - MCP adds an optional input and summary result variant. CLI can project saved scalar Evidence; default decompile behavior is unchanged. - Engine/bridge implementations, authority, dependencies and versions are unchanged. Complete Evidence remains retained; views link to it. Authored contracts/docs are updated; generated outputs stay ignored. Expected effects of the added delivery/projection behavior: - [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. The manual native check exercised the existing owned Ghidra headless startup and ephemeral database. The fixture executable was never run or modified. ## Evidence and regression coverage The first four regressions failed on unchanged base `23f984b1a`. Two added MCP cases reproduced lost function locations on `4e84920ca`; the shared Evidence writer now carries parent locations for both scalar summary and page views instead of reinterpreting selectors. Committed tests cover exact reconstruction, full retention, stable parent identity, Unicode/CRLF, null/empty output, malformed parents, no repeated provider calls during paging, retention preflight, callable overflow recovery, compiled CLI projection and SDK-converted schemas. **Real-provider verification at `688f24a40`:** production `scripts/rea.mjs mcp` over SDK stdio, macOS arm64, Node 24.18.0, JDK 21.0.7 and Ghidra 12.1.4. Its native arm64 decompiler was built from the source shipped in the SHA-256-verified [official release](https://github.com/NationalSecurityAgency/ghidra/releases/tag/Ghidra_12.1.4_build); doctor then passed. | Native fixture | Pseudocode UTF-16 units | Complete response JSON bytes | Summary response JSON bytes | Exact pages | | --- | ---: | ---: | ---: | ---: | | Small: `value * 7 + 13` | 73 | 7,135 | 8,223 | 5 | | Large: 1,600 mix calls/volatile writes | 102,670 | 427,123 | 8,231 | 6 | All pages reconstructed the complete text exactly. Omitted/explicit complete Evidence matched the summary parent ID; subjects and source links were preserved. The pagination phase made only retained-view requests; native internal invocation counts were not instrumented. Real `file-offset:0x3c0` and `0x3c0` fallback calls both retained offset 960 and resolved address `0x1000003c0` in every summary/page. Saved parents for both selectors were also projected through the compiled CLI offline with the same locations. After close: `open: false`, `cleanup_pending: false`, successful ping, unchanged artifact digest, and no surviving observed child processes. Sizes describe first responses only; later pages cost additional exchanges, and summary costs more for the small function. No model-token, speed or memory improvement is claimed. ## Validation performed Current head `688f24a40` retains two feature commits on `39275420e`, including the resolved-location follow-up for [the automated review finding](https://github.com/morluto/rea/pull/5046#discussion_r4311618666). - Focused source/MCP regressions: **154 passed / 6 files**; compiled CLI: **16 passed** — **170 total**. The new cases failed before the identity fix and cover explicit file offsets, hexadecimal fallback, null output and retained/portable Evidence. - `npm run check`, `npm run docs:check`, `git diff --check` and the normal pre-push `npm run check:fast` hook — passed. Documentation checks also passed all 5 README consistency tests. - Real Ghidra 12.1.4 over production stdio MCP — passed for direct small/large functions and both file-offset selector forms; exact text, parent identity, source locations, default/explicit complete parity, immutable target digest and owned-process cleanup were checked. Offline CLI projection passed for both saved file-offset observations. - Selectors are preserved as caller input. Resolved positions come from authenticated parent `locations`; generic address locations do not identify a canonical-entry role, so the scalar view does not guess `procedure_address` from their order. Other view types retain their existing projection behavior. - Full current-head upstream CI remains pending after this update; the platform/provider limitations below still apply. <details> <summary>Executed focused regression commands</summary> ```sh npm run test:focused -- \ src/ghidra/GhidraFunctionValues.test.ts \ src/domain/analysisView/procedurePseudocodeView.test.ts \ src/application/analysisView/AnalysisViewService.test.ts \ tests/boundary/mcp/pseudocodeDeliveryMcp.test.ts \ tests/boundary/mcp/analysisViewMcp.test.ts \ tests/boundary/mcp/toolSchemaValidity.test.ts npm run test:focused -- tests/boundary/cli/analysisViewCli.test.ts ``` </details> <details> <summary>Native fixture construction and verification scope</summary> Use Node 24.18.0, the existing C compiler and a compatible Ghidra/JDK installation. The exact source used for this check can be generated as follows; compile it for static analysis only: ```sh node --input-type=module <<'NODE' import { writeFileSync } from 'node:fs'; const lines = [ 'typedef unsigned int u32;', 'volatile u32 rea_issue5032_sink[64];', '__attribute__((noinline,used)) u32 rea_issue5032_small(u32 value) { return value * 7u + 13u; }', '__attribute__((noinline,used)) u32 rea_issue5032_mix(u32 value, u32 salt) { return (value * 33u) ^ salt; }', '__attribute__((noinline,used)) u32 rea_issue5032_large(u32 value) {' ]; for (let i = 0; i < 1600; i++) { const salt = (Math.imul(i + 1, 0x9e3779b1) >>> 0).toString(16); lines.push(` value = rea_issue5032_mix(value, 0x${salt}u);`, ` rea_issue5032_sink[${i % 64}] = value;`); } lines.push(' return value;', '}', 'int main(void) { return (int)(rea_issue5032_large(5u) ^ rea_issue5032_small(3u)); }', ''); writeFileSync('fixture.c', lines.join('\n')); NODE clang -O0 -g -fno-inline -fno-stack-protector fixture.c -o fixture ``` Source SHA-256: `4d17f1dd7ac434caca0496e93a33764fd6c2192eb364446e60e72a2dfc871937`. Local Mach-O SHA-256: `29597ddfcac805d51ef5c8b7b501116fb39d82868a8680d748152563e4bc50e0`; paths/debug information/UUIDs can change another build's bytes. Open the artifact through `open_binary` with `provider_id: "ghidra"`, select each observed entry, request summary, page to exhaustion, then compare with omitted and explicit complete calls. Save complete Evidence and use `inspect-analysis-view` with an inline source after closing the session. The successful four-case native correctness workflow had 37 public tool calls and took 21.009s; wait4 root/reaped-descendant CPU was 25.002s; 20ms sampled summed family RSS peaked at 1396.3 MiB. Shared pages may be counted twice and short peaks missed. This excludes fixture preparation and offline CLI and is not a before/after benchmark; no model-token measurement was performed. The first function call followed inventory/import, not a cold filesystem run. </details> <details> <summary>Earlier failures and corrective attempts</summary> An existing HAR test timed out in an earlier combined regression run; an intervening diagnostic overlapped dist rebuilding. The final serial CLI file passed without changing deadlines/skips/retries; the original timeout cause remains unproven. Native doctor initially lacked the arm64 decompiler; building the shipped component resolved that. The first manual native driver asserted the derived summary provider incorrectly; only the script was corrected, and the complete rerun passed. Both native attempts cleaned up their observed child processes. </details> ## Compatibility, safety, and release No migration or breaking change; summary requires session retention. The native fixture was analyzed but never executed or modified. Original iTunes/Claude Code, real Hopper/IDA, Windows/Linux, full native-provider lanes, upstream CI and real-model investigations remain unverified. Local drivers, binaries, raw logs and generated outputs are excluded from the commits. ## Author readiness Human owner: @L13an - [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 - [ ] 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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-11-01 21:42:50 +00:00

bsakaguchi referenced this issue

2026-11-01 21:47:42 +00:00

[PR #6809] [CLOSED] fix(cli): keep own __proto__ members selectable in output filters #7416