Skip to content

fix: respawn worker on stale dependency (cross-file false negatives) #4

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
fdaviddpt merged 1 commit into from
Jun 21, 2026
+169 −0
Merged

fix: respawn worker on stale dependency (cross-file false negatives) #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
68 changes: 68 additions & 0 deletions src/PhpstanRunner.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<string,bool>
*/
private array $analysedFiles = [];

public function isWarm(): bool
{
return $this->handshakeDone && $this->isWorkerAlive();
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down
91 changes: 91 additions & 0 deletions tests/Integration/ServerStdioTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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, "<?php\n\ndeclare(strict_types=1);\n\nfinal class Dep\n{\n public function other(): int\n {\n return 2;\n }\n}\n");
touch($dep, time() + 5);
$this->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}
*/
Expand Down Expand Up @@ -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',
"<?php\n\ndeclare(strict_types=1);\n\nfinal class Dep\n{\n public function value(): int\n {\n return 1;\n }\n}\n"
);
file_put_contents(
$dir . '/src/User.php',
"<?php\n\ndeclare(strict_types=1);\n\nfinal class User\n{\n public function go(): int\n {\n return (new Dep())->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';";
Expand Down
Loading