morluto/rea

Fork 0

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

[PR #6340] [MERGED] fix(browser): keep refresh guidance for a missing CDP target #7292

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/6340
Author: @Abelo9996
Created: 10/22/2026
Status: ✅ Merged
Merged: 10/22/2026
Merged by: @morluto

Base: main ← Head: fix-5947-cdp-target-not-found-remediation

📝 Commits (1)

3095ecb fix(browser): keep refresh guidance for a missing CDP target

📊 Changes

2 files changed (+6 additions, -1 deletions)

View changed files

📝 src/browser/CdpAuthorizedTarget.ts (+2 -0)
📝 tests/boundary/browser/cdpBrowserProvider.discovery_authorization.test.ts (+4 -1)

📄 Description

Summary

Fixes #5947.

When a caller selected a target ID that is not in the endpoint's discovery list, the CDP refusal kept the specific Target <id> was not found at the selected endpoint. detail but lost the instruction to refresh discovery. Both message and remediation.action repeated the detail. The refusal now keeps the detail as its message and carries the existing refresh instruction as its remediation.

Problem and expected behavior

authorizeCdpTarget in src/browser/CdpAuthorizedTarget.ts throws BrowserObservationError(operation, "target_not_found", { detail }). A detail replaces the reason-specific default message (which contains the refresh instruction), and with no remediationAction the remediation falls back to that same detail. Reproduced with the issue's loopback discovery fixture (empty /json/list) and the built CLI:

node scripts/rea.mjs inspect-web-page http://127.0.0.1:<port> stale-page --json

Before (exit 1):

{"code":"target_unavailable","category":"unavailable","message":"Target stale-page was not found at the selected endpoint.","retryable":false,"remediation":{"action":"Target stale-page was not found at the selected endpoint."},"details":{"operation":"inspect_web_page","reason":"target_not_found"}}

Expected: keep the ID-specific message, code, reason and retryability, and tell the caller to refresh discovery for that endpoint and retry with a returned ID.

Change and scope

Pass remediationAction: "Refresh target discovery for that endpoint and retry with a returned target ID." alongside the existing detail. analysisErrorRemediationAction already prefers a BrowserObservationError's remediationAction, and forOperation (from #6223) already carries it when the error is relabelled, so every CDP target operation gets it. The text is the same instruction the target_not_found default message uses.

The target_not_allowed refusal and the detail-less target_not_found producers (Electron, Playwright scenarios) are unchanged; the latter already use the default message with the refresh instruction.

Follow the contribution scope.
Scope decision: not applicable (ordinary fix within existing boundaries).

Contract and boundary impact

Semantic owner and earliest changed stage: authorizeCdpTarget, target authorization before any debugging session opens.
Contribution scope fit and linked maintainer decision: ordinary bug fix, not applicable.
CLI and MCP/tool-catalog contract: no schema change. remediation.action for a missing CDP target changes from the repeated detail to the refresh instruction. code, category, message, retryable and details are unchanged.
Provider, bridge, target-format, or platform compatibility: CDP browser provider target operations only.
Evidence, artifact, provenance, or reconstruction contract: none.
Target selection, authority, upstream behavior, and cleanup details (if applicable): unchanged. The ID is still refused before any debugging-session connection.
Generated metadata (docs/public/product-catalog.json), package, or installation impact (CI generates and validates ignored catalogs and skills/; edit authored instructions in .agents/skills/reverse-engineer-anything/ and never commit build outputs): none.

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.

(The existing HTTP discovery request to the caller's debugging endpoint, unchanged by this PR.)

Evidence and regression coverage

Tests added or updated: tests/boundary/browser/cdpBrowserProvider.discovery_authorization.test.ts, "keeps target selection detail for every target operation" now also asserts remediation.action for all five CDP target operations: the refresh instruction for missing, and the unchanged detail for disallowed-page.

Base reproduction (executed): with the test change and without the fix:

× keeps target selection detail for every target operation
-     "action": "Refresh target discovery for that endpoint and retry with a returned target ID.",
+     "action": "Target missing was not found at the selected endpoint.",
      Tests  1 failed | 15 passed (16)

With the fix: 16 passed.

User-visible CLI/MCP output (if applicable): the issue's Python discovery fixture against builds from npm run build:cached. Before is shown above. After (exit 1):

{"code":"target_unavailable","category":"unavailable","message":"Target stale-page was not found at the selected endpoint.","retryable":false,"remediation":{"action":"Refresh target discovery for that endpoint and retry with a returned target ID."},"details":{"operation":"inspect_web_page","reason":"target_not_found"}}

list-browser-targets exited 0 with targets: [], and the fixture saw only /json/version and /json/list requests.

Remaining proof gaps: no real browser. MCP transport not exercised. The remediation names the action generically rather than the CLI command list-browser-targets, so it reads the same over MCP and CLI.

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

Run with the pinned Node 24.18.0 on macOS arm64.

npm run test:focused -- tests/boundary/browser/cdpBrowserProvider.discovery_authorization.test.ts tests/boundary/browser/cdpBrowserProvider.browserSocket.test.ts tests/boundary/browser/cdpElectronProvider.test.ts src/domain/analysisErrorProjection.test.ts: 4 files, 107 tests passed (1 failure without the fix, shown above)
CLI reproduction above, with and without the fix
npm run check tasks: typecheck, lint:built (0 errors), knip, metadata:check and verify:architecture-guards passed. format:check was run as oxfmt --check on the two changed files (clean), because the repo-wide run in my checkout also picked up unrelated untracked local notes.

Compatibility, safety, and release

Breaking changes or migration steps: none. Only the remediation text changes.
Real Hopper/Ghidra, browser, or OS coverage: macOS, simulated CDP discovery endpoint and the repo's fake CDP browser. No real browser.
Package or release metadata impact: none.
Security, privacy, process, or containment review: no change to discovery, authorization or connections.

Author readiness

Human owner: @Abelo9996

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/6340 **Author:** [@Abelo9996](https://github.com/Abelo9996) **Created:** 10/22/2026 **Status:** ✅ Merged **Merged:** 10/22/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix-5947-cdp-target-not-found-remediation` --- ### 📝 Commits (1) - [`3095ecb`](https://github.com/morluto/rea/commit/3095ecb055e3586b2db91faa1ad32eff024b4810) fix(browser): keep refresh guidance for a missing CDP target ### 📊 Changes **2 files changed** (+6 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `src/browser/CdpAuthorizedTarget.ts` (+2 -0) 📝 `tests/boundary/browser/cdpBrowserProvider.discovery_authorization.test.ts` (+4 -1) </details> ### 📄 Description ## Summary Fixes #5947. When a caller selected a target ID that is not in the endpoint's discovery list, the CDP refusal kept the specific `Target <id> was not found at the selected endpoint.` detail but lost the instruction to refresh discovery. Both `message` and `remediation.action` repeated the detail. The refusal now keeps the detail as its message and carries the existing refresh instruction as its remediation. ## Problem and expected behavior `authorizeCdpTarget` in `src/browser/CdpAuthorizedTarget.ts` throws `BrowserObservationError(operation, "target_not_found", { detail })`. A `detail` replaces the reason-specific default message (which contains the refresh instruction), and with no `remediationAction` the remediation falls back to that same detail. Reproduced with the issue's loopback discovery fixture (empty `/json/list`) and the built CLI: ```sh node scripts/rea.mjs inspect-web-page http://127.0.0.1:<port> stale-page --json ``` Before (exit 1): ```json {"code":"target_unavailable","category":"unavailable","message":"Target stale-page was not found at the selected endpoint.","retryable":false,"remediation":{"action":"Target stale-page was not found at the selected endpoint."},"details":{"operation":"inspect_web_page","reason":"target_not_found"}} ``` Expected: keep the ID-specific message, code, reason and retryability, and tell the caller to refresh discovery for that endpoint and retry with a returned ID. ## Change and scope Pass `remediationAction: "Refresh target discovery for that endpoint and retry with a returned target ID."` alongside the existing `detail`. `analysisErrorRemediationAction` already prefers a `BrowserObservationError`'s `remediationAction`, and `forOperation` (from #6223) already carries it when the error is relabelled, so every CDP target operation gets it. The text is the same instruction the `target_not_found` default message uses. The `target_not_allowed` refusal and the detail-less `target_not_found` producers (Electron, Playwright scenarios) are unchanged; the latter already use the default message with the refresh instruction. Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). Scope decision: not applicable (ordinary fix within existing boundaries). ## Contract and boundary impact - Semantic owner and earliest changed stage: `authorizeCdpTarget`, target authorization before any debugging session opens. - Contribution scope fit and linked maintainer decision: ordinary bug fix, not applicable. - CLI and MCP/tool-catalog contract: no schema change. `remediation.action` for a missing CDP target changes from the repeated detail to the refresh instruction. `code`, `category`, `message`, `retryable` and `details` are unchanged. - Provider, bridge, target-format, or platform compatibility: CDP browser provider target operations only. - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details (if applicable): unchanged. The ID is still refused before any debugging-session connection. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact (CI generates and validates ignored catalogs and `skills/`; edit authored instructions in `.agents/skills/reverse-engineer-anything/` and never commit build outputs): none. 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. - [x] 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. (The existing HTTP discovery request to the caller's debugging endpoint, unchanged by this PR.) ## Evidence and regression coverage - Tests added or updated: `tests/boundary/browser/cdpBrowserProvider.discovery_authorization.test.ts`, "keeps target selection detail for every target operation" now also asserts `remediation.action` for all five CDP target operations: the refresh instruction for `missing`, and the unchanged detail for `disallowed-page`. - Base reproduction (executed): with the test change and without the fix: ``` × keeps target selection detail for every target operation - "action": "Refresh target discovery for that endpoint and retry with a returned target ID.", + "action": "Target missing was not found at the selected endpoint.", Tests 1 failed | 15 passed (16) ``` With the fix: 16 passed. - User-visible CLI/MCP output (if applicable): the issue's Python discovery fixture against builds from `npm run build:cached`. Before is shown above. After (exit 1): ```json {"code":"target_unavailable","category":"unavailable","message":"Target stale-page was not found at the selected endpoint.","retryable":false,"remediation":{"action":"Refresh target discovery for that endpoint and retry with a returned target ID."},"details":{"operation":"inspect_web_page","reason":"target_not_found"}} ``` `list-browser-targets` exited 0 with `targets: []`, and the fixture saw only `/json/version` and `/json/list` requests. - Remaining proof gaps: no real browser. MCP transport not exercised. The remediation names the action generically rather than the CLI command `list-browser-targets`, so it reads the same over MCP and CLI. 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 Run with the pinned Node 24.18.0 on macOS arm64. - `npm run test:focused -- tests/boundary/browser/cdpBrowserProvider.discovery_authorization.test.ts tests/boundary/browser/cdpBrowserProvider.browserSocket.test.ts tests/boundary/browser/cdpElectronProvider.test.ts src/domain/analysisErrorProjection.test.ts`: 4 files, 107 tests passed (1 failure without the fix, shown above) - CLI reproduction above, with and without the fix - `npm run check` tasks: `typecheck`, `lint:built` (0 errors), `knip`, `metadata:check` and `verify:architecture-guards` passed. `format:check` was run as `oxfmt --check` on the two changed files (clean), because the repo-wide run in my checkout also picked up unrelated untracked local notes. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Only the remediation text changes. - Real Hopper/Ghidra, browser, or OS coverage: macOS, simulated CDP discovery endpoint and the repo's fake CDP browser. No real browser. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no change to discovery, authorization or connections. ## Author readiness Human owner: @Abelo9996 - [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:46:48 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-03 02:13:14 +00:00

[GH-ISSUE #8508] [Bug] EVM limiter configuration failures expose internal codes as recovery guidance #6633

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

No description provided.

Rows
Columns