Skip to content

test(models): make anthropic-messages warning test hermetic - #37784

Closed
Kyzcreig wants to merge 18 commits into
NousResearch:mainfrom
ANG-Ventures:fix/anthropic-warning-test-hermetic
Closed

Kyzcreig wants to merge 18 commits into
NousResearch:mainfrom
ANG-Ventures:fix/anthropic-warning-test-hermetic

Conversation

@Kyzcreig

@Kyzcreig Kyzcreig commented Jun 3, 2026

Copy link
Copy Markdown

Problem

test_anthropic_messages_warning_clarity.py::test_anthropic_messages_warning_names_provider_and_endpoint is an environment-dependent flake. It asserts the fallback warning contains the friendly label Claude API Proxy, but claude-api-proxy is not a built-in canonical provider — it is only registered at runtime when a user plugin under $HERMES_HOME/plugins/model-providers/claude-api-proxy/ is present.

On a clean CI checkout (no user plugins), provider_label('claude-api-proxy') falls through to the raw slug claude-api-proxy, so the assert 'Claude API Proxy' in message fails. The test only passed on developer machines that happened to have the proxy plugin installed.

Root cause (verified)

# clean env reproduces the failure deterministically:
TMPH=$(mktemp -d) && HERMES_HOME=$TMPH python -m pytest \
  tests/hermes_cli/test_anthropic_messages_warning_clarity.py
# -> 1 failed (provider_label returns 'claude-api-proxy', not 'Claude API Proxy')

Fix

Seed _PROVIDER_LABELS['claude-api-proxy'] = 'Claude API Proxy' via monkeypatch.setitem in both tests so the message-building code path is exercised deterministically, independent of which providers are registered in the environment. Test-only change; no source behavior modified.

Verification

  • Both tests pass under a clean HERMES_HOME (the CI condition that previously failed).
  • Both tests still pass with the user plugin present.
  • File byte-compiles.

Kyzcreig and others added 18 commits May 28, 2026 19:40
Some OpenAI-compatible auxiliary providers return a 200 OK with a
control-plane error string (e.g. "There is an issue with the selected
model, run --model to pick a different model") rather than raising an
HTTP error. Before this fix, ContextCompressor would accept that text as
the conversation summary, store it as _previous_summary, and drop the
real middle turns. The result was silent context corruption: the agent
saw a "summary" that was actually a model-selection error message, and
had no way to recover because no exception was raised and
_last_summary_error stayed None.

The most common trigger today is the auxiliary.<task>.model: auto
resolver bug being fixed in a sibling PR (literal "auto" goes over the
wire, bridge politely refuses, compressor swallows the refusal as the
summary). But the failure mode is more general — content-policy refusals,
rate-limit refusals worded as text, and any "polite 200" path from a
misconfigured aggregator can produce the same corruption.

Fix

Add a conservative substring match against the summary content before
storing it. If it matches a known provider-error / refusal pattern,
treat it like a transient failure on the summary model: log clearly,
set _last_summary_error / _last_aux_model_failure_error so downstream
consumers can surface a warning, and either retry on the main model
(when a distinct summary_model is configured) or enter the same
short cooldown the existing transient-error branch uses.

The substring set is deliberately small and bridge / aggregator
control-plane shaped — it would not match a real conversation summary
that happened to discuss "models" or "selection". Pure detection is
tested with positive and negative cases; integration is tested both
with and without a distinct summary model configured.

Verification (script at /tmp/verify-compressor-error-guard.py and
/tmp/verify-compressor-fallback.py — not part of the diff):

  - Positive cases (5 known provider error / refusal strings) all match.
  - Negative cases (real summary mentioning "model", empty string,
    arbitrary summary, None, int) all reject.
  - Integration: compressor patched to return a provider error string
    correctly sets _last_summary_error, populates _last_aux_model_
    failure_error with a preview, and produces output messages free of
    the error string content.
  - Fallback retry: with summary_model_override set to a broken model,
    the guard fires, _fallback_to_main_for_compression is invoked, the
    second call goes to main without the override, and the resulting
    summary is used.
Two related fixes to delegate_task toolset scoping:

1. Remove code_execution from the default subagent block list.
   Subagents already inherit `terminal` (a strictly larger capability),
   so blocking only `execute_code` was asymmetric and prevented
   legitimate use cases — e.g. "compute SHA256 + sum of primes" would
   silently fail because the subagent had no execute_code tool despite
   requesting toolsets=['code_execution'].

2. Stop calling _strip_blocked_tools() on explicit caller-requested
   toolsets. When a user/parent agent explicitly passes `toolsets=[...]`
   to delegate_task(), their intent wins over the implicit safety block
   list. Inherited/default toolsets still get the strip applied. This
   prevents silent toolset deletion which was the root cause of the
   debugging session that surfaced this bug.

Repro before fix: parent with toolsets=['hermes-cli'] delegating with
toolsets=['code_execution'] → intersection preserves code_execution →
_strip_blocked_tools deletes it → child gets [] toolsets → falls back
to default tool surface, missing execute_code.

After fix: child receives ['code_execution'] as requested.

Tests updated:
- TestStripBlockedTools.test_removes_blocked_toolsets: assert
  code_execution survives strip.
- TestStripBlockedTools.test_code_execution_no_longer_blocked: new
  regression test pinning the explicit allow.

Local patch on main; upstream PR deferred per Ace's call.
When a /stop command (or any other generation invalidation) interrupts
an agent run, the gateway took the early-return path at
_message_handler around line 7541. That path discarded the stale
result but did NOT call stop_typing on the platform adapter, so the
chat_action: typing indicator stayed sticky until the NEXT inbound
message cycled it.

Observed 2026-05-28 in gateway.log:
  15:43:30  inbound message (image)
  15:43:55  STOP invalidates generation 18
  15:43:55  STOP response sent
  15:43:56  Discarding stale agent result — generation 18 is no longer current
                ^-- early return at run.py:7555, no stop_typing
  15:44:01  next inbound message (which cycles the indicator)

The happy path at 7533-7537 and the exception path at 7894-7898 both
already clear typing correctly. Adding the same guard to the
stale-result path closes the gap.

Defensive try/except matches the surrounding pattern: typing-clear is
never load-bearing — if it fails, the next message cycles it anyway.
…al model id

When auxiliary.<task>.model is set to "auto" in config.yaml,
_resolve_task_provider_model() was treating it as a truthy model id
and propagating the literal string "auto" to the wire. The provider
then returned a 200 OK with an error-text body (e.g. "the model auto
does not exist, run --model to pick a different model"), which
downstream consumers such as ContextCompressor accept as the
compressed summary -- silent corruption with no exception raised.

The provider-side auto-resolution path (_resolve_auto via main_runtime
fallback) is already wired up and does the right thing when cfg_model
is None. The fix is to normalize the auto sentinel at the resolver
layer: when cfg_model.lower() == "auto", drop it to None so the
resolver can fall through to main_runtime / auto-detect.

Reproduction (pre-fix):
  >>> from agent.auxiliary_client import _resolve_task_provider_model
  >>> _resolve_task_provider_model("compression")  # with model: auto in config
  ("auto", "auto", None, None, None)

Post-fix:
  >>> _resolve_task_provider_model("compression")
  ("auto", None, None, None, None)

Verified end-to-end: ContextCompressor.compress now produces a real
summary (~4KB of compaction text) instead of swallowing the bridge
error string. Aux compression on auto/auto config no longer silently
corrupts the conversation summary.
…odel on caller-passed model

_get_cached_client returns 'model or default_model' as the second tuple element.
When the caller passes a truthy model name — which is always, except when caller
explicitly passes None — the raw caller input wins, bypassing the namespace
stripping that resolve_provider_client just performed via _normalize_resolved_model.

For providers in _DOT_TO_HYPHEN_PROVIDERS (bundled: 'anthropic'; plugin-provider
plugins extending the set hit this too), namespaced model IDs like
'anthropic/claude-haiku-4-5' leak past normalization and arrive at the wire
unstripped. api.anthropic.com 404s with 'model not found: anthropic/...'.

The reason this hasn't been broadly noticed: most callers happen to pass
already-normalized names. But any caller passing the namespaced form (which
_PROVIDER_MODELS advertises for some providers, and which fallback chains
routinely produce) silently fails — fatally for Anthropic-wire providers.

Fix: re-apply _normalize_resolved_model on the raw caller-passed model before
falling back to default_model. Preserves caller-wins semantics — caller's
explicit override still wins over the provider's default — but no longer
bypasses namespace stripping. The normalizer is idempotent so running it twice
(once in the resolver, once here) produces the same output.

Companion to NousResearch#24586 (resolver consults ProviderProfile.api_mode). Either can
land first; together they fully enable third-party plugin providers that
target Anthropic-Messages-API endpoints via call_llm without per-call workarounds.

Reproduction (pre-fix):
  >>> _get_cached_client('anthropic', 'anthropic/claude-haiku-4-5')
  (<client>, 'anthropic/claude-haiku-4-5')  # un-stripped

Post-fix:
  >>> _get_cached_client('anthropic', 'anthropic/claude-haiku-4-5')
  (<client>, 'claude-haiku-4-5')  # correctly stripped
…warning + fix provider_label shadowing

Manual re-apply of a02a289 onto v0.15.1 (validate_requested_model moved
to ~3343). Improves the Anthropic-Messages /v1/models fallback warning to
name the active provider label + resolved endpoint, and fixes provider_label
shadowing by renaming the two local rebinds (catalog branch ->
provider_label_for_catalog; OAuth branch -> provider_label_oauth) so the
module-level provider_label() function stays reachable. Upstream added a
SECOND shadowing rebind (openai-codex/grok OAuth branch) not present in the
original commit; renamed that one too. Ports regression test
tests/hermes_cli/test_anthropic_messages_warning_clarity.py.

Original-commit: a02a289
Manual re-apply of c1d5f9e onto v0.15.1 (_resolve_task_provider_model
moved to ~4432). When an explicit provider is given with no task-config
api_mode override, fall back to the provider profile's declared api_mode so
plugin providers whose upstream speaks the Anthropic Messages API are wrapped
with the correct transport regardless of base-URL shape. Task config still
wins; wrapped in try/except. Adds a fresh regression test (original commit was
code-only) pinning profile fallback, task-config-wins, and None passthrough.

Original-commit: c1d5f9e
… path

resolve_provider_full() (the /model switch + --provider resolver) consulted
only the models.dev catalog, Hermes overlays, and config.yaml providers/
custom_providers. It never consulted the provider-module plugin registry
(plugins/model-providers/<name>/), even though hermes_cli.auth.PROVIDER_REGISTRY
auto-extends from that same source at import time.

Result: a provider declared only as a plugin profile (e.g. a local Anthropic
proxy registered via providers/) resolves fine during runtime startup — so it
works as the configured default model — but switching INTO it via /model fails
with "Unknown provider '<name>'. Check 'hermes model' ...". Two code paths,
two different provider registries.

Reproduction (pre-fix), with a plugin provider declaring api_mode=anthropic_messages:

    from hermes_cli.model_switch import switch_model
    r = switch_model(raw_input="my-proxy/some-model",
                     current_provider="openrouter", current_model="x",
                     current_base_url="", current_api_key="",
                     is_global=False, explicit_provider="my-proxy")
    assert r.success  # -> False, "Unknown provider 'my-proxy'"

Fix: add a resolution step (1b) in resolve_provider_full that consults the
providers/ plugin registry via get_provider_profile(), translating the
ProviderProfile into a ProviderDef. api_mode maps back to transport via the
inverse of TRANSPORT_TO_API_MODE (default openai_chat). This reunifies the
switch path with the startup path, so a provider that works as a default also
works as a /model target.

Scope: IN — make the switch resolver see plugin providers. OUT — refactoring
the two registries into one (larger change, own tradeoffs); this PR keeps both
but makes the switch path layer the plugin registry the same way auth.py does.

Verified: resolve_provider_full + switch_model now succeed for a plugin
provider with correct transport/base_url/api_mode; unknown providers still
return None. Pre-existing 3 failures in test_model_switch_custom_providers.py
(model-catalog 403 in sandbox) are unrelated and fail identically on origin/main.
…ult tolerance

A transient local DNS outage (nodename nor servname) makes getUpdates fail
repeatedly. The old hardcoded ladder (10 retries x 60s cap = ~7min) escalated
to a retryable-fatal error too eagerly. Make MAX_NETWORK_RETRIES / BASE_DELAY /
MAX_DELAY config-overridable via telegram.network_retry_* and raise defaults to
20 retries / 120s cap (~25min tolerance). Bridge the YAML keys into config.extra
and add coverage.
Free-response channels are explicitly allowed to trigger Hermes without a direct mention. The prior multi-agent guard dropped any message that mentioned another bot but not Hermes, which incorrectly suppressed migration/context messages that merely quoted an old bot mention.

This keeps the existing suppression outside free-response channels, and still yields inside free-response channels when the message begins with another bot mention, which is the direct-address case.

Verification:
- venv/bin/python -m pytest tests/gateway/test_discord_connect.py::test_free_response_channel_allows_inline_other_bot_mention tests/gateway/test_discord_connect.py::test_free_response_thread_inherits_parent_for_inline_other_bot_mention tests/gateway/test_discord_connect.py::test_free_response_channel_yields_when_message_starts_with_other_bot_mention tests/gateway/test_discord_connect.py::test_normal_channel_still_ignores_other_bot_mentions -q -o 'addopts='
- venv/bin/python -m pytest tests/gateway/test_discord_connect.py::test_free_response_inline_other_bot_mention_reaches_platform_event -q -o 'addopts='
- venv/bin/python -m pytest tests/gateway/test_discord*.py -q -o 'addopts='
…tach

The /status session recap lists touched file paths via _shortened_path,
which emits a bare absolute or ~-relative path for any file outside the
gateway cwd. The gateway scans outbound message text for bare local file
paths and auto-uploads matches as native attachments
(gateway/platforms/base.py _extract_local_media_paths). As a result,
/status could silently upload the contents of a touched file (e.g.
~/.hermes/config.yaml) into the chat -- a data-exposure footgun.

Wrap each recap path in inline-code backticks so it lands inside an
inline-code span, which the detector's _in_code filter explicitly skips.
Files touched stays readable; nothing gets uploaded.

Adds two regression tests pinning the backtick wrapping and the
inline-code-span structural guarantee.
Store the most recent successful provider-call usage in agent.last_turn_usage.
The session_* counters remain cumulative; this snapshot preserves the last
turn's normalized input/output/cache-read/cache-write/reasoning split for
future /context and /usage surfaces without parsing provider-specific payloads
later.

Reset the snapshot on new sessions so usage does not leak across session
boundaries.
…#2)

* feat(usage): persist last-turn token snapshot to sessions row

The last turn's token split (input/output/cache/reasoning) lived only on
the ephemeral AIAgent instance (agent.last_turn_usage) and was lost when
the idle sweep evicted the agent between turns. The cumulative session_*
counters already persisted; the per-turn snapshot did not.

Add 5 nullable last_turn_* columns to the sessions table (declarative
migration via _reconcile_columns), write the snapshot in update_token_counts
(COALESCE so omitting them preserves the prior snapshot), and add
get_last_turn_usage() to read it back. Snapshot survives a fresh SessionDB
handle == survives agent eviction.

Wire the conversation-loop persistence call to pass the snapshot each call;
the per-call overwrite leaves the turn's final call as the snapshot,
matching agent.last_turn_usage semantics.

* feat(usage): surface persisted last-turn snapshot in /usage between turns

When the agent has been evicted by the idle sweep (no running or cached
agent), the gateway /usage command previously fell through to only a rough
transcript token estimate. Now it reads the persisted last_turn_* snapshot
from the sessions row (SessionDB.get_last_turn_usage) and renders the real
last-turn token split — input/cache/output/reasoning — so users get accurate
last-turn numbers even between turns.

Defensive int-coercion guards against malformed snapshot values. Header is
inline English (single between-turn diagnostic line) to avoid adding keys to
all ~30 locale catalogs; existing label_* keys are reused and stay in i18n
parity. No behavior change when no snapshot exists.

---------

Co-authored-by: Kyzcreig <9063726+Kyzcreig@users.noreply.github.com>
…redaction (#3)

The ENV-assignment redaction pattern matches any VAR=value whose name
contains a secret-like substring. GIT_AUTHOR_NAME and GIT_AUTHOR_EMAIL
contain "AUTH" (inside "AUTHOR"), so they were being falsely redacted to
VAR=***  mangling git commit-authoring commands in tool output (observed
when re-authoring commits for the contributor-attribution CI check).

Add a leading word-boundary + negative-lookahead allowlist
(_GIT_IDENTITY_ALLOWLIST) that exempts the four git identity vars while
still redacting every genuine secret. Verified: real keys (OPENAI_API_KEY,
AWS_SECRET_ACCESS_KEY, *_AUTH_TOKEN, *_AUTHORIZATION_KEY) still redact;
git identity vars (incl. with 'export ' prefix) pass through untouched.

Tests: 3 new allowlist tests + 2 still-redact guards; full redact suite
79 passed, 0 regressions.

Co-authored-by: Kyzcreig <9063726+Kyzcreig@users.noreply.github.com>
The test asserted the fallback warning contains the provider's friendly
label 'Claude API Proxy', but that label only resolves when the
claude-api-proxy provider plugin is registered at runtime from
$HERMES_HOME/plugins/model-providers/. It is not a built-in canonical
provider, so on a clean CI checkout provider_label() falls through to the
raw 'claude-api-proxy' slug and the assertion fails — an environment-
dependent flake that only passed on machines with the user plugin present.

Seed _PROVIDER_LABELS via monkeypatch so the message-building path is
exercised deterministically regardless of which providers are registered.
@Kyzcreig Kyzcreig closed this Jun 3, 2026
@Kyzcreig
Kyzcreig deleted the fix/anthropic-warning-test-hermetic branch June 3, 2026 02:06
@Kyzcreig
Kyzcreig restored the fix/anthropic-warning-test-hermetic branch September 21, 2026 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant