Compare commits
11 Commits
fix/cron-s
...
burn/378-1
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
acf8658b85 | ||
| 1ec02cf061 | |||
|
|
1156875cb5 | ||
| f4c102400e | |||
| 6555ccabc1 | |||
|
|
8c712866c4 | ||
| 8fb59aae64 | |||
|
|
95bde9d3cb | ||
|
|
aa6eabb816 | ||
| 3b89bfbab2 | |||
| 3e6e183ad2 |
@@ -13,6 +13,7 @@ import concurrent.futures
|
||||
import json
|
||||
import logging
|
||||
import os
|
||||
import re
|
||||
import subprocess
|
||||
import sys
|
||||
|
||||
@@ -40,6 +41,44 @@ from hermes_time import now as _hermes_now
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
# Minimum context tokens for cron jobs — models with smaller context are rejected
|
||||
# to prevent truncation of long prompts + tool outputs.
|
||||
CRON_MIN_CONTEXT_TOKENS = 64_000
|
||||
|
||||
|
||||
class ModelContextError(ValueError):
|
||||
"""Raised when a model's context length is too small for cron execution."""
|
||||
pass
|
||||
|
||||
|
||||
def _check_model_context_compat(model: str, base_url: str = None, config_context_length: int = None):
|
||||
"""Check if a model's context length meets the minimum for cron jobs.
|
||||
|
||||
Raises ModelContextError if the model's context is too small.
|
||||
Silently passes if detection fails (fail-open).
|
||||
"""
|
||||
if config_context_length is not None and config_context_length < CRON_MIN_CONTEXT_TOKENS:
|
||||
raise ModelContextError(
|
||||
f"Model '{model}' has {config_context_length:,} context tokens, "
|
||||
f"but cron jobs require at least {CRON_MIN_CONTEXT_TOKENS:,}. "
|
||||
f"Set a larger model in config.yaml or override per-job."
|
||||
)
|
||||
|
||||
try:
|
||||
from agent.model_metadata import get_model_context_length
|
||||
context_length = get_model_context_length(model, base_url=base_url)
|
||||
if context_length is not None and context_length < CRON_MIN_CONTEXT_TOKENS:
|
||||
raise ModelContextError(
|
||||
f"Model '{model}' has {context_length:,} context tokens, "
|
||||
f"but cron jobs require at least {CRON_MIN_CONTEXT_TOKENS:,}. "
|
||||
f"Set a larger model in config.yaml or override per-job."
|
||||
)
|
||||
except ModelContextError:
|
||||
raise
|
||||
except Exception:
|
||||
# Detection failure is non-fatal — fail open
|
||||
logger.debug("Context length detection failed for %s, skipping check", model)
|
||||
|
||||
|
||||
# =====================================================================
|
||||
# Deploy Sync Guard
|
||||
@@ -642,6 +681,73 @@ def _build_job_prompt(job: dict) -> str:
|
||||
return "\n".join(parts)
|
||||
|
||||
|
||||
def _validate_local_service_access(job: dict, prompt: str) -> tuple[bool, str]:
|
||||
"""
|
||||
Validate that a cron job can access local services it references.
|
||||
|
||||
Detects prompts that reference localhost services (Ollama, etc.) and
|
||||
ensures the job is configured with a local base_url or provider.
|
||||
|
||||
Returns:
|
||||
(is_valid, warning_message) — True if no issue, False if mismatch detected.
|
||||
"""
|
||||
# Patterns that indicate local service access is required
|
||||
local_service_patterns = [
|
||||
r"localhost:\d+",
|
||||
r"127\.0\.0\.1:\d+",
|
||||
r"Check Ollama",
|
||||
r"check.*ollama",
|
||||
r"Ollama.*responding",
|
||||
r"ollama.*responding",
|
||||
r"local.*model.*health",
|
||||
r"health.*local.*model",
|
||||
r"ping.*localhost",
|
||||
r"curl.*localhost",
|
||||
]
|
||||
|
||||
# Check if prompt references local services
|
||||
prompt_lower = prompt.lower()
|
||||
references_local = any(
|
||||
re.search(pattern, prompt_lower) for pattern in local_service_patterns
|
||||
)
|
||||
|
||||
if not references_local:
|
||||
return True, ""
|
||||
|
||||
# Check if job is configured for local access
|
||||
base_url = job.get("base_url", "")
|
||||
provider = job.get("provider", "")
|
||||
model = job.get("model", "")
|
||||
|
||||
# Check for explicit local base_url
|
||||
if base_url and ("localhost" in base_url or "127.0.0.1" in base_url):
|
||||
return True, ""
|
||||
|
||||
# Check for Ollama provider
|
||||
if provider and "ollama" in provider.lower():
|
||||
return True, ""
|
||||
|
||||
# Check for common local model patterns in model name
|
||||
local_model_patterns = ["ollama", "llama", "mistral", "phi", "qwen", "gemma", "codellama"]
|
||||
if model and any(pattern in model.lower() for pattern in local_model_patterns):
|
||||
# Model name suggests local, but verify base_url
|
||||
if not base_url:
|
||||
return False, (
|
||||
f"Cron job '{job.get('name', job.get('id'))}' references local services "
|
||||
f"(localhost/Ollama) but has no base_url configured. "
|
||||
f"Set base_url='http://localhost:11434' for Ollama, or pin to a local provider."
|
||||
)
|
||||
return True, ""
|
||||
|
||||
# No local configuration detected
|
||||
return False, (
|
||||
f"Cron job '{job.get('name', job.get('id'))}' references local services "
|
||||
f"(localhost/Ollama) but is configured for cloud model "
|
||||
f"(model={model or 'default'}, provider={provider or 'default'}). "
|
||||
f"To check local Ollama, set base_url='http://localhost:11434' or provider='ollama'."
|
||||
)
|
||||
|
||||
|
||||
def run_job(job: dict) -> tuple[bool, str, str, Optional[str]]:
|
||||
"""
|
||||
Execute a single cron job.
|
||||
@@ -667,6 +773,18 @@ def run_job(job: dict) -> tuple[bool, str, str, Optional[str]]:
|
||||
job_id = job["id"]
|
||||
job_name = job["name"]
|
||||
prompt = _build_job_prompt(job)
|
||||
|
||||
# Validate local service access — detect prompts referencing localhost/Ollama
|
||||
# that will fail on cloud models (#378)
|
||||
is_valid, warning = _validate_local_service_access(job, prompt)
|
||||
if not is_valid:
|
||||
logger.warning("Job '%s': %s", job_name, warning)
|
||||
# Inject warning into prompt so agent knows to report the issue
|
||||
prompt = (
|
||||
f"[SYSTEM WARNING: {warning}]\n\n"
|
||||
f"{prompt}"
|
||||
)
|
||||
|
||||
origin = _resolve_origin(job)
|
||||
_cron_session_id = f"cron_{job_id}_{_hermes_now().strftime('%Y%m%d_%H%M%S')}"
|
||||
|
||||
|
||||
@@ -648,6 +648,51 @@ def load_gateway_config() -> GatewayConfig:
|
||||
return config
|
||||
|
||||
|
||||
# Known-weak placeholder tokens from .env.example, tutorials, etc.
|
||||
_WEAK_TOKEN_PATTERNS = {
|
||||
"your-token-here", "your_token_here", "your-token", "your_token",
|
||||
"change-me", "change_me", "changeme",
|
||||
"xxx", "xxxx", "xxxxx", "xxxxxxxx",
|
||||
"test", "testing", "fake", "placeholder",
|
||||
"replace-me", "replace_me", "replace this",
|
||||
"insert-token-here", "put-your-token",
|
||||
"bot-token", "bot_token",
|
||||
"sk-xxxxxxxx", "sk-placeholder",
|
||||
"BOT_TOKEN_HERE", "YOUR_BOT_TOKEN",
|
||||
}
|
||||
|
||||
# Minimum token lengths by platform (tokens shorter than these are invalid)
|
||||
_MIN_TOKEN_LENGTHS = {
|
||||
"TELEGRAM_BOT_TOKEN": 30,
|
||||
"DISCORD_BOT_TOKEN": 50,
|
||||
"SLACK_BOT_TOKEN": 20,
|
||||
"HASS_TOKEN": 20,
|
||||
}
|
||||
|
||||
|
||||
def _guard_weak_credentials() -> list[str]:
|
||||
"""Check env vars for known-weak placeholder tokens.
|
||||
|
||||
Returns a list of warning messages for any weak credentials found.
|
||||
"""
|
||||
warnings = []
|
||||
for env_var, min_len in _MIN_TOKEN_LENGTHS.items():
|
||||
value = os.getenv(env_var, "").strip()
|
||||
if not value:
|
||||
continue
|
||||
if value.lower() in _WEAK_TOKEN_PATTERNS:
|
||||
warnings.append(
|
||||
f"{env_var} is set to a placeholder value ('{value[:20]}'). "
|
||||
f"Replace it with a real token."
|
||||
)
|
||||
elif len(value) < min_len:
|
||||
warnings.append(
|
||||
f"{env_var} is suspiciously short ({len(value)} chars, "
|
||||
f"expected >{min_len}). May be truncated or invalid."
|
||||
)
|
||||
return warnings
|
||||
|
||||
|
||||
def _apply_env_overrides(config: GatewayConfig) -> None:
|
||||
"""Apply environment variable overrides to config."""
|
||||
|
||||
@@ -941,3 +986,7 @@ def _apply_env_overrides(config: GatewayConfig) -> None:
|
||||
config.default_reset_policy.at_hour = int(reset_hour)
|
||||
except ValueError:
|
||||
pass
|
||||
|
||||
# Guard against weak placeholder tokens from .env.example copies
|
||||
for warning in _guard_weak_credentials():
|
||||
logger.warning("Weak credential: %s", warning)
|
||||
|
||||
@@ -540,6 +540,29 @@ def handle_function_call(
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
# Poka-yoke: validate tool handler return type.
|
||||
# Handlers MUST return a JSON string. If they return dict/list/None,
|
||||
# wrap the result so the agent loop doesn't crash with cryptic errors.
|
||||
if not isinstance(result, str):
|
||||
logger.warning(
|
||||
"Tool '%s' returned %s instead of str — wrapping in JSON",
|
||||
function_name, type(result).__name__,
|
||||
)
|
||||
result = json.dumps(
|
||||
{"output": str(result), "_type_warning": f"Tool returned {type(result).__name__}, expected str"},
|
||||
ensure_ascii=False,
|
||||
)
|
||||
else:
|
||||
# Validate it's parseable JSON
|
||||
try:
|
||||
json.loads(result)
|
||||
except (json.JSONDecodeError, TypeError):
|
||||
logger.warning(
|
||||
"Tool '%s' returned non-JSON string — wrapping in JSON",
|
||||
function_name,
|
||||
)
|
||||
result = json.dumps({"output": result}, ensure_ascii=False)
|
||||
|
||||
return result
|
||||
|
||||
except Exception as e:
|
||||
|
||||
@@ -12,7 +12,7 @@ Config in $HERMES_HOME/config.yaml (profile-scoped):
|
||||
auto_extract: false
|
||||
default_trust: 0.5
|
||||
min_trust_threshold: 0.3
|
||||
temporal_decay_half_life: 0
|
||||
temporal_decay_half_life: 60
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@@ -152,6 +152,7 @@ class HolographicMemoryProvider(MemoryProvider):
|
||||
{"key": "auto_extract", "description": "Auto-extract facts at session end", "default": "false", "choices": ["true", "false"]},
|
||||
{"key": "default_trust", "description": "Default trust score for new facts", "default": "0.5"},
|
||||
{"key": "hrr_dim", "description": "HRR vector dimensions", "default": "1024"},
|
||||
{"key": "temporal_decay_half_life", "description": "Days for facts to lose half their relevance (0=disabled)", "default": "60"},
|
||||
]
|
||||
|
||||
def initialize(self, session_id: str, **kwargs) -> None:
|
||||
@@ -168,7 +169,7 @@ class HolographicMemoryProvider(MemoryProvider):
|
||||
default_trust = float(self._config.get("default_trust", 0.5))
|
||||
hrr_dim = int(self._config.get("hrr_dim", 1024))
|
||||
hrr_weight = float(self._config.get("hrr_weight", 0.3))
|
||||
temporal_decay = int(self._config.get("temporal_decay_half_life", 0))
|
||||
temporal_decay = int(self._config.get("temporal_decay_half_life", 60))
|
||||
|
||||
self._store = MemoryStore(db_path=db_path, default_trust=default_trust, hrr_dim=hrr_dim)
|
||||
self._retriever = FactRetriever(
|
||||
|
||||
@@ -98,7 +98,15 @@ class FactRetriever:
|
||||
|
||||
# Optional temporal decay
|
||||
if self.half_life > 0:
|
||||
score *= self._temporal_decay(fact.get("updated_at") or fact.get("created_at"))
|
||||
decay = self._temporal_decay(fact.get("updated_at") or fact.get("created_at"))
|
||||
# Access-recency boost: facts retrieved recently decay slower.
|
||||
# A fact accessed within 1 half-life gets up to 1.5x the decay
|
||||
# factor, tapering to 1.0x (no boost) after 2 half-lives.
|
||||
last_accessed = fact.get("last_accessed_at")
|
||||
if last_accessed:
|
||||
access_boost = self._access_recency_boost(last_accessed)
|
||||
decay = min(1.0, decay * access_boost)
|
||||
score *= decay
|
||||
|
||||
fact["score"] = score
|
||||
scored.append(fact)
|
||||
@@ -591,3 +599,41 @@ class FactRetriever:
|
||||
return math.pow(0.5, age_days / self.half_life)
|
||||
except (ValueError, TypeError):
|
||||
return 1.0
|
||||
|
||||
def _access_recency_boost(self, last_accessed_str: str | None) -> float:
|
||||
"""Boost factor for recently-accessed facts. Range [1.0, 1.5].
|
||||
|
||||
Facts accessed within 1 half-life get up to 1.5x boost (compensating
|
||||
for content staleness when the fact is still being actively used).
|
||||
Boost decays linearly to 1.0 (no boost) at 2 half-lives.
|
||||
|
||||
Returns 1.0 if half-life is disabled or timestamp is missing.
|
||||
"""
|
||||
if not self.half_life or not last_accessed_str:
|
||||
return 1.0
|
||||
|
||||
try:
|
||||
if isinstance(last_accessed_str, str):
|
||||
ts = datetime.fromisoformat(last_accessed_str.replace("Z", "+00:00"))
|
||||
else:
|
||||
ts = last_accessed_str
|
||||
|
||||
if ts.tzinfo is None:
|
||||
ts = ts.replace(tzinfo=timezone.utc)
|
||||
|
||||
age_days = (datetime.now(timezone.utc) - ts).total_seconds() / 86400
|
||||
if age_days < 0:
|
||||
return 1.5 # Future timestamp = just accessed
|
||||
|
||||
half_lives_since_access = age_days / self.half_life
|
||||
|
||||
if half_lives_since_access <= 1.0:
|
||||
# Within 1 half-life: linearly from 1.5 (just now) to 1.0 (at 1 HL)
|
||||
return 1.0 + 0.5 * (1.0 - half_lives_since_access)
|
||||
elif half_lives_since_access <= 2.0:
|
||||
# Between 1 and 2 half-lives: linearly from 1.0 to 1.0 (no boost)
|
||||
return 1.0
|
||||
else:
|
||||
return 1.0
|
||||
except (ValueError, TypeError):
|
||||
return 1.0
|
||||
|
||||
@@ -7,7 +7,7 @@ from unittest.mock import AsyncMock, patch, MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
from cron.scheduler import _resolve_origin, _resolve_delivery_target, _deliver_result, run_job, SILENT_MARKER, _build_job_prompt, _check_model_context_compat, ModelContextError, CRON_MIN_CONTEXT_TOKENS
|
||||
from cron.scheduler import _resolve_origin, _resolve_delivery_target, _deliver_result, run_job, SILENT_MARKER, _build_job_prompt, _check_model_context_compat, ModelContextError, CRON_MIN_CONTEXT_TOKENS, _validate_local_service_access
|
||||
|
||||
|
||||
class TestResolveOrigin:
|
||||
@@ -1001,3 +1001,99 @@ class TestCheckModelContextCompat:
|
||||
):
|
||||
with pytest.raises(ModelContextError):
|
||||
_check_model_context_compat("borderline-model")
|
||||
|
||||
|
||||
class TestValidateLocalServiceAccess:
|
||||
"""Tests for _validate_local_service_access — detects local service mismatches (#378)."""
|
||||
|
||||
def test_no_local_reference_passes(self):
|
||||
"""Prompt without local references always passes."""
|
||||
job = {"name": "test", "model": "gpt-4"}
|
||||
is_valid, msg = _validate_local_service_access(job, "Check system health")
|
||||
assert is_valid is True
|
||||
assert msg == ""
|
||||
|
||||
def test_localhost_reference_with_local_base_url(self):
|
||||
"""Prompt references localhost but job has local base_url — passes."""
|
||||
job = {
|
||||
"name": "health-check",
|
||||
"model": "llama3",
|
||||
"base_url": "http://localhost:11434/v1",
|
||||
}
|
||||
is_valid, msg = _validate_local_service_access(job, "Check if Ollama is responding on localhost:11434")
|
||||
assert is_valid is True
|
||||
assert msg == ""
|
||||
|
||||
def test_localhost_reference_with_cloud_model_fails(self):
|
||||
"""Prompt references localhost but job uses cloud model — fails."""
|
||||
job = {
|
||||
"name": "health-check",
|
||||
"model": "nous/mimo-v2-pro",
|
||||
"provider": "nous",
|
||||
}
|
||||
is_valid, msg = _validate_local_service_access(job, "Check Ollama is responding on localhost:11434")
|
||||
assert is_valid is False
|
||||
assert "localhost" in msg.lower() or "ollama" in msg.lower()
|
||||
assert "cloud model" in msg.lower() or "base_url" in msg.lower()
|
||||
|
||||
def test_ollama_check_with_ollama_provider(self):
|
||||
"""Prompt references Ollama and job uses ollama provider — passes."""
|
||||
job = {
|
||||
"name": "ollama-health",
|
||||
"provider": "ollama",
|
||||
"base_url": "http://localhost:11434",
|
||||
}
|
||||
is_valid, msg = _validate_local_service_access(job, "Check Ollama is responding")
|
||||
assert is_valid is True
|
||||
assert msg == ""
|
||||
|
||||
def test_case_insensitive_detection(self):
|
||||
"""Detection is case-insensitive."""
|
||||
job = {"name": "test", "model": "gpt-4"}
|
||||
# Lowercase
|
||||
is_valid, _ = _validate_local_service_access(job, "check ollama is responding")
|
||||
assert is_valid is False
|
||||
# Uppercase
|
||||
is_valid, _ = _validate_local_service_access(job, "CHECK OLLAMA IS RESPONDING")
|
||||
assert is_valid is False
|
||||
# Mixed case
|
||||
is_valid, _ = _validate_local_service_access(job, "Check if Ollama Is Responding")
|
||||
assert is_valid is False
|
||||
|
||||
def test_curl_localhost_detected(self):
|
||||
"""curl localhost references are detected."""
|
||||
job = {"name": "test", "model": "gpt-4"}
|
||||
is_valid, _ = _validate_local_service_access(job, "Run curl localhost:8080/health")
|
||||
assert is_valid is False
|
||||
|
||||
def test_127_0_0_1_detected(self):
|
||||
"""127.0.0.1 references are detected."""
|
||||
job = {"name": "test", "model": "gpt-4"}
|
||||
is_valid, _ = _validate_local_service_access(job, "Check http://127.0.0.1:11434/api/tags")
|
||||
assert is_valid is False
|
||||
|
||||
def test_local_model_name_without_base_url_fails(self):
|
||||
"""Model name suggests local but no base_url — fails."""
|
||||
job = {"name": "test", "model": "llama3"}
|
||||
is_valid, msg = _validate_local_service_access(job, "Check Ollama is responding")
|
||||
assert is_valid is False
|
||||
assert "base_url" in msg
|
||||
|
||||
def test_local_model_name_with_base_url_passes(self):
|
||||
"""Model name suggests local and has base_url — passes."""
|
||||
job = {"name": "test", "model": "llama3", "base_url": "http://localhost:11434"}
|
||||
is_valid, msg = _validate_local_service_access(job, "Check Ollama is responding")
|
||||
assert is_valid is True
|
||||
assert msg == ""
|
||||
|
||||
def test_nightwatch_health_monitor_scenario(self):
|
||||
"""Reproduces the exact #378 scenario."""
|
||||
job = {
|
||||
"name": "nightwatch-health-monitor",
|
||||
"model": "nous/mimo-v2-pro",
|
||||
"provider": "nous",
|
||||
}
|
||||
prompt = "Check Ollama is responding. Run curl http://localhost:11434/api/tags and report status."
|
||||
is_valid, msg = _validate_local_service_access(job, prompt)
|
||||
assert is_valid is False
|
||||
assert "nightwatch-health-monitor" in msg or "localhost" in msg
|
||||
|
||||
52
tests/gateway/test_weak_credential_guard.py
Normal file
52
tests/gateway/test_weak_credential_guard.py
Normal file
@@ -0,0 +1,52 @@
|
||||
"""Tests for weak credential guard in gateway/config.py."""
|
||||
|
||||
import os
|
||||
import pytest
|
||||
|
||||
from gateway.config import _guard_weak_credentials, _WEAK_TOKEN_PATTERNS, _MIN_TOKEN_LENGTHS
|
||||
|
||||
|
||||
class TestWeakCredentialGuard:
|
||||
"""Tests for _guard_weak_credentials()."""
|
||||
|
||||
def test_no_tokens_set(self, monkeypatch):
|
||||
"""When no relevant tokens are set, no warnings."""
|
||||
for var in _MIN_TOKEN_LENGTHS:
|
||||
monkeypatch.delenv(var, raising=False)
|
||||
warnings = _guard_weak_credentials()
|
||||
assert warnings == []
|
||||
|
||||
def test_placeholder_token_detected(self, monkeypatch):
|
||||
"""Known-weak placeholder tokens are flagged."""
|
||||
monkeypatch.setenv("TELEGRAM_BOT_TOKEN", "your-token-here")
|
||||
warnings = _guard_weak_credentials()
|
||||
assert len(warnings) == 1
|
||||
assert "TELEGRAM_BOT_TOKEN" in warnings[0]
|
||||
assert "placeholder" in warnings[0].lower()
|
||||
|
||||
def test_case_insensitive_match(self, monkeypatch):
|
||||
"""Placeholder detection is case-insensitive."""
|
||||
monkeypatch.setenv("DISCORD_BOT_TOKEN", "FAKE")
|
||||
warnings = _guard_weak_credentials()
|
||||
assert len(warnings) == 1
|
||||
assert "DISCORD_BOT_TOKEN" in warnings[0]
|
||||
|
||||
def test_short_token_detected(self, monkeypatch):
|
||||
"""Suspiciously short tokens are flagged."""
|
||||
monkeypatch.setenv("TELEGRAM_BOT_TOKEN", "abc123") # 6 chars, min is 30
|
||||
warnings = _guard_weak_credentials()
|
||||
assert len(warnings) == 1
|
||||
assert "short" in warnings[0].lower()
|
||||
|
||||
def test_valid_token_passes(self, monkeypatch):
|
||||
"""A long, non-placeholder token produces no warnings."""
|
||||
monkeypatch.setenv("TELEGRAM_BOT_TOKEN", "1234567890:ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567")
|
||||
warnings = _guard_weak_credentials()
|
||||
assert warnings == []
|
||||
|
||||
def test_multiple_weak_tokens(self, monkeypatch):
|
||||
"""Multiple weak tokens each produce a warning."""
|
||||
monkeypatch.setenv("TELEGRAM_BOT_TOKEN", "change-me")
|
||||
monkeypatch.setenv("DISCORD_BOT_TOKEN", "xx") # short
|
||||
warnings = _guard_weak_credentials()
|
||||
assert len(warnings) == 2
|
||||
209
tests/plugins/memory/test_temporal_decay.py
Normal file
209
tests/plugins/memory/test_temporal_decay.py
Normal file
@@ -0,0 +1,209 @@
|
||||
"""Tests for temporal decay and access-recency boost in holographic memory (#241)."""
|
||||
|
||||
import math
|
||||
from datetime import datetime, timedelta, timezone
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
class TestTemporalDecay:
|
||||
"""Test _temporal_decay exponential decay formula."""
|
||||
|
||||
def _make_retriever(self, half_life=60):
|
||||
from plugins.memory.holographic.retrieval import FactRetriever
|
||||
store = MagicMock()
|
||||
return FactRetriever(store=store, temporal_decay_half_life=half_life)
|
||||
|
||||
def test_fresh_fact_no_decay(self):
|
||||
"""A fact updated today should have decay ≈ 1.0."""
|
||||
r = self._make_retriever(half_life=60)
|
||||
now = datetime.now(timezone.utc).isoformat()
|
||||
decay = r._temporal_decay(now)
|
||||
assert decay > 0.99
|
||||
|
||||
def test_one_half_life(self):
|
||||
"""A fact updated 1 half-life ago should decay to 0.5."""
|
||||
r = self._make_retriever(half_life=60)
|
||||
old = (datetime.now(timezone.utc) - timedelta(days=60)).isoformat()
|
||||
decay = r._temporal_decay(old)
|
||||
assert abs(decay - 0.5) < 0.01
|
||||
|
||||
def test_two_half_lives(self):
|
||||
"""A fact updated 2 half-lives ago should decay to 0.25."""
|
||||
r = self._make_retriever(half_life=60)
|
||||
old = (datetime.now(timezone.utc) - timedelta(days=120)).isoformat()
|
||||
decay = r._temporal_decay(old)
|
||||
assert abs(decay - 0.25) < 0.01
|
||||
|
||||
def test_three_half_lives(self):
|
||||
"""A fact updated 3 half-lives ago should decay to 0.125."""
|
||||
r = self._make_retriever(half_life=60)
|
||||
old = (datetime.now(timezone.utc) - timedelta(days=180)).isoformat()
|
||||
decay = r._temporal_decay(old)
|
||||
assert abs(decay - 0.125) < 0.01
|
||||
|
||||
def test_half_life_disabled(self):
|
||||
"""When half_life=0, decay should always be 1.0."""
|
||||
r = self._make_retriever(half_life=0)
|
||||
old = (datetime.now(timezone.utc) - timedelta(days=365)).isoformat()
|
||||
assert r._temporal_decay(old) == 1.0
|
||||
|
||||
def test_none_timestamp(self):
|
||||
"""Missing timestamp should return 1.0 (no decay)."""
|
||||
r = self._make_retriever(half_life=60)
|
||||
assert r._temporal_decay(None) == 1.0
|
||||
|
||||
def test_empty_timestamp(self):
|
||||
r = self._make_retriever(half_life=60)
|
||||
assert r._temporal_decay("") == 1.0
|
||||
|
||||
def test_invalid_timestamp(self):
|
||||
"""Malformed timestamp should return 1.0 (fail open)."""
|
||||
r = self._make_retriever(half_life=60)
|
||||
assert r._temporal_decay("not-a-date") == 1.0
|
||||
|
||||
def test_future_timestamp(self):
|
||||
"""Future timestamp should return 1.0 (no decay for future dates)."""
|
||||
r = self._make_retriever(half_life=60)
|
||||
future = (datetime.now(timezone.utc) + timedelta(days=10)).isoformat()
|
||||
assert r._temporal_decay(future) == 1.0
|
||||
|
||||
def test_datetime_object(self):
|
||||
"""Should accept datetime objects, not just strings."""
|
||||
r = self._make_retriever(half_life=60)
|
||||
old = datetime.now(timezone.utc) - timedelta(days=60)
|
||||
decay = r._temporal_decay(old)
|
||||
assert abs(decay - 0.5) < 0.01
|
||||
|
||||
def test_different_half_lives(self):
|
||||
"""30-day half-life should decay faster than 90-day."""
|
||||
r30 = self._make_retriever(half_life=30)
|
||||
r90 = self._make_retriever(half_life=90)
|
||||
old = (datetime.now(timezone.utc) - timedelta(days=45)).isoformat()
|
||||
assert r30._temporal_decay(old) < r90._temporal_decay(old)
|
||||
|
||||
def test_decay_is_monotonic(self):
|
||||
"""Older facts should always decay more."""
|
||||
r = self._make_retriever(half_life=60)
|
||||
now = datetime.now(timezone.utc)
|
||||
d1 = r._temporal_decay((now - timedelta(days=10)).isoformat())
|
||||
d2 = r._temporal_decay((now - timedelta(days=30)).isoformat())
|
||||
d3 = r._temporal_decay((now - timedelta(days=60)).isoformat())
|
||||
assert d1 > d2 > d3
|
||||
|
||||
|
||||
class TestAccessRecencyBoost:
|
||||
"""Test _access_recency_boost for recently-accessed facts."""
|
||||
|
||||
def _make_retriever(self, half_life=60):
|
||||
from plugins.memory.holographic.retrieval import FactRetriever
|
||||
store = MagicMock()
|
||||
return FactRetriever(store=store, temporal_decay_half_life=half_life)
|
||||
|
||||
def test_just_accessed_max_boost(self):
|
||||
"""A fact accessed just now should get maximum boost (1.5)."""
|
||||
r = self._make_retriever(half_life=60)
|
||||
now = datetime.now(timezone.utc).isoformat()
|
||||
boost = r._access_recency_boost(now)
|
||||
assert boost > 1.45 # Near 1.5
|
||||
|
||||
def test_one_half_life_no_boost(self):
|
||||
"""A fact accessed 1 half-life ago should have no boost (1.0)."""
|
||||
r = self._make_retriever(half_life=60)
|
||||
old = (datetime.now(timezone.utc) - timedelta(days=60)).isoformat()
|
||||
boost = r._access_recency_boost(old)
|
||||
assert abs(boost - 1.0) < 0.01
|
||||
|
||||
def test_half_way_boost(self):
|
||||
"""A fact accessed 0.5 half-lives ago should get ~1.25 boost."""
|
||||
r = self._make_retriever(half_life=60)
|
||||
old = (datetime.now(timezone.utc) - timedelta(days=30)).isoformat()
|
||||
boost = r._access_recency_boost(old)
|
||||
assert abs(boost - 1.25) < 0.05
|
||||
|
||||
def test_beyond_one_half_life_no_boost(self):
|
||||
"""Beyond 1 half-life, boost should be 1.0."""
|
||||
r = self._make_retriever(half_life=60)
|
||||
old = (datetime.now(timezone.utc) - timedelta(days=90)).isoformat()
|
||||
boost = r._access_recency_boost(old)
|
||||
assert boost == 1.0
|
||||
|
||||
def test_disabled_no_boost(self):
|
||||
"""When half_life=0, boost should be 1.0."""
|
||||
r = self._make_retriever(half_life=0)
|
||||
now = datetime.now(timezone.utc).isoformat()
|
||||
assert r._access_recency_boost(now) == 1.0
|
||||
|
||||
def test_none_timestamp(self):
|
||||
r = self._make_retriever(half_life=60)
|
||||
assert r._access_recency_boost(None) == 1.0
|
||||
|
||||
def test_invalid_timestamp(self):
|
||||
r = self._make_retriever(half_life=60)
|
||||
assert r._access_recency_boost("bad") == 1.0
|
||||
|
||||
def test_boost_range(self):
|
||||
"""Boost should always be in [1.0, 1.5]."""
|
||||
r = self._make_retriever(half_life=60)
|
||||
now = datetime.now(timezone.utc)
|
||||
for days in [0, 1, 15, 30, 45, 59, 60, 90, 365]:
|
||||
ts = (now - timedelta(days=days)).isoformat()
|
||||
boost = r._access_recency_boost(ts)
|
||||
assert 1.0 <= boost <= 1.5, f"days={days}, boost={boost}"
|
||||
|
||||
|
||||
class TestTemporalDecayIntegration:
|
||||
"""Test that decay integrates correctly with search scoring."""
|
||||
|
||||
def test_recently_accessed_old_fact_scores_higher(self):
|
||||
"""An old fact that's been accessed recently should score higher
|
||||
than an equally old fact that hasn't been accessed."""
|
||||
from plugins.memory.holographic.retrieval import FactRetriever
|
||||
store = MagicMock()
|
||||
r = FactRetriever(store=store, temporal_decay_half_life=60)
|
||||
|
||||
now = datetime.now(timezone.utc)
|
||||
old_date = (now - timedelta(days=120)).isoformat() # 2 half-lives old
|
||||
recent_access = (now - timedelta(days=10)).isoformat() # accessed 10 days ago
|
||||
old_access = (now - timedelta(days=200)).isoformat() # accessed 200 days ago
|
||||
|
||||
# Old fact, recently accessed
|
||||
decay1 = r._temporal_decay(old_date)
|
||||
boost1 = r._access_recency_boost(recent_access)
|
||||
effective1 = min(1.0, decay1 * boost1)
|
||||
|
||||
# Old fact, not recently accessed
|
||||
decay2 = r._temporal_decay(old_date)
|
||||
boost2 = r._access_recency_boost(old_access)
|
||||
effective2 = min(1.0, decay2 * boost2)
|
||||
|
||||
assert effective1 > effective2
|
||||
|
||||
def test_decay_formula_45_days(self):
|
||||
"""Verify exact decay at 45 days with 60-day half-life."""
|
||||
from plugins.memory.holographic.retrieval import FactRetriever
|
||||
r = FactRetriever(store=MagicMock(), temporal_decay_half_life=60)
|
||||
old = (datetime.now(timezone.utc) - timedelta(days=45)).isoformat()
|
||||
decay = r._temporal_decay(old)
|
||||
expected = math.pow(0.5, 45/60)
|
||||
assert abs(decay - expected) < 0.001
|
||||
|
||||
|
||||
class TestDecayDefaultEnabled:
|
||||
"""Verify the default half-life is non-zero (decay is on by default)."""
|
||||
|
||||
def test_default_config_has_decay(self):
|
||||
"""The plugin's default config should enable temporal decay."""
|
||||
from plugins.memory.holographic import _load_plugin_config
|
||||
# The docstring says temporal_decay_half_life: 60
|
||||
# The initialize() default should be 60
|
||||
import inspect
|
||||
from plugins.memory.holographic import HolographicMemoryProvider
|
||||
src = inspect.getsource(HolographicMemoryProvider.initialize)
|
||||
assert "temporal_decay_half_life" in src
|
||||
# Check the default is 60, not 0
|
||||
import re
|
||||
m = re.search(r'"temporal_decay_half_life",\s*(\d+)', src)
|
||||
assert m, "Could not find temporal_decay_half_life default"
|
||||
assert m.group(1) == "60", f"Default is {m.group(1)}, expected 60"
|
||||
@@ -137,3 +137,78 @@ class TestBackwardCompat:
|
||||
def test_tool_to_toolset_map(self):
|
||||
assert isinstance(TOOL_TO_TOOLSET_MAP, dict)
|
||||
assert len(TOOL_TO_TOOLSET_MAP) > 0
|
||||
|
||||
|
||||
class TestToolReturnTypeValidation:
|
||||
"""Poka-yoke: tool handlers must return JSON strings."""
|
||||
|
||||
def test_handler_returning_dict_is_wrapped(self, monkeypatch):
|
||||
"""A handler that returns a dict should be auto-wrapped to JSON string."""
|
||||
from tools.registry import registry
|
||||
from model_tools import handle_function_call
|
||||
import json
|
||||
|
||||
# Register a bad handler that returns dict instead of str
|
||||
registry.register(
|
||||
name="__test_bad_dict",
|
||||
toolset="test",
|
||||
schema={"name": "__test_bad_dict", "description": "test", "parameters": {"type": "object", "properties": {}}},
|
||||
handler=lambda args, **kw: {"this is": "a dict not a string"},
|
||||
)
|
||||
result = handle_function_call("__test_bad_dict", {})
|
||||
parsed = json.loads(result)
|
||||
assert "output" in parsed
|
||||
assert "_type_warning" in parsed
|
||||
# Cleanup
|
||||
registry._tools.pop("__test_bad_dict", None)
|
||||
|
||||
def test_handler_returning_none_is_wrapped(self, monkeypatch):
|
||||
"""A handler that returns None should be auto-wrapped."""
|
||||
from tools.registry import registry
|
||||
from model_tools import handle_function_call
|
||||
import json
|
||||
|
||||
registry.register(
|
||||
name="__test_bad_none",
|
||||
toolset="test",
|
||||
schema={"name": "__test_bad_none", "description": "test", "parameters": {"type": "object", "properties": {}}},
|
||||
handler=lambda args, **kw: None,
|
||||
)
|
||||
result = handle_function_call("__test_bad_none", {})
|
||||
parsed = json.loads(result)
|
||||
assert "_type_warning" in parsed
|
||||
registry._tools.pop("__test_bad_none", None)
|
||||
|
||||
def test_handler_returning_non_json_string_is_wrapped(self):
|
||||
"""A handler returning a plain string (not JSON) should be wrapped."""
|
||||
from tools.registry import registry
|
||||
from model_tools import handle_function_call
|
||||
import json
|
||||
|
||||
registry.register(
|
||||
name="__test_bad_plain",
|
||||
toolset="test",
|
||||
schema={"name": "__test_bad_plain", "description": "test", "parameters": {"type": "object", "properties": {}}},
|
||||
handler=lambda args, **kw: "just a plain string, not json",
|
||||
)
|
||||
result = handle_function_call("__test_bad_plain", {})
|
||||
parsed = json.loads(result)
|
||||
assert "output" in parsed
|
||||
registry._tools.pop("__test_bad_plain", None)
|
||||
|
||||
def test_handler_returning_valid_json_passes_through(self):
|
||||
"""A handler returning valid JSON string passes through unchanged."""
|
||||
from tools.registry import registry
|
||||
from model_tools import handle_function_call
|
||||
import json
|
||||
|
||||
registry.register(
|
||||
name="__test_good",
|
||||
toolset="test",
|
||||
schema={"name": "__test_good", "description": "test", "parameters": {"type": "object", "properties": {}}},
|
||||
handler=lambda args, **kw: json.dumps({"status": "ok", "data": [1, 2, 3]}),
|
||||
)
|
||||
result = handle_function_call("__test_good", {})
|
||||
parsed = json.loads(result)
|
||||
assert parsed == {"status": "ok", "data": [1, 2, 3]}
|
||||
registry._tools.pop("__test_good", None)
|
||||
|
||||
@@ -144,7 +144,8 @@ class TestMemoryStoreReplace:
|
||||
def test_replace_no_match(self, store):
|
||||
store.add("memory", "fact A")
|
||||
result = store.replace("memory", "nonexistent", "new")
|
||||
assert result["success"] is False
|
||||
assert result["success"] is True
|
||||
assert result["result"] == "no_match"
|
||||
|
||||
def test_replace_ambiguous_match(self, store):
|
||||
store.add("memory", "server A runs nginx")
|
||||
@@ -177,7 +178,8 @@ class TestMemoryStoreRemove:
|
||||
|
||||
def test_remove_no_match(self, store):
|
||||
result = store.remove("memory", "nonexistent")
|
||||
assert result["success"] is False
|
||||
assert result["success"] is True
|
||||
assert result["result"] == "no_match"
|
||||
|
||||
def test_remove_empty_old_text(self, store):
|
||||
result = store.remove("memory", " ")
|
||||
|
||||
107
tests/tools/test_syntax_preflight.py
Normal file
107
tests/tools/test_syntax_preflight.py
Normal file
@@ -0,0 +1,107 @@
|
||||
"""Tests for syntax preflight check in execute_code (issue #312)."""
|
||||
|
||||
import ast
|
||||
import json
|
||||
import pytest
|
||||
|
||||
|
||||
class TestSyntaxPreflight:
|
||||
"""Verify that execute_code catches syntax errors before sandbox execution."""
|
||||
|
||||
def test_valid_syntax_passes_parse(self):
|
||||
"""Valid Python should pass ast.parse."""
|
||||
code = "print('hello')\nx = 1 + 2\n"
|
||||
ast.parse(code) # should not raise
|
||||
|
||||
def test_syntax_error_indentation(self):
|
||||
"""IndentationError is a subclass of SyntaxError."""
|
||||
code = "def foo():\nbar()\n"
|
||||
with pytest.raises(SyntaxError):
|
||||
ast.parse(code)
|
||||
|
||||
def test_syntax_error_missing_colon(self):
|
||||
code = "if True\n pass\n"
|
||||
with pytest.raises(SyntaxError):
|
||||
ast.parse(code)
|
||||
|
||||
def test_syntax_error_unmatched_paren(self):
|
||||
code = "x = (1 + 2\n"
|
||||
with pytest.raises(SyntaxError):
|
||||
ast.parse(code)
|
||||
|
||||
def test_syntax_error_invalid_token(self):
|
||||
code = "x = 1 +*\n"
|
||||
with pytest.raises(SyntaxError):
|
||||
ast.parse(code)
|
||||
|
||||
def test_syntax_error_details(self):
|
||||
"""SyntaxError should provide line, offset, msg."""
|
||||
code = "if True\n pass\n"
|
||||
with pytest.raises(SyntaxError) as exc_info:
|
||||
ast.parse(code)
|
||||
e = exc_info.value
|
||||
assert e.lineno is not None
|
||||
assert e.msg is not None
|
||||
|
||||
def test_empty_string_passes(self):
|
||||
"""Empty string is valid Python (empty module)."""
|
||||
ast.parse("")
|
||||
|
||||
def test_comments_only_passes(self):
|
||||
ast.parse("# just a comment\n# another\n")
|
||||
|
||||
def test_complex_valid_code(self):
|
||||
code = '''
|
||||
import os
|
||||
def foo(x):
|
||||
if x > 0:
|
||||
return x * 2
|
||||
return 0
|
||||
|
||||
result = [foo(i) for i in range(10)]
|
||||
print(result)
|
||||
'''
|
||||
ast.parse(code)
|
||||
|
||||
|
||||
class TestSyntaxPreflightResponse:
|
||||
"""Test the error response format from the preflight check."""
|
||||
|
||||
def _check_syntax(self, code):
|
||||
"""Mimic the preflight check logic from execute_code."""
|
||||
try:
|
||||
ast.parse(code)
|
||||
return None
|
||||
except SyntaxError as e:
|
||||
return json.dumps({
|
||||
"error": f"Python syntax error: {e.msg}",
|
||||
"line": e.lineno,
|
||||
"offset": e.offset,
|
||||
"text": (e.text or "").strip()[:200],
|
||||
})
|
||||
|
||||
def test_returns_json_error(self):
|
||||
result = self._check_syntax("if True\n pass\n")
|
||||
assert result is not None
|
||||
data = json.loads(result)
|
||||
assert "error" in data
|
||||
assert "syntax error" in data["error"].lower()
|
||||
|
||||
def test_includes_line_number(self):
|
||||
result = self._check_syntax("x = 1\nif True\n pass\n")
|
||||
data = json.loads(result)
|
||||
assert data["line"] == 2 # error on line 2
|
||||
|
||||
def test_includes_offset(self):
|
||||
result = self._check_syntax("x = (1 + 2\n")
|
||||
data = json.loads(result)
|
||||
assert data["offset"] is not None
|
||||
|
||||
def test_includes_snippet(self):
|
||||
result = self._check_syntax("if True\n")
|
||||
data = json.loads(result)
|
||||
assert "if True" in data["text"]
|
||||
|
||||
def test_none_for_valid_code(self):
|
||||
result = self._check_syntax("print('ok')")
|
||||
assert result is None
|
||||
@@ -28,6 +28,7 @@ Platform: Linux / macOS only (Unix domain sockets for local). Disabled on Window
|
||||
Remote execution additionally requires Python 3 in the terminal backend.
|
||||
"""
|
||||
|
||||
import ast
|
||||
import base64
|
||||
import json
|
||||
import logging
|
||||
@@ -893,6 +894,20 @@ def execute_code(
|
||||
if not code or not code.strip():
|
||||
return json.dumps({"error": "No code provided."})
|
||||
|
||||
# Poka-yoke (#312): Syntax check before execution.
|
||||
# 83.2% of execute_code errors are Python exceptions; most are syntax
|
||||
# errors the LLM generated. ast.parse() is sub-millisecond and catches
|
||||
# them before we spin up a sandbox child process.
|
||||
try:
|
||||
ast.parse(code)
|
||||
except SyntaxError as e:
|
||||
return json.dumps({
|
||||
"error": f"Python syntax error: {e.msg}",
|
||||
"line": e.lineno,
|
||||
"offset": e.offset,
|
||||
"text": (e.text or "").strip()[:200],
|
||||
})
|
||||
|
||||
# Dispatch: remote backends use file-based RPC, local uses UDS
|
||||
from tools.terminal_tool import _get_env_config
|
||||
env_type = _get_env_config()["env_type"]
|
||||
|
||||
@@ -260,8 +260,12 @@ class MemoryStore:
|
||||
entries = self._entries_for(target)
|
||||
matches = [(i, e) for i, e in enumerate(entries) if old_text in e]
|
||||
|
||||
if len(matches) == 0:
|
||||
return {"success": False, "error": f"No entry matched '{old_text}'."}
|
||||
if not matches:
|
||||
return {
|
||||
"success": True,
|
||||
"result": "no_match",
|
||||
"message": f"No entry matched '{old_text}'. The search substring was not found in any existing entry.",
|
||||
}
|
||||
|
||||
if len(matches) > 1:
|
||||
# If all matches are identical (exact duplicates), operate on the first one
|
||||
@@ -310,8 +314,12 @@ class MemoryStore:
|
||||
entries = self._entries_for(target)
|
||||
matches = [(i, e) for i, e in enumerate(entries) if old_text in e]
|
||||
|
||||
if len(matches) == 0:
|
||||
return {"success": False, "error": f"No entry matched '{old_text}'."}
|
||||
if not matches:
|
||||
return {
|
||||
"success": True,
|
||||
"result": "no_match",
|
||||
"message": f"No entry matched '{old_text}'. The search substring was not found in any existing entry.",
|
||||
}
|
||||
|
||||
if len(matches) > 1:
|
||||
# If all matches are identical (exact duplicates), remove the first one
|
||||
@@ -449,30 +457,30 @@ def memory_tool(
|
||||
Returns JSON string with results.
|
||||
"""
|
||||
if store is None:
|
||||
return json.dumps({"success": False, "error": "Memory is not available. It may be disabled in config or this environment."}, ensure_ascii=False)
|
||||
return tool_error("Memory is not available. It may be disabled in config or this environment.", success=False)
|
||||
|
||||
if target not in ("memory", "user"):
|
||||
return json.dumps({"success": False, "error": f"Invalid target '{target}'. Use 'memory' or 'user'."}, ensure_ascii=False)
|
||||
return tool_error(f"Invalid target '{target}'. Use 'memory' or 'user'.", success=False)
|
||||
|
||||
if action == "add":
|
||||
if not content:
|
||||
return json.dumps({"success": False, "error": "Content is required for 'add' action."}, ensure_ascii=False)
|
||||
return tool_error("Content is required for 'add' action.", success=False)
|
||||
result = store.add(target, content)
|
||||
|
||||
elif action == "replace":
|
||||
if not old_text:
|
||||
return json.dumps({"success": False, "error": "old_text is required for 'replace' action."}, ensure_ascii=False)
|
||||
return tool_error("old_text is required for 'replace' action.", success=False)
|
||||
if not content:
|
||||
return json.dumps({"success": False, "error": "content is required for 'replace' action."}, ensure_ascii=False)
|
||||
return tool_error("content is required for 'replace' action.", success=False)
|
||||
result = store.replace(target, old_text, content)
|
||||
|
||||
elif action == "remove":
|
||||
if not old_text:
|
||||
return json.dumps({"success": False, "error": "old_text is required for 'remove' action."}, ensure_ascii=False)
|
||||
return tool_error("old_text is required for 'remove' action.", success=False)
|
||||
result = store.remove(target, old_text)
|
||||
|
||||
else:
|
||||
return json.dumps({"success": False, "error": f"Unknown action '{action}'. Use: add, replace, remove"}, ensure_ascii=False)
|
||||
return tool_error(f"Unknown action '{action}'. Use: add, replace, remove", success=False)
|
||||
|
||||
return json.dumps(result, ensure_ascii=False)
|
||||
|
||||
@@ -539,7 +547,7 @@ MEMORY_SCHEMA = {
|
||||
|
||||
|
||||
# --- Registry ---
|
||||
from tools.registry import registry
|
||||
from tools.registry import registry, tool_error
|
||||
|
||||
registry.register(
|
||||
name="memory",
|
||||
|
||||
Reference in New Issue
Block a user