morluto/rea

Fork 0

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

[GH-ISSUE #6103] [Bug] Cutter retry admission discards descriptor cleanup errors from completion lookup #3734

New issue

Closed

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

bsakaguchi commented

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

Owner

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

Before submitting

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

Area

Native analysis or decompilation

Observed behavior

After a command response is lost, the persistent Cutter service retains the unknown command completion. If the next completion-proof read fails to close its descriptor, the actual client produces a typed descriptor cleanup error, but the service returns only the older command-completion error. The returned resource list contains the session and omits the still-owned descriptor; its reason describes only missing UI proof.

This was executed with the actual Cutter client/service, authenticated loopback protocol fixture and real descriptor reads. The fixture drops the dispatched command response, and the selected real descriptor close is controlled. No native Cutter application was run.

Reproduction

I ran the following focused source test under the repository-pinned Node 24.18.0 with locked dependencies and one worker. Save it as tests/process-global/cleanupRecovery.test.ts and run:

node node_modules/vitest/vitest.mjs run tests/process-global/cleanupRecovery.test.ts --maxWorkers 1

The assertions characterize the affected behavior; they are not a fix. The exact standalone excerpt below was replayed independently and passed. Production code remained unchanged.

import { createServer } from 'node:http';
import { mkdtemp, rm, writeFile, realpath, type FileHandle } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { pathToFileURL } from 'node:url';
import { createHash } from 'node:crypto';
import { it, expect, vi } from 'vitest';
import { WebSocketServer } from 'ws';
const fault = vi.hoisted(() => ({path:'', blocked:false, abort:undefined as AbortController|undefined, handles:[] as FileHandle[]}));
vi.mock('node:fs/promises', async original => {
 const real = await original<typeof import('node:fs/promises')>();
 return {...real,open:async (...args:Parameters<typeof real.open>) => {
  const h=await real.open(...args); if(String(args[0])!==fault.path) return h;
  fault.handles.push(h); const close=h.close.bind(h);
  h.close=async()=>{if(fault.blocked){fault.abort?.abort('abort during descriptor close');throw new Error('controlled descriptor close denial');} await close();};return h;
 }};
});
import { V8InspectorProvider } from '../../src/inspector/V8InspectorProvider.js';
import { observeJavaScriptRuntime } from '../../src/application/javascript/JavaScriptRuntimeObservationService.js';
import { finishAnalysisCommand } from '../../src/cli/finishAnalysisCommand.js';
import { projectAnalysisError } from '../../src/domain/analysisErrorProjection.js';
import { CutterBridgeClient } from '../../src/cutter/CutterBridgeClient.js';
import { cutterCommandInputSchema } from '../../src/contracts/cutterToolContracts.js';
import { CutterBridgeService } from '../../src/cutter/CutterBridgeService.js';
import { authenticatedServer, writeDescriptor } from '../fixtures/cutterBridgeProtocol.js';
import { GhidraProvider } from '../../src/ghidra/GhidraProvider.js';
import { parseConfig } from '../../src/config/parseConfig.js';
import { silentLogger } from '../../src/logger.js';
import { err, ok } from '../../src/domain/result.js';
import { ProviderCleanupError } from '../../src/domain/providerCleanupError.js';
import { GhidraSessionError } from '../../src/ghidra/GhidraSessionError.js';
import { GHIDRA_SESSION_CAPABILITIES } from '../../src/ghidra/GhidraSessionValues.js';
import { elf } from '../../src/domain/binaryTarget.fixture.js';
import { createTestBinarySession } from '../fixtures/binarySession.js';

it('Cutter drops a real descriptor error during retry admission', async()=>{
 const root=await realpath(await mkdtemp(join(tmpdir(),'rea-cutter-probe-')));const id='27d3e3f1-f1e5-49ae-91ec-95af1f343a5a';const token='synthetic-cutter-bridge-token-for-test';let dispatched!:()=>void;const dispatch=new Promise<void>(r=>dispatched=r);let complete=false;
 const server=authenticatedServer(token,id,(request,respond,connection)=>{if(request.kind==='command'){dispatched();connection.destroy();}else respond({ok:true,session_id:id,current_file:null,document_generation:0,cutter_version:'fixture',identity_status:'partial',completed_commands:[],completed_connection:complete?'ffffffffffffffff':'0000000000000000'});},true);
 await new Promise<void>(r=>server.listen(0,'127.0.0.1',r));await writeDescriptor(root,id,(server.address() as import('node:net').AddressInfo).port,token);
 const client=new CutterBridgeClient({REA_CUTTER_BRIDGE_DIR:root});const service=new CutterBridgeService(client);const input={session_id:id,expected_generation:0,command:'ij',json:false};
 try{
 const first=await service.execute(input);expect(first.ok).toBe(false);if(first.ok)throw Error('expected unknown');expect(first.error.cleanupResources).toEqual([`cutter-session:${id}`]);
 fault.path=join(root,`cutter-${process.pid}-${id}.json`);fault.blocked=true;
 const direct=await client.confirmCommandCompletion(id,'00000000-0000-4000-8000-000000000000');expect(direct.ok).toBe(false);if(direct.ok)throw Error('expected descriptor failure');expect(direct.error.cleanupResources).toContain(fault.path);
 const retry=await service.execute(input);expect(retry.ok).toBe(false);if(retry.ok)throw Error('expected blocked');expect(retry.error).toBe(first.error);expect(retry.error.cleanupResources).not.toContain(fault.path);
 console.log('CUTTER',JSON.stringify({descriptorError:projectAnalysisError(direct.error),retryError:projectAnalysisError(retry.error)}));
 fault.blocked=false;await client.retryDescriptorCleanup();expect(direct.error.cleanupIncomplete).toBe(false);complete=true;expect((await service.retryCleanup()).ok).toBe(true);expect(first.error.cleanupIncomplete).toBe(false);
 }finally{fault.blocked=false;complete=true;await service.close();await new Promise<void>(r=>server.close(()=>r()));await rm(root,{recursive:true,force:true});fault.path='';fault.handles=[];}
});

Expected behavior

Preserve the already-retained command uncertainty and the newly produced receipt-read failure, including each unresolved cleanup resource and exact owner. A successful receipt read with value false may return the pending command error alone; an error reading that receipt must remain visible. See retry/aggregate ownership and Cutter completion-proof behavior. Do not replay the command.

Impact and scope

P2: a persistent command-admission failure hides the descriptor path and new cleanup reason that the client is actually waiting on. The exact descriptor remains owned and no new command is dispatched. Restoring close and then supplying authenticated completion proof allowed retry cleanup to finish and cleared the original marker. This does not establish a leak or failed admission blocking. A fresh standalone CLI invocation does not inherit the service pending map; the reproduced scope is the persistent MCP/shared service path.

REA version

rea-agents 6.7.0; source revision 52415975942a4b9c53a817915b313ccc404f280e.

Runtime and platform

macOS 27.0.1 arm64; Node.js v24.18.0; locked Vitest 5.0.3. Simulated protocol/native-provider boundaries as described above.

Target and artifact context

A private synthetic bridge descriptor and an authenticated v2 loopback peer. A dispatched ij command loses its response; later descriptor close fails until released. No target binary is required.

Interface

Other

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

Observed, with resource names shortened:

{"client_completion_read":{"code":"cleanup_incomplete","resources":["<descriptor.json>"],"reason":"controlled descriptor close denial"},"service_retry":{"code":"cleanup_incomplete","resources":["cutter-session:<session-id>"],"reason":"The exact UI dispatcher completion receipt has not arrived; the dispatched command may still be running."}}

The returned retry error is the exact original command error. After restoring descriptor close, the direct client error clears; after providing completion proof, the pending command error also clears.

Typed outcome or recovery expected

Analysis or suspected cause

CutterBridgeService.#retryPendingCleanup combines !receipt.ok and !receipt.value in one return of pending.failure. confirmCommandCompletion returns typed errors from descriptor discovery. Those errors are lost before projection. The producer should compose the source failures and bind their live cleanup dependencies; no diagnostic-string parsing is needed.

Additional context

Related #4634 repaired descriptor failure operation attribution; direct completion-read failure now correctly identifies cutter_command on this revision. #4471 concerns descriptor FIFO admission and #4635 concerns opaque abort reasons. This service-level retry omission is separate. #4628 and the current one-shot finalizer do not compose errors discarded before they reach the finalizer.

Supported reachability is source-derived from the MCP registration, which invokes both service.execute and service.retryCleanup. The test executes that service boundary, not an MCP transport. No natural OS close failure or last-known-good version was established.

Originally created by @coygeek on GitHub (Oct 21, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/6103 ### 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 Native analysis or decompilation ### Observed behavior After a command response is lost, the persistent Cutter service retains the unknown command completion. If the next completion-proof read fails to close its descriptor, the actual client produces a typed descriptor cleanup error, but the service returns only the older command-completion error. The returned resource list contains the session and omits the still-owned descriptor; its reason describes only missing UI proof. This was executed with the actual Cutter client/service, authenticated loopback protocol fixture and real descriptor reads. The fixture drops the dispatched command response, and the selected real descriptor close is controlled. No native Cutter application was run. ### Reproduction I ran the following focused source test under the repository-pinned Node 24.18.0 with locked dependencies and one worker. Save it as `tests/process-global/cleanupRecovery.test.ts` and run: ```sh node node_modules/vitest/vitest.mjs run tests/process-global/cleanupRecovery.test.ts --maxWorkers 1 ``` The assertions characterize the affected behavior; they are not a fix. The exact standalone excerpt below was replayed independently and passed. Production code remained unchanged. ```ts import { createServer } from 'node:http'; import { mkdtemp, rm, writeFile, realpath, type FileHandle } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { pathToFileURL } from 'node:url'; import { createHash } from 'node:crypto'; import { it, expect, vi } from 'vitest'; import { WebSocketServer } from 'ws'; const fault = vi.hoisted(() => ({path:'', blocked:false, abort:undefined as AbortController|undefined, handles:[] as FileHandle[]})); vi.mock('node:fs/promises', async original => { const real = await original<typeof import('node:fs/promises')>(); return {...real,open:async (...args:Parameters<typeof real.open>) => { const h=await real.open(...args); if(String(args[0])!==fault.path) return h; fault.handles.push(h); const close=h.close.bind(h); h.close=async()=>{if(fault.blocked){fault.abort?.abort('abort during descriptor close');throw new Error('controlled descriptor close denial');} await close();};return h; }}; }); import { V8InspectorProvider } from '../../src/inspector/V8InspectorProvider.js'; import { observeJavaScriptRuntime } from '../../src/application/javascript/JavaScriptRuntimeObservationService.js'; import { finishAnalysisCommand } from '../../src/cli/finishAnalysisCommand.js'; import { projectAnalysisError } from '../../src/domain/analysisErrorProjection.js'; import { CutterBridgeClient } from '../../src/cutter/CutterBridgeClient.js'; import { cutterCommandInputSchema } from '../../src/contracts/cutterToolContracts.js'; import { CutterBridgeService } from '../../src/cutter/CutterBridgeService.js'; import { authenticatedServer, writeDescriptor } from '../fixtures/cutterBridgeProtocol.js'; import { GhidraProvider } from '../../src/ghidra/GhidraProvider.js'; import { parseConfig } from '../../src/config/parseConfig.js'; import { silentLogger } from '../../src/logger.js'; import { err, ok } from '../../src/domain/result.js'; import { ProviderCleanupError } from '../../src/domain/providerCleanupError.js'; import { GhidraSessionError } from '../../src/ghidra/GhidraSessionError.js'; import { GHIDRA_SESSION_CAPABILITIES } from '../../src/ghidra/GhidraSessionValues.js'; import { elf } from '../../src/domain/binaryTarget.fixture.js'; import { createTestBinarySession } from '../fixtures/binarySession.js'; it('Cutter drops a real descriptor error during retry admission', async()=>{ const root=await realpath(await mkdtemp(join(tmpdir(),'rea-cutter-probe-')));const id='27d3e3f1-f1e5-49ae-91ec-95af1f343a5a';const token='synthetic-cutter-bridge-token-for-test';let dispatched!:()=>void;const dispatch=new Promise<void>(r=>dispatched=r);let complete=false; const server=authenticatedServer(token,id,(request,respond,connection)=>{if(request.kind==='command'){dispatched();connection.destroy();}else respond({ok:true,session_id:id,current_file:null,document_generation:0,cutter_version:'fixture',identity_status:'partial',completed_commands:[],completed_connection:complete?'ffffffffffffffff':'0000000000000000'});},true); await new Promise<void>(r=>server.listen(0,'127.0.0.1',r));await writeDescriptor(root,id,(server.address() as import('node:net').AddressInfo).port,token); const client=new CutterBridgeClient({REA_CUTTER_BRIDGE_DIR:root});const service=new CutterBridgeService(client);const input={session_id:id,expected_generation:0,command:'ij',json:false}; try{ const first=await service.execute(input);expect(first.ok).toBe(false);if(first.ok)throw Error('expected unknown');expect(first.error.cleanupResources).toEqual([`cutter-session:${id}`]); fault.path=join(root,`cutter-${process.pid}-${id}.json`);fault.blocked=true; const direct=await client.confirmCommandCompletion(id,'00000000-0000-4000-8000-000000000000');expect(direct.ok).toBe(false);if(direct.ok)throw Error('expected descriptor failure');expect(direct.error.cleanupResources).toContain(fault.path); const retry=await service.execute(input);expect(retry.ok).toBe(false);if(retry.ok)throw Error('expected blocked');expect(retry.error).toBe(first.error);expect(retry.error.cleanupResources).not.toContain(fault.path); console.log('CUTTER',JSON.stringify({descriptorError:projectAnalysisError(direct.error),retryError:projectAnalysisError(retry.error)})); fault.blocked=false;await client.retryDescriptorCleanup();expect(direct.error.cleanupIncomplete).toBe(false);complete=true;expect((await service.retryCleanup()).ok).toBe(true);expect(first.error.cleanupIncomplete).toBe(false); }finally{fault.blocked=false;complete=true;await service.close();await new Promise<void>(r=>server.close(()=>r()));await rm(root,{recursive:true,force:true});fault.path='';fault.handles=[];} }); ``` ### Expected behavior Preserve the already-retained command uncertainty and the newly produced receipt-read failure, including each unresolved cleanup resource and exact owner. A successful receipt read with value false may return the pending command error alone; an error reading that receipt must remain visible. See [retry/aggregate ownership](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/tool-design.md#L123-L131) and [Cutter completion-proof behavior](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/reverse-engineering-tools.md#L66-L86). Do not replay the command. ### Impact and scope P2: a persistent command-admission failure hides the descriptor path and new cleanup reason that the client is actually waiting on. The exact descriptor remains owned and no new command is dispatched. Restoring close and then supplying authenticated completion proof allowed retry cleanup to finish and cleared the original marker. This does not establish a leak or failed admission blocking. A fresh standalone CLI invocation does not inherit the service pending map; the reproduced scope is the persistent MCP/shared service path. ### REA version `rea-agents` 6.7.0; source revision `52415975942a4b9c53a817915b313ccc404f280e`. ### Runtime and platform macOS 27.0.1 arm64; Node.js v24.18.0; locked Vitest 5.0.3. Simulated protocol/native-provider boundaries as described above. ### Target and artifact context A private synthetic bridge descriptor and an authenticated v2 loopback peer. A dispatched `ij` command loses its response; later descriptor close fails until released. No target binary is required. ### Interface Other ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence Observed, with resource names shortened: ```json {"client_completion_read":{"code":"cleanup_incomplete","resources":["<descriptor.json>"],"reason":"controlled descriptor close denial"},"service_retry":{"code":"cleanup_incomplete","resources":["cutter-session:<session-id>"],"reason":"The exact UI dispatcher completion receipt has not arrived; the dispatched command may still be running."}} ``` The returned retry error is the exact original command error. After restoring descriptor close, the direct client error clears; after providing completion proof, the pending command error also clears. ### Typed outcome or recovery expected Preserve the already-retained command uncertainty and the newly produced receipt-read failure, including each unresolved cleanup resource and exact owner. A successful receipt read with value false may return the pending command error alone; an error reading that receipt must remain visible. See [retry/aggregate ownership](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/tool-design.md#L123-L131) and [Cutter completion-proof behavior](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/reverse-engineering-tools.md#L66-L86). Do not replay the command. ### Analysis or suspected cause [CutterBridgeService.#retryPendingCleanup](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/cutter/CutterBridgeService.ts#L66-L81) combines `!receipt.ok` and `!receipt.value` in one return of `pending.failure`. [confirmCommandCompletion](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/cutter/CutterBridgeClient.ts#L520-L561) returns typed errors from descriptor discovery. Those errors are lost before projection. The producer should compose the source failures and bind their live cleanup dependencies; no diagnostic-string parsing is needed. ### Additional context Related #4634 repaired descriptor failure operation attribution; direct completion-read failure now correctly identifies `cutter_command` on this revision. #4471 concerns descriptor FIFO admission and #4635 concerns opaque abort reasons. This service-level retry omission is separate. #4628 and the current one-shot finalizer do not compose errors discarded before they reach the finalizer. Supported reachability is source-derived from [the MCP registration](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/server/registerCutterTools.ts#L45-L63), which invokes both `service.execute` and `service.retryCleanup`. The test executes that service boundary, not an MCP transport. No natural OS close failure or last-known-good version was established.

bsakaguchi closed this issue

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

bsakaguchi commented

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

Author

Owner

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

Checked the reported fix on main at d87e73f2db.

Retry admission no longer combines receipt error and false receipt: errors are composed with pending.failure using analysisErrorWithCleanupFailure and both exact error owners are bound as cleanup dependencies; a successful false receipt still returns only the original pending error. The composition helper preserves primary partial observation, both cleanup resource lists and nested cleanup diagnostics. Regression assertions check both descriptor-first and command-first resolution orders, exact union and later shrinking of resources, preservation of partial observation, new descriptor failure reason and single command dispatch (no replay).

Implementation: CutterBridgeService.ts, CutterBridgeService.ts.

The relevant regression checks in CutterBridgeService.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. Retry admission no longer combines receipt error and false receipt: errors are composed with pending.failure using analysisErrorWithCleanupFailure and both exact error owners are bound as cleanup dependencies; a successful false receipt still returns only the original pending error. The composition helper preserves primary partial observation, both cleanup resource lists and nested cleanup diagnostics. Regression assertions check both descriptor-first and command-first resolution orders, exact union and later shrinking of resources, preservation of partial observation, new descriptor failure reason and single command dispatch (no replay). Implementation: [CutterBridgeService.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/cutter/CutterBridgeService.ts#L76), [CutterBridgeService.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/cutter/CutterBridgeService.ts#L84). The relevant regression checks in [CutterBridgeService.test.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/tests/boundary/providers/CutterBridgeService.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#3734

No description provided.

Rows
Columns