diff --git a/CHANGELOG.md b/CHANGELOG.md index 1011ef1..e93d312 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,16 @@ This project follows [Semantic Versioning](https://semver.org/). ## [Unreleased] +## [0.6.0] — 2026-06-21 + +### Fixed + +- **Cross-file reflection staleness: warm no longer silently misses errors cold catches.** The warm worker memoises a class's reflection for its whole life, and re-analysing a file only re-reads *that file's* AST — it never refreshes the reflection of its **dependencies**. So after a dependency was edited (e.g. a method removed), re-analysing a dependent that calls it returned **0 errors**, while a cold `phpstan` run reported the now-undefined call. Unlike the same-file edit case (already handled — phpstan re-reads the analysed file), this is cross-file and was silent. The sibling of the loud rector failure in claude-supertool#273. PHPStan's worker exposes no per-class invalidation, so `PhpstanRunner` now **respawns the worker when a non-target file it has analysed changed since the worker booted** (`workerBootedAt` + an analysed-file set, checked before `ensureWorker()`). The respawn is scoped: the analysis **target is excluded** (phpstan re-reads it anyway), so iterating on a single file never respawns and stays fully warm — the cold boot is paid only when you switch to a different file after editing a dependency. The staleness check stats only the analysed working set, not the whole `--paths` tree. + +### Added + +- `testStaleDependencyIsCaughtWhenDependentReanalysed` — integration regression: edit a dependency on disk, then re-analyse a dependent through the same warm worker and assert the now-undefined call is reported (proven red without the respawn, green with it). + ## [0.5.0] — 2026-05-23 ### Fixed diff --git a/src/PhpstanRunner.php b/src/PhpstanRunner.php index ed73ea4..e01a1e2 100644 --- a/src/PhpstanRunner.php +++ b/src/PhpstanRunner.php @@ -69,6 +69,27 @@ final class PhpstanRunner */ private ?array $excludePaths = null; + /** + * Unix time the live worker (re)booted. The worker reflects every source file + * as of this moment and memoises it for its whole life — re-analysing a file + * never refreshes its reflection for OTHER files (verified). So a dependency + * edited after this timestamp is served stale to its dependents: warm silently + * misses an error cold catches (the cross-file sibling of claude-supertool#273). + * The only cure is a respawn; this is the baseline staleness is measured against. + */ + private ?int $workerBootedAt = null; + + /** + * Set of files the caller has analysed through this worker (path => true). + * Bounds the per-call staleness check to the working set instead of stat-ing + * the whole --paths tree (tens of thousands of files on a real project). A + * file here whose mtime is newer than {@see $workerBootedAt} is stale in the + * worker and forces a respawn before the next dependent is analysed. + * + * @var array + */ + private array $analysedFiles = []; + public function isWarm(): bool { return $this->handshakeDone && $this->isWorkerAlive(); @@ -93,6 +114,18 @@ public function analyse(string $path): array return []; } + // Correctness over warmth when a dependency moved: if any file we've + // analysed OTHER than the target has changed since the worker booted, the + // worker's memoised reflection of it is stale and re-analysing won't + // refresh it — only a fresh worker will. Respawn before analysing so the + // dependent is checked against current reflection. The target itself is + // excluded (phpstan re-reads the analysed file's own AST each call), so + // iterating on a single file never respawns and stays fully warm. Checked + // before ensureWorker() so the teardown + reboot happen in one step. + if ($this->isWarm() && $this->dependencyChangedSinceBoot($path)) { + $this->teardown(); + } + $this->ensureWorker(); // Defence in depth: phpstan's worker may surface source-line context @@ -124,9 +157,36 @@ public function analyse(string $path): array throw new \RuntimeException('Unexpected worker response: ' . trim($line)); } + // Remember we've reflected this file so a later edit to it registers as a + // stale dependency for whatever analyses it next. + $this->analysedFiles[$path] = true; + return $this->extractErrors($decoded['result'] ?? []); } + /** + * True when a file we've analysed — other than $target — has an on-disk mtime + * at or after the worker's boot, i.e. it was edited since the worker reflected + * it. Bounded to the analysed set, so the check costs a handful of stat() calls, + * not a walk of the whole --paths tree. + */ + private function dependencyChangedSinceBoot(string $target): bool + { + if ($this->workerBootedAt === null) { + return false; + } + foreach ($this->analysedFiles as $file => $_) { + if ($file === $target) { + continue; + } + $mtime = @filemtime($file); + if ($mtime !== false && $mtime >= $this->workerBootedAt) { + return true; + } + } + return false; + } + /** * Ensure the worker is running and the handshake is complete. * Respawns transparently if the previous worker died. @@ -140,6 +200,10 @@ public function ensureWorker(): void // Clean up dead state before respawning. $this->teardown(); + // Stamp the boot moment BEFORE the worker reads any source: every file is + // reflected as of now, so a later edit (mtime >= this) is detectably stale. + $this->workerBootedAt = time(); + // 1. Open TCP server on a random port. $this->serverSocket = stream_socket_server('tcp://127.0.0.1:0', $errno, $errstr); if ($this->serverSocket === false) { @@ -345,6 +409,10 @@ private function isWorkerAlive(): bool private function teardown(): void { $this->handshakeDone = false; + // The staleness baseline belongs to the dead worker — a respawn reflects + // every file fresh, so reset it together with the analysed-file set. + $this->workerBootedAt = null; + $this->analysedFiles = []; if (is_resource($this->workerStream)) { @fclose($this->workerStream); diff --git a/tests/Integration/ServerStdioTest.php b/tests/Integration/ServerStdioTest.php index e699f18..448da12 100644 --- a/tests/Integration/ServerStdioTest.php +++ b/tests/Integration/ServerStdioTest.php @@ -243,6 +243,74 @@ public function testEditedSourceIsReanalysedAcrossCalls(): void } } + /** + * Cross-file staleness guard: editing a DEPENDENCY between analyse calls must + * be reflected when a DEPENDENT is analysed next. + * + * The earlier test only edits the file being analysed — phpstan re-reads the + * target's AST, so that path was always fine. The trap is a dependency: the + * worker memoises a class's reflection for its whole life and re-analysing the + * dependency alone does NOT refresh it, so a dependent re-analysed afterwards + * was served the stale reflection and SILENTLY missed an error a cold run + * catches. The runner now respawns the worker when a non-target analysed file + * changed since boot. This pins it: User::go() calls Dep::value(); remove + * Dep::value() on disk; re-analysing User must report the undefined method. + */ + public function testStaleDependencyIsCaughtWhenDependentReanalysed(): void + { + $project = $this->makeDependencyProject(); + $user = $project . '/src/User.php'; + $dep = $project . '/src/Dep.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']); + + // Dependent is clean while Dep::value() exists. + $this->send($proc['stdin'], $this->analyseCall(2, $user)); + self::assertSame( + 0, + $this->readResponse($proc['stdout'], 2)['result']['structuredContent']['exit_code'], + 'User should be clean while Dep::value() exists' . $this->stderrTail($proc['stderr']) + ); + + // Remove the depended-on method on disk (mirrors editing the dependency), + // then validate the edited dependency itself — Dep alone is still fine. + file_put_contents($dep, "send($proc['stdin'], $this->analyseCall(3, $dep)); + $this->readResponse($proc['stdout'], 3); + + // Re-analyse the dependent. The worker reflected Dep with value() at boot; + // without a respawn it would still report 0. It must now report the call + // to the now-undefined Dep::value(). + $this->send($proc['stdin'], $this->analyseCall(4, $user)); + $structured = $this->readResponse($proc['stdout'], 4)['result']['structuredContent']; + self::assertSame( + 1, + $structured['exit_code'], + 'editing the dependency must surface on the dependent (stale reflection would report 0)' . $this->stderrTail($proc['stderr']) + ); + self::assertNotEmpty($structured['errors']); + self::assertStringContainsStringIgnoringCase( + 'value', + $structured['errors'][0]['message'] ?? '', + 'expected the undefined-method error on Dep::value(), got: ' . json_encode($structured['errors']) + ); + } finally { + fclose($proc['stdin']); + stream_get_contents($proc['stdout']); + fclose($proc['stdout']); + proc_close($proc['handle']); + } + } + /** * @return array{handle: resource, stdin: resource, stdout: resource, stderr: string} */ @@ -330,6 +398,29 @@ private function makeProject(bool $withError): string return $dir; } + /** + * Two-file fixture for the cross-file staleness test: Dep declares value(), + * User calls it. Editing Dep on disk must surface when User is re-analysed. + */ + private function makeDependencyProject(): string + { + $dir = sys_get_temp_dir() . '/phpstan_mcp_dep_' . bin2hex(random_bytes(6)); + mkdir($dir . '/src', 0777, true); + $this->tmpDirs[] = $dir; + + file_put_contents( + $dir . '/src/Dep.php', + "value();\n }\n}\n" + ); + file_put_contents($dir . '/phpstan.neon', "parameters:\n level: 5\n paths:\n - src\n"); + + return $dir; + } + private function probeClass(bool $withError): string { $body = $withError ? 'return 42;' : "return 'ok';";