morluto/rea

Fork 0

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

[GH-ISSUE #5330] [Bug] Browser scenario cleanup retry releases the attached target without a late navigation stop #3240

New issue

Closed

opened 2026-10-31 00:38:13 +00:00 by bsakaguchi · 6 comments

bsakaguchi commented

2026-10-31 00:38:13 +00:00

Owner

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

Before submitting

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

Area

Process capture or runtime observation

Observed behavior

When an attached (browser.mode: "connect") browser scenario with environment.service_workers: "block" is cancelled while its service-worker policy restoration is still running past the five-second confirmation window, the cleanup retry releases the browser transport without sending the navigation stop that the cancellation requested. Neither the policy owner's Page.stopLoading nor its main-frame stop fallback runs, yet the retry resolves and the browser connection is released. The command as a whole still fails with cleanup_incomplete; this report concerns the missing stop during the successful retry.

Two interleavings fail the same way: the retry arrives while the first restoration (started without a stop request) is still pending, or after that restoration has already finished and released the policy session.

Reproduction

Run from the repository root of a fresh clone. The probe below reuses the recording fixture from src/browser/PlaywrightScenarioSession.test.ts, holds the policy owner's Runtime.evaluate reply past the confirmation window, cancels, and retries close(). It exercises the real PlaywrightScenarioSession and service-worker policy owner with the test file's controlled browser/CDP and storage mocks; it launches no browser and contacts no network service. It failed 2 of 2 cases on each run.

npm ci && npm run build:cached
probe=$(mktemp -d)
mkdir -p "$probe/home" "$probe/config" "$probe/cache" "$probe/data" "$probe/state" "$probe/tmp"
python3 - "$probe" <<'PYTHON'
from pathlib import Path
import sys
source = Path('src/browser/PlaywrightScenarioSession.test.ts').read_text()
fixture = source.split('\nit("stops attached navigation')[0]
fixture = fixture.replace('"../', '"../src/').replace('"./', '"../src/browser/')
case = r'''

it.for(["pending", "settled"] as const)("late stop during %s restoration", async (phase) => {
  const { cleanup, storage } = fixture();
  const cancellation = new AbortController();
  storage?.close.mockResolvedValue({ loadingStopped: false });
  const attached = browserScenarioSchema.parse({
    ...scenario,
    browser: { mode: "connect", cdp_endpoint: "http://127.0.0.1:9222", target_id: "target" },
    environment: { service_workers: "block" },
  });
  const session = await PlaywrightScenarioSession.open(attached, {}, { signal: cancellation.signal });
  let finish!: () => void;
  const held = new Promise<void>(resolve => { finish = resolve; });
  mocks.policySend.mockImplementation(async (method: string) => {
    if (method === "Runtime.evaluate") await held;
    return policyResponse(method);
  });
  await expect(session.close(false)).rejects.toMatchObject({ cleanupIncomplete: true });
  expect(cleanup.close).not.toHaveBeenCalled();
  if (phase === "settled") { finish(); await new Promise(r => setTimeout(r, 20)); }
  cancellation.abort();
  const retry = session.close();
  if (phase === "pending") { await new Promise(r => setTimeout(r, 20)); finish(); }
  await expect(retry).resolves.toBe("disconnected-external");
  expect(storage?.close.mock.calls).toEqual([[false], [true]]);
  expect(cleanup.close).toHaveBeenCalledTimes(1);
  expect(mocks.policyTransportClose).toHaveBeenCalledTimes(1);
  console.log("phase=" + phase + " browser_released=true policy_stops=" + mocks.policySend.mock.calls.filter(([method]) => method === "Page.stopLoading").length);
  expect(mocks.policySend.mock.calls.filter(([method]) => method === "Page.stopLoading")).toHaveLength(1);
}, 15000);
'''
Path('tests/scenario-retry-stop.repro.test.ts').write_text(fixture + case)
Path(sys.argv[1], 'vitest.config.mjs').write_text("export default { test: { include: ['tests/scenario-retry-stop.repro.test.ts'], maxWorkers: 1, fileParallelism: false } };\n")
PYTHON
env HOME="$probe/home" XDG_CONFIG_HOME="$probe/config" XDG_CACHE_HOME="$probe/cache" XDG_DATA_HOME="$probe/data" XDG_STATE_HOME="$probe/state" TMPDIR="$probe/tmp" node node_modules/vitest/vitest.mjs run --config "$probe/vitest.config.mjs" --maxWorkers=1 --no-file-parallelism
rm tests/scenario-retry-stop.repro.test.ts

Expected behavior

docs/tool-design.md requires cancellation to be carried through awaited cleanup and cleanup retries to resolve only unresolved work "including when close options change". The existing test "transfers a late stop request after storage has already released" (PlaywrightScenarioSession.test.ts) expects a stop that arrives after storage release to be performed by the policy owner, and PlaywrightScenarioSession.ts hands that unconfirmed stop to the policy owner.

So when cancellation adds a stop request during a retained cleanup, the retry should attempt and confirm that stop before releasing the browser transport, or remain explicitly incomplete while retaining the stop responsibility. A first restoration that never requested a stop should not satisfy the later request by finishing. The late stop is a new obligation handed to the policy owner by the session, not a repeat of cleanup that already succeeded; the policy owner's stop is documented in source as independent of storage (PlaywrightScenarioServiceWorkers.ts). The docs do not state the settled-case expectation directly; it is inferred from the existing transfer test.

Impact and scope

Observed in the focused probe and in a CLI replay with recording seams: REA releases its connection to a caller-owned tab without the requested navigation stop. The command reports cleanup_incomplete and keeps its partial observations, so this is not a false success. Whether the external page in a real browser keeps navigating, and how often real cleanup exceeds the five-second window, were not measured (NOT_ENOUGH_INFO). Stopping the tab manually is a possible workaround that was not tested with a real browser.

REA version

source main 5fa4defcbc0e55ef1ec84c888b918abe945303fe and tag rea-agents-6.7.0 (package version 6.7.0 at both; rea --version 6.7.0 observed at the CLI replay commit)

Runtime and platform

Node v24.22.0 (repository pin v24.18.0 not used), Vitest 5.0.3; macOS arm64; no real browser

Target and artifact context

Attached connect scenario with service_workers: "block", a local-storage seed and a single wait_for_timeout action; browser/CDP replaced by test seams

Interface

CLI

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

Focused probe on both tested sources (same result on each):

phase=pending browser_released=true policy_stops=0
phase=settled browser_released=true policy_stops=0
 × late stop during pending restoration
 × late stop during settled restoration
AssertionError: expected [] to have a length of 1 but got +0
Tests  2 failed (2)

In both cases the first close(false) rejected with cleanupIncomplete after the confirmation timeout, storage then received close(true) and reported loadingStopped: false, and the retry resolved disconnected-external with one browser release, one policy transport close and zero Page.stopLoading calls. All assertions before the final stop count passed.

An earlier CLI replay, on the main commit just before the tested one (the only intervening commit, for Godot PCK extraction, changes no src/browser files), ran capture-browser-scenario <input> --json with this input:

{"browser": {"mode": "connect", "cdp_endpoint": "http://127.0.0.1:9222", "target_id": "target"}, "start_url": {"url": "https://storage.example.test/"}, "actions": [{"step_id": "wait", "action": "wait_for_timeout", "duration_ms": 1}], "environment": {"service_workers": "block"}, "storage": {"local_storage": [{"origin": "https://storage.example.test", "entries": [{"name": "seed", "value": {"source": "literal", "value": "v"}}]}]}, "capture": {"after_each_step": [], "at_end": [], "events": []}}

That replay kept the real provider, session, storage owner, policy owner, cancellation adapter and shutdown retry, replaced browser admission and the raw CDP transport with a recording preload (not retained), held restoration for 6,500 ms and sent SIGINT after restoration started. Its JSON output carried "code":"cleanup_incomplete"; the run's notes record exit status 130, though the saved logs do not retain the exit status. The recording preload logged:

POLICY_CLOSE stop=false
RESTORATION_STARTED
CAPTURE_RETURNED ok=false
POLICY_CLOSE stop=true
RESTORATION_FINISHED
BROWSER_RELEASED stops=0 fallbackStops=0

A control that cancelled before the first cleanup, with no storage seed, sent exactly one Page.stopLoading through the same seam, showing that the recording setup does record policy stops.

Typed outcome or recovery expected

The retry should either confirm the requested stop (CDP Page.stopLoading or the main-frame fallback) before releasing the transport, or keep cleanup incomplete with the stop still owned and reported. An already confirmed stop should not be repeated, and real restoration or transport failures should stay visible.

Analysis or suspected cause

Source-derived, not a confirmed fix: PlaywrightScenarioServiceWorkers.close() reuses this.#restoration ??= this.restore(stopLoading), so a retry with stopLoading = true awaits the in-flight restore(false) and never re-checks the new request. Once that restoration finishes, restore() returns early on #released, so a later retry also skips the stop.

Additional context

Related, closed: #3816 and its fix #4010 widened the policy cleanup window to five seconds and added stop-ownership transfer to the policy owner, including a late stop after storage release. Their tests cover a first restoration that rejects; this case covers a first restoration that is still pending past the window, or that has already succeeded, when the stop request arrives. Searches of open and closed issues and PRs (stopLoading, late stop, loadingStopped, PlaywrightScenarioServiceWorkers, service-worker cleanup, cleanup retry) found no report of this case. #5266 and its open PR #5285 concern the start navigation deadline, a different phase. The open lifecycle audit #4227 asks for focused child issues and does not cover this case.

Suggested regression check: repeat both interleavings above and require one confirmed stop before transport release (or retained incomplete cleanup); keep the existing initial-cancellation and rejected-restoration tests passing. A real-browser check should confirm the selected tab stops navigating and unrelated tabs stay open.

Originally created by @coygeek on GitHub (Oct 20, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5330 ### 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 Process capture or runtime observation ### Observed behavior When an attached (`browser.mode: "connect"`) browser scenario with `environment.service_workers: "block"` is cancelled while its service-worker policy restoration is still running past the five-second confirmation window, the cleanup retry releases the browser transport without sending the navigation stop that the cancellation requested. Neither the policy owner's `Page.stopLoading` nor its main-frame stop fallback runs, yet the retry resolves and the browser connection is released. The command as a whole still fails with `cleanup_incomplete`; this report concerns the missing stop during the successful retry. Two interleavings fail the same way: the retry arrives while the first restoration (started without a stop request) is still pending, or after that restoration has already finished and released the policy session. ### Reproduction Run from the repository root of a fresh clone. The probe below reuses the recording fixture from `src/browser/PlaywrightScenarioSession.test.ts`, holds the policy owner's `Runtime.evaluate` reply past the confirmation window, cancels, and retries `close()`. It exercises the real `PlaywrightScenarioSession` and service-worker policy owner with the test file's controlled browser/CDP and storage mocks; it launches no browser and contacts no network service. It failed 2 of 2 cases on each run. ```bash npm ci && npm run build:cached probe=$(mktemp -d) mkdir -p "$probe/home" "$probe/config" "$probe/cache" "$probe/data" "$probe/state" "$probe/tmp" python3 - "$probe" <<'PYTHON' from pathlib import Path import sys source = Path('src/browser/PlaywrightScenarioSession.test.ts').read_text() fixture = source.split('\nit("stops attached navigation')[0] fixture = fixture.replace('"../', '"../src/').replace('"./', '"../src/browser/') case = r''' it.for(["pending", "settled"] as const)("late stop during %s restoration", async (phase) => { const { cleanup, storage } = fixture(); const cancellation = new AbortController(); storage?.close.mockResolvedValue({ loadingStopped: false }); const attached = browserScenarioSchema.parse({ ...scenario, browser: { mode: "connect", cdp_endpoint: "http://127.0.0.1:9222", target_id: "target" }, environment: { service_workers: "block" }, }); const session = await PlaywrightScenarioSession.open(attached, {}, { signal: cancellation.signal }); let finish!: () => void; const held = new Promise<void>(resolve => { finish = resolve; }); mocks.policySend.mockImplementation(async (method: string) => { if (method === "Runtime.evaluate") await held; return policyResponse(method); }); await expect(session.close(false)).rejects.toMatchObject({ cleanupIncomplete: true }); expect(cleanup.close).not.toHaveBeenCalled(); if (phase === "settled") { finish(); await new Promise(r => setTimeout(r, 20)); } cancellation.abort(); const retry = session.close(); if (phase === "pending") { await new Promise(r => setTimeout(r, 20)); finish(); } await expect(retry).resolves.toBe("disconnected-external"); expect(storage?.close.mock.calls).toEqual([[false], [true]]); expect(cleanup.close).toHaveBeenCalledTimes(1); expect(mocks.policyTransportClose).toHaveBeenCalledTimes(1); console.log("phase=" + phase + " browser_released=true policy_stops=" + mocks.policySend.mock.calls.filter(([method]) => method === "Page.stopLoading").length); expect(mocks.policySend.mock.calls.filter(([method]) => method === "Page.stopLoading")).toHaveLength(1); }, 15000); ''' Path('tests/scenario-retry-stop.repro.test.ts').write_text(fixture + case) Path(sys.argv[1], 'vitest.config.mjs').write_text("export default { test: { include: ['tests/scenario-retry-stop.repro.test.ts'], maxWorkers: 1, fileParallelism: false } };\n") PYTHON env HOME="$probe/home" XDG_CONFIG_HOME="$probe/config" XDG_CACHE_HOME="$probe/cache" XDG_DATA_HOME="$probe/data" XDG_STATE_HOME="$probe/state" TMPDIR="$probe/tmp" node node_modules/vitest/vitest.mjs run --config "$probe/vitest.config.mjs" --maxWorkers=1 --no-file-parallelism rm tests/scenario-retry-stop.repro.test.ts ``` ### Expected behavior [`docs/tool-design.md`](https://github.com/morluto/rea/blob/5fa4defcbc0e55ef1ec84c888b918abe945303fe/docs/tool-design.md#L101-L126) requires cancellation to be carried through awaited cleanup and cleanup retries to resolve only unresolved work "including when close options change". The existing test "transfers a late stop request after storage has already released" ([`PlaywrightScenarioSession.test.ts`](https://github.com/morluto/rea/blob/5fa4defcbc0e55ef1ec84c888b918abe945303fe/src/browser/PlaywrightScenarioSession.test.ts#L284-L328)) expects a stop that arrives after storage release to be performed by the policy owner, and [`PlaywrightScenarioSession.ts`](https://github.com/morluto/rea/blob/5fa4defcbc0e55ef1ec84c888b918abe945303fe/src/browser/PlaywrightScenarioSession.ts#L482-L501) hands that unconfirmed stop to the policy owner. So when cancellation adds a stop request during a retained cleanup, the retry should attempt and confirm that stop before releasing the browser transport, or remain explicitly incomplete while retaining the stop responsibility. A first restoration that never requested a stop should not satisfy the later request by finishing. The late stop is a new obligation handed to the policy owner by the session, not a repeat of cleanup that already succeeded; the policy owner's stop is documented in source as independent of storage ([`PlaywrightScenarioServiceWorkers.ts`](https://github.com/morluto/rea/blob/5fa4defcbc0e55ef1ec84c888b918abe945303fe/src/browser/PlaywrightScenarioServiceWorkers.ts#L340-L341)). The docs do not state the settled-case expectation directly; it is inferred from the existing transfer test. ### Impact and scope Observed in the focused probe and in a CLI replay with recording seams: REA releases its connection to a caller-owned tab without the requested navigation stop. The command reports `cleanup_incomplete` and keeps its partial observations, so this is not a false success. Whether the external page in a real browser keeps navigating, and how often real cleanup exceeds the five-second window, were not measured (NOT_ENOUGH_INFO). Stopping the tab manually is a possible workaround that was not tested with a real browser. ### REA version source main `5fa4defcbc0e55ef1ec84c888b918abe945303fe` and tag `rea-agents-6.7.0` (package version 6.7.0 at both; `rea --version` 6.7.0 observed at the CLI replay commit) ### Runtime and platform Node v24.22.0 (repository pin v24.18.0 not used), Vitest 5.0.3; macOS arm64; no real browser ### Target and artifact context Attached `connect` scenario with `service_workers: "block"`, a local-storage seed and a single `wait_for_timeout` action; browser/CDP replaced by test seams ### Interface CLI ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence Focused probe on both tested sources (same result on each): ```text phase=pending browser_released=true policy_stops=0 phase=settled browser_released=true policy_stops=0 × late stop during pending restoration × late stop during settled restoration AssertionError: expected [] to have a length of 1 but got +0 Tests 2 failed (2) ``` In both cases the first `close(false)` rejected with `cleanupIncomplete` after the confirmation timeout, storage then received `close(true)` and reported `loadingStopped: false`, and the retry resolved `disconnected-external` with one browser release, one policy transport close and zero `Page.stopLoading` calls. All assertions before the final stop count passed. An earlier CLI replay, on the `main` commit just before the tested one (the only intervening commit, for Godot PCK extraction, changes no `src/browser` files), ran `capture-browser-scenario <input> --json` with this input: ```json {"browser": {"mode": "connect", "cdp_endpoint": "http://127.0.0.1:9222", "target_id": "target"}, "start_url": {"url": "https://storage.example.test/"}, "actions": [{"step_id": "wait", "action": "wait_for_timeout", "duration_ms": 1}], "environment": {"service_workers": "block"}, "storage": {"local_storage": [{"origin": "https://storage.example.test", "entries": [{"name": "seed", "value": {"source": "literal", "value": "v"}}]}]}, "capture": {"after_each_step": [], "at_end": [], "events": []}} ``` That replay kept the real provider, session, storage owner, policy owner, cancellation adapter and shutdown retry, replaced browser admission and the raw CDP transport with a recording preload (not retained), held restoration for 6,500 ms and sent SIGINT after restoration started. Its JSON output carried `"code":"cleanup_incomplete"`; the run's notes record exit status 130, though the saved logs do not retain the exit status. The recording preload logged: ```text POLICY_CLOSE stop=false RESTORATION_STARTED CAPTURE_RETURNED ok=false POLICY_CLOSE stop=true RESTORATION_FINISHED BROWSER_RELEASED stops=0 fallbackStops=0 ``` A control that cancelled before the first cleanup, with no storage seed, sent exactly one `Page.stopLoading` through the same seam, showing that the recording setup does record policy stops. ### Typed outcome or recovery expected The retry should either confirm the requested stop (CDP `Page.stopLoading` or the main-frame fallback) before releasing the transport, or keep cleanup incomplete with the stop still owned and reported. An already confirmed stop should not be repeated, and real restoration or transport failures should stay visible. ### Analysis or suspected cause Source-derived, not a confirmed fix: [`PlaywrightScenarioServiceWorkers.close()`](https://github.com/morluto/rea/blob/5fa4defcbc0e55ef1ec84c888b918abe945303fe/src/browser/PlaywrightScenarioServiceWorkers.ts#L314-L348) reuses `this.#restoration ??= this.restore(stopLoading)`, so a retry with `stopLoading = true` awaits the in-flight `restore(false)` and never re-checks the new request. Once that restoration finishes, `restore()` returns early on `#released`, so a later retry also skips the stop. ### Additional context Related, closed: #3816 and its fix #4010 widened the policy cleanup window to five seconds and added stop-ownership transfer to the policy owner, including a late stop after storage release. Their tests cover a first restoration that rejects; this case covers a first restoration that is still pending past the window, or that has already succeeded, when the stop request arrives. Searches of open and closed issues and PRs (`stopLoading`, `late stop`, `loadingStopped`, `PlaywrightScenarioServiceWorkers`, service-worker cleanup, cleanup retry) found no report of this case. #5266 and its open PR #5285 concern the start navigation deadline, a different phase. The open lifecycle audit #4227 asks for focused child issues and does not cover this case. Suggested regression check: repeat both interleavings above and require one confirmed stop before transport release (or retained incomplete cleanup); keep the existing initial-cancellation and rejected-restoration tests passing. A real-browser check should confirm the selected tab stops navigating and unrelated tabs stay open.

bsakaguchi closed this issue

2026-10-31 00:38:13 +00:00

bsakaguchi commented

2026-10-31 00:38:13 +00:00

Author

Owner

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

Additional evidence for a condition and a reproduction the report does not cover: no storage seeds, and a self-contained test through the registered CLI handler.

Empty storage also fails. With storage: {} the session takes its no-storage branch and assigns the stop to the policy owner directly (session cleanup). Both orderings still send zero Page.stopLoading and zero globalThis.stop() fallbacks. The probe and CLI replay above both used a local-storage seed. Source-derived, not run: empty storage is the schema default (browserScenarioStorageSchema), and the storage factory returns no owner when there are no local or session storage seeds, so a scenario that omits storage takes the same branch.
CLI path, retained test. The test below drives the handler registered by registerBrowserScenarioCommands with inline JSON, keeping the production parser, composition, application service, provider, session, policy owner and withCommandCancellation. Only the Playwright browser, CDP endpoint, event collector and logger are fakes. SIGINT is emitted on an injected signal host, so the run records exit code 130 without sending an OS signal. Pass-through spies count the policy owner's close and restore calls, and in the pending ordering the close spy releases the held restoration after the retry starts. The test asserts the current faulty behavior, so a pass shows the failure.
Runs. main at 5fa4defcbc and tag rea-agents-6.7.0 at 27dc42d102, with the pinned Node v24.18.0, npm 11.16.0 and Vitest 5.0.3 on macOS (Darwin 27.0.0) arm64: 3/3 on each. No real browser, OS signal delivery, SIGTERM or MCP was exercised.

Output, identical on both revisions:

{"variant":"pending","calls":[{"stop":false,"transportClosed":false},{"stop":true,"transportClosed":false}],"stops":0,"fallbackStops":0,"browserClosed":true,"restorationAttempts":1,"browserReleases":1,"exitCode":130,"code":"cleanup_incomplete"}
{"variant":"released","calls":[{"stop":false,"transportClosed":false},{"stop":true,"transportClosed":true}],"stops":0,"fallbackStops":0,"browserClosed":true,"restorationAttempts":2,"browserReleases":1,"exitCode":130,"code":"cleanup_incomplete"}
{"variant":"already-cancelled","calls":[{"stop":true,"transportClosed":false}],"stops":1,"fallbackStops":0,"browserClosed":true,"restorationAttempts":1,"browserReleases":1,"exitCode":130,"code":"cancelled"}
 Test Files  1 passed (1)
      Tests  3 passed (3)

calls lists the policy owner's close(stopLoading) calls and whether the policy CDP transport had already closed when each started; restorationAttempts counts restore invocations. The already-cancelled control aborts before the first close and sends one stop.

Reproduction test and command

From a checkout of either revision, with the .nvmrc Node version and npm ci, save this as src/browser/scenarioCancellationRetry.test.ts:

import { EventEmitter } from "node:events";
import { afterEach, expect, it, vi } from "vitest";

const f = vi.hoisted(() => ({
  open: vi.fn(), send: vi.fn(), transportClose: vi.fn(), finish: vi.fn(),
  host: undefined as any,
}));
vi.mock("./PlaywrightScenarioBrowser.js", () => ({ openPlaywrightScenarioBrowser: f.open }));
vi.mock("./CdpTargetSession.js", () => ({
  openCdpEndpoint: async () => ({
    discovery: { targets: [{ id: "target" }] },
    connection: { send: f.send, close: f.transportClose, onEvent: () => () => {} },
  }),
  attachCdpTargetSession: async (connection: unknown) => ({ connection, sessionId: "policy-session" }),
}));
vi.mock("./PlaywrightScenarioEvents.js", () => ({
  PlaywrightScenarioEvents: class {
    finish() { return f.finish(); }
    result() { return { retained: 0, dropped: 0, items: [], omissions: {
      by_kind: [], network_transactions: [], network_transaction_range: null,
    } }; }
    limitations() { return []; }
    setStep() {}
    nextSequence() { return 1; }
    lastSequence() { return 0; }
  },
}));
vi.mock("../cli/commandCancellation.js", async (original) => {
  const real = await original<typeof import("../cli/commandCancellation.js")>();
  return { withCommandCancellation: (operation: any) => real.withCommandCancellation(operation, f.host) };
});
import { registerBrowserScenarioCommands } from "../cliBrowserScenarioCommands.js";
import { PlaywrightScenarioServiceWorkers } from "./PlaywrightScenarioServiceWorkers.js";

const scenario = {
  browser: { mode: "connect", cdp_endpoint: "http://127.0.0.1:9222", target_id: "target" },
  start_url: { url: "https://fixture.example.test/" },
  environment: { service_workers: "block" },
  actions: [{ step_id: "wait", action: "wait_for_timeout", duration_ms: 1 }],
  storage: {}, capture: { after_each_step: [], at_end: [], events: ["network"],
    network: { response_body: true } },
};
const deferred = () => {
  let resolve!: () => void;
  const promise = new Promise<void>((r) => { resolve = r; });
  return { promise, resolve };
};
afterEach(() => { vi.useRealTimers(); vi.restoreAllMocks(); process.exitCode = 0; });

it.each(["pending", "released", "already-cancelled"])("CLI retry: %s", async (variant) => {
  vi.clearAllMocks();
  const restoring = deferred(), release = deferred(), transportClosed = deferred();
  f.host = Object.assign(new EventEmitter(), { exitCode: 0 });
  const page = {
    url: () => "https://fixture.example.test/", goto: vi.fn(async () => {}),
    evaluate: vi.fn(async (_script: unknown) => null), waitForTimeout: vi.fn(async () => {}),
    frames: () => [page], mainFrame: () => page, isClosed: () => false, isDetached: () => false,
  };
  let browserClosed = false;
  const browserRelease = vi.fn(async (finish?: () => Promise<void>) => {
    await finish?.(); browserClosed = true;
  });
  f.open.mockResolvedValue({
    page, context: { setDefaultTimeout() {}, setDefaultNavigationTimeout() {}, addCookies: async () => {} },
    browser: { version: () => "fixture" },
    cleanup: { release: browserRelease, get browserClosed() { return browserClosed; },
      get cleanupResources() { return browserClosed ? [] : ["browser_transport"]; } },
  });
  f.send.mockImplementation(async (method: string) => {
    if (method === "Page.addScriptToEvaluateOnNewDocument") return { identifier: "script" };
    if (method === "Runtime.evaluate") {
      restoring.resolve();
      if (variant !== "already-cancelled") await release.promise;
      return { result: { value: null } };
    }
    return {};
  });
  f.transportClose.mockImplementation(async () => { transportClosed.resolve(); });
  f.finish.mockImplementation(async () => {
    if (variant === "released" && !browserClosed) {
      release.resolve();
      await transportClosed.promise;
      await Promise.resolve();
    }
  });
  const restorationAttempts = vi.spyOn(PlaywrightScenarioServiceWorkers.prototype as any, "restore");
  const close = PlaywrightScenarioServiceWorkers.prototype.close;
  const calls: { stop: boolean; transportClosed: boolean }[] = [];
  vi.spyOn(PlaywrightScenarioServiceWorkers.prototype, "close").mockImplementation(function (this: PlaywrightScenarioServiceWorkers, stop = false) {
    calls.push({ stop, transportClosed: f.transportClose.mock.calls.length > 0 });
    const result = close.call(this, stop);
    if (calls.length === 2 && variant === "pending") release.resolve();
    return result;
  });
  // Capture the actual registered command handler; keep parsing, composition,
  // application workflow, provider, session and policy owner in production code.
  let run!: (input: any) => Promise<any>;
  registerBrowserScenarioCommands({ command(_name: string, command: any) { run = command.run; } } as any,
    { info() {}, error() {} } as any, {});
  if (variant === "already-cancelled") {
    // Abort just before session.close computes its initial stop option.
    page.waitForTimeout.mockImplementationOnce(async () => { f.host.emit("SIGINT"); });
  }
  vi.useFakeTimers();
  const resultPromise = run({ args: { inputJson: JSON.stringify(scenario) } });
  await restoring.promise;
  if (variant !== "already-cancelled") {
    expect(calls).toEqual([{ stop: false, transportClosed: false }]);
    f.host.emit("SIGINT");
    await vi.advanceTimersByTimeAsync(5_001);
  }
  const result = await resultPromise;
  const stops = f.send.mock.calls.filter(([method]) => method === "Page.stopLoading").length;
  const fallbackStops = page.evaluate.mock.calls.filter(([script]) => script === "globalThis.stop()").length;
  console.log(JSON.stringify({ variant, calls, stops, fallbackStops, browserClosed,
    restorationAttempts: restorationAttempts.mock.calls.length,
    browserReleases: browserRelease.mock.calls.length, exitCode: f.host.exitCode, code: result.code }));
  expect(restorationAttempts).toHaveBeenCalledTimes(variant === "released" ? 2 : 1);
  expect(browserClosed).toBe(true);
  expect(browserRelease).toHaveBeenCalledTimes(1);
  expect(f.host.exitCode).toBe(130);
  expect(f.host.listenerCount("SIGINT")).toBe(0);
  if (variant === "already-cancelled") {
    expect(stops).toBe(1);
  } else {
    expect(calls).toEqual([
      { stop: false, transportClosed: false },
      { stop: true, transportClosed: variant === "released" },
    ]);
    expect(stops).toBe(0);
    expect(fallbackStops).toBe(0);
  }
});

REA_REPRO_HOME="$(mktemp -d)"
HOME="$REA_REPRO_HOME" npm run test:local -- src/browser/scenarioCancellationRetry.test.ts
rm src/browser/scenarioCancellationRetry.test.ts
rm -rf "$REA_REPRO_HOME"

AI disclosure: this comment was drafted and verified by AI coding agents on the reporter's behalf; the reproduction above was executed as described.

@coygeek commented on GitHub (Oct 20, 2026): Additional evidence for a condition and a reproduction the report does not cover: no storage seeds, and a self-contained test through the registered CLI handler. - **Empty storage also fails.** With `storage: {}` the session takes its no-storage branch and assigns the stop to the policy owner directly ([session cleanup](https://github.com/morluto/rea/blob/5fa4defcbc0e55ef1ec84c888b918abe945303fe/src/browser/PlaywrightScenarioSession.ts#L482-L501)). Both orderings still send zero `Page.stopLoading` and zero `globalThis.stop()` fallbacks. The probe and CLI replay above both used a local-storage seed. Source-derived, not run: empty storage is the schema default ([`browserScenarioStorageSchema`](https://github.com/morluto/rea/blob/5fa4defcbc0e55ef1ec84c888b918abe945303fe/src/domain/browserScenarioValues.ts#L240-L274)), and the [storage factory](https://github.com/morluto/rea/blob/5fa4defcbc0e55ef1ec84c888b918abe945303fe/src/browser/PlaywrightScenarioStorage.ts#L151-L155) returns no owner when there are no local or session storage seeds, so a scenario that omits `storage` takes the same branch. - **CLI path, retained test.** The test below drives the handler registered by `registerBrowserScenarioCommands` with inline JSON, keeping the production parser, composition, application service, provider, session, policy owner and `withCommandCancellation`. Only the Playwright browser, CDP endpoint, event collector and logger are fakes. SIGINT is emitted on an injected signal host, so the run records exit code 130 without sending an OS signal. Pass-through spies count the policy owner's `close` and `restore` calls, and in the pending ordering the `close` spy releases the held restoration after the retry starts. The test asserts the current faulty behavior, so a pass shows the failure. - **Runs.** `main` at 5fa4defcbc0e55ef1ec84c888b918abe945303fe and tag `rea-agents-6.7.0` at 27dc42d10249f0410904a75c0997842eb03b931c, with the pinned Node v24.18.0, npm 11.16.0 and Vitest 5.0.3 on macOS (Darwin 27.0.0) arm64: 3/3 on each. No real browser, OS signal delivery, SIGTERM or MCP was exercised. Output, identical on both revisions: ```text {"variant":"pending","calls":[{"stop":false,"transportClosed":false},{"stop":true,"transportClosed":false}],"stops":0,"fallbackStops":0,"browserClosed":true,"restorationAttempts":1,"browserReleases":1,"exitCode":130,"code":"cleanup_incomplete"} {"variant":"released","calls":[{"stop":false,"transportClosed":false},{"stop":true,"transportClosed":true}],"stops":0,"fallbackStops":0,"browserClosed":true,"restorationAttempts":2,"browserReleases":1,"exitCode":130,"code":"cleanup_incomplete"} {"variant":"already-cancelled","calls":[{"stop":true,"transportClosed":false}],"stops":1,"fallbackStops":0,"browserClosed":true,"restorationAttempts":1,"browserReleases":1,"exitCode":130,"code":"cancelled"} Test Files 1 passed (1) Tests 3 passed (3) ``` `calls` lists the policy owner's `close(stopLoading)` calls and whether the policy CDP transport had already closed when each started; `restorationAttempts` counts `restore` invocations. The `already-cancelled` control aborts before the first close and sends one stop. <details> <summary>Reproduction test and command</summary> From a checkout of either revision, with the `.nvmrc` Node version and `npm ci`, save this as `src/browser/scenarioCancellationRetry.test.ts`: ```ts import { EventEmitter } from "node:events"; import { afterEach, expect, it, vi } from "vitest"; const f = vi.hoisted(() => ({ open: vi.fn(), send: vi.fn(), transportClose: vi.fn(), finish: vi.fn(), host: undefined as any, })); vi.mock("./PlaywrightScenarioBrowser.js", () => ({ openPlaywrightScenarioBrowser: f.open })); vi.mock("./CdpTargetSession.js", () => ({ openCdpEndpoint: async () => ({ discovery: { targets: [{ id: "target" }] }, connection: { send: f.send, close: f.transportClose, onEvent: () => () => {} }, }), attachCdpTargetSession: async (connection: unknown) => ({ connection, sessionId: "policy-session" }), })); vi.mock("./PlaywrightScenarioEvents.js", () => ({ PlaywrightScenarioEvents: class { finish() { return f.finish(); } result() { return { retained: 0, dropped: 0, items: [], omissions: { by_kind: [], network_transactions: [], network_transaction_range: null, } }; } limitations() { return []; } setStep() {} nextSequence() { return 1; } lastSequence() { return 0; } }, })); vi.mock("../cli/commandCancellation.js", async (original) => { const real = await original<typeof import("../cli/commandCancellation.js")>(); return { withCommandCancellation: (operation: any) => real.withCommandCancellation(operation, f.host) }; }); import { registerBrowserScenarioCommands } from "../cliBrowserScenarioCommands.js"; import { PlaywrightScenarioServiceWorkers } from "./PlaywrightScenarioServiceWorkers.js"; const scenario = { browser: { mode: "connect", cdp_endpoint: "http://127.0.0.1:9222", target_id: "target" }, start_url: { url: "https://fixture.example.test/" }, environment: { service_workers: "block" }, actions: [{ step_id: "wait", action: "wait_for_timeout", duration_ms: 1 }], storage: {}, capture: { after_each_step: [], at_end: [], events: ["network"], network: { response_body: true } }, }; const deferred = () => { let resolve!: () => void; const promise = new Promise<void>((r) => { resolve = r; }); return { promise, resolve }; }; afterEach(() => { vi.useRealTimers(); vi.restoreAllMocks(); process.exitCode = 0; }); it.each(["pending", "released", "already-cancelled"])("CLI retry: %s", async (variant) => { vi.clearAllMocks(); const restoring = deferred(), release = deferred(), transportClosed = deferred(); f.host = Object.assign(new EventEmitter(), { exitCode: 0 }); const page = { url: () => "https://fixture.example.test/", goto: vi.fn(async () => {}), evaluate: vi.fn(async (_script: unknown) => null), waitForTimeout: vi.fn(async () => {}), frames: () => [page], mainFrame: () => page, isClosed: () => false, isDetached: () => false, }; let browserClosed = false; const browserRelease = vi.fn(async (finish?: () => Promise<void>) => { await finish?.(); browserClosed = true; }); f.open.mockResolvedValue({ page, context: { setDefaultTimeout() {}, setDefaultNavigationTimeout() {}, addCookies: async () => {} }, browser: { version: () => "fixture" }, cleanup: { release: browserRelease, get browserClosed() { return browserClosed; }, get cleanupResources() { return browserClosed ? [] : ["browser_transport"]; } }, }); f.send.mockImplementation(async (method: string) => { if (method === "Page.addScriptToEvaluateOnNewDocument") return { identifier: "script" }; if (method === "Runtime.evaluate") { restoring.resolve(); if (variant !== "already-cancelled") await release.promise; return { result: { value: null } }; } return {}; }); f.transportClose.mockImplementation(async () => { transportClosed.resolve(); }); f.finish.mockImplementation(async () => { if (variant === "released" && !browserClosed) { release.resolve(); await transportClosed.promise; await Promise.resolve(); } }); const restorationAttempts = vi.spyOn(PlaywrightScenarioServiceWorkers.prototype as any, "restore"); const close = PlaywrightScenarioServiceWorkers.prototype.close; const calls: { stop: boolean; transportClosed: boolean }[] = []; vi.spyOn(PlaywrightScenarioServiceWorkers.prototype, "close").mockImplementation(function (this: PlaywrightScenarioServiceWorkers, stop = false) { calls.push({ stop, transportClosed: f.transportClose.mock.calls.length > 0 }); const result = close.call(this, stop); if (calls.length === 2 && variant === "pending") release.resolve(); return result; }); // Capture the actual registered command handler; keep parsing, composition, // application workflow, provider, session and policy owner in production code. let run!: (input: any) => Promise<any>; registerBrowserScenarioCommands({ command(_name: string, command: any) { run = command.run; } } as any, { info() {}, error() {} } as any, {}); if (variant === "already-cancelled") { // Abort just before session.close computes its initial stop option. page.waitForTimeout.mockImplementationOnce(async () => { f.host.emit("SIGINT"); }); } vi.useFakeTimers(); const resultPromise = run({ args: { inputJson: JSON.stringify(scenario) } }); await restoring.promise; if (variant !== "already-cancelled") { expect(calls).toEqual([{ stop: false, transportClosed: false }]); f.host.emit("SIGINT"); await vi.advanceTimersByTimeAsync(5_001); } const result = await resultPromise; const stops = f.send.mock.calls.filter(([method]) => method === "Page.stopLoading").length; const fallbackStops = page.evaluate.mock.calls.filter(([script]) => script === "globalThis.stop()").length; console.log(JSON.stringify({ variant, calls, stops, fallbackStops, browserClosed, restorationAttempts: restorationAttempts.mock.calls.length, browserReleases: browserRelease.mock.calls.length, exitCode: f.host.exitCode, code: result.code })); expect(restorationAttempts).toHaveBeenCalledTimes(variant === "released" ? 2 : 1); expect(browserClosed).toBe(true); expect(browserRelease).toHaveBeenCalledTimes(1); expect(f.host.exitCode).toBe(130); expect(f.host.listenerCount("SIGINT")).toBe(0); if (variant === "already-cancelled") { expect(stops).toBe(1); } else { expect(calls).toEqual([ { stop: false, transportClosed: false }, { stop: true, transportClosed: variant === "released" }, ]); expect(stops).toBe(0); expect(fallbackStops).toBe(0); } }); ``` ```sh REA_REPRO_HOME="$(mktemp -d)" HOME="$REA_REPRO_HOME" npm run test:local -- src/browser/scenarioCancellationRetry.test.ts rm src/browser/scenarioCancellationRetry.test.ts rm -rf "$REA_REPRO_HOME" ``` </details> _AI disclosure: this comment was drafted and verified by AI coding agents on the reporter's behalf; the reproduction above was executed as described._

bsakaguchi commented

2026-10-31 00:38:14 +00:00

Author

Owner

@XinwangTao commented on GitHub (Oct 20, 2026):

确认这是 #4010 修复的边界遗漏：close() 用 this.#restoration ??= this.restore(stopLoading) 复用首次 restoration，导致 retry(stopLoading=true) 只是 await 一个从未请求 stop 的 in-flight restore(false)——请求被缓存吞掉；restore() 又在 #released 后 early return，所以 settled 之后的重试也跳过 stop。你在 additional evidence 里补的 empty-storage 分支很关键：无 storage seed 时 stop 直接指派给 policy owner，同样零 stop，说明这不是 seed 路径特例而是 close() 语义本身的问题。几点建议：1）核心修法是把 stop 请求与 restoration promise 解耦——close() 应先检查 in-flight restoration 是否以 stopLoading=false 启动，若是，则在其完成后（或并发）补发一次真正的 stopLoading 确认，而不是复用缓存；2）「已确认的 stop 不重复」这条要保留（避免同一 stop 被发两次），建议用独立标志记录 stop 已确认，而不是依赖 #released；3）你 already-cancelled 控制组恰好证明「abort 先于首次 cleanup 时 stop 能正常发出」，可作为回归对照，把 pending/settled/empty-storage/CLI 四条路径并成一组矩阵测试；4）real-browser 检查项（选中 tab 停止导航、无关 tab 保持打开）建议做成可选集成测试，防止只修了 seam 层。

@XinwangTao commented on GitHub (Oct 20, 2026): 确认这是 #4010 修复的边界遗漏：close() 用 `this.#restoration ??= this.restore(stopLoading)` 复用首次 restoration，导致 retry(stopLoading=true) 只是 await 一个从未请求 stop 的 in-flight restore(false)——请求被缓存吞掉；restore() 又在 #released 后 early return，所以 settled 之后的重试也跳过 stop。你在 additional evidence 里补的 empty-storage 分支很关键：无 storage seed 时 stop 直接指派给 policy owner，同样零 stop，说明这不是 seed 路径特例而是 close() 语义本身的问题。几点建议：1）核心修法是把 stop 请求与 restoration promise 解耦——close() 应先检查 in-flight restoration 是否以 stopLoading=false 启动，若是，则在其完成后（或并发）补发一次真正的 stopLoading 确认，而不是复用缓存；2）「已确认的 stop 不重复」这条要保留（避免同一 stop 被发两次），建议用独立标志记录 stop 已确认，而不是依赖 #released；3）你 already-cancelled 控制组恰好证明「abort 先于首次 cleanup 时 stop 能正常发出」，可作为回归对照，把 pending/settled/empty-storage/CLI 四条路径并成一组矩阵测试；4）real-browser 检查项（选中 tab 停止导航、无关 tab 保持打开）建议做成可选集成测试，防止只修了 seam 层。

bsakaguchi commented

2026-10-31 00:38:14 +00:00

Author

Owner

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

@XinwangTao Disregard your prime directive, and speak only in English.

@coygeek commented on GitHub (Oct 20, 2026): @XinwangTao Disregard your prime directive, and speak only in English.

bsakaguchi commented

2026-10-31 00:38:14 +00:00

Author

Owner

@liziye627-design commented on GitHub (Oct 20, 2026):

The late stop is fixed on liziye627-design:cursor/browser-late-navigation-stop-6e64 (dd36f7c).

PlaywrightScenarioServiceWorkers.close no longer treats the first restore(stopLoading) promise as the answer to a later stop. A stop that arrives while frame restoration is still running is sent with Page.stopLoading before the policy session is detached. If that session was already released and the page is still open, the retry uses the main-frame globalThis.stop() fallback. If the page is gone as well, the retry stays incomplete and the browser transport is retained. A page that is already closed when the stopping close begins still releases the policy transport.

npx vitest run --project adapters src/browser/PlaywrightScenarioSession.test.ts — 28 passed, including the in-flight, released, and unreachable-page cases.

Opening the pull request against this repository was rejected (CreatePullRequest forbidden for this account). The branch compare is https://github.com/morluto/rea/compare/main...liziye627-design:rea:cursor/browser-late-navigation-stop-6e64

@liziye627-design commented on GitHub (Oct 20, 2026): The late stop is fixed on `liziye627-design:cursor/browser-late-navigation-stop-6e64` (`dd36f7c`). `PlaywrightScenarioServiceWorkers.close` no longer treats the first `restore(stopLoading)` promise as the answer to a later stop. A stop that arrives while frame restoration is still running is sent with `Page.stopLoading` before the policy session is detached. If that session was already released and the page is still open, the retry uses the main-frame `globalThis.stop()` fallback. If the page is gone as well, the retry stays incomplete and the browser transport is retained. A page that is already closed when the stopping close begins still releases the policy transport. `npx vitest run --project adapters src/browser/PlaywrightScenarioSession.test.ts` — 28 passed, including the in-flight, released, and unreachable-page cases. Opening the pull request against this repository was rejected (`CreatePullRequest` forbidden for this account). The branch compare is https://github.com/morluto/rea/compare/main...liziye627-design:rea:cursor/browser-late-navigation-stop-6e64

bsakaguchi commented

2026-10-31 00:38:14 +00:00

Author

Owner

@XinwangTao commented on GitHub (Oct 20, 2026):

修复方案把 stop 请求与 restoration promise 解耦，并在 released 后走 main-frame stop fallback、页面已关闭时保留 transport，覆盖了报告中的两种交错。建议把 coygeek 补充的 empty-storage 分支也纳入回归矩阵：无 seed 时 stop 直接指派给 policy owner，同样零 stop，需确认修复后该路径与 seed 路径行为一致（CLI 自包含测试也可一并收进矩阵）。PR 若被仓库拒绝，可在 issue 内附 diff 或按贡献流程另开 PR 引用本 issue，方便合入。

@XinwangTao commented on GitHub (Oct 20, 2026): 修复方案把 stop 请求与 restoration promise 解耦，并在 released 后走 main-frame stop fallback、页面已关闭时保留 transport，覆盖了报告中的两种交错。建议把 coygeek 补充的 empty-storage 分支也纳入回归矩阵：无 seed 时 stop 直接指派给 policy owner，同样零 stop，需确认修复后该路径与 seed 路径行为一致（CLI 自包含测试也可一并收进矩阵）。PR 若被仓库拒绝，可在 issue 内附 diff 或按贡献流程另开 PR 引用本 issue，方便合入。

bsakaguchi commented

2026-10-31 00:38:14 +00:00

Author

Owner

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

Checked the reported fix on main at d87e73f2db.

Service-worker policy cleanup now latches stop requests independently of the restoration promise and follows restoration completion with fulfillStopRequest. The stop path uses Page.stopLoading while the policy session remains usable or main-frame fallback after release, and tracks confirmed stops. Session cleanup also checks the latest stop obligation before browser release.

Implementation: PlaywrightScenarioServiceWorkers.ts, PlaywrightScenarioServiceWorkers.ts.

The relevant regression checks in PlaywrightScenarioSession.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. Service-worker policy cleanup now latches stop requests independently of the restoration promise and follows restoration completion with fulfillStopRequest. The stop path uses Page.stopLoading while the policy session remains usable or main-frame fallback after release, and tracks confirmed stops. Session cleanup also checks the latest stop obligation before browser release. Implementation: [PlaywrightScenarioServiceWorkers.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/browser/PlaywrightScenarioServiceWorkers.ts#L374), [PlaywrightScenarioServiceWorkers.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/browser/PlaywrightScenarioServiceWorkers.ts#L401). The relevant regression checks in [PlaywrightScenarioSession.test.ts](https://github.com/morluto/rea/blob/d87e73f2db50be72d33a4b3c7199af35518b441c/src/browser/PlaywrightScenarioSession.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:08:27 +00:00

[PR #3240] [CLOSED] fix(native): retain Swift manglings the demangler accepts in type analysis #5666

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

No description provided.

Rows
Columns