morluto/rea

Fork 0

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

reset) #1170

New issue

Closed

opened 2026-10-15 18:31:42 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-15 18:31:42 +00:00

Owner

Originally created by @trigger2k20 on GitHub (Oct 14, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/3269

[Bug]: Hopper provider - killing/crashing Hopper permanently wedges the session — cleanup_incomplete is unrecoverable and close_binary has no force/reset

Environment

rea-agents 6.4.0 (npm, latest publish, 2026-10-11) — MCP server (npx -y rea-agents@6.4.0 mcp)
Hopper Disassembler 6.6.0 (macOS native launch, launchMode: "native")
macOS 27.0.1, Apple Silicon (arm64)

Summary

On macOS the Hopper provider runs with providerLifetime: "external-application" and shutdownMode: "bridge-request", so cleanup can only be confirmed by an authenticated shutdown acknowledgement over the bridge socket. If the Hopper process dies before that ack (kill, crash, force-quit, or a kill during the startup-timeout teardown from the related issue), shutdownConfirmed can never become true. BinarySession then stores the client as #pendingCleanup and every subsequent open_binary / close_binary re-runs the same doomed cleanup and fails with cleanup_incomplete. There is no force/reset path; the only recovery is restarting the entire MCP server process.

Evidence (6.4.0 dist)

macOS native launch takes the external-application branch:

dist/hopper/BridgeLauncher.js:121–125 — providerLifetime: "external-application", shutdownMode: "bridge-request" (the vendor launcher exits immediately; Hopper itself is the external app, so ownsProcessLifetime is false).

Shutdown confirmation then has exactly one source, and it requires a live socket:

dist/hopper/HopperCleanup.js:57–96 (requestShutdown) — if the socket is destroyed, the shutdown request fails and shutdownConfirmed stays false.
dist/hopper/HopperCleanup.js:146–177 (stopProcess) — sets shutdownConfirmed = true only when providerLifetime === "launcher-process" (line 163). For external-application it sets state.processStopped = true but leaves shutdownConfirmed false.
dist/hopper/HopperCleanup.js:178–195 (recordUnconfirmedDocument) — the "verified provider lifetime ended ⇒ document is closed" fallback (lines 179–184) is gated on providerLifetime === "launcher-process", so it does not fire for the external-application case. It then unconditionally records hopper-document / "authenticated shutdown acknowledgement was not observed".
dist/hopper/HopperCleanup.js:41–54 — the lease is released only when state.processStopped && input.resources.shutdownConfirmed; with shutdownConfirmed false the lease and /tmp/rea-XXXX runtime are retained forever.

The wedge is then permanent at the session layer:

dist/application/binary/BinarySession.js:257–272 (#retryPendingCleanup) — clears #pendingCleanup only when closeAnalysisClient returns ok. Since the socket is dead and the process is gone, the retry can never succeed, so #pendingCleanup is never cleared and every later open_binary (line 145) / close_binary (line 236/255) returns the same error.

close_binary exposes only snapshot_path / overwrite — no force, no reset, no way to drop a dead session.

Reproduction

open_binary a small Mach-O with provider_id: "hopper"; confirm a query works (e.g. list_documents returns the .hop).
Kill the Hopper process out from under the session: pkill -f "Hopper Disassembler".
Call close_binary → cleanup_incomplete, resources ["hopper-document", "/tmp/rea-XXXX"].
Call open_binary on any target (even a different, tiny binary) → same cleanup_incomplete, referencing the stale /tmp/rea-XXXX.
Repeat forever. Deleting /tmp/rea-* and killing Hopper does not help — the block is in-memory #pendingCleanup. Only restarting the rea-agents mcp process clears it.

Actual vs. expected

Actual: one Hopper death (crash, or a kill during the startup-timeout teardown) poisons the whole MCP server session for every subsequent target, with no user-recoverable action short of a server restart.
Expected: once the Hopper process is verifiably gone, the document is definitionally closed; cleanup should succeed and the session should accept a new target.

Suggested fix

In recordUnconfirmedDocument / stopProcess, extend the "verified provider lifetime ended ⇒ document closed" fallback to providerLifetime === "external-application" when the process is confirmed stopped (not just launcher-process). A dead, unowned Hopper process cannot hold the document.
Add an explicit escape hatch: close_binary({ force: true }) (or a reset_session tool) that drops #pendingCleanup after confirming the Hopper process is no longer running, releasing the lease and runtime root.
When #retryPendingCleanup fails because the process is already gone, prefer treating it as cleaned rather than re-adding the same unconfirmed resources.

Notes

This compounds with the hardcoded 120 s startup deadline (separate issue): the recommended workaround for large binaries — pre-analyze to a .hop, then reopen the database — requires killing the timed-out Hopper, which immediately triggers this wedge and blocks the workaround.

Originally created by @trigger2k20 on GitHub (Oct 14, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/3269 [Bug]: Hopper provider - killing/crashing Hopper permanently wedges the session — `cleanup_incomplete` is unrecoverable and `close_binary` has no force/reset **Environment** - rea-agents **6.4.0** (npm, latest publish, 2026-10-11) — MCP server (`npx -y rea-agents@6.4.0 mcp`) - Hopper Disassembler **6.6.0** (macOS native launch, `launchMode: "native"`) - macOS 27.0.1, Apple Silicon (arm64) ## Summary On macOS the Hopper provider runs with `providerLifetime: "external-application"` and `shutdownMode: "bridge-request"`, so cleanup can **only** be confirmed by an authenticated shutdown acknowledgement over the bridge socket. If the Hopper process dies before that ack (kill, crash, force-quit, or a `kill` during the startup-timeout teardown from the related issue), `shutdownConfirmed` can never become true. `BinarySession` then stores the client as `#pendingCleanup` and **every** subsequent `open_binary` / `close_binary` re-runs the same doomed cleanup and fails with `cleanup_incomplete`. There is no force/reset path; the only recovery is restarting the entire MCP server process. ## Evidence (6.4.0 dist) macOS native launch takes the external-application branch: - `dist/hopper/BridgeLauncher.js:121–125` — `providerLifetime: "external-application"`, `shutdownMode: "bridge-request"` (the vendor launcher exits immediately; Hopper itself is the external app, so `ownsProcessLifetime` is false). Shutdown confirmation then has exactly one source, and it requires a live socket: - `dist/hopper/HopperCleanup.js:57–96` (`requestShutdown`) — if the socket is destroyed, the shutdown request fails and `shutdownConfirmed` stays false. - `dist/hopper/HopperCleanup.js:146–177` (`stopProcess`) — sets `shutdownConfirmed = true` **only** when `providerLifetime === "launcher-process"` (line 163). For `external-application` it sets `state.processStopped = true` but leaves `shutdownConfirmed` false. - `dist/hopper/HopperCleanup.js:178–195` (`recordUnconfirmedDocument`) — the "verified provider lifetime ended ⇒ document is closed" fallback (lines 179–184) is gated on `providerLifetime === "launcher-process"`, so it does **not** fire for the external-application case. It then unconditionally records `hopper-document` / "authenticated shutdown acknowledgement was not observed". - `dist/hopper/HopperCleanup.js:41–54` — the lease is released only when `state.processStopped && input.resources.shutdownConfirmed`; with `shutdownConfirmed` false the lease and `/tmp/rea-XXXX` runtime are retained forever. The wedge is then permanent at the session layer: - `dist/application/binary/BinarySession.js:257–272` (`#retryPendingCleanup`) — clears `#pendingCleanup` only when `closeAnalysisClient` returns `ok`. Since the socket is dead and the process is gone, the retry can never succeed, so `#pendingCleanup` is never cleared and every later `open_binary` (line 145) / `close_binary` (line 236/255) returns the same error. `close_binary` exposes only `snapshot_path` / `overwrite` — no `force`, no `reset`, no way to drop a dead session. ## Reproduction 1. `open_binary` a small Mach-O with `provider_id: "hopper"`; confirm a query works (e.g. `list_documents` returns the `.hop`). 2. Kill the Hopper process out from under the session: `pkill -f "Hopper Disassembler"`. 3. Call `close_binary` → `cleanup_incomplete`, resources `["hopper-document", "/tmp/rea-XXXX"]`. 4. Call `open_binary` on **any** target (even a different, tiny binary) → same `cleanup_incomplete`, referencing the stale `/tmp/rea-XXXX`. 5. Repeat forever. Deleting `/tmp/rea-*` and killing Hopper does not help — the block is in-memory `#pendingCleanup`. Only restarting the `rea-agents mcp` process clears it. ## Actual vs. expected - **Actual:** one Hopper death (crash, or a `kill` during the startup-timeout teardown) poisons the whole MCP server session for every subsequent target, with no user-recoverable action short of a server restart. - **Expected:** once the Hopper process is verifiably gone, the document is definitionally closed; cleanup should succeed and the session should accept a new target. ## Suggested fix 1. In `recordUnconfirmedDocument` / `stopProcess`, extend the "verified provider lifetime ended ⇒ document closed" fallback to `providerLifetime === "external-application"` when the process is confirmed stopped (not just `launcher-process`). A dead, unowned Hopper process cannot hold the document. 2. Add an explicit escape hatch: `close_binary({ force: true })` (or a `reset_session` tool) that drops `#pendingCleanup` after confirming the Hopper process is no longer running, releasing the lease and runtime root. 3. When `#retryPendingCleanup` fails because the process is already gone, prefer treating it as cleaned rather than re-adding the same unconfirmed resources. ## Notes This compounds with the hardcoded 120 s startup deadline (separate issue): the recommended workaround for large binaries — pre-analyze to a `.hop`, then reopen the database — requires killing the timed-out Hopper, which immediately triggers this wedge and blocks the workaround.

bsakaguchi

2026-10-15 18:31:42 +00:00

closed this issue
added the
bug
label

bsakaguchi commented

2026-10-15 18:31:43 +00:00

Author

Owner

@trigger2k20 commented on GitHub (Oct 14, 2026):

Duplicate of #2975. I have consolidated the additional reproduction context, the relationship to #3268, and a correction to the proposed cleanup condition there. Please track the recovery fix in #2975.

@trigger2k20 commented on GitHub (Oct 14, 2026): Duplicate of #2975. I have consolidated the additional reproduction context, the relationship to #3268, and a correction to the proposed cleanup condition there. Please track the recovery fix in #2975.

bsakaguchi referenced this issue

2026-10-17 21:09:19 +00:00

[PR #1195] [MERGED] fix(ghidra): resolve secondary symbols at function entries #2708

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#1170

No description provided.

Rows
Columns