morluto/rea

Fork 0

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

[PR #5516] fix(rizin): preserve completion across exit and cancellation #6946

New issue

Open

opened 2026-11-01 21:44:15 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:44:15 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5516
Author: @zerojackhear
Created: 10/20/2026
Status: 🔄 Open

Base: main ← Head: codex/rizin-cancellation-output-settlement

📝 Commits (1)

8b361ff fix(rizin): preserve completion across exit and cancellation

📊 Changes

3 files changed (+454 additions, -10 deletions)

View changed files

📝 src/rizin/RizinDebugSessionManager.ts (+34 -9)
📝 tests/boundary/providers/RizinInferiorTty.test.ts (+2 -1)
➕ tests/process-global/rizinExitOutputSettlement.test.ts (+418 -0)

📄 Description

Summary

Preserve confirmed Rizin command completion across cancellation and exit drainage. Cancellation still returns a cancellation outcome; cleanup failures now retain their actual reason instead of claiming that the debugger stopped.

Problem and expected behavior

A cancellation arriving after waitForCommandFrames() succeeded unconditionally poisoned protocol state and downgraded complete Evidence to unknown. This prevented the existing detach verification from running. Cancellation Evidence also said the debugger was stopped even when cleanup was incomplete. Separately, a complete NUL-delimited completion marker collected during exit drainage was always classified as an exit interruption.

Change and scope

Track completion only from a successful, nontruncated parser result. Build cancellation Evidence after cleanup, preserving genuine protocol failures and retained resource ownership. Recognize a completed marker in queued NUL frames during the existing bounded exit drain; keep the existing queue/capacity rules and unfinished-frame uncertainty. Publish fake-provider readiness after its PID record and command handler are ready.

This is an ordinary fix within existing Rizin authority; an additional maintainer capability-scope decision is not applicable. No issue is linked because this was an independently reproduced boundary defect.

Contract and boundary impact

Semantic owner: the Rizin command receiver and per-command capture in RizinDebugSessionManager.
Contribution scope: existing provider lifecycle/Evidence bug fix; no capability expansion.
CLI/MCP schemas and tool catalog: unchanged. Cancellation Evidence can now retain completion_status: "complete" when completion is proved.
Compatibility: Linux debugger admission remains required; upstream interrupted-byte, whitespace, capacity and process-identity behavior is preserved.
Evidence/provenance: raw and normalized output, invalid UTF-8 flags and caller launch inputs remain preserved; private markers remain excluded.
Authority/cleanup: use existing fresh olj, TTY and dbg.exitkills readback before debugger-only cleanup. Unverified cleanup retains the exact owner and reason.
Generated metadata, package and installation impact: none; generated artifacts and local verification helpers are not committed.

Expected effects exercised with self-owned fixtures under existing debugger authority:

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

Nine new deterministic regressions cover post-exit frames, bounded drainage, cancellation, complete/unterminated markers, genuine protocol failure, invalid UTF-8, failed cleanup retention/retry and Windows admission.
On unmodified upstream c374c2fe7, the original seven controls produced four passes and three expected failures: post-exit completion, post-marker cancellation cleanup, and truthful cleanup Evidence. Two further negative controls protect the final recovery conditions.
An earlier real Rizin 0.7.4 reproduction on fe7bcc3fb observed the complete NUL marker but returned AnalysisCapabilityUnavailableError with cleanup_incomplete. Final-version real results are recorded below.

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 scripts/run-exact-tests.mjs run with rizinExitOutputSettlement, RizinDebugSessionReviewRegressions, RizinInferiorTty, RizinInferiorExit, RizinCommandTimeout, RizinTimeoutEvidence, rizinDebuggerCancellationMcp and rizinPendingOutput — 78 passed, 0 failed, 0 skipped. The real Rizin 0.7.4 TTY/detach gate executed and passed.
npm run build:cached — passed; npm run check:fast — passed (typecheck and lint); modified-file oxfmt --check and git diff --check — passed.
Local real-provider verifier real-rizin-workflows.mjs --rizin <Rizin-0.7.4> against the final build — 5/5 passed (four public CLI/MCP workflows and one observed provider/process boundary):

Real workflow	Observed result
Public CLI normal command	Complete Evidence and exit 0; detach preserved the self-owned target and TTY output.
Public MCP normal command	Complete Evidence, verified close and surviving detached target.
Public MCP `q!` / engine exit	Partial Evidence with completion unknown; uncertainty was retained.
Public MCP cancellation before completion	Retained partial Evidence, closed command status, subsequent `ping` succeeded; close reported `cleanup_incomplete`, and the same owned engine was independently still alive before verifier cleanup.
Real NUL marker observed before controlled cancellation	`AnalysisCancelledError`, complete output retained, `cleanupIncomplete: false`, verified close, target still alive after detach.

Compatibility, safety, and remaining unknowns

Cancellation without a confirmed completion marker still leaves command completion and current inferior identity unknown. Cleanup may remain cleanup_incomplete, with the debugger retained; REA can exit 1 on shutdown. Separate identity-checked verifier cleanup is not reported as REA cleanup success.
Even verified debugger cleanup returns target_state: "unknown": unrestricted commands prevent inferring final target state from a session identifier or launch path.
Real coverage is WSL Debian/Linux, Node 24.18.0/npm 11.16.0, Rizin 0.7.4, using self-owned Python targets and PTYs. Native Windows debugger execution and other providers were not verified.
Real runs observed zero stdout events after actual Rizin exit. Exact exit-before-tail/marker ordering is established by deterministic tests; the observation launcher forwards real bytes and waits for real output close.
A cancelled MCP request may have no terminal wire reply. Cancellation verification uses retained Evidence, closed command status, subsequent ping and independent owned-engine identity observations.
No breaking schema, migration or release metadata change. This draft does not assert human readiness or merge approval.

Author readiness

Human owner: @zerojackhear

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/5516 **Author:** [@zerojackhear](https://github.com/zerojackhear) **Created:** 10/20/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `codex/rizin-cancellation-output-settlement` --- ### 📝 Commits (1) - [`8b361ff`](https://github.com/morluto/rea/commit/8b361fff219b8d98a5888f37ff640299b73655c1) fix(rizin): preserve completion across exit and cancellation ### 📊 Changes **3 files changed** (+454 additions, -10 deletions) <details> <summary>View changed files</summary> 📝 `src/rizin/RizinDebugSessionManager.ts` (+34 -9) 📝 `tests/boundary/providers/RizinInferiorTty.test.ts` (+2 -1) ➕ `tests/process-global/rizinExitOutputSettlement.test.ts` (+418 -0) </details> ### 📄 Description ## Summary Preserve confirmed Rizin command completion across cancellation and exit drainage. Cancellation still returns a cancellation outcome; cleanup failures now retain their actual reason instead of claiming that the debugger stopped. ## Problem and expected behavior A cancellation arriving after `waitForCommandFrames()` succeeded unconditionally poisoned protocol state and downgraded complete Evidence to unknown. This prevented the existing detach verification from running. Cancellation Evidence also said the debugger was stopped even when cleanup was incomplete. Separately, a complete NUL-delimited completion marker collected during exit drainage was always classified as an exit interruption. ## Change and scope Track completion only from a successful, nontruncated parser result. Build cancellation Evidence after cleanup, preserving genuine protocol failures and retained resource ownership. Recognize a completed marker in queued NUL frames during the existing bounded exit drain; keep the existing queue/capacity rules and unfinished-frame uncertainty. Publish fake-provider readiness after its PID record and command handler are ready. This is an ordinary fix within existing Rizin authority; an additional maintainer capability-scope decision is not applicable. No issue is linked because this was an independently reproduced boundary defect. ## Contract and boundary impact - Semantic owner: the Rizin command receiver and per-command capture in `RizinDebugSessionManager`. - Contribution scope: existing provider lifecycle/Evidence bug fix; no capability expansion. - CLI/MCP schemas and tool catalog: unchanged. Cancellation Evidence can now retain `completion_status: "complete"` when completion is proved. - Compatibility: Linux debugger admission remains required; upstream interrupted-byte, whitespace, capacity and process-identity behavior is preserved. - Evidence/provenance: raw and normalized output, invalid UTF-8 flags and caller launch inputs remain preserved; private markers remain excluded. - Authority/cleanup: use existing fresh `olj`, TTY and `dbg.exitkills` readback before debugger-only cleanup. Unverified cleanup retains the exact owner and reason. - Generated metadata, package and installation impact: none; generated artifacts and local verification helpers are not committed. Expected effects exercised with self-owned fixtures under existing debugger authority: - [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. - [ ] 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 - Nine new deterministic regressions cover post-exit frames, bounded drainage, cancellation, complete/unterminated markers, genuine protocol failure, invalid UTF-8, failed cleanup retention/retry and Windows admission. - On unmodified upstream `c374c2fe7`, the original seven controls produced four passes and three expected failures: post-exit completion, post-marker cancellation cleanup, and truthful cleanup Evidence. Two further negative controls protect the final recovery conditions. - An earlier real Rizin 0.7.4 reproduction on `fe7bcc3fb` observed the complete NUL marker but returned `AnalysisCapabilityUnavailableError` with `cleanup_incomplete`. Final-version real results are recorded below. 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 scripts/run-exact-tests.mjs run` with `rizinExitOutputSettlement`, `RizinDebugSessionReviewRegressions`, `RizinInferiorTty`, `RizinInferiorExit`, `RizinCommandTimeout`, `RizinTimeoutEvidence`, `rizinDebuggerCancellationMcp` and `rizinPendingOutput` — **78 passed, 0 failed, 0 skipped**. The real Rizin 0.7.4 TTY/detach gate executed and passed. - `npm run build:cached` — passed; `npm run check:fast` — passed (typecheck and lint); modified-file `oxfmt --check` and `git diff --check` — passed. - Local real-provider verifier `real-rizin-workflows.mjs --rizin <Rizin-0.7.4>` against the final build — **5/5 passed** (four public CLI/MCP workflows and one observed provider/process boundary): | Real workflow | Observed result | | --- | --- | | Public CLI normal command | Complete Evidence and exit 0; detach preserved the self-owned target and TTY output. | | Public MCP normal command | Complete Evidence, verified close and surviving detached target. | | Public MCP `q!` / engine exit | Partial Evidence with completion unknown; uncertainty was retained. | | Public MCP cancellation before completion | Retained partial Evidence, closed command status, subsequent `ping` succeeded; close reported `cleanup_incomplete`, and the same owned engine was independently still alive before verifier cleanup. | | Real NUL marker observed before controlled cancellation | `AnalysisCancelledError`, complete output retained, `cleanupIncomplete: false`, verified close, target still alive after detach. | ## Compatibility, safety, and remaining unknowns - Cancellation without a confirmed completion marker still leaves command completion and current inferior identity unknown. Cleanup may remain `cleanup_incomplete`, with the debugger retained; REA can exit 1 on shutdown. Separate identity-checked verifier cleanup is not reported as REA cleanup success. - Even verified debugger cleanup returns `target_state: "unknown"`: unrestricted commands prevent inferring final target state from a session identifier or launch path. - Real coverage is **WSL Debian/Linux, Node 24.18.0/npm 11.16.0, Rizin 0.7.4**, using self-owned Python targets and PTYs. Native Windows debugger execution and other providers were not verified. - Real runs observed zero stdout events after actual Rizin exit. Exact exit-before-tail/marker ordering is established by deterministic tests; the observation launcher forwards real bytes and waits for real output close. - A cancelled MCP request may have no terminal wire reply. Cancellation verification uses retained Evidence, closed command status, subsequent ping and independent owned-engine identity observations. - No breaking schema, migration or release metadata change. This draft does not assert human readiness or merge approval. ## Author readiness Human owner: @zerojackhear - [ ] 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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-11-01 21:44:15 +00:00

bsakaguchi referenced this issue

2026-11-01 21:47:58 +00:00

[PR #6929] [MERGED] fix(environment): bound create learning and invalidate stale write proofs #7454

bsakaguchi referenced this issue

2026-11-01 21:48:03 +00:00

[PR #6946] [MERGED] feat(environment): build agent environments from a prompt #7466

bsakaguchi referenced this issue

2026-11-01 21:48:08 +00:00