morluto/rea

Fork 0

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

[GH-ISSUE #6092] [Bug] JavaScript reconstruction loses typed cancellation when worker cleanup fails #3726

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/6092

Before submitting

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

Area

Evidence, artifacts, or reconstruction

Observed behavior

JavaScript reconstruction discards its typed cancellation when the analysis worker also fails final cleanup. While cleanup is pending the result is correctly classified cleanup_incomplete, but it has no cleanupPrimary. After the exact resource owner successfully retries cleanup, the same returned error changes to execution_failure instead of the original cancelled. Partial application Evidence survives.

Executed through the production static JavaScript application service with real files and the actual source-analysis worker. A controlled private-runtime removal denial produced the cleanup failure after process stop. Cancellation and cleanup retry were synchronized at their owning boundaries; no live process cleanup denial is claimed.

Reproduction

With the locked dependencies and built worker available, save the following test as tests/boundary/filesystem/javascriptCleanupPrimary.test.ts. Set REA_VERIFY_ROOT to an empty disposable directory and run:

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

These are the executed JavaScript characterization cases. worker-cancel aborts at the second actual analyze call, then invokes the original worker method. worker-cancel-cleanup adds only final private-root close denial. The between-file cases independently cover the original application-loop boundary. All spies are restored and exact-scope cleanup is awaited before fixture removal.

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.each(['control','graph','cancel','cancel-cleanup','worker-cancel','worker-cancel-cleanup'] as const)('characterizes JavaScript completion: %s',async mode=>{
 const root = await mkdtemp(join(process.env.REA_VERIFY_ROOT!, 'js-'));
 await writeFile(join(root,'a.js'),'module.exports = 42;\n');
 await writeFile(join(root,'b.js'),'module.exports = 43;\n');
 const scope = new ArtifactResourceScope();
 const controller = new AbortController();const reason={requested:'stop',mode};
 const original = PrivateRuntimeRoot.prototype.close;
 let denied=0;let parsed=0;let armed=false;
 const spy=vi.spyOn(PrivateRuntimeRoot.prototype,'close').mockImplementation(async function(this:PrivateRuntimeRoot){
  if((mode==='cancel-cleanup'||mode==='worker-cancel-cleanup')&&armed){denied++;throw new Error('controlled private runtime removal denial');}
  return original.call(this);
 });
 let calls=0;const analyze=JavaScriptAnalysisWorker.prototype.analyze;
 const analyzeSpy=vi.spyOn(JavaScriptAnalysisWorker.prototype,'analyze').mockImplementation(function(this:JavaScriptAnalysisWorker,...args){calls++;if(calls===2&&(mode==='worker-cancel'||mode==='worker-cancel-cleanup')){armed=true;controller.abort(reason);}return analyze.apply(this,args);});
 try {
  const result=await analyzeJavaScriptApplication({input_path:root},scope,{signal:controller.signal,progress:{async report(update){
   if(update.phase==='parse_javascript_source'){parsed++;if(parsed===2&&(mode==='cancel'||mode==='cancel-cleanup')){armed=true;controller.abort(reason);}}
   if(mode==='graph'&&update.phase==='build_javascript_application_graph')controller.abort(reason);
  }}});
  if(mode==='control'){expect(result.ok).toBe(true);console.log(JSON.stringify({kind:mode,ok:result.ok}));return;}
  expect(result.ok).toBe(false);if(result.ok)throw new Error('expected failure');
  const projected=projectAnalysisError(result.error);
  console.log(JSON.stringify({kind:mode,tag:result.error._tag,code:projected.code,cause_is_reason:result.error.cause===reason,cause_absent:result.error.cause===undefined,cleanup_primary:(result.error as any).cleanupPrimary?._tag,partial:result.error.partialObservation!==undefined,pending:scope.hasPending,denied,diagnostics:result.error.diagnostics}));
  expect(result.error.partialObservation).toBeDefined();
  if(mode==='graph'){expect(projected.code).toBe('cancelled');expect(result.error.cause).toBeUndefined();}
  if(mode==='cancel'||mode==='worker-cancel')expect(projected.code).toBe('cancelled');
  if(mode==='cancel-cleanup'||mode==='worker-cancel-cleanup'){expect(denied).toBeGreaterThan(0);expect(result.error).toBeInstanceOf(ProviderCleanupError);expect((result.error as any).cleanupPrimary).toBeUndefined();expect(projected.code).toBe('cleanup_incomplete');expect(scope.hasPending).toBe(true);spy.mockRestore();await scope.close();console.log(JSON.stringify({kind:'after-retry',code:projectAnalysisError(result.error).code,cleanup_incomplete:result.error.cleanupIncomplete,pending:scope.hasPending,cleanup_primary:(result.error as any).cleanupPrimary?._tag}));expect(projectAnalysisError(result.error).code).toBe('execution_failure');expect(result.error.cleanupIncomplete).toBe(false);}
 } finally {analyzeSpy.mockRestore();spy.mockRestore();await scope.close();expect(scope.hasPending).toBe(false);await rm(root,{recursive:true,force:true});}
});

Expected behavior

Retain the typed primary alongside independently classified incomplete cleanup, and project the primary again after confirmed release. The lifecycle contract requires primary preservation; cleanup regression guidance requires the original diagnostic and owner recovery to survive. cleanup_incomplete may take precedence while resources remain; it must not erase the typed cancellation.

Impact and scope

P2: machine-readable consumers cannot recover the primary cancellation classification after successful cleanup; they receive a generic execution failure. Partial Evidence and the real cleanup owner remain retained. In the worker-observed case the original cancellation also survives in a nested cause and historical diagnostic tag/message, but it is absent from typed primary composition and cannot drive normal projection. A consumer retaining its signal can still know it cancelled. Timeout shares the same reconstruction selection by source inspection, but was not reproduced. No data loss or naturally occurring filesystem refusal was observed.

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 directory containing two JavaScript files exporting numeric values. Actual source worker and resource scope; only cancellation timing and private-runtime close refusal are controlled. Target JavaScript is not executed.

Interface

Other

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

Final focused group: one file passed, seven tests passed. Selected results:

normal analysis: success
worker-cancel: AnalysisCancelledError, cancelled, exact caller cause, partial Evidence, no pending cleanup
worker-cancel-cleanup: ProviderCleanupError instance, _tag ProviderAdapterError, cleanup_incomplete, no cleanupPrimary, partial Evidence, pending cleanup
after restoring removal and awaiting exact scope.close: cleanupIncomplete false, pending false, projected code execution_failure

The cleanup fault had worker_pid: null, confirming that this demonstrated fault concerns retained private-root removal after process stop. The independent between-file pair also changes from cancelled to cleanup_incomplete and then execution_failure after release.

Typed outcome or recovery expected

Preserve the original AnalysisCancelledError as the typed cleanup primary, its exact cause where present, partial Evidence, and independently owned cleanup resources. Successful exact-owner cleanup should restore cancelled projection. Do not recover the type by parsing historical diagnostic text.

Analysis or suspected cause

The producer is reconstructJavaScriptArtifact: after worker release, cleanupFailure ?? outcome.interruption selects one error and drops the other. The selected error receives Evidence and is thrown. The service returns it unchanged. Worker close translates runtime removal failure into an owned cleanup error. Existing typed primary composition can retain both errors, but this producer bypasses it.

Additional context

#4628 is already fixed in this revision: it repairs typed-primary representation inside shared composition. This failure occurs before that helper is called; changing a shared CLI finalizer cannot restore a primary discarded inside reconstruction. #3935/#4240 concern other CLI finalization layers; #1653/PR #1654 retries a prior worker during preparation; #5393 is exit/cancellation ordering; #6083 drops a per-file application projection guard. #5992, #5989, #5963 and #6047 affect separate catalog/CDP/Rizin/Android producers. #5987 covers the separate abort-reason loss at JavaScript application checkpoints.

Fetched main d8a9673fc44fde4bd30ff98c241886e6683580ca and inspected the newer scoped-worker wrapper. Healthy workers may now park, changing the between-file timing. Cancelled/failed worker results still set that wrapper's failed flag and take physical worker close; the reconstruction error-selection expression remains unchanged. That newer runtime was source-inspected, not executed. No last-known-good version or public CLI/MCP transport reproduction was established.

Originally created by @coygeek on GitHub (Oct 21, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/6092 ### 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 Evidence, artifacts, or reconstruction ### Observed behavior JavaScript reconstruction discards its typed cancellation when the analysis worker also fails final cleanup. While cleanup is pending the result is correctly classified `cleanup_incomplete`, but it has no `cleanupPrimary`. After the exact resource owner successfully retries cleanup, the same returned error changes to `execution_failure` instead of the original `cancelled`. Partial application Evidence survives. Executed through the production static JavaScript application service with real files and the actual source-analysis worker. A controlled private-runtime removal denial produced the cleanup failure after process stop. Cancellation and cleanup retry were synchronized at their owning boundaries; no live process cleanup denial is claimed. ### Reproduction With the locked dependencies and built worker available, save the following test as `tests/boundary/filesystem/javascriptCleanupPrimary.test.ts`. Set `REA_VERIFY_ROOT` to an empty disposable directory and run: ```sh node node_modules/vitest/vitest.mjs run tests/boundary/filesystem/javascriptCleanupPrimary.test.ts --maxWorkers=1 ``` These are the executed JavaScript characterization cases. `worker-cancel` aborts at the second actual analyze call, then invokes the original worker method. `worker-cancel-cleanup` adds only final private-root close denial. The between-file cases independently cover the original application-loop boundary. All spies are restored and exact-scope cleanup is awaited before fixture removal. ```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.each(['control','graph','cancel','cancel-cleanup','worker-cancel','worker-cancel-cleanup'] as const)('characterizes JavaScript completion: %s',async mode=>{ const root = await mkdtemp(join(process.env.REA_VERIFY_ROOT!, 'js-')); await writeFile(join(root,'a.js'),'module.exports = 42;\n'); await writeFile(join(root,'b.js'),'module.exports = 43;\n'); const scope = new ArtifactResourceScope(); const controller = new AbortController();const reason={requested:'stop',mode}; const original = PrivateRuntimeRoot.prototype.close; let denied=0;let parsed=0;let armed=false; const spy=vi.spyOn(PrivateRuntimeRoot.prototype,'close').mockImplementation(async function(this:PrivateRuntimeRoot){ if((mode==='cancel-cleanup'||mode==='worker-cancel-cleanup')&&armed){denied++;throw new Error('controlled private runtime removal denial');} return original.call(this); }); let calls=0;const analyze=JavaScriptAnalysisWorker.prototype.analyze; const analyzeSpy=vi.spyOn(JavaScriptAnalysisWorker.prototype,'analyze').mockImplementation(function(this:JavaScriptAnalysisWorker,...args){calls++;if(calls===2&&(mode==='worker-cancel'||mode==='worker-cancel-cleanup')){armed=true;controller.abort(reason);}return analyze.apply(this,args);}); try { const result=await analyzeJavaScriptApplication({input_path:root},scope,{signal:controller.signal,progress:{async report(update){ if(update.phase==='parse_javascript_source'){parsed++;if(parsed===2&&(mode==='cancel'||mode==='cancel-cleanup')){armed=true;controller.abort(reason);}} if(mode==='graph'&&update.phase==='build_javascript_application_graph')controller.abort(reason); }}}); if(mode==='control'){expect(result.ok).toBe(true);console.log(JSON.stringify({kind:mode,ok:result.ok}));return;} expect(result.ok).toBe(false);if(result.ok)throw new Error('expected failure'); const projected=projectAnalysisError(result.error); console.log(JSON.stringify({kind:mode,tag:result.error._tag,code:projected.code,cause_is_reason:result.error.cause===reason,cause_absent:result.error.cause===undefined,cleanup_primary:(result.error as any).cleanupPrimary?._tag,partial:result.error.partialObservation!==undefined,pending:scope.hasPending,denied,diagnostics:result.error.diagnostics})); expect(result.error.partialObservation).toBeDefined(); if(mode==='graph'){expect(projected.code).toBe('cancelled');expect(result.error.cause).toBeUndefined();} if(mode==='cancel'||mode==='worker-cancel')expect(projected.code).toBe('cancelled'); if(mode==='cancel-cleanup'||mode==='worker-cancel-cleanup'){expect(denied).toBeGreaterThan(0);expect(result.error).toBeInstanceOf(ProviderCleanupError);expect((result.error as any).cleanupPrimary).toBeUndefined();expect(projected.code).toBe('cleanup_incomplete');expect(scope.hasPending).toBe(true);spy.mockRestore();await scope.close();console.log(JSON.stringify({kind:'after-retry',code:projectAnalysisError(result.error).code,cleanup_incomplete:result.error.cleanupIncomplete,pending:scope.hasPending,cleanup_primary:(result.error as any).cleanupPrimary?._tag}));expect(projectAnalysisError(result.error).code).toBe('execution_failure');expect(result.error.cleanupIncomplete).toBe(false);} } finally {analyzeSpy.mockRestore();spy.mockRestore();await scope.close();expect(scope.hasPending).toBe(false);await rm(root,{recursive:true,force:true});} }); ``` ### Expected behavior Retain the typed primary alongside independently classified incomplete cleanup, and project the primary again after confirmed release. [The lifecycle contract](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/tool-design.md#L101-L110) requires primary preservation; [cleanup regression guidance](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/testing.md#L257-L259) requires the original diagnostic and owner recovery to survive. `cleanup_incomplete` may take precedence while resources remain; it must not erase the typed cancellation. ### Impact and scope P2: machine-readable consumers cannot recover the primary cancellation classification after successful cleanup; they receive a generic execution failure. Partial Evidence and the real cleanup owner remain retained. In the worker-observed case the original cancellation also survives in a nested cause and historical diagnostic tag/message, but it is absent from typed primary composition and cannot drive normal projection. A consumer retaining its signal can still know it cancelled. Timeout shares the same reconstruction selection by source inspection, but was not reproduced. No data loss or naturally occurring filesystem refusal was observed. ### 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 directory containing two JavaScript files exporting numeric values. Actual source worker and resource scope; only cancellation timing and private-runtime close refusal are controlled. Target JavaScript is not executed. ### Interface Other ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence Final focused group: one file passed, seven tests passed. Selected results: ```text normal analysis: success worker-cancel: AnalysisCancelledError, cancelled, exact caller cause, partial Evidence, no pending cleanup worker-cancel-cleanup: ProviderCleanupError instance, _tag ProviderAdapterError, cleanup_incomplete, no cleanupPrimary, partial Evidence, pending cleanup after restoring removal and awaiting exact scope.close: cleanupIncomplete false, pending false, projected code execution_failure ``` The cleanup fault had `worker_pid: null`, confirming that this demonstrated fault concerns retained private-root removal after process stop. The independent between-file pair also changes from cancelled to cleanup_incomplete and then execution_failure after release. ### Typed outcome or recovery expected Preserve the original AnalysisCancelledError as the typed cleanup primary, its exact cause where present, partial Evidence, and independently owned cleanup resources. Successful exact-owner cleanup should restore `cancelled` projection. Do not recover the type by parsing historical diagnostic text. ### Analysis or suspected cause The producer is [reconstructJavaScriptArtifact](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/application/javascript/JavaScriptArtifactReconstruction.ts#L197-L218): after worker release, `cleanupFailure ?? outcome.interruption` selects one error and drops the other. [The selected error receives Evidence and is thrown](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/application/javascript/JavaScriptArtifactReconstruction.ts#L298-L312). [The service returns it unchanged](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/application/javascript/JavaScriptApplicationService.ts#L142-L145). Worker [close translates runtime removal failure](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/javascript/analysis/JavaScriptAnalysisWorker.ts#L497-L516) into an owned cleanup error. Existing [typed primary composition](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/domain/analysisErrorCleanup.ts#L5-L35) can retain both errors, but this producer bypasses it. ### Additional context #4628 is already fixed in this revision: it repairs typed-primary representation inside shared composition. This failure occurs before that helper is called; changing a shared CLI finalizer cannot restore a primary discarded inside reconstruction. #3935/#4240 concern other CLI finalization layers; #1653/PR #1654 retries a prior worker during preparation; #5393 is exit/cancellation ordering; #6083 drops a per-file application projection guard. #5992, #5989, #5963 and #6047 affect separate catalog/CDP/Rizin/Android producers. #5987 covers the separate abort-reason loss at JavaScript application checkpoints. Fetched main `d8a9673fc44fde4bd30ff98c241886e6683580ca` and inspected the newer scoped-worker wrapper. Healthy workers may now park, changing the between-file timing. Cancelled/failed worker results still set that wrapper's failed flag and take physical worker close; the reconstruction error-selection expression remains unchanged. That newer runtime was source-inspected, not executed. No last-known-good version or public CLI/MCP transport reproduction was established.

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 on main at d87e73f2db50be72d33a4b3c7199af35518b441c: Reconstruction now composes the interruption with worker cleanup failure. The regression retains the exact caller primary and partial Evidence, then checks cancelled projection after exact-scope cleanup succeeds. JavaScriptArtifactReconstruction.ts JavaScriptArtifactReconstruction.ts javascriptApplicationCancellation.test.ts

I ran the cited regression file on this revision; all passed as part of the focused 28-file, 603-test run. Provider-facing checks here use controlled fixtures; this does not claim a run with the real analysis engine. Closing as completed on main.

@coygeek commented on GitHub (Oct 24, 2026): Checked on main at `d87e73f2db50be72d33a4b3c7199af35518b441c`: Reconstruction now composes the interruption with worker cleanup failure. The regression retains the exact caller primary and partial Evidence, then checks cancelled projection after exact-scope cleanup succeeds. [JavaScriptArtifactReconstruction.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/application/javascript/JavaScriptArtifactReconstruction.ts#L376) [JavaScriptArtifactReconstruction.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/application/javascript/JavaScriptArtifactReconstruction.ts#L401) [javascriptApplicationCancellation.test.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/tests/boundary/javascript/javascriptApplicationCancellation.test.ts#L19) I ran the cited regression file on this revision; all passed as part of the focused 28-file, 603-test run. Provider-facing checks here use controlled fixtures; this does not claim a run with the real analysis engine. 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#3726

No description provided.

Rows
Columns