[rafaelsteil]

Summary

• Fix --background so the parent process exits after printing server info, while the detached child keeps running
• Replace stdout pipe handshake with Node IPC (process.send / child.on('message')) so the parent can exit without SIGPIPE killing the detached server
• Disconnect IPC after the handshake and ignore stdout/stderr errors on the background child so a closed parent terminal does not tear down the daemon

Root cause

startBackgroundProcess() spawned the detached child with piped stdout/stderr and waited for the server-ready JSON on child.stdout. The parent printed that line and called child.unref(), but the pipes stayed open until the parent exited. When the parent process ended, the broken stdout pipe could deliver SIGPIPE to the still-running child and kill the background server.

The background child also continued to use stdout for the handshake, so it remained coupled to the parent's lifecycle even though the process was meant to be detached.

What changed

src/cli/index.ts

• Spawn background child with stdio: ['ignore', 'ignore', 'ignore', 'ipc'] instead of piped stdout/stderr
• Child sends { port, url, pid } via process.send() and disconnects IPC after the handshake
• Parent waits on child.on('message'), prints JSON.stringify(handshake) to its own stdout, then disconnects and unrefs the child
• Background child registers no-op handlers for stdout/stderr error events after handshake so closed stdio does not crash the daemon

Reproduction (before this fix)

difit HEAD --background --no-open
# Parent hung or background server died shortly after startup when the parent exited

Test plan

• difit HEAD --background --no-open prints one JSON line (port, url, pid) and the parent exits immediately
• Background server keeps responding after the parent exits: curl http://localhost:<port>/api/diff → 200
• difit HEAD --background --no-open twice on the same port behavior still works (or port retry still works when busy)
• Normal foreground difit HEAD still opens the browser and blocks as before
• pnpm check / pnpm test / pnpm build pass

Manual validation

These checks compare the reviewed PR head (33a17c58c4a18ad725790f3774e46785e796d34b) with the current working tree. They use a detached Git worktree for the old code, so the current branch does not need to be changed.

1. Prepare the reviewed version

Run from the difit repository:

git worktree add --detach /tmp/difit-pr451-before 33a17c58c4a18ad725790f3774e46785e796d34b
cd /tmp/difit-pr451-before
pnpm install --offline --frozen-lockfile --ignore-scripts
pnpm build:cli

2. Reproduce the disconnected-IPC crash

From /tmp/difit-pr451-before:

node dist/cli/index.js definitely-not-a-valid-ref --background --no-open

Expected before the fix:

• The command exits with code 1.
• It prints an unhandled ERR_IPC_DISCONNECTED stack trace.
• The stack includes releaseBackgroundChild and child.disconnect().

Validate the current fix:

cd /home/rafael/dev/difit
pnpm build:cli
node dist/cli/index.js definitely-not-a-valid-ref --background --no-open

Expected after the fix:

• The command exits with code 1.
• It reports Invalid or non-existent commit: definitely-not-a-valid-ref.
• It does not print ERR_IPC_DISCONNECTED, Unhandled 'error' event, or a Node stack trace.

3. Verify that startup stderr is preserved

First show the diagnostic produced by the reviewed version's child:

cd /tmp/difit-pr451-before
DIFIT_BACKGROUND_CHILD=1 node dist/cli/index.js definitely-not-a-valid-ref --keep-alive --no-open

Expected:

Error: Invalid or non-existent commit: definitely-not-a-valid-ref

Now run the reviewed version through its background parent:

node dist/cli/index.js definitely-not-a-valid-ref --background --no-open

Expected before the fix:

• The useful invalid-ref diagnostic is not relayed by the parent.
• The disconnected-IPC stack trace masks the startup failure.

Run the equivalent command in the current working tree:

cd /home/rafael/dev/difit
node dist/cli/index.js definitely-not-a-valid-ref --background --no-open

Expected after the fix:

• The parent output contains the invalid-ref diagnostic.
• No disconnected-IPC stack trace is printed.

4. Reproduce and validate timeout cleanup

This is the hardest case to reproduce manually. The review concern is:

• Before: on handshake timeout, the parent calls releaseBackgroundChild() (disconnect + unref) instead of kill(), so a slow child can keep booting and hold the port.
• After: on timeout, the parent calls child.kill() so the detached server should not survive.

The preload below delays only the detached child (DIFIT_BACKGROUND_CHILD=1) for 12 seconds before difit starts. The parent times out at 10 seconds.

Create the delay preload once

cat >/tmp/difit-pr451-delay-child.cjs <<'EOF'
const fs = require('node:fs');

if (process.env.DIFIT_BACKGROUND_CHILD === '1') {
  fs.writeFileSync('/tmp/difit-pr451-child.pid', String(process.pid));
  Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 12_000);
}
EOF

Helper: watch port 4977 for 20 seconds

Run this in a second terminal before starting the background command:

for i in $(seq 1 20); do
  code="$(curl --silent --output /dev/null --write-out '%{http_code}' http://localhost:4977/api/diff 2>/dev/null || echo 000)"
  pid="$(cat /tmp/difit-pr451-child.pid 2>/dev/null || echo -)"
  alive="no"
  if [ -n "$pid" ] && kill -0 "$pid" 2>/dev/null; then alive="yes"; fi
  printf 't=%02ds pid=%s alive=%s curl=%s\n' "$i" "$pid" "$alive" "$code"
  sleep 1
done

Before the fix

In the first terminal:

cd /tmp/difit-pr451-before
rm -f /tmp/difit-pr451-child.pid
NODE_OPTIONS="--require /tmp/difit-pr451-delay-child.cjs" \
  node dist/cli/index.js HEAD --background --no-open --port 4977

Interpret the watcher output:

What you see	Meaning
Parent prints Timed out while starting background difit server after ~10s	Timeout path ran (good setup).
alive=yes for several seconds after t=10, then curl=200 around t=13–t=16	Orphan reproduced: child survived timeout and eventually served traffic. This matches the review concern.
alive=no soon after t=10 and curl=000 through t=20	Child did not orphan in this run. Timeout still happened, but you cannot confirm the orphan bug from this attempt alone.

If you saw the second row (like kill: No such process and curl 000 immediately after timeout), that means:

1. The timeout path did run.
2. The child PID was already gone (or never stayed alive long enough to bind port 4977).
3. That is not proof the orphan bug is absent in all environments — only that this preload setup did not leave a survivor on your machine.

The pre-fix code still does not call child.kill() on timeout; a slower or partially-started child can still orphan in other conditions.

Cleanup if anything is still running:

kill "$(cat /tmp/difit-pr451-child.pid 2>/dev/null)" 2>/dev/null || true

After the fix

Repeat with the watcher running, then in the first terminal:

cd /home/rafael/dev/difit
rm -f /tmp/difit-pr451-child.pid
NODE_OPTIONS="--require /tmp/difit-pr451-delay-child.cjs" \
  node dist/cli/index.js HEAD --background --no-open --port 4977

Expected after the fix:

• Parent still times out after ~10s.
• Within 1–2 seconds after timeout (t=11 or t=12), watcher shows alive=no.
• curl stays 000 for the full 20s window.

That confirms the timed-out child was killed instead of released to keep running.

5. Validate successful background startup

From the current working tree:

cd /home/rafael/dev/difit
SECONDS=0
server_json="$(node dist/cli/index.js HEAD --background --no-open --port 4978)"
elapsed="$SECONDS"
server_pid="$(node -e 'process.stdout.write(String(JSON.parse(process.argv[1]).pid))' "$server_json")"
server_url="$(node -e 'process.stdout.write(JSON.parse(process.argv[1]).url)' "$server_json")"
printf 'parent elapsed: %ss\nserver: %s\n' "$elapsed" "$server_json"
curl --fail --silent --output /dev/null --write-out '%{http_code}\n' \
  "$server_url/api/diff"
kill "$server_pid"

Expected after the fix:

• The parent exits promptly and prints one JSON object containing port, url, and pid.
• Curl returns 200 after the parent has exited.
• The final kill stops the detached server.

6. Cleanup

cd /home/rafael/dev/difit
git worktree remove /tmp/difit-pr451-before
rm -f /tmp/difit-pr451-delay-child.cjs /tmp/difit-pr451-child.pid

[yoshiko-pg]

Thanks for the fix — replacing the stdout handshake with IPC is the right direction. Found one reproducible crash and two smaller issues, inline below.

[yoshiko-pg]

Bug: child.disconnect() is called without a guard and crashes the parent with an unhandled 'error' event when the child dies before the handshake.

If the child exits early (e.g. an invalid commit-ish was passed), the IPC channel is already closed by the time the 'exit' event fires, so child.connected is false. Calling disconnect() in that state makes Node emit an 'error' event (ERR_IPC_DISCONNECTED) on the ChildProcess — and since removeAllListeners() has just stripped every listener, it becomes an uncaught exception. The parent crashes with a stack trace instead of printing the intended Background difit server exited early (code 1) message.

Reproduced on Node v24:

Error [ERR_IPC_DISCONNECTED]: IPC channel is already disconnected
    at target.disconnect (node:internal/child_process:926:26)

Suggested fix:

function releaseBackgroundChild(child: ChildProcess): void {
  child.removeAllListeners('message');
  child.removeAllListeners('error');
  child.removeAllListeners('exit');
  if (child.connected) {
    child.disconnect();
  }
  child.unref();
}

Note that a bare removeAllListeners() also strips Node's internal IPC listeners (internalMessage etc.), so removing only the events registered here is safer.

[yoshiko-pg]

Orphaned server on timeout: the timeout path releases the child (disconnect + unref) instead of killing it. The parent exits with Timed out while starting background difit server, but a slow-starting child will finish booting afterwards and — since --keep-alive is forced — keep running forever. The user sees a failure while an orphaned server silently holds a port.

The timeout path should child.kill() rather than release it.

[yoshiko-pg]

Lost startup diagnostics: with all stdio set to 'ignore', any startup error the child prints (invalid commit-ish, stdin read failure, etc.) goes to /dev/null. The previous implementation collected stderr and included it in the error message; now the parent can only report exited early (code 1) with no reason.

A way to keep both properties: leave stderr as 'pipe', collect it until the handshake, then child.stderr.destroy() on success — any later EPIPE after the parent exits is absorbed by the no-op 'error' handlers added in ignoreStdioErrorsForBackgroundDaemon(). Alternatively, send startup errors over the IPC channel.

Related: with the current all-'ignore' stdio config, the child's stdout/stderr point at /dev/null, so EPIPE can never happen and ignoreStdioErrorsForBackgroundDaemon() is effectively dead code (it becomes meaningful if stderr goes back to a pipe).

[yoshiko-pg]

Thanks for tracking this down, the IPC handshake approach is the right fix for the SIGPIPE issue. Two problems in the error paths though:

1. releaseBackgroundChild() calls child.disconnect() unconditionally after removeAllListeners(). When the child exits early (e.g. difit some-bad-ref --background), the IPC channel is already closed, so disconnect() emits an 'error' event on the child, and since all listeners were just removed, the parent crashes with an unhandled ERR_IPC_DISCONNECTED instead of the intended "Background difit server exited early" message. I reproduced this with a minimal script. Guarding with if (child.connected) child.disconnect(); fixes it.
2. Switching stderr to 'ignore' loses the actual error output the old code surfaced on early exit, so users only see "exited early (code 1)". Keeping stderr as 'pipe' just for the handshake window (and destroying it in releaseBackgroundChild) would preserve diagnostics; the no-op stderr error handler you added on the child side already covers the post-handshake EPIPE concern.

Minor: with all stdio ignored, ignoreStdioErrorsForBackgroundDaemon() is currently a no-op, and on handshake timeout the detached child now lingers forever, so a child.kill() there might be worth considering.

[rafaelsteil]

@yoshiko-pg I pushed a new commit with some fixes, and added a manual validation section to the pr's description. Step 4 I found it flaky to validate. If you have other steps to reproduce the bugs, please let me know so I can try them locally as well.

[yoshiko-pg]

@rafaelsteil Thank you!!