feat(agent): opt-in recovery hook for wedged local inference backends - #37176
feat(agent): opt-in recovery hook for wedged local inference backends#37176OmarB97 wants to merge 1 commit into
Conversation
Needs changes — Blocking: the bundled recovery script appends /slots directly to HERMES_RECOVERY_BASE_URL. Hermes passes OpenAI-compatible base URLs such as http://127.0.0.1:9090/v1 (the new hook test asserts that exact env value), so this probes http://127.0.0.1:9090/v1/slots instead of llama.cpp's management endpoint at /slots. That means the default shipped recovery command misses slot cancellation and falls through to no-op/kill behavior. Please normalize a trailing /v1 before building the /slots and /slots/{id}?action=cancel URLs, and add a script-level regression test for a /v1 base URL. Merge gate: Evidence-backed blocking review feedback must be resolved in a new attempt. |
Needs changes — Request changes: recover_local_llama_server.py must normalize OpenAI-compatible /v1 base URLs before probing/canceling llama.cpp /slots. Merge gate: Evidence-backed blocking review feedback must be resolved in a new attempt. |
|
Addressed the review blocker in
Verification:
|
2a39035 to
98799a6
Compare
98799a6 to
209253e
Compare
Reapply the opt-in recovery hook for wedged local inference backends (agent/local_backend_recovery.py, scripts/recover_local_llama_server.py, the resolve_stream_stale_timeout local-provider stream/first-chunk/ non-stream timeout resolvers) plus its three stacked fix-up commits (local non-stream timeout must beat the reasoning floor + restore first_chunk_seen; telegram /new "parallel work" de-flake; MoA virtual endpoint output-cap exclusion) onto current upstream/main. Conflict resolution: - agent/transports/chat_completions.py (import block): upstream removed the now-unused `import copy`; kept this branch's `import os` (used by the local-default-max-tokens env lookups added to this file) and dropped `copy` (confirmed no remaining use in the file). - agent/chat_completion_helpers.py (3 spots): upstream independently grew its own cross-turn stale-call circuit breaker (NousResearch#58962: _stale_streak/_bump_stale_streak/_reset_stale_streak/_check_stale_giveup) at the same insertion points as this branch's local-provider timeout resolvers and local-backend-recovery hook. Both features are additive with no logic overlap — kept both side by side: the circuit-breaker block ahead of the local timeout helpers; _bump_stale_streak(agent) alongside the local-backend-recovery call in the non-streaming stale-kill path; _check_stale_giveup(agent) folded into the same result-dict block that carries local_first_chunk_timeout. - tests/gateway/test_telegram_topic_mode.py: upstream already carries an equivalent fix for the same "parallel work" tip-collision flake in the same test (different patch target/style, identical intent) — took upstream's version; this branch's now-redundant duplicate nets to zero diff vs upstream in this file. Ran scripts/run_tests.sh across all 8 touched-module test files (test_error_classifier, test_local_backend_recovery, test_local_stream_timeout, transports/test_chat_completions, hermes_cli/test_timeouts, run_agent/test_run_agent, test_recover_local_llama_server, gateway/test_telegram_topic_mode): 821 passed, 0 failed. py_compile clean on all touched files. Refresh of PR NousResearch#37176; original head 025569e9573a72d00a6fb04937afbf85dd326d24.
025569e to
3b930f3
Compare
|
Refreshed onto upstream/main (rebase, live merge-base
A third apparent conflict ( Original head |
|
Thanks for the careful local-backend recovery work, including the follow-up that normalizes OpenAI-compatible Automated hermes-sweeper review found that this PR’s user-facing interface is new non-secret
Please re-scope any follow-up around a profile-safe This is an automated hermes-sweeper review. Closed as not-planned per standing maintainer policy ( |
What
When a local backend trips the first-chunk or non-stream stale watchdog (see #37168, which this PR stacks on), Hermes can now invoke an operator-configured recovery command — useful for self-hosted servers that wedge in ways only the operator knows how to repair (slot cancellation, child-process kill, service restart).
How
agent/local_backend_recovery.py:maybe_recover_local_backend()runsHERMES_LOCAL_BACKEND_RECOVERY_COMMAND(opt-in, default off) with sanitized metadata passed viaHERMES_RECOVERY_*env vars, rate-limited byHERMES_LOCAL_BACKEND_RECOVERY_COOLDOWN(default 60s) so retries cannot flap a shared GPU service.scripts/recover_local_llama_server.py: a reference recovery script for llama-server-style backends.HERMES_LOCAL_DEFAULT_MAX_TOKENScaps local completions (default 8192) so a wedged generation cannot run unbounded.Tests
tests/agent/test_local_backend_recovery.py(env contract, cooldown, opt-in gating),tests/test_recover_local_llama_server.py, transport tests for the max-tokens default. 300 tests pass locally on the stack.Stacked on #37168 — until that merges, this diff shows the union of both; the recovery-specific changes are
agent/local_backend_recovery.py, the two hook sites,scripts/recover_local_llama_server.py, and their tests. Part 5 of the former #35642 split.