feat(auth): support named credentials with manual selection (#76937) - #76987
feat(auth): support named credentials with manual selection (#76937)#76987webtecnica wants to merge 1 commit into
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for building the credential persistence and fallback mechanics. The current implementation is a useful base, but it does not yet provide the per-task/session selection requested in #76937.
Problems
- #76937 proposes
hermes chat ... --auth <name>. This PR adds--nameonly tohermes auth add(hermes_cli/subcommands/auth.py:+31) and reads one profile-widedefault_auth[provider]preference (agent/credential_pool.py:+492,+618), so concurrent sessions cannot choose different keys. - Delegated children bypass the preference: the PR narrows normal selection (
agent/credential_pool.py:+1810), butCredentialPool.acquire_lease()chooses from all available entries (agent/credential_pool.py:1971-1985) andtools/delegate_tool.py:1992uses that path. - A name is not unique: the new helper only strips it, while the selection helper accepts all matching entries. Duplicate
--name dailycredentials therefore do not identify one key.
Suggested changes
- Add and propagate a session-scoped selector, preserving a global default only as fallback.
- Apply the constraint to leasing and cover delegated children, duplicate names, and two simultaneous sessions.
- Document the finalized CLI/config behavior.
Automated hermes-sweeper review.
| @@ -28,6 +28,11 @@ def build_auth_parser(subparsers, *, cmd_auth: Callable) -> None: | |||
| help="Credential type to add", | |||
| ) | |||
| auth_add.add_argument("--label", help="Optional display label") | |||
| auth_add.add_argument( | |||
There was a problem hiding this comment.
This only names credentials at creation time. #76937 asks for hermes chat ... --auth <name> selection per task/session, but this diff adds no session-level selector or propagation into agent construction; a profile-wide config preference cannot let concurrent sessions choose different keys.
| def _select_unlocked(self, *, refresh: bool = True) -> Optional[PooledCredential]: | ||
| available = self._available_entries(clear_expired=True, refresh=refresh) | ||
| if not available: | ||
| self._current_id = None | ||
| self._log_no_available_entries() | ||
| return None | ||
|
|
||
| # Manual selection (#76937): pin to the configured default_auth | ||
| # credential while it is available; fall back to auto-rotate below | ||
| # when it is missing or exhausted. |
There was a problem hiding this comment.
Please apply this narrowing to acquire_lease() as well. tools/delegate_tool.py:1992 leases child credentials through that method, and its current implementation chooses from all available entries, so delegated tasks can bypass the selected credential.
SummaryOne PR addresses #76937. #76987 adds named-credential persistence and profile-wide default selection, but its diff does not implement the requested per-task/session selector and does not apply the selection constraint to delegated credential leases. Related pull requests
Suggested consolidationKeep #76987 open with a salvage path: retain its credential naming, persistence, profile-wide fallback, and tests, then add and propagate a session-scoped selector, apply the same narrowing to Complex graphflowchart 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
I76937(["issue #76937 (open)"])
P76987["PR #76987 (open)"]
P76987 -->|best fix| I76937
class I76937 open
class P76987 open
class P76987 best
class P76987 target
click I76937 "https://github.com/NousResearch/hermes-agent/issues/76937"
click P76987 "https://github.com/NousResearch/hermes-agent/pull/76987"
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 1 pull request and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 27 kB of PR diffs, 3 kB of issue/PR text, 2 kB of discussion (3 comments), 2 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
20d985a to
a9719fb
Compare
Summary
Adds optional named credentials to the credential pool, enabling manual selection of which API key to use per task/session — not just passive auto-rotation on 402/429 errors.
Change
namefield onPooledCredential(distinct from display label), set viahermes auth add <provider> --api-key ... --name dailydefault_authconfig map (provider -> credential name) under config.yaml, with schema entry--nameprompt/flag inauth_commands.py+auth.pypersistence + subcommand passthroughtests/agent/test_credential_pool_named_selection.py(new) +tests/hermes_cli/test_auth_commands.py(extended)Verification
pytest tests/agent/test_credential_pool_named_selection.py— 28 passedpytest tests/agent/test_credential_pool.py— 96 passedpytest tests/hermes_cli/test_auth_commands.py— 14 passedCloses #76937