fix: reset warm reflection state between calls (#273)

CHANGELOG.md

@@ -6,6 +6,16 @@ This project follows [Semantic Versioning](https://semver.org/).
 
 ## [Unreleased]
 
+## [0.4.0] — 2026-06-21
+
+### Fixed
+
+- **Warm reflection state no longer leaks between files (claude-supertool#273).** The warm container reused one Rector `Application` across every call but reset nothing between them. Rector's `DynamicSourceLocatorProvider` caches its `AggregateSourceLocator` for every non-PHPUnit run, so the second (and every later) file was analysed with a source locator that only knew the *first* file — and Rector emitted `System error: "ClassReflection must be resolved for class X"` on test classes whose hierarchy the stale locator could not resolve. A fresh `rector` CLI process never hit this; the warm daemon did, and the failure was being content-hash-cached and replayed (2100 poisoned entries). `RectorRunner::run()` now resets every service tagged `ResettableInterface` before each warm call — the same flush `AbstractRectorTestCase` performs between fixtures — so the warm daemon matches a cold CLI boot. The expensive bootstrap stays warm; only per-run reflection state is flushed (~no measurable per-call cost). This makes the rector-mcp adapter's `System error:` suppression defense-in-depth rather than load-bearing.
+
+### Added
+
+- **`testWarmReflectionMatchesColdForSequence` regression (env-gated).** Drives a sequence of files through one warm server and asserts no `System error` / stale-reflection failure. Self-contained synthetic classes cannot reproduce #273 (in-process PHPUnit disables the locator cache via `isPHPUnitRun()`, and the trigger needs real framework base classes extended from outside the configured paths), so the test points at a real project via `MCP_RECTOR_WARM_REPRO_DIR` / `MCP_RECTOR_WARM_REPRO_FILES` / `MCP_RECTOR_WARM_REPRO_BIN` and skips otherwise. `tools/repro-273.py` discovers a triggering sequence.
+
 ## [0.3.0] — 2026-05-23
 
 ### Security

src/RectorRunner.php

@@ -14,6 +14,7 @@
 final class RectorRunner
 {
     private ?object $application = null;
+    private ?object $container = null;
     private ?string $appClass = null;
     private ?string $inputClass = null;
     private ?string $outputClass = null;
@@ -36,6 +37,16 @@ public function run(array $argv): array
         $warmBoot = $this->isWarm();
         if (!$warmBoot) {
             $this->boot();
+        } else {
+            // Warm reuse: flush per-run reflection state before analysing the next
+            // file. Rector's DynamicSourceLocatorProvider caches its
+            // AggregateSourceLocator for every non-PHPUnit run, so a second, different
+            // file gets analysed with a locator that only knows the first file and
+            // Rector emits 'System error: ClassReflection must be resolved for class X'
+            // (claude-supertool#273). A fresh CLI process never hits this; the warm
+            // daemon must reset the same services AbstractRectorTestCase resets between
+            // fixtures to behave identically to cold.
+            $this->resetReflectionState();
         }
 
         $inputClass = $this->inputClass;
@@ -118,6 +129,31 @@ private function boot(): void
         $app->setCatchExceptions(true);
 
         $this->application = $app;
+        $this->container = $container;
+    }
+
+    /**
+     * Reset Rector's per-run reflection state between warm calls. Mirrors
+     * AbstractRectorTestCase::setUp(), which resets every service tagged
+     * ResettableInterface so each fixture analyses with a fresh source locator.
+     * The warm daemon reuses one container across files and needs the same flush;
+     * without it the cached AggregateSourceLocator from the previous file poisons
+     * the next one (claude-supertool#273).
+     */
+    private function resetReflectionState(): void
+    {
+        $container = $this->container;
+        if ($container === null || !method_exists($container, 'tagged')) {
+            return;
+        }
+
+        /** @var iterable<object> $resettables */
+        $resettables = $container->tagged(\Rector\Contract\DependencyInjection\ResettableInterface::class);
+        foreach ($resettables as $resettable) {
+            if (method_exists($resettable, 'reset')) {
+                $resettable->reset();
+            }
+        }
     }
 
     /**

tests/Integration/ServerStdioTest.php

@@ -196,6 +196,84 @@ public function testEditedSourceIsReprocessedAcrossCalls(): void
         }
     }
 
+    /**
+     * Regression for claude-supertool#273: the warm container must return the same
+     * result as a cold CLI for every file in a sequence — no spurious
+     * "System error: ClassReflection must be resolved for class X".
+     *
+     * Why this is env-gated and not a self-contained fixture:
+     *   1. The bug only manifests when Rector's DynamicSourceLocatorProvider caches
+     *      its AggregateSourceLocator, which it does ONLY for non-PHPUnit runs
+     *      (StaticPHPUnitEnvironment::isPHPUnitRun() === false). An in-process
+     *      PHPUnit test therefore can never reproduce it — the cache is bypassed.
+     *      The subprocess server is non-PHPUnit, which is why it triggers there.
+     *   2. The failing reflection needs real framework base classes (DVSI's
+     *      SiTestCase / SiModuleTestCase) extended from files outside the configured
+     *      paths. Trivial synthetic classes resolve cleanly and never trip it.
+     *
+     * Point this at a project that reproduces it (e.g. a DVSI checkout):
+     *   MCP_RECTOR_WARM_REPRO_DIR=/path/to/project \
+     *   MCP_RECTOR_WARM_REPRO_FILES=relA.php,relB.php \   # >=2 files, B fails warm pre-fix
+     *   MCP_RECTOR_WARM_REPRO_CONFIG=rector.php \         # optional, default rector.php
+     *   vendor/bin/phpunit --filter testWarmReflectionMatchesColdForSequence
+     */
+    public function testWarmReflectionMatchesColdForSequence(): void
+    {
+        $project = getenv('MCP_RECTOR_WARM_REPRO_DIR');
+        $filesEnv = getenv('MCP_RECTOR_WARM_REPRO_FILES');
+        if ($project === false || $filesEnv === false) {
+            self::markTestSkipped(
+                'Set MCP_RECTOR_WARM_REPRO_DIR + MCP_RECTOR_WARM_REPRO_FILES (>=2 comma-separated '
+                . 'paths relative to the dir) to run the #273 warm-reflection regression. '
+                . 'See tools/repro-273.py to discover a triggering sequence.',
+            );
+        }
+
+        $project = realpath($project) ?: $project;
+        $config = getenv('MCP_RECTOR_WARM_REPRO_CONFIG') ?: 'rector.php';
+        // The bundled bin only boots its own rector + autoload; point this at the
+        // project's installed bin (e.g. DVSI's libs/bin/mcp-rector-warm) when the
+        // config references project-specific custom rules.
+        $bin = getenv('MCP_RECTOR_WARM_REPRO_BIN') ?: self::$bin;
+        $files = array_values(array_filter(array_map('trim', explode(',', $filesEnv))));
+        self::assertGreaterThanOrEqual(2, count($files), 'need at least two files to warm then re-use');
+
+        $proc = $this->spawnServer($project, $config, $bin);
+
+        try {
+            $this->send($proc['stdin'], ['jsonrpc' => '2.0', 'id' => 1, 'method' => 'initialize', 'params' => [
+                'protocolVersion' => '2024-11-05',
+                'capabilities'    => new \stdClass(),
+                'clientInfo'      => ['name' => 'phpunit', 'version' => '1.0.0'],
+            ]]);
+            $this->send($proc['stdin'], ['jsonrpc' => '2.0', 'method' => 'notifications/initialized']);
+
+            $id = 1;
+            foreach ($files as $rel) {
+                $id++;
+                $abs = $project . '/' . ltrim($rel, '/');
+                $this->send($proc['stdin'], $this->processCall($id, $abs));
+                $resp = $this->readResponse($proc['stdout'], $id);
+                $blob = (string) json_encode($resp['result']['structuredContent'] ?? []);
+                self::assertStringNotContainsString(
+                    'System error',
+                    $blob,
+                    "warm container emitted a System error on '{$rel}' (#273)" . $this->stderrTail($proc['stderr']),
+                );
+                self::assertStringNotContainsString(
+                    'must be resolved',
+                    $blob,
+                    "warm container served a stale reflection locator on '{$rel}' (#273)" . $this->stderrTail($proc['stderr']),
+                );
+            }
+        } finally {
+            fclose($proc['stdin']);
+            stream_get_contents($proc['stdout']);
+            fclose($proc['stdout']);
+            proc_close($proc['handle']);
+        }
+    }
+
     /**
      * @param array<string,mixed> $response
      */
@@ -210,14 +288,17 @@ private function changedFiles(array $response): int
     /**
      * @return array{handle: resource, stdin: resource, stdout: resource, stderr: string}
      */
-    private function spawnServer(string $project): array
+    private function spawnServer(string $project, string $config = 'rector.php', ?string $bin = null): array
     {
         // Capture stderr to a file (not /dev/null) so a CI failure has diagnostics.
         $stderr = $project . '/server.stderr';
+        // Absolute config path passes through unchanged; a relative one resolves
+        // against the project (working) dir, matching the production daemon.
+        $configPath = str_starts_with($config, '/') ? $config : $project . '/' . $config;
         $cmd = [
-            self::$bin,
+            $bin ?? self::$bin,
             '--working-dir=' . $project,
-            '--config=' . $project . '/rector.php',
+            '--config=' . $configPath,
         ];
         $proc = proc_open(
             $cmd,

tools/repro-273.py

@@ -0,0 +1,148 @@
+#!/usr/bin/env python3
+"""
+Deterministic-repro harness for claude-supertool #273.
+
+Spawns ONE warm mcp-rector-warm server and feeds it a long sequence of real
+project files over a single connection, hunting for the first `System error:`
+— the warm-only ClassReflection failure (a stale AggregateSourceLocator from an
+earlier file poisoning a later one). On a hit it cold-checks the same file
+(fresh server, single call) to prove the failure is warm-only, not file-local.
+
+The trigger needs real framework base classes (e.g. a test-case base) extended
+from files outside the configured Rector paths; trivial synthetic classes
+resolve cleanly. Point <working_dir> at such a project (e.g. a DVSI checkout)
+and feed its *Test.php files.
+
+Usage: repro-273.py <bin> <working_dir> <config> <max_files> [start]
+File list on stdin (one path per line, relative to working_dir).
+"""
+import json
+import os
+import subprocess
+import sys
+import time
+
+
+def spawn(bin_path, working_dir, config):
+    proc = subprocess.Popen(
+        [bin_path, "--working-dir=" + working_dir, "--config=" + config],
+        cwd=working_dir,
+        stdin=subprocess.PIPE,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.PIPE,
+        bufsize=0,
+    )
+    return proc
+
+
+def send(proc, msg):
+    proc.stdin.write((json.dumps(msg) + "\n").encode())
+    proc.stdin.flush()
+
+
+def read_id(proc, want_id, timeout=120):
+    deadline = time.monotonic() + timeout
+    while time.monotonic() < deadline:
+        line = proc.stdout.readline()
+        if not line:
+            return None
+        line = line.strip()
+        if not line or line[:1] != b"{":
+            continue
+        try:
+            d = json.loads(line)
+        except Exception:
+            continue
+        if d.get("id") == want_id:
+            return d
+    return None
+
+
+def init(proc):
+    send(proc, {"jsonrpc": "2.0", "id": 1, "method": "initialize", "params": {
+        "protocolVersion": "2024-11-05", "capabilities": {},
+        "clientInfo": {"name": "repro", "version": "1.0.0"}}})
+    send(proc, {"jsonrpc": "2.0", "method": "notifications/initialized"})
+    read_id(proc, 1)
+
+
+def call(proc, mid, abs_path):
+    send(proc, {"jsonrpc": "2.0", "id": mid, "method": "tools/call", "params": {
+        "name": "rector_process", "arguments": {"path": abs_path, "dryRun": True}}})
+    return read_id(proc, mid)
+
+
+def structured(resp):
+    if not resp:
+        return {}
+    return (resp.get("result", {}) or {}).get("structuredContent", {}) or {}
+
+
+def has_system_error(sc):
+    blob = json.dumps(sc)
+    return "System error" in blob or "must be resolved" in blob
+
+
+def main():
+    bin_path, working_dir, config, max_files = sys.argv[1:5]
+    start = int(sys.argv[5]) if len(sys.argv) > 5 else 0
+    max_files = int(max_files)
+    files = [l.strip() for l in sys.stdin if l.strip()][start:start + max_files]
+
+    proc = spawn(bin_path, working_dir, config)
+    init(proc)
+    print(f"warm server up, feeding {len(files)} files (start={start})", flush=True)
+
+    hit = None
+    mid = 1
+    for i, rel in enumerate(files):
+        mid += 1
+        ap = os.path.join(working_dir, rel)
+        if not os.path.isfile(ap):
+            continue
+        resp = call(proc, mid, ap)
+        sc = structured(resp)
+        if resp is None:
+            print(f"[{i}] NO RESPONSE on {rel} (server died?)", flush=True)
+            print((proc.stderr.read() or b"")[-2000:].decode(errors="replace"), flush=True)
+            break
+        if has_system_error(sc):
+            hit = (i, rel, sc)
+            print(f"\n*** HIT at #{i} (warm_boot={sc.get('warm_boot')}): {rel}", flush=True)
+            print(json.dumps(sc, indent=2)[:3000], flush=True)
+            break
+        if i % 50 == 0:
+            print(f"[{i}] ok warm={sc.get('warm_boot')} exit={sc.get('exit_code')} {rel}", flush=True)
+    try:
+        proc.stdin.close()
+        proc.terminate()
+    except Exception:
+        pass
+
+    if not hit:
+        print("\nNo System error across the fed sequence.", flush=True)
+        return 0
+
+    # Cold-confirm: fresh server, single call on the culprit file.
+    i, rel, _ = hit
+    ap = os.path.join(working_dir, rel)
+    cold = spawn(bin_path, working_dir, config)
+    init(cold)
+    csc = structured(call(cold, 2, ap))
+    try:
+        cold.stdin.close()
+        cold.terminate()
+    except Exception:
+        pass
+    print(f"\n=== COLD check on culprit {rel} ===", flush=True)
+    print(f"cold warm_boot={csc.get('warm_boot')} exit={csc.get('exit_code')} "
+          f"system_error={has_system_error(csc)}", flush=True)
+    if not has_system_error(csc):
+        print(">>> CONFIRMED WARM-ONLY: fails warm, clean cold. Deterministic repro found.", flush=True)
+    else:
+        print(">>> Fails cold too — not warm-specific; keep looking.", flush=True)
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())