fix: away-mode side resolution, IANA timezone lookup, keychain fallback#36
Merged
omarshahine merged 4 commits intoApr 18, 2026
Merged
Conversation
Three independent bugs surfaced while testing steipete#35: 1. HouseholdUserTargets relied on `user.currentDevice.side`, which the Eight Sleep API overwrites with the sentinel "away" for every user when the household is in Away mode. `--side left|right` therefore errored with `available sides: away` for any Away-mode household. Resolve side from the `/devices` payload instead: when Away mode blanks the top-level `leftUserId`/`rightUserId`, the real IDs live inside `awaySides` as `{"leftUserId":"…","rightUserId":"…"}`. Also filter the "available sides" error list to drop the Away sentinel. 2. `resolveAPITimezone` fell back to UTC whenever `time.Local.String()` returned "Local". Port the `/etc/localtime` / `/etc/timezone` / `$TZ` IANA resolution from PR steipete#21 (@dtrinh) so `presence`, `sleep day`, `sleep range`, and `metrics` get the real local zone the API expects. 3. Tokencache picked the first backend whose `Open()` succeeded even when every `Set()` then failed with `Keychain Error (-61)` — e.g., macOS sessions without a writable login keychain. `Save`, `Load`, and `Clear` now fall through to a FileBackend-only keyring so the OAuth token survives across invocations instead of re-running the password grant on every command and tripping Eight Sleep's auth rate limiter. Co-Authored-By: Danny Trinh <[email protected]> Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
CI's gofumpt format check caught whitespace drift in the two new test files. No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The fallback path added in the previous commit made `tokencache.Load` reach into `~/.config/eightctl/keyring/` whenever the primary backend returned ErrKeyNotFound. Existing test helpers (`useTempKeyring` in cmd, `withTestKeyring` in tokencache) only overrode `openKeyring`, so tests that exercised the miss path could see the real filesystem and return stale state — this is what caused `TestRequireAuthFieldsFailsWithoutCacheOrCreds` to fail in CI. Both helpers now override `openKeyring` and `openFileKeyring` with the same tmp-backed opener, keeping the fallback entirely in-process. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
- Move the IANA-discovery doc comment onto `defaultLocalIANA` where the behavior lives; leave a one-liner on the swappable var. - Make `unwritableKeyring.Remove` return the same sentinel as `Set` so the fake behaves consistently if a future test exercises it. No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
0xble
added a commit
to 0xble/eightctl
that referenced
this pull request
Apr 20, 2026
Merge upstream/main (steipete#35 side-aware targeting, steipete#36 away-mode fixes and keychain fallback, steipete#37 cache OAuth across household userIDs). Conflict resolution highlights: - tokencache: keep fork's JSON-file fallback and FileBackend-only keyring; drop expectedUserID filter in Load to match upstream household sharing semantic from steipete#37 - eightsleep.go: add doApp wrapper routing to AppURL via doWithBase, keep fork's API drift resilience (fallbackBaseURL, legacyLoginBaseURL, explicit gzip handling), pull in resolveTZ for presence.go - cmd: use resolveAPITimezone (upstream's IANA fallback) instead of fork's time.Local.String() hack - root_test: combine SetOpenKeyringForTest with SetFallbackPathForTest
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.
Summary
Three independent fixes that surfaced while testing #35 against a live Pod 2 Pro in Away mode.
1. Away-mode side resolution (
internal/client/targets.go)HouseholdUserTargetswas reading each user'scurrentDevice.side, but the API overwrites that with the sentinel"away"for every user when the household is in Away mode. That broke--side left|rightfor anyone who'd ever used Away mode:The
/devicespayload has the authoritative assignment, just in a different shape in Away mode — the top-levelleftUserId/rightUserIdcome back blank and the real IDs live insideawaySidesas{"leftUserId":"…","rightUserId":"…"}. Read from there, fall through to thecurrentDevice.sidevalue only when it's one ofleft/right/solo(never the Away sentinel), and filter the "available sides" error message to dropaway.Live verification (household currently in Away mode, two users):
2. IANA timezone lookup (
internal/cmd/timezone.go)resolveAPITimezonepreviously fell back to UTC whenevertime.Local.String()returned"Local"(which is the common case on macOS/Linux when$TZisn't set — Eight Sleep rejects that value). Ports @dtrinh's/etc/localtime//etc/timezone/$TZresolution from #21 sopresence,sleep day,sleep range, andmetricsactually get the real local zone. Closes #21 with credit viaCo-Authored-By.Before:
After:
3. Keychain backend fallback (
internal/tokencache/tokencache.go)defaultOpenKeyringlistedKeychainBackendfirst inAllowedBackends, and the99designs/keyringlibrary picks the first backend whoseOpen()succeeds. On macOS sessions without a writable login keychain (which is more common than you'd expect — e.g., agent-spawned shells, kiosk accounts),Open()succeeds but everySet()fails withKeychain Error (-61), so the FileBackend fallback was never reached. The net effect: every CLI invocation re-ran the password grant, tripping Eight Sleep's auth rate limiter (401/429on/tokens) after a handful of commands.Save/Load/Clearnow try the primary backend, and on write/read failure open aFileBackend-only ring pointed at~/.config/eightctl/keyring/and retry there. The OAuth token then survives between invocations as intended.Live verification on a machine without a usable login keychain:
No re-auth on subsequent invocations, no rate-limit bursts.
Test plan
go test ./...— all pass, new tests cover each fixTestHouseholdUserTargetsUsesDeviceMappingInAwayMode— Away-mode side resolutionTestResolveAPITimezoneUsesLocalIANAWhenValueIsLocalandTestExtractZoneinfoSuffix— IANA lookupTestSaveFallsBackToFileWhenPrimarySetFails— keychain fallback via anunwritableKeyringfakego build ./...,go vet ./...cleanRefs #35 (follow-up), closes #21.