morluto/rea

Fork 0

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

[GH-ISSUE #5956] [Bug] Script export loses typed cancellation when partial-output rollback fails #3627

New issue

Closed

opened 2026-10-31 00:42:30 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

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

Owner

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

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

Executed exportWebScripts with a valid two-script capture. After the first script was durably written, the signal was aborted with a specific Error. When the first rollback attempt was made to reject, the workflow returned cleanup_incomplete but changed the initiating failure to artifact_operation_failed. The returned error had neither the caller reason as cause nor a cleanupPrimary. With the same cancellation and successful rollback, the control returned AnalysisCancelledError / cancelled and preserved the exact caller reason. No live browser was needed.

Reproduction

Save the following as tests/boundary/browser/repro.test.ts in the source checkout with locked dependencies, then run:

node node_modules/vitest/vitest.mjs run tests/boundary/browser/repro.test.ts --maxWorkers 1

This is the executed test body with unrelated cases and unused imports removed. Capture reading, output creation, verified file writing, resource ownership and the successful cleanup retry use production code. The test aborts only after the first real tree.write completes and injects one tree.rollback rejection. The second script reaches the next loop’s throwIfAborted before publication.

import { mkdtemp, writeFile, access, rm } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { expect, it, vi } from 'vitest';
import { exportWebScripts } from '../../../src/application/WebScriptExportService.js';
import { ArtifactResourceScope } from '../../../src/artifacts/ArtifactResourceScope.js';
import { SafeOutputTree } from '../../../src/artifacts/SafeOutputTree.js';
import { scriptScenarioFixture } from '../../fixtures/webScriptCapture.js';
import { projectAnalysisError } from '../../../src/domain/analysisErrorProjection.js';

it.each([false,true])('export cancellation with rollback failure=%s', async failRollback => {
  const root = await mkdtemp(join(tmpdir(), 'export-cancel-'));
  const resources = new ArtifactResourceScope();
  const input = {capture_path:join(root,'capture.json'),output_directory:join(root,'out')};
  await writeFile(input.capture_path,JSON.stringify(scriptScenarioFixture([{url:'https://fixture.test/first.js',bytes:Buffer.from('export const a=1')},{url:'https://fixture.test/second.js',bytes:Buffer.from('export const b=2')}])));
  const controller = new AbortController();
  const reason = new Error('caller requested stop');
  const createTree = SafeOutputTree.create.bind(SafeOutputTree);
  let attempts = 0;
  let retainedTree: SafeOutputTree | undefined;
  const create = vi.spyOn(SafeOutputTree,'create').mockImplementationOnce(async (...args) => {
    const tree = await createTree(...args); retainedTree=tree;
    const write = tree.write.bind(tree);
    vi.spyOn(tree,'write').mockImplementationOnce(async (...writeArgs) => {
      const result = await write(...writeArgs);
      controller.abort(reason);
      return result;
    });
    const rollback = tree.rollback.bind(tree);
    vi.spyOn(tree,'rollback').mockImplementation(async () => {
      attempts++;
      if (failRollback && attempts === 1) throw new Error('controlled rollback rejection');
      return rollback();
    });
    return tree;
  });
  try {
    const result = await exportWebScripts(input,resources,{signal:controller.signal});
    if(result.ok) throw new Error('Expected cancellation failure');
    const p=projectAnalysisError(result.error);
    if (!failRollback) {
      expect(result.error._tag).toBe('AnalysisCancelledError');
      expect(result.error.cause).toBe(reason);
      await expect(access(input.output_directory)).rejects.toMatchObject({code:'ENOENT'});
    } else {
      expect(p.code).toBe('cleanup_incomplete');
      expect(p.details?.execution_failure).toBe('artifact_operation_failed');
      expect(result.error.cause).toBeUndefined();
      expect(result.error.cleanupPrimary).toBeUndefined();
      expect(resources.hasPending).toBe(true);
      await expect(access(input.output_directory)).resolves.toBeUndefined();
    }
    console.log('EXPORT',JSON.stringify({failRollback,tag:result.error._tag,code:p.code,execution_failure:p.details?.execution_failure,causeRetained:result.error.cause===reason,primary:result.error.cleanupPrimary?._tag,pending:resources.hasPending,published:retainedTree?.published}));
  } finally {
    create.mockRestore();
    await resources.close();
    await expect(access(input.output_directory)).rejects.toMatchObject({code:'ENOENT'});
    await rm(root,{recursive:true,force:true});
    vi.restoreAllMocks();
  }
});

Expected behavior

Keep cleanup_incomplete and the residual output path while rollback remains unresolved, but preserve typed cancellation as the primary failure and retain the exact caller abort reason. A failed rollback must not turn caller cancellation into an unrelated publication I/O failure.

The cancellation contract requires preserving primary outcomes and the caller abort reason through cleanup. The export contract requires rollback on cancellation and reporting residual paths.

Impact and scope

P2: an interrupted export with failed rollback loses its machine-readable cancellation origin and exact cause, although the output-tree owner is correctly retained. The trigger is bounded to cancellation plus rollback failure; successful rollback passed the control. The diagnostic text still includes the caller error’s message, which is not equivalent to retaining its typed cause. The residual output existed during the failure and was removed by the real cleanup retry. No data loss or security boundary crossing was observed. Real filesystem failure frequency, CLI exit behavior and MCP transport cancellation were not tested.

REA version

rea-agents 6.7.0; source revision 5241597594

Runtime and platform

macOS 27.0.1 arm64; Node.js v26.12.0; Vitest 5.0.3 with locked dependencies. Repository Node pin 24.18.0 was not exercised.

Target and artifact context

Synthetic retained browser-scenario capture containing two JavaScript response bodies; new local output directory; exact caller abort Error; one injected rollback rejection.

Interface

Other

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

The workflow run passed six cases, including these two export cases. Actual selected output:

{"failRollback":false,"tag":"AnalysisCancelledError","code":"cancelled","causeRetained":true,"pending":false,"published":false}
{"failRollback":true,"tag":"ArtifactOperationError","code":"cleanup_incomplete","execution_failure":"artifact_operation_failed","causeRetained":false,"pending":true,"published":false}

In the failing-rollback case cleanupPrimary was absent, the residual output directory existed, and ArtifactResourceScope retained its owner. Awaiting resources.close retried the same tree and removed the output. In the passing-rollback control the output was already absent and cause was strictly identical to the original abort Error.

Typed outcome or recovery expected

cleanup_incomplete with the residual path, retaining an AnalysisCancelledError primary whose cause is the caller’s original reason. Preserve the incomplete-cleanup classification until release succeeds; restoring typed cancellation must not hide rollback failure.

Analysis or suspected cause

Source-derived: the next-script cancellation checkpoint throws signal.reason. If rollback fails, the new WebScriptExportError selects io for this raw Error and receives residualPaths without cause or primary. mapExportFailure returns this wrapper before checking signal.aborted. WebScriptExportError marks cleanup incomplete without restoring cancellation. Source reachability: MCP handler and CLI handler call the shared application workflow tested above.

Additional context

Fetched origin/main at ce1f4b1bba; relevant publication and mapping code unchanged. Cancellation inside tree.write can carry ArtifactReaderFailure reason cancelled, so this report specifically demonstrates cancellation between script writes; it does not claim every cancellation interleaving loses the code. Related #5842 concerns standalone Evidence-export staging retry, #5865 concerns Frida partial-observation placement, and merged #1240 concerns capture-path selection failures. None changes this publication rollback path. No regression range was established.

Originally created by @coygeek on GitHub (Oct 21, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5956 ### 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 Executed exportWebScripts with a valid two-script capture. After the first script was durably written, the signal was aborted with a specific Error. When the first rollback attempt was made to reject, the workflow returned cleanup_incomplete but changed the initiating failure to artifact_operation_failed. The returned error had neither the caller reason as cause nor a cleanupPrimary. With the same cancellation and successful rollback, the control returned AnalysisCancelledError / cancelled and preserved the exact caller reason. No live browser was needed. ### Reproduction Save the following as tests/boundary/browser/repro.test.ts in the source checkout with locked dependencies, then run: ```sh node node_modules/vitest/vitest.mjs run tests/boundary/browser/repro.test.ts --maxWorkers 1 ``` This is the executed test body with unrelated cases and unused imports removed. Capture reading, output creation, verified file writing, resource ownership and the successful cleanup retry use production code. The test aborts only after the first real tree.write completes and injects one tree.rollback rejection. The second script reaches the next loop’s throwIfAborted before publication. ```ts import { mkdtemp, writeFile, access, rm } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { expect, it, vi } from 'vitest'; import { exportWebScripts } from '../../../src/application/WebScriptExportService.js'; import { ArtifactResourceScope } from '../../../src/artifacts/ArtifactResourceScope.js'; import { SafeOutputTree } from '../../../src/artifacts/SafeOutputTree.js'; import { scriptScenarioFixture } from '../../fixtures/webScriptCapture.js'; import { projectAnalysisError } from '../../../src/domain/analysisErrorProjection.js'; it.each([false,true])('export cancellation with rollback failure=%s', async failRollback => { const root = await mkdtemp(join(tmpdir(), 'export-cancel-')); const resources = new ArtifactResourceScope(); const input = {capture_path:join(root,'capture.json'),output_directory:join(root,'out')}; await writeFile(input.capture_path,JSON.stringify(scriptScenarioFixture([{url:'https://fixture.test/first.js',bytes:Buffer.from('export const a=1')},{url:'https://fixture.test/second.js',bytes:Buffer.from('export const b=2')}]))); const controller = new AbortController(); const reason = new Error('caller requested stop'); const createTree = SafeOutputTree.create.bind(SafeOutputTree); let attempts = 0; let retainedTree: SafeOutputTree | undefined; const create = vi.spyOn(SafeOutputTree,'create').mockImplementationOnce(async (...args) => { const tree = await createTree(...args); retainedTree=tree; const write = tree.write.bind(tree); vi.spyOn(tree,'write').mockImplementationOnce(async (...writeArgs) => { const result = await write(...writeArgs); controller.abort(reason); return result; }); const rollback = tree.rollback.bind(tree); vi.spyOn(tree,'rollback').mockImplementation(async () => { attempts++; if (failRollback && attempts === 1) throw new Error('controlled rollback rejection'); return rollback(); }); return tree; }); try { const result = await exportWebScripts(input,resources,{signal:controller.signal}); if(result.ok) throw new Error('Expected cancellation failure'); const p=projectAnalysisError(result.error); if (!failRollback) { expect(result.error._tag).toBe('AnalysisCancelledError'); expect(result.error.cause).toBe(reason); await expect(access(input.output_directory)).rejects.toMatchObject({code:'ENOENT'}); } else { expect(p.code).toBe('cleanup_incomplete'); expect(p.details?.execution_failure).toBe('artifact_operation_failed'); expect(result.error.cause).toBeUndefined(); expect(result.error.cleanupPrimary).toBeUndefined(); expect(resources.hasPending).toBe(true); await expect(access(input.output_directory)).resolves.toBeUndefined(); } console.log('EXPORT',JSON.stringify({failRollback,tag:result.error._tag,code:p.code,execution_failure:p.details?.execution_failure,causeRetained:result.error.cause===reason,primary:result.error.cleanupPrimary?._tag,pending:resources.hasPending,published:retainedTree?.published})); } finally { create.mockRestore(); await resources.close(); await expect(access(input.output_directory)).rejects.toMatchObject({code:'ENOENT'}); await rm(root,{recursive:true,force:true}); vi.restoreAllMocks(); } }); ``` ### Expected behavior Keep cleanup_incomplete and the residual output path while rollback remains unresolved, but preserve typed cancellation as the primary failure and retain the exact caller abort reason. A failed rollback must not turn caller cancellation into an unrelated publication I/O failure. The [cancellation contract](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/tool-design.md#L101-L110) requires preserving primary outcomes and the caller abort reason through cleanup. The [export contract](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/website-script-export.md#L30-L34) requires rollback on cancellation and reporting residual paths. ### Impact and scope P2: an interrupted export with failed rollback loses its machine-readable cancellation origin and exact cause, although the output-tree owner is correctly retained. The trigger is bounded to cancellation plus rollback failure; successful rollback passed the control. The diagnostic text still includes the caller error’s message, which is not equivalent to retaining its typed cause. The residual output existed during the failure and was removed by the real cleanup retry. No data loss or security boundary crossing was observed. Real filesystem failure frequency, CLI exit behavior and MCP transport cancellation were not tested. ### REA version rea-agents 6.7.0; source revision 52415975942a4b9c53a817915b313ccc404f280e ### Runtime and platform macOS 27.0.1 arm64; Node.js v26.12.0; Vitest 5.0.3 with locked dependencies. Repository Node pin 24.18.0 was not exercised. ### Target and artifact context Synthetic retained browser-scenario capture containing two JavaScript response bodies; new local output directory; exact caller abort Error; one injected rollback rejection. ### Interface Other ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence The workflow run passed six cases, including these two export cases. Actual selected output: ```json {"failRollback":false,"tag":"AnalysisCancelledError","code":"cancelled","causeRetained":true,"pending":false,"published":false} {"failRollback":true,"tag":"ArtifactOperationError","code":"cleanup_incomplete","execution_failure":"artifact_operation_failed","causeRetained":false,"pending":true,"published":false} ``` In the failing-rollback case cleanupPrimary was absent, the residual output directory existed, and ArtifactResourceScope retained its owner. Awaiting resources.close retried the same tree and removed the output. In the passing-rollback control the output was already absent and cause was strictly identical to the original abort Error. ### Typed outcome or recovery expected cleanup_incomplete with the residual path, retaining an AnalysisCancelledError primary whose cause is the caller’s original reason. Preserve the incomplete-cleanup classification until release succeeds; restoring typed cancellation must not hide rollback failure. ### Analysis or suspected cause Source-derived: the [next-script cancellation checkpoint](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/browser/assets/PublishWebScripts.ts#L66-L74) throws signal.reason. If [rollback fails](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/browser/assets/PublishWebScripts.ts#L118-L146), the new WebScriptExportError selects io for this raw Error and receives residualPaths without cause or primary. [mapExportFailure](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/application/WebScriptExportService.ts#L167-L190) returns this wrapper before checking signal.aborted. [WebScriptExportError](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/domain/webScriptExportError.ts#L9-L27) marks cleanup incomplete without restoring cancellation. Source reachability: [MCP handler](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/server/registerWebScriptTool.ts#L23-L48) and [CLI handler](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/cli/webScriptCommands.ts#L20-L48) call the shared application workflow tested above. ### Additional context Fetched origin/main at ce1f4b1bbaced57529d609f3007a76de583e11c4; relevant publication and mapping code unchanged. Cancellation inside tree.write can carry ArtifactReaderFailure reason cancelled, so this report specifically demonstrates cancellation between script writes; it does not claim every cancellation interleaving loses the code. Related #5842 concerns standalone Evidence-export staging retry, #5865 concerns Frida partial-observation placement, and merged #1240 concerns capture-path selection failures. None changes this publication rollback path. No regression range was established.

bsakaguchi closed this issue

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

bsakaguchi commented

2026-10-31 00:42:31 +00:00

Author

Owner

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

Checked the reported fix on main at d87e73f2db.

The original two-script trigger cancels after a durable write and reaches the next-script checkpoint. That checkpoint now throws AnalysisCancelledError with the exact signal.reason, rather than a raw Error. The publication catch recognizes typed cancellation (and cancellation from ArtifactReaderFailure); if output-tree rollback fails it returns AnalysisCancelledError retaining cancellation.cause, cleanup diagnostics and the residual root, and binds the retained cleanup lease to the error.

Implementation: PublishWebScripts.ts, PublishWebScripts.ts.

The relevant regression checks in webScriptExport.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. The original two-script trigger cancels after a durable write and reaches the next-script checkpoint. That checkpoint now throws AnalysisCancelledError with the exact signal.reason, rather than a raw Error. The publication catch recognizes typed cancellation (and cancellation from ArtifactReaderFailure); if output-tree rollback fails it returns AnalysisCancelledError retaining cancellation.cause, cleanup diagnostics and the residual root, and binds the retained cleanup lease to the error. Implementation: [PublishWebScripts.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/browser/assets/PublishWebScripts.ts#L50), [PublishWebScripts.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/browser/assets/PublishWebScripts.ts#L128). The relevant regression checks in [webScriptExport.test.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/tests/boundary/filesystem/webScriptExport.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-10-31 01:09:58 +00:00

[PR #3627] [CLOSED] fix(harmony): retain JavaScript runtime hints #5856

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

No description provided.

Rows
Columns