diff --git a/README.md b/README.md index ced8401..fe4cd54 100644 --- a/README.md +++ b/README.md @@ -73,6 +73,84 @@ To add a new integration: - The repo ships only `config.example.toml`, documenting all fields and defaults. - CI/CD can inject secrets via environment-variable expansion in config parsing if needed. +## Cloud transport + +By default, `BusyBarClient` talks to your BUSY Bar directly over the LAN +(`[device].host`). As of v1.6, it can automatically fall back to BUSY's +cloud relay if the local device becomes unreachable — USB unplugged, +Wi-Fi drop, the device off — and recover back to local on its own once +it's reachable again. This is entirely optional and off by default. + +### Setting it up + +1. Create a token at [cloud.busy.app](https://cloud.busy.app) → **API + tokens** tab → create a new token with the **"BUSY Bar"** scope. This + scope grants full control of exactly one linked device — there's no + separate device ID to configure; the token itself identifies which + device it talks to. +2. Add it to your `config.toml` (**never** `config.example.toml`, and + never anything committed to the repo — see "Configuration" above): + ```toml + [device] + host = "10.0.4.20" + cloud_token = "paste-your-real-token-here" + ``` +3. Optionally set `transport` (default `"auto"`): + - `"auto"` — local first, cloud fallback when the local device is + unreachable and `cloud_token` is set. Recovers back to local + automatically. + - `"local"` — local only, never falls back (identical to pre-v1.6 + behavior; the default if you never set `cloud_token`). + - `"cloud"` — forced cloud only, never attempts local. Mainly useful + for deliberately exercising/debugging the cloud path. +4. `cloud_base_url` defaults to `https://api.busy.app/busybar` and + normally doesn't need to change — see the base-URL note below. + +### Rotating or revoking a token + +Manage tokens from the same **API tokens** tab on cloud.busy.app. +Revoking a token takes effect **immediately and cannot be undone** — if +you're rotating, create and deploy the replacement token first, then +revoke the old one, rather than revoking first. + +### What cloud fallback does NOT cover + +Continuous status streaming (`/api/status/ws`) is local-only by design — +the cloud API has no equivalent, so a caller relying on the status +WebSocket will not get a cloud fallback for it. Everything else this +client uses (`draw`, `clear`, `status`, `get_busy`, `set_busy_simple`, +`play_audio`) is a synchronous request/response call and mirrors 1:1 +over cloud. + +### Post-merge live-probe checklist + +This round's tests are entirely mocked — no cloud request has been made +against a real token, since the operator hadn't provisioned one yet. +**Before relying on cloud fallback in practice**, run this checklist +once a real `cloud_token` is in `config.toml`: + +1. **Forced-cloud draw probe.** Set `transport = "cloud"` temporarily + and run a `client.draw(...)` (e.g. via either integration's + `--once --dry-run=false` path, or a one-off script) to confirm the + token is valid and a real device draw round-trips over the cloud + relay end to end. +2. **Cadence headroom check.** No rate limit or cadence guidance is + documented anywhere for the cloud API (see + `scratchpad/busy-cloud-api-research.md`'s "Open items"). Run + `calendar_countdown` (10s ambient redraw cadence) forced onto cloud + transport for a few minutes and confirm no throttling/errors show up + before trusting cloud fallback to hold up under sustained polling. +3. **Base-URL ambiguity.** This codebase defaults `cloud_base_url` to + `https://api.busy.app/busybar`, but busylib-py's own hardcoded default + is the differently-hosted `https://proxy.busy.app` — an unresolved + discrepancy in the source research, not something this round's mocked + tests can settle. Confirm which base actually works against a live + token (or whether both do) and update the default/docs here if + `api.busy.app/busybar` turns out to be wrong or non-canonical. + +Set `transport` back to `"auto"` (or leave it, since `"auto"` is the +default) once the checklist above passes. + ## What's inside | Integration | Description | diff --git a/config.example.toml b/config.example.toml index cdd69b2..6bb5e19 100644 --- a/config.example.toml +++ b/config.example.toml @@ -3,6 +3,27 @@ [device] host = "10.0.4.20" # USB-Ethernet default; set your LAN IP for Wi-Fi +# Cloud transport fallback (v1.6, optional). When cloud_token is set, +# BusyBarClient automatically falls back to BUSY's cloud relay if the +# local device becomes unreachable (USB unplugged, Wi-Fi drop, etc.), and +# recovers back to local automatically once the device is reachable +# again. Leave cloud_token empty (the default) to disable cloud fallback +# entirely -- behavior is then identical to pre-v1.6. +# +# Mint a token at https://cloud.busy.app -> "API tokens" tab -> create a +# token with the "BUSY Bar" scope (full device control, tied to exactly +# one linked device -- no separate device id needed here). Revoking a +# token from the dashboard takes effect immediately and cannot be undone; +# create the replacement token first if you're rotating. +# +# The real token belongs ONLY in your git-ignored config.toml -- never +# here, and never committed anywhere. See README.md's "Cloud transport" +# section for the full walkthrough. +cloud_token = "" +cloud_base_url = "https://api.busy.app/busybar" +transport = "auto" # "auto" (local, fall back to cloud) | "local" | "cloud" (forced -- + # mainly for deliberately testing the cloud path) + [calendar_countdown] poll_seconds = 10 # ambient-tier redraw cadence (default: 10) -- matches the running-CI # overlay's 10s dwell gap so this app's redraws reliably land inside diff --git a/docs/superpowers/specs/2026-08-03-calendar-ci-integrations-design.md b/docs/superpowers/specs/2026-08-03-calendar-ci-integrations-design.md index a4b8a95..491f868 100644 --- a/docs/superpowers/specs/2026-08-03-calendar-ci-integrations-design.md +++ b/docs/superpowers/specs/2026-08-03-calendar-ci-integrations-design.md @@ -1460,3 +1460,210 @@ operator directly ear-verified the corrected `.snd` call as part of reporting the original bug, which is stronger evidence than a fresh scripted probe could add (a scripted probe can only confirm HTTP `200` again, exactly the signal already shown not to prove audibility). + +## 2026-08-04 — v1.6 cloud transport fallback + +**Status:** Implemented, branch `dev/claude/cloud-transport-v1.6` off +`main` (top of `main` at branch time: PR #13, the chirp-`.snd` fix). Not +pushed. **No live cloud verification performed this round** — the +operator hadn't provisioned a real `cloud_token` yet; see the README's +"Post-merge live-probe checklist" for what runs once one exists. + +Operator-approved feature, based on `scratchpad/busy-cloud-api-research.md` +(source citations: `busy-app/busylib-py` on GitHub, docs.busy.app, and the +live cloud OpenAPI spec at `api.busy.app/busybar/openapi.yaml`). BUSY +exposes a cloud relay in addition to the device's local HTTP API — same +endpoint surface, mounted under `/busybar/...` instead of `/api/...`, bearer- +token authenticated, synchronous (the relay bridges the HTTP call over MQTT +to the device and holds the connection until it replies, so callers see the +same 200/409 semantics as local — no async/poll pattern to add). One +confirmed gap: `/api/status/ws` (continuous status streaming) has no cloud +equivalent — local-only by construction, not something this round could +paper over. + +### `BusyBarClient` transport layer (`src/busybar/client.py`) + +Single-class transport-flag design, mirroring busylib-py's own pattern +(one class, a mode flag branching request construction) rather than a +class hierarchy — matches this codebase's existing single-`BusyBarClient` +shape and needed no changes to `DrawResult`'s enum members or to any +consumer's call to `draw`/`clear`/`status`/`get_busy`/`set_busy_simple`/ +`play_audio`; the fallback lives entirely inside `_request`. + +New constructor kwargs: `cloud_token` (default `""`, disables cloud +fallback entirely when empty), `cloud_base_url` (default +`"https://api.busy.app/busybar"`), `transport` (`"auto"` | `"local"` | +`"cloud"`, default `"auto"`), `cloud_timeout` (fixed `(5, 15)`, not +config-exposed — longer than local's `(3, 5)` since a cloud round-trip +crosses the public internet and bridges over MQTT to the device rather +than a direct LAN hop). + +- **`"local"`**: unchanged pre-v1.6 behavior exactly — `_request` calls + local only, never falls back, regardless of `cloud_token`. +- **`"cloud"`**: forced — `_request` calls cloud only, never attempts + local. For deliberately exercising/debugging the cloud path (e.g. the + live-probe checklist below). +- **`"auto"`** (default): local-first-with-cloud-fallback. + `active_transport` (`"local"` | `"cloud"`) tracks which transport last + succeeded. On a fresh/healthy client, every call tries local first with + the existing `(3, 5)` timeout; on a `requests.RequestException` AND a + non-empty `cloud_token`, the SAME request (same method, path, body) is + retried against `cloud_base_url` with `cloud_timeout` and an + `Authorization: Bearer ` header. A successful cloud call + transitions `active_transport` to `"cloud"` (logged once at INFO, + transition only — not on every request while already degraded). + - **Local-recovery probe** (`LOCAL_RETRY_SECONDS = 60`): while + degraded (`active_transport == "cloud"`), `_request` skips the local + attempt entirely and goes straight to cloud until + `LOCAL_RETRY_SECONDS` have elapsed since the last local failure, at + which point the next call tries local first again as a recovery + probe. Doing this inline per-request (no background prober thread) + is cheap specifically because a down local device fails fast + (connection refused/timeout, well under even the `(3, 5)` local + timeout) — the occasional 60s-interval probe costs little even if + local is still down, and if the probe itself fails, the client falls + through to cloud for that same request and resets the degraded timer + to the probe's own failure time (so the next probe is another full + window out, not immediately retried). + - **Path mapping**: local paths are `/api/`; per the research + doc, cloud mirrors them 1:1 under `/busybar/` relative to + the cloud host. Since `cloud_base_url`'s documented default already + carries that `/busybar` segment, `_cloud_path` simply strips the + local `/api` prefix and lets `cloud_base_url` supply the rest (e.g. + local `/api/display/draw` → cloud tail `/display/draw` → full URL + `https://api.busy.app/busybar/display/draw`). + - **`DrawResult.UNREACHABLE` redefinition**: now means both local AND + cloud (when configured) failed for this call — previously it only + ever meant local failed, since there was no other transport. When + `cloud_token` is empty, behavior is unchanged from pre-v1.6: a local + failure alone is `UNREACHABLE`, no cloud attempt is made at all. + +**Base-URL discrepancy, deliberately left unresolved this round.** +busylib-py's own hardcoded default is `https://proxy.busy.app` — a +different host entirely from this codebase's `https://api.busy.app/busybar` +default. The research doc flags this as unresolved; this round follows the +operator's explicit instruction to default to `api.busy.app` here and defer +resolution to the post-merge live probe (README checklist item 3) rather +than guessing which is authoritative without a real token to test against. + +**Security: `cloud_token` is never logged, at any level including DEBUG.** +Only transport *transitions* are logged (INFO), and those log lines are +static strings with no header/token interpolation — grep confirms no +f-string, `%s`, or `.format()` call anywhere in `client.py` ever +interpolates `cloud_token` or a header dict into a log call. A dedicated +`caplog`-based test (`test_cloud_token_never_appears_in_log_output`) +exercises both the degrade and the both-transports-fail paths and asserts +the placeholder token string never appears in any captured log record's +formatted message or args. + +### Config (`src/busybar/config.py`, `config.example.toml`) + +New `[device]` keys, added to `DEFAULTS["device"]` and mirrored in +`config.example.toml`: `cloud_token = ""`, `cloud_base_url = +"https://api.busy.app/busybar"`, `transport = "auto"`. Key names were +chosen to match `BusyBarClient`'s constructor kwargs exactly, so both +integration call sites (`calendar_countdown/main.py`, +`ci_status/main.py`) simplify from `BusyBarClient(host=cfg["device"] +["host"])` to `BusyBarClient(**cfg["device"])` — a single point of +plumbing rather than adding three more explicit keyword arguments at each +call site, and automatically future-proof against a v1.7 adding a fourth +`[device]` key. + +### Verification + +`TZ=UTC uv run pytest -v`: 337 passed (323 at the start of this branch's +work, after the chirp-`.snd` fix line). New coverage (`tests/test_client.py`): +auto-mode fallback on local failure (asserts both the local call and the +subsequent cloud call's exact URL), no fallback when `cloud_token` is +empty, `UNREACHABLE` requiring both transports to fail, forced `"local"` +never attempting cloud even on failure, forced `"cloud"` always going +straight to cloud, the cloud request's exact shape (Bearer header, +`cloud_timeout`, base URL), the `/api` → `/busybar` path mapping, three +recovery-probe timing tests (skip-local within the window, probe-and- +recover once the window elapses, probe-fails-so-stays-cloud-and-resets- +timer), a fresh-client sanity check that the skip logic never fires before +any degradation, and the token-never-logged `caplog` test. New coverage +(`tests/test_config.py`): the three new `[device]` defaults, and two +config.example.toml parity tests (device section value-for-value matches +`DEFAULTS["device"]`, and `cloud_token` in the shipped example is +literally `""` — not a placeholder that merely looks non-empty). + +### Docs + +README gained a "Cloud transport" section: token creation walkthrough +(cloud.busy.app → API tokens tab → "BUSY Bar" scope), config placement +(explicitly: the real token lives only in git-ignored `config.toml`, +never `config.example.toml`, never committed), rotation guidance +(revocation is immediate and irreversible — create the replacement before +revoking the old one), the status-WS cloud gap, and the post-merge +live-probe checklist (forced-cloud draw probe, cadence headroom check at +the 10s ambient-redraw cadence, base-URL ambiguity resolution) — +duplicated in condensed form above so both the report and the operator- +facing docs carry it. + +### Deferred to the operator (explicit — not an oversight) + +No live cloud verification was performed or attempted this round, per +the coordinator's explicit instruction: the operator had not yet inserted +a real `cloud_token`. All 12 new `test_client.py` tests and both new +`test_config.py` tests use `requests` mocks and a literal +placeholder-string token (`"test-placeholder-token-do-not-use"`) — no +network call was made to any `busy.app` host during this round's work. +The three-item live-probe checklist above is the explicit handoff for +the controller/operator pass that runs once a token exists. + +### Final-gate review fixes (same branch) + +The coordinator's final-gate review found two Important issues and three +Minor issues before merge: + +1. **Important — `**cfg["device"]` crash surface.** Both call sites + splatting the raw `[device]` dict meant an unknown/typo'd key (e.g. + `coud_token` for `cloud_token`) went from silently-ignored (pre-v1.6, + when only `host=` was ever passed explicitly) to a `TypeError` + crashing startup — exactly the wrong failure mode for a typo made + while first configuring `cloud_token`. Fixed with a new + `busybar.config.device_kwargs(cfg)` helper: filters `cfg["device"]` to + `BusyBarClient`'s actual constructor kwargs (introspected via + `inspect.signature`, so it can't drift out of sync with the + constructor) and logs a `WARNING` naming each dropped key, restoring + "ignored, not fatal" while adding the observability the pre-v1.6 code + never had. Both call sites now read + `BusyBarClient(**device_kwargs(cfg))`. Five new tests in + `test_config.py`: known keys pass through unchanged, an unknown key is + dropped without crashing (`caplog` confirms the `WARNING` names it), + constructing `BusyBarClient` from the filtered result never raises, + one `WARNING` fires per unknown key (not one combined message), and no + warnings fire when every key is known. +2. **Important — untested highest-risk semantic.** Added + `test_local_http_500_is_error_and_does_not_trigger_cloud_fallback`: + with `cloud_token` configured, a local HTTP `500` (a real response, no + exception) must return `DrawResult.ERROR` and must NOT attempt cloud + at all — locks in that fallback triggers strictly on + `requests.RequestException`, never on a non-2xx/409 response the + device actually returned. This was previously only implied by the + `_request` implementation, not directly asserted. +3. **Minor — inaccurate comment fixed.** + `test_cloud_token_never_appears_in_log_output`'s second `draw()` call + was commented "local fails again, cloud fails too" — wrong: per the + recovery-probe design, a call made well within `LOCAL_RETRY_SECONDS` + of degrading skips the local attempt entirely and goes straight to + cloud, so only one `requests.request` call happens on that second + draw. Corrected the comment and, per the reviewer's suggestion, added + `@patch("busybar.client.time.monotonic")` to control elapsed time + explicitly rather than relying on real wall-clock time staying under + 60s between two adjacent test statements. +4. **Minor — cloud-409 coverage added.** Two new tests: + `test_cloud_409_is_rejected_not_error_forced_cloud` (forced + `transport="cloud"`) and `test_cloud_409_is_rejected_not_error_while_degraded` + (auto mode, already degraded to cloud) — both assert a cloud `409` + maps to `DrawResult.REJECTED`, mirroring the existing local-409 + coverage. +5. **Minor — transport `ValueError` guard tested.** + `test_invalid_transport_value_raises_value_error` asserts + `BusyBarClient(transport="carrier-pigeon")` raises `ValueError` naming + the bad value — previously implemented but unverified by any test. + +`TZ=UTC uv run pytest -v`: 346 passed (337 before this review round's 9 +net-new tests: 5 in `test_config.py` for `device_kwargs`, 4 in +`test_client.py` for the 500/409/ValueError contracts). diff --git a/integrations/calendar_countdown/main.py b/integrations/calendar_countdown/main.py index eee6406..b83a7fb 100644 --- a/integrations/calendar_countdown/main.py +++ b/integrations/calendar_countdown/main.py @@ -12,7 +12,7 @@ from datetime import datetime, timezone from busybar.client import BusyBarClient, DrawResult -from busybar.config import load_config +from busybar.config import device_kwargs, load_config from busybar.display import PRIORITY_AMBIENT, ambient_timeout from .logic import (ascii_safe, build_elements, select_active_event, @@ -215,7 +215,7 @@ def main() -> int: ordering_warning = check_threshold_ordering(cfg["calendar_countdown"]) if ordering_warning is not None: log.warning(ordering_warning) - client = BusyBarClient(host=cfg["device"]["host"]) + client = BusyBarClient(**device_kwargs(cfg)) # Drop any stale elements from a previous process. This also protects a # restart onto this version against every id change made across the # v1.3 -> v1.3.1 -> v1.4 line: v1.3.1 replaced the native "countdown" diff --git a/integrations/ci_status/main.py b/integrations/ci_status/main.py index 1206656..c06504e 100644 --- a/integrations/ci_status/main.py +++ b/integrations/ci_status/main.py @@ -12,7 +12,7 @@ from datetime import datetime, timedelta, timezone from busybar.client import BusyBarClient, DrawResult -from busybar.config import load_config +from busybar.config import device_kwargs, load_config from busybar.display import OVERLAY_DWELL_SECONDS, overlay_gap_elapsed from .logic import ( @@ -366,7 +366,7 @@ def main() -> int: except RuntimeError as exc: log.error(str(exc)) return 1 - client = BusyBarClient(host=cfg["device"]["host"]) + client = BusyBarClient(**device_kwargs(cfg)) client.clear(APP) # drop any stale elements from a previous process (type collisions 400) state_cache: dict[str, RepoState] = {} diff --git a/src/busybar/client.py b/src/busybar/client.py index a335e76..1eccede 100644 --- a/src/busybar/client.py +++ b/src/busybar/client.py @@ -8,26 +8,137 @@ NULL_CARD_ID = "00000000-0000-0000-0000-000000000000" +# v1.6 cloud transport fallback -- while degraded to cloud, `_request` skips +# the (known-failing) local attempt and goes straight to cloud until this +# many seconds have elapsed since the last local failure, then tries local +# first again as a recovery probe. Doing this inline (no background thread) +# is cheap because a down local device fails fast (connection refused/ +# timeout well under `timeout`), so the occasional probe costs little. +LOCAL_RETRY_SECONDS = 60 + class DrawResult(Enum): DRAWN = "drawn" REJECTED = "rejected" # 409: higher-priority app on screen — expected - UNREACHABLE = "unreachable" # device off / USB unplugged — caller backs off + UNREACHABLE = "unreachable" # local AND cloud (if configured) both failed — caller backs off ERROR = "error" # non-200/409 from a live device — no backoff; retried next poll class BusyBarClient: - def __init__(self, host: str = "10.0.4.20", timeout: tuple = (3, 5)): + """Talks to a single BUSY Bar device, over the LAN (local transport, + the default and preferred path) and, when configured, via BUSY's cloud + relay as an automatic fallback when the local device is unreachable. + + Transport selection (`transport`, mirroring busylib-py's single-class + transport-flag pattern rather than a class hierarchy -- see + scratchpad/busy-cloud-api-research.md for the source citations): + + - `"auto"` (default): every call tries local first with `timeout`. On + a `requests.RequestException` AND a non-empty `cloud_token`, the + SAME request is retried against `cloud_base_url` with + `cloud_timeout` and an `Authorization: Bearer` header. ` + active_transport` tracks which transport last succeeded. While + degraded (`active_transport == "cloud"`), subsequent calls skip the + local attempt and go straight to cloud until `LOCAL_RETRY_SECONDS` + have elapsed since the last local failure, at which point local is + retried first again as a recovery probe (see module docstring + constant above). `cloud_token = ""` (the shipped default) disables + cloud fallback entirely regardless of `transport="auto"` -- calls + behave exactly as they did before v1.6. + - `"local"`: local only, never falls back. Pre-v1.6 behavior exactly. + - `"cloud"`: cloud only, forced -- never attempts local. For + deliberately testing/debugging the cloud path. + + Local endpoints are mounted at `/api/...`; the cloud API mirrors them + 1:1 under `/busybar/...` relative to the cloud host. `cloud_base_url`'s + documented default (`https://api.busy.app/busybar`) already carries + that `/busybar` segment, so cloud requests are built by stripping the + local `/api` prefix and appending the remainder to `cloud_base_url`. + + SECURITY: `cloud_token` is never logged or included in any log + statement, at any level including DEBUG -- only transport + *transitions* (local->cloud degradation, cloud->local recovery) are + logged, at INFO, and those log lines never include header values. + """ + + def __init__(self, host: str = "10.0.4.20", timeout: tuple = (3, 5), *, + cloud_token: str = "", cloud_base_url: str = "https://api.busy.app/busybar", + transport: str = "auto", cloud_timeout: tuple = (5, 15)): + if transport not in ("auto", "local", "cloud"): + raise ValueError(f"transport must be 'auto', 'local', or 'cloud', got {transport!r}") self.base = f"http://{host}" self.timeout = timeout + self.cloud_token = cloud_token + self.cloud_base = cloud_base_url.rstrip("/") + self.cloud_timeout = cloud_timeout + self.transport = transport + # Cloud fallback is only "configured" with a non-empty token; an + # empty string (the shipped default) disables it in "auto" mode + # regardless of anything else. Forced transport="cloud" is exempt + # from this gate deliberately -- it's the caller's explicit, + # non-"auto" choice, not a fallback decision this client makes. + self._cloud_configured = bool(cloud_token) + self.active_transport = "cloud" if transport == "cloud" else "local" + self._degraded_since: float | None = None # time.monotonic() of the last local + # failure while in "auto" mode; None + # whenever active_transport == "local" - def _request(self, method: str, path: str, **kwargs) -> requests.Response | None: + def _mark_degraded(self) -> None: + if self.active_transport != "cloud": + log.info("busybar transport: local -> cloud (local device unreachable; falling back)") + self.active_transport = "cloud" + self._degraded_since = time.monotonic() + + def _mark_recovered(self) -> None: + if self.active_transport != "local": + log.info("busybar transport: cloud -> local (local device reachable again)") + self.active_transport = "local" + self._degraded_since = None + + def _should_probe_local(self) -> bool: + return (self._degraded_since is not None + and (time.monotonic() - self._degraded_since) >= LOCAL_RETRY_SECONDS) + + def _cloud_path(self, path: str) -> str: + return path[len("/api"):] if path.startswith("/api") else path + + def _try_local(self, method: str, path: str, **kwargs) -> requests.Response | None: try: return requests.request(method, f"{self.base}{path}", timeout=self.timeout, **kwargs) except requests.RequestException as exc: log.debug("device unreachable: %s", exc) return None + def _try_cloud(self, method: str, path: str, **kwargs) -> requests.Response | None: + headers = {**(kwargs.pop("headers", None) or {}), "Authorization": f"Bearer {self.cloud_token}"} + try: + return requests.request(method, f"{self.cloud_base}{self._cloud_path(path)}", + timeout=self.cloud_timeout, headers=headers, **kwargs) + except requests.RequestException as exc: + log.debug("cloud unreachable: %s", exc) + return None + + def _request(self, method: str, path: str, **kwargs) -> requests.Response | None: + if self.transport == "local": + return self._try_local(method, path, **kwargs) + + if self.transport == "cloud": + return self._try_cloud(method, path, **kwargs) + + # transport == "auto": local-first-with-cloud-fallback, with the + # LOCAL_RETRY_SECONDS recovery probe described in the class + # docstring. + if self.active_transport == "local" or self._should_probe_local(): + resp = self._try_local(method, path, **kwargs) + if resp is not None: + self._mark_recovered() + return resp + if not self._cloud_configured: + return None + self._mark_degraded() + + return self._try_cloud(method, path, **kwargs) + def draw(self, application_name: str, elements: list[dict], priority: int = 50, led_notification_color: str | None = None) -> DrawResult: body: dict = {"application_name": application_name, "priority": priority, diff --git a/src/busybar/config.py b/src/busybar/config.py index 9390588..f8fb690 100644 --- a/src/busybar/config.py +++ b/src/busybar/config.py @@ -1,10 +1,26 @@ import copy +import inspect +import logging import os import tomllib from pathlib import Path +from busybar.client import BusyBarClient + +log = logging.getLogger(__name__) + DEFAULTS: dict = { - "device": {"host": "10.0.4.20"}, + "device": { + "host": "10.0.4.20", + # Cloud transport fallback (v1.6) -- see README's "Cloud + # transport" section before setting these. An empty cloud_token + # (the shipped default) disables cloud fallback entirely in + # "auto" mode; keys here MUST match BusyBarClient's constructor + # kwarg names exactly since call sites pass **cfg["device"]. + "cloud_token": "", + "cloud_base_url": "https://api.busy.app/busybar", + "transport": "auto", # "auto" | "local" | "cloud" (forced) + }, "calendar_countdown": { # 10s matches busybar.display.AMBIENT_REDRAW_SECONDS -- the ambient # tier's redraw contract, tuned (after on-device re-measurement @@ -69,6 +85,36 @@ def _merge(base: dict, override: dict) -> dict: return out +def device_kwargs(cfg: dict) -> dict: + """Filter `cfg["device"]` down to BusyBarClient's known constructor + kwargs, for use as `BusyBarClient(**device_kwargs(cfg))`. + + Before v1.6 both integration call sites only ever passed a single + explicit keyword (`host=cfg["device"]["host"]`), so an unknown/typo'd + [device] key in config.toml (e.g. `coud_token` for `cloud_token`) was + silently ignored -- it just never got read. v1.6 switched both call + sites to splat the whole [device] table so the three new cloud- + transport keys wouldn't need updating twice; splatting an *unfiltered* + dict, though, means that same typo now raises TypeError("unexpected + keyword argument") at startup instead -- a cryptic crash, and the + worst possible failure mode for exactly the moment an operator is + most likely to typo a key: first-time cloud_token setup. This + restores "unknown key doesn't crash startup" while adding + observability the pre-v1.6 code never had: each ignored key is + logged at WARNING (not silently dropped) so a genuine typo is still + visible, just not fatal. + + The known-kwargs set is derived from BusyBarClient's own signature + (rather than hardcoded here) so it can't drift out of sync with the + constructor as transport options evolve. + """ + known = set(inspect.signature(BusyBarClient.__init__).parameters) - {"self"} + device = cfg.get("device", {}) + for key in sorted(set(device) - known): + log.warning("config: ignoring unknown [device] key %r (not a BusyBarClient parameter)", key) + return {k: v for k, v in device.items() if k in known} + + def find_config() -> Path | None: candidate = Path(__file__).resolve().parents[2] / "config.toml" return candidate if candidate.exists() else None diff --git a/tests/test_client.py b/tests/test_client.py index 998899b..58d57aa 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -1,3 +1,4 @@ +import logging from unittest.mock import Mock, patch import requests from busybar.client import BusyBarClient, DrawResult @@ -150,3 +151,233 @@ def test_play_audio_never_touches_volume_endpoint(mock_request): BusyBarClient().play_audio("app", stock_path="shared/x.wav") for call in mock_request.call_args_list: assert "/api/audio/volume" not in call.args[1] + + +# --- cloud transport fallback (v1.6) ---------------------------------------- +# Placeholder tokens only -- never anything resembling a real credential. +FAKE_TOKEN = "test-placeholder-token-do-not-use" + + +def _cloud_client(host="192.0.2.1", **kwargs): + return BusyBarClient(host=host, cloud_token=FAKE_TOKEN, + cloud_base_url="https://cloud.example.test/busybar", **kwargs) + + +@patch("busybar.client.requests.request") +def test_auto_falls_back_to_cloud_on_local_failure(mock_request): + mock_request.side_effect = [requests.ConnectionError(), _response(200)] + client = _cloud_client() + assert client.draw("app", ELEMENTS) == DrawResult.DRAWN + assert mock_request.call_count == 2 + local_call, cloud_call = mock_request.call_args_list + assert local_call.args == ("POST", "http://192.0.2.1/api/display/draw") + assert cloud_call.args == ("POST", "https://cloud.example.test/busybar/display/draw") + assert client.active_transport == "cloud" + + +@patch("busybar.client.requests.request") +def test_no_fallback_when_cloud_unconfigured(mock_request): + mock_request.side_effect = requests.ConnectionError() + client = BusyBarClient(host="192.0.2.1") # cloud_token defaults to "" + assert client.draw("app", ELEMENTS) == DrawResult.UNREACHABLE + assert mock_request.call_count == 1 # only local was ever attempted + assert client.active_transport == "local" # never transitions without cloud configured + + +@patch("busybar.client.requests.request") +def test_unreachable_when_both_transports_fail(mock_request): + mock_request.side_effect = requests.ConnectionError() + client = _cloud_client() + assert client.draw("app", ELEMENTS) == DrawResult.UNREACHABLE + assert mock_request.call_count == 2 # local AND cloud were both attempted + assert client.active_transport == "cloud" + + +@patch("busybar.client.requests.request") +def test_forced_local_never_attempts_cloud_even_on_failure(mock_request): + mock_request.side_effect = requests.ConnectionError() + client = _cloud_client(transport="local") + assert client.draw("app", ELEMENTS) == DrawResult.UNREACHABLE + assert mock_request.call_count == 1 + assert mock_request.call_args.args[1].startswith("http://192.0.2.1") + + +@patch("busybar.client.requests.request") +def test_forced_cloud_always_goes_straight_to_cloud(mock_request): + mock_request.return_value = _response(200) + client = _cloud_client(transport="cloud") + assert client.draw("app", ELEMENTS) == DrawResult.DRAWN + assert mock_request.call_count == 1 + method, url = mock_request.call_args.args + assert method == "POST" and url == "https://cloud.example.test/busybar/display/draw" + assert client.active_transport == "cloud" + + +@patch("busybar.client.requests.request") +def test_cloud_request_shape_bearer_header_and_base_url(mock_request): + mock_request.side_effect = [requests.ConnectionError(), _response(200)] + client = _cloud_client() + client.draw("app", ELEMENTS, priority=30) + cloud_call = mock_request.call_args_list[1] + assert cloud_call.args == ("POST", "https://cloud.example.test/busybar/display/draw") + assert cloud_call.kwargs["headers"] == {"Authorization": f"Bearer {FAKE_TOKEN}"} + assert cloud_call.kwargs["timeout"] == (5, 15) + assert cloud_call.kwargs["json"]["application_name"] == "app" + + +@patch("busybar.client.requests.request") +def test_cloud_path_mapping_strips_api_prefix(mock_request): + mock_request.side_effect = [requests.ConnectionError(), _response(200, {})] + client = _cloud_client() + client.status() + cloud_call = mock_request.call_args_list[1] + assert cloud_call.args == ("GET", "https://cloud.example.test/busybar/status") + + +@patch("busybar.client.time.monotonic") +@patch("busybar.client.requests.request") +def test_degraded_client_skips_local_within_retry_window(mock_request, mock_time): + # First call degrades to cloud at t=0. A second call at t=30 (well + # inside LOCAL_RETRY_SECONDS=60) must skip the local attempt entirely + # and go straight to cloud -- only one requests.request call for the + # second draw, and it must be the cloud URL. + mock_time.return_value = 0.0 + mock_request.side_effect = [requests.ConnectionError(), _response(200)] + client = _cloud_client() + assert client.draw("app", ELEMENTS) == DrawResult.DRAWN + assert mock_request.call_count == 2 # local (failed) + cloud (succeeded) + + mock_time.return_value = 30.0 + mock_request.reset_mock() + mock_request.side_effect = None + mock_request.return_value = _response(200) + assert client.draw("app", ELEMENTS) == DrawResult.DRAWN + assert mock_request.call_count == 1 # local skipped -- straight to cloud + method, url = mock_request.call_args.args + assert url == "https://cloud.example.test/busybar/display/draw" + + +@patch("busybar.client.time.monotonic") +@patch("busybar.client.requests.request") +def test_degraded_client_probes_local_after_retry_window_elapses(mock_request, mock_time): + # Degrade at t=0, then let LOCAL_RETRY_SECONDS (60) elapse: the next + # call must try local FIRST again (the recovery probe), and recover + # to active_transport == "local" when it succeeds. + mock_time.return_value = 0.0 + mock_request.side_effect = [requests.ConnectionError(), _response(200)] + client = _cloud_client() + client.draw("app", ELEMENTS) + assert client.active_transport == "cloud" + + mock_time.return_value = 61.0 # LOCAL_RETRY_SECONDS elapsed + mock_request.reset_mock() + mock_request.side_effect = None + mock_request.return_value = _response(200) # local now recovered + assert client.draw("app", ELEMENTS) == DrawResult.DRAWN + assert mock_request.call_count == 1 # local probe succeeded, no cloud needed + method, url = mock_request.call_args.args + assert url == "http://192.0.2.1/api/display/draw" + assert client.active_transport == "local" + + +@patch("busybar.client.time.monotonic") +@patch("busybar.client.requests.request") +def test_degraded_client_reprobes_local_and_stays_cloud_if_still_down(mock_request, mock_time): + # Recovery probe fires after the window elapses but local is STILL + # down: client must fall back to cloud again for that same request + # (not return UNREACHABLE just because the probe failed) and reset + # the degraded timer so the next probe is another window out. + mock_time.return_value = 0.0 + mock_request.side_effect = [requests.ConnectionError(), _response(200)] + client = _cloud_client() + client.draw("app", ELEMENTS) + + mock_time.return_value = 61.0 + mock_request.side_effect = [requests.ConnectionError(), _response(200)] # local still down, cloud up + assert client.draw("app", ELEMENTS) == DrawResult.DRAWN + assert client.active_transport == "cloud" + assert client._degraded_since == 61.0 # timer reset to the failed probe's time + + +@patch("busybar.client.requests.request") +def test_no_recovery_probe_before_window_when_never_degraded(mock_request): + # A freshly-constructed "auto" client (active_transport == "local") + # always tries local first, regardless of LOCAL_RETRY_SECONDS -- the + # probe-skip logic only applies once actually degraded. + mock_request.return_value = _response(200) + client = _cloud_client() + assert client.active_transport == "local" + assert client.draw("app", ELEMENTS) == DrawResult.DRAWN + assert mock_request.call_count == 1 + assert mock_request.call_args.args[1] == "http://192.0.2.1/api/display/draw" + + +# --- token never logged (v1.6 security requirement) ------------------------- + +@patch("busybar.client.time.monotonic") +@patch("busybar.client.requests.request") +def test_cloud_token_never_appears_in_log_output(mock_request, mock_time, caplog): + caplog.set_level(logging.DEBUG, logger="busybar.client") + mock_time.return_value = 0.0 + mock_request.side_effect = [requests.ConnectionError(), _response(200)] + client = _cloud_client() + client.draw("app", ELEMENTS) # local fails, cloud succeeds -- degrades to cloud + + # Still well within LOCAL_RETRY_SECONDS (60): per the recovery-probe + # design (see test_degraded_client_skips_local_within_retry_window), + # this second call skips the local attempt entirely and goes straight + # to cloud -- only ONE requests.request call happens here, not two. + mock_time.return_value = 30.0 + mock_request.side_effect = [requests.ConnectionError()] # the sole (cloud) attempt fails + assert client.draw("app", ELEMENTS) == DrawResult.UNREACHABLE + + for record in caplog.records: + assert FAKE_TOKEN not in record.getMessage() + assert FAKE_TOKEN not in str(record.args) + + +# --- fallback-only-on-RequestException contract (final-gate review) -------- + +@patch("busybar.client.requests.request") +def test_local_http_500_is_error_and_does_not_trigger_cloud_fallback(mock_request): + # Highest-risk semantic in the whole feature: a local HTTP error + # response (no exception -- the device is reachable, it just returned + # a bad status) must NOT be treated as "local is down." Locks in that + # fallback triggers ONLY on requests.RequestException, never on a + # non-2xx/409 response the device actually returned. + local_500 = _response(500) + mock_request.return_value = local_500 + client = _cloud_client() + assert client.draw("app", ELEMENTS) == DrawResult.ERROR + assert mock_request.call_count == 1 # cloud was never attempted + assert mock_request.call_args.args == ("POST", "http://192.0.2.1/api/display/draw") + assert client.active_transport == "local" # never degraded + + +# --- cloud 409 -> REJECTED (final-gate review) ------------------------------- + +@patch("busybar.client.requests.request") +def test_cloud_409_is_rejected_not_error_forced_cloud(mock_request): + mock_request.return_value = _response(409) + client = _cloud_client(transport="cloud") + assert client.draw("app", ELEMENTS) == DrawResult.REJECTED + +@patch("busybar.client.requests.request") +def test_cloud_409_is_rejected_not_error_while_degraded(mock_request): + mock_request.side_effect = [requests.ConnectionError(), _response(200)] + client = _cloud_client() + client.draw("app", ELEMENTS) # degrades to cloud + assert client.active_transport == "cloud" + + mock_request.side_effect = [_response(409)] # degraded -- skips local, straight to cloud + assert client.draw("app", ELEMENTS) == DrawResult.REJECTED + + +# --- transport ValueError guard (final-gate review) -------------------------- + +def test_invalid_transport_value_raises_value_error(): + try: + BusyBarClient(transport="carrier-pigeon") + raise AssertionError("expected ValueError") + except ValueError as exc: + assert "carrier-pigeon" in str(exc) diff --git a/tests/test_config.py b/tests/test_config.py index 3694799..835b961 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1,9 +1,17 @@ +import logging +import tomllib from pathlib import Path -from busybar.config import load_config +from busybar.config import DEFAULTS, device_kwargs, load_config + +REPO_ROOT = Path(__file__).resolve().parents[1] def test_defaults_when_no_file(tmp_path): cfg = load_config(tmp_path / "missing.toml") assert cfg["device"]["host"] == "10.0.4.20" + # v1.6 cloud transport fallback -- disabled out of the box. + assert cfg["device"]["cloud_token"] == "" + assert cfg["device"]["cloud_base_url"] == "https://api.busy.app/busybar" + assert cfg["device"]["transport"] == "auto" assert cfg["calendar_countdown"]["poll_seconds"] == 10 # v1.5 ambient-tier default assert cfg["ci_status"]["repos"] == [] assert cfg["ci_status"]["show_running"] is True @@ -39,3 +47,68 @@ def test_returned_config_mutation_does_not_corrupt_defaults(tmp_path): cfg2 = load_config(tmp_path / "missing.toml") assert cfg2["ci_status"]["repos"] == [] assert cfg2["calendar_countdown"]["poll_seconds"] == 10 + + +# --- config.example.toml parity (v1.6) -------------------------------------- +# config.example.toml's own header claims "All keys optional; defaults +# shown" -- these tests hold that claim to account for the [device] table +# specifically, since it's the one this round touches and the one where a +# drifted example (e.g. a non-empty placeholder token) would be a real +# hygiene problem, not just documentation staleness. + +def test_example_toml_device_section_matches_defaults(): + with open(REPO_ROOT / "config.example.toml", "rb") as fh: + example = tomllib.load(fh) + assert example["device"] == DEFAULTS["device"] + +def test_example_toml_cloud_token_is_empty_placeholder_not_a_real_looking_token(): + with open(REPO_ROOT / "config.example.toml", "rb") as fh: + example = tomllib.load(fh) + # Guards against ever accidentally shipping a real-looking credential + # in the committed example file -- the real token belongs only in the + # git-ignored config.toml. + assert example["device"]["cloud_token"] == "" + + +# --- device_kwargs() unknown-key guard (final-gate review, v1.6) ----------- +# Both integration call sites splat cfg["device"] into BusyBarClient's +# constructor. Splatting the raw dict would TypeError on any unknown/typo'd +# [device] key (a silent-ignore -> crash regression from pre-v1.6, where +# only host= was ever passed explicitly) -- device_kwargs() must filter to +# known kwargs and warn, not crash. + +def test_device_kwargs_passes_through_all_known_keys(): + cfg = {"device": {"host": "192.0.2.1", "cloud_token": "x", + "cloud_base_url": "https://cloud.example.test", + "transport": "local"}} + assert device_kwargs(cfg) == cfg["device"] + +def test_device_kwargs_drops_unknown_key_without_crashing(caplog): + cfg = {"device": {"host": "192.0.2.1", "coud_token": "typo'd-key"}} + caplog.set_level(logging.WARNING, logger="busybar.config") + result = device_kwargs(cfg) + assert result == {"host": "192.0.2.1"} # unknown key silently dropped, not crashed + assert "coud_token" in caplog.text + assert any(record.levelname == "WARNING" for record in caplog.records) + +def test_device_kwargs_result_never_crashes_busybarclient_construction(): + from busybar.client import BusyBarClient + cfg = {"device": {"host": "192.0.2.1", "cloud_token": "x", "bogus_extra_key": 123}} + # This is the actual regression this guard exists for: a typo'd or + # unrecognized [device] key must not raise TypeError when splatted. + client = BusyBarClient(**device_kwargs(cfg)) + assert client.base == "http://192.0.2.1" + +def test_device_kwargs_logs_one_warning_per_unknown_key(caplog): + cfg = {"device": {"host": "192.0.2.1", "bogus_one": 1, "bogus_two": 2}} + caplog.set_level(logging.WARNING, logger="busybar.config") + device_kwargs(cfg) + warnings = [r for r in caplog.records if r.levelname == "WARNING"] + assert len(warnings) == 2 + assert any("bogus_one" in r.getMessage() for r in warnings) + assert any("bogus_two" in r.getMessage() for r in warnings) + +def test_device_kwargs_no_warnings_when_all_keys_known(caplog): + caplog.set_level(logging.WARNING, logger="busybar.config") + device_kwargs({"device": dict(DEFAULTS["device"])}) + assert not [r for r in caplog.records if r.levelname == "WARNING"]