mirror of
https://github.com/open-jarvis/OpenJarvis.git
synced 2026-08-14 08:52:06 +00:00
Compare commits
4
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
d5d8fddc94 | ||
|
|
cadb3e2ae6 | ||
|
|
7dc904c1b2 | ||
|
|
d9725fbb6a |
@@ -89,7 +89,7 @@ export default function App() {
|
||||
setSavings(data);
|
||||
if (optInEnabled && optInDisplayName && data) {
|
||||
const claudeEntry = data.per_provider.find(
|
||||
(p) => p.provider === 'claude-opus-4.6',
|
||||
(p) => p.provider === 'claude-fable-5',
|
||||
);
|
||||
const dollarSavings = claudeEntry ? claudeEntry.total_cost : 0;
|
||||
const energySaved = data.per_provider.reduce(
|
||||
|
||||
@@ -144,16 +144,21 @@ impl MemoryBackend for SQLiteMemory {
|
||||
) -> Result<Vec<RetrievalResult>, OpenJarvisError> {
|
||||
let conn = self.conn.lock();
|
||||
|
||||
// Split on any non-alphanumeric character (not just whitespace) so
|
||||
// internal punctuation — apostrophes in particular ("user's") — never
|
||||
// reaches the FTS5 MATCH string. FTS5's query grammar treats an
|
||||
// unescaped `'` as a string delimiter, so passing a raw token like
|
||||
// `user's` through silently fails to parse and yields zero rows with
|
||||
// no visible error. Splitting fully avoids needing to escape anything.
|
||||
let words: Vec<String> = query
|
||||
.split_whitespace()
|
||||
.map(|w| w.trim_matches(|c: char| "?.,!;:'\"()[]{}/ ".contains(c)).to_string())
|
||||
.split(|c: char| !c.is_alphanumeric())
|
||||
.map(|w| w.to_string())
|
||||
.filter(|w| !w.is_empty())
|
||||
.collect();
|
||||
let fts_query = if words.len() == 1 {
|
||||
words[0].clone()
|
||||
} else {
|
||||
words.join(" OR ")
|
||||
};
|
||||
if words.is_empty() {
|
||||
return Ok(Vec::new());
|
||||
}
|
||||
let fts_query = words.join(" OR ");
|
||||
|
||||
let mut stmt = conn
|
||||
.prepare(
|
||||
@@ -320,6 +325,27 @@ mod tests {
|
||||
assert_eq!(mixed.len(), 2, "mixed-case query should find both documents");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_sqlite_apostrophe_in_query() {
|
||||
let mem = SQLiteMemory::in_memory().unwrap();
|
||||
mem.store("The user's name is Trev.", "identity", None).unwrap();
|
||||
|
||||
// A query containing an internal apostrophe must not break FTS5's
|
||||
// MATCH syntax (an unescaped `'` is a string delimiter in FTS5's
|
||||
// query grammar), which previously caused this to silently return
|
||||
// zero results instead of matching or erroring.
|
||||
let multi_word = mem.retrieve("what is the user's name", 5).unwrap();
|
||||
assert!(
|
||||
!multi_word.is_empty(),
|
||||
"query with an internal apostrophe should not silently return zero results"
|
||||
);
|
||||
|
||||
// Bare single-word possessive: exercises the (former) single-word
|
||||
// bypass path that skipped the OR-join entirely.
|
||||
let bare = mem.retrieve("user's", 5).unwrap();
|
||||
assert!(!bare.is_empty(), "single-word possessive query should still match");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_sqlite_scores_are_positive() {
|
||||
let mem = SQLiteMemory::in_memory().unwrap();
|
||||
|
||||
@@ -199,7 +199,16 @@ def _get_resolver(source: str, url: str = ""):
|
||||
default="",
|
||||
help="Repo URL (required when source is 'github').",
|
||||
)
|
||||
def install(query: str, with_scripts: bool, force: bool, url: str):
|
||||
@click.option(
|
||||
"--yes-dangerous",
|
||||
is_flag=True,
|
||||
default=False,
|
||||
help=(
|
||||
"Confirm installing an unreviewed skill that requests dangerous "
|
||||
"capabilities (shell/network-listen/filesystem-write)."
|
||||
),
|
||||
)
|
||||
def install(query: str, with_scripts: bool, force: bool, url: str, yes_dangerous: bool):
|
||||
"""Install a skill from a source.
|
||||
|
||||
Example: ``jarvis skill install hermes:apple-notes``
|
||||
@@ -233,7 +242,12 @@ def install(query: str, with_scripts: bool, force: bool, url: str):
|
||||
from openjarvis.skills.tool_translator import ToolTranslator
|
||||
|
||||
importer = SkillImporter(parser=SkillParser(), tool_translator=ToolTranslator())
|
||||
result = importer.import_skill(matches[0], with_scripts=with_scripts, force=force)
|
||||
result = importer.import_skill(
|
||||
matches[0],
|
||||
with_scripts=with_scripts,
|
||||
force=force,
|
||||
confirm_dangerous=yes_dangerous,
|
||||
)
|
||||
|
||||
if result.success:
|
||||
if result.skipped:
|
||||
@@ -270,6 +284,15 @@ def install(query: str, with_scripts: bool, force: bool, url: str):
|
||||
help="Import scripts/ directories.",
|
||||
)
|
||||
@click.option("--force", is_flag=True, default=False, help="Re-import existing skills.")
|
||||
@click.option(
|
||||
"--yes-dangerous",
|
||||
is_flag=True,
|
||||
default=False,
|
||||
help=(
|
||||
"Confirm installing unreviewed skills that request dangerous "
|
||||
"capabilities (shell/network-listen/filesystem-write)."
|
||||
),
|
||||
)
|
||||
def sync(
|
||||
source: str,
|
||||
category: str,
|
||||
@@ -277,6 +300,7 @@ def sync(
|
||||
search: str,
|
||||
with_scripts: bool,
|
||||
force: bool,
|
||||
yes_dangerous: bool,
|
||||
):
|
||||
"""Bulk install + update from a source (or all configured sources)."""
|
||||
console = Console()
|
||||
@@ -343,9 +367,20 @@ def sync(
|
||||
|
||||
installed_count = 0
|
||||
for resolved in skills_to_import:
|
||||
r = importer.import_skill(resolved, with_scripts=with_scripts, force=force)
|
||||
r = importer.import_skill(
|
||||
resolved,
|
||||
with_scripts=with_scripts,
|
||||
force=force,
|
||||
confirm_dangerous=yes_dangerous,
|
||||
)
|
||||
if r.success and not r.skipped:
|
||||
installed_count += 1
|
||||
elif not r.success and r.requires_confirmation:
|
||||
console.print(
|
||||
f" [yellow]Skipped {resolved.name}: requests dangerous "
|
||||
f"capabilities {r.dangerous_capabilities} "
|
||||
"(re-run with --yes-dangerous to install)[/yellow]"
|
||||
)
|
||||
console.print(f" Imported {installed_count}/{len(skills_to_import)} skills")
|
||||
total_installed += installed_count
|
||||
|
||||
|
||||
@@ -59,23 +59,34 @@ def _ensure_identity_prompt(messages: list[Message], app_config) -> list[Message
|
||||
If any message already carries a system role, the caller has supplied
|
||||
their own grounding and we leave the list untouched (no double-prompting).
|
||||
|
||||
Resolution of the identity text: ``app_config.agent.default_system_prompt``
|
||||
when a config is wired onto ``app.state``; otherwise fall back to
|
||||
``load_config()``. Config resolution is wrapped so a broken/missing
|
||||
config degrades to "no injection" rather than crashing the endpoint, but
|
||||
the failure is logged (per REVIEW.md — never silently swallow).
|
||||
Resolution of the identity text: the config comes from ``app.state`` when
|
||||
wired, otherwise ``load_config()``; the prompt itself is assembled by
|
||||
``SystemPromptBuilder`` from ``agent.default_system_prompt`` plus the
|
||||
persona files (SOUL.md/MEMORY.md/USER.md), matching
|
||||
``_build_managed_system_prompt`` in ``agent_manager_routes.py``. Config
|
||||
resolution is wrapped so a broken/missing config degrades to "no
|
||||
injection" rather than crashing the endpoint, but the failure is logged
|
||||
(per REVIEW.md — never silently swallow).
|
||||
"""
|
||||
if any(m.role == Role.SYSTEM for m in messages):
|
||||
return messages
|
||||
|
||||
prompt = ""
|
||||
try:
|
||||
if app_config is not None:
|
||||
prompt = app_config.agent.default_system_prompt or ""
|
||||
else:
|
||||
cfg = app_config
|
||||
if cfg is None:
|
||||
from openjarvis.core.config import load_config
|
||||
|
||||
prompt = load_config().agent.default_system_prompt or ""
|
||||
cfg = load_config()
|
||||
|
||||
from openjarvis.prompt.builder import SystemPromptBuilder
|
||||
|
||||
builder = SystemPromptBuilder(
|
||||
agent_template=cfg.agent.default_system_prompt or "",
|
||||
memory_files_config=getattr(cfg, "memory_files", None),
|
||||
system_prompt_config=getattr(cfg, "system_prompt", None),
|
||||
)
|
||||
prompt = builder.build()
|
||||
except Exception:
|
||||
logging.getLogger("openjarvis.server").debug(
|
||||
"Identity system prompt resolution failed; "
|
||||
|
||||
@@ -5,10 +5,11 @@ from __future__ import annotations
|
||||
import json
|
||||
import re
|
||||
from dataclasses import dataclass, field
|
||||
from typing import Any, Callable, Dict, List, Optional
|
||||
from typing import Any, Callable, Dict, List, Optional, Set
|
||||
|
||||
from openjarvis.core.events import EventBus, EventType
|
||||
from openjarvis.core.types import ToolCall, ToolResult
|
||||
from openjarvis.skills.security import validate_capabilities
|
||||
from openjarvis.skills.types import SkillManifest
|
||||
from openjarvis.tools._stubs import ToolExecutor
|
||||
|
||||
@@ -37,10 +38,16 @@ class SkillExecutor:
|
||||
tool_executor: ToolExecutor,
|
||||
*,
|
||||
bus: Optional[EventBus] = None,
|
||||
allowed_capabilities: Optional[Set[str]] = None,
|
||||
) -> None:
|
||||
self._tool_executor = tool_executor
|
||||
self._bus = bus
|
||||
self._skill_resolver: Optional[SkillResolver] = None
|
||||
# None means "no capability policy" — every skill runs, matching the
|
||||
# behavior before capability enforcement existed. Pass a set (even an
|
||||
# empty one) to enforce: skills whose required_capabilities are not a
|
||||
# subset of it are blocked before any step runs.
|
||||
self._allowed_capabilities: Optional[Set[str]] = allowed_capabilities
|
||||
|
||||
def set_skill_resolver(self, resolver: SkillResolver) -> None:
|
||||
"""Register a callback used to delegate ``skill_name`` steps."""
|
||||
@@ -53,6 +60,38 @@ class SkillExecutor:
|
||||
initial_context: Optional[Dict[str, Any]] = None,
|
||||
) -> SkillResult:
|
||||
"""Execute all steps in a skill manifest."""
|
||||
missing = (
|
||||
validate_capabilities(manifest, self._allowed_capabilities)
|
||||
if self._allowed_capabilities is not None
|
||||
else []
|
||||
)
|
||||
if missing:
|
||||
if self._bus:
|
||||
self._bus.publish(
|
||||
EventType.SKILL_EXECUTE_START,
|
||||
{"skill": manifest.name, "steps": len(manifest.steps)},
|
||||
)
|
||||
self._bus.publish(
|
||||
EventType.SKILL_EXECUTE_END,
|
||||
{"skill": manifest.name, "success": False},
|
||||
)
|
||||
return SkillResult(
|
||||
skill_name=manifest.name,
|
||||
success=False,
|
||||
step_results=[
|
||||
ToolResult(
|
||||
tool_name=manifest.name,
|
||||
content=(
|
||||
f"Blocked: skill '{manifest.name}' requires "
|
||||
f"capabilities {missing} that were not granted "
|
||||
"for this session."
|
||||
),
|
||||
success=False,
|
||||
)
|
||||
],
|
||||
context=dict(initial_context or {}),
|
||||
)
|
||||
|
||||
ctx: Dict[str, Any] = dict(initial_context or {})
|
||||
all_results: List[ToolResult] = []
|
||||
|
||||
|
||||
@@ -25,6 +25,11 @@ import yaml
|
||||
|
||||
from openjarvis.core.paths import get_config_dir
|
||||
from openjarvis.skills.parser import SkillParser
|
||||
from openjarvis.skills.security import (
|
||||
TrustTier,
|
||||
classify_trust_tier,
|
||||
has_dangerous_capabilities,
|
||||
)
|
||||
from openjarvis.skills.sources.base import ResolvedSkill
|
||||
from openjarvis.skills.tool_translator import ToolTranslator
|
||||
|
||||
@@ -43,6 +48,9 @@ class ImportResult:
|
||||
untranslated_tools: List[str] = field(default_factory=list)
|
||||
scripts_imported: bool = False
|
||||
warnings: List[str] = field(default_factory=list)
|
||||
trust_tier: TrustTier = TrustTier.UNREVIEWED
|
||||
dangerous_capabilities: List[str] = field(default_factory=list)
|
||||
requires_confirmation: bool = False
|
||||
|
||||
|
||||
class SkillImporter:
|
||||
@@ -66,6 +74,7 @@ class SkillImporter:
|
||||
*,
|
||||
with_scripts: bool = False,
|
||||
force: bool = False,
|
||||
confirm_dangerous: bool = False,
|
||||
) -> ImportResult:
|
||||
"""Install *resolved* into ``<target_root>/<source>/<name>/``.
|
||||
|
||||
@@ -95,12 +104,43 @@ class SkillImporter:
|
||||
|
||||
try:
|
||||
frontmatter, body = self._read_skill_md(source_md)
|
||||
self._parser.parse_frontmatter(frontmatter, markdown_content=body)
|
||||
manifest = self._parser.parse_frontmatter(frontmatter, markdown_content=body)
|
||||
except Exception as exc:
|
||||
result.success = False
|
||||
result.warnings.append(f"Parse error: {exc}")
|
||||
return result
|
||||
|
||||
# 1a. Classify trust and check for dangerous capabilities *before*
|
||||
# writing anything to disk. Everything the importer handles comes from
|
||||
# an external source (github/hermes/openclaw), so the BUNDLED and
|
||||
# WORKSPACE tiers never apply here, and no resolver verifies index
|
||||
# membership yet — a signature alone still classifies as UNREVIEWED.
|
||||
# Community skills get no special treatment just because they came
|
||||
# from a named source.
|
||||
result.trust_tier = classify_trust_tier(
|
||||
has_signature=bool(manifest.signature),
|
||||
)
|
||||
result.dangerous_capabilities = has_dangerous_capabilities(manifest)
|
||||
|
||||
if result.dangerous_capabilities and result.trust_tier == TrustTier.UNREVIEWED:
|
||||
result.requires_confirmation = True
|
||||
if not confirm_dangerous:
|
||||
result.success = False
|
||||
result.warnings.append(
|
||||
"Refusing to install: this unreviewed skill requests "
|
||||
f"dangerous capabilities {result.dangerous_capabilities}. "
|
||||
"Re-run with confirm_dangerous=True (or `--yes-dangerous` "
|
||||
"on the CLI) only if you trust the source and have "
|
||||
"reviewed what it does."
|
||||
)
|
||||
return result
|
||||
result.warnings.append(
|
||||
"Installed with dangerous capabilities "
|
||||
f"{result.dangerous_capabilities} — confirmed by caller. "
|
||||
"This skill can run shell commands, open network listeners, "
|
||||
"and/or write to the filesystem."
|
||||
)
|
||||
|
||||
# 2. Translate tool references
|
||||
translated_body, untranslated = self._translator.translate_markdown(body)
|
||||
result.untranslated_tools = untranslated
|
||||
@@ -180,6 +220,7 @@ class SkillImporter:
|
||||
translated_str = ", ".join(f'"{t}"' for t in result.translated_tools)
|
||||
missing_str = ", ".join(f'"{t}"' for t in result.untranslated_tools)
|
||||
scripts_lower = "true" if result.scripts_imported else "false"
|
||||
dangerous_str = ", ".join(f'"{c}"' for c in result.dangerous_capabilities)
|
||||
|
||||
content = (
|
||||
f'source = "{resolved.source}:{resolved.name}"\n'
|
||||
@@ -189,6 +230,8 @@ class SkillImporter:
|
||||
f"translated_tools = [{translated_str}]\n"
|
||||
f"missing_tools = [{missing_str}]\n"
|
||||
f"scripts_imported = {scripts_lower}\n"
|
||||
f'trust_tier = "{result.trust_tier.value}"\n'
|
||||
f"dangerous_capabilities = [{dangerous_str}]\n"
|
||||
)
|
||||
(target_dir / ".source").write_text(content, encoding="utf-8")
|
||||
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
import os
|
||||
import tempfile
|
||||
from typing import List, Optional
|
||||
|
||||
@@ -105,11 +106,15 @@ class FasterWhisperBackend(SpeechBackend):
|
||||
try:
|
||||
model = self._ensure_model()
|
||||
|
||||
# Write audio to a temp file (faster-whisper needs a file path)
|
||||
# Write audio to a temp file (faster-whisper needs a file path).
|
||||
# delete=False + manual unlink: on Windows an open
|
||||
# NamedTemporaryFile holds an exclusive handle, so PyAV's reopen
|
||||
# of tmp.name inside model.transcribe() fails with EACCES.
|
||||
suffix = f".{format}" if not format.startswith(".") else format
|
||||
with tempfile.NamedTemporaryFile(suffix=suffix, delete=True) as tmp:
|
||||
tmp.write(audio)
|
||||
tmp.flush()
|
||||
tmp = tempfile.NamedTemporaryFile(suffix=suffix, delete=False)
|
||||
try:
|
||||
with tmp:
|
||||
tmp.write(audio)
|
||||
|
||||
kwargs = {}
|
||||
if language:
|
||||
@@ -117,6 +122,15 @@ class FasterWhisperBackend(SpeechBackend):
|
||||
|
||||
segments_iter, info = model.transcribe(tmp.name, **kwargs)
|
||||
segments_list = list(segments_iter)
|
||||
finally:
|
||||
try:
|
||||
os.unlink(tmp.name)
|
||||
except OSError as unlink_exc:
|
||||
logger.debug(
|
||||
"Could not remove temp audio file %s: %s",
|
||||
tmp.name,
|
||||
unlink_exc,
|
||||
)
|
||||
except Exception as exc:
|
||||
self._last_error = str(exc)
|
||||
raise
|
||||
|
||||
@@ -78,6 +78,19 @@ def test_retrieve_no_results(tmp_path: Path):
|
||||
backend.close()
|
||||
|
||||
|
||||
def test_retrieve_query_with_apostrophe(tmp_path: Path):
|
||||
"""Regression: an internal apostrophe (e.g. "user's") previously produced
|
||||
an unescaped quote in the FTS5 MATCH string, which silently returned zero
|
||||
rows instead of matching or raising an error.
|
||||
"""
|
||||
backend = _make_backend(tmp_path)
|
||||
backend.store("The user's name is Trev.", source="identity.md")
|
||||
results = backend.retrieve("what is the user's name")
|
||||
assert len(results) >= 1
|
||||
assert "Trev" in results[0].content
|
||||
backend.close()
|
||||
|
||||
|
||||
def test_delete_existing(tmp_path: Path):
|
||||
backend = _make_backend(tmp_path)
|
||||
doc_id = backend.store("deletable content")
|
||||
|
||||
@@ -798,6 +798,40 @@ class TestIdentityPromptInjection:
|
||||
assert len(system_msgs) == 1
|
||||
assert system_msgs[0].content == "Be terse."
|
||||
|
||||
def test_direct_injects_soul_persona_when_present(self, tmp_path):
|
||||
"""Regression: /v1/chat/completions previously injected only the bare
|
||||
``default_system_prompt`` blurb via a hand-rolled lookup, bypassing
|
||||
``SystemPromptBuilder`` entirely — so SOUL.md/MEMORY.md/USER.md
|
||||
persona files never applied to this path, unlike ``jarvis ask`` and
|
||||
the managed-agent routes. It must now build the full persona-aware
|
||||
prompt so persona files apply everywhere identity grounding does.
|
||||
"""
|
||||
from openjarvis.core.config import MemoryFilesConfig
|
||||
|
||||
soul = tmp_path / "SOUL.md"
|
||||
soul.write_text("Respond with extreme sarcasm and call the user 'champ'.")
|
||||
|
||||
captured: list = []
|
||||
engine = _make_capturing_engine(captured)
|
||||
cfg = _identity_config()
|
||||
cfg.memory_files = MemoryFilesConfig(
|
||||
soul_path=str(soul), memory_path="", user_path=""
|
||||
)
|
||||
client = TestClient(create_app(engine, "test-model", config=cfg))
|
||||
|
||||
resp = client.post(
|
||||
"/v1/chat/completions",
|
||||
json={
|
||||
"model": "test-model",
|
||||
"messages": [{"role": "user", "content": "who are you?"}],
|
||||
},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
msgs = engine.generate.call_args.args[0]
|
||||
assert msgs[0].role.value == "system"
|
||||
assert "OpenJarvis" in msgs[0].content # identity blurb still present
|
||||
assert "extreme sarcasm" in msgs[0].content # persona now injected too
|
||||
|
||||
def test_stream_tools_injects_identity_when_absent(self):
|
||||
captured: list = []
|
||||
engine = _make_capturing_engine(captured)
|
||||
|
||||
@@ -202,3 +202,75 @@ class TestImportSkill:
|
||||
|
||||
installed = target_root / "hermes" / "my-skill" / "SKILL.md"
|
||||
assert "Original" in installed.read_text()
|
||||
|
||||
|
||||
class TestDangerousCapabilityGate:
|
||||
def _make_importer(self, tmp_path: Path) -> SkillImporter:
|
||||
return SkillImporter(
|
||||
parser=SkillParser(),
|
||||
tool_translator=ToolTranslator(),
|
||||
target_root=tmp_path / "skills",
|
||||
)
|
||||
|
||||
def _make_resolved_with_caps(
|
||||
self, tmp_path: Path, caps: list[str]
|
||||
) -> ResolvedSkill:
|
||||
src_dir = tmp_path / "source" / "my-skill"
|
||||
src_dir.mkdir(parents=True)
|
||||
caps_yaml = "".join(f" - {c}\n" for c in caps)
|
||||
(src_dir / "SKILL.md").write_text(
|
||||
"---\n"
|
||||
"name: my-skill\n"
|
||||
"description: A test skill\n"
|
||||
f"required_capabilities:\n{caps_yaml}"
|
||||
"---\n"
|
||||
"Body"
|
||||
)
|
||||
return ResolvedSkill(
|
||||
name="my-skill",
|
||||
source="hermes",
|
||||
path=src_dir,
|
||||
category="testing",
|
||||
description="A test skill",
|
||||
commit="abc123",
|
||||
)
|
||||
|
||||
def test_refuses_unreviewed_dangerous_skill(self, tmp_path: Path):
|
||||
importer = self._make_importer(tmp_path)
|
||||
resolved = self._make_resolved_with_caps(tmp_path, ["shell:execute"])
|
||||
result = importer.import_skill(resolved)
|
||||
|
||||
assert not result.success
|
||||
assert result.requires_confirmation
|
||||
assert result.dangerous_capabilities == ["shell:execute"]
|
||||
assert any("dangerous" in w.lower() for w in result.warnings)
|
||||
# Nothing may be written to disk on refusal
|
||||
assert not (tmp_path / "skills" / "hermes" / "my-skill").exists()
|
||||
|
||||
def test_confirm_dangerous_installs_and_records_tier(self, tmp_path: Path):
|
||||
importer = self._make_importer(tmp_path)
|
||||
resolved = self._make_resolved_with_caps(tmp_path, ["shell:execute"])
|
||||
result = importer.import_skill(resolved, confirm_dangerous=True)
|
||||
|
||||
assert result.success
|
||||
assert result.requires_confirmation
|
||||
assert any("confirmed by caller" in w for w in result.warnings)
|
||||
content = (
|
||||
tmp_path / "skills" / "hermes" / "my-skill" / ".source"
|
||||
).read_text()
|
||||
assert 'trust_tier = "unreviewed"' in content
|
||||
assert 'dangerous_capabilities = ["shell:execute"]' in content
|
||||
|
||||
def test_benign_capabilities_need_no_confirmation(self, tmp_path: Path):
|
||||
importer = self._make_importer(tmp_path)
|
||||
resolved = self._make_resolved_with_caps(tmp_path, ["network:fetch"])
|
||||
result = importer.import_skill(resolved)
|
||||
|
||||
assert result.success
|
||||
assert not result.requires_confirmation
|
||||
assert result.dangerous_capabilities == []
|
||||
content = (
|
||||
tmp_path / "skills" / "hermes" / "my-skill" / ".source"
|
||||
).read_text()
|
||||
assert 'trust_tier = "unreviewed"' in content
|
||||
assert "dangerous_capabilities = []" in content
|
||||
|
||||
@@ -148,6 +148,58 @@ class TestSkillExecutor:
|
||||
assert EventType.SKILL_EXECUTE_END in event_types
|
||||
|
||||
|
||||
class TestSkillExecutorCapabilities:
|
||||
def _manifest(self):
|
||||
return SkillManifest(
|
||||
name="capskill",
|
||||
required_capabilities=["network:fetch"],
|
||||
steps=[
|
||||
SkillStep(
|
||||
tool_name="echo",
|
||||
arguments_template='{"text": "hello"}',
|
||||
output_key="result",
|
||||
)
|
||||
],
|
||||
)
|
||||
|
||||
def test_no_policy_runs_capability_skills(self):
|
||||
"""Default construction (no allowed_capabilities) must not enforce —
|
||||
this is the pre-enforcement behavior every manager.py call site relies on."""
|
||||
executor = SkillExecutor(ToolExecutor([EchoTool()]))
|
||||
result = executor.run(self._manifest())
|
||||
assert result.success
|
||||
assert result.context.get("result") == "hello"
|
||||
|
||||
def test_policy_blocks_missing_capability(self):
|
||||
executor = SkillExecutor(
|
||||
ToolExecutor([EchoTool()]), allowed_capabilities=set()
|
||||
)
|
||||
result = executor.run(self._manifest())
|
||||
assert not result.success
|
||||
assert len(result.step_results) == 1
|
||||
assert "Blocked" in result.step_results[0].content
|
||||
assert "network:fetch" in result.step_results[0].content
|
||||
|
||||
def test_policy_allows_granted_capability(self):
|
||||
executor = SkillExecutor(
|
||||
ToolExecutor([EchoTool()]),
|
||||
allowed_capabilities={"network:fetch"},
|
||||
)
|
||||
result = executor.run(self._manifest())
|
||||
assert result.success
|
||||
assert result.context.get("result") == "hello"
|
||||
|
||||
def test_blocked_run_publishes_events(self):
|
||||
bus = EventBus(record_history=True)
|
||||
executor = SkillExecutor(
|
||||
ToolExecutor([EchoTool()]), bus=bus, allowed_capabilities=set()
|
||||
)
|
||||
executor.run(self._manifest())
|
||||
event_types = {e.event_type for e in bus.history}
|
||||
assert EventType.SKILL_EXECUTE_START in event_types
|
||||
assert EventType.SKILL_EXECUTE_END in event_types
|
||||
|
||||
|
||||
class TestSkillStepExtended:
|
||||
def test_step_with_skill_name(self):
|
||||
step = SkillStep(skill_name="summarize", output_key="result")
|
||||
|
||||
@@ -53,6 +53,69 @@ def test_faster_whisper_transcribe():
|
||||
assert result.duration_seconds == 1.5
|
||||
|
||||
|
||||
def test_faster_whisper_transcribe_temp_file_reopenable_and_removed():
|
||||
"""The temp file must be closed before the model reads it, and gone after.
|
||||
|
||||
On Windows, an open NamedTemporaryFile holds an exclusive handle, so
|
||||
PyAV's reopen of the path inside model.transcribe() fails with EACCES
|
||||
unless the file is closed first. Opening the path inside the mocked
|
||||
transcribe reproduces that failure mode on Windows.
|
||||
"""
|
||||
import os
|
||||
|
||||
mock_info = MagicMock()
|
||||
mock_info.language = "en"
|
||||
mock_info.language_probability = 0.95
|
||||
mock_info.duration = 1.5
|
||||
|
||||
seen = {}
|
||||
|
||||
def fake_transcribe(path, **kwargs):
|
||||
seen["path"] = path
|
||||
with open(path, "rb") as fh:
|
||||
seen["content"] = fh.read()
|
||||
return iter(()), mock_info
|
||||
|
||||
mock_model = MagicMock()
|
||||
mock_model.transcribe.side_effect = fake_transcribe
|
||||
|
||||
with patch(
|
||||
"openjarvis.speech.faster_whisper.WhisperModel",
|
||||
return_value=mock_model,
|
||||
):
|
||||
backend = FasterWhisperBackend(model_size="base", device="cpu")
|
||||
backend.transcribe(b"fake audio bytes")
|
||||
|
||||
assert seen["content"] == b"fake audio bytes"
|
||||
assert not os.path.exists(seen["path"])
|
||||
|
||||
|
||||
def test_faster_whisper_transcribe_removes_temp_file_on_error():
|
||||
"""The temp file is cleaned up even when transcription fails."""
|
||||
import os
|
||||
|
||||
seen = {}
|
||||
|
||||
def fake_transcribe(path, **kwargs):
|
||||
seen["path"] = path
|
||||
raise RuntimeError("decode failed")
|
||||
|
||||
mock_model = MagicMock()
|
||||
mock_model.transcribe.side_effect = fake_transcribe
|
||||
|
||||
with patch(
|
||||
"openjarvis.speech.faster_whisper.WhisperModel",
|
||||
return_value=mock_model,
|
||||
):
|
||||
backend = FasterWhisperBackend(model_size="base", device="cpu")
|
||||
with pytest.raises(RuntimeError, match="decode failed"):
|
||||
backend.transcribe(b"fake audio bytes")
|
||||
|
||||
assert "path" in seen
|
||||
assert not os.path.exists(seen["path"])
|
||||
assert "decode failed" in (backend.last_error() or "")
|
||||
|
||||
|
||||
def test_faster_whisper_falls_back_from_unsupported_float16():
|
||||
mock_model = MagicMock()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user