morluto/rea

Fork 0

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

[GH-ISSUE #6091] [Bug] Session cleanup status omits retained external-mutation reopen blockers #3724

New issue

Closed

opened 2026-10-31 00:43:24 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-31 00:43:24 +00:00

Owner

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

Before submitting

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

Area

MCP or agent integration

Observed behavior

A retained external-mutation cleanup failure blocks opening a binary, but session status reports open: false, cleanup_pending: false. An intervening session close succeeds without clearing the blocker.

Executed the registered start_rizin_debug_session MCP handler with a simulated manager returning an unresolved cleanup error, production external-mutation admission, a real BinarySession and a fixture analysis provider. The next open returns the exact retained cleanup error. This is a controlled production-handler reproduction, not a Linux Rizin engine run or an MCP transport round trip.

Reproduction

In a source checkout with locked dependencies, save the following test as tests/boundary/filesystem/externalCleanupStatus.test.ts. Set REA_VERIFY_ROOT to an empty disposable directory, then run:

node node_modules/vitest/vitest.mjs run tests/boundary/filesystem/externalCleanupStatus.test.ts --maxWorkers=1

The assertions characterize current behavior. The simulated manager represents the external owner; the session and registered handler are unchanged production code. Resolving that owner is a control, and permits the same fixture to reopen.

import { writeFile, mkdtemp, rm } from 'node:fs/promises';
import { join } from 'node:path';
import { expect, it, vi } from 'vitest';
import pino from 'pino';
import { ArtifactResourceScope } from '../../../src/artifacts/ArtifactResourceScope.js';
import { analyzeJavaScriptApplication } from '../../../src/application/javascript/JavaScriptApplicationService.js';
import { JavaScriptAnalysisWorker } from '../../../src/javascript/analysis/JavaScriptAnalysisWorker.js';
import { PrivateRuntimeRoot } from '../../../src/process/PrivateRuntimeRoot.js';
import { projectAnalysisError } from '../../../src/domain/analysisErrorProjection.js';
import { ProviderCleanupError } from '../../../src/domain/providerCleanupError.js';
import { createTestBinarySession, createCacheProvider } from '../../fixtures/binarySession.js';
import { registerRizinDebugTools } from '../../../src/server/registerRizinDebugTools.js';
import { createReverseEngineeringMcpHarness } from '../../support/reverseEngineeringMcpHarness.js';
import { createToolResultDelivery } from '../../../src/server/toolResult.js';
import { err, ok } from '../../../src/domain/result.js';

it('reports the retained external mutation cleanup in session status', async () => {
 const root = await mkdtemp(join(process.env.REA_VERIFY_ROOT!, 'session-'));
 const target = join(root, 'target.hop'); await writeFile(target, 'inert');
 const session = createTestBinarySession(createCacheProvider([]));
 const failure = new ProviderCleanupError('rizin', ['simulated-debugger-owner'], {reason:'controlled cleanup denial'}, {operation:'start_rizin_debug_session'});
 const manager = {start:vi.fn(async()=>err(failure)), execute:vi.fn(),status:vi.fn(),close:vi.fn(),retryCleanupFailures:vi.fn(async()=>ok(null))};
 const {server,handler} = createReverseEngineeringMcpHarness();
 registerRizinDebugTools(server,manager as any,pino({enabled:false}),createToolResultDelivery(2*1024*1024),undefined,session);
 try {
  const before = await session.status() as any;
  const response = await handler('start_rizin_debug_session')({path:target},{mcpReq:{signal:new AbortController().signal}}) as any;
  expect(response.structuredContent.error.code).toBe('cleanup_incomplete');
  const closed = await session.close();
  const after = await session.status() as any;
  const blocked = await session.open(target);
  console.log(JSON.stringify({kind:'session',before:before.cleanup_pending,close_ok:closed.ok,open:after.open,cleanup_pending:after.cleanup_pending,blocked:!blocked.ok,exact_failure:!blocked.ok&&blocked.error===failure}));
  expect(closed.ok).toBe(true);expect(after.open).toBe(false);expect(after.cleanup_pending).toBe(false);
  expect(blocked.ok).toBe(false);if(!blocked.ok)expect(blocked.error).toBe(failure);
  failure.resolveCleanup();
  expect((await session.open(target)).ok).toBe(true);
 } finally {failure.resolveCleanup();await session.close();await rm(root,{recursive:true,force:true});}
});

Expected behavior

Session cleanup status should include the unresolved external-mutation cleanup condition that the session retains and uses to refuse new opens. Keep active-target state independent of pending cleanup. The cleanup status contract requires unresolved ownership to remain visible before reopening. The external mutation contract requires an explicit fresh open after command effects settle. This report does not require binary-session close to take ownership of a separately managed debugger.

Impact and scope

P2: clients gating reopen on cleanup_pending receive a false readiness signal and attempt an open that is guaranteed to return the retained failure. The original external command still supplies its cleanup diagnostics, so those diagnostics can guide owner recovery. No artifact corruption, permanent deadlock or natural failure rate was established. The simulated owner was resolved explicitly and reopening then succeeded. Persistent Rizin debugging is Linux-only; the controlled handler/session test ran on macOS and did not exercise platform admission or a real debugger.

REA version

rea-agents 6.7.0; source revision 52415975942a4b9c53a817915b313ccc404f280e.

Runtime and platform

Node.js v26.12.0; macOS 27.0.1 arm64; locked Vitest 5.0.3. The development Node.js 24.18.0 pin was not exercised.

Target and artifact context

An inert local target fixture and a simulated persistent Rizin manager cleanup failure. No target program or debugger executed.

Interface

MCP

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

The focused group completed with seven passing characterization/control tests. This case recorded:

{"kind":"session","before":false,"close_ok":true,"open":false,"cleanup_pending":false,"blocked":true,"exact_failure":true}

The handler first returned cleanup_incomplete. The same stored error was then returned by open, despite the successful close and false pending flag. The test removes its fixture and closes the session.

Typed outcome or recovery expected

Report cleanup_pending: true while an unresolved external-mutation cleanup failure is retained as a reopen blocker. Clear it only after the external owner confirms release; retain the original error resources and recovery action.

Analysis or suspected cause

External mutation completion retains the error, and binary open refuses it. The status expression checks other cleanup sources and omits this field. Rizin debugger registration reaches that production admission owner. This source explains the observed handler/session result.

Additional context

Fetched upstream main d8a9673fc44fde4bd30ff98c241886e6683580ca; the relevant session and admission code is unchanged. Related #5911 loses final close failure in one-shot provider-status CLI commands; #4277 omitted a snapshot-staging scope; #4226 concerns Hopper startup readiness. Their fixes do not include the external-mutation blocker. #5921 is profile restoration and #6063 is managed cleanup lease settlement. No newer runtime, naturally occurring OS denial, or full transport session was executed.

Originally created by @coygeek on GitHub (Oct 21, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/6091 ### 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 MCP or agent integration ### Observed behavior A retained external-mutation cleanup failure blocks opening a binary, but session status reports `open: false, cleanup_pending: false`. An intervening session close succeeds without clearing the blocker. Executed the registered `start_rizin_debug_session` MCP handler with a simulated manager returning an unresolved cleanup error, production external-mutation admission, a real BinarySession and a fixture analysis provider. The next open returns the exact retained cleanup error. This is a controlled production-handler reproduction, not a Linux Rizin engine run or an MCP transport round trip. ### Reproduction In a source checkout with locked dependencies, save the following test as `tests/boundary/filesystem/externalCleanupStatus.test.ts`. Set `REA_VERIFY_ROOT` to an empty disposable directory, then run: ```sh node node_modules/vitest/vitest.mjs run tests/boundary/filesystem/externalCleanupStatus.test.ts --maxWorkers=1 ``` The assertions characterize current behavior. The simulated manager represents the external owner; the session and registered handler are unchanged production code. Resolving that owner is a control, and permits the same fixture to reopen. ```ts import { writeFile, mkdtemp, rm } from 'node:fs/promises'; import { join } from 'node:path'; import { expect, it, vi } from 'vitest'; import pino from 'pino'; import { ArtifactResourceScope } from '../../../src/artifacts/ArtifactResourceScope.js'; import { analyzeJavaScriptApplication } from '../../../src/application/javascript/JavaScriptApplicationService.js'; import { JavaScriptAnalysisWorker } from '../../../src/javascript/analysis/JavaScriptAnalysisWorker.js'; import { PrivateRuntimeRoot } from '../../../src/process/PrivateRuntimeRoot.js'; import { projectAnalysisError } from '../../../src/domain/analysisErrorProjection.js'; import { ProviderCleanupError } from '../../../src/domain/providerCleanupError.js'; import { createTestBinarySession, createCacheProvider } from '../../fixtures/binarySession.js'; import { registerRizinDebugTools } from '../../../src/server/registerRizinDebugTools.js'; import { createReverseEngineeringMcpHarness } from '../../support/reverseEngineeringMcpHarness.js'; import { createToolResultDelivery } from '../../../src/server/toolResult.js'; import { err, ok } from '../../../src/domain/result.js'; it('reports the retained external mutation cleanup in session status', async () => { const root = await mkdtemp(join(process.env.REA_VERIFY_ROOT!, 'session-')); const target = join(root, 'target.hop'); await writeFile(target, 'inert'); const session = createTestBinarySession(createCacheProvider([])); const failure = new ProviderCleanupError('rizin', ['simulated-debugger-owner'], {reason:'controlled cleanup denial'}, {operation:'start_rizin_debug_session'}); const manager = {start:vi.fn(async()=>err(failure)), execute:vi.fn(),status:vi.fn(),close:vi.fn(),retryCleanupFailures:vi.fn(async()=>ok(null))}; const {server,handler} = createReverseEngineeringMcpHarness(); registerRizinDebugTools(server,manager as any,pino({enabled:false}),createToolResultDelivery(2*1024*1024),undefined,session); try { const before = await session.status() as any; const response = await handler('start_rizin_debug_session')({path:target},{mcpReq:{signal:new AbortController().signal}}) as any; expect(response.structuredContent.error.code).toBe('cleanup_incomplete'); const closed = await session.close(); const after = await session.status() as any; const blocked = await session.open(target); console.log(JSON.stringify({kind:'session',before:before.cleanup_pending,close_ok:closed.ok,open:after.open,cleanup_pending:after.cleanup_pending,blocked:!blocked.ok,exact_failure:!blocked.ok&&blocked.error===failure})); expect(closed.ok).toBe(true);expect(after.open).toBe(false);expect(after.cleanup_pending).toBe(false); expect(blocked.ok).toBe(false);if(!blocked.ok)expect(blocked.error).toBe(failure); failure.resolveCleanup(); expect((await session.open(target)).ok).toBe(true); } finally {failure.resolveCleanup();await session.close();await rm(root,{recursive:true,force:true});} }); ``` ### Expected behavior Session cleanup status should include the unresolved external-mutation cleanup condition that the session retains and uses to refuse new opens. Keep active-target state independent of pending cleanup. The [cleanup status contract](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/mcp-contracts.md#L552-L565) requires unresolved ownership to remain visible before reopening. The [external mutation contract](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/reverse-engineering-tools.md#L5-L13) requires an explicit fresh open after command effects settle. This report does not require binary-session close to take ownership of a separately managed debugger. ### Impact and scope P2: clients gating reopen on `cleanup_pending` receive a false readiness signal and attempt an open that is guaranteed to return the retained failure. The original external command still supplies its cleanup diagnostics, so those diagnostics can guide owner recovery. No artifact corruption, permanent deadlock or natural failure rate was established. The simulated owner was resolved explicitly and reopening then succeeded. Persistent Rizin debugging is Linux-only; the controlled handler/session test ran on macOS and did not exercise platform admission or a real debugger. ### REA version rea-agents 6.7.0; source revision `52415975942a4b9c53a817915b313ccc404f280e`. ### Runtime and platform Node.js v26.12.0; macOS 27.0.1 arm64; locked Vitest 5.0.3. The development Node.js 24.18.0 pin was not exercised. ### Target and artifact context An inert local target fixture and a simulated persistent Rizin manager cleanup failure. No target program or debugger executed. ### Interface MCP ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence The focused group completed with seven passing characterization/control tests. This case recorded: ```json {"kind":"session","before":false,"close_ok":true,"open":false,"cleanup_pending":false,"blocked":true,"exact_failure":true} ``` The handler first returned `cleanup_incomplete`. The same stored error was then returned by open, despite the successful close and false pending flag. The test removes its fixture and closes the session. ### Typed outcome or recovery expected Report `cleanup_pending: true` while an unresolved external-mutation cleanup failure is retained as a reopen blocker. Clear it only after the external owner confirms release; retain the original error resources and recovery action. ### Analysis or suspected cause [External mutation completion retains the error](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/application/binary/BinarySession.ts#L737-L749), and [binary open refuses it](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/application/binary/BinarySession.ts#L528-L534). The [status expression](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/application/binary/BinarySession.ts#L983-L988) checks other cleanup sources and omits this field. [Rizin debugger registration](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/server/registerRizinDebugTools.ts#L32-L59) reaches that production admission owner. This source explains the observed handler/session result. ### Additional context Fetched upstream main `d8a9673fc44fde4bd30ff98c241886e6683580ca`; the relevant session and admission code is unchanged. Related #5911 loses final close failure in one-shot provider-status CLI commands; #4277 omitted a snapshot-staging scope; #4226 concerns Hopper startup readiness. Their fixes do not include the external-mutation blocker. #5921 is profile restoration and #6063 is managed cleanup lease settlement. No newer runtime, naturally occurring OS denial, or full transport session was executed.

bsakaguchi closed this issue

2026-10-31 00:43:24 +00:00

bsakaguchi commented

2026-10-31 00:43:25 +00:00

Author

Owner

@coygeek commented on GitHub (Oct 24, 2026):

Checked the reported fix on main at d87e73f2db.

cleanupPending now includes the exact retained external-mutation cleanup error when cleanupIncomplete is true. Its regression retains cleanup_pending after close, returns the same error on open, then clears status only after the external lease releases and observes successful reopen. Active-target state remains separate.

Implementation: BinarySession.ts.

The relevant regression checks in binarySession.test.ts passed in the focused test run on this revision. This closure is based on the current source and those checks; provider fixtures do not establish a real-engine run. Closing as completed on main.

@coygeek commented on GitHub (Oct 24, 2026): Checked the reported fix on `main` at d87e73f2db50be72d33a4b3c7199af35518b441c. cleanupPending now includes the exact retained external-mutation cleanup error when cleanupIncomplete is true. Its regression retains cleanup_pending after close, returns the same error on open, then clears status only after the external lease releases and observes successful reopen. Active-target state remains separate. Implementation: [BinarySession.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/application/binary/BinarySession.ts#L1036). The relevant regression checks in [binarySession.test.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/tests/composition/analysis-sessions/binarySession.test.ts) passed in the focused test run on this revision. This closure is based on the current source and those checks; provider fixtures do not establish a real-engine run. Closing as completed on main.

bsakaguchi referenced this issue

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

[GH-ISSUE #3796] [Tracker] Open feature requests by area (October 2026) #1471

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

No description provided.

Rows
Columns