fix: stabilize PR comment sessions

Author: DjDeveloperr

actions/run-android-comment-session/action.yml

@@ -324,36 +324,6 @@ runs:
           exit 1
         fi
 
-        cat > simdeck-daemon-supervisor.sh <<'EOF'
-        #!/usr/bin/env bash
-        set +e
-        terminating=0
-        child=""
-        trap 'terminating=1; if [[ -n "${child}" ]]; then kill "${child}" 2>/dev/null; wait "${child}" 2>/dev/null; fi; exit 0' TERM INT HUP
-
-        while true; do
-          "$@" >> simdeck-daemon.log 2>&1 &
-          child="$!"
-          echo "${child}" > simdeck-child.pid
-          wait "${child}"
-          status="$?"
-          child=""
-
-          if [[ "${terminating}" -eq 1 ]]; then
-            exit 0
-          fi
-          if [[ "${status}" -eq 75 || "${status}" -ge 128 ]]; then
-            echo "[simdeck-action-supervisor] daemon exited with status ${status}; restarting" >> simdeck-daemon.log
-            sleep 1
-            continue
-          fi
-
-          echo "[simdeck-action-supervisor] daemon exited with status ${status}; not restarting" >> simdeck-daemon.log
-          exit "${status}"
-        done
-        EOF
-        chmod +x simdeck-daemon-supervisor.sh
-
         SIMDECK_VIDEO_CODEC=software \
           SIMDECK_ANDROID_VIDEO_CODEC=software \
           SIMDECK_ALLOWED_ORIGINS='*' \
@@ -364,7 +334,6 @@ runs:
           SIMDECK_REALTIME_MIN_BITRATE="${stream_min_bitrate}" \
           SIMDECK_REALTIME_BITS_PER_PIXEL="${stream_bits_per_pixel}" \
           SIMDECK_LOCAL_STREAM_FPS="${stream_fps}" \
-          ./simdeck-daemon-supervisor.sh \
           simdeck daemon run \
           --project-root "${GITHUB_WORKSPACE}" \
           --metadata-path "${metadata_path}" \
@@ -374,7 +343,8 @@ runs:
           --stream-quality "${SIMDECK_STREAM_PROFILE}" \
           --local-stream-fps "${stream_fps}" \
           --access-token "${access_token}" \
-          --pairing-code 000000 &
+          --pairing-code 000000 \
+          > simdeck-daemon.log 2>&1 &
         echo "$!" > simdeck.pid
 
         cloudflared tunnel --url "http://127.0.0.1:${SIMDECK_PORT}" --protocol http2 --no-autoupdate > cloudflared.log 2>&1 &

actions/run-ios-comment-session/action.yml

@@ -289,36 +289,6 @@ runs:
           exit 1
         fi
 
-        cat > simdeck-daemon-supervisor.sh <<'EOF'
-        #!/usr/bin/env bash
-        set +e
-        terminating=0
-        child=""
-        trap 'terminating=1; if [[ -n "${child}" ]]; then kill "${child}" 2>/dev/null; wait "${child}" 2>/dev/null; fi; exit 0' TERM INT HUP
-
-        while true; do
-          "$@" >> simdeck-daemon.log 2>&1 &
-          child="$!"
-          echo "${child}" > simdeck-child.pid
-          wait "${child}"
-          status="$?"
-          child=""
-
-          if [[ "${terminating}" -eq 1 ]]; then
-            exit 0
-          fi
-          if [[ "${status}" -eq 75 || "${status}" -ge 128 ]]; then
-            echo "[simdeck-action-supervisor] daemon exited with status ${status}; restarting" >> simdeck-daemon.log
-            sleep 1
-            continue
-          fi
-
-          echo "[simdeck-action-supervisor] daemon exited with status ${status}; not restarting" >> simdeck-daemon.log
-          exit "${status}"
-        done
-        EOF
-        chmod +x simdeck-daemon-supervisor.sh
-
         SIMDECK_VIDEO_CODEC=software \
           SIMDECK_ALLOWED_ORIGINS='*' \
           SIMDECK_REALTIME_STREAM=1 \
@@ -328,7 +298,6 @@ runs:
           SIMDECK_REALTIME_MIN_BITRATE="${stream_min_bitrate}" \
           SIMDECK_REALTIME_BITS_PER_PIXEL="${stream_bits_per_pixel}" \
           SIMDECK_LOCAL_STREAM_FPS="${stream_fps}" \
-          ./simdeck-daemon-supervisor.sh \
           simdeck daemon run \
           --project-root "${GITHUB_WORKSPACE}" \
           --metadata-path "${metadata_path}" \
@@ -338,7 +307,8 @@ runs:
           --stream-quality "${SIMDECK_STREAM_PROFILE}" \
           --local-stream-fps "${stream_fps}" \
           --access-token "${access_token}" \
-          --pairing-code 000000 &
+          --pairing-code 000000 \
+          > simdeck-daemon.log 2>&1 &
         echo "$!" > simdeck.pid
 
         cloudflared tunnel --url "http://127.0.0.1:${SIMDECK_PORT}" --protocol http2 --no-autoupdate > cloudflared.log 2>&1 &

bin/simdeck.mjs

@@ -5,11 +5,15 @@ import { existsSync } from "node:fs";
 import path from "node:path";
 import { fileURLToPath } from "node:url";
 
+const RECOVERABLE_RESTART_EXIT_CODE = 75;
+
 const packageRoot = path.resolve(
   path.dirname(fileURLToPath(import.meta.url)),
   "..",
 );
 const binaryPath = path.join(packageRoot, "build", "simdeck-bin");
+const childArgs = process.argv.slice(2);
+const isDaemonRun = childArgs[0] === "daemon" && childArgs[1] === "run";
 
 if (process.platform !== "darwin") {
   console.error("simdeck only supports macOS.");
@@ -23,28 +27,53 @@ if (!existsSync(binaryPath)) {
   process.exit(1);
 }
 
-const child = spawn(binaryPath, process.argv.slice(2), {
-  cwd: process.cwd(),
-  stdio: "inherit",
-});
-
-child.on("error", (error) => {
-  console.error(error.message);
-  process.exit(1);
-});
+let child;
+let terminating = false;
 
 for (const signal of ["SIGINT", "SIGTERM", "SIGHUP"]) {
   process.once(signal, () => {
-    if (!child.killed) {
+    terminating = true;
+    if (child && !child.killed) {
       child.kill(signal);
     }
   });
 }
 
-child.on("exit", (code, signal) => {
-  if (signal) {
-    process.kill(process.pid, signal);
-    return;
-  }
-  process.exit(code ?? 1);
-});
+function spawnChild() {
+  const env = isDaemonRun
+    ? {
+        ...process.env,
+        SIMDECK_DAEMON_METADATA_PID: String(process.pid),
+      }
+    : process.env;
+
+  child = spawn(binaryPath, childArgs, {
+    cwd: process.cwd(),
+    env,
+    stdio: "inherit",
+  });
+
+  child.on("error", (error) => {
+    console.error(error.message);
+    process.exit(1);
+  });
+
+  child.on("exit", (code, signal) => {
+    if (
+      isDaemonRun &&
+      !terminating &&
+      (code === RECOVERABLE_RESTART_EXIT_CODE || signal)
+    ) {
+      setTimeout(spawnChild, 500);
+      return;
+    }
+
+    if (signal) {
+      process.kill(process.pid, signal);
+      return;
+    }
+    process.exit(code ?? 1);
+  });
+}
+
+spawnChild();

scripts/github-actions.test.mjs

@@ -1,5 +1,16 @@
 import assert from "node:assert/strict";
-import { readFileSync } from "node:fs";
+import {
+  chmodSync,
+  copyFileSync,
+  mkdirSync,
+  mkdtempSync,
+  readFileSync,
+  rmSync,
+  writeFileSync,
+} from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { spawnSync } from "node:child_process";
 import { test } from "node:test";
 
 const iosAction = readFileSync(
@@ -25,6 +36,8 @@ function stepSlice(action, name, nextName) {
   return action.slice(startIndex, endIndex);
 }
 
+const darwinTest = process.platform === "darwin" ? test : test.skip;
+
 test("iOS PR comment waits for public simulator list access", () => {
   const prebootIndex = iosAction.indexOf(
     "- name: Select and preboot simulator",
@@ -149,33 +162,20 @@ for (const [platform, action, startStep, waitStep] of [
     );
   });
 
-  test(`${platform} PR comment supervises recoverable daemon exits`, () => {
+  test(`${platform} PR comment relies on the packaged CLI daemon supervisor`, () => {
     const startStepBody = stepSlice(
       action,
       "Install tools, start SimDeck and tunnel",
       "Resolve PR head",
     );
 
-    assert.match(
+    assert.doesNotMatch(
       startStepBody,
       /simdeck-daemon-supervisor\.sh/,
-      "action should run SimDeck through a local supervisor",
-    );
-    assert.match(
-      startStepBody,
-      /"\$\{status\}" -eq 75/,
-      "supervisor should restart recoverable SimDeck exits",
-    );
-    assert.match(
-      startStepBody,
-      /"\$\{status\}" -ge 128/,
-      "supervisor should restart signal-terminated daemon children",
-    );
-    assert.match(
-      startStepBody,
-      /simdeck-child\.pid/,
-      "supervisor should expose the active child pid for cleanup diagnostics",
+      "action should not carry a second workflow-local daemon supervisor",
     );
+    assert.match(startStepBody, /simdeck daemon run/);
+    assert.match(startStepBody, /echo "\$!" > simdeck\.pid/);
   });
 
   test(`${platform} PR comment keepalive tolerates transient daemon restarts`, () => {
@@ -207,3 +207,97 @@ for (const [platform, action, startStep, waitStep] of [
     );
   });
 }
+
+darwinTest(
+  "npm CLI wrapper restarts daemon run after recoverable native exit",
+  () => {
+    const root = mkdtempSync(join(tmpdir(), "simdeck-wrapper-test-"));
+    try {
+      mkdirSync(join(root, "bin"), { recursive: true });
+      mkdirSync(join(root, "build"), { recursive: true });
+      const wrapperPath = join(root, "bin", "simdeck.mjs");
+      const nativePath = join(root, "build", "simdeck-bin");
+      const logPath = join(root, "native.log");
+      const countPath = join(root, "count");
+
+      copyFileSync(new URL("../bin/simdeck.mjs", import.meta.url), wrapperPath);
+      chmodSync(wrapperPath, 0o755);
+      writeFileSync(
+        nativePath,
+        `#!/usr/bin/env bash
+set -euo pipefail
+count="$(cat "${countPath}" 2>/dev/null || echo 0)"
+count="$((count + 1))"
+echo "$count" > "${countPath}"
+echo "$$:\${SIMDECK_DAEMON_METADATA_PID:-}:\$*" >> "${logPath}"
+if [[ "$count" == "1" ]]; then
+  exit 75
+fi
+exit 0
+`,
+      );
+      chmodSync(nativePath, 0o755);
+
+      const result = spawnSync(
+        process.execPath,
+        [wrapperPath, "daemon", "run", "--port", "4310"],
+        {
+          encoding: "utf8",
+        },
+      );
+
+      assert.equal(result.status, 0, result.stderr);
+      const logLines = readFileSync(logPath, "utf8").trim().split("\n");
+      assert.equal(logLines.length, 2, "daemon run should be retried once");
+
+      const entries = logLines.map((line) => {
+        const [pid, metadataPid, args] = line.split(":");
+        return { pid, metadataPid, args };
+      });
+      assert.notEqual(entries[0].pid, entries[1].pid);
+      assert.match(entries[0].metadataPid, /^\d+$/);
+      assert.equal(entries[0].metadataPid, entries[1].metadataPid);
+      assert.notEqual(entries[0].pid, entries[0].metadataPid);
+      assert.equal(entries[0].args, "daemon run --port 4310");
+    } finally {
+      rmSync(root, { recursive: true, force: true });
+    }
+  },
+);
+
+darwinTest(
+  "npm CLI wrapper does not restart non-daemon commands on exit 75",
+  () => {
+    const root = mkdtempSync(join(tmpdir(), "simdeck-wrapper-test-"));
+    try {
+      mkdirSync(join(root, "bin"), { recursive: true });
+      mkdirSync(join(root, "build"), { recursive: true });
+      const wrapperPath = join(root, "bin", "simdeck.mjs");
+      const nativePath = join(root, "build", "simdeck-bin");
+      const logPath = join(root, "native.log");
+
+      copyFileSync(new URL("../bin/simdeck.mjs", import.meta.url), wrapperPath);
+      chmodSync(wrapperPath, 0o755);
+      writeFileSync(
+        nativePath,
+        `#!/usr/bin/env bash
+set -euo pipefail
+echo "$$:\${SIMDECK_DAEMON_METADATA_PID:-}:\$*" >> "${logPath}"
+exit 75
+`,
+      );
+      chmodSync(nativePath, 0o755);
+
+      const result = spawnSync(process.execPath, [wrapperPath, "list"], {
+        encoding: "utf8",
+      });
+
+      assert.equal(result.status, 75);
+      const logLines = readFileSync(logPath, "utf8").trim().split("\n");
+      assert.equal(logLines.length, 1);
+      assert.equal(logLines[0].split(":")[1], "");
+    } finally {
+      rmSync(root, { recursive: true, force: true });
+    }
+  },
+);