fix(tui): stream /browser connect progress as gateway events
Emit browser.progress JSON-RPC notifications during the connect work and render them in the TUI as system transcript lines, so users see the same step-by-step status the base CLI prints instead of nothing for ~1m followed by a final result.
This commit is contained in:
committed by
Teknium
parent
7d39a45749
commit
e750829015
@@ -2754,6 +2754,8 @@ def test_session_most_recent_handles_db_unavailable(monkeypatch):
|
||||
)
|
||||
|
||||
assert resp["result"]["session_id"] is None
|
||||
|
||||
|
||||
# ── browser.manage ───────────────────────────────────────────────────
|
||||
|
||||
|
||||
@@ -2903,14 +2905,27 @@ def test_browser_manage_connect_defaults_to_loopback(monkeypatch):
|
||||
|
||||
def test_browser_manage_connect_default_local_reports_launch_hint(monkeypatch):
|
||||
monkeypatch.delenv("BROWSER_CDP_URL", raising=False)
|
||||
emitted: list[tuple[str, dict]] = []
|
||||
monkeypatch.setattr(
|
||||
server,
|
||||
"_emit",
|
||||
lambda evt, sid, payload=None: emitted.append((evt, payload or {})),
|
||||
)
|
||||
fake = types.SimpleNamespace(
|
||||
cleanup_all_browsers=lambda: None,
|
||||
_get_cdp_override=lambda: os.environ.get("BROWSER_CDP_URL", ""),
|
||||
)
|
||||
with patch.dict(sys.modules, {"tools.browser_tool": fake}):
|
||||
_stub_urlopen(monkeypatch, ok=False)
|
||||
with patch("hermes_cli.browser_connect.try_launch_chrome_debug", return_value=False), \
|
||||
patch("hermes_cli.browser_connect.get_chrome_debug_candidates", return_value=[]):
|
||||
with (
|
||||
patch(
|
||||
"hermes_cli.browser_connect.try_launch_chrome_debug", return_value=False
|
||||
),
|
||||
patch(
|
||||
"hermes_cli.browser_connect.get_chrome_debug_candidates",
|
||||
return_value=[],
|
||||
),
|
||||
):
|
||||
resp = server.handle_request(
|
||||
{
|
||||
"id": "1",
|
||||
@@ -2921,10 +2936,20 @@ def test_browser_manage_connect_default_local_reports_launch_hint(monkeypatch):
|
||||
|
||||
assert resp["result"]["connected"] is False
|
||||
assert resp["result"]["url"] == "http://127.0.0.1:9222"
|
||||
assert resp["result"]["messages"][0] == "Chrome isn't running with remote debugging — attempting to launch..."
|
||||
assert any("No Chrome/Chromium executable was found" in line for line in resp["result"]["messages"])
|
||||
assert any("--remote-debugging-port=9222" in line for line in resp["result"]["messages"])
|
||||
assert (
|
||||
resp["result"]["messages"][0]
|
||||
== "Chrome isn't running with remote debugging — attempting to launch..."
|
||||
)
|
||||
assert any(
|
||||
"No Chrome/Chromium executable was found" in line
|
||||
for line in resp["result"]["messages"]
|
||||
)
|
||||
assert any(
|
||||
"--remote-debugging-port=9222" in line for line in resp["result"]["messages"]
|
||||
)
|
||||
assert "BROWSER_CDP_URL" not in os.environ
|
||||
progress = [p["message"] for evt, p in emitted if evt == "browser.progress"]
|
||||
assert progress == resp["result"]["messages"]
|
||||
|
||||
|
||||
def test_browser_manage_connect_default_local_retries_after_launch(monkeypatch):
|
||||
@@ -2956,7 +2981,9 @@ def test_browser_manage_connect_default_local_retries_after_launch(monkeypatch):
|
||||
|
||||
monkeypatch.setattr(urllib.request, "urlopen", _opener)
|
||||
with patch.dict(sys.modules, {"tools.browser_tool": fake}):
|
||||
with patch("hermes_cli.browser_connect.try_launch_chrome_debug", return_value=True):
|
||||
with patch(
|
||||
"hermes_cli.browser_connect.try_launch_chrome_debug", return_value=True
|
||||
):
|
||||
resp = server.handle_request(
|
||||
{"id": "1", "method": "browser.manage", "params": {"action": "connect"}}
|
||||
)
|
||||
@@ -2975,7 +3002,9 @@ def test_browser_manage_connect_rejects_unreachable_endpoint(monkeypatch):
|
||||
monkeypatch.setenv("BROWSER_CDP_URL", "http://existing:9222")
|
||||
cleanup_calls: list[str] = []
|
||||
fake = types.SimpleNamespace(
|
||||
cleanup_all_browsers=lambda: cleanup_calls.append(os.environ.get("BROWSER_CDP_URL", "")),
|
||||
cleanup_all_browsers=lambda: cleanup_calls.append(
|
||||
os.environ.get("BROWSER_CDP_URL", "")
|
||||
),
|
||||
_get_cdp_override=lambda: os.environ.get("BROWSER_CDP_URL", ""),
|
||||
)
|
||||
with patch.dict(sys.modules, {"tools.browser_tool": fake}):
|
||||
@@ -3055,14 +3084,19 @@ def test_browser_manage_connect_preserves_devtools_browser_endpoint(monkeypatch)
|
||||
concrete = "ws://browserbase.example/devtools/browser/abc123"
|
||||
|
||||
class _OkSocket:
|
||||
def __enter__(self): return self
|
||||
def __exit__(self, *a): return False
|
||||
def __enter__(self):
|
||||
return self
|
||||
|
||||
def __exit__(self, *a):
|
||||
return False
|
||||
|
||||
with patch.dict(sys.modules, {"tools.browser_tool": fake}):
|
||||
# If urlopen is reached for a concrete ws endpoint, the test
|
||||
# would still pass because _stub_urlopen returned ok=True before;
|
||||
# patch it to assert-fail so we prove the HTTP probe is skipped.
|
||||
with patch("urllib.request.urlopen", side_effect=AssertionError("urlopen called")):
|
||||
with patch(
|
||||
"urllib.request.urlopen", side_effect=AssertionError("urlopen called")
|
||||
):
|
||||
with patch("socket.create_connection", return_value=_OkSocket()):
|
||||
resp = server.handle_request(
|
||||
{
|
||||
@@ -3091,8 +3125,11 @@ def test_browser_manage_connect_concrete_ws_skips_http_probe(monkeypatch):
|
||||
seen_targets: list[tuple[str, int]] = []
|
||||
|
||||
class _OkSocket:
|
||||
def __enter__(self): return self
|
||||
def __exit__(self, *a): return False
|
||||
def __enter__(self):
|
||||
return self
|
||||
|
||||
def __exit__(self, *a):
|
||||
return False
|
||||
|
||||
def _fake_create_connection(addr, timeout=None):
|
||||
seen_targets.append(addr)
|
||||
@@ -3101,7 +3138,9 @@ def test_browser_manage_connect_concrete_ws_skips_http_probe(monkeypatch):
|
||||
with patch.dict(sys.modules, {"tools.browser_tool": fake}):
|
||||
# urlopen would 404/ECONNREFUSED on a real hosted CDP endpoint;
|
||||
# asserting it's never called proves the probe was skipped.
|
||||
with patch("urllib.request.urlopen", side_effect=AssertionError("urlopen called")):
|
||||
with patch(
|
||||
"urllib.request.urlopen", side_effect=AssertionError("urlopen called")
|
||||
):
|
||||
with patch("socket.create_connection", side_effect=_fake_create_connection):
|
||||
resp = server.handle_request(
|
||||
{
|
||||
@@ -3145,7 +3184,9 @@ def test_browser_manage_disconnect_drops_env_and_cleans(monkeypatch):
|
||||
monkeypatch.setenv("BROWSER_CDP_URL", "http://127.0.0.1:9222")
|
||||
cleanup_count = {"n": 0}
|
||||
fake = types.SimpleNamespace(
|
||||
cleanup_all_browsers=lambda: cleanup_count.__setitem__("n", cleanup_count["n"] + 1),
|
||||
cleanup_all_browsers=lambda: cleanup_count.__setitem__(
|
||||
"n", cleanup_count["n"] + 1
|
||||
),
|
||||
_get_cdp_override=lambda: os.environ.get("BROWSER_CDP_URL", ""),
|
||||
)
|
||||
with patch.dict(sys.modules, {"tools.browser_tool": fake}):
|
||||
@@ -3213,11 +3254,16 @@ def test_config_get_indicator_falls_back_when_unset(monkeypatch):
|
||||
def test_config_set_indicator_accepts_known_value(monkeypatch):
|
||||
written: dict = {}
|
||||
monkeypatch.setattr(
|
||||
server, "_write_config_key",
|
||||
server,
|
||||
"_write_config_key",
|
||||
lambda k, v: written.update({k: v}),
|
||||
)
|
||||
resp = server.handle_request(
|
||||
{"id": "1", "method": "config.set", "params": {"key": "indicator", "value": "EMOJI"}}
|
||||
{
|
||||
"id": "1",
|
||||
"method": "config.set",
|
||||
"params": {"key": "indicator", "value": "EMOJI"},
|
||||
}
|
||||
)
|
||||
assert resp["result"] == {"key": "indicator", "value": "emoji"}
|
||||
assert written == {"display.tui_status_indicator": "emoji"}
|
||||
@@ -3231,7 +3277,11 @@ def test_config_set_indicator_falsy_non_string_surfaces_in_error(monkeypatch):
|
||||
|
||||
for bad in (0, False, []):
|
||||
resp = server.handle_request(
|
||||
{"id": "1", "method": "config.set", "params": {"key": "indicator", "value": bad}}
|
||||
{
|
||||
"id": "1",
|
||||
"method": "config.set",
|
||||
"params": {"key": "indicator", "value": bad},
|
||||
}
|
||||
)
|
||||
assert "error" in resp
|
||||
msg = resp["error"]["message"]
|
||||
@@ -3246,7 +3296,11 @@ def test_config_set_indicator_none_keeps_blank_repr(monkeypatch):
|
||||
"""`None` is the genuine 'no value' case — empty raw is acceptable."""
|
||||
monkeypatch.setattr(server, "_write_config_key", lambda *a, **k: None)
|
||||
resp = server.handle_request(
|
||||
{"id": "1", "method": "config.set", "params": {"key": "indicator", "value": None}}
|
||||
{
|
||||
"id": "1",
|
||||
"method": "config.set",
|
||||
"params": {"key": "indicator", "value": None},
|
||||
}
|
||||
)
|
||||
assert "error" in resp
|
||||
assert "unknown indicator: ''" in resp["error"]["message"]
|
||||
|
||||
@@ -3210,7 +3210,8 @@ def _(rid, params: dict) -> dict:
|
||||
raw = ("" if value is None else str(value)).strip().lower()
|
||||
if raw not in _INDICATOR_STYLES:
|
||||
return _err(
|
||||
rid, 4002,
|
||||
rid,
|
||||
4002,
|
||||
f"unknown indicator: {raw!r}; pick one of {'|'.join(_INDICATOR_STYLES)}",
|
||||
)
|
||||
_write_config_key("display.tui_status_indicator", raw)
|
||||
@@ -4786,6 +4787,10 @@ def _browser_connect_failure_messages(url: str, port: int) -> list[str]:
|
||||
]
|
||||
|
||||
|
||||
def _browser_progress(sid: str, message: str, *, level: str = "info") -> None:
|
||||
_emit("browser.progress", sid, {"message": message, "level": level})
|
||||
|
||||
|
||||
@method("browser.manage")
|
||||
def _(rid, params: dict) -> dict:
|
||||
action = params.get("action", "status")
|
||||
@@ -4802,7 +4807,13 @@ def _(rid, params: dict) -> dict:
|
||||
from hermes_cli.browser_connect import DEFAULT_BROWSER_CDP_URL
|
||||
|
||||
url = params.get("url", DEFAULT_BROWSER_CDP_URL)
|
||||
sid = params.get("session_id") or ""
|
||||
messages: list[str] = []
|
||||
|
||||
def announce(message: str, *, level: str = "info") -> None:
|
||||
messages.append(message)
|
||||
_browser_progress(sid, message, level=level)
|
||||
|
||||
try:
|
||||
import urllib.request
|
||||
from urllib.parse import urlparse
|
||||
@@ -4822,9 +4833,8 @@ def _(rid, params: dict) -> dict:
|
||||
# path. Probing it would just reject valid endpoints. Skip
|
||||
# the HTTP probe and do a TCP-level reachability check instead;
|
||||
# the actual CDP handshake happens on the next ``browser_navigate``.
|
||||
is_concrete_ws = (
|
||||
parsed.scheme in {"ws", "wss"}
|
||||
and parsed.path.startswith("/devtools/browser/")
|
||||
is_concrete_ws = parsed.scheme in {"ws", "wss"} and parsed.path.startswith(
|
||||
"/devtools/browser/"
|
||||
)
|
||||
if is_concrete_ws:
|
||||
import socket
|
||||
@@ -4859,15 +4869,23 @@ def _(rid, params: dict) -> dict:
|
||||
from hermes_cli.browser_connect import try_launch_chrome_debug
|
||||
|
||||
port = parsed.port or 9222
|
||||
messages.append("Chrome isn't running with remote debugging — attempting to launch...")
|
||||
announce(
|
||||
"Chrome isn't running with remote debugging — attempting to launch..."
|
||||
)
|
||||
launched = try_launch_chrome_debug(port, platform.system())
|
||||
if launched:
|
||||
for _ in range(20):
|
||||
time.sleep(0.5)
|
||||
for probe in probe_urls:
|
||||
try:
|
||||
with urllib.request.urlopen(probe, timeout=1.0) as resp:
|
||||
if 200 <= getattr(resp, "status", 200) < 300:
|
||||
with urllib.request.urlopen(
|
||||
probe, timeout=1.0
|
||||
) as resp:
|
||||
if (
|
||||
200
|
||||
<= getattr(resp, "status", 200)
|
||||
< 300
|
||||
):
|
||||
ok = True
|
||||
break
|
||||
except Exception:
|
||||
@@ -4875,14 +4893,22 @@ def _(rid, params: dict) -> dict:
|
||||
if ok:
|
||||
break
|
||||
if ok:
|
||||
messages.append(f"Chrome launched and listening on port {port}")
|
||||
announce(
|
||||
f"Chrome launched and listening on port {port}"
|
||||
)
|
||||
if not ok:
|
||||
messages.extend(_browser_connect_failure_messages(url, port)[1:])
|
||||
return _ok(rid, {"connected": False, "url": url, "messages": messages})
|
||||
for line in _browser_connect_failure_messages(url, port)[1:]:
|
||||
announce(line, level="error")
|
||||
return _ok(
|
||||
rid,
|
||||
{"connected": False, "url": url, "messages": messages},
|
||||
)
|
||||
else:
|
||||
return _err(rid, 5031, f"could not reach browser CDP at {url}")
|
||||
elif _is_default_local_cdp(parsed):
|
||||
messages.append(f"Chrome is already listening on port {parsed.port or 9222}")
|
||||
announce(
|
||||
f"Chrome is already listening on port {parsed.port or 9222}"
|
||||
)
|
||||
|
||||
# Persist a normalized URL for downstream CDP resolution.
|
||||
# Discovery-style inputs (`http://host:port` or
|
||||
|
||||
@@ -293,6 +293,19 @@ describe('createGatewayEventHandler', () => {
|
||||
expect(appended[1]).toMatchObject({ role: 'assistant', text: 'final answer' })
|
||||
})
|
||||
|
||||
it('renders browser.progress events as system transcript lines as they stream in', () => {
|
||||
const appended: Msg[] = []
|
||||
const ctx = buildCtx(appended)
|
||||
const handler = createGatewayEventHandler(ctx)
|
||||
|
||||
handler({
|
||||
payload: { message: 'Chrome launched and listening on port 9222' },
|
||||
type: 'browser.progress'
|
||||
} as any)
|
||||
|
||||
expect(ctx.system.sys).toHaveBeenCalledWith('Chrome launched and listening on port 9222')
|
||||
})
|
||||
|
||||
it('annotates gateway.start_timeout with stderr tail lines so users can diagnose without /logs', () => {
|
||||
const appended: Msg[] = []
|
||||
const onEvent = createGatewayEventHandler(buildCtx(appended))
|
||||
|
||||
@@ -191,8 +191,12 @@ describe('createSlashHandler', () => {
|
||||
})
|
||||
|
||||
it.each([
|
||||
['/browser status', 'browser.manage', { action: 'status' }],
|
||||
['/browser connect', 'browser.manage', { action: 'connect', url: 'http://127.0.0.1:9222' }],
|
||||
['/browser status', 'browser.manage', { action: 'status', session_id: null }],
|
||||
[
|
||||
'/browser connect',
|
||||
'browser.manage',
|
||||
{ action: 'connect', session_id: null, url: 'http://127.0.0.1:9222' }
|
||||
],
|
||||
['/reload-mcp', 'reload.mcp', { session_id: null }],
|
||||
['/stop', 'process.stop', {}],
|
||||
['/fast status', 'config.get', { key: 'fast', session_id: null }],
|
||||
|
||||
@@ -307,6 +307,16 @@ export function createGatewayEventHandler(ctx: GatewayEventHandlerContext): (ev:
|
||||
return
|
||||
}
|
||||
|
||||
case 'browser.progress': {
|
||||
const message = String(ev.payload?.message ?? '').trim()
|
||||
|
||||
if (message) {
|
||||
sys(message)
|
||||
}
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
case 'voice.status': {
|
||||
// Continuous VAD loop reports its internal state so the status bar
|
||||
// can show listening / transcribing / idle without polling.
|
||||
|
||||
@@ -103,7 +103,8 @@ export const opsCommands: SlashCommand[] = [
|
||||
)
|
||||
}
|
||||
|
||||
const payload: Record<string, unknown> = { action }
|
||||
const sid = ctx.sid ?? null
|
||||
const payload: Record<string, unknown> = { action, session_id: sid }
|
||||
const requested = rest.join(' ').trim()
|
||||
|
||||
if (action === 'connect') {
|
||||
@@ -115,7 +116,9 @@ export const opsCommands: SlashCommand[] = [
|
||||
.rpc<BrowserManageResponse>('browser.manage', payload)
|
||||
.then(
|
||||
ctx.guarded<BrowserManageResponse>(r => {
|
||||
r.messages?.forEach(message => ctx.transcript.sys(message))
|
||||
if (!sid) {
|
||||
r.messages?.forEach(message => ctx.transcript.sys(message))
|
||||
}
|
||||
|
||||
if (action === 'status') {
|
||||
return ctx.transcript.sys(
|
||||
|
||||
@@ -433,6 +433,11 @@ export type GatewayEvent =
|
||||
| { payload?: { state?: 'idle' | 'listening' | 'transcribing' }; session_id?: string; type: 'voice.status' }
|
||||
| { payload?: { no_speech_limit?: boolean; text?: string }; session_id?: string; type: 'voice.transcript' }
|
||||
| { payload: { line: string }; session_id?: string; type: 'gateway.stderr' }
|
||||
| {
|
||||
payload?: { level?: 'info' | 'warn' | 'error'; message?: string }
|
||||
session_id?: string
|
||||
type: 'browser.progress'
|
||||
}
|
||||
| {
|
||||
payload?: { cwd?: string; python?: string; stderr_tail?: string }
|
||||
session_id?: string
|
||||
|
||||
Reference in New Issue
Block a user