From e3acdfb21da9a06f9b326f001563cf41641787c4 Mon Sep 17 00:00:00 2001
From: Brooklyn Nicholson <brooklyn.bb.nicholson@gmail.com>
Date: Mon, 27 Jul 2026 17:50:12 -0500
Subject: [PATCH] refactor(desktop): lift the completion-accept decision out of
 the keydown ladder

Which keys accept the highlighted completion was an inline condition in the
composer's keydown god-function, untestable without a DOM harness. Move it to
a pure helper beside the other slash-query utilities.
---
 .../app/chat/composer/composer-utils.test.ts  | 51 ++++++++++++++++++-
 .../src/app/chat/composer/composer-utils.ts   | 44 ++++++++++++++++
 2 files changed, 94 insertions(+), 1 deletion(-)

diff --git a/apps/desktop/src/app/chat/composer/composer-utils.test.ts b/apps/desktop/src/app/chat/composer/composer-utils.test.ts
index 4df8463ba2deb..062b256773851 100644
--- a/apps/desktop/src/app/chat/composer/composer-utils.test.ts
+++ b/apps/desktop/src/app/chat/composer/composer-utils.test.ts
@@ -2,12 +2,14 @@ import type { Unstable_TriggerItem } from '@assistant-ui/core'
 import { describe, expect, it } from 'vitest'
 
 import {
+  acceptsTriggerCompletion,
   isPendingDraftPersistCurrent,
   type PendingDraftPersist,
   pickPlaceholder,
   slashArgStage,
   slashChipKindForItem,
-  slashCommandToken
+  slashCommandToken,
+  type TriggerAcceptInput
 } from './composer-utils'
 
 const item = (group: string): Unstable_TriggerItem =>
@@ -39,6 +41,53 @@ describe('slashChipKindForItem', () => {
   })
 })
 
+describe('acceptsTriggerCompletion', () => {
+  const press = (key: string, overrides: Partial<TriggerAcceptInput> = {}) =>
+    acceptsTriggerCompletion({
+      activeExplicit: false,
+      freeTextArgStage: false,
+      key,
+      kind: '/',
+      query: 'personality alic',
+      ...overrides
+    })
+
+  it('accepts on Enter / Tab / Space for a finite option list', () => {
+    expect(press('Enter')).toBe(true)
+    expect(press('Tab')).toBe(true)
+    expect(press(' ')).toBe(true)
+  })
+
+  it('ignores keys that are neither navigation nor acceptance', () => {
+    expect(press('a')).toBe(false)
+    expect(press('Escape')).toBe(false)
+  })
+
+  it('lets an `@` mention take a literal space', () => {
+    expect(press(' ', { kind: '@', query: 'src/comp' })).toBe(false)
+    expect(press('Enter', { kind: '@', query: 'src/comp' })).toBe(true)
+  })
+
+  it('types a space on a bare `/ ` instead of accepting', () => {
+    expect(press(' ', { query: '' })).toBe(false)
+  })
+
+  // The `/goal <prose>` class: the popover may be live over free-form text, so
+  // the keys that mean something else in prose must keep meaning it.
+  it('sends the prose rather than the unchosen first row', () => {
+    expect(press('Enter', { freeTextArgStage: true, query: 'goal ship the redesign' })).toBe(false)
+    expect(press(' ', { freeTextArgStage: true, query: 'goal ship the' })).toBe(false)
+  })
+
+  it('accepts on Enter once the user has arrowed to a row deliberately', () => {
+    expect(press('Enter', { activeExplicit: true, freeTextArgStage: true, query: 'goal stat' })).toBe(true)
+  })
+
+  it('keeps Tab as the explicit accept even over free text', () => {
+    expect(press('Tab', { freeTextArgStage: true, query: 'goal stat' })).toBe(true)
+  })
+})
+
 describe('pickPlaceholder', () => {
   it('returns a member of the pool', () => {
     const pool = ['a', 'b', 'c'] as const
diff --git a/apps/desktop/src/app/chat/composer/composer-utils.ts b/apps/desktop/src/app/chat/composer/composer-utils.ts
index 547a210f06f08..21e3c1ac4a12e 100644
--- a/apps/desktop/src/app/chat/composer/composer-utils.ts
+++ b/apps/desktop/src/app/chat/composer/composer-utils.ts
@@ -4,6 +4,8 @@ import type { SlashChipKind } from '@/components/assistant-ui/directive-text'
 import type { ComposerAttachment } from '@/store/composer'
 import { setSessionPickerOpen } from '@/store/session'
 
+import type { TriggerState } from './text-utils'
+
 export const COMPOSER_STACK_BREAKPOINT_PX = 320
 
 // Above the stack breakpoint but still cramped: the model pill sheds its label
@@ -59,6 +61,48 @@ export const slashArgStage = (query: string) => query.includes(' ')
 /** The `/command` token of a slash query (`personality x` → `/personality`). */
 export const slashCommandToken = (query: string) => `/${query.split(/\s+/, 1)[0]?.toLowerCase() ?? ''}`
 
+export interface TriggerAcceptInput {
+  /** The user moved the highlight themselves (arrow keys) rather than
+   *  inheriting the list's default first row. */
+  activeExplicit: boolean
+  /** The trigger is a slash command whose argument is arbitrary prose. */
+  freeTextArgStage: boolean
+  key: string
+  kind: TriggerState['kind']
+  query: string
+}
+
+/**
+ * Whether a keypress accepts the highlighted completion while the popover is
+ * open. Tab is always an accept — it has no other meaning in the composer.
+ *
+ * Enter and Space are conditional, because both mean something else while a
+ * free-text argument is being written (`/goal ship the redesign`). Space types
+ * a space, and Enter sends the message; letting either take the popover's
+ * pre-highlighted row would swap the prose the user is mid-sentence on for a
+ * subcommand they never chose. Enter still accepts once the user has arrowed
+ * to a row deliberately, so the highlight never lies about what Enter will do.
+ */
+export function acceptsTriggerCompletion({
+  activeExplicit,
+  freeTextArgStage,
+  key,
+  kind,
+  query
+}: TriggerAcceptInput): boolean {
+  if (key === 'Tab') {
+    return true
+  }
+
+  if (key === 'Enter') {
+    return !freeTextArgStage || activeExplicit
+  }
+
+  // Space is slash-only (an `@` mention takes a literal space) and gated to a
+  // non-empty query so a bare `/ ` still types a space.
+  return key === ' ' && kind === '/' && Boolean(query.trim()) && !freeTextArgStage
+}
+
 export interface QueueEditState {
   attachments: ComposerAttachment[]
   draft: string