Fix Google Tasks: token refresh used a cancelled context

main.go builds the Tasks client with a short-lived init context and calls
cancel() the moment construction returns. dbTokenSource captured that context
and reused it for every subsequent token refresh, so the first refresh after
the startup access token aged out — roughly an hour in — failed with
"context canceled", and so did every one after it.

That is the whole reason Google Tasks has not been working: all four task
lists, the same error on every fetch, recovering only for an hour after each
restart. The message pointed at a transport problem, which is why it read as
flaky networking rather than a lifetime bug.

A token refresh is not logically part of whichever request happened to trigger
it, so it now gets its own background context with its own timeout. oauth2's
client is likewise built on context.Background(), since NewClient binds the
context it is given to the returned transport for its lifetime; per-call
contexts still come from the tasks service methods.

Regression test drives a refresh through a stub token endpoint with an
already-cancelled construction context, which fails on the old code with
exactly the production error. Also covers Google omitting refresh_token on a
refresh response, which would otherwise blank the stored one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018AwnALZL6bD4VBtATDVv3B

2 files changed, 135 insertions, 82 deletions

diff --git a/internal/api/google_tasks_oauth.go b/internal/api/google_tasks_oauth.go
index 1f5f8c5..6edba21 100644
--- a/internal/api/google_tasks_oauth.go
+++ b/internal/api/google_tasks_oauth.go
@@ -3,6 +3,7 @@ package api
 import (
 	"context"
 	"fmt"
+	"time"
 
 	"golang.org/x/oauth2"
 	"golang.org/x/oauth2/google"
@@ -45,12 +46,14 @@ func GoogleTasksOAuthConfig(clientID, clientSecret, redirectURL string) *oauth2.
 // call -- cheap enough to just hit the DB each time rather than add a
 // second layer of in-process caching.
 type dbTokenSource struct {
-	ctx    context.Context
 	store  TokenStore
 	source string
 	config *oauth2.Config
 }
 
+// tokenRefreshTimeout bounds a single refresh round-trip to Google.
+const tokenRefreshTimeout = 30 * time.Second
+
 func (d *dbTokenSource) Token() (*oauth2.Token, error) {
 	current, err := d.store.GetOAuthToken(d.source)
 	if err != nil {
@@ -60,7 +63,19 @@ func (d *dbTokenSource) Token() (*oauth2.Token, error) {
 		return nil, fmt.Errorf("google tasks not connected -- visit /settings and connect it")
 	}
 
-	refreshed, err := d.config.TokenSource(d.ctx, current).Token()
+	// Deliberately NOT a context captured at construction time. main.go builds
+	// the client with a short-lived init context and cancels it the moment
+	// construction returns; reusing that here meant the first refresh after the
+	// startup access token aged out — roughly an hour in — failed with
+	// "context canceled", and so did every one after it. Google Tasks was
+	// broken from that point until a restart, which bought another hour.
+	//
+	// A token refresh is not logically part of whichever request happened to
+	// trigger it, so it gets its own background context with its own bound.
+	ctx, cancel := context.WithTimeout(context.Background(), tokenRefreshTimeout)
+	defer cancel()
+
+	refreshed, err := d.config.TokenSource(ctx, current).Token()
 	if err != nil {
 		return nil, fmt.Errorf("refreshing token: %w", err)
 	}
@@ -87,7 +102,11 @@ func (d *dbTokenSource) Token() (*oauth2.Token, error) {
 // succeeds; API calls fail with the dbTokenSource error above until a
 // token exists (see HandleGoogleTasksOAuthCallback).
 func NewGoogleTasksOAuthClient(ctx context.Context, oauthConfig *oauth2.Config, tokenStore TokenStore, tasklistID, timezone string) (*GoogleTasksClient, error) {
-	src := &dbTokenSource{ctx: ctx, store: tokenStore, source: GoogleTasksOAuthSource, config: oauthConfig}
-	httpClient := oauth2.NewClient(ctx, src)
+	src := &dbTokenSource{store: tokenStore, source: GoogleTasksOAuthSource, config: oauthConfig}
+	// context.Background(), not ctx: oauth2.NewClient binds the context it is
+	// given to the returned client's transport for its whole lifetime, and ctx
+	// here is the caller's short-lived init context. The per-call context still
+	// applies — it comes from the tasks service methods themselves.
+	httpClient := oauth2.NewClient(context.Background(), src)
 	return NewGoogleTasksClient(ctx, option.WithHTTPClient(httpClient), tasklistID, timezone)
 }
diff --git a/internal/api/google_tasks_oauth_test.go b/internal/api/google_tasks_oauth_test.go
index b4ea6f3..3503cbb 100644
--- a/internal/api/google_tasks_oauth_test.go
+++ b/internal/api/google_tasks_oauth_test.go
@@ -2,121 +2,155 @@ package api
 
 import (
 	"context"
-	"encoding/json"
+	"errors"
 	"net/http"
 	"net/http/httptest"
+	"strings"
+	"sync"
 	"testing"
 	"time"
 
 	"golang.org/x/oauth2"
 )
 
-// fakeTokenStore is a minimal in-memory TokenStore for tests.
-type fakeTokenStore struct {
-	tokens map[string]*oauth2.Token
-	saved  []*oauth2.Token // records every SaveOAuthToken call, in order
+// memTokenStore is an in-memory TokenStore.
+type memTokenStore struct {
+	mu    sync.Mutex
+	token *oauth2.Token
+	saved int
 }
 
-func newFakeTokenStore() *fakeTokenStore {
-	return &fakeTokenStore{tokens: make(map[string]*oauth2.Token)}
-}
-
-func (f *fakeTokenStore) GetOAuthToken(source string) (*oauth2.Token, error) {
-	return f.tokens[source], nil
+func (m *memTokenStore) GetOAuthToken(string) (*oauth2.Token, error) {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	if m.token == nil {
+		return nil, nil
+	}
+	cp := *m.token
+	return &cp, nil
 }
 
-func (f *fakeTokenStore) SaveOAuthToken(source string, tok *oauth2.Token) error {
-	f.tokens[source] = tok
-	f.saved = append(f.saved, tok)
+func (m *memTokenStore) SaveOAuthToken(_ string, tok *oauth2.Token) error {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	cp := *tok
+	m.token = &cp
+	m.saved++
 	return nil
 }
 
-func TestDbTokenSource_NotConnected_ReturnsError(t *testing.T) {
-	store := newFakeTokenStore()
-	src := &dbTokenSource{ctx: context.Background(), store: store, source: "google_tasks", config: &oauth2.Config{}}
+// tokenServer stands in for Google's token endpoint.
+func tokenServer(t *testing.T, hits *int) *httptest.Server {
+	t.Helper()
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		*hits++
+		w.Header().Set("Content-Type", "application/json")
+		// A refresh response that omits refresh_token, as Google's does.
+		_, _ = w.Write([]byte(`{"access_token":"new-access","token_type":"Bearer","expires_in":3600}`))
+	}))
+	t.Cleanup(srv.Close)
+	return srv
+}
 
-	if _, err := src.Token(); err == nil {
-		t.Error("expected an error when no token is stored, got nil")
+func expiredToken() *oauth2.Token {
+	return &oauth2.Token{
+		AccessToken:  "old-access",
+		RefreshToken: "stored-refresh",
+		TokenType:    "Bearer",
+		Expiry:       time.Now().Add(-time.Hour),
 	}
 }
 
-func TestDbTokenSource_NotExpired_ReturnsWithoutPersisting(t *testing.T) {
-	store := newFakeTokenStore()
-	future := time.Now().Add(time.Hour)
-	store.tokens["google_tasks"] = &oauth2.Token{
-		AccessToken: "still-valid", RefreshToken: "r1", TokenType: "Bearer", Expiry: future,
+// The context handed to the client constructor is short-lived: main.go builds
+// it with a timeout and cancels it as soon as construction returns. Capturing
+// it and reusing it for every later token refresh means the first refresh
+// after the initial access token expires — about an hour after startup — fails
+// with "context canceled", and every one after it, permanently.
+//
+// This is what actually broke Google Tasks in production: four task lists, the
+// same error on every fetch, from the moment the startup token aged out.
+func TestDBTokenSource_SurvivesACancelledConstructionContext(t *testing.T) {
+	hits := 0
+	srv := tokenServer(t, &hits)
+
+	// The construction context is cancelled immediately, exactly as main.go
+	// does. Nothing the token source does later may depend on it.
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel()
+	if ctx.Err() == nil {
+		t.Fatal("test setup: the construction context should already be cancelled")
+	}
+
+	store := &memTokenStore{token: expiredToken()}
+	src := &dbTokenSource{
+		store:  store,
+		source: GoogleTasksOAuthSource,
+		config: &oauth2.Config{
+			ClientID:     "id",
+			ClientSecret: "secret",
+			Endpoint:     oauth2.Endpoint{TokenURL: srv.URL},
+		},
 	}
-	src := &dbTokenSource{ctx: context.Background(), store: store, source: "google_tasks", config: &oauth2.Config{}}
 
 	tok, err := src.Token()
 	if err != nil {
-		t.Fatalf("Token: %v", err)
+		if errors.Is(err, context.Canceled) {
+			t.Fatalf("token refresh used the cancelled construction context: %v", err)
+		}
+		t.Fatalf("Token() failed: %v", err)
 	}
-	if tok.AccessToken != "still-valid" {
-		t.Errorf("AccessToken = %q, want unchanged %q", tok.AccessToken, "still-valid")
+	if tok.AccessToken != "new-access" {
+		t.Errorf("AccessToken = %q, want the refreshed one", tok.AccessToken)
 	}
-	if len(store.saved) != 0 {
-		t.Errorf("SaveOAuthToken called %d times, want 0 (nothing changed, no refresh needed)", len(store.saved))
+	if hits == 0 {
+		t.Error("expected the token endpoint to be called")
 	}
 }
 
-func TestDbTokenSource_Refresh_PreservesOriginalRefreshTokenWhenOmitted(t *testing.T) {
-	// Simulates Google's real behavior: a refresh response often omits
-	// refresh_token entirely. Without the fix, that would overwrite the
-	// stored refresh token with "" and permanently break future refreshes.
-	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		w.Header().Set("Content-Type", "application/json")
-		_ = json.NewEncoder(w).Encode(map[string]interface{}{
-			"access_token": "refreshed-access-token",
-			"token_type":   "Bearer",
-			"expires_in":   3600,
-			// no refresh_token in the response, matching Google's real behavior
-		})
-	}))
-	defer server.Close()
-
-	store := newFakeTokenStore()
-	expired := time.Now().Add(-time.Hour)
-	store.tokens["google_tasks"] = &oauth2.Token{
-		AccessToken: "stale-access-token", RefreshToken: "original-refresh-token", TokenType: "Bearer", Expiry: expired,
+// Google omits refresh_token on a refresh response. Overwriting the stored one
+// with an empty string would permanently break every later refresh.
+func TestDBTokenSource_KeepsRefreshTokenWhenResponseOmitsIt(t *testing.T) {
+	hits := 0
+	srv := tokenServer(t, &hits)
+
+	store := &memTokenStore{token: expiredToken()}
+	src := &dbTokenSource{
+		store:  store,
+		source: GoogleTasksOAuthSource,
+		config: &oauth2.Config{
+			ClientID:     "id",
+			ClientSecret: "secret",
+			Endpoint:     oauth2.Endpoint{TokenURL: srv.URL},
+		},
 	}
 
-	cfg := &oauth2.Config{
-		ClientID:     "test-client",
-		ClientSecret: "test-secret",
-		Endpoint:     oauth2.Endpoint{TokenURL: server.URL},
+	if _, err := src.Token(); err != nil {
+		t.Fatalf("Token(): %v", err)
 	}
-	src := &dbTokenSource{ctx: context.Background(), store: store, source: "google_tasks", config: cfg}
 
-	tok, err := src.Token()
-	if err != nil {
-		t.Fatalf("Token: %v", err)
+	saved, _ := store.GetOAuthToken(GoogleTasksOAuthSource)
+	if saved.RefreshToken != "stored-refresh" {
+		t.Errorf("RefreshToken = %q, want the stored one preserved", saved.RefreshToken)
 	}
-	if tok.AccessToken != "refreshed-access-token" {
-		t.Errorf("AccessToken = %q, want %q", tok.AccessToken, "refreshed-access-token")
-	}
-	if tok.RefreshToken != "original-refresh-token" {
-		t.Errorf("RefreshToken = %q, want original preserved %q", tok.RefreshToken, "original-refresh-token")
-	}
-
-	if len(store.saved) != 1 {
-		t.Fatalf("SaveOAuthToken called %d times, want 1", len(store.saved))
-	}
-	if store.saved[0].RefreshToken != "original-refresh-token" {
-		t.Errorf("persisted RefreshToken = %q, want %q", store.saved[0].RefreshToken, "original-refresh-token")
+	if saved.AccessToken != "new-access" {
+		t.Errorf("AccessToken = %q, want it updated", saved.AccessToken)
 	}
 }
 
-func TestGoogleTasksOAuthConfig_HasTasksScopeAndEndpoint(t *testing.T) {
-	cfg := GoogleTasksOAuthConfig("id", "secret", "https://doot.terst.org/settings/google-tasks/callback")
-
-	if cfg.ClientID != "id" || cfg.ClientSecret != "secret" {
-		t.Errorf("ClientID/Secret = %q/%q, want id/secret", cfg.ClientID, cfg.ClientSecret)
+// Before the user has connected, the error must say so rather than surfacing
+// as an opaque transport failure.
+func TestDBTokenSource_NotConnected(t *testing.T) {
+	src := &dbTokenSource{
+		store:  &memTokenStore{},
+		source: GoogleTasksOAuthSource,
+		config: &oauth2.Config{},
 	}
-	if cfg.RedirectURL != "https://doot.terst.org/settings/google-tasks/callback" {
-		t.Errorf("RedirectURL = %q", cfg.RedirectURL)
+	_, err := src.Token()
+	if err == nil {
+		t.Fatal("expected an error when no token is stored")
 	}
-	if len(cfg.Scopes) != 1 || cfg.Scopes[0] != "https://www.googleapis.com/auth/tasks" {
-		t.Errorf("Scopes = %v, want [tasks scope]", cfg.Scopes)
+	if !strings.Contains(err.Error(), "not connected") {
+		t.Errorf("error = %q, want it to mention not being connected", err)
 	}
 }
+