fleetd #552: warn instead of aborting when the post-restart mktemp fails

By the time the fresh-log mktemp ran, the daemon had already been stopped, the
jar swapped, and the new daemon started — an unguarded mktemp failure there
aborted the whole script anyway, so a caller read the resulting non-zero exit
as "the redeploy failed" and would restart an already-correctly-restarted
daemon.

Extract the mktemp into capture_fresh_log_region, guarded the same way
unload_launchd_if_loaded/stop_systemd_if_loaded guard their own, but warn
instead of die: there is nothing left to protect by refusing after a
successful restart. The trap is now installed before the assignment it cleans
up, using an FRESH_LOG="" sentinel readers can check.

classify_amqp_connection_errors and report_shutdown_drain both gain a new
state (REDEPLOY_AMQP_CHECK_SKIPPED / REDEPLOY_DRAIN_STATE=skipped) for an
uncapturable log region, distinct from "captured a region with nothing in
it" — and the result section gains a matching branch, so a skipped capture
can never read as a clean bill of health.

scripts/redeploy-fleetd.sh

@@ -549,14 +549,51 @@ check_log_path_matches_plist() {
   ok "log path check: script and plist agree ($resolved_out)"
 }
 
+# fleetd #552: the post-restart fresh-log capture, pulled out of the main flow so it is testable by
+# sourcing (the same reason systemd_installed/systemd_loaded above guard their OWN mktemp inline
+# instead of leaving it bare) even though its only caller sits below the SOURCED guard. By the time
+# the caller reaches this, the daemon has already been stopped, the jar swapped, and the new daemon
+# started — fleetd #552's whole point is that a `mktemp` failure here must never undo that success
+# by aborting the script, the way the old unguarded `FRESH_LOG="$(mktemp ...)"` assignment did.
+#
+# So this function never dies, and — like detect_supervisor above — never reports failure through a
+# global either: it is meant to be called as `VAR="$(capture_fresh_log_region ...)"`, which runs it
+# in a subshell, and any global it set there would die with that subshell. Failure crosses back the
+# only way that survives: a non-zero exit and empty stdout. The caller decides what a failure means
+# (warn, and leave FRESH_LOG at its already-declared "" sentinel) and owns the EXIT trap, because
+# that trap has to outlive this function's own return — the file must survive until every reader
+# below (classify_amqp_connection_errors, report_shutdown_drain, the result section) has looked.
+capture_fresh_log_region() {
+  local out_file="$1" restart_mark="$2" log_file
+  if ! log_file="$(mktemp -t fleetd-fresh-log.XXXXXX)"; then
+    return 1
+  fi
+  tail -n "+$((restart_mark + 1))" "$out_file" > "$log_file" 2>/dev/null || true
+  printf '%s' "$log_file"
+}
+
 # Classify ERROR lines in one fresh log region. AMQP failure messages now include the connection
 # name, so a recovery can clear only errors for its own connection. A candidate with neither name
 # remains unexplained: it must never be quieted by a recovery on the other connection.
+#
+# fleetd #552: log_file can now arrive as "" — capture_fresh_log_region's own sentinel for "the
+# post-restart region could never be captured". That is a DIFFERENT fact from "captured a region
+# with nothing in it", which is exactly what every count below staying at its initialized 0 already
+# means; conflating the two would make "no errors found" and "could not look for errors" print
+# identically; the exact defect class this ticket exists to fix. REDEPLOY_AMQP_CHECK_SKIPPED carries
+# the distinction to the result section below, and this reader says so in its own output too.
 classify_amqp_connection_errors() {
   local log_file="$1" line pending_inbox=0 pending_lead_mailbox=0
   REDEPLOY_ERROR_COUNT=0
   REDEPLOY_RECOVERED_AMQP_ERRORS=0
   REDEPLOY_UNEXPLAINED_ERRORS=0
+  REDEPLOY_AMQP_CHECK_SKIPPED=0
+
+  if [ -z "$log_file" ]; then
+    REDEPLOY_AMQP_CHECK_SKIPPED=1
+    warn "cannot check for AMQP/ERROR lines since restart — the post-restart log region could not be captured"
+    return 0
+  fi
 
   while IFS= read -r line || [ -n "$line" ]; do
     case "$line" in
@@ -680,6 +717,19 @@ report_shutdown_drain() {
     return 0
   fi
 
+  # fleetd #552: checked AFTER the n/a case above, not before it — a cold start has nothing to check
+  # regardless of whether the log region could be captured, and that fact must not be overridden by
+  # an unrelated mktemp failure. "skipped" is a fifth state on REDEPLOY_DRAIN_STATE, distinct from
+  # "unknown" (the log WAS read, and neither a drain-complete line nor an exception shape was found
+  # in it): here there was no log to read at all, and that is a different fact that must say so in
+  # its own output, never silently reported as "unknown" or, worse, as "complete"/"n/a".
+  if [ -z "$log_file" ]; then
+    REDEPLOY_DRAIN_STATE="skipped"
+    warn "cannot tell whether the previous daemon's shutdown drain finished — the post-restart log"
+    warn "region could not be captured"
+    return 0
+  fi
+
   find_drain_complete_line "$log_file"
   scan_uncaught_exceptions "$log_file"
 
@@ -1110,9 +1160,24 @@ tail -n "+$((RESTART_MARK + 1))" "$OUT" 2>/dev/null \
 
 # Errors since the restart, anchored to the marker so old noise cannot leak in. Keep the fresh
 # region in a file because the classifier must preserve the order of errors and recoveries.
-FRESH_LOG="$(mktemp -t fleetd-fresh-log.XXXXXX)"
+#
+# fleetd #552: by this point the daemon has already been stopped, the jar swapped, and the new
+# daemon started. The old unguarded `FRESH_LOG="$(mktemp ...)"` here aborted the whole script on any
+# mktemp failure — AFTER all of that had already succeeded — so a caller read the resulting non-zero
+# exit as "the redeploy failed" and the likely next action was to restart an already-correctly-
+# restarted daemon. The mktemp itself now lives in capture_fresh_log_region (guarded the same way
+# :293/:317/:344/:359 guard their own), and a failure there only warns: there is nothing left to
+# protect by refusing after a successful restart. FRESH_LOG="" is declared and the trap installed
+# BEFORE the call that may fail to fill it in, not after — an empty FRESH_LOG makes `rm -f ""` a
+# harmless no-op, so there is no ordering hazard left between "the file might already exist" and
+# "the trap that cleans it up".
+FRESH_LOG=""
 trap 'rm -f "$FRESH_LOG"' EXIT
-tail -n "+$((RESTART_MARK + 1))" "$OUT" > "$FRESH_LOG" 2>/dev/null || true
+if ! FRESH_LOG="$(capture_fresh_log_region "$OUT" "$RESTART_MARK")"; then
+  warn "could not create a temp file for the post-restart log region — the restart itself already"
+  warn "succeeded (pid $NEW_PID); only the post-restart ERROR/drain checks below are skipped."
+  warn "Check $OUT yourself for anything since the restart."
+fi
 classify_amqp_connection_errors "$FRESH_LOG"
 
 # fleetd #512 part 2: the previous daemon's shutdown drain, checked in the same fresh-log region —
@@ -1131,7 +1196,14 @@ assert_single_daemon "$(running_pid)"
 
 say "result"
 ok "pid $NEW_PID, jar $(jar_id)"
-if [ "$REDEPLOY_ERROR_COUNT" -eq 0 ]; then
+if [ "$REDEPLOY_AMQP_CHECK_SKIPPED" -eq 1 ]; then
+  # fleetd #552: the fourth reader of the fresh-log region. Without this branch REDEPLOY_ERROR_COUNT
+  # stays at its untouched 0 (classify_amqp_connection_errors never got a file to read) and
+  # REDEPLOY_DRAIN_STATE is "skipped" (never "complete"/"n/a"), so the branches below would print
+  # NOTHING about ERROR lines at all — a silence that reads exactly like the clean bill of health
+  # this ticket exists to prevent. Say the unknown result out loud instead.
+  warn "ERROR-line count since restart is UNKNOWN — the post-restart log region could not be captured (see above)"
+elif [ "$REDEPLOY_ERROR_COUNT" -eq 0 ]; then
   # fleetd #512 item 4: this line must not print when the shutdown-drain check above found the
   # previous daemon's drain died, or could not tell — either would make "no ERROR lines" read as a
   # clean bill of health it is not (an uncaught exception never carries an ERROR token to begin