fix(loop): resolve O-2 atomic race-free lock and marker ownership release guard (22/22 PASS)

2026-08-11 12:30:16 +09:00
parent f8084cd927
commit c7df7ffd68
7 changed files with 577 additions and 27 deletions
@@ -0,0 +1,168 @@
+#!/usr/bin/env bash
+# loop_lock.sh — Atomic race-free loop lock acquisition and marker ownership (O-2)
+# Manages .mam/loop-guard-active with atomic hard links and pid+lstart identity validation.
+
+MAM_LOCK_STEAL_RETRIES="${MAM_LOCK_STEAL_RETRIES:-3}"
+
+mam_lstart() {
+  local pid="${1:-$$}"
+  ps -p "$pid" -o lstart= 2>/dev/null | tr -s ' ' | sed 's/^ *//;s/ *$//'
+}
+
+_mam_lock_field() {
+  local file="$1" key="$2"
+  if [ ! -f "$file" ]; then
+    return 1
+  fi
+  grep "^${key}=" "$file" 2>/dev/null | head -n 1 | cut -d'=' -f2- || true
+}
+
+_mam_lock_holder_state() {
+  local marker="$1"
+  if [ ! -f "$marker" ]; then
+    printf 'stale'
+    return 0
+  fi
+
+  local rec_pid rec_lstart cur_lstart own_lstart
+  rec_pid=$(_mam_lock_field "$marker" "pid")
+  rec_lstart=$(_mam_lock_field "$marker" "lstart")
+
+  if [ -z "$rec_pid" ] || ! [[ "$rec_pid" =~ ^[0-9]+$ ]]; then
+    printf 'stale'
+    return 0
+  fi
+
+  own_lstart=$(mam_lstart "$$")
+  if [ -z "$own_lstart" ]; then
+    printf 'unknown'
+    return 0
+  fi
+
+  cur_lstart=$(mam_lstart "$rec_pid")
+  if [ -z "$cur_lstart" ]; then
+    printf 'stale'
+    return 0
+  fi
+
+  if [ -n "$rec_lstart" ]; then
+    if [ "$cur_lstart" != "$rec_lstart" ]; then
+      printf 'stale'
+      return 0
+    fi
+  fi
+
+  printf 'live'
+  return 0
+}
+
+_mam_lock_publish() {
+  local marker="$1"
+  local parent_dir
+  parent_dir=$(dirname "$marker")
+  mkdir -p "$parent_dir" 2>/dev/null || true
+
+  local tmp_marker="${marker}.tmp.${$}.${RANDOM:-0}"
+  local lstart_val
+  lstart_val=$(mam_lstart "$$")
+
+  printf 'pid=%s\nlstart=%s\nstarted=%s\n' \
+    "$$" "$lstart_val" "$(date -u +%Y-%m-%dT%H:%M:%SZ 2>/dev/null || date)" > "$tmp_marker"
+
+  local rc=0
+  ln "$tmp_marker" "$marker" 2>/dev/null || rc=$?
+  rm -f "$tmp_marker" 2>/dev/null || true
+  return $rc
+}
+
+mam_acquire_loop_lock() {
+  local marker="${1:-}"
+  if [ -z "$marker" ]; then
+    return 2
+  fi
+
+  local steal_lock="${marker}.steal"
+  local retries=0
+
+  while [ "$retries" -le "$MAM_LOCK_STEAL_RETRIES" ]; do
+    # Upgrade path: clean up legacy directory at steal_lock if present
+    if [ -d "$steal_lock" ]; then
+      rmdir "$steal_lock" 2>/dev/null || true
+    fi
+
+    # Try publishing primary marker
+    if _mam_lock_publish "$marker"; then
+      return 0
+    fi
+
+    # Check primary marker holder state
+    local state
+    state=$(_mam_lock_holder_state "$marker")
+    if [ "$state" = "live" ]; then
+      return 3
+    fi
+    if [ "$state" = "unknown" ]; then
+      return 4
+    fi
+
+    # Primary marker is stale! Try publishing steal_lock
+    if _mam_lock_publish "$steal_lock"; then
+      if [ "$(_mam_lock_holder_state "$marker")" = "stale" ]; then
+        rm -f "$marker" 2>/dev/null || true
+      fi
+      rm -f "$steal_lock" 2>/dev/null || true
+      if _mam_lock_publish "$marker"; then
+        return 0
+      fi
+    fi
+
+    # Check steal_lock holder state
+    local steal_state
+    steal_state=$(_mam_lock_holder_state "$steal_lock")
+    case "$steal_state" in
+      live)
+        return 3
+        ;;
+      unknown)
+        return 4
+        ;;
+      stale)
+        rm -f "$steal_lock" 2>/dev/null || true
+        retries=$((retries + 1))
+        continue
+        ;;
+    esac
+
+    sleep 1
+    retries=$((retries + 1))
+  done
+
+  return 3
+}
+
+mam_release_loop_lock() {
+  local marker="${1:-}"
+  if [ -z "$marker" ] || [ ! -f "$marker" ]; then
+    return 0
+  fi
+
+  local rec_pid rec_lstart cur_lstart
+  rec_pid=$(_mam_lock_field "$marker" "pid")
+  rec_lstart=$(_mam_lock_field "$marker" "lstart")
+  cur_lstart=$(mam_lstart "$$")
+
+  if [ "$rec_pid" = "$$" ] && [ "$rec_lstart" = "$cur_lstart" ]; then
+    rm -f "$marker" 2>/dev/null || true
+    return 0
+  fi
+
+  return 1
+}
+
+if [ "${BASH_SOURCE[0]}" = "$0" ]; then
+  case "${1:-}" in
+    acquire) mam_acquire_loop_lock "${2:-}" ;;
+    release) mam_release_loop_lock "${2:-}" ;;
+    *) echo "Usage: $0 {acquire|release} <marker-path>" ; exit 1 ;;
+  esac
+fi
@@ -12,6 +12,8 @@ REPO_ROOT="$(cd "$SCRIPT_DIR/../../../.." && pwd)"
 source "$REPO_ROOT/.agents/skills/lib.sh"
 # shellcheck disable=SC1091
 source "$REPO_ROOT/.agents/skills/multi-agent-mux-loop/scripts/diff_collect.sh"
+# shellcheck disable=SC1091
+source "$REPO_ROOT/.agents/skills/multi-agent-mux-loop/scripts/loop_lock.sh"

 # Default configuration parameters
 PLAN_MODE=false
@@ -82,13 +84,8 @@ if [ -z "$TARGET_AGENT" ] || [ -z "$TASK" ]; then
  usage
 fi

-MAM_LOOP_MARKER="$REPO_ROOT/.mam/loop-guard-active"
-mkdir -p "$(dirname "$MAM_LOOP_MARKER")"
-_mam_lstart() { ps -p "$1" -o lstart= 2>/dev/null | tr -s ' ' | sed 's/^ *//;s/ *$//'; }
-printf 'pid=%s\nlstart=%s\nstarted=%s\n' \
-  "$$" "$(_mam_lstart $$)" "$(date -u +%Y-%m-%dT%H:%M:%SZ)" > "$MAM_LOOP_MARKER"
-_mam_release_guard() { rm -f "$MAM_LOOP_MARKER"; }
-trap _mam_release_guard EXIT INT TERM HUP
+MAM_LOOP_MARKER="${MAM_LOOP_MARKER:-$REPO_ROOT/.mam/loop-guard-active}"
+_mam_release_guard() { mam_release_loop_lock "$MAM_LOOP_MARKER" || true; }

 delegate_job_safe() {
  local orig_script="$REPO_ROOT/.agents/skills/multi-agent-mux-delegate-job/multi-agent-mux-delegate-job"
@@ -119,6 +116,25 @@ log_error() {
  echo -e "\033[1;31m[✗]\033[0m $1"
 }

+_mam_acquire_rc=0
+mam_acquire_loop_lock "$MAM_LOOP_MARKER" || _mam_acquire_rc=$?
+case "$_mam_acquire_rc" in
+  0) ;;
+  3)
+    _holder_pid=$(_mam_lock_field "$MAM_LOOP_MARKER" "pid" 2>/dev/null || echo "unknown")
+    log_error "Another multi-agent-mux-loop is already running in this workspace."
+    log_error "  holder PID: $_holder_pid"
+    log_error "  refusing to start a second loop (SQLite/YAML would race)."
+    exit 1
+    ;;
+  *)
+    log_error "Cannot determine whether another loop is running (ps unavailable?)."
+    log_error "  refusing to start rather than risk two loops writing the registry."
+    exit 1
+    ;;
+esac
+trap _mam_release_guard EXIT INT TERM HUP
+
 # --all-reviewer silently takes precedence over an explicit --reviewer list;
 # warn so the discarded list isn't mistaken for having been honored (P2-1).
 if [ "$ALL_REVIEWERS" = true ] && [ -n "$REVIEWER_LIST" ]; then