v1.6: cloud transport fallback for BusyBarClient - #14
Merged
Conversation
Adds an optional cloud relay fallback for BusyBarClient, mirroring busylib-py's single-class transport-flag design, so an integration keeps working when the local device (USB/Wi-Fi) becomes unreachable, as long as the operator has configured a BUSY cloud API token. New [device] config: cloud_token = "" (empty disables cloud fallback entirely -- out-of-the-box behavior is byte-for-byte unchanged from pre-v1.6), cloud_base_url (default https://api.busy.app/busybar), transport = "auto" | "local" | "cloud". Key names were chosen to match BusyBarClient's constructor kwargs exactly, so both integration call sites simplify to BusyBarClient(**cfg["device"]). In "auto" mode (the default), _request tries local first with the existing (3, 5) timeout; on a requests.RequestException with a non-empty cloud_token, the same request is retried against cloud_base_url with a (5, 15) timeout and an Authorization: Bearer header. A per-client active_transport attribute tracks which transport last succeeded; while degraded, subsequent calls skip the local attempt entirely and go straight to cloud until LOCAL_RETRY_SECONDS (60) have elapsed since the last local failure, at which point local is retried first again as a recovery probe -- cheap to do inline (no background thread) since a down local device fails fast. DrawResult.UNREACHABLE now means both transports failed when cloud is configured; unchanged (local-only) semantics when it isn't. Local paths ("/api/...") map to cloud paths by stripping the "/api" prefix and letting cloud_base_url (which already carries "/busybar") supply the rest. DrawResult's members, all six public BusyBarClient methods, and both integrations' logic are unchanged -- the fallback lives entirely inside the private _request/_try_local/_try_cloud layer. Per the coordinator's explicit security requirement, cloud_token is never logged at any level including DEBUG -- only transport transitions are logged (INFO, static strings, no interpolation). Verified both by a manual grep of every log call site in client.py and by a dedicated caplog-based regression test. No live cloud verification was performed this round -- the operator hadn't provisioned a real token yet, per explicit instruction. Tests use a literal placeholder token string only; no network call was made to any busy.app host. README's new "Cloud transport" section documents the token creation/rotation walkthrough and the post-merge live-probe checklist (forced-cloud draw probe, cadence headroom check at 10s redraws, and resolving the api.busy.app/busybar vs. busylib-py's proxy.busy.app base-URL discrepancy) for the controller pass that runs once a token exists. 323 -> 337 tests passing (14 net-new: 12 in test_client.py covering fallback/no-fallback/forced-mode/request-shape/recovery-probe-timing/ token-never-logged, 2 in test_config.py for config.example.toml/DEFAULTS parity on the new [device] keys). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ack contracts in tests Final-gate review fixes for the v1.6 cloud transport fallback feature, same branch (dev/claude/cloud-transport-v1.6), addressing two Important and three Minor findings. Important #1: BusyBarClient(**cfg["device"]) crashed with a cryptic TypeError on any unknown/typo'd [device] key -- a regression from pre-v1.6, where only host= was ever passed explicitly and an unrelated key was silently ignored. Worst-timed failure mode: an operator typo'ing cloud_token during first-time setup would hit this. Fixed with a new busybar.config.device_kwargs(cfg) helper that filters cfg["device"] down to BusyBarClient's actual constructor kwargs (introspected via inspect.signature so it can't drift out of sync with the constructor) and logs one WARNING per dropped key, restoring "ignored, not fatal" while adding observability the pre-v1.6 code never had. Both calendar_countdown/main.py and ci_status/main.py now construct BusyBarClient(**device_kwargs(cfg)). Important #2: added the single most important untested behavioral contract of the whole feature -- a client with cloud_token configured, where local returns HTTP 500 (a real response, no exception), must result in DrawResult.ERROR with cloud never attempted. Locks in "fallback triggers strictly on requests.RequestException, never on any HTTP response the device actually returned." Minor #1: fixed an inaccurate comment in test_cloud_token_never_appears_in_log_output (claimed the second call re-attempts local; it actually skips local within the recovery window) and added time.monotonic mocking for determinism instead of relying on real wall-clock time staying under LOCAL_RETRY_SECONDS between two adjacent statements. Minor #2: added cloud-409 -> REJECTED coverage for both forced-cloud and degraded-auto transport, mirroring the existing local-409 test. Minor #3: added a direct test for the transport ValueError guard. 337 -> 346 tests passing (9 net-new: 5 in test_config.py for device_kwargs, 4 in test_client.py for the HTTP-500/cloud-409/ValueError contracts). Report appended to .superpowers/sdd/display-v1.6-report.md (gitignored); spec doc gained a "Final-gate review fixes" subsection under the v1.6 section. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 628e3dded9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 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) |
There was a problem hiding this comment.
When the configured cloud token contains an accidental control character (for example a pasted newline), requests raises an InvalidHeader before sending the request and the exception text includes the rejected Authorization header value. Formatting exc here means DEBUG logs during token setup can write the bearer token to disk, contradicting the new guarantee that cloud_token is never logged; log only a sanitized failure or exception type for cloud failures.
Useful? React with 👍 / 👎.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds an optional cloud transport (BUSY account API token) to BusyBarClient, mirroring busylib-py's single-class transport-flag design:
transport = "auto"(default): local-first with cloud fallback on connection failure; 60s local-recovery probe while degraded; INFO logs on degradation/recovery transitions. Forcedlocal/cloudmodes available.cloud_token(empty disables; per-device scoped, lives only in git-ignored config),cloud_base_url,transport. Config splat guarded by signature-derived key filtering — typos warn (key name only, never values) instead of crashing.Tests: 346.
🤖 Generated with Claude Code