[PR #5246] [MERGED] fix(rizin): bound recent history frames by UTF-8 byte budget #6834

Closed
opened 2026-11-01 21:43:22 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5246
Author: @Sarthak-Pandey
Created: 10/19/2026
Status: ✅ Merged
Merged: 10/19/2026
Merged by: @gensokyo-dev

Base: main ← Head: fix/rizin-cjk-recent-output-budget


📝 Commits (3)

  • 4f2234a fix(rizin): bound recent history frames by UTF-8 byte budget
  • d3ce950 chore(rizin): sync latest main for history validation
  • a2f346a fix(rizin): share bounded UTF-8 prefixes across captures

📊 Changes

3 files changed (+125 additions, -32 deletions)

View changed files

📝 docs/reverse-engineering-tools.md (+2 -0)
📝 src/rizin/RizinDebugSessionManager.ts (+17 -22)
📝 tests/boundary/providers/RizinDebugSessionCoverage.part4.test.ts (+106 -10)

📄 Description

Summary

Fixes #5091 and #5242. Rizin recent history used UTF-16 positions for a 16 KiB UTF-8 budget. Real Rizin 0.9.1 reproduced 49,176 B retained for a 60,000 B CJK frame, a split emoji surrogate, and 18,000 B of decoded replacement characters from a 6,000 B invalid frame.

History now retains a complete UTF-8 prefix of at most 16 KiB plus the existing 24 B marker. Full decoded command output, per-command invalid-UTF-8 Evidence, session uncertainty, wire-byte queue accounting and ownership/cleanup policy are preserved.

Problem and expected behavior

Keep bounded recent history without shortening complete command Evidence. Exact CJK/emoji/invalid-byte prefixes are verified independently of the truncation implementation. Status history is still capped at 64 protocol frames; this change adds no query surface or cache.

Change and scope

Reuse the manager's truncateUtf8 for history, startup diagnostics and capped command output. Encode directly into a budget-sized buffer using Node's complete-character Buffer.write, avoiding duplicate boundary walkers and whole-frame temporary UTF-8 buffers for a small history prefix. Protocol assembly and full command Evidence still consume memory separately.

Replace the fixed-delay history tests with nine deterministic injected-stream cases, checking exact prefixes, truncation/decoding flags, complete command Evidence and unsolicited frames. Preserve oversized-frame, ring-capacity, cancellation and lifecycle coverage. Document the existing history budget.

Ordinary fix within the existing provider boundary; no new execution authority, dependency, installer or generated contract change.

Contract and boundary impact

  • Owner: RizinDebugSessionManager decoded-text prefix retention.
  • CLI/MCP schemas and tool catalog: unchanged.
  • Real verification: Linux x86-64, official Rizin 0.9.1 static build; other platforms unverified. Persistent Rizin debugging remains Linux-only.
  • Source-controlled sleep/exit executable used for lifecycle control; controlled CJK/emoji/invalid-byte files were read through native Rizin !cat. This is real-provider conformance, not a claim about complex-software investigation quality.

Expected effects:

  • Read existing files or captured data.
  • Launch or attach to a process.
  • Inject instrumentation or run scripts/code.
  • Change process memory, registers, files, or configuration.
  • Contact network services.
  • Recover protected code or work around protections.
  • Documentation or usability only; no target interaction.
  • Not sure yet; investigation or maintainer guidance needed.

The patch changes capture retention only; real-provider verification exercised existing launch/console/close operations.

Evidence and regression coverage

Executed on a2f346a36, integrating latest main b44cb5bd3:

  • npm run test:focused -- the 13 Rizin provider/MCP files: 151 passed, 1 skipped.
  • npm run check: all six tasks and architecture guards passed.
  • Rechecked the two upstream JavaScript CI regression files after main's fix: 42 passed; no unrelated test changes in this PR.
  • npm run docs:check: six tasks and five README tests passed.
  • Actual stdio MCP: four command/status cases, verified full decoded raw/normalized command output, loss flags, launch provenance, close and complete retained Evidence; actual CLI CJK output matched. Target digest unchanged, owned process-family cleanup verified, OOM kills 0.

Real CJK history: 49,176 → 16,407 B; emoji history now well-formed; invalid UTF-8 expansion is also bounded. A short-lived /usr/bin/true close attempt on unchanged main failed and recovered on explicit retry; recorded separately in #4227, excluded from the completed comparison.

Validation performed

Follow-up implementation/review and measurements by @gensokyo-dev; original author readiness declaration below is retained.

One matched before/after verification pair (fresh SDK/server, warm OS file cache, CPU1): elapsed 19.257 → 19.906 s, CPU 15.703 → 16.542 s, process-family peak 820.4 → 800.3 MiB. These observations do not establish a general speedup or memory improvement. All jobs used an 8 GiB maximum (public proofs 2 GiB); largest validation peak 4.51 GiB. Counters were observed at payload exit while the observer remained alive, not final whole-scope counters.

tiktoken 0.12.0 / cl100k_base estimates: first CJK status raw SDK reply 65,767 → 22,091; whole recorded MCP sequence including discovery, requests/replies, all four statuses and complete Evidence 1,447,088 → 1,272,043. The unchanged discovery alone is 807,510. Text/structured duplication is counted; protocol/init framing, actual client rendering and actual model usage are unmeasured. CLI conformance is separate from this token estimate.

Compatibility, safety, and release

No breaking API or package metadata changes. Buffer decoding uses only initialized bytes; raw wire queue charges and full command-result semantics remain unchanged. No new cache or retained inventory.

Author readiness

Human owner: @Sarthak-Pandey

  • 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.

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
  • 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/5246 **Author:** [@Sarthak-Pandey](https://github.com/Sarthak-Pandey) **Created:** 10/19/2026 **Status:** ✅ Merged **Merged:** 10/19/2026 **Merged by:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Base:** `main` ← **Head:** `fix/rizin-cjk-recent-output-budget` --- ### 📝 Commits (3) - [`4f2234a`](https://github.com/morluto/rea/commit/4f2234ab4138ecd54ccdabc8c7c61296a9848198) fix(rizin): bound recent history frames by UTF-8 byte budget - [`d3ce950`](https://github.com/morluto/rea/commit/d3ce9502ce7585fffe65da48554fab33f9834d1c) chore(rizin): sync latest main for history validation - [`a2f346a`](https://github.com/morluto/rea/commit/a2f346a3622bc70c7d5e27f24419f4b8ab0b2377) fix(rizin): share bounded UTF-8 prefixes across captures ### 📊 Changes **3 files changed** (+125 additions, -32 deletions) <details> <summary>View changed files</summary> 📝 `docs/reverse-engineering-tools.md` (+2 -0) 📝 `src/rizin/RizinDebugSessionManager.ts` (+17 -22) 📝 `tests/boundary/providers/RizinDebugSessionCoverage.part4.test.ts` (+106 -10) </details> ### 📄 Description ## Summary Fixes #5091 and #5242. Rizin recent history used UTF-16 positions for a 16 KiB UTF-8 budget. Real Rizin 0.9.1 reproduced 49,176 B retained for a 60,000 B CJK frame, a split emoji surrogate, and 18,000 B of decoded replacement characters from a 6,000 B invalid frame. History now retains a complete UTF-8 prefix of at most 16 KiB plus the existing 24 B marker. Full decoded command output, per-command invalid-UTF-8 Evidence, session uncertainty, wire-byte queue accounting and ownership/cleanup policy are preserved. ## Problem and expected behavior Keep bounded recent history without shortening complete command Evidence. Exact CJK/emoji/invalid-byte prefixes are verified independently of the truncation implementation. Status history is still capped at 64 protocol frames; this change adds no query surface or cache. ## Change and scope Reuse the manager's `truncateUtf8` for history, startup diagnostics and capped command output. Encode directly into a budget-sized buffer using Node's complete-character `Buffer.write`, avoiding duplicate boundary walkers and whole-frame temporary UTF-8 buffers for a small history prefix. Protocol assembly and full command Evidence still consume memory separately. Replace the fixed-delay history tests with nine deterministic injected-stream cases, checking exact prefixes, truncation/decoding flags, complete command Evidence and unsolicited frames. Preserve oversized-frame, ring-capacity, cancellation and lifecycle coverage. Document the existing history budget. Ordinary fix within the existing provider boundary; no new execution authority, dependency, installer or generated contract change. ## Contract and boundary impact - Owner: `RizinDebugSessionManager` decoded-text prefix retention. - CLI/MCP schemas and tool catalog: unchanged. - Real verification: Linux x86-64, official Rizin 0.9.1 static build; other platforms unverified. Persistent Rizin debugging remains Linux-only. - Source-controlled sleep/exit executable used for lifecycle control; controlled CJK/emoji/invalid-byte files were read through native Rizin `!cat`. This is real-provider conformance, not a claim about complex-software investigation quality. Expected effects: - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. The patch changes capture retention only; real-provider verification exercised existing launch/console/close operations. ## Evidence and regression coverage Executed on `a2f346a36`, integrating latest main `b44cb5bd3`: - `npm run test:focused --` the 13 Rizin provider/MCP files: **151 passed, 1 skipped**. - `npm run check`: all six tasks and architecture guards passed. - Rechecked the two upstream JavaScript CI regression files after main's fix: **42 passed**; no unrelated test changes in this PR. - `npm run docs:check`: six tasks and five README tests passed. - Actual stdio MCP: four command/status cases, verified full decoded raw/normalized command output, loss flags, launch provenance, close and complete retained Evidence; actual CLI CJK output matched. Target digest unchanged, owned process-family cleanup verified, OOM kills 0. Real CJK history: **49,176 → 16,407 B**; emoji history now well-formed; invalid UTF-8 expansion is also bounded. A short-lived `/usr/bin/true` close attempt on unchanged main failed and recovered on explicit retry; recorded separately in [#4227](https://github.com/morluto/rea/issues/4227#issuecomment-6273199435), excluded from the completed comparison. ## Validation performed Follow-up implementation/review and measurements by @gensokyo-dev; original author readiness declaration below is retained. One matched before/after verification pair (fresh SDK/server, warm OS file cache, CPU1): elapsed **19.257 → 19.906 s**, CPU **15.703 → 16.542 s**, process-family peak **820.4 → 800.3 MiB**. These observations do **not** establish a general speedup or memory improvement. All jobs used an 8 GiB maximum (public proofs 2 GiB); largest validation peak 4.51 GiB. Counters were observed at payload exit while the observer remained alive, not final whole-scope counters. `tiktoken 0.12.0 / cl100k_base` estimates: first CJK status raw SDK reply **65,767 → 22,091**; whole recorded MCP sequence including discovery, requests/replies, all four statuses and complete Evidence **1,447,088 → 1,272,043**. The unchanged discovery alone is 807,510. Text/structured duplication is counted; protocol/init framing, actual client rendering and actual model usage are unmeasured. CLI conformance is separate from this token estimate. ## Compatibility, safety, and release No breaking API or package metadata changes. Buffer decoding uses only initialized bytes; raw wire queue charges and full command-result semantics remain unchanged. No new cache or retained inventory. ## Author readiness Human owner: @Sarthak-Pandey - [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. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [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-11-01 21:43:22 +00:00
Sign in to join this conversation.
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#6834
No description provided.