morluto/rea

Fork 0

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

[PR #3088] [CLOSED] fix(pwntools): report actionable configuration recovery #5608

New issue

Closed

opened 2026-10-31 01:07:56 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:07:56 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3088
Author: @jpdefo
Created: 10/14/2026
Status: ❌ Closed

Base: main ← Head: fix/pwntools-recovery-guidance

📝 Commits (1)

c6a348f fix(pwntools): report actionable configuration recovery

📊 Changes

5 files changed (+143 additions, -3 deletions)

View changed files

📝 docs/cli.md (+4 -2)
📝 src/domain/providerSelectionError.ts (+4 -1)
📝 src/native/pwntools/PwntoolsFailures.ts (+1 -0)
➕ src/native/pwntools/PwntoolsRecovery.test.ts (+86 -0)
➕ tests/boundary/binaryDiagnostics/pwntoolsRecovery.test.ts (+48 -0)

📄 Description

Summary

Fixes #3055.

Unavailable pwntools operations now name the relevant configuration key and the actual refusal in both the top-level error message and recovery action. Previously these fields recommended rea doctor, which cannot diagnose pwntools.

Problem and expected behavior

On Linux x64, run inspect-binary-layout or inspect-recorded-crash with REA_PWNTOOLS_PYTHON unset, pointing to a directory, or pointing to a missing file. Before this change all six cases recommend doctor. They now expose the configuration requirement or path failure already retained in the provider rejection.

For example, a directory produces REA_PWNTOOLS_PYTHON: Selected Python executable must be a regular executable file, not a directory: <selected path> in message and remediation.action.

Change and scope

Add optional provider-owned recovery guidance to ProviderSelectionError, preserving its existing default. Supply that guidance from pwntoolsUnavailable, using its actual configuration key so input-limit errors still identify REA_PWNTOOLS_MAX_INPUT_BYTES. Narrow the CLI guide's doctor advice to its supported providers.

Contract and boundary impact

Semantic owner and earliest changed stage: pwntools refusal construction, before shared CLI/MCP error projection.
Contribution scope fit and maintainer decision: routine correction within an existing provider. Advance scope decision is not applicable.
CLI and MCP/tool-catalog contract: message and recovery text change. Error codes, retryability, provider identity, rejection details, and captured output are preserved. No schema or catalog change.
Provider, bridge, target-format, and platform compatibility: no provider execution, bridge, supported-format, or host-admission change. Default Hopper/Ghidra/IDA advice is preserved.
Evidence, artifacts, provenance, and reconstruction: unchanged.
Target selection, authority, and cleanup: unchanged. Regression fixtures use exact-path cleanup.
Generated metadata, package, and installation: no changes.
Documentation or usability only, no new target interaction.

Evidence and regression coverage

Two new portable regressions failed on the original source and passed after the fix.
The original compiled CLI at c7105d9723c835b5e0122f5fc6cfbea86265c349 reproduced the misleading guidance in all six scenarios on Ubuntu WSL x64. The same six compiled-CLI checks passed with the corrected guidance.
New integration tests exercise both real provider admission paths for unset, directory, and missing Python configuration.
Portable tests cover launch failure, alternate configuration keys, retained output, and default doctor recovery for Hopper/Ghidra/IDA.

Validation performed

Using the pinned Node 24.18.0 and npm 11.16.0:

Windows npm run check: passed, including typecheck, lint, format, knip, metadata, module boundaries, and architecture guards.
Windows npm run docs:check: passed, including five README consistency tests.
Windows selected source tests: 77 passed.
npm run verify:test-discovery: all 1,235 test files discovered exactly once.
Linux focused source and binary-diagnostics boundary tests: 155 passed, one expected skip for the unsupported-host case.
Linux compiled CLI: six before/after configuration scenarios passed.
Linux real pwntools 4.15.0, pyelftools 0.33, and Unicorn 2.1.2: a configured inspect-binary-layout call on a locally compiled ELF passed, and the selected file's SHA-256 remained unchanged.

Compatibility, safety, and release

No breaking change, migration, or release metadata change. Tests cover refusal and existing provider-boundary behavior. The full real-provider verification lanes, including strace and the optional GDB/pwndbg recorded-core workflow, were not run. Windows was used for portable checks, Ubuntu WSL for this Linux-specific issue.

Implementation and two independent code-review rounds used Codex, with four Luna reviewers and one Sol reviewer per round. The final round reported no actionable findings. A documentation overclaim found in the first round was corrected and all five reviews repeated.

Before merge

Concrete problem and linked issue.
Existing scope and authority preserved.
One focused outcome with a conventional title.
Regression coverage for changed observable behavior.
Owning guidance updated and generated checks passed.
Representative caller-visible output included.
Final diff checked for secrets, unrelated changes, 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/3088 **Author:** [@jpdefo](https://github.com/jpdefo) **Created:** 10/14/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/pwntools-recovery-guidance` --- ### 📝 Commits (1) - [`c6a348f`](https://github.com/morluto/rea/commit/c6a348f9d5fa2da1162659e3b90b6118d16c77f2) fix(pwntools): report actionable configuration recovery ### 📊 Changes **5 files changed** (+143 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `docs/cli.md` (+4 -2) 📝 `src/domain/providerSelectionError.ts` (+4 -1) 📝 `src/native/pwntools/PwntoolsFailures.ts` (+1 -0) ➕ `src/native/pwntools/PwntoolsRecovery.test.ts` (+86 -0) ➕ `tests/boundary/binaryDiagnostics/pwntoolsRecovery.test.ts` (+48 -0) </details> ### 📄 Description ## Summary Fixes #3055. Unavailable pwntools operations now name the relevant configuration key and the actual refusal in both the top-level error message and recovery action. Previously these fields recommended `rea doctor`, which cannot diagnose pwntools. ## Problem and expected behavior On Linux x64, run `inspect-binary-layout` or `inspect-recorded-crash` with `REA_PWNTOOLS_PYTHON` unset, pointing to a directory, or pointing to a missing file. Before this change all six cases recommend doctor. They now expose the configuration requirement or path failure already retained in the provider rejection. For example, a directory produces `REA_PWNTOOLS_PYTHON: Selected Python executable must be a regular executable file, not a directory: <selected path>` in `message` and `remediation.action`. ## Change and scope Add optional provider-owned recovery guidance to `ProviderSelectionError`, preserving its existing default. Supply that guidance from `pwntoolsUnavailable`, using its actual configuration key so input-limit errors still identify `REA_PWNTOOLS_MAX_INPUT_BYTES`. Narrow the CLI guide's doctor advice to its supported providers. ## Contract and boundary impact - Semantic owner and earliest changed stage: pwntools refusal construction, before shared CLI/MCP error projection. - Contribution scope fit and maintainer decision: routine correction within an existing provider. Advance scope decision is not applicable. - CLI and MCP/tool-catalog contract: message and recovery text change. Error codes, retryability, provider identity, rejection details, and captured output are preserved. No schema or catalog change. - Provider, bridge, target-format, and platform compatibility: no provider execution, bridge, supported-format, or host-admission change. Default Hopper/Ghidra/IDA advice is preserved. - Evidence, artifacts, provenance, and reconstruction: unchanged. - Target selection, authority, and cleanup: unchanged. Regression fixtures use exact-path cleanup. - Generated metadata, package, and installation: no changes. - [x] Documentation or usability only, no new target interaction. ## Evidence and regression coverage - Two new portable regressions failed on the original source and passed after the fix. - The original compiled CLI at `c7105d9723c835b5e0122f5fc6cfbea86265c349` reproduced the misleading guidance in all six scenarios on Ubuntu WSL x64. The same six compiled-CLI checks passed with the corrected guidance. - New integration tests exercise both real provider admission paths for unset, directory, and missing Python configuration. - Portable tests cover launch failure, alternate configuration keys, retained output, and default doctor recovery for Hopper/Ghidra/IDA. ## Validation performed Using the pinned Node 24.18.0 and npm 11.16.0: - Windows `npm run check`: passed, including typecheck, lint, format, knip, metadata, module boundaries, and architecture guards. - Windows `npm run docs:check`: passed, including five README consistency tests. - Windows selected source tests: 77 passed. - `npm run verify:test-discovery`: all 1,235 test files discovered exactly once. - Linux focused source and binary-diagnostics boundary tests: 155 passed, one expected skip for the unsupported-host case. - Linux compiled CLI: six before/after configuration scenarios passed. - Linux real pwntools 4.15.0, pyelftools 0.33, and Unicorn 2.1.2: a configured `inspect-binary-layout` call on a locally compiled ELF passed, and the selected file's SHA-256 remained unchanged. ## Compatibility, safety, and release No breaking change, migration, or release metadata change. Tests cover refusal and existing provider-boundary behavior. The full real-provider verification lanes, including strace and the optional GDB/pwndbg recorded-core workflow, were not run. Windows was used for portable checks, Ubuntu WSL for this Linux-specific issue. Implementation and two independent code-review rounds used Codex, with four Luna reviewers and one Sol reviewer per round. The final round reported no actionable findings. A documentation overclaim found in the first round was corrected and all five reviews repeated. ## Before merge - [x] Concrete problem and linked issue. - [x] Existing scope and authority preserved. - [x] One focused outcome with a conventional title. - [x] Regression coverage for changed observable behavior. - [x] Owning guidance updated and generated checks passed. - [x] Representative caller-visible output included. - [x] Final diff checked for secrets, unrelated changes, 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 01:07:56 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:44:33 +00:00

[PR #5608] [MERGED] fix(mcp): guide client and provider timeout recovery #6994

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

No description provided.

Rows
Columns