fix(rector-mcp): config-driven engine-glitch suppression, catch toMutatingScope#350
Merged
Merged
Conversation
…atingScope Move glitch knowledge out of the generic core (it was string-matching "System error:" in _validator_result_is_cacheable, breaking SCHEMA.md's "core never parses tool-specific output") into the rector-mcp adapter, where signatures are a config prop: validators.rector.engine_glitches in .supertool.json. The adapter reads its own prop straight from the file and drops a matching error at the source, so a non-deterministic engine glitch (e.g. "toMutatingScope() on null") never surfaces as a red or freezes into the file-hash-keyed validator cache. Defaults to the two known signatures when the prop is absent. Part of #345. Co-Authored-By: Max <noreply>
…f, edge tests - Memoize engine_glitch_signatures() (lru_cache): the adapter is a fresh process per validator call, so config can't change underneath a run — avoids re-reading/parsing .supertool.json once per rector error in the loop. - Comment accuracy: the signature *values* are config not env; the file is located via WORKING_DIR (a single read, no parent-dir walk unlike daemon.py / the core loader), which holds because the daemon cmd runs at project root. - CHANGELOG: state the tradeoff honestly — removing the core message-backstop means a brand-new uncatalogued glitch can still reach the cache until added to config (bounded to unknown signatures, one-line config fix). - Tests: malformed .supertool.json and a non-list engine_glitches prop both fall back to built-in defaults. 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
The warm
rectordaemon intermittently trips a PHP fatal inside rector's own engine —Call to a member function toMutatingScope() on null— a non-deterministic glitch a coldrectorCLI does not reproduce. Like the JuneSystem error: ClassReflectioncase it surfaced as a false red and — because the validator cache keys on file content — froze into the cache and replayed on every later run until the file changed (the 2100-poisoned-entries failure mode, new signature).The old guard string-matched
System error:inside_validator_result_is_cacheablein the core — which violatesSCHEMA.md's contract: "Validator core never parses tool-specific output." This PR removes that and pushes glitch knowledge down to the adapter, config-driven.Changes
validators/rector-mcp/rector-mcp.py— reads its glitch signatures from.supertool.json→validators.rector.engine_glitches(the adapter loads the file directly, the same onedaemon.pyalready reads from cwd), drops a matching error at the source. Built-in defaults["System error:", "toMutatingScope() on null"]when the prop is absent.supertool.py— deleted theSystem error:string-match in_validator_result_is_cacheable. The core cache filter is now fully generic: it keys only off non-deterministic error codes, never message text.validators/SCHEMA.md— documents the newengine_glitchesvalidator-entry field.docs/validators.md— updated the "never cached" section.tests/test_rector_mcp_engine_glitches.py(prop override, empty-disables, defaults, substring match, source-drop); updated one security-hardening test to the new generic-core contract.A new glitch signature is now a one-line config edit, not a code change.
Test plan
python3 -m pytest -q→ 3489 passed, 34 skipped (8 new).Notes
Only addresses the rector half of #345. The phpunit-mcp transient-DB-red half is a different mechanism (verify-on-red, not signature-matching) and lands separately — hence "Part of", not "Closes".
🤖 Generated by Max — the core forgot rector exists, which is exactly how it should be.