fix: keep the forked child's output out of the MCP transport

CHANGELOG.md

@@ -6,6 +6,17 @@ This project follows [Semantic Versioning](https://semver.org/).
 
 ## [Unreleased]
 
+### Fixed
+
+- **Child output no longer reaches the MCP transport (#3).** The forked test child inherits stdout, which for this server *is* the protocol stream. `runInProcess()` buffered PHPUnit's own output, but a test that fatals or calls `exit()` never reaches the matching `ob_get_clean()` — PHP flushes every active buffer during shutdown, straight to fd 1. Whatever user code had printed then landed in front of the next JSON-RPC frame and, being unterminated, glued itself to it (`</html>{"jsonrpc":...}`). Clients that frame on newlines could not parse that line and blocked until their own timeout on a result that had already arrived — measured at five minutes per call against a host application that renders an HTML error page on fatals. The child now seals stdout immediately after the fork with a never-ended output buffer whose callback returns nothing, so output from any phase, shutdown included, writes zero bytes. Results are unaffected: the child ships them over its socket pair.
+
+- **A dying child now reports what it printed, instead of losing it (#3).** Sealing stdout alone would have traded a loud failure for a silent one — a debug `echo`, or the error page a framework rendered on its way down, would simply vanish. The seal installs a shutdown hook that ships the buffered output over the result socket instead, so the crash payload carries it under the existing `echo` key, alongside a message naming the fatal (`... died before shipping a result: <message> in <file>:<line>`) when PHP recorded one. The bytes that used to corrupt the channel are now the diagnostic that explains the crash. The hook fires only on the crash path: a completed run writes its payload and dies by `SIGKILL`, which runs no shutdown function.
+
+### Added
+
+- Integration test `ServerStdioTest::testChildOutputNeverReachesTheProtocolStream`: a fixture project whose test echoes and then exits, asserting that the transport carries only parseable JSON-RPC frames *and* that the crash result reports the child's output and cause of death.
+- Transport purity is now asserted in `tearDown` for every integration test, not only the one written for it — a leak is a property of the transport, so it should fail wherever it appears rather than only where someone thought to look.
+
 ## [0.3.0] — 2026-05-23
 
 ### Security

src/PhpunitRunner.php

@@ -39,6 +39,13 @@ final class PhpunitRunner
 
     private static ?self $shared = null;
 
+    /**
+     * Set in the forked child once its result is on the wire, so the shutdown hook
+     * installed by {@see sealChildStdout()} does not ship a second, contradictory
+     * payload behind it.
+     */
+    private static bool $childResultSent = false;
+
     private bool $warm = false;
     private InMemorySubscriber $subscriber;
 
@@ -140,6 +147,7 @@ public function run(array $argv): array
         if ($pid === 0) {
             // ---- CHILD ----
             fclose($parentSock);
+            self::sealChildStdout($childSock);
 
             try {
                 $payload = $this->runInProcess($argv);
@@ -153,6 +161,7 @@ public function run(array $argv): array
             $json = json_encode($payload, JSON_UNESCAPED_UNICODE | JSON_UNESCAPED_SLASHES);
             fwrite($childSock, (string) $json);
             fflush($childSock);
+            self::$childResultSent = true;
             fclose($childSock);
 
             $this->terminateChild();
@@ -209,6 +218,82 @@ public function run(array $argv): array
         return $payload;
     }
 
+    /**
+     * Make the forked child incapable of reaching stdout, and give whatever it
+     * printed somewhere better to go.
+     *
+     * stdout is the MCP transport. The child inherits it across the fork, so every
+     * byte user code prints lands in the parent's protocol stream -- in front of
+     * the next frame, and glued to it when the print is unterminated:
+     * `</html>{"jsonrpc":...}`. A client that frames on newlines cannot parse that
+     * line, so it blocks until its own timeout on a result that already arrived.
+     * Five minutes per call, and no verdict, for a run that took two seconds (#3).
+     *
+     * `runInProcess()` already wraps Application::run in ob_start, but a buffer is
+     * only as good as its ending: a test that fatals or calls exit() never reaches
+     * the matching ob_get_clean(), and PHP flushes every active buffer on the way
+     * out -- to fd 1. So the seal is a buffer that is never ended and whose
+     * callback returns nothing: the shutdown flush still runs, and still writes
+     * zero bytes. Any phase is covered, destructors and shutdown functions included.
+     *
+     * Discarding alone would trade a loud bug for a silent one: the debug echo
+     * someone left in a test, and the fatal that killed the run, would both vanish.
+     * So the same hook ships them over the socket instead, and the parent reports
+     * them as the result. The bytes that used to corrupt the channel become the
+     * diagnostic that explains the crash.
+     *
+     * This only ever fires on the crash path. A run that completes writes its
+     * payload and dies by SIGKILL in {@see terminateChild()}, which runs no
+     * shutdown function at all; the flag covers the posix-less fallback there,
+     * where exit(0) does run them.
+     *
+     * @param resource $childSock
+     */
+    private static function sealChildStdout($childSock): void
+    {
+        ob_start(static fn (string $buffer, int $phase): string => '');
+
+        register_shutdown_function(static function () use ($childSock): void {
+            if (self::$childResultSent) {
+                return;
+            }
+
+            $printed = ob_get_contents();
+            $payload = [
+                'exit_code' => 255,
+                'output'    => self::errorOutput(
+                    self::deathMessage(error_get_last()),
+                    is_string($printed) ? $printed : '',
+                ),
+            ];
+
+            @fwrite($childSock, (string) json_encode($payload, JSON_UNESCAPED_UNICODE | JSON_UNESCAPED_SLASHES));
+            @fflush($childSock);
+        });
+    }
+
+    /**
+     * Name what killed the child, when PHP recorded it.
+     *
+     * `error_get_last()` also returns warnings and notices that were survived, so
+     * only the fatal classes are quoted -- a warning from early in the run would
+     * name the wrong culprit with total confidence.
+     *
+     * @param array{type: int, message: string, file: string, line: int}|null $lastError
+     */
+    private static function deathMessage(?array $lastError): string
+    {
+        $message = 'phpunit child died before shipping a result';
+
+        $fatal = [E_ERROR, E_PARSE, E_CORE_ERROR, E_COMPILE_ERROR, E_USER_ERROR];
+        if ($lastError !== null && \in_array($lastError['type'], $fatal, true)) {
+            return $message . ': ' . $lastError['message']
+                . ' in ' . $lastError['file'] . ':' . $lastError['line'];
+        }
+
+        return $message . ' (no fatal recorded -- exit(), a signal, or a crash in an extension)';
+    }
+
     /**
      * Execute PHPUnit in the current process and collect the in-memory result.
      *
@@ -288,10 +373,16 @@ private function terminateChild(): never
     /**
      * Build an InMemorySubscriber-shaped result JSON carrying a single error, so
      * the validator adapter renders the failure instead of choking on empty output.
+     *
+     * `$echoed` carries whatever the run printed, under the same `echo` key
+     * {@see runInProcess()} uses on the success path. It is the only route out for
+     * output produced by a run that died -- stdout is the MCP transport and the
+     * child is sealed off from it (#3) -- so a debug echo, or the error page a
+     * framework rendered on its way down, still reaches whoever reads the result.
      */
-    private function errorOutput(string $message): string
+    private static function errorOutput(string $message, string $echoed = ''): string
     {
-        return (string) json_encode([
+        $result = [
             'tests'      => 0,
             'assertions' => 0,
             'failures'   => [],
@@ -304,7 +395,13 @@ private function errorOutput(string $message): string
             ]],
             'skipped' => [],
             'time'    => 0.0,
-        ], JSON_UNESCAPED_UNICODE | JSON_UNESCAPED_SLASHES);
+        ];
+
+        if ($echoed !== '') {
+            $result['echo'] = $echoed;
+        }
+
+        return (string) json_encode($result, JSON_UNESCAPED_UNICODE | JSON_UNESCAPED_SLASHES);
     }
 
     private static function prewarmProbePath(): string

tests/Integration/ServerStdioTest.php

@@ -12,20 +12,52 @@
  */
 final class ServerStdioTest extends TestCase
 {
+    /** Printed by the child in the #3 fixture; must never appear on the transport. */
+    private const CHILD_MARKER = '<!DOCTYPE html>';
+
     private static string $bin;
     private static string $fixtureDir;
 
     /** @var list<string> temp project dirs created per test, removed in tearDown */
     private array $tmpDirs = [];
 
+    /** @var list<string> every byte each spawned server wrote to stdout this test */
+    private array $transcripts = [];
+
     protected function tearDown(): void
     {
+        // Checked for every test rather than only the one that targets it (#3):
+        // a leak is a property of the transport, so the suite should fail wherever
+        // it appears, not only where someone thought to look for it.
+        foreach ($this->transcripts as $transcript) {
+            $this->assertTransportCarriesOnlyFrames($transcript);
+        }
+        $this->transcripts = [];
+
         foreach ($this->tmpDirs as $dir) {
             $this->removeDir($dir);
         }
         $this->tmpDirs = [];
     }
 
+    /**
+     * Every non-empty line the server wrote must be a JSON-RPC frame and nothing
+     * else. Anything a client cannot parse belongs nowhere near this stream.
+     */
+    private function assertTransportCarriesOnlyFrames(string $transcript): void
+    {
+        foreach (explode("\n", trim($transcript)) as $line) {
+            $line = trim($line);
+            if ($line === '') {
+                continue;
+            }
+            self::assertIsArray(
+                json_decode($line, true),
+                'every line on the transport must be a JSON-RPC frame, got: ' . substr($line, 0, 200),
+            );
+        }
+    }
+
     private function removeDir(string $dir): void
     {
         if (!is_dir($dir)) {
@@ -234,6 +266,164 @@ public function testEditedSourceIsReloadedAcrossCalls(): void
         }
     }
 
+    /**
+     * Regression for #3: nothing the forked child prints may reach stdout.
+     *
+     * stdout IS the MCP transport. A test that echoes and then dies leaves PHP's
+     * shutdown path to flush every active output buffer to fd 1, so the echoed
+     * bytes land in front of the next protocol frame and glue themselves to it:
+     * `</html>{"jsonrpc":...}`. A client that frames on newlines cannot parse
+     * that line and waits out its entire timeout on an answer it already holds.
+     * Measured against a host application that renders an HTML error page on
+     * fatals: 45KB of it in the stream, and five minutes per call for a verdict
+     * the server had produced in two seconds.
+     *
+     * The assertion is deliberately about the transport rather than about the
+     * response: a fix that merely reordered the frames would still leave a
+     * client parsing HTML.
+     */
+    public function testChildOutputNeverReachesTheProtocolStream(): void
+    {
+        $project = $this->makeEchoingCrashProject();
+
+        $stdout = $this->rawStdout($project, [
+            ['jsonrpc' => '2.0', 'id' => 1, 'method' => 'initialize', 'params' => [
+                'protocolVersion' => '2024-11-05',
+                'capabilities'    => new \stdClass(),
+                'clientInfo'      => ['name' => 'phpunit', 'version' => '1.0.0'],
+            ]],
+            ['jsonrpc' => '2.0', 'method' => 'notifications/initialized'],
+            $this->runCall(2),
+        ]);
+
+        // Deliberately NOT a substring check for the marker: once the child's output
+        // is reported rather than discarded, the marker travels inside the frame as
+        // data, which is the whole point. What must never happen is the marker
+        // reaching the stream as bytes of its own -- and that is exactly what frame
+        // purity states. Before the fix this failed on
+        // `<!DOCTYPE html>...</html>{"jsonrpc":...}`, which decodes as nothing.
+        $this->assertTransportCarriesOnlyFrames($stdout);
+
+        // Sealed, not swallowed: the crash still has to be reported, and what the
+        // child printed on its way down is the most useful thing in the report.
+        $call = $this->frame($stdout, 2);
+        $output = json_decode($call['result']['structuredContent']['output'] ?? '', true);
+        self::assertIsArray($output, 'crash must still produce a result payload');
+
+        self::assertStringContainsString(
+            self::CHILD_MARKER,
+            $output['echo'] ?? '',
+            'output printed by the dying child must be reported in the result',
+        );
+        self::assertStringContainsString(
+            'died before shipping a result',
+            $output['errors'][0]['message'] ?? '',
+            'the result must say the child died rather than report an empty run',
+        );
+    }
+
+    /**
+     * The frame with $id, parsed out of a raw transcript.
+     *
+     * @return array<string,mixed>
+     */
+    private function frame(string $transcript, int $id): array
+    {
+        foreach (explode("\n", $transcript) as $line) {
+            $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 frame for id={$id} in: " . substr($transcript, 0, 500));
+    }
+
+    /**
+     * A project whose only test echoes and then ends the process, reproducing
+     * both halves of #3 at once: output produced by user code, and a child that
+     * dies before it can ship its result.
+     */
+    private function makeEchoingCrashProject(): string
+    {
+        $dir = sys_get_temp_dir() . '/phpunit_mcp_leak_' . bin2hex(random_bytes(6));
+        mkdir($dir . '/tests', 0777, true);
+        $this->tmpDirs[] = $dir;
+
+        $marker = self::CHILD_MARKER;
+        file_put_contents($dir . '/tests/EchoingCrashTest.php', <<<PHP
+            <?php
+
+            declare(strict_types=1);
+
+            use PHPUnit\\Framework\\TestCase;
+
+            final class EchoingCrashTest extends TestCase
+            {
+                public function testEchoesThenDies(): void
+                {
+                    echo '{$marker}<html><body>error page</body></html>';
+                    exit(1);
+                }
+            }
+            PHP);
+        file_put_contents($dir . '/phpunit.xml', <<<XML
+            <?xml version="1.0" encoding="UTF-8"?>
+            <phpunit colors="false" failOnRisky="false" failOnWarning="false">
+                <testsuites>
+                    <testsuite name="leak">
+                        <directory>tests</directory>
+                    </testsuite>
+                </testsuites>
+            </phpunit>
+            XML);
+
+        return $dir;
+    }
+
+    /**
+     * The server's stdout verbatim — unfiltered, because the filtering is what
+     * this test exists to check.
+     *
+     * @param list<array<string,mixed>> $messages
+     */
+    private function rawStdout(string $project, array $messages): string
+    {
+        $cmd = [
+            self::$bin,
+            '--no-prewarm',
+            '--working-dir=' . $project,
+            '--config=' . $project . '/phpunit.xml',
+        ];
+
+        $stdin = '';
+        foreach ($messages as $message) {
+            $stdin .= json_encode($message) . "\n";
+        }
+
+        $proc = proc_open(
+            $cmd,
+            [0 => ['pipe', 'r'], 1 => ['pipe', 'w'], 2 => ['pipe', 'w']],
+            $pipes,
+        );
+        self::assertIsResource($proc);
+        fwrite($pipes[0], $stdin);
+        fclose($pipes[0]);
+        $stdout = stream_get_contents($pipes[1]) ?: '';
+        stream_get_contents($pipes[2]);
+        fclose($pipes[1]);
+        fclose($pipes[2]);
+        proc_close($proc);
+
+        $this->transcripts[] = $stdout;
+
+        return $stdout;
+    }
+
     /**
      * @return array{handle: resource, stdin: resource, stdout: resource, stderr: string}
      */
@@ -385,6 +575,8 @@ private function invoke(array $messages, bool $withProject): array
         fclose($pipes[2]);
         proc_close($proc);
 
+        $this->transcripts[] = $stdout;
+
         $responses = [];
         foreach (explode("\n", $stdout) as $line) {
             $line = trim($line);