Stop validator cache from freezing transient engine failures + add TTL#272
Merged
Conversation
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. Co-Authored-By: Max <noreply>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
validate:...Test.php:rectorwas persistently reportingSystem error: "ClassReflection must be resolved for class XTest"— on Kevin and locally — always with an identical frozen duration.Root cause (proven):
mcp-rector-warm's long-lived daemon intermittently trips rector's own known reflection bug on test classes. It's warm-process-state dependent, not file dependent — a clean re-run and plainrectorCLI reflect the same file fine. But the failed result got written to the validator cache (keyed on file-content hash, with itsduration_ms), so every later run on the unchanged file replayed the stale failure. 2100 poisoned entries were found across a test suite. The identical frozen duration was the tell — live warm runs are ~200–500ms, never the cached number.Changes
Fix (two layers):
validators/rector-mcp/rector-mcp.py— drop rectorSystem error:results at the source. An engine/reflection glitch is not a code finding.supertool.py_validator_result_is_cacheable()— never cache non-deterministic engine failures (codesmcp/orchestrator/rector.exit, orSystem error:messages). Real findings (PHPStan types,rector.refactor) stay cached.Defense in depth (TTL):
supertool.py_validator_cache_read()— entries expirevalidator_cache_ttl_hoursafter write (default24;0disables). The cache key only hashes file content, so an entry can outlive changes it can't see — an updated adapter, a changedrector.php, or a transient failure. Expiry is on access (stale → miss → re-run → rewrite); no cron. Applies to all validators — the read path is the single chokepoint.Tests + docs:
tests/test_security_hardening_150.py—TestValidatorCacheTtl(fresh hit / expired miss / ttl=0 disables) +TestValidatorResultIsCacheable(ok & real findings cacheable; rector System error, mcp, exit not).docs/validators.md,README.md,.supertool.example.json,CHANGELOG.md.Test plan
validate:...Test.php:rectorreturnsokwith cache on; plainrectorCLI confirmed clean on the same file; purged the 2100 stale entries locally.🤖 Generated by Max — turns out the bug was the cache remembering a lie.