fix: cache Cloud profiles and make USB connections opt-in - #668
Open
jeroenwienk wants to merge 9 commits into
Open
jeroenwienk wants to merge 9 commits into
jeroenwienk wants to merge 9 commits into
Conversation
Local discovery probed one candidate address per network interface sequentially, each with a one second timeout, so every 10.x interface that is not a Homey added a full second to getHomeys. Virtualization adapters commonly add several of those. Interfaces sharing a subnet also produced a duplicate probe for the same address. Collect the unique candidate addresses first, then probe them concurrently. With four such interfaces, two of them in one subnet, this drops from 4006ms to 1004ms; with eight it drops from 9012ms to 1005ms. The total is now bounded by the timeout instead of scaling with the interface count.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Cross-process cache races can discard cooldowns, restore stale profiles, and cross OAuth account boundaries.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds persistent account-profile caching to keep CLI commands working during Cloud API rate limits.
Changes:
- Adds five-minute profile caching with 429 fallback and cooldown.
- Adds
--refreshtolistandwhoami. - Adds documentation and cache-focused tests.
File summaries
| File | Description |
|---|---|
lib/AthomApi.js |
Integrates profile caching and rate-limit handling. |
lib/AthomApiProfileCache.js |
Implements persistent atomic cache writes. |
bin/cmds/list.mjs |
Adds account-data refresh support. |
bin/cmds/whoami.mjs |
Adds account-data refresh support. |
README.md |
Documents caching and rate-limit behavior. |
tests/lib/athom-api.cache.test.mjs |
Tests cache behavior and isolation. |
tests/lib/athom-api.fetch.test.mjs |
Updates local-discovery mocks. |
Review details
Suppressed comments (1)
lib/AthomApi.js:211
- This is not a safe cross-process update because
storedwas captured before the network request. If another invocation successfully writes a newer profile while this one receives 429, this write restores stale data; in the reverse completion order, the success write removes the newly recorded cooldown. Serialize or atomically merge cache updates so both the newest profile and an activeretryAftersurvive overlapping refreshes.
await this._profileCache.set({
...stored,
retryAfter: Date.now() + PROFILE_RATE_LIMIT_COOLDOWN,
});
- Files reviewed: 5/7 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
jeroenwienk
changed the title
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Logout races can repopulate the profile cache, and disconnected USB diagnostics bypass the report structure.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 20/28 changed files
- Comments generated: 2
- Review effort level: Balanced
Comment on lines
+29
to
+30
| if (stored?.authKey !== value.authKey) { | ||
| return value; |
| const preferredStrategyIds = getPreferredAuthenticateStrategy(homey); | ||
| const attemptedStrategyIds = getDiagnoseStrategyOrder(homey); | ||
| export async function diagnoseHomeyStrategies({ homeyId, usb = false } = {}) { | ||
| const homey = await resolveRequestedHomey(homeyId, { usb }); |
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
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.
Repeated CLI commands could hit Athom Cloud profile rate limits before reaching Homey, and USB discovery added a one-second timeout for unrelated
10.*interfaces. Cache the account profile across invocations and make USB connections explicitly opt-in.Cloud profile caching
Cache the account profile and Homey connection details for five minutes. On HTTP 429, reuse the last successful profile and persist a one-minute refresh cooldown.
homey list --refreshandhomey whoami --refreshrefresh account data while respecting that cooldown.Bind cache entries to a hash of the OAuth access token or PAT captured before the request, clear caches on login/logout, and serialize updates with a process-shared lock. Cache I/O failures warn without discarding fetched profiles or usable rate-limit fallbacks. Actual Homey responses remain live.
Explicit USB connections
Existing USB users must now pass
--usbor setHOMEY_USB=1. Explicit--no-usboverrides the environment setting. Supported commands arelist,select,app run,app install, generated API operations,api rawand its aliases, andapi diagnose.--discovery-strategies, disables alternative discovery strategies, and never falls back to LAN or Cloud transport.--token <TOKEN> --homey-id <ID> --usb. Enabled USB mode rejects--address.api diagnose --usbreports only theusbstrategy using the existing report structure. Account authentication and session renewal can still require Athom Cloud.Merge Robin's
fix/parallel-usb-probebranch with its history, intervening commits, and 4.4.5 bump. Retain concurrent probes of unique addresses with the existing one-second timeout, and adapt the regression test togetProfile(). This branch integration leaves PR #674 open; it does not merge it intodevelop.Also integrate current
developto resolve its connection-strategy conflicts, retaining its 4.5.0 version, Homey API update, and Python find-links fix. Explicit network discovery strategies remain available in normal mode; enabled USB mode remains strict.Validation
The merge baseline passes all 269 tests. Feature tests cover flag/environment precedence, zero default probes, concurrency and deduplication, USB-only selection, offline and unsupported targets, mode isolation, authentication and request routing, token mode, API aliases, diagnostics, and app execution/cleanup. All 311 tests pass with coverage (51.78% lines, 75.75% branches, 68.26% functions), above repository thresholds. ESLint, repository-wide Prettier, and diff checks pass. Network behavior is tested with isolated fixtures and mocked endpoints; physical USB hardware has not been tested.
Asana: https://app.asana.com/1/739269396989735/task/1218190357212920