fleetd #504 item 1: stop the false ok on the loaded-but-not-running stop path #541

2026-09-12T08:43:21+02:00

agent commented

2026-09-12 08:43:21 +02:00

Item 1 of fleetd #504 only. Items 2-4 stay open.

Defect: the two stop branches for "loaded but not currently running" (launchd/systemd, reached when $OLD_PID is empty) ran launchctl unload/systemctl --user stop with 2>/dev/null || true and printed ok unconditionally. A real supervisor failure (e.g. launchd or the systemd user bus unreachable) read exactly like a harmless already-stopped answer, so the script proceeded to start believing nothing was loaded -- the two-daemons failure fleetd #492 exists to prevent.

Fix: two new functions, unload_launchd_if_loaded / stop_systemd_if_loaded (added right after systemd_loaded), apply that function's own pattern to the write side: capture stderr separately into a temp file, and only a non-zero exit WITH stderr content is a real failure (dies with the captured message); a non-zero exit with empty stderr is the genuine already-stopped/unloaded answer and is tolerated. No ${VAR:-default} anywhere -- the script runs under set -euo pipefail. The two call sites in the main flow now call these functions instead of the bare || true.

Tests (5 new, scripts/test-redeploy-fleetd.sh): dies-on-real-failure and tolerates-clean-negative for each function (using a stub launchctl/systemctl first on PATH, same technique the existing systemd-probe-error test uses), plus a source-text check that the main flow calls the new functions and the bare 2>/dev/null || true defect has not come back.

Verification:

bash scripts/test-redeploy-fleetd.sh: exit 0, anchored ^FAIL: count 0 (3 unanchored FAIL: lines are the suite's own internal mutation-cell fixtures, unrelated to this change), ends PASS: redeploy log classifier.
bash -n clean under both /bin/bash (3.2.57) and env bash (5.3.9).
Defined-vs-invoked test functions: 65/65, comm -3 empty (60 baseline + 5 new).
Each of the 5 new tests verified by mutation: reintroducing the original swallow-everything defect, and separately over-correcting to die unconditionally on any non-zero exit (breaking the genuine already-stopped case), and reverting the call sites to the bare || true. Each mutation made its own test go red with its own message (verified independently for both halves of the call-site test); each was then restored to a byte-identical file (full shasum -a 256 match) and a green control run followed.

Never ran redeploy-fleetd.sh itself against the live daemon (per instructions) -- all verification is via bash -n and the sourced test suite.

Item 1 of fleetd #504 only. Items 2-4 stay open. **Defect**: the two `stop` branches for "loaded but not currently running" (`launchd`/`systemd`, reached when `$OLD_PID` is empty) ran `launchctl unload`/`systemctl --user stop` with `2>/dev/null || true` and printed `ok` unconditionally. A real supervisor failure (e.g. launchd or the systemd user bus unreachable) read exactly like a harmless already-stopped answer, so the script proceeded to start believing nothing was loaded -- the two-daemons failure fleetd #492 exists to prevent. **Fix**: two new functions, `unload_launchd_if_loaded` / `stop_systemd_if_loaded` (added right after `systemd_loaded`), apply that function's own pattern to the write side: capture stderr separately into a temp file, and only a non-zero exit WITH stderr content is a real failure (dies with the captured message); a non-zero exit with empty stderr is the genuine already-stopped/unloaded answer and is tolerated. No `${VAR:-default}` anywhere -- the script runs under `set -euo pipefail`. The two call sites in the main flow now call these functions instead of the bare `|| true`. **Tests** (5 new, scripts/test-redeploy-fleetd.sh): dies-on-real-failure and tolerates-clean-negative for each function (using a stub `launchctl`/`systemctl` first on PATH, same technique the existing systemd-probe-error test uses), plus a source-text check that the main flow calls the new functions and the bare `2>/dev/null || true` defect has not come back. **Verification**: - `bash scripts/test-redeploy-fleetd.sh`: exit 0, anchored `^FAIL:` count 0 (3 unanchored FAIL: lines are the suite's own internal mutation-cell fixtures, unrelated to this change), ends `PASS: redeploy log classifier`. - `bash -n` clean under both `/bin/bash` (3.2.57) and `env bash` (5.3.9). - Defined-vs-invoked test functions: 65/65, `comm -3` empty (60 baseline + 5 new). - Each of the 5 new tests verified by mutation: reintroducing the original swallow-everything defect, and separately over-correcting to die unconditionally on any non-zero exit (breaking the genuine already-stopped case), and reverting the call sites to the bare `|| true`. Each mutation made its own test go red with its own message (verified independently for both halves of the call-site test); each was then restored to a byte-identical file (full `shasum -a 256` match) and a green control run followed. Never ran redeploy-fleetd.sh itself against the live daemon (per instructions) -- all verification is via bash -n and the sourced test suite.

agent added 1 commit 2026-09-12 08:43:21 +02:00

fleetd #504 item 1: stop swallowing real launchctl/systemctl failures on the 'loaded but not running' path

CI / contract (pull_request) Successful in 52s

Details

CI / build (pull_request) Successful in 1m55s

Details

e4eb3dbed4

The two 'loaded but not currently running' branches in the stop step (launchd/systemd, reached
when $OLD_PID is empty) ran 'launchctl unload'/'systemctl --user stop' with '2>/dev/null || true'
and printed 'ok' unconditionally. That swallowed a real supervisor failure (e.g. launchd or the
systemd user bus unreachable) exactly like a harmless already-stopped answer, and let the script
proceed to start a new daemon believing nothing was loaded -- the two-daemons failure fleetd #492
exists to prevent.

Adds unload_launchd_if_loaded/stop_systemd_if_loaded, applying systemd_loaded's own pattern
(capture stderr separately; a non-zero exit WITH stderr is a real failure, a non-zero exit with
empty stderr is a clean already-stopped answer) to the write side. The two call sites now use
these functions instead of the bare '|| true'.

Adds 5 tests: dies-on-real-failure and tolerates-clean-negative for each function, plus a
source-text check that the main flow calls the new functions instead of the original bare
'2>/dev/null || true'. All 5 verified by mutation (reintroducing the swallow, and separately
over-correcting to die unconditionally) -- each goes red with its own message, restores
byte-identical (full sha256), and passes a green control.

ltms merged commit 7611b69667 into main

2026-09-12 09:09:00 +02:00

ltms referenced this pull request

2026-09-12 09:11:45 +02:00

redeploy-fleetd.sh cannot run on Linux at all: every `mktemp -t NAME` fails under GNU coreutils, and the script blames the systemd bus for it #545

ltms referenced this pull request

2026-09-12 09:12:08 +02:00

redeploy-fleetd.sh: four places where a failed command is reported as a clean result #504

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: fleet/fleetd#541