morluto/rea

Fork 0

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

[PR #4459] [MERGED] fix(rizin): retain interrupted command frames #6254

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4459
Author: @N0zoM1z0
Created: 10/16/2026
Status: ✅ Merged
Merged: 10/16/2026
Merged by: @N0zoM1z0

Base: main ← Head: fix/rizin-pending-output-4015

📝 Commits (3)

816c425 fix(rizin): retain interrupted command frames
9d67605 fix(rizin): drain exited output before capturing evidence
0e480fb fix(rizin): retain captures across interruption precedence

📊 Changes

12 files changed (+784 additions, -280 deletions)

View changed files

📝 scripts/ci/scopes.mjs (+1 -0)
📝 src/rizin/RizinDebugSessionManager.ts (+284 -255)
📝 stryker.semantics.config.mjs (+2 -1)
📝 tests/boundary/filesystem/ciChangeScope.test.ts (+1 -0)
➕ tests/boundary/mcp/rizinPendingOutput.test.ts (+468 -0)
📝 tests/boundary/providers/RizinCommandTimeout.test.ts (+1 -18)
📝 tests/boundary/providers/RizinDebugSessionCoverage.part3.test.ts (+6 -1)
📝 tests/boundary/providers/RizinTestInferiorTty.ts (+17 -5)
📝 tests/boundary/providers/RizinTimeoutEvidence.test.ts (+1 -0)
📝 tsconfig.reverse-tools-mutation.json (+1 -0)
📝 vitest.reverse-tools-semantics.config.ts (+1 -0)
📝 vitest.reverse-tools.config.ts (+1 -0)

📄 Description

Summary

Fixes #4015; follow-up to #4227.

Rizin command interruption retained completed NUL frames but discarded bytes already received in an unfinished frame. It could also discard queued frames on cancellation. Exit, deadline and cancellation snapshots now retain those diagnostics with bounded decoded output and truthful unknown completion.

Change and boundary impact

Owner: RizinDebugSessionManager receiver and command capture. One buffered-frame decoder serves normal, startup and interrupted reads; one collector applies separators, byte limits, private-marker handling and UTF-8 facts.
Reject unclaimed complete/unfinished output before dispatch and cleanup readback, preventing older bytes from becoming a new command's evidence. Oversized unfinished frames publish their bounded prefix immediately. Preserve valid replacement characters at UTF-8 truncation boundaries; remove the superseded timeout-frame factory/test.
On exit, wait through the existing supervisor's bounded output-settlement interval before capture. Unverified stream closure remains truncated with retained cleanup ownership. Incomplete marker prefixes remain literal evidence with an explicit ambiguity limitation; removing them could delete real diagnostics.
Captures return a tagged Result independently of cancellation/protocol/cleanup precedence. Queue drops are recorded where bytes are rejected; active partial frames survive stdin failures. Superseded payload-bearing timeout/cancellation exception classes are removed.
Shared manager and Evidence projection remain the implementation boundary. CLI/MCP schemas, dependencies, installation and generated metadata are unchanged. Original launch paths remain provenance, while current debugger subject identity stays explicitly unknown.
Scope decision: not applicable; ordinary fix to existing capture semantics. Existing unrestricted-command effects and cleanup authority remain in force; failed cleanup retains its owner.

Expected effects under existing caller-selected commands:

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.

Evidence and regression coverage

Executed on Linux / Node24.18.0 / npm11.16.0:

Fresh main fe2a398dd: new registered-MCP/owner selection 8 failed,1 framed control passed. Covers unterminated exit/deadline, queued cancellation, UTF-8/decoded byte budgets, pre-command leftovers and an oversized unterminated frame. An existing zero-deadline assertion was corrected to retain the third already-received frame.
Final npm run test:focused -- <15 Rizin/configuration files>: 353 passed,1 existing skip. Full npm run check:ci and npm run docs:check passed; docs selection5 passed. CI initially found a stale Stryker reference to the removed factory; mutation targets and regression selections now follow the actual collector. Final focused snapshot mutation sweep:4 killed,2 rejected by TypeScript,0 survivors; not a full-project mutation claim.
Real official Linux Rizin0.9.1, unchanged source-owned C fixture: receiver observes REA_NATIVE_PENDING before NUL. Production-manager deadline/exit probes return empty text on main and the exact text after repair. Fault deadline is500ms rather than production120s; exit is injected into the exact owned debugger after real bytes arrive. No native stdout bytes/errors were substituted.
Review follow-up: two MCP cases failed on the previous head when stdout arrived after leader exit; the fix passes these plus a never-closing-output control. A genuine Rizin probe pauses actual stdout, observes owned-process exit before byte delivery, resumes the stream and retains the native diagnostic. The shared PTY fixture now belongs to each test and awaits native termination, avoiding cached closed paths across shared MCP workers.
Final capture-result review follow-up: all3 cancellation/queue-overflow races failed on 9d6760548;353 final owner/MCP/config tests pass, including stdin failure before/during waiting and byte-based queue drops. Actual Rizin exit-before-delivery plus cancellation during drainage now preserves the exact native diagnostic through the cleanup-error cause and partial Evidence. This is a real-provider boundary probe, not public SDK cancellation verification.
Fault probes first finish the controlled inferior naturally and confirm empty native IO inventory. Existing cleanup_incomplete remains explicit; test teardown uses the exact launched debugger after that independent proof. No automatic public recovery claim for these fault cases.
Actual compiled CLI/stdio MCP, default deadlines/cleanup: two paired completed runs per revision, each1CLI+11MCP requests, two explicit close/reopen cycles and active-session client shutdown. Caller TTY remains durable until detached fixture completion; unrelated rizin-named control survives; source ELF remains unchanged and final lineage is verified empty.
Actual SDK-converted catalog:207 tools/414 schemas passed2020-12 meta-schema validation. Other OSes/backends and the full120s native fault deadline were not tested.

Performance

Same host/two-core affinity; OS cache uncontrolled; no speedup claim.

Completed workflow	Base → fix wall /CPU /sampled family RSS
Named contracts,2 runs each (medians)	6.62→6.75s /7.02→7.42s /0.717→0.717GiB
Includes actual initialize/catalog discovery,1 run each	27.05→14.85s /30.91→16.85s /1.025→1.013GiB

Final reviewed/tested head: 0e480fb83. These are shared-host observations with uncontrolled caches, frequency and scheduling; the discovery timing difference is not attributed to this Rizin fix. Final source ELFs remained unchanged and all completed public runs verified empty final lineage.

Named-contract first/later session starts:315/303→323/310ms; first/later commands18.1/10.4→20.1/10.2ms. No new cache or normal-response payload is introduced.

o200k_base estimates: named-contract11 tool arguments/full SDK replies5028→4959 tokens, CLI arguments/reply1093 each. The complete discovery workflow records27 JSON-RPC messages including initialize, initialized notification, tools/list and11 tool exchanges: 836,209→836,295 tokens including CLI. Catalogs are identical and account for829,341 tokens; the four used tool advertisements alone are4219 tokens. These are serialized-wire proxies, including text/structured duplicates, not actual model billing or proof that a client feeds the whole catalog to its model. Discovery-enabled first start measured12.4/6.5s; initial timings did not establish the cause. A later actual-task profile measured207 client validator-preparation calls taking6.54s; this separate SDK cost is already tracked in #1065/T5 and is not attributed to this fix. No retries; staging/fixture compilation excluded. RSS sampled200ms; CPU includes reaped descendants only. Earlier shell-child CPU is excluded by delta measurement.

Compatibility, safety, and release

No breaking change, migration or new target authority. Unknown completion, cleanup uncertainty, output limits and provenance remain explicit. Controlled native fixture execution/owned-process exit injection were performed only for verification. No binary, provider workspace, build output or local test artifacts are committed.

Author readiness

Human owner: @N0zoM1z0

Entered under the human owner's explicit working-session instruction to complete the declarations and request review. The assistant performed implementation and validation review; this does not claim a separate manual human review.

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/4459 **Author:** [@N0zoM1z0](https://github.com/N0zoM1z0) **Created:** 10/16/2026 **Status:** ✅ Merged **Merged:** 10/16/2026 **Merged by:** [@N0zoM1z0](https://github.com/N0zoM1z0) **Base:** `main` ← **Head:** `fix/rizin-pending-output-4015` --- ### 📝 Commits (3) - [`816c425`](https://github.com/morluto/rea/commit/816c425d08d81c2805e38bc835da26717490b534) fix(rizin): retain interrupted command frames - [`9d67605`](https://github.com/morluto/rea/commit/9d676054858f963c817feb691a6afc71f50506f4) fix(rizin): drain exited output before capturing evidence - [`0e480fb`](https://github.com/morluto/rea/commit/0e480fb839708db27a04eb22e3f7966734303519) fix(rizin): retain captures across interruption precedence ### 📊 Changes **12 files changed** (+784 additions, -280 deletions) <details> <summary>View changed files</summary> 📝 `scripts/ci/scopes.mjs` (+1 -0) 📝 `src/rizin/RizinDebugSessionManager.ts` (+284 -255) 📝 `stryker.semantics.config.mjs` (+2 -1) 📝 `tests/boundary/filesystem/ciChangeScope.test.ts` (+1 -0) ➕ `tests/boundary/mcp/rizinPendingOutput.test.ts` (+468 -0) 📝 `tests/boundary/providers/RizinCommandTimeout.test.ts` (+1 -18) 📝 `tests/boundary/providers/RizinDebugSessionCoverage.part3.test.ts` (+6 -1) 📝 `tests/boundary/providers/RizinTestInferiorTty.ts` (+17 -5) 📝 `tests/boundary/providers/RizinTimeoutEvidence.test.ts` (+1 -0) 📝 `tsconfig.reverse-tools-mutation.json` (+1 -0) 📝 `vitest.reverse-tools-semantics.config.ts` (+1 -0) 📝 `vitest.reverse-tools.config.ts` (+1 -0) </details> ### 📄 Description ## Summary Fixes #4015; follow-up to #4227. Rizin command interruption retained completed NUL frames but discarded bytes already received in an unfinished frame. It could also discard queued frames on cancellation. Exit, deadline and cancellation snapshots now retain those diagnostics with bounded decoded output and truthful unknown completion. ## Change and boundary impact - Owner: `RizinDebugSessionManager` receiver and command capture. One buffered-frame decoder serves normal, startup and interrupted reads; one collector applies separators, byte limits, private-marker handling and UTF-8 facts. - Reject unclaimed complete/unfinished output before dispatch and cleanup readback, preventing older bytes from becoming a new command's evidence. Oversized unfinished frames publish their bounded prefix immediately. Preserve valid replacement characters at UTF-8 truncation boundaries; remove the superseded timeout-frame factory/test. - On exit, wait through the existing supervisor's bounded output-settlement interval before capture. Unverified stream closure remains truncated with retained cleanup ownership. Incomplete marker prefixes remain literal evidence with an explicit ambiguity limitation; removing them could delete real diagnostics. - Captures return a tagged `Result` independently of cancellation/protocol/cleanup precedence. Queue drops are recorded where bytes are rejected; active partial frames survive stdin failures. Superseded payload-bearing timeout/cancellation exception classes are removed. - Shared manager and Evidence projection remain the implementation boundary. CLI/MCP schemas, dependencies, installation and generated metadata are unchanged. Original launch paths remain provenance, while current debugger subject identity stays explicitly unknown. - Scope decision: not applicable; ordinary fix to existing capture semantics. Existing unrestricted-command effects and cleanup authority remain in force; failed cleanup retains its owner. Expected effects under existing caller-selected commands: - [x] Read existing files or captured data. - [x] Launch or attach to a process. - [x] Inject instrumentation or run scripts/code. - [x] Change process memory, registers, files, or configuration. ## Evidence and regression coverage Executed on Linux / Node24.18.0 / npm11.16.0: - Fresh main `fe2a398dd`: new registered-MCP/owner selection **8 failed,1 framed control passed**. Covers unterminated exit/deadline, queued cancellation, UTF-8/decoded byte budgets, pre-command leftovers and an oversized unterminated frame. An existing zero-deadline assertion was corrected to retain the third already-received frame. - Final `npm run test:focused -- <15 Rizin/configuration files>`: **353 passed,1 existing skip**. Full `npm run check:ci` and `npm run docs:check` passed; docs selection5 passed. CI initially found a stale Stryker reference to the removed factory; mutation targets and regression selections now follow the actual collector. Final focused snapshot mutation sweep:4 killed,2 rejected by TypeScript,0 survivors; not a full-project mutation claim. - **Real official Linux Rizin0.9.1**, unchanged source-owned C fixture: receiver observes `REA_NATIVE_PENDING` before NUL. Production-manager deadline/exit probes return empty text on main and the exact text after repair. Fault deadline is500ms rather than production120s; exit is injected into the exact owned debugger after real bytes arrive. No native stdout bytes/errors were substituted. - Review follow-up: two MCP cases failed on the previous head when stdout arrived after leader exit; the fix passes these plus a never-closing-output control. A genuine Rizin probe pauses actual stdout, observes owned-process exit before byte delivery, resumes the stream and retains the native diagnostic. The shared PTY fixture now belongs to each test and awaits native termination, avoiding cached closed paths across shared MCP workers. - Final capture-result review follow-up: all3 cancellation/queue-overflow races failed on `9d6760548`;353 final owner/MCP/config tests pass, including stdin failure before/during waiting and byte-based queue drops. Actual Rizin exit-before-delivery plus cancellation during drainage now preserves the exact native diagnostic through the cleanup-error cause and partial Evidence. This is a real-provider boundary probe, not public SDK cancellation verification. - Fault probes first finish the controlled inferior naturally and confirm empty native IO inventory. Existing `cleanup_incomplete` remains explicit; test teardown uses the exact launched debugger after that independent proof. **No automatic public recovery claim for these fault cases.** - Actual compiled CLI/stdio MCP, default deadlines/cleanup: **two paired completed runs per revision**, each1CLI+11MCP requests, two explicit close/reopen cycles and active-session client shutdown. Caller TTY remains durable until detached fixture completion; unrelated `rizin`-named control survives; source ELF remains unchanged and final lineage is verified empty. - Actual SDK-converted catalog:207 tools/414 schemas passed2020-12 meta-schema validation. Other OSes/backends and the full120s native fault deadline were not tested. ## Performance Same host/two-core affinity; OS cache uncontrolled; no speedup claim. | Completed workflow | Base → fix wall /CPU /sampled family RSS | |---|---| | Named contracts,2 runs each (medians) |6.62→6.75s /7.02→7.42s /0.717→0.717GiB| | Includes actual initialize/catalog discovery,1 run each |27.05→14.85s /30.91→16.85s /1.025→1.013GiB| Final reviewed/tested head: `0e480fb83`. These are shared-host observations with uncontrolled caches, frequency and scheduling; the discovery timing difference is not attributed to this Rizin fix. Final source ELFs remained unchanged and all completed public runs verified empty final lineage. Named-contract first/later session starts:315/303→323/310ms; first/later commands18.1/10.4→20.1/10.2ms. No new cache or normal-response payload is introduced. `o200k_base` estimates: named-contract11 tool arguments/full SDK replies5028→4959 tokens, CLI arguments/reply1093 each. The complete discovery workflow records27 JSON-RPC messages including initialize, initialized notification, `tools/list` and11 tool exchanges: **836,209→836,295 tokens including CLI**. Catalogs are identical and account for829,341 tokens; the four used tool advertisements alone are4219 tokens. These are serialized-wire proxies, including text/structured duplicates, **not actual model billing or proof that a client feeds the whole catalog to its model**. Discovery-enabled first start measured12.4/6.5s; initial timings did not establish the cause. A later actual-task profile measured207 client validator-preparation calls taking6.54s; this separate SDK cost is already tracked in #1065/T5 and is not attributed to this fix. No retries; staging/fixture compilation excluded. RSS sampled200ms; CPU includes reaped descendants only. Earlier shell-child CPU is excluded by delta measurement. ## Compatibility, safety, and release No breaking change, migration or new target authority. Unknown completion, cleanup uncertainty, output limits and provenance remain explicit. Controlled native fixture execution/owned-process exit injection were performed only for verification. No binary, provider workspace, build output or local test artifacts are committed. ## Author readiness Human owner: @N0zoM1z0 Entered under the human owner's explicit working-session instruction to complete the declarations and request review. The assistant performed implementation and validation review; this does not claim a separate manual human review. - [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-10-31 01:13:03 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

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

[PR #6254] [MERGED] fix(nativeaot): preserve COPY bytes when hydration output grows #7229

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

No description provided.

Rows
Columns