Skip to content

Respect configured ZAI base URL - #37816

Open
zhangr011 wants to merge 1 commit into
NousResearch:mainfrom
zhangr011:codex/fix-zai-config-base-url
Open

Respect configured ZAI base URL#37816
zhangr011 wants to merge 1 commit into
NousResearch:mainfrom
zhangr011:codex/fix-zai-config-base-url

Conversation

@zhangr011

@zhangr011 zhangr011 commented Jun 3, 2026

Copy link
Copy Markdown

Fixes #18302.

Summary

  • prefer explicit model.base_url for ZAI after GLM_BASE_URL and before endpoint probing/defaults
  • make ZAI provider status report the configured base URL
  • make auxiliary credential-pool resolution honor the configured ZAI base URL so stale pool entries do not send side tasks back to /api/paas/v4
  • add regression coverage for resolver precedence and auxiliary stale-pool override

Verification

  • python -m py_compile hermes_cli/auth.py agent/auxiliary_client.py tests/hermes_cli/test_api_key_providers.py tests/agent/test_auxiliary_client.py
  • PYTHONPATH=/Users/zhangrong/Documents/home/.pytest-shim:/Users/zhangrong/Documents/home/hermes-agent python -m pytest -q tests/hermes_cli/test_api_key_providers.py::TestZaiEndpointAutoDetect::test_config_base_url_skips_probe tests/agent/test_auxiliary_client.py::test_zai_pool_base_url_honors_config_override
    • Result: 2 passed, 8 warnings

Note: local Conda Python segfaults when importing native readline, so the targeted pytest run used a temporary sitecustomize.py shim that preloads a no-op readline module.

@alt-glitch alt-glitch added type/bug Something isn't working provider/zai ZAI provider P3 Low — cosmetic, nice to have P2 Medium — degraded but workaround exists labels Jun 3, 2026
@DeamonDev888

Copy link
Copy Markdown

This PR and #62467 solve the same problem from different angles.

What this PR does well: targets the specific case where config.yaml model.base_url should be honored for Z.AI. The approach of checking pool_url_is_default before applying config is correct and preserves per-credential routing.

What #62467 adds on top:

  1. All providers, not just Z.AI — the unified resolver agent/auth.py::resolve_provider_credentials() covers 19+ providers (Kimi, MiniMax-CN, Copilot Enterprise, DeepSeek, OpenRouter, etc.) with the same pool_url_is_default guard you implement here.

  2. Gateway + Desktop path — this PR fixes auxiliary_client.py, but the main runtime path (runtime_provider.py, used by Telegram/Discord/Desktop) has a separate copy of the base_url resolution logic that also ignores config. feat(auth): unified credential resolver + pool management API + security hardening #62467 replaces both paths with a single delegation to the unified resolver.

  3. hermes auth add --base-url — lets users pin the endpoint at add-time, avoiding the cache problem entirely.

  4. REST API — 6 endpoints (/api/credentials/pool/{provider}) for managing pool entries with base_url, strategy, reset, and health — backend for the Desktop Pool Manager UI.

If this PR merges first, #62467 will automatically rebase on top (the resolver will call through the fixed path). If #62467 merges first, this PR's logic is absorbed into the unified resolver Step 3-5.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for targeting the remaining Z.AI configuration gap. The auxiliary pool path on current main still derives its endpoint from the selected pool entry at agent/auxiliary_client.py:1855-1862, so the underlying issue is real.

Problems

  • The added early return in agent/auxiliary_client.py overrides every Z.AI pool entry when model.base_url is set. That discards deliberately pinned per-credential endpoints. Current runtime resolution preserves those entries by applying config only when the pool URL equals the registry default (hermes_cli/runtime_provider.py:481-491).
  • The new test covers only a default/stale pool URL, not an explicit non-default pool URL.

Suggested changes

  • Gate the auxiliary config override on the same pool_url_is_default condition used by hermes_cli/runtime_provider.py:487-491.
  • Add a test proving an explicit Z.AI pool endpoint is retained while a default endpoint is overridden.

Automated hermes-sweeper review.

Comment thread agent/auxiliary_client.py
return _config_model_base_url(provider_id)
except Exception:
return ""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This applies model.base_url before examining the selected pool entry, so it also replaces a deliberately pinned non-default Z.AI endpoint. Please mirror the pool_url_is_default guard in hermes_cli/runtime_provider.py:487-491, and add coverage for retaining an explicit pool URL.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Medium — degraded but workaround exists P3 Low — cosmetic, nice to have provider/zai ZAI provider sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: ZAI/GLM provider base_url causes 429/404 errors — config ignored by 5 code paths

4 participants