morluto/rea

Fork 0

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

[GH-ISSUE #5868] [Bug] Historical capture reply-read cancellation becomes a provider error and drops completed Evidence #3556

New issue

Closed

opened 2026-10-31 00:41:50 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-31 00:41:50 +00:00

Owner

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

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

Cancellation during the successful final close of a historical capture decoder's reply file returns ProviderAdapterError, with phase: "decoder" and failure_kind: "cancelled", instead of AnalysisCancelledError. No partial capture observation is attached, even though the real HAR decoder has finished and the reply bytes have been fully read.

I reproduced this at the built shared service used by inspect-web-network-capture and inspect_web_network_capture. Native file reading, descriptor closure, and the packaged HAR decoder subprocess were real. A controlled close hook injected cancellation immediately after the reply descriptor closed successfully. This did not exercise CLI signal delivery or an MCP cancellation notification.

Reproduction

In a prepared source checkout at the revision below, run the following focused Vitest test. The normal control succeeds. The cancellation case characterizes the incorrect error and confirms that its nested file-reader outcome contains the completed read while the service error has no partial observation. Both cases passed in two focused runs.

To rerun, save the test as tests/process-global/captureReplyCancellation.test.ts and run node node_modules/vitest/vitest.mjs run tests/process-global/captureReplyCancellation.test.ts --maxWorkers 1.

import { mkdtemp, rm, writeFile } from "node:fs/promises";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { expect, it, vi } from "vitest";
import { createWebNetworkCaptureService } from "../../dist/composition/webNetworkCaptures.js";
import { OwnedFileHandle } from "../../dist/filesystem/OwnedFileHandle.js";
import { historicalHar } from "../fixtures/historicalHar.js";

it.each([false, true])("real HAR reply read; cancel on reply close=%s", async (cancel) => {
  const root = await mkdtemp(join(tmpdir(), "rea-capture-reply-"));
  const capturePath = join(root, "capture.har");
  await writeFile(capturePath, JSON.stringify(historicalHar()));
  const controller = new AbortController();
  const reason = { request: "cancel during reply descriptor close" };
  const nativeClose = OwnedFileHandle.prototype.close;
  let completedReplySeen = false;
  const close = vi.spyOn(OwnedFileHandle.prototype, "close").mockImplementation(async function (this: OwnedFileHandle) {
    const handle = this.handle;
    if (handle.fd >= 0 && "read" in handle) {
      const prefix = Buffer.alloc(11);
      const { bytesRead } = await handle.read(prefix, 0, prefix.length, 0);
      if (prefix.subarray(0, bytesRead).toString() === '{"ok":true,') {
        completedReplySeen = true;
        await nativeClose.call(this);
        if (cancel) controller.abort(reason);
        return;
      }
    }
    await nativeClose.call(this);
  });
  const service = createWebNetworkCaptureService(process.env);
  try {
    const result = await service.inspect({ capture_path: capturePath, format: "har", record_ordinals: [0] }, { signal: controller.signal });
    expect(completedReplySeen).toBe(true);
    if (!cancel) {
      expect(result.ok).toBe(true);
      console.log(JSON.stringify({ cancel, ok: result.ok, completedReplySeen }));
    } else {
      expect(result.ok).toBe(false);
      if (result.ok) throw new Error("Expected cancellation");
      console.log(JSON.stringify({ cancel, tag: result.error._tag, failureKind: result.error.diagnostics.failure_kind, hasPartialObservation: result.error.partialObservation !== undefined, causeIsCancellationReason: result.error.cause === reason, physicalReadOutcome: result.error.cause.fileCancellation.outcome.kind, completedReplySeen }));
      expect(result.error._tag).toBe("ProviderAdapterError");
      expect(result.error.diagnostics).toMatchObject({ phase: "decoder", failure_kind: "cancelled" });
      expect(result.error.partialObservation).toBeUndefined();
      expect(result.error.cause.fileCancellation.outcome.kind).toBe("completed");
      expect(result.error.cause.fileCancellation.reason).toBe(reason);
    }
  } finally {
    close.mockRestore();
    await service.close();
    await rm(root, { recursive: true, force: true });
  }
});

Expected behavior

Return typed cancellation with the caller's exact reason and retain the completed, validated capture as Evidence under the requested record selection and sensitivity rules.

The historical-capture contract explicitly requires completed decoding to survive cancellation. The lifecycle regression guidance includes cancellation after workload exit during result reads.

Impact and scope

P2: a bounded cancellation window changes the error category and prevents completed capture Evidence from reaching the shared service result. The unmodified original capture can be inspected again; the control without cancellation succeeded. Natural occurrence frequency is unknown. HAR was executed; other historical formats share the source path but were not tested. No original-artifact loss or persistent resource leak was observed.

REA version

5241597594; package metadata reports 6.7.0.

Runtime and platform

macOS 27.0.1 arm64, Node.js v24.22.0, Vitest 5.0.3.

Target and artifact context

The repository's authored HAR 1.2 fixture and the packaged HAR decoder. One selected producer record; no browser connection or recorded-URL fetching.

Interface

Other

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

Focused run: 1 test file passed, 2 tests passed. Relevant output, with paths omitted:

{"cancel":false,"ok":true,"completedReplySeen":true}
{"cancel":true,"tag":"ProviderAdapterError","failureKind":"cancelled","hasPartialObservation":false,"causeIsCancellationReason":false,"physicalReadOutcome":"completed","completedReplySeen":true}

The test also checks that result.error.cause.fileCancellation.reason is the exact supplied reason object. The reader still carries it, together with the completed read; the top-level service result does not expose the expected cancellation type or partial capture.

Typed outcome or recovery expected

AnalysisCancelledError, with the supplied reason as its cause and the completed selected capture retained as partial Evidence. Earlier genuine decode or validation errors should retain their precedence.

Analysis or suspected cause

The reply read and catch propagate the request signal and immediately rethrow ArtifactReaderFailure. Successful-close cancellation is represented by ArtifactFileCancelled, which carries the completed physical outcome.

The failure mapper excludes phase: "decoder" from artifact cancellation handling, then falls back to a provider error. Since reply parsing never returns a capture, the later completed-capture retention cannot attach it. This explains the observed result; no repair was tested.

Additional context

Related closed report #5186 concerns cancellation during private-root release after a capture result has already been assembled, and cleanup retry attribution. Its fix is present in this revision. This reproduction reaches the earlier reply-file finalization boundary, before capture assembly.

Fetched upstream main at f801b2fbfd; the relevant decoder, failure mapper, and stable-reader source files are unchanged from the tested revision. Session-ledger behavior and other cancellation timings were not executed.

Originally created by @coygeek on GitHub (Oct 21, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5868 ### 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 Cancellation during the successful final close of a historical capture decoder's reply file returns `ProviderAdapterError`, with `phase: "decoder"` and `failure_kind: "cancelled"`, instead of `AnalysisCancelledError`. No partial capture observation is attached, even though the real HAR decoder has finished and the reply bytes have been fully read. I reproduced this at the built shared service used by `inspect-web-network-capture` and `inspect_web_network_capture`. Native file reading, descriptor closure, and the packaged HAR decoder subprocess were real. A controlled close hook injected cancellation immediately after the reply descriptor closed successfully. This did not exercise CLI signal delivery or an MCP cancellation notification. ### Reproduction In a prepared source checkout at the revision below, run the following focused Vitest test. The normal control succeeds. The cancellation case characterizes the incorrect error and confirms that its nested file-reader outcome contains the completed read while the service error has no partial observation. Both cases passed in two focused runs. To rerun, save the test as `tests/process-global/captureReplyCancellation.test.ts` and run `node node_modules/vitest/vitest.mjs run tests/process-global/captureReplyCancellation.test.ts --maxWorkers 1`. ```ts import { mkdtemp, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { expect, it, vi } from "vitest"; import { createWebNetworkCaptureService } from "../../dist/composition/webNetworkCaptures.js"; import { OwnedFileHandle } from "../../dist/filesystem/OwnedFileHandle.js"; import { historicalHar } from "../fixtures/historicalHar.js"; it.each([false, true])("real HAR reply read; cancel on reply close=%s", async (cancel) => { const root = await mkdtemp(join(tmpdir(), "rea-capture-reply-")); const capturePath = join(root, "capture.har"); await writeFile(capturePath, JSON.stringify(historicalHar())); const controller = new AbortController(); const reason = { request: "cancel during reply descriptor close" }; const nativeClose = OwnedFileHandle.prototype.close; let completedReplySeen = false; const close = vi.spyOn(OwnedFileHandle.prototype, "close").mockImplementation(async function (this: OwnedFileHandle) { const handle = this.handle; if (handle.fd >= 0 && "read" in handle) { const prefix = Buffer.alloc(11); const { bytesRead } = await handle.read(prefix, 0, prefix.length, 0); if (prefix.subarray(0, bytesRead).toString() === '{"ok":true,') { completedReplySeen = true; await nativeClose.call(this); if (cancel) controller.abort(reason); return; } } await nativeClose.call(this); }); const service = createWebNetworkCaptureService(process.env); try { const result = await service.inspect({ capture_path: capturePath, format: "har", record_ordinals: [0] }, { signal: controller.signal }); expect(completedReplySeen).toBe(true); if (!cancel) { expect(result.ok).toBe(true); console.log(JSON.stringify({ cancel, ok: result.ok, completedReplySeen })); } else { expect(result.ok).toBe(false); if (result.ok) throw new Error("Expected cancellation"); console.log(JSON.stringify({ cancel, tag: result.error._tag, failureKind: result.error.diagnostics.failure_kind, hasPartialObservation: result.error.partialObservation !== undefined, causeIsCancellationReason: result.error.cause === reason, physicalReadOutcome: result.error.cause.fileCancellation.outcome.kind, completedReplySeen })); expect(result.error._tag).toBe("ProviderAdapterError"); expect(result.error.diagnostics).toMatchObject({ phase: "decoder", failure_kind: "cancelled" }); expect(result.error.partialObservation).toBeUndefined(); expect(result.error.cause.fileCancellation.outcome.kind).toBe("completed"); expect(result.error.cause.fileCancellation.reason).toBe(reason); } } finally { close.mockRestore(); await service.close(); await rm(root, { recursive: true, force: true }); } }); ``` ### Expected behavior Return typed cancellation with the caller's exact reason and retain the completed, validated capture as Evidence under the requested record selection and sensitivity rules. The [historical-capture contract](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/web-network-captures.md#L58-L62) explicitly requires completed decoding to survive cancellation. The [lifecycle regression guidance](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/docs/testing.md#L245-L270) includes cancellation after workload exit during result reads. ### Impact and scope P2: a bounded cancellation window changes the error category and prevents completed capture Evidence from reaching the shared service result. The unmodified original capture can be inspected again; the control without cancellation succeeded. Natural occurrence frequency is unknown. HAR was executed; other historical formats share the source path but were not tested. No original-artifact loss or persistent resource leak was observed. ### REA version 52415975942a4b9c53a817915b313ccc404f280e; package metadata reports 6.7.0. ### Runtime and platform macOS 27.0.1 arm64, Node.js v24.22.0, Vitest 5.0.3. ### Target and artifact context The repository's authored HAR 1.2 fixture and the packaged HAR decoder. One selected producer record; no browser connection or recorded-URL fetching. ### Interface Other ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence Focused run: 1 test file passed, 2 tests passed. Relevant output, with paths omitted: ```json {"cancel":false,"ok":true,"completedReplySeen":true} {"cancel":true,"tag":"ProviderAdapterError","failureKind":"cancelled","hasPartialObservation":false,"causeIsCancellationReason":false,"physicalReadOutcome":"completed","completedReplySeen":true} ``` The test also checks that `result.error.cause.fileCancellation.reason` is the exact supplied reason object. The reader still carries it, together with the completed read; the top-level service result does not expose the expected cancellation type or partial capture. ### Typed outcome or recovery expected `AnalysisCancelledError`, with the supplied reason as its cause and the completed selected capture retained as partial Evidence. Earlier genuine decode or validation errors should retain their precedence. ### Analysis or suspected cause The [reply read and catch](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/browser/history/HistoricalCaptureDecoder.ts#L188-L213) propagate the request signal and immediately rethrow `ArtifactReaderFailure`. Successful-close cancellation is represented by [ArtifactFileCancelled](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/artifacts/ArtifactFileOperation.ts#L129-L142), which carries the completed physical outcome. The [failure mapper](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/browser/history/CaptureFailures.ts#L122-L129) excludes `phase: "decoder"` from artifact cancellation handling, then [falls back to a provider error](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/browser/history/CaptureFailures.ts#L179-L212). Since reply parsing never returns a capture, the [later completed-capture retention](https://github.com/morluto/rea/blob/52415975942a4b9c53a817915b313ccc404f280e/src/browser/history/HistoricalCaptureDecoder.ts#L353-L413) cannot attach it. This explains the observed result; no repair was tested. ### Additional context Related closed report #5186 concerns cancellation during private-root release after a capture result has already been assembled, and cleanup retry attribution. Its fix is present in this revision. This reproduction reaches the earlier reply-file finalization boundary, before capture assembly. Fetched upstream main at f801b2fbfd28f4e3713adefddc7bd73db44c284c; the relevant decoder, failure mapper, and stable-reader source files are unchanged from the tested revision. Session-ledger behavior and other cancellation timings were not executed.

bsakaguchi closed this issue

2026-10-31 00:41:50 +00:00

bsakaguchi commented

2026-10-31 00:41:51 +00:00

Author

Owner

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

Checked the reported fix on main at d87e73f2db.

HistoricalCaptureReply now recovers completed read bytes from an ArtifactReaderFailure, validates them, and returns the finalization failure separately. Decoder assembly retains the selected capture as partial observation, and cancellation mapping no longer excludes decoder phase. Tests inject cancellation at reply descriptor close with successful and failed close variants, assert selected records, exact cause identity and cleanup retirement.

Implementation: HistoricalCaptureReply.ts, HistoricalCaptureDecoder.ts.

The relevant regression checks in historicalCaptureDecoder.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. HistoricalCaptureReply now recovers completed read bytes from an ArtifactReaderFailure, validates them, and returns the finalization failure separately. Decoder assembly retains the selected capture as partial observation, and cancellation mapping no longer excludes decoder phase. Tests inject cancellation at reply descriptor close with successful and failed close variants, assert selected records, exact cause identity and cleanup retirement. Implementation: [HistoricalCaptureReply.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/browser/history/HistoricalCaptureReply.ts#L54), [HistoricalCaptureDecoder.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/browser/history/HistoricalCaptureDecoder.ts#L360). The relevant regression checks in [historicalCaptureDecoder.test.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/tests/boundary/browser/historicalCaptureDecoder.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:42 +00:00

[PR #3557] [CLOSED] fix(update): report optional native module readiness #5822

bsakaguchi referenced this issue

2026-11-03 13:39:53 +00:00

[PR #7216] fix(update): report Frida native module readiness #7722