diff --git a/apps/desktop/electron/entitlements.mac.inherit.plist b/apps/desktop/electron/entitlements.mac.inherit.plist index 53fdf0fc4370..a3defc5f8cfa 100644 --- a/apps/desktop/electron/entitlements.mac.inherit.plist +++ b/apps/desktop/electron/entitlements.mac.inherit.plist @@ -10,5 +10,7 @@ com.apple.security.device.audio-input + com.apple.security.device.camera + diff --git a/apps/desktop/electron/entitlements.mac.plist b/apps/desktop/electron/entitlements.mac.plist index 53fdf0fc4370..a3defc5f8cfa 100644 --- a/apps/desktop/electron/entitlements.mac.plist +++ b/apps/desktop/electron/entitlements.mac.plist @@ -10,5 +10,7 @@ com.apple.security.device.audio-input + com.apple.security.device.camera + diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index b9e966067a38..dc4efb801a5f 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -5510,18 +5510,22 @@ function installContextMenu(window) { }) } -// Microphone capture for the voice composer. The renderer drives mic access +// Microphone and camera capture. The voice composer drives mic access and +// renderer features (e.g. desktop plugins) can drive camera access, both // through getUserMedia, which Chromium gates behind these two session hooks. // // The naive `details.mediaTypes.includes('audio')` check works on macOS but -// breaks on Windows: Chromium frequently fires the mic permission request with -// an empty/undefined `mediaTypes`, so the strict check denies it and -// getUserMedia throws NotAllowedError ("Microphone permission was denied"). -// We therefore treat an audio-capture request as allowed whenever it's the -// 'media'/'audioCapture' permission AND mediaTypes either includes 'audio' OR -// is empty/absent (the Windows case). Video is still denied. -function isAudioCapturePermission(permission, details) { - if (permission === 'audioCapture') { +// breaks on Windows: Chromium frequently fires the request with an empty or +// undefined `mediaTypes`, so a strict check denies it and getUserMedia throws +// NotAllowedError. We therefore allow the capture permissions and treat absent +// metadata as allowed. +// +// Granting here is not the last gate: the OS still applies its own capture +// permission (macOS TCC prompts on first use, per the NSMicrophone/NSCamera +// usage strings), so the user keeps a real allow/deny and can revoke it in +// System Settings afterwards. +function isMediaCapturePermission(permission, details) { + if (permission === 'audioCapture' || permission === 'videoCapture') { return true } @@ -5531,38 +5535,31 @@ function isAudioCapturePermission(permission, details) { const mediaTypes = details?.mediaTypes + // Windows: mediaTypes is often empty for a capture request. Don't deny on + // missing metadata. if (!Array.isArray(mediaTypes) || mediaTypes.length === 0) { - // Windows: mediaTypes is often empty for a mic request. Don't deny on - // missing metadata. (A video request would carry mediaTypes:['video'].) return true } - return mediaTypes.includes('audio') && !mediaTypes.includes('video') + return mediaTypes.includes('audio') || mediaTypes.includes('video') } function installMediaPermissions() { // Async request handler: the prompt-style path (most platforms). session.defaultSession.setPermissionRequestHandler((_webContents, permission, callback, details) => { - callback(isAudioCapturePermission(permission, details)) + callback(isMediaCapturePermission(permission, details)) }) // Synchronous check handler: Chromium consults this for getUserMedia on // Windows in addition to (or instead of) the request handler. Without it, - // the check defaults to false and the mic is denied before the request + // the check defaults to false and capture is denied before the request // handler ever runs. - session.defaultSession.setPermissionCheckHandler((_webContents, permission, _origin, details) => { - if (permission === 'media' || permission === ('audioCapture' as any) /* todo: is this needed? */) { - // details.mediaType is a single string here (not the mediaTypes array). - const mediaType = details?.mediaType - - if (mediaType === 'video') { - return false - } - - return true - } - - return false + session.defaultSession.setPermissionCheckHandler((_webContents, permission) => { + return ( + permission === 'media' || + permission === ('audioCapture' as any) /* todo: is this needed? */ || + permission === ('videoCapture' as any) + ) }) } @@ -8023,6 +8020,17 @@ async function spawnPoolBackend(profile, entry) { entry.token = authToken + // Verify the WebSocket session token before declaring backend ready. + // HTTP /api/status can pass while WS auth fails (separate transport, separate guards). + const wsUrl = `ws://127.0.0.1:${port}/api/ws?token=${encodeURIComponent(authToken)}` + const wsProbe = await probeGatewayWebSocket(wsUrl, { WebSocketImpl: globalThis.WebSocket }) + + if (!wsProbe.ok) { + throw new Error( + `Hermes backend for profile "${profile}" is HTTP-reachable but the WebSocket (/api/ws) rejected the session token: ${wsProbe.reason}` + ) + } + return { baseUrl, mode: 'local', @@ -8030,7 +8038,7 @@ async function spawnPoolBackend(profile, entry) { authMode: 'token', token: authToken, profile, - wsUrl: `ws://127.0.0.1:${port}/api/ws?token=${encodeURIComponent(authToken)}`, + wsUrl, logs: hermesLog.slice(-80), ...getWindowState() } @@ -8348,6 +8356,16 @@ async function startHermes() { rememberLog }) + // Verify the WebSocket session token before declaring backend ready. + const wsUrl = `ws://127.0.0.1:${port}/api/ws?token=${encodeURIComponent(authToken)}` + const wsProbe = await probeGatewayWebSocket(wsUrl, { WebSocketImpl: globalThis.WebSocket }) + + if (!wsProbe.ok) { + throw new Error( + `Local Hermes backend is HTTP-reachable but the WebSocket (/api/ws) rejected the session token: ${wsProbe.reason}` + ) + } + updateBootProgress({ phase: 'backend.ready', message: 'Hermes backend is ready. Finalizing desktop startup', @@ -8362,7 +8380,7 @@ async function startHermes() { source: 'local', authMode: 'token', token: authToken, - wsUrl: `ws://127.0.0.1:${port}/api/ws?token=${encodeURIComponent(authToken)}`, + wsUrl, logs: hermesLog.slice(-80), ...getWindowState() } diff --git a/apps/desktop/package.json b/apps/desktop/package.json index 91e51e6fbf02..01f0e45eb7b0 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -220,6 +220,7 @@ "CFBundleExecutable": "Hermes", "CFBundleName": "Hermes", "NSAudioCaptureUsageDescription": "Hermes uses audio capture for voice conversations.", + "NSCameraUsageDescription": "Hermes uses the camera when a plugin or feature you enable requests it.", "NSMicrophoneUsageDescription": "Hermes uses the microphone for voice input and voice conversations." }, "gatekeeperAssess": false, diff --git a/apps/desktop/src/app/chat/session-tile-actions.ts b/apps/desktop/src/app/chat/session-tile-actions.ts index 469691384598..16b47b995207 100644 --- a/apps/desktop/src/app/chat/session-tile-actions.ts +++ b/apps/desktop/src/app/chat/session-tile-actions.ts @@ -169,7 +169,12 @@ export function useSessionTileActions({ runtimeId, scope, storedSessionId }: Ses } if (attachment.kind === 'image' || attachment.kind === 'file') { - const next = await uploadComposerAttachment(attachment, { remote, requestGateway, sessionId }) + const next = await uploadComposerAttachment(attachment, { + backendCwd: readState()?.cwd, + remote, + requestGateway, + sessionId + }) if (options.updateComposerAttachments ?? true) { scope.attachments.update(next) diff --git a/apps/desktop/src/app/right-sidebar/files/ipc.test.ts b/apps/desktop/src/app/right-sidebar/files/ipc.test.ts index bcaddad55b5b..f4cc61b393ad 100644 --- a/apps/desktop/src/app/right-sidebar/files/ipc.test.ts +++ b/apps/desktop/src/app/right-sidebar/files/ipc.test.ts @@ -82,6 +82,33 @@ describe('readProjectDir', () => { expect(readFileDataUrl).toHaveBeenCalledWith('C:/repo/.gitignore') }) + it('filters gitignored entries when Windows path casing differs across IPC results', async () => { + gitRoot.mockResolvedValue('C:\\Repo') + readDir.mockImplementation(async path => { + if (path === 'c:\\repo\\src') { + return ok([ + { name: 'debug.log', path: 'c:\\repo\\src\\debug.log', isDirectory: false }, + { name: 'keep.ts', path: 'c:\\repo\\src\\keep.ts', isDirectory: false } + ]) + } + + if (path === 'C:/Repo') { + return ok([{ name: '.gitignore', path: 'C:/Repo/.gitignore', isDirectory: false }]) + } + + if (path === 'C:/Repo/src') { + return ok([]) + } + + return ok([]) + }) + readFileDataUrl.mockResolvedValue(dataUrl('src/*.log\n')) + + const result = await readProjectDir('c:\\repo\\src', 'c:\\repo') + + expect(result.entries.map(entry => entry.name)).toEqual(['keep.ts']) + }) + it('does not fetch .gitignore contents when listings do not contain .gitignore', async () => { gitRoot.mockResolvedValue('/repo') readDir.mockImplementation(async path => { diff --git a/apps/desktop/src/app/right-sidebar/files/ipc.ts b/apps/desktop/src/app/right-sidebar/files/ipc.ts index 7f60e4a20a1a..29fc46859255 100644 --- a/apps/desktop/src/app/right-sidebar/files/ipc.ts +++ b/apps/desktop/src/app/right-sidebar/files/ipc.ts @@ -32,16 +32,25 @@ function clean(path: string) { return path.replace(/\\/g, '/').replace(/\/+$/, '') || '/' } +// Windows path identity is case-insensitive. Fold only comparison keys so the +// relative path returned below keeps the filesystem's original spelling; +// POSIX paths remain case-sensitive. +function comparisonPath(path: string) { + return /^[A-Za-z]:(?:\/|$)/.test(path) || path.startsWith('//') ? path.toLowerCase() : path +} + /** Strict POSIX-style relative path; null if `child` is not inside `root`. */ function relativeTo(root: string, child: string) { const r = clean(root) const c = clean(child) + const rKey = comparisonPath(r) + const cKey = comparisonPath(c) - if (c === r) { + if (cKey === rKey) { return '' } - return c.startsWith(`${r}/`) ? c.slice(r.length + 1) : null + return cKey.startsWith(`${rKey}/`) ? c.slice(r.length + 1) : null } /** Repo-root → repo-root/a → repo-root/a/b → … for every dir between root and `dir`. */ diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx index c1660530a035..290e631f4ee4 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx @@ -12,6 +12,7 @@ import { $notifications, clearNotifications } from '@/store/notifications' import { $busy, $connection, + $currentCwd, $currentUsage, $messages, $sessions, @@ -2128,6 +2129,7 @@ describe('usePromptActions file attachment sync', () => { afterEach(() => { cleanup() $connection.set(null) + $currentCwd.set('') vi.restoreAllMocks() }) @@ -2190,6 +2192,100 @@ describe('usePromptActions file attachment sync', () => { }) }) + + it('uploads Windows file bytes when local mode fronts a POSIX WSL/Docker backend', async () => { + $connection.set({ mode: 'local' } as never) + $currentCwd.set('/root') + const readFileDataUrl = vi.fn(async () => 'data:text/plain;base64,aGVsbG8=') + Object.defineProperty(window, 'hermesDesktop', { + configurable: true, + value: { readFileDataUrl } + }) + + const attachment: ComposerAttachment = { + ...fileAttachment(), + path: 'C:\\Users\\alice\\Downloads\\report.txt', + refText: '@file:`C:\\Users\\alice\\Downloads\\report.txt`' + } + const calls: { method: string; params?: Record }[] = [] + + const requestGateway = vi.fn(async (method: string, params?: Record) => { + calls.push({ method, params }) + + if (method === 'file.attach') { + return { + attached: true, + path: '/root/.hermes/desktop-attachments/report.txt', + ref_text: '@file:.hermes/desktop-attachments/report.txt', + uploaded: true + } as never + } + + return {} as never + }) + + let handle: HarnessHandle | null = null + await actRender( + (handle = h)} refreshSessions={async () => undefined} requestGateway={requestGateway} /> + ) + + expect(await handle!.submitText('summarize', { attachments: [attachment] })).toBe(true) + expect(readFileDataUrl).toHaveBeenCalledWith('C:\\Users\\alice\\Downloads\\report.txt') + expect(calls[0]).toEqual({ + method: 'file.attach', + params: { + data_url: 'data:text/plain;base64,aGVsbG8=', + name: 'report.txt', + path: 'C:\\Users\\alice\\Downloads\\report.txt', + session_id: RUNTIME_SESSION_ID + } + }) + expect(calls[1]).toEqual({ + method: 'prompt.submit', + params: { session_id: RUNTIME_SESSION_ID, text: '@file:.hermes/desktop-attachments/report.txt\n\nsummarize' } + }) + }) + + it('uses image.attach_bytes for a Windows image when the local backend cwd is POSIX', async () => { + const readFileDataUrl = vi.fn(async () => 'data:image/jpeg;base64,aGVsbG8=') + Object.defineProperty(window, 'hermesDesktop', { + configurable: true, + value: { readFileDataUrl } + }) + + const requestGateway = vi.fn(async (method: string) => { + if (method === 'image.attach_bytes') { + return { attached: true, path: '/root/tmp/photo.jpg' } as never + } + + return {} as never + }) + + const uploaded = await uploadComposerAttachment( + { + id: 'image:photo.jpg', + kind: 'image', + label: 'photo.jpg', + path: 'C:\\Users\\alice\\Pictures\\photo.jpg' + }, + { + backendCwd: '/root', + remote: false, + requestGateway, + sessionId: RUNTIME_SESSION_ID + } + ) + + expect(readFileDataUrl).toHaveBeenCalledWith('C:\\Users\\alice\\Pictures\\photo.jpg') + expect(requestGateway).toHaveBeenCalledWith('image.attach_bytes', { + content_base64: 'aGVsbG8=', + filename: 'photo.jpg', + session_id: RUNTIME_SESSION_ID + }) + expect(requestGateway).not.toHaveBeenCalledWith('image.attach', expect.anything()) + expect(uploaded.path).toBe('/root/tmp/photo.jpg') + }) + it('passes a path-less @file: ref straight through (no path = nothing to upload)', async () => { // Submit-layer contract: only attachments that carry a `path` are upload // candidates. A path-less ref (an @-mention/context ref or pasted text) @@ -2237,8 +2333,20 @@ describe('usePromptActions file attachment sync', () => { expect(calls[0]?.params?.text).toContain('@file:`/Users/mahmoud/Downloads/DEVIS_signed.pdf`') }) - it('passes the path directly via file.attach in local mode (no byte upload)', async () => { + it('passes a Windows path directly for a native Windows local backend', async () => { $connection.set({ mode: 'local' } as never) + $currentCwd.set('C:\\Users\\alice\\project') + const readFileDataUrl = vi.fn(async () => 'data:text/plain;base64,c2hvdWxkLW5vdC1iZS1yZWFk') + Object.defineProperty(window, 'hermesDesktop', { + configurable: true, + value: { readFileDataUrl } + }) + + const attachment: ComposerAttachment = { + ...fileAttachment(), + path: 'C:\\Users\\alice\\Downloads\\report.txt', + refText: '@file:`C:\\Users\\alice\\Downloads\\report.txt`' + } const calls: { method: string; params?: Record }[] = [] @@ -2257,11 +2365,12 @@ describe('usePromptActions file attachment sync', () => { (handle = h)} refreshSessions={async () => undefined} requestGateway={requestGateway} /> ) - const ok = await handle!.submitText('summarize', { attachments: [fileAttachment()] }) + const ok = await handle!.submitText('summarize', { attachments: [attachment] }) expect(ok).toBe(true) expect(calls[0]?.method).toBe('file.attach') - // Local mode sends no data_url — the gateway shares this disk. + expect(readFileDataUrl).not.toHaveBeenCalled() + // Native Windows local mode shares the same path namespace. expect(calls[0]?.params).not.toHaveProperty('data_url') expect(calls[1]).toEqual({ method: 'prompt.submit', diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts index 681ef8d4199b..6307969421bf 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts @@ -25,6 +25,7 @@ import { clearAllPrompts } from '@/store/prompts' import { $busy, $connection, + $currentCwd, $messages, setAwaitingResponse, setBusy, @@ -76,26 +77,38 @@ interface HandoffResult { error?: string } +const WINDOWS_ABSOLUTE_PATH_RE = /^(?:[A-Za-z]:[\\/]|\\\\)/ +const POSIX_ABSOLUTE_PATH_RE = /^\/(?!\/)/ + +// `mode: local` means the gateway was launched locally, not necessarily that +// Electron and the gateway share a filesystem. Windows Desktop can front a +// WSL/Docker backend whose cwd is POSIX, so a Windows host path must cross the +// boundary as bytes just like a remote attachment. +function attachmentPathNeedsUpload(path: string, backendCwd?: null | string): boolean { + return WINDOWS_ABSOLUTE_PATH_RE.test(path.trim()) && POSIX_ABSOLUTE_PATH_RE.test(backendCwd?.trim() || '') +} + /** * Stage one file/image attachment into the session workspace and return the - * attachment rewritten with the gateway-side ref. Images upload their bytes in - * remote mode (so vision works) and pass the path locally; non-image files - * upload bytes remotely and pass the path locally. Throws on failure so callers - * can surface an error. Shared by submit-time sync, the eager drop-time upload, - * and the message-edit composer drop — keep them in lockstep. + * attachment rewritten with the gateway-side ref. Attachments upload their + * bytes for remote gateways and local cross-filesystem backends; otherwise the + * gateway receives the shared local path. Throws on failure so callers can + * surface an error. Shared by submit-time sync, the eager drop-time upload, and + * the message-edit composer drop — keep them in lockstep. */ export async function uploadComposerAttachment( attachment: ComposerAttachment, - opts: { remote: boolean; requestGateway: GatewayRequest; sessionId: string } + opts: { backendCwd?: null | string; remote: boolean; requestGateway: GatewayRequest; sessionId: string } ): Promise { - const { remote, requestGateway, sessionId } = opts + const { backendCwd, remote, requestGateway, sessionId } = opts const path = attachment.path ?? '' const label = attachment.label || pathLabel(path) + const uploadBytes = remote || attachmentPathNeedsUpload(path, backendCwd) if (attachment.kind === 'image') { let result: ImageAttachResponse - if (remote) { + if (uploadBytes) { let payload: Awaited> try { @@ -138,7 +151,7 @@ export async function uploadComposerAttachment( // Non-image file. let dataUrl: string | null = null - if (remote) { + if (uploadBytes) { try { dataUrl = await readFileDataUrlForAttach(path) } catch (err) { @@ -317,7 +330,12 @@ export function usePromptActions({ } if (attachment.kind === 'image' || attachment.kind === 'file') { - const nextAttachment = await uploadComposerAttachment(attachment, { remote, requestGateway, sessionId }) + const nextAttachment = await uploadComposerAttachment(attachment, { + backendCwd: $currentCwd.get(), + remote, + requestGateway, + sessionId + }) // Update-only: never resurrect a chip the user removed mid-upload. if (updateComposerAttachments) { @@ -356,7 +374,14 @@ export function usePromptActions({ try { // Update-only: if the user removed the chip while this was uploading, // don't resurrect it — just drop the staged result on the floor. - updateComposerAttachment(await uploadComposerAttachment(attachment, { remote, requestGateway, sessionId })) + updateComposerAttachment( + await uploadComposerAttachment(attachment, { + backendCwd: $currentCwd.get(), + remote, + requestGateway, + sessionId + }) + ) } catch (err) { // Leave the chip in place so submit-time sync can retry (or the user can // remove it) and flag the card; also toast so a hard failure (unreadable diff --git a/apps/desktop/src/components/assistant-ui/thread/user-edit-composer.tsx b/apps/desktop/src/components/assistant-ui/thread/user-edit-composer.tsx index 52411d6bd3e9..bf8df4ea01e8 100644 --- a/apps/desktop/src/components/assistant-ui/thread/user-edit-composer.tsx +++ b/apps/desktop/src/components/assistant-ui/thread/user-edit-composer.tsx @@ -415,7 +415,7 @@ export const UserEditComposer: FC = ({ cwd, gateway, sess try { const uploaded = await uploadComposerAttachment( { detail: path, id: attachmentId(kind, path), kind, label: pathLabel(path), path }, - { remote, requestGateway, sessionId } + { backendCwd: cwd, remote, requestGateway, sessionId } ) const ref = attachmentDisplayText(uploaded) diff --git a/hermes_cli/observability/relay_shared_metrics.py b/hermes_cli/observability/relay_shared_metrics.py index fb6f885b2f05..c35be04d697e 100644 --- a/hermes_cli/observability/relay_shared_metrics.py +++ b/hermes_cli/observability/relay_shared_metrics.py @@ -351,7 +351,17 @@ class _Runtime: with session.lock: if session.closing: return - self._finish_task(session, task_id, event) + finished = self._finish_task(session, task_id, event) + if finished: + try: + self.relay.subscribers.flush() + except Exception: + logger.warning( + "Hermes shared-metrics task flush failed", + exc_info=True, + ) + else: + self._export() def close_session(self, event: dict[str, Any]) -> None: session = self._session(event) @@ -380,7 +390,8 @@ class _Runtime: self.relay.subscribers.flush() except Exception as exc: failures.append(f"subscriber flush failed: {exc}") - self._export() + else: + self._export() with self._sessions_lock: if self._sessions.get(session.session_id) is session: self._sessions.pop(session.session_id, None) @@ -399,8 +410,15 @@ class _Runtime: self._safe(self.close_session, {"session_id": session_id}) if not self._registered: return - self._safe(self.relay.subscribers.flush) - self._export() + try: + self.relay.subscribers.flush() + except Exception: + logger.warning( + "Hermes shared-metrics shutdown flush failed", + exc_info=True, + ) + else: + self._export() self._safe(self.relay.subscribers.deregister, self._subscriber_name) self.host.release_managed_execution(self._subscriber_name) self._registered = False @@ -560,10 +578,10 @@ class _Runtime: session: _MetricsSession, task_id: str, event: dict[str, Any], - ) -> None: + ) -> bool: task = session.tasks.get(task_id) if task is None: - return + return False self._end_pending_model_calls(session, {**event, "task_id": task_id}) fields = task_terminal_fields( {**task.start_fields, **event}, @@ -592,9 +610,10 @@ class _Runtime: turn_key = (session.session_id, turn_id) if self._turn_sessions.get(turn_key) is session: self._turn_sessions.pop(turn_key, None) + return True def _export(self) -> None: - self._safe(self.subscriber.store.create_and_export_package) + self._safe(self.subscriber.store.create_and_export_package_if_due) def _event_metadata(self) -> dict[str, str]: return { diff --git a/hermes_cli/observability/shared_metrics.py b/hermes_cli/observability/shared_metrics.py index 506fb4ab1314..666053e0d19d 100644 --- a/hermes_cli/observability/shared_metrics.py +++ b/hermes_cli/observability/shared_metrics.py @@ -114,6 +114,14 @@ class SharedMetricsStore: for _ in range(pending_periods): if self._create_package() is None: break + return self._export_and_prune() + + def create_and_export_package_if_due(self) -> list[Path]: + """Create pending packages at most once per UTC day, then export them.""" + self._create_pending_packages_if_due() + return self._export_and_prune() + + def _export_and_prune(self) -> list[Path]: exported = self._export_pending_packages() try: self._prune_expired_history() @@ -281,6 +289,26 @@ class SharedMetricsStore: ).fetchone() return int(row["period_count"]) if row is not None else 0 + def _create_pending_packages_if_due(self) -> None: + now = _utc_now() + with self._connection() as connection: + with write_txn(connection): + # Gate on the committed package, not its file write, so a failed + # outbox export can be retried without packaging deltas twice. + package_created_today = connection.execute( + """ + SELECT 1 + FROM package_outbox + WHERE substr(created_at, 1, 10) >= ? + LIMIT 1 + """, + (now.date().isoformat(),), + ).fetchone() + if package_created_today is not None: + return + while self._create_package_in_transaction(connection, now) is not None: + pass + def _create_package(self) -> dict[str, Any] | None: now = _utc_now() with self._connection() as connection: diff --git a/tests/hermes_cli/test_relay_shared_metrics.py b/tests/hermes_cli/test_relay_shared_metrics.py index 058e07db7bc7..573835be9845 100644 --- a/tests/hermes_cli/test_relay_shared_metrics.py +++ b/tests/hermes_cli/test_relay_shared_metrics.py @@ -18,6 +18,7 @@ from types import SimpleNamespace from typing import Any import pytest +from hermes_cli.observability import shared_metrics as shared_metrics_module from hermes_cli.observability.shared_metrics import SharedMetricsStore from hermes_cli.observability.shared_metrics_contract import ( COUNT_BUCKETS, @@ -142,6 +143,43 @@ def test_model_call_counter_survives_restart_and_exports_only_new_deltas(tmp_pat assert restarted.counter_snapshot()[0]["packaged_value"] == 3 +def test_due_export_runs_once_per_utc_day_and_catches_up_pending_deltas( + tmp_path, monkeypatch +): + current_time = datetime(2026, 7, 28, 9, tzinfo=timezone.utc) + monkeypatch.setattr(shared_metrics_module, "_utc_now", lambda: current_time) + store = SharedMetricsStore(tmp_path / "metrics.sqlite3", tmp_path / "outbox") + + store.record_model_call(_dimensions(), "test-version") + assert len(store.create_and_export_package_if_due()) == 1 + + current_time = datetime(2026, 7, 28, 18, tzinfo=timezone.utc) + store = SharedMetricsStore(tmp_path / "metrics.sqlite3", tmp_path / "outbox") + store.record_model_call(_dimensions(), "test-version") + assert store.create_and_export_package_if_due() == [] + assert len(list((tmp_path / "outbox").glob("*.json"))) == 1 + assert store.counter_snapshot()[0] == { + "period_start": "2026-07-28", + "metric_name": "hermes.model_call.count", + "hermes_version": "test-version", + "dimensions": _dimensions(), + "value": 2, + "packaged_value": 1, + } + + current_time = datetime(2026, 7, 29, 9, tzinfo=timezone.utc) + store.record_model_call(_dimensions(), "test-version") + assert len(store.create_and_export_package_if_due()) == 2 + assert len(list((tmp_path / "outbox").glob("*.json"))) == 3 + assert all( + row["value"] == row["packaged_value"] for row in store.counter_snapshot() + ) + + store.record_model_call(_dimensions(), "test-version") + assert store.create_and_export_package_if_due() == [] + assert len(list((tmp_path / "outbox").glob("*.json"))) == 3 + + def test_package_schema_matches_the_model_call_contract(): properties = _package_dimension_schema()["properties"] @@ -758,6 +796,32 @@ def test_concurrent_package_builders_commit_one_delta(tmp_path): assert store.counter_snapshot()[0]["packaged_value"] == 1 +def test_concurrent_due_exports_create_one_daily_package(tmp_path): + database_path = tmp_path / "metrics.sqlite3" + outbox_directory = tmp_path / "outbox" + store = SharedMetricsStore(database_path, outbox_directory) + store.record_model_call(_dimensions(), "test-version") + ready = threading.Barrier(8) + + def export() -> None: + worker_store = SharedMetricsStore(database_path, outbox_directory) + ready.wait(timeout=5) + worker_store.create_and_export_package_if_due() + + with ThreadPoolExecutor(max_workers=8) as executor: + futures = [executor.submit(export) for _ in range(8)] + for future in futures: + future.result() + + with sqlite3.connect(database_path) as connection: + [outbox_count] = connection.execute( + "SELECT COUNT(*) FROM package_outbox" + ).fetchone() + assert outbox_count == 1 + assert len(list(outbox_directory.glob("*.json"))) == 1 + assert store.counter_snapshot()[0]["packaged_value"] == 1 + + def test_concurrent_model_call_updates_are_transactional(tmp_path): database_path = tmp_path / "metrics.sqlite3" outbox_directory = 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 ccda2abc1f4c..f1690dc39266 100644 --- a/tests/hermes_cli/test_relay_shared_metrics_runtime.py +++ b/tests/hermes_cli/test_relay_shared_metrics_runtime.py @@ -5,7 +5,9 @@ from __future__ import annotations import contextvars import asyncio import json +import sqlite3 import threading +from datetime import datetime, timedelta, timezone from pathlib import Path from types import SimpleNamespace from typing import Any @@ -321,6 +323,7 @@ def test_direct_runtime_records_without_enabling_a_plugin(direct_runtime, tmp_pa def test_real_binding_drives_lifecycle_aggregation_export_and_snapshot( real_binding_runtime, tmp_path, + monkeypatch, ): assert real_binding_runtime._native is not None prompt_canary = "real-relay-sensitive-prompt" @@ -416,6 +419,12 @@ def test_real_binding_drives_lifecycle_aggregation_export_and_snapshot( root = tmp_path / "hermes-home" / "telemetry" / "shared_metrics" store = SharedMetricsStore(root / "metrics.sqlite3", root / "outbox") + tomorrow = datetime.now(timezone.utc) + timedelta(days=1) + monkeypatch.setattr( + "hermes_cli.observability.shared_metrics._utc_now", + lambda: tomorrow, + ) + assert len(store.create_and_export_package_if_due()) == 1 snapshot = store.counter_snapshot() by_metric: dict[str, list[dict[str, Any]]] = {} for counter in snapshot: @@ -451,7 +460,7 @@ def test_real_binding_drives_lifecycle_aggregation_export_and_snapshot( } package_values: dict[tuple[str, tuple[tuple[str, str], ...]], int] = {} packages = sorted((root / "outbox").glob("*.json")) - assert len(packages) == 3 + assert len(packages) == 2 package_payloads = [ json.loads(package.read_text(encoding="utf-8")) for package in packages ] @@ -2279,32 +2288,133 @@ def test_session_finalize_closes_a_pending_task_as_system_aborted(direct_runtime } -def test_sequential_tasks_in_one_session_aggregate_once_each(direct_runtime, tmp_path): +def test_desktop_task_completion_exports_once_per_utc_day( + direct_runtime, tmp_path, monkeypatch +): + current_time = datetime(2026, 7, 28, 9, tzinfo=timezone.utc) + monkeypatch.setattr( + "hermes_cli.observability.shared_metrics._utc_now", + lambda: current_time, + ) for task_id in ("t1", "t2"): lifecycle.invoke_hook( "pre_llm_call", session_id="s1", task_id=task_id, - platform="cli", + platform="desktop", ) lifecycle.invoke_hook( "on_session_end", session_id="s1", task_id=task_id, - platform="cli", + platform="desktop", completed=True, failed=False, interrupted=False, turn_exit_reason="text_response(stop)", ) - lifecycle.finalize_session(session_id="s1") outbox = tmp_path / "hermes-home" / "telemetry" / "shared_metrics" / "outbox" - [package_path] = list(outbox.glob("*.json")) + [first_package_path] = list(outbox.glob("*.json")) + first_package = json.loads(first_package_path.read_text(encoding="utf-8")) + first_metrics = {metric["name"]: metric for metric in first_package["metrics"]} + assert first_metrics["hermes.task_run.started"]["value"] == 1 + assert first_metrics["hermes.task_run.started"]["dimensions"] == { + "entrypoint": "interactive", + "execution_surface": "desktop", + } + assert first_metrics["hermes.task_run.finished"]["value"] == 1 + + lifecycle.finalize_session(session_id="s1") + assert list(outbox.glob("*.json")) == [first_package_path] + + current_time = datetime(2026, 7, 29, 9, tzinfo=timezone.utc) + lifecycle.invoke_hook( + "pre_llm_call", + session_id="s1", + task_id="t3", + platform="desktop", + ) + lifecycle.invoke_hook( + "on_session_end", + session_id="s1", + task_id="t3", + platform="desktop", + completed=True, + failed=False, + interrupted=False, + turn_exit_reason="text_response(stop)", + ) + + packages = [ + json.loads(package_path.read_text(encoding="utf-8")) + for package_path in outbox.glob("*.json") + ] + totals: dict[str, int] = {} + for package in packages: + for metric in package["metrics"]: + totals[metric["name"]] = totals.get(metric["name"], 0) + metric["value"] + assert totals["hermes.task_run.started"] == 3 + assert totals["hermes.task_run.finished"] == 3 + + +def test_failed_flush_keeps_daily_export_open_for_later_task( + direct_runtime, tmp_path, monkeypatch, caplog +): + current_time = datetime(2026, 7, 28, 9, tzinfo=timezone.utc) + monkeypatch.setattr( + "hermes_cli.observability.shared_metrics._utc_now", + lambda: current_time, + ) + original_flush = direct_runtime.subscribers.flush + flush_attempts = 0 + + def fail_first_flush() -> None: + nonlocal flush_attempts + flush_attempts += 1 + if flush_attempts == 1: + raise RuntimeError("simulated flush failure") + original_flush() + + direct_runtime.subscribers.flush = fail_first_flush + + def finish_desktop_task(task_id: str) -> None: + lifecycle.invoke_hook( + "pre_llm_call", + session_id="s1", + task_id=task_id, + platform="desktop", + ) + lifecycle.invoke_hook( + "on_session_end", + session_id="s1", + task_id=task_id, + platform="desktop", + completed=True, + failed=False, + interrupted=False, + turn_exit_reason="text_response(stop)", + ) + + finish_desktop_task("t1") + + root = tmp_path / "hermes-home" / "telemetry" / "shared_metrics" + assert list((root / "outbox").glob("*.json")) == [] + with sqlite3.connect(root / "metrics.sqlite3") as connection: + [package_count] = connection.execute( + "SELECT COUNT(*) FROM package_outbox" + ).fetchone() + assert package_count == 0 + + finish_desktop_task("t2") + + [package_path] = list((root / "outbox").glob("*.json")) package = json.loads(package_path.read_text(encoding="utf-8")) metrics = {metric["name"]: metric for metric in package["metrics"]} assert metrics["hermes.task_run.started"]["value"] == 2 assert metrics["hermes.task_run.finished"]["value"] == 2 + assert flush_attempts == 2 + assert "Hermes shared-metrics task flush failed" in caplog.text def test_task_ownership_survives_session_id_rotation(direct_runtime): diff --git a/tests/tools/test_tts_xai_speech_tags.py b/tests/tools/test_tts_xai_speech_tags.py index 5559241bff62..a2e407a887a6 100644 --- a/tests/tools/test_tts_xai_speech_tags.py +++ b/tests/tools/test_tts_xai_speech_tags.py @@ -143,7 +143,7 @@ def test_generate_xai_tts_uses_oauth_pinned_base_url(tmp_path, monkeypatch): def raise_for_status(self): pass - def fake_post(url, headers, json, timeout): + def fake_post(url, headers, json, timeout, stream=False): captured["url"] = url captured["headers"] = headers return FakeResponse() @@ -590,7 +590,7 @@ def test_generate_xai_tts_omits_text_normalization_by_default(tmp_path, monkeypa fake_response.content = b"mp3" fake_response.raise_for_status.return_value = None - def fake_post(url, headers, json, timeout): + def fake_post(url, headers, json, timeout, stream=False): captured["json"] = json return fake_response @@ -614,7 +614,7 @@ def test_generate_xai_tts_sends_text_normalization_when_enabled(tmp_path, monkey fake_response.content = b"mp3" fake_response.raise_for_status.return_value = None - def fake_post(url, headers, json, timeout): + def fake_post(url, headers, json, timeout, stream=False): captured["json"] = json return fake_response @@ -640,7 +640,7 @@ def test_generate_xai_tts_omits_text_normalization_when_explicit_false( fake_response.content = b"mp3" fake_response.raise_for_status.return_value = None - def fake_post(url, headers, json, timeout): + def fake_post(url, headers, json, timeout, stream=False): captured["json"] = json return fake_response