Two related issues that have shown up across recent PRs:
1. press_key always emitted a `char` event after every single-char
keyDown, regardless of modifiers. With Ctrl/Cmd held, that `char`
makes Chrome treat the press as printable text input (typing "a")
instead of firing the shortcut (Cmd+A). PR #258 worked around this
inside fill_input by dispatching the select-all directly via raw
CDP calls, but every other shortcut was still broken.
2. The keyDown's `code` and `windowsVirtualKeyCode` for letters/digits
came from a literal-key fallback (code="a", vk=ord("a")=97). CDP's
shortcut handlers compare against canonical physical-key codes —
"KeyA" / 65 for the A key, "Digit5" / 53 for the 5 key. Without
that, e.code in JS is wrong and shortcut listeners that check
`e.code === "KeyA"` (a common pattern) do not fire.
Fix at source so every shortcut works for any caller:
- Added _key_metadata(key) which returns the canonical (vk, code, text)
for letters (Key{X}, ord(upper)), digits (Digit{N}, ord(N)), and the
pre-existing special-key table. Punctuation/symbols fall back to
ASCII vk + literal code.
- press_key suppresses both `text` on keyDown and the entire `char`
event when any of Alt/Ctrl/Meta is set (modifier bits 0b0111).
Shift alone is still text input.
- fill_input's clear path now just calls press_key("a", modifiers=...)
instead of dispatching directly via cdp; the helper does the right
thing now.
10 new tests in tests/unit/test_helpers.py cover:
- canonical code/vk for letters (KeyA/65, KeyZ/90) and digits (Digit5/53)
- Enter/Backspace/etc still use the _KEYS table
- no-modifier press emits text + char
- Ctrl / Meta / Alt each suppress text + char
- Shift alone keeps text + char
- Ctrl+Shift combo suppresses (modifier wins over Shift)
- keyUp metadata is consistent
Identified via codex review (P1). Full suite: 93 passed (83 -> 93).
codex flagged that set_session was now performing up to four sequential
domain enables (5s timeout each) plus a 2s Network.disable, so a slow
or remote daemon could block the synchronous IPC reply for ~22s while
the helper's _send() socket has only a 5s read timeout. Old code only
awaited Page.enable (3s) on this path.
Three changes to keep the reply under the IPC deadline:
1. _enable_default_domains now awaits asyncio.gather over the four
Domain.enable coroutines instead of looping sequentially. Per-call
timeout reduced from 5s to 4s. Worst case is bounded by a single
CDP round trip rather than four.
2. set_session schedules the old-session Network.disable in the same
gather as the four enables on the new session — independent CDP
sessions, no ordering required for correctness (the consumer-side
filter in wait_for_network_idle is the actual correctness gate).
3. The 🟢 tab-marker title-prefix Runtime.evaluate is now fire-and-forget
via asyncio.create_task (+ _silent). It's purely cosmetic; agents
shouldn't wait on it.
Worst-case set_session reply time: max(2s disable, 4s parallel enables)
≈ 4s, comfortably under the 5s IPC timeout. Normal case on a remote
daemon drops from ~800ms (4 sequential round trips) to ~200ms (1 round
trip in parallel).
Two new tests in tests/unit/test_daemon.py use a fake CDP whose
send_raw blocks on an asyncio.Event, then assert the peak in-flight
call count:
- with a previous session: peak == 5 (1 disable + 4 enables)
- first attach: peak == 4 (4 enables, disable skipped)
Sequential await would peak at 1 on both. Full suite: 65 passed.
codex flagged a P2 follow-on to the just-added Network.enable: my fix
reliably enables Network on every fresh session, but old sessions
never get Network disabled (helpers.switch_tab attaches without
detaching), and wait_for_network_idle reads from the daemon's global
drain_events stream without filtering by session_id. Net effect: an
agent that visited a polling/SSE tab and switched away would observe
that background tab's traffic in a later wait, either timing out or
waiting on the wrong tab's requests.
Two narrow fixes:
1. helpers.wait_for_network_idle now captures the active session at
the start of the wait and skips events whose session_id doesn't
match. That's the consumer-side root-cause fix.
2. daemon set_session now calls Network.disable on the previous
session before enabling on the new one. Defense in depth — keeps
the daemon's event buffer from filling with background-tab noise
in the first place. Best-effort with its own 2s timeout; failure
doesn't abort the rest of the handler.
Tests added:
- tests/unit/test_helpers.py: events from a background session are
ignored; the active session can reach idle even when the
background session is busy.
- tests/unit/test_daemon.py: set_session disables Network on the old
session; first set_session call (no prior session) does not call
Network.disable.
Full suite: 63 passed (60 -> 63).
Prior to this change, the daemon's set_session meta-handler only called
Page.enable on the new CDP session. The initial-attach path enabled all
four of Page/DOM/Runtime/Network. set_session is what backs switch_tab()
and new_tab() in helpers.py, so any helper that depends on Network
events — most notably the wait_for_network_idle() that just landed in
PR #258 — silently stopped receiving events after a tab switch.
Refactored: extracted the domain-enable loop into a private
_enable_default_domains(session_id) helper used by both attach_first_page
and the set_session handler. Each domain is enabled with its own
timeout, and a single failure does not abort the others.
Also tightened the target_id fallback semantics: if a caller passes
target_id=None on set_session, the daemon keeps its existing target_id
rather than overwriting with None (preserves the existing 'or' fallback,
just covered by a test now).
Tests in new tests/unit/test_daemon.py drive Daemon.handle() directly
with a fake CDP client and assert:
- set_session enables all four default domains on the new session
(the regression — would fail against the old single-Page.enable code)
- target_id is preserved when caller passes None
- _enable_default_domains attempts every domain even when one raises
Identified via codex review (P1). Full suite: 60 passed.
codex flagged that the SIGTERM gate could never pass on Windows because
_process_start_time() returned None there, while ipc.identify() also
returns None during the slow-shutdown window (the daemon's serve()
tears down the IPC endpoint before stop_remote() runs its 15s-timeout
PATCH to api.browser-use.com). Combined: a Windows user with a remote
daemon would lose force-kill, leaving an orphan that may keep a billed
cloud browser alive.
Implemented the Windows branch via ctypes:
OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, pid)
GetProcessTimes -> creation FILETIME (100-ns intervals since 1601)
CloseHandle on the way out
The combined high/low DWORDs of the creation FILETIME serve as the
opaque fingerprint. PROCESS_QUERY_LIMITED_INFORMATION is sufficient
for GetProcessTimes (Vista+) and avoids needing PROCESS_QUERY_INFORMATION
which is more privilege-sensitive.
Also tightened the top guard from isinstance(pid, int) to
type(pid) is int — bool subclasses int in Python and was previously
slipping through (though the f-string formatting in the Linux branch
saved us by hitting /proc/True/stat which doesn't exist).
Test coverage: extended test_process_start_time_returns_stable_fingerprint_for_self
to also run on win32. Full suite: 75 passed locally on macOS.
Two more codex findings on ad39a95, both real:
1. _ipc.identify() accepted any positive int as pid, including values
too large for C pid_t (typically signed 32-bit). os.kill(huge_pid, 0)
then raises OverflowError, which propagates out of restart_daemon()
before its cleanup runs. Bounded the accepted range to 0 < pid <
2**31. Linux pid_max is also <2**22 in practice. Defense-in-depth:
added OverflowError to the except lists wrapping the os.kill calls
in restart_daemon, in case a different code path ever feeds it a
too-large pid.
2. The daemon's serve() tears down the IPC socket BEFORE the daemon
process actually exits — the daemon then runs slow cleanup work
(notably stop_remote()'s 15s-timeout PATCH to api.browser-use.com
to release a cloud browser). During that window identify() returns
None even though the process is still our daemon, so the strict
identify-only re-verification skipped SIGTERM and let the orphan
keep running. The next bh invocation would then spawn a new daemon
competing with the old one for the same Chrome.
Fix: snapshot a process-start-time fingerprint at the top of
restart_daemon, and accept it as a secondary identity signal in
the SIGTERM gate. Two reads returning the same fingerprint means
the PID still refers to the same process; a different fingerprint
means PID reuse, in which case SIGTERM is skipped (preserving the
protection from cubic's earlier finding). Implementation uses
/proc/<pid>/stat field 22 on Linux and ps -o lstart= on macOS;
on Windows / unsupported platforms the helper returns None and
restart_daemon falls back to the strict identify-only check.
Tests in tests/unit/test_admin.py and tests/unit/test_ipc.py:
- restart_daemon SIGTERMs via start-time match when socket is gone
(slow-shutdown recovery)
- restart_daemon skips SIGTERM when start-time has changed (PID
reuse during the wait window)
- _process_start_time returns a stable fingerprint for the current
process and None for invalid pids (None/0/negatives/non-int/dead)
- identify rejects oversized ints (covered via the bounded check)
Full suite: 75 passed.
Two additional codex findings on 6d412c9:
1. _ipc.identify() accepted any int as pid, including 0 and negatives.
On POSIX, os.kill(0, sig) signals every process in the calling
process group, and os.kill(-1, sig) signals every process the
caller can. A hostile or buggy daemon replying {pid: 0} or
{pid: -1} would have turned restart_daemon() into a process-group
kill. Restrict to pid > 0.
2. _ipc.ping() still did resp.get('pong') without a type check, so a
list/scalar/null reply would raise AttributeError. The previous
commit added that guard to identify() but left ping() bare. With
the new daemon_alive fallback in restart_daemon() that now calls
ping(), an unhandled raise here would abort restart before
cleanup ran. Mirrored the identify() guards: isinstance(resp, dict)
plus AttributeError in the except.
Tests in tests/unit/test_ipc.py now also cover:
- identify() rejects pid=0, pid=-1, pid=-42, pid=-99999
- ping() returns False for non-dict payloads (list/str/int/None)
- ping() requires pong is exactly True (rejects truthy non-True values)
Full suite: 71 passed.
Two related cubic/codex findings on _ipc.identify():
1. isinstance(pid, int) accepts bool (since bool subclasses int in
Python), so a hostile or buggy daemon replying {pid: True} would
yield PID 1 and os.kill(1, SIGTERM) would target init on POSIX.
Switched to type(pid) is int — strict same-type check, no subclass
surprises.
2. request() returns whatever JSON the daemon sent, which can be a
list, scalar, or None for a stale/hostile endpoint. resp.get(...)
would then raise AttributeError, propagating out of identify() and
crashing restart_daemon() before its cleanup runs. Added an
isinstance(resp, dict) guard and AttributeError to the except.
Tests in new tests/unit/test_ipc.py cover both rejections plus the
happy path, the missing-pid path (pre-upgrade daemon), pong=False,
and several non-dict shapes (list, str, int, None). Full suite: 67
passed.
Two issues raised on the initial commit:
1. Pre-upgrade daemons that don't include 'pid' in their ping reply made
identify() return None, which short-circuited the entire shutdown
path. The function would still delete the socket and pid file,
orphaning the still-running daemon. Fix: keep daemon_pid (verified
PID, used for signaling) separate from daemon_alive (any pong reply
counts, falls back to ipc.ping for the alive check). Shutdown IPC
is now sent whenever daemon_alive is true, regardless of whether
we got a verifiable PID; SIGTERM still requires a verifiable PID.
2. A single identify() at the top didn't prevent PID reuse during
the 15-second wait loop. If the daemon exited and the kernel
reused its PID before the loop timed out, SIGTERM would land on
the new owner. Fix: re-call identify() right before SIGTERM and
only signal if it still returns the same PID we've been waiting
on — any other state (None, different PID) means PID reuse is
possible and we skip the kill.
Two new tests in tests/unit/test_admin.py lock in both:
- test_restart_daemon_sends_shutdown_to_pre_upgrade_daemon_without_pid_in_ping
- test_restart_daemon_skips_sigterm_if_pid_was_reused_during_wait
Existing tests updated to also stub ipc.ping. Full suite: 61 passed.
The previous restart_daemon() read the daemon's PID from the
/tmp/<name>.pid file and could end up calling os.kill(pid, SIGTERM)
on whatever process now owned that PID — a real hazard whenever the
daemon had crashed without removing the file and the OS subsequently
reused the PID for an unrelated local process (browser, editor,
language server, ...). The 15-second polling loop made the issue
worse: any time os.kill(pid, 0) kept succeeding (because the new
owner stayed alive), the loop would fall through to SIGTERM.
Fix: ask the live daemon for its PID over the existing IPC ping
channel before signaling anything. The 'ping' meta-handler now
returns {pong: True, pid: os.getpid()}, and a new ipc.identify(name)
helper returns that PID or None if the daemon is unreachable.
restart_daemon() now signals only the PID returned by identify(),
never the contents of the pid file. If the daemon is unreachable we
just clean up the socket and pid file and return — we never escalate
to a kill-by-pid-file.
Adds two unit tests in tests/unit/test_admin.py:
- the unreachable-daemon path performs zero os.kill calls (the
exact regression: previously the stale pid file would still get
signaled);
- when identify() returns a different PID than the pid file, only
the identify-supplied PID is signaled.
Identified via codex code review (P0). All 59 unit tests pass.
urlparse(...).hostname strips the [] from IPv6 hosts, so building
ws://{host}:{port}{path} produced ws://::1:9333/... which is malformed.
Restore the brackets when the resolved host contains ':'.
- Move from top-level domain-skills/tasksquad.ai/ to the canonical
agent-workspace/domain-skills/tasksquad-ai/ location, switching to
kebab-case to match sibling dirs (booking-com, dev-to, archive-org).
- Replace find_text_coords(...) with self-contained inline Python
helpers (_coords_by_text, _coords_by_selector) that wrap querySelector
+ getBoundingClientRect. find_text_coords does not exist in the
harness or agent_helpers — every call site was a NameError waiting to
happen.
- Fix the compose-task snippet: drop the dead subject_input = js(...)
line, focus the subject input via _coords_by_selector before typing,
and add a placeholder click for the agent dropdown item.
- Fix the polling-interval contradiction: the inbox section now defers
to the gotchas, and the gotchas list the Free (5s) / Pro (2s) cadence
in one place.
- Move agent-workspace/domain-skills/x.com/ to .../x/ to match repo
convention (siblings use bare platform names like 'tiktok' / 'youtube'
or kebab-case like 'booking-com'; nothing else uses a literal dot in
a dir name).
- Guard the compose-textarea and post-button querySelector lookups so
the snippet raises a clear RuntimeError if the elements aren't there
yet, instead of throwing a confusing JS TypeError on
el.getBoundingClientRect().
PROFILES already covers Brave on macOS (Library/Application Support/
BraveSoftware/Brave-Browser) and Linux Flatpak (.var/app/com.brave.Browser/...)
but is missing the standard Windows install path. As a result, "Way 1"
(chrome://inspect/#remote-debugging) discovery fails on Windows + Brave with
"DevToolsActivePort not found" even when remote debugging is enabled.
Add Path.home() / "AppData/Local/BraveSoftware/Brave-Browser/User Data" to
match the macOS/Linux Brave entries already present.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds auth, task inbox, and agent management playbooks for tasksquad.ai —
field-tested against the live site and source code of the React SPA.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Cross-checked AGENTS.md, README.md, SKILL.md, install.md, and profile-sync.md for accuracy. Tightened the sync_local_profile docstring and the chrome://inspect stderr message to match. Cloud-bootstrap test updated to set BU_AUTOSPAWN. All unit tests pass.
If an agent runs the interceptor snippet before GTM has initialized,
window.dataLayer is undefined and .push.bind throws. Seeding with [] is
safe — GTM picks up a pre-existing array on init.
Saurav Panda