From 4437354198dcd282841da7ca60f9cdf3553965cc Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Sun, 5 Apr 2026 12:16:20 +0530 Subject: [PATCH] Preserve numeric credential labels in auth removal Resolve exact label matches before treating digit-only input as a positional index so destructive auth removal does not mis-target credentials named with numeric labels. Constraint: The CLI remove path must keep supporting existing index-based usage while adding safer label targeting Rejected: Ban numeric labels | labels are free-form and existing users may already rely on them Confidence: high Scope-risk: narrow Reversibility: clean Directive: When a destructive command accepts multiple identifier forms, prefer exact identity matches before fallback parsing heuristics Tested: Focused pytest slice for auth commands, credential pool recovery, and routing (273 passed); py_compile on changed Python files Not-tested: Full repository pytest suite --- agent/credential_pool.py | 10 ++++---- tests/test_auth_commands.py | 50 +++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/agent/credential_pool.py b/agent/credential_pool.py index cfdf9b2ac..311abea98 100644 --- a/agent/credential_pool.py +++ b/agent/credential_pool.py @@ -760,11 +760,6 @@ class CredentialPool: raw = str(target or "").strip() if not raw: return None, None, "No credential target provided." - if raw.isdigit(): - index = int(raw) - if 1 <= index <= len(self._entries): - return index, self._entries[index - 1], None - return None, None, f"No credential #{index}." for idx, entry in enumerate(self._entries, start=1): if entry.id == raw: @@ -779,6 +774,11 @@ class CredentialPool: return label_matches[0][0], label_matches[0][1], None if len(label_matches) > 1: return None, None, f'Ambiguous credential label "{raw}". Use the numeric index or entry id instead.' + if raw.isdigit(): + index = int(raw) + if 1 <= index <= len(self._entries): + return index, self._entries[index - 1], None + return None, None, f"No credential #{index}." return None, None, f'No credential matching "{raw}".' def add_entry(self, entry: PooledCredential) -> PooledCredential: diff --git a/tests/test_auth_commands.py b/tests/test_auth_commands.py index bd40cb885..a89bd4081 100644 --- a/tests/test_auth_commands.py +++ b/tests/test_auth_commands.py @@ -279,6 +279,56 @@ def test_auth_remove_accepts_label_target(tmp_path, monkeypatch): assert entries[0]["label"] == "work-account" +def test_auth_remove_prefers_exact_numeric_label_over_index(tmp_path, monkeypatch): + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes")) + _write_auth_store( + tmp_path, + { + "version": 1, + "credential_pool": { + "openai-codex": [ + { + "id": "cred-a", + "label": "first", + "auth_type": "oauth", + "priority": 0, + "source": "manual:device_code", + "access_token": "tok-a", + }, + { + "id": "cred-b", + "label": "2", + "auth_type": "oauth", + "priority": 1, + "source": "manual:device_code", + "access_token": "tok-b", + }, + { + "id": "cred-c", + "label": "third", + "auth_type": "oauth", + "priority": 2, + "source": "manual:device_code", + "access_token": "tok-c", + }, + ] + }, + }, + ) + + from hermes_cli.auth_commands import auth_remove_command + + class _Args: + provider = "openai-codex" + target = "2" + + auth_remove_command(_Args()) + + payload = json.loads((tmp_path / "hermes" / "auth.json").read_text()) + labels = [entry["label"] for entry in payload["credential_pool"]["openai-codex"]] + assert labels == ["first", "third"] + + def test_auth_reset_clears_provider_statuses(tmp_path, monkeypatch, capsys): monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes")) _write_auth_store(