morluto/rea

Fork 0

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

[PR #2449] [MERGED] fix(ida): retain bounded stderr on upstream transport failures #5238

New issue

Closed

opened 2026-10-31 01:05:03 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:05:03 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2449
Author: @Melon-tgj
Created: 10/13/2026
Status: ✅ Merged
Merged: 10/13/2026
Merged by: @morluto

Base: main ← Head: fix/ida-startup-diagnostics-20261013

📝 Commits (3)

179f9de fix(ida): retain bounded stderr on upstream transport failures
6acda01 fix(ida): preserve stderr through operation and cleanup boundaries
ab46104 fix(ida): preserve cancellation and single-owner stderr diagnostics

📊 Changes

4 files changed (+291 additions, -7 deletions)

View changed files

📝 src/ida/IdaMcpConnection.ts (+32 -3)
📝 src/ida/IdaSessionClient.test.ts (+104 -0)
📝 src/ida/IdaSessionClient.ts (+44 -4)
➕ tests/boundary/idaStartupDiagnostics.test.ts (+111 -0)

📄 Description

Summary

Retain bounded upstream stderr when the IDA MCP transport fails.

Problem and expected behavior

If an upstream IDA launcher writes its actual initialization failure to stderr and exits before MCP initialization, REA currently drains and discards that output. The caller receives only CONNECTION_CLOSED, which hides the diagnostic needed to distinguish an IDA initialization failure from a transport problem. Expected behavior is a failed operation retaining the available producer diagnostic.

Change and scope

This retains a bounded 16 KiB diagnostic tail and attaches it to genuine transport failures as ProviderAdapterError.capturedOutput, with the original byte count and explicit truncation flag. Retained bytes are copied so the tail does not keep a larger backing buffer alive. Existing transport credential redaction and failure behavior remain in place; no ambient environment is persisted. Maintainer scope decision: not applicable to this routine diagnostic fix within the existing read-only IDA adapter.

Contract and boundary impact

Semantic owner and earliest changed stage: IdaMcpConnection's stdio diagnostic stream.
Contribution scope: existing IDA analysis transport; no new provider or target execution capability.
CLI/MCP contract: existing error evidence gains producer stderr, byte counts and truncation state; successful results and catalog are unchanged.
Provider/platform compatibility: real SDK/Node child-process boundary; no IDA installation required for the regression.
Evidence/provenance: failed connection remains a failure; observed producer bytes remain attributed to the configured upstream.
Target selection/authority/cleanup: existing connection selection and close lifecycle remain in place.
Generated metadata/package/installation: none required by this error-evidence change.

Expected effects: the existing stdio adapter launches the configured upstream process; the added tests launch controlled Node children. No new target mutation, injection or protection recovery is introduced.

Evidence and regression coverage

Two real SDK/child-process boundary regressions fail against original main and pass with the fix: initialization stderr is retained, and a 20,009-byte diagnostic retains only the 16,384-byte tail with truncated: true.
Existing MCP connection tests pass; typecheck, lint and build pass on Node 24.20.0.
HTTP diagnostics and failures with no stderr preserve their existing path.

Validation performed

npm run test:focused -- tests/boundary/idaStartupDiagnostics.test.ts — 2 pass; both fail against original main because capturedOutput is missing.
npm run test:local -- src/ida/IdaMcpConnection.test.ts src/ida/IdaOperationFailures.test.ts src/domain/functionComparison.test.ts — 67 pass in the combined local validation build.
npm run check:fast and npm run build:cached — pass.

Compatibility, safety, and release

No migration is required. HTTP diagnostics and failures with no stderr retain their existing path. This error-evidence change does not establish real Hopper/Ghidra or OS/provider coverage. Only configured transport authentication values are redacted; arbitrary upstream producer text is preserved under the existing evidence policy. The child-process regressions require no licensed IDA installation.

Before merge

Concrete problem, focused title and regression coverage.
Final diff checked for secrets, unrelated changes and private targets.
Observations, failed attempts and explicit truncation remain visible.
Maintainer review. No separate issue is linked because this PR includes the reproducible bug and regression.

_{🔄 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/2449 **Author:** [@Melon-tgj](https://github.com/Melon-tgj) **Created:** 10/13/2026 **Status:** ✅ Merged **Merged:** 10/13/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/ida-startup-diagnostics-20261013` --- ### 📝 Commits (3) - [`179f9de`](https://github.com/morluto/rea/commit/179f9de299dff9ffa912e0c0352cc146048de94d) fix(ida): retain bounded stderr on upstream transport failures - [`6acda01`](https://github.com/morluto/rea/commit/6acda019a5d5057dbc1c94aaaa47a3f1d2bda70b) fix(ida): preserve stderr through operation and cleanup boundaries - [`ab46104`](https://github.com/morluto/rea/commit/ab461048a3abe13cdcac10efd367a139d56526f5) fix(ida): preserve cancellation and single-owner stderr diagnostics ### 📊 Changes **4 files changed** (+291 additions, -7 deletions) <details> <summary>View changed files</summary> 📝 `src/ida/IdaMcpConnection.ts` (+32 -3) 📝 `src/ida/IdaSessionClient.test.ts` (+104 -0) 📝 `src/ida/IdaSessionClient.ts` (+44 -4) ➕ `tests/boundary/idaStartupDiagnostics.test.ts` (+111 -0) </details> ### 📄 Description ## Summary Retain bounded upstream stderr when the IDA MCP transport fails. ## Problem and expected behavior If an upstream IDA launcher writes its actual initialization failure to stderr and exits before MCP initialization, REA currently drains and discards that output. The caller receives only `CONNECTION_CLOSED`, which hides the diagnostic needed to distinguish an IDA initialization failure from a transport problem. Expected behavior is a failed operation retaining the available producer diagnostic. ## Change and scope This retains a bounded 16 KiB diagnostic tail and attaches it to genuine transport failures as `ProviderAdapterError.capturedOutput`, with the original byte count and explicit truncation flag. Retained bytes are copied so the tail does not keep a larger backing buffer alive. Existing transport credential redaction and failure behavior remain in place; no ambient environment is persisted. Maintainer scope decision: not applicable to this routine diagnostic fix within the existing read-only IDA adapter. ## Contract and boundary impact - Semantic owner and earliest changed stage: `IdaMcpConnection`'s stdio diagnostic stream. - Contribution scope: existing IDA analysis transport; no new provider or target execution capability. - CLI/MCP contract: existing error evidence gains producer stderr, byte counts and truncation state; successful results and catalog are unchanged. - Provider/platform compatibility: real SDK/Node child-process boundary; no IDA installation required for the regression. - Evidence/provenance: failed connection remains a failure; observed producer bytes remain attributed to the configured upstream. - Target selection/authority/cleanup: existing connection selection and close lifecycle remain in place. - Generated metadata/package/installation: none required by this error-evidence change. Expected effects: the existing stdio adapter launches the configured upstream process; the added tests launch controlled Node children. No new target mutation, injection or protection recovery is introduced. ## Evidence and regression coverage - Two real SDK/child-process boundary regressions fail against original main and pass with the fix: initialization stderr is retained, and a 20,009-byte diagnostic retains only the 16,384-byte tail with `truncated: true`. - Existing MCP connection tests pass; typecheck, lint and build pass on Node 24.20.0. - HTTP diagnostics and failures with no stderr preserve their existing path. ## Validation performed - `npm run test:focused -- tests/boundary/idaStartupDiagnostics.test.ts` — 2 pass; both fail against original main because `capturedOutput` is missing. - `npm run test:local -- src/ida/IdaMcpConnection.test.ts src/ida/IdaOperationFailures.test.ts src/domain/functionComparison.test.ts` — 67 pass in the combined local validation build. - `npm run check:fast` and `npm run build:cached` — pass. ## Compatibility, safety, and release No migration is required. HTTP diagnostics and failures with no stderr retain their existing path. This error-evidence change does not establish real Hopper/Ghidra or OS/provider coverage. Only configured transport authentication values are redacted; arbitrary upstream producer text is preserved under the existing evidence policy. The child-process regressions require no licensed IDA installation. ## Before merge - [x] Concrete problem, focused title and regression coverage. - [x] Final diff checked for secrets, unrelated changes and private targets. - [x] Observations, failed attempts and explicit truncation remain visible. - [ ] Maintainer review. No separate issue is linked because this PR includes the reproducible bug and regression. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:05:03 +00:00

closed this issue
added the
pull-request
label

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

No description provided.

Rows
Columns