[GH-ISSUE #2597] fix(browser): cancelled opening prevents retained cleanup from settling #913

New issue

Closed

opened 2026-10-15 18:29:32 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-15 18:29:32 +00:00

Owner

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

Summary

Cancelling a browser-scenario request while the browser is opening can leave its provider unable to capture another scenario. Even after the browser and profile have been released, retries report incomplete cleanup for the cancelled request.

Steps to reproduce

The following production-boundary probe controls the browser-opening promise and exercises the real session, cleanup owner, and provider. It does not launch Chromium. Save it as src/browser/cancelled-open.probe.test.ts and run npm run test:focused -- src/browser/cancelled-open.probe.test.ts. Its assertions describe the current failure.

import { expect, it, vi } from 'vitest';
const mocks = vi.hoisted(() => ({ open: vi.fn() }));
vi.mock('./PlaywrightScenarioBrowser.js', async (original) => ({ ...await original<object>(), openPlaywrightScenarioBrowser: mocks.open }));
import { PlaywrightScenarioBrowserCleanupOwner } from './PlaywrightScenarioBrowser.js';
import { PlaywrightScenarioSession } from './PlaywrightScenarioSession.js';
import { PlaywrightBrowserScenarioProvider } from './PlaywrightBrowserScenarioProvider.js';
import { browserScenarioSchema } from '../domain/browserScenario.js';
it('late cancelled opening releases resources but blocks subsequent captures', async () => {
  const scenario = browserScenarioSchema.parse({browser:{mode:'launch',executable_path:process.execPath},start_url:{url:'https://example.test/'},actions:[{step_id:'wait',action:'wait_for_timeout',duration_ms:1}]});
  const controller = new AbortController();
  let release!: (opened: unknown) => void;
  let began!: () => void;
  const started = new Promise<void>(resolve => { began = resolve; });
  mocks.open.mockImplementation(() => { began(); return new Promise(resolve => { release = resolve; }); });
  const closeBrowser = vi.fn(async () => undefined);
  const removeProfile = vi.fn(async () => undefined);
  const owner = new PlaywrightScenarioBrowserCleanupOwner({closeBrowser,removeProfile});
  const provider = new PlaywrightBrowserScenarioProvider((s, options) => PlaywrightScenarioSession.open(s, {}, options));
  const first = provider.captureScenario(scenario, {signal:controller.signal});
  await started;
  controller.abort();
  expect((await first).ok).toBe(false);
  release({cleanup:owner});
  await new Promise(resolve => setTimeout(resolve, 10));
  expect(owner.browserClosed).toBe(true);
  expect(removeProfile).toHaveBeenCalledOnce();
  for(let i=0;i<2;i++) {
    const retry = await provider.captureScenario(scenario);
    expect(retry.ok).toBe(false);
    if(!retry.ok) expect(retry.error).toMatchObject({reason:'cleanup_failed'});
  }
  expect(mocks.open).toHaveBeenCalledOnce();
  await expect(owner.close()).resolves.toBeUndefined();
  await expect(provider.close()).rejects.toMatchObject({reason:'cleanup_failed'});
});

Expected behavior

After a cancelled request releases its browser and private profile, retained cleanup should confirm completion and let the same provider accept later scenarios. Genuine release failures must remain owned and retryable.

Actual behavior

The probe observed browser close and profile removal once. Both later capture calls returned an error with reason cleanup_failed, and the browser-opening function was never called again. Direct cleanup without the cancelled signal succeeded, while provider shutdown still rejected.

Evidence

Expected source: AGENTS.md, Architecture and Cleanup; src/browser/PlaywrightBrowserScenarioProvider.ts:415-431 removes retained cleanup only after success.
Failure source: src/browser/PlaywrightScenarioSession.ts:151-158 retains the original signal; src/browser/PlaywrightScenarioBrowser.ts:69-89 reports cancellation as cleanup failure; src/browser/PlaywrightExecutionBoundary.ts:15-16 rejects an already-aborted signal.
Evidence provenance: observed
Local verification: reproduced
Reproduction completeness: complete

Restoration check

Repeat the cancelled deferred-opening case. After resource release, the retained callback must settle and a later capture must enter a new opening. Keep a control whose close genuinely rejects and confirm that its resources remain retained.

Runtime or environment

Source revision 8b181716aca2cb467be499cba226abfb5ae19141, Node v26.10.0, npm 11.19.1, Vitest 5.0.3, and the installed lockfile dependencies.

Triage assessment

Impact level: P1
Assessment status: supported
Impact basis: One cancellation blocks all future browser-scenario capture in the same provider despite completed release. Provider shutdown also continues to report incomplete cleanup.
Workaround status: unverified
Workaround basis: A fresh provider or restarted process avoids the retained callback by construction, but preservation of existing investigation state through a process restart was not verified.

Originally created by @coygeek on GitHub (Oct 13, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/2597 ## Summary Cancelling a browser-scenario request while the browser is opening can leave its provider unable to capture another scenario. Even after the browser and profile have been released, retries report incomplete cleanup for the cancelled request. ## Steps to reproduce The following production-boundary probe controls the browser-opening promise and exercises the real session, cleanup owner, and provider. It does not launch Chromium. Save it as `src/browser/cancelled-open.probe.test.ts` and run `npm run test:focused -- src/browser/cancelled-open.probe.test.ts`. Its assertions describe the current failure. ```ts import { expect, it, vi } from 'vitest'; const mocks = vi.hoisted(() => ({ open: vi.fn() })); vi.mock('./PlaywrightScenarioBrowser.js', async (original) => ({ ...await original<object>(), openPlaywrightScenarioBrowser: mocks.open })); import { PlaywrightScenarioBrowserCleanupOwner } from './PlaywrightScenarioBrowser.js'; import { PlaywrightScenarioSession } from './PlaywrightScenarioSession.js'; import { PlaywrightBrowserScenarioProvider } from './PlaywrightBrowserScenarioProvider.js'; import { browserScenarioSchema } from '../domain/browserScenario.js'; it('late cancelled opening releases resources but blocks subsequent captures', async () => { const scenario = browserScenarioSchema.parse({browser:{mode:'launch',executable_path:process.execPath},start_url:{url:'https://example.test/'},actions:[{step_id:'wait',action:'wait_for_timeout',duration_ms:1}]}); const controller = new AbortController(); let release!: (opened: unknown) => void; let began!: () => void; const started = new Promise<void>(resolve => { began = resolve; }); mocks.open.mockImplementation(() => { began(); return new Promise(resolve => { release = resolve; }); }); const closeBrowser = vi.fn(async () => undefined); const removeProfile = vi.fn(async () => undefined); const owner = new PlaywrightScenarioBrowserCleanupOwner({closeBrowser,removeProfile}); const provider = new PlaywrightBrowserScenarioProvider((s, options) => PlaywrightScenarioSession.open(s, {}, options)); const first = provider.captureScenario(scenario, {signal:controller.signal}); await started; controller.abort(); expect((await first).ok).toBe(false); release({cleanup:owner}); await new Promise(resolve => setTimeout(resolve, 10)); expect(owner.browserClosed).toBe(true); expect(removeProfile).toHaveBeenCalledOnce(); for(let i=0;i<2;i++) { const retry = await provider.captureScenario(scenario); expect(retry.ok).toBe(false); if(!retry.ok) expect(retry.error).toMatchObject({reason:'cleanup_failed'}); } expect(mocks.open).toHaveBeenCalledOnce(); await expect(owner.close()).resolves.toBeUndefined(); await expect(provider.close()).rejects.toMatchObject({reason:'cleanup_failed'}); }); ``` ## Expected behavior After a cancelled request releases its browser and private profile, retained cleanup should confirm completion and let the same provider accept later scenarios. Genuine release failures must remain owned and retryable. ## Actual behavior The probe observed browser close and profile removal once. Both later capture calls returned an error with reason `cleanup_failed`, and the browser-opening function was never called again. Direct cleanup without the cancelled signal succeeded, while provider shutdown still rejected. ## Evidence - Expected source: `AGENTS.md`, Architecture and Cleanup; `src/browser/PlaywrightBrowserScenarioProvider.ts:415-431` removes retained cleanup only after success. - Failure source: `src/browser/PlaywrightScenarioSession.ts:151-158` retains the original signal; `src/browser/PlaywrightScenarioBrowser.ts:69-89` reports cancellation as cleanup failure; `src/browser/PlaywrightExecutionBoundary.ts:15-16` rejects an already-aborted signal. - Evidence provenance: observed - Local verification: reproduced - Reproduction completeness: complete ## Restoration check Repeat the cancelled deferred-opening case. After resource release, the retained callback must settle and a later capture must enter a new opening. Keep a control whose close genuinely rejects and confirm that its resources remain retained. ## Runtime or environment Source revision `8b181716aca2cb467be499cba226abfb5ae19141`, Node v26.10.0, npm 11.19.1, Vitest 5.0.3, and the installed lockfile dependencies. ## Triage assessment - Impact level: P1 - Assessment status: supported - Impact basis: One cancellation blocks all future browser-scenario capture in the same provider despite completed release. Provider shutdown also continues to report incomplete cleanup. - Workaround status: unverified - Workaround basis: A fresh provider or restarted process avoids the retained callback by construction, but preservation of existing investigation state through a process restart was not verified.

bsakaguchi

2026-10-15 18:29:32 +00:00