[PR #2010] [CLOSED] fix(ghidra): retain bounded LaunchSupport failure output #4964

New issue

Closed

opened 2026-10-31 00:57:28 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 00:57:28 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2010
Author: @lyly-bonbon
Created: 10/12/2026
Status: ❌ Closed

Base: main ← Head: fix/ghidra-launch-output

📝 Commits (10+)

c2c170b fix(ghidra): retain bounded LaunchSupport failure output
90cda4e fix(ghidra): preserve LaunchSupport failure provenance
ffd5f41 fix: reconcile main and review findings for PR #2010
fc83565 fix(ghidra): share redacted launch failure diagnostics
4d16e6a fix(process): report no remaining time after a provider deadline fires
32be546 fix(ghidra): preserve typed helper outcomes and redact retained credentials
cdc39ed fix(comparison): recheck historical calls-only changes (#2223)
1e1b7ed fix(android): make DEX descriptor pattern portable
38556a1 fix(ci): isolate Linux Hopper process fixture inventory
8cd3edb style: format comparison and IDA sources

📊 Changes

11 files changed (+662 additions, -81 deletions)

View changed files

📝 src/ghidra/GhidraClient.ts (+9 -5)
📝 src/ghidra/GhidraJavaLaunch.ts (+51 -17)
📝 src/ghidra/GhidraLaunchFailure.test.ts (+8 -7)
📝 src/ghidra/GhidraLauncher.ts (+25 -7)
📝 src/ghidra/GhidraSessionError.test.ts (+166 -4)
📝 src/ghidra/GhidraSessionError.ts (+55 -13)
📝 src/process/ExecFileOutput.ts (+1 -1)
➕ src/process/ProviderDiagnosticRedaction.test.ts (+37 -0)
📝 src/process/ProviderDiagnosticRedaction.ts (+67 -19)
📝 tests/boundary/mcp/ghidraFailureDiagnostics.test.ts (+202 -2)
📝 tests/boundary/providers/ghidra/ghidraJavaLaunch.test.ts (+41 -6)

📄 Description

Summary

Retain the output of failed Ghidra LaunchSupport commands in session diagnostics. -envvars and -vmargs failures already preserve their exit code and nested cause, but stdout is dropped and stderr is only visible inside a message limited to 2 KiB.

Refs #1992.

Problem and expected behavior

A LaunchSupport command that writes diagnostics and exits with code 7 should expose those diagnostics after startup cleanup. Before this change, stdout disappears; a useful reason at the end of a 3 KiB stderr stream is also lost when the error message is truncated.

Wrap the failed command at the LaunchSupport boundary and project its captured streams into failure_cause.cause.command_outcome, with an 8 KiB UTF-8 budget per stream after credential redaction. The original command error remains the wrapper's cause, including its exit code and signal.

Selected fields from the fixture's nested failure cause:

{
  "name": "GhidraLaunchSupportError",
  "command_outcome": {
    "stdout": "LaunchSupport -envvars stdout diagnostic",
    "stdoutTruncated": false,
    "stderrTruncated": false,
    "outputTruncated": false
  },
  "cause": { "code": 7, "signal": null }
}

Change and scope

Parse captured output into a typed GhidraLaunchSupportError at the two LaunchSupport calls. The session projection only exposes command_outcome for that error type, so unrelated failures such as the Swift ownership-helper compiler retain their existing diagnostic representation.

stdoutBytes and stderrBytes measure the captured text before redaction; they do not estimate bytes lost upstream. stdoutTruncated and stderrTruncated describe the projection budget, while outputTruncated records capture truncation. On ABORT_ERR, killed and outputTruncated are null: native execFile cancellation can settle before the child's exit and final stream delivery are observed.

Contract and boundary impact

Semantic owner and earliest changed stage: LaunchSupport command failure capture, then Ghidra session-error diagnostics.
CLI and MCP/tool-catalog contract: additive diagnostic fields; no tool/schema changes.
Provider, bridge, target-format, or platform compatibility: no change to launch behavior or supported targets.
Evidence, artifact, provenance, or reconstruction contract: none.
Process execution, authorization, cleanup, or containment impact: none.
Generated metadata, package, or installation impact: none.

Evidence and regression coverage

Real fixture subprocesses fail at each LaunchSupport phase. The tests verify complete stdout and stderr beyond the message limit, exit code 7, runtime cleanup, and propagation through the provider, CLI adapter and in-memory SDK MCP transport.
Both new boundary cases failed on the base revision because command_outcome was missing, then passed with the change.
Caller-cancellation cases for both phases failed on c2c170b3 because unknown termination and capture completeness were reported as false. They pass with nullable fields and verify the fixture exits and its runtime is removed. Cancellation coverage stops at the production client diagnostic boundary; provider cancellation mapping is unchanged.
A compiler-error regression failed on c2c170b3 because the nested command outcome duplicated the existing compiler outcome. The typed boundary prevents that duplication.
Unit coverage checks credential redaction across the UTF-8 truncation boundary and distinguishes capture truncation from projection truncation.
Remaining proof gap: the original macOS x64/JDK/Ghidra failure has not been reproduced. These tests cover the remaining diagnostic-loss branch, not its underlying launch failure.

For evidence-bearing changes:

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

On macOS arm64 with Node 24.18.0 and npm 11.16.0:

npm run test:focused -- src/ghidra/GhidraSessionError.test.ts tests/boundary/mcp/ghidraFailureDiagnostics.test.ts src/ghidra/GhidraRequestQueue.test.ts tests/boundary/providers/ghidra/ghidraClient.test.ts tests/boundary/providers/ghidra/ghidraLauncher.test.ts — 70 passed, 1 Windows-only test skipped.
npm run check — passed, including typecheck, lint, formatting, unused-code checks, metadata and architecture guards. Lint reports warnings but no errors.
git diff --check — passed.
node scripts/verify-real-ghidra.mjs — could not run: GHIDRA_INSTALL_DIR is unset and this host has no configured Ghidra/JDK installation.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: fixture subprocesses on macOS arm64; real Ghidra and native Windows/Linux not verified.
Package or release metadata impact: none.
Security, privacy, process, or containment review: output uses the existing credential redactor before bounded projection. No arbitrary error properties, environment or stack traces are added.

Review checklist

This PR addresses a concrete problem or an agreed enhancement.
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/2010 **Author:** [@lyly-bonbon](https://github.com/lyly-bonbon) **Created:** 10/12/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/ghidra-launch-output` --- ### 📝 Commits (10+) - [`c2c170b`](https://github.com/morluto/rea/commit/c2c170b3ccc55af0bdf8042fcfb573f3e5305fdc) fix(ghidra): retain bounded LaunchSupport failure output - [`90cda4e`](https://github.com/morluto/rea/commit/90cda4eb79c0da4398e3f9b57dc564d4c0266c42) fix(ghidra): preserve LaunchSupport failure provenance - [`ffd5f41`](https://github.com/morluto/rea/commit/ffd5f4119e0c03d94c62cb3b0fc66c9c4b201f95) fix: reconcile main and review findings for PR #2010 - [`fc83565`](https://github.com/morluto/rea/commit/fc835656b5dbf0f3567264947b340f92dcb9a903) fix(ghidra): share redacted launch failure diagnostics - [`4d16e6a`](https://github.com/morluto/rea/commit/4d16e6ae564cfdc4511c7e31c654f46214d65a0e) fix(process): report no remaining time after a provider deadline fires - [`32be546`](https://github.com/morluto/rea/commit/32be5466dbbf054f73491314c36832b9941f6c4c) fix(ghidra): preserve typed helper outcomes and redact retained credentials - [`cdc39ed`](https://github.com/morluto/rea/commit/cdc39edddae06704c5874042b2e86596e6471e84) fix(comparison): recheck historical calls-only changes (#2223) - [`1e1b7ed`](https://github.com/morluto/rea/commit/1e1b7eddaacb21628b46f9afedc5f69d12609b16) fix(android): make DEX descriptor pattern portable - [`38556a1`](https://github.com/morluto/rea/commit/38556a1e37c24c5628170a1eba30f505c12b2f83) fix(ci): isolate Linux Hopper process fixture inventory - [`8cd3edb`](https://github.com/morluto/rea/commit/8cd3edb8fec9d8e401883aaf8f64072bf471e2af) style: format comparison and IDA sources ### 📊 Changes **11 files changed** (+662 additions, -81 deletions) <details> <summary>View changed files</summary> 📝 `src/ghidra/GhidraClient.ts` (+9 -5) 📝 `src/ghidra/GhidraJavaLaunch.ts` (+51 -17) 📝 `src/ghidra/GhidraLaunchFailure.test.ts` (+8 -7) 📝 `src/ghidra/GhidraLauncher.ts` (+25 -7) 📝 `src/ghidra/GhidraSessionError.test.ts` (+166 -4) 📝 `src/ghidra/GhidraSessionError.ts` (+55 -13) 📝 `src/process/ExecFileOutput.ts` (+1 -1) ➕ `src/process/ProviderDiagnosticRedaction.test.ts` (+37 -0) 📝 `src/process/ProviderDiagnosticRedaction.ts` (+67 -19) 📝 `tests/boundary/mcp/ghidraFailureDiagnostics.test.ts` (+202 -2) 📝 `tests/boundary/providers/ghidra/ghidraJavaLaunch.test.ts` (+41 -6) </details> ### 📄 Description ## Summary Retain the output of failed Ghidra LaunchSupport commands in session diagnostics. `-envvars` and `-vmargs` failures already preserve their exit code and nested cause, but stdout is dropped and stderr is only visible inside a message limited to 2 KiB. Refs #1992. ## Problem and expected behavior A LaunchSupport command that writes diagnostics and exits with code 7 should expose those diagnostics after startup cleanup. Before this change, stdout disappears; a useful reason at the end of a 3 KiB stderr stream is also lost when the error message is truncated. Wrap the failed command at the LaunchSupport boundary and project its captured streams into `failure_cause.cause.command_outcome`, with an 8 KiB UTF-8 budget per stream after credential redaction. The original command error remains the wrapper's cause, including its exit code and signal. Selected fields from the fixture's nested failure cause: ```json { "name": "GhidraLaunchSupportError", "command_outcome": { "stdout": "LaunchSupport -envvars stdout diagnostic", "stdoutTruncated": false, "stderrTruncated": false, "outputTruncated": false }, "cause": { "code": 7, "signal": null } } ``` ## Change and scope Parse captured output into a typed `GhidraLaunchSupportError` at the two LaunchSupport calls. The session projection only exposes `command_outcome` for that error type, so unrelated failures such as the Swift ownership-helper compiler retain their existing diagnostic representation. `stdoutBytes` and `stderrBytes` measure the captured text before redaction; they do not estimate bytes lost upstream. `stdoutTruncated` and `stderrTruncated` describe the projection budget, while `outputTruncated` records capture truncation. On `ABORT_ERR`, `killed` and `outputTruncated` are `null`: native `execFile` cancellation can settle before the child's exit and final stream delivery are observed. ## Contract and boundary impact - Semantic owner and earliest changed stage: LaunchSupport command failure capture, then Ghidra session-error diagnostics. - CLI and MCP/tool-catalog contract: additive diagnostic fields; no tool/schema changes. - Provider, bridge, target-format, or platform compatibility: no change to launch behavior or supported targets. - Evidence, artifact, provenance, or reconstruction contract: none. - Process execution, authorization, cleanup, or containment impact: none. - Generated metadata, package, or installation impact: none. ## Evidence and regression coverage - Real fixture subprocesses fail at each LaunchSupport phase. The tests verify complete stdout and stderr beyond the message limit, exit code 7, runtime cleanup, and propagation through the provider, CLI adapter and in-memory SDK MCP transport. - Both new boundary cases failed on the base revision because `command_outcome` was missing, then passed with the change. - Caller-cancellation cases for both phases failed on `c2c170b3` because unknown termination and capture completeness were reported as `false`. They pass with nullable fields and verify the fixture exits and its runtime is removed. Cancellation coverage stops at the production client diagnostic boundary; provider cancellation mapping is unchanged. - A compiler-error regression failed on `c2c170b3` because the nested command outcome duplicated the existing compiler outcome. The typed boundary prevents that duplication. - Unit coverage checks credential redaction across the UTF-8 truncation boundary and distinguishes capture truncation from projection truncation. - Remaining proof gap: the original macOS x64/JDK/Ghidra failure has not been reproduced. These tests cover the remaining diagnostic-loss branch, not its underlying launch failure. For evidence-bearing changes: - [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 On macOS arm64 with Node 24.18.0 and npm 11.16.0: - `npm run test:focused -- src/ghidra/GhidraSessionError.test.ts tests/boundary/mcp/ghidraFailureDiagnostics.test.ts src/ghidra/GhidraRequestQueue.test.ts tests/boundary/providers/ghidra/ghidraClient.test.ts tests/boundary/providers/ghidra/ghidraLauncher.test.ts` — 70 passed, 1 Windows-only test skipped. - `npm run check` — passed, including typecheck, lint, formatting, unused-code checks, metadata and architecture guards. Lint reports warnings but no errors. - `git diff --check` — passed. - `node scripts/verify-real-ghidra.mjs` — could not run: `GHIDRA_INSTALL_DIR` is unset and this host has no configured Ghidra/JDK installation. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: fixture subprocesses on macOS arm64; real Ghidra and native Windows/Linux not verified. - Package or release metadata impact: none. - Security, privacy, process, or containment review: output uses the existing credential redactor before bounded projection. No arbitrary error properties, environment or stack traces are added. ## Review checklist - [x] This PR addresses a concrete problem or an agreed enhancement. - [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 00:57:28 +00:00