diff --git a/agent/skill_commands.py b/agent/skill_commands.py index 294ca2b1754..3f1156a8592 100644 --- a/agent/skill_commands.py +++ b/agent/skill_commands.py @@ -97,7 +97,7 @@ def extract_user_instruction_from_skill_message(content: Any) -> Optional[str]: return None -def describe_skill_invocation(content: Any) -> Optional[str]: +def describe_skill_invocation(content: Any, separator: str = " — ") -> Optional[str]: """Render a slash-skill-expanded turn the way the user typed it. The expanded message embeds the whole skill body, so any surface that @@ -109,6 +109,10 @@ def describe_skill_invocation(content: Any) -> Optional[str]: Returns ``"/work — fix the title leak"``, or ``"/work"`` for a bare invocation, or ``None`` when *content* is not skill scaffolding (the caller should then summarize it as an ordinary message). + + *separator* joins the command and the instruction. Previews use the + default em dash; pass ``" "`` for the literal invocation the user typed, + which is what chat transcripts render. """ if not isinstance(content, str) or not content.startswith(_SKILL_INVOCATION_PREFIX): return None @@ -127,7 +131,7 @@ def describe_skill_invocation(content: Any) -> Optional[str]: instruction = instruction.split(SKILL_EXCERPT_JOINT)[0] instruction = " ".join(instruction.split()) if instruction: - return f"{label} — {instruction}" if name else instruction + return f"{label}{separator}{instruction}" if name else instruction return label if name else None diff --git a/apps/desktop/src/app/chat/composer/hooks/use-composer-queue.ts b/apps/desktop/src/app/chat/composer/hooks/use-composer-queue.ts index dff3804bb69..e1bb5442fef 100644 --- a/apps/desktop/src/app/chat/composer/hooks/use-composer-queue.ts +++ b/apps/desktop/src/app/chat/composer/hooks/use-composer-queue.ts @@ -106,7 +106,9 @@ export function useComposerQueue({ entryId: entry.id, sessionKey: activeQueueSessionKey }) - loadIntoComposer(entry.text, entry.attachments) + // Edit what the panel SHOWS. A queued `/skill` entry's text is the + // expanded skill body — never drop that into the composer. + loadIntoComposer(entry.displayText ?? entry.text, entry.attachments) triggerHaptic('selection') focusInput() } @@ -135,7 +137,7 @@ export function useComposerQueue({ if (next) { setQueueEditSnapshot({ ...queueEdit, entryId: next.id }) - loadIntoComposer(next.text, next.attachments) + loadIntoComposer(next.displayText ?? next.text, next.attachments) } else { setQueueEditSnapshot(null) loadIntoComposer(queueEdit.draft, queueEdit.attachments) @@ -213,6 +215,7 @@ export function useComposerQueue({ const accepted = await Promise.resolve( onSubmit(entry.text, { attachments: entry.attachments, + ...(entry.displayText ? { displayText: entry.displayText } : {}), fromQueue: true, sessionId: drainRuntimeSessionId, storedSessionId: drainQueueSessionKey diff --git a/apps/desktop/src/app/chat/composer/queue-panel.tsx b/apps/desktop/src/app/chat/composer/queue-panel.tsx index 591eeb10ebb..2f5612efd13 100644 --- a/apps/desktop/src/app/chat/composer/queue-panel.tsx +++ b/apps/desktop/src/app/chat/composer/queue-panel.tsx @@ -22,7 +22,7 @@ interface QueuePanelProps { } const entryPreview = (entry: QueuedPromptEntry, c: Translations['composer']) => - entry.text.trim() || (entry.attachments.length > 0 ? c.attachmentOnly : c.emptyTurn) + (entry.displayText ?? entry.text).trim() || (entry.attachments.length > 0 ? c.attachmentOnly : c.emptyTurn) export function QueuePanel({ busy, diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx index c9cd19c6406..6bf51c1e0fc 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx @@ -1215,9 +1215,9 @@ describe('usePromptActions slash.exec dispatch payloads', () => { it("sends a skill's kickoff into the TAB that invoked it, not the foreground chat", async () => { // `/work` in a fresh ⌘T tab: slash.exec returns a skill dispatch whose // `message` is the kickoff prompt. The dispatcher resolved the tab as its - // target, printed "⚡ loading skill" there — then submitted the kickoff - // with no target at all, so submit re-resolved from activeSessionIdRef and - // fired it as a user message into whatever conversation was on screen. + // target, then submitted the kickoff with no target at all, so submit + // re-resolved from activeSessionIdRef and fired it as a user message into + // whatever conversation was on screen. const tabRuntimeId = 'tab-runtime' const tabStoredId = 'tab-stored' @@ -1262,6 +1262,56 @@ describe('usePromptActions slash.exec dispatch payloads', () => { $queuedPromptsBySession.set({}) }) + it('renders a skill turn as its invocation — the expanded body never reaches a bubble', async () => { + // A `/skill` dispatch's `message` is the whole skill body (model-facing + // scaffolding). The agent must receive it verbatim; every UI surface — + // the user bubble and any system line — must show only `/work fix it`. + const skillBody = + '[IMPORTANT: The user has invoked the "work" skill, indicating they want you to follow its instructions.\n' + + 'The full skill content is loaded below.]\n\nSPIN UP A WORKTREE, never the primary checkout.\n\n' + + 'The user has provided the following instruction alongside the skill invocation: fix it' + + const states: Record[] = [] + const submitted: (Record | undefined)[] = [] + + const requestGateway = vi.fn(async (method: string, params?: Record) => { + if (method === 'prompt.submit') { + submitted.push(params) + } + + return ( + method === 'slash.exec' ? { type: 'skill', name: 'work', message: skillBody, display: '/work fix it' } : {} + ) as never + }) + + let handle: HarnessHandle | null = null + await actRender( + (handle = h)} + onSeedState={s => states.push(s)} + refreshSessions={async () => undefined} + requestGateway={requestGateway} + /> + ) + + await handle!.submitText('/work fix it') + + // The agent still gets the full skill. + expect(submitted).toEqual([expect.objectContaining({ text: skillBody })]) + + const rendered = states.flatMap(state => { + const messages = Array.isArray(state.messages) + ? (state.messages as Array<{ parts?: Array<{ text?: string }> }>) + : [] + + return messages.flatMap(message => (message.parts ?? []).map(part => part.text ?? '')) + }) + + expect(rendered).toContain('/work fix it') + expect(rendered.join('\n')).not.toContain('SPIN UP A WORKTREE') + expect(rendered.join('\n')).not.toContain('IMPORTANT: The user has invoked') + }) + it('slash status header carries the command token, not the full invocation', async () => { // `/goal ` used to echo the entire invocation in the mono // header AND the goal text again in the backend notice right under it. diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/slash.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions/slash.ts index c70e6977c0a..ad30e30cae6 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/slash.ts +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/slash.ts @@ -1,3 +1,4 @@ +import { skillInvocationText } from '@hermes/shared' import { type MutableRefObject, useCallback, useRef } from 'react' import { getProfiles } from '@/hermes' @@ -264,9 +265,12 @@ export function useSlashCommand(deps: SlashCommandDeps) { return } - if (dispatch.type === 'skill') { - renderSlashOutput(`⚡ loading skill: ${dispatch.name}`) - } + // A skill/bundle dispatch's `message` is the expanded skill body — + // model-facing scaffolding. Never render it; the bubble shows the + // invocation the gateway projected, or one read from the payload + // when the backend is older than this app. + const projected = 'display' in dispatch ? dispatch.display?.trim() : '' + const displayText = projected || skillInvocationText(message) || undefined // Gate on the TARGET session's own busy state, not the foreground // view's — see isTargetSessionBusy. `busyRef` mirrors whatever chat @@ -286,7 +290,7 @@ export function useSlashCommand(deps: SlashCommandDeps) { // whichever chat is now in front. const queueKey = resolveComposerSessionKey(storedSessionId, $sessions.get()) || storedSessionId || sessionId - if (enqueueQueuedPrompt(queueKey, { attachments: [], text: message })) { + if (enqueueQueuedPrompt(queueKey, { attachments: [], text: message, displayText })) { renderSlashOutput('session busy — message queued to send when the current turn finishes') } else { renderSlashOutput('session busy — /interrupt the current turn before sending this command') @@ -299,12 +303,11 @@ export function useSlashCommand(deps: SlashCommandDeps) { // same pair the output writer and the busy gate above already use. // Bare `submitPromptText(message)` let submit re-resolve from // `activeSessionIdRef`, which names the FOREGROUND chat: a `/work` - // typed into a fresh ⌘T tab loaded the skill in that tab, printed - // "⚡ loading skill" there, then fired its kickoff as a user message - // into whatever conversation was on screen. Every other target the - // dispatcher serves (tile, background queue drain, a session created - // by this very call) had the same leak. - await submitPromptText(message, { sessionId, storedSessionId }) + // typed into a fresh ⌘T tab loaded the skill in that tab, then fired + // its kickoff as a user message into whatever conversation was on + // screen. Every other target the dispatcher serves (tile, background + // queue drain, a session created by this very call) had the same leak. + await submitPromptText(message, { sessionId, storedSessionId, displayText }) } try { diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts index d8988a0c926..33b6ab4c33e 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts @@ -268,10 +268,16 @@ export function useSubmitPrompt(deps: SubmitPromptDeps) { const optimisticId = `user-${Date.now()}-${Math.random().toString(36).slice(2, 8)}` + // What the bubble shows. A `/skill` send carries the whole expanded + // skill body as its text — model-facing scaffolding — so the dispatcher + // hands us the invocation to render instead. Everything else shows what + // was typed. + const bubbleText = options?.displayText ?? visibleText + const buildUserMessage = (): ChatMessage => ({ id: optimisticId, role: 'user', - parts: [textPart(visibleText || (attachmentRefs.length ? '' : attachments.map(a => a.label).join(', ')))], + parts: [textPart(bubbleText || (attachmentRefs.length ? '' : attachments.map(a => a.label).join(', ')))], attachmentRefs }) @@ -466,7 +472,7 @@ export function useSubmitPrompt(deps: SubmitPromptDeps) { if (!sessionId) { try { - sessionId = await createBackendSessionForSend(visibleText) + sessionId = await createBackendSessionForSend(bubbleText) } catch (err) { dropOptimistic(null) releaseBusy() diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/utils.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions/utils.ts index b193b1aebf1..a2ec14eb2a5 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/utils.ts +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/utils.ts @@ -393,6 +393,11 @@ export interface SubmitTextOptions { * (queue drain, steer, external submit requests): the check is a no-op * without it. */ composerScope?: string | null + /** What the transcript shows for this send, when it differs from the text + * the agent receives. A `/skill` invocation expands into the whole skill + * body — model-facing scaffolding the UI must never render — so the slash + * dispatcher passes the invocation (`/work fix the leak`) here. */ + displayText?: string fromQueue?: boolean /** Runtime session id to submit into. Queue drains pass this so a * backgrounded/source session cannot be replaced by the current foreground diff --git a/apps/desktop/src/app/types.ts b/apps/desktop/src/app/types.ts index 01ada56e935..f7452607f7c 100644 --- a/apps/desktop/src/app/types.ts +++ b/apps/desktop/src/app/types.ts @@ -132,12 +132,17 @@ export interface SkillCommandDispatchResponse { type: 'skill' name: string message?: string + /** The invocation the UI renders (`/work fix the leak`). `message` is the + * expanded skill body — model-facing scaffolding no surface may show. */ + display?: string } export interface SendCommandDispatchResponse { type: 'send' message: string notice?: string + /** Set for a skill-bundle send: see SkillCommandDispatchResponse.display. */ + display?: string } export interface PrefillCommandDispatchResponse { 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 d5f7074e5d1..c177e2f5066 100644 --- a/apps/desktop/src/components/assistant-ui/directive-text.test.ts +++ b/apps/desktop/src/components/assistant-ui/directive-text.test.ts @@ -74,8 +74,14 @@ describe('inline skill references', () => { 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('chips a leading slash, which now reaches the transcript as a skill invocation', () => { + // #71664 asserted the opposite, and was right at the time: a leading slash + // only ever EXECUTED, so it never reached a rendered message as text — + // the turn that reached the bubble was the expanded skill body. Projecting + // a skill turn back onto `/work fix it` changes that precondition, so the + // invocation now has to chip like any other skill reference. + expect(skills('/clean')).toEqual(['/clean']) + expect(skills('/work fix the leak')).toEqual(['/work']) }) it('parses a skill chip alongside an @ reference', () => { diff --git a/apps/desktop/src/components/assistant-ui/directive-text.tsx b/apps/desktop/src/components/assistant-ui/directive-text.tsx index 94edc90963d..418f59eda71 100644 --- a/apps/desktop/src/components/assistant-ui/directive-text.tsx +++ b/apps/desktop/src/components/assistant-ui/directive-text.tsx @@ -177,16 +177,22 @@ 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. +// A skill referenced in a sent message — either the invocation that opens it +// (`/work fix the leak`, which is all a skill turn ever renders as) or one +// named mid-prose (`clean this up with /clean`). The composer inserts both as +// pills, so the sent message renders them as pills too rather than flattening +// back to raw text. +// +// #71664 deliberately excluded a LEADING slash, and was right then: a command +// only ever executed, so it never reached a rendered message as text. Skill +// turns now project back onto their invocation, so that precondition is gone +// and `^` joins the lookbehind. // // 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 SLASH_SKILL_RE = /(?<=^|\s)\/([a-zA-Z][\w-]*)(?![\w-]*\/)/g const TRAILING_PUNCTUATION_RE = /[,.;!?]+$/ diff --git a/apps/desktop/src/lib/chat-messages.ts b/apps/desktop/src/lib/chat-messages.ts index 13cca5b511b..6ee82ebbb6e 100644 --- a/apps/desktop/src/lib/chat-messages.ts +++ b/apps/desktop/src/lib/chat-messages.ts @@ -1,5 +1,5 @@ import type { ThreadMessageLike } from '@assistant-ui/react' -import type { BillingBlock } from '@hermes/shared' +import { type BillingBlock, skillInvocationText } from '@hermes/shared' import { extractImageRefs } from '@/lib/embedded-images' import { dedupeGeneratedImageEchoesInParts } from '@/lib/generated-images' @@ -301,6 +301,15 @@ function displayContentForMessage(role: SessionMessage['role'], content: unknown return textContent } + // A `/skill` turn is stored expanded (the whole skill body). Current + // gateways project it to the invocation before it ever reaches us; this is + // the fallback for an older backend that still ships the raw payload. + const invocation = skillInvocationText(textContent) + + if (invocation) { + return invocation + } + const marker = textContent.match(ATTACHED_CONTEXT_MARKER_RE) if (!marker || marker.index === undefined) { diff --git a/apps/desktop/src/lib/chat-runtime.ts b/apps/desktop/src/lib/chat-runtime.ts index b392d2692a3..65965c41b67 100644 --- a/apps/desktop/src/lib/chat-runtime.ts +++ b/apps/desktop/src/lib/chat-runtime.ts @@ -283,10 +283,14 @@ export function parseCommandDispatch(raw: unknown): CommandDispatchResponse | nu return typeof row.target === 'string' ? { type: 'alias', target: row.target } : null case 'skill': - return typeof row.name === 'string' ? { type: 'skill', name: row.name, message: str(row.message) } : null + return typeof row.name === 'string' + ? { type: 'skill', name: row.name, message: str(row.message), display: str(row.display) } + : null case 'send': - return typeof row.message === 'string' ? { type: 'send', message: row.message, notice: str(row.notice) } : null + return typeof row.message === 'string' + ? { type: 'send', message: row.message, notice: str(row.notice), display: str(row.display) } + : null case 'prefill': return typeof row.message === 'string' ? { type: 'prefill', message: row.message, notice: str(row.notice) } : null diff --git a/apps/desktop/src/store/composer-queue.ts b/apps/desktop/src/store/composer-queue.ts index 9a048c78699..ba610b36275 100644 --- a/apps/desktop/src/store/composer-queue.ts +++ b/apps/desktop/src/store/composer-queue.ts @@ -5,6 +5,10 @@ import type { ComposerAttachment } from './composer' export interface QueuedPromptEntry { id: string text: string + /** What the queue panel and the sent bubble show, when it differs from the + * text the agent receives. A queued `/skill` invocation carries the whole + * expanded skill body as `text` — the UI shows the invocation instead. */ + displayText?: string attachments: ComposerAttachment[] queuedAt: number } @@ -110,7 +114,7 @@ export const getQueuedPrompts = (key: string | null | undefined): QueuedPromptEn export const enqueueQueuedPrompt = ( key: string | null | undefined, - payload: { text: string; attachments: ComposerAttachment[] } + payload: { text: string; attachments: ComposerAttachment[]; displayText?: string } ): null | QueuedPromptEntry => { const sid = sidOf(key) @@ -121,6 +125,7 @@ export const enqueueQueuedPrompt = ( const entry: QueuedPromptEntry = { id: nextId(), text: payload.text, + ...(payload.displayText ? { displayText: payload.displayText } : {}), attachments: cloneAttachments(payload.attachments), queuedAt: Date.now() } @@ -218,7 +223,12 @@ export const updateQueuedPrompt = ( changed = true - return { ...entry, text: update.text, attachments } + // The user rewrote the text, so any display projection it carried (a + // `/skill` invocation standing in for the expanded body) no longer + // describes it — what they typed is now what sends. + const { displayText: _dropped, ...rest } = entry + + return { ...rest, text: update.text, attachments } }) if (!changed) { diff --git a/apps/shared/src/index.ts b/apps/shared/src/index.ts index 21c40a716db..391a3715bd2 100644 --- a/apps/shared/src/index.ts +++ b/apps/shared/src/index.ts @@ -44,6 +44,7 @@ export { JsonRpcGatewayClient, type WebSocketLike } from './json-rpc-gateway' +export { skillInvocationText } from './skill-scaffold' export { type HermesSkin, SKIN_BRANDING_TOKENS, diff --git a/apps/shared/src/skill-scaffold.test.ts b/apps/shared/src/skill-scaffold.test.ts new file mode 100644 index 00000000000..954ff41a6c9 --- /dev/null +++ b/apps/shared/src/skill-scaffold.test.ts @@ -0,0 +1,61 @@ +import { describe, expect, it } from 'vitest' + +import { skillInvocationText } from './skill-scaffold' + +// Byte-identical to what agent/skill_commands.py emits — a desktop/TUI talking +// to an older gateway sees exactly these strings. +const BODY = 'SPIN UP A WORKTREE. Never edit the primary checkout.\n'.repeat(20) + +const singleSkill = (instruction?: string) => + [ + '[IMPORTANT: The user has invoked the "work" skill, indicating they want you to follow its instructions.', + 'The full skill content is loaded below.]', + '', + BODY, + '', + '[Skill directory: /Users/x/skills/work]', + ...(instruction + ? ['', `The user has provided the following instruction alongside the skill invocation: ${instruction}`] + : []) + ].join('\n') + +const bundle = (instruction?: string) => + [ + '[IMPORTANT: The user has invoked the "/clean /work" stacked skill bundle, loading 2 skills together.]', + '', + 'Skills loaded: clean, work', + ...(instruction ? ['', `User instruction: ${instruction}`] : []), + '', + '[Loaded as part of the stacked skill invocation "clean".]', + '', + BODY + ].join('\n') + +describe('skillInvocationText', () => { + it('renders a single-skill turn as the invocation, never the body', () => { + const projected = skillInvocationText(singleSkill('fix the title leak')) + + expect(projected).toBe('/work fix the title leak') + expect(projected).not.toContain('WORKTREE') + }) + + it('renders a bare invocation as just the command', () => { + expect(skillInvocationText(singleSkill())).toBe('/work') + }) + + it('renders a bundle turn as the typed keys plus the instruction', () => { + const projected = skillInvocationText(bundle('ship it')) + + expect(projected).toBe('/clean /work ship it') + expect(projected).not.toContain('WORKTREE') + }) + + it('collapses newlines in a multi-line instruction so the bubble stays one line', () => { + expect(skillInvocationText(singleSkill('fix the leak\n\nthen ship'))).toBe('/work fix the leak then ship') + }) + + it('leaves ordinary user prose alone', () => { + expect(skillInvocationText('just a normal message')).toBeNull() + expect(skillInvocationText('[IMPORTANT: read the docs]')).toBeNull() + }) +}) diff --git a/apps/shared/src/skill-scaffold.ts b/apps/shared/src/skill-scaffold.ts new file mode 100644 index 00000000000..fffae405b2a --- /dev/null +++ b/apps/shared/src/skill-scaffold.ts @@ -0,0 +1,69 @@ +/** + * A `/skill` invocation expands into a model-facing message that embeds the + * whole skill body. That payload is for the agent — the UI shows the + * invocation the user typed (`/work fix the leak`) and nothing else. + * + * The gateway already projects this (see `_skill_scaffold_projection` in + * tui_gateway/server.py) and ships the result as `display` on a dispatch and + * as the `text` of a `skill_invocation` history row. This module is the + * client-side twin so a desktop/TUI talking to an older gateway — or any + * future path that hands raw scaffolding to a bubble — still renders the + * invocation instead of the body. + * + * The markers below mirror `agent/skill_commands.py` byte for byte. + */ + +const INVOCATION_PREFIX = '[IMPORTANT: The user has invoked the ' +const SINGLE_MARKER = 'The full skill content is loaded below.]' +const SINGLE_INSTRUCTION = 'The user has provided the following instruction alongside the skill invocation: ' +const RUNTIME_NOTE = '\n\n[Runtime note:' +const BUNDLE_MARKER = ' skill bundle,' +const BUNDLE_INSTRUCTION = '\nUser instruction: ' +const BUNDLE_SKILL_BLOCK = '\n\n[Loaded as part of the ' + +// The skill name is the first quoted span of the activation note, for both the +// single-skill (`work`) and the bundle (`/clean /work`) header. +const NAME_RE = new RegExp(`^${INVOCATION_PREFIX.replace(/[[\]]/g, '\\$&')}"([^"]*)"`) + +/** Text between `marker` and `end`, or '' when the marker is absent. */ +function between(text: string, marker: string, end: string, fromEnd = false): string { + const index = fromEnd ? text.lastIndexOf(marker) : text.indexOf(marker) + + if (index < 0) { + return '' + } + + const tail = text.slice(index + marker.length) + const stop = tail.indexOf(end) + + return (stop >= 0 ? tail.slice(0, stop) : tail).trim() +} + +/** + * The invocation a scaffolded turn came from (`/work fix the leak`), or null + * when `text` is ordinary user prose that should render as written. + */ +export function skillInvocationText(text: string): null | string { + if (!text.startsWith(INVOCATION_PREFIX)) { + return null + } + + const name = (NAME_RE.exec(text)?.[1] ?? '').trim() + + if (!name) { + return null + } + + // Bundle headers already carry their typed "/a /b" keys; a single skill is + // a bare name. The single-skill instruction trails the body (which may quote + // the marker), so match it from the end. + const label = name.startsWith('/') ? name : `/${name}` + + const instruction = text.includes(BUNDLE_MARKER) + ? between(text, BUNDLE_INSTRUCTION, BUNDLE_SKILL_BLOCK) + : text.includes(SINGLE_MARKER) + ? between(text, SINGLE_INSTRUCTION, RUNTIME_NOTE, true) + : '' + + return instruction ? `${label} ${instruction.replace(/\s+/g, ' ')}` : label +} diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index 1425e6dd75e..7e638493020 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -1753,6 +1753,93 @@ def test_history_to_messages_drops_display_hidden_scaffolding(): assert all("api_content" not in m for m in projected) +def test_history_to_messages_projects_a_skill_turn_to_its_invocation(): + # A /skill invocation is persisted EXPANDED: the activation note plus the + # entire skill body. That payload is model-facing scaffolding -- this + # projection is the single display source every client reads, so it must + # hand back the invocation the user typed and never the body. Without it a + # chat bubble renders the whole skill as if the user had written it. + scaffolded = ( + '[IMPORTANT: The user has invoked the "work" skill, indicating they ' + "want you to follow its instructions. The full skill content is " + "loaded below.]\n\n" + "# /work\n\nSPIN UP A WORKTREE, never the primary checkout.\n\n" + "The user has provided the following instruction alongside the skill " + "invocation: fix the title leak" + ) + + history = [ + {"role": "user", "content": scaffolded}, + {"role": "assistant", "content": "on it"}, + ] + + assert server._history_to_messages(history) == [ + { + "role": "user", + "text": "/work fix the title leak", + "display_kind": "skill_invocation", + }, + {"role": "assistant", "text": "on it"}, + ] + + +def test_history_to_messages_projects_a_bare_skill_turn_to_the_command(): + scaffolded = ( + '[IMPORTANT: The user has invoked the "work" skill, indicating they ' + "want you to follow its instructions. The full skill content is " + "loaded below.]\n\n# /work\n\nSPIN UP A WORKTREE." + ) + + assert server._history_to_messages([{"role": "user", "content": scaffolded}]) == [ + {"role": "user", "text": "/work", "display_kind": "skill_invocation"} + ] + + +def test_expand_skill_invocation_for_replay_round_trips_the_projection( + tmp_path, monkeypatch +): + # Rewind/regenerate replays a turn from what the transcript SHOWS, and a + # skill turn shows its invocation. Re-running that verbatim would send the + # agent the literal "/work fix it" instead of the skill, so the server + # re-expands it — the exact inverse of _skill_scaffold_projection, with the + # body never leaving the server. + import agent.skill_commands as skill_commands + import agent.skill_utils as skill_utils + import tools.skills_tool as skills_tool + + skills_dir = tmp_path / "skills" + (skills_dir / "worktree-kickoff").mkdir(parents=True) + (skills_dir / "worktree-kickoff" / "SKILL.md").write_text( + "---\nname: worktree-kickoff\ndescription: Spin up a worktree\n---\n\n" + "# kickoff\n\nSPIN UP A WORKTREE, never the primary checkout.\n" + ) + monkeypatch.setattr(skills_tool, "SKILLS_DIR", skills_dir) + monkeypatch.setattr(skill_utils, "get_external_skills_dirs", lambda *a, **k: []) + monkeypatch.setattr(skill_commands, "_skill_commands", {}) + monkeypatch.setattr(skill_commands, "_skill_commands_platform", None) + skill_commands.scan_skill_commands() + + expanded = server._expand_skill_invocation_for_replay( + "/worktree-kickoff fix it", "task-1" + ) + + assert "SPIN UP A WORKTREE" in expanded + assert server._skill_scaffold_projection(expanded) == "/worktree-kickoff fix it" + + +def test_expand_skill_invocation_for_replay_leaves_ordinary_text_alone(monkeypatch): + import agent.skill_commands as skill_commands + import agent.skill_utils as skill_utils + + monkeypatch.setattr(skill_utils, "get_external_skills_dirs", lambda *a, **k: []) + monkeypatch.setattr(skill_commands, "_skill_commands", {}) + monkeypatch.setattr(skill_commands, "_skill_commands_platform", None) + + assert server._expand_skill_invocation_for_replay("just words", "t") == "just words" + # A core slash command is not a skill — nothing to expand. + assert server._expand_skill_invocation_for_replay("/status", "t") == "/status" + + def test_history_to_messages_types_a_legacy_auto_continue_row(): # A crash-interrupted turn used to be typed only AFTER it finished, so a # turn killed a second time (or any row written before turn-start typing diff --git a/tests/tui_gateway/test_protocol.py b/tests/tui_gateway/test_protocol.py index a32fea2768f..7e55821301f 100644 --- a/tests/tui_gateway/test_protocol.py +++ b/tests/tui_gateway/test_protocol.py @@ -1592,6 +1592,8 @@ def test_slash_exec_routes_custom_skill_bundle_away_from_worker(server): "type": "send", "message": fake_msg, "notice": "⚡ Loading bundle: analysis-pack (2 skills)", + # UIs render this invocation; `message` stays model-facing scaffolding. + "display": "/analysis-pack", } assert worker.calls == [] @@ -2018,6 +2020,8 @@ def test_command_dispatch_returns_custom_bundle_payload(server): "type": "send", "message": fake_msg, "notice": "⚡ Loading bundle: review-suite (3 skills)", + # UIs render this invocation; `message` stays model-facing scaffolding. + "display": "/review-suite", } build_bundle.assert_called_once_with( "/review-suite", diff --git a/tui_gateway/server.py b/tui_gateway/server.py index f6f7241da89..b555504dfe2 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -33,6 +33,7 @@ from hermes_cli.env_loader import load_hermes_dotenv from utils import is_truthy_value from tools.environments.local import hermes_subprocess_env from agent.replay_cleanup import sanitize_replay_history +from agent.skill_commands import describe_skill_invocation from agent.conversation_loop import INTERRUPT_WAITING_FOR_MODEL_PREFIX from tui_gateway import git_probe from tui_gateway.turn_marker import ( @@ -6237,6 +6238,51 @@ def _is_display_hidden_marker(role: str | None, text: str) -> bool: return role == "user" and text.lstrip().startswith("[System:") +def _skill_scaffold_projection(content_text: str) -> str: + """Return the invocation a slash-skill-expanded turn came from, else "". + + A ``/skill`` invocation expands into a model-facing message that embeds the + whole skill body. That payload belongs to the agent — every UI renders the + invocation (``/work fix the leak``) instead, so no surface can leak the + body into a chat bubble. + """ + return describe_skill_invocation(content_text, separator=" ") or "" + + +def _expand_skill_invocation_for_replay(text: str, task_id: str) -> str: + """Re-expand a projected `/skill` invocation before re-running that turn. + + The inverse of :func:`_skill_scaffold_projection`. Because a skill turn is + displayed as its invocation, a rewind/regenerate hands us back + ``/work fix the leak`` rather than the body the agent originally saw — + re-running that verbatim would drop the skill. Re-expanding here keeps the + body server-side (no client ever holds it) and makes the replayed turn + identical to the original. + + Returns *text* unchanged when it isn't a resolvable skill invocation. + """ + head, _, arg = (text or "").strip().partition(" ") + if not head.startswith("/"): + return text + + try: + from agent.skill_commands import ( + build_skill_invocation_message, + resolve_skill_command_key, + ) + + cmd_key = resolve_skill_command_key(head.lstrip("/")) + if cmd_key is None: + return text + + return build_skill_invocation_message(cmd_key, arg.strip(), task_id=task_id) or text + except Exception: + # A skill that no longer resolves (renamed, disabled, external dir + # gone) must not break the rewind — replay the text as typed. + logger.debug("skill re-expansion failed for replay", exc_info=True) + return text + + # Opening of the crash-recovery note synthesized by _auto_continue_note. # Matched (not just built) so a row persisted before the display type was # stamped at turn start still reads as a timeline event, and to recognize the @@ -6319,6 +6365,14 @@ def _history_to_messages(history: list[dict]) -> list[dict]: if not content_text.strip() and not has_reasoning: continue msg = {"role": role, "text": content_text} + if role == "user": + invocation = _skill_scaffold_projection(content_text) + if invocation: + # Show the invocation, never the expanded skill body. The raw + # payload stays server-side: a rewind/regenerate re-sends the + # turn by ordinal, so no client needs it. + msg["text"] = invocation + msg["display_kind"] = "skill_invocation" if role == "assistant": for key in reasoning_keys: if key in m and m.get(key) is not None: @@ -10865,6 +10919,14 @@ def _(rid, params: dict) -> dict: session, err = _sess_nowait(params, rid) if err: return err + if truncate_user_ordinal is not None and isinstance(text, str): + # A rewind/regenerate replays a turn from what the transcript shows. A + # skill turn shows its invocation, so re-expand it here — otherwise + # re-running `/work fix it` sends the agent nine literal characters + # instead of the skill it originally loaded. + text = _expand_skill_invocation_for_replay( + text, str(session.get("session_key") or "") + ) isolation_cfg = _load_dashboard_process_isolation_config() turn_isolation = _session_uses_compute_host(session, isolation_cfg) # Re-bind to the current client transport for this request. This keeps @@ -15483,6 +15545,9 @@ def _(rid, params: dict) -> dict: "type": "send", "message": msg, "notice": notice, + # UIs render this, never `message` — the expanded bundle body + # is model-facing scaffolding (see _skill_scaffold_projection). + "display": _skill_scaffold_projection(msg), }, ) @@ -15505,6 +15570,9 @@ def _(rid, params: dict) -> dict: "type": "skill", "message": msg, "name": cmds[key].get("name", name), + # UIs render this, never `message` — the expanded skill + # body is model-facing scaffolding. + "display": _skill_scaffold_projection(msg), }, ) except Exception: diff --git a/ui-tui/src/__tests__/createSlashHandler.test.ts b/ui-tui/src/__tests__/createSlashHandler.test.ts index d00646ac900..d4c8f6c44ce 100644 --- a/ui-tui/src/__tests__/createSlashHandler.test.ts +++ b/ui-tui/src/__tests__/createSlashHandler.test.ts @@ -810,8 +810,10 @@ describe('createSlashHandler', () => { expect(ctx.gateway.gw.request).not.toHaveBeenCalled() }) - it('falls through to command.dispatch for skill commands and sends the message', async () => { - const skillMessage = 'Use this skill to do X.\n\n## Steps\n1. First step' + it('falls through to command.dispatch for skill commands, sending the body but showing the invocation', async () => { + const skillMessage = + '[IMPORTANT: The user has invoked the "hermes-agent-dev" skill, indicating they want you to follow its instructions.\n' + + 'The full skill content is loaded below.]\n\nUse this skill to do X.\n\n## Steps\n1. First step' const ctx = buildCtx({ gateway: { @@ -823,7 +825,12 @@ describe('createSlashHandler', () => { } if (method === 'command.dispatch') { - return Promise.resolve({ type: 'skill', message: skillMessage, name: 'hermes-agent-dev' }) + return Promise.resolve({ + type: 'skill', + message: skillMessage, + name: 'hermes-agent-dev', + display: '/hermes-agent-dev' + }) } return Promise.resolve({}) @@ -836,9 +843,12 @@ describe('createSlashHandler', () => { const h = createSlashHandler(ctx) expect(h('/hermes-agent-dev')).toBe(true) await vi.waitFor(() => { - expect(ctx.transcript.sys).toHaveBeenCalledWith('⚡ loading skill: hermes-agent-dev') + expect(ctx.transcript.send).toHaveBeenCalledWith(skillMessage, true, '/hermes-agent-dev') }) - expect(ctx.transcript.send).toHaveBeenCalledWith(skillMessage) + // The expanded skill body is model-facing: no transcript line may carry it. + for (const [line] of ctx.transcript.sys.mock.calls) { + expect(line).not.toContain('Use this skill to do X') + } }) it('handles command.dispatch payloads returned directly by slash.exec', async () => { diff --git a/ui-tui/src/app/createSlashHandler.ts b/ui-tui/src/app/createSlashHandler.ts index dc798e842c6..f0d38257e26 100644 --- a/ui-tui/src/app/createSlashHandler.ts +++ b/ui-tui/src/app/createSlashHandler.ts @@ -104,10 +104,22 @@ export function createSlashHandler(ctx: SlashHandlerContext): (cmd: string) => b return void handler(`/${d.target}${argTail}`) } - if (d.type === 'skill') { - sys(`⚡ loading skill: ${d.name}`) + // A skill/bundle dispatch's `message` is the expanded skill body — + // model-facing scaffolding. `display` is the invocation the gateway + // projected; the transcript shows that instead. An ordinary send has no + // projection and goes through unchanged. No client-side fallback here: + // the TUI spawns its gateway from this same checkout, so the two can't + // version-skew (unlike the desktop, which can meet an older backend). + const sendDispatch = (display: string | undefined, message: string) => { + const shown = display?.trim() - return d.message?.trim() ? send(d.message) : sys(`/${parsed.name}: skill payload missing message`) + return shown ? send(message, true, shown) : send(message) + } + + if (d.type === 'skill') { + return d.message?.trim() + ? sendDispatch(d.display, d.message) + : sys(`/${parsed.name}: skill payload missing message`) } if (d.type === 'send') { @@ -115,7 +127,7 @@ export function createSlashHandler(ctx: SlashHandlerContext): (cmd: string) => b sys(d.notice) } - return d.message?.trim() ? send(d.message) : sys(`/${parsed.name}: empty message`) + return d.message?.trim() ? sendDispatch(d.display, d.message) : sys(`/${parsed.name}: empty message`) } if (d.type === 'prefill') { diff --git a/ui-tui/src/app/interfaces.ts b/ui-tui/src/app/interfaces.ts index d82fa1a90ed..1a446f69e15 100644 --- a/ui-tui/src/app/interfaces.ts +++ b/ui-tui/src/app/interfaces.ts @@ -526,7 +526,7 @@ export interface SlashHandlerContext { transcript: { page: (text: string, title?: string) => void panel: (title: string, sections: PanelSection[]) => void - send: (text: string) => void + send: (text: string, showUserMessage?: boolean, displayText?: string) => void setHistoryItems: StateSetter sys: (text: string) => void trimLastExchange: (items: Msg[]) => Msg[] diff --git a/ui-tui/src/app/submissionCore.ts b/ui-tui/src/app/submissionCore.ts index 534ef6c8f03..98b416c44d9 100644 --- a/ui-tui/src/app/submissionCore.ts +++ b/ui-tui/src/app/submissionCore.ts @@ -42,7 +42,16 @@ export function markSubmitting(): void { // Submit a ready prompt (already resolved to be neither a slash command nor a // shell escape, with a live session). Pulled out of useSubmission so the // synchronous-busy invariant above is unit-testable without React test infra. -export function submitPrompt(text: string, deps: SubmitPromptDeps, showUserMessage = true): void { +// +// `displayOverride` is what the transcript shows when it differs from what the +// agent receives — a `/skill` invocation expands into the whole skill body, and +// that scaffolding is model-facing only. +export function submitPrompt( + text: string, + deps: SubmitPromptDeps, + showUserMessage = true, + displayOverride?: string +): void { const sid = getUiState().sid if (!sid) { @@ -63,7 +72,7 @@ export function submitPrompt(text: string, deps: SubmitPromptDeps, showUserMessa deps.setLastUserMsg(text) if (show) { - deps.appendMessage({ role: 'user', text: displayText }) + deps.appendMessage({ role: 'user', text: displayOverride || displayText }) } patchUiState({ busy: true, status: 'running…' }) diff --git a/ui-tui/src/app/useSubmission.ts b/ui-tui/src/app/useSubmission.ts index a70b1fd7390..0ced5f0b8a2 100644 --- a/ui-tui/src/app/useSubmission.ts +++ b/ui-tui/src/app/useSubmission.ts @@ -67,7 +67,7 @@ export function useSubmission(opts: UseSubmissionOptions) { }, [composerState.input, composerState.inputBuf]) const send = useCallback( - (text: string, showUserMessage = true) => { + (text: string, showUserMessage = true, displayText?: string) => { const expand = expandSnips(composerState.pasteSnips) submitPrompt( @@ -80,7 +80,8 @@ export function useSubmission(opts: UseSubmissionOptions) { setLastUserMsg, sys }, - showUserMessage + showUserMessage, + displayText ) }, [appendMessage, composerActions, composerState.pasteSnips, gw, setLastUserMsg, sys] diff --git a/ui-tui/src/gatewayTypes.ts b/ui-tui/src/gatewayTypes.ts index 63219a1ab71..41c5063295e 100644 --- a/ui-tui/src/gatewayTypes.ts +++ b/ui-tui/src/gatewayTypes.ts @@ -70,8 +70,8 @@ export type { export type CommandDispatchResponse = | { output?: string; type: 'exec' | 'plugin' } | { target: string; type: 'alias' } - | { message?: string; name: string; type: 'skill' } - | { message: string; notice?: string; type: 'send' } + | { display?: string; message?: string; name: string; type: 'skill' } + | { display?: string; message: string; notice?: string; type: 'send' } | { message: string; notice?: string; type: 'prefill' } // ── Config ─────────────────────────────────────────────────────────── diff --git a/ui-tui/src/lib/rpc.ts b/ui-tui/src/lib/rpc.ts index fda9694ddeb..f54fb166f77 100644 --- a/ui-tui/src/lib/rpc.ts +++ b/ui-tui/src/lib/rpc.ts @@ -22,15 +22,18 @@ export const asCommandDispatch = (value: unknown): CommandDispatchResponse | nul return { type: 'alias', target: o.target } } + const str = (value: unknown) => (typeof value === 'string' ? value : undefined) + if (t === 'skill' && typeof o.name === 'string') { - return { type: 'skill', name: o.name, message: typeof o.message === 'string' ? o.message : undefined } + return { type: 'skill', name: o.name, message: str(o.message), display: str(o.display) } } if (t === 'send' && typeof o.message === 'string') { return { type: 'send', message: o.message, - notice: typeof o.notice === 'string' ? o.notice : undefined + notice: str(o.notice), + display: str(o.display) } }