Commit graph

9022 commits

Author SHA1 Message Date
Josh Tsai
df03120cb6 fix(gateway): recheck stop immediately before watchdog hard exit
- A stop() landing while the final diagnostics (critical log,
  traceback dump) are executing could still reach os._exit(75) after
  the pre-diagnostic check. Add a third stop_event recheck immediately
  before the hard exit: diagnostics may complete, but a disarmed
  watchdog never exits.
- Deterministic regressions for both windows (stop triggered from
  inside logger.critical and from inside faulthandler.dump_traceback);
  mutation-verified (removing the check turns both red). Frozen-loop
  semantics unchanged.

Addresses the second round of the shutdown-race review on #69164.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-24 16:03:42 -07:00
Josh Tsai
aff415b653 fix(gateway): close watchdog shutdown race against final-strike exit
- Re-check stop_event after a missed probe (before the strike
  increment) and again on entering the final-strike branch (before the
  critical log, dump, and hard exit), so a normal stop() landing
  between the last timeout check and the exit path can no longer be
  misclassified as a freeze and trigger a supervisor restart.
- Deterministic boundary tests pin both re-checks independently
  (mutation-verified: removing either check turns its own test red);
  frozen-loop semantics are unchanged.

Addresses the shutdown-race review on #69164.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-24 16:03:42 -07:00
Josh Tsai
7af3a6fc7c fix(gateway): detect and escape silent event-loop freezes
- A self-rescheduling 5s call_later floor timer, armed before any
  adapter connects, guarantees the selector always has a finite
  timeout, so the existing async defenses (polling heartbeat, timeout
  guards) regain a chance to run after a zero-pending-timer stall.
- A resident daemon-thread liveness watchdog probes the loop via
  call_soon_threadsafe every 30s; after 3 consecutive 10s-timeout
  misses (~120s of total unresponsiveness) it dumps all thread
  tracebacks and exits with the established
  GATEWAY_SERVICE_RESTART_EXIT_CODE (75) so a supervisor restarts the
  gateway - async-level recovery cannot run on a frozen loop.
- stop() disarms both guards before any teardown await so a busy
  shutdown is never misjudged as a freeze.
  HERMES_GATEWAY_LOOP_WATCHDOG=0 disables; _INTERVAL/_TIMEOUT/_STRIKES
  tune the thresholds.

Fixes #69089

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-24 16:03:42 -07:00
Shannon Sands
eb2f5b5723 fix(gateway): stay alive on mixed retryable + non-retryable startup failures
When connected_count == 0 and at least one platform failed with a
non-retryable error, the runner exited with GATEWAY_FATAL_CONFIG_EXIT_CODE
(78) even if OTHER platforms failed for merely transient reasons.

Real-world shape (NS-609, hosted instance): WhatsApp enabled but never
paired (non-retryable whatsapp_not_paired) + Telegram TimedOut during
polling startup (retryable) => exit 78 => the gateway either goes
permanently down (supervisors honoring the exit-78 contract via
RestartPreventExitStatus / the s6 finish->125 translation from #51228) or
crash-loops (anything else). Either way Telegram never gets its retry and
the dashboard drops with every exit, so a single unpaired platform plus
one network blip disconnected every channel on the instance.

Now exit 78 is reserved for the case where ALL startup failures are
non-retryable (true config error, nothing to wait for). With mixed
failures the gateway stays alive in degraded state: the reconnect watcher
recovers the retryable platforms and the misconfigured ones stay
fatal-parked and visible in runtime status.
2026-07-24 16:03:10 -07:00
webtecnica
83fe362e80 fix(gateway): prevent reconnect watcher wedge after network-loss fatal error (#70344)
Three-part fix for the gateway going silently deaf after a retryable
fatal adapter error (e.g. httpx.ConnectError on Telegram):

1. **Detach-on-timeout in _connect_adapter_with_timeout** — Replaced
   plain asyncio.wait_for with the task-detach pattern used by
   _await_adapter_cleanup_with_timeout. asyncio.wait_for cancels the
   overdue task but then waits for it to exit, so a connect() that
   catches CancelledError can block recovery forever. The detach
   pattern releases the runner at the deadline via
   consume_detached_task_result.

2. **Ensure reconnect watcher always runs after escalation** — Added
   _ensure_reconnect_watcher_running(), called after queueing a
   retryable fatal error. If the reconnect watcher task has died
   (exhausted restart budget, terminal exception), it is respawned
   so queued platforms are never permanently stranded.

3. **Faulthandler at gateway startup** — Enabled faulthandler +
   SIGUSR2 dump to a rotating file under HERMES_HOME/logs/ for
   post-mortem diagnosis of future event-loop freezes.

Tests added for _ensure_reconnect_watcher_running (alive, dead,
not-started, not-running), fatal-error integration (retryable calls
ensure, non-retryable does not), and _connect_adapter_with_timeout
(timeout raises, success returns).
2026-07-24 16:03:10 -07:00
teknium1
9c65cdb043 test: accept the new profile-scoped kwargs in status fakes
/api/status?profile= now passes pid_path=/path=/expected_home= to the
PID and runtime-status readers; the profile-unification fakes had
zero-arg signatures and raised TypeError. Plain /api/status call shapes
are unchanged (pinned by the existing zero-arg tests in
test_web_server.py).
2026-07-24 16:02:39 -07:00
teknium1
ddb86725b5 test(web): pin per-profile gateway state scoping on /api/status
Follow-up for the salvaged #70498 fix: replace the original PR's
mock-signature churn (28 lambda **kw edits, needed only because it changed
the no-profile call shape) with two targeted regression tests:

- ?profile=<name> must pass the profile's gateway.pid / gateway_state.json
  paths and expected_home to the gateway status readers (HOME-anchored
  per-profile state under ~/.hermes/profiles/<name>/)
- ?profile=<unknown> must 404 via _resolve_profile_dir

The production change keeps plain /api/status on the exact zero-arg calls,
so every pre-existing test passes unmodified.
2026-07-24 16:02:39 -07:00
Eugeniusz Gilewski
6fba781945 fix(config): preserve opaque .env values
The .env sanitizer inferred missing newlines from known KEY= substrings
inside existing values. Plain secrets containing those bytes could therefore
be split into synthetic assignments and rewritten to disk.

Treat each physical line as the only assignment boundary and keep bytes after
the first equals sign opaque for boundary discovery. Preserve safe formatting,
null-byte removal, BOM handling, and normal one-assignment-per-line parsing.

Cover direct loading, dotenv loading, sanitization, writers, and migration
with behavioral regressions.

Fixes #29155
2026-07-24 16:02:08 -07:00
teknium1
18af81bb5b fix(caching): reconstruct static system prefix on session restore and post-compression reuse
Follow-up to the cherry-picked #68258 base: the cross-session-stable
prefix (_cached_system_prompt_static) was only recorded on fresh
builds, so two paths silently degraded to the legacy single-breakpoint
layout (flagged in review of #68258/#69341/#69704):

- Session restore: gateway surfaces build a fresh AIAgent per turn and
  restore the persisted prompt verbatim from the session DB; the static
  prefix stayed None from turn 2 onward, flip-flopping the wire layout.
- Post-compression cached-prompt reuse: _invalidate_system_prompt()
  clears the static prefix, and the keep-cached-prompt branch never
  restored it.

Both sites now reconstruct the stable tier and adopt it ONLY when the
authoritative prompt string literally startswith() it — stable-tier
drift (skills edited, identity changed) falls back to the legacy layout
with the stored bytes untouched. Fail-open on any builder error. The
restore-path rebuild is gated on _use_prompt_caching so non-Anthropic
routes skip it entirely.

Refs #68191

Co-authored-by: JonthanaHanh <92574114+JonthanaHanh@users.noreply.github.com>
Co-authored-by: joaomarcos <joaomarcosdias444@gmail.com>
Co-authored-by: webtecnica <webtecnica@users.noreply.github.com>
2026-07-24 16:01:38 -07:00
Drexuxux
fb1b89b09e fix(prompt-caching): inject cache breakpoints after message normalization
The conversation loop normalizes message text right before the API call so
the request prefix is byte-identical across turns -- the stated reason is
KV cache reuse on local inference servers and better cache hit rates on
cloud providers. Cache breakpoints were injected *before* that pass, which
defeats it.

`_apply_cache_marker` rewrites a plain-string `content` into a
`[{"type": "text", ...}]` block. The normalization pass is guarded on
`isinstance(content, str)`, so every message that just got marked is
silently skipped by it and keeps its raw leading/trailing whitespace. A
message is only marked while it sits in the last-3 window, so:

    turn N      in the window  -> marked, content "file1\nfile2\n"
    turn N+1    rolled out     -> plain,  content "file1\nfile2"

The same logical message is sent with different bytes on consecutive
turns. The prefix stops matching at that position -- which is inside the
span the breakpoints were placed to protect -- so the reusable prefix
collapses back toward the system breakpoint on every turn. Tool results
carry a trailing newline almost by default (any shell command output), so
this is the common case, not an edge case.

Move the injection below every message mutation. Besides fixing the
whitespace divergence this stops breakpoints from being spent on messages
that the orphan sweep or the thinking-only drop is about to remove or
merge away -- a marker on a dropped message is a wasted breakpoint out of
the four available.

Nothing between the old and new call sites reads `cache_control`, and the
mutators now see the plain-string shapes they were written against.
2026-07-24 16:01:38 -07:00
konsisumer
9fdadf0cd7 fix(agent): cache static system prompt prefixes 2026-07-24 16:01:38 -07:00
teknium1
6c14b12d1f fix(checkpoints): bind an empty orphan preview to an empty deletion allowlist
Follow-up for salvaged PR #69141, addressing the last open review point:
cmd_prune() only set orphan_allowlist inside 'if orphans or pre_v2_orphans',
so a zero-orphan preview passed the unrestricted None sentinel down to
prune_checkpoints(), authorizing deletion of any project that became
orphaned between the preview and the rescan — with zero confirmation
calls. The allowlist is now bound unconditionally for every non-force
run (empty preview => empty allowlist); --force keeps None. Adds the
zero-orphan-preview timing regression plus allowlist-identity tests.
2026-07-24 16:01:06 -07:00
joaomarcos
a373fab1ce fix(checkpoints): bind orphan confirmation to previewed identities
Address P1 from PR review: cmd_prune()'s y/N preview reads
store_status() but the confirmed deletion re-scans both the v2 and
pre-v2 layouts from scratch. A workdir that goes missing while the
human is answering the prompt gets swept in as if it had been shown
and approved.

prune_checkpoints() now accepts orphan_allowlist — a set of v2 project
hashes and/or pre-v2 shadow repo paths. When set, only orphans whose
identity is in the set are deleted; anything newly orphaned since the
scan survives the run. cmd_prune() builds this set from the exact
projects it just displayed and passed confirmation for. --force still
passes None (no preview shown, so nothing to bind to).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-07-24 16:01:06 -07:00
joaomarcos
3ad8552f78 test(checkpoints): cover prune decline/accept/--force for pre-v2-only and mixed stores
Requested by egilewski on #69141: the orphan confirmation flow had no
test coverage at all before this. Exercises hermes_cli.checkpoints.cmd_prune
directly against pre-v2-only and mixed (v2 + pre-v2) fake stores —
decline aborts with nothing deleted, accept deletes both layouts,
--force and --keep-orphans skip the prompt as expected.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-07-24 16:01:06 -07:00
teknium1
3960315af7 test: order compression-tip fixtures around the closed-parent write guard
Two compression-tip hydration tests simulated legacy state by emptying
the parent AFTER end_session(compression) — exactly the durable write
the new closed-parent guard refuses. Reordered: empty first, close
second. The tests' actual contract (old id hydrates from the live tip)
is unchanged and still pinned.
2026-07-24 16:00:34 -07:00
Anthony Ruiz
0ee8d41878 fix(compression): recover rotated session lineage 2026-07-24 16:00:34 -07:00
teknium1
62ee34570e test: accept kwargs in managed_uv fixture fakes
The runtime-repair change passes repair_observer= to update_managed_uv/
ensure_uv; the autouse fixture fakes had zero-arg signatures and raised
TypeError through the mock. Sibling test file to the PR's own suite.
2026-07-24 16:00:03 -07:00
teknium1
be633c1c33 fix(runtime): request minor line for SQLite runtime repair + tests
Follow-up on the #70186 salvage. The cherry-picked repair pinned the
candidate to the exact current CPython patch (e.g. 3.11.14). Verified
live with uv 0.11.19: every published python-build-standalone artifact
for 3.11.14 links vulnerable SQLite 3.50.4 — even with --reinstall — so
the exact-patch pin made the repair permanently impossible on the
installs that need it most (repair_vulnerable_runtime returned
'failed: could not provision a fixed private Python runtime').

Request the minor line (3.11) instead — the same resolution a fresh
'uv python install' would make, still inside requires-python — and
tighten the drift gate to 'same minor, no downgrade'. E2E-verified
end-to-end on a real vulnerable venv: repair_vulnerable_runtime()
provisioned 3.11.15, built + smoke-tested the sibling venv, cut over,
and reported SQLite 3.50.4 → 3.53.1 with the old venv parked for
rollback.
2026-07-24 16:00:03 -07:00
Justin Bennington
05a799e41c fix(runtime): preserve cutover lifecycle on retry (E-949) 2026-07-24 16:00:03 -07:00
Justin Bennington
bbee3011fd test(runtime): cover managed SQLite cutover (E-949) 2026-07-24 16:00:03 -07:00
teknium1
a5f9ea2741 fix(update): correct integrity-guard bugs from #70553 salvage + tests
Follow-up fixes on top of the cherry-picked guard:
- verify_sqlite_integrity(): an oversized (max_bytes-exceeding) database
  now still fails on a zeroed/invalid header — previously valid=True was
  set before the header check, so a >1GiB zeroed state.db (the exact
  #68474 signature at 95MB scale) passed as valid.
- _run_pre_update_backup(): the guard referenced get_hermes_home before a
  later function-local 'from hermes_constants import get_hermes_home'
  shadowed it → UnboundLocalError swallowed by the snapshot try/except,
  silently disabling the post-snapshot check AND the snapshot-id output.
  Alias the import explicitly.
- Import _quick_snapshot_root where used (was NameError in all 3 guards).
- tests/hermes_cli/test_state_db_guard.py: real-SQLite E2E coverage —
  valid/zeroed/truncated files, oversized header gate, copy+verify
  roundtrip, snapshot restore flow, and the live _run_pre_update_backup
  path against a temp HERMES_HOME with mid-flight zeroing.
2026-07-24 15:59:32 -07:00
teknium1
9e4492fd74 fix(process_registry): reader loop no longer hangs when an orphaned grandchild holds the stdout pipe
When a background terminal() command backgrounds its own long-lived
child (`node server.js &`, `sleep 300 &`), the grandchild inherits the
write end of the reader thread's stdout pipe. The direct bash child
exits promptly, but the pipe never reaches EOF while the grandchild
lives — so `_reader_loop`'s blocking `read1()` parked the thread
forever, `session.exited` never flipped on its own, and
`notify_on_complete` was silently lost. `_reconcile_local_exit`
(#17327) only runs lazily from poll()/wait(), so nothing autonomous
ever surfaced the exit; each occurrence also leaked a reader thread
and pipe fd for the grandchild's lifetime.

Fix: on POSIX, drain via select() with a short poll interval and stop
shortly after the direct child exits even if the pipe hasn't EOF'd —
the same pattern the foreground path uses in
tools/environments/base.py::_wait_for_process (#8340). Windows pipes
don't support select(), so the blocking path is kept there with the
existing lazy reconcile as the safety net; mocked/iterator stdout
streams (no usable fileno) also keep the historical path.

Fixes #68915
2026-07-24 15:59:02 -07:00
teknium1
3b762ba695 fix: serialize on-demand slash-worker spawn per session
With the eager pre-warm removed (PR #66783), slash.exec is the only spawn
path — and it runs on the RPC thread pool, so two concurrent worker-routed
commands on a fresh session could both see slash_worker=None and each fork
a full stdio-MCP-fleet worker (the _attach_worker race loser leaking
unclosed). Add a per-session spawn lock with a double-check, plus a
regression test racing two slash.exec calls through handle_request.

Also maps Ne0teric's contributor email.
2026-07-24 15:58:32 -07:00
Ne0teric
5aa3536b32 fix(tui): spawn slash workers on demand instead of one per session
Every slash_worker child runs its own MCP discovery (#61891), which
forks the full configured stdio MCP fleet — on a config with a handful
of stdio servers that is ~20 OS processes per worker once npx/cmd
wrappers are counted. The gateway pre-warmed a worker for every session
at create/build time, and sessions held by a live transport are (by
design) never reaped, so a desktop app left open for days accumulates
one fleet per retained session. On a real setup this reached ~120
processes across 6 sessions and pushed Windows commit charge to the
point where CreateProcess started failing system-wide ("Not enough
memory resources are available to process this command").

slash.exec already spawns a worker on demand when the session has none
and already recovers from a dead worker the same way, so the eager
pre-warm is pure pre-warming:

- drop the pre-warm in the deferred session-build path
- drop the pre-warm in _init_session
- make _restart_slash_worker a no-op for sessions that never spawned a
  worker (the next slash.exec builds one with the current session
  key/model, so no stale-key worker can exist)

Only sessions that actually run a worker-routed slash command now pay
for a fleet. Cost: the first such command in a session takes the CLI
build + MCP discovery hit that session.create used to absorb.

Tests: the two create/close-race guards now assert the build thread
never constructs a worker (the notify-unregister guarantees are kept);
the restart-orphan guard seeds a live worker so the close path is still
exercised; new test pins the restart no-op for workerless sessions.
2026-07-24 15:58:32 -07:00
teknium1
410877c7e1 fix(memory): close second-read drift race and treat invalid UTF-8 as unreadable
Follow-up hardening on top of the salvaged #69745 guard, addressing both
review findings:

- Drift detection no longer re-reads the file. _reload_target performs ONE
  checked read and derives both the drift check and the entry parse from that
  same raw snapshot (_detect_external_drift now takes the raw text). The old
  second read swallowed OSError as 'no drift', so a read failure between the
  two reads let replace/remove/apply_batch rewrite the file from a stale view,
  discarding externally added entries.
- Invalid UTF-8 now counts as unreadable: the checked read catches
  UnicodeDecodeError and mutations return the preservation refusal instead of
  raising (or worse, rewriting bytes we can't round-trip).
- USER.md is covered by the same guard (shared _reload_target path) and now
  pinned by an explicit test.

Tests: read-once structural invariant, invalid-UTF-8 refusal with
byte-identical file, user-store refusal.
2026-07-24 15:58:01 -07:00
Frowtek
0c4c8f95e1 fix(memory): don't wipe MEMORY.md when a read-modify-write reads it as unreadable
`_read_file` degraded any read failure to `[]`, conflating "file exists but
couldn't be read" with "empty store". That is a silent, total data-loss bug on
the `add` path.

`add` re-reads the file under lock, appends the new entry, and rewrites the
WHOLE file from the parsed entries. It deliberately skips the drift guard
(#42874: "appending never clobbers existing content") — but that reasoning only
holds when the reload actually saw the file. When `read_text` raises
(an external editor momentarily holding the file on Windows, a permission
change, a filesystem/EINTR blip), `_read_file` returns `[]`, so `add` treats
the store as empty and rewrites the file down to just the new entry — every
prior memory gone — while returning `success: True`.

Reproduced with a transient read failure during `add`:

    entries on disk before : 3   (dark-mode pref, deadline, deploy target)
    add("A brand new fact") : success=True
    entries on disk after  : 1   ("A brand new fact")   <-- the other 2 wiped

replace/remove/apply_batch were shielded only incidentally — an empty view
means `old_text` never matches, so they abort before writing — but they still
returned a misleading "no entry matched" instead of naming the real problem.

Fix: distinguish unreadable from empty. `_read_entries_checked` returns
`(entries, read_ok)`, with `read_ok=False` only when the file exists but can't
be read; absent/empty stays a clean `([], True)`. `_reload_target` returns a
`_READ_FAILED` sentinel in that case without touching in-memory state, and all
four mutation paths (add, replace, remove, apply_batch) refuse the write with a
clear "retry in a moment" error. This is the same posture as the drift guard
and the pairing/checkpoint fixes: never rewrite a file from a view that isn't
the real one. `_read_file` keeps its `[]`-on-error contract for the read-only
`load_from_disk` caller, which never persists.

tests/tools/test_memory_tool.py: new TestUnreadableFileDoesNotWipeMemory —
add/replace/remove/apply_batch all refuse and leave the file byte-identical on
a transient read failure, plus controls that an absent file is still a clean
empty store and the happy path is undisturbed. The four refusal tests fail on
main. Full suite: 90 passed, 1 pre-existing failure (`test_deduplication_on_load`,
a UnicodeDecodeError unrelated to this change, identical on clean main).
2026-07-24 15:58:01 -07:00
Israel Lot
4be38125af feat(acp): list named custom providers in the ACP model selector
Named endpoints from the providers: mapping (and legacy custom_providers:
list) never appear in the ACP model selector: _build_model_state lists
only the canonical current provider's catalog, and canonical provider
enumeration does not include user-defined named endpoints. The TUI
/model picker already renders these entries (#47039, implemented for the
TUI surface only), so editor clients silently hide endpoints the user
configured — e.g. an OpenAI-compatible Bedrock Mantle Responses provider.

Add _named_custom_provider_catalogs(), sourcing entries from
get_compatible_custom_providers() (covers both config shapes), and append
its models to the selector payload. Choice ids use the custom:<name>
slug shape so custom:<name>:<model> selections round-trip through
parse_model_input / resolve_runtime_provider unchanged on set_session_model.

Declared models (default_model + models) survive failed live /models
discovery — some OpenAI-compatible endpoints expose no /models route yet
serve their declared models fine. Honors providers.<name>.enabled: false
and discover_models: false.

Verified: scripts/run_tests.sh tests/acp/ — 318 passed, 0 failed;
scripts/check-windows-footguns.py clean.
2026-07-24 15:57:30 -07:00
teknium1
62bec4b3f8 fix(compression): add recovery path to anti-thrash auto-compaction block
When two consecutive compactions each failed to clear the threshold, the
anti-thrashing breaker blocked automatic compaction PERMANENTLY for the
life of the session: nothing decremented _ineffective_compression_count
(or _fallback_compression_streak) while blocked, so a session whose
middle region was briefly too small to compact never auto-compacted
again — it grew unbounded until the provider's hard context limit, and
only /new or /reset recovered it.

Recovery is a probation probe, not amnesty: after
_ANTI_THRASH_RECOVERY_SECONDS (300s) of continuous block the gate grants
exactly ONE attempt by dropping tripped counters to 1 strike (persisted,
so sibling agents on the same session row — gateway hygiene — unblock
too). An ineffective probe re-trips the guard on the next real-usage
verdict and the next recovery waits a full fresh window, so the worst
case in a truly incompressible session is one compaction attempt per
window — bounded, not thrash.

The recovery clock is armed lazily on the first BLOCKED evaluation and
is deliberately not durable: a restart that loads a durable tripped
counter (#69872) starts a full fresh window blocked, preserving the
restart-must-never-disarm contract (#54923).

Fixes #14694
2026-07-24 15:57:09 -07:00
teknium1
5a0f51325c fix(agent): make dropped tool-call nudge pair ephemeral scaffolding
Review follow-up for the dropped tool-call recovery (#69630): the
re-prompt pair was tagged _dropped_toolcall_nudge, but that marker was
not part of the ephemeral-scaffolding contract. _persist_session /
_flush_messages_to_session_db would therefore write the synthetic
'issue the actual tool call now' user message (and the narration-only
interim assistant turn) as real transcript rows — a resumed session
could replay the internal retry instruction as user-authored context
and prompt unsolicited tool use.

- Add _dropped_toolcall_nudge to _EPHEMERAL_SCAFFOLDING_FLAGS
  (run_agent.py) so both SQLite and JSON persistence skip the pair.
- Add it to _SYNTHETIC_USER_FLAGS (conversation_compression.py) so the
  compressor never treats the nudge as human intent.
- Flag the interim assistant half of the pair too, and include the
  marker in the finalization scaffolding pop so a genuine turn end
  strips the pair from the live transcript (mirrors the
  _empty_recovery_synthetic pattern).
- Regression tests: flagged messages classify as ephemeral, the
  returned transcript contains no scaffolding, and the turn tail stays
  on the real assistant answer.
2026-07-24 15:56:39 -07:00
Jash Lee
923704c7c2 fix(agent): move dropped tool-call recovery to the finalization chokepoint
The initial fix guarded the no-tool-calls else branch, but that branch only
SETS final_response — the turn actually finalizes later, in a separate block
after final_msg is built. Runs that reached finalization via that path exited
without the guard ever running (observed live: a scheduled PR reviewer stalled
at tool_turns=1-2 with zero recovery nudges).

Move the recovery to the finalization chokepoint (right after final_msg is
built), so it catches every path that ends a turn. Single guard now:
- increments a consecutive-stall counter and re-prompts (bounded to 3),
- resets on any successful tool round, and
- resets on a genuine (non-mismatch) turn end,
so it guards each stall independently without capping the whole run and
without looping forever.

Verified live: the scheduled reviewer now recovers through the stalls and
submits real reviews — PR 57800 APPROVED, PR 54826 COMMENTED — one PR per run.
2026-07-24 15:56:39 -07:00
Jash Lee
63954d508c fix(agent): recover from dropped tool calls (finish_reason=tool_calls, empty array)
Some providers (observed: claude-opus-4.8 / claude-sonnet-4.5 on GitHub
Copilot, ~2026-07) return finish_reason="tool_calls" while the parsed
tool_calls array is empty — the model signalled it wanted to act but the
payload shipped no call. The conversation loop took the no-tool-calls
else branch, treated the turn's narration as the final answer, and exited
with the task unstarted.

On unattended multi-step jobs this is silent failure: a scheduled PR
reviewer, for example, would narrate "Let me verify the PR..." and stop
at tool_turns=0 every run, never submitting a review, while the job still
reported success.

Fix: in the no-tool-calls branch, detect the provider contract violation
(finish_reason == "tool_calls" with zero tool_calls) and re-prompt the
model to emit the call instead of exiting. Bounded to 3 consecutive
stalls; the budget resets after any successful tool round so it guards
each stall independently rather than capping the whole run. The narration
may live in content or only in the reasoning field (empty content) — the
guard keys on the finish_reason/tool_calls mismatch, so both are covered.
A genuine finish_reason="stop" text turn is unaffected.

Verified live: a scheduled reviewer that died at tool_turns=0 every run
now recovers through 20+ stalls per run and submits real reviews.

Tests: tests/run_agent/test_dropped_tool_call_recovery.py covers the
re-prompt, the empty-content case, the clean-stop control, and the
bounded-loop guard.
2026-07-24 15:56:39 -07:00
kshitijk4poor
4582376cf3 fix: update heartbeat test stubs to accept workdir kwarg
Follow-up to salvaged PR #70548 — _run_job_script now accepts
workdir= kwarg, test mocks need to accept it too.
2026-07-24 15:55:39 -07:00
kshitijk4poor
0aa15c08dd fix: update test stub to accept workdir kwarg from salvaged PR #70548 2026-07-24 15:55:39 -07:00
joaomarcos
1d721a66f7 fix(cron): close sqlite connections deterministically in execution ledger 2026-07-24 15:55:08 -07:00
Dhruv Raajeev
d10d3d7b42 fix(gateway,tools,agent): close leaked SQLite connections in delivery, delegation, and verification ledgers
Three durable ledgers used `with _connect() as conn:` where the sqlite3
connection context manager commits/rolls back but never closes, leaking the
db/-wal/-shm file descriptors on every call. On a long-running gateway this
exhausts RLIMIT_NOFILE and fails unrelated components with
`[Errno 24] Too many open files`. Same bug class as the cron execution ledger
(#69567 / PR #69594), which the connection helpers here are modeled on.

Fix: route every ledger operation through a `_transaction()` context manager
that guarantees `conn.close()` on exit. `_connect()` keeps its
schema-on-connect contract (several tests call it directly) and now self-closes
if schema init fails.

Adds per-module regression tests asserting every opened connection is closed,
including the no-op-update and exception-mid-transaction paths.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-24 15:55:08 -07:00
teknium1
dc6cb5c500 fix(anthropic): keep replay content schema-valid when every block is blank
Follow-up to the cherry-picked #68633 commits, closing the final open
review point (egilewski): _relocated_replay_cache_control was applied
only inside `if replayed:`. When anthropic_content_blocks contained
only a blank cache-marked text block, `replayed` came out empty, the
function fell through to the main path's placeholder, and the cache
marker was lost; signed thinking + a blank marked text block likewise
returned with no cacheable carrier for the relocated marker.

The replay branch now appends the non-whitespace "(empty)" placeholder
when no cacheable (text/tool_use) block survives the blank filter and a
blank text block was dropped (or a marker needs a carrier) — so replay
stays schema-valid on Bedrock/strict endpoints and the breakpoint
survives on the placeholder.

Also reconciles the block-level tests from #69517 with the new
drop-then-fallback contract (blank blocks are dropped at the block
level; the message-level result is still always non-blank).

Refs #69512

Co-authored-by: ygd58 <buraysandro9@gmail.com>
2026-07-24 15:54:38 -07:00
ygd58
29f9cfeb4a fix(anthropic): don't fall back to raw content when all blocks filtered
Follow-up per independent review of #68633 (GPT-5.6-sol-xhigh in Codex,
reviewer egilewski) on this PR.

Two real bugs in the blank-text-block filtering added by that fix:

1. `effective = blocks or content` fell back to the RAW, unfiltered
   `content` variable whenever every block was filtered out as blank --
   which happens precisely when the entire message content WAS the
   blank/whitespace payload the filter exists to remove (a sole blank
   text block, a sole cache-marked blank block, or standalone
   whitespace scalar content with no tool_calls). The fallback silently
   restored the exact invalid content the filtering just stripped,
   leaving the message provider-invalid.

   Fixed: `effective = blocks if blocks else [{"type": "text", "text":
   "(empty)"}]` -- never falls back to raw `content`. Also moved the
   cache_control application (both the relocated-from-a-dropped-block
   marker and the message-level marker) to run against `effective`
   instead of the pre-fallback `blocks`, so a cache marker on a block
   that was the ONLY content still lands on the (empty) placeholder
   rather than being silently lost when `blocks` was empty at the
   point it would otherwise have been applied.

2. The normal-path blank-text check used `(blk.get("text") or "").strip()`,
   which is not type-safe for a truthy NON-string, non-None text value
   (e.g. an int or dict from an invalid upstream payload) -- `or`
   doesn't substitute for a truthy value, so `(7 or "").strip()` still
   raises AttributeError. Now checks `isinstance(text, str)` first,
   matching the replay path's `_sanitize_replay_block()`, which the
   reviewer confirmed was already correctly type-safe.

Added regression tests for: sole blank list block, sole whitespace
scalar content, sole cache-marked blank block (marker relocation to
the placeholder), a truthy non-string (int) text value both mixed with
a surviving tool_use and as the sole content, and a dict-valued text
field. 7/7 new tests pass; 193/193 in the full
tests/agent/test_anthropic_adapter.py file; 23/23 in
tests/agent/test_prompt_caching.py (unaffected, confirmed).
2026-07-24 15:54:38 -07:00
ygd58
c55d780d48 fix(anthropic): filter blank text blocks in both normal and replay paths
Ports #63228 forward onto current main per teknium1's review.

Bedrock and strict Anthropic-compatible endpoints reject text blocks
where text is empty or whitespace-only with HTTP 400. The normal
list-content path extended blocks without filtering, and the
ordered-replay fast path (_sanitize_replay_block) returned blank text
blocks unfiltered.

Per review, fixes three gaps in the original port:

1. Type safety: the normal-path filter used blk.get('text', '').strip(),
   which crashes with AttributeError when text is explicitly None (not
   absent) -- .get()'s default only applies when the key is missing.
   _convert_content_part_to_anthropic() can preserve None from an
   invalid upstream input text block. Now uses
   (blk.get('text') or '').strip() on both paths.

2. Cache marker loss: prompt_caching.py's _apply_cache_marker() sets
   cache_control directly on content[-1] for list content. If that last
   part happens to be blank text, dropping it without relocating
   cache_control silently loses the breakpoint. Both the normal and
   replay paths now capture a dropped block's cache_control and reapply
   it to the new last surviving cacheable block via the existing
   _apply_assistant_cache_control_to_last_cacheable_block() helper
   (setdefault semantics, so it never clobbers a legitimately-placed
   marker).

3. Scalar whitespace: the non-list content branch
   (blocks.append({'type': 'text', 'text': str(content)})) accepted a
   truthy whitespace-only string unfiltered. Now filtered the same way
   as list-content blocks.

8/8 new tests pass in TestBlankTextBlockFiltering (including None-safety,
scalar-whitespace, and cache_control-relocation regressions on both
paths); 186/186 in the full tests/agent/test_anthropic_adapter.py file.
2026-07-24 15:54:38 -07:00
Israel Lot
6bc8d68ad7 fix(codex): scope 24h retention to Bedrock Mantle 2026-07-24 15:54:08 -07:00
Israel Lot
851b72b8f2 fix(codex): extend 24h cache retention to supported models 2026-07-24 15:54:08 -07:00
Israel Lot
f54fa1bcb7 fix(codex): exclude models.github.ai from auxiliary cache retention 2026-07-24 15:54:08 -07:00
Israel Lot
48049a1d31 fix(codex): skip auxiliary cache retention on Codex backend 2026-07-24 15:54:08 -07:00
Israel Lot
339be21542 fix(codex): send prompt_cache_retention 24h for the GPT-5.5 family
OpenAI documents GPT-5.5 / GPT-5.5 Pro as extended-cache-only: in-memory
prompt cache retention is not available for them, and only
prompt_cache_retention: "24h" is supported. Responses requests that omit
the field see near-zero cached_tokens even with a stable prompt_cache_key
and identical prefixes (observed on an OpenAI-compatible Responses relay:
0 cached across repeated identical calls before; 97% cache reads after).

Send the field for the gpt-5.5 model family (bare and namespaced ids like
openai.gpt-5.5) on OpenAI-compatible Responses routes, mirrored in the
auxiliary Codex adapter, and pass it through preflight normalization.
Skipped for xAI, GitHub/Copilot, and the chatgpt.com Codex backend, which
reject or ignore body-level cache fields.
2026-07-24 15:54:08 -07:00
teknium1
98470ae33b fix(ssl): detect and repair a missing certifi cacert.pem via existing venv-repair infra
A brew Python upgrade (original report) or an interrupted venv rebuild
(v0.19.0 report in the same thread) can leave certifi importable while
its bundled cacert.pem is missing or a dangling symlink. Every TLS
connection then fails — Feishu/Telegram/WeChat/DingTalk all down —
with an opaque 'Could not find a suitable TLS CA certificate bundle'
from deep inside httpx/requests.

The existing repair infrastructure only probed
`hasattr(certifi, 'contents')`, which PASSES in exactly this failure
state, so neither the early venv self-heal nor `hermes update`'s
import-probe repair ever classified certifi as broken. Extended, not
replaced:

- hermes_cli/_early_recovery.py: the in-process probe now also
  validates that certifi.where() exists and is a plausible bundle
  (>=1KiB), so the pre-import self-heal repairs it like any other
  wiped core package.
- hermes_cli/main.py (_detect_broken_lazy_refresh_imports): the
  subprocess probe script used by `hermes update`'s venv repair
  applies the same bundle-file check inside the target venv.
- hermes_cli/doctor.py: `hermes doctor` already failed the cert
  check; `hermes doctor --fix` now repairs it (pip force-reinstall
  certifi + module-cache invalidation + re-verify), covering
  brew/manual venvs where no update marker exists. Failures funnel
  into the manual-action list with the exact command.
- agent/ssl_guard.py: the startup SSLConfigurationError hint now leads
  with `hermes doctor --fix` instead of only the raw pip command.

Fixes #29866
2026-07-24 15:53:38 -07:00
teknium1
722bf5d510 fix(cron): preserve jobs.json ownership on root rewrite + surface failing-tick reason
Running any state-writing `hermes cron` CLI command as root (the
default for `docker exec`) rewrote jobs.json via mkstemp +
atomic_replace, leaving it root:root mode 600. The gateway's ticker
(uid 1000 via PUID/PGID) was then locked out of every tick with
PermissionError — silently: the liveness heartbeat stayed fresh,
`hermes cron status` opened with 'Gateway is running — cron jobs will
fire automatically', and in the field ~14h of scheduled jobs were
skipped before a human noticed the absence of messages.

Fixes, per the issue's suggested items 1 and 3:

1. Ownership preservation on save (cron/jobs.py): snapshot the owner
   before the atomic replace; when the writer is privileged (euid 0)
   and the previous owner differs, chown the rewritten file back.
   First-time creation inherits the cron dir's owner. Unprivileged
   writers never call chown. POSIX-only (guarded via os.name/getattr),
   best-effort — a chown failure logs a warning but never breaks the
   save. 0600 hardening is unchanged.

2. Zombie-ticker surfacing: the ticker loop (both single-profile and
   multiplex paths) now persists the failure reason to a
   ticker_last_error marker next to the heartbeat files on every
   failed tick, and clears it on the next clean tick. `hermes cron
   status` shows the recorded reason in its 'ticks may be failing'
   branch, plus an actionable ownership hint when the error is a
   PermissionError (recommend `docker exec -u <uid>:<gid>`).

Fixes #68483
2026-07-24 15:52:13 -07:00
amanning3390
366242e479 fix(acp): allow hosts to skip configured MCP startup 2026-07-24 15:51:20 -07:00
amanning3390
615a0d9141 perf(acp): bound the cross-provider model inventory for ACP clients
ACP clients (Zed, Buzz) render the whole availableModels array in a single
dropdown, so requesting the shared inventory with max_models=None could
hand an editor an unbounded cross-provider catalog.

Request the same per-provider cap the MoA picker already uses
(hermes_cli/moa_cmd.py), exposed as ACP_MAX_MODELS_PER_PROVIDER so the
intent is documented at the call site.

This bounds each provider's row rather than the total, matching the shared
inventory's own semantics: aggregator providers stay intentionally
uncapped, and the existing current-model fallback still re-inserts a
selection that falls outside the cap. At present no authenticated provider
approaches 200 models, so the visible catalog is unchanged; the cap is a
guardrail for large catalogs (e.g. OpenRouter) rather than a change to
today's lists.

The new test asserts the contract - bounded row plus a reachable current
selection - instead of a fixed catalog size, so growing the inventory
cannot turn it into a change-detector.

Co-authored-by: amanning3390 <adam.manning@pro-serveinc.com>
Signed-off-by: amanning3390 <adam.manning@pro-serveinc.com>
2026-07-24 15:51:06 -07:00
amanning3390
33908ff9ff feat(acp): expose authenticated cross-provider model choices 2026-07-24 15:51:06 -07:00
JonthanaHanh
431d2a628c fix: break unbounded 401 retry loop in credential pool OAuth path
When api_key_hint from a 401 response doesn't match any pool entry
(common with OAuth tokens where runtime_api_key rotates), the pool
rotated without marking anything exhausted and handed back a fresh
selection. Because nothing was ever marked, the pool could never reach
the "no available entries" state — the caller retried the same dead
token forever (~6 attempts/sec), starving the event loop so /stop was
never processed; only killing the gateway ended it.

Rebased onto the identity-tracking rework that landed on main
(73c4b5a045): the single-entry escape from that commit already stops
the most common OAuth case, so this fix bounds the REMAINING gap —
multi-entry pools ping-ponging A->B->A with an unmatched hint. Cap
consecutive no-mark rotations at one full lap of the available
entries, then return None so the error surfaces / fallback activates.

Deliberately does NOT mark innocent entries exhausted (the original
PR's approach): that would quarantine a healthy key for the full
cooldown TTL on a hint that provably matches nothing. No cooldown is
written by the escape, so healthy keys stay available next turn --
bounded without hammering.

The streak resets when a rotation identifies a real entry and on any
successful normal select(), so only genuinely consecutive unmatched
rotations trip the bound.

Fixes #70401
2026-07-24 15:50:36 -07:00
teknium1
8b45a8b0d4 fix(gateway): deregister transient reload label after one-shot job + test
Follow-up on the #69500 salvage: 'launchctl submit' jobs remain
registered with launchd after they exit, so every plist reload leaked
one dead '<label>.reload.<pid>.<ts>' label. The helper script now ends
with 'launchctl remove' of its own transient label, and the recovery
test asserts the self-removal is present.
2026-07-24 15:49:29 -07:00