fix(agent): adopt .env credential/base-url edits at the turn boundary (#67843)

* fix(agent): adopt .env credential/base-url edits at the turn boundary

A Settings save (desktop PUT /api/env, hermes setup) updates .env and
the saving process's os.environ, but a live session worker keeps the
base_url/api_key captured at agent init until restart — an open chat
silently kept calling the old endpoint (e.g. a local-server key sent to
api.openai.com, failing with an opaque 401).

Add AIAgent._try_refresh_env_client_credentials(), called at the start
of each conversation turn: re-resolve the provider's env-sourced
credentials (load_env() is mtime-memoized, so an unchanged file costs
one stat()) and rebuild the client via the existing
_replace_primary_openai_client machinery when the user edited them.

The refresh reacts only to env edits — resolved values changed since
the last look — never to mere divergence from the agent's current
values: credential-pool rotation and failover legitimately move the
session off the env credential, and stomping those back would flap.
Config model.base_url / pool custom endpoints keep precedence: edits
are only adopted while the session still runs on the registry default
or the previously-seen env value.

Lift _get_env_prefer_dotenv out of _seed_from_env to module level
(get_env_prefer_dotenv) so both the pool seeder and the per-turn
refresh share the same .env-over-os.environ resolution, including the
op:// indirection handling.

Fixes #67821

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(agent): address sweeper review on env credential refresh

- Cover named custom providers (#67935): provider="custom" has no
  PROVIDER_REGISTRY entry, so resolve the config block's key_env through
  the same lookup the runtime resolver uses.
- Make the edit baseline transactional: a failed client rebuild rolls the
  agent back and leaves _env_creds_seen un-advanced so the unchanged edit
  is retried next turn.
- Recompute route-derived TLS material and default headers on a base-url
  change, via a _reapply_route_client_config helper shared with
  credential-pool rotation so the two paths cannot drift.
- Rebase onto main: get_env_prefer_dotenv keeps the scoped _get_secret
  semantics from the profile-isolation fix (no raw os.environ reads).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* chore: map jskang@lablup.com to rapsealk

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Brooklyn Nicholson <brooklyn.bb.nicholson@gmail.com>
This commit is contained in:
Jeongseok Kang 2026-08-05 02:53:17 +09:00 committed by GitHub
parent 6acae30751
commit 2d70f56327
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 483 additions and 24 deletions

View File

@ -1291,6 +1291,14 @@ def run_conversation(
agent._last_compression_attempt_recorded = False
agent._last_compression_attempt_in_place = None
# Adopt any ~/.hermes/.env credential/base-url edits made since the last
# turn — a Settings save updates .env but not this worker's client, which
# was built at agent init (#67821). No-op when .env is unchanged.
try:
agent._try_refresh_env_client_credentials()
except Exception:
logger.debug("per-turn env credential refresh failed", exc_info=True)
# ── Per-turn setup (the prologue) ──
# All once-per-turn setup — stdio guarding, retry-counter resets, user
# message sanitization, todo/nudge hydration, system-prompt restore-or-

View File

@ -2742,6 +2742,31 @@ def _seed_from_singletons(provider: str, entries: List[PooledCredential]) -> Tup
return changed, active_sources
# Prefer ~/.hermes/.env over os.environ — the user's config file is the
# authoritative source for Hermes credentials. Stale env vars from parent
# processes (Codex CLI, test scripts, etc.) should not override deliberate
# changes to the .env file. load_env() memoizes on the .env mtime, so
# per-call reads (pool seeding, per-turn credential refresh) cost a stat()
# when the file is unchanged.
def get_env_prefer_dotenv(key: str) -> str:
env_file = load_env()
raw = env_file.get(key, "").strip()
scoped_value = (_get_secret(key, "") or "").strip()
# If .env contains an unresolved op:// reference, prefer the
# already-resolved value supplied by the active secret scope (or by
# os.environ in legacy single-profile mode), set by
# load_hermes_dotenv() -> apply_onepassword_secrets()). The raw
# "op://Vault/Item/field" string would otherwise win and every
# provider auth attempt would receive a URL instead of a key. This
# happens during a partial migration, or when the user wrote op://
# references straight into .env rather than the secrets.onepassword
# config block. For every non-op:// value the original
# .env-takes-precedence behaviour is preserved unchanged.
if raw.startswith("op://") and scoped_value:
return scoped_value
return raw or scoped_value
def _seed_from_env(provider: str, entries: List[PooledCredential]) -> Tuple[bool, Set[str]]:
changed = False
active_sources: Set[str] = set()
@ -2760,27 +2785,10 @@ def _seed_from_env(provider: str, entries: List[PooledCredential]) -> Tuple[bool
if provider == "copilot":
return False, active_sources
# Prefer ~/.hermes/.env over os.environ — the user's config file is the
# authoritative source for Hermes credentials. Stale env vars from parent
# processes (Codex CLI, test scripts, etc.) should not override deliberate
# changes to the .env file.
def _get_env_prefer_dotenv(key: str) -> str:
env_file = load_env()
raw = env_file.get(key, "").strip()
scoped_value = (_get_secret(key, "") or "").strip()
# If .env contains an unresolved op:// reference, prefer the
# already-resolved value supplied by the active secret scope (or by
# os.environ in legacy single-profile mode), set by
# load_hermes_dotenv() -> apply_onepassword_secrets()). The raw
# "op://Vault/Item/field" string would otherwise win and every
# provider auth attempt would receive a URL instead of a key. This
# happens during a partial migration, or when the user wrote op://
# references straight into .env rather than the secrets.onepassword
# config block. For every non-op:// value the original
# .env-takes-precedence behaviour is preserved unchanged.
if raw.startswith("op://") and scoped_value:
return scoped_value
return raw or scoped_value
# The .env-preferring resolution lives at module level
# (``get_env_prefer_dotenv``) so the pool seeder and the per-turn
# credential refresh share one implementation.
_get_env_prefer_dotenv = get_env_prefer_dotenv
# Honour user suppression — `hermes auth remove <provider> <N>` for an
# env-seeded credential marks the env:<VAR> source as suppressed so it

View File

@ -0,0 +1 @@
rapsealk

View File

@ -5390,6 +5390,168 @@ class AIAgent:
return True
def _try_refresh_env_client_credentials(self) -> bool:
"""Adopt ~/.hermes/.env credential/base-url edits at the turn boundary.
A Settings save (desktop ``PUT /api/env``, ``hermes setup``) updates
``.env`` and the *saving* process's os.environ, but a live session
worker keeps the base_url/api_key captured at agent init until it
restarts — so an open chat silently keeps calling the old endpoint
(#67821). Called at the start of each conversation turn, this
re-resolves the provider's env-sourced credentials (load_env() is
mtime-memoized, so an unchanged file costs one stat()) and rebuilds
the client when the user edited them.
Reacts only to env *edits* (resolved values changed since the last
look), never to mere divergence from the agent's current values —
credential-pool rotation and failover legitimately move the session
off the env credential, and stomping those back every turn would
flap. A config.yaml ``model.base_url`` (or a pool entry with a
custom endpoint) also wins: edits are only adopted while the
session's current base_url is still the registry default or the
previously-seen env value.
Covers api-key registry providers and named custom providers with a
``key_env`` (#67935) — the latter resolve to ``provider="custom"``
with no registry entry, so they are matched through the runtime
provider's config lookup instead.
"""
if self.api_mode != "chat_completions":
return False
if getattr(self, "_fallback_activated", False):
return False
try:
from agent.credential_pool import get_env_prefer_dotenv
from hermes_cli.auth import PROVIDER_REGISTRY
except ImportError:
return False
pconfig = PROVIDER_REGISTRY.get(self.provider)
if (
pconfig
and getattr(pconfig, "auth_type", "") == "api_key"
and getattr(pconfig, "api_key_env_vars", ())
):
api_key = ""
for env_var in pconfig.api_key_env_vars:
api_key = get_env_prefer_dotenv(env_var).strip()
if api_key:
break
if not api_key:
return False
env_url = ""
if pconfig.base_url_env_var:
env_url = get_env_prefer_dotenv(pconfig.base_url_env_var).strip().rstrip("/")
default_base = (pconfig.inference_base_url or "").strip().rstrip("/")
base_url = env_url or default_base
if self.provider == "kimi-coding":
from hermes_cli.auth import _resolve_kimi_base_url
base_url = _resolve_kimi_base_url(
api_key, pconfig.inference_base_url, env_url
).rstrip("/")
elif self.provider == "zai":
from hermes_cli.auth import _resolve_zai_base_url
base_url = _resolve_zai_base_url(
api_key, pconfig.inference_base_url, env_url
).rstrip("/")
elif self.provider == "custom":
# Named custom provider (#67935): identity lives in config
# (``providers.<name>`` / ``custom_providers``), the credential in
# the env var it names via ``key_env``. Re-resolve through the
# same config lookup the runtime resolver uses; entries without
# ``key_env`` (inline ``api_key``, pool-backed) have no
# env-sourced credential to watch.
try:
from hermes_cli.runtime_provider import _get_named_custom_provider
except ImportError:
return False
custom_provider = _get_named_custom_provider(
getattr(self, "requested_provider", "") or ""
)
if not custom_provider:
return False
key_env = str(custom_provider.get("key_env") or "").strip()
if not key_env:
return False
api_key = get_env_prefer_dotenv(key_env).strip()
if not api_key:
return False
# Custom providers pin their endpoint in config, not env — the
# config base_url is both the resolved and the "default" base, so
# only key edits are ever adopted here.
default_base = str(custom_provider.get("base_url") or "").strip().rstrip("/")
base_url = default_base
else:
return False
if not base_url:
return False
resolved = (base_url, api_key)
prev = getattr(self, "_env_creds_seen", None)
current_base = (self.base_url or "").strip().rstrip("/")
if prev is None:
# First look — no baseline to diff against. Adopt only the
# boot-default case (worker spawned before the user saved an
# override); anything else is unattributable on turn one.
adopt = current_base == default_base and not (
base_url == current_base and api_key == self.api_key
)
else:
# Env unchanged → no-op; any drift from self.* is rotation/
# failover or config precedence — leave it alone. An edit is
# only adopted while the session still runs on the registry
# default or the previously-seen env value.
adopt = (
resolved != prev
and current_base in {default_base, prev[0]}
and not (base_url == current_base and api_key == self.api_key)
)
if not adopt:
self._env_creds_seen = resolved
return False
from hermes_cli.route_identity import normalize_route_base_url
route_changed = normalize_route_base_url(self.base_url) != normalize_route_base_url(
base_url
)
prior_api_key = self.api_key
prior_base_url = self.base_url
prior_client_kwargs = dict(self._client_kwargs)
self.api_key = api_key
self.base_url = base_url
self._client_kwargs["api_key"] = self.api_key
self._client_kwargs["base_url"] = self.base_url
# A base-url change moves the route: TLS material and default
# headers derived from the old endpoint must be recomputed, exactly
# as on credential-pool rotation.
self._reapply_route_client_config(route_changed=route_changed)
if not self._replace_primary_openai_client(reason="env_credential_refresh"):
# Leave the baseline un-advanced so the unchanged edit is
# retried next turn, and roll the agent back so its state keeps
# matching the still-live old client.
self.api_key = prior_api_key
self.base_url = prior_base_url
self._client_kwargs.clear()
self._client_kwargs.update(prior_client_kwargs)
return False
self._env_creds_seen = resolved
logger.info(
"Applied updated .env credentials for %s: endpoint %s",
self.provider,
self.base_url,
)
return True
def _try_refresh_vertex_client_credentials(self) -> bool:
"""Re-mint the Vertex OAuth2 access token and rebuild the OpenAI client.
@ -5743,6 +5905,19 @@ class AIAgent:
self.base_url = runtime_base.rstrip("/") if isinstance(runtime_base, str) else runtime_base
self._client_kwargs["api_key"] = self.api_key
self._client_kwargs["base_url"] = self.base_url
self._reapply_route_client_config(route_changed=route_changed)
self._replace_primary_openai_client(reason="credential_rotation")
def _reapply_route_client_config(self, *, route_changed: bool) -> None:
"""Recompute route-derived client kwargs for the current ``self.base_url``.
TLS material (``ssl_verify``/``ssl_ca_cert``) and default headers are
derived from the endpoint, not the credential — any client rebuild
that may have moved ``base_url`` must recompute them or the new
endpoint inherits configuration computed for the old one. Shared by
credential-pool rotation and the per-turn env refresh so the two
paths cannot drift.
"""
self._client_kwargs.pop("ssl_verify", None)
self._client_kwargs.pop("ssl_ca_cert", None)
try:
@ -5766,7 +5941,6 @@ class AIAgent:
self.base_url,
apply_user_headers=not route_changed,
)
self._replace_primary_openai_client(reason="credential_rotation")
def _recover_with_credential_pool(
self,

View File

@ -22,6 +22,10 @@ def test_credential_rotation_replaces_route_scoped_tls_settings():
_apply_client_headers_for_base_url=MagicMock(),
_replace_primary_openai_client=MagicMock(),
)
agent._reapply_route_client_config = MethodType(
AIAgent._reapply_route_client_config,
agent,
)
entry = SimpleNamespace(
runtime_api_key="new",
access_token="",
@ -70,6 +74,10 @@ def test_credential_rotation_does_not_carry_global_headers_across_routes():
AIAgent._apply_user_default_headers,
agent,
)
agent._reapply_route_client_config = MethodType(
AIAgent._reapply_route_client_config,
agent,
)
entry = SimpleNamespace(
runtime_api_key="new",
access_token="",
@ -101,5 +109,3 @@ def test_credential_rotation_does_not_carry_global_headers_across_routes():
headers = agent._client_kwargs["default_headers"]
assert "Authorization" not in headers
assert headers["X-Route"] == "b"

View File

@ -0,0 +1,262 @@
"""Per-turn adoption of ~/.hermes/.env credential edits (#67821).
A Settings save (desktop ``PUT /api/env``, ``hermes setup``) updates .env and
the saving process's os.environ, but a live session worker keeps the
base_url/api_key captured at agent init until restart — an open chat silently
kept calling the old endpoint (e.g. a local-server key sent to
api.openai.com → opaque 401).
``AIAgent._try_refresh_env_client_credentials`` re-resolves env-sourced
credentials at the start of each conversation turn and rebuilds the client
when the user edited them. It must react only to env *edits*, never to mere
divergence from the agent's current values: credential-pool rotation and
failover legitimately move the session off the env credential, and config
``model.base_url`` has higher precedence than the env override.
"""
import os
import sys
from unittest.mock import MagicMock
import pytest
sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", ".."))
from run_agent import AIAgent
DEFAULT_BASE = "https://api.openai.com/v1"
LOCAL_BASE = "http://127.0.0.1:39080"
def _make_agent(*, provider="openai-api", base_url=DEFAULT_BASE, api_key="sk-old"):
agent = object.__new__(AIAgent)
agent.provider = provider
agent.requested_provider = provider
agent.api_mode = "chat_completions"
agent.base_url = base_url
agent.api_key = api_key
agent._client_kwargs = {"base_url": base_url, "api_key": api_key}
agent._fallback_activated = False
agent._replace_primary_openai_client = MagicMock(return_value=True)
agent._reapply_route_client_config = MagicMock()
return agent
@pytest.fixture
def env(monkeypatch):
"""Dict-driven stand-in for the .env/os.environ resolution chain."""
values = {}
import agent.credential_pool as cp
monkeypatch.setattr(
cp, "get_env_prefer_dotenv", lambda key: values.get(key, "")
)
return values
class TestAdoptsEnvEdits:
def test_boot_default_adopts_override_on_first_look(self, env):
"""The reported scenario: worker spawned before the user saved the
override — first turn after the save must switch to the local URL."""
agent = _make_agent()
env["OPENAI_API_KEY"] = "sk-old"
env["OPENAI_BASE_URL"] = LOCAL_BASE
assert agent._try_refresh_env_client_credentials() is True
assert agent.base_url == LOCAL_BASE
assert agent._client_kwargs["base_url"] == LOCAL_BASE
agent._replace_primary_openai_client.assert_called_once_with(
reason="env_credential_refresh"
)
def test_edit_between_turns_is_adopted(self, env):
"""No-op first turn, then the user saves an override → next turn
rebuilds the client onto the new endpoint."""
agent = _make_agent()
env["OPENAI_API_KEY"] = "sk-old"
assert agent._try_refresh_env_client_credentials() is False
env["OPENAI_BASE_URL"] = LOCAL_BASE
assert agent._try_refresh_env_client_credentials() is True
assert agent.base_url == LOCAL_BASE
def test_key_rotation_in_env_is_adopted(self, env):
agent = _make_agent()
env["OPENAI_API_KEY"] = "sk-old"
assert agent._try_refresh_env_client_credentials() is False
env["OPENAI_API_KEY"] = "sk-new"
assert agent._try_refresh_env_client_credentials() is True
assert agent.api_key == "sk-new"
assert agent._client_kwargs["api_key"] == "sk-new"
class TestLeavesNonEnvStateAlone:
def test_unchanged_env_is_a_noop(self, env):
agent = _make_agent()
env["OPENAI_API_KEY"] = "sk-old"
assert agent._try_refresh_env_client_credentials() is False
agent._replace_primary_openai_client.assert_not_called()
def test_pool_rotation_is_not_stomped(self, env):
"""After the pool rotates the session onto a different key, an
unchanged env must not flap the session back every turn."""
agent = _make_agent()
env["OPENAI_API_KEY"] = "sk-old"
assert agent._try_refresh_env_client_credentials() is False
agent.api_key = "sk-rotated-pool-entry"
assert agent._try_refresh_env_client_credentials() is False
assert agent.api_key == "sk-rotated-pool-entry"
def test_custom_endpoint_wins_over_env_edit(self, env):
"""A session running on a config/pool custom endpoint (not the
registry default, not a previously-seen env value) keeps it."""
agent = _make_agent(base_url="https://my-proxy.corp.example/v1")
env["OPENAI_API_KEY"] = "sk-old"
env["OPENAI_BASE_URL"] = LOCAL_BASE
assert agent._try_refresh_env_client_credentials() is False
assert agent.base_url == "https://my-proxy.corp.example/v1"
def test_skipped_while_failed_over(self, env):
agent = _make_agent()
agent._fallback_activated = True
env["OPENAI_API_KEY"] = "sk-old"
env["OPENAI_BASE_URL"] = LOCAL_BASE
assert agent._try_refresh_env_client_credentials() is False
def test_skipped_for_non_api_key_provider(self, env):
agent = _make_agent(provider="openai-codex")
assert agent._try_refresh_env_client_credentials() is False
def test_skipped_for_non_chat_completions_api_mode(self, env):
agent = _make_agent()
agent.api_mode = "anthropic_messages"
assert agent._try_refresh_env_client_credentials() is False
def test_skipped_when_no_key_resolves(self, env):
agent = _make_agent()
env["OPENAI_BASE_URL"] = LOCAL_BASE
assert agent._try_refresh_env_client_credentials() is False
class TestFailedRebuildRetries:
def test_failed_rebuild_rolls_back_and_retries_next_turn(self, env):
"""A failed client rebuild must not advance the edit baseline: the
agent rolls back to the still-live old client's state and the same
unchanged edit is retried on the next turn."""
agent = _make_agent()
env["OPENAI_API_KEY"] = "sk-old"
assert agent._try_refresh_env_client_credentials() is False
env["OPENAI_BASE_URL"] = LOCAL_BASE
agent._replace_primary_openai_client.return_value = False
assert agent._try_refresh_env_client_credentials() is False
# Rolled back: agent state still matches the old client.
assert agent.base_url == DEFAULT_BASE
assert agent.api_key == "sk-old"
assert agent._client_kwargs == {"base_url": DEFAULT_BASE, "api_key": "sk-old"}
agent._replace_primary_openai_client.return_value = True
assert agent._try_refresh_env_client_credentials() is True
assert agent.base_url == LOCAL_BASE
assert agent._client_kwargs["base_url"] == LOCAL_BASE
class TestRouteConfigRefresh:
def test_base_url_change_recomputes_route_tls_and_headers(self, env):
"""Moving to a new endpoint must recompute route-derived TLS material
and default headers, exactly as credential-pool rotation does."""
agent = _make_agent()
env["OPENAI_API_KEY"] = "sk-old"
env["OPENAI_BASE_URL"] = LOCAL_BASE
assert agent._try_refresh_env_client_credentials() is True
agent._reapply_route_client_config.assert_called_once_with(route_changed=True)
def test_key_only_change_keeps_route_config(self, env):
agent = _make_agent()
env["OPENAI_API_KEY"] = "sk-old"
assert agent._try_refresh_env_client_credentials() is False
env["OPENAI_API_KEY"] = "sk-new"
assert agent._try_refresh_env_client_credentials() is True
agent._reapply_route_client_config.assert_called_once_with(route_changed=False)
CUSTOM_BASE = "https://api.longcat.example/openai/v1"
@pytest.fixture
def named_custom_provider(monkeypatch):
"""Register a named custom provider (config `providers.longcat` block)."""
block = {"name": "longcat", "base_url": CUSTOM_BASE, "key_env": "LONGCAT_API_KEY"}
import hermes_cli.runtime_provider as rp
monkeypatch.setattr(
rp,
"_get_named_custom_provider",
lambda requested: block if requested == "longcat" else None,
)
return block
class TestNamedCustomProviders:
"""#67935: named custom providers resolve to provider="custom" with no
PROVIDER_REGISTRY entry — their `key_env` credential must refresh too."""
def _make_custom_agent(self, *, api_key="no-key-required"):
agent = _make_agent(provider="custom", base_url=CUSTOM_BASE, api_key=api_key)
agent.requested_provider = "longcat"
return agent
def test_key_added_mid_session_is_adopted(self, env, named_custom_provider):
"""The #67935 repro: long-lived worker spawned before the key_env var
was written to .env — the first turn after the save must pick it up."""
agent = self._make_custom_agent()
env["LONGCAT_API_KEY"] = "lc-fresh"
assert agent._try_refresh_env_client_credentials() is True
assert agent.api_key == "lc-fresh"
assert agent._client_kwargs["api_key"] == "lc-fresh"
agent._replace_primary_openai_client.assert_called_once_with(
reason="env_credential_refresh"
)
def test_key_rotation_between_turns_is_adopted(self, env, named_custom_provider):
agent = self._make_custom_agent(api_key="lc-old")
env["LONGCAT_API_KEY"] = "lc-old"
assert agent._try_refresh_env_client_credentials() is False
env["LONGCAT_API_KEY"] = "lc-new"
assert agent._try_refresh_env_client_credentials() is True
assert agent.api_key == "lc-new"
def test_unchanged_env_is_a_noop(self, env, named_custom_provider):
agent = self._make_custom_agent(api_key="lc-old")
env["LONGCAT_API_KEY"] = "lc-old"
assert agent._try_refresh_env_client_credentials() is False
agent._replace_primary_openai_client.assert_not_called()
def test_skipped_without_key_env(self, env, named_custom_provider):
"""Inline `api_key` / pool-backed entries have no env-sourced
credential to watch."""
named_custom_provider.pop("key_env")
agent = self._make_custom_agent()
env["LONGCAT_API_KEY"] = "lc-fresh"
assert agent._try_refresh_env_client_credentials() is False
def test_skipped_for_unknown_custom_provider(self, env, named_custom_provider):
agent = self._make_custom_agent()
agent.requested_provider = "someone-else"
env["LONGCAT_API_KEY"] = "lc-fresh"
assert agent._try_refresh_env_client_credentials() is False