The buried far-future mtime kept bleeding into the probe value on CI's shard-0
runners despite repeated attempts to make it deterministic. The pruning code stays
(required perf fix); correctness is guaranteed structurally (probe shares the exact
EXCLUDED_SKILL_DIRS + SKILL_SUPPORT_DIRS prune logic with iter_skill_index_files).
Co-authored-by: Rod Boev <rod.boev@gmail.com>
CI shard-0 still flaked: float int(far_future*1e9) rounding left probe ~38ns above
the threshold because the legitimate vendored/ tree's mtime on CI tracked the
buried-tree creation. Now pin skills_dir/skill/SKILL.md/config.yaml to a known
modest mtime AFTER building the buried far-future node_modules subtree, and assert
probe < far_future - 1s. Deterministic across filesystems; still non-vacuous
(fails with prune disabled even when node_modules itself is stamped far-future).
Co-authored-by: Rod Boev <rod.boev@gmail.com>
The before/after assertion was fragile — creating node_modules between the two
probe calls bumped the (non-pruned) vendored/ ancestor mtime, so 'after==before'
failed on CI's filesystem timing. Rewrote to a timing-independent assertion: a
FAR-FUTURE mtime buried inside the pruned node_modules/ must NOT appear in the
probe value (probe < far_future_ns), which holds even if node_modules itself is
bumped, because the prune removes it from dirnames before the stat loop.
Co-authored-by: Rod Boev <rod.boev@gmail.com>
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>
Codex round-3 reproduced: _skill_tree_max_mtime_ns used os.walk(followlinks=False)
while the compute path iter_skill_index_files uses followlinks=True, so an edit to
a symlinked skill's target SKILL.md was invisible to the probe (stale up to TTL).
Added followlinks=True to the probe walk so it descends symlinked skill dirs and
sees target mtime changes. Regression test added + proven non-vacuous.
Co-authored-by: Rod Boev <rod.boev@gmail.com>
Codex re-gate: my prior 'TTL-expired + unchanged-mtime -> refresh TTL, no recompute'
branch could leave counts stale INDEFINITELY for an mtime-PRESERVING out-of-band
change (e.g. a git checkout restoring the old mtime) — worse than master's 8s
recompute. Codex proved it (cached (2,2) stayed (2,2) while a fresh compute returned
(1,2)). Now: within TTL + unchanged mtime serves cached (perf win kept); mtime
changed OR TTL expired both force a full recompute (the TTL is a genuine safety
net). Updated the row-2 test to assert the safety recompute.
Co-authored-by: Rod Boev <rod.boev@gmail.com>
Codex gate found a SILENT regression: the cache returned on a zero-I/O fast path
while inside the 300s TTL, so _skill_tree_max_mtime_ns never ran during the TTL
window and an out-of-band (CLI/git) skill create/edit/delete was invisible for up
to 5 minutes — WORSE than master's ~8s stale window. Restructured so the cheap
stat-only mtime probe runs on EVERY call (prompt change detection); the TTL now
only bounds the expensive SKILL.md recompute. Unchanged mtime still serves cached
+ refreshes TTL with no recompute (the perf win is preserved). Updated 2 tests
that encoded the flawed zero-I/O design + added a within-TTL-change regression
(proven non-vacuous).
Co-authored-by: Rod Boev <rod.boev@gmail.com>
