fix(mp-client): honor expires_in instead of capping every token at 5 minutes

.claude/docs/TestCoverage.md

@@ -258,12 +258,23 @@ remaining callers are one per page load and one per `contact-logs.tsx` mount; ca
 process-wide singleton would hide a newly added contact log type until restart, for a single-digit
 request saving.
 
-### 5.7 `client.ts` token lifetime ignores `expires_in` 🟡
-
-→ `.claude/TODO/mp-client-token-lifetime-ignores-expires-in.md`
-
-The comment says "refresh 5 minutes *before* actual expiration"; the code caps every token at 5
-minutes total, discarding `expires_in`. Roughly 12× more token requests than necessary.
+### 5.7 `client.ts` token lifetime ignores `expires_in` ✅ FIXED
+
+Was: the comment said "refresh 5 minutes *before* actual expiration"; the code set
+`expiresAt = now + 5min` for every token, discarding the `expires_in` the OAuth endpoint returned.
+Since `MinistryPlatformProvider` is a singleton and `ensureValidToken()` runs before every service
+call, a 1-hour MP token was thrown away after 5 minutes — roughly 12× more token requests than
+necessary. Two tests in `client.test.ts` pinned the wrong behavior by advancing timers past the
+5-minute mark and asserting a refresh, so the cap looked deliberate.
+
+Now: the lifetime comes from `expires_in`, minus a `TOKEN_SAFETY_MARGIN` of 5 minutes, floored at 30
+seconds so a pathologically short or negative value cannot drive a refresh storm. A missing or
+non-numeric `expires_in` falls back to `DEFAULT_TOKEN_LIFETIME_SECONDS` (3600).
+`getClientCredentialsToken()` now declares a `ClientCredentialsToken` return type instead of leaking
+`any` out of `response.json()`. The two misleading tests were rewritten against the real boundary and
+four cases added (3600s → 55min, `expires_in` absent, `expires_in: 60` → 30s floor, non-numeric
+→ default). Verified by mutation: restoring the flat 5-minute cap fails all four, and dropping just
+the `Math.max` floor fails the clamp test.
 
 ### 5.8 Resolved: `auth.test.ts` asserted against a copy of the logic ✅
 

src/lib/providers/ministry-platform/auth/client-credentials.ts

@@ -1,4 +1,18 @@
-export async function getClientCredentialsToken() {
+/**
+ * Shape of the OAuth2 client-credentials token response returned by the
+ * Ministry Platform `/oauth/connect/token` endpoint.
+ *
+ * `expires_in` is the token's lifetime in seconds. It is optional here because
+ * the endpoint's response is untrusted at the type level — callers must handle
+ * it being absent or non-numeric.
+ */
+export interface ClientCredentialsToken {
+  access_token: string;
+  token_type: string;
+  expires_in?: number;
+}
+
+export async function getClientCredentialsToken(): Promise<ClientCredentialsToken> {
   const mpBaseUrl = process.env.MINISTRY_PLATFORM_BASE_URL!;
   const mpOauthUrl = `${mpBaseUrl}/oauth`;
 
@@ -21,5 +35,5 @@ export async function getClientCredentialsToken() {
     throw new Error(`Failed to get client credentials token: ${response.statusText}`);
   }
 
-  return await response.json();
+  return (await response.json()) as ClientCredentialsToken;
 }

src/lib/providers/ministry-platform/client.test.ts

@@ -72,7 +72,7 @@ describe('MinistryPlatformClient', () => {
       await client.ensureValidToken();
       expect(mockGetClientCredentialsToken).toHaveBeenCalledTimes(1);
 
-      // Advance time by 1 minute (still within 5-minute validity window)
+      // Advance time by 1 minute (well inside the token's validity window)
       vi.advanceTimersByTime(60 * 1000);
 
       // Second call - should NOT fetch new token
@@ -99,8 +99,8 @@ describe('MinistryPlatformClient', () => {
       await client.ensureValidToken();
       expect(mockGetClientCredentialsToken).toHaveBeenCalledTimes(1);
 
-      // Advance time by 6 minutes (past the 5-minute token life)
-      vi.advanceTimersByTime(6 * 60 * 1000);
+      // Advance time past the token's usable life (3600s - 5min safety margin)
+      vi.advanceTimersByTime(56 * 60 * 1000);
 
       // Second call - should fetch new token
       await client.ensureValidToken();
@@ -148,7 +148,8 @@ describe('MinistryPlatformClient', () => {
   });
 
   describe('Token Lifecycle', () => {
-    it('should use 5-minute safety buffer for token expiration', async () => {
+    it('should refresh 5 minutes before the reported expiration', async () => {
+      // expires_in: 3600 minus the 5-minute safety margin => 55 minutes usable
       mockGetClientCredentialsToken
         .mockResolvedValueOnce({
           access_token: 'token-1',
@@ -167,20 +168,94 @@ describe('MinistryPlatformClient', () => {
       await client.ensureValidToken();
       expect(mockGetClientCredentialsToken).toHaveBeenCalledTimes(1);
 
-      // Advance time by 4 minutes 59 seconds (just under 5-minute buffer)
-      vi.advanceTimersByTime(4 * 60 * 1000 + 59 * 1000);
+      // Advance to 54:59 - just inside the 55-minute window
+      vi.advanceTimersByTime(54 * 60 * 1000 + 59 * 1000);
 
       // Should still be valid
       await client.ensureValidToken();
       expect(mockGetClientCredentialsToken).toHaveBeenCalledTimes(1);
 
-      // Advance time by 2 more seconds (past 5-minute buffer)
+      // Advance 2 more seconds, past 55:00
       vi.advanceTimersByTime(2000);
 
       // Should refresh now
       await client.ensureValidToken();
       expect(mockGetClientCredentialsToken).toHaveBeenCalledTimes(2);
     });
+
+    it('should fall back to a 1-hour lifetime when expires_in is missing', async () => {
+      mockGetClientCredentialsToken
+        .mockResolvedValueOnce({
+          access_token: 'no-expiry-token',
+          token_type: 'Bearer',
+        })
+        .mockResolvedValueOnce({
+          access_token: 'refreshed-token',
+          token_type: 'Bearer',
+        });
+
+      const client = new MinistryPlatformClient();
+
+      await client.ensureValidToken();
+      expect(mockGetClientCredentialsToken).toHaveBeenCalledTimes(1);
+
+      // Same 55-minute boundary as an explicit expires_in: 3600
+      vi.advanceTimersByTime(54 * 60 * 1000 + 59 * 1000);
+      await client.ensureValidToken();
+      expect(mockGetClientCredentialsToken).toHaveBeenCalledTimes(1);
+
+      vi.advanceTimersByTime(2000);
+      await client.ensureValidToken();
+      expect(mockGetClientCredentialsToken).toHaveBeenCalledTimes(2);
+    });
+
+    it('should clamp a short expires_in to the 30-second floor', async () => {
+      // 60s - 5min margin is negative, so the floor applies instead
+      mockGetClientCredentialsToken
+        .mockResolvedValueOnce({
+          access_token: 'short-lived-token',
+          expires_in: 60,
+          token_type: 'Bearer',
+        })
+        .mockResolvedValueOnce({
+          access_token: 'refreshed-token',
+          expires_in: 60,
+          token_type: 'Bearer',
+        });
+
+      const client = new MinistryPlatformClient();
+
+      await client.ensureValidToken();
+      expect(mockGetClientCredentialsToken).toHaveBeenCalledTimes(1);
+
+      // Just inside the 30-second floor
+      vi.advanceTimersByTime(29 * 1000);
+      await client.ensureValidToken();
+      expect(mockGetClientCredentialsToken).toHaveBeenCalledTimes(1);
+
+      // Just past it
+      vi.advanceTimersByTime(2000);
+      await client.ensureValidToken();
+      expect(mockGetClientCredentialsToken).toHaveBeenCalledTimes(2);
+    });
+
+    it('should ignore a non-numeric expires_in and use the default lifetime', async () => {
+      mockGetClientCredentialsToken.mockResolvedValueOnce({
+        access_token: 'bogus-expiry-token',
+        expires_in: 'not-a-number',
+        token_type: 'Bearer',
+      });
+
+      const client = new MinistryPlatformClient();
+
+      await client.ensureValidToken();
+      expect(mockGetClientCredentialsToken).toHaveBeenCalledTimes(1);
+
+      // Would have refreshed immediately if NaN had reached expiresAt
+      vi.advanceTimersByTime(54 * 60 * 1000);
+      await client.ensureValidToken();
+      expect(mockGetClientCredentialsToken).toHaveBeenCalledTimes(1);
+    });
   });
 
   describe('HTTP Client', () => {

src/lib/providers/ministry-platform/client.ts

@@ -1,8 +1,17 @@
 import { getClientCredentialsToken } from "./auth/client-credentials";
 import { HttpClient } from "./utils/http-client";
 
-// Token refresh interval - refresh 5 minutes before actual expiration for safety
-const TOKEN_LIFE = 5 * 60 * 1000; // 5 minutes
+// Refresh this far ahead of the token's real expiration, so a request that is
+// already in flight never races the expiry boundary.
+const TOKEN_SAFETY_MARGIN = 5 * 60 * 1000; // 5 minutes
+
+// Used when the token response omits expires_in. MP client-credentials tokens
+// are issued with a 1 hour lifetime.
+const DEFAULT_TOKEN_LIFETIME_SECONDS = 3600;
+
+// Floor on usable token life, so a pathologically short expires_in cannot drive
+// a refresh storm (or, after subtracting the margin, go negative).
+const MIN_TOKEN_LIFETIME = 30 * 1000; // 30 seconds
 
 /**
  * MinistryPlatformClient - Core HTTP client with automatic authentication management
@@ -48,8 +57,16 @@ export class MinistryPlatformClient {
                 const creds = await getClientCredentialsToken();
                 this.token = creds.access_token;
 
-                // Set expiration time with safety buffer (TOKEN_LIFE before actual expiration)
-                this.expiresAt = new Date(Date.now() + TOKEN_LIFE);
+                // Expire the token TOKEN_SAFETY_MARGIN before the lifetime the
+                // server reported, never sooner than MIN_TOKEN_LIFETIME from now.
+                const seconds = Number(creds.expires_in);
+                const lifetimeMs =
+                    (Number.isFinite(seconds) && seconds > 0
+                        ? seconds
+                        : DEFAULT_TOKEN_LIFETIME_SECONDS) * 1000;
+                this.expiresAt = new Date(
+                    Date.now() + Math.max(lifetimeMs - TOKEN_SAFETY_MARGIN, MIN_TOKEN_LIFETIME)
+                );
 
                 console.log("Token refreshed. Expires at: ", this.expiresAt);
             } catch (error) {
@@ -66,4 +83,4 @@ export class MinistryPlatformClient {
     public getHttpClient(): HttpClient {
         return this.httpClient;
     }
-}
+}