[PR #4357] [CLOSED] fix(browser): retain interrupted raw cancellation observations #6198

New issue

Closed

opened 2026-10-31 01:12:37 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:12:37 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4357
Author: @geekwolf
Created: 10/16/2026
Status: ❌ Closed

Base: main ← Head: fix/browser-cancellation-evidence

📝 Commits (1)

d5e29aa fix(browser): retain interrupted raw cancellation observations

📊 Changes

3 files changed (+91 additions, -12 deletions)

View changed files

📝 src/application/BrowserScenarioCaptureService.test.ts (+52 -2)
📝 src/application/BrowserScenarioCaptureService.ts (+2 -10)
📝 tests/process-global/browserScenarioCleanup.test.ts (+37 -0)

📄 Description

Summary

Preserve typed cancellation and raw observations when a browser scenario is interrupted before a completed capture exists.

Current scope

Rebased onto main 8ab5024397ddbd3b37ab68a13cabedc2eb7ce7dd after #4274 independently fixed the broader completion and transport ownership regressions. This revision drops the superseded provider/CLI changes and keeps only the remaining application-boundary defect. No separate issue was opened; the reproductions below identify the residual bug precisely.

A retained browser-scenario-observation is explicitly allowed to omit global completeness and unfinished steps. The service incorrectly validated that failure partial as a completed capture, replacing AnalysisCancelledError with unreadable_output and discarding the observation. This is reachable through the real provider when the initial snapshot or session admission is interrupted.

Keep the original typed failure when the raw partial cannot be promoted. Preserve main's stronger rules: successful reports still require complete-schema validation, only fully completed retained reports become Evidence, and the provider still owns final cancellation classification. Raw partials are not fabricated into Evidence. Scope decision: not applicable to this correction within existing authority.

Executed validation

On clean main 8ab50243, the new service regression and the public CLI-boundary initial-snapshot regression both failed: cancellation became unreadable_output, and partial_observation disappeared. The CLI regression uses the real Playwright scenario provider and cleanup owner with a controlled session.
After the fix, five focused files yielded 37 passing tests and two existing Windows-gated POSIX signal skips. Coverage includes raw failure/cause identity, initial snapshot cancellation through CLI, active/final cancellation, primary and cleanup failures, completed-report Evidence promotion, malformed-success rejection and in-memory MCP.
Full npm run check, npm run check:fast, scoped formatting and git diff --check passed on Windows x64, Node 24.18.0 / npm 11.16.0.
No real Chromium/Electron, local POSIX signal or complete-suite claim. Hosted CI must evaluate this new head; previous-head results do not qualify it.

Related #4356 and #4358 are now covered by #4274 on main; their older implementations are not replayed here.

Author readiness

Human owner: @geekwolf

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/4357 **Author:** [@geekwolf](https://github.com/geekwolf) **Created:** 10/16/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/browser-cancellation-evidence` --- ### 📝 Commits (1) - [`d5e29aa`](https://github.com/morluto/rea/commit/d5e29aa44f43ef864311100a010a4e9c653485db) fix(browser): retain interrupted raw cancellation observations ### 📊 Changes **3 files changed** (+91 additions, -12 deletions) <details> <summary>View changed files</summary> 📝 `src/application/BrowserScenarioCaptureService.test.ts` (+52 -2) 📝 `src/application/BrowserScenarioCaptureService.ts` (+2 -10) 📝 `tests/process-global/browserScenarioCleanup.test.ts` (+37 -0) </details> ### 📄 Description ## Summary Preserve typed cancellation and raw observations when a browser scenario is interrupted before a completed capture exists. ## Current scope Rebased onto main `8ab5024397ddbd3b37ab68a13cabedc2eb7ce7dd` after #4274 independently fixed the broader completion and transport ownership regressions. This revision drops the superseded provider/CLI changes and keeps only the remaining application-boundary defect. No separate issue was opened; the reproductions below identify the residual bug precisely. A retained `browser-scenario-observation` is explicitly allowed to omit global completeness and unfinished steps. The service incorrectly validated that failure partial as a completed capture, replacing `AnalysisCancelledError` with `unreadable_output` and discarding the observation. This is reachable through the real provider when the initial snapshot or session admission is interrupted. Keep the original typed failure when the raw partial cannot be promoted. Preserve main's stronger rules: successful reports still require complete-schema validation, only fully completed retained reports become Evidence, and the provider still owns final cancellation classification. Raw partials are not fabricated into Evidence. Scope decision: not applicable to this correction within existing authority. ## Executed validation - On clean main `8ab50243`, the new service regression and the public CLI-boundary initial-snapshot regression both failed: cancellation became `unreadable_output`, and `partial_observation` disappeared. The CLI regression uses the real Playwright scenario provider and cleanup owner with a controlled session. - After the fix, five focused files yielded 37 passing tests and two existing Windows-gated POSIX signal skips. Coverage includes raw failure/cause identity, initial snapshot cancellation through CLI, active/final cancellation, primary and cleanup failures, completed-report Evidence promotion, malformed-success rejection and in-memory MCP. - Full `npm run check`, `npm run check:fast`, scoped formatting and `git diff --check` passed on Windows x64, Node 24.18.0 / npm 11.16.0. - No real Chromium/Electron, local POSIX signal or complete-suite claim. Hosted CI must evaluate this new head; previous-head results do not qualify it. Related #4356 and #4358 are now covered by #4274 on main; their older implementations are not replayed here. ## Author readiness Human owner: @geekwolf - [ ] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [ ] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [ ] I declare this PR ready for initial maintainer review. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:12:37 +00:00