diff --git a/docs/limitations.md b/docs/limitations.md index fc70d7d..1a0c132 100644 --- a/docs/limitations.md +++ b/docs/limitations.md @@ -9,6 +9,17 @@ including those that only surface once you provision connections yourself. or alerting. - CLI mode runs one `hookdeck listen` process per route. That is fine for a handful; a gateway with dozens of routes wants push mode. +- In CLI mode the CLI's project and the API key's project are separate pieces + of state, and only the operator reconciles them. `hermes hookdeck setup`, + `status`, `retry`, the dashboard tab and the agent tools all act on the + project `HOOKDECK_API_KEY` belongs to; `hookdeck listen` forwards from + whichever project `~/.config/hookdeck/config.toml` records. When they + differ the gateway starts, reports healthy, and receives nothing — every + event is ignored as `CLI_DISCONNECTED` while the tunnel restart-loops. Run + `hermes hookdeck doctor`, which compares the two and names both project ids. + `cli_config` points the CLI at a config of the gateway's own, but populating + one still needs `hookdeck ci`: the CLI does not accept a project API key as + a session key. - `setup` only pushes an `events` filter down into Hookdeck when it knows where the event name lives: a header for GitHub, GitLab and Shopify, or a body path you set with `event_path`. Otherwise the filter stays adapter-side, because a diff --git a/examples/config.yaml b/examples/config.yaml index 5a44c2a..acfe2b0 100644 --- a/examples/config.yaml +++ b/examples/config.yaml @@ -53,6 +53,21 @@ gateway: # gateway should do to a shared tool. Run `hookdeck login` instead. # cli_login: false + # Which CLI config `hookdeck listen` reads, via --hookdeck-config. + # Unset, it uses ~/.config/hookdeck/config.toml — shared with every + # other Hookdeck CLI use on the machine, and its active project is + # ambient state this gateway does not control. That project is what + # `hookdeck listen` forwards from, and it is entirely independent of + # HOOKDECK_API_KEY: when the two differ, `hermes hookdeck setup` + # creates the connection in one project while the tunnel looks for it + # in the other, and the gateway reports healthy while receiving + # nothing. `hermes hookdeck doctor` compares them and says so. + # + # Note that populating a gateway-owned config still requires + # `hookdeck ci`, because the CLI will not accept a project API key as + # a session key. + # cli_config: ~/.hermes/hookdeck-cli.toml + routes: # A GitHub PR reviewer. `source` is the Hookdeck source name. github-prs: diff --git a/hookdeck/adapter.py b/hookdeck/adapter.py index e35ddb8..2cf1f4d 100644 --- a/hookdeck/adapter.py +++ b/hookdeck/adapter.py @@ -294,6 +294,7 @@ async def _start_tunnels(self) -> bool: connection_name=route_name, binary=self.settings.cli_binary, login=self.settings.cli_login, + config_path=self.settings.cli_config, ) await tunnel.start() self._tunnels.append(tunnel) diff --git a/hookdeck/cli.py b/hookdeck/cli.py index ad6bfb1..191a600 100644 --- a/hookdeck/cli.py +++ b/hookdeck/cli.py @@ -18,6 +18,7 @@ from typing import Any, Optional from .api import HookdeckAPI, HookdeckAPIError, run_sync +from .cliconfig import DEFAULT_CLI_CONFIG, cli_project_id from .constants import DEFAULT_PATH, DEFAULT_PORT from .provision import ( build_connection_payload, @@ -442,6 +443,17 @@ def _check_routes(routes: dict) -> Check: ) +def _cli_config_path(extra: dict) -> Path: + """Where the CLI's session lives, honouring an override. + + ``hookdeck listen`` takes ``--hookdeck-config``, so an operator who points + the gateway at a config of its own must have doctor read the same one — + otherwise the check reports on a file nothing uses. + """ + configured = extra.get("cli_config") + return Path(configured).expanduser() if configured else DEFAULT_CLI_CONFIG + + def _check_cli(extra: dict) -> list[Check]: """The CLI is only reachable in cli mode, and only the resolved one matters.""" configured = extra.get("cli_binary") or "hookdeck" @@ -505,7 +517,79 @@ def _report_stranded_runs() -> None: ledger.close() -async def _check_live_connections(routes: dict) -> list[Check]: +async def _api_project_id( + api: HookdeckAPI, already_found: list[dict] +) -> Optional[str]: + """Which project the API key belongs to. + + There is no endpoint that answers this directly, but every resource the key + can reach carries ``team_id`` — so the connections the caller already + listed answer it for free, and only an empty result needs a request of its + own. ``None`` means the key reaches no connections at all, which is not + itself a fault: a project can legitimately be empty. + """ + for connection in already_found: + if connection.get("team_id"): + return str(connection["team_id"]) + + result = await api.list_connections(limit=1) + models = (result or {}).get("models") or (result or {}).get("data") or [] + for connection in models: + if connection.get("team_id"): + return str(connection["team_id"]) + return None + + +async def _check_cli_project( + api: HookdeckAPI, extra: dict, already_found: list[dict] +) -> Check: + """Whether the CLI forwards from the project the API key acts on. + + These are two unrelated pieces of state — an env var and + ``~/.config/hookdeck/config.toml`` — and nothing else reconciles them. When + they differ, `setup` creates the connection in one project while + ``hookdeck listen`` looks for it in the other: the gateway logs that it is + listening, the tunnel restart-loops out of sight, and every event is + ignored as ``CLI_DISCONNECTED``. From the outside that is indistinguishable + from "no events are arriving", which is the whole class of problem this + command exists to make diagnosable. + """ + config_path = _cli_config_path(extra) + cli_project = cli_project_id(config_path) + api_project = await _api_project_id(api, already_found) + + if cli_project is None: + return Check( + False, + f"No Hookdeck CLI session found at {config_path} — `hookdeck " + "listen` cannot start, so no events will reach the gateway. Run " + "`hookdeck login`.", + ) + if api_project is None: + return Check( + True, + f"Hookdeck CLI is logged into project {cli_project}", + note="The API key's project could not be determined, because it " + "reaches no connections yet — so the two are unverified. Re-run " + "this after `hermes hookdeck setup`.", + ) + if cli_project != api_project: + return Check( + False, + f"Project mismatch: the CLI forwards from {cli_project} but the " + f"API key acts on {api_project}. `hermes hookdeck setup` creates " + "connections in the API key's project while `hookdeck listen` " + "looks for them in the CLI's, so the gateway will report healthy " + "and receive nothing.", + note="Point the CLI at the same project with `hookdeck login`, or " + "set the API key to one belonging to " + cli_project + ".", + ) + return Check(True, f"CLI and API key agree on project {api_project}") + + +async def _check_live_connections( + routes: dict, *, mode: str, extra: dict +) -> list[Check]: """Reachability, plus whether each retry rule covers what the adapter emits. A rule narrower than the emitted statuses is silent data loss — a deferred @@ -523,6 +607,9 @@ async def _check_live_connections(routes: dict) -> list[Check]: result = await api.list_connections(name=route_name, limit=10) found += (result or {}).get("models") or (result or {}).get("data") or [] + if mode == "cli": + checks.append(await _check_cli_project(api, extra, found)) + for connection in found: if connection.get("name") not in routes: continue @@ -575,7 +662,7 @@ def _cmd_doctor(_args: argparse.Namespace) -> int: if os.getenv("HOOKDECK_API_KEY"): print() try: - live = run_sync(_check_live_connections(routes)) + live = run_sync(_check_live_connections(routes, mode=mode, extra=extra)) except HookdeckAPIError as exc: live = [Check(False, f"Hookdeck API check failed: {exc}")] for check in live: diff --git a/hookdeck/cliconfig.py b/hookdeck/cliconfig.py new file mode 100644 index 0000000..de9e63c --- /dev/null +++ b/hookdeck/cliconfig.py @@ -0,0 +1,86 @@ +"""Reading the Hookdeck CLI's own configuration. + +In ``cli`` mode there are two independent notions of "which project": the API +key, which every ``hermes hookdeck`` command and the dashboard act on, and the +CLI's stored session, which is what ``hookdeck listen`` actually forwards from. +Nothing reconciles them, and when they differ the gateway reports healthy while +no event ever arrives — the tunnel restart-loops out of sight, and the source +gets auto-created in the wrong project on the way. + +This module is the read-only half of catching that: it reports which project +the CLI would use. Nothing here writes, because ``hookdeck ci`` — the only way +to change it — rewrites the operator's shared config and switches its active +project, which is not something starting a gateway should do. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +#: Where the CLI keeps its session unless told otherwise, matching the CLI's +#: own ``--hookdeck-config`` default. +DEFAULT_CLI_CONFIG = Path.home() / ".config" / "hookdeck" / "config.toml" + + +def _parse_toml(text: str) -> dict: + """Parse the CLI config, by whatever means this interpreter has. + + ``tomllib`` is stdlib from 3.11 and this package supports 3.10, so there is + a fallback. The file is a handful of ``key = "value"`` lines under one + table header, so the fallback covers it — and a diagnostic that degrades is + better than one that raises on the older interpreter. + """ + try: + import tomllib + except ImportError: # pragma: no cover - only on 3.10 + return _parse_flat_toml(text) + try: + return tomllib.loads(text) + except Exception: # noqa: BLE001 - a malformed config is "unknown", not fatal + return _parse_flat_toml(text) + + +_ASSIGNMENT = re.compile(r"""^\s*([A-Za-z0-9_-]+)\s*=\s*(.+?)\s*$""") +_TABLE = re.compile(r"""^\s*\[([^\]]+)\]\s*$""") + + +def _parse_flat_toml(text: str) -> dict: + """Enough TOML for this one file: top-level keys and single-level tables.""" + result: dict = {} + table = result + for line in text.splitlines(): + if not line.strip() or line.lstrip().startswith("#"): + continue + header = _TABLE.match(line) + if header: + table = result.setdefault(header.group(1).strip(), {}) + continue + assignment = _ASSIGNMENT.match(line) + if assignment: + table[assignment.group(1)] = assignment.group(2).strip().strip('"\'') + return result + + +def cli_project_id(path: Path | None = None) -> str | None: + """The project ``hookdeck listen`` would forward from. + + ``None`` when it cannot be determined — no config, unreadable, or no + project recorded. That is reported as "unknown" rather than as a mismatch, + because guessing here would be worse than saying so. + """ + config_path = DEFAULT_CLI_CONFIG if path is None else path + try: + text = config_path.read_text() + except OSError: + return None + + parsed = _parse_toml(text) + # The CLI supports named profiles and records which one is active at the + # top level; each profile is a table of its own. + profile = parsed.get("profile") or "default" + section = parsed.get(profile) + if not isinstance(section, dict): + return None + project = section.get("project_id") + return str(project) if project else None diff --git a/hookdeck/settings.py b/hookdeck/settings.py index d8c590f..c1a8176 100644 --- a/hookdeck/settings.py +++ b/hookdeck/settings.py @@ -122,6 +122,11 @@ class AdapterSettings: ledger_ttl_seconds: float = DEFAULT_LEDGER_TTL_SECONDS max_body_bytes: int = DEFAULT_MAX_BODY_BYTES cli_binary: str = "hookdeck" + #: Path passed to `hookdeck listen --hookdeck-config`. Empty means the + #: CLI's shared default, whose active project is ambient state the gateway + #: does not control — `hermes hookdeck doctor` reports when that disagrees + #: with the API key's project. + cli_config: str = "" cli_login: bool = False # ------------------------------------------------------------------ @@ -186,6 +191,9 @@ def text(key: str, env: str = "", default: str = "") -> str: # An npm global shadowing a Homebrew install is the common case, # and PATH silently picks the older one. cli_binary=text("cli_binary", default="hookdeck"), + cli_config=str( + Path(extra["cli_config"]).expanduser() if extra.get("cli_config") else "" + ), # Off by default: `hookdeck ci` rewrites the shared CLI config and # repoints its active project — not something starting a gateway # should do to a tool the operator uses for other work. diff --git a/hookdeck/tunnel.py b/hookdeck/tunnel.py index 119b508..5cc5be4 100644 --- a/hookdeck/tunnel.py +++ b/hookdeck/tunnel.py @@ -29,6 +29,47 @@ # Generous enough that CLI output never kills a working tunnel. _STDOUT_LINE_LIMIT = 1024 * 1024 +# How many consecutive too-short runs before the restart loop is treated as a +# standing failure rather than a blip. Two would fire on an ordinary flap; this +# is small enough to be prompt and large enough not to cry wolf. +_FAST_FAILURES_BEFORE_ESCALATING = 3 + +# How much of the CLI's output to keep for diagnosis. The useful line is the +# last one before it exits. +_OUTPUT_MEMORY = 12 + +#: Substrings the CLI prints for failures that retrying cannot fix, paired with +#: what to do about them. The backoff exists for network blips; these need a +#: person, and without this they scroll past at warning level for ever. +_DETERMINISTIC_FAILURES: tuple[tuple[str, str], ...] = ( + ( + "no connection found matching filter", + "the CLI is logged into a different Hookdeck project than the API key " + "used by `hermes hookdeck setup`, or setup has not been run for this " + "route. Run `hermes hookdeck doctor` — it compares the two.", + ), + ( + "automatically creating source", + "the source does not exist in the project the CLI is logged into, so " + "the CLI has just created a stray one there. That is usually a " + "different project than the API key acts on. Run `hermes hookdeck " + "doctor`.", + ), + ( + "authentication failed", + "the CLI's stored session is invalid or expired. Run `hookdeck login`.", + ), +) + + +def diagnose(lines: list[str]) -> str: + """A likely cause for the CLI's exit, or "" if none is recognised.""" + haystack = "\n".join(lines).lower() + for marker, explanation in _DETERMINISTIC_FAILURES: + if marker in haystack: + return explanation + return "" + class HookdeckCLIMissing(RuntimeError): """The ``hookdeck`` binary is not on PATH.""" @@ -54,6 +95,7 @@ def __init__( api_key: str = "", binary: str = "hookdeck", login: bool = False, + config_path: str = "", ): if not source: raise ValueError( @@ -68,9 +110,16 @@ def __init__( self._api_key = api_key or os.getenv("HOOKDECK_API_KEY", "") self._login_enabled = login self._binary = binary + # Which CLI config `hookdeck listen` reads. Empty means the CLI's own + # default, which is the operator's shared session — see #4 for why + # that being ambient is a problem worth being able to opt out of. + self._config_path = config_path self._process: Optional[asyncio.subprocess.Process] = None self._supervisor: Optional[asyncio.Task] = None self._stopping = False + #: Tail of the last run's output, kept so a failing restart loop can + #: say *why* rather than only that it is looping. + self._recent_output: list[str] = [] # ------------------------------------------------------------------ # Command construction @@ -99,6 +148,8 @@ def listen_args(self) -> list[str]: args.append(self._connection_name) if self._path: args += ["--path", self._path] + if self._config_path: + args += ["--hookdeck-config", self._config_path] # The default `interactive` output renders a full-screen UI and exits # immediately when stdout is not a TTY — which it never is here, since # the supervisor pipes it into the gateway log. @@ -176,6 +227,8 @@ async def _login(self, binary: str) -> None: async def _supervise(self, binary: str) -> None: backoff = _BACKOFF_INITIAL + fast_failures = 0 + escalated = False while not self._stopping: started = asyncio.get_running_loop().time() try: @@ -191,6 +244,28 @@ async def _supervise(self, binary: str) -> None: ran_for = asyncio.get_running_loop().time() - started if ran_for >= _HEALTHY_RUN_SECONDS: backoff = _BACKOFF_INITIAL + fast_failures = 0 + escalated = False + else: + fast_failures += 1 + + # A tunnel that never stays up is not retrying its way out of + # anything, and at warning level it reads as routine churn. Say so + # once, loudly, with the cause if the CLI named one — then fall + # back to the quiet line so the log does not fill up. + if fast_failures >= _FAST_FAILURES_BEFORE_ESCALATING and not escalated: + escalated = True + cause = diagnose(self._recent_output) + logger.error( + "[hookdeck] CLI tunnel has failed to stay up %d times in a " + "row (last run %.0fs). No events are reaching the gateway. " + "%sLast output: %s", + fast_failures, + ran_for, + f"Likely cause: {cause} " if cause else "", + " / ".join(self._recent_output[-3:]) or "(none)", + ) + logger.warning( "[hookdeck] CLI tunnel exited after %.0fs — restarting in %.0fs", ran_for, @@ -213,11 +288,17 @@ async def _run_once(self, binary: str) -> None: limit=_STDOUT_LINE_LIMIT, env={**os.environ, **({"HOOKDECK_API_KEY": self._api_key} if self._api_key else {})}, ) + # Reset per run: the diagnosis is about why *this* run ended, and + # carrying lines over from the previous one would name a cause that has + # since been fixed. + self._recent_output = [] assert self._process.stdout is not None async for line in self._process.stdout: text = line.decode("utf-8", "replace").rstrip() if text: logger.info("[hookdeck cli] %s", text) + self._recent_output.append(text) + del self._recent_output[:-_OUTPUT_MEMORY] await self._process.wait() async def _terminate(self) -> None: diff --git a/tests/test_cli.py b/tests/test_cli.py index 4e6cb8c..dbe7281 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -376,17 +376,24 @@ def test_a_failing_retry_exits_non_zero(fake_api, capsys): @pytest.fixture() -def doctor_env(monkeypatch, ledger_at): - """A doctor run with nothing live: no API key, no CLI binary. +def doctor_env(monkeypatch, ledger_at, tmp_path): + """A doctor run with nothing live: no API key, no CLI binary, no session. Both env-var spellings are cleared. Only the unprefixed names are read on this branch, but an operator's shell may have either set, and a doctor test that passes because of the caller's environment is worse than none. + + The CLI config path is redirected for the same reason: read from the real + location, these tests pass on a developer's laptop — where a session + exists — and fail in CI, where none does. """ for name in ("API_KEY", "WEBHOOK_SECRET", "MODE", "PROJECT_ID"): monkeypatch.delenv(f"HOOKDECK_{name}", raising=False) monkeypatch.delenv(f"HOOKDECK_EG_{name}", raising=False) + monkeypatch.cli_config = tmp_path / "hookdeck-cli.toml" + monkeypatch.setattr(cli, "_cli_config_path", lambda _extra: monkeypatch.cli_config) + # Recorded, not just stubbed: "did the CLI checks run at all?" is the # question in push mode, and answering it by grepping the output for # "Hookdeck CLI" breaks as soon as an unrelated message mentions the CLI. @@ -402,6 +409,16 @@ def which(binary): return monkeypatch +def _write_cli_config(doctor_env, *, project_id, profile="default"): + """Give the fixture's CLI config a logged-in session.""" + doctor_env.cli_config.write_text( + f'profile = "{profile}"\n\n' + f"[{profile}]\n" + 'api_key = "cli_session_key"\n' + f'project_id = "{project_id}"\n' + ) + + def _configure(monkeypatch, *, routes=None, **extra): """Set both config readers doctor uses, so they cannot disagree.""" full = {**extra, "routes": routes or {}} @@ -449,6 +466,7 @@ def test_doctor_passes_when_everything_is_in_place( "models": [ { "name": "github", + "team_id": "tm_same", "rules": [ { "type": "retry", @@ -458,6 +476,7 @@ def test_doctor_passes_when_everything_is_in_place( } ] } + _write_cli_config(doctor_env, project_id="tm_same") _configure(doctor_env, secret="whsec_x", routes={"github": {"source": "github"}}) assert cli.hookdeck_command(_ns("doctor")) == 0 @@ -553,3 +572,105 @@ def test_doctor_reports_an_unreachable_api_as_a_failed_check( _configure(doctor_env, routes={"github": {}}) assert cli.hookdeck_command(_ns("doctor")) == 1 assert "Hookdeck API check failed" in capsys.readouterr().out + + +# ── doctor: the two notions of "which project" (#4) ───────────────── + + +def _live_doctor(doctor_env, fake_api, monkeypatch, *, connections): + """A doctor run in cli mode with an API key, so the live checks run.""" + monkeypatch.setenv("HOOKDECK_API_KEY", "key") + monkeypatch.setenv("HOOKDECK_WEBHOOK_SECRET", "whsec_x") + doctor_env.setattr(cli.shutil, "which", lambda _b: "/usr/local/bin/hookdeck") + doctor_env.setattr(cli, "_cli_version", lambda _b: "2.4.0") + doctor_env.setattr(cli, "_other_hookdeck_binaries", lambda _r: []) + fake_api.responses["list_connections"] = {"models": connections} + _configure(doctor_env, secret="whsec_x", routes={"github": {"source": "github"}}) + return cli.hookdeck_command(_ns("doctor")) + + +def test_doctor_catches_the_cli_and_the_api_key_pointing_at_different_projects( + doctor_env, fake_api, monkeypatch, capsys +): + # The failure this exists for: `setup` creates the connection in the API + # key's project while `hookdeck listen` looks for it in the CLI's. The + # gateway logs that it is listening and receives nothing, which looks + # exactly like "no events are arriving". + _write_cli_config(doctor_env, project_id="tm_cli") + code = _live_doctor( + doctor_env, fake_api, monkeypatch, + connections=[{"name": "github", "team_id": "tm_apikey"}], + ) + out = capsys.readouterr().out + assert code == 1 + assert "Project mismatch" in out + # Both ids named, because the fix is to change one of them and the + # operator has to know which is which. + assert "tm_cli" in out and "tm_apikey" in out + assert "hookdeck login" in out + + +def test_doctor_is_quiet_when_both_agree(doctor_env, fake_api, monkeypatch, capsys): + _write_cli_config(doctor_env, project_id="tm_same") + code = _live_doctor( + doctor_env, fake_api, monkeypatch, + connections=[{"name": "github", "team_id": "tm_same"}], + ) + out = capsys.readouterr().out + assert code == 0 + assert "agree on project tm_same" in out + assert "mismatch" not in out.lower() + + +def test_doctor_reports_a_cli_that_was_never_logged_in( + doctor_env, fake_api, monkeypatch, capsys +): + # No config file at all. `hookdeck listen` cannot start, so this is a + # failure in its own right rather than an unknown. + code = _live_doctor( + doctor_env, fake_api, monkeypatch, + connections=[{"name": "github", "team_id": "tm_apikey"}], + ) + out = capsys.readouterr().out + assert code == 1 + assert "No Hookdeck CLI session found" in out + assert "hookdeck login" in out + + +def test_an_empty_project_is_unknown_rather_than_a_mismatch( + doctor_env, fake_api, monkeypatch, capsys +): + # A key that reaches no connections cannot be placed. Reporting that as a + # mismatch would send an operator to fix something that is not broken. + _write_cli_config(doctor_env, project_id="tm_cli") + code = _live_doctor(doctor_env, fake_api, monkeypatch, connections=[]) + out = capsys.readouterr().out + assert code == 0 + assert "logged into project tm_cli" in out + assert "could not be determined" in out + + +def test_the_project_check_does_not_run_in_push_mode( + doctor_env, fake_api, monkeypatch, capsys +): + # Push mode does not run `hookdeck listen`, so the CLI's project is + # irrelevant and reporting on it would be noise an operator must dismiss. + monkeypatch.setenv("HOOKDECK_API_KEY", "key") + _write_cli_config(doctor_env, project_id="tm_cli") + fake_api.responses["list_connections"] = { + "models": [{"name": "github", "team_id": "tm_apikey"}] + } + _configure(doctor_env, mode="push", public_url="https://example.com", + secret="whsec_x", routes={"github": {}}) + cli.hookdeck_command(_ns("doctor")) + out = capsys.readouterr().out + assert "Project mismatch" not in out + assert "CLI session" not in out + + +def test_the_project_is_read_from_the_configured_cli_config(tmp_path): + # `hookdeck listen --hookdeck-config` lets the config move; doctor reading + # the default path anyway would report on a file nothing uses. + custom = tmp_path / "gateway-cli.toml" + assert cli._cli_config_path({"cli_config": str(custom)}) == custom + assert cli._cli_config_path({}) == cli.DEFAULT_CLI_CONFIG diff --git a/tests/test_cliconfig.py b/tests/test_cliconfig.py new file mode 100644 index 0000000..ddfa12c --- /dev/null +++ b/tests/test_cliconfig.py @@ -0,0 +1,78 @@ +"""Reading the Hookdeck CLI's session config. + +The project recorded here is what `hookdeck listen` forwards from, and it is +entirely independent of the API key. Misreading it produces a false "projects +agree", which is worse than no check at all. +""" + +from __future__ import annotations + +from hookdeck.cliconfig import _parse_flat_toml, cli_project_id + +REAL_SHAPE = '''profile = "default" + +[default] +api_key = "cli_abc" +guest_url = "" +project_id = "tm_WYrcWDMnjZpG" +project_mode = "console" +project_type = "team" +''' + + +def test_the_project_is_read_from_the_active_profile(tmp_path): + path = tmp_path / "config.toml" + path.write_text(REAL_SHAPE) + assert cli_project_id(path) == "tm_WYrcWDMnjZpG" + + +def test_a_named_profile_wins_over_the_default_table(tmp_path): + # `hookdeck --profile` writes a second table and repoints `profile`. Always + # reading [default] would report a project the CLI is not using. + path = tmp_path / "config.toml" + path.write_text( + 'profile = "work"\n\n' + '[default]\nproject_id = "tm_personal"\n\n' + '[work]\nproject_id = "tm_work"\n' + ) + assert cli_project_id(path) == "tm_work" + + +def test_a_missing_file_is_unknown_rather_than_an_error(tmp_path): + assert cli_project_id(tmp_path / "absent.toml") is None + + +def test_a_config_with_no_project_is_unknown(tmp_path): + path = tmp_path / "config.toml" + path.write_text('profile = "default"\n\n[default]\napi_key = "cli_abc"\n') + assert cli_project_id(path) is None + + +def test_a_profile_pointing_at_no_table_is_unknown(tmp_path): + path = tmp_path / "config.toml" + path.write_text('profile = "ghost"\n\n[default]\nproject_id = "tm_x"\n') + assert cli_project_id(path) is None + + +def test_garbage_is_unknown_rather_than_a_crash(tmp_path): + # doctor runs when things are already broken; a corrupt config must not be + # the thing that stops it reporting everything else. + path = tmp_path / "config.toml" + path.write_text("\x00\x01 not toml at all [[[") + assert cli_project_id(path) is None + + +def test_the_fallback_parser_handles_this_files_shape(): + # Exercised directly, because on 3.11+ tomllib means the fallback is never + # reached by the tests above — and 3.10 is a supported interpreter. + parsed = _parse_flat_toml(REAL_SHAPE) + assert parsed["profile"] == "default" + assert parsed["default"]["project_id"] == "tm_WYrcWDMnjZpG" + + +def test_the_fallback_parser_ignores_comments_and_blank_lines(): + parsed = _parse_flat_toml( + '# a comment\n\nprofile = "p"\n\n[p]\n # indented comment\n' + ' project_id = "tm_1"\n' + ) + assert parsed["p"]["project_id"] == "tm_1" diff --git a/tests/test_settings.py b/tests/test_settings.py index 969fb3c..f211059 100644 --- a/tests/test_settings.py +++ b/tests/test_settings.py @@ -112,3 +112,21 @@ def test_the_default_is_used_when_nothing_is_configured(monkeypatch): monkeypatch.setattr("hookdeck.settings.load_hermes_config", dict) assert configured_state_path() == default_state_path() + + +def test_a_gateway_owned_cli_config_is_resolved_to_an_absolute_path(): + from hookdeck.settings import AdapterSettings + + settings = AdapterSettings.from_extra( + {"secret": "s", "source": "src", "cli_config": "~/hermes/hookdeck.toml"} + ) + assert settings.cli_config.endswith("/hermes/hookdeck.toml") + assert "~" not in settings.cli_config + + +def test_the_cli_config_defaults_to_the_clis_own_default(): + from hookdeck.settings import AdapterSettings + + # Empty rather than a path: the flag is then omitted entirely, so the CLI + # applies its own default rather than being handed our guess at it. + assert AdapterSettings.from_extra({"secret": "s"}).cli_config == "" diff --git a/tests/test_tunnel.py b/tests/test_tunnel.py index 8d32f32..4c2f6d5 100644 --- a/tests/test_tunnel.py +++ b/tests/test_tunnel.py @@ -329,3 +329,148 @@ async def communicate_ok(self): assert "rewrites" in caplog.text assert spawned.commands[0][0][1:] == ("ci", "--api-key", "k") + + +# ---------------------------------------------------------------------- +# Escalating a failure that retrying cannot fix (#4) +# ---------------------------------------------------------------------- + +#: The output observed in #4, verbatim. The project the CLI is logged into has +#: no such source, so it invents one and then finds no connection for it. +PROJECT_MISMATCH_OUTPUT = [ + b'Source "hermes-livetest" not found.\n', + b'Non-interactive mode detected. Automatically creating source "hermes-livetest".\n', + b'no connection found matching filter "livetest" for source "hermes-livetest"\n', +] + + +def test_a_project_mismatch_is_recognised_from_the_cli_output(): + cause = tunnel_mod.diagnose( + [line.decode().strip() for line in PROJECT_MISMATCH_OUTPUT] + ) + assert "different Hookdeck project" in cause + assert "hermes hookdeck doctor" in cause + + +def test_an_expired_session_is_recognised(): + assert "hookdeck login" in tunnel_mod.diagnose( + ["Authentication failed: your API key is invalid or expired."] + ) + + +def test_output_with_no_known_cause_diagnoses_nothing(): + # Better to say only what is known than to guess a cause and send an + # operator after the wrong thing. + assert tunnel_mod.diagnose(["some unrecognised failure"]) == "" + assert tunnel_mod.diagnose([]) == "" + + +async def test_a_tunnel_stuck_in_a_restart_loop_escalates_and_names_the_cause( + spawned, no_waiting, caplog +): + # #4: the gateway logs that it is listening, the tunnel fails identically + # every couple of seconds at warning level, and nothing ever says the + # gateway is receiving no events. + for _ in range(10): + spawned.queue.append(FakeProcess(lines=list(PROJECT_MISMATCH_OUTPUT))) + + tunnel = HookdeckTunnel(port=1, path="/x", source="hermes-livetest") + with caplog.at_level(logging.INFO): + await _supervise_rounds(tunnel, 5, no_waiting) + + errors = [r.getMessage() for r in caplog.records if r.levelno >= logging.ERROR] + assert len(errors) == 1, "escalate once per streak, not on every restart" + assert "No events are reaching the gateway" in errors[0] + assert "different Hookdeck project" in errors[0] + # The operator needs the CLI's own words too, not just our interpretation. + assert "no connection found matching filter" in errors[0] + + +async def test_escalation_waits_for_a_streak_rather_than_one_bad_start( + spawned, no_waiting, caplog +): + # A single fast exit happens on an ordinary flap. Crying wolf there would + # make the error level meaningless for the case that matters. + for _ in range(10): + spawned.queue.append(FakeProcess(lines=list(PROJECT_MISMATCH_OUTPUT))) + + tunnel = HookdeckTunnel(port=1, path="/x", source="s") + await _supervise_rounds(tunnel, tunnel_mod._FAST_FAILURES_BEFORE_ESCALATING - 1, + no_waiting) + + assert [r for r in caplog.records if r.levelno >= logging.ERROR] == [] + + +async def test_a_healthy_run_rearms_the_escalation( + spawned, no_waiting, caplog, monkeypatch +): + # A tunnel that recovers and later breaks again deserves to be shouted + # about again — otherwise the second outage is silent. + clock = {"now": 0.0} + + class FakeLoop: + def time(self): + return clock["now"] + + monkeypatch.setattr(tunnel_mod.asyncio, "get_running_loop", lambda: FakeLoop()) + for _ in range(20): + spawned.queue.append(FakeProcess(lines=list(PROJECT_MISMATCH_OUTPUT))) + + tunnel = HookdeckTunnel(port=1, path="/x", source="s") + runs = {"n": 0} + original = tunnel._run_once + + async def timed_run(binary): + runs["n"] += 1 + # Three fast failures, one healthy session, then three more failures. + healthy = runs["n"] == 4 + clock["now"] += tunnel_mod._HEALTHY_RUN_SECONDS * 2 if healthy else 1.0 + await original(binary) + + tunnel._run_once = timed_run + await _supervise_rounds(tunnel, 7, no_waiting) + + errors = [r for r in caplog.records if r.levelno >= logging.ERROR] + assert len(errors) == 2, "once before the recovery, once after it breaks again" + + +async def test_the_diagnosis_does_not_outlive_the_run_that_produced_it(spawned): + # Carrying output across runs would name a cause that has since been + # fixed — the worst kind of wrong, because it looks specific. + tunnel = HookdeckTunnel(port=1, path="/x", source="s") + spawned.queue.append(FakeProcess(lines=list(PROJECT_MISMATCH_OUTPUT))) + await tunnel._run_once("/usr/bin/hookdeck") + assert tunnel_mod.diagnose(tunnel._recent_output) + + spawned.queue.append(FakeProcess(lines=[b"Ready!\n"])) + await tunnel._run_once("/usr/bin/hookdeck") + assert tunnel._recent_output == ["Ready!"] + assert tunnel_mod.diagnose(tunnel._recent_output) == "" + + +async def test_the_output_buffer_is_bounded(spawned): + # A chatty tunnel runs for days; the tail is what diagnoses, not the whole + # session. + tunnel = HookdeckTunnel(port=1, path="/x", source="s") + spawned.queue.append( + FakeProcess(lines=[f"line {i}\n".encode() for i in range(500)]) + ) + await tunnel._run_once("/usr/bin/hookdeck") + assert len(tunnel._recent_output) == tunnel_mod._OUTPUT_MEMORY + assert tunnel._recent_output[-1] == "line 499" + + +async def test_a_gateway_owned_config_is_passed_to_the_cli(): + # `--hookdeck-config` is what makes the CLI's project a plugin setting + # rather than ambient state shared with whatever else the operator runs. + args = HookdeckTunnel( + port=1, path="/x", source="s", config_path="/etc/hermes/hookdeck.toml" + ).listen_args() + assert args[args.index("--hookdeck-config") + 1] == "/etc/hermes/hookdeck.toml" + + +def test_no_config_flag_is_passed_when_none_is_configured(): + # Absent, not empty: `--hookdeck-config ""` would point the CLI at nothing. + assert "--hookdeck-config" not in HookdeckTunnel( + port=1, path="/x", source="s" + ).listen_args()