morluto/rea

Fork 0

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

[PR #2637] [CLOSED] fix(browser): compare concurrent scenario network events by their request step, not receipt order #5297

New issue

Closed

opened 2026-10-31 01:05:32 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:05:32 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2637
Author: @jayhemnani9910
Created: 10/13/2026
Status: ❌ Closed

Base: main ← Head: fix/scenario-network-event-order

📝 Commits (8)

17bb551 fix(browser): compare concurrent scenario network events by their request step, not receipt order
312b694 fix(browser): commit network normalization and realistic event ordering
7ee0a73 fix(browser): retain redirect predecessor outcomes in comparisons
ac32ebf fix(browser): preserve full redirect ancestry in scenario comparisons
5fc7ce9 fix(browser): preserve redirect roots and reuse network fingerprints
7a459b2 fix(browser): preserve partial event comparison semantics
182f400 fix(browser): preserve scenario event uncertainty
a524b32 fix(browser): reject root ancestry contradictions

📊 Changes

13 files changed (+947 additions, -150 deletions)

View changed files

📝 docs/browser-scenario-contract.md (+30 -5)
📝 src/browser/PlaywrightBrowserScenarioProvider.test.ts (+1 -0)
📝 src/browser/network/PlaywrightScenarioNetwork.test.ts (+3 -0)
📝 src/browser/network/PlaywrightScenarioNetwork.ts (+14 -3)
📝 src/domain/browserScenarioCapture.test.ts (+8 -0)
📝 src/domain/browserScenarioCaptureValues.ts (+106 -90)
📝 src/domain/browserScenarioDiff.test.ts (+298 -0)
📝 src/domain/browserScenarioDiff.ts (+171 -11)
📝 src/domain/browserScenarioDiffEvidence.ts (+62 -19)
📝 src/domain/browserScenarioDiffValues.ts (+12 -3)
➕ src/domain/browserScenarioNetworkComparison.ts (+222 -0)
📝 src/domain/browserScenarioNormalization.ts (+19 -19)
📝 tests/fixtures/webScriptCapture.ts (+1 -0)

📄 Description

Summary

Two capture-browser-scenario runs of the same static page, with capture.events: ["network"], compare as changed in compare-web-captures. The page is a local file with a stylesheet, four images and three parallel fetch() calls; nothing on it changes between runs:

compare-web-captures run1.json run2.json
overall_status: changed
  scenario_start: events changed
  settle:         events changed

Every pair I tried did this (16 of 16 on 6.4.0 and main), so a network-selected scenario comparison cannot report unchanged for an ordinary page.

No related issue; the fix is small and stays inside the existing comparison contract, so it comes straight as a PR.

Problem and expected behavior

eventsFor (src/domain/browserScenarioDiffEvidence.ts:97-117) returns a step's events in receipt order and keeps transaction_id / redirected_from_transaction_id. The built-in rules drop only sequence and step_index. Two things then differ between identical runs:

Concurrent responses and request-finished events arrive in a different order each run, and request-N IDs follow request start order.
A response that arrives just after a step ends is attributed to the next step, so the same transaction is split across steps differently each run (scenario_start vs settle above).

Expected: the same requests with the same outcomes compare unchanged; a different status, header set, body or URL still compares changed. The passive comparison already works this way (webCaptureDiff.ts compares requests as sorted digests per identity), and content events are already attributed to their source event's step.

Change and scope

Network events (request, response, request-failed, request-finished, request-unfinished, network-content) are grouped per transaction and attributed to the step that sent the request (receipt step when the request was not observed). Content follows the same rule.
Capture-local transaction IDs are dropped; a redirect keeps its predecessor's method and URL instead of its ID.
Each transaction's own fields (method, URL, status, header names, failure, captured headers and body, redirect source) are still compared exactly; only the order between transactions and the capture-local IDs are dropped. Transactions are compared as an unordered collection. The ordering runs after caller literal rules, through a new optional order argument to digestNormalizedScenarioValue, so rules still apply to every field.
Console, page errors, frames, workers, popups, WebSocket frames and downloads keep receipt order and receipt step.
The comparison limitation text and docs/browser-scenario-contract.md describe the new rule.

Not included: ordering for non-network events. Console output from concurrently loaded scripts can still reorder; that is page behavior, not capture noise.

Follow the contribution scope: not applicable, a fix within the existing comparison.

Contract and boundary impact

Semantic owner and earliest changed stage: browser scenario comparison, event projection (browserScenarioDiffEvidence.ts).
Contribution scope fit and linked maintainer decision: routine fix; no decision needed.
CLI and MCP/tool-catalog contract: same result schema; one limitation string reworded.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: event digests in scenario diffs change for network-selected captures; capture Evidence is unchanged.
Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable.
Generated metadata (docs/public/product-catalog.json), package, or installation impact: none.
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: compares concurrent network transactions without receipt order, receipt step or capture-local IDs in src/domain/browserScenarioDiff.test.ts. Two transactions, the second redirected from the first, captured in opposite orders with the responses in different steps: unchanged. The same with the two statuses swapped: changed. It fails on main; reverting only the step attribution or only the redirect mapping also fails it.
Base reproduction or other evidence: executed on Linux x64, Chromium 153, local page above served on 127.0.0.1, scenario goto + wait_for_timeout 1500, network events only. Four captures, capture 1 compared with 2, 3, 4:
```
6.4.0:         changed, changed, changed
main 3ae826ce: changed, changed, changed
this branch:   unchanged, unchanged, unchanged
this branch, d2.json removed from the page: changed (scenario_start events)
```
A second page whose server delays each script 0-200 ms at random also compares unchanged on this branch.
User-visible CLI/MCP output: the overall_status lines above.
Remaining proof gaps: MCP not exercised separately (same shared workflow).

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

npx vitest run --project domain src/domain/browserScenarioDiff.test.ts — 11 passed; the new case fails on 3ae826ce without the change.
npm run check:fast — passed.
npx vitest run --project <name> for each project except acceptance, after npm run build:cached, rebased on main 3ae826ce — 10045 passed, 64 failed, 86 skipped, against 10044 / 64 / 86 on untouched 3ae826ce with an identical failure list (environment: firmware tests that need Binwalk 3.1.x, ASAR tests whose unpacked companion file is missing here, Ghidra provider tests). acceptance was not run (out of memory on this machine).

Compatibility, safety, and release

Breaking changes or migration steps: none. Stored diffs keep their meaning; new comparisons of network-selected captures can now return unchanged where they returned changed.
Real Hopper/Ghidra, browser, or OS coverage: Linux x64 with Playwright Chromium 153.
Package or release metadata impact: none.
Security, privacy, process, or containment review: none; comparison of already-redacted capture data only.

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/2637 **Author:** [@jayhemnani9910](https://github.com/jayhemnani9910) **Created:** 10/13/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/scenario-network-event-order` --- ### 📝 Commits (8) - [`17bb551`](https://github.com/morluto/rea/commit/17bb55160235bf710e3e3df056e6a05df6d80c30) fix(browser): compare concurrent scenario network events by their request step, not receipt order - [`312b694`](https://github.com/morluto/rea/commit/312b69456d8e3bde0667de99183c6458aaeb7485) fix(browser): commit network normalization and realistic event ordering - [`7ee0a73`](https://github.com/morluto/rea/commit/7ee0a73a2a3eb70e95922644b5aaa20228bb5981) fix(browser): retain redirect predecessor outcomes in comparisons - [`ac32ebf`](https://github.com/morluto/rea/commit/ac32ebfaa80c8b025453ff17a31816b6a7de5152) fix(browser): preserve full redirect ancestry in scenario comparisons - [`5fc7ce9`](https://github.com/morluto/rea/commit/5fc7ce95216ea6872bdf1a6477fbe6554d1cd239) fix(browser): preserve redirect roots and reuse network fingerprints - [`7a459b2`](https://github.com/morluto/rea/commit/7a459b2cb145e1cf82567cf2510d5c467725dbae) fix(browser): preserve partial event comparison semantics - [`182f400`](https://github.com/morluto/rea/commit/182f400c06dc8b65d2ddf3919a936acfb40abf67) fix(browser): preserve scenario event uncertainty - [`a524b32`](https://github.com/morluto/rea/commit/a524b32aedf1020d8e16acf9aaf567a94bc6d4ea) fix(browser): reject root ancestry contradictions ### 📊 Changes **13 files changed** (+947 additions, -150 deletions) <details> <summary>View changed files</summary> 📝 `docs/browser-scenario-contract.md` (+30 -5) 📝 `src/browser/PlaywrightBrowserScenarioProvider.test.ts` (+1 -0) 📝 `src/browser/network/PlaywrightScenarioNetwork.test.ts` (+3 -0) 📝 `src/browser/network/PlaywrightScenarioNetwork.ts` (+14 -3) 📝 `src/domain/browserScenarioCapture.test.ts` (+8 -0) 📝 `src/domain/browserScenarioCaptureValues.ts` (+106 -90) 📝 `src/domain/browserScenarioDiff.test.ts` (+298 -0) 📝 `src/domain/browserScenarioDiff.ts` (+171 -11) 📝 `src/domain/browserScenarioDiffEvidence.ts` (+62 -19) 📝 `src/domain/browserScenarioDiffValues.ts` (+12 -3) ➕ `src/domain/browserScenarioNetworkComparison.ts` (+222 -0) 📝 `src/domain/browserScenarioNormalization.ts` (+19 -19) 📝 `tests/fixtures/webScriptCapture.ts` (+1 -0) </details> ### 📄 Description ## Summary Two `capture-browser-scenario` runs of the same static page, with `capture.events: ["network"]`, compare as `changed` in `compare-web-captures`. The page is a local file with a stylesheet, four images and three parallel `fetch()` calls; nothing on it changes between runs: ``` compare-web-captures run1.json run2.json overall_status: changed scenario_start: events changed settle: events changed ``` Every pair I tried did this (16 of 16 on 6.4.0 and main), so a network-selected scenario comparison cannot report `unchanged` for an ordinary page. No related issue; the fix is small and stays inside the existing comparison contract, so it comes straight as a PR. ## Problem and expected behavior `eventsFor` (`src/domain/browserScenarioDiffEvidence.ts:97-117`) returns a step's events in receipt order and keeps `transaction_id` / `redirected_from_transaction_id`. The built-in rules drop only `sequence` and `step_index`. Two things then differ between identical runs: 1. Concurrent responses and `request-finished` events arrive in a different order each run, and `request-N` IDs follow request start order. 2. A response that arrives just after a step ends is attributed to the next step, so the same transaction is split across steps differently each run (scenario_start vs settle above). Expected: the same requests with the same outcomes compare `unchanged`; a different status, header set, body or URL still compares `changed`. The passive comparison already works this way (`webCaptureDiff.ts` compares requests as sorted digests per identity), and content events are already attributed to their source event's step. ## Change and scope - Network events (`request`, `response`, `request-failed`, `request-finished`, `request-unfinished`, `network-content`) are grouped per transaction and attributed to the step that sent the request (receipt step when the request was not observed). Content follows the same rule. - Capture-local transaction IDs are dropped; a redirect keeps its predecessor's method and URL instead of its ID. - Each transaction's own fields (method, URL, status, header names, failure, captured headers and body, redirect source) are still compared exactly; only the order between transactions and the capture-local IDs are dropped. Transactions are compared as an unordered collection. The ordering runs after caller literal rules, through a new optional `order` argument to `digestNormalizedScenarioValue`, so rules still apply to every field. - Console, page errors, frames, workers, popups, WebSocket frames and downloads keep receipt order and receipt step. - The comparison limitation text and `docs/browser-scenario-contract.md` describe the new rule. Not included: ordering for non-network events. Console output from concurrently loaded scripts can still reorder; that is page behavior, not capture noise. Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope): not applicable, a fix within the existing comparison. ## Contract and boundary impact - Semantic owner and earliest changed stage: browser scenario comparison, event projection (`browserScenarioDiffEvidence.ts`). - Contribution scope fit and linked maintainer decision: routine fix; no decision needed. - CLI and MCP/tool-catalog contract: same result schema; one limitation string reworded. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: event digests in scenario diffs change for network-selected captures; capture Evidence is unchanged. - Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact: none. - [x] 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: `compares concurrent network transactions without receipt order, receipt step or capture-local IDs` in `src/domain/browserScenarioDiff.test.ts`. Two transactions, the second redirected from the first, captured in opposite orders with the responses in different steps: `unchanged`. The same with the two statuses swapped: `changed`. It fails on main; reverting only the step attribution or only the redirect mapping also fails it. - Base reproduction or other evidence: executed on Linux x64, Chromium 153, local page above served on 127.0.0.1, scenario `goto` + `wait_for_timeout 1500`, network events only. Four captures, capture 1 compared with 2, 3, 4: ``` 6.4.0: changed, changed, changed main 3ae826ce: changed, changed, changed this branch: unchanged, unchanged, unchanged this branch, d2.json removed from the page: changed (scenario_start events) ``` A second page whose server delays each script 0-200 ms at random also compares `unchanged` on this branch. - User-visible CLI/MCP output: the `overall_status` lines above. - Remaining proof gaps: MCP not exercised separately (same shared workflow). 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 - `npx vitest run --project domain src/domain/browserScenarioDiff.test.ts` — 11 passed; the new case fails on 3ae826ce without the change. - `npm run check:fast` — passed. - `npx vitest run --project <name>` for each project except `acceptance`, after `npm run build:cached`, rebased on main 3ae826ce — 10045 passed, 64 failed, 86 skipped, against 10044 / 64 / 86 on untouched 3ae826ce with an identical failure list (environment: firmware tests that need Binwalk 3.1.x, ASAR tests whose unpacked companion file is missing here, Ghidra provider tests). `acceptance` was not run (out of memory on this machine). ## Compatibility, safety, and release - Breaking changes or migration steps: none. Stored diffs keep their meaning; new comparisons of network-selected captures can now return `unchanged` where they returned `changed`. - Real Hopper/Ghidra, browser, or OS coverage: Linux x64 with Playwright Chromium 153. - Package or release metadata impact: none. - Security, privacy, process, or containment review: none; comparison of already-redacted capture data only. ## 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-10-31 01:05:32 +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#5297

No description provided.

Rows
Columns