morluto/rea

Fork 0

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

[GH-ISSUE #4016] [Bug] Rizin completion-marker parsing strips trailing command-output whitespace #1602

New issue

Closed

opened 2026-10-16 21:07:46 +00:00 by bsakaguchi · 3 comments

bsakaguchi commented

2026-10-16 21:07:46 +00:00

Owner

Originally created by @coygeek on GitHub (Oct 15, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/4016

Before submitting

I searched the open and closed issues for a related report.
I removed credentials, tokens, private paths, and sensitive target data.

Area

Evidence, artifacts, or reconstruction

Observed behavior

rizin_debug_command removes trailing spaces and a newline when command output and its generated completion marker share one NUL frame. The identical command-output prefix survives when it ends in its own NUL frame. Both results claim complete, untruncated output.

Reproduction

The saved verification ran the following reduced probe through the registered MCP tools and linked in-memory transport. In a disposable built checkout with locked dependencies, save it as tests/boundary/mcp/rizinWhitespaceProbe.test.ts and run npx vitest run tests/boundary/mcp/rizinWhitespaceProbe.test.ts with isolated HOME and TMPDIR directories. The assertions characterize the current failure, not the expected repair.

Executed reduced probe

import { Client, InMemoryTransport } from "@modelcontextprotocol/client";
import { McpServer } from "@modelcontextprotocol/server";
import { expect, it } from "vitest";
import { z } from "zod";
import { createInjectedProcess } from "../providers/RizinDebugSessionCoverage.fixtures.js";
import { createRizinTestManager, rizinTestInferiorTty } from "../providers/RizinTestInferiorTty.js";
import { registerRizinDebugTools } from "../../../src/server/registerRizinDebugTools.js";
import { createToolResultDelivery } from "../../../src/server/toolResult.js";
import { silentLogger } from "../../../src/logger.js";

for (const mode of ["whitespace", "framed-whitespace"] as const) {
  it(`observes ${mode} through the registered Rizin MCP tools`, async () => {
    const injected = createInjectedProcess({
      onWrite: (text, control) => {
        const marker = /!echo (.+)\n/u.exec(text)?.[1];
        if (!marker) return;
        setImmediate(() => {
          if (mode === "whitespace") {
            control.output(`value  \n${marker}\n\0`);
          } else if (mode === "framed-whitespace") {
            control.output(`value  \n\0${marker}\n\0`);
          }
        });
      },
    });
    const manager = createRizinTestManager({}, {
      spawnProcess: injected.spawnProcess,
      frameTimeoutMs: 30,
    });
    const server = new McpServer({ name: "rizin-observation", version: "1" });
    registerRizinDebugTools(server, manager, silentLogger, createToolResultDelivery(undefined));
    const client = new Client({ name: "rizin-observation", version: "1" });
    const [ct, st] = InMemoryTransport.createLinkedPair();
    try {
      await server.connect(st);
      await client.connect(ct);
      const started = await client.callTool({ name: "start_rizin_debug_session", arguments: {
        path: "fixture.bin", inferior_tty: await rizinTestInferiorTty(),
      }});
      const sessionId = z.object({ session_id: z.string() }).parse(started.structuredContent).session_id;
      const result = await client.callTool({ name: "rizin_debug_command", arguments: {
        session_id: sessionId, command: "dr",
      }});
      expect(result.isError).not.toBe(true);
      const evidence = z.object({ normalized_result: z.object({ output: z.string(), completion_status: z.string(), output_truncated: z.boolean() }) }).parse(result.structuredContent);
      console.log(JSON.stringify({ mode, ...evidence.normalized_result, output: evidence.normalized_result.output.length > 100 ? "<large output>" : evidence.normalized_result.output, output_bytes: Buffer.byteLength(evidence.normalized_result.output) }));
      if (mode === "whitespace") expect(evidence.normalized_result.output).toBe("value");
      if (mode === "framed-whitespace") expect(evidence.normalized_result.output).toBe("value  \n");
    } finally {
      injected.control.exit();
      await Promise.allSettled([client.close(), server.close()]);
      await manager.closeAll().catch(() => undefined);
    }
  });
}

Each case ran once in the reduced probe after the same observations in a broader probe. Responses occur after command dispatch, excluding unsolicited startup frames.

Expected behavior

Return the command prefix exactly while excluding only framing and the generated completion marker. For this fixture, retain "value \n" regardless of the NUL frame boundary. src/contracts/rizinDebugToolContracts.ts:48 promises raw output, including backend output before the completion marker. tests/boundary/providers/RizinDebugSessionCoverage.part3.test.ts:220 already exercises a coalesced-marker frame.

Impact and scope

P2. Raw command Evidence silently differs from received text while reporting complete, untruncated output. The observed case loses three bytes. Production-provider frequency is unknown. Frame boundaries belong to the provider protocol, and no general caller-controlled workaround was verified.

REA version

rea 6.5.0 at source commit 4692bb3330

Runtime and platform

Node 24.18.0; macOS arm64

Target and artifact context

Simulated Linux Rizin process and no-inferior cleanup fixtures; valid host fixture TTY; no target program launched.

Interface

MCP

Earliest failure stage

Capture or observation

Relevant output or evidence

The reduced MCP probe returned:

{"mode":"whitespace","output":"value","completion_status":"complete","output_truncated":false,"output_bytes":5}
{"mode":"framed-whitespace","output":"value  \n","completion_status":"complete","output_truncated":false,"output_bytes":8}

The coalesced input is value, two spaces, newline, the generated marker, newline and NUL. The control adds only a NUL after the command prefix. Both normalized and raw Evidence contain the altered text in the coalesced case.

Typed outcome or recovery expected

Retain "value \n" exactly in both cases and retain whitespace-only prefixes. Continue omitting the generated marker, enforcing output bounds and keeping completion unknown when no marker arrives. Trimming the separately framed control would not restore the raw-output contract.

Analysis or suspected cause

Source-derived: src/rizin/RizinDebugSessionManager.ts:1469 applies trimEnd() to the entire prefix before the completion marker. src/server/registerRizinDebugTools.ts:66 routes the request to this manager; createRizinCommandEvidence in the manager places the altered value in normalized and raw Evidence.

Additional context

Vitest 5.0.3. Real Rizin and Linux descriptor readback were not exercised. The cited source files are unchanged between the reproduced baseline and origin/main 6fcfc5b5e0. The reproduction was not rerun for this submission.

Originally created by @coygeek on GitHub (Oct 15, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/4016 ### Before submitting - [x] I searched the open and closed issues for a related report. - [x] I removed credentials, tokens, private paths, and sensitive target data. ### Area Evidence, artifacts, or reconstruction ### Observed behavior `rizin_debug_command` removes trailing spaces and a newline when command output and its generated completion marker share one NUL frame. The identical command-output prefix survives when it ends in its own NUL frame. Both results claim complete, untruncated output. ### Reproduction The saved verification ran the following reduced probe through the registered MCP tools and linked in-memory transport. In a disposable built checkout with locked dependencies, save it as `tests/boundary/mcp/rizinWhitespaceProbe.test.ts` and run `npx vitest run tests/boundary/mcp/rizinWhitespaceProbe.test.ts` with isolated `HOME` and `TMPDIR` directories. The assertions characterize the current failure, not the expected repair. <details> <summary>Executed reduced probe</summary> ```ts import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; import { McpServer } from "@modelcontextprotocol/server"; import { expect, it } from "vitest"; import { z } from "zod"; import { createInjectedProcess } from "../providers/RizinDebugSessionCoverage.fixtures.js"; import { createRizinTestManager, rizinTestInferiorTty } from "../providers/RizinTestInferiorTty.js"; import { registerRizinDebugTools } from "../../../src/server/registerRizinDebugTools.js"; import { createToolResultDelivery } from "../../../src/server/toolResult.js"; import { silentLogger } from "../../../src/logger.js"; for (const mode of ["whitespace", "framed-whitespace"] as const) { it(`observes ${mode} through the registered Rizin MCP tools`, async () => { const injected = createInjectedProcess({ onWrite: (text, control) => { const marker = /!echo (.+)\n/u.exec(text)?.[1]; if (!marker) return; setImmediate(() => { if (mode === "whitespace") { control.output(`value \n${marker}\n\0`); } else if (mode === "framed-whitespace") { control.output(`value \n\0${marker}\n\0`); } }); }, }); const manager = createRizinTestManager({}, { spawnProcess: injected.spawnProcess, frameTimeoutMs: 30, }); const server = new McpServer({ name: "rizin-observation", version: "1" }); registerRizinDebugTools(server, manager, silentLogger, createToolResultDelivery(undefined)); const client = new Client({ name: "rizin-observation", version: "1" }); const [ct, st] = InMemoryTransport.createLinkedPair(); try { await server.connect(st); await client.connect(ct); const started = await client.callTool({ name: "start_rizin_debug_session", arguments: { path: "fixture.bin", inferior_tty: await rizinTestInferiorTty(), }}); const sessionId = z.object({ session_id: z.string() }).parse(started.structuredContent).session_id; const result = await client.callTool({ name: "rizin_debug_command", arguments: { session_id: sessionId, command: "dr", }}); expect(result.isError).not.toBe(true); const evidence = z.object({ normalized_result: z.object({ output: z.string(), completion_status: z.string(), output_truncated: z.boolean() }) }).parse(result.structuredContent); console.log(JSON.stringify({ mode, ...evidence.normalized_result, output: evidence.normalized_result.output.length > 100 ? "<large output>" : evidence.normalized_result.output, output_bytes: Buffer.byteLength(evidence.normalized_result.output) })); if (mode === "whitespace") expect(evidence.normalized_result.output).toBe("value"); if (mode === "framed-whitespace") expect(evidence.normalized_result.output).toBe("value \n"); } finally { injected.control.exit(); await Promise.allSettled([client.close(), server.close()]); await manager.closeAll().catch(() => undefined); } }); } ``` </details> Each case ran once in the reduced probe after the same observations in a broader probe. Responses occur after command dispatch, excluding unsolicited startup frames. ### Expected behavior Return the command prefix exactly while excluding only framing and the generated completion marker. For this fixture, retain `"value \n"` regardless of the NUL frame boundary. `src/contracts/rizinDebugToolContracts.ts:48` promises raw output, including backend output before the completion marker. `tests/boundary/providers/RizinDebugSessionCoverage.part3.test.ts:220` already exercises a coalesced-marker frame. ### Impact and scope P2. Raw command Evidence silently differs from received text while reporting complete, untruncated output. The observed case loses three bytes. Production-provider frequency is unknown. Frame boundaries belong to the provider protocol, and no general caller-controlled workaround was verified. ### REA version rea 6.5.0 at source commit 4692bb33300c7cb77b095b6cc0c33da861c1b860 ### Runtime and platform Node 24.18.0; macOS arm64 ### Target and artifact context Simulated Linux Rizin process and no-inferior cleanup fixtures; valid host fixture TTY; no target program launched. ### Interface MCP ### Earliest failure stage Capture or observation ### Relevant output or evidence The reduced MCP probe returned: ```json {"mode":"whitespace","output":"value","completion_status":"complete","output_truncated":false,"output_bytes":5} {"mode":"framed-whitespace","output":"value \n","completion_status":"complete","output_truncated":false,"output_bytes":8} ``` The coalesced input is `value`, two spaces, newline, the generated marker, newline and NUL. The control adds only a NUL after the command prefix. Both normalized and raw Evidence contain the altered text in the coalesced case. ### Typed outcome or recovery expected Retain `"value \n"` exactly in both cases and retain whitespace-only prefixes. Continue omitting the generated marker, enforcing output bounds and keeping completion unknown when no marker arrives. Trimming the separately framed control would not restore the raw-output contract. ### Analysis or suspected cause Source-derived: `src/rizin/RizinDebugSessionManager.ts:1469` applies `trimEnd()` to the entire prefix before the completion marker. `src/server/registerRizinDebugTools.ts:66` routes the request to this manager; `createRizinCommandEvidence` in the manager places the altered value in normalized and raw Evidence. ### Additional context Vitest 5.0.3. Real Rizin and Linux descriptor readback were not exercised. The cited source files are unchanged between the reproduced baseline and origin/main 6fcfc5b5e0c063300cd5278dd8bad5b1ab1374f6. The reproduction was not rerun for this submission.

bsakaguchi commented

2026-10-16 21:07:47 +00:00

Author

Owner

@XinwangTao commented on GitHub (Oct 15, 2026):

The repro makes the contract violation clear: trimEnd() over the whole prefix before the marker treats frame coalescing as content, so the same output yields different Evidence depending on the provider's framing. Since the marker itself is the only generated part, a targeted fix would split at the marker boundary and trim only that generated segment (or drop the trim entirely and let the caller handle whitespace), keeping the raw-output promise in the contracts intact. The part3 coalesced-marker test is a good place to pin the expected "value \n" retention.

@XinwangTao commented on GitHub (Oct 15, 2026): The repro makes the contract violation clear: trimEnd() over the whole prefix before the marker treats frame coalescing as content, so the same output yields different Evidence depending on the provider's framing. Since the marker itself is the only generated part, a targeted fix would split at the marker boundary and trim only that generated segment (or drop the trim entirely and let the caller handle whitespace), keeping the raw-output promise in the contracts intact. The part3 coalesced-marker test is a good place to pin the expected "value \n" retention.

bsakaguchi referenced this issue

2026-10-17 21:12:43 +00:00

[PR #1602] [MERGED] fix(browser): report SVG xlink:href links in dom_urls #2998

bsakaguchi closed this issue

2026-10-31 00:31:40 +00:00

bsakaguchi commented

2026-10-31 00:31:41 +00:00

Author

Owner

@morluto commented on GitHub (Oct 18, 2026):

Fixed in d5fc7e6ebf on main. Adapted Xander-AJ's proposal #4044 (8b33c12bf) at the current shared normal/interrupted frame parser, with verified co-author attribution.

The owner now slices the decoded prefix before the generated completion marker without trimEnd. That same unmodified prefix reaches both byte-budget accounting and raw/normalized Evidence. No downstream whitespace repair or new parser wrapper was introduced. The adjacent trim operations parse /proc fields and a typed dbg.exitkills readback; they are separate semantic parsers, not raw command-output trimming. Existing inter-frame presentation and marker exclusion semantics remain intact.

Fresh baseline: eight of 17 new MCP controls failed before the fix; nine controls passed. Final focused suite: all 56 tests across five files passed, including new MCP whitespace controls, interrupted/pending output, UTF-8 limits, frame/byte capacity and timeout Evidence. Typecheck/lint, docs checks with five conformance tests, discovery of all 1,573 files and commit/push hooks passed. Read-only producing-boundary/consumer review found no remaining blocker.

Additional confirmed consequence: whitespace must count against the output cap. The eight-byte prefix value \n now truncates at a seven-byte budget with completion unknown, and remains intact at exactly eight bytes. Before the fix it was trimmed to five bytes and incorrectly reported complete/untruncated in both controls. The 15 other cases cover coalesced/separate NUL frames, split transport writes (including multibyte UTF-8), whitespace-only/empty strings, tabs/CR/LF and Unicode, checking both raw_result and normalized_result through registered tools and the MCP SDK.

Verification scope: injected Rizin process with actual manager, registered MCP tools and SDK transport; no real Rizin binary is installed on this host. No native inferior launch/attach, Linux ptrace or Windows-provider qualification is claimed. No CPU/memory/elapsed/token performance comparison or optimization claim was made for this correctness repair. The heavy string-limit stress and profiler stayed stopped.

Upstream rz-pipe process reader corroborates retaining payload before NUL. DeepWiki has no index for that repository; source and executed boundary checks were used instead. Broader debugger/lifecycle audit remains incomplete; PR #4044 itself was not merged or declared human-ready by this direct-main fix.

@morluto commented on GitHub (Oct 18, 2026): Fixed in d5fc7e6ebf9af050081fc4c6e18b9079cfaa5cdc on main. Adapted Xander-AJ's proposal #4044 (8b33c12bf) at the current shared normal/interrupted frame parser, with verified co-author attribution. The owner now slices the decoded prefix before the generated completion marker without trimEnd. That same unmodified prefix reaches both byte-budget accounting and raw/normalized Evidence. No downstream whitespace repair or new parser wrapper was introduced. The adjacent trim operations parse /proc fields and a typed dbg.exitkills readback; they are separate semantic parsers, not raw command-output trimming. Existing inter-frame presentation and marker exclusion semantics remain intact. Fresh baseline: eight of 17 new MCP controls failed before the fix; nine controls passed. Final focused suite: all 56 tests across five files passed, including new MCP whitespace controls, interrupted/pending output, UTF-8 limits, frame/byte capacity and timeout Evidence. Typecheck/lint, docs checks with five conformance tests, discovery of all 1,573 files and commit/push hooks passed. Read-only producing-boundary/consumer review found no remaining blocker. Additional confirmed consequence: whitespace must count against the output cap. The eight-byte prefix `value \n` now truncates at a seven-byte budget with completion unknown, and remains intact at exactly eight bytes. Before the fix it was trimmed to five bytes and incorrectly reported complete/untruncated in both controls. The 15 other cases cover coalesced/separate NUL frames, split transport writes (including multibyte UTF-8), whitespace-only/empty strings, tabs/CR/LF and Unicode, checking both raw_result and normalized_result through registered tools and the MCP SDK. Verification scope: injected Rizin process with actual manager, registered MCP tools and SDK transport; no real Rizin binary is installed on this host. No native inferior launch/attach, Linux ptrace or Windows-provider qualification is claimed. No CPU/memory/elapsed/token performance comparison or optimization claim was made for this correctness repair. The heavy string-limit stress and profiler stayed stopped. Upstream [rz-pipe process reader](https://github.com/rizinorg/rz-pipe/blob/master/python/rzpipe/open_sync.py#L121-L141) corroborates retaining payload before NUL. DeepWiki has no index for that repository; source and executed boundary checks were used instead. Broader debugger/lifecycle audit remains incomplete; PR #4044 itself was not merged or declared human-ready by this direct-main fix.

bsakaguchi commented

2026-10-31 00:31:41 +00:00

Author

Owner

@XinwangTao commented on GitHub (Oct 18, 2026):

Clean fix — slicing the decoded prefix at the marker boundary instead of trimEnd-ing the whole frame matches the raw-output contract far better, and making the preserved whitespace count against the output cap is the right accounting: the coalesced case was silently dropping bytes while still claiming complete/untruncated, so treating those bytes as real payload keeps the two controls honest. The coverage matrix (coalesced vs separate NUL frames, split multibyte writes, whitespace-only/empty, tabs/CR/LF) also directly pins the framing-independence that the original repro exposed.

@XinwangTao commented on GitHub (Oct 18, 2026): Clean fix — slicing the decoded prefix at the marker boundary instead of trimEnd-ing the whole frame matches the raw-output contract far better, and making the preserved whitespace count against the output cap is the right accounting: the coalesced case was silently dropping bytes while still claiming complete/untruncated, so treating those bytes as real payload keeps the two controls honest. The coverage matrix (coalesced vs separate NUL frames, split multibyte writes, whitespace-only/empty, tabs/CR/LF) also directly pins the framing-independence that the original repro exposed.

bsakaguchi referenced this issue

2026-10-31 00:54:41 +00:00

[PR #1683] [MERGED] fix(browser): resolve XLink and SVG prefixes in XHTML dom_urls #4752

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

No description provided.

Rows
Columns