fix(spm): serialize concurrent scans to protect synthetic Package.swift (DEVA11Y-483)

scripts/bash/spm.sh

@@ -1,7 +1,5 @@
 #!/usr/bin/env bash -il
 
-[ -f "${PWD}/Package.swift" ]
-PACKAGE_EXISTS="$?"
 GIT_ROOT=$(git rev-parse --show-toplevel 2>/dev/null)
 SCRIPT_PATH=$(realpath --relative-to="$GIT_ROOT" "$0" 2>/dev/null || realpath "$0")
 SUBCOMMAND="$1"
@@ -38,22 +36,76 @@
   fi
 }
 
-a11y_scan() {
-  # Ensure Package.swift is removed on exit (acts like a finally block)
-  cleanup() {
-      if [ $PACKAGE_EXISTS -eq 0 ]; then
-          return
-      fi
-      rm -f -- "${PWD}/Package.swift" "${PWD}/Package.resolved"
-  }
-  trap cleanup EXIT
+# Resolve a lock directory OUTSIDE the scanned package, keyed by the package path,
+# so a crash can never leave a lock inside the user's working tree (nor let a
+# pre-commit `git add -A` stage it). cksum is POSIX and always present.
+_spm_lock_dir() {
+  local key
+  key=$(printf '%s' "$PWD" | cksum | cut -d' ' -f1)
+  printf '%s/browserstack-a11y-spm-%s.lock' "${TMPDIR:-/tmp}" "$key"
+}
 
-  setup() {
-      if [ $PACKAGE_EXISTS -eq 0 ]; then
-          return
+# Acquire an exclusive per-directory lock via atomic mkdir. Serializes ALL
+# concurrent scans of the same directory regardless of when each started, so no
+# instance ever deletes a synthetic Package.swift another is still using. A lock
+# left by a crashed peer is reclaimed by PID liveness (kill -0) -- never by
+# wall-clock age, which cannot tell a slow-but-alive scan from a dead one -- and
+# the reclaim is claimed atomically via rename so two waiters can't both take it.
+# Returns: 0 acquired, 2 timed out (a peer is scanning), 1 lock dir unusable.
+_spm_acquire_lock() {
+  local dir="$1" waited=0 announced=0 owner_pid stale
+  while ! mkdir "$dir" 2>/dev/null; do
+    # A non-EEXIST failure (unwritable/read-only/full TMPDIR) is not contention --
+    # fail fast rather than waiting out the full timeout on a misleading message.
+    if [ ! -d "$dir" ]; then
+      echo "A11y scan: cannot create lock at ${dir} (check TMPDIR permissions/space)." >&2
+      return 1
+    fi
+    owner_pid=$(cat "${dir}/pid" 2>/dev/null)
+    if [ -n "$owner_pid" ] && ! kill -0 "$owner_pid" 2>/dev/null; then
+      # Owner is dead: claim the stale lock. Only one racer's mv can succeed; the
+      # losers fall through and retry mkdir. Never rm a path a peer may recreate.
+      stale="${dir}.stale.$$"
+      if mv "$dir" "$stale" 2>/dev/null; then
+        rm -rf -- "$stale"
       fi
+      continue
+    fi
+    if [ "$announced" -eq 0 ]; then
+      echo "A11y scan: waiting for another scan in ${PWD} to finish..." >&2
+      announced=1
+    fi
+    if [ "$waited" -ge 300 ]; then
+      return 2
+    fi
+    sleep 1
+    waited=$((waited + 1))
+  done
+  echo "$$" > "${dir}/pid"
+  return 0
+}
+
+# lock_dir/have_lock/created_package are GLOBAL on purpose: the EXIT trap fires
+# after a11y_scan has returned, when its `local`s are already out of scope, so
+# cleanup state must be global to survive.
+a11y_scan() {
+  lock_dir=$(_spm_lock_dir)
+  have_lock=0
+  created_package=0
 
-      cat > Package.swift <<EOF
+  _spm_acquire_lock "$lock_dir"
+  case "$?" in
+    0) have_lock=1 ;;
+    2) echo "A11y scan: another scan is already running in ${PWD}; skipping." >&2
+       return 0 ;;
+    *) echo "A11y scan: proceeding without a lock; concurrent scans in ${PWD} may conflict." >&2 ;;
+  esac
+
+  # Decide ownership UNDER the lock, from the live filesystem -- not a snapshot
+  # taken at script start. Only create the synthetic manifest if none exists now,
+  # and on exit delete only what this instance created.
+  if [ ! -f "${PWD}/Package.swift" ]; then
+    cat > Package.swift <<EOF
 // swift-tools-version: 5.9
 import PackageDescription
 
@@ -65,9 +117,20 @@
     targets: []
 )
 EOF
+    created_package=1
+  fi
+
+  # finally-block: remove only our own synthetic manifest, then release the lock.
+  cleanup() {
+      if [ "${created_package:-0}" -eq 1 ]; then
+          rm -f -- "${PWD}/Package.swift" "${PWD}/Package.resolved"
+      fi
+      if [ "${have_lock:-0}" -eq 1 ]; then
+          rm -rf -- "$lock_dir"
+      fi
   }
+  trap cleanup EXIT
 
-  setup
   if [[ -z "$EXTRA_ARGS" ]]; then
     EXTRA_ARGS="--include **/*.swift --include **/*.xib --include **/*.storyboard"
   fi

scripts/bash/spm.sh.sha256

@@ -1 +1 @@
-2d627fbe06991da445f7fbcd8bc8c4f5c4dd126c04372f74dd0f4ab39222c57e  spm.sh
+1987749e047ca43b15f10b0eb6a98d0adcfbf5f7de9f54f49831b486406c8ba1  spm.sh

scripts/fish/spm.sh

@@ -1,3 +1,3 @@
 #!/usr/bin/env bash -il
 
 export PATH="$PATH:/opt/homebrew/bin"
@@ -13,8 +13,6 @@
 export BROWSERSTACK_ACCESS_KEY=$($fish_bin -lic 'echo $BROWSERSTACK_ACCESS_KEY' | tail -n 1)
 
 # Don't change anything after this, same as the bash equivalent
-[ -f "${PWD}/Package.swift" ]
-PACKAGE_EXISTS="$?"
 GIT_ROOT=$(git rev-parse --show-toplevel 2>/dev/null)
 SCRIPT_PATH=$(realpath --relative-to="$GIT_ROOT" "$0" 2>/dev/null || realpath "$0")
 SUBCOMMAND="$1"
@@ -51,22 +49,76 @@
   fi
 }
 
-a11y_scan() {
-  # Ensure Package.swift is removed on exit (acts like a finally block)
-  cleanup() {
-      if [ $PACKAGE_EXISTS -eq 0 ]; then
-          return
-      fi
-      rm -f -- "${PWD}/Package.swift" "${PWD}/Package.resolved"
-  }
-  trap cleanup EXIT
+# Resolve a lock directory OUTSIDE the scanned package, keyed by the package path,
+# so a crash can never leave a lock inside the user's working tree (nor let a
+# pre-commit `git add -A` stage it). cksum is POSIX and always present.
+_spm_lock_dir() {
+  local key
+  key=$(printf '%s' "$PWD" | cksum | cut -d' ' -f1)
+  printf '%s/browserstack-a11y-spm-%s.lock' "${TMPDIR:-/tmp}" "$key"
+}
 
-  setup() {
-      if [ $PACKAGE_EXISTS -eq 0 ]; then
-          return
+# Acquire an exclusive per-directory lock via atomic mkdir. Serializes ALL
+# concurrent scans of the same directory regardless of when each started, so no
+# instance ever deletes a synthetic Package.swift another is still using. A lock
+# left by a crashed peer is reclaimed by PID liveness (kill -0) -- never by
+# wall-clock age, which cannot tell a slow-but-alive scan from a dead one -- and
+# the reclaim is claimed atomically via rename so two waiters can't both take it.
+# Returns: 0 acquired, 2 timed out (a peer is scanning), 1 lock dir unusable.
+_spm_acquire_lock() {
+  local dir="$1" waited=0 announced=0 owner_pid stale
+  while ! mkdir "$dir" 2>/dev/null; do
+    # A non-EEXIST failure (unwritable/read-only/full TMPDIR) is not contention --
+    # fail fast rather than waiting out the full timeout on a misleading message.
+    if [ ! -d "$dir" ]; then
+      echo "A11y scan: cannot create lock at ${dir} (check TMPDIR permissions/space)." >&2
+      return 1
+    fi
+    owner_pid=$(cat "${dir}/pid" 2>/dev/null)
+    if [ -n "$owner_pid" ] && ! kill -0 "$owner_pid" 2>/dev/null; then
+      # Owner is dead: claim the stale lock. Only one racer's mv can succeed; the
+      # losers fall through and retry mkdir. Never rm a path a peer may recreate.
+      stale="${dir}.stale.$$"
+      if mv "$dir" "$stale" 2>/dev/null; then
+        rm -rf -- "$stale"
       fi
+      continue
+    fi
+    if [ "$announced" -eq 0 ]; then
+      echo "A11y scan: waiting for another scan in ${PWD} to finish..." >&2
+      announced=1
+    fi
+    if [ "$waited" -ge 300 ]; then
+      return 2
+    fi
+    sleep 1
+    waited=$((waited + 1))
+  done
+  echo "$$" > "${dir}/pid"
+  return 0
+}
+
+# lock_dir/have_lock/created_package are GLOBAL on purpose: the EXIT trap fires
+# after a11y_scan has returned, when its `local`s are already out of scope, so
+# cleanup state must be global to survive.
+a11y_scan() {
+  lock_dir=$(_spm_lock_dir)
+  have_lock=0
+  created_package=0
 
-      cat > Package.swift <<EOF
+  _spm_acquire_lock "$lock_dir"
+  case "$?" in
+    0) have_lock=1 ;;
+    2) echo "A11y scan: another scan is already running in ${PWD}; skipping." >&2
+       return 0 ;;
+    *) echo "A11y scan: proceeding without a lock; concurrent scans in ${PWD} may conflict." >&2 ;;
+  esac
+
+  # Decide ownership UNDER the lock, from the live filesystem -- not a snapshot
+  # taken at script start. Only create the synthetic manifest if none exists now,
+  # and on exit delete only what this instance created.
+  if [ ! -f "${PWD}/Package.swift" ]; then
+    cat > Package.swift <<EOF
 // swift-tools-version: 5.9
 import PackageDescription
 
@@ -78,9 +130,20 @@
     targets: []
 )
 EOF
+    created_package=1
+  fi
+
+  # finally-block: remove only our own synthetic manifest, then release the lock.
+  cleanup() {
+      if [ "${created_package:-0}" -eq 1 ]; then
+          rm -f -- "${PWD}/Package.swift" "${PWD}/Package.resolved"
+      fi
+      if [ "${have_lock:-0}" -eq 1 ]; then
+          rm -rf -- "$lock_dir"
+      fi
   }
+  trap cleanup EXIT
 
-  setup
   if [[ -z "$EXTRA_ARGS" ]]; then
     EXTRA_ARGS="--include **/*.swift --include **/*.xib --include **/*.storyboard"
   fi

scripts/fish/spm.sh.sha256

@@ -1 +1 @@
-8fe00a7635d3ad66d7197994631c1a58581d4b3a845453f72e5e237338dea24e  spm.sh
+87bf749b365b86157934671d913b3e4bcdb2b54e43b765d0850218d442ca4e26  spm.sh

scripts/zsh/spm.sh

@@ -1,3 +1,3 @@
 #!/usr/bin/env bash -il
 
 # Shell specific
@@ -12,8 +12,6 @@
 export BROWSERSTACK_ACCESS_KEY=$($zsh_bin -lic 'echo $BROWSERSTACK_ACCESS_KEY' | tail -n 1)
 
 # Don't change anything after this, same as the bash equivalent
-[ -f "${PWD}/Package.swift" ]
-PACKAGE_EXISTS="$?"
 GIT_ROOT=$(git rev-parse --show-toplevel 2>/dev/null)
 SCRIPT_PATH=$(realpath --relative-to="$GIT_ROOT" "$0" 2>/dev/null || realpath "$0")
 SUBCOMMAND="$1"
@@ -50,22 +48,76 @@
   fi
 }
 
-a11y_scan() {
-  # Ensure Package.swift is removed on exit (acts like a finally block)
-  cleanup() {
-      if [ $PACKAGE_EXISTS -eq 0 ]; then
-          return
-      fi
-      rm -f -- "${PWD}/Package.swift" "${PWD}/Package.resolved"
-  }
-  trap cleanup EXIT
+# Resolve a lock directory OUTSIDE the scanned package, keyed by the package path,
+# so a crash can never leave a lock inside the user's working tree (nor let a
+# pre-commit `git add -A` stage it). cksum is POSIX and always present.
+_spm_lock_dir() {
+  local key
+  key=$(printf '%s' "$PWD" | cksum | cut -d' ' -f1)
+  printf '%s/browserstack-a11y-spm-%s.lock' "${TMPDIR:-/tmp}" "$key"
+}
 
-  setup() {
-      if [ $PACKAGE_EXISTS -eq 0 ]; then
-          return
+# Acquire an exclusive per-directory lock via atomic mkdir. Serializes ALL
+# concurrent scans of the same directory regardless of when each started, so no
+# instance ever deletes a synthetic Package.swift another is still using. A lock
+# left by a crashed peer is reclaimed by PID liveness (kill -0) -- never by
+# wall-clock age, which cannot tell a slow-but-alive scan from a dead one -- and
+# the reclaim is claimed atomically via rename so two waiters can't both take it.
+# Returns: 0 acquired, 2 timed out (a peer is scanning), 1 lock dir unusable.
+_spm_acquire_lock() {
+  local dir="$1" waited=0 announced=0 owner_pid stale
+  while ! mkdir "$dir" 2>/dev/null; do
+    # A non-EEXIST failure (unwritable/read-only/full TMPDIR) is not contention --
+    # fail fast rather than waiting out the full timeout on a misleading message.
+    if [ ! -d "$dir" ]; then
+      echo "A11y scan: cannot create lock at ${dir} (check TMPDIR permissions/space)." >&2
+      return 1
+    fi
+    owner_pid=$(cat "${dir}/pid" 2>/dev/null)
+    if [ -n "$owner_pid" ] && ! kill -0 "$owner_pid" 2>/dev/null; then
+      # Owner is dead: claim the stale lock. Only one racer's mv can succeed; the
+      # losers fall through and retry mkdir. Never rm a path a peer may recreate.
+      stale="${dir}.stale.$$"
+      if mv "$dir" "$stale" 2>/dev/null; then
+        rm -rf -- "$stale"
       fi
+      continue
+    fi
+    if [ "$announced" -eq 0 ]; then
+      echo "A11y scan: waiting for another scan in ${PWD} to finish..." >&2
+      announced=1
+    fi
+    if [ "$waited" -ge 300 ]; then
+      return 2
+    fi
+    sleep 1
+    waited=$((waited + 1))
+  done
+  echo "$$" > "${dir}/pid"
+  return 0
+}
+
+# lock_dir/have_lock/created_package are GLOBAL on purpose: the EXIT trap fires
+# after a11y_scan has returned, when its `local`s are already out of scope, so
+# cleanup state must be global to survive.
+a11y_scan() {
+  lock_dir=$(_spm_lock_dir)
+  have_lock=0
+  created_package=0
 
-      cat > Package.swift <<EOF
+  _spm_acquire_lock "$lock_dir"
+  case "$?" in
+    0) have_lock=1 ;;
+    2) echo "A11y scan: another scan is already running in ${PWD}; skipping." >&2
+       return 0 ;;
+    *) echo "A11y scan: proceeding without a lock; concurrent scans in ${PWD} may conflict." >&2 ;;
+  esac
+
+  # Decide ownership UNDER the lock, from the live filesystem -- not a snapshot
+  # taken at script start. Only create the synthetic manifest if none exists now,
+  # and on exit delete only what this instance created.
+  if [ ! -f "${PWD}/Package.swift" ]; then
+    cat > Package.swift <<EOF
 // swift-tools-version: 5.9
 import PackageDescription
 
@@ -77,9 +129,20 @@
     targets: []
 )
 EOF
+    created_package=1
+  fi
+
+  # finally-block: remove only our own synthetic manifest, then release the lock.
+  cleanup() {
+      if [ "${created_package:-0}" -eq 1 ]; then
+          rm -f -- "${PWD}/Package.swift" "${PWD}/Package.resolved"
+      fi
+      if [ "${have_lock:-0}" -eq 1 ]; then
+          rm -rf -- "$lock_dir"
+      fi
   }
+  trap cleanup EXIT
 
-  setup
   if [[ -z "$EXTRA_ARGS" ]]; then
     EXTRA_ARGS="--include **/*.swift --include **/*.xib --include **/*.storyboard"
   fi

scripts/zsh/spm.sh.sha256

@@ -1 +1 @@
-507bb44d5a17a4e15f59fcd3cdc04692a39a65abc2e4bf5375340939ce484a98  spm.sh
+c798dc6be53c1dfdfa5113697cfe1a23ef48ccf33206e9484feb671d84bad45c  spm.sh