[PR #5174] fix(browser): enforce web bundle result acceptance deadlines #6802

New issue

Open

opened 2026-11-01 21:43:08 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:43:08 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5174
Author: @August-Z
Created: 10/19/2026
Status: 🔄 Open

Base: main ← Head: augustz/fix-web-bundle-owned-worker

📝 Commits (6)

33686bf fix(browser): analyze captured web bundles in an owned worker
5604f8e fix(browser): keep web bundle worker results through late failures
03f0502 fix(browser): enforce bundle deadlines and provider cleanup
0e48146 fix(browser): bound bundle resources and retain cleanup failures
48d34b0 fix(browser): retain accepted findings through file cleanup
ce995f0 fix(browser): enforce result acceptance deadlines

📊 Changes

2 files changed (+75 additions, -1 deletions)

View changed files

📝 src/browser/WebBundleAnalysisWorker.test.ts (+72 -1)
📝 src/browser/WebBundleAnalysisWorker.ts (+3 -0)

📄 Description

Summary

A web-bundle worker result could still be accepted after its script deadline if synchronous JSON decoding or schema validation blocked the host event loop. Check the existing absolute deadline immediately after validation, before retaining the facts, so an overdue result follows the existing timeout and worker cleanup path.

Problem and expected behavior

The timer cannot interrupt synchronous result decoding or validation. Previously, a result that completed those steps after the deadline could be reported as successful. It must instead return AnalysisTimeoutError, retain no completed facts for that script, and allow a fresh owned worker to handle the next request.

Change and scope

The final diff contains only the result-acceptance check and two real-worker regressions. Main now contains the broader isolation and partial-result implementation in 9aca543f4, along with the shared owned-worker runtime. This PR adopts that implementation and removes its superseded parallel implementation. The upstream analyzer also addresses the latest accepted-prefix review finding: accepted facts remain available while cancelled and unstarted scripts remain explicit unknowns.

This is a focused correction within the existing browser-analysis boundary. A new maintainer scope decision is not applicable. No separate issue is linked; the result-acceptance gap was reproduced while resolving this PR's automated review.

Contract and boundary impact

Owner: WebBundleAnalysisWorker checks the existing absolute script deadline at the point where it accepts validated results.
CLI/MCP and compatibility: no input, schema, package, generated metadata, or installation changes relative to current main. Both adapters retain the existing partial-Evidence contract.
Evidence and provenance: overdue results are rejected before recording completed facts; earlier accepted scripts continue to use main's partial-result handling. Existing completed-read cleanup/cancellation recovery is preserved.
Authority and resources: static analysis of captured source; the existing target selection, CDP behavior, owned worker supervision, and private-file cleanup remain in effect. No new target execution or mutation capability.

Expected effects through the existing workflow:

Read existing files or captured data.
Launch or attach to a process.
Inject instrumentation or run target scripts/code.
Change files: create/remove owned private source and result files.
Contact network services through existing CDP/source-map behavior.
Recover protected code or work around protections.

Evidence and regression coverage

On Node 24.18.0/macOS, both new regressions failed against main at 2e5171e39: a controlled one-second host stall during decoding or validation exceeded a 750 ms budget, yet analysis returned success. With the check, both return a timeout with no completed facts, observe the old worker exit, and verify a replacement successfully analyzes the next script.

The focused suite also covers accepted-prefix preservation on cancellation/timeouts, unstarted suffixes, cleanup retry, completed-read recovery, worker capacity, result limits, and public CLI/MCP timeout Evidence and recovery.

Validation performed

npm run check — passed typecheck, lint, format, knip, metadata, module boundaries, and architecture guards.
npm run verify:test-discovery — passed; all 1,632 test files assigned exactly once.
Focused tests — 80 passed across nine files, using the command below.
git diff --check — passed.
Temporary real headless Chrome loopback fixture through public CLI analyze-web-bundle and stdio MCP analyze_web_bundle — passed with equal observations, complete results, one script, route /verified-route, and endpoint /api. Owned browser/profile/server cleanup completed.

Focused test command

npm run test:focused -- src/browser/WebBundleAnalysisWorker.test.ts src/browser/IsolatedWebBundleAnalyzer.test.ts src/browser/WebBundleAnalysisWorkerReadCompletion.test.ts src/browser/WebBundleAnalysisWorkerCapacity.test.ts src/browser/WebBundleAnalysisWorkerServer.test.ts src/domain/webBundleAnalyzer.test.ts src/application/BrowserObservationService.test.ts tests/acceptance/browser/webBundleContainmentCli.test.ts tests/boundary/mcp/webBundleContainmentMcp.test.ts

Compatibility, safety, and release

No breaking change, migration, or release metadata change. Real-browser coverage is macOS Chrome, and the regression workers ran on Node. Windows, Linux, Bun, and packaged installation were not verified for this final diff. The synchronous work is detected after it returns; this does not make JSON decoding or validation preemptible. No performance improvement is claimed or benchmarked. Local validation does not establish a passing GitHub CI run; CI and human readiness declarations remain separate requirements.

Author readiness

Human owner: @August-Z

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.

Before merge

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked, or the reason for not linking one is stated above.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed.
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

_{🔄 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/5174 **Author:** [@August-Z](https://github.com/August-Z) **Created:** 10/19/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `augustz/fix-web-bundle-owned-worker` --- ### 📝 Commits (6) - [`33686bf`](https://github.com/morluto/rea/commit/33686bf44b4709e193cab1b51991ab2fcb2e41fd) fix(browser): analyze captured web bundles in an owned worker - [`5604f8e`](https://github.com/morluto/rea/commit/5604f8e7478bab2b6de24b84940c9fb33167d21f) fix(browser): keep web bundle worker results through late failures - [`03f0502`](https://github.com/morluto/rea/commit/03f05026ca88ecbad22d03ca5344159e41acae27) fix(browser): enforce bundle deadlines and provider cleanup - [`0e48146`](https://github.com/morluto/rea/commit/0e481469aa9fd377bcc5a9281c54be1df6acb0a6) fix(browser): bound bundle resources and retain cleanup failures - [`48d34b0`](https://github.com/morluto/rea/commit/48d34b0816dc4b9b11903f57e804c150ea46d543) fix(browser): retain accepted findings through file cleanup - [`ce995f0`](https://github.com/morluto/rea/commit/ce995f0650175c4d0a4ee151747a5788d2f5784f) fix(browser): enforce result acceptance deadlines ### 📊 Changes **2 files changed** (+75 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `src/browser/WebBundleAnalysisWorker.test.ts` (+72 -1) 📝 `src/browser/WebBundleAnalysisWorker.ts` (+3 -0) </details> ### 📄 Description ## Summary A web-bundle worker result could still be accepted after its script deadline if synchronous JSON decoding or schema validation blocked the host event loop. Check the existing absolute deadline immediately after validation, before retaining the facts, so an overdue result follows the existing timeout and worker cleanup path. ## Problem and expected behavior The timer cannot interrupt synchronous result decoding or validation. Previously, a result that completed those steps after the deadline could be reported as successful. It must instead return `AnalysisTimeoutError`, retain no completed facts for that script, and allow a fresh owned worker to handle the next request. ## Change and scope The final diff contains only the result-acceptance check and two real-worker regressions. Main now contains the broader isolation and partial-result implementation in `9aca543f4`, along with the shared owned-worker runtime. This PR adopts that implementation and removes its superseded parallel implementation. The upstream analyzer also addresses the latest [accepted-prefix review finding](https://github.com/morluto/rea/pull/5174#discussion_r4310562667): accepted facts remain available while cancelled and unstarted scripts remain explicit unknowns. This is a focused correction within the existing browser-analysis boundary. A new maintainer scope decision is not applicable. No separate issue is linked; the result-acceptance gap was reproduced while resolving this PR's automated review. ## Contract and boundary impact - **Owner:** `WebBundleAnalysisWorker` checks the existing absolute script deadline at the point where it accepts validated results. - **CLI/MCP and compatibility:** no input, schema, package, generated metadata, or installation changes relative to current main. Both adapters retain the existing partial-Evidence contract. - **Evidence and provenance:** overdue results are rejected before recording completed facts; earlier accepted scripts continue to use main's partial-result handling. Existing completed-read cleanup/cancellation recovery is preserved. - **Authority and resources:** static analysis of captured source; the existing target selection, CDP behavior, owned worker supervision, and private-file cleanup remain in effect. No new target execution or mutation capability. Expected effects through the existing workflow: - [x] Read existing files or captured data. - [x] Launch or attach to a process. - [ ] Inject instrumentation or run target scripts/code. - [x] Change files: create/remove owned private source and result files. - [x] Contact network services through existing CDP/source-map behavior. - [ ] Recover protected code or work around protections. ## Evidence and regression coverage On Node 24.18.0/macOS, both new regressions failed against main at `2e5171e39`: a controlled one-second host stall during decoding or validation exceeded a 750 ms budget, yet analysis returned success. With the check, both return a timeout with no completed facts, observe the old worker exit, and verify a replacement successfully analyzes the next script. The focused suite also covers accepted-prefix preservation on cancellation/timeouts, unstarted suffixes, cleanup retry, completed-read recovery, worker capacity, result limits, and public CLI/MCP timeout Evidence and recovery. ## Validation performed - `npm run check` — passed typecheck, lint, format, knip, metadata, module boundaries, and architecture guards. - `npm run verify:test-discovery` — passed; all 1,632 test files assigned exactly once. - Focused tests — **80 passed across nine files**, using the command below. - `git diff --check` — passed. - Temporary real headless Chrome loopback fixture through public CLI `analyze-web-bundle` and stdio MCP `analyze_web_bundle` — passed with equal observations, complete results, one script, route `/verified-route`, and endpoint `/api`. Owned browser/profile/server cleanup completed. <details> <summary>Focused test command</summary> ```sh npm run test:focused -- src/browser/WebBundleAnalysisWorker.test.ts src/browser/IsolatedWebBundleAnalyzer.test.ts src/browser/WebBundleAnalysisWorkerReadCompletion.test.ts src/browser/WebBundleAnalysisWorkerCapacity.test.ts src/browser/WebBundleAnalysisWorkerServer.test.ts src/domain/webBundleAnalyzer.test.ts src/application/BrowserObservationService.test.ts tests/acceptance/browser/webBundleContainmentCli.test.ts tests/boundary/mcp/webBundleContainmentMcp.test.ts ``` </details> ## Compatibility, safety, and release No breaking change, migration, or release metadata change. Real-browser coverage is macOS Chrome, and the regression workers ran on Node. Windows, Linux, Bun, and packaged installation were not verified for this final diff. The synchronous work is detected after it returns; this does not make JSON decoding or validation preemptible. No performance improvement is claimed or benchmarked. Local validation does not establish a passing GitHub CI run; CI and human readiness declarations remain separate requirements. ## Author readiness  Human owner: @August-Z - [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. ## Before merge - [ ] This PR addresses a concrete problem or an agreed enhancement. - [ ] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [ ] The PR has one focused outcome and the title follows `type(scope): outcome`. - [ ] Related issue is linked, or the reason for not linking one is stated above. - [ ] Tests cover changed observable behavior and meaningful failure paths. - [ ] Owning docs, contracts, and generated metadata are updated where needed. - [ ] User-visible CLI/MCP changes include representative output. - [ ] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-11-01 21:43:08 +00:00

bsakaguchi referenced this issue

2026-11-01 21:47:42 +00:00

[PR #6802] [MERGED] fix(wasm): retain glue query and fragment across line terminators #7417

bsakaguchi referenced this issue

2026-11-03 00:29:36 +00:00

[GH-ISSUE #5654] [Tracker] Open PRs that can't merge yet: conflicts and red CI (October 2026) #3425