fix(cron): warn and skip missing skills instead of crashing job
When a cron job references a skill that is no longer installed, _build_job_prompt() now logs a warning and injects a user-visible notice into the prompt instead of raising RuntimeError. The job continues with any remaining valid skills and the user prompt. Adds 4 regression tests for missing skill handling.
This commit is contained in:
parent
35558dadf4
commit
defbe0f9e9
2 changed files with 55 additions and 4 deletions
|
|
@ -207,11 +207,14 @@ def _build_job_prompt(job: dict) -> str:
|
||||||
from tools.skills_tool import skill_view
|
from tools.skills_tool import skill_view
|
||||||
|
|
||||||
parts = []
|
parts = []
|
||||||
|
skipped: list[str] = []
|
||||||
for skill_name in skill_names:
|
for skill_name in skill_names:
|
||||||
loaded = json.loads(skill_view(skill_name))
|
loaded = json.loads(skill_view(skill_name))
|
||||||
if not loaded.get("success"):
|
if not loaded.get("success"):
|
||||||
error = loaded.get("error") or f"Failed to load skill '{skill_name}'"
|
error = loaded.get("error") or f"Failed to load skill '{skill_name}'"
|
||||||
raise RuntimeError(error)
|
logger.warning("Cron job '%s': skill not found, skipping — %s", job.get("name", job.get("id")), error)
|
||||||
|
skipped.append(skill_name)
|
||||||
|
continue
|
||||||
|
|
||||||
content = str(loaded.get("content") or "").strip()
|
content = str(loaded.get("content") or "").strip()
|
||||||
if parts:
|
if parts:
|
||||||
|
|
@ -224,6 +227,15 @@ def _build_job_prompt(job: dict) -> str:
|
||||||
]
|
]
|
||||||
)
|
)
|
||||||
|
|
||||||
|
if skipped:
|
||||||
|
notice = (
|
||||||
|
f"[SYSTEM: The following skill(s) were listed for this job but could not be found "
|
||||||
|
f"and were skipped: {', '.join(skipped)}. "
|
||||||
|
f"Start your response with a brief notice so the user is aware, e.g.: "
|
||||||
|
f"'⚠️ Skill(s) not found and skipped: {', '.join(skipped)}']"
|
||||||
|
)
|
||||||
|
parts.insert(0, notice)
|
||||||
|
|
||||||
if prompt:
|
if prompt:
|
||||||
parts.extend(["", f"The user has provided the following instruction alongside the skill invocation: {prompt}"])
|
parts.extend(["", f"The user has provided the following instruction alongside the skill invocation: {prompt}"])
|
||||||
return "\n".join(parts)
|
return "\n".join(parts)
|
||||||
|
|
|
||||||
|
|
@ -7,7 +7,7 @@ from unittest.mock import AsyncMock, patch, MagicMock
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
from cron.scheduler import _resolve_origin, _resolve_delivery_target, _deliver_result, run_job, SILENT_MARKER
|
from cron.scheduler import _resolve_origin, _resolve_delivery_target, _deliver_result, run_job, SILENT_MARKER, _build_job_prompt
|
||||||
|
|
||||||
|
|
||||||
class TestResolveOrigin:
|
class TestResolveOrigin:
|
||||||
|
|
@ -532,14 +532,53 @@ class TestBuildJobPromptSilentHint:
|
||||||
"""Verify _build_job_prompt always injects [SILENT] guidance."""
|
"""Verify _build_job_prompt always injects [SILENT] guidance."""
|
||||||
|
|
||||||
def test_hint_always_present(self):
|
def test_hint_always_present(self):
|
||||||
from cron.scheduler import _build_job_prompt
|
|
||||||
job = {"prompt": "Check for updates"}
|
job = {"prompt": "Check for updates"}
|
||||||
result = _build_job_prompt(job)
|
result = _build_job_prompt(job)
|
||||||
assert "[SILENT]" in result
|
assert "[SILENT]" in result
|
||||||
assert "Check for updates" in result
|
assert "Check for updates" in result
|
||||||
|
|
||||||
def test_hint_present_even_without_prompt(self):
|
def test_hint_present_even_without_prompt(self):
|
||||||
from cron.scheduler import _build_job_prompt
|
|
||||||
job = {"prompt": ""}
|
job = {"prompt": ""}
|
||||||
result = _build_job_prompt(job)
|
result = _build_job_prompt(job)
|
||||||
assert "[SILENT]" in result
|
assert "[SILENT]" in result
|
||||||
|
|
||||||
|
|
||||||
|
class TestBuildJobPromptMissingSkill:
|
||||||
|
"""Verify that a missing skill logs a warning and does not crash the job."""
|
||||||
|
|
||||||
|
def _missing_skill_view(self, name: str) -> str:
|
||||||
|
return json.dumps({"success": False, "error": f"Skill '{name}' not found."})
|
||||||
|
|
||||||
|
def test_missing_skill_does_not_raise(self):
|
||||||
|
"""Job should run even when a referenced skill is not installed."""
|
||||||
|
with patch("tools.skills_tool.skill_view", side_effect=self._missing_skill_view):
|
||||||
|
result = _build_job_prompt({"skills": ["ghost-skill"], "prompt": "do something"})
|
||||||
|
# prompt is preserved even though skill was skipped
|
||||||
|
assert "do something" in result
|
||||||
|
|
||||||
|
def test_missing_skill_injects_user_notice_into_prompt(self):
|
||||||
|
"""A system notice about the missing skill is injected into the prompt."""
|
||||||
|
with patch("tools.skills_tool.skill_view", side_effect=self._missing_skill_view):
|
||||||
|
result = _build_job_prompt({"skills": ["ghost-skill"], "prompt": "do something"})
|
||||||
|
assert "ghost-skill" in result
|
||||||
|
assert "not found" in result.lower() or "skipped" in result.lower()
|
||||||
|
|
||||||
|
def test_missing_skill_logs_warning(self, caplog):
|
||||||
|
"""A warning is logged when a skill cannot be found."""
|
||||||
|
with caplog.at_level(logging.WARNING, logger="cron.scheduler"):
|
||||||
|
with patch("tools.skills_tool.skill_view", side_effect=self._missing_skill_view):
|
||||||
|
_build_job_prompt({"name": "My Job", "skills": ["ghost-skill"], "prompt": "do something"})
|
||||||
|
assert any("ghost-skill" in record.message for record in caplog.records)
|
||||||
|
|
||||||
|
def test_valid_skill_loaded_alongside_missing(self):
|
||||||
|
"""A valid skill is still loaded when another skill in the list is missing."""
|
||||||
|
|
||||||
|
def _mixed_skill_view(name: str) -> str:
|
||||||
|
if name == "real-skill":
|
||||||
|
return json.dumps({"success": True, "content": "Real skill content."})
|
||||||
|
return json.dumps({"success": False, "error": f"Skill '{name}' not found."})
|
||||||
|
|
||||||
|
with patch("tools.skills_tool.skill_view", side_effect=_mixed_skill_view):
|
||||||
|
result = _build_job_prompt({"skills": ["ghost-skill", "real-skill"], "prompt": "go"})
|
||||||
|
assert "Real skill content." in result
|
||||||
|
assert "go" in result
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue