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 ea968fb3643..c59bcda6d7f 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 @@ -16,6 +16,7 @@ import { placeCaretEnd, refChipElement, renderComposerContents, + replaceBeforeCaret, slashChipElement } from '../rich-editor' import { detectTrigger, textBeforeCaret, type TriggerState } from '../text-utils' @@ -29,6 +30,8 @@ interface UseComposerTriggerOptions { at: CompletionSource draftRef: MutableRefObject editorRef: RefObject + /** Bank the pre-commit state so a popover pick is a single undo step. */ + recordUndoPoint?: () => void requestMainFocus: () => void setComposerText: (text: string) => void slash: CompletionSource @@ -47,6 +50,7 @@ export function useComposerTrigger({ at, draftRef, editorRef, + recordUndoPoint, requestMainFocus, setComposerText, slash @@ -214,17 +218,31 @@ export function useComposerTrigger({ return } + // Bank the pre-commit state first — every path below mutates the editor, + // and a pick must be exactly one undo step. + recordUndoPoint?.() + + // Rebuild-from-text fallback for carets the range walk can't anchor (a + // non-collapsed selection, a caret not preceded by contiguous text). It + // re-renders the whole editor from serialized text, so it only runs when + // the in-place path reports failure — never as the default. + const rebuildWith = (render: (prefix: string) => void) => { + const current = composerPlainText(editor) + + render(current.slice(0, Math.max(0, current.length - trigger.tokenLength))) + placeCaretEnd(editor) + } + // Action items (e.g. "Browse all sessions…") run a side effect instead of // inserting a chip: strip the typed trigger token, then fire the action. const completionAction = (item.metadata as { action?: unknown } | undefined)?.action const runAction = typeof completionAction === 'string' ? COMPLETION_ACTIONS[completionAction] : undefined if (runAction) { - const current = composerPlainText(editor) - const prefix = current.slice(0, Math.max(0, current.length - trigger.tokenLength)) + if (!replaceBeforeCaret(editor, trigger.tokenLength, document.createDocumentFragment())) { + rebuildWith(prefix => renderComposerContents(editor, prefix)) + } - renderComposerContents(editor, prefix) - placeCaretEnd(editor) draftRef.current = composerPlainText(editor) setComposerText(draftRef.current) closeTrigger() @@ -247,19 +265,24 @@ export function useComposerTrigger({ ? String((item.metadata as { insertId?: unknown } | undefined)?.insertId ?? '') : '' - if (descendInto) { - const path = descendInto.endsWith('/') ? descendInto : `${descendInto}/` - const current = composerPlainText(editor) - const prefix = current.slice(0, Math.max(0, current.length - trigger.tokenLength)) - - renderComposerContents(editor, `${prefix}@${path}`) - placeCaretEnd(editor) + const finish = (keepOpen: boolean) => { draftRef.current = composerPlainText(editor) setComposerText(draftRef.current) requestMainFocus() - window.setTimeout(refreshTrigger, 0) + keepOpen ? window.setTimeout(refreshTrigger, 0) : closeTrigger() + } - return + if (descendInto) { + const path = descendInto.endsWith('/') ? descendInto : `${descendInto}/` + const fragment = document.createDocumentFragment() + + fragment.append(document.createTextNode(`@${path}`)) + + if (!replaceBeforeCaret(editor, trigger.tokenLength, fragment)) { + rebuildWith(prefix => renderComposerContents(editor, `${prefix}@${path}`)) + } + + return finish(true) } // Picking a bare arg-taking command (e.g. `/personality`) shouldn't commit @@ -280,67 +303,32 @@ export function useComposerTrigger({ const slashKind = !expandsToArgs && trigger.kind === '/' ? slashChipKindForItem(item) : null const keepTriggerOpen = starter || (expandsToArgs && argumentMode !== 'text') - const finish = () => { - draftRef.current = composerPlainText(editor) - setComposerText(draftRef.current) - requestMainFocus() - keepTriggerOpen ? window.setTimeout(refreshTrigger, 0) : closeTrigger() - } - - const sel = window.getSelection() - const range = sel?.rangeCount ? sel.getRangeAt(0) : null - const node = range?.startContainer - const offset = range?.startOffset ?? 0 - - if (!sel || !range || node?.nodeType !== Node.TEXT_NODE || offset < trigger.tokenLength) { - const current = composerPlainText(editor) - const prefix = current.slice(0, Math.max(0, current.length - trigger.tokenLength)) - - if (slashKind) { - // Two-step arg picks (e.g. `/handoff` pill already inserted, now picking - // the platform) land here because the caret sits past a contenteditable - // chip. Rebuild the prefix and re-emit a single pill for the full command. - renderComposerContents(editor, prefix) - editor.append(slashChipElement(serialized, slashKind), document.createTextNode(' ')) - placeCaretEnd(editor) - - return finish() - } - - renderComposerContents(editor, `${prefix}${text}`) - placeCaretEnd(editor) - - return finish() - } - - const replaceRange = document.createRange() - replaceRange.setStart(node, offset - trigger.tokenLength) - replaceRange.setEnd(node, offset) - replaceRange.deleteContents() - const chip = slashKind ? slashChipElement(serialized, slashKind) : directive ? refChipElement(directive[1], directive[2]) : null - if (chip) { - const space = document.createTextNode(' ') - const fragment = document.createDocumentFragment() - fragment.append(chip, space) - replaceRange.insertNode(fragment) + const fragment = document.createDocumentFragment() - const caret = document.createRange() - caret.setStart(space, 1) - caret.collapse(true) - sel.removeAllRanges() - sel.addRange(caret) + chip ? fragment.append(chip, document.createTextNode(' ')) : fragment.append(document.createTextNode(text)) - return finish() + if (!replaceBeforeCaret(editor, trigger.tokenLength, fragment)) { + rebuildWith(prefix => { + if (chip) { + // The failed in-place attempt never consumed the fragment, so the + // chip + trailing space land here instead. Appending the element + // keeps mid-message slash pills alive — they have no text + // hydration, unlike `@` refs and the leading command. + renderComposerContents(editor, prefix) + editor.append(fragment) + } else { + renderComposerContents(editor, `${prefix}${text}`) + } + }) } - document.execCommand('insertText', false, text) - finish() + finish(keepTriggerOpen) } /** Backspace inside an `@` path drops the last segment (`a/b/` → `a/`) @@ -359,11 +347,23 @@ export function useComposerTrigger({ const trimmed = trigger.query.replace(/\/$/, '') const parent = trimmed.slice(0, trimmed.lastIndexOf('/') + 1) - const current = composerPlainText(editor) - const prefix = current.slice(0, Math.max(0, current.length - trigger.tokenLength)) + recordUndoPoint?.() + + const fragment = document.createDocumentFragment() + + fragment.append(document.createTextNode(`@${parent}`)) + + // In place first: the destructive re-render fallback rebuilds the editor + // from text, which is exactly what used to demote a leading command pill + // to plaintext on every Backspace inside a path. + if (!replaceBeforeCaret(editor, trigger.tokenLength, fragment)) { + const current = composerPlainText(editor) + const prefix = current.slice(0, Math.max(0, current.length - trigger.tokenLength)) + + renderComposerContents(editor, `${prefix}@${parent}`) + placeCaretEnd(editor) + } - renderComposerContents(editor, `${prefix}@${parent}`) - placeCaretEnd(editor) draftRef.current = composerPlainText(editor) setComposerText(draftRef.current) window.setTimeout(refreshTrigger, 0) diff --git a/apps/desktop/src/app/chat/composer/index.tsx b/apps/desktop/src/app/chat/composer/index.tsx index cfb5e751889..478e40c281f 100644 --- a/apps/desktop/src/app/chat/composer/index.tsx +++ b/apps/desktop/src/app/chat/composer/index.tsx @@ -346,7 +346,7 @@ export function ChatBar({ triggerItems, triggerKeyConsumedRef, triggerLoading - } = useComposerTrigger({ at, draftRef, editorRef, requestMainFocus, setComposerText, slash }) + } = useComposerTrigger({ at, draftRef, editorRef, recordUndoPoint, requestMainFocus, setComposerText, slash }) // Pull the live contentEditable text into draftRef + the AUI composer state // (which drives `hasComposerPayload` → the send button). Shared by the input @@ -569,6 +569,17 @@ export function ChatBar({ return } + // The popover is open but its items are still in flight (debounce + RPC). + // Tab must not fall through to the browser — it would move focus out of + // the composer mid-completion, which reads as the popover "eating" the + // keypress. Swallow it; the refresh lands with the items. + if (trigger && triggerLoading && triggerItems.length === 0 && event.key === 'Tab') { + event.preventDefault() + triggerKeyConsumedRef.current = true + + return + } + if (trigger && triggerItems.length > 0) { if (event.key === 'ArrowDown') { event.preventDefault() diff --git a/apps/desktop/src/app/chat/composer/rich-editor.ts b/apps/desktop/src/app/chat/composer/rich-editor.ts index 7bf770eda17..bc6a1f26bc8 100644 --- a/apps/desktop/src/app/chat/composer/rich-editor.ts +++ b/apps/desktop/src/app/chat/composer/rich-editor.ts @@ -16,11 +16,22 @@ import { type SlashChipKind, slashIconElement } from '@/components/assistant-ui/directive-text' +import { + desktopSlashCommandArgumentMode, + isDesktopSlashCommand, + resolveDesktopCommand +} from '@/lib/desktop-slash-commands' export const RICH_INPUT_SLOT = 'composer-rich-input' export const REF_RE = /@(file|folder|url|image|tool|line|terminal|session):(`[^`\n]+`|"[^"\n]+"|'[^'\n]+'|\S+)/g +/** A committed leading slash command: `/name` followed by whitespace. The + * whitespace requirement is what separates a committed command (chips always + * serialize with their auto-inserted trailing space) from one still being + * typed, which must stay editable text. */ +const LEADING_SLASH_COMMAND_RE = /^\/[a-zA-Z][\w-]*(?=\s)/ + const ESC: Record = { '&': '&', '<': '<', '>': '>', '"': '"', "'": ''' } export function escapeHtml(value: string) { @@ -129,6 +140,24 @@ export function appendComposerContents(target: DocumentFragment | HTMLElement, t export function renderComposerContents(target: HTMLElement, text: string) { target.replaceChildren() + + // A leading `/command` hydrates back to its pill — parity with REF_RE for + // `@` refs, so a full re-render from serialized text (draft restore, undo, + // the trigger commit fallback) doesn't demote a committed command chip to + // plain text. Only commands with NO argument stage qualify (skills, quick + // commands, no-arg built-ins): their committed pill is exactly the bare + // `/name`, so the boundary is unambiguous. Arg-taking commands (`/goal ship + // it`, `/personality alice`) stay text — their tail may be prose that was + // never committed. The trailing whitespace is load-bearing too: a committed + // pill always serializes with its auto-inserted space, while a half-typed + // `/wor` must stay editable text. + const command = LEADING_SLASH_COMMAND_RE.exec(text)?.[0] + + if (command && isDesktopSlashCommand(command) && desktopSlashCommandArgumentMode(command) === null) { + target.append(slashChipElement(command, resolveDesktopCommand(command) ? 'command' : 'skill')) + text = text.slice(command.length) + } + appendComposerContents(target, text) } @@ -173,27 +202,84 @@ export function insertComposerContentsAtCaret(editor: HTMLElement, text: string) } } -/** Swap the `length` characters immediately before a collapsed caret for - * `fragment`, leaving the caret after it. Returns whether it ran — a caret that - * isn't inside a text node holding the whole token is left alone. */ -export function replaceBeforeCaret(editor: HTMLElement, length: number, fragment: DocumentFragment) { +/** Range covering exactly `length` serialized characters immediately before a + * collapsed caret, spanning Chromium's split text nodes. Null when the caret + * isn't a collapsed selection in `editor`, or when a chip/
/block boundary + * interrupts before `length` characters are covered — a trigger token is + * always contiguous text, so anything else means "don't touch the DOM here". + * + * This is what keeps chip insertion stable: Chromium fragments text nodes + * around contenteditable=false chips on every edit, so any commit path that + * demands the whole token inside ONE text node (the old check) degrades to a + * full re-render as soon as a chip exists anywhere in the line. */ +export function rangeBeforeCaret(editor: HTMLElement, length: number): Range | null { const hit = composerSelectionRange(editor) - if (!hit?.range.collapsed) { - return false + if (!hit?.range.collapsed || length <= 0) { + return null } - const { startContainer, startOffset } = hit.range + let node: Node | null = hit.range.startContainer + let offset = hit.range.startOffset - if (startContainer.nodeType !== Node.TEXT_NODE || startOffset < length) { - return false + // An element-positioned caret (common right after programmatic caret moves) + // resolves to the end of the text node before it. A chip or
there means + // no text token precedes the caret — bail rather than guess. + if (node.nodeType !== Node.TEXT_NODE) { + node = node.childNodes[offset - 1] ?? null + + if (node?.nodeType !== Node.TEXT_NODE) { + return null + } + + offset = (node.textContent || '').length + } + + let startNode = node as Text + let startOffset = offset + let remaining = length + + while (remaining > 0) { + if (startOffset >= remaining) { + startOffset -= remaining + remaining = 0 + + break + } + + remaining -= startOffset + + const prev: Node | null = startNode.previousSibling + + if (prev?.nodeType !== Node.TEXT_NODE) { + return null + } + + startNode = prev as Text + startOffset = (prev.textContent || '').length } const range = document.createRange() + + range.setStart(startNode, startOffset) + range.setEnd(hit.range.startContainer, hit.range.startOffset) + + return range +} + +/** Swap the `length` characters immediately before a collapsed caret for + * `fragment`, leaving the caret after it. Returns whether it ran. Spans split + * text nodes (see rangeBeforeCaret) — a token typed around existing chips + * still commits in place instead of falling back to a full re-render. */ +export function replaceBeforeCaret(editor: HTMLElement, length: number, fragment: DocumentFragment) { + const range = rangeBeforeCaret(editor, length) + + if (!range) { + return false + } + const tail = fragment.lastChild - range.setStart(startContainer, startOffset - length) - range.setEnd(startContainer, startOffset) range.deleteContents() range.insertNode(fragment) @@ -202,8 +288,11 @@ export function replaceBeforeCaret(editor: HTMLElement, length: number, fragment } range.collapse(true) - hit.selection.removeAllRanges() - hit.selection.addRange(range) + + const selection = window.getSelection() + + selection?.removeAllRanges() + selection?.addRange(range) return true } diff --git a/apps/desktop/src/components/assistant-ui/thread/user-edit-composer.tsx b/apps/desktop/src/components/assistant-ui/thread/user-edit-composer.tsx index 8791ac3b75e..72e9b8e1ddb 100644 --- a/apps/desktop/src/components/assistant-ui/thread/user-edit-composer.tsx +++ b/apps/desktop/src/components/assistant-ui/thread/user-edit-composer.tsx @@ -38,6 +38,7 @@ import { placeCaretEnd, refChipElement, renderComposerContents, + replaceBeforeCaret, RICH_INPUT_SLOT } from '@/app/chat/composer/rich-editor' import { detectTrigger, textBeforeCaret, type TriggerState } from '@/app/chat/composer/text-utils' @@ -321,41 +322,22 @@ export const UserEditComposer: FC = ({ cwd, gateway, sess starter ? window.setTimeout(refreshTrigger, 0) : closeTrigger() } - const sel = window.getSelection() - const range = sel?.rangeCount ? sel.getRangeAt(0) : null - const node = range?.startContainer - const offset = range?.startOffset ?? 0 + // In place first, spanning Chromium's split text nodes (see + // rangeBeforeCaret). The re-render fallback only runs when the caret + // genuinely can't anchor the token — it rebuilds from serialized text, + // which re-chips `@` refs but resets the caret to the end. + const fragment = document.createDocumentFragment() - if (!sel || !range || node?.nodeType !== Node.TEXT_NODE || offset < trigger.tokenLength) { + directive + ? fragment.append(refChipElement(directive[1], directive[2]), document.createTextNode(' ')) + : fragment.append(document.createTextNode(text)) + + if (!replaceBeforeCaret(editor, trigger.tokenLength, fragment)) { const current = composerPlainText(editor) renderComposerContents(editor, `${current.slice(0, Math.max(0, current.length - trigger.tokenLength))}${text}`) placeCaretEnd(editor) - - return finish() } - const replaceRange = document.createRange() - replaceRange.setStart(node, offset - trigger.tokenLength) - replaceRange.setEnd(node, offset) - replaceRange.deleteContents() - - if (directive) { - const chip = refChipElement(directive[1], directive[2]) - const space = document.createTextNode(' ') - const fragment = document.createDocumentFragment() - fragment.append(chip, space) - replaceRange.insertNode(fragment) - - const caret = document.createRange() - caret.setStart(space, 1) - caret.collapse(true) - sel.removeAllRanges() - sel.addRange(caret) - - return finish() - } - - document.execCommand('insertText', false, text) finish() }, [aui, closeTrigger, recordUndoPoint, refreshTrigger, rememberInitialDraft, requestEditFocus, trigger]