Skip to content

fix(auth): persist detected z.ai endpoint cache safely - #18863

Closed
z80dev wants to merge 1 commit into
NousResearch:mainfrom
z80dev:fix/zai-endpoint-cache-persistence
Closed

z80dev wants to merge 1 commit into
NousResearch:mainfrom
z80dev:fix/zai-endpoint-cache-persistence

Conversation

@z80dev

@z80dev z80dev commented May 2, 2026

Copy link
Copy Markdown

What does this PR do?

Fixes Z.AI endpoint detection so a discovered coding-plan endpoint is actually persisted to auth.json, while avoiding an accidental switch of the user's active provider to zai during passive endpoint resolution.

Related Issue

No existing issue found/linked. This came from debugging Z.AI / GLM coding-plan endpoint selection where keys can work on the coding endpoint but fail or show insufficient balance on the generic endpoint.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • hermes_cli/auth.py
    • Persist detected Z.AI endpoint metadata under the auth-store lock.
    • Reload the auth store inside the lock before writing the cache entry.
    • Store providers.zai.detected_endpoint with _store_provider_state(..., set_active=False).
    • Flush the updated auth store with _save_auth_store(auth_store).
  • tests/hermes_cli/test_api_key_providers.py
    • Add regression coverage verifying the detected endpoint is persisted.
    • Verify passive Z.AI endpoint detection preserves an arbitrary pre-existing active provider and its provider state exactly.

How to Test

  1. Configure a Z.AI / GLM API key with coding-plan access and no explicit GLM_BASE_URL override.
  2. Resolve Z.AI API-key provider credentials, e.g. by selecting/using a zai/... GLM model.
  3. Confirm auth.json contains providers.zai.detected_endpoint for the detected coding endpoint.
  4. Confirm the existing active provider is not silently switched to zai unless the user explicitly selects it.

Regression test runs:

python -m pytest tests/hermes_cli/test_api_key_providers.py::TestZaiEndpointAutoDetect::test_probe_success_persists_detected_endpoint -q -o addopts=''
python -m pytest tests/hermes_cli/test_api_key_providers.py -q -o addopts=''

Latest targeted local verification on EndeavourOS x86_64 with Python 3.11.14:

1 passed in 0.61s
158 passed in 8.80s

Full-suite attempt after rebasing on current origin/main:

HERMES_TEST_WORKERS=4 scripts/run_tests.sh -q

Current local result is not green due failures outside this PR's touched files:

27 failed, 20011 passed, 59 skipped, 214 warnings in 279.42s

The full-suite checkbox below is intentionally left unchecked until CI or a maintainer-confirmed local environment produces a green full-suite run.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: EndeavourOS x86_64, Python 3.11.14

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Screenshots / Logs

Targeted regression output:

1 passed in 0.61s
158 passed in 8.80s

Copilot AI review requested due to automatic review settings May 2, 2026 14:54
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists area/auth Authentication, OAuth, credential pools provider/zai ZAI provider comp/cli CLI entry point, hermes_cli/, setup wizard labels May 2, 2026

Copilot AI 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.

Pull request overview

This PR fixes Z.AI endpoint auto-detection caching by persisting the detected endpoint to auth.json under the auth-store lock, while ensuring the detection flow does not change the user’s active_provider.

Changes:

  • Persist detected Z.AI endpoint results by reloading auth.json inside _auth_store_lock() and flushing via _save_auth_store().
  • Store providers.zai.detected_endpoint using _store_provider_state(..., set_active=False) to avoid promoting zai to the active provider.
  • Add a regression test verifying both persistence of detected_endpoint and that active_provider is not switched to zai.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
hermes_cli/auth.py Writes detected Z.AI endpoint cache to auth.json safely under lock without changing active_provider.
tests/hermes_cli/test_api_key_providers.py Adds regression coverage for persistence + non-promotion of zai as active provider.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

assert detected["base_url"] == "https://api.z.ai/api/coding/paas/v4"
assert detected["endpoint_id"] == "coding-global"
assert detected["model"] == "glm-5.1"
assert payload.get("active_provider") != "zai"
@z80dev
z80dev force-pushed the fix/zai-endpoint-cache-persistence branch 2 times, most recently from 57102b0 to ba54107 Compare May 5, 2026 22:44
@z80dev

z80dev commented May 5, 2026

Copy link
Copy Markdown
Author

Updated this PR to line up with the contributor guidelines and PR template:

  • Rebased onto current origin/main.
  • Addressed Copilot's valid test nit by asserting the existing active provider remains exactly openai-codex, not merely "not zai".
  • Kept the PR focused to the Z.AI endpoint-cache persistence fix and its regression test.
  • Updated the PR body/checklist with changed files, test plan, platform, and verification logs.

Targeted verification is green locally:

python -m pytest tests/hermes_cli/test_api_key_providers.py::TestZaiEndpointAutoDetect::test_probe_success_persists_detected_endpoint -q -o addopts=''
1 passed in 0.61s

python -m pytest tests/hermes_cli/test_api_key_providers.py -q -o addopts=''
158 passed in 9.11s

I also attempted the canonical local full suite after installing the missing pytest-split helper into the shared venv:

HERMES_TEST_WORKERS=4 scripts/run_tests.sh -q

That run is not currently green in my local environment (27 failed, 20011 passed, 59 skipped) with failures outside this PR's touched files, so I intentionally left the full-suite checkbox unchecked rather than overstating it. CI still appears to need maintainer approval to run on the PR head.

@z80dev
z80dev force-pushed the fix/zai-endpoint-cache-persistence branch from ba54107 to d5f04ba Compare May 5, 2026 22:53
@z80dev

z80dev commented May 5, 2026

Copy link
Copy Markdown
Author

Good catch on the openai-codex wording/fixture value. That was only intended as the pre-existing provider in the regression fixture, but you're right that the test should not encode an OpenAI Codex assumption.

I updated the regression to use an arbitrary previous-provider value and assert that both:

  • active_provider remains exactly whatever was there before endpoint detection
  • the previous provider's stored state is preserved unchanged

Targeted verification after that update:

python -m pytest tests/hermes_cli/test_api_key_providers.py::TestZaiEndpointAutoDetect::test_probe_success_persists_detected_endpoint -q -o addopts=''
1 passed in 0.46s

python -m pytest tests/hermes_cli/test_api_key_providers.py -q -o addopts=''
158 passed in 8.80s

Force-pushed the amended commit with that more general regression coverage.

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the focused reproduction and regression coverage. This is an automated hermes-sweeper review; current main already provides this exact behavior and a stronger failure path.

  • hermes_cli/auth.py:715-724 reloads the auth store under _auth_store_lock(), persists providers.zai.detected_endpoint, and calls _store_provider_state(..., set_active=False) before _save_auth_store().
  • 832c5f9bc9018b5540c13cbc35805cc49ce8b073 added the locked disk persistence; follow-up 6eeed3f1e8847a352ae1090450a71f0975a3d3a9 prevents the cache write from switching active_provider.
  • 1192f29450f1dc440d44b094817977f00475643c further makes persistence failure non-fatal after successful endpoint detection.
  • Related PR Fix slow startup for Z.AI provider by caching auto-detected endpoint to disk #41201 records the maintainer-confirmed cherry-pick of the persistence fix and these follow-ups.

@teknium1 teknium1 closed this Jul 12, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jul 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists provider/zai ZAI provider sweeper:implemented-on-main Sweeper: behavior already present on current main type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants