[PR #5639] [CLOSED] fix(browser): exclude replayed lifecycle milestones from session activity #7006

New issue

Closed

opened 2026-11-01 21:44:39 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:44:39 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5639
Author: @Plastic-time
Created: 10/20/2026
Status: ❌ Closed

Base: main ← Head: fix/5609-lifecycle-window

📝 Commits (1)

5cdd46c fix(browser): exclude replayed lifecycle milestones from session activity

📊 Changes

4 files changed (+423 additions, -1 deletions)

View changed files

📝 docs/browser-observation.md (+6 -0)
📝 src/browser/CdpObservationSession.ts (+46 -1)
➕ tests/boundary/browser/cdpObservationLifecycleWindow.test.ts (+367 -0)
📝 tests/fixtures/fakeCdpBrowserResults.ts (+4 -0)

📄 Description

Status: superseded by upstream implementation.

Maintainer commit 2a0410f1 addresses #5609 with a unified observation-window boundary, pre-arm buffering, and explicit unknown event times. This replaces this PR's independent lifecycle cutoff. The setup-time review finding on this branch is therefore not being patched onto the obsolete implementation; merging that logic back would conflict with the new upstream contract.

Verification on upstream 23196daa containing that fix, Windows, Node 24.18.0/npm 11.16.0: runtime build passed; 38 tests across the window buffer/scope, timeline/lifecycle-failure, and public CLI/stdio MCP acceptance suites passed. An owned headless Chrome fixture also passed: public CLI idle observation returned zero timeline rows; production-provider reload and history controls retained lifecycle and same-document navigation respectively. This was focused validation, not the full suite or a Linux/macOS claim. The PR branch was left unchanged; no merge or force-push was performed.

AI-assisted follow-up verification. The original description below records the previous implementation and its historical evidence; it is not a claim that this branch is now compatible with current main.

Summary

Fixes #5609. Stop reporting replayed lifecycle milestones from an already-loaded page as new session activity. Preserve genuine milestones, including those emitted by the initial loader after observation starts.

Problem and expected behavior

On Windows, a real public CLI capture of an untouched loopback page returned six old lifecycle records despite zero fixture requests and unchanged NavigationStart. The pre-command browser Timestamp was 7743.787796; the returned lifecycle timestamps were 7741.275806 through 7741.313858. The CLI exited 0 with empty stderr and window_elapsed.

An observation should exclude those historical milestones while retaining actual reload, history navigation, and new lifecycle activity.

Change and scope

The existing CdpObservationSession owner samples the target's monotonic clock with Performance.enable/getMetrics/disable while its scope/termination listeners are active, before enabling lifecycle collection. It filters older lifecycle timestamps, preserves equality and new milestones from any loader, and records malformed event times as incomplete coverage. Missing/invalid clocks prevent arming explicitly.

This is an attach-time cutoff preceding armed_at, not a claim that all setup events before the armed notification are excluded. It adds three passive CDP requests and requires Performance-domain support. Ordinary correction within the existing capability boundary; no advance maintainer scope decision is required under the contribution rules.

Deduplication: #5609 remained open and the latest direct issue-to-PR search returned no results. #5605 already fixes long-window timing; it is retained. Open #4391 addresses a timer input limit, not historical replay. Open #4040 overlaps this owner: its current head 5251c43522f9b2ea6e81c7ccb25938b3af239976 was checked after its refresh and still changes URL identity/scope, not historical lifecycle admission. This PR uses a separate regression file; integration with #4040 will still need review. These are targeted checks, not a guarantee that every possible duplicate has been found.

Contract and boundary impact

Semantic owner and earliest changed stage: CDP session setup and TimelineCapture lifecycle admission.
Contribution scope: existing passive observation; no new execution, injection, mutation, or recovery authority.
CLI/MCP/tool-catalog: schemas unchanged; both consume the corrected owner result.
Provider compatibility: requires one finite, nonnegative browser Timestamp; unsupported/malformed clocks fail with a protocol error instead of silently accepting historical state.
Evidence: preserves timestamps, loader identity and ordering; invalid event times produce invalid_protocol_value coverage diagnostics.
Target/cleanup: same selected session and cancellation signal; Performance is disabled after sampling, and failures use existing owned-session transport cleanup.
Generated metadata/package/install: no dependency, package, binary, or tracked generated-file changes.

Expected effects of the existing workflow and this correction:

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.

REA attaches and sends passive protocol requests to the selected browser. The independent verifier owns its disposable browser and drives only its own loopback fixture.

Evidence and regression coverage

Original failure/failure-first base: 5b676ba9f302228b44c28ab076525abed3e647c1. Final candidate was reapplied without conflicts and fully revalidated against 7d6f90485ee078834ce64656770a34947bb81803 (REA 6.7.0). Node 24.18.0/npm 11.16.0, Windows x64, playwright-core 1.58.2, Chrome for Testing 145.0.7632.6.

Real public CLI control	Historical lifecycle before / after	Requests before / after	Fixed behavior
Idle, 2000 ms	6 / 0	0 / 0	Empty timeline; unchanged NavigationStart
Reload, 6000 ms	5 / 0	1 / 1	New load follows the independently sampled action time
History, 6000 ms	6 / 0	0 / 0	Same-document navigation retained

A deliberately blocked initial loader was released at the production provider's actual armed handshake: its load timestamp 8764.469588 follows the independent pre-release sample 8764.467771 and retains the same loader ID. This control uses the real provider, not the public CLI. A separate public stdio MCP call retained history navigation triggered by its actual armed notification, with zero old lifecycle records. REA left the borrowed page open; the verifier then closed its MCP client, independent CDP session, browser and HTTP server.

The initial four regressions failed on the unchanged base because [99, 100, 101] was returned instead of [100, 101]. Final tests cover late replay with acknowledged reply barriers, equal cutoff, initial-loader activity, both socket modes, frame/session isolation, malformed timestamps, all three new command failure/cancellation stages, sampling-time scope/termination, and post-terminal suppression. Existing long-window, armed/finalization cancellation, redirect, final-URL, application, CLI and MCP coverage is retained.

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

On the final tested base:

npm run test:focused -- tests/boundary/browser/cdpObservationLifecycleWindow.test.ts tests/boundary/browser/cdpBrowserProvider.timelineEvents.test.ts tests/boundary/browser/cdpObservationResourceTypes.test.ts tests/boundary/browser/cdpBrowserProvider.lifecycle_failures.test.ts tests/boundary/browser/cdpBrowserProvider.browserSocket.test.ts tests/boundary/cli/browserCli.test.ts tests/boundary/mcp/browserMcp.test.ts src/application/BrowserObservationService.test.ts src/domain/browserObservation.test.ts 鈥?9 files / 89 tests passed, 27.54 s; prerequisite runtime build passed.
npm run check 鈥?passed, including typecheck, lint, format, knip, metadata, module and architecture guards. Lint reports 0 errors and 1967 warnings.
npm run docs:check 鈥?passed, including 5 README consistency tests.
npm run verify:test-discovery 鈥?all 1826 test files discovered once; this is not a full-suite execution.
git diff --check 鈥?passed.
Real Chromium public CLI idle/reload/history, initial-loader control and public stdio MCP handshake 鈥?all rerun successfully on the final build, as detailed above.

The local harness/evidence remains preserved; this PR includes only the relevant source, tests and documentation, not personal paths, profiles, dependencies or binaries.

Compatibility, safety, and release

No schema migration or package change. Performance-domain support is a new explicit prerequisite. The test scope is Windows plus the stated Chromium version; simulated protocol faults do not establish real-engine fault behavior. No full repository suite, coverage, Linux/macOS, other browsers, multi-day runtime wait, or CPU/peak-memory benchmark was run. Remote CI is not claimed as passed by these local results.

Before publication, main was fetched at 17f581ff75d2b6cf643a997275e3e296f6c95760; the relevant owner, contracts, tests and contribution rules remain unchanged from the tested snapshot. No execution on that newer main is claimed. Independent AI review was source-only and identified a setup scope-listener race and test cleanup/barrier issues, all corrected before final validation; it does not replace human review.

Author readiness

Human owner: @Plastic-time

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/5639 **Author:** [@Plastic-time](https://github.com/Plastic-time) **Created:** 10/20/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/5609-lifecycle-window` --- ### 📝 Commits (1) - [`5cdd46c`](https://github.com/morluto/rea/commit/5cdd46c4f08ae20f35c50565d95b9c2bb42b1196) fix(browser): exclude replayed lifecycle milestones from session activity ### 📊 Changes **4 files changed** (+423 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `docs/browser-observation.md` (+6 -0) 📝 `src/browser/CdpObservationSession.ts` (+46 -1) ➕ `tests/boundary/browser/cdpObservationLifecycleWindow.test.ts` (+367 -0) 📝 `tests/fixtures/fakeCdpBrowserResults.ts` (+4 -0) </details> ### 📄 Description > **Status: superseded by upstream implementation.** > > Maintainer commit [2a0410f1](https://github.com/morluto/rea/commit/2a0410f1717b3126e8a95063d77dea66a597010a) addresses #5609 with a unified observation-window boundary, pre-arm buffering, and explicit unknown event times. This replaces this PR's independent lifecycle cutoff. The setup-time review finding on this branch is therefore not being patched onto the obsolete implementation; merging that logic back would conflict with the new upstream contract. > > Verification on upstream `23196daa` containing that fix, Windows, Node 24.18.0/npm 11.16.0: runtime build passed; 38 tests across the window buffer/scope, timeline/lifecycle-failure, and public CLI/stdio MCP acceptance suites passed. An owned headless Chrome fixture also passed: public CLI idle observation returned zero timeline rows; production-provider reload and history controls retained lifecycle and same-document navigation respectively. This was focused validation, not the full suite or a Linux/macOS claim. The PR branch was left unchanged; no merge or force-push was performed. > > AI-assisted follow-up verification. The original description below records the previous implementation and its historical evidence; it is not a claim that this branch is now compatible with current main. --- ## Summary Fixes #5609. Stop reporting replayed lifecycle milestones from an already-loaded page as new session activity. Preserve genuine milestones, including those emitted by the initial loader after observation starts. ## Problem and expected behavior On Windows, a real public CLI capture of an untouched loopback page returned six old lifecycle records despite zero fixture requests and unchanged NavigationStart. The pre-command browser Timestamp was `7743.787796`; the returned lifecycle timestamps were `7741.275806` through `7741.313858`. The CLI exited 0 with empty stderr and `window_elapsed`. An observation should exclude those historical milestones while retaining actual reload, history navigation, and new lifecycle activity. ## Change and scope The existing `CdpObservationSession` owner samples the target's monotonic clock with `Performance.enable/getMetrics/disable` while its scope/termination listeners are active, before enabling lifecycle collection. It filters older lifecycle timestamps, preserves equality and new milestones from any loader, and records malformed event times as incomplete coverage. Missing/invalid clocks prevent arming explicitly. This is an attach-time cutoff preceding `armed_at`, not a claim that all setup events before the armed notification are excluded. It adds three passive CDP requests and requires Performance-domain support. Ordinary correction within the existing capability boundary; no advance maintainer scope decision is required under the contribution rules. Deduplication: #5609 remained open and the latest direct issue-to-PR search returned no results. #5605 already fixes long-window timing; it is retained. Open #4391 addresses a timer input limit, not historical replay. Open #4040 overlaps this owner: its current head `5251c43522f9b2ea6e81c7ccb25938b3af239976` was checked after its refresh and still changes URL identity/scope, not historical lifecycle admission. This PR uses a separate regression file; integration with #4040 will still need review. These are targeted checks, not a guarantee that every possible duplicate has been found. ## Contract and boundary impact - Semantic owner and earliest changed stage: CDP session setup and `TimelineCapture` lifecycle admission. - Contribution scope: existing passive observation; no new execution, injection, mutation, or recovery authority. - CLI/MCP/tool-catalog: schemas unchanged; both consume the corrected owner result. - Provider compatibility: requires one finite, nonnegative browser Timestamp; unsupported/malformed clocks fail with a protocol error instead of silently accepting historical state. - Evidence: preserves timestamps, loader identity and ordering; invalid event times produce `invalid_protocol_value` coverage diagnostics. - Target/cleanup: same selected session and cancellation signal; Performance is disabled after sampling, and failures use existing owned-session transport cleanup. - Generated metadata/package/install: no dependency, package, binary, or tracked generated-file changes. Expected effects of the existing workflow and this correction: - [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. - [x] 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. REA attaches and sends passive protocol requests to the selected browser. The independent verifier owns its disposable browser and drives only its own loopback fixture. ## Evidence and regression coverage Original failure/failure-first base: `5b676ba9f302228b44c28ab076525abed3e647c1`. Final candidate was reapplied without conflicts and fully revalidated against `7d6f90485ee078834ce64656770a34947bb81803` (REA 6.7.0). Node 24.18.0/npm 11.16.0, Windows x64, playwright-core 1.58.2, Chrome for Testing 145.0.7632.6. | Real public CLI control | Historical lifecycle before / after | Requests before / after | Fixed behavior | | --- | --- | --- | --- | | Idle, 2000 ms | 6 / 0 | 0 / 0 | Empty timeline; unchanged NavigationStart | | Reload, 6000 ms | 5 / 0 | 1 / 1 | New load follows the independently sampled action time | | History, 6000 ms | 6 / 0 | 0 / 0 | Same-document navigation retained | A deliberately blocked initial loader was released at the production provider's actual armed handshake: its load timestamp `8764.469588` follows the independent pre-release sample `8764.467771` and retains the same loader ID. This control uses the real provider, not the public CLI. A separate public stdio MCP call retained history navigation triggered by its actual armed notification, with zero old lifecycle records. REA left the borrowed page open; the verifier then closed its MCP client, independent CDP session, browser and HTTP server. The initial four regressions failed on the unchanged base because `[99, 100, 101]` was returned instead of `[100, 101]`. Final tests cover late replay with acknowledged reply barriers, equal cutoff, initial-loader activity, both socket modes, frame/session isolation, malformed timestamps, all three new command failure/cancellation stages, sampling-time scope/termination, and post-terminal suppression. Existing long-window, armed/finalization cancellation, redirect, final-URL, application, CLI and MCP coverage is retained. - [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 On the final tested base: - `npm run test:focused -- tests/boundary/browser/cdpObservationLifecycleWindow.test.ts tests/boundary/browser/cdpBrowserProvider.timelineEvents.test.ts tests/boundary/browser/cdpObservationResourceTypes.test.ts tests/boundary/browser/cdpBrowserProvider.lifecycle_failures.test.ts tests/boundary/browser/cdpBrowserProvider.browserSocket.test.ts tests/boundary/cli/browserCli.test.ts tests/boundary/mcp/browserMcp.test.ts src/application/BrowserObservationService.test.ts src/domain/browserObservation.test.ts` 鈥?9 files / 89 tests passed, 27.54 s; prerequisite runtime build passed. - `npm run check` 鈥?passed, including typecheck, lint, format, knip, metadata, module and architecture guards. Lint reports 0 errors and 1967 warnings. - `npm run docs:check` 鈥?passed, including 5 README consistency tests. - `npm run verify:test-discovery` 鈥?all 1826 test files discovered once; this is not a full-suite execution. - `git diff --check` 鈥?passed. - Real Chromium public CLI idle/reload/history, initial-loader control and public stdio MCP handshake 鈥?all rerun successfully on the final build, as detailed above. The local harness/evidence remains preserved; this PR includes only the relevant source, tests and documentation, not personal paths, profiles, dependencies or binaries. ## Compatibility, safety, and release No schema migration or package change. Performance-domain support is a new explicit prerequisite. The test scope is Windows plus the stated Chromium version; simulated protocol faults do not establish real-engine fault behavior. No full repository suite, coverage, Linux/macOS, other browsers, multi-day runtime wait, or CPU/peak-memory benchmark was run. Remote CI is not claimed as passed by these local results. Before publication, main was fetched at `17f581ff75d2b6cf643a997275e3e296f6c95760`; the relevant owner, contracts, tests and contribution rules remain unchanged from the tested snapshot. No execution on that newer main is claimed. Independent AI review was source-only and identified a setup scope-listener race and test cleanup/barrier issues, all corrected before final validation; it does not replace human review. ## Author readiness Human owner: @Plastic-time - [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 - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] 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:44:39 +00:00