Skip to content

fix(auth): tolerate legacy Codex suppression data - #80581

Closed
DanDo385 wants to merge 3 commits into
NousResearch:mainfrom
DanDo385:feat/oauth-profile-switching
Closed

DanDo385 wants to merge 3 commits into
NousResearch:mainfrom
DanDo385:feat/oauth-profile-switching

Conversation

@DanDo385

@DanDo385 DanDo385 commented Aug 6, 2026 •

Copy link
Copy Markdown

What does this PR do?

Fixes a legacy auth-store compatibility crash during hermes auth remove openai-codex CREDENTIAL_ID.

Older stores can represent suppressed_sources[provider] as a mapping. Removing a registered Codex credential then reached .append() on that mapping and raised AttributeError. The removal path now normalizes mapping keys into the canonical list form before adding the requested source.

Related Issue

No exact duplicate issue or PR found. Related auth-removal work does not cover the legacy mapping-shaped suppression value.

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: normalize legacy mapping-shaped provider suppression data to a list before source suppression is appended.
  • tests/hermes_cli/test_auth_commands.py: add a regression covering removal of a Codex credential from a legacy auth store, canonical persisted markers, and an empty resulting pool.

How to Test

  1. Create an auth.json with a pooled openai-codex credential from manual:device_code and legacy data such as "suppressed_sources": {"openai-codex": {"legacy": true}}.
  2. Run hermes auth remove openai-codex CREDENTIAL_ID with that store as HERMES_HOME.
  3. Confirm removal succeeds and persists suppressed_sources["openai-codex"] as a list containing the legacy key and Codex suppression markers.

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: macOS 13.7.8

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A: N/A; behavior is a backward-compatibility fix and the helper docstring documents the legacy shape.
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A: N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A: N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A: reviewed scripts/check-windows-footguns.py --diff origin/main; its 12 reports are pre-existing calls in the touched test file and the new file read declares UTF-8.
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A: N/A; no command syntax or schema changed.

Screenshots / Logs

Focused CI-parity tests passed:

scripts/run_tests.sh tests/hermes_cli/test_auth_commands.py tests/hermes_cli/test_auth_xai_oauth_provider.py -q
43 passed

Manual CLI proof passed with a temporary HERMES_HOME:

Removed openai-codex credential #1 (qb)
Suppressed openai-codex device_code source
suppressed_sources: ["legacy", "device_code", "manual:device_code"]

git diff --check origin/main...HEAD passed.

Full-suite blocker

The full suite was attempted in a fresh external Python 3.11.15 environment with .[all,dev] installed, then stopped after confirming two failures that reproduce unchanged on clean upstream main:

  • tests/agent/test_anthropic_output_field_leak.py: imports stale code from ~/.hermes/hermes-agent, which lacks get_process_hermes_home.
  • tests/agent/test_credential_pool_routing.py::TestFailureAttribution::test_unmatched_key_does_not_retry_only_pool_entry: expects recovered is False; clean upstream main returns True.

The full-suite checkbox remains unchecked. This PR is Draft until those unrelated upstream/environment failures have a project-approved disposition.

@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 sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Aug 6, 2026
@DanDo385
DanDo385 marked this pull request as draft August 7, 2026 18:23
@DanDo385
DanDo385 force-pushed the feat/oauth-profile-switching branch 3 times, most recently from 9a3db70 to 3e50f17 Compare August 8, 2026 00:58
@DanDo385
DanDo385 marked this pull request as ready for review August 8, 2026 16:28
@DanDo385
DanDo385 force-pushed the feat/oauth-profile-switching branch from c68b5fb to 8a85f5c Compare August 8, 2026 16:38
@DanDo385

Copy link
Copy Markdown
Author

Superseded by #83278. The replacement is rebased on current main, uses the correct fix/auth-legacy-codex-suppression branch name, and contains only the legacy suppression compatibility fix plus its regression test.

@DanDo385 DanDo385 closed this Aug 10, 2026
@DanDo385
DanDo385 deleted the feat/oauth-profile-switching branch August 25, 2026 23:52
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 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.

2 participants