From 914d19b3a9e46b8956c009b572a42b98e110ec6f Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Fri, 3 Jul 2026 14:14:40 -0500 Subject: [PATCH] =?UTF-8?q?fix(desktop,gateway,mcp):=20post-merge=20?= =?UTF-8?q?=E2=80=94=20CI=20contract,=20review=20corrections,=20hub=20sear?= =?UTF-8?q?ch?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Post-merge follow-ups + several review rounds + a hub-search rework, folded together. Merge-scuff restores (a stale-base refactor had reverted two live-on-main fixes): - gateway: SessionStore compression-tip healing + its regression test. - desktop: messaging session/transcript polling in desktop-controller (MESSAGING_POLL / ACTIVE_MESSAGING_SESSION_POLL, refreshMessagingSessions, refreshActiveMessagingTranscript, the richer sameCronSignature) so inbound platform traffic updates live again instead of freezing until manual refresh. Profile-switch isolation (epoch/close/guard on every profile-scoped async): - Hub store clears + in-flight runHubAction bails (and swallows the post-switch 404 instead of a phantom toast); hub preview/scan/search/sources profile-scoped. - MCP: probe/auth epoch guards, dirty-draft reset, sidebar mutations blocked until config resettles AND every persist re-checks the epoch post-await; profilePending clears on config settle incl. error; logs re-key on profile. - Model settings reload on switch and epoch-guard setModelAssignment / saveMoaModels / API-key activation. - Config draft resets + cancels its autosave on switch; skill editor/archive and star-map node dialogs close on switch; openSkillEditor / star-map openEdit discard stale fetches; tool-usage analytics loads are profile-guarded/keyed. Correctness + UX: - Unique per-skill action names for hub install AND uninstall; hub/​catalog rows flip only on a clean exit_code; catalog install polls the background bootstrap to completion, reconciles the mcp.json draft (no dropped server), and fails loudly on non-zero exit; MCP catalog query keyed by profile. - /test reports needs-auth for anonymous auth:oauth servers; /auth snapshots + restores tokens on a failed re-auth and clears the full 300s callback window. - config-settings shows a retry on load failure; CodeEditor/JsonDocumentEditor go read-only while saving so edits typed mid-save aren't dropped. - Deep-link highlighter deletes its param only after a successful scroll. - Restored the PageSearchShell trailing slot → Artifacts refresh button/spinner. - /settings?tab=mcp redirect keeps server=. Progressive hub search: fan out one query per backend-searchable source (index-covered API sources stay unsearchable → no ~70-call GitHub re-hammer), merge/dedupe by trust as each lands, per-source spinner overlaid on the dimmed chip — results stream in without blocking on the slowest, no layout shift. test(web): /api/skills list carries usage + provenance (CI contract). --- apps/desktop/src/app/artifacts/index.tsx | 33 ++- .../src/app/desktop-controller-utils.ts | 17 +- apps/desktop/src/app/desktop-controller.tsx | 128 +++++++++++- apps/desktop/src/app/page-search-shell.tsx | 6 +- .../session/hooks/use-session-list-actions.ts | 1 + .../src/app/settings/config-settings.tsx | 45 +++- .../src/app/settings/credential-key-ui.tsx | 14 ++ apps/desktop/src/app/settings/index.tsx | 11 +- .../src/app/settings/model-settings.tsx | 46 ++++- .../app/settings/use-deep-link-highlight.ts | 54 +++-- apps/desktop/src/app/skills/hub.tsx | 101 +++++++-- apps/desktop/src/app/skills/index.tsx | 55 ++++- apps/desktop/src/app/skills/mcp-tab.tsx | 194 ++++++++++++++++-- .../src/app/starmap/node-context-menu.tsx | 24 ++- .../src/components/chat/code-editor.tsx | 9 + .../components/chat/json-document-editor.tsx | 1 + apps/desktop/src/hermes.ts | 6 +- apps/desktop/src/lib/media.remote.test.ts | 2 + apps/desktop/src/store/hub-actions.ts | 55 ++++- apps/desktop/src/types/hermes.ts | 3 + gateway/session.py | 60 ++++++ hermes_cli/web_server.py | 129 +++++++++--- tests/gateway/test_restart_resume_pending.py | 21 ++ .../test_session_store_runtime_stale_guard.py | 4 + tests/hermes_cli/test_web_server.py | 6 +- .../test_web_server_profile_unification.py | 35 +++- .../test_web_server_skills_profiles.py | 2 +- tools/mcp_oauth.py | 35 ++++ 28 files changed, 987 insertions(+), 110 deletions(-) diff --git a/apps/desktop/src/app/artifacts/index.tsx b/apps/desktop/src/app/artifacts/index.tsx index cd6eed12338..049e0a437f4 100644 --- a/apps/desktop/src/app/artifacts/index.tsx +++ b/apps/desktop/src/app/artifacts/index.tsx @@ -20,7 +20,8 @@ import { Tip } from '@/components/ui/tooltip' import { getSessionMessages, listAllProfileSessions } from '@/hermes' import { type Translations, useI18n } from '@/i18n' import { ExternalLink, ExternalLinkIcon, hostPathLabel, urlSlugTitleLabel, useLinkTitle } from '@/lib/external-link' -import { FileImage, FileText, FolderOpen, Link2 } from '@/lib/icons' +import { FileImage, FileText, FolderOpen, Link2, Loader2, RefreshCw } from '@/lib/icons' +import { downloadGatewayMediaFile, isRemoteGateway } from '@/lib/media' import { normalize } from '@/lib/text' import { fmtDayTime } from '@/lib/time' import { cn } from '@/lib/utils' @@ -114,7 +115,11 @@ export function ArtifactsView({ setStatusbarItemGroup: _setStatusbarItemGroup, . const [imagePage, setImagePage] = useState(1) const [filePage, setFilePage] = useState(1) + const [refreshing, setRefreshing] = useState(false) + const refreshArtifacts = useCallback(async () => { + setRefreshing(true) + try { const sessions = (await listAllProfileSessions(30, 1)).sessions const results = await Promise.allSettled(sessions.map(session => getSessionMessages(session.id, session.profile))) @@ -133,6 +138,8 @@ export function ArtifactsView({ setStatusbarItemGroup: _setStatusbarItemGroup, . } catch (err) { notifyError(err, a.failedLoad) setArtifacts([]) + } finally { + setRefreshing(false) } }, [a]) @@ -229,6 +236,16 @@ export function ArtifactsView({ setStatusbarItemGroup: _setStatusbarItemGroup, . const openArtifact = useCallback( async (href: string) => { try { + // A gateway-local file resolves to file:// in remote mode (the file + // lives on the gateway, not this disk). Opening that locally fails — + // and an OAuth remote connection has no query token to build a download + // URL. Fetch the bytes over the authenticated fs bridge instead. + if (isRemoteGateway() && /^file:/i.test(href)) { + await downloadGatewayMediaFile(href) + + return + } + if (window.hermesDesktop?.openExternal) { await window.hermesDesktop.openExternal(href) } else { @@ -265,6 +282,20 @@ export function ArtifactsView({ setStatusbarItemGroup: _setStatusbarItemGroup, . searchHidden={counts.all === 0} searchHints={searchHints} searchPlaceholder={a.search} + searchTrailingAction={ + + + + } searchValue={query} tabs={[ { id: 'all', label: a.tabAll, meta: artifacts ? counts.all : null }, diff --git a/apps/desktop/src/app/desktop-controller-utils.ts b/apps/desktop/src/app/desktop-controller-utils.ts index cb7d618e6af..5754d69ef81 100644 --- a/apps/desktop/src/app/desktop-controller-utils.ts +++ b/apps/desktop/src/app/desktop-controller-utils.ts @@ -7,5 +7,20 @@ export function sameCronSignature(a: SessionInfo[], b: SessionInfo[]): boolean { return false } - return a.every((session, i) => session.id === b[i]?.id && session.title === b[i]?.title) + return a.every((session, i) => { + const other = b[i] + + return ( + other != null && + session.id === other.id && + session._lineage_root_id === other._lineage_root_id && + session.title === other.title && + session.source === other.source && + session.profile === other.profile && + session.preview === other.preview && + session.message_count === other.message_count && + session.last_active === other.last_active && + session.ended_at === other.ended_at + ) + }) } diff --git a/apps/desktop/src/app/desktop-controller.tsx b/apps/desktop/src/app/desktop-controller.tsx index d585054aeab..4b8c249bced 100644 --- a/apps/desktop/src/app/desktop-controller.tsx +++ b/apps/desktop/src/app/desktop-controller.tsx @@ -15,9 +15,10 @@ import { cn } from '@/lib/utils' import { useSkinCommand } from '@/themes/use-skin-command' import { formatRefValue } from '../components/assistant-ui/directive-text' -import { getSessionMessages, triggerCronJob } from '../hermes' +import { getSessionMessages, type SessionMessage, triggerCronJob } from '../hermes' import { type ChatMessage, chatMessageText, preserveLocalAssistantErrors, toChatMessages } from '../lib/chat-messages' import { storedSessionIdForNotification } from '../lib/session-ids' +import { isMessagingSource } from '../lib/session-source' import { latestSessionTodos } from '../lib/todos' import { setCronFocusJobId } from '../store/cron' import { @@ -55,6 +56,7 @@ import { $freshDraftReady, $gatewayState, $messages, + $messagingSessions, $resumeExhaustedSessionId, $resumeFailedSessionId, $selectedStoredSessionId, @@ -141,6 +143,39 @@ const SkillsView = lazy(async () => ({ default: (await import('./skills')).Skill // this cadence while the app is open + visible so new runs surface promptly // instead of waiting for the next user-triggered refreshSessions(). const CRON_POLL_INTERVAL_MS = 30_000 +// Messaging-platform turns are written by the background gateway (WeChat, +// Telegram, Discord, …), not the desktop websocket that drives local chats. +// Poll the bounded messaging slice while visible so inbound platform traffic +// appears without requiring a manual refresh or route change. +const MESSAGING_POLL_INTERVAL_MS = 10_000 +const ACTIVE_MESSAGING_SESSION_POLL_INTERVAL_MS = 5_000 + +function sessionMatchesStoredId(session: { id: string; _lineage_root_id?: null | string }, id: string): boolean { + return session.id === id || session._lineage_root_id === id +} + +function hashString(hash: number, value: string): number { + let next = hash + + for (let i = 0; i < value.length; i++) { + next ^= value.charCodeAt(i) + next = Math.imul(next, 16777619) + } + + return next >>> 0 +} + +function sessionMessagesSignature(messages: SessionMessage[]): string { + let hash = 2166136261 + + for (const m of messages) { + hash = hashString(hash, m.role) + hash = hashString(hash, String(m.timestamp ?? '')) + hash = hashString(hash, typeof m.content === 'string' ? m.content : (JSON.stringify(m.content) ?? '')) + } + + return `${messages.length}:${hash}` +} export function DesktopController() { const queryClient = useQueryClient() @@ -149,6 +184,7 @@ export function DesktopController() { const busyRef = useRef(false) const creatingSessionRef = useRef(false) + const messagingTranscriptSignatureRef = useRef(new Map()) const gatewayState = useStore($gatewayState) const activeSessionId = useStore($activeSessionId) @@ -159,6 +195,7 @@ export function DesktopController() { const filePreviewTarget = useStore($filePreviewTarget) const previewTarget = useStore($previewTarget) const selectedStoredSessionId = useStore($selectedStoredSessionId) + const messagingSessions = useStore($messagingSessions) const terminalTakeover = useStore($terminalTakeover) const reviewOpen = useStore($reviewOpen) const fileBrowserOpen = useStore($fileBrowserOpen) @@ -359,6 +396,7 @@ export function DesktopController() { loadMoreSessions, loadMoreSessionsForProfile, refreshCronJobs, + refreshMessagingSessions, refreshSessions } = useSessionListActions({ profileScope }) @@ -507,6 +545,42 @@ export function DesktopController() { [activeSessionIdRef, selectedStoredSessionIdRef, updateSessionState] ) + const refreshActiveMessagingTranscript = useCallback(async () => { + const storedSessionId = selectedStoredSessionIdRef.current + const runtimeSessionId = activeSessionIdRef.current + + if (!storedSessionId || !runtimeSessionId || busyRef.current) { + return + } + + const stored = $messagingSessions.get().find(s => sessionMatchesStoredId(s, storedSessionId)) + + if (!stored || !isMessagingSource(stored.source)) { + return + } + + try { + const latest = await getSessionMessages(storedSessionId, stored.profile) + const signatureKey = `${stored.profile ?? 'default'}:${storedSessionId}` + const sig = sessionMessagesSignature(latest.messages) + + if (messagingTranscriptSignatureRef.current.get(signatureKey) === sig) { + return + } + + messagingTranscriptSignatureRef.current.set(signatureKey, sig) + const messages = toChatMessages(latest.messages) + + updateSessionState( + runtimeSessionId, + state => ({ ...state, messages: preserveLocalAssistantErrors(messages, state.messages) }), + storedSessionId + ) + } catch { + // Non-fatal: next poll or manual refresh can hydrate. + } + }, [activeSessionIdRef, busyRef, selectedStoredSessionIdRef, updateSessionState]) + const { handleGatewayEvent } = useMessageStream({ activeSessionIdRef, hydrateFromStoredSession, @@ -845,6 +919,58 @@ export function DesktopController() { } }, [gatewayState, refreshCronJobs]) + // Keep messaging-platform session lists live: inbound Telegram/WeChat/Discord + // turns are written by the gateway, not the desktop websocket, so they won't + // appear without polling. + useEffect(() => { + if (gatewayState !== 'open') { + return + } + + const tick = () => { + if (document.visibilityState === 'visible') { + void refreshMessagingSessions() + } + } + + const intervalId = window.setInterval(tick, MESSAGING_POLL_INTERVAL_MS) + document.addEventListener('visibilitychange', tick) + + return () => { + window.clearInterval(intervalId) + document.removeEventListener('visibilitychange', tick) + } + }, [gatewayState, refreshMessagingSessions]) + + // Only the open messaging transcript needs a poll — local chats are already + // live over the websocket, so arming a timer for them would just no-op every + // tick. Gate on the active session actually being a messaging source. + const activeIsMessaging = + !!selectedStoredSessionId && + isMessagingSource(messagingSessions.find(s => sessionMatchesStoredId(s, selectedStoredSessionId))?.source) + + // Keep the currently-viewed messaging transcript live. + useEffect(() => { + if (gatewayState !== 'open' || !activeIsMessaging) { + return + } + + const tick = () => { + if (document.visibilityState === 'visible') { + void refreshActiveMessagingTranscript() + } + } + + const intervalId = window.setInterval(tick, ACTIVE_MESSAGING_SESSION_POLL_INTERVAL_MS) + document.addEventListener('visibilitychange', tick) + tick() + + return () => { + window.clearInterval(intervalId) + document.removeEventListener('visibilitychange', tick) + } + }, [activeIsMessaging, gatewayState, refreshActiveMessagingTranscript]) + useEffect(() => { if (gatewayState === 'open' && !activeSessionId && freshDraftReady) { void refreshCurrentModel() diff --git a/apps/desktop/src/app/page-search-shell.tsx b/apps/desktop/src/app/page-search-shell.tsx index eee9f94cdd1..b94f28114ca 100644 --- a/apps/desktop/src/app/page-search-shell.tsx +++ b/apps/desktop/src/app/page-search-shell.tsx @@ -28,6 +28,9 @@ interface PageSearchShellProps extends React.ComponentProps<'section'> { searchValue: string /** Hide the search field when there's nothing to search (empty dataset). */ searchHidden?: boolean + /** Right-aligned control in the header's trailing cell (e.g. a refresh button) + * so mouse users get a visible affordance for the refresh hotkey. */ + searchTrailingAction?: ReactNode } function ShellTabs({ @@ -61,6 +64,7 @@ export function PageSearchShell({ searchHints, searchValue, searchHidden = false, + searchTrailingAction, ...props }: PageSearchShellProps) { const hasTabs = (tabs?.length ?? 0) > 0 @@ -100,7 +104,7 @@ export function PageSearchShell({ )} {hasTabs ? : } - +
{searchTrailingAction}
)} {filters ?
{filters}
: null} diff --git a/apps/desktop/src/app/session/hooks/use-session-list-actions.ts b/apps/desktop/src/app/session/hooks/use-session-list-actions.ts index 6c5d89e7112..6f971c3165b 100644 --- a/apps/desktop/src/app/session/hooks/use-session-list-actions.ts +++ b/apps/desktop/src/app/session/hooks/use-session-list-actions.ts @@ -225,6 +225,7 @@ export function useSessionListActions({ profileScope }: UseSessionListActionsArg loadMoreSessions, loadMoreSessionsForProfile, refreshCronJobs, + refreshMessagingSessions, refreshSessions } } diff --git a/apps/desktop/src/app/settings/config-settings.tsx b/apps/desktop/src/app/settings/config-settings.tsx index 534e279f891..6a87590b3cb 100644 --- a/apps/desktop/src/app/settings/config-settings.tsx +++ b/apps/desktop/src/app/settings/config-settings.tsx @@ -3,6 +3,7 @@ import type { ChangeEvent, ReactNode } from 'react' import { useEffect, useMemo, useRef, useState } from 'react' import { useSearchParams } from 'react-router-dom' +import { Button } from '@/components/ui/button' import { Input } from '@/components/ui/input' import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@/components/ui/select' import { Switch } from '@/components/ui/switch' @@ -14,6 +15,8 @@ import { notify, notifyError } from '@/store/notifications' import type { ConfigFieldSchema, HermesConfigRecord } from '@/types/hermes' import { setHermesConfigCache, useHermesConfigRecord } from '../hooks/use-config-record' +import { useOnProfileSwitch } from '../hooks/use-on-profile-switch' +import { PanelEmpty } from '../overlays/panel' import { CONTROL_TEXT, EMPTY_SELECT_VALUE, FIELD_DESCRIPTIONS, FIELD_LABELS, SECTIONS } from './constants' import { fieldCopyForSchemaKey } from './field-copy' @@ -226,9 +229,13 @@ export function ConfigSettings({ // from — and saved back through — the shared config cache, so edits are visible // in the MCP/model surfaces and reopening the page doesn't reload-flash. const [config, setConfig] = useState(null) - const { data: loadedConfig } = useHermesConfigRecord() + const { data: loadedConfig, isError: configLoadFailed, refetch: refetchConfig } = useHermesConfigRecord() - const { data: schemaResponse } = useQuery({ + const { + data: schemaResponse, + isError: schemaFailed, + refetch: refetchSchema + } = useQuery({ queryKey: ['hermes-config-schema'], queryFn: getHermesConfigSchema, staleTime: 5 * 60 * 1000 @@ -251,6 +258,17 @@ export function ConfigSettings({ } }, [loadedConfig]) + // A profile switch invalidates (but doesn't clear) the shared config query, so + // the local draft would otherwise keep profile A's data and autosave it into + // B. Drop the seed + draft (re-seeds from B's refetch) and zero saveVersion so + // the pending debounced autosave is cancelled by its effect cleanup. + useOnProfileSwitch(() => { + configSeeded.current = false + setConfig(null) + saveVersionRef.current = 0 + setSaveVersion(0) + }) + useEffect(() => { let cancelled = false @@ -378,6 +396,29 @@ export function ConfigSettings({ } if (!config || !schema) { + // A failed config/schema fetch must surface a retry, not spin forever. + if ((configLoadFailed && !config) || (schemaFailed && !schema)) { + return ( +
+ { + void refetchConfig() + void refetchSchema() + }} + size="sm" + > + {t.skills.refresh} + + } + icon="error" + title={c.failedLoad} + /> +
+ ) + } + // Model keeps its shape via a skeleton (its catalog fetch is the slow part); // other sections are quick config/schema reads, so a light loader is fine. if (activeSectionId === 'model') { diff --git a/apps/desktop/src/app/settings/credential-key-ui.tsx b/apps/desktop/src/app/settings/credential-key-ui.tsx index 4c93d83b0f9..e963c3c0f83 100644 --- a/apps/desktop/src/app/settings/credential-key-ui.tsx +++ b/apps/desktop/src/app/settings/credential-key-ui.tsx @@ -184,6 +184,13 @@ export function CredentialKeyCard({ onKeyDown={ expandable ? e => { + // Only the card's own focus toggles it — ignore Enter/Space + // bubbling up from the inputs/buttons inside (Enter saves a key, + // Space types a space) so keyboard editing never collapses the card. + if (e.target !== e.currentTarget) { + return + } + if (e.key === 'Enter' || e.key === ' ') { e.preventDefault() onToggle() @@ -264,6 +271,13 @@ export function ProviderKeyRows({ expanded, group, onExpand, onToggle, rowProps onKeyDown={ expandable ? e => { + // Only the card's own focus toggles it — ignore Enter/Space + // bubbling up from the inputs/buttons inside (Enter saves a key, + // Space types a space) so keyboard editing never collapses the card. + if (e.target !== e.currentTarget) { + return + } + if (e.key === 'Enter' || e.key === ' ') { e.preventDefault() onToggle() diff --git a/apps/desktop/src/app/settings/index.tsx b/apps/desktop/src/app/settings/index.tsx index b49438b3db4..9c7c4dd0b4e 100644 --- a/apps/desktop/src/app/settings/index.tsx +++ b/apps/desktop/src/app/settings/index.tsx @@ -43,10 +43,15 @@ export function SettingsView({ onClose, onConfigSaved, onMainModelChanged }: Set // MCP moved out of Settings into Capabilities (/skills?tab=mcp). Keep old // `/settings?tab=mcp` deep links working — `useRouteEnumParam` would silently - // coerce the unknown tab to the default view otherwise. + // coerce the unknown tab to the default view otherwise. Preserve `server=` so + // an old bookmark still lands on (and highlights) the selected server. useEffect(() => { - if (new URLSearchParams(search).get('tab') === 'mcp') { - navigate(`${SKILLS_ROUTE}?tab=mcp`, { replace: true }) + const params = new URLSearchParams(search) + + if (params.get('tab') === 'mcp') { + const server = params.get('server') + const suffix = server ? `&server=${encodeURIComponent(server)}` : '' + navigate(`${SKILLS_ROUTE}?tab=mcp${suffix}`, { replace: true }) } }, [navigate, search]) diff --git a/apps/desktop/src/app/settings/model-settings.tsx b/apps/desktop/src/app/settings/model-settings.tsx index e17681d8c14..01c57f9925b 100644 --- a/apps/desktop/src/app/settings/model-settings.tsx +++ b/apps/desktop/src/app/settings/model-settings.tsx @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useMemo, useState } from 'react' +import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { Button } from '@/components/ui/button' import { Input } from '@/components/ui/input' @@ -30,6 +30,7 @@ import { notifyError } from '@/store/notifications' import { startManualLocalEndpoint, startManualProviderOAuth } from '@/store/onboarding' import { invalidateHermesConfig, setHermesConfigCache, useHermesConfigRecord } from '../hooks/use-config-record' +import { useOnProfileSwitch } from '../hooks/use-on-profile-switch' import { CONTROL_TEXT } from './constants' import { getNested, setNested } from './helpers' @@ -196,7 +197,13 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) { const [apiKeyDraft, setApiKeyDraft] = useState('') const [activating, setActivating] = useState(false) + // Every profile-scoped async here captures this and bails before writing back, + // so a request in flight when the user switches profiles can't paint profile + // A's models/providers into profile B (or fire onMainModelChanged for A). + const profileEpoch = useRef(0) + const refresh = useCallback(async () => { + const epoch = profileEpoch.current setLoading(true) setError('') @@ -208,6 +215,10 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) { getMoaModels().catch(() => null) ]) + if (profileEpoch.current !== epoch) { + return + } + setMainModel({ model: modelInfo.model, provider: modelInfo.provider }) setProviders(modelOptions.providers || []) setSelectedProvider(prev => prev || modelInfo.provider) @@ -223,9 +234,13 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) { // change it server-side (aux slots), so nudge that cache to refetch. void invalidateHermesConfig() } catch (err) { - setError(err instanceof Error ? err.message : String(err)) + if (profileEpoch.current === epoch) { + setError(err instanceof Error ? err.message : String(err)) + } } finally { - setLoading(false) + if (profileEpoch.current === epoch) { + setLoading(false) + } } }, []) @@ -233,6 +248,13 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) { void refresh() }, [refresh]) + // A profile switch swaps the backend under the mounted panel — reload for the + // new profile (bumping the epoch first so any in-flight A request is discarded). + useOnProfileSwitch(() => { + profileEpoch.current += 1 + void refresh() + }) + const providerOptions = providers.length ? providers : NO_PROVIDERS // MoA reference/aggregator slots must never be the moa virtual provider — @@ -306,11 +328,17 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) { }, []) const saveMoa = useCallback(async (next: MoaConfigResponse) => { + const epoch = profileEpoch.current setApplying(true) setError('') try { const saved = await saveMoaModels(next) + + if (profileEpoch.current !== epoch) { + return + } + setMoa(saved) } catch (err) { setError(err instanceof Error ? err.message : String(err)) @@ -395,6 +423,7 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) { return } + const epoch = profileEpoch.current setActivating(true) setError('') @@ -415,6 +444,11 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) { } const options = await getGlobalModelOptions() + + if (profileEpoch.current !== epoch) { + return + } + setProviders(options.providers || []) const refreshedRow = options.providers?.find(p => p.slug === slug) const fallbackModel = refreshedRow?.models?.[0] ?? '' @@ -452,11 +486,17 @@ export function ModelSettings({ onMainModelChanged }: ModelSettingsProps) { return } + const epoch = profileEpoch.current setApplying(true) setError('') try { const result = await setModelAssignment({ model: selectedModel, provider: selectedProvider, scope: 'main' }) + + if (profileEpoch.current !== epoch) { + return + } + const provider = result.provider || selectedProvider const model = result.model || selectedModel setMainModel({ provider, model }) diff --git a/apps/desktop/src/app/settings/use-deep-link-highlight.ts b/apps/desktop/src/app/settings/use-deep-link-highlight.ts index a4cabce3a46..a7f9e2fb786 100644 --- a/apps/desktop/src/app/settings/use-deep-link-highlight.ts +++ b/apps/desktop/src/app/settings/use-deep-link-highlight.ts @@ -30,30 +30,50 @@ export function useDeepLinkHighlight({ onResolve?.(target) - // Defer a frame so async state (expansion, selection) mounts the row first. - const scrollTimeout = window.setTimeout(() => { - const element = document.getElementById(elementId(target)) + let cancelled = false + let timer = 0 - if (!element) { + // onResolve may flip view state that mounts the row a few frames later, so + // poll briefly for it and only drop the param AFTER a successful scroll — + // deleting up front would lose the deep link when the target mounts late. + let attempts = 0 + + const attempt = () => { + if (cancelled) { return } - element.scrollIntoView({ behavior: 'smooth', block }) - element.classList.add('setting-field-highlight') - window.setTimeout(() => element.classList.remove('setting-field-highlight'), 1600) - }, 80) + const element = document.getElementById(elementId(target)) - setSearchParams( - previous => { - const next = new URLSearchParams(previous) - next.delete(param) + if (element) { + element.scrollIntoView({ behavior: 'smooth', block }) + element.classList.add('setting-field-highlight') + window.setTimeout(() => element.classList.remove('setting-field-highlight'), 1600) - return next - }, - { replace: true } - ) + setSearchParams( + previous => { + const next = new URLSearchParams(previous) + next.delete(param) - return () => window.clearTimeout(scrollTimeout) + return next + }, + { replace: true } + ) + + return + } + + if (attempts++ < 20) { + timer = window.setTimeout(attempt, 80) + } + } + + timer = window.setTimeout(attempt, 80) + + return () => { + cancelled = true + window.clearTimeout(timer) + } }, [block, elementId, onResolve, param, ready, setSearchParams, target]) return target diff --git a/apps/desktop/src/app/skills/hub.tsx b/apps/desktop/src/app/skills/hub.tsx index 503e5feb10f..a7ed3ca8fe5 100644 --- a/apps/desktop/src/app/skills/hub.tsx +++ b/apps/desktop/src/app/skills/hub.tsx @@ -1,6 +1,6 @@ import { useStore } from '@nanostores/react' -import { useQuery } from '@tanstack/react-query' -import { useCallback, useState } from 'react' +import { useQueries, useQuery } from '@tanstack/react-query' +import { useCallback, useMemo, useState } from 'react' import { useDebounced } from '@/app/hooks/use-debounced' import { DetailPane } from '@/app/master-detail' @@ -42,6 +42,10 @@ import { } from '@/store/hub-actions' import { notify, notifyError } from '@/store/notifications' +// Dedup rank when the same skill surfaces from multiple sources — higher trust +// wins. Mirrors the backend's unified_search `_TRUST_RANK`. +const TRUST_RANK: Record = { builtin: 2, trusted: 1, community: 0 } + function trustTone(level: string): string { switch (level) { case 'builtin': @@ -149,14 +153,26 @@ export function SkillsHub({ query }: SkillsHubProps) { }) // Debounced hub search, keyed on the settled query so RQ dedupes/caches per - // term and cancels stale terms for us (no hand-rolled sequence guard). + // term and abandons stale terms for us (no hand-rolled sequence guard). const term = useDebounced(query.trim(), 350) - const searchQuery = useQuery({ - queryKey: ['skill-hub-search', term], - queryFn: () => searchSkillsHub(term), - enabled: term.length > 0, - staleTime: 60_000 + // Progressive per-source search: one query per source the backend says is + // worth hitting individually (it marks index-covered API sources unsearchable + // so we don't re-hammer ~70 GitHub calls). Each resolves independently, so the + // list fills in as sources return instead of blocking on the slowest one, and + // each source shows its own spinner. Stale terms key out and are abandoned. + const searchableSources = useMemo( + () => (sourcesQuery.data?.sources ?? []).filter(source => source.searchable !== false), + [sourcesQuery.data] + ) + + const sourceSearches = useQueries({ + queries: searchableSources.map(source => ({ + queryKey: ['skill-hub-search', term, source.id], + queryFn: () => searchSkillsHub(term, source.id), + enabled: term.length > 0, + staleTime: 60_000 + })) }) // Per-item action lifecycle + log live in the store (store/hub-actions): each @@ -211,21 +227,57 @@ export function SkillsHub({ query }: SkillsHubProps) { setScan(null) }, []) + // Per-source progress, keyed by source id (drives the connected-hub chips' + // spinner/degraded tint while a search is streaming in). + const searchStateById = new Map() + searchableSources.forEach((source, i) => { + const q = sourceSearches[i] + searchStateById.set(source.id, { failed: q.isError, fetching: term.length > 0 && q.isFetching }) + }) + + // Merge every source's results, deduped by identifier preferring higher trust + // (mirrors the backend's unified_search rank). Recomputes as each source lands. + const results = useMemo(() => { + const seen = new Map() + + for (const q of sourceSearches) { + for (const r of q.data?.results ?? []) { + const prev = seen.get(r.identifier) + + if (!prev || (TRUST_RANK[r.trust_level] ?? 0) > (TRUST_RANK[prev.trust_level] ?? 0)) { + seen.set(r.identifier, r) + } + } + } + + return [...seen.values()].sort( + (a, b) => (TRUST_RANK[b.trust_level] ?? 0) - (TRUST_RANK[a.trust_level] ?? 0) || a.name.localeCompare(b.name) + ) + }, [sourceSearches]) + // Installed map: sources seeds it, search results patch it (a term can surface // installs the sources list didn't feature); the optimistic override wins so a // just-(un)installed row reflects its own outcome without the refetch race. - const installed = { ...(sourcesQuery.data?.installed ?? {}), ...(searchQuery.data?.installed ?? {}) } + const installed = { ...(sourcesQuery.data?.installed ?? {}) } + + for (const q of sourceSearches) { + Object.assign(installed, q.data?.installed ?? {}) + } + const isInstalled = (identifier: string) => overrides[identifier] ?? Boolean(installed[identifier]) const sources = sourcesQuery.data?.sources ?? [] const featured = sourcesQuery.data?.featured ?? [] - const results = searchQuery.data?.results ?? [] - const timedOut = searchQuery.data?.timed_out ?? [] - const searching = term.length > 0 && searchQuery.isFetching - const searched = term.length > 0 && searchQuery.isSuccess + // Still fetching from at least one source; "done" only once every source has + // settled (so "No results" doesn't flash while slower sources are still in). + const anyFetching = term.length > 0 && sourceSearches.some(q => q.isFetching) + const searched = term.length > 0 && sourceSearches.length > 0 && sourceSearches.every(q => !q.isFetching) const showLanding = term.length === 0 const listed = showLanding ? featured : results + // Only block the whole pane on the first sources landing; after that results + // stream in progressively while a subtle footer shows more are coming. + const searching = anyFetching && results.length === 0 const hasInstalled = Object.keys(installed).length > 0 return ( @@ -237,17 +289,28 @@ export function SkillsHub({ query }: SkillsHubProps) { {sourcesQuery.isLoading ? null : sources.map(source => { - const degraded = source.available === false || source.rate_limited === true + const state = searchStateById.get(source.id) + const degraded = source.available === false || source.rate_limited === true || state?.failed + const fetching = state?.fetching ?? false return ( 0 && !fetching && !state?.failed && 'opacity-55' )} key={source.id} > - {source.label} + {/* Spinner overlays the (dimmed) label rather than pushing it, + so a chip never resizes as its search starts/finishes. */} + {source.label} + {fetching && ( + + + + )} ) })} @@ -259,8 +322,8 @@ export function SkillsHub({ query }: SkillsHubProps) { {listed.length > 0 && (
- {searched ? h.resultCount(results.length, null) : h.featured} - {timedOut.length > 0 && {h.timedOut(timedOut.join(', '))}} + {term.length > 0 ? h.resultCount(results.length, null) : h.featured} + {anyFetching && results.length > 0 && {h.searching}} {hasInstalled && ( diff --git a/apps/desktop/src/app/skills/index.tsx b/apps/desktop/src/app/skills/index.tsx index 83599edf93f..845a15f7c14 100644 --- a/apps/desktop/src/app/skills/index.tsx +++ b/apps/desktop/src/app/skills/index.tsx @@ -1,7 +1,7 @@ import { useStore } from '@nanostores/react' import { useQuery } from '@tanstack/react-query' import type * as React from 'react' -import { useCallback, useEffect, useMemo, useState } from 'react' +import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { ArchiveSkillConfirmDialog } from '@/app/learning/archive-skill-confirm-dialog' import { CodeEditor } from '@/components/chat/code-editor' @@ -88,7 +88,12 @@ async function loadToolCalls(force = false): Promise> { const analytics = await getUsageAnalytics(365) const value = Object.fromEntries((analytics.tools ?? []).map(e => [e.tool, e.count])) - toolCallsCache.set(key, { at: Date.now(), value }) + + // Only cache if the active profile hasn't changed during the request — else a + // switch mid-flight would file this result under the wrong profile's key. + if (normalizeProfileKey($activeGatewayProfile.get()) === key) { + toolCallsCache.set(key, { at: Date.now(), value }) + } return value } @@ -206,6 +211,9 @@ export function SkillsView({ setStatusbarItemGroup: _setStatusbarItemGroup, ...p // tool name -> call count over the analytics window. null = still loading // (badges show skeletons); {} = loaded empty / unavailable backend. const [toolCalls, setToolCalls] = useState | null>(null) + // Bumped on profile switch so a slow analytics load from profile A can't set + // toolCalls after the user moved to B. + const toolCallsEpoch = useRef(0) const skillsSortDesc = useStore($skillsSortDesc) const toolsetsSortDesc = useStore($toolsetsSortDesc) const [bulkBusy, setBulkBusy] = useState(false) @@ -220,11 +228,14 @@ export function SkillsView({ setStatusbarItemGroup: _setStatusbarItemGroup, ...p // An explicit refresh is the one time we bypass the analytics TTL — but // only if the badges are already on screen; otherwise let the lazy load - // pick it up when Toolsets is first shown. + // pick it up when Toolsets is first shown. Guard the async set against a + // profile switch landing before it resolves. if (toolCallsCache.size > 0) { + const epoch = toolCallsEpoch.current + loadToolCalls(true) - .then(setToolCalls) - .catch(() => setToolCalls({})) + .then(value => toolCallsEpoch.current === epoch && setToolCalls(value)) + .catch(() => toolCallsEpoch.current === epoch && setToolCalls({})) } }, []) @@ -244,10 +255,15 @@ export function SkillsView({ setStatusbarItemGroup: _setStatusbarItemGroup, ...p } let cancelled = false + // Guard the setter by epoch too: when toolCalls is already null at switch + // time, setToolCalls(null) is a no-op so this effect never re-runs to flip + // `cancelled` — the epoch check catches that gap. + const epoch = toolCallsEpoch.current + const live = () => !cancelled && toolCallsEpoch.current === epoch loadToolCalls() - .then(value => !cancelled && setToolCalls(value)) - .catch(() => !cancelled && setToolCalls({})) + .then(value => live() && setToolCalls(value)) + .catch(() => live() && setToolCalls({})) return () => void (cancelled = true) }, [mode, toolCalls]) @@ -256,7 +272,10 @@ export function SkillsView({ setStatusbarItemGroup: _setStatusbarItemGroup, ...p // toolCalls state isn't — leaving it non-null would keep the lazy effect from // ever re-running, so badges/sort would show the previous profile's counts. // Reset to null so the next Toolsets view reloads for the active profile. - useOnProfileSwitch(() => setToolCalls(null)) + useOnProfileSwitch(() => { + toolCallsEpoch.current += 1 + setToolCalls(null) + }) const visibleSkills = useMemo( () => (skills ? filteredSkills(skills, query, skillsSortDesc) : []), @@ -444,10 +463,30 @@ export function SkillsView({ setStatusbarItemGroup: _setStatusbarItemGroup, ...p const [skillDraft, setSkillDraft] = useState('') const [skillSaving, setSkillSaving] = useState(false) const [archiveTarget, setArchiveTarget] = useState(null) + // Bumped on profile switch so an in-flight openSkillEditor fetch from profile + // A can't reopen the editor with A's content after switching to B. + const skillEditorEpoch = useRef(0) + + // A profile switch swaps the backend under the open editor/archive dialog — + // their targets belong to profile A, so a save/archive would hit B. Drop them + // so nothing edits or archives against the newly active profile. + useOnProfileSwitch(() => { + skillEditorEpoch.current += 1 + setSkillEditor(null) + setSkillDraft('') + setArchiveTarget(null) + }) const openSkillEditor = async (name: string) => { + const epoch = skillEditorEpoch.current + try { const node = await getLearningNode(name) + + if (skillEditorEpoch.current !== epoch) { + return + } + setSkillEditor({ content: node.content, name }) setSkillDraft(node.content) } catch (err) { diff --git a/apps/desktop/src/app/skills/mcp-tab.tsx b/apps/desktop/src/app/skills/mcp-tab.tsx index 7d43fddb74e..dca667ff2dc 100644 --- a/apps/desktop/src/app/skills/mcp-tab.tsx +++ b/apps/desktop/src/app/skills/mcp-tab.tsx @@ -26,6 +26,7 @@ import { Switch } from '@/components/ui/switch' import { TextTab } from '@/components/ui/text-tab' import { authMcpServer, + getActionStatus, getLogs, getMcpCatalog, type HermesGateway, @@ -43,7 +44,7 @@ import { $activeGatewayProfile, normalizeProfileKey } from '@/store/profile' import { $activeSessionId } from '@/store/session' import type { HermesConfigRecord } from '@/types/hermes' -import { invalidateHermesConfig, setHermesConfigCache, useHermesConfigRecord } from '../hooks/use-config-record' +import { setHermesConfigCache, useHermesConfigRecord } from '../hooks/use-config-record' import { useOnProfileSwitch } from '../hooks/use-on-profile-switch' import { DetailPane, ICON_BUTTON, MASTER_DETAIL_WIDE_COLS } from '../master-detail' import { PanelAddButton, PanelEmpty } from '../overlays/panel' @@ -349,16 +350,29 @@ export function McpTab({ gateway }: { gateway: HermesGateway | null }) { isLoading: configLoading, isError: configFailed, error: configError, - refetch: refetchConfig + refetch: refetchConfig, + dataUpdatedAt: configUpdatedAt, + errorUpdatedAt: configErroredAt } = useHermesConfigRecord() const setConfig = setHermesConfigCache + // True from a profile switch until the config query resettles for the new + // profile. Until then `config` (and thus `servers`) still holds profile A's + // data, so any persist would write A's server list into B — block mutations. + const [profilePending, setProfilePending] = useState(false) + const staleConfigStamp = useRef(null) + const staleErrorStamp = useRef(null) + const [saving, setSaving] = useState(false) const [probes, setProbes] = useState>({}) const probesRef = useRef(probes) probesRef.current = probes + // Blocks the browser until an OAuth flow lands a token; also reset on profile + // switch, so declared up here alongside the other per-profile view state. + const [authing, setAuthing] = useState(null) + // Master document draft. `docVersion` remounts the editor when the draft is // regenerated programmatically (list-side mutations); `dirty` guards user // edits from being clobbered by those regenerations. @@ -399,7 +413,15 @@ export function McpTab({ gateway }: { gateway: HermesGateway | null }) { // install from. Both share one cached catalog fetch (also feeds description // enrichment below), so switching between them never re-requests. const [leftView, setLeftView] = useState<'catalog' | 'servers'>('servers') - const catalogQuery = useQuery({ queryKey: MCP_CATALOG_KEY, queryFn: getMcpCatalog, staleTime: 5 * 60_000 }) + + // Key by active profile — installed/enabled badges are per-profile, so sharing + // one cache across profiles would flash the previous profile's state on switch. + const catalogQuery = useQuery({ + queryKey: [...MCP_CATALOG_KEY, normalizeProfileKey(useStore($activeGatewayProfile))], + queryFn: getMcpCatalog, + staleTime: 5 * 60_000 + }) + const catalog = catalogQuery.data?.entries ?? [] const descriptionFor = (serverName: string, server: Record): null | string => { @@ -444,16 +466,48 @@ export function McpTab({ gateway }: { gateway: HermesGateway | null }) { } }, [config]) + // Bumped on every profile switch. Async probe/auth completions capture the + // epoch at call time and bail if it changed, so a slow profile-A request can't + // write its result into profile B's state after the user switched. + const profileEpoch = useRef(0) + // A profile switch invalidates the config query (see store/profile.ts), which - // refetches the new backend's mcp.json. Reset per-profile view state so the - // draft reseeds for the new profile and the old profile's probes don't linger - // (the probe cache is already profile-keyed, so this just forces a re-probe). + // refetches the new backend's mcp.json. Reset ALL per-profile view state — the + // draft (incl. a dirty one, so profile A's edits can't be saved into B), its + // seed latch, probes, and cursor — so everything reseeds for the new profile. + // The probe cache is already profile-keyed, so this just forces a re-probe. useOnProfileSwitch(() => { + profileEpoch.current += 1 draftSeeded.current = false setProbes({}) setCursor(0) + setAuthing(null) + setDirty(false) + setDraft('') + setDocVersion(version => version + 1) + // Mark stale until the config query replaces profile A's data — guards + // sidebar mutations from persisting A's server list into B mid-refetch. + staleConfigStamp.current = configUpdatedAt + staleErrorStamp.current = configErroredAt + setProfilePending(true) }) + // Clear once the config query settles for the new profile: dataUpdatedAt bumps + // on a fresh success, errorUpdatedAt on a fresh failure. Releasing on error too + // means a failed refetch surfaces the retry UI instead of leaving mutations + // silently no-op forever. + useEffect(() => { + if ( + profilePending && + staleConfigStamp.current !== null && + (configUpdatedAt !== staleConfigStamp.current || configErroredAt !== staleErrorStamp.current) + ) { + setProfilePending(false) + staleConfigStamp.current = null + staleErrorStamp.current = null + } + }, [profilePending, configUpdatedAt, configErroredAt]) + useDeepLinkHighlight({ block: 'nearest', elementId: serverName => `mcp-server-${serverName}`, @@ -463,14 +517,25 @@ export function McpTab({ gateway }: { gateway: HermesGateway | null }) { }) const runProbe = async (serverName: string) => { + const epoch = profileEpoch.current const key = probeKey(serverName, servers[serverName]) setProbes(current => ({ ...current, [serverName]: 'probing' })) try { const result = await testMcpServer(serverName) + + // Drop the result if the profile changed mid-probe — it belongs to A. + if (profileEpoch.current !== epoch) { + return + } + probeCache.set(key, { at: Date.now(), result }) setProbes(current => ({ ...current, [serverName]: result })) } catch (err) { + if (profileEpoch.current !== epoch) { + return + } + const result = { ok: false, error: err instanceof Error ? err.message : String(err), tools: [] } probeCache.set(key, { at: Date.now(), result }) setProbes(current => ({ ...current, [serverName]: result })) @@ -480,14 +545,19 @@ export function McpTab({ gateway }: { gateway: HermesGateway | null }) { // First-class OAuth: opens the system browser, blocks until the flow lands a // token (verified on disk — a friendly tools/list is not proof), then the // auth result doubles as the probe (it carries the tool list). - const [authing, setAuthing] = useState(null) - const authenticate = async (serverName: string) => { + const epoch = profileEpoch.current setAuthing(serverName) setProbes(current => ({ ...current, [serverName]: 'probing' })) try { const result = await authMcpServer(serverName) + + // Bail if the user switched profiles mid-flow — this result is profile A's. + if (profileEpoch.current !== epoch) { + return + } + setProbes(current => ({ ...current, [serverName]: result })) // Cache under the POST-auth fingerprint (auth: oauth) on success — that's // the config the mount effect will read back, so it hits this entry. @@ -516,13 +586,19 @@ export function McpTab({ gateway }: { gateway: HermesGateway | null }) { notifyError(new Error(result.error), serverName) } } catch (err) { + if (profileEpoch.current !== epoch) { + return + } + setProbes(current => ({ ...current, [serverName]: { ok: false, error: err instanceof Error ? err.message : String(err), tools: [] } })) notifyError(err, serverName) } finally { - setAuthing(null) + if (profileEpoch.current === epoch) { + setAuthing(null) + } } } @@ -563,18 +639,41 @@ export function McpTab({ gateway }: { gateway: HermesGateway | null }) { // Whole-map replace (NOT saveHermesConfig, which deep-merges and so can never // delete a server, drop `enabled: false`, or remove a nested field). Only // after the replace lands do we write the cache through + reload live sessions. - const persist = async (nextServers: McpServers) => { + // Returns false when the profile switched mid-save: the write hit profile A's + // backend (correct), but the client-side cache/editor now belong to B, so the + // caller must skip its post-await writes. + const persist = async (nextServers: McpServers): Promise => { + const epoch = profileEpoch.current await saveMcpServers(nextServers) + + if (profileEpoch.current !== epoch) { + return false + } + setConfig(current => ({ ...current, mcp_servers: nextServers })) void silentReload() + + return true } // A catalog install wrote a new server into config.yaml on the backend — - // refresh both the config (so the fleet + editor pick it up) and the catalog - // (installed state), then reload live sessions. - const onCatalogInstalled = () => { - void invalidateHermesConfig() + // refresh the catalog (installed state) and the config, then RECONCILE THE + // EDITOR DRAFT with the fresh servers. Without this a dirty draft (or even a + // clean one the seed never refreshes) would omit the new server, and the next + // whole-map Save would silently drop it. + const onCatalogInstalled = async () => { void catalogQuery.refetch() + const { data } = await refetchConfig() + const nextServers = getServers(data ?? null) + + if (dirty) { + // Keep the user's in-progress edits (doc wins), add any server the install + // introduced that the draft doesn't have yet. + patchDraft(doc => ({ ...nextServers, ...doc })) + } else { + resetDraft(nextServers) + } + void silentReload() } @@ -591,8 +690,14 @@ export function McpTab({ gateway }: { gateway: HermesGateway | null }) { } const toggleServer = async (serverName: string, enabled: boolean) => { + if (profilePending) { + return + } + try { - await persist({ ...servers, [serverName]: withEnabled(servers[serverName], enabled) }) + if (!(await persist({ ...servers, [serverName]: withEnabled(servers[serverName], enabled) }))) { + return + } if (dirty) { patchDraft(doc => (doc[serverName] ? { ...doc, [serverName]: withEnabled(doc[serverName], enabled) } : doc)) @@ -615,14 +720,16 @@ export function McpTab({ gateway }: { gateway: HermesGateway | null }) { const toggleTool = async (serverName: string, toolName: string) => { const base = servers[serverName] - if (!base) { + if (!base || profilePending) { return } const next = toggleToolInServer(base, toolName) try { - await persist({ ...servers, [serverName]: next }) + if (!(await persist({ ...servers, [serverName]: next }))) { + return + } if (dirty) { patchDraft(doc => @@ -637,13 +744,19 @@ export function McpTab({ gateway }: { gateway: HermesGateway | null }) { } const removeServer = async (serverName: string) => { + if (profilePending) { + return + } + setSaving(true) try { const next = { ...servers } delete next[serverName] - await persist(next) + if (!(await persist(next))) { + return + } if (dirty) { patchDraft(doc => { @@ -667,6 +780,10 @@ export function McpTab({ gateway }: { gateway: HermesGateway | null }) { // "+" seeds a starter entry into the document (unique key) and marks it // dirty — naming happens in the editor, like every other mcp.json. const addServer = () => { + if (profilePending) { + return + } + let base: McpServers try { @@ -698,6 +815,10 @@ export function McpTab({ gateway }: { gateway: HermesGateway | null }) { } const saveDoc = async () => { + if (profilePending) { + return + } + let entries: McpServers try { @@ -713,7 +834,10 @@ export function McpTab({ gateway }: { gateway: HermesGateway | null }) { const prevServers = servers try { - await persist(entries) + if (!(await persist(entries))) { + return + } + resetDraft(entries) // Keep only probes for servers that survived AND kept the same config; // removed OR edited entries drop their probe so the mount effect re-probes @@ -1210,7 +1334,28 @@ function McpCatalog({ setInstalling(entry.name) try { - await installMcpCatalogEntry(entry.name, draft) + const res = await installMcpCatalogEntry(entry.name, draft) + + // Git-backed entries clone in the background — keep the row busy and poll + // the action to completion before refetching / re-enabling, so a re-click + // can't spawn a second install over the first's tracked process. A non-zero + // exit is a real failure — surface it instead of a false success. + if (res.background && res.action) { + for (;;) { + const status = await getActionStatus(res.action, 1) + + if (!status.running) { + if (status.exit_code !== 0) { + throw new Error(m.catalogInstallFailed(entry.name)) + } + + break + } + + await new Promise(resolve => setTimeout(resolve, CATALOG_INSTALL_POLL_MS)) + } + } + notify({ kind: 'success', title: m.catalogInstallStarted(entry.name), message: '' }) setEnvOpenFor(null) onInstalled() @@ -1307,6 +1452,9 @@ function McpCatalog({ const LOG_POLL_MS = 2000 +// Cadence for polling a background (git-bootstrap) catalog install to completion. +const CATALOG_INSTALL_POLL_MS = 1500 + const STDIO_MARKER_RE = /^===== \[.*\] starting MCP server '(.+)' =====$/ // Keep only the stdio-log sections belonging to one server. The shared file @@ -1337,6 +1485,10 @@ function filterStdioSections(lines: string[], server: string): string[] { // surface: CodeCardBody typography + the floating hover-reveal copy button. function McpLogs({ server, source }: { server: null | string; source: 'stdio' | 'agent' }) { const [lines, setLines] = useState(null) + // A profile switch reroutes getLogs to the new backend; keying the effect on + // the active profile tears down the old poll (its `cancelled` flag blocks a + // late setLines) so profile A's logs never flash in B. + const activeProfile = useStore($activeGatewayProfile) useEffect(() => { let cancelled = false @@ -1364,7 +1516,7 @@ function McpLogs({ server, source }: { server: null | string; source: 'stdio' | cancelled = true window.clearInterval(timer) } - }, [server, source]) + }, [server, source, activeProfile]) // TODO(i18n): literal until the UX settles. return diff --git a/apps/desktop/src/app/starmap/node-context-menu.tsx b/apps/desktop/src/app/starmap/node-context-menu.tsx index daf4b172973..7a5e75b32f4 100644 --- a/apps/desktop/src/app/starmap/node-context-menu.tsx +++ b/apps/desktop/src/app/starmap/node-context-menu.tsx @@ -1,4 +1,4 @@ -import { useState } from 'react' +import { useRef, useState } from 'react' import { ArchiveSkillConfirmDialog, fireOptimistic } from '@/app/learning/archive-skill-confirm-dialog' import { CodeEditor } from '@/components/chat/code-editor' @@ -9,6 +9,8 @@ import { deleteLearningNode, editLearningNode, getLearningNode } from '@/hermes' import { notifyError } from '@/store/notifications' import { evictStarmapNode, loadStarmapGraph } from '@/store/starmap' +import { useOnProfileSwitch } from '../hooks/use-on-profile-switch' + export interface NodeMenuTarget { id: string kind: 'memory' | 'skill' @@ -37,6 +39,20 @@ export function NodeContextMenu({ onClose, onNodeRemoved, target }: NodeContextM const [saving, setSaving] = useState(false) const [error, setError] = useState(null) + // Bumped on profile switch so an in-flight openEdit fetch from profile A can't + // reopen the editor with A's node content after switching to B. + const editEpoch = useRef(0) + + // A profile switch swaps the backend under an open edit/delete dialog — its + // node id belongs to the previous profile, so a Save/Delete after the switch + // would hit the newly active profile. Close everything on switch. + useOnProfileSwitch(() => { + editEpoch.current += 1 + setEditing(null) + setDeleting(null) + setError(null) + }) + const noun = target?.kind === 'memory' ? 'memory' : 'skill' const openEdit = async () => { @@ -44,11 +60,17 @@ export function NodeContextMenu({ onClose, onNodeRemoved, target }: NodeContextM return } + const epoch = editEpoch.current setLoading(true) setError(null) try { const detail = await getLearningNode(target.id) + + if (editEpoch.current !== epoch) { + return + } + setEditing({ content: detail.content, id: target.id, label: target.label }) onClose() } catch (e) { diff --git a/apps/desktop/src/components/chat/code-editor.tsx b/apps/desktop/src/components/chat/code-editor.tsx index cec7c902c52..0059ed62163 100644 --- a/apps/desktop/src/components/chat/code-editor.tsx +++ b/apps/desktop/src/components/chat/code-editor.tsx @@ -40,6 +40,8 @@ export interface CodeEditorApi { interface CodeEditorProps { apiRef?: RefObject className?: string + /** Read-only: block edits (e.g. while a save is in flight) without unmounting. */ + disabled?: boolean /** Mod-Shift-F + `apiRef.formatJson()`. In-memory JSON docs only. */ formatJson?: boolean /** @@ -175,6 +177,7 @@ const FRAMED_THEME = EditorView.theme({ export function CodeEditor({ apiRef, className, + disabled = false, formatJson = false, framed = false, filePath, @@ -192,6 +195,7 @@ export function CodeEditor({ const languageConf = useRef(new Compartment()) const themeConf = useRef(new Compartment()) const highlightConf = useRef(new Compartment()) + const editableConf = useRef(new Compartment()) const onCancelRef = useRef(onCancel) const onChangeRef = useRef(onChange) const onCursorChangeRef = useRef(onCursorChange) @@ -262,6 +266,7 @@ export function CodeEditor({ languageConf.current.of([]), themeConf.current.of(githubEditorTheme(isDark)), highlightConf.current.of([]), + editableConf.current.of(EditorState.readOnly.of(disabled)), EditorView.updateListener.of(update => { if (update.docChanged) { onChangeRef.current(update.state.doc.toString()) @@ -359,6 +364,10 @@ export function CodeEditor({ }) }, [highlightFrom, highlightTo]) + useEffect(() => { + viewRef.current?.dispatch({ effects: editableConf.current.reconfigure(EditorState.readOnly.of(disabled)) }) + }, [disabled]) + if (!framed) { return
} diff --git a/apps/desktop/src/components/chat/json-document-editor.tsx b/apps/desktop/src/components/chat/json-document-editor.tsx index f22afc24fbe..cd4493e4db6 100644 --- a/apps/desktop/src/components/chat/json-document-editor.tsx +++ b/apps/desktop/src/components/chat/json-document-editor.tsx @@ -80,6 +80,7 @@ export function JsonDocumentEditor({
{ return window.hermesDesktop.api({ + ...profileScoped(), path: `/api/skills/hub/preview?identifier=${encodeURIComponent(identifier)}`, timeoutMs: HUB_REQUEST_TIMEOUT_MS }) @@ -1035,6 +1036,7 @@ export function previewSkillHub(identifier: string): Promise { export function scanSkillHub(identifier: string): Promise { return window.hermesDesktop.api({ + ...profileScoped(), path: `/api/skills/hub/scan?identifier=${encodeURIComponent(identifier)}`, timeoutMs: HUB_REQUEST_TIMEOUT_MS }) @@ -1099,8 +1101,8 @@ export function getMcpCatalog(): Promise { export function installMcpCatalogEntry( name: string, env: Record = {} -): Promise<{ ok: boolean; name?: string; pid?: number; action?: string }> { - return window.hermesDesktop.api<{ ok: boolean; name?: string; pid?: number; action?: string }>({ +): Promise<{ ok: boolean; name?: string; pid?: number; action?: string; background?: boolean }> { + return window.hermesDesktop.api<{ ok: boolean; name?: string; pid?: number; action?: string; background?: boolean }>({ ...profileScoped(), path: '/api/mcp/catalog/install', method: 'POST', diff --git a/apps/desktop/src/lib/media.remote.test.ts b/apps/desktop/src/lib/media.remote.test.ts index 9bb47ce6808..e10ca5f59ec 100644 --- a/apps/desktop/src/lib/media.remote.test.ts +++ b/apps/desktop/src/lib/media.remote.test.ts @@ -1,3 +1,5 @@ +// @vitest-environment jsdom +// downloadGatewayMediaFile drives an click, so these need a DOM. import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { $connection } from '@/store/session' diff --git a/apps/desktop/src/store/hub-actions.ts b/apps/desktop/src/store/hub-actions.ts index 7d5db1b87ea..3483cb589bd 100644 --- a/apps/desktop/src/store/hub-actions.ts +++ b/apps/desktop/src/store/hub-actions.ts @@ -3,6 +3,7 @@ import { atom, map } from 'nanostores' import { getActionStatus, installSkillFromHub, uninstallSkillFromHub, updateSkillsFromHub } from '@/hermes' import { queryClient } from '@/lib/query-client' import { upsertDesktopActionTask } from '@/store/activity' +import { $activeGatewayProfile, normalizeProfileKey } from '@/store/profile' const POLL_MS = 1200 @@ -35,29 +36,67 @@ export const $hubInstalledOverride = map>({} // The key whose log the bottom pane currently tails (the latest-started action). export const $hubActiveLog = atom(null) +// Hub action state is per-profile: a profile switch must drop every in-flight +// entry, optimistic override, and active log so profile A's install/uninstall +// state can never render (or be polled) in profile B. Cleared at the source so +// it holds regardless of whether the Hub view is mounted. The epoch bumps on +// every switch; a runHubAction() started before the switch captures it and bails +// before any store write once it no longer matches (so an A action finishing +// after the clear can't repopulate B). +let _hubProfile: null | string = null +let _hubEpoch = 0 + +$activeGatewayProfile.subscribe(value => { + const key = normalizeProfileKey(value) + + if (_hubProfile !== null && _hubProfile !== key) { + _hubEpoch += 1 + $hubActions.set({}) + $hubInstalledOverride.set({}) + $hubActiveLog.set(null) + } + + _hubProfile = key +}) + // One self-contained task: spawn → tail its own action log into the store → // mark resolved. Concurrency-safe: state is per-key, so parallel installs never // stomp each other, and the sources query is invalidated once at the end. async function runHubAction(key: string, kind: HubActionKind, spawn: () => Promise<{ name: string }>): Promise { + const epoch = _hubEpoch + const switched = () => _hubEpoch !== epoch + $hubActions.setKey(key, { kind, running: true, lines: [] }) $hubActiveLog.set(key) try { const started = await spawn() + let exitCode: number | null = null for (;;) { const status = await getActionStatus(started.name, 200) + + // Profile switched mid-flight: the store was cleared for the new profile, + // so drop this A-profile result instead of writing it back into B. + if (switched()) { + return + } + upsertDesktopActionTask(status) $hubActions.setKey(key, { kind, running: status.running, lines: status.lines }) if (!status.running) { + exitCode = status.exit_code + break } await new Promise(resolve => setTimeout(resolve, POLL_MS)) } - if (key !== UPDATE_ALL_KEY) { + // Only flip the row on a clean exit — a failed install/uninstall must not + // render as installed/removed. + if (key !== UPDATE_ALL_KEY && exitCode === 0) { $hubInstalledOverride.setKey(key, kind !== 'uninstall') } @@ -65,10 +104,22 @@ async function runHubAction(key: string, kind: HubActionKind, spawn: () => Promi // (un)install adds/removes a skill, so its count/rows must update too. void queryClient.invalidateQueries({ queryKey: HUB_SOURCES_KEY }) void queryClient.invalidateQueries({ queryKey: SKILLS_LIST_KEY }) + } catch (err) { + // A profile switch points the next poll at the new backend, which 404s the + // old action name — that's an abandonment, not a failure, so swallow it + // instead of letting the caller toast a phantom error. Real (same-profile) + // failures still propagate. + if (switched()) { + return + } + + throw err } finally { + // Skip the running=false write after a switch — it would re-add the key the + // profile-switch clear just dropped. const current = $hubActions.get()[key] - if (current) { + if (current && !switched()) { $hubActions.setKey(key, { ...current, running: false }) } } diff --git a/apps/desktop/src/types/hermes.ts b/apps/desktop/src/types/hermes.ts index 946c2d327b9..63ac7f9de40 100644 --- a/apps/desktop/src/types/hermes.ts +++ b/apps/desktop/src/types/hermes.ts @@ -906,6 +906,9 @@ export interface SkillHubSource { label: string available?: boolean rate_limited?: boolean + // False when the centralized index already covers this source, so the UI's + // per-source search fan-out skips it (avoids redundant external API calls). + searchable?: boolean } /** A searchable/installable hub skill from `GET /api/skills/hub/search`. */ diff --git a/gateway/session.py b/gateway/session.py index 67bd84aaa16..5ced2d8aa78 100644 --- a/gateway/session.py +++ b/gateway/session.py @@ -1369,6 +1369,48 @@ class SessionStore: return None + def _compression_tip_for_session_id(self, session_id: Optional[str]) -> Optional[str]: + """Return the latest compression continuation for *session_id*. + + When an agent compresses context mid-turn the transcript moves to a + child session, but a restart or failed send can leave the SessionStore + mapping pointing at the compressed parent. Heal that on read so the + next inbound message resumes the child instead of reloading the parent. + """ + if not session_id or self._db is None: + return session_id + try: + return self._db.get_compression_tip(session_id) or session_id + except Exception: + logger.debug( + "Compression-tip lookup failed for session %s", + session_id, + exc_info=True, + ) + return session_id + + def _heal_compression_tip_locked( + self, + entry: "SessionEntry", + original_session_id: Optional[str], + canonical_session_id: Optional[str], + ) -> bool: + """Rewrite *entry* to the compression continuation if stale. Lock held.""" + if ( + not original_session_id + or not canonical_session_id + or entry.session_id != original_session_id + or canonical_session_id == original_session_id + ): + return False + logger.info( + "SessionStore healed compressed session mapping: %s -> %s", + entry.session_id, + canonical_session_id, + ) + entry.session_id = canonical_session_id + return True + def has_any_sessions(self) -> bool: """Check if any sessions have ever been created (across all platforms). @@ -1409,12 +1451,30 @@ class SessionStore: # All _entries / _loaded mutations are protected by self._lock. db_end_session_id = None db_create_kwargs = None + existing_session_id = None + + if not force_new: + with self._lock: + self._ensure_loaded_locked() + entry = self._entries.get(session_key) + if entry is not None: + existing_session_id = entry.session_id + + # Look up the compression continuation outside the lock (DB I/O). + canonical_existing_session_id = ( + self._compression_tip_for_session_id(existing_session_id) + if existing_session_id + else None + ) with self._lock: self._ensure_loaded_locked() if session_key in self._entries and not force_new: entry = self._entries[session_key] + self._heal_compression_tip_locked( + entry, existing_session_id, canonical_existing_session_id + ) # Self-heal stale routing: if this session_key still points at # a session that has ALREADY been ended in state.db (end_reason diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 0e2863cc21b..c3af49f2531 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -19,6 +19,7 @@ import concurrent.futures import functools from dataclasses import dataclass from datetime import datetime, timezone +import hashlib import hmac import importlib.util import json @@ -8953,7 +8954,11 @@ async def remove_mcp_server(name: str, profile: Optional[str] = None): @app.post("/api/mcp/servers/{name}/test") async def test_mcp_server(name: str, profile: Optional[str] = None): """Connect to the server, list its tools, disconnect. Returns tool list.""" - from hermes_cli.mcp_config import _get_mcp_servers, _probe_single_server + from hermes_cli.mcp_config import ( + _get_mcp_servers, + _oauth_tokens_present, + _probe_single_server, + ) with _profile_scope(profile): servers = _get_mcp_servers() @@ -8961,6 +8966,10 @@ async def test_mcp_server(name: str, profile: Optional[str] = None): raise HTTPException(status_code=404, detail=f"Server '{name}' not found") details: Dict[str, Any] = {} + # An `auth: oauth` server that serves tools/list anonymously would probe OK + # with no token — a false green. Require a token on disk for it, matching the + # /auth verification (some providers don't enforce auth on tools/list). + needs_oauth_token = servers[name].get("auth") == "oauth" def _probe_scoped(): # Home-only scope (contextvar), NOT _profile_scope. A probe blocks for @@ -8974,18 +8983,26 @@ async def test_mcp_server(name: str, profile: Optional[str] = None): # contextvar provides (copied into this to_thread worker; and # _run_on_mcp_loop re-wraps it onto the MCP event-loop thread). with _config_profile_scope(profile): - return _probe_single_server(name, servers[name], details=details) + tools = _probe_single_server(name, servers[name], details=details) + token_present = _oauth_tokens_present(name) if needs_oauth_token else True + return tools, token_present try: # Probe blocks on a dedicated MCP event loop — run in a thread so the # FastAPI event loop is never blocked. - tools = await asyncio.to_thread(_probe_scoped) + tools, token_present = await asyncio.to_thread(_probe_scoped) except Exception as exc: return { "ok": False, "error": str(exc), "tools": [], } + if not token_present: + return { + "ok": False, + "error": "OAuth authentication required — no token found.", + "tools": [], + } return { "ok": True, "tools": [{"name": t, "description": d} for t, d in tools], @@ -9033,24 +9050,35 @@ async def auth_mcp_server(name: str, profile: Optional[str] = None): cfg["auth"] = "oauth" def _run(): - from tools.mcp_oauth import force_interactive_oauth + from tools.mcp_oauth import HermesTokenStorage, force_interactive_oauth # Home-only scope, not _profile_scope: this blocks on the browser flow # for up to minutes; holding the shared skills lock that whole time # would freeze every other endpoint. Config writes here (_save_mcp_server) # resolve HERMES_HOME via the contextvar override, which is all they need. with _config_profile_scope(profile), force_interactive_oauth(): + storage = HermesTokenStorage(name) + # Snapshot before clearing: a re-auth wipes cached state to force a + # fresh consent, but if the flow fails we must NOT leave the user + # worse off than before — restore the working token on any failure. + backup = storage.snapshot() try: from tools.mcp_oauth_manager import get_manager get_manager().remove(name) except Exception: pass # No cached state to clear — fine. - # The default 30s connect timeout would kill the flow while the - # user is still looking at the browser consent screen — give the - # whole browser round-trip room (the callback itself caps at 300s). - tools = _probe_single_server(name, cfg, connect_timeout=240) + try: + # The default 30s connect timeout would kill the flow while the + # user is still on the consent screen — give the browser + # round-trip the full callback window (300s in mcp_oauth) plus + # headroom so the connect wrapper can't pre-empt it. + tools = _probe_single_server(name, cfg, connect_timeout=315) + except Exception: + storage.restore(backup) + raise if not _oauth_tokens_present(name): + storage.restore(backup) return { "ok": False, "error": ( @@ -9227,16 +9255,20 @@ async def install_mcp_catalog_entry(body: MCPCatalogInstall, profile: Optional[s # action path so the request returns immediately and the UI can tail logs. # The -p subprocess rebinds HERMES_HOME-derived paths in the child. if entry.install is not None: + # Unique per-entry action name: a shared "mcp-install" would let a + # re-click (or a second entry) overwrite the tracked process/log while + # the first clone is still running. + action = _mcp_install_action_name(name) try: proc = _spawn_hermes_action( _profile_cli_args(effective_profile) + ["mcp", "install", name], - "mcp-install", + action, ) except HTTPException: raise except Exception as exc: raise HTTPException(status_code=500, detail=f"Install failed: {exc}") - return {"ok": True, "name": name, "background": True, "action": "mcp-install"} + return {"ok": True, "name": name, "background": True, "action": action} # No git step — install synchronously via the catalog API. install_entry # routes through load_config/save_config + save_env_value, all call-time @@ -9258,8 +9290,17 @@ async def install_mcp_catalog_entry(body: MCPCatalogInstall, profile: Optional[s return {"ok": True, "name": name, "background": False} -# Register the mcp-install action log so /api/actions/mcp-install/status works. -_ACTION_LOG_FILES.setdefault("mcp-install", "action-mcp-install.log") +def _mcp_install_action_name(name: str) -> str: + """Unique per-entry mcp-install action name (+ registered log file), so a + re-click or a second catalog install doesn't overwrite the first's tracked + process/log while its git clone is still running.""" + slug = re.sub(r"[^a-z0-9]+", "-", name.lower()).strip("-")[:48] or "server" + digest = hashlib.sha1(name.encode()).hexdigest()[:8] + action = f"mcp-install-{slug}-{digest}" + _ACTION_LOG_FILES.setdefault(action, f"action-{action}.log") + return action + + _ACTION_LOG_FILES.setdefault("computer-use-grant", "action-computer-use-grant.log") @@ -10205,23 +10246,39 @@ def _profile_cli_args(profile: Optional[str]) -> List[str]: return ["-p", profiles_mod.normalize_profile_name(requested)] +def _hub_action_name(verb: str, key: str) -> str: + """Unique per-skill hub action name (+ registered log file). + + ``_spawn_hermes_action`` tracks one process/log per name, so a shared + "skills-install"/"skills-uninstall" would make concurrent row-level actions + overwrite each other's status/log while the UI polls per identifier. Slug + (readable) + hash (collision-proof) keys each action to its own row. + """ + slug = re.sub(r"[^a-z0-9]+", "-", key.lower()).strip("-")[:48] or "skill" + digest = hashlib.sha1(key.encode()).hexdigest()[:8] + name = f"skills-{verb}-{slug}-{digest}" + _ACTION_LOG_FILES.setdefault(name, f"action-{name}.log") + return name + + @app.post("/api/skills/hub/install") async def install_skill_hub(body: SkillInstallRequest, profile: Optional[str] = None): identifier = (body.identifier or "").strip() if not identifier: raise HTTPException(status_code=400, detail="identifier is required") + name = _hub_action_name("install", identifier) try: proc = _spawn_hermes_action( _profile_cli_args(body.profile or profile) + ["skills", "install", identifier, "--yes"], - "skills-install", + name, ) 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}") - return {"ok": True, "pid": proc.pid, "name": "skills-install"} + return {"ok": True, "pid": proc.pid, "name": name} class SkillUninstallRequest(BaseModel): @@ -10234,17 +10291,18 @@ async def uninstall_skill_hub(body: SkillUninstallRequest, profile: Optional[str name = (body.name or "").strip() if not name: raise HTTPException(status_code=400, detail="name is required") + action = _hub_action_name("uninstall", name) try: proc = _spawn_hermes_action( _profile_cli_args(body.profile or profile) + ["skills", "uninstall", name, "--yes"], - "skills-uninstall", + action, ) 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"} + return {"ok": True, "pid": proc.pid, "name": action} class SkillsUpdateRequest(BaseModel): @@ -10341,7 +10399,8 @@ async def list_skills_hub_sources(profile: Optional[str] = None): def _run(): from tools.skills_hub import create_source_router - sources = create_source_router() + with _config_profile_scope(profile): + sources = create_source_router() out = [] index_available = False featured = [] @@ -10372,6 +10431,17 @@ async def list_skills_hub_sources(profile: Optional[str] = None): except Exception: featured = [] out.append(entry) + # Tell the UI which sources are worth searching individually (for its + # progressive per-source fan-out). Mirror parallel_search_sources: when + # the centralized index is available it already subsumes the external + # API sources, so they're redundant — skipping them avoids ~70 GitHub + # calls per keystroke. Keep this set in sync with that function's + # ``_api_source_ids``. + _api_source_ids = frozenset( + {"github", "skills-sh", "clawhub", "claude-marketplace", "lobehub", "well-known"} + ) + for entry in out: + entry["searchable"] = not (index_available and entry["id"] in _api_source_ids) return { "sources": out, "index_available": index_available, @@ -10406,7 +10476,8 @@ async def search_skills_hub( def _run(): from tools.skills_hub import create_source_router, parallel_search_sources - sources = create_source_router() + with _config_profile_scope(profile): + sources = create_source_router() capped = min(max(limit, 1), 50) all_results, source_counts, timed_out = parallel_search_sources( sources, query=query, source_filter=source or "all", overall_timeout=30 @@ -10439,13 +10510,16 @@ async def search_skills_hub( @app.get("/api/skills/hub/preview") -async def preview_skill_hub(identifier: str = ""): +async def preview_skill_hub(identifier: str = "", profile: Optional[str] = None): """Fetch a hub skill's SKILL.md content + metadata for in-dashboard reading. Resolves the identifier across configured sources (same path the CLI installer uses), then returns the rendered SKILL.md text and the file manifest WITHOUT installing anything. This is the 'read the actual skill before installing' affordance the Browse-hub tab was missing. + + Scoped to ``profile`` so a non-default profile with different hub taps + resolves against ITS source router, not the default profile's. """ ident = (identifier or "").strip() if not ident: @@ -10455,8 +10529,9 @@ async def preview_skill_hub(identifier: str = ""): from hermes_cli.skills_hub import _resolve_source_meta_and_bundle from tools.skills_hub import create_source_router - sources = create_source_router() - meta, bundle, _src = _resolve_source_meta_and_bundle(ident, sources) + with _config_profile_scope(profile): + sources = create_source_router() + meta, bundle, _src = _resolve_source_meta_and_bundle(ident, sources) if not bundle and not meta: return None @@ -10500,7 +10575,7 @@ async def preview_skill_hub(identifier: str = ""): @app.get("/api/skills/hub/scan") -async def scan_skill_hub(identifier: str = ""): +async def scan_skill_hub(identifier: str = "", profile: Optional[str] = None): """Run the install-time security scan on a hub skill WITHOUT installing it. Fetches the bundle, quarantines it, and runs the same `scan_skill` / @@ -10508,6 +10583,9 @@ async def scan_skill_hub(identifier: str = ""): quarantine. Returns the verdict, per-finding detail, trust tier, and the install-policy decision so the dashboard can show a visual safety result on demand (the 'scan' button the Browse-hub tab was missing). + + Scoped to ``profile`` so the bundle resolves against that profile's hub + source router, matching where an install would pull it from. """ ident = (identifier or "").strip() if not ident: @@ -10520,8 +10598,9 @@ async def scan_skill_hub(identifier: str = ""): from tools.skills_hub import create_source_router, quarantine_bundle from tools.skills_guard import scan_skill, should_allow_install - sources = create_source_router() - meta, bundle, _src = _resolve_source_meta_and_bundle(ident, sources) + with _config_profile_scope(profile): + sources = create_source_router() + meta, bundle, _src = _resolve_source_meta_and_bundle(ident, sources) if not bundle: return None @@ -10963,7 +11042,7 @@ async def create_profile_endpoint(body: ProfileCreate): try: proc = _spawn_hermes_action( ["-p", body.name, "skills", "install", ident, "--yes"], - "skills-install", + _hub_action_name("install", ident), ) hub_installs.append({"identifier": ident, "pid": proc.pid}) except Exception: diff --git a/tests/gateway/test_restart_resume_pending.py b/tests/gateway/test_restart_resume_pending.py index 9b159ae3b03..23c5e674f47 100644 --- a/tests/gateway/test_restart_resume_pending.py +++ b/tests/gateway/test_restart_resume_pending.py @@ -384,6 +384,27 @@ class TestGetOrCreateResumePending: # Flag is NOT cleared on read — only on successful turn completion. assert second.resume_pending is True + def test_resume_pending_follows_compression_tip(self, tmp_path): + """Interrupted platform mappings must not stay pinned to compressed roots.""" + store = _make_store(tmp_path) + source = _make_source( + platform=Platform.WEIXIN, + chat_id="wx-chat", + user_id="wx-user", + ) + first = store.get_or_create_session(source) + original_sid = first.session_id + store.mark_resume_pending(first.session_key) + + with patch.object( + store, "_compression_tip_for_session_id", return_value="child-session" + ) as mock_tip: + second = store.get_or_create_session(source) + + assert second.session_id == "child-session" + assert second.resume_pending is True + mock_tip.assert_called_with(original_sid) + def test_suspended_still_creates_new_session(self, tmp_path): """Regression guard — suspended must still force a clean slate.""" store = _make_store(tmp_path) diff --git a/tests/gateway/test_session_store_runtime_stale_guard.py b/tests/gateway/test_session_store_runtime_stale_guard.py index 262d1a489bb..57f8c624bf2 100644 --- a/tests/gateway/test_session_store_runtime_stale_guard.py +++ b/tests/gateway/test_session_store_runtime_stale_guard.py @@ -49,6 +49,10 @@ def _db_returning(rows: dict) -> MagicMock: db.find_latest_gateway_session_for_peer.return_value = None db.reopen_session.return_value = None db.create_session.return_value = None + # No compression continuation → the tip is the session itself (identity), + # mirroring the real SessionDB.get_compression_tip. Without this a bare Mock + # would return a Mock the routing heal then assigns as session_id. + db.get_compression_tip.side_effect = lambda sid: sid return db diff --git a/tests/hermes_cli/test_web_server.py b/tests/hermes_cli/test_web_server.py index cbd714bd9d0..7797be63177 100644 --- a/tests/hermes_cli/test_web_server.py +++ b/tests/hermes_cli/test_web_server.py @@ -3551,7 +3551,7 @@ class TestNewEndpoints: assert spawned == [ ( ["-p", "builder", "skills", "install", "someuser/some-skill", "--yes"], - "skills-install", + web_server._hub_action_name("install", "someuser/some-skill"), ) ] @@ -3800,12 +3800,16 @@ class TestNewEndpoints: "description": "active", "category": "demo", "enabled": True, + "usage": 0, + "provenance": "agent", }, { "name": "disabled-skill", "description": "disabled", "category": "demo", "enabled": False, + "usage": 0, + "provenance": "agent", }, ] diff --git a/tests/hermes_cli/test_web_server_profile_unification.py b/tests/hermes_cli/test_web_server_profile_unification.py index 40dde33aed4..a7fc2145995 100644 --- a/tests/hermes_cli/test_web_server_profile_unification.py +++ b/tests/hermes_cli/test_web_server_profile_unification.py @@ -188,7 +188,7 @@ class TestProfileScopedMcp: ) seen = {} - def fake_probe(name, config, connect_timeout=30): + def fake_probe(name, config, connect_timeout=30, details=None): seen["home"] = str(get_hermes_home()) return [("tool-a", "desc")] @@ -200,6 +200,39 @@ class TestProfileScopedMcp: assert resp.json()["ok"] is True assert seen["home"] == str(isolated_profiles["worker_beta"]) + def test_mcp_test_oauth_server_without_token_is_not_ok( + self, client, isolated_profiles, monkeypatch + ): + """An `auth: oauth` server that serves tools/list anonymously must not + false-green: a successful probe with no token on disk reports needs-auth.""" + import hermes_cli.mcp_config as mcp_config + + (isolated_profiles["worker_beta"] / "config.yaml").write_text( + "mcp_servers:\n oauth-srv:\n url: http://x/sse\n auth: oauth\n", + encoding="utf-8", + ) + monkeypatch.setattr( + mcp_config, + "_probe_single_server", + lambda name, config, connect_timeout=30, details=None: [("tool-a", "desc")], + ) + monkeypatch.setattr(mcp_config, "_oauth_tokens_present", lambda name: False) + + resp = client.post( + "/api/mcp/servers/oauth-srv/test", params={"profile": "worker_beta"} + ) + assert resp.status_code == 200 + body = resp.json() + assert body["ok"] is False + assert "oauth" in body["error"].lower() + + # With a token present, the same probe is genuinely authenticated. + monkeypatch.setattr(mcp_config, "_oauth_tokens_present", lambda name: True) + resp = client.post( + "/api/mcp/servers/oauth-srv/test", params={"profile": "worker_beta"} + ) + assert resp.json()["ok"] is True + def test_mcp_remove_scoped(self, client, isolated_profiles): (isolated_profiles["worker_beta"] / "config.yaml").write_text( "mcp_servers:\n srv2:\n url: http://x/sse\n", encoding="utf-8" diff --git a/tests/hermes_cli/test_web_server_skills_profiles.py b/tests/hermes_cli/test_web_server_skills_profiles.py index 76325d628f2..df62dd833b3 100644 --- a/tests/hermes_cli/test_web_server_skills_profiles.py +++ b/tests/hermes_cli/test_web_server_skills_profiles.py @@ -180,7 +180,7 @@ class TestProfileScopedHubActions: assert calls == [ ( ["-p", "worker_alpha", "skills", "install", "official/demo", "--yes"], - "skills-install", + web_server._hub_action_name("install", "official/demo"), ) ] diff --git a/tools/mcp_oauth.py b/tools/mcp_oauth.py index 89d0051bc9b..80bf30b0986 100644 --- a/tools/mcp_oauth.py +++ b/tools/mcp_oauth.py @@ -399,6 +399,41 @@ class HermesTokenStorage: for p in (self._tokens_path(), self._client_info_path(), self._meta_path()): p.unlink(missing_ok=True) + def snapshot(self) -> dict[str, bytes]: + """Capture on-disk OAuth state so a failed re-auth can restore it. + + Maps filename -> bytes for whichever of the three state files exist. + Feed back to ``restore()`` to undo an intervening ``remove()`` when a + re-authentication attempt fails, so a still-valid token isn't destroyed. + """ + snap: dict[str, bytes] = {} + for p in (self._tokens_path(), self._client_info_path(), self._meta_path()): + try: + snap[p.name] = p.read_bytes() + except OSError: + pass + return snap + + def restore(self, snapshot: dict[str, bytes]) -> None: + """Revert to a ``snapshot()`` capture (dropping any newer partial state).""" + self.remove() + if not snapshot: + return + token_dir = _get_token_dir() + token_dir.mkdir(parents=True, exist_ok=True) + for fname, data in snapshot.items(): + path = token_dir / fname + try: + fd = os.open( + str(path), + os.O_WRONLY | os.O_CREAT | os.O_TRUNC, + stat.S_IRUSR | stat.S_IWUSR, + ) + with os.fdopen(fd, "wb") as fh: + fh.write(data) + except OSError as exc: + logger.warning("Failed to restore OAuth state %s: %s", fname, exc) + def poison_client_registration(self) -> bool: """Discard a dead dynamically-registered client so it gets re-created.