diff --git a/apps/desktop/src/app/chat/composer/composer-utils.ts b/apps/desktop/src/app/chat/composer/composer-utils.ts index 7939be35b6b6..547a210f06f0 100644 --- a/apps/desktop/src/app/chat/composer/composer-utils.ts +++ b/apps/desktop/src/app/chat/composer/composer-utils.ts @@ -50,6 +50,9 @@ export function slashChipKindForItem(item: Unstable_TriggerItem): SlashChipKind return 'command' } +/** True for a skill completion — the only kind offered mid-message. */ +export const isSkillItem = (item: Unstable_TriggerItem) => slashChipKindForItem(item) === 'skill' + /** A `/` query is at its arg stage once it's past the command name. */ export const slashArgStage = (query: string) => query.includes(' ') 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 new file mode 100644 index 000000000000..c356fae3af84 --- /dev/null +++ b/apps/desktop/src/app/chat/composer/hooks/use-composer-trigger.test.ts @@ -0,0 +1,123 @@ +import type { Unstable_TriggerAdapter, Unstable_TriggerItem } from '@assistant-ui/core' +import { act, renderHook } from '@testing-library/react' +import { createRef } from 'react' +import { describe, expect, it, vi } from 'vitest' + +import { composerPlainText, renderComposerContents, RICH_INPUT_SLOT } from '../rich-editor' + +import { useComposerTrigger } from './use-composer-trigger' + +/** A live contentEditable seeded with `text`, caret parked at the end. */ +function mountEditor(text: string) { + const editor = document.createElement('div') + editor.dataset.slot = RICH_INPUT_SLOT + editor.contentEditable = 'true' + document.body.append(editor) + renderComposerContents(editor, text) + + const range = document.createRange() + range.selectNodeContents(editor) + range.collapse(false) + const selection = window.getSelection()! + selection.removeAllRanges() + selection.addRange(range) + + return editor +} + +const item = (command: string, group = 'Skills'): Unstable_TriggerItem => ({ + id: command, + type: 'slash', + label: command.slice(1), + metadata: { command, display: command, meta: '', group, action: '', rawText: command } +}) + +function mountTrigger(editor: HTMLDivElement, items: Unstable_TriggerItem[]) { + const editorRef = createRef() as { current: HTMLDivElement | null } + editorRef.current = editor + + const draftRef = { current: composerPlainText(editor) } + + const adapter: Unstable_TriggerAdapter = { + categories: () => [], + categoryItems: () => [], + search: () => items + } + + const setComposerText = vi.fn() + + const hook = renderHook(() => + useComposerTrigger({ + at: { adapter: null, loading: false }, + draftRef, + editorRef, + requestMainFocus: vi.fn(), + setComposerText, + slash: { adapter, loading: false } + }) + ) + + return { draftRef, hook, setComposerText } +} + +describe('useComposerTrigger — slash anywhere in the prompt', () => { + it('opens the completion list for a slash typed mid-message', () => { + const editor = mountEditor('please run /cle') + const { hook } = mountTrigger(editor, [item('/clean')]) + + act(() => hook.result.current.refreshTrigger()) + + expect(hook.result.current.trigger).toMatchObject({ kind: '/', inline: true, query: 'cle' }) + expect(hook.result.current.triggerItems).toHaveLength(1) + }) + + it('inserts the picked skill inline and keeps the surrounding prose intact', () => { + const editor = mountEditor('please run /cle') + const { hook } = mountTrigger(editor, [item('/clean')]) + + act(() => hook.result.current.refreshTrigger()) + act(() => hook.result.current.replaceTriggerWithChip(item('/clean'))) + + // The `/cle` the user typed is replaced by the full command; "please run" + // in front of it survives untouched. + expect(composerPlainText(editor)).toBe('please run /clean ') + }) + + it('offers only skills mid-message, not app commands', () => { + // `/model` and `/new` act on the app — meaningless as a reference in prose. + const editor = mountEditor('please run /') + const { hook } = mountTrigger(editor, [item('/clean'), item('/model', 'Commands'), item('/new', 'Commands')]) + + act(() => hook.result.current.refreshTrigger()) + + expect(hook.result.current.triggerItems.map(i => i.label)).toEqual(['clean']) + }) + + it('still offers the full command set at the start of the prompt', () => { + const editor = mountEditor('/') + const { hook } = mountTrigger(editor, [item('/clean'), item('/model', 'Commands')]) + + act(() => hook.result.current.refreshTrigger()) + + expect(hook.result.current.triggerItems.map(i => i.label)).toEqual(['clean', 'model']) + }) + + it('still opens the list for a slash at the start of the prompt', () => { + const editor = mountEditor('/cle') + const { hook } = mountTrigger(editor, [item('/clean')]) + + act(() => hook.result.current.refreshTrigger()) + + expect(hook.result.current.trigger).toMatchObject({ kind: '/', query: 'cle' }) + expect(hook.result.current.trigger?.inline).toBeUndefined() + }) + + it('leaves a mid-message file path alone', () => { + const editor = mountEditor('open src/foo/bar') + const { hook } = mountTrigger(editor, [item('/clean')]) + + act(() => hook.result.current.refreshTrigger()) + + expect(hook.result.current.trigger).toBeNull() + }) +}) 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 20abc03309f5..3c22fddb6348 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 @@ -4,7 +4,13 @@ import { type MutableRefObject, type RefObject, useCallback, useEffect, useRef, import { hermesDirectiveFormatter } from '@/components/assistant-ui/directive-text' import { desktopSlashCommandTakesArgs } from '@/lib/desktop-slash-commands' -import { COMPLETION_ACTIONS, slashArgStage, slashChipKindForItem, slashCommandToken } from '../composer-utils' +import { + COMPLETION_ACTIONS, + isSkillItem, + slashArgStage, + slashChipKindForItem, + slashCommandToken +} from '../composer-utils' import { composerPlainText, placeCaretEnd, @@ -112,7 +118,13 @@ export function useComposerTrigger({ return } - setTriggerItems(triggerAdapter.search(trigger.query)) + const items = triggerAdapter.search(trigger.query) + + // Mid-message only offers SKILLS. A built-in like `/model` or `/new` acts + // on the app, so it's meaningless as a reference inside prose — only a + // skill reads as "handle this part with X". Filtering here rather than in + // the fetcher keeps one completion source for both shapes. + setTriggerItems(trigger.inline ? items.filter(isSkillItem) : items) }, [trigger, triggerAdapter]) const triggerLoading = trigger?.kind === '@' ? at.loading : trigger?.kind === '/' ? slash.loading : false @@ -191,10 +203,13 @@ export function useComposerTrigger({ // Picking a bare arg-taking command (e.g. `/personality`) shouldn't commit // it — expand to its options step so the popover shows the inline list, just // as typing `/personality ` by hand would. A serialized value with a space is - // already an arg pick (`/personality alice`), so it commits normally. + // already an arg pick (`/personality alice`), so it commits normally. An + // inline (mid-message) pick never expands: it's a reference inside prose, so + // there's no command invocation for the args to belong to. const command = (item.metadata as { command?: string } | undefined)?.command ?? '' - const expandsToArgs = trigger.kind === '/' && !serialized.includes(' ') && desktopSlashCommandTakesArgs(command) + const expandsToArgs = + trigger.kind === '/' && !trigger.inline && !serialized.includes(' ') && desktopSlashCommandTakesArgs(command) const text = starter || serialized.endsWith(' ') ? serialized : `${serialized} ` const directive = !starter && serialized.match(/^@([^:]+):(.+)$/) diff --git a/apps/desktop/src/app/chat/composer/text-utils.test.ts b/apps/desktop/src/app/chat/composer/text-utils.test.ts index 6c6a20780f64..ca2ac8035761 100644 --- a/apps/desktop/src/app/chat/composer/text-utils.test.ts +++ b/apps/desktop/src/app/chat/composer/text-utils.test.ts @@ -44,14 +44,25 @@ describe('detectTrigger', () => { it('does not treat file-style paths as slash triggers', () => { expect(detectTrigger('src/foo/bar')).toBeNull() expect(detectTrigger('/path/to/file')).toBeNull() + // Mid-message paths stay excluded too: a path keeps going past the command + // token, so the trailing-anchored inline trigger never matches it. + expect(detectTrigger('check src/foo/bar')).toBeNull() + expect(detectTrigger('look at /usr/local/bin')).toBeNull() + expect(detectTrigger('and/or')).toBeNull() }) - it('does not trigger slash popover mid-message', () => { - expect(detectTrigger('hello /')).toBeNull() - expect(detectTrigger('hello /skill')).toBeNull() + it('treats a mid-message slash as an inline reference', () => { + // Skills have to be reachable anywhere in a prompt, not just at position 0. + expect(detectTrigger('hello /')).toEqual({ kind: '/', inline: true, query: '', tokenLength: 1 }) + expect(detectTrigger('hello /clean')).toEqual({ kind: '/', inline: true, query: 'clean', tokenLength: 6 }) + expect(detectTrigger('text\n/skill')).toEqual({ kind: '/', inline: true, query: 'skill', tokenLength: 6 }) + }) + + it('does not carry arg completion into an inline slash reference', () => { + // Only a position-0 slash is a real invocation, so `/personality alic` + // mid-message is prose — the trigger ends at the command token. expect(detectTrigger('hello there /personality alic')).toBeNull() - expect(detectTrigger('text\n/skill')).toBeNull() - expect(detectTrigger('multi word message /')).toBeNull() + expect(detectTrigger('run /tools enable foo')).toBeNull() }) it('still anchors at-mention triggers strictly at the token edge', () => { diff --git a/apps/desktop/src/app/chat/composer/text-utils.ts b/apps/desktop/src/app/chat/composer/text-utils.ts index b9b6adc07f1e..8bc6663210e5 100644 --- a/apps/desktop/src/app/chat/composer/text-utils.ts +++ b/apps/desktop/src/app/chat/composer/text-utils.ts @@ -1,22 +1,35 @@ import { DATA_IMAGE_URL_RE, dataUrlToBlob } from '@/lib/embedded-images' export interface TriggerState { + /** True for a `/` typed mid-message — an inline skill/command reference in + * prose rather than a command invocation. Arg completion doesn't apply. */ + inline?: boolean kind: '@' | '/' query: string tokenLength: number } // `@` triggers stop at the first whitespace — `@file:path` and `@diff` are -// single tokens. `/` triggers keep going so the popover stays live while the -// user types args (`/personality alic` → arg completer suggests `alice`). -// Restricting the slash command name to `[a-zA-Z][\w-]*` avoids matching file -// paths like `src/foo/bar`. +// single tokens. Restricting the slash command name to `[a-zA-Z][\w-]*` avoids +// matching file paths like `src/foo/bar`. // -// Slash commands only execute at the beginning of a message, so the `/` -// trigger is anchored strictly at position 0 — not after whitespace — to -// avoid opening the popover mid-message (e.g. `hello /`). +// `/` triggers fire in two shapes, because a slash means two different things +// depending on where it sits: +// +// - At position 0 it's a COMMAND invocation the app executes (SLASH_COMMAND_RE +// is `^`-anchored, and so is the backend's). The popover stays live past the +// command name so arg completion works (`/personality alic` → `alice`). +// - After whitespace it's an inline REFERENCE the user is dropping into prose +// ("clean this up with /clean"). The text submits as an ordinary message, so +// there are no args to complete — the trigger is a single token that ends at +// the next space, exactly like `@`. +// +// The inline shape is what makes skills reachable anywhere in a prompt. Both +// shapes need the trailing `$`: detection runs against the text BEFORE the +// caret, so the match must end where the user is typing. const AT_TRIGGER_RE = /(?:^|[\s])(@)([^\s@/]*)$/ -const SLASH_TRIGGER_RE = /^(\/)((?:[a-zA-Z][\w-]*(?:\s+\S*)*)?)$/ +const SLASH_COMMAND_TRIGGER_RE = /^(\/)((?:[a-zA-Z][\w-]*(?:\s+\S*)*)?)$/ +const SLASH_INLINE_TRIGGER_RE = /[\s](\/)([a-zA-Z][\w-]*)?$/ /** Stable key for paste dedupe — `items` and `files` often mirror the same image as different objects. */ export function blobDedupeKey(blob: Blob): string { @@ -107,10 +120,20 @@ export function textBeforeCaret(editor: HTMLDivElement): string | null { } export function detectTrigger(textBefore: string): TriggerState | null { - const slash = SLASH_TRIGGER_RE.exec(textBefore) + const command = SLASH_COMMAND_TRIGGER_RE.exec(textBefore) - if (slash) { - return { kind: '/', query: slash[2], tokenLength: 1 + slash[2].length } + if (command) { + return { kind: '/', query: command[2], tokenLength: 1 + command[2].length } + } + + // An inline `/skill` is a reference dropped into prose, so it carries no args + // and the whole match is the token the chip replaces. + const inline = SLASH_INLINE_TRIGGER_RE.exec(textBefore) + + if (inline) { + const query = inline[2] ?? '' + + return { inline: true, kind: '/', query, tokenLength: 1 + query.length } } const at = AT_TRIGGER_RE.exec(textBefore) diff --git a/apps/desktop/src/app/chat/session-tile.tsx b/apps/desktop/src/app/chat/session-tile.tsx index 410cf87e2e42..6ca29e70ade9 100644 --- a/apps/desktop/src/app/chat/session-tile.tsx +++ b/apps/desktop/src/app/chat/session-tile.tsx @@ -113,7 +113,7 @@ function TileChat({ storedSessionId: string view: SessionView }) { - const { gatewayRef, requestGateway } = useGatewayRequest() + const { gateway, requestGateway } = useGatewayRequest() const queryClient = useQueryClient() const { selectModel } = useModelControls({ queryClient, requestGateway }) const activeGatewayProfile = useStore($activeGatewayProfile) @@ -151,20 +151,20 @@ function TileChat({ () => gatewayOpen ? ( ) : null, - [activeGatewayProfile, gatewayOpen, gatewayRef, requestGateway, selectModel] + [activeGatewayProfile, gateway, gatewayOpen, requestGateway, selectModel] ) return ( composer.addContextRefAttachment(`@url:${formatRefValue(url)}`, url)} diff --git a/apps/desktop/src/app/contrib/wiring.tsx b/apps/desktop/src/app/contrib/wiring.tsx index e8716bde9262..963fc67c3d0f 100644 --- a/apps/desktop/src/app/contrib/wiring.tsx +++ b/apps/desktop/src/app/contrib/wiring.tsx @@ -222,7 +222,7 @@ export function ContribWiring({ children }: { children: ReactNode }) { setMessages }) - const { connectionRef, gatewayRef, requestGateway } = useGatewayRequest() + const { connectionRef, gateway, gatewayRef, requestGateway } = useGatewayRequest() const { loadMoreMessagingForPlatform, @@ -943,13 +943,13 @@ export function ContribWiring({ children }: { children: ReactNode }) { /> )} @@ -965,7 +965,7 @@ export function ContribWiring({ children }: { children: ReactNode }) { {settingsOpen && ( { void refreshHermesConfig() diff --git a/apps/desktop/src/app/gateway/hooks/use-gateway-request.test.ts b/apps/desktop/src/app/gateway/hooks/use-gateway-request.test.ts new file mode 100644 index 000000000000..72ad6a2a2687 --- /dev/null +++ b/apps/desktop/src/app/gateway/hooks/use-gateway-request.test.ts @@ -0,0 +1,39 @@ +import { act, renderHook } from '@testing-library/react' +import { afterEach, describe, expect, it } from 'vitest' + +import type { HermesGateway } from '@/hermes' +import { $gateway } from '@/store/gateway' + +import { useGatewayRequest } from './use-gateway-request' + +const fakeGateway = { connectionState: 'open' } as unknown as HermesGateway + +afterEach(() => { + $gateway.set(null) +}) + +describe('useGatewayRequest', () => { + // The composer's `/` completions only exist when ChatBar receives a non-null + // gateway PROP. `gatewayRef` is populated by a subscription effect, so it is + // still null on the first render — a surface that read the ref while + // rendering (session tiles / ⌘T tabs) shipped `gateway={null}` and silently + // lost slash completions. The returned `gateway` value must be live + // immediately so that never happens again. + it('exposes the live gateway on the first render, before effects run', () => { + $gateway.set(fakeGateway) + + const { result } = renderHook(() => useGatewayRequest()) + + expect(result.current.gateway).toBe(fakeGateway) + }) + + it('tracks the gateway when the active socket changes', () => { + const { result } = renderHook(() => useGatewayRequest()) + + expect(result.current.gateway).toBeNull() + + act(() => $gateway.set(fakeGateway)) + + expect(result.current.gateway).toBe(fakeGateway) + }) +}) diff --git a/apps/desktop/src/app/gateway/hooks/use-gateway-request.ts b/apps/desktop/src/app/gateway/hooks/use-gateway-request.ts index 76c98c17a230..04f53f785b48 100644 --- a/apps/desktop/src/app/gateway/hooks/use-gateway-request.ts +++ b/apps/desktop/src/app/gateway/hooks/use-gateway-request.ts @@ -9,6 +9,14 @@ import { $gatewayState, setConnection } from '@/store/session' export function useGatewayRequest() { const gatewayState = useStore($gatewayState) + // Reactive companion to `gatewayRef`. The ref exists so `requestGateway` + // keeps a stable identity and always reaches the live socket, but it is only + // populated by the subscription effect below — i.e. AFTER the first render. + // A component that reads `gatewayRef.current` while rendering therefore sees + // null on mount, and if the connection state doesn't happen to flip + // afterwards it never re-renders to pick the instance up. Anything that needs + // the gateway as a render-time VALUE (props, memo deps) must use this. + const gateway = useStore($gateway) as HermesGateway | null const gatewayRef = useRef(null) const connectionRef = useRef['getConnection']>> | null>( @@ -136,5 +144,5 @@ export function useGatewayRequest() { [ensureGatewayOpen] ) - return { connectionRef, gatewayRef, requestGateway } + return { connectionRef, gateway, gatewayRef, requestGateway } } diff --git a/apps/desktop/src/components/assistant-ui/directive-text.test.ts b/apps/desktop/src/components/assistant-ui/directive-text.test.ts index 36ab54d743e7..d5f7074e5d13 100644 --- a/apps/desktop/src/components/assistant-ui/directive-text.test.ts +++ b/apps/desktop/src/components/assistant-ui/directive-text.test.ts @@ -47,3 +47,42 @@ describe('hermesDirectiveFormatter.parse', () => { ]) }) }) + +describe('inline skill references', () => { + const skills = (text: string) => + [...hermesDirectiveFormatter.parse(text)] + .filter(segment => segment.kind === 'mention' && segment.type === 'skill') + .map(segment => (segment.kind === 'mention' ? segment.id : '')) + + it('keeps a picked skill a chip in the sent message instead of flattening it', () => { + expect(skills('please run /clean on this')).toEqual(['/clean']) + }) + + it('keeps the surrounding prose as text around the chip', () => { + const segments = hermesDirectiveFormatter.parse('tidy this with /clean thanks') + + expect(segments).toEqual([ + { kind: 'text', text: 'tidy this with ' }, + { kind: 'mention', type: 'skill', label: 'clean', id: '/clean' }, + { kind: 'text', text: ' thanks' } + ]) + }) + + it('leaves file paths and fractions alone', () => { + expect(skills('check src/foo/bar')).toEqual([]) + expect(skills('look at /usr/local/bin')).toEqual([]) + expect(skills('roughly 3 /4 of it')).toEqual([]) + }) + + it('does not chip a leading slash — that is a command invocation, not prose', () => { + expect(skills('/clean')).toEqual([]) + }) + + it('parses a skill chip alongside an @ reference', () => { + const mentions = [...hermesDirectiveFormatter.parse('run /clean on @file:`src/a.ts`')].filter( + segment => segment.kind === 'mention' + ) + + expect(mentions.map(segment => (segment.kind === 'mention' ? segment.type : ''))).toEqual(['skill', 'file']) + }) +}) diff --git a/apps/desktop/src/components/assistant-ui/directive-text.tsx b/apps/desktop/src/components/assistant-ui/directive-text.tsx index 6ea2580f16b8..c9a45f2d3f76 100644 --- a/apps/desktop/src/components/assistant-ui/directive-text.tsx +++ b/apps/desktop/src/components/assistant-ui/directive-text.tsx @@ -171,6 +171,17 @@ const HERMES_DIRECTIVE_RE = new RegExp( 'g' ) +// A skill referenced mid-prose (`clean this up with /clean`). The composer +// inserts it as a pill, so the sent message renders it as one too rather than +// flattening back to raw text. Only matches after whitespace — a leading `/` +// is a command invocation, which never reaches a rendered message as text. +// +// Unlike the composer's caret-anchored trigger, this scans finished text, so +// it must reject a token that continues into a path: `/usr/local/bin` would +// otherwise chip as `/usr`. `(?![\w-]*\/)` requires the token to end at +// something other than another slash. +const SLASH_SKILL_RE = /(?<=\s)\/([a-zA-Z][\w-]*)(?![\w-]*\/)/g + const TRAILING_PUNCTUATION_RE = /[,.;!?]+$/ function unwrapRefValue(raw: string): string { @@ -269,7 +280,14 @@ function parseDirectiveText(text: string): Unstable_DirectiveSegment[] { label: shortLabel(match[1] as HermesRefType, id), id } - }) + }), + ...Array.from(text.matchAll(SLASH_SKILL_RE)).map(match => ({ + start: match.index ?? 0, + end: (match.index ?? 0) + match[0].length, + type: 'skill', + label: match[1], + id: `/${match[1]}` + })) ] .filter(match => match.id) .sort((a, b) => a.start - b.start) @@ -369,6 +387,8 @@ export function DirectiveContent({ text }: { text: string }) { {segment.text} ) : segment.type === 'image' ? null : segment.type === 'session' ? ( + ) : segment.type === 'skill' ? ( + ) : ( ) @@ -505,6 +525,28 @@ export const SessionRefLink: FC<{ ) } +/** A skill referenced inside a sent message — the rendered twin of the + * composer's slash pill, so a picked skill stays a chip after send. */ +const SlashChip: FC<{ kind: SlashChipKind; label: string; value: string }> = ({ kind, label, value }) => ( + + + {SLASH_ICON_PATHS[kind].map(d => ( + + ))} + + {label} + +) + /** Inert by default; `onClick` promotes the chip to a real button (session * refs, which open the session they name). */ const DirectiveChip: FC<{