From deccd306d67eb6aa07a0969e237d7ef80d7796cf Mon Sep 17 00:00:00 2001
From: Eli Mallon <eli@iame.li>
Date: Sat, 19 Sep 2026 17:55:39 -0700
Subject: [PATCH] e2e: stop leaking the harness after a successful run
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

hack/e2e-web-local.sh ended with `exec pnpm exec playwright test`, which
replaces the shell and discards the EXIT trap — so every *green* run left
the harness, its dev-env node and the forked server node running, holding
ports and /tmp data dirs that look like the run you are debugging.

Start the harness in its own process group (setsid) and kill the group on
exit, then sweep any binary from this build dir that appeared during the
run: the ingest worker re-setsid's itself and escapes the group kill. PIDs
that already existed before the run are left alone, so a scratch node from
`make dev` is not collateral damage.

Verified: two consecutive green runs (4 passed, 36.5s), no live
libstreamplace or dev-env process afterwards (only unreapable zombies
under the container's `tail -f /dev/null` PID 1).
---
 AGENTS.md             | 12 ++++++++----
 hack/e2e-web-local.sh | 28 +++++++++++++++++++++++-----
 2 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/AGENTS.md b/AGENTS.md
index fba1e17f..53d938ee 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -304,14 +304,18 @@ hatch, the explicit `getLiveUsers` limit and the current sidebar labels.
 - `place.stream.live.getLiveUsers` with no `limit` answers `{}` no matter
   how live the stream is: the handler truncates the streamer list to `limit`,
   so zero means zero. Pass `?limit=50`.
-- A killed harness leaves orphaned node/PDS processes behind. They hold
-  ports and their data dirs under `/tmp` look like the run you are
+- A harness you kill by hand leaves orphaned node/PDS processes behind.
+  They hold ports and their data dirs under `/tmp` look like the run you are
   debugging, so sweep them before re-running:
   `pkill -f 'libstreamplace e2e'; pkill -f 'js/dev-env/run.mjs'`. If you run
   that from a wrapper whose own command line contains the pattern (e.g.
   `docker exec … bash -c "pkill -f 'libstreamplace e2e'"`) it matches and
-  kills itself; bracket a character (`'libstreamplace e2[o]e'`) to exclude
-  the wrapper.
+  kills itself; bracket a character that is in the target
+  (`'libstreamplace e2[e]'`) to exclude the wrapper. The harness's ingest
+  worker re-`setsid`s itself, so it outlives a plain top-level kill — kill it
+  by name too (`pkill -f 'libstreamplace ingest-worker'`), or just let
+  `hack/e2e-web-local.sh` clean up after itself (it sweeps the binaries that
+  appeared during the run, leaving any pre-existing scratch node alone).
 - `js/dev-env` needs Node 22 (better-sqlite3 pin).
 
 ## 6. Git and GitHub _(unverified)_
diff --git a/hack/e2e-web-local.sh b/hack/e2e-web-local.sh
index 6931b71e..1923abf8 100755
--- a/hack/e2e-web-local.sh
+++ b/hack/e2e-web-local.sh
@@ -40,14 +40,30 @@ fi
 ENVFILE="$(mktemp)"
 LOGFILE="$(mktemp)"
 echo "starting e2e harness…"
-"$BUILDDIR/streamplace" e2e > "$ENVFILE" 2> "$LOGFILE" &
+# `streamplace e2e` forks a dev-env node, the server node and an ingest
+# worker; run it in its own process group (setsid) so cleanup can take the
+# whole tree down instead of orphaning the grandchildren.
+#
+# PIDs matching this checkout's binary before we start — never kill a scratch
+# node someone else is running from the same build dir.
+PREEXISTING=" $( { pgrep -f "$BUILDDIR/libstreamplace" 2>/dev/null || true; } | tr '\n' ' ') "
+if command -v setsid >/dev/null 2>&1; then
+  setsid "$BUILDDIR/streamplace" e2e > "$ENVFILE" 2> "$LOGFILE" &
+else
+  "$BUILDDIR/streamplace" e2e > "$ENVFILE" 2> "$LOGFILE" &
+fi
 HARNESS_PID=$!
 cleanup() {
-  kill "$HARNESS_PID" 2>/dev/null || true
-  pkill -P "$HARNESS_PID" 2>/dev/null || true
+  kill -- -"$HARNESS_PID" 2>/dev/null || kill "$HARNESS_PID" 2>/dev/null || true
+  wait "$HARNESS_PID" 2>/dev/null || true
+  # The ingest worker re-setsid's itself, escaping the group kill; sweep the
+  # binaries that appeared since we started.
+  for pid in $(pgrep -f "$BUILDDIR/libstreamplace" 2>/dev/null || true); do
+    case "$PREEXISTING" in *" $pid "*) ;; *) kill "$pid" 2>/dev/null || true ;; esac
+  done
   rm -f "$ENVFILE" "$LOGFILE"
 }
-trap cleanup EXIT
+trap cleanup EXIT INT TERM
 
 for _ in $(seq 1 90); do grep -q SERVER_URL "$ENVFILE" && break; sleep 2; done
 if ! grep -q SERVER_URL "$ENVFILE"; then
@@ -59,5 +75,7 @@ export SERVER_URL ACCOUNT_HANDLE ACCOUNT_DID
 echo "harness up: SERVER_URL=$SERVER_URL ACCOUNT_HANDLE=$ACCOUNT_HANDLE"
 
 # --- run the flows ---------------------------------------------------------
+# No `exec` here: that would replace this shell and discard the EXIT trap,
+# leaving the harness and its children running after a successful run.
 cd "$REPO/js/e2e-web"
-exec pnpm exec playwright test "$@"
+pnpm exec playwright test "$@"
-- 
2.51.2