morluto/rea

Fork 0

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

[PR #3236] [CLOSED] fix(browser): report malformed resource inventories as incomplete #5665

New issue

Closed

opened 2026-10-31 01:08:25 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:08:25 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3236
Author: @ShuaixinHuang
Created: 10/14/2026
Status: ❌ Closed

Base: main ← Head: codex/fix-resource-capture-completeness

📝 Commits (3)

8c19aa2 fix(browser): report malformed resource inventories as incomplete
5d33a62 fix(browser): preserve malformed resource field uncertainty
ea620d4 Merge remote-tracking branch 'origin/main' into HEAD

📊 Changes

4 files changed (+307 additions, -8 deletions)

View changed files

📝 docs/browser-observation.md (+12 -0)
📝 src/browser/CdpCaptureDocuments.test.ts (+10 -2)
📝 src/browser/CdpCaptureDocuments.ts (+26 -6)
➕ tests/boundary/browser/cdpResourceCompleteness.test.ts (+259 -0)

📄 Description

Summary

inspect_web_page silently treated a missing or malformed Page.getResourceTree resource collection as empty, discarded invalid entries without recording an omission, and invented defaults for malformed resource fields. Comparing the resulting captures could report a resource deletion, addition, or equality that incomplete evidence did not establish.

Validate resource collections at the CDP producer boundary and record omissions in the existing completeness ledger, preserving valid sibling/frame resources. Legitimate empty arrays continue to support complete inventory comparison. This is a focused resource-admission fix based directly on main; #3125 addresses DOM/frame comparison in the same producing module.

No issue is linked: this was independently reproduced during source investigation, and searches for Page.getResourceTree completeness and malformed resource inventory reports found no matching issue/PR. The executable reproductions are included here.

Problem and expected behavior

Baseline 58300885207133422af235e0f2b10658cf958b03:

CDP resource evidence	Baseline comparison	Expected/fixed result
Root `resources` absent, null, or a non-array object	Two recorded resources reported removed	`unknown` resource inventory
`[validResource, null]`	Invalid entry disappears, peer resource reported removed	Keep the valid resource and record one omission; `unknown`
A child frame lacks its `resources` collection	`unchanged` despite unexamined child evidence	Preserve root resources; `unknown`
Resource type or MIME type missing or malformed	Defaults invent a resource identity and report removal/addition	Omit the malformed record with explicit incomplete coverage; preserve valid peers
Supplied content size is negative, null or non-numeric	Negative size becomes zero; malformed size looks like an ordinary optional omission	Retain other resource facts, use unknown size, and mark coverage incomplete
Valid `resources: []` after a complete two-resource capture	Two resources removed	Still `changed`, with two removals

The producing loop used recordsValue, which collapses malformed collections to [] and silently drops non-object entries. The resource comparator already respects its completeness ledger; the absent producer accounting caused the false claims.

Change and scope

captureResources checks the array before enumeration and parses each record once. A non-array collection records resources / invalid_protocol_value with an unknown omitted count; invalid entries record known single omissions. Other valid observations are retained. The former lossy conversion is removed from this path. No downstream repair or schema expansion is needed.

Resource field admission is owned by the same loop. Required type and mimeType values are parsed once, without Other/empty-string fallback: type must be nonempty and MIME type must be a string, including a valid explicit empty string. Missing optional contentSize stays null without a malformed-input exclusion. A supplied invalid or negative size becomes null with one exclusion, while the URL, type and MIME facts remain retained. Nonnegative numbers, including zero and fractional values, are preserved. The upstream CDP FrameResource definition distinguishes required type/MIME fields from optional content size. The existing deep-tree fixture now supplies the required MIME field; this preserves its traversal-capacity assertion. No superseded field defaults or size-clamping repair remain on this path.

The browser observation guide explains unavailable versus explicitly empty inventories. This fits the existing passive capture and offline evidence-comparison scope. A maintainer decision for new execution, mutation, provider or protection-recovery authority is not applicable; commands, target selection and cleanup are unchanged.

Contract and boundary impact

Semantic owner: CDP Page.getResourceTree resource admission in CdpCaptureDocuments.ts.
Consumers: CdpPageCapture retains the parsed resources and ledger; existing resource comparison uses the recorded coverage. Existing script/resource association checks remain covered.
CLI/MCP: existing capture and comparison schemas, parameters and operation names. Resource omissions become explicit in the existing completeness fields.
Evidence: retain valid facts; failed protocol admission cannot establish absence or equality.
Effects and packaging: correction of an existing response parser; no new commands or authority and no generated/package artifacts committed.

Evidence and regression coverage

cdpResourceCompleteness.test.ts calls the production CdpBrowserProvider over the repository's real loopback HTTP/WebSocket fixture and passes its normalized results through the production comparison input parser/comparator. It covers missing/null/non-array collections, invalid entries mixed with valid records, child omissions, both comparison directions and self-comparison, omission counts, retained facts, and a complete empty-inventory control.

The public CLI case writes the production captures to JSON files and executes the compiled rea compare-web-captures ... --json: partial membership returns resources.status: unknown with zero proven changes; the valid empty inventory returns changed with two removals.

The same production-boundary suite adds ten malformed-field cases, checking both directions and self-comparison, exclusion counts and preserved peer facts. Three legal-input controls cover explicit Other/empty MIME/absent size, zero, and fractional size. The public CLI workflow also covers missing type and negative size.

On untouched baseline production code, the five malformed-evidence cases fail for actual false changed/unchanged results while the legal empty-inventory control passes. The public CLI case was excluded from that initial source reproduction and was executed successfully after the runtime build.

On the previously published PR head 8c19aa2fa34a2883aed61be10732b50212cda13e, all ten new malformed-field cases and the extended public CLI case fail because resource comparison falsely reports changed. The nine remaining cases, including all three new legal-input controls, pass. After the field-admission correction all twenty cases pass. The first sandboxed attempt was blocked before meaningful assertions by loopback connection EACCES; the failure reproduction and final boundary verification ran outside that sandbox using only the local fixture endpoints.

Validation performed

Windows x64, pinned Node 24.18.0 / npm 11.16.0 and locked dependencies:

npm run test:focused -- tests/boundary/browser/cdpResourceCompleteness.test.ts tests/boundary/browser/cdpCaptureDocuments.test.ts tests/boundary/browser/webCaptureDiff.test.ts src/browser/CdpCaptureDocuments.test.ts tests/boundary/browser/cdpBrowserProvider.document_script.test.ts tests/boundary/browser/cdpBrowserProvider.sensitive_data.test.ts — 99 passed, 6 files, including all 20 resource-completeness cases and compiled CLI comparison.
npm run check — all six tasks and architecture guards passed; zero lint errors.
npm run docs:check — passed, including 5 README consistency tests.
npm run verify:test-discovery — all 1,243 files discovered exactly once.
git diff --check — passed.

Verification limits: the protocol fixture establishes production-boundary handling of malformed replies, not their frequency in conforming browsers. No real Chrome, Linux/macOS, or complete repository-wide test-suite verification is claimed. Dedicated MCP transport verification was not run for this change; the shared producer/comparator and real CLI path are covered above. No performance improvement is claimed.

Before merge

Concrete scoped defect and Conventional Commit title.
Production-boundary failure reproduction, legal-input control and actual public CLI Evidence.
Valid peer facts, provenance and uncertainty preserved.
Owning documentation updated; diff checked for unrelated files and generated/private artifacts.
Required current-head CI and applicable provider verification accepted by the maintainer.

_{🔄 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/3236 **Author:** [@ShuaixinHuang](https://github.com/ShuaixinHuang) **Created:** 10/14/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `codex/fix-resource-capture-completeness` --- ### 📝 Commits (3) - [`8c19aa2`](https://github.com/morluto/rea/commit/8c19aa2fa34a2883aed61be10732b50212cda13e) fix(browser): report malformed resource inventories as incomplete - [`5d33a62`](https://github.com/morluto/rea/commit/5d33a62d2b1c958ad14b9c5ea9a85ee7a74665b9) fix(browser): preserve malformed resource field uncertainty - [`ea620d4`](https://github.com/morluto/rea/commit/ea620d42fc3077d1bed490ba527acfcbbf245342) Merge remote-tracking branch 'origin/main' into HEAD ### 📊 Changes **4 files changed** (+307 additions, -8 deletions) <details> <summary>View changed files</summary> 📝 `docs/browser-observation.md` (+12 -0) 📝 `src/browser/CdpCaptureDocuments.test.ts` (+10 -2) 📝 `src/browser/CdpCaptureDocuments.ts` (+26 -6) ➕ `tests/boundary/browser/cdpResourceCompleteness.test.ts` (+259 -0) </details> ### 📄 Description ## Summary `inspect_web_page` silently treated a missing or malformed `Page.getResourceTree` resource collection as empty, discarded invalid entries without recording an omission, and invented defaults for malformed resource fields. Comparing the resulting captures could report a resource deletion, addition, or equality that incomplete evidence did not establish. Validate resource collections at the CDP producer boundary and record omissions in the existing completeness ledger, preserving valid sibling/frame resources. Legitimate empty arrays continue to support complete inventory comparison. This is a focused resource-admission fix based directly on main; #3125 addresses DOM/frame comparison in the same producing module. No issue is linked: this was independently reproduced during source investigation, and searches for `Page.getResourceTree` completeness and malformed resource inventory reports found no matching issue/PR. The executable reproductions are included here. ## Problem and expected behavior Baseline `58300885207133422af235e0f2b10658cf958b03`: | CDP resource evidence | Baseline comparison | Expected/fixed result | | --- | --- | --- | | Root `resources` absent, null, or a non-array object | Two recorded resources reported removed | `unknown` resource inventory | | `[validResource, null]` | Invalid entry disappears, peer resource reported removed | Keep the valid resource and record one omission; `unknown` | | A child frame lacks its `resources` collection | `unchanged` despite unexamined child evidence | Preserve root resources; `unknown` | | Resource type or MIME type missing or malformed | Defaults invent a resource identity and report removal/addition | Omit the malformed record with explicit incomplete coverage; preserve valid peers | | Supplied content size is negative, null or non-numeric | Negative size becomes zero; malformed size looks like an ordinary optional omission | Retain other resource facts, use unknown size, and mark coverage incomplete | | Valid `resources: []` after a complete two-resource capture | Two resources removed | Still `changed`, with two removals | The producing loop used `recordsValue`, which collapses malformed collections to `[]` and silently drops non-object entries. The resource comparator already respects its completeness ledger; the absent producer accounting caused the false claims. ## Change and scope `captureResources` checks the array before enumeration and parses each record once. A non-array collection records `resources / invalid_protocol_value` with an unknown omitted count; invalid entries record known single omissions. Other valid observations are retained. The former lossy conversion is removed from this path. No downstream repair or schema expansion is needed. Resource field admission is owned by the same loop. Required `type` and `mimeType` values are parsed once, without `Other`/empty-string fallback: type must be nonempty and MIME type must be a string, including a valid explicit empty string. Missing optional `contentSize` stays null without a malformed-input exclusion. A supplied invalid or negative size becomes null with one exclusion, while the URL, type and MIME facts remain retained. Nonnegative numbers, including zero and fractional values, are preserved. The [upstream CDP FrameResource definition](https://github.com/ChromeDevTools/devtools-protocol/blob/master/pdl/domains/Page.pdl) distinguishes required type/MIME fields from optional content size. The existing deep-tree fixture now supplies the required MIME field; this preserves its traversal-capacity assertion. No superseded field defaults or size-clamping repair remain on this path. The browser observation guide explains unavailable versus explicitly empty inventories. This fits the existing passive capture and offline evidence-comparison scope. A maintainer decision for new execution, mutation, provider or protection-recovery authority is not applicable; commands, target selection and cleanup are unchanged. ## Contract and boundary impact - Semantic owner: CDP `Page.getResourceTree` resource admission in `CdpCaptureDocuments.ts`. - Consumers: `CdpPageCapture` retains the parsed resources and ledger; existing resource comparison uses the recorded coverage. Existing script/resource association checks remain covered. - CLI/MCP: existing capture and comparison schemas, parameters and operation names. Resource omissions become explicit in the existing completeness fields. - Evidence: retain valid facts; failed protocol admission cannot establish absence or equality. - Effects and packaging: correction of an existing response parser; no new commands or authority and no generated/package artifacts committed. ## Evidence and regression coverage `cdpResourceCompleteness.test.ts` calls the production `CdpBrowserProvider` over the repository's real loopback HTTP/WebSocket fixture and passes its normalized results through the production comparison input parser/comparator. It covers missing/null/non-array collections, invalid entries mixed with valid records, child omissions, both comparison directions and self-comparison, omission counts, retained facts, and a complete empty-inventory control. The public CLI case writes the production captures to JSON files and executes the compiled `rea compare-web-captures ... --json`: partial membership returns `resources.status: unknown` with zero proven changes; the valid empty inventory returns `changed` with two removals. The same production-boundary suite adds ten malformed-field cases, checking both directions and self-comparison, exclusion counts and preserved peer facts. Three legal-input controls cover explicit `Other`/empty MIME/absent size, zero, and fractional size. The public CLI workflow also covers missing type and negative size. On untouched baseline production code, the five malformed-evidence cases fail for actual false `changed`/`unchanged` results while the legal empty-inventory control passes. The public CLI case was excluded from that initial source reproduction and was executed successfully after the runtime build. On the previously published PR head `8c19aa2fa34a2883aed61be10732b50212cda13e`, all ten new malformed-field cases and the extended public CLI case fail because resource comparison falsely reports `changed`. The nine remaining cases, including all three new legal-input controls, pass. After the field-admission correction all twenty cases pass. The first sandboxed attempt was blocked before meaningful assertions by loopback connection `EACCES`; the failure reproduction and final boundary verification ran outside that sandbox using only the local fixture endpoints. ## Validation performed Windows x64, pinned Node 24.18.0 / npm 11.16.0 and locked dependencies: - `npm run test:focused -- tests/boundary/browser/cdpResourceCompleteness.test.ts tests/boundary/browser/cdpCaptureDocuments.test.ts tests/boundary/browser/webCaptureDiff.test.ts src/browser/CdpCaptureDocuments.test.ts tests/boundary/browser/cdpBrowserProvider.document_script.test.ts tests/boundary/browser/cdpBrowserProvider.sensitive_data.test.ts` — **99 passed, 6 files**, including all 20 resource-completeness cases and compiled CLI comparison. - `npm run check` — all six tasks and architecture guards passed; zero lint errors. - `npm run docs:check` — passed, including 5 README consistency tests. - `npm run verify:test-discovery` — all 1,243 files discovered exactly once. - `git diff --check` — passed. Verification limits: the protocol fixture establishes production-boundary handling of malformed replies, not their frequency in conforming browsers. No real Chrome, Linux/macOS, or complete repository-wide test-suite verification is claimed. Dedicated MCP transport verification was not run for this change; the shared producer/comparator and real CLI path are covered above. No performance improvement is claimed. ## Before merge - [x] Concrete scoped defect and Conventional Commit title. - [x] Production-boundary failure reproduction, legal-input control and actual public CLI Evidence. - [x] Valid peer facts, provenance and uncertainty preserved. - [x] Owning documentation updated; diff checked for unrelated files and generated/private artifacts. - [ ] Required current-head CI and applicable provider verification accepted by the maintainer. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:08:25 +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#5665

No description provided.

Rows
Columns