KrustyPlanet/hermes-agent
6
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

test: port JS/package.json invariant tests from Python to vitest

Browse source 
The CI change classifier routes package.json / package-lock.json /
.ts/.tsx changes to the frontend lane, not the Python lane. Four
Python tests asserted about these JS-side artifacts, so a PR touching
only those files would skip the Python suite — the regression goes
green on the PR and red on main (where the classifier fails open).

Ported all four to vitest so they run in the correct CI lane:

  tests/test_package_json_lazy_deps.py
    → apps/desktop/electron/package-json-lazy-deps.test.ts
    (camofox is lazy, agent-browser is eager, lockfile clean)

  tests/test_desktop_electron_pin.py
    → apps/desktop/electron/desktop-electron-pin.test.ts
    (electron dep is exact, matches build.electronVersion, lockfile agrees)

  tests/test_assistant_ui_tap_compat.py
    → apps/desktop/electron/assistant-ui-tap-compat.test.ts
    (@assistant-ui cluster shares one tap version + semver helper)

  tests/test_dashboard_sidecar_close_on_disconnect.py
    → web/src/lib/chat-sidebar-session-params.test.ts
    (sidecar session.create opts into close_on_disconnect + profile)

The ChatSidebar test was regex-matching .tsx source text (the
source-reading anti-pattern). Extracted sidecarSessionCreateParams()
from the component's effect into an exported pure function so the
test calls real code instead of pattern-matching a string.

Verified: 8 electron tests + 76 web tests pass; both typecheck clean.
This commit is contained in:
ethernet 2026-07-15 12:14:10 -04:00
parent f8abc521f3
commit 2f3007ff51
9 changed files with 391 additions and 370 deletions
Download patch file Download diff file Expand all files Collapse all files

154
apps/desktop/electron/assistant-ui-tap-compat.test.ts Normal file
View file

@ -0,0 +1,154 @@
/**
* Invariant: the @assistant-ui dependency cluster agrees on one tap version.
*
* The Hermes desktop app (``apps/desktop``) is built from source on every
* install/update via ``scripts/install.ps1`` → ``npm ci``/``npm install`` →
* ``tsc -b && vite build``. The ``@assistant-ui`` packages share an internal
* reactivity lib, ``@assistant-ui/tap``, and they only interoperate when they
* all resolve the *same* tap version:
*
* - ``@assistant-ui/react@0.12.28`` and ``@assistant-ui/core`` pin
* ``@assistant-ui/tap@^0.5.x`` (which exports ``.`` and ``./react``).
* - ``@assistant-ui/store@0.2.18`` bumped its tap peer to ``^0.9.0`` and started
* importing ``@assistant-ui/tap/react-shim`` — an entry point that only exists
* in the tap ``0.9.x`` line.
*
* Because ``react@0.12.28`` requests ``store@^0.2.9`` (a caret range), a fresh
* install silently floated ``store`` up to ``0.2.18``, which then could not find
* ``./react-shim`` in the hoisted ``tap@0.5.x`` and crashed ``vite build`` with:
*
* "./react-shim" is not exported ... from package @assistant-ui/tap
*
* i.e. the opaque "apps/desktop build failed (exit 1)" every user hit when
* updating. The fix pins ``@assistant-ui/store`` (via root ``overrides``) to the
* last release that targets ``tap@^0.5.x``.
*
* This is a *contract* test, not a snapshot: it does not assert specific version
* numbers, only that the cluster resolves a single shared tap (wherever npm
* places it — hoisted to root, or nested under the ``apps/desktop`` workspace
* since the 0.14 bump dropped the ``store`` override) and that this tap satisfies
* every ``@assistant-ui/*`` package's declared requirement. It fails if any
* future bump reintroduces a split tap version or requirement across the cluster.
*/
import assert from 'node:assert/strict'
import fs from 'node:fs'
import path from 'node:path'
import { test } from 'vitest'
const REPO_ROOT = path.resolve(__dirname, '..', '..', '..')
const LOCK_PATH = path.join(REPO_ROOT, 'package-lock.json')
const TAP = '@assistant-ui/tap'
/**
* Minimal npm semver check for the ranges this cluster actually uses.
*
* Supports exact versions, ``^x.y.z`` (with correct 0.x semantics), and
* ``||`` unions. Pre-release tags are ignored (none are used here).
*/
function caretSatisfies(version: string, spec: string): boolean {
function parse(v: string): [number, number, number] {
const core = v.replace(/^[^0-9]+/, '').split('-')[0].split('+')[0]
const parts = core.split('.').slice(0, 3)
while (parts.length < 3) parts.push('0')
return [parseInt(parts[0], 10), parseInt(parts[1], 10), parseInt(parts[2], 10)]
}
const ver = parse(version)
for (const clause of spec.split('||')) {
const trimmed = clause.trim()
if (!trimmed) continue
if (trimmed.startsWith('^')) {
const lo = parse(trimmed)
if (ver[0] < lo[0] || (ver[0] === lo[0] && ver[1] < lo[1]) || (ver[0] === lo[0] && ver[1] === lo[1] && ver[2] < lo[2])) continue
let hi: [number, number, number]
if (lo[0] > 0) {
hi = [lo[0] + 1, 0, 0]
} else if (lo[1] > 0) {
hi = [0, lo[1] + 1, 0]
} else {
hi = [0, 0, lo[2] + 1]
}
if (
(ver[0] < hi[0]) ||
(ver[0] === hi[0] && ver[1] < hi[1]) ||
(ver[0] === hi[0] && ver[1] === hi[1] && ver[2] < hi[2])
) {
return true
}
} else if (trimmed[0].match(/\d/) || trimmed.startsWith('v')) {
if (ver[0] === parse(trimmed)[0] && ver[1] === parse(trimmed)[1] && ver[2] === parse(trimmed)[2]) {
return true
}
}
}
return false
}
interface LockPackage {
version?: string
dependencies?: Record<string, string>
peerDependencies?: Record<string, string>
peerDependenciesMeta?: Record<string, { optional?: boolean }>
}
function lockPackages(): Record<string, LockPackage> {
const lock = JSON.parse(fs.readFileSync(LOCK_PATH, 'utf-8'))
return (lock.packages ?? {}) as Record<string, LockPackage>
}
function sharedTapVersion(packages: Record<string, LockPackage>): string {
/** The one tap version every install site resolves to. */
const versions = new Set<string>()
for (const [key, meta] of Object.entries(packages)) {
const idx = key.lastIndexOf('node_modules/')
const name = idx >= 0 ? key.slice(idx + 'node_modules/'.length) : key
if (name === TAP) {
if (meta.version) versions.add(meta.version)
}
}
assert.ok(versions.size > 0, 'package-lock.json has no @assistant-ui/tap entry — the @assistant-ui cluster should resolve a single shared tap version.')
assert.ok(versions.size === 1, `@assistant-ui/tap resolves to multiple versions ${[...versions].sort()} — the cluster must share one tap line (see this test's docstring).`)
return [...versions][0]!
}
test('every @assistant-ui/* package\'s tap requirement is satisfiable', () => {
const packages = lockPackages()
const tapVersion = sharedTapVersion(packages)
const offenders: string[] = []
for (const [key, meta] of Object.entries(packages)) {
const idx = key.lastIndexOf('node_modules/')
const name = idx >= 0 ? key.slice(idx + 'node_modules/'.length) : key
if (!name.startsWith('@assistant-ui/') || name === TAP) continue
const peerMeta = (meta.peerDependenciesMeta ?? {})[TAP]
if (peerMeta?.optional) continue
const spec = (meta.dependencies ?? {})[TAP] || (meta.peerDependencies ?? {})[TAP]
if (!spec) continue
if (!caretSatisfies(tapVersion, spec)) {
offenders.push(`${name} requires ${TAP}"${spec}"`)
}
}
assert.deepEqual(
offenders,
[],
`Hoisted ${TAP}@${tapVersion} does not satisfy: ` +
offenders.join('; ') +
'. The @assistant-ui cluster has split tap requirements — pin the ' +
'offending package (e.g. via root package.json `overrides`) so the ' +
'whole cluster shares one tap line. See this test\'s module docstring.'
)
})
test('caretSatisfies helper', () => {
assert.ok(caretSatisfies('0.5.14', '^0.5.10'))
assert.ok(caretSatisfies('0.5.14', '^0.5.14'))
assert.ok(!caretSatisfies('0.5.14', '^0.9.0'))
assert.ok(!caretSatisfies('0.5.14', '^0.6.0'))
assert.ok(caretSatisfies('1.2.5', '^1.2.0'))
assert.ok(!caretSatisfies('2.0.0', '^1.2.0'))
assert.ok(caretSatisfies('0.5.14', '^0.5.0 || ^0.9.0'))
})

98
apps/desktop/electron/desktop-electron-pin.test.ts Normal file
View file

@ -0,0 +1,98 @@
/**
* Regression: the desktop Electron dependency must be an exact, consistent pin.
*
* The Windows desktop install failed at "Building desktop app" because Electron
* changed its install mechanism mid patch-series:
*
* electron 40.9.3 .. 40.10.2 -> @electron/get@^2 + extract-zip@^2 (pure JS)
* electron 40.10.3 / 40.10.4 -> @electron/get@^5 +
* @electron-internal/extract-zip@^1 (native napi)
*
* ``apps/desktop/package.json`` declared ``electronVersion: 40.9.3`` (the tested,
* JS-extract build) but pinned the dependency loosely as ``electron: ^40.9.3``.
* ``npm ci`` then resolved 40.10.3/40.10.4 — the new *native* extract-zip whose
* win32-x64 binding fails to ``dlopen`` on some Windows hosts
* (``ERR_DLOPEN_FAILED loading index.win32-x64-msvc.node``).
*
* These tests lock the contract that prevents that drift, without hard-coding the
* specific version (which is allowed to move):
*
* 1. the Electron dependency is an *exact* version (Electron Builder needs the
* installed binary to match ``electronVersion`` / ``electronDist``), and
* 2. the dependency, ``build.electronVersion``, and the resolved lockfile entry
* all agree — so ``npm ci`` installs exactly what the build packages.
*/
import assert from 'node:assert/strict'
import fs from 'node:fs'
import path from 'node:path'
import { test } from 'vitest'
const REPO_ROOT = path.resolve(__dirname, '..', '..', '..')
const DESKTOP_PKG = path.join(REPO_ROOT, 'apps', 'desktop', 'package.json')
const ROOT_LOCK = path.join(REPO_ROOT, 'package-lock.json')
// An exact semver: digits.digits.digits with an optional prerelease/build tag,
// but NO range operators (^ ~ > < = * x || spaces || -range).
const EXACT_SEMVER = /^\d+\.\d+\.\d+(?:[-+][0-9A-Za-z.-]+)?$/
function desktopPkg(): Record<string, unknown> {
assert.ok(fs.existsSync(DESKTOP_PKG), `missing ${DESKTOP_PKG}`)
return JSON.parse(fs.readFileSync(DESKTOP_PKG, 'utf-8'))
}
function electronSpec(pkg: Record<string, unknown>): string {
for (const section of ['dependencies', 'devDependencies'] as const) {
const deps = (pkg[section] ?? {}) as Record<string, string>
const spec = deps['electron']
if (spec) return spec
}
assert.fail('electron is not listed in apps/desktop dependencies')
}
test('electron dependency is exactly pinned', () => {
const spec = electronSpec(desktopPkg())
assert.match(
spec,
EXACT_SEMVER,
`electron must be pinned to an exact version, got "${spec}". ` +
'A range (^/~) lets npm ci resolve a newer Electron whose postinstall ' +
'may differ from the one the build was validated against.'
)
})
test('electron dependency matches build.electronVersion', () => {
const pkg = desktopPkg()
const spec = electronSpec(pkg)
const build = (pkg.build ?? {}) as Record<string, unknown>
const builderVersion = build.electronVersion as string | undefined
assert.ok(builderVersion, 'build.electronVersion is missing')
assert.equal(
spec,
builderVersion,
`electron dependency ("${spec}") must equal build.electronVersion ` +
`("${builderVersion}"); otherwise electron-builder packages a different ` +
'version than npm installs into electronDist.'
)
})
test('lockfile resolves the pinned electron', () => {
if (!fs.existsSync(ROOT_LOCK)) return // skip if lockfile not present
const spec = electronSpec(desktopPkg())
const lock = JSON.parse(fs.readFileSync(ROOT_LOCK, 'utf-8'))
const packages = (lock.packages ?? {}) as Record<string, { version?: string }>
const resolved = Object.entries(packages)
.filter(([key]) => key.endsWith('node_modules/electron'))
.map(([, meta]) => meta.version)
.filter((v): v is string => !!v)
assert.ok(resolved.length > 0, 'no electron entry found in package-lock.json')
for (const v of resolved) {
assert.equal(
v,
spec,
`package-lock.json resolves electron to ${v}, but the pin is "${spec}"; ` +
'run `npm install --package-lock-only` so `npm ci` stays consistent.'
)
}
})

88
apps/desktop/electron/package-json-lazy-deps.test.ts Normal file
View file

@ -0,0 +1,88 @@
/**
* Invariants for what is eager vs lazy in the root ``package.json``.
*
* The root ``package.json`` is installed by ``hermes update`` on every user,
* including users who never opted into a given browser backend. Anything
* listed in ``dependencies`` therefore runs its npm postinstall script for
* everyone — including binary-fetching backends, on every update.
*
* The contract:
*
* - ``agent-browser`` IS eager. It is the default Chromium-driving backend
* used whenever the agent makes a browser call without a cloud provider
* configured, so it must already be installed before any session starts.
* Its postinstall is also small.
*
* - ``@askjo/camofox-browser`` is NOT eager. It is an explicit opt-in
* alternative browser backend, selected by the user via
* ``hermes tools`` → Browser Automation → Camofox, and only used at
* runtime when ``CAMOFOX_URL`` is set. Its postinstall fetches a ~300MB
* Firefox-fork binary, which silently blocked ``hermes update`` for
* multi-minute stretches on slow / network-restricted connections
* (notably users in China running through a VPN). The package is
* installed on demand by ``tools_config.py`` ``post_setup_key ==
* "camofox"`` when the user actually selects Camofox.
*
* If a future PR re-adds Camofox (or any other binary-postinstall package)
* to root ``dependencies``, this test fails — read the lazy-install
* guidance in the ``hermes-agent-dev`` skill before changing the
* expectations.
*/
import assert from 'node:assert/strict'
import fs from 'node:fs'
import path from 'node:path'
import { test } from 'vitest'
const REPO_ROOT = path.resolve(__dirname, '..', '..', '..')
const ROOT_PKG = path.join(REPO_ROOT, 'package.json')
const ROOT_LOCK = path.join(REPO_ROOT, 'package-lock.json')
function rootPackageJson(): Record<string, unknown> {
return JSON.parse(fs.readFileSync(ROOT_PKG, 'utf-8'))
}
test('camofox is not in root dependencies (must stay opt-in)', () => {
const deps = (rootPackageJson().dependencies ?? {}) as Record<string, string>
assert.ok(
!('@askjo/camofox-browser' in deps),
'Camofox is a ~300MB binary-postinstall backend that must stay ' +
'out of root package.json dependencies. It belongs in the ' +
'Camofox post_setup handler in hermes_cli/tools_config.py so it ' +
'only installs when the user explicitly selects Camofox via ' +
'`hermes tools` → Browser Automation → Camofox.'
)
})
test('agent-browser stays eager (default backend)', () => {
const deps = (rootPackageJson().dependencies ?? {}) as Record<string, string>
assert.ok(
'agent-browser' in deps,
'agent-browser is the default browser-tool backend used by every ' +
'session that doesn\'t have a cloud browser provider configured. ' +
'It must stay in root package.json dependencies so it is present ' +
'after `hermes setup` / `hermes update` without an explicit ' +
'post_setup step.'
)
})
test('root lockfile has no camofox entries', () => {
if (!fs.existsSync(ROOT_LOCK)) {
// Some CI matrix shards skip lockfile materialization.
return
}
const text = fs.readFileSync(ROOT_LOCK, 'utf-8')
assert.ok(
!text.includes('@askjo/camofox-browser'),
'package-lock.json still references @askjo/camofox-browser. ' +
'Regenerate the lockfile after removing the dep: ' +
'`rm package-lock.json && npm install --package-lock-only ' +
'--ignore-scripts --no-fund --no-audit`.'
)
assert.ok(
!text.includes('camoufox-js'),
'package-lock.json still references camoufox-js (transitive of ' +
'@askjo/camofox-browser). Regenerate the lockfile.'
)
})

159
tests/test_assistant_ui_tap_compat.py
View file

@ -1,159 +0,0 @@
"""Invariant: the @assistant-ui dependency cluster agrees on one tap version.
The Hermes desktop app (``apps/desktop``) is built from source on every
install/update via ``scripts/install.ps1`` → ``npm ci``/``npm install`` →
``tsc -b && vite build``. The ``@assistant-ui`` packages share an internal
reactivity lib, ``@assistant-ui/tap``, and they only interoperate when they
all resolve the *same* tap version:
* ``@assistant-ui/react@0.12.28`` and ``@assistant-ui/core`` pin
``@assistant-ui/tap@^0.5.x`` (which exports ``.`` and ``./react``).
* ``@assistant-ui/store@0.2.18`` bumped its tap peer to ``^0.9.0`` and started
importing ``@assistant-ui/tap/react-shim`` — an entry point that only exists
in the tap ``0.9.x`` line.
Because ``react@0.12.28`` requests ``store@^0.2.9`` (a caret range), a fresh
install silently floated ``store`` up to ``0.2.18``, which then could not find
``./react-shim`` in the hoisted ``tap@0.5.x`` and crashed ``vite build`` with::
"./react-shim" is not exported ... from package @assistant-ui/tap
i.e. the opaque "apps/desktop build failed (exit 1)" every user hit when
updating. The fix pins ``@assistant-ui/store`` (via root ``overrides``) to the
last release that targets ``tap@^0.5.x``.
This is a *contract* test, not a snapshot: it does not assert specific version
numbers, only that the cluster resolves a single shared tap (wherever npm
places it — hoisted to root, or nested under the ``apps/desktop`` workspace
since the 0.14 bump dropped the ``store`` override) and that this tap satisfies
every ``@assistant-ui/*`` package's declared requirement. It fails if any
future bump reintroduces a split tap version or requirement across the cluster.
"""
from __future__ import annotations
import json
from pathlib import Path
import pytest
REPO_ROOT = Path(__file__).resolve().parent.parent
TAP = "@assistant-ui/tap"
def _caret_satisfies(version: str, spec: str) -> bool:
"""Minimal npm semver check for the ranges this cluster actually uses.
Supports exact versions, ``^x.y.z`` (with correct 0.x semantics), and
``||`` unions. Pre-release tags are ignored (none are used here).
"""
def parse(v: str) -> tuple[int, int, int]:
core = v.lstrip("^~>=<v ").split("-")[0].split("+")[0]
parts = (core.split(".") + ["0", "0", "0"])[:3]
return tuple(int(p) for p in parts) # type: ignore[return-value]
ver = parse(version)
for clause in spec.split("||"):
clause = clause.strip()
if not clause:
continue
if clause.startswith("^"):
lo = parse(clause)
if ver < lo:
continue
major, minor, _ = lo
if major > 0:
hi = (major + 1, 0, 0)
elif minor > 0:
hi = (0, minor + 1, 0)
else:
hi = (0, 0, lo[2] + 1)
if ver < hi:
return True
elif clause[0].isdigit() or clause.startswith("v"):
if ver == parse(clause):
return True
return False
def _lock_packages() -> dict:
lock_path = REPO_ROOT / "package-lock.json"
if not lock_path.exists():
pytest.skip("package-lock.json not materialized in this CI shard")
with lock_path.open("r", encoding="utf-8") as fh:
return json.load(fh).get("packages", {})
def _shared_tap_version(packages: dict) -> str:
"""The one tap version every install site resolves to.
npm hoists tap to root ``node_modules`` when the cluster lives at the top
level, but nests the whole cluster under a workspace
(``apps/desktop/node_modules``) when only that workspace depends on it — as
it does since the 0.14 bump dropped the ``@assistant-ui/store`` override.
Either layout is fine; the invariant is that a single tap version is shared
across every install site, so ``vite build`` never hits a split API.
"""
versions = {
meta["version"]
for key, meta in packages.items()
if key.rsplit("node_modules/", 1)[-1] == TAP
}
assert versions, (
"package-lock.json has no @assistant-ui/tap entry — the "
"@assistant-ui cluster should resolve a single shared tap version."
)
assert len(versions) == 1, (
f"@assistant-ui/tap resolves to multiple versions {sorted(versions)} — "
"the cluster must share one tap line (see this test's docstring)."
)
return versions.pop()
def test_assistant_ui_cluster_agrees_on_one_tap() -> None:
"""Every @assistant-ui/* package's tap requirement must be satisfiable.
Encodes the contract that broke the desktop build: a single hoisted
@assistant-ui/tap must satisfy the tap range declared by react, core,
store, and any sibling — otherwise the missing ``./react-shim`` export
(or a similar API split) breaks ``vite build``.
"""
packages = _lock_packages()
tap_version = _shared_tap_version(packages)
offenders: list[str] = []
for key, meta in packages.items():
name = key.rsplit("node_modules/", 1)[-1]
if not name.startswith("@assistant-ui/") or name == TAP:
continue
peer_meta = meta.get("peerDependenciesMeta", {}).get(TAP, {})
if peer_meta.get("optional"):
continue
spec = meta.get("dependencies", {}).get(TAP) or meta.get(
"peerDependencies", {}
).get(TAP)
if not spec:
continue
if not _caret_satisfies(tap_version, spec):
offenders.append(f"{name} requires {TAP}{spec!r}")
assert not offenders, (
f"Hoisted {TAP}@{tap_version} does not satisfy: "
+ "; ".join(offenders)
+ ". The @assistant-ui cluster has split tap requirements — pin the "
"offending package (e.g. via root package.json `overrides`) so the "
"whole cluster shares one tap line. See this test's module docstring."
)
def test_caret_satisfies_helper() -> None:
"""Guard the tiny semver helper the invariant relies on."""
assert _caret_satisfies("0.5.14", "^0.5.10")
assert _caret_satisfies("0.5.14", "^0.5.14")
assert not _caret_satisfies("0.5.14", "^0.9.0")
assert not _caret_satisfies("0.5.14", "^0.6.0")
assert _caret_satisfies("1.2.5", "^1.2.0")
assert not _caret_satisfies("2.0.0", "^1.2.0")
assert _caret_satisfies("0.5.14", "^0.5.0 || ^0.9.0")

25
tests/test_dashboard_sidecar_close_on_disconnect.py
View file

@ -1,25 +0,0 @@
import re
from pathlib import Path
CHAT_SIDEBAR = Path(__file__).resolve().parent.parent / "web/src/components/ChatSidebar.tsx"
def test_sidecar_session_create_requests_close_on_disconnect():
"""The sidecar must opt its session into close_on_disconnect so the gateway
reaps the slash_worker on WS disconnect (the #21370/#21467 leak)."""
source = CHAT_SIDEBAR.read_text(encoding="utf-8")
call = re.search(r'"session\.create",\s*\{(.*?)\}', source, re.DOTALL)
assert call, "sidecar session.create call not found"
assert re.search(r"close_on_disconnect:\s*true", call.group(1))
def test_sidecar_session_create_scopes_profile():
"""The sidecar must pass the dashboard's selected profile so model/credential
info matches the PTY child under profile-scoped chat."""
source = CHAT_SIDEBAR.read_text(encoding="utf-8")
call = re.search(r'"session\.create",\s*\{(.*?)\}\);', source, re.DOTALL)
assert call, "sidecar session.create call not found"
body = call.group(1)
assert re.search(r"close_on_disconnect:\s*true", body)
assert re.search(r'source:\s*"tool"', body)
assert re.search(r"\.\.\.\(profile\s*\?\s*\{\s*profile\s*\}\s*:\s*\{\}\)", body)

96
tests/test_desktop_electron_pin.py
View file

@ -1,96 +0,0 @@
"""Regression: the desktop Electron dependency must be an exact, consistent pin.
The Windows desktop install failed at "Building desktop app" because Electron
changed its install mechanism mid patch-series:
electron 40.9.3 .. 40.10.2 -> @electron/get@^2 + extract-zip@^2 (pure JS)
electron 40.10.3 / 40.10.4 -> @electron/get@^5 +
@electron-internal/extract-zip@^1 (native napi)
``apps/desktop/package.json`` declared ``electronVersion: 40.9.3`` (the tested,
JS-extract build) but pinned the dependency loosely as ``electron: ^40.9.3``.
``npm ci`` then resolved 40.10.3/40.10.4 — the new *native* extract-zip whose
win32-x64 binding fails to ``dlopen`` on some Windows hosts
(``ERR_DLOPEN_FAILED loading index.win32-x64-msvc.node``).
These tests lock the contract that prevents that drift, without hard-coding the
specific version (which is allowed to move):
1. the Electron dependency is an *exact* version (Electron Builder needs the
installed binary to match ``electronVersion`` / ``electronDist``), and
2. the dependency, ``build.electronVersion``, and the resolved lockfile entry
all agree — so ``npm ci`` installs exactly what the build packages.
"""
from __future__ import annotations
import json
import re
from pathlib import Path
import pytest
REPO_ROOT = Path(__file__).resolve().parent.parent
DESKTOP_PKG = REPO_ROOT / "apps" / "desktop" / "package.json"
ROOT_LOCK = REPO_ROOT / "package-lock.json"
# An exact semver: digits.digits.digits with an optional prerelease/build tag,
# but NO range operators (^ ~ > < = * x || spaces || -range).
_EXACT_SEMVER = re.compile(r"^\d+\.\d+\.\d+(?:[-+][0-9A-Za-z.-]+)?$")
def _desktop_pkg() -> dict:
assert DESKTOP_PKG.is_file(), f"missing {DESKTOP_PKG}"
return json.loads(DESKTOP_PKG.read_text(encoding="utf-8"))
def _electron_spec(pkg: dict) -> str:
for section in ("dependencies", "devDependencies"):
spec = pkg.get(section, {}).get("electron")
if spec:
return spec
pytest.fail("electron is not listed in apps/desktop dependencies")
def test_electron_dependency_is_exactly_pinned():
"""A loose range lets npm drift onto an Electron with a different installer."""
spec = _electron_spec(_desktop_pkg())
assert _EXACT_SEMVER.match(spec), (
f"electron must be pinned to an exact version, got {spec!r}. "
"A range (^/~) lets npm ci resolve a newer Electron whose postinstall "
"may differ from the one the build was validated against."
)
def test_electron_dependency_matches_electron_version():
"""electron-builder packages build.electronVersion against the installed binary."""
pkg = _desktop_pkg()
spec = _electron_spec(pkg)
builder_version = pkg.get("build", {}).get("electronVersion")
assert builder_version, "build.electronVersion is missing"
assert spec == builder_version, (
f"electron dependency ({spec!r}) must equal build.electronVersion "
f"({builder_version!r}); otherwise electron-builder packages a different "
"version than npm installs into electronDist."
)
def test_lockfile_resolves_the_pinned_electron():
"""npm ci installs from the lockfile, so it must agree with the pin."""
if not ROOT_LOCK.is_file():
pytest.skip("root package-lock.json not present")
spec = _electron_spec(_desktop_pkg())
lock = json.loads(ROOT_LOCK.read_text(encoding="utf-8"))
packages = lock.get("packages", {})
resolved = [
meta.get("version")
for path, meta in packages.items()
if path.endswith("node_modules/electron") and meta.get("version")
]
assert resolved, "no electron entry found in package-lock.json"
assert all(v == spec for v in resolved), (
f"package-lock.json resolves electron to {sorted(set(resolved))}, "
f"but the pin is {spec!r}; run `npm install --package-lock-only` so "
"`npm ci` stays consistent."
)

85
tests/test_package_json_lazy_deps.py
View file

@ -1,85 +0,0 @@
"""Invariants for what is eager vs lazy in the root ``package.json``.
The root ``package.json`` is installed by ``hermes update`` on every user,
including users who never opted into a given browser backend. Anything
listed in ``dependencies`` therefore runs its npm postinstall script for
everyone — including binary-fetching backends, on every update.
The contract:
* ``agent-browser`` IS eager. It is the default Chromium-driving backend
used whenever the agent makes a browser call without a cloud provider
configured, so it must already be installed before any session starts.
Its postinstall is also small.
* ``@askjo/camofox-browser`` is NOT eager. It is an explicit opt-in
alternative browser backend, selected by the user via
``hermes tools`` → Browser Automation → Camofox, and only used at
runtime when ``CAMOFOX_URL`` is set. Its postinstall fetches a ~300MB
Firefox-fork binary, which silently blocked ``hermes update`` for
multi-minute stretches on slow / network-restricted connections
(notably users in China running through a VPN). The package is
installed on demand by ``tools_config.py`` ``post_setup_key ==
"camofox"`` when the user actually selects Camofox.
If a future PR re-adds Camofox (or any other binary-postinstall package)
to root ``dependencies``, this test fails — read the lazy-install
guidance in the ``hermes-agent-dev`` skill before changing the
expectations.
"""
from __future__ import annotations
import json
from pathlib import Path
REPO_ROOT = Path(__file__).resolve().parent.parent
def _root_package_json() -> dict:
with (REPO_ROOT / "package.json").open("r", encoding="utf-8") as fh:
return json.load(fh)
def test_camofox_is_not_in_root_dependencies() -> None:
"""Camofox must be opt-in, installed lazily by its post_setup handler."""
deps = _root_package_json().get("dependencies", {})
assert "@askjo/camofox-browser" not in deps, (
"Camofox is a ~300MB binary-postinstall backend that must stay "
"out of root package.json dependencies. It belongs in the "
"Camofox post_setup handler in hermes_cli/tools_config.py so it "
"only installs when the user explicitly selects Camofox via "
"`hermes tools` → Browser Automation → Camofox."
)
def test_agent_browser_stays_eager() -> None:
"""agent-browser is the default backend; it must remain eager."""
deps = _root_package_json().get("dependencies", {})
assert "agent-browser" in deps, (
"agent-browser is the default browser-tool backend used by every "
"session that doesn't have a cloud browser provider configured. "
"It must stay in root package.json dependencies so it is present "
"after `hermes setup` / `hermes update` without an explicit "
"post_setup step."
)
def test_root_lockfile_has_no_camofox_entries() -> None:
"""Regenerated lockfiles should not contain Camofox tree entries."""
lock_path = REPO_ROOT / "package-lock.json"
if not lock_path.exists():
# Some CI matrix shards skip lockfile materialization.
return
text = lock_path.read_text(encoding="utf-8")
assert "@askjo/camofox-browser" not in text, (
"package-lock.json still references @askjo/camofox-browser. "
"Regenerate the lockfile after removing the dep: "
"`rm package-lock.json && npm install --package-lock-only "
"--ignore-scripts --no-fund --no-audit`."
)
assert "camoufox-js" not in text, (
"package-lock.json still references camoufox-js (transitive of "
"@askjo/camofox-browser). Regenerate the lockfile."
)

21
web/src/components/ChatSidebar.tsx
View file

@ -79,6 +79,21 @@ interface ChatSidebarProps {
onSessionTitleChange?: (title: string | null) => void;
}
/** Build the ``session.create`` params for the sidecar session.
*
* Extracted from the effect below so the invariant — close_on_disconnect
* is set, source is "tool", and the profile is forwarded when present —
* can be tested without reading component source text. See
* ``chat-sidebar-session-params.test.ts``.
*/
export function sidecarSessionCreateParams(profile?: string): Record<string, unknown> {
return {
close_on_disconnect: true,
source: "tool",
...(profile ? { profile } : {}),
};
}
export function ChatSidebar({
channel,
profile,
@ -189,11 +204,7 @@ export function ChatSidebar({
}
// close_on_disconnect: the gateway reaps this sidecar session (and its
// slash_worker subprocess) when the WS drops, instead of leaking it.
return gw.request<{ session_id: string }>("session.create", {
close_on_disconnect: true,
source: "tool",
...(profile ? { profile } : {}),
});
return gw.request<{ session_id: string }>("session.create", sidecarSessionCreateParams(profile));
})
.catch((e: Error) => {
if (!cancelled) {

35
web/src/lib/chat-sidebar-session-params.test.ts Normal file
View file

@ -0,0 +1,35 @@
/**
* Tests for the sidecar ``session.create`` params built by ChatSidebar.
*
* The sidecar must opt its session into close_on_disconnect so the gateway
* reaps the slash_worker on WS disconnect (the #21370/#21467 leak), and it
* must pass the dashboard's selected profile so model/credential info
* matches the PTY child under profile-scoped chat.
*
*/
import { describe, expect, it } from "vitest";
import { sidecarSessionCreateParams } from "@/components/ChatSidebar";
describe("sidecarSessionCreateParams", () => {
it("opts into close_on_disconnect", () => {
const params = sidecarSessionCreateParams();
expect(params.close_on_disconnect).toBe(true);
});
it("sets source to 'tool'", () => {
const params = sidecarSessionCreateParams();
expect(params.source).toBe("tool");
});
it("forwards the profile when present", () => {
const params = sidecarSessionCreateParams("work");
expect(params.profile).toBe("work");
});
it("omits profile when undefined", () => {
const params = sidecarSessionCreateParams();
expect(params).not.toHaveProperty("profile");
});
});