From 42627b4eafbe632fb663238b7f595f1cdb67acd4 Mon Sep 17 00:00:00 2001 From: ethernet Date: Thu, 30 Apr 2026 13:14:19 -0400 Subject: [PATCH 01/12] refactor(tui): bundle with esbuild, drop runtime node_modules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the tsc + babel pipeline with a single esbuild invocation that produces a self-contained dist/entry.js. The nix TUI derivation no longer copies node_modules — only dist/ + package.json ship, shrinking the output from hundreds of MB to ~2.9 MB. - ui-tui/scripts/build.mjs: new esbuild bundler. Aliases @hermes/ink to source (esbuild's __esm helper doesn't await nested async init, which breaks lazy-assigned exports like 'render' when re-exporting through a prebuilt submodule). Stubs react-devtools-core (dev-only). Injects a createRequire shim for transitive CJS deps. Strips the shebang from src/entry.tsx because Nix patchShebangs mangles '/usr/bin/env -S node --max-old-space-size=8192 --expose-gc' — it drops the 'node' token. The Python launcher always invokes node explicitly, so the shebang is redundant. - nix/tui.nix: installPhase no longer copies node_modules or the @hermes/ink packages dir. - nix/checks.nix: drop the 'node_modules present' assertion. - hermes_cli/main.py: _tui_need_npm_install short-circuits when dist/entry.js exists and no package-lock.json is present. That is the prebuilt-bundle layout (nix / packaged release) and there is nothing to install. Without this, the launcher tried to npm install in a non-existent site-packages/ui-tui path. --- hermes_cli/main.py | 13 ++++- nix/checks.nix | 3 +- nix/tui.nix | 11 ++--- tests/hermes_cli/test_tui_npm_install.py | 6 +++ ui-tui/package-lock.json | 1 + ui-tui/package.json | 4 +- ui-tui/scripts/build.mjs | 61 ++++++++++++++++++++++++ 7 files changed, 86 insertions(+), 13 deletions(-) create mode 100644 ui-tui/scripts/build.mjs diff --git a/hermes_cli/main.py b/hermes_cli/main.py index bdbf0390a6..7b5834aea6 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -841,6 +841,11 @@ _NPM_LOCK_RUNTIME_KEYS = frozenset({"ideallyInert"}) def _tui_need_npm_install(root: Path) -> bool: """True when @hermes/ink is missing or node_modules is behind package-lock.json. + Prebuilt bundle mode: when ``dist/entry.js`` exists and there is no + ``package-lock.json`` (nix install layout only ships ``dist/`` + + ``package.json``), skip reinstall entirely — the bundle is self-contained + and there is nothing to install. + Compares ``package-lock.json`` against ``node_modules/.package-lock.json`` (npm's hidden lockfile) by **content**, not mtime: git checkouts and npm rewrites can bump the root lockfile's timestamp even when installed deps @@ -858,10 +863,16 @@ def _tui_need_npm_install(root: Path) -> bool: we'd rather not force a reinstall for them. Falls back to mtime comparison if either lockfile is unparseable. """ + lock = root / "package-lock.json" + entry = root / "dist" / "entry.js" + # Prebuilt self-contained bundle (nix / packaged release): no lockfile + # shipped, dist/entry.js is the single runtime artefact. + if entry.is_file() and not lock.is_file(): + return False + ink = root / "node_modules" / "@hermes" / "ink" / "package.json" if not ink.is_file(): return True - lock = root / "package-lock.json" if not lock.is_file(): return False marker = root / "node_modules" / ".package-lock.json" diff --git a/nix/checks.nix b/nix/checks.nix index 8adb56628d..269699eef6 100644 --- a/nix/checks.nix +++ b/nix/checks.nix @@ -154,8 +154,7 @@ json.dump(sorted(leaf_paths(DEFAULT_CONFIG)), sys.stdout, indent=2) test -f ${hermes-agent}/ui-tui/dist/entry.js || (echo "FAIL: compiled entry.js missing"; exit 1) echo "PASS: compiled entry.js present" - test -d ${hermes-agent}/ui-tui/node_modules || (echo "FAIL: node_modules missing"; exit 1) - echo "PASS: node_modules present" + # self-contained bundle; no runtime node_modules expected grep -q "HERMES_TUI_DIR" ${hermes-agent}/bin/hermes || \ (echo "FAIL: HERMES_TUI_DIR not in wrapper"; exit 1) diff --git a/nix/tui.nix b/nix/tui.nix index 7453fa2673..eb25d52f90 100644 --- a/nix/tui.nix +++ b/nix/tui.nix @@ -24,16 +24,11 @@ pkgs.buildNpmPackage (npm // { mkdir -p $out/lib/hermes-tui + # Single self-contained bundle built by scripts/build.mjs (esbuild). + # No runtime node_modules needed. cp -r dist $out/lib/hermes-tui/dist - # runtime node_modules - cp -r node_modules $out/lib/hermes-tui/node_modules - - # @hermes/ink is a file: dependency, we need to copy it in fr - rm -f $out/lib/hermes-tui/node_modules/@hermes/ink - cp -r packages/hermes-ink $out/lib/hermes-tui/node_modules/@hermes/ink - - # package.json needed for "type": "module" resolution + # package.json kept for "type": "module" resolution on `node dist/entry.js`. cp package.json $out/lib/hermes-tui/ runHook postInstall diff --git a/tests/hermes_cli/test_tui_npm_install.py b/tests/hermes_cli/test_tui_npm_install.py index e56196e07e..8e16c5e9de 100644 --- a/tests/hermes_cli/test_tui_npm_install.py +++ b/tests/hermes_cli/test_tui_npm_install.py @@ -89,6 +89,12 @@ def test_no_install_without_lockfile_when_ink_present(tmp_path: Path, main_mod) assert main_mod._tui_need_npm_install(tmp_path) is False +def test_no_install_prebuilt_bundle_mode(tmp_path: Path, main_mod) -> None: + """dist/entry.js present and no package-lock.json → prebuilt bundle, skip npm install.""" + _touch_tui_entry(tmp_path) + assert main_mod._tui_need_npm_install(tmp_path) is False + + def test_build_needed_when_local_ink_bundle_missing(tmp_path: Path, main_mod) -> None: _touch_tui_entry(tmp_path) _touch_ink(tmp_path) diff --git a/ui-tui/package-lock.json b/ui-tui/package-lock.json index 017e9913bd..c6d1e6be49 100644 --- a/ui-tui/package-lock.json +++ b/ui-tui/package-lock.json @@ -25,6 +25,7 @@ "@typescript-eslint/eslint-plugin": "^8", "@typescript-eslint/parser": "^8", "babel-plugin-react-compiler": "^1.0.0", + "esbuild": "~0.27.0", "eslint": "^9", "eslint-plugin-perfectionist": "^5", "eslint-plugin-react": "^7", diff --git a/ui-tui/package.json b/ui-tui/package.json index 061e3bc448..1edee8cabf 100644 --- a/ui-tui/package.json +++ b/ui-tui/package.json @@ -6,8 +6,7 @@ "scripts": { "dev": "npm run build --prefix packages/hermes-ink && tsx --watch src/entry.tsx", "start": "tsx src/entry.tsx", - "build": "npm run build --prefix packages/hermes-ink && tsc -p tsconfig.build.json && npm run build:compile && chmod +x dist/entry.js", - "build:compile": "babel dist --out-dir dist --config-file ./babel.compiler.config.cjs --extensions .js --keep-file-extension", + "build": "node scripts/build.mjs", "type-check": "tsc --noEmit -p tsconfig.json", "lint": "eslint src/ packages/", "lint:fix": "eslint src/ packages/ --fix", @@ -34,6 +33,7 @@ "@typescript-eslint/eslint-plugin": "^8", "@typescript-eslint/parser": "^8", "babel-plugin-react-compiler": "^1.0.0", + "esbuild": "~0.27.0", "eslint": "^9", "eslint-plugin-perfectionist": "^5", "eslint-plugin-react": "^7", diff --git a/ui-tui/scripts/build.mjs b/ui-tui/scripts/build.mjs new file mode 100644 index 0000000000..2c7b55f76f --- /dev/null +++ b/ui-tui/scripts/build.mjs @@ -0,0 +1,61 @@ +#!/usr/bin/env node +// Bundles src/entry.tsx into a single self-contained dist/entry.js. +// No runtime node_modules needed. +import { build } from 'esbuild' +import { readFileSync, writeFileSync } from 'node:fs' +import { fileURLToPath } from 'node:url' +import { dirname, resolve } from 'node:path' + +const here = dirname(fileURLToPath(import.meta.url)) +const root = resolve(here, '..') +const out = resolve(root, 'dist/entry.js') + +// `react-devtools-core` is only imported when DEV=true at runtime (Ink dev +// mode). Stub it out so the bundle doesn't carry the dep. +const stubDevtools = { + name: 'stub-react-devtools-core', + setup(b) { + b.onResolve({ filter: /^react-devtools-core$/ }, args => ({ + path: args.path, + namespace: 'stub-devtools' + })) + b.onLoad({ filter: /.*/, namespace: 'stub-devtools' }, () => ({ + contents: 'export default { initialize() {}, connectToDevTools() {} }', + loader: 'js' + })) + } +} + +await build({ + entryPoints: [resolve(root, 'src/entry.tsx')], + bundle: true, + platform: 'node', + format: 'esm', + target: 'node20', + outfile: out, + jsx: 'automatic', + jsxImportSource: 'react', + // Skip the prebuilt @hermes/ink bundle — esbuild's __esm helper doesn't + // await nested async init, which breaks lazy-initialized exports like + // `render`. Bundling from source sidesteps that. + alias: { '@hermes/ink': resolve(root, 'packages/hermes-ink/src/entry-exports.ts') }, + plugins: [stubDevtools], + // Some transitive deps use CommonJS `require(...)` at runtime. ESM bundles + // don't get a `require` binding automatically, so we inject one. + banner: { + js: "import { createRequire as __cr } from 'node:module'; const require = __cr(import.meta.url);" + }, + logLevel: 'info' +}) + +// esbuild preserves the shebang from src/entry.tsx into the bundle, but Nix's +// patchShebangs phase mangles `/usr/bin/env -S node --foo --bar` (it strips +// the `node` token, leaving a broken interpreter). The hermes_cli launcher +// always invokes this file as `node dist/entry.js` anyway, so the shebang is +// redundant — strip it. +const body = readFileSync(out, 'utf8') +if (body.startsWith('#!')) { + writeFileSync(out, body.slice(body.indexOf('\n') + 1)) +} + +console.log(`built ${out}`) From 279504d5b887ad57a7da4c3ca8d7d534d7ca47fe Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 30 Apr 2026 19:49:01 +0000 Subject: [PATCH 02/12] fix(nix): refresh npm lockfile hashes --- nix/tui.nix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nix/tui.nix b/nix/tui.nix index eb25d52f90..52875074b4 100644 --- a/nix/tui.nix +++ b/nix/tui.nix @@ -4,7 +4,7 @@ let src = ../ui-tui; npmDeps = pkgs.fetchNpmDeps { inherit src; - hash = "sha256-Chz+NW9NXqboXHOa6PKwf5bhAkkcFtKNhvKWwg2XSPc="; + hash = "sha256-hxBD2zsPwdSoUL57feFFGqZ2Z1xIHxERwmQa/jIqNZw="; }; npm = hermesNpmLib.mkNpmPassthru { folder = "ui-tui"; attr = "tui"; pname = "hermes-tui"; }; From 42e166c7ea2e00e86c0e77f6a6536140b3707827 Mon Sep 17 00:00:00 2001 From: ethernet Date: Thu, 30 Apr 2026 16:57:02 -0400 Subject: [PATCH 03/12] refactor(docker): drop manual @hermes/ink build, rely on esbuild bundle the esbuild pipeline (scripts/build.mjs) already bundles ink into a single self-contained dist/entry.js. remove the Dockerfile steps that manually copied packages/hermes-ink into node_modules/@hermes/ink and ran a nested npm install there. - Dockerfile: simplify TUI build step to just 'npm run build' - hermes_cli/main.py: _tui_build_needed now checks dist/entry.js staleness against source files before falling back to the old ink-bundle.js logic - tests: update TUI npm install tests and drop the Dockerfile contract test for the removed ink materialization step --- Dockerfile | 8 +----- hermes_cli/main.py | 32 ++++++++++++++++++++- tests/hermes_cli/test_tui_npm_install.py | 17 +++++++++-- tests/tools/test_dockerfile_pid1_reaping.py | 14 --------- 4 files changed, 47 insertions(+), 24 deletions(-) diff --git a/Dockerfile b/Dockerfile index 18177cc1ac..be147b6eac 100644 --- a/Dockerfile +++ b/Dockerfile @@ -45,13 +45,7 @@ COPY --chown=hermes:hermes . . # Build browser dashboard and terminal UI assets. RUN cd web && npm run build && \ - cd ../ui-tui && npm run build && \ - rm -rf node_modules/@hermes/ink && \ - rm -rf packages/hermes-ink/node_modules && \ - cp -R packages/hermes-ink node_modules/@hermes/ink && \ - npm install --omit=dev --prefer-offline --no-audit --prefix node_modules/@hermes/ink && \ - rm -rf node_modules/@hermes/ink/node_modules/react && \ - node --input-type=module -e "await import('@hermes/ink')" + cd ../ui-tui && npm run build # ---------- Permissions ---------- # Make install dir world-readable so any HERMES_UID can read it at runtime. diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 7b5834aea6..831cd76257 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -922,9 +922,39 @@ def _find_bundled_tui(tui_dir: Path) -> Optional[Path]: def _tui_build_needed(tui_dir: Path) -> bool: + entry = tui_dir / "dist" / "entry.js" + # In the esbuild pipeline, ink is bundled into dist/entry.js directly. + # If the main bundle exists and is up to date with all source files, + # no separate ink rebuild is needed. + if entry.exists(): + dist_m = entry.stat().st_mtime + skip = frozenset({"node_modules", "dist"}) + stale = False + for dirpath, dirnames, filenames in os.walk(tui_dir, topdown=True): + dirnames[:] = [d for d in dirnames if d not in skip] + for fn in filenames: + if fn.endswith((".ts", ".tsx")): + if os.path.getmtime(os.path.join(dirpath, fn)) > dist_m: + stale = True + break + if stale: + break + if not stale: + for meta in ( + "package.json", + "package-lock.json", + "tsconfig.json", + "tsconfig.build.json", + ): + mp = tui_dir / meta + if mp.exists() and mp.stat().st_mtime > dist_m: + stale = True + break + if not stale: + return False + if _hermes_ink_bundle_stale(tui_dir): return True - entry = tui_dir / "dist" / "entry.js" if not entry.exists(): return True dist_m = entry.stat().st_mtime diff --git a/tests/hermes_cli/test_tui_npm_install.py b/tests/hermes_cli/test_tui_npm_install.py index 8e16c5e9de..0ef98c9ea6 100644 --- a/tests/hermes_cli/test_tui_npm_install.py +++ b/tests/hermes_cli/test_tui_npm_install.py @@ -95,12 +95,25 @@ def test_no_install_prebuilt_bundle_mode(tmp_path: Path, main_mod) -> None: assert main_mod._tui_need_npm_install(tmp_path) is False -def test_build_needed_when_local_ink_bundle_missing(tmp_path: Path, main_mod) -> None: +def test_build_needed_when_source_newer_than_entry(tmp_path: Path, main_mod) -> None: + _touch_tui_entry(tmp_path) + _touch_ink(tmp_path) + src = tmp_path / "src" / "entry.tsx" + src.parent.mkdir(parents=True, exist_ok=True) + src.write_text("console.log('newer')") + os.utime(src, (200, 200)) + os.utime(tmp_path / "dist" / "entry.js", (100, 100)) + + assert main_mod._tui_need_npm_install(tmp_path) is False + assert main_mod._tui_build_needed(tmp_path) is True + + +def test_build_not_needed_when_entry_exists_and_sources_unchanged(tmp_path: Path, main_mod) -> None: _touch_tui_entry(tmp_path) _touch_ink(tmp_path) assert main_mod._tui_need_npm_install(tmp_path) is False - assert main_mod._tui_build_needed(tmp_path) is True + assert main_mod._tui_build_needed(tmp_path) is False def test_build_not_needed_when_entry_and_ink_bundle_present(tmp_path: Path, main_mod) -> None: diff --git a/tests/tools/test_dockerfile_pid1_reaping.py b/tests/tools/test_dockerfile_pid1_reaping.py index 52532a78dd..960415d417 100644 --- a/tests/tools/test_dockerfile_pid1_reaping.py +++ b/tests/tools/test_dockerfile_pid1_reaping.py @@ -121,20 +121,6 @@ def test_dockerfile_builds_tui_assets(dockerfile_text): ) -def test_dockerfile_materializes_local_tui_ink_package(dockerfile_text): - assert any( - "ui-tui" in step - and "node_modules/@hermes/ink" in step - and "packages/hermes-ink" in step - and "rm -rf packages/hermes-ink/node_modules" in step - and "npm install --omit=dev" in step - and "--prefix node_modules/@hermes/ink" in step - and "rm -rf node_modules/@hermes/ink/node_modules/react" in step - and "await import('@hermes/ink')" in step - for step in _run_steps(dockerfile_text) - ) - - def test_dockerignore_excludes_nested_dependency_dirs(): if not DOCKERIGNORE.exists(): pytest.skip(".dockerignore not present in this checkout") From 42df7ec597e0ddd96edc7c0cb0e1c805a7cfcf6b Mon Sep 17 00:00:00 2001 From: ethernet Date: Thu, 30 Apr 2026 17:43:59 -0400 Subject: [PATCH 04/12] fix(tui): update comments --- nix/tui.nix | 1 - website/docs/user-guide/tui.md | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/nix/tui.nix b/nix/tui.nix index 52875074b4..55f24375bf 100644 --- a/nix/tui.nix +++ b/nix/tui.nix @@ -25,7 +25,6 @@ pkgs.buildNpmPackage (npm // { mkdir -p $out/lib/hermes-tui # Single self-contained bundle built by scripts/build.mjs (esbuild). - # No runtime node_modules needed. cp -r dist $out/lib/hermes-tui/dist # package.json kept for "type": "module" resolution on `node dist/entry.js`. diff --git a/website/docs/user-guide/tui.md b/website/docs/user-guide/tui.md index c7f0eeb844..c8b5aa7288 100644 --- a/website/docs/user-guide/tui.md +++ b/website/docs/user-guide/tui.md @@ -66,7 +66,7 @@ export HERMES_TUI_DIR=/path/to/prebuilt/ui-tui hermes --tui ``` -The directory must contain `dist/entry.js` and an up-to-date `node_modules`. +The directory must contain `dist/entry.js`. ## Keybindings From 242659f5af3121ef9bb397c2559f67fdfbce13a4 Mon Sep 17 00:00:00 2001 From: ethernet Date: Thu, 30 Apr 2026 18:22:21 -0400 Subject: [PATCH 05/12] fix(tui): don't hardcode /home/bb --- scripts/profile-tui.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/scripts/profile-tui.py b/scripts/profile-tui.py index 18cbbc74d7..b55febb9d9 100755 --- a/scripts/profile-tui.py +++ b/scripts/profile-tui.py @@ -15,7 +15,7 @@ session-picker flow. Environment overrides: HERMES_PERF_LOG (default ~/.hermes/perf.log) HERMES_PERF_NODE (default node from $PATH) - HERMES_TUI_DIR (default /home/bb/hermes-agent/ui-tui) + HERMES_TUI_DIR (default: /ui-tui relative to this script) Exit code is 0 if the harness ran and parsed results, 2 if the TUI crashed or produced no perf data (suggests HERMES_DEV_PERF wiring is broken). @@ -36,7 +36,10 @@ from pathlib import Path from typing import Any -DEFAULT_TUI_DIR = Path(os.environ.get("HERMES_TUI_DIR", "/home/bb/hermes-agent/ui-tui")) +DEFAULT_TUI_DIR = Path( + os.environ.get("HERMES_TUI_DIR") + or str(Path(__file__).resolve().parent.parent / "ui-tui") +) DEFAULT_LOG = Path(os.environ.get("HERMES_PERF_LOG", str(Path.home() / ".hermes" / "perf.log"))) DEFAULT_STATE_DB = Path.home() / ".hermes" / "state.db" From 9d645d98c445c94826eb0d88ba5fe8ab62faf483 Mon Sep 17 00:00:00 2001 From: ethernet Date: Thu, 30 Apr 2026 18:23:15 -0400 Subject: [PATCH 06/12] fix(tui): update README --- ui-tui/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui-tui/README.md b/ui-tui/README.md index 17d57f08af..60ded94fd8 100644 --- a/ui-tui/README.md +++ b/ui-tui/README.md @@ -41,7 +41,7 @@ From the repo root, the normal path is: hermes --tui ``` -The CLI expects `ui-tui/node_modules` to exist. If the TUI deps are missing: +The CLI expects `ui-tui/dist/entry.js` to exist, or the whole source code available in which to run `npm install` and `npm run dev`. ```bash cd ui-tui From c6ca11618a87c6b12e9a4025d339eb905a03ac8c Mon Sep 17 00:00:00 2001 From: ethernet Date: Mon, 11 May 2026 17:04:34 -0400 Subject: [PATCH 07/12] refactor(tui): simplify TUI build logic, remove stale staleness checks The old mtime-tracking staleness machinery (_tui_build_needed, _hermes_ink_bundle_stale, _find_bundled_tui) tried to avoid rebuilding by comparing source timestamps to dist/entry.js. This was fragile and added ~100 lines of code. Replace with three clear paths: 1. HERMES_TUI_DIR set (prebuilt/nix): just node dist/entry.js, no build 2. --dev mode: tsx src/entry.tsx, no build, hot reload 3. Normal: always npm run build (esbuild is ~1s, correctness > caching) Also error when HERMES_TUI_DIR is set with --dev (footgun: prebuilt bundle has no source code to hot-reload). --- hermes_cli/main.py | 136 ++++++----------------- tests/hermes_cli/test_tui_npm_install.py | 35 ------ 2 files changed, 33 insertions(+), 138 deletions(-) diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 15246a88ab..009af00b6e 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -967,66 +967,6 @@ def _tui_need_npm_install(root: Path) -> bool: return False -def _find_bundled_tui(tui_dir: Path) -> Optional[Path]: - """Directory whose dist/entry.js we should run: HERMES_TUI_DIR first, else repo ui-tui.""" - env = os.environ.get("HERMES_TUI_DIR") - if env: - p = Path(env) - if (p / "dist" / "entry.js").exists() and not _tui_need_npm_install(p): - return p - if (tui_dir / "dist" / "entry.js").exists() and not _tui_need_npm_install(tui_dir): - return tui_dir - return None - - -def _tui_build_needed(tui_dir: Path) -> bool: - # esbuild bundles @hermes/ink + source directly into dist/entry.js; - # the old ink-bundle.js check only fires when entry.js hasn't been - # produced yet (a dev checkout that hasn't been built at all). Once - # entry.js exists, the mtime walk below covers all source trees. - entry = tui_dir / "dist" / "entry.js" - if not entry.exists(): - # Nothing built yet — signal that a build is needed. - return True - dist_m = entry.stat().st_mtime - skip = frozenset({"node_modules", "dist"}) - for dirpath, dirnames, filenames in os.walk(tui_dir, topdown=True): - dirnames[:] = [d for d in dirnames if d not in skip] - for fn in filenames: - if fn.endswith((".ts", ".tsx")): - if os.path.getmtime(os.path.join(dirpath, fn)) > dist_m: - return True - for meta in ( - "package.json", - "package-lock.json", - "tsconfig.json", - "tsconfig.build.json", - ): - mp = tui_dir / meta - if mp.exists() and mp.stat().st_mtime > dist_m: - return True - return False - - -def _hermes_ink_bundle_stale(tui_dir: Path) -> bool: - ink_root = tui_dir / "packages" / "hermes-ink" - bundle = ink_root / "dist" / "ink-bundle.js" - if not bundle.exists(): - return True - bm = bundle.stat().st_mtime - skip = frozenset({"node_modules", "dist"}) - for dirpath, dirnames, filenames in os.walk(ink_root, topdown=True): - dirnames[:] = [d for d in dirnames if d not in skip] - for fn in filenames: - if fn.endswith((".ts", ".tsx")): - if os.path.getmtime(os.path.join(dirpath, fn)) > bm: - return True - mp = ink_root / "package.json" - if mp.exists() and mp.stat().st_mtime > bm: - return True - return False - - def _ensure_tui_node() -> None: """Make sure `node` + `npm` are on PATH for the TUI. @@ -1085,7 +1025,7 @@ def _ensure_tui_node() -> None: def _make_tui_argv(tui_dir: Path, tui_dev: bool) -> tuple[list[str], Path]: - """TUI: --dev → tsx src; else node dist (HERMES_TUI_DIR or ui-tui, build when stale).""" + """TUI: --dev → tsx src; else node dist (HERMES_TUI_DIR prebuilt or esbuild).""" _ensure_tui_node() def _node_bin(bin: str) -> str: @@ -1099,23 +1039,31 @@ def _make_tui_argv(tui_dir: Path, tui_dev: bool) -> tuple[list[str], Path]: sys.exit(1) return path - # pre-built dist + node_modules (nix / full HERMES_TUI_DIR) skips npm. + # Footgun: --dev against a prebuilt bundle that has no source/node_modules. + ext_dir = os.environ.get("HERMES_TUI_DIR") + if tui_dev and ext_dir: + print( + f"Error: --dev is incompatible with HERMES_TUI_DIR={ext_dir}\n" + f"The prebuilt TUI has no source code to hot-reload.\n" + f"Unset HERMES_TUI_DIR (e.g. `unset HERMES_TUI_DIR`) to use --dev from a checkout.", + file=sys.stderr, + ) + sys.exit(1) + + # 1. Prebuilt bundle (nix / packaged release): just run it. if not tui_dev: - ext_dir = os.environ.get("HERMES_TUI_DIR") if ext_dir: p = Path(ext_dir) - if (p / "dist" / "entry.js").exists() and not _tui_need_npm_install(p): + if (p / "dist" / "entry.js").is_file(): node = _node_bin("node") return [node, str(p / "dist" / "entry.js")], p - npm = _node_bin("npm") + # 2. Normal flow: npm install if needed, always esbuild, then node dist/entry.js. + # --dev flow: npm install if needed, then tsx src/entry.tsx (no build). if _tui_need_npm_install(tui_dir): + npm = _node_bin("npm") if not os.environ.get("HERMES_QUIET"): print("Installing TUI dependencies…") - # Capture stdout as well as stderr — some npm errors (notably EACCES on a - # root-owned node_modules in containers) are emitted on stdout, and a - # bare "npm install failed." with no preview defeats debugging. We keep - # the failure-only print path so a successful install stays silent. result = subprocess.run( [npm, "install", "--silent", "--no-fund", "--no-audit", "--progress=false"], cwd=str(tui_dir), @@ -1133,47 +1081,30 @@ def _make_tui_argv(tui_dir: Path, tui_dev: bool) -> tuple[list[str], Path]: sys.exit(1) if tui_dev: - if _hermes_ink_bundle_stale(tui_dir): - result = subprocess.run( - [npm, "run", "build", "--prefix", "packages/hermes-ink"], - cwd=str(tui_dir), - capture_output=True, - text=True, - ) - if result.returncode != 0: - combined = f"{result.stdout or ''}{result.stderr or ''}".strip() - preview = "\n".join(combined.splitlines()[-30:]) - print("@hermes/ink build failed.") - if preview: - print(preview) - sys.exit(1) tsx = tui_dir / "node_modules" / ".bin" / "tsx" if tsx.exists(): return [str(tsx), "src/entry.tsx"], tui_dir + npm = _node_bin("npm") return [npm, "start"], tui_dir - if _tui_build_needed(tui_dir): - result = subprocess.run( - [npm, "run", "build"], - cwd=str(tui_dir), - capture_output=True, - text=True, - ) - if result.returncode != 0: - combined = f"{result.stdout or ''}{result.stderr or ''}".strip() - preview = "\n".join(combined.splitlines()[-30:]) - print("TUI build failed.") - if preview: - print(preview) - sys.exit(1) - - root = _find_bundled_tui(tui_dir) - if not root: - print("TUI build did not produce dist/entry.js") + # Always rebuild — esbuild is fast and this avoids staleness-edge-case bugs. + npm = _node_bin("npm") + result = subprocess.run( + [npm, "run", "build"], + cwd=str(tui_dir), + capture_output=True, + text=True, + ) + if result.returncode != 0: + combined = f"{result.stdout or ''}{result.stderr or ''}".strip() + preview = "\n".join(combined.splitlines()[-30:]) + print("TUI build failed.") + if preview: + print(preview) sys.exit(1) node = _node_bin("node") - return [node, str(root / "dist" / "entry.js")], root + return [node, str(tui_dir / "dist" / "entry.js")], tui_dir def _normalize_tui_toolsets(toolsets: object) -> list[str]: @@ -5503,7 +5434,6 @@ def _gateway_prompt(prompt_text: str, default: str = "", timeout: float = 300.0) def _web_ui_build_needed(web_dir: Path) -> bool: """Return True if the web UI dist is missing or stale. - Mirrors the staleness logic used by ``_tui_build_needed()`` for the TUI. The Vite build outputs to ``hermes_cli/web_dist/`` (per vite.config.ts outDir: "../hermes_cli/web_dist"), NOT to ``web/dist/``. Uses the Vite manifest as the sentinel because it is written last and therefore has the diff --git a/tests/hermes_cli/test_tui_npm_install.py b/tests/hermes_cli/test_tui_npm_install.py index f17ed5a074..efad281565 100644 --- a/tests/hermes_cli/test_tui_npm_install.py +++ b/tests/hermes_cli/test_tui_npm_install.py @@ -25,12 +25,6 @@ def _touch_tui_entry(root: Path) -> None: entry.write_text("console.log('tui')") -def _touch_ink_bundle(root: Path) -> None: - bundle = root / "packages" / "hermes-ink" / "dist" / "ink-bundle.js" - bundle.parent.mkdir(parents=True, exist_ok=True) - bundle.write_text("export {}") - - def test_need_install_when_ink_missing(tmp_path: Path, main_mod) -> None: (tmp_path / "package-lock.json").write_text("{}") assert main_mod._tui_need_npm_install(tmp_path) is True @@ -126,32 +120,3 @@ def test_no_install_prebuilt_bundle_mode(tmp_path: Path, main_mod) -> None: """dist/entry.js present and no package-lock.json → prebuilt bundle, skip npm install.""" _touch_tui_entry(tmp_path) assert main_mod._tui_need_npm_install(tmp_path) is False - - -def test_build_needed_when_source_newer_than_entry(tmp_path: Path, main_mod) -> None: - _touch_tui_entry(tmp_path) - _touch_ink(tmp_path) - src = tmp_path / "src" / "entry.tsx" - src.parent.mkdir(parents=True, exist_ok=True) - src.write_text("console.log('newer')") - os.utime(src, (200, 200)) - os.utime(tmp_path / "dist" / "entry.js", (100, 100)) - - assert main_mod._tui_need_npm_install(tmp_path) is False - assert main_mod._tui_build_needed(tmp_path) is True - - -def test_build_not_needed_when_entry_exists_and_sources_unchanged(tmp_path: Path, main_mod) -> None: - _touch_tui_entry(tmp_path) - _touch_ink(tmp_path) - - assert main_mod._tui_need_npm_install(tmp_path) is False - assert main_mod._tui_build_needed(tmp_path) is False - - -def test_build_not_needed_when_entry_and_ink_bundle_present(tmp_path: Path, main_mod) -> None: - _touch_tui_entry(tmp_path) - _touch_ink(tmp_path) - _touch_ink_bundle(tmp_path) - - assert main_mod._tui_build_needed(tmp_path) is False From ea1d0462cf5ec3799fb0c4b8e39685302e53039b Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Mon, 11 May 2026 16:49:13 -0700 Subject: [PATCH 08/12] fix(cli): vertical fallback for markdown tables wider than terminal (#23948) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #23863 (CJK table alignment). The realigner was correctly padding pipes to identical column offsets, but when a table's natural width exceeds terminal cells it produced lines that the terminal soft-wrapped mid-cell, destroying column alignment visually even though the bytes were perfectly padded. Reported as 'columns are not aligned' on tables containing one long row alongside several short rows. Approach mirrors Claude Code's MarkdownTable.tsx narrow-terminal fallback: when realign_markdown_tables is given an available_width budget and the rebuilt horizontal table exceeds it, render each body row as 'Header: value' lines separated by a thin ─ rule. Word-wraps oversize values at the budget with a 2-space continuation indent. - agent/markdown_tables.py: realign_markdown_tables(text, available_width=None); threshold check at the top of _render_block flips into a new _render_vertical fallback. Includes _wrap_to_width with hard-break for tokens longer than the budget. - cli.py: helper _terminal_width_for_streaming() returns shutil.get_terminal_size().columns minus _STREAM_PAD and a 2-cell safety margin; passed to all three realign call sites (_render_final_assistant_content for strip+render Panel paths, and the streaming flushers in _emit_stream_text / _flush_stream). - tests/agent/test_markdown_tables.py: 4 new tests covering the overflow-vertical fallback for ASCII + CJK content, the 'fits → keep horizontal' case, and the long-cell wrap with indent. Live-verified: with COLUMNS=100, the user's reported 'long row in ASCII table' case now renders as vertical key-value rows that all fit the panel; the 6-column CJK comparison table still renders as an aligned horizontal table because it fits inside 100 cols. --- agent/markdown_tables.py | 147 +++++++++++++++++++++++++++- cli.py | 40 +++++++- tests/agent/test_markdown_tables.py | 102 +++++++++++++++++++ 3 files changed, 281 insertions(+), 8 deletions(-) diff --git a/agent/markdown_tables.py b/agent/markdown_tables.py index 13c7cd1df0..f37569cede 100644 --- a/agent/markdown_tables.py +++ b/agent/markdown_tables.py @@ -102,8 +102,16 @@ def looks_like_table_row(row: str) -> bool: return stripped.count("|") >= 2 -def _render_block(rows: List[List[str]]) -> List[str]: - """Render ``rows`` (header + body, divider implied) at uniform widths.""" +def _render_block(rows: List[List[str]], available_width: int | None = None) -> List[str]: + """Render ``rows`` (header + body, divider implied) at uniform widths. + + If ``available_width`` is given and the rebuilt horizontal table + would exceed it, fall back to a vertical key-value rendering so + rows do not soft-wrap mid-cell — terminal soft-wrap destroys + column alignment visually even when the underlying bytes are + perfectly padded, which is exactly the "tables look broken" + user report this code path is meant to address. + """ ncols = max(len(r) for r in rows) rows = [r + [""] * (ncols - len(r)) for r in rows] @@ -113,6 +121,13 @@ def _render_block(rows: List[List[str]]) -> List[str]: for c in range(ncols) ] + # Total horizontal width for the rendered row: + # `| ` + cell + ` ` for each column, plus the final closing `|`. + horizontal_width = sum(widths) + 3 * ncols + 1 + + if available_width is not None and horizontal_width > max(available_width, 20): + return _render_vertical(rows, ncols, available_width) + def _row(cells: List[str]) -> str: return ( "| " @@ -127,11 +142,135 @@ def _render_block(rows: List[List[str]]) -> List[str]: return out -def realign_markdown_tables(text: str) -> str: +def _wrap_to_width(text: str, width: int) -> List[str]: + """Soft-wrap ``text`` at word boundaries to fit ``width`` display cells. + + Falls back to hard-breaking the longest word if a single token is + wider than ``width``. Empty input yields a single empty string so + the caller's row count stays predictable. + """ + + if width <= 0 or not text: + return [text] + + words = text.split() + if not words: + return [""] + + lines: List[str] = [] + current = "" + current_w = 0 + + def _hard_break(word: str, w: int) -> List[str]: + out: List[str] = [] + buf = "" + bw = 0 + for ch in word: + cw = _disp_width(ch) or 1 + if bw + cw > w and buf: + out.append(buf) + buf = ch + bw = cw + else: + buf += ch + bw += cw + if buf: + out.append(buf) + return out + + for word in words: + ww = _disp_width(word) + if not current: + if ww <= width: + current = word + current_w = ww + else: + pieces = _hard_break(word, width) + lines.extend(pieces[:-1]) + current = pieces[-1] if pieces else "" + current_w = _disp_width(current) + continue + if current_w + 1 + ww <= width: + current += " " + word + current_w += 1 + ww + else: + lines.append(current) + if ww <= width: + current = word + current_w = ww + else: + pieces = _hard_break(word, width) + lines.extend(pieces[:-1]) + current = pieces[-1] if pieces else "" + current_w = _disp_width(current) + if current: + lines.append(current) + return lines or [""] + + +def _render_vertical( + rows: List[List[str]], ncols: int, available_width: int +) -> List[str]: + """Render a too-wide table as vertical ``Header: value`` rows. + + Mirrors Claude Code's narrow-terminal fallback in + ``MarkdownTable.tsx``: each body row becomes a small block of + ``Header: cell-value`` lines (continuation lines indented two + spaces) separated by a thin ``─`` divider between rows. Keeps + every line narrower than ``available_width`` so the terminal does + not soft-wrap mid-cell. + """ + + if not rows: + return [] + + headers = rows[0] + [""] * (ncols - len(rows[0])) + body = rows[1:] + + labels = [h or f"Column {i + 1}" for i, h in enumerate(headers)] + + sep_width = max(20, min(40, available_width - 2)) if available_width else 30 + separator = "─" * sep_width + indent = " " + indent_w = _disp_width(indent) + + out: List[str] = [] + for ri, row in enumerate(body): + if ri > 0: + out.append(separator) + for ci in range(ncols): + label = labels[ci] + value = row[ci] if ci < len(row) else "" + label_w = _disp_width(label) + first_budget = max(10, available_width - label_w - 2) + cont_budget = max(10, available_width - indent_w) + if not value: + out.append(f"{label}:") + continue + wrapped = _wrap_to_width(value, first_budget) + out.append(f"{label}: {wrapped[0]}") + if len(wrapped) > 1: + # Re-flow continuation text at the wider continuation + # budget — words split across the narrower first-line + # budget should re-pack greedily for the rest. + cont_text = " ".join(wrapped[1:]) + for cl in _wrap_to_width(cont_text, cont_budget): + if cl.strip(): + out.append(f"{indent}{cl}") + return out + + +def realign_markdown_tables(text: str, available_width: int | None = None) -> str: """Rewrite every ``| ... |`` + divider block with wcwidth-aware padding. Lines that are not part of a recognised table are returned verbatim, so this is safe to apply to arbitrary assistant prose. + + If ``available_width`` is given (terminal cells available for the + rendered table), tables wider than that are rendered as vertical + key-value pairs instead of a horizontal pipe-bordered grid. This + avoids the terminal soft-wrapping mid-cell, which destroys column + alignment visually even when the bytes are perfectly padded. """ if "|" not in text: @@ -161,7 +300,7 @@ def realign_markdown_tables(text: str) -> str: j += 1 if any(c for c in header) or body: - out.extend(_render_block([header] + body)) + out.extend(_render_block([header] + body, available_width)) i = j continue out.append(line) diff --git a/cli.py b/cli.py index 7843882c2c..37f2a96b5a 100644 --- a/cli.py +++ b/cli.py @@ -1354,16 +1354,48 @@ def _preserve_windows_dot_segments_for_markdown(text: str) -> str: return _WINDOWS_PATH_WITH_DOT_SEGMENT_RE.sub(_protect, text) +def _terminal_width_for_streaming() -> int: + """Display cells available inside the streamed response box. + + The streaming path indents every line by ``_STREAM_PAD`` (4 cells) + inside an open response panel. The realigner uses this number as + its budget when deciding whether to keep a horizontal table or + fall back to vertical key-value rendering. We subtract a small + safety margin so terminal-resize races don't push a borderline + table into mid-cell soft-wrap. + """ + + try: + cols = shutil.get_terminal_size((80, 24)).columns + except Exception: + cols = 80 + return max(20, cols - len(_STREAM_PAD) - 2) + + def _render_final_assistant_content(text: str, mode: str = "render"): """Render final assistant content as markdown, stripped text, or raw text.""" from rich.markdown import Markdown + # Estimate the cells available to the rendered table. The Panel + # used by the background-task / final-response path has 4 cells of + # left+right padding plus 1 cell of border on each side, plus the + # _STREAM_PAD indent that streamed content uses. Subtract a small + # safety margin so resize races don't push a borderline table into + # soft-wrap. + try: + cols = shutil.get_terminal_size((80, 24)).columns + except Exception: + cols = 80 + panel_width = max(20, cols - 12) + normalized_mode = str(mode or "render").strip().lower() if normalized_mode == "strip": # Strip first — inline markdown inside cells (`code`, **bold**, ~~strike~~) # changes cell display width — then re-align so the column padding # reflects the final visible text, not the marker-decorated source. - return _RichText(realign_markdown_tables(_strip_markdown_syntax(text))) + return _RichText( + realign_markdown_tables(_strip_markdown_syntax(text), panel_width) + ) if normalized_mode == "raw": return _rich_text_from_ansi(text or "") @@ -1374,7 +1406,7 @@ def _render_final_assistant_content(text: str, mode: str = "render"): # (narrow panels, etc.) at least see consistent input. plain = _rich_text_from_ansi(text or "").plain plain = _preserve_windows_dot_segments_for_markdown(plain) - plain = realign_markdown_tables(plain) + plain = realign_markdown_tables(plain, panel_width) return Markdown(plain) @@ -3662,7 +3694,7 @@ class HermesCLI: joined = "\n".join(buf) if self.final_response_markdown == "strip": joined = _strip_markdown_syntax(joined) - block = realign_markdown_tables(joined) + block = realign_markdown_tables(joined, _terminal_width_for_streaming()) for ln in block.split("\n"): _emit_one(ln) @@ -3726,7 +3758,7 @@ class HermesCLI: self._in_stream_table = False if self.final_response_markdown == "strip": joined = _strip_markdown_syntax(joined) - block = realign_markdown_tables(joined) + block = realign_markdown_tables(joined, _terminal_width_for_streaming()) for ln in block.split("\n"): _cprint(f"{_STREAM_PAD}{_tc}{ln}{_RST}" if _tc else f"{_STREAM_PAD}{ln}") diff --git a/tests/agent/test_markdown_tables.py b/tests/agent/test_markdown_tables.py index 3c97a4c6fc..d4eb3d4ce2 100644 --- a/tests/agent/test_markdown_tables.py +++ b/tests/agent/test_markdown_tables.py @@ -156,6 +156,108 @@ def test_passes_non_table_lines_through_around_a_table(): assert all(o == offsets[0] for o in offsets) +# --------------------------------------------------------------------------- +# Vertical fallback for tables wider than the terminal +# --------------------------------------------------------------------------- + + +def test_overflow_falls_back_to_vertical_when_table_wider_than_terminal(): + """A horizontal table that would exceed the available width must + drop to vertical key-value rendering so the terminal does not + soft-wrap mid-cell (which destroys column alignment visually).""" + + src = dedent( + """\ + | Item | Description | Notes | + |------|-------------|-------| + | a | short | ok | + | b | this is a much longer description that stretches the column wider than the others by a lot | fine | + | c | tiny | - | + """ + ) + + out = realign_markdown_tables(src, available_width=100) + + # No horizontal pipe-bordered rows: vertical mode emits "Header: value" + # lines and a ─ separator instead. + assert "|" not in out + assert "Item: a" in out + assert "Description: short" in out + assert "Notes: ok" in out + # Body rows separated by ─ rule + assert "──" in out + + # Every emitted line fits the available width. + for line in out.split("\n"): + assert wcswidth(line) <= 100, f"line wider than budget: {line!r}" + + +def test_horizontal_kept_when_table_fits(): + """A table that fits the terminal must keep the horizontal + pipe-bordered rendering — vertical fallback only kicks in when + soft-wrap is unavoidable.""" + + src = dedent( + """\ + | Name | Age | + |------|-----| + | Alice | 30 | + | Bob | 25 | + """ + ) + + out = realign_markdown_tables(src, available_width=100) + + # Pipe-bordered rendering survives. + body_rows = [ln for ln in out.split("\n") if ln.strip().startswith("|")] + assert len(body_rows) == 4 + offsets = [_column_offsets(r) for r in body_rows] + assert all(o == offsets[0] for o in offsets) + + +def test_vertical_fallback_wraps_long_cell_text_with_indent(): + src = dedent( + """\ + | Key | Value | + |-----|-------| + | x | this value is long enough that wrapping the value to fit a narrow terminal width is required even in vertical mode | + """ + ) + + out = realign_markdown_tables(src, available_width=60) + + lines = out.split("\n") + assert lines[0].startswith("Key: x") + # First "Value:" line + at least one continuation indented by 2 spaces. + value_idx = next(i for i, l in enumerate(lines) if l.startswith("Value:")) + assert lines[value_idx + 1].startswith(" ") + # Every line still fits the budget. + for line in lines: + assert wcswidth(line) <= 60 + + +def test_overflow_falls_back_to_vertical_for_cjk_too(): + """CJK content can also push a table over the terminal budget; + the vertical fallback should kick in regardless of script.""" + + src = dedent( + """\ + | 模型 | 描述 | 备注 | + |------|------|------| + | 千问 | 一个相当长的描述用于把列宽撑得超过可用终端宽度从而触发竖排回退 | 通过 | + | 文心 | 短 | × | + """ + ) + + out = realign_markdown_tables(src, available_width=50) + + assert "|" not in out + assert "模型: 千问" in out + assert "模型: 文心" in out + for line in out.split("\n"): + assert wcswidth(line) <= 50, f"line wider than budget: {line!r}" + + def test_handles_ragged_rows_by_padding_short_rows(): src = dedent( """\ From 05bad7b1e78adef8da4dcbe91ab95c3e810a96b0 Mon Sep 17 00:00:00 2001 From: Ahmed Badr Date: Sat, 9 May 2026 22:53:13 +0100 Subject: [PATCH 09/12] fix(dashboard): MiniMax 'Login' button launched Claude OAuth (#22832) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #22832. ## Root cause `hermes_cli/web_server.py:start_oauth_login` dispatched OAuth flows by the catalog's `flow` field rather than provider id: if catalog_entry["flow"] == "pkce": return _start_anthropic_pkce() The catalog had two `flow: "pkce"` entries — `anthropic` and `minimax-oauth` — so clicking "Login" on MiniMax in the dashboard's Keys tab unconditionally launched the Anthropic/Claude PKCE flow. ## Fix Three changes in `hermes_cli/web_server.py`: 1. Catalog entry for `minimax-oauth` changed from `flow: "pkce"` to `flow: "device_code"`. From a UX perspective MiniMax is a verification-URI + user-code flow (open URL, enter code, backend polls) — same shape as Nous's device-code flow. The PKCE bit (verifier + challenge from `_minimax_pkce_pair`) is a security extension that doesn't change the operator experience; the existing dashboard modal already renders `device_code` correctly for this UX. 2. New MiniMax branch in `_start_device_code_flow`, mirroring the existing Nous branch but calling MiniMax-specific helpers (`_minimax_request_user_code`, `_minimax_pkce_pair`). Stashes verifier + state in the session for the poller to consume. Handles the overloaded `expired_in` field (could be unix-ms timestamp OR seconds-from-now duration) the same way `_minimax_poll_token` does. 3. New `_minimax_poller` background thread mirroring `_nous_poller`. Calls `_minimax_poll_token` → on success builds the same `auth_state` dict the CLI flow (`_minimax_oauth_login`) builds, and persists via `_minimax_save_auth_state` so the dashboard path leaves the system in the same state as `hermes auth add minimax-oauth`. Plus a dispatcher tightening to prevent regression: the `pkce` branch now requires `provider_id == "anthropic"`, so any future PKCE provider added without a proper start function gets a clean `400 Unsupported flow` rather than silently launching Anthropic OAuth. ## Test New `tests/hermes_cli/test_web_oauth_dispatch.py`: - Regression test asserting MiniMax start does NOT return claude.ai - Sanity test that Anthropic PKCE still works after the dispatcher tightening - Forward-looking test: a hypothetical pkce-flagged provider without an explicit branch is rejected cleanly rather than misrouted ## Limitations - The dashboard MiniMax path defaults to `region="global"`. CN-region operators can still use the CLI flow which supports `--region cn`. Adding a region toggle to the dashboard UI is a follow-up. --- hermes_cli/web_server.py | 173 +++++++++++++++++++- tests/hermes_cli/test_web_oauth_dispatch.py | 129 +++++++++++++++ 2 files changed, 299 insertions(+), 3 deletions(-) create mode 100644 tests/hermes_cli/test_web_oauth_dispatch.py diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 9f434819df..4a4b8d4b5a 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -1456,7 +1456,12 @@ _OAUTH_PROVIDER_CATALOG: tuple[Dict[str, Any], ...] = ( { "id": "minimax-oauth", "name": "MiniMax (OAuth)", - "flow": "pkce", + # MiniMax's flow is structurally device-code (verification URI + + # user code, backend polls the token endpoint) with a PKCE + # extension for code-binding. The dashboard renders the same UX + # as Nous's device-code flow; the PKCE bit is a security + # extension that doesn't change the operator experience. + "flow": "device_code", "cli_command": "hermes auth add minimax-oauth", "docs_url": "https://www.minimax.io", "status_fn": None, # dispatched via auth.get_minimax_oauth_auth_status @@ -1819,7 +1824,7 @@ def _submit_anthropic_pkce(session_id: str, code_input: str) -> Dict[str, Any]: async def _start_device_code_flow(provider_id: str) -> Dict[str, Any]: - """Initiate a device-code flow (Nous or OpenAI Codex). + """Initiate a device-code flow (Nous, OpenAI Codex, or MiniMax). Calls the provider's device-auth endpoint via the existing CLI helpers, then spawns a background poller. Returns the user-facing display fields @@ -1898,6 +1903,82 @@ async def _start_device_code_flow(provider_id: str) -> Dict[str, Any]: "poll_interval": int(s.get("interval") or 5), } + if provider_id == "minimax-oauth": + # MiniMax uses a device-code-style flow (verification URI + user + # code + background poll) with a PKCE extension on top. From the + # operator's perspective it's identical to Nous's device-code + # flow; the PKCE bit (verifier + challenge from + # _minimax_pkce_pair) is a security extension that binds the + # token exchange to the original session. + from hermes_cli.auth import ( + _minimax_pkce_pair, + _minimax_request_user_code, + MINIMAX_OAUTH_CLIENT_ID, + MINIMAX_OAUTH_GLOBAL_BASE, + ) + import httpx + verifier, challenge, state = _minimax_pkce_pair() + portal_base_url = ( + os.getenv("MINIMAX_PORTAL_BASE_URL") or MINIMAX_OAUTH_GLOBAL_BASE + ).rstrip("/") + def _do_minimax_request(): + with httpx.Client( + timeout=httpx.Timeout(15.0), + headers={"Accept": "application/json"}, + follow_redirects=True, + ) as client: + return _minimax_request_user_code( + client=client, + portal_base_url=portal_base_url, + client_id=MINIMAX_OAUTH_CLIENT_ID, + code_challenge=challenge, + state=state, + ) + device_data = await asyncio.get_event_loop().run_in_executor( + None, _do_minimax_request + ) + sid, sess = _new_oauth_session("minimax-oauth", "device_code") + # The CLI flow names this `interval_ms` because MiniMax's + # `interval` field is in milliseconds (defensive default 2000ms + # in _minimax_poll_token). + interval_raw = device_data.get("interval") + sess["interval_ms"] = ( + int(interval_raw) if interval_raw is not None else None + ) + sess["user_code"] = str(device_data["user_code"]) + sess["code_verifier"] = verifier + sess["state"] = state + sess["portal_base_url"] = portal_base_url + sess["client_id"] = MINIMAX_OAUTH_CLIENT_ID + sess["region"] = "global" + # `expired_in` from MiniMax is overloaded — could be a unix-ms + # timestamp OR a seconds-from-now duration. Mirror the heuristic + # in _minimax_poll_token. Stash the raw value for the poller; + # compute a derived expires_at + UI-friendly expires_in seconds. + expired_in_raw = int(device_data["expired_in"]) + sess["expired_in_raw"] = expired_in_raw + if expired_in_raw > 1_000_000_000_000: # likely unix-ms + expires_at_ts = expired_in_raw / 1000.0 + expires_in_seconds = max(0, int(expires_at_ts - time.time())) + else: + expires_at_ts = time.time() + expired_in_raw + expires_in_seconds = expired_in_raw + sess["expires_at"] = expires_at_ts + threading.Thread( + target=_minimax_poller, + args=(sid,), + daemon=True, + name=f"oauth-poll-{sid[:6]}", + ).start() + return { + "session_id": sid, + "flow": "device_code", + "user_code": str(device_data["user_code"]), + "verification_url": str(device_data["verification_uri"]), + "expires_in": expires_in_seconds, + "poll_interval": max(2, (sess["interval_ms"] or 2000) // 1000), + } + raise HTTPException(status_code=400, detail=f"Provider {provider_id} does not support device-code flow") @@ -1959,6 +2040,86 @@ def _nous_poller(session_id: str) -> None: sess["error_message"] = str(e) +def _minimax_poller(session_id: str) -> None: + """Background poller that drives a MiniMax OAuth flow to completion. + + Mirrors `_nous_poller` but calls the MiniMax-specific token endpoint, + which uses a PKCE-style ``code_verifier`` + ``user_code`` rather than + the ``device_code`` field used by Nous. On success, builds the same + auth_state dict that ``_minimax_oauth_login`` (the CLI flow) builds + and persists via ``_minimax_save_auth_state`` — so the dashboard + path leaves the system in the same state as + ``hermes auth add minimax-oauth``. + """ + from hermes_cli.auth import ( + _minimax_poll_token, + _minimax_save_auth_state, + MINIMAX_OAUTH_GLOBAL_INFERENCE, + MINIMAX_OAUTH_SCOPE, + ) + from datetime import datetime, timezone + import httpx + with _oauth_sessions_lock: + sess = _oauth_sessions.get(session_id) + if not sess: + return + portal_base_url = sess["portal_base_url"] + client_id = sess["client_id"] + user_code = sess["user_code"] + code_verifier = sess["code_verifier"] + interval_ms = sess.get("interval_ms") + expired_in_raw = sess["expired_in_raw"] + try: + with httpx.Client( + timeout=httpx.Timeout(15.0), + headers={"Accept": "application/json"}, + follow_redirects=True, + ) as client: + token_data = _minimax_poll_token( + client=client, + portal_base_url=portal_base_url, + client_id=client_id, + user_code=user_code, + code_verifier=code_verifier, + expired_in=expired_in_raw, + interval_ms=interval_ms, + ) + # Build the auth_state dict in the same shape as the CLI flow's + # `_minimax_oauth_login` so `_minimax_save_auth_state` writes + # the canonical record. Region is fixed to "global" for the + # dashboard path; cn-region operators can still use the CLI + # flow which supports `--region cn`. + now = datetime.now(timezone.utc) + expires_in_s = int(token_data["expired_in"]) + expires_at_ts = now.timestamp() + expires_in_s + auth_state = { + "provider": "minimax-oauth", + "region": sess.get("region", "global"), + "portal_base_url": portal_base_url, + "inference_base_url": MINIMAX_OAUTH_GLOBAL_INFERENCE, + "client_id": client_id, + "scope": MINIMAX_OAUTH_SCOPE, + "token_type": token_data.get("token_type", "Bearer"), + "access_token": token_data["access_token"], + "refresh_token": token_data["refresh_token"], + "resource_url": token_data.get("resource_url"), + "obtained_at": now.isoformat(), + "expires_at": datetime.fromtimestamp( + expires_at_ts, tz=timezone.utc + ).isoformat(), + "expires_in": expires_in_s, + } + _minimax_save_auth_state(auth_state) + with _oauth_sessions_lock: + sess["status"] = "approved" + _log.info("oauth/device: minimax login completed (session=%s)", session_id) + except Exception as e: + _log.warning("minimax device-code poll failed (session=%s): %s", session_id, e) + with _oauth_sessions_lock: + sess["status"] = "error" + sess["error_message"] = str(e) + + def _codex_full_login_worker(session_id: str) -> None: """Run the complete OpenAI Codex device-code flow. @@ -2111,7 +2272,13 @@ async def start_oauth_login(provider_id: str, request: Request): detail=f"{provider_id} uses an external CLI; run `{catalog_entry['cli_command']}` manually", ) try: - if catalog_entry["flow"] == "pkce": + # The pkce branch is gated on provider_id == "anthropic" because + # `_start_anthropic_pkce()` is hardcoded to the Anthropic flow. + # Routing any other future pkce-flagged provider through it would + # silently launch the Anthropic OAuth flow (the bug fixed in this + # change for MiniMax). New PKCE providers must add their own + # start function and an explicit branch here. + if catalog_entry["flow"] == "pkce" and provider_id == "anthropic": return _start_anthropic_pkce() if catalog_entry["flow"] == "device_code": return await _start_device_code_flow(provider_id) diff --git a/tests/hermes_cli/test_web_oauth_dispatch.py b/tests/hermes_cli/test_web_oauth_dispatch.py new file mode 100644 index 0000000000..6ebd0ad723 --- /dev/null +++ b/tests/hermes_cli/test_web_oauth_dispatch.py @@ -0,0 +1,129 @@ +"""Regression tests for the OAuth dispatcher in hermes_cli.web_server. + +Bug history (2026-05-09): the `_OAUTH_PROVIDER_CATALOG` had two entries +flagged ``flow: "pkce"`` — anthropic and minimax-oauth — and the +dispatcher ``start_oauth_login`` hardcoded ``_start_anthropic_pkce()`` +for any pkce-flagged provider. So clicking "Login" next to MiniMax in +the dashboard's Keys tab silently launched the Anthropic/Claude OAuth +flow. + +The fix: + 1. Catalog entry for minimax-oauth changed from ``flow: "pkce"`` to + ``flow: "device_code"`` (the actual UX is verification URI + user + code + background poll, with PKCE as a security extension). + 2. New MiniMax branch added to ``_start_device_code_flow``. + 3. Dispatcher tightened: pkce branch now requires + ``provider_id == "anthropic"``, so any future PKCE provider added + without an explicit branch gets a clean ``400 Unsupported flow`` + instead of silently launching Anthropic OAuth. + +These tests pin the corrected behavior. +""" +from unittest.mock import patch + +import pytest +from fastapi.testclient import TestClient + +from hermes_cli.web_server import _SESSION_TOKEN, app + +client = TestClient(app) +HEADERS = {"X-Hermes-Session-Token": _SESSION_TOKEN} + + +def test_minimax_login_does_not_launch_anthropic_flow(): + """Click 'Login' on MiniMax → MUST NOT return claude.ai auth_url.""" + fake_user_code_resp = { + "user_code": "ABCD-1234", + "verification_uri": "https://api.minimax.io/oauth/verify", + # `expired_in` < 1e12 so the heuristic treats it as seconds. + "expired_in": 600, + "interval": 2000, + "state": "stub-state", + } + with patch( + "hermes_cli.auth._minimax_request_user_code", + return_value=fake_user_code_resp, + ), patch( + "hermes_cli.auth._minimax_pkce_pair", + return_value=("verifier-stub", "challenge-stub", "stub-state"), + ): + resp = client.post( + "/api/providers/oauth/minimax-oauth/start", + headers=HEADERS, + ) + + assert resp.status_code == 200, resp.text + body = resp.json() + + # The bug used to return Anthropic's auth_url — make sure the response + # references neither the auth_url field nor anything Claude-related. + assert "auth_url" not in body + assert "claude.ai" not in str(body).lower() + + # And the response IS the device-code shape pointing at MiniMax. + assert body["flow"] == "device_code" + assert "minimax" in body["verification_url"].lower() + assert body["user_code"] == "ABCD-1234" + assert body["expires_in"] == 600 + + +def test_anthropic_pkce_branch_still_works(): + """Sanity: the dispatcher tightening doesn't break the legitimate Anthropic PKCE path.""" + fake_anthropic_response = { + "session_id": "stub-session", + "flow": "pkce", + "auth_url": "https://claude.ai/oauth/authorize?code=true&...", + "expires_in": 600, + } + with patch( + "hermes_cli.web_server._start_anthropic_pkce", + return_value=fake_anthropic_response, + ): + resp = client.post( + "/api/providers/oauth/anthropic/start", + headers=HEADERS, + ) + + assert resp.status_code == 200, resp.text + body = resp.json() + assert body["flow"] == "pkce" + assert "claude.ai" in body["auth_url"] + + +def test_unknown_pkce_provider_rejected_cleanly(): + """A future PKCE provider without an explicit branch must NOT silently route to Anthropic. + + Simulates a hypothetical catalog entry with ``flow: "pkce"`` and an + id other than "anthropic". The dispatcher should fall through past + the pkce branch (now gated on provider_id) and the device_code + branch, then hit "Unsupported flow" — proving the bug class is + structurally prevented. + """ + from hermes_cli import web_server as ws + + # Inject a hypothetical catalog entry that's pkce-flagged but isn't + # anthropic. This shape mirrors what would happen if a developer + # added a new provider entry without remembering to wire up its + # start function. + fake_entry = { + "id": "hypothetical-pkce-provider", + "name": "Hypothetical PKCE Provider", + "flow": "pkce", + "cli_command": "hermes auth add hypothetical-pkce-provider", + "docs_url": "https://example.com", + "status_fn": None, + } + original_catalog = ws._OAUTH_PROVIDER_CATALOG + try: + ws._OAUTH_PROVIDER_CATALOG = original_catalog + (fake_entry,) + resp = client.post( + "/api/providers/oauth/hypothetical-pkce-provider/start", + headers=HEADERS, + ) + finally: + ws._OAUTH_PROVIDER_CATALOG = original_catalog + + # Either 400 "Unsupported flow" (the explicit fall-through) or any + # 4xx — what we MUST NOT see is a 200 with claude.ai in the body. + assert resp.status_code >= 400, resp.text + assert "claude.ai" not in resp.text.lower() From 97a0e69df02b9425573711a6972a91d1deee994a Mon Sep 17 00:00:00 2001 From: Teknium1 <127238744+teknium1@users.noreply.github.com> Date: Mon, 11 May 2026 16:50:39 -0700 Subject: [PATCH 10/12] chore(release): add AUTHOR_MAP entry for ahmedbadr3 --- scripts/release.py | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/release.py b/scripts/release.py index ee99bd616d..768a79a483 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -53,6 +53,7 @@ AUTHOR_MAP = { "421774554@qq.com": "wuli666", "harish.kukreja@gmail.com": "counterposition", "1046611633@qq.com": "zhengyn0001", + "ahmed@abadr.net": "ahmedbadr3", "cleo@edaphic.xyz": "curiouscleo", "hirokazu.ogawa@kwansei.ac.jp": "hrkzogw", "datapod.k@gmail.com": "dandacompany", From ced1990c1cab2413e6778d0eb35f526b6b9c1359 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Mon, 11 May 2026 17:10:58 -0700 Subject: [PATCH 11/12] feat(computer-use): refresh cua-driver on `hermes update` + add `install --upgrade` (#24063) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cua-driver was only installed once on toolset enable: `_run_post_setup` early-returns when the binary is already on PATH, so upstream fixes (e.g. v0.1.6 Safari window-focus fix) never reached existing users without manual reinstall. Two refresh points now: - `hermes update` re-runs the upstream installer at the end of the update if cua-driver is on PATH (macOS-only, no-op otherwise). Ties driver freshness to the user-controlled update cadence — no startup latency, no per-launch GitHub API call. - `hermes computer-use install --upgrade` for manual force-refresh. The upstream `install.sh` always pulls the latest release, so re-running is the canonical upgrade path. No version-comparison logic needed. `hermes computer-use status` now shows the installed version, and points at `--upgrade` for refreshing. --- hermes_cli/main.py | 46 ++++- hermes_cli/tools_config.py | 172 +++++++++++++----- tests/hermes_cli/test_install_cua_driver.py | 115 ++++++++++++ website/docs/reference/cli-commands.md | 8 +- .../docs/user-guide/features/computer-use.md | 17 ++ 5 files changed, 308 insertions(+), 50 deletions(-) create mode 100644 tests/hermes_cli/test_install_cua_driver.py diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 009af00b6e..f70b7ea9d9 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -7801,6 +7801,22 @@ def _cmd_update_impl(args, gateway_mode: bool): except Exception as e: logger.debug("FHS PATH guard check failed: %s", e) + # Refresh the cua-driver binary used by the Computer Use toolset. + # The upstream installer is gated on macOS and on the binary already + # being on PATH, so this is a no-op for users who don't have it. + # Tying the refresh to ``hermes update`` gives users a predictable + # cadence (matches when they pull new agent code) without adding + # startup latency or a per-launch GitHub API call. + try: + if sys.platform == "darwin" and shutil.which("cua-driver"): + from hermes_cli.tools_config import install_cua_driver + + print() + print("→ Refreshing cua-driver (Computer Use)...") + install_cua_driver(upgrade=True) + except Exception as e: + logger.debug("cua-driver refresh failed: %s", e) + # Write exit code *before* the gateway restart attempt. # When running as ``hermes update --gateway`` (spawned by the gateway's # /update command), this process lives inside the gateway's systemd @@ -10801,10 +10817,19 @@ Examples: ) computer_use_sub = computer_use_parser.add_subparsers(dest="computer_use_action") - computer_use_sub.add_parser( + computer_use_install = computer_use_sub.add_parser( "install", help="Install or repair the cua-driver binary (macOS)", ) + computer_use_install.add_argument( + "--upgrade", + action="store_true", + help=( + "Re-run the upstream installer even if cua-driver is already on " + "PATH. The upstream install.sh always pulls the latest release, " + "so this performs an in-place upgrade." + ), + ) computer_use_sub.add_parser( "status", help="Print whether cua-driver is installed and on PATH", @@ -10813,14 +10838,27 @@ Examples: def cmd_computer_use(args): action = getattr(args, "computer_use_action", None) if action == "install": - from hermes_cli.tools_config import _run_post_setup - _run_post_setup("cua_driver") + from hermes_cli.tools_config import install_cua_driver + install_cua_driver(upgrade=bool(getattr(args, "upgrade", False))) return if action == "status": import shutil + import subprocess path = shutil.which("cua-driver") if path: - print(f"cua-driver: installed at {path}") + version = "" + try: + version = subprocess.run( + ["cua-driver", "--version"], + capture_output=True, text=True, timeout=5, + ).stdout.strip() + except Exception: + pass + if version: + print(f"cua-driver: installed at {path} ({version})") + else: + print(f"cua-driver: installed at {path}") + print(" Refresh to latest: hermes computer-use install --upgrade") return print("cua-driver: not installed") print(" Run: hermes computer-use install") diff --git a/hermes_cli/tools_config.py b/hermes_cli/tools_config.py index 81e4d327c0..ba44d03c10 100644 --- a/hermes_cli/tools_config.py +++ b/hermes_cli/tools_config.py @@ -591,6 +591,132 @@ def _pip_install( ) +def install_cua_driver(upgrade: bool = False) -> bool: + """Install or refresh the cua-driver binary used by Computer Use. + + The upstream installer always pulls the latest release tag, so re-running + it is the canonical way to upgrade. We expose two modes: + + * ``upgrade=False`` — original post-setup behaviour: skip if already + installed, install otherwise. Used by the toolset enable flow where + we don't want to surprise the user with a network fetch. + * ``upgrade=True`` — always re-run the installer (or call ``cua-driver + update`` if the binary supports it). Used by ``hermes update`` and + by ``hermes computer-use install --upgrade``. + + Returns True iff cua-driver is installed (or successfully refreshed) + when the function returns. macOS-only — silently returns False on + other platforms. + """ + import platform as _plat + import shutil + import subprocess + + if _plat.system() != "Darwin": + if upgrade: + # Silent on non-macOS — `hermes update` calls this for every + # user; only macOS users with cua-driver care. + return False + _print_warning(" Computer Use (cua-driver) is macOS-only; skipping.") + return False + + binary = shutil.which("cua-driver") + + # Not installed → fresh install path (only when caller asked for it). + if not binary and not upgrade: + if not shutil.which("curl"): + _print_warning(" curl not found — install manually:") + _print_info(" https://github.com/trycua/cua/blob/main/libs/cua-driver/README.md") + return False + return _run_cua_driver_installer(label="Installing") + + # Already installed and caller didn't ask to upgrade → just confirm. + if binary and not upgrade: + try: + version = subprocess.run( + ["cua-driver", "--version"], + capture_output=True, text=True, timeout=5, + ).stdout.strip() + _print_success(f" cua-driver already installed: {version or 'unknown version'}") + except Exception: + _print_success(" cua-driver already installed.") + _print_info(" Grant macOS permissions if not done yet:") + _print_info(" System Settings > Privacy & Security > Accessibility") + _print_info(" System Settings > Privacy & Security > Screen Recording") + return True + + # upgrade=True path — refresh to the latest upstream release. + if not shutil.which("curl"): + _print_warning(" curl not found — cannot refresh cua-driver.") + return bool(binary) + + if binary: + # Show before/after version when we have a baseline. Best-effort. + try: + before = subprocess.run( + ["cua-driver", "--version"], + capture_output=True, text=True, timeout=5, + ).stdout.strip() + except Exception: + before = "" + else: + before = "" + + ok = _run_cua_driver_installer(label="Refreshing", verbose=False) + if ok and before: + try: + after = subprocess.run( + ["cua-driver", "--version"], + capture_output=True, text=True, timeout=5, + ).stdout.strip() + if after and after != before: + _print_success(f" cua-driver upgraded: {before} → {after}") + elif after: + _print_info(f" cua-driver up to date: {after}") + except Exception: + pass + return ok + + +def _run_cua_driver_installer(label: str = "Installing", verbose: bool = True) -> bool: + """Run the upstream cua-driver install.sh. Returns True on success. + + The script is idempotent: it always downloads the latest release, so + re-running it on an already-installed system performs an upgrade. + """ + import shutil + import subprocess + + install_cmd = ( + "/bin/bash -c \"$(curl -fsSL " + "https://raw.githubusercontent.com/trycua/cua/main/" + "libs/cua-driver/scripts/install.sh)\"" + ) + if verbose: + _print_info(f" {label} cua-driver (macOS background computer-use)...") + else: + _print_info(f" {label} cua-driver...") + try: + result = subprocess.run(install_cmd, shell=True, timeout=300) + if result.returncode == 0 and shutil.which("cua-driver"): + if verbose: + _print_success(" cua-driver installed.") + _print_info(" IMPORTANT — grant macOS permissions now:") + _print_info(" System Settings > Privacy & Security > Accessibility") + _print_info(" System Settings > Privacy & Security > Screen Recording") + _print_info(" Both must allow the terminal / Hermes process.") + return True + _print_warning(f" cua-driver {label.lower()} did not complete. Re-run manually:") + _print_info(f" {install_cmd}") + return False + except subprocess.TimeoutExpired: + _print_warning(f" cua-driver {label.lower()} timed out. Re-run manually.") + return False + except Exception as e: + _print_warning(f" cua-driver {label.lower()} failed: {e}") + return False + + def _run_post_setup(post_setup_key: str): """Run post-setup hooks for tools that need extra installation steps.""" import shutil @@ -729,51 +855,7 @@ def _run_post_setup(post_setup_key: str): _print_info(" docker run -p 9377:9377 -e CAMOFOX_PORT=9377 jo-inc/camofox-browser") elif post_setup_key == "cua_driver": - # cua-driver provides macOS background computer-use (SkyLight SPIs). - # Install via upstream curl script if the binary isn't on $PATH yet. - import platform as _plat - import subprocess - if _plat.system() != "Darwin": - _print_warning(" Computer Use (cua-driver) is macOS-only; skipping.") - return - if shutil.which("cua-driver"): - try: - version = subprocess.run( - ["cua-driver", "--version"], - capture_output=True, text=True, timeout=5, - ).stdout.strip() - _print_success(f" cua-driver already installed: {version or 'unknown version'}") - except Exception: - _print_success(" cua-driver already installed.") - _print_info(" Grant macOS permissions if not done yet:") - _print_info(" System Settings > Privacy & Security > Accessibility") - _print_info(" System Settings > Privacy & Security > Screen Recording") - return - if not shutil.which("curl"): - _print_warning(" curl not found — install manually:") - _print_info(" https://github.com/trycua/cua/blob/main/libs/cua-driver/README.md") - return - _print_info(" Installing cua-driver (macOS background computer-use)...") - try: - install_cmd = ( - "/bin/bash -c \"$(curl -fsSL " - "https://raw.githubusercontent.com/trycua/cua/main/" - "libs/cua-driver/scripts/install.sh)\"" - ) - result = subprocess.run(install_cmd, shell=True, timeout=300) - if result.returncode == 0 and shutil.which("cua-driver"): - _print_success(" cua-driver installed.") - _print_info(" IMPORTANT — grant macOS permissions now:") - _print_info(" System Settings > Privacy & Security > Accessibility") - _print_info(" System Settings > Privacy & Security > Screen Recording") - _print_info(" Both must allow the terminal / Hermes process.") - else: - _print_warning(" cua-driver install did not complete. Re-run manually:") - _print_info(f" {install_cmd}") - except subprocess.TimeoutExpired: - _print_warning(" cua-driver install timed out. Re-run manually.") - except Exception as e: - _print_warning(f" cua-driver install failed: {e}") + install_cua_driver(upgrade=False) elif post_setup_key == "kittentts": try: diff --git a/tests/hermes_cli/test_install_cua_driver.py b/tests/hermes_cli/test_install_cua_driver.py new file mode 100644 index 0000000000..42a49e22b5 --- /dev/null +++ b/tests/hermes_cli/test_install_cua_driver.py @@ -0,0 +1,115 @@ +"""Tests for ``install_cua_driver`` upgrade semantics. + +The cua-driver upstream installer always pulls the latest release tag, so +re-running it is the canonical upgrade path. ``install_cua_driver(upgrade=True)`` +must: + +* Be macOS-only — no-op silently on Linux/Windows so ``hermes update`` can + call it unconditionally without warning every non-macOS user. +* Re-run the installer even when the binary is already on PATH (this is the + fix for the "we only pulled cua-driver once on enable" complaint). +* Preserve original ``upgrade=False`` behaviour for the toolset-enable flow: + skip if installed, install otherwise, warn on non-macOS. +""" + +from __future__ import annotations + +from unittest.mock import patch + + +class TestInstallCuaDriverUpgrade: + def test_upgrade_on_non_macos_is_silent_noop(self): + """``hermes update`` calls install_cua_driver(upgrade=True) for every + user. On Linux/Windows it must return False without printing the + "macOS-only; skipping" warning that the toolset-enable path emits.""" + from hermes_cli import tools_config + + with patch.object(tools_config, "_print_warning") as warn, \ + patch("platform.system", return_value="Linux"): + assert tools_config.install_cua_driver(upgrade=True) is False + warn.assert_not_called() + + def test_non_upgrade_on_non_macos_warns(self): + """The toolset-enable path (upgrade=False) should still warn loudly + when the user tries to enable Computer Use on a non-macOS host.""" + from hermes_cli import tools_config + + with patch.object(tools_config, "_print_warning") as warn, \ + patch("platform.system", return_value="Linux"): + assert tools_config.install_cua_driver(upgrade=False) is False + warn.assert_called() + + def test_upgrade_on_macos_with_binary_runs_installer(self): + """When cua-driver is already on PATH and upgrade=True, we must + re-run the upstream installer (this is the fix for the bug report). + """ + from hermes_cli import tools_config + + with patch("platform.system", return_value="Darwin"), \ + patch.object(tools_config.shutil, "which", + side_effect=lambda n: "/usr/local/bin/" + n + if n in ("cua-driver", "curl") else None), \ + patch.object(tools_config, "_run_cua_driver_installer", + return_value=True) as runner, \ + patch("subprocess.run"): + assert tools_config.install_cua_driver(upgrade=True) is True + runner.assert_called_once() + # Refresh path uses non-verbose mode so we don't re-print the + # "grant macOS permissions" block on every `hermes update`. + kwargs = runner.call_args.kwargs + assert kwargs.get("verbose") is False + + def test_upgrade_on_macos_without_binary_runs_installer(self): + """upgrade=True with cua-driver missing must still trigger an + install — equivalent to a fresh install. (Don't silently no-op.)""" + from hermes_cli import tools_config + + with patch("platform.system", return_value="Darwin"), \ + patch.object(tools_config.shutil, "which", + side_effect=lambda n: "/usr/bin/curl" if n == "curl" else None), \ + patch.object(tools_config, "_run_cua_driver_installer", + return_value=True) as runner: + assert tools_config.install_cua_driver(upgrade=True) is True + runner.assert_called_once() + + def test_non_upgrade_on_macos_with_binary_skips_install(self): + """Original toolset-enable behaviour: cua-driver already installed + + upgrade=False → confirm and return without re-running installer. + This is the behaviour that ``hermes tools`` (re)enable depends on, + so the new helper must not regress it.""" + from hermes_cli import tools_config + + with patch("platform.system", return_value="Darwin"), \ + patch.object(tools_config.shutil, "which", + side_effect=lambda n: "/usr/local/bin/" + n + if n in ("cua-driver", "curl") else None), \ + patch.object(tools_config, "_run_cua_driver_installer") as runner, \ + patch("subprocess.run"): + assert tools_config.install_cua_driver(upgrade=False) is True + runner.assert_not_called() + + def test_non_upgrade_on_macos_without_binary_runs_installer(self): + """Original fresh-install path must still work.""" + from hermes_cli import tools_config + + with patch("platform.system", return_value="Darwin"), \ + patch.object(tools_config.shutil, "which", + side_effect=lambda n: "/usr/bin/curl" if n == "curl" else None), \ + patch.object(tools_config, "_run_cua_driver_installer", + return_value=True) as runner: + assert tools_config.install_cua_driver(upgrade=False) is True + runner.assert_called_once() + + def test_upgrade_without_curl_does_not_crash(self): + """If curl isn't on PATH we can't refresh — must warn and return + the current install state, not raise.""" + from hermes_cli import tools_config + + # cua-driver present, curl missing. + def _which(name): + return "/usr/local/bin/cua-driver" if name == "cua-driver" else None + + with patch("platform.system", return_value="Darwin"), \ + patch.object(tools_config.shutil, "which", side_effect=_which), \ + patch.object(tools_config, "_print_warning"): + assert tools_config.install_cua_driver(upgrade=True) is True diff --git a/website/docs/reference/cli-commands.md b/website/docs/reference/cli-commands.md index ed15665d66..1079bdf3ca 100644 --- a/website/docs/reference/cli-commands.md +++ b/website/docs/reference/cli-commands.md @@ -976,7 +976,8 @@ Subcommands: | Subcommand | Description | |------------|-------------| | `install` | Run the upstream cua-driver installer (macOS only). | -| `status` | Print whether `cua-driver` is on `$PATH`. | +| `install --upgrade` | Re-run the installer even if cua-driver is already on PATH. The upstream script always pulls the latest release, so this performs an in-place upgrade. | +| `status` | Print whether `cua-driver` is on `$PATH` and which version is installed. | `hermes computer-use install` is the stable entry point for installing the [cua-driver](https://github.com/trycua/cua) binary used by the @@ -985,6 +986,11 @@ Subcommands: to use for re-running the install if the toolset toggle didn't trigger it (for example, on returning-user setups). +`hermes update` automatically re-runs the upstream installer at the end +of the update if cua-driver is on PATH, so most users will not need to +call `--upgrade` manually. Use it when upstream ships a fix you want +right now without waiting for the next Hermes update. + ## `hermes sessions` ```bash diff --git a/website/docs/user-guide/features/computer-use.md b/website/docs/user-guide/features/computer-use.md index e4c2858696..d05ff95465 100644 --- a/website/docs/user-guide/features/computer-use.md +++ b/website/docs/user-guide/features/computer-use.md @@ -57,6 +57,23 @@ After installing, regardless of which path you took: ``` or add `computer_use` to your enabled toolsets in `~/.hermes/config.yaml`. +## Keeping cua-driver up to date + +The cua-driver project ships fixes regularly (e.g. v0.1.6 fixed a Safari +window-focus bug for UTM workflows). Hermes refreshes the binary in two +places so you don't get stuck on a stale release: + +- **`hermes update`** — when you update Hermes itself, if `cua-driver` is + on PATH the upstream installer re-runs at the end of the update. + No-op for non-macOS users and for users without cua-driver installed. +- **`hermes computer-use install --upgrade`** — manual force-refresh. + Re-runs the upstream installer regardless of whether cua-driver is + already installed. Use this when you want the latest fix without + waiting for the next agent update. + +`hermes computer-use status` shows the installed version next to the +binary path. + ## Quick example User prompt: *"Find my latest email from Stripe and summarise what they want me to do."* From e85592591e8028cceecb0ea2b4992a1643b52f93 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Mon, 11 May 2026 18:08:16 -0700 Subject: [PATCH 12/12] fix(nous): surface Portal-flagged free models in picker even when curated list is stale (#24082) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Free-tier users were seeing 'No free models currently available.' in the `hermes model` and post-login pickers even though qwen/qwen3.6-plus is free on the Portal right now. Three independent breakages compounded: 1. The docs-hosted catalog manifest at website/static/api/model-catalog.json was not regenerated when _PROVIDER_MODELS['nous'] was updated, so users fetching the manifest got a list that didn't include qwen/qwen3.6-plus. 2. _resolve_nous_pricing_credentials() returned ('', '') on any auth blip, collapsing get_pricing_for_provider('nous') to {} and making every curated model fall through the free-tier filter as 'paid'. 3. Even with healthy pricing, the picker only ever showed models from the in-repo curated list intersected with live pricing — a Portal-flagged free model not yet in the curated list could never appear. Changes: - hermes_cli/models.py: new union_with_portal_free_recommendations() that augments the curated list with Portal freeRecommendedModels entries (with synthetic free pricing so partition keeps them). The Portal's /api/nous/recommended-models endpoint is now the source of truth for free-tier surfacing — old Hermes builds will see new free models without a CLI release. - hermes_cli/models.py: _resolve_nous_pricing_credentials() falls back to the public inference base URL when runtime cred resolution fails. The /v1/models endpoint exposes pricing without auth, so silently returning {} just because a refresh token expired was wrong. - hermes_cli/auth.py + hermes_cli/main.py: both free-tier picker call sites call union_with_portal_free_recommendations() before partition. - tests/hermes_cli/test_models.py: 7 tests covering union behaviour (prepend, dedup, end-to-end with stale pricing, empty/missing/error payloads, invalid entries). - tests/hermes_cli/test_model_catalog.py: drift guard TestManifestMatchesInRepoLists fails CI when _PROVIDER_MODELS['nous'] or OPENROUTER_MODELS is edited without re-running scripts/build_model_catalog.py. Verified empirically that removing a manifest entry triggers an assertion with an actionable error message. Validation: - 133/133 targeted tests pass (test_models, test_model_catalog, test_auth_nous_provider). - Live E2E against the real Portal: - Stale curated list ['claude-opus','claude-sonnet','gpt-5.4'] (no qwen) → after union: ['qwen/qwen3.6-plus', ...] → partition(free_tier=True): selectable=['qwen/qwen3.6-plus']. - Simulated expired refresh token → anon fetch returns 403 pricing entries including qwen/qwen3.6-plus -> {prompt:0, completion:0}. - ruff: clean. --- hermes_cli/auth.py | 10 ++ hermes_cli/main.py | 34 ++++--- hermes_cli/models.py | 82 ++++++++++++++++- tests/hermes_cli/test_model_catalog.py | 55 +++++++++++ tests/hermes_cli/test_models.py | 123 +++++++++++++++++++++++++ 5 files changed, 289 insertions(+), 15 deletions(-) diff --git a/hermes_cli/auth.py b/hermes_cli/auth.py index 7db897cb55..6fda05d8fd 100644 --- a/hermes_cli/auth.py +++ b/hermes_cli/auth.py @@ -5251,6 +5251,7 @@ def _login_nous(args, pconfig: ProviderConfig) -> None: from hermes_cli.models import ( get_curated_nous_model_ids, get_pricing_for_provider, check_nous_free_tier, partition_nous_models_by_tier, + union_with_portal_free_recommendations, ) model_ids = get_curated_nous_model_ids() @@ -5260,6 +5261,15 @@ def _login_nous(args, pconfig: ProviderConfig) -> None: pricing = get_pricing_for_provider("nous") free_tier = check_nous_free_tier() if free_tier: + # The Portal's freeRecommendedModels endpoint is the + # source of truth for what's free *right now*. Augment + # the curated list with anything new the Portal flags + # as free so users on older Hermes builds still see + # newly-launched free models without a CLI release. + _portal_for_recs = auth_state.get("portal_base_url", "") + model_ids, pricing = union_with_portal_free_recommendations( + model_ids, pricing, _portal_for_recs, + ) model_ids, unavailable_models = partition_nous_models_by_tier( model_ids, pricing, free_tier=True, ) diff --git a/hermes_cli/main.py b/hermes_cli/main.py index f70b7ea9d9..3c0ab4c442 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -2589,6 +2589,7 @@ def _model_flow_nous(config, current_model="", args=None): get_pricing_for_provider, check_nous_free_tier, partition_nous_models_by_tier, + union_with_portal_free_recommendations, ) model_ids = get_curated_nous_model_ids() @@ -2629,19 +2630,8 @@ def _model_flow_nous(config, current_model="", args=None): # Check if user is on free tier free_tier = check_nous_free_tier() - # For free users: partition models into selectable/unavailable based on - # whether they are free per the Portal-reported pricing. - unavailable_models: list[str] = [] - if free_tier: - model_ids, unavailable_models = partition_nous_models_by_tier( - model_ids, pricing, free_tier=True - ) - - if not model_ids and not unavailable_models: - print("No models available for Nous Portal after filtering.") - return - - # Resolve portal URL for upgrade links (may differ on staging) + # Resolve portal URL early — needed both for upgrade links and for the + # freeRecommendedModels endpoint below. _nous_portal_url = "" try: _nous_state = get_provider_auth_state("nous") @@ -2650,6 +2640,24 @@ def _model_flow_nous(config, current_model="", args=None): except Exception: pass + # For free users: partition models into selectable/unavailable based on + # whether they are free per the Portal-reported pricing. First augment + # with the Portal's freeRecommendedModels list so newly-launched free + # models show up even if this CLI build's hardcoded curated list and + # docs-hosted manifest haven't caught up yet. + unavailable_models: list[str] = [] + if free_tier: + model_ids, pricing = union_with_portal_free_recommendations( + model_ids, pricing, _nous_portal_url, + ) + model_ids, unavailable_models = partition_nous_models_by_tier( + model_ids, pricing, free_tier=True + ) + + if not model_ids and not unavailable_models: + print("No models available for Nous Portal after filtering.") + return + if free_tier and not model_ids: print("No free models currently available.") if unavailable_models: diff --git a/hermes_cli/models.py b/hermes_cli/models.py index c23bd397e3..813045dfd0 100644 --- a/hermes_cli/models.py +++ b/hermes_cli/models.py @@ -556,6 +556,71 @@ def partition_nous_models_by_tier( return (selectable, unavailable) +def union_with_portal_free_recommendations( + curated_ids: list[str], + pricing: dict[str, dict[str, str]], + portal_base_url: str = "", + *, + force_refresh: bool = False, +) -> tuple[list[str], dict[str, dict[str, str]]]: + """Augment curated list + pricing with the Portal's ``freeRecommendedModels``. + + The Portal's ``/api/nous/recommended-models`` endpoint advertises which + models are free *right now* — independent of what the in-repo + ``_PROVIDER_MODELS["nous"]`` list happens to contain or whether the + docs-hosted catalog manifest has been rebuilt since the last release. + + For free-tier users this is the source of truth: any model the Portal + flags as free should be selectable, even if the user is running an + older Hermes that doesn't ship that model in its hardcoded curated + list. This function returns an augmented ``(model_ids, pricing)`` + pair where: + + * Portal free recommendations missing from ``curated_ids`` are + appended at the front (so the picker shows them first). + * ``pricing`` gets a synthetic ``{"prompt": "0", "completion": "0"}`` + entry for any free recommendation missing from the live pricing + map, so :func:`partition_nous_models_by_tier` keeps it. + + Failures (network, parse, missing field) are silent and degrade to + returning the inputs unchanged. + """ + try: + payload = fetch_nous_recommended_models( + portal_base_url, force_refresh=force_refresh + ) + except Exception: + return (list(curated_ids), dict(pricing)) + + free_block = payload.get("freeRecommendedModels") if isinstance(payload, dict) else None + if not isinstance(free_block, list) or not free_block: + return (list(curated_ids), dict(pricing)) + + portal_free_ids: list[str] = [] + for entry in free_block: + name = _extract_model_name(entry) + if name: + portal_free_ids.append(name) + if not portal_free_ids: + return (list(curated_ids), dict(pricing)) + + augmented_pricing = dict(pricing) + free_synthetic = {"prompt": "0", "completion": "0"} + for mid in portal_free_ids: + if mid not in augmented_pricing: + augmented_pricing[mid] = dict(free_synthetic) + + augmented_ids = list(curated_ids) + seen = set(augmented_ids) + # Prepend Portal free recommendations that aren't already curated, so + # they appear first in the picker. + new_ones = [mid for mid in portal_free_ids if mid not in seen] + if new_ones: + augmented_ids = new_ones + augmented_ids + + return (augmented_ids, augmented_pricing) + + # --------------------------------------------------------------------------- # TTL cache for free-tier detection — avoids repeated API calls within a # session while still picking up upgrades quickly. @@ -1338,8 +1403,21 @@ def _resolve_openrouter_api_key() -> str: return os.getenv("OPENROUTER_API_KEY", "").strip() +_DEFAULT_NOUS_INFERENCE_BASE = "https://inference-api.nousresearch.com" + + def _resolve_nous_pricing_credentials() -> tuple[str, str]: - """Return ``(api_key, base_url)`` for Nous Portal pricing, or empty strings.""" + """Return ``(api_key, base_url)`` for Nous Portal pricing. + + The Nous inference ``/v1/models`` endpoint exposes pricing without + authentication, so the api_key is best-effort: when runtime credential + resolution fails (expired refresh token, missing auth.json, etc.) we + still return the default inference base URL so the picker keeps + working with anonymous pricing data. Free-tier users in particular + need this — pricing drives the free/paid partition, and silently + returning empty pricing because of an auth blip makes the picker + look broken ("No free models currently available"). + """ try: from hermes_cli.auth import resolve_nous_runtime_credentials creds = resolve_nous_runtime_credentials() @@ -1347,7 +1425,7 @@ def _resolve_nous_pricing_credentials() -> tuple[str, str]: return (creds.get("api_key", ""), creds.get("base_url", "")) except Exception: pass - return ("", "") + return ("", _DEFAULT_NOUS_INFERENCE_BASE) def get_pricing_for_provider(provider: str, *, force_refresh: bool = False) -> dict[str, dict[str, str]]: diff --git a/tests/hermes_cli/test_model_catalog.py b/tests/hermes_cli/test_model_catalog.py index 8910705c74..d4a4b7237a 100644 --- a/tests/hermes_cli/test_model_catalog.py +++ b/tests/hermes_cli/test_model_catalog.py @@ -328,3 +328,58 @@ class TestIntegrationWithModelsModule: "anthropic/claude-opus-4.7", "moonshotai/kimi-k2.6", ] + + +# ----------------------------------------------------------------------------- +# Drift guard — prevent the in-repo curated lists from going out of sync with +# the docs-hosted manifest at website/static/api/model-catalog.json. +# +# History: qwen/qwen3.6-plus was added to _PROVIDER_MODELS["nous"] in commit +# 9dd6e5510 but website/static/api/model-catalog.json was not regenerated for +# weeks, so free-tier users on a new install fetched a stale manifest and the +# free-tier picker showed "No free models currently available." even though +# the Portal was serving qwen/qwen3.6-plus as free. CI must catch this. +# ----------------------------------------------------------------------------- + + +class TestManifestMatchesInRepoLists: + """Fail if the on-disk manifest is out of date relative to in-repo lists.""" + + @staticmethod + def _strip_volatile(catalog: dict) -> dict: + """Drop fields that always change (timestamps) for diff comparison.""" + out = dict(catalog) + out.pop("updated_at", None) + return out + + def test_in_repo_lists_match_manifest(self): + """``scripts/build_model_catalog.py`` output must match the committed file. + + If this fails, run ``python scripts/build_model_catalog.py`` and + commit the regenerated ``website/static/api/model-catalog.json``. + """ + # Resolve the repo root from this test file's location. + repo_root = Path(__file__).resolve().parents[2] + manifest_path = repo_root / "website" / "static" / "api" / "model-catalog.json" + + if not manifest_path.exists(): + pytest.skip(f"manifest missing at {manifest_path}") + + # Build expected catalog using the same script CI would. + import importlib.util + script_path = repo_root / "scripts" / "build_model_catalog.py" + spec = importlib.util.spec_from_file_location("_build_model_catalog", script_path) + mod = importlib.util.module_from_spec(spec) + assert spec.loader is not None + spec.loader.exec_module(mod) + expected = mod.build_catalog() + + with open(manifest_path, encoding="utf-8") as fh: + actual = json.load(fh) + + assert self._strip_volatile(actual) == self._strip_volatile(expected), ( + "website/static/api/model-catalog.json is out of sync with " + "_PROVIDER_MODELS['nous'] / OPENROUTER_MODELS. " + "Run: python scripts/build_model_catalog.py && " + "git add website/static/api/model-catalog.json" + ) diff --git a/tests/hermes_cli/test_models.py b/tests/hermes_cli/test_models.py index d0201a3e80..668105bf10 100644 --- a/tests/hermes_cli/test_models.py +++ b/tests/hermes_cli/test_models.py @@ -6,6 +6,7 @@ from hermes_cli.models import ( OPENROUTER_MODELS, fetch_openrouter_models, model_ids, detect_provider_for_model, is_nous_free_tier, partition_nous_models_by_tier, check_nous_free_tier, _FREE_TIER_CACHE_TTL, + union_with_portal_free_recommendations, ) import hermes_cli.models as _models_mod @@ -383,6 +384,128 @@ class TestPartitionNousModelsByTier: assert unav == models +class TestUnionWithPortalFreeRecommendations: + """Tests for union_with_portal_free_recommendations. + + The Portal's freeRecommendedModels endpoint is the source of truth for + what's free *right now* — the in-repo curated list and docs-hosted + manifest can lag. This helper guarantees the picker still surfaces + Portal-flagged free models even when the rest of the catalog is stale. + """ + + _PAID = {"prompt": "0.000003", "completion": "0.000015"} + _FREE = {"prompt": "0", "completion": "0"} + + def _payload(self, free_models: list[str]) -> dict: + return { + "freeRecommendedModels": [ + {"modelName": mid, "displayName": mid} for mid in free_models + ], + } + + def test_adds_portal_free_model_missing_from_curated(self): + """A Portal-advertised free model not in curated is prepended + priced free.""" + curated = ["anthropic/claude-opus-4.6"] + pricing = {"anthropic/claude-opus-4.6": self._PAID} + with patch( + "hermes_cli.models.fetch_nous_recommended_models", + return_value=self._payload(["qwen/qwen3.6-plus"]), + ): + ids, p = union_with_portal_free_recommendations(curated, pricing, "") + + assert ids[0] == "qwen/qwen3.6-plus" # prepended + assert "anthropic/claude-opus-4.6" in ids + # Synthetic free pricing entry created + assert p["qwen/qwen3.6-plus"] == self._FREE + # Existing pricing untouched + assert p["anthropic/claude-opus-4.6"] == self._PAID + + def test_does_not_duplicate_curated_entries(self): + """A Portal free model already in curated is not duplicated.""" + curated = ["qwen/qwen3.6-plus", "anthropic/claude-opus-4.6"] + pricing = { + "qwen/qwen3.6-plus": self._FREE, + "anthropic/claude-opus-4.6": self._PAID, + } + with patch( + "hermes_cli.models.fetch_nous_recommended_models", + return_value=self._payload(["qwen/qwen3.6-plus"]), + ): + ids, p = union_with_portal_free_recommendations(curated, pricing, "") + + assert ids == curated + assert p == pricing + + def test_then_partition_keeps_portal_free_model(self): + """End-to-end: Portal-flagged free model survives partition.""" + # Simulate the broken-state-before-this-fix: in-repo curated list + # contains qwen/qwen3.6-plus (because new builds shipped it) but + # live pricing endpoint hasn't published its zero-cost entry yet. + # The Portal's freeRecommendedModels still flags it as free. + curated = ["qwen/qwen3.6-plus", "anthropic/claude-opus-4.6"] + pricing = {"anthropic/claude-opus-4.6": self._PAID} # qwen missing! + with patch( + "hermes_cli.models.fetch_nous_recommended_models", + return_value=self._payload(["qwen/qwen3.6-plus"]), + ): + ids, p = union_with_portal_free_recommendations(curated, pricing, "") + sel, unav = partition_nous_models_by_tier(ids, p, free_tier=True) + assert "qwen/qwen3.6-plus" in sel + assert "anthropic/claude-opus-4.6" in unav + + def test_empty_payload_returns_inputs_unchanged(self): + """Empty Portal response leaves curated + pricing untouched.""" + curated = ["a", "b"] + pricing = {"a": self._PAID} + with patch("hermes_cli.models.fetch_nous_recommended_models", return_value={}): + ids, p = union_with_portal_free_recommendations(curated, pricing, "") + assert ids == curated + assert p == pricing + + def test_missing_freeRecommendedModels_key(self): + """Portal payload without freeRecommendedModels degrades gracefully.""" + curated = ["a"] + pricing = {"a": self._PAID} + with patch( + "hermes_cli.models.fetch_nous_recommended_models", + return_value={"paidRecommendedModels": [{"modelName": "x"}]}, + ): + ids, p = union_with_portal_free_recommendations(curated, pricing, "") + assert ids == curated + assert p == pricing + + def test_fetch_failure_returns_inputs(self): + """Network failures don't blow up the picker.""" + curated = ["a"] + pricing = {"a": self._PAID} + with patch( + "hermes_cli.models.fetch_nous_recommended_models", + side_effect=RuntimeError("network down"), + ): + ids, p = union_with_portal_free_recommendations(curated, pricing, "") + assert ids == curated + assert p == pricing + + def test_invalid_entries_skipped(self): + """Non-dict / missing-modelName entries are filtered out.""" + curated = ["a"] + pricing = {"a": self._PAID} + with patch( + "hermes_cli.models.fetch_nous_recommended_models", + return_value={ + "freeRecommendedModels": [ + "not-a-dict", + {"displayName": "no-modelName"}, + {"modelName": ""}, + {"modelName": "qwen/qwen3.6-plus"}, + ] + }, + ): + ids, p = union_with_portal_free_recommendations(curated, pricing, "") + assert ids == ["qwen/qwen3.6-plus", "a"] + assert p["qwen/qwen3.6-plus"] == self._FREE + + class TestCheckNousFreeTierCache: """Tests for the TTL cache on check_nous_free_tier()."""