Improve remote Matrix token refresh

src/bootstrap.ts

@@ -22,9 +22,9 @@ import {
   ChatService,
   MatrixClient,
   MatrixMediaDownloader,
+  MatrixTokenRefresher,
   extractMatrixCredentials,
   loadCachedCredentials,
-  type TokenRefresher,
   type UsernameResolver,
   type MatrixCredentials,
 } from './services/chat/index.js';
@@ -187,16 +187,15 @@ export async function bootstrap(config: Config, logger: Logger): Promise<AppCont
   const mediaDownloader = new MatrixMediaDownloader(matrixClient, logger);
   const mediaService = new MediaService(mediaDownloader, logger);
 
-  // 3. Token refresher — on 401, re-sniff via the running browser session
+  // 3. Token refresher — on 401, restore the persistent Reddit session when
-  //    and swap the new credentials into the chat service.
+  //    possible, re-sniff once, and swap credentials into the chat service.
+  //    Concurrent callers share that refresh rather than opening extra pages.
   let chatService: ChatService;
-  const tokenRefresher: TokenRefresher = {
+  const tokenRefresher = new MatrixTokenRefresher(
-    async refresh() {
+    browserClient,
-      const fresh = await extractMatrixCredentials(browserClient.getContext()!, logger);
+    logger,
-      const next = new MatrixClient(fresh, logger);
+    (fresh) => chatService.setMatrixClient(new MatrixClient(fresh, logger)),
-      chatService.setMatrixClient(next);
+  );
-    },
-  };
 
   // 4. Username resolver — turns t2 IDs into real Reddit usernames so the
   //    dashboard isn't full of `t2_abc123`. Uses Reddit's bulk endpoint.

src/services/chat/index.ts

@@ -20,6 +20,11 @@ export type { MatrixCredentials } from './matrix/types.js';
 export {
   extractMatrixCredentials,
   loadCachedCredentials,
+  MatrixBrowserLoginRequiredError,
   saveCredentials,
 } from './matrix/token-extractor.js';
+export {
+  MatrixRemoteLoginRequiredError,
+  MatrixTokenRefresher,
+} from './matrix/token-refresher.js';
 export { MatrixMediaDownloader } from './matrix/http-media-downloader.js';

src/services/chat/matrix/token-extractor.ts

@@ -16,6 +16,16 @@ export interface TokenExtractorOptions {
   timeoutMs?: number;
 }
 
+export class MatrixBrowserLoginRequiredError extends Error {
+  constructor(loginUrl: string) {
+    super(
+      `Reddit redirected Matrix token capture to the login page (${loginUrl}). ` +
+      'Restore the browser session before retrying.',
+    );
+    this.name = 'MatrixBrowserLoginRequiredError';
+  }
+}
+
 export async function loadCachedCredentials(
   cachePath = DEFAULT_CACHE_PATH,
 ): Promise<MatrixCredentials | null> {
@@ -101,6 +111,20 @@ export async function extractMatrixCredentials(
     };
 
     page.on('request', onRequest);
+    page.on('framenavigated', (frame) => {
+      if (done || frame !== page.mainFrame()) return;
+      try {
+        const url = new URL(frame.url());
+        const isRedditHost = url.hostname === 'reddit.com' || url.hostname === 'www.reddit.com';
+        if (isRedditHost && url.pathname.startsWith('/login')) {
+          done = true;
+          logger.warn('[matrix] chat page redirected to Reddit login; token capture aborted');
+          rejectCreds(new MatrixBrowserLoginRequiredError(url.toString()));
+        }
+      } catch {
+        // Ignore transient or malformed navigation URLs.
+      }
+    });
 
     // Reddit now redirects `chat.reddit.com` → `www.reddit.com/chat/`. Going
     // direct to the canonical URL avoids one redirect hop and means the
@@ -124,17 +148,19 @@ export async function extractMatrixCredentials(
         );
       });
 
-    const timeout = new Promise<MatrixCredentials>((_, reject) =>
+    let timeoutHandle: ReturnType<typeof setTimeout> | undefined;
-      setTimeout(
+    const timeout = new Promise<MatrixCredentials>((_, reject) => {
+      timeoutHandle = setTimeout(
         () => reject(new Error(`Matrix token capture timed out after ${timeoutMs}ms`)),
         timeoutMs,
-      ),
+      );
-    );
+    });
 
     let captured: MatrixCredentials;
     try {
       captured = await Promise.race([credsPromise, timeout]);
     } catch (err) {
+      if (err instanceof MatrixBrowserLoginRequiredError) throw err;
       // On timeout, surface enough info to tell the three failure modes apart:
       //   (a) chat.reddit.com never loaded         → page.url() still about:blank, almost no hosts
       //   (b) page loaded, no Matrix host hit      → reddit hosts present, no matrix.*
@@ -156,6 +182,8 @@ export async function extractMatrixCredentials(
         logger.warn('[matrix] failed to collect timeout diagnostics:', diagErr);
       }
       throw err;
+    } finally {
+      if (timeoutHandle) clearTimeout(timeoutHandle);
     }
 
     // Resolve the user_id by calling /whoami so the caller knows who we are.

src/services/chat/matrix/token-refresher.ts

@@ -0,0 +1,69 @@
+import type { BrowserContext } from 'playwright';
+import type { Logger } from '../../../utils/logger.js';
+import type { TokenRefresher } from '../service.js';
+import type { MatrixCredentials } from './types.js';
+import { extractMatrixCredentials } from './token-extractor.js';
+
+export interface MatrixSessionBrowser {
+  getContext(): BrowserContext | null;
+  isSessionValid(): Promise<boolean>;
+  login(): Promise<boolean>;
+}
+
+export type MatrixCredentialExtractor = (
+  context: BrowserContext,
+  logger: Logger,
+) => Promise<MatrixCredentials>;
+
+export class MatrixRemoteLoginRequiredError extends Error {
+  constructor() {
+    super(
+      'Reddit browser session has expired and automatic login could not restore it. ' +
+      'Open the dashboard and use the noVNC console to log in to Reddit, then retry chat sync.',
+    );
+    this.name = 'MatrixRemoteLoginRequiredError';
+  }
+}
+
+/**
+ * Restores the browser login when needed, then captures and applies one fresh
+ * Matrix credential set. Concurrent callers share the same browser operation.
+ */
+export class MatrixTokenRefresher implements TokenRefresher {
+  private refreshInFlight: Promise<void> | null = null;
+
+  constructor(
+    private readonly browser: MatrixSessionBrowser,
+    private readonly logger: Logger,
+    private readonly applyCredentials: (credentials: MatrixCredentials) => void,
+    private readonly extract: MatrixCredentialExtractor = extractMatrixCredentials,
+  ) {}
+
+  refresh(): Promise<void> {
+    if (this.refreshInFlight) return this.refreshInFlight;
+
+    this.refreshInFlight = this.performRefresh().finally(() => {
+      this.refreshInFlight = null;
+    });
+    return this.refreshInFlight;
+  }
+
+  private async performRefresh(): Promise<void> {
+    const sessionValid = await this.browser.isSessionValid();
+    if (!sessionValid) {
+      this.logger.warn('[matrix] Reddit browser session expired; attempting login recovery');
+      if (!(await this.browser.login())) {
+        throw new MatrixRemoteLoginRequiredError();
+      }
+      this.logger.info('[matrix] Reddit browser session restored');
+    }
+
+    const context = this.browser.getContext();
+    if (!context) {
+      throw new Error('Matrix token refresh requires an initialized browser context');
+    }
+
+    const credentials = await this.extract(context, this.logger);
+    this.applyCredentials(credentials);
+  }
+}

tests/matrix-token-extractor.test.ts

@@ -0,0 +1,33 @@
+import { describe, expect, it, vi } from 'vitest';
+import type { BrowserContext, Frame } from 'playwright';
+import { createMockLogger } from './mockLogger.js';
+import {
+  extractMatrixCredentials,
+  MatrixBrowserLoginRequiredError,
+} from '../src/services/chat/matrix/token-extractor.js';
+
+describe('extractMatrixCredentials', () => {
+  it('fails as soon as Reddit redirects the chat page to login', async () => {
+    const handlers = new Map<string, (value: never) => void>();
+    const frame = { url: () => 'https://www.reddit.com/login/?dest=https://www.reddit.com/chat/' } as Frame;
+    const page = {
+      on: vi.fn((event: string, handler: (value: never) => void) => {
+        handlers.set(event, handler);
+      }),
+      mainFrame: vi.fn(() => frame),
+      goto: vi.fn(async () => {
+        handlers.get('framenavigated')?.(frame as never);
+        return null;
+      }),
+      url: vi.fn(() => frame.url()),
+      screenshot: vi.fn(async () => undefined),
+      close: vi.fn(async () => undefined),
+    };
+    const context = { newPage: vi.fn(async () => page) } as unknown as BrowserContext;
+
+    await expect(
+      extractMatrixCredentials(context, createMockLogger(), { timeoutMs: 5_000 }),
+    ).rejects.toBeInstanceOf(MatrixBrowserLoginRequiredError);
+    expect(page.screenshot).not.toHaveBeenCalled();
+  });
+});

tests/matrix-token-refresher.test.ts

@@ -0,0 +1,78 @@
+import { describe, expect, it, vi } from 'vitest';
+import type { BrowserContext } from 'playwright';
+import { createMockLogger } from './mockLogger.js';
+import {
+  MatrixRemoteLoginRequiredError,
+  MatrixTokenRefresher,
+} from '../src/services/chat/matrix/token-refresher.js';
+import type { MatrixCredentials } from '../src/services/chat/matrix/types.js';
+
+const credentials: MatrixCredentials = {
+  homeserver: 'https://matrix.redditspace.com',
+  accessToken: 'fresh-token',
+  userId: '@t2_bot:reddit.com',
+  capturedAt: 123,
+};
+
+function setup(options: { sessionValid?: boolean; loginSucceeds?: boolean } = {}) {
+  const context = {} as BrowserContext;
+  const browser = {
+    getContext: vi.fn(() => context),
+    isSessionValid: vi.fn(async () => options.sessionValid ?? true),
+    login: vi.fn(async () => options.loginSucceeds ?? true),
+  };
+  const extract = vi.fn(async () => credentials);
+  const applyCredentials = vi.fn();
+  const logger = createMockLogger();
+  const refresher = new MatrixTokenRefresher(
+    browser,
+    logger,
+    applyCredentials,
+    extract,
+  );
+
+  return { refresher, browser, context, extract, applyCredentials, logger };
+}
+
+describe('MatrixTokenRefresher', () => {
+  it('repairs an expired Reddit browser session before capturing a Matrix token', async () => {
+    const { refresher, browser, context, extract, applyCredentials } = setup({
+      sessionValid: false,
+      loginSucceeds: true,
+    });
+
+    await refresher.refresh();
+
+    expect(browser.login).toHaveBeenCalledOnce();
+    expect(extract).toHaveBeenCalledWith(context, expect.anything());
+    expect(applyCredentials).toHaveBeenCalledWith(credentials);
+  });
+
+  it('fails fast with remote-login guidance when the Reddit session cannot be restored', async () => {
+    const { refresher, browser, extract } = setup({
+      sessionValid: false,
+      loginSucceeds: false,
+    });
+
+    await expect(refresher.refresh()).rejects.toBeInstanceOf(MatrixRemoteLoginRequiredError);
+    await expect(refresher.refresh()).rejects.toThrow(/dashboard.*noVNC/i);
+    expect(browser.login).toHaveBeenCalledTimes(2);
+    expect(extract).not.toHaveBeenCalled();
+  });
+
+  it('coalesces concurrent refreshes so only one browser capture runs', async () => {
+    const { refresher, extract, applyCredentials } = setup();
+    let release: ((value: MatrixCredentials) => void) | undefined;
+    extract.mockImplementation(
+      () => new Promise<MatrixCredentials>((resolve) => { release = resolve; }),
+    );
+
+    const first = refresher.refresh();
+    const second = refresher.refresh();
+    await vi.waitFor(() => expect(extract).toHaveBeenCalledOnce());
+    release?.(credentials);
+    await Promise.all([first, second]);
+
+    expect(applyCredentials).toHaveBeenCalledOnce();
+  });
+});