Skip to content

feat(cli): add --base-url flag to hermes auth add + compact auth list display - #66971

Open
DeamonDev888 wants to merge 3 commits into
NousResearch:mainfrom
DeamonDev888:feat/cli-auth-base-url-flag
Open

feat(cli): add --base-url flag to hermes auth add + compact auth list display#66971
DeamonDev888 wants to merge 3 commits into
NousResearch:mainfrom
DeamonDev888:feat/cli-auth-base-url-flag

Conversation

@DeamonDev888

Copy link
Copy Markdown

Summary

Two CLI improvements for credential management:

1. hermes auth add --base-url

New flag lets users specify the inference base URL when adding a credential. Previously base_url was hardcoded to pconfig.inference_base_url — wrong for providers with multiple endpoints (Z.AI Coding Plan, MiniMax-CN, Kimi, etc.).

hermes auth add zai --type api-key --api-key KEY --base-url https://api.z.ai/api/coding/paas/v4

2. hermes auth list — compact display

Multi-endpoint providers show short tags instead of full URLs:

Before:

  #1  GLM coding 26   api_key manual  url=https://api.z.ai/api/coding/paas/v4

After:

  # 1  GLM 26       api_key manual  coding ←

Rules:

  • coding, anthropic, zai-cn, minimax-cn for known multi-endpoint providers
  • Custom URLs: truncated hostname (12 chars)
  • Single-endpoint providers: URL column suppressed
  • Strategy displayed in header

Tests

6 unit tests covering parser + display formatting.

Files

  • hermes_cli/subcommands/auth.py (+6 lines — --base-url flag)
  • hermes_cli/auth_commands.py (+74/-2 lines — handler + auth list display)
  • tests/agent/test_auth_cli_improvements.py (+208 lines, NEW)

Related

@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #54524 and #62467: both cover the base-url credential direction, but #54524 also changes per-credential runtime pooling. This PR is a narrower CLI/display alternative; its tests do not exercise the agent/runtime resolver, so maintainers should choose the intended scope.

… entry

Addresses alt-glitch feedback: tests now exercise the full auth_add_command
handler with a FakePool to verify that:
1. --base-url value is stored on PooledCredential.base_url
2. Without --base-url, _provider_base_url() default is used

This proves the CLI flag actually reaches the credential pool storage.
@DeamonDev888

Copy link
Copy Markdown
Author

Addressed. Commit 20ef3433d adds 2 integration tests that exercise the full auth_add_command handler with a FakePool to verify the base_url actually reaches PooledCredential.base_url:

  1. test_base_url_flag_stored_on_entry — passes --base-url https://api.z.ai/api/coding/paas/v4 and asserts the value lands on entry.base_url
  2. test_no_base_url_flag_uses_registry_default — omits --base-url and asserts _provider_base_url() default is used instead

Regarding scope vs #54524: this PR is intentionally narrow (CLI display + storage only). It does not change the runtime resolver or pooling logic — that broader scope lives in #62467. The tests here prove the CLI layer works correctly; the runtime layer is separately tested in #62467.

@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 the focused CLI contribution. The core API-key path addresses a current-main gap: hermes_cli/auth_commands.py:218 still stores _provider_base_url(provider), while hermes_cli/runtime_provider.py:415 already resolves a selected credential's explicit endpoint and preserves a non-default entry URL at :487-491.

Problems

  • The new shared parser option is only consumed by the API-key branch in the proposed auth_add_command hunk. Existing OAuth branches construct entries from OAuth-derived/provider URLs (hermes_cli/auth_commands.py:224-432 on current main), so hermes auth add <oauth-provider> --type oauth --base-url ... would be accepted and silently ignored.

Suggested changes

  • Make the flag API-key-specific and reject it for OAuth additions, or wire it through only the OAuth flows for which an override is valid; add a regression test for that contract.
  • Keep CLI auth integration tests with the existing suite at tests/hermes_cli/test_auth_commands.py.

Automated hermes-sweeper review.

auth_add.add_argument(
"--api-key", help="API key value (otherwise prompted securely)"
)
auth_add.add_argument(

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.

--base-url is accepted for every auth add type, but the proposed handler only reads it in the API-key branch. Please either reject it for OAuth additions or define the supported OAuth semantics and test them; silently accepting an ignored endpoint override is misleading.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data 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 19, 2026
Addresses teknium1 feedback: --base-url was silently ignored for OAuth
additions. Now raises SystemExit early with a clear message:

  '--base-url is only supported for API-key credentials.'

Added test_base_url_rejected_for_oauth_type as regression test.
9/9 tests pass.
@DeamonDev888

Copy link
Copy Markdown
Author

Addressed in commit 047b8552a:

--base-url now rejected for OAuth: added an early guard in auth_add_command that raises SystemExit with a clear message if --base-url is provided for a non-API-key auth type:

--base-url is only supported for API-key credentials.
OAuth providers derive their endpoint automatically.

Added test_base_url_rejected_for_oauth_type regression test. 9/9 tests pass.

Regarding the existing test suite at tests/hermes_cli/test_auth_commands.py — good suggestion. I'll move the integration tests there in a follow-up commit if this PR direction is approved.

freqyfreqy added a commit to freqyfreqy/hermes-agent that referenced this pull request Aug 1, 2026
Remove the --base-url flag from hermes auth add and the handler in
auth_commands.py. This CLI surface is fully covered by NousResearch#66971 with
better UX (compact auth list display with endpoint tags).

This PR retains only the unique scope:
- config credentials: list seeding for per-credential base_url
- runtime URL resolution from selected pool entry
- pool identity matching for nested credentials[].base_url
- 6 regression tests for all three paths + backward compat

Removes the only overlap with NousResearch#66971 to simplify merge sequencing.
@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary

Three open PRs address this issue complex through complementary surfaces: #54524 implements config-seeded per-credential endpoints and the runtime/recovery identity needed for rotation, #58088 fixes Z.AI config.yaml endpoint precedence, and #66971 adds explicit CLI storage plus endpoint visibility. The diffs therefore cover the reported causes at the custom-pool runtime, Z.AI resolver, and credential-entry CLI layers respectively.

Related pull requests

Suggested consolidation

Keep #54524 open with a salvage path centered on its recorded-best-fix config seeding, selected-entry runtime URL, nested-URL recovery identity, and regressions; this follows the automated keep_open verdict and the contributor finding that its scope is distinct. Keep #58088 open with a salvage path for its recorded-best-fix Z.AI precedence change, with author action to add the glm-alias runtime and auxiliary-caller regressions requested by the automated keep_open review; keep #66971 open for its API-key CLI override and display work, with author action to move the integration coverage into tests/hermes_cli/test_auth_commands.py as requested by its contributor keep_open review.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I54011(["issue #54011 (open)"])
    I58071(["issue #58071 (open)"])
    P66971["PR #66971 (open)"]
    P66971 -.->|partial| I54011
    P66971 -.->|partial| I58071
    class I54011 open
    class I58071 open
    class P66971 open
    class P66971 target
    click I54011 "https://github.com/NousResearch/hermes-agent/issues/54011"
    click I58071 "https://github.com/NousResearch/hermes-agent/issues/58071"
    click P66971 "https://github.com/NousResearch/hermes-agent/pull/66971"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 3 pull requests and 2 issues in this complex. Each diff was read against this issue; Assessment working set: 41 kB of PR diffs, 13 kB of issue/PR text, 14 kB of discussion (15 comments), 6 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

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 needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have 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 sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants