morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #6341] [MERGED] fix(browser)!: preserve storage value code units #7290

New issue

Closed

opened 2026-11-01 21:46:48 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:46:48 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6341
Author: @gongfpp
Created: 10/22/2026
Status: ✅ Merged
Merged: 10/22/2026
Merged by: @morluto

Base: main ← Head: fix/browser-storage-surrogates-6289

📝 Commits (1)

8d46743 fix(browser)!: preserve storage value code units

📊 Changes

10 files changed (+517 additions, -4 deletions)

View changed files

📝 docs/browser-scenario-contract.md (+18 -0)
➕ scripts/lib/browser-scenario-storage-fingerprints-e2e.mjs (+227 -0)
📝 scripts/verify-real-browser.mjs (+7 -0)
📝 src/browser/BrowserScenario.test.ts (+56 -0)
📝 src/browser/BrowserScenarioSecrets.ts (+6 -4)
📝 src/browser/PlaywrightScenarioArtifacts.ts (+2 -0)
📝 src/domain/browserScenarioCaptureValues.ts (+7 -0)
📝 src/domain/browserScenarioDiff.test.ts (+152 -0)
📝 src/domain/browserScenarioDiff.ts (+7 -0)
📝 src/domain/browserScenarioDiffEvidence.ts (+35 -0)

📄 Description

Summary

Fixes #6289. Preserve JavaScript storage value identity when strings contain unpaired UTF-16 surrogates; do not turn irreversible legacy information loss or encoding metadata into a website change.

Problem and expected behavior

On base 3d472bde23b70f9aa7bcf0d28a0cd10e469545e9, real owned Chromium plus public CLI capture/comparison returned unchanged for separately captured \ud800 and \ud801 local/session storage. Both values had SHA-256 83d544ccc223c057d2bf80d3f2a32982c32c3c0db8e2674820da5064783fb097: UTF-8 replaced the distinct code units before hashing.

New captures report these pairs, and \ud800 versus \ufffd, as changed. Independently recaptured identical values remain unchanged.

Change and scope

Fix the existing BrowserScenarioSecrets.fingerprint owner: ordinary strings keep their existing UTF-8 digest; unpaired code units use raw 0xff followed by exact UTF-16LE. The byte domains are disjoint. Declared secrets still return redacted-secret / null before hashing; no extra secret or legacy digest.
Successful storage snapshots carry optional-contract fingerprint_version: 2. Absence means legacy, not v2. The comparison boundary checks provenance before normalization and excludes the version from content projection.
Any legacy hashed storage conservatively yields unknown, even old-to-old and when a difference could otherwise be proven. This intentionally trades legacy comparison coverage for soundness; recapture both sides. Empty/all-secret-redacted storage retains its existing semantics. No legacy Evidence is rewritten. This compatibility policy remains subject to maintainer review.
Ordinary fix within existing active-scenario boundaries: maintainer execution-scope decision not applicable. No new provider, target authority, passive capture behavior, secret inference or storage ordering fix (#5035 is separate).

Contract and boundary impact

Semantic owner: storage value fingerprint producer; active-scenario storage comparison owns comparability.
CLI/MCP: same operations and arguments; additive optional result field and corresponding generated catalog schema; legacy inputs remain accepted with explicit uncertainty.
Evidence: preserve capture identity, provenance, completeness and declared-secret redaction; retained MCP records remain available intact.
Existing active capture launches the caller-selected browser, seeds selected storage and evaluates its capture scripts. The regression exercises only an owned loopback fixture; no new runtime authority or external service.
No dependency, installation, package version or committed generated-output changes.

Evidence and regression coverage

Producer cases cover ordinary UTF-8, paired/unpaired units, an exact v2 byte golden, declared-secret redaction and a cross-encoding alias that defeats bare UTF-16LE fallback. Comparison cases cover all three storage collections, both legacy sides, different legacy digests, normalization order, and metadata-only changes.

The real-browser helper runs four independent captures through each public adapter (8 total), checks changed/unchanged controls, ordinary cookie/local/session digests, secret redaction, version-less imported results as unknown, CLI/MCP parity, full retained MCP records, server liveness and owned-profile cleanup. It is wired into the existing real-browser verifier.

Validation performed

On macOS arm64, pinned Node 24.18.0 / npm 11.16.0; base 2127095e3b4a377abbc124ca83af46b98a2a557b, tested head 8d46743eb64167eba7a0062a754d18ffb9316e8e:

npm run test:focused -- src/browser/BrowserScenario.test.ts src/browser/PlaywrightScenarioEvents.test.ts src/browser/PlaywrightScenarioArtifacts.test.ts src/browser/PlaywrightScenarioStorage.test.ts src/domain/browserScenarioCapture.test.ts src/domain/browserScenarioDiff.test.ts tests/boundary/mcp/browserScenarioMcp.test.ts — 86 tests passed.
Targeted verifyScenarioStorageFingerprints with the existing owned loopback verifier fixture and installed Chromium 155.0.8059.85 — passed: 8 independent captures, both adapters, imported version-less comparisons, retention and cleanup; 49.421 s including loopback-site setup/teardown (not a before/after performance benchmark).
npm run check:fast — typecheck/lint passed.
npm run docs:check — generated facts/catalog checks and 9 conformance tests passed.
npm run verify:test-discovery — all 2051 test files discovered exactly once.
git diff --check — passed. Independent read-only review found no blocking issue; its suggested codec/legacy regressions are included.

Compatibility, safety, and release

Breaking comparison behavior: previously accepted legacy captures still parse, but hashed legacy storage comparisons now return unknown; recapture both sides for sound value comparison. Already lost code units cannot be reconstructed. New ordinary-string digests remain compatible, without inferring that an unversioned capture was lossless.

Not run: the complete real-browser verifier, full test/CI suite, Windows/Linux, attached/passive-browser workflows or other providers. No performance claim; token usage, CPU and peak process-family memory were not measured. Local validation does not establish CI success, maintainer approval or merge permission.

Author readiness

Human owner: @gongfpp

AI-assisted contribution; the human owner confirmed review and requested initial maintainer review.

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

Human ownership/readiness and required current-head CI checks are complete.
Maintainer review confirms the conservative legacy comparison policy and contribution scope.

_{🔄 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/6341 **Author:** [@gongfpp](https://github.com/gongfpp) **Created:** 10/22/2026 **Status:** ✅ Merged **Merged:** 10/22/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/browser-storage-surrogates-6289` --- ### 📝 Commits (1) - [`8d46743`](https://github.com/morluto/rea/commit/8d46743eb64167eba7a0062a754d18ffb9316e8e) fix(browser)!: preserve storage value code units ### 📊 Changes **10 files changed** (+517 additions, -4 deletions) <details> <summary>View changed files</summary> 📝 `docs/browser-scenario-contract.md` (+18 -0) ➕ `scripts/lib/browser-scenario-storage-fingerprints-e2e.mjs` (+227 -0) 📝 `scripts/verify-real-browser.mjs` (+7 -0) 📝 `src/browser/BrowserScenario.test.ts` (+56 -0) 📝 `src/browser/BrowserScenarioSecrets.ts` (+6 -4) 📝 `src/browser/PlaywrightScenarioArtifacts.ts` (+2 -0) 📝 `src/domain/browserScenarioCaptureValues.ts` (+7 -0) 📝 `src/domain/browserScenarioDiff.test.ts` (+152 -0) 📝 `src/domain/browserScenarioDiff.ts` (+7 -0) 📝 `src/domain/browserScenarioDiffEvidence.ts` (+35 -0) </details> ### 📄 Description ## Summary Fixes #6289. Preserve JavaScript storage value identity when strings contain unpaired UTF-16 surrogates; do not turn irreversible legacy information loss or encoding metadata into a website change. ## Problem and expected behavior On base `3d472bde23b70f9aa7bcf0d28a0cd10e469545e9`, real owned Chromium plus public CLI capture/comparison returned `unchanged` for separately captured `\ud800` and `\ud801` local/session storage. Both values had SHA-256 `83d544ccc223c057d2bf80d3f2a32982c32c3c0db8e2674820da5064783fb097`: UTF-8 replaced the distinct code units before hashing. New captures report these pairs, and `\ud800` versus `\ufffd`, as `changed`. Independently recaptured identical values remain `unchanged`. ## Change and scope - Fix the existing `BrowserScenarioSecrets.fingerprint` owner: ordinary strings keep their existing UTF-8 digest; unpaired code units use raw `0xff` followed by exact UTF-16LE. The byte domains are disjoint. Declared secrets still return `redacted-secret` / null before hashing; no extra secret or legacy digest. - Successful storage snapshots carry optional-contract `fingerprint_version: 2`. Absence means legacy, not v2. The comparison boundary checks provenance before normalization and excludes the version from content projection. - Any legacy hashed storage conservatively yields `unknown`, even old-to-old and when a difference could otherwise be proven. This intentionally trades legacy comparison coverage for soundness; recapture both sides. Empty/all-secret-redacted storage retains its existing semantics. No legacy Evidence is rewritten. This compatibility policy remains subject to maintainer review. - Ordinary fix within existing active-scenario boundaries: maintainer execution-scope decision **not applicable**. No new provider, target authority, passive capture behavior, secret inference or storage ordering fix (#5035 is separate). ## Contract and boundary impact - Semantic owner: storage value fingerprint producer; active-scenario storage comparison owns comparability. - CLI/MCP: same operations and arguments; additive optional result field and corresponding generated catalog schema; legacy inputs remain accepted with explicit uncertainty. - Evidence: preserve capture identity, provenance, completeness and declared-secret redaction; retained MCP records remain available intact. - Existing active capture launches the caller-selected browser, seeds selected storage and evaluates its capture scripts. The regression exercises only an owned loopback fixture; no new runtime authority or external service. - No dependency, installation, package version or committed generated-output changes. ## Evidence and regression coverage Producer cases cover ordinary UTF-8, paired/unpaired units, an exact v2 byte golden, declared-secret redaction and a cross-encoding alias that defeats bare UTF-16LE fallback. Comparison cases cover all three storage collections, both legacy sides, different legacy digests, normalization order, and metadata-only changes. The real-browser helper runs four independent captures through each public adapter (8 total), checks changed/unchanged controls, ordinary cookie/local/session digests, secret redaction, version-less imported results as `unknown`, CLI/MCP parity, full retained MCP records, server liveness and owned-profile cleanup. It is wired into the existing real-browser verifier. ## Validation performed On macOS arm64, pinned Node 24.18.0 / npm 11.16.0; base `2127095e3b4a377abbc124ca83af46b98a2a557b`, tested head `8d46743eb64167eba7a0062a754d18ffb9316e8e`: - `npm run test:focused -- src/browser/BrowserScenario.test.ts src/browser/PlaywrightScenarioEvents.test.ts src/browser/PlaywrightScenarioArtifacts.test.ts src/browser/PlaywrightScenarioStorage.test.ts src/domain/browserScenarioCapture.test.ts src/domain/browserScenarioDiff.test.ts tests/boundary/mcp/browserScenarioMcp.test.ts` — 86 tests passed. - Targeted `verifyScenarioStorageFingerprints` with the existing owned loopback verifier fixture and installed Chromium 155.0.8059.85 — passed: 8 independent captures, both adapters, imported version-less comparisons, retention and cleanup; 49.421 s including loopback-site setup/teardown (not a before/after performance benchmark). - `npm run check:fast` — typecheck/lint passed. - `npm run docs:check` — generated facts/catalog checks and 9 conformance tests passed. - `npm run verify:test-discovery` — all 2051 test files discovered exactly once. - `git diff --check` — passed. Independent read-only review found no blocking issue; its suggested codec/legacy regressions are included. ## Compatibility, safety, and release **Breaking comparison behavior:** previously accepted legacy captures still parse, but hashed legacy storage comparisons now return `unknown`; recapture both sides for sound value comparison. Already lost code units cannot be reconstructed. New ordinary-string digests remain compatible, without inferring that an unversioned capture was lossless. Not run: the complete real-browser verifier, full test/CI suite, Windows/Linux, attached/passive-browser workflows or other providers. No performance claim; token usage, CPU and peak process-family memory were not measured. Local validation does not establish CI success, maintainer approval or merge permission. ## Author readiness Human owner: @gongfpp AI-assisted contribution; the human owner confirmed review and requested initial maintainer review. - [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 - [ ] Human ownership/readiness and required current-head CI checks are complete. - [ ] Maintainer review confirms the conservative legacy comparison policy and contribution scope. --- <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:48 +00:00

closed this issue
added the
pull-request
label

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#7290

No description provided.

Rows
Columns