Skip to content

Commit c1dfe45

Browse files
wesmclaude
andcommitted
Add test coverage for token reauth and fix messaging accuracy
Extract tokenReauthorizer interface from getTokenSourceWithReauth to enable testing without real OAuth. Move the isatty check from inside the function to each call site via an interactive parameter. Add table-driven tests for isAuthInvalidError (8 cases) and getTokenSourceWithReauth (9 cases including mock side-effect assertions for DeleteToken and Authorize calls). Wire delete-staged through getTokenSourceWithReauth instead of direct TokenSource, matching the pattern used by sync/verify commands. Update error messages and help text to reflect that auto-reauth handles most token issues automatically: remove --force hint from add-account output, reframe --force description, and update serve.go daemon error to suggest running sync/verify from an interactive terminal. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent d4cac61 commit c1dfe45

8 files changed

Lines changed: 354 additions & 22 deletions

File tree

cmd/msgvault/cmd/addaccount.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ By default, opens a browser for authorization. Use --headless to see instruction
2323
for authorizing on headless servers (Google does not support Gmail in device flow).
2424
2525
If a token already exists, the command skips authorization. Use --force to delete
26-
the existing token and re-authorize (useful when a token has expired or been revoked).
26+
the existing token and start a fresh OAuth flow (most token issues are handled
27+
automatically during sync and verify).
2728
2829
Examples:
2930
msgvault add-account you@gmail.com
@@ -95,7 +96,6 @@ Examples:
9596
}
9697
fmt.Printf("Account %s is already authorized.\n", email)
9798
fmt.Println("Next step: msgvault sync-full", email)
98-
fmt.Println("To re-authorize (e.g., expired token), run: msgvault add-account", email, "--force")
9999
return nil
100100
}
101101

@@ -128,7 +128,7 @@ Examples:
128128

129129
func init() {
130130
addAccountCmd.Flags().BoolVar(&headless, "headless", false, "Show instructions for headless server setup")
131-
addAccountCmd.Flags().BoolVar(&forceReauth, "force", false, "Delete existing token and re-authorize (use when token is expired or revoked)")
131+
addAccountCmd.Flags().BoolVar(&forceReauth, "force", false, "Delete existing token and re-authorize")
132132
addAccountCmd.Flags().StringVar(&accountDisplayName, "display-name", "", "Display name for the account (e.g., \"Work\", \"Personal\")")
133133
rootCmd.AddCommand(addAccountCmd)
134134
}

cmd/msgvault/cmd/deletions.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"syscall"
1212
"time"
1313

14+
"github.com/mattn/go-isatty"
1415
"github.com/spf13/cobra"
1516
"github.com/wesm/msgvault/internal/deletion"
1617
"github.com/wesm/msgvault/internal/gmail"
@@ -410,9 +411,11 @@ Examples:
410411
// If no scope metadata at all (legacy token), fall through to reactive detection
411412
}
412413

413-
tokenSource, err := oauthMgr.TokenSource(ctx, account)
414+
interactive := isatty.IsTerminal(os.Stdin.Fd()) ||
415+
isatty.IsCygwinTerminal(os.Stdin.Fd())
416+
tokenSource, err := getTokenSourceWithReauth(ctx, oauthMgr, account, interactive)
414417
if err != nil {
415-
return fmt.Errorf("get token source: %w", err)
418+
return err
416419
}
417420

418421
// Create Gmail client

cmd/msgvault/cmd/root.go

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,8 @@ import (
88
"os"
99
"path/filepath"
1010

11-
"github.com/mattn/go-isatty"
1211
"github.com/spf13/cobra"
1312
"github.com/wesm/msgvault/internal/config"
14-
"github.com/wesm/msgvault/internal/oauth"
1513
"golang.org/x/oauth2"
1614
)
1715

@@ -158,22 +156,35 @@ func isAuthInvalidError(err error) bool {
158156
return false
159157
}
160158

159+
// tokenReauthorizer abstracts the oauth.Manager methods used by
160+
// getTokenSourceWithReauth, making the function testable without real OAuth.
161+
type tokenReauthorizer interface {
162+
TokenSource(ctx context.Context, email string) (oauth2.TokenSource, error)
163+
HasToken(email string) bool
164+
DeleteToken(email string) error
165+
Authorize(ctx context.Context, email string) error
166+
}
167+
161168
// getTokenSourceWithReauth tries to get a token source for the given email.
162169
// If the token exists but is expired/revoked (invalid_grant), it automatically
163170
// deletes the old token and re-initiates the OAuth browser flow.
164171
// Transient errors (network, context cancellation) are returned as-is without
165172
// deleting the token.
166-
// NOTE: This function requires a browser and an interactive terminal for
167-
// re-authorization. Only use from interactive CLI commands (sync, sync-full,
168-
// verify), not from daemon mode (serve).
169-
func getTokenSourceWithReauth(ctx context.Context, oauthMgr *oauth.Manager, email string) (oauth2.TokenSource, error) {
170-
tokenSource, err := oauthMgr.TokenSource(ctx, email)
173+
// The interactive parameter controls whether the function can open a browser
174+
// for re-authorization. Callers should pass the result of an isatty check.
175+
func getTokenSourceWithReauth(
176+
ctx context.Context,
177+
mgr tokenReauthorizer,
178+
email string,
179+
interactive bool,
180+
) (oauth2.TokenSource, error) {
181+
tokenSource, err := mgr.TokenSource(ctx, email)
171182
if err == nil {
172183
return tokenSource, nil
173184
}
174185

175186
// No token at all — user needs to run add-account
176-
if !oauthMgr.HasToken(email) {
187+
if !mgr.HasToken(email) {
177188
return nil, fmt.Errorf("get token source: %w (run 'add-account %s' first)", err, email)
178189
}
179190

@@ -183,22 +194,27 @@ func getTokenSourceWithReauth(ctx context.Context, oauthMgr *oauth.Manager, emai
183194
}
184195

185196
// Non-interactive session cannot open a browser for reauth
186-
if !isatty.IsTerminal(os.Stdin.Fd()) && !isatty.IsCygwinTerminal(os.Stdin.Fd()) {
187-
return nil, fmt.Errorf("token for %s is expired or revoked, but cannot re-authorize in a non-interactive session (run 'add-account --force %s' from a terminal)", email, email)
197+
if !interactive {
198+
return nil, fmt.Errorf(
199+
"token for %s is expired or revoked, but cannot re-authorize "+
200+
"in a non-interactive session (run this command from an "+
201+
"interactive terminal to re-authorize automatically)",
202+
email,
203+
)
188204
}
189205

190206
fmt.Printf("Token for %s is expired or revoked. Re-authorizing...\n", email)
191207

192-
if delErr := oauthMgr.DeleteToken(email); delErr != nil {
208+
if delErr := mgr.DeleteToken(email); delErr != nil {
193209
return nil, fmt.Errorf("delete expired token: %w", delErr)
194210
}
195211

196-
if authErr := oauthMgr.Authorize(ctx, email); authErr != nil {
212+
if authErr := mgr.Authorize(ctx, email); authErr != nil {
197213
return nil, fmt.Errorf("re-authorize %s: %w", email, authErr)
198214
}
199215

200216
// Retry with the new token
201-
tokenSource, err = oauthMgr.TokenSource(ctx, email)
217+
tokenSource, err = mgr.TokenSource(ctx, email)
202218
if err != nil {
203219
return nil, fmt.Errorf("get token source after re-authorization: %w", err)
204220
}

0 commit comments

Comments
 (0)