Skip to content

Commit cf54766

Browse files
authored
Stop validator cache from freezing transient engine failures + add TTL (#272)
rector-mcp's warm daemon intermittently trips rector's own "System error: ClassReflection must be resolved for class XTest" reflection bug on test classes — warm-process-state dependent, not file dependent (a clean re-run and plain rector CLI pass the same file). That failure was cached keyed on file content and replayed on every later run, frozen duration and all; 2100 entries were poisoned this way across a test suite. - rector-mcp.py: drop rector `System error:` results at the source — an engine glitch is not a code finding. - supertool.py `_validator_result_is_cacheable()`: never cache non-deterministic engine failures (mcp/orchestrator/rector.exit codes, `System error:` messages). Real findings stay cached. - supertool.py `_validator_cache_read()`: TTL on entries (`validator_cache_ttl_hours`, default 24, 0=off). The key only hashes file content, so an entry can outlive an updated adapter/rector.php or a transient failure. Expire on access, self-healing. - Tests for both behaviors; docs + CHANGELOG + example config.
1 parent 9082bb2 commit cf54766

7 files changed

Lines changed: 142 additions & 4 deletions

File tree

.supertool.example.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
"rtk": false,
44
"parallel": 0,
55
"adviseForNewTest": false,
6+
"validator_cache": true,
7+
"validator_cache_ttl_hours": 24,
68
"presets": [
79
"git",
810
"github",

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1212
- **`watch` preset — background event pollers with async wake into Claude Code.** New ops `watch:SOURCE:ID[:only=...]`, `unwatch:SOURCE:ID`, `watches`. Each invocation forks a detached poller that emits state-change events to a UDS socket, a status file, and macOS Notification Center. Source-plugin contract makes new sources ~50 lines. Bundled sources: `github-pr` (PR state, checks, reviews, comments, merge/close, conflicts) and `gitlab-mr` (MR state, pipeline, merge/close, conflicts). Closes [#165](https://github.com/Digital-Process-Tools/claude-supertool/issues/165). See [docs/presets/watch.md](docs/presets/watch.md).
1313
- **`notifiers/claude-channel/` — MCP channel server for async wake.** TypeScript / Bun server that binds the watch UDS socket and pushes events into a running Claude Code session via the Channels feature (research preview, requires Claude Code v2.1.80+). Events arrive as `<channel source="claude-channel" watcher_source="<source>" id="<id>" event="<event>" ...>` tags. UDS file mode 0600 + localhost-only auth model. Launch with `claude --dangerously-load-development-channels server:claude-channel`. See [notifiers/claude-channel/README.md](notifiers/claude-channel/README.md).
1414
- **`glob` brace expansion.** `glob:src/**/*.{json,xml}` now fans out into `*.json` + `*.xml` (shell/fd/ripgrep semantics) and dedupes results, instead of silently returning 0 files. Supports multiple groups (`{a,b}.{x,y}` → 4) and nesting (`{a,b{1,2}}` → 3). Patterns without braces are unchanged. Closes [#161](https://github.com/Digital-Process-Tools/claude-supertool/issues/161).
15+
- **`validator_cache_ttl_hours` config** (default `24`, `0` disables). Validator cache entries expire this long after being written. The cache key only hashes file content, so without a TTL an entry outlives changes it can't see — an updated adapter/`rector.php`, or a transient engine failure. Expiry is on access (stale → miss → re-run → rewrite); no cron needed.
16+
17+
### Fixed
18+
19+
- **Validator cache no longer freezes transient engine failures.** `rector-mcp`'s warm daemon intermittently trips rector's own `System error: "ClassReflection must be resolved for class X"` reflection bug on test classes — warm-process-state dependent, not file dependent (a clean re-run and plain `rector` CLI pass the same file). That failure was cached keyed on file content and replayed on every later run, including its frozen `duration_ms`; 2100 entries were poisoned this way across a test suite. Two-layer fix: (1) `validators/rector-mcp/rector-mcp.py` drops `System error:` results at the source — an engine glitch is not a code finding; (2) the validator cache now excludes non-deterministic engine failures (MCP transport errors, non-zero exits, `System error:` messages) from being written. Real findings (PHPStan types, `rector.refactor`) stay cached.
1520

1621
## [0.14.0] — 2026-05-23
1722

README.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,9 @@ Example: edit a `.json` file with a missing comma → `jsonlint` catches it →
161161

162162
18 validators bundled out of the box (PHP, XML, JSON, YAML, INI, Python syntax + types, Bash, JS, TS, SCSS, Markdown, Ruby, Dockerfile, Go, Terraform, Rust, TOML). Graceful skip when toolchain missing.
163163

164-
Full reference: [docs/validators.md](docs/validators.md) — bundled list, how they hook in, adding your own.
164+
Results are cached per file-content hash and skipped on unchanged files, with a TTL (`validator_cache_ttl_hours`, default 24h) so stale results self-heal. Non-deterministic engine failures are never cached.
165+
166+
Full reference: [docs/validators.md](docs/validators.md) — bundled list, how they hook in, caching, adding your own.
165167

166168
---
167169

docs/validators.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,10 @@ Results are auto-cached at `~/.cache/supertool/validators/`, keyed on `sha256(fi
178178

179179
Disable caching per-call with the `SUPERTOOL_NO_VALIDATOR_CACHE=1` env var, or per-project with `"validator_cache": false` in `.supertool.json`.
180180

181+
**TTL.** Entries expire `validator_cache_ttl_hours` after they're written (default `24`; set `0` to disable expiry). The key only hashes file content, so an entry can outlive changes it can't see — an updated validator adapter, a changed `rector.php`, or a transient engine failure a clean re-run would now pass. Expiry is on access: a stale entry is treated as a miss, re-runs, and is rewritten with a fresh timestamp. No cron needed.
182+
183+
**Engine failures are never cached.** Non-deterministic infrastructure errors — MCP transport errors, non-zero adapter exits, and rector's `System error: ...` reflection failures — are excluded from the cache. Real findings (PHPStan types, `rector.refactor` suggestions) are deterministic and stay cached. This prevents a transient warm-daemon hiccup from freezing a failure that replays on every later run. (See `validators/rector-mcp/rector-mcp.py`, which also drops rector `System error:` results at the source.)
184+
181185
## Manual run
182186

183187
Run validators explicitly against any file without an edit op:

supertool.py

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8582,9 +8582,25 @@ def _validator_cache_read(key: str) -> Optional[Dict[str, Any]]:
85828582
import hashlib
85838583
import hmac
85848584
import json
8585+
import time
85858586
p = _validator_cache_path(key)
85868587
if not p.exists():
85878588
return None
8589+
# TTL: the cache key only hashes file content, so an entry can outlive changes
8590+
# it can't see — an updated validator adapter / rector.php, or a transient engine
8591+
# failure that a clean re-run would now pass. Expire on access (treat as a miss,
8592+
# which re-runs and rewrites with a fresh mtime) so no staleness survives past the
8593+
# window. Config `validator_cache_ttl_hours` (default 24; 0 disables expiry).
8594+
try:
8595+
_ttl_hours = float(_load_config().get("validator_cache_ttl_hours", 24))
8596+
except (TypeError, ValueError):
8597+
_ttl_hours = 24.0
8598+
if _ttl_hours > 0:
8599+
try:
8600+
if time.time() - p.stat().st_mtime > _ttl_hours * 3600:
8601+
return None
8602+
except OSError:
8603+
return None
85888604
try:
85898605
wrapped = json.loads(p.read_text())
85908606
except (OSError, json.JSONDecodeError):
@@ -8614,6 +8630,43 @@ def _validator_cache_write(key: str, data: Dict[str, Any]) -> None:
86148630
pass
86158631

86168632

8633+
# Engine-failure error codes/messages that are NON-deterministic: a clean re-run
8634+
# can flip them. These must never be cached (the cache key is the file's content
8635+
# hash, so a frozen failure replays on every later run until the file changes).
8636+
_NONDETERMINISTIC_ERROR_CODES = {"mcp", "orchestrator", "rector.exit"}
8637+
8638+
8639+
def _validator_result_is_cacheable(data: Dict[str, Any]) -> bool:
8640+
"""True unless the result is a non-deterministic engine/transport failure.
8641+
8642+
WHY THIS EXISTS (2026-06, the 2100-poisoned-entries incident):
8643+
rector-mcp's warm daemon intermittently trips rector's own known bug —
8644+
`System error: "ClassReflection must be resolved for class XTest"` — on test
8645+
classes. It depends on warm-process state, NOT on the file: a cold/clean
8646+
daemon (and plain `rector` CLI) reflect the same file fine. But the failed
8647+
result got cached keyed on file content, so every subsequent run replayed the
8648+
stale error — same message, same frozen duration_ms — long after the live
8649+
daemon recovered. 2100 test files were silently "failing" rector this way.
8650+
8651+
Real findings stay cacheable (phpstan types, `rector.refactor` suggestions are
8652+
deterministic — same input, same output, caching them is the whole point).
8653+
Only engine glitches — MCP transport errors, non-zero exits, and rector's
8654+
"System error:" reflection failures — are filtered out here so a transient
8655+
hiccup can't pin itself into the cache. See validators/rector-mcp/rector-mcp.py
8656+
which also drops "System error:" at the source.
8657+
"""
8658+
if data.get("ok"):
8659+
return True
8660+
for err in data.get("errors") or []:
8661+
if not isinstance(err, dict):
8662+
continue
8663+
if err.get("code") in _NONDETERMINISTIC_ERROR_CODES:
8664+
return False
8665+
if str(err.get("msg") or "").startswith("System error:"):
8666+
return False
8667+
return True
8668+
8669+
86178670
def _validator_run_one(name: str, spec: Dict[str, Any], file: str) -> Optional[Dict[str, Any]]:
86188671
"""Run one validator adapter on `file`. Returns SCHEMA.md-compliant dict.
86198672
@@ -8676,7 +8729,7 @@ def _validator_run_one(name: str, spec: Dict[str, Any], file: str) -> Optional[D
86768729
data["elapsed_s"] = _elapsed
86778730
if target != file:
86788731
data["resolved_to"] = target
8679-
if cache_key:
8732+
if cache_key and _validator_result_is_cacheable(data):
86808733
_validator_cache_write(cache_key, data)
86818734
return data
86828735
except subprocess.TimeoutExpired:

tests/test_security_hardening_150.py

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import os
1414
import subprocess
1515
import sys
16+
import time
1617
from pathlib import Path
1718

1819
import pytest
@@ -163,3 +164,67 @@ def test_different_secret_invalidates_cache(self, cache_dir, tmp_path):
163164
# Force regeneration with a fresh secret.
164165
supertool._validator_cache_secret()
165166
assert supertool._validator_cache_read("k4") is None
167+
168+
169+
class TestValidatorCacheTtl:
170+
"""TTL expiry on the validator cache read path (validator_cache_ttl_hours)."""
171+
172+
@pytest.fixture
173+
def cache_dir(self, tmp_path, monkeypatch):
174+
original_home = Path.home
175+
monkeypatch.setattr(Path, "home", classmethod(lambda cls: tmp_path))
176+
yield tmp_path / ".cache" / "supertool" / "validators"
177+
monkeypatch.setattr(Path, "home", original_home)
178+
179+
def _set_ttl(self, monkeypatch, hours):
180+
monkeypatch.setattr(supertool, "_load_config",
181+
lambda: {"validator_cache_ttl_hours": hours})
182+
183+
def test_fresh_entry_is_a_hit(self, cache_dir, monkeypatch):
184+
self._set_ttl(monkeypatch, 24)
185+
data = {"tool": "fake", "ok": True, "count": 0}
186+
supertool._validator_cache_write("ttl1", data)
187+
assert supertool._validator_cache_read("ttl1") == data
188+
189+
def test_expired_entry_is_a_miss(self, cache_dir, monkeypatch):
190+
self._set_ttl(monkeypatch, 24)
191+
data = {"tool": "fake", "ok": True, "count": 0}
192+
supertool._validator_cache_write("ttl2", data)
193+
# Backdate the file mtime well past the 24h window.
194+
old = time.time() - 100 * 3600
195+
os.utime(supertool._validator_cache_path("ttl2"), (old, old))
196+
assert supertool._validator_cache_read("ttl2") is None
197+
198+
def test_ttl_zero_disables_expiry(self, cache_dir, monkeypatch):
199+
self._set_ttl(monkeypatch, 0)
200+
data = {"tool": "fake", "ok": True, "count": 0}
201+
supertool._validator_cache_write("ttl3", data)
202+
old = time.time() - 100 * 3600
203+
os.utime(supertool._validator_cache_path("ttl3"), (old, old))
204+
assert supertool._validator_cache_read("ttl3") == data
205+
206+
207+
class TestValidatorResultIsCacheable:
208+
"""Non-deterministic engine failures must not be cached (poisoning guard)."""
209+
210+
def test_ok_result_is_cacheable(self):
211+
assert supertool._validator_result_is_cacheable({"ok": True, "errors": []})
212+
213+
def test_real_finding_is_cacheable(self):
214+
data = {"ok": False, "errors": [
215+
{"code": "rector.refactor", "msg": "Would apply SomeRector"}]}
216+
assert supertool._validator_result_is_cacheable(data)
217+
218+
def test_rector_system_error_not_cacheable(self):
219+
data = {"ok": False, "errors": [
220+
{"code": "rector.error",
221+
"msg": 'System error: "ClassReflection must be resolved for class XTest"'}]}
222+
assert not supertool._validator_result_is_cacheable(data)
223+
224+
def test_mcp_transport_error_not_cacheable(self):
225+
data = {"ok": False, "errors": [{"code": "mcp", "msg": "connection refused"}]}
226+
assert not supertool._validator_result_is_cacheable(data)
227+
228+
def test_exit_code_failure_not_cacheable(self):
229+
data = {"ok": False, "errors": [{"code": "rector.exit", "msg": "rector exit 1"}]}
230+
assert not supertool._validator_result_is_cacheable(data)

validators/rector-mcp/rector-mcp.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,10 +191,17 @@ def format_response(file_path: str, mcp_resp: dict, duration_ms: int) -> dict:
191191
"diff": diff,
192192
})
193193
if errors:
194-
base["ok"] = False
195194
for e in errors:
196-
base["count"] += 1
197195
msg = e.get("message", str(e)) if isinstance(e, dict) else str(e)
196+
# Rector's warm in-process engine intermittently fails to resolve a
197+
# BetterReflection for the analyzed class and emits
198+
# "System error: ... must be resolved". It's an engine/reflection
199+
# failure, NOT a code finding — plain rector CLI runs the same file
200+
# clean — so suppress it: don't fail, don't print, don't get cached.
201+
if msg.startswith("System error:"):
202+
continue
203+
base["ok"] = False
204+
base["count"] += 1
198205
# Cap msg — rector can dump diffs that explode validator output.
199206
# Override via env: RECTOR_MCP_MSG_MAX_CHARS.
200207
_cap = int(os.environ.get("RECTOR_MCP_MSG_MAX_CHARS", "2000"))

0 commit comments

Comments
 (0)