From 43571601aa71fbd30e839f567e540106a92d8a03 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Mon, 27 Jul 2026 17:50:18 -0500 Subject: [PATCH] fix(desktop): don't let Enter swap a free-text slash argument for a completion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `/goal` keeps its completion popover open across arbitrary prose so its subcommands stay reachable. The popover highlights its first row on open, and Enter accepted that highlight unconditionally — so pressing Enter to send `/goal ship the redesign` would replace the sentence with a row the user never chose. Space was already guarded; Enter and Tab were not. Enter now accepts only after the user has arrowed to a row deliberately, so the highlight never lies about what Enter will do. Tab stays an unconditional accept, since it means nothing else in the composer. This is latent rather than reproducible today: `/goal` is absent from `SUBCOMMANDS` (its `args_hint` pipes are spaced, so the extraction regex misses them), so the backend returns no arg completions and the branch never runs. Giving `/goal` the subcommands it already advertises would resurrect the #71963 symptom in a worse form — losing the prose instead of chipping it. --- .../hooks/use-composer-trigger.test.ts | 24 ++++++++++++++ .../composer/hooks/use-composer-trigger.ts | 26 ++++++++++++--- apps/desktop/src/app/chat/composer/index.tsx | 32 ++++++++++++------- 3 files changed, 66 insertions(+), 16 deletions(-) diff --git a/apps/desktop/src/app/chat/composer/hooks/use-composer-trigger.test.ts b/apps/desktop/src/app/chat/composer/hooks/use-composer-trigger.test.ts index c7a4e2f138a..a4ab38b3b3a 100644 --- a/apps/desktop/src/app/chat/composer/hooks/use-composer-trigger.test.ts +++ b/apps/desktop/src/app/chat/composer/hooks/use-composer-trigger.test.ts @@ -148,6 +148,30 @@ describe('useComposerTrigger — free-text slash arguments', () => { expect(editor.querySelector('[data-slash-kind]')).toBeNull() }) + it('treats the default highlight as a suggestion until the user arrows to a row', () => { + const editor = mountEditor('/goal stat') + const { hook } = mountTrigger(editor, [item('/goal status', 'Options')]) + + act(() => hook.result.current.refreshTrigger()) + expect(hook.result.current.triggerActiveExplicit).toBe(false) + + act(() => hook.result.current.moveTriggerActive(1)) + expect(hook.result.current.triggerActiveExplicit).toBe(true) + }) + + it('drops a deliberate selection once the query moves on', () => { + const editor = mountEditor('/goal stat') + const { hook } = mountTrigger(editor, [item('/goal status', 'Options')]) + + act(() => hook.result.current.refreshTrigger()) + act(() => hook.result.current.moveTriggerActive(1)) + + renderComposerContents(editor, '/goal start the migration') + act(() => hook.result.current.refreshTrigger()) + + expect(hook.result.current.triggerActiveExplicit).toBe(false) + }) + it('still commits a fully typed finite option as one directive chip', () => { const editor = mountEditor('/personality creative') const { hook } = mountTrigger(editor, []) diff --git a/apps/desktop/src/app/chat/composer/hooks/use-composer-trigger.ts b/apps/desktop/src/app/chat/composer/hooks/use-composer-trigger.ts index 5f0be41c80c..65f09225f65 100644 --- a/apps/desktop/src/app/chat/composer/hooks/use-composer-trigger.ts +++ b/apps/desktop/src/app/chat/composer/hooks/use-composer-trigger.ts @@ -53,6 +53,11 @@ export function useComposerTrigger({ }: UseComposerTriggerOptions) { const [trigger, setTrigger] = useState(null) const [triggerActive, setTriggerActive] = useState(0) + // The list highlights its first row on open, which is a suggestion rather + // than a choice. This records that the user moved the highlight themselves, + // which is what lets Enter accept a completion in a free-text argument stage + // without stealing prose from everyone who never touched the arrows. + const [triggerActiveExplicit, setTriggerActiveExplicit] = useState(false) const [triggerItems, setTriggerItems] = useState([]) // Set synchronously in keydown when the open trigger popover consumes a // navigation/control key (Arrow/Enter/Tab/Escape). The subsequent keyup must @@ -63,6 +68,11 @@ export function useComposerTrigger({ // re-rendered and the handler closure sees the post-keydown state. const triggerKeyConsumedRef = useRef(false) + const resetTriggerActive = useCallback(() => { + setTriggerActive(0) + setTriggerActiveExplicit(false) + }, []) + const refreshTrigger = useCallback(() => { const editor = editorRef.current @@ -80,7 +90,7 @@ export function useComposerTrigger({ if (!rawText.includes('@') && !rawText.includes('/')) { if (trigger) { setTrigger(null) - setTriggerActive(0) + resetTriggerActive() } return @@ -109,9 +119,9 @@ export function useComposerTrigger({ // caret move (mouseup) or a stray refresh — must preserve the user's // current selection instead of snapping back to the first item. if (detected?.kind !== trigger?.kind || detected?.query !== trigger?.query) { - setTriggerActive(0) + resetTriggerActive() } - }, [editorRef, trigger]) + }, [editorRef, resetTriggerActive, trigger]) const triggerAdapter: Unstable_TriggerAdapter | null = trigger?.kind === '@' ? at.adapter : trigger?.kind === '/' ? slash.adapter : null @@ -147,7 +157,13 @@ export function useComposerTrigger({ const closeTrigger = () => { setTrigger(null) setTriggerItems([]) - setTriggerActive(0) + resetTriggerActive() + } + + /** Step the highlight, marking it as the user's own deliberate pick. */ + const moveTriggerActive = (delta: number) => { + setTriggerActiveExplicit(true) + setTriggerActive(idx => (idx + delta + triggerItems.length) % triggerItems.length) } useEffect(() => { @@ -358,12 +374,14 @@ export function useComposerTrigger({ ascendTriggerPath, closeTrigger, commitTypedSlashDirective, + moveTriggerActive, refreshTrigger, replaceTriggerWithChip, setTriggerActive, slashFreeTextArgStage, trigger, triggerActive, + triggerActiveExplicit, triggerItems, triggerKeyConsumedRef, triggerLoading diff --git a/apps/desktop/src/app/chat/composer/index.tsx b/apps/desktop/src/app/chat/composer/index.tsx index 6ce8289fa0a..ac4f5dd3027 100644 --- a/apps/desktop/src/app/chat/composer/index.tsx +++ b/apps/desktop/src/app/chat/composer/index.tsx @@ -22,7 +22,12 @@ import { $autoSpeakReplies } from '@/store/voice-prefs' import { useTheme } from '@/themes' import { AttachmentList } from './attachments' -import { COMPOSER_FADE_BACKGROUND, type QueueEditState, slashArgStage } from './composer-utils' +import { + acceptsTriggerCompletion, + COMPOSER_FADE_BACKGROUND, + type QueueEditState, + slashArgStage +} from './composer-utils' import { ContextMenu } from './context-menu' import { COMPOSER_AREAS, runComposerMiddleware } from './contrib' import { ComposerControls } from './controls' @@ -302,12 +307,14 @@ export function ChatBar({ ascendTriggerPath, closeTrigger, commitTypedSlashDirective, + moveTriggerActive, refreshTrigger, replaceTriggerWithChip, setTriggerActive, slashFreeTextArgStage, trigger, triggerActive, + triggerActiveExplicit, triggerItems, triggerKeyConsumedRef, triggerLoading @@ -528,7 +535,7 @@ export function ChatBar({ if (event.key === 'ArrowDown') { event.preventDefault() triggerKeyConsumedRef.current = true - setTriggerActive(idx => (idx + 1) % triggerItems.length) + moveTriggerActive(1) return } @@ -536,20 +543,21 @@ export function ChatBar({ if (event.key === 'ArrowUp') { event.preventDefault() triggerKeyConsumedRef.current = true - setTriggerActive(idx => (idx - 1 + triggerItems.length) % triggerItems.length) + moveTriggerActive(-1) return } - // Enter / Tab / Space all accept the highlighted item: a no-arg command - // commits its directive chip, an arg-taking command expands to its - // options step, and an arg option commits the full `/cmd arg` chip. Space - // is slash-only (an `@` mention takes a literal space) and gated to a - // non-empty query so a bare `/ ` still types a space. - const acceptOnSpace = - event.key === ' ' && trigger.kind === '/' && Boolean(trigger.query.trim()) && !slashFreeTextArgStage - - const accept = event.key === 'Enter' || event.key === 'Tab' || acceptOnSpace + // Accepting the highlighted item: a no-arg command commits its directive + // chip, an arg-taking command expands to its options step, and an arg + // option commits the full `/cmd arg` chip. + const accept = acceptsTriggerCompletion({ + activeExplicit: triggerActiveExplicit, + freeTextArgStage: slashFreeTextArgStage, + key: event.key, + kind: trigger.kind, + query: trigger.query + }) if (accept) { event.preventDefault()