Fix: add label to keyring entry

[carole-lavillonniere]

With this change, macOS keychain users won't see an empty string in the pop-up:

Other changes

• Simplified the keyring API: the generic Keyring interface is replaces with a purpose-specific AuthTokenStorage interface. This removes the need for passing redundant key parameters.
• Also simplified the API of the keyring used in integration tests.

[carole-lavillonniere]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[carole-lavillonniere]

The added Label is the fix.

[coderabbitai]

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

This pull request refactors token storage in the authentication module by replacing direct keyring interactions with a new AuthTokenStorage interface abstraction. The changes include a core interface definition, updates to the auth module to use the new abstraction, integration test wrappers, and corresponding mock implementations.

Changes

Cohort / File(s)	Summary
Core Auth Refactoring internal/auth/token_storage.go, internal/auth/auth.go	Replaced Keyring interface with AuthTokenStorage interface; renamed systemKeyring to authTokenStorage; updated token retrieval to use GetAuthToken(), SetAuthToken(), and DeleteAuthToken() methods; added environment variable fallback for LOCALSTACK_AUTH_TOKEN.
Auth Test Infrastructure internal/auth/auth_test.go, internal/auth/mock_keyring_test.go, internal/auth/mock_token_storage_test.go	Replaced MockKeyring with NewMockAuthTokenStorage mock; removed old keyring mock file; updated test wiring to inject and expect calls on the new token storage interface.
Integration Test Utilities test/integration/main_test.go	Introduced keyring configuration constants (keyringService, keyringAuthTokenKey, keyringPassword, keyringFilename, keyringAuthTokenLabel); refactored keyring functions to public wrappers: GetAuthTokenFromKeyring(), SetAuthTokenInKeyring(), DeleteAuthTokenFromKeyring().
Integration Tests test/integration/login_browser_flow_test.go, test/integration/login_device_flow_test.go, test/integration/logout_test.go, test/integration/start_test.go	Updated test assertions and setup to use new public keyring wrapper functions (GetAuthTokenFromKeyring(), SetAuthTokenInKeyring(), DeleteAuthTokenFromKeyring()) instead of direct keyring access; removed local keyring constants.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

• silv-io

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title references adding a label to a keyring entry, which is a real change, but the PR also includes significant API simplification work (replacing the generic Keyring interface with AuthTokenStorage). The title describes only one aspect of the overall changes.
Description check	✅ Passed	The description is related to the changeset and explains both the main user-facing fix (macOS keychain label) and the other changes (API simplification), accurately reflecting the scope of work in the PR.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into main

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch carole/drg-501

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (4)

internal/auth/auth.go (1)

57-64: Redundant error check—DeleteAuthToken() already handles ErrKeyNotFound.

The DeleteAuthToken() implementation in keyring.go (lines 83-88) already returns nil when the key is not found. This check duplicates that logic and leaks the keyring package into the abstraction layer.

♻️ Suggested simplification

 // Logout removes the stored auth token from the keyring
 func (a *Auth) Logout() error {
-	err := a.tokenStorage.DeleteAuthToken()
-	if errors.Is(err, keyring.ErrKeyNotFound) {
-		return nil
-	}
-	return err
+	return a.tokenStorage.DeleteAuthToken()
 }

This also allows removing the "github.com/99designs/keyring" import from this file.

test/integration/main_test.go (1)

19-25: Constants are intentionally duplicated from production code.

The duplication is noted (line 38-39 mentions similar for configDir), but if these constants diverge from internal/auth/keyring.go, tests may not accurately reflect production behavior.

Consider adding a comment noting these must stay in sync with internal/auth/keyring.go, or alternatively, exporting the constants if test coverage of the exact values is important.

internal/auth/keyring.go (1)

64-73: Custom error message hides the original error type.

Returning fmt.Errorf("credential not found") on line 68 loses the original keyring.ErrKeyNotFound. While this provides abstraction, callers cannot distinguish "key not found" from other errors without string matching.

Consider wrapping with a sentinel error if callers need to handle "not found" differently:

♻️ Optional: Define a sentinel error for better error handling

// At package level
var ErrTokenNotFound = errors.New("credential not found")

// In GetAuthToken
if errors.Is(err, keyring.ErrKeyNotFound) {
    return "", ErrTokenNotFound
}

This allows callers to use errors.Is(err, auth.ErrTokenNotFound) without depending on the keyring package.

internal/auth/mock_keyring_test.go (1)

1-10: Generated mock file – consider renaming to match the interface.

The file is correctly generated, but the filename mock_keyring_test.go no longer reflects the interface name AuthTokenStorage. Consider updating the mockgen command to use -destination=mock_auth_token_storage_test.go for clarity:

-//	mockgen -source=keyring.go -destination=mock_keyring_test.go -package=auth
+//	mockgen -source=keyring.go -destination=mock_auth_token_storage_test.go -package=auth

[silv-io]

LGTM! Got nothing besides the nitpicks that the rabbit already found