usb-ready: wait for a queued unit, not just a running one

bin/prod/usb-ready

@@ -36,8 +36,36 @@ fi
 
 state=$(systemctl show -p ActiveState --value "$UNIT" 2>/dev/null || echo "unknown")
 cond=$(systemctl show -p ConditionResult --value "$UNIT" 2>/dev/null || echo "unknown")
+# The job, if systemd still has one for this unit. This is the only property
+# that separates "queued, about to run" from "dispatched and skipped":
+#
+#   case                       ActiveState  ConditionResult  Job
+#   skipped by its condition   inactive     no               (empty)
+#   queued, not started yet    inactive     no               12561
+#   ExecStart running          activating   yes              12561
+#   finished                   active       yes              (empty)
+#
+# Measured on systemd 257 (Debian 13, the board's own) and 237 - the first two
+# rows are identical in every other property, including ConditionTimestamp and
+# ExecMainStartTimestamp, which are 0 for both.
+#
+# That mattered: reading only ActiveState and ConditionResult, this said "true"
+# in the queued window, Reflash mounted /dev/sda2 while expand-usb's mkfs was
+# still running inside ssh-keygen-boot, and the mount failed with "bad
+# superblock" - the filesystem did not exist yet. Storage was marked FAILED and
+# no host keys were written, so the board came up with no sshd (#147).
+job=$(systemctl show -p Job --value "$UNIT" 2>/dev/null | awk '{print $1}')
 
 case "$state" in
     active|failed) echo "true" ;;                       # ran to completion
-    *) [ "$cond" = "no" ] && echo "true" || echo "false" ;;  # skipped, or not yet
+    activating)    echo "false" ;;                      # running right now
+    *)
+        if [ -n "$job" ]; then
+            echo "false"                                # queued: it has not run yet
+        elif [ "$cond" = "no" ]; then
+            echo "true"                                 # dispatched and skipped
+        else
+            echo "false"
+        fi
+        ;;
 esac

test/bats/getters.bats

@@ -45,14 +45,34 @@ teardown() { teardown_sandbox; }
 # Reflash holds /mnt/usb for the life of the process, so it waits for whatever
 # needs the drive read-write to finish rather than racing it.
 
-# systemctl show -p X --value is called twice: ActiveState then ConditionResult.
+# systemctl show -p X --value, for the three properties usb-ready reads:
+# ActiveState, ConditionResult and Job.
+#
+# WHAT A BOARD ACTUALLY REPORTS. Stub these from the table, not from what seems
+# reasonable - the previous version of this helper guessed ConditionResult=yes
+# for a unit that had not started, systemd says "no", and the test passed for
+# years while the real case failed (#147):
+#
+#   case                       ActiveState  ConditionResult  Job
+#   skipped by its condition   inactive     no               (empty)
+#   queued, not started yet    inactive     no               12561
+#   ExecStart running          activating   yes              12561
+#   finished                   active       yes              (empty)
+#
+# Measured on systemd 257 (Debian 13, what the board runs) and 237, with a
+# probe unit of the same shape as ssh-keygen-boot: Type=oneshot,
+# RemainAfterExit=yes, a ConditionPathExists that passes or fails. The first two
+# rows differ in Job and in nothing else - ConditionTimestamp and
+# ExecMainStartTimestamp are 0 for both - which is why Job is the property
+# usb-ready has to read.
 stub_unit() {
   cat > "$SHIMDIR/systemctl" <<EOF
 #!/usr/bin/env bash
 echo "systemctl \$*" >> "$CALLS"
 case "\$*" in
   *ConditionResult*) echo "${2:-yes}" ;;
   *ActiveState*)     echo "${1:-inactive}" ;;
+  *Job*)             echo "${3:-}" ;;
 esac
 EOF
   chmod +x "$SHIMDIR/systemctl"
@@ -74,10 +94,20 @@ with_partition() {
 }
 
 # The race this closes: a oneshot reads "inactive" before it starts as well as
-# after a Condition skips it. Mounting in that gap let the unit steal the mount.
+# after a Condition skips it, and ConditionResult is "no" in BOTH - this test
+# used to stub it as "yes", which is not what systemd reports in that window, so
+# it passed while the real case failed (#147). Only the queued job tells them
+# apart.
 @test "usb-ready: false while the owning unit has not started yet" {
   with_partition
-  stub_unit inactive yes
+  stub_unit inactive no 12561
+  run "$PROD_BIN/usb-ready"
+  [ "$output" = "false" ]
+}
+
+@test "usb-ready: false while the owning unit is queued behind something else" {
+  with_partition
+  stub_unit inactive yes 12561
   run "$PROD_BIN/usb-ready"
   [ "$output" = "false" ]
 }
@@ -98,9 +128,10 @@ with_partition() {
   [ "$output" = "true" ]
 }
 
+# Skipped means dispatched and decided: no job is left for it.
 @test "usb-ready: true when the owning unit was skipped by its condition" {
   with_partition
-  stub_unit inactive no
+  stub_unit inactive no ""
   run "$PROD_BIN/usb-ready"
   [ "$output" = "true" ]
 }