Skip to content

feat: upgrade sdk-go to v0.2.0 (credstore generic API)#20

Merged
appleboy merged 2 commits intomainfrom
feat/upgrade-sdk-go-v0.2.0
Mar 11, 2026
Merged

feat: upgrade sdk-go to v0.2.0 (credstore generic API)#20
appleboy merged 2 commits intomainfrom
feat/upgrade-sdk-go-v0.2.0

Conversation

@appleboy
Copy link
Member

@appleboy appleboy commented Mar 10, 2026

Summary

  • Upgrade github.com/go-authgate/sdk-go from v0.1.0 to v0.2.0
  • Migrate all usages from the tokenstore package to the new credstore package
  • Adapt to the new generic Store[T any] interface and updated method signatures

Key API Changes

v0.1.0 (tokenstore) v0.2.0 (credstore)
Package tokenstore credstore
Store type Store Store[T any]
Save Save(*Token) error Save(clientID string, Token) error
Load (*Token, error) (Token, error) (value type)
FileStore NewFileStore(path) NewTokenFileStore(path)
KeyringStore NewKeyringStore(svc) NewTokenKeyringStore(svc)
SecureStore NewSecureStore(kr, file) DefaultTokenSecureStore(svc, path)

Go version

Bumped from 1.24.21.25.0 — required by sdk-go v0.2.0 (pulled in automatically by go mod tidy).

Test plan

  • go build ./... passes with no errors
  • go test ./... — all 3 packages pass (cli, tui, tui/components)
  • Token save/load/concurrent-write tests updated to new API
  • Added unit tests for tui_adapter.go (was 0% coverage)
  • Flow display in token_view.go guarded with if storage.Flow != ""

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings March 10, 2026 08:13
@codecov-commenter
Copy link

codecov-commenter commented Mar 10, 2026

Codecov Report

❌ Patch coverage is 26.31579% with 28 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
tui/token_view.go 0.00% 10 Missing ⚠️
main.go 22.22% 6 Missing and 1 partial ⚠️
config.go 44.44% 5 Missing ⚠️
browser_flow.go 0.00% 4 Missing ⚠️
device_flow.go 0.00% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR upgrades the github.com/go-authgate/sdk-go dependency from v0.1.0 to v0.2.0, migrating from the tokenstore package to the new generic credstore package. The migration introduces support for multiple storage backends (file, OS keyring, and auto-detect) and replaces the custom file-based token persistence (including hand-rolled file locking) with the SDK's built-in implementations.

Changes:

  • Replaced main.TokenStorage type and custom loadTokens/saveTokens/filelock logic with credstore.Store[credstore.Token] interface and SDK-provided implementations (NewTokenFileStore, NewTokenKeyringStore, DefaultTokenSecureStore)
  • Added --token-store flag and TOKEN_STORE env var to select between file, keyring, and auto storage backends
  • Adapted the tui_adapter.go to handle the Flow field separately (as a display-only concept) since credstore.Token no longer includes it

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
go.mod Bumped Go to 1.25.0, added sdk-go v0.2.0 and new transitive deps (keyring, dbus, shellescape)
go.sum Updated checksums for new and upgraded dependencies
config.go Added tokenStore global, --token-store flag, and store initialization logic with file/keyring/auto modes
main.go Replaced loadTokens/saveTokens calls with tokenStore.Load/tokenStore.Save, added credstore.ErrNotFound handling, adapted authenticate to return flow separately
browser_flow.go Updated type references from TokenStorage to credstore.Token, use tokenStore.Save
device_flow.go Updated type references from TokenStorage to credstore.Token, use tokenStore.Save
callback.go Updated callbackResult.Storage and startCallbackServer types to credstore.Token
tui_adapter.go Adapted converters for new credstore.Token type, added flowFromTUI helper, Flow passed as separate parameter
tokens.go Removed TokenStorage, TokenStorageMap, loadTokens, saveTokens (now in SDK)
filelock.go Removed entirely (file locking now handled by SDK)
filelock_test.go Removed tests for deleted file lock code
main_test.go Updated tests to use credstore.Token and tokenStore.Save/tokenStore.Load
callback_test.go Updated type references from TokenStorage to credstore.Token

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


// Display token info.
ui.ShowTokenInfo(toTUITokenStorage(storage))
ui.ShowTokenInfo(toTUITokenStorage(storage, flow))
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When tokens are loaded from the store (existing valid or refreshed tokens), flow remains its zero value "" because credstore.Token no longer has a Flow field. This means toTUITokenStorage(storage, flow) on line 81 will pass an empty flow, and tui/token_view.go:224-232 will unconditionally render "Flow:" with a blank value. Previously, Flow was persisted in the TokenStorage JSON. Consider either: (1) storing the flow as separate metadata alongside the token, or (2) making the Flow display conditional in token_view.go (like simple_manager.go:231 already does with its if storage.Flow != "" guard).

Copilot uses AI. Check for mistakes.
config.go Outdated

// Initialize token store based on mode
tokenStoreMode = getConfig(*flagTokenStore, "TOKEN_STORE", "auto")
fileStore := credstore.NewTokenFileStore(tokenFile)
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fileStore is allocated unconditionally on this line, but it's only used in the "file" case. For the "keyring" and "auto" cases it's wasted. Consider moving credstore.NewTokenFileStore(tokenFile) inside the case "file": branch to avoid the unnecessary allocation.

Copilot uses AI. Check for mistakes.
module github.com/go-authgate/cli

go 1.24.2
go 1.25.0
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The minimum Go version was bumped from 1.24.2 to 1.25.0, but this change isn't mentioned in the PR description. If this is required by the sdk-go v0.2.0 dependency, it would be helpful to note that in the summary. If not, consider whether this bump is intentional, as it may affect CI pipelines and users who haven't upgraded to Go 1.25 yet.

Copilot uses AI. Check for mistakes.
- Migrate all authentication and storage logic from tokenstore to the new credstore API across browser, device, callback, TUI, and main flows
- Update token persistence to require an explicit client ID when saving credentials, enabling multi-client support
- Simplify secure store initialization by using the default credstore secure store helper
- Adjust callback and authentication function signatures to return credstore tokens instead of tokenstore tokens
- Update tests to reflect the new credential store API, including concurrent and multi-client save behavior
- Bump the go-authgate SDK dependency to version 0.2.0

Signed-off-by: Bo-Yi Wu <appleboy.tw@gmail.com>
@appleboy appleboy force-pushed the feat/upgrade-sdk-go-v0.2.0 branch from 84cd1c5 to 7d743f7 Compare March 10, 2026 08:41
- tui/token_view.go: guard Flow display with if != "" to avoid blank
  line when reusing stored tokens (no flow metadata available)
- config.go: move NewTokenFileStore allocation inside case "file" to
  avoid unnecessary allocation for keyring/auto modes
- tui_adapter_test.go: add unit tests for toTUITokenStorage,
  fromTUITokenStorage, and flowFromTUI (was 0% coverage)

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@appleboy appleboy force-pushed the feat/upgrade-sdk-go-v0.2.0 branch from 7d743f7 to e931ad0 Compare March 10, 2026 08:44
@appleboy appleboy merged commit 0c6430f into main Mar 11, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants