morluto/rea

Fork 0

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

[GH-ISSUE #2076] [Bug] Prompt argument completion cold-starts Ghidra/Hopper and cannot be cancelled #686

New issue

Closed

opened 2026-10-15 18:27:23 +00:00 by bsakaguchi · 5 comments

bsakaguchi commented

2026-10-15 18:27:23 +00:00

Owner

Originally created by @Ghost-Kang on GitHub (Oct 12, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/2076

Before submitting

I searched the open and closed issues for a related report. (Related but distinct: #1719, #1720, #1065. Completion was added in #102 / #104.)
I removed credentials, tokens, private paths, and sensitive target data.

Area

MCP or agent integration

Observed behavior

On a session server, a completion/complete request for a guided prompt's document or procedure argument, sent after open_binary, cold-starts the deep provider (Ghidra or Hopper). Clients that complete as the user types will trigger this routinely. The startup:

cannot be cancelled: the client's cancel never reaches REA, and the startup runs to completion;
reports no progress, and the completion request blocks for the whole startup (default deadline 330 s for Ghidra, 120 s for Hopper);
makes a tool call that arrives during it uncancellable as well, because it joins the existing startup promise and its own signal is ignored;
blocks close_binary, target switches and close-with-snapshot until the startup finishes, because the completion's call is drained without a signal.

Reproduction

I demonstrated this without Ghidra. The setup was the production createServer({ kind: "session" }), a BinarySession from the production registry (so the deep binding is a CompositeProvider), an MCP client over InMemoryTransport, and a fake deep provider that mimics GhidraClient.start: a 3 s startup, shared by later callers, which ignores the signals of callers that join it.

Case	Result
Completion with nothing open	`[]`, no provider call (correct)
`open_binary`, then document completion	no child started on open; completion called `list_documents` with no signal and no progress, cold-started the provider, and returned after ~3.0 s
Completion cancelled by the client at 200 ms	the client rejected immediately, but the startup continued to ~3.0 s; a `list_documents` tool call made meanwhile joined it, and its own abort at ~0.5 s could not stop it
Control: tool call alone, aborted at 200 ms	the startup was aborted at ~0.2 s (tool calls are cancellable when they own the startup)
Cancelled completion, then `close_binary`	`close_binary` blocked ~2.8 s until the completion-started startup finished

Not demonstrated: a real Ghidra/Hopper launch. That part is source-derived; see Analysis.

Expected behavior

Argument completion should be best-effort and cheap. It should not start a heavyweight provider as a side effect, block lifecycle operations, or make later tool calls uncancellable. If the provider is not already running, document and procedure completion should return [] (or cached values) quickly.

Impact and scope

Affects MCP clients that use prompt argument completion with Ghidra/Hopper-backed sessions.
Nothing leaks or is corrupted: the started client becomes the session's warm client and is reused.
The cost is unrequested multi-minute provider startups, lost cancellability, and stalled close_binary / target switches.
docs/mcp-prompts.md says target-free completion does not start providers, but that statement covers only the provider argument. The document and procedure rows say nothing either way.

REA version

main fa87dd2aa

Runtime and platform

Node 24.21.0; macOS 26.6.2 arm64; @modelcontextprotocol/server 2.3.1

Target and artifact context

Any target routed to a deep provider (Ghidra or Hopper)

Interface

MCP

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

completion(document) after open_binary: list_documents signal=ABSENT progress=ABSENT -> provider start -> returned after 3004 ms
close_binary after cancelled completion: blocked 2802 ms (drained in-flight completion call)

Analysis or suspected cause

Source-derived, from reading main:

Completion gets the session itself. src/server/createServer.ts sets analysis = source.session and passes it to registerGuidedPrompts, bypassing the admission wrapper that tools use. src/server/promptCompletion.ts then calls analysis.execute("list_documents", {}) and analysis.execute("list_procedures", input) with no options, so there is no signal and no progress.
The first non-health operation starts the provider. BinarySession.execute routes to the CompositeProvider. Its health returns ok without creating a child, which is why open_binary starts nothing (#1719). Any other operation creates the child and runs it, and GhidraClient / HopperClient start() then launches the provider.
The SDK drops the request context for completion. In @modelcontextprotocol/server 2.3.1, the completion/complete handler is registered as async (request), and completable callbacks receive only the argument value and context. A completer therefore cannot observe the request's AbortSignal.
Joiners can't cancel. GhidraClient.start returns the existing #startPromise to later callers without racing it against their signal (Hopper does the same). The completion's call is registered in BinarySession's in-flight set, and #drainCalls waits for it with Promise.allSettled during close and switch.
Caching doesn't help. list_documents is a stateful operation, so it is never cached. list_procedures is cacheable only with a document argument.

Typed outcome or recovery expected

Completion returns [] immediately when the deep provider is not already ready, and never delays close_binary or target transitions.

Additional context

Proposed fix (happy to send a PR if this direction looks right):

Only use warm clients. Add a read-only readiness check:
- for example an optional AnalysisClient.isOperationReady?(operation);
- CompositeProvider answers from its existing child map, and a child that was never created is not ready;
- LazyAnalysisClient is not ready until loaded;
- GhidraClient / HopperClient are ready when their health snapshot is idle or busy.
Expose this as a narrow session method used only by completion. It should return [] when no binary is open, a transition is pending, or the child is not ready, and it should not register in the drain set.
Add a short deadline. Pass AbortSignal.timeout(~2-5 s) into the call, so that completion doesn't wait behind Ghidra's serial per-program queue once the provider is warm.
Tests in tests/boundary/mcp/promptMcp.test.ts, using a lazily started provider behind the production router:
- after open_binary, completion returns [] and never creates the child;
- after a tool call warms the provider, completion returns live values;
- a completion during a startup returns quickly and doesn't block close_binary.
Plus unit tests for the readiness checks.

Open questions:

Is warm-up-on-completion ever wanted? If so, it should probably be an explicit opt-in that reports progress.
Should a tool call that joins an in-progress startup honour its own signal? That affects tool calls generally, not only completion.
#104 described procedure discovery as capped (5,000 addresses, 500 per page). The current completer makes one unbounded list_procedures call. Is that intended?

Originally created by @Ghost-Kang on GitHub (Oct 12, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/2076 ### Before submitting - [x] I searched the open and closed issues for a related report. (Related but distinct: #1719, #1720, #1065. Completion was added in #102 / #104.) - [x] I removed credentials, tokens, private paths, and sensitive target data. ### Area MCP or agent integration ### Observed behavior On a session server, a `completion/complete` request for a guided prompt's `document` or `procedure` argument, sent after `open_binary`, cold-starts the deep provider (Ghidra or Hopper). Clients that complete as the user types will trigger this routinely. The startup: - cannot be cancelled: the client's cancel never reaches REA, and the startup runs to completion; - reports no progress, and the completion request blocks for the whole startup (default deadline 330 s for Ghidra, 120 s for Hopper); - makes a tool call that arrives during it uncancellable as well, because it joins the existing startup promise and its own signal is ignored; - blocks `close_binary`, target switches and close-with-snapshot until the startup finishes, because the completion's call is drained without a signal. ### Reproduction I demonstrated this without Ghidra. The setup was the production `createServer({ kind: "session" })`, a `BinarySession` from the production registry (so the deep binding is a `CompositeProvider`), an MCP client over `InMemoryTransport`, and a fake deep provider that mimics `GhidraClient.start`: a 3 s startup, shared by later callers, which ignores the signals of callers that join it. | Case | Result | |---|---| | Completion with nothing open | `[]`, no provider call (correct) | | `open_binary`, then document completion | no child started on open; completion called `list_documents` with no signal and no progress, cold-started the provider, and returned after ~3.0 s | | Completion cancelled by the client at 200 ms | the client rejected immediately, but the startup continued to ~3.0 s; a `list_documents` tool call made meanwhile joined it, and its own abort at ~0.5 s could not stop it | | Control: tool call alone, aborted at 200 ms | the startup was aborted at ~0.2 s (tool calls are cancellable when they own the startup) | | Cancelled completion, then `close_binary` | `close_binary` blocked ~2.8 s until the completion-started startup finished | Not demonstrated: a real Ghidra/Hopper launch. That part is source-derived; see Analysis. ### Expected behavior Argument completion should be best-effort and cheap. It should not start a heavyweight provider as a side effect, block lifecycle operations, or make later tool calls uncancellable. If the provider is not already running, document and procedure completion should return `[]` (or cached values) quickly. ### Impact and scope - Affects MCP clients that use prompt argument completion with Ghidra/Hopper-backed sessions. - Nothing leaks or is corrupted: the started client becomes the session's warm client and is reused. - The cost is unrequested multi-minute provider startups, lost cancellability, and stalled `close_binary` / target switches. - `docs/mcp-prompts.md` says target-free completion does not start providers, but that statement covers only the `provider` argument. The document and procedure rows say nothing either way. ### REA version main `fa87dd2aa` ### Runtime and platform Node 24.21.0; macOS 26.6.2 arm64; `@modelcontextprotocol/server` 2.3.1 ### Target and artifact context Any target routed to a deep provider (Ghidra or Hopper) ### Interface MCP ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence ```text completion(document) after open_binary: list_documents signal=ABSENT progress=ABSENT -> provider start -> returned after 3004 ms close_binary after cancelled completion: blocked 2802 ms (drained in-flight completion call) ``` ### Analysis or suspected cause Source-derived, from reading `main`: 1. **Completion gets the session itself.** `src/server/createServer.ts` sets `analysis = source.session` and passes it to `registerGuidedPrompts`, bypassing the admission wrapper that tools use. `src/server/promptCompletion.ts` then calls `analysis.execute("list_documents", {})` and `analysis.execute("list_procedures", input)` with no options, so there is no signal and no progress. 2. **The first non-health operation starts the provider.** `BinarySession.execute` routes to the `CompositeProvider`. Its `health` returns ok without creating a child, which is why `open_binary` starts nothing (#1719). Any other operation creates the child and runs it, and `GhidraClient` / `HopperClient` `start()` then launches the provider. 3. **The SDK drops the request context for completion.** In `@modelcontextprotocol/server` 2.3.1, the `completion/complete` handler is registered as `async (request)`, and `completable` callbacks receive only the argument value and context. A completer therefore cannot observe the request's AbortSignal. 4. **Joiners can't cancel.** `GhidraClient.start` returns the existing `#startPromise` to later callers without racing it against their signal (Hopper does the same). The completion's call is registered in `BinarySession`'s in-flight set, and `#drainCalls` waits for it with `Promise.allSettled` during close and switch. 5. **Caching doesn't help.** `list_documents` is a stateful operation, so it is never cached. `list_procedures` is cacheable only with a `document` argument. ### Typed outcome or recovery expected Completion returns `[]` immediately when the deep provider is not already ready, and never delays `close_binary` or target transitions. ### Additional context **Proposed fix (happy to send a PR if this direction looks right):** 1. **Only use warm clients.** Add a read-only readiness check: - for example an optional `AnalysisClient.isOperationReady?(operation)`; - `CompositeProvider` answers from its existing child map, and a child that was never created is not ready; - `LazyAnalysisClient` is not ready until loaded; - `GhidraClient` / `HopperClient` are ready when their health snapshot is idle or busy. Expose this as a narrow session method used only by completion. It should return `[]` when no binary is open, a transition is pending, or the child is not ready, and it should not register in the drain set. 2. **Add a short deadline.** Pass `AbortSignal.timeout(~2-5 s)` into the call, so that completion doesn't wait behind Ghidra's serial per-program queue once the provider is warm. 3. **Tests** in `tests/boundary/mcp/promptMcp.test.ts`, using a lazily started provider behind the production router: - after `open_binary`, completion returns `[]` and never creates the child; - after a tool call warms the provider, completion returns live values; - a completion during a startup returns quickly and doesn't block `close_binary`. Plus unit tests for the readiness checks. **Open questions:** - Is warm-up-on-completion ever wanted? If so, it should probably be an explicit opt-in that reports progress. - Should a tool call that joins an in-progress startup honour its own signal? That affects tool calls generally, not only completion. - #104 described procedure discovery as capped (5,000 addresses, 500 per page). The current completer makes one unbounded `list_procedures` call. Is that intended?

bsakaguchi

2026-10-15 18:27:23 +00:00

closed this issue
added the
area: cli

bug

P2: medium
labels

bsakaguchi commented

2026-10-15 18:27:23 +00:00

Author

Owner

@matbrgz commented on GitHub (Oct 12, 2026):

Root Cause & Performance Review

Triggering a cold-start of heavy deep analysis providers (Ghidra/Hopper with 120s–330s deadlines) during fast-path MCP argument completion (completion/complete) severely degrades interactive client responsiveness:

Fast-Path Invariant: MCP prompt completions are designed to complete in milliseconds. Under no circumstances should completion/complete trigger deferred provider initialization.
Fix: If the provider client is not yet active (isClientRunning === false), autocomplete handlers should complete solely from statically available metadata (e.g. basic target information) or return an empty suggestions list, leaving heavy initialization strictly to explicit analysis tool calls (decompile, list_procedures).
Cancellation: Furthermore, provider startup promises must plumb through AbortSignal so client cancellation terminates the spawned headless worker rather than locking the session.

@matbrgz commented on GitHub (Oct 12, 2026): ### Root Cause & Performance Review Triggering a cold-start of heavy deep analysis providers (Ghidra/Hopper with 120s–330s deadlines) during fast-path MCP argument completion (`completion/complete`) severely degrades interactive client responsiveness: - **Fast-Path Invariant:** MCP prompt completions are designed to complete in milliseconds. Under no circumstances should `completion/complete` trigger deferred provider initialization. - **Fix:** If the provider client is not yet active (`isClientRunning === false`), autocomplete handlers should complete solely from statically available metadata (e.g. basic target information) or return an empty suggestions list, leaving heavy initialization strictly to explicit analysis tool calls (`decompile`, `list_procedures`). - **Cancellation:** Furthermore, provider startup promises must plumb through `AbortSignal` so client cancellation terminates the spawned headless worker rather than locking the session.

bsakaguchi commented

2026-10-15 18:27:24 +00:00

Author

Owner

@maxiaochaos commented on GitHub (Oct 13, 2026):

I'd like to take this if nobody is actively working on it.

Plan (aligned with the issue analysis):

Gate document/procedure prompt completion so it only runs against an already-warm inventory provider — return [] after open_binary until a real tool call has started the deep client (no cold-start of Ghidra/Hopper from completion/complete).
Pass a short AbortSignal.timeout (~2–5s) on the warm completion path so it cannot block behind the provider queue or stall close_binary.
Add boundary tests in tests/boundary/mcp/promptMcp.test.ts (and unit coverage for the readiness check): cold after open → [] + no child created; warm after a tool call → live values; completion during startup returns quickly and does not block close.

Note: closed PR #2086 looks empty (no commits); I'll rebase on current main and open a focused PR. Happy to adjust if maintainers prefer a different readiness API shape.

Approx timeline: PR within a few days. Please assign me or say if someone else already has it.

@maxiaochaos commented on GitHub (Oct 13, 2026): I'd like to take this if nobody is actively working on it. Plan (aligned with the issue analysis): 1. Gate document/procedure prompt completion so it only runs against an already-warm inventory provider — return `[]` after `open_binary` until a real tool call has started the deep client (no cold-start of Ghidra/Hopper from `completion/complete`). 2. Pass a short `AbortSignal.timeout` (~2–5s) on the warm completion path so it cannot block behind the provider queue or stall `close_binary`. 3. Add boundary tests in `tests/boundary/mcp/promptMcp.test.ts` (and unit coverage for the readiness check): cold after open → `[]` + no child created; warm after a tool call → live values; completion during startup returns quickly and does not block close. Note: closed PR #2086 looks empty (no commits); I'll rebase on current main and open a focused PR. Happy to adjust if maintainers prefer a different readiness API shape. Approx timeline: PR within a few days. Please assign me or say if someone else already has it.

bsakaguchi commented

2026-10-15 18:27:24 +00:00

Author

Owner

@CassieFox23 commented on GitHub (Oct 13, 2026):

Looks like main already covers the cold start and the cancel parts of this. Commit 07b36c148 landed on Oct 12. Now a prompt fill-in no longer starts Ghidra or Hopper. It also passes the MCP cancel signal through. The change is in src/server/promptCompletion.ts. Evidence: github.com/morluto/rea@07b36c1489. I only read the diff and did not run it. I did not see the short deadline from the report. A second PR for the same change may not be needed.

🤖 Generated with Claude Code (Sonnet 5.5, verified by Opus 5.5) 🦊

@CassieFox23 commented on GitHub (Oct 13, 2026): Looks like main already covers the cold start and the cancel parts of this. Commit 07b36c148 landed on Oct 12. Now a prompt fill-in no longer starts Ghidra or Hopper. It also passes the MCP cancel signal through. The change is in `src/server/promptCompletion.ts`. Evidence: https://github.com/morluto/rea/commit/07b36c148946f46b99a4ce04e55726e97d0ee03a. I only read the diff and did not run it. I did not see the short deadline from the report. A second PR for the same change may not be needed. 🤖 Generated with Claude Code (Sonnet 5.5, verified by Opus 5.5) 🦊

bsakaguchi commented

2026-10-15 18:27:24 +00:00

Author

Owner

@maxiaochaos commented on GitHub (Oct 13, 2026):

Stepping back — main already covers the cold-start/cancel parts (07b36c148). Closing my draft; happy for anyone else to pick up a short-deadline follow-up if needed.

@maxiaochaos commented on GitHub (Oct 13, 2026): Stepping back — main already covers the cold-start/cancel parts (07b36c148). Closing my draft; happy for anyone else to pick up a short-deadline follow-up if needed.

bsakaguchi commented

2026-10-15 18:27:24 +00:00

Author

Owner

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

Addressed on main in 07b36c148.

Prompt completion remains within already-ready provider sessions and follows cancellation/session transition lifecycle.

Validation: promptMcp 9 passed; package verifier promptCompletionLifecycle=true; global gates pass.

Verification scope: No live provider startup exercise.

@morluto commented on GitHub (Oct 14, 2026): Addressed on main in [07b36c148](https://github.com/morluto/rea/commit/07b36c148). Prompt completion remains within already-ready provider sessions and follows cancellation/session transition lifecycle. Validation: promptMcp 9 passed; package verifier promptCompletionLifecycle=true; global gates pass. Verification scope: No live provider startup exercise.

bsakaguchi referenced this issue

2026-10-16 21:15:09 +00:00

[PR #686] [CLOSED] fix(browser): respect metadata key and overwrite semantics #2167

bsakaguchi referenced this issue

2026-10-16 21:15:24 +00:00

[PR #705] [MERGED] fix: integrate 14 approved fix PRs (668,671,673,675,678-681,685-690) #2185