Skip to content

fix: reset auth cooldowns across profiles - #58146

Open
ruizanthony wants to merge 2 commits into
NousResearch:mainfrom
ruizanthony:fix/auth-reset-profiles
Open

fix: reset auth cooldowns across profiles#58146
ruizanthony wants to merge 2 commits into
NousResearch:mainfrom
ruizanthony:fix/auth-reset-profiles

Conversation

@ruizanthony

Copy link
Copy Markdown
Contributor

Summary

  • make hermes auth reset <provider> clear persisted credential status fields across named profiles when run from the default profile
  • add --all-profiles and --current-profile-only scope controls
  • reset profile-local stores directly so global fallback credentials are not materialized into profiles that only inherit root auth

Why

Credential cooldown/exhaustion state is stored per HERMES_HOME. In multi-profile setups, resetting a provider from the default profile only cleared the default profile's auth.json, leaving named profiles such as agent workers stuck in stale exhausted state.

Tests

  • python3 -m py_compile hermes_cli/auth_commands.py hermes_cli/subcommands/auth.py
  • PYTHONPATH=/tmp/hermes-agent-pr-auth-reset-profiles /usr/local/lib/hermes-agent/venv/bin/python -m pytest tests/hermes_cli/test_auth_commands.py tests/hermes_cli/test_auth_profile_fallback.py -q -o 'addopts='

Note: scripts/run_tests.sh ... was attempted in the clean worktree but that temporary worktree has no local .venv/venv, so the targeted pytest command used the installed Hermes venv while keeping PYTHONPATH on the clean PR worktree.

@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard area/auth Authentication, OAuth, credential pools P2 Medium — degraded but workaround exists labels Jul 4, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary

Verdict: LGTM

Clean fix PR adding multi-profile auth cooldown reset functionality. Well-scoped, adds dedicated helpers for iterating profile homes and resetting provider status across profiles. No security concerns, no test files touched.

Looks Good

  • Good separation: _iter_known_profile_homes, _auth_reset_targets, _reset_provider_statuses_in_home each have single responsibility
  • Path resolution with resolve(strict=False) handles edge cases gracefully
  • Deduplication logic prevents redundant resets
  • Informational print statements are appropriate for a CLI command

Minor

  • _AUTH_STATUS_FIELDS is a tuple used as a frozenset — consider naming it _AUTH_STATUS_FIELD_NAMES to clarify intent

Reviewed by Hermes Agent

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Supplementary review: confirms LGTM. Auth cooldown reset across profiles is a clean, well-scoped fix. The 1 prior COMMENT review noted no blocking issues. No additional concerns identified from diff inspection.


Reviewed by Hermes Agent

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused profile-isolation fix. The current reset path is active-profile-only at hermes_cli/auth_commands.py:480-484, while profile credential reads can use a root fallback but writes remain profile-local at hermes_cli/auth.py:1322-1366; the proposed direct local-store update addresses the reported stale-cooldown case.

Problems

  • The new coverage in tests/hermes_cli/test_auth_commands.py (PR 380993034d1c) exercises the default implicit-all-profiles path, but not either new scope control. In particular, --current-profile-only must prove it does not modify the fallback root store, and --all-profiles from a named profile must prove it reaches every profile.

Suggested changes

  • Add those two scope tests and a parser test that rejects the mutually exclusive flags together.

Automated hermes-sweeper review.

@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 area/profiles Multi-profile isolation, HERMES_HOME scoping labels Jul 15, 2026
@ruizanthony
ruizanthony force-pushed the fix/auth-reset-profiles branch from 3809930 to d4b273a Compare July 29, 2026 18:13
@ruizanthony

Copy link
Copy Markdown
Contributor Author

Addressed the requested scope coverage and rebased onto current main. Added tests for --current-profile-only, explicit --all-profiles from a named profile, and mutual exclusion. Exact head: d4b273aa7f9346f345b39cb7e279ec9a9b18ad02; targeted auth-reset gate: 6 passed.

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 area/profiles Multi-profile isolation, HERMES_HOME scoping comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists 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/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants