[PR #4546] fix(rizin): default debugger sessions to plain output #6297

New issue

Open

opened 2026-10-31 01:13:23 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:13:23 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4546
Author: @VuxSussy
Created: 10/16/2026
Status: 🔄 Open

Base: main ← Head: fix/rizin-default-no-color

📝 Commits (2)

7081c09 fix(rizin): disable ANSI colors by default
0483337 fix(rizin): narrow color default to debugger sessions

📊 Changes

4 files changed (+29 additions, -4 deletions)

View changed files

📝 docs/reverse-engineering-tools.md (+2 -0)
📝 src/rizin/RizinDebugSessionManager.ts (+2 -0)
📝 tests/boundary/providers/RizinDebugSessionManagers.test.ts (+15 -4)
📝 tests/boundary/providers/RizinInferiorTty.test.ts (+10 -0)

📄 Description

Summary

Default persistent Linux Rizin debugger sessions to plain output by adding -e scr.color=0 at the shared session launch boundary used by MCP and rea debug-with-rizin. Callers can still enable colours with e scr.color=1, and REA preserves returned ANSI bytes.

Compared against main fetched before this update at a2e2c29b59e8775b0e6aedb09a9eb16adf32646b. The one-shot default from #4488 is already on main and is excluded from this diff. The session default remains a separate proposal, as requested in the maintainer comment.

Change and scope

Set the presentation default once at session startup, without rewriting later commands.
Extend the existing manager regression to check launch arguments, command preservation, raw ANSI output, completion framing, and close.
Extend the existing Linux/Rizin 0.7.4 test to read back the default, enable colours, and check that the override persists across commands.
Document the session default and override. The diff against main touches four files; one-shot implementation and coverage match main.

The semantic owner is RizinDebugSessionManager. CLI/MCP schemas, target selection, execution authority, TTY routing and cleanup semantics retain their current contracts. This PR proposes a presentation default within the existing debugger; it does not claim that #4488 approved the session proposal.

Evidence and validation

Executed with Node.js 24.22.1 and npm 11.16.0 on macOS arm64:

On the main session implementation, the updated manager tests produced 2 failures and 4 passes (baseline run at c9df40f8; the production session source is identical at a2e2c29b). Both failures identified the missing -e scr.color=0 launch arguments.
After the change, the focused selection below produced 43 passes and 19 skips.
npm run typecheck, npm run lint, npm run verify:architecture-guards, npm run knip:production, and npm run lint:typed — passed individually. Formatting and package metadata checks passed within npm run check.
npm run check — failed at Knip on three existing findings, also reproduced without the session production change on the main snapshot: staticFileUrlPath, AndroidManifestComponentInventory, and ObjcClassImportBindingCoverage. They are outside this session-only diff.

Focused command:

npm run test:focused -- tests/boundary/providers/RizinDebugSessionManagers.test.ts tests/boundary/providers/RizinDebugSessionCoverage.part1.test.ts tests/boundary/providers/RizinInferiorTty.test.ts tests/boundary/mcp/rizinOutputPreservation.test.ts

Coverage limits: the passing process fixtures model Linux protocol behavior on macOS, and the MCP boundary uses a simulated provider. The 19 skipped tests require Linux, including the real Rizin 0.7.4 lane. The added real-provider assertions have not executed here. No real Rizin debugger session or real-provider CLI/MCP workflow was run in this pass, and no full-suite pass is claimed.

Author readiness

Human owner: @VuxSussy

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.

_{🔄 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/4546 **Author:** [@VuxSussy](https://github.com/VuxSussy) **Created:** 10/16/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/rizin-default-no-color` --- ### 📝 Commits (2) - [`7081c09`](https://github.com/morluto/rea/commit/7081c096ba50f1c44905fffc191b997cf6bf2434) fix(rizin): disable ANSI colors by default - [`0483337`](https://github.com/morluto/rea/commit/0483337bbc45bdb5aea3787e24734081633f214e) fix(rizin): narrow color default to debugger sessions ### 📊 Changes **4 files changed** (+29 additions, -4 deletions) <details> <summary>View changed files</summary> 📝 `docs/reverse-engineering-tools.md` (+2 -0) 📝 `src/rizin/RizinDebugSessionManager.ts` (+2 -0) 📝 `tests/boundary/providers/RizinDebugSessionManagers.test.ts` (+15 -4) 📝 `tests/boundary/providers/RizinInferiorTty.test.ts` (+10 -0) </details> ### 📄 Description ## Summary Default persistent Linux Rizin debugger sessions to plain output by adding `-e scr.color=0` at the shared session launch boundary used by MCP and `rea debug-with-rizin`. Callers can still enable colours with `e scr.color=1`, and REA preserves returned ANSI bytes. Compared against main fetched before this update at `a2e2c29b59e8775b0e6aedb09a9eb16adf32646b`. The one-shot default from #4488 is already on main and is excluded from this diff. The session default remains a separate proposal, as requested in [the maintainer comment](https://github.com/morluto/rea/pull/4546#issuecomment-6427892971). ## Change and scope - Set the presentation default once at session startup, without rewriting later commands. - Extend the existing manager regression to check launch arguments, command preservation, raw ANSI output, completion framing, and close. - Extend the existing Linux/Rizin 0.7.4 test to read back the default, enable colours, and check that the override persists across commands. - Document the session default and override. The diff against main touches four files; one-shot implementation and coverage match main. The semantic owner is `RizinDebugSessionManager`. CLI/MCP schemas, target selection, execution authority, TTY routing and cleanup semantics retain their current contracts. This PR proposes a presentation default within the existing debugger; it does not claim that #4488 approved the session proposal. ## Evidence and validation Executed with Node.js 24.22.1 and npm 11.16.0 on macOS arm64: - On the main session implementation, the updated manager tests produced **2 failures and 4 passes** (baseline run at `c9df40f8`; the production session source is identical at `a2e2c29b`). Both failures identified the missing `-e scr.color=0` launch arguments. - After the change, the focused selection below produced **43 passes and 19 skips**. - `npm run typecheck`, `npm run lint`, `npm run verify:architecture-guards`, `npm run knip:production`, and `npm run lint:typed` — passed individually. Formatting and package metadata checks passed within `npm run check`. - `npm run check` — failed at Knip on three existing findings, also reproduced without the session production change on the main snapshot: `staticFileUrlPath`, `AndroidManifestComponentInventory`, and `ObjcClassImportBindingCoverage`. They are outside this session-only diff. Focused command: ```sh npm run test:focused -- tests/boundary/providers/RizinDebugSessionManagers.test.ts tests/boundary/providers/RizinDebugSessionCoverage.part1.test.ts tests/boundary/providers/RizinInferiorTty.test.ts tests/boundary/mcp/rizinOutputPreservation.test.ts ``` Coverage limits: the passing process fixtures model Linux protocol behavior on macOS, and the MCP boundary uses a simulated provider. The 19 skipped tests require Linux, including the real Rizin 0.7.4 lane. The added real-provider assertions have not executed here. No real Rizin debugger session or real-provider CLI/MCP workflow was run in this pass, and no full-suite pass is claimed. ## Author readiness Human owner: @VuxSussy - [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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-10-31 01:13:23 +00:00

bsakaguchi referenced this issue

2026-11-01 21:46:35 +00:00

[PR #6297] [CLOSED] fix(cli): align repeatable inputs with canonical array bounds #7264