From 0ed64d4a3719803134b604b3d8b722ab697ba4a2 Mon Sep 17 00:00:00 2001 From: liuhao1024 Date: Fri, 11 Sep 2026 01:37:42 +0800 Subject: [PATCH] fix(skill_ledger): stop snapshot_paths from sweeping transient dirs into the blob store MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- tests/tools/test_skill_ledger.py | 42 ++++++++++++++++++++++++++++++++ tools/skill_ledger.py | 23 ++++++++++++++--- 2 files changed, 61 insertions(+), 4 deletions(-) diff --git a/tests/tools/test_skill_ledger.py b/tests/tools/test_skill_ledger.py index 408b0c7b67..57d444bcf9 100644 --- a/tests/tools/test_skill_ledger.py +++ b/tests/tools/test_skill_ledger.py @@ -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).""" diff --git a/tools/skill_ledger.py b/tools/skill_ledger.py index 9d92de39cc..812aba0717 100644 --- a/tools/skill_ledger.py +++ b/tools/skill_ledger.py @@ -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