[GH-ISSUE #6111] [Bug] Objdump and Rizin CLI omit typed cancellation during final cleanup #3744

New issue

Closed

opened 2026-10-31 00:43:35 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

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

Owner

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

Before submitting

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

Area

CLI

Observed behavior

The registered inspect-with-objdump and execute-rizin-command CLI handlers return successful Evidence when SIGINT arrives during a retained provider-cleanup retry. Cleanup succeeds, but the result has no cancelled code while process.exitCode is 130.

This was executed with production command registration, ReverseEngineeringService, error/cleanup ownership and result promotion, using a simulated owned-command result and cleanup supervisor. No real objdump or Rizin engine ran. Additional service-seam controls reproduced the same result for list-objdump-architectures and directly confirmed the command signal was aborted before close returned.

Reproduction

In the built source checkout with locked dependencies, save the following as tests/boundary/cli/commandCleanupCancellation.test.ts and run:

node node_modules/vitest/vitest.mjs run --project cli-boundary \
  tests/boundary/cli/commandCleanupCancellation.test.ts \
  --maxWorkers=1 --no-file-parallelism

These are the four executed production-service cases from a ten-case focused run. Assertions characterize the current behavior. The simulated process has already completed with retained output; the production service owns and retries its unresolved cleanup. The signal event is emitted within that awaited retry, not at an arbitrary delay after dispatch. The selected package.json is an inert readable artifact; the simulated executable does not inspect it.

import pino from 'pino';
import {expect,it} from 'vitest';
import {registerReverseEngineeringCommands} from '../../../src/cli/reverseEngineeringCommands.js';
import {CLI_COMMANDS} from '../../../src/cliCommandNames.js';
import type {CliInstance} from '../../../src/cli/types.js';
import {ReverseEngineeringService as CoreService} from '../../../src/application/reverse/ReverseEngineeringService.js';
import {createReverseEngineeringCommandPort} from '../../../src/composition/reverseEngineering.js';
import {OwnedCommandFailure} from '../../../src/process/OwnedCommand.js';
import {providerSnapshot} from '../../support/reverseEngineeringCompositionFixtures.js';
import type {ProviderProcessSupervisor} from '../../../src/process/ProviderProcess.js';
for (const command of [CLI_COMMANDS.inspectWithObjdump,CLI_COMMANDS.executeRizinCommand]) {
 for (const interrupt of [false,true]) {
  it(`${command}: production service retained cleanup cancellation=${interrupt}`, async()=>{
   const commands=new Map<string,any>();
   const cli={command:(name:string,definition:any)=>{commands.set(name,definition);return cli;}} as unknown as CliInstance;
   let stops=0;
   const owner={stop:async()=>{
    stops++;
    await new Promise<void>(resolve=>setImmediate(()=>{if(interrupt)process.emit('SIGINT');resolve();}));
    return {status:'verified-cleanup' as const};
   }} as unknown as ProviderProcessSupervisor;
   const service=new CoreService({commands:createReverseEngineeringCommandPort({}),runOwned:async()=>{
    throw new OwnedCommandFailure('process','Cleanup after process completion was incomplete.',providerSnapshot(),'provider process still running',undefined,['owned-run'],owner);
   }});
   registerReverseEngineeringCommands(cli,pino({enabled:false}),{},undefined,()=>service);
   const previous=process.exitCode;
   try{
    const result=await commands.get(command).run({args:command===CLI_COMMANDS.executeRizinCommand?{path:'package.json',command:'iI'}:command===CLI_COMMANDS.listObjdumpArchitectures?{}:{path:'package.json',operation:'file_headers'},options:{followDebugLinks:false,detail:'complete'}});
    expect(stops).toBe(1);
    expect(result).toMatchObject({normalized_result:{completion_status:'complete'}});
    expect(result.code).toBeUndefined();
    expect(process.exitCode).toBe(interrupt?130:previous);
    console.log(JSON.stringify({command,productionService:true,interrupt,cleanupRetries:stops,returnedEvidence:typeof result.evidence_id==='string',returnedCode:result.code??null,exitCode:process.exitCode??null}));
   }finally{process.exitCode=previous;await service.close();}
  });
 }
}

Remove the temporary test after running it. process.emit('SIGINT') invokes the real CLI-installed listener in-process; this is a controlled interleaving, not an OS-signal subprocess or real-engine test.

Expected behavior

Return typed cancellation after the final awaited cleanup, retaining the completed Evidence as a partial observation and the caller's abort reason as the cause. Keep a pre-existing primary failure or genuinely unresolved cleanup classification when applicable.

The lifecycle contract explicitly requires this for cancellable observations, including cancellation observed only after provider completion. The lifecycle regression table requires typed cancellation after workload exit during finalization. These observation commands have no committed-output publication cutoff that would make late cancellation inapplicable.

Impact and scope

P2: structured consumers receive success-shaped Evidence after the caller cancelled, without the required typed cancellation or partial-observation provenance. Checking the signal-derived exit status lets CLI wrappers reject the result. The controlled tests establish the interleaving and reachable cleanup retry, not its frequency with real engines. This report covers the one-shot shared-helper commands; persistent debugger-manager paths and MCP behavior were not reproduced.

REA version

6.7.0; source revision 52415975942a4b9c53a817915b313ccc404f280e. Relevant code and lifecycle requirements remain unchanged at origin/main d8a9673fc44fde4bd30ff98c241886e6683580ca.

Runtime and platform

macOS 27.0.1 arm64; Node.js v26.12.0; Vitest 5.0.3. Simulated provider process completion and cleanup supervisor.

Target and artifact context

An unchanged local regular file used as the artifact, a completed zero-exit provider snapshot, and one retained cleanup owner that confirms release on retry. No native target process ran.

Interface

CLI

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

The full focused run passed 10 characterization cases: six command/service-seam cases plus four production-service cleanup cases. In both interrupted production-service cases:

{"cleanupRetries":1,"returnedEvidence":true,"returnedCode":null,"exitCode":130}

The uninterrupted production-service controls returned successful Evidence with the exit status unchanged. Direct service-seam cases additionally observed signalAborted: true and closeFinished: true before successful Evidence was returned for all three commands.

Typed outcome or recovery expected

AnalysisCancelledError projected as code: "cancelled", containing the completed Evidence in details.partial_observation; preserve signal.reason. SIGINT should retain the conventional 130 status after owned cleanup settles.

Analysis or suspected cause

Source-derived: executeWithCleanup has no signal parameter. It awaits service close, then either promotes recovered complete Evidence or returns the original result without checking cancellation. Production close awaits retained cleanup. The default objdump delivery returns Evidence directly when no output bundle is requested; it adds no compensating check. Rizin returns the helper's value directly. finishAnalysisCommand already expresses the missing post-close policy.

Additional context

#5967 concerns a lost output-truncation flag, #5670 concerns application cancellation causes/classification, and #5959/#5963 concern persistent Rizin cancellation construction. #6072 concerns a separate Frida finalizer; #6048/#6049/#6078/#6080/#6095 have other owners or publication-receipt contracts. PRs #1960/#1961 change retained ownership but still leave this helper without the cancellation check. None is the same fix.

NOT_ENOUGH_INFO: introducing commit, last known good release and frequency with real engines. No source fix or restored behavior was tested.

Originally created by @coygeek on GitHub (Oct 21, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/6111 ### 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 CLI ### Observed behavior The registered `inspect-with-objdump` and `execute-rizin-command` CLI handlers return successful Evidence when SIGINT arrives during a retained provider-cleanup retry. Cleanup succeeds, but the result has no `cancelled` code while `process.exitCode` is 130. This was executed with production command registration, `ReverseEngineeringService`, error/cleanup ownership and result promotion, using a simulated owned-command result and cleanup supervisor. No real objdump or Rizin engine ran. Additional service-seam controls reproduced the same result for `list-objdump-architectures` and directly confirmed the command signal was aborted before close returned. ### Reproduction In the built source checkout with locked dependencies, save the following as `tests/boundary/cli/commandCleanupCancellation.test.ts` and run: ```sh node node_modules/vitest/vitest.mjs run --project cli-boundary \ tests/boundary/cli/commandCleanupCancellation.test.ts \ --maxWorkers=1 --no-file-parallelism ``` These are the four executed production-service cases from a ten-case focused run. Assertions characterize the current behavior. The simulated process has already completed with retained output; the production service owns and retries its unresolved cleanup. The signal event is emitted within that awaited retry, not at an arbitrary delay after dispatch. The selected `package.json` is an inert readable artifact; the simulated executable does not inspect it. ```ts import pino from 'pino'; import {expect,it} from 'vitest'; import {registerReverseEngineeringCommands} from '../../../src/cli/reverseEngineeringCommands.js'; import {CLI_COMMANDS} from '../../../src/cliCommandNames.js'; import type {CliInstance} from '../../../src/cli/types.js'; import {ReverseEngineeringService as CoreService} from '../../../src/application/reverse/ReverseEngineeringService.js'; import {createReverseEngineeringCommandPort} from '../../../src/composition/reverseEngineering.js'; import {OwnedCommandFailure} from '../../../src/process/OwnedCommand.js'; import {providerSnapshot} from '../../support/reverseEngineeringCompositionFixtures.js'; import type {ProviderProcessSupervisor} from '../../../src/process/ProviderProcess.js'; for (const command of [CLI_COMMANDS.inspectWithObjdump,CLI_COMMANDS.executeRizinCommand]) { for (const interrupt of [false,true]) { it(`${command}: production service retained cleanup cancellation=${interrupt}`, async()=>{ const commands=new Map<string,any>(); const cli={command:(name:string,definition:any)=>{commands.set(name,definition);return cli;}} as unknown as CliInstance; let stops=0; const owner={stop:async()=>{ stops++; await new Promise<void>(resolve=>setImmediate(()=>{if(interrupt)process.emit('SIGINT');resolve();})); return {status:'verified-cleanup' as const}; }} as unknown as ProviderProcessSupervisor; const service=new CoreService({commands:createReverseEngineeringCommandPort({}),runOwned:async()=>{ throw new OwnedCommandFailure('process','Cleanup after process completion was incomplete.',providerSnapshot(),'provider process still running',undefined,['owned-run'],owner); }}); registerReverseEngineeringCommands(cli,pino({enabled:false}),{},undefined,()=>service); const previous=process.exitCode; try{ const result=await commands.get(command).run({args:command===CLI_COMMANDS.executeRizinCommand?{path:'package.json',command:'iI'}:command===CLI_COMMANDS.listObjdumpArchitectures?{}:{path:'package.json',operation:'file_headers'},options:{followDebugLinks:false,detail:'complete'}}); expect(stops).toBe(1); expect(result).toMatchObject({normalized_result:{completion_status:'complete'}}); expect(result.code).toBeUndefined(); expect(process.exitCode).toBe(interrupt?130:previous); console.log(JSON.stringify({command,productionService:true,interrupt,cleanupRetries:stops,returnedEvidence:typeof result.evidence_id==='string',returnedCode:result.code??null,exitCode:process.exitCode??null})); }finally{process.exitCode=previous;await service.close();} }); } } ``` Remove the temporary test after running it. `process.emit('SIGINT')` invokes the real CLI-installed listener in-process; this is a controlled interleaving, not an OS-signal subprocess or real-engine test. ### Expected behavior Return typed cancellation after the final awaited cleanup, retaining the completed Evidence as a partial observation and the caller's abort reason as the cause. Keep a pre-existing primary failure or genuinely unresolved cleanup classification when applicable. The [lifecycle contract](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/tool-design.md#L101-L111) explicitly requires this for cancellable observations, including cancellation observed only after provider completion. The [lifecycle regression table](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/testing.md#L257) requires typed cancellation after workload exit during finalization. These observation commands have no committed-output publication cutoff that would make late cancellation inapplicable. ### Impact and scope P2: structured consumers receive success-shaped Evidence after the caller cancelled, without the required typed cancellation or partial-observation provenance. Checking the signal-derived exit status lets CLI wrappers reject the result. The controlled tests establish the interleaving and reachable cleanup retry, not its frequency with real engines. This report covers the one-shot shared-helper commands; persistent debugger-manager paths and MCP behavior were not reproduced. ### REA version 6.7.0; source revision `52415975942a4b9c53a817915b313ccc404f280e`. Relevant code and lifecycle requirements remain unchanged at origin/main `d8a9673fc44fde4bd30ff98c241886e6683580ca`. ### Runtime and platform macOS 27.0.1 arm64; Node.js v26.12.0; Vitest 5.0.3. Simulated provider process completion and cleanup supervisor. ### Target and artifact context An unchanged local regular file used as the artifact, a completed zero-exit provider snapshot, and one retained cleanup owner that confirms release on retry. No native target process ran. ### Interface CLI ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence The full focused run passed 10 characterization cases: six command/service-seam cases plus four production-service cleanup cases. In both interrupted production-service cases: ```json {"cleanupRetries":1,"returnedEvidence":true,"returnedCode":null,"exitCode":130} ``` The uninterrupted production-service controls returned successful Evidence with the exit status unchanged. Direct service-seam cases additionally observed `signalAborted: true` and `closeFinished: true` before successful Evidence was returned for all three commands. ### Typed outcome or recovery expected `AnalysisCancelledError` projected as `code: "cancelled"`, containing the completed Evidence in `details.partial_observation`; preserve `signal.reason`. SIGINT should retain the conventional 130 status after owned cleanup settles. ### Analysis or suspected cause Source-derived: [executeWithCleanup](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/cli/reverseEngineeringCommands.ts#L31-L69) has no signal parameter. It awaits service close, then either promotes recovered complete Evidence or returns the original result without checking cancellation. [Production close](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/application/reverse/ReverseEngineeringService.ts#L167-L186) awaits retained cleanup. [The default objdump delivery](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/cli/analysisEvidenceDelivery.ts#L114-L129) returns Evidence directly when no output bundle is requested; it adds no compensating check. Rizin returns the helper's value directly. [finishAnalysisCommand](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/cli/finishAnalysisCommand.ts#L72-L78) already expresses the missing post-close policy. ### Additional context #5967 concerns a lost output-truncation flag, #5670 concerns application cancellation causes/classification, and #5959/#5963 concern persistent Rizin cancellation construction. #6072 concerns a separate Frida finalizer; #6048/#6049/#6078/#6080/#6095 have other owners or publication-receipt contracts. PRs #1960/#1961 change retained ownership but still leave this helper without the cancellation check. None is the same fix. NOT_ENOUGH_INFO: introducing commit, last known good release and frequency with real engines. No source fix or restored behavior was tested.

bsakaguchi closed this issue

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

bsakaguchi referenced this issue

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

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