From 26f1aeaa9d8265e327c3c1dba1daa2ce2b49b290 Mon Sep 17 00:00:00 2001 From: Marcel Date: Wed, 20 May 2026 00:00:53 +0200 Subject: [PATCH] fix(transcription): clip @mention displayName to MAX_QUERY_LENGTH MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dropdown's editor-mirror clips at 100 chars (CWE-400, Nora #1), but the host editor previously fed renderProps.query directly to displayName on selection — so a 200-char @-suffix would search the first 100 chars but insert 200 chars. Clip once in updateState and use the clipped value for both the inserted displayName and the dropdown's editorQuery mirror, keeping "what I searched" and "what got inserted" in sync. Felix #3 on PR #629. Co-Authored-By: Claude Opus 4.7 --- .../discussion/PersonMentionEditor.svelte | 14 +++++++-- .../PersonMentionEditor.svelte.spec.ts | 30 +++++++++++++++++++ 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte b/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte index 8714a574..466c00ed 100644 --- a/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte +++ b/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte @@ -9,7 +9,7 @@ import type { PersonMention } from '$lib/shared/types'; import { deserialize, serialize } from '$lib/shared/discussion/mentionSerializer'; import { debounce } from '$lib/shared/utils/debounce'; import MentionDropdown from './MentionDropdown.svelte'; -import { SEARCH_DEBOUNCE_MS, SEARCH_RESULT_LIMIT } from './mentionConstants'; +import { MAX_QUERY_LENGTH, SEARCH_DEBOUNCE_MS, SEARCH_RESULT_LIMIT } from './mentionConstants'; type Person = components['schemas']['Person']; @@ -225,14 +225,22 @@ onMount(() => { }; const updateState = (renderProps: LooseRenderProps) => { + // Clip once here so both the inserted displayName and the + // dropdown's editor-mirror see the same value. The dropdown + // already clips the mirror (Nora #1 CWE-400), but without + // clipping at the command boundary an unclipped query would + // still flow through as the inserted displayName — visible + // UI divergence between "what I searched" and "what was + // inserted". Felix #3 on PR #629. + const clippedQuery = renderProps.query.slice(0, MAX_QUERY_LENGTH); // AC-1: pass typed query as displayName, not person.displayName dropdownState.command = (item: Person) => renderProps.command({ personId: item.id, - displayName: renderProps.query + displayName: clippedQuery }); dropdownState.clientRect = renderProps.clientRect ?? null; - dropdownState.editorQuery = renderProps.query; + dropdownState.editorQuery = clippedQuery; }; return { diff --git a/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte.spec.ts b/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte.spec.ts index 591a7ef8..0620ecfb 100644 --- a/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte.spec.ts +++ b/frontend/src/lib/shared/discussion/PersonMentionEditor.svelte.spec.ts @@ -473,6 +473,36 @@ describe('PersonMentionEditor — AC-1: typed text as displayName', () => { }); }); + it('clips the inserted displayName to MAX_QUERY_LENGTH=100 chars (Felix #3 on PR #629)', async () => { + // CWE-400 amplification: the dropdown clips its search input + mirror at + // 100 chars (Nora #1), but the host editor was passing the unclipped + // renderProps.query straight through to displayName — so a 105-char + // @-suffix in the editor could insert a 105-char displayName into the + // sidecar even though the dropdown only searched the first 100. + mockFetchWithPersons(); + const host = renderHost(); + + // Type @ + 105 'A' chars in the contenteditable. The renderProps.query + // fed into the command callback derives from the editor text after `@`, + // not the dropdown's searchbox — so we must drive the editor. + await userEvent.type(page.getByRole('textbox'), '@' + 'A'.repeat(105)); + + // The mocked /api/persons returns AUGUSTE for any query — wait for it. + await vi.waitFor(async () => { + await expect.element(page.getByRole('option', { name: /Auguste Raddatz/ })).toBeVisible(); + }); + + const option = (await page + .getByRole('option', { name: /Auguste Raddatz/ }) + .element()) as HTMLElement; + option.dispatchEvent(new MouseEvent('mousedown', { bubbles: true, cancelable: true })); + + await vi.waitFor(() => { + expect(host.snapshot.mentionedPersons).toHaveLength(1); + expect(host.snapshot.mentionedPersons[0].displayName.length).toBeLessThanOrEqual(100); + }); + }); + it('does not duplicate the sidecar entry when the same person is selected twice', async () => { mockFetchWithPersons(); const host = renderHost({