morluto/rea

Fork 0

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

[PR #3416] [MERGED] test(browser): preserve resource uncertainty through MCP #5729

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3416
Author: @ShuaixinHuang
Created: 10/14/2026
Status: ✅ Merged
Merged: 10/15/2026
Merged by: @N0zoM1z0

Base: main ← Head: codex/test-resource-mcp-completeness

📝 Commits (4)

6ed055d test(browser): preserve resource uncertainty through MCP
4b3a0f8 Merge branch 'morluto:main' into codex/test-resource-mcp-completeness
64da682 test(browser): assert MCP resource details and exclusions
07ea877 Merge branch 'morluto:main' into codex/test-resource-mcp-completeness

📊 Changes

1 file changed (+150 additions, -3 deletions)

View changed files

📝 tests/boundary/browser/cdpResourceCompleteness.test.ts (+150 -3)

📄 Description

Summary

Verify resource uncertainty through the actual MCP SDK and production registration for inspect_web_page and compare_web_captures.

The resource-admission fix from #3236 is already on main. Its existing tests exercise the CDP provider and public CLI, but do not assert malformed-resource comparisons through the MCP capture/result/input-schema path. Add this coverage to the existing fixture family rather than rebuilding a separate fake browser.

Change and scope

Capture a complete inventory, then missing collections, invalid entries, missing required types, invalid sizes, and a valid empty inventory through inspect_web_page.
Validate capture Evidence and inspection schema, exact retained resource URLs/origins/types/MIME/sizes, and exact resource exclusion reasons/counts. Invalid sizes remain null; valid empty inventories have no resource exclusions.
Compare captures forward, backward, and with themselves through compare_web_captures. Incomplete inventories must remain unknown; the complete empty control must prove two changes in either direction and an unchanged self-comparison.
Close the SDK harness with the existing test cleanup hook and retain every existing provider/CLI case.

One test file. Production behavior, schemas, dependencies and generated metadata are unchanged. This is test coverage within existing contribution scope; a separate capability decision is not applicable. Calls use the production server registration and real SDK in-memory transport against a controlled loopback CDP fixture. Test fixtures write isolated temporary files. No live Chromium/Edge, stdio-MCP process, target injection or external network service is exercised.

Evidence and regression coverage

Original baseline 7c962cc70bcebf6201da9c5db5b7c0edd2564ae7. Review follow-up: 64da682; published synchronization: 07ea877, with upstream parent f75b1a416ba67b8421e7dbd248160afedb79581f. Existing MCP browser tests assert generic unknown comparisons but not these malformed resource states and the corresponding resource dimension. The new scenario performs six SDK captures and fifteen SDK comparisons, checking the externally returned Evidence rather than calling the comparison implementation directly.

Validation performed

Windows x64, pinned Node 24.18.0 / npm 11.16.0, unchanged locked dependencies. Final local validation completed at 9115d8d1de68d9abde510d399347b6e5cdeb7ab2 (the resource review follow-up plus upstream c2b79aa):

npm run test:focused -- tests/boundary/browser/cdpResourceCompleteness.test.ts — 23 passed, including the new SDK scenario and all existing provider/CLI cases.
npm run check — all six tasks and architecture guards passed; zero lint errors.
git diff --check — passed.

Additional Windows investigation on the synchronized tree selected the resource file, cliOutput.test.ts, dshClientSetup.test.ts, and doctorHopperDiscovery.test.ts: 49 passed, one failed, one skipped. Both previously timed-out scenarios passed locally. The separate CLI JavaScript projection case returned exit 1 instead of 0; Hopper is platform-skipped on Windows. This combined run is not claimed green. The final resource-only focused run passed all 23 tests after the field/exclusion assertions were added; npm run check also passed on that revision.

First Linux CI run failed three tests: the Hopper launcher fixture, a CLI process deadline (3000ms), and a DSH test deadline (30000ms). The next observed Linux run passed both timed-out scenarios and finished with 1,624 passed, 22 skipped, and one remaining Hopper assertion failure. The final synchronization includes the maintainer fix c2b79aa that separates launcher availability from Hopper version readiness. It also includes the subsequent upstream service-worker fixture changes. Current-head Linux CI passed on published head 07ea877: all five test shards, source tests, build, static checks, Portable tests, and CI required succeeded. Greptile re-reviewed this head at 5/5. The separate PR readiness status is pending because GitHub has not confirmed the head association; the page requests a rerun. No timeouts, retries, production behavior, or unrelated assertions were changed by this PR. Live browser qualification lanes were skipped in this run; no live browser verification is claimed. This adds transport/schema regression coverage and does not claim a newly reproduced production defect.

Author readiness

Human owner: @ShuaixinHuang

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

Focused SDK coverage preserves missing-evidence uncertainty and valid empty inventories.
Existing production and provider/CLI assertions are retained.
Validation and controlled-provider limits are recorded.
Human owner has declared initial readiness.
Required CI workflow gates pass for the current head.
PR readiness publishes a confirmed head status.

_{🔄 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/3416 **Author:** [@ShuaixinHuang](https://github.com/ShuaixinHuang) **Created:** 10/14/2026 **Status:** ✅ Merged **Merged:** 10/15/2026 **Merged by:** [@N0zoM1z0](https://github.com/N0zoM1z0) **Base:** `main` ← **Head:** `codex/test-resource-mcp-completeness` --- ### 📝 Commits (4) - [`6ed055d`](https://github.com/morluto/rea/commit/6ed055dc97a0fbd773e526dc25fc310ca6a8fd4c) test(browser): preserve resource uncertainty through MCP - [`4b3a0f8`](https://github.com/morluto/rea/commit/4b3a0f870e4494eb53de4bb587fb2604842f32c8) Merge branch 'morluto:main' into codex/test-resource-mcp-completeness - [`64da682`](https://github.com/morluto/rea/commit/64da68267bf53a8d2b5a73b49912d27d581adc20) test(browser): assert MCP resource details and exclusions - [`07ea877`](https://github.com/morluto/rea/commit/07ea877a388ee2cd0586f04b1fb75bd104b484be) Merge branch 'morluto:main' into codex/test-resource-mcp-completeness ### 📊 Changes **1 file changed** (+150 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `tests/boundary/browser/cdpResourceCompleteness.test.ts` (+150 -3) </details> ### 📄 Description ## Summary Verify resource uncertainty through the actual MCP SDK and production registration for `inspect_web_page` and `compare_web_captures`. The resource-admission fix from #3236 is already on main. Its existing tests exercise the CDP provider and public CLI, but do not assert malformed-resource comparisons through the MCP capture/result/input-schema path. Add this coverage to the existing fixture family rather than rebuilding a separate fake browser. ## Change and scope - Capture a complete inventory, then missing collections, invalid entries, missing required types, invalid sizes, and a valid empty inventory through `inspect_web_page`. - Validate capture Evidence and inspection schema, exact retained resource URLs/origins/types/MIME/sizes, and exact resource exclusion reasons/counts. Invalid sizes remain null; valid empty inventories have no resource exclusions. - Compare captures forward, backward, and with themselves through `compare_web_captures`. Incomplete inventories must remain `unknown`; the complete empty control must prove two changes in either direction and an unchanged self-comparison. - Close the SDK harness with the existing test cleanup hook and retain every existing provider/CLI case. One test file. Production behavior, schemas, dependencies and generated metadata are unchanged. This is test coverage within existing contribution scope; a separate capability decision is not applicable. Calls use the production server registration and real SDK in-memory transport against a controlled loopback CDP fixture. Test fixtures write isolated temporary files. No live Chromium/Edge, stdio-MCP process, target injection or external network service is exercised. ## Evidence and regression coverage Original baseline `7c962cc70bcebf6201da9c5db5b7c0edd2564ae7`. Review follow-up: `64da682`; published synchronization: `07ea877`, with upstream parent `f75b1a416ba67b8421e7dbd248160afedb79581f`. Existing MCP browser tests assert generic unknown comparisons but not these malformed resource states and the corresponding resource dimension. The new scenario performs six SDK captures and fifteen SDK comparisons, checking the externally returned Evidence rather than calling the comparison implementation directly. ## Validation performed Windows x64, pinned Node 24.18.0 / npm 11.16.0, unchanged locked dependencies. Final local validation completed at `9115d8d1de68d9abde510d399347b6e5cdeb7ab2` (the resource review follow-up plus upstream `c2b79aa`): - `npm run test:focused -- tests/boundary/browser/cdpResourceCompleteness.test.ts` — 23 passed, including the new SDK scenario and all existing provider/CLI cases. - `npm run check` — all six tasks and architecture guards passed; zero lint errors. - `git diff --check` — passed. Additional Windows investigation on the synchronized tree selected the resource file, `cliOutput.test.ts`, `dshClientSetup.test.ts`, and `doctorHopperDiscovery.test.ts`: 49 passed, one failed, one skipped. Both previously timed-out scenarios passed locally. The separate CLI JavaScript projection case returned exit 1 instead of 0; Hopper is platform-skipped on Windows. This combined run is not claimed green. The final resource-only focused run passed all 23 tests after the field/exclusion assertions were added; `npm run check` also passed on that revision. [First Linux CI run](https://github.com/morluto/rea/actions/runs/38546065985/job/115792770235?pr=3416) failed three tests: the Hopper launcher fixture, a CLI process deadline (3000ms), and a DSH test deadline (30000ms). [The next observed Linux run](https://github.com/morluto/rea/actions/runs/38554003926/job/115819337031?pr=3416) passed both timed-out scenarios and finished with 1,624 passed, 22 skipped, and one remaining Hopper assertion failure. The final synchronization includes the maintainer fix `c2b79aa` that separates launcher availability from Hopper version readiness. It also includes the subsequent upstream service-worker fixture changes. [Current-head Linux CI](https://github.com/morluto/rea/actions/runs/38556462720?pr=3416) passed on published head `07ea877`: all five test shards, source tests, build, static checks, Portable tests, and CI required succeeded. Greptile re-reviewed this head at 5/5. The separate PR readiness status is pending because GitHub has not confirmed the head association; the page requests a rerun. No timeouts, retries, production behavior, or unrelated assertions were changed by this PR. Live browser qualification lanes were skipped in this run; no live browser verification is claimed. This adds transport/schema regression coverage and does not claim a newly reproduced production defect. ## Author readiness Human owner: @ShuaixinHuang - [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] Focused SDK coverage preserves missing-evidence uncertainty and valid empty inventories. - [x] Existing production and provider/CLI assertions are retained. - [x] Validation and controlled-provider limits are recorded. - [x] Human owner has declared initial readiness. - [x] Required CI workflow gates pass for the current head. - [ ] PR readiness publishes a confirmed head status. --- <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:59 +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#5729

No description provided.

Rows
Columns