fix(memory): restore prefix cache hits in background review fork (~26% token saving per run)#17276
Closed
WorldWriter wants to merge 2 commits into
Closed
fix(memory): restore prefix cache hits in background review fork (~26% token saving per run)#17276WorldWriter wants to merge 2 commits into
WorldWriter wants to merge 2 commits into
Conversation
Adds set_thread_tool_whitelist / clear_thread_tool_whitelist to hermes_cli/plugins.py. When set on the current thread, restricts which tools can pass through get_pre_tool_call_block_message; non-whitelisted tools are blocked with a configurable deny message. Mirrors the per-thread approval-callback pattern already used by set_approval_callback (tools/terminal_tool.py:190). Used by _spawn_background_review to deny non-memory/non-skill tools at runtime while inheriting the parent agent's full tools schema for prefix-cache parity (see follow-up commit). Tests cover allow / deny / clear / cross-thread isolation. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Background review fork is supposed to hit Anthropic's prefix cache on the parent's messages_snapshot, but currently doesn't (cache_read=0 on every fork). Two root causes, fixed in this commit: 1. System prompt is rebuilt at fork time. _cached_system_prompt starts as None, so run_conversation calls _build_system_prompt, which embeds a minute-precision "Conversation started: ..." timestamp. Reviews fire 10+ turns after session start, so the minute differs from main's, producing a 1-character diff that invalidates the byte-exact cache key. Fix: inherit the parent's _cached_system_prompt directly (same idea as NousResearch#17089, which was self-closed for only fixing this half). 2. Tools schema was narrowed via enabled_toolsets=["memory","skills"] for safety. Anthropic's cache key includes `tools`, which sits before `system` in the cache hierarchy, so even byte-identical `system` won't hit when `tools` differs from main's full set. Fix: drop the schema-level restriction so `tools` matches main, and deny non-whitelisted tools at runtime via the existing get_pre_tool_call_block_message gate (hermes_cli/plugins.py:1085, already called at all three dispatch sites). Install/clear a thread- local whitelist (added in the previous commit) on the daemon thread. Append a soft constraint to the review prompt so the model knows. Real E2E on Sonnet 4.5 (12-tool task + auto-triggered review): - Per review-call cost: $0.331 → $0.035 (~89% reduction) - End-to-end per run: $0.848 → $0.629 (~26% reduction) - Review fork cache_create / cache_read: 88,385 / 0 → 1,234 / 94,404 Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
This was referenced May 14, 2026
teknium1
added a commit
that referenced
this pull request
May 14, 2026
teknium1
added a commit
that referenced
this pull request
May 14, 2026
…view fork - test_background_review_does_not_narrow_toolset_schema: review fork must NOT pass enabled_toolsets to AIAgent (full parent schema = matching Anthropic cache key on the 'tools' field). - test_background_review_installs_thread_local_whitelist: the runtime whitelist that replaces schema-level narrowing must contain memory + skills tools and exclude terminal / send_message / delegate_task / web_search / execute_code. - test_review_fork_inherits_parent_cached_system_prompt: new test for PR #17276's first root cause — the fork's _cached_system_prompt must equal the parent's byte-for-byte. - test_review_fork_pins_session_start_and_session_id: defensive belt-and- suspenders for the cached-prompt inheritance. Inverted the original test_background_review_agent_uses_restricted_toolsets (which asserted the schema-level narrowing) — that narrowing was the direct cause of #25322's cache miss, and the runtime whitelist replaces its safety claim without breaking cache parity. Refs #25322, #15204, PR #17276.
Contributor
|
Salvaged via PR #25434 (merged as commits 3a30c60 + 5fe0672 + follow-ups on main). Your implementation landed essentially as-is with rebase-merge so your authorship is preserved in git log. Added simpolism's defensive session_start/session_id pinning from #25427 on top, plus expanded test coverage. Thanks for the careful E2E measurement — the cost numbers in your PR body were what made this an easy merge decision. |
13 tasks
jsboige
pushed a commit
to jsboige/hermes-agent
that referenced
this pull request
May 14, 2026
jsboige
pushed a commit
to jsboige/hermes-agent
that referenced
this pull request
May 14, 2026
…view fork - test_background_review_does_not_narrow_toolset_schema: review fork must NOT pass enabled_toolsets to AIAgent (full parent schema = matching Anthropic cache key on the 'tools' field). - test_background_review_installs_thread_local_whitelist: the runtime whitelist that replaces schema-level narrowing must contain memory + skills tools and exclude terminal / send_message / delegate_task / web_search / execute_code. - test_review_fork_inherits_parent_cached_system_prompt: new test for PR NousResearch#17276's first root cause — the fork's _cached_system_prompt must equal the parent's byte-for-byte. - test_review_fork_pins_session_start_and_session_id: defensive belt-and- suspenders for the cached-prompt inheritance. Inverted the original test_background_review_agent_uses_restricted_toolsets (which asserted the schema-level narrowing) — that narrowing was the direct cause of NousResearch#25322's cache miss, and the runtime whitelist replaces its safety claim without breaking cache parity. Refs NousResearch#25322, NousResearch#15204, PR NousResearch#17276.
This was referenced May 21, 2026
alt-glitch
pushed a commit
that referenced
this pull request
May 21, 2026
…[] 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#1 (root-cause analysis with wire-level cache-write measurements). Extends PR #17276's `system`-bytes invariant to the `tools[]` slot.
AlexFoxD
pushed a commit
to AlexFoxD/hermes-agent
that referenced
this pull request
May 21, 2026
AlexFoxD
pushed a commit
to AlexFoxD/hermes-agent
that referenced
this pull request
May 21, 2026
…view fork - test_background_review_does_not_narrow_toolset_schema: review fork must NOT pass enabled_toolsets to AIAgent (full parent schema = matching Anthropic cache key on the 'tools' field). - test_background_review_installs_thread_local_whitelist: the runtime whitelist that replaces schema-level narrowing must contain memory + skills tools and exclude terminal / send_message / delegate_task / web_search / execute_code. - test_review_fork_inherits_parent_cached_system_prompt: new test for PR NousResearch#17276's first root cause — the fork's _cached_system_prompt must equal the parent's byte-for-byte. - test_review_fork_pins_session_start_and_session_id: defensive belt-and- suspenders for the cached-prompt inheritance. Inverted the original test_background_review_agent_uses_restricted_toolsets (which asserted the schema-level narrowing) — that narrowing was the direct cause of NousResearch#25322's cache miss, and the runtime whitelist replaces its safety claim without breaking cache parity. Refs NousResearch#25322, NousResearch#15204, PR NousResearch#17276.
This was referenced May 22, 2026
This was referenced May 22, 2026
waefrebeorn
pushed a commit
to waefrebeorn/slermes
that referenced
this pull request
Jul 2, 2026
…[] 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 NousResearch#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 NousResearch#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 NousResearch#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#1 (root-cause analysis with wire-level cache-write measurements). Extends PR NousResearch#17276's `system`-bytes invariant to the `tools[]` slot.
liuchanchen
pushed a commit
to liuchanchen/hermes-agent
that referenced
this pull request
Jul 3, 2026
liuchanchen
pushed a commit
to liuchanchen/hermes-agent
that referenced
this pull request
Jul 3, 2026
…view fork - test_background_review_does_not_narrow_toolset_schema: review fork must NOT pass enabled_toolsets to AIAgent (full parent schema = matching Anthropic cache key on the 'tools' field). - test_background_review_installs_thread_local_whitelist: the runtime whitelist that replaces schema-level narrowing must contain memory + skills tools and exclude terminal / send_message / delegate_task / web_search / execute_code. - test_review_fork_inherits_parent_cached_system_prompt: new test for PR NousResearch#17276's first root cause — the fork's _cached_system_prompt must equal the parent's byte-for-byte. - test_review_fork_pins_session_start_and_session_id: defensive belt-and- suspenders for the cached-prompt inheritance. Inverted the original test_background_review_agent_uses_restricted_toolsets (which asserted the schema-level narrowing) — that narrowing was the direct cause of NousResearch#25322's cache miss, and the runtime whitelist replaces its safety claim without breaking cache parity. Refs NousResearch#25322, NousResearch#15204, PR NousResearch#17276.
liuchanchen
pushed a commit
to liuchanchen/hermes-agent
that referenced
this pull request
Jul 3, 2026
…[] 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 NousResearch#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 NousResearch#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 NousResearch#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#1 (root-cause analysis with wire-level cache-write measurements). Extends PR NousResearch#17276's `system`-bytes invariant to the `tools[]` slot.
liuchanchen
pushed a commit
to liuchanchen/hermes-agent
that referenced
this pull request
Jul 3, 2026
liuchanchen
pushed a commit
to liuchanchen/hermes-agent
that referenced
this pull request
Jul 3, 2026
…view fork - test_background_review_does_not_narrow_toolset_schema: review fork must NOT pass enabled_toolsets to AIAgent (full parent schema = matching Anthropic cache key on the 'tools' field). - test_background_review_installs_thread_local_whitelist: the runtime whitelist that replaces schema-level narrowing must contain memory + skills tools and exclude terminal / send_message / delegate_task / web_search / execute_code. - test_review_fork_inherits_parent_cached_system_prompt: new test for PR NousResearch#17276's first root cause — the fork's _cached_system_prompt must equal the parent's byte-for-byte. - test_review_fork_pins_session_start_and_session_id: defensive belt-and- suspenders for the cached-prompt inheritance. Inverted the original test_background_review_agent_uses_restricted_toolsets (which asserted the schema-level narrowing) — that narrowing was the direct cause of NousResearch#25322's cache miss, and the runtime whitelist replaces its safety claim without breaking cache parity. Refs NousResearch#25322, NousResearch#15204, PR NousResearch#17276.
liuchanchen
pushed a commit
to liuchanchen/hermes-agent
that referenced
this pull request
Jul 3, 2026
…[] 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 NousResearch#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 NousResearch#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 NousResearch#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#1 (root-cause analysis with wire-level cache-write measurements). Extends PR NousResearch#17276's `system`-bytes invariant to the `tools[]` slot.
donbowman
pushed a commit
to donbowman/hermes-agent
that referenced
this pull request
Jul 13, 2026
…[] 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 NousResearch#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 NousResearch#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 NousResearch#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#1 (root-cause analysis with wire-level cache-write measurements). Extends PR NousResearch#17276's `system`-bytes invariant to the `tools[]` slot.
ziliangpeng
added a commit
to ziliangpeng/hermes-agent
that referenced
this pull request
Jul 14, 2026
… Anthropic cache namespace PR NousResearch#17276 painstakingly pinned `_cached_system_prompt`, `session_start`, `session_id`, and the toolset config on the background-review fork so its outbound request body would byte-match the parent's and hit Anthropic's exact-prefix cache. The contributor measured a ~26% end-to-end cost reduction on Sonnet 4.5. That optimization is currently being silently undone by a missing `reasoning_config` kwarg. The fork's `AIAgent(...)` call omits it, so the fork's `reasoning_config` defaults to `None`. `anthropic_adapter.build_anthropic_kwargs` (line ~2165) then short-circuits the `thinking` / `output_config` block, and the fork's request body lands in a DIFFERENT Anthropic cache namespace from the parent's. Result on the wire: 0 `cache_read_input_tokens`, full `cache_creation_input_tokens` of the entire parent prefix — every single background review. 7 days of midagent.db traffic from one host running stock Hermes against Anthropic Sonnet: ``` Background-review FIRST calls (the moment a review fork is born): count = 68 cache_write tokens = 7,004,297 cache_read tokens = 1,016,335 Cost on Sonnet ($3.75/M write vs $0.30/M read): Spent on these writes: $26.27 Cost if they had hit parent cache instead: $2.10 WASTED: $24.16 / week / user ``` That is from one user. Multiply by Hermes's installed base for the full impact. Tested against api.anthropic.com directly (see refs/api-tests/ in the attached investigation repo if needed): | pair | cache_r | cache_w | |---------------------------------------------|---------|---------| | parent fresh | 0 | 24,047 | | parent same again | 24,047 | 0 | | fork: appends 2 new tail msgs, thinking ON | 24,047 | 22 | | fork: appends 2 new tail msgs, thinking OFF | 0 | 24,047 | Same fork-shape request, only difference is `thinking`. With the fix, the fork hits the parent's full prefix and only writes the delta (the `Review the conversation above…` prompt block, ~3-5K tokens). One line in `agent/background_review.py`: pass `reasoning_config=getattr(agent, "reasoning_config", None)` to the `AIAgent(...)` constructor of the review fork. A short comment block above it explains why so the next person who reads this code doesn't re-introduce the regression. `tests/run_agent/test_background_review_cache_parity.py` already covers the system-prompt / session-id / toolset-config parity contracts that PR NousResearch#17276 introduced. I added: * a `reasoning_config` attribute to `_make_agent_stub` so the stub has a non-None parent value the test can verify is propagated. * `test_review_fork_inherits_parent_reasoning_config()` — asserts the fork's `AIAgent(...)` kwargs carry the parent's `reasoning_config`. Pre-fix this test fails with `None vs expected {'enabled': True, 'effort': 'medium'}`; post-fix all 4 tests in the file pass. ``` $ python -m pytest tests/run_agent/test_background_review_cache_parity.py -v test_review_fork_inherits_parent_cached_system_prompt PASSED test_review_fork_pins_session_start_and_session_id PASSED test_review_fork_inherits_parent_toolset_config PASSED test_review_fork_inherits_parent_reasoning_config PASSED ← new ``` Also runs against the broader background-review test suite: `test_background_review.py` (4), `test_background_review_summary.py` (8), `test_background_review_toolset_restriction.py` (3) — 19/19 pass. `agent/curator.py:1691` has the same omission for the umbrella-curation fork, but curator's prompt is "curate all skills" — it shares no prefix with any user conversation, so cache-parity is a non-issue there. Worth auditing if the curator ever takes a parent conversation as input, but not part of this PR. The `agent/auxiliary_client.py:1006` `reasoning_config=None` hardcode is intentional (title/summary one-shots on short prompts — per-call cost of namespace flip is negligible) and is also out of scope.
ziliangpeng
added a commit
to ziliangpeng/hermes-agent
that referenced
this pull request
Jul 14, 2026
… Anthropic cache namespace PR NousResearch#17276 painstakingly pinned `_cached_system_prompt`, `session_start`, `session_id`, and the toolset config on the background-review fork so its outbound request body would byte-match the parent's and hit Anthropic's exact-prefix cache. The contributor measured a ~26% end-to-end cost reduction on Sonnet 4.5. That optimization is currently being silently undone by a missing `reasoning_config` kwarg. The fork's `AIAgent(...)` call omits it, so the fork's `reasoning_config` defaults to `None`. `anthropic_adapter.build_anthropic_kwargs` (line ~2165) then short-circuits the `thinking` / `output_config` block, and the fork's request body lands in a DIFFERENT Anthropic cache namespace from the parent's. Result on the wire: 0 `cache_read_input_tokens`, full `cache_creation_input_tokens` of the entire parent prefix — every single background review. 7 days of midagent.db traffic from one host running stock Hermes against Anthropic Sonnet: ``` Background-review FIRST calls (the moment a review fork is born): count = 68 cache_write tokens = 7,004,297 cache_read tokens = 1,016,335 Cost on Sonnet ($3.75/M write vs $0.30/M read): Spent on these writes: $26.27 Cost if they had hit parent cache instead: $2.10 WASTED: $24.16 / week / user ``` That is from one user. Multiply by Hermes's installed base for the full impact. Tested against api.anthropic.com directly (see refs/api-tests/ in the attached investigation repo if needed): | pair | cache_r | cache_w | |---------------------------------------------|---------|---------| | parent fresh | 0 | 24,047 | | parent same again | 24,047 | 0 | | fork: appends 2 new tail msgs, thinking ON | 24,047 | 22 | | fork: appends 2 new tail msgs, thinking OFF | 0 | 24,047 | Same fork-shape request, only difference is `thinking`. With the fix, the fork hits the parent's full prefix and only writes the delta (the `Review the conversation above…` prompt block, ~3-5K tokens). One line in `agent/background_review.py`: pass `reasoning_config=getattr(agent, "reasoning_config", None)` to the `AIAgent(...)` constructor of the review fork. A short comment block above it explains why so the next person who reads this code doesn't re-introduce the regression. `tests/run_agent/test_background_review_cache_parity.py` already covers the system-prompt / session-id / toolset-config parity contracts that PR NousResearch#17276 introduced. I added: * a `reasoning_config` attribute to `_make_agent_stub` so the stub has a non-None parent value the test can verify is propagated. * `test_review_fork_inherits_parent_reasoning_config()` — asserts the fork's `AIAgent(...)` kwargs carry the parent's `reasoning_config`. Pre-fix this test fails with `None vs expected {'enabled': True, 'effort': 'medium'}`; post-fix all 4 tests in the file pass. ``` $ python -m pytest tests/run_agent/test_background_review_cache_parity.py -v test_review_fork_inherits_parent_cached_system_prompt PASSED test_review_fork_pins_session_start_and_session_id PASSED test_review_fork_inherits_parent_toolset_config PASSED test_review_fork_inherits_parent_reasoning_config PASSED ← new ``` Also runs against the broader background-review test suite: `test_background_review.py` (4), `test_background_review_summary.py` (8), `test_background_review_toolset_restriction.py` (3) — 19/19 pass. `agent/curator.py:1691` has the same omission for the umbrella-curation fork, but curator's prompt is "curate all skills" — it shares no prefix with any user conversation, so cache-parity is a non-issue there. Worth auditing if the curator ever takes a parent conversation as input, but not part of this PR. The `agent/auxiliary_client.py:1006` `reasoning_config=None` hardcode is intentional (title/summary one-shots on short prompts — per-call cost of namespace flip is negligible) and is also out of scope.
kshitijk4poor
pushed a commit
that referenced
this pull request
Jul 14, 2026
… Anthropic cache namespace PR #17276 painstakingly pinned `_cached_system_prompt`, `session_start`, `session_id`, and the toolset config on the background-review fork so its outbound request body would byte-match the parent's and hit Anthropic's exact-prefix cache. The contributor measured a ~26% end-to-end cost reduction on Sonnet 4.5. That optimization is currently being silently undone by a missing `reasoning_config` kwarg. The fork's `AIAgent(...)` call omits it, so the fork's `reasoning_config` defaults to `None`. `anthropic_adapter.build_anthropic_kwargs` (line ~2165) then short-circuits the `thinking` / `output_config` block, and the fork's request body lands in a DIFFERENT Anthropic cache namespace from the parent's. Result on the wire: 0 `cache_read_input_tokens`, full `cache_creation_input_tokens` of the entire parent prefix — every single background review. 7 days of midagent.db traffic from one host running stock Hermes against Anthropic Sonnet: ``` Background-review FIRST calls (the moment a review fork is born): count = 68 cache_write tokens = 7,004,297 cache_read tokens = 1,016,335 Cost on Sonnet ($3.75/M write vs $0.30/M read): Spent on these writes: $26.27 Cost if they had hit parent cache instead: $2.10 WASTED: $24.16 / week / user ``` That is from one user. Multiply by Hermes's installed base for the full impact. Tested against api.anthropic.com directly (see refs/api-tests/ in the attached investigation repo if needed): | pair | cache_r | cache_w | |---------------------------------------------|---------|---------| | parent fresh | 0 | 24,047 | | parent same again | 24,047 | 0 | | fork: appends 2 new tail msgs, thinking ON | 24,047 | 22 | | fork: appends 2 new tail msgs, thinking OFF | 0 | 24,047 | Same fork-shape request, only difference is `thinking`. With the fix, the fork hits the parent's full prefix and only writes the delta (the `Review the conversation above…` prompt block, ~3-5K tokens). One line in `agent/background_review.py`: pass `reasoning_config=getattr(agent, "reasoning_config", None)` to the `AIAgent(...)` constructor of the review fork. A short comment block above it explains why so the next person who reads this code doesn't re-introduce the regression. `tests/run_agent/test_background_review_cache_parity.py` already covers the system-prompt / session-id / toolset-config parity contracts that PR #17276 introduced. I added: * a `reasoning_config` attribute to `_make_agent_stub` so the stub has a non-None parent value the test can verify is propagated. * `test_review_fork_inherits_parent_reasoning_config()` — asserts the fork's `AIAgent(...)` kwargs carry the parent's `reasoning_config`. Pre-fix this test fails with `None vs expected {'enabled': True, 'effort': 'medium'}`; post-fix all 4 tests in the file pass. ``` $ python -m pytest tests/run_agent/test_background_review_cache_parity.py -v test_review_fork_inherits_parent_cached_system_prompt PASSED test_review_fork_pins_session_start_and_session_id PASSED test_review_fork_inherits_parent_toolset_config PASSED test_review_fork_inherits_parent_reasoning_config PASSED ← new ``` Also runs against the broader background-review test suite: `test_background_review.py` (4), `test_background_review_summary.py` (8), `test_background_review_toolset_restriction.py` (3) — 19/19 pass. `agent/curator.py:1691` has the same omission for the umbrella-curation fork, but curator's prompt is "curate all skills" — it shares no prefix with any user conversation, so cache-parity is a non-issue there. Worth auditing if the curator ever takes a parent conversation as input, but not part of this PR. The `agent/auxiliary_client.py:1006` `reasoning_config=None` hardcode is intentional (title/summary one-shots on short prompts — per-call cost of namespace flip is negligible) and is also out of scope.
exiao
added a commit
to exiao/hermes-agent
that referenced
this pull request
Jul 15, 2026
…n (+ upstream merge) (#117) * feat(desktop): layout-tree model + store + workspace geometry * feat(desktop): layout-tree renderer — splits, zones, pointer drag-session, tab strip * feat(desktop): shared UI — per-session prompt overlays, gateway overlays, tab primitives * feat(desktop): routes, nav, and command palette as contributions * feat(desktop): multi-session tiles — per-profile state, tile pane, pane mirror * feat(desktop): pointer session drag/drop + row/tab menus with close others/right/all * feat(desktop): ⌘W close-tab, ⌘⇧T reopen, ⌘T new tab, ⌘1-9 + ⌃Tab tab switching * feat(desktop): chat view — drop overlays, composer scoping, tile integration * feat(desktop): session hooks — open-in-tile, per-session actions, resilient resume * feat(desktop): focused-session-aware titlebar + statusbar * feat(desktop): contribution controller, surfaces, and wiring * feat(desktop): store + lib — layout/preview/session atoms, escape-layers, keybind helpers * feat(desktop): electron — openDir IPC + ⌘W menu bridge (tabs, not windows) * refactor(desktop): retire desktop-controller for the contribution shell; views as contributions * chore(desktop): i18n strings for tabs, zones, and session menus * docs(desktop): hermes-desktop-plugins skill + starter template * chore(desktop): build config — keep tsc emit out of src, gitignore artifacts * fix(desktop): render reasoning text in the Thinking widget The Thinking disclosure rendered blank for every reasoning-emitting model (Fable, DeepSeek, GPT-5.5, ...). Two causes: 1. ReasoningTextPart read a `text` prop that assistant-ui never populates — reasoning parts arrive via context, same as text parts — so it always got an empty string. Read the text via useMessagePartReasoning() instead, mirroring how MarkdownText uses useMessagePartText(). 2. The reasoning-only SmoothStreamingText / useSmoothReveal layer stalled at revealed="": the reasoning part stays isRunning for the whole message while the answer streams and thrashes re-renders, so the char-reveal never advanced past 0. Render reasoning through the same DeferStreamingText → surface path the assistant answer uses, and drop the dead smoothing code. * fix(cron): bound SessionDB init so a hang can't wedge cron forever run_job() constructs SessionDB() synchronously with no timeout of its own, unlike the agent's run_conversation call further down, which is already bounded by HERMES_CRON_TIMEOUT. A wedged sqlite3.connect (e.g. a stale flock from a crashed sibling process) hangs this call indefinitely. That hang is invisible to every existing cron safeguard because it happens before _submit_with_guard's future exists: the finally block that discards the job ID from _running_job_ids never runs. The job stays wedged "running" — every later tick logs "already running — skipping" — until the whole gateway process is restarted. Observed in production: a cron job's worker thread was confirmed via a live py-spy thread dump to be parked inside SessionDB.__init__'s sqlite3.connect for 3+ days, silently skipping every scheduled fire in between across a gateway process that otherwise stayed healthy. Bound the SessionDB() construction with its own timeout (HERMES_CRON_SESSION_DB_TIMEOUT, default 10s), following the same bounded-thread-pool pattern already used elsewhere in this file (the delivery retry path, and the agent inactivity watchdog just below). On timeout, log at ERROR and proceed with session_db=None instead of degrading silently to debug level, since an actual hang here is a new condition worth surfacing. Adds tests/cron/test_sessiondb_init_hang.py, including an end-to-end regression proving the dispatch guard is released and a subsequent tick can fire the same job again after a simulated hang. * fix(cron): resolve SessionDB timeout from config.yaml Salvage of #63935. The original fix read HERMES_CRON_SESSION_DB_TIMEOUT from a bare env var, but AGENTS.md requires non-secret behavioral settings to live in config.yaml with an env var bridge only for backward compatibility. Changes: - Add cron.session_db_timeout_seconds to DEFAULT_CONFIG (default 10s) - Resolution order: HERMES_CRON_SESSION_DB_TIMEOUT env override → cron.session_db_timeout_seconds in config.yaml → 10s default (mirrors the existing script_timeout_seconds pattern) - 0 = unlimited (opt-in for debugging, skips the bound) - Strengthen test: assert the warning is logged on invalid env value (caplog was taken but never asserted) - Add test: verify config.yaml resolution path works end-to-end Co-authored-by: LoicHmh <[email protected]> * fix(cli): persist close transcript without history alias * fix(cli): preserve resumed history during close flush Retain a distinct CLI history baseline during the signal window before a turn's normal persistence flush. When CLI history aliases the live agent list, use marker-only persistence so a genuinely unflushed tail is written. * fix(cli): serialize close persistence handoff Preserve one durable staged input across terminal close and the worker's early turn flush, without duplicating resumed transcripts or creating a session with a null prompt. Fixes #63766. * fix(cli): preserve noted staged input on close * fix(session): preserve clean multimodal persistence override * fix(session): serialize direct persistence flushes * fix(session): preserve clean shortened close snapshots * fix(cli): clear stale persistence override before staging * fix(cli): snapshot close state under staging lock * fix(session): restore clean API-local turn content * test(session): type finalizer clean-history assertions * test(cli): cover noted multimodal persistence handoff * fix(deepinfra): restore provider-prefix aliases for model parsing The _PROVIDER_PREFIXES frozenset in agent/model_metadata.py is static and does not auto-extend from ProviderProfile. Removing deepinfra and deep-infra from it broke provider:model prefix stripping for DeepInfra. * perf(tools): text prefilter before AST parse in tool discovery `_module_registers_tools()` reads each `tools/*.py` file and fully AST-parses it to check for a top-level `registry.register()` call. 90 files are scanned on every process start — but only 32 actually register tools. Add a cheap text prefilter: after reading the file (which we need to do anyway for AST), check that both `"registry"` and `"register"` appear in the source before calling `ast.parse`. A file with a top-level `registry.register()` call must contain both strings, so this is a perfect superset — zero false negatives. 50 of 90 files skip the AST parse entirely. The `source=` parameter is not threaded through `discover_builtin_tools`; the prefilter lives entirely inside `_module_registers_tools`, keeping the public API unchanged. Benchmark (median of 10 runs, scanning 90 files): before (read + ast.parse all): 305.9ms after (text prefilter + ast): 187.8ms speedup: 1.6x (118ms saved) Identical module set: 32 modules, same names, same order. * fix: recalculate safe_out from current input on each output-cap retry (#55546) The retry loop computed safe_out from the error's available_tokens, which reflected the *previous* request. Between retries the agent appends tool results and error text, so the real input token count grows. Deriving safe_out from the stale budget meant every retry still exceeded the context ceiling by 1+ tokens, burning through the 3-attempt limit. Compute safe_out from estimate_messages_tokens_rough(messages) so the cap tracks the growing input on each retry attempt. * fix: use provider available_out + request estimate for output-cap retry cap The branch computed safe_out from estimate_messages_tokens_rough(messages), but the provider rejected the larger api_messages request (system prompt, injected context, tool schemas). When API-only content is large, safe_out could far exceed the provider's available_tokens. Compute safe_out from estimate_request_tokens_rough(api_messages, tools=...) and keep provider available_out as an upper bound. Do not alter context_length or trigger compression for output-cap errors. Add production-path run_conversation tests that assert the retry API call's max_tokens, including a case where a large system prompt makes messages-only estimation undercount the real request. Fixes #55546 * test: add output-cap retry with compression disabled + fix request-pressure test * fix: exempt output-cap errors from compression-disabled guard * chore: remove trailing blank lines from test_ctx_halving_fix.py * chore: restore test_ctx_halving_fix.py to main * fix(agent): exempt parseable vLLM/LM Studio output-cap errors from compression-disabled guard Salvage of #63862. is_output_cap_error() returns False for vLLM/LM Studio error messages that contain 'prompt contains ... input tokens' (treated as input-overflow signal). But parse_available_output_tokens_from_error() CAN extract a valid available_tokens from those same messages. The compression-disabled guard only checked is_output_cap_error(), so vLLM/LM Studio users with compression off still got a terminal failure instead of the max-tokens retry. Fix: also exempt when parse_available_output_tokens_from_error() returns a value — that function determines whether the retry path can actually handle the error, so it's the right predicate for the exemption. Added test: verify vLLM-format error with compression_disabled=False still triggers the max-tokens retry path. Co-authored-by: dmabry <[email protected]> * fix(nemo-relay): align dynamic plugin configuration Signed-off-by: Bryan Bednarski <[email protected]> * fix(windows): put Git Bash coreutils on PATH for the non-login fallback #63955 made Hermes survive a broken `bash -l` (Ainz's `Directory \drivers\etc does not exist`) by falling back to non-login `bash -c`. But a non-login shell never sources /etc/profile, so it never gets `…\usr\bin` on PATH — and that dir holds every coreutil the file/terminal tools shell out to (cat, mktemp, mv, wc, head, stat, chmod, mkdir, find). Result: `write_file` returned bytes_written:0 with an EMPTY error (the failure text went to a missing binary's stderr) and terminal commands exited 127. The survive-broken-login-bash fix was only half-done: it stopped crashing but silently failed every write. Derive Git Bash's bin dirs (mingw64/bin, usr/bin, bin, …) from the resolved bash.exe and prepend them to the subprocess PATH on Windows, in /etc/profile precedence order so coreutils win over same-named System32 tools (find.exe, sort.exe) inside the shell. No-op off Windows and when a login snapshot is healthy (the snapshot re-exports the full PATH inside the shell), so this only bites on the broken-login fallback path. Adds _git_bash_bin_dirs() (derivation, cached) + _prepend_git_bash_dirs() (PATH merge), plus regression tests for PortableGit/MinGit layouts and the run-env injection ordering. * fix(desktop): render reasoning text in the Thinking widget (#63999) * refactor(desktop): tighten reasoning part typing, drop dead useRef Self-review nits on the Thinking-widget fix: - type ReasoningTextPart as ReasoningMessagePartComponent and read the typed useMessagePartReasoning() directly, dropping the ad-hoc cast (the hook already returns text/status). - remove useRef, now unused after deleting useSmoothReveal. - trim the autopsy comments; the PR body carries the narrative. * fix(dashboard): keep memory.provider in the config schema so Desktop's dropdown survives (#63886) The dashboard's dedicated memory-provider UI (4b184cbe5) excluded memory.provider from /api/config/schema server-side. Desktop's settings page builds its field list from that schema, so the Memory Provider dropdown silently vanished from Desktop after v0.18.1. - web_server.py: restore memory.provider as a select in _SCHEMA_OVERRIDES, with options built from plugins.memory discovery (was a stale hardcoded [builtin, honcho] list before the removal) - plugins/memory: add list_memory_provider_names() — directory-scan-only name listing, safe at module import time (no provider imports) - web ConfigPage: hide memory.provider client-side instead — the Plugins page owns the dedicated provider-switching UI there - tests: schema contract (select present, category memory, builtin sentinel) + invariant that every discoverable provider is selectable * fix(desktop): restore curated declared schema for the provider panel The desktop provider panel previously rendered the curated declarations from hermes_cli/memory_providers.py: five hindsight fields, and no panel at all for undeclared providers like honcho (OAuth connect only). The dashboard provider-switching rework re-pointed the shared config route at raw plugin schemas, so the desktop began dumping every internal field (35 for hindsight) and grew a bespoke honcho panel. Serve both surfaces from the same route: ?surface=declared returns the curated schema (empty for undeclared providers) with the original config-file + env-store write semantics; the dashboard keeps the raw plugin schema unchanged. The desktop client opts into declared. * feat(gemini): improve request context for support and compatibility Include the Hermes client name and version with Gemini inference, model and tier checks, and TTS requests. Add focused coverage for the request headers and keep the Gemini-specific context scoped to Google Gemini endpoints. * fix(gemini): restrict TTS client context to official host * fix(tests): patch catalog urlopen wrapper in gemini probe tests (#64318) test_probe_sends_client_context_to_gemini and test_probe_omits_gemini_client_context_for_other_providers (added in b8eb89f5c) patch hermes_cli.models.urllib.request.urlopen, but probe_api_models routes requests through the _urlopen_model_catalog_request wrapper (open_credentialed_url from the urllib_security hardening), so the mock is never invoked and mock_urlopen.call_args is None -> TypeError. Every CI run on main and every PR has been failing test slice 7/8 on these two tests. Point the patches at _urlopen_model_catalog_request, the same target every sibling test in TestProbeApiModelsUserAgent already uses. 89/89 tests in the file now pass. * fix(moa): flatten structured message content in the advisory view (#64319) Cache-decorated turns (apply_anthropic_cache_control converts string content to [{type: text, ..., cache_control}] lists — applied BEFORE the MoA facade since the #57675 cache-cold fix) and multimodal turns (text + image_url parts) flattened to empty strings in _reference_messages, which only read str content. On turn 1 of a provider:moa session with a Claude aggregator the references received a single EMPTY user message: Anthropic-side providers 400'd ('messages: at least one message is required') while tolerant models answered 'no user request is present' (live incident Jul 14 2026, preset 'closed'). Fixes, in totality: - _reference_messages: extract visible text via agent/message_content.flatten_message_text for user/assistant/tool turns (skips image parts, so no base64 leaks into the advisory view); decorated and undecorated transcripts now produce a byte-identical advisory view (advisor cache prefix stays stable). - image-only user turns get a placeholder instead of an empty message (Anthropic rejects empty text blocks) or a silently dropped turn (would break user/assistant alternation). - degenerate-case fallback flattens structured content too. - _attach_reference_guidance: a decorated/multimodal trailing user turn now receives the guidance as a NEW text part appended AFTER the cache_control-marked part (cached prefix byte-stable) instead of falling through to a second consecutive user message (strict providers reject user/user). - conversation_loop MoA injection: multimodal user turns get the MoA context appended as a trailing text part instead of being dropped; user_prompt for the one-shot path flattens content lists instead of str()-ing them (which leaked base64 payloads into the prompt). Live-verified on the 'closed' preset (real OpenRouter wire, 2 user turns, tool loop): all 4 reference calls carry the full document + rendered tool state, end on user, zero tool-role/tool_calls; advisor cache_write 7968 then cache_read 5909+; aggregator cache_read 14880-15237 on iterations 2+. Co-authored-by: bo.fu <[email protected]> * feat(codex): redeem banked usage-limit resets via /usage reset (#64280) OpenAI lets ChatGPT-plan Codex users bank rate-limit reset credits, but until now they could only be redeemed from the Codex CLI/app or the website. This wires the same backend API into Hermes: - /usage on the openai-codex provider now shows "You have N resets banked - use /usage reset to activate" (parsed from the rate_limit_reset_credits field the /usage endpoint already returns). - New /usage reset subcommand (CLI + gateway) redeems one banked credit via POST .../rate-limit-reset-credits/consume with a UUID idempotency key, mirroring codex-rs backend-client semantics (PathStyle /wham vs /api/codex, ChatGPT-Account-Id header, reset/nothing_to_reset/no_credit/already_redeemed outcomes). - Guard: redemption is refused while no rate-limit window is fully exhausted, since a banked reset restores the FULL 5h + weekly allowance and spending it early wastes it. /usage reset --force overrides. Zero banked credits and non-codex providers are refused with clear messages; nothing_to_reset reports the credit was NOT spent. - i18n: new gateway.usage.unknown_subcommand / reset_wrong_provider keys across all 16 locales; docs updated (cli.md, messaging index). Tested with unit tests plus a real-socket E2E against a local fake Codex backend exercising redeem/guard/force and the /usage hint. * fix(conversation): clear stale housekeeping fallback on substantive tool-only turns A cached _last_content_with_tools response from a housekeeping-only turn could survive a later substantive tool-only turn. When the model returned an empty response, Hermes incorrectly finalized the older housekeeping narration instead of invoking the post-tool empty-response nudge. Production impact: scheduled cron jobs could return early without completing their actual work (e.g., daily report job returning a housekeeping message instead of producing the report artifact). Root cause: The fallback state was only updated when a turn had both content AND tool_calls. A turn with tool_calls but empty visible content would skip state updates entirely, leaving stale fallback state intact. Fix: Classify tools in every tool-call turn (regardless of visible content). When any tool is substantive (non-housekeeping), clear the older fallback state before processing later empty responses. This prevents two-turn-old housekeeping narration from being treated as if it belonged to the immediately preceding substantive tool turn. Regression test added: tests/run_agent/test_conversation_fallback_state.py Fixes #63860 * fix(conversation): clear _mute_post_response on substantive tool-only turn Salvage of #63888. The original fix clears stale _last_content_with_tools on substantive tool-only turns but doesn't clear _mute_post_response, which a prior housekeeping turn may have set. This suppresses tool progress output via _vprint until the no-tool-call branch resets it at line ~4834 — after all tools have finished executing. Fix: also reset _mute_post_response = False when clearing stale fallback. Added test: verify pure housekeeping turns (content + only housekeeping tools) still set the fallback correctly — the original use case the fallback was designed for. Co-authored-by: liuhao1024 <[email protected]> * fix(kanban): nudge workers that exit without complete/block Add a bounded turn-end stop guard for kanban workers. When a worker tries to exit with finish_reason=stop without having called kanban_complete or kanban_block, inject up to two synthetic nudges so the conversation loop continues instead of exiting cleanly (which the dispatcher records as protocol_violation). Mirrors the existing verify-on-stop pattern: same ephemeral scaffolding flag (_kanban_stop_synthetic), same role-alternation contract, same _pending_verification_response fallback for budget exhaustion. Disabled by default (gated on HERMES_KANBAN_TASK env var set by the dispatcher); kill switch via HERMES_KANBAN_STOP_NUDGE=0. Salvaged from #62262 by @mdc2122. The original branch was 272 commits behind main with ~538 files of stale-base reversions; this salvage applies only the 4 substantive files (agent/kanban_stop.py, conversation_loop.py insertion, run_agent.py _EPHEMERAL_SCAFFOLDING_FLAGS, tests/agent/test_kanban_stop.py). * fix(state): enforce synchronous=FULL on macOS to prevent btree corruption On Darwin, the default synchronous=NORMAL only calls fsync(), which Apple explicitly states does not guarantee data-on-platter or write-ordering. During a WAL checkpoint race with process termination (e.g., launchd shutdown), this can leave the main DB with half-written btree pages, resulting in btreeInitPage error 11 corruption. WAL mode's durability guarantee assumes the OS honors fsync barriers; macOS does not unless we explicitly set synchronous=FULL (which issues fsync() and F_FULLFSYNC via checkpoint_fullfsync=1). Previously, apply_wal_with_fallback() skipped setting synchronous=FULL when the DB was already in WAL mode, leaving connections at the unsafe synchronous=NORMAL default. This commit adds _enforce_macos_synchronous_full() to always enforce synchronous=FULL on macOS after any WAL activation. Fixes #63531 * docs(state): fix _enforce_macos_synchronous_full docstring synchronous=FULL issues plain fsync(), not F_FULLFSYNC. The F_FULLFSYNC barrier comes from checkpoint_fullfsync=1, set by the separate _apply_macos_checkpoint_barrier(). The original docstring conflated the two PRAGMAs. * fix(skills): guard skill slash commands against core-command and slug collisions scan_skill_commands() had two collision bugs in the same loop body: 1. Core-command collision: a skill whose normalized slug matches a core Hermes command name or alias (e.g. "skills", "learn", "bg") would get an auto-generated /command that shadows the core command in the gateway dispatch path (skill map is consulted before built-in handlers). The skill command silently overrode the core command. 2. Inter-skill slug collision: the seen_names set deduped on the raw frontmatter name, but the command map was keyed by the normalized slug. Two distinct names collapsing to the same slug (e.g. "git_helper" vs "git-helper") both passed the dedup, and the second silently clobbered the first. Fix: add two guards in scan_skill_commands() after slug normalization: - resolve_command(cmd_name) check skips skills colliding with any core CommandDef (name or alias), logging a warning. Uses the existing resolve_command() API so aliases and case variants are covered without a separate cache. The skill remains loadable via /skill. - cmd_key in _skill_commands check dedups on the resolved slug, first-wins (preserving local-before-external precedence), logging a warning naming the shadowed skill. Combines and supersedes #31204 (@cyrkstudios), #53450 (@Gridzilla), #50304 (@petrichor-op), and #63305 (@Vissirexa). Co-authored-by: cyrkstudios <[email protected]> Co-authored-by: Gridzilla <[email protected]> Co-authored-by: petrichor-op <[email protected]> Co-authored-by: Vissirexa <[email protected]> * fix(kanban_db): bounded retry for clean-exit protocol violations A worker that exits 0 without calling kanban_complete/kanban_block (model stops early, transient tool wedge) tripped the failure breaker on FIRST occurrence and the task was blocked. These are overwhelmingly transient: with a bounded retry (limit 3, tracked via a violation fingerprint) ~96%% of them complete on respawn. Genuine repeat offenders still trip the breaker at the limit. Co-Authored-By: Claude Fable 5 <[email protected]> * review follow-up: violation-only retry streak with defined max_retries precedence Address the hermes-sweeper review of #61233: the bounded retry budget is now a clean-exit-specific streak, not a share of the unified consecutive_failures counter. - detect_crashed_workers stamps a protocol_violation marker into the violation run's metadata (via the event payload _end_run copies); _protocol_violation_streak derives the streak from run history: consecutive most-recent violation runs, rate_limited runs neutral (mirroring their unified-counter treatment), any other closed run resets it. Mixed failure kinds can neither consume nor extend the budget. - Below-budget violations no longer call _record_task_failure at all: the task returns to ready with last_failure_error stamped directly (including the corrective retry guidance wording adopted from #61817, which build_worker_context surfaces to the retry worker) and the unified counter is untouched, keeping the two budgets independent. - At the bound the trip funnels through _record_task_failure with a new keyword-only force_trip=True: the reaper has already resolved the per-task max_retries override against the violation streak itself, so the threshold comparison is skipped rather than double-applied. max_retries keeps its documented top precedence in both directions: max_retries=1 blocks on the first violation, max_retries=5 blocks on the fifth consecutive one, unset uses the default bound of 3. - Replace the first-violation-blocks regression test with five tests: first occurrence retries (ready + guidance stamped + no gave_up + unified counter untouched); streak trips exactly at the bound with protocol_violations/protocol_violation_limit in the gave_up payload and the auto-blocked side channel set; a prior nonzero crash does not consume the violation budget; a non-violation failure between violations resets the streak; max_retries precedence both directions. All five fail against the previously reviewed diff and pass with this follow-up. The test harness resolves hermes_cli.kanban_db fresh and uses that single module object for the exit registry, liveness patch, and reaper — earlier suite tests reload the module, and the old mixed-object harness made _classify_worker_exit return unknown (the reason the old test failed in full-suite runs on main). Kanban suite: zero introduced failures vs upstream/main tip (62 vs 63 pre-existing environmental failures — the one no longer failing is the old violation test this replaces; 662 passed vs 657). Co-Authored-By: Claude Fable 5 <[email protected]> * docs(kanban): worker-lifecycle + events table reflect the bounded protocol-violation retry Fold in kevinb361's suggested lifecycle wording (#61817 conceded in favor of this PR) and update the second stale site his sweep didn't cover: the task-events table still said the dispatcher 'auto-blocks immediately instead of retrying'. Both now describe the violation-only streak: protocol_violation fires on every violation (its payload marker feeds the budget), below-budget runs return the task to ready, and gave_up + auto-block happen only when the consecutive streak reaches _PROTOCOL_VIOLATION_FAILURE_LIMIT (default 3, per-task max_retries overriding). * follow-up: integrate agent nudge + dispatcher retry docs and tests - Nudge text now warns that repeated protocol violations will block the task and require manual intervention, so the model understands the consequence of ignoring the nudge. - Kanban docs restructured to clearly separate the two defense layers: agent-side prevention (nudge, from #64350) and dispatcher-side recovery (bounded retry, from this PR). - Two new integration tests verifying the nudge mentions blocking and that the agent-side and dispatcher-side budgets are independent. * fix(agent): validate credential pool after provider auto-detection (#63425) Provider auto-detection (URL-based inference for Anthropic, OpenAI Codex, and xAI endpoints) runs before credential-pool validation in AIAgent init, but #63048 placed the pool validation before auto-detection. When the agent is constructed with provider=None and a recognized endpoint URL, the pool is validated against an empty provider identity and discarded, even though auto-detection correctly resolves the provider moments later. Fix: move the credential-pool validation block to after the URL-based auto-detection chain. The pool is stored on the agent before auto-detection; validation now checks the resolved provider and only nullifies agent._credential_pool when the pool's scoped provider genuinely doesn't match. Regression test covers all three auto-detection paths: - Anthropic (api.anthropic.com) - OpenAI Codex (chatgpt.com/backend-api/codex) - xAI (api.x.ai) Fixes #63425. * fix(cron): keep live one-shots when running-set check fails * fix(gateway): fail closed on compression state probe errors * perf(cli): skip npm install during update when lockfile is unchanged (#17268) (cherry picked from commit 8fb6d5e910b6fd89bdc698c477cc5f039a0deabd) (cherry picked from commit 27474007b9463d7ce19d981a24aeeac552e79f48) * fix: derive skip-key manifests from npm workspaces config Review round 2 from @ethernet8023 on #61580: 1. The manifest list was a hardcoded root/ui-tui/web trio — desktop and any future workspace escaped the skip key even though step 1's root install hoists deps for every workspace. The list is now expanded from the root package.json 'workspaces' globs (npm's own source of truth): on the real repo that yields all 8 manifests incl. apps/desktop, apps/bootstrap-installer, apps/shared, and the nested ui-tui/packages/hermes-ink. Unreadable package.json falls back to root manifests only (never skips more than main would install). 2. --prefer-offline dropped entirely (this branch no longer carries #39399): local 3-run benchmarks on the repo's real manifests show the flag is noise on npm ci with a warm cache (root: 0.90s vs 0.84s avg; ws: 4.02s vs 4.00s avg) — npm ci does no resolution and the content-addressed cache already serves tarballs locally. It also carried the stale-resolution risk on the npm install fallback the reviewer flagged. All the real win is the skip itself (0s vs ~5s+). Tests: workspace-glob edit (desktop), literal-listed edit, and new-workspace-under-glob all defeat the skip; verified against the real repo's workspace config (8 manifests picked up). * test(update): document shared npm cache scope * fix(gateway): allow ws_orphan_reap rows in session recovery (#63207) Whitelist ws_orphan_reap alongside agent_close in find_latest_gateway_session_for_peer so gateway stale-routing self-heal can reopen wrongly-reaped messaging sessions instead of minting empty replacements. Layer A prevention already landed in #60609. * test(gateway): cover ws_orphan_reap session recovery (#63207) Regression tests for find_latest_gateway_session_for_peer and SessionStore stale-routing self-heal when end_reason is ws_orphan_reap. Pin manual approval mode in blocking E2E tests so smart aux-LLM resolution does not flake CI. * fix(telegram): classify and dedup post-reconnect probe failures (#63243) * docs: clarify write safety, HERMES_WRITE_SAFE_ROOT, and file-mutation verifier Document that safe-root violations are hard-blocked (not approval-gated), add a security guide section for write_file/patch guards, and link cron and verifier docs so users trust the footer over agent summaries. * fix(file-safety): distinguish safe-root write denial from credential blocks Return actionable errors when HERMES_WRITE_SAFE_ROOT blocks a path instead of labeling every denial as a protected credential file. Wire the helper through write_file, patch, delete/move, and the Copilot ACP shim; sync docs examples. * test(file-safety): add integration tests for safe-root denial messages Exercises the actual ShellFileOperations.write_file and patch_replace code paths (not just the helper in isolation) to verify that safe-root denials surface 'outside HERMES_WRITE_SAFE_ROOT' and credential-path denials surface 'protected system/credential file'. Adapted from PR #55615 by @liuhao1024. * fix(telegram): diagnose blocked-loop init hangs, unbind DoH from system DNS The #63309 hang class — gateway stuck at 'Connecting to Telegram (attempt 1/8)' with no retry, no timeout, for minutes — can only occur when the event loop thread itself is blocked in a synchronous call: _await_with_thread_deadline's timer fires off-loop, but its expiry hand-off (call_soon_threadsafe) still needs the loop to run, and the gateway's outer wait_for is a pure loop timer. When the loop is pinned, every layer goes silent simultaneously and the process wedges with no evidence of where. Two changes: 1. Loop-blocked watchdog in _await_with_thread_deadline: a second daemon timer fires one grace period (5s) after the deadline; if the loop still hasn't processed the expiry, it logs a WARNING from the timer thread and faulthandler-dumps all thread stacks to stderr — converting the silent hang into a trace that names the exact blocking frame. A threading.Event set by the expiry callback (and on normal exit) keeps completed awaits from ever being misreported. 2. discover_fallback_ips: the system-resolver leg runs socket.getaddrinfo in a worker thread with no timeout, and asyncio.gather waited on it unboundedly — a wedged OS resolver stalled discovery for minutes between the two startup log lines. Its result only feeds a log message, so it no longer gates discovery: DoH legs (already client-bounded) are gathered alone and the system leg is awaited with a _DOH_TIMEOUT cap, best-effort. Refs #63309 Tests: 3 watchdog regressions (blocked-loop dump fires; responsive-loop timeout does not; completed await does not) + 2 hung-resolver regressions (DoH results returned promptly; worst-case seed fallback stays bounded). * fix(cron): prevent long-running scheduled scripts from running twice * fix(gateway): never prune sessions when active-process check fails prune_old_entries' active-process guard failed open: when has_active_processes_fn raised, the except block logged at debug and fell through to the age check, so sessions with live background processes attached could still be pruned — violating the documented invariant that such sessions are never dropped. Add a continue so an exception in the safety check fails safe (the entry is kept). Commit 6b408e131 fixed the session_key/session_id mismatch in this same guard but left the exception path failing open. Co-Authored-By: Claude Fable 5 <[email protected]> * fix(background_review): inherit parent's reasoning_config to preserve Anthropic cache namespace PR #17276 painstakingly pinned `_cached_system_prompt`, `session_start`, `session_id`, and the toolset config on the background-review fork so its outbound request body would byte-match the parent's and hit Anthropic's exact-prefix cache. The contributor measured a ~26% end-to-end cost reduction on Sonnet 4.5. That optimization is currently being silently undone by a missing `reasoning_config` kwarg. The fork's `AIAgent(...)` call omits it, so the fork's `reasoning_config` defaults to `None`. `anthropic_adapter.build_anthropic_kwargs` (line ~2165) then short-circuits the `thinking` / `output_config` block, and the fork's request body lands in a DIFFERENT Anthropic cache namespace from the parent's. Result on the wire: 0 `cache_read_input_tokens`, full `cache_creation_input_tokens` of the entire parent prefix — every single background review. 7 days of midagent.db traffic from one host running stock Hermes against Anthropic Sonnet: ``` Background-review FIRST calls (the moment a review fork is born): count = 68 cache_write tokens = 7,004,297 cache_read tokens = 1,016,335 Cost on Sonnet ($3.75/M write vs $0.30/M read): Spent on these writes: $26.27 Cost if they had hit parent cache instead: $2.10 WASTED: $24.16 / week / user ``` That is from one user. Multiply by Hermes's installed base for the full impact. Tested against api.anthropic.com directly (see refs/api-tests/ in the attached investigation repo if needed): | pair | cache_r | cache_w | |---------------------------------------------|---------|---------| | parent fresh | 0 | 24,047 | | parent same again | 24,047 | 0 | | fork: appends 2 new tail msgs, thinking ON | 24,047 | 22 | | fork: appends 2 new tail msgs, thinking OFF | 0 | 24,047 | Same fork-shape request, only difference is `thinking`. With the fix, the fork hits the parent's full prefix and only writes the delta (the `Review the conversation above…` prompt block, ~3-5K tokens). One line in `agent/background_review.py`: pass `reasoning_config=getattr(agent, "reasoning_config", None)` to the `AIAgent(...)` constructor of the review fork. A short comment block above it explains why so the next person who reads this code doesn't re-introduce the regression. `tests/run_agent/test_background_review_cache_parity.py` already covers the system-prompt / session-id / toolset-config parity contracts that PR #17276 introduced. I added: * a `reasoning_config` attribute to `_make_agent_stub` so the stub has a non-None parent value the test can verify is propagated. * `test_review_fork_inherits_parent_reasoning_config()` — asserts the fork's `AIAgent(...)` kwargs carry the parent's `reasoning_config`. Pre-fix this test fails with `None vs expected {'enabled': True, 'effort': 'medium'}`; post-fix all 4 tests in the file pass. ``` $ python -m pytest tests/run_agent/test_background_review_cache_parity.py -v test_review_fork_inherits_parent_cached_system_prompt PASSED test_review_fork_pins_session_start_and_session_id PASSED test_review_fork_inherits_parent_toolset_config PASSED test_review_fork_inherits_parent_reasoning_config PASSED ← new ``` Also runs against the broader background-review test suite: `test_background_review.py` (4), `test_background_review_summary.py` (8), `test_background_review_toolset_restriction.py` (3) — 19/19 pass. `agent/curator.py:1691` has the same omission for the umbrella-curation fork, but curator's prompt is "curate all skills" — it shares no prefix with any user conversation, so cache-parity is a non-issue there. Worth auditing if the curator ever takes a parent conversation as input, but not part of this PR. The `agent/auxiliary_client.py:1006` `reasoning_config=None` hardcode is intentional (title/summary one-shots on short prompts — per-call cost of namespace flip is negligible) and is also out of scope. * fix(background_review): gate reasoning_config inheritance on not-routed + dedupe recorder stubs Review follow-up to the reasoning_config cache-parity fix: - Only inherit the parent's reasoning_config when the fork runs on the parent's model (not routed). On the routed aux path (auxiliary.background_review.{provider,model}) the cache is cold regardless, so parity buys nothing, and the parent's effort vocabulary can be invalid for the routed model/provider: OpenRouter extra_body.reasoning.effort is forwarded unclamped (chat_completions.py) and codex_responses only maps max/ultra for gpt-5.6 — an exotic parent effort routed to a strict provider could 400 the review. Mirrors the existing 'not _routed' gate on _cached_system_prompt / session_start three lines below. - Add a routed-path regression test asserting reasoning_config is omitted from the fork kwargs when _resolve_review_runtime returns routed=True. - Extract the four copy-pasted recorder stubs in test_background_review_cache_parity.py into a single _make_recorder_class() factory so a new fork attribute needs one stub edit, not four. * test(telegram): define polling progress contract * fix(telegram): gate polling health on getUpdates progress * chore(release): map @Roseyco-management in AUTHOR_MAP For PR #63581 salvage (telegram: require getUpdates progress before polling is healthy). SilentKnight87 uses a noreply GitHub email which auto-skips. * test(telegram): guard PTB integration tests with importorskip CI test slices don't install python-telegram-bot (optional dep), causing a ModuleNotFoundError on collection. Add pytest.importorskip('telegram') before the PTB imports. * chore(release): map arnispiekus in AUTHOR_MAP For PR #63581 salvage (telegram: require getUpdates progress before polling is healthy). * fix(desktop): clear stale compaction status across session switches (#64127) * fix(desktop): clear stale compaction status Clear the compaction phase when a turn resumes with model or tool activity, and key response timers by session and turn so switching chats preserves elapsed time.\n\nSupersedes #48115 by porting its resumed-content approach to the current split stream hook and covering tool-first resumptions.\n\nCo-authored-by: liuhao1024 <[email protected]> * fix(desktop): resume after thinking activity * fix(desktop): clear turn timer on stop * test: remove flaky test_crashed_runner_produces_error_completion (#64431) Flaked 3 times today across 3 unrelated PRs (#64321, #64319, #64409), on two different CI shards (slice 1 and slice 8), while passing deterministically on local runs of the same SHAs. The test polls process_registry.completion_queue for 5s waiting for a daemon-thread completion event; since the durable completion delivery work (67f4e1b4a, d0e9a42ce) the crashed-runner path also writes through the sqlite-backed persistence layer, and on slow CI runners the in-memory enqueue can lose the 5s race. Coverage note: the durable-delivery suite in this file covers the completed-runner and submit-failure paths through persistence, but not a runner that raises mid-flight — that specific path loses its direct test with this removal. A deterministic (non-racing) replacement can follow separately if wanted. * fix(telegram): respect rich_messages config for pipe table routing Remove the pipe-table bypass from _rich_delivery_enabled() so that rich_messages: false is fully honoured. Previously, pipe tables were auto-routed to sendRichMessage regardless of the config flag, breaking delivery on clients without Bot API 10.1 support (AyuGram, Telegram Web, some desktop clients). Fixes #53824 * fix(agent): gate Telegram rich-Markdown hint on rich_messages config The platform hint in PLATFORM_HINTS['telegram'] always encouraged rich Markdown constructs (tables, task lists, math, collapsible details) even when rich_messages: false (the default). This caused the agent to produce formatting that MarkdownV2 cannot render, especially broken on Telegram Web. Split the hint into a base hint (MarkdownV2-compatible) and a TELEGRAM_RICH_MESSAGES_HINT extension. The extension is conditionally appended in system_prompt.py only when platforms.telegram.extra.rich_messages is true. Fixes #57122 * refactor(telegram): drop dead _content_is_pipe_table_primary helper After #53825's fix removed the auto-rich table bypass from _rich_delivery_enabled(), _content_is_pipe_table_primary() had zero callers. Remove it and simplify _rich_delivery_enabled() to the bare rich_messages opt-in check (content param no longer used). * fix: drop empty user turns from MoA advisory view (strict-provider 400) MoA's _reference_messages() unconditionally appended every user-role message to the advisory view sent to reference models, even when the message content was an empty string or a non-string/multimodal payload that the text-extraction step flattens to "". Strict providers (Kimi/Moonshot, and others that enforce non-empty user content) reject such a message with: 400 Invalid request: the message at position N with role 'user' must not be empty Lenient providers (DeepSeek) accept it, so an identical rendered view passes on one reference and 400s on another within the same fan-out — the user sees "kimi doesn't support MoA" when the real cause is an empty user turn leaking into the advisory transcript. Skip empty user turns, mirroring the existing behavior for empty assistant turns (which are already dropped when they carry no parts). The end-on-user invariant is preserved: the synthetic advisory-request user turn is still appended when the view would otherwise end on an assistant turn. Adds a regression test asserting the advisory view contains no empty user turn and still ends on a user turn. * fix(moa): scope the non-text placeholder to structured content only Follow-up to the cherry-picked empty-user-turn drop: the placeholder introduced in 8582f35d9 fired for whitespace-only STRING turns too (content=' ' flattens to non-stripping text but isn't in the (None, '', []) exclusion set), fabricating an attachment note for a turn that carried nothing. Gate the placeholder on isinstance(content, list) so only genuinely structured (e.g. image-only) turns get it; empty and whitespace-only string turns now fall through to the drop path. Edge cases verified: trailing empty user turn still ends the view on the synthetic advisory marker; an all-empty transcript degenerates to []. * chore: add neo-claw-bot to AUTHOR_MAP (PR #58465 salvage) * fix(auth): route session refresh with provider hint cookie * fix(auth): preserve provider fallback during refresh * chore(release): map unsupportedpastels in AUTHOR_MAP * feat(agent): add Upstage Solar as a model provider Adds Upstage Solar as a bundled model-provider plugin. Solar exposes an OpenAI-compatible chat-completions endpoint at https://api.upstage.ai/v1, so the generic chat_completions transport handles request/response/streaming/tool calls — the profile is the core integration. Provider registration (Upstage isn't in models.dev, so each registry that does not auto-wire from the plugin layer needs an explicit entry — same pattern as nvidia/gmi): - plugins/model-providers/upstage/: UpstageProfile + plugin.yaml. Picker default and offline catalog list only the agentic Solar Pro models, led by `solar-pro` (rolling alias for the latest Pro). default_aux_model empty so aux tasks use the main model. `solar` alias. UPSTAGE_BASE_URL overrides the host. - hermes_cli/providers.py: HERMES_OVERLAYS + label + `solar` alias, so resolve_provider_full('upstage') resolves (without this, an explicit `provider: upstage` in config was dropped and fell through to auto-detect). - hermes_cli/auth.py: PROVIDER_REGISTRY entry + `solar` alias, so `hermes doctor` / resolve_provider recognise upstage (the static-registry path the lazy profile-extension doesn't reliably cover at validation time). - hermes_cli/models.py: CANONICAL_PROVIDERS entry places Upstage Solar in the curated picker order (above the auto-appended `custom`). - agent/model_metadata.py: context-window fallbacks (/v1/models omits context_length); `solar-pro` carries the 128K Pro context as the catch-all. Reasoning: UpstageProfile.build_api_kwargs_extras wires Solar's top-level `reasoning_effort` (low|medium|high; xhigh/max→high). Reasoning-capable families are solar-pro* and solar-open*; solar-mini/syn-pro never receive it. Defaults ON at medium when unset (matches the /reasoning "medium (default)" label); `/reasoning none` disables; explicit/saved settings are honored. No reasoning_content echo handling needed (unlike DeepSeek/Kimi). Web dashboard: - web/src/pages/EnvPage.tsx: add an "Upstage Solar" provider group so UPSTAGE_API_KEY / UPSTAGE_BASE_URL appear under LLM Providers (not "Other"). Docs/tests: - .env.example: documents UPSTAGE_API_KEY / UPSTAGE_BASE_URL. - tests: profile wiring, reasoning_effort mapping (pro/open/mini, efforts, disabled, default-on), provider-resolver regression (resolve_provider_full / get_provider / solar alias / overlay), `solar-pro` default. Testing: pytest tests/providers tests/plugins/model_providers tests/hermes_cli/test_upstage_provider.py tests/run_agent/test_provider_parity.py tests/hermes_cli/test_api_key_providers.py; ruff clean. Verified end-to-end: `hermes doctor` shows "Upstage Solar", and live chat works via both `--provider upstage` and `--provider solar`. Reasoning wire format per https://console.upstage.ai/api/docs/for-agents/raw. Platforms tested: macOS. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * refactor(agent): treat unknown Solar models as reasoning-capable Invert the reasoning-support check from an allow-list (solar-pro, solar-open) to a deny-list of the known non-reasoning families (solar-mini, syn-pro). Newly released Solar models now get reasoning_effort by default instead of having it silently dropped. Co-Authored-By: Claude Fable 5 <[email protected]> * fix(agent): register Upstage keys in the env-var catalog `UPSTAGE_API_KEY` / `UPSTAGE_BASE_URL` were wired through the provider resolver, auth registry, and the EnvPage grouping, but never added to `OPTIONAL_ENV_VARS` in hermes_cli/config.py. The dashboard/desktop Providers page builds its list from that catalog (`/api/env` iterates `OPTIONAL_ENV_VARS`), so with no entry the keys were never emitted and "Upstage Solar" never rendered — the EnvPage prefix group stayed empty. Add both keys under `category: "provider"` (matching gmi/minimax) so they show up in `hermes dashboard` / `hermes desktop` under "Upstage Solar". Adds a regression test asserting the catalog contains them, mirroring the existing GMI coverage. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * refactor(agent): drop solar-open2-preview from Solar context fallbacks Remove the `solar-open2-preview` context-window entry; `solar-open2` covers the Open 2 family at the same 256K window. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * refactor(agent): drop the solar-pro rolling alias, default to solar-pro3 Pin the Upstage default to the concrete solar-pro3 instead of the solar-pro rolling alias: - plugin fallback_models is now ("solar-pro3",); entry [0] is the setup default - drop the "solar-pro" context-window fallback entry (solar-pro3 covers it) - update the reasoning default-on docstring and profile tests accordingly Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * refactor(upstage): drop manual auth/models registrations covered by profile auto-extend PROVIDER_REGISTRY, its alias map, and CANONICAL_PROVIDERS all auto-extend from registered ProviderProfiles since the provider-modules refactor (20a4f79ed). Verified with real imports: registry entry, 'solar' alias resolution via resolve_provider(), and the picker entry are identical with the manual entries removed. The hermes_cli/providers.py overlay stays (models.dev has a stale /v1/solar base URL and no UPSTAGE_BASE_URL var), and the manual OPTIONAL_ENV_VARS entries stay (non-advanced key + curated prompt text, matching the fireworks convention). * chore: add [email protected] to AUTHOR_MAP (minchang, PR #42231) * fix(upstage): map 'ultra' reasoning effort to Solar's high Main added max/ultra effort levels (#62650) after this PR branched; without the mapping 'ultra' silently fell through to the medium default. Matches the xhigh/max collapse-to-strongest convention used by other profiles. * docs: add Upstage Solar to provider docs (env vars, fallback table, --provider list) * fix(upstage): collapse unknown future efforts to high; behavior-contract tests Review findings from the 4-angle pass: - Unknown-but-enabled effort levels now collapse to Solar's strongest (high) instead of silently downgrading to the medium default — guards against the next #62650-style vocabulary addition. Explicit-empty effort keeps the medium default. - fallback_models test now asserts the behavior contract (non-empty, no denied families) instead of freezing the exact model tuple (change-detector, AGENTS.md reject reason). - Drop unused pytest import in test_upstage_provider.py. * feat(config): support per-model reasoning_effort overrides Add agent.reasoning_overrides dict to config.yaml. Users can now set a reasoning_effort per model, overriding the global agent.reasoning_effort. Example: agent: reasoning_effort: "medium" # global default reasoning_overrides: "openrouter/anthropic/claude-opus-4.5": "xhigh" "openai/gpt-5": "low" "claude-sonnet-4.6": "high" # bare model name also works The helper is spelling-tolerant: override keys match regardless of provider prefix or dots-vs-dashes normalization, so users can write keys in any sensible form and they'll match. Resolution priority: 1. Session-scoped /reasoning --session override (gateway only; unchanged) 2. Per-model override from agent.reasoning_overrides (spelling-tolerant) 3. Global agent.reasoning_effort (existing) 4. Provider default (unchanged) Wired into: - CLI startup (cli.py) - Messaging gateway agent construction (gateway/run.py) - Desktop/TUI _load_reasoning_config (tui_gateway/server.py) - Cron job scheduler (cron/scheduler.py) - /model mid-session switch (agent/agent_runtime_helpers.py) + _primary_runtime now tracks reasoning_config for correct fallback recovery - Fallback activation (agent/chat_completion_helpers.py::try_activate_fallback) + Re-resolves reasoning_config for the fallback model (best-effort) Closes #21256 (per-model reasoning_effort defaults). Note: no hermes config set agent.reasoning_overrides.<model> support; users edit the YAML directly. _set_nested splits on "." and would corrupt model keys containing version dots. * refactor(reasoning): unify per-model reasoning resolution behind a single chokepoint Collapse the six per-surface copies of override-then-global resolution (CLI startup, gateway, TUI, cron, /model switch, fallback activation) onto one shared resolve_reasoning_config() in hermes_constants. Also fixes the gateway resolving reasoning against config model.default instead of the session's effective model: after a session-only /model switch, the switched model's override now applies (gateway message paths pass the resolved session model through _resolve_session_reasoning_config; /reasoning status reads the session model override). Cleanup: drop docs/PER_MODEL_REASONING.md (duplicates the website docs page), drop the change-detector _config_version test (no bump needed — deep-merge handles new keys), remove a stale plan-reference comment. Adds chokepoint contract tests (13) and gateway session-effective-model regression tests (2). * test: update stale _load_reasoning_config mocks for new model parameter Two test mocks stubbed the old zero-arg signature; the chokepoint refactor added an optional model param that call sites now pass. Swept the full test tree for other stale stubs of the changed functions — the rest use MagicMock/patch(return_value=...), which tolerate the new arg. * perf(agent): segment mixed tool batches to recover lost concurrency (#64460) A model response containing several parallel-safe reads plus one unsafe tool used to lose ALL concurrency: _should_parallelize_tool_batch was all-or-nothing, so a single barrier call (terminal, clarify, unknown tool, malformed args) forced the entire batch onto the sequential path. _plan_tool_batch_segments now splits the batch into ordered segments: maximal contiguous runs of parallel-safe calls execute on the existing concurrent path, barrier calls on the sequential path, strictly in the model's emission order. Invariants preserved: - one tool result per call, appended in emission order (segments are contiguous, so no result reordering across a barrier) - side-effect boundaries: no call starts before an earlier barrier ends - overlapping file targets split into separate ordered parallel runs - turn-end budget enforcement + /steer injection run exactly once per batch (segment executors run with finalize=False; the segmented dispatcher owns the whole-turn finalize) - interrupt during segment k drains segments k+1..n with cancelled results, keeping one result per tool_call_id Homogeneous batches keep their original single-path dispatch (zero behavior delta); _should_parallelize_tool_batch remains as a thin view over the planner for existing callers and tests. * fix(terminal): ignore stale env.cwd from a different session's cd The terminal environment is shared process-globally (collapsed to the default key), so env.cwd tracks the LAST session that ran a command. _resolve_command_cwd() trusted env.cwd unconditionally — no ownership check — so when session A left env.cwd pointing at A's checkout, session B's first terminal command inherited A's stale cwd and ran in the wrong workspace. The file tools already solved this exact shared-env problem with _live_cwd_if_owned() checking env.cwd_owner. The terminal tool never got the same guard. Fix: capture env.cwd_owner BEFORE the current session claims it, and pass it as prev_owner to _resolve_command_cwd. When the previous owner was a different session, env.cwd is stale — fall through to default_cwd (the config/override cwd for this session) instead. Once the session has claimed the env, subsequent calls in the same session still trust env.cwd so in-session state survives. * fix(desktop): clear the transcript on every cold resume so sessions can't share one resumeSession hand-rolls $messages (it paints before a runtime id is bound), and only cleared the old transcript on the cold path at entry. But a warm-cache hit can bail down to the full resume — an empty-transcript drop, or the cache being purged during the profile-swap await — without ever clearing, so the previous session's array leaked into the next one. Symptom: switching sessions kept showing the same messages (deterministic once tiling pre-warms the cache on boot). Clear $messages at the single point every cold/bail path converges, so carryover is structurally impossible; the warm fast-path still repaints in place. * feat(desktop): add a chat backdrop on/off toggle The faint statue backdrop behind the transcript was only switchable via the DEV-only leva panel. Add a persisted Appearance toggle (default on) so users can hide it; the Backdrop simply skips rendering when off. * fix(dashboard): pass backup output with -o * feat(slack): support agent view manifests * feat(slack): cover agent view assistant APIs * fix(slack): complete agent view workspace routing * fix(slack): scope Agent View workspace state * fix(slack): clear uniquely scoped assistant status * fix(slack): gate feedback buttons behind rich_blocks as documented The docs state feedback_buttons requires rich_blocks: true, but _maybe_blocks rendered full Block Kit whenever feedback_buttons alone was enabled — implicitly turning on rich-block rendering the user never opted into. Align the code with the documented contract and add a regression test. * chore(slack): bump slack-bolt to 1.29.0 and slack-sdk to 3.43.0 Slack's June 30 Agent messaging experience changelog lists Bolt Python 1.29.0 / Python SDK 3.43.0 as the Agent View minimums. Bump the messaging/slack extras and the platform.slack lazy-install pins to match, and regenerate uv.lock. All adapter API surfaces verified present against the new versions in a clean venv. * fix(mcp): materialize ResourceLink/EmbeddedResource/Audio blocks instead of dropping them MCP tool results with non-image binary resources (PDFs, archives, office docs) were silently dropped: the success path only handled TextContent and ImageContent, so a PDF-returning MCP tool appeared to return metadata only. - EmbeddedResource blob contents are decoded (50MB cap), materialized into the Hermes document cache via cache_document_from_bytes (sanitized filename, traversal-safe), and surfaced as a local-path marker the agent can read with file/terminal tools. - EmbeddedResource text contents are inlined directly. - ResourceLink blocks preserve the URI and point the agent at the server's read_resource tool; no arbitrary network fetch outside the MCP session. - AudioContent blocks are cached via cache_audio_from_bytes as MEDIA: tags. - read_resource blob contents are materialized the same way instead of returning '[binary data, N bytes]'. - Unsupported blocks are logged instead of silently discarded. - Existing ImageContent MEDIA: behavior unchanged. Reported by an enterprise customer; reproduced against an HTTP MCP server returning application/pdf resources. * fix(mcp): use real wire name in ResourceLink marker + surface resource text in isError path Follow-ups on top of #64061's salvage: - ResourceLink markers now point at mcp__<server>__read_resource (the actual registered tool name via mcp_prefixed_tool_name) instead of a nonexistent <server>_read_resource the agent could hallucinate-call. - The isError path now surfaces EmbeddedResource .resource.text blocks instead of dropping them, so error payloads carried in resources no longer collapse to a bare 'MCP tool returned an error'. (Same-class fix flagged in #64061 and independently addressed in #63576 by @alauer.) - 3 new error-path tests + updated ResourceLink wire-name assertion. * refactor(desktop): trim backdrop store to match tool-view style * feat(auxiliary): per-task reasoning_effort for auxiliary models (#64597) Every auxiliary task block (vision, web_extract, compression, title_generation, curator, background_review, moa_reference, ...) now accepts a reasoning_effort shorthand: auxiliary: compression: reasoning_effort: low vision: reasoning_effort: none _get_task_extra_body() folds it into extra_body.reasoning, which every auxiliary wire already translates: chat.completions passes it through, the Codex Responses adapter maps it to top-level reasoning/include, and the Anthropic auxiliary adapter now forwards it into build_anthropic_kwargs(reasoning_config=...) (previously hardcoded None). An explicit extra_body.reasoning on the same task wins over the shorthand. Invalid levels are ignored with a warning. Empty string (the shipped default) is a no-op — zero behavior change. Config: reasoning_effort added to all 16 auxiliary task blocks in DEFAULT_CONFIG (no version bump — deep-merge handles new keys). * fix(desktop): full-reset the thread runtime on a disjoint transcript swap The incremental external-store runtime reconciles message repositories in place (addOrUpdateMessage + prune-non-incoming). On a session switch the incoming transcript shares no ids with the current one, and grafting the new chain onto the old tree before pruning can strand a stale head/branch — the thread keeps showing the previous session. When nothing carries over there's nothing to preserve, so clear the tree first (leaves→root) then rebuild clean. Belt-and- suspenders alongside the $messages-carryover fix. * fix(cron/chronos): cache PyJWKClient across fires to stop JWKS fetch storm (#64641) The inbound cron-fire verifier constructed a fresh PyJWKClient on every fire, discarding the client's key cache and forcing a synchronous JWKS HTTP GET to the portal on each fire. Under a burst of concurrent fires (a hosted instance with several cron jobs firing in the same window) this fanned out into N simultaneous JWKS fetches that the portal rate-limited (HTTP 403 -> verification fails -> agent 401), or that blocked the event loop long enough that the fire webhook could not return its 202 before the relay's 30s timeout (observed in prod as relay 504s concentrated on high-job-count instances). Cache one PyJWKClient per JWKS URL at module scope (double-checked lock) so the signing keys are reused across fires; NAS keys rotate rarely, so the steady state is zero JWKS fetches per fire. Regression test proves 5 fires -> 1 client construction (was 5). * feat(relay): consume channel context from the connector (#64649) Phase 3 of relay-channel-context (gateway/agent side, single PR). The connector (gateway-gateway #122/#123/#124) now attaches read-only surrounding channel/group context to an addressed relay turn; this wires the gateway to consume it. - descriptor.py: additive optional supports_context (default False) on CapabilityDescriptor. from_json already filters unknown keys, so this is back-compat both directions within contract_version 1. - ws_transport.py: _event_from_wire maps the connector's read-only context[] array into the EXISTING MessageEvent.channel_context field via a new _render_relay_context() helper — reusing the same read-only injection path history-backfill uses (run.py prepends channel_context ahead of the trigger message). Never raises; absent/empty/malformed -> channel_context unset (byte-identical to today). - docs/relay-connector-contract.md: document supports_context in the §2 descriptor table (fixes the contract-doc conformance test) + the context/context_error inbound fields in §3. - tests: descriptor default/round-trip/forward-compat; _render_relay_context rendering + malformed-safe; …
iykwak10-sys
pushed a commit
to iykwak10-sys/hermes-agent
that referenced
this pull request
Jul 16, 2026
… Anthropic cache namespace PR NousResearch#17276 painstakingly pinned `_cached_system_prompt`, `session_start`, `session_id`, and the toolset config on the background-review fork so its outbound request body would byte-match the parent's and hit Anthropic's exact-prefix cache. The contributor measured a ~26% end-to-end cost reduction on Sonnet 4.5. That optimization is currently being silently undone by a missing `reasoning_config` kwarg. The fork's `AIAgent(...)` call omits it, so the fork's `reasoning_config` defaults to `None`. `anthropic_adapter.build_anthropic_kwargs` (line ~2165) then short-circuits the `thinking` / `output_config` block, and the fork's request body lands in a DIFFERENT Anthropic cache namespace from the parent's. Result on the wire: 0 `cache_read_input_tokens`, full `cache_creation_input_tokens` of the entire parent prefix — every single background review. 7 days of midagent.db traffic from one host running stock Hermes against Anthropic Sonnet: ``` Background-review FIRST calls (the moment a review fork is born): count = 68 cache_write tokens = 7,004,297 cache_read tokens = 1,016,335 Cost on Sonnet ($3.75/M write vs $0.30/M read): Spent on these writes: $26.27 Cost if they had hit parent cache instead: $2.10 WASTED: $24.16 / week / user ``` That is from one user. Multiply by Hermes's installed base for the full impact. Tested against api.anthropic.com directly (see refs/api-tests/ in the attached investigation repo if needed): | pair | cache_r | cache_w | |---------------------------------------------|---------|---------| | parent fresh | 0 | 24,047 | | parent same again | 24,047 | 0 | | fork: appends 2 new tail msgs, thinking ON | 24,047 | 22 | | fork: appends 2 new tail msgs, thinking OFF | 0 | 24,047 | Same fork-shape request, only difference is `thinking`. With the fix, the fork hits the parent's full prefix and only writes the delta (the `Review the conversation above…` prompt block, ~3-5K tokens). One line in `agent/background_review.py`: pass `reasoning_config=getattr(agent, "reasoning_config", None)` to the `AIAgent(...)` constructor of the review fork. A short comment block above it explains why so the next person who reads this code doesn't re-introduce the regression. `tests/run_agent/test_background_review_cache_parity.py` already covers the system-prompt / session-id / toolset-config parity contracts that PR NousResearch#17276 introduced. I added: * a `reasoning_config` attribute to `_make_agent_stub` so the stub has a non-None parent value the test can verify is propagated. * `test_review_fork_inherits_parent_reasoning_config()` — asserts the fork's `AIAgent(...)` kwargs carry the parent's `reasoning_config`. Pre-fix this test fails with `None vs expected {'enabled': True, 'effort': 'medium'}`; post-fix all 4 tests in the file pass. ``` $ python -m pytest tests/run_agent/test_background_review_cache_parity.py -v test_review_fork_inherits_parent_cached_system_prompt PASSED test_review_fork_pins_session_start_and_session_id PASSED test_review_fork_inherits_parent_toolset_config PASSED test_review_fork_inherits_parent_reasoning_config PASSED ← new ``` Also runs against the broader background-review test suite: `test_background_review.py` (4), `test_background_review_summary.py` (8), `test_background_review_toolset_restriction.py` (3) — 19/19 pass. `agent/curator.py:1691` has the same omission for the umbrella-curation fork, but curator's prompt is "curate all skills" — it shares no prefix with any user conversation, so cache-parity is a non-issue there. Worth auditing if the curator ever takes a parent conversation as input, but not part of this PR. The `agent/auxiliary_client.py:1006` `reasoning_config=None` hardcode is intentional (title/summary one-shots on short prompts — per-call cost of namespace flip is negligible) and is also out of scope.
kulikman
pushed a commit
to kulikman/hermes-agent
that referenced
this pull request
Jul 16, 2026
kulikman
pushed a commit
to kulikman/hermes-agent
that referenced
this pull request
Jul 16, 2026
…view fork - test_background_review_does_not_narrow_toolset_schema: review fork must NOT pass enabled_toolsets to AIAgent (full parent schema = matching Anthropic cache key on the 'tools' field). - test_background_review_installs_thread_local_whitelist: the runtime whitelist that replaces schema-level narrowing must contain memory + skills tools and exclude terminal / send_message / delegate_task / web_search / execute_code. - test_review_fork_inherits_parent_cached_system_prompt: new test for PR NousResearch#17276's first root cause — the fork's _cached_system_prompt must equal the parent's byte-for-byte. - test_review_fork_pins_session_start_and_session_id: defensive belt-and- suspenders for the cached-prompt inheritance. Inverted the original test_background_review_agent_uses_restricted_toolsets (which asserted the schema-level narrowing) — that narrowing was the direct cause of NousResearch#25322's cache miss, and the runtime whitelist replaces its safety claim without breaking cache parity. Refs NousResearch#25322, NousResearch#15204, PR NousResearch#17276.
kulikman
pushed a commit
to kulikman/hermes-agent
that referenced
this pull request
Jul 16, 2026
…[] 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 NousResearch#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 NousResearch#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 NousResearch#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#1 (root-cause analysis with wire-level cache-write measurements). Extends PR NousResearch#17276's `system`-bytes invariant to the `tools[]` slot.
Gravezzz
pushed a commit
to Gravezzz/hermes-agent
that referenced
this pull request
Jul 21, 2026
Gravezzz
pushed a commit
to Gravezzz/hermes-agent
that referenced
this pull request
Jul 21, 2026
…view fork - test_background_review_does_not_narrow_toolset_schema: review fork must NOT pass enabled_toolsets to AIAgent (full parent schema = matching Anthropic cache key on the 'tools' field). - test_background_review_installs_thread_local_whitelist: the runtime whitelist that replaces schema-level narrowing must contain memory + skills tools and exclude terminal / send_message / delegate_task / web_search / execute_code. - test_review_fork_inherits_parent_cached_system_prompt: new test for PR NousResearch#17276's first root cause — the fork's _cached_system_prompt must equal the parent's byte-for-byte. - test_review_fork_pins_session_start_and_session_id: defensive belt-and- suspenders for the cached-prompt inheritance. Inverted the original test_background_review_agent_uses_restricted_toolsets (which asserted the schema-level narrowing) — that narrowing was the direct cause of NousResearch#25322's cache miss, and the runtime whitelist replaces its safety claim without breaking cache parity. Refs NousResearch#25322, NousResearch#15204, PR NousResearch#17276.
Gravezzz
pushed a commit
to Gravezzz/hermes-agent
that referenced
this pull request
Jul 21, 2026
…[] 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 NousResearch#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 NousResearch#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 NousResearch#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#1 (root-cause analysis with wire-level cache-write measurements). Extends PR NousResearch#17276's `system`-bytes invariant to the `tools[]` slot.
Gravezzz
pushed a commit
to Gravezzz/hermes-agent
that referenced
this pull request
Jul 21, 2026
… Anthropic cache namespace PR NousResearch#17276 painstakingly pinned `_cached_system_prompt`, `session_start`, `session_id`, and the toolset config on the background-review fork so its outbound request body would byte-match the parent's and hit Anthropic's exact-prefix cache. The contributor measured a ~26% end-to-end cost reduction on Sonnet 4.5. That optimization is currently being silently undone by a missing `reasoning_config` kwarg. The fork's `AIAgent(...)` call omits it, so the fork's `reasoning_config` defaults to `None`. `anthropic_adapter.build_anthropic_kwargs` (line ~2165) then short-circuits the `thinking` / `output_config` block, and the fork's request body lands in a DIFFERENT Anthropic cache namespace from the parent's. Result on the wire: 0 `cache_read_input_tokens`, full `cache_creation_input_tokens` of the entire parent prefix — every single background review. 7 days of midagent.db traffic from one host running stock Hermes against Anthropic Sonnet: ``` Background-review FIRST calls (the moment a review fork is born): count = 68 cache_write tokens = 7,004,297 cache_read tokens = 1,016,335 Cost on Sonnet ($3.75/M write vs $0.30/M read): Spent on these writes: $26.27 Cost if they had hit parent cache instead: $2.10 WASTED: $24.16 / week / user ``` That is from one user. Multiply by Hermes's installed base for the full impact. Tested against api.anthropic.com directly (see refs/api-tests/ in the attached investigation repo if needed): | pair | cache_r | cache_w | |---------------------------------------------|---------|---------| | parent fresh | 0 | 24,047 | | parent same again | 24,047 | 0 | | fork: appends 2 new tail msgs, thinking ON | 24,047 | 22 | | fork: appends 2 new tail msgs, thinking OFF | 0 | 24,047 | Same fork-shape request, only difference is `thinking`. With the fix, the fork hits the parent's full prefix and only writes the delta (the `Review the conversation above…` prompt block, ~3-5K tokens). One line in `agent/background_review.py`: pass `reasoning_config=getattr(agent, "reasoning_config", None)` to the `AIAgent(...)` constructor of the review fork. A short comment block above it explains why so the next person who reads this code doesn't re-introduce the regression. `tests/run_agent/test_background_review_cache_parity.py` already covers the system-prompt / session-id / toolset-config parity contracts that PR NousResearch#17276 introduced. I added: * a `reasoning_config` attribute to `_make_agent_stub` so the stub has a non-None parent value the test can verify is propagated. * `test_review_fork_inherits_parent_reasoning_config()` — asserts the fork's `AIAgent(...)` kwargs carry the parent's `reasoning_config`. Pre-fix this test fails with `None vs expected {'enabled': True, 'effort': 'medium'}`; post-fix all 4 tests in the file pass. ``` $ python -m pytest tests/run_agent/test_background_review_cache_parity.py -v test_review_fork_inherits_parent_cached_system_prompt PASSED test_review_fork_pins_session_start_and_session_id PASSED test_review_fork_inherits_parent_toolset_config PASSED test_review_fork_inherits_parent_reasoning_config PASSED ← new ``` Also runs against the broader background-review test suite: `test_background_review.py` (4), `test_background_review_summary.py` (8), `test_background_review_toolset_restriction.py` (3) — 19/19 pass. `agent/curator.py:1691` has the same omission for the umbrella-curation fork, but curator's prompt is "curate all skills" — it shares no prefix with any user conversation, so cache-parity is a non-issue there. Worth auditing if the curator ever takes a parent conversation as input, but not part of this PR. The `agent/auxiliary_client.py:1006` `reasoning_config=None` hardcode is intentional (title/summary one-shots on short prompts — per-call cost of namespace flip is negligible) and is also out of scope.
waym0reom3ga
added a commit
to waym0reom3ga/autolycus-agent
that referenced
this pull request
Jul 21, 2026
waym0reom3ga
added a commit
to waym0reom3ga/autolycus-agent
that referenced
this pull request
Jul 21, 2026
…view fork - test_background_review_does_not_narrow_toolset_schema: review fork must NOT pass enabled_toolsets to AIAgent (full parent schema = matching Anthropic cache key on the 'tools' field). - test_background_review_installs_thread_local_whitelist: the runtime whitelist that replaces schema-level narrowing must contain memory + skills tools and exclude terminal / send_message / delegate_task / web_search / execute_code. - test_review_fork_inherits_parent_cached_system_prompt: new test for PR NousResearch#17276's first root cause — the fork's _cached_system_prompt must equal the parent's byte-for-byte. - test_review_fork_pins_session_start_and_session_id: defensive belt-and- suspenders for the cached-prompt inheritance. Inverted the original test_background_review_agent_uses_restricted_toolsets (which asserted the schema-level narrowing) — that narrowing was the direct cause of NousResearch#25322's cache miss, and the runtime whitelist replaces its safety claim without breaking cache parity. Refs NousResearch#25322, NousResearch#15204, PR NousResearch#17276.
waym0reom3ga
added a commit
to waym0reom3ga/autolycus-agent
that referenced
this pull request
Jul 21, 2026
…[] 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 NousResearch#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 NousResearch#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 NousResearch#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#1 (root-cause analysis with wire-level cache-write measurements). Extends PR NousResearch#17276's `system`-bytes invariant to the `tools[]` slot.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background review fork is supposed to hit Anthropic's prefix cache on the parent's
messages_snapshot, but currently doesn't (cache_read=0on every fork). This PR fixes that.Real E2E on Sonnet 4.5 (single run: model reads 12 files via 12
read_filecalls, then_spawn_background_reviewfires naturally on skill-nudge):cache_create/cache_readTwo root causes, one fix each.
Root cause #1 — System prompt rebuilt at fork time → 1-character timestamp drift
The fork's
_cached_system_promptstarts asNone, sorun_conversationrebuilds via_build_system_prompt, which embedsConversation started: Wednesday, April 29, 2026 12:58 AMat%I:%M %p(minute precision). Reviews fire 10+ turns after session start, so the minute almost always differs from main's — one character diverges, byte-exact cache key misses, entire system prefix invalidated.Fix: inherit the parent's cached prompt directly.
(Same idea as #17089, which I self-closed because it only fixed half the problem.)
Root cause #2 — Narrowed tools schema → cache key mismatch on
toolsThe fork uses
enabled_toolsets=["memory","skills"](4 tools) for safety. Anthropic's cache key includestools, which sits beforesystemin the hierarchy, so even byte-identicalsystemwon't hit whentoolsdiffers from main's 16-tool set.Fix: drop the schema-level restriction so
toolsmatches main, and deny non-whitelisted tools at runtime via the existingget_pre_tool_call_block_messagegate (hermes_cli/plugins.py:1085, already called at all 3 dispatch sites). A small thread-local whitelist is added on top; the fork installs it on its daemon thread beforerun_conversationand clears it after.Diff
hermes_cli/plugins.pyrun_agent.py_spawn_background_reviewenabled_toolsets; share_cached_system_prompt; install/clear whitelist; soft constraint in prompttests/hermes_cli/test_plugins.pypytest tests/hermes_cli/test_plugins.py→ 62/62 passed. Tested on macOS 14 / Python 3.12.