Compare commits

..

1 Commits

Author SHA1 Message Date
Dai Ha d105da978d fleetd #680: pin the JAR/BUILD_JAR split, check the plist's jar path, and widen the daemon-locator pattern
CI / shell-tests (pull_request) Failing after 7s
CI / contract (pull_request) Successful in 1m5s
CI / build (pull_request) Failing after 2m28s
Part 1: asserts JAR and BUILD_JAR as the script sources them (no test assigns
them first), so reverting JAR to a path under target/ now fails the suite.

Part 2: check_jar_path_matches_plist reads the installed launchd plist's
ProgramArguments and refuses when its jar path does not resolve to $JAR,
mirroring check_log_path_matches_plist. Wired into report_supervisor_state's
launchd branch; unaffected when no plist is installed.

Part 3 (added to the ticket after the brief, by comment): PATTERN narrowed to
'fleetd.jar' so running_pid()/assert_single_daemon see a daemon regardless of
which build layout (run/ or target/) its jar sits under. The comm=java
allowlist still excludes a self-matching shell. Brought
.claude/skills/fleets-status/SKILL.md's pgrep pattern into agreement.

Verified: bash scripts/test-redeploy-fleetd.sh exits 0. Mutation both
directions for part 1 (JAR under target/ -> suite fails; JAR elsewhere ->
suite passes), a positive control for part 2 (neutralizing the mismatch
check makes the new test fail), and a positive control for part 3 (narrowing
PATTERN back to run/fleetd.jar makes the new target/-dir test fail). mvn -o
clean install: Tests run: 1929, Failures: 0, Errors: 0, Skipped: 0, BUILD
SUCCESS, 172 surefire report files.
2026-10-03 21:28:59 +02:00
5 changed files with 170 additions and 56 deletions
+1 -1
View File
@@ -59,7 +59,7 @@ as `matches HEAD`, `drift`, or `unknown`; do not turn an unclear timestamp into
Report the process identifier (PID) and uptime too:
```bash
PIDS="$(pgrep -f 'run/fleetd.jar' || true)"
PIDS="$(pgrep -f 'fleetd.jar' || true)"
if [ -z "$PIDS" ]; then
printf '%s\n' 'fleetd: not running'
else
@@ -1399,13 +1399,11 @@ public record FleetConfig(
* called FROM the calling lead's own turn, so its pane is still {@code WORKING} the instant
* {@code confirm()} validates every gate and schedules the roll. {@code
* dev.ltms.fleet.lead.LeadRollover}'s deferred continuation waits up to this many seconds for
* that SAME pane to report {@code IDLE} or {@code DONE} — i.e. for the calling turn to actually
* end — before it sends {@code /clear} at all. {@code BLOCKED} does not count: that is a live
* turn merely paused, not one that has finished. If that wait times out, no {@code /clear} is
* that SAME pane to report an injectable state again — i.e. for the calling turn to actually
* end — before it sends {@code /clear} at all. If that wait times out, no {@code /clear} is
* ever sent: a lead that never goes idle is still doing real work, and clearing it would
* destroy live context. This is a separate wait from {@code clearSettleSeconds} below, which
* bounds the SECOND wait, for the pane to reach {@code IDLE} or {@code DONE} again AFTER
* {@code /clear} has already gone out.
* bounds the SECOND wait, for the pane to re-settle AFTER {@code /clear} has already gone out.
*
* @param handoverPath required when this block is present — where the handover file a fresh
* lead session reads must live. There is no sane non-null default for an
@@ -1424,13 +1422,12 @@ public record FleetConfig(
* @param maxDocAgeSeconds default 3600 — refuse a handover file whose modified time is older
* than this many seconds, so a stale leftover from an earlier rollover
* attempt can never be mistaken for a fresh one.
* @param turnSettleSeconds default 300 — bound on how long the deferred roll waits for the
* CALLING lead's own turn to end (its pane to report {@code IDLE} or
* {@code DONE}) before sending {@code /clear} at all. See the paragraph
* above.
* @param turnSettleSeconds default 20 — bound on how long the deferred roll waits for the
* CALLING lead's own turn to end (its pane to report injectable again)
* before sending {@code /clear} at all. See the paragraph above.
* @param clearSettleSeconds default 20 — bound on how long to wait for the lead's pane to
* report {@code IDLE} or {@code DONE} again after {@code /clear} before
* giving up. A roll that times out here never sends {@code bootstrapText}.
* report an injectable state again after {@code /clear} before giving up. A
* roll that times out here never sends {@code bootstrapText}.
* @param bootstrapText default a sentence naming the RESOLVED handover path — sent to the
* lead's pane once it settles after {@code /clear}, telling the fresh
* session where to read the handover and carry on. Left {@code null} here
@@ -1447,7 +1444,7 @@ public record FleetConfig(
public LeadRollover {
requireOperatorConfirm = requireOperatorConfirm == null || requireOperatorConfirm;
maxDocAgeSeconds = (maxDocAgeSeconds == null || maxDocAgeSeconds <= 0) ? 3600 : maxDocAgeSeconds;
turnSettleSeconds = (turnSettleSeconds == null || turnSettleSeconds <= 0) ? 300 : turnSettleSeconds;
turnSettleSeconds = (turnSettleSeconds == null || turnSettleSeconds <= 0) ? 20 : turnSettleSeconds;
clearSettleSeconds = (clearSettleSeconds == null || clearSettleSeconds <= 0) ? 20 : clearSettleSeconds;
bootstrapText = (bootstrapText == null || bootstrapText.isBlank()) ? null : bootstrapText;
}
@@ -3175,41 +3175,4 @@ class FleetConfigTest {
FleetConfig cfg = FleetConfig.load(f);
assertTrue(cfg.models().offIds().isEmpty());
}
// ── fleetd #651: leadRollover.turnSettleSeconds default resolution ─────────────────────────
@Test
void turnSettleSecondsDefaultsTo300WhenUnset(@TempDir Path dir) throws Exception {
Path f = dir.resolve("bare-rollover.yaml");
Files.writeString(f, "bind:\n port: 8080\nleadRollover: {}\n");
FleetConfig.LeadRollover rollover = FleetConfig.load(f).leadRollover();
assertNotNull(rollover);
assertEquals(300, rollover.turnSettleSeconds());
}
@Test
void turnSettleSecondsUsesAnExplicitPositiveValue(@TempDir Path dir) throws Exception {
Path f = dir.resolve("rollover.yaml");
Files.writeString(f, """
bind:
port: 8080
leadRollover:
turnSettleSeconds: 45
""");
FleetConfig.LeadRollover rollover = FleetConfig.load(f).leadRollover();
assertEquals(45, rollover.turnSettleSeconds());
}
@Test
void turnSettleSecondsFallsBackTo300WhenZeroOrNegative(@TempDir Path dir) throws Exception {
Path zero = dir.resolve("zero.yaml");
Files.writeString(zero, "bind:\n port: 8080\nleadRollover:\n turnSettleSeconds: 0\n");
assertEquals(300, FleetConfig.load(zero).leadRollover().turnSettleSeconds());
Path negative = dir.resolve("negative.yaml");
Files.writeString(negative, "bind:\n port: 8080\nleadRollover:\n turnSettleSeconds: -5\n");
assertEquals(300, FleetConfig.load(negative).leadRollover().turnSettleSeconds());
}
}
+47 -6
View File
@@ -81,11 +81,12 @@ MODULE="$REPO/fleetd"
BUILD_JAR="$MODULE/target/fleetd.jar"
JAR="$MODULE/run/fleetd.jar"
OUT="$MODULE/fleetd.out"
# Matches BOTH the absolute form and the relative `java -jar run/fleetd.jar` a hand-start
# produces from inside fleetd/. Anchoring on the absolute path alone was a real bug: the daemon
# restarted correctly and the script still reported "no process appeared", because it launched with
# a relative path and then looked for an absolute one.
PATTERN='run/fleetd.jar'
# Matches a fleetd daemon's command line wherever its jar sits — absolute or relative, under
# run/, under target/, or anywhere else a build or a hand-start might point it. Detecting a
# daemon this script did not start, including one running from a jar outside $JAR's own
# directory, is this pattern's whole job; running_pid()'s `comm = java` allowlist below is what
# keeps that breadth from counting a shell that merely types the pattern as literal text.
PATTERN='fleetd.jar'
HEALTH='http://127.0.0.1:8765/healthz'
STOP_WAIT=30 # seconds to wait for a clean exit before reporting failure
HEALTH_WAIT=60 # seconds to wait for /healthz to answer after start — fleetd #603: also the pid-
@@ -231,7 +232,7 @@ report_jar_state() {
# launched as `java -jar ...` — a native image, a renamed launcher — `running_pid()` silently
# returns nothing and `assert_single_daemon` stops noticing a second daemon at all. For a guard,
# that false-negative direction is the worse one to be wrong in. This is not a new assumption,
# though: `PATTERN='run/fleetd.jar'` two lines up already assumes the daemon is a jar, which
# though: `PATTERN='fleetd.jar'` two lines up already assumes the daemon is a jar, which
# is only ever run by `java`. If that launch method changes, `PATTERN` stops matching anything
# before this allowlist would ever get the chance to be wrong — the allowlist rides on the same
# assumption that is already load-bearing, it does not add a new one. Whoever changes the launch
@@ -627,6 +628,45 @@ check_log_path_matches_plist() {
ok "log path check: script and plist agree ($resolved_out)"
}
# Reads the launchd plist's ProgramArguments for the argument that follows "-jar", resolves it
# alongside $jar_path, and dies when the two differ. Call it only when the agent is loaded; it
# never touches launchd or the daemon itself.
check_jar_path_matches_plist() {
local jar_path="$1" plist_path="$2"
local plist_args plist_jar resolved_jar resolved_plist_jar
if ! plist_args="$(/usr/libexec/PlistBuddy -c 'Print :ProgramArguments' "$plist_path" 2>/dev/null)"; then
die "launchd agent is loaded but PlistBuddy could not read ProgramArguments from
$plist_path
— cannot verify which jar the supervised daemon launches. Fix the plist before
redeploying supervised."
fi
plist_jar="$(printf '%s\n' "$plist_args" | awk '
{ gsub(/^[ \t]+|[ \t]+$/, "") }
prev == "-jar" { print; exit }
{ prev = $0 }
')"
if [ -z "$plist_jar" ]; then
die "launchd agent is loaded but its ProgramArguments at
$plist_path
do not contain a '-jar <path>' pair — cannot verify which jar the supervised daemon
launches. Fix the plist before redeploying supervised."
fi
resolved_jar="$(cd "$(dirname "$jar_path")" 2>/dev/null && pwd -P)/$(basename "$jar_path")" || true
resolved_plist_jar="$(cd "$(dirname "$plist_jar")" 2>/dev/null && pwd -P)/$(basename "$plist_jar")" || true
if [ -z "$resolved_jar" ] || [ -z "$resolved_plist_jar" ] || [ "$resolved_jar" != "$resolved_plist_jar" ]; then
die "jar path mismatch — this script deploys to
$jar_path (resolved: ${resolved_jar:-<directory does not exist>})
but the loaded plist's ProgramArguments names
$plist_jar (resolved: ${resolved_plist_jar:-<directory does not exist>})
The swap renames the built jar into place, so the old path stops existing after a redeploy;
a launchd-initiated start from this plist (a reboot, or KeepAlive after a crash) would then
run java against a missing file. Reinstall the plist at
$plist_path
so its ProgramArguments names $jar_path before redeploying supervised."
fi
ok "jar path check: script and plist agree ($resolved_jar)"
}
# 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
@@ -960,6 +1000,7 @@ report_supervisor_state() {
SUPERVISED=1
ok "launchd agent loaded ($LAUNCHD_LABEL) — launchd supervises this daemon"
check_log_path_matches_plist "$OUT" "$LAUNCHD_PLIST"
check_jar_path_matches_plist "$JAR" "$LAUNCHD_PLIST"
;;
systemd)
SUPERVISED=1
+113
View File
@@ -492,6 +492,38 @@ test_running_pid_finds_a_real_java_named_second_process() {
|| fail "running_pid() did not find a real second process (pid $standin_pid, comm forced to 'java' via exec -a) whose own argv holds the pattern: before=[$before] after=[$after]"
}
# PATTERN matches a fleetd jar in either build layout, not only the run/ one: a process whose
# argv names a jar under target/ must be found too, the same way the run/ case above is.
test_running_pid_finds_a_real_java_named_process_from_target_dir() {
local before after standin_pid
before="$(running_pid)"
( exec -a java sh -c 'echo "target/fleetd.jar" >/dev/null; sleep 20' ) &
standin_pid=$!
sleep 0.3
after="$(running_pid)"
kill "$standin_pid" 2>/dev/null || true
wait "$standin_pid" 2>/dev/null || true
printf '%s\n' "$after" | grep -qxF "$standin_pid" \
|| fail "running_pid() did not find a real second process (pid $standin_pid, comm forced to 'java' via exec -a) naming a jar under target/: before=[$before] after=[$after]"
}
# Broadening PATTERN to match both build layouts must not also broaden it into matching a
# non-exec'ing shell that merely holds the target/ text as a literal argument, the same
# self-matching shape test_running_pid_excludes_self_matching_wrapper_shell above excludes for
# the run/ text.
test_running_pid_excludes_self_matching_wrapper_shell_naming_target_dir() {
local before after wrapper_pid
before="$(running_pid)"
sh -c 'echo "target/fleetd.jar" >/dev/null; sleep 20' &
wrapper_pid=$!
sleep 0.3
after="$(running_pid)"
kill "$wrapper_pid" 2>/dev/null || true
wait "$wrapper_pid" 2>/dev/null || true
[ "$after" = "$before" ] \
|| fail "running_pid() counted a self-matching wrapper shell (pid $wrapper_pid, holding 'target/fleetd.jar' as literal text in its own argv, not the daemon): before=[$before] after=[$after]"
}
# fleetd #593 CORRECTION 1, hole 2 — the round-1 filter denied known shell names (sh/bash/zsh/
# dash/ksh) and counted everything else. `ssh`, `perl`, `python3`, `ruby`, `tail` — anything not on
# that list, carrying the pattern in its own argv — was still counted right alongside the real
@@ -702,6 +734,19 @@ test_report_jar_state_both_absent_is_not_a_mismatch() {
fi
}
# Reads JAR and BUILD_JAR exactly as the script sources them, with nothing here assigning
# either first. JAR must resolve outside $MODULE/target/, and JAR must differ from BUILD_JAR:
# the daemon's live path and Maven's own build output are never the same file.
test_jar_and_build_jar_are_sourced_outside_target_and_differ() {
source "$ROOT/scripts/redeploy-fleetd.sh"
case "$JAR" in
"$MODULE"/target/*)
fail "\$JAR must not live under \$MODULE/target/ — got $JAR" ;;
esac
[ "$JAR" != "$BUILD_JAR" ] \
|| fail "\$JAR and \$BUILD_JAR must not be the same path — got $JAR"
}
# fleetd #493/#664 — never build into the path a running process holds. swap_staged_jar is
# exercised directly against real files on disk (not stubs), because the whole point is file
# behavior (does the content move, does the source disappear, does a failure leave both sides
@@ -1305,12 +1350,71 @@ test_run_drain_gate_declined_reply_refuses() {
source "$ROOT/scripts/redeploy-fleetd.sh"
}
# Writes a launchd-plist fixture naming jar_path as the ProgramArguments entry after "-jar", so
# check_jar_path_matches_plist has something real to read back.
write_launchd_plist_fixture() {
local path="$1" jar_path="$2"
cat > "$path" <<PLIST
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>Label</key>
<string>test.fixture</string>
<key>ProgramArguments</key>
<array>
<string>/usr/bin/java</string>
<string>-jar</string>
<string>$jar_path</string>
<string>fleetd.yaml</string>
</array>
</dict>
</plist>
PLIST
}
# Agreeing case: a plist whose ProgramArguments names the same jar, resolved, must proceed and
# say so, never die.
test_check_jar_path_matches_plist_agrees_ok() {
local dir jar plist output rc=0
dir="$TMP/jar-path-agree"; mkdir -p "$dir/run"
jar="$dir/run/fleetd.jar"
plist="$dir/agree.plist"
write_launchd_plist_fixture "$plist" "$jar"
output="$(check_jar_path_matches_plist "$jar" "$plist" 2>&1)" || rc=$?
[ "$rc" -eq 0 ] \
|| fail "check_jar_path_matches_plist must succeed when the plist names the same jar: $output"
printf '%s' "$output" | grep -qF 'jar path check' \
|| fail "check_jar_path_matches_plist did not print the agreement line: $output"
}
# The disagreeing case, and the positive control this check exists for: a plist naming a
# different jar must die, naming both paths.
test_check_jar_path_matches_plist_mismatch_dies() {
local dir jar other_jar plist output rc=0
dir="$TMP/jar-path-mismatch"; mkdir -p "$dir/run" "$dir/target"
jar="$dir/run/fleetd.jar"
other_jar="$dir/target/fleetd.jar"
plist="$dir/mismatch.plist"
write_launchd_plist_fixture "$plist" "$other_jar"
output="$(check_jar_path_matches_plist "$jar" "$plist" 2>&1)" || rc=$?
[ "$rc" -ne 0 ] \
|| fail "check_jar_path_matches_plist must die when the plist names a different jar"
printf '%s' "$output" | grep -qF "$jar" \
|| fail "die message does not name this script's jar path: $output"
printf '%s' "$output" | grep -qF "$other_jar" \
|| fail "die message does not name the plist's jar path: $output"
}
# fleetd #555 item 4 — the report-state dispatch on $SUPERVISOR_KIND. Inverting this used to report
# the wrong supervisor and, for the launchd arm specifically, skip check_log_path_matches_plist.
CHECK_LOG_PATH_CALLED=0
CHECK_JAR_PATH_CALLED=0
stub_check_log_path_recorder() {
CHECK_LOG_PATH_CALLED=0
CHECK_JAR_PATH_CALLED=0
check_log_path_matches_plist() { CHECK_LOG_PATH_CALLED=1; }
check_jar_path_matches_plist() { CHECK_JAR_PATH_CALLED=1; }
}
test_report_supervisor_state_launchd_sets_supervised_and_checks_log_path() {
@@ -1321,6 +1425,8 @@ test_report_supervisor_state_launchd_sets_supervised_and_checks_log_path() {
[ "$SUPERVISED" = 1 ] || fail "report_supervisor_state launchd must set SUPERVISED=1"
[ "$CHECK_LOG_PATH_CALLED" = 1 ] \
|| fail "report_supervisor_state launchd must call check_log_path_matches_plist"
[ "$CHECK_JAR_PATH_CALLED" = 1 ] \
|| fail "report_supervisor_state launchd must call check_jar_path_matches_plist"
source "$ROOT/scripts/redeploy-fleetd.sh"
}
@@ -1332,6 +1438,8 @@ test_report_supervisor_state_systemd_sets_supervised_without_log_path_check() {
[ "$SUPERVISED" = 1 ] || fail "report_supervisor_state systemd must set SUPERVISED=1"
[ "$CHECK_LOG_PATH_CALLED" = 0 ] \
|| fail "report_supervisor_state systemd must NOT call check_log_path_matches_plist"
[ "$CHECK_JAR_PATH_CALLED" = 0 ] \
|| fail "report_supervisor_state systemd must NOT call check_jar_path_matches_plist"
source "$ROOT/scripts/redeploy-fleetd.sh"
}
@@ -2189,6 +2297,8 @@ test_assert_single_daemon_accepts_one_pid
test_assert_single_daemon_rejects_two_pids
test_running_pid_excludes_self_matching_wrapper_shell
test_running_pid_finds_a_real_java_named_second_process
test_running_pid_finds_a_real_java_named_process_from_target_dir
test_running_pid_excludes_self_matching_wrapper_shell_naming_target_dir
test_running_pid_drops_a_pid_whose_comm_is_not_java
test_running_pid_drops_a_pid_that_exited_before_the_comm_lookup
test_running_pid_counts_a_pid_whose_comm_is_java
@@ -2200,6 +2310,7 @@ test_jar_id_reports_unhashable_when_no_hasher_on_path
test_report_jar_state_agrees_when_hashes_match
test_report_jar_state_warns_when_hashes_differ
test_report_jar_state_both_absent_is_not_a_mismatch
test_jar_and_build_jar_are_sourced_outside_target_and_differ
test_swap_staged_jar_moves_staged_onto_live
test_swap_staged_jar_dies_without_staged_file
test_swap_staged_jar_dies_when_mv_fails
@@ -2241,6 +2352,8 @@ test_drain_confirmed_false_on_anything_else
test_run_drain_gate_skips_prompt_when_not_required
test_run_drain_gate_confirmed_reply_does_not_refuse
test_run_drain_gate_declined_reply_refuses
test_check_jar_path_matches_plist_agrees_ok
test_check_jar_path_matches_plist_mismatch_dies
test_report_supervisor_state_launchd_sets_supervised_and_checks_log_path
test_report_supervisor_state_systemd_sets_supervised_without_log_path_check
test_report_supervisor_state_none_leaves_supervised_zero