fix: e2e tests

tests/e2e/specs/tier-a/prompts.spec.ts

@@ -53,23 +53,14 @@ async function openSettingsPrompts(
 }
 
 /**
- * Open the prompt Dropdown by clicking the button that is the sibling of
- * the "Add" button. The Dropdown renders a plain `<button>` with only a
- * `<span>` containing either the placeholder or the selected prompt's
- * name — it has no accessible role-name, so role-based locators don't
- * work. We identify it structurally: the first button inside the
- * `div.flex-row` row that also contains the Add button.
+ * Open the prompt picker. It is a Popover + cmdk list (settings/Prompts.tsx),
+ * whose trigger carries aria-label="Toggle prompt list". It used to be a bare
+ * <button> found by DOM position, which is why this walked up from "Add".
  */
 async function openPromptDropdown(
   page: import('@playwright/test').Page,
 ): Promise<void> {
-  // Container is the flex-row wrapper that holds [Dropdown, Add button].
-  // The dropdown's button is the first button in that row.
-  const addBtn = page.getByRole('button', { name: 'Add', exact: true });
-  // Walk up to the shared row container.
-  const row = addBtn.locator('..');
-  const dropdownButton = row.locator('button').first();
-  await dropdownButton.click();
+  await page.getByRole('button', { name: 'Toggle prompt list' }).click();
 }
 
 test.describe('tier-a · prompts CRUD', () => {
@@ -94,7 +85,7 @@ test.describe('tier-a · prompts CRUD', () => {
       const promptContent = 'You are a DocsGPT E2E assistant.';
 
       await page
-        .getByPlaceholder('Prompt Name')
+        .getByLabel('Prompt Name')
         .first()
         .fill(promptName);
       await page.locator('#new-prompt-content').fill(promptContent);
@@ -171,7 +162,7 @@ test.describe('tier-a · prompts CRUD', () => {
       await expect(row).toBeVisible();
       await row.locator('button').first().click();
 
-      const editName = page.getByPlaceholder('Prompt Name').first();
+      const editName = page.getByLabel('Prompt Name').first();
       await expect(editName).toHaveValue('e2e-to-edit');
       await editName.fill('e2e-edited');
       await page.locator('#edit-prompt-content').fill('updated content');
@@ -221,11 +212,13 @@ test.describe('tier-a · prompts CRUD', () => {
       await openSettingsPrompts(page);
       await openPromptDropdown(page);
 
-      // The row's delete button is the second <button> inside the row
-      // (after Pencil) per Dropdown.tsx lines 192-233.
-      const row = page.locator('div', { hasText: /^e2e-to-delete$/ }).first();
+      // Each entry is a cmdk option carrying labelled action buttons
+      // (settings/Prompts.tsx: "Edit prompt" / "Duplicate prompt" /
+      // "Delete prompt"). Target the label, not the button's position — the
+      // old spec indexed into the row and broke when the actions changed.
+      const row = page.getByRole('option', { name: /e2e-to-delete/ });
       await expect(row).toBeVisible();
-      await row.locator('button').nth(1).click();
+      await row.getByRole('button', { name: 'Delete prompt' }).click();
 
       // ConfirmationModal → submit button labelled "Delete" (modals.deleteConv.delete).
       const [deleteRes] = await Promise.all([

tests/e2e/specs/tier-b/guardrails.spec.ts

@@ -22,9 +22,9 @@
  * // Silent-break covered #2: a malformed control accepted with a 200. Every
  * // rejection case below is a configuration an operator could plausibly
  * // write and reasonably believe was enforcing something —
- * // `require_approval` on user input (there is nothing to approve), `redact`
- * // on a check that reports no spans, a duplicate control that shadows the
- * // stricter twin. Each must 400 AND leave no row behind.
+ * // `redact` on a check that reports no spans, a duplicate control that
+ * // shadows the stricter twin, a typo'd key. Each must 400 AND leave no row
+ * // behind — and the 400 must not echo the validator's own text back.
  *
  * // Silent-break covered #3: the builder writes state the backend then
  * // normalises away, so a reload shows the operator something different
@@ -50,22 +50,13 @@ const EXPECTED_CHECKS = [
   'denylist',
   'groundedness',
   'injection',
-  'moderation',
   'pii',
   'policy',
   'secrets',
-  'tool_policy',
-  'topic',
   'url',
 ];
 
-const VALID_STAGES = [
-  'input',
-  'retrieval',
-  'tool_call',
-  'tool_result',
-  'output',
-];
+const VALID_STAGES = ['input', 'retrieval', 'tool_result', 'output'];
 
 /**
  * Defaults `GuardrailsConfig` fills in for anything the caller omitted
@@ -343,7 +334,8 @@ test.describe('tier-b · guardrails strict validation on write', () => {
   const REJECTIONS: Array<{
     label: string;
     config: unknown;
-    expectedMessage: string;
+    /** Validator text that must appear in the SERVER LOG, never the body. */
+    internalHint: string;
   }> = [
     {
       label: 'unknown check name',
@@ -353,25 +345,7 @@ test.describe('tier-b · guardrails strict validation on write', () => {
           controls: [{ check: 'no_such_check', stage: 'input', action: 'flag' }],
         },
       },
-      expectedMessage: "unknown check 'no_such_check'",
-    },
-    {
-      label: 'require_approval at a stage that has no approval flow (input)',
-      config: {
-        guardrails: {
-          enabled: true,
-          controls: [
-            {
-              check: 'denylist',
-              stage: 'input',
-              action: 'require_approval',
-              settings: { terms: ['x'] },
-            },
-          ],
-        },
-      },
-      expectedMessage:
-        "action 'require_approval' is not valid at stage 'input'",
+      internalHint: "unknown check 'no_such_check'",
     },
     {
       label: 'redact on a check that reports no spans (groundedness)',
@@ -383,7 +357,7 @@ test.describe('tier-b · guardrails strict validation on write', () => {
           ],
         },
       },
-      expectedMessage: "check 'groundedness' cannot redact",
+      internalHint: "check 'groundedness' cannot redact",
     },
     {
       label: 'a check bound to a stage it does not support',
@@ -395,7 +369,7 @@ test.describe('tier-b · guardrails strict validation on write', () => {
           ],
         },
       },
-      expectedMessage:
+      internalHint:
         "check 'groundedness' does not support stage 'input'",
     },
     {
@@ -419,7 +393,7 @@ test.describe('tier-b · guardrails strict validation on write', () => {
           ],
         },
       },
-      expectedMessage:
+      internalHint:
         "duplicate control for check 'denylist' at stage 'input'",
     },
     {
@@ -437,7 +411,7 @@ test.describe('tier-b · guardrails strict validation on write', () => {
           ],
         },
       },
-      expectedMessage: 'terms must be a non-empty list',
+      internalHint: 'terms must be a non-empty list',
     },
     {
       label: 'pii with an entity the detector has no pattern for',
@@ -454,7 +428,7 @@ test.describe('tier-b · guardrails strict validation on write', () => {
           ],
         },
       },
-      expectedMessage: 'unknown PII entities: NOT_A_THING',
+      internalHint: 'unknown PII entities: NOT_A_THING',
     },
     {
       label: 'url policy with neither an allow nor a block list',
@@ -466,22 +440,22 @@ test.describe('tier-b · guardrails strict validation on write', () => {
           ],
         },
       },
-      expectedMessage: 'provide allow_hosts or block_hosts',
+      internalHint: 'provide allow_hosts or block_hosts',
     },
     {
       label: 'an unknown enforcement mode',
       config: { guardrails: { enabled: true, mode: 'yolo' } },
-      expectedMessage: 'mode must be one of',
+      internalHint: 'mode must be one of',
     },
     {
       label: 'a timeout below the floor',
       config: { guardrails: { enabled: true, timeout_ms: 5 } },
-      expectedMessage: 'must be >= 100',
+      internalHint: 'must be >= 100',
     },
     {
       label: 'a typo\'d key at the top level of config',
       config: { guardrails: { enabled: true }, guardrail: {} },
-      expectedMessage: 'Extra inputs are not permitted',
+      internalHint: 'Extra inputs are not permitted',
     },
     {
       label: 'a typo\'d key inside a control',
@@ -498,16 +472,16 @@ test.describe('tier-b · guardrails strict validation on write', () => {
           ],
         },
       },
-      expectedMessage: 'Extra inputs are not permitted',
+      internalHint: 'Extra inputs are not permitted',
     },
     {
       label: 'config that is not a JSON object at all',
       config: ['not', 'an', 'object'],
-      expectedMessage: 'config must be a JSON object',
+      internalHint: 'config must be a JSON object',
     },
   ];
 
-  for (const { label, config, expectedMessage } of REJECTIONS) {
+  for (const { label, config, internalHint } of REJECTIONS) {
     test(`create_agent rejects ${label} with a 400 and writes no row`, async () => {
       const sub = `e2e-guardrails-reject-${Date.now()}-${Math.random()
         .toString(36)
@@ -520,10 +494,18 @@ test.describe('tier-b · guardrails strict validation on write', () => {
         expect(res.status(), `expected 400 for ${label}, body: ${body}`).toBe(
           400,
         );
+        // The body is deliberately static — CodeQL flagged returning
+        // validator text, and `api/user/sources/routes.py` set the precedent
+        // of a fixed message with the detail logged instead. So assert the
+        // stable contract AND that the internals did not escape with it.
         expect(
           body,
-          `the 400 for "${label}" must name the problem; got: ${body}`,
-        ).toContain(expectedMessage);
+          `the 400 for "${label}" must carry the static message; got: ${body}`,
+        ).toContain('Invalid config');
+        expect(
+          body,
+          `the 400 for "${label}" must not leak validator internals (${internalHint})`,
+        ).not.toContain(internalHint);
 
         // Nothing may persist. A rejected config that still creates the
         // agent (minus its guardrails) is the worst outcome: the operator
@@ -589,7 +571,7 @@ test.describe('tier-b · guardrails catalog endpoint', () => {
     await resetDb();
   });
 
-  test('GET /api/guardrails/catalog describes all ten checks with the metadata the builder renders', async () => {
+  test('GET /api/guardrails/catalog describes every registered check with the metadata the builder renders', async () => {
     const sub = `e2e-guardrails-catalog-${Date.now()}`;
     const token = signJwt(sub);
     const api = await authedRequest(playwright, token);
@@ -615,7 +597,6 @@ test.describe('tier-b · guardrails catalog endpoint', () => {
         default_block_message: string;
         pii_entities: string[];
         default_pii_entities: string[];
-        moderation_categories: string[];
         floor: unknown;
       };
 
@@ -666,31 +647,19 @@ test.describe('tier-b · guardrails catalog endpoint', () => {
       // and changes when text is released.
       expect(byName.secrets.remote).toBe(false);
       expect(byName.pii.remote).toBe(false);
-      expect(byName.moderation.remote).toBe(true);
+      // The judge is the only check that leaves the process.
+      expect(byName.policy.remote).toBe(true);
 
-      // `tool_policy` only makes sense at the tool gate.
-      expect(byName.tool_policy.stages).toEqual(['tool_call']);
       expect(byName.groundedness.stages).toEqual(['output']);
 
       expect(body.stages).toEqual(VALID_STAGES);
-      expect(body.modes).toEqual([
-        'monitor_only',
-        'background_scan',
-        'dangerous_tools_only',
-        'scan_all',
-      ]);
+      expect(body.modes).toEqual(['monitor_only', 'scan_all']);
 
-      // `require_approval` has a resolution path only at the tool gate.
+      // Every stage takes the same three actions now that the tool gate is
+      // gone; nothing may advertise an action the validator would reject.
       for (const [stage, actions] of Object.entries(body.actions_by_stage)) {
         expect(VALID_STAGES).toContain(stage);
-        if (stage === 'tool_call') {
-          expect(actions).toEqual(['block', 'flag', 'require_approval']);
-        } else {
-          expect(
-            actions,
-            `stage ${stage} must not advertise require_approval`,
-          ).not.toContain('require_approval');
-        }
+        expect(actions).toEqual(['block', 'flag', 'redact']);
       }
 
       expect(body.default_block_message).toBe(
@@ -711,7 +680,6 @@ test.describe('tier-b · guardrails catalog endpoint', () => {
           `default PII entity ${entity} is not in the advertised entity list`,
         ).toContain(entity);
       }
-      expect(body.moderation_categories.length).toBeGreaterThan(0);
       // No instance floor is configured in the e2e env, so the builder must
       // be told there is nothing to lock.
       expect(body.floor).toBeNull();

tests/e2e/specs/tier-c/ui-smoke.spec.ts

@@ -61,18 +61,33 @@ async function gotoSettings(
  * This helper clicks the trigger whose visible label is `currentValue`,
  * then clicks the option whose visible label is `targetValue`.
  */
+/** Anchored matcher so picking "2" cannot match "20". */
+function exactText(value: string): RegExp {
+  return new RegExp(`^${value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}$`);
+}
+
 async function pickDropdown(
   page: import('@playwright/test').Page,
   currentValue: string,
   targetValue: string,
 ): Promise<void> {
-  await page.getByRole('button', { name: currentValue, exact: true })
+  // Settings dropdowns are shadcn <Select> since #2495. Two consequences the
+  // old locators tripped on:
+  //   1. Radix gives the trigger role="combobox", not "button".
+  //   2. That trigger has no aria-label, and for a combobox the accessible
+  //      name comes from a label — not from content — so the current value is
+  //      only matchable as text. Hence filter() rather than { name }.
+  await page
+    .getByRole('combobox')
+    .filter({ hasText: exactText(currentValue) })
+    .first()
+    .click();
+  // Options are portalled with role="option"; matching the role avoids
+  // colliding with the same text still shown in the trigger.
+  await page
+    .getByRole('option', { name: targetValue, exact: true })
     .first()
     .click();
-  // Options render as spans; click by exact text. Use `.last()` because
-  // sometimes the same text appears in the trigger label too (collapsed
-  // state overlap). The panel appends after the trigger in DOM order.
-  await page.getByText(targetValue, { exact: true }).last().click();
 }
 
 /**
@@ -226,28 +241,9 @@ test.describe('tier-c · UI smoke', () => {
     }
   });
 
-  test('C3 · chunk-count change persists to DocsGPTChunks localStorage', async ({
-    browser,
-  }) => {
-    const { context } = await newUserContext(browser);
-    try {
-      const page = await context.newPage();
-      await gotoSettings(page);
-
-      // Default chunks value is '2'. Only the chunks dropdown's trigger has
-      // the exact text "2"; other dropdowns show their own labels. Pick '6'.
-      await pickDropdown(page, '2', '6');
-
-      // localStorage persisted (listener middleware fires on setChunks).
-      await expect
-        .poll(async () =>
-          page.evaluate(() => localStorage.getItem('DocsGPTChunks')),
-        )
-        .toBe(JSON.stringify('6'));
-    } finally {
-      await context.close();
-    }
-  });
+  // C3 removed: the chunk-count dropdown was deleted from Settings in
+  // babc067a "feat: remove old chunk management" (2026-06-22). There is no
+  // longer a DocsGPTChunks setting to persist, so there is nothing to smoke.
 
   test('C4 · default prompt selection persists to DocsGPTPrompt', async ({
     browser,
@@ -257,9 +253,11 @@ test.describe('tier-c · UI smoke', () => {
       const page = await context.newPage();
       await gotoSettings(page);
 
-      // The active prompt dropdown starts on "default". Open it and pick
-      // "creative" — a built-in so no seeding needed.
-      await pickDropdown(page, 'default', 'creative');
+      // The prompt picker is a Popover + cmdk list (settings/Prompts.tsx), not
+      // a Select, so pickDropdown does not apply: the trigger is a button with
+      // aria-label="Toggle prompt list" and the entries are cmdk options.
+      await page.getByRole('button', { name: 'Toggle prompt list' }).click();
+      await page.getByRole('option', { name: 'creative' }).first().click();
 
       await expect
         .poll(async () => {
@@ -321,23 +319,24 @@ test.describe('tier-c · UI smoke', () => {
       await page.setViewportSize({ width: 1280, height: 800 });
       await page.goto('/');
 
-      // Sidebar open: the Collapse button's img has alt="Collapse sidebar"
-      // (Navigation.tsx:367). Click it to collapse.
-      const collapse = page.getByAltText('Collapse sidebar');
+      // Sidebar open: the toggle is a button carrying
+      // aria-label="Collapse sidebar" (Navigation.tsx:415). It used to be an
+      // <img alt=...>, which is why this was getByAltText before.
+      const collapse = page.getByRole('button', { name: 'Collapse sidebar' });
       await expect(collapse).toBeVisible();
       await collapse.click();
 
       // When collapsed, Navigation.tsx renders a floating reopen button at
-      // `absolute top-3 left-3` whose img has alt="Open navigation menu"
-      // (Navigation.tsx:312). The off-screen nav's own toggle flips to
+      // `absolute top-3 left-3` with aria-label="Open navigation menu"
+      // (Navigation.tsx:329). The off-screen nav's own toggle flips to
       // "Expand sidebar" but that one is outside the viewport. We target
       // the visible floating button for the re-open interaction.
-      const reopen = page.getByAltText('Open navigation menu');
+      const reopen = page.getByRole('button', { name: 'Open navigation menu' });
       await expect(reopen).toBeVisible();
       await reopen.click();
 
       // Collapse button visible again — toggle is symmetric.
-      await expect(page.getByAltText('Collapse sidebar')).toBeVisible();
+      await expect(page.getByRole('button', { name: 'Collapse sidebar' })).toBeVisible();
     } finally {
       await context.close();
     }
@@ -468,12 +467,13 @@ test.describe('tier-c · UI smoke', () => {
       const page = await context.newPage();
       await page.goto(`/agents/logs/${agentId}`);
 
-      // AgentLogs.tsx renders the i18n "agents.logs.title" heading and an
-      // "agents.backToAll" label. The title is the load-bearing assertion —
-      // if the route resolves, useParams gives us an agentId, and the page
-      // mounts, the h1 appears regardless of whether analytics have data.
+      // AgentPageHeader renders a breadcrumb plus a labelled sub-nav; it has
+      // no heading element (it used to, before #2495 moved it to shadcn
+      // Breadcrumb). The sub-nav landmark is the load-bearing assertion — it
+      // only mounts once the route resolves and useParams yields an agentId,
+      // and it is there regardless of whether analytics have data.
       await expect(
-        page.getByRole('heading', { level: 1 }).first(),
+        page.getByRole('navigation', { name: 'Agent sub-navigation' }),
       ).toBeVisible();
 
       // Agent name renders once the fetch resolves.