From 66f4663be7fd8fffc327da5c1100add71722f544 Mon Sep 17 00:00:00 2001 From: Florian DAVID Date: Sat, 30 May 2026 09:56:48 +0200 Subject: [PATCH] test: edit-staleness regression + harden against rector version skew MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Proves Rector's warm container has no #265-class staleness: unlike phpunit (which *executes* user classes and can't reload them after an edit), Rector parses source to an AST and re-reads it each run. testEditedSourceIsReprocessedAcrossCalls runs a clean fixture (0 changed), introduces an all-readonly promoted class on disk (1 changed via ReadOnlyClassRector), and asserts the same warm container reports it. Review hardening: - Pin the SINGLE rule (ReadOnlyClassRector) instead of withPhpSets(php82): a broad set's "0 changes" baseline isn't stable across rector versions and could fail for the wrong reason. - Capture server stderr to a file (was /dev/null) and surface it in failure messages. - Assert warm_boot before changed_files; guard the unparseable-output sentinel (-1). - Bump composer.json floor rector/rector ^2.0 -> ^2.4: 2.2.7 is incompatible with current phpstan (OptimizedSingleFileSourceLocator $cache arg string->Cache) and errored on single-file analysis. composer.lock is gitignored, so the floor is what prevents a regenerated lock from pinning the broken version. (DVSI was never affected — resolves its own newer rector.) Ran on rector 2.4.5 / phpstan 2.2.1. Suite 7/7. Co-Authored-By: Max --- composer.json | 2 +- tests/Integration/ServerStdioTest.php | 210 ++++++++++++++++++++++++++ 2 files changed, 211 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 0ed6cb0..6232482 100644 --- a/composer.json +++ b/composer.json @@ -8,7 +8,7 @@ "require": { "php": ">=8.2", "mcp/sdk": "^0.5", - "rector/rector": "^2.0", + "rector/rector": "^2.4", "symfony/finder": "^7.4" }, "autoload": { diff --git a/tests/Integration/ServerStdioTest.php b/tests/Integration/ServerStdioTest.php index 30525cc..9892d35 100644 --- a/tests/Integration/ServerStdioTest.php +++ b/tests/Integration/ServerStdioTest.php @@ -16,6 +16,32 @@ final class ServerStdioTest extends TestCase private static string $fixtureDir; private static string $fixtureFile; + /** @var list temp project dirs created per test, removed in tearDown */ + private array $tmpDirs = []; + + protected function tearDown(): void + { + foreach ($this->tmpDirs as $dir) { + $this->removeDir($dir); + } + $this->tmpDirs = []; + } + + private function removeDir(string $dir): void + { + if (!is_dir($dir)) { + return; + } + $items = new \RecursiveIteratorIterator( + new \RecursiveDirectoryIterator($dir, \FilesystemIterator::SKIP_DOTS), + \RecursiveIteratorIterator::CHILD_FIRST, + ); + foreach ($items as $item) { + $item->isDir() ? rmdir($item->getPathname()) : unlink($item->getPathname()); + } + rmdir($dir); + } + public static function setUpBeforeClass(): void { self::$bin = dirname(__DIR__, 2) . '/bin/mcp-rector-warm'; @@ -109,6 +135,190 @@ public function testWarmBootOnSecondCall(): void self::assertTrue($structured['warm_boot'], 'second tools/call should reuse warm container'); } + /** + * Staleness guard: an edit made BETWEEN two process calls on the same warm + * container must be reflected on the second call. + * + * Unlike phpunit (which *executes* classes and so can't reload them after an + * edit — claude-supertool#265), Rector parses source to an AST and re-reads it + * each run, so a re-processed file should surface the edit. This pins it: a + * file Rector leaves alone (0 changed) → introduce an all-readonly promoted + * class on disk (ReadOnlyClassRector applies → 1 changed) → the same warm + * container must report the change. + */ + public function testEditedSourceIsReprocessedAcrossCalls(): void + { + $project = $this->makeProject(withChange: false); + $file = $project . '/src/RectorProbe.php'; + + $proc = $this->spawnServer($project); + + 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']); + + // Clean file → Rector finds nothing to change. + $this->send($proc['stdin'], $this->processCall(2, $file)); + $first = $this->readResponse($proc['stdout'], 2); + self::assertNotSame(-1, $this->changedFiles($first), 'rector output was unparseable' . $this->stderrTail($proc['stderr'])); + self::assertSame( + 0, + $this->changedFiles($first), + 'clean fixture should yield 0 changed files, got: ' . json_encode($first['result']['structuredContent'] ?? []) . $this->stderrTail($proc['stderr']) + ); + + // Introduce a refactorable pattern on disk; bump mtime past 1s granularity. + file_put_contents($file, $this->probeClass(withChange: true)); + touch($file, time() + 5); + + // Same warm container must re-read the file and report the change. + $this->send($proc['stdin'], $this->processCall(3, $file)); + $second = $this->readResponse($proc['stdout'], 3); + self::assertTrue( + $second['result']['structuredContent']['warm_boot'], + 'second call should reuse the warm container' . $this->stderrTail($proc['stderr']) + ); + self::assertNotSame(-1, $this->changedFiles($second), 'rector output was unparseable' . $this->stderrTail($proc['stderr'])); + self::assertSame( + 1, + $this->changedFiles($second), + 'edited source becomes refactorable — warm container must re-read and report it (stale AST would still report 0)' . $this->stderrTail($proc['stderr']) + ); + } finally { + fclose($proc['stdin']); + stream_get_contents($proc['stdout']); + fclose($proc['stdout']); + proc_close($proc['handle']); + } + } + + /** + * @param array $response + */ + private function changedFiles(array $response): int + { + $output = $response['result']['structuredContent']['output'] ?? ''; + $decoded = is_string($output) && $output !== '' ? json_decode($output, true) : []; + + return (int) ($decoded['totals']['changed_files'] ?? -1); + } + + /** + * @return array{handle: resource, stdin: resource, stdout: resource, stderr: string} + */ + private function spawnServer(string $project): array + { + // Capture stderr to a file (not /dev/null) so a CI failure has diagnostics. + $stderr = $project . '/server.stderr'; + $cmd = [ + self::$bin, + '--working-dir=' . $project, + '--config=' . $project . '/rector.php', + ]; + $proc = proc_open( + $cmd, + [0 => ['pipe', 'r'], 1 => ['pipe', 'w'], 2 => ['file', $stderr, 'w']], + $pipes, + ); + self::assertIsResource($proc); + + return ['handle' => $proc, 'stdin' => $pipes[0], 'stdout' => $pipes[1], 'stderr' => $stderr]; + } + + private function stderrTail(string $path): string + { + $contents = @file_get_contents($path); + + return ($contents === false || $contents === '') ? '' : ' | server stderr: ' . substr($contents, -1500); + } + + /** + * @param resource $stdin + * @param array $message + */ + private function send($stdin, array $message): void + { + fwrite($stdin, json_encode($message) . "\n"); + fflush($stdin); + } + + /** + * Block reading newline-delimited JSON-RPC until the response with $id arrives. + * Rector cold boot can take several seconds — allow a generous read timeout. + * + * @param resource $stdout + * @return array + */ + private function readResponse($stdout, int $id): array + { + stream_set_timeout($stdout, 120); + while (($line = fgets($stdout)) !== false) { + $line = trim($line); + if ($line === '' || $line[0] !== '{') { + continue; + } + $decoded = json_decode($line, true); + if (is_array($decoded) && ($decoded['id'] ?? null) === $id) { + return $decoded; + } + } + + self::fail("no response for id={$id}"); + } + + /** + * @return array + */ + private function processCall(int $id, string $file): array + { + return ['jsonrpc' => '2.0', 'id' => $id, 'method' => 'tools/call', 'params' => [ + 'name' => 'rector_process', + 'arguments' => ['path' => $file, 'dryRun' => true], + ]]; + } + + private function makeProject(bool $withChange): string + { + $dir = sys_get_temp_dir() . '/rector_mcp_regr_' . bin2hex(random_bytes(6)); + mkdir($dir . '/src', 0777, true); + $this->tmpDirs[] = $dir; + + file_put_contents($dir . '/src/RectorProbe.php', $this->probeClass($withChange)); + // Pin the SINGLE rule under test rather than the broad php82 set — a broad + // set's "0 changes" baseline isn't contractually stable across rector + // versions and could rewrite the clean fixture, failing for the wrong reason. + file_put_contents( + $dir . '/rector.php', + "withPaths([__DIR__ . '/src'])->withRules([ReadOnlyClassRector::class]);\n" + ); + + return $dir; + } + + /** + * Clean version has no property to modernise (0 changes). The changed version + * is a final class whose only state is a promoted readonly property, which + * ReadOnlyClassRector rewrites to a `readonly class` (1 change). + */ + private function probeClass(bool $withChange): string + { + if ($withChange) { + return "value;\n }\n}\n"; + } + + return "> $messages * @return list>