fix(core): address dev server review follow-up

.changeset/dev-server-review-followup.md

@@ -0,0 +1,5 @@
+---
+"@aws-blocks/core": patch
+---
+
+Tighten dev server singleton and retry handling for port contention follow-ups.

packages/core/src/scripts/dev-server-reclaim.test.ts

@@ -8,6 +8,7 @@ import {
   reclaimMessage,
   evaluatePortBindRetry,
   createBindRetryController,
+  scheduleBindRetry,
   DEFAULT_PORT_BIND_RETRY_POLICY,
   evaluateSingleton,
   parsePidRecord,
@@ -188,8 +189,8 @@ describe('evaluatePortBindRetry — front-door bind retry budget', () => {
 });
 
 // ── evaluateSingleton — singleton guard decision ────────────────────────────
-// Prevents two fighting supervisors while never blocking a `tsx watch` reload of
-// the SAME supervisor (same parent pid).
+// Prevents two fighting supervisors while allowing a `tsx watch` reload after
+// the previous child has exited.
 describe('evaluateSingleton — one supervisor per port, hot-reload safe', () => {
   const rec = (over: Partial<DevServerPidRecord> = {}): DevServerPidRecord => ({
     pid: 1000,
@@ -204,12 +205,19 @@ describe('evaluateSingleton — one supervisor per port, hot-reload safe', () =>
     assert.deepStrictEqual(evaluateSingleton(null, { pid: 1, ppid: 2 }, true, alive), { action: 'proceed' });
   });
 
-  it('proceeds on a tsx-watch relaunch of our own supervisor (same parent pid)', () => {
-    // New child, DIFFERENT own pid, but SAME watcher parent → not a competitor.
-    const d = evaluateSingleton(rec({ pid: 1000, ppid: 500 }), { pid: 1001, ppid: 500 }, true, alive);
+  it('proceeds on a tsx-watch relaunch after the previous child has exited', () => {
+    // New child, DIFFERENT own pid, SAME watcher parent, and old child is dead.
+    const d = evaluateSingleton(rec({ pid: 1000, ppid: 500 }), { pid: 1001, ppid: 500 }, true, dead);
     assert.deepStrictEqual(d, { action: 'proceed' });
   });
 
+  it('exits when a same-parent live owner is actually holding the port', () => {
+    const d = evaluateSingleton(rec({ pid: 1000, ppid: 500, port: 3000 }), { pid: 1001, ppid: 500 }, true, alive);
+    if (d.action !== 'exit') assert.fail(`expected exit, got ${d.action}`);
+    assert.match(d.reason, /already running on :3000/);
+    assert.match(d.reason, /pid 1000/);
+  });
+
   it('exits when a different, live supervisor is actually holding the port', () => {
     const d = evaluateSingleton(rec({ pid: 1000, ppid: 500, port: 3000 }), { pid: 2000, ppid: 900 }, true, alive);
     if (d.action !== 'exit') assert.fail(`expected exit, got ${d.action}`);
@@ -428,3 +436,29 @@ describe('createBindRetryController — bounded EADDRINUSE reclaim-and-rebind',
     assert.strictEqual(c.scheduled.length, 1);
   });
 });
+
+describe('scheduleBindRetry — front-door retry timer', () => {
+  it('does not unref the retry timer', () => {
+    let callback: (() => void) | undefined;
+    let delay: number | undefined;
+    let unrefCalls = 0;
+    const timer = {
+      unref: () => { unrefCalls += 1; return timer; },
+    } as NodeJS.Timeout;
+
+    const returned = scheduleBindRetry(
+      () => {},
+      250,
+      (fn, delayMs) => {
+        callback = fn;
+        delay = delayMs;
+        return timer;
+      },
+    );
+
+    assert.strictEqual(returned, timer);
+    assert.strictEqual(delay, 250);
+    assert.ok(callback);
+    assert.strictEqual(unrefCalls, 0, 'front-door bind retry must keep the event loop alive');
+  });
+});

packages/core/src/scripts/dev-server.ts

@@ -396,6 +396,15 @@ export interface BindRetryDeps {
   warn: (msg: string) => void;
 }
 
+/** Schedule a front-door rebind retry; keep it refed so the retry deterministically fires. */
+export function scheduleBindRetry(
+  fn: () => void,
+  delayMs: number,
+  setTimer: (fn: () => void, delayMs: number) => NodeJS.Timeout = setTimeout,
+): NodeJS.Timeout {
+  return setTimer(fn, delayMs);
+}
+
 /**
  * Build the `:3000` front-door EADDRINUSE bind-retry handler. Extracted from the
  * `server.on('error')` closure so the retry *wiring* — the 1-based attempt
@@ -490,10 +499,11 @@ export type SingletonDecision = { action: 'proceed' } | { action: 'exit'; reason
  *
  * - **No / corrupt pidfile** → proceed (first start; startup reclaim covers any orphan socket).
  * - **Same pid** → proceed (defensive; the record is our own).
- * - **Same parent (`ppid`)** → proceed. `tsx watch` is the stable parent across
- *   reloads, so a matching parent means the watcher is relaunching OUR OWN script
- *   — not a competitor. A second `npm run dev` runs under a *different* watcher,
- *   so it never matches here. This carve-out is what preserves hot reload.
+ * - **Same parent (`ppid`) and dead recorded child** → proceed. `tsx watch` is
+ *   the stable parent across reloads, and it relaunches after the old child exits.
+ *   A live recorded child with the same parent can also happen when two
+ *   `npm run dev` jobs share a shell, so that still goes through the live-owner
+ *   check below.
  * - **Different, still-live owner actually holding the port** → exit cleanly with
  *   a clear message (do not spawn a competing supervisor).
  * - **Otherwise** (recorded owner is dead → stale pidfile, or the port is free)
@@ -507,8 +517,9 @@ export function evaluateSingleton(
 ): SingletonDecision {
   if (!existing) return { action: 'proceed' };
   if (existing.pid === self.pid) return { action: 'proceed' };
-  if (existing.ppid === self.ppid) return { action: 'proceed' }; // tsx-watch relaunch of our own supervisor
-  const ownerAlive = isAlive(existing.pid) || (existing.ppid > 1 && isAlive(existing.ppid));
+  const existingPidAlive = isAlive(existing.pid);
+  if (existing.ppid === self.ppid && !existingPidAlive) return { action: 'proceed' }; // tsx-watch relaunch after old child exit
+  const ownerAlive = existingPidAlive || (existing.ppid > 1 && isAlive(existing.ppid));
   if (ownerAlive && portInUse) {
     return { action: 'exit', reason: `dev server already running on :${existing.port} (pid ${existing.pid})` };
   }
@@ -984,7 +995,7 @@ export async function startDevServer(options: DevServerOptions) {
   const onEaddrinuse = createBindRetryController(port, {
     reclaim: (p) => reclaimPort(p),
     relisten: () => server.listen(port, onListening),
-    scheduleRetry: (fn, delayMs) => { setTimeout(fn, delayMs).unref?.(); },
+    scheduleRetry: (fn, delayMs) => { scheduleBindRetry(fn, delayMs); },
     onExhausted: () => process.exit(1),
     warn: (msg) => console.error(msg),
   });