Fix: pid-less lock silently aborts the run (v0.4.0 release blocker)

CHANGELOG.md

@@ -27,7 +27,9 @@ dead on the first broken app and said almost nothing about the rest.
   (`.dockerdance.lock`), so another local user can no longer pre-create the
   predictable name and permanently lock the tool. It records the owning pid,
   and a lock whose process is gone clears itself instead of demanding manual
-  removal after a crash or reboot.
+  removal after a crash or reboot. Liveness is checked with `ps` rather than
+  `kill -0`, which cannot see another user's process and would otherwise
+  declare their running job dead and take the lock from under it.
 - **Webhook payloads escape their content.** An app name or error message
   containing a quote, backslash, tab or newline no longer breaks (or
   rewrites) the JSON sent to `NOTIFY_WEBHOOK`.
@@ -56,8 +58,9 @@ dead on the first broken app and said almost nothing about the rest.
   many older archives exist.
 - **Releases carry a checksum, and `update-self` verifies it.** The release
   workflow publishes `manage.sh.sha256` beside the notes; `update-self`
-  checks the downloaded script against it before installing (releases
-  without one - everything before 0.4.1 - still install as before).
+  checks the downloaded script against it before installing (0.4.0 is the
+  first release to carry one; older releases without it still install as
+  before).
   `update-self` also now tells a GitHub rate-limit apart from being offline.
 - **A test suite.** `tests/run-tests.sh` drives `manage.sh` against a stub
   docker (no daemon needed) and asserts on exit codes, output and the exact
@@ -106,6 +109,11 @@ dead on the first broken app and said almost nothing about the rest.
 - A `PARALLEL_PULLS` of `0`, a negative or a typo left the pull orchestrator
   with no slot it could ever fill, hanging the run; it now falls back to the
   default of 3.
+- A lock directory with no readable pid inside - left by a run killed between
+  creating the lock and recording its owner, or by a full disk - made every
+  state-changing command exit 1 with no output whatsoever, and cut `doctor`'s
+  report off mid-way. Such a lock is now reported (and left alone, since
+  there is no owner to judge) instead of aborting the script.
 
 ## [0.3.0] - 2026-07-13
 

docker_volumes/manage.sh

@@ -226,24 +226,45 @@ run_step() {
   return 0
 }
 
+#Is process $1 still running? ps is asked first because it can see other
+#users' processes: kill -0 only reports "not permitted" for those, which
+#would make someone else's live run look dead and let us steal their lock.
+#Where ps has no -p (busybox) this falls back to kill -0.
+pid_alive() {
+  ps -p "$1" >/dev/null 2>&1 && return 0
+  kill -0 "$1" 2>/dev/null
+}
+
+#Read the pid recorded in the lock, or "" when there isn't a readable one.
+#The `|| true` matters: an unguarded assignment from a failing command
+#substitution aborts the whole script under set -e, and acquire_lock is
+#called as the last command of an AND-OR list, where errexit still applies.
+lock_pid() {
+  lp=$(cat "$LOCK_DIR/pid" 2>/dev/null) || lp=""
+  printf '%s' "$lp"
+}
+
 acquire_lock() {
   [ -n "$HAVE_LOCK" ] && return 0
   if ! mkdir "$LOCK_DIR" 2>/dev/null; then
     #A lock whose recorded process is gone is left over from a crash or a
-    #reboot - clear it and try once more. (kill -0 can't tell a foreign
-    #user's live process from a dead one, but the lock sits inside this
-    #user's own docker_volumes folder, so that ambiguity doesn't arise.)
-    al_pid=$(cat "$LOCK_DIR/pid" 2>/dev/null)
-    if [ -n "$al_pid" ] && ! kill -0 "$al_pid" 2>/dev/null; then
+    #reboot - clear it and try once more. A lock with no readable pid can't
+    #be judged, so it's left alone and reported rather than stolen.
+    al_pid=$(lock_pid)
+    if [ -n "$al_pid" ] && ! pid_alive "$al_pid"; then
       warn "Clearing a stale lock left by exited run (pid $al_pid)"
-      rm -rf "$LOCK_DIR"
+      rm -rf "$LOCK_DIR" 2>/dev/null || true
     fi
     if ! mkdir "$LOCK_DIR" 2>/dev/null; then
-      error "Another manage.sh run is active here (lock: $LOCK_DIR, pid ${al_pid:-unknown})."
+      if [ -n "$al_pid" ]; then
+        error "Another manage.sh run is active here (lock: $LOCK_DIR, pid $al_pid)."
+      else
+        error "A lock with no owner recorded is in the way: $LOCK_DIR - remove it if no run is active."
+      fi
       exit 1
     fi
   fi
-  echo "$$" >"$LOCK_DIR/pid"
+  echo "$$" >"$LOCK_DIR/pid" 2>/dev/null || true
   HAVE_LOCK=1
 }
 
@@ -1058,7 +1079,7 @@ backup_app() {
 
 #Newest-first list of $1's own backup archives. The name after the app part
 #must be exactly a date (with an optional _HHMMSS), in either the current
-#'app_YYYY-MM-DD' form or the pre-0.4.1 'appYYYY-MM-DD' form - so an app
+#'app_YYYY-MM-DD' form or the pre-0.4.0 'appYYYY-MM-DD' form - so an app
 #whose name extends another's (vault / vault2) never matches its neighbour's
 #archives. A bare glob did, which let BACKUP_KEEP prune the wrong app's
 #backups and restore pick the wrong archive.
@@ -1401,11 +1422,13 @@ run_doctor() {
     fi
   fi
   if [ -d "$LOCK_DIR" ]; then
-    dr_lockpid=$(cat "$LOCK_DIR/pid" 2>/dev/null)
-    if [ -n "$dr_lockpid" ] && kill -0 "$dr_lockpid" 2>/dev/null; then
+    dr_lockpid=$(lock_pid)
+    if [ -n "$dr_lockpid" ] && pid_alive "$dr_lockpid"; then
       dwarn "A run lock is held by an active run (pid $dr_lockpid)"
-    else
+    elif [ -n "$dr_lockpid" ]; then
       dwarn "A stale run lock exists ($LOCK_DIR) - the next state-changing command will clear it"
+    else
+      dwarn "A lock with no owner recorded exists ($LOCK_DIR) - remove it if no run is active"
     fi
   else
     dok "No stale run lock"
@@ -1491,7 +1514,7 @@ update_self() {
     error "The downloaded script failed a syntax check - not installing it."
     exit 1
   fi
-  #Releases from v0.4.1 publish a checksum next to the script; verify when
+  #Releases from v0.4.0 publish a checksum next to the script; verify when
   #it's there (and this host can hash). Older releases simply don't have one.
   expected_sha=$(fetch_url "https://github.com/$SELF_REPO/releases/download/$latest_tag/manage.sh.sha256" 2>/dev/null | awk 'NR==1{print $1}') || expected_sha=""
   if [ -n "$expected_sha" ]; then

tests/run-tests.sh

@@ -170,7 +170,7 @@ assert_no_action "UP:alpha"
 begin "archives: prefix apps (vault/vault2) never cross - prune and restore"
 apps vault vault2
 mkdir -p "$SANDBOX/backup"
-#vault's own legacy no-separator archive (pre-0.4.1), and vault2's newer one
+#vault's own legacy no-separator archive (pre-0.4.0), and vault2's newer one
 echo v1 | tar -cjf "$SANDBOX/backup/vault2026-08-01.tar.bz2" -T /dev/null 2>/dev/null || : >"$SANDBOX/backup/vault2026-08-01.tar.bz2"
 sleep 1
 : >"$SANDBOX/backup/vault22026-08-03.tar.bz2"
@@ -283,6 +283,27 @@ assert_out "unhealthy: beta"
 assert_out "Updated 1 of 2 apps"
 assert_action "UP:beta"     #it did start - health is what failed
 
+begin "lock: a pid-less lock is reported, never silently fatal"
+#A run killed between mkdir and the pid write (or a full disk) leaves a lock
+#with no owner recorded. That must not abort the script under set -e with no
+#output at all, and must not be stolen either - we can't tell if it's live.
+apps alpha
+printf 'alpha\n' >"$DD_STATE/up"
+mkdir "$SANDBOX/.dockerdance.lock"
+run stop alpha
+assert_status 1
+assert_out "no owner recorded"
+assert_no_action "STOP:alpha"
+#doctor must still print its whole report rather than stopping mid-way
+run doctor
+assert_status 0
+assert_out "no owner recorded"
+assert_out "settings:"           #the section printed after the lock check
+rmdir "$SANDBOX/.dockerdance.lock"
+run stop alpha                    #and the folder works again once cleared
+assert_status 0
+assert_action "STOP:alpha"
+
 begin "missing app folder: reported, run continues"
 
 apps alpha gamma