Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 98 additions & 12 deletions scripts/remote.sh
Original file line number Diff line number Diff line change
Expand Up @@ -1959,7 +1959,7 @@ _remote_sync_engine_start_locked() {
# Stop only an engine whose argv proves that it owns this team. A stale
# pidfile may point at a recycled, unrelated process and must never authorize
# signalling that process.
IFS=$'\t' read -r old_state old_pid < <(_remote_sync_engine_status "$team")
IFS=$'\t' read -r old_state old_pid < <(_remote_sync_engine_status "$team" --pidfile-only)
if [ "$old_state" = "running" ]; then
kill "$old_pid" 2>/dev/null || true
fi
Expand Down Expand Up @@ -2026,7 +2026,7 @@ _remote_sync_engine_stop() {
local team="$1" pidfile pid state
pidfile="$(_remote_sync_engine_pidfile "$team")"
[ -f "$pidfile" ] || return 0
IFS=$'\t' read -r state pid < <(_remote_sync_engine_status "$team")
IFS=$'\t' read -r state pid < <(_remote_sync_engine_status "$team" --pidfile-only)
if [ "$state" = "running" ]; then
if ! _remote_sync_engine_reap_owned "$team" "$pid"; then
echo "agmsg: sync engine pid $pid did not stop" >&2
Expand All @@ -2048,11 +2048,84 @@ _remote_sync_engine_stop() {
rm -f "$(_remote_sync_engine_cycle_stamp "$team")" 2>/dev/null || true
}

# Return the systemd user-unit state as "state<TAB>pid".
#
# A unit that is not installed is not a systemd-managed team here, so callers
# retain the pidfile behavior. An existing unit is different: an active unit
# whose MainPID cannot be authenticated is UNKNOWN and must not be shadowed by
# a new unmanaged engine. The command can be replaced in tests; no host
# systemd query is needed there.
_remote_systemd_engine_status() {
local team="$1" unit show load active sub pid command systemctl_bin
unit="agmsg-remote-sync-$team.service"
systemctl_bin="${AGMSG_SYSTEMCTL:-systemctl}"
command -v "$systemctl_bin" >/dev/null 2>&1 || { printf 'unavailable\t\n'; return; }
show="$($systemctl_bin --user show "$unit" -p LoadState -p ActiveState -p SubState -p MainPID 2>/dev/null)" || {
printf 'absent\t\n'
return
}
load="$(printf '%s\n' "$show" | sed -n 's/^LoadState=//p')"
active="$(printf '%s\n' "$show" | sed -n 's/^ActiveState=//p')"
sub="$(printf '%s\n' "$show" | sed -n 's/^SubState=//p')"
pid="$(printf '%s\n' "$show" | sed -n 's/^MainPID=//p')"
[ "$load" = "not-found" ] && { printf 'absent\t\n'; return; }
case "$active:$sub" in
active:running)
if _agmsg_pid_valid "$pid" && _agmsg_pid_alive_local "$pid"; then
command="$(compat_get_cmdline "$pid" 2>/dev/null || true)"
if agmsg_cmdline_names_path "$command" "$SCRIPT_DIR/internal/remote-sync.mjs" &&
case "$command" in *" run --team $team") true ;; *) false ;; esac; then
printf 'running\t%s\n' "$pid"
else
printf 'unknown\t%s\n' "$pid"
fi
else
printf 'unknown\t%s\n' "$pid"
fi
;;
active:starting|active:reloading|active:auto-restart|activating:*|deactivating:*)
printf 'starting\t%s\n' "$pid"
;;
inactive:*|failed:*)
printf 'inactive\t%s\n' "$pid"
;;
*)
printf 'unknown\t%s\n' "$pid"
;;
esac
}

# Print "<state>\t<pid>", where pid is empty when no valid pid is available.
# A live PID is not enough: PID reuse can make an unrelated process pass
# kill -0, so running requires the exact engine script/team suffix in argv.
# The optional --pidfile-only mode is internal: lifecycle operations use it
# when they must inspect only the unmanaged engine represented by this
# command's pidfile. Human/JSON status and sync-start admission deliberately
# omit it so an exported environment value cannot hide a systemd-owned engine.
_remote_sync_engine_status() {
local team="$1" pidfile pid command expected
local team="$1" mode="${2:-}" pidfile pid command expected systemd_state systemd_pid
case "$mode" in
""|--pidfile-only) ;;
*)
echo "agmsg: internal error: unknown sync engine status mode '$mode'" >&2
return 2
;;
esac
REMOTE_SYNC_ENGINE_SUPERVISOR=""
REMOTE_SYNC_ENGINE_SUPERVISOR_PID=""
if [ "$mode" != --pidfile-only ]; then
IFS=$'\t' read -r systemd_state systemd_pid < <(_remote_systemd_engine_status "$team")
else
systemd_state=absent
fi
case "$systemd_state" in
running|starting|inactive|unknown)
REMOTE_SYNC_ENGINE_SUPERVISOR="systemd"
REMOTE_SYNC_ENGINE_SUPERVISOR_PID="$systemd_pid"
printf '%s\t%s\n' "$systemd_state" "$systemd_pid"
return
;;
esac
pidfile="$(_remote_sync_engine_pidfile "$team")"
if [ ! -f "$pidfile" ]; then
printf 'stopped\t\n'
Expand Down Expand Up @@ -2084,7 +2157,7 @@ _remote_sync_engine_status() {
_remote_sync_engine_reap_owned() {
local team="$1" owned_pid="$2" state pid signal attempts
for signal in TERM KILL; do
IFS=$'\t' read -r state pid < <(_remote_sync_engine_status "$team")
IFS=$'\t' read -r state pid < <(_remote_sync_engine_status "$team" --pidfile-only)
if ! _agmsg_pid_alive_local "$owned_pid"; then return 0; fi
[ "$state" = "running" ] && [ "$pid" = "$owned_pid" ] || return 1
kill "-$signal" "$owned_pid" 2>/dev/null || true
Expand Down Expand Up @@ -2496,6 +2569,12 @@ _remote_status_one() {
case "$engine_state" in
running)
echo "$team connected (engine running, pid $engine_pid) since $connected_at" ;;
starting)
echo "$team connected (engine starting under systemd, pid $engine_pid; do not run sync start) since $connected_at" ;;
inactive)
echo "$team connected (engine inactive under systemd; run: systemctl --user restart agmsg-remote-sync-$team.service) since $connected_at" ;;
unknown)
echo "$team connected (engine state unknown under systemd; do not run sync start; inspect systemctl --user status agmsg-remote-sync-$team.service) since $connected_at" ;;
stopped)
echo "$team connected (engine stopped — run: bash $(agmsg_shq "$SKILL_DIR/scripts/remote.sh") sync start $(agmsg_shq "$team")) since $connected_at" ;;
stale)
Expand Down Expand Up @@ -2870,11 +2949,18 @@ cmd_sync_start() {
fi

IFS=$'\t' read -r engine_state engine_pid < <(_remote_sync_engine_status "$team")
if [ "$engine_state" = "running" ]; then
echo "Sync engine already running (pid $engine_pid)."
agmsg_lock_release
return
fi
case "$engine_state" in
running)
echo "Sync engine already running (pid $engine_pid)."
agmsg_lock_release
return
;;
starting|inactive|unknown)
echo "agmsg: systemd owns team '$team' in state '$engine_state'; inspect or restart the user unit instead of sync start" >&2
agmsg_lock_release
return 1
;;
esac

logfile="$CONNECTION_ROOT/run/remote-sync.$team.log"
[ -f "$logfile" ] && log_offset=$(( $(wc -c < "$logfile" | tr -d ' ') + 1 ))
Expand Down Expand Up @@ -2924,7 +3010,7 @@ cmd_sync_start() {
# not the rest of the machine.
agmsg_lock_release
while [ "$i" -lt "$ready_ceiling" ]; do
IFS=$'\t' read -r engine_state ready_pid < <(_remote_sync_engine_status "$team")
IFS=$'\t' read -r engine_state ready_pid < <(_remote_sync_engine_status "$team" --pidfile-only)
if [ "$engine_state" = "running" ] && [ "$ready_pid" = "$started_pid" ] &&
tail -c "+$log_offset" "$logfile" 2>/dev/null |
awk -v nonce="\"startup_nonce\":\"$startup_nonce\"" '
Expand Down Expand Up @@ -3419,7 +3505,7 @@ cmd_set_endpoint() {
done
fi

IFS=$'\t' read -r engine_state engine_pid < <(_remote_sync_engine_status "$team")
IFS=$'\t' read -r engine_state engine_pid < <(_remote_sync_engine_status "$team" --pidfile-only)
[ "$engine_state" = "running" ] && was_running=1
_remote_sync_engine_stop "$team" || {
echo "agmsg: the sync engine did not stop; refusing to move the endpoint under it" >&2
Expand Down Expand Up @@ -3450,7 +3536,7 @@ cmd_set_endpoint() {
# command ran is restarted too (never silently left stopped, and a restart
# is what hands it the moved address -- a running engine keeps its old
# config in memory). _remote_sync_engine_start kills a live engine first.
IFS=$'\t' read -r end_state end_pid < <(_remote_sync_engine_status "$team")
IFS=$'\t' read -r end_state end_pid < <(_remote_sync_engine_status "$team" --pidfile-only)
if [ "$was_running" -eq 1 ] || [ "$end_state" = "running" ]; then
# Same rule as cmd_pull and cmd_connect: the move is this command's purpose
# and it is done by here, so a start failure reports rather than fails --
Expand Down
121 changes: 121 additions & 0 deletions tests/test_remote_status_liveness.bats
Original file line number Diff line number Diff line change
Expand Up @@ -889,3 +889,124 @@ write_unownable_ps_fixture() {
run bash -c "grep -v '^[[:space:]]*#' \"\$1\" | sed 's/_agmsg_pid_alive_local//g' | grep -c '_agmsg_pid_alive'" _ "$SCRIPTS/remote.sh"
[ "$output" = "0" ]
}


# systemd-supervised engine detection for #894. The process itself is real;
# only systemctl is replaced, so these tests exercise the same argv/liveness
# checks used on a host while remaining independent of the test runner's user bus.
write_systemd_show_fixture() {
local state="$1" pid="$2" sub=dead fake="$TEST_SKILL_DIR/fake-systemctl"
[ "$state" = active ] && sub=running
printf '%s\n' '#!/usr/bin/env bash' \
'if [ "${1:-}" = "--user" ] && [ "${2:-}" = "show" ]; then' \
" printf '%s\n' 'LoadState=loaded' 'ActiveState=$state' 'SubState=$sub' 'MainPID=$pid'" \
' exit 0' \
'fi' \
'exit 1' > "$fake"
chmod +x "$fake"
printf '%s\n' "$fake"
}

legacy_skip_systemd_probe_name() {
# Build the removed name without retaining it as a discoverable setting in
# the tree. These tests prove that an old wrapper exporting it is harmless.
printf '%s%s\n' AGMSG_SKIP_SYSTEMD _PROBE
}

@test "status: exported skip variable cannot hide a verified systemd engine (#894)" {
start_matching_engine
rm -f "$TEST_SKILL_DIR/run/remote-sync.testteam.pid"
local fake_bin fake_systemctl
fake_bin="$(write_matching_ps_fixture)"
fake_systemctl="$(write_systemd_show_fixture active "$ENGINE_PID")"

run env PATH="$fake_bin:$PATH" AGMSG_SYSTEMCTL="$fake_systemctl" \
"$(legacy_skip_systemd_probe_name)=1" \
bash "$SCRIPTS/remote.sh" status testteam
[ "$status" -eq 0 ]
printf '%s\n' "$output" | grep -q -F -- "connected (engine running, pid $ENGINE_PID)"
refute grep -qi 'stale\|stopped' <<<"$output"
}

@test "JSON status: exported skip variable cannot hide a verified systemd engine (#894)" {
start_matching_engine
rm -f "$TEST_SKILL_DIR/run/remote-sync.testteam.pid"
local fake_bin fake_systemctl
fake_bin="$(write_matching_ps_fixture)"
fake_systemctl="$(write_systemd_show_fixture active "$ENGINE_PID")"

run env PATH="$fake_bin:$PATH" AGMSG_SYSTEMCTL="$fake_systemctl" \
"$(legacy_skip_systemd_probe_name)=1" \
bash "$SCRIPTS/remote.sh" status testteam --json
[ "$status" -eq 0 ]
[ "$(sqlite_mem "SELECT json_extract('$(printf '%s' "$output" | sed "s/'/''/g")', '\$.engine_state');")" = running ]
[ "$(sqlite_mem "SELECT json_extract('$(printf '%s' "$output" | sed "s/'/''/g")', '\$.engine_pid');")" = "$ENGINE_PID" ]
}

@test "sync start: exported skip variable cannot duplicate an active systemd engine (#894)" {
start_matching_engine
rm -f "$TEST_SKILL_DIR/run/remote-sync.testteam.pid"
local fake_bin fake_systemctl
fake_bin="$(write_matching_ps_fixture)"
fake_systemctl="$(write_systemd_show_fixture active "$ENGINE_PID")"

run env PATH="$fake_bin:$PATH" AGMSG_SYSTEMCTL="$fake_systemctl" \
"$(legacy_skip_systemd_probe_name)=1" \
bash "$SCRIPTS/remote.sh" sync start testteam
[ "$status" -eq 0 ]
printf '%s\n' "$output" | grep -q -F -- "already running (pid $ENGINE_PID)"
[ "$(find "$TEST_SKILL_DIR/run" -maxdepth 1 -name 'remote-sync.testteam.pid' | wc -l)" -eq 0 ]
}

@test "engine status: pidfile-only mode does not invoke the systemd probe (#894)" {
start_matching_engine
local fake_bin fake_systemctl probe_log="$TEST_SKILL_DIR/systemctl-called"
fake_bin="$(write_matching_ps_fixture)"
fake_systemctl="$TEST_SKILL_DIR/fake-systemctl-counting"
printf '%s\n' '#!/usr/bin/env bash' \
': > "$AGMSG_TEST_SYSTEMCTL_PROBE_LOG"' \
"printf '%s\n' 'LoadState=loaded' 'ActiveState=active' 'SubState=running' 'MainPID=$ENGINE_PID'" \
> "$fake_systemctl"
chmod +x "$fake_systemctl"

run env PATH="$fake_bin:$PATH" AGMSG_SYSTEMCTL="$fake_systemctl" \
AGMSG_TEST_SYSTEMCTL_PROBE_LOG="$probe_log" \
bash -c 'source "$1/remote.sh"; _remote_sync_engine_status testteam --pidfile-only' _ "$SCRIPTS"
[ "$status" -eq 0 ]
[ "$output" = $'running\t'"$ENGINE_PID" ]
[ ! -e "$probe_log" ]
}

@test "engine status: rejects an unknown internal mode (#894)" {
run bash -c 'source "$1/remote.sh"; _remote_sync_engine_status testteam --unknown-mode' _ "$SCRIPTS"
[ "$status" -eq 2 ]
[[ "$output" == *"unknown sync engine status mode '--unknown-mode'"* ]]
}

@test "sync start: refuses an active systemd unit with unverified identity (#894)" {
sleep 30 &
local foreign_pid=$!
ENGINE_PIDS="$ENGINE_PIDS $foreign_pid"
local fake_bin fake_systemctl
fake_bin="$(write_matching_ps_fixture)"
fake_systemctl="$(write_systemd_show_fixture active "$foreign_pid")"

run env PATH="$fake_bin:$PATH" AGMSG_SYSTEMCTL="$fake_systemctl" \
bash "$SCRIPTS/remote.sh" sync start testteam
[ "$status" -eq 1 ]
printf '%s\n' "$output" | grep -q -F -- "systemd owns team 'testteam'"
refute test -f "$TEST_SKILL_DIR/run/remote-sync.testteam.pid"
}


@test "status: reports an inactive systemd unit with restart guidance (#894)" {
start_matching_engine
rm -f "$TEST_SKILL_DIR/run/remote-sync.testteam.pid"
local fake_systemctl
fake_systemctl="$(write_systemd_show_fixture inactive 0)"

run env AGMSG_SYSTEMCTL="$fake_systemctl" bash "$SCRIPTS/remote.sh" status testteam
[ "$status" -eq 0 ]
printf '%s\n' "$output" | grep -q -F -- "engine inactive under systemd"
printf '%s\n' "$output" | grep -q -F -- "systemctl --user restart agmsg-remote-sync-testteam.service"
}
Loading