Exclude controller from self fan-out, distinguish provision-failed reasons, fix ssh test stub arg capture

CLAUDE.md

@@ -24,7 +24,11 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co
    - `ssh [user@]client pmset displaysleepnow`.
    - Log exit status per client.
 
-SSH-to-self is a no-op on an already-locked Mac — do not special-case it.
+SSH-to-self is a no-op on an already-locked Mac; the *lock* itself never needs
+special-casing. `lock-fanout` does, however, skip the controller entirely when
+it appears in its own Synergy client list (`warn=skip-self`), since the SSH
+round-trip to reach that same no-op achieves nothing a local invocation
+wouldn't (see issue #27).
 
 ## Key external inputs
 
@@ -69,7 +73,7 @@ and mic-active checks still work normally.
 - `bin/uninstall` — bootout the agent, remove plist and symlinks. Preserves the log file. Idempotent.
 - `bin/list-clients [path]` — slice (a). Parse Synergy conf, emit `<host>.local` hostnames, resolving real display names via the sibling `db.json` when available (see Key external inputs). No arg → reads `~/Library/Preferences/Synergy/synergy.conf`.
 - `bin/lock-watcher` — slices (b)+(c1). Subscribe to `com.apple.sessionagent.screenIsLocked`; on each event emit `<ISO-8601> locked` and invoke `bin/lock-fanout`. Blocks until signaled.
-- `bin/lock-fanout` — slice (c1). Reads hosts from `list-clients`, applies per-host overrides from `~/.config/lock-sync/config`, provisions `lock-guard` on each client on demand over SSH if missing or stale (checksum-compared; see "Upgrade path" above), then SSHes `lock-guard` to each client (see the `bin/lock-guard` bullet below; falls back to bare `pmset displaysleepnow` if provisioning fails), emits one `<ISO-8601> client=<host> user=<user> ssh_exit=<rc>` line per host (plus a `warn=provision-failed` line when provisioning itself failed for that host).
+- `bin/lock-fanout` — slice (c1). Reads hosts from `list-clients`, skips any host that resolves to the controller itself (case-insensitive match against `hostname -s` + `.local`; logs `client=<host> warn=skip-self` and moves on — see issue #27), applies per-host overrides from `~/.config/lock-sync/config`, provisions `lock-guard` on each client on demand over SSH if missing or stale (checksum-compared; see "Upgrade path" above), then SSHes `lock-guard` to each client (see the `bin/lock-guard` bullet below; falls back to bare `pmset displaysleepnow` if provisioning fails), emits one `<ISO-8601> client=<host> user=<user> ssh_exit=<rc>` line per host (plus a `warn=provision-failed reason=<missing-local-lock-guard|ssh-failed>` line, with `ssh_exit=<rc>` appended when the reason is `ssh-failed`, when provisioning itself failed for that host).
 - `bin/lock-guard` — runs on each client (invoked remotely by `lock-fanout` in place of a bare `pmset displaysleepnow`). Skips the lock and logs why if the client looks like it's in a call: a known meeting app is running (`~/.config/lock-sync/guard-processes`), the microphone is actively in use, or a Google Meet room tab is open in Chrome. Emits `<ISO-8601> action=sleep` or `<ISO-8601> action=suppress reason=<reason>`.
 
 ## `.claude/` directory

README.md

@@ -105,9 +105,13 @@ against the copy at `~/.local/bin/lock-guard` on the client (`shasum -a
 256` over SSH) and pushes a fresh copy only if it's missing or stale,
 creating `~/.local/bin` on the client if needed. If that provisioning step
 fails for a client (unreachable, missing `shasum`, etc.), `lock-fanout`
-logs `warn=provision-failed` for that client and falls back to a bare
-`pmset displaysleepnow` call for that cycle — the client still locks, just
-without meeting-suppression until the client is reachable again.
+logs `warn=provision-failed reason=<missing-local-lock-guard|ssh-failed>`
+(with `ssh_exit=<rc>` appended for the `ssh-failed` case) for that client
+and falls back to a bare `pmset displaysleepnow` call for that cycle — the
+client still locks, just without meeting-suppression until the client is
+reachable again. The controller itself is always skipped in fan-out
+(`warn=skip-self`), since it appears in its own Synergy client list but an
+SSH round-trip to itself would achieve nothing a local lock doesn't.
 
 ## Verify
 

bin/lock-fanout

@@ -10,6 +10,13 @@ ssh_cmd="${LOCK_SYNC_SSH:-/usr/bin/ssh}"
 default_user="${USER:-$(id -un)}"
 local_lock_guard="$script_dir/lock-guard"
 shasum_cmd="${LOCK_SYNC_SHASUM:-/usr/bin/shasum}"
+hostname_cmd="${LOCK_SYNC_HOSTNAME:-/bin/hostname}"
+# list-clients emits lowercased <name>.local hostnames (see resolve() there);
+# match that shape here so a controller that's also a member of its own
+# Synergy client list compares equal to itself and gets skipped below,
+# rather than SSHing to itself for no benefit (see fanout_host's is-self
+# check). Lowercased for the same case-insensitive-match reason.
+local_host="$("$hostname_cmd" -s 2>/dev/null | tr '[:upper:]' '[:lower:]').local"
 
 log() {
   local ts
@@ -38,8 +45,25 @@ lookup_user() {
 # doesn't exist yet (new client) is NOT treated as a failure here: it's
 # expected to fall through to the push logic (see the `|| true` comment
 # below on the checksum call for why).
+#
+# On failure (return 1), also sets the caller-visible globals
+# provision_fail_reason (a short machine-readable tag: "missing-local-
+# lock-guard" or "ssh-failed") and provision_fail_rc (the ssh exit code,
+# only meaningful when reason is "ssh-failed" — left at 0 for the
+# missing-local-file guard clause, since no ssh call happens in that case).
+# Globals rather than stdout/return-code-per-class: fanout_host already
+# reads provision_host's success/failure via `if provision_host ...; then`,
+# and threading a second signal through return code or stdout would either
+# collide with that boolean or require capturing stdout on every call site,
+# including the common-case success path. Reset at the top of every call so
+# a stale reason from a prior host can never leak into this host's log line.
+provision_fail_reason=""
+provision_fail_rc=0
+
 provision_host() {
   local host="$1" user="$2" local_sha remote_sha rc=0
+  provision_fail_reason=""
+  provision_fail_rc=0
 
   # Fail fast if the local lock-guard is missing/unreadable, before any
   # shasum or ssh call. Without this guard, a missing local_lock_guard
@@ -49,7 +73,10 @@ provision_host() {
   # empty stream and leaving the client with a zero-byte-but-executable
   # lock-guard that exits 0 without ever calling pmset (silently never
   # locking), instead of cleanly falling back to the pmset path.
-  [[ -r "$local_lock_guard" ]] || return 1
+  if [[ ! -r "$local_lock_guard" ]]; then
+    provision_fail_reason="missing-local-lock-guard"
+    return 1
+  fi
 
   local_sha="$("$shasum_cmd" -a 256 "$local_lock_guard" 2>/dev/null | awk '{print $1}')"
 
@@ -81,6 +108,8 @@ provision_host() {
     -o StrictHostKeyChecking=accept-new \
     "$user@$host" 'shasum -a 256 ~/.local/bin/lock-guard 2>/dev/null || true' 2>/dev/null)" || rc=$?
   if ((rc != 0)); then
+    provision_fail_reason="ssh-failed"
+    provision_fail_rc=$rc
     return 1
   fi
   remote_sha="${remote_sha%% *}"
@@ -102,13 +131,30 @@ provision_host() {
     "$user@$host" 'mkdir -p ~/.local/bin && cat >~/.local/bin/.lock-guard.tmp && chmod +x ~/.local/bin/.lock-guard.tmp && mv -f ~/.local/bin/.lock-guard.tmp ~/.local/bin/lock-guard' \
     <"$local_lock_guard" >/dev/null 2>&1 || rc=$?
   if ((rc != 0)); then
+    provision_fail_reason="ssh-failed"
+    provision_fail_rc=$rc
     return 1
   fi
   return 0
 }
 
 fanout_host() {
-  local host="$1" override user rc=0
+  local host="$1" override user rc=0 host_lower
+
+  # Skip the controller itself: it's a no-op round-trip (a local
+  # pmset/lock-guard invocation would achieve the same lock this SSH call
+  # does) and issue #27 flags the SSH-to-self round-trip as an unnecessary,
+  # possibly risky trigger vector when the controller is also listed as its
+  # own Synergy client. This is a fan-out-level skip, not a change to the
+  # underlying "SSH-to-self is a no-op on an already-locked Mac" fact noted
+  # in CLAUDE.md — that guidance is about the lock itself, not about
+  # whether to attempt the round-trip at all.
+  host_lower="$(printf '%s' "$host" | tr '[:upper:]' '[:lower:]')"
+  if [[ "$host_lower" == "$local_host" ]]; then
+    log "client=$host warn=skip-self"
+    return 0
+  fi
+
   override=$(lookup_user "$host")
   user="${override:-$default_user}"
 
@@ -127,7 +173,11 @@ fanout_host() {
       "$user@$host" "\$HOME/.local/bin/lock-guard" >/dev/null 2>&1 || rc=$?
     log "client=$host user=$user ssh_exit=$rc"
   else
-    log "client=$host user=$user warn=provision-failed"
+    if [[ "$provision_fail_reason" == "ssh-failed" ]]; then
+      log "client=$host user=$user warn=provision-failed reason=$provision_fail_reason ssh_exit=$provision_fail_rc"
+    else
+      log "client=$host user=$user warn=provision-failed reason=$provision_fail_reason"
+    fi
     "$ssh_cmd" -n -o BatchMode=yes \
       -o ConnectTimeout=5 \
       -o StrictHostKeyChecking=accept-new \

tests/lock-fanout.bats

@@ -16,6 +16,24 @@ setup() {
   SSH_LOG="$TMPDIR/ssh.log"
   export SSH_LOG
   export LOCK_SYNC_SSH="$STUB_DIR/ssh"
+  # Pin the "local host" identity used by the self-exclusion check to a
+  # fixed, non-matching value by default, so existing tests (none of whose
+  # client lists are meant to collide with the real test-runner's hostname)
+  # don't accidentally skip a host because the CI/dev machine happens to be
+  # named the same as a fixture client. Tests targeting self-exclusion
+  # override this to a hostname that does match one of their fixture hosts.
+  make_hostname_stub "no-such-controller-host"
+}
+
+# Stub for hostname -s; echoes the given short name.
+make_hostname_stub() {
+  local short_name="$1"
+  cat >"$STUB_DIR/hostname-stub" <<EOF
+#!/bin/bash
+echo "$short_name"
+EOF
+  chmod +x "$STUB_DIR/hostname-stub"
+  export LOCK_SYNC_HOSTNAME="$STUB_DIR/hostname-stub"
 }
 
 teardown() {
@@ -62,7 +80,15 @@ for a in "$@"; do
     -n) has_n=1 ;;
     -o|BatchMode=yes|ConnectTimeout=5|StrictHostKeyChecking=accept-new) ;;
     *@*) target="$a" ;;
-    *) [[ -n "$target" ]] && remote_cmd="$a" ;;
+    *)
+      if [[ -n "$target" ]]; then
+        if [[ -n "$remote_cmd" ]]; then
+          remote_cmd="$remote_cmd $a"
+        else
+          remote_cmd="$a"
+        fi
+      fi
+      ;;
   esac
 done
 stdin_marker=""
@@ -379,3 +405,109 @@ EOF
   [[ "${lines[0]}" == *"client=asiago.local"* ]]
   [[ "${lines[0]}" == *"ssh_exit=0"* ]]
 }
+
+# --- Issue #27: self-exclusion ---
+
+@test "self-exclusion: client list including the local hostname is filtered out of fan-out, others unaffected" {
+  make_list_clients_stub "asiago.local
+tilsit.local
+mimolette.local"
+  make_hostname_stub "asiago"
+  local_sha=$(shasum -a 256 "$BATS_TEST_DIRNAME/../bin/lock-guard" | awk '{print $1}')
+  export STUB_SSH_CHECKSUM_ALL_MATCH="$local_sha"
+  make_ssh_stub
+
+  run "$SCRIPT"
+  [ "$status" -eq 0 ]
+  [ "${#lines[@]}" -eq 3 ]
+  [[ "${lines[0]}" == *"client=asiago.local"* ]]
+  [[ "${lines[0]}" == *"warn=skip-self"* ]]
+  [[ "${lines[1]}" == *"client=tilsit.local"* ]]
+  [[ "${lines[1]}" == *"ssh_exit=0"* ]]
+  [[ "${lines[2]}" == *"client=mimolette.local"* ]]
+  [[ "${lines[2]}" == *"ssh_exit=0"* ]]
+
+  # No ssh call should ever target asiago.local — the skip happens before
+  # any ssh invocation, not just before the final lock-guard call.
+  run grep -q 'TARGET=.*asiago.local' "$SSH_LOG"
+  [ "$status" -ne 0 ]
+  grep -q 'TARGET=.*tilsit.local' "$SSH_LOG"
+  grep -q 'TARGET=.*mimolette.local' "$SSH_LOG"
+}
+
+@test "self-exclusion: hostname comparison is case-insensitive" {
+  make_list_clients_stub "asiago.local"
+  make_hostname_stub "ASIAGO"
+  make_ssh_stub
+
+  run "$SCRIPT"
+  [ "$status" -eq 0 ]
+  [ "${#lines[@]}" -eq 1 ]
+  [[ "${lines[0]}" == *"client=asiago.local"* ]]
+  [[ "${lines[0]}" == *"warn=skip-self"* ]]
+  [ ! -s "$SSH_LOG" ]
+}
+
+@test "self-exclusion: no client matches local hostname, all clients proceed normally" {
+  make_list_clients_stub "tilsit.local
+mimolette.local"
+  make_hostname_stub "asiago"
+  local_sha=$(shasum -a 256 "$BATS_TEST_DIRNAME/../bin/lock-guard" | awk '{print $1}')
+  export STUB_SSH_CHECKSUM_ALL_MATCH="$local_sha"
+  make_ssh_stub
+
+  run "$SCRIPT"
+  [ "$status" -eq 0 ]
+  [ "${#lines[@]}" -eq 2 ]
+  [[ "${lines[0]}" == *"client=tilsit.local"* ]]
+  [[ "${lines[0]}" == *"ssh_exit=0"* ]]
+  [[ "${lines[1]}" == *"client=mimolette.local"* ]]
+  [[ "${lines[1]}" == *"ssh_exit=0"* ]]
+
+  run grep -q 'warn=skip-self' <<<"$output"
+  [ "$status" -ne 0 ]
+}
+
+# --- Issue #28: ssh stub multi-arg remote-command capture ---
+
+@test "ssh stub regression: multiple trailing positional args after user@host are all captured, not just the last" {
+  make_ssh_stub
+  "$STUB_DIR/ssh" -o BatchMode=yes user@host.local "pmset" "displaysleepnow" "extra-arg" >/dev/null
+  grep -q 'TARGET=user@host.local CMD=pmset displaysleepnow extra-arg' "$SSH_LOG"
+}
+
+# --- Issue #29: provision-failed reason distinction ---
+
+@test "provision-failed reason: ssh failure during checksum check logs reason=ssh-failed and the ssh exit code" {
+  make_list_clients_stub "asiago.local"
+  export STUB_SSH_FAIL_HOST="asiago.local"
+  make_ssh_stub
+
+  run "$SCRIPT"
+  [ "$status" -eq 0 ]
+  [[ "${lines[0]}" == *"client=asiago.local"* ]]
+  [[ "${lines[0]}" == *"warn=provision-failed"* ]]
+  [[ "${lines[0]}" == *"reason=ssh-failed"* ]]
+  [[ "${lines[0]}" == *"ssh_exit=255"* ]]
+}
+
+@test "provision-failed reason: missing local lock-guard logs reason=missing-local-lock-guard with no ssh_exit field" {
+  FIXTURE_BIN="$TMPDIR/fixture-bin"
+  mkdir -p "$FIXTURE_BIN"
+  cp "$BATS_TEST_DIRNAME/../bin/lock-fanout" "$FIXTURE_BIN/lock-fanout"
+  # No lock-guard copied into $FIXTURE_BIN — that's the point of this test.
+
+  make_list_clients_stub "asiago.local"
+  make_ssh_stub
+
+  run "$FIXTURE_BIN/lock-fanout"
+  [ "$status" -eq 0 ]
+  [[ "${lines[0]}" == *"client=asiago.local"* ]]
+  [[ "${lines[0]}" == *"warn=provision-failed"* ]]
+  [[ "${lines[0]}" == *"reason=missing-local-lock-guard"* ]]
+  # No ssh_exit field on the provision-failed line itself: no ssh call ever
+  # happened for the checksum/push step in this guard-clause case, so there
+  # is no real ssh exit code to report here (distinct from the fallback
+  # pmset line immediately after, which does have its own ssh_exit=).
+  [[ "${lines[0]}" != *"ssh_exit="* ]]
+}