diff --git a/agent/auxiliary_client.py b/agent/auxiliary_client.py index d49a797ed8..d1a513c7ec 100644 --- a/agent/auxiliary_client.py +++ b/agent/auxiliary_client.py @@ -6483,6 +6483,17 @@ def resolve_provider_client( custom_key_env = (custom_entry.get("key_env") or custom_entry.get("api_key_env") or "").strip() if not custom_key and custom_key_env: custom_key = _scoped_key_env(custom_key_env) + # Auxiliary tasks resolve named custom providers here rather than + # through _resolve_named_custom_runtime, so key_cmd has to be + # honoured on both paths at matching precedence: otherwise the main + # agent turn works while every auxiliary call (title generation, + # compression, vision, embedding) 401s on the placeholder below. + custom_key_cmd = str(custom_entry.get("key_cmd", "") or "").strip() + if custom_key_cmd: + from agent.command_token_source import build_command_token_provider + custom_key = build_command_token_provider( + custom_key_cmd, custom_entry.get("name") or provider + ) or custom_key custom_key = custom_key or "no-key-required" if custom_key == "no-key-required": logger.warning( diff --git a/agent/command_token_source.py b/agent/command_token_source.py new file mode 100644 index 0000000000..fac8aa64a4 --- /dev/null +++ b/agent/command_token_source.py @@ -0,0 +1,190 @@ +"""Mint a provider API key by running a command (``key_cmd``). + +Static API keys are the exception at enterprise gateways: SSO/OIDC brokers, +cloud IAM, and internal auth proxies all issue SHORT-LIVED bearers instead. +A key copied into ``.env`` (``key_env``) is stale within the hour, so every +request after that 401s and the user has to restart the session. + +``key_cmd`` names a command that PRINTS a token, so the credential is derived +rather than stored:: + + providers: + my-gateway: + base_url: https://gateway.internal.example.com/v1 + api_mode: chat_completions + key_cmd: my-auth-cli print-token --profile prod + +This is the established pattern for agent tooling — Claude Code's +``apiKeyHelper``, the ``gcloud auth print-access-token`` / ``aws ecr +get-login-password`` idiom, and vendor helpers such as ``databricks auth +token`` all expose exactly this contract. Hermes already accepts a callable +API key on both wire clients (the Entra ID / Azure identity path) and invokes +it per request, so nothing downstream changes: the token is simply always +fresh. It is cached until shortly before expiry, so the command runs about +once per token lifetime rather than once per request. + +Output contract: print ONLY the token on stdout, either bare or as JSON with +an ``access_token`` field (``expires_in`` is honoured when present) — the +shape OAuth 2.0 token endpoints and the helpers above already emit. + +Precedence: an explicit ``--api-key`` still wins (the one-off recovery escape +hatch); otherwise ``key_cmd`` is preferred over a static ``api_key`` / +``key_env`` on the same entry. +""" + +from __future__ import annotations + +import json +import logging +import subprocess +import threading +import time +from typing import Callable, Optional + +logger = logging.getLogger(__name__) + +# Treat a cached token as spent slightly before its stated expiry, so a request +# can't be signed with a token that dies in flight. 60s matches the leeway used +# by comparable OAuth token caches. +_TOKEN_REFRESH_LEEWAY_SECONDS = 60.0 +# A token helper reads a local credential cache and should answer in +# milliseconds; anything approaching this budget is hung, not slow. +_MINT_TIMEOUT_SECONDS = 15 +# When a helper advertises NO expiry, the token cannot be cached for the life +# of the process: nothing in the request path re-mints on 401 (the SDK retries +# 429/5xx only), so an expired no-TTL token would 401 every request until +# restart. Re-mint on a bounded window instead — the helper answers from a +# local credential cache in milliseconds, so a periodic re-run is cheap, and a +# helper that wants a longer cache can simply advertise its real expiry. +_NO_TTL_REFRESH_SECONDS = 900.0 + + +class CommandTokenError(RuntimeError): + """A ``key_cmd`` failed to produce a usable token.""" + + +def _mint(command: str, label: str) -> tuple[str, Optional[float]]: + """Run *command*, returning ``(token, ttl_seconds_or_None)``.""" + try: + completed = subprocess.run( + command, + shell=True, + capture_output=True, + text=True, + timeout=_MINT_TIMEOUT_SECONDS, + ) + except subprocess.TimeoutExpired as exc: + raise CommandTokenError( + f"key_cmd for provider {label!r} timed out after " + f"{_MINT_TIMEOUT_SECONDS}s" + ) from exc + except OSError as exc: + raise CommandTokenError( + f"key_cmd for provider {label!r} could not be executed: {exc}" + ) from exc + + if completed.returncode != 0: + # NEVER include stdout/stderr: a partially-successful auth helper can + # print a token or refresh secret there. The command STRING is also + # withheld — a key_cmd can legitimately embed a secret + # (`print-token --client-secret=…`), so echoing it back would leak the + # very credential this module exists to protect. Name the provider so + # the user knows which config entry to run by hand. + raise CommandTokenError( + f"key_cmd for provider {label!r} exited {completed.returncode}. " + f"Run that provider's key_cmd manually to see why " + f"(e.g. `databricks auth login` if its OAuth session expired)." + ) + + stdout = completed.stdout or "" + if not stdout.strip(): + raise CommandTokenError(f"key_cmd for provider {label!r} produced no output") + + # JSON payload — the shape `databricks auth token --output json` prints. + # Token extraction mirrors databricks/ucode's get_databricks_token: + # json.loads(result.stdout or "{}").get("access_token", "") + if stdout.lstrip().startswith("{"): + try: + payload = json.loads(stdout) + except json.JSONDecodeError: + payload = None + if isinstance(payload, dict): + token = str(payload.get("access_token") or "").strip() + if not token: + raise CommandTokenError( + f"key_cmd for provider {label!r} returned JSON without an " + "'access_token' field" + ) + ttl = payload.get("expires_in") + if isinstance(ttl, (int, float)) and ttl > 0: + return token, float(ttl) + # A relative lifetime is the OAuth 2.0 field, but CLI token helpers + # commonly print an absolute ISO 8601 deadline instead. Treating + # that as "no TTL advertised" caches the token for the life of the + # process, so every request 401s once the deadline passes. + # Imported lazily: hermes_cli.auth imports from agent.* at module + # level, so a top-level import here would risk a cycle. + from hermes_cli.auth import _parse_iso_timestamp + + for field in ("expiry", "expiresOn"): + deadline = _parse_iso_timestamp(payload.get(field)) + if deadline is not None: + remaining = deadline - time.time() + if remaining > 0: + return token, remaining + return token, None + + # Bare token. The contract every comparable helper documents is "stdout + # carries the token and nothing else" — extra output would be consumed as + # part of the credential. Strip surrounding whitespace and take the rest + # verbatim; do NOT silently keep one line of several, which converts a + # misconfigured helper (banner, warning, two tokens) into a corrupt-key 401 + # that is far harder to diagnose than an explicit refusal. + token = stdout.strip() + if "\n" in token: + raise CommandTokenError( + f"key_cmd for provider {label!r} printed multiple lines; it must " + "print only the token (or JSON with an 'access_token' field)" + ) + return token, None + + +class CommandTokenSource: + """Callable returning a bearer token, cached until shortly before expiry.""" + + def __init__(self, command: str, label: str = "custom") -> None: + self._command = command + self._label = label or "custom" + self._lock = threading.Lock() + self._token = "" + self._expires_at: float = 0.0 + + def __call__(self) -> str: + with self._lock: + if self._token and time.monotonic() < self._expires_at: + return self._token + token, ttl = _mint(self._command, self._label) + self._token = token + self._expires_at = ( + time.monotonic() + max(ttl - _TOKEN_REFRESH_LEEWAY_SECONDS, 5.0) + if ttl + # No advertised TTL: bounded cache (see _NO_TTL_REFRESH_SECONDS) + # — there is no 401-driven re-mint hook to fall back on. + else time.monotonic() + _NO_TTL_REFRESH_SECONDS + ) + logger.debug( + "Minted key_cmd token for provider %s (ttl=%s)", + self._label, f"{int(ttl)}s" if ttl else "unknown", + ) + return token + + +def build_command_token_provider( + key_cmd: str, + provider_label: str = "custom", +) -> Optional[Callable[[], str]]: + """A per-request token provider for *key_cmd*, or ``None`` when unset.""" + command = str(key_cmd or "").strip() + if not command: + return None + return CommandTokenSource(command, provider_label) diff --git a/agent/secret_sources/bitwarden.py b/agent/secret_sources/bitwarden.py index 357f69cc6f..6419522177 100644 --- a/agent/secret_sources/bitwarden.py +++ b/agent/secret_sources/bitwarden.py @@ -47,10 +47,6 @@ import zipfile from pathlib import Path from typing import Dict, List, Optional, Tuple -from cryptography.hazmat.primitives import hashes -from cryptography.hazmat.primitives.ciphers.aead import AESGCM -from cryptography.hazmat.primitives.kdf.hkdf import HKDF - from agent.secret_sources._cache import ( CachedFetch as _CachedFetch, DiskCache, @@ -375,6 +371,13 @@ def _b64d(text: str) -> bytes: def _derive_encrypted_cache_key(access_token: str, salt: bytes) -> bytes: """Derive the local cache encryption key from the bootstrap BWS token.""" + # Keep the native cryptography extension lazy. Most CLI commands import + # this module while building argparse, even though only encrypted-cache + # reads/writes need it. Eagerly importing it maps ``_rust.pyd`` into a + # Windows updater and prevents uv from replacing that file (#73381). + from cryptography.hazmat.primitives import hashes + from cryptography.hazmat.primitives.kdf.hkdf import HKDF + return HKDF( algorithm=hashes.SHA256(), length=32, @@ -397,6 +400,8 @@ def _write_encrypted_disk_cache( """ path = _encrypted_disk_cache_path(home_path) try: + from cryptography.hazmat.primitives.ciphers.aead import AESGCM + cache_dir = path.parent cache_dir.mkdir(parents=True, exist_ok=True) try: @@ -459,6 +464,8 @@ def _read_encrypted_disk_cache( return None path = _encrypted_disk_cache_path(home_path) try: + from cryptography.hazmat.primitives.ciphers.aead import AESGCM + payload = json.loads(path.read_text(encoding="utf-8")) if not isinstance(payload, dict): return None diff --git a/apps/desktop/electron/connection-registry.test.ts b/apps/desktop/electron/connection-registry.test.ts index 5e41b61187..67ff99c92a 100644 --- a/apps/desktop/electron/connection-registry.test.ts +++ b/apps/desktop/electron/connection-registry.test.ts @@ -14,6 +14,7 @@ import { agentHandle, backendScopeKey, backendScopePrefix, + buildAgentRoster, connectionIdForLabel, labelKey, labelSlug, @@ -26,6 +27,7 @@ import { removeConnection, setPrimaryConnection, uniqueLabel, + updateEligibility, upsertConnection } from './connection-registry' @@ -74,6 +76,39 @@ test('uniqueLabel counts up (never "X 2 2") and clamps long candidates', () => { // --- backendScopeKey (composite pool keys) --- +// The electron and @hermes/shared implementations MUST stay byte-identical — +// the renderer keys its socket registry with the shared copy while the main +// process keys the backend pool with this one. This contract test is the +// enforcement (see the NOTE on backendScopeKey). +test('backendScopeKey: electron and shared implementations agree everywhere', async () => { + // Non-literal specifier on purpose: tsconfig.electron.json's project + // boundary excludes apps/shared sources, but vitest resolves the workspace + // package fine at runtime — which is exactly what this test needs. + const shared = (await import(String('@hermes/shared'))) as { + backendScopeKey: typeof backendScopeKey + backendScopePrefix: typeof backendScopePrefix + LOCAL_CONNECTION_ID: string + } + + const cases: [null | string | undefined, null | string | undefined][] = [ + [null, null], + [undefined, undefined], + ['', ''], + ['local', 'research'], + ['homelab', 'research'], + ['homelab', ''], + [' homelab ', ' research '], + ['spark-2', 'default'] + ] + + for (const [conn, profile] of cases) { + assert.equal(backendScopeKey(conn, profile), shared.backendScopeKey(conn, profile)) + } + + assert.equal(backendScopePrefix('homelab'), shared.backendScopePrefix('homelab')) + assert.equal(LOCAL_CONNECTION_ID, shared.LOCAL_CONNECTION_ID) +}) + test('backendScopeKey: local/empty connection keeps the bare profile key', () => { assert.equal(backendScopeKey(null, 'research'), 'research') assert.equal(backendScopeKey('', 'research'), 'research') @@ -92,6 +127,54 @@ test('backendScopeKey: non-local connections get an unambiguous composite', () = assert.ok(!'research'.startsWith(backendScopePrefix('homelab'))) }) +// --- buildAgentRoster (union roster + @name-device rule) --- + +test('roster: unique profiles keep bare handles; duplicates get @name-device', () => { + const local = { id: 'local', kind: 'local' as const, label: 'This device' } + const homelab = { id: 'homelab', kind: 'remote' as const, label: 'Homelab', url: 'http://h:1' } + + const roster = buildAgentRoster([ + { connection: local, profiles: ['default', 'research'] }, + { connection: homelab, profiles: ['research', 'coder'] } + ]) + + const byKey = new Map(roster.map(a => [`${a.connectionId}/${a.profile}`, a.handle])) + + // research exists on both sources → both disambiguate. + assert.equal(byKey.get('local/research'), 'research-this-device') + assert.equal(byKey.get('homelab/research'), 'research-homelab') + // default and coder are unique → bare names. + assert.equal(byKey.get('local/default'), 'default') + assert.equal(byKey.get('homelab/coder'), 'coder') + assert.equal(roster.length, 4) +}) + +test('roster: unreachable sources contribute no rows and cannot fake duplicates', () => { + const local = { id: 'local', kind: 'local' as const, label: 'This device' } + const dead = { id: 'dead', kind: 'remote' as const, label: 'Dead box', url: 'http://d:1' } + + const roster = buildAgentRoster([ + { connection: local, profiles: ['research'] }, + { connection: dead, profiles: null, error: 'unreachable' } + ]) + + assert.equal(roster.length, 1) + // Only one live source has research → bare handle, no phantom duplicate. + assert.equal(roster[0].handle, 'research') +}) + +// --- updateEligibility --- + +test('update fan-out: cloud is platform-managed, everything else eligible', () => { + assert.deepEqual(updateEligibility({ id: 'c', kind: 'cloud', label: 'Cloud' }), { + eligible: false, + reason: 'cloud-managed' + }) + assert.equal(updateEligibility({ id: 'local', kind: 'local', label: 'x' }).eligible, true) + assert.equal(updateEligibility({ id: 'r', kind: 'remote', label: 'x' }).eligible, true) + assert.equal(updateEligibility({ id: 's', kind: 'ssh', label: 'x' }).eligible, true) +}) + // --- normalizeConnectionInput --- test('save rejects the reserved "local" id on non-local kinds', () => { diff --git a/apps/desktop/electron/connection-registry.ts b/apps/desktop/electron/connection-registry.ts index 9752aa2ca0..ca3c789d63 100644 --- a/apps/desktop/electron/connection-registry.ts +++ b/apps/desktop/electron/connection-registry.ts @@ -140,8 +140,13 @@ export function agentHandle(profile: string, connectionLabel: string, duplicated * log line, and touch call stays byte-identical for single-source users; * non-local connections get an unambiguous composite (`conn:::`) * that cannot collide with a plain profile name (colons are invalid in - * profile names). The single home of the composite-key rule — Electron pool - * keying and any renderer socket registry must both derive keys here. + * profile names). + * + * NOTE: the renderer's socket registry uses the twin implementation in + * apps/shared/src/backend-scope.ts (`@hermes/shared`) — tsconfig project + * boundaries prevent a single physical module here. The two are pinned + * byte-identical by the cross-copy contract test in + * connection-registry.test.ts; change BOTH or that test fails. */ export function backendScopeKey(connectionId: null | string | undefined, profile: null | string | undefined): string { const profileKey = String(profile ?? '').trim() || 'default' @@ -159,6 +164,83 @@ export function backendScopePrefix(connectionId: string): string { return `conn:${String(connectionId).trim()}::` } +// ── Union agent roster ────────────────────────────────────────────────────── + +export interface ConnectionAgents { + connection: RegistryConnection + /** Profile names enumerated from the connection, or null when unreachable / + * connect-on-demand (ssh not yet dialed). */ + profiles: null | string[] + /** Present when profiles is null: why enumeration was skipped. */ + error?: string +} + +export interface RosterAgent { + connectionId: string + connectionKind: ConnectionKind + connectionLabel: string + profile: string + /** Bare profile name, or `-` when the profile name + * exists on more than one registered source (the @name-device rule). */ + handle: string +} + +/** + * Flatten per-connection profile enumerations into the union roster, applying + * the duplicate-handle rule ONCE across all sources. Pure so the disambiguation + * policy is testable without IPC; main.ts feeds it live enumerations. + */ +export function buildAgentRoster(enumerations: ConnectionAgents[]): RosterAgent[] { + const counts = new Map() + + for (const { profiles } of enumerations) { + for (const profile of profiles || []) { + const name = String(profile || '').trim() || 'default' + counts.set(name, (counts.get(name) || 0) + 1) + } + } + + const roster: RosterAgent[] = [] + + for (const { connection, profiles } of enumerations) { + for (const profile of profiles || []) { + const name = String(profile || '').trim() || 'default' + + roster.push({ + connectionId: connection.id, + connectionKind: connection.kind, + connectionLabel: connection.label, + profile: name, + handle: agentHandle(name, connection.label, (counts.get(name) || 0) > 1) + }) + } + } + + return roster +} + +// ── Fan-out update eligibility ────────────────────────────────────────────── + +export interface UpdateEligibility { + eligible: boolean + /** Present when not eligible: 'cloud-managed' (platform updates it). */ + reason?: 'cloud-managed' +} + +/** + * Whether "Update all instances" may drive this connection. Hermes Cloud + * instances are platform-managed — we never run `hermes update` against them. + * Local, remote, and ssh sources are all eligible (reachability and busy + * checks happen at dispatch time, not here). + */ +export function updateEligibility(connection: RegistryConnection): UpdateEligibility { + if (connection.kind === 'cloud') { + return { eligible: false, reason: 'cloud-managed' } + } + + return { eligible: true } +} + /** Mint a registry-unique id from a label (slug, then -2/-3… suffixes). */ export function connectionIdForLabel(label: string, taken: Iterable): string { const used = new Set([...taken]) diff --git a/apps/desktop/electron/gitlock.test.ts b/apps/desktop/electron/gitlock.test.ts new file mode 100644 index 0000000000..342a9cd4b1 --- /dev/null +++ b/apps/desktop/electron/gitlock.test.ts @@ -0,0 +1,71 @@ +import assert from 'node:assert/strict' +import fs from 'node:fs' +import os from 'node:os' +import path from 'node:path' + +import { test } from 'vitest' + +import { clearStaleGitLocks, LOCK_NAMES, STALE_LOCK_MIN_AGE_MS } from './gitlock' + +function makeRepo(): string { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gitlock-test-')) + fs.mkdirSync(path.join(root, '.git')) + + return root +} + +function writeLock(root: string, name: string, ageMs: number): string { + const p = path.join(root, '.git', name) + fs.writeFileSync(p, '') + const t = new Date(Date.now() - ageMs) + fs.utimesSync(p, t, t) + + return p +} + +const noGit = async () => false +const gitRunning = async () => true + +test('stale shallow.lock older than min age is removed', async () => { + const root = makeRepo() + const lock = writeLock(root, 'shallow.lock', STALE_LOCK_MIN_AGE_MS + 60_000) + const removed = await clearStaleGitLocks(root, { isGitRunning: noGit }) + assert.deepEqual(removed, [lock]) + assert.equal(fs.existsSync(lock), false) +}) + +test('fresh lock is presumed live and never removed', async () => { + const root = makeRepo() + const lock = writeLock(root, 'shallow.lock', 1_000) + const removed = await clearStaleGitLocks(root, { isGitRunning: noGit }) + assert.deepEqual(removed, []) + assert.equal(fs.existsSync(lock), true) +}) + +test('running git process protects even ancient locks', async () => { + const root = makeRepo() + const lock = writeLock(root, 'shallow.lock', STALE_LOCK_MIN_AGE_MS * 10) + const removed = await clearStaleGitLocks(root, { isGitRunning: gitRunning }) + assert.deepEqual(removed, []) + assert.equal(fs.existsSync(lock), true) +}) + +test('all known lock names are cleared when stale', async () => { + const root = makeRepo() + const locks = LOCK_NAMES.map(name => writeLock(root, name, STALE_LOCK_MIN_AGE_MS + 60_000)) + const removed = await clearStaleGitLocks(root, { isGitRunning: noGit }) + assert.deepEqual(removed.sort(), locks.sort()) +}) + +test('unknown lock-like files are left alone', async () => { + const root = makeRepo() + const stray = writeLock(root, 'config.lock', STALE_LOCK_MIN_AGE_MS * 10) + await clearStaleGitLocks(root, { isGitRunning: noGit }) + assert.equal(fs.existsSync(stray), true) +}) + +test('missing .git dir is a silent no-op', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gitlock-nogit-')) + const removed = await clearStaleGitLocks(root, { isGitRunning: noGit }) + assert.deepEqual(removed, []) +}) diff --git a/apps/desktop/electron/gitlock.ts b/apps/desktop/electron/gitlock.ts new file mode 100644 index 0000000000..6beab799ea --- /dev/null +++ b/apps/desktop/electron/gitlock.ts @@ -0,0 +1,96 @@ +// Stale git lock-file recovery for the desktop update-check path. +// +// A crashed or killed `git fetch` on a shallow clone can leave +// `.git/shallow.lock` behind. Every later fetch then fails with +// "fatal: Unable to create '.git/shallow.lock': File exists", so the desktop +// update check reports 'fetch-failed' forever — git never self-heals these +// lock files. Mirrors hermes_cli/gitlock.py: a lock is removed only when it +// is older than the min age AND no git process is currently running. + +import { execFile } from 'node:child_process' +import fs from 'node:fs' +import path from 'node:path' + +// Lock files younger than this are presumed live (a fetch is in flight) and +// are never removed. git lock files live for seconds under normal operation; +// anything older than 10 minutes is abandoned. +export const STALE_LOCK_MIN_AGE_MS = 10 * 60 * 1000 + +// Same self-healable lock set as hermes_cli/gitlock.py. +export const LOCK_NAMES = ['shallow.lock', 'index.lock', 'HEAD.lock', 'MERGE_HEAD.lock'] + +function gitProcessRunning(): Promise { + return new Promise(resolve => { + const [cmd, args] = + process.platform === 'win32' + ? ['tasklist', ['/FI', 'IMAGENAME eq git.exe', '/FO', 'CSV']] + : ['pgrep', ['-x', 'git']] + + execFile(cmd, args, { timeout: 10_000 }, (error, stdout) => { + if (process.platform === 'win32') { + // tasklist exits 0 either way; presence is signaled in stdout. + resolve(Boolean(stdout && stdout.toLowerCase().includes('git.exe'))) + + return + } + + // pgrep: exit 0 = at least one match; 1 = none; other = probe failure. + // On probe failure stay conservative: report "running" so no lock is + // touched when we cannot tell. + if (error && (error as any).code === 1) { + resolve(false) + + return + } + + resolve(true) + }) + }) +} + +// Remove abandoned .git lock files under repoRoot. Returns removed paths. +// Never throws: a lock we cannot stat or unlink is skipped. +export async function clearStaleGitLocks( + repoRoot: string, + { + minAgeMs = STALE_LOCK_MIN_AGE_MS, + isGitRunning = gitProcessRunning + }: { + minAgeMs?: number + isGitRunning?: () => Promise + } = {} +): Promise { + const gitDir = path.join(repoRoot, '.git') + const removed: string[] = [] + + try { + if (!fs.statSync(gitDir).isDirectory()) { + return removed + } + } catch { + return removed + } + + if (await isGitRunning()) { + return removed + } + + const cutoff = Date.now() - minAgeMs + + for (const name of LOCK_NAMES) { + const lockPath = path.join(gitDir, name) + + try { + const st = fs.statSync(lockPath) + + if (st.isFile() && st.mtimeMs < cutoff) { + fs.unlinkSync(lockPath) + removed.push(lockPath) + } + } catch { + // Missing or concurrently removed — skipping is always safe. + } + } + + return removed +} diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index e55adacb0c..b7c095ba86 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -85,12 +85,14 @@ import { import { backendScopeKey, backendScopePrefix, + buildAgentRoster, mergeConnectionInput, migrateV1ToRegistry, normalizeConnectionInput, normalizeRegistry, removeConnection, setPrimaryConnection, + updateEligibility, upsertConnection } from './connection-registry' import { describeCrashReason, installCrashForensics } from './crash-forensics' @@ -160,6 +162,7 @@ import { removeWorktree, switchBranch } from './git-worktree-ops' +import { clearStaleGitLocks } from './gitlock' import { readAndConsumeHandoffResult } from './handoff-result' import { ATTACHMENT_UPLOAD_DEFAULT_MAX_BYTES, @@ -2659,6 +2662,12 @@ async function checkUpdates() { } } + // Self-heal abandoned git lock files before fetching. A stale + // .git/shallow.lock from a crashed/interrupted fetch otherwise fails every + // later fetch ("Unable to create '.git/shallow.lock': File exists") and this + // check reports 'fetch-failed' forever — git never removes these itself. + await clearStaleGitLocks(updateRoot) + const fetched = await runGit(['fetch', '--quiet', 'origin', branch], { cwd: updateRoot }) if (fetched.code !== 0) { @@ -11915,6 +11924,151 @@ ipcMain.handle('hermes:connections:test', async (_event, id) => { return { ok: true, baseUrl, version: status?.version || null } }) + +// ── Union agent roster + registry ws-url + fan-out updates (phase 3-5) ───── + +// Enumerate every registered connection's profiles concurrently and flatten +// into the union roster. Eager REST enumeration, lazy sockets: local + already +// -dialed sources answer instantly; unreachable ones return an error entry +// instead of failing the whole roster. ssh sources that have never been dialed +// are SKIPPED (connect-on-demand — dialing every ssh box just to list agents +// would spawn tunnels the user never asked for); once dialed, their pooled +// descriptor serves the enumeration like any remote. +ipcMain.handle('hermes:agents:roster', async () => { + const registry = readDesktopConnectionsRegistry() + + const enumerations = await Promise.all( + registry.connections.map(async connection => { + try { + if ( + connection.kind === 'ssh' && + ![...sshConnections.keys()].some(scope => String(scope).startsWith(backendScopePrefix(connection.id))) + ) { + return { connection, profiles: null, error: 'connect-on-demand' } + } + + const descriptor: any = await ensureRegistryBackend(connection.id, null) + const body: any = await getJsonForBackend(descriptor, '/api/profiles', { timeoutMs: 8_000 }) + + const profiles = Array.isArray(body?.profiles) + ? body.profiles.map(p => String(p?.name || '').trim()).filter(Boolean) + : [] + + // The root HERMES_HOME is an agent too; enumerations that omit it + // (older backends list only named profiles) still get a default row. + if (!profiles.includes('default')) { + profiles.unshift('default') + } + + return { connection, profiles } + } catch (error: any) { + return { connection, profiles: null, error: String(error?.message || error) } + } + }) + ) + + return { + agents: buildAgentRoster(enumerations), + sources: enumerations.map(({ connection, profiles, error }) => ({ + connectionId: connection.id, + label: connection.label, + kind: connection.kind, + reachable: profiles !== null, + ...(error ? { error } : {}) + })) + } +}) + +// Registry-scoped fresh WS URL: the (connectionId, profile) analogue of +// hermes:gateway:ws-url. Same single-use-ticket discipline for OAuth sources. +ipcMain.handle('hermes:gateway:ws-url-for', async (_event, payload) => { + const { connectionId, profile } = payload && typeof payload === 'object' ? (payload as any) : ({} as any) + + return gatewayWsUrlIpcResult(async () => { + const connection: any = await ensureRegistryBackend(connectionId, profile) + + if (connection.authMode === 'oauth') { + const ticket = await mintGatewayWsTicket(connection.baseUrl) + + return buildGatewayWsUrlWithTicket(connection.baseUrl, ticket) + } + + return connection.wsUrl + }) +}) + +// Fan out `hermes update` to every eligible registered connection at once. +// Cloud entries are excluded (platform-managed); each dispatch reports +// independently so one dead LAN box can't wedge the batch. Local reuses the +// app's own update pipeline; remote/ssh POST the backend's own +// /api/hermes/update endpoint (the dashboard updater), which runs +// `hermes update` on THAT machine. +ipcMain.handle('hermes:connections:update-all', async () => { + const registry = readDesktopConnectionsRegistry() + + const results = await Promise.all( + registry.connections.map(async connection => { + const base = { connectionId: connection.id, label: connection.label, kind: connection.kind } + const eligibility = updateEligibility(connection) + + if (!eligibility.eligible) { + return { ...base, ok: false, skipped: true, reason: eligibility.reason } + } + + try { + if (connection.kind === 'local') { + // The app-managed runtime updates through the same pipeline as the + // Settings → Updates button (marker + venv gate + relaunch flow). + const result: any = await applyUpdates({}) + + return { ...base, ok: result?.ok !== false, detail: result?.message || 'update started' } + } + + const descriptor: any = await ensureRegistryBackend(connection.id, null) + + const body: any = await postJsonForBackend(descriptor, '/api/hermes/update', {}, { timeoutMs: 15_000 }) + + if (body?.ok === false) { + // The backend refused (docker/nix/externally-managed installs) — + // surface ITS message, per-row, instead of failing the batch. + return { ...base, ok: false, skipped: true, reason: body?.error || 'backend-refused', detail: body?.message } + } + + return { ...base, ok: true, detail: body?.message || 'update started' } + } catch (error: any) { + return { ...base, ok: false, error: String(error?.message || error) } + } + }) + ) + + return { ok: true, results } +}) + +// POST helper against a resolved backend descriptor. Token-auth descriptors +// use the session-token header; OAuth descriptors have token: null and +// authenticate via the OAuth partition's cookies (same split as the rest of +// the REST surface). +async function postJsonForBackend(descriptor, path, body, opts: any = {}) { + const url = `${descriptor.baseUrl}${path}` + + if (descriptor.authMode === 'oauth') { + return fetchJsonViaOauthSession(url, { ...opts, body: body ?? {}, method: 'POST' }) + } + + return fetchJson(url, descriptor.token, { ...opts, body: body ?? {}, method: 'POST' }) +} + +// GET twin of postJsonForBackend — same token/cookie auth split. +async function getJsonForBackend(descriptor, path, opts: any = {}) { + const url = `${descriptor.baseUrl}${path}` + + if (descriptor.authMode === 'oauth') { + return fetchJsonViaOauthSession(url, opts) + } + + return fetchJson(url, descriptor.token, opts) +} + ipcMain.handle('hermes:connection-config:probe', async (_event, rawUrl) => probeRemoteAuthMode(rawUrl)) ipcMain.handle('hermes:connection-config:oauth-login', async (_event, rawUrl) => { // Capability-gated login (RFC 8252). Probe the gateway's public /api/status: diff --git a/apps/desktop/electron/preload.ts b/apps/desktop/electron/preload.ts index e6ff1ec104..3c9f2ab1eb 100644 --- a/apps/desktop/electron/preload.ts +++ b/apps/desktop/electron/preload.ts @@ -7,6 +7,11 @@ contextBridge.exposeInMainWorld('hermesDesktop', { revalidateConnection: () => ipcRenderer.invoke('hermes:connection:revalidate'), touchBackend: profile => ipcRenderer.invoke('hermes:backend:touch', profile), getGatewayWsUrl: profile => ipcRenderer.invoke('hermes:gateway:ws-url', profile), + // Registry-scoped fresh WS URL: { connectionId, profile } → result shape of + // getGatewayWsUrl, minted against that connection's backend. + getGatewayWsUrlFor: payload => ipcRenderer.invoke('hermes:gateway:ws-url-for', payload), + // Union agent roster across every registered connection. + getAgentRoster: () => ipcRenderer.invoke('hermes:agents:roster'), openSessionWindow: (sessionId, opts) => ipcRenderer.invoke('hermes:window:openSession', sessionId, opts), openSessionInTerminal: (sessionId, opts) => ipcRenderer.invoke('hermes:window:openInTerminal', sessionId, opts), openWindow: () => ipcRenderer.invoke('hermes:window:openInstance'), @@ -131,7 +136,9 @@ contextBridge.exposeInMainWorld('hermesDesktop', { save: payload => ipcRenderer.invoke('hermes:connections:save', payload), remove: id => ipcRenderer.invoke('hermes:connections:remove', id), setPrimary: id => ipcRenderer.invoke('hermes:connections:set-primary', id), - test: id => ipcRenderer.invoke('hermes:connections:test', id) + test: id => ipcRenderer.invoke('hermes:connections:test', id), + // Fan out `hermes update` to every eligible registered connection. + updateAll: () => ipcRenderer.invoke('hermes:connections:update-all') }, sshConfigHosts: () => ipcRenderer.invoke('hermes:ssh-config:hosts'), sshResolveHost: host => ipcRenderer.invoke('hermes:ssh-config:resolve', host), diff --git a/apps/desktop/src/app/contrib/hooks/use-background-sync.ts b/apps/desktop/src/app/contrib/hooks/use-background-sync.ts index 0be9bacb95..2239ea527d 100644 --- a/apps/desktop/src/app/contrib/hooks/use-background-sync.ts +++ b/apps/desktop/src/app/contrib/hooks/use-background-sync.ts @@ -255,6 +255,7 @@ export function rehydrateLiveSessionStatuses( needsInput: false, streamId: null, turnStartedAt: null, + turnLive: false, // The turn ended without its completion events reaching us — a lost // `tool.complete` would otherwise leave a spinning tool row in an // idle session. Seal open tool parts the same way the settle path diff --git a/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event.ts b/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event.ts index 50ce3d7ac3..50291775e6 100644 --- a/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event.ts +++ b/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event.ts @@ -231,8 +231,14 @@ interface GatewayEventDeps { failAssistantMessage: (sessionId: string, errorMessage: string, occurredAt?: number) => void flushQueuedDeltas: (sessionId?: string) => void finalizeInterimAssistantMessage: (sessionId: string, text: string, occurredAt?: number) => void + hydrateFromStoredSession: ( + attempts?: number, + storedSessionId?: string | null, + runtimeSessionId?: string | null + ) => Promise queryClient: QueryClient refreshHermesConfig: () => Promise + scheduleSessionsRefresh: () => void sessionInterrupted: (sessionId: string) => boolean sessionStateByRuntimeIdRef: MutableRefObject> updateSessionState: ( @@ -263,8 +269,10 @@ export function useGatewayEventHandler(deps: GatewayEventDeps) { failAssistantMessage, flushQueuedDeltas, finalizeInterimAssistantMessage, + hydrateFromStoredSession, queryClient, refreshHermesConfig, + scheduleSessionsRefresh, sessionInterrupted, sessionStateByRuntimeIdRef, updateSessionState, @@ -566,7 +574,13 @@ export function useGatewayEventHandler(deps: GatewayEventDeps) { // mutates the per-runtime cache entry, and syncSessionStateToView // guards the view publish to the active session, so this is safe. if (runningChanged && sessionId) { - updateSessionState( + // Set when THIS event released a turn that ended without ever + // producing an assistant payload, so the catch-up side effects below + // run on that edge only. The updater is invoked exactly once, + // synchronously, by updateSessionState. + let recoveredWithoutPayload = false + + const nextState = updateSessionState( sessionId, state => { const busy = Boolean(payload!.running) @@ -585,17 +599,50 @@ export function useGatewayEventHandler(deps: GatewayEventDeps) { return state } + // Prefer the gateway-reported turn_started_at so the timer + // survives session switches and session.info heartbeats. + const gatewayTurnStartedAt = + typeof payload!.turn_started_at === 'number' && payload!.turn_started_at > 0 + ? payload!.turn_started_at * 1000 + : null + return { ...state, busy, - turnStartedAt: state.turnStartedAt ?? Date.now() + // running=true from the backend is turn-live proof, same as + // message.start (e.g. resuming an already-running session + // that never replays its start event). + turnLive: true, + turnStartedAt: state.turnStartedAt ?? gatewayTurnStartedAt ?? Date.now() } } - if (state.awaitingResponse && !state.sawAssistantPayload) { + // The turn has not started backend-side yet. submit arms + // busy/awaitingResponse optimistically, so a running=false + // heartbeat that lands in the gap before the turn spins up is a + // pre-start report, not a finished turn — settling on it would + // drop the spinner and re-open the send guard mid-flight. + // turnLive is stamped only once the backend reports the turn + // live (message.start, the running=true edge, or a resumed + // in-flight turn) and is cleared by every settle, so false + // here is exactly "no turn has been reported running yet". + // (turnStartedAt can't discriminate — it is optimistically + // seeded at submit so the visible timer starts at Enter.) + if (state.awaitingResponse && !state.sawAssistantPayload && !state.turnLive) { return state } + // Past that gate the turn DID start and the backend now reports it + // finished. When no assistant payload ever arrived (gateway crash + // mid-stream, provider error before the first delta, agent-build + // failure) message.complete never fires, so this is the only event + // that can release the session. Bailing here instead left + // awaitingResponse/busy latched until app restart (#46517): the + // per-session busy flag is authoritative for isTargetSessionBusy, + // so submitPrompt and the slash dispatcher silently returned false + // and the session accepted no further input. + recoveredWithoutPayload = state.awaitingResponse && !state.sawAssistantPayload + return { ...state, awaitingResponse: false, @@ -612,11 +659,31 @@ export function useGatewayEventHandler(deps: GatewayEventDeps) { messages: finalizeInterruptedMessages(state.messages, state.streamId, occurredAt), pendingBranchGroup: null, streamId: null, - turnStartedAt: null + turnStartedAt: null, + turnLive: false } }, payload?.stored_session_id || undefined ) + + if (recoveredWithoutPayload) { + // Stays unscoped, like the settle above: a background session's + // sidebar row has to drop its working dot without the user opening + // it. This fires on the recovery edge only — once awaitingResponse + // is false the `state.busy === busy` guard above short-circuits + // every later heartbeat — so it costs one coalesced refresh per + // broken turn, not one per tick. + scheduleSessionsRefresh() + + // The transcript catch-up IS scoped. The stream died, but the turn + // itself may have completed and been persisted, so refetch stored + // history for the session actually on screen; a background session + // reads its history when the user opens it, and hydrating every one + // of them here would fan a REST call out per idle session. + if (isActiveEvent) { + void hydrateFromStoredSession(3, nextState.storedSessionId, sessionId) + } + } } if (payload?.usage && (!explicitSid || isActiveEvent)) { @@ -674,6 +741,16 @@ export function useGatewayEventHandler(deps: GatewayEventDeps) { triggerHaptic('streamStart') } + // Submit→accept latency: seedOptimistic armed the clock at Enter; this + // event is the backend accepting the turn. Debug-only visibility into + // how long the arm actually took (the "no progress box for seconds" + // complaint) — reads the pre-update cache, costs nothing when clean. + const seededAt = sessionStateByRuntimeIdRef.current.get(sessionId)?.turnStartedAt + + if (typeof seededAt === 'number') { + console.debug('[turn-accept-latency]', { sessionId, ms: Date.now() - seededAt }) + } + updateSessionState(sessionId, state => { // If the user clicked Stop (cancelRun set interrupted=true), don't // let a stale message.start from a chained turn (goal follow-up, @@ -693,12 +770,24 @@ export function useGatewayEventHandler(deps: GatewayEventDeps) { sawAssistantPayload: false, interrupted: false, interimBoundaryPending: false, - turnStartedAt: Date.now() + // Backend accepted the turn — the no-payload settle gate below may + // now treat a running=false heartbeat as a real turn end. + turnLive: true, + // Keep the submit-time seed (submit.ts seedOptimistic) — resetting + // here would hide the submit→accept round trip from the timer. + // Backend-originated turns (queue drain elsewhere, goal follow-up) + // have no seed and arm here. + turnStartedAt: state.turnStartedAt ?? Date.now() } }) if (isActiveEvent) { - setTurnStartedAt(Date.now()) + // Belt-and-suspenders mirror of the ACTIVE session's per-session + // clock (the load-bearing mirror is the view-sync flush in + // use-session-state-cache). Mirror the seeded value, not Date.now(): + // resetting to accept-time here would visibly snap the timer back + // after the submit-time seed above already started it. + setTurnStartedAt(sessionStateByRuntimeIdRef.current.get(sessionId)?.turnStartedAt ?? Date.now()) } } else if (event.type === 'message.delta') { if (sessionId) { @@ -1442,10 +1531,12 @@ export function useGatewayEventHandler(deps: GatewayEventDeps) { failAssistantMessage, finalizeInterimAssistantMessage, flushQueuedDeltas, + hydrateFromStoredSession, lastCwdInfoSessionRef, nativeSubagentSessionsRef, queryClient, scheduleConfigRefresh, + scheduleSessionsRefresh, sessionInterrupted, sessionStateByRuntimeIdRef, updateSessionState, diff --git a/apps/desktop/src/app/session/hooks/use-message-stream/index.ts b/apps/desktop/src/app/session/hooks/use-message-stream/index.ts index ccf60aa916..b7eeae2a17 100644 --- a/apps/desktop/src/app/session/hooks/use-message-stream/index.ts +++ b/apps/desktop/src/app/session/hooks/use-message-stream/index.ts @@ -579,7 +579,8 @@ export function useMessageStream({ needsInput: false, pendingBranchGroup: null, streamId: null, - turnStartedAt: null + turnStartedAt: null, + turnLive: false } } @@ -594,6 +595,12 @@ export function useMessageStream({ const keepFailedPartialText = Boolean(failure?.partial && finalText) const interimBoundaryPending = state.interimBoundaryPending + // Wall-clock seconds this turn actually ran (message.start stamped + // turnStartedAt). Read BEFORE the state return below nulls it. + const durationS = state.turnStartedAt + ? Math.max(1, Math.round((Date.now() - state.turnStartedAt) / 1000)) + : undefined + const replaceTextPart = (parts: ChatMessagePart[]) => { const visibleFinalText = stripGeneratedImageEchoes(finalText, generatedImageEchoSources(parts)).trim() @@ -608,7 +615,8 @@ export function useMessageStream({ completedAt: occurredAt, parts: completeOpenTimelineParts(message.parts, occurredAt), pending: false, - interim: false + interim: false, + ...(durationS !== undefined ? { durationS } : {}) } if (completionError && !keepFailedPartialText) { @@ -632,6 +640,7 @@ export function useMessageStream({ timestamp: occurredAt, completedAt: occurredAt, branchGroupId: state.pendingBranchGroup ?? undefined, + ...(durationS !== undefined ? { durationS } : {}), ...(completionError && { error: completionError }) }) @@ -734,7 +743,8 @@ export function useMessageStream({ busy: false, needsInput: false, interimBoundaryPending: false, - turnStartedAt: null + turnStartedAt: null, + turnLive: false } }) @@ -777,6 +787,10 @@ export function useMessageStream({ const prev = state.messages const error = errorMessage.trim() || 'Hermes reported an error' + const durationS = state.turnStartedAt + ? Math.max(1, Math.round((Date.now() - state.turnStartedAt) / 1000)) + : undefined + const nextMessages = prev.some(m => m.id === streamId) ? prev.map(message => message.id === streamId @@ -785,7 +799,8 @@ export function useMessageStream({ completedAt: occurredAt, error, parts: completeOpenTimelineParts(message.parts, occurredAt), - pending: false + pending: false, + ...(durationS !== undefined ? { durationS } : {}) } : message ) @@ -799,7 +814,8 @@ export function useMessageStream({ completedAt: occurredAt, error, pending: false, - branchGroupId: groupId + branchGroupId: groupId, + ...(durationS !== undefined ? { durationS } : {}) } ] @@ -813,7 +829,8 @@ export function useMessageStream({ busy: false, needsInput: false, interimBoundaryPending: false, - turnStartedAt: null + turnStartedAt: null, + turnLive: false } }) }, @@ -832,8 +849,10 @@ export function useMessageStream({ failAssistantMessage, flushQueuedDeltas, finalizeInterimAssistantMessage, + hydrateFromStoredSession, queryClient, refreshHermesConfig, + scheduleSessionsRefresh, sessionInterrupted, sessionStateByRuntimeIdRef, updateSessionState, diff --git a/apps/desktop/src/app/session/hooks/use-message-stream/session-info-side-effects.test.tsx b/apps/desktop/src/app/session/hooks/use-message-stream/session-info-side-effects.test.tsx index 305d31f833..1906efb039 100644 --- a/apps/desktop/src/app/session/hooks/use-message-stream/session-info-side-effects.test.tsx +++ b/apps/desktop/src/app/session/hooks/use-message-stream/session-info-side-effects.test.tsx @@ -3,6 +3,7 @@ import { act, cleanup, render, waitFor } from '@testing-library/react' import { useEffect, useRef } from 'react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { isTargetSessionBusy } from '@/app/session/hooks/use-prompt-actions/utils' import type { ClientSessionState } from '@/app/types' import { createClientSessionState } from '@/lib/chat-runtime' import { modelOptionsQueryKey } from '@/lib/model-options' @@ -21,16 +22,20 @@ const ACTIVE_PROFILE = 'compass' let handleEvent: ((event: RpcEvent) => void) | null = null let refreshHermesConfig: ReturnType Promise>> let refreshSessions: ReturnType Promise>> +let hydrateFromStoredSession: ReturnType Promise>> let queryClient: QueryClient +let sessionStates: Map | null = null function Harness() { const activeSessionIdRef = useRef(ACTIVE_SID) const sessionStateByRuntimeIdRef = useRef(new Map()) + sessionStates = sessionStateByRuntimeIdRef.current + const stream = useMessageStream({ activeGatewayProfile: ACTIVE_PROFILE, activeSessionIdRef, - hydrateFromStoredSession: vi.fn(async () => undefined), + hydrateFromStoredSession, queryClient, refreshHermesConfig, refreshSessions, @@ -61,8 +66,10 @@ const sessionInfo = (sessionId: string, payload: Record) => beforeEach(() => { handleEvent = null + sessionStates = null refreshHermesConfig = vi.fn<() => Promise>(async () => undefined) refreshSessions = vi.fn<() => Promise>(async () => undefined) + hydrateFromStoredSession = vi.fn<() => Promise>(async () => undefined) queryClient = new QueryClient() setCurrentModel('') setCurrentProvider('') @@ -139,6 +146,96 @@ describe('session.info model-options invalidation gating', () => { }) }) +describe('session.info settles a turn that produced no assistant payload', () => { + // #46517: a turn that ends without ever emitting an assistant payload (gateway + // crash mid-stream, provider error before the first delta, agent-build + // failure) never reaches message.complete, so session.info running=false is + // the only event that can release it. It used to return state unchanged, + // latching awaitingResponse/busy until app restart — and because + // isTargetSessionBusy reads the per-session busy flag as authoritative, + // submitPrompt and the slash dispatcher then silently refused every send. + const busyFor = (sessionId: string) => isTargetSessionBusy(Object.fromEntries(sessionStates!), sessionId, false) + + const startTurn = (sessionId: string) => + act(() => handleEvent!({ payload: {}, session_id: sessionId, type: 'message.start' })) + + it('leaves the session sendable after a started turn ends with no payload', async () => { + await mountStream() + + startTurn(ACTIVE_SID) + expect(busyFor(ACTIVE_SID)).toBe(true) + + sessionInfo(ACTIVE_SID, { running: false }) + + const state = sessionStates!.get(ACTIVE_SID)! + expect(state.awaitingResponse).toBe(false) + expect(state.busy).toBe(false) + expect(state.streamId).toBeNull() + expect(state.turnStartedAt).toBeNull() + // The predicate submit.ts and slash.ts actually gate on. + expect(busyFor(ACTIVE_SID)).toBe(false) + }) + + it('keeps waiting when running=false lands before the turn ever started', async () => { + await mountStream() + + // submit arms busy/awaitingResponse optimistically — and seeds the visible + // turn clock (turnStartedAt) at Enter — so this heartbeat is the pre-start + // report, not a finished turn: the spinner must stay up and the send guard + // must stay closed. turnLive (backend-confirmed) is the discriminator. + act(() => { + sessionStates!.set(ACTIVE_SID, { + ...createClientSessionState(), + awaitingResponse: true, + busy: true, + sawAssistantPayload: false, + turnStartedAt: Date.now(), + turnLive: false + }) + }) + + sessionInfo(ACTIVE_SID, { running: false }) + + const state = sessionStates!.get(ACTIVE_SID)! + expect(state.awaitingResponse).toBe(true) + expect(busyFor(ACTIVE_SID)).toBe(true) + expect(hydrateFromStoredSession).not.toHaveBeenCalled() + }) + + it('un-latches a background session but does not hydrate its transcript', async () => { + await mountStream() + + startTurn('session-background') + sessionInfo('session-background', { running: false }) + + // The settle is unscoped — a background session's sidebar dot must clear + // without the user opening it. + expect(busyFor('session-background')).toBe(false) + // The transcript refetch is scoped to the session on screen, so an idle + // background session does not cost a REST fan-out. + expect(hydrateFromStoredSession).not.toHaveBeenCalled() + }) + + it('hydrates the foreground transcript once and coalesces the sidebar refresh', async () => { + await mountStream() + + startTurn(ACTIVE_SID) + vi.useFakeTimers() + + sessionInfo(ACTIVE_SID, { running: false }) + // Later heartbeats hit the unchanged-state guard, so recovery is edge-only. + sessionInfo(ACTIVE_SID, { running: false }) + sessionInfo(ACTIVE_SID, { running: false }) + + await act(async () => { + await vi.advanceTimersByTimeAsync(400) + }) + + expect(hydrateFromStoredSession).toHaveBeenCalledTimes(1) + expect(refreshSessions).toHaveBeenCalledTimes(1) + }) +}) + describe('message.complete sidebar refresh coalescing', () => { it('collapses near-simultaneous completions into one refresh', async () => { await mountStream() 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 12ae168265..1c14665d44 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 @@ -106,6 +106,7 @@ function Harness({ runtimeIdByStoredSessionIdRef: runtimeIdByStoredSessionIdRefProp, seedMessages, seedStreamId, + seedTurnStartedAt, selectedStoredSessionIdRef: selectedStoredSessionIdRefProp, storedSessionId, activeSessionId, @@ -130,6 +131,7 @@ function Harness({ runtimeIdByStoredSessionIdRef?: MutableRefObject> seedMessages?: unknown[] seedStreamId?: null | string + seedTurnStartedAt?: null | number selectedStoredSessionIdRef?: MutableRefObject storedSessionId?: null | string activeSessionId?: null | string @@ -163,6 +165,7 @@ function Harness({ awaitingResponse: false, interrupted: true, streamId: seedStreamId ?? null, + turnStartedAt: seedTurnStartedAt ?? null, interimBoundaryPending: false } as never) @@ -1720,6 +1723,59 @@ describe('usePromptActions submit / queue drain semantics', () => { ) }) + it('arms turnStartedAt at submit time instead of waiting for message.start', async () => { + const seeds: Record[] = [] + const requestGateway = vi.fn(async () => ({}) as never) + + let handle: HarnessHandle | null = null + await actRender( + (handle = h)} + onSeedState={s => seeds.push(s)} + refreshSessions={async () => undefined} + requestGateway={requestGateway} + /> + ) + + const before = Date.now() + await handle!.submitText('arm the clock now') + + // The optimistic seed carries the clock — the progress box's timer must + // not be hostage to the submit→gateway-accept round trip (which can take + // seconds under load). message.start later keeps this value (?? guard). + expect(seeds.length).toBeGreaterThan(0) + const armed = seeds[0].turnStartedAt + + expect(typeof armed).toBe('number') + expect(armed as number).toBeGreaterThanOrEqual(before) + expect(armed as number).toBeLessThanOrEqual(Date.now()) + }) + + it('keeps a live turn clock when a second seed races it (?? guard)', async () => { + const seeds: Record[] = [] + const requestGateway = vi.fn(async () => ({}) as never) + const preArmed = Date.now() - 12_345 + + let handle: HarnessHandle | null = null + await actRender( + (handle = h)} + onSeedState={s => seeds.push(s)} + refreshSessions={async () => undefined} + requestGateway={requestGateway} + seedTurnStartedAt={preArmed} + /> + ) + + // Submit into state that already carries a live clock (queued send racing + // a running turn): the seed must preserve it, not restart the visible + // elapsed time. + await handle!.submitText('send racing a live clock') + + expect(seeds.length).toBeGreaterThan(0) + expect(seeds[0].turnStartedAt).toBe(preArmed) + }) + it('flags prompt.submit with interrupted:true after a voice-playback barge', async () => { const { markVoicePlaybackInterrupted } = await import('@/lib/voice-playback') const requestGateway = vi.fn(async () => ({}) as never) 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 155f788836..333bca63e3 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 @@ -678,7 +678,8 @@ export function usePromptActions({ pendingBranchGroup: null, needsInput: false, interrupted: true, - turnStartedAt: null + turnStartedAt: null, + turnLive: false } }) diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts index bbc11910b8..67f13044c4 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts @@ -371,7 +371,15 @@ export function useSubmitPrompt(deps: SubmitPromptDeps) { // Fresh submit = new turn — clear any leftover interrupt flag, else // mutateStream/completeAssistantMessage drop every delta of this turn // (what made drained-after-interrupt sends go silent). - interrupted: false + interrupted: false, + // Arm the turn clock at send, not at the backend's message.start — + // the round trip (submit RPC → gateway accept → WS event) can take + // seconds under load, and the honest latency clock starts when the + // user hit Enter. message.start keeps this seed (?? Date.now()), + // and the settle paths clear it as before. `??` on our side too: + // a queued send that loses the settle race against a still-live + // turn must not restart that turn's clock. + turnStartedAt: state.turnStartedAt ?? Date.now() }), targetStoredSessionId ) @@ -405,7 +413,11 @@ export function useSubmitPrompt(deps: SubmitPromptDeps) { messages: state.messages.filter(m => m.id !== optimisticId), busy: false, awaitingResponse: false, - pendingBranchGroup: null + pendingBranchGroup: null, + // Retire the submit-time clock seed with the turn it belonged to — + // only when no live stream claimed it (a queued send aborting must + // not wipe a running turn's clock). + turnStartedAt: state.streamId || state.sawAssistantPayload ? state.turnStartedAt : null }), targetStoredSessionId ) @@ -762,7 +774,9 @@ export function useSubmitPrompt(deps: SubmitPromptDeps) { busy: false, awaitingResponse: false, pendingBranchGroup: null, - sawAssistantPayload: true + sawAssistantPayload: true, + // The failed submit's clock seed dies with the turn it never got. + turnStartedAt: null }), targetStoredSessionId ) diff --git a/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx b/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx index 8c408e5a87..ea5cc084ce 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx +++ b/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx @@ -1,7 +1,8 @@ +import { useStore } from '@nanostores/react' import { act, cleanup, render, waitFor } from '@testing-library/react' import type { MutableRefObject } from 'react' -import { useEffect } from 'react' -import { afterEach, describe, expect, it, vi } from 'vitest' +import { useEffect, useRef } from 'react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { $terminalTakeover, setTerminalTakeover } from '@/app/right-sidebar/store' import { noteActiveTreeGroup, revealTreePane } from '@/components/pane-shell/tree/store' @@ -22,8 +23,11 @@ import { $newChatWorkspaceTarget, $resumeFailedSessionId, $selectedStoredSessionId, + $turnStartedAt, setActiveSessionId, setActiveSessionStoredIdRotation, + setAwaitingResponse, + setBusy, setCurrentCwd, setCurrentFastMode, setCurrentModel, @@ -33,14 +37,17 @@ import { setNewChatWorkspaceTarget, setResumeFailedSessionId, setSelectedStoredSessionId, - setSessions + setSessions, + setTurnStartedAt } from '@/store/session' import { $sessionTiles } from '@/store/session-states' +import sessionResumeActiveTurn from '../../../../../../tests/fixtures/session-resume-active-turn.json' import { sessionRoute } from '../../routes' import type { ClientSessionState } from '../../types' import { useSessionActions } from './use-session-actions' +import { useSessionStateCache } from './use-session-state-cache' vi.mock('@/hermes', async importOriginal => ({ ...(await importOriginal>()), @@ -631,8 +638,10 @@ function ResumeHarness({ selectedStoredSessionIdRef: ref(selectedStoredSessionId), sessionStateByRuntimeIdRef: stateMapRef, syncSessionStateToView: vi.fn(), - updateSessionState: (sessionId, updater) => { - const current = stateMapRef.current.get(sessionId) ?? ({} as ClientSessionState) + updateSessionState: (sessionId, updater, storedSessionId) => { + // Full default shape (not a bare {} cast) so seeded/derived fields like + // turnStartedAt behave as in production state updates. + const current = stateMapRef.current.get(sessionId) ?? createClientSessionState(storedSessionId ?? null) const next = updater(current) stateMapRef.current.set(sessionId, next) @@ -649,6 +658,51 @@ function ResumeHarness({ return null } +function ResumeTimerHarness({ + onReady, + requestGateway +}: { + onReady: (resume: (storedSessionId: string, replaceRoute?: boolean) => Promise) => void + requestGateway: (method: string, params?: Record) => Promise +}) { + const activeSessionId = useStore($activeSessionId) + const busyRef = useRef(false) + + const cache = useSessionStateCache({ + activeSessionId, + busyRef, + selectedStoredSessionId: null, + setAwaitingResponse, + setBusy, + setMessages + }) + + const actions = useSessionActions({ + activeSessionId, + activeSessionIdRef: cache.activeSessionIdRef, + busyRef, + creatingSessionRef: useRef(false), + ensureSessionState: cache.ensureSessionState, + getRouteToken: () => 'timer-contract', + navigate: vi.fn() as never, + requestGateway, + resetViewSync: cache.resetViewSync, + runtimeIdByStoredSessionIdRef: cache.runtimeIdByStoredSessionIdRef, + selectedStoredSessionId: null, + selectedStoredSessionIdRef: cache.selectedStoredSessionIdRef, + sessionStateByRuntimeIdRef: cache.sessionStateByRuntimeIdRef, + syncSessionStateToView: cache.syncSessionStateToView, + getRoutedStoredSessionId: () => null, + updateSessionState: cache.updateSessionState + }) + + useEffect(() => { + onReady(actions.resumeSession) + }, [actions.resumeSession, onReady]) + + return null +} + describe('resumeSession failure recovery', () => { afterEach(() => { cleanup() @@ -793,6 +847,7 @@ describe('resumeSession failure recovery', () => { message_count: compressedRuntimeMessages.length, messages: compressedRuntimeMessages, running: true, + turn_started_at: 1_700_000_000, inflight: { user: 'current prompt', assistant: 'partial answer', @@ -823,6 +878,7 @@ describe('resumeSession failure recovery', () => { expect(renderedMessages).toContain('current prompt') expect(renderedMessages).toContain('partial answer') expect(renderedMessages).toContain('newest prompt') + expect(resumedState?.turnStartedAt).toBe(1_700_000_000_000) }) it('preserves a runtime-cache delta that arrives while cold resume waits for REST', async () => { @@ -1072,6 +1128,7 @@ describe('resumeSession failure recovery', () => { storedSessionId: 'stored-1', streamId: null, turnStartedAt: null, + turnLive: false, usage: null, yolo: false } @@ -1107,6 +1164,86 @@ describe('resumeSession failure recovery', () => { }) }) +describe('session.resume turn timer contract', () => { + beforeEach(() => { + vi.spyOn(window, 'requestAnimationFrame').mockImplementation((callback: FrameRequestCallback) => { + callback(0) + + return null as unknown as number + }) + setActiveSessionId(null) + setAwaitingResponse(false) + setBusy(false) + setMessages([]) + setSessions([]) + setTurnStartedAt(null) + }) + + afterEach(() => { + cleanup() + setActiveSessionId(null) + setAwaitingResponse(false) + setBusy(false) + setMessages([]) + setSessions([]) + setTurnStartedAt(null) + vi.restoreAllMocks() + }) + + async function resumeFrom(response: unknown): Promise { + const requestGateway = vi.fn(async (method: string) => { + if (method === 'session.resume') { + // Model the JSON-RPC serialization/deserialization boundary. The shared + // fixture is asserted against the real gateway response in Python. + return JSON.parse(JSON.stringify(response)) as never + } + + return {} as never + }) + + vi.mocked(getAllSessionMessages).mockResolvedValue({ messages: [], session_id: 'stored-running' } as never) + + let resume: ((storedSessionId: string, replaceRoute?: boolean) => Promise) | null = null + render( (resume = ready)} requestGateway={requestGateway} />) + await waitFor(() => expect(resume).not.toBeNull()) + await act(async () => { + await resume!('stored-running', true) + }) + } + + it('restores the canonical gateway turn timestamp in milliseconds', async () => { + await resumeFrom(sessionResumeActiveTurn) + + expect($turnStartedAt.get()).toBe(sessionResumeActiveTurn.turn_started_at * 1000) + }) + + it('clears a stale timer when the gateway response is not running', async () => { + setTurnStartedAt(1_600_000_000_000) + + await resumeFrom({ ...sessionResumeActiveTurn, running: false }) + + expect($turnStartedAt.get()).toBeNull() + }) + + it('clears a stale timer when the running gateway response omits its timestamp', async () => { + const missingTimestamp: Record = JSON.parse(JSON.stringify(sessionResumeActiveTurn)) + delete missingTimestamp.turn_started_at + setTurnStartedAt(1_600_000_000_000) + + await resumeFrom(missingTimestamp) + + expect($turnStartedAt.get()).toBeNull() + }) + + it('clears a stale timer when the running gateway response has a non-numeric timestamp', async () => { + setTurnStartedAt(1_600_000_000_000) + + await resumeFrom({ ...sessionResumeActiveTurn, turn_started_at: 'not-a-timestamp' }) + + expect($turnStartedAt.get()).toBeNull() + }) +}) + function BranchHarness({ activeSessionId = null, navigate = vi.fn(), @@ -1732,6 +1869,67 @@ describe('resumeSession warm-cache mapping integrity', () => { expect(resumedState?.messages[0]?.attachmentRefs).toEqual(['@image:/tmp/photo.png']) }) + it('restores the warm reconnect turn clock from session.activate', async () => { + const turnStartedAtSeconds = 1_700_000_123 + + const runtimeIdByStoredSessionIdRef: MutableRefObject> = { + current: new Map([['stored-A', 'rt-A']]) + } + + const cachedState = clientState('stored-A') + cachedState.busy = true + cachedState.turnStartedAt = null + + const sessionStateByRuntimeIdRef: MutableRefObject> = { + current: new Map([['rt-A', cachedState]]) + } + + const requestGateway = vi.fn(async (method: string) => { + if (method === 'session.activate') { + return { + session_id: 'rt-A', + session_key: 'stored-A', + resumed: 'stored-A', + message_count: 0, + messages: [], + running: true, + turn_started_at: turnStartedAtSeconds, + inflight: { + user: 'current prompt', + assistant: 'partial answer', + streaming: true + }, + info: {} + } as never + } + + return {} as never + }) + + vi.mocked(getAllSessionMessages).mockResolvedValue({ messages: [], session_id: 'stored-A' } as never) + + let resumedState: ClientSessionState | undefined + let resume: ((storedSessionId: string, replaceRoute?: boolean) => Promise) | null = null + render( + (resume = ready)} + onStateUpdate={(_sessionId, state) => (resumedState = state)} + requestGateway={requestGateway} + runtimeIdByStoredSessionIdRef={runtimeIdByStoredSessionIdRef} + sessionStateByRuntimeIdRef={sessionStateByRuntimeIdRef} + /> + ) + await waitFor(() => expect(resume).not.toBeNull()) + await resume!('stored-A', true) + + expect(resumedState).toMatchObject({ + awaitingResponse: true, + busy: true, + turnStartedAt: turnStartedAtSeconds * 1000 + }) + expect(JSON.stringify(resumedState?.messages)).toContain('partial answer') + }) + it('repairs an idle warm cache from a divergent equal-length persisted transcript', async () => { const runtimeIdByStoredSessionIdRef: MutableRefObject> = { current: new Map([['stored-A', 'rt-A']]) diff --git a/apps/desktop/src/app/session/hooks/use-session-actions/index.ts b/apps/desktop/src/app/session/hooks/use-session-actions/index.ts index 66b0a54e70..361a553fd0 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions/index.ts +++ b/apps/desktop/src/app/session/hooks/use-session-actions/index.ts @@ -854,6 +854,11 @@ export function useSessionActions({ Boolean(sessionStateByRuntimeIdRef.current.get(cachedRuntimeId)?.busy) ) + const activatedTurnStartedAt = + typeof activated.turn_started_at === 'number' && activated.turn_started_at > 0 + ? activated.turn_started_at * 1000 + : null + // The persisted REST transcript is the display authority: a live // runtime may carry only the agent's compressed context projection, // which is intentionally smaller than the user-visible conversation. @@ -920,11 +925,15 @@ export function useSessionActions({ messages: activatedMessages, busy: running, awaitingResponse: running, + // Resumed onto an already-running turn — that IS backend + // proof the turn is live (no message.start will replay). + turnLive: state.turnLive || running, needsInput: pendingApproval || pendingClarify || state.needsInput, // Adopting someone else's turn: we'll stream its reply // without ever having received its prompt, so the settle // path must not take the "I saw it all" shortcut. - adoptedRunningTurn: state.adoptedRunningTurn || running + adoptedRunningTurn: state.adoptedRunningTurn || running, + turnStartedAt: running ? (activatedTurnStartedAt ?? state.turnStartedAt ?? Date.now()) : null }), storedSessionId ) @@ -1192,6 +1201,14 @@ export function useSessionActions({ patchSessionWorkspace(storedSessionId, runtimeInfo?.cwd) + // Preserve the turn-elapsed timer across cold resume: the gateway + // reports when the in-flight turn started so the desktop can restore + // the clock instead of resetting it to 0:00. + const resumedTurnStartedAt = + typeof resumed.turn_started_at === 'number' && resumed.turn_started_at > 0 + ? resumed.turn_started_at * 1000 + : null + updateSessionState( resumed.session_id, state => ({ @@ -1200,6 +1217,8 @@ export function useSessionActions({ messages: messagesForView, busy: resumedRunning, awaitingResponse: resumedRunning && !recoveredInFlightTail, + // Backend reported this turn running at resume time — live proof. + turnLive: state.turnLive || resumedRunning, needsInput: pendingApproval || pendingClarify || state.needsInput, adoptedRunningTurn: state.adoptedRunningTurn || resumedRunning, ...(inFlightRecovery.applied @@ -1208,11 +1227,11 @@ export function useSessionActions({ // Point live deltas at the recovered row when the backend is // still mid-turn; a settled recovery keeps the stream idle. streamId: resumedRunning ? inFlightRecovery.streamId : null, - turnStartedAt: resumedRunning - ? (inFlightRecovery.turnStartedAt ?? state.turnStartedAt ?? Date.now()) - : state.turnStartedAt + turnStartedAt: resumedRunning ? (inFlightRecovery.turnStartedAt ?? resumedTurnStartedAt) : null } - : {}) + : { + turnStartedAt: resumedRunning && resumedTurnStartedAt !== null ? resumedTurnStartedAt : null + }) }), storedSessionId ) diff --git a/apps/desktop/src/app/session/hooks/use-session-actions/utils.ts b/apps/desktop/src/app/session/hooks/use-session-actions/utils.ts index 4d46dc5fa5..533f0e9ca9 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions/utils.ts +++ b/apps/desktop/src/app/session/hooks/use-session-actions/utils.ts @@ -149,7 +149,10 @@ const COMPARED_FIELDS = [ 'interim', 'reactions', 'timestamp', - 'completedAt' + 'completedAt', + // Turn wall-clock duration — stamps the visible "⏱ 38s" badge, so a change + // must re-render (set once at completion; stable afterwards). + 'durationS' ] as const const IGNORED_FIELDS = ['attachmentRefs', 'parts', 'rowId'] as const diff --git a/apps/desktop/src/app/settings/connections-settings.tsx b/apps/desktop/src/app/settings/connections-settings.tsx index 9f504b2ef8..619531f367 100644 --- a/apps/desktop/src/app/settings/connections-settings.tsx +++ b/apps/desktop/src/app/settings/connections-settings.tsx @@ -11,7 +11,7 @@ import type { } from '@/global' import { useI18n } from '@/i18n' import { triggerHaptic } from '@/lib/haptics' -import { Cloud, Globe, Loader2, Monitor, Pencil, Plus, Terminal, Trash2 } from '@/lib/icons' +import { Cloud, Globe, Loader2, Monitor, Pencil, Plus, RefreshCw, Terminal, Trash2 } from '@/lib/icons' import { notify, notifyError } from '@/store/notifications' import { EmptyState, ListRow, Pill, SectionHeading, SettingsContent, SettingsSkeleton } from './primitives' @@ -74,6 +74,7 @@ export function ConnectionsSettings() { const [testingId, setTestingId] = useState(null) const [removeTarget, setRemoveTarget] = useState(null) const [plainTextConfirm, setPlainTextConfirm] = useState(false) + const [updatingAll, setUpdatingAll] = useState(false) const bridge = window.hermesDesktop?.connections @@ -227,6 +228,34 @@ export function ConnectionsSettings() { [bridge, s.testFailed, s.testOk] ) + // Fan out `hermes update` to every eligible source; per-connection results + // land as individual toasts so one dead box doesn't hide the others. + const updateAll = useCallback(async () => { + if (!bridge?.updateAll) { + return + } + + setUpdatingAll(true) + + try { + const { results } = await bridge.updateAll() + + for (const row of results) { + if (row.ok) { + notify({ title: row.label, message: row.detail || s.updateAllDone }) + } else if (row.skipped && row.reason === 'cloud-managed') { + notify({ title: row.label, message: s.updateSkippedCloud }) + } else { + notifyError(new Error(row.error || row.detail || row.reason || row.label), s.updateAllFailed) + } + } + } catch (err) { + notifyError(err, s.updateAllFailed) + } finally { + setUpdatingAll(false) + } + }, [bridge, s.updateAllDone, s.updateAllFailed, s.updateSkippedCloud]) + const kindMeta: Record = { cloud: { desc: s.kindCloudDesc, label: s.kindCloud }, local: { desc: s.kindLocalDesc, label: s.kindLocal }, @@ -425,7 +454,7 @@ export function ConnectionsSettings() { ) : ( -
+
+ {bridge?.updateAll && (registry?.connections.length ?? 0) > 1 && ( + + )}
)} diff --git a/apps/desktop/src/app/settings/toolset-config-panel.tsx b/apps/desktop/src/app/settings/toolset-config-panel.tsx index 40571042eb..c3ca93cc51 100644 --- a/apps/desktop/src/app/settings/toolset-config-panel.tsx +++ b/apps/desktop/src/app/settings/toolset-config-panel.tsx @@ -50,6 +50,22 @@ interface ToolsetConfigPanelProps { * backend's _MODEL_CATALOG_TOOLSETS map). */ const MODEL_CATALOG_TOOLSETS = new Set(['image_gen', 'video_gen']) +/** + * `useNavigate` throws when there is no react-router context. Inside Settings + * (the panel's original home) there always is one, so behavior is unchanged; + * embedded in a plugin dialog OUTSIDE the router there is none, and this + * degrades to `null` instead of crashing the whole panel. Router presence is + * stable for a mounted instance's lifetime, so the try/catch never changes the + * hook count between renders (rules-of-hooks safe). + */ +function useOptionalNavigate(): null | ReturnType { + try { + return useNavigate() + } catch { + return null + } +} + function providerConfigured(provider: ToolProvider, envState: Record): boolean { if (provider.env_vars.length === 0) { return true @@ -91,7 +107,7 @@ interface EnvVarFieldProps { function EnvVarField({ envVar, isSet, onSaved, onCleared, profile }: EnvVarFieldProps) { const { t } = useI18n() const copy = t.settings.toolsets - const navigate = useNavigate() + const navigate = useOptionalNavigate() const [editing, setEditing] = useState(false) const [value, setValue] = useState('') const [revealed, setRevealed] = useState(null) @@ -99,7 +115,9 @@ function EnvVarField({ envVar, isSet, onSaved, onCleared, profile }: EnvVarField // Internal route change to Settings → API Keys (tools sub-view) with the // deep-link param keys-settings consumes to scroll + flash this key's card. - const openInKeys = () => navigate(`${SETTINGS_ROUTE}?tab=keys&key=${encodeURIComponent(envVar.key)}`) + // No-op when there is no router (embedded outside Settings, e.g. a plugin + // dialog): the "Manage keys" affordance simply doesn't navigate there. + const openInKeys = () => navigate?.(`${SETTINGS_ROUTE}?tab=keys&key=${encodeURIComponent(envVar.key)}`) async function handleSave() { if (!value) { diff --git a/apps/desktop/src/app/settings/use-deep-link-highlight.ts b/apps/desktop/src/app/settings/use-deep-link-highlight.ts index 215202ff8e..35321c1ac6 100644 --- a/apps/desktop/src/app/settings/use-deep-link-highlight.ts +++ b/apps/desktop/src/app/settings/use-deep-link-highlight.ts @@ -9,6 +9,20 @@ interface DeepLinkHighlightOptions { block?: ScrollLogicalPosition } +// react-router's useSearchParams throws with no router context. Inside Settings +// (every original caller) there always is one, so behavior is unchanged; when a +// consumer is embedded OUTSIDE the router (e.g. McpTab in a plugin dialog) there +// is none, and this degrades to an inert [empty params, no-op setter] instead of +// crashing. Router presence is stable for a mounted instance's lifetime, so the +// try/catch never changes the hook count between renders (rules-of-hooks safe). +function useOptionalSearchParams(): ReturnType { + try { + return useSearchParams() + } catch { + return [new URLSearchParams(), () => undefined] + } +} + // Deep-link from the command palette (?=): once the target row is // renderable, scroll it into view and flash it, then drop the param so it // doesn't re-fire. Returns the pending target (null once consumed) so callers @@ -20,7 +34,7 @@ export function useDeepLinkHighlight({ onResolve, block = 'center' }: DeepLinkHighlightOptions): null | string { - const [searchParams, setSearchParams] = useSearchParams() + const [searchParams, setSearchParams] = useOptionalSearchParams() const target = searchParams.get(param) useEffect(() => { diff --git a/apps/desktop/src/app/types.ts b/apps/desktop/src/app/types.ts index fe9398d7bb..d5afb3e479 100644 --- a/apps/desktop/src/app/types.ts +++ b/apps/desktop/src/app/types.ts @@ -203,8 +203,15 @@ export interface ClientSessionState { /** Epoch ms the current turn started, or null when idle. Per-session so a * background turn's elapsed timer keeps counting while another session is * focused, and switching sessions doesn't zero a still-running turn's clock. + * Seeded optimistically at submit (before the backend accepts), so it is a + * CLOCK, not proof the turn is live — gate on turnLive for that. * The global $turnStartedAt mirrors whichever session is currently viewed. */ turnStartedAt: number | null + /** The backend has confirmed this turn is running (message.start, a + * running=true session.info edge, or resuming onto an in-flight turn). + * False while a submit is only optimistically armed — the discriminator the + * no-payload settle gate needs now that turnStartedAt is seeded at send. */ + turnLive: boolean /** Cumulative token usage, updated per completed turn. Per-session twin of * the primary-only $currentUsage — the statusbar reads it for a focused * tile's context count. Null until the first turn reports. */ diff --git a/apps/desktop/src/components/assistant-ui/thread/assistant-message.tsx b/apps/desktop/src/components/assistant-ui/thread/assistant-message.tsx index a4ef0d5fbe..55b49e6bd1 100644 --- a/apps/desktop/src/components/assistant-ui/thread/assistant-message.tsx +++ b/apps/desktop/src/components/assistant-ui/thread/assistant-message.tsx @@ -22,6 +22,7 @@ import { MessageTimelineTimestamp } from '@/components/assistant-ui/thread/timel import { useMessageReactions, useTapbackDoubleClick } from '@/components/assistant-ui/thread/use-message-reactions' import { AGENT_MESSAGE_RE } from '@/components/assistant-ui/thread/user-message' import { TooltipIconButton } from '@/components/assistant-ui/tooltip-icon-button' +import { formatElapsed } from '@/components/chat/activity-timer' import { PreviewAttachment } from '@/components/chat/preview-attachment' import { Codicon } from '@/components/ui/codicon' import { CopyButton } from '@/components/ui/copy-button' @@ -105,6 +106,10 @@ export const AssistantMessage: FC<{ // ChatMessage.interim). const isInterim = useAuiState(s => s.message.metadata?.custom?.interim === true) + // Whole-turn wall-clock seconds (set once at completion — referentially + // stable across the 30 Hz delta stream, so this adds no per-token renders). + const turnDurationS = useAuiState(s => s.message.metadata?.custom?.durationS as number | undefined) + // The thinking/stall indicator belongs to the TAIL of the thread, period. A // stale pending bubble mid-transcript (a turn that ended without its settle // event, a steer race) must never show one — a spinner above a later user @@ -223,7 +228,12 @@ export const AssistantMessage: FC<{
{hasVisibleText && !isInterim && ( - + )} {/* Last thing in the turn — under the action bar, the way Cursor ends a turn on its summary rather than burying it above the controls. */} @@ -363,22 +373,35 @@ const ReadAloudButton: FC<{ getText: () => string; messageId: string }> = ({ get ) } -const AssistantFooter: FC = props => ( -
- - - - - - / - - - - - - -
-) +const AssistantFooter: FC = ({ durationS, ...props }) => { + const { t } = useI18n() + + return ( +
+ {durationS !== undefined && ( + + ⏱ {formatElapsed(durationS)} + + )} + + + + + + / + + + + + + +
+ ) +} diff --git a/apps/desktop/src/global.d.ts b/apps/desktop/src/global.d.ts index def8874a34..81aeadb235 100644 --- a/apps/desktop/src/global.d.ts +++ b/apps/desktop/src/global.d.ts @@ -24,6 +24,13 @@ declare global { connectionId?: null | string profile?: null | string }) => Promise + // Registry-scoped fresh WS URL (same result contract as getGatewayWsUrl). + getGatewayWsUrlFor?: (payload: { + connectionId?: null | string + profile?: null | string + }) => Promise + // Union agent roster across every registered connection. + getAgentRoster?: () => Promise // Reconnect-after-wake recovery: liveness-probe the cached PRIMARY backend // and drop it if a remote one has gone unreachable, so the next // getConnection() rebuilds a reachable descriptor instead of the renderer @@ -133,6 +140,9 @@ declare global { remove: (id: string) => Promise<{ ok: boolean; registry: DesktopConnectionsRegistry }> setPrimary: (id: string) => Promise<{ ok: boolean; registry: DesktopConnectionsRegistry }> test: (id: string) => Promise + // Fan out `hermes update` to every eligible registered connection; + // cloud entries are skipped (platform-managed), each row independent. + updateAll?: () => Promise<{ ok: boolean; results: DesktopConnectionUpdateResult[] }> } sshConfigHosts: () => Promise sshResolveHost: (host: string) => Promise @@ -743,6 +753,39 @@ export interface DesktopRegistryConnectionInput { remoteProfile?: string } +// One agent in the union roster: a profile on a registered source, with the +// pre-computed @name-device handle for duplicate profile names. +export interface DesktopRosterAgent { + connectionId: string + connectionKind: DesktopConnectionKind + connectionLabel: string + profile: string + handle: string +} + +export interface DesktopAgentRoster { + agents: DesktopRosterAgent[] + sources: { + connectionId: string + label: string + kind: DesktopConnectionKind + reachable: boolean + error?: string + }[] +} + +// Per-connection result row from the update fan-out. +export interface DesktopConnectionUpdateResult { + connectionId: string + label: string + kind: DesktopConnectionKind + ok: boolean + skipped?: boolean + reason?: string + detail?: string + error?: string +} + export interface DesktopSshResolveResult { hostname: string | null identityFile: string | null diff --git a/apps/desktop/src/i18n/ar.ts b/apps/desktop/src/i18n/ar.ts index 1ba11274e0..91d6c16314 100644 --- a/apps/desktop/src/i18n/ar.ts +++ b/apps/desktop/src/i18n/ar.ts @@ -2310,6 +2310,7 @@ export const ar = defineLocale({ thought: 'فكّر', thoughtBriefly: 'فكّر قليلاً', thoughtFor: duration => `فكّر لمدة ${duration}`, + turnDuration: duration => `استغرقت هذه الجولة ${duration}`, today: time => `اليوم ${time}`, yesterday: time => `أمس ${time}`, copy: 'نسخ', diff --git a/apps/desktop/src/i18n/en.ts b/apps/desktop/src/i18n/en.ts index b1cd428201..2aa80e8b68 100644 --- a/apps/desktop/src/i18n/en.ts +++ b/apps/desktop/src/i18n/en.ts @@ -642,7 +642,7 @@ export const en: Translations = { intro: 'Register every place your agents live — this device, remote gateways on your network, and Hermes Cloud instances. All of them are stored here.', stagedNote: - 'Side-by-side routing is rolling out in stages: connections are managed here today, while the active connection is still chosen in Settings → Gateway.', + 'Chats and the agent roster follow the source you pick; the app-managed window backend is still chosen in Settings → Gateway.', loadFailed: 'Could not load connections', primaryPill: 'Primary', managedPill: 'This device', @@ -658,6 +658,11 @@ export const en: Translations = { testFailed: 'Connection test failed', saveFailed: 'Could not save the connection', removeFailed: 'Could not remove the connection', + updateAll: 'Update all instances', + updateAllRunning: 'Updating all instances…', + updateAllDone: 'Updates dispatched', + updateAllFailed: 'Update fan-out failed', + updateSkippedCloud: 'Managed by Hermes Cloud', kindLocal: 'Local', kindRemote: 'Remote gateway', kindCloud: 'Hermes Cloud', @@ -2873,6 +2878,7 @@ export const en: Translations = { thought: 'Thought', thoughtBriefly: 'Thought briefly', thoughtFor: duration => `Thought for ${duration}`, + turnDuration: duration => `This turn took ${duration}`, today: time => `Today, ${time}`, yesterday: time => `Yesterday, ${time}`, copy: 'Copy', diff --git a/apps/desktop/src/i18n/ja.ts b/apps/desktop/src/i18n/ja.ts index 671f60cb9b..a3674007fd 100644 --- a/apps/desktop/src/i18n/ja.ts +++ b/apps/desktop/src/i18n/ja.ts @@ -2589,6 +2589,7 @@ export const ja = defineLocale({ thought: '思考済み', thoughtBriefly: '少し思考', thoughtFor: duration => `${duration} 思考`, + turnDuration: duration => `このターンの所要時間: ${duration}`, today: time => `今日 ${time}`, yesterday: time => `昨日 ${time}`, copy: 'コピー', diff --git a/apps/desktop/src/i18n/types.ts b/apps/desktop/src/i18n/types.ts index d8b4b00842..86c28057f3 100644 --- a/apps/desktop/src/i18n/types.ts +++ b/apps/desktop/src/i18n/types.ts @@ -547,6 +547,11 @@ export interface Translations { testFailed: string saveFailed: string removeFailed: string + updateAll: string + updateAllRunning: string + updateAllDone: string + updateAllFailed: string + updateSkippedCloud: string kindLocal: string kindRemote: string kindCloud: string @@ -2453,6 +2458,7 @@ export interface Translations { thought: string thoughtBriefly: string thoughtFor: (duration: string) => string + turnDuration: (duration: string) => string today: (time: string) => string yesterday: (time: string) => string copy: string diff --git a/apps/desktop/src/i18n/zh-hant.ts b/apps/desktop/src/i18n/zh-hant.ts index 2374a97393..5d5abe0e07 100644 --- a/apps/desktop/src/i18n/zh-hant.ts +++ b/apps/desktop/src/i18n/zh-hant.ts @@ -2507,6 +2507,7 @@ export const zhHant = defineLocale({ thought: '已思考', thoughtBriefly: '思考了片刻', thoughtFor: duration => `思考了 ${duration}`, + turnDuration: duration => `本輪耗時 ${duration}`, today: time => `今天，${time}`, yesterday: time => `昨天，${time}`, copy: '複製', diff --git a/apps/desktop/src/i18n/zh.ts b/apps/desktop/src/i18n/zh.ts index fa0ad3bd67..3f2641e62b 100644 --- a/apps/desktop/src/i18n/zh.ts +++ b/apps/desktop/src/i18n/zh.ts @@ -847,7 +847,7 @@ export const zh: Translations = { connections: { title: '连接', intro: '注册你的智能体所在的每个位置——本机、局域网中的远程网关、Hermes Cloud 实例——全部保存在这里。', - stagedNote: '多来源路由正在分阶段推出：目前在此管理连接，活动连接仍在“设置 → 网关”中选择。', + stagedNote: '聊天和智能体列表会跟随你选择的来源；应用管理的窗口后端仍在“设置 → 网关”中选择。', loadFailed: '无法加载连接', primaryPill: '主连接', managedPill: '本机', @@ -862,6 +862,11 @@ export const zh: Translations = { testFailed: '连接测试失败', saveFailed: '无法保存连接', removeFailed: '无法移除连接', + updateAll: '更新所有实例', + updateAllRunning: '正在更新所有实例…', + updateAllDone: '更新已分发', + updateAllFailed: '批量更新失败', + updateSkippedCloud: '由 Hermes Cloud 托管', kindLocal: '本地', kindRemote: '远程网关', kindCloud: 'Hermes Cloud', @@ -3043,6 +3048,7 @@ export const zh: Translations = { thought: '已思考', thoughtBriefly: '思考了片刻', thoughtFor: duration => `思考了 ${duration}`, + turnDuration: duration => `本轮耗时 ${duration}`, today: time => `今天，${time}`, yesterday: time => `昨天，${time}`, copy: '复制', diff --git a/apps/desktop/src/lib/chat-messages.ts b/apps/desktop/src/lib/chat-messages.ts index 8f26f8c000..52eb1cf1e2 100644 --- a/apps/desktop/src/lib/chat-messages.ts +++ b/apps/desktop/src/lib/chat-messages.ts @@ -32,6 +32,10 @@ export type ChatMessage = { * action footer so only the turn's final reply carries copy/refresh, and * the live view matches rehydration (which merges the turn into one bubble). */ interim?: boolean + /** Whole-turn wall-clock seconds (message.start → message.complete), + * stamped by the desktop when it watched the turn run. Absent for + * messages hydrated from history — the backend doesn't persist it. */ + durationS?: number /** Composer attachment ref strings (`@file:...`, `@image:...`) sent with this user message. */ attachmentRefs?: string[] /** Durable backend `messages.id`. Absent until the row is persisted. */ @@ -71,6 +75,7 @@ export type GatewayEventPayload = { approval_mode?: string yolo?: boolean running?: boolean + turn_started_at?: number | null cwd?: string branch?: string terminal_backend?: string diff --git a/apps/desktop/src/lib/chat-runtime.ts b/apps/desktop/src/lib/chat-runtime.ts index c00dd185ea..8bf133b674 100644 --- a/apps/desktop/src/lib/chat-runtime.ts +++ b/apps/desktop/src/lib/chat-runtime.ts @@ -43,6 +43,7 @@ export function createClientSessionState( interimBoundaryPending: false, needsInput: false, turnStartedAt: null, + turnLive: false, usage: null } } @@ -482,6 +483,7 @@ export function toRuntimeMessage(message: ChatMessage): ThreadMessage { ...(message.interim ? { interim: true } : {}), ...timelineMeta, ...(message.completedAt !== undefined ? { timelineCompletedAt: message.completedAt } : {}), + ...(message.durationS !== undefined ? { durationS: message.durationS } : {}), ...reactionMeta } } diff --git a/apps/desktop/src/sdk/index.ts b/apps/desktop/src/sdk/index.ts index adf70aa325..499b046324 100644 --- a/apps/desktop/src/sdk/index.ts +++ b/apps/desktop/src/sdk/index.ts @@ -23,8 +23,8 @@ import { atom, type ReadableAtom } from 'nanostores' import { openSession, type OpenSessionIntent } from '@/app/open-session' import { $narrowViewport } from '@/components/pane-shell/tree/store' import { onGatewayEvent } from '@/contrib/events' -import { getLogs, getStatus } from '@/hermes' -import { $gateway, openGatewayForProfile } from '@/store/gateway' +import { getLogs, getStatus, type HermesGateway } from '@/hermes' +import { $gateway, ensureGatewayForAgent, openGatewayForAgent, openGatewayForProfile } from '@/store/gateway' import { notify, notifyError } from '@/store/notifications' import { $activeGatewayProfile, ensureGatewayProfile, newSessionInProfile, setShowAllProfiles } from '@/store/profile' import { $activeSessionId, $currentCwd, $currentModel, $gatewayState } from '@/store/session' @@ -113,6 +113,47 @@ export const host = { void openGatewayForProfile(name).catch(() => undefined) }, + // ── Multi-source agents (the Bot Mode door) ─────────────────────────────── + + /** The registered connection list (labels, kinds, primary) — token bytes + * never included. Rejects on Desktop builds without the registry. */ + connections: async () => { + const bridge = window.hermesDesktop?.connections + + if (!bridge) { + throw new Error('This Desktop build has no connection registry. Update Hermes Desktop.') + } + + return bridge.list() + }, + + /** The union agent roster across every registered connection: one row per + * (source, profile) with the pre-computed @name-device handle for + * duplicates. Sources that are unreachable (or ssh connect-on-demand) + * appear in `sources` with an error instead of failing the call. */ + agents: async () => { + const roster = window.hermesDesktop?.getAgentRoster + + if (!roster) { + throw new Error('This Desktop build cannot enumerate multi-source agents. Update Hermes Desktop.') + } + + return roster() + }, + + /** Pre-dial an agent's socket on ITS source — the (connection, profile) + * analogue of warmProfile. Fire-and-forget, same semantics. */ + warmAgent: (connectionId: null | string, profile: string): void => { + void openGatewayForAgent(connectionId, (profile ?? '').trim() || 'default').catch(() => undefined) + }, + + /** Activate an agent's gateway (dialing it if needed) so subsequent + * host.request calls hit that agent's backend. The local source falls + * through to the profile path — single-source plugins keep working + * against older behavior unchanged. */ + ensureAgent: async (connectionId: null | string, profile: string): Promise => + ensureGatewayForAgent(connectionId, (profile ?? '').trim() || 'default'), + openSession: async ( storedSessionId: string, options: { intent?: OpenSessionIntent; keepAllProfilesScope?: boolean; profile?: null | string } = {} @@ -171,7 +212,14 @@ export const host = { } return gateway.request(method, params) - } + }, + + /** The LIVE gateway instance for the active profile (null before the first + * socket opens). Most plugins want `host.request`; this exists for SDK + * components that take a `HermesGateway` prop directly (e.g. `McpTab`), + * which need the instance, not just a JSON-RPC door. Re-read per use — the + * active instance changes on a profile swap. */ + getGateway: (): HermesGateway | null => $gateway.get() } // -- react bridge ------------------------------------------------------------- @@ -185,6 +233,11 @@ export { COMPOSER_AREAS, type ComposerAttachmentProvider, type ComposerMiddlewar export { PALETTE_AREA, type PaletteContribution } from '@/app/command-palette/contrib' export { type RouteContribution, ROUTES_AREA, SIDEBAR_NAV_AREA, type SidebarNavContribution } from '@/app/routes' +/** THE full per-toolset config panel core Settings renders — provider picker, + * env vars / API keys, model catalog picker, and post-setup runners. Route- + * decoupled (the "manage keys" deep link is a no-op outside the router); pass + * `toolset`, optional `onConfiguredChange`, and an optional `profile`. */ +export { ToolsetConfigPanel } from '@/app/settings/toolset-config-panel' /** THE model catalog menu — the same searchable, provider-grouped, family- * collapsing picker the chat composer uses, including the per-row * thinking/effort/fast submenu. Drive it with a `ModelMenuController`: the @@ -200,6 +253,11 @@ export { export type { StatusbarItem } from '@/app/shell/statusbar-controls' export type { TitlebarTool } from '@/app/shell/titlebar-controls' +/** THE full MCP tab core Settings renders — per-server enable + OAuth sign-in + * + API-key setup + live probes, not a checkbox list. Route-decoupled so it + * renders anywhere (a plugin dialog); pass a live `gateway` (see + * `host.getGateway()`) and an optional `profile` to scope it to one bot. */ +export { McpTab } from '@/app/skills/mcp-tab' /** Pane placement roles. `'floating'` is the one NON-tiling value: the pane is * excluded from the layout tree and rendered as a fixed, draggable card above * it — it takes no width from any zone, has no tab, and can't be docked. @@ -278,6 +336,9 @@ export type { * id with your plugin slug (`kanban:board-switcher`). */ export { Contribute, type ContributeProps } from '@/contrib/react/contribute' export type { Contribution } from '@/contrib/types' +/** The live gateway instance type — for typing the `gateway` prop `McpTab` + * takes; obtain the instance from `host.getGateway()`. */ +export type { HermesGateway } from '@/hermes' /** Grab-to-pan for overflow containers (boards, timelines, wide tables) — * the shared scrub primitive; don't hand-roll drag-to-scroll. */ export { type GrabScroll, useGrabScroll } from '@/hooks/use-grab-scroll' diff --git a/apps/desktop/src/store/gateway.ts b/apps/desktop/src/store/gateway.ts index 8e73a1bfbf..9c78d185d6 100644 --- a/apps/desktop/src/store/gateway.ts +++ b/apps/desktop/src/store/gateway.ts @@ -1,4 +1,4 @@ -import { type ConnectionState, type GatewayEvent, resolveGatewayWsUrl } from '@hermes/shared' +import { backendScopeKey, type ConnectionState, type GatewayEvent, resolveGatewayWsUrl } from '@hermes/shared' import { atom } from 'nanostores' import { HermesGateway } from '@/hermes' @@ -28,7 +28,11 @@ interface RegistryConfig { // ── Secondary (pool) backends ────────────────────────────────────────────── interface Secondary { + /** Scope key from backendScopeKey(connectionId, profile). */ + scope: string profile: string + /** Registry connection serving this socket; null = the local/legacy path. */ + connectionId: null | string gateway: HermesGateway offEvent: () => void offState: () => void @@ -174,15 +178,31 @@ async function openSecondary(entry: Secondary): Promise { return } - const conn = await desktop.getConnection(entry.profile) - const wsUrl = await resolveGatewayWsUrl(desktop, conn) + // Registry-scoped entries dial through getConnectionFor when the bridge has + // it (feature-detected: an older Electron main lacks the door and those + // entries simply can't exist yet — createSecondary guards creation). + const conn = + entry.connectionId && desktop.getConnectionFor + ? await desktop.getConnectionFor({ connectionId: entry.connectionId, profile: entry.profile }) + : await desktop.getConnection(entry.profile) + + const wsUrl = await resolveGatewayWsUrl( + entry.connectionId && desktop.getGatewayWsUrlFor + ? { + getGatewayWsUrl: () => + desktop.getGatewayWsUrlFor!({ connectionId: entry.connectionId, profile: entry.profile }) + } + : desktop, + conn + ) + await entry.gateway.connect(wsUrl) - if (g.activeKey === entry.profile) { + if (g.activeKey === entry.scope) { setConnection(conn) } - void desktop.touchBackend?.(entry.profile).catch(() => undefined) + void desktop.touchBackend?.(entry.scope).catch(() => undefined) } function scheduleReconnect(entry: Secondary): void { @@ -221,11 +241,14 @@ async function reconnectSecondary(entry: Secondary): Promise { } } -function createSecondary(profile: string): Secondary { +function createSecondary(profile: string, connectionId: null | string = null): Secondary { const gateway = new HermesGateway() + const scope = backendScopeKey(connectionId, profile) const entry: Secondary = { + scope, profile, + connectionId, gateway, offEvent: () => {}, offState: () => {}, @@ -235,9 +258,13 @@ function createSecondary(profile: string): Secondary { wantOpen: true } - entry.offEvent = gateway.onEvent(event => g.config?.onEvent({ ...event, profile })) + // Events keep carrying the bare profile — session routing is profile-keyed + // everywhere. connectionId rides along for surfaces that need the source. + entry.offEvent = gateway.onEvent(event => + g.config?.onEvent({ ...event, profile, ...(connectionId ? { connectionId } : {}) }) + ) entry.offState = gateway.onState(state => { - reportGatewayState(profile, state) + reportGatewayState(scope, state) if (state === 'open') { entry.reconnectAttempt = 0 @@ -247,7 +274,7 @@ function createSecondary(profile: string): Secondary { } }) - g.secondaries.set(profile, entry) + g.secondaries.set(scope, entry) return entry } @@ -303,6 +330,65 @@ export async function openGatewayForProfile(profile: string): Promise { } } +// ── Connection-scoped agents (multi-source roster) ───────────────────────── +// The (connectionId, profile) analogues of the profile functions above. A +// null/'local' connectionId falls straight through to the profile path, so +// callers can pass roster rows verbatim without special-casing the local +// source. Feature-detected: without the Electron getConnectionFor door these +// throw, and roster surfaces disable non-local rows instead. + +export async function openGatewayForAgent(connectionId: null | string, profile: string): Promise { + const scope = backendScopeKey(connectionId, profile) + + if (scope === normKey(profile)) { + return openGatewayForProfile(profile) + } + + if (!window.hermesDesktop?.getConnectionFor) { + throw new Error('This Desktop build cannot dial registry connections. Update Hermes Desktop.') + } + + const entry = g.secondaries.get(scope) ?? createSecondary(profile, connectionId) + entry.wantOpen = true + + if (!isOpen(entry.gateway)) { + await openSecondary(entry) + } +} + +export async function ensureGatewayForAgent(connectionId: null | string, profile: string): Promise { + const scope = backendScopeKey(connectionId, profile) + + if (scope === normKey(profile)) { + return ensureGatewayForProfile(profile) + } + + if (!window.hermesDesktop?.getConnectionFor) { + throw new Error('This Desktop build cannot dial registry connections. Update Hermes Desktop.') + } + + let entry = g.secondaries.get(scope) + + if (!entry) { + entry = createSecondary(profile, connectionId) + } + + entry.wantOpen = true + + if (!isOpen(entry.gateway)) { + clearTimer(entry) + entry.reconnectAttempt = 0 + + try { + await openSecondary(entry) + } catch { + scheduleReconnect(entry) + } + } + + setActive(scope) +} + // Make `profile` the active gateway, lazily opening its socket if needed. The // primary is a no-op fast path. Background sockets are never closed here. export async function ensureGatewayForProfile(profile: string): Promise { @@ -387,7 +473,7 @@ export function touchSecondaryGateways(): void { for (const entry of g.secondaries.values()) { if (entry.wantOpen) { - void desktop?.touchBackend?.(entry.profile).catch(() => undefined) + void desktop?.touchBackend?.(entry.scope).catch(() => undefined) } } } @@ -404,9 +490,13 @@ function disposeSecondary(entry: Secondary): void { // Close + evict secondaries whose profile is neither active nor in `keep` // (profiles with a running / needs-input session). Bounds cost to live work. +// `keep` carries PROFILE names (session ownership is profile-keyed), so a +// registry-scoped entry survives when ITS profile has live work — matching on +// the composite key alone would prune every non-local socket the moment the +// user looks away. export function pruneSecondaryGateways(keep: Set): void { for (const [key, entry] of [...g.secondaries]) { - if (key === g.activeKey || keep.has(key)) { + if (key === g.activeKey || keep.has(key) || keep.has(entry.profile)) { continue } diff --git a/apps/desktop/src/types/hermes.ts b/apps/desktop/src/types/hermes.ts index 97b0039ec5..b6291e8455 100644 --- a/apps/desktop/src/types/hermes.ts +++ b/apps/desktop/src/types/hermes.ts @@ -658,6 +658,8 @@ export interface SessionResumeResponse { session_key?: string started_at?: number status?: string + /** Epoch seconds the current turn started, or null when idle. */ + turn_started_at?: number | null } export interface SessionRuntimeInfo { diff --git a/apps/shared/src/backend-scope.ts b/apps/shared/src/backend-scope.ts new file mode 100644 index 0000000000..0da9a73069 --- /dev/null +++ b/apps/shared/src/backend-scope.ts @@ -0,0 +1,29 @@ +/** + * Composite backend scope keys for the multi-connection registry — shared + * between the Electron main process (backend pool keying) and the renderer + * (secondary socket registry), so both sides derive identical keys. + * + * The local/primary connection keeps the BARE profile key: every legacy pool + * entry, reaper log line, and touch call stays byte-identical for + * single-source users. Non-local connections get `conn:::`, + * which cannot collide with a plain profile name (colons are invalid in + * profile names). + */ + +export const LOCAL_CONNECTION_ID = 'local' + +export function backendScopeKey(connectionId: null | string | undefined, profile: null | string | undefined): string { + const profileKey = String(profile ?? '').trim() || 'default' + const connection = String(connectionId ?? '').trim() + + if (!connection || connection === LOCAL_CONNECTION_ID) { + return profileKey + } + + return `conn:${connection}::${profileKey}` +} + +/** All pool keys owned by a connection share this prefix (teardown on remove). */ +export function backendScopePrefix(connectionId: string): string { + return `conn:${String(connectionId).trim()}::` +} diff --git a/apps/shared/src/index.ts b/apps/shared/src/index.ts index 8a17342961..fd40c47174 100644 --- a/apps/shared/src/index.ts +++ b/apps/shared/src/index.ts @@ -1,3 +1,8 @@ +export { + backendScopeKey, + backendScopePrefix, + LOCAL_CONNECTION_ID +} from './backend-scope' export { BILLING_REFUSAL_POLICY, type BillingRecovery, diff --git a/cli-config.yaml.example b/cli-config.yaml.example index 4dee7a61f3..f23783e6f5 100644 --- a/cli-config.yaml.example +++ b/cli-config.yaml.example @@ -133,6 +133,59 @@ model: # CF-Access-Client-Secret: "${CF_ACCESS_SECRET}" # X-Client-Name: "hermes-agent" +# Command-minted credentials (optional): key_cmd +# ------------------------------------------------------------------ +# Enterprise gateways often issue SHORT-LIVED bearers (SSO/OIDC brokers, cloud +# IAM, internal auth proxies) rather than static API keys, so a value copied +# into .env via `key_env` is stale within the hour and every later request 401s. +# `key_cmd` names a command that PRINTS a token instead: Hermes runs it per +# request (cached until shortly before expiry), so long sessions keep working +# with no restart. +# +# Contract: print ONLY the token on stdout, either bare or as JSON with an +# "access_token" field ("expires_in" is honoured). Same shape as OAuth 2.0 +# token endpoints, Claude Code's `apiKeyHelper`, `gcloud auth +# print-access-token`, and `aws ecr get-login-password`. +# +# Precedence: an explicit --api-key still wins; otherwise key_cmd is preferred +# over inline api_key / key_env on that entry. +# +# Applies to the main agent turn and to auxiliary tasks (title generation, +# context compression, vision, embedding) alike. +# +# Not to be confused with `secrets.command`, which is a different mechanism: +# that one runs a helper ONCE at startup to populate env vars for many secrets +# at the process level. Use it for a vault or keychain helper that hands back a +# KEY=VALUE blob. Use `key_cmd` when ONE provider needs a credential refreshed +# DURING a session, because a startup-time env var cannot be re-minted after it +# expires. +# +# providers: +# my-gateway: +# base_url: "https://gateway.internal.example.com/v1" +# api_mode: chat_completions +# key_cmd: "my-auth-cli print-token --profile prod" +# +# Worked example — an AI gateway that routes by model family, so one entry per +# wire format shares the same credential helper: +# +# providers: +# dbx: # OpenAI-compatible (MLflow) route +# base_url: "https://.cloud.databricks.com/ai-gateway/mlflow/v1" +# api_mode: chat_completions +# model: databricks-claude-sonnet-4-6 +# key_cmd: "databricks auth token -p MY-PROFILE" +# dbx-gpt: # OpenAI Responses route +# base_url: "https://.cloud.databricks.com/ai-gateway/openai/v1" +# api_mode: codex_responses +# model: databricks-gpt-5-5 +# key_cmd: "databricks auth token -p MY-PROFILE" +# dbx-claude: # Anthropic Messages route +# base_url: "https://.cloud.databricks.com/ai-gateway/anthropic" +# api_mode: anthropic_messages +# model: databricks-claude-fable-5 +# key_cmd: "databricks auth token -p MY-PROFILE" + # Named provider overrides (optional) # Use this for per-provider request timeouts, non-stream stale timeouts, # and per-model exceptions. @@ -1832,3 +1885,6 @@ updates: # command: "cat /run/user/1000/hermes-secrets.env" # helper_timeout_seconds: 3 # override_existing: false # .env/shell win by default +# # This runs ONCE per process at startup, so it cannot replace a credential +# # that expires mid-session. For a single provider whose token needs +# # re-minting during a session, use `providers..key_cmd` instead. diff --git a/cli.py b/cli.py index 1eea326a5b..f136fd9e29 100644 --- a/cli.py +++ b/cli.py @@ -1798,6 +1798,14 @@ def _worktree_has_unpushed_commits(worktree_path: str, timeout: int = 10) -> boo ``refs/remotes/*``. If a repo has no remote-tracking refs yet, there is no usable remote baseline to compare against, so treat it as having no "unpushed" commits. + + SHALLOW-CLONE CAVEAT: in a shallow clone (the installer default) the + shallow boundary can disconnect an older worktree HEAD from origin/*, + making already-public commits look unpushed. The verdict here stays + conservative (True) on purpose — deleting on unverifiable history would + risk real work. Callers that can afford it should deepen first via + ``_deepen_shallow_repo`` (the startup pruner does) or check + ``_repo_is_shallow`` before presenting this verdict as fact. """ import subprocess @@ -1843,6 +1851,88 @@ def _worktree_is_dirty(worktree_path: str, timeout: int = 10) -> bool: return True +def _repo_is_shallow(repo_path: str, timeout: int = 5) -> bool: + """Return whether *repo_path* belongs to a shallow clone. + + Shallowness poisons every history-connectivity verdict the worktree + machinery relies on: an older worktree's HEAD (a past snapshot of main) + is disconnected from current ``origin/main`` by the shallow boundary, so + ``git log HEAD --not --remotes`` misreports thousands of already-public + commits as "unpushed" and the worktree is preserved forever. The default + installer clones with ``--depth 1``, so this is the normal state of a + user install, not an edge case. + + Fails toward False: if git can't be queried we don't want callers to + take shallow-specific branches on top of an unknown state. + """ + import subprocess + + try: + result = subprocess.run( + ["git", "rev-parse", "--is-shallow-repository"], + capture_output=True, text=True, encoding="utf-8", errors="replace", timeout=timeout, cwd=repo_path, + ) + return result.returncode == 0 and result.stdout.strip() == "true" + except Exception: + return False + + +def _deepen_shallow_repo(repo_root: str, timeout: int = 600) -> bool: + """One-time blobless unshallow so history-based verdicts become correct. + + Fetches the full commit/tree graph (``--unshallow --filter=blob:none``) + without downloading historical file contents, which keeps the transfer a + small fraction of a full clone. Runs only from background paths (the + startup pruner thread), never on the interactive session-close path. + + Falls back to a plain ``--unshallow`` if the server rejects partial-clone + filters. Fail-soft: returns whether the repo is actually non-shallow + afterwards; on failure (offline, no remote) callers keep today's + preserve-everything behavior. + """ + import subprocess + + if not _repo_is_shallow(repo_root): + return True + + try: + remotes = subprocess.run( + ["git", "remote"], + capture_output=True, text=True, encoding="utf-8", errors="replace", timeout=10, cwd=repo_root, + ) + names = [r.strip() for r in remotes.stdout.splitlines() if r.strip()] + if remotes.returncode != 0 or not names: + return False + remote = "origin" if "origin" in names else names[0] + + for extra in (["--filter=blob:none"], []): + try: + result = subprocess.run( + ["git", "fetch", remote, "--unshallow", *extra], + capture_output=True, text=True, encoding="utf-8", errors="replace", timeout=timeout, cwd=repo_root, + ) + except subprocess.TimeoutExpired: + return False + if result.returncode == 0: + break + logger.debug( + "git fetch --unshallow%s failed: %s", + " " + " ".join(extra) if extra else "", + result.stderr.strip()[-500:], + ) + except Exception as e: + logger.debug("Deepening shallow repo failed (non-fatal): %s", e) + return False + + deepened = not _repo_is_shallow(repo_root) + if deepened: + logger.info( + "Deepened shallow clone at %s so worktree cleanup can verify " + "push state", repo_root, + ) + return deepened + + # Upper bound on retained `git cherry` verdict entries (see # _save_worktree_merge_cache). Each entry is ~90 bytes, so this caps the cache # near 90 KB even on a repo that churns thousands of worktree branches. @@ -2080,8 +2170,17 @@ def _cleanup_worktree(info: Dict[str, str] = None) -> None: has_unpushed = _worktree_has_unpushed_commits(wt_path, timeout=10) if has_unpushed: - print(f"\n\033[33m⚠ Worktree has unpushed commits, keeping: {wt_path}\033[0m") - print(f" To clean up manually: git worktree remove --force {wt_path}") + if _repo_is_shallow(repo_root): + # In a shallow clone the unpushed verdict is unreliable: the + # shallow boundary disconnects this worktree's history from + # origin/*, so already-public commits look "unpushed". Be honest + # about why we're keeping it — the startup pruner deepens the + # clone in the background and will reap it on a later startup. + print(f"\n\033[33m⚠ Shallow clone — cannot verify push state, keeping: {wt_path}\033[0m") + print(" The next `hermes -w` session deepens the clone and prunes merged worktrees automatically.") + else: + print(f"\n\033[33m⚠ Worktree has unpushed commits, keeping: {wt_path}\033[0m") + print(f" To clean up manually: git worktree remove --force {wt_path}") _active_worktree = None return @@ -2274,6 +2373,15 @@ def _prune_stale_worktrees(repo_root: str, max_age_hours: int = 24) -> None: _prune_orphaned_branches(repo_root) return + # A shallow clone (the installer's default `--depth 1`) disconnects old + # worktree HEADs from current origin/main, so the unpushed-commits guard + # misclassifies every aged worktree as unpushed work and preserves it + # forever. Deepen once — bloblessly, in this background thread — so all + # history verdicts below (and the session-exit cleanup) become correct. + # Fail-soft: offline, we just keep today's preserve-everything behavior. + if _repo_is_shallow(repo_root): + _deepen_shallow_repo(repo_root) + now = time.time() stale_work_cutoff = now - (7 * 24 * 3600) preserved_stale: list = [] @@ -3777,7 +3885,7 @@ _TERMINAL_INPUT_MODE_RESET_SEQ = ( "\x1b[0m" # reset text attributes "\x1b[?25h" # ensure cursor visible ) -_EXTENDED_ENTER_KEYS_SEQ = "\x1b[>1u\x1b[>4;2m" +_EXTENDED_ENTER_KEYS_SEQ = "\x1b[>4;2m" _BACKSLASH_LINE_CONTINUATION_RE = re.compile(r"\\[ \t]*$") @@ -3812,10 +3920,14 @@ def _terminal_supports_extended_enter_keys(env: Optional[Mapping[str, str]] = No def _enable_extended_enter_keys(output=None, env: Optional[Mapping[str, str]] = None) -> bool: """Ask allowlisted terminals to report Shift+Enter distinctly. - Writes both the Kitty keyboard protocol push (CSI >1u) and xterm - modifyOtherKeys level 2 (CSI >4;2m), mirroring the Ink TUI. The exit reset - sequence already pops/resets both modes, so this is safe across normal - exits, Ctrl+C, and SIGTERM cleanup. + Writes xterm modifyOtherKeys level 2 (CSI >4;2m), mirroring the Ink TUI. + We do NOT push the Kitty keyboard protocol (CSI >1u) here because + prompt_toolkit 3.x cannot parse Kitty CSI-u sequences for control + characters — Ctrl+C arrives as ``\\x1b[99;5u`` instead of ``\\x03``, + which neither prompt_toolkit's key bindings nor the kernel's INTR + mechanism can match, leaving Ctrl+C completely dead (#56684). + The exit reset sequence already pops/resets both modes, so this is + safe across normal exits, Ctrl+C, and SIGTERM cleanup. """ if not _terminal_supports_extended_enter_keys(env): return False diff --git a/contributors/emails/807847218@qq.com b/contributors/emails/807847218@qq.com new file mode 100644 index 0000000000..d3939c0f5d --- /dev/null +++ b/contributors/emails/807847218@qq.com @@ -0,0 +1 @@ +Tommy00748 diff --git a/contributors/emails/kray@block.xyz b/contributors/emails/kray@block.xyz new file mode 100644 index 0000000000..a2862ad8f8 --- /dev/null +++ b/contributors/emails/kray@block.xyz @@ -0,0 +1,2 @@ +LordMelkor +# key_cmd credential source diff --git a/contributors/emails/rgerrish@outlook.com b/contributors/emails/rgerrish@outlook.com new file mode 100644 index 0000000000..be73bcfea7 --- /dev/null +++ b/contributors/emails/rgerrish@outlook.com @@ -0,0 +1 @@ +RGerrish diff --git a/hermes_cli/_early_recovery.py b/hermes_cli/_early_recovery.py index 910f8566c3..942eb632d2 100644 --- a/hermes_cli/_early_recovery.py +++ b/hermes_cli/_early_recovery.py @@ -56,11 +56,85 @@ LAZY_REFRESH_REPAIR_PACKAGES: dict[str, str] = { "jwt": "PyJWT", } +# Set only when this process successfully finishes a deferred core install for +# an ``update`` invocation. The normal CLI import that follows must not resolve +# external secret sources: a configured source can map cryptography._rust and +# immediately recreate the self-lock marker this fresh process just consumed. +# Process-local state is intentional so child processes do not inherit the +# bootstrap exception. +_UPDATE_RETRY_RECOVERED = False + + +def _should_skip_external_secret_sources() -> bool: + """Whether this updater already completed its deferred native install.""" + return _UPDATE_RETRY_RECOVERED + def _project_root() -> Path: return Path(__file__).resolve().parent.parent +def _pid_is_running(pid: int) -> bool: + """Best-effort stdlib-only process liveness probe. + + ``os.kill(pid, 0)`` is not a no-op on Windows, so use the Win32 process + handle API there. An access-denied result is conservatively live: racing + an elevated updater is worse than postponing recovery for one launch. + """ + if pid <= 0: + return False + if sys.platform == "win32": + try: + import ctypes + + synchronize = 0x00100000 + kernel32 = ctypes.WinDLL("kernel32", use_last_error=True) + kernel32.OpenProcess.argtypes = [ + ctypes.c_ulong, + ctypes.c_int, + ctypes.c_ulong, + ] + kernel32.OpenProcess.restype = ctypes.c_void_p + kernel32.WaitForSingleObject.argtypes = [ctypes.c_void_p, ctypes.c_ulong] + kernel32.WaitForSingleObject.restype = ctypes.c_ulong + kernel32.CloseHandle.argtypes = [ctypes.c_void_p] + kernel32.CloseHandle.restype = ctypes.c_int + handle = kernel32.OpenProcess(synchronize, False, pid) + if not handle: + return ctypes.get_last_error() == 5 # ERROR_ACCESS_DENIED + try: + return kernel32.WaitForSingleObject(handle, 0) == 258 + finally: + kernel32.CloseHandle(handle) + except Exception: + return True + try: + os.kill(pid, 0) # windows-footgun: ok — Windows returns above + except ProcessLookupError: + return False + except PermissionError: + return True + except OSError: + return False + return True + + +def _marker_owner_is_live(marker: Path) -> bool: + """True when a legacy update marker names a process still running.""" + try: + body = marker.read_text(encoding="utf-8", errors="replace") + except OSError: + return False + for line in body.splitlines(): + key, separator, value = line.partition("=") + if separator and key.strip() == "pid": + try: + return _pid_is_running(int(value.strip())) + except ValueError: + return False + return False + + def _pinned_specs(packages: list[str], project_root: Path) -> list[str]: """Map bare package names to their pinned specs from pyproject.toml. @@ -291,12 +365,10 @@ def recover_if_needed( Never raises: on any failure the import of main.py proceeds and surfaces the real error. """ + global _UPDATE_RETRY_RECOVERED + try: args = sys.argv[1:] if argv is None else argv - # Same deliberately-loose match as main(): the real update flow writes - # and clears its own markers — a recovery install must not race it. - if "update" in args: - return root = _project_root() if project_root is None else project_root if _pytest_owns_live_checkout(root): return @@ -319,8 +391,23 @@ def recover_if_needed( # every launch, so attempts past the ceiling are left for main.py's # post-import recovery path (which can safely probe-import after this # process already holds whatever extensions it needs). + # A live marker owner means another updater is currently inside the + # marker-to-install window. Never race it. A dead owner means this is + # a prior deferral/interruption and MUST be recovered even when this + # launch is itself `hermes update`: CLI and Desktop retries preserve + # that argv, and skipping solely on argv recreates the self-lock loop. if core_marker.exists(): - _complete_pending_core_install(root, core_marker) + if _marker_owner_is_live(core_marker): + return + completed = _complete_pending_core_install(root, core_marker) + if completed and "update" in args: + _UPDATE_RETRY_RECOVERED = True + return + + # Keep the historical update-argv exclusion for the lazy-refresh + # marker. Unlike the core marker it is not a deferred native install, + # and the active update flow owns its probe/repair lifecycle. + if "update" in args: return broken = _probe_broken_packages() @@ -412,7 +499,7 @@ def _release_recovery_lock(root: Path) -> None: pass -def _complete_pending_core_install(root: Path, core_marker: Path) -> None: +def _complete_pending_core_install(root: Path, core_marker: Path) -> bool: """Run the pending core install BEFORE main.py can import native modules. ``recover_if_needed`` invokes this when ``.update-incomplete`` exists — @@ -428,7 +515,8 @@ def _complete_pending_core_install(root: Path, core_marker: Path) -> None: attempts ceiling caps automatic retries so a persistent installer failure does not block every launch (``hermes acp`` included). - Never raises: any failure leaves the marker for the post-import path. + Never raises: any failure leaves the marker for the post-import path and + returns ``False``. Returns ``True`` only after the install succeeds. """ try: from hermes_cli import _install_repair as ir @@ -456,10 +544,10 @@ def _complete_pending_core_install(root: Path, core_marker: Path) -> None: "post-import recovery path.", file=sys.stderr, ) - return + return False if not _claim_recovery_lock(root): - return + return False try: print( @@ -481,7 +569,7 @@ def _complete_pending_core_install(root: Path, core_marker: Path) -> None: "the current venv in the meantime.", file=sys.stderr, ) - return + return False finally: _release_recovery_lock(root) @@ -493,6 +581,7 @@ def _complete_pending_core_install(root: Path, core_marker: Path) -> None: " ✓ Dependency installation completed in the early pass.", file=sys.stderr, ) + return True except Exception: # Never block launch — the marker stays for the post-import path. - pass + return False diff --git a/hermes_cli/banner.py b/hermes_cli/banner.py index 0b15493a39..a1cd4748f6 100644 --- a/hermes_cli/banner.py +++ b/hermes_cli/banner.py @@ -319,6 +319,15 @@ def _check_via_local_git(repo_dir: Path) -> Optional[int]: is_shallow = shallow == "true" try: + # Self-heal abandoned git lock files before fetching. A stale + # .git/shallow.lock from a crashed fetch makes the fetch fail, the + # exception below is swallowed, and stale refs get compared against + # HEAD — silently degrading the passive check until a human removes + # the lock (git never self-heals these). + from hermes_cli.gitlock import clear_stale_git_locks + + clear_stale_git_locks(repo_dir) + # Scope the fetch to the one branch the behind-count compares against. # An unscoped ``git fetch origin`` transfers every remote head (~1,400 # on this repo — measured 3.0 s vs 0.55 s scoped) and can burn the full diff --git a/hermes_cli/config.py b/hermes_cli/config.py index bca802d075..cd226a92ad 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -1360,6 +1360,7 @@ def _normalize_custom_provider_entry( # configs don't warn on every load. "provider", "name", "api", "url", "base_url", "api_key", "key_env", "api_key_env", + "key_cmd", "api_mode", "transport", "model", "default_model", "models", "context_length", "rate_limit_delay", "request_timeout_seconds", "stale_timeout_seconds", diff --git a/hermes_cli/env_loader.py b/hermes_cli/env_loader.py index b3a79654a7..f926c6ed4e 100644 --- a/hermes_cli/env_loader.py +++ b/hermes_cli/env_loader.py @@ -471,6 +471,7 @@ def load_hermes_dotenv( *, hermes_home: str | os.PathLike | None = None, project_env: str | os.PathLike | None = None, + load_external_secrets: bool = True, ) -> list[Path]: """Load Hermes environment files with user config taking precedence. @@ -479,6 +480,9 @@ def load_hermes_dotenv( - project `.env` acts as a dev fallback and only fills missing values when the user env exists. - if no user env exists, the project `.env` also overrides stale shell vars. + - callers that only maintain the installation can set + ``load_external_secrets=False`` to avoid loading optional secret-manager + dependencies into the process that replaces that same environment. """ loaded: list[Path] = [] @@ -517,7 +521,21 @@ def load_hermes_dotenv( _load_dotenv_with_fallback(project_env_path, override=not loaded) loaded.append(project_env_path) - _apply_external_secret_sources(home_path) + # External secret sources are skipped in two updater situations: + # 1. ``load_external_secrets=False`` — the caller is an ``update`` + # invocation that must not import optional secret-manager libraries + # (Bitwarden → cryptography → ``_rust.pyd``) into the process that + # replaces that same environment on Windows (#73381, #86735). + # 2. A fresh ``hermes update`` retry just completed a deferred dependency + # install before importing this module. Do not remap native + # secret-source dependencies in that same updater process or the + # self-lock preflight will recreate the marker and exit 2 again. + # Dotenv and managed env still load in both cases; only external source + # resolution is unnecessary for the updater. + from hermes_cli import _early_recovery + + if load_external_secrets and not _early_recovery._should_skip_external_secret_sources(): + _apply_external_secret_sources(home_path) _apply_managed_env() # config.yaml is the documented source of truth for terminal.* settings, diff --git a/hermes_cli/gateway.py b/hermes_cli/gateway.py index ea58a89e47..a949bb70db 100644 --- a/hermes_cli/gateway.py +++ b/hermes_cli/gateway.py @@ -1705,6 +1705,26 @@ def _reap_unsupervised_gateway_orphans(extra_exclude: set | None = None) -> bool except Exception: return False + # Windows Task Scheduler is a supervisor too — and the most reliable + # signal is the task's own state, not a parent-chain walk. A Scheduled + # Task gateway whose conhost bootstrap has already exited is invisible + # to `_reaper_candidate_is_supervisor_owned` (the parent chain breaks + # before services.exe, fail-open), yet it is alive and supervised. + # Querying the task state catches that case: if HermesGateway is + # Running, the reaper must not touch its process tree (#86098). + if is_windows(): + try: + # The install-time task name is profile-aware (Hermes_Gateway / + # Hermes_Gateway_) — never hardcode it, or the guard is + # dormant on every standard `hermes gateway install` deployment. + from hermes_cli.gateway_windows import get_task_name + + _task_name = get_task_name() + except Exception: + _task_name = "Hermes_Gateway" + if _windows_scheduled_task_running(_task_name): + return False + from gateway.status import _pid_exists, write_planned_stop_marker own = {os.getpid()} @@ -1937,6 +1957,51 @@ def is_windows() -> bool: return sys.platform == "win32" +def _windows_scheduled_task_running(task_name: str) -> bool: + """Return True when a Windows scheduled task with ``task_name`` is Running. + + Used to treat Task Scheduler as a gateway supervisor on Windows: the + orphan-reap sweep must not kill a gateway that a scheduled task is + actively managing (it writes the planned-stop marker, the gateway exits + cleanly with code 0, and the scheduler never restarts it — silently + killing A2A/messaging on every desktop-app launch). + + Best-effort: any failure (missing task, powershell unavailable, timeout) + returns False so the caller falls back to its existing behaviour. + + Implemented with PowerShell (``Get-ScheduledTask``) instead of ``schtasks`` + because the latter localizes its output (a Chinese Windows prints + ``状态: 正在运行``, not ``Status: Running``) and emits the local codepage, + which ``subprocess`` with ``encoding="utf-8"`` silently mangles. The + ``State`` property of ``Get-ScheduledTask`` is an English enum value + (``Running`` / ``Ready`` / ``Disabled``), stable across locales. + """ + if not is_windows(): + return False + try: + powershell = shutil.which("powershell") or shutil.which("pwsh") + if powershell is None: + return False + ps_cmd = ( + f"$t = Get-ScheduledTask -TaskName '{task_name}' " + "-ErrorAction SilentlyContinue; if ($t) { $t.State } else { 'MISSING' }" + ) + result = subprocess.run( + [powershell, "-NoProfile", "-Command", ps_cmd], + capture_output=True, + text=True, + encoding="utf-8", + errors="ignore", + timeout=10, + ) + if result.returncode != 0: + return False + state = (result.stdout or "").strip() + return state == "Running" + except (OSError, subprocess.TimeoutExpired): + return False + + def _windows_gateway_should_absorb_console_controls() -> bool: """Return True for detached Windows gateway runs that should ignore Ctrl+C. diff --git a/hermes_cli/gitlock.py b/hermes_cli/gitlock.py new file mode 100644 index 0000000000..1485f78f7f --- /dev/null +++ b/hermes_cli/gitlock.py @@ -0,0 +1,127 @@ +"""Stale git lock-file recovery for update/check paths. + +A crashed or killed ``git fetch`` on a shallow clone can leave +``.git/shallow.lock`` behind. Every later fetch then fails with:: + + fatal: Unable to create '/path/.git/shallow.lock': File exists. + +This wedges ``hermes update --check`` (hard failure) and silently degrades the +passive banner check in :mod:`hermes_cli.banner` (the fetch is swallowed, the +stale refs are compared, and the user can be told an update is available when +the checkout already contains the remote tip). Git does not self-heal these +lock files — they persist until a human removes them. + +This module provides two small, defensive helpers used by the update paths: + +* :func:`clear_stale_git_locks` — remove abandoned ``.git`` lock files (with + an age + git-process guard so a live fetch is never yanked). +* :func:`is_ancestor_of_head` — ask whether a remote tip is already contained + in HEAD. Used by the shallow-clone update check to avoid reporting a false + "update available" when local cherry-picks sit on top of the remote tip. +""" + +from __future__ import annotations + +import logging +import os +import subprocess +import time +from pathlib import Path +from typing import List, Optional + +logger = logging.getLogger(__name__) + +# Lock files younger than this are presumed live (a fetch is in flight) and +# are never removed. git lock files are created and removed within a single +# fetch (seconds); anything older than 10 minutes is abandoned by any +# reasonable standard. +STALE_LOCK_MIN_AGE_SECONDS = 10 * 60 + +# Lock files we know how to self-heal. ``shallow.lock`` is the one observed in +# the wild (interrupted fetch on a shallow clone); the others are the same +# class of failure (interrupted git operation) and harmless to clear when +# stale. Index/HEAD locks from a live git process are protected by the +# process guard in :func:`clear_stale_git_locks`. +LOCK_NAMES = ("shallow.lock", "index.lock", "HEAD.lock", "MERGE_HEAD.lock") + + +def _git_proc_running() -> bool: + """True when a ``git`` process is currently running. + + The conservative answer on any platform we can't probe: if we can't tell, + treat a lock as possibly-live and don't remove it. This is the safety + check that stops us from yanking a lock a real fetch is holding. + """ + try: + if os.name == "nt": + out = subprocess.run( + ["tasklist", "/FI", "IMAGENAME eq git.exe", "/FO", "CSV"], + capture_output=True, text=True, timeout=10, + ).stdout.lower() + return "git.exe" in out + out = subprocess.run( + ["pgrep", "-x", "git"], capture_output=True, text=True, timeout=10, + ) + return out.returncode == 0 + except Exception: + logger.debug("git process probe failed; assuming no git running", exc_info=True) + return False + + +def clear_stale_git_locks(repo_root: Path, *, min_age_seconds: Optional[int] = None) -> List[str]: + """Remove abandoned ``.git`` lock files under ``repo_root``. + + A lock is removed only when BOTH conditions hold: + + * it is older than :data:`STALE_LOCK_MIN_AGE_SECONDS` (default), and + * no ``git`` process is currently running. + + Returns the list of removed lock file paths. Never raises: a lock we + cannot stat or unlink is skipped (a concurrently-held lock may have just + been created between our age check and the unlink — the process guard + makes that window vanishingly small, and skipping is always safe). + """ + git_dir = Path(repo_root) / ".git" + if not git_dir.is_dir(): + return [] + + if _git_proc_running(): + logger.debug("git process running; skipping stale-lock sweep") + return [] + + cutoff = time.time() - (min_age_seconds if min_age_seconds is not None else STALE_LOCK_MIN_AGE_SECONDS) + removed: List[str] = [] + for name in LOCK_NAMES: + lock_path = git_dir / name + try: + if lock_path.is_file() and lock_path.stat().st_mtime < cutoff: + lock_path.unlink() + removed.append(str(lock_path)) + logger.info("Removed stale git lock %s", lock_path) + except OSError: + logger.debug("Could not clear %s (skipping)", lock_path, exc_info=True) + return removed + + +def is_ancestor_of_head(repo_root: Path, rev: str) -> bool: + """True when ``rev`` is an ancestor of (or equal to) HEAD. + + Wraps ``git merge-base --is-ancestor HEAD``. This is the correct + question for update checks: a local cherry-pick on top of the remote tip + makes HEAD *different* from ``origin/main`` but still *contains* it, so + the answer to "is there an update?" is no. + + Returns False on any probe failure (missing rev, shallow boundary, git + error) — callers treat that as "can't prove contained", which is the + conservative direction for an update check. + """ + try: + result = subprocess.run( + ["git", "merge-base", "--is-ancestor", rev, "HEAD"], + cwd=str(repo_root), + capture_output=True, text=True, timeout=10, + ) + return result.returncode == 0 + except Exception: + logger.debug("merge-base --is-ancestor probe failed for %s", rev, exc_info=True) + return False diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 93758d0c00..7c3efbb415 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -696,7 +696,17 @@ _apply_profile_override() from hermes_cli.config import get_hermes_home from hermes_cli.env_loader import load_hermes_dotenv -load_hermes_dotenv(project_env=PROJECT_ROOT / ".env") +# Updating dependencies must not import optional secret-manager libraries into +# the updater process before ``uv`` replaces the environment. On Windows, +# Bitwarden's cryptography import maps ``_rust.pyd`` and the parent updater then +# prevents its own child installer from replacing that file (#73381). Profile +# flags have already been stripped above, so the first remaining argument is +# the authoritative argparse subcommand. Dotenv/managed config still loads; +# only external secret fetches are unnecessary for installation maintenance. +load_hermes_dotenv( + project_env=PROJECT_ROOT / ".env", + load_external_secrets=sys.argv[1:2] != ["update"], +) # Bridge security.redact_secrets from config.yaml → HERMES_REDACT_SECRETS env # var BEFORE hermes_logging imports agent.redact (which snapshots the flag at diff --git a/hermes_cli/runtime_provider.py b/hermes_cli/runtime_provider.py index 92cb28c845..a7da0ae7f1 100644 --- a/hermes_cli/runtime_provider.py +++ b/hermes_cli/runtime_provider.py @@ -763,6 +763,13 @@ def _get_named_custom_provider(requested_provider: str) -> Optional[Dict[str, An if isinstance(extra_body, dict): result["extra_body"] = dict(extra_body) _lift_extra_headers(entry, result) + # Command that PRINTS a credential, for gateways issuing + # short-lived bearers instead of static keys. Propagated + # raw; wrapped in a per-request token provider at + # resolution. + key_cmd = str(entry.get("key_cmd", "") or "").strip() + if key_cmd: + result["key_cmd"] = key_cmd # The v11→v12 migration writes the API mode under the new # ``transport`` field, but hand-edited configs may still # use the legacy ``api_mode`` spelling. Accept both — @@ -1171,6 +1178,22 @@ def _resolve_named_custom_runtime( ] api_key = next((candidate for candidate in api_key_candidates if has_usable_secret(candidate)), "") + # A ``key_cmd`` credential is minted per request rather than resolved once: + # gateways that issue short-lived bearers would otherwise go stale + # mid-session and 401. Both wire clients already accept a callable api_key + # (the Entra ID contract) and invoke it per request. An explicit --api-key + # still wins — it is the one-off recovery escape hatch. + key_cmd = str(custom_provider.get("key_cmd", "") or "").strip() + if key_cmd and not has_usable_secret((explicit_api_key or "").strip()): + from agent.command_token_source import build_command_token_provider + + token_provider = build_command_token_provider( + key_cmd, + str(custom_provider.get("name", requested_provider) or "custom"), + ) + if token_provider is not None: + api_key = token_provider + result = { "provider": "custom", "api_mode": custom_provider.get("api_mode") diff --git a/hermes_cli/tools_config.py b/hermes_cli/tools_config.py index 5858bf950e..9aaef475cd 100644 --- a/hermes_cli/tools_config.py +++ b/hermes_cli/tools_config.py @@ -3671,9 +3671,17 @@ def _is_provider_active( ) -> bool: """Check if a provider entry matches the currently active config.""" plugin_name = provider.get("image_gen_plugin_name") - if plugin_name: + if plugin_name and not provider.get("managed_nous_feature"): + # Managed (Nous-subscription) entries fall through to the + # managed_feature branch below, which also checks use_gateway — + # otherwise a managed FAL pick and a direct-key FAL pick would both + # report active for the same provider name (video already guards). image_cfg = config.get("image_gen", {}) - return isinstance(image_cfg, dict) and image_cfg.get("provider") == plugin_name + if not (isinstance(image_cfg, dict) and image_cfg.get("provider") == plugin_name): + return False + # A direct-key entry is only active when the managed route is OFF — + # mirror of the managed branch's use_gateway check. + return not is_truthy_value(image_cfg.get("use_gateway"), default=False) video_plugin_name = provider.get("video_gen_plugin_name") if video_plugin_name and not provider.get("managed_nous_feature"): @@ -4026,14 +4034,22 @@ def _configure_xai_imagine_storage(section_name: str, config: dict) -> None: _print_success(" xAI stored public URLs enabled without automatic expiry") -def _select_plugin_image_gen_provider(plugin_name: str, config: dict) -> None: - """Persist a plugin-backed image generation provider selection.""" +def _select_plugin_image_gen_provider(plugin_name: str, config: dict, *, use_gateway: bool = False) -> None: + """Persist a plugin-backed image generation provider selection. + + ``use_gateway`` mirrors :func:`_select_plugin_video_gen_provider`: a + provider picked through the Nous-managed flow must keep routing through + the gateway. Hardcoding ``False`` here silently flipped Nous-managed FAL + picks onto the user's personal FAL_KEY — _write_provider_config sets + ``image_gen.use_gateway = True`` for a managed pick, and this function + runs AFTER it, so the hardcoded value clobbered the managed flag. + """ img_cfg = config.setdefault("image_gen", {}) if not isinstance(img_cfg, dict): img_cfg = {} config["image_gen"] = img_cfg img_cfg["provider"] = plugin_name - img_cfg["use_gateway"] = False + img_cfg["use_gateway"] = use_gateway _print_success(f" image_gen.provider set to: {plugin_name}") _configure_imagegen_model_for_plugin(plugin_name, config) if plugin_name == "xai": @@ -4386,7 +4402,7 @@ def _configure_provider( # and route model selection to the plugin's own catalog. plugin_name = provider.get("image_gen_plugin_name") if plugin_name: - _select_plugin_image_gen_provider(plugin_name, config) + _select_plugin_image_gen_provider(plugin_name, config, use_gateway=bool(managed_feature)) return # Plugin-registered video_gen provider — same flow, different # registry. @@ -4471,7 +4487,7 @@ def _configure_provider( _print_success(f" {provider['name']} configured!") plugin_name = provider.get("image_gen_plugin_name") if plugin_name: - _select_plugin_image_gen_provider(plugin_name, config) + _select_plugin_image_gen_provider(plugin_name, config, use_gateway=bool(managed_feature)) return video_plugin = provider.get("video_gen_plugin_name") if video_plugin: @@ -4917,7 +4933,7 @@ def _reconfigure_provider( _print_info(" Requests for this tool will be billed to your Nous subscription.") plugin_name = provider.get("image_gen_plugin_name") if plugin_name: - _select_plugin_image_gen_provider(plugin_name, config) + _select_plugin_image_gen_provider(plugin_name, config, use_gateway=bool(managed_feature)) return # Plugin-registered video_gen provider — same flow, different registry. video_plugin = provider.get("video_gen_plugin_name") @@ -4959,7 +4975,7 @@ def _reconfigure_provider( # Imagegen backends prompt for model selection on reconfig too. plugin_name = provider.get("image_gen_plugin_name") if plugin_name: - _select_plugin_image_gen_provider(plugin_name, config) + _select_plugin_image_gen_provider(plugin_name, config, use_gateway=bool(managed_feature)) return # Plugin-registered video_gen provider — same flow, different registry. diff --git a/hermes_cli/update_cmd.py b/hermes_cli/update_cmd.py index c9df3f94de..9ed5895041 100644 --- a/hermes_cli/update_cmd.py +++ b/hermes_cli/update_cmd.py @@ -2449,6 +2449,16 @@ def _cmd_update_check(branch: str = "main", *, branch_explicit: bool = False): if sys.platform == "win32": git_cmd = ["git", "-c", "windows.appendAtomically=false"] + # A crashed/interrupted fetch can leave .git/shallow.lock (or another git + # lock file) behind; every later fetch then fails with "File exists" and + # the check reports a hard failure (or, in the banner path, silently + # compares stale refs). Self-heal abandoned locks before fetching. + from hermes_cli.gitlock import clear_stale_git_locks + + cleared = clear_stale_git_locks(_m().PROJECT_ROOT) + for lock_path in cleared: + print(f" (removed stale git lock: {lock_path})") + # Fetch only the branch we compare against; prefer upstream as the canonical # reference. A bare `git fetch ` pulls every ref, and this repo has # thousands of auto-generated branches, so scope the fetch to . @@ -4524,6 +4534,16 @@ def _cmd_update_impl(args, gateway_mode: bool): # against. branch = _m()._resolve_update_branch(args) + # Self-heal abandoned git lock files (e.g. .git/shallow.lock left by a + # crashed fetch) before the fetch — otherwise the update fails with + # "Unable to create .../shallow.lock: File exists" and never reaches + # the network. + from hermes_cli.gitlock import clear_stale_git_locks + + cleared = clear_stale_git_locks(_m().PROJECT_ROOT) + if cleared: + print(" (removed stale git lock(s): %s)" % ", ".join(cleared)) + print("→ Fetching updates...") fetch_result = subprocess.run( git_cmd + ["fetch", "origin", branch], diff --git a/run_agent.py b/run_agent.py index 18a5660077..dca335fdcc 100644 --- a/run_agent.py +++ b/run_agent.py @@ -8541,6 +8541,7 @@ class AIAgent: conversation_history = _turn_db.get_messages_as_conversation( self.session_id, repair_alternation=True, + include_row_ids=True, ) # Long model/tool/compression turns outlive a fixed TTL. Refresh diff --git a/tests/agent/test_command_token_source.py b/tests/agent/test_command_token_source.py new file mode 100644 index 0000000000..e7210480ef --- /dev/null +++ b/tests/agent/test_command_token_source.py @@ -0,0 +1,349 @@ +"""``key_cmd``: derive a provider API key by running a command. + +Gateways that issue short-lived bearers (SSO/OIDC brokers, cloud IAM, internal +auth proxies) make a stored key go stale mid-session. These tests pin the three +behaviours that make the feature work: + +* resolution yields a CALLABLE (invoked per request) rather than a resolved + string, so a long session never sends a stale token; +* the token is cached until shortly before expiry, so the command is not run + once per request; +* a failure never leaks the helper's output or the command string, either of + which can contain a credential. +""" + +from __future__ import annotations + +import time +from types import SimpleNamespace + +import pytest + +from agent.command_token_source import ( + CommandTokenError, + CommandTokenSource, + _mint, + build_command_token_provider, +) + + +class TestMinting: + def test_bare_token_stdout(self): + source = CommandTokenSource("printf 'tok-abc'", "dbx") + assert source() == "tok-abc" + + def test_json_access_token(self): + """The OAuth 2.0 token-endpoint response shape.""" + source = CommandTokenSource( + """printf '{"access_token":"tok-json","expires_in":3600}'""", "dbx" + ) + assert source() == "tok-json" + + def test_trailing_newline_is_stripped(self): + """A raw newline in the credential would corrupt the auth header.""" + assert CommandTokenSource("echo tok-nl", "dbx")() == "tok-nl" + + def test_multiline_output_is_rejected_not_guessed(self): + """Only the token may land on stdout. + + Silently taking the first line turns a misconfigured helper (banner, + warning, two tokens) into a corrupt-credential 401 that is much harder + to diagnose than an explicit refusal. + """ + source = CommandTokenSource("printf 'banner\\ntok-real'", "dbx") + with pytest.raises(CommandTokenError, match="multiple lines"): + source() + + def test_json_without_access_token_is_an_error(self): + source = CommandTokenSource("""printf '{"nope":1}'""", "dbx") + with pytest.raises(CommandTokenError, match="access_token"): + source() + + def test_empty_output_is_an_error(self): + with pytest.raises(CommandTokenError, match="no output"): + CommandTokenSource("true", "dbx")() + + def test_nonzero_exit_is_an_error(self): + with pytest.raises(CommandTokenError, match="exited 3"): + CommandTokenSource("exit 3", "dbx")() + + def test_failure_message_is_actionable_without_echoing_the_command(self): + """Actionable, but never echoes the command (it may embed a secret).""" + secret_cmd = "print-token --client-secret=SENTINEL-SECRET; exit 1" + with pytest.raises(CommandTokenError) as excinfo: + CommandTokenSource(secret_cmd, "dbx")() + message = str(excinfo.value) + assert "SENTINEL-SECRET" not in message + assert "dbx" in message # names the provider to fix + assert "exited" in message # states what happened + + +class TestNoCredentialLeak: + def test_failure_message_excludes_command_output(self): + """A failing auth helper may print a token — it must not be surfaced.""" + source = CommandTokenSource( + "printf 'SENTINEL-SECRET'; printf 'stderr-SENTINEL' >&2; exit 1", + "dbx", + ) + with pytest.raises(CommandTokenError) as excinfo: + source() + assert "SENTINEL" not in str(excinfo.value) + + +class TestCaching: + def test_token_is_cached_between_calls(self): + """Without caching the command would run on every request.""" + # A command whose output changes each run: equal results prove caching. + source = CommandTokenSource("date +%s%N", "dbx") + assert source() == source() + + def test_expired_token_is_reminted(self): + # date +%s%N changes every run; $RANDOM would be bash-only (empty + # under dash, which is what /bin/sh is on Debian-family CI). + source = CommandTokenSource( + """printf '{"access_token":"tok-%s","expires_in":3600}' "$(date +%s%N)" """, + "dbx", + ) + first = source() + # Force the cache past its expiry. + source._expires_at = 0.0 + assert source() != first + + def test_no_advertised_ttl_caches_on_a_bounded_window(self): + """No TTL means a bounded cache, not a process-lifetime one. + + Nothing in the request path re-mints on 401 (SDK retries cover + 429/5xx only), so caching forever would wedge an expired token until + restart. The window keeps the helper from running per-request while + guaranteeing an eventual re-mint. + """ + from agent.command_token_source import _NO_TTL_REFRESH_SECONDS + + source = CommandTokenSource("date +%s%N", "dbx") + first = source() + assert 0 < source._expires_at - time.monotonic() <= _NO_TTL_REFRESH_SECONDS + assert source() == first # cached inside the window + source._expires_at = time.monotonic() - 1 # cross the window + assert source() != first # re-minted after it + + def test_advertised_ttl_sets_an_expiry(self): + source = CommandTokenSource( + """printf '{"access_token":"tok","expires_in":3600}'""", "dbx" + ) + source() + assert source._expires_at is not None + + def test_ttl_shorter_than_the_leeway_still_caches_briefly(self): + """A leeway larger than the TTL must not disable caching entirely.""" + source = CommandTokenSource( + """printf '{"access_token":"tok","expires_in":1}'""", "dbx" + ) + source() + assert source._expires_at is not None + assert source._expires_at > 0.0 + + +class TestBuilder: + def test_returns_none_when_unset(self): + assert build_command_token_provider("") is None + assert build_command_token_provider(" ") is None + + def test_returns_callable_when_set(self): + provider = build_command_token_provider("printf tok", "dbx") + assert callable(provider) + assert provider() == "tok" + + +class TestResolutionYieldsACallable: + """The integration contract: a callable reaches the wire client.""" + + def test_key_cmd_entry_resolves_to_a_callable(self, monkeypatch): + from hermes_cli import runtime_provider as rp + + config = { + "providers": { + "dbx": { + "base_url": "https://example.invalid/v1", + "api_mode": "chat_completions", + "model": "m1", + "key_cmd": "printf minted-token", + } + } + } + monkeypatch.setattr(rp, "load_config", lambda *a, **k: config) + monkeypatch.setattr("hermes_cli.config.load_config", lambda *a, **k: config) + + runtime = rp.resolve_runtime_provider(requested="custom:dbx") + api_key = runtime["api_key"] + assert callable(api_key), "key_cmd must resolve to a per-request callable" + assert api_key() == "minted-token" + + def test_explicit_api_key_still_wins(self, monkeypatch): + """``--api-key`` stays the one-off recovery escape hatch.""" + from hermes_cli import runtime_provider as rp + + config = { + "providers": { + "dbx": { + "base_url": "https://example.invalid/v1", + "api_mode": "chat_completions", + "model": "m1", + "key_cmd": "printf minted-token", + } + } + } + monkeypatch.setattr(rp, "load_config", lambda *a, **k: config) + monkeypatch.setattr("hermes_cli.config.load_config", lambda *a, **k: config) + + runtime = rp.resolve_runtime_provider( + requested="custom:dbx", explicit_api_key="sk-explicit-override" + ) + assert runtime["api_key"] == "sk-explicit-override" + + +class TestCallableKeyGetsBearerAuth: + """A callable api_key must reach the Anthropic bearer-hook client path. + + This is why key_cmd needs no per-vendor auth wiring: a static string is + sent as ``x-api-key`` (which OAuth-gated gateways reject with 401), while a + callable routes through the per-request ``Authorization: Bearer`` hook the + Entra ID path already established. Verified against a live gateway with the + SAME token value: static -> 401, callable -> 200. + """ + + def test_callable_takes_the_bearer_hook_path(self, monkeypatch): + import agent.anthropic_adapter as aa + + seen = {} + + def _fake_hook(api_key, base_url, timeout, **kw): + seen["callable"] = callable(api_key) + return object() + + monkeypatch.setattr( + aa, "_build_anthropic_client_with_bearer_hook", _fake_hook + ) + aa.build_anthropic_client( + lambda: "minted-token", "https://gateway.invalid/anthropic" + ) + assert seen.get("callable") is True + + +class TestAbsoluteExpiry: + """Helpers that advertise a deadline instead of a lifetime. + + OAuth 2.0 token endpoints send a relative ``expires_in``, but CLI token + helpers commonly print an absolute ISO 8601 timestamp instead (Databricks + ``expiry``, older Azure ``expiresOn``). Reading only ``expires_in`` treats + those as "no TTL advertised", caches the token for the life of the process, + and every request 401s once the real deadline passes. + """ + + @staticmethod + def _iso(seconds_from_now: float) -> str: + from datetime import datetime, timedelta, timezone + + return ( + datetime.now(timezone.utc) + timedelta(seconds=seconds_from_now) + ).isoformat() + + def test_iso_expiry_yields_a_ttl(self): + deadline = self._iso(3600) + _, ttl = _mint(f"printf '%s' '{{\"access_token\":\"t\",\"expiry\":\"{deadline}\"}}'", "p") + assert ttl is not None, "an advertised deadline must produce a TTL" + assert 3500 < ttl <= 3600 + + def test_azure_expires_on_spelling(self): + deadline = self._iso(1800) + _, ttl = _mint(f"printf '%s' '{{\"access_token\":\"t\",\"expiresOn\":\"{deadline}\"}}'", "p") + assert ttl is not None and 1700 < ttl <= 1800 + + def test_expires_in_still_wins_when_both_present(self): + """The RFC 6749 field is authoritative where a helper sends both.""" + deadline = self._iso(3600) + _, ttl = _mint( + f"printf '%s' '{{\"access_token\":\"t\",\"expires_in\":120,\"expiry\":\"{deadline}\"}}'", + "p", + ) + assert ttl == 120.0 + + def test_unparseable_expiry_is_not_a_ttl(self): + """Junk must fall back to refresh-on-401, never to a guessed deadline.""" + _, ttl = _mint('printf \'%s\' \'{"access_token":"t","expiry":"whenever"}\'', "p") + assert ttl is None + + def test_already_past_expiry_is_not_a_ttl(self): + """A stale deadline must not become a negative or zero TTL.""" + _, ttl = _mint( + f"printf '%s' '{{\"access_token\":\"t\",\"expiry\":\"{self._iso(-60)}\"}}'", "p" + ) + assert ttl is None + + def test_the_token_actually_gets_re_minted(self, tmp_path): + """The regression that mattered: a deadline must expire the cache.""" + counter = tmp_path / "calls" + cmd = ( + f"printf x >> {counter}; " + f"printf '%s' '{{\"access_token\":\"t\",\"expiry\":\"{self._iso(1)}\"}}'" + ) + src = CommandTokenSource(cmd, "p") + src() + assert src._expires_at is not None, "cache must carry a deadline" + src._expires_at = time.monotonic() - 1 # simulate crossing it + src() + assert len(counter.read_text()) == 2, "expired cache must re-run the helper" + + +class TestAuxiliaryResolverHonoursKeyCmd: + """Auxiliary tasks resolve credentials on their own path. + + ``agent.auxiliary_client.resolve_provider_client`` does not go through + ``_resolve_named_custom_runtime``, so a key_cmd honoured only there leaves + title generation, compression, vision and embedding falling back to the + ``no-key-required`` placeholder — the main agent turn succeeds while every + auxiliary call 401s. + """ + + @staticmethod + def _resolve(monkeypatch, entry): + """Resolve *entry* as a named custom provider; return the api_key seen.""" + import agent.auxiliary_client as ac + from hermes_cli import runtime_provider as rp + + monkeypatch.setattr( + rp, "_get_named_custom_provider", + lambda name: dict(entry, name="dbx") if name == "dbx" else None, + ) + seen = {} + + def _spy(*, api_key, base_url, **kw): + seen["api_key"] = api_key + return SimpleNamespace(api_key=api_key, base_url=base_url) + + monkeypatch.setattr(ac, "_create_openai_client", _spy) + ac.resolve_provider_client("dbx") + return seen.get("api_key") + + BASE = {"base_url": "https://example.invalid/v1", "model": "m1"} + + def test_key_cmd_resolves_to_a_callable(self, monkeypatch): + api_key = self._resolve(monkeypatch, {**self.BASE, "key_cmd": "printf minted-token"}) + assert callable(api_key), "auxiliary tasks must mint per request too" + assert api_key() == "minted-token" + + def test_key_cmd_beats_static_credentials(self, monkeypatch): + """Precedence matches the runtime resolver, so both agree on one entry.""" + api_key = self._resolve( + monkeypatch, + {**self.BASE, "api_key": "stale-static", "key_cmd": "printf minted-token"}, + ) + assert callable(api_key) and api_key() == "minted-token" + + def test_static_credentials_still_resolve(self, monkeypatch): + assert self._resolve(monkeypatch, {**self.BASE, "api_key": "static"}) == "static" + + def test_blank_key_cmd_keeps_the_placeholder(self, monkeypatch): + """A blank command must not become a callable that mints nothing.""" + assert self._resolve( + monkeypatch, {**self.BASE, "key_cmd": " "} + ) == "no-key-required" diff --git a/tests/cli/test_worktree.py b/tests/cli/test_worktree.py index 626ca29bd4..aab76a6193 100644 --- a/tests/cli/test_worktree.py +++ b/tests/cli/test_worktree.py @@ -1182,3 +1182,161 @@ class TestPruneParallelEquivalence: cli._prune_stale_worktrees(str(git_repo)) assert not wt.exists(), "serial fallback must still reap the merged tree" + + +class TestShallowCloneDeepening: + """Shallow installer clones (`git clone --depth 1`) break the unpushed + guard: the shallow boundary disconnects an older worktree HEAD from + origin/*, so `git log HEAD --not --remotes` misreports already-public + commits as unpushed and every aged worktree is preserved forever + (real incident: 21 of 25 hermes-* trees stuck on a default install). + + These build a REAL shallow clone over file:// and verify the pruner + deepens it and reaps the false-positive tree. + """ + + @staticmethod + def _run(cmd, cwd): + return subprocess.run( + cmd, cwd=cwd, capture_output=True, text=True, + ) + + @classmethod + def _upstream(cls, tmp_path): + """Upstream repo with one commit (A). Returns its path.""" + up = tmp_path / "upstream" + up.mkdir() + cls._run(["git", "init", "-b", "main"], up) + cls._run(["git", "config", "user.email", "test@test.com"], up) + cls._run(["git", "config", "user.name", "Test"], up) + (up / "README.md").write_text("# upstream\n") + cls._run(["git", "add", "."], up) + cls._run(["git", "commit", "-m", "A"], up) + return up + + @classmethod + def _advance_upstream(cls, up, name): + (up / f"{name}.txt").write_text(f"{name}\n") + cls._run(["git", "add", "."], up) + cls._run(["git", "commit", "-m", name], up) + + @classmethod + def _shallow_clone(cls, tmp_path, up): + clone = tmp_path / "shallow-clone" + subprocess.run( + ["git", "clone", "--depth", "1", f"file://{up}", str(clone)], + capture_output=True, text=True, + ) + cls._run(["git", "config", "user.email", "test@test.com"], clone) + cls._run(["git", "config", "user.name", "Test"], clone) + return clone + + @staticmethod + def _age(path, hours=100): + import time + t = time.time() - (hours * 3600) + os.utime(path, (t, t)) + + def _stuck_worktree(self, tmp_path): + """Build the incident shape: shallow clone at A, worktree at A, + upstream advances to B, shallow fetch moves origin/main to B. + Worktree HEAD (A) is now disconnected from origin/main (B).""" + import cli + + up = self._upstream(tmp_path) + clone = self._shallow_clone(tmp_path, up) + assert cli._repo_is_shallow(str(clone)), "fixture must start shallow" + + wt = clone / ".worktrees" / "hermes-shallowstuck" + (clone / ".worktrees").mkdir() + self._run( + ["git", "worktree", "add", str(wt), "-b", "hermes/hermes-shallowstuck", "HEAD"], + clone, + ) + + self._advance_upstream(up, "B") + # Same shape as the updater: shallow fetch of the new tip only. + self._run(["git", "fetch", "--depth", "1", "origin", "main"], clone) + self._run( + ["git", "update-ref", "refs/remotes/origin/main", "FETCH_HEAD"], clone, + ) + self._age(wt) + return up, clone, wt + + def test_shallow_disconnect_reproduces_false_unpushed(self, tmp_path): + """Sanity: without deepening, the primitive misreports unpushed.""" + import cli + + _, clone, wt = self._stuck_worktree(tmp_path) + assert cli._worktree_has_unpushed_commits(str(wt)), ( + "expected the shallow disconnect to look like unpushed commits — " + "if this stops reproducing, the fixture no longer exercises the bug" + ) + + def test_repo_is_shallow_detection(self, tmp_path, git_repo): + import cli + + up = self._upstream(tmp_path) + clone = self._shallow_clone(tmp_path, up) + assert cli._repo_is_shallow(str(clone)) is True + assert cli._repo_is_shallow(str(git_repo)) is False + assert cli._repo_is_shallow(str(tmp_path / "nonexistent")) is False + + def test_deepen_connects_history_and_clears_false_unpushed(self, tmp_path): + import cli + + _, clone, wt = self._stuck_worktree(tmp_path) + assert cli._worktree_has_unpushed_commits(str(wt)) + + assert cli._deepen_shallow_repo(str(clone)) is True + assert not cli._repo_is_shallow(str(clone)) + assert not cli._worktree_has_unpushed_commits(str(wt)), ( + "after deepening, the worktree's HEAD is an ancestor of " + "origin/main and must no longer count as unpushed" + ) + + def test_pruner_deepens_and_reaps_stuck_worktree(self, tmp_path): + """E2E: the startup pruner itself unshallows and reaps the tree.""" + import cli + + _, clone, wt = self._stuck_worktree(tmp_path) + cli._prune_stale_worktrees(str(clone)) + assert not cli._repo_is_shallow(str(clone)), "pruner should deepen" + assert not wt.exists(), ( + "deepened history proves the tree is merged/public — reap it" + ) + + def test_deepen_offline_fails_soft_and_preserves(self, tmp_path): + """Unreachable remote: deepen fails, verdicts stay conservative.""" + import cli + + _, clone, wt = self._stuck_worktree(tmp_path) + # Point origin somewhere that does not exist. + self._run( + ["git", "remote", "set-url", "origin", f"file://{tmp_path}/gone"], + clone, + ) + assert cli._deepen_shallow_repo(str(clone), timeout=30) is False + cli._prune_stale_worktrees(str(clone)) + assert wt.exists(), ( + "offline deepen failure must fall back to preserving the tree" + ) + + def test_deepen_noop_on_full_clone(self, git_repo): + import cli + assert cli._deepen_shallow_repo(str(git_repo)) is True + + def test_real_unpushed_work_survives_deepening(self, tmp_path): + """Deepening must not turn genuinely unpushed commits reapable.""" + import cli + + _, clone, wt = self._stuck_worktree(tmp_path) + (wt / "real-work.txt").write_text("novel\n") + self._run(["git", "add", "real-work.txt"], wt) + self._run(["git", "commit", "-m", "real unpushed work"], wt) + self._age(wt) + + cli._prune_stale_worktrees(str(clone)) + assert wt.exists(), ( + "genuinely unpushed commit must survive even after deepening" + ) diff --git a/tests/cron/test_cron_run_stale_claim_reap_86721.py b/tests/cron/test_cron_run_stale_claim_reap_86721.py new file mode 100644 index 0000000000..8cb2780d2e --- /dev/null +++ b/tests/cron/test_cron_run_stale_claim_reap_86721.py @@ -0,0 +1,143 @@ +"""Regression for #86721 — a one-shot `hermes cron run` invocation's +dispatched runner thread dies with the exiting process, leaving a stale +'claimed'/'running' row in cron/executions.db that blocks every subsequent +manual run of the same job. recover_interrupted_executions() already +existed and is correctly implemented, but was only ever called at the +long-lived scheduler ticker's own startup -- a one-shot CLI invocation has +no equivalent moment of its own, so the self-heal never ran for it. +""" + +from __future__ import annotations + +import json +import os +import subprocess +import sys +from pathlib import Path + + +def test_stale_claim_from_a_dead_one_shot_process_blocks_new_execution_creation( + tmp_path, +): + """Sanity/negative-control: reproduces the exact reported symptom in + isolation -- a genuinely dead-owner 'claimed' row for a job, with no + recovery step run, simply sits there. (create_execution() doesn't + itself gate on prior rows for the same job_id; the blocking happens + at a higher layer that checks for an existing claimed/running row -- + this test establishes the stale row exists and stays 'claimed' + without recovery, matching the bug report's own SQL evidence.)""" + home = tmp_path / "home" + repo = Path(__file__).resolve().parents[2] + env = os.environ.copy() + env["HERMES_HOME"] = str(home) + env["PYTHONPATH"] = str(repo) + + # Simulate the dispatched runner's owner process dying mid-flight, + # exactly as issue #86721 describes: a one-shot process creates the + # execution row (status='claimed') and exits before finishing it. + create = subprocess.run( + [ + sys.executable, "-c", + "from cron.executions import create_execution; " + "r=create_execution('cron-run-job', source='direct'); " + "print(r['id'])", + ], + cwd=repo, env=env, text=True, capture_output=True, check=True, + ) + execution_id = create.stdout.strip() + + # Without recovery, the row is exactly where the bug report found it. + check = subprocess.run( + [ + sys.executable, "-c", + "import json; from cron.executions import list_executions; " + "print(json.dumps(list_executions(job_id='cron-run-job')))", + ], + cwd=repo, env=env, text=True, capture_output=True, check=True, + ) + records = json.loads(check.stdout.strip()) + assert len(records) == 1 + assert records[0]["id"] == execution_id + assert records[0]["status"] == "claimed" # stranded, matching the report + + +def test_recover_interrupted_executions_reaps_the_stale_claim_from_a_dead_process( + tmp_path, +): + """The self-heal this issue's fix now triggers per one-shot + invocation: recover_interrupted_executions() correctly identifies and + clears a stale claim left by a process that has genuinely exited + (real dead PID, not a mock), unblocking the job for a new run.""" + home = tmp_path / "home" + repo = Path(__file__).resolve().parents[2] + env = os.environ.copy() + env["HERMES_HOME"] = str(home) + env["PYTHONPATH"] = str(repo) + + create = subprocess.run( + [ + sys.executable, "-c", + "from cron.executions import create_execution; " + "r=create_execution('cron-run-job-2', source='direct'); " + "print(r['id'])", + ], + cwd=repo, env=env, text=True, capture_output=True, check=True, + ) + execution_id = create.stdout.strip() + + # This is what #86721's fix now calls, from a FRESH process, mirroring + # exactly what a subsequent `hermes cron run` invocation would trigger + # before attempting its own claim. + recover = subprocess.run( + [ + sys.executable, "-c", + "import json; " + "from cron.executions import recover_interrupted_executions, list_executions; " + "print(recover_interrupted_executions()); " + "print(json.dumps(list_executions(job_id='cron-run-job-2')))", + ], + cwd=repo, env=env, text=True, capture_output=True, check=True, + ) + lines = recover.stdout.strip().splitlines() + assert lines[0] == "1", "exactly one stale execution must be reaped" + records = json.loads(lines[1]) + assert records[0]["id"] == execution_id + assert records[0]["status"] == "unknown", ( + "the stale claim from the dead owner must be reclassified, " + "no longer blocking a fresh claim attempt for this job" + ) + + +def test_try_dispatch_background_run_calls_recovery_before_claiming(monkeypatch): + """Direct unit check on the fix's exact insertion point: the one-shot + dispatch path must call recover_interrupted_executions() before + proceeding to claim/create an execution for THIS run, so a stale row + left by a prior dead one-shot invocation is cleared first.""" + import tools.cronjob_tools as cronjob_tools + + calls = [] + monkeypatch.setattr( + "cron.executions.recover_interrupted_executions", + lambda: calls.append("recovered") or 0, + ) + # Force the function past its own early-return guards so execution + # reaches the point where recovery is called, without needing a real + # gateway/session runtime. + monkeypatch.setattr( + "gateway.session_context.async_delivery_supported", lambda: True + ) + monkeypatch.setattr( + "tools.approval.get_current_session_key", lambda default="": "" + ) + + job = {"id": "unit-test-job", "name": "unit test job", "deliver": "local"} + # session_id is intentionally empty/None too, matching the direct- + # caller ("hermes cron run", tests) early return documented just + # below the recovery call -- this test only needs to confirm recovery + # ran before that point, not exercise the full dispatch. + cronjob_tools._try_dispatch_background_run(job, session_id=None) + + assert calls == ["recovered"], ( + "recover_interrupted_executions() must be called before any claim " + "attempt in the one-shot dispatch path" + ) diff --git a/tests/fixtures/session-resume-active-turn.json b/tests/fixtures/session-resume-active-turn.json new file mode 100644 index 0000000000..048f30039f --- /dev/null +++ b/tests/fixtures/session-resume-active-turn.json @@ -0,0 +1,29 @@ +{ + "info": { + "cwd": "/workspace", + "lazy": false, + "model": "test/model", + "skills": {}, + "tools": {} + }, + "inflight": { + "assistant": "partial answer", + "streaming": true, + "user": "current prompt" + }, + "message_count": 1, + "messages": [ + { + "role": "user", + "text": "earlier prompt" + } + ], + "messages_omitted": false, + "resumed": "stored-running", + "running": true, + "session_id": "rt-running", + "session_key": "stored-running", + "started_at": 1700000000.0, + "status": "working", + "turn_started_at": 1700000123.5 +} diff --git a/tests/hermes_cli/test_early_recovery.py b/tests/hermes_cli/test_early_recovery.py index d7f6688254..d8bac329be 100644 --- a/tests/hermes_cli/test_early_recovery.py +++ b/tests/hermes_cli/test_early_recovery.py @@ -142,6 +142,24 @@ def test_early_recovery_module_is_stdlib_only(tmp_path): # recover_if_needed unit behavior # --------------------------------------------------------------------------- + +def test_pid_liveness_recognizes_current_process(): + assert er._pid_is_running(os.getpid()) is True + assert er._pid_is_running(0) is False + + +def test_marker_owner_liveness_uses_recorded_pid(tmp_path, monkeypatch): + marker = tmp_path / ".update-incomplete" + marker.write_text("started=1\npid=4321\n", encoding="utf-8") + seen = [] + monkeypatch.setattr( + er, "_pid_is_running", lambda pid: seen.append(pid) or True + ) + + assert er._marker_owner_is_live(marker) is True + assert seen == [4321] + + def _project(tmp_path: Path, *, pyproject: bool = True) -> Path: root = tmp_path / "proj" root.mkdir(exist_ok=True) @@ -453,11 +471,40 @@ def test_lazy_marker_alone_does_not_trigger_core_install(tmp_path, monkeypatch): er.recover_if_needed(project_root=root, argv=[]) -def test_core_marker_skipped_when_user_is_running_update(tmp_path, monkeypatch): - """A launch of `hermes update` itself must not race its own markers.""" +def test_core_marker_from_dead_updater_is_recovered_on_update_retry( + tmp_path, monkeypatch +): + """Retrying ``hermes update`` must consume a prior deferral marker. + + The self-lock preflight exits after writing this marker. Desktop and CLI + retries both keep ``update`` in argv, so an argv-only skip loops forever. + """ root = _project(tmp_path) core_marker = root / ".update-incomplete" - core_marker.write_text('{"attempts": 0}', encoding="utf-8") + core_marker.write_text("started=1\npid=1234\n", encoding="utf-8") + + from hermes_cli import _install_repair as ir + + calls = [] + monkeypatch.setattr(ir, "run_core_install", lambda project_root: calls.append(project_root)) + monkeypatch.setattr(er, "_marker_owner_is_live", lambda _marker: False, raising=False) + monkeypatch.setattr(er, "_UPDATE_RETRY_RECOVERED", False) + import hermes_cli._install_repair # noqa: F401 + + er.recover_if_needed(project_root=root, argv=["update"]) + + assert calls == [root] + assert not core_marker.exists() + assert er._should_skip_external_secret_sources() is True + + +def test_core_marker_owned_by_live_updater_is_not_recovered( + tmp_path, monkeypatch +): + """A second launch must not reinstall into an active updater's venv.""" + root = _project(tmp_path) + core_marker = root / ".update-incomplete" + core_marker.write_text("started=1\npid=1234\n", encoding="utf-8") from hermes_cli import _install_repair as ir @@ -465,12 +512,14 @@ def test_core_marker_skipped_when_user_is_running_update(tmp_path, monkeypatch): ir, "run_core_install", lambda _r: (_ for _ in ()).throw( - AssertionError("install must not run for `hermes update` argv") + AssertionError("must not race a live updater") ), ) + monkeypatch.setattr(er, "_marker_owner_is_live", lambda _marker: True, raising=False) import hermes_cli._install_repair # noqa: F401 - er.recover_if_needed(project_root=root, argv=["update"]) + er.recover_if_needed(project_root=root, argv=[]) + assert core_marker.exists() diff --git a/tests/hermes_cli/test_env_loader.py b/tests/hermes_cli/test_env_loader.py index d870e11f6b..1eb34a41c0 100644 --- a/tests/hermes_cli/test_env_loader.py +++ b/tests/hermes_cli/test_env_loader.py @@ -6,6 +6,31 @@ import sys from hermes_cli.env_loader import load_hermes_dotenv +def test_recovered_update_retry_skips_external_secret_sources(tmp_path, monkeypatch): + """The post-recovery updater must not remap native vault dependencies.""" + import hermes_cli.env_loader as env_loader + from hermes_cli import _early_recovery + + home = tmp_path / "hermes" + home.mkdir() + env_file = home / ".env" + env_file.write_text("UPDATE_RETRY_DOTENV=loaded\n", encoding="utf-8") + monkeypatch.delenv("UPDATE_RETRY_DOTENV", raising=False) + monkeypatch.setattr(_early_recovery, "_UPDATE_RETRY_RECOVERED", True) + external_calls = [] + monkeypatch.setattr( + env_loader, + "_apply_external_secret_sources", + lambda path: external_calls.append(path), + ) + + loaded = load_hermes_dotenv(hermes_home=home) + + assert loaded == [env_file] + assert os.environ["UPDATE_RETRY_DOTENV"] == "loaded" + assert external_calls == [] + + def test_utf8_bom_does_not_mangle_first_key(tmp_path, monkeypatch): """A leading UTF-8 BOM must not prefix the first key name in os.environ. diff --git a/tests/hermes_cli/test_gateway.py b/tests/hermes_cli/test_gateway.py index ecda995a25..4e8cfe5747 100644 --- a/tests/hermes_cli/test_gateway.py +++ b/tests/hermes_cli/test_gateway.py @@ -771,3 +771,98 @@ def test_module_has_logger(): """Verify module has a logger instance (regression guard for #27154).""" assert hasattr(gateway, "logger") assert gateway.logger.name == "hermes_cli.gateway" + + +class TestWindowsScheduledTaskSupervisorGuard: + """The reaper must skip entirely when the HermesGateway scheduled task is + Running — Task Scheduler is a supervisor, and the task's own state is the + authoritative signal. + + Regression guard: ``_reaper_candidate_is_supervisor_owned`` walks the + parent chain up to ``services.exe`` and fails open when the Task-launched + bootstrap has already exited (Windows does not reparent, so the chain + breaks). A gateway whose conhost bootstrap exited is then treated as an + orphan: the reaper writes the planned-stop marker, the gateway exits + cleanly with code 0, and the scheduler never restarts it — silently + killing A2A/messaging on every desktop-app launch. Querying the task + state closes that gap without depending on process ancestry. + """ + + def test_running_task_skips_reap(self, monkeypatch): + """HermesGateway is Running => reaper returns False, kills nothing.""" + monkeypatch.setattr(gateway, "is_windows", lambda: True) + monkeypatch.setattr(gateway, "is_macos", lambda: False) + monkeypatch.setattr(gateway, "supports_systemd_services", lambda: False) + # The guard must query the PROFILE-AWARE install-time task name from + # gateway_windows.get_task_name(), never a hardcoded literal — a + # hardcoded "HermesGateway" would leave the guard dormant on every + # standard install (task name is Hermes_Gateway / Hermes_Gateway_

). + import hermes_cli.gateway_windows as gateway_windows + + monkeypatch.setattr( + gateway_windows, "get_task_name", lambda: "Hermes_Gateway_testprof" + ) + queried = [] + + def _fake_task_running(name): + queried.append(name) + return True + + monkeypatch.setattr( + gateway, "_windows_scheduled_task_running", _fake_task_running + ) + + # Guard: if the task check were bypassed, these would be reaped. + def _boom_find_gateway_pids(exclude_pids=None): + raise AssertionError("must not scan when scheduled task is Running") + + monkeypatch.setattr(gateway, "find_gateway_pids", _boom_find_gateway_pids) + killed_pids = [] + monkeypatch.setattr(gateway.os, "kill", lambda pid, sig: killed_pids.append((pid, sig))) + + result = gateway._reap_unsupervised_gateway_orphans() + + assert result is False + assert killed_pids == [] + assert queried == ["Hermes_Gateway_testprof"] + + def test_not_running_task_still_reaps_real_orphan(self, monkeypatch): + """HermesGateway not Running (or missing) => reaper behaves as before + and still reaps a genuine orphan.""" + orphan_pid = 99998 + + monkeypatch.setattr(gateway, "is_windows", lambda: True) + monkeypatch.setattr(gateway, "is_macos", lambda: False) + monkeypatch.setattr(gateway, "supports_systemd_services", lambda: False) + monkeypatch.setattr(gateway, "_windows_scheduled_task_running", lambda name: False) + monkeypatch.setattr("gateway.status.get_running_pid", lambda: None) + monkeypatch.setattr(gateway, "_get_service_pids", lambda: set()) + + monkeypatch.setattr( + gateway, + "find_gateway_pids", + lambda exclude_pids=None: [ + p for p in [orphan_pid] if p not in (exclude_pids or set()) + ], + ) + killed_pids = [] + monkeypatch.setattr(gateway.os, "kill", lambda pid, sig: killed_pids.append((pid, sig))) + monkeypatch.setattr("gateway.status._pid_exists", lambda pid: False) + monkeypatch.setattr("gateway.status.write_planned_stop_marker", lambda pid: None) + monkeypatch.setattr("time.sleep", lambda _: None) + monkeypatch.setattr("time.monotonic", lambda: 1.0) + + result = gateway._reap_unsupervised_gateway_orphans() + + assert result is True + assert orphan_pid in [pid for pid, _ in killed_pids] + + def test_windows_scheduled_task_running_returns_false_off_windows(self, monkeypatch): + """The state helper is inert on POSIX (no subprocess spawned).""" + monkeypatch.setattr(gateway, "is_windows", lambda: False) + + def _boom_run(*_a, **_k): + raise AssertionError("subprocess must not run off Windows") + + monkeypatch.setattr(gateway.subprocess, "run", _boom_run) + assert gateway._windows_scheduled_task_running("HermesGateway") is False diff --git a/tests/hermes_cli/test_imagegen_managed_gateway.py b/tests/hermes_cli/test_imagegen_managed_gateway.py new file mode 100644 index 0000000000..b4ebabadec --- /dev/null +++ b/tests/hermes_cli/test_imagegen_managed_gateway.py @@ -0,0 +1,67 @@ +"""Regression tests for image_gen use_gateway persistence (managed FAL clobber). + +Bug: ``_select_plugin_image_gen_provider`` hardcoded +``image_gen.use_gateway = False``. When a user picked FAL through the +Nous-subscription managed flow, ``_write_provider_config`` first set +``use_gateway = True`` — then the image selector ran and clobbered it back +to False, silently routing every generation through the user's personal +FAL_KEY instead of the Nous Tool Gateway (real incident: personal key +drained to zero while the subscription sat unused). + +The video twin (``_select_plugin_video_gen_provider``) already accepted a +``use_gateway`` kwarg; these tests pin the image path to the same contract. +""" + +from hermes_cli.tools_config import ( + _select_plugin_image_gen_provider, + _select_plugin_video_gen_provider, + _write_provider_config, +) + + +def _quiet(monkeypatch): + import hermes_cli.tools_config as tc + + monkeypatch.setattr(tc, "_print_success", lambda *a, **k: None) + monkeypatch.setattr(tc, "_print_info", lambda *a, **k: None, raising=False) + monkeypatch.setattr(tc, "_configure_imagegen_model_for_plugin", lambda *a, **k: None) + monkeypatch.setattr(tc, "_configure_videogen_model_for_plugin", lambda *a, **k: None) + + +def test_image_gen_selector_preserves_managed_gateway_flag(monkeypatch): + """Managed pick: use_gateway=True must survive the selector.""" + _quiet(monkeypatch) + config = {} + + # The managed flow first writes the managed flag... + _write_provider_config( + {"image_gen_plugin_name": "fal"}, config, managed_feature="image_gen" + ) + assert config["image_gen"]["use_gateway"] is True + + # ...then the selector runs; passing the managed flag must NOT clobber it. + _select_plugin_image_gen_provider("fal", config, use_gateway=True) + assert config["image_gen"]["provider"] == "fal" + assert config["image_gen"]["use_gateway"] is True + + +def test_image_gen_selector_direct_key_pick_clears_gateway(monkeypatch): + """Non-managed pick keeps the historical default: direct key, no gateway.""" + _quiet(monkeypatch) + config = {"image_gen": {"use_gateway": True}} + + _select_plugin_image_gen_provider("fal", config) + assert config["image_gen"]["provider"] == "fal" + assert config["image_gen"]["use_gateway"] is False + + +def test_image_and_video_selectors_share_the_gateway_contract(monkeypatch): + """The two selectors are twins: same kwarg, same persistence behavior.""" + _quiet(monkeypatch) + + for use_gateway in (True, False): + config = {} + _select_plugin_image_gen_provider("fal", config, use_gateway=use_gateway) + _select_plugin_video_gen_provider("fal", config, use_gateway=use_gateway) + assert config["image_gen"]["use_gateway"] is use_gateway + assert config["video_gen"]["use_gateway"] is use_gateway diff --git a/tests/hermes_cli/test_update_secret_import_lock.py b/tests/hermes_cli/test_update_secret_import_lock.py new file mode 100644 index 0000000000..3dd95042d5 --- /dev/null +++ b/tests/hermes_cli/test_update_secret_import_lock.py @@ -0,0 +1,119 @@ +"""Regression coverage for Windows updater self-locking native dependencies. + +External secret backends are useful during normal Hermes startup, but the +updater must not load them before replacing packages in its own environment. +On Windows, importing Bitwarden's ``cryptography`` dependency maps +``_rust.pyd`` into the updater process and prevents ``uv`` from replacing it. +""" + +from __future__ import annotations + +import json +import os +import subprocess +import sys +from pathlib import Path + +import pytest + +from hermes_cli import env_loader + + +REPO_ROOT = Path(__file__).resolve().parents[2] + + +def _probe_startup_modules( + tmp_path: Path, argv: list[str], *, run_main: bool = False +) -> set[str]: + home = tmp_path / "hermes-home" + home.mkdir() + (home / "config.yaml").write_text( + """\ +secrets: + bitwarden: + enabled: true + project_id: test-project +""", + encoding="utf-8", + ) + + dispatch = ( + "hermes_main.cmd_update = lambda _args: 0\n" + "hermes_main.main()\n" + if run_main + else "" + ) + probe = ( + "import json, sys\n" + f"sys.argv = {argv!r}\n" + "import hermes_cli.main as hermes_main\n" + f"{dispatch}" + "print('LOADED_MODULES=' + json.dumps(sorted(sys.modules)))\n" + ) + result = subprocess.run( + [sys.executable, "-c", probe], + capture_output=True, + text=True, + timeout=120, + cwd=REPO_ROOT, + env={**os.environ, "HERMES_HOME": str(home), "BWS_ACCESS_TOKEN": ""}, + ) + assert result.returncode == 0, result.stderr + line = next( + line for line in result.stdout.splitlines() if line.startswith("LOADED_MODULES=") + ) + return set(json.loads(line.removeprefix("LOADED_MODULES="))) + + +def test_update_startup_does_not_import_bitwarden_or_cryptography(tmp_path): + loaded = _probe_startup_modules(tmp_path, ["hermes", "update", "--check"]) + + assert "agent.secret_sources.bitwarden" not in loaded + assert not any( + name == "cryptography" or name.startswith("cryptography.") for name in loaded + ) + + +def test_complete_update_dispatch_does_not_import_cryptography(tmp_path): + """Building every CLI parser used to re-import Bitwarden via secrets_cli.""" + loaded = _probe_startup_modules( + tmp_path, + ["hermes", "update", "--check"], + run_main=True, + ) + + assert not any( + name == "cryptography" or name.startswith("cryptography.") for name in loaded + ) + + +def test_normal_startup_still_loads_enabled_external_secret_source(tmp_path): + loaded = _probe_startup_modules(tmp_path, ["hermes", "chat"]) + + assert "agent.secret_sources.bitwarden" in loaded + + +@pytest.mark.parametrize("external_secrets", [True, False]) +def test_dotenv_loading_is_preserved_when_external_secrets_are_skipped( + tmp_path, monkeypatch, external_secrets +): + home = tmp_path / "hermes-home" + home.mkdir() + env_file = home / ".env" + env_file.write_text("UPDATE_TEST_VALUE=from-dotenv\n", encoding="utf-8") + applied = [] + monkeypatch.delenv("UPDATE_TEST_VALUE", raising=False) + monkeypatch.setattr( + env_loader, + "_apply_external_secret_sources", + lambda path: applied.append(path), + ) + + loaded = env_loader.load_hermes_dotenv( + hermes_home=home, + load_external_secrets=external_secrets, + ) + + assert loaded == [env_file] + assert os.environ["UPDATE_TEST_VALUE"] == "from-dotenv" + assert applied == ([home] if external_secrets else []) diff --git a/tests/hermes_cli/test_update_self_lock.py b/tests/hermes_cli/test_update_self_lock.py index 3c477a6792..3c9d8151ff 100644 --- a/tests/hermes_cli/test_update_self_lock.py +++ b/tests/hermes_cli/test_update_self_lock.py @@ -33,6 +33,7 @@ import pytest import hermes_cli.main as cli_main import hermes_cli.update_cmd as update_cmd +from hermes_cli import _early_recovery ROOT = Path(__file__).resolve().parents[2] @@ -160,6 +161,22 @@ def test_self_lock_detection_clean_when_rust_not_loaded(_winp): ) +def test_recovered_update_retry_builds_parser_without_native_secret_modules( + monkeypatch, +): + """Parser registration must not re-lock a freshly recovered updater.""" + called = [] + monkeypatch.setattr(_early_recovery, "_UPDATE_RETRY_RECOVERED", True) + monkeypatch.setattr(cli_main, "cmd_update", lambda args: called.append(args)) + monkeypatch.setattr(sys, "argv", ["hermes", "update", "--yes"]) + sys.modules.pop("cryptography.hazmat.bindings._rust", None) + + cli_main.main() + + assert len(called) == 1 + assert "cryptography.hazmat.bindings._rust" not in sys.modules + + # --------------------------------------------------------------------------- # _abort_dependency_sync_if_self_locked — deferral wiring # --------------------------------------------------------------------------- diff --git a/tests/run_agent/test_cross_process_turn_lease.py b/tests/run_agent/test_cross_process_turn_lease.py index 847a888abd..a248dc3190 100644 --- a/tests/run_agent/test_cross_process_turn_lease.py +++ b/tests/run_agent/test_cross_process_turn_lease.py @@ -109,6 +109,10 @@ def test_run_conversation_acquires_then_reloads_latest_tip(monkeypatch): "reload", "release", ] + assert db.events[2][2] == { + "repair_alternation": True, + "include_row_ids": True, + } assert any( kind == "lifecycle" and text diff --git a/tests/test_gitlock.py b/tests/test_gitlock.py new file mode 100644 index 0000000000..5d4910421f --- /dev/null +++ b/tests/test_gitlock.py @@ -0,0 +1,111 @@ +"""Tests for hermes_cli.gitlock — stale git lock recovery + ancestry probe. + +These cover the two failure modes that produced the false "update available" +notification and the hard ``update --check`` failure after a crashed fetch on +a shallow clone: + +1. A stale ``.git/shallow.lock`` makes every later ``git fetch`` fail with + "File exists" unless cleared. +2. On a shallow clone the update check compares tip SHAs, so local + cherry-picks on top of the remote tip look like "update available" even + though HEAD already contains the remote tip. + +The module's safety rules are also pinned: a *young* lock or a *running git +process* must never be cleared. +""" + +from __future__ import annotations + +import os +import subprocess +import time +from pathlib import Path + +import pytest + +from hermes_cli.gitlock import ( + LOCK_NAMES, + STALE_LOCK_MIN_AGE_SECONDS, + clear_stale_git_locks, + is_ancestor_of_head, +) + + +@pytest.fixture +def repo(tmp_path: Path) -> Path: + """A real, tiny git repo with two commits (no network).""" + root = tmp_path / "repo" + root.mkdir() + subprocess.run(["git", "init", "-q"], cwd=root, check=True) + subprocess.run(["git", "config", "user.email", "test@example.com"], cwd=root, check=True) + subprocess.run(["git", "config", "user.name", "Test"], cwd=root, check=True) + (root / "a.txt").write_text("one\n") + subprocess.run(["git", "add", "a.txt"], cwd=root, check=True) + subprocess.run(["git", "commit", "-q", "-m", "first"], cwd=root, check=True) + (root / "b.txt").write_text("two\n") + subprocess.run(["git", "add", "b.txt"], cwd=root, check=True) + subprocess.run(["git", "commit", "-q", "-m", "second"], cwd=root, check=True) + return root + + +def _touch(path: Path, age_seconds: float) -> None: + """Create (or truncate) a file and backdate its mtime.""" + path.touch() + old = time.time() - age_seconds + os.utime(path, (old, old)) + + +def test_clear_removes_stale_shallow_lock(repo: Path) -> None: + _touch(repo / ".git" / "shallow.lock", STALE_LOCK_MIN_AGE_SECONDS + 60) + removed = clear_stale_git_locks(repo) + assert str(repo / ".git" / "shallow.lock") in removed + assert not (repo / ".git" / "shallow.lock").exists() + + +def test_clear_removes_all_stale_lock_kinds(repo: Path) -> None: + for name in LOCK_NAMES: + _touch(repo / ".git" / name, STALE_LOCK_MIN_AGE_SECONDS + 60) + removed = clear_stale_git_locks(repo) + assert len(removed) == len(LOCK_NAMES) + for name in LOCK_NAMES: + assert not (repo / ".git" / name).exists() + + +def test_clear_keeps_young_lock(repo: Path) -> None: + _touch(repo / ".git" / "shallow.lock", 1) # 1 second old — presumably live + removed = clear_stale_git_locks(repo) + assert removed == [] + assert (repo / ".git" / "shallow.lock").exists() + + +def test_clear_noop_on_non_repo(tmp_path: Path) -> None: + bare = tmp_path / "not-a-repo" + bare.mkdir() + assert clear_stale_git_locks(bare) == [] + + +def test_clear_noop_with_no_locks(repo: Path) -> None: + assert clear_stale_git_locks(repo) == [] + + +def test_is_ancestor_true_for_first_commit(repo: Path) -> None: + first = subprocess.run( + ["git", "rev-list", "--max-parents=0", "HEAD"], + cwd=repo, capture_output=True, text=True, check=True, + ).stdout.strip() + assert is_ancestor_of_head(repo, first) is True + + +def test_is_ancestor_true_for_head_itself(repo: Path) -> None: + head = subprocess.run( + ["git", "rev-parse", "HEAD"], cwd=repo, capture_output=True, text=True, check=True, + ).stdout.strip() + assert is_ancestor_of_head(repo, head) is True + + +def test_is_ancestor_false_for_unknown_rev(repo: Path) -> None: + assert is_ancestor_of_head(repo, "deadbeef" * 5) is False + + +def test_is_ancestor_false_for_nonexistent_repo(tmp_path: Path) -> None: + assert is_ancestor_of_head(tmp_path / "missing", "HEAD") is False diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index 3f870cc0ec..37250052d0 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -2929,8 +2929,17 @@ def test_session_resume_uses_parent_lineage_for_display(monkeypatch, omit_messag def get_ancestor_display_prefix(self, _sid): return [] - def get_messages_as_conversation(self, target, include_ancestors=False, repair_alternation=False, **_kwargs): - captured.setdefault("history_calls", []).append((target, include_ancestors)) + def get_messages_as_conversation( + self, + target, + include_ancestors=False, + repair_alternation=False, + include_row_ids=False, + **_kwargs, + ): + captured.setdefault("history_calls", []).append( + (target, include_ancestors, include_row_ids) + ) return ( [ {"role": "user", "content": "root prompt"}, @@ -2975,9 +2984,9 @@ def test_session_resume_uses_parent_lineage_for_display(monkeypatch, omit_messag assert resp["result"]["messages"] == expected assert resp["result"]["message_count"] == (1 if omit_messages else 2) assert resp["result"]["messages_omitted"] is omit_messages - expected_calls = [(target, False)] if omit_messages else [ - (target, False), - (target, True), + expected_calls = [(target, False, True)] if omit_messages else [ + (target, False, False), + (target, True, False), ] assert captured["history_calls"] == expected_calls @@ -5097,10 +5106,10 @@ def test_prompt_submit_truncation_falls_back_to_sid_when_session_key_null(monkey replaced.append((key, list(messages))) history = [ - {"role": "user", "content": "first"}, - {"role": "assistant", "content": "reply 1"}, - {"role": "user", "content": "second"}, - {"role": "assistant", "content": "reply 2"}, + {"_row_id": 101, "role": "user", "content": "first"}, + {"_row_id": 102, "role": "assistant", "content": "reply 1"}, + {"_row_id": 103, "role": "user", "content": "second"}, + {"_row_id": 104, "role": "assistant", "content": "reply 2"}, ] server._sessions["null-key-trunc-sid"] = _session( history=list(history), session_key=None @@ -5117,6 +5126,7 @@ def test_prompt_submit_truncation_falls_back_to_sid_when_session_key_null(monkey "params": { "session_id": "null-key-trunc-sid", "text": "new turn", + "truncate_before_row_id": 103, "truncate_before_user_ordinal": 1, "confirm_truncate": True, }, @@ -5221,6 +5231,56 @@ def test_prompt_submit_refuses_ordinal_only_when_history_has_row_ids(monkeypatch server._sessions.pop("ordinal-only-durable-sid", None) +def test_prompt_submit_refuses_ordinal_only_when_durable_history_is_unstamped(monkeypatch): + """Durability comes from state.db, not optional stamps on the live copy.""" + replaced = [] + history = [ + {"role": "user", "content": "first"}, + {"role": "assistant", "content": "reply 1"}, + {"role": "user", "content": "second"}, + {"role": "assistant", "content": "reply 2"}, + ] + + class _FakeDB: + def get_messages_as_conversation(self, key, **kwargs): + assert key == "session-key" + assert kwargs["include_row_ids"] is True + return [dict(message, _row_id=100 + index) for index, message in enumerate(history)] + + def replace_messages(self, key, messages, active_only=False, archive_dropped=False): + replaced.append((key, list(messages))) + + server._sessions["unstamped-durable-sid"] = _session(history=list(history)) + monkeypatch.setattr(server, "_get_db", lambda: _FakeDB()) + monkeypatch.setattr( + server, "_start_agent_build", lambda *a, **k: pytest.fail("must not start a turn") + ) + monkeypatch.setattr( + server, "_start_inflight_turn", lambda *a, **k: pytest.fail("must not start a turn") + ) + + try: + resp = server.handle_request( + { + "id": "1", + "method": "prompt.submit", + "params": { + "session_id": "unstamped-durable-sid", + "text": "retry", + "truncate_before_user_ordinal": 1, + "confirm_truncate": True, + }, + } + ) + + assert resp["error"]["code"] == 4004 + assert "truncate_before_row_id" in resp["error"]["message"] + assert server._sessions["unstamped-durable-sid"]["history"] == history + assert replaced == [] + finally: + server._sessions.pop("unstamped-durable-sid", None) + + def test_prompt_submit_truncates_by_row_id(monkeypatch): """#82959: prompt.submit with truncate_before_row_id must cut at the target row id.""" replaced = [] @@ -5450,10 +5510,10 @@ def test_prompt_submit_refuses_empty_truncation_without_confirm(monkeypatch): replaced.append((key, list(messages))) history = [ - {"role": "user", "content": "first"}, - {"role": "assistant", "content": "ok"}, - {"role": "user", "content": "second"}, - {"role": "assistant", "content": "done"}, + {"_row_id": 101, "role": "user", "content": "first"}, + {"_row_id": 102, "role": "assistant", "content": "ok"}, + {"_row_id": 103, "role": "user", "content": "second"}, + {"_row_id": 104, "role": "assistant", "content": "done"}, ] server._sessions["empty-trunc-sid"] = _session(history=list(history)) monkeypatch.setattr(server, "_get_db", lambda: _FakeDB()) @@ -5473,6 +5533,7 @@ def test_prompt_submit_refuses_empty_truncation_without_confirm(monkeypatch): "params": { "session_id": "empty-trunc-sid", "text": "fresh typed message", + "truncate_before_row_id": 101, "truncate_before_user_ordinal": 0, "confirm_truncate": True, }, @@ -5489,6 +5550,7 @@ def test_prompt_submit_refuses_empty_truncation_without_confirm(monkeypatch): "params": { "session_id": "empty-trunc-sid", "text": "fresh typed message", + "truncate_before_row_id": 101, "truncate_before_user_ordinal": 0, "confirm_truncate": True, "confirm_empty_truncate": falsey, @@ -5544,10 +5606,10 @@ def test_prompt_submit_empty_truncation_allowed_with_confirm(monkeypatch): replaced.append((key, list(messages))) history = [ - {"role": "user", "content": "first"}, - {"role": "assistant", "content": "ok"}, - {"role": "user", "content": "second"}, - {"role": "assistant", "content": "done"}, + {"_row_id": 101, "role": "user", "content": "first"}, + {"_row_id": 102, "role": "assistant", "content": "ok"}, + {"_row_id": 103, "role": "user", "content": "second"}, + {"_row_id": 104, "role": "assistant", "content": "done"}, ] server._sessions["confirm-empty-sid"] = _session( agent=_Agent(), history=list(history) @@ -5567,6 +5629,7 @@ def test_prompt_submit_empty_truncation_allowed_with_confirm(monkeypatch): "params": { "session_id": "confirm-empty-sid", "text": "first", + "truncate_before_row_id": 101, "truncate_before_user_ordinal": 0, "confirm_truncate": True, "confirm_empty_truncate": True, @@ -10350,6 +10413,21 @@ def test_session_info_reports_pending_model_switch(monkeypatch): assert server._session_info(agent, session)["model"] == "old/model" +def test_session_info_includes_turn_started_at(): + agent = types.SimpleNamespace(tools=[], model="", provider="") + session = { + "history": [], + "inflight_turn": {"started_at": 1_700_000_123.5}, + "running": True, + } + + assert server._session_info(agent, session)["turn_started_at"] == 1_700_000_123.5 + + session["inflight_turn"] = None + session["running"] = False + assert server._session_info(agent, session)["turn_started_at"] is None + + # --------------------------------------------------------------------------- # History-mutating commands must reject while session.running is True. # Without these guards, prompt.submit's post-run history write either @@ -10884,6 +10962,9 @@ def test_prompt_submit_can_truncate_before_user_ordinal(monkeypatch): def __init__(self): self.replaced = [] + def get_messages_as_conversation(self, *_args, **_kwargs): + return [] + def replace_messages( self, session_id, @@ -10948,6 +11029,9 @@ def test_prompt_submit_refuses_turn_when_truncate_persist_fails(monkeypatch): server._sessions["trunc-fail-sid"] = sess class _FailDb: + def get_messages_as_conversation(self, *_args, **_kwargs): + return [] + def replace_messages( self, session_id, @@ -11047,6 +11131,9 @@ def test_prompt_submit_truncate_ordinal_skips_display_kind_rows(monkeypatch): def __init__(self): self.replaced = [] + def get_messages_as_conversation(self, *_args, **_kwargs): + return [] + def replace_messages( self, session_id, @@ -14122,6 +14209,9 @@ def test_session_activate_returns_inflight_stream_before_completion(monkeypatch) "streaming": True, "user": "write a long answer", } + turn_started_at = resp["result"]["turn_started_at"] + assert turn_started_at == server._sessions["sid-live"]["inflight_turn"]["started_at"] + assert turn_started_at > 0 assert resp["result"]["messages"] == [] release.set() @@ -14134,6 +14224,7 @@ def test_session_activate_returns_inflight_stream_before_completion(monkeypatch) } ) assert completed["result"].get("inflight") is None + assert completed["result"]["turn_started_at"] is None assert completed["result"]["messages"] == [ {"role": "user", "text": "write a long answer"}, {"role": "assistant", "text": "partial answer complete"}, @@ -18320,6 +18411,9 @@ def test_personality_marker_does_not_shift_truncate_ordinal(monkeypatch): def __init__(self): self.replaced = [] + def get_messages_as_conversation(self, *_args, **_kwargs): + return [] + def replace_messages( self, session_id, @@ -18434,6 +18528,9 @@ def test_prompt_submit_truncation_archives_instead_of_deleting(monkeypatch): self._target() class _StubDb: + def get_messages_as_conversation(self, *_args, **_kwargs): + return [] + def replace_messages( self, session_id, diff --git a/tests/tools/test_browser_use_cli.py b/tests/tools/test_browser_use_cli.py index 0f6a197be9..aa1a3c44fb 100644 --- a/tests/tools/test_browser_use_cli.py +++ b/tests/tools/test_browser_use_cli.py @@ -395,20 +395,129 @@ class TestBackendCdpResolution: err = bu_cli._resolve_backend_cdp(self._env(), "t1") assert err and "no" in err.lower() and "CDP" in err - def test_named_session_skips_backend_resolution(self, tmp_path, monkeypatch): - """session= (BU_NAME cloud browser) must not consume a backend - provider session.""" + def test_named_session_composes_with_provider_backend(self, tmp_path, monkeypatch): + """session= composes with a configured provider backend: the + name keys its OWN provider browser (bu-named-), so concurrent + named sessions never share one browser (#86894).""" import tools.browser_tool as bt - def fail(task_id): - raise AssertionError("backend resolution must be skipped") + seen = [] - monkeypatch.setattr(bt, "_get_session_info", fail) - cli = _fake_cli(tmp_path, 'cat > /dev/null\necho "bu:$BU_NAME"\n') + def fake_session_info(key): + seen.append(key) + return {"cdp_url": "wss://browser.example/cdp/" + key} + + monkeypatch.setattr(bt, "_get_cdp_override", lambda: "") + monkeypatch.setattr(bt, "_get_cloud_provider", lambda: object()) + monkeypatch.setattr(bt, "_get_session_info", fake_session_info) + cli = _fake_cli(tmp_path, 'cat > /dev/null\necho "bu:$BU_NAME ws:$BU_CDP_WS"\n') monkeypatch.setattr(bu_cli, "_find_cli", lambda: [cli]) result = json.loads(bu_cli.browser_exec("print(1)", session="r7k2")) assert result["success"] is True + assert seen == ["bu-named-r7k2"] assert "bu:r7k2" in result["output"] + assert "ws:wss://browser.example/cdp/bu-named-r7k2" in result["output"] + + def test_named_session_key_stable_across_tasks(self, monkeypatch): + """The same session name maps to the same provider cache key no + matter which task calls it — that is what lets a follow-up call + reattach to the same cloud browser.""" + import tools.browser_tool as bt + + seen = [] + monkeypatch.setattr(bt, "_get_cdp_override", lambda: "") + monkeypatch.setattr(bt, "_get_cloud_provider", lambda: object()) + monkeypatch.setattr( + bt, "_get_session_info", + lambda key: seen.append(key) or {"cdp_url": "wss://x/cdp/a"}, + ) + env1, env2 = {}, {} + assert bu_cli._resolve_backend_cdp(env1, "task-A", session_name="research") is None + assert bu_cli._resolve_backend_cdp(env2, "task-B", session_name="research") is None + assert seen == ["bu-named-research", "bu-named-research"] + + def test_named_session_direct_api_bu_cloud_still_skips_provider( + self, tmp_path, monkeypatch + ): + """Direct-API Browser Use cloud configs keep the native named-daemon + path: resolving through the provider would double-session and + double-bill.""" + import tools.browser_tool as bt + + class _BUProvider: + name = "browser-use" + + monkeypatch.setattr(bt, "_get_cdp_override", lambda: "") + monkeypatch.setattr(bt, "_get_cloud_provider", lambda: _BUProvider()) + monkeypatch.setattr( + bt, "_get_session_info", + lambda key: (_ for _ in ()).throw(AssertionError("must skip provider")), + ) + monkeypatch.setattr(bu_cli, "_read_browser_cfg", lambda: {"cloud_provider": "browser-use"}) + env = {} + assert bu_cli._resolve_backend_cdp(env, "t1", session_name="r7k2") is None + assert "BU_CDP_WS" not in env and "BU_CDP_URL" not in env + + +class TestOwnTabPreamble: + """Named sessions on SHARED browsers get the own-tab preamble prepended; + private per-name browsers and unnamed sessions do not.""" + + def _run(self, tmp_path, monkeypatch, *, session="", private=False, provider=False): + import tools.browser_tool as bt + + monkeypatch.setattr(bt, "_get_cdp_override", lambda: "") + if provider: + monkeypatch.setattr(bt, "_get_cloud_provider", lambda: object()) + monkeypatch.setattr( + bt, "_get_session_info", + lambda key: {"cdp_url": "wss://browser.example/cdp/" + key}, + ) + else: + monkeypatch.setattr(bt, "_get_cloud_provider", lambda: None) + # fake CLI echoes stdin back so we can inspect what code was sent + cli = _fake_cli(tmp_path, "cat\n") + monkeypatch.setattr(bu_cli, "_find_cli", lambda: [cli]) + return json.loads(bu_cli.browser_exec("print('payload')", session=session)) + + def test_named_shared_browser_gets_preamble(self, tmp_path, monkeypatch): + result = self._run(tmp_path, monkeypatch, session="r7k2") + assert result["success"] is True + assert "_hermes_ensure_own_tab" in result["output"] + # model code still present, after the preamble + assert result["output"].index("_hermes_ensure_own_tab") < result["output"].index("print('payload')") + + def test_unnamed_session_gets_no_preamble(self, tmp_path, monkeypatch): + result = self._run(tmp_path, monkeypatch, session="") + assert result["success"] is True + assert "_hermes_ensure_own_tab" not in result["output"] + + def test_named_provider_browser_skips_preamble(self, tmp_path, monkeypatch): + """Per-name provider browsers are private — preamble would leak a tab.""" + result = self._run(tmp_path, monkeypatch, session="r7k2", provider=True) + assert result["success"] is True + assert "_hermes_ensure_own_tab" not in result["output"] + + def test_sentinel_never_reaches_subprocess_env(self, tmp_path, monkeypatch): + import tools.browser_tool as bt + + monkeypatch.setattr(bt, "_get_cdp_override", lambda: "") + monkeypatch.setattr(bt, "_get_cloud_provider", lambda: object()) + monkeypatch.setattr( + bt, "_get_session_info", + lambda key: {"cdp_url": "wss://browser.example/cdp/" + key}, + ) + cli = _fake_cli(tmp_path, 'cat > /dev/null\necho "sentinel:${_HERMES_BU_PRIVATE_BROWSER:-unset}"\n') + monkeypatch.setattr(bu_cli, "_find_cli", lambda: [cli]) + result = json.loads(bu_cli.browser_exec("print(1)", session="r7k2")) + assert "sentinel:unset" in result["output"] + + def test_preamble_is_valid_python(self): + import ast + + ast.parse(bu_cli._OWN_TAB_PREAMBLE) + # and composes with model code + ast.parse(bu_cli._OWN_TAB_PREAMBLE + "print('x')") class TestProviderPickerIntegration: diff --git a/tests/tui_gateway/test_composite_carrier_rewind.py b/tests/tui_gateway/test_composite_carrier_rewind.py index 6af08745f8..bef256746d 100644 --- a/tests/tui_gateway/test_composite_carrier_rewind.py +++ b/tests/tui_gateway/test_composite_carrier_rewind.py @@ -417,13 +417,16 @@ def test_retry_rejects_pending_attachments_before_mutating_history(carrier_sessi assert session["history"] == before_memory assert len(db.get_messages_as_conversation(session_key)) == 2 -def test_prompt_ordinal_rewind_preserves_scaffold_before_regeneration( +def test_prompt_row_id_rewind_preserves_scaffold_before_regeneration( carrier_session, monkeypatch ): db, install = carrier_session sid, session_key, session = install( [_composite_carrier(), {"role": "assistant", "content": "failed"}] ) + target_row_id = db.get_messages_as_conversation( + session_key, include_row_ids=True + )[0]["_row_id"] seen = {} class _Agent: @@ -463,6 +466,7 @@ def test_prompt_ordinal_rewind_preserves_scaffold_before_regeneration( { "session_id": sid, "text": "EDITED ASK", + "truncate_before_row_id": target_row_id, "truncate_before_user_ordinal": 0, "confirm_truncate": True, }, @@ -476,112 +480,3 @@ def test_prompt_ordinal_rewind_preserves_scaffold_before_regeneration( active = db.get_messages_as_conversation(session_key, include_row_ids=True) assert len(active) == 1 assert active[0]["display_kind"] == "hidden" - - -def test_prompt_row_id_rewind_uses_profile_db_and_rebinds_survivors( - monkeypatch, tmp_path -): - profile_home = tmp_path / "profiles" / "work" - profile_home.mkdir(parents=True) - profile_db = SessionDB(db_path=profile_home / "state.db") - launch_db = SessionDB(db_path=tmp_path / "launch.db") - session_key = "profile-carrier-row-id" - profile_db.create_session(session_key, source="tui") - launch_db.create_session(session_key, source="tui") - launch_db.append_message(session_key, "user", "launch profile must stay untouched") - - carrier = _composite_carrier() - carrier["content"] = carrier["content"].replace( - "REAL ASK", - " REAL ASK\n\nprivate ", - ) - persisted = [ - {"role": "user", "content": "OLDER ASK"}, - {"role": "assistant", "content": "older answer"}, - carrier, - {"role": "assistant", "content": "failed"}, - ] - for message in persisted: - profile_db.append_message( - session_key, - message["role"], - message["content"], - ) - target_row_id = profile_db.get_messages_as_conversation( - session_key, include_row_ids=True - )[2]["_row_id"] - - sid = "profile-carrier-row-id-sid" - session = { - "agent": SimpleNamespace(), - "attached_images": [], - "history": [dict(message) for message in persisted], - "history_lock": threading.Lock(), - "history_version": 0, - "image_counter": 0, - "profile_home": str(profile_home), - "running": False, - "session_key": session_key, - "show_reasoning": False, - "slash_worker": None, - "tool_progress_mode": "all", - } - - class _DormantThread: - def __init__(self, target=None, daemon=None): - self._target = target - - def start(self): - pass - - old_db = server._db - server._db = launch_db - server._sessions[sid] = session - monkeypatch.setattr(server, "_start_agent_build", lambda *_args, **_kwargs: None) - monkeypatch.setattr(server, "_start_inflight_turn", lambda *_args, **_kwargs: None) - monkeypatch.setattr(server.threading, "Thread", _DormantThread) - - try: - response = server._methods["prompt.submit"]( - "request-id", - { - "session_id": sid, - "text": "EDITED ASK", - "truncate_before_row_id": target_row_id, - "truncate_before_user_ordinal": 1, - "confirm_truncate": True, - }, - ) - - assert response["result"]["status"] == "streaming" - survivor_ids = response["result"]["survivor_user_row_ids"] - assert len(survivor_ids) == 1 - assert isinstance(survivor_ids[0], int) - - active = profile_db.get_messages_as_conversation( - session_key, include_row_ids=True - ) - assert [message["content"] for message in active[:2]] == [ - "OLDER ASK", - "older answer", - ] - assert active[0]["_row_id"] == survivor_ids[0] - assert active[2]["display_kind"] == "hidden" - assert "REAL ASK" not in active[2]["content"] - assert "private" not in active[2]["content"] - assert [ - (message["role"], message["content"], message.get("display_kind")) - for message in session["history"] - ] == [ - (message["role"], message["content"], message.get("display_kind")) - for message in active - ] - assert [ - message["content"] - for message in launch_db.get_messages_as_conversation(session_key) - ] == ["launch profile must stay untouched"] - finally: - server._sessions.pop(sid, None) - server._db = old_db - profile_db.close() - launch_db.close() diff --git a/tests/tui_gateway/test_image_ref_message.py b/tests/tui_gateway/test_image_ref_message.py new file mode 100644 index 0000000000..4a1cf95d81 --- /dev/null +++ b/tests/tui_gateway/test_image_ref_message.py @@ -0,0 +1,78 @@ +"""Desktop image submit path must never block on vision calls (#83291). + +The old ``_enrich_with_attached_images`` pre-analyzed every attached image +with the auxiliary vision model *before* the turn was dispatched — serial +blocking calls of 60-90s per large photo, silent failure swallowing, and an +interrupt window that killed the turn with zero API calls. The replacement +``_build_image_ref_message`` only references the image paths so the agent +analyzes them in-loop with ``vision_analyze`` (the same shape as the +``@folder:`` path, which was always fast). +""" + +import time +from unittest.mock import patch + +import pytest + +from tui_gateway.server import _build_image_ref_message + + +@pytest.fixture() +def img(tmp_path): + p = tmp_path / "photo.jpg" + p.write_bytes(b"\xff\xd8\xff fake jpeg") + return p + + +def test_never_calls_vision_on_submit_path(img): + """The whole point of the fix: zero vision traffic before dispatch.""" + + def _boom(*a, **k): # pragma: no cover - must never run + raise AssertionError("submit path called the vision tool") + + with patch("tools.vision_tools.vision_analyze_tool", _boom): + out = _build_image_ref_message("what is this?", [str(img)]) + + assert "what is this?" in out + assert str(img) in out + assert "vision_analyze" in out + + +def test_returns_instantly_for_many_images(tmp_path): + """No per-image network round-trips: 20 images in well under a second.""" + paths = [] + for i in range(20): + p = tmp_path / f"img{i}.png" + p.write_bytes(b"\x89PNG\r\n\x1a\n fake") + paths.append(str(p)) + + start = time.monotonic() + out = _build_image_ref_message("caption", paths) + elapsed = time.monotonic() - start + + assert elapsed < 1.0 + for p in paths: + assert p in out + + +def test_missing_paths_skipped(tmp_path, img): + gone = tmp_path / "nope.png" + out = _build_image_ref_message("hi", [str(gone), str(img)]) + assert str(gone) not in out + assert str(img) in out + + +def test_text_preserved_and_trails_refs(img): + out = _build_image_ref_message("my caption", [str(img)]) + assert out.index("vision_analyze") < out.index("my caption") + + +def test_no_text_defaults_to_question_when_no_valid_images(tmp_path): + out = _build_image_ref_message("", [str(tmp_path / "missing.png")]) + assert out == "What do you see in this image?" + + +def test_no_text_with_image_yields_refs_only(img): + out = _build_image_ref_message("", [str(img)]) + assert str(img) in out + assert out.strip().startswith("[The user attached an image") diff --git a/tests/tui_gateway/test_protocol.py b/tests/tui_gateway/test_protocol.py index b6b265c777..f043780848 100644 --- a/tests/tui_gateway/test_protocol.py +++ b/tests/tui_gateway/test_protocol.py @@ -7,6 +7,7 @@ import threading import time import types from unittest.mock import MagicMock, patch +from pathlib import Path import pytest @@ -564,6 +565,62 @@ def test_session_resume_guard_failure_fails_open(server, monkeypatch): assert reopened == ["transient-guard-session"] +def test_session_resume_active_turn_payload_matches_desktop_fixture(server, monkeypatch): + """A live resume serializes the exact timer payload consumed by Desktop.""" + fixture = json.loads( + (Path(__file__).parents[1] / "fixtures" / "session-resume-active-turn.json").read_text( + encoding="utf-8" + ) + ) + + class _DB: + def get_session(self, session_id): + return {"id": session_id} + + def get_session_by_title(self, _title): + return None + + def resolve_resume_session_id(self, session_id): + return session_id + + active_turn = { + "assistant": "partial answer", + "started_at": fixture["turn_started_at"], + "streaming": True, + "user": "current prompt", + } + server._sessions[fixture["session_id"]] = { + "agent": types.SimpleNamespace(session_id=fixture["session_key"]), + "created_at": fixture["started_at"], + "history": [{"content": "earlier prompt", "role": "user"}], + "history_lock": threading.Lock(), + "inflight_turn": active_turn, + "running": True, + "session_key": fixture["session_key"], + } + monkeypatch.setattr(server, "_get_db", lambda: _DB()) + monkeypatch.setattr(server, "_session_info", lambda _agent: fixture["info"]) + + # JSON round-trip the real RPC envelope: the desktop fixture must stay + # faithful to what the gateway actually serializes, not a copied shape. + response = json.loads( + json.dumps( + server.handle_request( + { + "id": "resume-running", + "method": "session.resume", + "params": {"session_id": fixture["session_key"]}, + } + ) + ) + ) + result = response["result"] + + assert result["running"] is True + assert result["turn_started_at"] == active_turn["started_at"] + assert result == fixture + + def test_enforce_session_cap_evicts_oldest_detached_only(server, monkeypatch): """The LRU cap frees the least-recently-active DETACHED sessions when over the limit, and never a live-transport / running / mid-build one.""" diff --git a/tests/tui_gateway/test_undo_command.py b/tests/tui_gateway/test_undo_command.py index 6a5739b6aa..4472cf8e2b 100644 --- a/tests/tui_gateway/test_undo_command.py +++ b/tests/tui_gateway/test_undo_command.py @@ -106,5 +106,7 @@ def test_undo_returns_prefill_with_target_text(server, session_with_history): # Default /undo backs up one user turn — "question 3" assert result["message"] == "question 3" assert "Undid" in result["notice"] + assert s["history"] + assert all("_row_id" in message for message in s["history"]) diff --git a/tools/browser_use_cli.py b/tools/browser_use_cli.py index 15828edf35..0c99f42c46 100644 --- a/tools/browser_use_cli.py +++ b/tools/browser_use_cli.py @@ -24,6 +24,54 @@ BACKEND_DISABLED = "off" # Cloud daemon names become the BU_NAME env var _SESSION_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9_-]{0,63}$") +# Internal marker set by _resolve_backend_cdp on the env dict when the +# resolved browser is EXCLUSIVE to this named session (per-name provider +# browser, or a named Browser Use cloud browser). Popped before the +# subprocess launches — never exported to the CLI. +_PRIVATE_BROWSER_SENTINEL = "_HERMES_BU_PRIVATE_BROWSER" + +# Preamble prepended to the model's code for named sessions on SHARED +# browsers (local Chrome / CDP override). The harness daemon attaches to the +# first existing page at startup, so two fresh named daemons can land on the +# SAME tab; steering this daemon onto a tab it created keeps concurrent named +# sessions from clobbering each other before their first new_tab(). Runs +# once per daemon (marker file keyed by BU_NAME under the harness runtime +# state), costs one IPC round-trip on later calls. +_OWN_TAB_PREAMBLE = """\ +# hermes: pin this named session to its own tab (once per daemon process) +def _hermes_ensure_own_tab(): + import os as _os, tempfile as _tf + _name = _os.environ.get("BU_NAME", "default") + try: + # Key the marker by the daemon's pid so a daemon restart (which + # re-attaches to the first shared page) re-pins automatically, + # while agent-driven tab switches mid-session are left alone. + from browser_harness import _ipc as _bipc + _dpid = _bipc.pid_path(_name).read_text().strip() or "0" + except Exception: + _dpid = "0" + _uid = _os.getuid() if hasattr(_os, "getuid") else 0 + _marker = _os.path.join( + _tf.gettempdir(), "hermes-bu-owntab-%s-%s-%s" % (_uid, _name, _dpid) + ) + if _os.path.exists(_marker): + return + try: + # Force a fresh target: new_tab() would REUSE a blank current tab, + # which is exactly the tab a sibling daemon may also hold. + _tid = cdp("Target.createTarget", url="about:blank").get("targetId") + if _tid: + switch_tab(_tid) + except Exception: + pass # best-effort: worst case is pre-fix behavior + try: + open(_marker, "w").close() + except OSError: + pass +_hermes_ensure_own_tab() +del _hermes_ensure_own_tab +""" + _DEFAULT_TIMEOUT_S = 300 _MIN_TIMEOUT_S = 5 _MAX_TIMEOUT_S = 1800 @@ -399,7 +447,9 @@ def _native_screenshot_result(result: Dict[str, Any], path: str) -> Optional[Dic return None -def _resolve_backend_cdp(env: dict, task_id: Optional[str]) -> Optional[str]: +def _resolve_backend_cdp( + env: dict, task_id: Optional[str], session_name: str = "" +) -> Optional[str]: """Point the harness at the configured browser backend's CDP endpoint. Resolution order (first hit wins): @@ -416,6 +466,12 @@ def _resolve_backend_cdp(env: dict, task_id: Optional[str]) -> Optional[str]: 4. Nothing configured: return None; the harness attaches to local Chrome (or Browser Use cloud via BU_AUTOSPAWN for legacy configs). + ``session_name`` (the tool's ``session`` argument / BU_NAME) keys the + provider session cache when set, so every distinct name gets its OWN + cloud browser and the same name reuses one — that is what makes named + sessions actually concurrent-safe on provider backends instead of all + names sharing a single per-task browser. + Returns an error string on provider failure, None on success. """ if env.get("BU_CDP_WS") or env.get("BU_CDP_URL"): @@ -457,10 +513,17 @@ def _resolve_backend_cdp(env: dict, task_id: Optional[str]) -> Optional[str]: if provider_key == _BACKEND_KEY and not is_truthy_value( _read_browser_cfg().get("use_gateway"), default=False ): + # Named BU cloud browsers are exclusive to their daemon — no shared + # tab to isolate from. + env[_PRIVATE_BROWSER_SENTINEL] = "1" return None try: - session_info = _get_session_info(task_id or "browser-exec-default") + # Named sessions get their OWN provider browser, keyed by name so the + # same name reuses one browser across calls and tasks, and different + # names never collide. Unnamed calls keep the per-task key. + cache_key = f"bu-named-{session_name}" if session_name else (task_id or "browser-exec-default") + session_info = _get_session_info(cache_key) except Exception as e: return ( f"Cloud browser provider {type(provider).__name__} failed to " @@ -475,6 +538,11 @@ def _resolve_backend_cdp(env: dict, task_id: Optional[str]) -> Optional[str]: "the built-in browser tools for this provider." ) env["BU_CDP_URL" if cdp.startswith(("http://", "https://")) else "BU_CDP_WS"] = cdp + # A provider browser keyed bu-named- is exclusive to this session — + # the own-tab preamble is unnecessary there (it would just leak a blank + # tab into a browser nobody else touches). + if session_name: + env[_PRIVATE_BROWSER_SENTINEL] = "1" return None @@ -511,13 +579,26 @@ def browser_exec( "dashes, or underscores (e.g. 'r7k2')." ) env["BU_NAME"] = session - else: - # Route through the configured browser backend (Browserbase, - # Firecrawl, Nous gateway, CDP override, …). Explicit BU_NAME cloud - # sessions manage their own browser and skip backend resolution. - backend_err = _resolve_backend_cdp(env, task_id) - if backend_err: - return tool_error(backend_err) + # Route through the configured browser backend (Browserbase, Firecrawl, + # Nous gateway, CDP override, local Chrome, …). Named sessions compose + # with the backend: BU_NAME namespaces the harness daemon (its IPC + # socket, log, and pid), and on provider backends the name additionally + # keys its own cloud browser — so concurrent sessions stop clobbering + # each other's daemon (#86894). Browser Use direct-API cloud configs + # are the one exception: the CLI manages named cloud browsers natively, + # and _resolve_backend_cdp skips provider resolution for them. + backend_err = _resolve_backend_cdp(env, task_id, session_name=session) + if backend_err: + return tool_error(backend_err) + + # On a SHARED browser (local Chrome / CDP override) a fresh named daemon + # attaches to the first existing page — the same page a sibling daemon + # may hold. Pin each named session to a tab it created before running + # the model's code. Private per-name browsers (provider-keyed or BU + # cloud) skip this: no one to collide with, and the extra tab would leak. + private_browser = env.pop(_PRIVATE_BROWSER_SENTINEL, None) + if session and not private_browser: + code = _OWN_TAB_PREAMBLE + code workspace = _workspace_dir(task_id) if workspace: @@ -616,8 +697,10 @@ _HEADER_BASE = ( "Batch each sub-procedure (navigate, wait, extract, act) into one call " "— do not spend a call per action — but for long extractions prefer " "several medium calls that append to workspace files over one giant " - "call, so progress survives timeouts. For a named cloud browser, pass " - "session= (never BU_NAME env syntax)." + "call, so progress survives timeouts. For an isolated concurrent " + "browser session (parallel tasks that must not share tabs), pass " + "session= (never BU_NAME env syntax) and reuse the same name on " + "every related call." ) _HEADER_VISION = ( @@ -716,7 +799,7 @@ BROWSER_EXEC_SCHEMA = { }, "session": { "type": "string", - "description": "Named cloud browser session (sets BU_NAME). Omit for the local default daemon. Use the same name you passed to start_remote_daemon().", + "description": "Named isolated browser session (sets BU_NAME): each name gets its own harness daemon — and on cloud backends its own browser — so concurrent tasks don't clobber each other. Omit for the shared default session. Reuse the same name across calls to keep working in that session (and the name passed to start_remote_daemon(), if used).", }, "timeout_s": { "type": "integer", diff --git a/tools/cronjob_tools.py b/tools/cronjob_tools.py index 15638fe0ab..11c94b4772 100644 --- a/tools/cronjob_tools.py +++ b/tools/cronjob_tools.py @@ -953,6 +953,34 @@ def _try_dispatch_background_run( job_id = job["id"] job_name = str(job.get("name") or job_id) + # Reap any execution row this job (or any job) left stranded 'claimed'/ + # 'running' by a dead owner process -- e.g. a PRIOR one-shot `hermes + # cron run` invocation whose dispatched runner died with the exiting + # process before writing a terminal status (issue #86721). The + # long-lived scheduler ticker already does this once at its own + # startup (cron/scheduler.py's self.recover_interrupted()); a one-shot + # CLI invocation has no equivalent "startup" moment of its own, so it + # never got this self-heal -- leaving a permanently-stale claim that + # blocked every subsequent manual run on the same job. Safe and cheap: + # only provably-dead owners (PID gone, or PID reused by a different + # process per its start time) are reaped; a genuinely live owner's row + # is left untouched. + try: + from cron.executions import recover_interrupted_executions + + _reclaimed = recover_interrupted_executions() + if _reclaimed: + logger.warning( + "Reclaimed %d stale cron execution(s) from dead owner(s) " + "before dispatching job '%s'", + _reclaimed, + job_name, + ) + except Exception as _reap_exc: + # Best-effort self-heal; a failure here must not block dispatch — + # but stay diagnosable (mirrors the scheduler tick's reap handling). + logger.debug("Stale execution reclaim failed: %s", _reap_exc) + # ---- routing capture (on THIS thread; contextvars don't cross the pool) ---- # Resolved BEFORE the claim: with no routable session there is no durable # consumer for a detached completion, so we must not claim-and-dispatch. diff --git a/tui_gateway/methods_prompt.py b/tui_gateway/methods_prompt.py index 6c79512a3d..79251dad9c 100644 --- a/tui_gateway/methods_prompt.py +++ b/tui_gateway/methods_prompt.py @@ -85,6 +85,29 @@ def _find_user_turn_by_row_id(history: list, target_row_id: int): return None +def _load_durable_truncation_history(session: dict, fallback_sid: str = ""): + """Load the durable live-replay transcript, or None when it cannot be proven safe.""" + session_key = str(session.get("session_key") or fallback_sid or "") + if not session_key: + return [] + try: + with _session_db(session) as db: + get_conv = getattr(db, "get_messages_as_conversation", None) + if not callable(get_conv): + return None + history = get_conv( + session_key, repair_alternation=True, include_row_ids=True + ) + except Exception: + logger.debug( + "prompt.submit: failed loading durable history for session %s", + session_key, + exc_info=True, + ) + return None + return history if isinstance(history, list) else None + + def _resolve_truncate_row_id(session: dict, history: list, target_row_id: int): """Resolve ``truncate_before_row_id`` to ``(user_ordinal, history_index)``. @@ -98,30 +121,8 @@ def _resolve_truncate_row_id(session: dict, history: list, target_row_id: int): if hit is not None: return hit - session_key = str(session.get("session_key") or "") - if not session_key: - return None - - try: - with _session_db(session) as db: - if db is None: - return None - get_conv = getattr(db, "get_messages_as_conversation", None) - if not callable(get_conv): - return None - db_history = get_conv( - session_key, repair_alternation=True, include_row_ids=True - ) - except Exception: - logger.debug( - "prompt.submit: failed loading DB history for row_id %s session %s", - target_row_id, - session_key, - exc_info=True, - ) - return None - - if not isinstance(db_history, list): + db_history = _load_durable_truncation_history(session) + if db_history is None: return None # Heal missing in-memory stamps when the live list still lines up 1:1 with @@ -505,12 +506,26 @@ def _(rid, params: dict) -> dict: if err is not None: return err else: - # Once active user turns carry durable row ids, an ordinal-only - # target is an unsafe downgrade: renderer and gateway ordinals - # can diverge after compaction/rebuild while the row id remains - # stable. Require the client to prove which durable turn it - # means instead of persisting a potentially mis-aimed cut. - if any(_message_row_id(history[h_idx]) is not None for h_idx in user_indices): + if client_ordinal < 0 or client_ordinal >= len(user_indices): + return _err( + rid, 4018, "target user message is no longer in session history" + ) + # Durability is a state.db property, not an optional annotation + # on the live copy. Resume/reload paths historically omitted + # _row_id stamps, which made an ordinal-only request look safe + # even though it could destructively replace a long transcript. + # If the durable state cannot be read, fail closed too: absence + # of proof is not proof that this is an ephemeral conversation. + has_stamped_user = any( + _message_row_id(history[h_idx]) is not None + for h_idx in user_indices + ) + durable_history = ( + [] + if has_stamped_user + else _load_durable_truncation_history(session, sid) + ) + if has_stamped_user or durable_history is None or durable_history: logger.warning( "prompt.submit: REFUSED ordinal-only truncation of durable " "session %s (ordinal=%d); truncate_before_row_id required", @@ -579,50 +594,49 @@ def _(rid, params: dict) -> dict: # new exchange is appended on top of the "undone" turns — durable # zombie history on resume, and the edit/regenerate never sticks. # Fail closed: refuse the turn and leave memory/DB unchanged. - try: - with _session_db(session) as db: - if db is not None: - # active_only=True: replace only the live (active=1) rows. - # In-place compaction (#38763) keeps the pre-compaction - # transcript as active=0/compacted=1 rows under this same - # session key; a bare replace_messages() would DELETE that - # durable archive on every edit/regenerate — the same bug - # class #80216 fixed for /retry. On an uncompacted session - # all rows are active=1, so this is behaviorally identical - # to the full replace. - # archive_dropped: a rewind overwrites turns the user may - # not have meant to drop, and this write is the last step - # before they are gone — three reported incidents ended - # here with nothing to restore from (#70516, #80763, - # #82756). Soft-archiving keeps them on disk (active=0) and - # in the FTS index, so a mis-aimed cut is recoverable - # instead of terminal. The live transcript is unchanged. - # Fall back to session id when session_key is NULL — CLI-origin - # sessions created before the session_key default fix have no - # key, and replace_messages(None) triggers an FK violation. - truncation_key = session.get("session_key") or sid - db.replace_messages( - truncation_key, - truncated, - active_only=True, - archive_dropped=True, - reject_active_turn_lease=True, - ) - except Exception as exc: - logger.error( - "prompt.submit: replace_messages failed for session %s " - "(ordinal=%d); refusing turn so memory and DB stay " - "aligned: %s", - sid, - ordinal, - exc, - exc_info=True, - ) - return _err( - rid, - 5008, - f"failed to persist history truncation: {exc}", - ) + if (db := _get_db()) is not None: + try: + # active_only=True: replace only the live (active=1) rows. + # In-place compaction (#38763) keeps the pre-compaction + # transcript as active=0/compacted=1 rows under this same + # session key; a bare replace_messages() would DELETE that + # durable archive on every edit/regenerate — the same bug + # class #80216 fixed for /retry. On an uncompacted session + # all rows are active=1, so this is behaviorally identical + # to the full replace. + # archive_dropped: a rewind overwrites turns the user may + # not have meant to drop, and this write is the last step + # before they are gone — three reported incidents ended + # here with nothing to restore from (#70516, #80763, + # #82756). Soft-archiving keeps them on disk (active=0) and + # in the FTS index, so a mis-aimed cut is recoverable + # instead of terminal. The live transcript is unchanged. + # Fall back to session id when session_key is NULL — CLI-origin + # sessions created before the session_key default fix have no + # key, and replace_messages(None) triggers an FK violation. + truncation_key = session.get("session_key") or sid + db.replace_messages( + truncation_key, + truncated, + active_only=True, + archive_dropped=True, + reject_active_turn_lease=True, + ) + except Exception as exc: + logger.error( + "prompt.submit: replace_messages failed for session %s " + "(ordinal=%d); refusing turn so memory and DB stay " + "aligned: %s", + sid, + ordinal, + exc, + exc_info=True, + ) + return _err( + rid, + 5008, + f"failed to persist history truncation: {exc}", + ) session["history"] = truncated session["history_version"] = int(session.get("history_version", 0)) + 1 if db is not None: @@ -1441,6 +1455,7 @@ def register(server) -> None: _message_row_id, _mem_db_pair_agrees, _find_user_turn_by_row_id, + _load_durable_truncation_history, _resolve_truncate_row_id, _coerce_truncate_int, _reconcile_client_ordinal, diff --git a/tui_gateway/methods_session.py b/tui_gateway/methods_session.py index 48c289e351..c652e22f54 100644 --- a/tui_gateway/methods_session.py +++ b/tui_gateway/methods_session.py @@ -541,7 +541,7 @@ def _(rid, params: dict) -> dict: # inspection/export must show what is actually stored. if omit_messages: raw_history = db.get_messages_as_conversation( - target, repair_alternation=True + target, repair_alternation=True, include_row_ids=True ) display_history = [] else: @@ -627,7 +627,7 @@ def _(rid, params: dict) -> dict: # display copy stays verbatim. if omit_messages: raw_history = db.get_messages_as_conversation( - target, repair_alternation=True + target, repair_alternation=True, include_row_ids=True ) display_history = [] else: diff --git a/tui_gateway/server.py b/tui_gateway/server.py index eb4b3c353c..31f77d8e69 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -3185,6 +3185,21 @@ def _rewind_active_session_history( durable_prefix[-1]["_row_id"] = replacement_id durable_prefix[-1]["_db_persisted"] = True installed[-1] = durable_prefix[-1] + # Current clients address destructive follow-ups by durable row id. + # Preserve the richer warm content (for example image parts), but + # copy row identities when the retained warm/durable shapes align. + if len(installed) == len(durable_prefix) and all( + warm.get("role") == durable_message.get("role") + and bool(warm.get("display_kind")) + == bool(durable_message.get("display_kind")) + and _comparison_content(warm) + == _comparison_content(durable_message) + for warm, durable_message in zip(installed, durable_prefix) + ): + for warm, durable_message in zip(installed, durable_prefix): + row_id = durable_message.get("_row_id") + if isinstance(row_id, int): + warm["_row_id"] = row_id live_view = durable_live_view rewound_count = int(result.get("rewound_count", 0)) persisted = True @@ -5584,6 +5599,16 @@ def _session_info(agent, session: dict | None = None) -> dict: pending_switch = (session or {}).get("pending_model_switch") or {} pending_model = str(pending_switch.get("display_model") or "").strip() pending_provider = str(pending_switch.get("display_provider") or "").strip() + # Epoch seconds the current turn started, or None when idle. Lets the + # desktop preserve the turn-elapsed timer across session switches (cold + # resume path) instead of resetting it to 0:00. + inflight = (session or {}).get("inflight_turn") + turn_started_at = ( + float(inflight["started_at"]) + if isinstance(inflight, dict) and inflight.get("started_at") + else None + ) + info: dict = { "model": pending_model or mirror.get("model", getattr(agent, "model", "")), "provider": pending_provider @@ -5601,6 +5626,7 @@ def _session_info(agent, session: dict | None = None) -> dict: "terminal_backend": _effective_terminal_backend(), "personality": str(personality or ""), "running": bool((session or {}).get("running")), + "turn_started_at": turn_started_at, "title": _session_live_title(session or {}, session_key) if session_key else "", "stored_session_id": session_key or "", "desktop_contract": DESKTOP_BACKEND_CONTRACT, @@ -7183,35 +7209,33 @@ def _active_image_routing_identity(agent: Any) -> tuple[str, str]: ) -def _enrich_with_attached_images(user_text: str, image_paths: list[str]) -> str: - """Pre-analyze attached images via vision and prepend descriptions to user text.""" - import asyncio, json as _json - from tools.vision_tools import vision_analyze_tool +def _build_image_ref_message(user_text: str, image_paths: list[str]) -> str: + """Reference attached images by path so the agent analyzes them in-loop. - prompt = ( - "Describe everything visible in this image in thorough detail. " - "Include any text, code, data, objects, people, layout, colors, " - "and any other notable visual information." - ) + This used to pre-analyze every image with the auxiliary vision model + *before* the turn was dispatched (``_enrich_with_attached_images``): + serial blocking calls on the submit path — 60-90s per large photo — + with failures silently swallowed and an interrupt during the window + killing the turn with zero API calls (#83291). It also prepended the + vision description to the first user message, poisoning session + auto-titles (#82339). The CLI never gates turn dispatch on vision + like this, which is why the same message was seconds there and + minutes on desktop. + Now the turn starts immediately. The agent examines each image itself + with ``vision_analyze`` — its own retries, visible tool progress — + exactly how the ``@folder:`` reference path already behaves, which + responds in seconds for the same images. + """ parts: list[str] = [] for path in image_paths: p = Path(path) if not p.exists(): continue - hint = f"[You can examine it with vision_analyze using image_url: {p}]" - try: - r = _json.loads( - asyncio.run(vision_analyze_tool(image_url=str(p), user_prompt=prompt)) - ) - desc = r.get("analysis", "") if r.get("success") else None - parts.append( - f"[The user attached an image:\n{desc}]\n{hint}" - if desc - else f"[The user attached an image but analysis failed.]\n{hint}" - ) - except Exception: - parts.append(f"[The user attached an image but analysis failed.]\n{hint}") + parts.append( + f"[The user attached an image: {p.name}]\n" + f"[Examine it with the vision_analyze tool using image_url: {p}]" + ) text = user_text or "" prefix = "\n\n".join(parts) @@ -7225,8 +7249,8 @@ def _build_persist_message_with_image_refs(user_text: str, image_paths: list[str persisting to session history. Uses ``@image:`` directives — the format the desktop client (directive-text.tsx / HERMES_DIRECTIVE_RE) actually parses and renders as an image — unlike - ``_enrich_with_attached_images``, which embeds a vision description and - an ``image_url:`` hint meant only for the model and must never be + ``_build_image_ref_message``, which embeds an + ``image_url:`` hint meant only for the model and must never be persisted as-is (it silently breaks image rendering after a full restart, and reorders image/text on live session-switch reconciliation). @@ -8702,6 +8726,12 @@ def _live_session_payload( inflight = _inflight_snapshot(session) queued = _queued_prompt_snapshot(session) running = bool(session.get("running")) + inflight_turn = session.get("inflight_turn") + turn_started_at = ( + float(inflight_turn["started_at"]) + if isinstance(inflight_turn, dict) and inflight_turn.get("started_at") + else None + ) # Prefer the persisted display lineage (candidate-inclusive) so this payload # matches the eager session.resume + REST transcript. Use the session's # profile-aware DB (not launch ``_get_db()``): app-global remote profile @@ -8721,6 +8751,7 @@ def _live_session_payload( "messages": [] if omit_messages else _history_to_messages(history), "messages_omitted": omit_messages, "running": running, + "turn_started_at": turn_started_at, "session_id": sid, "session_key": _session_lookup_key(session, fallback=sid), "started_at": float(session.get("created_at") or time.time()), @@ -10588,7 +10619,9 @@ def _run_prompt_submit( # Decide image routing per-turn based on active provider/model. # "native" → pass pixels to the main model as OpenAI-style content # parts (adapters translate for Anthropic/Gemini/Bedrock/etc.). - # "text" → pre-analyze with vision_analyze and prepend the text. + # "text" → reference the image paths in the message so the agent + # analyzes them in-loop with vision_analyze (never + # blocking the submit path on vision calls — #83291). # See agent/image_routing.py for the full decision table. run_message: Any = prompt if images: @@ -10632,15 +10665,15 @@ def _run_prompt_submit( if any(p.get("type") == "image_url" for p in _parts): run_message = _parts else: - run_message = _enrich_with_attached_images(prompt, images) + run_message = _build_image_ref_message(prompt, images) except Exception as _img_exc: print( f"[tui_gateway] native attach failed, falling back to text: {_img_exc}", file=sys.stderr, ) - run_message = _enrich_with_attached_images(prompt, images) + run_message = _build_image_ref_message(prompt, images) else: - run_message = _enrich_with_attached_images(prompt, images) + run_message = _build_image_ref_message(prompt, images) # Streaming TTS: voice-mode replies are spoken sentence-by-sentence # as tokens arrive (CLI parity) instead of after the full turn. diff --git a/website/docs/integrations/providers.md b/website/docs/integrations/providers.md index 549c93049f..6137f8f559 100644 --- a/website/docs/integrations/providers.md +++ b/website/docs/integrations/providers.md @@ -1271,7 +1271,27 @@ providers: transport: anthropic_messages # for Anthropic-compatible proxies ``` -Each entry accepts: `api` (the endpoint base URL — `base_url`/`url` are accepted aliases), `name` (optional display name; defaults to the dict key), `key_env` or inline `api_key`, `transport` (`chat_completions` / `anthropic_messages` / `codex_responses`), `default_model`, `models`, `context_length`, `discover_models`, `extra_body`, `extra_headers`, `ssl_ca_cert` / `ssl_verify`, and `enabled: false` to hide an entry without deleting it. +Each entry accepts: `api` (the endpoint base URL — `base_url`/`url` are accepted aliases), `name` (optional display name; defaults to the dict key), `key_env` or inline `api_key` or `key_cmd` (see below), `transport` (`chat_completions` / `anthropic_messages` / `codex_responses`), `default_model`, `models`, `context_length`, `discover_models`, `extra_body`, `extra_headers`, `ssl_ca_cert` / `ssl_verify`, and `enabled: false` to hide an entry without deleting it. + +#### Command-minted credentials (`key_cmd`) + +Enterprise gateways often issue short-lived bearer tokens (SSO/OIDC brokers, cloud IAM, internal auth proxies) rather than static API keys, so a token copied into `.env` goes stale mid-session and requests start returning 401. `key_cmd` names a command that *prints* a token; Hermes runs it and caches the result until shortly before expiry, so long sessions keep working with no restart: + +```yaml +providers: + my-gateway: + base_url: "https://gateway.internal.example.com/v1" + api_mode: chat_completions + key_cmd: "my-auth-cli print-token --profile prod" +``` + +Works with any helper that prints a token — `databricks auth token`, `gcloud auth print-access-token`, `az account get-access-token`, `vault read`, or Claude Code-style `apiKeyHelper` scripts. + +The command must print **only** the token on stdout: either bare, or as JSON with an `access_token` field (`expires_in` is honored; absolute `expiry`/`expiresOn` ISO timestamps too). Multi-line output is rejected rather than guessed at. If no expiry is advertised, the token is re-minted on a bounded window. + +Precedence: an explicit `--api-key` flag still wins; otherwise `key_cmd` beats a static `api_key`/`key_env` on the same entry. The minted credential applies to the main agent turn and to auxiliary tasks (title generation, compression, vision, embedding) alike. + +Not to be confused with `secrets.command`, which runs a helper **once at startup** to populate env vars process-wide. Use that for a vault/keychain helper handing back many secrets; use `key_cmd` when one provider's credential must be re-minted *during* a session. :::note Legacy format Older configs used a top-level `custom_providers:` list instead. It still works — Hermes reads both — and `hermes update` auto-migrates it to the `providers:` dict (config v12). Field names differ slightly in the dict format: legacy `model` is `default_model`, and legacy `api_mode` is `transport`. diff --git a/website/docs/user-guide/desktop.md b/website/docs/user-guide/desktop.md index a881c806c4..68de972a12 100644 --- a/website/docs/user-guide/desktop.md +++ b/website/docs/user-guide/desktop.md @@ -230,7 +230,7 @@ Connection modes are configured **per profile** — a per-profile override can p ### Settings → Connections: the multi-connection registry -Alongside the per-profile connection mode above, **Settings → Connections** manages a named registry of every agent source the app knows about — the local runtime, any number of remote gateways (LAN, Tailscale, internet), Hermes Cloud instances, and SSH hosts — all persisted together in one place. +Alongside the per-profile connection mode above, **Settings → Connections** manages a named registry of every agent source the app knows about — the local runtime, any number of remote gateways (LAN, Tailscale, internet), Hermes Cloud instances, and SSH hosts — all persisted together in one place. The full guide, including the union agent roster, `@name-device` handles, fleet-wide updates, and the plugin SDK surface, is at [Connecting Desktop to Many Hermes Instances](./multi-connection-desktop.md). - **Every connection needs a unique name** (a device name such as "Homelab" or "Work laptop"). When the same profile name exists on several registered sources, surfaces disambiguate it as `@profile-device` (e.g. `@research-homelab`). - **Add / edit / remove / test** connections from the panel. The local entry is managed by the app and cannot be removed. **Test** probes the connection's own HTTP and WebSocket legs directly. @@ -238,7 +238,7 @@ Alongside the per-profile connection mode above, **Settings → Connections** ma - Cloud entries come from the Hermes Cloud sign-in/discovery flow above, not from a hand-typed URL. - Tokens are stored encrypted with the OS keyring (with the same explicit plain-text opt-in as Settings → Gateway on keyring-less Linux). -Side-by-side routing is rolling out in stages: connections are managed here today, while the active connection is still chosen in **Settings → Gateway**. Follow-up releases route chats, the agent roster, and updates across all registered sources. +Side-by-side routing is live: each registered source dials its own backends and sockets on demand (keyed per connection + profile), the plugin SDK exposes the union agent roster (`host.agents()` / `host.ensureAgent()`), and **Update all instances** in the Connections panel dispatches `hermes update` to every eligible source at once — Hermes Cloud entries are skipped (the platform updates them), and each instance reports its own result. :::info The remote backend is a running `hermes serve` process diff --git a/website/docs/user-guide/features/browser.md b/website/docs/user-guide/features/browser.md index 9b064a6bc0..99fb0e84c2 100644 --- a/website/docs/user-guide/features/browser.md +++ b/website/docs/user-guide/features/browser.md @@ -70,6 +70,8 @@ Browser Use mode uses the [Browser Use CLI 3.0](https://github.com/browser-use/b The mode is a **driver** that composes with your configured browser backend: it drives your local Chrome, a Nous-subscription cloud browser, Browserbase, Firecrawl, or Browser Use cloud browsers — whichever browser source is selected in `hermes tools` → Browser Automation. The one exception is Camofox, which has no CDP endpoint for the harness to attach to; Camofox setups automatically keep the built-in browser tools. +**Concurrent sessions:** `browser_exec` accepts a `session=` argument that isolates browser work per name on every backend. Each name gets its own harness daemon (its own IPC socket, log, and state), and on cloud backends its own browser — so parallel subagents or simultaneous chats no longer clobber a single shared connection. Omitting `session` uses the shared default daemon, which is fine for one-at-a-time browsing. + To opt out and force the built-in browser tools, use `/browser use off`, or: ```yaml diff --git a/website/docs/user-guide/multi-connection-desktop.md b/website/docs/user-guide/multi-connection-desktop.md new file mode 100644 index 0000000000..8af9f9ba9f --- /dev/null +++ b/website/docs/user-guide/multi-connection-desktop.md @@ -0,0 +1,119 @@ +--- +sidebar_position: 5 +--- + +# Connecting Desktop to Many Hermes Instances + +Register every Hermes backend you own — the local runtime, remote gateways on +your LAN or VPS, SSH hosts, and Hermes Cloud instances — in one desktop app, +and use the agents on all of them side by side. + +This is the desktop-side complement to +[Running Many Gateways at Once](./multi-profile-gateways.md): that page is +about hosting several gateways on one machine; this one is about one desktop +app talking to several machines. + +## The connection registry + +**Settings → Connections** manages a named registry of agent sources. Each +entry is a *connection*: + +| Kind | What it is | Auth | +|---|---|---| +| **Local** | The runtime this app manages on your machine | automatic | +| **Remote gateway** | A `hermes serve` backend reachable over HTTP(S) — LAN, Tailscale, VPS | session token or OAuth | +| **SSH** | A Hermes install reached over SSH; the app opens the tunnel and starts the dashboard for you | SSH key + adopted token | +| **Hermes Cloud** | A hosted instance discovered through your Nous account | portal sign-in | + +Rules worth knowing: + +- **Every connection needs a unique device name** ("Homelab", "Work laptop"). + The name shows up everywhere the instance appears — roster badges, handles, + update results. +- The **local** entry is managed by the app and cannot be removed. Removing + any other connection tears down its live backends and tunnels; the instance + itself is untouched. +- **Test** probes the connection's own HTTP *and* WebSocket legs, so a pass + means chat will actually work — not just that the host pinged. +- Cloud entries come from the Hermes Cloud sign-in/discovery flow, not a + hand-typed URL. +- Tokens are encrypted with the OS keyring (same plain-text opt-in as + Settings → Gateway on keyring-less Linux), and never leave the Electron + main process. + +### Migrating from the single-connection settings + +The first launch of a registry-capable build imports your existing settings +automatically: the global connection mode and any per-profile overrides from +Settings → Gateway become named registry entries (deduplicated by URL/host). +The legacy settings file is left untouched, so older builds on the same +machine keep working. + +## Agents across sources + +Every profile on every registered connection is an *agent*. The union roster +is what multi-source surfaces (and plugins like +[Bot Mode](https://github.com/NousResearch/Hermes-Bot-Mode)) render: + +- When the same profile name exists on several sources, handles disambiguate + as **`@name-device`** — `research` on your Homelab renders as + `@research-homelab`, while a profile unique across all sources keeps its + bare name. +- Enumeration is eager but sockets are lazy: the app lists agents over REST + without dialing every source's WebSocket. An unreachable source reports + per-row instead of breaking the roster; SSH sources stay connect-on-demand + until you first open an agent on them (no surprise tunnels). +- Opening an agent dials **its own source** — chats, sessions, and memory + live on the machine that owns the profile, exactly as if you were using + that instance directly. + +Each `(connection, profile)` pair gets its own backend and socket, pooled +with the same idle-reaping as local per-profile backends — background agents +keep streaming while you look at another source. + +## Updating every instance at once + +**Settings → Connections → Update all instances** dispatches `hermes update` +to every eligible connection in parallel: + +- **Local** updates through the app's own update pipeline (the same flow as + Settings → Updates). +- **Remote and SSH** connections are told to update themselves via their own + backend — the update runs on *that* machine. +- **Hermes Cloud** instances are skipped: the platform manages their + versions. + +Each instance reports independently, so one unreachable box never wedges the +batch. Backends that manage updates externally (Docker, Nix) refuse politely +with their own message, per row. + +## For plugin authors + +The Desktop [plugin SDK](../developer-guide/desktop-plugin-sdk.md) exposes the +multi-source surface directly: + +- `host.connections()` — the registered connection list (labels, kinds, + primary; never token bytes). +- `host.agents()` — the union roster: one row per `(source, profile)` with + the precomputed `@name-device` handle. +- `host.ensureAgent(connectionId, profile)` — activate an agent's gateway so + subsequent `host.request` calls hit its backend. +- `host.warmAgent(connectionId, profile)` — fire-and-forget socket pre-warm + (hover-intent). + +All four are feature-detected: on an older Desktop build they're absent and a +plugin should fall back to the single-source `profiles.list` flow. Bot Mode's +multi-source roster is the reference consumer. + +## Troubleshooting + +- **An agent shows but won't open** — run **Test** on its connection. The + WebSocket leg failing while HTTP passes usually means a proxy, firewall, or + gateway auth/origin guard is blocking `/api/ws`. +- **A remote source is missing from the roster** — its backend is down or + unreachable; the roster lists it under sources with the error. SSH sources + show *connect-on-demand* until first use — that's by design, not a failure. +- **"Update Hermes Desktop to chat with agents on other connections"** — the + app predates the multi-connection stack; update the desktop app itself. +- **Duplicate device names** — not possible; names are enforced unique at + save time. If a migrated name collided, it was suffixed (`Homelab 2`). diff --git a/website/docs/user-guide/multi-profile-gateways.md b/website/docs/user-guide/multi-profile-gateways.md index b73d184595..7feddd069a 100644 --- a/website/docs/user-guide/multi-profile-gateways.md +++ b/website/docs/user-guide/multi-profile-gateways.md @@ -11,7 +11,9 @@ across profiles, preventing the host from sleeping, and recovering from common launchd/systemd quirks. If you only run one Hermes agent, you don't need this page — see -[Profiles](./profiles.md) for the basics. +[Profiles](./profiles.md) for the basics. And if your instances live on +*different* machines that one desktop app should reach simultaneously, see +[Connecting Desktop to Many Hermes Instances](./multi-connection-desktop.md). ## When to use this diff --git a/website/sidebars.ts b/website/sidebars.ts index 0839f3bbe6..d857d33ed6 100644 --- a/website/sidebars.ts +++ b/website/sidebars.ts @@ -53,6 +53,7 @@ const sidebars: SidebarsConfig = { 'user-guide/profiles', 'user-guide/profile-distributions', 'user-guide/multi-profile-gateways', + 'user-guide/multi-connection-desktop', 'user-guide/git-worktrees', 'user-guide/docker', 'user-guide/security',