Drive CLIChatTurnMixin._chat_render_turn through a minimal stub instead of a full
HermesCLI (prompt_toolkit stubs, cli reload): the invariants are that a (text, images)
payload reaches _pending_input intact and that several queued parts join their text
and merge their images. Both are red on origin/main.
_clui mixin bundles Enter submissions carrying images as (text, images)
tuples, and _tui_enter_while_busy puts those tuples on _interrupt_queue in
interrupt mode. _chat_render_turn then did "\n".join(all_parts) over the raw
payloads, raising TypeError on the tuple — swallowed by the outer chat()
handler, so the interrupt message was silently lost and no next turn started.
Unpack tuple payloads in the re-queue block: join the text parts and re-attach
all images as a single (text, images) tuple, matching the shape
_tui_process_one_input already accepts. Plain-string payloads are unchanged.
authored with AI assistance (Muse, Meta's Muse Spark) under the contributor's direction; all changes reviewed and approved by the contributor
When viewer A's replay fails because viewer B superseded it mid-send, attach(A)
returns False while session.attached is True for B. The handler then logged a
misleading "pty input stalled … recycling terminal session" warning and issued
a second close on A. Checking `session._ws is ws` limits the close to the one
case it exists for: this socket is still attached and its redraw write stalled.
Review finding: superseded socket's failed replay logged a stall and re-closed itself because the gate tested session-wide state.
attach() now returns False both when the client dropped mid-replay (session
already detached, socket dead) and when the force-redraw write stalled
(socket still attached, worth closing with 1013). Only close the socket in
the second case; the registry detach stays as the guard for both.
Also trims the salvaged tests to two invariants, both red on origin/main:
a mid-replay drop leaves the session reap_idle()-reclaimable, and a drain
send failure detaches the current socket without touching a replacement
that attached during the send (#110849).
connect() called _register_slash_commands inline on every (re)connect, and
/reload-skills called refresh_skill_group inline; both run
discord_skill_commands_by_category, the same per-skill path-resolution walk
the Telegram menu paid in #110707. On a 1.5k-skill install that holds the
loop past the liveness watchdog. Registration now hops through
asyncio.to_thread from connect(); refresh_skill_group is a coroutine that
hops the rescan the same way (the reload handler already awaits an awaitable
result). Contextvar-scoped profile overrides propagate through to_thread.
One invariant test: the loop keeps ticking while the scan blocks, from both
sites. Red on the PR head, green here.
Review finding: Discord _register_slash_commands/refresh_skill_group ran the skill catalog disk scan synchronously on the event loop.
The gateway's idle-command path resolved skill slash commands inline on the
loop: a cold skill scan, skill file loads and the unavailable-skill rglob over
every skills dir. On a 1.5k-skill install that held the loop ~2 minutes, the
loop-liveness watchdog fired and the gateway exited mid-session (#111091).
_hm_skill_slash_rewrite now runs through _run_in_executor_with_context so the
profile contextvars the scan is scoped to survive the hop. Known commands still
short-circuit before any I/O (previous commit).
Same class in the Telegram inline picker: build_inline_results rebuilds the
command/skill catalog per keystroke via _collect_gateway_skill_entries, the
same path-resolution pass #110707 traced in the command menu.
One invariant test: a known command triggers no scan; an unknown command's scan
runs while the loop keeps ticking. Red on origin/main and on the reorder-only
tree, green here.
Reorder _hm_unknown_slash_reply so the I/O-free GATEWAY_KNOWN_COMMANDS
membership test short-circuits before _check_unavailable_skill walks the
skills tree (#111091). Taken from #111099 by @KoNit-K; the index cache half of
that PR is superseded by running the scan off the loop.
Follow-up to the salvaged #110716 commit. Drops the module-level
_build_telegram_command_menu wrapper (telegram_menu_max_commands is a config
read and stays on the loop; asyncio.to_thread takes kwargs directly), and
applies the same hop to _ensure_forum_commands, which rebuilds the menu on the
inbound-message path for every forum chat until registration succeeds — the
second live fire site named in #110707.
Replaces the contributor's test with one that proves the invariant for both
sites: the loop keeps ticking while telegram_menu_commands blocks. Red on
origin/main, green here.
The live branch of _run_delivery returns from _wait_live_dm before the
try/finally that removes <dm>.txt, so a settled delivery dropped its
.live.json intent but left the sibling .txt — the same plaintext — on disk.
_wait_live_dm now takes the dm file and removes both on settled; pending
and failed outcomes still keep both for the retry.
Review finding: settled live path unlinked <dm>.live.json but the sibling <dm>.txt survived.
A live DM's intent file (<dm file>.live.json — owner, delivery id and the
message plaintext) has to outlive its runner so a retry replays the same
delivery id instead of minting a second one. Nothing ever removed it: the
runner unlinks only the dm file, and cleanup_bot_dm_cache sweeps only *.txt,
so every live DM left its plaintext in the DM cache directory indefinitely.
Now the sweep reaps intents past the stale cutoff, and a delivery the owner
settled drops its intent immediately — nothing retries a settled delivery.
The salvaged switch-back regression only ever exercised the profile-less
primary shape; the mock's string branch had no writer. Publish the
profile-less local descriptor unconditionally and say why.
The local primary backend (startHermes) publishes its descriptor without a
profile field, unlike the pooled/forced-local child path. Downstream, a
profile-less descriptor normalizes to 'default', so on a switch back to the
local source the store either refused to remember the source/profile pair or
killed the commit in targetIsActive() — landing the window on 'default'
every time (#110819).
- The primary local descriptor now carries the profile it booted with, and
the unshared primary branch of ensureBackend() keeps the requested key on
the descriptor instead of dropping it.
- selectConnection() tolerates a profile-less descriptor on the source it is
landing: the activation already published the route that was asked for, so
a missing profile no longer strands the switch.
ChatPage no longer pins the page to (0, 0); keyboardRevealScrollDelta is the only caller-facing
helper left in keyboard-inset.ts. Update the isActive-effect comment that still described the pin.
Pinning the dashboard to (0, 0) on keyboard inset fights Safari's visual
viewport and leaves the Ink input line off-screen. Scroll by the delta
between the xterm host bottom and visualViewport bottom instead, and
re-sync after focus while the keyboard animates.
Fixes#110414
_is_bot_mode_session runs on every completion of every session; a non-empty
title hint already decides the answer, so only an empty hint should fall
through to the live-title lookup.
Review finding: plain sessions paid a session-store read per turn in _is_bot_mode_session.
The completion-time emptying only takes effect when nothing streamed: the
Desktop keeps already-streamed text when message.complete carries an empty
text (#95514), so a bare NO_REPLY that arrived via message.delta still
rendered in the Bot Chat bubble. Mirror gateway.stream_consumer: for Bot
Chat sessions withhold deltas while the accumulated buffer satisfies
is_partial_silence_marker; flush the held text verbatim once the buffer
diverges from every marker, so a marker never streams and prose that merely
starts like one is delivered intact.
Review finding: streamed NO_REPLY still visible in Desktop Bot Chat because message.delta carried the marker verbatim.
Follow-up to the salvaged #110786 commit:
- `_is_bot_mode_session` mirrors the system-prompt gate (`agent._session_title_hint`
first, then the live DB title) instead of reading `pending_title`/`title` off the
session dict: `pending_title` is cleared after turn 1 and the record never carries
`title`, so the contributor's gate matched only the very first Bot Chat turn.
- `tools/bot_mode_dm.py::_run_local_turn` (the `hermes -p X chat -c "Bot Chat" -Q`
transport behind `message_agent` when no live owner holds the target) re-emits ""
for a successful bare marker — the third delivery path of the same class.
- Tests trimmed to one invariant per surface (live completion, relay RPC, one-shot
transport), each proven red on origin/main sources.
- Bot Mode docs gain a "Staying silent" line pointing at the shared token list.
Three caches that read config.yaml still keyed change detection on
st_mtime (or st_mtime_ns + st_size), so a same-size replacement that
keeps the old timestamp (cp -p, rsync -t, a timestamp-pinning writer)
was never noticed:
- model_tools._tool_defs_cache_key: get_tool_definitions kept serving
stale dynamic tool schemas / mcp_servers for the process lifetime.
- CLI mcp_servers auto-reload watcher (cli_tui_mixin seed +
cli_info_mixin._check_config_mcp_changes): the replaced mcp_servers
section was never reloaded. The seed is now _config_sig.
- tui_gateway/server._load_cfg_raw / _save_cfg (_cfg_mtime -> _cfg_sig):
the raw-config cache served the stale document and the next _save_cfg
would write it back over the on-disk file.
All three now use utils.file_signature like the rest of the PR. Tests that
reset the renamed module/instance attributes follow the rename; one
pinned-mtime replacement test per cache, red on the previous head.
Also corrects two stale type/comment annotations in hermes_cli/config.py
(_env_cache key shape, _RAW_CONFIG_CACHE record shape).
Review finding: three sibling config.yaml caches (tool-defs memo, CLI mcp watcher, TUI-gateway raw cfg) still compared mtime/size only.
The cherry-picked commit extends the config/profile/MCP/managed-scope/completer/
OAuth/skills-manifest signatures. This commit finishes the class and trims it:
- `file_signature()` lives in `utils.py` next to the other stat/metadata helpers
instead of `hermes_cli.managed_scope` (gateway/ and agent/ callers no longer
reach into the managed-scope module for a generic stat helper).
- `hermes_cli/config_effective.py` was left comparing 2-/4-wide prefixes against
the widened `_RAW_CONFIG_CACHE` / `_load_config_cache_sig` records, so
`load_user_config_effective()` re-parsed on every call (3 parses for 3 calls on
an unchanged file, 1 before); index by the new widths.
- Sibling caches keyed on the same (mtime, size) shape and reading the SAME files
now use the helper: `load_env()` memo, `agent/skill_utils` raw-config and
external-dirs caches, `hermes_cli/model_switch` alias identity, `agent/moa_loop`
preset stamp, `hermes_cli/auth` global auth-store memo.
- Tests trimmed to one invariant each (pinned-mtime replacement invalidates; an
unchanged file still hits), both red on origin/main.
Left alone on purpose: `tools/registry.py`, `tools/skills_tool_dedup.py`,
`gateway/status.py`, `hermes_cli/banner.py`, `hermes_cli/main.py`,
`hermes_cli/session_recovery.py` — those fingerprint source files, PID/lock files
or write to persisted on-disk caches shared across processes, where an inode/ctime
key would churn on every checkout/copy rather than catch a replaced config.
Config caches, the profile re-scan watcher, the MCP reconciler, managed-scope
reads, the completer memo, OAuth heal marks, and the skill-manifest snapshot
keyed change detection on (st_mtime_ns, st_size) only, so a replacement that
preserved both (cp -p, rsync -t, timestamp-pinning scripts, sync clients) was
treated as unchanged and stale values were served until restart.
Add st_ino (fresh inode on atomic replace) and st_ctime_ns (cannot be
backdated via os.utime) to every signature via a shared
hermes_cli.managed_scope.file_signature() helper.
Fixes#111105
The flat-install block only ignored the bare cron/executions.db. All three
cron stores (executions, deliveries, notepad) are opened through
sqlite_util.open_db in WAL mode, so executions.db-wal/-shm exist whenever
the scheduler is live, and deliveries.db / notepad.db were not ignored at
all. `git stash push --include-untracked` therefore still unlinked the
live WAL/SHM and the whole deliveries/notepad stores under the running
gateway - the same mechanism this PR closes for the root state.db.
Switch the cron entries to the by-class shape already used at the root
(/cron/*.db plus -wal/-shm/-journal/retired-wal sidecars) and also ignore
the cron lock/heartbeat/output files and the per-launch root markers
(.update_check, gateway-starts.log, .clean_shutdown, active_profile,
.hermes_history, slack_tokens.json) that otherwise force every update
into the stash step. `git ls-files -i -c --exclude-standard` is unchanged
(no tracked file newly hidden). The test's FLAT_INSTALL_RUNTIME_STATE
gains one representative per new class; red before, green after.
Review finding: cron/executions.db-wal, cron/deliveries.db and cron/notepad.db were unignored and swept by the flat-install autostash.
`backups/` is where the pre-update snapshot the updater restores a swept
state.db FROM lives — leaving it unignored means the recovery copy is
swept together with the live database. `vault.key`/`vault.json.enc` are
the local secret vault.
Docs: website/docs/getting-started/updating.md explains that on a flat
install (checkout root == $HERMES_HOME) runtime state is git-ignored and
never enters the autostash.
scoped_spawn_lost_user_bus() re-derived the user bus from an empty base env,
so it only ever looked under /run/user/<uid>. The worker itself is launched
with systemd_user_bus_env(worker_env), which honours a configured
XDG_RUNTIME_DIR. On a host whose bus lives outside the default runtime dir,
any unrelated systemd-run exit was therefore misread as "bus gone": the job
error named a missing bus that was still there, the cached scope verdict
flipped to False, and the next 60s of cron fires dispatched without cgroup
isolation.
The check now takes the spawn env, drops the bus address the spawn already
carried, re-derives from that, and decides on the DBUS_SESSION_BUS_ADDRESS
key rather than on dict truthiness (a non-empty env with no bus was never
"bus present").
Review finding: scoped_spawn_lost_user_bus used systemd_user_bus_env({}) instead of the worker's spawn env, so a configured XDG_RUNTIME_DIR made unrelated wrapper exits flip the scope cache to unscoped dispatch.
One TTL for both probe verdicts (the success-TTL constant collapses into
`_SYSTEMD_SCOPE_PROBE_TTL_SECONDS`), and the ack-wait loop consults
`scoped_spawn_lost_user_bus()` when a scoped dispatch exits before the
worker acknowledged: with `/run/user/<uid>/bus` gone the job error names
the missing bus and the enable-linger remedy instead of the wrapper's bare
`exit 1`, and the cached True flips so the next fire degrades to a direct
external subprocess rather than consuming another occurrence on a dead
wrapper. Contributor test trimmed to the revalidation invariant.
The optional line-number gutter was written as `^(?:[ \t]*GUTTER)?[ \t]*`,
stacking two adjacent whitespace runs so _CFG_ANCHORED_RE / _YAML_ASSIGN_RE
went quadratic on any indented line with a secret keyword and no `=` (2s per
5k spaces, 50s at 20k) — and these run on every terminal output and file read.
The gutter now carries the single optional group and callers own the one
leading whitespace run. It also accepts `cat -n` / `nl` (number + TAB) and
`grep -A/-B/-C` (`5-`) gutters, which the comment claimed but leaked.
The strong-key branch masked SSH_AUTH_SOCK=$HOME/..., DOCKER_AUTH_CONFIG=/...
on shell rc reads, leaving the agent unable to edit them; a value starting with
$, / or ~ is a variable or path reference and is now kept unless the key is
password-class. _is_secret_file_arg only extends config.yaml to the
backups/config .good./.corrupt. copies, not config.yaml.pdf/.bak.
Review finding: quadratic gutter regex; cat -n/nl TAB and grep context gutters leaked; over-redaction of rc path references.
search_files over $HERMES_HOME returned the token from backups/config/config.yaml.good.<stamp>
in cleartext while config.yaml itself was masked; the predicate only matched the exact basename.
Same predicate serves the terminal side, so `cat backups/config/config.yaml.good.*` is covered too.
Five tests instead of fourteen: one parametrised table proves every assignment
shape (YAML, shell rc, dotenv, JSON, read_file and grep -n gutters, vendor
prefix) masks to the non-reusable sentinel with key/gutter/quoting intact; one
proves unclassified reads and non-secret scalars are byte-identical; one pins
the resolved-home classification (Windows home without a .hermes segment) plus
the project-config negative; the two tool-level tests drive read_file and
search_files through the real call sites, with the project config.yaml
negative folded into the read test.
read_file/search_files passed file_read=True, which folded into code_file=True and skipped
the ENV/JSON/YAML assignment passes, so an opaque prefix-less credential under a
credential-shaped key reached the model in cleartext from a secret-bearing file — the
file-read half of the #110228 gate (#110567).
Two defects on that path, both fixed here:
- The rendered line-number gutter ("5| ADS_API_TOKEN: ..." from read_file,
"6: ADS_API_TOKEN: ..." from grep -n / cat -n) defeated the line-anchored patterns,
so the real rendered read leaked exactly what the raw text masked. A gutter-free fixture
cannot see this, which is why the tool-level tests carry the real render shape.
- _is_secret_file_arg() could not see the RESOLVED Hermes home: the default home's basename
is an installation detail (".hermes" on POSIX, "hermes" under AppData/Local on Windows) and
a resolved path never spells $HERMES_HOME, so the managed Windows home's config.yaml was
classified as ordinary YAML on both the file-read and the terminal surface.
Changes:
- redact_sensitive_text(): secret_file= re-enables the assignment passes for content the
caller classified with _is_secret_file_arg, keeping code_file behaviour everywhere else.
It is authoritative over code_file, so a caller cannot be fail-open on the security flag
by setting both.
- _redact_assignments(): mask_nonreusable selects the non-reusable sentinel for file reads,
so the #35519 write-back hazard stays closed.
- _should_redact_assignment(): no longer re-masks an already-masked value, which was erasing
the vendor label the sentinel deliberately keeps.
- _is_secret_file_arg(): consult the resolved Hermes home for the config.yaml arm.
- _CFG_ANCHORED_RE / _YAML_ASSIGN_RE: tolerate a rendered line-number gutter.
- file_tools.py: classify the resolved path at all three file-read call sites.
Closes#110567
create_task(parents=[open parent]) — the reporter's actual incident path —
parked the card in todo with only a `created` event, and kanban_create's
payload carried no `gated`, so the board still showed an unexplained todo
while only the link surface was fixed. create_task now appends the same
dependency_wait {reason: parent_not_done, parent} event and kanban_create
returns gated/gated_by, mirroring kanban_link.
link_tasks gated on `status != 'done'`, but _parents_satisfied and
recompute_ready treat `archived` as terminal: linking a ready child under an
archived parent demoted it to todo with a false parent_not_done event and the
next recompute promoted it straight back. Gate on not in ('done','archived').
Review finding: create_task(parents=...) emitted no dependency_wait/gated; link under an archived parent flapped ready->todo->ready with a false reason.
The dashboard's POST /links is the fourth writer of link_tasks (CLI, tool,
dashboard, plus the graph builder); return the same ``gated`` flag so every
surface that can create the deadlock can see it. Document the
``dependency_wait`` payload the link path emits and the delegation rule the
reporter derived (never link a support card under the card it unblocks).
Drops the CLI output test (a change-detector on prose); the two DB-level
invariants (event emitted on demotion / none for a done parent) stay.
A ready child linked under an unfinished parent drops to todo with no
event and no operator signal; the only trace used to be claim_rejected
after a forced promote. Record a dependency_wait event when the demotion
fires, return the gate from link_tasks, warn in the CLI link command,
report gated in the kanban_link tool, and document the gate.
The per-root merge in _copy_dist_payload treated the first level under
skills/ as the replace unit, but skills live at skills/<category>/<skill>.
A distribution shipping one skill inside a category rmtree'd the whole
category directory, wiping every skill the user (or hermes update) had
placed there — the exact scenario of issue #25120 that this PR claims to
fix. A shipped directory with no files of its own is now a container:
its children are merged one level down, and the replace unit becomes the
nearest directory that holds a file (a skill dir always holds SKILL.md).
A symlinked category (skills/devops -> shared dir) is a container too,
so the pre-write symlink check now covers it instead of silently
unlinking the link and dropping the shipped copy in its place.
Review finding: categorised skill payload rmtree'd skills/<category>/,
destroying sibling user and bundled skills; symlinked category was
unlinked rather than refused.
The user guide, command reference, `hermes profile update --help` and the
install-over-plain-profile warning all said skills/ and cron/ are overwritten
wholesale; they are now merged per skill / cron job, so say so, and document the
symlinked-container refusal.
Fold the skills/cron/force-install cases into one per-root merge test and keep the
symlink-refusal test; the three separate tests asserted the same mechanism.
The symlink refusal lived in `_real_dir`, which runs per entry inside the copy
loop. By the time it fired for `skills/`, `SOUL.md`/`mcp.json`/`cron` had already
been replaced and `write_manifest` had not run yet, leaving a half-updated
profile that fails identically on every retry. Add a pre-flight pass over
`_owned_entries` that walks each target path (whole chain for directories,
parent chain for files) and raises before any mutation; `_real_dir` keeps its
check as the backstop.
The `.env.template` branch used a bare `shutil.copy2`, which writes through an
existing `.env.EXAMPLE` symlink into whatever it points at. Route it through
`_replace_entry` so the link is unlinked first, like every other owned entry.
The error text now tells the user what they can actually do (remove the link or
replace it with a real directory) instead of pointing at `distribution_owned`,
which is the author's manifest, not the user's.
The existing symlink test now changes every other shipped entry upstream and
asserts the profile is byte-for-byte untouched after the refusal.
A user who points `profiles/x/skills` at a shared directory did so on
purpose. The merge path replaced such a symlink with a real directory
silently, discarding that configuration, where the base rmtree at least
failed loudly. Raise DistributionError naming the link instead; the
file->directory replacement stays because a stray file there is never
deliberate configuration.
`_remove_existing` now removes anything that lexists but is not a real
directory via unlink, so fifos and sockets no longer slip through.
Reviewer P1 on the original PR (symlink container must survive intact).
Keep the tests that pin behaviour a regression would break: user-added and
stale skill roots survive an update while shipped roots are replaced, a
forced reinstall keeps a user skill, and a user-added cron file survives an
update. The allowlist and symlink-transition cases exercise the same code
path (`_real_dir` + `_replace_entry`) and were duplicates of those.
Refs #110933
The skills-only special case (a `preserve_skills` kwarg plus nested closures
keyed on the literal "skills") left cron/ and any future owned directory on
the old rmtree-then-copytree path, which deletes user-added files on update
and on --force reinstall. The rule is the same for all of them: a top-level
owned directory is a container of roots; replace only the roots the payload
ships, never the whole destination.
Apply that rule to every owned directory, drop the kwarg and the string
check, and lift the helpers to module level. `_remove_existing` keeps its
symlink-no-follow semantics and `_real_dir` replaces any symlink or file on
the destination path so the payload is never written outside the profile.
The fresh-install path had nothing to preserve, so it needs no flag.
Restore "skills" in DEFAULT_DIST_OWNED; removing it was never needed for the
fix. Add one invariant test: a user-added cron/ file survives an update.
Refs #110933
_BLOCKED_PROJECT_ENV_BASENAMES is consulted only by get_read_block_error;
project .env / .env.local / .envrc are read-denied but writable on both main
and this branch, so the write-safety table row saying "anywhere on disk" was
wrong. Denying writes would stop the agent from creating .env files for the
user, so the doc is corrected rather than the behaviour changed.
Review finding: security.md write-safety table listed project .env files that are only read-denied.
The salvaged fix added a second directory tuple and a second loop for the same
predicate. _HERMES_PROTECTED_SUBPATHS already expresses "HERMES_HOME subpaths the
file tools must not rewrite", so the two secret directories join it and the
duplicate loop goes. Tests trimmed to two invariants: every read-denied secret
store is also write-denied on both the profile and the global root, and the
#45947 control files plus same-named paths outside HERMES_HOME stay writable.
Narrow the fix to secret material and stop reverting #45947.
The earlier revision added every read-denied name to the write denylist,
which re-blocked auth.json and webhook_subscriptions.json and rewrote the
test guarding them. #45947 freed those control files deliberately:
containment belongs in Docker/remote backends and OS permissions, not an
expanding hardcoded denylist.
What #45947 kept blocked is secret material, and that list had drifted:
auth/google_oauth.json (OAuth token store), the plaintext Bitwarden cache,
vault/ (key + ciphertext side by side) and browser-profile/ (copied
cookies / Login Data) were writable via write_file / patch.
- Write-deny those four; control files stay writable and read-denied.
- _WRITE_DENIED_SECRET_DIRS is its own tuple rather than _READ_DENIED_DIRS,
so a future read-only convenience deny cannot silently become a write deny.
- tests/tools/test_write_deny.py is restored unchanged from main.
- New tests pin both halves: secrets denied, control files writable, and
write denies stay a subset of read denies.
Fixes#110464
teknium1