fix(adapters/linear): default a token without expires_in to 30 days

adapters/linear/README.md

@@ -92,7 +92,7 @@ The adapter owns its own rate-limit retry (`RetryPolicy`) and Linear-specific ex
 ### Tokens and tenants
 
 - The adapter refreshes a token when it is empty or within 1 hour of expiry. [linear.go:ensureToken,tokenRefreshBuffer]
-- A token response without a positive `expires_in` gets no expiry, so the adapter never refreshes that token. This holds for the single-install token and for cached per-tenant tokens. [linear.go:exchangeClientCredentials,ensureToken,tenantToken]
+- A token response without a positive `expires_in` gets Linear's documented 30-day lifetime, so the adapter refreshes it 1 hour before 30 days pass. This holds for the single-install token and for cached per-tenant tokens. The adapter does not refresh a token after a 401. [linear.go:exchangeClientCredentials,defaultTokenLifetime,ensureToken,tenantToken]
 - A client-credentials response must contain `access_token` and grant every requested scope. [linear.go:exchangeClientCredentials,verifyGrantedScopes]
 - An install credential must be a `LinearInstall` or a non-nil `*LinearInstall`. A multi-tenant install needs a webhook secret. A pre-supplied `AccessToken` is used as is. [linear.go:linearCredential,resolveWebhookInstall,tenantToken]
 - `GraphQL`, `CreateSessionOnIssue`, and `CreateSessionOnComment` fail in multi-tenant mode. Use the `ForTenant` forms. [session.go:GraphQL,CreateSessionOnIssue,CreateSessionOnComment]

adapters/linear/linear.go

@@ -40,6 +40,7 @@ const (
 	defaultSignatureTolerance = time.Minute
 	maxWebhookBodyBytes       = 1 << 20
 	tokenRefreshBuffer        = time.Hour
+	defaultTokenLifetime      = 30 * 24 * time.Hour
 )
 
 // ClientCredentials configures Linear app-actor authentication.
@@ -638,7 +639,7 @@ func (a *Adapter) verifyTimestamp(timestamp int64) error {
 
 func (a *Adapter) ensureToken(ctx context.Context) error {
 	a.mu.Lock()
-	needsRefresh := a.accessToken == "" || (!a.tokenExpiry.IsZero() && a.now().After(a.tokenExpiry.Add(-tokenRefreshBuffer)))
+	needsRefresh := a.accessToken == "" || a.now().After(a.tokenExpiry.Add(-tokenRefreshBuffer))
 	a.mu.Unlock()
 	if !needsRefresh {
 		return nil
@@ -686,11 +687,13 @@ func (a *Adapter) exchangeClientCredentials(ctx context.Context, creds ClientCre
 	if err := verifyGrantedScopes(scopes, resp.Scope); err != nil {
 		return "", time.Time{}, err
 	}
-	var expiry time.Time
+	// expires_in is optional (RFC 6749 section 5.1), so fall back to Linear's
+	// documented lifetime: https://linear.app/developers/oauth-2-0-authentication
+	lifetime := defaultTokenLifetime
 	if resp.ExpiresIn > 0 {
-		expiry = a.now().Add(time.Duration(resp.ExpiresIn) * time.Second)
+		lifetime = time.Duration(resp.ExpiresIn) * time.Second
 	}
-	return resp.AccessToken, expiry, nil
+	return resp.AccessToken, a.now().Add(lifetime), nil
 }
 
 // tenantToken resolves the access token for one Platform Tenant in multi-tenant
@@ -706,7 +709,7 @@ func (a *Adapter) tenantToken(ctx context.Context, tenant string, cred LinearIns
 	}
 	a.tenantTokensMu.Lock()
 	cached := a.tenantTokens[tenant]
-	if cached != nil && cached.accessToken != "" && (cached.expiry.IsZero() || a.now().Before(cached.expiry.Add(-tokenRefreshBuffer))) {
+	if cached != nil && cached.accessToken != "" && a.now().Before(cached.expiry.Add(-tokenRefreshBuffer)) {
 		token := cached.accessToken
 		a.tenantTokensMu.Unlock()
 		return token, nil

adapters/linear/token_lifetime_test.go

@@ -0,0 +1,115 @@
+package linear_test
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/coder/chat"
+	"github.com/coder/chat/adapters/linear"
+)
+
+type tokenWithoutExpiryServer struct {
+	*httptest.Server
+	mu     sync.Mutex
+	tokens int
+}
+
+func newTokenWithoutExpiryServer(t *testing.T) *tokenWithoutExpiryServer {
+	t.Helper()
+	api := &tokenWithoutExpiryServer{}
+	api.Server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch r.URL.Path {
+		case "/oauth/token":
+			api.mu.Lock()
+			api.tokens++
+			api.mu.Unlock()
+			writeJSON(t, w, map[string]any{"access_token": "token", "scope": "read write app:mentionable app:assignable"})
+		case "/graphql":
+			writeJSON(t, w, map[string]any{"data": map[string]any{}})
+		default:
+			t.Fatalf("unexpected path: %s", r.URL.Path)
+		}
+	}))
+	t.Cleanup(api.Close)
+	return api
+}
+
+func (api *tokenWithoutExpiryServer) tokenRequests() int {
+	api.mu.Lock()
+	defer api.mu.Unlock()
+	return api.tokens
+}
+
+func assertTokenRefreshedAfterDefaultLifetime(t *testing.T, api *tokenWithoutExpiryServer, clock *mutableClock, query func() error) {
+	t.Helper()
+	const lifetime, refreshBuffer = 30 * 24 * time.Hour, time.Hour
+	if err := query(); err != nil {
+		t.Fatalf("first query: %v", err)
+	}
+	if got := api.tokenRequests(); got != 1 {
+		t.Fatalf("token requests after first query = %d, want 1", got)
+	}
+	clock.advance(lifetime - refreshBuffer - time.Minute)
+	if err := query(); err != nil {
+		t.Fatalf("query within lifetime: %v", err)
+	}
+	if got := api.tokenRequests(); got != 1 {
+		t.Fatalf("token requests within lifetime = %d, want 1 (cached)", got)
+	}
+	clock.advance(2 * time.Minute)
+	if err := query(); err != nil {
+		t.Fatalf("query past lifetime: %v", err)
+	}
+	if got := api.tokenRequests(); got != 2 {
+		t.Fatalf("token requests past 30 days minus refresh buffer = %d, want 2 (refreshed)", got)
+	}
+}
+
+func TestSingleInstallRefreshesTokenWithoutExpiresIn(t *testing.T) {
+	t.Parallel()
+	api := newTokenWithoutExpiryServer(t)
+	clock := newMutableClock(time.UnixMilli(1_700_000_000_000))
+	adapter, err := linear.New(context.Background(), linear.Options{
+		WebhookSecret:     "whsec",
+		ClientCredentials: linear.ClientCredentials{ClientID: "client", ClientSecret: "secret"},
+		APIBaseURL:        api.URL,
+		Client:            api.Client(),
+		Now:               clock.now,
+	})
+	if err != nil {
+		t.Fatalf("adapter: %v", err)
+	}
+	assertTokenRefreshedAfterDefaultLifetime(t, api, clock, func() error {
+		var dest map[string]any
+		return adapter.GraphQL(context.Background(), `query Q { viewer { id } }`, nil, &dest)
+	})
+}
+
+func TestMultiTenantRefreshesTokenWithoutExpiresIn(t *testing.T) {
+	t.Parallel()
+	api := newTokenWithoutExpiryServer(t)
+	clock := newMutableClock(time.UnixMilli(1_700_000_000_000))
+	store := newFakeInstallStore()
+	store.set("ORG_A", chat.Install{
+		Tenant:     "ORG_A",
+		BotActorID: "APP_A",
+		Credential: linear.LinearInstall{WebhookSecret: "secretA", ClientCredentials: linear.ClientCredentials{ClientID: "clientA", ClientSecret: "csA"}},
+	})
+	adapter, err := linear.New(context.Background(), linear.Options{
+		InstallStore: store,
+		APIBaseURL:   api.URL,
+		Client:       api.Client(),
+		Now:          clock.now,
+	})
+	if err != nil {
+		t.Fatalf("adapter: %v", err)
+	}
+	assertTokenRefreshedAfterDefaultLifetime(t, api, clock, func() error {
+		var dest map[string]any
+		return adapter.GraphQLForTenant(context.Background(), "ORG_A", `query Q { viewer { id } }`, nil, &dest)
+	})
+}