diff --git a/scripts/check-inbox.sh b/scripts/check-inbox.sh index 67642e7c..b0759002 100755 --- a/scripts/check-inbox.sh +++ b/scripts/check-inbox.sh @@ -75,7 +75,8 @@ if [ -n "$SESSION_ID" ]; then PIDFILE="$SKILL_DIR/run/watch.$SESSION_ID.pid" if [ -f "$PIDFILE" ]; then WATCH_PID=$(cat "$PIDFILE" 2>/dev/null || true) - if [ -n "$WATCH_PID" ] && kill -0 "$WATCH_PID" 2>/dev/null; then + # EPERM-aware liveness (_agmsg_pid_alive): a sandbox-unsignalable watcher is still alive. + if [ -n "$WATCH_PID" ] && _agmsg_pid_alive "$WATCH_PID"; then exit 0 fi fi diff --git a/scripts/delivery.sh b/scripts/delivery.sh index 9ae0e94d..d9f05ff3 100755 --- a/scripts/delivery.sh +++ b/scripts/delivery.sh @@ -284,7 +284,9 @@ emit_monitor_directive() { if [ -f "$pidfile" ]; then local existing existing=$(cat "$pidfile" 2>/dev/null || true) - if [ -n "$existing" ] && kill -0 "$existing" 2>/dev/null; then + # EPERM-aware liveness (_agmsg_pid_alive): a sandbox-unsignalable watcher is + # still alive, so we must not re-emit and spawn a duplicate. + if [ -n "$existing" ] && _agmsg_pid_alive "$existing"; then cat </dev/null || true) - if [ -n "$existing" ] && kill -0 "$existing" 2>/dev/null; then + # EPERM-aware liveness (_agmsg_pid_alive), mirroring delivery.sh emit dedup. + if [ -n "$existing" ] && _agmsg_pid_alive "$existing"; then cat </dev/null + # A non-zero `kill -0` is ESRCH (dead) or EPERM (alive but unsignalable under + # the sandbox). Only ESRCH is dead. `export LC_ALL=C` (not a bare prefix, which + # misses the builtin on bash 3.2) forces English error text for the match. + local err + err="$(export LC_ALL=C; kill -0 "$pid" 2>&1)" && return 0 + case "$err" in + *[Nn]'o such process'*) return 1 ;; + *) return 0 ;; + esac } # Compose from an explicit pid. Bare sid when pid is empty/non-numeric. diff --git a/scripts/lib/resolve-project.sh b/scripts/lib/resolve-project.sh index b539f709..0d1268a8 100644 --- a/scripts/lib/resolve-project.sh +++ b/scripts/lib/resolve-project.sh @@ -335,12 +335,15 @@ agmsg_read_project_marker() { agmsg_marker_gc_stale() { local dir; dir="$(_agmsg_run_dir)" [ -d "$dir" ] || return 0 + # Skip if _agmsg_pid_alive (EPERM-aware; instance-id.sh) isn't loaded, so a + # "command not found" can't fall through to `|| rm -f` and delete a live marker. + declare -F _agmsg_pid_alive >/dev/null 2>&1 || return 0 local f pid for f in "$dir"/proj.*.project; do [ -f "$f" ] || continue pid=${f##*/proj.}; pid=${pid%.project} case "$pid" in ''|*[!0-9]*) continue ;; esac - kill -0 "$pid" 2>/dev/null || rm -f "$f" + _agmsg_pid_alive "$pid" || rm -f "$f" done } diff --git a/scripts/session-end.sh b/scripts/session-end.sh index bc863630..e8ba2a26 100755 --- a/scripts/session-end.sh +++ b/scripts/session-end.sh @@ -55,7 +55,8 @@ INSTANCE_ID="$(agmsg_instance_id "$SESSION_ID" "$TYPE")" PIDFILE="$RUN_DIR/watch.$INSTANCE_ID.pid" if [ -f "$PIDFILE" ]; then pid=$(cat "$PIDFILE" 2>/dev/null || true) - if [ -n "$pid" ] && kill -0 "$pid" 2>/dev/null; then + # EPERM-aware liveness (_agmsg_pid_alive): don't skip cleaning a live watcher. + if [ -n "$pid" ] && _agmsg_pid_alive "$pid"; then # Defensive: only kill if the pid's command line still looks like our # watch.sh. Pids can be recycled — a stale pidfile could point at an # unrelated process that took the same pid. diff --git a/scripts/session-start.sh b/scripts/session-start.sh index 60c5aa81..44ebdb7d 100755 --- a/scripts/session-start.sh +++ b/scripts/session-start.sh @@ -145,7 +145,7 @@ for f in "$RUN_DIR"/cc-instance.*; do [ -f "$f" ] || continue pid=${f##*.} case "$pid" in ''|*[!0-9]*) continue ;; esac - if kill -0 "$pid" 2>/dev/null; then + if _agmsg_pid_alive "$pid"; then s=$(cat "$f" 2>/dev/null || true) [ -n "$s" ] && live_sids="$live_sids|$s" fi @@ -157,14 +157,14 @@ for f in "$RUN_DIR"/cc-instance.*; do [ -f "$f" ] || continue pid=${f##*.} case "$pid" in ''|*[!0-9]*) continue ;; esac - kill -0 "$pid" 2>/dev/null && continue + _agmsg_pid_alive "$pid" && continue dead_sid=$(cat "$f" 2>/dev/null || true) if [ -n "$dead_sid" ] \ && ! printf '%s\n' "$live_sids" | tr '|' '\n' | grep -Fxq "$dead_sid"; then orphan_pidfile="$RUN_DIR/watch.$dead_sid.pid" if [ -f "$orphan_pidfile" ]; then orphan_pid=$(cat "$orphan_pidfile" 2>/dev/null || true) - if [ -n "$orphan_pid" ] && kill -0 "$orphan_pid" 2>/dev/null; then + if [ -n "$orphan_pid" ] && _agmsg_pid_alive "$orphan_pid"; then # Defensive: only kill if the pid's command line actually matches # our watch.sh. Defends against pid recycling — a stale pidfile # could point at an unrelated process that took the same pid. @@ -191,7 +191,7 @@ for f in "$RUN_DIR"/watch.*.pid; do rm -f "$f" continue fi - kill -0 "$pid" 2>/dev/null || rm -f "$f" + _agmsg_pid_alive "$pid" || rm -f "$f" done # Garbage-collect actas exclusivity locks whose owner session_id no longer @@ -240,7 +240,7 @@ if [ -n "$CC_PID" ]; then prev_pidfile="$RUN_DIR/watch.$prev.pid" if [ -f "$prev_pidfile" ]; then prev_pid=$(cat "$prev_pidfile" 2>/dev/null || true) - if [ -n "$prev_pid" ] && kill -0 "$prev_pid" 2>/dev/null; then + if [ -n "$prev_pid" ] && _agmsg_pid_alive "$prev_pid"; then kill "$prev_pid" 2>/dev/null || true fi fi @@ -258,7 +258,7 @@ fi WATCHER_PIDFILE="$RUN_DIR/watch.$INSTANCE_ID.pid" if [ -f "$WATCHER_PIDFILE" ]; then existing=$(cat "$WATCHER_PIDFILE" 2>/dev/null || true) - if [ -n "$existing" ] && kill -0 "$existing" 2>/dev/null; then + if [ -n "$existing" ] && _agmsg_pid_alive "$existing"; then cat </dev/null || true # against pid recycling — only touch processes whose cmdline still matches # our watch.sh. See #66. # -# When ps is unavailable (e.g. Claude Code sandbox), fall back to kill -0 -# which confirms the pid is alive but cannot validate the cmdline. +# When ps is unavailable (e.g. Claude Code sandbox), fall back to _agmsg_pid_alive +# which confirms the pid is alive but cannot validate the cmdline. It is EPERM-aware +# so a live-but-unsignalable sibling watcher isn't misread as dead and left running. if [ -f "$PIDFILE" ]; then prev_pid=$(cat "$PIDFILE" 2>/dev/null || true) - if [ -n "$prev_pid" ] && [ "$prev_pid" != "$$" ] && kill -0 "$prev_pid" 2>/dev/null; then + if [ -n "$prev_pid" ] && [ "$prev_pid" != "$$" ] && _agmsg_pid_alive "$prev_pid"; then prev_cmd=$(compat_get_cmdline "$prev_pid" 2>/dev/null || true) if [ -n "$prev_cmd" ]; then case "$prev_cmd" in *"$SKILL_DIR/scripts/watch.sh"*) kill "$prev_pid" 2>/dev/null || true ;; esac else - # ps unavailable (sandboxed) — skip cmdline validation, rely on kill -0 + # ps unavailable (sandboxed) — skip cmdline validation, rely on the + # _agmsg_pid_alive check above kill "$prev_pid" 2>/dev/null || true fi fi diff --git a/tests/test_instance_id.bats b/tests/test_instance_id.bats index 0c54d538..8b976e92 100644 --- a/tests/test_instance_id.bats +++ b/tests/test_instance_id.bats @@ -130,6 +130,52 @@ teardown() { teardown_test_env; } ! agmsg_instance_alive "" } +# --- _agmsg_pid_alive: EPERM vs ESRCH (Claude Code sandbox) --- +# +# Under the sandbox `kill -0` on a live pid returns EPERM, not ESRCH. Only ESRCH +# is dead; EPERM must read as alive or the watcher self-exits. `kill` is stubbed +# to script each errno string (real EPERM is hard to force). + +@test "pid_alive: a real live pid (self) is alive" { + skip_on_windows "POSIX kill path; Windows uses tasklist (#134)" + _agmsg_pid_alive $$ +} + +@test "pid_alive: a real dead pid is not alive (ESRCH end-to-end)" { + skip_on_windows "POSIX kill path; Windows uses tasklist (#134)" + ! _agmsg_pid_alive 2147483647 +} + +@test "pid_alive: a signalable pid (kill -0 exit 0) is alive" { + skip_on_windows "POSIX kill path; Windows uses tasklist (#134)" + kill() { return 0; } + _agmsg_pid_alive 12345 +} + +@test "pid_alive: ESRCH 'No such process' reads as dead" { + skip_on_windows "POSIX kill path; Windows uses tasklist (#134)" + kill() { echo "bash: kill: (999) - No such process" >&2; return 1; } + ! _agmsg_pid_alive 999 +} + +@test "pid_alive: lowercase 'no such process' (zsh wording) also reads as dead" { + skip_on_windows "POSIX kill path; Windows uses tasklist (#134)" + kill() { echo "kill: (999) - no such process" >&2; return 1; } + ! _agmsg_pid_alive 999 +} + +@test "pid_alive: EPERM 'Operation not permitted' reads as alive (sandbox)" { + skip_on_windows "POSIX kill path; Windows uses tasklist (#134)" + kill() { echo "bash: kill: (1) - Operation not permitted" >&2; return 1; } + _agmsg_pid_alive 1 +} + +@test "pid_alive: an unrecognized kill failure defaults to alive (fail-safe)" { + skip_on_windows "POSIX kill path; Windows uses tasklist (#134)" + kill() { echo "kill: some novel platform error" >&2; return 1; } + _agmsg_pid_alive 42 +} + # --- agmsg_normalize_instance_id --- @test "normalize: a composite token passes through unchanged (idempotent)" { diff --git a/tests/test_resolve_project.bats b/tests/test_resolve_project.bats index f8f41c7d..e8fa7045 100644 --- a/tests/test_resolve_project.bats +++ b/tests/test_resolve_project.bats @@ -24,6 +24,10 @@ setup() { # shellcheck disable=SC1090 source "$SKILL_DIR/scripts/lib/resolve-project.sh" + # Real gc_stale callers load _agmsg_pid_alive via actas-lock.sh; mirror that so + # the GC exercises the real liveness path, not its missing-helper guard. + # shellcheck disable=SC1090 + source "$SKILL_DIR/scripts/lib/instance-id.sh" } teardown() { @@ -202,6 +206,40 @@ JSON [ -f "$(agmsg_project_marker_path "$$")" ] } +# EPERM-aware GC: under the sandbox `kill -0` on a live pid returns EPERM. Reading +# that as dead would delete a live session's marker; only ESRCH drops it. `kill` +# is stubbed to script each errno string (real EPERM is hard to force). + +@test "marker-gc: keeps a marker whose pid is EPERM-live (sandbox)" { + skip_on_windows "POSIX kill path; Windows uses tasklist (#134)" + agmsg_write_project_marker 4242 "$ROOT" + kill() { echo "bash: kill: (4242) - Operation not permitted" >&2; return 1; } + agmsg_marker_gc_stale + [ -f "$(agmsg_project_marker_path 4242)" ] +} + +@test "marker-gc: drops a marker whose pid is ESRCH-dead" { + skip_on_windows "POSIX kill path; Windows uses tasklist (#134)" + agmsg_write_project_marker 4242 "$ROOT" + kill() { echo "bash: kill: (4242) - No such process" >&2; return 1; } + agmsg_marker_gc_stale + [ ! -f "$(agmsg_project_marker_path 4242)" ] +} + +@test "marker-gc: skips (keeps marker) when _agmsg_pid_alive is unavailable" { + # Guard: without the helper, GC must skip rather than `|| rm -f` a live marker. + # Isolated shell sources ONLY resolve-project.sh, so the helper is truly absent. + agmsg_write_project_marker 4242 "$ROOT" + run bash -c ' + export SKILL_DIR="'"$SKILL_DIR"'" + source "$SKILL_DIR/scripts/lib/resolve-project.sh" + declare -F _agmsg_pid_alive >/dev/null && { echo "helper unexpectedly present"; exit 2; } + agmsg_marker_gc_stale + ' + [ "$status" -eq 0 ] + [ -f "$(agmsg_project_marker_path 4242)" ] +} + # --- pid-recycling guard --- @test "pid-is-agent: a live non-agent process is not trusted" {