From f268215019acf8c8b31c1d945767428c2bb80ca2 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Fri, 17 Apr 2026 04:10:17 -0700 Subject: [PATCH] fix(auth): codex auth remove no longer silently undone by auto-import (#11485) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 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 --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. --- agent/credential_pool.py | 13 ++ hermes_cli/auth.py | 22 +++ hermes_cli/auth_commands.py | 32 +++- tests/hermes_cli/test_auth_commands.py | 228 +++++++++++++++++++++++++ 4 files changed, 294 insertions(+), 1 deletion(-) diff --git a/agent/credential_pool.py b/agent/credential_pool.py index e1307e51..43a67a92 100644 --- a/agent/credential_pool.py +++ b/agent/credential_pool.py @@ -1208,6 +1208,19 @@ def _seed_from_singletons(provider: str, entries: List[PooledCredential]) -> Tup logger.debug("Qwen OAuth token seed failed: %s", exc) elif provider == "openai-codex": + # Respect user suppression — `hermes auth remove openai-codex` marks + # the device_code source as suppressed so it won't be re-seeded from + # either the Hermes auth store or ~/.codex/auth.json. Without this + # gate the removal is instantly undone on the next load_pool() call. + codex_suppressed = False + try: + from hermes_cli.auth import is_source_suppressed + codex_suppressed = is_source_suppressed(provider, "device_code") + except ImportError: + pass + if codex_suppressed: + return changed, active_sources + state = _load_provider_state(auth_store, "openai-codex") tokens = state.get("tokens") if isinstance(state, dict) else None # Fallback: import from Codex CLI (~/.codex/auth.json) if Hermes auth diff --git a/hermes_cli/auth.py b/hermes_cli/auth.py index 1763a8e6..e79a6dca 100644 --- a/hermes_cli/auth.py +++ b/hermes_cli/auth.py @@ -773,6 +773,28 @@ def is_source_suppressed(provider_id: str, source: str) -> bool: return False +def unsuppress_credential_source(provider_id: str, source: str) -> bool: + """Clear a suppression marker so the source will be re-seeded on the next load. + + Returns True if a marker was cleared, False if no marker existed. + """ + with _auth_store_lock(): + auth_store = _load_auth_store() + suppressed = auth_store.get("suppressed_sources") + if not isinstance(suppressed, dict): + return False + provider_list = suppressed.get(provider_id) + if not isinstance(provider_list, list) or source not in provider_list: + return False + provider_list.remove(source) + if not provider_list: + suppressed.pop(provider_id, None) + if not suppressed: + auth_store.pop("suppressed_sources", None) + _save_auth_store(auth_store) + return True + + def get_provider_auth_state(provider_id: str) -> Optional[Dict[str, Any]]: """Return persisted auth state for a provider, or None.""" auth_store = _load_auth_store() diff --git a/hermes_cli/auth_commands.py b/hermes_cli/auth_commands.py index d58a6a38..baca5c90 100644 --- a/hermes_cli/auth_commands.py +++ b/hermes_cli/auth_commands.py @@ -233,6 +233,9 @@ def auth_add_command(args) -> None: return if provider == "openai-codex": + # Clear any existing suppression marker so a re-link after `hermes auth + # remove openai-codex` works without the new tokens being skipped. + auth_mod.unsuppress_credential_source(provider, "device_code") creds = auth_mod._codex_device_code_login() label = (getattr(args, "label", None) or "").strip() or label_from_token( creds["tokens"]["access_token"], @@ -352,7 +355,34 @@ def auth_remove_command(args) -> None: # If this was a singleton-seeded credential (OAuth device_code, hermes_pkce), # clear the underlying auth store / credential file so it doesn't get # re-seeded on the next load_pool() call. - elif removed.source == "device_code" and provider in ("openai-codex", "nous"): + elif provider == "openai-codex" and ( + removed.source == "device_code" or removed.source.endswith(":device_code") + ): + # Codex tokens live in TWO places: the Hermes auth store and + # ~/.codex/auth.json (the Codex CLI shared file). On every refresh, + # refresh_codex_oauth_pure() writes to both. So clearing only the + # Hermes auth store is not enough — _seed_from_singletons() will + # auto-import from ~/.codex/auth.json on the next load_pool() and + # the removal is instantly undone. Mark the source as suppressed + # so auto-import is skipped; leave ~/.codex/auth.json untouched so + # the Codex CLI itself keeps working. + from hermes_cli.auth import ( + _load_auth_store, _save_auth_store, _auth_store_lock, + suppress_credential_source, + ) + with _auth_store_lock(): + auth_store = _load_auth_store() + providers_dict = auth_store.get("providers") + if isinstance(providers_dict, dict) and provider in providers_dict: + del providers_dict[provider] + _save_auth_store(auth_store) + print(f"Cleared {provider} OAuth tokens from auth store") + suppress_credential_source(provider, "device_code") + print("Suppressed openai-codex device_code source — it will not be re-seeded.") + print("Note: Codex CLI credentials still live in ~/.codex/auth.json") + print("Run `hermes auth add openai-codex` to re-enable if needed.") + + elif removed.source == "device_code" and provider == "nous": from hermes_cli.auth import ( _load_auth_store, _save_auth_store, _auth_store_lock, ) diff --git a/tests/hermes_cli/test_auth_commands.py b/tests/hermes_cli/test_auth_commands.py index b26757a2..a9db9059 100644 --- a/tests/hermes_cli/test_auth_commands.py +++ b/tests/hermes_cli/test_auth_commands.py @@ -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", {})