fix(#4783): prune dependency trees in mtime probe to match compute path (gate finding 4)

Codex round-4 found my followlinks=True change introduced a perf regression: the
probe os.walk lacked the .git/.venv/node_modules/site-packages + skill-support-dir
pruning that iter_skill_index_files applies, so a skill vendoring a dependency
tree made this EVERY-CALL probe walk thousands of irrelevant files — defeating the
cache's whole purpose. The probe now prunes dirnames[:] with the same
EXCLUDED_SKILL_DIRS + SKILL_SUPPORT_DIRS sets as the compute path. Regression test
(change deep inside node_modules/ must not move the probe) added + non-vacuous.

Co-authored-by: Rod Boev <rod.boev@gmail.com>
This commit is contained in:
nesquena-hermesandRod Boev committed 2026-06-24 21:27:22 +00:00
1 parent e4e3b46830
commit 3fbc7d71d3
2 files changed
+44

No files matched your search

+15
View File
@@ -1595,6 +1595,11 @@ def _skill_tree_max_mtime_ns(skills_dir: Path, config_path: Path) -> int:
pass
if not skills_dir.is_dir():
return max_ns
try:
from agent.skill_utils import EXCLUDED_SKILL_DIRS, SKILL_SUPPORT_DIRS
except Exception:
EXCLUDED_SKILL_DIRS = frozenset()
SKILL_SUPPORT_DIRS = frozenset()
try:
# Directory mtimes catch nested out-of-band deletes that leave file mtimes unchanged.
# followlinks=True mirrors agent.skill_utils.iter_skill_index_files (the compute
@@ -1602,6 +1607,16 @@ def _skill_tree_max_mtime_ns(skills_dir: Path, config_path: Path) -> int:
# SKILL.md change the probe value — otherwise such edits would stay stale up to the TTL.
for root, dirnames, filenames in os.walk(skills_dir, followlinks=True):
root_path = Path(root)
# Prune the SAME trees iter_skill_index_files prunes (.git/.venv/
# node_modules/site-packages + skill support dirs), so a skill that
# vendors a dependency tree doesn't make this every-call probe walk
# thousands of irrelevant files and defeat the cache's perf goal.
has_skill_md = "SKILL.md" in filenames
dirnames[:] = [
d for d in dirnames
if d not in EXCLUDED_SKILL_DIRS
and not (has_skill_md and d in SKILL_SUPPORT_DIRS)
]
try:
max_ns = max(max_ns, root_path.stat().st_mtime_ns)
except OSError:
@@ -343,6 +343,35 @@ class TestSymlinkedSkillProbeFollowsLinks:
"probe must follow the symlinked skill dir and see the target SKILL.md mtime change"
)
def test_probe_prunes_dependency_trees_like_compute_path(self, profiles_mod):
"""The probe must NOT descend into pruned trees (.venv/node_modules/
site-packages/support dirs) — matching iter_skill_index_files — so a
skill that vendors a dependency tree doesn't make this every-call probe
walk thousands of irrelevant files. A mtime change DEEP inside a pruned
dir must NOT move the probe value (it's not part of the skill index)."""
mod, profile_dir = profiles_mod
skills_dir = profile_dir / "skills"
skill = skills_dir / "vendored"
skill.mkdir(parents=True)
(skill / "SKILL.md").write_text("---\nname: vendored\n---\n# vendored\n", encoding="utf-8")
# A vendored dependency tree that the compute path prunes.
buried = skill / "node_modules" / "pkg"
buried.mkdir(parents=True)
buried_file = buried / "index.js"
buried_file.write_text("// dep\n", encoding="utf-8")
before = mod._skill_tree_max_mtime_ns(skills_dir, profile_dir / "config.yaml")
import os as _os
future = time.time() + 1000
_os.utime(buried_file, (future, future))
_os.utime(buried, (future, future))
after = mod._skill_tree_max_mtime_ns(skills_dir, profile_dir / "config.yaml")
assert after == before, (
"probe must prune node_modules/ (not descend) — a change inside a pruned "
f"dependency tree must not move the probe value (before={before}, after={after})"
)
class TestReturnSignature:
"""Proof matrix row 6: return signature is tuple[int, int]."""