diff --git a/.ace/upstream-main.sha b/.ace/upstream-main.sha index 1415839670b3c..d869b4b1b9462 100644 --- a/.ace/upstream-main.sha +++ b/.ace/upstream-main.sha @@ -1 +1 @@ -b3e45a3d46ce1af52a267cc3aaa3cb6c4f52d1e8 +43717123ca1566a073270c5a61431e2e0e4a0211 diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index 00cfe583dd336..25bb67a46d66a 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -4243,6 +4243,28 @@ def _perform_api_call(next_api_kwargs): f" Check which providers support tools: https://openrouter.ai/models/{_model}" ) + # Actionable hint for a bare 404 on a provider whose catalogue + # uses ``vendor/model`` ids. A model id that lost its prefix + # (e.g. ``nemotron-…`` instead of ``nvidia/nemotron-…``) gets + # a content-free "404 page not found" from the provider that + # never names the model, so it reads like an outage or an auth + # failure. Name the real cause and the exact id to use (#78796). + if getattr(api_error, "status_code", None) == 404: + try: + from hermes_cli.model_normalize import suggest_prefixed_model_id + + _suggestion = suggest_prefixed_model_id(_provider, _model) + except Exception: + _suggestion = None + if _suggestion: + agent._buffer_vprint( + f" 💡 Model '{_model}' is not a valid id for provider {_provider} — " + f"it is missing its vendor prefix." + ) + agent._buffer_vprint( + f" Did you mean '{_suggestion}'? Re-pick it with `hermes model`." + ) + # Check for interrupt before deciding to retry if agent._interrupt_requested: # Preserve a pending redirect (mid-stream correction): the diff --git a/agent/error_classifier.py b/agent/error_classifier.py index 8ac0b6c872345..92d9fd43efba0 100644 --- a/agent/error_classifier.py +++ b/agent/error_classifier.py @@ -339,6 +339,32 @@ def is_auth(self) -> bool: "no endpoints found that support tool use", ] + +def _model_id_missing_known_prefix(model: str, provider: str) -> bool: + """True when a bare model id is only known to the provider as ``vendor/id``. + + Some providers answer a malformed model id with a naked 404 that names + nothing — NVIDIA NIM returns ``404 page not found`` for a bare + ``nemotron-3-ultra-550b-a55b``, indistinguishable from a bad endpoint + path. Consulting the curated catalogue tells the two apart: if the id + carries no ``/`` but the catalogue has exactly one entry ending in + ``/``, the prefix was dropped and the failure is deterministic. + + Never guesses — an id absent from the catalogue (a local NIM container, + a proxied model) returns False so genuine endpoint problems keep their + retryable ``unknown`` classification. + """ + name = (model or "").strip() + if not name or "/" in name: + return False + try: + from hermes_cli.model_normalize import suggest_prefixed_model_id + + return bool(suggest_prefixed_model_id((provider or "").strip(), name)) + except Exception: + return False + + # Malformed-message-array 400s. Deterministic request-shape rejections that # describe the *transcript* being invalid, not a parameter. The canonical # case: a stream dies mid-response and Hermes persists a content-less @@ -1061,6 +1087,18 @@ def _classify_by_status( retryable=False, should_fallback=True, ) + # A bare id that the provider's catalogue only knows in prefixed form + # is a malformed model id, not a routing glitch — NVIDIA NIM answers + # one with a naked ``404 page not found`` that names nothing, so the + # generic branch below burns three retries and reports what looks + # like an outage (#78796). Deterministic: don't retry, and let the + # model_not_found surface carry the real cause. + if _model_id_missing_known_prefix(model, provider): + return result_fn( + FailoverReason.model_not_found, + retryable=False, + should_fallback=True, + ) # Generic 404 with no "model not found" signal — could be a wrong # endpoint path (common with local llama.cpp / Ollama / vLLM when # the URL is slightly misconfigured), a proxy routing glitch, or diff --git a/agent/tool_executor.py b/agent/tool_executor.py index 5c13d061d1e74..e1a3f3013df60 100644 --- a/agent/tool_executor.py +++ b/agent/tool_executor.py @@ -339,6 +339,7 @@ class _ManagedToolResult: args: dict[str, Any] middleware_trace: list[dict[str, Any]] blocked: bool + dispatched: bool class _ConcurrentToolAuthorizationGate: @@ -383,12 +384,13 @@ def excluded_seconds(self) -> float: def _managed_values( outcome: _ManagedToolResult, -) -> tuple[Any, dict[str, Any], list[dict[str, Any]], bool]: +) -> tuple[Any, dict[str, Any], list[dict[str, Any]], bool, bool]: return ( outcome.result, outcome.args, outcome.middleware_trace, outcome.blocked, + outcome.dispatched, ) @@ -571,6 +573,7 @@ def _hermes_pipeline(relay_args: dict[str, Any]) -> Any: args=state["args"], middleware_trace=state["middleware_trace"], blocked=bool(state["blocked"]), + dispatched=bool(state["dispatched"]), ) @@ -688,12 +691,27 @@ def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effe if agent._interrupt_requested: print(f"{agent.log_prefix}⚡ Interrupt: skipping {num_tools} tool call(s)") for tc in tool_calls: + cancelled_result = ( + f"[Tool execution cancelled — {tc.function.name} was skipped " + "due to user interrupt]" + ) messages.append(make_tool_result_message( tc.function.name, - f"[Tool execution cancelled — {tc.function.name} was skipped due to user interrupt]", + cancelled_result, tc.id, effect_disposition="none", )) + _emit_terminal_post_tool_call( + agent, + function_name=tc.function.name, + function_args={}, + result=cancelled_result, + effective_task_id=effective_task_id, + tool_call_id=getattr(tc, "id", "") or "", + status="cancelled", + error_type="user_interrupt", + error_message="Tool execution skipped due to user interrupt", + ) _flush_session_db_after_tool_progress( agent, messages, @@ -840,6 +858,7 @@ def _run_tool( # submit site below (GHSA-qg5c-hvr5-hjgr, #13617). start = time.time() blocked = False + dispatched = False start_advanced = False def _advance_start(callback=None) -> None: @@ -883,6 +902,7 @@ def _execute(next_args: dict[str, Any]) -> Any: function_args = managed.args middleware_trace = managed.middleware_trace blocked = managed.blocked + dispatched = managed.dispatched except KeyboardInterrupt: try: agent.interrupt("keyboard interrupt") @@ -913,6 +933,17 @@ def _execute(next_args: dict[str, Any]) -> Any: result = f"Error executing tool '{function_name}': {tool_error}" logger.error("_invoke_tool raised for %s: %s", function_name, tool_error, exc_info=True) duration = time.time() - start + if not blocked and not dispatched: + _emit_terminal_post_tool_call( + agent, + function_name=function_name, + function_args=function_args, + result=result, + effective_task_id=effective_task_id, + tool_call_id=getattr(tool_call, "id", "") or "", + duration_ms=int(duration * 1000), + middleware_trace=list(middleware_trace), + ) is_error, _ = _detect_tool_failure(function_name, result) if is_error: logger.info("tool %s failed (%.2fs): %s", function_name, duration, result[:200]) @@ -1162,6 +1193,7 @@ def _execute(next_args: dict[str, Any]) -> Any: result=function_result, effective_task_id=effective_task_id, tool_call_id=getattr(tc, "id", "") or "", + duration_ms=int((timeout_s or 0.0) * 1000), status="timeout", error_type="tool_timeout", error_message=function_result, @@ -1204,6 +1236,19 @@ def _execute(next_args: dict[str, Any]) -> Any: name = function_name args = function_args progress_function_name = function_name + if _parse_error is not None: + _emit_terminal_post_tool_call( + agent, + function_name=function_name, + function_args=function_args, + result=function_result, + effective_task_id=effective_task_id, + tool_call_id=getattr(tc, "id", "") or "", + status="error", + error_type="invalid_tool_arguments", + error_message="Tool arguments must be a valid JSON object", + middleware_trace=list(middleware_trace), + ) if blocked: effect_disposition = "none" @@ -1394,12 +1439,27 @@ def execute_tool_calls_sequential(agent, assistant_message, messages: list, effe agent._vprint(f"{agent.log_prefix}⚡ Interrupt: skipping {len(remaining_calls)} tool call(s)", force=True) for skipped_tc in remaining_calls: skipped_name = skipped_tc.function.name + cancelled_result = ( + f"[Tool execution cancelled — {skipped_name} was skipped " + "due to user interrupt]" + ) messages.append(make_tool_result_message( skipped_name, - f"[Tool execution cancelled — {skipped_name} was skipped due to user interrupt]", + cancelled_result, skipped_tc.id, effect_disposition="none", )) + _emit_terminal_post_tool_call( + agent, + function_name=skipped_name, + function_args={}, + result=cancelled_result, + effective_task_id=effective_task_id, + tool_call_id=getattr(skipped_tc, "id", "") or "", + status="cancelled", + error_type="user_interrupt", + error_message="Tool execution skipped due to user interrupt", + ) if not _flush_session_db_after_tool_progress( agent, messages, @@ -1414,6 +1474,17 @@ def execute_tool_calls_sequential(agent, assistant_message, messages: list, effe tool_call.function.arguments ) if malformed_args_result is not None: + _emit_terminal_post_tool_call( + agent, + function_name=function_name, + function_args=function_args, + result=malformed_args_result, + effective_task_id=effective_task_id, + tool_call_id=getattr(tool_call, "id", "") or "", + status="error", + error_type="invalid_tool_arguments", + error_message="Tool arguments must be a valid JSON object", + ) messages.append( make_tool_result_message( function_name, @@ -1468,6 +1539,7 @@ def execute_tool_calls_sequential(agent, assistant_message, messages: list, effe middleware_trace: list[dict[str, Any]] = [] _execution_blocked = False + _execution_dispatched = False tool_start_time = time.time() @@ -1479,7 +1551,7 @@ def _execute(next_args: dict) -> Any: merge=next_args.get("merge", False), store=agent._todo_store, ) - function_result, function_args, middleware_trace, _execution_blocked = _managed_values(_run_agent_tool_execution_middleware( + function_result, function_args, middleware_trace, _execution_blocked, _execution_dispatched = _managed_values(_run_agent_tool_execution_middleware( agent, function_name=function_name, function_args=function_args, @@ -1510,7 +1582,7 @@ def _execute(next_args: dict) -> Any: db=session_db, current_session_id=agent.session_id, ) - function_result, function_args, middleware_trace, _execution_blocked = _managed_values(_run_agent_tool_execution_middleware( + function_result, function_args, middleware_trace, _execution_blocked, _execution_dispatched = _managed_values(_run_agent_tool_execution_middleware( agent, function_name=function_name, function_args=function_args, @@ -1549,7 +1621,7 @@ def _execute(next_args: dict) -> Any: ), ) return result - function_result, function_args, middleware_trace, _execution_blocked = _managed_values(_run_agent_tool_execution_middleware( + function_result, function_args, middleware_trace, _execution_blocked, _execution_dispatched = _managed_values(_run_agent_tool_execution_middleware( agent, function_name=function_name, function_args=function_args, @@ -1571,7 +1643,7 @@ def _execute(next_args: dict) -> Any: multi_select=next_args.get("multi_select", False), callback=agent.clarify_callback, ) - function_result, function_args, middleware_trace, _execution_blocked = _managed_values(_run_agent_tool_execution_middleware( + function_result, function_args, middleware_trace, _execution_blocked, _execution_dispatched = _managed_values(_run_agent_tool_execution_middleware( agent, function_name=function_name, function_args=function_args, @@ -1592,7 +1664,7 @@ def _execute(next_args: dict) -> Any: count=next_args.get("count"), callback=getattr(agent, "read_terminal_callback", None), ) - function_result, function_args, middleware_trace, _execution_blocked = _managed_values(_run_agent_tool_execution_middleware( + function_result, function_args, middleware_trace, _execution_blocked, _execution_dispatched = _managed_values(_run_agent_tool_execution_middleware( agent, function_name=function_name, function_args=function_args, @@ -1626,7 +1698,7 @@ def _execute(next_args: dict) -> Any: try: def _execute(next_args: dict) -> Any: return agent._dispatch_delegate_task(next_args) - function_result, function_args, middleware_trace, _execution_blocked = _managed_values(_run_agent_tool_execution_middleware( + function_result, function_args, middleware_trace, _execution_blocked, _execution_dispatched = _managed_values(_run_agent_tool_execution_middleware( agent, function_name=function_name, function_args=function_args, @@ -1659,7 +1731,7 @@ def _execute(next_args: dict) -> Any: try: def _execute(next_args: dict) -> Any: return agent.context_compressor.handle_tool_call(function_name, next_args, messages=messages) - function_result, function_args, middleware_trace, _execution_blocked = _managed_values(_run_agent_tool_execution_middleware( + function_result, function_args, middleware_trace, _execution_blocked, _execution_dispatched = _managed_values(_run_agent_tool_execution_middleware( agent, function_name=function_name, function_args=function_args, @@ -1695,7 +1767,7 @@ def _execute(next_args: dict) -> Any: try: def _execute(next_args: dict) -> Any: return agent._memory_manager.handle_tool_call(function_name, next_args) - function_result, function_args, middleware_trace, _execution_blocked = _managed_values(_run_agent_tool_execution_middleware( + function_result, function_args, middleware_trace, _execution_blocked, _execution_dispatched = _managed_values(_run_agent_tool_execution_middleware( agent, function_name=function_name, function_args=function_args, @@ -1755,6 +1827,7 @@ def _execute(next_args: dict) -> Any: function_args, middleware_trace, _execution_blocked, + _execution_dispatched, ) = _managed_values( _run_agent_tool_execution_middleware( agent, @@ -1833,6 +1906,7 @@ def _execute(next_args: dict) -> Any: function_args, middleware_trace, _execution_blocked, + _execution_dispatched, ) = _managed_values( _run_agent_tool_execution_middleware( agent, @@ -1895,7 +1969,10 @@ def _execute(next_args: dict) -> Any: from agent.agent_runtime_helpers import agent_runtime_owns_post_tool_hook _executor_must_emit_post_hook = ( not _execution_blocked - and agent_runtime_owns_post_tool_hook(agent, function_name) + and ( + not _execution_dispatched + or agent_runtime_owns_post_tool_hook(agent, function_name) + ) ) if _executor_must_emit_post_hook: _emit_terminal_post_tool_call( diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index 2f5340d6e9a0c..a082e2ba60b2d 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -10510,6 +10510,22 @@ ipcMain.handle('hermes:writeClipboard', (_event, text) => { return true }) +// Native save-location picker (profile export etc.) — the write itself happens +// elsewhere (the backend, for profile archives); this only picks the path. +ipcMain.handle('hermes:selectSavePath', async (_event, options: any = {}) => { + const result = await dialog.showSaveDialog(mainWindow, { + title: options?.title || 'Save', + defaultPath: options?.defaultPath ? String(options.defaultPath) : undefined, + filters: Array.isArray(options?.filters) ? options.filters : undefined + }) + + if (result.canceled || !result.filePath) { + return null + } + + return result.filePath +}) + // Paired reader for the GUI terminal's paste chord: the renderer's // navigator.clipboard.readText() throws "Document is not focused" whenever a // portaled overlay has focus, and there's no way to route a read through the diff --git a/apps/desktop/electron/preload.ts b/apps/desktop/electron/preload.ts index 11483d9ce8268..dd9537b26fd73 100644 --- a/apps/desktop/electron/preload.ts +++ b/apps/desktop/electron/preload.ts @@ -115,6 +115,7 @@ contextBridge.exposeInMainWorld('hermesDesktop', { }, readFileText: filePath => ipcRenderer.invoke('hermes:readFileText', filePath), selectPaths: options => ipcRenderer.invoke('hermes:selectPaths', options), + selectSavePath: options => ipcRenderer.invoke('hermes:selectSavePath', options), writeClipboard: text => ipcRenderer.invoke('hermes:writeClipboard', text), readClipboard: () => ipcRenderer.invoke('hermes:readClipboard'), saveImageFromUrl: url => ipcRenderer.invoke('hermes:saveImageFromUrl', url), diff --git a/apps/desktop/src/app/chat/sidebar/profile-switcher.tsx b/apps/desktop/src/app/chat/sidebar/profile-switcher.tsx index 1bd5ad3c1d1cb..4a178f15c5f3c 100644 --- a/apps/desktop/src/app/chat/sidebar/profile-switcher.tsx +++ b/apps/desktop/src/app/chat/sidebar/profile-switcher.tsx @@ -59,6 +59,7 @@ import { setShowAllProfiles, sortByProfileOrder } from '@/store/profile' +import { runExportProfileFlow, runImportProfileFlow } from '@/store/profile-share' import type { ProfileInfo } from '@/types/hermes' import { CreateProfileDialog } from '../../profiles/create-profile-dialog' @@ -264,6 +265,7 @@ export function ProfileRail() { profiles={named} /> setCreateOpen(true)} /> + ) : (
setCreateOpen(true)} /> +
)} @@ -435,6 +438,24 @@ function AddProfileButton({ label, onClick }: { label: string; onClick: () => vo ) } +// Import-archive door beside the "+": adopt a shared profile bundle (theme, +// skills, layout) as a new profile. Same chrome as AddProfileButton; the whole +// flow (picker → import → apply overlay → switch) lives in the store. +function ImportProfileButton({ label }: { label: string }) { + return ( + + + + ) +} + // The condensed rail: every named profile in one compact select. The trigger // shows the active profile (tinted initial + name); on default/all scope it // falls back to the placeholder since the left toggle pill carries that state. @@ -692,6 +713,10 @@ function ProfileSquare({ {p.editSoul} + void runExportProfileFlow(label)}> + + {p.exportProfile} + ({ t: { common: { cancel: 'Cancel', close: 'Close', delete: 'Delete', save: 'Save' }, sidebar: { - projects: { menuAppearance: 'Appearance', noColor: 'No color' }, + projects: { + menuAppearance: 'Appearance', + moveFailed: 'Could not move session', + moveNoProjects: 'No other projects', + movedTo: (name: string) => `Moved to ${name}`, + moveToProject: 'Move to project', + noColor: 'No color' + }, row: { archive: 'Archive', branchFrom: 'Branch from here', @@ -50,6 +57,12 @@ vi.mock('@/lib/profile-color', () => ({ PROFILE_SWATCHES: [] })) vi.mock('@/lib/session-export', () => ({ exportSession: vi.fn() })) vi.mock('@/store/gateway', () => ({ activeGateway: vi.fn(() => null) })) vi.mock('@/store/notifications', () => ({ notify: vi.fn(), notifyError: vi.fn() })) +vi.mock('@/store/projects', () => ({ + $projectTree: atom([]), + moveSessionToProject: vi.fn(), + projectIdForCwd: vi.fn(() => null), + projectRootCwd: vi.fn(() => '') +})) vi.mock('@/store/session', () => ({ $activeSessionId: atom(null), $selectedStoredSessionId: atom(null), diff --git a/apps/desktop/src/app/chat/sidebar/session-actions-menu.tsx b/apps/desktop/src/app/chat/sidebar/session-actions-menu.tsx index 57d8a9f9623f1..0a32e1881f85b 100644 --- a/apps/desktop/src/app/chat/sidebar/session-actions-menu.tsx +++ b/apps/desktop/src/app/chat/sidebar/session-actions-menu.tsx @@ -30,6 +30,7 @@ import { PROFILE_SWATCHES } from '@/lib/profile-color' import { exportSession } from '@/lib/session-export' import { activeGateway } from '@/store/gateway' import { notify, notifyError } from '@/store/notifications' +import { $projectTree, moveSessionToProject, projectIdForCwd, projectRootCwd } from '@/store/projects' import { $activeSessionId, $selectedStoredSessionId, @@ -133,6 +134,44 @@ function SessionColorSwatches({ sessionId }: { sessionId: string }) { ) } +// The project list inside the session menu's "Move to project" submenu. Its own +// component so only an OPEN submenu subscribes to the stores (same reasoning as +// SessionColorSwatches). Re-homes the session's workspace at the target +// project's root — the fix for a chat created in the wrong folder. The current +// owner and folderless projects (the Home bucket) are excluded: there is +// nothing to move into. +function MoveToProjectItems({ kit, sessionId, profile }: { kit: MenuKit; sessionId: string; profile?: string }) { + const { t } = useI18n() + const p = t.sidebar.projects + const tree = useStore($projectTree) + const session = useStore($sessions).find(s => sessionMatchesStoredId(s, sessionId)) + const cwd = session?.cwd?.trim() || '' + const currentProjectId = cwd ? projectIdForCwd(cwd) : null + const targets = tree.filter(node => node.id !== currentProjectId && !node.isNoProject && projectRootCwd(node)) + + if (targets.length === 0) { + return {p.moveNoProjects} + } + + return ( + <> + {targets.map(node => ( + { + triggerHaptic('selection') + moveSessionToProject(sessionId, node.id, profile) + .then(() => notify({ durationMs: 2_000, kind: 'success', message: p.movedTo(node.label) })) + .catch(err => notifyError(err, p.moveFailed)) + }} + > + {node.label} + + ))} + + ) +} + function useSessionActions({ sessionId, title, @@ -355,6 +394,15 @@ function useSessionActions({ /> {workItems.map(item => renderActionItem(kit, item))} + + + + {t.sidebar.projects.moveToProject} + + + + + {tabItems.length > 0 && ( <> diff --git a/apps/desktop/src/app/contrib/controller.tsx b/apps/desktop/src/app/contrib/controller.tsx index da4ff828cb85b..b643a31c1b436 100644 --- a/apps/desktop/src/app/contrib/controller.tsx +++ b/apps/desktop/src/app/contrib/controller.tsx @@ -39,7 +39,7 @@ import { useContributions } from '@/contrib/react/use-contributions' import { registry } from '@/contrib/registry' import { discoverRuntimePlugins } from '@/contrib/runtime-loader' import { sessionTitle as storedSessionTitle } from '@/lib/chat-runtime' -import { FileText, LayoutDashboard, PanelBottom, Terminal, Zap } from '@/lib/icons' +import { Download, FileText, LayoutDashboard, PanelBottom, Terminal, Upload, Zap } from '@/lib/icons' import { type KeybindContribution, KEYBINDS_AREA } from '@/lib/keybinds/actions' import { setYoloEnabled } from '@/lib/yolo-session' import { pruneComposerPopoutZones } from '@/store/composer-popout' @@ -56,6 +56,7 @@ import { SIDEBAR_MAX_WIDTH } from '@/store/layout' import { $previewOpenRequest, $previewTabs, closeRightRail } from '@/store/preview' +import { runExportProfileFlow, runImportProfileFlow } from '@/store/profile-share' import { $reviewOpen, closeReview, openReview, REVIEW_PANE_ID } from '@/store/review' import { $currentCwd, $selectedStoredSessionId, $sessions, $yoloActive, sessionMatchesStoredId } from '@/store/session' import { watchSessionPins } from '@/store/session-pin-sync' @@ -314,6 +315,31 @@ registry.registerMany([ keywords: ['keybinds', 'shortcuts', 'hotkeys', 'keyboard'], run: () => window.dispatchEvent(new CustomEvent('hermes:open-keybinds')) } satisfies PaletteContribution + }, + // Profile sharing: bundle the active profile (config, skills, theme, layout) + // into a portable archive, or adopt someone else's. Both open native dialogs, + // so the palette closing on select is correct. + { + id: 'profile.export', + area: PALETTE_AREA, + data: { + id: 'profile.export', + label: 'Export profile…', + icon: Upload, + keywords: ['profile', 'export', 'share', 'bundle', 'theme', 'settings', 'backup'], + run: () => void runExportProfileFlow() + } satisfies PaletteContribution + }, + { + id: 'profile.import', + area: PALETTE_AREA, + data: { + id: 'profile.import', + label: 'Import profile…', + icon: Download, + keywords: ['profile', 'import', 'share', 'bundle', 'archive', 'restore'], + run: () => void runImportProfileFlow() + } satisfies PaletteContribution } ]) diff --git a/apps/desktop/src/components/pane-shell/tree/model.ts b/apps/desktop/src/components/pane-shell/tree/model.ts index 7f61143babf44..06810cb4c5858 100644 --- a/apps/desktop/src/components/pane-shell/tree/model.ts +++ b/apps/desktop/src/components/pane-shell/tree/model.ts @@ -317,6 +317,66 @@ export function movePane( return shapeSignature(next) === shapeSignature(root) ? root : next } +/** + * Move a SELECTION of panes together (multi-tab drag), preserving their strip + * order. The lead pane lands exactly like a single `movePane` (center joins at + * `before`, an edge opens the split); the rest stack in behind it. `activeId` + * (the pressed tab) fronts in the landing group. Same no-op guard as + * `movePane`: a drop that rebuilds the visible arrangement returns `root`. + */ +export function movePanes( + root: LayoutNode, + paneIds: readonly string[], + target: { groupId: string; pos: DropPosition; before?: null | string }, + activeId: string = paneIds[0] ?? '' +): LayoutNode { + if (paneIds.length <= 1) { + return paneIds.length === 1 ? movePane(root, paneIds[0], target) : root + } + + let without: LayoutNode | null = root + + for (const id of paneIds) { + without = without && removePane(without, id) + } + + // The selection was the whole tree, or removal dissolved the target zone + // (the selection was its only occupancy) — nowhere left to land. + if (!without || !findGroup(without, target.groupId)) { + return root + } + + // The lead insert decides geometry; the rest stack into the lead's group at + // the same slot (each lands before `before`, so the block keeps its order). + // Only the lead activates — `insertAtGroup(activate)` would otherwise front + // each follower in turn. + const lead = paneIds[0] + let next: LayoutNode | null = insertAtGroup(without, target.groupId, lead, target.pos, target.before) + + for (let i = 1; next && i < paneIds.length; i++) { + const leadGroup = findGroupOfPane(next, lead) + + if (!leadGroup) { + return root + } + + const before = target.pos === 'center' ? (target.before ?? null) : null + next = insertAtGroup(next, leadGroup.id, paneIds[i], 'center', before, false) + } + + if (!next) { + return root + } + + const landed = findGroupOfPane(next, lead) + + if (landed && landed.panes.includes(activeId)) { + next = setActivePane(next, landed.id, activeId) + } + + return shapeSignature(next) === shapeSignature(root) ? root : next +} + /** Group ids of every leaf under a node, in tree order. */ export function groupLeafIds(node: LayoutNode): string[] { return node.type === 'group' ? [node.id] : node.children.flatMap(groupLeafIds) @@ -347,26 +407,32 @@ function findCover(node: LayoutNode, set: Set): LayoutNode | null { } /** - * FancyZones span: merge the highlighted zones into ONE group holding - * `paneId`, absorbing any panes that lived in those zones as tabs. Only works - * when the highlighted set forms a rectangular subtree (it always does for a - * combined zone range on a guillotine tree); returns null otherwise so the - * caller can fall back to a single-zone drop. + * FancyZones span: merge the highlighted zones into ONE group holding the + * dragged pane block (one pane, or a multi-tab selection in strip order), + * absorbing any panes that lived in those zones as tabs. Only works when the + * highlighted set forms a rectangular subtree (it always does for a combined + * zone range on a guillotine tree); returns null otherwise so the caller can + * fall back to a single-zone drop. */ -export function mergeZonesWithPane(root: LayoutNode, groupIds: string[], paneId: string): LayoutNode | null { +export function mergeZonesWithPane( + root: LayoutNode, + groupIds: string[], + paneId: string | readonly string[] +): LayoutNode | null { + const paneIds = typeof paneId === 'string' ? [paneId] : [...paneId] const set = new Set(groupIds) if (set.size <= 1 || !findCover(root, set)) { return null } - // Panes from the merged zones (tree order), minus the dragged one. + // Panes from the merged zones (tree order), minus the dragged block. const panesInSet: string[] = [] const collect = (n: LayoutNode) => { if (n.type === 'group') { if (set.has(n.id)) { - panesInSet.push(...n.panes.filter(p => p !== paneId)) + panesInSet.push(...n.panes.filter(p => !paneIds.includes(p))) } } else { n.children.forEach(collect) @@ -375,16 +441,19 @@ export function mergeZonesWithPane(root: LayoutNode, groupIds: string[], paneId: collect(root) - // If the dragged pane lives OUTSIDE the merged set, pull it from its origin + // Any dragged pane living OUTSIDE the merged set is pulled from its origin // first (leaving that origin an empty zone). Inside the set it's absorbed. - const origin = findGroupOfPane(root, paneId) let working = root - if (origin && !set.has(origin.id)) { - working = removePane(root, paneId) ?? root + for (const id of paneIds) { + const origin = findGroupOfPane(working, id) + + if (origin && !set.has(origin.id)) { + working = removePane(working, id) ?? working + } } - const merged = group([paneId, ...panesInSet]) + const merged = group([...paneIds, ...panesInSet]) const replace = (n: LayoutNode): LayoutNode => { if (sameSet(groupLeafIds(n), set)) { @@ -409,16 +478,23 @@ export function setActivePane(root: LayoutNode, groupId: string, paneId: string) return mapGroups(root, g => (g.id === groupId && g.panes.includes(paneId) ? { ...g, active: paneId } : g)) } -/** Reorder a pane within its group's tab stack (browser-tab drag semantics). */ -export function reorderPaneInGroup(root: LayoutNode, groupId: string, paneId: string, toIndex: number): LayoutNode { +/** Reorder a block of panes within a group as one unit (browser-tab drag + * semantics; a single-tab drag is a one-id block): the block lands at + * `toIndex` among the remaining tabs, keeping its own order. */ +export function reorderPanesInGroup( + root: LayoutNode, + groupId: string, + paneIds: readonly string[], + toIndex: number +): LayoutNode { return mapGroups(root, g => { - if (g.id !== groupId || !g.panes.includes(paneId)) { + if (g.id !== groupId || !paneIds.every(p => g.panes.includes(p))) { return g } - const without = g.panes.filter(p => p !== paneId) + const without = g.panes.filter(p => !paneIds.includes(p)) const index = Math.max(0, Math.min(without.length, toIndex)) - const panes = [...without.slice(0, index), paneId, ...without.slice(index)] + const panes = [...without.slice(0, index), ...paneIds, ...without.slice(index)] return { ...g, panes } }) diff --git a/apps/desktop/src/components/pane-shell/tree/multi-tab-drag.test.ts b/apps/desktop/src/components/pane-shell/tree/multi-tab-drag.test.ts new file mode 100644 index 0000000000000..1766e53fc75fa --- /dev/null +++ b/apps/desktop/src/components/pane-shell/tree/multi-tab-drag.test.ts @@ -0,0 +1,147 @@ +import { describe, expect, it } from 'vitest' + +import { findGroup, findGroupOfPane, group, mergeZonesWithPane, movePanes, reorderPanesInGroup, split } from './model' +import { $tabSelection, clearTabSelection, selectionFor, selectTabRange, toggleTabSelected } from './tab-selection' + +describe('movePanes (multi-tab drag)', () => { + it('stacks the whole block into the target group at the divider slot, in strip order', () => { + const tree = split('row', [ + group(['a', 'b', 'c'], { active: 'a', id: 'left' }), + group(['x', 'y'], { active: 'x', id: 'right' }) + ]) + + const next = movePanes(tree, ['a', 'c'], { before: 'y', groupId: 'right', pos: 'center' }, 'c') + const right = findGroup(next, 'right') + + expect(right).toMatchObject({ panes: ['x', 'a', 'c', 'y'], active: 'c' }) + expect(findGroup(next, 'left')).toMatchObject({ panes: ['b'] }) + }) + + it('an edge drop opens ONE split holding the block as tabs, pressed tab fronted', () => { + const tree = split('row', [ + group(['a', 'b', 'c'], { active: 'a', id: 'left' }), + group(['x'], { active: 'x', id: 'right' }) + ]) + + const next = movePanes(tree, ['b', 'c'], { groupId: 'right', pos: 'bottom' }, 'b') + const landed = findGroupOfPane(next, 'b') + + expect(landed).toMatchObject({ panes: ['b', 'c'], active: 'b' }) + // One new zone, not one per pane: b and c share a group. + expect(findGroupOfPane(next, 'c')).toBe(landed) + expect(findGroup(next, 'left')).toMatchObject({ panes: ['a'] }) + }) + + it('dragging a whole zone into a sibling dissolves the source zone', () => { + const tree = split('row', [ + group(['a', 'b'], { active: 'a', id: 'left' }), + group(['x'], { active: 'x', id: 'right' }) + ]) + + const next = movePanes(tree, ['a', 'b'], { groupId: 'right', pos: 'center' }, 'a') + + expect(next).toMatchObject({ type: 'group', panes: ['x', 'a', 'b'], active: 'a' }) + }) + + it('is a no-op when removal dissolves the target zone itself', () => { + const tree = split('row', [ + group(['a', 'b'], { active: 'a', id: 'left' }), + group(['x'], { active: 'x', id: 'right' }) + ]) + + // Dropping right's only pane (as part of a block) "into right" — the + // target vanishes with the removal, so nothing moves. + expect(movePanes(tree, ['x', 'a'], { groupId: 'right', pos: 'center' }, 'x')).toBe(tree) + }) + + it('falls back to single-pane semantics for a one-id block', () => { + const tree = split('row', [ + group(['a', 'b'], { active: 'a', id: 'left' }), + group(['x'], { active: 'x', id: 'right' }) + ]) + + const next = movePanes(tree, ['b'], { groupId: 'right', pos: 'center' }) + + expect(findGroup(next, 'right')).toMatchObject({ panes: ['x', 'b'], active: 'b' }) + }) +}) + +describe('reorderPanesInGroup (block reorder)', () => { + it('moves a selection as one unit, preserving its internal order', () => { + const tree = group(['a', 'b', 'c', 'd'], { active: 'a', id: 'g' }) + + // [a, c] to the end: index 2 among the remaining [b, d]. + expect(reorderPanesInGroup(tree, 'g', ['a', 'c'], 2)).toMatchObject({ panes: ['b', 'd', 'a', 'c'] }) + }) + + it('leaves the group alone when any id is missing (stale selection)', () => { + const tree = group(['a', 'b'], { active: 'a', id: 'g' }) + + expect(reorderPanesInGroup(tree, 'g', ['a', 'ghost'], 0)).toBe(tree) + }) +}) + +describe('mergeZonesWithPane with a multi-tab block', () => { + it('merges the span into one group led by the block in strip order', () => { + const tree = split('row', [ + group(['a', 'b'], { active: 'a', id: 'left' }), + split('column', [group(['x'], { active: 'x', id: 'mid' }), group(['y'], { active: 'y', id: 'right' })]) + ]) + + const next = mergeZonesWithPane(tree, ['mid', 'right'], ['a', 'b']) + + expect(next).toMatchObject({ type: 'group', panes: ['a', 'b', 'x', 'y'] }) + }) + + it('returns null for a non-rectangular span (caller falls back to a single-zone drop)', () => { + const tree = split('row', [ + group(['a', 'b'], { active: 'a', id: 'left' }), + group(['x'], { active: 'x', id: 'mid' }), + group(['y'], { active: 'y', id: 'right' }) + ]) + + expect(mergeZonesWithPane(tree, ['mid', 'right'], ['a', 'b'])).toBeNull() + }) +}) + +describe('tab selection (Chrome grammar)', () => { + it('⌥-click seeds with the active tab, toggles, and dissolves at ≤1', () => { + clearTabSelection() + toggleTabSelected('g', 'c', 'a') + + expect([...$tabSelection.get()!.ids].sort()).toEqual(['a', 'c']) + + toggleTabSelected('g', 'c', 'a') + + expect($tabSelection.get()).toBeNull() + }) + + it('shift-click ranges from the anchor and re-ranges on the next shift-click', () => { + clearTabSelection() + + const order = ['a', 'b', 'c', 'd'] + selectTabRange('g', order, 'c', 'a') + + expect(selectionFor('g', order, 'b')).toEqual(['a', 'b', 'c']) + + // Anchor holds at a (Chrome): re-ranging to d replaces, not extends. + selectTabRange('g', order, 'd', 'a') + + expect(selectionFor('g', order, 'd')).toEqual(['a', 'b', 'c', 'd']) + }) + + it('selectionFor answers null for an unselected pressed tab and drops stale ids', () => { + clearTabSelection() + toggleTabSelected('g', 'b', 'a') + toggleTabSelected('g', 'c', 'a') + + // Pressed tab outside the selection = single-tab drag. + expect(selectionFor('g', ['a', 'b', 'c', 'd'], 'd')).toBeNull() + // 'a' closed since: it silently falls out, strip order preserved. + expect(selectionFor('g', ['b', 'c', 'd'], 'b')).toEqual(['b', 'c']) + // Another zone never sees it. + expect(selectionFor('other', ['b', 'c'], 'b')).toBeNull() + + clearTabSelection() + }) +}) diff --git a/apps/desktop/src/components/pane-shell/tree/renderer/drag-session.ts b/apps/desktop/src/components/pane-shell/tree/renderer/drag-session.ts index d37853f8e1694..6e479c679527e 100644 --- a/apps/desktop/src/components/pane-shell/tree/renderer/drag-session.ts +++ b/apps/desktop/src/components/pane-shell/tree/renderer/drag-session.ts @@ -35,7 +35,8 @@ import { ESCAPE_PRIORITY, pushEscapeLayer } from '@/lib/escape-layers' import { reorderCommitHaptic, reorderStepHaptic } from '@/lib/reorder' import type { DropPosition } from '../model' -import { $dropHint, $treeDragging, type DropHint, mergeTreeZones, moveTreePane, reorderTreePane } from '../store' +import { $dropHint, $treeDragging, type DropHint, mergeTreeZones, moveTreePanes, reorderTreePanes } from '../store' +import { clearTabSelection } from '../tab-selection' import { type EngineZone, HighlightedZones, primaryZone, type ZoneRect } from '../zones-engine' const DRAG_THRESHOLD_PX = 4 @@ -96,10 +97,18 @@ const stripSlots = (strip: HTMLElement): StripSlot[] => }) /** Insertion slot from the pointer x against the OTHER tabs' midpoints: - * stack BEFORE the returned pane id (`null` = append). */ -export function slotBefore(slots: StripSlot[], x: number, excludePaneId = ''): { before: null | string } { + * stack BEFORE the returned pane id (`null` = append). `exclude` is the + * dragged tab — or the whole selection on a multi-tab drag, so the block + * can't target a slot inside itself. */ +export function slotBefore( + slots: StripSlot[], + x: number, + exclude: readonly string[] | string = '' +): { before: null | string } { + const excluded = typeof exclude === 'string' ? [exclude] : exclude + for (const slot of slots) { - if (slot.id === excludePaneId) { + if (excluded.includes(slot.id)) { continue } @@ -422,7 +431,11 @@ export function startPaneDrag( onTap?: () => void, reorder?: ReorderContext, double?: DoubleTapContext, - ghostLabel?: string + ghostLabel?: string, + /** Multi-tab selection riding this drag (strip order, includes `paneId`). + * The whole block moves/reorders together; `paneId` stays the pressed tab + * (it fronts at the destination). */ + selection?: readonly string[] ) { if (e.button !== 0) { return @@ -431,17 +444,28 @@ export function startPaneDrag( e.preventDefault() e.stopPropagation() + // The moving block: the selection when the pressed tab rides one, else just + // the pressed tab. Order is strip order (selectionFor guarantees it). + const moving: readonly string[] = selection && selection.length > 1 ? selection : [paneId] + const highlighted = new HighlightedZones() let zones: EngineZone[] = [] let strips: StripSnapshot[] = [] let mode: 'reorder' | 'zone' | null = null - let dimmed: HTMLElement | null = null + let dimmed: HTMLElement[] = [] const markSource = () => { - // The dragged tab dims for the drag's life — the divider says where it - // GOES, the dim says what MOVES. No live shuffle (placement-on-release). - dimmed ??= reorder?.strip.querySelector(`[data-tree-tab="${CSS.escape(paneId)}"]`) ?? null - dimmed?.style.setProperty('opacity', '0.45') + // Every dragged tab dims for the drag's life — the divider says where they + // GO, the dim says what MOVES. No live shuffle (placement-on-release). + if (dimmed.length === 0 && reorder) { + dimmed = moving + .map(id => reorder.strip.querySelector(`[data-tree-tab="${CSS.escape(id)}"]`)) + .filter((el): el is HTMLElement => el !== null) + } + + for (const el of dimmed) { + el.style.setProperty('opacity', '0.45') + } } const enterZoneMode = () => { @@ -494,7 +518,7 @@ export function startPaneDrag( groupId: reorder!.groupId, groupIds: [reorder!.groupId], pos: 'center', - stack: slotBefore(reorderStrip().slots, x, paneId) + stack: slotBefore(reorderStrip().slots, x, moving) } } @@ -525,7 +549,7 @@ export function startPaneDrag( const strip = groupIds.length === 1 && groupId ? strips.find(s => s.groupId === groupId && rectContains(s.rect, x, y)) : null - const stack = strip ? slotBefore(strip.slots, x, paneId) : undefined + const stack = strip ? slotBefore(strip.slots, x, moving) : undefined const pos: DropPosition = stack ? 'center' @@ -537,17 +561,26 @@ export function startPaneDrag( }, onCommit(hint) { + // A multi-tab selection is spent by a LANDED drop (reorder or zone) — + // a deny-area release keeps it, so a missed drop can just be retried. + const spendSelection = () => { + if (moving.length > 1) { + clearTabSelection() + } + } + if (mode === 'reorder' && reorder && hint?.stack !== undefined) { - // Slot -> index among the OTHER tabs (reorderPaneInGroup inserts there). + // Slot -> index among the OTHER tabs (the block re-inserts there). const others = [...reorder.strip.querySelectorAll('[data-tree-tab]')] .map(el => el.dataset.treeTab) - .filter((id): id is string => Boolean(id) && id !== paneId) + .filter((id): id is string => Boolean(id) && !moving.includes(id!)) const toIndex = hint.stack.before ? others.indexOf(hint.stack.before) : others.length if (toIndex >= 0) { - reorderTreePane(reorder.groupId, paneId, toIndex) + reorderTreePanes(reorder.groupId, moving, toIndex) reorderCommitHaptic() + spendSelection() } } @@ -559,18 +592,28 @@ export function startPaneDrag( const targets = hint?.groupIds ?? [] if (targets.length > 1) { - // Shift-span: merge the highlighted zones, dropping the pane across them. - mergeTreeZones([...targets], paneId, hint?.groupId ?? null) + // Shift-span: merge the highlighted zones, dropping the block across them. + mergeTreeZones([...targets], moving, hint?.groupId ?? null) + spendSelection() } else if (hint?.groupId) { // strip = stack at the divider slot; center = join the stack; - // an edge = split the zone and land there. - moveTreePane(paneId, { groupId: hint.groupId, pos: hint.pos ?? 'center', before: hint.stack?.before }) + // an edge = split the zone and land there. The whole selection + // rides — the pressed tab fronts at the destination. + moveTreePanes( + moving, + { groupId: hint.groupId, pos: hint.pos ?? 'center', before: hint.stack?.before }, + paneId + ) + spendSelection() } } }, onEnd() { - dimmed?.style.removeProperty('opacity') + for (const el of dimmed) { + el.style.removeProperty('opacity') + } + highlighted.reset() } }) diff --git a/apps/desktop/src/components/pane-shell/tree/renderer/tree-group.tsx b/apps/desktop/src/components/pane-shell/tree/renderer/tree-group.tsx index 4f7d7ada115d2..0694c6c3afa80 100644 --- a/apps/desktop/src/components/pane-shell/tree/renderer/tree-group.tsx +++ b/apps/desktop/src/components/pane-shell/tree/renderer/tree-group.tsx @@ -50,6 +50,14 @@ import { setTreeGroupMinimized, treeTabCloseTargets } from '../store' +import { + $tabSelection, + clearTabSelection, + isToggleSelectClick, + selectionFor, + selectTabRange, + toggleTabSelected +} from '../tab-selection' import { type DoubleTapContext, startPaneDrag } from './drag-session' import { forceLoneHeaderForPanes } from './lone-header' @@ -186,6 +194,9 @@ export function TreeGroup({ const narrow = useStore($narrowViewport) const newSessionTabAction = useStore($newSessionTabAction) const panesWithCloser = useStore($panesWithCloser) + // Multi-tab selection (⌥/Ctrl-click, Shift-click) — null for every zone but + // the one holding it, so this subscription is quiet during normal use. + const tabSelection = useStore($tabSelection) // Reload epochs: only an explicit tab-menu Reload writes here, so this // subscription costs nothing on a normal render. const paneEpochs = useStore($treePaneEpochs) @@ -435,6 +446,7 @@ export function TreeGroup({ const chrome = paneChrome(paneFor(paneId)) const closeable = closeableTab(paneId) const title = paneFor(paneId)?.title ?? paneId + const isSelected = tabSelection?.groupId === node.id && tabSelection.ids.has(paneId) const tab = ( closeTab(paneId) : undefined} onPointerDown={e => { + // Chrome's tab-selection grammar, ahead of activate/drag: + // Shift-click ranges from the anchor, ⌥-click (Ctrl-click + // off-Mac) toggles. Neither activates nor starts a drag — + // the press IS the selection edit. ⌘-click stays close + // (PaneTab claims it first) and ⌃-click stays the macOS + // context menu. + if (e.button === 0 && e.shiftKey) { + e.preventDefault() + e.stopPropagation() + selectTabRange(node.id, shown, paneId, activeId) + + return + } + + if (isToggleSelectClick(e)) { + e.preventDefault() + e.stopPropagation() + toggleTabSelected(node.id, paneId, activeId) + + return + } + // Tabs ACTIVATE (restoring a collapsed group). Minimize // lives on the chevron / single-pane label — overloading // the active tab made double-click a minimize/restore/hide - // lottery. + // lottery. A plain click also collapses any multi-tab + // selection back to the one tab (Chrome semantics). const onTap = () => { + clearTabSelection() + if (node.minimized) { restoreTreePane(paneId) } @@ -465,6 +502,26 @@ export function TreeGroup({ e.stopPropagation() } + // Dragging a SELECTED tab carries the whole selection as + // one block through the generic pane move — a multi-tab + // drag outranks the pane's own tab drag (the session drop + // language is single-session). + const dragSelection = selectionFor(node.id, shown, paneId) + + if (dragSelection) { + startPaneDrag( + paneId, + e, + onTap, + stripRef.current ? { groupId: node.id, strip: stripRef.current } : undefined, + hideHeaderDoubleTap, + t.zones.tabCount(dragSelection.length), + dragSelection + ) + + return + } + // A pane may own its tab drag (a session tab speaks the // session drop language — link/stack/split); `false` defers // to the generic pane move (the workspace tab on a fresh @@ -481,6 +538,7 @@ export function TreeGroup({ } }} role="tab" + selected={isSelected} style={{ cursor: 'grab' }} > {chrome.tabLead ? ( diff --git a/apps/desktop/src/components/pane-shell/tree/store.ts b/apps/desktop/src/components/pane-shell/tree/store.ts index abfb76e34c768..7392a04b05dc0 100644 --- a/apps/desktop/src/components/pane-shell/tree/store.ts +++ b/apps/desktop/src/components/pane-shell/tree/store.ts @@ -28,9 +28,10 @@ import { mergeZonesWithPane as mergeZonesWithPaneOp, mirrorTreeHorizontal, movePane as movePaneOp, + movePanes as movePanesOp, normalize, removePane, - reorderPaneInGroup as reorderPaneInGroupOp, + reorderPanesInGroup as reorderPanesInGroupOp, setActivePane as setActivePaneOp, setGroupHeaderHidden as setGroupHeaderHiddenOp, setGroupMinimized, @@ -1246,26 +1247,62 @@ export function applyTree(tree: LayoutNode, presetId: string) { } } +/** + * Move a multi-tab SELECTION in one commit (drag any selected tab): the lead + * pane takes the drop geometry, the rest stack in behind it in strip order, + * and `activeId` (the pressed tab) fronts in the landing group. + */ +export function moveTreePanes( + paneIds: readonly string[], + target: { groupId: string; pos: DropPosition; before?: null | string }, + activeId?: string +) { + const tree = $layoutTree.get() + + if (!tree) { + return + } + + const next = movePanesOp(tree, paneIds, target, activeId) + + if (next !== tree) { + commit(next) + markActivePreset('custom') + + for (const paneId of paneIds) { + markPaneUserPlaced(paneId) + } + } +} + /** * Shift-drag span: merge the highlighted zones into one holding `paneId`. Falls * back to a single-zone move at `fallbackGroupId` when the set can't merge * (non-rectangular selection). */ -export function mergeTreeZones(groupIds: string[], paneId: string, fallbackGroupId: string | null) { +export function mergeTreeZones( + groupIds: string[], + paneId: string | readonly string[], + fallbackGroupId: null | string +) { const tree = $layoutTree.get() if (!tree) { return } + const paneIds = typeof paneId === 'string' ? [paneId] : paneId const merged = mergeZonesWithPaneOp(tree, groupIds, paneId) if (merged) { commit(merged) markActivePreset('custom') - markPaneUserPlaced(paneId) + + for (const id of paneIds) { + markPaneUserPlaced(id) + } } else if (fallbackGroupId) { - moveTreePane(paneId, { groupId: fallbackGroupId, pos: 'center' }) + moveTreePanes(paneIds, { groupId: fallbackGroupId, pos: 'center' }) } } @@ -1277,11 +1314,13 @@ export function activateTreePane(groupId: string, paneId: string) { } } -export function reorderTreePane(groupId: string, paneId: string, toIndex: number) { +/** Reorder a tab block (multi-tab selection, or a single tab) within its + * group's strip — the block keeps its own order. */ +export function reorderTreePanes(groupId: string, paneIds: readonly string[], toIndex: number) { const tree = $layoutTree.get() if (tree) { - commit(reorderPaneInGroupOp(tree, groupId, paneId, toIndex)) + commit(reorderPanesInGroupOp(tree, groupId, paneIds, toIndex)) markActivePreset('custom') } } diff --git a/apps/desktop/src/components/pane-shell/tree/tab-selection.ts b/apps/desktop/src/components/pane-shell/tree/tab-selection.ts new file mode 100644 index 0000000000000..9653468ddab9e --- /dev/null +++ b/apps/desktop/src/components/pane-shell/tree/tab-selection.ts @@ -0,0 +1,99 @@ +/** + * Multi-tab selection on a zone's tab strip — Chrome's tab-selection grammar: + * + * - ⌥-click (Ctrl-click off-Mac) → toggle the tab in/out of the selection; + * - Shift-click → select the range from the anchor + * (the last explicitly clicked tab, else + * the active one) to the clicked tab; + * - plain click → collapse back to a single tab. + * + * ⌘-click stays CLOSE (middle-click.ts) and ⌃-click stays the macOS context + * menu, so the toggle chord is ⌥ on Mac / Ctrl elsewhere. One selection at a + * time, scoped to one zone — dragging any selected tab carries the whole set + * (drag-session resolves it), and ids are validated against the strip's + * current tabs at use time, so closed/moved panes fall out on their own. + */ + +import { atom } from 'nanostores' + +export interface TabSelection { + groupId: string + ids: ReadonlySet + /** Range anchor: the last explicitly clicked tab (Chrome semantics). */ + anchor: string +} + +export const $tabSelection = atom(null) + +const isMac = typeof navigator !== 'undefined' && /Mac|iP(hone|ad|od)/.test(navigator.platform) + +/** The toggle-select chord: ⌥-click on Mac (⌘ closes, ⌃ is the context menu), + * Ctrl-click elsewhere — ⌥ is accepted everywhere for one muscle memory. */ +export const isToggleSelectClick = (event: { altKey: boolean; button: number; ctrlKey: boolean; metaKey: boolean }) => + event.button === 0 && !event.metaKey && (event.altKey || (!isMac && event.ctrlKey)) + +export function clearTabSelection() { + if ($tabSelection.get()) { + $tabSelection.set(null) + } +} + +/** ⌥/Ctrl-click: toggle `paneId`. A fresh selection seeds with the active tab + * (it is implicitly selected, as in Chrome); collapsing to ≤1 dissolves the + * selection entirely — a single "selected" tab is just a tab. */ +export function toggleTabSelected(groupId: string, paneId: string, activeId: string) { + const current = $tabSelection.get() + const ids = new Set(current?.groupId === groupId ? current.ids : [activeId]) + + if (ids.has(paneId)) { + ids.delete(paneId) + } else { + ids.add(paneId) + } + + if (ids.size <= 1) { + $tabSelection.set(null) + + return + } + + $tabSelection.set({ anchor: paneId, groupId, ids }) +} + +/** Shift-click: select the contiguous range anchor→`paneId` in strip order, + * replacing the previous range (the anchor holds, Chrome-style). */ +export function selectTabRange(groupId: string, orderedPanes: readonly string[], paneId: string, activeId: string) { + const current = $tabSelection.get() + const anchor = current?.groupId === groupId && orderedPanes.includes(current.anchor) ? current.anchor : activeId + const a = orderedPanes.indexOf(anchor) + const b = orderedPanes.indexOf(paneId) + + if (a === -1 || b === -1) { + return + } + + const ids = new Set(orderedPanes.slice(Math.min(a, b), Math.max(a, b) + 1)) + + if (ids.size <= 1) { + $tabSelection.set(null) + + return + } + + $tabSelection.set({ anchor, groupId, ids }) +} + +/** The selection as an ordered slice of `orderedPanes` — but only when the + * pressed tab rides it (dragging an unselected tab is a single-tab drag). + * Stale ids (closed panes) drop out here. */ +export function selectionFor(groupId: string, orderedPanes: readonly string[], paneId: string): null | string[] { + const current = $tabSelection.get() + + if (current?.groupId !== groupId || !current.ids.has(paneId)) { + return null + } + + const ids = orderedPanes.filter(id => current.ids.has(id)) + + return ids.length > 1 ? ids : null +} diff --git a/apps/desktop/src/components/ui/pane-tab.tsx b/apps/desktop/src/components/ui/pane-tab.tsx index 47aad2614dfe1..906b0d1f80c53 100644 --- a/apps/desktop/src/components/ui/pane-tab.tsx +++ b/apps/desktop/src/components/ui/pane-tab.tsx @@ -31,12 +31,21 @@ const TAB_ACTIVE_UNDERLINE = 'shadow-[inset_0_-2px_0_var(--pane-tab-active-accen const TAB_IDLE = 'text-(--ui-text-tertiary) [--tab-bg:var(--pane-tab-strip-bg,var(--ui-sidebar-surface-background))] hover:shadow-[inset_0_0_0_100vmax_color-mix(in_srgb,#000_var(--ui-tab-hover-darken),transparent)] hover:text-(--ui-text-secondary)' +// A tab riding a multi-tab selection: an accent wash over whatever surface the +// tab sits on. A background-image gradient (not a shadow) so it stacks cleanly +// over `--tab-bg` without fighting the active underline / hover shadows. +const TAB_SELECTED = + '[background-image:linear-gradient(color-mix(in_srgb,var(--ui-accent)_14%,transparent),color-mix(in_srgb,var(--ui-accent)_14%,transparent))] text-foreground' + interface PaneTabProps extends React.ComponentProps<'div'> { active?: boolean dirty?: boolean /** Close gesture, no hover X (too easy to hit on small tabs): middle-click, * or ⌘-click as the trackpad-friendly Mac equivalent. */ onClose?: () => void + /** Part of a multi-tab selection (⌥/Ctrl-click, Shift-click) — an accent + * wash marks every tab that a drag would carry, Chrome-style. */ + selected?: boolean /** Vertical rail form (collapsed sidebar zones). */ vertical?: boolean /** Content-facing edge of a vertical rail — the strip line the active tab cuts. */ @@ -59,6 +68,7 @@ export const PaneTab = React.forwardRef(function P onPointerDown, onPointerUp, onClickCapture, + selected = false, vertical = false, side = 'left', children, @@ -81,9 +91,11 @@ export const PaneTab = React.forwardRef(function P active ? cn(TAB_ACTIVE, !vertical && TAB_ACTIVE_UNDERLINE) : cn(TAB_IDLE, edge && `${edge}-(--ui-stroke-tertiary)`), + selected && TAB_SELECTED, className )} data-active={active} + data-selected={selected || undefined} data-vertical={vertical || undefined} onClickCapture={event => { // Sites whose tab activates on the label's own onClick (the preview diff --git a/apps/desktop/src/global.d.ts b/apps/desktop/src/global.d.ts index 548badf5e6b7a..471b5c0dc6a7e 100644 --- a/apps/desktop/src/global.d.ts +++ b/apps/desktop/src/global.d.ts @@ -129,6 +129,12 @@ declare global { } readFileText: (filePath: string) => Promise selectPaths: (options?: HermesSelectPathsOptions) => Promise + /** Native save dialog; returns the chosen path or null on cancel. */ + selectSavePath?: (options?: { + defaultPath?: string + filters?: Array<{ extensions: string[]; name: string }> + title?: string + }) => Promise writeClipboard: (text: string) => Promise readClipboard: () => Promise saveImageFromUrl: (url: string) => Promise diff --git a/apps/desktop/src/hermes.ts b/apps/desktop/src/hermes.ts index b3d5a45555abb..f33d05226c106 100644 --- a/apps/desktop/src/hermes.ts +++ b/apps/desktop/src/hermes.ts @@ -47,6 +47,7 @@ import type { PairingResponse, PairingUser, ProfileCreatePayload, + ProfileDesktopOverlay, ProfileSetupCommand, ProfileSoul, ProfilesResponse, @@ -186,6 +187,7 @@ export type { PairingResponse, PairingUser, ProfileCreatePayload, + ProfileDesktopOverlay, ProfileInfo, ProfileSetupCommand, ProfileSoul, @@ -1431,6 +1433,36 @@ export function getProfileSetupCommand(name: string): Promise; output?: string } = {} +): Promise<{ archive: string; ok: boolean }> { + return window.hermesDesktop.api<{ archive: string; ok: boolean }>({ + path: `/api/profiles/${encodeURIComponent(name)}/export`, + method: 'POST', + body: { extra_files: opts.extraFiles ?? {}, output: opts.output ?? '' }, + timeoutMs: STARTUP_REQUEST_TIMEOUT_MS + }) +} + +/** Import a profile .tar.gz as a new profile. Returns the bundled desktop + * appearance overlay too (when the archive carried one) so the caller can + * apply theme/layout without another round-trip. */ +export function importProfileArchive( + archive: string, + name?: string +): Promise<{ desktop: null | ProfileDesktopOverlay; name: string; ok: boolean; path: string }> { + return window.hermesDesktop.api<{ desktop: null | ProfileDesktopOverlay; name: string; ok: boolean; path: string }>({ + path: '/api/profiles/import', + method: 'POST', + body: { archive, name: name || null }, + timeoutMs: STARTUP_REQUEST_TIMEOUT_MS + }) +} + export function getUsageAnalytics(days = 30): Promise { return window.hermesDesktop.api({ ...profileScoped(), diff --git a/apps/desktop/src/i18n/ar.ts b/apps/desktop/src/i18n/ar.ts index 6091c916c335b..79bb9aee36721 100644 --- a/apps/desktop/src/i18n/ar.ts +++ b/apps/desktop/src/i18n/ar.ts @@ -1293,6 +1293,12 @@ export const ar = defineLocale({ count: count => `${count} ملف شخصي`, loading: 'جار التحميل...', newProfile: 'ملف شخصي جديد', + importProfile: 'استيراد ملف شخصي…', + exportProfile: 'تصدير ملف شخصي…', + imported: 'تم استيراد الملف الشخصي', + exported: 'تم تصدير الملف الشخصي', + failedImport: 'فشل استيراد الملف الشخصي', + failedExport: 'فشل تصدير الملف الشخصي', allProfiles: 'كل الملفات الشخصية', showAllProfiles: 'إظهار كل الملفات الشخصية', switchToProfile: name => `التبديل إلى ${name}`, @@ -2265,7 +2271,8 @@ export const ar = defineLocale({ layoutNamePlaceholder: fallback => `اسم التخطيط (${fallback})`, saveApply: 'حفظ وتطبيق', notExpressible: 'هذا الترتيب متشابك — لا يمكن تمثيله كتقسيمات متداخلة بعد', - zoneCount: count => `${count} مناطق` + zoneCount: count => `${count} مناطق`, + tabCount: count => `${count} تبويبات` }, assistant: { thread: { diff --git a/apps/desktop/src/i18n/en.ts b/apps/desktop/src/i18n/en.ts index 2ec60d2859a19..60a8d298add6d 100644 --- a/apps/desktop/src/i18n/en.ts +++ b/apps/desktop/src/i18n/en.ts @@ -1563,6 +1563,12 @@ export const en: Translations = { search: 'Search profiles...', loading: 'Loading profiles...', newProfile: 'New profile', + importProfile: 'Import profile…', + exportProfile: 'Export profile…', + imported: 'Profile imported', + exported: 'Profile exported', + failedImport: 'Failed to import profile', + failedExport: 'Failed to export profile', allProfiles: 'All profiles', showAllProfiles: 'Show all profiles', switchToProfile: name => `Switch to ${name}`, @@ -1876,6 +1882,11 @@ export const en: Translations = { menuAddFolder: 'Add folder', menuSetActive: 'Set active', menuDelete: 'Delete', + moveToProject: 'Move to project', + movedTo: name => `Moved to ${name}`, + moveFailed: 'Could not move session', + moveNoFolder: 'That project has no folder to move into', + moveNoProjects: 'No other projects', reveal: 'Reveal in folder', copyPath: 'Copy path', removeFromSidebar: 'Hide from sidebar', @@ -2697,7 +2708,8 @@ export const en: Translations = { layoutNamePlaceholder: fallback => `Layout name (${fallback})`, saveApply: 'Save & apply', notExpressible: 'this arrangement interlocks (pinwheel) — not expressible as nested splits yet', - zoneCount: count => `${count} zones` + zoneCount: count => `${count} zones`, + tabCount: count => `${count} tabs` }, assistant: { diff --git a/apps/desktop/src/i18n/ja.ts b/apps/desktop/src/i18n/ja.ts index d1913d24d83e1..85444e123dea1 100644 --- a/apps/desktop/src/i18n/ja.ts +++ b/apps/desktop/src/i18n/ja.ts @@ -1400,6 +1400,12 @@ export const ja = defineLocale({ search: 'プロファイルを検索...', loading: 'プロファイルを読み込み中...', newProfile: '新しいプロファイル', + importProfile: 'プロファイルをインポート…', + exportProfile: 'プロファイルをエクスポート…', + imported: 'プロファイルをインポートしました', + exported: 'プロファイルをエクスポートしました', + failedImport: 'プロファイルのインポートに失敗しました', + failedExport: 'プロファイルのエクスポートに失敗しました', allProfiles: 'すべてのプロファイル', showAllProfiles: 'すべてのプロファイルを表示', switchToProfile: name => `${name} に切り替え`, @@ -2523,7 +2529,8 @@ export const ja = defineLocale({ layoutNamePlaceholder: fallback => `レイアウト名(${fallback})`, saveApply: '保存して適用', notExpressible: 'この配置は互いに噛み合っています(風車型)— 入れ子の分割では表現できません', - zoneCount: count => `${count} ゾーン` + zoneCount: count => `${count} ゾーン`, + tabCount: count => `${count} 個のタブ` }, assistant: { diff --git a/apps/desktop/src/i18n/types.ts b/apps/desktop/src/i18n/types.ts index e2a49f4234dc1..e5d1fe4bcea0c 100644 --- a/apps/desktop/src/i18n/types.ts +++ b/apps/desktop/src/i18n/types.ts @@ -1305,6 +1305,12 @@ export interface Translations { search: string loading: string newProfile: string + importProfile: string + exportProfile: string + imported: string + exported: string + failedImport: string + failedExport: string allProfiles: string showAllProfiles: string switchToProfile: (name: string) => string @@ -1573,6 +1579,11 @@ export interface Translations { menuAddFolder: string menuSetActive: string menuDelete: string + moveToProject: string + movedTo: (name: string) => string + moveFailed: string + moveNoFolder: string + moveNoProjects: string reveal: string copyPath: string removeFromSidebar: string @@ -2293,6 +2304,7 @@ export interface Translations { saveApply: string notExpressible: string zoneCount: (count: number) => string + tabCount: (count: number) => string } assistant: { diff --git a/apps/desktop/src/i18n/zh-hant.ts b/apps/desktop/src/i18n/zh-hant.ts index bb71f3d094ec3..452313c1121b8 100644 --- a/apps/desktop/src/i18n/zh-hant.ts +++ b/apps/desktop/src/i18n/zh-hant.ts @@ -1349,6 +1349,12 @@ export const zhHant = defineLocale({ search: '搜尋設定檔…', loading: '正在載入設定檔…', newProfile: '新增設定檔', + importProfile: '匯入設定檔…', + exportProfile: '匯出設定檔…', + imported: '設定檔已匯入', + exported: '設定檔已匯出', + failedImport: '匯入設定檔失敗', + failedExport: '匯出設定檔失敗', allProfiles: '全部設定檔', showAllProfiles: '顯示全部設定檔', switchToProfile: name => `切換至 ${name}`, @@ -2443,7 +2449,8 @@ export const zhHant = defineLocale({ layoutNamePlaceholder: fallback => `版面名稱(${fallback})`, saveApply: '儲存並套用', notExpressible: '此排列互相咬合(風車形)——暫時無法表示為巢狀分割', - zoneCount: count => `${count} 個區域` + zoneCount: count => `${count} 個區域`, + tabCount: count => `${count} 個分頁` }, assistant: { diff --git a/apps/desktop/src/i18n/zh.ts b/apps/desktop/src/i18n/zh.ts index 79a9a1c143eb2..e4f8cae9347a0 100644 --- a/apps/desktop/src/i18n/zh.ts +++ b/apps/desktop/src/i18n/zh.ts @@ -1757,6 +1757,12 @@ export const zh: Translations = { search: '搜索配置档案…', loading: '正在加载配置档案…', newProfile: '新建配置档案', + importProfile: '导入配置档案…', + exportProfile: '导出配置档案…', + imported: '配置档案已导入', + exported: '配置档案已导出', + failedImport: '导入配置档案失败', + failedExport: '导出配置档案失败', allProfiles: '全部配置档案', showAllProfiles: '显示全部配置档案', switchToProfile: name => `切换到 ${name}`, @@ -2070,6 +2076,11 @@ export const zh: Translations = { menuAddFolder: '添加文件夹', menuSetActive: '设为活动', menuDelete: '删除', + moveToProject: '移动到项目', + movedTo: name => `已移动到 ${name}`, + moveFailed: '无法移动会话', + moveNoFolder: '该项目没有可移入的文件夹', + moveNoProjects: '没有其他项目', reveal: '在文件夹中显示', copyPath: '复制路径', removeFromSidebar: '从侧边栏移除', @@ -2875,7 +2886,8 @@ export const zh: Translations = { layoutNamePlaceholder: fallback => `布局名称(${fallback})`, saveApply: '保存并应用', notExpressible: '此排列互相咬合(风车形)——暂无法表示为嵌套拆分', - zoneCount: count => `${count} 个区域` + zoneCount: count => `${count} 个区域`, + tabCount: count => `${count} 个标签页` }, assistant: { diff --git a/apps/desktop/src/store/profile-share.test.ts b/apps/desktop/src/store/profile-share.test.ts new file mode 100644 index 0000000000000..985564e934225 --- /dev/null +++ b/apps/desktop/src/store/profile-share.test.ts @@ -0,0 +1,126 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import type { DesktopTheme } from '@/themes/types' +import type { ProfileDesktopOverlay } from '@/types/hermes' + +// Keep side-effecting transitive imports inert (gateway sockets, REST). +vi.mock('@/store/gateway', async () => { + const { atom } = await import('nanostores') + + return { + $gateway: atom(null), + ensureGatewayForProfile: vi.fn(async () => undefined), + openGatewayForProfile: vi.fn(async () => undefined) + } +}) +vi.mock('@/hermes', () => ({ + exportProfileArchive: vi.fn(async () => ({ archive: '/tmp/out.tar.gz', ok: true })), + getProfiles: vi.fn(async () => ({ profiles: [] })), + importProfileArchive: vi.fn(async () => ({ desktop: null, name: 'imported', ok: true, path: '/tmp/p' })), + setApiRequestProfile: vi.fn() +})) +vi.mock('@/lib/query-client', () => ({ invalidateProfileScopedQueries: vi.fn() })) +vi.mock('@/store/starmap', () => ({ resetStarmapGraph: vi.fn() })) + +const { applyDesktopOverlay, buildDesktopOverlay, exportProfileBundle } = await import('./profile-share') +const { $profileColors, setProfileColor } = await import('./profile') +const { modePref, skinPref } = await import('@/themes/context') +const { $userThemes } = await import('@/themes/user-themes') +const { $layoutTree } = await import('@/components/pane-shell/tree/store') +const { exportProfileArchive } = await import('@/hermes') + +// isValidTheme only requires background/foreground/primary at runtime; the +// static type wants the full palette, hence the cast. +const roseTheme = { + name: 'rose-quartz', + label: 'Rose Quartz', + description: 'test theme', + colors: { background: '#fff0f5', foreground: '#221122', primary: '#e91e63' } +} as unknown as DesktopTheme + +beforeEach(() => { + window.localStorage.clear() + $userThemes.set({}) + $profileColors.set({}) +}) + +afterEach(() => { + vi.clearAllMocks() +}) + +describe('buildDesktopOverlay', () => { + it('snapshots skin, mode, rail color, and the layout tree for the profile', () => { + skinPref.assign('glam', 'mono') + modePref.assign('glam', 'dark') + setProfileColor('glam', '#e91e63') + + const overlay = buildDesktopOverlay('glam') + + expect(overlay.version).toBe(1) + expect(overlay.skin).toBe('mono') + expect(overlay.mode).toBe('dark') + expect(overlay.profileColor).toBe('#e91e63') + // Built-in skin → no bundled theme definitions. + expect(overlay.themes).toBeUndefined() + }) + + it('bundles the full definition of a non-built-in skin', () => { + $userThemes.set({ 'rose-quartz': roseTheme }) + skinPref.assign('glam', 'rose-quartz') + + const overlay = buildDesktopOverlay('glam') + + expect(overlay.skin).toBe('rose-quartz') + expect(overlay.themes).toEqual({ 'rose-quartz': roseTheme }) + }) +}) + +describe('applyDesktopOverlay', () => { + it('installs bundled themes and assigns skin/mode/color to the new profile', () => { + applyDesktopOverlay('glam-copy', { + version: 1, + skin: 'rose-quartz', + mode: 'dark', + themes: { 'rose-quartz': roseTheme }, + profileColor: '#e91e63' + }) + + expect($userThemes.get()['rose-quartz']).toEqual(roseTheme) + expect(skinPref.resolve('glam-copy')).toBe('rose-quartz') + expect(modePref.resolve('glam-copy')).toBe('dark') + expect($profileColors.get()['glam-copy']).toBe('#e91e63') + }) + + it('ignores a skin that resolves to nothing and junk layout trees', () => { + const before = $layoutTree.get() + + applyDesktopOverlay('glam-copy', { + skin: 'no-such-skin', + layoutTree: { bogus: true } + } as ProfileDesktopOverlay) + + // Unresolvable skin → pref falls back to the default resolution. + expect(skinPref.resolve('glam-copy')).toBe(skinPref.resolve('some-unassigned')) + expect($layoutTree.get()).toBe(before) + }) + + it('is a no-op for a plain CLI archive (no overlay)', () => { + expect(() => applyDesktopOverlay('glam-copy', null)).not.toThrow() + expect(() => applyDesktopOverlay('glam-copy', undefined)).not.toThrow() + }) +}) + +describe('exportProfileBundle', () => { + it('stages desktop.json into the archive through extra_files', async () => { + skinPref.assign('glam', 'mono') + + const archive = await exportProfileBundle('glam', '/tmp/glam.tar.gz') + + expect(archive).toBe('/tmp/out.tar.gz') + const call = vi.mocked(exportProfileArchive).mock.calls[0] + expect(call[0]).toBe('glam') + const overlay = JSON.parse(call[1]?.extraFiles?.['desktop.json'] ?? '{}') as ProfileDesktopOverlay + expect(overlay.skin).toBe('mono') + expect(call[1]?.output).toBe('/tmp/glam.tar.gz') + }) +}) diff --git a/apps/desktop/src/store/profile-share.ts b/apps/desktop/src/store/profile-share.ts new file mode 100644 index 0000000000000..ee5d052871287 --- /dev/null +++ b/apps/desktop/src/store/profile-share.ts @@ -0,0 +1,219 @@ +/** + * Profile share: export/import a profile as a portable bundle. + * + * The archive is the CLI's own `hermes profile export` tar.gz (config, skills, + * SOUL.md, cron — credentials always excluded), plus one desktop-only file at + * the root: `desktop.json`, the appearance/interface overlay (skin + mode, + * any user-theme definitions the skin needs, the profile rail color, and the + * layout tree). A CLI import of the same archive simply carries the file + * along; the desktop import applies it so the receiving user gets the whole + * look — theme, layout, skills — as a ready-to-use profile. + * + * Paths, not bytes, cross the renderer↔backend boundary: the native save/open + * dialogs and the backend share the filesystem for local and pooled backends. + */ + +import { isLayoutNode, normalize } from '@/components/pane-shell/tree/model' +import { $layoutTree, markActivePreset, persistTree } from '@/components/pane-shell/tree/store' +import { exportProfileArchive, importProfileArchive } from '@/hermes' +import { translateNow } from '@/i18n' +import { modePref, skinPref, type ThemeMode } from '@/themes/context' +import { BUILTIN_THEMES } from '@/themes/presets' +import type { DesktopTheme } from '@/themes/types' +import { $userThemes, installUserTheme, resolveTheme } from '@/themes/user-themes' +import type { ProfileDesktopOverlay } from '@/types/hermes' + +import { notify, notifyError } from './notifications' +import { + $activeGatewayProfile, + $profileColors, + normalizeProfileKey, + refreshActiveProfile, + selectProfile, + setProfileColor +} from './profile' + +/** Filename of the overlay inside the archive (profile root). */ +export const DESKTOP_OVERLAY_FILENAME = 'desktop.json' + +const OVERLAY_VERSION = 1 + +/** + * Snapshot the desktop appearance/interface for `profile` into the overlay. + * The layout tree is global (one window layout, not per-profile) — it rides + * along so the receiver can opt into the sender's whole interface. + */ +export function buildDesktopOverlay(profile: string): ProfileDesktopOverlay { + const key = normalizeProfileKey(profile) + const skin = skinPref.resolve(key) + const mode = modePref.resolve(key) + + // Bundle the full definition of any non-built-in theme the skin points at, + // so the receiver's picker can resolve it. Built-ins resolve by name. + const themes: Record = {} + const userTheme = BUILTIN_THEMES[skin] ? undefined : $userThemes.get()[skin] + + if (userTheme) { + themes[userTheme.name] = userTheme + } + + return { + version: OVERLAY_VERSION, + skin, + mode, + ...(Object.keys(themes).length ? { themes } : {}), + profileColor: $profileColors.get()[key] ?? null, + layoutTree: $layoutTree.get() + } +} + +/** Export `profile` (backend archive + desktop overlay) to `output` (or the + * backend's staging dir when omitted). Returns the archive path. */ +export async function exportProfileBundle(profile: string, output?: string): Promise { + const overlay = buildDesktopOverlay(profile) + + const { archive } = await exportProfileArchive(profile, { + extraFiles: { [DESKTOP_OVERLAY_FILENAME]: JSON.stringify(overlay, null, 2) }, + output + }) + + return archive +} + +const isThemeMode = (value: unknown): value is ThemeMode => + value === 'light' || value === 'dark' || value === 'system' + +/** + * Apply an imported overlay: install bundled themes, assign the new profile's + * skin + mode + rail color, and (when present) adopt the sender's layout tree. + * Every step is independent and best-effort — a malformed half never blocks + * the rest, and a missing overlay is a plain CLI-exported archive (no-op). + */ +export function applyDesktopOverlay(profile: string, overlay: null | ProfileDesktopOverlay | undefined): void { + if (!overlay || typeof overlay !== 'object') { + return + } + + const key = normalizeProfileKey(profile) + + // 1. Bundled theme definitions. installUserTheme validates shape and refuses + // built-in collisions; a bad entry just doesn't install. + for (const theme of Object.values(overlay.themes ?? {})) { + try { + installUserTheme(theme as DesktopTheme) + } catch { + // Invalid/colliding theme — the skin assignment below falls back. + } + } + + // 2. Appearance assignment for the new profile. Only assign a skin that + // actually resolves so the pref never points at nothing. + if (typeof overlay.skin === 'string' && resolveTheme(overlay.skin)) { + skinPref.assign(key, overlay.skin) + } + + if (isThemeMode(overlay.mode)) { + modePref.assign(key, overlay.mode) + } + + // 3. Rail color. + if (typeof overlay.profileColor === 'string' && overlay.profileColor) { + setProfileColor(key, overlay.profileColor) + } + + // 4. Layout tree — global by design (one window layout). Normalize through + // the same canonicalizer the boot load uses; a null result means the + // tree was junk, so the current layout stays. + if (overlay.layoutTree != null && isLayoutNode(overlay.layoutTree)) { + const tree = normalize(overlay.layoutTree) + + if (tree) { + $layoutTree.set(tree) + persistTree() + markActivePreset('custom') + } + } +} + +/** Import an archive, apply its desktop overlay, return the new profile name. */ +export async function importProfileBundle(archive: string, name?: string): Promise { + const result = await importProfileArchive(archive, name) + applyDesktopOverlay(result.name, result.desktop) + + return result.name +} + +/** The profile the export pickers should default to — the active one. */ +export function activeProfileKey(): string { + return normalizeProfileKey($activeGatewayProfile.get()) +} + +// ── Dialog-driven flows ────────────────────────────────────────────────────── +// One store function per user verb (⌘K row, rail button, and any future menu +// item all funnel here). Toasts via the shared notification store; strings via +// translateNow so the flows stay callable from non-React surfaces. + +const ARCHIVE_FILTERS = [{ extensions: ['tar.gz', 'tgz'], name: 'Hermes profile' }] + +/** Pick a save location and export `profile` (default: the active one). + * Returns the archive path, or null when the user cancelled. */ +export async function runExportProfileFlow(profile?: string): Promise { + const target = normalizeProfileKey(profile ?? activeProfileKey()) + const pick = window.hermesDesktop?.selectSavePath + + if (!pick) { + return null + } + + const output = await pick({ + title: translateNow('profiles.exportProfile'), + defaultPath: `${target}.tar.gz`, + filters: ARCHIVE_FILTERS + }) + + if (!output) { + return null + } + + try { + const archive = await exportProfileBundle(target, output) + notify({ kind: 'success', title: translateNow('profiles.exported'), message: archive }) + + return archive + } catch (error) { + notifyError(error, translateNow('profiles.failedExport')) + + return null + } +} + +/** Pick an archive and import it as a new profile; lands the user in it on a + * fresh chat. Returns the new profile name, or null when cancelled/failed. */ +export async function runImportProfileFlow(): Promise { + const paths = await window.hermesDesktop?.selectPaths?.({ + title: translateNow('profiles.importProfile'), + multiple: false, + filters: ARCHIVE_FILTERS + }) + + const archive = paths?.[0] + + if (!archive) { + return null + } + + try { + const name = await importProfileBundle(archive) + notify({ kind: 'success', title: translateNow('profiles.imported'), message: name }) + // Same landing as CreateProfileDialog's onCreated: refresh the list, then + // switch into the new profile on a fresh chat. + await refreshActiveProfile() + selectProfile(name) + + return name + } catch (error) { + notifyError(error, translateNow('profiles.failedImport')) + + return null + } +} diff --git a/apps/desktop/src/store/projects.ts b/apps/desktop/src/store/projects.ts index 2ba0d7e59bbc9..6bb3451a88ba7 100644 --- a/apps/desktop/src/store/projects.ts +++ b/apps/desktop/src/store/projects.ts @@ -22,6 +22,7 @@ import { $sessions, idsShareLineage, sessionMatchesStoredId, + setSessions, workspaceCwdForNewSession } from '@/store/session' import { $focusedSessionState, $focusedStoredSessionId } from '@/store/session-states' @@ -173,7 +174,7 @@ export function exitProjectScope(): void { // one. Empty for the path-less Home bucket. (The sidebar's `projectTreeCwd` is // the same rule over the same tree — this is the store-side copy so the store // doesn't reach into the sidebar's React module.) -const projectRootCwd = (project: SidebarProjectTree | undefined): string => +export const projectRootCwd = (project: SidebarProjectTree | undefined): string => (project?.path || project?.repos.find(repo => repo.path)?.path || '').trim() // ⌘K "go to project": flip the sidebar into grouped mode and enter the project @@ -520,6 +521,45 @@ export async function fetchProjectSessions(projectId: string): Promise { + const cwd = projectRootCwd($projectTree.get().find(node => node.id === projectId)) + + if (!cwd) { + throw new Error(translateNow('sidebar.projects.moveNoFolder')) + } + + const res = await gatewayRequest('session.workspace.move', { + cwd, + session_key: sessionId, + ...(profile ? { profile } : {}) + }) + + const moved = res.cwd || cwd + setSessions(prev => + prev.map(s => + sessionMatchesStoredId(s, sessionId) + ? { ...s, cwd: moved, git_branch: res.branch ?? null, git_repo_root: res.git_repo_root ?? null } + : s + ) + ) + void refreshProjectTree() +} + export interface RepoDiscoveryPolicy { enabled: boolean roots: string[] diff --git a/apps/desktop/src/types/hermes.ts b/apps/desktop/src/types/hermes.ts index bc1fb5c581bf0..e468d21d12a4b 100644 --- a/apps/desktop/src/types/hermes.ts +++ b/apps/desktop/src/types/hermes.ts @@ -871,6 +871,26 @@ export interface ProfileSetupCommand { command: string } +// The desktop appearance/interface overlay bundled into a profile export as +// `desktop.json`. Everything optional — an archive exported by an older (or +// non-desktop) Hermes simply carries none of it. See store/profile-share.ts. +export interface ProfileDesktopOverlay { + /** Overlay schema version (1). */ + version?: number + /** Skin name (built-in or bundled user theme). */ + skin?: string + /** Light/dark/system preference. */ + mode?: string + /** Full user-theme definitions the skin may reference (DesktopTheme JSON). */ + themes?: Record + /** Rail color override for this profile. */ + profileColor?: null | string + /** Layout tree (hermes.desktop.layoutTree.v2 shape). */ + layoutTree?: unknown + /** Active layout preset id. */ + layoutPreset?: string +} + // ── Projects ─────────────────────────────────────────────────────────────── // A first-class, per-profile, human-named workspace spanning one or more // folders. Mirrors hermes_cli/projects_db.Project.to_dict(). diff --git a/cli.py b/cli.py index aed3992922b4a..9583383f61d99 100644 --- a/cli.py +++ b/cli.py @@ -10249,6 +10249,10 @@ def process_command(self, command: str) -> bool: self._handle_rollback_command(cmd_original) elif canonical == "snapshot": self._handle_snapshot_command(cmd_original) + elif canonical == "export": + self._handle_export_command(cmd_original) + elif canonical == "import": + self._handle_import_command(cmd_original) elif canonical == "stop": self._handle_stop_command() elif canonical == "agents": diff --git a/docs/observability/relay-shared-metrics.md b/docs/observability/relay-shared-metrics.md index c32430c21572b..9454598d6b349 100644 --- a/docs/observability/relay-shared-metrics.md +++ b/docs/observability/relay-shared-metrics.md @@ -55,11 +55,12 @@ dependency does not change the collection or privacy policy. ## Current Slices -The current vertical slices record logical model calls and top-level task runs: +The current vertical slices record logical model calls, top-level task runs, +and tool and approval outcomes: ```text -Hermes turn, API, and tool hooks - -> Relay session, task, and LLM lifecycle +Hermes turn, API, tool, and approval hooks + -> Relay session, task, LLM, tool, and mark lifecycle -> Hermes shared-metrics subscriber -> SQLite counters -> immutable JSON delta package @@ -90,6 +91,21 @@ boundary closes the task for normal returns, early returns, exceptions, and cancellations. Active task ownership follows the task ID if Hermes rotates its conversation session during context compression. +Each tool invocation is represented by a Relay tool lifecycle named +`hermes.tool_call`. The terminal counter contains only bounded tool category, +outcome, approval outcome, latency, and explicit retry-count buckets. Hermes +derives the category from the toolset already declared in its runtime registry; +custom and unrecognized toolsets collapse to `other` rather than exporting +tool or plugin names. Hermes does not infer retries from repeated tool names or +adjacent calls; when the +hook does not provide an explicit retry relationship, the retry bucket is +`unknown`. Approval decisions are emitted as `hermes.tool_approval` marks and +recorded as attributed to a tool call or explicitly `unattributed`. Tool names, +call IDs, arguments, results, commands, descriptions, and error text are not +included in shared-metrics events or packages. A started tool that is still +open when its task terminates is closed as failed, timed out, or cancelled and +remains in the task's tool-count bucket. + Local state is written under: ```text @@ -131,6 +147,8 @@ The script uses the installed `nemo-relay` dependency by default. Pass `--relay-python ../nemo-relay/python` only when testing a locally built Relay binding. -The smoke verifies the model request reached the local server, model and task -counters were stored with the expected model and provider, one package was -exported, and prompt and response canaries are absent from the package. +The smoke has the local model request a real `read_file` tool call before its +final response. It verifies model, provider, task, and bounded tool counters in +SQLite, validates the exported package against the closed schema, and checks +that prompt, response, tool-call ID, and tool-result canaries are absent from +the package. diff --git a/hermes_cli/cli_commands_mixin.py b/hermes_cli/cli_commands_mixin.py index 5ec16a5fe4df0..9f9071ea94ef0 100644 --- a/hermes_cli/cli_commands_mixin.py +++ b/hermes_cli/cli_commands_mixin.py @@ -363,6 +363,80 @@ def _handle_snapshot_command(self, command: str): print(f" Unknown subcommand: {subcmd}") print(" Usage: /snapshot [list|create [label]|restore |prune [N]]") + def _handle_export_command(self, command: str): + """Handle /export — export a profile to a shareable .tar.gz archive. + + Syntax: + /export — export the active profile + /export — export a named profile + /export [profile] -o — choose the output path + """ + from hermes_cli.profiles import export_profile, get_active_profile_name + + parts = command.split()[1:] + output = None + if "-o" in parts: + idx = parts.index("-o") + if idx + 1 >= len(parts): + print(" Usage: /export [profile] [-o output.tar.gz]") + return + output = parts[idx + 1] + parts = parts[:idx] + parts[idx + 2:] + + name = parts[0] if parts else (get_active_profile_name() or "default") + if not output: + output = f"{name}.tar.gz" + + try: + result = export_profile(name, output) + print(f" ✓ Exported '{name}' to {result}") + print(" Share it: the other user runs /import or `hermes profile import `.") + except (ValueError, FileNotFoundError) as e: + print(f" Error: {e}") + + def _handle_import_command(self, command: str): + """Handle /import — import a shared profile archive as a new profile. + + Syntax: + /import [--name ] + """ + from hermes_cli.profiles import ( + check_alias_collision, create_wrapper_script, import_profile, + ) + + parts = command.split()[1:] + name = None + if "--name" in parts: + idx = parts.index("--name") + if idx + 1 >= len(parts): + print(" Usage: /import [--name ]") + return + name = parts[idx + 1] + parts = parts[:idx] + parts[idx + 2:] + + if not parts: + print(" Usage: /import [--name ]") + return + + archive = " ".join(parts) # paths may contain spaces + + try: + profile_dir = import_profile(archive, name=name) + except (ValueError, FileExistsError, FileNotFoundError) as e: + print(f" Error: {e}") + return + + imported = profile_dir.name + print(f" ✓ Imported profile '{imported}' at {profile_dir}") + try: + if not check_alias_collision(imported): + wrapper_path = create_wrapper_script(imported) + if wrapper_path: + print(f" Wrapper created: {wrapper_path}") + except Exception: + pass + print(f" Use it: hermes -p {imported}") + def _handle_stop_command(self): """Handle /stop — kill all running background processes and background (async) delegations. diff --git a/hermes_cli/commands.py b/hermes_cli/commands.py index 34280abc2464f..a803ee925f30e 100644 --- a/hermes_cli/commands.py +++ b/hermes_cli/commands.py @@ -133,6 +133,10 @@ class CommandDef: args_hint="[number]"), CommandDef("snapshot", "Create or restore state snapshots of Hermes config/state", "Session", cli_only=True, aliases=("snap",), args_hint="[create|restore |prune]"), + CommandDef("export", "Export a profile (config, skills, theme) to a shareable archive", "Configuration", + cli_only=True, args_hint="[profile] [-o output.tar.gz]"), + CommandDef("import", "Import a shared profile archive as a new profile", "Configuration", + cli_only=True, args_hint=" [--name ]"), CommandDef("stop", "Kill all running background processes", "Session", busy_policy="interrupt_then_dispatch", busy_handler="stop"), CommandDef("approve", "Approve a pending dangerous command", "Session", diff --git a/hermes_cli/model_normalize.py b/hermes_cli/model_normalize.py index 041aa47f0db7b..d2dfe132a8a41 100644 --- a/hermes_cli/model_normalize.py +++ b/hermes_cli/model_normalize.py @@ -109,6 +109,19 @@ "xai", }) +# Providers whose API serves ``vendor/model`` ids but whose endpoint can also +# front arbitrary self-hosted models, so a bare name cannot be prefixed +# blindly. A bare id is repaired only when the curated catalogue for that +# provider holds exactly one entry ending in ``/`` — a lookup, not a +# guess. NVIDIA NIM is the case in hand: build.nvidia.com serves +# ``nvidia/nemotron-…`` (and third-party ``z-ai/glm-…``), while the same +# provider id also points at local NIM containers with their own naming. +# Without this repair a bare ``nemotron-3-ultra-550b-a55b`` reaches the API +# and returns a bare ``404 page not found`` that never names the model (#78796). +_CATALOGUE_PREFIX_REPAIR_PROVIDERS: frozenset[str] = frozenset({ + "nvidia", +}) + # Providers whose APIs require lowercase model IDs. Xiaomi's # ``api.xiaomimimo.com`` rejects mixed-case names like ``MiMo-V2.5-Pro`` # that users might copy from marketing docs — it only accepts @@ -350,6 +363,63 @@ def _prepend_vendor(model_name: str) -> str: return model_name +def _repair_prefix_from_catalogue(model_name: str, provider: str) -> str: + """Restore a dropped ``vendor/`` prefix using the provider's catalogue. + + Unlike :func:`_prepend_vendor`, this never guesses from the model's name + shape — it only repairs a bare id that matches **exactly one** curated + entry for this provider modulo the prefix. That keeps self-hosted models + behind the same provider id (local NIM containers, proxies) untouched, + since they aren't in the catalogue. + + Examples:: + + >>> _repair_prefix_from_catalogue("nemotron-3-ultra-550b-a55b", "nvidia") + 'nvidia/nemotron-3-ultra-550b-a55b' + >>> _repair_prefix_from_catalogue("my-local-nim", "nvidia") + 'my-local-nim' + """ + if "/" in model_name: + return model_name + try: + from hermes_cli.models import _PROVIDER_MODELS + except Exception: + return model_name + + catalogue = _PROVIDER_MODELS.get(provider) or [] + # Compare against the catalogue's own suffix, tag included: a bare + # ``…:free`` id must resolve to the ``:free`` entry, not its paid sibling. + needle = model_name.strip().lower() + matches = { + entry + for entry in catalogue + if "/" in entry and entry.split("/", 1)[1].strip().lower() == needle + } + if len(matches) == 1: + return matches.pop() + return model_name + + +def suggest_prefixed_model_id(provider: str, model_name: str) -> Optional[str]: + """Return the prefixed catalogue id for a bare *model_name*, if unambiguous. + + The diagnostic counterpart to :func:`_repair_prefix_from_catalogue`: used + to explain a provider's content-free 404 when the configured id lost its + ``vendor/`` prefix. Returns ``None`` when the name already has a prefix, + the provider has no curated catalogue, or nothing matches — so callers can + stay silent rather than guess (#78796). + """ + name = (model_name or "").strip() + if not name or "/" in name: + return None + try: + canonical = _normalize_provider_alias(provider) + except Exception: + return None + repaired = _repair_prefix_from_catalogue(name, canonical) + return repaired if repaired != name else None + + # --------------------------------------------------------------------------- # Main normalisation entry point # --------------------------------------------------------------------------- @@ -492,6 +562,12 @@ def normalize_model_for_provider(model_input: str, target_provider: str) -> str: result = result.lower() return result + # --- Catalogue-backed prefix repair: restore a dropped ``vendor/`` on a + # bare id that matches exactly one curated entry. Unknown names (a + # local NIM container, a proxied model) pass through untouched. --- + if provider in _CATALOGUE_PREFIX_REPAIR_PROVIDERS: + return _repair_prefix_from_catalogue(name, provider) + # --- Authoritative native providers: preserve user-facing slugs as-is --- if provider in _AUTHORITATIVE_NATIVE_PROVIDERS: return name diff --git a/hermes_cli/observability/relay_shared_metrics.py b/hermes_cli/observability/relay_shared_metrics.py index bd603dd0eba66..d391344a6fda7 100644 --- a/hermes_cli/observability/relay_shared_metrics.py +++ b/hermes_cli/observability/relay_shared_metrics.py @@ -6,6 +6,7 @@ import contextvars import logging import threading +from collections import deque from dataclasses import dataclass, field from time import monotonic_ns from typing import Any, Callable @@ -21,9 +22,15 @@ SCHEMA_VERSION, SUBSCRIBER_NAME, TASK_SCOPE, + TOOL_APPROVAL_MARK, + TOOL_CALL_SCOPE, model_call_fields, task_start_fields, task_terminal_fields, + task_terminal_state, + tool_approval_outcome, + tool_category, + tool_terminal_fields, ) from .shared_metrics_subscriber import SharedMetricsSubscriber @@ -36,7 +43,9 @@ "on_session_reset", "pre_llm_call", "pre_api_request", + "pre_tool_call", "post_tool_call", + "post_approval_response", "post_api_request", "api_request_error", "subagent_stop", @@ -62,15 +71,27 @@ class _ModelCall: retry_ordinal: int | None = None +@dataclass +class _ToolCall: + handle: Any + task_id: str + category: str + started_ns: int + approval_outcome: str = "not_required" + + @dataclass class _TaskRun: + task_id: str handle: Any context: contextvars.Context started_ns: int start_fields: dict[str, str] model_call_ids: set[str] = field(default_factory=set) - tool_call_ids: set[str] = field(default_factory=set) + tool_call_ids: set[tuple[str, str, str]] = field(default_factory=set) turn_ids: set[str] = field(default_factory=set) + retired_turn_ids: frozenset[str] = field(default_factory=frozenset) + completed_tool_call_ids: set[tuple[str, str, str]] = field(default_factory=set) unidentified_tool_calls: int = 0 retry_count: int = 0 @@ -83,6 +104,12 @@ class _MetricsSession: closing: bool = False model_calls: dict[tuple[str, str], _ModelCall] = field(default_factory=dict) tasks: dict[str, _TaskRun] = field(default_factory=dict) + tool_calls: dict[tuple[str, str, str, str], _ToolCall] = field( + default_factory=dict + ) + retired_turn_ids: deque[str] = field( + default_factory=lambda: deque(maxlen=256), + ) class _Runtime: @@ -169,7 +196,10 @@ def start_task(self, event: dict[str, Any]) -> _TaskRun | None: if session is None: return None with session.lock: - if session.closing or session.relay_session.context is None: + if ( + session.closing + or session.relay_session.context is None + ): return None task_context = session.relay_session.context.copy() start_fields = task_start_fields(event) @@ -195,10 +225,12 @@ def push_task() -> Any: handle = task_context.run(push_task) task = _TaskRun( + task_id=task_id, handle=handle, context=task_context, started_ns=monotonic_ns(), start_fields=start_fields, + retired_turn_ids=frozenset(session.retired_turn_ids), ) session.tasks[task_id] = task with self._task_sessions_lock: @@ -306,8 +338,8 @@ def record_model_call_error(self, event: dict[str, Any]) -> None: return model_call.fields = model_call_fields(event) - def record_tool_call(self, event: dict[str, Any]) -> None: - """Count one unique tool invocation under its owning task.""" + def start_tool_call(self, event: dict[str, Any]) -> None: + """Open one privacy-safe Relay tool lifecycle under its task.""" task_id = str(event.get("task_id") or "") session = self._task_session(event, allow_task_id_fallback=True) task = session.tasks.get(task_id) if session is not None else None @@ -317,14 +349,121 @@ def record_tool_call(self, event: dict[str, Any]) -> None: if session is None or task is None: return tool_call_id = str(event.get("tool_call_id") or "") + if not tool_call_id: + return + identity = self._tool_call_identity(event) + with session.lock: + if session.closing: + return + if not self._event_matches_task_turn(task, event): + return + self._remember_turn(session, task, event) + key = (task_id, *identity) + if identity in task.completed_tool_call_ids or key in session.tool_calls: + return + task.tool_call_ids.add(identity) + session.tool_calls[key] = self._open_tool_call(task, event) + + def record_approval(self, event: dict[str, Any]) -> None: + """Record one bounded approval result without approval text or commands.""" + session, task = self._approval_task(event) + if session is None or task is None: + return + outcome = tool_approval_outcome(event) + tool_call_id = str(event.get("tool_call_id") or "") + attribution = "unattributed" + with session.lock: + if session.closing: + return + if not self._event_matches_task_turn(task, event): + return + if tool_call_id: + identity = self._tool_call_identity(event) + tool_call = session.tool_calls.get((task.task_id, *identity)) + if tool_call is None: + matching_keys = [ + key + for key in session.tool_calls + if key[0] == task.task_id + and self._tool_call_identities_are_compatible( + key[1:], + identity, + ) + ] + tool_call = ( + session.tool_calls[matching_keys[0]] + if len(matching_keys) == 1 + else None + ) + if tool_call is not None: + tool_call.approval_outcome = outcome + attribution = "tool_call" + self._run_in_task( + task, + self.relay.scope.event, + TOOL_APPROVAL_MARK, + handle=task.handle, + data={"attribution": attribution, "outcome": outcome}, + metadata=self._event_metadata(), + ) + + def record_tool_call(self, event: dict[str, Any]) -> None: + """Close and count one unique privacy-safe tool lifecycle.""" + task_id = str(event.get("task_id") or "") + session = self._task_session(event, allow_task_id_fallback=True) + task = session.tasks.get(task_id) if session is not None else None + if session is None or task is None: + return + tool_call_id = str(event.get("tool_call_id") or "") with session.lock: if session.closing: return + if not self._event_matches_task_turn(task, event): + return self._remember_turn(session, task, event) if tool_call_id: - task.tool_call_ids.add(tool_call_id) + observed_identity = self._tool_call_identity(event) + if observed_identity in task.completed_tool_call_ids: + return + identity = observed_identity + tool_call = session.tool_calls.pop((task_id, *identity), None) + if tool_call is None: + if any( + self._tool_call_identities_are_compatible( + completed_identity, + observed_identity, + ) + for completed_identity in task.completed_tool_call_ids + ): + return + matching_keys = [ + key + for key in session.tool_calls + if key[0] == task_id + and self._tool_call_identities_are_compatible( + key[1:], + observed_identity, + ) + ] + if len(matching_keys) > 1: + # Partial context cannot safely choose between + # concurrent calls that reused the provider-local ID. + return + if matching_keys: + key = matching_keys[0] + identity = key[1:] + tool_call = session.tool_calls.pop(key) + task.completed_tool_call_ids.update({ + identity, + observed_identity, + }) + task.tool_call_ids.add(identity) else: task.unidentified_tool_calls += 1 + tool_call = None + if tool_call is None: + tool_call = self._open_tool_call(task, event) + self._finish_tool_call(task, tool_call, event) def end_model_call(self, event: dict[str, Any]) -> None: session = self._task_session(event, allow_task_id_fallback=True) @@ -546,6 +685,162 @@ def _remember_turn( with self._task_sessions_lock: self._turn_sessions[(session.session_id, turn_id)] = session + @staticmethod + def _tool_call_identity(event: dict[str, Any]) -> tuple[str, str, str]: + """Identify one provider-local tool call without exporting its IDs.""" + return ( + str(event.get("api_request_id") or ""), + str(event.get("turn_id") or ""), + str(event.get("tool_call_id") or ""), + ) + + @staticmethod + def _tool_call_identities_are_compatible( + candidate: tuple[str, str, str], + observed: tuple[str, str, str], + ) -> bool: + """Match partial hook context without crossing known call boundaries.""" + if not observed[2] or candidate[2] != observed[2]: + return False + return all( + not candidate_value + or not observed_value + or candidate_value == observed_value + for candidate_value, observed_value in zip( + candidate[:2], + observed[:2], + strict=True, + ) + ) + + @staticmethod + def _event_matches_task_turn( + task: _TaskRun, + event: dict[str, Any], + ) -> bool: + """Reject delayed hooks from a prior run that reused the task ID.""" + turn_id = str(event.get("turn_id") or "") + if not turn_id: + return True + if turn_id in task.retired_turn_ids: + return False + return not task.turn_ids or turn_id in task.turn_ids + + def _approval_task( + self, + event: dict[str, Any], + ) -> tuple[_MetricsSession | None, _TaskRun | None]: + """Resolve approval correlation without guessing across ambiguous turns.""" + active = relay_runtime.active_turn() + if active is not None: + correlated = { + **event, + "session_id": active.lease.session_id, + "task_id": active.task_id, + } + session = self._task_session(correlated) + task = session.tasks.get(active.task_id) if session is not None else None + if task is not None: + return session, task + + session = self._task_session(event) + task_id = str(event.get("task_id") or "") + task = session.tasks.get(task_id) if session is not None else None + if task is not None: + return session, task + + turn_id = str(event.get("turn_id") or "") + if not turn_id: + return None, None + with self._task_sessions_lock: + candidates = [ + candidate + for ( + candidate_session_id, + candidate_turn_id, + ), candidate in self._turn_sessions.items() + if candidate_turn_id == turn_id + and self._sessions.get(candidate_session_id) is candidate + ] + unique_sessions = {id(candidate): candidate for candidate in candidates} + if len(unique_sessions) != 1: + return None, None + session = next(iter(unique_sessions.values())) + matching_tasks = [ + candidate + for candidate in session.tasks.values() + if turn_id in candidate.turn_ids + ] + if len(matching_tasks) != 1: + return None, None + return session, matching_tasks[0] + + def _open_tool_call( + self, + task: _TaskRun, + event: dict[str, Any], + ) -> _ToolCall: + handle = self._run_in_task( + task, + self.relay.tools.call, + TOOL_CALL_SCOPE, + {}, + handle=task.handle, + metadata=self._event_metadata(), + ) + return _ToolCall( + handle=handle, + task_id=task.task_id, + category=tool_category(event), + started_ns=monotonic_ns(), + ) + + def _finish_tool_call( + self, + task: _TaskRun, + tool_call: _ToolCall, + event: dict[str, Any], + ) -> None: + fields = tool_terminal_fields( + event, + category=tool_call.category, + approval_outcome=tool_call.approval_outcome, + fallback_duration_ms=max( + 0, + (monotonic_ns() - tool_call.started_ns) // 1_000_000, + ), + ) + try: + self._run_in_task( + task, + self.relay.tools.call_end, + tool_call.handle, + fields, + metadata=self._event_metadata(), + ) + except Exception: + logger.warning( + "Hermes shared-metrics tool call close failed", + exc_info=True, + ) + + def _end_pending_tool_calls( + self, + session: _MetricsSession, + task: _TaskRun, + event: dict[str, Any], + ) -> None: + pending_keys = [key for key in session.tool_calls if key[0] == task.task_id] + task_outcome, _, _ = task_terminal_state(event) + status = { + "cancelled": "cancelled", + "timed_out": "timeout", + }.get(task_outcome, "error") + for key in pending_keys: + tool_call = session.tool_calls.pop(key, None) + if tool_call is not None: + self._finish_tool_call(task, tool_call, {**event, "status": status}) + def _finish_model_call( self, session: _MetricsSession, @@ -628,6 +923,7 @@ def _finish_task( task = session.tasks.get(task_id) if task is None: return False + self._end_pending_tool_calls(session, task, event) self._end_pending_model_calls(session, {**event, "task_id": task_id}) fields = task_terminal_fields( {**task.start_fields, **event}, @@ -648,6 +944,7 @@ def _finish_task( logger.warning("Hermes shared-metrics task close failed", exc_info=True) finally: session.tasks.pop(task_id, None) + session.retired_turn_ids.extend(task.turn_ids) with self._task_sessions_lock: task_key = (session.session_id, task_id) if self._task_sessions.get(task_key) is session: @@ -697,8 +994,7 @@ def enabled() -> bool: telemetry.get("shared_metrics") if isinstance(telemetry, dict) else None ) value = ( - isinstance(shared_metrics, dict) - and shared_metrics.get("enabled") is True + isinstance(shared_metrics, dict) and shared_metrics.get("enabled") is True ) if value: return True @@ -729,8 +1025,12 @@ def observe_lifecycle(hook_name: str, **kwargs: Any) -> None: runtime.start_task(kwargs) elif hook_name == "pre_api_request": runtime.start_model_call(kwargs) + elif hook_name == "pre_tool_call": + runtime.start_tool_call(_with_runtime_toolset(kwargs)) elif hook_name == "post_tool_call": - runtime.record_tool_call(kwargs) + runtime.record_tool_call(_with_runtime_toolset(kwargs)) + elif hook_name == "post_approval_response": + runtime.record_approval(kwargs) elif hook_name == "post_api_request": runtime.end_model_call(kwargs) elif hook_name == "api_request_error": @@ -749,6 +1049,22 @@ def observe_lifecycle(hook_name: str, **kwargs: Any) -> None: ) +def _with_runtime_toolset(event: dict[str, Any]) -> dict[str, Any]: + """Attach the toolset already declared by Hermes's runtime registry.""" + if event.get("toolset"): + return event + tool_name = str(event.get("tool_name") or "") + if not tool_name: + return event + try: + from model_tools import get_toolset_for_tool + + toolset = get_toolset_for_tool(tool_name) + except Exception: + toolset = None + return {**event, "toolset": toolset or "other"} + + def prepare_session_start() -> None: """Register the subscriber before any producer opens the session scope.""" if enabled(): diff --git a/hermes_cli/observability/schemas/hermes.shared_metrics.v2.schema.json b/hermes_cli/observability/schemas/hermes.shared_metrics.v2.schema.json index 112b9f3ce8698..da8e7c78d0c0f 100644 --- a/hermes_cli/observability/schemas/hermes.shared_metrics.v2.schema.json +++ b/hermes_cli/observability/schemas/hermes.shared_metrics.v2.schema.json @@ -67,6 +67,12 @@ }, { "$ref": "#/$defs/task_finished_counter" + }, + { + "$ref": "#/$defs/tool_call_counter" + }, + { + "$ref": "#/$defs/tool_approval_counter" } ] } @@ -380,6 +386,159 @@ "6_to_10", "gte_11" ] + }, + "tool_latency_bucket": { + "enum": [ + "100ms_to_250ms", + "10s_to_30s", + "1s_to_2s", + "250ms_to_500ms", + "2s_to_5s", + "500ms_to_1s", + "5s_to_10s", + "gte_30s", + "lt_100ms", + "unknown" + ] + }, + "tool_retry_bucket": { + "enum": [ + "0", + "1", + "2", + "3_to_5", + "6_to_10", + "gte_11", + "unknown" + ] + }, + "tool_call_counter": { + "type": "object", + "additionalProperties": false, + "required": [ + "name", + "type", + "dimensions", + "value" + ], + "properties": { + "name": { + "const": "hermes.tool_call.count" + }, + "type": { + "const": "counter" + }, + "dimensions": { + "type": "object", + "additionalProperties": false, + "required": [ + "approval_outcome", + "latency_bucket", + "outcome", + "retry_count_bucket", + "tool_category" + ], + "properties": { + "approval_outcome": { + "enum": [ + "approved", + "denied", + "not_required", + "timed_out", + "unknown" + ] + }, + "latency_bucket": { + "$ref": "#/$defs/tool_latency_bucket" + }, + "outcome": { + "enum": [ + "blocked", + "cancelled", + "failed", + "success", + "timed_out", + "unknown" + ] + }, + "retry_count_bucket": { + "$ref": "#/$defs/tool_retry_bucket" + }, + "tool_category": { + "enum": [ + "browser", + "code_execution", + "communication", + "computer_use", + "delegation", + "file", + "home_automation", + "mcp", + "media", + "memory", + "other", + "planning", + "project", + "scheduler", + "skill", + "terminal", + "unknown", + "web" + ] + } + } + }, + "value": { + "type": "integer", + "minimum": 1 + } + } + }, + "tool_approval_counter": { + "type": "object", + "additionalProperties": false, + "required": [ + "name", + "type", + "dimensions", + "value" + ], + "properties": { + "name": { + "const": "hermes.tool_approval.count" + }, + "type": { + "const": "counter" + }, + "dimensions": { + "type": "object", + "additionalProperties": false, + "required": [ + "attribution", + "outcome" + ], + "properties": { + "attribution": { + "enum": [ + "tool_call", + "unattributed" + ] + }, + "outcome": { + "enum": [ + "approved", + "denied", + "timed_out", + "unknown" + ] + } + } + }, + "value": { + "type": "integer", + "minimum": 1 + } + } } } } diff --git a/hermes_cli/observability/shared_metrics_contract.py b/hermes_cli/observability/shared_metrics_contract.py index bb48952cb89d0..870152634f53c 100644 --- a/hermes_cli/observability/shared_metrics_contract.py +++ b/hermes_cli/observability/shared_metrics_contract.py @@ -2,6 +2,7 @@ from __future__ import annotations +from math import isfinite from typing import Any from agent.relay_runtime import ( @@ -16,11 +17,15 @@ MODEL_CALL_SCOPE = "hermes.model_call" MODEL_CALL_PROFILE_MODEL = "unknown" TASK_SCOPE = "hermes.task_run" +TOOL_CALL_SCOPE = "hermes.tool_call" +TOOL_APPROVAL_MARK = "hermes.tool_approval" SUBSCRIBER_NAME = "hermes.nemo_relay.shared_metrics" LEGACY_MODEL_CALL_METRIC = "hermes.model_call.count" MODEL_ROUTE_METRIC = "hermes.model_route.count" TASK_STARTED_METRIC = "hermes.task_run.started" TASK_FINISHED_METRIC = "hermes.task_run.finished" +TOOL_CALL_METRIC = "hermes.tool_call.count" +TOOL_APPROVAL_METRIC = "hermes.tool_approval.count" MODEL_IDENTIFIER_MAX_LENGTH = 256 PROVIDER_IDENTIFIER_MAX_LENGTH = 64 _METRIC_IDENTIFIER_CHARACTERS = frozenset( @@ -95,6 +100,58 @@ "6_to_10", "gte_11", }) +TOOL_CATEGORIES: frozenset[str] = frozenset({ + "browser", + "code_execution", + "communication", + "computer_use", + "delegation", + "file", + "home_automation", + "mcp", + "media", + "memory", + "other", + "planning", + "project", + "scheduler", + "skill", + "terminal", + "unknown", + "web", +}) +TOOL_OUTCOMES: frozenset[str] = frozenset({ + "blocked", + "cancelled", + "failed", + "success", + "timed_out", + "unknown", +}) +TOOL_APPROVAL_OUTCOMES: frozenset[str] = frozenset({ + "approved", + "denied", + "not_required", + "timed_out", + "unknown", +}) +TOOL_APPROVAL_ATTRIBUTIONS: frozenset[str] = frozenset({ + "tool_call", + "unattributed", +}) +TOOL_LATENCY_BUCKETS: frozenset[str] = frozenset({ + "100ms_to_250ms", + "10s_to_30s", + "1s_to_2s", + "250ms_to_500ms", + "2s_to_5s", + "500ms_to_1s", + "5s_to_10s", + "gte_30s", + "lt_100ms", + "unknown", +}) +TOOL_RETRY_BUCKETS: frozenset[str] = COUNT_BUCKETS | frozenset({"unknown"}) _LEGACY_PROVIDER_FAMILIES = frozenset({ "aggregator", @@ -153,11 +210,24 @@ "termination": TASK_TERMINATIONS, "tool_call_count_bucket": COUNT_BUCKETS, }, + TOOL_CALL_METRIC: { + "approval_outcome": TOOL_APPROVAL_OUTCOMES, + "latency_bucket": TOOL_LATENCY_BUCKETS, + "outcome": TOOL_OUTCOMES, + "retry_count_bucket": TOOL_RETRY_BUCKETS, + "tool_category": TOOL_CATEGORIES, + }, + TOOL_APPROVAL_METRIC: { + "attribution": TOOL_APPROVAL_ATTRIBUTIONS, + "outcome": TOOL_APPROVAL_OUTCOMES - {"not_required"}, + }, } COUNTER_METRICS: frozenset[str] = frozenset({ MODEL_ROUTE_METRIC, TASK_FINISHED_METRIC, TASK_STARTED_METRIC, + TOOL_APPROVAL_METRIC, + TOOL_CALL_METRIC, }) @@ -184,25 +254,27 @@ def counter_dimensions_are_valid( if contract is None or set(dimensions) != set(contract): return False return all( - isinstance(dimensions[field], str) - and dimensions[field] in allowed_values + isinstance(dimensions[field], str) and dimensions[field] in allowed_values for field, allowed_values in contract.items() ) +def _event_metadata_is_valid(event: Any) -> bool: + metadata = getattr(event, "metadata", None) + if not isinstance(metadata, dict) or metadata.get(SCHEMA_KEY) != SCHEMA_VERSION: + return False + relay_metadata = set(metadata) - {SCHEMA_KEY, RUNTIME_INSTANCE_KEY} + return not relay_metadata - {"otel.status_code"} and metadata.get( + "otel.status_code", "OK" + ) in {"OK", "ERROR"} + + def model_call_dimensions(event: Any) -> dict[str, str] | None: """Return package dimensions for one valid logical model-call end event.""" auxiliary = _auxiliary_model_call_dimensions(event) if auxiliary is not None: return auxiliary - - metadata = getattr(event, "metadata", None) - if not isinstance(metadata, dict) or metadata.get(SCHEMA_KEY) != SCHEMA_VERSION: - return None - relay_metadata = set(metadata) - {SCHEMA_KEY, RUNTIME_INSTANCE_KEY} - if relay_metadata - {"otel.status_code"} or metadata.get( - "otel.status_code", "OK" - ) not in {"OK", "ERROR"}: + if not _event_metadata_is_valid(event): return None if ( str(getattr(event, "kind", "") or "") != "scope" @@ -277,13 +349,7 @@ def _auxiliary_model_call_dimensions(event: Any) -> dict[str, str] | None: def task_counter(event: Any) -> tuple[str, dict[str, str]] | None: """Return one validated task counter from a task scope event.""" - metadata = getattr(event, "metadata", None) - if not isinstance(metadata, dict) or metadata.get(SCHEMA_KEY) != SCHEMA_VERSION: - return None - relay_metadata = set(metadata) - {SCHEMA_KEY, RUNTIME_INSTANCE_KEY} - if relay_metadata - {"otel.status_code"} or metadata.get( - "otel.status_code", "OK" - ) not in {"OK", "ERROR"}: + if not _event_metadata_is_valid(event): return None if ( str(getattr(event, "kind", "") or "") != "scope" @@ -331,6 +397,56 @@ def task_counter(event: Any) -> tuple[str, dict[str, str]] | None: return TASK_FINISHED_METRIC, dimensions +def tool_call_dimensions(event: Any) -> dict[str, str] | None: + """Return package dimensions for one allowlisted tool lifecycle end event.""" + if not _event_metadata_is_valid(event): + return None + if ( + str(getattr(event, "kind", "") or "") != "scope" + or str(getattr(event, "category", "") or "") != "tool" + or str(getattr(event, "name", "") or "") != TOOL_CALL_SCOPE + or str(getattr(event, "scope_category", "") or "") != "end" + or getattr(event, "category_profile", None) != {} + ): + return None + data = getattr(event, "data", None) + expected_fields = { + "approval_outcome", + "latency_bucket", + "outcome", + "retry_count_bucket", + "tool_category", + } + if not isinstance(data, dict) or set(data) != expected_fields: + return None + dimensions = {field: data.get(field) for field in sorted(expected_fields)} + if not counter_dimensions_are_valid(TOOL_CALL_METRIC, dimensions): + return None + return dimensions + + +def tool_approval_counter(event: Any) -> tuple[str, dict[str, str]] | None: + """Return one validated approval counter from a safe Relay mark event.""" + if not _event_metadata_is_valid(event): + return None + if ( + str(getattr(event, "kind", "") or "") != "mark" + or str(getattr(event, "name", "") or "") != TOOL_APPROVAL_MARK + or getattr(event, "category", None) is not None + or getattr(event, "scope_category", None) is not None + or getattr(event, "category_profile", None) is not None + ): + return None + data = getattr(event, "data", None) + expected_fields = {"attribution", "outcome"} + if not isinstance(data, dict) or set(data) != expected_fields: + return None + dimensions = {field: data.get(field) for field in sorted(expected_fields)} + if not counter_dimensions_are_valid(TOOL_APPROVAL_METRIC, dimensions): + return None + return TOOL_APPROVAL_METRIC, dimensions + + def execution_surface(kwargs: dict[str, Any]) -> str: """Normalize the safe session surface carried by the parent Relay scope.""" value = ( @@ -459,6 +575,138 @@ def count_bucket(count: int) -> str: return "gte_11" +def tool_category(kwargs: dict[str, Any]) -> str: + """Map Hermes registry toolset metadata to a low-cardinality category.""" + toolset = str(kwargs.get("toolset") or "").strip().lower() + if not toolset: + return "unknown" + if toolset in TOOL_CATEGORIES: + return toolset + if toolset.startswith("mcp"): + return "mcp" + if toolset.startswith("browser"): + return "browser" + if toolset.startswith(("image", "tts", "video", "vision")): + return "media" + if toolset.startswith("homeassistant"): + return "home_automation" + if toolset in {"clarify", "kanban", "todo"}: + return "planning" + if toolset == "session_search": + return "memory" + if toolset == "cronjob": + return "scheduler" + if toolset == "skills": + return "skill" + if toolset == "x_search": + return "web" + if toolset.startswith( + ("discord", "email", "feishu", "hermes-yuanbao", "slack", "sms") + ): + return "communication" + return "other" + + +def tool_outcome(kwargs: dict[str, Any]) -> str: + """Normalize the terminal Hermes tool status without inspecting its result.""" + status = str(kwargs.get("status") or "").strip().lower() + return { + "blocked": "blocked", + "cancelled": "cancelled", + "error": "failed", + "failed": "failed", + "ok": "success", + "success": "success", + "timed_out": "timed_out", + "timeout": "timed_out", + }.get(status, "unknown") + + +def tool_approval_outcome(kwargs: dict[str, Any]) -> str: + """Normalize a terminal approval choice to a bounded outcome.""" + choice = str(kwargs.get("choice") or "").strip().lower() + if choice in {"always", "approve", "approved", "once", "session", "smart_approve"}: + return "approved" + if choice in {"deny", "denied", "smart_deny"}: + return "denied" + if choice in {"timed_out", "timeout"}: + return "timed_out" + return "unknown" + + +def tool_terminal_fields( + kwargs: dict[str, Any], + *, + category: str | None = None, + approval_outcome: str = "not_required", + fallback_duration_ms: int | None = None, +) -> dict[str, str]: + """Build one bounded tool-call terminal payload.""" + return { + "approval_outcome": ( + approval_outcome + if approval_outcome in TOOL_APPROVAL_OUTCOMES + else "unknown" + ), + "latency_bucket": tool_latency_bucket( + kwargs.get("duration_ms"), + fallback_duration_ms=fallback_duration_ms, + ), + "outcome": tool_outcome(kwargs), + "retry_count_bucket": tool_retry_bucket(kwargs.get("retry_count")), + "tool_category": ( + category if category in TOOL_CATEGORIES else tool_category(kwargs) + ), + } + + +def tool_latency_bucket( + value: Any, + *, + fallback_duration_ms: int | None = None, +) -> str: + """Bucket a tool duration reported in milliseconds.""" + duration_ms = _non_negative_number(value) + if duration_ms is None: + duration_ms = _non_negative_number(fallback_duration_ms) + if duration_ms is None: + return "unknown" + if duration_ms < 100: + return "lt_100ms" + if duration_ms < 250: + return "100ms_to_250ms" + if duration_ms < 500: + return "250ms_to_500ms" + if duration_ms < 1_000: + return "500ms_to_1s" + if duration_ms < 2_000: + return "1s_to_2s" + if duration_ms < 5_000: + return "2s_to_5s" + if duration_ms < 10_000: + return "5s_to_10s" + if duration_ms < 30_000: + return "10s_to_30s" + return "gte_30s" + + +def tool_retry_bucket(value: Any) -> str: + """Bucket only explicit tool retries; missing relationships stay unknown.""" + if isinstance(value, bool) or not isinstance(value, int) or value < 0: + return "unknown" + return count_bucket(value) + + +def _non_negative_number(value: Any) -> float | None: + if isinstance(value, bool) or not isinstance(value, (int, float)): + return None + try: + number = float(value) + except (OverflowError, TypeError, ValueError): + return None + return number if isfinite(number) and number >= 0 else None + + def model_call_fields(kwargs: dict[str, Any]) -> dict[str, str]: """Return the terminal model identity and provider route known to Hermes.""" model = _metric_identifier( diff --git a/hermes_cli/observability/shared_metrics_subscriber.py b/hermes_cli/observability/shared_metrics_subscriber.py index 259a6f6b4a2d5..086f88d4c23b6 100644 --- a/hermes_cli/observability/shared_metrics_subscriber.py +++ b/hermes_cli/observability/shared_metrics_subscriber.py @@ -9,7 +9,14 @@ from agent.relay_runtime import RUNTIME_INSTANCE_KEY from .shared_metrics import SharedMetricsStore -from .shared_metrics_contract import MODEL_ROUTE_METRIC, model_call_dimensions, task_counter +from .shared_metrics_contract import ( + MODEL_ROUTE_METRIC, + TOOL_CALL_METRIC, + model_call_dimensions, + task_counter, + tool_approval_counter, + tool_call_dimensions, +) logger = logging.getLogger(__name__) @@ -46,10 +53,13 @@ def __call__(self, event: Any) -> None: dimensions = model_call_dimensions(event) metric_name = MODEL_ROUTE_METRIC if dimensions is None: - task_metric = task_counter(event) - if task_metric is None: + dimensions = tool_call_dimensions(event) + metric_name = TOOL_CALL_METRIC + if dimensions is None: + metric = task_counter(event) or tool_approval_counter(event) + if metric is None: return - metric_name, dimensions = task_metric + metric_name, dimensions = metric with self._lock: if not self._active: return diff --git a/hermes_cli/plugins.py b/hermes_cli/plugins.py index 8dd02a1bdf5ec..23ed3c26e3c3e 100644 --- a/hermes_cli/plugins.py +++ b/hermes_cli/plugins.py @@ -2161,7 +2161,9 @@ def _get_pre_tool_call_directive_details( message=fmt.format(tool_name=tool_name), ) - hook_results = invoke_hook( + from hermes_cli.lifecycle import invoke_hook as invoke_lifecycle_hook + + hook_results = invoke_lifecycle_hook( "pre_tool_call", tool_name=tool_name, args=args if isinstance(args, dict) else {}, @@ -2277,12 +2279,32 @@ def resolve_pre_tool_block( return details.message if details.action == "approve": try: - from tools.approval import request_tool_approval - result = request_tool_approval( - tool_name, - details.message or "", - rule_key=details.rule_key or tool_name, + from tools.approval import ( + request_tool_approval, + reset_current_observability_context, + set_current_observability_context, ) + + approval_tokens = None + try: + approval_tokens = set_current_observability_context( + turn_id=turn_id, + tool_call_id=tool_call_id, + ) + except Exception: + pass + try: + result = request_tool_approval( + tool_name, + details.message or "", + rule_key=details.rule_key or tool_name, + ) + finally: + if approval_tokens is not None: + try: + reset_current_observability_context(approval_tokens) + except Exception: + pass except Exception: # Fail-closed: if the gate itself errors, block rather than # silently execute an action a plugin flagged for approval. diff --git a/hermes_cli/profiles.py b/hermes_cli/profiles.py index 4ed717668aabb..71acde626c212 100644 --- a/hermes_cli/profiles.py +++ b/hermes_cli/profiles.py @@ -30,7 +30,7 @@ import time from dataclasses import dataclass from pathlib import Path, PurePosixPath, PureWindowsPath -from typing import List, Optional, Tuple +from typing import Dict, List, Optional, Tuple from agent.skill_utils import is_excluded_skill_path @@ -237,6 +237,9 @@ def _ignore(directory: str, names: List[str]) -> List[str]: # Configuration / persona "config.yaml", "SOUL.md", "MEMORY.md", "USER.md", "todo.json", "system_prompt.md", "AGENTS.md", "CLAUDE.md", ".cursorrules", + # Desktop appearance/interface overlay (written by the desktop app's + # profile export; applied by its import — see desktop.json handling). + "desktop.json", # User-facing skill, cron, and session artifacts "skills", "cron", "scripts", "sessions", # Plugin / memory surfaces (per-profile overrides live here) @@ -1898,9 +1901,29 @@ def _ignore(directory: str, contents: list) -> set: return _ignore -def export_profile(name: str, output_path: str) -> Path: +def _make_profile_archive(base: str, root_dir: str, base_dir: str) -> str: + """Create ``.tar.gz`` of ``root_dir/base_dir`` — GNU tar format. + + Not :func:`shutil.make_archive`: that writes PAX (Python's tarfile default + since 3.8), whose fractional-mtime records macOS Archive Utility rejects — + double-clicking an exported profile threw "Error 94 - Bad message." GNU + format keeps long paths working (longlink extensions) and stays integer- + mtime, so Finder, bsdtar, and gnutar all extract it. + """ + import tarfile + + archive_path = f"{base}.tar.gz" + with tarfile.open(archive_path, "w:gz", format=tarfile.GNU_FORMAT) as tf: + tf.add(str(Path(root_dir) / base_dir), arcname=base_dir) + return archive_path + + +def export_profile(name: str, output_path: str, extra_files: Optional[Dict[str, str]] = None) -> Path: """Export a profile to a tar.gz archive. + ``extra_files`` maps root-relative filenames (e.g. ``desktop.json``) to + text content staged into the archive alongside the profile's own files — + the desktop app uses it to bundle its appearance/interface overlay. Returns the output file path. """ import tempfile @@ -1912,9 +1935,16 @@ def export_profile(name: str, output_path: str) -> Path: raise FileNotFoundError(f"Profile '{canon}' does not exist.") output = Path(output_path) - # shutil.make_archive wants the base name without extension + # Archive base name without extension (.tar.gz appended by the writer). base = str(output).removesuffix(".tar.gz").removesuffix(".tgz") + def _stage_extras(staged: Path) -> None: + for rel, content in (extra_files or {}).items(): + parts = _normalize_profile_archive_parts(rel) + target = staged.joinpath(*parts) + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(content, encoding="utf-8") + if canon == "default": # The default profile IS ~/.hermes itself — its parent is ~/ and its # directory name is ".hermes", not "default". We stage a clean copy @@ -1927,7 +1957,8 @@ def export_profile(name: str, output_path: str) -> Path: symlinks=True, ignore=_default_export_ignore(profile_dir), ) - result = shutil.make_archive(base, "gztar", tmpdir, "default") + _stage_extras(staged) + result = _make_profile_archive(base, tmpdir, "default") return Path(result) # Named profiles — stage a filtered copy to exclude credentials @@ -1940,7 +1971,8 @@ def export_profile(name: str, output_path: str) -> Path: symlinks=True, ignore=lambda d, contents: _CREDENTIAL_FILES & set(contents), ) - result = shutil.make_archive(base, "gztar", tmpdir, canon) + _stage_extras(staged) + result = _make_profile_archive(base, tmpdir, canon) return Path(result) diff --git a/hermes_cli/web_models.py b/hermes_cli/web_models.py index 3ff438c63800f..03dd3a97365b6 100644 --- a/hermes_cli/web_models.py +++ b/hermes_cli/web_models.py @@ -578,6 +578,22 @@ class ProfileRename(BaseModel): new_name: str +class ProfileExport(BaseModel): + # Optional extra root-level files to stage into the archive, filename → + # text content (e.g. desktop.json — the desktop appearance overlay). + extra_files: Dict[str, str] = {} + # Where to write the archive. Empty → a staging path under HERMES_HOME. + output: str = "" + + +class ProfileImport(BaseModel): + # Path to a profile .tar.gz on the backend's filesystem (the desktop's + # local/pooled backends share the machine with the picker dialog). + archive: str + # Override the profile name inferred from the archive root. + name: Optional[str] = None + + class ProfileSoulUpdate(BaseModel): content: str diff --git a/hermes_cli/web_routers/profiles.py b/hermes_cli/web_routers/profiles.py index d7bc45c2d8130..b3587ccd9eb1f 100644 --- a/hermes_cli/web_routers/profiles.py +++ b/hermes_cli/web_routers/profiles.py @@ -26,6 +26,8 @@ from hermes_cli.web_models import ( ProfileCreate, ProfileActiveUpdate, + ProfileExport, + ProfileImport, ProfileRename, ProfileSoulUpdate, ProfileDescriptionUpdate, @@ -687,3 +689,105 @@ async def describe_profile_auto_endpoint(name: str, body: ProfileDescribeAuto): # auto-generated. "description_auto": bool(outcome.ok), } + + +# ── Export / Import ────────────────────────────────────────────────────────── +# Profile sharing for the desktop: wraps hermes_cli.profiles.export_profile / +# import_profile (the same machinery behind `hermes profile export|import`). +# Paths are exchanged, not bytes — the desktop's local and pooled backends +# share the filesystem with the native save/open dialogs that produce them. + + +@router.post("/api/profiles/{name}/export") +async def export_profile_endpoint(name: str, body: ProfileExport): + from hermes_cli import profiles as profiles_mod + + output = (body.output or "").strip() + if not output: + from hermes_constants import get_hermes_home + staging = get_hermes_home() / "profile-exports" + try: + staging.mkdir(parents=True, exist_ok=True) + except OSError as exc: + raise HTTPException(status_code=500, detail=f"Could not create export directory: {exc}") + stamp = time.strftime("%Y%m%d-%H%M%S") + output = str(staging / f"{profiles_mod.normalize_profile_name(name)}-{stamp}.tar.gz") + + loop = asyncio.get_running_loop() + try: + result = await loop.run_in_executor( + None, + lambda: profiles_mod.export_profile(name, output, extra_files=body.extra_files or None), + ) + except FileNotFoundError as e: + raise HTTPException(status_code=404, detail=str(e)) + except ValueError as e: + raise HTTPException(status_code=400, detail=str(e)) + except Exception as e: + _log.exception("POST /api/profiles/%s/export failed", name) + raise HTTPException(status_code=500, detail=str(e)) + return {"ok": True, "archive": str(result)} + + +@router.post("/api/profiles/import") +async def import_profile_endpoint(body: ProfileImport): + from hermes_cli import profiles as profiles_mod + + archive = (body.archive or "").strip() + if not archive: + raise HTTPException(status_code=400, detail="archive path is required") + + loop = asyncio.get_running_loop() + try: + profile_dir = await loop.run_in_executor( + None, + lambda: profiles_mod.import_profile(archive, name=(body.name or "").strip() or None), + ) + except FileNotFoundError as e: + raise HTTPException(status_code=404, detail=str(e)) + except (ValueError, FileExistsError) as e: + raise HTTPException(status_code=400, detail=str(e)) + except Exception as e: + _log.exception("POST /api/profiles/import failed") + raise HTTPException(status_code=500, detail=str(e)) + + imported = profile_dir.name + # Match the CLI import flow: create the wrapper alias when it's safe. + try: + if not profiles_mod.check_alias_collision(imported): + profiles_mod.create_wrapper_script(imported) + except Exception: + _log.exception("Creating wrapper for imported profile %s failed", imported) + + # Surface the bundled desktop appearance overlay (if the archive carried + # one) so the desktop can apply theme/interface prefs without re-reading + # the file over another round-trip. + desktop_overlay = None + overlay_path = profile_dir / "desktop.json" + if overlay_path.is_file(): + try: + import json as _json + desktop_overlay = _json.loads(overlay_path.read_text(encoding="utf-8")) + except Exception: + _log.exception("Reading desktop.json from imported profile %s failed", imported) + + return { + "ok": True, + "name": imported, + "path": str(profile_dir), + "desktop": desktop_overlay, + } + + +@router.get("/api/profiles/{name}/desktop-overlay") +async def get_profile_desktop_overlay(name: str): + """The desktop appearance/interface overlay bundled with an imported + profile (``desktop.json`` at the profile root), or ``exists: false``.""" + overlay_path = _resolve_profile_dir(name) / "desktop.json" + if not overlay_path.is_file(): + return {"exists": False, "desktop": None} + try: + import json as _json + return {"exists": True, "desktop": _json.loads(overlay_path.read_text(encoding="utf-8"))} + except Exception as e: + raise HTTPException(status_code=500, detail=f"Could not read desktop.json: {e}") diff --git a/hermes_state.py b/hermes_state.py index 2c7210fa71d5a..c7dc0dfe11bda 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -3649,7 +3649,12 @@ def _do(conn): return False def update_session_cwd( - self, session_id: str, cwd: str, git_branch: str = None, git_repo_root: str = None + self, + session_id: str, + cwd: str, + git_branch: str = None, + git_repo_root: str = None, + replace_git_meta: bool = False, ) -> None: """Persist the session working directory when a frontend knows it. @@ -3664,6 +3669,11 @@ def update_session_cwd( every surface reads the same membership instead of re-probing git in the GUI over a partial page. Each field is only written when non-empty so a probe failure never clobbers a previously-captured value. + + ``replace_git_meta`` inverts that non-empty rule: a deliberate workspace + MOVE (re-homing a session into another project) must overwrite the old + repo identity even when the new cwd resolves to none — keeping the stale + root would leave the session grouped under the project it just left. """ if not session_id or not cwd: return @@ -3673,12 +3683,12 @@ def update_session_cwd( sets = ["cwd = ?"] params: List[Any] = [cwd] - if branch: + if branch or replace_git_meta: sets.append("git_branch = ?") - params.append(branch) - if repo_root: + params.append(branch or None) + if repo_root or replace_git_meta: sets.append("git_repo_root = ?") - params.append(repo_root) + params.append(repo_root or None) params.append(session_id) def _do(conn): @@ -5478,6 +5488,80 @@ def _do(conn): rowcount = self._execute_write(_do) return rowcount > 0 + def set_session_read(self, session_id: str, read: bool = True) -> bool: + """Mark a session read or unread (and its whole compression lineage). + + Read state is a watermark, not a flag: ``last_read_at`` records when + the conversation was last read, and it counts as unread when activity + postdates that watermark (the derived ``unread`` key on + :meth:`list_sessions_rich` rows). New messages therefore flip a read + conversation back to unread without any write on the message path. + Three states: + + * NULL — never tracked (every pre-feature row): treated as read, so + shipping the column doesn't badge a user's entire history at once. + * 0 — explicitly marked unread: any activity postdates it. + * timestamp — read up to that moment. + + Like :meth:`set_session_archived` / :meth:`set_session_pinned`, the + whole compression chain is stamped as a unit, so reading the surfaced + tip clears the root (and vice-versa) no matter which id the caller + holds. Returns True when at least one row changed. + """ + def _do(conn): + cursor = conn.execute( + """ + WITH RECURSIVE + ancestors(id) AS ( + SELECT ? + UNION + SELECT parent.id + FROM ancestors a + JOIN sessions child ON child.id = a.id + JOIN sessions parent ON parent.id = child.parent_session_id + WHERE parent.end_reason = 'compression' + ), + descendants(id) AS ( + SELECT ? + UNION + SELECT child.id + FROM descendants d + JOIN sessions parent ON parent.id = d.id + JOIN sessions child ON child.parent_session_id = parent.id + WHERE parent.end_reason = 'compression' + ), + lineage(id) AS ( + SELECT id FROM ancestors + UNION + SELECT id FROM descendants + ) + UPDATE sessions + SET last_read_at = ? + WHERE id IN (SELECT id FROM lineage) + """, + (session_id, session_id, time.time() if read else 0.0), + ) + rowcount = cursor.rowcount + if rowcount is None or rowcount < 0: + rowcount = conn.execute("SELECT changes()").fetchone()[0] + return rowcount + rowcount = self._execute_write(_do) + return rowcount > 0 + + @staticmethod + def session_unread(session_row: Dict[str, Any]) -> bool: + """Derive unread from a session row's watermark and activity. + + Shared by ``list_sessions_rich`` and any future surface that holds a + row (or projected row) with ``last_read_at`` and ``last_active``. + NULL watermark = never tracked = read. + """ + last_read = session_row.get("last_read_at") + if last_read is None: + return False + last_active = session_row.get("last_active") or session_row.get("started_at") + return float(last_active or 0) > float(last_read) + def get_session_by_title(self, title: str) -> Optional[Dict[str, Any]]: """Look up a session by exact title. Returns session dict or None.""" with self._read_ctx() as conn: @@ -5994,6 +6078,13 @@ def _like_pattern(needle: str) -> str: projected.append(merged) sessions = projected + # Derive read state per surfaced conversation. ``last_read_at`` is + # lineage-stamped by set_session_read, so a projected row's root + # watermark and its tip's are the same value — comparing it against + # the tip's last_active is correct either way. + for s in sessions: + s["unread"] = self.session_unread(s) + return sessions # ========================================================================= diff --git a/hermes_state_common.py b/hermes_state_common.py index c520f1c51df63..4e66115f81780 100644 --- a/hermes_state_common.py +++ b/hermes_state_common.py @@ -245,6 +245,7 @@ def _sql_session_last_active_by_id(session_id_expr: str) -> str: rewind_count INTEGER NOT NULL DEFAULT 0, archived INTEGER NOT NULL DEFAULT 0, pinned INTEGER NOT NULL DEFAULT 0, + last_read_at REAL, FOREIGN KEY (parent_session_id) REFERENCES sessions(id), FOREIGN KEY (system_prompt_hash) REFERENCES system_prompts(hash) ); diff --git a/model_tools.py b/model_tools.py index 40a8f83b3ec88..0f1290258985e 100644 --- a/model_tools.py +++ b/model_tools.py @@ -1032,13 +1032,24 @@ def _coerce_boolean(value: str): return value -def _tool_result_observer_fields(result: Any) -> tuple[str, Optional[str], Optional[str]]: +def _tool_result_observer_fields( + tool_name: str, + result: Any, +) -> tuple[str, Optional[str], Optional[str]]: try: parsed_result = json.loads(result) if isinstance(result, str) else result if isinstance(parsed_result, dict) and parsed_result.get("error"): return "error", "tool_error", str(parsed_result.get("error")) except Exception: pass + try: + from agent.display import _detect_tool_failure + + failed, suffix = _detect_tool_failure(tool_name, result) + if failed: + return "error", "tool_error", suffix.strip().strip("[]") or None + except Exception: + pass return "ok", None, None @@ -1072,7 +1083,10 @@ def _emit_post_tool_call_hook( if not has_hook("post_tool_call"): return if status is None: - status, error_type, error_message = _tool_result_observer_fields(result) + status, error_type, error_message = _tool_result_observer_fields( + function_name, + result, + ) invoke_hook( "post_tool_call", tool_name=function_name, @@ -1145,6 +1159,23 @@ def handle_function_call( # inline. tool_call is unwrapped to the underlying tool so that every # downstream hook (pre/post, edit approval, guardrails) sees the real # tool name, not the bridge. + _dispatch_start = time.monotonic() + + def _return_bridge_result(result: Any) -> Any: + _emit_post_tool_call_hook( + function_name=function_name, + function_args=function_args, + result=result, + task_id=task_id, + session_id=session_id, + tool_call_id=tool_call_id, + turn_id=turn_id, + api_request_id=api_request_id, + duration_ms=int((time.monotonic() - _dispatch_start) * 1000), + middleware_trace=list(_tool_middleware_trace), + ) + return result + _ts_mod = None try: from tools import tool_search as _ts_mod # noqa: F401 @@ -1174,15 +1205,25 @@ def handle_function_call( except Exception: current_defs = [] if function_name == _ts_mod.TOOL_SEARCH_NAME: - return _ts_mod.dispatch_tool_search(function_args or {}, - current_tool_defs=current_defs) + return _return_bridge_result( + _ts_mod.dispatch_tool_search( + function_args or {}, + current_tool_defs=current_defs, + ) + ) if function_name == _ts_mod.TOOL_DESCRIBE_NAME: - return _ts_mod.dispatch_tool_describe(function_args or {}, - current_tool_defs=current_defs) + return _return_bridge_result( + _ts_mod.dispatch_tool_describe( + function_args or {}, + current_tool_defs=current_defs, + ) + ) if function_name == _ts_mod.TOOL_CALL_NAME: underlying_name, underlying_args, err = _ts_mod.resolve_underlying_call(function_args or {}) if err or not underlying_name: - return tool_error(err or "tool_call could not be resolved") + return _return_bridge_result( + tool_error(err or "tool_call could not be resolved") + ) # Defense in depth: the underlying tool MUST be in the session's # scoped deferrable catalog. resolve_underlying_call() only checks # that the name is deferrable in the global registry; this gate @@ -1191,16 +1232,18 @@ def handle_function_call( # the bridge even if the catalog scoping above regressed. _scoped_deferrable = _ts_mod.scoped_deferrable_names(current_defs) if underlying_name not in _scoped_deferrable: - return tool_error( - f"'{underlying_name}' is not available in this session. " - "Use tool_search to find tools you can call." + return _return_bridge_result( + tool_error( + f"'{underlying_name}' is not available in this session. " + "Use tool_search to find tools you can call." + ) ) # Probe-validate against the deferred tool's schema (ironclaw#5149): # a blind call missing required arguments returns the parameter # schema instead of dispatching into an opaque downstream failure. _probe_err = _ts_mod.validate_deferred_call_args(underlying_name, underlying_args) if _probe_err is not None: - return _probe_err + return _return_bridge_result(_probe_err) # Recurse with the underlying tool. All hooks fire against the # real tool name. The bridge is invisible to hooks by design. return handle_function_call( @@ -1209,6 +1252,8 @@ def handle_function_call( task_id=task_id, tool_call_id=tool_call_id, session_id=session_id, + turn_id=turn_id, + api_request_id=api_request_id, user_task=user_task, enabled_tools=enabled_tools, skip_pre_tool_call_hook=skip_pre_tool_call_hook, @@ -1297,11 +1342,38 @@ def handle_function_call( edit_block_message = maybe_require_edit_approval(function_name, function_args) if edit_block_message is not None: + _emit_post_tool_call_hook( + function_name=function_name, + function_args=function_args, + result=edit_block_message, + task_id=task_id, + session_id=session_id, + tool_call_id=tool_call_id, + turn_id=turn_id, + api_request_id=api_request_id, + status="blocked", + error_type="edit_approval_denied", + middleware_trace=list(_tool_middleware_trace), + ) return edit_block_message except Exception as _edit_approval_err: logger.debug("ACP edit approval guard error: %s", _edit_approval_err) if function_name in {"write_file", "patch"}: - return tool_error("Edit approval denied: approval guard failed") + result = tool_error("Edit approval denied: approval guard failed") + _emit_post_tool_call_hook( + function_name=function_name, + function_args=function_args, + result=result, + task_id=task_id, + session_id=session_id, + tool_call_id=tool_call_id, + turn_id=turn_id, + api_request_id=api_request_id, + status="blocked", + error_type="edit_approval_error", + middleware_trace=list(_tool_middleware_trace), + ) + return result # Notify the read-loop tracker when a non-read/search tool runs, # so the *consecutive* counter resets (reads after other work are fine). @@ -1400,7 +1472,10 @@ def _dispatch(next_args: Dict[str, Any]) -> Any: try: from hermes_cli.lifecycle import has_hook, invoke_hook if has_hook("transform_tool_result"): - status, error_type, error_message = _tool_result_observer_fields(result) + status, error_type, error_message = _tool_result_observer_fields( + function_name, + result, + ) hook_results = invoke_hook( "transform_tool_result", tool_name=function_name, @@ -1428,7 +1503,28 @@ def _dispatch(next_args: Dict[str, Any]) -> Any: except Exception as e: error_msg = f"Error executing {function_name}: {str(e)}" logger.exception(error_msg) - return tool_error(_sanitize_tool_error(error_msg)) + result = tool_error(_sanitize_tool_error(error_msg)) + duration_ms = ( + int((time.monotonic() - _dispatch_start) * 1000) + if _dispatch_start is not None + else 0 + ) + _emit_post_tool_call_hook( + function_name=function_name, + function_args=function_args, + result=result, + task_id=task_id, + session_id=session_id, + tool_call_id=tool_call_id, + turn_id=turn_id, + api_request_id=api_request_id, + duration_ms=duration_ms, + status="error", + error_type=type(e).__name__, + error_message=str(e), + middleware_trace=list(_tool_middleware_trace), + ) + return result # ============================================================================= diff --git a/scripts/smoke_nemo_relay_shared_metrics.py b/scripts/smoke_nemo_relay_shared_metrics.py index a8bf4adb0121a..64c69bdc3c068 100644 --- a/scripts/smoke_nemo_relay_shared_metrics.py +++ b/scripts/smoke_nemo_relay_shared_metrics.py @@ -20,6 +20,9 @@ PROMPT_CANARY = "relay-smoke-sensitive-prompt" MODEL_CANARY = "gpt-relay-smoke-sensitive-model" RESPONSE_CANARY = "relay-smoke-sensitive-response" +TOOL_CALL_CANARY = "relay-smoke-sensitive-tool-call" +TOOL_RESULT_CANARY = "relay-smoke-sensitive-tool-result" +TOOL_FILE = "relay-smoke-input.txt" def _resolve_hermes_executable(hermes_repo: Path) -> Path: @@ -68,30 +71,51 @@ def do_POST(self) -> None: # noqa: N802 length = int(self.headers.get("Content-Length", "0")) request = json.loads(self.rfile.read(length) or b"{}") type(self).requests.append(request) + request_tool = not any( + message.get("role") == "tool" + for message in request.get("messages") or [] + if isinstance(message, dict) + ) if request.get("stream"): - self._write_stream() + self._write_stream(request_tool=request_tool) else: - self._write_json({ - "id": "chatcmpl-relay-smoke", - "object": "chat.completion", - "created": int(time.time()), - "model": MODEL_CANARY, - "choices": [ - { - "index": 0, - "message": { - "role": "assistant", - "content": RESPONSE_CANARY, - }, - "finish_reason": "stop", - } - ], - "usage": { - "prompt_tokens": 10, - "completion_tokens": 1, - "total_tokens": 11, - }, - }) + self._write_json(self._completion(request_tool=request_tool)) + + def _completion(self, *, request_tool: bool) -> dict[str, Any]: + message: dict[str, Any] = { + "role": "assistant", + "content": "" if request_tool else RESPONSE_CANARY, + } + finish_reason = "tool_calls" if request_tool else "stop" + if request_tool: + message["tool_calls"] = [ + { + "id": TOOL_CALL_CANARY, + "type": "function", + "function": { + "name": "read_file", + "arguments": json.dumps({"path": TOOL_FILE}), + }, + } + ] + return { + "id": "chatcmpl-relay-smoke", + "object": "chat.completion", + "created": int(time.time()), + "model": MODEL_CANARY, + "choices": [ + { + "index": 0, + "message": message, + "finish_reason": finish_reason, + } + ], + "usage": { + "prompt_tokens": 10, + "completion_tokens": 1, + "total_tokens": 11, + }, + } def log_message(self, format: str, *args: Any) -> None: return @@ -106,9 +130,9 @@ def _write_json(self, payload: dict[str, Any]) -> None: self.wfile.write(body) self.close_connection = True - def _write_stream(self) -> None: + def _write_stream(self, *, request_tool: bool) -> None: now = int(time.time()) - chunks = [ + chunks: list[dict[str, Any]] = [ { "id": "chatcmpl-relay-smoke", "object": "chat.completion.chunk", @@ -119,18 +143,66 @@ def _write_stream(self) -> None: "index": 0, "delta": { "role": "assistant", - "content": RESPONSE_CANARY, + "content": "", }, "finish_reason": None, } ], - }, + } + ] + if request_tool: + chunks.append({ + "id": "chatcmpl-relay-smoke", + "object": "chat.completion.chunk", + "created": now, + "model": MODEL_CANARY, + "choices": [ + { + "index": 0, + "delta": { + "tool_calls": [ + { + "index": 0, + "id": TOOL_CALL_CANARY, + "type": "function", + "function": { + "name": "read_file", + "arguments": json.dumps({"path": TOOL_FILE}), + }, + } + ] + }, + "finish_reason": None, + } + ], + }) + else: + chunks.append({ + "id": "chatcmpl-relay-smoke", + "object": "chat.completion.chunk", + "created": now, + "model": MODEL_CANARY, + "choices": [ + { + "index": 0, + "delta": {"content": RESPONSE_CANARY}, + "finish_reason": None, + } + ], + }) + chunks.extend([ { "id": "chatcmpl-relay-smoke", "object": "chat.completion.chunk", "created": now, "model": MODEL_CANARY, - "choices": [{"index": 0, "delta": {}, "finish_reason": "stop"}], + "choices": [ + { + "index": 0, + "delta": {}, + "finish_reason": "tool_calls" if request_tool else "stop", + } + ], }, { "id": "chatcmpl-relay-smoke", @@ -144,7 +216,7 @@ def _write_stream(self) -> None: "total_tokens": 11, }, }, - ] + ]) self.send_response(200) self.send_header("Content-Type", "text/event-stream") self.send_header("Cache-Control", "no-cache") @@ -220,25 +292,29 @@ def _validate_store(database_path: Path) -> list[dict[str, Any]]: } for name, dimensions, value, packaged_value in rows ] - by_name = {counter["name"]: counter for counter in counters} + by_name: dict[str, list[dict[str, Any]]] = {} + for counter in counters: + by_name.setdefault(counter["name"], []).append(counter) if set(by_name) != { "hermes.model_route.count", "hermes.task_run.finished", "hermes.task_run.started", + "hermes.tool_call.count", }: raise AssertionError( f"Unexpected SQLite counters:\n{json.dumps(counters, indent=2)}" ) + [model] = by_name["hermes.model_route.count"] expected_model = { "name": "hermes.model_route.count", "dimensions": { "model": MODEL_CANARY, "provider": "custom", }, - "value": 1, - "packaged_value": 1, + "value": 2, + "packaged_value": 2, } - if by_name["hermes.model_route.count"] != expected_model: + if model != expected_model: raise AssertionError( f"Unexpected model counter: {by_name['hermes.model_route.count']}" ) @@ -251,21 +327,21 @@ def _validate_store(database_path: Path) -> list[dict[str, Any]]: "value": 1, "packaged_value": 1, } - if by_name["hermes.task_run.started"] != expected_start: + if by_name["hermes.task_run.started"] != [expected_start]: raise AssertionError( f"Unexpected task start: {by_name['hermes.task_run.started']}" ) - terminal = by_name["hermes.task_run.finished"] + [terminal] = by_name["hermes.task_run.finished"] expected_terminal_dimensions = { "duration_bucket": terminal["dimensions"].get("duration_bucket"), "end_reason": "completed", "entrypoint": "interactive", "execution_surface": "cli", - "model_call_count_bucket": "1", + "model_call_count_bucket": "2", "outcome": "success", "retry_count_bucket": "0", "termination": "none", - "tool_call_count_bucket": "0", + "tool_call_count_bucket": "1", } if ( terminal["dimensions"] != expected_terminal_dimensions @@ -273,6 +349,21 @@ def _validate_store(database_path: Path) -> list[dict[str, Any]]: or terminal["packaged_value"] != 1 ): raise AssertionError(f"Unexpected task terminal counter: {terminal}") + [tool] = by_name["hermes.tool_call.count"] + expected_tool_dimensions = { + "approval_outcome": "not_required", + "latency_bucket": tool["dimensions"].get("latency_bucket"), + "outcome": "success", + "retry_count_bucket": "unknown", + "tool_category": "file", + } + if ( + tool["dimensions"] != expected_tool_dimensions + or tool["dimensions"]["latency_bucket"] == "unknown" + or tool["value"] != 1 + or tool["packaged_value"] != 1 + ): + raise AssertionError(f"Unexpected tool counter: {tool}") return counters @@ -292,41 +383,62 @@ def _validate_package(outbox: Path, schema_path: Path) -> tuple[Path, dict[str, jsonschema.validate(package, schema) serialized = json.dumps(package) - for prohibited in (PROMPT_CANARY, RESPONSE_CANARY): + for prohibited in ( + PROMPT_CANARY, + RESPONSE_CANARY, + TOOL_CALL_CANARY, + TOOL_RESULT_CANARY, + ): if prohibited in serialized: raise AssertionError( f"Exported package leaked prohibited value: {prohibited!r}" ) - metrics = {metric["name"]: metric for metric in package.get("metrics", [])} + metrics: dict[str, list[dict[str, Any]]] = {} + for metric in package.get("metrics", []): + metrics.setdefault(metric["name"], []).append(metric) if set(metrics) != { "hermes.model_route.count", "hermes.task_run.finished", "hermes.task_run.started", + "hermes.tool_call.count", }: raise AssertionError( f"Unexpected package metrics:\n{json.dumps(package.get('metrics'), indent=2)}" ) - model_dimensions = metrics["hermes.model_route.count"]["dimensions"] - if model_dimensions != { + [model] = metrics["hermes.model_route.count"] + if model["dimensions"] != { "model": MODEL_CANARY, "provider": "custom", - }: + } or model["value"] != 2: raise AssertionError( f"Unexpected model metric: {metrics['hermes.model_route.count']}" ) - terminal = metrics["hermes.task_run.finished"] + [terminal] = metrics["hermes.task_run.finished"] if terminal["dimensions"] != { "duration_bucket": terminal["dimensions"].get("duration_bucket"), "end_reason": "completed", "entrypoint": "interactive", "execution_surface": "cli", - "model_call_count_bucket": "1", + "model_call_count_bucket": "2", "outcome": "success", "retry_count_bucket": "0", "termination": "none", - "tool_call_count_bucket": "0", + "tool_call_count_bucket": "1", }: raise AssertionError(f"Unexpected task terminal metric: {terminal}") + [tool] = metrics["hermes.tool_call.count"] + if ( + tool["dimensions"] + != { + "approval_outcome": "not_required", + "latency_bucket": tool["dimensions"].get("latency_bucket"), + "outcome": "success", + "retry_count_bucket": "unknown", + "tool_category": "file", + } + or tool["dimensions"]["latency_bucket"] == "unknown" + ): + raise AssertionError(f"Unexpected tool metric: {tool}") return package_path, package @@ -353,6 +465,7 @@ def main() -> int: home = root / "hermes-home" workdir = root / "workspace" workdir.mkdir() + (workdir / TOOL_FILE).write_text(TOOL_RESULT_CANARY, encoding="utf-8") home.mkdir() (home / ".no-bundled-skills").touch() @@ -382,7 +495,7 @@ def main() -> int: "--quiet", "--ignore-rules", "--toolsets", - "search", + "file", "--max-turns", "2", ], @@ -404,13 +517,18 @@ def main() -> int: f"Hermes exited with {result.returncode}\n" f"stdout:\n{result.stdout}\nstderr:\n{result.stderr}" ) - if not _ModelHandler.requests: - raise AssertionError("Hermes did not call the local model endpoint") + if len(_ModelHandler.requests) != 2: + raise AssertionError( + f"Expected two model requests, got {len(_ModelHandler.requests)}" + ) request = _ModelHandler.requests[0] if request.get("model") != MODEL_CANARY: raise AssertionError(f"Unexpected model request: {request.get('model')!r}") if PROMPT_CANARY not in json.dumps(request.get("messages", [])): raise AssertionError("Hermes model request did not contain the prompt canary") + follow_up = json.dumps(_ModelHandler.requests[1].get("messages", [])) + if TOOL_CALL_CANARY not in follow_up or TOOL_RESULT_CANARY not in follow_up: + raise AssertionError("Hermes did not return the tool result to the model") if RESPONSE_CANARY not in result.stdout: raise AssertionError("Hermes did not print the mock model response") diff --git a/tests/agent/test_error_classifier.py b/tests/agent/test_error_classifier.py index be162f5aa3a81..37b498b251c17 100644 --- a/tests/agent/test_error_classifier.py +++ b/tests/agent/test_error_classifier.py @@ -371,6 +371,39 @@ def test_404_generic(self): assert result.retryable is True assert result.should_fallback is False + def test_404_bare_model_id_missing_prefix_is_model_not_found(self): + """A bare id the provider only serves as ``vendor/id`` is malformed. + + Regression for #78796: NVIDIA NIM answers a prefix-less + ``nemotron-3-ultra-550b-a55b`` with a naked ``404 page not found``. + Without the catalogue check this fell into the generic branch and + burned three retries on a deterministic failure, reporting what + looked like an outage. + """ + e = MockAPIError("404 page not found", status_code=404) + result = classify_api_error( + e, provider="nvidia", model="nemotron-3-ultra-550b-a55b" + ) + assert result.reason == FailoverReason.model_not_found + assert result.retryable is False + + def test_404_correctly_prefixed_model_stays_generic(self): + """A properly prefixed id hitting a 404 is a real endpoint problem — + it must keep the retryable generic classification.""" + e = MockAPIError("404 page not found", status_code=404) + result = classify_api_error( + e, provider="nvidia", model="nvidia/nemotron-3-ultra-550b-a55b" + ) + assert result.reason == FailoverReason.unknown + assert result.retryable is True + + def test_404_unknown_bare_model_stays_generic(self): + """A local NIM container isn't in the catalogue — no verdict invented.""" + e = MockAPIError("404 page not found", status_code=404) + result = classify_api_error(e, provider="nvidia", model="my-local-nim") + assert result.reason == FailoverReason.unknown + assert result.retryable is True + # ── Provider policy-block (OpenRouter privacy/guardrail) ── diff --git a/tests/hermes_cli/test_model_normalize.py b/tests/hermes_cli/test_model_normalize.py index 767ed8ffd1a0b..765525fb2badd 100644 --- a/tests/hermes_cli/test_model_normalize.py +++ b/tests/hermes_cli/test_model_normalize.py @@ -137,3 +137,53 @@ def test_provider_path_rewrites_reasoner(self): def test_reasoner_keywords_map_to_v4_flash(self, model): assert _normalize_for_deepseek(model) == "deepseek-v4-flash" + +# ── Regression: issue #78796 ─────────────────────────────────────────── + +class TestIssue78796NvidiaPrefixRepair: + """A bare NVIDIA model id must regain its ``vendor/`` prefix. + + build.nvidia.com serves ``nvidia/nemotron-…``; a bare + ``nemotron-3-ultra-550b-a55b`` returns a naked ``404 page not found`` + that never names the model, so the failure reads like an outage. + """ + + @pytest.mark.parametrize("model,expected", [ + ("nemotron-3-ultra-550b-a55b", "nvidia/nemotron-3-ultra-550b-a55b"), + ("nemotron-3-super-120b-a12b", "nvidia/nemotron-3-super-120b-a12b"), + ( + "nemotron-3-nano-omni-30b-a3b-reasoning", + "nvidia/nemotron-3-nano-omni-30b-a3b-reasoning", + ), + ]) + def test_bare_nemotron_regains_prefix(self, model, expected): + assert normalize_model_for_provider(model, "nvidia") == expected + + def test_third_party_model_gets_its_own_vendor(self): + """NIM also hosts third-party models — the prefix is the catalogue's, + not a hardcoded ``nvidia/``.""" + assert normalize_model_for_provider("glm-5.2", "nvidia") == "z-ai/glm-5.2" + + @pytest.mark.parametrize("model", [ + "nvidia/nemotron-3-ultra-550b-a55b", + "z-ai/glm-5.2", + ]) + def test_already_prefixed_is_untouched(self, model): + assert normalize_model_for_provider(model, "nvidia") == model + + @pytest.mark.parametrize("model", [ + "my-local-nim-container", + "some-finetune-v2", + ]) + def test_unknown_names_pass_through(self, model): + """The same provider id fronts local NIM containers. An id absent from + the catalogue is a lookup miss, not a guess — leave it alone.""" + assert normalize_model_for_provider(model, "nvidia") == model + + def test_other_providers_unaffected(self): + assert normalize_model_for_provider("my-model", "custom") == "my-model" + assert ( + normalize_model_for_provider("claude-sonnet-4.6", "openrouter") + == "anthropic/claude-sonnet-4.6" + ) + diff --git a/tests/hermes_cli/test_plugins.py b/tests/hermes_cli/test_plugins.py index 4d604ef8995c8..d4ea437b82255 100644 --- a/tests/hermes_cli/test_plugins.py +++ b/tests/hermes_cli/test_plugins.py @@ -383,6 +383,54 @@ def test_block_message_returned_for_valid_directive(self, monkeypatch): class TestPreToolCallDirective: """Tests for the extended (block | approve) directive helper.""" + def test_first_party_observer_receives_pre_tool_call(self, monkeypatch): + from hermes_cli import observability + from hermes_cli.plugins import get_pre_tool_call_directive + + observed = [] + monkeypatch.setattr( + observability, + "observe_lifecycle", + lambda hook_name, **kwargs: observed.append((hook_name, kwargs)), + ) + monkeypatch.setattr( + "hermes_cli.plugins.invoke_hook", + lambda hook_name, **kwargs: [], + ) + + assert get_pre_tool_call_directive( + "write_file", + {"path": "README.md"}, + task_id="task-1", + session_id="session-1", + tool_call_id="call-1", + ) == (None, None) + assert observed == [ + ( + "pre_tool_call", + { + "tool_name": "write_file", + "args": {"path": "README.md"}, + "task_id": "task-1", + "session_id": "session-1", + "tool_call_id": "call-1", + "turn_id": "", + "api_request_id": "", + "middleware_trace": [], + }, + ) + ] + + def test_approve_directive_returned(self, monkeypatch): + from hermes_cli.plugins import get_pre_tool_call_directive + monkeypatch.setattr( + "hermes_cli.plugins.invoke_hook", + lambda hook_name, **kwargs: [ + {"action": "approve", "message": "needs human ok"} + ], + ) + assert get_pre_tool_call_directive("write_file", {}) == ( + "approve", "needs human ok") def test_approve_without_message_is_valid(self, monkeypatch): """approve may omit a message (block may not).""" @@ -399,6 +447,33 @@ class TestResolvePreToolBlock: directive (incl. the approve→gate escalation) to a block message.""" + def test_approve_gate_receives_tool_observability_context(self, monkeypatch): + from hermes_cli.plugins import resolve_pre_tool_block + from tools import approval + + seen = {} + monkeypatch.setattr( + "hermes_cli.plugins.invoke_hook", + lambda hook_name, **kwargs: [ + {"action": "approve", "message": "why"} + ], + ) + + def _approve(*args, **kwargs): + seen["turn_id"] = approval._approval_turn_id.get() + seen["tool_call_id"] = approval._approval_tool_call_id.get() + return {"approved": True, "message": None} + + monkeypatch.setattr("tools.approval.request_tool_approval", _approve) + + assert resolve_pre_tool_block( + "write_file", + {}, + turn_id="turn-1", + tool_call_id="call-1", + ) is None + assert seen == {"turn_id": "turn-1", "tool_call_id": "call-1"} + def test_approve_passes_plugin_rule_key_to_gate(self, monkeypatch): from hermes_cli.plugins import resolve_pre_tool_block diff --git a/tests/hermes_cli/test_relay_shared_metrics.py b/tests/hermes_cli/test_relay_shared_metrics.py index 71abad092d48f..18190d9cb19ba 100644 --- a/tests/hermes_cli/test_relay_shared_metrics.py +++ b/tests/hermes_cli/test_relay_shared_metrics.py @@ -36,6 +36,12 @@ TASK_ENTRYPOINTS, TASK_OUTCOMES, TASK_TERMINATIONS, + TOOL_APPROVAL_ATTRIBUTIONS, + TOOL_APPROVAL_OUTCOMES, + TOOL_CATEGORIES, + TOOL_LATENCY_BUCKETS, + TOOL_OUTCOMES, + TOOL_RETRY_BUCKETS, count_bucket, duration_bucket, execution_surface, @@ -45,6 +51,14 @@ task_start_fields, task_terminal_fields, task_terminal_state, + tool_approval_counter, + tool_approval_outcome, + tool_call_dimensions, + tool_category, + tool_latency_bucket, + tool_outcome, + tool_retry_bucket, + tool_terminal_fields, ) @@ -78,6 +92,11 @@ def _task_dimension_schema(kind: str) -> dict[str, object]: return schema["$defs"][kind]["properties"]["dimensions"] +def _tool_dimension_schema(kind: str) -> dict[str, object]: + schema = json.loads(SCHEMA_PATH.read_text(encoding="utf-8")) + return schema["$defs"][kind]["properties"]["dimensions"] + + def _dimensions() -> dict[str, str]: return { "model": "anthropic/claude-sonnet-4.6", @@ -329,6 +348,129 @@ def test_v1_package_schema_retains_the_legacy_model_contract(): } +def test_package_schema_matches_the_tool_contract(): + schema = json.loads(SCHEMA_PATH.read_text(encoding="utf-8")) + tool = _tool_dimension_schema("tool_call_counter")["properties"] + approval = _tool_dimension_schema("tool_approval_counter")["properties"] + + assert set(tool["tool_category"]["enum"]) == TOOL_CATEGORIES + assert set(tool["outcome"]["enum"]) == TOOL_OUTCOMES + assert set(tool["approval_outcome"]["enum"]) == TOOL_APPROVAL_OUTCOMES + assert tool["latency_bucket"] == {"$ref": "#/$defs/tool_latency_bucket"} + assert tool["retry_count_bucket"] == {"$ref": "#/$defs/tool_retry_bucket"} + assert set(schema["$defs"]["tool_latency_bucket"]["enum"]) == ( + TOOL_LATENCY_BUCKETS + ) + assert set(schema["$defs"]["tool_retry_bucket"]["enum"]) == TOOL_RETRY_BUCKETS + assert set(approval["attribution"]["enum"]) == TOOL_APPROVAL_ATTRIBUTIONS + assert set(approval["outcome"]["enum"]) == ( + TOOL_APPROVAL_OUTCOMES - {"not_required"} + ) + + +@pytest.mark.parametrize( + ("toolset", "expected"), + [ + ("", "unknown"), + ("file", "file"), + ("terminal", "terminal"), + ("code_execution", "code_execution"), + ("delegation", "delegation"), + ("skills", "skill"), + ("browser-cdp", "browser"), + ("image_gen", "media"), + ("homeassistant", "home_automation"), + ("kanban", "planning"), + ("project", "project"), + ("discord", "communication"), + ("feishu_doc", "communication"), + ("mcp-github", "mcp"), + ("private_plugin", "other"), + ], +) +def test_tool_category_uses_bounded_runtime_toolsets(toolset, expected): + assert tool_category({"toolset": toolset}) == expected + + +def test_tool_category_does_not_classify_raw_tool_names(): + assert tool_category({"tool_name": "read_file"}) == "unknown" + + +@pytest.mark.parametrize( + ("status", "expected"), + [ + ("ok", "success"), + ("error", "failed"), + ("blocked", "blocked"), + ("cancelled", "cancelled"), + ("timeout", "timed_out"), + ("private", "unknown"), + (None, "unknown"), + ], +) +def test_tool_outcome_is_bounded(status, expected): + assert tool_outcome({"status": status}) == expected + + +@pytest.mark.parametrize( + ("choice", "expected"), + [ + ("once", "approved"), + ("session", "approved"), + ("always", "approved"), + ("smart_approve", "approved"), + ("deny", "denied"), + ("smart_deny", "denied"), + ("timeout", "timed_out"), + (None, "unknown"), + ], +) +def test_tool_approval_outcome_is_bounded(choice, expected): + assert tool_approval_outcome({"choice": choice}) == expected + + +@pytest.mark.parametrize( + ("duration_ms", "expected"), + [ + (0, "lt_100ms"), + (100, "100ms_to_250ms"), + (250, "250ms_to_500ms"), + (500, "500ms_to_1s"), + (1_000, "1s_to_2s"), + (2_000, "2s_to_5s"), + (5_000, "5s_to_10s"), + (10_000, "10s_to_30s"), + (30_000, "gte_30s"), + (-1, "unknown"), + (True, "unknown"), + ("100", "unknown"), + ], +) +def test_tool_latency_bucket_is_bounded(duration_ms, expected): + assert tool_latency_bucket(duration_ms) == expected + + +@pytest.mark.parametrize( + ("retry_count", "expected"), + [ + (0, "0"), + (1, "1"), + (2, "2"), + (3, "3_to_5"), + (6, "6_to_10"), + (11, "gte_11"), + (None, "unknown"), + (-1, "unknown"), + (True, "unknown"), + ], +) +def test_tool_retry_bucket_requires_an_explicit_non_negative_count( + retry_count, + expected, +): + assert tool_retry_bucket(retry_count) == expected + + def test_model_call_fields_report_terminal_model_and_provider_without_a_catalog(): assert model_call_fields({ "model": "fallback/model", @@ -425,6 +567,50 @@ def test_model_call_fields_collapse_malformed_identifiers(field, value): assert model_call_fields(event)[field] == "unknown" +def test_tool_subscriber_contract_accepts_only_bounded_events(): + terminal = SimpleNamespace( + kind="scope", + category="tool", + category_profile={}, + name="hermes.tool_call", + scope_category="end", + metadata={"hermes.metrics.schema_version": "hermes.metrics.event.v2"}, + data={ + "approval_outcome": "approved", + "latency_bucket": "250ms_to_500ms", + "outcome": "success", + "retry_count_bucket": "0", + "tool_category": "terminal", + }, + ) + assert tool_call_dimensions(terminal) == terminal.data + + terminal.data["result"] = "must-not-pass" + assert tool_call_dimensions(terminal) is None + terminal.data.pop("result") + terminal.data["tool_category"] = "private-tool-name" + assert tool_call_dimensions(terminal) is None + terminal.data["tool_category"] = "terminal" + terminal.category_profile["tool_name"] = "must-not-pass" + assert tool_call_dimensions(terminal) is None + + approval = SimpleNamespace( + kind="mark", + category=None, + category_profile=None, + name="hermes.tool_approval", + scope_category=None, + metadata={"hermes.metrics.schema_version": "hermes.metrics.event.v2"}, + data={"attribution": "unattributed", "outcome": "denied"}, + ) + assert tool_approval_counter(approval) == ( + "hermes.tool_approval.count", + approval.data, + ) + approval.data["command"] = "must-not-pass" + assert tool_approval_counter(approval) is None + + def test_store_does_not_record_the_retired_model_metric(tmp_path): store = SharedMetricsStore(tmp_path / "metrics.sqlite3", tmp_path / "outbox") diff --git a/tests/hermes_cli/test_relay_shared_metrics_runtime.py b/tests/hermes_cli/test_relay_shared_metrics_runtime.py index ec5f4e3c7f436..3dabef54493ec 100644 --- a/tests/hermes_cli/test_relay_shared_metrics_runtime.py +++ b/tests/hermes_cli/test_relay_shared_metrics_runtime.py @@ -30,11 +30,14 @@ def __init__(self) -> None: self.events: list[tuple[Any, ...]] = [] self._callbacks: dict[str, Any] = {} self._starts: dict[Any, dict[str, Any]] = {} + self._tool_starts: dict[Any, dict[str, Any]] = {} self._scope_starts: dict[Any, dict[str, Any]] = {} self._scope = contextvars.ContextVar("relay_scope", default=None) self._scope_stack = contextvars.ContextVar("relay_scope_stack", default=None) self._scope_serial = 0 - self.ScopeType = SimpleNamespace(Agent="agent", Function="function") + self.ScopeType = SimpleNamespace( + Agent="agent", Function="function", Tool="tool" + ) self.LLMRequest = _Request self.scope = SimpleNamespace( push=self._scope_push, @@ -42,6 +45,7 @@ def __init__(self) -> None: event=self._scope_event, ) self.llm = SimpleNamespace(call=self._llm_call, call_end=self._llm_call_end) + self.tools = SimpleNamespace(call=self._tool_call, call_end=self._tool_call_end) self.subscribers = SimpleNamespace( register=self._register, deregister=self._deregister, @@ -102,6 +106,17 @@ def _scope_pop(self, handle: Any, **kwargs: Any) -> None: def _scope_event(self, name: str, **kwargs: Any) -> None: self.events.append(("scope.event", name, kwargs)) + event = SimpleNamespace( + kind="mark", + category=None, + name=name, + scope_category=None, + category_profile=None, + metadata=kwargs.get("metadata"), + data=kwargs.get("data"), + ) + for callback in list(self._callbacks.values()): + callback(event) def _get_scope_stack(self) -> Any: stack = self._scope_stack.get() @@ -145,6 +160,41 @@ def _llm_call_end( for callback in list(self._callbacks.values()): callback(event) + def _tool_call( + self, + name: str, + args: dict[str, Any], + **kwargs: Any, + ) -> Any: + handle = ("tool", name, len(self._tool_starts)) + self._tool_starts[handle] = kwargs + self.events.append(("tool.call", name, args, kwargs)) + return handle + + def _tool_call_end( + self, + handle: Any, + result: dict[str, Any], + **kwargs: Any, + ) -> None: + start = self._tool_starts.pop(handle) + self.events.append(("tool.call_end", handle, result, kwargs)) + event = SimpleNamespace( + kind="scope", + category="tool", + name=handle[1], + scope_category="end", + category_profile={}, + metadata={ + **start["metadata"], + **kwargs["metadata"], + "otel.status_code": "OK", + }, + data=result, + ) + for callback in list(self._callbacks.values()): + callback(event) + def _register(self, name: str, callback: Any) -> None: self._callbacks[name] = callback self.events.append(("subscribers.register", name)) @@ -196,6 +246,7 @@ def test_direct_runtime_records_without_enabling_a_plugin(direct_runtime, tmp_pa base = { "session_id": "sensitive-session", "task_id": "task-1", + "turn_id": "turn-1", "api_request_id": "request-1", "platform": "cli", "provider": "custom", @@ -211,14 +262,33 @@ def test_direct_runtime_records_without_enabling_a_plugin(direct_runtime, tmp_pa **base, request={"body": {"messages": ["sensitive-prompt"]}}, ) + lifecycle.invoke_hook( + "pre_tool_call", + **base, + tool_call_id="sensitive-tool-call", + tool_name="terminal", + toolset="terminal", + args={"command": "sensitive-command"}, + ) + lifecycle.invoke_hook( + "post_approval_response", + turn_id=base["turn_id"], + tool_call_id="sensitive-tool-call", + choice="once", + command="sensitive-command", + description="sensitive-approval-description", + ) lifecycle.invoke_hook( "post_tool_call", **base, tool_call_id="sensitive-tool-call", tool_name="terminal", + toolset="terminal", args={"command": "sensitive-command"}, result={"output": "sensitive-tool-result"}, status="ok", + duration_ms=275, + retry_count=0, ) lifecycle.invoke_hook( "api_request_error", @@ -258,6 +328,10 @@ def test_direct_runtime_records_without_enabling_a_plugin(direct_runtime, tmp_pa starts = [event for event in direct_runtime.events if event[0] == "llm.call"] ends = [event for event in direct_runtime.events if event[0] == "llm.call_end"] + tool_starts = [event for event in direct_runtime.events if event[0] == "tool.call"] + tool_ends = [ + event for event in direct_runtime.events if event[0] == "tool.call_end" + ] scope_starts = [ event for event in direct_runtime.events if event[0] == "scope.push" ] @@ -272,6 +346,17 @@ def test_direct_runtime_records_without_enabling_a_plugin(direct_runtime, tmp_pa } assert len(starts) == 1 assert len(ends) == 1 + assert len(tool_starts) == 1 + assert len(tool_ends) == 1 + assert tool_starts[0][1] == "hermes.tool_call" + assert tool_starts[0][2] == {} + assert tool_ends[0][2] == { + "approval_outcome": "approved", + "latency_bucket": "250ms_to_500ms", + "outcome": "success", + "retry_count_bucket": "0", + "tool_category": "terminal", + } assert starts[0][2] == {} assert starts[0][3]["model_name"] == "unknown" assert ends[0][2] == { @@ -285,6 +370,7 @@ def test_direct_runtime_records_without_enabling_a_plugin(direct_runtime, tmp_pa assert "sensitive-command" not in serialized_events assert "sensitive-tool-result" not in serialized_events assert "sensitive-tool-call" not in serialized_events + assert "sensitive-approval-description" not in serialized_events assert "gpt-sensitive-model-id" not in serialized_events assert plugins.get_plugin_manager().list_plugins() == [] @@ -297,12 +383,35 @@ def test_direct_runtime_records_without_enabling_a_plugin(direct_runtime, tmp_pa "hermes.model_route.count", "hermes.task_run.finished", "hermes.task_run.started", + "hermes.tool_approval.count", + "hermes.tool_call.count", } assert metrics["hermes.model_route.count"]["dimensions"] == { "model": "claude-sonnet", "provider": "anthropic", } assert metrics["hermes.model_route.count"]["value"] == 1 + assert metrics["hermes.tool_call.count"] == { + "name": "hermes.tool_call.count", + "type": "counter", + "dimensions": { + "approval_outcome": "approved", + "latency_bucket": "250ms_to_500ms", + "outcome": "success", + "retry_count_bucket": "0", + "tool_category": "terminal", + }, + "value": 1, + } + assert metrics["hermes.tool_approval.count"] == { + "name": "hermes.tool_approval.count", + "type": "counter", + "dimensions": { + "attribution": "tool_call", + "outcome": "approved", + }, + "value": 1, + } assert metrics["hermes.task_run.started"] == { "name": "hermes.task_run.started", "type": "counter", @@ -370,6 +479,21 @@ def base(index: int) -> dict[str, Any]: error={"message": prompt_canary}, ) lifecycle.invoke_hook("pre_api_request", **success, retry_count=1) + lifecycle.invoke_hook( + "pre_tool_call", + **success, + tool_call_id="sensitive-tool-call", + tool_name="terminal", + args={"command": prompt_canary}, + ) + lifecycle.invoke_hook( + "post_approval_response", + turn_id=success["turn_id"], + tool_call_id="sensitive-tool-call", + choice="session", + command=prompt_canary, + description="sensitive-approval-description", + ) lifecycle.invoke_hook( "post_tool_call", **success, @@ -378,6 +502,8 @@ def base(index: int) -> dict[str, Any]: args={"command": prompt_canary}, result={"output": tool_canary}, status="ok", + duration_ms=125, + retry_count=0, ) lifecycle.invoke_hook( "post_api_request", @@ -399,6 +525,23 @@ def base(index: int) -> dict[str, Any]: lifecycle.invoke_hook("on_session_start", **failed) lifecycle.invoke_hook("pre_llm_call", **failed, messages=[prompt_canary]) lifecycle.invoke_hook("pre_api_request", **failed, retry_count=0) + lifecycle.invoke_hook( + "pre_tool_call", + **failed, + tool_call_id="sensitive-failed-tool-call", + tool_name="read_file", + args={"path": prompt_canary}, + ) + lifecycle.invoke_hook( + "post_tool_call", + **failed, + tool_call_id="sensitive-failed-tool-call", + tool_name="read_file", + args={"path": prompt_canary}, + result={"error": tool_canary}, + status="error", + duration_ms=750, + ) lifecycle.invoke_hook( "api_request_error", **failed, @@ -420,6 +563,23 @@ def base(index: int) -> dict[str, Any]: lifecycle.invoke_hook("on_session_start", **cancelled) lifecycle.invoke_hook("pre_llm_call", **cancelled, messages=[prompt_canary]) lifecycle.invoke_hook("pre_api_request", **cancelled, retry_count=0) + lifecycle.invoke_hook( + "pre_tool_call", + **cancelled, + tool_call_id="sensitive-cancelled-tool-call", + tool_name="browser_navigate", + args={"url": prompt_canary}, + ) + lifecycle.invoke_hook( + "post_tool_call", + **cancelled, + tool_call_id="sensitive-cancelled-tool-call", + tool_name="browser_navigate", + args={"url": prompt_canary}, + result={"error": tool_canary}, + status="cancelled", + duration_ms=31_000, + ) lifecycle.invoke_hook( "on_session_end", **cancelled, @@ -454,6 +614,43 @@ def base(index: int) -> dict[str, Any]: "provider": "custom", } assert model_counter["value"] == 3 + assert { + counter["dimensions"]["outcome"] + for counter in by_metric["hermes.tool_call.count"] + } == {"success", "failed", "cancelled"} + tool_by_outcome = { + counter["dimensions"]["outcome"]: counter["dimensions"] + for counter in by_metric["hermes.tool_call.count"] + } + assert tool_by_outcome["success"] == { + "approval_outcome": "approved", + "latency_bucket": "100ms_to_250ms", + "outcome": "success", + "retry_count_bucket": "0", + "tool_category": "terminal", + } + assert tool_by_outcome["failed"] == { + "approval_outcome": "not_required", + "latency_bucket": "500ms_to_1s", + "outcome": "failed", + "retry_count_bucket": "unknown", + "tool_category": "file", + } + assert tool_by_outcome["cancelled"] == { + "approval_outcome": "not_required", + "latency_bucket": "gte_30s", + "outcome": "cancelled", + "retry_count_bucket": "unknown", + "tool_category": "browser", + } + assert len(by_metric["hermes.tool_approval.count"]) == 1 + approval_counter = by_metric["hermes.tool_approval.count"][0] + assert approval_counter["dimensions"] == { + "attribution": "tool_call", + "outcome": "approved", + } + assert approval_counter["value"] == 1 + assert approval_counter["packaged_value"] == 1 terminal_by_outcome = { counter["dimensions"]["outcome"]: counter for counter in by_metric["hermes.task_run.finished"] @@ -502,10 +699,183 @@ def base(index: int) -> dict[str, Any]: "sensitive-task", "sensitive-request", "sensitive-tool-call", + "sensitive-failed-tool-call", + "sensitive-cancelled-tool-call", + "sensitive-approval-description", ): assert canary not in serialized_analytics +def test_real_binding_correlates_plugin_approval_denial_to_tool_metric( + real_binding_runtime, + tmp_path, + monkeypatch, +): + from hermes_cli.observability.shared_metrics import SharedMetricsStore + from tools import approval + + assert real_binding_runtime._native is not None + base = { + "session_id": "sensitive-session", + "task_id": "sensitive-task", + "turn_id": "sensitive-turn", + "api_request_id": "sensitive-request", + "platform": "cli", + } + + def plugin_hook(hook_name: str, **kwargs: Any) -> list[dict[str, str]]: + if hook_name == "pre_tool_call": + return [{"action": "approve", "message": "sensitive-rule"}] + return [] + + monkeypatch.setattr(plugins, "invoke_hook", plugin_hook) + monkeypatch.setattr(approval, "_YOLO_MODE_FROZEN", False) + monkeypatch.setattr(approval, "is_current_session_yolo_enabled", lambda: False) + monkeypatch.setattr(approval, "is_approved", lambda *args: False) + monkeypatch.setattr(approval, "get_current_session_key", lambda: "session-key") + monkeypatch.setattr(approval, "_is_interactive_cli", lambda: True) + monkeypatch.setattr(approval, "_is_gateway_approval_context", lambda: False) + monkeypatch.setattr(approval, "prompt_dangerous_approval", lambda *args, **kwargs: "deny") + + lifecycle.invoke_hook("on_session_start", **base) + lifecycle.invoke_hook("pre_llm_call", **base, messages=["sensitive-prompt"]) + block_message = plugins.resolve_pre_tool_block( + "write_file", + {"path": "sensitive-path"}, + task_id=base["task_id"], + session_id=base["session_id"], + turn_id=base["turn_id"], + api_request_id=base["api_request_id"], + tool_call_id="sensitive-tool-call", + ) + assert block_message is not None + assert "User denied" in block_message + + lifecycle.invoke_hook( + "post_tool_call", + **base, + tool_call_id="sensitive-tool-call", + tool_name="write_file", + args={"path": "sensitive-path"}, + result={"error": block_message}, + status="blocked", + duration_ms=12, + ) + lifecycle.invoke_hook( + "on_session_end", + **base, + completed=False, + failed=True, + interrupted=False, + turn_exit_reason="approval_denied", + ) + lifecycle.finalize_session(session_id=base["session_id"]) + + root = tmp_path / "hermes-home" / "telemetry" / "shared_metrics" + store = SharedMetricsStore(root / "metrics.sqlite3", root / "outbox") + snapshot = store.counter_snapshot() + tool_metrics = [ + counter + for counter in snapshot + if counter["metric_name"] == "hermes.tool_call.count" + ] + assert len(tool_metrics) == 1 + assert tool_metrics[0]["dimensions"] == { + "approval_outcome": "denied", + "latency_bucket": "lt_100ms", + "outcome": "blocked", + "retry_count_bucket": "unknown", + "tool_category": "file", + } + approval_metrics = [ + counter + for counter in snapshot + if counter["metric_name"] == "hermes.tool_approval.count" + ] + assert len(approval_metrics) == 1 + assert approval_metrics[0]["dimensions"] == { + "attribution": "tool_call", + "outcome": "denied", + } + assert "sensitive" not in json.dumps(snapshot) + + +def test_real_binding_aggregates_tool_and_approval_timeouts( + real_binding_runtime, + tmp_path, +): + from hermes_cli.observability.shared_metrics import SharedMetricsStore + + assert real_binding_runtime._native is not None + base = { + "session_id": "timeout-sensitive-session", + "task_id": "timeout-sensitive-task", + "turn_id": "timeout-sensitive-turn", + "platform": "cli", + } + + lifecycle.invoke_hook("on_session_start", **base) + lifecycle.invoke_hook("pre_llm_call", **base, messages=["timeout-sensitive-prompt"]) + lifecycle.invoke_hook( + "pre_tool_call", + **base, + tool_call_id="timeout-sensitive-tool-call", + tool_name="terminal", + args={"command": "timeout-sensitive-command"}, + ) + lifecycle.invoke_hook( + "post_approval_response", + **base, + tool_call_id="timeout-sensitive-tool-call", + choice="timeout", + command="timeout-sensitive-command", + ) + lifecycle.invoke_hook( + "post_tool_call", + **base, + tool_call_id="timeout-sensitive-tool-call", + tool_name="terminal", + result={"error": "timeout-sensitive-result"}, + status="timeout", + duration_ms=30_000, + ) + lifecycle.invoke_hook( + "on_session_end", + **base, + completed=False, + failed=True, + interrupted=False, + turn_exit_reason="provider_timeout", + ) + lifecycle.finalize_session(session_id=base["session_id"]) + + root = tmp_path / "hermes-home" / "telemetry" / "shared_metrics" + snapshot = SharedMetricsStore( + root / "metrics.sqlite3", + root / "outbox", + ).counter_snapshot() + [tool_metric] = [ + counter + for counter in snapshot + if counter["metric_name"] == "hermes.tool_call.count" + ] + assert tool_metric["dimensions"] == { + "approval_outcome": "timed_out", + "latency_bucket": "gte_30s", + "outcome": "timed_out", + "retry_count_bucket": "unknown", + "tool_category": "terminal", + } + [approval_metric] = [ + counter + for counter in snapshot + if counter["metric_name"] == "hermes.tool_approval.count" + ] + assert approval_metric["dimensions"] == { + "attribution": "tool_call", + "outcome": "timed_out", + } + assert "timeout-sensitive" not in json.dumps(snapshot) @@ -810,24 +1180,29 @@ def test_disabling_shared_metrics_stops_collection_and_shutdown_export( assert not relay_shared_metrics.enabled() counters_before_stale_event = runtime.subscriber.store.counter_snapshot() - runtime.subscriber(SimpleNamespace( - kind="scope", - category="function", - category_profile=None, - name="hermes.task_run", - scope_category="start", - metadata={ - "hermes.metrics.schema_version": "hermes.metrics.event.v1", - relay_runtime.RUNTIME_INSTANCE_KEY: runtime.host.runtime_id, - }, - data={"entrypoint": "interactive", "execution_surface": "cli"}, - )) + runtime.subscriber( + SimpleNamespace( + kind="scope", + category="function", + category_profile=None, + name="hermes.task_run", + scope_category="start", + metadata={ + "hermes.metrics.schema_version": "hermes.metrics.event.v1", + relay_runtime.RUNTIME_INSTANCE_KEY: runtime.host.runtime_id, + }, + data={"entrypoint": "interactive", "execution_surface": "cli"}, + ) + ) assert runtime.subscriber.store.counter_snapshot() == counters_before_stale_event - assert runtime.start_task({ - "session_id": "session", - "task_id": "stale-runtime-task", - "platform": "cli", - }) is None + assert ( + runtime.start_task({ + "session_id": "session", + "task_id": "stale-runtime-task", + "platform": "cli", + }) + is None + ) relay_shared_metrics.finish_task_run( session_id="session", task_id="task", @@ -1092,6 +1467,7 @@ def test_subagent_agent_boundary_closes_its_own_scope( ) AIAgent.run_conversation(child_agent, "private", task_id="child-task") elif terminal == "exception": + def fail(*_args, **_kwargs): raise RuntimeError("child failed") @@ -1099,6 +1475,7 @@ def fail(*_args, **_kwargs): with pytest.raises(RuntimeError, match="child failed"): AIAgent.run_conversation(child_agent, "private", task_id="child-task") elif terminal == "cancelled": + def cancel(*_args, **_kwargs): raise KeyboardInterrupt @@ -1106,6 +1483,7 @@ def cancel(*_args, **_kwargs): with pytest.raises(KeyboardInterrupt): AIAgent.run_conversation(child_agent, "private", task_id="child-task") else: + def time_out(*_args, **_kwargs): raise TimeoutError("child timed out") @@ -1278,6 +1656,525 @@ def test_same_request_id_is_isolated_between_tasks(direct_runtime): assert all(fields["retry_count_bucket"] == "0" for fields in task_ends) +def test_reused_tool_call_id_is_counted_for_each_provider_request(direct_runtime): + base = { + "session_id": "s1", + "task_id": "t1", + "turn_id": "turn-1", + "platform": "cli", + } + lifecycle.invoke_hook("pre_llm_call", **base) + + for api_request_id in ("request-1", "request-2"): + call = { + **base, + "api_request_id": api_request_id, + "tool_call_id": "provider-reused-id", + "tool_name": "terminal", + } + lifecycle.invoke_hook("pre_tool_call", **call, args={"command": "private"}) + lifecycle.invoke_hook( + "post_tool_call", + **call, + result={"output": "private"}, + status="ok", + ) + + lifecycle.invoke_hook( + "on_session_end", + **base, + completed=True, + failed=False, + interrupted=False, + turn_exit_reason="text_response(stop)", + ) + lifecycle.finalize_session(session_id="s1") + + tool_ends = [ + event for event in direct_runtime.events if event[0] == "tool.call_end" + ] + assert len(tool_ends) == 2 + [task_end] = [ + event + for event in direct_runtime.events + if event[0] == "scope.pop" and event[1][1] == "hermes.task_run" + ] + assert task_end[2]["output"]["tool_call_count_bucket"] == "2" + + +def test_partial_terminal_context_reuses_the_pending_tool_span(direct_runtime): + base = { + "session_id": "s1", + "task_id": "t1", + "turn_id": "turn-1", + "api_request_id": "request-1", + "platform": "cli", + "tool_call_id": "tool-1", + "tool_name": "terminal", + } + lifecycle.invoke_hook("pre_llm_call", **base) + lifecycle.invoke_hook("pre_tool_call", **base) + lifecycle.invoke_hook( + "post_tool_call", + **{key: value for key, value in base.items() if key != "api_request_id"}, + result={"output": "private"}, + status="ok", + ) + lifecycle.invoke_hook( + "on_session_end", + **base, + completed=True, + failed=False, + interrupted=False, + turn_exit_reason="text_response(stop)", + ) + lifecycle.finalize_session(session_id="s1") + + [tool_end] = [ + event for event in direct_runtime.events if event[0] == "tool.call_end" + ] + assert tool_end[2]["outcome"] == "success" + [task_end] = [ + event + for event in direct_runtime.events + if event[0] == "scope.pop" and event[1][1] == "hermes.task_run" + ] + assert task_end[2]["output"]["tool_call_count_bucket"] == "1" + + +def test_partial_terminal_variants_do_not_double_count_a_completed_call( + direct_runtime, +): + base = { + "session_id": "s1", + "task_id": "t1", + "turn_id": "turn-1", + "api_request_id": "request-1", + "platform": "cli", + "tool_call_id": "tool-1", + "tool_name": "terminal", + } + lifecycle.invoke_hook("pre_llm_call", **base) + lifecycle.invoke_hook("pre_tool_call", **base) + for omitted_field in ("api_request_id", "turn_id"): + lifecycle.invoke_hook( + "post_tool_call", + **{key: value for key, value in base.items() if key != omitted_field}, + result={"output": "private"}, + status="ok", + ) + lifecycle.invoke_hook( + "on_session_end", + **base, + completed=True, + failed=False, + interrupted=False, + turn_exit_reason="text_response(stop)", + ) + lifecycle.finalize_session(session_id="s1") + + tool_ends = [ + event for event in direct_runtime.events if event[0] == "tool.call_end" + ] + assert len(tool_ends) == 1 + [task_end] = [ + event + for event in direct_runtime.events + if event[0] == "scope.pop" and event[1][1] == "hermes.task_run" + ] + assert task_end[2]["output"]["tool_call_count_bucket"] == "1" + + +def test_ambiguous_partial_terminal_does_not_create_a_phantom_tool_span( + direct_runtime, +): + base = { + "session_id": "s1", + "task_id": "t1", + "turn_id": "turn-1", + "platform": "cli", + "tool_call_id": "provider-reused-id", + "tool_name": "terminal", + } + lifecycle.invoke_hook("pre_llm_call", **base) + for api_request_id in ("request-1", "request-2"): + lifecycle.invoke_hook( + "pre_tool_call", + **base, + api_request_id=api_request_id, + ) + + lifecycle.invoke_hook( + "post_tool_call", + **base, + result={"output": "ambiguous-private-result"}, + status="ok", + ) + lifecycle.invoke_hook( + "on_session_end", + **base, + completed=False, + failed=True, + interrupted=False, + turn_exit_reason="system_aborted", + ) + lifecycle.finalize_session(session_id="s1") + + tool_ends = [ + event for event in direct_runtime.events if event[0] == "tool.call_end" + ] + assert len(tool_ends) == 2 + assert all(event[2]["outcome"] == "failed" for event in tool_ends) + [task_end] = [ + event + for event in direct_runtime.events + if event[0] == "scope.pop" and event[1][1] == "hermes.task_run" + ] + assert task_end[2]["output"]["tool_call_count_bucket"] == "2" + + +def test_reused_task_id_starts_a_new_run_for_each_turn(direct_runtime): + for turn_id in ("turn-1", "turn-2"): + base = { + "session_id": "reused-session", + "task_id": "reused-session", + "turn_id": turn_id, + "platform": "api", + } + lifecycle.invoke_hook("pre_llm_call", **base) + lifecycle.invoke_hook( + "post_tool_call", + **base, + api_request_id=f"request-{turn_id}", + tool_call_id=f"tool-{turn_id}", + tool_name="read_file", + result={"output": "private"}, + status="ok", + ) + lifecycle.invoke_hook( + "on_session_end", + **base, + completed=True, + failed=False, + interrupted=False, + turn_exit_reason="text_response(stop)", + ) + + lifecycle.finalize_session(session_id="reused-session") + + task_starts = [ + event + for event in direct_runtime.events + if event[0] == "scope.push" and event[1] == "hermes.task_run" + ] + task_ends = [ + event + for event in direct_runtime.events + if event[0] == "scope.pop" and event[1][1] == "hermes.task_run" + ] + tool_ends = [ + event for event in direct_runtime.events if event[0] == "tool.call_end" + ] + assert len(task_starts) == 2 + assert len(task_ends) == 2 + assert len(tool_ends) == 2 + assert all( + event[2]["output"]["tool_call_count_bucket"] == "1" for event in task_ends + ) + + +def test_late_tool_result_does_not_attach_to_reused_task_id(direct_runtime): + first = { + "session_id": "reused-session", + "task_id": "reused-session", + "turn_id": "turn-1", + "platform": "api", + } + lifecycle.invoke_hook("pre_llm_call", **first) + lifecycle.invoke_hook( + "pre_tool_call", + **first, + api_request_id="request-1", + tool_call_id="tool-1", + tool_name="terminal", + ) + lifecycle.invoke_hook( + "on_session_end", + **first, + completed=False, + failed=True, + interrupted=False, + turn_exit_reason="timed_out", + ) + + second = {**first, "turn_id": "turn-2"} + runtime = relay_shared_metrics._get_runtime() + assert runtime is not None + assert runtime.start_task({ + "session_id": second["session_id"], + "task_id": second["task_id"], + "platform": second["platform"], + }) + lifecycle.invoke_hook( + "post_tool_call", + **first, + api_request_id="request-1", + tool_call_id="tool-1", + tool_name="terminal", + result={"output": "late-private-result"}, + status="ok", + ) + lifecycle.invoke_hook("pre_llm_call", **second) + lifecycle.invoke_hook( + "post_tool_call", + **second, + api_request_id="request-2", + tool_call_id="tool-2", + tool_name="read_file", + result={"output": "current-private-result"}, + status="ok", + ) + lifecycle.invoke_hook( + "on_session_end", + **second, + completed=True, + failed=False, + interrupted=False, + turn_exit_reason="text_response(stop)", + ) + lifecycle.finalize_session(session_id="reused-session") + + tool_ends = [ + event for event in direct_runtime.events if event[0] == "tool.call_end" + ] + assert len(tool_ends) == 2 + assert [event[2]["outcome"] for event in tool_ends] == [ + "timed_out", + "success", + ] + assert [event[2]["tool_category"] for event in tool_ends] == [ + "terminal", + "file", + ] + task_ends = [ + event + for event in direct_runtime.events + if event[0] == "scope.pop" and event[1][1] == "hermes.task_run" + ] + assert [ + event[2]["output"]["tool_call_count_bucket"] for event in task_ends + ] == ["1", "1"] + + +def test_pending_tool_is_closed_and_counted_when_task_is_interrupted(direct_runtime): + base = { + "session_id": "s1", + "task_id": "t1", + "turn_id": "turn-1", + "platform": "cli", + } + + lifecycle.invoke_hook("on_session_start", **base) + lifecycle.invoke_hook( + "pre_tool_call", + **base, + tool_call_id="tool-1", + tool_name="terminal", + args={"command": "must-not-pass"}, + ) + lifecycle.invoke_hook( + "on_session_end", + **base, + completed=False, + failed=False, + interrupted=True, + turn_exit_reason="interrupted_by_user", + ) + lifecycle.invoke_hook( + "post_tool_call", + **base, + tool_call_id="tool-1", + tool_name="terminal", + result={"output": "late-result-must-not-pass"}, + status="ok", + ) + lifecycle.finalize_session(session_id="s1") + + [tool_end] = [ + event for event in direct_runtime.events if event[0] == "tool.call_end" + ] + assert tool_end[2] == { + "approval_outcome": "not_required", + "latency_bucket": tool_end[2]["latency_bucket"], + "outcome": "cancelled", + "retry_count_bucket": "unknown", + "tool_category": "terminal", + } + [task_end] = [ + event + for event in direct_runtime.events + if event[0] == "scope.pop" and event[1][1] == "hermes.task_run" + ] + assert task_end[2]["output"]["tool_call_count_bucket"] == "1" + task_starts = [ + event + for event in direct_runtime.events + if event[0] == "scope.push" and event[1] == "hermes.task_run" + ] + assert len(task_starts) == 1 + + +def test_pending_tool_uses_the_outer_task_timeout_outcome(direct_runtime): + base = { + "session_id": "s1", + "task_id": "t1", + "turn_id": "turn-1", + "platform": "api", + } + lifecycle.invoke_hook("pre_llm_call", **base) + lifecycle.invoke_hook( + "pre_tool_call", + **base, + tool_call_id="tool-1", + tool_name="web_search", + ) + + relay_shared_metrics.finish_task_run( + session_id="s1", + task_id="t1", + platform="api", + error=TimeoutError("private timeout detail"), + ) + lifecycle.finalize_session(session_id="s1") + + [tool_end] = [ + event for event in direct_runtime.events if event[0] == "tool.call_end" + ] + assert tool_end[2]["outcome"] == "timed_out" + [task_end] = [ + event + for event in direct_runtime.events + if event[0] == "scope.pop" and event[1][1] == "hermes.task_run" + ] + assert task_end[2]["output"]["outcome"] == "timed_out" + assert task_end[2]["output"]["tool_call_count_bucket"] == "1" + assert "private timeout detail" not in repr(direct_runtime.events) + + +def test_approval_without_tool_context_is_counted_as_unattributed(direct_runtime): + base = { + "session_id": "s1", + "task_id": "t1", + "turn_id": "turn-1", + "platform": "cli", + } + + lifecycle.invoke_hook("on_session_start", **base) + lifecycle.invoke_hook("pre_llm_call", **base) + lifecycle.invoke_hook( + "post_approval_response", + turn_id="turn-1", + choice="deny", + command="must-not-pass", + ) + lifecycle.invoke_hook( + "on_session_end", + **base, + completed=False, + failed=True, + interrupted=False, + turn_exit_reason="approval_denied", + ) + lifecycle.finalize_session(session_id="s1") + + [approval] = [ + event + for event in direct_runtime.events + if event[0] == "scope.event" and event[1] == "hermes.tool_approval" + ] + assert approval[2]["data"] == { + "attribution": "unattributed", + "outcome": "denied", + } + + +def test_approval_with_unmatched_tool_id_is_counted_as_unattributed(direct_runtime): + base = { + "session_id": "s1", + "task_id": "t1", + "turn_id": "turn-1", + "platform": "cli", + } + + lifecycle.invoke_hook("pre_llm_call", **base) + lifecycle.invoke_hook( + "post_approval_response", + **base, + tool_call_id="spoofed-tool-call", + choice="deny", + command="must-not-pass", + ) + lifecycle.invoke_hook( + "on_session_end", + **base, + completed=False, + failed=True, + interrupted=False, + turn_exit_reason="approval_denied", + ) + lifecycle.finalize_session(session_id="s1") + + [approval] = [ + event + for event in direct_runtime.events + if event[0] == "scope.event" and event[1] == "hermes.tool_approval" + ] + assert approval[2]["data"] == { + "attribution": "unattributed", + "outcome": "denied", + } + + +def test_tool_category_comes_from_runtime_registry_metadata( + direct_runtime, + monkeypatch, +): + import model_tools + + monkeypatch.setattr( + model_tools, + "get_toolset_for_tool", + lambda name: "terminal" if name == "runtime_only_tool" else None, + ) + base = { + "session_id": "s1", + "task_id": "t1", + "turn_id": "turn-1", + "platform": "cli", + } + lifecycle.invoke_hook("pre_llm_call", **base) + lifecycle.invoke_hook( + "post_tool_call", + **base, + tool_call_id="tool-1", + tool_name="runtime_only_tool", + result={"output": "private"}, + status="ok", + ) + lifecycle.invoke_hook( + "on_session_end", + **base, + completed=True, + failed=False, + interrupted=False, + turn_exit_reason="text_response(stop)", + ) + lifecycle.finalize_session(session_id="s1") + + [tool_end] = [ + event for event in direct_runtime.events if event[0] == "tool.call_end" + ] + assert tool_end[2]["tool_category"] == "terminal" def test_task_retry_count_survives_provider_fallback_ordinal_reset(direct_runtime): base = { "session_id": "s1", diff --git a/tests/hermes_state/test_session_read_state.py b/tests/hermes_state/test_session_read_state.py new file mode 100644 index 0000000000000..94f229584cf44 --- /dev/null +++ b/tests/hermes_state/test_session_read_state.py @@ -0,0 +1,105 @@ +import time + +import pytest + +from hermes_state import SessionDB + + +@pytest.fixture +def db(tmp_path): + database = SessionDB(tmp_path / "state.db") + try: + yield database + finally: + database.close() + + +def _last_read(db, sid): + row = db._conn.execute( + "SELECT last_read_at FROM sessions WHERE id = ?", (sid,) + ).fetchone() + return row["last_read_at"] if row is not None else None + + +def _row(db, sid): + rows = db.list_sessions_rich(include_archived=True) + return next(s for s in rows if s["id"] == sid) + + +def test_untracked_sessions_are_read(db): + """NULL watermark = never tracked = read, so shipping the column doesn't + badge a user's entire pre-feature history at once.""" + db.create_session(session_id="s1", source="cli") + db.append_message(session_id="s1", role="user", content="hi") + + assert _last_read(db, "s1") is None + assert _row(db, "s1")["unread"] is False + + +def test_mark_read_then_new_activity_flips_back_to_unread(db): + db.create_session(session_id="s1", source="cli") + db.append_message(session_id="s1", role="user", content="hi") + + assert db.set_session_read("s1") is True + assert _row(db, "s1")["unread"] is False + + # New activity postdating the watermark makes it unread again without + # any write on the message path. + time.sleep(0.01) + db.append_message(session_id="s1", role="assistant", content="reply") + assert _row(db, "s1")["unread"] is True + + +def test_mark_unread_explicitly(db): + db.create_session(session_id="s1", source="cli") + db.append_message(session_id="s1", role="user", content="hi") + db.set_session_read("s1") + + assert db.set_session_read("s1", read=False) is True + assert _last_read(db, "s1") == 0.0 + assert _row(db, "s1")["unread"] is True + + +def test_missing_session_returns_false(db): + assert db.set_session_read("nope") is False + + +def _compression_pair(db: SessionDB): + base = time.time() - 100 + db.create_session("root", source="cli") + db.create_session("tip", source="cli", parent_session_id="root") + db._conn.execute( + "UPDATE sessions SET started_at = ?, ended_at = ?, end_reason = 'compression', message_count = 1 WHERE id = 'root'", + (base, base + 10), + ) + db._conn.execute( + "UPDATE sessions SET started_at = ?, message_count = 1 WHERE id = 'tip'", + (base + 20,), + ) + db._conn.commit() + + +def test_reading_compression_tip_stamps_whole_lineage(db): + _compression_pair(db) + + assert db.set_session_read("tip") is True + + root_read = _last_read(db, "root") + assert root_read is not None and root_read > 0 + assert root_read == _last_read(db, "tip") + + # The projected conversation row (root surfaced as tip) derives read. + rows = db.list_sessions_rich(order_by_last_active=True) + assert [s["id"] for s in rows] == ["tip"] + assert rows[0]["unread"] is False + + +def test_marking_root_unread_marks_projected_conversation(db): + _compression_pair(db) + db.set_session_read("tip") + + assert db.set_session_read("root", read=False) is True + + rows = db.list_sessions_rich(order_by_last_active=True) + assert [s["id"] for s in rows] == ["tip"] + assert rows[0]["unread"] is True diff --git a/tests/run_agent/test_run_agent.py b/tests/run_agent/test_run_agent.py index e26728214dc10..721dafbeae4f4 100644 --- a/tests/run_agent/test_run_agent.py +++ b/tests/run_agent/test_run_agent.py @@ -1580,11 +1580,18 @@ def _capture_hook(hook_name, **kwargs): assert post_calls[0]["error_type"] == "keyboard_interrupt" assert json.loads(post_calls[0]["result"])["status"] == "cancelled" - def test_interrupt_skips_remaining(self, agent): + def test_interrupt_skips_remaining(self, agent, monkeypatch): tc1 = _mock_tool_call(name="web_search", arguments="{}", call_id="c1") tc2 = _mock_tool_call(name="web_search", arguments="{}", call_id="c2") mock_msg = _mock_assistant_msg(content="", tool_calls=[tc1, tc2]) messages = [] + hook_calls = [] + + monkeypatch.setattr("hermes_cli.lifecycle.has_hook", lambda name: True) + monkeypatch.setattr( + "hermes_cli.lifecycle.invoke_hook", + lambda hook_name, **kwargs: hook_calls.append((hook_name, kwargs)) or [], + ) with patch("run_agent._set_interrupt"): agent.interrupt() @@ -1596,13 +1603,22 @@ def test_interrupt_skips_remaining(self, agent): "cancelled" in messages[0]["content"].lower() or "interrupted" in messages[0]["content"].lower() ) + post_calls = [kwargs for name, kwargs in hook_calls if name == "post_tool_call"] + assert [call["tool_call_id"] for call in post_calls] == ["c1", "c2"] + assert all(call["status"] == "cancelled" for call in post_calls) - def test_invalid_json_args_are_rejected_without_dispatch(self, agent): + def test_invalid_json_args_are_rejected_without_dispatch(self, agent, monkeypatch): tc = _mock_tool_call( name="web_search", arguments="not valid json", call_id="c1" ) mock_msg = _mock_assistant_msg(content="", tool_calls=[tc]) messages = [] + hook_calls = [] + monkeypatch.setattr("hermes_cli.lifecycle.has_hook", lambda name: True) + monkeypatch.setattr( + "hermes_cli.lifecycle.invoke_hook", + lambda hook_name, **kwargs: hook_calls.append((hook_name, kwargs)) or [], + ) with patch("run_agent.handle_function_call", return_value="ok") as mock_hfc: agent._execute_tool_calls(mock_msg, messages, "task-1") mock_hfc.assert_not_called() @@ -1611,6 +1627,34 @@ def test_invalid_json_args_are_rejected_without_dispatch(self, agent): assert messages[0]["tool_call_id"] == "c1" assert "valid json object" in messages[0]["content"].lower() assert "tool was not executed" in messages[0]["content"].lower() + [post_call] = [ + kwargs for name, kwargs in hook_calls if name == "post_tool_call" + ] + assert post_call["tool_call_id"] == "c1" + assert post_call["status"] == "error" + assert post_call["error_type"] == "invalid_tool_arguments" + + def test_concurrent_invalid_json_args_emit_terminal_hook(self, agent, monkeypatch): + tc = _mock_tool_call( + name="web_search", arguments="not valid json", call_id="c1" + ) + mock_msg = _mock_assistant_msg(content="", tool_calls=[tc]) + messages = [] + hook_calls = [] + monkeypatch.setattr("hermes_cli.lifecycle.has_hook", lambda name: True) + monkeypatch.setattr( + "hermes_cli.lifecycle.invoke_hook", + lambda hook_name, **kwargs: hook_calls.append((hook_name, kwargs)) or [], + ) + + agent._execute_tool_calls_concurrent(mock_msg, messages, "task-1") + + [post_call] = [ + kwargs for name, kwargs in hook_calls if name == "post_tool_call" + ] + assert post_call["tool_call_id"] == "c1" + assert post_call["status"] == "error" + assert post_call["error_type"] == "invalid_tool_arguments" def test_none_args_rejected_without_dispatch(self, agent): """None arguments must not crash the dispatch path. Current contract: @@ -1884,7 +1928,6 @@ def shutdown(self, *args, **kwargs): - def test_invoke_tool_dispatches_to_handle_function_call(self, agent): """_invoke_tool should route regular tools through handle_function_call.""" with patch("run_agent.handle_function_call", return_value="result") as mock_hfc: @@ -2036,6 +2079,54 @@ def test_sequential_blocked_tool_skips_checkpoints_and_callbacks(self, agent, mo + @pytest.mark.parametrize("concurrent", [False, True]) + def test_tool_execution_middleware_replacement_emits_one_terminal_hook( + self, + agent, + monkeypatch, + concurrent, + ): + """A middleware replacement owns the result but not lifecycle closure.""" + tool_call = _mock_tool_call( + name="terminal", + arguments='{"command":"must-not-run"}', + call_id="terminal-1", + ) + mock_msg = _mock_assistant_msg(content="", tool_calls=[tool_call]) + messages = [] + hook_calls = [] + + def execution_middleware(**kwargs): + return '{"intercepted":true}' + + manager = SimpleNamespace(_middleware={ + "tool_request": [], + "tool_execution": [execution_middleware], + }) + monkeypatch.setattr("hermes_cli.plugins.get_plugin_manager", lambda: manager) + monkeypatch.setattr( + "hermes_cli.lifecycle.invoke_hook", + lambda hook_name, **kwargs: hook_calls.append((hook_name, kwargs)) or [], + ) + monkeypatch.setattr("hermes_cli.lifecycle.has_hook", lambda name: True) + + with patch( + "run_agent.handle_function_call", + side_effect=AssertionError("middleware replacement must not dispatch"), + ): + if concurrent: + agent._execute_tool_calls_concurrent(mock_msg, messages, "task-1") + else: + agent._execute_tool_calls_sequential(mock_msg, messages, "task-1") + + post_calls = [ + payload for name, payload in hook_calls if name == "post_tool_call" + ] + assert len(post_calls) == 1 + assert post_calls[0]["tool_name"] == "terminal" + assert post_calls[0]["tool_call_id"] == "terminal-1" + assert post_calls[0]["status"] == "ok" + assert post_calls[0]["result"] == '{"intercepted":true}' def test_agent_runtime_post_hook_ownership_predicate_covers_agent_tools(self, agent): """Sequential and concurrent agent-level paths share post-hook ownership.""" diff --git a/tests/test_model_tools.py b/tests/test_model_tools.py index 5110f42162993..a967f615759a5 100644 --- a/tests/test_model_tools.py +++ b/tests/test_model_tools.py @@ -60,6 +60,42 @@ def test_post_tool_call_receives_non_negative_integer_duration_ms(self): # pre_tool_call does NOT get duration_ms (nothing has run yet). assert "duration_ms" not in kwargs_by_hook["pre_tool_call"] + def test_terminal_nonzero_exit_is_reported_as_error(self): + result = json.dumps({"output": "", "exit_code": 1, "error": None}) + with ( + patch("model_tools.registry.dispatch", return_value=result), + patch("hermes_cli.plugins.has_hook", return_value=True), + patch("hermes_cli.plugins.invoke_hook") as mock_invoke_hook, + ): + assert handle_function_call("terminal", {"command": "false"}) == result + + kwargs_by_hook = { + hook.args[0]: hook.kwargs for hook in mock_invoke_hook.call_args_list + } + for hook_name in ("post_tool_call", "transform_tool_result"): + assert kwargs_by_hook[hook_name]["status"] == "error" + assert kwargs_by_hook[hook_name]["error_type"] == "tool_error" + assert kwargs_by_hook[hook_name]["error_message"] == "exit 1" + + def test_no_listener_skips_post_and_transform_emit(self): + """When no plugin is registered for post_tool_call / + transform_tool_result, the emit path must short-circuit on + ``has_hook`` and never build/dispatch a payload — so the + no-listener hot path stays cheap. ``pre_tool_call`` is always + polled (block-check), so it may still fire; the observer/transform + emits must not. + """ + with ( + patch("model_tools.registry.dispatch", return_value='{"ok":true}'), + patch("hermes_cli.plugins.has_hook", return_value=False), + patch("hermes_cli.plugins.invoke_hook") as mock_invoke_hook, + ): + result = handle_function_call("web_search", {"q": "test"}, task_id="t1") + + assert result == '{"ok":true}' + fired = {c.args[0] for c in mock_invoke_hook.call_args_list} + assert "post_tool_call" not in fired + assert "transform_tool_result" not in fired def test_tool_request_and_execution_middleware_wrap_registry_dispatch(self, monkeypatch): seen = {} @@ -115,6 +151,75 @@ def fake_dispatch(tool_name, args, **kwargs): assert pre_call[1]["middleware_trace"] == expected_trace assert post_call[1]["middleware_trace"] == expected_trace + def test_registry_exception_emits_terminal_tool_hook(self, monkeypatch): + from hermes_cli import lifecycle + + hook_calls = [] + monkeypatch.setattr("hermes_cli.plugins.invoke_hook", lambda *_args, **_kwargs: []) + monkeypatch.setattr(lifecycle, "has_hook", lambda name: name == "post_tool_call") + monkeypatch.setattr( + lifecycle, + "invoke_hook", + lambda name, **kwargs: hook_calls.append((name, kwargs)) or [], + ) + monkeypatch.setattr( + "model_tools.registry.dispatch", + lambda *_args, **_kwargs: (_ for _ in ()).throw(RuntimeError("boom")), + ) + + result = json.loads( + handle_function_call( + "web_search", + {"q": "test"}, + task_id="task-1", + session_id="session-1", + tool_call_id="tool-1", + ) + ) + + assert "error" in result + [post_call] = [call for call in hook_calls if call[0] == "post_tool_call"] + assert post_call[1]["status"] == "error" + assert post_call[1]["error_type"] == "RuntimeError" + assert post_call[1]["duration_ms"] >= 0 + + def test_acp_edit_denial_emits_blocked_terminal_tool_hook(self, monkeypatch): + from hermes_cli import lifecycle + + hook_calls = [] + monkeypatch.setattr("hermes_cli.plugins.invoke_hook", lambda *_args, **_kwargs: []) + monkeypatch.setattr(lifecycle, "has_hook", lambda name: name == "post_tool_call") + monkeypatch.setattr( + lifecycle, + "invoke_hook", + lambda name, **kwargs: hook_calls.append((name, kwargs)) or [], + ) + monkeypatch.setattr( + "acp_adapter.edit_approval.maybe_require_edit_approval", + lambda *_args, **_kwargs: json.dumps({"error": "Edit approval denied"}), + ) + monkeypatch.setattr( + "model_tools.registry.dispatch", + lambda *_args, **_kwargs: (_ for _ in ()).throw( + AssertionError("denied edit must not dispatch") + ), + ) + + result = json.loads( + handle_function_call( + "write_file", + {"path": "private.txt", "content": "private"}, + task_id="task-1", + session_id="session-1", + tool_call_id="tool-1", + ) + ) + + assert result == {"error": "Edit approval denied"} + [post_call] = [call for call in hook_calls if call[0] == "post_tool_call"] + assert post_call[1]["status"] == "blocked" + assert post_call[1]["error_type"] == "edit_approval_denied" + # ========================================================================= # Agent loop tools diff --git a/tests/tools/test_approval.py b/tests/tools/test_approval.py index 9c4b0cbc681ba..8892a089fc419 100644 --- a/tests/tools/test_approval.py +++ b/tests/tools/test_approval.py @@ -1245,6 +1245,71 @@ def _check(): assert "NOT consented" in r["message"] assert "rephrase" in r["message"].lower() + def test_timeout_emits_post_hook_with_timeout_outcome(self, monkeypatch): + """Plugins must be able to distinguish timeout from explicit deny. + + This is what an audit / notification plugin needs to alert + operators on 'agent asked, user never replied' incidents like #24912. + """ + from tools import approval as mod + self._force_short_timeout(monkeypatch, seconds=1) + mod.register_gateway_notify(self.SESSION_KEY, lambda data: None) + + hook_calls = [] + original_fire = mod._fire_approval_hook + + def _capture(event_name, **kwargs): + hook_calls.append((event_name, kwargs)) + return original_fire(event_name, **kwargs) + + monkeypatch.setattr(mod, "_fire_approval_hook", _capture) + + mod.check_all_command_guards("rm -rf .git", "local") + + # post_approval_response must be in the hook log with choice=timeout + posts = [c for c in hook_calls if c[0] == "post_approval_response"] + assert posts, "post_approval_response hook did not fire" + last_post = posts[-1][1] + assert last_post.get("choice") == "timeout", ( + f"hook choice should be 'timeout' on no-response, got {last_post.get('choice')!r}" + ) + + def test_notify_failure_emits_post_hook_and_cleans_up(self, monkeypatch): + """A failed notification still terminates the approval lifecycle.""" + from tools import approval as mod + + hook_calls = [] + + def _capture(event_name, **kwargs): + hook_calls.append((event_name, kwargs)) + + monkeypatch.setattr(mod, "_fire_approval_hook", _capture) + + def _fail_notify(_data): + raise RuntimeError("private gateway failure") + + decision = mod._await_gateway_decision( + self.SESSION_KEY, + _fail_notify, + { + "command": "redacted-command", + "description": "redacted-description", + "pattern_key": "dangerous", + "pattern_keys": ["dangerous"], + }, + ) + + assert decision == { + "resolved": False, + "choice": None, + "notify_failed": True, + } + assert self.SESSION_KEY not in mod._gateway_queues + assert [name for name, _ in hook_calls] == [ + "pre_approval_request", + "post_approval_response", + ] + assert hook_calls[-1][1]["choice"] == "notify_failed" class TestTirithImportErrorFailOpenPolicy: """Regression guard for #20733. diff --git a/tests/tools/test_request_tool_approval.py b/tests/tools/test_request_tool_approval.py index da6aaa27b97ed..5f2c8ef401630 100644 --- a/tests/tools/test_request_tool_approval.py +++ b/tests/tools/test_request_tool_approval.py @@ -50,13 +50,35 @@ def test_cli_approve_once(self, monkeypatch): assert res["approved"] is True def test_cli_deny_blocks(self, monkeypatch): + from hermes_cli import lifecycle + monkeypatch.setattr(approval, "_is_interactive_cli", lambda: True) monkeypatch.setattr(approval, "_is_gateway_approval_context", lambda: False) monkeypatch.setattr(approval, "prompt_dangerous_approval", lambda *a, **k: "deny") - res = request_tool_approval("terminal", "curl PUT to external API") + events = [] + monkeypatch.setattr( + lifecycle, + "invoke_hook", + lambda hook_name, **kwargs: events.append((hook_name, kwargs)) or [], + ) + tokens = approval.set_current_observability_context( + turn_id="turn-1", + tool_call_id="call-1", + ) + try: + res = request_tool_approval("terminal", "curl PUT to external API") + finally: + approval.reset_current_observability_context(tokens) assert res["approved"] is False assert "denied" in res["message"].lower() assert res["pattern_key"].startswith("plugin_rule:") + assert [name for name, _ in events] == [ + "pre_approval_request", + "post_approval_response", + ] + assert all(event["turn_id"] == "turn-1" for _, event in events) + assert all(event["tool_call_id"] == "call-1" for _, event in events) + assert events[-1][1]["choice"] == "deny" def test_cli_session_persists_session_only(self, monkeypatch): monkeypatch.setattr(approval, "_is_interactive_cli", lambda: True) diff --git a/tests/tools/test_tool_search.py b/tests/tools/test_tool_search.py index 575fcffb21afd..d4d66f283c13a 100644 --- a/tests/tools/test_tool_search.py +++ b/tests/tools/test_tool_search.py @@ -297,6 +297,48 @@ def test_tool_search_dispatch_through_handle_function_call(self): # dispatch path completed without error. assert "matches" in parsed or "error" in parsed + def test_tool_search_emits_one_terminal_hook(self, monkeypatch): + """Inline bridge results still complete the tool lifecycle.""" + import model_tools + from hermes_cli import lifecycle + from tools import tool_search + + events = [] + monkeypatch.setattr( + lifecycle, + "has_hook", + lambda name: name == "post_tool_call", + ) + monkeypatch.setattr( + lifecycle, + "invoke_hook", + lambda name, **kwargs: events.append((name, kwargs)), + ) + monkeypatch.setattr( + tool_search, + "dispatch_tool_search", + lambda *args, **kwargs: json.dumps({"matches": []}), + ) + + result = model_tools.handle_function_call( + function_name="tool_search", + function_args={"query": "private-query"}, + session_id="private-session", + task_id="private-task", + turn_id="private-turn", + api_request_id="private-request", + tool_call_id="private-call", + ) + + assert json.loads(result) == {"matches": []} + assert len(events) == 1 + hook_name, payload = events[0] + assert hook_name == "post_tool_call" + assert payload["status"] == "ok" + assert payload["turn_id"] == "private-turn" + assert payload["api_request_id"] == "private-request" + assert payload["tool_call_id"] == "private-call" + class TestRegression_OpenClawCron84141: """Regression guard for the OpenClaw cron-tool-loss class of bug. diff --git a/tools/approval.py b/tools/approval.py index 151e534065539..f5e7f0beb1546 100644 --- a/tools/approval.py +++ b/tools/approval.py @@ -3170,8 +3170,27 @@ def _run_approval_gate( ), } + _fire_approval_hook( + "pre_approval_request", + command=display_target, + description=description, + pattern_key=pattern_key, + pattern_keys=[pattern_key], + session_key=session_key, + surface="cli", + ) choice = prompt_dangerous_approval(display_target, description, approval_callback=approval_callback) + _fire_approval_hook( + "post_approval_response", + command=display_target, + description=description, + pattern_key=pattern_key, + pattern_keys=[pattern_key], + session_key=session_key, + surface="cli", + choice=choice, + ) if choice == "timeout": return { @@ -3464,6 +3483,16 @@ def _drop_entry() -> None: except Exception as exc: logger.warning("Gateway approval notify failed: %s", exc) _drop_entry() + _fire_approval_hook( + "post_approval_response", + command=command, + description=description, + pattern_key=primary_key, + pattern_keys=list(all_keys), + session_key=session_key, + surface=surface, + choice="notify_failed", + ) return {"resolved": False, "choice": None, "notify_failed": True} # Block until the user responds or the canonical approval timeout elapses diff --git a/tui_gateway/methods_session.py b/tui_gateway/methods_session.py index 5cad091c643ed..a4a9284fea57a 100644 --- a/tui_gateway/methods_session.py +++ b/tui_gateway/methods_session.py @@ -748,6 +748,80 @@ def _(rid, params: dict) -> dict: return _ok(rid, info) +@method("session.workspace.move") +def _(rid, params: dict) -> dict: + """Re-home a STORED session's workspace into another folder/project. + + Unlike ``session.cwd.set`` (which acts on a live runtime session by its UI + id), this targets a persisted row by ``session_key`` so the desktop can fix + a session that was created in the wrong directory — no live agent required. + The git branch/root columns are REPLACED (not merely enriched), because the + whole point of the move is to change which project claims the session; a + stale ``git_repo_root`` would keep it grouped under the project it left. + + A live agent bound to the row follows through the runtime path too, so its + terminal/file tools re-anchor immediately; a mid-turn session refuses the + move rather than yanking the workspace out from under its tools. + """ + target = str(params.get("session_key") or "").strip() + if not target: + return _err(rid, 4007, "session_key required") + raw = str(params.get("cwd", "") or "").strip() + if not raw: + return _err(rid, 4016, "cwd required") + from hermes_constants import translate_cwd_for_wsl_backend + + resolved = os.path.abspath(os.path.expanduser(translate_cwd_for_wsl_backend(raw))) + if not os.path.isdir(resolved): + return _err(rid, 4017, f"working directory does not exist: {raw}") + + # Snapshot under the lock — concurrent RPCs mutate _sessions (same pattern + # as _cwd_for_session_key). + live = None + live_sid = "" + with _sessions_lock: + for sid, sess in list(_sessions.items()): + if sess.get("session_key") == target: + live, live_sid = sess, sid + break + if live is not None and live.get("running"): + return _err(rid, 4009, "session busy") + + branch = _git_branch_for_cwd(resolved) + root = _git_common_repo_root_for_cwd(resolved) + with _profile_db(params) as db: + if db is None: + return _db_unavailable_error(rid, code=5007) + # A brand-new draft has no persisted row yet; the live re-home below + # still applies and the row inherits the cwd when it is first written. + row_exists = bool(db.get_session(target)) + if not row_exists and live is None: + return _err(rid, 4007, "session not found") + if row_exists: + try: + db.update_session_cwd( + target, resolved, branch, root, replace_git_meta=True + ) + except Exception as e: + return _err(rid, 5007, f"move failed: {e}") + + if live is not None: + try: + _set_session_cwd(live, resolved) + except ValueError as e: + return _err(rid, 4017, str(e)) + agent = live.get("agent") + info = _session_info(agent, live) if agent is not None else { + "cwd": resolved, + "branch": branch, + "project": _project_info_for_cwd(resolved), + "lazy": True, + } + _emit("session.info", live_sid, info) + + return _ok(rid, {"cwd": resolved, "branch": branch, "git_repo_root": root}) + + @method("session.active_list") def _(rid, params: dict) -> dict: """Return live TUI sessions in this gateway process. diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 37063430e48c7..3405b889b930b 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -280,6 +280,9 @@ def _thread_panic_hook(args): "session.compress", "session.list", "session.resume", + # Workspace re-home runs git branch/root subprocess probes against an + # arbitrary folder — inline they'd stall the reader on a slow mount. + "session.workspace.move", "shell.exec", "skills.manage", "slash.exec",