fix: tighten OAuth refresh validation

Refresh doctor probes online, fail closed on partial OAuth metadata, and cover resource-aware shared client creation.

Author: amix

src/commands/doctor.test.ts

@@ -114,6 +114,7 @@ describe('doctor command', () => {
         expect(consoleSpy).toHaveBeenCalledWith(
             expect.stringContaining('PASS Authenticated as person@example.com via secure-store'),
         )
+        expect(mockProbeApiToken).toHaveBeenCalledWith({ refresh: true })
         expect(mockCreateWrappedCommsClient).toHaveBeenCalledWith('test_token_123456789', {
             baseUrl: 'https://comms.staging.todoist.com',
         })
@@ -236,6 +237,7 @@ describe('doctor command', () => {
         const program = createProgram()
         await program.parseAsync(['node', 'tdc', 'doctor', '--offline'])
 
+        expect(mockProbeApiToken).toHaveBeenCalledWith({ refresh: false })
         expect(consoleSpy).toHaveBeenCalledWith(
             expect.stringContaining(
                 'SKIP Auth validation skipped (--offline); credentials found via secure-store',

src/commands/doctor.ts

@@ -245,7 +245,7 @@ async function checkAuthentication(offline: boolean): Promise<DoctorCheck> {
     let metadata: Awaited<ReturnType<typeof probeApiToken>>['metadata']
 
     try {
-        const probe = await probeApiToken()
+        const probe = await probeApiToken({ refresh: !offline })
         token = probe.token
         metadata = probe.metadata
     } catch (error) {

src/lib/api.test.ts

@@ -35,7 +35,13 @@ vi.mock('@doist/comms-sdk', () => {
 vi.mock('./auth.js', () => ({
     getApiTokenSnapshot: vi.fn().mockResolvedValue({
         token: 'test-token',
-        account: { id: '42', label: 'Ada', authMode: 'read-write', authScope: '' },
+        account: {
+            id: '42',
+            label: 'Ada',
+            authMode: 'read-write',
+            authResource: 'https://comms.staging.todoist.com',
+            authScope: '',
+        },
     }),
     getAuthMetadata: vi.fn().mockResolvedValue({ authMode: 'full' }),
 }))
@@ -59,12 +65,18 @@ vi.mock('./progress.js', () => ({
     getProgressTracker: () => ({ isEnabled: () => false, emitApiCall: vi.fn() }),
 }))
 
-const { clearWorkspaceUserCache, createWrappedCommsClient, getWorkspaceUsers } =
-    await import('./api.js')
+const {
+    clearApiClientCache,
+    clearWorkspaceUserCache,
+    createWrappedCommsClient,
+    getCommsClient,
+    getWorkspaceUsers,
+} = await import('./api.js')
 
 describe('getWorkspaceUsers', () => {
     beforeEach(() => {
         getWorkspaceUsersMock.mockClear()
+        clearApiClientCache()
         clearWorkspaceUserCache()
     })
 
@@ -109,6 +121,7 @@ describe('getWorkspaceUsers', () => {
 
 describe('wrapResult — central 403 translation', () => {
     beforeEach(() => {
+        clearApiClientCache()
         sdkMocks.createClient.mockReset()
         sdkMocks.deleteChannel.mockReset()
         sdkMocks.uploadAttachment.mockReset()
@@ -123,6 +136,14 @@ describe('wrapResult — central 403 translation', () => {
         })
     })
 
+    it('uses the active OAuth account resource when creating the shared SDK client', async () => {
+        await getCommsClient()
+
+        expect(sdkMocks.createClient).toHaveBeenCalledWith('test-token', {
+            baseUrl: 'https://comms.staging.todoist.com',
+        })
+    })
+
     it('translates a plain 403 into a FORBIDDEN CliError', async () => {
         sdkMocks.deleteChannel.mockRejectedValueOnce(
             new CommsRequestError('Request failed with status 403', 403, {}),

src/lib/api.ts

@@ -247,6 +247,10 @@ function analyzeAndEmitApiResponse(
 
 let apiClient: CommsApi | null = null
 
+export function clearApiClientCache(): void {
+    apiClient = null
+}
+
 export function createWrappedCommsClient(
     token: string,
     options: { baseUrl?: string } = {},

src/lib/auth-provider.test.ts

@@ -275,6 +275,40 @@ describe('createCommsAuthProvider', () => {
         expect(exchange.refreshToken).toBe('refresh_rotated')
     })
 
+    it('keeps stored account scope metadata when a refresh response omits scope', async () => {
+        const fetchImpl = vi.fn().mockResolvedValue(
+            jsonResponse({
+                access_token: 'tdc_fresh',
+                expires_in: 3600,
+            }),
+        )
+
+        const exchange = await createCommsAuthProvider(fetchImpl).refreshToken!({
+            refreshToken: 'refresh_old',
+            handshake: {
+                oauthClientId: 'tdd_123',
+                accountId: '42',
+                accountLabel: 'Ada',
+                authScope: READ_WRITE_SCOPES.join(' '),
+                authBaseUrl: 'https://todoist.com',
+                authorizationUrl: 'https://todoist.com/oauth/authorize',
+                tokenUrl: 'https://todoist.com/oauth/access_token',
+                registrationUrl: 'https://todoist.com/oauth/register',
+                resource: 'https://comms.todoist.com',
+            },
+        })
+
+        expect(exchange.account).toEqual({
+            id: '42',
+            label: 'Ada',
+            authMode: 'read-write',
+            authScope: READ_WRITE_SCOPES.join(' '),
+            oauthClientId: 'tdd_123',
+            authBaseUrl: 'https://todoist.com',
+            authResource: 'https://comms.todoist.com',
+        })
+    })
+
     it('translates invalid_grant refresh failures into AUTH_REFRESH_EXPIRED', async () => {
         const fetchImpl = vi.fn().mockResolvedValue(
             jsonResponse(

src/lib/auth-provider.ts

@@ -322,7 +322,14 @@ export function createCommsAuthProvider(
 }
 
 export function getCommsOAuthRefreshHandshake(account: CommsAccount): Record<string, unknown> {
-    const config = getCommsOAuthConfigForAccount(account)
+    if (!account.oauthClientId || !account.authBaseUrl || !account.authResource) {
+        throw new CliError(
+            'NO_TOKEN',
+            'Stored OAuth token cannot be refreshed because its client metadata is missing.',
+            ['Run: tdc auth login'],
+        )
+    }
+    const config = buildCommsOAuthConfig(account.authBaseUrl, account.authResource)
     return {
         ...config,
         oauthClientId: account.oauthClientId,
@@ -337,13 +344,6 @@ function getCommsOAuthConfig(): CommsOAuthConfig {
     return buildCommsOAuthConfig(getTodoistAuthBaseUrl(resource), resource)
 }
 
-function getCommsOAuthConfigForAccount(account: CommsAccount): CommsOAuthConfig {
-    if (account.authBaseUrl && account.authResource) {
-        return buildCommsOAuthConfig(account.authBaseUrl, account.authResource)
-    }
-    return getCommsOAuthConfig()
-}
-
 function buildCommsOAuthConfig(authBaseUrl: string, resource: string): CommsOAuthConfig {
     return {
         authBaseUrl,

src/lib/auth.test.ts

@@ -178,6 +178,28 @@ describe('auth shims over the cli-core keyring store', () => {
         )
     })
 
+    it('getApiToken rejects partial OAuth client metadata instead of defaulting refresh target', async () => {
+        mocks.activeBundleMock.mockResolvedValue({
+            account: {
+                ...STORED_ACCOUNT,
+                oauthClientId: 'tdd_123',
+            },
+            bundle: {
+                accessToken: 'tdc_old',
+                refreshToken: 'rt_old',
+                accessTokenExpiresAt: Date.now() - 1000,
+            },
+        })
+
+        await expect(getApiToken()).rejects.toMatchObject({
+            code: 'NO_TOKEN',
+            message:
+                'Stored OAuth token cannot be refreshed because its client metadata is missing.',
+        })
+
+        expect(mocks.refreshAccessTokenMock).not.toHaveBeenCalled()
+    })
+
     it('getApiToken uses a legacy OAuth token until it actually expires', async () => {
         mocks.activeBundleMock.mockResolvedValue({
             account: STORED_ACCOUNT,

src/lib/auth.ts

@@ -46,6 +46,10 @@ export type AuthProbeResult = {
     metadata: AuthProbeMetadata
 }
 
+export type AuthProbeOptions = {
+    refresh?: boolean
+}
+
 export type ActiveAuthSnapshot = {
     token: string
     account: CommsAccount
@@ -76,8 +80,8 @@ export async function getApiTokenSnapshot(ref?: string): Promise<ActiveAuthSnaps
 }
 
 /** Token + metadata in one round-trip for `tdc config view` / `tdc doctor`. */
-export async function probeApiToken(): Promise<AuthProbeResult> {
-    const snapshot = await getActiveSnapshot({ refresh: false })
+export async function probeApiToken(options: AuthProbeOptions = {}): Promise<AuthProbeResult> {
+    const snapshot = await getActiveSnapshot({ refresh: options.refresh === true })
     const source = await getActiveTokenSource()
     return {
         token: snapshot.token,