fix: fail closed when named daemon loses endpoint - #603
Open
bferanmi806-sketch wants to merge 1 commit into
Open
fix: fail closed when named daemon loses endpoint#603bferanmi806-sketch wants to merge 1 commit into
bferanmi806-sketch wants to merge 1 commit into
Conversation
✅ Skill review passedReviewed 1 file(s) — no findings. |
bferanmi806-sketch
marked this pull request as ready for review
August 14, 2026 14:23
Contributor
There was a problem hiding this comment.
1 issue found across 8 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/browser_harness/admin.py">
<violation number="1" location="src/browser_harness/admin.py:356">
P3: For named daemons, this sets `local` false and skips Chrome launch and `chrome://inspect` recovery, but the function docstring still promises both. Update the docstring to state that these recovery paths apply only to the default daemon.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
|
|
||
| import subprocess, sys | ||
| local = _is_local_chrome_mode(env) | ||
| local = _is_local_chrome_mode(env, name) |
Contributor
There was a problem hiding this comment.
P3: For named daemons, this sets local false and skips Chrome launch and chrome://inspect recovery, but the function docstring still promises both. Update the docstring to state that these recovery paths apply only to the default daemon.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/browser_harness/admin.py, line 356:
<comment>For named daemons, this sets `local` false and skips Chrome launch and `chrome://inspect` recovery, but the function docstring still promises both. Update the docstring to state that these recovery paths apply only to the default daemon.</comment>
<file context>
@@ -351,11 +353,11 @@ def ensure_daemon(wait=60.0, name=None, env=None):
import subprocess, sys
- local = _is_local_chrome_mode(env)
+ local = _is_local_chrome_mode(env, name)
launched_browser = False
opened_inspect = False
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #479.
A start_remote_daemon(name) child starts with BU_NAME plus BU_CDP_WS and BU_BROWSER_ID. The documented later invocation carries only BU_NAME. If the daemon has died, ensure_daemon() respawns it without the endpoint, the child is classified as local, and get_ws_url() can scan default Chrome profiles and probe ports 9222/9223. admin.py can then launch local Chrome or open chrome://inspect, allowing a managed daemon to attach to the user's daily browser.
Change
This uses the existing BU_NAME semantics: the initial multi-daemon history describes named daemons as independent remote daemons, while the default daemon remains the local Chrome path. No new flag, supervisor, browser manager, or persistence layer is introduced.
Regression coverage
Validation
Summary by cubic
Named (non-default) daemons now fail closed if respawned without an explicit CDP endpoint, instead of falling back to local Chrome discovery. This prevents managed daemons from attaching to a user’s local browser and clarifies the default daemon as the only local-discovery path.
get_ws_url()now raises forBU_NAME != "default"whenBU_CDP_WS/BU_CDP_URLare absent;_is_local_chrome_mode()enforces this.ensure_daemon()no longer launches Chrome or openschrome://inspectfor named daemon failures and now ensures the requestednameoverrides any childBU_NAME.run._run()skips local probes and cloud bootstrap for named daemons before strict ensure.Migration
BU_CDP_WSorBU_CDP_URL, or callstart_remote_daemon(name)to recreate the endpoint;BU_NAME-only invocations now fail with an explicit error.BU_CDP_WS/BU_CDP_URLare set.Written for commit 4c16f16. Summary will update on new commits.