[PR #6276] [CLOSED] fix(browser): start coverage at domain enablement #7249

Closed
opened 2026-11-01 21:46:28 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6276
Author: @sachinkr7368
Created: 10/22/2026
Status: ❌ Closed

Base: main ← Head: fix/6232-coverage-start


📝 Commits (1)

  • 5abcff3 fix(browser): start coverage at domain enablement

📊 Changes

3 files changed (+105 additions, -10 deletions)

View changed files

📝 docs/browser-observation.md (+1 -1)
📝 src/browser/CdpPageCapture.ts (+15 -9)
➕ tests/boundary/mcp/coverageStart.test.ts (+89 -0)

📄 Description

Summary

inspect_web_page now reports network.coverage_started_at and console.coverage_started_at from the moment REA dispatches the corresponding CDP domain-enable command, instead of reusing capture_window.started_at from before attach and setup. Events delivered while a domain is being enabled therefore remain inside that section's reported coverage boundary. Fixes #6232.

Problem and expected behavior

On the base revision, normalizedInspection set both section timestamps to state.startedAt, which is captured before Page.enable, the authorized-main-frame check and enableObservationDomains. A successful MCP request against the repository's loopback CDP fake therefore returned:

  • network.coverage_started_at === capture_window.started_at, and
  • console.coverage_started_at === capture_window.started_at,

even though Runtime.enable and Network.enable had not been dispatched yet. CDP can deliver events after a command is dispatched but before its reply, and the capture already ingests those events; reporting a start later than their delivery would place retained events outside their own coverage window.

Expected: each section's coverage start reflects when observation of that domain becomes available, and an event delivered while a domain is being enabled remains represented within the boundary. capture_window.started_at and prior_activity_available: false are unchanged.

Change and scope

  • src/browser/CdpPageCapture.ts: add networkCoverageStartedAt / consoleCoverageStartedAt to CaptureState; record each immediately before dispatching Network.enable / Runtime.enable; report them in the two section objects.
  • tests/boundary/mcp/coverageStart.test.ts: MCP regression with deterministic 25 ms reply barriers for Page.enable, Runtime.enable and Network.enable, a Network.enable event delivered before its reply, and assertions that both starts are later than capture entry, at/after the Page.enable reply, at/before their enable command reaches the fake, and that the pre-reply request remains present. The per-command barriers make a timestamp recorded after an enable reply fail the test.
  • docs/browser-observation.md: clarify that coverage starts when REA begins enabling the domains and includes events delivered during enablement.

Not included: no change to event filtering or retention, capture_window, prior_activity_available, the scripts/workers/storage sections, or CDP command order. This is an ordinary fix inside the existing inspect_web_page boundary; no new capability or maintainer scope decision is required.

Contract and boundary impact

  • Semantic owner and earliest changed stage: browser observation normalization in src/browser/CdpPageCapture.ts; no new capture stage.
  • Contribution scope fit and linked maintainer decision: existing inspect_web_page behavior; not applicable (no new capability).
  • CLI and MCP/tool-catalog contract: the inspect_web_page output schema is unchanged; the two timestamps now reflect domain enablement. CLI projection carries the same corrected timestamps.
  • Provider, bridge, target-format, or platform compatibility: none; CDP command order and transport are unchanged.
  • Evidence, artifact, provenance, or reconstruction contract: unchanged; raw headers, bodies, cookies, storage values and console objects remain unretained.
  • Target selection, authority, upstream behavior, and cleanup details (if applicable): unchanged; authorized-main-frame, origin checks and capture cleanup are untouched.
  • Generated metadata (docs/public/product-catalog.json), package, or installation impact: none.

Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.

  • Read existing files or captured data.
  • Launch or attach to a process.
  • Inject instrumentation or run scripts/code.
  • Change process memory, registers, files, or configuration.
  • Contact network services.
  • Recover protected code or work around protections.
  • Documentation or usability only; no target interaction.
  • Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

  • Tests added or updated: tests/boundary/mcp/coverageStart.test.ts exercises the registered MCP tool and production CDP provider against the loopback fake, with a 25 ms Page.enable reply barrier and a Network.enable event sent before its reply.
  • Base reproduction: with the source fix reverted and the regression kept, npm run test:boundary -- tests/boundary/mcp/coverageStart.test.ts failed at expect(networkStart).toBeGreaterThan(captureStart) with expected 1792652942473 to be greater than 1792652942473; the same test passes with the fix. This shows coverage previously started at capture entry rather than at domain enablement.
  • Forward check: a temporary implementation that recorded both timestamps after the enable replies failed the same test with expected 1792654033686 to be less than or equal to 1792654033654, confirming the barriers distinguish dispatch from reply. The mutation was reverted.
  • User-visible CLI/MCP output (if applicable): inspect_web_page now returns network.coverage_started_at and console.coverage_started_at after capture_window.started_at, at or before the corresponding Network.enable / Runtime.enable dispatch. All other output is unchanged.
  • Remaining proof gaps: no real browser was run; ordering is exercised against the repository's loopback CDP fake, which models command latency and pre-reply events deterministically. The change is platform-neutral and has no OS-specific branches.

For evidence-bearing changes:

  • Observed, derived, and inferred claims remain distinguishable.
  • Artifact identity, source provenance, and failed attempts remain preserved.
  • Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

  • Base reproduction with the source fix reverted — 1 file, 1 failed (networkStart === captureStart).
  • Temporary post-reply timestamp mutation (reverted) — 1 file, 1 failed (networkStart exceeded the Network.enable receipt time).
  • npm run test:boundary -- tests/boundary/mcp/coverageStart.test.ts — 1 file, 1 test passed with the fix.
  • npm run test:boundary -- tests/boundary/mcp/browserMcp.test.ts tests/boundary/browser/ — 60 passed | 1 skipped (61 files), 792 passed | 6 skipped (798 tests).
  • npm run test:local -- <all 56 src/browser/**/*.test.ts files> — 56 files, 744 tests passed.
  • npm run typecheck — exit 0.
  • npm run check — pass (Turbo tasks and architecture guards across 1,785 source files).
  • npm run docs:check — 9 tests passed.
  • npx oxfmt --check src/browser/CdpPageCapture.ts tests/boundary/mcp/coverageStart.test.ts docs/browser-observation.md — pass.
  • git diff --check — clean.

Compatibility, safety, and release

  • Breaking changes or migration steps: none. The timestamps are descriptive metadata within the existing schema.
  • Real Hopper/Ghidra, browser, or OS coverage: no real browser was used; the production CDP provider was exercised against the suite's loopback HTTP/WebSocket fake. See the proof gaps above.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: none. The change only moves two timestamps later within an already-reported capture window; no new data is retained.

Author readiness

Human owner: @sachinkr7368

  • 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.

AI-assisted: the diff and evidence were prepared by an AI coding agent. The human owner directed that this PR be marked ready for initial review and dictated the readiness declarations above.

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/6276 **Author:** [@sachinkr7368](https://github.com/sachinkr7368) **Created:** 10/22/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/6232-coverage-start` --- ### 📝 Commits (1) - [`5abcff3`](https://github.com/morluto/rea/commit/5abcff327182b4a218f55717cb247e454d9a9994) fix(browser): start coverage at domain enablement ### 📊 Changes **3 files changed** (+105 additions, -10 deletions) <details> <summary>View changed files</summary> 📝 `docs/browser-observation.md` (+1 -1) 📝 `src/browser/CdpPageCapture.ts` (+15 -9) ➕ `tests/boundary/mcp/coverageStart.test.ts` (+89 -0) </details> ### 📄 Description ## Summary `inspect_web_page` now reports `network.coverage_started_at` and `console.coverage_started_at` from the moment REA dispatches the corresponding CDP domain-enable command, instead of reusing `capture_window.started_at` from before attach and setup. Events delivered while a domain is being enabled therefore remain inside that section's reported coverage boundary. Fixes #6232. ## Problem and expected behavior On the base revision, `normalizedInspection` set both section timestamps to `state.startedAt`, which is captured before `Page.enable`, the authorized-main-frame check and `enableObservationDomains`. A successful MCP request against the repository's loopback CDP fake therefore returned: - `network.coverage_started_at === capture_window.started_at`, and - `console.coverage_started_at === capture_window.started_at`, even though `Runtime.enable` and `Network.enable` had not been dispatched yet. CDP can deliver events after a command is dispatched but before its reply, and the capture already ingests those events; reporting a start later than their delivery would place retained events outside their own coverage window. Expected: each section's coverage start reflects when observation of that domain becomes available, and an event delivered while a domain is being enabled remains represented within the boundary. `capture_window.started_at` and `prior_activity_available: false` are unchanged. ## Change and scope - `src/browser/CdpPageCapture.ts`: add `networkCoverageStartedAt` / `consoleCoverageStartedAt` to `CaptureState`; record each immediately before dispatching `Network.enable` / `Runtime.enable`; report them in the two section objects. - `tests/boundary/mcp/coverageStart.test.ts`: MCP regression with deterministic 25 ms reply barriers for `Page.enable`, `Runtime.enable` and `Network.enable`, a `Network.enable` event delivered before its reply, and assertions that both starts are later than capture entry, at/after the `Page.enable` reply, at/before their enable command reaches the fake, and that the pre-reply request remains present. The per-command barriers make a timestamp recorded after an enable reply fail the test. - `docs/browser-observation.md`: clarify that coverage starts when REA begins enabling the domains and includes events delivered during enablement. Not included: no change to event filtering or retention, `capture_window`, `prior_activity_available`, the scripts/workers/storage sections, or CDP command order. This is an ordinary fix inside the existing `inspect_web_page` boundary; no new capability or maintainer scope decision is required. ## Contract and boundary impact - Semantic owner and earliest changed stage: browser observation normalization in `src/browser/CdpPageCapture.ts`; no new capture stage. - Contribution scope fit and linked maintainer decision: existing `inspect_web_page` behavior; not applicable (no new capability). - CLI and MCP/tool-catalog contract: the `inspect_web_page` output schema is unchanged; the two timestamps now reflect domain enablement. CLI projection carries the same corrected timestamps. - Provider, bridge, target-format, or platform compatibility: none; CDP command order and transport are unchanged. - Evidence, artifact, provenance, or reconstruction contract: unchanged; raw headers, bodies, cookies, storage values and console objects remain unretained. - Target selection, authority, upstream behavior, and cleanup details (if applicable): unchanged; authorized-main-frame, origin checks and capture cleanup are untouched. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact: none. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. - [x] Read existing files or captured data. - [x] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: `tests/boundary/mcp/coverageStart.test.ts` exercises the registered MCP tool and production CDP provider against the loopback fake, with a 25 ms `Page.enable` reply barrier and a `Network.enable` event sent before its reply. - Base reproduction: with the source fix reverted and the regression kept, `npm run test:boundary -- tests/boundary/mcp/coverageStart.test.ts` failed at `expect(networkStart).toBeGreaterThan(captureStart)` with `expected 1792652942473 to be greater than 1792652942473`; the same test passes with the fix. This shows coverage previously started at capture entry rather than at domain enablement. - Forward check: a temporary implementation that recorded both timestamps after the enable replies failed the same test with `expected 1792654033686 to be less than or equal to 1792654033654`, confirming the barriers distinguish dispatch from reply. The mutation was reverted. - User-visible CLI/MCP output (if applicable): `inspect_web_page` now returns `network.coverage_started_at` and `console.coverage_started_at` after `capture_window.started_at`, at or before the corresponding `Network.enable` / `Runtime.enable` dispatch. All other output is unchanged. - Remaining proof gaps: no real browser was run; ordering is exercised against the repository's loopback CDP fake, which models command latency and pre-reply events deterministically. The change is platform-neutral and has no OS-specific branches. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - Base reproduction with the source fix reverted — 1 file, 1 failed (`networkStart === captureStart`). - Temporary post-reply timestamp mutation (reverted) — 1 file, 1 failed (`networkStart` exceeded the `Network.enable` receipt time). - `npm run test:boundary -- tests/boundary/mcp/coverageStart.test.ts` — 1 file, 1 test passed with the fix. - `npm run test:boundary -- tests/boundary/mcp/browserMcp.test.ts tests/boundary/browser/` — 60 passed | 1 skipped (61 files), 792 passed | 6 skipped (798 tests). - `npm run test:local -- <all 56 src/browser/**/*.test.ts files>` — 56 files, 744 tests passed. - `npm run typecheck` — exit 0. - `npm run check` — pass (Turbo tasks and architecture guards across 1,785 source files). - `npm run docs:check` — 9 tests passed. - `npx oxfmt --check src/browser/CdpPageCapture.ts tests/boundary/mcp/coverageStart.test.ts docs/browser-observation.md` — pass. - `git diff --check` — clean. ## Compatibility, safety, and release - Breaking changes or migration steps: none. The timestamps are descriptive metadata within the existing schema. - Real Hopper/Ghidra, browser, or OS coverage: no real browser was used; the production CDP provider was exercised against the suite's loopback HTTP/WebSocket fake. See the proof gaps above. - Package or release metadata impact: none. - Security, privacy, process, or containment review: none. The change only moves two timestamps later within an already-reported capture window; no new data is retained. ## Author readiness Human owner: @sachinkr7368 - [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. _AI-assisted: the diff and evidence were prepared by an AI coding agent. The human owner directed that this PR be marked ready for initial review and dictated the readiness declarations above._ ## 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 2026-11-01 21:46:28 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
morluto/rea#7249
No description provided.