[PR #4034] [MERGED] fix(ci): reject false browser environment cancellation passes #6005

New issue

Closed

opened 2026-10-31 01:11:06 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:11:06 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4034
Author: @N0zoM1z0
Created: 10/15/2026
Status: ✅ Merged
Merged: 10/15/2026
Merged by: @N0zoM1z0

Base: main ← Head: fix/ci-browser-environment-cancellation-20261015

📝 Commits (1)

7810786 fix(ci): reject false browser environment cancellation passes

📊 Changes

2 files changed (+114 additions, -19 deletions)

View changed files

📝 scripts/lib/browser-scenario-environment-e2e.mjs (+37 -19)
➕ tests/boundary/process/browserEnvironmentVerifier.test.ts (+77 -0)

📄 Description

Summary

The real-browser environment verifier accepted unrelated provider errors and incomplete cleanup as successful cancellation. Require AnalysisCancelledError with confirmed cleanup, or a cancelled step with disconnected-external cleanup. Close the owned provider and observer on every protected outcome, preserving primary and cleanup failures.

Fixes #4033. Verifier-only change within existing authority; no CLI/MCP, Evidence, provider or generated-metadata contract changes. Storage/service-worker cleanup work in #4010 is separate. Main subsequently added fixture restoration fix e45eb33f; this PR does not claim to diagnose the earlier main browser failure.

Evidence and validation

Six controlled outcomes execute the verifier's actual cancellation function. The regression fails on baseline 337d2e33 because protocol failure and two incomplete-cleanup outcomes incorrectly pass; all six expectations pass after the fix. Typed cancellation and completed external cleanup remain accepted.
npm run test:focused -- tests/boundary/process/browserEnvironmentVerifier.test.ts tests/boundary/process/verifierCleanup.test.ts — 3/3 passed.
npm run verify:test-discovery — all 1,328 test files discovered once.
npm run check:fast, npm run check:ci, changed-file formatting and syntax checks — passed.
npm run verify:browser — complete real Chromium 153.0.8010.12 CLI/MCP lane passed on Linux x64 with Node 24.18.0/npm 11.16.0. Rerun on the latest-main integration candidate (e45eb33f plus this patch) also passed, with no owned descendants remaining; focused regression and check:fast passed on that candidate.
Complete PR CI — all selected lanes passed: 1,328 files, 15,350 tests passed, 135 skipped, zero failed. Hosted Chrome 154.0.8037.97 verified the complete browser lane and empty final descendants. Codex and Greptile reviews completed with no actionable findings.
Merged-main CI — passed on e18fccc5; all 30 jobs accounted for, selected gates successful. The merged tree equals the tested latest-main candidate; focused tests and discovery also passed after merge.
Audit controls: 274/274 CI selection/cache/report/inventory tests passed on initial main 6688ed6d.

The completed browser run used a warm portable build cache and a fresh owned browser/profile. It took 147.86 s wall, 139.09/36.90 s user/system CPU and 4.31 GiB sampled descendant RSS at 50 ms. Shared pages count repeatedly; transient/reparented children may be missed. The shared host and concurrent checks prevent a speedup claim. Agent tokens and Windows/macOS engine behavior were not measured. Output contracts and required coverage remain unchanged.

Author readiness

Human owner: @N0zoM1z0

The owner explicitly directed the agent to complete these declarations and open a ready PR in the task. Technical diff review and validation were delegated to Codex; no separate human diff review is claimed.

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/4034 **Author:** [@N0zoM1z0](https://github.com/N0zoM1z0) **Created:** 10/15/2026 **Status:** ✅ Merged **Merged:** 10/15/2026 **Merged by:** [@N0zoM1z0](https://github.com/N0zoM1z0) **Base:** `main` ← **Head:** `fix/ci-browser-environment-cancellation-20261015` --- ### 📝 Commits (1) - [`7810786`](https://github.com/morluto/rea/commit/78107867d98dc4a2f748a3f50e4df8b3aace1c53) fix(ci): reject false browser environment cancellation passes ### 📊 Changes **2 files changed** (+114 additions, -19 deletions) <details> <summary>View changed files</summary> 📝 `scripts/lib/browser-scenario-environment-e2e.mjs` (+37 -19) ➕ `tests/boundary/process/browserEnvironmentVerifier.test.ts` (+77 -0) </details> ### 📄 Description ## Summary The real-browser environment verifier accepted unrelated provider errors and incomplete cleanup as successful cancellation. Require `AnalysisCancelledError` with confirmed cleanup, or a cancelled step with `disconnected-external` cleanup. Close the owned provider and observer on every protected outcome, preserving primary and cleanup failures. Fixes #4033. Verifier-only change within existing authority; no CLI/MCP, Evidence, provider or generated-metadata contract changes. Storage/service-worker cleanup work in #4010 is separate. Main subsequently added fixture restoration fix `e45eb33f`; this PR does not claim to diagnose the earlier main browser failure. ## Evidence and validation - Six controlled outcomes execute the verifier's actual cancellation function. The regression fails on baseline `337d2e33` because protocol failure and two incomplete-cleanup outcomes incorrectly pass; all six expectations pass after the fix. Typed cancellation and completed external cleanup remain accepted. - `npm run test:focused -- tests/boundary/process/browserEnvironmentVerifier.test.ts tests/boundary/process/verifierCleanup.test.ts` — 3/3 passed. - `npm run verify:test-discovery` — all 1,328 test files discovered once. - `npm run check:fast`, `npm run check:ci`, changed-file formatting and syntax checks — passed. - `npm run verify:browser` — complete real Chromium 153.0.8010.12 CLI/MCP lane passed on Linux x64 with Node 24.18.0/npm 11.16.0. Rerun on the latest-main integration candidate (`e45eb33f` plus this patch) also passed, with no owned descendants remaining; focused regression and `check:fast` passed on that candidate. - [Complete PR CI](https://github.com/morluto/rea/actions/runs/38707119971) — all selected lanes passed: 1,328 files, 15,350 tests passed, 135 skipped, zero failed. Hosted Chrome 154.0.8037.97 verified the complete browser lane and empty final descendants. Codex and Greptile reviews completed with no actionable findings. - [Merged-main CI](https://github.com/morluto/rea/actions/runs/38709096952) — passed on `e18fccc5`; all 30 jobs accounted for, selected gates successful. The merged tree equals the tested latest-main candidate; focused tests and discovery also passed after merge. - Audit controls: 274/274 CI selection/cache/report/inventory tests passed on initial main `6688ed6d`. The completed browser run used a warm portable build cache and a fresh owned browser/profile. It took 147.86 s wall, 139.09/36.90 s user/system CPU and 4.31 GiB sampled descendant RSS at 50 ms. Shared pages count repeatedly; transient/reparented children may be missed. The shared host and concurrent checks prevent a speedup claim. Agent tokens and Windows/macOS engine behavior were not measured. Output contracts and required coverage remain unchanged. ## Author readiness Human owner: @N0zoM1z0 The owner explicitly directed the agent to complete these declarations and open a ready PR in the task. Technical diff review and validation were delegated to Codex; no separate human diff review is claimed. - [x] 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. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] 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:11:06 +00:00