common/hermes
2
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix: normalise Nous device-code pool source to avoid duplicates

Browse Source 
Review feedback on the original commit: the helper wrote a pool entry
with source `manual:device_code` while `_seed_from_singletons()` upserts
with `device_code` (no `manual:` prefix), so the pool grew a duplicate
row on every `load_pool()` after login.

Normalise: the helper now writes `providers.nous` and delegates the pool
write entirely to `_seed_from_singletons()` via a follow-up
`load_pool()` call. The canonical source is `device_code`; the helper
never materialises a parallel `manual:device_code` entry.

- `persist_nous_credentials()` loses its `label` and `source` kwargs —
  both are now derived by the seed path from the singleton state.
- CLI and web dashboard call sites simplified accordingly.
- New test `test_persist_nous_credentials_idempotent_no_duplicate_pool_entries`
  asserts that two consecutive persists leave exactly one pool row and
  no stray `manual:` entries.
- Existing `test_auth_add_nous_oauth_persists_pool_entry` updated to
  assert the canonical source and single-entry invariant.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Antoine Khater
2026-04-18 01:08:09 +00:00
committed by Teknium
parent c096a6935f
commit c7fece1f9d
5 changed files with 98 additions and 80 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
tests/hermes_cli/test_auth_commands.py
View File

@@ -141,10 +141,18 @@ def test_auth_add_nous_oauth_persists_pool_entry(tmp_path, monkeypatch):
auth_add_command(_Args())
payload = json.loads((tmp_path / "hermes" / "auth.json").read_text())
# Pool has exactly one canonical `device_code` entry — not a duplicate
# pair of `manual:device_code` + `device_code` (the latter would be
# materialised by _seed_from_singletons on every load_pool).
entries = payload["credential_pool"]["nous"]
entry = next(item for item in entries if item["source"] == "manual:device_code")
assert entry["label"] == "nous@example.com"
assert entry["source"] == "manual:device_code"
device_code_entries = [
item for item in entries if item["source"] == "device_code"
]
assert len(device_code_entries) == 1, entries
assert not any(item["source"] == "manual:device_code" for item in entries)
entry = device_code_entries[0]
assert entry["source"] == "device_code"
assert entry["agent_key"] == "ak-test"
assert entry["portal_base_url"] == "https://portal.example.com"

82
tests/hermes_cli/test_auth_nous_provider.py
View File

@@ -495,7 +495,7 @@ def test_persist_nous_credentials_writes_both_pool_and_providers(tmp_path, monke
agent failed with "Non-retryable client error". Both stores must stay
in sync at write time.
"""
from hermes_cli.auth import persist_nous_credentials
from hermes_cli.auth import persist_nous_credentials, NOUS_DEVICE_CODE_SOURCE
hermes_home = tmp_path / "hermes"
hermes_home.mkdir(parents=True, exist_ok=True)
@@ -504,38 +504,35 @@ def test_persist_nous_credentials_writes_both_pool_and_providers(tmp_path, monke
}))
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
creds = _full_state_fixture()
entry = persist_nous_credentials(
creds, label="test-label", source="manual:device_code",
)
entry = persist_nous_credentials(_full_state_fixture())
assert entry is not None
assert entry.provider == "nous"
assert entry.source == NOUS_DEVICE_CODE_SOURCE
payload = json.loads((hermes_home / "auth.json").read_text())
# providers.nous populated with the full state (new behavior)
# providers.nous populated with the full state (new behaviour)
singleton = payload["providers"]["nous"]
assert singleton["access_token"] == "access-tok"
assert singleton["refresh_token"] == "refresh-tok"
assert singleton["agent_key"] == "agent-key-value"
assert singleton["agent_key_expires_at"] == "2026-04-18T22:00:00+00:00"
# credential_pool.nous populated with the pool entry
# credential_pool.nous has exactly one canonical device_code entry
pool_entries = payload["credential_pool"]["nous"]
pool_entry = next(
item for item in pool_entries if item["source"] == "manual:device_code"
)
assert pool_entry["label"] == "test-label"
assert len(pool_entries) == 1, pool_entries
pool_entry = pool_entries[0]
assert pool_entry["source"] == NOUS_DEVICE_CODE_SOURCE
assert pool_entry["agent_key"] == "agent-key-value"
assert pool_entry["base_url"] == "https://inference.example.com/v1"
assert pool_entry["inference_base_url"] == "https://inference.example.com/v1"
def test_persist_nous_credentials_allows_recovery_from_401(tmp_path, monkeypatch):
"""End-to-end: after persisting via the helper, resolve_nous_runtime_credentials
must succeed (not raise "Hermes is not logged into Nous Portal").
This is the exact path that ran_agent.py's `_try_refresh_nous_client_credentials`
This is the exact path that run_agent.py's `_try_refresh_nous_client_credentials`
calls after a Nous 401 — before the fix it would raise AuthError because
providers.nous was empty.
"""
@@ -548,11 +545,7 @@ def test_persist_nous_credentials_allows_recovery_from_401(tmp_path, monkeypatch
}))
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
persist_nous_credentials(
_full_state_fixture(),
label="recovery-test",
source="manual:device_code",
)
persist_nous_credentials(_full_state_fixture())
# Stub the network-touching steps so we don't actually contact the
# portal — the point of this test is that state lookup succeeds and
@@ -575,9 +568,17 @@ def test_persist_nous_credentials_allows_recovery_from_401(tmp_path, monkeypatch
assert creds["api_key"] == "new-agent-key"
def test_persist_nous_credentials_preserves_existing_providers_entry(tmp_path, monkeypatch):
"""Calling persist twice must upsert providers.nous (not duplicate or crash)."""
from hermes_cli.auth import persist_nous_credentials
def test_persist_nous_credentials_idempotent_no_duplicate_pool_entries(tmp_path, monkeypatch):
"""Re-running persist must upsert — not accumulate duplicate device_code rows.
Regression guard for the review comment on PR #11858: before normalisation,
the helper wrote `manual:device_code` while `_seed_from_singletons` wrote
`device_code`, so the pool grew a second duplicate entry on every
``load_pool()``. The helper now writes providers.nous and lets seeding
materialise the pool entry under the canonical ``device_code`` source, so
two persists still leave the pool with exactly one row.
"""
from hermes_cli.auth import persist_nous_credentials, NOUS_DEVICE_CODE_SOURCE
hermes_home = tmp_path / "hermes"
hermes_home.mkdir(parents=True, exist_ok=True)
@@ -587,12 +588,12 @@ def test_persist_nous_credentials_preserves_existing_providers_entry(tmp_path, m
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
first = _full_state_fixture()
persist_nous_credentials(first, label="first", source="manual:device_code")
persist_nous_credentials(first)
second = _full_state_fixture()
second["access_token"] = "access-second"
second["agent_key"] = "agent-key-second"
persist_nous_credentials(second, label="second", source="manual:device_code")
persist_nous_credentials(second)
payload = json.loads((hermes_home / "auth.json").read_text())
@@ -600,8 +601,35 @@ def test_persist_nous_credentials_preserves_existing_providers_entry(tmp_path, m
assert payload["providers"]["nous"]["access_token"] == "access-second"
assert payload["providers"]["nous"]["agent_key"] == "agent-key-second"
# credential_pool.nous has both entries (pool = multi-credential)
# credential_pool.nous has exactly one entry, carrying the latest agent_key
pool_entries = payload["credential_pool"]["nous"]
labels = [e.get("label") for e in pool_entries]
assert "first" in labels
assert "second" in labels
assert len(pool_entries) == 1, pool_entries
assert pool_entries[0]["source"] == NOUS_DEVICE_CODE_SOURCE
assert pool_entries[0]["agent_key"] == "agent-key-second"
# And no stray `manual:device_code` / `manual:dashboard_device_code` rows
assert not any(
e["source"].startswith("manual:") for e in pool_entries
)
def test_persist_nous_credentials_reloads_pool_after_singleton_write(tmp_path, monkeypatch):
"""The entry returned by the helper must come from a fresh ``load_pool`` so
callers observe the canonical seeded state, including any legacy entries
that ``_seed_from_singletons`` pruned or upserted.
"""
from hermes_cli.auth import persist_nous_credentials, NOUS_DEVICE_CODE_SOURCE
hermes_home = tmp_path / "hermes"
hermes_home.mkdir(parents=True, exist_ok=True)
(hermes_home / "auth.json").write_text(json.dumps({
"version": 1, "providers": {},
}))
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
entry = persist_nous_credentials(_full_state_fixture())
assert entry is not None
assert entry.source == NOUS_DEVICE_CODE_SOURCE
# Label derived by _seed_from_singletons via label_from_token; we don't
# assert its exact value, just that the helper returned a real entry.
assert entry.access_token == "access-tok"
assert entry.agent_key == "agent-key-value"