fix(scripts): install-refresh removes pad.pid only when it is provably not a live sibling's (BUG-4068) (#2311)

* fix(scripts): install-refresh removes pad.pid only when it is provably not a live sibling's (BUG-4068)

The refresh removed <data dir>/pad.pid unconditionally after the install,
whether or not it stopped anything and whoever the file named. A second
server sharing the data dir lost its record, so `pad server stop` and
`pad server status` could no longer find it.

Now the file is removed when it names the pid this refresh stopped, or a pid
that is provably dead (kill -0 fails and ps finds no such process, so EPERM
is not read as dead). It is kept when it names any live process, when its
liveness is unknown, and when it does not parse as a pid record (JSON or the
legacy bare integer). A kept stale file is the safe failure: server stop
refuses a record it cannot prove (BUG-2969). The BUG-4064 tests, which
clear a dead-pid file with nothing stopped, stand unchanged.

Claude-Session: https://claude.ai/code/session_01Hk4vxsaKDm3E1LvsoHtHeV

* test(buildtools): pin install-refresh's pad.pid removal rule (BUG-4068)

Claude-Session: https://claude.ai/code/session_01Hk4vxsaKDm3E1LvsoHtHeV

* fix(scripts,buildtools): pad.pid is read no more loosely than pad writes it, and only ESRCH is dead (BUG-4068 r1)

r1 items, as ruled:
1, 3: pid_file_pid accepts only MarshalIndent's record (one "pid", at the
   two-space top-level indent) or a bare integer with surrounding whitespace,
   at most 10 digits. Duplicate or nested pid keys, compact JSON, "1 2" and
   huge values no longer parse, so the file is kept.
2: pid_liveness reads the builtin kill's own message under LC_ALL=C. Only
   "No such process" is dead; EPERM and anything else are unknown. ps is not
   asked, so a missing, busybox or hidepid-restricted ps cannot call a live
   sibling dead.
4: STOPPED_PID is recorded only when kill -TERM is delivered.
5: the decision is pid_file_removable, tested on its own, so the stopped-pid
   leg (a zombie or a quick reuse) has a row of its own.
6: the dead-pid row uses a reaped pid, not 999999.

Also: the harness guard's testTempScope cuts the test name to 64 bytes, as
Go's makeTempDir does. A top-level test name of 64 bytes or more was refused
by its own temp dir; this unit's long test names found it.

Claude-Session: https://claude.ai/code/session_01Hk4vxsaKDm3E1LvsoHtHeV

* fix(scripts): pad.pid's reader checks the whole record's line shape and caps the pid (BUG-4068 r2)

r2 items, as ruled:
1: pid_file_pid now checks the record's line SHAPE end to end, not only the
   pid line: first non-blank line exactly "{", last exactly "}", every line
   between a two-space-indent key line, one of them the pid. A "\r" is
   dropped only at a line end. This is still a shape check, not full JSON
   validation. The r1 commit's "accepts only MarshalIndent's record" said
   more than r1 did; this commit is what makes the shape claim true.
2: a pid above 4194304 (Linux's largest pid_max) does not parse, so it can
   never wrap as a pid_t.
3: pid_liveness states that ESRCH is per pid namespace.
5: the stopped-pid test comment is corrected: the e2e pre-server is a zombie
   until cleanup reaps it, so the e2e stopped-server rows reach that leg too.

Claude-Session: https://claude.ai/code/session_01Hk4vxsaKDm3E1LvsoHtHeV

* docs(scripts,buildtools): pid_file_pid's comments state the inner-line check exactly (BUG-4068 r3)

Comment-only. pid_file_pid checks that each line between the braces STARTS
like a two-space-indent quoted key; the rest of such a line is unchecked.
The comments said "every line between a two-space-indent key line", and
"read no more loosely than the SHAPE", which claimed more than the pattern
checks (r3, ruled a decline: the code stays as it is after the close).

Claude-Session: https://claude.ai/code/session_01Hk4vxsaKDm3E1LvsoHtHeV

internal/buildtools/bug4066_harness_guard_test.go

@@ -60,14 +60,18 @@ func testTempBase(t *testing.T) string {
 }
 
 // tempScope is where THIS test's temp dirs live: the shared base and the
-// test's top-level name. t.TempDir() names its dirs "<Name><random>", and a
-// subtest's "<Parent>_<sub><random>", so the top-level name is a prefix of
-// both and of no other test's (BUG-4066 r2).
+// test's top-level name. t.TempDir() names its dir for the FULL test name cut
+// to 64 bytes, symbols such as "/" dropped, then random digits (Go's
+// testing.go makeTempDir). So a subtest's is "<Parent><sub…>" and digits, and
+// a top-level name of 64 bytes or more appears only as its first 64. top is
+// cut the same way, or a long test name is refused by its own temp dir
+// (BUG-4068, found when its tests first ran under this guard).
 type tempScope struct{ base, top string }
 
 func testTempScope(t *testing.T) tempScope {
 	t.Helper()
-	return tempScope{testTempBase(t), strings.SplitN(t.Name(), "/", 2)[0]}
+	top := strings.SplitN(t.Name(), "/", 2)[0]
+	return tempScope{testTempBase(t), top[:min(len(top), 64)]}
 }
 
 // insideTestTempDir reports whether p is inside this test's temp dir: under

internal/buildtools/bug4068_pidfile_test.go

@@ -0,0 +1,262 @@
+package buildtools
+
+import (
+	"fmt"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strconv"
+	"strings"
+	"testing"
+)
+
+// BUG-4068: section 4 of install-refresh.sh removes <data dir>/pad.pid only
+// when it names the pid this refresh STOPPED or a pid that is provably DEAD.
+// It keeps the file when it names any live process (a sibling server sharing
+// the data dir, or a reused pid), when liveness is unknown (EPERM), and when
+// it does not parse. The testdata stub never writes pad.pid itself, so after a
+// refresh the file is either the fixture's bytes or absent.
+
+type pidFileCase struct {
+	// body builds the pad.pid content from the stopped stub's pid.
+	body func(stopped int) string
+	// server starts a stub on the port first, so the refresh stops it.
+	server bool
+}
+
+func jsonRecord(pid int, exe string) string {
+	return fmt.Sprintf("{\n  \"pid\": %d,\n  \"started_at\": \"2026-10-25T00:00:00Z\",\n  \"exe\": %q\n}\n", pid, exe)
+}
+
+// liveSibling starts a long-lived process this test owns, unrelated to the
+// stub, and returns its pid.
+func liveSibling(t *testing.T) int {
+	t.Helper()
+	cmd := exec.Command("sleep", "600")
+	if err := cmd.Start(); err != nil {
+		t.Fatalf("start sibling: %v", err)
+	}
+	t.Cleanup(func() { _ = cmd.Process.Kill(); _ = cmd.Wait() })
+	return cmd.Process.Pid
+}
+
+// runPidFileRefresh runs the real script against a stub on the test's own
+// port and returns the pad.pid path and the fixture bytes written to it.
+func runPidFileRefresh(t *testing.T, c pidFileCase) (pidFile, written string) {
+	t.Helper()
+	requireScriptDeps(t)
+	home, dir := t.TempDir(), t.TempDir()
+	name := uniqueName(t)
+	defer killStub(t, name)
+	port := freePort(t)
+	const commit = "abc1234"
+
+	built := installStub(t, dir, name)
+	installed := filepath.Join(home, "bin", name)
+	e := stubEnv{version: "pad version dev (" + commit + " x)", healthy: "1",
+		argvLog: filepath.Join(home, "argv.log"), port: port, home: home,
+		dataDir: filepath.Join(t.TempDir(), "data")}
+	if err := os.MkdirAll(e.dataDir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	stopped := 0
+	if c.server {
+		pre := exec.Command(built, "server", "start", "--host", "127.0.0.1")
+		pre.Env = e.env()
+		startPreServer(t, pre, "127.0.0.1", port)
+		stopped = pre.Process.Pid
+	}
+	pidFile = filepath.Join(e.dataDir, "pad.pid")
+	written = c.body(stopped)
+	if err := os.WriteFile(pidFile, []byte(written), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	res := runScript(t, e, built, installed, commit)
+	if res.err != nil {
+		t.Fatalf("refresh failed: %v\nstdout=%s\nstderr=%s", res.err, res.stdout, res.stderr)
+	}
+	return pidFile, written
+}
+
+func wantRemoved(t *testing.T, pidFile string) {
+	t.Helper()
+	if b, err := os.ReadFile(pidFile); err == nil {
+		t.Errorf("pad.pid survived the refresh with content %q", b)
+	} else if !os.IsNotExist(err) {
+		t.Errorf("stat pad.pid: %v", err)
+	}
+}
+
+func wantKept(t *testing.T, pidFile, written string) {
+	t.Helper()
+	b, err := os.ReadFile(pidFile)
+	if err != nil {
+		t.Fatalf("pad.pid was removed (%v); it must be kept byte-identical", err)
+	}
+	if string(b) != written {
+		t.Errorf("pad.pid changed: got %q, want %q", b, written)
+	}
+}
+
+func TestInstallRefresh_PIDFile_RemovesAJSONRecordNamingTheStoppedServer(t *testing.T) {
+	requireScriptDeps(t)
+	f, _ := runPidFileRefresh(t, pidFileCase{server: true, body: func(p int) string { return jsonRecord(p, "/x/pad") }})
+	wantRemoved(t, f)
+}
+
+func TestInstallRefresh_PIDFile_KeepsARecordNamingALiveSiblingWhileTheServerIsStopped(t *testing.T) {
+	requireScriptDeps(t)
+	sib := liveSibling(t)
+	f, w := runPidFileRefresh(t, pidFileCase{server: true, body: func(int) string { return jsonRecord(sib, "/x/pad") }})
+	wantKept(t, f, w)
+}
+
+func TestInstallRefresh_PIDFile_RemovesALegacyBareIntegerNamingTheStoppedServer(t *testing.T) {
+	requireScriptDeps(t)
+	f, _ := runPidFileRefresh(t, pidFileCase{server: true, body: func(p int) string { return strconv.Itoa(p) + "\n" }})
+	wantRemoved(t, f)
+}
+
+func TestInstallRefresh_PIDFile_RemovesADeadPidWhenNothingWasStopped(t *testing.T) {
+	requireScriptDeps(t)
+	dead := reapedPid(t)
+	f, _ := runPidFileRefresh(t, pidFileCase{body: func(int) string { return jsonRecord(dead, "/x/pad") }})
+	wantRemoved(t, f)
+}
+
+func TestInstallRefresh_PIDFile_KeepsALiveSiblingWhenNothingWasStopped(t *testing.T) {
+	requireScriptDeps(t)
+	sib := liveSibling(t)
+	f, w := runPidFileRefresh(t, pidFileCase{body: func(int) string { return jsonRecord(sib, "/x/pad") }})
+	wantKept(t, f, w)
+}
+
+func TestInstallRefresh_PIDFile_KeepsAFileThatDoesNotParse(t *testing.T) {
+	requireScriptDeps(t)
+	f, w := runPidFileRefresh(t, pidFileCase{body: func(int) string { return "garbage\n" }})
+	wantKept(t, f, w)
+}
+
+func TestInstallRefresh_PIDFile_RemovesTheStoppedPidEvenWhenTheExeDiffersBecauseTheStopProvedTheProcess(t *testing.T) {
+	requireScriptDeps(t)
+	f, _ := runPidFileRefresh(t, pidFileCase{server: true, body: func(p int) string { return jsonRecord(p, "/somewhere/else/pad") }})
+	wantRemoved(t, f)
+}
+
+// scriptFns runs prog after the script's own numeric, pid_file_pid,
+// pid_liveness and pid_file_removable, extracted with sed: the rest of the
+// script never runs. pre is bash placed between the functions and prog, used
+// to stand in a fake kill for the builtin.
+func scriptFns(t *testing.T, pre, prog string) string {
+	t.Helper()
+	requireScriptDeps(t)
+	script := scriptPath(t)
+	var fns strings.Builder
+	for _, expr := range []string{`/^numeric() /p`, `/^pid_file_pid() {/,/^}/p`, `/^pid_liveness() {/,/^}/p`, `/^pid_file_removable() {/,/^}/p`} {
+		out, err := exec.Command("sed", "-n", expr, script).Output()
+		if err != nil || len(out) == 0 {
+			t.Fatalf("extract %q from the script: %v", expr, err)
+		}
+		fns.Write(out)
+	}
+	cmd := exec.Command("bash", "-c", fns.String()+"\n"+pre+"\n"+prog)
+	cmd.Env = os.Environ()
+	out, err := cmd.Output()
+	if err != nil {
+		t.Fatalf("run extracted functions: %v", err)
+	}
+	return strings.TrimSpace(string(out))
+}
+
+// reapedPid is a pid that existed and is gone now.
+func reapedPid(t *testing.T) int {
+	t.Helper()
+	cmd := exec.Command("true")
+	if err := cmd.Run(); err != nil {
+		t.Fatal(err)
+	}
+	return cmd.Process.Pid
+}
+
+// The reader checks lines against the SHAPE internal/cli/pidfile.go writes
+// (BUG-4068 r1 items 1 and 3, r2 items 1 and 2): braces on their own lines,
+// each inner line STARTING like a two-space-indent quoted key (the rest of
+// the line is unchecked), one pid, at most 4194304; or a bare integer. It is a
+// line-shape check, not full JSON validation. The REFUSED rows do not parse,
+// and the file is kept.
+func TestInstallRefresh_PIDFile_ReadsOnlyWhatPadWrites(t *testing.T) {
+	for _, tc := range []struct{ name, body, want string }{
+		{"MarshalIndent", jsonRecord(4242, "/x/pad"), "4242"},
+		{"CRLF", strings.ReplaceAll(jsonRecord(4242, "/x/pad"), "\n", "\r\n"), "4242"},
+		{"bare", " 777 \n", "777"},
+		{"bare leading zeros", "007\n", "7"},
+		{"duplicate pid keys", "{\n  \"pid\": 5,\n  \"pid\": 6\n}\n", "REFUSED"},
+		{"nested pid", "{\n  \"x\": {\n    \"pid\": 5\n  }\n}\n", "REFUSED"},
+		{"compact one-line JSON is not what pad writes", "{\"pid\": 4242}\n", "REFUSED"},
+		{"inner whitespace", "1 2\n", "REFUSED"},
+		{"more than 10 digits", "12345678901\n", "REFUSED"},
+		{"zero", "0\n", "REFUSED"},
+		{"negative", "-5\n", "REFUSED"},
+		{"garbage", "garbage\n", "REFUSED"},
+		{"missing closing brace", "{\n  \"pid\": 5,\n garbage\n", "REFUSED"},
+		{"garbage after the pid line", "{\n  \"pid\": 5,\ngarbage\n}\n", "REFUSED"},
+		{"a CR inside a bare pid", "1\r2\n", "REFUSED"},
+		{"Linux's largest pid_max", "4194304\n", "4194304"},
+		{"above any pid_max", "4194305\n", "REFUSED"},
+		{"would wrap as a pid_t", "2147483648\n", "REFUSED"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			f := filepath.Join(t.TempDir(), "pad.pid")
+			if err := os.WriteFile(f, []byte(tc.body), 0o644); err != nil {
+				t.Fatal(err)
+			}
+			got := scriptFns(t, "", fmt.Sprintf("pid_file_pid %q || printf REFUSED", f))
+			if got != tc.want {
+				t.Errorf("pid_file_pid(%q) = %q, want %q", tc.body, got, tc.want)
+			}
+		})
+	}
+}
+
+// DEAD is ESRCH only (BUG-4068 r1 item 2): a fake kill prints each message
+// the builtin can, so EPERM and anything unrecognised are not dead.
+func TestInstallRefresh_PIDFile_OnlyESRCHIsDead(t *testing.T) {
+	for _, tc := range []struct{ name, msg, want string }{
+		{"EPERM", "kill: (5) - Operation not permitted", "unknown"},
+		{"ESRCH", "kill: (5) - No such process", "dead"},
+		{"unrecognised", "kill: something else", "unknown"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			fake := fmt.Sprintf("kill() { echo %q >&2; return 1; }", tc.msg)
+			if got := scriptFns(t, fake, "pid_liveness 5"); got != tc.want {
+				t.Errorf("pid_liveness with %q = %q, want %q", tc.msg, got, tc.want)
+			}
+		})
+	}
+	if got := scriptFns(t, "", fmt.Sprintf("pid_liveness %d", reapedPid(t))); got != "dead" {
+		t.Errorf("the real kill on a reaped pid: %q, want dead", got)
+	}
+	if got := scriptFns(t, "", fmt.Sprintf("pid_liveness %d", os.Getpid())); got != "alive" {
+		t.Errorf("the real kill on this test's own pid: %q, want alive", got)
+	}
+}
+
+// The stopped-pid leg on its own (BUG-4068 r1 item 5). The e2e stopped-server
+// rows reach it too: their pre-server is this test's own child, a zombie
+// after the TERM until cleanup reaps it, and kill -0 on a zombie succeeds, so
+// the dead leg cannot answer for them. This row does not depend on reaping: a
+// LIVE pid named as the stopped pid is removed, and with no stopped pid the
+// same file is kept.
+func TestInstallRefresh_PIDFile_TheStoppedPidLegRemovesEvenALivePid(t *testing.T) {
+	sib := liveSibling(t)
+	f := filepath.Join(t.TempDir(), "pad.pid")
+	if err := os.WriteFile(f, []byte(jsonRecord(sib, "/x/pad")), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if got := scriptFns(t, "", fmt.Sprintf("pid_file_removable %q %d && echo REMOVE", f, sib)); got != "REMOVE" {
+		t.Errorf("a file naming the stopped pid %d was not removable: %q", sib, got)
+	}
+	if got := scriptFns(t, "", fmt.Sprintf("pid_file_removable %q '' && echo REMOVE; true", f)); !strings.Contains(got, "is alive") {
+		t.Errorf("with no stopped pid, a live pid's file was not kept as alive: %q", got)
+	}
+}

scripts/install-refresh.sh

@@ -397,6 +397,7 @@ listener_pids() {
 }
 
 TARGET_PID=""
+STOPPED_PID=""
 LISTENERS=()
 HIDDEN=""
 while read -r lp; do
@@ -710,7 +711,13 @@ if [ -n "$TARGET_PID" ]; then
 			die "pid $TARGET_PID no longer runs the captured server.
   Nothing was stopped or installed; re-run."
 		fi
-		kill -TERM "$TARGET_PID" 2>/dev/null || true
+		# The pid this refresh signalled: the only pid whose pad.pid record
+		# section 4 may treat as its own (BUG-4068). Recorded only when the
+		# signal was delivered, so a pid that exited and was reused after
+		# still_target is never claimed.
+		if kill -TERM "$TARGET_PID" 2>/dev/null; then
+			STOPPED_PID="$TARGET_PID"
+		fi
 		for _ in 1 2 3 4 5; do
 			kill -0 "$TARGET_PID" 2>/dev/null || break
 			sleep 1
@@ -727,7 +734,104 @@ STAGED=""
 # cfg.PIDFile() is <data dir>/pad.pid (config.go), claimed by `pad server
 # start` (cmd/pad/cmd_server.go); not $HOME/.pad under PAD_DATA_DIR (BUG-4064).
 # DATA_DIR is section 2a's, anchored like the log.
-rm -f "$DATA_DIR/pad.pid"
+#
+# The file is removed only when it is provably not someone else's (BUG-4068):
+# it names the pid this refresh stopped, or a pid that is provably DEAD. It is
+# kept when it names any live process, a sibling server sharing the data dir
+# or a reused pid, and when its liveness is unknown (EPERM) or it does not
+# parse. A kept stale file is the safe failure: `pad server stop` refuses a
+# record it cannot prove (BUG-2969).
+pid_file_pid() {
+	# The pid in pad.pid, read by checking each line against the SHAPE that
+	# internal/cli/pidfile.go writes (BUG-4068 r1, r2). This is a line-shape
+	# check, not full JSON validation:
+	#   - the MarshalIndent record: first non-blank line exactly "{", last
+	#     exactly "}", and each line between must START like a two-space-indent
+	#     quoted key ('  "' and a key character); the rest of such a line is
+	#     unchecked. Exactly one of them is "  \"pid\": N" (with or without a
+	#     trailing comma), and "pid" appears nowhere else in the file;
+	#   - or the legacy bare integer: one non-blank line, surrounding space only.
+	# A "\r" is dropped only at a line end. The pid is at most 10 digits,
+	# checked before any arithmetic, and at most 4194304 (Linux's largest
+	# pid_max; macOS's is 99999), so it never wraps as a pid_t. Anything else
+	# does not parse, and the file is kept.
+	local raw line pid="" n=0 lines=()
+	raw="$(cat "$1" 2>/dev/null)" || return 1
+	while IFS= read -r line || [ -n "$line" ]; do
+		line="${line%$'\r'}"
+		case "$line" in *[![:space:]]*) lines+=("$line") ;; esac
+	done <<<"$raw"
+	[ ${#lines[@]} -gt 0 ] || return 1
+	case "$raw" in *'"pid"'*'"pid"'*) return 1 ;; esac
+	if [ "${lines[0]}" = "{" ]; then
+		[ ${#lines[@]} -ge 3 ] && [ "${lines[${#lines[@]} - 1]}" = "}" ] || return 1
+		local i
+		for ((i = 1; i < ${#lines[@]} - 1; i++)); do
+			line="${lines[i]}"
+			case "$line" in
+			'  "pid": '*)
+				pid="${line#'  "pid": '}"
+				pid="${pid%,}"
+				n=$((n + 1))
+				;;
+			'  "'[!\ \"]*) ;;
+			*) return 1 ;;
+			esac
+		done
+		[ "$n" -eq 1 ] || return 1
+	else
+		[ ${#lines[@]} -eq 1 ] || return 1
+		pid="${lines[0]}"
+		while case "$pid" in [[:space:]]*) true ;; *) false ;; esac; do pid="${pid#?}"; done
+		while case "$pid" in *[[:space:]]) true ;; *) false ;; esac; do pid="${pid%?}"; done
+	fi
+	numeric "$pid" && [ ${#pid} -le 10 ] || return 1
+	pid=$((10#$pid))
+	[ "$pid" -gt 0 ] && [ "$pid" -le 4194304 ] || return 1
+	printf '%s' "$pid"
+}
+pid_liveness() {
+	# alive, dead or unknown. DEAD is ESRCH only: kill -0 also fails with EPERM
+	# for a live process this user may not signal (BUG-4068 r1), so the
+	# builtin's own message is read under LC_ALL=C. ps is not asked: a missing,
+	# busybox or hidepid-restricted ps would read a live process as gone.
+	# ESRCH is per pid namespace: a sibling in another container that shares
+	# the data dir reads as dead here (ruled inherent, f8009dad).
+	local out
+	if out="$(LC_ALL=C kill -0 "$1" 2>&1)"; then
+		echo alive
+		return
+	fi
+	case "$out" in
+	*'No such process'*) echo dead ;;
+	*) echo unknown ;;
+	esac
+}
+pid_file_removable() {
+	# pid_file_removable <pad.pid> <stopped pid, or empty>: succeeds when the
+	# file is provably not a live sibling's, the pid this refresh stopped or a
+	# dead one. Otherwise it prints why the file is kept, and fails.
+	local file_pid
+	if ! file_pid="$(pid_file_pid "$1")"; then
+		echo "it does not parse as a pid record"
+		return 1
+	fi
+	[ -n "$2" ] && [ "$file_pid" = "$2" ] && return 0
+	case "$(pid_liveness "$file_pid")" in
+	dead) return 0 ;;
+	alive) echo "it names pid $file_pid, which is alive and is not the server this refresh stopped" ;;
+	*) echo "it names pid $file_pid, whose liveness is unknown" ;;
+	esac
+	return 1
+}
+pid_file="$DATA_DIR/pad.pid"
+if [ -e "$pid_file" ]; then
+	if kept_why="$(pid_file_removable "$pid_file" "$STOPPED_PID")"; then
+		rm -f "$pid_file"
+	else
+		note "kept $pid_file: $kept_why"
+	fi
+fi
 
 # The check TASK-2787 asks for, and it is deliberately on the installed file
 # rather than on any step: read what is now at the install path and require it