fix: clear stale client registration on re-login after token expiry

biome.json

@@ -19,5 +19,17 @@
     "rules": {
       "recommended": true
     }
-  }
+  },
+  "overrides": [
+    {
+      "includes": ["**/*.test.ts"],
+      "linter": {
+        "rules": {
+          "style": {
+            "noNonNullAssertion": "off"
+          }
+        }
+      }
+    }
+  ]
 }

src/commands/auth-status.test.ts

@@ -4,7 +4,6 @@ import {
   createMockAuthStorage,
   createValidTokenData,
   defaultClientRegistration,
-  defaultTokenResponse,
 } from "../services/test-helpers.js";
 import { authStatusAction } from "./auth-status.js";
 

src/commands/login.test.ts

@@ -152,6 +152,164 @@ describe("loginAction", () => {
     consoleSpy.mockRestore();
   });
 
+  it("clears stale client registration when tokens are absent", async () => {
+    const consoleSpy = spyOn(console, "log").mockImplementation(() => {});
+    const authStorage = createMockAuthStorage();
+    const authService = createMockAuthService();
+
+    await loginAction(
+      { port: 8080 },
+      {
+        authService,
+        authStorage,
+        browserService: createMockBrowserService(),
+        mcpUrl,
+      },
+    );
+
+    expect(authStorage.clearClient).toHaveBeenCalledWith(mcpUrl);
+    expect(authService.registerClient).toHaveBeenCalled();
+    expect(authStorage.saveClient).toHaveBeenCalled();
+    consoleSpy.mockRestore();
+  });
+
+  it("clears client registration on token exchange failure", async () => {
+    const consoleSpy = spyOn(console, "log").mockImplementation(() => {});
+    const errorSpy = spyOn(console, "error").mockImplementation(() => {});
+    const exitSpy = spyOn(process, "exit").mockImplementation(() => {
+      throw new Error("process.exit");
+    });
+
+    const authStorage = createMockAuthStorage();
+    const authService = createMockAuthService({
+      exchangeCodeForTokens: mock(() => {
+        throw new Error("invalid_grant");
+      }),
+    });
+
+    try {
+      await loginAction(
+        { port: 8080 },
+        {
+          authService,
+          authStorage,
+          browserService: createMockBrowserService(),
+          mcpUrl,
+        },
+      );
+    } catch {
+      // Expected: process.exit mock throws
+    }
+
+    expect(authStorage.clearClient).toHaveBeenCalledWith(mcpUrl);
+    expect(exitSpy).toHaveBeenCalledWith(1);
+
+    consoleSpy.mockRestore();
+    errorSpy.mockRestore();
+    exitSpy.mockRestore();
+  });
+
+  it("still shows error and exits when clearClient fails during token exchange error", async () => {
+    const consoleSpy = spyOn(console, "log").mockImplementation(() => {});
+    const errorSpy = spyOn(console, "error").mockImplementation(() => {});
+    const exitSpy = spyOn(process, "exit").mockImplementation(() => {
+      throw new Error("process.exit");
+    });
+
+    let clearClientCallCount = 0;
+    const authStorage = createMockAuthStorage({
+      clearClient: mock(() => {
+        clearClientCallCount++;
+        // First call is from the !existing path (Part 1), let it succeed.
+        // Second call is from the catch block (Part 2), make it fail.
+        if (clearClientCallCount > 1) {
+          throw new Error("fs error");
+        }
+        return Promise.resolve();
+      }),
+    });
+    const authService = createMockAuthService({
+      exchangeCodeForTokens: mock(() => {
+        throw new Error("invalid_grant");
+      }),
+    });
+
+    try {
+      await loginAction(
+        { port: 8080 },
+        {
+          authService,
+          authStorage,
+          browserService: createMockBrowserService(),
+          mcpUrl,
+        },
+      );
+    } catch {
+      // Expected: process.exit mock throws
+    }
+
+    expect(exitSpy).toHaveBeenCalledWith(1);
+    expect(errorSpy).toHaveBeenCalled();
+
+    consoleSpy.mockRestore();
+    errorSpy.mockRestore();
+    exitSpy.mockRestore();
+  });
+
+  it("does not clear client when tokens are still valid", async () => {
+    const consoleSpy = spyOn(console, "log").mockImplementation(() => {});
+    const authStorage = createMockAuthStorage({
+      loadTokens: mock(() =>
+        Promise.resolve(
+          createValidTokenData({
+            expiresAt: new Date(Date.now() + 3600_000).toISOString(),
+          }),
+        ),
+      ),
+    });
+
+    await loginAction(
+      {},
+      {
+        authService: createMockAuthService(),
+        authStorage,
+        browserService: createMockBrowserService(),
+        mcpUrl,
+      },
+    );
+
+    expect(authStorage.clearClient).not.toHaveBeenCalled();
+    expect(authStorage.saveTokens).not.toHaveBeenCalled();
+    consoleSpy.mockRestore();
+  });
+
+  it("does not clear client when tokens are expired but present", async () => {
+    const consoleSpy = spyOn(console, "log").mockImplementation(() => {});
+    const authStorage = createMockAuthStorage({
+      loadTokens: mock(() =>
+        Promise.resolve(
+          createValidTokenData({
+            expiresAt: new Date(Date.now() - 3600_000).toISOString(),
+          }),
+        ),
+      ),
+    });
+
+    await loginAction(
+      { port: 8080 },
+      {
+        authService: createMockAuthService(),
+        authStorage,
+        browserService: createMockBrowserService(),
+        mcpUrl,
+      },
+    );
+
+    expect(authStorage.clearClient).not.toHaveBeenCalled();
+    expect(authStorage.saveTokens).toHaveBeenCalled();
+    consoleSpy.mockRestore();
+  });
+
   it("proceeds with login when token is expired", async () => {
     const consoleSpy = spyOn(console, "log").mockImplementation(() => {});
     const authStorage = createMockAuthStorage({

src/commands/login.ts

@@ -59,6 +59,12 @@ export async function loginAction(
     console.log("Re-authenticating (--force flag)...\n");
   }
 
+  // If tokens were cleared (expired+refreshFailed or never existed) but a stale
+  // client registration remains, clear it so we get a fresh DCR registration.
+  if (!existing) {
+    await authStorage.clearClient(mcpUrl);
+  }
+
   // Step 1: Discover OAuth endpoints
   console.log("Discovering OAuth endpoints...");
   const metadata = await authService.discoverEndpoints(mcpUrl);
@@ -183,6 +189,13 @@ export async function loginAction(
       redirectUri,
     });
   } catch (error) {
+    // Best-effort: clear potentially stale client so next login starts fresh.
+    // Swallow clearClient errors to ensure we always reach the user-facing error path.
+    try {
+      await authStorage.clearClient(mcpUrl);
+    } catch {
+      // Ignore -- client cleanup is best-effort
+    }
     console.error(
       `Failed to complete authentication: ${error instanceof Error ? error.message : error}\n`,
     );

src/services/chunking-keyring-service.test.ts

@@ -222,7 +222,7 @@ describe("ChunkingKeyringService", () => {
       expect(sentinel).toMatch(/^CHUNKED:[a-z0-9]+:3$/);
 
       // Parse sentinel to find writeId and verify chunks
-      const parsed = parseChunkedSentinel(sentinel!);
+      const parsed = parseChunkedSentinel(sentinel as string);
       expect(parsed).not.toBeNull();
       expect(
         inner.getPassword(SERVICE, chunkKey(ACCOUNT, parsed!.writeId, 0)),
@@ -242,7 +242,7 @@ describe("ChunkingKeyringService", () => {
       // Write a chunked value
       chunking.setPassword(SERVICE, ACCOUNT, "a".repeat(25));
       const oldSentinel = parseChunkedSentinel(
-        inner.getPassword(SERVICE, ACCOUNT)!,
+        inner.getPassword(SERVICE, ACCOUNT) as string,
       );
       expect(oldSentinel).not.toBeNull();
 
@@ -268,13 +268,13 @@ describe("ChunkingKeyringService", () => {
       // Write 3-chunk value
       chunking.setPassword(SERVICE, ACCOUNT, "a".repeat(25));
       const oldSentinel = parseChunkedSentinel(
-        inner.getPassword(SERVICE, ACCOUNT)!,
+        inner.getPassword(SERVICE, ACCOUNT) as string,
       );
 
       // Overwrite with 2-chunk value
       chunking.setPassword(SERVICE, ACCOUNT, "b".repeat(15));
       const newSentinel = parseChunkedSentinel(
-        inner.getPassword(SERVICE, ACCOUNT)!,
+        inner.getPassword(SERVICE, ACCOUNT) as string,
       );
       expect(newSentinel!.count).toBe(2);
 
@@ -311,10 +311,14 @@ describe("ChunkingKeyringService", () => {
       const chunking = new ChunkingKeyringService(inner, 10);
 
       chunking.setPassword(SERVICE, ACCOUNT, "a".repeat(15));
-      const first = parseChunkedSentinel(inner.getPassword(SERVICE, ACCOUNT)!);
+      const first = parseChunkedSentinel(
+        inner.getPassword(SERVICE, ACCOUNT) as string,
+      );
 
       chunking.setPassword(SERVICE, ACCOUNT, "b".repeat(15));
-      const second = parseChunkedSentinel(inner.getPassword(SERVICE, ACCOUNT)!);
+      const second = parseChunkedSentinel(
+        inner.getPassword(SERVICE, ACCOUNT) as string,
+      );
 
       expect(first!.writeId).not.toBe(second!.writeId);
     });
@@ -371,7 +375,7 @@ describe("ChunkingKeyringService", () => {
       const chunking = new ChunkingKeyringService(inner, 10);
       chunking.setPassword(SERVICE, ACCOUNT, "a".repeat(25));
       const sentinel = parseChunkedSentinel(
-        inner.getPassword(SERVICE, ACCOUNT)!,
+        inner.getPassword(SERVICE, ACCOUNT) as string,
       );
 
       const result = chunking.deletePassword(SERVICE, ACCOUNT);
@@ -414,15 +418,14 @@ describe("ChunkingKeyringService", () => {
 
       const sentinel = inner.getPassword(SERVICE, ACCOUNT);
       expect(sentinel).toMatch(/^CHUNKED:/);
-      const parsed = parseChunkedSentinel(sentinel!);
-      expect(parsed!.count).toBe(2);
+      const parsed = parseChunkedSentinel(sentinel as string);
+      expect(parsed?.count).toBe(2);
     });
 
     it("round-trips a chunked value through set and get", () => {
       const inner = createInMemoryKeyring();
       const chunking = new ChunkingKeyringService(inner, 10);
-      const value =
-        '{"accessToken":"' + "a".repeat(50) + '","refreshToken":"b"}';
+      const value = `{"accessToken":"${"a".repeat(50)}","refreshToken":"b"}`;
 
       chunking.setPassword(SERVICE, ACCOUNT, value);
       const result = chunking.getPassword(SERVICE, ACCOUNT);

src/services/token-manager.test.ts

@@ -1,11 +1,9 @@
 import { describe, expect, it, mock } from "bun:test";
-import type { TokenData } from "./auth-storage.js";
 import {
   createMockAuthService,
   createMockAuthStorage,
   createValidTokenData,
   defaultClientRegistration,
-  defaultOAuthMetadata,
   defaultTokenResponse,
 } from "./test-helpers.js";
 import {