Live server: stop ends the process, SSE skips the mutation lane

Two Rust-only regressions found while investigating #719, both of which
can leave a tab waiting on a broadcast that never comes.

/stop ran shutdown() but never set shutting_down, and the accept loop
only breaks on that flag or a signal, so a stopped server kept its port
and kept answering while its server.json was already deleted. The next
`impeccable live` then booted a second server on another port and a tab
could reattach to the zombie. Node's shutdown() ended in process.exit(0).
The flag is now set after the response is written, so `stop` still reads
"stopping" instead of a reset connection, and the accept loop (already
non-blocking) exits on its next pass.

GET /events took a turnstile ticket and waited its turn before
registering, even though handle_sse releases that ticket two statements
later and needs no arrival ordering. A peer that stalls mid-request holds
the lane for the whole READ_REQUEST_DEADLINE, so a reconnecting stream
could sit unregistered for up to 10 seconds (measured 9.71s against 0.00s
on Node); broadcast is fire-and-forget, so a `done` landing in that
window reaches an empty client set and is gone. Registering early can
only make a stream see more broadcasts. The one cost is that the
connected frame's activeSessions snapshot may miss a mutation still in
flight, and the browser treats that snapshot as a hint. Preflights still
take a turn: answering those out of order reorders the POSTs the browser
issues behind them.

The route classification moved into releases_ticket_up_front so it can be
unit tested. tests/live-server-leak.test.mjs gains a guard that a stopped
server's pid is gone and its port is free.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vau2X53xGTjjTCXWMVBoNY

crates/live/src/live_server.rs

@@ -549,6 +549,39 @@ fn is_loopback_origin(origin: &str) -> bool {
     host == "localhost" || host == "127.0.0.1" || host == "::1" || host == "[::1]"
 }
 
+/// Routes that give their turnstile ticket up before touching server state.
+///
+/// Static assets and read-only file routes need no arrival ordering. Neither
+/// does the SSE stream: `handle_sse` releases its ticket two statements after
+/// it registers anyway, but taking a turn first means the registration waits
+/// behind every connection accepted before it, and a peer that stalls
+/// mid-request holds the lane for the whole `READ_REQUEST_DEADLINE` (10s).
+/// A `done` broadcast that lands in that window reaches an empty client set
+/// and is gone for good, which is one way a reconnecting tab sits at the
+/// generating loader forever (issue #719). Registering early can only make a
+/// stream see MORE broadcasts; the one thing it costs is that the `connected`
+/// frame's `activeSessions` snapshot may miss a mutation still in flight, and
+/// the browser treats that snapshot as a hint, not as truth.
+///
+/// CORS preflights deliberately do NOT release: the browser issues the real
+/// POSTs as their preflights are answered, so answering preflights out of
+/// order would reorder the POSTs (checkpoint before generate) at the source.
+fn releases_ticket_up_front(path: &str, method: &str) -> bool {
+    matches!(
+        (path, method),
+        ("/live.js", _)
+            | ("/detect.js", _)
+            | ("/", _)
+            | ("/modern-screenshot.js", _)
+            | ("/health", _)
+            | ("/status", _)
+            | ("/design-system.json", _)
+            | ("/design-system/raw", _)
+            | ("/source", _)
+            | ("/events", "GET")
+    )
+}
+
 fn respond(stream: &mut TcpStream, cors: &[(String, String)], res: Response) {
     send_response(stream, cors, &res);
 }
@@ -586,17 +619,10 @@ fn handle_connection(shared: Shared, mut stream: TcpStream, mut ticket: Ticket)
     // CORS preflights do take a turn: the browser issues the real POSTs as
     // their preflights are answered, so answering preflights out of order
     // would reorder the POSTs (checkpoint before generate) at the source.
-    match (req.path.as_str(), req.method.as_str()) {
-        ("/live.js", _)
-        | ("/detect.js", _)
-        | ("/", _)
-        | ("/modern-screenshot.js", _)
-        | ("/health", _)
-        | ("/status", _)
-        | ("/design-system.json", _)
-        | ("/design-system/raw", _)
-        | ("/source", _) => ticket.release(),
-        _ => ticket.wait_turn(),
+    if releases_ticket_up_front(&req.path, &req.method) {
+        ticket.release();
+    } else {
+        ticket.wait_turn();
     }
     let token_now = lock(&shared).token.clone();
     let mut cors: Vec<(String, String)> = Vec::new();
@@ -904,6 +930,14 @@ fn handle_connection(shared: Shared, mut stream: TcpStream, mut ticket: Ticket)
                 &cors,
                 text_res(200, Some("text/plain"), "stopping"),
             );
+            // JS: shutdown() ends in `process.exit(0)`. Without this the
+            // accept loop never sees a reason to stop, so a stopped server
+            // keeps the port and keeps answering while its `server.json` is
+            // already gone: the next `impeccable live` boots a second server
+            // on another port and a tab can reattach to the zombie. Set the
+            // flag after the response is written so `stop` still reads
+            // `stopping` rather than a reset connection.
+            lock(&shared).shutting_down = true;
         }
         ("/poll", "GET") => handle_poll_get(&shared, stream, &cors, &req, token_ok, &mut ticket),
         ("/poll", "POST") => {
@@ -2585,4 +2619,36 @@ mod content_type_tests {
             .iter()
             .any(|(k, v)| k == "Content-Type" && v == "application/javascript; charset=utf-8"));
     }
+
+    #[test]
+    fn sse_stream_does_not_take_a_turn_in_the_mutation_lane() {
+        // The stream releases its ticket right after it registers anyway, so
+        // waiting for a turn first buys nothing and can park the registration
+        // behind a stalled peer for the whole read deadline (issue #719).
+        assert!(releases_ticket_up_front("/events", "GET"));
+        // Everything that mutates state still passes through the lane in
+        // arrival order, preflights included: answering those out of order
+        // reorders the POSTs the browser issues behind them.
+        assert!(!releases_ticket_up_front("/events", "POST"));
+        assert!(!releases_ticket_up_front("/events", "OPTIONS"));
+        assert!(!releases_ticket_up_front("/poll", "POST"));
+        assert!(!releases_ticket_up_front("/stop", "GET"));
+    }
+
+    #[test]
+    fn read_only_and_asset_routes_still_skip_the_lane() {
+        for path in [
+            "/live.js",
+            "/detect.js",
+            "/",
+            "/modern-screenshot.js",
+            "/health",
+            "/status",
+            "/design-system.json",
+            "/design-system/raw",
+            "/source",
+        ] {
+            assert!(releases_ticket_up_front(path, "GET"), "{path}");
+        }
+    }
 }

tests/live-server-leak.test.mjs

@@ -37,6 +37,8 @@ const WINDOWS = process.platform === 'win32';
 
 const REPO_ROOT = fileURLToPath(new URL('..', import.meta.url));
 const PORT = 8591;
+// A second port so the stop guard never collides with the reaper guard.
+const STOP_PORT = 8592;
 // The server under test is the engine's `live-server` verb, not a script: this
 // guarantee has to hold for whatever binary the harness is pointed at.
 const ENGINE_BIN = findEngineBinary();
@@ -150,6 +152,59 @@ describe('live server leak guard', () => {
     }
   });
 
+  it('ends the server process on stop, so the port is free again', {
+    skip: NO_ENGINE,
+  }, async () => {
+    // Node's shutdown() finished with process.exit(0). The engine's did not,
+    // and nothing set `shutting_down`, so a stopped server kept the port and
+    // kept answering while its server.json was already gone: the next
+    // `impeccable live` booted a second server elsewhere and a tab could
+    // reattach to the zombie and never hear another broadcast (issue #719).
+    const cwd = mkdtempSync(join(tmpdir(), 'impeccable-stop-'));
+    let server;
+    try {
+      writeFileSync(join(cwd, 'package.json'), '{"name":"stop-fixture"}\n');
+      server = spawn(ENGINE_BIN, ['live-server', `--port=${STOP_PORT}`], {
+        cwd,
+        stdio: ['ignore', 'pipe', 'pipe'],
+        env: {
+          ...process.env,
+          IMPECCABLE_SKILL_DIR: join(REPO_ROOT, 'skill'),
+          IMPECCABLE_SELF: ENGINE_BIN,
+        },
+      });
+      const up = await waitUntil(async () => {
+        try {
+          const res = await fetch(`http://127.0.0.1:${STOP_PORT}/health`);
+          return res.ok || res.status === 401;
+        } catch { return false; }
+      }, { timeoutMs: 20_000 });
+      assert.equal(up, true, 'live server never came up');
+
+      const stop = spawn(ENGINE_BIN, ['live-server', 'stop'], {
+        cwd,
+        stdio: ['ignore', 'pipe', 'pipe'],
+        env: {
+          ...process.env,
+          IMPECCABLE_SKILL_DIR: join(REPO_ROOT, 'skill'),
+          IMPECCABLE_SELF: ENGINE_BIN,
+        },
+      });
+      await new Promise((resolve) => stop.on('exit', resolve));
+
+      const gone = await waitUntil(() => !alive(server.pid), { timeoutMs: 15_000 });
+      assert.equal(gone, true, `live server pid ${server.pid} survived its own stop`);
+
+      const stillServing = await fetch(`http://127.0.0.1:${STOP_PORT}/health`)
+        .then(() => true)
+        .catch(() => false);
+      assert.equal(stillServing, false, 'stopped server is still answering on its port');
+    } finally {
+      if (server && server.exitCode == null) server.kill('SIGKILL');
+      rmSync(cwd, { recursive: true, force: true });
+    }
+  });
+
   it('scopes a sweep to the marker, never to the name alone', () => {
     // No marker, no match: a sweep can never take out a live server that some
     // other checkout, or the user's own session, is running.