From 9dd9ef0ec99a87f078f7272b4323df5440b4b3f9 Mon Sep 17 00:00:00 2001 From: Austin Pickett Date: Wed, 10 Jun 2026 20:43:22 -0400 Subject: [PATCH 01/28] fix(web): profiles page modal (#43858) * fix(web): profiles page modal * chore: drop unrelated package-lock.json changes Co-authored-by: Cursor --------- Co-authored-by: Cursor --- web/src/pages/ProfilesPage.tsx | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/web/src/pages/ProfilesPage.tsx b/web/src/pages/ProfilesPage.tsx index 463e5a4120b8..bda2515528f0 100644 --- a/web/src/pages/ProfilesPage.tsx +++ b/web/src/pages/ProfilesPage.tsx @@ -794,7 +794,7 @@ export default function ProfilesPage() {
- )) - )} -
- - ) -} 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 5ef677f4d0fe..f80e6db4385d 100644 --- a/apps/desktop/src/app/chat/composer/text-utils.test.ts +++ b/apps/desktop/src/app/chat/composer/text-utils.test.ts @@ -22,6 +22,33 @@ describe('detectTrigger', () => { it('returns null for plain text', () => { expect(detectTrigger('hello there')).toBeNull() }) + + it('keeps the slash trigger live while typing args', () => { + expect(detectTrigger('/personality ')).toEqual({ + kind: '/', + query: 'personality ', + tokenLength: 13 + }) + expect(detectTrigger('/personality alic')).toEqual({ + kind: '/', + query: 'personality alic', + tokenLength: 17 + }) + expect(detectTrigger('/tools enable foo')).toEqual({ + kind: '/', + query: 'tools enable foo', + tokenLength: 17 + }) + }) + + it('does not treat file-style paths as slash triggers', () => { + expect(detectTrigger('src/foo/bar')).toBeNull() + expect(detectTrigger('/path/to/file')).toBeNull() + }) + + it('still anchors at-mention triggers strictly at the token edge', () => { + expect(detectTrigger('@file:path with space')).toBeNull() + }) }) describe('extractClipboardImageBlobs', () => { diff --git a/apps/desktop/src/app/chat/composer/text-utils.ts b/apps/desktop/src/app/chat/composer/text-utils.ts index e9a8fb6aaee7..4535d6963c3a 100644 --- a/apps/desktop/src/app/chat/composer/text-utils.ts +++ b/apps/desktop/src/app/chat/composer/text-utils.ts @@ -6,7 +6,13 @@ export interface TriggerState { tokenLength: number } -const TRIGGER_RE = /(?:^|[\s])([@/])([^\s@/]*)$/ +// `@` 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`. +const AT_TRIGGER_RE = /(?:^|[\s])(@)([^\s@/]*)$/ +const SLASH_TRIGGER_RE = /(?:^|[\s])(\/)((?:[a-zA-Z][\w-]*(?:\s+\S*)*)?)$/ /** Stable key for paste dedupe — `items` and `files` often mirror the same image as different objects. */ export function blobDedupeKey(blob: Blob): string { @@ -97,11 +103,17 @@ export function textBeforeCaret(editor: HTMLDivElement): string | null { } export function detectTrigger(textBefore: string): TriggerState | null { - const match = TRIGGER_RE.exec(textBefore) + const slash = SLASH_TRIGGER_RE.exec(textBefore) - if (!match) { - return null + if (slash) { + return { kind: '/', query: slash[2], tokenLength: 1 + slash[2].length } } - return { kind: match[1] as '@' | '/', query: match[2], tokenLength: 1 + match[2].length } + const at = AT_TRIGGER_RE.exec(textBefore) + + if (at) { + return { kind: '@', query: at[2], tokenLength: 1 + at[2].length } + } + + return null } diff --git a/apps/desktop/src/app/chat/composer/trigger-popover.test.tsx b/apps/desktop/src/app/chat/composer/trigger-popover.test.tsx index 9acc43f7f198..3aefbfee0a5b 100644 --- a/apps/desktop/src/app/chat/composer/trigger-popover.test.tsx +++ b/apps/desktop/src/app/chat/composer/trigger-popover.test.tsx @@ -34,9 +34,17 @@ describe('ComposerTriggerPopover i18n', () => { }) it('renders localized loading copy for slash commands', () => { - const { container } = renderPopover('/', true) + renderPopover('/', true) + // While loading the popover shows only the spinner + loading copy — the + // `/help` empty-state hint is reserved for the resolved (not-loading) state. expect(screen.getByText('查找中…')).toBeTruthy() + }) + + it('renders the slash empty-state hint when not loading', () => { + const { container } = renderPopover('/') + + expect(screen.getByText('没有匹配项。')).toBeTruthy() expect(container.textContent).toContain('/help') }) }) diff --git a/apps/desktop/src/app/chat/composer/trigger-popover.tsx b/apps/desktop/src/app/chat/composer/trigger-popover.tsx index a09190dd6b3c..1099c0748ba3 100644 --- a/apps/desktop/src/app/chat/composer/trigger-popover.tsx +++ b/apps/desktop/src/app/chat/composer/trigger-popover.tsx @@ -1,5 +1,7 @@ import type { Unstable_TriggerItem } from '@assistant-ui/core' +import { Fragment } from 'react' +import { BrailleSpinner } from '@/components/ui/braille-spinner' import { Codicon } from '@/components/ui/codicon' import { useI18n } from '@/i18n' import { cn } from '@/lib/utils' @@ -7,7 +9,6 @@ import { cn } from '@/lib/utils' import { COMPLETION_DRAWER_BELOW_CLASS, COMPLETION_DRAWER_CLASS, - COMPLETION_DRAWER_ROW_CLASS, CompletionDrawerEmpty } from './completion-drawer' @@ -23,11 +24,7 @@ const AT_ICON_BY_TYPE: Record = { url: 'globe' } -function completionIcon(kind: '@' | '/', item: Unstable_TriggerItem) { - if (kind === '/') { - return 'terminal' - } - +function atIcon(item: Unstable_TriggerItem) { const meta = item.metadata as { rawText?: string } | undefined const raw = meta?.rawText || item.label @@ -42,6 +39,18 @@ function completionIcon(kind: '@' | '/', item: Unstable_TriggerItem) { return AT_ICON_BY_TYPE[item.type] || AT_ICON_BY_TYPE.simple } +interface RowMeta { + display?: string + group?: string + meta?: string +} + +const ROW_BASE_CLASS = [ + 'relative flex w-full cursor-default select-none rounded-md px-2 py-1 text-left', + 'outline-hidden transition-colors hover:bg-(--ui-bg-tertiary)', + 'data-[highlighted]:bg-(--ui-bg-tertiary) data-[highlighted]:text-foreground' +].join(' ') + interface ComposerTriggerPopoverProps { activeIndex: number items: readonly Unstable_TriggerItem[] @@ -63,6 +72,9 @@ export function ComposerTriggerPopover({ }: ComposerTriggerPopoverProps) { const { t } = useI18n() const copy = t.composer + const isSlash = kind === '/' + + let lastGroup: string | undefined return (
{items.length === 0 ? ( - - {kind === '@' ? ( - <> - {copy.lookupTry} @file: {copy.lookupOr}{' '} - @folder:. - - ) : ( - <> - {copy.lookupTry} /help. - - )} - + loading ? ( +
+ + {copy.lookupLoading} +
+ ) : ( + + {kind === '@' ? ( + <> + {copy.lookupTry} @file: {copy.lookupOr}{' '} + @folder:. + + ) : ( + <> + {copy.lookupTry} /help. + + )} + + ) ) : ( items.map((item, index) => { - const meta = item.metadata as { display?: string; meta?: string } | undefined - const display = meta?.display ?? (kind === '/' ? `/${item.label}` : item.label) + const meta = item.metadata as RowMeta | undefined + const display = meta?.display ?? (isSlash ? `/${item.label}` : item.label) const description = meta?.meta || item.description + const group = meta?.group?.trim() + const showHeader = isSlash && Boolean(group) && group !== lastGroup + const isFirstHeader = lastGroup === undefined + lastGroup = group || lastGroup + const active = index === activeIndex return ( - + + ) }) )} diff --git a/apps/desktop/src/app/desktop-controller.tsx b/apps/desktop/src/app/desktop-controller.tsx index ab4f3f0eb0e7..0da266395449 100644 --- a/apps/desktop/src/app/desktop-controller.tsx +++ b/apps/desktop/src/app/desktop-controller.tsx @@ -98,6 +98,7 @@ import { RightSidebarPane } from './right-sidebar' import { $terminalTakeover } from './right-sidebar/store' import { PersistentTerminal, TerminalSlot } from './right-sidebar/terminal/persistent' import { CRON_ROUTE, NEW_CHAT_ROUTE, routeSessionId, sessionRoute, SETTINGS_ROUTE } from './routes' +import { SessionPickerOverlay } from './session-picker-overlay' import { SessionSwitcher } from './session-switcher' import { useContextSuggestions } from './session/hooks/use-context-suggestions' import { useCwdActions } from './session/hooks/use-cwd-actions' @@ -694,6 +695,7 @@ export function DesktopController() { handleSkinCommand, refreshSessions, requestGateway, + resumeStoredSession: resumeSession, selectedStoredSessionIdRef, startFreshSessionDraft, sttEnabled, @@ -829,6 +831,7 @@ export function DesktopController() { /> )} + diff --git a/apps/desktop/src/app/session-picker-overlay.tsx b/apps/desktop/src/app/session-picker-overlay.tsx new file mode 100644 index 000000000000..65344fcac26b --- /dev/null +++ b/apps/desktop/src/app/session-picker-overlay.tsx @@ -0,0 +1,32 @@ +import { useStore } from '@nanostores/react' + +import { SessionPickerDialog } from '@/components/session-picker' +import { $gatewayState, $selectedStoredSessionId, $sessionPickerOpen, setSessionPickerOpen } from '@/store/session' + +interface SessionPickerOverlayProps { + onResume: (storedSessionId: string) => void +} + +/** + * Mounts the session picker that `/resume` (and `/sessions`, `/switch`) opens — + * the desktop equivalent of the TUI's sessions overlay. Resuming runs through + * the same `resumeSession` path the sidebar uses. + */ +export function SessionPickerOverlay({ onResume }: SessionPickerOverlayProps) { + const open = useStore($sessionPickerOpen) + const gatewayOpen = useStore($gatewayState) === 'open' + const activeStoredSessionId = useStore($selectedStoredSessionId) + + if (!gatewayOpen) { + return null + } + + return ( + + ) +} diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions.test.tsx b/apps/desktop/src/app/session/hooks/use-prompt-actions.test.tsx index 96af1e8400eb..3418e0bad807 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions.test.tsx +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions.test.tsx @@ -1,6 +1,6 @@ import { cleanup, render, waitFor } from '@testing-library/react' import type { MutableRefObject } from 'react' -import { useEffect } from 'react' +import { useEffect, useRef } from 'react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { $composerAttachments, type ComposerAttachment } from '@/store/composer' @@ -55,6 +55,7 @@ function Harness({ onSeedState, refreshSessions, requestGateway, + resumeStoredSession, storedSessionId }: { busyRef?: MutableRefObject @@ -62,6 +63,7 @@ function Harness({ onSeedState?: (state: Record) => void refreshSessions: () => Promise requestGateway: (method: string, params?: Record) => Promise + resumeStoredSession?: (storedSessionId: string) => Promise | void storedSessionId?: null | string }) { const activeSessionIdRef: MutableRefObject = { current: RUNTIME_SESSION_ID } @@ -69,6 +71,12 @@ function Harness({ current: storedSessionId === undefined ? RUNTIME_SESSION_ID : storedSessionId } const localBusyRef = busyRef ?? { current: false } + const stateRef = useRef({ + messages: [], + busy: false, + awaitingResponse: false, + interrupted: true + } as never) const actions = usePromptActions({ activeSessionId: RUNTIME_SESSION_ID, @@ -79,17 +87,14 @@ function Harness({ handleSkinCommand: () => '', refreshSessions, requestGateway, + resumeStoredSession: resumeStoredSession ?? (() => undefined), selectedStoredSessionIdRef, startFreshSessionDraft: () => undefined, sttEnabled: false, updateSessionState: (_sessionId, updater) => { // Seed with interrupted:true so we can prove a fresh submit clears it. - const next = updater({ - messages: [], - busy: false, - awaitingResponse: false, - interrupted: true - } as never) as unknown as Record + const next = updater(stateRef.current) as unknown as Record + stateRef.current = next as never onSeedState?.(next) return next as never @@ -190,6 +195,68 @@ describe('usePromptActions /title', () => { }) }) +describe('usePromptActions desktop slash pickers', () => { + beforeEach(() => { + setSessions(() => [sessionInfo({ id: '20260610_120000_abcdef', title: 'Loaded session' })]) + }) + + afterEach(() => { + cleanup() + vi.useRealTimers() + vi.restoreAllMocks() + }) + + it('resumes an exact session id even when it is not in the loaded sidebar cache', async () => { + const resumeStoredSession = vi.fn(async () => undefined) + const requestGateway = vi.fn(async () => ({}) as never) + + let handle: HarnessHandle | null = null + render( + (handle = h)} + refreshSessions={async () => undefined} + requestGateway={requestGateway} + resumeStoredSession={resumeStoredSession} + /> + ) + + await handle!.submitText('/resume 20260610_130000_123abc') + + expect(resumeStoredSession).toHaveBeenCalledWith('20260610_130000_123abc') + expect(requestGateway).not.toHaveBeenCalledWith('slash.exec', expect.anything()) + }) + + it('marks a timed-out handoff as failed so the next attempt can retry', async () => { + vi.useFakeTimers() + const calls: { method: string; params?: Record }[] = [] + const requestGateway = vi.fn(async (method: string, params?: Record) => { + calls.push({ method, params }) + + if (method === 'handoff.state') { + return { state: 'pending' } as never + } + + return {} as never + }) + + let handle: HarnessHandle | null = null + render( (handle = h)} refreshSessions={async () => undefined} requestGateway={requestGateway} />) + + const result = handle!.submitText('/handoff telegram') + await vi.advanceTimersByTimeAsync(61_000) + await result + + expect(calls.some(call => call.method === 'handoff.request')).toBe(true) + expect(calls).toContainEqual({ + method: 'handoff.fail', + params: { + error: expect.stringContaining('Timed out'), + session_id: RUNTIME_SESSION_ID + } + }) + }) +}) + describe('usePromptActions submit / queue drain semantics', () => { afterEach(() => { cleanup() diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions.ts index 167f0d3224fa..15831bb41891 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions.ts +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions.ts @@ -4,20 +4,24 @@ import { type MutableRefObject, useCallback, useEffect, useRef } from 'react' import { getProfiles, transcribeAudio } from '@/hermes' import { translateNow, type Translations, useI18n } from '@/i18n' +import { stripAnsi } from '@/lib/ansi' import { branchGroupForUser, type ChatMessage, chatMessageText, textPart } from '@/lib/chat-messages' import { optimisticAttachmentRef, parseCommandDispatch, parseSlashCommand, pathLabel, + sessionTitle, SLASH_COMMAND_RE } from '@/lib/chat-runtime' import { type CommandsCatalogLike, + type DesktopActionId, + type DesktopPickerId, desktopSlashUnavailableMessage, filterDesktopCommandsCatalog, isDesktopSlashCommand, - isModelPickerCommand + resolveDesktopCommand } from '@/lib/desktop-slash-commands' import { triggerHaptic } from '@/lib/haptics' import { setMutableRef } from '@/lib/mutable-ref' @@ -38,11 +42,13 @@ import { $busy, $connection, $messages, + $sessions, $yoloActive, setAwaitingResponse, setBusy, setMessages, setModelPickerOpen, + setSessionPickerOpen, setSessions, setYoloActive } from '@/store/session' @@ -50,12 +56,30 @@ import { import type { ClientSessionState, FileAttachResponse, + HandoffFailResponse, + HandoffRequestResponse, + HandoffStateResponse, ImageAttachResponse, SessionSteerResponse, SessionTitleResponse, SlashExecResponse } from '../../types' +interface HandoffResult { + ok: boolean + error?: string +} + +function delay(ms: number): Promise { + return new Promise(resolve => setTimeout(resolve, ms)) +} + +function isSessionIdCandidate(value: string): boolean { + const trimmed = value.trim() + + return /^\d{8}_\d{6}_[A-Fa-f0-9]{6}$/.test(trimmed) || /^[A-Fa-f0-9]{32}$/.test(trimmed) +} + function blobToDataUrl(blob: Blob): Promise { return new Promise((resolve, reject) => { const reader = new FileReader() @@ -245,6 +269,7 @@ interface PromptActionsOptions { handleSkinCommand: (arg: string) => string refreshSessions: () => Promise requestGateway: (method: string, params?: Record) => Promise + resumeStoredSession: (storedSessionId: string) => Promise | void selectedStoredSessionIdRef: MutableRefObject startFreshSessionDraft: () => void sttEnabled: boolean @@ -260,6 +285,15 @@ interface SubmitTextOptions { fromQueue?: boolean } +/** Everything a slash handler needs about the invocation it's serving. */ +interface SlashActionCtx { + arg: string + command: string + name: string + recordInput: boolean + sessionHint?: string +} + function renderCommandsCatalog(catalog: CommandsCatalogLike, copy: Translations['desktop']): string { const desktopCatalog = filterDesktopCommandsCatalog(catalog) @@ -310,6 +344,7 @@ export function usePromptActions({ handleSkinCommand, refreshSessions, requestGateway, + resumeStoredSession, selectedStoredSessionIdRef, startFreshSessionDraft, sttEnabled, @@ -320,7 +355,11 @@ export function usePromptActions({ const appendSessionTextMessage = useCallback( (sessionId: string, role: ChatMessage['role'], text: string) => { - const body = text.trim() + // Strip ANSI: slash-command output from the backend worker carries SGR + // color codes (e.g. "Unknown command" in red). The ESC byte is invisible + // in the chat panel, so without this the `[1;31m…[0m` payload leaks as + // literal text. + const body = stripAnsi(text).trim() if (!body) { return @@ -696,230 +735,124 @@ export function usePromptActions({ ] ) + // Queue a handoff of this session to a messaging platform and watch it to + // a terminal state. We only write the request through the gateway; the + // separate `hermes gateway` process performs the actual transfer, so we + // poll `handoff.state` (mirror of the CLI's block-poll) for the result. + const handoffSession = useCallback( + async ( + platform: string, + options?: { onProgress?: (state: string) => void; sessionId?: string } + ): Promise => { + const sid = options?.sessionId || activeSessionIdRef.current + + if (!sid) { + return { error: copy.sessionUnavailable, ok: false } + } + + const target = platform.trim().toLowerCase() + + if (!target) { + return { error: copy.handoff.failed(''), ok: false } + } + + try { + options?.onProgress?.('pending') + await requestGateway('handoff.request', { + platform: target, + session_id: sid + }) + } catch (err) { + return { error: inlineErrorMessage(err, copy.handoff.failed(target)), ok: false } + } + + const deadline = Date.now() + 60_000 + let lastState = 'pending' + + while (Date.now() < deadline) { + await delay(800) + + let record: HandoffStateResponse + + try { + record = await requestGateway('handoff.state', { session_id: sid }) + } catch { + continue + } + + const state = record.state || 'pending' + + if (state !== lastState) { + options?.onProgress?.(state) + lastState = state + } + + if (state === 'completed') { + appendSessionTextMessage(sid, 'system', copy.handoff.systemNote(target)) + notify({ kind: 'success', message: copy.handoff.success(target) }) + + return { ok: true } + } + + if (state === 'failed') { + return { error: record.error || copy.handoff.failed(target), ok: false } + } + } + + const cleanup = await requestGateway('handoff.fail', { + error: copy.handoff.timedOut, + session_id: sid + }).catch(() => null) + + if (cleanup?.state === 'completed') { + appendSessionTextMessage(sid, 'system', copy.handoff.systemNote(target)) + notify({ kind: 'success', message: copy.handoff.success(target) }) + + return { ok: true } + } + + return { error: copy.handoff.timedOut, ok: false } + }, + [activeSessionIdRef, appendSessionTextMessage, copy, requestGateway] + ) + const executeSlashCommand = useCallback( async (rawCommand: string, options?: { sessionId?: string; recordInput?: boolean }) => { - const runSlash = async (commandText: string, sessionHint?: string, recordInput = true): Promise => { - const command = commandText.trim() - const { name, arg } = parseSlashCommand(command) - const normalizedName = name.toLowerCase() + const ensureSessionId = async (sessionHint?: string) => + sessionHint || activeSessionIdRef.current || (await createBackendSessionForSend()) - if (!name) { - const sessionId = sessionHint || activeSessionIdRef.current || (await createBackendSessionForSend()) - - if (sessionId) { - appendSessionTextMessage(sessionId, 'system', copy.emptySlashCommand) - } - - return - } - - if (normalizedName === 'new' || normalizedName === 'reset') { - startFreshSessionDraft() - - return - } - - if (normalizedName === 'branch' || normalizedName === 'fork') { - await branchCurrentSession() - - return - } - - // /yolo maps to the status-bar YOLO control — a per-session approval - // bypass, same scope as the TUI's Shift+Tab. With no session yet we arm - // it locally; the session-create path applies it on the first message. - if (normalizedName === 'yolo') { - const sid = sessionHint || activeSessionIdRef.current - const next = !$yoloActive.get() - - if (!sid) { - setYoloActive(next) - notify({ kind: 'success', message: next ? copy.yoloArmed : copy.yoloOff }) - - return - } - - try { - const active = await setSessionYolo(requestGateway, sid, next) - appendSessionTextMessage(sid, 'system', copy.yoloSystem(active)) - } catch { - notify({ kind: 'error', title: copy.yoloTitle, message: copy.yoloToggleFailed }) - } - - return - } - - // /model opens the desktop model picker overlay — the same full - // provider+model picker reachable from the status-bar model button — - // instead of the headless prompt_toolkit modal the slash worker can't - // render. With explicit args (`/model [--provider ...]`) run the - // switch directly through slash.exec so power users can still type it. - if (isModelPickerCommand(`/${normalizedName}`)) { - if (!arg.trim()) { - setModelPickerOpen(true) - - return - } - - const sid = sessionHint || activeSessionIdRef.current || (await createBackendSessionForSend()) - - if (!sid) { - notify({ kind: 'error', title: 'Session unavailable', message: 'Could not create a new session' }) - - return - } - - try { - const result = await requestGateway('slash.exec', { - session_id: sid, - command: command.replace(/^\/+/, '') - }) - - const body = result?.output || `/${name}: model switched` - appendSessionTextMessage( - sid, - 'system', - recordInput ? slashStatusText(command, body) : body - ) - } catch (err) { - appendSessionTextMessage( - sid, - 'system', - `error: ${err instanceof Error ? err.message : String(err)}` - ) - } - - return - } - - if (normalizedName === 'skin' && !sessionHint && !activeSessionIdRef.current) { - notify({ kind: 'success', message: handleSkinCommand(arg) }) - - return - } - - // /profile selects which profile new chats open in — no app relaunch. - // A profile is per-session now, so an existing thread can't change its - // profile mid-stream; `/profile ` instead points the next new chat - // (and the current empty draft) at that profile's backend. - if (normalizedName === 'profile') { - const target = arg.trim() - const current = normalizeProfileKey($activeGatewayProfile.get()) - - if (!target) { - notify({ - kind: 'success', - message: copy.profileStatus(current) - }) - - return - } - - try { - const { profiles } = await getProfiles() - const match = profiles.find(profile => profile.name === target) - - if (!match) { - notify({ - kind: 'error', - title: copy.unknownProfile, - message: copy.noProfileNamed(target, profiles.map(profile => profile.name).join(', ')) - }) - - return - } - - const key = normalizeProfileKey(match.name) - - $newChatProfile.set(key) - // Swap the live gateway now so an empty draft sends into this - // profile immediately; an existing thread keeps its own profile. - await ensureGatewayProfile(key) - notify({ kind: 'success', message: copy.newChatsProfile(match.name) }) - } catch (err) { - notifyError(err, copy.setProfileFailed) - } - - return - } - - const sessionId = sessionHint || activeSessionIdRef.current || (await createBackendSessionForSend()) + // Resolve the target session plus a writer for inline slash output, or + // notify + return null when none can be created. Folds the ensure / bail / + // build-renderSlashOutput boilerplate every exec-style handler repeats. + const withSlashOutput = async ( + ctx: SlashActionCtx + ): Promise<{ render: (text: string) => void; sessionId: string } | null> => { + const sessionId = await ensureSessionId(ctx.sessionHint) if (!sessionId) { - notify({ - kind: 'error', - title: copy.sessionUnavailable, - message: copy.createSessionFailed - }) + notify({ kind: 'error', title: copy.sessionUnavailable, message: copy.createSessionFailed }) + return null + } + + const render = (text: string) => + appendSessionTextMessage(sessionId, 'system', ctx.recordInput ? slashStatusText(ctx.command, text) : text) + + return { render, sessionId } + } + + // `exec` commands (and unknown skill / quick commands the backend owns) + // run on the gateway and render their text output inline. This is the only + // path that talks to slash.exec / command.dispatch. + async function runExec(ctx: SlashActionCtx): Promise { + const { arg, command, name } = ctx + const resolved = await withSlashOutput(ctx) + + if (!resolved) { return } - const renderSlashOutput = (text: string) => - appendSessionTextMessage(sessionId, 'system', recordInput ? slashStatusText(command, text) : text) - - // /title renames the session. Route through the gateway's - // `session.title` RPC — the same path the TUI uses — NOT the REST - // renameSession endpoint and NOT the slash worker. - // - // Why not the slash worker: it's a separate HermesCLI subprocess whose - // SQLite write to the shared state.db can silently fail (notably on - // Windows), and it never refreshes the sidebar. - // - // Why not REST renameSession: `sessionId` here is the *runtime* session - // id returned by session.create — it is NOT the stored DB `sessions.id`, - // and session.create deliberately does not persist a DB row until the - // first turn. The REST PATCH endpoint resolves against the sessions - // table, so a runtime id (or a brand-new, not-yet-persisted session) - // 404s with "Session not found" on every platform. See #38508 / #38576. - // - // session.title maps the runtime id to the in-memory session, writes - // through the gateway's own DB connection, and QUEUES the title - // (`pending: true`) when the row isn't persisted yet — so it works for a - // fresh chat too. refreshSessions() then pulls the authoritative title - // back into the sidebar. A bare `/title` (no arg) still falls through to - // the worker to display the current title. - if (normalizedName === 'title' && arg) { - try { - const result = await requestGateway('session.title', { - session_id: sessionId, - title: arg - }) - - const finalTitle = (result?.title || arg).trim() - const queued = result?.pending === true - - setSessions(prev => prev.map(s => (s.id === sessionId ? { ...s, title: finalTitle || null } : s))) - await refreshSessions().catch(() => undefined) - renderSlashOutput( - finalTitle - ? `Session title set: ${finalTitle}${queued ? ' (queued while session initializes)' : ''}` - : 'Session title cleared.' - ) - } catch (err) { - renderSlashOutput(`error: ${err instanceof Error ? err.message : String(err)}`) - } - - return - } - - if (normalizedName === 'skin') { - renderSlashOutput(handleSkinCommand(arg)) - - return - } - - if (name === 'help' || name === 'commands') { - try { - const catalog = await requestGateway('commands.catalog', { session_id: sessionId }) - - renderSlashOutput(renderCommandsCatalog(catalog, copy)) - } catch (err) { - renderSlashOutput(`error: ${err instanceof Error ? err.message : String(err)}`) - } - - return - } + const { render: renderSlashOutput, sessionId } = resolved if (!isDesktopSlashCommand(name)) { renderSlashOutput(desktopSlashUnavailableMessage(name) || `/${name} is not available in the desktop app.`) @@ -943,11 +876,7 @@ export function usePromptActions({ try { const dispatch = parseCommandDispatch( - await requestGateway('command.dispatch', { - session_id: sessionId, - name, - arg - }) + await requestGateway('command.dispatch', { session_id: sessionId, name, arg }) ) if (!dispatch) { @@ -994,6 +923,261 @@ export function usePromptActions({ } } + // One handler per `action` command. Adding a desktop-native command is a + // registry row in desktop-slash-commands.ts plus an entry here — never a + // new branch in a dispatch ladder. + const actionHandlers: Record Promise> = { + new: async () => { + startFreshSessionDraft() + }, + branch: async () => { + await branchCurrentSession() + }, + // /yolo maps to the status-bar YOLO control — a per-session approval + // bypass, same scope as the TUI's Shift+Tab. With no session yet we arm + // it locally; the session-create path applies it on the first message. + yolo: async ({ sessionHint }) => { + const sid = sessionHint || activeSessionIdRef.current + const next = !$yoloActive.get() + + if (!sid) { + setYoloActive(next) + notify({ kind: 'success', message: next ? copy.yoloArmed : copy.yoloOff }) + + return + } + + try { + const active = await setSessionYolo(requestGateway, sid, next) + appendSessionTextMessage(sid, 'system', copy.yoloSystem(active)) + } catch { + notify({ kind: 'error', title: copy.yoloTitle, message: copy.yoloToggleFailed }) + } + }, + // /handoff hands this session to a messaging platform. The platform is + // completed inline in the slash popover (backend _handoff_completions), + // so there is no overlay: `/handoff ` runs the desktop's own + // handoff RPC. cli_only on the backend, so it must not reach slash.exec. + handoff: async ({ arg, command, recordInput, sessionHint }) => { + const platform = arg.trim() + + if (!platform) { + notify({ kind: 'success', message: copy.handoff.pickPlatform }) + + return + } + + const sid = sessionHint || activeSessionIdRef.current + + if (!sid) { + notify({ kind: 'error', title: copy.sessionUnavailable, message: copy.createSessionFailed }) + + return + } + + const result = await handoffSession(platform, { sessionId: sid }) + + if (!result.ok && result.error) { + appendSessionTextMessage(sid, 'system', recordInput ? slashStatusText(command, result.error) : result.error) + } + }, + // /profile selects which profile new chats open in — no app relaunch. + // A profile is per-session now, so an existing thread can't change its + // profile mid-stream; `/profile ` points the next new chat (and + // the current empty draft) at that profile's backend. + profile: async ({ arg }) => { + const target = arg.trim() + const current = normalizeProfileKey($activeGatewayProfile.get()) + + if (!target) { + notify({ kind: 'success', message: copy.profileStatus(current) }) + + return + } + + try { + const { profiles } = await getProfiles() + const match = profiles.find(profile => profile.name === target) + + if (!match) { + notify({ + kind: 'error', + title: copy.unknownProfile, + message: copy.noProfileNamed(target, profiles.map(profile => profile.name).join(', ')) + }) + + return + } + + const key = normalizeProfileKey(match.name) + + $newChatProfile.set(key) + await ensureGatewayProfile(key) + notify({ kind: 'success', message: copy.newChatsProfile(match.name) }) + } catch (err) { + notifyError(err, copy.setProfileFailed) + } + }, + skin: async ({ arg, command, recordInput, sessionHint }) => { + const sid = sessionHint || activeSessionIdRef.current + const message = handleSkinCommand(arg) + + // No session to print into yet — surface it as a toast instead of + // spinning up a backend session just to change the theme. + if (!sid) { + notify({ kind: 'success', message }) + + return + } + + appendSessionTextMessage(sid, 'system', recordInput ? slashStatusText(command, message) : message) + }, + // /title renames via the gateway's session.title RPC — the same + // path the TUI uses, NOT REST renameSession (which 404s on runtime ids) + // nor the slash worker (whose DB write can silently fail). Bare /title + // shows the current title, which the worker owns, so delegate to exec. + title: async ctx => { + if (!ctx.arg) { + await runExec(ctx) + + return + } + + const resolved = await withSlashOutput(ctx) + + if (!resolved) { + return + } + + const { render: renderSlashOutput, sessionId } = resolved + const { arg } = ctx + + try { + const result = await requestGateway('session.title', { + session_id: sessionId, + title: arg + }) + + const finalTitle = (result?.title || arg).trim() + const queued = result?.pending === true + + setSessions(prev => prev.map(s => (s.id === sessionId ? { ...s, title: finalTitle || null } : s))) + await refreshSessions().catch(() => undefined) + renderSlashOutput( + finalTitle + ? `Session title set: ${finalTitle}${queued ? ' (queued while session initializes)' : ''}` + : 'Session title cleared.' + ) + } catch (err) { + renderSlashOutput(`error: ${err instanceof Error ? err.message : String(err)}`) + } + }, + help: async ctx => { + const resolved = await withSlashOutput(ctx) + + if (!resolved) { + return + } + + const { render: renderSlashOutput, sessionId } = resolved + + try { + const catalog = await requestGateway('commands.catalog', { session_id: sessionId }) + + renderSlashOutput(renderCommandsCatalog(catalog, copy)) + } catch (err) { + renderSlashOutput(`error: ${err instanceof Error ? err.message : String(err)}`) + } + } + } + + // Picker commands open a desktop overlay; a typed arg is resolved by that + // picker so the command never dead-ends or falls through to the backend. + const openPicker = async (pickerId: DesktopPickerId, ctx: SlashActionCtx): Promise => { + if (pickerId === 'model') { + if (!ctx.arg.trim()) { + setModelPickerOpen(true) + + return + } + + // Power users can still type `/model ` — run it on the backend. + await runExec(ctx) + + return + } + + // session picker — /resume, /sessions, /switch + const query = ctx.arg.trim() + + if (!query) { + setSessionPickerOpen(true) + + return + } + + const sessions = $sessions.get() + const lower = query.toLowerCase() + + const match = + sessions.find(session => session.id === query) || + sessions.find(session => sessionTitle(session).toLowerCase().includes(lower)) || + sessions.find(session => (session.preview ?? '').toLowerCase().includes(lower)) + + if (!match) { + if (isSessionIdCandidate(query)) { + await resumeStoredSession(query) + + return + } + + notify({ kind: 'error', message: copy.resumeFailed }) + + return + } + + await resumeStoredSession(match.id) + } + + // The whole dispatcher: resolve the command's desktop surface, then act on + // its kind. No per-command ladder — behavior lives in the registry. + async function runSlash(commandText: string, sessionHint?: string, recordInput = true): Promise { + const command = commandText.trim() + const { name, arg } = parseSlashCommand(command) + + if (!name) { + const sessionId = await ensureSessionId(sessionHint) + + if (sessionId) { + appendSessionTextMessage(sessionId, 'system', copy.emptySlashCommand) + } + + return + } + + const ctx: SlashActionCtx = { arg, command, name, recordInput, sessionHint } + const surface = resolveDesktopCommand(`/${name}`)?.surface + + switch (surface?.kind) { + case 'unavailable': { + const resolved = await withSlashOutput(ctx) + resolved?.render(desktopSlashUnavailableMessage(name) || `/${name} is not available in the desktop app.`) + + return + } + + case 'picker': + return openPicker(surface.picker, ctx) + + case 'action': + return actionHandlers[surface.action](ctx) + + default: + // exec spec, or an unknown skill / quick command the backend owns. + return runExec(ctx) + } + } + await runSlash(rawCommand, options?.sessionId, options?.recordInput ?? true) }, [ @@ -1004,8 +1188,10 @@ export function usePromptActions({ copy, createBackendSessionForSend, handleSkinCommand, + handoffSession, refreshSessions, requestGateway, + resumeStoredSession, startFreshSessionDraft, submitPromptText ] @@ -1314,6 +1500,7 @@ export function usePromptActions({ cancelRun, editMessage, handleThreadMessagesChange, + handoffSession, reloadFromMessage, steerPrompt, submitText, diff --git a/apps/desktop/src/app/types.ts b/apps/desktop/src/app/types.ts index 672beb9a0898..01694dc82208 100644 --- a/apps/desktop/src/app/types.ts +++ b/apps/desktop/src/app/types.ts @@ -61,6 +61,26 @@ export interface SessionTitleResponse { session_key?: string } +export interface HandoffRequestResponse { + queued?: boolean + session_key?: string + platform?: string + // Human-readable home channel name for the destination platform. + home_name?: string +} + +export interface HandoffStateResponse { + // '' | 'pending' | 'running' | 'completed' | 'failed' + state?: string + platform?: string + error?: string +} + +export interface HandoffFailResponse { + failed?: boolean + state?: string +} + export interface ExecCommandDispatchResponse { type: 'exec' | 'plugin' output?: string diff --git a/apps/desktop/src/components/assistant-ui/directive-text.tsx b/apps/desktop/src/components/assistant-ui/directive-text.tsx index 79f772d450fe..b870913b012f 100644 --- a/apps/desktop/src/components/assistant-ui/directive-text.tsx +++ b/apps/desktop/src/components/assistant-ui/directive-text.tsx @@ -63,7 +63,7 @@ export function directiveIconSvg(type: string) { return `${inner}` } -export function directiveIconElement(type: string) { +function iconElementFromPaths(paths: string[]) { const svg = document.createElementNS('http://www.w3.org/2000/svg', 'svg') svg.setAttribute('class', 'size-3 shrink-0 opacity-80') svg.setAttribute('fill', 'none') @@ -74,7 +74,7 @@ export function directiveIconElement(type: string) { svg.setAttribute('viewBox', '0 0 24 24') svg.setAttribute('xmlns', 'http://www.w3.org/2000/svg') - for (const d of iconPathsFor(type)) { + for (const d of paths) { const path = document.createElementNS('http://www.w3.org/2000/svg', 'path') path.setAttribute('d', d) svg.append(path) @@ -83,6 +83,46 @@ export function directiveIconElement(type: string) { return svg } +export function directiveIconElement(type: string) { + return iconElementFromPaths(iconPathsFor(type)) +} + +/** Per-type slash-command pill styling. The composer inserts these chips when a + * command is picked; the kind drives a theme-aware accent so commands, skills, + * and themes read distinctly (Cursor-style). */ +export type SlashChipKind = 'command' | 'skill' | 'theme' + +const SLASH_ICON_PATHS: Record = { + command: ['M5 7l5 5l-5 5', 'M12 19l7 0'], + skill: ['M13 3l0 7l6 0l-8 11l0 -7l-6 0l8 -11'], + theme: [ + 'M3 21v-4a4 4 0 1 1 4 4h-4', + 'M21 3a16 16 0 0 0 -12.8 10.2', + 'M21 3a16 16 0 0 1 -10.2 12.8', + 'M10.6 9a9 9 0 0 1 4.4 4.4' + ] +} + +const SLASH_CHIP_VARIANT: Record = { + command: + 'bg-[color-mix(in_srgb,var(--ui-accent)_14%,transparent)] text-[color-mix(in_srgb,var(--ui-accent)_82%,var(--foreground))]', + skill: + 'bg-[color-mix(in_srgb,var(--ui-warm)_18%,transparent)] text-[color-mix(in_srgb,var(--ui-warm)_82%,var(--foreground))]', + theme: + 'bg-[color-mix(in_srgb,var(--ui-accent-secondary)_16%,transparent)] text-[color-mix(in_srgb,var(--ui-accent-secondary)_82%,var(--foreground))]' +} + +export const SLASH_CHIP_BASE_CLASS = + 'mx-0.5 inline-flex max-w-64 items-center gap-1 rounded px-1.5 py-0.5 align-middle text-[0.86em] font-medium leading-none' + +export function slashChipClass(kind: SlashChipKind): string { + return `${SLASH_CHIP_BASE_CLASS} ${SLASH_CHIP_VARIANT[kind]}` +} + +export function slashIconElement(kind: SlashChipKind) { + return iconElementFromPaths(SLASH_ICON_PATHS[kind]) +} + const DirectiveIcon: FC<{ type: string }> = ({ type }) => ( { const slashStatus = text.match(SLASH_STATUS_RE) if (slashStatus?.groups) { + const output = slashStatus.groups.output.trim() + // Single-line status (e.g. "model → x") reads best centered inline; padded + // multiline output (catalogs, usage tables) needs left-aligned, wider room + // or the column alignment breaks. + const multiline = output.includes('\n') + return ( {slashStatus.groups.command} - · - + {multiline ? ( + + ) : ( + <> + · + + + )} ) } + const multiline = text.includes('\n') + return ( diff --git a/apps/desktop/src/components/session-picker.tsx b/apps/desktop/src/components/session-picker.tsx new file mode 100644 index 000000000000..048fa32a2089 --- /dev/null +++ b/apps/desktop/src/components/session-picker.tsx @@ -0,0 +1,108 @@ +import { useQuery } from '@tanstack/react-query' +import { Dialog as DialogPrimitive } from 'radix-ui' +import { useEffect, useMemo, useState } from 'react' + +import { Command, CommandEmpty, CommandGroup, CommandInput, CommandItem, CommandList } from '@/components/ui/command' +import { listSessions } from '@/hermes' +import { useI18n } from '@/i18n' +import { sessionTitle } from '@/lib/chat-runtime' +import { Check, MessageCircle } from '@/lib/icons' +import { cn } from '@/lib/utils' + +interface SessionPickerDialogProps { + /** Stored id of the session currently open, so it can be flagged in the list. */ + activeStoredSessionId?: string | null + onOpenChange: (open: boolean) => void + onResume: (storedSessionId: string) => void + open: boolean +} + +/** + * Desktop equivalent of the TUI's sessions overlay (`/resume`, `/sessions`, + * `/switch`): a focused, type-to-filter list of recent sessions that resumes + * the picked one. Mirrors the command palette's cmdk surface but scoped to + * sessions only, so `/resume` feels first-class instead of falling through to + * the headless slash worker (which can't render the picker). + */ +export function SessionPickerDialog({ + activeStoredSessionId, + onOpenChange, + onResume, + open +}: SessionPickerDialogProps) { + const { t } = useI18n() + const [search, setSearch] = useState('') + + const sessionsQuery = useQuery({ + enabled: open, + queryFn: () => listSessions(200, 1, 'exclude'), + queryKey: ['session-picker', 'sessions'] + }) + + useEffect(() => { + if (!open) { + setSearch('') + } + }, [open]) + + const sessions = useMemo(() => sessionsQuery.data?.sessions ?? [], [sessionsQuery.data]) + + return ( + + + + + {t.commandCenter.sections.sessions} + + + + {t.commandCenter.noResults} + + {sessions.map(session => { + const title = sessionTitle(session) + const preview = session.preview?.trim() + + return ( + { + onResume(session.id) + onOpenChange(false) + }} + value={`${title} ${preview ?? ''} ${session.id}`} + > + + + {title} + {preview ? ( + {preview} + ) : null} + + + + ) + })} + + + + + + + ) +} diff --git a/apps/desktop/src/i18n/en.ts b/apps/desktop/src/i18n/en.ts index 5aaf090d7e66..5a18d47efa97 100644 --- a/apps/desktop/src/i18n/en.ts +++ b/apps/desktop/src/i18n/en.ts @@ -1778,7 +1778,14 @@ export const en: Translations = { clipboard: 'Clipboard', noClipboardImage: 'No image found in clipboard', clipboardPasteFailed: 'Clipboard paste failed', - dropFiles: 'Drop files' + dropFiles: 'Drop files', + handoff: { + pickPlatform: 'Choose a destination', + success: platform => `Handed off to ${platform}. Resume here anytime.`, + systemNote: platform => `↻ Handed off to ${platform} — resume here anytime.`, + failed: error => `Handoff failed: ${error}`, + timedOut: 'Timed out waiting for the gateway. Is `hermes gateway` running?' + } }, errors: { diff --git a/apps/desktop/src/i18n/ja.ts b/apps/desktop/src/i18n/ja.ts index 956788067ede..36634a6c0251 100644 --- a/apps/desktop/src/i18n/ja.ts +++ b/apps/desktop/src/i18n/ja.ts @@ -1914,7 +1914,14 @@ export const ja = defineLocale({ clipboard: 'クリップボード', noClipboardImage: 'クリップボードに画像が見つかりません', clipboardPasteFailed: 'クリップボードからの貼り付けに失敗しました', - dropFiles: 'ファイルをドロップ' + dropFiles: 'ファイルをドロップ', + handoff: { + pickPlatform: '送信先を選択', + success: platform => `${platform} に引き継ぎました。いつでもここで再開できます。`, + systemNote: platform => `↻ ${platform} に引き継ぎました — いつでもここで再開できます。`, + failed: error => `引き継ぎに失敗しました: ${error}`, + timedOut: 'ゲートウェイの待機がタイムアウトしました。`hermes gateway` は起動していますか?' + } }, errors: { diff --git a/apps/desktop/src/i18n/types.ts b/apps/desktop/src/i18n/types.ts index 77424e426ac8..7a10e5f3d1c2 100644 --- a/apps/desktop/src/i18n/types.ts +++ b/apps/desktop/src/i18n/types.ts @@ -1437,6 +1437,13 @@ export interface Translations { noClipboardImage: string clipboardPasteFailed: string dropFiles: string + handoff: { + pickPlatform: string + success: (platform: string) => string + systemNote: (platform: string) => string + failed: (error: string) => string + timedOut: string + } } errors: { diff --git a/apps/desktop/src/i18n/zh-hant.ts b/apps/desktop/src/i18n/zh-hant.ts index 9f045c4d0228..830dc475134e 100644 --- a/apps/desktop/src/i18n/zh-hant.ts +++ b/apps/desktop/src/i18n/zh-hant.ts @@ -1873,7 +1873,14 @@ export const zhHant = defineLocale({ clipboard: '剪貼簿', noClipboardImage: '剪貼簿中沒有圖片', clipboardPasteFailed: '剪貼簿貼上失敗', - dropFiles: '拖曳檔案' + dropFiles: '拖曳檔案', + handoff: { + pickPlatform: '選擇目標平台', + success: platform => `已移交到 ${platform}。隨時可在此處恢復。`, + systemNote: platform => `↻ 已移交到 ${platform} — 隨時可在此處恢復。`, + failed: error => `移交失敗:${error}`, + timedOut: '等待閘道逾時。`hermes gateway` 是否正在執行?' + } }, errors: { diff --git a/apps/desktop/src/i18n/zh.ts b/apps/desktop/src/i18n/zh.ts index f6b119a27771..dbad00cf5d11 100644 --- a/apps/desktop/src/i18n/zh.ts +++ b/apps/desktop/src/i18n/zh.ts @@ -1956,7 +1956,14 @@ export const zh: Translations = { clipboard: '剪贴板', noClipboardImage: '剪贴板中没有图片', clipboardPasteFailed: '粘贴剪贴板失败', - dropFiles: '拖放文件' + dropFiles: '拖放文件', + handoff: { + pickPlatform: '选择目标平台', + success: platform => `已移交到 ${platform}。随时可在此处恢复。`, + systemNote: platform => `↻ 已移交到 ${platform} — 随时可在此处恢复。`, + failed: error => `移交失败:${error}`, + timedOut: '等待网关超时。`hermes gateway` 是否正在运行?' + } }, errors: { diff --git a/apps/desktop/src/lib/ansi.ts b/apps/desktop/src/lib/ansi.ts index f30987ec6053..c7770e8b7778 100644 --- a/apps/desktop/src/lib/ansi.ts +++ b/apps/desktop/src/lib/ansi.ts @@ -173,3 +173,14 @@ export function hasAnsiCodes(input: string): boolean { // eslint-disable-next-line no-control-regex return /\x1b\[/.test(input) } + +/** Remove all ANSI escape sequences, returning plain text. Use when output is + * rendered as text (e.g. chat system messages) rather than styled segments — + * otherwise the ESC byte is invisible and the `[1;31m…` payload leaks through. */ +export function stripAnsi(input: string): string { + if (!input) { + return input + } + + return input.replace(OTHER_ESCAPE_RE, '').replace(CSI_RE, '') +} diff --git a/apps/desktop/src/lib/desktop-slash-commands.test.ts b/apps/desktop/src/lib/desktop-slash-commands.test.ts index de0e72ec28b4..d37738173ce9 100644 --- a/apps/desktop/src/lib/desktop-slash-commands.test.ts +++ b/apps/desktop/src/lib/desktop-slash-commands.test.ts @@ -7,7 +7,9 @@ import { filterDesktopCommandsCatalog, isDesktopSlashCommand, isDesktopSlashSuggestion, - isModelPickerCommand + isModelPickerCommand, + isPickerCommand, + resolveDesktopCommand } from './desktop-slash-commands' describe('desktop slash command curation', () => { @@ -38,6 +40,18 @@ describe('desktop slash command curation', () => { expect(isDesktopSlashSuggestion('/curator')).toBe(false) }) + it('surfaces /tools, /save, and /personality on the desktop', () => { + expect(isDesktopSlashSuggestion('/tools')).toBe(true) + expect(isDesktopSlashSuggestion('/save')).toBe(true) + expect(isDesktopSlashSuggestion('/personality')).toBe(true) + expect(isDesktopSlashCommand('/tools')).toBe(true) + expect(isDesktopSlashCommand('/save')).toBe(true) + expect(isDesktopSlashCommand('/personality')).toBe(true) + expect(desktopSlashUnavailableMessage('/tools')).toBeNull() + expect(desktopSlashUnavailableMessage('/save')).toBeNull() + expect(desktopSlashUnavailableMessage('/personality')).toBeNull() + }) + it('allows aliases to execute without cluttering the popover', () => { expect(isDesktopSlashSuggestion('/reset')).toBe(false) expect(isDesktopSlashCommand('/reset')).toBe(true) @@ -74,6 +88,24 @@ describe('desktop slash command curation', () => { ['/new', 'Start a new desktop chat'], ['/ship-it', 'Run release checklist'] ]) + // skill_count is recomputed from the filtered output (only /ship-it is an + // extension command — /new is a built-in) so the /help footer matches what + // the user actually sees rather than echoing the unfiltered backend total. + expect(filtered.skill_count).toBe(1) + }) + + it('recomputes skill_count to reflect only extensions surfaced on desktop', () => { + const filtered = filterDesktopCommandsCatalog({ + pairs: [ + ['/new', 'Start a new session'], + ['/clear', 'Clear terminal screen'], + ['/gif-search', 'Search for a gif'], + ['/ship-it', 'Run release checklist'] + ], + skill_count: 12 + }) + + expect(filtered.pairs?.map(([cmd]) => cmd)).toEqual(['/new', '/gif-search', '/ship-it']) expect(filtered.skill_count).toBe(2) }) @@ -123,4 +155,26 @@ describe('desktop slash command curation', () => { expect(isModelPickerCommand('/new')).toBe(false) expect(isModelPickerCommand('/skills')).toBe(false) }) + + it('gives /resume (and its aliases) a first-class session picker surface', () => { + expect(isPickerCommand('/resume', 'session')).toBe(true) + expect(isPickerCommand('/sessions', 'session')).toBe(true) + expect(isPickerCommand('/switch', 'session')).toBe(true) + // Unlike /model, /resume shows in the popover; its aliases stay hidden. + expect(isDesktopSlashSuggestion('/resume')).toBe(true) + expect(isDesktopSlashSuggestion('/sessions')).toBe(false) + expect(isDesktopSlashCommand('/switch')).toBe(true) + // The session picker is distinct from the model picker. + expect(isModelPickerCommand('/resume')).toBe(false) + }) + + it('resolves commands and aliases to their declared surface', () => { + expect(resolveDesktopCommand('/new')?.surface).toEqual({ kind: 'action', action: 'new' }) + expect(resolveDesktopCommand('/reset')?.surface).toEqual({ kind: 'action', action: 'new' }) + expect(resolveDesktopCommand('/resume')?.surface).toEqual({ kind: 'picker', picker: 'session' }) + expect(resolveDesktopCommand('/usage')?.surface).toEqual({ kind: 'exec' }) + expect(resolveDesktopCommand('/clear')?.surface).toEqual({ kind: 'unavailable', reason: 'terminal' }) + // Skill / quick commands aren't in the registry. + expect(resolveDesktopCommand('/gif-search')).toBeNull() + }) }) diff --git a/apps/desktop/src/lib/desktop-slash-commands.ts b/apps/desktop/src/lib/desktop-slash-commands.ts index e373ac94317f..d898a6c83f16 100644 --- a/apps/desktop/src/lib/desktop-slash-commands.ts +++ b/apps/desktop/src/lib/desktop-slash-commands.ts @@ -22,110 +22,161 @@ export interface DesktopThemeCommandOption { name: string } -const DESKTOP_COMMAND_META = [ - ['/agents', 'Show active desktop sessions and running tasks'], - ['/background', 'Run a prompt in the background'], - ['/branch', 'Branch the latest message into a new chat'], - ['/compress', 'Compress this conversation context'], - ['/debug', 'Create a debug report'], - ['/goal', 'Manage the standing goal for this session'], - ['/help', 'Show desktop slash commands'], - ['/new', 'Start a new desktop chat'], - ['/profile', 'Switch the active Hermes profile'], - ['/queue', 'Queue a prompt for the next turn'], - ['/resume', 'Resume a saved session'], - ['/retry', 'Retry the last user message'], - ['/rollback', 'List or restore filesystem checkpoints'], - ['/skin', 'Switch desktop theme or cycle to the next one'], - ['/status', 'Show current session status'], - ['/steer', 'Steer the current run after the next tool call'], - ['/stop', 'Stop running background processes'], - ['/title', 'Rename the current session'], - ['/undo', 'Remove the last user/assistant exchange'], - ['/usage', 'Show token usage for this session'], - ['/version', 'Show Hermes Agent version'], - ['/yolo', 'Toggle YOLO — auto-approve dangerous commands'] -] as const +/** + * Local client action a command resolves to. Each id maps to exactly one + * handler in the dispatcher (`use-prompt-actions`), so adding a command never + * means adding a branch to a switch ladder — you add a row here + a handler + * keyed by the id. + */ +export type DesktopActionId = + | 'branch' + | 'handoff' + | 'help' + | 'new' + | 'profile' + | 'skin' + | 'title' + | 'yolo' -const DESKTOP_COMMANDS: ReadonlySet = new Set(DESKTOP_COMMAND_META.map(([command]) => command)) +/** A command fulfilled by opening a desktop overlay picker. */ +export type DesktopPickerId = 'model' | 'session' -const DESKTOP_ALIASES = new Map([ - ['/bg', '/background'], - ['/btw', '/background'], - ['/fork', '/branch'], - ['/q', '/queue'], - ['/reload_mcp', '/reload-mcp'], - ['/reload_skills', '/reload-skills'], - ['/reset', '/new'], - ['/tasks', '/agents'] -]) +/** Why a known Hermes command has no desktop UI surface. */ +export type DesktopUnavailableReason = 'advanced' | 'messaging' | 'settings' | 'terminal' -const DESKTOP_COMMAND_DESCRIPTIONS: ReadonlyMap = new Map(DESKTOP_COMMAND_META) +/** + * How the desktop fulfils a command. This is the single discriminator the + * dispatcher, popover, pills, and pickers all read — no parallel block-lists. + * + * - `action` → handled by a local client handler (new chat, branch, …) + * - `picker` → opens an overlay (`/model`, `/resume`); a typed arg is + * resolved by that picker instead of falling through + * - `exec` → runs on the backend via slash.exec / command.dispatch and + * renders its text output inline + * - `unavailable`→ a known command with genuinely no desktop UI (terminal-only, + * messaging-only, …); shows a reason instead of executing + */ +export type DesktopCommandSurface = + | { kind: 'action'; action: DesktopActionId } + | { kind: 'picker'; picker: DesktopPickerId } + | { kind: 'exec' } + | { kind: 'unavailable'; reason: DesktopUnavailableReason } -const PICKER_OWNED_COMMANDS = new Set(['/model']) +export interface DesktopCommandSpec { + /** Canonical command, leading slash included (e.g. `/resume`). */ + name: string + /** Popover/help label; omitted for unavailable commands (never surfaced). */ + description?: string + aliases?: string[] + surface: DesktopCommandSurface + /** + * Hide from the slash popover / completions while still letting it execute. + * Used for picker commands reachable from chrome (the model picker lives on + * the status bar), so the popover doesn't dead-end on inline completion. + */ + hidden?: boolean + /** + * The command has an inline options "screen" (theme / personality / session / + * platform / toolset list). Picking the bare command in the popover expands to + * that argument step instead of committing — mirroring typing `/ ` by hand. + */ + args?: boolean +} -const TERMINAL_ONLY_COMMANDS = new Set([ - '/browser', - '/busy', - '/clear', - '/commands', - '/compact', - '/config', - '/copy', - '/cron', - '/details', - '/exit', - '/footer', - '/gateway', - '/gquota', - '/history', - '/image', - '/indicator', - '/logs', - '/mouse', - '/paste', - '/platforms', - '/plugins', - '/quit', - '/redraw', - '/reload', - '/restart', - '/save', - '/sb', - '/set-home', - '/sethome', - '/snap', - '/snapshot', - '/statusbar', - '/toolsets', - '/tools', - '/update', - '/verbose' -]) +const exec = (): DesktopCommandSurface => ({ kind: 'exec' }) +const action = (id: DesktopActionId): DesktopCommandSurface => ({ kind: 'action', action: id }) +const picker = (id: DesktopPickerId): DesktopCommandSurface => ({ kind: 'picker', picker: id }) +const unavailable = (reason: DesktopUnavailableReason): DesktopCommandSurface => ({ kind: 'unavailable', reason }) -const MESSAGING_ONLY_COMMANDS = new Set(['/approve', '/deny']) +/** + * THE source of truth for desktop slash commands. Everything below — execution + * gating, popover suggestions, catalog filtering, pill grouping, and the + * dispatcher's behavior — derives from this one table. + */ +const DESKTOP_COMMAND_SPECS: readonly DesktopCommandSpec[] = [ + // Local client actions + { name: '/new', description: 'Start a new desktop chat', aliases: ['/reset'], surface: action('new') }, + { name: '/branch', description: 'Branch the latest message into a new chat', aliases: ['/fork'], surface: action('branch') }, + { name: '/yolo', description: 'Toggle YOLO — auto-approve dangerous commands', surface: action('yolo') }, + { name: '/handoff', description: 'Hand off this session to a messaging platform', surface: action('handoff'), args: true }, + { name: '/profile', description: 'Switch the active Hermes profile', surface: action('profile') }, + { name: '/skin', description: 'Switch desktop theme or cycle to the next one', surface: action('skin'), args: true }, + { name: '/title', description: 'Rename the current session', surface: action('title') }, + { name: '/help', description: 'Show desktop slash commands', aliases: ['/commands'], surface: action('help') }, -const SETTINGS_OWNED_COMMANDS = new Set(['/skills']) + // Overlay pickers + { name: '/model', description: 'Switch the model for this session', surface: picker('model'), hidden: true }, + { + name: '/resume', + description: 'Resume a saved session', + aliases: ['/sessions', '/switch'], + surface: picker('session'), + args: true + }, -const ADVANCED_COMMANDS = new Set([ - '/curator', - '/fast', - '/insights', - '/kanban', - '/personality', - '/reasoning', - '/reload-mcp', - '/reload-skills', - '/voice' -]) + // Backend-executed commands that render useful inline output + { name: '/agents', description: 'Show active desktop sessions and running tasks', aliases: ['/tasks'], surface: exec() }, + { name: '/background', description: 'Run a prompt in the background', aliases: ['/bg', '/btw'], surface: exec() }, + { name: '/compress', description: 'Compress this conversation context', surface: exec() }, + { name: '/debug', description: 'Create a debug report', surface: exec() }, + { name: '/goal', description: 'Manage the standing goal for this session', surface: exec() }, + { name: '/personality', description: 'Switch personality for this session', surface: exec(), args: true }, + { name: '/queue', description: 'Queue a prompt for the next turn', aliases: ['/q'], surface: exec() }, + { name: '/retry', description: 'Retry the last user message', surface: exec() }, + { name: '/rollback', description: 'List or restore filesystem checkpoints', surface: exec() }, + { name: '/save', description: 'Save the current transcript to JSON', surface: exec() }, + { name: '/status', description: 'Show current session status', surface: exec() }, + { name: '/steer', description: 'Steer the current run after the next tool call', surface: exec() }, + { name: '/stop', description: 'Stop running background processes', surface: exec() }, + { name: '/tools', description: 'List or toggle tools available to the agent', surface: exec(), args: true }, + { name: '/undo', description: 'Remove the last user/assistant exchange', surface: exec() }, + { name: '/usage', description: 'Show token usage for this session', surface: exec() }, + { name: '/version', description: 'Show Hermes Agent version', surface: exec() }, -const BLOCKED_COMMANDS = new Set([ - ...PICKER_OWNED_COMMANDS, - ...TERMINAL_ONLY_COMMANDS, - ...MESSAGING_ONLY_COMMANDS, - ...SETTINGS_OWNED_COMMANDS, - ...ADVANCED_COMMANDS -]) + // No desktop surface, but carry an alias (underscore spelling variants). + { name: '/reload-mcp', aliases: ['/reload_mcp'], surface: unavailable('advanced') }, + { name: '/reload-skills', aliases: ['/reload_skills'], surface: unavailable('advanced') } +] + +// Known commands with no desktop surface (and no alias) — a flat name list +// per reason beats 40 identical object literals. +const NO_DESKTOP_SURFACE: Record = { + terminal: [ + '/browser', '/busy', '/clear', '/compact', '/config', '/copy', '/cron', '/details', + '/exit', '/footer', '/gateway', '/gquota', '/history', '/image', '/indicator', '/logs', + '/mouse', '/paste', '/platforms', '/plugins', '/quit', '/redraw', '/reload', '/restart', + '/sb', '/set-home', '/sethome', '/snap', '/snapshot', '/statusbar', '/toolsets', '/update', '/verbose' + ], + messaging: ['/approve', '/deny'], + settings: ['/skills'], + advanced: ['/curator', '/fast', '/insights', '/kanban', '/reasoning', '/voice'] +} + +const ALL_SPECS: readonly DesktopCommandSpec[] = [ + ...DESKTOP_COMMAND_SPECS, + ...(Object.entries(NO_DESKTOP_SURFACE) as [DesktopUnavailableReason, readonly string[]][]).flatMap( + ([reason, names]) => names.map(name => ({ name, surface: unavailable(reason) })) + ) +] + +const SPEC_BY_NAME = new Map(ALL_SPECS.map(spec => [spec.name, spec])) + +const ALIAS_TO_CANONICAL = new Map( + ALL_SPECS.flatMap(spec => (spec.aliases ?? []).map(alias => [alias, spec.name] as const)) +) + +const UNAVAILABLE_MESSAGE: Record string> = { + advanced: command => + `${command} is not shown in the desktop slash palette. Use the relevant desktop control or terminal interface instead.`, + messaging: command => `${command} is only used from messaging platforms.`, + settings: command => `${command} is managed from the desktop sidebar.`, + terminal: command => `${command} is only available in the terminal interface.` +} + +const PICKER_UNAVAILABLE_MESSAGE: Record string> = { + model: command => `${command} uses the desktop model picker instead of a slash command.`, + session: command => `${command} uses the desktop session picker instead of a slash command.` +} function normalizeCommand(command: string): string { const trimmed = command.trim() @@ -137,27 +188,25 @@ function normalizeCommand(command: string): string { export function canonicalDesktopSlashCommand(command: string): string { const normalized = normalizeCommand(command) - return DESKTOP_ALIASES.get(normalized) || normalized + return ALIAS_TO_CANONICAL.get(normalized) || normalized } -export function isDesktopSlashCommand(command: string): boolean { +/** Resolve a command (or alias) to its desktop spec, or null for unknown/extension commands. */ +export function resolveDesktopCommand(command: string): DesktopCommandSpec | null { + return SPEC_BY_NAME.get(canonicalDesktopSlashCommand(command)) ?? null +} + +function isKnownHermesSlashCommand(command: string): boolean { const normalized = normalizeCommand(command) - const canonical = canonicalDesktopSlashCommand(normalized) - if (BLOCKED_COMMANDS.has(normalized) || BLOCKED_COMMANDS.has(canonical)) { - return false - } - - return DESKTOP_COMMANDS.has(canonical) || !isKnownHermesSlashCommand(normalized) + return SPEC_BY_NAME.has(normalized) || ALIAS_TO_CANONICAL.has(normalized) } /** * An "extension" command is anything the backend surfaces that is NOT one of * Hermes' built-in slash commands — i.e. skill commands (`/gif-search`, * `/codex`, …) and user-defined quick commands. These are user-activated, so - * they should appear in the desktop slash palette even though they aren't in - * the curated `DESKTOP_COMMANDS` allow-list. This mirrors the predicate in - * `isDesktopSlashCommand` that already lets them EXECUTE when typed. + * they appear in the desktop slash palette and execute when typed. */ export function isDesktopSlashExtensionCommand(command: string): boolean { const normalized = normalizeCommand(command) @@ -169,63 +218,85 @@ export function isDesktopSlashExtensionCommand(command: string): boolean { return !isKnownHermesSlashCommand(normalized) } -export function isDesktopSlashSuggestion(command: string): boolean { - const normalized = normalizeCommand(command) - const canonical = canonicalDesktopSlashCommand(normalized) +/** Gates execution: true unless the command is a known no-desktop-surface command. */ +export function isDesktopSlashCommand(command: string): boolean { + const spec = resolveDesktopCommand(command) - // Surface skill / quick commands (extensions the backend provides) alongside - // the curated built-ins. Built-in aliases stay hidden so the popover isn't - // cluttered with duplicates. - if (isDesktopSlashExtensionCommand(normalized)) { - return true + if (spec) { + return spec.surface.kind !== 'unavailable' } - return DESKTOP_COMMANDS.has(canonical) && !DESKTOP_ALIASES.has(normalized) + return isDesktopSlashExtensionCommand(command) +} + +/** Gates discovery in the popover/completions. */ +export function isDesktopSlashSuggestion(command: string): boolean { + const normalized = normalizeCommand(command) + + // Aliases stay hidden so the popover isn't cluttered with duplicates. + if (ALIAS_TO_CANONICAL.has(normalized)) { + return false + } + + const spec = SPEC_BY_NAME.get(normalized) + + if (spec) { + return spec.surface.kind !== 'unavailable' && !spec.hidden + } + + // Skill / quick commands the backend provides. + return isDesktopSlashExtensionCommand(normalized) } /** - * True for commands the desktop fulfils by opening the model picker overlay - * (e.g. `/model`) rather than executing a slash command. The caller opens the - * picker UI instead of printing the "uses the desktop model picker" notice. + * True for commands the desktop fulfils by opening an overlay picker + * (`/model`, `/resume`/`/sessions`/`/switch`). Optionally pin to one picker. */ -export function isModelPickerCommand(command: string): boolean { - const normalized = normalizeCommand(command) - const canonical = canonicalDesktopSlashCommand(normalized) +export function isPickerCommand(command: string, picker?: DesktopPickerId): boolean { + const surface = resolveDesktopCommand(command)?.surface - return PICKER_OWNED_COMMANDS.has(canonical) + if (surface?.kind !== 'picker') { + return false + } + + return picker ? surface.picker === picker : true +} + +/** Back-compat shim for the model picker check. */ +export function isModelPickerCommand(command: string): boolean { + return isPickerCommand(command, 'model') } export function desktopSlashUnavailableMessage(command: string): string | null { - const normalized = normalizeCommand(command) - const canonical = canonicalDesktopSlashCommand(normalized) + const canonical = canonicalDesktopSlashCommand(command) + const surface = SPEC_BY_NAME.get(canonical)?.surface - if (PICKER_OWNED_COMMANDS.has(canonical)) { - return `/${canonical.slice(1)} uses the desktop model picker instead of a slash command.` + if (!surface) { + return null } - if (SETTINGS_OWNED_COMMANDS.has(canonical)) { - return `/${canonical.slice(1)} is managed from the desktop sidebar.` + if (surface.kind === 'unavailable') { + return UNAVAILABLE_MESSAGE[surface.reason](canonical) } - if (MESSAGING_ONLY_COMMANDS.has(canonical)) { - return `/${canonical.slice(1)} is only used from messaging platforms.` - } - - if (ADVANCED_COMMANDS.has(canonical)) { - return `/${canonical.slice(1)} is not shown in the desktop slash palette. Use the relevant desktop control or terminal interface instead.` - } - - if (TERMINAL_ONLY_COMMANDS.has(normalized) || TERMINAL_ONLY_COMMANDS.has(canonical)) { - return `/${canonical.slice(1)} is only available in the terminal interface.` + if (surface.kind === 'picker') { + return PICKER_UNAVAILABLE_MESSAGE[surface.picker](canonical) } return null } export function desktopSlashDescription(command: string, fallback = ''): string { - const canonical = canonicalDesktopSlashCommand(command) + return SPEC_BY_NAME.get(canonicalDesktopSlashCommand(command))?.description || fallback +} - return DESKTOP_COMMAND_DESCRIPTIONS.get(canonical) || fallback +/** + * True when picking the bare command should expand to its inline argument + * options (theme / personality / session / platform / toolset) rather than + * committing immediately. Lets the popover act as a two-step picker. + */ +export function desktopSlashCommandTakesArgs(command: string): boolean { + return resolveDesktopCommand(command)?.args ?? false } export function desktopSkinSlashCompletions( @@ -274,13 +345,36 @@ export function filterDesktopCommandsCatalog(catalog: CommandsCatalogLike): Comm ?.filter(([command]) => isDesktopSlashSuggestion(command)) .map(([command, description]) => [command, desktopSlashDescription(command, description)] as [string, string]) + // Recount skill commands from the filtered output so /help's footer reflects + // what the user actually sees. Backend's skill_count includes commands the + // desktop hides (terminal-only, picker-owned, advanced), producing a footer + // like "60 skill commands available" while only ~29 appear in the list. + const filteredCommands = new Set() + + for (const section of categories ?? []) { + for (const [command] of section.pairs) { + filteredCommands.add(canonicalDesktopSlashCommand(command)) + } + } + + for (const [command] of pairs ?? []) { + filteredCommands.add(canonicalDesktopSlashCommand(command)) + } + + let skillCount = 0 + + for (const command of filteredCommands) { + if (isDesktopSlashExtensionCommand(command)) { + skillCount += 1 + } + } + + const hasSkillCount = catalog.skill_count !== undefined || skillCount > 0 + return { ...catalog, ...(categories ? { categories } : {}), - ...(pairs ? { pairs } : {}) + ...(pairs ? { pairs } : {}), + ...(hasSkillCount ? { skill_count: skillCount } : {}) } } - -function isKnownHermesSlashCommand(command: string): boolean { - return DESKTOP_COMMANDS.has(command) || DESKTOP_ALIASES.has(command) || BLOCKED_COMMANDS.has(command) -} diff --git a/apps/desktop/src/store/session.ts b/apps/desktop/src/store/session.ts index 6df96946bf1c..4139915cea2c 100644 --- a/apps/desktop/src/store/session.ts +++ b/apps/desktop/src/store/session.ts @@ -200,6 +200,7 @@ export const $availablePersonalities = atom([]) export const $introSeed = atom(0) export const $contextSuggestions = atom([]) export const $modelPickerOpen = atom(false) +export const $sessionPickerOpen = atom(false) export const setConnection = (next: Updater) => updateAtom($connection, next) export const setGatewayState = (next: Updater) => updateAtom($gatewayState, next) @@ -249,6 +250,7 @@ export const setAvailablePersonalities = (next: Updater) => updateAtom export const setIntroSeed = (next: Updater) => updateAtom($introSeed, next) export const setContextSuggestions = (next: Updater) => updateAtom($contextSuggestions, next) export const setModelPickerOpen = (next: Updater) => updateAtom($modelPickerOpen, next) +export const setSessionPickerOpen = (next: Updater) => updateAtom($sessionPickerOpen, next) // Watchdog tracking — when does a "working" session count as stuck? // Long-running tool calls (LLM inference, long shell commands, web fetches) diff --git a/hermes_cli/commands.py b/hermes_cli/commands.py index f23d1960da77..aded4d41d81c 100644 --- a/hermes_cli/commands.py +++ b/hermes_cli/commands.py @@ -1544,12 +1544,140 @@ class SlashCommandCompleter(Completer): except Exception: pass + @staticmethod + def _tools_completions(sub_text: str, sub_lower: str): + """Yield completions for /tools — subcommand + toolset/MCP-server name. + + Handles both ``/tools `` (suggesting ``list|disable|enable``) and + ``/tools enable `` / ``/tools disable `` (suggesting toolset + keys and MCP server prefixes, filtered by current enable state so the + user only sees actionable options). + """ + SUBS = ("list", "disable", "enable") + parts = sub_text.split() + trailing_space = sub_text.endswith(" ") + + # Subcommand stage: zero words typed, or completing the first word. + if len(parts) == 0 or (len(parts) == 1 and not trailing_space): + partial = sub_text if not trailing_space else "" + for sub in SUBS: + if sub.startswith(partial.lower()) and sub != partial.lower(): + yield Completion(sub, start_position=-len(partial), display=sub) + return + + subcommand = parts[0].lower() + if subcommand not in ("enable", "disable"): + return + + partial = "" if trailing_space else parts[-1] + partial_lower = partial.lower() + already = set(parts[1:] if trailing_space else parts[1:-1]) + + try: + from hermes_cli.config import load_config + from hermes_cli.tools_config import ( + CONFIGURABLE_TOOLSETS, + _get_platform_tools, + _get_plugin_toolset_keys, + ) + + config = load_config() + enabled = _get_platform_tools(config, "cli", include_default_mcp_servers=False) + + for ts_key, label, _desc in CONFIGURABLE_TOOLSETS: + if ts_key in already or not ts_key.startswith(partial_lower): + continue + is_on = ts_key in enabled + if subcommand == "enable" and is_on: + continue + if subcommand == "disable" and not is_on: + continue + yield Completion( + ts_key, + start_position=-len(partial), + display=ts_key, + display_meta=label, + ) + + for ts_key in sorted(_get_plugin_toolset_keys()): + if ts_key in already or not ts_key.startswith(partial_lower): + continue + is_on = ts_key in enabled + if subcommand == "enable" and is_on: + continue + if subcommand == "disable" and not is_on: + continue + yield Completion( + ts_key, + start_position=-len(partial), + display=ts_key, + display_meta="plugin toolset", + ) + + mcp_servers = config.get("mcp_servers") or {} + if isinstance(mcp_servers, dict): + for server in sorted(mcp_servers): + prefix = f"{server}:" + if prefix in already or not prefix.startswith(partial_lower): + continue + yield Completion( + prefix, + start_position=-len(partial), + display=prefix, + display_meta=f"MCP server '{server}'", + ) + except Exception: + return + + @staticmethod + def _handoff_completions(sub_text: str, sub_lower: str): + """Yield platform completions for /handoff. + + Offers connected (enabled + configured) gateway platforms. A recorded + home channel is NOT required to list a platform — it's often learned at + runtime — so the meta hints whether one is set yet. Completes only the + first arg (the platform); once one is chosen, stop. + """ + parts = sub_text.split() + trailing_space = sub_text.endswith(" ") + if len(parts) > 1 or (len(parts) == 1 and trailing_space): + return + partial = "" if (not parts or trailing_space) else parts[-1] + partial_lower = partial.lower() + try: + from gateway.config import load_gateway_config + + gw = load_gateway_config() + platforms = gw.get_connected_platforms() + except Exception: + return + for platform in platforms: + name = platform.value + if not name.startswith(partial_lower): + continue + try: + home = gw.get_home_channel(platform) + except Exception: + home = None + meta = f"→ {home.name}" if home and getattr(home, "name", None) else "send this session here" + yield Completion( + name, + start_position=-len(partial), + display=name, + display_meta=meta, + ) + @staticmethod def _personality_completions(sub_text: str, sub_lower: str): """Yield completions for /personality from configured personalities.""" try: - from hermes_cli.config import load_config - personalities = load_config().get("agent", {}).get("personalities", {}) + # Resolve from the same source the runtime applies personalities — + # agent.personalities via the CLI config (which ships the built-ins). + # load_config()'s schema has no agent.personalities, so the completer + # used to come back empty even with personalities available. + from cli import load_cli_config + + personalities = (load_cli_config().get("agent") or {}).get("personalities", {}) or {} if "none".startswith(sub_lower) and "none" != sub_lower: yield Completion( "none", @@ -1602,6 +1730,17 @@ class SlashCommandCompleter(Completer): yield from self._personality_completions(sub_text, sub_lower) return + # /tools needs multi-word completion (subcommand + toolset name) + # so it handles both stages itself, bypassing the single-word + # SUBCOMMANDS branch below. + if base_cmd == "/tools": + yield from self._tools_completions(sub_text, sub_lower) + return + + if base_cmd == "/handoff": + yield from self._handoff_completions(sub_text, sub_lower) + return + # Static subcommand completions if " " not in sub_text and base_cmd in SUBCOMMANDS and self._command_allowed(base_cmd): for sub in SUBCOMMANDS[base_cmd]: diff --git a/tests/hermes_cli/test_commands.py b/tests/hermes_cli/test_commands.py index 6a63ebe73e5b..62c2be4ab79b 100644 --- a/tests/hermes_cli/test_commands.py +++ b/tests/hermes_cli/test_commands.py @@ -691,6 +691,169 @@ class TestSubcommandCompletion: completions = _completions(SlashCommandCompleter(), "/help ") assert completions == [] + def test_tools_subcommand_completion(self): + """`/tools ` should suggest list, disable, enable.""" + completions = _completions(SlashCommandCompleter(), "/tools ") + texts = {c.text for c in completions} + assert texts == {"list", "disable", "enable"} + + def test_tools_subcommand_prefix_filters(self): + completions = _completions(SlashCommandCompleter(), "/tools en") + texts = {c.text for c in completions} + assert texts == {"enable"} + + def test_tools_enable_completes_toolset_names(self, monkeypatch): + """`/tools enable ` should suggest currently-disabled toolsets.""" + from hermes_cli import commands as commands_mod + + # `web` is enabled, `spotify` is disabled — enabling should only offer + # the disabled ones. + monkeypatch.setattr( + "hermes_cli.tools_config._get_platform_tools", + lambda *_a, **_k: {"web", "file"}, + ) + monkeypatch.setattr("hermes_cli.config.load_config", lambda: {}) + monkeypatch.setattr( + "hermes_cli.tools_config._get_plugin_toolset_keys", + lambda: set(), + ) + + completions = _completions(SlashCommandCompleter(), "/tools enable ") + texts = {c.text for c in completions} + # Should include disabled toolsets, exclude already-enabled ones. + assert "web" not in texts + assert "file" not in texts + assert "spotify" in texts + + def test_tools_disable_completes_enabled_toolsets_only(self, monkeypatch): + monkeypatch.setattr( + "hermes_cli.tools_config._get_platform_tools", + lambda *_a, **_k: {"web", "file"}, + ) + monkeypatch.setattr("hermes_cli.config.load_config", lambda: {}) + monkeypatch.setattr( + "hermes_cli.tools_config._get_plugin_toolset_keys", + lambda: set(), + ) + + completions = _completions(SlashCommandCompleter(), "/tools disable ") + texts = {c.text for c in completions} + # Should include enabled toolsets, exclude disabled ones. + assert texts == {"web", "file"} + + def test_tools_enable_partial_filters(self, monkeypatch): + monkeypatch.setattr( + "hermes_cli.tools_config._get_platform_tools", + lambda *_a, **_k: set(), + ) + monkeypatch.setattr("hermes_cli.config.load_config", lambda: {}) + monkeypatch.setattr( + "hermes_cli.tools_config._get_plugin_toolset_keys", + lambda: set(), + ) + + completions = _completions(SlashCommandCompleter(), "/tools enable sp") + texts = {c.text for c in completions} + assert texts == {"spotify"} + + def test_tools_enable_skips_already_listed(self, monkeypatch): + """If the user already typed a name, don't suggest it again.""" + monkeypatch.setattr( + "hermes_cli.tools_config._get_platform_tools", + lambda *_a, **_k: set(), + ) + monkeypatch.setattr("hermes_cli.config.load_config", lambda: {}) + monkeypatch.setattr( + "hermes_cli.tools_config._get_plugin_toolset_keys", + lambda: set(), + ) + + completions = _completions(SlashCommandCompleter(), "/tools enable spotify ") + texts = {c.text for c in completions} + assert "spotify" not in texts + + def test_tools_suggests_mcp_server_prefixes(self, monkeypatch): + monkeypatch.setattr( + "hermes_cli.tools_config._get_platform_tools", + lambda *_a, **_k: set(), + ) + monkeypatch.setattr( + "hermes_cli.config.load_config", + lambda: {"mcp_servers": {"github": {}, "linear": {}}}, + ) + monkeypatch.setattr( + "hermes_cli.tools_config._get_plugin_toolset_keys", + lambda: set(), + ) + + completions = _completions(SlashCommandCompleter(), "/tools enable git") + texts = {c.text for c in completions} + assert "github:" in texts + + def _fake_gateway(self, monkeypatch, platforms): + """Patch load_gateway_config with a fake whose connected platforms are + the keys of `platforms` (name -> home as None or a (chat_id, name) tuple). + """ + from types import SimpleNamespace + + enums = {name: SimpleNamespace(value=name) for name in platforms} + homes = { + name: (None if home is None else SimpleNamespace(chat_id=home[0], name=home[1])) + for name, home in platforms.items() + } + fake = SimpleNamespace( + get_connected_platforms=lambda: list(enums.values()), + get_home_channel=lambda p: homes[p.value], + ) + monkeypatch.setattr("gateway.config.load_gateway_config", lambda: fake) + + def test_handoff_completes_connected_platforms(self, monkeypatch): + """`/handoff ` offers connected platforms, with or without a home channel.""" + self._fake_gateway( + monkeypatch, + { + "telegram": ("123", "Me"), + "discord": None, # no home channel yet -> still listed + }, + ) + + texts = {c.text for c in _completions(SlashCommandCompleter(), "/handoff ")} + assert texts == {"telegram", "discord"} + + def test_handoff_filters_by_prefix(self, monkeypatch): + self._fake_gateway( + monkeypatch, + { + "telegram": ("1", "H"), + "signal": ("2", "H"), + }, + ) + + texts = {c.text for c in _completions(SlashCommandCompleter(), "/handoff te")} + assert texts == {"telegram"} + + def test_handoff_no_completion_after_platform_chosen(self, monkeypatch): + self._fake_gateway(monkeypatch, {"telegram": ("1", "H")}) + assert _completions(SlashCommandCompleter(), "/handoff telegram ") == [] + + def test_handoff_completion_swallows_config_errors(self, monkeypatch): + def _boom(): + raise RuntimeError("no gateway config") + + monkeypatch.setattr("gateway.config.load_gateway_config", _boom) + assert _completions(SlashCommandCompleter(), "/handoff ") == [] + + def test_personality_completes_configured_personalities(self): + """`/personality ` lists real personalities, not just `none`. + + Regression: the completer read load_config().agent.personalities, a path + that never exists, so it always came back empty. It must resolve from the + CLI config the runtime actually applies (which ships built-ins). + """ + texts = {c.text for c in _completions(SlashCommandCompleter(), "/personality ")} + assert "none" in texts + assert len(texts) > 1 + # ── Ghost text (SlashCommandAutoSuggest) ──────────────────────────────── diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index 3b95b8dceb8c..c510c4ef230f 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -86,6 +86,47 @@ def test_session_context_uses_session_cwd(monkeypatch, tmp_path): server._sessions.pop(sid, None) +def test_handoff_fail_marks_only_inflight_rows(monkeypatch): + class DbContext: + def __init__(self, db): + self.db = db + + def __enter__(self): + return self.db + + def __exit__(self, *_args): + return False + + class FakeDb: + def __init__(self, state): + self.state = state + self.failed_with = None + + def get_handoff_state(self, _key): + return {"state": self.state, "platform": "telegram", "error": None} + + def fail_handoff(self, _key, error): + self.failed_with = error + self.state = "failed" + + sid = "rt-handoff" + server._sessions[sid] = {"session_key": "stored-handoff"} + try: + pending = FakeDb("pending") + monkeypatch.setattr(server, "_session_db", lambda _session: DbContext(pending)) + result = server._methods["handoff.fail"]("r1", {"session_id": sid, "error": "timed out"}) + assert result["result"] == {"failed": True, "state": "failed"} + assert pending.failed_with == "timed out" + + completed = FakeDb("completed") + monkeypatch.setattr(server, "_session_db", lambda _session: DbContext(completed)) + result = server._methods["handoff.fail"]("r2", {"session_id": sid, "error": "late timeout"}) + assert result["result"] == {"failed": False, "state": "completed"} + assert completed.failed_with is None + finally: + server._sessions.pop(sid, None) + + def test_session_context_explicit_cwd_for_ephemeral_task(monkeypatch, tmp_path): """Background/preview tasks use ephemeral ids absent from `_sessions`, so the parent workspace is passed explicitly; it must pin instead of clearing back diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 390c31b092e0..5af8530abc5b 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -1,5 +1,6 @@ import atexit import concurrent.futures +import contextlib import contextvars import copy import inspect @@ -1171,6 +1172,34 @@ def _ensure_session_db_row(session: dict) -> None: pass +@contextlib.contextmanager +def _session_db(session: dict): + """Yield the SessionDB that owns this session's row (profile-aware). + + Mirrors :func:`_ensure_session_db_row`: a remote/profile session persists + into its own profile's ``state.db`` (a fresh handle we close on exit); + everything else borrows the shared ``_get_db()`` handle (left open). Yields + None when the db is unavailable. + """ + db, close_db = None, False + profile_home = session.get("profile_home") + if profile_home: + from hermes_state import SessionDB + + try: + db, close_db = SessionDB(db_path=Path(profile_home) / "state.db"), True + except Exception: + logger.debug("failed to open profile db for session", exc_info=True) + else: + db = _get_db() + try: + yield db + finally: + if close_db and db is not None: + with contextlib.suppress(Exception): + db.close() + + def _set_session_cwd(session: dict, cwd: str) -> str: resolved = os.path.abspath(os.path.expanduser(str(cwd))) if not os.path.isdir(resolved): @@ -4193,6 +4222,145 @@ def _(rid, params: dict) -> dict: return _err(rid, 5007, str(e)) +@method("handoff.request") +def _(rid, params: dict) -> dict: + """Queue a handoff of this session to a messaging platform. + + Desktop parity with the CLI ``/handoff`` command: we only write + ``handoff_state='pending'`` onto the persisted session row. The actual + transfer is performed by the separate ``hermes gateway`` process, whose + ``_handoff_watcher`` claims the row, re-binds the session to the platform's + home channel, and forges a synthetic turn. The desktop then polls + ``handoff.state`` for the terminal result. + """ + session, err = _sess_nowait(params, rid) + if err: + return err + if session.get("running"): + return _err( + rid, + 4009, + "session busy — wait for the current turn to finish, then retry the handoff", + ) + + platform_name = (params.get("platform", "") or "").strip().lower() + if not platform_name: + return _err(rid, 4023, "platform required") + + # Validate against the live gateway config — an unconfigured platform or a + # missing home channel would leave the handoff pending forever, so reject + # up front with a clear, actionable message (mirrors cli.py). + try: + from gateway.config import Platform, load_gateway_config + except Exception as e: # pragma: no cover — gateway pkg always ships + return _err(rid, 5021, f"could not load gateway config: {e}") + try: + platform = Platform(platform_name) + except (ValueError, KeyError): + return _err(rid, 4024, f"unknown platform '{platform_name}'") + try: + gw_config = load_gateway_config() + except Exception as e: + return _err(rid, 5021, f"could not load gateway config: {e}") + pcfg = gw_config.platforms.get(platform) + if not pcfg or not pcfg.enabled: + return _err( + rid, + 4025, + f"platform '{platform_name}' is not configured/enabled in the gateway", + ) + home = gw_config.get_home_channel(platform) + if not home or not home.chat_id: + return _err( + rid, + 4026, + f"no home channel configured for {platform_name} — set one with " + "/sethome on the destination chat first", + ) + + # The watcher transfers a persisted DB row, so make sure one exists even + # for a brand-new empty chat (mirrors the CLI's set_session_title stub). + _ensure_session_db_row(session) + + with _session_db(session) as db: + if db is None: + return _db_unavailable_error(rid, code=5007) + key = session["session_key"] + try: + if not db.get_session(key): + db.set_session_title(key, f"handoff-{key[:8]}") + ok = db.request_handoff(key, platform_name) + except Exception as e: + return _err(rid, 5007, str(e)) + + if not ok: + return _err( + rid, + 4027, + "session is already in flight for handoff — wait for it to settle, then retry", + ) + return _ok( + rid, + { + "queued": True, + "session_key": key, + "platform": platform_name, + "home_name": home.name, + }, + ) + + +@method("handoff.state") +def _(rid, params: dict) -> dict: + """Poll the handoff state for a session. + + Returns ``{state, platform, error}`` where ``state`` is one of + ``pending|running|completed|failed`` (or empty when no handoff record + exists). Desktop polls this after ``handoff.request``. + """ + session, err = _sess_nowait(params, rid) + if err: + return err + with _session_db(session) as db: + if db is None: + return _db_unavailable_error(rid, code=5007) + record = db.get_handoff_state(session["session_key"]) + + record = record or {} + return _ok( + rid, + { + "state": record.get("state") or "", + "platform": record.get("platform") or "", + "error": record.get("error") or "", + }, + ) + + +@method("handoff.fail") +def _(rid, params: dict) -> dict: + """Mark an in-flight handoff as failed so the user can retry. + + Desktop calls this when its bounded poll times out. Only pending/running + rows are changed so a late success from the gateway watcher is not clobbered. + """ + session, err = _sess_nowait(params, rid) + if err: + return err + reason = str(params.get("error") or "handoff failed").strip()[:500] + with _session_db(session) as db: + if db is None: + return _db_unavailable_error(rid, code=5007) + key = session["session_key"] + record = db.get_handoff_state(key) or {} + state = record.get("state") or "" + if state in {"pending", "running"}: + db.fail_handoff(key, reason) + return _ok(rid, {"failed": True, "state": "failed"}) + + return _ok(rid, {"failed": False, "state": state}) + + @method("session.usage") def _(rid, params: dict) -> dict: session, err = _sess_nowait(params, rid) From fe54960142d1e6edc9e43299c0e8889964f4e837 Mon Sep 17 00:00:00 2001 From: brooklyn! Date: Wed, 10 Jun 2026 21:35:38 -0500 Subject: [PATCH 04/28] desktop: un-truncate the active slash/@ row so long descriptions stay readable (#43926) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #42351. Slash command rows render the command label and description with `truncate`, so skill commands and longer blurbs were clipped with no way to read the full text. Rather than add a floating tooltip (which overlaps the popover and only helps the mouse), the active row — the one reached by keyboard arrows or hover, since onMouseEnter already sets activeIndex — now drops truncation and wraps inline (whitespace-normal break-words). Idle rows stay single-line/truncated so the list reads compact. --- .../src/app/chat/composer/trigger-popover.tsx | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/apps/desktop/src/app/chat/composer/trigger-popover.tsx b/apps/desktop/src/app/chat/composer/trigger-popover.tsx index 1099c0748ba3..dffa1ae77459 100644 --- a/apps/desktop/src/app/chat/composer/trigger-popover.tsx +++ b/apps/desktop/src/app/chat/composer/trigger-popover.tsx @@ -136,9 +136,24 @@ export function ComposerTriggerPopover({ > {isSlash ? ( <> - {display} + {/* Active row (keyboard nav or hover) un-truncates inline so + long command names / descriptions stay readable without a + floating tooltip. */} + + {display} + {description && ( - + {description} )} From e0e25717116c818d22f05a1875fe44960b189ad7 Mon Sep 17 00:00:00 2001 From: Matt Harris Date: Fri, 29 May 2026 17:51:52 -0400 Subject: [PATCH 05/28] =?UTF-8?q?feat(web):=20Parallel-backed=20web=20sear?= =?UTF-8?q?ch=20&=20extract=20=E2=80=94=20free=20Search=20MCP=20when=20key?= =?UTF-8?q?less,=20v1=20REST=20when=20keyed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make Parallel the web search/extract backend with a zero-setup free tier: - Keyless (no PARALLEL_API_KEY): web_search/web_extract work out of the box via Parallel's free hosted Search MCP (search.parallel.ai/mcp), and parallel becomes the default backend when no other web credentials are configured (ahead of ddgs, which is search-only). A small hand-rolled Streamable-HTTP JSON-RPC client speaks the MCP's web_search/web_fetch tools; the existing web_search/web_extract tools are the only tools registered. - Keyed (PARALLEL_API_KEY set): uses the Parallel v1 REST endpoints (client.search / client.extract with advanced_settings.full_content) — no beta. Bumps parallel-web 0.4.2 -> 0.6.0. - Attribution: on the free path only, results carry provider/attribution and the CLI tool line reads "Parallel search" / "Parallel fetch"; the paid path is unbranded. - Selection/registration: web tools register unconditionally (free MCP backstop) while check_web_api_key remains a real usability probe; explicit per-capability backends are honored (so misconfig surfaces) rather than masked by the fallback. Tested: live web_search/web_extract against search.parallel.ai in keyless and keyed modes; unit suites for the MCP client, backend selection, and display labeling; full agent run shows the "Parallel search" label on the free path. --- agent/display.py | 22 +- hermes_cli/tools_config.py | 9 +- plugins/web/parallel/provider.py | 470 ++++++++++++++++-- pyproject.toml | 2 +- tests/agent/test_display.py | 41 ++ tests/hermes_cli/test_tools_config.py | 13 + .../plugins/web/test_parallel_keyless_mcp.py | 382 ++++++++++++++ .../web/test_web_search_provider_plugins.py | 27 +- tests/tools/test_web_providers.py | 73 ++- tests/tools/test_web_providers_ddgs.py | 8 +- tests/tools/test_web_providers_searxng.py | 7 +- tests/tools/test_web_tools_config.py | 98 +++- tools/lazy_deps.py | 2 +- tools/web_tools.py | 142 +++++- uv.lock | 8 +- 15 files changed, 1206 insertions(+), 98 deletions(-) create mode 100644 tests/plugins/web/test_parallel_keyless_mcp.py diff --git a/agent/display.py b/agent/display.py index 8514279888ea..84c8509faed2 100644 --- a/agent/display.py +++ b/agent/display.py @@ -858,6 +858,20 @@ def _detect_tool_failure(tool_name: str, result: str | None) -> tuple[bool, str] return False, "" +def _used_free_parallel(result: str | None) -> bool: + """True when a web result came from Parallel's free Search MCP. + + Only the keyless Parallel path tags its result with ``provider="parallel"``; + the paid REST path and every other provider omit it. Used to label the tool + line "Parallel search" / "Parallel fetch" exactly when the free MCP served + the call. + """ + if not isinstance(result, str) or '"provider"' not in result: + return False + data = safe_json_loads(result) + return isinstance(data, dict) and str(data.get("provider", "")).lower() == "parallel" + + def get_cute_tool_message( tool_name: str, args: dict, duration: float, result: str | None = None, ) -> str: @@ -895,15 +909,17 @@ def get_cute_tool_message( return f"{line}{failure_suffix}" if tool_name == "web_search": - return _wrap(f"┊ 🔍 search {_trunc(args.get('query', ''), 42)} {dur}") + verb = "Parallel search" if _used_free_parallel(result) else "search" + return _wrap(f"┊ 🔍 {verb:<9} {_trunc(args.get('query', ''), 42)} {dur}") if tool_name == "web_extract": + verb = "Parallel fetch" if _used_free_parallel(result) else "fetch" urls = args.get("urls", []) if urls: url = urls[0] if isinstance(urls, list) else str(urls) domain = url.replace("https://", "").replace("http://", "").split("/")[0] extra = f" +{len(urls)-1}" if len(urls) > 1 else "" - return _wrap(f"┊ 📄 fetch {_trunc(domain, 35)}{extra} {dur}") - return _wrap(f"┊ 📄 fetch pages {dur}") + return _wrap(f"┊ 📄 {verb:<9} {_trunc(domain, 35)}{extra} {dur}") + return _wrap(f"┊ 📄 {verb:<9} pages {dur}") if tool_name == "terminal": return _wrap(f"┊ 💻 $ {_trunc(args.get('command', ''), 42)} {dur}") if tool_name == "process": diff --git a/hermes_cli/tools_config.py b/hermes_cli/tools_config.py index ae97dbf54a29..01d4ba727931 100644 --- a/hermes_cli/tools_config.py +++ b/hermes_cli/tools_config.py @@ -2178,8 +2178,13 @@ def _toolset_needs_configuration_prompt( tts_cfg = config.get("tts", {}) return not isinstance(tts_cfg, dict) or "provider" not in tts_cfg if ts_key == "web": - web_cfg = config.get("web", {}) - return not isinstance(web_cfg, dict) or "backend" not in web_cfg + # Web works out of the box via Parallel's free Search MCP (no key), so + # don't force setup just because ``web.backend`` is unset — only prompt + # when web isn't actually usable (e.g. an explicit backend configured + # without its credentials). Lazy import: web_tools is heavy and most + # tools_config callers don't need it. + from tools.web_tools import check_web_api_key + return not check_web_api_key() if ts_key == "browser": browser_cfg = config.get("browser", {}) return not isinstance(browser_cfg, dict) or "cloud_provider" not in browser_cfg diff --git a/plugins/web/parallel/provider.py b/plugins/web/parallel/provider.py index 38578e6b52c3..20c4291d77b9 100644 --- a/plugins/web/parallel/provider.py +++ b/plugins/web/parallel/provider.py @@ -1,14 +1,20 @@ """Parallel.ai web search + content extraction — plugin form. -Subclasses :class:`agent.web_search_provider.WebSearchProvider`. Uses two -distinct Parallel SDK clients: +Subclasses :class:`agent.web_search_provider.WebSearchProvider`. -- ``Parallel`` (sync) — for :meth:`search` -- ``AsyncParallel`` (async) — for :meth:`extract` +Search runs on one of two transports, picked by credential: -This is the first plugin to exercise the **async-extract** code path in -the ABC: :meth:`extract` is declared ``async def``, and the dispatcher -in :func:`tools.web_tools.web_extract_tool` detects coroutines via +- **No key →** the free hosted Search MCP at ``https://search.parallel.ai/mcp`` + (anonymous Streamable-HTTP JSON-RPC). This makes ``web_search`` work out of + the box with zero setup, which is why ``parallel`` is the keyless default + backend in :func:`tools.web_tools._get_backend`. +- **``PARALLEL_API_KEY`` →** the ``parallel`` SDK's v1 ``search`` / ``extract`` + REST endpoints (objective-tuned, mode-selectable, higher rate limits). + +Extract mirrors search: keyed uses the async SDK (``AsyncParallel``) v1 +``extract``; keyless uses the free MCP's ``web_fetch``. :meth:`extract` is +declared ``async def`` and the dispatcher in +:func:`tools.web_tools.web_extract_tool` detects coroutines via :func:`inspect.iscoroutinefunction` and awaits. Config keys this provider responds to:: @@ -17,25 +23,63 @@ Config keys this provider responds to:: search_backend: "parallel" # explicit per-capability extract_backend: "parallel" # explicit per-capability backend: "parallel" # shared fallback - # Optional: search mode (default "agentic"; also "fast" or "one-shot") - # via the PARALLEL_SEARCH_MODE env var. + # Optional: search mode (default "advanced"; also "basic") + # via the PARALLEL_SEARCH_MODE env var. REST path only. Env vars:: - PARALLEL_API_KEY=... # https://parallel.ai (required) - PARALLEL_SEARCH_MODE=agentic # optional: agentic|fast|one-shot + PARALLEL_API_KEY=... # https://parallel.ai (optional — unlocks + # the v1 REST Search API; without it, + # search and extract use the free MCP) + PARALLEL_SEARCH_MODE=advanced # optional: basic|advanced (legacy + # fast/one-shot map to basic, agentic to + # advanced). REST path only. """ from __future__ import annotations +import asyncio +import json import logging import os +import uuid from typing import Any, Dict, List +import httpx + from agent.web_search_provider import WebSearchProvider logger = logging.getLogger(__name__) +# Free hosted Search MCP — anonymous-friendly, used when no PARALLEL_API_KEY is +# configured. Docs: https://docs.parallel.ai/integrations/mcp/search-mcp +_MCP_SEARCH_URL = "https://search.parallel.ai/mcp" +_MCP_PROTOCOL_VERSION = "2025-06-18" +_MCP_CLIENT_NAME = "hermes-agent" +_MCP_CLIENT_VERSION = "1.0.0" +# Identify free-tier traffic at the HTTP layer. Without this, httpx sends a +# generic ``python-httpx/`` User-Agent and hermes usage is only visible +# via the JSON-RPC ``clientInfo`` payload. +_MCP_USER_AGENT = f"{_MCP_CLIENT_NAME}/{_MCP_CLIENT_VERSION}" +_MCP_TIMEOUT_SECONDS = 30.0 + +# Free-tier attribution. The hosted Search MCP is free to use; surfacing this +# on keyless results credits Parallel and matches the free-tier terms +# (https://parallel.ai/customer-terms). +_FREE_MCP_ATTRIBUTION = ( + "Search powered by the free Parallel Web Search MCP (https://parallel.ai)." +) + + +def _new_session_id() -> str: + """Mint a fresh Parallel ``session_id`` for a single tool call. + + Per-call rather than process-global: one process serves many unrelated + chats in the gateway/batch runners, and a shared id would pool their + searches into one Parallel session. + """ + return f"hermes-agent-{uuid.uuid4().hex}" + # Module-level note: the canonical cache slots ``_parallel_client`` and # ``_async_parallel_client`` live on :mod:`tools.web_tools` so tests that do # ``tools.web_tools._parallel_client = None`` between cases see fresh state. @@ -133,11 +177,319 @@ _get_async_parallel_client = _get_async_client def _resolve_search_mode() -> str: - """Return the validated PARALLEL_SEARCH_MODE value (default "agentic").""" - mode = os.getenv("PARALLEL_SEARCH_MODE", "agentic").lower().strip() - if mode not in {"fast", "one-shot", "agentic"}: - mode = "agentic" - return mode + """Return the validated v1 search mode (default "advanced"). + + V1 collapses the three Beta modes into two. We accept the v1 values + directly and map the legacy Beta values for back-compat with anyone who + still sets ``PARALLEL_SEARCH_MODE=fast|one-shot|agentic``: + + - ``fast`` / ``one-shot`` → ``basic`` (lower latency) + - ``agentic`` → ``advanced`` (higher quality, the v1 default) + """ + mode = os.getenv("PARALLEL_SEARCH_MODE", "advanced").lower().strip() + if mode == "basic" or mode in {"fast", "one-shot"}: + return "basic" + # advanced, legacy "agentic", and anything unrecognized → the v1 default. + return "advanced" + + +# --------------------------------------------------------------------------- +# Free Search MCP transport (keyless path) +# --------------------------------------------------------------------------- +# +# A small hand-rolled Streamable-HTTP JSON-RPC client for the hosted Search +# MCP, rather than the full MCP-client subsystem: we only call two tools +# (``web_search`` / ``web_fetch``), so keeping it inline lets web_search and +# web_extract stay ordinary tools with the MCP endpoint as just their wire +# protocol. + + +def _mcp_headers( + session_id: str | None, + api_key: str | None, + protocol_version: str | None = None, +) -> Dict[str, str]: + """Headers for an MCP request. + + A Bearer token is attached only when we actually hold a key — the free + endpoint is anonymous, and sending an empty/garbage token would make it + 401 instead of serving the anonymous tier. After ``initialize`` the + Streamable-HTTP spec expects the negotiated ``MCP-Protocol-Version`` on + every follow-up request, so we echo it once known. + """ + headers = { + "Content-Type": "application/json", + "Accept": "application/json, text/event-stream", + "User-Agent": _MCP_USER_AGENT, + } + if session_id: + headers["Mcp-Session-Id"] = session_id + if protocol_version: + headers["MCP-Protocol-Version"] = protocol_version + if api_key: + headers["Authorization"] = f"Bearer {api_key}" + return headers + + +def _iter_mcp_messages(text: str): + """Yield JSON-RPC message dicts from a plain-JSON or SSE response body. + + Handles ``application/json`` (a single object) and ``text/event-stream`` + (SSE: events separated by blank lines; an event's one-or-more ``data:`` + lines concatenate into a single JSON payload). Unparseable chunks and + non-``data`` SSE fields (``event:``/``id:``/comments) are skipped. + """ + def _emit(payload): + # Streamable HTTP allows batching responses/notifications into a JSON + # array — flatten so callers always see individual message dicts. + if isinstance(payload, list): + yield from payload + elif payload is not None: + yield payload + + body = (text or "").strip() + if not body: + return + if body.startswith("{") or body.startswith("["): + try: + parsed = json.loads(body) + except json.JSONDecodeError: + return + yield from _emit(parsed) + return + + data_lines: List[str] = [] + + def _flush(): + if not data_lines: + return None + try: + return json.loads("\n".join(data_lines)) + except json.JSONDecodeError: + return None + + for raw in body.split("\n"): + line = raw.rstrip("\r") + if line.startswith("data:"): + data_lines.append(line[len("data:"):].lstrip()) + elif line.strip() == "": # event boundary + yield from _emit(_flush()) + data_lines = [] + yield from _emit(_flush()) + + +def _mcp_response_envelope(text: str, request_id: str) -> Dict[str, Any]: + """Select the JSON-RPC response for *request_id* from an MCP response body. + + Streamable-HTTP servers may emit progress/log notifications before the + final result, so we scan the whole stream and return the result/error + message whose ``id`` matches our request. Falls back to the last + result/error-bearing message if no id matches; ``{}`` if none is present. + """ + fallback: Dict[str, Any] = {} + for msg in _iter_mcp_messages(text): + if not isinstance(msg, dict) or not ("result" in msg or "error" in msg): + continue + if msg.get("id") == request_id: + return msg + fallback = msg + return fallback + + +def _mcp_payload(envelope: Dict[str, Any]) -> Dict[str, Any]: + """Extract the tool result payload from a ``tools/call`` envelope. + + Prefers ``structuredContent`` (authoritative machine-readable form); + otherwise scans text blocks for the first JSON-parseable one. Raises on a + JSON-RPC error or a tool-level ``isError``. + """ + if "error" in envelope: + raise RuntimeError(f"Parallel MCP error: {str(envelope['error'])[:500]}") + result = envelope.get("result") or {} + if result.get("isError"): + raise RuntimeError(f"Parallel MCP tool error: {str(result)[:500]}") + + structured = result.get("structuredContent") + if isinstance(structured, dict): + return structured + + for block in result.get("content", []) or []: + if isinstance(block, dict) and block.get("type") == "text": + text = str(block.get("text") or "") + if not text: + continue + try: + return json.loads(text) + except json.JSONDecodeError: + continue + raise RuntimeError( + f"Parallel MCP returned no parseable content: {str(result)[:500]}" + ) + + +def _mcp_call( + tool_name: str, arguments: Dict[str, Any], api_key: str | None +) -> Dict[str, Any]: + """Run the MCP handshake then a single ``tools/call`` and return its payload. + + initialize → (capture ``Mcp-Session-Id``) → notifications/initialized → + tools/call ``tool_name``. Returns the parsed tool payload dict (see + :func:`_mcp_payload`). A Bearer token is attached only when *api_key* is set. + """ + with httpx.Client(timeout=_MCP_TIMEOUT_SECONDS) as client: + # 1. initialize — capture the server-assigned MCP session id. + init_id = str(uuid.uuid4()) + init = client.post( + _MCP_SEARCH_URL, + headers=_mcp_headers(None, api_key), + json={ + "jsonrpc": "2.0", + "id": init_id, + "method": "initialize", + "params": { + "protocolVersion": _MCP_PROTOCOL_VERSION, + "capabilities": {}, + "clientInfo": { + "name": _MCP_CLIENT_NAME, + "version": _MCP_CLIENT_VERSION, + }, + }, + }, + ) + init.raise_for_status() + # Only echo a session id the server actually issued. Stateless + # Streamable-HTTP servers may omit it; inventing one and sending it on + # follow-up requests can get those requests rejected (the server never + # created that session). When absent, the Mcp-Session-Id header is simply + # omitted (see _mcp_headers). This is separate from the tool-arg + # ``session_id`` below, which is a client-minted rate-limit/grouping id. + mcp_session_id = init.headers.get("mcp-session-id") + init_env = _mcp_response_envelope(init.text, init_id) + # Echo the negotiated protocol version on every post-init request, per + # the Streamable-HTTP spec (servers may enforce it). + negotiated_version = ( + (init_env.get("result") or {}).get("protocolVersion") + or _MCP_PROTOCOL_VERSION + ) + + # 2. notifications/initialized — required handshake ack. + client.post( + _MCP_SEARCH_URL, + headers=_mcp_headers(mcp_session_id, api_key, negotiated_version), + json={"jsonrpc": "2.0", "method": "notifications/initialized"}, + ) + + # 3. tools/call. + call_id = str(uuid.uuid4()) + call = client.post( + _MCP_SEARCH_URL, + headers=_mcp_headers(mcp_session_id, api_key, negotiated_version), + json={ + "jsonrpc": "2.0", + "id": call_id, + "method": "tools/call", + "params": {"name": tool_name, "arguments": arguments}, + }, + ) + call.raise_for_status() + return _mcp_payload(_mcp_response_envelope(call.text, call_id)) + + +def _mcp_web_search(query: str, limit: int, api_key: str | None) -> Dict[str, Any]: + """Run a ``web_search`` tool call against the hosted Search MCP. + + Returns the standard provider search shape + (``{"success": True, "data": {"web": [...]}}``). The MCP serves a fixed + result count, so ``limit`` is applied client-side. The MCP requires + ``objective`` (REST treats it as optional), so we mirror the query. + """ + payload = _mcp_call( + "web_search", + { + "objective": query, + "search_queries": [query], + "session_id": _new_session_id(), + }, + api_key, + ) + + web_results: List[Dict[str, Any]] = [] + for i, result in enumerate((payload.get("results") or [])[: max(limit, 1)]): + if not isinstance(result, dict): + continue + excerpts = result.get("excerpts") or [] + web_results.append( + { + "url": result.get("url") or "", + "title": result.get("title") or "", + "description": " ".join(excerpts) if excerpts else "", + "position": i + 1, + } + ) + + # Credit the free tier (anonymous path only — keyed search uses REST and + # carries no attribution). + return { + "success": True, + "data": {"web": web_results}, + "provider": "parallel", + "attribution": _FREE_MCP_ATTRIBUTION, + } + + +def _mcp_web_fetch(urls: List[str], api_key: str | None) -> List[Dict[str, Any]]: + """Run a ``web_fetch`` tool call against the hosted Search MCP. + + Returns the per-URL extract shape that + :func:`tools.web_tools.web_extract_tool` expects — exactly one row per input + URL, in request order (including duplicates). We pass ``full_content=True`` + so the page body comes back as markdown (matching the keyed SDK path and + what extract callers/summarizers expect), falling back to excerpts only when + full content is absent. Any input the MCP didn't return is emitted as a + per-URL error row. + """ + payload = _mcp_call( + "web_fetch", + {"urls": list(urls), "full_content": True, "session_id": _new_session_id()}, + api_key, + ) + + # Index the response by URL, then emit one row per *input* URL in order so + # duplicates and positional alignment with the request list are preserved. + by_url: Dict[str, Dict[str, Any]] = {} + for item in payload.get("results") or []: + if isinstance(item, dict) and item.get("url"): + by_url.setdefault(item["url"], item) + + results: List[Dict[str, Any]] = [] + for url in urls: + item = by_url.get(url) + if item is None: + results.append( + { + "url": url, + "title": "", + "content": "", + "error": "extraction failed (no content returned)", + "metadata": {"sourceURL": url}, + } + ) + continue + title = item.get("title") or "" + # Prefer the full page body; fall back to joined excerpts (mirrors the + # keyed SDK extract path). + content = item.get("full_content") or "\n\n".join(item.get("excerpts") or []) + results.append( + { + "url": url, + "title": title, + "content": content, + "raw_content": content, + "metadata": {"sourceURL": url, "title": title}, + } + ) + + return results class ParallelWebSearchProvider(WebSearchProvider): @@ -152,7 +504,14 @@ class ParallelWebSearchProvider(WebSearchProvider): return "Parallel" def is_available(self) -> bool: - """Return True when ``PARALLEL_API_KEY`` is set to a non-empty value.""" + """Return True when ``PARALLEL_API_KEY`` is set. + + Deliberately key-based: this gates the registry's active-provider walk + and the ``hermes tools`` picker (auto-selecting Parallel for a user who + hasn't named it), so it must not claim availability on the keyless path. + The keyless free-MCP path is reached independently via + :func:`tools.web_tools._get_backend`'s ``parallel`` terminal default. + """ return bool(os.getenv("PARALLEL_API_KEY", "").strip()) def supports_search(self) -> bool: @@ -164,9 +523,11 @@ class ParallelWebSearchProvider(WebSearchProvider): def search(self, query: str, limit: int = 5) -> Dict[str, Any]: """Execute a Parallel search (sync). - Uses the ``beta.search`` endpoint with the configured mode - (``PARALLEL_SEARCH_MODE`` env var, default "agentic"). Limit is - capped at 20 server-side. + With ``PARALLEL_API_KEY`` set, uses the v1 ``search`` REST endpoint with + the configured mode (``PARALLEL_SEARCH_MODE`` env var, default + "advanced"; limit requested via advanced_settings.max_results, capped at + 20). Without a key, falls back to the free hosted Search MCP so search + still works with zero setup. """ try: from tools.interrupt import is_interrupted @@ -174,19 +535,31 @@ class ParallelWebSearchProvider(WebSearchProvider): if is_interrupted(): return {"success": False, "error": "Interrupted"} + api_key = os.getenv("PARALLEL_API_KEY", "").strip() + if not api_key: + logger.info( + "Parallel search (free MCP): '%s' (limit=%d)", query, limit + ) + return _mcp_web_search(query, limit, api_key=None) + mode = _resolve_search_mode() logger.info( - "Parallel search: '%s' (mode=%s, limit=%d)", query, mode, limit + "Parallel search (v1 REST): '%s' (mode=%s, limit=%d)", + query, mode, limit, ) - response = _get_sync_client().beta.search( + # v1 Search API. Request the caller's limit via max_results (capped + # at 20) so we don't rely on the API default — the slice below can + # only trim, not ask for more. + response = _get_sync_client().search( search_queries=[query], objective=query, mode=mode, - max_results=min(limit, 20), + session_id=_new_session_id(), + advanced_settings={"max_results": min(max(limit, 1), 20)}, ) web_results = [] - for i, result in enumerate(response.results or []): + for i, result in enumerate((response.results or [])[: max(limit, 1)]): excerpts = result.excerpts or [] web_results.append( { @@ -197,6 +570,8 @@ class ParallelWebSearchProvider(WebSearchProvider): } ) + # Paid/REST path: no attribution and no "[Parallel]" label — the + # branding is specifically for the free Search MCP tier. return {"success": True, "data": {"web": web_results}} except ValueError as exc: return {"success": False, "error": str(exc)} @@ -212,7 +587,12 @@ class ParallelWebSearchProvider(WebSearchProvider): async def extract( self, urls: List[str], **kwargs: Any ) -> List[Dict[str, Any]]: - """Extract content from one or more URLs via the async SDK. + """Extract content from one or more URLs. + + With ``PARALLEL_API_KEY`` set, uses the async SDK's v1 ``extract`` for + full page content. Without a key, falls back to the free hosted Search + MCP's ``web_fetch`` tool so extraction works with zero setup, mirroring + the keyless search path. Returns the legacy list-of-results shape that :func:`tools.web_tools.web_extract_tool` expects: one entry per @@ -227,10 +607,21 @@ class ParallelWebSearchProvider(WebSearchProvider): {"url": u, "error": "Interrupted", "title": ""} for u in urls ] - logger.info("Parallel extract: %d URL(s)", len(urls)) - response = await _get_async_client().beta.extract( + api_key = os.getenv("PARALLEL_API_KEY", "").strip() + if not api_key: + logger.info( + "Parallel extract (free MCP web_fetch): %d URL(s)", len(urls) + ) + # _mcp_web_fetch is sync httpx; run off the event loop. + return await asyncio.to_thread(_mcp_web_fetch, list(urls), None) + + logger.info("Parallel extract (v1 REST): %d URL(s)", len(urls)) + # v1 Extract API (client.extract, /v1/extract); full_content is set + # via advanced_settings. + response = await _get_async_client().extract( urls=urls, - full_content=True, + advanced_settings={"full_content": True}, + session_id=_new_session_id(), ) results: List[Dict[str, Any]] = [] @@ -251,13 +642,20 @@ class ParallelWebSearchProvider(WebSearchProvider): ) for error in response.errors or []: + err_url = getattr(error, "url", "") or "" + err_msg = ( + getattr(error, "message", None) + or getattr(error, "content", None) + or getattr(error, "error_type", None) + or "extraction failed" + ) results.append( { - "url": error.url or "", + "url": err_url, "title": "", "content": "", - "error": error.content or error.error_type or "extraction failed", - "metadata": {"sourceURL": error.url or ""}, + "error": err_msg, + "metadata": {"sourceURL": err_url}, } ) @@ -279,12 +677,16 @@ class ParallelWebSearchProvider(WebSearchProvider): def get_setup_schema(self) -> Dict[str, Any]: return { "name": "Parallel", - "badge": "paid", - "tag": "Objective-tuned search + parallel page extraction.", + "badge": "free", + "tag": ( + "Free web search + extraction via Parallel's hosted Search MCP " + "— no key needed. Add PARALLEL_API_KEY for the v1 REST Search " + "API (richer modes, higher limits)." + ), "env_vars": [ { "key": "PARALLEL_API_KEY", - "prompt": "Parallel API key", + "prompt": "Parallel API key (optional — unlocks the v1 REST Search API)", "url": "https://parallel.ai", }, ], diff --git a/pyproject.toml b/pyproject.toml index b2a486aefd0f..e5bf882d87a4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -122,7 +122,7 @@ anthropic = ["anthropic==0.87.0"] # CVE-2026-34450, CVE-2026-34452 # search provider (configured via `hermes tools` or config.yaml). exa = ["exa-py==2.10.2"] firecrawl = ["firecrawl-py==4.17.0"] -parallel-web = ["parallel-web==0.4.2"] +parallel-web = ["parallel-web==0.6.0"] # Image generation backends fal = ["fal-client==0.13.1"] # Edge TTS — default TTS provider but still optional (users can pick diff --git a/tests/agent/test_display.py b/tests/agent/test_display.py index 994aae286484..0203e38b3cd7 100644 --- a/tests/agent/test_display.py +++ b/tests/agent/test_display.py @@ -12,6 +12,7 @@ from agent.display import ( set_tool_preview_max_len, _render_inline_unified_diff, _summarize_rendered_diff_sections, + _used_free_parallel, render_edit_diff_with_delta, ) @@ -171,6 +172,46 @@ class TestCuteToolMessagePreviewLength: assert "[error]" not in line +class TestWebProviderLabel: + """The free-path "Parallel search"/"Parallel fetch" verb labeling.""" + + def test_free_search_verb_is_parallel(self): + result = json.dumps({"success": True, "data": {"web": []}, "provider": "parallel"}) + line = get_cute_tool_message("web_search", {"query": "hello"}, 0.1, result=result) + assert "Parallel search" in line + assert "hello" in line + + def test_paid_search_verb_is_plain(self): + result = json.dumps({"success": True, "data": {"web": [{"url": "u"}]}}) + line = get_cute_tool_message("web_search", {"query": "hi"}, 0.1, result=result) + assert "Parallel" not in line + assert "search" in line + + def test_missing_result_verb_is_plain(self): + line = get_cute_tool_message("web_search", {"query": "hello"}, 0.1) + assert "Parallel" not in line + assert "search" in line + + def test_helper_is_parallel_free_specific(self): + # Only Parallel's free MCP path marks results; nothing else does. + assert _used_free_parallel(json.dumps({"provider": "parallel"})) is True + assert _used_free_parallel(json.dumps({"provider": "exa"})) is False + assert _used_free_parallel(json.dumps({"provider": "firecrawl"})) is False + assert _used_free_parallel(json.dumps({"success": True, "data": {}})) is False + assert _used_free_parallel('not json') is False + assert _used_free_parallel(None) is False + + def test_free_extract_verb_is_parallel(self): + result = json.dumps({"results": [{"url": "u", "content": "x"}], "provider": "parallel"}) + line = get_cute_tool_message("web_extract", {"urls": ["https://a.test"]}, 0.1, result=result) + assert "Parallel fetch" in line + + def test_paid_extract_verb_is_plain(self): + result = json.dumps({"results": [{"url": "u", "content": "x"}]}) + line = get_cute_tool_message("web_extract", {"urls": ["https://a.test"]}, 0.1, result=result) + assert "Parallel" not in line + + class TestEditDiffPreview: def test_extract_edit_diff_for_patch(self): diff = extract_edit_diff("patch", '{"success": true, "diff": "--- a/x\\n+++ b/x\\n"}') diff --git a/tests/hermes_cli/test_tools_config.py b/tests/hermes_cli/test_tools_config.py index 5b24d2b6ebd2..3a68c9758973 100644 --- a/tests/hermes_cli/test_tools_config.py +++ b/tests/hermes_cli/test_tools_config.py @@ -975,6 +975,19 @@ def test_toolset_has_keys_treats_no_key_providers_as_configured(): assert _toolset_has_keys("computer_use", config) is True +def test_web_no_prompt_when_usable_keyless(): + """Fresh install: web works via the free Parallel MCP, so enabling the web + toolset should not force provider setup.""" + with patch("tools.web_tools.check_web_api_key", return_value=True): + assert _toolset_needs_configuration_prompt("web", {}) is False + + +def test_web_no_prompt_when_extract_backend_is_extract_capable(): + with patch("tools.web_tools.check_web_api_key", return_value=True): + cfg = {"web": {"extract_backend": "parallel"}} + assert _toolset_needs_configuration_prompt("web", cfg) is False + + def test_computer_use_needs_configuration_when_cua_driver_post_setup_pending(): """No-key providers can still need setup when their post_setup is unsatisfied. diff --git a/tests/plugins/web/test_parallel_keyless_mcp.py b/tests/plugins/web/test_parallel_keyless_mcp.py new file mode 100644 index 000000000000..49975c47f2f0 --- /dev/null +++ b/tests/plugins/web/test_parallel_keyless_mcp.py @@ -0,0 +1,382 @@ +"""Keyless Parallel search via the free hosted Search MCP. + +Covers the transport added in ``plugins/web/parallel/provider.py`` that lets +``web_search`` work with no ``PARALLEL_API_KEY``: + +- ``_mcp_headers`` — Bearer attached only when a key is held +- ``_decode_mcp_envelope`` — plain-JSON and SSE (``data:``) response bodies +- ``_mcp_payload`` — structuredContent preferred, text-block JSON fallback, errors +- ``_mcp_web_search`` — full handshake (mocked transport) → standard search shape +- ``ParallelWebSearchProvider.search`` — keyless path routes to the MCP +""" + +from __future__ import annotations + +import asyncio +import json +from unittest.mock import patch + +import pytest + +import plugins.web.parallel.provider as pp + + +# ─── _mcp_headers ────────────────────────────────────────────────────────── + +class TestMcpHeaders: + def test_anonymous_has_no_authorization(self): + h = pp._mcp_headers(session_id=None, api_key=None) + assert "Authorization" not in h + assert h["Accept"] == "application/json, text/event-stream" + assert "Mcp-Session-Id" not in h + + def test_identifies_hermes_via_user_agent(self): + # Free-tier traffic is attributable at the HTTP layer (not just via the + # JSON-RPC clientInfo payload), on both the anonymous and keyed paths. + assert pp._mcp_headers(session_id=None, api_key=None)["User-Agent"].startswith( + "hermes-agent/" + ) + assert pp._mcp_headers(session_id="sid", api_key="pk-live")["User-Agent"].startswith( + "hermes-agent/" + ) + + def test_session_id_and_bearer_when_present(self): + h = pp._mcp_headers(session_id="sid-123", api_key="pk-live") + assert h["Mcp-Session-Id"] == "sid-123" + assert h["Authorization"] == "Bearer pk-live" + + +# ─── SSE / JSON-RPC parsing ────────────────────────────────────────────────── + +class TestMcpResponseParsing: + def test_plain_json_matched_by_id(self): + body = '{"jsonrpc":"2.0","id":"abc","result":{"ok":true}}' + assert pp._mcp_response_envelope(body, "abc")["result"]["ok"] is True + + def test_sse_selects_response_for_request_id_skipping_notifications(self): + # A progress notification (no id) precedes the real result; an unrelated + # response id is also present. We must pick the one matching our id. + body = ( + 'event: message\ndata: {"jsonrpc":"2.0","method":"notifications/progress","params":{"p":1}}\n\n' + 'event: message\ndata: {"jsonrpc":"2.0","id":"other","result":{"ok":false}}\n\n' + 'event: message\ndata: {"jsonrpc":"2.0","id":"req-1","result":{"ok":true}}\n\n' + ) + env = pp._mcp_response_envelope(body, "req-1") + assert env["result"]["ok"] is True + + def test_sse_multiline_data_concatenated(self): + body = 'data: {"jsonrpc":"2.0","id":"x",\ndata: "result":{"n":42}}\n\n' + assert pp._mcp_response_envelope(body, "x")["result"]["n"] == 42 + + def test_falls_back_to_last_result_when_id_absent(self): + body = '{"jsonrpc":"2.0","id":"server-chose","result":{"ok":true}}' + # request id doesn't match, but there's a single result → use it + assert pp._mcp_response_envelope(body, "mismatch")["result"]["ok"] is True + + def test_empty_body(self): + assert pp._mcp_response_envelope("", "x") == {} + assert pp._mcp_response_envelope(" ", "x") == {} + + def test_batched_json_array_flattened(self): + # Streamable HTTP may batch messages into a JSON array. + body = ('[{"jsonrpc":"2.0","method":"notifications/progress"},' + '{"jsonrpc":"2.0","id":"req-9","result":{"ok":true}}]') + assert pp._mcp_response_envelope(body, "req-9")["result"]["ok"] is True + + def test_batched_sse_data_array_flattened(self): + body = 'data: [{"jsonrpc":"2.0","id":"a","result":{"n":1}}]\n\n' + assert pp._mcp_response_envelope(body, "a")["result"]["n"] == 1 + + +# ─── _mcp_payload ──────────────────────────────────────────────────────────── + +class TestMcpPayload: + def test_prefers_structured_content(self): + env = {"result": {"structuredContent": {"results": [{"url": "u"}]}, + "content": [{"type": "text", "text": "ignored"}]}} + assert pp._mcp_payload(env) == {"results": [{"url": "u"}]} + + def test_parses_text_block_json(self): + inner = {"search_id": "s1", "results": [{"url": "u", "title": "t"}]} + env = {"result": {"content": [{"type": "text", "text": json.dumps(inner)}]}} + assert pp._mcp_payload(env)["search_id"] == "s1" + + def test_raises_on_jsonrpc_error(self): + with pytest.raises(RuntimeError, match="Parallel MCP error"): + pp._mcp_payload({"error": {"code": -32000, "message": "boom"}}) + + def test_raises_on_tool_iserror(self): + with pytest.raises(RuntimeError, match="Parallel MCP tool error"): + pp._mcp_payload({"result": {"isError": True, "content": []}}) + + +# ─── _mcp_web_search (mocked transport) ────────────────────────────────────── + +class _FakeResponse: + def __init__(self, *, text="", headers=None): + self.text = text + self.headers = headers or {} + + def raise_for_status(self): + return None + + +class _FakeClient: + """Stands in for httpx.Client: replays init → ack → tools/call.""" + + def __init__(self, search_payload, init_session_id="server-sid"): + self._search_payload = search_payload + self._init_session_id = init_session_id + self.calls = [] + + def __enter__(self): + return self + + def __exit__(self, *exc): + return False + + def post(self, url, headers=None, json=None): + self.calls.append({"headers": headers, "json": json}) + req = json or {} + method = req.get("method") + req_id = req.get("id") + if method == "initialize": + # Echo the request id, as the real server does. + return _FakeResponse( + text=json_dumps({"jsonrpc": "2.0", "id": req_id, + "result": {"protocolVersion": "2099-01-01"}}), + headers=( + {"mcp-session-id": self._init_session_id} + if self._init_session_id is not None + else {} + ), + ) + if method == "notifications/initialized": + return _FakeResponse(text="") + # tools/call + envelope = {"jsonrpc": "2.0", "id": req_id, "result": { + "content": [{"type": "text", "text": json_dumps(self._search_payload)}], + }} + return _FakeResponse(text=json_dumps(envelope)) + + +def json_dumps(obj): + return json.dumps(obj) + + +class TestMcpWebSearch: + def _payload(self, n): + return {"search_id": "s", "results": [ + {"url": f"https://ex/{i}", "title": f"t{i}", + "excerpts": [f"a{i}", f"b{i}"]} + for i in range(n) + ]} + + def test_returns_standard_shape_and_handshake(self): + fake = _FakeClient(self._payload(3)) + with patch.object(pp.httpx, "Client", return_value=fake): + out = pp._mcp_web_search("hello", limit=5, api_key=None) + + assert out["success"] is True + # Free-tier results credit Parallel. + assert "Parallel" in out["attribution"] + web = out["data"]["web"] + assert [r["position"] for r in web] == [1, 2, 3] + assert web[0]["url"] == "https://ex/0" + assert web[0]["description"] == "a0 b0" # excerpts joined + # handshake order + methods = [c["json"].get("method") for c in fake.calls] + assert methods == ["initialize", "notifications/initialized", "tools/call"] + # session id from the initialize response header is reused + assert fake.calls[-1]["headers"]["Mcp-Session-Id"] == "server-sid" + + def test_stateless_server_no_session_header_not_invented(self): + # A stateless Streamable-HTTP server may omit mcp-session-id on + # initialize; we must NOT invent one (sending an unissued session id can + # get follow-up requests rejected). The follow-ups carry no header. + fake = _FakeClient(self._payload(1), init_session_id=None) + with patch.object(pp.httpx, "Client", return_value=fake): + out = pp._mcp_web_search("hello", limit=5, api_key=None) + assert out["success"] is True + follow_ups = [c for c in fake.calls if c["json"].get("method") != "initialize"] + assert follow_ups, "expected notifications/initialized + tools/call" + assert all("Mcp-Session-Id" not in c["headers"] for c in follow_ups) + # anonymous → no Authorization on any call + assert all("Authorization" not in c["headers"] for c in fake.calls) + # tools/call mirrors query into objective + search_queries + args = fake.calls[-1]["json"]["params"]["arguments"] + assert args["objective"] == "hello" + assert args["search_queries"] == ["hello"] + + def test_limit_is_applied_client_side(self): + fake = _FakeClient(self._payload(10)) + with patch.object(pp.httpx, "Client", return_value=fake): + out = pp._mcp_web_search("q", limit=2, api_key=None) + assert len(out["data"]["web"]) == 2 + + def test_bearer_attached_when_key_present(self): + fake = _FakeClient(self._payload(1)) + with patch.object(pp.httpx, "Client", return_value=fake): + pp._mcp_web_search("q", limit=1, api_key="pk-live") + assert all(c["headers"]["Authorization"] == "Bearer pk-live" for c in fake.calls) + + def test_negotiated_protocol_version_echoed_post_init(self): + fake = _FakeClient(self._payload(1)) + with patch.object(pp.httpx, "Client", return_value=fake): + pp._mcp_web_search("q", limit=1, api_key=None) + # initialize request doesn't carry the (not-yet-negotiated) version... + assert "MCP-Protocol-Version" not in fake.calls[0]["headers"] + # ...but notifications/initialized and tools/call echo the negotiated one. + assert fake.calls[1]["headers"]["MCP-Protocol-Version"] == "2099-01-01" + assert fake.calls[-1]["headers"]["MCP-Protocol-Version"] == "2099-01-01" + + +# ─── provider.search keyless routing ───────────────────────────────────────── + +class TestProviderKeylessSearch: + def test_search_without_key_uses_mcp(self, monkeypatch): + monkeypatch.delenv("PARALLEL_API_KEY", raising=False) + captured = {} + + def _fake(query, limit, api_key): + captured.update(query=query, limit=limit, api_key=api_key) + return {"success": True, "data": {"web": []}} + + monkeypatch.setattr(pp, "_mcp_web_search", _fake) + out = pp.ParallelWebSearchProvider().search("kittens", limit=4) + assert out["success"] is True + assert captured == {"query": "kittens", "limit": 4, "api_key": None} + + def test_is_available_reflects_key(self, monkeypatch): + # is_available() gates the registry's active-provider walk + picker, so + # it's key-based (keyless dispatch is handled by _get_backend, not this). + monkeypatch.delenv("PARALLEL_API_KEY", raising=False) + assert pp.ParallelWebSearchProvider().is_available() is False + monkeypatch.setenv("PARALLEL_API_KEY", "k") + assert pp.ParallelWebSearchProvider().is_available() is True + + +# ─── web_fetch (keyless extract) ───────────────────────────────────────────── + +class TestMcpWebFetch: + def _payload(self, urls): + return {"extract_id": "e1", "results": [ + {"url": u, "title": f"T{i}", "publish_date": None, + "excerpts": [f"chunk-a-{i}", f"chunk-b-{i}"]} + for i, u in enumerate(urls) + ]} + + def test_maps_to_extract_shape(self): + urls = ["https://a.test", "https://b.test"] + fake = _FakeClient(self._payload(urls)) + with patch.object(pp.httpx, "Client", return_value=fake): + out = pp._mcp_web_fetch(urls, api_key=None) + assert [r["url"] for r in out] == urls + assert out[0]["content"] == "chunk-a-0\n\nchunk-b-0" + assert out[0]["raw_content"] == out[0]["content"] + assert out[0]["metadata"] == {"sourceURL": "https://a.test", "title": "T0"} + # tools/call targeted web_fetch, requesting full page bodies. + args = fake.calls[-1]["json"]["params"] + assert args["name"] == "web_fetch" + assert args["arguments"]["urls"] == urls + assert args["arguments"]["full_content"] is True + assert args["arguments"]["session_id"].startswith("hermes-agent-") + + def test_prefers_full_content_over_excerpts(self): + payload = {"results": [ + {"url": "https://a.test", "title": "T", + "excerpts": ["snippet"], "full_content": "the entire page body"}, + ]} + fake = _FakeClient(payload) + with patch.object(pp.httpx, "Client", return_value=fake): + out = pp._mcp_web_fetch(["https://a.test"], api_key=None) + assert out[0]["content"] == "the entire page body" + + def test_missing_url_becomes_error_entry(self): + # Server returns only one of the two requested URLs. + fake = _FakeClient(self._payload(["https://a.test"])) + with patch.object(pp.httpx, "Client", return_value=fake): + out = pp._mcp_web_fetch(["https://a.test", "https://missing.test"], api_key=None) + assert len(out) == 2 + missing = [r for r in out if r["url"] == "https://missing.test"][0] + assert "error" in missing + assert missing["content"] == "" + + def test_preserves_order_and_duplicate_inputs(self): + # MCP returns each unique URL once; output must still be one row per + # input, in order, including the duplicate. + fake = _FakeClient(self._payload(["https://a.test", "https://b.test"])) + urls = ["https://b.test", "https://a.test", "https://b.test"] + with patch.object(pp.httpx, "Client", return_value=fake): + out = pp._mcp_web_fetch(urls, api_key=None) + assert [r["url"] for r in out] == urls # one row per input, in order + assert all("error" not in r for r in out) # all three resolved + + def test_extract_without_key_uses_web_fetch(self, monkeypatch): + monkeypatch.delenv("PARALLEL_API_KEY", raising=False) + captured = {} + + def _fake(urls, api_key): + captured.update(urls=list(urls), api_key=api_key) + return [{"url": urls[0], "title": "", "content": "x", + "raw_content": "x", "metadata": {}}] + + monkeypatch.setattr(pp, "_mcp_web_fetch", _fake) + out = asyncio.run(pp.ParallelWebSearchProvider().extract(["https://x.test"])) + assert out[0]["content"] == "x" + assert captured == {"urls": ["https://x.test"], "api_key": None} + + +# ─── keyed v1 REST search ──────────────────────────────────────────────────── + +class TestKeyedV1Search: + def test_passes_max_results_and_omits_branding(self, monkeypatch): + monkeypatch.setenv("PARALLEL_API_KEY", "pk-live") + monkeypatch.delenv("PARALLEL_SEARCH_MODE", raising=False) + captured = {} + + class _Res: + def __init__(self, url): + self.url, self.title, self.excerpts = url, "T", ["x"] + + class _Resp: + results = [_Res(f"https://r/{i}") for i in range(10)] + + class _Client: + def search(self, **kw): + captured.update(kw) + return _Resp() + + monkeypatch.setattr(pp, "_get_sync_client", lambda: _Client()) + out = pp.ParallelWebSearchProvider().search("q", limit=7) + + assert out["success"] is True + # honors the caller's limit via advanced_settings.max_results + assert captured["advanced_settings"] == {"max_results": 7} + assert captured["mode"] == "advanced" # v1 default + assert captured["session_id"].startswith("hermes-agent-") # per-call id + assert len(out["data"]["web"]) == 7 # client-side slice + # paid path: no free-tier attribution, no [Parallel] label signal + assert "attribution" not in out + assert "provider" not in out + + +# ─── v1 search mode mapping ────────────────────────────────────────────────── + +class TestResolveSearchMode: + @pytest.mark.parametrize("env,expected", [ + (None, "advanced"), # default + ("advanced", "advanced"), + ("basic", "basic"), + ("fast", "basic"), # legacy → basic + ("one-shot", "basic"), # legacy → basic + ("agentic", "advanced"), # legacy → advanced + ("garbage", "advanced"), # invalid → default + ("BASIC", "basic"), # case-insensitive + ]) + def test_mode_mapping(self, monkeypatch, env, expected): + if env is None: + monkeypatch.delenv("PARALLEL_SEARCH_MODE", raising=False) + else: + monkeypatch.setenv("PARALLEL_SEARCH_MODE", env) + assert pp._resolve_search_mode() == expected diff --git a/tests/plugins/web/test_web_search_provider_plugins.py b/tests/plugins/web/test_web_search_provider_plugins.py index 2177d875c4b1..2d74b2a18137 100644 --- a/tests/plugins/web/test_web_search_provider_plugins.py +++ b/tests/plugins/web/test_web_search_provider_plugins.py @@ -193,11 +193,16 @@ class TestIsAvailable: assert p.is_available() is True def test_parallel_requires_api_key(self, monkeypatch: pytest.MonkeyPatch) -> None: + """is_available() is key-based — it gates the registry's active-provider + walk/picker. (Keyless search/extract still work via the free MCP through + _get_backend's terminal default, independent of this flag.) + """ _ensure_plugins_loaded() from agent.web_search_registry import get_provider p = get_provider("parallel") assert p is not None + monkeypatch.delenv("PARALLEL_API_KEY", raising=False) assert p.is_available() is False monkeypatch.setenv("PARALLEL_API_KEY", "real") assert p.is_available() is True @@ -422,17 +427,33 @@ class TestErrorResponseShapes: assert result.get("success") is False assert "error" in result - def test_parallel_extract_returns_per_url_errors_when_unconfigured(self) -> None: + def test_parallel_extract_keyless_uses_mcp_web_fetch( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Without a key, extract routes to the free MCP web_fetch tool rather + than erroring. The MCP transport is mocked so the test stays offline.""" _ensure_plugins_loaded() from agent.web_search_registry import get_provider + import plugins.web.parallel.provider as parallel_provider + + monkeypatch.delenv("PARALLEL_API_KEY", raising=False) + captured = {} + + def _fake_fetch(urls, api_key): + captured["urls"] = list(urls) + captured["api_key"] = api_key + return [{"url": urls[0], "title": "Example", "content": "body", + "raw_content": "body", "metadata": {"sourceURL": urls[0]}}] + + monkeypatch.setattr(parallel_provider, "_mcp_web_fetch", _fake_fetch) p = get_provider("parallel") assert p is not None result = asyncio.run(p.extract(["https://example.com"])) assert isinstance(result, list) - assert len(result) == 1 - assert "error" in result[0] assert result[0]["url"] == "https://example.com" + assert result[0]["content"] == "body" + assert captured == {"urls": ["https://example.com"], "api_key": None} def test_firecrawl_extract_returns_per_url_errors_when_unconfigured(self) -> None: _ensure_plugins_loaded() diff --git a/tests/tools/test_web_providers.py b/tests/tools/test_web_providers.py index bd3cce8754aa..230f909b5fea 100644 --- a/tests/tools/test_web_providers.py +++ b/tests/tools/test_web_providers.py @@ -167,6 +167,21 @@ class TestPerCapabilityBackendSelection: monkeypatch.setenv("TAVILY_API_KEY", "test-key") assert web_tools._get_search_backend() == "tavily" + def test_explicit_extract_backend_honored_when_unavailable(self, monkeypatch): + """An explicit per-capability backend is honored even with no creds, so + its setup error surfaces instead of silently rerouting to the keyless + Parallel default (which would send user URLs to a different provider).""" + from tools import web_tools + + monkeypatch.setattr(web_tools, "_load_web_config", lambda: { + "extract_backend": "firecrawl", + }) + for key in ("FIRECRAWL_API_KEY", "FIRECRAWL_API_URL", "FIRECRAWL_GATEWAY_URL"): + monkeypatch.delenv(key, raising=False) + monkeypatch.setattr(web_tools, "_is_tool_gateway_ready", lambda: False, raising=False) + # Resolves to firecrawl (not parallel) despite firecrawl being unavailable. + assert web_tools._get_extract_backend() == "firecrawl" + def test_falls_back_to_generic_backend_when_extract_backend_empty(self, monkeypatch): from tools import web_tools @@ -177,7 +192,7 @@ class TestPerCapabilityBackendSelection: monkeypatch.setenv("PARALLEL_API_KEY", "test-key") assert web_tools._get_extract_backend() == "parallel" - def test_search_backend_ignored_when_not_available(self, monkeypatch): + def test_explicit_search_backend_honored_when_unavailable(self, monkeypatch): from tools import web_tools monkeypatch.setattr(web_tools, "_load_web_config", lambda: { @@ -186,8 +201,10 @@ class TestPerCapabilityBackendSelection: }) monkeypatch.delenv("EXA_API_KEY", raising=False) monkeypatch.setenv("FIRECRAWL_API_KEY", "fc-key") - # Should fall back to firecrawl since exa isn't configured - assert web_tools._get_search_backend() == "firecrawl" + # The explicit per-capability choice (exa) is honored even though it's + # unavailable, so its setup error surfaces — we don't silently reroute + # to the shared backend (or the keyless Parallel default). + assert web_tools._get_search_backend() == "exa" def test_fully_backward_compatible_with_web_backend_only(self, monkeypatch): from tools import web_tools @@ -291,25 +308,55 @@ class TestUnconfiguredErrorEnvelopeParity: ): monkeypatch.delenv(k, raising=False) - def test_unconfigured_search_emits_top_level_error(self, monkeypatch): - """``web_search_tool`` with no creds returns ``{"error": "Error searching web: ..."}`` - — matching main's ``tool_error()`` envelope, not a per-result shape. + def test_extract_empty_urls_does_not_raise(self, monkeypatch): + """Regression: empty (or fully SSRF-blocked) URL sets skip the dispatch + branch; the free-Parallel flag must still be initialized so the tool + returns an error envelope instead of UnboundLocalError.""" + import asyncio + from tools import web_tools + self._clear_web_creds(monkeypatch) + monkeypatch.setattr(web_tools, "_load_web_config", lambda: {}) + out = asyncio.run(web_tools.web_extract_tool([], "markdown")) + # The key assertion is that it returns a normal error envelope (a + # string) rather than raising UnboundLocalError. + assert isinstance(out, str) + result = json.loads(out) + assert "error" in result + + def test_unconfigured_search_falls_back_to_free_parallel(self, monkeypatch): + """``web_search_tool`` with no creds routes to Parallel's free Search + MCP rather than erroring. The MCP transport is mocked so the test + stays offline; we assert dispatch landed on parallel and returned the + standard search envelope. """ from tools import web_tools + import plugins.web.parallel.provider as parallel_provider self._clear_web_creds(monkeypatch) - # Reset firecrawl client cache so the unconfigured state is re-evaluated monkeypatch.setattr(web_tools, "_firecrawl_client", None, raising=False) monkeypatch.setattr(web_tools, "_firecrawl_client_config", None, raising=False) monkeypatch.setattr(web_tools, "_load_web_config", lambda: {}) + captured = {} + + def _fake_mcp(query, limit, api_key): + captured["query"] = query + captured["api_key"] = api_key + return { + "success": True, + "data": {"web": [ + {"url": "https://example.com", "title": "Example", + "description": "hit", "position": 1}, + ]}, + } + + monkeypatch.setattr(parallel_provider, "_mcp_web_search", _fake_mcp) + result = json.loads(web_tools.web_search_tool("hello world", limit=3)) - assert "error" in result, f"expected top-level 'error' key, got {result}" - # ``Error searching web:`` prefix comes from web_tools' top-level except handler - assert "Error searching web:" in result["error"] - assert "FIRECRAWL_API_KEY" in result["error"] - # No per-result burying - assert "results" not in result + assert result.get("success") is True, f"expected success, got {result}" + assert result["data"]["web"][0]["url"] == "https://example.com" + # Keyless path: dispatched to parallel with no Bearer token. + assert captured == {"query": "hello world", "api_key": None} class TestDispatchersTriggerPluginDiscovery: diff --git a/tests/tools/test_web_providers_ddgs.py b/tests/tools/test_web_providers_ddgs.py index 283a25f0a1be..1050a4e554a5 100644 --- a/tests/tools/test_web_providers_ddgs.py +++ b/tests/tools/test_web_providers_ddgs.py @@ -190,7 +190,11 @@ class TestDDGSBackendWiring: monkeypatch.setattr(web_tools, "_ddgs_package_importable", lambda: True) assert web_tools._get_backend() == "exa" - def test_auto_detect_picks_ddgs_as_last_resort(self, monkeypatch): + def test_auto_detect_prefers_keyless_parallel_over_ddgs(self, monkeypatch): + # With no credentials, keyless Parallel is the auto-detect default even + # when the ddgs package is installed — ddgs is search-only (can't + # extract), so Parallel is preferred so both search and extract work. + # ddgs remains reachable via an explicit web.backend=ddgs. from tools import web_tools monkeypatch.setattr(web_tools, "_load_web_config", lambda: {}) for key in ("FIRECRAWL_API_KEY", "FIRECRAWL_API_URL", "PARALLEL_API_KEY", @@ -198,7 +202,7 @@ class TestDDGSBackendWiring: monkeypatch.delenv(key, raising=False) monkeypatch.setattr(web_tools, "_is_tool_gateway_ready", lambda: False) monkeypatch.setattr(web_tools, "_ddgs_package_importable", lambda: True) - assert web_tools._get_backend() == "ddgs" + assert web_tools._get_backend() == "parallel" def test_check_web_api_key_true_when_ddgs_configured(self, monkeypatch): from tools import web_tools diff --git a/tests/tools/test_web_providers_searxng.py b/tests/tools/test_web_providers_searxng.py index e093532bf377..2877d56b868c 100644 --- a/tests/tools/test_web_providers_searxng.py +++ b/tests/tools/test_web_providers_searxng.py @@ -313,7 +313,9 @@ class TestCheckWebApiKey: ) assert web_tools.check_web_api_key() is True - def test_no_credentials_fails(self, monkeypatch): + def test_no_credentials_usable_via_free_parallel(self, monkeypatch): + """No credentials → check_web_api_key True: the keyless Parallel free MCP + services calls, so web is usable out of the box.""" from tools import web_tools monkeypatch.setattr(web_tools, "_load_web_config", lambda: {}) monkeypatch.delenv("FIRECRAWL_API_KEY", raising=False) @@ -324,7 +326,8 @@ class TestCheckWebApiKey: monkeypatch.delenv("SEARXNG_URL", raising=False) monkeypatch.setattr(web_tools, "_is_tool_gateway_ready", lambda: False) monkeypatch.setattr(web_tools, "check_firecrawl_api_key", lambda: False) - assert web_tools.check_web_api_key() is False + monkeypatch.setattr(web_tools, "_ddgs_package_importable", lambda: False) + assert web_tools.check_web_api_key() is True # --------------------------------------------------------------------------- diff --git a/tests/tools/test_web_tools_config.py b/tests/tools/test_web_tools_config.py index 28323122aca0..c9a6b3b31a33 100644 --- a/tests/tools/test_web_tools_config.py +++ b/tests/tools/test_web_tools_config.py @@ -384,11 +384,14 @@ class TestBackendSelection: patch.dict(os.environ, {"FIRECRAWL_API_KEY": "fc-test"}): assert _get_backend() == "firecrawl" - def test_fallback_no_keys_defaults_to_firecrawl(self): - """No keys, no config → 'firecrawl' (will fail at client init).""" + def test_fallback_no_keys_defaults_to_parallel(self): + """No credentials, no config → 'parallel' (free Search MCP works + keyless). Selection is purely credential-based.""" from tools.web_tools import _get_backend - with patch("tools.web_tools._load_web_config", return_value={}): - assert _get_backend() == "firecrawl" + with patch("tools.web_tools._load_web_config", return_value={}), \ + patch("tools.web_tools._is_tool_gateway_ready", return_value=False), \ + patch("tools.web_tools._ddgs_package_importable", return_value=False): + assert _get_backend() == "parallel" def test_invalid_config_falls_through_to_fallback(self): """web.backend=invalid → ignored, uses key-based fallback.""" @@ -623,9 +626,74 @@ class TestCheckWebApiKey: from tools.web_tools import check_web_api_key assert check_web_api_key() is True - def test_no_keys_returns_false(self): + def test_no_keys_usable_via_free_parallel(self): + """No credentials → check_web_api_key True: selection resolves to the + keyless Parallel free MCP, which genuinely services calls (web works out + of the box). check_web_api_key is a usability probe, not a key check.""" from tools.web_tools import check_web_api_key - assert check_web_api_key() is False + with patch("tools.web_tools._load_web_config", return_value={}), \ + patch("tools.web_tools._is_tool_gateway_ready", return_value=False), \ + patch("tools.web_tools._ddgs_package_importable", return_value=False), \ + patch.dict(os.environ, {}, clear=False): + for k in ("PARALLEL_API_KEY", "FIRECRAWL_API_KEY", "FIRECRAWL_API_URL", + "TAVILY_API_KEY", "EXA_API_KEY", "SEARXNG_URL", "BRAVE_SEARCH_API_KEY"): + os.environ.pop(k, None) + assert check_web_api_key() is True + + def test_typo_extract_backend_not_masked_by_parallel(self): + """A typo'd per-capability backend is honored (so dispatch errors) + rather than silently falling through to keyless Parallel.""" + from tools.web_tools import _get_extract_backend, check_web_api_key + with patch("tools.web_tools._load_web_config", + return_value={"extract_backend": "parrallel"}): + assert _get_extract_backend() == "parrallel" # not "parallel" + assert check_web_api_key() is False # unknown → unusable + + def test_keyless_parallel_unusable_when_provider_disabled(self): + """If the bundled web-parallel provider is disabled/unregistered, the + keyless free-MCP path must NOT report web as usable — otherwise setup is + skipped but web tools fail at runtime with no provider.""" + from tools.web_tools import check_web_api_key + with patch("tools.web_tools._load_web_config", return_value={}), \ + patch("tools.web_tools._parallel_provider_registered", return_value=False), \ + patch("tools.web_tools._is_tool_gateway_ready", return_value=False), \ + patch("tools.web_tools.check_firecrawl_api_key", return_value=False), \ + patch("tools.web_tools._ddgs_package_importable", return_value=False), \ + patch.dict(os.environ, {}, clear=False): + for var in ( + "PARALLEL_API_KEY", "FIRECRAWL_API_KEY", "FIRECRAWL_API_URL", + "TAVILY_API_KEY", "EXA_API_KEY", "BRAVE_SEARCH_API_KEY", "SEARXNG_URL", + ): + os.environ.pop(var, None) + assert check_web_api_key() is False + + def test_extract_autodetect_skips_search_only_for_keyless_parallel(self): + """A search-only env credential (SEARXNG_URL) must not shadow the keyless + Parallel free-MCP extract fallback: extract auto-detect skips search-only + backends, so _get_extract_backend resolves to parallel (which can fetch), + while search auto-detect still prefers the configured searxng.""" + from tools.web_tools import _get_extract_backend, _get_search_backend + with patch("tools.web_tools._load_web_config", return_value={}), \ + patch.dict(os.environ, {}, clear=False): + for var in ( + "PARALLEL_API_KEY", "FIRECRAWL_API_KEY", "FIRECRAWL_API_URL", + "TAVILY_API_KEY", "EXA_API_KEY", "BRAVE_SEARCH_API_KEY", + ): + os.environ.pop(var, None) + os.environ["SEARXNG_URL"] = "http://localhost:8080" + with patch("tools.web_tools._is_tool_gateway_ready", return_value=False): + assert _get_search_backend() == "searxng" + assert _get_extract_backend() == "parallel" + + def test_configured_but_unavailable_backend_reports_unusable(self): + """An explicitly configured backend with no creds (exa, no key) → + check_web_api_key False so diagnostics flag the misconfiguration — + even though the tools stay registered.""" + from tools.web_tools import check_web_api_key + with patch("tools.web_tools._load_web_config", return_value={"backend": "exa"}), \ + patch.dict(os.environ, {}, clear=False): + os.environ.pop("EXA_API_KEY", None) + assert check_web_api_key() is False def test_both_keys_returns_true(self): with patch.dict(os.environ, { @@ -688,12 +756,18 @@ class TestCheckWebApiKey: assert refresh_calls == [] - def test_configured_backend_must_match_available_provider(self): - with patch("tools.web_tools._load_web_config", return_value={"backend": "parallel"}): - with patch("tools.web_tools._read_nous_access_token", return_value="nous-token"): - with patch.dict(os.environ, {"FIRECRAWL_GATEWAY_URL": "http://127.0.0.1:3002"}, clear=False): - from tools.web_tools import check_web_api_key - assert check_web_api_key() is False + def test_web_tools_registered_even_when_configured_backend_unavailable(self): + # Registration is unconditional (web_tools_registered) so an explicitly + # configured but unavailable backend (exa without EXA_API_KEY) keeps the + # tools registered to surface exa's setup error at call time — while the + # readiness probe (check_web_api_key) honestly reports not-configured. + from tools.web_tools import web_tools_registered, check_web_api_key + assert web_tools_registered() is True + with patch("tools.web_tools._load_web_config", return_value={"backend": "exa"}), \ + patch.dict(os.environ, {}, clear=False): + os.environ.pop("EXA_API_KEY", None) + assert web_tools_registered() is True + assert check_web_api_key() is False def test_configured_firecrawl_backend_accepts_managed_gateway(self): with patch("tools.web_tools._load_web_config", return_value={"backend": "firecrawl"}): diff --git a/tools/lazy_deps.py b/tools/lazy_deps.py index e4b0a9a57f0e..76f146c7869c 100644 --- a/tools/lazy_deps.py +++ b/tools/lazy_deps.py @@ -90,7 +90,7 @@ LAZY_DEPS: dict[str, tuple[str, ...]] = { # ─── Web search backends ─────────────────────────────────────────────── "search.exa": ("exa-py==2.10.2",), "search.firecrawl": ("firecrawl-py==4.17.0",), - "search.parallel": ("parallel-web==0.4.2",), + "search.parallel": ("parallel-web==0.6.0",), # ─── TTS providers ───────────────────────────────────────────────────── # Pinned to exact versions to match pyproject.toml's no-ranges policy diff --git a/tools/web_tools.py b/tools/web_tools.py index 133489b0a892..6bf522f33ecf 100644 --- a/tools/web_tools.py +++ b/tools/web_tools.py @@ -141,15 +141,35 @@ def _load_web_config() -> dict: except (ImportError, Exception): return {} -def _get_backend() -> str: +# Recognized web backend names (config values accepted in ``web.backend`` / +# ``web.search_backend`` / ``web.extract_backend``). Kept as a single source of +# truth for config validation across the selection helpers. +_KNOWN_WEB_BACKENDS = frozenset( + {"parallel", "firecrawl", "tavily", "exa", "searxng", "brave-free", "ddgs", "xai"} +) + +# Backends that only service web_search (their provider's ``supports_extract()`` +# is False). They are skipped during *extract* auto-detect so a search-only +# credential (e.g. SEARXNG_URL) does not shadow the keyless Parallel free-MCP +# fallback, which would otherwise leave web_extract broken on a no-key install. +_SEARCH_ONLY_BACKENDS = frozenset({"searxng", "brave-free", "ddgs", "xai"}) + + +def _get_backend(capability: str = "search") -> str: """Determine which web backend to use (shared fallback). Reads ``web.backend`` from config.yaml (set by ``hermes tools``). Falls back to whichever API key is present for users who configured keys manually without running setup. + + ``capability`` ("search" | "extract") only affects auto-detect: for + ``extract`` we skip search-only backends (``_SEARCH_ONLY_BACKENDS``) so a + search-only credential never shadows the keyless Parallel free-MCP extract + fallback. An explicit ``web.backend`` value is honored as-is (explicit wins, + surfacing that backend's own search-only error rather than rerouting). """ configured = (_load_web_config().get("backend") or "").lower().strip() - if configured in {"parallel", "firecrawl", "tavily", "exa", "searxng", "brave-free", "ddgs", "xai"}: + if configured in _KNOWN_WEB_BACKENDS: return configured # Fallback for manual / legacy config — pick the highest-priority @@ -158,7 +178,8 @@ def _get_backend() -> str: # pre-empted by a Nous OAuth token whose subscription tier may not # actually grant web-search access (the gateway then fails at runtime # with "no subscription" and the tool returns an error to the agent - # without falling back). Free-tier backends trail the paid ones. + # without falling back). Free-tier backends (searxng / brave-free / + # keyless parallel / ddgs) trail the keyed ones. backend_candidates = ( ("tavily", _has_env("TAVILY_API_KEY")), ("exa", _has_env("EXA_API_KEY")), @@ -167,13 +188,24 @@ def _get_backend() -> str: ("firecrawl", _is_tool_gateway_ready()), ("searxng", _has_env("SEARXNG_URL")), ("brave-free", _has_env("BRAVE_SEARCH_API_KEY")), + # Keyless Parallel free MCP — always available, the intended no-key + # default for both search and extract. Ahead of ddgs (search-only, so it + # can't service web_extract); ddgs stays reachable via web.backend=ddgs. + ("parallel", True), ("ddgs", _ddgs_package_importable()), ) for backend, available in backend_candidates: - if available: - return backend + if not available: + continue + # For extract, skip search-only backends so the keyless Parallel + # free-MCP fallback (which can fetch URLs) is reached instead. + if capability == "extract" and backend in _SEARCH_ONLY_BACKENDS: + continue + return backend - return "firecrawl" # default (backward compat) + # Defensive terminal (the keyless ``parallel`` candidate above is always + # available, so this is effectively unreachable). + return "parallel" def _get_search_backend() -> str: @@ -204,14 +236,19 @@ def _get_extract_backend() -> str: def _get_capability_backend(capability: str) -> str: """Shared helper for per-capability backend selection. - Reads ``web.{capability}_backend`` from config; if set and available, - uses it. Otherwise falls through to the shared ``_get_backend()``. + Reads ``web.{capability}_backend`` from config. Any explicit value is + honored **regardless of availability** — including unrecognized typos like + ``parrallel`` — so the dispatcher surfaces that backend's own setup/config + error rather than silently rerouting to the keyless Parallel default (which + would send user queries to a different provider and hide the + misconfiguration). This matches ``web_search_registry``'s "explicit config + wins" rule. Only an *unset* value falls through to ``_get_backend()``. """ cfg = _load_web_config() specific = (cfg.get(f"{capability}_backend") or "").lower().strip() - if specific and _is_backend_available(specific): + if specific: return specific - return _get_backend() + return _get_backend(capability) def _is_backend_available(backend: str) -> bool: @@ -219,6 +256,8 @@ def _is_backend_available(backend: str) -> bool: if backend == "exa": return _has_env("EXA_API_KEY") if backend == "parallel": + # Credential probe: True only with a real key. The keyless free-MCP + # fallback is handled by _get_backend()'s terminal default, not here. return _has_env("PARALLEL_API_KEY") if backend == "firecrawl": return check_firecrawl_api_key() @@ -972,11 +1011,19 @@ async def web_extract_tool( else: safe_urls.append(url) + # Tracks the free-tier Parallel extract path (no key → web_fetch via the + # hosted Search MCP) so we can credit Parallel in the output/UI. Bound + # here so empty/all-blocked inputs (which skip dispatch) stay defined. + _free_parallel_extract = False + # Dispatch only safe URLs to the configured backend if not safe_urls: results = [] else: backend = _get_extract_backend() + _free_parallel_extract = ( + backend == "parallel" and not _has_env("PARALLEL_API_KEY") + ) # All seven providers (brave-free, ddgs, searxng, exa, parallel, # tavily, firecrawl) now live as plugins. The dispatcher is a @@ -1150,6 +1197,14 @@ async def web_extract_tool( for r in response.get("results", []) ] trimmed_response = {"results": trimmed_results} + if _free_parallel_extract: + # Credit Parallel's free Search MCP (drives the "[Parallel]" UI tag + # + lets the model cite the source). Free tier only. + trimmed_response["provider"] = "parallel" + trimmed_response["attribution"] = ( + "Extraction powered by the free Parallel Web Search MCP " + "(https://parallel.ai)." + ) if trimmed_response.get("results") == []: result_json = tool_error("Content was inaccessible or not found") @@ -1181,16 +1236,61 @@ async def web_extract_tool( return tool_error(error_msg) -# Convenience function to check Firecrawl credentials +def web_tools_registered() -> bool: + """Whether the web tools should be registered. Always True. + + Registration is decoupled from credential readiness: with no credentials, + search/extract fall back to Parallel's free hosted Search MCP, and an + explicitly configured-but-unavailable backend must stay registered so + dispatch surfaces that backend's own setup error rather than the tool + silently vanishing. For "is web actually configured?" use + :func:`check_web_api_key`. + """ + return True + + +def _parallel_provider_registered() -> bool: + """True when the bundled ``web-parallel`` provider is registered/enabled. + + Plugin discovery skips disabled plugins, so a disabled (``plugins.disabled``) + or otherwise-unregistered parallel provider yields ``None`` here. + """ + _ensure_web_plugins_loaded() + try: + from agent.web_search_registry import get_provider + + return get_provider("parallel") is not None + except Exception: # noqa: BLE001 + return False + + +def _backend_usable(backend: str) -> bool: + """True when *backend* can service calls. Keyless Parallel counts (free MCP). + + Unknown/typo'd backend names are not usable (so an explicit typo is reported + as a config problem rather than masked by the keyless fallback). + """ + if backend == "parallel" and not _has_env("PARALLEL_API_KEY"): + # Keyless Parallel is only genuinely usable when its provider is actually + # registered/enabled. If web-parallel is disabled or discovery failed, + # report unusable so setup is not skipped and the user is not left with + # web tools that fail at runtime ("No web search provider configured"). + return _parallel_provider_registered() + return _is_backend_available(backend) + + def check_web_api_key() -> bool: - """Check whether the configured web backend is available.""" - configured = _load_web_config().get("backend", "").lower().strip() - if configured in {"exa", "parallel", "firecrawl", "tavily", "searxng", "brave-free", "ddgs", "xai"}: - return _is_backend_available(configured) - return any( - _is_backend_available(backend) - for backend in ("exa", "parallel", "firecrawl", "tavily", "searxng", "brave-free", "ddgs", "xai") - ) + """Usability probe: True when the selected web backends can service calls. + + Probes the backends that :func:`_get_search_backend` / + :func:`_get_extract_backend` actually select (not just shared + ``web.backend``), so an explicit per-capability backend with missing + credentials — or a typo'd name — reports unusable instead of being masked by + the keyless Parallel fallback. Keyless Parallel itself genuinely services + calls, so a zero-setup install reports usable. Distinct from + :func:`web_tools_registered` (always True — whether the tool is offered). + """ + return _backend_usable(_get_search_backend()) and _backend_usable(_get_extract_backend()) def check_auxiliary_model() -> bool: @@ -1358,7 +1458,7 @@ registry.register( toolset="web", schema=WEB_SEARCH_SCHEMA, handler=lambda args, **kw: web_search_tool(args.get("query", ""), limit=args.get("limit", 5)), - check_fn=check_web_api_key, + check_fn=web_tools_registered, requires_env=_web_requires_env(), emoji="🔍", max_result_size_chars=100_000, @@ -1369,7 +1469,7 @@ registry.register( schema=WEB_EXTRACT_SCHEMA, handler=lambda args, **kw: web_extract_tool( args.get("urls", [])[:5] if isinstance(args.get("urls"), list) else [], "markdown"), - check_fn=check_web_api_key, + check_fn=web_tools_registered, requires_env=_web_requires_env(), is_async=True, emoji="📄", diff --git a/uv.lock b/uv.lock index 55d7da4a4a84..f90a3a4270cb 100644 --- a/uv.lock +++ b/uv.lock @@ -1653,7 +1653,7 @@ requires-dist = [ { name = "numpy", marker = "extra == 'voice'", specifier = "==2.4.3" }, { name = "openai", specifier = "==2.24.0" }, { name = "packaging", specifier = "==26.0" }, - { name = "parallel-web", marker = "extra == 'parallel-web'", specifier = "==0.4.2" }, + { name = "parallel-web", marker = "extra == 'parallel-web'", specifier = "==0.6.0" }, { name = "pathspec", specifier = "==1.1.1" }, { name = "pillow", specifier = "==12.2.0" }, { name = "prompt-toolkit", specifier = "==3.0.52" }, @@ -2690,7 +2690,7 @@ wheels = [ [[package]] name = "parallel-web" -version = "0.4.2" +version = "0.6.0" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "anyio" }, @@ -2700,9 +2700,9 @@ dependencies = [ { name = "sniffio" }, { name = "typing-extensions" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/24/50/fb9b28a679e01682006b5259abff96de3d16e114e9447a7793fec31715de/parallel_web-0.4.2.tar.gz", hash = "sha256:599b5a8f387dc35c7dc8c81e372eadf6958a40acacea58bf170dfc663c003da7", size = 140026, upload-time = "2026-03-09T22:24:35.448Z" } +sdist = { url = "https://files.pythonhosted.org/packages/7f/81/101c961fe6665212df01fb39a70ebb379dc33529c7bc9210675c0f525139/parallel_web-0.6.0.tar.gz", hash = "sha256:f8aecd3f1958090090c4516881cefea4f55c40948ba3bb99217ca9a6d4263225", size = 173149, upload-time = "2026-05-06T19:13:09.782Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/a0/3e/2218fa29637781b8e7ac35a928108ff2614ddd40879389d3af2caa725af5/parallel_web-0.4.2-py3-none-any.whl", hash = "sha256:aa3a4a9aecc08972c5ce9303271d4917903373dff4dd277d9a3e30f9cff53346", size = 144012, upload-time = "2026-03-09T22:24:33.979Z" }, + { url = "https://files.pythonhosted.org/packages/a2/7c/7e8b63a0e90efaf567a818fca86c6ad3a85711f8995d2657b51b0cae2351/parallel_web-0.6.0-py3-none-any.whl", hash = "sha256:dc5342ef7262bd2e9f85eb7eace32833bd3d7e3af0bf5fbd780d1ea8c8d9ceb0", size = 199217, upload-time = "2026-05-06T19:13:08.316Z" }, ] [[package]] From 0a5762c78d11f4d6626dbf99da5f62cc34cfe2c4 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 10 Jun 2026 15:13:42 -0700 Subject: [PATCH 06/28] fix(web): genericize free-MCP client identity per telemetry policy Replace the hermes-identifying clientInfo/User-Agent/session-id prefix on the keyless Parallel Search MCP path with a neutral 'mcp-web-client' identity. Project policy forbids third-party usage attribution without an explicit user opt-in (see telemetry PR policy); MCP requires a clientInfo, so a generic one satisfies the spec without attributing traffic. Also adds the contributor AUTHOR_MAP entry and refreshes uv.lock against current main (parallel-web 0.6.0). --- plugins/web/parallel/provider.py | 15 +++++++----- scripts/release.py | 1 + .../plugins/web/test_parallel_keyless_mcp.py | 23 ++++++++++--------- 3 files changed, 22 insertions(+), 17 deletions(-) diff --git a/plugins/web/parallel/provider.py b/plugins/web/parallel/provider.py index 20c4291d77b9..7a15b3d3f80a 100644 --- a/plugins/web/parallel/provider.py +++ b/plugins/web/parallel/provider.py @@ -55,11 +55,13 @@ logger = logging.getLogger(__name__) # configured. Docs: https://docs.parallel.ai/integrations/mcp/search-mcp _MCP_SEARCH_URL = "https://search.parallel.ai/mcp" _MCP_PROTOCOL_VERSION = "2025-06-18" -_MCP_CLIENT_NAME = "hermes-agent" +# Deliberately generic client identity. Project policy (see the telemetry PR +# policy in AGENTS.md) forbids third-party usage attribution without an +# explicit user opt-in, so neither clientInfo nor the User-Agent names +# hermes. MCP requires *a* clientInfo; a neutral one satisfies the spec +# without attributing traffic. +_MCP_CLIENT_NAME = "mcp-web-client" _MCP_CLIENT_VERSION = "1.0.0" -# Identify free-tier traffic at the HTTP layer. Without this, httpx sends a -# generic ``python-httpx/`` User-Agent and hermes usage is only visible -# via the JSON-RPC ``clientInfo`` payload. _MCP_USER_AGENT = f"{_MCP_CLIENT_NAME}/{_MCP_CLIENT_VERSION}" _MCP_TIMEOUT_SECONDS = 30.0 @@ -76,9 +78,10 @@ def _new_session_id() -> str: Per-call rather than process-global: one process serves many unrelated chats in the gateway/batch runners, and a shared id would pool their - searches into one Parallel session. + searches into one Parallel session. The prefix is deliberately generic + (no hermes attribution — telemetry policy). """ - return f"hermes-agent-{uuid.uuid4().hex}" + return f"{_MCP_CLIENT_NAME}-{uuid.uuid4().hex}" # Module-level note: the canonical cache slots ``_parallel_client`` and # ``_async_parallel_client`` live on :mod:`tools.web_tools` so tests that do diff --git a/scripts/release.py b/scripts/release.py index cd0fe475d5dc..68ad134d6cdb 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -75,6 +75,7 @@ AUTHOR_MAP = { "129007007+HeLLGURD@users.noreply.github.com": "HeLLGURD", "290859878+synapsesx@users.noreply.github.com": "synapsesx", "dirtyren@users.noreply.github.com": "dirtyren", + "mharris@parallel.ai": "NormallyGaussian", "ted.malone@outlook.com": "temalo", "adityamalik2833@gmail.com": "alarcritty", "islam666@users.noreply.github.com": "islam666", diff --git a/tests/plugins/web/test_parallel_keyless_mcp.py b/tests/plugins/web/test_parallel_keyless_mcp.py index 49975c47f2f0..8495df144b46 100644 --- a/tests/plugins/web/test_parallel_keyless_mcp.py +++ b/tests/plugins/web/test_parallel_keyless_mcp.py @@ -30,15 +30,16 @@ class TestMcpHeaders: assert h["Accept"] == "application/json, text/event-stream" assert "Mcp-Session-Id" not in h - def test_identifies_hermes_via_user_agent(self): - # Free-tier traffic is attributable at the HTTP layer (not just via the - # JSON-RPC clientInfo payload), on both the anonymous and keyed paths. - assert pp._mcp_headers(session_id=None, api_key=None)["User-Agent"].startswith( - "hermes-agent/" - ) - assert pp._mcp_headers(session_id="sid", api_key="pk-live")["User-Agent"].startswith( - "hermes-agent/" - ) + def test_user_agent_is_generic_not_hermes(self): + # Telemetry policy: no third-party usage attribution without opt-in. + # The UA must be set (not python-httpx default) but must not name + # hermes, on both the anonymous and keyed paths. + for ua in ( + pp._mcp_headers(session_id=None, api_key=None)["User-Agent"], + pp._mcp_headers(session_id="sid", api_key="pk-live")["User-Agent"], + ): + assert ua == f"{pp._MCP_CLIENT_NAME}/{pp._MCP_CLIENT_VERSION}" + assert "hermes" not in ua.lower() def test_session_id_and_bearer_when_present(self): h = pp._mcp_headers(session_id="sid-123", api_key="pk-live") @@ -280,7 +281,7 @@ class TestMcpWebFetch: assert args["name"] == "web_fetch" assert args["arguments"]["urls"] == urls assert args["arguments"]["full_content"] is True - assert args["arguments"]["session_id"].startswith("hermes-agent-") + assert args["arguments"]["session_id"].startswith(f"{pp._MCP_CLIENT_NAME}-") def test_prefers_full_content_over_excerpts(self): payload = {"results": [ @@ -354,7 +355,7 @@ class TestKeyedV1Search: # honors the caller's limit via advanced_settings.max_results assert captured["advanced_settings"] == {"max_results": 7} assert captured["mode"] == "advanced" # v1 default - assert captured["session_id"].startswith("hermes-agent-") # per-call id + assert captured["session_id"].startswith(f"{pp._MCP_CLIENT_NAME}-") # per-call id assert len(out["data"]["web"]) == 7 # client-side slice # paid path: no free-tier attribution, no [Parallel] label signal assert "attribution" not in out From acd7932c0fc5f98331f01af6073115e6cf3ccf9d Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 10 Jun 2026 19:54:44 -0700 Subject: [PATCH 07/28] docs: cross-link write-approval gate from skills, configuration, and slash-command docs (#43801) The memory/skill write-approval gate (#38199, #43354, #43452) was only documented inside features/memory.md. Surface it everywhere users will actually look: - features/skills.md: new 'Gating agent skill writes' section under skill_manage, with the staging semantics, review commands, and the distinction from skills.guard_agent_created - configuration.md: memory.write_approval added to the Memory Configuration block; new 'Write approval for skill writes' subsection next to the guard_agent_created scanner - reference/slash-commands.md: /memory and /skills review subcommands in both the CLI and messaging tables; Notes updated since /skills pending/approve/reject/diff/approval now works on the gateway - features/memory.md: cross-link to the new skills section --- website/docs/reference/slash-commands.md | 8 +++-- website/docs/user-guide/configuration.md | 14 ++++++++ website/docs/user-guide/features/memory.md | 1 + website/docs/user-guide/features/skills.md | 37 ++++++++++++++++++++++ 4 files changed, 58 insertions(+), 2 deletions(-) diff --git a/website/docs/reference/slash-commands.md b/website/docs/reference/slash-commands.md index 44a9a303a624..a9951263d7ff 100644 --- a/website/docs/reference/slash-commands.md +++ b/website/docs/reference/slash-commands.md @@ -86,7 +86,8 @@ Type `/` in the CLI to open the autocomplete menu. Built-in commands are case-in | `/tools [list\|disable\|enable] [name...]` | Manage tools: list available tools, or disable/enable specific tools for the current session. Disabling a tool removes it from the agent's toolset and triggers a session reset. | | `/toolsets` | List available toolsets | | `/browser [connect\|disconnect\|status]` | Manage a local Chromium-family CDP connection. `connect` attaches browser tools to a running Chrome, Brave, Chromium, or Edge instance (default: `http://127.0.0.1:9222`). `disconnect` detaches. `status` shows current connection. Auto-launches a supported Chromium-family browser if no debugger is detected. | -| `/skills` | Search, install, inspect, or manage skills from online registries | +| `/skills` | Search, install, inspect, or manage skills from online registries. Also the review surface for the skill write-approval gate: `/skills pending`, `/skills diff `, `/skills approve `, `/skills reject `, `/skills approval on\|off`. See [Gating agent skill writes](/user-guide/features/skills#gating-agent-skill-writes-skillswrite_approval). | +| `/memory [pending\|approve\|reject\|approval]` | Review pending memory writes staged by the write-approval gate (`memory.write_approval`) and toggle the gate. See [Controlling memory writes](/user-guide/features/memory#controlling-memory-writes-write_approval). | | `/bundles` | List configured skill bundles — `/` slash aliases that preload several skills at once. Configure under `bundles:` in `~/.hermes/config.yaml`. See [Skill Bundles](/user-guide/features/skills#skill-bundles). | | `/cron` | Manage scheduled tasks (list, add/create, edit, pause, resume, run, remove) | | `/curator` | Background skill maintenance — `status`, `run`, `pin`, `archive`. See [Curator](/user-guide/features/curator). | @@ -222,6 +223,8 @@ The messaging gateway supports the following built-in commands inside Telegram, | `/goal ` | Set a standing goal Hermes works toward across turns — our take on the Ralph loop. A judge model checks after each turn; if not done, Hermes auto-continues until it is, you pause/clear it, or the turn budget (default 20) is hit. Subcommands: `/goal status`, `/goal pause`, `/goal resume`, `/goal clear`. Safe to run mid-agent for status/pause/clear; setting a new goal requires `/stop` first. See [Persistent Goals](/user-guide/features/goals). | | `/footer [on\|off\|status]` | Toggle the runtime-metadata footer on final replies (shows model, context %, and cwd). | | `/curator [status\|run\|pin\|archive]` | Background skill maintenance controls. | +| `/memory [pending\|approve\|reject\|approval]` | Review pending memory writes staged by the write-approval gate (`memory.write_approval`) — approve or reject them right in chat — and toggle the gate with `/memory approval on\|off`. See [Controlling memory writes](/user-guide/features/memory#controlling-memory-writes-write_approval). | +| `/skills [pending\|approve\|reject\|diff\|approval]` | Review pending **skill** writes staged by the write-approval gate (`skills.write_approval`). Shows a one-line gist per staged write; `/skills diff ` is truncated for chat — read the full diff on the CLI or in `~/.hermes/pending/skills/.json`. Only appears when the gate is on (or staged writes remain); search/install stay CLI-only. | | `/kanban ` | Drive the multi-profile, multi-project collaboration board from chat — identical argument surface to the CLI. Bypasses the running-agent guard, so `/kanban unblock t_abc`, `/kanban comment t_abc "…"`, `/kanban list --mine`, `/kanban boards switch `, etc. work mid-turn. `/kanban create …` auto-subscribes the originating chat to the new task's terminal events. See [Kanban slash command](/user-guide/features/kanban#kanban-slash-command). | | `/reload-mcp` (alias: `/reload_mcp`) | Reload MCP servers from config. | | `/yolo` | Toggle YOLO mode — skip all dangerous command approval prompts. | @@ -236,7 +239,8 @@ The messaging gateway supports the following built-in commands inside Telegram, ## Notes -- `/skin`, `/snapshot`, `/gquota`, `/reload`, `/tools`, `/toolsets`, `/browser`, `/config`, `/cron`, `/skills`, `/platforms`, `/paste`, `/image`, `/statusbar`, `/plugins`, `/busy`, `/indicator`, `/redraw`, `/clear`, `/history`, `/save`, `/copy`, `/handoff`, and `/quit` are **CLI-only** commands. +- `/skin`, `/snapshot`, `/gquota`, `/reload`, `/tools`, `/toolsets`, `/browser`, `/config`, `/cron`, `/platforms`, `/paste`, `/image`, `/statusbar`, `/plugins`, `/busy`, `/indicator`, `/redraw`, `/clear`, `/history`, `/save`, `/copy`, `/handoff`, and `/quit` are **CLI-only** commands. +- `/skills` is **CLI-only for search/browse/install**; its write-approval review subcommands (`pending`, `approve`, `reject`, `diff`, `approval`) also work on messaging platforms when `skills.write_approval` is on. `/memory` works on **both** surfaces. - `/verbose` is **CLI-only by default**, but can be enabled for messaging platforms by setting `display.tool_progress_command: true` in `config.yaml`. When enabled, it cycles the `display.tool_progress` mode and saves to config. - `/sethome`, `/update`, `/restart`, `/approve`, `/deny`, `/topic`, and `/commands` are **messaging-only** commands. - `/status`, `/version`, `/background`, `/queue`, `/steer`, `/voice`, `/reload-mcp`, `/reload-skills`, `/rollback`, `/debug`, `/fast`, `/footer`, `/curator`, `/kanban`, `/sessions`, and `/yolo` work in **both** the CLI and the messaging gateway. diff --git a/website/docs/user-guide/configuration.md b/website/docs/user-guide/configuration.md index 4b2d2c40e93f..062eb53d3447 100644 --- a/website/docs/user-guide/configuration.md +++ b/website/docs/user-guide/configuration.md @@ -533,6 +533,17 @@ skills: When on, any flagged `skill_manage` write surfaces as an approval prompt with the scanner's rationale. Accepted writes land; denied writes return an explanatory error to the agent. +### Write approval for skill writes + +Independent of the content scanner above, `skills.write_approval` gates **every** agent skill write (create / edit / patch / delete / supporting files) behind your explicit approval — the same approve/deny mechanism as dangerous commands: + +```yaml +skills: + write_approval: false # false = write freely (default) | true = stage every write for review +``` + +When on, skill writes are staged under `~/.hermes/pending/skills/` and reviewed with `/skills pending`, `/skills diff `, `/skills approve `, `/skills reject ` — from the CLI or any messaging platform. Toggle at runtime with `/skills approval on|off`. Memory has the same gate (`memory.write_approval`, below). Full walkthrough: [Gating agent skill writes](/user-guide/features/skills#gating-agent-skill-writes-skillswrite_approval). + ## Memory Configuration ```yaml @@ -541,8 +552,11 @@ memory: user_profile_enabled: true memory_char_limit: 2200 # ~800 tokens user_char_limit: 1375 # ~500 tokens + write_approval: false # true = require approval before any memory write ``` +With `memory.write_approval: true`, memory writes need your approval before they land: interactive CLI turns prompt inline; messaging sessions and the background self-improvement review stage the write for `/memory pending` → `/memory approve ` / `/memory reject ` review. Toggle at runtime with `/memory approval on|off`. See [Controlling memory writes](/user-guide/features/memory#controlling-memory-writes-write_approval). + ## File Read Safety Controls how much content a single `read_file` call can return. Reads that exceed the limit are rejected with an error telling the agent to use `offset` and `limit` for a smaller range. This prevents a single read of a minified JS bundle or large data file from flooding the context window. diff --git a/website/docs/user-guide/features/memory.md b/website/docs/user-guide/features/memory.md index b4814d2a178f..1f0ee16942f4 100644 --- a/website/docs/user-guide/features/memory.md +++ b/website/docs/user-guide/features/memory.md @@ -270,6 +270,7 @@ inline, but the full diff stays out-of-band: On a messaging platform, approve a skill from its gist + metadata, or open `/skills diff` on the CLI / dashboard / the staged file under `~/.hermes/pending/skills/.json` when you want to read the whole change. +Full details in [Gating agent skill writes](/user-guide/features/skills#gating-agent-skill-writes-skillswrite_approval). ## External Memory Providers diff --git a/website/docs/user-guide/features/skills.md b/website/docs/user-guide/features/skills.md index 8a5209c6944e..6cfbafee3c3a 100644 --- a/website/docs/user-guide/features/skills.md +++ b/website/docs/user-guide/features/skills.md @@ -401,6 +401,43 @@ The agent can create, update, and delete its own skills via the `skill_manage` t The `patch` action is preferred for updates — it's more token-efficient than `edit` because only the changed text appears in the tool call. ::: +### Gating agent skill writes (`skills.write_approval`) + +By default the agent writes skills freely — including from the [background +self-improvement review](/user-guide/features/memory#controlling-memory-writes-write_approval) +that runs after a turn. If you'd rather approve every skill write first +(small models that misjudge what they learned, secure environments, or just +wanting eyes on the self-improvement loop), turn on the write-approval gate: + +```yaml +skills: + write_approval: false # false = write freely (default) | true = require approval +``` + +When `write_approval: true`, every `skill_manage` write (create / edit / +patch / delete / write_file / remove_file) is **staged** instead of committed — +a SKILL.md is too large to review inline, so staging applies regardless of +whether the write came from a foreground turn or the background review. +Staged writes survive restarts under `~/.hermes/pending/skills/` and are +reviewed with the same familiar approve/deny flow as dangerous commands: + +``` +/skills pending # list staged skill writes + a one-line gist each +/skills diff # full unified diff (best viewed in CLI or dashboard) +/skills approve # apply it (or 'all') +/skills reject # drop it (or 'all') +/skills approval on # turn the gate on (or 'off') and persist it +``` + +The review surface works in the interactive CLI and on messaging platforms +(diff output is truncated for chat bubbles — read the full diff on the CLI or +in the pending JSON file). Memory writes have the same gate under +`memory.write_approval` — see [Controlling memory writes](/user-guide/features/memory#controlling-memory-writes-write_approval). + +> The separate `skills.guard_agent_created` setting is a content scanner +> (dangerous-pattern heuristics), not an approval gate — the two are +> independent. See [Guard on agent-created skill writes](/user-guide/configuration#guard-on-agent-created-skill-writes). + ## Skills Hub Browse, search, install, and manage skills from online registries, `skills.sh`, direct well-known skill endpoints, and official optional skills. From 18d61bd06e062049f524d6b1d3a51956db2cf1ea Mon Sep 17 00:00:00 2001 From: Roger Date: Wed, 10 Jun 2026 13:00:45 -0400 Subject: [PATCH 08/28] fix(desktop): persist composer drafts across reloads Save in-progress composer text to browser localStorage per chat session and restore it when the desktop composer remounts. Keep the draft when submit is rejected or throws, and clear it only after the prompt is accepted. --- apps/desktop/src/app/chat/composer/index.tsx | 54 ++++++++++++++++- apps/desktop/src/store/composer.test.ts | 42 ++++++++++++- apps/desktop/src/store/composer.ts | 62 ++++++++++++++++++++ 3 files changed, 154 insertions(+), 4 deletions(-) diff --git a/apps/desktop/src/app/chat/composer/index.tsx b/apps/desktop/src/app/chat/composer/index.tsx index bf9488348376..05fa4a451bce 100644 --- a/apps/desktop/src/app/chat/composer/index.tsx +++ b/apps/desktop/src/app/chat/composer/index.tsx @@ -24,7 +24,14 @@ import { desktopSlashCommandTakesArgs } from '@/lib/desktop-slash-commands' import { DATA_IMAGE_URL_RE } from '@/lib/embedded-images' import { triggerHaptic } from '@/lib/haptics' import { cn } from '@/lib/utils' -import { $composerAttachments, clearComposerAttachments, type ComposerAttachment } from '@/store/composer' +import { + $composerAttachments, + clearComposerAttachments, + clearPersistedComposerDraft, + type ComposerAttachment, + readPersistedComposerDraft, + writePersistedComposerDraft +} from '@/store/composer' import { browseBackward, browseForward, @@ -160,6 +167,7 @@ export function ChatBar({ const scrolledUp = useStore($threadScrolledUp) const sessionMessages = useStore($messages) const activeQueueSessionKey = queueSessionKey || sessionId || null + const draftPersistenceScope = activeQueueSessionKey || null const queuedPrompts = useMemo( () => (activeQueueSessionKey ? (queuedPromptsBySession[activeQueueSessionKey] ?? []) : []), @@ -171,6 +179,7 @@ export function ChatBar({ const editorRef = useRef(null) const draftRef = useRef(draft) const previousBusyRef = useRef(busy) + const skipNextDraftPersistScopeRef = useRef(null) const drainingQueueRef = useRef(false) const urlInputRef = useRef(null) @@ -1097,6 +1106,22 @@ export function ChatBar({ } } + useEffect(() => { + const persisted = readPersistedComposerDraft(draftPersistenceScope) + skipNextDraftPersistScopeRef.current = draftPersistenceScope + loadIntoComposer(persisted, []) + }, [draftPersistenceScope]) // eslint-disable-line react-hooks/exhaustive-deps + + useEffect(() => { + if (skipNextDraftPersistScopeRef.current === draftPersistenceScope) { + skipNextDraftPersistScopeRef.current = null + + return + } + + writePersistedComposerDraft(draftPersistenceScope, draft) + }, [draft, draftPersistenceScope]) + const beginQueuedEdit = (entry: QueuedPromptEntry) => { if (!activeQueueSessionKey || queueEdit) { return @@ -1323,8 +1348,10 @@ export function ChatBar({ // input event; refresh it from the editor once more to also cover an // in-flight keystroke that hasn't fired its input event yet. const editor = editorRef.current + if (editor) { const domText = composerPlainText(editor) + if (domText !== draftRef.current) { draftRef.current = domText aui.composer().setText(domText) @@ -1348,7 +1375,17 @@ export function ChatBar({ const submitted = text triggerHaptic('submit') clearDraft() - void onSubmit(submitted) + void Promise.resolve(onSubmit(submitted)).then(accepted => { + if (accepted === false) { + loadIntoComposer(submitted, []) + writePersistedComposerDraft(draftPersistenceScope, submitted) + } else { + clearPersistedComposerDraft(draftPersistenceScope) + } + }).catch(() => { + loadIntoComposer(submitted, []) + writePersistedComposerDraft(draftPersistenceScope, submitted) + }) } else if (payloadPresent) { queueCurrentDraft() } else { @@ -1361,11 +1398,22 @@ export function ChatBar({ void drainNextQueued() } else if (payloadPresent) { const submitted = text + const submittedAttachments = cloneAttachments(attachments) triggerHaptic('submit') resetBrowseState(sessionId) clearDraft() clearComposerAttachments() - void onSubmit(submitted, { attachments }) + void Promise.resolve(onSubmit(submitted, { attachments: submittedAttachments })).then(accepted => { + if (accepted === false) { + loadIntoComposer(submitted, submittedAttachments) + writePersistedComposerDraft(draftPersistenceScope, submitted) + } else { + clearPersistedComposerDraft(draftPersistenceScope) + } + }).catch(() => { + loadIntoComposer(submitted, submittedAttachments) + writePersistedComposerDraft(draftPersistenceScope, submitted) + }) } focusInput() diff --git a/apps/desktop/src/store/composer.test.ts b/apps/desktop/src/store/composer.test.ts index 83f0a3feb96f..7bb44c74bd06 100644 --- a/apps/desktop/src/store/composer.test.ts +++ b/apps/desktop/src/store/composer.test.ts @@ -3,9 +3,13 @@ import { afterEach, describe, expect, it } from 'vitest' import { $composerAttachments, addComposerAttachment, + clearPersistedComposerDraft, type ComposerAttachment, + composerDraftStorageKey, + readPersistedComposerDraft, removeComposerAttachment, - updateComposerAttachment + updateComposerAttachment, + writePersistedComposerDraft } from './composer' function attachment(overrides: Partial & Pick): ComposerAttachment { @@ -41,3 +45,39 @@ describe('updateComposerAttachment', () => { expect($composerAttachments.get()).toHaveLength(0) }) }) + +describe('persisted composer drafts', () => { + afterEach(() => { + window.localStorage.clear() + }) + + it('stores and restores text drafts per session scope', () => { + writePersistedComposerDraft('session-a', 'almost submitted prompt') + writePersistedComposerDraft('session-b', 'other draft') + + expect(readPersistedComposerDraft('session-a')).toBe('almost submitted prompt') + expect(readPersistedComposerDraft('session-b')).toBe('other draft') + }) + + it('uses a stable new-session key when no session id exists yet', () => { + writePersistedComposerDraft(null, 'first prompt draft') + + expect(window.localStorage.getItem(composerDraftStorageKey(null))).toBe('first prompt draft') + expect(readPersistedComposerDraft(undefined)).toBe('first prompt draft') + }) + + it('removes empty drafts instead of leaving stale text behind', () => { + writePersistedComposerDraft('session-a', 'saved') + writePersistedComposerDraft('session-a', '') + + expect(readPersistedComposerDraft('session-a')).toBe('') + expect(window.localStorage.getItem(composerDraftStorageKey('session-a'))).toBeNull() + }) + + it('can explicitly clear a saved draft after submit', () => { + writePersistedComposerDraft('session-a', 'saved') + clearPersistedComposerDraft('session-a') + + expect(readPersistedComposerDraft('session-a')).toBe('') + }) +}) diff --git a/apps/desktop/src/store/composer.ts b/apps/desktop/src/store/composer.ts index 6b2b58ccb8d3..5af98a49e3b4 100644 --- a/apps/desktop/src/store/composer.ts +++ b/apps/desktop/src/store/composer.ts @@ -21,6 +21,68 @@ export const $composerDraft = atom('') export const $composerAttachments = atom([]) export const $composerTerminalSelections = atom>({}) +const COMPOSER_DRAFT_STORAGE_PREFIX = 'hermes:composer-draft:v1:' +const NEW_SESSION_DRAFT_SCOPE = '__new__' + +function storageScope(scope: string | null | undefined): string { + const trimmed = scope?.trim() + + return trimmed || NEW_SESSION_DRAFT_SCOPE +} + +function browserStorage(): Storage | null { + if (typeof window === 'undefined') { + return null + } + + try { + return window.localStorage + } catch { + return null + } +} + +export function composerDraftStorageKey(scope: string | null | undefined): string { + return `${COMPOSER_DRAFT_STORAGE_PREFIX}${encodeURIComponent(storageScope(scope))}` +} + +export function readPersistedComposerDraft(scope: string | null | undefined): string { + try { + return browserStorage()?.getItem(composerDraftStorageKey(scope)) ?? '' + } catch { + return '' + } +} + +export function writePersistedComposerDraft(scope: string | null | undefined, value: string) { + try { + const storage = browserStorage() + + if (!storage) { + return + } + + const key = composerDraftStorageKey(scope) + + if (value.length === 0) { + storage.removeItem(key) + } else { + storage.setItem(key, value) + } + } catch { + // Draft persistence is a safety net only; storage quota/private-mode errors + // must never break typing or submission. + } +} + +export function clearPersistedComposerDraft(scope: string | null | undefined) { + try { + browserStorage()?.removeItem(composerDraftStorageKey(scope)) + } catch { + // Best-effort only. + } +} + export function setComposerDraft(value: string) { $composerDraft.set(value) } From 3d14f01fd674ab2add062d3a302c30a6b457b791 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 10 Jun 2026 19:57:09 -0700 Subject: [PATCH 09/28] fix(desktop): debounce per-keystroke draft persistence writes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The salvaged draft-persistence effect wrote to localStorage on every keystroke — the composer's per-keystroke path was deliberately slimmed down previously, so debounce the write (400ms) and flush pending text on scope change/unmount so a fast session switch can't drop trailing keystrokes. Also add AUTHOR_MAP entry for the salvaged commit. --- apps/desktop/src/app/chat/composer/index.tsx | 33 +++++++++++++++++++- scripts/release.py | 1 + 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/apps/desktop/src/app/chat/composer/index.tsx b/apps/desktop/src/app/chat/composer/index.tsx index 05fa4a451bce..5530ffb60aef 100644 --- a/apps/desktop/src/app/chat/composer/index.tsx +++ b/apps/desktop/src/app/chat/composer/index.tsx @@ -137,6 +137,10 @@ interface QueueEditState { const cloneAttachments = (attachments: ComposerAttachment[]) => attachments.map(a => ({ ...a })) +// How long the composer waits after the last keystroke before persisting the +// draft to localStorage. Scope-change/unmount flushes bypass the delay. +const DRAFT_PERSIST_DEBOUNCE_MS = 400 + export function ChatBar({ busy, cwd, @@ -180,6 +184,7 @@ export function ChatBar({ const draftRef = useRef(draft) const previousBusyRef = useRef(busy) const skipNextDraftPersistScopeRef = useRef(null) + const pendingDraftPersistRef = useRef<{ scope: string | null; value: string } | null>(null) const drainingQueueRef = useRef(false) const urlInputRef = useRef(null) @@ -1119,9 +1124,35 @@ export function ChatBar({ return } - writePersistedComposerDraft(draftPersistenceScope, draft) + // Debounce the localStorage write: the composer's per-keystroke path was + // deliberately slimmed down (see the draftRef sync comment above), so we + // don't touch storage on every keypress. The pending ref below is flushed + // on scope change / unmount so a fast session switch can't drop the + // trailing keystrokes. + pendingDraftPersistRef.current = { scope: draftPersistenceScope, value: draft } + + const handle = window.setTimeout(() => { + pendingDraftPersistRef.current = null + writePersistedComposerDraft(draftPersistenceScope, draft) + }, DRAFT_PERSIST_DEBOUNCE_MS) + + return () => window.clearTimeout(handle) }, [draft, draftPersistenceScope]) + // Flush any pending debounced draft write when leaving a session scope or + // unmounting, so the departing session's latest text is always persisted. + useEffect( + () => () => { + const pending = pendingDraftPersistRef.current + + if (pending) { + pendingDraftPersistRef.current = null + writePersistedComposerDraft(pending.scope, pending.value) + } + }, + [draftPersistenceScope] + ) + const beginQueuedEdit = (entry: QueuedPromptEntry) => { if (!activeQueueSessionKey || queueEdit) { return diff --git a/scripts/release.py b/scripts/release.py index 68ad134d6cdb..10ed7b658a2f 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -76,6 +76,7 @@ AUTHOR_MAP = { "290859878+synapsesx@users.noreply.github.com": "synapsesx", "dirtyren@users.noreply.github.com": "dirtyren", "mharris@parallel.ai": "NormallyGaussian", + "roger@roger.local": "mollusk", "ted.malone@outlook.com": "temalo", "adityamalik2833@gmail.com": "alarcritty", "islam666@users.noreply.github.com": "islam666", From 914befa9aaae46f2709373bc3d7352e813a18c54 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 10 Jun 2026 15:36:51 -0700 Subject: [PATCH 10/28] feat(dashboard): profile-scoped skills & toolsets management MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 'Set as active' on the Profiles page only flips the sticky active_profile file (future CLI/gateway runs) — it never retargets the running dashboard process. The skills/toolsets endpoints called bare load_config()/ save_config(), so after 'activating' a profile in the web UI, deactivating a skill silently wrote into the dashboard's own profile and the activated profile was untouched. Backend: - _profile_scope() context manager on the skills/toolsets endpoints: context-local HERMES_HOME override for call-time config resolution + cron-style locked swap of tools.skills_tool's import-time SKILLS_DIR - profile param on /api/skills, /api/skills/toggle, /api/tools/toolsets* (list/toggle/config/provider/env), hub sources/search installed-state - hub install/uninstall/update spawn 'hermes -p skills ...' so the child rebinds skills_hub.SKILLS_DIR at import (the override cannot reach import-time globals); profile validated -> 404/400 before spawn Frontend: - Skills page: profile selector (deep-linkable /skills?profile=), amber banner naming the managed profile, threaded through skill toggles, toolset drawer, and hub browser - Profiles page: 'Manage skills & tools' action per card; 'Set as active' toast now says it applies to new CLI/gateway runs only Omitted profile keeps legacy behavior (dashboard's own profile). --- hermes_cli/web_server.py | 327 ++++++++++++------ .../test_web_server_skills_profiles.py | 210 +++++++++++ web/src/components/ToolsetConfigDrawer.tsx | 15 +- web/src/i18n/en.ts | 4 + web/src/i18n/types.ts | 6 + web/src/lib/api.ts | 66 ++-- web/src/pages/ProfilesPage.tsx | 34 +- web/src/pages/SkillsPage.tsx | 152 +++++++- 8 files changed, 662 insertions(+), 152 deletions(-) create mode 100644 tests/hermes_cli/test_web_server_skills_profiles.py diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index e1f1c62051df..f7cc695874ad 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -9,7 +9,7 @@ Usage: python -m hermes_cli.main web --port 8080 """ -from contextlib import asynccontextmanager +from contextlib import asynccontextmanager, contextmanager import asyncio import base64 @@ -7373,6 +7373,24 @@ async def prune_checkpoints(): class SkillInstallRequest(BaseModel): identifier: str + profile: Optional[str] = None + + +def _profile_cli_args(profile: Optional[str]) -> List[str]: + """Return ``["-p", ]`` for a validated non-default profile. + + Hub install/uninstall/update run in a fresh ``hermes`` subprocess, and + ``_apply_profile_override()`` reads ``-p`` from argv in the child — the + only mechanism that reaches import-time-bound globals like + ``skills_hub.SKILLS_DIR``. Empty/"current" means the dashboard's own + profile (no args, legacy behavior). + """ + requested = (profile or "").strip() + if not requested or requested.lower() == "current": + return [] + from hermes_cli import profiles as profiles_mod + _resolve_profile_dir(requested) + return ["-p", profiles_mod.normalize_profile_name(requested)] @app.post("/api/skills/hub/install") @@ -7381,7 +7399,12 @@ async def install_skill_hub(body: SkillInstallRequest): if not identifier: raise HTTPException(status_code=400, detail="identifier is required") try: - proc = _spawn_hermes_action(["skills", "install", identifier], "skills-install") + proc = _spawn_hermes_action( + _profile_cli_args(body.profile) + ["skills", "install", identifier], + "skills-install", + ) + except HTTPException: + raise except Exception as exc: _log.exception("Failed to spawn skills install") raise HTTPException(status_code=500, detail=f"Failed to install skill: {exc}") @@ -7390,6 +7413,7 @@ async def install_skill_hub(body: SkillInstallRequest): class SkillUninstallRequest(BaseModel): name: str + profile: Optional[str] = None @app.post("/api/skills/hub/uninstall") @@ -7398,17 +7422,31 @@ async def uninstall_skill_hub(body: SkillUninstallRequest): if not name: raise HTTPException(status_code=400, detail="name is required") try: - proc = _spawn_hermes_action(["skills", "uninstall", name, "--yes"], "skills-uninstall") + proc = _spawn_hermes_action( + _profile_cli_args(body.profile) + ["skills", "uninstall", name, "--yes"], + "skills-uninstall", + ) + except HTTPException: + raise except Exception as exc: _log.exception("Failed to spawn skills uninstall") raise HTTPException(status_code=500, detail=f"Failed to uninstall skill: {exc}") return {"ok": True, "pid": proc.pid, "name": "skills-uninstall"} +class SkillsUpdateRequest(BaseModel): + profile: Optional[str] = None + + @app.post("/api/skills/hub/update") -async def update_skills_hub(): +async def update_skills_hub(body: Optional[SkillsUpdateRequest] = None): try: - proc = _spawn_hermes_action(["skills", "update"], "skills-update") + profile = body.profile if body else None + proc = _spawn_hermes_action( + _profile_cli_args(profile) + ["skills", "update"], "skills-update" + ) + except HTTPException: + raise except Exception as exc: _log.exception("Failed to spawn skills update") raise HTTPException(status_code=500, detail=f"Failed to update skills: {exc}") @@ -7443,17 +7481,25 @@ def _skill_meta_to_payload(m) -> dict: } -def _installed_hub_identifiers() -> dict: +def _installed_hub_identifiers(profile: Optional[str] = None) -> dict: """Map identifier -> installed lock entry for hub-installed skills. - Lets the UI mark search results that are already installed. Best-effort: - returns an empty dict if the lock file can't be read. + Lets the UI mark search results that are already installed. Scoped to + ``profile``'s skills/.hub/lock.json when provided (HubLockFile takes an + explicit path, sidestepping the import-time LOCK_FILE binding). + Best-effort: returns an empty dict if the lock file can't be read. """ try: from tools.skills_hub import HubLockFile + requested = (profile or "").strip() + if requested and requested.lower() != "current": + profile_dir = _resolve_profile_dir(requested) + lock = HubLockFile(profile_dir / "skills" / ".hub" / "lock.json") + else: + lock = HubLockFile() out = {} - for entry in HubLockFile().list_installed(): + for entry in lock.list_installed(): ident = entry.get("identifier") if ident: out[ident] = { @@ -7467,13 +7513,14 @@ def _installed_hub_identifiers() -> dict: @app.get("/api/skills/hub/sources") -async def list_skills_hub_sources(): +async def list_skills_hub_sources(profile: Optional[str] = None): """List the configured skill-hub sources and installed-skill provenance. Gives the dashboard something to show BEFORE a search runs — which hubs are wired up, their trust tier, and a set of featured skills pulled from the centralized index (zero extra API calls). Without this the Browse-hub tab is a blank page with no indication it's even connected to anything. + ``profile`` scopes the installed-skill provenance to that profile. """ def _run(): @@ -7514,18 +7561,22 @@ async def list_skills_hub_sources(): "sources": out, "index_available": index_available, "featured": featured, - "installed": _installed_hub_identifiers(), + "installed": _installed_hub_identifiers(profile), } try: return await asyncio.to_thread(_run) + except HTTPException: + raise except Exception as exc: _log.exception("skills hub sources listing failed") raise HTTPException(status_code=502, detail=f"Hub sources failed: {exc}") @app.get("/api/skills/hub/search") -async def search_skills_hub(q: str = "", source: str = "all", limit: int = 20): +async def search_skills_hub( + q: str = "", source: str = "all", limit: int = 20, profile: Optional[str] = None +): """Search the skill hub across all configured sources. Network-bound (parallel source search); runs in a thread so the FastAPI @@ -7560,11 +7611,13 @@ async def search_skills_hub(q: str = "", source: str = "all", limit: int = 20): "results": [_skill_meta_to_payload(m) for m in deduped], "source_counts": source_counts, "timed_out": timed_out, - "installed": _installed_hub_identifiers(), + "installed": _installed_hub_identifiers(profile), } try: return await asyncio.to_thread(_run) + except HTTPException: + raise except Exception as exc: _log.exception("skills hub search failed") raise HTTPException(status_code=502, detail=f"Hub search failed: {exc}") @@ -8333,21 +8386,75 @@ async def describe_profile_auto_endpoint(name: str, body: ProfileDescribeAuto): # --------------------------------------------------------------------------- # Skills & Tools endpoints +# +# Every read/write below accepts an optional ``profile`` query param so the +# dashboard can manage ANY profile's skills/toolsets, not just the profile +# the dashboard process happens to be running under. Without this, "Set as +# active" on the Profiles page (which only flips the sticky ``active_profile`` +# file for FUTURE CLI/gateway invocations) misled users into thinking skill +# toggles would land in the activated profile — they silently wrote into the +# dashboard's own config instead. See _profile_scope() for the mechanism. # --------------------------------------------------------------------------- +_SKILLS_PROFILE_LOCK = threading.RLock() + + +@contextmanager +def _profile_scope(profile: Optional[str]): + """Scope config + skill-directory resolution to ``profile`` for one request. + + Two seams must be redirected for skills/toolsets endpoints: + + 1. ``load_config``/``save_config`` resolve ``get_hermes_home()`` at call + time — the context-local override from ``set_hermes_home_override`` + reaches them (same pattern as ``_write_profile_model``). + 2. ``tools.skills_tool`` binds ``SKILLS_DIR`` at import time, so the + override CANNOT reach it. Like ``_call_cron_for_profile`` does for + cron's module globals, temporarily retarget it under a lock and + restore it immediately after. + + ``profile`` of None/""/"current" means "the dashboard's own profile" — + a no-op scope, preserving existing behavior for old clients. + """ + requested = (profile or "").strip() + if not requested or requested.lower() == "current": + yield None + return + + profile_dir = _resolve_profile_dir(requested) + + from hermes_constants import set_hermes_home_override, reset_hermes_home_override + from tools import skills_tool as _skills_tool + + token = set_hermes_home_override(str(profile_dir)) + with _SKILLS_PROFILE_LOCK: + old_home = _skills_tool.HERMES_HOME + old_skills_dir = _skills_tool.SKILLS_DIR + _skills_tool.HERMES_HOME = profile_dir + _skills_tool.SKILLS_DIR = profile_dir / "skills" + try: + yield profile_dir + finally: + _skills_tool.HERMES_HOME = old_home + _skills_tool.SKILLS_DIR = old_skills_dir + reset_hermes_home_override(token) + + class SkillToggle(BaseModel): name: str enabled: bool + profile: Optional[str] = None @app.get("/api/skills") -async def get_skills(): +async def get_skills(profile: Optional[str] = None): from tools.skills_tool import _find_all_skills from hermes_cli.skills_config import get_disabled_skills - config = load_config() - disabled = get_disabled_skills(config) - skills = _find_all_skills(skip_disabled=True) + with _profile_scope(profile): + config = load_config() + disabled = get_disabled_skills(config) + skills = _find_all_skills(skip_disabled=True) for s in skills: s["enabled"] = s["name"] not in disabled return skills @@ -8356,18 +8463,19 @@ async def get_skills(): @app.put("/api/skills/toggle") async def toggle_skill(body: SkillToggle): from hermes_cli.skills_config import get_disabled_skills, save_disabled_skills - config = load_config() - disabled = get_disabled_skills(config) - if body.enabled: - disabled.discard(body.name) - else: - disabled.add(body.name) - save_disabled_skills(config, disabled) + with _profile_scope(body.profile): + config = load_config() + disabled = get_disabled_skills(config) + if body.enabled: + disabled.discard(body.name) + else: + disabled.add(body.name) + save_disabled_skills(config, disabled) return {"ok": True, "name": body.name, "enabled": body.enabled} @app.get("/api/tools/toolsets") -async def get_toolsets(): +async def get_toolsets(profile: Optional[str] = None): from hermes_cli.tools_config import ( _get_effective_configurable_toolsets, _get_platform_tools, @@ -8376,12 +8484,13 @@ async def get_toolsets(): ) from toolsets import resolve_toolset - config = load_config() - enabled_toolsets = _get_platform_tools( - config, - "cli", - include_default_mcp_servers=False, - ) + with _profile_scope(profile): + config = load_config() + enabled_toolsets = _get_platform_tools( + config, + "cli", + include_default_mcp_servers=False, + ) result = [] for name, label, desc in _get_effective_configurable_toolsets(): try: @@ -8403,6 +8512,7 @@ async def get_toolsets(): class ToolsetToggle(BaseModel): enabled: bool + profile: Optional[str] = None @app.put("/api/tools/toolsets/{name}") @@ -8411,7 +8521,8 @@ async def toggle_toolset(name: str, body: ToolsetToggle): Persists to ``platform_toolsets.cli`` via the same ``_save_platform_tools`` helper the CLI ``hermes tools`` picker uses, so the GUI and CLI stay in - lockstep. Returns 400 for unknown toolset keys. + lockstep. Scoped to ``body.profile`` when provided. Returns 400 for + unknown toolset keys. """ from hermes_cli.tools_config import ( _get_effective_configurable_toolsets, @@ -8423,20 +8534,21 @@ async def toggle_toolset(name: str, body: ToolsetToggle): if name not in valid: raise HTTPException(status_code=400, detail=f"Unknown toolset: {name}") - config = load_config() - enabled = set( - _get_platform_tools(config, "cli", include_default_mcp_servers=False) - ) - if body.enabled: - enabled.add(name) - else: - enabled.discard(name) - _save_platform_tools(config, "cli", enabled) + with _profile_scope(body.profile): + config = load_config() + enabled = set( + _get_platform_tools(config, "cli", include_default_mcp_servers=False) + ) + if body.enabled: + enabled.add(name) + else: + enabled.discard(name) + _save_platform_tools(config, "cli", enabled) return {"ok": True, "name": name, "enabled": body.enabled} @app.get("/api/tools/toolsets/{name}/config") -async def get_toolset_config(name: str): +async def get_toolset_config(name: str, profile: Optional[str] = None): """Return the provider matrix + key status for a toolset's config panel. Surfaces the same provider rows the CLI ``hermes tools`` picker shows @@ -8457,38 +8569,39 @@ async def get_toolset_config(name: str): if name not in valid: raise HTTPException(status_code=400, detail=f"Unknown toolset: {name}") - config = load_config() - cat = TOOL_CATEGORIES.get(name) - providers = [] - active_provider = None - if cat: - for prov in _visible_providers(cat, config, force_fresh=True): - env_vars = [ - { - "key": e["key"], - "prompt": e.get("prompt", e["key"]), - "url": e.get("url"), - "default": e.get("default"), - "is_set": bool(get_env_value(e["key"])), - } - for e in prov.get("env_vars", []) - ] - # Surface the same active-provider determination the CLI picker - # uses (``_is_provider_active``) so the GUI highlights the provider - # actually written to config (e.g. web.backend), not just the first - # keyless one in the list. - is_active = _is_provider_active(prov, config, force_fresh=True) - if is_active and active_provider is None: - active_provider = prov["name"] - providers.append({ - "name": prov["name"], - "badge": prov.get("badge", ""), - "tag": prov.get("tag", ""), - "env_vars": env_vars, - "post_setup": prov.get("post_setup"), - "requires_nous_auth": bool(prov.get("requires_nous_auth")), - "is_active": is_active, - }) + with _profile_scope(profile): + config = load_config() + cat = TOOL_CATEGORIES.get(name) + providers = [] + active_provider = None + if cat: + for prov in _visible_providers(cat, config, force_fresh=True): + env_vars = [ + { + "key": e["key"], + "prompt": e.get("prompt", e["key"]), + "url": e.get("url"), + "default": e.get("default"), + "is_set": bool(get_env_value(e["key"])), + } + for e in prov.get("env_vars", []) + ] + # Surface the same active-provider determination the CLI picker + # uses (``_is_provider_active``) so the GUI highlights the provider + # actually written to config (e.g. web.backend), not just the first + # keyless one in the list. + is_active = _is_provider_active(prov, config, force_fresh=True) + if is_active and active_provider is None: + active_provider = prov["name"] + providers.append({ + "name": prov["name"], + "badge": prov.get("badge", ""), + "tag": prov.get("tag", ""), + "env_vars": env_vars, + "post_setup": prov.get("post_setup"), + "requires_nous_auth": bool(prov.get("requires_nous_auth")), + "is_active": is_active, + }) return { "name": name, "has_category": cat is not None, @@ -8499,6 +8612,7 @@ async def get_toolset_config(name: str): class ToolsetProviderSelect(BaseModel): provider: str + profile: Optional[str] = None @app.put("/api/tools/toolsets/{name}/provider") @@ -8520,17 +8634,19 @@ async def select_toolset_provider(name: str, body: ToolsetProviderSelect): if name not in valid: raise HTTPException(status_code=400, detail=f"Unknown toolset: {name}") - config = load_config() - try: - apply_provider_selection(name, body.provider, config) - except KeyError as exc: - raise HTTPException(status_code=400, detail=str(exc).strip('"')) - save_config(config) + with _profile_scope(body.profile): + config = load_config() + try: + apply_provider_selection(name, body.provider, config) + except KeyError as exc: + raise HTTPException(status_code=400, detail=str(exc).strip('"')) + save_config(config) return {"ok": True, "name": name, "provider": body.provider} class ToolsetEnvUpdate(BaseModel): env: Dict[str, str] + profile: Optional[str] = None @app.put("/api/tools/toolsets/{name}/env") @@ -8556,34 +8672,35 @@ async def save_toolset_env(name: str, body: ToolsetEnvUpdate): if name not in valid_ts: raise HTTPException(status_code=400, detail=f"Unknown toolset: {name}") - config = load_config() - cat = TOOL_CATEGORIES.get(name) - allowed: set[str] = set() - if cat: - for prov in _visible_providers(cat, config, force_fresh=True): - for e in prov.get("env_vars", []): - allowed.add(e["key"]) + with _profile_scope(body.profile): + config = load_config() + cat = TOOL_CATEGORIES.get(name) + allowed: set[str] = set() + if cat: + for prov in _visible_providers(cat, config, force_fresh=True): + for e in prov.get("env_vars", []): + allowed.add(e["key"]) - unknown = [k for k in body.env if k not in allowed] - if unknown: - raise HTTPException( - status_code=400, - detail=f"Unknown env var(s) for toolset {name}: {', '.join(sorted(unknown))}", - ) + unknown = [k for k in body.env if k not in allowed] + if unknown: + raise HTTPException( + status_code=400, + detail=f"Unknown env var(s) for toolset {name}: {', '.join(sorted(unknown))}", + ) - saved: List[str] = [] - skipped: List[str] = [] - for key, value in body.env.items(): - if value and value.strip(): - try: - save_env_value(key, value.strip()) - except ValueError as exc: - raise HTTPException(status_code=400, detail=str(exc)) - saved.append(key) - else: - skipped.append(key) + saved: List[str] = [] + skipped: List[str] = [] + for key, value in body.env.items(): + if value and value.strip(): + try: + save_env_value(key, value.strip()) + except ValueError as exc: + raise HTTPException(status_code=400, detail=str(exc)) + saved.append(key) + else: + skipped.append(key) - status = {k: bool(get_env_value(k)) for k in allowed} + status = {k: bool(get_env_value(k)) for k in allowed} return {"ok": True, "name": name, "saved": saved, "skipped": skipped, "is_set": status} diff --git a/tests/hermes_cli/test_web_server_skills_profiles.py b/tests/hermes_cli/test_web_server_skills_profiles.py new file mode 100644 index 000000000000..9a131bbb2460 --- /dev/null +++ b/tests/hermes_cli/test_web_server_skills_profiles.py @@ -0,0 +1,210 @@ +"""Regression tests for dashboard profile-scoped skills/toolsets management. + +"Set as active" on the Profiles page only flips the sticky ``active_profile`` +file (future CLI/gateway runs) — it never retargets the running dashboard +process. Before the ``profile`` parameter existed, toggling a skill after +"activating" a profile silently wrote into the dashboard's own config. +These tests pin the new behavior: reads and writes land in the REQUESTED +profile's HERMES_HOME, and the dashboard's own profile stays untouched. +""" +import pytest +import yaml + + +def _write_skill(skills_dir, name, description="test skill"): + d = skills_dir / name + d.mkdir(parents=True, exist_ok=True) + (d / "SKILL.md").write_text( + f"---\nname: {name}\ndescription: {description}\n---\n\n# {name}\n", + encoding="utf-8", + ) + + +@pytest.fixture +def isolated_profiles(tmp_path, monkeypatch, _isolate_hermes_home): + """Isolated default home + one named profile, each with its own skills.""" + from hermes_constants import get_hermes_home + from hermes_cli import profiles + + default_home = get_hermes_home() + profiles_root = default_home / "profiles" + worker_home = profiles_root / "worker_alpha" + for home in (default_home, worker_home): + (home / "skills").mkdir(parents=True, exist_ok=True) + (home / "config.yaml").write_text("{}\n", encoding="utf-8") + + _write_skill(default_home / "skills", "dashboard-skill") + _write_skill(worker_home / "skills", "worker-skill") + + monkeypatch.setattr(profiles, "_get_default_hermes_home", lambda: default_home) + monkeypatch.setattr(profiles, "_get_profiles_root", lambda: profiles_root) + return {"default": default_home, "worker_alpha": worker_home} + + +@pytest.fixture +def client(monkeypatch, isolated_profiles): + try: + from starlette.testclient import TestClient + except ImportError: + pytest.skip("fastapi/starlette not installed") + + import hermes_state + from hermes_constants import get_hermes_home + from hermes_cli.web_server import app, _SESSION_HEADER_NAME, _SESSION_TOKEN + + monkeypatch.setattr(hermes_state, "DEFAULT_DB_PATH", get_hermes_home() / "state.db") + c = TestClient(app) + c.headers[_SESSION_HEADER_NAME] = _SESSION_TOKEN + return c + + +def _load_cfg(home): + return yaml.safe_load((home / "config.yaml").read_text()) or {} + + +class TestProfileScopedSkills: + def test_skills_list_scopes_to_requested_profile(self, client, isolated_profiles): + resp = client.get("/api/skills", params={"profile": "worker_alpha"}) + assert resp.status_code == 200 + names = {s["name"] for s in resp.json()} + assert "worker-skill" in names + assert "dashboard-skill" not in names + + def test_skills_list_without_profile_uses_dashboard_home( + self, client, isolated_profiles + ): + resp = client.get("/api/skills") + assert resp.status_code == 200 + names = {s["name"] for s in resp.json()} + assert "dashboard-skill" in names + assert "worker-skill" not in names + + def test_toggle_writes_into_target_profile_only(self, client, isolated_profiles): + resp = client.put( + "/api/skills/toggle", + json={"name": "worker-skill", "enabled": False, "profile": "worker_alpha"}, + ) + assert resp.status_code == 200 + assert resp.json() == {"ok": True, "name": "worker-skill", "enabled": False} + + worker_cfg = _load_cfg(isolated_profiles["worker_alpha"]) + assert "worker-skill" in worker_cfg.get("skills", {}).get("disabled", []) + # The dashboard's own config must stay untouched — this was the bug. + default_cfg = _load_cfg(isolated_profiles["default"]) + assert "worker-skill" not in default_cfg.get("skills", {}).get("disabled", []) + + def test_toggle_reenable_round_trip(self, client, isolated_profiles): + for enabled in (False, True): + client.put( + "/api/skills/toggle", + json={ + "name": "worker-skill", + "enabled": enabled, + "profile": "worker_alpha", + }, + ) + worker_cfg = _load_cfg(isolated_profiles["worker_alpha"]) + assert "worker-skill" not in worker_cfg.get("skills", {}).get("disabled", []) + + def test_unknown_profile_returns_404(self, client, isolated_profiles): + resp = client.get("/api/skills", params={"profile": "no_such_profile"}) + assert resp.status_code == 404 + + def test_invalid_profile_name_returns_400(self, client, isolated_profiles): + resp = client.get("/api/skills", params={"profile": "Bad Name!"}) + assert resp.status_code == 400 + + def test_scope_restores_module_globals(self, client, isolated_profiles): + """The SKILLS_DIR swap is per-request; the module global must be + restored even after a scoped call (cron-style locked swap).""" + import tools.skills_tool as skills_tool + + before = skills_tool.SKILLS_DIR + client.get("/api/skills", params={"profile": "worker_alpha"}) + assert skills_tool.SKILLS_DIR == before + + +class TestProfileScopedToolsets: + def test_toolset_toggle_scopes_to_profile(self, client, isolated_profiles): + resp = client.put( + "/api/tools/toolsets/x_search", + json={"enabled": True, "profile": "worker_alpha"}, + ) + assert resp.status_code == 200 + + worker_cfg = _load_cfg(isolated_profiles["worker_alpha"]) + assert "x_search" in worker_cfg.get("platform_toolsets", {}).get("cli", []) + default_cfg = _load_cfg(isolated_profiles["default"]) + assert "x_search" not in default_cfg.get("platform_toolsets", {}).get("cli", []) + + listing = client.get( + "/api/tools/toolsets", params={"profile": "worker_alpha"} + ).json() + assert {t["name"]: t for t in listing}["x_search"]["enabled"] is True + # Unscoped listing reflects the dashboard's own (untouched) config. + listing = client.get("/api/tools/toolsets").json() + assert {t["name"]: t for t in listing}["x_search"]["enabled"] is False + + def test_toolset_toggle_unknown_profile_404(self, client, isolated_profiles): + resp = client.put( + "/api/tools/toolsets/x_search", + json={"enabled": True, "profile": "ghost"}, + ) + assert resp.status_code == 404 + + +class TestProfileScopedHubActions: + def test_hub_install_spawns_with_profile_flag( + self, client, isolated_profiles, monkeypatch + ): + """Hub installs must go through a fresh ``hermes -p `` + subprocess — the in-process scope can't reach skills_hub's + import-time SKILLS_DIR binding.""" + import hermes_cli.web_server as web_server + + calls = [] + + class _FakeProc: + pid = 4242 + + def _fake_spawn(subcommand, name): + calls.append((list(subcommand), name)) + return _FakeProc() + + monkeypatch.setattr(web_server, "_spawn_hermes_action", _fake_spawn) + resp = client.post( + "/api/skills/hub/install", + json={"identifier": "official/demo", "profile": "worker_alpha"}, + ) + assert resp.status_code == 200 + assert calls == [ + (["-p", "worker_alpha", "skills", "install", "official/demo"], "skills-install") + ] + + def test_hub_install_without_profile_keeps_legacy_argv( + self, client, isolated_profiles, monkeypatch + ): + import hermes_cli.web_server as web_server + + calls = [] + + class _FakeProc: + pid = 4242 + + monkeypatch.setattr( + web_server, + "_spawn_hermes_action", + lambda subcommand, name: calls.append(list(subcommand)) or _FakeProc(), + ) + resp = client.post( + "/api/skills/hub/install", json={"identifier": "official/demo"} + ) + assert resp.status_code == 200 + assert calls == [["skills", "install", "official/demo"]] + + def test_hub_install_unknown_profile_404(self, client, isolated_profiles): + resp = client.post( + "/api/skills/hub/install", + json={"identifier": "official/demo", "profile": "ghost"}, + ) + assert resp.status_code == 404 diff --git a/web/src/components/ToolsetConfigDrawer.tsx b/web/src/components/ToolsetConfigDrawer.tsx index 42e58d589f5b..5bbcba618662 100644 --- a/web/src/components/ToolsetConfigDrawer.tsx +++ b/web/src/components/ToolsetConfigDrawer.tsx @@ -20,6 +20,9 @@ import { cn, themedBody } from "@/lib/utils"; interface Props { /** The toolset whose backends are being configured. */ toolset: ToolsetInfo; + /** Optional profile to scope config reads/writes to (Skills page profile + * selector). Omitted = the dashboard process's own profile. */ + profile?: string; onClose: () => void; /** Called after a toggle/provider/key change so the parent grid refreshes. */ onChanged: () => void; @@ -31,7 +34,7 @@ interface Props { * the toolset on/off, pick a provider, enter API keys, and run a provider's * post-setup install hook (npm/pip/binary) with a live log tail. */ -export function ToolsetConfigDrawer({ toolset, onClose, onChanged }: Props) { +export function ToolsetConfigDrawer({ toolset, profile, onClose, onChanged }: Props) { const { toast, showToast } = useToast(); const [config, setConfig] = useState(null); const [loading, setLoading] = useState(true); @@ -60,7 +63,7 @@ export function ToolsetConfigDrawer({ toolset, onClose, onChanged }: Props) { // react-hooks/set-state-in-effect — setState only fires inside the // async .then/.catch/.finally callbacks. return api - .getToolsetConfig(toolset.name) + .getToolsetConfig(toolset.name, profile) .then((cfg) => { setConfig(cfg); setActiveProvider(cfg.active_provider); @@ -72,7 +75,7 @@ export function ToolsetConfigDrawer({ toolset, onClose, onChanged }: Props) { }) .catch(() => showToast("Failed to load toolset config", "error")) .finally(() => setLoading(false)); - }, [toolset.name, showToast]); + }, [toolset.name, profile, showToast]); useEffect(() => { void loadConfig(); @@ -121,7 +124,7 @@ export function ToolsetConfigDrawer({ toolset, onClose, onChanged }: Props) { const handleToggle = async (next: boolean) => { setToggling(true); try { - await api.toggleToolset(toolset.name, next); + await api.toggleToolset(toolset.name, next, profile); setEnabled(next); showToast( `${toolset.label || toolset.name} ${next ? "enabled" : "disabled"}`, @@ -138,7 +141,7 @@ export function ToolsetConfigDrawer({ toolset, onClose, onChanged }: Props) { const handleSelectProvider = async (provider: ToolsetProvider) => { setSelecting(provider.name); try { - await api.selectToolsetProvider(toolset.name, provider.name); + await api.selectToolsetProvider(toolset.name, provider.name, profile); setActiveProvider(provider.name); showToast(`Provider set to ${provider.name}`, "success"); onChanged(); @@ -164,7 +167,7 @@ export function ToolsetConfigDrawer({ toolset, onClose, onChanged }: Props) { } setSavingProvider(provider.name); try { - const res = await api.saveToolsetEnv(toolset.name, env); + const res = await api.saveToolsetEnv(toolset.name, env, profile); setIsSet((prev) => ({ ...prev, ...res.is_set })); // Clear saved drafts so the inputs reset to the "saved" placeholder. setDrafts((prev) => { diff --git a/web/src/i18n/en.ts b/web/src/i18n/en.ts index 4f593487fd39..39cf80d6995e 100644 --- a/web/src/i18n/en.ts +++ b/web/src/i18n/en.ts @@ -408,6 +408,10 @@ export const en: Translations = { setupNeeded: "Setup needed", disabledForCli: "Disabled for CLI", more: "+{count} more", + profileSelector: "Profile", + currentProfile: "current ({name})", + managingProfile: + "Managing profile \u201c{name}\u201d — toggles apply to that profile, not this dashboard\u2019s.", }, config: { diff --git a/web/src/i18n/types.ts b/web/src/i18n/types.ts index 14bc41f2d085..cac5688bdc6d 100644 --- a/web/src/i18n/types.ts +++ b/web/src/i18n/types.ts @@ -404,6 +404,8 @@ export interface Translations { modelSaved?: string; modelSelect?: string; actions?: string; + manageSkills?: string; + activeSetHint?: string; }; // ── Skills page ── @@ -425,6 +427,10 @@ export interface Translations { setupNeeded: string; disabledForCli: string; more: string; + /** Optional — fall back to English literals until translated. */ + profileSelector?: string; + currentProfile?: string; + managingProfile?: string; }; // ── Config page ── diff --git a/web/src/lib/api.ts b/web/src/lib/api.ts index 37a8f15eba26..7cde6eb78f95 100644 --- a/web/src/lib/api.ts +++ b/web/src/lib/api.ts @@ -249,6 +249,14 @@ export async function buildWsUrl( return `${proto}//${window.location.host}${BASE}${path}?${qs}`; } +/** Build a ``?profile=`` query suffix, or "" when unset. + * + * Used by the skills/toolsets endpoints so the dashboard can manage a + * profile other than the one the server process runs under. */ +function profileQuery(profile?: string): string { + return profile ? `?profile=${encodeURIComponent(profile)}` : ""; +} + export const api = { getStatus: () => fetchJSON("/api/status"), /** @@ -542,43 +550,49 @@ export const api = { ), // Skills & Toolsets - getSkills: () => fetchJSON("/api/skills"), - toggleSkill: (name: string, enabled: boolean) => + // + // All calls accept an optional ``profile`` so the Skills page can manage + // any profile's skills/toolsets — not just the one the dashboard process + // runs under. Omitted/empty profile = the dashboard's own profile. + getSkills: (profile?: string) => + fetchJSON(`/api/skills${profileQuery(profile)}`), + toggleSkill: (name: string, enabled: boolean, profile?: string) => fetchJSON<{ ok: boolean }>("/api/skills/toggle", { method: "PUT", headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ name, enabled }), + body: JSON.stringify({ name, enabled, profile: profile || undefined }), }), - getToolsets: () => fetchJSON("/api/tools/toolsets"), - toggleToolset: (name: string, enabled: boolean) => + getToolsets: (profile?: string) => + fetchJSON(`/api/tools/toolsets${profileQuery(profile)}`), + toggleToolset: (name: string, enabled: boolean, profile?: string) => fetchJSON<{ ok: boolean; name: string; enabled: boolean }>( `/api/tools/toolsets/${encodeURIComponent(name)}`, { method: "PUT", headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ enabled }), + body: JSON.stringify({ enabled, profile: profile || undefined }), }, ), - getToolsetConfig: (name: string) => + getToolsetConfig: (name: string, profile?: string) => fetchJSON( - `/api/tools/toolsets/${encodeURIComponent(name)}/config`, + `/api/tools/toolsets/${encodeURIComponent(name)}/config${profileQuery(profile)}`, ), - selectToolsetProvider: (name: string, provider: string) => + selectToolsetProvider: (name: string, provider: string, profile?: string) => fetchJSON<{ ok: boolean; name: string; provider: string }>( `/api/tools/toolsets/${encodeURIComponent(name)}/provider`, { method: "PUT", headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ provider }), + body: JSON.stringify({ provider, profile: profile || undefined }), }, ), - saveToolsetEnv: (name: string, env: Record) => + saveToolsetEnv: (name: string, env: Record, profile?: string) => fetchJSON( `/api/tools/toolsets/${encodeURIComponent(name)}/env`, { method: "PUT", headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ env }), + body: JSON.stringify({ env, profile: profile || undefined }), }, ), runToolsetPostSetup: (name: string, key: string) => @@ -986,26 +1000,34 @@ export const api = { fetchJSON("/api/ops/checkpoints/prune", { method: "POST" }), // ── Admin: Skills hub ─────────────────────────────────────────────── - installSkillFromHub: (identifier: string) => + // ``profile`` scopes install/uninstall/update and the installed-state + // annotations to that profile (omitted = the dashboard's own profile). + installSkillFromHub: (identifier: string, profile?: string) => fetchJSON("/api/skills/hub/install", { method: "POST", headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ identifier }), + body: JSON.stringify({ identifier, profile: profile || undefined }), }), - uninstallSkillFromHub: (name: string) => + uninstallSkillFromHub: (name: string, profile?: string) => fetchJSON("/api/skills/hub/uninstall", { method: "POST", headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ name }), + body: JSON.stringify({ name, profile: profile || undefined }), }), - updateSkillsFromHub: () => - fetchJSON("/api/skills/hub/update", { method: "POST" }), - searchSkillsHub: (q: string, source = "all", limit = 20) => + updateSkillsFromHub: (profile?: string) => + fetchJSON("/api/skills/hub/update", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ profile: profile || undefined }), + }), + searchSkillsHub: (q: string, source = "all", limit = 20, profile?: string) => fetchJSON( - `/api/skills/hub/search?q=${encodeURIComponent(q)}&source=${encodeURIComponent(source)}&limit=${limit}`, + `/api/skills/hub/search?q=${encodeURIComponent(q)}&source=${encodeURIComponent(source)}&limit=${limit}${profile ? `&profile=${encodeURIComponent(profile)}` : ""}`, + ), + getSkillHubSources: (profile?: string) => + fetchJSON( + `/api/skills/hub/sources${profileQuery(profile)}`, ), - getSkillHubSources: () => - fetchJSON("/api/skills/hub/sources"), previewSkillFromHub: (identifier: string) => fetchJSON( `/api/skills/hub/preview?identifier=${encodeURIComponent(identifier)}`, diff --git a/web/src/pages/ProfilesPage.tsx b/web/src/pages/ProfilesPage.tsx index bda2515528f0..a1e69bfbcc84 100644 --- a/web/src/pages/ProfilesPage.tsx +++ b/web/src/pages/ProfilesPage.tsx @@ -22,6 +22,7 @@ import { X, } from "lucide-react"; import spinners from "unicode-animations"; +import { useNavigate } from "react-router-dom"; import { H2 } from "@nous-research/ui/ui/components/typography/h2"; import { api } from "@/lib/api"; import type { ActiveProfileInfo, ProfileInfo } from "@/lib/api"; @@ -96,6 +97,7 @@ function ProfileActionsMenu({ onEditDescription, onEditModel, onEditSoul, + onManageSkills, onRename, onSetActive, }: ProfileActionsMenuProps) { @@ -201,6 +203,16 @@ function ProfileActionsMenu({ {labels.editSoul} + + + + + )} + + {restartMessage && !restartNeeded && ( + + + + {restartMessage} + + + )} + + {restartNeeded && ( + + +
+ + + {restartError ?? + "Webhooks are enabled, but the gateway still needs a restart before the receiver can come online."}
+
)} @@ -400,8 +530,8 @@ export default function WebhooksPage() {

- Disabled webhooks reject incoming events; the gateway hot-reloads - changes (no restart needed). + Subscription changes hot-reload once the webhook receiver is running. + Disabled subscriptions reject incoming events.

{subscriptions.length === 0 && ( From 264ac72b676b634c0f63b26af53c90205121bffd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=BC=AC=E5=90=9B=E5=A4=8F=E7=BA=AA?= <470766206@qq.com> Date: Sun, 7 Jun 2026 20:49:36 +0800 Subject: [PATCH 27/28] fix(gateway,windows): preserve restart watcher env --- gateway/run.py | 72 +++++++++++++++++++++++++++++ tests/gateway/test_restart_drain.py | 64 +++++++++++++++++++++++++ 2 files changed, 136 insertions(+) diff --git a/gateway/run.py b/gateway/run.py index 708106ad8c8b..ff82fe435825 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -32,6 +32,7 @@ import logging import os import re import shlex +import site import sys import signal import tempfile @@ -135,6 +136,60 @@ _GATEWAY_SECRET_PATTERNS = ( ) +def _ensure_windows_gateway_venv_imports() -> None: + """Make detached Windows gateway runs see the Hermes venv packages. + + Some Windows restart paths run the gateway under uv's base ``pythonw.exe`` + to avoid the venv launcher respawning a visible console interpreter. That + mode can import the source tree via cwd/PYTHONPATH but still miss optional + packages installed only in ``venv/Lib/site-packages`` (notably the MCP SDK). + Patch the live process before MCP discovery so tool injection does not + depend on every launcher preserving PYTHONPATH perfectly. + """ + if sys.platform != "win32": + return + + project_root = Path(__file__).resolve().parent.parent + candidates: list[Path] = [] + if os.environ.get("VIRTUAL_ENV"): + candidates.append(Path(os.environ["VIRTUAL_ENV"])) + candidates.append(project_root / "venv") + + seen: set[str] = set() + for venv_dir in candidates: + try: + resolved_venv = venv_dir.resolve() + except OSError: + resolved_venv = venv_dir + venv_key = str(resolved_venv).lower() + if venv_key in seen: + continue + seen.add(venv_key) + + site_packages = resolved_venv / "Lib" / "site-packages" + if not site_packages.exists(): + continue + + project_entry = str(project_root) + site_entry = str(site_packages) + if project_entry not in sys.path: + sys.path.insert(0, project_entry) + # addsitepackages() semantics matter here: pywin32, used by the MCP + # SDK on Windows, relies on .pth processing to expose pywintypes. + site.addsitedir(site_entry) + if site_entry in sys.path: + sys.path.remove(site_entry) + insert_at = 1 if sys.path and sys.path[0] == project_entry else 0 + sys.path.insert(insert_at, site_entry) + + os.environ["VIRTUAL_ENV"] = str(resolved_venv) + pythonpath = [project_entry, site_entry] + if os.environ.get("PYTHONPATH"): + pythonpath.append(os.environ["PYTHONPATH"]) + os.environ["PYTHONPATH"] = os.pathsep.join(dict.fromkeys(pythonpath)) + return + + def _gateway_platform_value(platform: Any) -> str: """Return a normalized gateway platform value for enums or raw strings.""" return str(getattr(platform, "value", platform) or "").strip().lower() @@ -4255,10 +4310,25 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew ) """ ).strip() + watcher_env = os.environ.copy() + # This watcher is intentionally outside the running gateway. If it + # inherits the gateway marker, `hermes gateway restart` refuses to + # run as a self-restart loop guard and the gateway stays stopped. + watcher_env.pop("_HERMES_GATEWAY", None) + project_root = Path(__file__).resolve().parent.parent + venv_dir = Path(watcher_env.get("VIRTUAL_ENV") or project_root / "venv") + site_packages = venv_dir / "Lib" / "site-packages" + if site_packages.exists(): + watcher_env["VIRTUAL_ENV"] = str(venv_dir) + pythonpath = [str(project_root), str(site_packages)] + if watcher_env.get("PYTHONPATH"): + pythonpath.append(watcher_env["PYTHONPATH"]) + watcher_env["PYTHONPATH"] = os.pathsep.join(dict.fromkeys(pythonpath)) subprocess.Popen( [sys.executable, "-c", watcher, str(current_pid), *cmd_argv], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + env=watcher_env, **windows_detach_popen_kwargs(), ) return @@ -15910,6 +15980,8 @@ async def start_gateway(config: Optional[GatewayConfig] = None, replace: bool = atexit.register(remove_pid_file) atexit.register(release_gateway_runtime_lock) + _ensure_windows_gateway_venv_imports() + # MCP tool discovery — run in an executor so the asyncio event loop # stays responsive even when a configured MCP server is slow or # unreachable. discover_mcp_tools() uses a blocking 120s wait diff --git a/tests/gateway/test_restart_drain.py b/tests/gateway/test_restart_drain.py index a48e5f737810..32710fdb8979 100644 --- a/tests/gateway/test_restart_drain.py +++ b/tests/gateway/test_restart_drain.py @@ -197,6 +197,7 @@ async def test_launch_detached_restart_command_uses_setsid(monkeypatch): runner, _adapter = make_restart_runner() popen_calls = [] + monkeypatch.setattr(gateway_run.sys, "platform", "linux") monkeypatch.setattr(gateway_run, "_resolve_hermes_bin", lambda: ["/usr/bin/hermes"]) monkeypatch.setattr(gateway_run.os, "getpid", lambda: 321) monkeypatch.setattr(shutil, "which", lambda cmd: "/usr/bin/setsid" if cmd == "setsid" else None) @@ -219,6 +220,69 @@ async def test_launch_detached_restart_command_uses_setsid(monkeypatch): assert kwargs["stderr"] is subprocess.DEVNULL +def test_windows_gateway_venv_imports_add_site_packages(monkeypatch, tmp_path): + venv_dir = tmp_path / "venv" + site_packages = venv_dir / "Lib" / "site-packages" + pth_extra = tmp_path / "pywin32_system32" + site_packages.mkdir(parents=True) + pth_extra.mkdir() + (site_packages / "pywin32.pth").write_text(str(pth_extra), encoding="utf-8") + project_root = str(gateway_run.Path(gateway_run.__file__).resolve().parent.parent) + + monkeypatch.setattr(gateway_run.sys, "platform", "win32") + monkeypatch.setattr(gateway_run.sys, "path", ["existing"]) + monkeypatch.setenv("VIRTUAL_ENV", str(venv_dir)) + monkeypatch.setenv("PYTHONPATH", "already-there") + + gateway_run._ensure_windows_gateway_venv_imports() + + assert gateway_run.sys.path[:2] == [project_root, str(site_packages)] + assert str(pth_extra) in gateway_run.sys.path + assert gateway_run.os.environ["VIRTUAL_ENV"] == str(venv_dir.resolve()) + pythonpath = gateway_run.os.environ["PYTHONPATH"].split(gateway_run.os.pathsep) + assert pythonpath[:3] == [project_root, str(site_packages), "already-there"] + + +@pytest.mark.asyncio +async def test_windows_detached_restart_scrubs_gateway_marker(monkeypatch, tmp_path): + runner, _adapter = make_restart_runner() + popen_calls = [] + venv_dir = tmp_path / "venv" + site_packages = venv_dir / "Lib" / "site-packages" + site_packages.mkdir(parents=True) + + monkeypatch.setattr(gateway_run.sys, "platform", "win32") + monkeypatch.setattr(gateway_run, "_resolve_hermes_bin", lambda: ["hermes"]) + monkeypatch.setattr(gateway_run.os, "getpid", lambda: 321) + monkeypatch.setenv("_HERMES_GATEWAY", "1") + monkeypatch.setenv("VIRTUAL_ENV", str(venv_dir)) + + import hermes_cli._subprocess_compat as subprocess_compat + + monkeypatch.setattr( + subprocess_compat, + "windows_detach_popen_kwargs", + lambda: {}, + ) + + def fake_popen(cmd, **kwargs): + popen_calls.append((cmd, kwargs)) + return MagicMock() + + monkeypatch.setattr(subprocess, "Popen", fake_popen) + + await runner._launch_detached_restart_command() + + assert len(popen_calls) == 1 + cmd, kwargs = popen_calls[0] + assert cmd[-3:] == ["hermes", "gateway", "restart"] + assert kwargs["env"].get("_HERMES_GATEWAY") is None + assert kwargs["env"]["VIRTUAL_ENV"] == str(venv_dir) + assert str(site_packages) in kwargs["env"]["PYTHONPATH"].split(gateway_run.os.pathsep) + assert kwargs["stdout"] is subprocess.DEVNULL + assert kwargs["stderr"] is subprocess.DEVNULL + + # ── Shutdown notification tests ────────────────────────────────────── From cb2c13055ed9c3f467be61dbef1f3a7a5dcdc5f6 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Wed, 10 Jun 2026 20:54:32 -0700 Subject: [PATCH 28/28] fix(gateway): scrub _HERMES_GATEWAY from POSIX detached restart watcher too Follow-up to the salvaged #41264 (Windows watcher): the setsid/bash detached restart watcher on Linux/macOS inherits _HERMES_GATEWAY=1 the same way, so the CLI's self-restart loop guard silently refuses 'hermes gateway restart' and the gateway never comes back. Scrub the marker from the watcher env on the POSIX branch as well, and extend the setsid test to assert it. --- gateway/run.py | 9 +++++++++ scripts/release.py | 1 + tests/gateway/test_restart_drain.py | 4 ++++ 3 files changed, 14 insertions(+) diff --git a/gateway/run.py b/gateway/run.py index ff82fe435825..897cb85f6523 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -4338,12 +4338,20 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew f"while kill -0 {current_pid} 2>/dev/null; do sleep 0.2; done; " f"{cmd} gateway restart" ) + # Same marker scrub as the Windows watcher above: this watcher runs + # `hermes gateway restart` from outside the gateway, but it inherits + # _HERMES_GATEWAY=1 from us, and the CLI's self-restart loop guard + # refuses to run when that marker is set — silently (DEVNULL), so the + # gateway stops and never comes back. + watcher_env = os.environ.copy() + watcher_env.pop("_HERMES_GATEWAY", None) setsid_bin = shutil.which("setsid") if setsid_bin: subprocess.Popen( [setsid_bin, "bash", "-lc", shell_cmd], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + env=watcher_env, start_new_session=True, ) else: @@ -4351,6 +4359,7 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew ["bash", "-lc", shell_cmd], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + env=watcher_env, start_new_session=True, ) diff --git a/scripts/release.py b/scripts/release.py index 16b8bd7cdb84..ff4f4bc6da7a 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -75,6 +75,7 @@ AUTHOR_MAP = { "129007007+HeLLGURD@users.noreply.github.com": "HeLLGURD", "290859878+synapsesx@users.noreply.github.com": "synapsesx", "dirtyren@users.noreply.github.com": "dirtyren", + "470766206@qq.com": "youjunxiaji", "mharris@parallel.ai": "NormallyGaussian", "roger@roger.local": "mollusk", "ted.malone@outlook.com": "temalo", diff --git a/tests/gateway/test_restart_drain.py b/tests/gateway/test_restart_drain.py index 32710fdb8979..15b948a4f79f 100644 --- a/tests/gateway/test_restart_drain.py +++ b/tests/gateway/test_restart_drain.py @@ -200,6 +200,7 @@ async def test_launch_detached_restart_command_uses_setsid(monkeypatch): monkeypatch.setattr(gateway_run.sys, "platform", "linux") monkeypatch.setattr(gateway_run, "_resolve_hermes_bin", lambda: ["/usr/bin/hermes"]) monkeypatch.setattr(gateway_run.os, "getpid", lambda: 321) + monkeypatch.setenv("_HERMES_GATEWAY", "1") monkeypatch.setattr(shutil, "which", lambda cmd: "/usr/bin/setsid" if cmd == "setsid" else None) def fake_popen(cmd, **kwargs): @@ -218,6 +219,9 @@ async def test_launch_detached_restart_command_uses_setsid(monkeypatch): assert kwargs["start_new_session"] is True assert kwargs["stdout"] is subprocess.DEVNULL assert kwargs["stderr"] is subprocess.DEVNULL + # The watcher must NOT inherit the gateway marker, or the CLI's + # self-restart loop guard refuses to run `hermes gateway restart`. + assert kwargs["env"].get("_HERMES_GATEWAY") is None def test_windows_gateway_venv_imports_add_site_packages(monkeypatch, tmp_path):