Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions agent/context_compressor.py
Original file line number Diff line number Diff line change
Expand Up @@ -940,6 +940,37 @@ def _sanitize_tool_pairs(self, messages: List[Dict[str, Any]]) -> List[Dict[str,
if not self.quiet_mode:
logger.info("Compression sanitizer: added %d stub tool result(s)", len(missing_results))

# 3. Remove tool messages at the start of the list (or after a non-assistant
# message) that have no preceding assistant with tool_calls. This handles
# the boundary case where compress_end lands on a tool message whose parent
# assistant was in the summarised region. The orphan removal in step 1
# catches tool results whose call_id no longer matches any surviving
# assistant, but some providers (DeepSeek, etc.) additionally require that
# every ``tool`` message is *immediately* preceded by the ``assistant``
# message that issued the corresponding ``tool_calls``.
filtered: List[Dict[str, Any]] = []
last_assistant_had_calls = False
removed_boundary = 0
for msg in messages:
if msg.get("role") == "tool":
# Valid only if immediately preceded by an assistant with tool_calls
# or another tool message from the same group
if not last_assistant_had_calls and not (
filtered and filtered[-1].get("role") == "tool"
):
removed_boundary += 1
continue
filtered.append(msg)
last_assistant_had_calls = (
msg.get("role") == "assistant" and bool(msg.get("tool_calls"))
)
if removed_boundary and not self.quiet_mode:
logger.info(
"Compression sanitizer: removed %d tool message(s) at invalid boundary",
removed_boundary,
)
messages = filtered

return messages

def _align_boundary_forward(self, messages: List[Dict[str, Any]], idx: int) -> int:
Expand Down
5 changes: 4 additions & 1 deletion gateway/platforms/whatsapp.py
Original file line number Diff line number Diff line change
Expand Up @@ -379,12 +379,15 @@ async def connect(self) -> bool:
if not (bridge_dir / "node_modules").exists():
print(f"[{self.name}] Installing WhatsApp bridge dependencies...")
try:
# Read timeout from environment variable, default to 300 seconds (5 minutes)
# to accommodate slower systems like Unraid NAS
npm_install_timeout = int(os.environ.get("WHATSAPP_NPM_INSTALL_TIMEOUT", "300"))
install_result = subprocess.run(
["npm", "install", "--silent"],
cwd=str(bridge_dir),
capture_output=True,
text=True,
timeout=60,
timeout=npm_install_timeout,
)
if install_result.returncode != 0:
print(f"[{self.name}] npm install failed: {install_result.stderr}")
Expand Down
90 changes: 90 additions & 0 deletions tests/agent/test_context_compressor.py
Original file line number Diff line number Diff line change
Expand Up @@ -969,3 +969,93 @@ def test_pass3_emits_valid_json_for_downstream_provider(self):
parsed = _json.loads(shrunk)
assert parsed["path"] == "~/.hermes/skills/shopping/browser-setup-notes.md"
assert parsed["content"].endswith("...[truncated]")


class TestSanitizeToolPairsBoundary:
"""Regression: tool messages at compression boundary cause HTTP 400.

After compression, the first message in the tail may be a ``tool``
message whose parent ``assistant`` was summarised away. Some providers
(DeepSeek, etc.) require every ``tool`` message to be *immediately*
preceded by the ``assistant`` that issued the corresponding
``tool_calls``. The sanitizer must strip such boundary orphans.
"""

def test_tool_message_at_start_is_removed(self, compressor):
"""A tool message at position 0 (no preceding assistant) is removed."""
messages = [
{"role": "tool", "tool_call_id": "tc_orphan", "content": "result"},
{"role": "user", "content": "hello"},
{"role": "assistant", "content": "hi"},
]
result = compressor._sanitize_tool_pairs(messages)
assert len(result) == 2
assert result[0]["role"] == "user"

def test_tool_message_after_summary_is_removed(self, compressor):
"""A tool message after a summary (non-assistant-with-calls) is removed.

Simulates the post-compression layout: [...head..., summary, tool, ...tail].
"""
messages = [
{"role": "system", "content": "sys"},
{"role": "user", "content": "question"},
{"role": "assistant", "content": "summary of earlier turns"},
{"role": "tool", "tool_call_id": "tc_old", "content": "old result"},
{"role": "user", "content": "follow up"},
{"role": "assistant", "content": "answer"},
]
result = compressor._sanitize_tool_pairs(messages)
# The orphaned tool(tc_old) should be removed — its parent assistant
# was summarised and no longer has matching tool_calls.
tool_msgs = [m for m in result if m.get("role") == "tool"]
assert len(tool_msgs) == 0

def test_valid_tool_group_preserved(self, compressor):
"""A valid assistant→tool group is NOT affected by the boundary check."""
messages = [
{"role": "user", "content": "question"},
{"role": "assistant", "content": None, "tool_calls": [
{"id": "tc_1", "type": "function",
"function": {"name": "read_file", "arguments": "{}"}},
]},
{"role": "tool", "tool_call_id": "tc_1", "content": "file contents"},
{"role": "user", "content": "thanks"},
]
result = compressor._sanitize_tool_pairs(messages)
assert len(result) == 4
tool_msgs = [m for m in result if m.get("role") == "tool"]
assert len(tool_msgs) == 1

def test_consecutive_orphaned_tools_at_boundary(self, compressor):
"""Multiple consecutive orphaned tool messages at the boundary are all removed."""
messages = [
{"role": "assistant", "content": "summary"},
{"role": "tool", "tool_call_id": "tc_a", "content": "a"},
{"role": "tool", "tool_call_id": "tc_b", "content": "b"},
{"role": "user", "content": "next"},
]
result = compressor._sanitize_tool_pairs(messages)
assert len(result) == 2
assert result[0]["role"] == "assistant"
assert result[1]["role"] == "user"

def test_mixed_valid_and_orphaned_tools(self, compressor):
"""Valid tool group preserved; orphaned tools after it removed."""
messages = [
{"role": "user", "content": "q1"},
{"role": "assistant", "content": None, "tool_calls": [
{"id": "tc_good", "type": "function",
"function": {"name": "foo", "arguments": "{}"}},
]},
{"role": "tool", "tool_call_id": "tc_good", "content": "ok"},
{"role": "assistant", "content": "summary"},
{"role": "tool", "tool_call_id": "tc_bad", "content": "orphan"},
{"role": "user", "content": "q2"},
]
result = compressor._sanitize_tool_pairs(messages)
# tc_good is valid (preceded by assistant with tool_calls)
# tc_bad is orphaned (preceded by assistant without tool_calls)
tool_msgs = [m for m in result if m.get("role") == "tool"]
assert len(tool_msgs) == 1
assert tool_msgs[0]["tool_call_id"] == "tc_good"
37 changes: 37 additions & 0 deletions tests/tools/test_file_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -323,4 +323,41 @@ def test_truncated_hint_with_nonzero_offset(self, mock_get):
assert "offset=100" in raw


class TestPatchSchema:
"""Tests for PATCH_SCHEMA to ensure required parameters are properly declared."""

def test_patch_schema_includes_all_required_params(self):
"""PATCH_SCHEMA should include all parameters that are conditionally required."""
from tools.file_tools import PATCH_SCHEMA

# Verify schema structure
assert "parameters" in PATCH_SCHEMA
assert "required" in PATCH_SCHEMA["parameters"]

# All parameters that are mode-specific should be in required list
required = PATCH_SCHEMA["parameters"]["required"]
assert "mode" in required
assert "path" in required
assert "old_string" in required
assert "new_string" in required
assert "patch" in required

# replace_all is optional (has default), so it should NOT be in required
assert "replace_all" not in required

def test_patch_schema_description_mentions_mode_specific_requirements(self):
"""PATCH_SCHEMA description should explain mode-specific requirements."""
from tools.file_tools import PATCH_SCHEMA

description = PATCH_SCHEMA.get("description", "")

# Description should mention mode-specific requirements
assert "mode-specific" in description.lower() or "IMPORTANT:" in description

# Should mention both modes
assert "mode='replace'" in description
assert "mode='patch'" in description




12 changes: 6 additions & 6 deletions tools/file_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -908,18 +908,18 @@ def _check_file_reqs():

PATCH_SCHEMA = {
"name": "patch",
"description": "Targeted find-and-replace edits in files. Use this instead of sed/awk in terminal. Uses fuzzy matching (9 strategies) so minor whitespace/indentation differences won't break it. Returns a unified diff. Auto-runs syntax checks after editing.\n\nReplace mode (default): find a unique string and replace it.\nPatch mode: apply V4A multi-file patches for bulk changes.",
"description": "Targeted find-and-replace edits in files. Use this instead of sed/awk in terminal. Uses fuzzy matching (9 strategies) so minor whitespace/indentation differences won't break it. Returns a unified diff. Auto-runs syntax checks after editing.\n\nIMPORTANT: Parameters are mode-specific:\n- For mode='replace': provide path, old_string, new_string (optionally replace_all)\n- For mode='patch': provide patch content",
"parameters": {
"type": "object",
"properties": {
"mode": {"type": "string", "enum": ["replace", "patch"], "description": "Edit mode: 'replace' for targeted find-and-replace, 'patch' for V4A multi-file patches", "default": "replace"},
"path": {"type": "string", "description": "File path to edit (required for 'replace' mode)"},
"old_string": {"type": "string", "description": "Text to find in the file (required for 'replace' mode). Must be unique in the file unless replace_all=true. Include enough surrounding context to ensure uniqueness."},
"new_string": {"type": "string", "description": "Replacement text (required for 'replace' mode). Can be empty string to delete the matched text."},
"path": {"type": "string", "description": "File path to edit (required when mode='replace')"},
"old_string": {"type": "string", "description": "Text to find in file (required when mode='replace'). Must be unique in file unless replace_all=true. Include enough surrounding context to ensure uniqueness."},
"new_string": {"type": "string", "description": "Replacement text (required when mode='replace'). Can be empty string to delete the matched text."},
"replace_all": {"type": "boolean", "description": "Replace all occurrences instead of requiring a unique match (default: false)", "default": False},
"patch": {"type": "string", "description": "V4A format patch content (required for 'patch' mode). Format:\n*** Begin Patch\n*** Update File: path/to/file\n@@ context hint @@\n context line\n-removed line\n+added line\n*** End Patch"}
"patch": {"type": "string", "description": "V4A format patch content (required when mode='patch'). Format:\n*** Begin Patch\n*** Update File: path/to/file\n@@ context hint @@\n context line\n-removed line\n+added line\n*** End Patch"}
},
"required": ["mode"]
"required": ["mode", "path", "old_string", "new_string", "patch"]
}
}

Expand Down
Loading