fix(auth): codex auth remove no longer silently undone by auto-import (#11485)

* feat(skills): add 'hermes skills reset' to un-stick bundled skills

When a user edits a bundled skill, sync flags it as user_modified and
skips it forever. The problem: if the user later tries to undo the edit
by copying the current bundled version back into ~/.hermes/skills/, the
manifest still holds the old origin hash from the last successful
sync, so the fresh bundled hash still doesn't match and the skill stays
stuck as user_modified.

Adds an escape hatch for this case.

  hermes skills reset <name>
      Drops the skill's entry from ~/.hermes/skills/.bundled_manifest and
      re-baselines against the user's current copy. Future 'hermes update'
      runs accept upstream changes again. Non-destructive.

  hermes skills reset <name> --restore
      Also deletes the user's copy and re-copies the bundled version.
      Use when you want the pristine upstream skill back.

Also available as /skills reset in chat.

- tools/skills_sync.py: new reset_bundled_skill(name, restore=False)
- hermes_cli/skills_hub.py: do_reset() + wired into skills_command and
  handle_skills_slash; added to the slash /skills help panel
- hermes_cli/main.py: argparse entry for 'hermes skills reset'
- tests/tools/test_skills_sync.py: 5 new tests covering the stuck-flag
  repro, --restore, unknown-skill error, upstream-removed-skill, and
  no-op on already-clean state
- website/docs/user-guide/features/skills.md: new 'Bundled skill updates'
  section explaining the origin-hash mechanic + reset usage

* fix(auth): codex auth remove no longer silently undone by auto-import

'hermes auth remove openai-codex' appeared to succeed but the credential
reappeared on the next command.  Two compounding bugs:

1. _seed_from_singletons() for openai-codex unconditionally re-imports
   tokens from ~/.codex/auth.json whenever the Hermes auth store is
   empty (by design — the Codex CLI and Hermes share that file).  There
   was no suppression check, unlike the claude_code seed path.

2. auth_remove_command's cleanup branch only matched
   removed.source == 'device_code' exactly.  Entries added via
   'hermes auth add openai-codex' have source 'manual:device_code', so
   for those the Hermes auth store's providers['openai-codex'] state was
   never cleared on remove — the next load_pool() re-seeded straight
   from there.

Net effect: there was no way to make a codex removal stick short of
manually editing both ~/.hermes/auth.json and ~/.codex/auth.json before
opening Hermes again.

Fix:

- Add unsuppress_credential_source() helper (mirrors
  suppress_credential_source()).
- Gate the openai-codex branch in _seed_from_singletons() with
  is_source_suppressed(), matching the claude_code pattern.
- Broaden auth_remove_command's codex match to handle both
  'device_code' and 'manual:device_code' (via endswith check), always
  call suppress_credential_source(), and print guidance about the
  unchanged ~/.codex/auth.json file.
- Clear the suppression marker in auth_add_command's openai-codex
  branch so re-linking via 'hermes auth add openai-codex' works.

~/.codex/auth.json is left untouched — that's the Codex CLI's own
credential store, not ours to delete.

Tests cover: unsuppress helper behavior, remove of both source
variants, add clears suppression, seed respects suppression.  E2E
verified: remove → load → add → load flow now behaves correctly.
This commit is contained in:
Teknium 2026-04-17 04:10:17 -07:00 committed by GitHub
parent 8b312248dc
commit f268215019
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 294 additions and 1 deletions

View file

@ -703,3 +703,231 @@ def test_auth_remove_claude_code_suppresses_reseed(tmp_path, monkeypatch):
suppressed = updated.get("suppressed_sources", {})
assert "anthropic" in suppressed
assert "claude_code" in suppressed["anthropic"]
def test_unsuppress_credential_source_clears_marker(tmp_path, monkeypatch):
"""unsuppress_credential_source() removes a previously-set marker."""
monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes"))
_write_auth_store(tmp_path, {"version": 1})
from hermes_cli.auth import suppress_credential_source, unsuppress_credential_source, is_source_suppressed
suppress_credential_source("openai-codex", "device_code")
assert is_source_suppressed("openai-codex", "device_code") is True
cleared = unsuppress_credential_source("openai-codex", "device_code")
assert cleared is True
assert is_source_suppressed("openai-codex", "device_code") is False
payload = json.loads((tmp_path / "hermes" / "auth.json").read_text())
# Empty suppressed_sources dict should be cleaned up entirely
assert "suppressed_sources" not in payload
def test_unsuppress_credential_source_returns_false_when_absent(tmp_path, monkeypatch):
"""unsuppress_credential_source() returns False if no marker exists."""
monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes"))
_write_auth_store(tmp_path, {"version": 1})
from hermes_cli.auth import unsuppress_credential_source
assert unsuppress_credential_source("openai-codex", "device_code") is False
assert unsuppress_credential_source("nonexistent", "whatever") is False
def test_unsuppress_credential_source_preserves_other_markers(tmp_path, monkeypatch):
"""Clearing one marker must not affect unrelated markers."""
monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes"))
_write_auth_store(tmp_path, {"version": 1})
from hermes_cli.auth import (
suppress_credential_source,
unsuppress_credential_source,
is_source_suppressed,
)
suppress_credential_source("openai-codex", "device_code")
suppress_credential_source("anthropic", "claude_code")
assert unsuppress_credential_source("openai-codex", "device_code") is True
assert is_source_suppressed("anthropic", "claude_code") is True
def test_auth_remove_codex_device_code_suppresses_reseed(tmp_path, monkeypatch):
"""Removing an auto-seeded openai-codex credential must mark the source as suppressed."""
monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes"))
monkeypatch.setattr(
"agent.credential_pool._seed_from_singletons",
lambda provider, entries: (False, {"device_code"}),
)
hermes_home = tmp_path / "hermes"
hermes_home.mkdir(parents=True, exist_ok=True)
auth_store = {
"version": 1,
"providers": {
"openai-codex": {
"tokens": {
"access_token": "acc-1",
"refresh_token": "ref-1",
},
},
},
"credential_pool": {
"openai-codex": [{
"id": "cx1",
"label": "codex-auto",
"auth_type": "oauth",
"priority": 0,
"source": "device_code",
"access_token": "acc-1",
"refresh_token": "ref-1",
}]
},
}
(hermes_home / "auth.json").write_text(json.dumps(auth_store))
from types import SimpleNamespace
from hermes_cli.auth_commands import auth_remove_command
auth_remove_command(SimpleNamespace(provider="openai-codex", target="1"))
updated = json.loads((hermes_home / "auth.json").read_text())
suppressed = updated.get("suppressed_sources", {})
assert "openai-codex" in suppressed
assert "device_code" in suppressed["openai-codex"]
# Tokens in providers state should also be cleared
assert "openai-codex" not in updated.get("providers", {})
def test_auth_remove_codex_manual_source_suppresses_reseed(tmp_path, monkeypatch):
"""Removing a manually-added (`manual:device_code`) openai-codex credential must also suppress."""
monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes"))
monkeypatch.setattr(
"agent.credential_pool._seed_from_singletons",
lambda provider, entries: (False, set()),
)
hermes_home = tmp_path / "hermes"
hermes_home.mkdir(parents=True, exist_ok=True)
auth_store = {
"version": 1,
"providers": {
"openai-codex": {
"tokens": {
"access_token": "acc-2",
"refresh_token": "ref-2",
},
},
},
"credential_pool": {
"openai-codex": [{
"id": "cx2",
"label": "manual-codex",
"auth_type": "oauth",
"priority": 0,
"source": "manual:device_code",
"access_token": "acc-2",
"refresh_token": "ref-2",
}]
},
}
(hermes_home / "auth.json").write_text(json.dumps(auth_store))
from types import SimpleNamespace
from hermes_cli.auth_commands import auth_remove_command
auth_remove_command(SimpleNamespace(provider="openai-codex", target="1"))
updated = json.loads((hermes_home / "auth.json").read_text())
suppressed = updated.get("suppressed_sources", {})
# Critical: manual:device_code source must also trigger the suppression path
assert "openai-codex" in suppressed
assert "device_code" in suppressed["openai-codex"]
assert "openai-codex" not in updated.get("providers", {})
def test_auth_add_codex_clears_suppression_marker(tmp_path, monkeypatch):
"""Re-linking codex via `hermes auth add openai-codex` must clear any suppression marker."""
monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes"))
monkeypatch.setattr(
"agent.credential_pool._seed_from_singletons",
lambda provider, entries: (False, set()),
)
hermes_home = tmp_path / "hermes"
hermes_home.mkdir(parents=True, exist_ok=True)
# Pre-existing suppression (simulating a prior `hermes auth remove`)
(hermes_home / "auth.json").write_text(json.dumps({
"version": 1,
"providers": {},
"suppressed_sources": {"openai-codex": ["device_code"]},
}))
token = _jwt_with_email("codex@example.com")
monkeypatch.setattr(
"hermes_cli.auth._codex_device_code_login",
lambda: {
"tokens": {
"access_token": token,
"refresh_token": "refreshed",
},
"base_url": "https://chatgpt.com/backend-api/codex",
"last_refresh": "2026-01-01T00:00:00Z",
},
)
from hermes_cli.auth_commands import auth_add_command
class _Args:
provider = "openai-codex"
auth_type = "oauth"
api_key = None
label = None
auth_add_command(_Args())
payload = json.loads((hermes_home / "auth.json").read_text())
# Suppression marker must be cleared
assert "openai-codex" not in payload.get("suppressed_sources", {})
# New pool entry must be present
entries = payload["credential_pool"]["openai-codex"]
assert any(e["source"] == "manual:device_code" for e in entries)
def test_seed_from_singletons_respects_codex_suppression(tmp_path, monkeypatch):
"""_seed_from_singletons() for openai-codex must skip auto-import when suppressed."""
monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes"))
hermes_home = tmp_path / "hermes"
hermes_home.mkdir(parents=True, exist_ok=True)
# Suppression marker in place
(hermes_home / "auth.json").write_text(json.dumps({
"version": 1,
"providers": {},
"suppressed_sources": {"openai-codex": ["device_code"]},
}))
# Make _import_codex_cli_tokens return tokens — these would normally trigger
# a re-seed, but suppression must skip it.
def _fake_import():
return {
"access_token": "would-be-reimported",
"refresh_token": "would-be-reimported",
}
monkeypatch.setattr("hermes_cli.auth._import_codex_cli_tokens", _fake_import)
from agent.credential_pool import _seed_from_singletons
entries = []
changed, active_sources = _seed_from_singletons("openai-codex", entries)
# With suppression in place: nothing changes, no entries added, no sources
assert changed is False
assert entries == []
assert active_sources == set()
# Verify the auth store was NOT modified (no auto-import happened)
after = json.loads((hermes_home / "auth.json").read_text())
assert "openai-codex" not in after.get("providers", {})