feat: add daemon identity to tab markers - #605
Conversation
✅ Skill review passedReviewed 1 file(s) — no findings. |
There was a problem hiding this comment.
4 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="tests/unit/test_daemon.py">
<violation number="1" location="tests/unit/test_daemon.py:27">
P3: The new test's three assertions are tautological — they look for string fragments (`daemon.NAME`, `"clean + suffix"`, `"document.title"`) that the implementation builds verbatim from the same constants, so they can never fail while `_tab_marker_expression()` is implemented as shown. They don't validate the marker behavior this PR introduces. In particular `daemon.NAME in expression` passes even if the function hardcoded the daemon name instead of using `NAME`, because `NAME` is already baked into the `TAB_MARKER` constant the expression interpolates. Strengthen the test to evaluate the generated script against a stubbed `document` (set `document.title`, run the IIFE, assert the resulting title embeds `[NAME]` as the suffix), so it actually guards the appends-daemon-name behavior rather than mirroring the source.</violation>
</file>
<file name="src/browser_harness/daemon.py">
<violation number="1" location="src/browser_harness/daemon.py:42">
P3: daemon.py redefines TAB_MARKER, TAB_MARKER_SUFFIX and _tab_marker_expression() verbatim from helpers.py lines 41-58, including the same regex-laden JS string. The two copies will drift (indeed switch_tab in daemon already diverges from helpers' format), and BU_NAME changes affect both simultaneously. Extract the marker constants and expression into a shared module (e.g. _ipc or paths, both already imported by daemon.py) that both daemon.py and helpers.py import, rather than duplicating.</violation>
</file>
<file name="src/browser_harness/helpers.py">
<violation number="1" location="src/browser_harness/helpers.py:54">
P2: Marking a page with an empty title is not idempotent. `_tab_marker_expression()` sets `document.title` to the bare `marker` (`🐴 [NAME]`, no ` | ` prefix) when `clean` is falsy, but on the next application the cleanup only strips a trailing ` | 🐴 [name]` suffix or a leading `🐴 ` prefix. Re-applying to `🐴 [NAME]` yields `clean = "[NAME]"` (truthy), then `[NAME] | 🐴 [NAME]` — a doubled marker. The daemon re-applies this expression on every `Page.loadEventFired`/`domContentEventFired`, so any page that resolves to an empty title (e.g. about:blank) converges to the doubled form. Make the empty-title branch use the same suffix-form so it round-trips, e.g. always `document.title = clean + suffix;`.</violation>
<violation number="2" location="src/browser_harness/helpers.py:312">
P2: The new suffix-form marker in `_mark_tab()` races with the daemon's `set_session` handler, which still applies the old prefix-form marker (`if(!document.title.startsWith('🐴'))document.title='🐴 '+document.title`) as a fire-and-forget task. In `switch_tab`, `set_session` and `_mark_tab` both run against the newly attached tab; if `_mark_tab`'s suffix lands first, the daemon's prefix task then sees a title that doesn't start with 🐴 and prepends, producing `🐴 <title> | 🐴 [NAME]` (doubled marker). Update the daemon's `set_session` marker to the new suffix form so both paths agree.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| """Prepend horse emoji to tab title so the user can see which tab the agent controls.""" | ||
| try: cdp("Runtime.evaluate", expression="if(!document.title.startsWith('\U0001F434'))document.title='\U0001F434 '+document.title") | ||
| """Append the daemon identity to the title of the controlled tab.""" | ||
| try: cdp("Runtime.evaluate", expression=_tab_marker_expression()) |
There was a problem hiding this comment.
P2: The new suffix-form marker in _mark_tab() races with the daemon's set_session handler, which still applies the old prefix-form marker (if(!document.title.startsWith('🐴'))document.title='🐴 '+document.title) as a fire-and-forget task. In switch_tab, set_session and _mark_tab both run against the newly attached tab; if _mark_tab's suffix lands first, the daemon's prefix task then sees a title that doesn't start with 🐴 and prepends, producing 🐴 <title> | 🐴 [NAME] (doubled marker). Update the daemon's set_session marker to the new suffix form so both paths agree.
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 312:
<comment>The new suffix-form marker in `_mark_tab()` races with the daemon's `set_session` handler, which still applies the old prefix-form marker (`if(!document.title.startsWith('🐴'))document.title='🐴 '+document.title`) as a fire-and-forget task. In `switch_tab`, `set_session` and `_mark_tab` both run against the newly attached tab; if `_mark_tab`'s suffix lands first, the daemon's prefix task then sees a title that doesn't start with 🐴 and prepends, producing `🐴 <title> | 🐴 [NAME]` (doubled marker). Update the daemon's `set_session` marker to the new suffix form so both paths agree.</comment>
<file context>
@@ -287,17 +308,16 @@ def current_tab():
- """Prepend horse emoji to tab title so the user can see which tab the agent controls."""
- try: cdp("Runtime.evaluate", expression="if(!document.title.startsWith('\U0001F434'))document.title='\U0001F434 '+document.title")
+ """Append the daemon identity to the title of the controlled tab."""
+ try: cdp("Runtime.evaluate", expression=_tab_marker_expression())
except Exception: pass
</file context>
| const clean = document.title | ||
| .replace(/\\s*\\|\\s*🐴\\s*\\[[^\\]]+\\]\\s*$/, \"\") | ||
| .replace(/^🐴\\s*/, \"\"); | ||
| document.title = clean ? clean + suffix : marker; |
There was a problem hiding this comment.
P2: Marking a page with an empty title is not idempotent. _tab_marker_expression() sets document.title to the bare marker (🐴 [NAME], no | prefix) when clean is falsy, but on the next application the cleanup only strips a trailing | 🐴 [name] suffix or a leading 🐴 prefix. Re-applying to 🐴 [NAME] yields clean = "[NAME]" (truthy), then [NAME] | 🐴 [NAME] — a doubled marker. The daemon re-applies this expression on every Page.loadEventFired/domContentEventFired, so any page that resolves to an empty title (e.g. about:blank) converges to the doubled form. Make the empty-title branch use the same suffix-form so it round-trips, e.g. always document.title = clean + suffix;.
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 54:
<comment>Marking a page with an empty title is not idempotent. `_tab_marker_expression()` sets `document.title` to the bare `marker` (`🐴 [NAME]`, no ` | ` prefix) when `clean` is falsy, but on the next application the cleanup only strips a trailing ` | 🐴 [name]` suffix or a leading `🐴 ` prefix. Re-applying to `🐴 [NAME]` yields `clean = "[NAME]"` (truthy), then `[NAME] | 🐴 [NAME]` — a doubled marker. The daemon re-applies this expression on every `Page.loadEventFired`/`domContentEventFired`, so any page that resolves to an empty title (e.g. about:blank) converges to the doubled form. Make the empty-title branch use the same suffix-form so it round-trips, e.g. always `document.title = clean + suffix;`.</comment>
<file context>
@@ -38,6 +38,27 @@ def _load_env_file(p):
+ const clean = document.title
+ .replace(/\\s*\\|\\s*🐴\\s*\\[[^\\]]+\\]\\s*$/, \"\")
+ .replace(/^🐴\\s*/, \"\");
+ document.title = clean ? clean + suffix : marker;
+ }})()"""
+
</file context>
| document.title = clean ? clean + suffix : marker; | |
| document.title = clean + suffix; |
| def test_daemon_tab_marker_appends_dynamic_name(): | ||
| expression = daemon._tab_marker_expression() | ||
|
|
||
| assert daemon.NAME in expression |
There was a problem hiding this comment.
P3: The new test's three assertions are tautological — they look for string fragments (daemon.NAME, "clean + suffix", "document.title") that the implementation builds verbatim from the same constants, so they can never fail while _tab_marker_expression() is implemented as shown. They don't validate the marker behavior this PR introduces. In particular daemon.NAME in expression passes even if the function hardcoded the daemon name instead of using NAME, because NAME is already baked into the TAB_MARKER constant the expression interpolates. Strengthen the test to evaluate the generated script against a stubbed document (set document.title, run the IIFE, assert the resulting title embeds [NAME] as the suffix), so it actually guards the appends-daemon-name behavior rather than mirroring the source.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/unit/test_daemon.py, line 27:
<comment>The new test's three assertions are tautological — they look for string fragments (`daemon.NAME`, `"clean + suffix"`, `"document.title"`) that the implementation builds verbatim from the same constants, so they can never fail while `_tab_marker_expression()` is implemented as shown. They don't validate the marker behavior this PR introduces. In particular `daemon.NAME in expression` passes even if the function hardcoded the daemon name instead of using `NAME`, because `NAME` is already baked into the `TAB_MARKER` constant the expression interpolates. Strengthen the test to evaluate the generated script against a stubbed `document` (set `document.title`, run the IIFE, assert the resulting title embeds `[NAME]` as the suffix), so it actually guards the appends-daemon-name behavior rather than mirroring the source.</comment>
<file context>
@@ -21,6 +21,14 @@ def _fresh_daemon():
+def test_daemon_tab_marker_appends_dynamic_name():
+ expression = daemon._tab_marker_expression()
+
+ assert daemon.NAME in expression
+ assert "clean + suffix" in expression
+ assert "document.title" in expression
</file context>
| TAB_MARKER_SUFFIX = f" | {TAB_MARKER}" | ||
|
|
||
|
|
||
| def _tab_marker_expression(): |
There was a problem hiding this comment.
P3: daemon.py redefines TAB_MARKER, TAB_MARKER_SUFFIX and _tab_marker_expression() verbatim from helpers.py lines 41-58, including the same regex-laden JS string. The two copies will drift (indeed switch_tab in daemon already diverges from helpers' format), and BU_NAME changes affect both simultaneously. Extract the marker constants and expression into a shared module (e.g. _ipc or paths, both already imported by daemon.py) that both daemon.py and helpers.py import, rather than duplicating.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/browser_harness/daemon.py, line 42:
<comment>daemon.py redefines TAB_MARKER, TAB_MARKER_SUFFIX and _tab_marker_expression() verbatim from helpers.py lines 41-58, including the same regex-laden JS string. The two copies will drift (indeed switch_tab in daemon already diverges from helpers' format), and BU_NAME changes affect both simultaneously. Extract the marker constants and expression into a shared module (e.g. _ipc or paths, both already imported by daemon.py) that both daemon.py and helpers.py import, rather than duplicating.</comment>
<file context>
@@ -35,6 +35,21 @@ def _load_env_file(p):
+TAB_MARKER_SUFFIX = f" | {TAB_MARKER}"
+
+
+def _tab_marker_expression():
+ marker = json.dumps(TAB_MARKER)
+ suffix = json.dumps(TAB_MARKER_SUFFIX)
</file context>
Fixes #604
Summary
BU_NAMEto the horse tab marker;Scope
This change labels the currently attached worker tab. It does not introduce tab locks, profile management, or focus-preservation behavior.
Verification
uv run --with pytest pytest -q— 117 passedgit diff --checkpassedSummary by cubic
Adds daemon identity to controlled tab titles so parallel daemons are easy to tell apart. Previously we prefixed a horse emoji; now we append " | 🐴 [BU_NAME]", keep it across navigations, and remove/update it when switching targets.
_tab_marker_expressionand_tab_unmarker_expressionand used bydaemon.start()andhelpers._mark_tab()/switch_tab().BU_NAMEvalues when running multiple daemons; the harness does not add a shared prefix.document.titleto tolerate the new suffix.Written for commit f690adb. Summary will update on new commits.