[PR #4044] [CLOSED] fix(rizin): preserve trailing whitespace before the completion marker #6014

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4044
Author: @Xander-AJ
Created: 10/15/2026
Status: ❌ Closed

Base: main ← Head: fix/rizin-marker-preserve-trailing-whitespace

📝 Commits (2)

8b33c12 fix(rizin): preserve trailing whitespace before the completion marker
c419e55 test(rizin): throw typed errors and reject unmodeled commands in padding fixture

📊 Changes

2 files changed (+36 additions, -1 deletions)

View changed files

📝 src/rizin/RizinDebugSessionManager.ts (+3 -1)
📝 tests/boundary/providers/RizinDebugSessionCoverage.part4.test.ts (+33 -0)

📄 Description

Summary

Fixes #4016. rizin_debug_command removed trailing spaces and a newline from command output when the output and the generated completion marker arrived in one NUL frame, while the same output ending in its own frame was kept. Both results reported complete, untruncated output.

Problem and expected behavior

Trigger: Rizin writes value \n<marker>\n\0 (coalesced) instead of value \n\0<marker>\n\0 (framed). waitForCommandFrames ran trimEnd() over the text before the marker, so the coalesced run returned "value" and the framed run returned "value \n". The same command output should give the same Evidence regardless of how the provider frames it.

Change and scope

Only the text before the marker is no longer trimmed (src/rizin/RizinDebugSessionManager.ts). Nothing else in marker handling, truncation or limits changes. Not included: any change to how frames are joined.

Contract and boundary impact

Semantic owner and earliest changed stage: Rizin command frame parsing (waitForCommandFrames).
Contribution scope fit and linked maintainer decision: ordinary fix within existing boundaries; not applicable.
CLI and MCP/tool-catalog contract: no schema change; rizin_debug_command output may now keep trailing whitespace it previously dropped when coalesced.
Provider, bridge, target-format, or platform compatibility: Rizin only.
Evidence, artifact, provenance, or reconstruction contract: output is now independent of framing.

Expected effects:

Read existing files or captured data.

Evidence and regression coverage

Tests added or updated: tests/boundary/providers/RizinDebugSessionCoverage.part4.test.ts runs the same output framed and coalesced and requires identical results.
Base reproduction or other evidence: the new test fails on the base revision (expected 'value' to be 'value \n') and passes with the change. Executed against the injected-process fixture; not against a real Rizin.
Remaining proof gaps: no real Rizin run; the MCP boundary tests (tests/boundary/mcp) were not run because dist was not built. History check: the trimEnd() was added in the large squashed commit 4aa51bfe1 (#2799) with no stated rationale, and no test or doc I found depends on it. The framed path (output and marker in separate frames) never trimmed, so the old behaviour only applied when frames coalesced. Edge case: a coalesced frame that is only whitespace before the marker (for example a stray \n) now yields that whitespace as output instead of being dropped, matching what a standalone whitespace frame already did.

Validation performed

npx vitest run tests/boundary/providers/Rizin src/rizin — 10 files passed, 83 tests passed, 21 skipped (Node 24.18.0)
npx oxfmt --check on both changed files — formatted
npx oxlint on both changed files — only pre-existing complexity/length warnings in RizinDebugSessionManager.ts

Compatibility, safety, and release

Breaking changes or migration steps: none intended; output may retain trailing whitespace.
Real Hopper/Ghidra, browser, or OS coverage: none.
Package or release metadata impact: none.
Security, privacy, process, or containment review: not applicable.

Author readiness

Drafted with AI assistance and then reviewed by the human owner, who made the declarations below.

Human owner: @Xander-AJ

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/4044 **Author:** [@Xander-AJ](https://github.com/Xander-AJ) **Created:** 10/15/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/rizin-marker-preserve-trailing-whitespace` --- ### 📝 Commits (2) - [`8b33c12`](https://github.com/morluto/rea/commit/8b33c12bf4b77a069960b1b0e0950aa19a036832) fix(rizin): preserve trailing whitespace before the completion marker - [`c419e55`](https://github.com/morluto/rea/commit/c419e55bb765a5141b3c7e71dedd27a6c59fbd07) test(rizin): throw typed errors and reject unmodeled commands in padding fixture ### 📊 Changes **2 files changed** (+36 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `src/rizin/RizinDebugSessionManager.ts` (+3 -1) 📝 `tests/boundary/providers/RizinDebugSessionCoverage.part4.test.ts` (+33 -0) </details> ### 📄 Description ## Summary Fixes #4016. `rizin_debug_command` removed trailing spaces and a newline from command output when the output and the generated completion marker arrived in one NUL frame, while the same output ending in its own frame was kept. Both results reported complete, untruncated output. ## Problem and expected behavior Trigger: Rizin writes `value \n<marker>\n\0` (coalesced) instead of `value \n\0<marker>\n\0` (framed). `waitForCommandFrames` ran `trimEnd()` over the text before the marker, so the coalesced run returned `"value"` and the framed run returned `"value \n"`. The same command output should give the same Evidence regardless of how the provider frames it. ## Change and scope Only the text before the marker is no longer trimmed (`src/rizin/RizinDebugSessionManager.ts`). Nothing else in marker handling, truncation or limits changes. Not included: any change to how frames are joined. ## Contract and boundary impact - Semantic owner and earliest changed stage: Rizin command frame parsing (`waitForCommandFrames`). - Contribution scope fit and linked maintainer decision: ordinary fix within existing boundaries; not applicable. - CLI and MCP/tool-catalog contract: no schema change; `rizin_debug_command` output may now keep trailing whitespace it previously dropped when coalesced. - Provider, bridge, target-format, or platform compatibility: Rizin only. - Evidence, artifact, provenance, or reconstruction contract: output is now independent of framing. Expected effects: - [x] Read existing files or captured data. ## Evidence and regression coverage - Tests added or updated: `tests/boundary/providers/RizinDebugSessionCoverage.part4.test.ts` runs the same output framed and coalesced and requires identical results. - Base reproduction or other evidence: the new test fails on the base revision (`expected 'value' to be 'value \n'`) and passes with the change. Executed against the injected-process fixture; not against a real Rizin. - Remaining proof gaps: no real Rizin run; the MCP boundary tests (`tests/boundary/mcp`) were not run because `dist` was not built. History check: the `trimEnd()` was added in the large squashed commit 4aa51bfe1 (#2799) with no stated rationale, and no test or doc I found depends on it. The framed path (output and marker in separate frames) never trimmed, so the old behaviour only applied when frames coalesced. Edge case: a coalesced frame that is only whitespace before the marker (for example a stray `\n`) now yields that whitespace as output instead of being dropped, matching what a standalone whitespace frame already did. ## Validation performed - `npx vitest run tests/boundary/providers/Rizin src/rizin` — 10 files passed, 83 tests passed, 21 skipped (Node 24.18.0) - `npx oxfmt --check` on both changed files — formatted - `npx oxlint` on both changed files — only pre-existing complexity/length warnings in `RizinDebugSessionManager.ts` ## Compatibility, safety, and release - Breaking changes or migration steps: none intended; output may retain trailing whitespace. - Real Hopper/Ghidra, browser, or OS coverage: none. - Package or release metadata impact: none. - Security, privacy, process, or containment review: not applicable. ## Author readiness Drafted with AI assistance and then reviewed by the human owner, who made the declarations below. Human owner: @Xander-AJ - [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

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