Calling Target.getTargetInfo from helpers can't work: the daemon strips
session_id for any Target.* method, so the call hit the browser-level
connection with no targetId, and Chrome silently returned info about the
*browser* target (empty url/title) instead of the attached page. New
current_tab meta uses self.target_id like connection_status already does.
Fixes#304.
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.
Hunnyboy1217