morluto/rea

Fork 0

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

[PR #4464] [CLOSED] fix(rizin): bound recent frame history by UTF-8 bytes #6257

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4464
Author: @shreyans-chowdry
Created: 10/16/2026
Status: ❌ Closed

Base: main ← Head: fix/rizin-recent-frame-utf8-bound

📝 Commits (2)

3a494cd fix(rizin): bound recent frame history by UTF-8 bytes
7bc8687 fix(rizin): preserve bounded decoded history and deterministic coverage

📊 Changes

2 files changed (+65 additions, -16 deletions)

View changed files

📝 src/rizin/RizinDebugSessionManager.ts (+7 -6)
📝 tests/boundary/providers/RizinDebugSessionCoverage.part4.test.ts (+58 -10)

📄 Description

Summary

Rizin recent-output history now retains at most 16 KiB of decoded UTF-8 text per frame, plus the existing truncation marker, without cutting a code point or dropping a literal U+FFFD.

Problem and expected behavior

The original history path compared raw byte length but sliced UTF-16 code units. The initial PR switched to truncateUtf8, which re-encoded the entire frame and stripped a terminal U+FFFD even when it was legitimate source text. Invalid raw bytes could also expand past the history limit during decoding without crossing the raw-byte threshold.

History should bound the decoded representation it actually retains, preserve every complete code point that fits, and expose truncation. There is no linked issue: this is a directly reproduced correction to the existing history limiter.

Change and scope

Use the existing boundedUtf8Text owner at history delivery and derive the history marker from its result or protocol truncation. Replace the ASCII/non-ASCII sleep-based cases with synchronous injected-frame coverage for ASCII, two-/three-/four-byte code points, an exact limit, a literal replacement character at the boundary, and invalid bytes that expand when decoded.

This is an ordinary correction within existing debugger-output retention boundaries; a new maintainer scope decision is not applicable. Existing command-output limits and their separate helper are outside this recent-history change. No new launch, command, target, or mutation authority is introduced.

Contract and boundary impact

Semantic owner and earliest changed stage: RizinDebugSessionManager.#deliverFrame, where decoded frames enter recent history.
Contribution scope fit and linked maintainer decision: existing debugger observation/retention correction; not applicable.
CLI and MCP/tool-catalog contract: schemas unchanged; recent_output respects its existing byte bound and keeps complete source characters.
Provider, bridge, target-format, or platform compatibility: no protocol or launch changes; boundary tests ran on macOS arm64.
Evidence, artifact, provenance, or reconstruction contract: raw command frames and provenance remain unchanged; history truncation remains explicit.
Target selection, authority, upstream behavior, and cleanup: existing authority and cleanup are preserved.
Generated metadata, package, or installation impact: none.

Expected effects of this change, within the existing runtime workflow:

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.

Evidence and regression coverage

At the previous PR head 3a494cdf393e504bb736043ec1ecb006c3c3bf6f, the revised part4 test file produced 2 failures and 15 passes: the literal-U+FFFD boundary and invalid-byte expansion cases failed.
The same file passes after the production correction. Injected frame delivery completes synchronously after session startup, removing the arbitrary 100 ms delay and retaining the existing ASCII assertion.
The broader fake-provider and MCP suite passed: 86 tests passed and 2 existing tests were skipped across 8 files.
Initial baseline execution also exposed seven macOS compiler-cache sandbox failures; after granting the compiler-cache access, the baseline isolated the two product regressions above. Those environmental failures were not suppressed or counted as passing.
A separate MCP probe through the registered tools and in-memory SDK transport, with a fake provider, observed recent_output_truncated: true. For an input of 6,000 界 characters, the retained entry was exactly 5,461 界 characters (16,383 UTF-8 bytes) followed by [recent frame truncated]; the untruncated command output remained available.
Real Rizin is not installed in this environment. The runs use existing injected/fake providers, and do not establish real-engine behavior or Linux/Windows coverage. The full deterministic suite was not run.

For evidence-bearing changes:

Observed, derived, and inferred claims remain distinguishable.
Artifact identity, source provenance, and failed attempts remain preserved.
Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

Node 24.18.0 / npm 11.16.0, macOS arm64:

npm run test:focused -- tests/boundary/providers/RizinDebugSessionCoverage.part1.test.ts tests/boundary/providers/RizinDebugSessionCoverage.part2.test.ts tests/boundary/providers/RizinDebugSessionCoverage.part3.test.ts tests/boundary/providers/RizinDebugSessionCoverage.part4.test.ts tests/boundary/providers/RizinDebugSessionCoverage.part5.test.ts tests/boundary/providers/RizinDebugSessionReviewRegressions.test.ts tests/boundary/providers/RizinDebugSessionManagers.test.ts tests/boundary/mcp/rizinDebuggerCancellationMcp.test.ts — 86 passed, 2 skipped; 8 files passed.
npm run check — passed typecheck, lint, formatting, unused-code checks, metadata, module boundaries, and architecture guards.
git diff --check — passed.

History-prefix microbenchmark (not a complete provider benchmark)

Separate fresh Node processes compared the previous encode/slice/regex prefix routine with boundedUtf8Text. A 16 MiB ASCII string was pre-created and flattened, then a 16 KiB prefix was retained once cold and 20 times warm. One local sample per implementation:

Measurement	Previous PR	Revised
Cold prefix	3.64 ms	1.17 ms
20 warm prefixes	172.50 ms	13.35 ms
CPU user/system, measured region	106.33 / 18.93 ms	16.47 / 0.98 ms
Peak whole-process RSS	188,240 KiB	81,456 KiB

For the expanded replacement-text case (48 MiB decoded text representing 16 MiB invalid input), 20 warm prefixes took 1,741.78 ms versus 25.20 ms; peak process RSS was 277,184 versus 97,904 KiB. The revised reader stops at the prefix budget instead of allocating an encoding of the whole decoded frame. Peak RSS includes process startup and imports; these single-process samples are not process-family or complete session measurements. End-to-end provider time, agent tokens, and multi-process peak memory were not measured.

Review follow-up

Timing findings (Greptile, Codex): replaced fixed sleeps with acknowledged startup followed by synchronous injected output.
Literal replacement-character finding: boundedUtf8Text preserves valid U+FFFD code points; the new regression fails on the previous PR head.
Full-frame re-encoding finding: the history path uses the existing early-stopping reader, with the bounded-prefix measurements above. It no longer invokes the command-output helper.
Remaining related scope: the pre-existing command-output helper is unchanged; this PR corrects recent-history retention only.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: macOS fake-provider and MCP boundaries only; real Rizin and Linux/Windows remain unverified.
Package or release metadata impact: none.
Security, privacy, process, or containment review: no additional authority or lifecycle changes; decoded history remains bounded and truncation visible.

Author readiness

Human owner: @shreyans-chowdry

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.

The checked Author readiness declaration was already present on this PR; the human owner is now named and the evidence above is current. This follow-up was prepared with OpenAI Codex assistance. Upstream CI still requires maintainer approval before execution.

_{🔄 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/4464 **Author:** [@shreyans-chowdry](https://github.com/shreyans-chowdry) **Created:** 10/16/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/rizin-recent-frame-utf8-bound` --- ### 📝 Commits (2) - [`3a494cd`](https://github.com/morluto/rea/commit/3a494cdf393e504bb736043ec1ecb006c3c3bf6f) fix(rizin): bound recent frame history by UTF-8 bytes - [`7bc8687`](https://github.com/morluto/rea/commit/7bc86870bcc5e75577d8f81933c74281585176b8) fix(rizin): preserve bounded decoded history and deterministic coverage ### 📊 Changes **2 files changed** (+65 additions, -16 deletions) <details> <summary>View changed files</summary> 📝 `src/rizin/RizinDebugSessionManager.ts` (+7 -6) 📝 `tests/boundary/providers/RizinDebugSessionCoverage.part4.test.ts` (+58 -10) </details> ### 📄 Description ## Summary Rizin recent-output history now retains at most 16 KiB of decoded UTF-8 text per frame, plus the existing truncation marker, without cutting a code point or dropping a literal U+FFFD. ## Problem and expected behavior The original history path compared raw byte length but sliced UTF-16 code units. The initial PR switched to `truncateUtf8`, which re-encoded the entire frame and stripped a terminal U+FFFD even when it was legitimate source text. Invalid raw bytes could also expand past the history limit during decoding without crossing the raw-byte threshold. History should bound the decoded representation it actually retains, preserve every complete code point that fits, and expose truncation. There is no linked issue: this is a directly reproduced correction to the existing history limiter. ## Change and scope Use the existing `boundedUtf8Text` owner at history delivery and derive the history marker from its result or protocol truncation. Replace the ASCII/non-ASCII sleep-based cases with synchronous injected-frame coverage for ASCII, two-/three-/four-byte code points, an exact limit, a literal replacement character at the boundary, and invalid bytes that expand when decoded. This is an ordinary correction within existing debugger-output retention boundaries; a new maintainer scope decision is not applicable. Existing command-output limits and their separate helper are outside this recent-history change. No new launch, command, target, or mutation authority is introduced. ## Contract and boundary impact - Semantic owner and earliest changed stage: `RizinDebugSessionManager.#deliverFrame`, where decoded frames enter recent history. - Contribution scope fit and linked maintainer decision: existing debugger observation/retention correction; not applicable. - CLI and MCP/tool-catalog contract: schemas unchanged; `recent_output` respects its existing byte bound and keeps complete source characters. - Provider, bridge, target-format, or platform compatibility: no protocol or launch changes; boundary tests ran on macOS arm64. - Evidence, artifact, provenance, or reconstruction contract: raw command frames and provenance remain unchanged; history truncation remains explicit. - Target selection, authority, upstream behavior, and cleanup: existing authority and cleanup are preserved. - Generated metadata, package, or installation impact: none. Expected effects of this change, within the existing runtime workflow: - [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. ## Evidence and regression coverage - At the previous PR head `3a494cdf393e504bb736043ec1ecb006c3c3bf6f`, the revised part4 test file produced 2 failures and 15 passes: the literal-U+FFFD boundary and invalid-byte expansion cases failed. - The same file passes after the production correction. Injected frame delivery completes synchronously after session startup, removing the arbitrary 100 ms delay and retaining the existing ASCII assertion. - The broader fake-provider and MCP suite passed: 86 tests passed and 2 existing tests were skipped across 8 files. - Initial baseline execution also exposed seven macOS compiler-cache sandbox failures; after granting the compiler-cache access, the baseline isolated the two product regressions above. Those environmental failures were not suppressed or counted as passing. - A separate MCP probe through the registered tools and in-memory SDK transport, with a fake provider, observed `recent_output_truncated: true`. For an input of 6,000 `界` characters, the retained entry was exactly 5,461 `界` characters (16,383 UTF-8 bytes) followed by `[recent frame truncated]`; the untruncated command output remained available. - Real Rizin is not installed in this environment. The runs use existing injected/fake providers, and do not establish real-engine behavior or Linux/Windows coverage. The full deterministic suite was not run. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed Node 24.18.0 / npm 11.16.0, macOS arm64: - `npm run test:focused -- tests/boundary/providers/RizinDebugSessionCoverage.part1.test.ts tests/boundary/providers/RizinDebugSessionCoverage.part2.test.ts tests/boundary/providers/RizinDebugSessionCoverage.part3.test.ts tests/boundary/providers/RizinDebugSessionCoverage.part4.test.ts tests/boundary/providers/RizinDebugSessionCoverage.part5.test.ts tests/boundary/providers/RizinDebugSessionReviewRegressions.test.ts tests/boundary/providers/RizinDebugSessionManagers.test.ts tests/boundary/mcp/rizinDebuggerCancellationMcp.test.ts` — 86 passed, 2 skipped; 8 files passed. - `npm run check` — passed typecheck, lint, formatting, unused-code checks, metadata, module boundaries, and architecture guards. - `git diff --check` — passed. <details> <summary>History-prefix microbenchmark (not a complete provider benchmark)</summary> Separate fresh Node processes compared the previous encode/slice/regex prefix routine with `boundedUtf8Text`. A 16 MiB ASCII string was pre-created and flattened, then a 16 KiB prefix was retained once cold and 20 times warm. One local sample per implementation: | Measurement | Previous PR | Revised | | --- | ---: | ---: | | Cold prefix | 3.64 ms | 1.17 ms | | 20 warm prefixes | 172.50 ms | 13.35 ms | | CPU user/system, measured region | 106.33 / 18.93 ms | 16.47 / 0.98 ms | | Peak whole-process RSS | 188,240 KiB | 81,456 KiB | For the expanded replacement-text case (48 MiB decoded text representing 16 MiB invalid input), 20 warm prefixes took 1,741.78 ms versus 25.20 ms; peak process RSS was 277,184 versus 97,904 KiB. The revised reader stops at the prefix budget instead of allocating an encoding of the whole decoded frame. Peak RSS includes process startup and imports; these single-process samples are not process-family or complete session measurements. End-to-end provider time, agent tokens, and multi-process peak memory were not measured. </details> ## Review follow-up - Timing findings ([Greptile](https://github.com/morluto/rea/pull/4464#discussion_r4294675674), [Codex](https://github.com/morluto/rea/pull/4464#discussion_r4294698859)): replaced fixed sleeps with acknowledged startup followed by synchronous injected output. - [Literal replacement-character finding](https://github.com/morluto/rea/pull/4464#discussion_r4294698868): `boundedUtf8Text` preserves valid U+FFFD code points; the new regression fails on the previous PR head. - [Full-frame re-encoding finding](https://github.com/morluto/rea/pull/4464#discussion_r4294698876): the history path uses the existing early-stopping reader, with the bounded-prefix measurements above. It no longer invokes the command-output helper. - Remaining related scope: the pre-existing command-output helper is unchanged; this PR corrects recent-history retention only. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: macOS fake-provider and MCP boundaries only; real Rizin and Linux/Windows remain unverified. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no additional authority or lifecycle changes; decoded history remains bounded and truncation visible. ## Author readiness Human owner: @shreyans-chowdry - [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. The checked Author readiness declaration was already present on this PR; the human owner is now named and the evidence above is current. This follow-up was prepared with OpenAI Codex assistance. Upstream CI still requires maintainer approval before execution. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

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

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

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

[PR #6257] [MERGED] feat(native): decode Swift nominal type declarations from __swift5_types #7231

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

No description provided.

Rows
Columns