diff --git a/crates/headroom-core/src/ccr/backends/sqlite.rs b/crates/headroom-core/src/ccr/backends/sqlite.rs index 906ecfca4..987fcbddb 100644 --- a/crates/headroom-core/src/ccr/backends/sqlite.rs +++ b/crates/headroom-core/src/ccr/backends/sqlite.rs @@ -16,8 +16,8 @@ //! The TTL is an **idle window** (#2604): every successful `get` //! restarts the row's clock via `last_accessed`, bounded by an absolute //! max lifetime measured from `created_at`. On every `get` we -//! lazy-purge stale rows (`WHERE last_accessed + ttl_seconds <= now OR -//! created_at + max_lifetime <= now`) — no background reaper thread, +//! lazy-purge stale rows (`WHERE last_accessed + ttl_seconds < now OR +//! created_at + max_lifetime < now`) — no background reaper thread, //! no cron. DBs created by pre-sliding builds are migrated in place //! (the `last_accessed` column is added, backfilled from `created_at`). //! @@ -153,10 +153,13 @@ impl SqliteCcrStore { /// absolute max lifetime. Lazy — invoked from `get`. Returns the /// number of rows purged. fn purge_expired(&self, conn: &Connection, now: u64) -> rusqlite::Result { + // Timestamps have whole-second resolution. Use a strict boundary so + // truncation can extend a cache entry by less than one second but can + // never expire it before the configured idle or lifetime window. let purged = conn.execute( "DELETE FROM ccr_entries - WHERE last_accessed + ttl_seconds <= ?1 - OR created_at + ?2 <= ?1", + WHERE last_accessed + ttl_seconds < ?1 + OR created_at + ?2 < ?1", params![now as i64, self.max_lifetime_seconds as i64], )?; Ok(purged) @@ -170,6 +173,58 @@ impl SqliteCcrStore { .map(|d| d.as_secs()) .unwrap_or(0) } + + fn get_at(&self, hash: &str, now: u64) -> Option { + let conn = self.conn.lock().expect("ccr sqlite mutex poisoned"); + + // Lazy purge sweep, then the real lookup. Both happen under + // the same mutex so the row we read is guaranteed not to have + // been just-deleted by another caller. + if let Err(err) = self.purge_expired(&conn, now) { + tracing::warn!( + target = "ccr.sqlite", + error = %err, + "ccr_sqlite_purge_failed" + ); + } + + let row: Option> = conn + .query_row( + "SELECT original FROM ccr_entries + WHERE hash = ?1 + AND last_accessed + ttl_seconds >= ?2 + AND created_at + ?3 >= ?2", + params![hash, now as i64, self.max_lifetime_seconds as i64], + |r| r.get::<_, Vec>(0), + ) + .optional() + .unwrap_or_else(|err| { + tracing::warn!( + target = "ccr.sqlite", + hash = %hash, + error = %err, + "ccr_sqlite_get_failed" + ); + None + }); + + let row = row?; + // Sliding idle window (#2604): a successful hit restarts the + // row's idle clock. Still under the same mutex as the lookup. + if let Err(err) = conn.execute( + "UPDATE ccr_entries SET last_accessed = ?2 WHERE hash = ?1", + params![hash, now as i64], + ) { + tracing::warn!( + target = "ccr.sqlite", + hash = %hash, + error = %err, + "ccr_sqlite_touch_failed" + ); + } + + String::from_utf8(row).ok() + } } impl CcrStore for SqliteCcrStore { @@ -210,56 +265,7 @@ impl CcrStore for SqliteCcrStore { } fn get(&self, hash: &str) -> Option { - let now = Self::now_unix_seconds(); - let conn = self.conn.lock().expect("ccr sqlite mutex poisoned"); - - // Lazy purge sweep, then the real lookup. Both happen under - // the same mutex so the row we read is guaranteed not to have - // been just-deleted by another caller. - if let Err(err) = self.purge_expired(&conn, now) { - tracing::warn!( - target = "ccr.sqlite", - error = %err, - "ccr_sqlite_purge_failed" - ); - } - - let row: Option> = conn - .query_row( - "SELECT original FROM ccr_entries - WHERE hash = ?1 - AND last_accessed + ttl_seconds > ?2 - AND created_at + ?3 > ?2", - params![hash, now as i64, self.max_lifetime_seconds as i64], - |r| r.get::<_, Vec>(0), - ) - .optional() - .unwrap_or_else(|err| { - tracing::warn!( - target = "ccr.sqlite", - hash = %hash, - error = %err, - "ccr_sqlite_get_failed" - ); - None - }); - - let row = row?; - // Sliding idle window (#2604): a successful hit restarts the - // row's idle clock. Still under the same mutex as the lookup. - if let Err(err) = conn.execute( - "UPDATE ccr_entries SET last_accessed = ?2 WHERE hash = ?1", - params![hash, now as i64], - ) { - tracing::warn!( - target = "ccr.sqlite", - hash = %hash, - error = %err, - "ccr_sqlite_touch_failed" - ); - } - - String::from_utf8(row).ok() + self.get_at(hash, Self::now_unix_seconds()) } fn len(&self) -> usize { @@ -271,3 +277,56 @@ impl CcrStore for SqliteCcrStore { .unwrap_or(0) } } + +#[cfg(test)] +mod tests { + use super::*; + + fn store_with_row( + idle_ttl: u64, + max_lifetime: u64, + created_at: u64, + last_accessed: u64, + ) -> (tempfile::TempDir, SqliteCcrStore, String) { + let dir = tempfile::tempdir().expect("tempdir"); + let store = + SqliteCcrStore::open_with_ttls(dir.path().join("ccr.sqlite"), idle_ttl, max_lifetime) + .expect("open sqlite store"); + let hash = "boundary-entry".to_string(); + { + let conn = store.conn.lock().expect("ccr sqlite mutex poisoned"); + conn.execute( + "INSERT INTO ccr_entries + (hash, original, created_at, ttl_seconds, last_accessed) + VALUES (?1, ?2, ?3, ?4, ?5)", + params![ + &hash, + b"payload".as_slice(), + created_at as i64, + idle_ttl as i64, + last_accessed as i64, + ], + ) + .expect("insert boundary row"); + } + (dir, store, hash) + } + + #[test] + fn exact_idle_ttl_boundary_is_still_valid() { + let (_dir, store, hash) = store_with_row(5, 20, 100, 100); + + assert_eq!(store.get_at(&hash, 105).as_deref(), Some("payload")); + assert_eq!(store.get_at(&hash, 111), None); + assert_eq!(store.len(), 0, "expired row must be purged"); + } + + #[test] + fn exact_max_lifetime_boundary_is_still_valid() { + let (_dir, store, hash) = store_with_row(5, 10, 100, 108); + + assert_eq!(store.get_at(&hash, 110).as_deref(), Some("payload")); + assert_eq!(store.get_at(&hash, 111), None); + assert_eq!(store.len(), 0, "expired row must be purged"); + } +} diff --git a/headroom/config.py b/headroom/config.py index 32761d836..2b01d6bd6 100644 --- a/headroom/config.py +++ b/headroom/config.py @@ -3,6 +3,7 @@ from __future__ import annotations import fnmatch +import json from collections.abc import Iterable from dataclasses import InitVar, dataclass, field from datetime import datetime @@ -282,6 +283,37 @@ def _tool_name_aliases(name: str) -> tuple[str, ...]: return tuple(dict.fromkeys(aliases)) +# Hermes Agent's deferred-tool bridge. Hermes loads on-demand tools via a +# `tool_search`/`tool_describe`/`tool_call` indirection; on the wire the +# emitted tool call is named `tool_call` and the REAL tool name lives in the +# arguments payload (`{"name": "...", "arguments": {...}}`). Tool exclusion / +# protect lists match on the real name, so we must unwrap this bridge before +# building the tool_call_id -> name map, or whitelists silently no-op for all +# deferred tools. +_HERMES_TOOL_CALL_WRAPPER = "tool_call" + + +def unwrap_tool_call_name(name: str, arguments: Any) -> str: + """Extract the real tool name from a Hermes deferred ``tool_call`` wrapper. + + Non-wrapper names pass through unchanged. Malformed/unparseable wrappers + fail open and return the wrapper name (caller decides what that means). + """ + if name != _HERMES_TOOL_CALL_WRAPPER: + return name + raw = arguments + if isinstance(raw, str): + try: + raw = json.loads(raw) + except (ValueError, TypeError): + return name + if isinstance(raw, dict): + inner = raw.get("name") + if isinstance(inner, str) and inner.strip(): + return inner.strip() + return name + + def is_tool_excluded(name: str, exclude_tools: Iterable[str]) -> bool: """Return True if ``name`` matches the tool-exclusion set. diff --git a/headroom/proxy/handlers/openai.py b/headroom/proxy/handlers/openai.py index ffe770ef5..dd12a2290 100644 --- a/headroom/proxy/handlers/openai.py +++ b/headroom/proxy/handlers/openai.py @@ -44,6 +44,7 @@ if TYPE_CHECKING: import httpx from headroom.agent_savings import proxy_pipeline_kwargs +from headroom.config import unwrap_tool_call_name from headroom.copilot_auth import ( apply_copilot_api_auth, build_copilot_upstream_url, @@ -1638,6 +1639,10 @@ class OpenAIHandlerMixin: continue name = item.get("name") call_id = item.get("call_id") + if name: + # Hermes deferred tools arrive wrapped as `tool_call` with + # the real name inside the arguments/input payload. + name = unwrap_tool_call_name(name, item.get("arguments") or item.get("input")) if isinstance(name, str) and isinstance(call_id, str) and call_id: function_name_by_call_id[call_id] = name if isinstance(name, str) and ( diff --git a/headroom/transforms/content_router.py b/headroom/transforms/content_router.py index 242e0aaa2..4aeb03f3e 100644 --- a/headroom/transforms/content_router.py +++ b/headroom/transforms/content_router.py @@ -57,6 +57,7 @@ from ..config import ( RelevanceScorerConfig, TransformResult, is_tool_excluded, + unwrap_tool_call_name, ) from ..parser import CCR_RETRIEVAL_MARKER_RE from ..tokenizer import Tokenizer @@ -4399,6 +4400,10 @@ class ContentRouter(Transform): tc_id = tc.get("id", "") fn = tc.get("function", {}) name = fn.get("name", "") + if name: + # Hermes deferred tools arrive wrapped as `tool_call` + # with the real name inside the arguments payload. + name = unwrap_tool_call_name(name, fn.get("arguments")) if tc_id and name: mapping[tc_id] = name args = _tool_call_args_text(fn.get("arguments")) @@ -4415,6 +4420,10 @@ class ContentRouter(Transform): if isinstance(block, dict) and block.get("type") == "tool_use": tc_id = block.get("id", "") name = block.get("name", "") + if name: + # Hermes deferred tools arrive wrapped as `tool_call` + # with the real name inside the input payload. + name = unwrap_tool_call_name(name, block.get("input")) if tc_id and name: mapping[tc_id] = name args = _tool_call_args_text(block.get("input")) diff --git a/plugins/openclaw/package-lock.json b/plugins/openclaw/package-lock.json index 4fcb59837..c69d37af2 100644 --- a/plugins/openclaw/package-lock.json +++ b/plugins/openclaw/package-lock.json @@ -1,12 +1,12 @@ { "name": "headroom-openclaw", - "version": "0.32.0", + "version": "0.33.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "headroom-openclaw", - "version": "0.32.0", + "version": "0.33.0", "license": "Apache-2.0", "dependencies": { "headroom-ai": "^0.22.3" @@ -2011,9 +2011,9 @@ } }, "node_modules/nanoid": { - "version": "3.3.12", - "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.12.tgz", - "integrity": "sha512-ZB9RH/39qpq5Vu6Y+NmUaFhQR6pp+M2Xt76XBnEwDaGcVAqhlvxrl3B2bKS5D3NH3QR76v3aSrKaF/Kiy7lEtQ==", + "version": "3.3.17", + "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.17.tgz", + "integrity": "sha512-xQLf0A3HOMlgHq0n247/LRuAOYmB7dXJ/DvAxGvsSBij45XtBSmQycu+F8ODbHwns/XyFZagyL1+J0Offw1E0g==", "dev": true, "funding": [ { @@ -2100,9 +2100,9 @@ } }, "node_modules/postcss": { - "version": "8.5.19", - "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.5.19.tgz", - "integrity": "sha512-Mz8SaolMd8nB+G13WkORcxQKHZ/NE4xXevtkJHVuG+guo9/wYKlIMTKAqGdEmYOXR2ijPjTYNHssizdaVSUNdQ==", + "version": "8.5.25", + "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.5.25.tgz", + "integrity": "sha512-DTPx3RWSSnWyzLxQnlH0rJP+EW5ekl16ZU4/psbIhA0e53kJfdgaN5vKM+xP7yJtXVu+nfdVFmlgFDEKAe4Pyw==", "dev": true, "funding": [ { @@ -2120,7 +2120,7 @@ ], "license": "MIT", "dependencies": { - "nanoid": "^3.3.12", + "nanoid": "^3.3.16", "picocolors": "^1.1.1", "source-map-js": "^1.2.1" }, diff --git a/plugins/opencode/package-lock.json b/plugins/opencode/package-lock.json index d4506817a..700553f65 100644 --- a/plugins/opencode/package-lock.json +++ b/plugins/opencode/package-lock.json @@ -2240,9 +2240,9 @@ } }, "node_modules/nanoid": { - "version": "3.3.15", - "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.15.tgz", - "integrity": "sha512-y7Wygv/7mEOvxTuEQDB8StXdMRBWf1kR/tlhAzBRUFkB2jfcLOAxO/SHmOO2zgz1pVgK29/kyupn059/bCHdjA==", + "version": "3.3.17", + "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.17.tgz", + "integrity": "sha512-xQLf0A3HOMlgHq0n247/LRuAOYmB7dXJ/DvAxGvsSBij45XtBSmQycu+F8ODbHwns/XyFZagyL1+J0Offw1E0g==", "dev": true, "funding": [ { @@ -2356,9 +2356,9 @@ } }, "node_modules/postcss": { - "version": "8.5.19", - "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.5.19.tgz", - "integrity": "sha512-Mz8SaolMd8nB+G13WkORcxQKHZ/NE4xXevtkJHVuG+guo9/wYKlIMTKAqGdEmYOXR2ijPjTYNHssizdaVSUNdQ==", + "version": "8.5.25", + "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.5.25.tgz", + "integrity": "sha512-DTPx3RWSSnWyzLxQnlH0rJP+EW5ekl16ZU4/psbIhA0e53kJfdgaN5vKM+xP7yJtXVu+nfdVFmlgFDEKAe4Pyw==", "dev": true, "funding": [ { @@ -2376,7 +2376,7 @@ ], "license": "MIT", "dependencies": { - "nanoid": "^3.3.12", + "nanoid": "^3.3.16", "picocolors": "^1.1.1", "source-map-js": "^1.2.1" }, diff --git a/sdk/typescript/package-lock.json b/sdk/typescript/package-lock.json index 91c6e4e49..7e6915d0f 100644 --- a/sdk/typescript/package-lock.json +++ b/sdk/typescript/package-lock.json @@ -1,12 +1,12 @@ { "name": "headroom-ai", - "version": "0.32.0", + "version": "0.33.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "headroom-ai", - "version": "0.32.0", + "version": "0.33.0", "license": "Apache-2.0", "devDependencies": { "@ai-sdk/anthropic": "^3.0.64", @@ -2617,9 +2617,9 @@ } }, "node_modules/nanoid": { - "version": "3.3.12", - "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.12.tgz", - "integrity": "sha512-ZB9RH/39qpq5Vu6Y+NmUaFhQR6pp+M2Xt76XBnEwDaGcVAqhlvxrl3B2bKS5D3NH3QR76v3aSrKaF/Kiy7lEtQ==", + "version": "3.3.17", + "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.17.tgz", + "integrity": "sha512-xQLf0A3HOMlgHq0n247/LRuAOYmB7dXJ/DvAxGvsSBij45XtBSmQycu+F8ODbHwns/XyFZagyL1+J0Offw1E0g==", "dev": true, "funding": [ { @@ -2796,9 +2796,9 @@ } }, "node_modules/postcss": { - "version": "8.5.19", - "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.5.19.tgz", - "integrity": "sha512-Mz8SaolMd8nB+G13WkORcxQKHZ/NE4xXevtkJHVuG+guo9/wYKlIMTKAqGdEmYOXR2ijPjTYNHssizdaVSUNdQ==", + "version": "8.5.25", + "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.5.25.tgz", + "integrity": "sha512-DTPx3RWSSnWyzLxQnlH0rJP+EW5ekl16ZU4/psbIhA0e53kJfdgaN5vKM+xP7yJtXVu+nfdVFmlgFDEKAe4Pyw==", "dev": true, "funding": [ { @@ -2816,7 +2816,7 @@ ], "license": "MIT", "dependencies": { - "nanoid": "^3.3.12", + "nanoid": "^3.3.16", "picocolors": "^1.1.1", "source-map-js": "^1.2.1" }, diff --git a/tests/test_hermes_tool_call_unwrap.py b/tests/test_hermes_tool_call_unwrap.py new file mode 100644 index 000000000..b5c393989 --- /dev/null +++ b/tests/test_hermes_tool_call_unwrap.py @@ -0,0 +1,191 @@ +"""Tests for Hermes deferred-tool (`tool_call` wrapper) unwrapping. + +Hermes Agent loads on-demand tools via a `tool_search`/`tool_describe`/ +`tool_call` indirection: on the wire the emitted tool call is named +`tool_call` and the REAL tool name lives in the arguments payload +(`{"name": "...", "arguments": {...}}`). Tool exclusion / protect lists +match on the real name, so `_build_tool_name_map` must unwrap the bridge +or whitelists silently no-op for all deferred tools. + +These tests pin the `unwrap_tool_call_name` helper and its integration +into `ContentRouter._build_tool_name_map` (OpenAI + Anthropic paths). +""" + +from __future__ import annotations + +from headroom.config import ( + DEFAULT_EXCLUDE_TOOLS, + is_tool_excluded, + unwrap_tool_call_name, +) +from headroom.transforms.content_router import ContentRouter, ContentRouterConfig + +# --------------------------------------------------------------------------- +# Helper unit tests +# --------------------------------------------------------------------------- + + +def test_unwrap_passthrough_plain_name() -> None: + assert unwrap_tool_call_name("read_file", '{"path": "/x"}') == "read_file" + + +def test_unwrap_passthrough_none_arguments() -> None: + assert unwrap_tool_call_name("tool_call", None) == "tool_call" + + +def test_unwrap_passthrough_bad_json() -> None: + assert unwrap_tool_call_name("tool_call", "bad json") == "tool_call" + + +def test_unwrap_passthrough_missing_name_key() -> None: + assert unwrap_tool_call_name("tool_call", '{"no_name": true}') == "tool_call" + + +def test_unwrap_passthrough_empty_name() -> None: + assert unwrap_tool_call_name("", None) == "" + + +def test_unwrap_web_search() -> None: + assert ( + unwrap_tool_call_name("tool_call", '{"name": "web_search", "arguments": {}}') + == "web_search" + ) + + +def test_unwrap_read_file() -> None: + assert ( + unwrap_tool_call_name("tool_call", '{"name": "read_file", "arguments": {"path": "/x"}}') + == "read_file" + ) + + +def test_unwrap_mcp_tool() -> None: + assert ( + unwrap_tool_call_name( + "tool_call", '{"name": "mcp__codebase_memory__search", "arguments": {}}' + ) + == "mcp__codebase_memory__search" + ) + + +def test_unwrap_dict_arguments_form() -> None: + """Arguments may arrive as a dict (not JSON string) on some paths.""" + assert ( + unwrap_tool_call_name("tool_call", {"name": "search_files", "arguments": {"pattern": "x"}}) + == "search_files" + ) + + +def test_unwrap_whitelist_activation() -> None: + """Unwrapped names must activate the DEFAULT_EXCLUDE_TOOLS whitelist.""" + assert is_tool_excluded("web_search", DEFAULT_EXCLUDE_TOOLS) is True + unwrapped = unwrap_tool_call_name("tool_call", '{"name": "web_search", "arguments": {}}') + assert is_tool_excluded(unwrapped, DEFAULT_EXCLUDE_TOOLS) is True + + +# --------------------------------------------------------------------------- +# _build_tool_name_map integration tests +# --------------------------------------------------------------------------- + + +def _router(exclude_tools: set[str] | None = None) -> ContentRouter: + config = ContentRouterConfig( + min_section_tokens=10, + enable_kompress=False, + exclude_tools=exclude_tools, + ) + return ContentRouter(config) + + +def test_build_tool_name_map_openai_wrapped() -> None: + """OpenAI-format assistant tool_calls with Hermes tool_call wrapper.""" + messages = [ + { + "role": "assistant", + "content": None, + "tool_calls": [ + { + "id": "call_wrapped_1", + "type": "function", + "function": { + "name": "tool_call", + "arguments": '{"name": "read_file", "arguments": {"path": "/x"}}', + }, + }, + { + "id": "call_plain_2", + "type": "function", + "function": {"name": "web_search", "arguments": '{"query": "q"}'}, + }, + ], + } + ] + router = _router() + mapping = router._build_tool_name_map(messages) + assert mapping["call_wrapped_1"] == "read_file", ( + "wrapped tool_call must map to the real tool name" + ) + assert mapping["call_plain_2"] == "web_search", "plain tool names must pass through unchanged" + + +def test_build_tool_name_map_anthropic_wrapped() -> None: + """Anthropic-format tool_use blocks with Hermes tool_call wrapper.""" + messages = [ + { + "role": "assistant", + "content": [ + { + "type": "tool_use", + "id": "toolu_wrapped_1", + "name": "tool_call", + "input": {"name": "headroom_retrieve", "arguments": {"hash": "abc"}}, + }, + { + "type": "tool_use", + "id": "toolu_plain_2", + "name": "Read", + "input": {"file_path": "/x"}, + }, + ], + } + ] + router = _router() + mapping = router._build_tool_name_map(messages) + assert mapping["toolu_wrapped_1"] == "headroom_retrieve", ( + "wrapped tool_call must map to the real tool name" + ) + assert mapping["toolu_plain_2"] == "Read", "plain tool names must pass through unchanged" + + +def test_build_tool_name_map_wrapped_not_excluded_before_unwrap() -> None: + """Sanity: without unwrapping, a wrapped read_file is NOT excluded. + + This documents the failure mode the fix addresses: `tool_call` is not in + DEFAULT_EXCLUDE_TOOLS, so a whitelist match would never fire. + """ + assert is_tool_excluded("tool_call", DEFAULT_EXCLUDE_TOOLS) is False + assert is_tool_excluded("read_file", DEFAULT_EXCLUDE_TOOLS) is False + + +def test_build_tool_name_map_exclusion_after_unwrap() -> None: + """Unwrapped names feed is_tool_excluded for whitelist decisions.""" + router = _router(exclude_tools=set(DEFAULT_EXCLUDE_TOOLS) | {"read_file"}) + messages = [ + { + "role": "assistant", + "content": None, + "tool_calls": [ + { + "id": "call_rf_1", + "type": "function", + "function": { + "name": "tool_call", + "arguments": '{"name": "read_file", "arguments": {"path": "/x"}}', + }, + } + ], + } + ] + mapping = router._build_tool_name_map(messages) + assert mapping["call_rf_1"] == "read_file" + assert is_tool_excluded(mapping["call_rf_1"], router.config.exclude_tools or set()) is True