When the redirect cap trips, the correction that cancelled the final attempt is
still sitting in _pending_redirect; finalize_turn's clear_interrupt() would drop
it silently. Drain it into the steer slot so it rides result["pending_steer"] and
becomes the next user turn on every surface that already honours that key.
Both new exit reasons get a turn-completion explanation so the user sees why the
turn stopped instead of an empty reply.
Collapse the four class-based tests into two parametrized invariants over both
refunding restart flags and move them to tests/agent/ (the phase modules live in
agent/): a single restart still refunds-and-continues; a re-armed restart breaks
after max_retries refunds. The stub grows the redirect seam the follow-up commit
uses so the queued-correction contract is covered by the same test.
The redirect and rebuilt-for-fallback restart paths in apply_retry_restarts
refund the iteration budget and re-issue the iteration with no per-turn
bound. A redirect/interrupt that keeps re-arming the flag refunds forever,
so the turn loop never exits and the durable session turn lease is held
indefinitely (concurrent processes block up to LEASE_WAIT_SECONDS).
Add a per-turn restart_count accumulator (threaded through _run_phase like
the other loop locals) and break out once it exceeds max_retries, matching
the bound the compression path already has.
#106167 widened the base contract to SendResult but left six native-batch
overrides (Discord, Email, Matrix, Mattermost, Slack, Telegram) returning
None, which (a) meant a media-only reply on those platforms still reported
FAILURE because _record_delivery(None) records nothing, and (b) produced new
`ty` invalid-method-override diagnostics against the widened base (#106192).
Make the contract honest instead of annotating it Optional: each override
now rolls its batches (and any per-image fallback) into one SendResult, so
the turn-outcome accounting works on every platform, not just Signal and
the base loop. The "legacy overrides return None" comment in
_send_image_batch goes away with the legacy.
ty on the 8 touched files: origin/main 241 diagnostics / 20 override,
this branch 241 / 20 — byte-identical diagnostic set; the intermediate
`-> SendResult` head without this commit had 246 / 25.
Refs #106192
Thread record_delivery through _deliver_attachments, _deliver_media_attachments
and _send_image_batch so attachment sends feed the turn outcome tracker.
send_multiple_images (base default and Signal override) now returns SendResult
(success when at least one image/batch was accepted); _send_attachment_batch
returns bool. Legacy native-batch overrides that still return None record
nothing, keeping their previous behavior until migrated.
Fixes#106153
kanban_request_review now rejects reviewers that are not installed profiles
(#106163); the cross-surface lifecycle test used a bare "reviewer" name with no
profile behind it, which is exactly the phantom the guard exists to catch.
Use the module's `_check`/`_Reject` idiom instead of an inline
`return tool_error(...)` so every kanban_request_review validation
failure renders through the same path, and drop the unreachable
`or "none"` (list_profile_names() always contains "default").
Tests: compare the task's (status, assignee, run) tuple and the event
log before/after instead of the unordered 6-assert block, use the
context-managed kanban_db_connect.connect (the kb.connect alias is a
plugin-compat pointer — scripts/check_compat_pointers.py flagged it),
and reference #106163 in the invariant's docstring.
Salvage note vs #106214 (@gaoanze888): that PR guards the same condition
inside hermes_cli/kanban_db.py::request_review, but the DB primitive is
also the chokepoint for `hermes kanban request-review` and the
dashboard's drag-to-review, both operator surfaces where a non-profile
assignee (external/human review lane) is a documented board shape
(website/docs/user-guide/features/kanban-worker-lanes.md) — and it forced
five unrelated test fixtures to monkeypatch profile_exists to True. The
model-facing tool wrapper is the layer where a typo'd string is a bug,
so the guard lives there.
kanban_request_review(reviewer=<name>) reassigned the task to whatever
string the model supplied. A non-profile value (e.g. the literal
"reviewer") parked the card in `review` on an assignee the dispatcher
can never spawn, with no error to the worker — the chain stalled
silently (#106163). Validate the explicit reviewer against installed
profiles before touching the board and return a tool error listing the
installed profiles so the model can self-correct.
Salvage of #97429: the kanban_diagnostics `review_reopened` hunk and its
tests were dropped (main already replaced that loop with
`_latest_event_ts`; the tests targeted the PR's pre-refactor base).
Re-authored from the placeholder identity `regen <regen@local>` to the
PR author's GitHub noreply address (misconfigured local git, not malice).
Drop the sentinel-only batch test: a batch sentinel is already rejected by the
shared _is_clarify_non_response_sentinel list check that the existing sentinel
tests pin, so the case adds no new contract. Also add the contributor email
mapping for the cherry-picked commit so release CI can attribute it.
_sum_clarify only extracted the top-level ``user_response`` key, so batch clarify
results (questions=[...] -> responses[].user_response) fell through to the generic
placeholder and the summarizer never saw the user's answer/permission decision.
Closes#106077.
`hermes peer dm` resolves the target's canonical Bot Chat with
GET /api/sessions?title=Bot%20Chat&include_hidden=1. list_sessions_rich
admits the hidden root via the chain search, then _project_compression_tips
overwrites every surfaced field — title included — with the live tip's. The
title is carried root->tip by the agent AFTER publish_compression_child's
transaction; a rotation cut off in between (crash, closed app, the tip's
title write failing) leaves "Bot Chat" on the ended root and NULL on the tip,
so the projected row carries title=None, the handler's exact-title filter
drops it, the peer POSTs a duplicate and the UNIQUE(title) guard answers
400 "Title already in use" (#106165).
Fix at the projection: fall back to the root's title only when the tip has
none (a titled tip keeps winning). Same COALESCE in the bounded recent-
sessions lister, the other place that projects a lineage onto its tip. This
replaces the handler-level fallback in PR #106365 (a second lookup path
bolted onto _handle_list_sessions with try/except: pass) with a 5-line fix
at the one place the title is lost, so every list consumer sees the name.
Salvage of #106365 by @finn763.
Regression test from PR #106365 (one of its two tests kept: the full peer-dm
e2e; the listing-shape test asserted the same row and was dropped). The
handler-level fallback that shipped with the test is replaced by a SessionDB
fix in the follow-up commit, so this commit carries only the test.
Review follow-up for #106276: the compensating UPDATE can succeed and the
session row can still be retired before the verification read-back runs.
That window raised the same RuntimeError as a genuine restore mismatch.
Accept the absent session (warn + return) exactly like the pre-update
deletion window, keep strict verification for a surviving row, and add a
regression test that deletes the session after the UPDATE commits.
[salvage: test hunk dropped from this pick, see previous commit]
The exact-row rollback in restore_compression_failure_cooldown_row raised
RuntimeError when its UPDATE hit rowcount == 0, so a session row retired or
expired mid-attempt (e.g. by the maintenance sweep) crashed the turn
dispatcher during the compression summary failure path (#106271). A missing
session row means the cooldown died with it: nothing is left to restore, so
treat rowcount == 0 as a tolerated no-op (warn + return, skipping read-back
verification), mirroring the existing early-return for snapshots taken while
the session already did not exist. Write and verification failures still
propagate.
[salvage: contributor test file dropped from this pick; regression tests are carried from #106277 and trimmed to two invariants]
`hermes kanban promote --force <id>` printed `Promoted <id> -> ready` and
then the very next claim (a human `claim`, or the dispatcher tick seconds
later) demoted the task back to `todo` with `claim_rejected
{parents_not_done}` and returned None (#106195). The non-force refusal
even pointed operators at `--force` as the escape hatch.
The claim gate is deliberate: `claim_task` is the single enforcement point
("never ready -> running with an undone parent, whichever writer set
'ready'", cda20eec0c), and `complete_task`/`request_review` re-check the
same predicate, so a child let through by a forced claim could still never
finish. A promotion override therefore has no honest outcome; the
dependency edge is the real knob.
- drop `--force` from `promote` (parser, CLI handler, `promote_task`
kwarg, the `forced` event field nothing read)
- the refusal message now states why the gate cannot be bypassed and names
the working remedies: complete the parents or `hermes kanban unlink`
- two invariant tests: refusal on an undone parent leaves `todo` with no
fake `ready`; the flag no longer parses
Salvage direction from #75354 by @vyacheslavk (diagnosis of the promote ->
claim gap); the consume-at-claim authorization there is not taken because
the same parent gate also blocks completion of the forced child.
bridge.js carried its own copy of getMessageContent with the single-layer
envelope list, alongside an unused getContextInfo. With envelope peeling now
living in bridge_helpers.js, the copy would drift (a nested envelope around
a pollUpdateMessage is peeled by the helpers but not by the copy), so import
the shared one instead of keeping a second unwrapping list.
A quoted message can carry its own envelope stack (ephemeral
wrapping viewOnce wrapping the payload), and a reply itself can be
enveloped: both layers now unwrap iteratively (bounded) before quote
extraction. Peeled top-level envelopes return their inner message
immediately, as before — the template/buttons/list branches only
apply to unenveloped messages.
Keep the two tests that fail without the fix (one-shot -> recurring drops the
implicit times=1, recurring -> one-shot gains it) and fold the explicit-repeat
case into the first; drop the same-kind and explicit-finite cases, which only
pin behaviour the fix never touches.
`--in DIR` only chdir'd. Every cwd consumer (resolve_agent_cwd -> Codex
app-server thread cwd, the terminal tool, context-file discovery) prefers
TERMINAL_CWD over the process cwd, so a value inherited from a parent
Hermes surface, the shell or .env survived the chdir and the session kept
running in the old directory. The local backend was rescued by cli.py's
force-export at import time; docker/ssh backends and the TUI launch path,
which never imports cli.py, were not.
Refresh TERMINAL_CWD to the --in target when it is already set. An unset
variable stays unset so the backends keep deriving from the new process
cwd and no host path is pre-seeded into ssh/container backends.
Fixes#106220
Slim redo of the mechanism from #106410 on top of its pick (no wrappers, no
persisted "kind" enum, no process-local flag that dies with the process):
- Futility = the SAME holder PID set has blocked >= _FTS_HOLDER_FUTILE_ATTEMPTS
(10) deferrals over >= _FTS_HOLDER_FUTILE_SECONDS (30 min); tracked as
holders_since/holders_attempts in the persisted fts_rebuild_deferral record
and reset whenever the holder set changes. The 3-deferral/60 s escalate
window is the orphan-reap gate and stays as is.
- ONE escalated ERROR line names each holder pid + cmdline and the remedy that
can actually be followed from inside a gateway session: stop ONLY the other
holder; this process's own retry admits the rebuild within 60 s. The old
"with the gateway stopped" advice was unrunnable from a gateway-hosted
session (the gateway is the session) and is gone from both log and doctor.
- hermes doctor renders the futile record distinctly.
- retry_deferred_fts_recovery: a capped backoff earned by holder set X no
longer applies once the live holder set differs from X, so stopping the
other service is followed by a retry on the next tick, not up to an hour
later (the issue's 16-min wait).
- Tests trimmed from 5 to 2 invariants (futile line + doctor entry after N
same-holder deferrals; backoff reset when the holder set changes); the
contributor's control tests for changing PIDs / orphan reap are covered by
the existing test_repeated_deferrals_reap_inactive_orphan_then_rebuild.
The "canonical writes and LIKE search remain available" WARNING is kept
because it is true on origin/main: a stale open drops every FTS trigger, so
the messages INSERT succeeds (probed live with a real state.db + a second
process holding it). Writes fail only when a peer re-publishes triggers over
the corrupt index — a separate class, not this diagnostic.
Refs #106393
A supervised peer never satisfies the orphan reap, so stale-FTS repair retried forever with a misleading "canonical writes remain available" warning.
(cherry picked from commit e57f3a975d311aa44da1e92c5e727eba7c8cff70)
Tests that exercise the dashboard's gateway-restart path can end up
spawning a REAL `python -m hermes_cli.main gateway restart` child when
the spawn seam is not intercepted. `_spawn_hermes_action` launches it
with start_new_session=True, so it outlives the pytest worker; the
child inherits the pytest-tmp HERMES_HOME, which is not a profile and
hashes to no service suffix, so `get_service_name()` resolves the
DEVELOPER's `hermes-gateway` unit, `systemd_restart` restarts the live
gateway, and without systemd the fallback runs `run_gateway()`
in-process forever and squats the webhook port.
Live repro on this machine (origin/main): an unintercepted spawn of
["gateway", "restart"] from a test restarted the production gateway
(MainPID 136820 -> 1689090, NRestarts=1). On 2026-09-03 a sibling
refactor moved `_spawn_hermes_action` from the `hermes_cli.web_server`
facade to `hermes_cli.web_server_gateway` ~10 minutes before the tests
were repointed; runs in that window patched a name production never
read and left 39 orphans alive for six days.
The live-system guard now rejects any subprocess primitive whose
command line the canonical matcher (`gateway.status.
_gateway_command_subcommand`) classifies as `gateway run|start|restart`.
Argv substrings are never consulted, so `gateway status`, `gateway
--help`, `hermes_cli.main serve`, etc. pass through. Three files that
deliberately spawn and reap a stub child with a gateway-shaped argv
(flock holders, sleep sleepers with an argv tail) opt out with the new
`spawns_gateway_lookalike` marker, which lifts only this check and keeps
os.kill guarded. Two canary tests pin the block and the pass-through.
Defence at the exact boundary the incident crossed: systemd_uninstall() and
uninstall._remove_systemd_gateway() unlinked whatever get_systemd_unit_path()
returned. Before stop/disable/unlink, read the unit's own
Environment="HERMES_HOME=..." line (the parser status/refresh already use)
and, when it names a different home than this process, warn with both paths
and leave the unit alone. A unit without the line (hand-written) is still
removed as before.
With the previous commit a Docker/custom root (HERMES_HOME=/opt/data) gets a
hashed host-service suffix. Three callers used `_profile_suffix() or
"default"` as the PROFILE id, which is a different question: the s6
supervisor's slot for the root home is `gateway-default` regardless of where
the root lives, and the multiplexer's "am I a named profile" probe must not
treat a hash as a profile name. Route them through hermes_constants.
profile_name_for_home() (root -> "default", <root>/profiles/<name> -> name)
with the service suffix as the fallback for unknown layouts.
_profile_suffix() compared HERMES_HOME against get_default_hermes_root(),
which treats ANY home outside ~/.hermes (Docker /opt/data, a mktemp dir) as
"the root itself". Every such home therefore collapsed to the bare
`hermes-gateway` service name and the default profile's unit path
(~/.config/systemd/user/hermes-gateway.service); the documented
"else a short hash of the path" branch was unreachable.
A parity harness run with HERMES_HOME=$(mktemp -d) called
uninstall_gateway_service(), resolved to the production unit, ran
`systemctl --user stop/disable`, unlinked it and daemon-reloaded. With the
unit gone Restart= could not revive it: all cron jobs and every messaging
platform were down for 6.5 days.
Compare against the platform-native default home (~/.hermes) for the bare
name; keep the profile name for <root>/profiles/<name>; everything else
(temp dirs, Docker /opt/data) gets its sha256[:8] suffix as the docstring
always promised. The Docker image supervises with s6 (`gateway-<profile>`
slots), not systemd/launchd, so the bare host-service name was never load-
bearing there.
_size duplicated _mtime_ns with a different attribute and the fingerprint
stat'ed each of the four files twice, so mtime and size could come from two
different versions of the file. _stat_sig returns both from one stat. The
persisted compare goes through a JSON round-trip so nested tuples match the
lists they read back as.
A fresh process with an unchanged store takes zero auth-store locks, and a
root store that gains a forked grant invalidates the persisted mark so the
heal re-runs. The corrupt-mark fallback, same-mtime size change and
no-secrets checks were pinning implementation details of the same cache.
`_heal_forked_single_use_oauth_grants()` runs on every `load_pool()`, and its
clean mark lived only in memory. Every fresh `hermes` invocation and every new
worker therefore re-took the heal's two nested EXCLUSIVE auth-store locks just
to rediscover a store it had already cleared — 4 acquisitions per process on a
two-provider profile, every one of them finding nothing to consolidate. Behind
a sibling process holding those locks that costs a full
`AUTH_LOCK_TIMEOUT_SECONDS` per provider before the process can do anything at
all: measured 30.1s for two providers.
Persist the mark next to the store it describes (`<profile>/cache/
oauth_heal_clean.json`, 0600, paths and stat data only — no credential
material) and consult it BEFORE taking any lock.
Outliving the process means the mark needs a stronger key than the in-memory
one did:
- The ROOT store joins the fingerprint. This heal consolidates root → profile,
so root acquiring a counterpart turns a row the heal deliberately KEPT into a
fork it must strip. The in-memory mark could ignore root because it died with
the process; a persisted mark would keep skipping a heal that has become
necessary.
- File sizes join it too, so a metadata-preserving rewrite (`rsync -t`,
`tar -p`, a restore) cannot leave a stale mark looking current indefinitely
rather than for one process.
Measured on an isolated HERMES_HOME with two OAuth providers:
lock acquisitions per fresh process 4 -> 0
contended load_pool() x2 30.1s -> 0.00s
mark-file reads per 100 load_pool() - -> 2 (one per provider)
The mark stays a cache: absent, unreadable, corrupt or wrong-shaped content all
mean "unknown" and fall through to the locked heal, and a failed write only
means the next process re-runs it — the behaviour before this cache existed.
An HTTPError means the host answered — a 401 from a wrong API key must not
be remembered as "unreachable" for the next 60s, or a user who fixes the key
gets a cached empty catalog on the immediate re-probe. Connection-level
failures (timeouts, refused, DNS) are the only thing the cache records.
_probe_neg_key hand-rolled scheme/port defaulting that utils.base_url_origin
already provides; use it.
The contributor's 15-test module pinned encoding details (frozen-arg shapes,
postponed-annotation resolution, monkeypatch seams). The two behaviour
contracts that matter survive: a 12-request /api/profiles burst admits ONE
worker and leaves /api/status responsive on a 2-token pool (this file), and
the kanban board read shares one worker per key (test_kanban_read_admission).
Both go red when the coalescing wrapper is removed from the route.
refresh_interval_seconds() honours model_catalog.ttl_minutes / legacy ttl_hours;
reading DEFAULT_TTL_MINUTES would let the snapshot and the manifest it is
filtered from go stale on different clocks for anyone who changed the TTL.
Re-derived from PR #96099 (f127ec4e) on current main; the :nitro/:floor validate hunk is omitted because main already handles routing suffixes in hermes_cli/models_validate.py.
_nous_picker_model_ids only uses the ids the Portal unions append — both
unions discard the pricing map (`model_ids, _ = union_with_portal_*`) — yet
it called get_pricing_for_provider("nous") without cached_only, so a cold
pricing cache paid a full /v1/models round-trip (network timeout on a slow
Portal) on the picker-open path for nothing. Pass cached_only=True; the
background pricing prewarm (#101685) fills the same cache for later opens.
Re-derived from #102099 by @finn763: the original patched
hermes_cli/model_switch.py, which 3b1ecfc0a1 decomposed; the live call site is
hermes_cli/model_switch_providers.py.
Based on #102099 by @finn763.
Teknium