Reliability fixes: auto-IPv4 on dead IPv6, local-endpoint billing, silent worker-config warning - #52538
Reliability fixes: auto-IPv4 on dead IPv6, local-endpoint billing, silent worker-config warning#52538TinkerOfThings wants to merge 5 commits into
Conversation
…lve_billing_route resolve_billing_route only matched 'custom'/'local' providers or a base_url containing the literal 'localhost', so LM Studio's default http://127.0.0.1:1234 and LAN/Tailscale-hosted models fell through to the generic route. Use the existing is_local_endpoint() helper (loopback, private, Tailscale) and a _LOCAL_PROVIDERS set (custom/local/lmstudio/ollama/vllm/llamacpp) so self-hosted model servers are classified consistently regardless of how they're addressed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rovider has no base_url _resolve_named_custom_runtime returned None when a configured named custom/local provider had no base_url, and the caller silently fell back to default (OpenRouter) resolution — which produces nothing if no other credentials exist, with zero trace of why. This is the NousResearch#1 silent kanban-worker footgun. Log a WARNING naming the provider (and pointing at LM_BASE_URL) so a stuck worker's per-task log explains the cause. Fires only on a genuine misconfig (named provider, empty endpoint) — no false positives. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
apply_ipv4_preference was a no-op unless network.force_ipv4 was set, so a host with a configured-but-dead IPv6 stack (a v6 address with no route) kept hanging on AAAA-first DNS for the full TCP timeout — the failure that flapped the Telegram poller. Add ipv6_route_alive() (an instant, packet-free UDP-connect probe, cached per process); apply_ipv4_preference now auto-enables the patch when IPv6 is dead and logs a WARNING (a self-check that rides every entrypoint). The three entrypoints (main, gateway, cron) now always call it with force=<flag> instead of only when the flag is set. Healthy dual-stack hosts probe 'alive' and are untouched; force_ipv4:true still forces it on and skips the probe. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ion) Now that apply_ipv4_preference auto-applies at import time on IPv6-dead hosts, importing an entrypoint can leave socket.getaddrinfo patched before test_ipv4_preference runs, and the double-patch guard then blocks the tests' re-patch assertions (order-dependent failure, invisible in isolation). Stash the pristine resolver on the patched fn (_hermes_original) and have the test setup unwrap it so each test starts from a clean global. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
7f6d6bb to
ed72cce
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused reliability fixes. The three premises are present on current main: agent/usage_pricing.py:675 only recognizes custom/local or literal localhost; hermes_cli/runtime_provider.py:982-983 returns silently for an empty named-provider endpoint; and hermes_constants.py:998-999 makes the IPv4 helper a no-op unless forced.
Problems
- At PR-head
hermes_constants.py:1002, the dead-route warning runs before the existing patch guard. PR-headcron/scheduler.py:2767invokes the helper for every job, so a cached dead probe will repeat the warning after the resolver is already patched. - The new automatic behavior at PR-head
hermes_constants.py:1000-1005is not reflected inwebsite/docs/user-guide/configuration.md:2003-2007, which still presentsforce_ipv4as an opt-in workaround.
Suggested changes
- Emit the warning only on the patch transition, or return on an existing patch before probing/logging.
- Update the configuration documentation and related CLI tip for automatic fallback and the remaining explicit override.
This is an automated hermes-sweeper review.
| # No explicit force: auto-enable only when IPv6 is configured-but-dead. | ||
| if ipv6_route_alive(): | ||
| return | ||
| logging.getLogger(__name__).warning( |
There was a problem hiding this comment.
This warning precedes the existing double-patch guard. Since cron/scheduler.py now calls this helper for every job, a cached dead probe will emit this line repeatedly after getaddrinfo is already patched. Please return on an existing patch before probing/logging, or log only when the patch is first applied.
…back Address review on NousResearch#52538: - apply_ipv4_preference: move the double-patch guard to the top of the function so it returns before the IPv6 probe and dead-route warning. The warning now fires only on the patch transition, not on every already-patched call — cron/scheduler.py invokes this per job, which previously logged the warning on every tick. - docs(configuration): document the auto-on-dead-IPv6 fallback as the default behavior in the Network section, and reframe force_ipv4 as an explicit override that skips the probe (the PR previously touched no docs). - test: add test_second_call_does_not_re_warn asserting a repeat force=False call neither re-wraps getaddrinfo nor re-emits the warning. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks for the review — both points addressed in 1. Return on an existing patch before probing/logging. Moved the double-patch guard to the top of 2. Docs. The Network section of Added a regression test ( |
Three small, independent reliability fixes, each with regression tests. Happy to split into separate PRs if you'd prefer.
1.
fix(network): auto-enable IPv4 preference when the IPv6 route is deadapply_ipv4_preferencewas a no-op unlessnetwork.force_ipv4was set, so a host with a configured-but-dead IPv6 stack (a v6 address with no route) kept hanging on AAAA-first DNS for the full TCP timeout. Addsipv6_route_alive()— an instant, packet-free UDP-connect()probe, cached per process. The patch now auto-enables when IPv6 is dead and logs a one-line self-check warning that rides every entrypoint (dashboard / gateway / cron). Healthy dual-stack hosts probe "alive" and are left untouched;force_ipv4: truestill forces it on and skips the probe. The patch is also made reversible (_hermes_original) so the import-time auto-apply can't break test isolation across files.2.
fix(billing): recognise loopback / LAN / local-provider endpoints inresolve_billing_routeIt matched provider
custom/localor a base_url containing the literallocalhost, so LM Studio's defaulthttp://127.0.0.1:1234and LAN/Tailscale-hosted models fell through to the generic route. Now uses the existingis_local_endpoint()helper (loopback, RFC-1918, Tailscale) plus a_LOCAL_PROVIDERSset (custom/local/lmstudio/ollama/vllm/llamacpp), so self-hosted servers classify consistently regardless of how they're addressed.3.
fix(provider): warn instead of failing silently when a named custom provider has nobase_url_resolve_named_custom_runtimereturnedNonewhen a configured named custom/local provider had nobase_url, and the caller silently fell back to default (OpenRouter) resolution — producing nothing when no other credentials exist, with zero trace of why. Now logs a WARNING naming the provider (and pointing atLM_BASE_URL). Fires only on a genuine misconfig, so no false positives.Tests
New regression tests cover each fix (loopback/LAN/provider-id billing classification + cloud-not-misclassified; the no-base_url warning; auto-enable-on-dead-IPv6, force-skips-probe, and the reversibility/isolation behaviour).
🤖 Generated with Claude Code