morluto/rea

Fork 0

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

[PR #6725] [MERGED] fix(captures): advertise offline capture inspection without a browser provider #7401

New issue

Closed

opened 2026-11-01 21:47:35 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:47:35 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6725
Author: @Sriram-PR
Created: 10/23/2026
Status: ✅ Merged
Merged: 10/23/2026
Merged by: @morluto

Base: main ← Head: fix/offline-capture-availability

📝 Commits (1)

ec4df76 fix(captures): advertise offline capture inspection without a browser provider

📊 Changes

2 files changed (+79 additions, -3 deletions)

View changed files

📝 src/contracts/webNetworkCaptureToolContracts.ts (+1 -1)
📝 tests/boundary/mcp/webNetworkCaptureMcp.test.ts (+78 -2)

📄 Description

Summary

binary_session.result.tool_availability reports inspect_web_network_capture as unavailable whenever no browser observation provider is running (provider_missing, or provider_unavailable after a recorded browser load failure), with "Start REA with its browser observation provider available." as the remediation. The same call works fine in that session, because the tool only reads a local HAR/mitmproxy/SAZ/qlog file and never uses the browser adapter. A client that follows the availability list, as the MCP contract tells it to, skips a working tool and gets the wrong repair advice.

Fixes #3947

Problem and expected behavior

Smallest trigger: a session-backed server with no browser port. binary_session lists inspect_web_network_capture with available: false, reason: "provider_missing"; add optionalProviderLoadFailures.browserObservation and it becomes provider_unavailable. In both cases calling the tool still returns Evidence. Expected: the offline inspection shows as available, while list_browser_targets and the other live browser tools stay unavailable with their current reasons.

Change and scope

The contract declared kind: "browser-provider", so CapabilityInventory applied the browser provider's presence and load-failure state to it. kind is the adapter family implementing the tool, and this one is implemented by WebNetworkCaptureService, which createServer composes independently of the browser port (the contract's own comment already says historical inspection is distinct from live browser capture). It's now kind: "application", like the inspect_analysis_view paging over the same retained captures, which CapabilityInventory already reports as available.
Fixing the kind keeps the browser availability rules unchanged and avoids a name-based exception in CapabilityInventory.
Not included: compare_web_captures and compare_web_screenshots are also browser-provider and only compare supplied results, but both currently call requireBrowser and refuse without a browser, so their unavailable entry matches what they do. Changing that is a separate behavior change.

Contribution scope: ordinary fix within existing boundaries; no scope decision needed.

Contract and boundary impact

Semantic owner and earliest changed stage: the inspect_web_network_capture contract in src/contracts/webNetworkCaptureToolContracts.ts; CapabilityInventory is unchanged.
Contribution scope fit and linked maintainer decision: not applicable (bug fix, #3947).
CLI and MCP/tool-catalog contract: the tool's catalog kind (and availability surface) changes from browser-provider to application, and its tool_availability entry is now available without a browser provider. Input/output schemas and behavior are unchanged.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: none.
Target selection, authority, upstream behavior, and cleanup details: not applicable.
Generated metadata, package, or installation impact: the generated MCP tool catalog picks up the new kind; mcp-catalog:check and docs:check pass.

Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.

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: tests/boundary/mcp/webNetworkCaptureMcp.test.ts adds advertises offline capture inspection without a browser provider, for both of the issue's sessions (no browser port, and a recorded browserObservation load failure). It reads binary_session availability and expects inspect_web_network_capture available and list_browser_targets unavailable with provider_missing / provider_unavailable, then calls both tools on the same server: the capture inspection succeeds and list_browser_targets errors. It uses the file's existing synthetic capture port (now a shared syntheticCaptureService helper), since the real decoder child process only resolves from the built dist/ tree; real offline decoding is already covered by the acceptance capture tests.
Base reproduction: the new test fails on main's contract with:
```
-   "available": true,
-   "reason": "available",
+   "available": false,
+   "reason": "provider_missing",
```
and provider_unavailable in the load-failure case.
User-visible CLI/MCP output: binary_session now lists {"name": "inspect_web_network_capture", "available": true, "reason": "available", ...} in sessions without a browser provider.
Remaining proof gaps: not checked against a real browser-adapter load failure; the failure is simulated through optionalProviderLoadFailures like the existing isolation tests.

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

npm run test:focused -- tests/boundary/mcp/webNetworkCaptureMcp.test.ts src/application/CapabilityInventory.test.ts: 41 passed (the two new cases fail on main's contract)
npm run test:focused -- tests/boundary/mcp/*.test.ts src/application/CapabilityInventory.test.ts tests/acceptance/tools/localToolContracts.test.ts: 1304 passed, 7 skipped, 2 failed. The 2 failures are tests/boundary/mcp/gdbCloseDisposition.test.ts (Invalid analysis input for gdb_console), which fail the same way on main without this change.
npm run check:fast: typecheck and lint passed
npm run mcp-catalog:check: passed
npm run docs:check: passed

Compatibility, safety, and release

Breaking changes or migration steps: none for callers of the tool. Anything that grouped tools by catalog kind will now see this tool under application instead of browser-provider.
Real Hopper/Ghidra, browser, or OS coverage: Linux x64, Node 24.18.0; no browser involved.
Package or release metadata impact: generated MCP tool catalog kind for this tool.
Security, privacy, process, or containment review: none; the tool's behavior and effects declaration are unchanged.

Author readiness

Human owner: @Sriram-PR

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

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/6725 **Author:** [@Sriram-PR](https://github.com/Sriram-PR) **Created:** 10/23/2026 **Status:** ✅ Merged **Merged:** 10/23/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/offline-capture-availability` --- ### 📝 Commits (1) - [`ec4df76`](https://github.com/morluto/rea/commit/ec4df76bf7784b340b387344103d319dc2ac1e0e) fix(captures): advertise offline capture inspection without a browser provider ### 📊 Changes **2 files changed** (+79 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `src/contracts/webNetworkCaptureToolContracts.ts` (+1 -1) 📝 `tests/boundary/mcp/webNetworkCaptureMcp.test.ts` (+78 -2) </details> ### 📄 Description ## Summary `binary_session.result.tool_availability` reports `inspect_web_network_capture` as unavailable whenever no browser observation provider is running (`provider_missing`, or `provider_unavailable` after a recorded browser load failure), with "Start REA with its browser observation provider available." as the remediation. The same call works fine in that session, because the tool only reads a local HAR/mitmproxy/SAZ/qlog file and never uses the browser adapter. A client that follows the availability list, as the MCP contract tells it to, skips a working tool and gets the wrong repair advice. Fixes #3947 ## Problem and expected behavior Smallest trigger: a session-backed server with no browser port. `binary_session` lists `inspect_web_network_capture` with `available: false, reason: "provider_missing"`; add `optionalProviderLoadFailures.browserObservation` and it becomes `provider_unavailable`. In both cases calling the tool still returns Evidence. Expected: the offline inspection shows as available, while `list_browser_targets` and the other live browser tools stay unavailable with their current reasons. ## Change and scope - The contract declared `kind: "browser-provider"`, so `CapabilityInventory` applied the browser provider's presence and load-failure state to it. `kind` is the adapter family implementing the tool, and this one is implemented by `WebNetworkCaptureService`, which `createServer` composes independently of the browser port (the contract's own comment already says historical inspection is distinct from live browser capture). It's now `kind: "application"`, like the `inspect_analysis_view` paging over the same retained captures, which `CapabilityInventory` already reports as available. - Fixing the kind keeps the browser availability rules unchanged and avoids a name-based exception in `CapabilityInventory`. - Not included: `compare_web_captures` and `compare_web_screenshots` are also `browser-provider` and only compare supplied results, but both currently call `requireBrowser` and refuse without a browser, so their unavailable entry matches what they do. Changing that is a separate behavior change. Contribution scope: ordinary fix within existing boundaries; no scope decision needed. ## Contract and boundary impact - Semantic owner and earliest changed stage: the `inspect_web_network_capture` contract in `src/contracts/webNetworkCaptureToolContracts.ts`; `CapabilityInventory` is unchanged. - Contribution scope fit and linked maintainer decision: not applicable (bug fix, #3947). - CLI and MCP/tool-catalog contract: the tool's catalog `kind` (and availability `surface`) changes from `browser-provider` to `application`, and its `tool_availability` entry is now available without a browser provider. Input/output schemas and behavior are unchanged. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: the generated MCP tool catalog picks up the new kind; `mcp-catalog:check` and `docs:check` pass. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. - [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: `tests/boundary/mcp/webNetworkCaptureMcp.test.ts` adds `advertises offline capture inspection without a browser provider`, for both of the issue's sessions (no browser port, and a recorded `browserObservation` load failure). It reads `binary_session` availability and expects `inspect_web_network_capture` available and `list_browser_targets` unavailable with `provider_missing` / `provider_unavailable`, then calls both tools on the same server: the capture inspection succeeds and `list_browser_targets` errors. It uses the file's existing synthetic capture port (now a shared `syntheticCaptureService` helper), since the real decoder child process only resolves from the built `dist/` tree; real offline decoding is already covered by the acceptance capture tests. - Base reproduction: the new test fails on main's contract with: ``` - "available": true, - "reason": "available", + "available": false, + "reason": "provider_missing", ``` and `provider_unavailable` in the load-failure case. - User-visible CLI/MCP output: `binary_session` now lists `{"name": "inspect_web_network_capture", "available": true, "reason": "available", ...}` in sessions without a browser provider. - Remaining proof gaps: not checked against a real browser-adapter load failure; the failure is simulated through `optionalProviderLoadFailures` like the existing isolation tests. 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 - `npm run test:focused -- tests/boundary/mcp/webNetworkCaptureMcp.test.ts src/application/CapabilityInventory.test.ts`: 41 passed (the two new cases fail on main's contract) - `npm run test:focused -- tests/boundary/mcp/*.test.ts src/application/CapabilityInventory.test.ts tests/acceptance/tools/localToolContracts.test.ts`: 1304 passed, 7 skipped, 2 failed. The 2 failures are `tests/boundary/mcp/gdbCloseDisposition.test.ts` (`Invalid analysis input for gdb_console`), which fail the same way on main without this change. - `npm run check:fast`: typecheck and lint passed - `npm run mcp-catalog:check`: passed - `npm run docs:check`: passed ## Compatibility, safety, and release - Breaking changes or migration steps: none for callers of the tool. Anything that grouped tools by catalog `kind` will now see this tool under `application` instead of `browser-provider`. - Real Hopper/Ghidra, browser, or OS coverage: Linux x64, Node 24.18.0; no browser involved. - Package or release metadata impact: generated MCP tool catalog kind for this tool. - Security, privacy, process, or containment review: none; the tool's behavior and effects declaration are unchanged. ## Author readiness Human owner: @Sriram-PR - [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] 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-11-01 21:47:35 +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#7401

No description provided.

Rows
Columns