diff --git a/frontend/e2e/helpers/pdf.ts b/frontend/e2e/helpers/pdf.ts index 635c4c66..a1560df4 100644 --- a/frontend/e2e/helpers/pdf.ts +++ b/frontend/e2e/helpers/pdf.ts @@ -45,8 +45,9 @@ const inflateStreams = (pdf: Buffer): Buffer[] => { /** * Structural read of a generated PDF. It deliberately does not decode the text: each font subset - * carries its own glyph map, so a merged decode garbles the result. What the report's text says is - * pinned by the markdown export, which is asserted byte for byte from the same source string. + * carries its own glyph map, so a merged decode garbles the result. The report's wording is checked + * on the markdown export instead, which is built from the same string and whose bytes are searched + * for the task and result text. */ export const inspectPdf = (pdf: Buffer): PdfShape => { const head = pdf.subarray(0, 5).toString(); diff --git a/frontend/e2e/mocks/cassettes/settings-providers.ts b/frontend/e2e/mocks/cassettes/settings-providers.ts index 815501fe..0eb56049 100644 --- a/frontend/e2e/mocks/cassettes/settings-providers.ts +++ b/frontend/e2e/mocks/cassettes/settings-providers.ts @@ -187,13 +187,18 @@ export const providersList = (...userDefined: ProviderConfigFragmentFragment[]) }), }); -/** A second row so a delete spec can prove the table survived rather than merely emptied. */ +/** A second row so a delete spec can operate a row that is not index 0, and prove the table survived. */ +export const OTHER_PROVIDER = { + id: 'custom-2', + name: 'Second Endpoint', +}; + export const OTHER_PROVIDER_ROW = (): ProviderConfigFragmentFragment => entity('ProviderConfig', { agents: agentsConfig(), createdAt: T, - id: 'custom-2', - name: 'Second Endpoint', + id: OTHER_PROVIDER.id, + name: OTHER_PROVIDER.name, type: ProviderType.Custom, updatedAt: T, }); diff --git a/frontend/e2e/specs/cross/a11y.spec.ts b/frontend/e2e/specs/cross/a11y.spec.ts index 1a3fbdad..fb2539bd 100644 --- a/frontend/e2e/specs/cross/a11y.spec.ts +++ b/frontend/e2e/specs/cross/a11y.spec.ts @@ -1,7 +1,7 @@ import { expect, test } from '../../fixtures/test.ts'; import { scanA11y, waiversForScan } from '../../helpers/a11y.ts'; import { expectCleanPage } from '../../helpers/errors.ts'; -import { resourcesCassette } from '../../mocks/cassettes/resources.ts'; +import { FILE_RESOURCE, resourcesCassette } from '../../mocks/cassettes/resources.ts'; import { populatedSettingsProvidersCassette } from '../../mocks/cassettes/settings-providers.ts'; import { loginJourneyCassette } from '../../mocks/cassettes/smoke.ts'; import { ROUTE_MANIFEST } from '../../routes.ts'; @@ -93,4 +93,23 @@ test.describe('dialog keyboard contract', { tag: '@cross' }, () => { await expect(opener).toBeFocused(); expectCleanPage(pageErrorLog); }); + + // The other dialog family: ConfirmationDialog renders its DialogContent unconditionally, so a + // focus hook living on the wrapper would capture whatever had focus when the PAGE mounted and + // restore it on navigation instead. This case is what tells the two apart. + test('Escape returns focus for a dialog whose content is always mounted', async ({ page, pageErrorLog }) => { + await page.goto('/resources'); + await page.getByRole('checkbox', { name: `Select ${FILE_RESOURCE.name}` }).click(); + + const opener = page.getByRole('button', { exact: true, name: 'Delete' }); + + await opener.click(); + await expect(page.getByRole('dialog')).toBeVisible(); + + await page.keyboard.press('Escape'); + await expect(page.getByRole('dialog')).toBeHidden(); + + await expect(opener).toBeFocused(); + expectCleanPage(pageErrorLog); + }); }); diff --git a/frontend/e2e/specs/real/resources-upload-limits.spec.ts b/frontend/e2e/specs/real/resources-upload-limits.spec.ts index efee6f93..67091806 100644 --- a/frontend/e2e/specs/real/resources-upload-limits.spec.ts +++ b/frontend/e2e/specs/real/resources-upload-limits.spec.ts @@ -35,7 +35,11 @@ test.describe('resources upload limits at the endpoint', { tag: '@real' }, () => const tooMany = await upload(request, many); - expect(tooMany.status(), 'one file past the 1000-file cap').toBe(400); + // 400, but not from the handler's own count check: Go's multipart reader caps a form at 1000 + // parts and answers `multipart: message too large` first (confirmed in the backend log), which + // makes `resources.MaxUploadFiles` unreachable for this endpoint. What this pins is the + // user-visible contract — 1001 is refused and nothing is written. + expect(tooMany.status(), 'a batch past 1000 files is refused').toBe(400); const afterReject = await request.get('/api/v1/resources/', { params: { recursive: 'true' } }); const rejectedPaths = ((await afterReject.json()).data.items ?? []).map((item: { path: string }) => item.path); @@ -51,9 +55,15 @@ test.describe('resources upload limits at the endpoint', { tag: '@real' }, () => expect(atCap.status(), 'a batch exactly on the cap').toBe(200); - await request.delete('/api/v1/resources/', { - params: Object.fromEntries(many.slice(0, 1000).map(({ name }) => ['paths[]', name])), - }); + // Repeated keys, not an object: `Object.fromEntries` keeps only the last of 1000 identical + // `paths[]` entries and would delete one file while leaving 999 on the stand. + const cleanup = new URLSearchParams(); + + many.slice(0, 1000).forEach(({ name }) => cleanup.append('paths[]', name)); + + const deleted = await request.delete(`/api/v1/resources/?${cleanup}`); + + expect(deleted.status(), 'the seeded batch is removed again').toBe(200); }); // The server sanitises rather than rejects, which is a legitimate choice — what must hold is that diff --git a/frontend/e2e/specs/settings/providers.spec.ts b/frontend/e2e/specs/settings/providers.spec.ts index a7cfdd57..5ab403ce 100644 --- a/frontend/e2e/specs/settings/providers.spec.ts +++ b/frontend/e2e/specs/settings/providers.spec.ts @@ -4,6 +4,7 @@ import { expect, test } from '../../fixtures/test.ts'; import { expectCleanPage } from '../../helpers/errors.ts'; import { agentTestResult, + OTHER_PROVIDER, OTHER_PROVIDER_ROW, populatedSettingsProvidersCassette, providersList, @@ -160,8 +161,6 @@ test.describe('settings provider edit paths', { tag: '@coverage' }, () => { expect(variables.providerId).toBe(SEEDED_PROVIDER.id); expect(variables.name).toBe(RENAMED); - // The mutation has no `type` argument, so an edit can never move a provider between types. - expect(variables.type).toBeUndefined(); expect(Object.keys(variables.agents), 'the whole agents map is resubmitted').toHaveLength(13); await expect(page, 'a saved provider returns to the list').toHaveURL(/\/settings\/providers$/); expectCleanPage(pageErrorLog); @@ -210,14 +209,14 @@ test.describe('settings provider edit paths', { tag: '@coverage' }, () => { { data: { deleteProvider: 'success' } as never, setFlag: 'provider-deleted', - variables: { providerId: SEEDED_PROVIDER.id }, + variables: { providerId: OTHER_PROVIDER.id }, }, ], }, queries: { settingsProviders: [ { data: providersList(seededProviderRow(), OTHER_PROVIDER_ROW()) }, - { data: providersList(OTHER_PROVIDER_ROW()), whenFlag: 'provider-deleted' }, + { data: providersList(seededProviderRow()), whenFlag: 'provider-deleted' }, ], }, }), @@ -226,7 +225,9 @@ test.describe('settings provider edit paths', { tag: '@coverage' }, () => { test('the row menu deletes the row it belongs to', async ({ page, pageErrorLog }) => { await page.goto('/settings/providers'); - const row = page.getByRole('row', { name: new RegExp(SEEDED_PROVIDER.name) }); + // The second row on purpose: a menu wired to a fixed index would still send the first + // row's id, and a spec that operated row one could not tell the two apart. + const row = page.getByRole('row', { name: new RegExp(OTHER_PROVIDER.name) }); await expect(row).toBeVisible(); await row.hover(); @@ -237,11 +238,11 @@ test.describe('settings provider edit paths', { tag: '@coverage' }, () => { await page.getByRole('dialog').getByRole('button', { name: 'Delete' }).click(); - expect((await request).postDataJSON().variables).toEqual({ providerId: SEEDED_PROVIDER.id }); + expect((await request).postDataJSON().variables).toEqual({ providerId: OTHER_PROVIDER.id }); // The row goes only because the refetch answers without it, and the sibling proves the // table itself survived — a vanished table would satisfy a bare toBeHidden() too. await expect(row).toBeHidden(); - await expect(page.getByRole('row', { name: /Second Endpoint/ })).toBeVisible(); + await expect(page.getByRole('row', { name: new RegExp(SEEDED_PROVIDER.name) })).toBeVisible(); expectCleanPage(pageErrorLog); }); }); diff --git a/frontend/src/components/ui/dialog.tsx b/frontend/src/components/ui/dialog.tsx index ef6dd1d3..aa32e74b 100644 --- a/frontend/src/components/ui/dialog.tsx +++ b/frontend/src/components/ui/dialog.tsx @@ -23,22 +23,6 @@ function DialogClose({ ...props }: React.ComponentProps) { - // Radix hands focus back to its own DialogTrigger, and almost every dialog here is a controlled - // `` with no trigger — and its content is unmounted by the page before Radix's - // close sequence runs at all, so `onCloseAutoFocus` never fires. Without this, closing a dialog - // drops focus on and a keyboard user restarts from the top of the page. Captured during - // the first render, because by the time effects run focus is already inside the dialog. - const [opener] = React.useState(() => document.activeElement as HTMLElement | null); - - React.useEffect( - () => () => { - if (opener?.isConnected) { - opener.focus(); - } - }, - [opener], - ); - return ( @@ -50,6 +34,10 @@ function DialogContent({ children, className, ...props }: React.ComponentProps + {/* Radix returns focus to its own DialogTrigger; these dialogs are opened from state, + and their content is often unmounted before Radix's close sequence runs, so its + `onCloseAutoFocus` never fires. */} + {children} ` unconditionally and let Radix decide whether it is on screen. + */ +function DialogFocusReturn() { + // Captured during this component's first render: by the time effects run, Radix has already + // moved focus inside the dialog. + const [opener] = React.useState(() => document.activeElement as HTMLElement | null); + + React.useEffect( + () => () => { + if (opener?.isConnected) { + opener.focus(); + } + }, + [opener], + ); + + return null; +} + function DialogFooter({ className, ...props }: React.ComponentProps<'div'>) { return (
{ form.reset({ search: '' }); setDebouncedSearchValue(''); diff --git a/frontend/src/features/flows/flow-form.tsx b/frontend/src/features/flows/flow-form.tsx index a0b1d555..16796ebf 100644 --- a/frontend/src/features/flows/flow-form.tsx +++ b/frontend/src/features/flows/flow-form.tsx @@ -339,6 +339,7 @@ export function FlowForm({ {templateSearch && ( { event.stopPropagation(); setTemplateSearch(''); @@ -393,6 +394,7 @@ export function FlowForm({ {resourceSearch && ( { event.stopPropagation(); setResourceSearch(''); @@ -577,6 +579,7 @@ export function FlowForm({ {providerSearch && ( { event.stopPropagation(); setProviderSearch(''); diff --git a/frontend/src/features/flows/messages/flow-assistant-messages.tsx b/frontend/src/features/flows/messages/flow-assistant-messages.tsx index 0cdfa59b..034cd598 100644 --- a/frontend/src/features/flows/messages/flow-assistant-messages.tsx +++ b/frontend/src/features/flows/messages/flow-assistant-messages.tsx @@ -526,6 +526,7 @@ function FlowAssistantMessages({ className }: FlowAssistantMessagesProps) { {field.value && ( { form.reset({ search: '' }); diff --git a/frontend/src/features/flows/messages/flow-automation-messages.tsx b/frontend/src/features/flows/messages/flow-automation-messages.tsx index 8469f0cc..a8723763 100644 --- a/frontend/src/features/flows/messages/flow-automation-messages.tsx +++ b/frontend/src/features/flows/messages/flow-automation-messages.tsx @@ -216,6 +216,7 @@ function FlowAutomationMessages({ className }: FlowAutomationMessagesProps) { {field.value && ( { form.reset({ search: '' }); setDebouncedSearchValue(''); diff --git a/frontend/src/features/flows/screenshots/flow-screenshots.tsx b/frontend/src/features/flows/screenshots/flow-screenshots.tsx index df81a101..f47866f1 100644 --- a/frontend/src/features/flows/screenshots/flow-screenshots.tsx +++ b/frontend/src/features/flows/screenshots/flow-screenshots.tsx @@ -99,6 +99,7 @@ function FlowScreenshots() { {field.value && ( { form.reset({ search: '' }); setDebouncedSearchValue(''); diff --git a/frontend/src/features/flows/tasks/flow-tasks.tsx b/frontend/src/features/flows/tasks/flow-tasks.tsx index ec0881f6..ffe0abae 100644 --- a/frontend/src/features/flows/tasks/flow-tasks.tsx +++ b/frontend/src/features/flows/tasks/flow-tasks.tsx @@ -115,6 +115,7 @@ function FlowTasks() { {field.value && ( { form.reset({ search: '' }); setDebouncedSearchValue(''); diff --git a/frontend/src/features/flows/terminal/flow-terminal.tsx b/frontend/src/features/flows/terminal/flow-terminal.tsx index af10bca2..c7909f9a 100644 --- a/frontend/src/features/flows/terminal/flow-terminal.tsx +++ b/frontend/src/features/flows/terminal/flow-terminal.tsx @@ -192,6 +192,7 @@ function FlowTerminal() { )} {field.value && ( { form.reset({ search: '' }); setDebouncedSearchValue(''); diff --git a/frontend/src/features/flows/vector-stores/flow-vector-stores.tsx b/frontend/src/features/flows/vector-stores/flow-vector-stores.tsx index 73ba19a5..b59fcfe0 100644 --- a/frontend/src/features/flows/vector-stores/flow-vector-stores.tsx +++ b/frontend/src/features/flows/vector-stores/flow-vector-stores.tsx @@ -159,6 +159,7 @@ function FlowVectorStores() { {field.value && ( { form.reset({ search: '' }); setDebouncedSearchValue('');