fix(skill_ledger): stop snapshot_paths from sweeping transient dirs into the blob store
snapshot_paths() hashed every file under a skill dir with no exclusion filter, so a stray venv/node_modules/__pycache__/.git under a skill was copied content-addressed into ~/.hermes/.curator_backups/blobs/ on every mutation — and nothing ever prunes that store, so the blobs grew without bound (real install: 47k blobs / 1.3 GB in one day, 98.9% unreferenced). Filter at capture: skip any file whose parent-chain component matches _SNAPSHOT_EXCLUDE_DIRS (the same set proposed for the curator tarball in transient dir is still captured — only files inside those dirs are dropped. The unreferenced-blob GC suggested in the issue is intentionally left out; capture-side filtering stops the growth, and pruning existing garbage is a separate, riskier change.
This commit is contained in:
@@ -8,7 +8,9 @@ The first four tests are adapted from PR #50261 by @yu-xin-c (autonomous
|
||||
skill history), reshaped for the all-actor JSONL ledger design.
|
||||
"""
|
||||
|
||||
import hashlib
|
||||
import json
|
||||
import os
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
@@ -251,6 +253,46 @@ def test_blob_dedupe_same_content_one_blob(ledger_env):
|
||||
assert len(blobs) == 1 # → one blob on disk
|
||||
|
||||
|
||||
def test_snapshot_paths_skips_transient_dirs(ledger_env):
|
||||
"""Transient local artifacts (venv, node_modules, caches, .git) never reach
|
||||
the manifest or the blob store — sweeping them in grows the blob dir
|
||||
unboundedly on real installs (#107539)."""
|
||||
from tools import skill_ledger
|
||||
|
||||
d = ledger_env["skills"] / "has-venv"
|
||||
d.mkdir()
|
||||
for rel, body in (("SKILL.md", "# skill"), ("scripts/run.py", "print('hi')"),
|
||||
("node_modules/pkg/index.js", "junk"), ("venv/bin/python", "junk"),
|
||||
("__pycache__/run.cpython-311.pyc", "junk"), (".git/config", "junk")):
|
||||
p = d / rel
|
||||
p.parent.mkdir(parents=True, exist_ok=True)
|
||||
p.write_text(body, encoding="utf-8")
|
||||
|
||||
manifest = skill_ledger.snapshot_paths(d)
|
||||
rel = {str(Path(i["path"]).relative_to(d)) for i in manifest}
|
||||
assert rel == {"SKILL.md", os.path.join("scripts", "run.py")}
|
||||
|
||||
# None of the transient content was stored as a blob either.
|
||||
junk_sha = hashlib.sha256(b"junk").hexdigest()
|
||||
assert junk_sha not in {p.name for p in skill_ledger.blobs_dir().iterdir()}
|
||||
|
||||
|
||||
def test_snapshot_paths_keeps_file_named_like_transient_dir(ledger_env):
|
||||
"""The filter drops files *inside* transient dirs; a plain file whose own
|
||||
name collides with one (e.g. a ``venv`` bootstrap script) is skill content."""
|
||||
from tools import skill_ledger
|
||||
|
||||
d = ledger_env["skills"] / "edge"
|
||||
d.mkdir()
|
||||
(d / "SKILL.md").write_text("# skill", encoding="utf-8")
|
||||
(d / "venv").write_text("#!/bin/sh\n", encoding="utf-8") # a FILE, not a dir
|
||||
|
||||
manifest = skill_ledger.snapshot_paths(d)
|
||||
rel = {str(Path(i["path"]).relative_to(d)) for i in manifest}
|
||||
assert "venv" in rel
|
||||
assert "SKILL.md" in rel
|
||||
|
||||
|
||||
def test_rollback_fails_closed_when_safety_capture_fails(ledger_env, monkeypatch):
|
||||
"""If the pre-rollback safety ledger entry can't be written, the rollback
|
||||
must abort with nothing changed (consistent with #63366)."""
|
||||
|
||||
@@ -36,6 +36,17 @@ _ARCHIVE_TS_SUFFIX_RE = re.compile(r"^(.+)-\d{14}$")
|
||||
_PACKAGE_RESTORE_ACTIONS = frozenset({"delete", "archive", "purge"})
|
||||
_VALID_ACTORS = {"curator", "agent", "user"}
|
||||
_NON_PACKAGE_TOPS = {".curator_backups", ".hub", ".archive", ".locks"}
|
||||
# Transient/regeneratable local artifacts that must never be swept into a
|
||||
# snapshot, no matter how deep they sit under the skill dir — a stray venv or
|
||||
# node_modules turns a multi-KB ledger capture into gigabytes of blobs (#107539).
|
||||
# agent.curator_backup applies the same set to the whole-tree tarball.
|
||||
TRANSIENT_DIRS = frozenset({
|
||||
".venv", "venv", "env", ".env",
|
||||
"node_modules", "__pycache__",
|
||||
".pytest_cache", ".mypy_cache", ".ruff_cache",
|
||||
".git",
|
||||
})
|
||||
_SNAPSHOT_EXCLUDE_DIRS = TRANSIENT_DIRS
|
||||
|
||||
# Explicit actor override: the CLI sets "user", the curator walk sets "curator".
|
||||
_actor_override: contextvars.ContextVar[Optional[str]] = contextvars.ContextVar(
|
||||
@@ -121,14 +132,18 @@ def read_blob(sha256: str) -> Optional[bytes]:
|
||||
|
||||
def snapshot_paths(root: Optional[Path], *, complete_package: bool = False) -> List[Dict[str, str]]:
|
||||
"""{path, sha256} for every file under *root*, each stored as a blob; [] when root is
|
||||
None/missing. Raises on I/O failure — callers decide whether that is fatal (rollback safety
|
||||
capture) or swallowed (telemetry). ``complete_package`` unions in the newest curator
|
||||
tarball's files (disk hashes win)."""
|
||||
None/missing. Transient local artifacts (venvs, node_modules, caches, .git) are
|
||||
excluded wherever they appear under *root*. Raises on I/O failure — callers decide
|
||||
whether that is fatal (rollback safety capture) or swallowed (telemetry).
|
||||
``complete_package`` unions in the newest curator tarball's files (disk hashes win)."""
|
||||
if root is None:
|
||||
return []
|
||||
root = Path(root) # gone from disk -> []; the complete_package fill may still recover it
|
||||
files = ([root] if root.is_file()
|
||||
else sorted(p for p in root.rglob("*") if p.is_file()) if root.is_dir() else [])
|
||||
else sorted(p for p in root.rglob("*") if p.is_file()
|
||||
and not any(part in _SNAPSHOT_EXCLUDE_DIRS
|
||||
for part in p.relative_to(root).parts[:-1]))
|
||||
if root.is_dir() else [])
|
||||
out = [{"path": str(f), "sha256": _store_blob(f.read_bytes())} for f in files]
|
||||
return fill_snapshot_from_curator_backup(root, out) if complete_package else out
|
||||
|
||||
|
||||
Reference in New Issue
Block a user