morluto/rea

Fork 0

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

[PR #4157] [CLOSED] docs(mcp): describe target recovery after a connection loss #6098

New issue

Closed

opened 2026-10-31 01:11:51 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:11:51 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4157
Author: @berelels
Created: 10/15/2026
Status: ❌ Closed

Base: main ← Head: docs/mcp-connection-loss-recovery

📝 Commits (1)

7fb3aac docs(mcp): describe target recovery after a connection loss

📊 Changes

1 file changed (+31 additions, -0 deletions)

View changed files

📝 docs/mcp-contracts.md (+31 -0)

📄 Description

Summary

Refs #3509 (documentation slice; does not close it). It documents what happens to the open target when an MCP stdio connection fails, how a client can tell a restarted server from a target closed on the same connection, and how to recover. The new section, docs/mcp-contracts.md → "Ghidra first-query deadlines and recovery" → "Connection loss", is based on a real stdio run.

Problem and expected behavior

#3509 reports that after a transport failure binary_session showed open: false with no explanation. The existing recovery text covers close/reopen on the same connection only. Source and the run below show that the target is lost because the stdio server shuts down when its input closes (src/main.ts registers shutdown on stdin end/close). A reconnecting client therefore talks to a new process. In-process, BinarySession clears the active target only on open/close.

Change and scope

Documentation only:

After connection loss: a new process, open: false, a different server_identity.server.started_at, and target_unavailable for target queries. A call in flight has no result.
Recovery: reopen with the same path (cold import again), keep retained records via export_evidence_bundle/import_evidence_bundle, and point idle-timeout clients to progress tokens with timeout reset.
The measured Linux run, including the SIGKILL case.

Intentionally not included, for maintainers to scope:

(a) Progress notifications during long Ghidra startup already landed on main in #1877, after 6.5.0 and still unreleased. Not repeated here.
(b) Preserving or reattaching an open target across server processes changes session semantics and ownership, so it needs a maintainer scope decision.
(c) Concurrent client housekeeping traffic is client-side.
Follow-up candidate: after SIGKILL of the server, the owned rea-ghidra-* runtime directory remained under TMPDIR (see below).

Contract and boundary impact

Semantic owner and earliest changed stage: docs/mcp-contracts.md (MCP recovery guidance).
Contribution scope fit and linked maintainer decision: not applicable (documentation of existing behavior).
CLI and MCP/tool-catalog contract: unchanged.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: none.
Target selection, authority, upstream behavior, and cleanup details: describes existing cleanup only.
Generated metadata, package, or installation impact: none.
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 added or updated: none (documentation).
Executed evidence: a stdio MCP probe (@modelcontextprotocol/client StdioClientTransport → node scripts/rea.mjs mcp, REA_ANALYSIS_PROVIDER=ghidra, isolated HOME/TMPDIR) on the 192-byte DOS MZ conformance fixture. Linux x64 (kernel 7.0), Ghidra 12.1.4, JDK 21.0.12.1, Node 24.18.0:

{"step":"first.session","open":true,"started_at":"2026-10-15T14:51:48.518Z","descendants":[{"comm":"java"},{"comm":"demangler_gnu_v"},{"comm":"decompile"}]}
{"step":"first.after_stdin_close","surviving_pids":[],"runtime_entries":[]}
{"step":"second.session","open":false,"started_at":"2026-10-15T14:51:58.892Z","started_at_changed":true}
{"step":"second.query_without_target","isError":true,"code":"target_unavailable","message":"No app is open. Ask the user which app to investigate, then call open_binary with its local path."}
{"step":"second.reopen","open_error":false,"overview_error":false,"overview_ms":8278}
{"step":"second.after_sigkill","surviving":[]}
{"step":"second.after_sigkill_30s","surviving_pids":[],"runtime_entries":["rea-ghidra-5MMVJO"]}

Remaining proof gaps: macOS, Windows, large targets, a real idle-timeout client (Qwen Code) and an actual OOM kill were not exercised. SIGKILL stands in for the kill. Evidence bundle export/import across a restart is documented from existing tool contracts and was not run in this probe.

Validation performed

npm run docs:check: passed (readmeConsistency 5/5).
node scripts/check-doc-facts.mjs: exit 0. npm run test:focused -- tests/boundary/filesystem/documentationFacts.test.ts: 1 passed.
npx oxfmt --check docs/mcp-contracts.md: formatted.
npm run check:fast (pre-push): 2/2 tasks successful.
Stdio probe: output above.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: Ghidra 12.1.4 on Linux only.
Package or release metadata impact: none.
Security, privacy, process, or containment review: none. The text describes existing behavior.

Author readiness

Human owner: @berelels

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.

🤖 Generated with Claude Code

_{🔄 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/4157 **Author:** [@berelels](https://github.com/berelels) **Created:** 10/15/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `docs/mcp-connection-loss-recovery` --- ### 📝 Commits (1) - [`7fb3aac`](https://github.com/morluto/rea/commit/7fb3aacd7589368135210fa1b8cc412ecea4053c) docs(mcp): describe target recovery after a connection loss ### 📊 Changes **1 file changed** (+31 additions, -0 deletions) <details> <summary>View changed files</summary> 📝 `docs/mcp-contracts.md` (+31 -0) </details> ### 📄 Description ## Summary Refs #3509 (documentation slice; does not close it). It documents what happens to the open target when an MCP stdio connection fails, how a client can tell a restarted server from a target closed on the same connection, and how to recover. The new section, `docs/mcp-contracts.md` → "Ghidra first-query deadlines and recovery" → "Connection loss", is based on a real stdio run. ## Problem and expected behavior #3509 reports that after a transport failure `binary_session` showed `open: false` with no explanation. The existing recovery text covers close/reopen on the *same* connection only. Source and the run below show that the target is lost because the stdio server shuts down when its input closes (`src/main.ts` registers shutdown on stdin `end`/`close`). A reconnecting client therefore talks to a new process. In-process, `BinarySession` clears the active target only on open/close. ## Change and scope Documentation only: - After connection loss: a new process, `open: false`, a different `server_identity.server.started_at`, and `target_unavailable` for target queries. A call in flight has no result. - Recovery: reopen with the same path (cold import again), keep retained records via `export_evidence_bundle`/`import_evidence_bundle`, and point idle-timeout clients to progress tokens with timeout reset. - The measured Linux run, including the SIGKILL case. Intentionally not included, for maintainers to scope: - (a) Progress notifications during long Ghidra startup already landed on `main` in #1877, after 6.5.0 and still unreleased. Not repeated here. - (b) Preserving or reattaching an open target across server processes changes session semantics and ownership, so it needs a maintainer scope decision. - (c) Concurrent client housekeeping traffic is client-side. - Follow-up candidate: after SIGKILL of the server, the owned `rea-ghidra-*` runtime directory remained under `TMPDIR` (see below). ## Contract and boundary impact - Semantic owner and earliest changed stage: `docs/mcp-contracts.md` (MCP recovery guidance). - Contribution scope fit and linked maintainer decision: not applicable (documentation of existing behavior). - CLI and MCP/tool-catalog contract: unchanged. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details: describes existing cleanup only. - Generated metadata, package, or installation impact: none. - [ ] 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. - [x] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: none (documentation). - Executed evidence: a stdio MCP probe (`@modelcontextprotocol/client` `StdioClientTransport` → `node scripts/rea.mjs mcp`, `REA_ANALYSIS_PROVIDER=ghidra`, isolated `HOME`/`TMPDIR`) on the 192-byte DOS MZ conformance fixture. Linux x64 (kernel 7.0), Ghidra 12.1.4, JDK 21.0.12.1, Node 24.18.0: ```json {"step":"first.session","open":true,"started_at":"2026-10-15T14:51:48.518Z","descendants":[{"comm":"java"},{"comm":"demangler_gnu_v"},{"comm":"decompile"}]} {"step":"first.after_stdin_close","surviving_pids":[],"runtime_entries":[]} {"step":"second.session","open":false,"started_at":"2026-10-15T14:51:58.892Z","started_at_changed":true} {"step":"second.query_without_target","isError":true,"code":"target_unavailable","message":"No app is open. Ask the user which app to investigate, then call open_binary with its local path."} {"step":"second.reopen","open_error":false,"overview_error":false,"overview_ms":8278} {"step":"second.after_sigkill","surviving":[]} {"step":"second.after_sigkill_30s","surviving_pids":[],"runtime_entries":["rea-ghidra-5MMVJO"]} ``` - Remaining proof gaps: macOS, Windows, large targets, a real idle-timeout client (Qwen Code) and an actual OOM kill were not exercised. SIGKILL stands in for the kill. Evidence bundle export/import across a restart is documented from existing tool contracts and was not run in this probe. ## Validation performed - `npm run docs:check`: passed (readmeConsistency 5/5). - `node scripts/check-doc-facts.mjs`: exit 0. `npm run test:focused -- tests/boundary/filesystem/documentationFacts.test.ts`: 1 passed. - `npx oxfmt --check docs/mcp-contracts.md`: formatted. - `npm run check:fast` (pre-push): 2/2 tasks successful. - Stdio probe: output above. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: Ghidra 12.1.4 on Linux only. - Package or release metadata impact: none. - Security, privacy, process, or containment review: none. The text describes existing behavior. ## Author readiness Human owner: @berelels - [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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:11:51 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:45:52 +00:00

[PR #6152] [MERGED] fix(artifacts): refuse thin Mach-O extraction before inventory #7166

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

No description provided.

Rows
Columns