[GH-ISSUE #6065] [Bug] Managed snapshot final-close failure loses typed cancellation and its reason #3708

New issue

Closed

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

bsakaguchi commented

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

Owner

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

Before submitting

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

Area

Other

Observed behavior

If a managed snapshot read completes and cancellation arrives during a rejected final close, the producer records the exact abort reason but the managed translator returns only a cleanup error. There is no retained AnalysisCancelledError primary. Projection reports cleanup_incomplete with details.execution_failure: "execution_failure", losing the typed cancellation contract.

This was executed at the real managed provider/file boundary with synchronized close refusals. It was not an MCP transport cancellation test.

Reproduction

With locked dependencies and prepared artifacts, save this executed subset as tests/boundary/providers/managed/finalCloseCancellation.repro.test.ts, then run:

node node_modules/vitest/vitest.mjs run tests/boundary/providers/managed/finalCloseCancellation.repro.test.ts --maxWorkers=1

The tests assert the current defect and matched controls. Reading and managed execution use real local fixture bytes. Only close refusal and abort timing are controlled; permitting close invokes the real owner method.

import { writeFile } from 'node:fs/promises';
import { join } from 'node:path';
import { expect, it, vi } from 'vitest';
import { createTestTempDirectory } from '../../../fixtures/temporaryDirectory.js';
import { parseBinaryTarget } from '../../../../src/application/BinaryTargetResolver.js';
import { ManagedStaticProvider } from '../../../../src/dotnet/ManagedStaticProvider.js';
import { buildManagedPeFixture } from '../../../../src/dotnet/ManagedPe.fixture.js';
import { OwnedFileHandle } from '../../../../src/filesystem/OwnedFileHandle.js';
import { RegularFileCleanupFailure } from '../../../../src/application/RegularFileRead.js';
import { projectAnalysisError } from '../../../../src/domain/analysisErrorProjection.js';
import { AnalysisCancelledError } from '../../../../src/domain/analysisErrorCore.js';
import { createManagedBinarySession } from '../../../../src/composition/binary.js';
import { runManagedProviderExecution } from '../../../../src/application/DirectAnalysis.js';

async function fixture() {
  const directory = await createTestTempDirectory('rea-managed-lifecycle-');
  const path = join(directory, 'assembly.dll');
  await writeFile(path, buildManagedPeFixture());
  return path;
}
function injectClose(deny: () => boolean, abort?: () => void) {
  const original = OwnedFileHandle.prototype.close;
  const owners: OwnedFileHandle[] = [];
  const spy = vi.spyOn(OwnedFileHandle.prototype, 'close').mockImplementation(async function(this: OwnedFileHandle) {
    owners.push(this);
    abort?.();
    if (deny()) throw new Error('controlled close refusal');
    return original.call(this);
  });
  return { owners, spy };
}

it('completed snapshot final-close cancellation is recorded but never typed', async () => {
  const path = await fixture();
  const parsed = await parseBinaryTarget(path);
  if (!parsed.ok) throw parsed.error;
  const client = new ManagedStaticProvider().createClient(parsed.value);
  const controller = new AbortController();
  const reason = Object.freeze({request:'stop-at-final-close'});
  let deny=true;
  const { owners, spy } = injectClose(() => deny, () => controller.abort(reason));
  try {
    const result = await client.execute('inspect_managed_artifact', {}, {signal:controller.signal});
    if (result.ok) throw new Error('Expected final close failure');
    expect(result.error.cause).toBeInstanceOf(RegularFileCleanupFailure);
    const failure = result.error.cause as RegularFileCleanupFailure;
    expect(failure.outcome.kind).toBe('completed');
    expect(failure.completionCancellation?.reason).toBe(reason);
    expect(result.error.cleanupPrimary).toBeUndefined();
    expect(result.error).not.toBeInstanceOf(AnalysisCancelledError);
    expect(projectAnalysisError(result.error)).toMatchObject({code:'cleanup_incomplete',details:{execution_failure:'execution_failure'}});
    deny=false;
    expect((await client.close()).ok).toBe(true);
    expect(owners[0]?.handle.fd).toBe(-1);
    console.log(JSON.stringify({case:'completed-cancelled-close', completedRead:true, recordedReasonIdentical:true, typedCancellation:false, executionFailure:projectAnalysisError(result.error).details?.execution_failure, fd:owners[0]?.handle.fd}));
  } finally { deny=false; await client.close(); spy.mockRestore(); }
});

it('non-aborted execution and cancellation during successful final close are controls', async () => {
  const path = await fixture();
  const parsed = await parseBinaryTarget(path);
  if (!parsed.ok) throw parsed.error;
  const control = new ManagedStaticProvider().createClient(parsed.value);
  expect((await control.execute('inspect_managed_artifact', {})).ok).toBe(true);
  expect((await control.close()).ok).toBe(true);
  const client = new ManagedStaticProvider().createClient(parsed.value);
  const controller = new AbortController();
  const reason = Object.freeze({request:'successful-close-cancel'});
  const { spy } = injectClose(() => false, () => controller.abort(reason));
  try {
    const result = await client.execute('inspect_managed_artifact', {}, {signal:controller.signal});
    if (result.ok) throw new Error('Expected cancellation');
    expect(result.error).toBeInstanceOf(AnalysisCancelledError);
    expect(result.error.cause).toBe(reason);
    expect(result.error.cleanupIncomplete).toBe(false);
    console.log(JSON.stringify({case:'controls', ordinaryOk:true, successfulCloseCancellationReason:true}));
  } finally { await client.close(); spy.mockRestore(); }
});

Expected behavior

Preserve a typed AnalysisCancelledError primary with the exact recorded completion-cancellation reason as its cause. While close is unresolved, keep the outer cleanup_incomplete classification and owned resources. After confirmed cleanup, recover cancelled. Preserve any earlier physical read failure rather than replacing it with late cancellation; this reproduction has a completed read. See the finalization cancellation contract and synchronized lifecycle test guidance.

Impact and scope

P2: a caller cannot identify the cancellation or access its reason through the typed primary when cancellation coincides with final-close failure. The original signal and nested producer object still retain the reason; it is not erased from all internal state. The successful-close control produces the correct typed cancellation. No incorrect successful analysis, target modification, data loss, or persistent descriptor leak was observed.

The snapshot case and its controls reproduced on Node.js v24.18.0 and v26.12.0. Cached hashing uses the same translator by source inspection but was not separately executed. Natural close-failure frequency and public transport cancellation delivery were not measured.

REA version

rea-agents 6.7.0; source revision 52415975942a4b9c53a817915b313ccc404f280e.

Runtime and platform

macOS 27.0.1 arm64; Node.js v24.18.0 (repository pin) and v26.12.0; locked Vitest 5.0.3. No external provider.

Target and artifact context

Synthetic PE/CLI assembly from buildManagedPeFixture, read through a real file descriptor without executing target code.

Interface

Other

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

Observed on both runtimes:

{"case":"completed-cancelled-close","completedRead":true,"recordedReasonIdentical":true,"typedCancellation":false,"executionFailure":"execution_failure","fd":-1}
{"case":"controls","ordinaryOk":true,"successfulCloseCancellationReason":true}

The failed-close case asserted that RegularFileCleanupFailure.outcome.kind === "completed", its completionCancellation.reason is the exact supplied object, and the returned error has no cleanupPrimary. After close was permitted, client close succeeded and the descriptor became -1. The separate managed cleanup-binding defect means its old marker also remains stale; resolving that marker alone would not create the missing cancellation primary.

Typed outcome or recovery expected

Retain AnalysisCancelledError(operation, { cause: recordedReason }) as the typed primary beside the independent cleanup owner. Preserve reason identity internally; this report does not require serializing arbitrary opaque caller values into public JSON.

Analysis or suspected cause

Source-derived: the file owner records completionCancellation after completed reads whose final close rejects. managedCleanupError normalizes only failed read outcomes and ignores completionCancellation for completed outcomes. the managed client then returns the resulting AnalysisError unchanged. The shared artifact operation already handles this recorded completion fact. CLI/MCP reachability is source-derived from the shared managed workflow and MCP signal forwarding.

Additional context

Related #5985 concerns the managed entry checkpoint constructing a cancellation without a cause. This final-close path is caught and translated inside ManagedSnapshot and never reaches that entry constructor. A fix to either path alone leaves the other. #4381 / PR #4446 address hashing stream ownership and active-read cancellation. #4598 concerns physical failure precedence; #6043 concerns a different web-script capture consumer of completionCancellation.

The companion managed cleanup-binding report #6063 concerns owner-confirmed marker resolution; this case additionally requires constructing the missing cancellation primary. The two can share a refactor but enforce distinct invariants. Fetched upstream main 8e3bf56e11bcb56d0e8754fdfa32a7a9f004ca13 leaves this managed translator and owner provenance unchanged; newer main was inspected, not executed. No regression range was established.

Originally created by @coygeek on GitHub (Oct 21, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/6065 ### 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 Other ### Observed behavior If a managed snapshot read completes and cancellation arrives during a rejected final close, the producer records the exact abort reason but the managed translator returns only a cleanup error. There is no retained `AnalysisCancelledError` primary. Projection reports `cleanup_incomplete` with `details.execution_failure: "execution_failure"`, losing the typed cancellation contract. This was executed at the real managed provider/file boundary with synchronized close refusals. It was not an MCP transport cancellation test. ### Reproduction With locked dependencies and prepared artifacts, save this executed subset as `tests/boundary/providers/managed/finalCloseCancellation.repro.test.ts`, then run: ```sh node node_modules/vitest/vitest.mjs run tests/boundary/providers/managed/finalCloseCancellation.repro.test.ts --maxWorkers=1 ``` The tests assert the current defect and matched controls. Reading and managed execution use real local fixture bytes. Only close refusal and abort timing are controlled; permitting close invokes the real owner method. ```ts import { writeFile } from 'node:fs/promises'; import { join } from 'node:path'; import { expect, it, vi } from 'vitest'; import { createTestTempDirectory } from '../../../fixtures/temporaryDirectory.js'; import { parseBinaryTarget } from '../../../../src/application/BinaryTargetResolver.js'; import { ManagedStaticProvider } from '../../../../src/dotnet/ManagedStaticProvider.js'; import { buildManagedPeFixture } from '../../../../src/dotnet/ManagedPe.fixture.js'; import { OwnedFileHandle } from '../../../../src/filesystem/OwnedFileHandle.js'; import { RegularFileCleanupFailure } from '../../../../src/application/RegularFileRead.js'; import { projectAnalysisError } from '../../../../src/domain/analysisErrorProjection.js'; import { AnalysisCancelledError } from '../../../../src/domain/analysisErrorCore.js'; import { createManagedBinarySession } from '../../../../src/composition/binary.js'; import { runManagedProviderExecution } from '../../../../src/application/DirectAnalysis.js'; async function fixture() { const directory = await createTestTempDirectory('rea-managed-lifecycle-'); const path = join(directory, 'assembly.dll'); await writeFile(path, buildManagedPeFixture()); return path; } function injectClose(deny: () => boolean, abort?: () => void) { const original = OwnedFileHandle.prototype.close; const owners: OwnedFileHandle[] = []; const spy = vi.spyOn(OwnedFileHandle.prototype, 'close').mockImplementation(async function(this: OwnedFileHandle) { owners.push(this); abort?.(); if (deny()) throw new Error('controlled close refusal'); return original.call(this); }); return { owners, spy }; } it('completed snapshot final-close cancellation is recorded but never typed', async () => { const path = await fixture(); const parsed = await parseBinaryTarget(path); if (!parsed.ok) throw parsed.error; const client = new ManagedStaticProvider().createClient(parsed.value); const controller = new AbortController(); const reason = Object.freeze({request:'stop-at-final-close'}); let deny=true; const { owners, spy } = injectClose(() => deny, () => controller.abort(reason)); try { const result = await client.execute('inspect_managed_artifact', {}, {signal:controller.signal}); if (result.ok) throw new Error('Expected final close failure'); expect(result.error.cause).toBeInstanceOf(RegularFileCleanupFailure); const failure = result.error.cause as RegularFileCleanupFailure; expect(failure.outcome.kind).toBe('completed'); expect(failure.completionCancellation?.reason).toBe(reason); expect(result.error.cleanupPrimary).toBeUndefined(); expect(result.error).not.toBeInstanceOf(AnalysisCancelledError); expect(projectAnalysisError(result.error)).toMatchObject({code:'cleanup_incomplete',details:{execution_failure:'execution_failure'}}); deny=false; expect((await client.close()).ok).toBe(true); expect(owners[0]?.handle.fd).toBe(-1); console.log(JSON.stringify({case:'completed-cancelled-close', completedRead:true, recordedReasonIdentical:true, typedCancellation:false, executionFailure:projectAnalysisError(result.error).details?.execution_failure, fd:owners[0]?.handle.fd})); } finally { deny=false; await client.close(); spy.mockRestore(); } }); it('non-aborted execution and cancellation during successful final close are controls', async () => { const path = await fixture(); const parsed = await parseBinaryTarget(path); if (!parsed.ok) throw parsed.error; const control = new ManagedStaticProvider().createClient(parsed.value); expect((await control.execute('inspect_managed_artifact', {})).ok).toBe(true); expect((await control.close()).ok).toBe(true); const client = new ManagedStaticProvider().createClient(parsed.value); const controller = new AbortController(); const reason = Object.freeze({request:'successful-close-cancel'}); const { spy } = injectClose(() => false, () => controller.abort(reason)); try { const result = await client.execute('inspect_managed_artifact', {}, {signal:controller.signal}); if (result.ok) throw new Error('Expected cancellation'); expect(result.error).toBeInstanceOf(AnalysisCancelledError); expect(result.error.cause).toBe(reason); expect(result.error.cleanupIncomplete).toBe(false); console.log(JSON.stringify({case:'controls', ordinaryOk:true, successfulCloseCancellationReason:true})); } finally { await client.close(); spy.mockRestore(); } }); ``` ### Expected behavior Preserve a typed `AnalysisCancelledError` primary with the exact recorded completion-cancellation reason as its cause. While close is unresolved, keep the outer `cleanup_incomplete` classification and owned resources. After confirmed cleanup, recover `cancelled`. Preserve any earlier physical read failure rather than replacing it with late cancellation; this reproduction has a completed read. See the [finalization cancellation contract](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/tool-design.md#L101-L110) and [synchronized lifecycle test guidance](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/testing.md#L268-L275). ### Impact and scope P2: a caller cannot identify the cancellation or access its reason through the typed primary when cancellation coincides with final-close failure. The original signal and nested producer object still retain the reason; it is not erased from all internal state. The successful-close control produces the correct typed cancellation. No incorrect successful analysis, target modification, data loss, or persistent descriptor leak was observed. The snapshot case and its controls reproduced on Node.js v24.18.0 and v26.12.0. Cached hashing uses the same translator by source inspection but was not separately executed. Natural close-failure frequency and public transport cancellation delivery were not measured. ### REA version rea-agents 6.7.0; source revision `52415975942a4b9c53a817915b313ccc404f280e`. ### Runtime and platform macOS 27.0.1 arm64; Node.js v24.18.0 (repository pin) and v26.12.0; locked Vitest 5.0.3. No external provider. ### Target and artifact context Synthetic PE/CLI assembly from buildManagedPeFixture, read through a real file descriptor without executing target code. ### Interface Other ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence Observed on both runtimes: ```json {"case":"completed-cancelled-close","completedRead":true,"recordedReasonIdentical":true,"typedCancellation":false,"executionFailure":"execution_failure","fd":-1} {"case":"controls","ordinaryOk":true,"successfulCloseCancellationReason":true} ``` The failed-close case asserted that `RegularFileCleanupFailure.outcome.kind === "completed"`, its `completionCancellation.reason` is the exact supplied object, and the returned error has no `cleanupPrimary`. After close was permitted, client close succeeded and the descriptor became -1. The separate managed cleanup-binding defect means its old marker also remains stale; resolving that marker alone would not create the missing cancellation primary. ### Typed outcome or recovery expected Retain `AnalysisCancelledError(operation, { cause: recordedReason })` as the typed primary beside the independent cleanup owner. Preserve reason identity internally; this report does not require serializing arbitrary opaque caller values into public JSON. ### Analysis or suspected cause Source-derived: [the file owner](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/filesystem/OwnedRegularFile.ts#L162-L191) records completionCancellation after completed reads whose final close rejects. [managedCleanupError](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/dotnet/ManagedSnapshot.ts#L144-L180) normalizes only failed read outcomes and ignores completionCancellation for completed outcomes. [the managed client](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/dotnet/ManagedStaticProvider.ts#L134-L138) then returns the resulting AnalysisError unchanged. The [shared artifact operation](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/artifacts/ArtifactFileOperation.ts#L47-L66) already handles this recorded completion fact. CLI/MCP reachability is source-derived from [the shared managed workflow](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/application/DirectAnalysis.ts#L180-L200) and [MCP signal forwarding](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/server/registerManagedTools.ts#L54-L59). ### Additional context Related [#5985](https://github.com/morluto/rea/issues/5985) concerns the managed entry checkpoint constructing a cancellation without a cause. This final-close path is caught and translated inside ManagedSnapshot and never reaches that entry constructor. A fix to either path alone leaves the other. #4381 / PR #4446 address hashing stream ownership and active-read cancellation. #4598 concerns physical failure precedence; #6043 concerns a different web-script capture consumer of completionCancellation. The companion managed cleanup-binding report [#6063](https://github.com/morluto/rea/issues/6063) concerns owner-confirmed marker resolution; this case additionally requires constructing the missing cancellation primary. The two can share a refactor but enforce distinct invariants. Fetched upstream main `8e3bf56e11bcb56d0e8754fdfa32a7a9f004ca13` leaves this managed translator and owner provenance unchanged; newer main was inspected, not executed. No regression range was established.

bsakaguchi closed this issue

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

bsakaguchi referenced this issue

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

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