From 82ec8782b490a01f4e89edbf93356ebbaefc08b6 Mon Sep 17 00:00:00 2001
From: Vito Sansevero <vito.sansevero@gmail.com>
Date: Wed, 21 Oct 2026 22:57:03 -0700
Subject: [PATCH] fix(browser): server shutdown and its tests on Windows,
 Python < 3.13 and CI

Three CI failures on ed809f7:

- Windows: server.py exited 1 on stdin EOF because create_app works with
  the temp root as its cwd, and Windows cannot remove a process's cwd.
  main() now restores the original cwd before the root is removed.
- Python 3.10 to 3.12: the lifetime test's cleanup called communicate()
  after closing stdin, which flushes the closed pipe (ValueError); 3.13
  tolerates it. It now waits and closes the pipes itself, and the exit
  assertion shows the server's stderr. Under 3.12: errors=2 before, OK
  after.
- Tooling (CI container): the SIGKILLed-worker tests treated an exited
  server as alive, because the container's PID 1 does not reap orphans
  and kill(pid, 0) succeeds on a zombie. Reproduced locally with docker
  without --init (2 failed); a zombie now counts as exited (2 passed).
---
 tests/browser/server.py                      |  7 ++++++-
 tests/test_browser_server_lifetime.py        | 10 ++++++++--
 tests/vendor/browser-server-fixture.test.mjs |  7 ++++++-
 3 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/tests/browser/server.py b/tests/browser/server.py
index c446ed0..77dc0f4 100644
--- a/tests/browser/server.py
+++ b/tests/browser/server.py
@@ -583,8 +583,13 @@ def main(argv: list[str] | None = None) -> None:
     loop = asyncio.new_event_loop()
     if args.exit_on_stdin_eof:
         exit_when_stdin_closes(loop)
+    original_cwd = os.getcwd()
     with tempfile.TemporaryDirectory(prefix="pm-browser-") as directory:
-        web.run_app(create_app(Path(directory).resolve()), sock=listener, loop=loop)
+        try:
+            web.run_app(create_app(Path(directory).resolve()), sock=listener, loop=loop)
+        finally:
+            # create_app works inside the root; Windows cannot remove a process's cwd.
+            os.chdir(original_cwd)
 
 
 if __name__ == "__main__":
diff --git a/tests/test_browser_server_lifetime.py b/tests/test_browser_server_lifetime.py
index 1380ed5..eb68bbb 100644
--- a/tests/test_browser_server_lifetime.py
+++ b/tests/test_browser_server_lifetime.py
@@ -39,7 +39,10 @@ class TestBrowserServerLifetime(unittest.TestCase):
         def stop():
             if process.poll() is None:
                 process.kill()
-            process.communicate(timeout=SERVER_TIMEOUT_S)
+            process.wait(timeout=SERVER_TIMEOUT_S)
+            # communicate() would flush a closed stdin, which Python < 3.13 refuses.
+            for stream in (process.stdin, process.stdout, process.stderr):
+                stream.close()
 
         self.addCleanup(stop)
         line = process.stdout.readline().decode()
@@ -63,7 +66,10 @@ class TestBrowserServerLifetime(unittest.TestCase):
         process = self.start("--exit-on-stdin-eof")
         self.assertEqual(len(self.roots()), 1, "the server works in its own temp root")
         process.stdin.close()
-        self.assertEqual(process.wait(timeout=SERVER_TIMEOUT_S), 0)
+        code = process.wait(timeout=SERVER_TIMEOUT_S)
+        self.assertEqual(
+            code, 0, process.stderr.read().decode(errors="replace")[-4000:]
+        )
         self.assertEqual(self.roots(), [], "the temp root and its database are removed")
 
     def test_server_keeps_running_with_stdin_closed_without_the_flag(self):
diff --git a/tests/vendor/browser-server-fixture.test.mjs b/tests/vendor/browser-server-fixture.test.mjs
index aa8c3e1..9b05c4b 100644
--- a/tests/vendor/browser-server-fixture.test.mjs
+++ b/tests/vendor/browser-server-fixture.test.mjs
@@ -3,6 +3,7 @@ import assert from 'node:assert/strict';
 import { createServer } from 'node:net';
 import { spawn } from 'node:child_process';
 import { once } from 'node:events';
+import { readFileSync } from 'node:fs';
 import { fileURLToPath, pathToFileURL } from 'node:url';
 import { setTimeout as delay } from 'node:timers/promises';
 import { startBrowserServer, createServerSupervisor, attachServerStderr } from '../../tools/vendor/start-browser-server.mjs';
@@ -27,7 +28,11 @@ async function writeStderr(server, text) {
     await waitFor(() => server.stderrTail().endsWith(text.trimEnd()), `stderr ${JSON.stringify(text)}`);
 }
 
-const alive = pid => { try { process.kill(pid, 0); return true; } catch { return false; } };
+/** A process that exited counts as gone even while a non-reaping PID 1 (CI's container) leaves it a zombie. */
+function alive(pid) {
+    try { process.kill(pid, 0); } catch { return false; }
+    try { return readFileSync(`/proc/${pid}/stat`, 'utf8').replace(/^.*\) /s, '')[0] !== 'Z'; } catch { return true; }
+}
 
 /** Starts a server from a separate Node worker process, SIGKILLs that worker and returns the server's pid. */
 async function serverOfKilledWorker(options) {