From 63c82acdfaa2ecada04c6b144ea9a958967ec200 Mon Sep 17 00:00:00 2001 From: Leonid Gorkin Date: Thu, 29 Oct 2026 13:54:33 -0400 Subject: [PATCH] fix(openbot_cli): wire `connect` to ping /health and drop inline pathlib import MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two carried-forward nits from the terminal-client work (#314): - `connect` only saved config; it now builds an OpenBotClient from the saved settings and probes the server's public `/api/v1/health` route, so a bad URL or key surfaces immediately. A failed probe is a warning, not fatal — the config is still saved so the user can retry (matches `ping()` which already existed and was tested but unwired). - Replace `__import__("pathlib").Path(...)` with a top-level `from pathlib import Path` import. Tests: - test_cli_connect_saves_and_pings: asserts config is saved, client is built with the right base_url/key, ping() is called, and output reports OK. - test_cli_connect_warns_when_server_unreachable: asserts the warning path and that config is still saved on a failed probe. ruff clean on the openbot_cli package; 17 CLI tests + 50 (cli+config) pass. --- backend/openbot_cli/cli.py | 21 +++++++++++--- backend/tests/test_openbot_cli.py | 46 +++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 4 deletions(-) diff --git a/backend/openbot_cli/cli.py b/backend/openbot_cli/cli.py index 3c755ba..70dcd3c 100644 --- a/backend/openbot_cli/cli.py +++ b/backend/openbot_cli/cli.py @@ -2,7 +2,7 @@ Commands mirror the core web-UI flows against a running OpenBot server: - openbot connect [--api-key KEY] Save connection settings + openbot connect [--api-key KEY] Save + verify connection settings openbot threads [list] List threads openbot thread [watch] Show a thread (or live-follow it) openbot send "" [--bot @h] Post a message, optionally @-ing a bot @@ -19,10 +19,11 @@ import json import os import sys +from pathlib import Path import click -from openbot_cli.client import GitHubIssuesClient, OpenBotClient +from openbot_cli.client import GitHubIssuesClient, OpenBotClient, OpenBotClientError from openbot_cli.config import ( CONFIG_PATH, ConfigError, @@ -69,8 +70,13 @@ def cli(ctx: click.Context, base_url: str | None, api_key: str | None) -> None: @click.option("--api-key", default=None, help="API key to store alongside the URL.") @click.option("--config", "config_path", default=None, type=click.Path(), help="Config file (advanced).") def connect(url: str | None, api_key: str | None, config_path: str | None) -> None: - """Save the OpenBot server URL (and optional API key) for future commands.""" - path = CONFIG_PATH if config_path is None else __import__("pathlib").Path(config_path) + """Save the OpenBot server URL (and optional API key) and verify it responds. + + Persists the connection to the config file, then probes the server's public + ``/api/v1/health`` route so a bad URL or key surfaces immediately. A failed + probe is only a warning — the config is still saved so the user can retry. + """ + path = CONFIG_PATH if config_path is None else Path(config_path) data = load_config(path) if url: data["base_url"] = url.rstrip("/") @@ -78,6 +84,13 @@ def connect(url: str | None, api_key: str | None, config_path: str | None) -> No data["api_key"] = api_key saved = save_config(data, path) click.echo(f"Saved connection to {data['base_url']} ({saved})") + probe = OpenBotClient(base_url=data["base_url"], api_key=data.get("api_key")) + try: + probe.ping() + except OpenBotClientError as e: + click.echo(f"Warning: could not reach the server: {e}", err=True) + return + click.echo("Connection OK (/api/v1/health)") @cli.command("threads") diff --git a/backend/tests/test_openbot_cli.py b/backend/tests/test_openbot_cli.py index 0fc11f4..e1a62a7 100644 --- a/backend/tests/test_openbot_cli.py +++ b/backend/tests/test_openbot_cli.py @@ -275,6 +275,52 @@ def handler(req): assert r2.exit_code == 0 and "r1" in r2.output +def test_cli_connect_saves_and_pings(tmp_path, monkeypatch): + cfg = tmp_path / "cfg.json" + captured = {} + + class _FakeClient: + def __init__(self, base_url, api_key=None): + captured["base_url"] = base_url + captured["api_key"] = api_key + + def ping(self): + captured["pinged"] = True + return {"status": "ok"} + + monkeypatch.setattr(cli_mod, "OpenBotClient", _FakeClient) + runner = CliRunner() + result = runner.invoke(cli_mod.cli, ["connect", "https://example.test", "--api-key", "sek", "--config", str(cfg)]) + assert result.exit_code == 0, result.output + assert "Saved connection" in result.output + assert "Connection OK" in result.output + assert captured["base_url"] == "https://example.test" + assert captured["api_key"] == "sek" + assert captured["pinged"] is True + saved = json.loads(cfg.read_text()) + assert saved["base_url"] == "https://example.test" + assert saved["api_key"] == "sek" + + +def test_cli_connect_warns_when_server_unreachable(tmp_path, monkeypatch): + cfg = tmp_path / "cfg.json" + + class _FakeClient: + def __init__(self, base_url, api_key=None): + pass + + def ping(self): + raise OpenBotClientError("GET /api/v1/health -> 500") + + monkeypatch.setattr(cli_mod, "OpenBotClient", _FakeClient) + runner = CliRunner() + result = runner.invoke(cli_mod.cli, ["connect", "https://down.test", "--config", str(cfg)]) + assert result.exit_code == 0, result.output + assert "Warning:" in result.output + # Config is still saved even though the probe failed. + assert json.loads(cfg.read_text())["base_url"] == "https://down.test" + + def test_cli_issues_fetches_github(monkeypatch): class FakeResp: status_code = 200