_spawn_detached now forwards home= to _build_gateway_argv so the post-update
cold-start can launch another profile's gateway; the two windows_only spawn
tests stubbed _build_gateway_argv with a zero-arg lambda and raised TypeError
on the Windows runner.
- Run `_cold_start_attested_profiles` AFTER the paused profiles are relaunched,
so a sibling that fails to cold-start can never keep the profiles that were
running from coming back.
- A profile that stays down raises like the active-profile cold-start does;
the merged outcome then marks the update incomplete instead of printing
success with a gateway still offline.
- Trim the salvaged tests to the two invariants (dead-attested default beside
a live beta is cold-started under its own home; every dead-attested profile
is cold-started when nothing runs, active first). The "no attestation →
token unchanged" case is the pre-existing behaviour already covered by
test_pause_skips_cold_start_plan_when_desktop_owns_lifecycle.
Review follow-ups on the per-profile obligation:
- Order: the active profile's cold-start guard is fleet-wide (any live
gateway ⇒ done), so a sibling spawned first would have left the active
profile down. Per-profile spawns now run after it.
- A token is built even when the active plan owes nothing (clean exit,
autostart not installed), so a dead-attested sibling still rides on it.
- Readiness for a per-profile spawn is probed in THAT home's identity files
(`_live_gateway_pids(home=)` → `get_running_pid(home/"gateway.pid")`), not
the fleet: a still-running sibling no longer vouches for a dead spawn. The
wait/confirm helpers share one probe function.
- The new PID is attested in the profile home (`_write_start_attestation(...,
home=)`) so a death after the CLI exits reaches the next update, and an
already-live profile is not spawned twice.
`_pause_windows_gateways_for_update` built the attested cold-start plan only when the
ALL-profile running PID list was empty, so a default gateway that died after a ✓ beside a
still-running `beta` never received a cold-start obligation and stayed down after the
update (#110959, fifth review thread on #110020).
- gateway_windows: thread `home` through `_start_attestation_path` → `_read_start_attestation`
→ `_attested_pid_exited_cleanly`/`_attested_dead` → `attested_death_generation(pids, home=)`
and `_consume_start_attestation(gen, home=)`; `_spawn_detached(home=)` builds the argv/env
for that profile home (`--profile` derived by `_launcher_settings`). Defaults unchanged.
- update_cmd_windows: `_record_attested_cold_start_profiles` evaluates every
`profiles_to_serve(multiplex=True)` profile that is not running (Desktop-owned installs only,
active profile left to the existing plan so nothing is spawned twice) and records
`token["cold_start_profiles"] = {name: generation}` — a sibling key, so `profiles[name]`
stays an int for relaunch/verify. `_cold_start_attested_profiles` runs on resume before the
ordinary relaunch, spawns under the profile home, waits for readiness, consumes exactly that
generation; one profile's failure never aborts the others.
- tests: default dead-attested + beta running → token carries the obligation, resume spawns
under the default home and consumes its marker while beta is relaunched; no attested
profile → token and resume unchanged.
Why: the rebase conflict resolution in tests/tools/test_model_tools.py
deleted the unrelated TestBridgeDispatch class (3 tests from 73163e3);
it is restored verbatim from main with TestBrowserRetrievalHints after it.
The fix had five tests for one invariant: the OPENAI_MODEL_EXECUTION_GUIDANCE
check duplicated test_phantom_tool_references, and the two static-schema
"toolset-neutral" checks are now folded into test_silent_without_web_tools,
which runs _apply_dynamic_schemas over the real browser_navigate/browser_cdp
schemas so the rendered descriptions are what is asserted.
execution_guidance_text() no longer takes valid_tool_names: the guidance is
toolset-neutral, so the parameter was ignored; the single caller in
agent/system_prompt.py and its test are updated.
The rebased guidance text no longer names web_search anywhere, so
execution_guidance_text()'s replace() calls (3733e4aff5) matched
nothing and were dead; the function now returns the neutral text for
every toolset and its phantom-tool test asserts "no web tool named"
instead of the removed sentence. model_tools ports the PR's hint layer
into main's _DYNAMIC_SCHEMA_REWRITERS table (browser_navigate +
browser_cdp) rather than a second pass after it.
Tests: the two browser_cdp registry tests were re-added by the PR but
main pruned them in 39975613b13b4; replaced with one schema-neutrality
invariant. Exact-wording assertions ("lightweight retrieval tool",
"appropriate permitted retrieval/search tool") were change detectors and
are dropped. tools-reference.md row updated to the new schema text.
rename_profile moves the profile directory while this process may still
hold the cached per-profile mcp-stderr.log handle (left behind by a
completed probe or a running server). On Windows a directory containing
an open file cannot be renamed, the same WinError class delete_profile
now avoids. On other platforms the stale handle stayed cached under the
old home key, so a later probe on the renamed profile opened a second
handle and a new profile re-created under the old name wrote its MCP
stderr into the renamed profile's log. Release the scoped handle next to
the multiplexer unroute, mirroring delete_profile.
Review finding: rename_profile missed the sibling surface of the
delete_profile handle release.
Squashed integration of the user-facing message audit for this surface set.
Full per-finding receipts: /tmp/ux-audit/lanes/*-receipt.md (campaign artifacts).
The salvaged fix replaced the early return with a `pass` + `elif`;
fold it into a single `if primary and rerouted is not None` so the
probe is reached by construction and the WHY lives in one comment.
resolve_hermes_bin's rerun hides argv[0], so a non-None reroute could
only come from PATH — which the first call would already have returned.
primary is None therefore implies rerouted is None, and the fall-through
is equivalent to return rerouted or primary; only the durable-wrapper
probe below can still find anything on that branch. (KeyArgo's review
asked why the branch was merged rather than dropped.)
_resolve_hermes_bin_for_desktop_entry returned the resolver's None
outright when neither argv[0] nor PATH yielded a launcher (a cold
relaunch under `python -m` with a stripped PATH), skipping the
known-wrapper probe its own docstring describes. The persisted
`Exec=` then flipped to the bare module form, while a DE- or
terminal-launched context renders the wrapper form.
Every flip rewrites ~/.local/share/applications/hermes.desktop on the
next launch. gnome-shell 50.x removes a ShellApp from id_to_app on any
.desktop content change without checking its state; if the app was
still STARTING, the last strong reference drops and a later GC-triggered
dispose trips shell-app.c's `state == STOPPED` assertion — taking the
whole Wayland session down (observed crash, 50.4-1.fc44).
Probe the installer's known wrapper locations on the None path too, so
the entry converges on the durable wrapper wherever one exists and only
falls back to the module form when none does. A missing entry being
(re)created can still change the file regardless — this removes the
gratuitous rewrites, not the rescan trigger itself.
The "named once" guarantee only held when open() itself failed. In the reported
case open() succeeds and write/seek/flush raise EIO, so every record went
handleError -> stream=None -> next emit reopens -> _open() reset
_unavailable_reported -> the path line was printed once per record (25 for 25).
Reset the flag in emit() only after a record actually reached the stream; that
is the moment the destination has recovered.
Adds the open-succeeds/I-O-fails case to the test file: red on the previous
head, green here.
Review finding: EIO on write after a successful reopen printed the path per record.
bounded_probe_run collapses spawn failure and timeout into one None, so a venv
interpreter that exists but cannot be executed (PermissionError, ENOEXEC, fork
failure) was reported as "timed out before reporting import health" -- a bogus
warning on the git path and sys.exit(1) on the ZIP path -- and the
`except OSError: return {}` branch beneath it was dead. Let the Popen error
propagate (opt-in flag, default unchanged for the other probes) so "we could not
run our own probe" says nothing about the checkout and no longer blocks the
update, while a spawned child that hangs is still a verdict.
Restores the non-fatal test the earlier rewrite dropped, now driving the real
bounded_probe_run/Popen: red on the previous head, green here.
Review finding: spawn failure of the import probe reported as a fatal timeout.
Builds on the salvaged EIO suppression: the reporter asked that logging
"degrade gracefully and identify the affected path". Print one stderr line
naming the file and errno when the stream first fails, reset the flag when
`_open()` succeeds again so a later failure is reported anew. The contributor's
test is replaced by one invariant: five records through a stream raising EIO
produce zero tracebacks and exactly one path mention, and the next emit
recovers into the real file.
The critical-module import probe (`_critical_module_import_failures`) imports
`run_agent`, whose module-level `load_hermes_dotenv()` resolves every enabled
external secret source. The main-process skip only lived in `hermes_cli.main`'s
own dotenv call (`load_external_secrets=sys.argv[1:2] != ["update"]`), so the
probe child ran op/bws/command helpers with a 120s per-source budget inside the
120s probe and a healthy install reported "critical-module probe still fails to
import after updating: timed out before reporting import health" (#110823).
Move the argv check into `_early_recovery._should_skip_external_secret_sources`,
which every dotenv load already consults, and stamp `sys.argv = ['hermes',
'update']` into the probe so its imports inherit the updater contract.
Invariant test spawns the real probe against a home whose configured helper
touches a marker: red on main, green here.
_purge_stale_hermes_modules evicts hermes_cli.* but not root modules
(hermes_constants, utils, toolsets, ...), so the post-purge
`from hermes_cli.config import ...` re-executes the NEW config.py against
OLD cached root modules. When a pull adds a root-module symbol that
config.py imports at module level, that raises ImportError. The import
sat outside the step's try/except, so the error escaped
_run_post_update_maintenance and aborted the fleet restart -- where
origin/main printed the "Could not check config version / run hermes
config migrate" fallback and continued. Move the purge, reload and
import inside the existing try so the step stays fail-open.
Test: a cached hermes_constants lacking get_process_hermes_home (the
symbol hermes_cli/config.py imports at module level) must print the
fallback and return instead of raising. Red before, green after.
Review finding: fail-open -> fail-closed flip; post-purge hermes_cli.config import outside the try let ImportError abort post-update maintenance.
The pinned-list reload (tools_config) fixes the reported symbol; any
future symbol a pull adds to any module a migration imports at call time
(agent.skill_utils, hermes_cli.toolset_scope, ...) would fail the same
way. Evict every cached Hermes module at the migration entry point with
_purge_stale_hermes_modules — the class fix the fleet-restart phase
already relies on — so the migration graph is rebuilt from the new tree.
Tests: the reporter's exact shape (cached tools_config lacking
_configurable_keys, on-disk v44 config with an explicit platform
toolset list) must still migrate v44 -> v45 through the updater's own
entry point. Existing entry-point tests stub the purge like the rest of
the update suite does.
The updater process is the pre-pull process: hermes_cli.tools_config cached in sys.modules lacks symbols the pull added, so a migration that imports them at call time fails with ImportError and the config silently stays at the old version (#111271).
Review follow-up on #91651: _recover_terminal_input_modes and the termios
drift check used the same `now - 0.0 < interval` idiom as the repaint
throttles. Their windows (0.5s / 1.0s) are unreachable in practice, but
converting them to the None sentinel retires the bug class instead of the
instance, so nobody "simplifies" a None back to 0.0 later. Also adds the
missing regression test for _invalidate, the throttle the PR title is about.
time.monotonic() counts from an arbitrary epoch (boot on Linux). Two CLI
repaint throttles used 0.0 as the never-fired sentinel, so on a freshly
booted VM (CI runners, containers) now - 0.0 < min_interval suppressed
the FIRST repaint ever requested:
- _schedule_focus_regain_redraw: min_interval=60 suppressed the first
focus-regain redraw whenever uptime < 60s — the exact failure in CI
run 32494557030 (test_focus_regain_redraw_is_rate_limited, both
attempts red on a fresh runner, green everywhere else).
- _invalidate: same 0.0 sentinel; a first spinner/stream repaint inside
the first 250ms of uptime was droppable the same way.
Both now use None as the never-fired sentinel. Regression test pins
monotonic()=3.0 with min_interval=60 and asserts the first redraw fires.
_reload_dynamic_routes runs from _handle_webhook and admitted routes through
_dynamic_route_allowed without validate_coalesce_config, so a dynamic route
with a malformed coalesce block (no key, non-numeric window) raised inside
the request handler on its first event instead of being rejected up front
like a static route. The admission check now runs the same validator and
skips (warns on) the offending route. The parametrized rejection test covers
the dynamic path too; upstream-source reference dropped from the module
docstring.
Rapid distinct events on the same logical entity (five pushes to one PR,
a burst of ticket edits, a flapping alert) each carry a fresh delivery ID,
so the idempotency cache cannot suppress them and every event wakes a
separate agent run. Roomote solved this for PR review tasks by keeping one
durable review task per PR and superseding stale heads; this ports the
same debounce-and-supersede pattern to the generic webhook adapter.
New opt-in per-route 'coalesce' block: events group by a payload-derived
key, each new event replaces the pending one and re-arms a quiet-window
timer (window_seconds, default 30), bounded by max_wait_seconds (default
300) past the group's first event so a steady stream cannot starve
dispatch. The settled group dispatches ONE agent run on the latest
event's payload/prompt/delivery templates, with a note when earlier
events were superseded. Pending groups flush on disconnect. Startup
validation rejects missing keys, non-positive windows, and the
deliver_only+coalesce combination.
Rebase onto the decomposed webhook adapter (salvage, #92066):
- Coalescing lives in a topical sibling, gateway/platforms/webhook_coalesce.py
(WebhookCoalescer + validate_coalesce_config); webhook.py only wires it in
(__init__, _validate_route, disconnect, _handle_webhook) and splits main's
_dispatch_agent_run into the HTTP-response wrapper plus _spawn_agent_run,
shared by the immediate and coalesced paths.
- Review finding (unresolved key fields collapsed unrelated entities into one
group): an event whose rendered key still contains a {placeholder} is now
dispatched immediately instead of coalesced; documented.
- Review finding (flush-on-disconnect vs process exit): disconnect() awaits
the handoff of flushed runs; the docs claim is scoped to adapter disconnect
and states that a hard kill loses the current window's buffer.
- cron_job + coalesce is rejected like deliver_only + coalesce (cron_job
landed on main after the PR branched).
- Tests trimmed from 17 to 4 (validation parametrized; debounce/supersede/
independent groups/duplicate-first in one behavioural test; max-wait +
unresolved-key; flush-on-disconnect).
The dashboard's system-scope elevation gate hard-failed on a refused
`sudo -n true`, but the `hermes update` fleet restart it claims to
mirror treats that blanket probe as inconclusive and falls back to
probing the targeted command. A host with a sudoers entry scoped to
the hermes command (the hardened shape) therefore updated fine from
the CLI while the dashboard reported "passwordless sudo is
unavailable".
Factor the fleet's two-step probe into update_cmd_fleet
._sudo_noninteractive_ok and call it from both sites: the fleet keeps
its `reset-failed <unit>` fallback, the dashboard falls back to a
non-destructive `sudo -n -l -- <exact argv>` check before spawning.
Review finding: dashboard sudo gate diverged from the fleet posture it
borrowed (_needs_sudo only) and rejected targeted NOPASSWD sudoers.
The root/sudo decision now reuses `update_cmd_fleet._needs_sudo` (the helper `hermes update`'s
own fleet restart already uses for `sudo -n systemctl --no-ask-password`) instead of a second
euid check. Tests reduced to one parametrized argv invariant (system-scope lifecycle verbs get
`sudo -n`; status and both-units-installed never do) plus the no-passwordless-sudo request
failure. Dashboard docs note the passwordless-sudo requirement on system-scope installs.
The Restart Gateway button spawned `hermes gateway restart` as the dashboard's
own user. On a systemd *system* install the CLI refuses every lifecycle verb
below root (`_require_root_for_system_service`), so the button could never work:
the refusal landed in `~/.hermes/logs/gateway-restart.log` while the endpoint
reported a started action. `start`/`stop` shared the defect.
`_spawn_hermes_action` now prefixes `sudo -n` for gateway restart/start/stop when
the action resolves to the system unit. Scope comes from the CLI's own picker
(`_select_systemd_scope`) evaluated against the profile the action addresses, so
a host with a user unit installed keeps the unprivileged path and root never
shells out to sudo. With no passwordless path the request fails with an
actionable message instead of reporting a started action whose child refuses.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
gateway_declares_external_supervisor treated the control-socket identify answer
as final whenever `supervisor` was non-empty and only accepted "external".
_detect_supervisor() answers "systemd" as soon as INVOCATION_ID is set (and
"launchd" under the XPC name), so a gateway run by a custom, non-canonical
systemd unit / launchd agent with `ExecStart=... gateway run --external-supervisor`
— the documented contract — was classified non-external and `hermes gateway
restart` still took the stop + foreground run_gateway path that stamps the CLI's
PID and wedges every respawn (#110637). The update path's argv check on the same
gateway already said external-supervisor, so the two restart paths disagreed.
Any self-declared supervisor other than "manual" now means hand back (the
declaring supervisor owns the respawn); otherwise the argv/state-file marker
decides as before. The drain-failure message no longer claims a timeout when
SIGUSR1 could not be sent at all.
Review finding: socket-first detection classified a custom systemd unit's
--external-supervisor gateway as manual and took the wedge path.
The handback logic was appended to the hermes_cli/gateway.py facade; it now lives in a
topical sibling. Supervisor detection also reads the gateway's own declaration (control
socket `identify` -> supervisor: "external", then the live argv marker, then the argv the
gateway stamped into gateway_state.json) so a gateway whose command line cannot be read via
psutil is still handed back rather than SIGTERMed and shadowed by a foreground run.
Tests trimmed to the two invariants (handback with fresh-PID success; either failure branch
never takes ownership) plus the plain-manual control. Docs: `hermes gateway restart` is now
part of the --external-supervisor contract.
Address review feedback on the SIGUSR1 handback:
- A graceful exit no longer reports success by itself: the pidfile is
polled for the supervisor's replacement (bounded wait, identity via
get_running_pid's lock+PID liveness check, freshness via != old PID)
before the restart is called done. An unloaded/broken supervisor now
surfaces as a failed restart instead of success printed over a dead
gateway — the same contract launchd_restart enforces through
_wait_for_launchd_service_pid.
- A drain timeout no longer falls back to SIGTERM + foreground run: that
fallback recreated the competing-owner wedge (#110637) this handback
exists to remove. Both failure branches keep the supervisor as the
sole restart owner and fail loudly (exit 1) instead.
Credit: both lifecycle branches flagged by @ehz0ah in review.
A custom launchd agent (plist outside the canonical ai.hermes.gateway path)
is invisible to _installed_service_kind_for, so 'hermes gateway restart'
fell through to the manual stop + foreground run_gateway fallback. The
foreground run stamps the restart CLI's own PID into gateway.pid, and every
KeepAlive respawn of 'gateway run --external-supervisor' then refuses with
'Gateway already running (PID <restart>)' — the gateway stays down until
the restart process is killed (#110637).
Mirror the update path (_prepare_profile_gateway_update_restart): trust the
--external-supervisor argv marker on the live gateway and SIGUSR1 it
(_graceful_restart_via_sigusr1) so it drains, exits, and lets the supervisor
relaunch it. Drain failure falls back to the existing stop/restart path.
Fixes#110637
_marker_only_restart_obsolete cleared the marker when every row from
collect_fleet_versions() was current. At CLI startup the probe runs without
pre_restart_pids, so a gateway the restart phase stopped and never brought
back produces no row at all (dead-pid records are skipped, no DOWN
classification possible). With alpha current and beta down the marker was
discharged and beta's catch-up restart never happened.
Before clearing, also require every gateway identity latest.json owes
(plan.runtimes / fleet entries) to be covered by a current row; the
rows-only rule applies only when the receipt names no gateways. The
identity extraction is shared with _live_fleet_covers_receipt.
Review finding: marker discharged with a DOWN sibling gateway absent from the startup probe.
collect_fleet_versions now classifies a live pid from the state file only when the
home's identity resolver verifies it (#110420); these two tests write the record about
the pytest process itself, so they verify it explicitly. Dead-pid rows are unchanged.
collect_fleet_versions now classifies a gateway_state.json row current/stale
only when live_gateway_pid_for_home verifies that PID; a live non-gateway
writer (this test process) is an unknown row with no self-reported sha
(#110420). The sibling test asserted the pre-fix 'stale' contract; it now
covers both branches: unverified -> unknown, verified -> stale.
The five #105417 tests collapse into one positive (fleet current at expected_sha
discharges the marker) and one parametrized negative (stale row / empty probe /
unknown identity / checkout moved past the marker keep the warning). Both are red
on origin/main. collect_fleet_versions' docstring now names why a live PID alone
is not a `current` row (#110420): write_runtime_status re-stamps pid/code_sha for
whatever process writes it, so the state-file fallback must pass
live_gateway_pid_for_home like the inventory already does (#109680).
_pending_fleet_restart_needed() returns True on marker existence alone. A
supervisor-level gateway restart (`systemctl --user restart`, launchctl, an ops
script) never passes through _clear_fleet_restart_pending_marker(), so a marker
written by a pulled update survives a restart that DID bring every live gateway
to the new code — and every later CLI call prints the interrupted-update warning
forever, a permanent false positive that trains operators to ignore it.
The comment on that branch is right that "an older receipt cannot discharge that
unknown obligation" — but the obligation is not unknown. The marker records its
own expected_sha, so it can be verified against the live fleet directly, with no
receipt involved. _live_fleet_covers_receipt() cannot substitute: it is
receipt-anchored and returns False when `owed` is empty, which is exactly the
marker-only case.
Hold the marker to the same evidence bar _live_fleet_covers_receipt() applies to
a receipt: at least one row, and every row a `current` gateway under a known
profile whose code_sha equals the marker's expected_sha, with the checkout HEAD
not moved past the marker.
Still keeps the marker (warns) on:
- stale / down rows — the restart genuinely is owed
- an all-`unknown` fleet — pre-code-identity gateways cannot prove currency
(same conservatism as #88848/#74973)
- a marker with no expected_sha — nothing to verify against
- a newer pull that moved the checkout — it owns a fresh obligation
- a probe that raises or answers empty — no proof either way
Discharging deletes the marker only after every row passes; the historical
receipt is left intact, since a supervisor restart is still not a successful
update.
Tests: discharge on a provably-current fleet; keep on stale, empty probe,
unknown identity, and newer-pull-moved-checkout (which asserts the fleet is
never probed once HEAD has moved).
When the state-file writer cannot be verified as the profile gateway, null code_version with the SHA so the row does not display a self-reported identity claim.
Co-authored-by: Cursor <cursoragent@cursor.com>
SKILL.md is restructured to authoring standard 5 (When to Use,
Prerequisites, How to Run, Quick Reference, Procedure, Pitfalls,
Verification) — headings only, upstream body text kept. The asset
references still carried the upstream per-CLI routing tables and command
lines for other agent products; those are replaced with the native
`image_generate` route (product names are allowed only in LICENSE and
credit lines). `WebSearch` residue in recon docs/refscout becomes
`web_search`.
source.mjs wrote its ~2.6MB Google Fonts metadata cache to
$TEMP||$TMPDIR||'.', which is the project cwd on most Linux shells; it
now uses os.tmpdir() and Pitfalls documents the location. Network-access
note now mentions that moodboard.mjs also downloads the image URLs the
search hosts return. Docs page regenerated for this skill only.
Review follow-ups on the port (all verified against the upstream snapshot,
which I re-downloaded and diffed: every scripts/*.mjs and template is the
upstream file byte-for-byte after CRLF→LF, except one `reference/` →
`references/` path fix; the reference docs differ only by Hermes adaptation
notes and the same path fix).
- LICENSE: header naming the upstream repo, pinned commit 9bca227d… and the
copyright holder above the verbatim MIT text.
- SKILL.md frontmatter: `author` credits the upstream human first, Hermes
Agent second (skills/AGENTS.md rule 4); `metadata.hermes.upstream` pin in
the same shape mono-color/pr-lens use; `category: creative`; H1
`# Auteur Skill` with a linked provenance blockquote.
- `platforms` gains `windows`: the declared prerequisites (Node 18+,
Playwright, optional ffmpeg) all run on Windows and no script uses a
POSIX-only primitive (audited: no /tmp, spawn/exec of shells, fcntl, etc).
- Routing examples translated from Russian to English (marked as translated
from upstream) so an English-language skill doesn't carry stray artefacts.
- tests/skills/test_auteur_skill.py: keep the two port-specific invariants
(path annotations, de-Claude residue). Dropped the exact-count tree
snapshot (change detector), the `~/.hermes/hermes-agent` host-dependent
related_skills fallback, and the frontmatter/description checks that
tests/skills/test_authoring_standards.py already enforces repo-wide.
- Regenerated the docs page with website/scripts/generate-skill-docs.py
(scoped to this skill).
The scope-dispatching cron worker runs inside the container there, so a
host user manager would start for nothing (as PR #110641 by @liuhao1024
also gated it). nix-setup.md gains the note operators need when they
declare the user themselves.
teknium1