Commit graph

4082 commits

Author SHA1 Message Date
helix4u
ba9964ff0d fix(custom): pass custom provider extra body
Allow custom OpenAI-compatible providers declared under `custom_providers:`
to set provider-specific `extra_body` fields and have Hermes merge them into
chat-completions requests when the matching custom endpoint is active.

This is a manual per-provider override rather than a model-name heuristic.
OpenAI-compatible Gemma thinking support is real, but the on-wire payload
shape is backend-specific: some servers want top-level `enable_thinking`,
while vLLM Gemma and NIM-style endpoints expect `chat_template_kwargs`.
A per-provider override is safer than picking one assumed payload.

Example config:

```yaml
custom_providers:
  - name: gemma-local
    base_url: http://localhost:8080/v1
    model: google/gemma-4-31b-it
    extra_body:
      enable_thinking: true
      reasoning_effort: high
```

For vLLM Gemma or NIM-style endpoints, use the nested shape those servers
expect:

```yaml
extra_body:
  chat_template_kwargs:
    enable_thinking: true
```

Changes:

- `hermes_cli/config.py`: preserve `extra_body` in normalized
  `custom_providers:` entries and allow it in the validated field set.
- `hermes_cli/runtime_provider.py`: propagate custom-provider `extra_body`
  as `request_overrides.extra_body` for named custom runtime resolution,
  including credential-pool paths.
- `agent/agent_init.py`: at agent init, locate the matching custom-provider
  entry by `base_url` (+ optional model) and merge its `extra_body` into
  `AIAgent.request_overrides`, with caller-provided overrides winning on
  conflicting top-level keys.
- `plugins/model-providers/custom/__init__.py`: keep existing CustomProfile
  behavior (Ollama `num_ctx`, `think=False` when reasoning disabled);
  user-configured `extra_body` flows through `request_overrides`.
- `website/docs/integrations/providers.md`: document the explicit
  `extra_body` override and the vLLM/Gemma `chat_template_kwargs` variant.
- Tests cover config normalization, runtime propagation, model matching,
  trailing-slash equivalence, fallback when no `model` field is set, and
  caller-override merging precedence.

Verified end-to-end against `CustomProfile` via `ChatCompletionsTransport`:
configured `extra_body` reaches `kwargs.extra_body` on the wire request,
and coexists with profile-generated entries (Ollama `num_ctx`, `think=False`)
without clobber.

Salvaged from #29022 onto current `main`. Cosmetic typing edit in
`plugins/model-providers/custom/__init__.py` and a stale-base docs revert
in `providers.md` were dropped during cherry-pick.

Closes #29022
2026-05-21 07:48:53 -07:00
ethernet
48be2e0e4d
test: use subprocesses for each test file (#29016)
* ci(tests): install ripgrep from prebuilt tarball instead of apt

apt-get update + install of ripgrep takes ~4 min on the GHA Ubuntu
runners (the apt-get update against archive.ubuntu.com is the slow
part; ripgrep itself is small). Switching to the upstream musl
binary tarball cuts the step to a few seconds.

- Pinned to ripgrep 15.1.0 with sha256 verification (same hash as
  published in the releases sha256 sidecar file).
- Drops the `rg` binary into /usr/local/bin so it is on PATH for
  every subsequent step without GITHUB_PATH manipulation.
- Applied to both the test and e2e jobs in tests.yml.

* fix(cli): compile syntax check to tempdir, not source __pycache__

`_validate_critical_files_syntax` runs `py_compile.compile()` on each
critical bootstrap file after a successful `git pull`. The default
`py_compile` writes the resulting `.pyc` next to the source under
`__pycache__/`, which causes two real problems:

1. Parallel test workers walking the same source tree (e.g. running
   the suite under per-file process isolation) can race against each
   other on the `__pycache__` write — manifests as flaky 'directory
   not empty' errors during teardown.
2. In production, the post-pull syntax check leaves a `.pyc` behind
   that the next interpreter run might pick up — fine when the
   interpreter version matches, sketchy if it doesn't.

Fix: write the compiled output to a `tempfile.TemporaryDirectory()`
that's discarded on function exit. We only care about the compile-or-not
signal, not the artifact.

* test(runner): per-file process isolation, drop manual state reset + xdist

Replace fragile manual _reset_module_state test fixtures with robust
per-file subprocess isolation. Each test file runs in a fresh
`python -m pytest <file>` subprocess via ThreadPoolExecutor. No xdist,
no custom pytest plugin, no shared worker state.

Key changes:
  * scripts/run_tests_parallel.py — new runner: discovers test files,
    runs N in parallel via ThreadPoolExecutor, captures stdout per file,
    treats exit code 5 (no tests collected) as pass, kills all children
    on exit. Change from cpu_count to cpu_count*2. The runner is
    I/O-bound (waiting on subprocess.communicate() from pytest children)
    The parent process does almost no CPU work, so 2x oversubscription
    keeps more pipes full. When a file fails, immediately show the last
    30 lines of pytest output (stack traces + FAILED summary) plus a
    ready-to-copy repro command:
      python -m pytest tests/agent/test_auxiliary_client.py
  * scripts/run_tests.sh — delegates to run_tests_parallel.py
  * .github/workflows/tests.yml — test step: python
scripts/run_tests_parallel.py
  * pyproject.toml — drop pytest-xdist, pytest-split; simplify addopts
  * tests/conftest.py — remove ~200 lines of manual state-reset fixtures
  * AGENTS.md — update Testing section for per-file design

* test(runner): speed gateway test antipattern scan up

* fix(test): web search provider plugin test missing xai

* fix(tests): make 14 test files pass under per-file subprocess isolation

Tests that relied on cross-file state pollution from xdist workers
fail when run in isolation (per-file subprocess model). Root causes
and fixes:

Tool registry not populated:
  - test_video_generation_tool_surface_matrix: add discover_builtin_tools()
  - test_web_providers_brave_free/ddgs/searxng/general: autouse fixtures
    registering all 8 bundled web providers, reset after each test
  - test_website_policy: same provider registration pattern
  - test_web_tools_tavily: same pattern across 3 dispatch test classes
  - Also add is_safe_url/check_website_access mocks where SSRF check
    blocks example.com (DNS resolution fails in isolated envs)

Stale check_fn cache:
  - test_kanban_tools: invalidate_check_fn_cache() + _clear_tool_defs_cache()
    in both kanban guidance tests (prior test cached False for kanban_show)
  - test_discord_tool: cache invalidation in setup/teardown
  - test_homeassistant_tool: invalidate_check_fn_cache() before registry queries

Module-level state pollution:
  - test_auxiliary_client: autouse fixture clearing _aux_unhealthy_until cache
  - test_skill_commands: set_session_vars() instead of patch.dict(os.environ)
    (ContextVar takes precedence over os.environ)
  - test_dm_topics: overwrite sys.modules + separate telegram.constants mock
    + force-reimport of gateway.platforms.telegram
  - test_terminal_tool_requirements: removed duplicate class declaration,
    autouse _clear_caches fixture

* change(tests): run_tests.sh explicitly includes env vars

instead of manually dropping some vars, now we just only include some

* fix(tests): 5 more isolation/NixOS fixes

- test_approval_plugin_hooks: isolate HERMES_HOME so real user's
  command_allowlist doesn't short-circuit the approval path
- test_google_chat: skipif when Platform.GOOGLE_CHAT not in enum
  (feature not merged on this branch)
- test_write_deny: test systemd prefix against tmp_path instead of
  /etc/systemd which resolves to /nix/store on NixOS
- test_pty_bridge: use shutil.which('cat') instead of /bin/cat
  (doesn't exist on NixOS)
- profiles.py: rmtree onexc handler chmod's parent dirs too, fixing
  profile deletion when copytree preserved read-only modes from
  nix store

* fix(tests): clear unhealthy cache in autouse fixture for auxiliary_client

* fix(tests): skip send_message when telegram not installed; handle missing worker_id in browser_supervisor

* fix: py3.11 rmtree onexc compat + belt-and-suspenders unhealthy cache clear for expired codex test

* fix: address PR #29016 review feedback

- Remove tracked .pytest-cache/ artifact and add to .gitignore
- Fix stale 'xdist worker' comment in conftest.py
- Deduplicate web provider registration into tests/tools/conftest.py
  shared helper (register_all_web_providers), replacing 8 copy-pasted
  blocks across 6 test files
- Update PR description: remove stale recovered-test-files claim,
  fix worker count to match code (cpu_count*2)

* fix: eliminate race in stale-cache achievements test

The background scan thread could complete and overwrite _SNAPSHOT_CACHE
before evaluate_all() returned the stale data — only 10 fake sessions
made the scan finish instantly. Added scan_delay param to _FakeSessionDB
and set it to 2s in the stale-cache test so the background thread can't
win the race.
2026-05-21 16:40:04 +05:30
alt-glitch
87d9239009 chore: trim verbose comments/docstrings, add AUTHOR_MAP entry
- Replace 18-line comment block with 3-line invariant statement
- Trim test docstrings from multi-paragraph to single-line summaries
- Trim assertion messages from 4-line to 2-line mismatch reports
- Replace 5-line WHAT comments in stubs with 1-line WHY comments
- Add ziliangdotme@gmail.com -> ziliangpeng to AUTHOR_MAP
2026-05-21 12:49:21 +05:30
Ziliang Peng
c3a09f7835 fix(background_review): propagate parent toolset config to keep tools[] cache-stable
## Summary

The background skill/memory-review fork constructed a child `AIAgent`
without propagating `enabled_toolsets` / `disabled_toolsets` from the
parent. When the parent narrowed its toolset (via `hermes tools
disable` or `config.yaml`), the fork's default `enabled_toolsets=None`
expanded to "all registered tools" — and the fork's outbound request
body sent a wider `tools[]` array than the parent's main-turn request.

Anthropic's prompt-cache key includes the `tools[]` array byte-for-byte,
so this divergence forked the cache lineage on every nudge and forced a
full prefix rewrite. On a captured ~4 hour Claude-via-Hermes session
this cost roughly 4.3 M cache-write tokens — about half of those
attributable to the per-nudge alternation between the main turn's
narrowed `tools[]` and the review fork's wider `tools[]`.

## Goal

Extend the byte-stability invariant established by PR #17276 (which
fixed `system`) to the `tools[]` slot of the request body, so the
review fork's outbound request hits the parent's warmed Anthropic
prefix cache regardless of how the parent's toolset is configured.

## Implementation

Two-line change in `agent/background_review.py`: pass
`enabled_toolsets=getattr(agent, "enabled_toolsets", None)` and the
matching `disabled_toolsets` kwarg into the `AIAgent(...)` call inside
`_spawn_background_review`. Adds an explanatory block comment that
calls out the cache-key dependency and the relationship to PR #17276.

The post-construction runtime whitelist
(`set_thread_tool_whitelist({memory, skills})`) is untouched — it
still gates which tools the model is allowed to *dispatch*. This
change aligns only what the request body *transmits*, not what the
review is allowed to do, so the safety contract from issue #15204
remains intact.

## Testing

- `tests/run_agent/test_background_review_cache_parity.py`: new
  `test_review_fork_inherits_parent_toolset_config` asserts the
  parent's `enabled_toolsets` and `disabled_toolsets` reach the
  review-fork constructor as kwargs.
- `tests/run_agent/test_background_review_toolset_restriction.py`:
  the existing `test_background_review_does_not_narrow_toolset_schema`
  was inverted (its old "must NOT pass enabled_toolsets" rule was
  built on the assumption that the parent always ran with the
  registry default — wrong in practice when the parent is narrowed).
  Renamed to `test_background_review_matches_parent_toolset_config`
  and updated to assert the parent's value propagates verbatim.
- Verified the new positive test fails without the fix and passes
  with it.
- Full suite for `test_background_review*`:

  ```
  $ python -m pytest tests/run_agent/test_background_review.py \
                     tests/run_agent/test_background_review_summary.py \
                     tests/run_agent/test_background_review_toolset_restriction.py \
                     tests/run_agent/test_background_review_cache_parity.py -q
  18 passed in 1.85s
  ```

## Scope

- `agent/background_review.py`: 2 added kwargs + explanatory comment.
- Two test files: one new positive test, one inverted existing test.
- No production code paths outside the review fork; no schema changes;
  no public-API changes.

Refs: ziliangpeng/hermes-agent#1 (root-cause analysis with wire-level
cache-write measurements). Extends PR #17276's `system`-bytes
invariant to the `tools[]` slot.
2026-05-21 12:49:21 +05:30
EloquentBrush0x
6c26727bb3 fix(gateway): extend observe+attribution to location and media handlers
_handle_location_message and _handle_media_message were skipped when the
observe-unmentioned-group-messages feature landed (a9db0e2c7). Both handlers
now:

1. Check _should_observe_unmentioned_group_message on the skipped path and
   call _observe_unmentioned_group_message so group chatter is stored as
   shared session context even when the bot is not addressed.

2. Call _apply_telegram_group_observe_attribution on the triggered path so
   the dispatched event uses the shared (user_id=None) group session instead
   of the per-user session, letting the model see previously observed context.
   For stickers the attribution is applied after _handle_sticker completes
   (which overwrites event.text with the vision description); for all other
   media types it is applied once after caption cleaning.

Four new tests cover the observe and attribution paths for both handlers.
2026-05-20 23:52:18 -07:00
0xsir0000
5edb346c75 security(file-safety): also write-deny <root>/.env when running under a profile (#15981)
build_write_denied_paths() resolved the protected ``.env`` via
get_hermes_home(), which is profile-aware. When a profile is active
HERMES_HOME points at ``<root>/profiles/<name>`` and ``hermes_home / ".env"``
expands to the *profile* env file only — the global ``<root>/.env`` is left
off the deny list and a write_file call against it succeeds. Since the
top-level .env supplies credentials inherited by every profile, this is a
P0 credential-exfiltration / overwrite path.

Add a parallel ``_hermes_root_path()`` helper that returns the Hermes root
(via the existing ``get_default_hermes_root()`` constant) and include
``<root>/.env`` in the deny list alongside ``<active_profile>/.env``. Both
paths now refuse write_file/patch regardless of profile state. The active
HERMES_HOME .env entry is preserved so the protection in non-profile mode
is unchanged.

A regression test exercises the profile-active scenario by pointing
HERMES_HOME at ``<tmp>/profiles/coder`` and asserting that ``<tmp>/.env``
is denied.

Fixes #15981
2026-05-20 23:37:37 -07:00
Teknium
32aea113f0 fix(agent): consult supports_vision override in auto-mode routing
The contributor PR (#17936) only patched the strip path in
`_model_supports_vision()`. The auto-mode router in
`agent/image_routing._lookup_supports_vision` still only read models.dev,
so a custom-provider model declared as vision-capable would still get its
images routed through vision_analyze in the default `agent.image_input_mode:
auto` setting. Users had to set both `supports_vision: true` AND
`image_input_mode: native` to bypass the text pipeline.

Single-knob behavior now: `supports_vision: true` alone is enough in auto
mode. The strip path and the routing path consult the same resolver.

- Extract override resolution into `_supports_vision_override()` in
  agent/image_routing.py and wire it into `_lookup_supports_vision()`.
- Refactor `run_agent._model_supports_vision` to call the same helper
  (DRY, single source of truth for the resolution order).
- Strict YAML boolean coercion: `supports_vision: "false"` (quoted —
  a common YAML mistake) no longer coerces to True via bool() truthiness.
  Recognised tokens: true/false/yes/no/on/off/1/0 plus real bools and 0/1.
  Unrecognised values return None and fall through to models.dev.
- Add @CNSeniorious000 to AUTHOR_MAP for release attribution.

Tests: 26 new (TestCoerceCapabilityBool, TestSupportsVisionOverride,
TestLookupSupportsVisionOverride, TestAutoModeRespectsOverride). Existing
contributor tests + image_routing + vision_native_fast_path +
native_image_buffer_isolation all green (92/92).
2026-05-20 23:27:10 -07:00
Muspi Merol
1c76689b28 fix(agent): resolve supports_vision override for named custom providers
Named custom providers are rewritten to provider="custom" at runtime
(hermes_cli/runtime_provider.py:_resolve_named_custom_runtime), so a
config under providers.my-vllm.models.my-llava.supports_vision was
unreachable via self.provider alone. Also try cfg.model.provider as a
candidate provider key, covering both runtime and config naming.

Adds a regression test for the named-provider path.
2026-05-20 23:27:10 -07:00
Muspi Merol
24c7ce0fb8 feat(agent): allow declaring supports_vision via user config
Custom/local provider models absent from models.dev get classified as
non-vision and have their image content stripped before reaching the
upstream API. Surface a user-facing override:

  model:
    supports_vision: true

  providers:
    my-vllm:
      models:
        my-llava:
          supports_vision: true

The override short-circuits the models.dev lookup in
_model_supports_vision(), which is the single gate guarding image-strip
preprocessing on every transport path.

Refs #8731.
2026-05-20 23:27:10 -07:00
Teknium
b4afc6546e fix(xai): restore encrypted reasoning replay across turns
xAI partner integration requires Hermes to thread `encrypted_content`
reasoning items back to the Responses API on every turn so Grok can
maintain cross-turn reasoning coherence. PR #26644 (May 15) gated this
off for `is_xai_responses` on the theory that the OAuth/SuperGrok
surface rejected replayed encrypted blobs and produced the multi-turn
"Expected to have received \`response.created\` before \`error\`"
failure. That diagnosis was wrong — the prelude-SSE fallback added in
the same PR is what actually fixed that failure mode. Suppressing the
replay was an unnecessary side-effect that broke the whole point of
xAI's partnership integration.

Changes:
- agent/codex_responses_adapter.py — drop the `is_xai_responses` gate
  in `_chat_messages_to_responses_input`. Keep the kwarg in the
  signature for transport compatibility; update the docstring to
  document the May 2026 reversal.
- agent/transports/codex.py — restore
  `kwargs["include"] = ["reasoning.encrypted_content"]` on the xAI
  Responses path so xAI echoes encrypted reasoning back to us.
- tests/run_agent/test_codex_xai_oauth_recovery.py — flip the three
  xAI assertions (now: xAI MUST receive replayed reasoning AND we MUST
  include encrypted_content in the request).
- tests/agent/transports/test_codex_transport.py — flip the
  `include` assertions on `test_xai_reasoning_effort_passed` and
  `test_xai_grok_4_omits_reasoning_effort`; update the allowlist
  block comment.

The prelude-SSE fallback and the entitlement-403 surfacing fixes from
#26644 are untouched — they were independent fixes that happened to
ride along with the reasoning-replay gate.

Validation:
- Targeted: tests/run_agent/test_codex_xai_oauth_recovery.py +
  tests/agent/transports/test_codex_transport.py → 65/65 pass
- Broader: tests/agent/transports/ + tests/run_agent/ →
  1674 passed, 3 skipped, 0 failures
- E2E (real imports, isolated HERMES_HOME, ResponsesApiTransport
  build_kwargs): turn-1 request carries
  `include: ["reasoning.encrypted_content"]`; turn-2 input replays
  the encrypted_content blob from turn-1's
  `codex_reasoning_items`; native Codex unchanged.
2026-05-20 23:12:45 -07:00
liuhao1024
4ead464f97 fix(security): guard os.chmod(parent) against / and top-level dirs
Five call sites do os.chmod(path.parent, 0o700) without checking that
the parent resolves to a safe directory. If HERMES_HOME or another
path env var resolves to /, the chmod strips traversal permission from
the root inode and bricks the entire host.

Add secure_parent_dir() to hermes_constants.py that refuses to chmod
/ or any top-level directory (depth < 2). Replace all 5 call sites
with this helper.

Fixes #25821
2026-05-20 22:56:55 -07:00
Markus
a9db0e2c74 Observe unmentioned Telegram group messages 2026-05-20 22:55:31 -07:00
Teknium
c6a992e3e3 fix(security): derive <VENDOR>_API_KEY from host as final credential fallback
After #28660's host-gating fix, users with provider=custom and base_url
pointed at a commercial endpoint (DeepSeek, Groq, Mistral, …) hit
no-key-required even when they had the vendor-named env var set
(DEEPSEEK_API_KEY, GROQ_API_KEY, …). The issue author flagged this as
'what users intuitively expect'.

Adds _host_derived_api_key() to derive an env var name from the base URL
host using the *registrable* label (second-to-last). Appended to all three
api_key_candidates chains (_resolve_named_custom_runtime direct-alias path,
named-custom path, _resolve_openrouter_runtime non-openrouter branch).

Lookalike resistance: api.deepseek.com.attacker.test resolves to vendor
label 'attacker', NOT 'deepseek' — DEEPSEEK_API_KEY stays put. IPs and
loopback yield no vendor label. Already-handled vendors (OPENAI/OPENROUTER/
OLLAMA) are filtered to prevent bypass of the explicit host-gated paths.

Adds 6 tests covering positive paths (DeepSeek, Groq), the lookalike attack,
loopback rejection, the already-handled-vendor filter, and direct helper
unit tests.

Also adds erhnysr to AUTHOR_MAP.
2026-05-20 22:12:09 -07:00
Erhnysr
9514ddbee2 fix(security): address review feedback from pmos69
- Preserve OPENROUTER_API_KEY for explicit mirror/proxy configs when
  requested provider is openrouter and OPENROUTER_BASE_URL is set
- Gate OPENAI_API_KEY and OPENROUTER_API_KEY in named custom provider
  path (_resolve_named_custom_runtime) on authoritative hosts
- Gate same keys in direct-alias path
- Update tests to reflect secure-by-default behavior for local endpoints
2026-05-20 22:12:09 -07:00
Erhnysr
59088228f6 fix(security): prevent API key leakage to non-authoritative custom endpoints
Custom endpoint provider was forwarding OPENAI_API_KEY and OLLAMA_API_KEY
to arbitrary hosts. Keys should only be sent to their authoritative domains
(openai.com, ollama.com) or when explicitly configured via pool/env.

- Gate OPENAI_API_KEY to openai.com hosts only
- Gate OLLAMA_API_KEY to ollama.com hosts only
- Return 'no-key-required' for unrecognized custom endpoints
- Update tests to reflect secure-by-default behavior

Closes #28660
2026-05-20 22:12:09 -07:00
helix4u
b9b6e034d5 fix(gateway): prioritize Telegram command menu 2026-05-20 19:14:21 -07:00
Teknium
c6a380eb6c fix(skills-hub): widen identifier-dedup to GitHubSource + fix test patch path
Sibling fix on top of @EloquentBrush0x's PR #29441.

- tools/skills_hub.py GitHubSource.search() had the same r.name dedup bug.
  Two configured GitHub taps publishing same-named skills would collapse to one.
- tests/hermes_cli/test_skills_hub.py:test_browse_skills_dedup_uses_identifier_not_name
  patched hermes_cli.skills_hub.create_source_router, but browse_skills() imports
  it locally from tools.skills_hub. Fixed patch path.
2026-05-20 15:04:01 -07:00
EloquentBrush0x
8f92327891 fix(skills-hub): fix dedup in browse_skills() programmatic API
browse_skills() is the TUI gateway's API for the web UI skills browser
(tui_gateway/server.py:6574). It had the same dedup-by-name bug as
do_browse() and unified_search() fixed in the parent commit: r.name is
not unique for browse-sh skills (Airbnb, Booking.com, Zillow all publish
"search-listings"), so the dedup loop silently dropped all but the first
skill with each task name.

Switch to r.identifier, which is always globally unique.

Add a regression test asserting that two browse-sh skills with the same
name but different hostnames both appear in the browse_skills() result.
2026-05-20 15:04:01 -07:00
EloquentBrush0x
fc7e04e9ed fix(skills-hub): deduplicate search results by identifier, not name
Browse.sh exposes skills by task name (e.g. "search-listings"), which is
shared across hundreds of sites. Deduplicating by name silently dropped
every browse-sh skill after the first one with a given task name — e.g.
only Airbnb's "search-listings" would survive, collapsing Booking.com,
Zillow, and every other site's variant into nothing.

Switch unified_search() and do_browse() to use r.identifier as the dedup
key. identifier is always globally unique (e.g.
"browse-sh/airbnb.com/search-listings-ddgioa"), so same-named skills from
different browse-sh hostnames are preserved as distinct results.

Update existing TestUnifiedSearchDedup tests to model the real scenario
(same identifier appearing from two sources) and add a regression test
that asserts browse-sh skills with the same name but different hostnames
are never collapsed.
2026-05-20 15:04:01 -07:00
kshitij
3ce1cf2bb7
Merge pull request #29484 from kshitijk4poor/kp/x-search-degraded-flag
Merged after self-review + local verification of date validation and degraded flag. All tests pass, claims confirmed end-to-end.
2026-05-20 14:39:27 -07:00
helix4u
1a7bb988fc fix(gateway): harden kanban and provider cleanup races 2026-05-20 14:31:22 -07:00
kshitijk4poor
2a352f96ee fix(x_search): surface degraded results + validate dates
The xAI Responses API for x_search returns 200 OK with a
synthesized fluff answer in two failure modes that callers currently
cannot distinguish from a real, citation-backed result:

1. Any narrowing filter (allowed_x_handles, excluded_x_handles,
   from_date, to_date) was active, but the X index returned no
   matching posts. The model then answers from training data.
2. The date range is malformed, inverted, or pure-future (e.g.
   from_date=2030-01-01). The API call burns quota and Grok
   responds with a generic answer.

Mitigations, both client-side:

* Validate from_date / to_date before the HTTP call:
  - Strict YYYY-MM-DD.
  - from_date <= to_date when both set.
  - from_date <= today UTC (no posts in a window that hasn't
    started). to_date in the future remains allowed so callers
    can request 'from yesterday to tomorrow'.

* Add 'degraded' + 'degraded_reason' to successful responses.
  degraded=True iff any narrowing filter was active AND both the
  top-level 'citations' array and inline 'url_citation'
  annotations came back empty. A broad query with no filters that
  returns no citations is *not* flagged degraded — that case is
  just an unsourced answer, not a filter miss.

Tests cover all four validation paths plus six degraded-flag
scenarios (each filter type, inline vs top-level citation
recovery, broad query baseline). All existing tests continue to
pass; the additions are purely additive on the success-path
response shape.

Discovered while testing the x_search toolset end-to-end:
queries scoped to @Teknium1 returned confident-sounding generic
text about Nous Research with zero citations, and from_date in
2030 produced sassy non-answers. Both are now detectable by the
caller.
2026-05-21 02:38:45 +05:30
Teknium
31a0100104 feat(state.db): persist platform_message_id; restore yuanbao exact-id recall
PR #29211 dropped JSONL gateway transcripts and noted that the platform's
own `message_id` field (used by Yuanbao's recall guard to redact a
message by exact platform id) was no longer preserved — falling back to
content-match.  That fallback works for the common case but redacts the
wrong row when two messages share text (or fails to match when content
is post-processed).

Restore exact-id matching by giving state.db a column for it:

- New `platform_message_id TEXT` column on the messages table
  (SCHEMA_VERSION bump 11 → 12; column added via declarative reconciler
  on existing DBs, no version-gated migration block needed)
- Partial index `idx_messages_platform_msg_id` on
  (session_id, platform_message_id) to keep recall's point-lookup cheap
  even on large sessions
- `append_message()` and `replace_messages()` accept the new value:
  the gateway-facing `append_to_transcript` in `gateway/session.py`
  forwards either `message["platform_message_id"]` or the legacy
  `message["message_id"]` key (yuanbao's existing convention)
- `get_messages_as_conversation()` surfaces the column back on the
  message dict as `message_id` so platform code reads the same shape
  it used to read from JSONL
- Yuanbao `_patch_transcript`: restore branch A1 (exact id match)
  ahead of A2 (content match) ahead of B (system-note).  Both branches
  log which one fired so operators can tell from gateway.log whether
  recall hit the canonical path or had to fall back.

Tests:
- New low-level round-trip tests in `test_hermes_state.py` for both
  `append_message` and `replace_messages` paths
- The PR's `test_yuanbao_recall_db_only.py` was rewritten to assert
  the new contract: branch A1 (id match) works against DB-only
  transcripts, and branch A2 (content match) still recovers rows that
  were observed without a platform id (e.g. agent-processed @bot
  messages where run.py doesn't carry msg_id through)
2026-05-20 13:00:57 -07:00
yoniebans
0cc1a1d2d9 refactor(yuanbao): drop dead branch A1 message_id loop + pin missing fixture
PR #29211 review findings:

1. test_retry_replacement: pin DEFAULT_DB_PATH so SessionDB() doesn't write
   to the real ~/.hermes/state.db. Same fix as the other DB-only fixtures.

2. yuanbao recall branch A1 (message_id exact match) was structurally dead
   once load_transcript() became DB-only — state.db never preserves the
   platform message_id. Removed the dead loop, consolidated to a single
   content-match branch (renamed 'A: content match'). Branch B (system
   note) unchanged. Updated the test name + docstring to reflect this.

Note: self._lock is no longer taken in append_to_transcript (was guarding
the JSONL file append). SQLite append_message handles its own concurrency
via WAL mode, so this is safe; flagging for awareness.
2026-05-20 13:00:57 -07:00
yoniebans
c634c07bcc test(gateway): pin DEFAULT_DB_PATH in fixtures to prevent real state.db writes
Fixtures that instantiate SessionStore() trigger SessionDB() with no args,
which resolves to ~/.hermes/state.db via the DEFAULT_DB_PATH module constant
(snapshot of get_hermes_home() at hermes_state import time).

The autouse _hermetic_environment fixture in tests/conftest.py monkeypatches
HERMES_HOME env, but DEFAULT_DB_PATH is already cached by then. Per-test
monkeypatch.setattr(hermes_state, 'DEFAULT_DB_PATH', tmp_path/'state.db')
forces the DB into tmp_path so the tests can't leak into the real profile.

Verified by counting u1-prefixed sessions in real state.db before/after:
delta=0.
2026-05-20 13:00:57 -07:00
yoniebans
b4b118c201 refactor(gateway): drop _append_to_jsonl from mirror
Mirror messages are persisted via _append_to_sqlite. JSONL writer was
a redundant dual-write. Updated test assertions from JSONL file checks
to SQLite mock verification.
2026-05-20 13:00:57 -07:00
yoniebans
351fdcc6e6 refactor(gateway): stop writing JSONL in append_to_transcript / rewrite_transcript
state.db is canonical. JSONL transcripts were a transition fallback;
the fallback was removed in the previous commit. Existing *.jsonl files
on disk are left untouched.
2026-05-20 13:00:57 -07:00
yoniebans
971cfaa38c refactor(yuanbao): migrate recall to load_transcript()
Yuanbao's recall feature was reading the gateway JSONL directly to look up
messages by platform message_id, which state.db does not preserve. Migrated
to use load_transcript() which returns DB messages.

Recall branch A1 (message_id match) now falls through to A2 (content match)
or B (system note) for all sessions — a documented degradation. Follow-up
issue: add platform_message_id column to state.db messages to restore
exact-id matching.
2026-05-20 13:00:57 -07:00
yoniebans
024a8e3ee9 refactor(gateway): drop JSONL fallback in load_transcript
state.db is canonical. The 'use whichever source is longer' branch was
defensive code for the pre-DB migration; on every real DB it has not
fired (verified on a session corpus with 27 jsonl files / 950 sessions —
zero jsonl-bigger cases).

Test changes:
- TestLoadTranscriptCorruptLines: deleted (tested dead JSONL code path)
- TestLoadTranscriptPreferLongerSource: deleted (tested removed fallback)
- Replaced with TestLoadTranscriptDBOnly (DB-only reads)
- TestSessionStoreRewriteTranscript: fixture now creates DB session
- test_gateway_retry_replaces_last_user_turn: fixture uses real DB
2026-05-20 13:00:57 -07:00
yoniebans
1d27be0ff3 test(gateway): pin SQLite-only load_transcript behaviour 2026-05-20 13:00:57 -07:00
Teknium
eeb747de25 feat(sessions): opt-in per-session JSON snapshot writer
PR #29182 deleted the per-session JSON snapshot writer outright because
state.db is canonical and the snapshots had no in-tree consumer.  Some
users have external tooling that reads `~/.hermes/sessions/session_{sid}.json`
directly, so reintroduce the writer behind a config flag that defaults
to off.

- Add `sessions.write_json_snapshots` (default False) to DEFAULT_CONFIG
- Restore `AIAgent._save_session_log` + `_clean_session_content` as
  gated methods.  When the flag is off the call is a fast no-op; when
  on, the writer behaves as before (atomic write, truncation guard
  preserved, REASONING_SCRATCHPAD → think tag normalization)
- Re-derive the target path from `agent.session_id` on each call so
  `/branch` and `/compress` re-points happen automatically — no need
  to restore the explicit re-point bookkeeping at call sites
- Wire the single call site in `_persist_session` (the cleanup-on-exit
  hook).  Did NOT restore the 7 intra-turn calls the original PR deleted
  — those were redundant writes within the same turn that doubled disk
  I/O without adding any persistence guarantee `_persist_session` does
  not already provide
- Read the flag once at agent init via `load_config()`, cache as
  `agent._session_json_enabled`
- Update `TestNoSessionJsonSnapshot` → `TestSessionJsonSnapshotOptIn`
  to pin behavior: default off (no file), opt-in true (file written),
  no-op method on default agents, logs_dir retained unconditionally
- Update CONTRIBUTING.md and the bundled `hermes-agent` skill to
  document the flag and its default
2026-05-20 11:44:10 -07:00
yoniebans
b6c6f650ee test(session-log): pin no-session_json regression + drop trailing whitespace
Adds TestNoSessionJsonSnapshot to lock the contract that session_log_file
attribute, _save_session_log method, and the per-session JSON snapshot
writer are gone. logs_dir is retained for request_dump_*.json.

Also cleans up stray trailing whitespace in test_run_agent_codex_responses
introduced when the _save_session_log stub line was deleted.
2026-05-20 11:44:10 -07:00
yoniebans
6f1a5f8597 refactor(session-log): delete dead _clean_session_content helper
Only caller was the removed _save_session_log. Also removes the unused
convert_scratchpad_to_think and has_incomplete_scratchpad imports from
run_agent.py (both still used elsewhere via their own imports).
2026-05-20 11:44:10 -07:00
yoniebans
cebd480818 refactor(session-log): drop branch/compress re-point of session_log_file
The attribute no longer exists; nothing to re-point.
2026-05-20 11:44:10 -07:00
yoniebans
ce26785187 refactor(session-log): delete _save_session_log and all callers
state.db now stores every message field the JSON snapshot stored. Removed
the method, all 7 call-sites, and ~13 test stubs that suppressed its file I/O.
Body is in git history if it ever needs to come back.
2026-05-20 11:44:10 -07:00
adybag14-cyber
c29b4f55d9 perf(termux): speed up tui cold start 2026-05-20 11:41:52 -07:00
Julien Talbot
ca192cfb77 Add opt-in xAI TTS speech tag pauses 2026-05-20 09:22:28 -07:00
Julien Talbot
5af4b73f87 fix(xai): align migrate retirement map with docs 2026-05-20 09:18:23 -07:00
Julien Talbot
9ff98daf71 feat(xai): apply_migration — rewrite config.yaml in-place via ruamel round-trip
Extends hermes_cli.xai_retirement with apply_migration(config_path,
issues, backup=True), used by the upcoming `hermes migrate xai`
sub-command.

Uses ruamel.yaml round-trip mode so that comments, key order,
indentation, quoting style, and scalar types are preserved on
rewrite — config.yaml is treated as a user-edited file, not a
data dump.

Behavior:
  - Each issue rewrites parent[leaf] to issue.replacement
  - When issue.reasoning_effort is set (non-reasoning variants
    that map to grok-4.3), a sibling reasoning_effort key is
    added/updated alongside the model
  - Empty issues list or missing slots are no-ops (no backup,
    no rewrite)
  - When changes occur, a timestamped backup
    (.bak-pre-migrate-xai-YYYYMMDD-HHMMSS) is written first
    unless backup=False

17 unit tests cover dry-run/no-op, surgical replacement (each
slot), comment + key-order preservation, backup creation, and
idempotence (apply twice → no-op the second time).
2026-05-20 09:18:23 -07:00
Julien Talbot
6f3a020e62 feat(xai): detect retired xAI models (May 15, 2026)
Add hermes_cli.xai_retirement module that walks a Hermes config and
flags references to models being retired by xAI on May 15, 2026 per
the official migration guide.

Pure logic + dataclass, no I/O — testable in isolation and reusable
from a future hermes migrate xai sub-command.

Mappings (per https://docs.x.ai/developers/migration/may-15-retirement):
  - grok-4 / grok-4-0709                  -> grok-4.3
  - grok-4-fast{,-reasoning,-non-reasoning}    -> grok-4.3 (+reasoning_effort=none for non-reasoning)
  - grok-4-1-fast{,-reasoning,-non-reasoning}  -> grok-4.3 (+reasoning_effort=none for non-reasoning)
  - grok-code-fast-1                      -> grok-4.3
  - grok-imagine-image-pro                -> grok-imagine-image-quality

Slots scanned: principal.model, auxiliary.<any>.model (introspective),
delegation.model, tts.xai.model, plugins.image_gen.xai.model. Provider
prefix x-ai/ is normalized.

33 unit tests covering edge cases (empty/non-dict config, valid models,
ambiguous variants, all retired slots, formatter).
2026-05-20 09:18:23 -07:00
Savanne Kham
258965663c fix(chat_completions): strip tool_name from messages for strict providers
The 'tool_name' key on role=tool messages is an internal Hermes field
(stored in the messages.tool_name SQLite column for FTS indexing) that
is not part of the OpenAI Chat Completions schema. Strict OpenAI-compatible
providers — notably Moonshot AI (Kimi) — reject it with HTTP 400:

  Error from provider: Extra inputs are not permitted,
  field: 'messages[N].tool_name', value: 'execute_code'

Add 'tool_name' to the sanitize block in ChatCompletionsTransport.convert_messages
alongside the existing Codex Responses API fields (codex_reasoning_items,
codex_message_items) so it is popped before the request is sent.

Reproducer:
  hermes chat --model kimi-k2.6
  > list the top 5 Hacker News stories
  -> assistant emits tool_call(execute_code)
  -> tool result message gets tool_name='execute_code'
  -> next turn's payload includes messages[N].tool_name -> 400

Permissive backends (MiniMax, OpenRouter on most routes) ignore the extra
field and were masking the bug.
2026-05-20 02:44:08 -07:00
brooklyn!
5e743559e0
fix(lint): skip per-file shell linter when LSP will handle the file (#29054)
* fix(lint): skip per-file shell linter when LSP will handle the file

`_check_lint` ran `npx tsc --noEmit FILE.ts` after every `.ts`/`.tsx`
edit. `tsc` ignores `tsconfig.json` when given an explicit file argument
(documented quirk) and defaults to no-lib / ES5, so every ES2015+ stdlib
reference reports as missing:

  - `Cannot find global value 'Promise'`
  - `Cannot find name 'Map' / 'Set' / 'ReadonlySet' / 'Iterable'`
  - `Property 'isFinite' does not exist on type 'NumberConstructor'`
  - `Module 'phaser' can only be default-imported using esModuleInterop`
  - `import.meta is only allowed when --module is es2020+`

On real TypeScript projects this floods the `lint` field on
WriteResult / PatchResult with up to 25K tokens of false positives
per edit. The delta filter in `_check_lint_delta` is supposed to mask
them, but a tiny edit shifts line numbers and every phantom resurfaces
as "introduced by this edit". The result is a 1MB+ phantom-error dump
on every patch that eats the agent's context budget. Same shape for
`.go` (`go vet` outside a module) and `.rs` (`rustfmt --check` outside
a Cargo project).

PR #24168 added an LSP tier on top of this — real `tsserver` / `gopls`
/ `rust-analyzer` diagnostics surface in the separate `lsp_diagnostics`
field. But the broken shell linter kept running underneath, so the
phantom-error dump kept happening even when LSP was giving us a clean
authoritative signal.

This change short-circuits the shell linter for the structurally-broken
extensions (`.ts`, `.tsx`, `.go`, `.rs`) when an LSP server is active
and claims the file via `LSPService.enabled_for(path)`. The LSP tier
runs as before and carries the real diagnostics in `lsp_diagnostics`.
Other shell linters (`py_compile`, `node --check`) keep running
unconditionally — they're fast, file-local, and correct.

Default behavior (LSP disabled, LSP misconfigured, remote backend, file
outside a workspace) is unchanged — the existing fallback paths trigger
when `_lsp_will_handle` returns False, so users who haven't opted into
LSP get the same shell-linter behavior they had before.

Drive-by: `.tsx` was missing from the `LINTERS` table entirely, so TS
React files got no post-edit syntax check at all. Added it for
symmetry; in practice it now hits the LSP-skip path.

Tests:
  - `tests/agent/lsp/test_shell_linter_lsp_skip.py` — 14 tests covering:
    * skip happens for each redundant extension when LSP claims the file
      (asserted by patching `_exec` to raise on any shell-linter call)
    * shell linter still runs when LSP is inactive (regression guard)
    * `.py` / `.js` continue to run unconditionally even with LSP active
    * `_lsp_will_handle` is exception-safe: returns False on None
      service, remote backend, or `enabled_for` raising
    * `.tsx` is in both `LINTERS` and `_SHELL_LINTER_LSP_REDUNDANT`
  - All pre-existing tests in `tests/agent/lsp/` and
    `tests/tools/test_file_operations*.py` still pass (233/233).

* fix(lint): address Copilot review on #29054

Two fixes from copilot-pull-request-reviewer on PR #29054:

1. `.tsx` regression with LSP disabled
   (https://github.com/NousResearch/hermes-agent/pull/29054#discussion_r3271017282)

   The first revision added `.tsx` to the `LINTERS` table so that
   TypeScript React files would hit the LSP skip path. Side effect:
   when LSP is *disabled* (the default), `.tsx` edits would suddenly
   run `npx tsc --noEmit FILE.tsx` and inherit the same phantom-error
   dump this PR is supposed to fix. Pre-PR behavior was implicit
   `skipped` (no `LINTERS` entry); restore that.

   - Remove `.tsx` from `LINTERS`.
   - Remove `.tsx` from `_SHELL_LINTER_LSP_REDUNDANT` (the skip path
     is unreachable without a `LINTERS` entry — falls through to
     `ext not in LINTERS` first).
   - When LSP IS enabled, `.tsx` is still covered by the LSP tier
     via `_maybe_lsp_diagnostics` (typescript-language-server's
     `extensions` tuple includes `.tsx`), so the diagnostics still
     surface — just on the `lsp_diagnostics` channel, not `lint`.
   - Update test_shell_linter_lsp_skip.py to reflect this contract
     (drop `.tsx` from the parametrize lists; add
     `test_tsx_stays_out_of_linters_table_for_default_compatibility`
     and `test_tsx_default_check_lint_returns_skipped`).

2. V4A patches dropped `WriteResult.lsp_diagnostics`
   (https://github.com/NousResearch/hermes-agent/pull/29054#discussion_r3271017295)

   `tools/patch_parser.py::apply_v4a_operations` calls
   `file_ops.write_file()` per operation, then calls `_check_lint()`
   directly afterwards — but never propagates `WriteResult.lsp_diagnostics`
   to the `PatchResult`. The shell-linter skip introduced in this PR
   makes the gap visible: a `.ts` / `.go` / `.rs` V4A patch with LSP
   active would return `lint = {f: {skipped: True}}` and zero
   diagnostics from any channel.

   - `_apply_add` and `_apply_update` now return
     `Tuple[bool, str, Optional[str]]` where the third element is
     `WriteResult.lsp_diagnostics` (or `None` on failure / no diags).
   - `_apply_delete` and `_apply_move` stay 2-tuples — they don't
     produce diagnostics, no write goes through `write_file`.
   - `apply_v4a_operations` accumulates per-file diagnostics blocks
     and surfaces a combined block on `PatchResult.lsp_diagnostics`.
     Each block already carries its `<diagnostics file="...">` header
     from `LSPService.report_for_file`, so concatenation preserves
     per-file attribution.

Tests added (`test_patch_parser.py::TestV4ALspDiagnosticsPropagation`):

- ADD op: `WriteResult.lsp_diagnostics` flows to `PatchResult`
- UPDATE op: same
- No diagnostics → `PatchResult.lsp_diagnostics is None` (not "")
- Multi-file patch: combined block contains every per-file block

Verification:

- Targeted test scope: 257/257 pass
  (tests/agent/lsp/, tests/tools/test_file_operations*.py,
  tests/tools/test_patch_parser.py)
- Wider sweep: 5400 pass; 11 failures all pre-existing on origin/main
  (file_staleness / file_read_guards / file_state_registry — unrelated
  macOS /var/folders tmp-path sensitivity issues, confirmed by
  re-running on a clean origin/main checkout)

* docs(test): align shell-linter LSP skip docstring with .tsx behavior

Copilot review feedback (review #4324947616, comment #3271049036):
the test module docstring still listed .tsx alongside .ts/.go/.rs in
the skip contract, but .tsx is now intentionally NOT in LINTERS or
_SHELL_LINTER_LSP_REDUNDANT. Updated the bullet list to drop .tsx from
the skip contract and added a paragraph documenting why .tsx is left
out (preserves pre-PR implicit-skip behavior for LSP-disabled users;
LSP coverage still happens via _maybe_lsp_diagnostics).

* test(lsp): drop unused tmp_path from _make_fops helper

Copilot review #3271069484: the helper accepted tmp_path but never
used it. Callers still need tmp_path themselves for the file they're
asserting against, so we just drop the helper's parameter.
2026-05-20 01:46:40 -05:00
H-Ali13381
6a6766fb89 test(cli): cover Brave binary CDP launch detection 2026-05-19 22:34:05 -07:00
H-Ali13381
697d38a3f4 feat: auto-launch Chromium-family browser for CDP
Add browser CDP launch candidates for Chrome, Chromium, Brave, and Edge while preserving Chrome-first selection. Retry candidate launch failures instead of giving up after the first executable.

Update /browser CLI and TUI messaging, docs, and tool descriptions from Chrome-only wording to Chromium-family browser support. Add regression coverage for Brave/Edge paths, Chrome-first precedence, fallback launches, and CDP endpoint probing.
2026-05-19 22:34:05 -07:00
Jaaneek
a0c031299b feat(web): add xAI Web Search provider plugin
Adds a new bundled web search provider plugin backed by xAI's agentic
Web Search tool (server-side `web_search` on the Responses API). Slots
in alongside the existing Firecrawl / Tavily / Exa / Brave / SearXNG /
DDGS providers; opt in via `web.backend: xai` (or auto-selected by the
registry's single-provider shortcut when it's the only available web
provider, matching every other backend's behavior).

Reuses the existing xAI HTTP credential plumbing (`tools/xai_http.py`)
so it works with both `hermes auth login xai-oauth` (SuperGrok OAuth)
and `XAI_API_KEY` — no new credential paths, no new env vars, no new
setup-wizard prompts. The existing `xai_grok` post_setup hook handles
credential collection.

Reference: https://docs.x.ai/developers/tools/web-search

Provider behavior
-----------------
- Sends a structured prompt to Grok with `tools=[{"type": "web_search"}]`
  enabled and `include=["no_inline_citations"]`, then parses results
  from a `{"results": [...]}` JSON block (primary), falling back to
  `url_citation` annotations (secondary) and the top-level `citations`
  list (last-ditch). Annotation fallback falls through to citations
  when no rows are extractable, so future annotation types xAI may
  add don't silently mask real data.
- HTTP 200 + `{"error": {...}}` envelopes (model-overload, refusal)
  are surfaced as failures rather than masked as success-with-empty-
  results.
- HTTP 401 on the OAuth path triggers a single `force_refresh=True`
  retry — closes two gaps the resolver's proactive JWT-exp shortcut
  doesn't cover: opaque (non-JWT) access tokens and mid-window
  revocation. Env-var (`XAI_API_KEY`) credentials never retry; they
  can't be refreshed and an immediate retry would just burn quota.
- `is_available()` is a cheap probe (env var OR auth.json read), never
  invokes the OAuth resolver — required by the ABC contract because
  it runs on every `hermes tools` repaint and at tool-registration time.
- Class docstring documents the LLM-in-a-trench-coat trust model so
  callers piping untrusted input into `web_search` know returned URLs
  are model-generated and should be validated before fetching.

Config (`config.yaml`):

    web:
      backend: xai
      xai:
        model: grok-4.3         # optional, defaults to grok-4.3
        allowed_domains:        # optional, max 5 — mutex with excluded_domains
          - arxiv.org
        excluded_domains:       # optional, max 5
          - example-spam.com
        timeout: 90             # optional, seconds

Files
-----
- plugins/web/xai/plugin.yaml          (new) plugin manifest
- plugins/web/xai/__init__.py          (new) register(ctx) hook
- plugins/web/xai/provider.py          (new) XAIWebSearchProvider impl
- tools/xai_http.py                    (+47) has_xai_credentials()
                                            cheap-probe helper +
                                            keyword-only force_refresh
                                            arg on resolve_xai_http_
                                            credentials() (backwards
                                            compatible; all 9 other
                                            call sites unaffected)
- tools/web_tools.py                   (+11) "xai" added to configured-
                                            backend set + branch in
                                            _is_backend_available()
- tests/tools/test_web_providers_xai.py (new, 39 tests) covers
                                        identity, cheap-probe semantics,
                                        JSON / annotation / citations
                                        parse paths, request payload
                                        shape, error envelopes, OAuth
                                        force-refresh-on-401 retry,
                                        env-var-no-retry guard, 500-not-
                                        retried guard, refresh-returns-
                                        same-token guard, OAuth runtime
                                        resolution, and backend wiring.

Tests
-----
- 39 xai-suite passes
- 79 sibling web-provider tests (brave-free, ddgs, searxng, base) pass
- 119 cross-suite tests for other xai_http callers (transcription,
  x_search, tts) pass — verifies the new keyword-only arg is BC
- scripts/check-windows-footguns.py: clean on all 5 modified files

No edits to run_agent.py, cli.py, gateway/, toolsets, config schema,
plugin core, or auth core.
2026-05-19 19:27:34 -07:00
Teknium
e2fd462ebe
ci(tests): add pytest-timeout 60s hard cap to break suite-teardown deadlock (#28861)
* ci(tests): add pytest-timeout 60s hard cap to break suite-teardown deadlock

The full pytest suite reliably hangs at ~96% on origin/main, blowing through
the 20-minute GHA job timeout on every CI push since yesterday. Individual
tests complete in <30s — the deadlock builds up at session teardown after
all tests run, when leaked threads and atexit handlers from thousands of
tests interact and one of them lands in a futex-wait that never resolves.

This PR is a stopgap that unblocks CI immediately + speeds up several slow
tests we found while diagnosing.

Changes
- pyproject.toml: add pytest-timeout==2.4.0 to dev deps; bake
  --timeout=60 --timeout-method=thread into the default addopts.
- scripts/run_tests.sh: re-add --timeout flags directly because the script
  wipes pyproject addopts with -o 'addopts='.
- .github/workflows/tests.yml: explicit --timeout/--timeout-method on the
  CI pytest invocation for clarity.
- gateway/run.py: in _run_agent, if the stream consumer was never created
  (e.g. non-streaming agent or test stub), cancel the stream_task
  immediately instead of waiting out the 5s wait_for timeout. ~5s saved
  per non-streaming gateway test run.
- tests/run_agent/conftest.py: extend _fast_retry_backoff to patch
  agent.conversation_loop.jittered_backoff alongside run_agent.jittered_backoff.
  The retry loop was extracted into agent.conversation_loop which holds its
  own import — patching the run_agent reference alone left tests burning
  real wall-clock backoff seconds.
- tests/run_agent/test_anthropic_error_handling.py
  tests/run_agent/test_run_agent.py (TestRetryExhaustion)
  tests/run_agent/test_fallback_model.py: same conversation_loop fix for
  per-test fixtures (defensive — the conftest covers them too).
- tests/gateway/test_gateway_inactivity_timeout.py: trim run_duration
  10.0 → 2.0 / 5.0 → 2.0 on three tests that wait the full SlowFakeAgent
  duration. Adjusted thresholds proportionally.
- tests/gateway/test_api_server_runs.py: test_stop_interrupt_exception_does_not_crash
  trips the interrupted event in addition to raising, so the slow_run
  thread unblocks at teardown instead of waiting 10s.
- tests/hermes_cli/test_update_gateway_restart.py: also patch
  time.monotonic in the autouse fixture. _wait_for_service_active loops
  on a wall-clock deadline; with sleep no-op'd the loop spun on real
  monotonic until 10s real-time per restart attempt (20s+ per test).
- tests/tools/test_zombie_process_cleanup.py: cut runner._restart_drain_timeout
  5.0 → 0.1 in test_gateway_stop_calls_close.

Suite still hangs at 96% on full no-timeout runs; with these changes CI
runs through to a real pass/fail signal.

* chore(lock): regenerate uv.lock after adding pytest-timeout

* ci: drop pytest-timeout 60 → 30s + bump GHA job 20 → 30 min

Prior commit's timeout=60 was too generous — CI test job still hit the
20-min wall-clock cap with the suite hung at 96% (orphan agent-browser
subprocesses blocking pytest session teardown). The local timeout=20
run completed in 6:17, so 30s is conservative enough to let real tests
finish but aggressive enough to short-circuit deadlocks. Also bump GHA
job timeout to 30 min as a safety margin.

* test: delete 11 pre-existing failing tests + revert monotonic patch

The previous PR commit landed pytest-timeout=30s and the suite now
completes in 18:14 instead of hanging at 96%, but 11 pre-existing tests
fail with real assertions. Per Teknium: nuke them.

Deleted (no replacements):
- tests/gateway/test_restart_resume_pending.py::test_clean_drain_does_not_mark_resume_pending
- tests/gateway/test_restart_resume_pending.py::test_drain_timeout_only_marks_still_running_sessions
- tests/hermes_cli/test_gateway_service.py::TestGatewaySystemServiceRouting::test_gateway_install_passes_system_flags
- tests/hermes_cli/test_gateway_wsl.py::TestGatewayCommandWSLMessages::test_install_wsl_with_systemd_warns
- tests/hermes_cli/test_update_gateway_restart.py::TestCmdUpdateLaunchdRestart::test_update_detects_launchd_and_skips_manual_restart_message
- tests/hermes_cli/test_update_gateway_restart.py::TestCmdUpdateLaunchdRestart::test_update_restarts_profile_manual_gateways
- tests/tools/test_file_operations.py::TestGitBaselineCheck::* (6 tests, entire class — _check_git_baseline helper doesn't exist)

Also reverted my time.monotonic autouse-fixture hack in
test_update_gateway_restart.py — it was causing worker crashes in CI by
poisoning later tests in the same xdist worker. The two slow tests in
that file (~24s and ~20s) will go back to taking real time but should
still finish under the 30s pytest-timeout.

* test: delete more pre-existing CI failures

After previous push 3 more tests failed on CI; cull them all.

Removed:
- tests/hermes_cli/test_update_gateway_restart.py::TestCmdUpdateLaunchdRestart::test_update_without_launchd_shows_manual_restart
- tests/hermes_cli/test_update_gateway_restart.py::TestCmdUpdateLaunchdRestart::test_update_profile_manual_gateway_falls_back_to_sigterm
- tests/hermes_cli/test_update_gateway_restart.py::TestCmdUpdateResetFailedBeforeRestart::test_reset_failed_also_runs_before_retry_restart
- tests/hermes_cli/test_update_gateway_restart.py::TestCmdUpdateResetFailedBeforeRestart::test_final_failure_message_tells_user_to_reset_failed
- tests/run_agent/test_tool_call_args_sanitizer.py::test_marker_message_inserted_when_missing

The 4 update_gateway_restart tests trigger `_wait_for_service_active`
polling on a real wall-clock deadline that occasionally exceeds the 30s
pytest-timeout cap and crashes xdist workers. The marker test has a
pre-existing assertion mismatch.

* test: nuke entire TestCmdUpdateLaunchdRestart class

After surgical deletes of 4 tests this class keeps producing new
worker-crashing tests. The pattern is consistent: any test in this
class that triggers cmd_update's _wait_for_service_active polling
spins on real wall-clock time and trips pytest-timeout's thread
method, crashing the xdist worker.

Just delete the whole class (285 lines, ~10 tests). These exercise
macOS-only launchd behavior that's better tested on a real macOS
runner than in linux xdist.

* test: stub the 2 fallback_model tests that crash xdist workers on CI

* test: delete test_anthropic_error_handling.py + test_fallback_model.py entirely

These two files exercise the agent retry/fallback code paths and
consistently crash xdist workers under pytest-timeout's thread method.
Whack-a-mole-stubbing individual tests just surfaces the next ones.
Nuke both files.

* test: delete tests/hermes_cli/test_update_gateway_restart.py entirely

This file's cmd_update integration tests consistently crash xdist
workers under pytest-timeout's thread method. Surgical deletes just
surface the next set. Removing the whole file.

* ci(tests): switch pytest-timeout method thread → signal

Thread-method has been crashing xdist workers when it interrupts code
that's not interruption-safe (retry loops, threading.Event waits, etc).
Signal method uses SIGALRM which is interpreter-level and cleanly raises
a Failed: Timeout exception in test code. Should stop the worker crash
cascade — failures will surface as proper Timeout markers we can
diagnose individually.
2026-05-19 17:27:24 -07:00
Teknium
6cb9917c73
perf(compression): defer feasibility check to first compression attempt (#28957)
`AIAgent.__init__` was eagerly calling
`_check_compression_model_feasibility()` which probes the auxiliary
provider chain and runs `get_model_context_length()` (potentially
network-bound) to decide whether the configured auxiliary model can
fit a full compression-threshold window. That cost ~440ms cold on
every agent construction.

Most `chat -q` invocations finish in 1-5 seconds and never accumulate
enough context to trip the compression threshold, so the feasibility
check is pure overhead. The result is also only consumed when
compression actually fires (the function adjusts the live threshold
downward if the aux model can't fit; absent that mutation, the gate
in `conversation_loop.py:442` would never fire anyway).

Defer to first `compress_context()` call via
`agent._compression_feasibility_checked` sentinel. Runs at most once
per agent lifetime, just before the first compression pass. The
warning storage (`_compression_warning`) and gateway replay
machinery is unchanged — it still emits to status_callback on the
first turn that actually needs compression.

E2E timing (chat -q 'hi', 3 runs each):
                BEFORE   AFTER    delta
  median wall   2.03s    1.86s    -8% (-169ms)
  min wall      1.92s    1.63s    -15% (-293ms)

Real cold-start observation (synthetic 31-turn agent loop): identical
behavior since feasibility check fires once on first compression and
caches. No semantic difference for sessions that DO compress.

UX trade-off: users with broken auxiliary-provider config no longer
see the warning at session start. They see it when compression first
fires — which is exactly when it matters. For users with working
config (the vast majority), the warning never fires anyway, so the
deferral is invisible.

Tests:
- tests/run_agent/test_compression_feasibility.py — 16/16 pass
  (the one test that asserted call-at-init was updated to drive the
  lazy check explicitly via agent._check_compression_model_feasibility())
- Live tmux session: 2-turn conversation + tool call completes clean,
  zero errors in agent.log
2026-05-19 17:27:17 -07:00
Teknium
93734c26e5 fix(dingtalk): transcribe native voice notes
Sibling fix to PR #28918 (Discord voice notes). DingTalk's rich-text
"voice" item type is its native voice-message format, but the adapter
was routing it to MessageType.AUDIO — which gateway/run.py:7605 skips
for STT. The docs claim every voice-capable platform auto-transcribes,
so this brings DingTalk in line.

Generic audio uploads (mapped to "file" by DINGTALK_TYPE_MAPPING) are
unchanged — they were already classified as DOCUMENT, not AUDIO.

Adds tests/gateway/test_dingtalk.py::TestExtractMedia covering both the
voice path and the audio-passthrough invariant.
2026-05-19 17:26:26 -07:00
helix4u
448a3f9ea2 fix(discord): transcribe native voice notes 2026-05-19 17:26:26 -07:00
xxxigm
d35f8932e8 test(kanban): cover sticky blocks for worker-initiated kanban_block (#28712)
Six regression tests pinning the dispatcher contract that was broken
in #28712:

* test_worker_block_is_not_auto_promoted_by_recompute_ready —
  kanban_block survives five back-to-back ticks (compressed dispatcher
  loop).
* test_worker_block_on_child_with_done_parents_is_still_sticky —
  the parent-completion code path was the worst false-positive; even
  when every parent is done, an explicit worker block stays blocked.
* test_circuit_breaker_block_still_auto_promotes — preserves the
  pre-#28712 recovery semantics for circuit-breaker blocks (direct
  UPDATE + no "blocked" event).
* test_gave_up_event_alone_does_not_make_block_sticky — explicit
  guard so the gave_up event is never accidentally treated as
  sticky; covers the second leg of the protocol_violation loop.
* test_unblock_clears_sticky_state_and_lets_block_recover — only
  unblock_task resolves the sticky state; subsequent circuit-breaker
  blocks recover normally.
* test_protocol_violation_loop_is_broken — full bug-shaped
  reproduction: block → tick → (would-be) crash + gave_up → next tick
  still blocked.  Without the fix this would loop indefinitely.

The seventh test from the original PR (legacy-DB init recovery) was
dropped during salvage — the schema-init half of #28712 is already
fixed on main by #28754 and #28781, and the contract is covered by
test_kanban_db.py::test_connect_migrates_legacy_db_before_optional_column_indexes.
2026-05-19 17:26:23 -07:00