Keep agent tab switching in the background - #606
Conversation
⛔ Skill review blockedAn automated security review found 2 finding(s) across 1 file(s).
Skill authorship is restricted to maintainers. Please do not attempt to self-fix — a maintainer will review and follow up. |
There was a problem hiding this comment.
2 issues found across 5 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/helpers.py">
<violation number="1" location="src/browser_harness/helpers.py:301">
P3: The new `activate_tab()` duplicates the dict-or-string targetId extraction that already exists in `switch_tab()` (line 314) and `close_tab()` (line 353). Extract this into a small helper (e.g. `_target_id(target)`) and reuse it in all three functions so the accepted-input contract (raw ID string or the dict from `current_tab()`/`list_tabs()`) stays consistent in one place.</violation>
<violation number="2" location="src/browser_harness/helpers.py:305">
P1: `new_tab()` can still foreground Chrome despite the new background-only default. Pass `background=True` to `Target.createTarget` so tab creation stays in the background; callers can use `activate_tab()` when visibility is explicitly required.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| cdp("Target.activateTarget", targetId=target_id) | ||
| return target_id | ||
|
|
||
| def switch_tab(target, activate=False): |
There was a problem hiding this comment.
P1: new_tab() can still foreground Chrome despite the new background-only default. Pass background=True to Target.createTarget so tab creation stays in the background; callers can use activate_tab() when visibility is explicitly required.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/browser_harness/helpers.py, line 305:
<comment>`new_tab()` can still foreground Chrome despite the new background-only default. Pass `background=True` to `Target.createTarget` so tab creation stays in the background; callers can use `activate_tab()` when visibility is explicitly required.</comment>
<file context>
@@ -291,15 +291,33 @@ def _mark_tab():
+ cdp("Target.activateTarget", targetId=target_id)
+ return target_id
+
+def switch_tab(target, activate=False):
+ """Attach the agent to a target without changing Chrome's visible tab.
+
</file context>
| target does not require taking Chrome or the user's current application to | ||
| the foreground. | ||
| """ | ||
| target_id = (target.get("targetId") or target.get("target_id")) if isinstance(target, dict) else target |
There was a problem hiding this comment.
P3: The new activate_tab() duplicates the dict-or-string targetId extraction that already exists in switch_tab() (line 314) and close_tab() (line 353). Extract this into a small helper (e.g. _target_id(target)) and reuse it in all three functions so the accepted-input contract (raw ID string or the dict from current_tab()/list_tabs()) stays consistent in one place.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/browser_harness/helpers.py, line 301:
<comment>The new `activate_tab()` duplicates the dict-or-string targetId extraction that already exists in `switch_tab()` (line 314) and `close_tab()` (line 353). Extract this into a small helper (e.g. `_target_id(target)`) and reuse it in all three functions so the accepted-input contract (raw ID string or the dict from `current_tab()`/`list_tabs()`) stays consistent in one place.</comment>
<file context>
@@ -291,15 +291,33 @@ def _mark_tab():
+ target does not require taking Chrome or the user's current application to
+ the foreground.
+ """
+ target_id = (target.get("targetId") or target.get("target_id")) if isinstance(target, dict) else target
+ cdp("Target.activateTarget", targetId=target_id)
+ return target_id
</file context>
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
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="SKILL.md">
<violation number="1" location="SKILL.md:84">
P2: This example adds a second `start_remote_daemon("r7k2")` call after the existing default one, but the two cannot run in sequence. `start_remote_daemon` raises `RuntimeError("daemon 'r7k2' already alive")` when the daemon already exists, so an agent following the three heredoc blocks literally runs the first (which opens the live viewer locally, contradicting the "fully backgrounded" intent) and then errors on the second. Make the blocks alternatives or keep only the `openLiveUrl=False` variant as the recommended backgrounded flow.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
|
|
||
| # Keep the cloud session fully backgrounded; this prints liveUrl without opening it locally. | ||
| browser-harness <<'PY' | ||
| start_remote_daemon("r7k2", openLiveUrl=False) |
There was a problem hiding this comment.
P2: This example adds a second start_remote_daemon("r7k2") call after the existing default one, but the two cannot run in sequence. start_remote_daemon raises RuntimeError("daemon 'r7k2' already alive") when the daemon already exists, so an agent following the three heredoc blocks literally runs the first (which opens the live viewer locally, contradicting the "fully backgrounded" intent) and then errors on the second. Make the blocks alternatives or keep only the openLiveUrl=False variant as the recommended backgrounded flow.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At SKILL.md, line 84:
<comment>This example adds a second `start_remote_daemon("r7k2")` call after the existing default one, but the two cannot run in sequence. `start_remote_daemon` raises `RuntimeError("daemon 'r7k2' already alive")` when the daemon already exists, so an agent following the three heredoc blocks literally runs the first (which opens the live viewer locally, contradicting the "fully backgrounded" intent) and then errors on the second. Make the blocks alternatives or keep only the `openLiveUrl=False` variant as the recommended backgrounded flow.</comment>
<file context>
@@ -79,6 +79,11 @@ browser-harness <<'PY'
+# Keep the cloud session fully backgrounded; this prints liveUrl without opening it locally.
+browser-harness <<'PY'
+start_remote_daemon("r7k2", openLiveUrl=False)
+PY
+
</file context>
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
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:623">
P3: The new `openLiveUrl` bullet is listed inside the docstring block headed "kwargs forwarded to `POST /browsers` (camelCase):", but `openLiveUrl` is a local Python parameter that is consumed by this function and never forwarded in the request body (every other bullet in that block is a real POST field). A reader could mistake it for an API field. Move the `openLiveUrl` description out of the forwarded-kwargs block and clarify it is a local behavior switch.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
| timeout — minutes, 1..240. | ||
| customProxy — {host, port, username, password, ignoreCertErrors}. | ||
| browserScreenWidth / browserScreenHeight, allowResizing, enableRecording. | ||
| openLiveUrl — open the cloud live viewer locally (default True). Set |
There was a problem hiding this comment.
P3: The new openLiveUrl bullet is listed inside the docstring block headed "kwargs forwarded to POST /browsers (camelCase):", but openLiveUrl is a local Python parameter that is consumed by this function and never forwarded in the request body (every other bullet in that block is a real POST field). A reader could mistake it for an API field. Move the openLiveUrl description out of the forwarded-kwargs block and clarify it is a local behavior switch.
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 623:
<comment>The new `openLiveUrl` bullet is listed inside the docstring block headed "kwargs forwarded to `POST /browsers` (camelCase):", but `openLiveUrl` is a local Python parameter that is consumed by this function and never forwarded in the request body (every other bullet in that block is a real POST field). A reader could mistake it for an API field. Move the `openLiveUrl` description out of the forwarded-kwargs block and clarify it is a local behavior switch.</comment>
<file context>
@@ -620,9 +620,12 @@ def start_remote_daemon(name="remote", profileName=None, **create_kwargs):
timeout — minutes, 1..240.
customProxy — {host, port, username, password, ignoreCertErrors}.
browserScreenWidth / browserScreenHeight, allowResizing, enableRecording.
+ openLiveUrl — open the cloud live viewer locally (default True). Set
+ False to keep the local browser completely untouched.
</file context>
Summary
switch_tab(),new_tab(), andensure_real_tab()background-only by default.activate_tab()andswitch_tab(..., activate=True)for explicit visible switching.start_remote_daemon(..., openLiveUrl=False)for cloud sessions that must not open or foreground a local browser.Motivation
On macOS, CDP
Target.activateTargetchanges Chrome's selected tab and reactivates the owning Chrome window. Chrome/macOS may restore that window's frame near the top of the screen.Target.attachToTarget, session setup, and the horse marker work without activating Chrome, so users can switch to the marked tab manually.Cloud sessions avoid local Chrome entirely; the only local focus path was the optional
liveUrlviewer opened bywebbrowser.open().Validation
uv run --with pytest pytest -q— 117 passedTarget.activateTargetforegrounded Chrome; attach/evaluate did not.