From 5f51a401abec379d67c823e0d520709f34e4333c Mon Sep 17 00:00:00 2001 From: Claude Code Bot Date: Thu, 13 Aug 2026 19:20:35 -0700 Subject: [PATCH] fix: exclude controller from its own fan-out, distinguish provision-failed reasons, fix ssh test stub arg capture Excludes the controller from its own Synergy client fan-out (an SSH round-trip to self achieves nothing a local lock-guard invocation wouldn't, per issue #27's follow-up); makes warn=provision-failed distinguish a missing local lock-guard from an actual SSH failure, including the ssh_exit code for the latter (issue #29); and fixes the lock-fanout.bats ssh stub to capture all trailing positional args after user@host instead of only the last one (issue #28). Closes #27 Closes #28 Closes #29 Claude-Session: https://claude.ai/code/session_01GBtkU7NAZXszj4MyLFpi6V --- CLAUDE.md | 8 ++- README.md | 10 ++- bin/lock-fanout | 56 ++++++++++++++++- tests/lock-fanout.bats | 134 ++++++++++++++++++++++++++++++++++++++++- 4 files changed, 199 insertions(+), 9 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index d7d4b7e..526e99c 100644 --- a/CLAUDE.md +++ b/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 `.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 ` 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 ` client= user= ssh_exit=` 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= 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 ` client= user= ssh_exit=` line per host (plus a `warn=provision-failed reason=` line, with `ssh_exit=` 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 ` action=sleep` or ` action=suppress reason=`. ## `.claude/` directory diff --git a/README.md b/README.md index cc8010d..aeb6219 100644 --- a/README.md +++ b/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=` +(with `ssh_exit=` 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 diff --git a/bin/lock-fanout b/bin/lock-fanout index e4a17ad..f00f0df 100755 --- a/bin/lock-fanout +++ b/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 .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 \ diff --git a/tests/lock-fanout.bats b/tests/lock-fanout.bats index 963a0ac..7886f6a 100755 --- a/tests/lock-fanout.bats +++ b/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" </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="* ]] +}