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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion docs/knowledge-operations.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ returned item payloads, not the omission/control envelope. Each omission carries
`reason` code (`unknown_identity`, `ambiguous_identity`, `invalid_identity`,
`duplicate_identity`, `unavailable_revision`, `invalid`, `filters`, `query`,
`output_limit`) and, for errors, a `detail` message. No-match is explicit.
Per-item omission applies after identity discovery: a checkout skill or case
record that cannot be parsed or validated during discovery (for example invalid
frontmatter or JSON, a name/directory mismatch, conflicting metadata, a missing
or duplicate case ID) fails the whole reader call explicitly, because
identities cannot be trusted.
Deterministic identity order is not a learned relevance ranking.

## Identity and resources
Expand All @@ -46,8 +51,15 @@ or installation rejects an old revision; it never substitutes new bytes.
This reader does not itself retain history or claim standalone exports.

- Local skills include their complete owner folder (except incidental
`__pycache__`, `*.pyc` and `.DS_Store`) and declared cross-skill references. Other links remain navigation in the required full checkout.
`__pycache__`, `*.pyc` and `.DS_Store`) and declared cross-skill references,
whether declared in the sidecar or in legacy frontmatter. Other links remain navigation in the required full checkout.
- Local structured cases keep their own identity, observation and source limits.
A case bundle holds its record, content and any `required_resources` declared
relative to the record; missing, unsafe or escaping resources fail. Case
`references`/`dependencies` declarations are rejected rather than ignored;
external evidence stays a source pointer.
- Every declared `skill_documents` edition must be present in the retained
library; a missing target fails the read instead of leaving a dangling link.
- Existing catalog records retain IDs/tiers/state. Bilingual records include the
entire imported library as one conservative dependency closure: languages,
manifests, licenses, helpers and transitive references are preserved. This is
Expand Down
45 changes: 33 additions & 12 deletions src/omnirsi/knowledge_operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -271,20 +271,37 @@ def _checkout(self, identity: str) -> Bundle:
if identity in cases:
path = cases[identity]
record = json.loads(path.read_text(encoding="utf-8"))
for field in ("references", "dependencies"):
if field in record:
raise ValueError(f"Case {field} are not supported; declare required_resources")
declared = record.get("required_resources", [])
if not isinstance(declared, list) or any(not isinstance(p, str) for p in declared):
raise ValueError("Case required_resources must be a string list")
if len(set(declared)) != len(declared):
raise ValueError("Case required_resources must not repeat a path")
relative = safe_path(record["content"])
content_path = path.parent / relative
if content_path.is_symlink() or not content_path.resolve().is_relative_to(
self.skill_root
):
raise ValueError("Case content escapes the skills root")
required = [relative, "case.json", *(p for p in declared if p != relative)]
content = {"case.json": path.read_bytes()}
for resource in [relative, *required[2:]]:
safe_path(resource)
if resource == "case.json":
raise ValueError("Case resource collides with its case.json record")
resource_path = path.parent / resource
if resource_path.is_symlink() or not resource_path.resolve().is_relative_to(
self.skill_root
):
raise ValueError(f"Case resource escapes the skills root: {resource}")
if not resource_path.is_file():
raise ValueError(f"Missing case resource: {resource}")
content[resource] = resource_path.read_bytes()
return Bundle(
{
**record,
"entrypoint": relative,
"sources": _sources(record["sources"]),
"required_resources": [relative, "case.json"],
"required_resources": required,
},
{relative: content_path.read_bytes(), "case.json": path.read_bytes()},
content,
{},
)
return self._skill(identity)
Expand All @@ -301,6 +318,12 @@ def _knowledge(self, identity: str) -> Bundle:
library, digest = self._skill_library()
content.update(library)
dependencies["data/skill_library"] = digest
documents = record["skill_documents"]
if not isinstance(documents, dict):
raise ValueError(f"skill_documents must map editions to paths: {identity}")
for target in documents.values():
if not isinstance(target, str) or safe_path(target) not in library:
raise ValueError(f"Missing declared skill document: {target}")
return Bundle(
{
"id": identity,
Expand Down Expand Up @@ -331,14 +354,12 @@ def _knowledge(self, identity: str) -> Bundle:
)

def _skill(self, identity: str) -> Bundle:
from .skill_catalog import get_skill, skill_reference
from .skill_catalog import get_skill, skill_metadata, skill_reference

root = self.skill_root
record = get_skill(root, identity)
directory = root / identity
content = _tree(directory, identity)
sidecar = directory / "metadata.json"
metadata = json.loads(sidecar.read_text(encoding="utf-8")) if sidecar.exists() else {}
content = _tree(root / identity, identity)
metadata = skill_metadata(root, identity)
dependencies = {}
for reference in metadata.get("references", []):
path = skill_reference(root, identity, reference)
Expand Down
2 changes: 2 additions & 0 deletions src/omnirsi/knowledge_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ def create(self, bundle: Bundle) -> str:
def update(self, bundle: Bundle, *, expected: str, reason: str, evidence: list[dict]) -> str:
if not expected or not reason.strip():
raise ValueError("Update requires expected old revision and reason")
if not isinstance(evidence, list):
raise ValueError("Update evidence must be a list; an empty list means none supplied")
return self._publish(bundle, expected=expected, reason=reason, evidence=evidence)

def _lineage(self, metadata: dict) -> None:
Expand Down
15 changes: 12 additions & 3 deletions src/omnirsi/skill_catalog.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ def _references(metadata: dict, name: str) -> list[str]:
return references


def _metadata(directory: Path) -> dict:
def _effective(directory: Path) -> dict:
# Read bytes line by line; parse only the frontmatter, never decode the body.
with _entry(directory, "SKILL.md").open("rb") as stream:
record = _frontmatter(stream)
Expand Down Expand Up @@ -95,9 +95,19 @@ def _metadata(directory: Path) -> dict:
or any(not isinstance(tag, str) for tag in record["tags"])
):
raise ValueError(f"Skill tags must be a string list: {directory.name}")
return record


def _metadata(directory: Path) -> dict:
record = _effective(directory)
return {key: record[key] for key in ("name", "description", "status", "tags") if key in record}


def skill_metadata(root: Path, name: str) -> dict:
"""Effective legacy frontmatter plus sidecar fields, after conflict checks."""
return _effective(_directory(root, name))


def skill_records(root: Path) -> list[dict]:
"""Project metadata only; this projection is not an authoritative matrix."""
if not root.is_dir():
Expand Down Expand Up @@ -143,8 +153,7 @@ def skill_resource(root: Path, name: str, resource: str) -> Path:
def skill_reference(root: Path, name: str, reference: str) -> Path:
"""Resolve a declared checkout reference without escaping the skills root."""
directory = _directory(root, name)
metadata = _sidecar(directory)
if reference not in _references(metadata or {}, directory.name):
if reference not in _references(_effective(directory), directory.name):
raise ValueError(f"Undeclared skill reference: {reference}")
if "\\" in reference or Path(reference).is_absolute():
raise ValueError("Reference must use a relative POSIX path")
Expand Down
80 changes: 80 additions & 0 deletions tests/test_knowledge_operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,86 @@ def test_qualified_identity_rules(tmp_path):
assert package_filter["omissions"] == [{"id": "case.local-only", "reason": "filters"}]


def test_legacy_frontmatter_reference_is_a_dependency(tmp_path):
(tmp_path / "other").mkdir()
(tmp_path / "other" / "SKILL.md").write_text("---\nname: other\ndescription: t\n---\nB")
target = tmp_path / "other" / "notes.md"
target.write_text("first")
(tmp_path / "selected").mkdir()
(tmp_path / "selected" / "SKILL.md").write_text(
"---\nname: selected\ndescription: t\nreferences: [../other/notes.md]\n"
"applicability: {backend: any}\n---\nBody"
)
reader = KnowledgeReader(skill_root=tmp_path)
original = reader.read("selected")
assert original.metadata["applicability"] == {"backend": "any"}
assert original.content["other/notes.md"] == b"first"
assert "other/notes.md" in original.dependencies
target.write_text("second")
assert reader.read("selected").revision != original.revision
target.unlink()
with pytest.raises(ValueError, match="Missing skill reference"):
reader.read("selected")


def _declare(root: Path, **fields) -> None:
path = root / "owner" / "cases" / "note.json"
record = json.loads(path.read_text(encoding="utf-8"))
path.write_text(json.dumps({**record, **fields}), encoding="utf-8")


def test_case_required_resources_are_retained_and_checked(tmp_path):
_case(tmp_path, "case.measured")
data = tmp_path / "owner" / "cases" / "measurements.csv"
data.write_text("a,1", encoding="utf-8")
plain = KnowledgeReader(skill_root=tmp_path).read("case.measured")
assert plain.metadata["required_resources"] == ["note.md", "case.json"]
_declare(tmp_path, required_resources=["measurements.csv"])
reader = KnowledgeReader(skill_root=tmp_path)
original = reader.read("case.measured")
assert original.content["measurements.csv"] == b"a,1"
data.write_text("a,2", encoding="utf-8")
assert reader.read("case.measured").revision != original.revision
data.unlink()
with pytest.raises(ValueError, match="Missing case resource"):
reader.read("case.measured")
data.write_text("a,1", encoding="utf-8")
for fields, message in [
({"required_resources": ["../SKILL.md"]}, "Unsafe"),
({"required_resources": "measurements.csv"}, "string list"),
({"required_resources": ["measurements.csv"] * 2}, "repeat"),
({"required_resources": ["case.json"]}, "collides"),
({"required_resources": [], "references": ["x.md"]}, "not supported"),
({"required_resources": [], "dependencies": {}}, "not supported"),
]:
path = tmp_path / "owner" / "cases" / "note.json"
record = json.loads(path.read_text(encoding="utf-8"))
record.pop("references", None)
record.pop("dependencies", None)
path.write_text(json.dumps({**record, **fields}), encoding="utf-8")
with pytest.raises(ValueError, match=message):
KnowledgeReader(skill_root=tmp_path).read("case.measured")


def test_missing_declared_skill_document_is_explicit():
reader = KnowledgeReader()
identity, record = next((i, r) for i, r in reader.records().items() if r.get("skill_documents"))
library, digest = reader._skill_library()
target = next(iter(record["skill_documents"].values()))
reader._library = ({k: v for k, v in library.items() if k != target}, digest)
with pytest.raises(ValueError, match="Missing declared skill document"):
reader.read(identity)
result = reader.collect(ids=[identity])
assert result["omissions"][0]["reason"] == "invalid"


def test_malformed_checkout_discovery_fails_explicitly(tmp_path):
_case(tmp_path, "case.broken")
(tmp_path / "owner" / "cases" / "note.json").write_text("{", encoding="utf-8")
with pytest.raises(ValueError):
KnowledgeReader(skill_root=tmp_path).collect(ids=["owner"])


@pytest.mark.parametrize("path", ["../outside", "/absolute", "a\\b", "C:/outside", "a/../b"])
def test_unsafe_resource_paths_fail(path):
with pytest.raises(ValueError, match="Unsafe"):
Expand Down
2 changes: 2 additions & 0 deletions tests/test_knowledge_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ def test_duplicate_stale_update_and_immutable_resources(tmp_path):
with pytest.raises(RevisionConflict, match="Duplicate"):
store.create(original)
successor = changed(original, b"\nScoped interpretation amendment.\n")
with pytest.raises(ValueError, match="evidence must be a list"):
store.update(successor, expected=revision, reason="clarify", evidence="log.txt")
next_revision = store.update(successor, expected=revision, reason="clarify", evidence=[])
assert next_revision != revision
with pytest.raises(RevisionConflict, match="Stale"):
Expand Down
Loading