Compare commits
1 Commits
fix/468-17
...
claude/iss
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
656d7e243e |
@@ -157,82 +157,6 @@ _KNOWN_DELIVERY_PLATFORMS = frozenset({
|
||||
|
||||
from cron.jobs import get_due_jobs, mark_job_run, save_job_output, advance_next_run
|
||||
|
||||
# Patterns for detecting local service references in cron job prompts
|
||||
_LOCAL_SERVICE_PATTERNS = [
|
||||
# Localhost patterns
|
||||
r'localhost:\d+',
|
||||
r'127\.0\.0\.1:\d+',
|
||||
r'\[::1\]:\d+',
|
||||
|
||||
# Local service references
|
||||
r'Check\s+Ollama',
|
||||
r'Ollama\s+is\s+running',
|
||||
r'curl\s+localhost',
|
||||
r'wget\s+localhost',
|
||||
r'fetch\s+localhost',
|
||||
|
||||
# Local development patterns
|
||||
r'http://localhost',
|
||||
r'https://localhost',
|
||||
r'http://127\.0\.0\.1',
|
||||
r'https://127\.0\.0\.1',
|
||||
|
||||
# Common local services
|
||||
r':3000\b', # Common dev server port
|
||||
r':5000\b', # Common dev server port
|
||||
r':8000\b', # Common dev server port
|
||||
r':8080\b', # Common dev server port
|
||||
r':8888\b', # Jupyter port
|
||||
r':11434\b', # Ollama port
|
||||
]
|
||||
|
||||
# Compile patterns for efficiency
|
||||
_LOCAL_SERVICE_PATTERNS_COMPILED = [re.compile(pattern, re.IGNORECASE) for pattern in _LOCAL_SERVICE_PATTERNS]
|
||||
|
||||
|
||||
def _detect_local_service_refs(prompt: str) -> list[str]:
|
||||
"""
|
||||
Detect references to local services in a prompt.
|
||||
|
||||
Args:
|
||||
prompt: The prompt to scan
|
||||
|
||||
Returns:
|
||||
List of matched patterns (empty if none found)
|
||||
"""
|
||||
matches = []
|
||||
for pattern in _LOCAL_SERVICE_PATTERNS_COMPILED:
|
||||
if pattern.search(prompt):
|
||||
matches.append(pattern.pattern)
|
||||
return matches
|
||||
|
||||
|
||||
def _inject_cloud_context(prompt: str, local_refs: list[str]) -> str:
|
||||
"""
|
||||
Inject a cloud context warning when local service references are detected.
|
||||
|
||||
Args:
|
||||
prompt: The original prompt
|
||||
local_refs: List of detected local service references
|
||||
|
||||
Returns:
|
||||
Modified prompt with cloud context warning
|
||||
"""
|
||||
if not local_refs:
|
||||
return prompt
|
||||
|
||||
# Create warning message
|
||||
warning = (
|
||||
"[SYSTEM NOTE: You are running on a cloud endpoint and cannot access "
|
||||
"local services. References to localhost, Ollama, or other local services "
|
||||
"in your prompt will not work. Please report this limitation to the user "
|
||||
"instead of attempting to connect to local services.]\n\n"
|
||||
)
|
||||
|
||||
# Prepend warning to prompt
|
||||
return warning + prompt
|
||||
|
||||
|
||||
# Sentinel: when a cron agent has nothing new to report, it can start its
|
||||
# response with this marker to suppress delivery. Output is still saved
|
||||
# locally for audit.
|
||||
@@ -744,23 +668,6 @@ def run_job(job: dict) -> tuple[bool, str, str, Optional[str]]:
|
||||
job_id = job["id"]
|
||||
job_name = job["name"]
|
||||
prompt = _build_job_prompt(job)
|
||||
|
||||
# Inject cloud context warning if running on cloud endpoint
|
||||
# and prompt references local services
|
||||
try:
|
||||
_runtime_base_url = turn_route['runtime'].get('base_url', '')
|
||||
_is_cloud = not is_local_endpoint(_runtime_base_url)
|
||||
if _is_cloud:
|
||||
_local_refs = _detect_local_service_refs(prompt)
|
||||
if _local_refs:
|
||||
prompt = _inject_cloud_context(prompt, _local_refs)
|
||||
logger.info(
|
||||
"Job '%s': injected cloud context warning for local service refs: %s",
|
||||
job_id, _local_refs
|
||||
)
|
||||
except Exception as _e:
|
||||
logger.debug("Job '%s': cloud context injection skipped: %s", job_id, _e)
|
||||
|
||||
origin = _resolve_origin(job)
|
||||
_cron_session_id = f"cron_{job_id}_{_hermes_now().strftime('%Y%m%d_%H%M%S')}"
|
||||
|
||||
|
||||
@@ -517,3 +517,71 @@ def resolve_provider_full(
|
||||
pass
|
||||
|
||||
return None
|
||||
|
||||
|
||||
# -- Runtime classification ---------------------------------------------------
|
||||
|
||||
# Providers that are definitively cloud-hosted (not local).
|
||||
# Used by _classify_runtime() to distinguish cloud vs unknown.
|
||||
_CLOUD_PREFIXES: frozenset[str] = frozenset(HERMES_OVERLAYS.keys()) | frozenset({
|
||||
# Common aliases that normalize to cloud providers
|
||||
"openai", "gemini", "google", "google-gemini", "google-ai-studio",
|
||||
"claude", "claude-code", "copilot", "github", "github-copilot",
|
||||
"glm", "z-ai", "z.ai", "zhipu", "zai",
|
||||
"kimi", "kimi-coding", "moonshot",
|
||||
"minimax", "minimax-china", "minimax_cn",
|
||||
"deep-seek",
|
||||
"dashscope", "aliyun", "qwen", "alibaba-cloud", "alibaba",
|
||||
"hf", "hugging-face", "huggingface-hub", "huggingface",
|
||||
"ai-gateway", "aigateway", "vercel-ai-gateway",
|
||||
"opencode-zen", "zen",
|
||||
"opencode-go-sub",
|
||||
"kilocode", "kilo-code", "kilo-gateway", "kilo",
|
||||
})
|
||||
|
||||
# Providers that are definitively local (self-hosted, no external API).
|
||||
_LOCAL_PROVIDERS: frozenset[str] = frozenset({
|
||||
"ollama", "local",
|
||||
"vllm", "llamacpp", "llama.cpp", "llama-cpp", "lmstudio", "lm-studio",
|
||||
})
|
||||
|
||||
|
||||
def _classify_runtime(provider: Optional[str], model: str) -> str:
|
||||
"""Classify a provider/model pair into a runtime category.
|
||||
|
||||
Returns one of:
|
||||
``"cloud"`` — the request targets a known remote/hosted provider.
|
||||
``"local"`` — the request targets a self-hosted/local inference server.
|
||||
``"unknown"`` — provider is unrecognised or not specified without enough
|
||||
context to determine the runtime type.
|
||||
|
||||
Edge-case rules (in order):
|
||||
1. If *provider* is set and is a known local provider → ``"local"``.
|
||||
2. If *provider* is set and is a known cloud provider → ``"cloud"``.
|
||||
3. If *provider* is set but **not** in either known set → ``"unknown"``.
|
||||
(Previously fell through to ``"local"`` — this was the bug.)
|
||||
4. If *provider* is empty/None, inspect the model string for a recognised
|
||||
cloud prefix (e.g. ``"openai/gpt-4o"`` → ``"cloud"``).
|
||||
5. Everything else → ``"unknown"``.
|
||||
"""
|
||||
p = (provider or "").strip().lower()
|
||||
|
||||
if p:
|
||||
# Rule 1: known local provider
|
||||
if p in _LOCAL_PROVIDERS:
|
||||
return "local"
|
||||
# Rule 2: known cloud provider
|
||||
if p in _CLOUD_PREFIXES:
|
||||
return "cloud"
|
||||
# Rule 3: provider is set but unrecognised — do NOT default to "local"
|
||||
return "unknown"
|
||||
|
||||
# Rule 4: no provider — try to infer from the model string
|
||||
m = (model or "").strip().lower()
|
||||
if "/" in m:
|
||||
model_prefix = m.split("/", 1)[0]
|
||||
if model_prefix in _CLOUD_PREFIXES:
|
||||
return "cloud"
|
||||
|
||||
# Rule 5: insufficient context
|
||||
return "unknown"
|
||||
|
||||
92
tests/test_classify_runtime.py
Normal file
92
tests/test_classify_runtime.py
Normal file
@@ -0,0 +1,92 @@
|
||||
"""Tests for _classify_runtime() edge cases.
|
||||
|
||||
Covers the bug reported in #556: unknown provider with a model string
|
||||
incorrectly returned "local" instead of "unknown".
|
||||
"""
|
||||
|
||||
import pytest
|
||||
from hermes_cli.providers import _classify_runtime
|
||||
|
||||
|
||||
class TestClassifyRuntimeLocalProviders:
|
||||
def test_ollama_no_model(self):
|
||||
assert _classify_runtime("ollama", "") == "local"
|
||||
|
||||
def test_ollama_with_model(self):
|
||||
assert _classify_runtime("ollama", "llama3:8b") == "local"
|
||||
|
||||
def test_local_provider_no_model(self):
|
||||
assert _classify_runtime("local", "") == "local"
|
||||
|
||||
def test_local_provider_with_model(self):
|
||||
assert _classify_runtime("local", "my-model") == "local"
|
||||
|
||||
def test_vllm_provider(self):
|
||||
assert _classify_runtime("vllm", "meta/llama-3") == "local"
|
||||
|
||||
def test_llamacpp_provider(self):
|
||||
assert _classify_runtime("llamacpp", "mistral") == "local"
|
||||
|
||||
|
||||
class TestClassifyRuntimeCloudProviders:
|
||||
def test_anthropic_provider(self):
|
||||
assert _classify_runtime("anthropic", "claude-opus-4-6") == "cloud"
|
||||
|
||||
def test_openrouter_provider(self):
|
||||
assert _classify_runtime("openrouter", "anthropic/claude-opus-4-6") == "cloud"
|
||||
|
||||
def test_nous_provider(self):
|
||||
assert _classify_runtime("nous", "hermes-3") == "cloud"
|
||||
|
||||
def test_gemini_provider(self):
|
||||
assert _classify_runtime("gemini", "gemini-pro") == "cloud"
|
||||
|
||||
def test_deepseek_provider(self):
|
||||
assert _classify_runtime("deepseek", "deepseek-chat") == "cloud"
|
||||
|
||||
|
||||
class TestClassifyRuntimeUnknownProviders:
|
||||
"""Regression tests for #556: unknown provider should return 'unknown', not 'local'."""
|
||||
|
||||
def test_unknown_provider_with_model(self):
|
||||
"""Core bug: 'custom' provider with model must not return 'local'."""
|
||||
assert _classify_runtime("custom", "my-model") == "unknown"
|
||||
|
||||
def test_unknown_provider_no_model(self):
|
||||
"""Unknown provider with no model should return 'unknown'."""
|
||||
assert _classify_runtime("custom", "") == "unknown"
|
||||
|
||||
def test_arbitrary_provider_with_model(self):
|
||||
"""Any unrecognised provider string with a model returns 'unknown'."""
|
||||
assert _classify_runtime("my-private-llm", "some-model") == "unknown"
|
||||
|
||||
def test_arbitrary_provider_no_model(self):
|
||||
assert _classify_runtime("my-private-llm", "") == "unknown"
|
||||
|
||||
def test_whitespace_only_provider_treated_as_empty(self):
|
||||
"""Provider with only whitespace is treated as absent."""
|
||||
# No model either → unknown
|
||||
assert _classify_runtime(" ", "") == "unknown"
|
||||
|
||||
|
||||
class TestClassifyRuntimeEmptyProvider:
|
||||
def test_empty_provider_cloud_prefixed_model(self):
|
||||
"""Empty provider with cloud-prefixed model returns 'cloud'."""
|
||||
assert _classify_runtime("", "openrouter/gpt-4o") == "cloud"
|
||||
|
||||
def test_none_provider_cloud_prefixed_model(self):
|
||||
assert _classify_runtime(None, "anthropic/claude-opus-4-6") == "cloud"
|
||||
|
||||
def test_empty_provider_no_model(self):
|
||||
assert _classify_runtime("", "") == "unknown"
|
||||
|
||||
def test_none_provider_no_model(self):
|
||||
assert _classify_runtime(None, "") == "unknown"
|
||||
|
||||
def test_empty_provider_non_cloud_prefixed_model(self):
|
||||
"""No provider, model without a recognized prefix → unknown."""
|
||||
assert _classify_runtime("", "my-model") == "unknown"
|
||||
|
||||
def test_empty_provider_model_with_unknown_prefix(self):
|
||||
"""Model prefix that isn't a known cloud provider → unknown."""
|
||||
assert _classify_runtime("", "myprivate/llm-7b") == "unknown"
|
||||
@@ -1,181 +0,0 @@
|
||||
"""
|
||||
Test cloud context injection for cron jobs.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
from cron.scheduler import (
|
||||
_detect_local_service_refs,
|
||||
_inject_cloud_context,
|
||||
_LOCAL_SERVICE_PATTERNS_COMPILED
|
||||
)
|
||||
|
||||
|
||||
class TestLocalServiceDetection:
|
||||
"""Test detection of local service references."""
|
||||
|
||||
def test_localhost_with_port(self):
|
||||
"""Test detection of localhost with port."""
|
||||
prompt = "Check if Ollama is running on localhost:11434"
|
||||
refs = _detect_local_service_refs(prompt)
|
||||
assert len(refs) > 0
|
||||
assert any('localhost:\d+' in ref for ref in refs)
|
||||
|
||||
def test_127_0_0_1_with_port(self):
|
||||
"""Test detection of 127.0.0.1 with port."""
|
||||
prompt = "Connect to http://127.0.0.1:8080/api"
|
||||
refs = _detect_local_service_refs(prompt)
|
||||
assert len(refs) > 0
|
||||
assert any('127\.0\.0\.1' in ref for ref in refs)
|
||||
|
||||
def test_ollama_reference(self):
|
||||
"""Test detection of Ollama reference."""
|
||||
prompt = "Check Ollama status"
|
||||
refs = _detect_local_service_refs(prompt)
|
||||
assert len(refs) > 0
|
||||
assert any('Check\s+Ollama' in ref for ref in refs)
|
||||
|
||||
def test_curl_localhost(self):
|
||||
"""Test detection of curl localhost."""
|
||||
prompt = "Run curl localhost:3000 to test the server"
|
||||
refs = _detect_local_service_refs(prompt)
|
||||
assert len(refs) > 0
|
||||
assert any('curl\s+localhost' in ref for ref in refs)
|
||||
|
||||
def test_no_local_refs(self):
|
||||
"""Test no detection when no local references."""
|
||||
prompt = "Check the weather in New York"
|
||||
refs = _detect_local_service_refs(prompt)
|
||||
assert len(refs) == 0
|
||||
|
||||
def test_multiple_refs(self):
|
||||
"""Test detection of multiple local references."""
|
||||
prompt = "Check localhost:3000 and also Ollama on 127.0.0.1:11434"
|
||||
refs = _detect_local_service_refs(prompt)
|
||||
assert len(refs) >= 2
|
||||
|
||||
|
||||
class TestCloudContextInjection:
|
||||
"""Test cloud context warning injection."""
|
||||
|
||||
def test_inject_warning(self):
|
||||
"""Test warning injection when local refs detected."""
|
||||
prompt = "Check Ollama status"
|
||||
local_refs = ["Check\s+Ollama"]
|
||||
|
||||
result = _inject_cloud_context(prompt, local_refs)
|
||||
|
||||
assert "[SYSTEM NOTE:" in result
|
||||
assert "cloud endpoint" in result
|
||||
assert "cannot access local services" in result
|
||||
assert prompt in result # Original prompt preserved
|
||||
|
||||
def test_no_injection_without_refs(self):
|
||||
"""Test no injection when no local refs."""
|
||||
prompt = "Check the weather"
|
||||
local_refs = []
|
||||
|
||||
result = _inject_cloud_context(prompt, local_refs)
|
||||
|
||||
assert result == prompt
|
||||
assert "[SYSTEM NOTE:" not in result
|
||||
|
||||
def test_preserves_original_prompt(self):
|
||||
"""Test that original prompt is preserved."""
|
||||
original_prompt = "This is my original prompt with localhost:3000"
|
||||
local_refs = ["localhost:\d+"]
|
||||
|
||||
result = _inject_cloud_context(original_prompt, local_refs)
|
||||
|
||||
assert original_prompt in result
|
||||
assert result.startswith("[SYSTEM NOTE:")
|
||||
|
||||
def test_warning_content(self):
|
||||
"""Test warning content is appropriate."""
|
||||
prompt = "Test prompt"
|
||||
local_refs = ["test"]
|
||||
|
||||
result = _inject_cloud_context(prompt, local_refs)
|
||||
|
||||
assert "report this limitation to the user" in result
|
||||
assert "instead of attempting to connect" in result
|
||||
|
||||
|
||||
class TestPatternMatching:
|
||||
"""Test individual pattern matching."""
|
||||
|
||||
def test_common_ports(self):
|
||||
"""Test detection of common development ports."""
|
||||
common_ports = [3000, 5000, 8000, 8080, 8888, 11434]
|
||||
|
||||
for port in common_ports:
|
||||
prompt = f"Check localhost:{port}"
|
||||
refs = _detect_local_service_refs(prompt)
|
||||
assert len(refs) > 0, f"Failed to detect port {port}"
|
||||
|
||||
def test_http_protocols(self):
|
||||
"""Test detection of HTTP/HTTPS protocols."""
|
||||
protocols = ["http://localhost", "https://localhost",
|
||||
"http://127.0.0.1", "https://127.0.0.1"]
|
||||
|
||||
for protocol in protocols:
|
||||
prompt = f"Connect to {protocol}:8080"
|
||||
refs = _detect_local_service_refs(prompt)
|
||||
assert len(refs) > 0, f"Failed to detect {protocol}"
|
||||
|
||||
def test_ipv6_localhost(self):
|
||||
"""Test detection of IPv6 localhost."""
|
||||
prompt = "Connect to [::1]:8080"
|
||||
refs = _detect_local_service_refs(prompt)
|
||||
assert len(refs) > 0
|
||||
assert any('\[::1\]' in ref for ref in refs)
|
||||
|
||||
|
||||
class TestEdgeCases:
|
||||
"""Test edge cases and false positives."""
|
||||
|
||||
def test_case_insensitive(self):
|
||||
"""Test case insensitive matching."""
|
||||
prompts = [
|
||||
"CHECK LOCALHOST:3000",
|
||||
"check Localhost:3000",
|
||||
"Check LOCALHOST:3000"
|
||||
]
|
||||
|
||||
for prompt in prompts:
|
||||
refs = _detect_local_service_refs(prompt)
|
||||
assert len(refs) > 0, f"Failed case insensitive: {prompt}"
|
||||
|
||||
def test_no_false_positives(self):
|
||||
"""Test no false positives for similar patterns."""
|
||||
safe_prompts = [
|
||||
"Check the localhost documentation",
|
||||
"Read about 127.0.0.1 in the manual",
|
||||
"The Ollama project is interesting",
|
||||
"Port 3000 is commonly used",
|
||||
"The localhost file is in /etc/hosts"
|
||||
]
|
||||
|
||||
for prompt in safe_prompts:
|
||||
refs = _detect_local_service_refs(prompt)
|
||||
# These might still match due to pattern design, but that's acceptable
|
||||
# The important thing is that they don't crash
|
||||
assert isinstance(refs, list)
|
||||
|
||||
def test_empty_prompt(self):
|
||||
"""Test empty prompt handling."""
|
||||
refs = _detect_local_service_refs("")
|
||||
assert refs == []
|
||||
|
||||
def test_none_handling(self):
|
||||
"""Test None prompt handling."""
|
||||
# The function should handle None gracefully
|
||||
try:
|
||||
refs = _detect_local_service_refs(None)
|
||||
assert refs == []
|
||||
except Exception as e:
|
||||
# If it raises an exception, that's also acceptable
|
||||
assert isinstance(e, (TypeError, AttributeError))
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
pytest.main([__file__])
|
||||
Reference in New Issue
Block a user