Sync upstream hermes-agent @ acc614e72 (CONFLICTS) - #334
Draft
hermes-upstream-sync[bot] wants to merge 1990 commits into
Draft
Sync upstream hermes-agent @ acc614e72 (CONFLICTS)#334hermes-upstream-sync[bot] wants to merge 1990 commits into
hermes-upstream-sync[bot] wants to merge 1990 commits into
Conversation
…sers Two data-loss bugs reported by users: 1. /handoff CLI→gateway race (#88234): After /handoff completed, CLI cleanup called finalize_session on the session the gateway just reopened. This set end_reason on a row the gateway was actively writing to, causing the handoff leg to vanish from session history and breaking session_search recall. Fix: add _handed_off_session_ids module-level set (mirrors _single_query_finalize_attempted_session_ids pattern). _handle_handoff_command registers the session_id on completion; _should_emit_cleanup_session_finalize and _emit_interrupted_session_end check it before firing. 2. state.db corruption silent failure (#88235): When SessionDB init failed at gateway startup, the error stayed in logs — messages flowed but nothing was persisted, with no user-visible indication. Fix: store _session_db_init_error on GatewayRunner, broadcast a recovery-guidance message to all home channels via _send_session_db_warning_notifications() after the gateway connects. Also improved the 'corrupt' persistence cause wording in _format_turn_completion_explanation to include the full recovery path (hermes doctor --fix, sqlite3 .recover, backups). Tests: 6 new tests for handoff cleanup race, 3 for corruption wording. All existing CLI/turn-completion tests pass.
/simplify-code review found _notify_single_query_session_finalize was missing the _handed_off_session_ids guard that _should_emit_cleanup_session_finalize and _emit_interrupted_session_end already had. One-shot CLI queries that somehow handed off would still finalize the session via this path. Added guard + test.
_read_journal_mode opened each Hermes database with a bare open(db_path, "rb") to read header byte 18. The read itself is harmless; the close() is not. Per sqlite.org/howtocorrupt.html, close() on *any* descriptor for a file cancels every POSIX advisory lock this process holds on it — so the close at the end of that with-block drops the locks a live connection is holding, including the EXCLUSIVE lock a VACUUM holds while it rewrites the whole file. Another process is then free to write into a file its writer still believes it owns, which is the documented route to "database disk image is malformed". This is reachable. run_doctor is not only a standalone CLI process: the dashboard console registers "doctor" (console_engine.py:570) and calls run_doctor directly, in-process (console_engine.py:1297), on the web server's console thread pool — in a process that holds live SessionDB connections (web_server.py:11673, :11689). Typing "doctor" there raw-opened and closed state.db, projects.db, response_store.db, cron/executions.db and every board's kanban.db while those connections were live. The HTTP route at /api/ops/doctor deliberately spawns a subprocess instead; the console path did not. hermes_cli.sqlite_safe_read exists to prevent exactly this, and its read_header_bytes_preopen is documented as "the ONLY sanctioned byte-level read of a database file". It performs the registry check and the open/read/close together under the connection-lifecycle lock, so it refuses once any connection to the path is live. The audit that converted the other byte-probes (hermes_state.py:2750, backup.py:436, kanban_db.py:1861) landed in 95fb477 on 2026-07-25; _read_journal_mode was added in 6583297 on 2026-08-06 and reintroduced the pattern, so this is a regression against an invariant the tree already states, not a refactor preference. The helper is a plain byte read, so the docstring's stated property is preserved: no SQLite engine open, and no -wal/-shm sidecars are created. Only the acquisition of `header` changes; the empty / not-a-database / unrecognized-format-version branches are untouched.
read_header_bytes_preopen returns None for every failure, so routing the probe through it flattened "[Errno 2] No such file or directory: …" and "[Errno 13] Permission denied: …" into one opaque "file could not be read". doctor exists to name the problem, so that detail is worth keeping: _report_database_journal_modes prints the string verbatim, and on a vulnerable SQLite it is the only clue the user gets about why WAL exposure could not be ruled out. _unreadable_reason recovers it from metadata only. stat() reports the missing file, the dangling symlink and the unsearchable parent directory; os.access(..., R_OK) reports the unreadable file that stat() can still see. Neither call takes a file descriptor, so neither can cancel the POSIX advisory locks the previous commit was about — the invariant holds.
Locks the invariant the probe now honours: while this process holds a registered connection to a database, _read_journal_mode reports it as unreadable instead of taking a descriptor whose close() would cancel that connection's POSIX advisory locks. Against the previous implementation the four regression cases fail with `assert 'wal' is None` — it read the header straight out of a live database — and pass once the read is routed through read_header_bytes_preopen. Coverage is both the registry API (track_connection) and connect_tracked, the path SessionDB actually takes, plus the _report_database_journal_modes output so the degraded row is asserted end to end. Two cases deliberately hold in both directions and are guards rather than probes: - an untracked sqlite3.connect holding BEGIN EXCLUSIVE must NOT block the read. Only connections this process registered can be cancelled by a close() we make; another process's locks are irrelevant. Without this, a later "just refuse whenever the file looks busy" change would silently turn every doctor row into "could not be read". - the refusal creates no new -wal/-shm sidecars, which is the property the function's docstring promises and the reason it byte-probes rather than opening a connection in the first place.
…rows read_header_bytes_preopen answers None for a live connection, a missing file and an unreadable file alike, so the error string doctor prints is now chosen rather than inherited from the OSError. These cases pin that choice: the missing file keeps its errno text, and the chmod-000 file is still reported as a permission problem rather than collapsing into the generic message — the behaviour the raw open() gave before. test_reason_does_not_open_the_file is the load-bearing one. It patches builtins.open to raise and asserts _unreadable_reason still answers, which fixes the constraint that makes the helper safe to call on a database path at all: stat() and access() read metadata and take no file descriptor, so no close() of ours can cancel the file's advisory locks. A future edit that reached for open() here to get a better message would reintroduce the original bug on the error path, and this test fails loudly if it does. The root check is written as hasattr(os, "geteuid") and os.geteuid() == 0 rather than the bare call the surrounding tests use. skipif conditions are evaluated at collection time and os.geteuid is POSIX-only, so the bare form raises AttributeError and takes the whole module down on Windows. The pre-existing occurrences are left alone — #81926 and #84073 are already open against exactly those lines, and this only avoids adding a third instance of the same defect.
…ndent clean_registry cleared the connection registry only on teardown, so it protected the tests that ran after it but not the test holding it. A leak from earlier in the session — a failed test that never reached its close(), or any test that does not take this fixture — would leave a stale entry behind, and read_header_bytes_preopen would then refuse for that stale reason instead of the one under test. The refusal assertions would still pass, but for the wrong reason, which is the failure mode a regression test can least afford. Clearing on entry as well makes the fixture independent of what ran before it, and the teardown clear now runs under try/finally so a failing test cannot skip it.
The Bots pane header + is now a dropdown (New Agent / New Group Chat). New Group Chat opens a checkbox-picker modal: searchable roster list, member cap at GROUP_CHAT_MAX_MEMBERS, group-name input that defaults to the selected members' names, and a Create button that assigns the existing per-bot group meta field - so the room rides the ui_meta sync path unchanged and the user lands directly in the new room.
…plete The @ popover only completed filesystem references; bot handles worked when fully typed (mention middleware parses at submit) but were never offered, so users had to know the exact handle — worse with multi-source @name-device handles. Fixes #88060 (ported from Hermes-Bot-Mode#43). - composer contrib: new 'composer.atCompletions' data area (ComposerAtCompletionSource) — contributed rows merge AHEAD of path results; a throwing source drops its rows, never the popover - use-at-completions: merge contributed entries in all three fetch paths (gateway results, gateway-less, fetch error) - SDK: export the new area + types for plugins - bundled Bot Mode plugin: registers 'mention-completions' — roster handles from the query cache (\u22645s stale), active profile excluded, 'default' offered as @Hermes, multi-source @name-device handles via botHandle, display name + connection label in the row meta, capped at 8 - registered early in register(ctx) so vm harnesses reach it before the pane/UI registrations that stubs can't fully model
…ncurrency cap (OOF-30) Two production incidents (OOF-77 "larrikin-lollies", OOF-30 "synclare-task-manager") followed the same shape: no kanban.max_in_progress configured, a busy board, and a 1 GiB hosted VM. The dispatcher fanned out 26-31 concurrent workers, the host went into swap-thrash/OOM, and the whole machine — dashboard included — became unreachable. NAS restart loops then masked the problem: each restart "recovered" briefly before the kanban dispatcher immediately respawned unbounded workers. Building on the cherry-picked max_in_progress-across-both-lanes fix (PR #28695, credit @Dusk1e), this adds two complementary safeguards to hermes_cli/kanban_db.py: 1. Memory-DERIVED default concurrency cap. When kanban.max_in_progress is unset, resolve_max_in_progress() derives a default of clamp(MemTotal / 512 MiB, 2, 8) — e.g. 2 workers on a 1 GiB VM, 8 on 4 GiB+. Explicit config always wins in either direction. On hosts where total memory can't be read (macOS/Windows dev machines), the default stays None (no cap — unchanged behaviour). Wired into both dispatch entry points (gateway/kanban_watchers.py and hermes kanban dispatch) so behaviour matches regardless of path. 2. Live memory-PRESSURE guard inside dispatch_once. A static cap can't see the host's actual memory state (other tenants, bloated long-lived workers). The dispatcher now samples system memory each tick via gateway.lifecycle_ledger.sample_memory() and classifies it with gateway.memory_status.classify_pressure() (same thresholds as the dashboard memory banner and OOM-suspicion heuristics from NS-608/NS-656): critical -> spawn nothing this tick; elevated -> at most one new worker; unknown -> no restriction (fail-open). Reclaim/promotion bookkeeping still runs under pressure, and deferred tasks stay queued — nothing is dropped. Restriction is surfaced on DispatchResult.memory_pressure and logged. Tests: tests/hermes_cli/test_kanban_memory_guard.py (14 tests) covers the derived cap (floor/ceiling/fail-open/explicit-config-wins), the pressure classifier, and dispatch behaviour under critical/elevated/ unknown pressure including defer-not-drop and bookkeeping-still-runs. An autouse fixture in tests/conftest.py pins the memory sample to "no data" suite-wide so existing dispatch tests don't depend on the CI runner's live memory state (opt-out marker: real_memory_guard).
…-lane reservation (OOF-30 review) Addresses three gaps found in review of the memory-guard PR: P1a — standalone daemon was the one uncapped entry point. run_daemon() now resolves kanban.max_in_progress every tick (explicit config wins, else the memory-derived default) exactly like the gateway dispatcher and `hermes kanban dispatch`. New shared parser configured_max_in_progress() so all three entry points agree on what "explicitly configured" means. P1b — max_in_progress was enforced per board while the gateway ticks every active board, multiplying the host budget by the number of boards (2 boards x cap 2 = 4 workers on a host sized for 2). The cap is now host-level: _dispatch_once_locked() adds count_running_tasks_other_boards() to the running count before deriving the tick's spawn budget. Enforced in the shared locked path, so gateway, CLI, and daemon all inherit it. max_spawn deliberately keeps its historical per-board semantics. Fails open per board so one corrupt board can't brick dispatch on the rest. P2 — the ready loop consumed the entire shared spawn budget before the review loop ran, so a sustained ready backlog starved autonomous reviews indefinitely. When spawnable review work exists (assigned + real profile, mirroring the review loop's own gate) and the tick has budget, one slot is held back from the ready lane. Reservation is per-tick and self-releasing; the review lane still spends from the shared budget — it gains fairness, not extra capacity. 11 new tests in tests/hermes_cli/test_kanban_host_cap.py. Existing kanban suites: 278 passed (15 failures pre-existing, identical on clean main baseline). ruff clean.
…arget logging
A user typed their root password into the Desktop SSH host field
(root@IP:PASSWORD form). Three failures compounded:
1. validateSshTarget() only checked for option injection (leading dash),
control chars, and port range — commas in an IP, whitespace ("ssh "
prefix pastes), and non-numeric ":<segment>" leftovers all dialed ssh
with garbage and failed silently five times.
2. normalizeSshConfig() only strips a ":<segment>" when it is numeric, so
a pasted password stayed glued to the hostname all the way into ssh
argv and the desktop.log connect line.
3. redactSecrets() had no pattern for ssh targets, so the password landed
verbatim in desktop.log and then in a PUBLIC debug-share paste.
Changes:
- validateSshTarget(): reject whitespace, commas, non-numeric colon
segments (with a "never put a password in the host field" hint that
does NOT echo the credential), and garbage hostnames; still accepts
bare IPv6 (::1, fe80::1%eth0). Reject whitespace/@ in user.
- redactSecrets(): new pattern masks any non-numeric segment where a
port belongs in user@host:... strings — defense in depth so future
parse gaps can't leak credentials into logs or debug shares.
- normalizeSshConfig(): strip a pasted leading "ssh " prefix.
- Tests for all three, including the exact incident shapes.
`ElicitationHandler` read `params.requested_schema`, but on the pinned
`mcp==1.28.1` the model field is spelled `requestedSchema`. The getattr
always missed and returned its `{}` default, so
`_format_elicitation_schema_summary` took its no-properties branch and the
approval prompt collapsed to the generic
Approval requested by MCP server '<name>'.
for every request. The field names, types, and descriptions the summary
exists to surface never reached the user, so an elicitation asking for a
card number rendered identically to one asking for a nickname — consent
without the substance of what was being consented to.
Read both spellings rather than just correcting to the 1.x name: mcp 2.0
renames this field to `requested_schema` (it renamed every model field to
snake_case and kept camelCase only as a serialization alias, which
pydantic does not expose to attribute access), so a dual read is correct
on either SDK generation and does not go wrong again on the next bump.
Verified against real 1.28.1 and 2.0.0 installs.
Every existing test in tests/tools/test_mcp_elicitation.py builds a
duck-typed `SimpleNamespace` stand-in, which carries whatever field name
the test wrote and therefore cannot detect a mismatch with the real model.
Add one test that constructs the actual `ElicitRequestFormParams` and
asserts the requested field name reaches the consent description; it fails
on the unfixed tree. The cheap stand-ins are left alone elsewhere.
Found while porting the tree to the mcp 2.x SDK in #76736, but independent
of it: this reproduces on the current pin with no other changes, #76736
does not touch this line, and the two branches merge cleanly in either
order.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Remove the cross-version heuristic from _strip_mismatched_site_packages: the subprocess env builder cannot know which Python version a child will run, so judging user PYTHONPATH entries against the backend interpreter's version deletes legitimate paths meant for a different child Python (e.g. /custom/lib/python3.13/site-packages while Hermes runs 3.11). Also fix over-strip: entries merely containing a pythonX.Y path component (e.g. /opt/tools/python3.13/bin) were stripped even though they are not site-packages. Hermes-owned entries (repo root, own venv site-packages) are now identified by path ownership, not by version. Regression tests cover both cases; user paths with any pythonX.Y component are preserved.
The gateway runs inside its own venv; if its PYTHONHOME leaks into subprocesses (terminal commands, cron no_agent scripts, TTS providers), any child interpreter redirects its stdlib search to the Hermes venv and crashes with version-mismatch errors before importing anything. PYTHONHOME is now part of _ACTIVE_VENV_MARKER_VARS so all env builders (_make_run_env, _sanitize_subprocess_env, hermes_subprocess_env, and build_subprocess_env used by cron) drop it, consistent with Hermes' existing PYTHONHOME handling in managed_uv.py and sqlite_runtime.py. execute_code already scrubbed it via _SAFE_ENV_PREFIXES. Tests cover all four builders plus the marker constant.
Adversarial review of the previous two commits (and #78917 itself) found three ownership-boundary issues; this commit addresses them: 1. Repo direct-child over-strip (Finding A) No launcher injects <repo>/tools or another direct child as an independent PYTHONPATH entry - audited all four producers (Electron electron-main.mjs, gateway/run.py::_ensure_windows_gateway_venv_imports, cron/scheduler.py::_windows_cron_python_invocation, tui_gateway/host_supervisor.py). The depth<=1 rule deleted user paths that merely live under the repo directory; only the EXACT repo root is now stripped. 2. Windows junction/symlink alias (Finding B) The gateway launcher renders Hermes-owned paths under the configured HERMES_HOME spelling (gateway_windows.py::_preserve_hermes_home_path), which may be a junction to another drive, so it differs lexically from the resolved repo root. _hermes_repo_root_aliases now carries both the resolved and unresolved spellings; both are recognized as Hermes-owned. 3. Stale abstraction rename (Phase 4) _strip_mismatched_site_packages -> _strip_hermes_owned_pythonpath: the cross-version heuristic is gone, so the old name misdescribes the behavior (ownership-based, not version-based). Tests: direct-child now preserved; junction alias stripped (lexical pair monkeypatched); Windows-only real-semantics test added (POSIX test remains a safety test); mixed-ordering, duplicate-Hermes, and no-scrub PYTHONHOME contract tests added. Full file: 52 passed / 16 failed (identical failure set to base, all isolation-venv environment issues).
…ted inherited env Integration test for the #84500 + #82581 intersection: seeds a contaminated inherited PYTHONPATH (Hermes repo root + Hermes venv site-packages + user entries) through os.environ and drives execute_code to Popen. Asserts the staging tmpdir stays first, inherited Hermes site-packages never survive, the repo root is re-added exactly once for a same-env child (proving the inherited copy was stripped) and stays absent for an external-env child, and user entries survive in order.
The PYTHONPATH/PATH sanitization suite was written POSIX-centric and failed on real Windows 11 (reproduced natively: 4 failures before this change). Fix the tests to express the true per-platform contract: - test_other_major_version_site_packages_preserved / test_make_run_env_injects_hermes_bin_dir: build inputs with os.pathsep instead of hardcoded ':'. - test_make_run_env_appends_homebrew_on_minimal_path: split on os.pathsep, neutralise Git Bash dir prepending, and assert the documented Windows passthrough (_append_missing_sane_path_entries is a no-op off POSIX) instead of the Homebrew append. - test_make_run_env_real_launchd_path_gains_homebrew: mark macos_only per repo OS-marker policy (the regression is the macOS launchd PATH; the merge is a passthrough on Windows). - test_configured_home_alias_matches_launcher_output: create the configured-home link via a helper that falls back to an unprivileged directory junction (cmd /c mklink /J) when symlink creation raises WinError 1314, and skips with a clear reason if no mechanism exists. Also correct a stale comment in execute_code: the child is not always the same Python as Hermes (project mode can select an external venv), so the strip is about compatibility, not redundancy.
Confirmed on native Windows 11 with a real junction and the real startup chain: when the desktop/CLI spawns the backend with HERMES_HOME in the configured (lexical) spelling and --profile / sticky active_profile is in play, _apply_profile_override() re-homes HERMES_HOME through resolve_profile_env(), which resolves the junction under the platform default and returns the PHYSICAL spelling. tools.environments.local is imported after that mutation, so _hermes_repo_root_aliases is built from the physical home, the lexical repo-root spelling written into PYTHONPATH by the launcher (D:\hermes\hermes-agent) is not derivable, and the entry survives stripping (reproduced: cases --profile default / named / sticky active_profile / cross-drive junction all leave it in place; no-profile strips it). Two narrow changes, no heuristics, no new env vars: - hermes_cli/profiles.py::resolve_profile_env: when HERMES_HOME is set, the configured spelling IS the launch root (junction-transparent, physically identical dirs); keep it instead of re-deriving the native default. This is the same producer contract _preserve_hermes_home_path already follows. - tools/environments/local.py::_build_hermes_repo_root_aliases: when the configured home is a profile home (<root>/profiles/<name>), also derive the root spelling lexically (parent of the profiles component, same rule get_default_hermes_root uses) and run the exact-ownership mapping against it, so the launcher's lexical root is recovered after re-home without ever matching arbitrary descendants of HERMES_HOME. Regression test test_profile_rehome_keeps_junction_lexical_alias covers junction + profile re-home + inherited lexical PYTHONPATH end to end.
The junction fix made resolve_profile_env preserve the configured HERMES_HOME spelling as the launch root. Cover the four pre-existing resolution invariants so the spelling-preservation never regresses them: - root env + named profile -> <root>/profiles/<name> - profile-shaped env + named profile -> <root>/profiles/<name> (no nesting) - profile-shaped env + default -> <root> - custom root env never falls back to the platform default Plus existence/validation semantics (missing named profile still raises FileNotFoundError) and the unset-env fallback contract.
Second real-world topology reported and confirmed on native Windows 11: the repository itself is a cross-drive junction (D:\hermes\hermes-agent -> C:\...\hermes-agent) under a real HERMES_HOME directory. The editable import spelling resolves to the physical location, so _hermes_repo_root is physical while the launcher writes the lexical spelling into PYTHONPATH. The home-relative mapping cannot express a cross-drive link (commonpath raises on different drives), so the lexical repo root survives stripping; and with the repo alias missing, a lexical VIRTUAL_ENV (D:\hermes\hermes-agent\venv) also fails _validated_runtime_venv, so the venv site-packages survives too (uv-base gateway: both entries survive). Fix: after the existing home/profile-root mapping, try the single deterministic candidate <lexical root>/<repo dirname> for every trusted home candidate (configured home, plus the profile root when the configured home is a profile path) and accept it only when strict resolve proves it is the exact physical repo root (fail-closed: missing paths, real directories that are not the known repo, and unrelated spellings are never aliased). This also re-enables the VIRTUAL_ENV validation for lexical venv spellings, so uv-base gateway site-packages cleanup follows the repo alias. Tests: repo-level junction positive + negative control (same-named real directory preserved), profile-home + repo-level junction combination, lexical VIRTUAL_ENV validation after recovery (root + site-packages stripped, user entries kept), and a no-provenance lookalike preserved. The execute_code composition test now compares composed paths with os.path.normcase so a Windows case-only spelling difference (resolve() vs abspath() casing) can never fail the composition contract.
Same behavior, same coverage, less boilerplate (test file 1691 -> 1512 lines; PR diff unchanged in semantics). Production (mechanical only): - Extract _strip_hermes_owned_pythonpath_and_runtime_markers(): the three builders (_make_run_env, _sanitize_subprocess_env, hermes_subprocess_env) ran the identical strip-then-pop-markers sequence in the same order (ordering is load-bearing for VIRTUAL_ENV validation); the helper makes that explicit once instead of three times. Tests: - Non-owned preservation: 11 single-shape tests -> one parametrized matrix (user/Nix/other-version/python2.7/pythonX.Y-contained/raw spelling/empty component/empty PYTHONPATH) + one runtime-shaped matrix (other-version SP, venv-SP descendant, repo direct child, repo deep child). - Owned stripping: venv SP, repo root (independent parents[2] computation), duplicates, all-owned key removal, mixed ordering -> one matrix. - Builder integration: _make_run_env/_sanitize_subprocess_env/ hermes_subprocess_env venv-SP stripping -> one parametrized test; same for the four PYTHONHOME builders (incl. build_subprocess_env). - Junction: same-named non-owned negative control now covers both the configured-root location and an unrelated location; shared _physical_repo_root helper; profile resolution matrix (root->named, profile-shaped->named no nesting, profile-shaped->default, custom root). - Every independent proof preserved: home-level junction, repo-level junction, profile interaction, negative identity control, uv-base lexical VIRTUAL_ENV, validated/unrelated VIRTUAL_ENV, no-scrub escape hatch, #84500 same-env/external-env composition, PYTHONHOME removal, real Windows-only semantics, POSIX fail-closed backslash paths.
Shorter, single-source ownership explanation for _strip_hermes_owned_pythonpath (the code-level Check comments already carry the per-branch detail; the docstring only needs the contract).
Two independent bugs let a deleted profile reappear / leave orphaned resources on next launch: 1. hermes_cli/profiles.py's backend-process scanner required argv[0] to resolve to an executable literally named "hermes". Electron's pool-backend spawn resolves the hermes console-script shim's path and execs it via the interpreter directly (python3 /path/to/hermes ...), so argv[0] reports as "python3" and the scanner never matched the running backend -- delete removed the profile's files but left its live backend process running (still bound to a port via uvicorn), which accumulates across repeated delete/recreate cycles. 2. The desktop sidebar's ProfileRail only refreshed its cached profile list once, on mount, so a delete/create/rename from another surface (another window, or the CLI) left a stale ghost entry until something unrelated triggered a refetch. Note: a delete via this window's own Manage-Profiles view already refreshes the shared $profiles atom ProfileRail subscribes to (confirmed by reading refreshProfiles() and handleConfirmDelete()) -- this fix only covers the cross-window/cross- process staleness gap, not a duplicate of the already-merged #57329's Manage-Profiles rail-refresh work. Fix 1: recognize a python-interpreter argv[0] exec'ing a hermes-named console-script shim via argv[1]. Fix 2: refresh the profile list on window focus/visibilitychange, matching the existing pattern used elsewhere in the sidebar (sidebar/index.tsx, use-background-sync.ts, star-map.tsx, use-gateway-boot.ts all use the same focus+visibilitychange pattern). ## Related work already on main PR #57329 (merged) fixed the *headline* symptom from issue #52279 (deleted profile respawns) via a different, non-overlapping mechanism: routing profile-delete through the primary backend instead of spawning a fresh pool backend, plus a separate recreation guard in ensure_hermes_home() (#49435, merged) that makes a backend spawned into a deleted profile's directory raise FileNotFoundError instead of silently recreating it. This PR is NOT a duplicate of that fix. Verified: even with both of those merged, a backend process that survives because of gap #1 above still holds a bound port via uvicorn -- it just can no longer resurrect the profile directory. That's real resource-hygiene, not a symptom already covered. Gap #2 touches a different file/component (ProfileRail / profile-switcher.tsx) than #57329's rail-refresh half (which touched the Manage-Profiles view's own $profiles.ts / index.tsx) and covers a distinct staleness path (cross-window/cross-process, not same-window delete-then-refresh). Tests: tests/hermes_cli/test_profiles.py -- 156 passed (existing + regression coverage for the argv[0] python-interpreter detection case). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… scripts
External review (Fable) caught a real false-positive widening in the
original commit: the new argv[1] script-name check reused the loose
`script_name == "hermes" or script_name.startswith("hermes")` pattern
(copy-pasted from the exe_name check above it), but argv[1] can be ANY
user-invoked python script path when argv[0] is a bare interpreter --
unlike a directly-resolved executable name, where a false match on the
substring is rare. A user's own script named e.g. "hermes-notes.py" or
"hermes-unrelated-tool" run via `python3 <script>` would be misidentified
as the console-script shim and become killable by profile delete.
Match against the actual known console-script entry points instead
(pyproject.toml [project.scripts]: hermes, hermes-agent, hermes-acp),
stripping the script's extension before comparing.
Added 2 regression tests: one confirms the false-positive case is now
rejected (fails against the pre-fix loose-match code, confirmed via a
scripted revert), the other confirms the other two real entry points
(hermes-agent, hermes-acp) still match via the shebang-exec path.
Tests: tests/hermes_cli/test_profiles.py -- 158 passed (156 previous + 2
new).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… a tested hook Addresses review feedback from the hermes-sweeper (salvageability=high, keep_open): "The new focus/visibility listener behavior lacks a runtime UI regression test... no ProfileRail test." Rendering the full ProfileRail component for this would drag in drag-and-drop, dialogs, hotkeys, and i18n unrelated to what needs testing. Instead, extracted the focus/visibilitychange wiring into its own use-profile-rail-refresh-on-active hook, matching this exact directory's own established convention (use-profile-prewarm.ts is the same shape: a small side-effect hook pulled out of ProfileRail specifically so it's unit-testable in isolation). Added 6 tests covering exactly what the review asked for: refresh on mount, refresh on window focus, refresh on visibilitychange while visible, NO refresh on visibilitychange while hidden, listener cleanup on unmount, and no listener accumulation across repeated mount/unmount cycles. Verified the tests have real teeth: simulated the exact bug this PR originally fixed (dropped the cleanup return, leaving listeners attached after unmount) and confirmed 4 of 6 tests correctly fail against it -- including "no accumulate listeners" showing 7 calls instead of 1, the exact leaked-listener signature. Restored the real fix and all 6 pass. ProfileRail itself is otherwise unchanged in behavior -- this is a pure extraction (same effect, same dependencies, same cleanup), not a behavior change. Full sidebar test suite: 93 passed across 12 files (up from 87 across 11), 0 regressions. Python side unaffected: 158 passed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ha-0
Constructor backgroundColor with alpha is silently treated as opaque on a
non-transparent window (Electron only documents constructor alpha with
`transparent: true`), so windows created while glass was persisted were
born with an opaque backing and the vibrancy material never showed —
exactly the state a user lands in after toggling glass on and relaunching,
or when the renderer re-reports the persisted state at boot (the IPC
handler correctly dedupes it, so no runtime swap ever fired).
Measured on macOS 26 / Electron 40 (side-by-side spike windows, pixel
luminance): ctor '#00000000' = flat opaque (lum 38, same as no glass);
omitting backgroundColor entirely = vibrancy visible (lum 57). Runtime
setBackgroundColor swaps are also LOST while a fresh process's compositor
is settling — swaps at 1s/3s/6s after creation never landed, including
from 'ready-to-show' and 'did-finish-load'; a 10s swap stuck. So cold
launches must be right at creation: windowBackingOptions() spreads either
{} (glass) or the themed anti-flash backing (everything else) into the
three chat-window constructors. The runtime swap path stays for live
Settings toggles, where the window is long settled.
The mapping had grown three copies: the clear-mode ramp in electron/window-opacity.ts, a second clamp + mode normalizer in electron/translucency.ts, and a third clamp in the renderer store, with a "keep in sync" comment standing in for a shared type. Anything the two processes must agree on -- what a mode is, where the lever clamps, what intensity means as an opacity -- now lives in apps/shared/src/translucency.ts and both ends import it. electron/translucency.ts keeps only the piece that needs a BrowserWindow to mean anything (the constructor backing), and re-exports the rest so main.ts has a single import. Its relative specifier is deliberate: the electron bundle is built by esbuild with no tsconfig path resolution, so a bare @hermes/shared/translucency would typecheck and then fail to bundle -- the same constraint connection-registry.test.ts documents for backendScopeKey. The renderer's tsconfig drops its reference to the electron project. With both projects claiming the shared file, that edge made the renderer resolve it through the electron project's build output and demand a prior `tsc --build`. Nothing in src/ consumes electron's emitted types, so the reference bought nothing; `npm run typecheck` still checks both projects.
Three small dedupes in the surfaces this feature touches: - GLASS_SUPPORTED was a fourth inline copy of "am I on a Mac" in the renderer. src/lib/platform.ts owns it now and the terminal's isMacPlatform re-exports it, so the two call sites can't drift. - The store held intensity and mode as two atoms behind two localStorage keys with two subscriptions calling one sync. They are one setting: one atom, one key, one subscription. A pre-mode value under that key is a bare intensity, which has only ever meant clear -- read() keeps it there. - global.d.ts re-declared the IPC payload as an inline union. It takes TranslucencyState, so widening the state can't leave the bridge behind. isChatWindow is exported and takes its search string as a parameter, because "which windows may thin their surfaces" is the contract worth pinning.
Glass thins the field tokens, so anything that needs a fill for a reason of its own had to be exempted. Those exemptions were written as styles.css reaching into other components by class name and slot -- .cursor-grabbing, .composer-human-message-container, [data-slot='file-diff-panel'] -- which puts the knowledge in the wrong file: rename the class, lose the fill, and nothing fails until someone turns glass on over a diff. There are only two roles. A surface that MASKS its siblings (the diff gutter, a dragged row) must stay opaque or it reads as text through text. A surface RAISED above the field (overlay cards, the inline edit box) stays near-opaque and never thinner than the field behind it. Each one now says which it is at its own call site, and styles.css styles the roles. This also narrows the diff-panel rule to the sticky gutter that actually masks, rather than the whole panel.
Every chat window constructor repeated the same three translucency-related options -- the vibrancy material, the native opacity, the webContents backing -- and the glass work was about to make that four things to keep in step across three sites, with the cold-launch backing rule (omit backgroundColor, never pass alpha) restated at each. chatWindowSurfaceOptions() states it once, and the comment naming the HUD, pet overlay, quick entry and wake indicator as deliberately-not-chat-windows lives with it. The settings row reads the store's single object rather than two atoms.
Restores the four features SHL0MS built on #84329 that an earlier pass on
this branch had carved out, reconciled onto the shared translucency state
rather than the four-atom store they were written against.
- Frost picker. macOS exposes no blur-radius knob, so the vibrancy material
IS the frost control. The four in the ladder come from a pixel census on
macOS 26: the 14 Electron materials collapse to 9 distinct looks, and
these four are the widest separations that stay distinct in BOTH
appearances. sidebar/hud collapse into under-window when unfocused, which
is why they're deliberately absent -- normalizeMaterial rejects them, with
a test saying why.
- Sidebar-only glass, the Finder shape. <body> stays the single painter and
splits at the rail's live-measured edge with a hard gradient stop, so
there's no smear across the seam and no per-layer tint stacking. RTL
mirrors.
- Full-range tint. glassSurfaceKeep runs linear to zero, so the top of the
lever is bare untinted blur instead of stopping at a 30% wash. Text, cards
and the composer keep their own opaque tokens, which is what makes 100%
usable rather than unreadable.
- Peek. The settings overlay covers the very effect its slider controls, so
holding the slider ghosts the whole overlay layer and the live window
becomes the preview. A counter, not a boolean: a held drag and a timed
pulse from a picker click overlap, and the drag must not be cancelled by a
pulse expiring underneath it.
One change from the original: the peek's transition is scoped with :has() to
the overlay that arms it. The version on #84329 shipped a bare
`[data-overlay-surface] { transition: opacity 420ms }`, which gave every
overlay in the app -- command center, cron, agents, model picker -- a 420ms
opacity transition for the life of the process to serve one slider. Verified
by running the candidate rules through lightningcss: the scoped selectors
survive minification and no un-gated overlay transition remains.
visualEffectState is pinned to 'active' at each chat window, because several
materials collapse to a shared inactive look on blur -- without it the frost
choice silently erases itself whenever the user clicks another app.
Co-authored-by: SHL0MS <SHL0MS@users.noreply.github.com>
jsdom reports an EMPTY navigator.platform and a userAgent of "darwin", so GLASS_SUPPORTED resolved false and every `if (GLASS_SUPPORTED)` assertion in the store suite took its else-branch — on a Mac too. The suite was green because it was checking that glass does nothing. Found by mutation: removing the `data-hermes-glass-scope` cleanup, hardcoding the keep percentage, and dropping the isChatWindow guard all survived a full run. Pinning navigator.platform before the store module evaluates makes the glass path the one under test, and all three now fail as they should. Also adds the coverage those mutants exposed as missing: - Each of the three ways glass can end (intensity to zero, mode to clear, window kind) clears the scope attribute independently — a stale scope keeps the split-paint gradient selector live over a non-glass field. - The store must CONSULT isChatWindow, not merely export it. Driving window.location.search proves the HUD / pet overlay / quick entry don't get their page surfaces rewritten while still honouring the user's saved mode. - A guard asserting GLASS_SUPPORTED is true in this environment, so if the env ever stops reporting mac the suite fails instead of quietly hollowing itself out again.
Dragging the intensity slider was janky, and the four frost levels looked nearly identical. Same cause. At step=1 a drag emits ~100 updates, and every one of them did four expensive things: a synchronous localStorage.setItem, an IPC wake, a synchronous fs.writeFileSync in main, and setVibrancy + setBackgroundColor on every open window. The vibrancy call is the one that also broke the frost picker — it animates over 150ms, so re-issuing it per tick restarted the animation before macOS could ever settle the material. The levels weren't indistinguishable, they were never finishing. The fix follows from a property the mode already has: under glass the intensity is a pure renderer concern. windowOpacityFor returns 1 for the whole range, so main has nothing to do on an intensity change at all. - main diffs the incoming state against the current one and passes a `changed` set to applyWindowTranslucency. An intensity-only change under glass now touches zero native properties. Crossing zero still moves the backing, since that flips glass on and off. - main's disk write is coalesced onto a 250ms trailing timer, flushed on before-quit. Only a cold launch reads that file. - the renderer paints every tick (the field has to track the hand) but coalesces the localStorage write and the IPC send onto a 120ms trailing timer, flushed on pagehide. Covered as contracts rather than timings: a table asserting exactly what each kind of update changes natively (nothing, across the whole intensity range under glass), and store tests that a six-tick drag produces one write and one IPC call while every intermediate value still paints. Both directions mutation-checked — restoring per-tick writes fails, and debouncing the paint fails too.
The perf pass over-corrected: it debounced the renderer's IPC send along with the localStorage write. Under glass that was harmless (the renderer paints the effect itself), but in clear mode the effect IS the native window opacity, and main can only move it when told — so dragging the slider did nothing for 120ms and then snapped to the released value. Exactly the jank the debounce was meant to remove, reintroduced on the other mode. The send is per-tick again; only the localStorage write stays debounced. Per-tick sends are cheap now because main diffs the state: one setOpacity in clear mode, nothing at all under glass. The store test asserting the send was coalesced encoded the bug — flipped to assert every tick reaches the bridge, with a comment naming the rule.
Audit pass over the whole feature's lifecycle, closing four holes: - Stuck peek. Escape mid-drag unmounts the slider before its pointerup ever fires, stranding the counter above zero — every LATER settings overlay then renders ghosted at 8% opacity. The appearance surface now drops all outstanding holds on unmount (resetTranslucencyPeek); expiring pulse timers become no-ops on the zero floor. - Frozen sibling windows. Under glass an intensity change touches nothing native (by design, that's the perf fix), so a second chat window never heard about it and its tint froze until reload. The store now adopts a sibling's persisted state off the storage event — the same cross-window pattern themes/context and store/session already use. - Layout thrash on the sidebar-scope drag. startRailTracking re-measured the rail on every store sync: a getBoundingClientRect (forced layout) right after the tint's style write, once per slider tick. While tracking is live, the ResizeObserver and the resize listener own geometry; the settled hot path is now one boolean check. Re-acquisition still covers the two real cases — a rail that hasn't mounted yet, and a rail that REMOUNTED (layout reset swaps the element, leaving the observer on a detached node that never fires again). - The rail observer/listener pair was already torn down whenever glass or the sidebar scope ends (stopRailTracking) — audited, covered by the scope attribute tests, unchanged.
Pre-handoff polish, no behavior change. The three translucency pickers shared a verbatim five-line onChange (haptic, set, conditional pulse) — one pickTranslucency helper now serves mode, frost and area. Dead re-exports cut: the electron adapter no longer forwards renderer-only symbols (TRANSLUCENCY_STEP, TranslucencyMode, GlassScope), and the store's re-export block shrinks to what its call sites read; the store test takes the shared constants from @hermes/shared/translucency directly.
Window Translucency shipped defaulting to Clear, which means the mode worth finding is the one nobody sees — Glass is the better-looking half and the reason the feature exists. A fresh macOS profile now starts with Glass selected. Nothing turns on. The intensity still defaults to 0, so the window is byte-for-byte what it is today until the user moves the lever; the default only decides which mode that lever will drive. windowOpacityFor stays 1 and the window is still born with its opaque backing. The one profile that must NOT flip is one already carrying a non-zero intensity with no mode recorded: it predates the setting, has been rendering as clear the whole time, and defaulting it to glass would change a window someone deliberately tuned. normalizeMode takes the saved intensity and keeps those on clear. The renderer store was hand-rolling its own copy of this rule, so it now routes through the shared normalizer and the two can't disagree. The store's default test only passed because a beforeEach reset the atom before it looked — it asserted the post-reset value, not the default, so it would have stayed green through this change. It now snapshots the atom at import time. All three mutations (default back to clear, escape hatch removed, glass leaking onto non-mac) fail the suite.
The empty cronjobs list rendered both a calendar-icon placeholder blurb
("Cronjobs are recurring tasks this agent runs on a schedule.") and the
Create Cronjob button — two elements saying the same thing (empty).
Per Teknium's review, drop the generic placeholder and keep only the
create button. The filter hint ("jobs exist but are hidden by the bot
filter") still renders, since it carries real information rather than
just marking emptiness.
…ptor connectionId (salvage #88697) compose without re-appending the twin-address primary
…ture branch — switches back when safe, warns loudly when not Live incident 2026-08-17: the source checkout was parked on a stale feature branch (claude-code-inspired/local-terminal-memory-limit, days behind main), left there by earlier tooling. 'hermes update' autostashed, refreshed lazy backends, synced skills, and printed '✓ Code updated!' / '✓ Update complete!' while the checkout stayed on the stale branch with none of main's new code. Two sessions burned time on 'the fix is missing' confusion. - Parked-branch guard: auto-switch back to the update target ONLY when the parked branch is clean and fully merged (git cherry origin/<target> shows nothing unmerged); the checkout then STAYS on the target instead of being re-parked. Otherwise: loud CODE UPDATE SKIPPED block naming the branch, behind-count, and resolution commands; exit 1; branch untouched. - The up-to-date (commit_count == 0) path no longer switches back to a fully-merged parked branch either. - Post-pull gate additionally refuses to print '✓ Code updated!' when HEAD ends up attached to a non-target branch. - Summary lines now carry the actual branch + HEAD short-sha: '✓ Update complete! [main @ 30fcf95]' — drift visible at a glance. - New config toggle updates.auto_switch_parked_branch (default true). - Real-git-fixture regression tests (init/clone/branch, no subprocess mocks): clean+merged auto-switch, dirty skip, unmerged skip, cherry-picked equivalence, config opt-out, unverifiable ref, on-main fast path, up-to-date no-repark, summary branch/sha assertions.
…suffix test_update_hangup_protection pinned the exact stdout of _print_update_completion; the new branch+HEAD suffix (parked-branch guard) broke that pin. The two receipt tests assert the action-identity contract, not the branch display, so they now neutralize _branch_head_suffix — the suffix behavior itself is covered by test_update_parked_branch_guard.py.
…parent-agent rebuilds, and child-started process notifications carry delegation attribution Control path: delegate_task(action=list/steer/stop) resolved ownership purely through the _delegate_parent_ref weakref identity chain. The CLI rebuilds its AIAgent mid-session (self.agent = None on route-signature change, credential refresh, /model, MoA one-shots), so a running child's chain pointed at a dead object and the child went invisible/unsteerable while completion delivery (durable session-id routed) still worked. Observed live 2026-08-17: deleg_88454b70 / sa-0-dc0100f4. Fix: register each child with the owning conversation's durable session id (owner_agent_session_id, the same spine delivery routes by) and add a second ownership tier that matches it against the calling parent's session_id with compression-lineage resolution on both sides. Foreign sessions still fail closed. Presentation path: background processes started BY a subagent (task_id == subagent_id) route their notify_on_complete notifications to the parent conversation by design, but arrived as anonymous raw output walls. The formatter now resolves the task_id against the live + recently-finished subagent registry (bounded retention survives child completion) and adds a provenance line (subagent id, delegation id, goal snippet), trimming the output tail for subagent-owned processes. Parent-owned process notifications are byte-identical to before.
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…just the legacy scalar With multi-group membership, a bot's meta carries groups[]; the create dialog's taken-name scan must union all of them (botGroups) or a name only present as a secondary membership could be reused and resurrect that room.
Contributor
૮ >ﻌ< ა ci reviewrunning on 8c4b9db — WIP: upstream sync with conflicts (resolve before merging) waiting for jobs to start… |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Conflicts detected. Resolve manually before marking ready.
Files with conflict markers:
Brings in upstream commits up to
acc614e72(upstream/main).Generated by
ops/hermes-upstream-sync. Branch protection onmainenforces that the agent cannot self-merge — review and merge manually.