[PR #6199] [CLOSED] fix(browser): explain target changes without suggesting configuration repair #7199

New issue

Closed

opened 2026-11-01 21:46:04 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:46:04 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6199
Author: @hoffieyang
Created: 10/22/2026
Status: ❌ Closed

Base: main ← Head: fix/browser-target-changed-recovery

📝 Commits (1)

03f875d fix(browser): explain target changes without suggesting configuration repair

📊 Changes

2 files changed (+49 additions, -0 deletions)

View changed files

📝 src/domain/analysisErrorProjection.test.ts (+47 -0)
📝 src/domain/browserObservationError.ts (+2 -0)

📄 Description

Summary

Fixes #5724. A changing target is correctly rejected, but the default error currently recommends rea doctor. Explain that the target changed and tell the caller to let it settle, refresh discovery on the same endpoint if needed, verify the selection, and retry.

Problem and expected behavior

On base 73be86c39e6c13139e0755c8ff2f8e95751d4a72, projecting new BrowserObservationError("inspect_web_page", "target_changed") produces the generic installation-diagnostics message for both message and remediation.action. The existing issue documents a real reload trigger. This contribution independently reproduces the presentation defect at the shared error-projection boundary; it does not claim to rerun that real-browser fixture.

The new default is:

inspect_web_page could not complete because the selected target changed during observation. Wait for the target to settle. If needed, refresh target discovery on the same debugging endpoint, verify the selected target, and retry the observation.

Change and scope

Add one reason-specific branch in the existing observationFailureMessage owner. The existing shared projection supplies the fallback remediation from that message; no second classification or recovery mechanism is introduced. Add regression cases for three browser-family operation names and for detail-only, remediation-only, and combined producer overrides.

This is a usability fix within an existing boundary; an advance maintainer capability/scope decision is not applicable. AI assistance was used. No target interaction is added.

Contract and boundary impact

Owner: src/domain/browserObservationError.ts, default error presentation.
Public text changes; target_unavailable, unavailable, retryable: false, operation/reason details, and schema stay unchanged.
Explicit producer detail/remediation retains precedence. Existing cleanup, partial-observation, capture identity, navigation refusal, and retry policy are unchanged.
No provider protocol, target authority, generated metadata, dependencies, package version, or installation changes.

Evidence and regression coverage

Executed RED on the base plus only the new tests: 4 failures / 63 passes. Failures received the generic doctor message instead of target-change guidance.
Executed GREEN after the two-line production change: 67/67 tests pass. This includes the existing cleanup-composition case retaining primary_reason: target_changed and browser_transport ownership, and existing partial-observation projection tests.
Tests assert the public projection and validate its schema, preserving machine-readable classification and producer overrides.
Proof gaps: no real-browser, compiled CLI/MCP, or full test-suite run in this contribution. The change is in shared presentation, not a capture/provider implementation.

Validation performed

macOS arm64, Node 24.18.0, npm 11.16.0, locked dependencies:

npm run test:local -- src/domain/analysisErrorProjection.test.ts — 67 passed after the repair; RED results above.
npm run check — passed (6 tasks plus architecture guards); lint reported 0 errors and 2,122 warnings across the repository.
git diff --check — passed.

Compatibility, safety, and release

No migration or release metadata change. Consumers comparing the English message verbatim will see new text; structured fields remain compatible. A target that never settles can still be rejected. No automatic retry, navigation, scope expansion, or cleanup change is introduced. Real browser/OS-provider compatibility beyond this local presentation check remains unverified.

Author readiness

Human owner: @hoffieyang

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

Human owner completes the readiness declaration and marks the draft ready.
Required checks pass for the current PR revision.

_{🔄 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/6199 **Author:** [@hoffieyang](https://github.com/hoffieyang) **Created:** 10/22/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/browser-target-changed-recovery` --- ### 📝 Commits (1) - [`03f875d`](https://github.com/morluto/rea/commit/03f875d22a11ef508be7170d29487ed5d77a5d15) fix(browser): explain target changes without suggesting configuration repair ### 📊 Changes **2 files changed** (+49 additions, -0 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/analysisErrorProjection.test.ts` (+47 -0) 📝 `src/domain/browserObservationError.ts` (+2 -0) </details> ### 📄 Description ## Summary Fixes #5724. A changing target is correctly rejected, but the default error currently recommends `rea doctor`. Explain that the target changed and tell the caller to let it settle, refresh discovery on the same endpoint if needed, verify the selection, and retry. ## Problem and expected behavior On base `73be86c39e6c13139e0755c8ff2f8e95751d4a72`, projecting `new BrowserObservationError("inspect_web_page", "target_changed")` produces the generic installation-diagnostics message for both `message` and `remediation.action`. The existing issue documents a real reload trigger. This contribution independently reproduces the presentation defect at the shared error-projection boundary; it does not claim to rerun that real-browser fixture. The new default is: > inspect_web_page could not complete because the selected target changed during observation. Wait for the target to settle. If needed, refresh target discovery on the same debugging endpoint, verify the selected target, and retry the observation. ## Change and scope Add one reason-specific branch in the existing `observationFailureMessage` owner. The existing shared projection supplies the fallback remediation from that message; no second classification or recovery mechanism is introduced. Add regression cases for three browser-family operation names and for detail-only, remediation-only, and combined producer overrides. This is a usability fix within an existing boundary; an advance maintainer capability/scope decision is not applicable. AI assistance was used. No target interaction is added. ## Contract and boundary impact - Owner: `src/domain/browserObservationError.ts`, default error presentation. - Public text changes; `target_unavailable`, `unavailable`, `retryable: false`, operation/reason details, and schema stay unchanged. - Explicit producer detail/remediation retains precedence. Existing cleanup, partial-observation, capture identity, navigation refusal, and retry policy are unchanged. - No provider protocol, target authority, generated metadata, dependencies, package version, or installation changes. ## Evidence and regression coverage - Executed RED on the base plus only the new tests: 4 failures / 63 passes. Failures received the generic doctor message instead of target-change guidance. - Executed GREEN after the two-line production change: 67/67 tests pass. This includes the existing cleanup-composition case retaining `primary_reason: target_changed` and `browser_transport` ownership, and existing partial-observation projection tests. - Tests assert the public projection and validate its schema, preserving machine-readable classification and producer overrides. - Proof gaps: no real-browser, compiled CLI/MCP, or full test-suite run in this contribution. The change is in shared presentation, not a capture/provider implementation. ## Validation performed macOS arm64, Node **24.18.0**, npm **11.16.0**, locked dependencies: - `npm run test:local -- src/domain/analysisErrorProjection.test.ts` — 67 passed after the repair; RED results above. - `npm run check` — passed (6 tasks plus architecture guards); lint reported 0 errors and 2,122 warnings across the repository. - `git diff --check` — passed. ## Compatibility, safety, and release No migration or release metadata change. Consumers comparing the English message verbatim will see new text; structured fields remain compatible. A target that never settles can still be rejected. No automatic retry, navigation, scope expansion, or cleanup change is introduced. Real browser/OS-provider compatibility beyond this local presentation check remains unverified. ## Author readiness Human owner: @hoffieyang - [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] Human owner completes the readiness declaration and marks the draft ready. - [ ] Required checks pass for the current PR revision. --- <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:04 +00:00