loop recovery: wait on the watchdog's ActiveState, and only a root flysim that is the running build

review-ladder r2: a oneshot probe mid-run is 'activating', which
is-active does not count, so the wait never waited. The root copy must
also be byte-identical to /opt/fly/current/flysim, so a manual rollback
cannot approve an archive the running build refuses. Docs: the hold
after an unrestorable rung, the post-deploy --list check, and never
stopping the unit mid-step.

infra/bin/fly-loop-reset

@@ -94,12 +94,21 @@ migration_accepted() {
     return 1
 }
 
+# The root copy must be the flysim that runs: a manual rollback of /opt/fly/current without a
+# deploy would otherwise approve archives the running build refuses.
+LIVE_FLYSIM=/opt/fly/current/flysim
+[ "$(id -u)" -eq 0 ] || LIVE_FLYSIM="${FLY_LOOP_RESET_TEST_LIVE_FLYSIM:-$FLYSIM}"
+same_build() {
+    [ -f "$LIVE_FLYSIM" ] \
+        && [ "$(sha256sum < "$FLYSIM" | cut -d' ' -f1)" = "$(sha256sum < "$LIVE_FLYSIM" | cut -d' ' -f1)" ]
+}
+
 build_compat=""
-if [ -x "$FLYSIM" ] && [ "$(stat -c %u "$FLYSIM")" = "$(id -u)" ]; then
+if [ -x "$FLYSIM" ] && [ "$(stat -c %u "$FLYSIM")" = "$(id -u)" ] && same_build; then
     build_compat="$(clean_env timeout 90 "$FLYSIM" --print-compatibility 2>/dev/null | tail -n1 || true)"
 fi
 if [ -z "$build_compat" ]; then
-    log "no compatibility string from $FLYSIM (missing, not owned by $(id -un), or failed); no rung is restorable"
+    log "no compatibility string from $FLYSIM (missing, not owned by $(id -un), not the running build, or failed); no rung is restorable"
     [ "$LIST" -eq 1 ] && exit 0
     exit 3
 fi
@@ -141,11 +150,18 @@ trap 'exit 143' TERM INT HUP
 
 log "pausing ${WATCHDOG_TIMER} and stopping ${SERVICE} to reset to rung ${RANK}"
 systemctl stop "$WATCHDOG_TIMER"
+# A oneshot probe mid-run is "activating", which `is-active` does not count; wait on ActiveState.
+watchdog_idle() {
+    case "$(systemctl show -p ActiveState --value "$WATCHDOG_SERVICE")" in
+        inactive|failed) return 0 ;;
+        *) return 1 ;;
+    esac
+}
 for _ in $(seq 1 60); do
-    systemctl is-active --quiet "$WATCHDOG_SERVICE" || break
+    watchdog_idle && break
     sleep 1
 done
-if systemctl is-active --quiet "$WATCHDOG_SERVICE"; then
+if ! watchdog_idle; then
     log "${WATCHDOG_SERVICE} still running after 60 s; not resetting"
     exit 4
 fi

infra/docs/loop-recovery.md

@@ -15,7 +15,8 @@ a ladder one step per trap that outlives the previous step:
 - **Outlives** means the reports that confirm it again are at least 20 minutes after the step, so
   their 10-brain-minute window lies after it. The helper waits that long after every step.
 - **Budget:** at most two milestone resets per 24 hours. With the budget spent the step is a
-  restart, at most every three hours, until a reset is free again.
+  restart, at most every three hours, until a reset is free again. The same three-hour spacing applies
+  when a reset level finds no restorable rung and restarts instead.
 - **Starting over:** the ladder returns to level 0 when the fly reaches a new best rung, or after
   six hours with no suspected report.
 
@@ -29,9 +30,12 @@ release env file so a deploy does not drop it.
 The step runs `/opt/fly/bin/fly-loop-reset <rung>` through sudo (the one line in
 `config/fly-sudoers`). As root it:
 
-- runs only the root-owned `/opt/fly/sbin/flysim` that `05-deploy.sh` installs from the release
-  tarball after checking it against the tarball's MANIFEST, never the fly-owned release tree;
-  without that copy no rung is restorable;
+- runs only the root-owned `/opt/fly/sbin/flysim`, and only while it is byte-identical to the
+  flysim `/opt/fly/current` points to, that `05-deploy.sh` installs from the release
+  tarball after checking it against the tarball's MANIFEST (never the fly-owned release tree).
+  Without that copy (an infra-only deploy, or a manual rollback) no rung is restorable and the
+  ladder only restarts; after a release deploy, check as `fly` that
+  `sudo -n /opt/fly/bin/fly-loop-reset --list` names the current rung;
 - reads `fly.env` as `KEY=VALUE` data, never sources it, and ignores the caller's environment;
 - stops `fly-watchdog.timer` and waits for a running probe to finish, so the watchdog cannot
   start flysim on a half-rewritten store; stops flysim; runs `fly-reset-to-milestone`; and on every
@@ -41,7 +45,9 @@ The reset copies both stores to `/srv/fly/state.reset-<UTC>` first, as in the ru
 clears milestone archives above the rung. After each step the helper waits up to four minutes for
 `/status` to report `running` (and the target rank for a reset). Otherwise the step is recorded
 as failed and the ladder still climbs. The step is recorded before it runs, so a helper killed
-mid-step has still climbed and spent the reset.
+mid-step has still climbed and spent the reset. Do not stop `fly-loop-recover.service` during a
+step: that kills the reset too, and the wrapper's exit trap starts flysim on whatever state is
+left.
 
 ## On stream
 

infra/tests/test_loop_recover.py

@@ -267,7 +267,12 @@ class WrapperTests(unittest.TestCase):
         self.calls = root / "calls.log"
         (root / "state").mkdir()
         (root / "bin").mkdir()
-        self.script(root / "bin/systemctl", f'echo "systemctl $*" >> {self.calls}; [ "$1" != is-active ] || exit 3')
+        # The watchdog probe is mid-run ("activating") for the first two looks.
+        self.script(root / "bin/systemctl", f'''echo "systemctl $*" >> {self.calls}
+if [ "$1" = show ]; then
+  n=$(grep -c "^systemctl show" {self.calls})
+  if [ "$n" -le 2 ]; then echo activating; else echo inactive; fi
+fi''')
         self.script(root / "flysim", f'[ "$1" = --print-compatibility ] && echo "{BUILD}"')
         self.script(root / "reset", f'echo "reset $* FLY_BIN=$FLY_BIN" >> {self.calls}')
         self.env_file = root / "fly.env"
@@ -288,6 +293,7 @@ class WrapperTests(unittest.TestCase):
         env = {"PATH": f"{self.root}/bin:/usr/bin:/bin", "FLY_LOOP_RESET_TEST_STATE_DIR": str(self.root / "state"),
                "FLY_LOOP_RESET_TEST_ENV_FILE": str(self.env_file), "FLY_LOOP_RESET_TEST_FLYSIM": str(self.root / "flysim"),
                "FLY_LOOP_RESET_TEST_RESET_BIN": str(self.root / "reset")}
+        env.update(getattr(self, "extra_env", {}))
         return recover.subprocess.run([str(WRAPPER), *args], env=env, capture_output=True, text=True, timeout=60)
 
     def log(self):
@@ -303,7 +309,9 @@ class WrapperTests(unittest.TestCase):
         done = self.run_wrapper("12")
         self.assertEqual(done.returncode, 0, done.stderr)
         self.assertEqual([line.split()[:3] for line in self.log().splitlines() if not line.startswith("systemctl is-active")], [
-            ["systemctl", "stop", "fly-watchdog.timer"], ["systemctl", "stop", "flysim.service"],
+            ["systemctl", "stop", "fly-watchdog.timer"],
+            ["systemctl", "show", "-p"], ["systemctl", "show", "-p"], ["systemctl", "show", "-p"], ["systemctl", "show", "-p"],
+            ["systemctl", "stop", "flysim.service"],
             ["reset", "12", f"FLY_BIN={self.root}/flysim"],
             ["systemctl", "start", "flysim.service"], ["systemctl", "start", "fly-watchdog.timer"]])
 
@@ -324,6 +332,20 @@ class WrapperTests(unittest.TestCase):
         self.assertEqual(self.run_wrapper("12").returncode, 3)
         self.assertEqual(self.log(), "")
 
+    def test_a_root_copy_that_is_not_the_running_build_restores_nothing(self):
+        other = self.root / "live-flysim"
+        self.script(other, "echo other build")
+        self.extra_env = {"FLY_LOOP_RESET_TEST_LIVE_FLYSIM": str(other)}
+        self.assertEqual(self.run_wrapper("--list").stdout, "")
+        self.assertEqual(self.run_wrapper("12").returncode, 3)
+
+    def test_a_watchdog_probe_that_never_finishes_blocks_the_reset(self):
+        self.script(self.root / "bin/systemctl", f'echo "systemctl $*" >> {self.calls}; [ "$1" != show ] || echo activating')
+        self.script(self.root / "bin/sleep", "exit 0")
+        self.assertEqual(self.run_wrapper("12").returncode, 4)
+        self.assertNotIn("systemctl stop flysim.service", self.log())
+        self.assertIn("systemctl start fly-watchdog.timer", self.log())
+
     def test_bad_arguments_are_refused(self):
         for args in ((), ("--check", "12"), ("12", "13"), ("../12",), ("123",), ("-1",)):
             self.assertEqual(self.run_wrapper(*args).returncode, 2, args)