fix: TTS sanitization (code-scanning #21) + repair broken Playwright e2e suite (#190)

README.md

@@ -132,6 +132,7 @@ Open [localhost:3000](http://localhost:3000). Log in with GitHub. Done.
 | `TOKEN_MAX_AGE_MS` | `86400000` | Force re-auth interval (24h) |
 | `SESSION_POOL_TTL_MS` | `300000` | Session TTL when disconnected (5 min) |
 | `MAX_SESSIONS_PER_USER` | `5` | Max concurrent tabs/devices |
+| `RATE_LIMIT_MAX` | `200` | Max requests per IP per 15 min; `0` disables (used by E2E tests) |
 | `COPILOT_CONFIG_DIR` | `~/.copilot` | Share with CLI for bidirectional sync |
 | `SESSION_STORE_PATH` | `/data/sessions` | Persistent session directory |
 | `SETTINGS_STORE_PATH` | `/data/settings` | Per-user settings directory |

playwright.config.ts

@@ -34,6 +34,8 @@ export default defineConfig({
       GITHUB_CLIENT_ID: 'test-client-id',
       SESSION_SECRET: 'test-secret-for-playwright',
       NODE_ENV: 'development',
+      // E2E suite issues far more than the production limit from a single IP — disable it.
+      RATE_LIMIT_MAX: '0',
     },
   },
 });

src/hooks.server.test.ts

@@ -69,6 +69,7 @@ vi.mock('$lib/server/config.js', () => ({
 	config: {
 		sessionSecret: 'test-secret',
 		tokenMaxAge: 7 * 24 * 60 * 60 * 1000,
+		rateLimitMax: 200,
 	},
 }));
 

src/hooks.server.ts

@@ -92,7 +92,10 @@ const csrfProtection: Handle = async ({ event, resolve }) => {
 // Rate limiting with periodic cleanup to prevent unbounded Map growth
 const rateLimitMap = new Map<string, { count: number; resetTime: number }>();
 const RATE_LIMIT_WINDOW = 15 * 60 * 1000;
-const RATE_LIMIT_MAX = 200;
+// Configurable via RATE_LIMIT_MAX. A value <= 0 disables rate limiting entirely
+// (used by the E2E test harness, which issues far more than 200 requests per run from one IP).
+const RATE_LIMIT_MAX = config.rateLimitMax;
+const RATE_LIMIT_ENABLED = RATE_LIMIT_MAX > 0;
 
 // Purge expired entries every 15 minutes
 setInterval(() => {
@@ -103,6 +106,10 @@ setInterval(() => {
 }, RATE_LIMIT_WINDOW);
 
 const rateLimit: Handle = async ({ event, resolve }) => {
+  if (!RATE_LIMIT_ENABLED) {
+    return resolve(event);
+  }
+
   const ip = event.getClientAddress();
   const now = Date.now();
 

src/lib/server/config.ts

@@ -25,6 +25,7 @@ function getConfig() {
       ? process.env.ALLOWED_GITHUB_USERS.split(',').map((u: string) => u.trim().toLowerCase())
       : [],
     tokenMaxAge: parseInt(env('TOKEN_MAX_AGE_MS', String(7 * 24 * 60 * 60 * 1000))),
+    rateLimitMax: parseInt(env('RATE_LIMIT_MAX', '200')),
     sessionPoolTtl: parseInt(env('SESSION_POOL_TTL_MS', String(5 * 60 * 1000))),
     maxSessionsPerUser: parseInt(env('MAX_SESSIONS_PER_USER', '5')),
     copilotConfigDir: process.env.COPILOT_CONFIG_DIR?.trim().replace(/^~/, homedir()) || undefined,

src/lib/stores/tts.svelte.ts

@@ -1,29 +1,41 @@
 const SUPPORTED = typeof window !== 'undefined' && 'speechSynthesis' in window;
 
+/** Remove all HTML tags, repeating until no tags remain (prevents incomplete sanitization). */
+function stripHtmlTags(text: string): string {
+  const tagPattern = /<[^>]*>/g;
+  let previous = text;
+  let result = text.replace(tagPattern, '');
+  while (result !== previous) {
+    previous = result;
+    result = result.replace(tagPattern, '');
+  }
+  return result;
+}
+
 /** Strip markdown/HTML to plain text suitable for speech synthesis. */
 function stripToPlainText(markdown: string): string {
-  return markdown
-    // Remove code blocks (``` ... ```)
-    .replace(/```[\s\S]*?```/g, ' code block omitted ')
-    // Remove inline code
-    .replace(/`([^`]+)`/g, '$1')
-    // Remove images
-    .replace(/!\[([^\]]*)\]\([^)]+\)/g, '$1')
-    // Remove links — keep text
-    .replace(/\[([^\]]+)\]\([^)]+\)/g, '$1')
-    // Remove headings markers
-    .replace(/^#{1,6}\s+/gm, '')
-    // Remove bold/italic markers
-    .replace(/\*{1,3}([^*]+)\*{1,3}/g, '$1')
-    .replace(/_{1,3}([^_]+)_{1,3}/g, '$1')
-    // Remove strikethrough
-    .replace(/~~([^~]+)~~/g, '$1')
-    // Remove blockquotes
-    .replace(/^>\s+/gm, '')
-    // Remove horizontal rules
-    .replace(/^[-*_]{3,}\s*$/gm, '')
-    // Remove HTML tags
-    .replace(/<[^>]+>/g, '')
+  return stripHtmlTags(
+    markdown
+      // Remove code blocks (``` ... ```)
+      .replace(/```[\s\S]*?```/g, ' code block omitted ')
+      // Remove inline code
+      .replace(/`([^`]+)`/g, '$1')
+      // Remove images
+      .replace(/!\[([^\]]*)\]\([^)]+\)/g, '$1')
+      // Remove links — keep text
+      .replace(/\[([^\]]+)\]\([^)]+\)/g, '$1')
+      // Remove headings markers
+      .replace(/^#{1,6}\s+/gm, '')
+      // Remove bold/italic markers
+      .replace(/\*{1,3}([^*]+)\*{1,3}/g, '$1')
+      .replace(/_{1,3}([^_]+)_{1,3}/g, '$1')
+      // Remove strikethrough
+      .replace(/~~([^~]+)~~/g, '$1')
+      // Remove blockquotes
+      .replace(/^>\s+/gm, '')
+      // Remove horizontal rules
+      .replace(/^[-*_]{3,}\s*$/gm, '')
+  )
     // Collapse multiple newlines/spaces
     .replace(/\n{2,}/g, '. ')
     .replace(/\n/g, ' ')

tests/auth-flow.spec.ts

@@ -1,7 +1,13 @@
 import { test, expect } from '@playwright/test';
 import type { Page } from '@playwright/test';
 
-import { MOCK_USER } from './helpers';
+import { MOCK_USER, openSidebar } from './helpers';
+
+// The service worker's `controllerchange` handler triggers `window.location.reload()`,
+// and on reload the SW serves precached HTML without the test's auth patch — flipping
+// the page back to the login screen. Block service workers so the mocked authenticated
+// state persists (mirrors `createAuthenticatedPage` in helpers.ts).
+test.use({ serviceWorkers: 'block' });
 
 const DEFAULT_DEVICE_FLOW = {
   user_code: 'ABCD-1234',
@@ -17,21 +23,25 @@ const AUTHORIZED_USER = {
 
 function buildLayoutData(authenticated: boolean) {
   if (authenticated) {
+    // Layout node (index 0) must explicitly carry authenticated=true via devalue so
+    // that invalidateAll() after the device-flow reload upgrades a previously
+    // unauthenticated page. `byokEnabled` mirrors the current +layout.server shape.
     return {
       type: 'data',
       nodes: [
         {
           type: 'data',
           data: [
-            { authenticated: 1, user: 2 },
+            { authenticated: 1, user: 2, byokEnabled: 5 },
             true,
             { login: 3, name: 4 },
             MOCK_USER.login,
             MOCK_USER.name,
+            false,
           ],
           uses: {},
         },
-        { type: 'skip' },
+        null,
       ],
     };
   }
@@ -41,10 +51,10 @@ function buildLayoutData(authenticated: boolean) {
     nodes: [
       {
         type: 'data',
-        data: [{ authenticated: 1, user: 2 }, false, null],
+        data: [{ authenticated: 1, user: 2, byokEnabled: 3 }, false, null, false],
         uses: {},
       },
-      { type: 'skip' },
+      null,
     ],
   };
 }
@@ -113,8 +123,8 @@ async function mockAuthReloadState(
 
     if (authState.authenticated) {
       html = html.replace(
-        /data:\{authenticated:false,user:null\}/g,
-        `data:{authenticated:true,user:{login:"${MOCK_USER.login}",name:"${MOCK_USER.name}"}}`,
+        /authenticated:false,user:null/g,
+        `authenticated:true,user:{login:"${MOCK_USER.login}",name:"${MOCK_USER.name}"}`,
       );
     }
 
@@ -174,7 +184,7 @@ test.describe('Device flow authentication', () => {
     await expect(page.locator('.login-status')).toContainText('Waiting for authorization');
 
     await expect(page.locator('.terminal')).toBeVisible({ timeout: 15000 });
-    await expect(page.locator('.hamburger-btn')).toBeVisible({ timeout: 15000 });
+    await expect(page.locator('button.model-pill')).toBeVisible({ timeout: 15000 });
     await expect(page.locator('.login-screen')).toBeHidden();
   });
 
@@ -298,7 +308,7 @@ test.describe('Authenticated session actions', () => {
     await page.goto('/');
     await expect(page.locator('.terminal')).toBeVisible({ timeout: 10000 });
 
-    await page.locator('button.hamburger-btn').click();
+    await openSidebar(page);
     const signOutButton = page.locator('button.sidebar-action.sidebar-action-danger');
     await expect(signOutButton).toBeVisible();
 

tests/chat-messaging.spec.ts

@@ -35,7 +35,7 @@ test.describe('Chat messaging', () => {
   test('shows the banner before the first message', async ({ browser }) => {
     await withAuthenticatedChat(browser, {}, async (page) => {
       await expect(page.locator('.banner-box')).toBeVisible();
-      await expect(page.locator('button.send-btn')).toBeVisible();
+      await expect(page.locator('button.send-btn, button.mic-btn').first()).toBeVisible();
     });
   });
 
@@ -166,9 +166,9 @@ test.describe('Chat messaging', () => {
             seq
               .send({ type: 'turn_start' }, 20)
               .send({ type: 'delta', content: 'Usage details coming up.' }, 100)
+              .send({ type: 'usage', inputTokens: 12, outputTokens: 8 }, 50)
               .send({ type: 'turn_end' }, 120)
-              .send({ type: 'done' }, 50)
-              .send({ type: 'usage', inputTokens: 12, outputTokens: 8 }, 50);
+              .send({ type: 'done' }, 50);
           }
         },
       },

tests/chat.spec.ts

@@ -54,7 +54,7 @@ test.describe('Chat screen structure', () => {
     expect(response.headers()['content-type']).toContain('application/json');
 
     const data = await response.json();
-    expect(data).toEqual({ status: 'ok' });
+    expect(data).toMatchObject({ status: 'ok' });
   });
 
   test('auth status endpoint returns unauthenticated JSON shape', async ({ page }) => {

tests/error-handling.spec.ts

@@ -87,9 +87,10 @@ test.describe('Error handling', () => {
     const { page, context } = await setupAuthenticatedChat(browser);
 
     try {
-      await expect(page.locator('.conn-dot.dot-connected')).toBeVisible();
-      await expect(page.locator('.conn-dot.dot-disconnected')).toHaveCount(0);
-      await expect(page.locator('.conn-dot.dot-connecting')).toHaveCount(0);
+      // The connection-dot indicator was removed in the TopBar redesign; a
+      // connected session is now reflected by the chat input being enabled.
+      await expect(page.locator('.input-area textarea')).toBeEnabled();
+      await expect(page.locator('.input-area textarea')).toHaveAttribute('placeholder', 'Ask anything…');
     } finally {
       await context.close();
     }
@@ -99,7 +100,7 @@ test.describe('Error handling', () => {
     const response = await request.get('/health');
 
     expect(response.status()).toBe(200);
-    expect(await response.json()).toEqual({ status: 'ok' });
+    expect(await response.json()).toMatchObject({ status: 'ok' });
   });
 
   test('auth status shows unauthenticated', async ({ request }) => {
@@ -203,7 +204,7 @@ test.describe('Error handling', () => {
 
     try {
       await goToChat(page);
-      await expect(page.locator('.conn-dot.dot-connected')).toBeVisible();
+      await expect(page.locator('.input-area textarea')).toBeEnabled();
       await sendMessage(page, 'sanity check');
       await expect(page.locator('.message.assistant')).toContainText('All good here');
 

tests/helpers.ts

@@ -7,7 +7,7 @@
  * - MOCK_MODELS, MOCK_USER — shared test data
  */
 
-import type { Browser, Page, BrowserContext } from '@playwright/test';
+import { expect, type Browser, type Page, type BrowserContext } from '@playwright/test';
 
 // ── Shared test data ──────────────────────────────────────────────────────────
 
@@ -28,8 +28,8 @@ export const MOCK_TOOLS = [
 ];
 
 export const MOCK_AGENTS = [
-  { slug: 'copilot', name: 'Copilot', description: 'Default assistant', current: true },
-  { slug: 'reviewer', name: 'Code Reviewer', description: 'Reviews code changes', current: false },
+  { name: 'Copilot', description: 'Default assistant', source: 'builtin', isSelected: true },
+  { name: 'Code Reviewer', description: 'Reviews code changes', source: 'user', isSelected: false },
 ];
 
 export const MOCK_SESSIONS = [
@@ -216,6 +216,20 @@ export async function sendMessage(page: Page, text: string) {
   await page.keyboard.press('Enter');
 }
 
+/**
+ * Opens the sidebar so its actions (Sessions, Settings, Sign Out) are clickable.
+ *
+ * On mobile widths the sidebar is hidden behind a hamburger toggle; on desktop
+ * (>=1024px) it is persistent and always visible. This helper handles both cases.
+ */
+export async function openSidebar(page: Page) {
+  const hamburger = page.locator('button.hamburger-btn');
+  if (await hamburger.isVisible().catch(() => false)) {
+    await hamburger.click();
+  }
+  await expect(page.locator('.sidebar')).toBeVisible();
+}
+
 /**
  * Creates a delayed message sender for simulating server response sequences.
  */

tests/model-selection.spec.ts

@@ -56,7 +56,7 @@ test.describe('Model selection', () => {
     const { page, context } = await openAuthenticatedChat(browser);
 
     try {
-      await expect(page.locator('.conn-dot')).toHaveClass(/dot-connected/);
+      await expect(page.locator('.input-area textarea')).toBeEnabled();
       await expect(page.locator('.model-name')).toHaveText('gpt-4.1');
     } finally {
       await context.close();
@@ -150,15 +150,11 @@ test.describe('Model selection', () => {
   });
 
   test('reasoning effort toggle works for reasoning models', async ({ browser }) => {
-    const { page, context, sentMessages } = await openAuthenticatedChat(browser, {
+    const { page, context } = await openAuthenticatedChat(browser, {
       onMessage(msg, ws) {
         if (msg.type === 'set_model' && msg.model === 'o3') {
           ws.send(JSON.stringify({ type: 'model_changed', model: 'o3' }));
         }
-
-        if (msg.type === 'new_session' && msg.model === 'o3' && msg.reasoningEffort === 'high') {
-          ws.send(JSON.stringify({ type: 'session_created', model: 'o3' }));
-        }
       },
     });
 
@@ -178,13 +174,9 @@ test.describe('Model selection', () => {
 
       await highButton.click();
 
-      await expectSentMessage(
-        sentMessages,
-        (msg) =>
-          (msg.type === 'new_session' && msg.model === 'o3' && msg.reasoningEffort === 'high') ||
-          (msg.type === 'set_reasoning' && msg.effort === 'high') ||
-          (msg.type === 'set_reasoning_effort' && msg.effort === 'high'),
-      );
+      // Changing reasoning effort is now a client-side preference applied to the
+      // next new session (it intentionally does NOT restart the current session,
+      // which would wipe chat history), so no WS message is emitted on click.
       await expect(highButton).toHaveClass(/active/);
       await expect(mediumButton).not.toHaveClass(/active/);
     } finally {