[GH-ISSUE #4256] [Bug] Browser scenario returns success after cancellation during final session close #1678

New issue

Closed

opened 2026-10-16 21:08:57 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-16 21:08:57 +00:00

Owner

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

[Bug] Browser scenario reports success when cancellation arrives during final cleanup

Observed result

Reproduced through PlaywrightBrowserScenarioProvider with a disposable fake session: the scenario steps complete, session.close() aborts the request signal during finalization, and the provider returns ok: true with a completed capture. This does not claim that an MCP client receives a success payload after its cancellation promise settles.

Baseline: repository 0e1f0bf2d, macOS, Node v24.18.0.

Evidence

src/browser/PlaywrightBrowserScenarioProvider.ts:273-337 awaits session.close() and then returns a schema-validated capture without rechecking options.signal.
src/application/BrowserScenarioCaptureService.ts:25-39 converts provider success directly into Evidence and has no post-await cancellation check.
Reproduce with the retained ignored fixture: PATH=/opt/homebrew/opt/node@24/bin:$PATH ./node_modules/.bin/vitest run --config .cache/codebase-audit/browser/vitest-audit.config.mts .cache/codebase-audit/browser/scenario-late-cancellation.repro.test.ts. Output confirms trigger=session.close, signal_aborted=true, completed_steps=2, and provider_result=success.
Existing #4005 describes a similar late-cancellation outcome in process capture; #4227 is the broad lifecycle audit design. No exact browser-scenario report was found in all-state GitHub search.

Impact

Cancellation that wins during the last awaited cleanup is silently reported as a successful scenario capture, contrary to the lifecycle rule that success must check cancellation after finalization. The service publishes an Evidence record for the completed output.

Proposed structural fix

After successful cleanup, check the request signal before returning capture success. On late cancellation, retain the completed scenario capture as partial_observation under the typed cancellation error, including the confirmed cleanup state. Preserve cleanup failure precedence and retained cleanup ownership.

Acceptance check

Synchronize cancellation inside session.close() after all steps are captured. Verify the non-aborted control succeeds; the late-aborted case returns cancellation with complete step/event facts and cleanup state retained; cleanup-failure and operation-failure controls preserve their current primary/cleanup precedence and retry owner; the external target remains usable.

Originally created by @morluto on GitHub (Oct 15, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/4256 # [Bug] Browser scenario reports success when cancellation arrives during final cleanup ## Observed result Reproduced through `PlaywrightBrowserScenarioProvider` with a disposable fake session: the scenario steps complete, `session.close()` aborts the request signal during finalization, and the provider returns `ok: true` with a completed capture. This does not claim that an MCP client receives a success payload after its cancellation promise settles. Baseline: repository `0e1f0bf2d`, macOS, Node `v24.18.0`. ## Evidence - `src/browser/PlaywrightBrowserScenarioProvider.ts:273-337` awaits `session.close()` and then returns a schema-validated capture without rechecking `options.signal`. - `src/application/BrowserScenarioCaptureService.ts:25-39` converts provider success directly into Evidence and has no post-await cancellation check. - Reproduce with the retained ignored fixture: `PATH=/opt/homebrew/opt/node@24/bin:$PATH ./node_modules/.bin/vitest run --config .cache/codebase-audit/browser/vitest-audit.config.mts .cache/codebase-audit/browser/scenario-late-cancellation.repro.test.ts`. Output confirms `trigger=session.close`, `signal_aborted=true`, `completed_steps=2`, and `provider_result=success`. - Existing #4005 describes a similar late-cancellation outcome in process capture; #4227 is the broad lifecycle audit design. No exact browser-scenario report was found in all-state GitHub search. ## Impact Cancellation that wins during the last awaited cleanup is silently reported as a successful scenario capture, contrary to the lifecycle rule that success must check cancellation after finalization. The service publishes an Evidence record for the completed output. ## Proposed structural fix After successful cleanup, check the request signal before returning capture success. On late cancellation, retain the completed scenario capture as `partial_observation` under the typed cancellation error, including the confirmed cleanup state. Preserve cleanup failure precedence and retained cleanup ownership. ## Acceptance check Synchronize cancellation inside `session.close()` after all steps are captured. Verify the non-aborted control succeeds; the late-aborted case returns cancellation with complete step/event facts and cleanup state retained; cleanup-failure and operation-failure controls preserve their current primary/cleanup precedence and retry owner; the external target remains usable.

bsakaguchi

2026-10-16 21:08:57 +00:00

closed this issue
added the
triage

bug
labels

bsakaguchi commented

2026-10-16 21:09:00 +00:00

Author

Owner

@morluto commented on GitHub (Oct 16, 2026):

Fixed on main in 93c620c3e. Browser scenario completion now checks cancellation after final session close and retains completed scenario facts under typed cancellation.

A barrier regression aborts during final close and checks completed steps and cleanup using a fixture session.

Hosted main CI is still pending; this closure is based on the implemented fix and the scoped regression evidence.

@morluto commented on GitHub (Oct 16, 2026): Fixed on `main` in [93c620c3e](https://github.com/morluto/rea/commit/93c620c3e7c401be40b71c480518147c76ade4dc). Browser scenario completion now checks cancellation after final session close and retains completed scenario facts under typed cancellation. A barrier regression aborts during final close and checks completed steps and cleanup using a fixture session. Hosted main CI is still pending; this closure is based on the implemented fix and the scoped regression evidence.

bsakaguchi referenced this issue

2026-10-16 21:09:33 +00:00

[GH-ISSUE #4373] [Bug] Empty header resource mapping skips a section starting at the same RVA #1729

bsakaguchi referenced this issue

2026-10-17 21:02:17 +00:00

[GH-ISSUE #3970] [Bug] inspect-pe-resources rejects a Windows-readable zero-length resource at the section end #1572

bsakaguchi referenced this issue

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

[PR #1678] [MERGED] fix(native): bound PE resource RVAs by section virtual size #4747

bsakaguchi referenced this issue

2026-10-31 00:54:39 +00:00

[PR #1679] [CLOSED] fix(native): bound NativeAOT PE reads by section virtual size #4749