[PR #1579] [CLOSED] fix(browser): seed storage once per scenario storage area #2986

Closed
opened 2026-10-17 21:12:35 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1579
Author: @dakjdakd
Created: 10/10/2026
Status: ❌ Closed

Base: main ← Head: fix/browser-storage-seed-lifecycle


📝 Commits (1)

  • 1465ed8 fix(browser): seed storage once per scenario storage area

📊 Changes

7 files changed (+1014 additions, -44 deletions)

View changed files

📝 docs/browser-scenario-contract.md (+9 -0)
➕ scripts/lib/browser-scenario-storage-e2e.mjs (+454 -0)
📝 scripts/verify-real-browser.mjs (+12 -0)
📝 src/browser/BrowserScenarioPartialObservation.ts (+8 -1)
➕ src/browser/PlaywrightScenarioSession.test.ts (+55 -0)
📝 src/browser/PlaywrightScenarioSession.ts (+68 -43)
➕ src/browser/PlaywrightScenarioStorage.ts (+408 -0)

📄 Description

Summary

Fixes #1578.

Initialize browser-scenario web storage once per actual browser storage area, before application code runs, and preserve the application's subsequent state across navigation. A scenario that starts with state=initial, clicks a button writing updated, and navigates on the same origin now captures updated on the next document. Removal and empty strings are preserved as well.

The change moves initialization ownership out of a persistent page script and into a provider-owned Chromium session. This avoids adding hidden keys to application storage and handles a declared origin first reached through an out-of-process iframe or through a later top-level navigation. The same implementation serves launch and external-connect scenarios through the existing shared provider.

Problem and expected behavior

browserScenarioStorageSchema calls the supplied cookies and web-storage entries initial state. Before this change, cookies were added once, while local and session storage were written by page.addInitScript on every new document. The application could visibly update its state, then have that state reset by the observer before its next script executed.

The smallest executed reproduction is a local HTTP application with an Update button and a document-load read of both stores. The public CLI captures initial at scenario_start, updated after the click, and initial again after a same-origin goto. The ordinary Chrome control preserves updated. REA nevertheless reports completed steps and a complete, equality-eligible result, so the problem affects the fidelity of the captured execution rather than only the presentation of an artifact.

Checking for an absent key would not repair the ownership rule. Logout and other ordinary workflows intentionally remove entries, and "" is a valid stored value. The observer must remember that it has initialized the storage area without interpreting the current application value as an initialization flag. The first read also needs to see the supplied seed: applying state after navigation or during artifact collection is too late for startup code.

Change and scope

PlaywrightScenarioStorage owns one additional CDP connection for scenarios with declared web-storage blocks. It resolves seed values through the existing BrowserScenarioSecrets, combines entries for duplicate origin blocks, and keeps the initialized-area set in the provider process. No page-storage sentinel, window-name marker, synthetic navigation, or request interception is needed. Scenarios with no local/session storage blocks skip this owner entirely.

For each document, a short isolated-world bootstrap has a unique source URL. A native breakpoint targets only that bootstrap's executable line. When the bootstrap pauses, the provider identifies its execution context and frame, obtains Storage.getStorageKeyForFrame, and evaluates the initial assignments on that paused call frame. Application scripts proceed only after the provider resumes the bootstrap. Subsequent bootstrap executions for an initialized key resume without writing the stores.

The storage key is the browser's actual area identity. In an externally launched Chrome with ordinary third-party partitioning, the same origin inside a cross-site iframe and at the top level has different keys. Each area receives its own initial state once. In a Playwright launch where those areas are shared, the shared key is initialized once and later frames see the application's changes. Session storage remains within the selected root tab because the owner attaches only to that tab and its iframe target tree.

Out-of-process iframe targets are configured while their startup is paused, then resumed. Nested target sessions forward protocol replies through their parent session. Startup targets are resumed even when configuration fails, and detached targets reject outstanding replies for their complete descendant tree. Page, context, and browser disconnection also settle the manually tracked child replies.

The change is confined to seven files:

  • src/browser/PlaywrightScenarioStorage.ts owns native initialization, area identity, descendant sessions, and teardown.
  • src/browser/PlaywrightScenarioSession.ts acquires that owner, checks pending initialization failures within the existing execution boundaries, and releases it with the session. Storage cleanup runs independently of the browser owner's cached event finalization, so a failed storage close can be retried without repeating event finalization or browser closure. Startup cleanup retains the initialization promise and retry closure, including storage acquired after cancellation.
  • src/browser/PlaywrightScenarioSession.test.ts verifies that storage cleanup retries while event finalization and browser closure each run once.
  • src/browser/BrowserScenarioPartialObservation.ts adds the applicable instrumentation explanation to capture limitations.
  • scripts/lib/browser-scenario-storage-e2e.mjs contains the real-browser storage regressions and assertions.
  • scripts/verify-real-browser.mjs calls those regressions from the existing browser verifier.
  • docs/browser-scenario-contract.md documents initial-state lifetime, partition identity, and the debugging interaction.

Contract and boundary impact

  • Semantic owner and earliest changed stage: The Chromium scenario adapter owns initialization. The correction occurs at the document bootstrap, before application code reads or changes storage. Application workflows and Evidence projection continue to consume the existing session port.
  • CLI and MCP/tool-catalog contract: No request fields, action shapes, tool names, or catalog schemas change. Both adapters use the same provider implementation. The corrected application state appears in the existing DOM and storage artifacts. Capture limitations explain the native debugger interaction when seeds are selected.
  • Provider, bridge, target-format, or platform compatibility: This uses the existing Chromium CDP boundary and pinned Playwright dependency. It covers the selected page's iframe descendants, including out-of-process frames. Actual storage keys preserve the browser's partition decision rather than imposing an origin-only approximation. No new dependency or bridge is introduced.
  • Evidence, artifact, provenance, or reconstruction contract: Existing observed-state artifacts and their hashes remain the output surface. Initialization bookkeeping is kept outside page storage, so it does not create additional captured storage entries. The change does not reinterpret an application deletion, modify the existing secret-redaction policy, or repair results downstream after the target has executed incorrectly.
  • Process execution, authorization, cleanup, or containment impact: Launch still owns the temporary profile and browser process; connect still disconnects from an externally owned browser. The added CDP owner removes its bootstrap and breakpoint before disconnecting, resumes startup targets, and settles descendant replies on disconnection. Cancellation or failed initial navigation can stop the unfinished navigation so renderer-side teardown completes; ordinary connect cleanup does not stop the external page's network activity.
  • Generated metadata, package, or installation impact: None. No generated catalog, packaged binary, installation setting, lockfile, or release metadata changes are included.

There is a real debugging interaction worth calling out. Storage initialization briefly pauses the provider's isolated bootstrap on document startup. While its debugger connection is active, target debugger statements are resumed so a scenario does not become stuck on an unrelated pause. A simultaneous debugging session on an attached tab can therefore be affected. The docs and capture limitations state this explicitly. There is no global before-script instrumentation breakpoint, and scenarios without web-storage seeds do not enable this storage debugger owner.

Evidence and regression coverage

  • Tests added or updated: The new real-browser helper verifies first-script values, update/navigation persistence, removal, empty values, first visits to later origins, returns to previously initialized origins, and the absence of bookkeeping keys in storage Evidence. It also exercises multiple live OOPIFs, a separate simultaneous-navigation counter case, and failed/cancelled navigation cleanup. These functions are called by the normal real-browser verifier.
  • Base reproduction or other evidence: Executed public CLI and independent ordinary-browser control establish the reset. The affected session source was identical between the original reproduction revision 9ffc6210 and the initial contribution base c19e15a2. The updated base 603d04ee changes cleanup ownership but retains the same unconditional per-document storage assignment. The baseline CLI returned complete Evidence and successful owned-process cleanup while its post-navigation values reverted to the seeds.
  • User-visible CLI/MCP output: The public CLI's DOM observations and storage value fingerprints now describe the application's retained values. For the reduced Update scenario, both stores follow initial → updated → updated; removal follows initial → null → null, and the empty-string case remains empty on the next document. No new output object or storage key is introduced. The shared provider correction also serves MCP callers.
  • Remaining proof gaps: Executed regression coverage comprises focused Windows/Chrome launch and external-connect CLI workflows, plus provider-boundary cancellation because AbortSignal is an API argument. The broader verify:browser journey and a separate MCP storage execution were not run. The listed browser version defines the real-provider coverage for this change.

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

Verification used Windows, Chrome 154.0.8037.98, Node 24.18.0, npm 11.16.0, and Playwright 1.58.2. The fix was rebased onto 603d04ee3ac6ed496f84478164f832a9b5ce3b91, with startup and session teardown adapted to the current cleanup owner. Build, static checks, documentation, test discovery, and all focused real-browser workflows below were rerun on the updated commit.

  • npm run build:cached — passed.
  • npm run check — passed, including typecheck, lint, formatting, knip, metadata checks, resolved module boundaries, and architecture guards.
  • npm run docs:check — passed.
  • npm run test:local -- src/browser/PlaywrightScenarioBrowser.test.ts src/browser/PlaywrightBrowserScenarioProvider.test.ts src/browser/PlaywrightExecutionBoundary.test.ts src/browser/PlaywrightScenarioArtifacts.test.ts src/browser/PlaywrightScenarioEvents.test.ts src/browser/PlaywrightScenarioSession.test.ts — 28 tests passed, including the new independent storage-cleanup retry regression. The existing assertion keeps failed event finalization failed across cleanup retries also fails on unmodified base 603d04ee; that upstream event-finalization behavior is outside this storage correction.
  • npm run verify:test-discovery — all 766 test files discovered exactly once.
  • Direct calls to verifyScenarioStorage and verifyScenarioStorageFrames against a real launch-mode browser — passed. They check actual public CLI artifacts and confirm temporary-profile cleanup.
  • The same functions against an independently launched, externally owned Chrome through connect — passed. Ordinary browser partitioning creates distinct iframe/top-level areas, each of which is initialized correctly. After capture, a native update/reload and deletion/reload retain their results, and the external target remains open.
  • verifyScenarioStorageFailures against the real provider — passed. A failed navigation retains its failed step with confirmed owned-browser cleanup. Cancelling an unfinished initial navigation returns AnalysisCancelledError with cleanupIncomplete: false.

The simultaneous-frame regression creates four iframe navigations together. Their first scripts read and increment a shared seeded counter; the observed values, sorted independently of arrival order, are exactly 0, 1, 2, 3 in both stores. This catches reseeding that would be hidden by testing only sequential navigation. The live-frame regression separately covers update, delete, and empty-string transitions before a top-level partition change and a return to the original iframe area.

Compatibility, safety, and release

  • Breaking changes or migration steps: None. Existing scenario inputs remain valid. Callers receive the initial-state lifetime already described by the storage contract.
  • Real browser or OS coverage: Real launch and external-connect Chrome on Windows, including OOPIFs and browser-default storage partitioning, as detailed above.
  • Package or release metadata impact: None; this is a focused browser behavior correction.
  • Security, privacy, process, or containment review: Only caller-declared seed values are assigned. Bookkeeping remains provider-owned. External process ownership is preserved, native instrumentation is removed during cleanup, and cancellation settles owned resources. The documented debugger interaction should be considered when capturing an attached tab that is also being debugged interactively.

Review checklist

  • This PR addresses a concrete problem.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • The related issue is linked above.
  • Regressions cover observable behavior and meaningful failure paths.
  • Owning documentation and capture limitations are updated.
  • Representative public CLI behavior is described above.
  • The final diff was checked 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/1579 **Author:** [@dakjdakd](https://github.com/dakjdakd) **Created:** 10/10/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/browser-storage-seed-lifecycle` --- ### 📝 Commits (1) - [`1465ed8`](https://github.com/morluto/rea/commit/1465ed8c4fdf4cda657dc79378a23f8c81400bda) fix(browser): seed storage once per scenario storage area ### 📊 Changes **7 files changed** (+1014 additions, -44 deletions) <details> <summary>View changed files</summary> 📝 `docs/browser-scenario-contract.md` (+9 -0) ➕ `scripts/lib/browser-scenario-storage-e2e.mjs` (+454 -0) 📝 `scripts/verify-real-browser.mjs` (+12 -0) 📝 `src/browser/BrowserScenarioPartialObservation.ts` (+8 -1) ➕ `src/browser/PlaywrightScenarioSession.test.ts` (+55 -0) 📝 `src/browser/PlaywrightScenarioSession.ts` (+68 -43) ➕ `src/browser/PlaywrightScenarioStorage.ts` (+408 -0) </details> ### 📄 Description ## Summary Fixes #1578. Initialize browser-scenario web storage once per actual browser storage area, before application code runs, and preserve the application's subsequent state across navigation. A scenario that starts with `state=initial`, clicks a button writing `updated`, and navigates on the same origin now captures `updated` on the next document. Removal and empty strings are preserved as well. The change moves initialization ownership out of a persistent page script and into a provider-owned Chromium session. This avoids adding hidden keys to application storage and handles a declared origin first reached through an out-of-process iframe or through a later top-level navigation. The same implementation serves launch and external-connect scenarios through the existing shared provider. ## Problem and expected behavior `browserScenarioStorageSchema` calls the supplied cookies and web-storage entries **initial** state. Before this change, cookies were added once, while local and session storage were written by `page.addInitScript` on every new document. The application could visibly update its state, then have that state reset by the observer before its next script executed. The smallest executed reproduction is a local HTTP application with an Update button and a document-load read of both stores. The public CLI captures `initial` at `scenario_start`, `updated` after the click, and `initial` again after a same-origin `goto`. The ordinary Chrome control preserves `updated`. REA nevertheless reports completed steps and a complete, equality-eligible result, so the problem affects the fidelity of the captured execution rather than only the presentation of an artifact. Checking for an absent key would not repair the ownership rule. Logout and other ordinary workflows intentionally remove entries, and `""` is a valid stored value. The observer must remember that it has initialized the storage area without interpreting the current application value as an initialization flag. The first read also needs to see the supplied seed: applying state after navigation or during artifact collection is too late for startup code. ## Change and scope `PlaywrightScenarioStorage` owns one additional CDP connection for scenarios with declared web-storage blocks. It resolves seed values through the existing `BrowserScenarioSecrets`, combines entries for duplicate origin blocks, and keeps the initialized-area set in the provider process. No page-storage sentinel, window-name marker, synthetic navigation, or request interception is needed. Scenarios with no local/session storage blocks skip this owner entirely. For each document, a short isolated-world bootstrap has a unique source URL. A native breakpoint targets only that bootstrap's executable line. When the bootstrap pauses, the provider identifies its execution context and frame, obtains `Storage.getStorageKeyForFrame`, and evaluates the initial assignments on that paused call frame. Application scripts proceed only after the provider resumes the bootstrap. Subsequent bootstrap executions for an initialized key resume without writing the stores. The storage key is the browser's actual area identity. In an externally launched Chrome with ordinary third-party partitioning, the same origin inside a cross-site iframe and at the top level has different keys. Each area receives its own initial state once. In a Playwright launch where those areas are shared, the shared key is initialized once and later frames see the application's changes. Session storage remains within the selected root tab because the owner attaches only to that tab and its iframe target tree. Out-of-process iframe targets are configured while their startup is paused, then resumed. Nested target sessions forward protocol replies through their parent session. Startup targets are resumed even when configuration fails, and detached targets reject outstanding replies for their complete descendant tree. Page, context, and browser disconnection also settle the manually tracked child replies. The change is confined to seven files: - `src/browser/PlaywrightScenarioStorage.ts` owns native initialization, area identity, descendant sessions, and teardown. - `src/browser/PlaywrightScenarioSession.ts` acquires that owner, checks pending initialization failures within the existing execution boundaries, and releases it with the session. Storage cleanup runs independently of the browser owner's cached event finalization, so a failed storage close can be retried without repeating event finalization or browser closure. Startup cleanup retains the initialization promise and retry closure, including storage acquired after cancellation. - `src/browser/PlaywrightScenarioSession.test.ts` verifies that storage cleanup retries while event finalization and browser closure each run once. - `src/browser/BrowserScenarioPartialObservation.ts` adds the applicable instrumentation explanation to capture limitations. - `scripts/lib/browser-scenario-storage-e2e.mjs` contains the real-browser storage regressions and assertions. - `scripts/verify-real-browser.mjs` calls those regressions from the existing browser verifier. - `docs/browser-scenario-contract.md` documents initial-state lifetime, partition identity, and the debugging interaction. ## Contract and boundary impact - **Semantic owner and earliest changed stage:** The Chromium scenario adapter owns initialization. The correction occurs at the document bootstrap, before application code reads or changes storage. Application workflows and Evidence projection continue to consume the existing session port. - **CLI and MCP/tool-catalog contract:** No request fields, action shapes, tool names, or catalog schemas change. Both adapters use the same provider implementation. The corrected application state appears in the existing DOM and storage artifacts. Capture limitations explain the native debugger interaction when seeds are selected. - **Provider, bridge, target-format, or platform compatibility:** This uses the existing Chromium CDP boundary and pinned Playwright dependency. It covers the selected page's iframe descendants, including out-of-process frames. Actual storage keys preserve the browser's partition decision rather than imposing an origin-only approximation. No new dependency or bridge is introduced. - **Evidence, artifact, provenance, or reconstruction contract:** Existing observed-state artifacts and their hashes remain the output surface. Initialization bookkeeping is kept outside page storage, so it does not create additional captured storage entries. The change does not reinterpret an application deletion, modify the existing secret-redaction policy, or repair results downstream after the target has executed incorrectly. - **Process execution, authorization, cleanup, or containment impact:** Launch still owns the temporary profile and browser process; connect still disconnects from an externally owned browser. The added CDP owner removes its bootstrap and breakpoint before disconnecting, resumes startup targets, and settles descendant replies on disconnection. Cancellation or failed initial navigation can stop the unfinished navigation so renderer-side teardown completes; ordinary connect cleanup does not stop the external page's network activity. - **Generated metadata, package, or installation impact:** None. No generated catalog, packaged binary, installation setting, lockfile, or release metadata changes are included. There is a real debugging interaction worth calling out. Storage initialization briefly pauses the provider's isolated bootstrap on document startup. While its debugger connection is active, target debugger statements are resumed so a scenario does not become stuck on an unrelated pause. A simultaneous debugging session on an attached tab can therefore be affected. The docs and capture limitations state this explicitly. There is no global before-script instrumentation breakpoint, and scenarios without web-storage seeds do not enable this storage debugger owner. ## Evidence and regression coverage - **Tests added or updated:** The new real-browser helper verifies first-script values, update/navigation persistence, removal, empty values, first visits to later origins, returns to previously initialized origins, and the absence of bookkeeping keys in storage Evidence. It also exercises multiple live OOPIFs, a separate simultaneous-navigation counter case, and failed/cancelled navigation cleanup. These functions are called by the normal real-browser verifier. - **Base reproduction or other evidence:** Executed public CLI and independent ordinary-browser control establish the reset. The affected session source was identical between the original reproduction revision `9ffc6210` and the initial contribution base `c19e15a2`. The updated base `603d04ee` changes cleanup ownership but retains the same unconditional per-document storage assignment. The baseline CLI returned complete Evidence and successful owned-process cleanup while its post-navigation values reverted to the seeds. - **User-visible CLI/MCP output:** The public CLI's DOM observations and storage value fingerprints now describe the application's retained values. For the reduced Update scenario, both stores follow `initial → updated → updated`; removal follows `initial → null → null`, and the empty-string case remains empty on the next document. No new output object or storage key is introduced. The shared provider correction also serves MCP callers. - **Remaining proof gaps:** Executed regression coverage comprises focused Windows/Chrome launch and external-connect CLI workflows, plus provider-boundary cancellation because `AbortSignal` is an API argument. The broader `verify:browser` journey and a separate MCP storage execution were not run. The listed browser version defines the real-provider coverage for this change. 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 Verification used Windows, Chrome `154.0.8037.98`, Node `24.18.0`, npm `11.16.0`, and Playwright `1.58.2`. The fix was rebased onto `603d04ee3ac6ed496f84478164f832a9b5ce3b91`, with startup and session teardown adapted to the current cleanup owner. Build, static checks, documentation, test discovery, and all focused real-browser workflows below were rerun on the updated commit. - `npm run build:cached` — passed. - `npm run check` — passed, including typecheck, lint, formatting, knip, metadata checks, resolved module boundaries, and architecture guards. - `npm run docs:check` — passed. - `npm run test:local -- src/browser/PlaywrightScenarioBrowser.test.ts src/browser/PlaywrightBrowserScenarioProvider.test.ts src/browser/PlaywrightExecutionBoundary.test.ts src/browser/PlaywrightScenarioArtifacts.test.ts src/browser/PlaywrightScenarioEvents.test.ts src/browser/PlaywrightScenarioSession.test.ts` — 28 tests passed, including the new independent storage-cleanup retry regression. The existing assertion `keeps failed event finalization failed across cleanup retries` also fails on unmodified base `603d04ee`; that upstream event-finalization behavior is outside this storage correction. - `npm run verify:test-discovery` — all 766 test files discovered exactly once. - Direct calls to `verifyScenarioStorage` and `verifyScenarioStorageFrames` against a real launch-mode browser — passed. They check actual public CLI artifacts and confirm temporary-profile cleanup. - The same functions against an independently launched, externally owned Chrome through `connect` — passed. Ordinary browser partitioning creates distinct iframe/top-level areas, each of which is initialized correctly. After capture, a native update/reload and deletion/reload retain their results, and the external target remains open. - `verifyScenarioStorageFailures` against the real provider — passed. A failed navigation retains its failed step with confirmed owned-browser cleanup. Cancelling an unfinished initial navigation returns `AnalysisCancelledError` with `cleanupIncomplete: false`. The simultaneous-frame regression creates four iframe navigations together. Their first scripts read and increment a shared seeded counter; the observed values, sorted independently of arrival order, are exactly `0, 1, 2, 3` in both stores. This catches reseeding that would be hidden by testing only sequential navigation. The live-frame regression separately covers update, delete, and empty-string transitions before a top-level partition change and a return to the original iframe area. ## Compatibility, safety, and release - **Breaking changes or migration steps:** None. Existing scenario inputs remain valid. Callers receive the initial-state lifetime already described by the storage contract. - **Real browser or OS coverage:** Real launch and external-connect Chrome on Windows, including OOPIFs and browser-default storage partitioning, as detailed above. - **Package or release metadata impact:** None; this is a focused browser behavior correction. - **Security, privacy, process, or containment review:** Only caller-declared seed values are assigned. Bookkeeping remains provider-owned. External process ownership is preserved, native instrumentation is removed during cleanup, and cancellation settles owned resources. The documented debugger interaction should be considered when capturing an attached tab that is also being debugged interactively. ## Review checklist - [x] This PR addresses a concrete problem. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] The related issue is linked above. - [x] Regressions cover observable behavior and meaningful failure paths. - [x] Owning documentation and capture limitations are updated. - [x] Representative public CLI behavior is described above. - [x] The final diff was checked 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-10-17 21:12:35 +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#2986
No description provided.