fix(aux): use full 600s unhealthy TTL for permanently unavailable providers - #59985
fix(aux): use full 600s unhealthy TTL for permanently unavailable providers#59985isheng-eqi wants to merge 11 commits into
Conversation
…usResearch#58774) _restore_or_build_system_prompt unconditionally restored the session-DB stored prompt when it matched the current runtime identity, even when the caller set an explicit ephemeral_system_prompt (e.g. /personality). Check ephemeral_system_prompt before the stored-prompt fast path so a deliberate personality switch takes effect immediately instead of being silently ignored until the next fresh session.
…nto fix/58774-personality-ephemeral
…ibuteError (NousResearch#59845) The Copilot x-initiator injection block calls agent._is_copilot_url() without a getattr guard, unlike the sibling _is_user_initiated_turn check one line above. On some agent construction paths (module-reload, wrapper agents) _is_copilot_url may be missing, causing every API call in the conversation to fail with AttributeError and the cron job to error out. Wrap the call with getattr(agent, '_is_copilot_url', lambda: False)() so non-Copilot and partially-initialized agents fall through cleanly. Github-Issue:NousResearch#59845
…nt delivery The TUI notification poller (_notification_poller_loop) only watched process_registry.completion_queue, never polling kanban_notify_subs. Kanban task subscriptions with platform='tui' were therefore never delivered — the gateway's _kanban_notifier_watcher has no TUI adapter, and the TUI poller had no kanban polling logic. Add _poll_kanban_task_events() which mirrors the gateway watcher's pattern: list kanban_notify_subs for the session, claim unseen terminal events via kanban_db.claim_unseen_events_for_sub(), and emit status.update messages to the TUI session. Polled every ~5 seconds on the existing completion_queue.get() timeout path. Github-Issue:NousResearch#59960
…e kind The kanban_block tool schema documents all four block kinds and says 'kind' is optional, but goal_mode tasks silently rejected an omitted (or capability/transient) kind. This broke workers that followed the published schema contract. Two changes: 1. Update KANBAN_BLOCK_SCHEMA kind description to document the goal_mode restriction (only dependency/needs_input accepted, omit→needs_input). 2. Coerce kind=None to 'needs_input' in the goal_mode gate so workers that follow the schema's optional-kind contract don't get a hard error. capability/transient are still rejected for goal_mode. Github-Issue:NousResearch#59764
…ace partial skips When a kanban worker is spawned with --skills and ALL named skills are missing from the assignee profile, the CLI raised ValueError, causing the worker process to die. The dispatcher retried → crash-loop until the failure breaker gave up. Fix: when HERMES_KANBAN_TASK is set and all skills are missing, call kanban_block with a structured 'capability' error instead of raising. The task is blocked with a human-readable reason, no retry loop. Additionally, when only SOME skills are missing (graceful degradation path), add a kanban_comment so the card author can see the skip on the board instead of it being hidden in the worker log file. Github-Issue:NousResearch#59764
…viders _try_openrouter() and _try_nous() used ttl=60 when no credentials were configured, treating a permanent configuration state as a transient payment error. After 60s the mark expired and the provider was retried → failed identically → logged another WARNING. This flooded errors.log: on a session with only DeepSeek configured, 1,582 'marking unhealthy' and 791 'Nous unavailable' WARNINGs drowned out the 16 actual ERRORs. Use the default _AUX_UNHEALTHY_TTL_SECONDS (600s) when the provider is unavailable due to missing credentials rather than a transient payment/rate-limit error. The 60s ttl is preserved for genuine transient failures at the other call sites. Github-Issue:NousResearch#59984
…iliary unhealthy-marking _mark_provider_unhealthy() hardcoded "(payment / credit error)" at WARNING into its log line, but four of its eight call sites quarantine for non-billing reasons: absent OpenRouter/Nous credentials, a stale fallback credential, or a rate limit. A local-only setup with zero cloud credentials saw repeated WARNINGs claiming billing problems on providers it never configured. Thread a truthful reason (echoed by both the mark log and the _log_skip_unhealthy skip log) and a log level through the function. The absent-credentials sites now log "no credentials configured" at DEBUG; confirmed 402 paths keep "payment / credit error" at WARNING. Quarantine semantics (TTL, skip behavior) are unchanged. Complements NousResearch#59984/NousResearch#59985 (which fix the retry TTL for the same call sites); this fixes the wording and severity.
teknium1
left a comment
There was a problem hiding this comment.
Thanks for identifying the auxiliary retry-noise issue. The TTL premise is still present on current main: absent OpenRouter credentials use ttl=60 at agent/auxiliary_client.py:2053, absent Nous auth uses it at agent/auxiliary_client.py:2097, while the helper default is 600 seconds at agent/auxiliary_client.py:2845-2883. The related #64146 work is complementary: referenced commit 2f52a04 explicitly preserves TTL semantics while changing reason/level labeling.
Problems
- As submitted, the PR includes 10 changed files and unrelated commits affecting
agent/conversation_loop.py,cli.py,tools/kanban_tools.py, andtui_gateway/server.py. The focused TTL change should be isolated for salvage. agent/auxiliary_client.py:2140-2146is a sibling unavailable-credential path: no usable Nous inference JWT still calls_mark_provider_unhealthy("nous", ttl=60). The submitted test update covers only OpenRouter.
Suggested changes
- Keep the salvage limited to the TTL fix and its tests.
- Cover both Nous unavailable paths and remove the 60-second override from the no-usable-JWT path if it has the same configuration-state semantics.
Automated hermes-sweeper review.
| assert model is None | ||
| mock_openai.assert_not_called() | ||
| mock_mark.assert_called_once_with("openrouter", ttl=60) | ||
| # Permanently unavailable (no key) -> default TTL, not ttl=60 |
There was a problem hiding this comment.
This assertion covers only OpenRouter. Please add direct coverage for both unavailable Nous branches as well; current main's no-usable-inference-JWT path still uses ttl=60 at agent/auxiliary_client.py:2145.
Summary
Fixes #59984
Problem
_try_openrouter()and_try_nous()usedttl=60when marking providers unhealthy after discovering no credentials were configured. This treated a permanent configuration state as a transient payment error. Every 60s the mark expired, the provider was retried, failed identically, and logged another WARNING.On a typical session with only DeepSeek configured:
Real errors were buried under thousands of identical noise lines.
Fix
Use the default
_AUX_UNHEALTHY_TTL_SECONDS(600s) when the provider is permanently unavailable due to missing credentials. The 60s TTL is preserved at the other call sites for genuine transient errors (payment/credit exhaustion, rate limits).Changes
agent/auxiliary_client.py:_try_openrouter: dropttl=60when no key configuredagent/auxiliary_client.py:_try_nous: dropttl=60when no auth found