[PR #4774] [CLOSED] fix(ghidra): recognize OutOfMemoryError beyond the diagnostic budget #6404

Closed
opened 2026-10-31 01:14:12 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4774
Author: @sx4im
Created: 10/17/2026
Status: ❌ Closed

Base: main ← Head: fix/4682-ghidra-oom-beyond-diagnostic-budget


📝 Commits (5)

  • 6edf099 fix(ghidra): report unconfirmed memory cause when OOME output is truncated
  • 037900f fix(ghidra): detect OutOfMemoryError beyond the diagnostic budget
  • 7bef62f docs(ghidra): describe OOME observation beyond the retention budget
  • 31935e1 Merge pull request #3 from sx4im/reason/4682-oome-marker-scan
  • 1825647 Merge upstream main into fix/4682-ghidra-oom-beyond-diagnostic-budget

📊 Changes

11 files changed (+658 additions, -17 deletions)

View changed files

📝 docs/binary-diagnostics.md (+6 -1)
📝 src/ghidra/GhidraClient.ts (+6 -10)
📝 src/ghidra/GhidraDiagnostics.ts (+43 -1)
📝 src/ghidra/GhidraProvider.behavior.test.ts (+39 -0)
📝 src/ghidra/GhidraProviderClient.ts (+19 -2)
➕ src/process/ProcessOutputMarkerScanner.test.ts (+158 -0)
➕ src/process/ProcessOutputMarkerScanner.ts (+127 -0)
📝 src/process/ProviderProcess.ts (+60 -2)
📝 tests/boundary/providers/ghidra/ghidraClient.fixture.ts (+5 -0)
➕ tests/boundary/providers/ghidra/ghidraOomRetention.test.ts (+159 -0)
📝 tests/fixtures/fakeGhidra.mjs (+36 -1)

📄 Description

Fixes #4682

Summary

Ghidra output is kept only up to an 8 MiB budget, so an OutOfMemoryError printed after that was lost and reported as execution_failure ("run rea doctor"). It is now reported as resource_constraint (memory) with the Java heap detail and configured_jvm_max_heap, however much output came first.

Problem and expected behavior

Trigger: >8 MiB of earlier output, then a failing operation prints java.lang.OutOfMemoryError: Java heap space. Expected: the same result as without the earlier output.

Change and scope

  • ProviderProcessSupervisor takes an outputMarker and checks every chunk before the budget applies (new ProcessOutputMarkerScanner). It keeps a small tail per stream so a marker split across chunks is found, and stores only the latest match (offset + line, max 1 KiB each side).
  • snapshotOutputSince returns a match only if its offset is inside the checkpoint range, so an OOME from an earlier operation or before resetOutput is ignored.
  • GhidraClient sets the marker to java.lang.OutOfMemoryError and adds matches as failure_output.observed_oome (redacted). projectObservedJvmMemoryFailure reads them through the existing OOME path.
  • Removes the earlier complete === false guess, which turned cancelled drains, earlier budget use and startup timeouts into memory errors. Incomplete output with no OOME keeps the original code.
    Scope: ordinary fix within existing boundaries; no maintainer decision needed.

Contract and boundary impact

  • Owner: src/process scans output; Ghidra projection reads it.
  • CLI/MCP: additive failure_output.observed_oome; codes change only when an OOME was actually printed.
  • Provider/platform: Ghidra only; other providers don't set a marker.
  • Evidence: line copy is bounded, redacted, keeps its stream and byte offset.
  • Generated metadata/package: 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: new tests/boundary/providers/ghidra/ghidraOomRetention.test.ts (real provider/client/capture/projection, fake Ghidra modes oom_exit_tools, flood_then_oom_exit_tools, flood_then_exit_tools, flood_then_malformed, flood_silent); src/process/ProcessOutputMarkerScanner.test.ts (after budget, split marker, earlier checkpoint, reset, limits); updated GhidraProvider.behavior.test.ts. The existing cancelled-drain protocol test still passes.

  • Base: the new test fails on e5ee3ac and upstream main 5d6029e (only the >8 MiB case: execution_failure), and passes here. Thanks to @RishTheFish21, who offered this regression in #4682; the fake-Ghidra modes follow it.

  • Output (>8 MiB case): resource_constraint, resource: memory, reported_limits.configured_jvm_max_heap: "4G", message "...reports java.lang.OutOfMemoryError: Java heap space...", failure_output.complete: false, stdout 11014752 bytes / 8388608 retained, one observed_oome entry from stderr.

  • Gaps: real Ghidra not run (the JVM is simulated); Windows not run.

  • Observed, derived, and inferred claims remain distinguishable.

  • Artifact identity, source provenance, and failed attempts remain preserved.

  • Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

  • npx vitest run src/ghidra src/process tests/boundary/providers/ghidra (NODE_OPTIONS=--no-warnings) — 67 files, 1134 tests pass (7 skipped). Without the flag, 7 tests fail only because the sandbox proxy makes Node print a warning into captured child output; the same 7 fail on unchanged e5ee3ac
  • New regression ghidraOomRetention.test.ts — fails on e5ee3ac and upstream main 5d6029e (>8 MiB case reports execution_failure), passes here
  • npm run lint — exit 0; npm run format:check — clean on the branch; npm run docs:check — pass
  • npx tsc --noEmit — pass
  • Scanner: 512 MiB in 64 KiB chunks in 115 ms, constant memory
  • CI on 31935e1: test shards 1–5, Portable tests, Source tests, Build package, Docs Generation pass. Static checks fail on tests/boundary/cli/keyedArchivePrototypeKeyCli.test.ts (from main; upstream main 7aa4b98 fails the same check), not on files in this PR

Compatibility, safety, and release

  • Breaking changes: none (additive field).
  • Real Ghidra/OS coverage: Linux, simulated Ghidra only.
  • Package metadata: none.
  • Security: OOME lines are redacted like other output.

Author readiness

Human owner: @sx4im

  • 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/4774 **Author:** [@sx4im](https://github.com/sx4im) **Created:** 10/17/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/4682-ghidra-oom-beyond-diagnostic-budget` --- ### 📝 Commits (5) - [`6edf099`](https://github.com/morluto/rea/commit/6edf0996ba92ec4f31201aecd633bf7123e916fa) fix(ghidra): report unconfirmed memory cause when OOME output is truncated - [`037900f`](https://github.com/morluto/rea/commit/037900fbba8a541484c5a7f63cc26dc4e86c23a7) fix(ghidra): detect OutOfMemoryError beyond the diagnostic budget - [`7bef62f`](https://github.com/morluto/rea/commit/7bef62f2b70eac5e46e2178cdada9b1ea716c0be) docs(ghidra): describe OOME observation beyond the retention budget - [`31935e1`](https://github.com/morluto/rea/commit/31935e13007ff8f6cedda814111a4084a1e6fec9) Merge pull request #3 from sx4im/reason/4682-oome-marker-scan - [`1825647`](https://github.com/morluto/rea/commit/18256474e149093085b3dc25719770da844605f9) Merge upstream main into fix/4682-ghidra-oom-beyond-diagnostic-budget ### 📊 Changes **11 files changed** (+658 additions, -17 deletions) <details> <summary>View changed files</summary> 📝 `docs/binary-diagnostics.md` (+6 -1) 📝 `src/ghidra/GhidraClient.ts` (+6 -10) 📝 `src/ghidra/GhidraDiagnostics.ts` (+43 -1) 📝 `src/ghidra/GhidraProvider.behavior.test.ts` (+39 -0) 📝 `src/ghidra/GhidraProviderClient.ts` (+19 -2) ➕ `src/process/ProcessOutputMarkerScanner.test.ts` (+158 -0) ➕ `src/process/ProcessOutputMarkerScanner.ts` (+127 -0) 📝 `src/process/ProviderProcess.ts` (+60 -2) 📝 `tests/boundary/providers/ghidra/ghidraClient.fixture.ts` (+5 -0) ➕ `tests/boundary/providers/ghidra/ghidraOomRetention.test.ts` (+159 -0) 📝 `tests/fixtures/fakeGhidra.mjs` (+36 -1) </details> ### 📄 Description Fixes #4682 ## Summary Ghidra output is kept only up to an 8 MiB budget, so an OutOfMemoryError printed after that was lost and reported as `execution_failure` ("run `rea doctor`"). It is now reported as `resource_constraint` (memory) with the Java heap detail and `configured_jvm_max_heap`, however much output came first. ## Problem and expected behavior Trigger: >8 MiB of earlier output, then a failing operation prints `java.lang.OutOfMemoryError: Java heap space`. Expected: the same result as without the earlier output. ## Change and scope - `ProviderProcessSupervisor` takes an `outputMarker` and checks every chunk before the budget applies (new `ProcessOutputMarkerScanner`). It keeps a small tail per stream so a marker split across chunks is found, and stores only the latest match (offset + line, max 1 KiB each side). - `snapshotOutputSince` returns a match only if its offset is inside the checkpoint range, so an OOME from an earlier operation or before `resetOutput` is ignored. - `GhidraClient` sets the marker to `java.lang.OutOfMemoryError` and adds matches as `failure_output.observed_oome` (redacted). `projectObservedJvmMemoryFailure` reads them through the existing OOME path. - Removes the earlier `complete === false` guess, which turned cancelled drains, earlier budget use and startup timeouts into memory errors. Incomplete output with no OOME keeps the original code. Scope: ordinary fix within existing boundaries; no maintainer decision needed. ## Contract and boundary impact - Owner: `src/process` scans output; Ghidra projection reads it. - CLI/MCP: additive `failure_output.observed_oome`; codes change only when an OOME was actually printed. - Provider/platform: Ghidra only; other providers don't set a marker. - Evidence: line copy is bounded, redacted, keeps its stream and byte offset. - Generated metadata/package: 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: new `tests/boundary/providers/ghidra/ghidraOomRetention.test.ts` (real provider/client/capture/projection, fake Ghidra modes `oom_exit_tools`, `flood_then_oom_exit_tools`, `flood_then_exit_tools`, `flood_then_malformed`, `flood_silent`); `src/process/ProcessOutputMarkerScanner.test.ts` (after budget, split marker, earlier checkpoint, reset, limits); updated `GhidraProvider.behavior.test.ts`. The existing cancelled-drain protocol test still passes. - Base: the new test fails on e5ee3ac and upstream main 5d6029e (only the >8 MiB case: `execution_failure`), and passes here. Thanks to @RishTheFish21, who offered this regression in #4682; the fake-Ghidra modes follow it. - Output (>8 MiB case): `resource_constraint`, `resource: memory`, `reported_limits.configured_jvm_max_heap: "4G"`, message "...reports java.lang.OutOfMemoryError: Java heap space...", `failure_output.complete: false`, stdout 11014752 bytes / 8388608 retained, one `observed_oome` entry from stderr. - Gaps: real Ghidra not run (the JVM is simulated); Windows not run. - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `npx vitest run src/ghidra src/process tests/boundary/providers/ghidra` (NODE_OPTIONS=--no-warnings) — 67 files, 1134 tests pass (7 skipped). Without the flag, 7 tests fail only because the sandbox proxy makes Node print a warning into captured child output; the same 7 fail on unchanged e5ee3ac - New regression `ghidraOomRetention.test.ts` — fails on e5ee3ac and upstream main 5d6029e (>8 MiB case reports `execution_failure`), passes here - `npm run lint` — exit 0; `npm run format:check` — clean on the branch; `npm run docs:check` — pass - `npx tsc --noEmit` — pass - Scanner: 512 MiB in 64 KiB chunks in 115 ms, constant memory - CI on 31935e1: test shards 1–5, Portable tests, Source tests, Build package, Docs Generation pass. Static checks fail on `tests/boundary/cli/keyedArchivePrototypeKeyCli.test.ts` (from main; upstream main 7aa4b98 fails the same check), not on files in this PR ## Compatibility, safety, and release - Breaking changes: none (additive field). - Real Ghidra/OS coverage: Linux, simulated Ghidra only. - Package metadata: none. - Security: OOME lines are redacted like other output. ## Author readiness Human owner: @sx4im - [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>
bsakaguchi 2026-10-31 01:14:12 +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#6404
No description provided.