From cbce965f34ccca3b67ce5a7c0bc6edb6bfed92b1 Mon Sep 17 00:00:00 2001 From: Omar Shahine <10343873+omarshahine@users.noreply.github.com> Date: Sat, 18 Apr 2026 07:10:48 -0700 Subject: [PATCH 1/4] fix: away-mode side resolution, IANA timezone lookup, keychain fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three independent bugs surfaced while testing #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 #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 Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/client/targets.go | 47 +++++++++++++- internal/client/targets_test.go | 55 ++++++++++++++++ internal/cmd/timezone.go | 57 +++++++++++++++-- internal/cmd/timezone_test.go | 36 ++++++++++- internal/tokencache/tokencache.go | 87 +++++++++++++++++++++----- internal/tokencache/tokencache_test.go | 49 +++++++++++++++ 6 files changed, 307 insertions(+), 24 deletions(-) diff --git a/internal/client/targets.go b/internal/client/targets.go index 1ea46f4..e368094 100644 --- a/internal/client/targets.go +++ b/internal/client/targets.go @@ -55,6 +55,9 @@ func (c *Client) HouseholdUserTargets(ctx context.Context) ([]HouseholdUserTarge if err := c.do(ctx, http.MethodGet, path, query, nil, &deviceRes); err != nil { return nil, err } + + sideByUser := sideAssignmentsFromDevice(deviceRes.Result.LeftUserID, deviceRes.Result.RightUserID, deviceRes.Result.AwaySides) + userIDs := orderedUniqueStrings( deviceRes.Result.LeftUserID, deviceRes.Result.RightUserID, @@ -80,7 +83,7 @@ func (c *Client) HouseholdUserTargets(ctx context.Context) ([]HouseholdUserTarge } targets = append(targets, HouseholdUserTarget{ UserID: userRes.User.UserID, - Side: strings.ToLower(strings.TrimSpace(userRes.User.CurrentDevice.Side)), + Side: resolveTargetSide(sideByUser[userRes.User.UserID], userRes.User.CurrentDevice.Side), FirstName: userRes.User.FirstName, LastName: userRes.User.LastName, Email: userRes.User.Email, @@ -92,6 +95,45 @@ func (c *Client) HouseholdUserTargets(ctx context.Context) ([]HouseholdUserTarge return targets, nil } +// sideAssignmentsFromDevice builds a userID -> side map from the /devices payload. +// In Away mode the top-level leftUserId/rightUserId come back empty and the +// real IDs are stashed inside awaySides as {"leftUserId":"…","rightUserId":"…"}. +func sideAssignmentsFromDevice(leftUserID, rightUserID string, awaySides map[string]string) map[string]string { + if leftUserID == "" { + leftUserID = awaySides["leftUserId"] + } + if rightUserID == "" { + rightUserID = awaySides["rightUserId"] + } + out := map[string]string{} + switch { + case leftUserID != "" && rightUserID != "" && leftUserID == rightUserID: + out[leftUserID] = "solo" + case leftUserID != "" && rightUserID != "": + out[leftUserID] = "left" + out[rightUserID] = "right" + case leftUserID != "" && rightUserID == "": + out[leftUserID] = "solo" + case leftUserID == "" && rightUserID != "": + out[rightUserID] = "solo" + } + return out +} + +// resolveTargetSide prefers the device-level assignment and ignores +// user.currentDevice.side when it is the Away-mode sentinel. +func resolveTargetSide(deviceAssigned, userReported string) string { + if deviceAssigned != "" { + return deviceAssigned + } + candidate := strings.ToLower(strings.TrimSpace(userReported)) + switch candidate { + case "left", "right", "solo": + return candidate + } + return "" +} + // ResolveHouseholdSide resolves a single user target for left/right/solo side-aware commands. func ResolveHouseholdSide(targets []HouseholdUserTarget, side string) (*HouseholdUserTarget, error) { side = strings.ToLower(strings.TrimSpace(side)) @@ -104,7 +146,8 @@ func ResolveHouseholdSide(targets []HouseholdUserTarget, side string) (*Househol matches := []HouseholdUserTarget{} available := []string{} for _, target := range targets { - if target.Side != "" { + switch target.Side { + case "left", "right", "solo": available = appendUniqueString(available, target.Side) } if target.Side == side { diff --git a/internal/client/targets_test.go b/internal/client/targets_test.go index 541efcd..474bd2e 100644 --- a/internal/client/targets_test.go +++ b/internal/client/targets_test.go @@ -106,6 +106,61 @@ func TestHouseholdUserTargets(t *testing.T) { } } +func TestHouseholdUserTargetsUsesDeviceMappingInAwayMode(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/users/me", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"user":{"devices":["dev-1"]}}`)) + }) + mux.HandleFunc("/devices/dev-1", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + // In Away mode the API blanks top-level leftUserId/rightUserId and + // stashes them inside awaySides with the original field names. + w.Write([]byte(`{"result":{"awaySides":{"leftUserId":"left-user","rightUserId":"right-user"}}}`)) + }) + // In Away mode the user payload reports side "away" for everyone. + mux.HandleFunc("/users/left-user", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"user":{"userId":"left-user","firstName":"Igor","lastName":"Left","email":"left@example.com","currentDevice":{"side":"away"}}}`)) + }) + mux.HandleFunc("/users/right-user", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"user":{"userId":"right-user","firstName":"Renata","lastName":"Right","email":"right@example.com","currentDevice":{"side":"away"}}}`)) + }) + + srv := httptest.NewServer(mux) + defer srv.Close() + + c := New("email", "pass", "", "", "") + c.BaseURL = srv.URL + c.token = "t" + c.tokenExp = time.Now().Add(time.Hour) + c.HTTP = srv.Client() + + targets, err := c.HouseholdUserTargets(context.Background()) + if err != nil { + t.Fatalf("HouseholdUserTargets: %v", err) + } + if len(targets) != 2 { + t.Fatalf("len(targets) = %d, want 2", len(targets)) + } + sideByID := map[string]string{} + for _, target := range targets { + sideByID[target.UserID] = target.Side + } + if sideByID["left-user"] != "left" || sideByID["right-user"] != "right" { + t.Fatalf("side map = %+v, want left/right", sideByID) + } + + resolved, err := ResolveHouseholdSide(targets, "left") + if err != nil { + t.Fatalf("ResolveHouseholdSide left: %v", err) + } + if resolved.UserID != "left-user" { + t.Fatalf("left target user = %q, want left-user", resolved.UserID) + } +} + func TestHouseholdUserTargetsInfersSoloWhenOnlyOneUserExists(t *testing.T) { mux := http.NewServeMux() mux.HandleFunc("/users/me", func(w http.ResponseWriter, r *http.Request) { diff --git a/internal/cmd/timezone.go b/internal/cmd/timezone.go index 61a6fdb..fc46438 100644 --- a/internal/cmd/timezone.go +++ b/internal/cmd/timezone.go @@ -1,20 +1,65 @@ package cmd import ( + "os" + "path/filepath" + "runtime" "strings" "time" ) func resolveAPITimezone(value string) (string, error) { tz := strings.TrimSpace(value) - if tz == "" || strings.EqualFold(tz, "local") { - tz = strings.TrimSpace(time.Now().Location().String()) + if tz != "" && !strings.EqualFold(tz, "local") { + return tz, nil } - if tz == "" || strings.EqualFold(tz, "local") { - logger.Warn("system local timezone is not an IANA zone; falling back to UTC for API queries") - return "UTC", nil + if iana := localIANA(); iana != "" { + return iana, nil } - return tz, nil + if loc := strings.TrimSpace(time.Now().Location().String()); loc != "" && !strings.EqualFold(loc, "local") { + return loc, nil + } + logger.Warn("system local timezone is not an IANA zone; falling back to UTC for API queries") + return "UTC", nil +} + +// localIANA discovers the IANA zone name the OS considers local. Go's +// time.Local.String() reports "Local" when TZ is unset, which the Eight Sleep +// API rejects, so we read the platform-specific source of truth. +var localIANA = defaultLocalIANA + +func defaultLocalIANA() string { + if tz := strings.TrimSpace(os.Getenv("TZ")); tz != "" && !strings.EqualFold(tz, "local") { + return tz + } + switch runtime.GOOS { + case "darwin": + if target, err := os.Readlink("/etc/localtime"); err == nil { + if zone := extractZoneinfoSuffix(target); zone != "" { + return zone + } + } + case "linux": + if b, err := os.ReadFile("/etc/timezone"); err == nil { + if zone := strings.TrimSpace(string(b)); zone != "" { + return zone + } + } + if target, err := filepath.EvalSymlinks("/etc/localtime"); err == nil { + if zone := extractZoneinfoSuffix(target); zone != "" { + return zone + } + } + } + return "" +} + +func extractZoneinfoSuffix(path string) string { + const marker = "zoneinfo/" + if idx := strings.Index(path, marker); idx >= 0 { + return path[idx+len(marker):] + } + return "" } func currentDate() string { diff --git a/internal/cmd/timezone_test.go b/internal/cmd/timezone_test.go index dd15ed9..47888fd 100644 --- a/internal/cmd/timezone_test.go +++ b/internal/cmd/timezone_test.go @@ -15,10 +15,28 @@ func TestResolveAPITimezoneExplicit(t *testing.T) { } } +func TestResolveAPITimezoneUsesLocalIANAWhenValueIsLocal(t *testing.T) { + orig := localIANA + localIANA = func() string { return "America/Los_Angeles" } + t.Cleanup(func() { localIANA = orig }) + + got, err := resolveAPITimezone("local") + if err != nil { + t.Fatalf("resolveAPITimezone: %v", err) + } + if got != "America/Los_Angeles" { + t.Fatalf("timezone = %q, want America/Los_Angeles", got) + } +} + func TestResolveAPITimezoneFallsBackToUTCWhenLocalIsUnknown(t *testing.T) { - original := time.Local + origLocal := time.Local time.Local = time.FixedZone("Local", 0) - t.Cleanup(func() { time.Local = original }) + t.Cleanup(func() { time.Local = origLocal }) + + origIANA := localIANA + localIANA = func() string { return "" } + t.Cleanup(func() { localIANA = origIANA }) got, err := resolveAPITimezone("local") if err != nil { @@ -28,3 +46,17 @@ func TestResolveAPITimezoneFallsBackToUTCWhenLocalIsUnknown(t *testing.T) { t.Fatalf("timezone = %q, want UTC", got) } } + +func TestExtractZoneinfoSuffix(t *testing.T) { + cases := map[string]string{ + "/var/db/timezone/zoneinfo/America/New_York": "America/New_York", + "/private/var/db/timezone/tz/2024a.1.0/zoneinfo/Etc/UTC": "Etc/UTC", + "/usr/share/zoneinfo/Europe/Berlin": "Europe/Berlin", + "no-zoneinfo-here": "", + } + for input, want := range cases { + if got := extractZoneinfoSuffix(input); got != want { + t.Errorf("extractZoneinfoSuffix(%q) = %q, want %q", input, got, want) + } + } +} diff --git a/internal/tokencache/tokencache.go b/internal/tokencache/tokencache.go index 801d039..bfbe720 100644 --- a/internal/tokencache/tokencache.go +++ b/internal/tokencache/tokencache.go @@ -34,7 +34,10 @@ type Identity struct { Email string } -var openKeyring = defaultOpenKeyring +var ( + openKeyring = defaultOpenKeyring + openFileKeyring = defaultOpenFileKeyring +) // SetOpenKeyringForTest swaps the keyring opener; it returns a restore func. // Not safe for concurrent tests; intended for isolated test scenarios. @@ -44,6 +47,14 @@ func SetOpenKeyringForTest(fn func() (keyring.Keyring, error)) (restore func()) return func() { openKeyring = prev } } +// SetOpenFileKeyringForTest swaps the file-backed fallback opener. +// Use with SetOpenKeyringForTest to exercise the fallback path in isolation. +func SetOpenFileKeyringForTest(fn func() (keyring.Keyring, error)) (restore func()) { + prev := openFileKeyring + openFileKeyring = fn + return func() { openFileKeyring = prev } +} + func defaultOpenKeyring() (keyring.Keyring, error) { home, _ := os.UserHomeDir() return keyring.Open(keyring.Config{ @@ -59,16 +70,21 @@ func defaultOpenKeyring() (keyring.Keyring, error) { }) } +func defaultOpenFileKeyring() (keyring.Keyring, error) { + home, _ := os.UserHomeDir() + return keyring.Open(keyring.Config{ + ServiceName: serviceName, + AllowedBackends: []keyring.BackendType{keyring.FileBackend}, + FileDir: filepath.Join(home, ".config", "eightctl", "keyring"), + FilePasswordFunc: filePassword, + }) +} + func filePassword(_ string) (string, error) { return serviceName + "-fallback", nil } func Save(id Identity, token string, expiresAt time.Time, userID string) error { - ring, err := openKeyring() - if err != nil { - log.Debug("keyring open failed (save)", "error", err) - return err - } data, err := json.Marshal(CachedToken{ Token: token, ExpiresAt: expiresAt, @@ -77,20 +93,55 @@ func Save(id Identity, token string, expiresAt time.Time, userID string) error { if err != nil { return err } - if err := ring.Set(keyring.Item{ + item := keyring.Item{ Key: storageKey(id), Label: serviceName + " token", Data: data, - }); err != nil { - log.Debug("keyring set failed", "error", err) - return err } - log.Debug("keyring saved token") + + primaryErr := trySetWith(openKeyring, item) + if primaryErr == nil { + log.Debug("keyring saved token") + return nil + } + log.Debug("primary keyring set failed; falling back to file backend", "error", primaryErr) + + if fileErr := trySetWith(openFileKeyring, item); fileErr != nil { + log.Debug("file keyring set failed", "error", fileErr) + return primaryErr + } + log.Debug("keyring saved token to file fallback") return nil } +func trySetWith(opener func() (keyring.Keyring, error), item keyring.Item) error { + ring, err := opener() + if err != nil { + return err + } + return ring.Set(item) +} + func Load(id Identity, expectedUserID string) (*CachedToken, error) { - ring, err := openKeyring() + cached, err := loadFrom(openKeyring, id, expectedUserID) + if err == nil { + return cached, nil + } + if err != keyring.ErrKeyNotFound { + log.Debug("primary keyring load failed", "error", err) + } + fallback, fallbackErr := loadFrom(openFileKeyring, id, expectedUserID) + if fallbackErr == nil { + return fallback, nil + } + if fallbackErr != keyring.ErrKeyNotFound { + log.Debug("file keyring load failed", "error", fallbackErr) + } + return nil, err +} + +func loadFrom(opener func() (keyring.Keyring, error), id Identity, expectedUserID string) (*CachedToken, error) { + ring, err := opener() if err != nil { log.Debug("keyring open failed (load)", "error", err) return nil, err @@ -116,7 +167,6 @@ func Load(id Identity, expectedUserID string) (*CachedToken, error) { } } if err != nil { - log.Debug("keyring get failed", "error", err) return nil, err } var cached CachedToken @@ -134,7 +184,16 @@ func Load(id Identity, expectedUserID string) (*CachedToken, error) { } func Clear(id Identity) error { - ring, err := openKeyring() + primaryErr := clearFrom(openKeyring, id) + fallbackErr := clearFrom(openFileKeyring, id) + if primaryErr != nil && fallbackErr != nil { + return primaryErr + } + return nil +} + +func clearFrom(opener func() (keyring.Keyring, error), id Identity) error { + ring, err := opener() if err != nil { return err } diff --git a/internal/tokencache/tokencache_test.go b/internal/tokencache/tokencache_test.go index 410a066..2ffa923 100644 --- a/internal/tokencache/tokencache_test.go +++ b/internal/tokencache/tokencache_test.go @@ -1,6 +1,7 @@ package tokencache import ( + "errors" "path/filepath" "testing" "time" @@ -190,6 +191,54 @@ func TestLoadWithoutEmailMultipleMatchesFails(t *testing.T) { } } +// unwritableKeyring simulates a backend like the macOS login keychain when the +// current session has no writable keychain: Open and Get succeed, but Set fails. +type unwritableKeyring struct{} + +var errUnwritable = errors.New("keyring: write denied") + +func (unwritableKeyring) Set(keyring.Item) error { return errUnwritable } +func (unwritableKeyring) Get(string) (keyring.Item, error) { + return keyring.Item{}, keyring.ErrKeyNotFound +} +func (unwritableKeyring) GetMetadata(string) (keyring.Metadata, error) { + return keyring.Metadata{}, keyring.ErrKeyNotFound +} +func (unwritableKeyring) Remove(string) error { return keyring.ErrKeyNotFound } +func (unwritableKeyring) Keys() ([]string, error) { return nil, nil } + +func TestSaveFallsBackToFileWhenPrimarySetFails(t *testing.T) { + tmp := t.TempDir() + + restorePrimary := SetOpenKeyringForTest(func() (keyring.Keyring, error) { + return unwritableKeyring{}, nil + }) + t.Cleanup(restorePrimary) + + restoreFile := SetOpenFileKeyringForTest(func() (keyring.Keyring, error) { + return keyring.Open(keyring.Config{ + ServiceName: serviceName + "-test", + AllowedBackends: []keyring.BackendType{keyring.FileBackend}, + FileDir: filepath.Join(tmp, "keyring"), + FilePasswordFunc: func(_ string) (string, error) { return "test-pass", nil }, + }) + }) + t.Cleanup(restoreFile) + + id := Identity{BaseURL: "https://api.example.com", ClientID: "client-1", Email: "u@example.com"} + if err := Save(id, "tok", time.Now().Add(time.Hour), "u1"); err != nil { + t.Fatalf("Save should fall back to file: %v", err) + } + + got, err := Load(id, "u1") + if err != nil { + t.Fatalf("Load from file fallback: %v", err) + } + if got.Token != "tok" { + t.Fatalf("token = %q, want tok", got.Token) + } +} + func TestFilePasswordFunc(t *testing.T) { pw, err := filePassword("ignored") if err != nil { From 572dd864a374e790f992907253da1607fed97870 Mon Sep 17 00:00:00 2001 From: Omar Shahine <10343873+omarshahine@users.noreply.github.com> Date: Sat, 18 Apr 2026 07:12:58 -0700 Subject: [PATCH 2/4] style: apply gofumpt formatting to test files 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) --- internal/cmd/timezone_test.go | 8 ++++---- internal/tokencache/tokencache_test.go | 5 +++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/internal/cmd/timezone_test.go b/internal/cmd/timezone_test.go index 47888fd..3c220cd 100644 --- a/internal/cmd/timezone_test.go +++ b/internal/cmd/timezone_test.go @@ -49,10 +49,10 @@ func TestResolveAPITimezoneFallsBackToUTCWhenLocalIsUnknown(t *testing.T) { func TestExtractZoneinfoSuffix(t *testing.T) { cases := map[string]string{ - "/var/db/timezone/zoneinfo/America/New_York": "America/New_York", - "/private/var/db/timezone/tz/2024a.1.0/zoneinfo/Etc/UTC": "Etc/UTC", - "/usr/share/zoneinfo/Europe/Berlin": "Europe/Berlin", - "no-zoneinfo-here": "", + "/var/db/timezone/zoneinfo/America/New_York": "America/New_York", + "/private/var/db/timezone/tz/2024a.1.0/zoneinfo/Etc/UTC": "Etc/UTC", + "/usr/share/zoneinfo/Europe/Berlin": "Europe/Berlin", + "no-zoneinfo-here": "", } for input, want := range cases { if got := extractZoneinfoSuffix(input); got != want { diff --git a/internal/tokencache/tokencache_test.go b/internal/tokencache/tokencache_test.go index 2ffa923..d02744c 100644 --- a/internal/tokencache/tokencache_test.go +++ b/internal/tokencache/tokencache_test.go @@ -197,14 +197,15 @@ type unwritableKeyring struct{} var errUnwritable = errors.New("keyring: write denied") -func (unwritableKeyring) Set(keyring.Item) error { return errUnwritable } +func (unwritableKeyring) Set(keyring.Item) error { return errUnwritable } func (unwritableKeyring) Get(string) (keyring.Item, error) { return keyring.Item{}, keyring.ErrKeyNotFound } + func (unwritableKeyring) GetMetadata(string) (keyring.Metadata, error) { return keyring.Metadata{}, keyring.ErrKeyNotFound } -func (unwritableKeyring) Remove(string) error { return keyring.ErrKeyNotFound } +func (unwritableKeyring) Remove(string) error { return keyring.ErrKeyNotFound } func (unwritableKeyring) Keys() ([]string, error) { return nil, nil } func TestSaveFallsBackToFileWhenPrimarySetFails(t *testing.T) { From 62c1cec6e5bcafe2db22def3b95cacdef7f43b91 Mon Sep 17 00:00:00 2001 From: Omar Shahine <10343873+omarshahine@users.noreply.github.com> Date: Sat, 18 Apr 2026 07:16:59 -0700 Subject: [PATCH 3/4] test: isolate test helpers from the default file keyring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- internal/cmd/root_test.go | 7 +++++-- internal/tokencache/tokencache_test.go | 12 +++++++++--- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/internal/cmd/root_test.go b/internal/cmd/root_test.go index d0b3cf9..f748f55 100644 --- a/internal/cmd/root_test.go +++ b/internal/cmd/root_test.go @@ -15,15 +15,18 @@ import ( func useTempKeyring(t *testing.T) func() { t.Helper() tmp := t.TempDir() - restore := tokencache.SetOpenKeyringForTest(func() (keyring.Keyring, error) { + opener := func() (keyring.Keyring, error) { return keyring.Open(keyring.Config{ ServiceName: "eightctl-test", AllowedBackends: []keyring.BackendType{keyring.FileBackend}, FileDir: filepath.Join(tmp, "keyring"), FilePasswordFunc: func(_ string) (string, error) { return "test-pass", nil }, }) - }) + } + restore := tokencache.SetOpenKeyringForTest(opener) + restoreFile := tokencache.SetOpenFileKeyringForTest(opener) t.Cleanup(restore) + t.Cleanup(restoreFile) return restore } diff --git a/internal/tokencache/tokencache_test.go b/internal/tokencache/tokencache_test.go index d02744c..8560182 100644 --- a/internal/tokencache/tokencache_test.go +++ b/internal/tokencache/tokencache_test.go @@ -12,8 +12,7 @@ import ( func withTestKeyring(t *testing.T) { t.Helper() tmpDir := t.TempDir() - orig := openKeyring - openKeyring = func() (keyring.Keyring, error) { + opener := func() (keyring.Keyring, error) { return keyring.Open(keyring.Config{ ServiceName: serviceName + "-test", AllowedBackends: []keyring.BackendType{keyring.FileBackend}, @@ -21,7 +20,14 @@ func withTestKeyring(t *testing.T) { FilePasswordFunc: func(_ string) (string, error) { return "test-pass", nil }, }) } - t.Cleanup(func() { openKeyring = orig }) + origKeyring := openKeyring + origFile := openFileKeyring + openKeyring = opener + openFileKeyring = opener + t.Cleanup(func() { + openKeyring = origKeyring + openFileKeyring = origFile + }) } func TestSaveLoadRoundTrip(t *testing.T) { From fcb4b81d1b10a61ce62281199e5769cc3b0d57ab Mon Sep 17 00:00:00 2001 From: Omar Shahine <10343873+omarshahine@users.noreply.github.com> Date: Sat, 18 Apr 2026 07:31:01 -0700 Subject: [PATCH 4/4] style: address self-review nits - 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) --- internal/cmd/timezone.go | 7 ++++--- internal/tokencache/tokencache_test.go | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/internal/cmd/timezone.go b/internal/cmd/timezone.go index fc46438..c67c5cd 100644 --- a/internal/cmd/timezone.go +++ b/internal/cmd/timezone.go @@ -23,11 +23,12 @@ func resolveAPITimezone(value string) (string, error) { return "UTC", nil } -// localIANA discovers the IANA zone name the OS considers local. Go's -// time.Local.String() reports "Local" when TZ is unset, which the Eight Sleep -// API rejects, so we read the platform-specific source of truth. +// localIANA is overridable in tests. var localIANA = defaultLocalIANA +// defaultLocalIANA discovers the IANA zone name the OS considers local. Go's +// time.Local.String() reports "Local" when TZ is unset, which the Eight Sleep +// API rejects, so we read the platform-specific source of truth. func defaultLocalIANA() string { if tz := strings.TrimSpace(os.Getenv("TZ")); tz != "" && !strings.EqualFold(tz, "local") { return tz diff --git a/internal/tokencache/tokencache_test.go b/internal/tokencache/tokencache_test.go index 8560182..36b0d6b 100644 --- a/internal/tokencache/tokencache_test.go +++ b/internal/tokencache/tokencache_test.go @@ -211,7 +211,7 @@ func (unwritableKeyring) Get(string) (keyring.Item, error) { func (unwritableKeyring) GetMetadata(string) (keyring.Metadata, error) { return keyring.Metadata{}, keyring.ErrKeyNotFound } -func (unwritableKeyring) Remove(string) error { return keyring.ErrKeyNotFound } +func (unwritableKeyring) Remove(string) error { return errUnwritable } func (unwritableKeyring) Keys() ([]string, error) { return nil, nil } func TestSaveFallsBackToFileWhenPrimarySetFails(t *testing.T) {