fleet/fleetd
3
0
Fork 0
Code Issues 73 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

fleetd #492: systemd --user as a third supervisor in redeploy-fleetd.sh #495

Closed
agent wants to merge 0 commits from worker/492-209647-1 into main
pull from: worker/492-209647-1
merge into: fleet:main
Conversation 4 Commits - Files Changed -
agent commented 2026-09-12 04:02:01 +02:00
Member
Copy Link
Copy Source

fleetd #492 — systemd --user as a third supervisor

Problem: scripts/redeploy-fleetd.sh only knew launchd. On a Linux host running fleetd under a systemd --user unit with Restart=on-failure, the script fell into its unsupervised branch: kill "$OLD_PID". Since a SIGTERM'd JVM exits 143 even when its shutdown hook completes cleanly (CB-594), systemd treats that as a failure and revives the OLD jar — racing the script's own start of the NEW jar. Every check the script ran (healthz 200, jar id, fresh "listening" line) was satisfied by EITHER daemon, so the script reported ok while two daemons were running against one herdr session.

Fix:

  • detect_supervisor() now returns one of four answers: launchd, systemd, none, or ambiguous (both signals fire at once — the one case this script genuinely cannot tell apart).
  • require_drivable_supervisor() die()s on ambiguous (or any value detect_supervisor did not return) — it never falls through to kill. This runs in the state-report section, so --check reports the refusal too, without touching anything.
  • The systemd branch stops/starts through systemctl --user stop|start "$SYSTEMD_UNIT", mirroring the existing launchd branch's structure. Unlike launchd's KeepAlive, systemd's Restart=on-failure does not fire on a deliberate stop, so no unload/disable dance is needed there.
  • assert_single_daemon() runs after start+verify and die()s if pgrep -f "$PATTERN" finds more than one pid — the exact symptom this ticket exists to catch, which none of the existing checks can see.
  • launchd behavior is unchanged in effect; --check stays read-only.

Testing seam: detect_supervisor/require_drivable_supervisor/count_daemon_pids/assert_single_daemon are pure functions. launchd_installed/launchd_loaded/systemd_installed/systemd_loaded are simple overridable functions (same pattern as the existing launchd probes) so scripts/test-redeploy-fleetd.sh can simulate launchd-only, systemd-only, neither, and both-at-once without a real launchd or systemd on this (macOS) dev box.

Verification (measured in-worktree, not derived by reading):

  • Baseline (unmodified tree): bash scripts/test-redeploy-fleetd.sh → exit 0, prints the 3 existing mutation-proof lines + PASS: redeploy log classifier (11 test invocations).
  • Final tree: same command → exit 0, same output plus the 8 new tests all silently passing (19 test invocations total).
  • Mutation A (require_drivable_supervisor accepts ambiguous instead of refusing — proved applied via two greps: mutant pattern present, original pattern absent): tests → 1 failure, exit 1: FAIL: require_drivable_supervisor accepted an ambiguous (undrivable) supervisor.
  • Mutation B (assert_single_daemon accepts 2 pids, i.e. -gt 1 → -gt 2, proved applied the same way): tests → 1 failure, exit 1: FAIL: assert_single_daemon accepted two simultaneously running pids.
  • Both mutations reverted; grep confirms the original lines are back verbatim; tests green again as a positive control (exit 0, identical output to the final-tree run).
  • cd fleetd && mvn clean install: BUILD SUCCESS, Tests run: 1677, Failures: 0, Errors: 0, Skipped: 0. (No pom.xml at the repo root — only under fleetd/ — so this ran from the module root per the implementer skill, not literally "the repo root".)

Out of scope, spotted not fixed:

  • A pre-existing dev.ltms.fleet.deploy.SystemdUnitSafetyTest (Java, 8 tests, all passing) already exists in the main build — unrelated to this shell-script ticket, not touched.
  • Per this session's MEMORY.md, ticket #493 describes redeploy-fleetd.sh building into the LIVE target/fleetd.jar path before stopping the daemon — a separate, real defect in this same script, out of scope here and not touched.

No IDE MCP tools were available to me as a worker; verification is bash -n, the shell test suite above, and mvn clean install.

## fleetd #492 — systemd --user as a third supervisor **Problem:** `scripts/redeploy-fleetd.sh` only knew launchd. On a Linux host running fleetd under a `systemd --user` unit with `Restart=on-failure`, the script fell into its unsupervised branch: `kill "$OLD_PID"`. Since a SIGTERM'd JVM exits 143 even when its shutdown hook completes cleanly (CB-594), systemd treats that as a failure and revives the OLD jar — racing the script's own start of the NEW jar. Every check the script ran (healthz 200, jar id, fresh "listening" line) was satisfied by EITHER daemon, so the script reported `ok` while two daemons were running against one herdr session. **Fix:** - `detect_supervisor()` now returns one of four answers: `launchd`, `systemd`, `none`, or `ambiguous` (both signals fire at once — the one case this script genuinely cannot tell apart). - `require_drivable_supervisor()` `die()`s on `ambiguous` (or any value detect_supervisor did not return) — it never falls through to `kill`. This runs in the state-report section, so `--check` reports the refusal too, without touching anything. - The systemd branch stops/starts through `systemctl --user stop|start "$SYSTEMD_UNIT"`, mirroring the existing launchd branch's structure. Unlike launchd's KeepAlive, systemd's `Restart=on-failure` does not fire on a deliberate `stop`, so no unload/disable dance is needed there. - `assert_single_daemon()` runs after start+verify and `die()`s if `pgrep -f "$PATTERN"` finds more than one pid — the exact symptom this ticket exists to catch, which none of the existing checks can see. - launchd behavior is unchanged in effect; `--check` stays read-only. **Testing seam:** `detect_supervisor`/`require_drivable_supervisor`/`count_daemon_pids`/`assert_single_daemon` are pure functions. `launchd_installed`/`launchd_loaded`/`systemd_installed`/`systemd_loaded` are simple overridable functions (same pattern as the existing launchd probes) so `scripts/test-redeploy-fleetd.sh` can simulate launchd-only, systemd-only, neither, and both-at-once without a real launchd or systemd on this (macOS) dev box. **Verification (measured in-worktree, not derived by reading):** - Baseline (unmodified tree): `bash scripts/test-redeploy-fleetd.sh` → exit 0, prints the 3 existing mutation-proof lines + `PASS: redeploy log classifier` (11 test invocations). - Final tree: same command → exit 0, same output plus the 8 new tests all silently passing (19 test invocations total). - Mutation A (`require_drivable_supervisor` accepts `ambiguous` instead of refusing — proved applied via two greps: mutant pattern present, original pattern absent): tests → 1 failure, exit 1: `FAIL: require_drivable_supervisor accepted an ambiguous (undrivable) supervisor`. - Mutation B (`assert_single_daemon` accepts 2 pids, i.e. `-gt 1` → `-gt 2`, proved applied the same way): tests → 1 failure, exit 1: `FAIL: assert_single_daemon accepted two simultaneously running pids`. - Both mutations reverted; grep confirms the original lines are back verbatim; tests green again as a positive control (exit 0, identical output to the final-tree run). - `cd fleetd && mvn clean install`: `BUILD SUCCESS`, `Tests run: 1677, Failures: 0, Errors: 0, Skipped: 0`. (No `pom.xml` at the repo root — only under `fleetd/` — so this ran from the module root per the implementer skill, not literally "the repo root".) **Out of scope, spotted not fixed:** - A pre-existing `dev.ltms.fleet.deploy.SystemdUnitSafetyTest` (Java, 8 tests, all passing) already exists in the main build — unrelated to this shell-script ticket, not touched. - Per this session's MEMORY.md, ticket #493 describes `redeploy-fleetd.sh` building into the LIVE `target/fleetd.jar` path before stopping the daemon — a separate, real defect in this same script, out of scope here and not touched. No IDE MCP tools were available to me as a worker; verification is `bash -n`, the shell test suite above, and `mvn clean install`.
agent added 1 commit 2026-09-12 04:02:01 +02:00
fleetd #492: teach redeploy-fleetd.sh systemd --user as a third supervisor
CI / contract (pull_request) Successful in 56s
Details
CI / build (pull_request) Successful in 1m40s
Details
dcd505286f 
launchd, systemd --user, and unsupervised are three different answers, not two.
Refuse (die) rather than fall through to kill+nohup when a supervisor is
detected that this script cannot drive (e.g. both signals fire at once), and
add a post-restart check that fails the run if more than one fleetd process
is alive. detect_supervisor()/require_drivable_supervisor()/
count_daemon_pids()/assert_single_daemon() are pure, overridable functions so
scripts/test-redeploy-fleetd.sh can exercise them without a real launchd or
systemd.
ltms commented 2026-09-12 04:06:38 +02:00
Owner
Copy Link
Copy Source

Not merging yet. none means two different things, and only one of them is safe.

Good work overall — the overridable-probe seam, require_drivable_supervisor, and assert_single_daemon are the right shapes, and the two mutations you ran are proper proofs. The problem is one level up, in detect_supervisor.

detect_supervisor (scripts/redeploy-fleetd.sh:123) reads only the two *_loaded probes. So none currently means both:

  • "proven unsupervised" — safe, and the raw kill + nohup fallback is correct for it, and
  • "I could not tell" — not safe, and it takes the same fallback.

require_drivable_supervisor accepts none without question, so the refuse-don't-guess gate never sees the second case.

Input 1 — unit installed, not active at this instant. I measured this.

Run in a detached worktree at dcd5052, sourcing the script behind its own SOURCED guard and overriding the probes, exactly as your harness does:

launchd_installed -> 1   launchd_loaded -> 1      (no launchd)
systemd_installed -> 0   systemd_loaded -> 1      (unit file IS there; is-active says no)

detect_supervisor            -> none
require_drivable_supervisor  -> exit 0     (accepted)

systemctl --user is-active exits non-zero for activating, deactivating, failed, and while an auto-restart is pending. Every one of those is a host that is under systemd. The script then starts an unsupervised daemon with nohup while systemd is about to start its own. That is the two-daemon race this ticket exists to prevent — it ran for 20 hours on fleet01 and only an unrelated consumer count revealed it.

systemd_installed already knows the answer. It is computed at :294 for a warn and then never consulted by the decision.

Input 2 — systemctl errors instead of answering. Found by the reviewer; I could not test it here.

Both probes are command -v systemctl && systemctl … 2>/dev/null. A systemctl that runs but cannot reach the user bus — a headless ssh session with no lingering, which is exactly the second-host case — fails the same way a real "no" does. Same landing: none, same fallback. This is the reviewer's finding, reasoned from the code; I have no systemd on this host, so I did not measure it.

Two different inputs, one defect. That makes them two data points, not one.

Asked for

  1. detect_supervisor must return a fourth undrivable answer — call it unclear — for installed but not loaded, on both supervisors. require_drivable_supervisor must die() on it, with a message that names which supervisor looked present and what to check.
  2. Tell "systemctl answered no" apart from "systemctl could not answer". Capture the exit status and stderr separately rather than discarding both with 2>/dev/null; a probe that errors must land in unclear, never in none.
  3. none must keep meaning only proven unsupervised: no plist, no unit file, and both probes answered cleanly.
  4. Tests for both inputs. Your existing tests stub the probes with clean return 0 / return 1 bodies, so no test ever exercises a probe that errors. At least one test must drive the real function body with a systemctl stub that exits non-zero and writes to stderr.

Keep everything else. The mutation discipline in your report is exactly right and I want the same on the new tests: prove each mutant applied with two different search strings, and run a control after restoring.

Note for whoever picks this up: #493 also changes this file (it builds into the live target/fleetd.jar before stopping the daemon). #493 lands after this PR, not before.

## Not merging yet. `none` means two different things, and only one of them is safe. Good work overall — the overridable-probe seam, `require_drivable_supervisor`, and `assert_single_daemon` are the right shapes, and the two mutations you ran are proper proofs. The problem is one level up, in `detect_supervisor`. `detect_supervisor` (`scripts/redeploy-fleetd.sh:123`) reads **only** the two `*_loaded` probes. So `none` currently means both: - **"proven unsupervised"** — safe, and the raw `kill` + `nohup` fallback is correct for it, and - **"I could not tell"** — not safe, and it takes the same fallback. `require_drivable_supervisor` accepts `none` without question, so the refuse-don't-guess gate never sees the second case. ### Input 1 — unit installed, not active at this instant. I measured this. Run in a detached worktree at `dcd5052`, sourcing the script behind its own `SOURCED` guard and overriding the probes, exactly as your harness does: ``` launchd_installed -> 1 launchd_loaded -> 1 (no launchd) systemd_installed -> 0 systemd_loaded -> 1 (unit file IS there; is-active says no) detect_supervisor -> none require_drivable_supervisor -> exit 0 (accepted) ``` `systemctl --user is-active` exits non-zero for `activating`, `deactivating`, `failed`, and while an auto-restart is pending. Every one of those is a host that **is** under systemd. The script then starts an unsupervised daemon with `nohup` while systemd is about to start its own. That is the two-daemon race this ticket exists to prevent — it ran for 20 hours on fleet01 and only an unrelated consumer count revealed it. `systemd_installed` already knows the answer. It is computed at `:294` for a `warn` and then never consulted by the decision. ### Input 2 — `systemctl` errors instead of answering. Found by the reviewer; I could not test it here. Both probes are `command -v systemctl && systemctl … 2>/dev/null`. A `systemctl` that runs but cannot reach the user bus — a headless ssh session with no lingering, which is exactly the second-host case — fails the same way a real "no" does. Same landing: `none`, same fallback. This is the reviewer's finding, reasoned from the code; I have no systemd on this host, so I did not measure it. Two different inputs, one defect. That makes them two data points, not one. ### Asked for 1. `detect_supervisor` must return a fourth undrivable answer — call it `unclear` — for **installed but not loaded**, on both supervisors. `require_drivable_supervisor` must `die()` on it, with a message that names which supervisor looked present and what to check. 2. Tell "systemctl answered no" apart from "systemctl could not answer". Capture the exit status and stderr separately rather than discarding both with `2>/dev/null`; a probe that errors must land in `unclear`, never in `none`. 3. `none` must keep meaning only *proven unsupervised*: no plist, no unit file, and both probes answered cleanly. 4. Tests for both inputs. Your existing tests stub the probes with clean `return 0` / `return 1` bodies, so no test ever exercises a probe that errors. At least one test must drive the real function body with a `systemctl` stub that exits non-zero **and** writes to stderr. Keep everything else. The mutation discipline in your report is exactly right and I want the same on the new tests: prove each mutant applied with two different search strings, and run a control after restoring. Note for whoever picks this up: **#493 also changes this file** (it builds into the live `target/fleetd.jar` before stopping the daemon). #493 lands after this PR, not before.
ltms commented 2026-09-12 04:13:11 +02:00
Owner
Copy Link
Copy Source

Two more requirements, to apply at review. Both found by the fleet01 lead.

A worker is already on the unclear state. These two go on top, and the first one is a trap that the fix itself creates.

1. Adding a state without a default arm makes the script worse, not better

bash has no exhaustiveness check. A case with no *) arm silently matches nothing and carries on. Measured on dcd5052:

CONTROL, ')' occurrences in the file:  149   (non-zero — the grep works)

require_drivable_supervisor  :144   launchd|systemd|none, ambiguous, *) die     <- has one
report state                 :308   launchd, systemd, none                      <- NO *)
stop                         :419   launchd, systemd, none                      <- NO *)
start                        :477   launchd, systemd, none                      <- NO *)

require_drivable_supervisor runs once, at :305. The three switches that actually drive the daemon each enumerate exactly three values. So if a new state reaches them — because a future edit adds one, or because the guard is moved, weakened, or skipped on some path — nothing matches in the stop switch, the daemon is never stopped, nothing matches in the start switch, and the script reports no error at any point.

Requirement: :308, :419 and :477 each get a *) die … arm naming the unhandled value. Then a fourth state fails loudly at every site instead of silently skipping the stop or the start.

The guard staying is necessary but not sufficient. One gate at the top of a script is the same shape as a one-way gate: it closes the direction you came from.

2. Two different tests, and the easy one is not the useful one

A test that constructs the undrivable state directly proves the switch handles it. It does not prove the probe ever returns it. Only the second catches a probe that quietly maps its own failures to none before the state is ever consulted.

That is the same gap as asserting the nudge happens versus asserting the wait waits on #489, and it is the gap that let this defect into dcd5052 in the first place — every existing test stubs systemd_loaded with a clean return 0 / return 1, so no test ever drives the real body.

Both are required. The brief already asks for a systemctl stub that exits non-zero and writes to stderr; that is the second kind. Do not let it be replaced by a direct construction of the state.

Filed separately, on purpose

The underlying shape is now #497 — a sentinel that conflates "measured: no" with "could not measure", with three instances across three subsystems. It is explicitly not the same family as #494, and the fleet01 lead was firm about not merging the two: #494's fix is compute the value honestly, and here the value already is honest — the vocabulary is one symbol short. A worker who reads #494's rule against this code will confirm none is genuinely what the probe returned and close it.

## Two more requirements, to apply at review. Both found by the fleet01 lead. A worker is already on the `unclear` state. These two go on top, and the first one is a trap that the fix itself creates. ### 1. Adding a state without a default arm makes the script worse, not better bash has no exhaustiveness check. A `case` with no `*)` arm silently matches nothing and carries on. Measured on `dcd5052`: ``` CONTROL, ')' occurrences in the file: 149 (non-zero — the grep works) require_drivable_supervisor :144 launchd|systemd|none, ambiguous, *) die <- has one report state :308 launchd, systemd, none <- NO *) stop :419 launchd, systemd, none <- NO *) start :477 launchd, systemd, none <- NO *) ``` `require_drivable_supervisor` runs **once**, at `:305`. The three switches that actually drive the daemon each enumerate exactly three values. So if a new state reaches them — because a future edit adds one, or because the guard is moved, weakened, or skipped on some path — nothing matches in the stop switch, the daemon is never stopped, nothing matches in the start switch, and **the script reports no error at any point**. **Requirement: `:308`, `:419` and `:477` each get a `*) die …` arm** naming the unhandled value. Then a fourth state fails loudly at every site instead of silently skipping the stop or the start. The guard staying is necessary but not sufficient. One gate at the top of a script is the same shape as a one-way gate: it closes the direction you came from. ### 2. Two different tests, and the easy one is not the useful one > A test that constructs the undrivable state directly proves the **switch** handles it. It does not prove the **probe** ever returns it. Only the second catches a probe that quietly maps its own failures to `none` before the state is ever consulted. That is the same gap as *asserting the nudge happens* versus *asserting the wait waits* on #489, and it is the gap that let this defect into `dcd5052` in the first place — every existing test stubs `systemd_loaded` with a clean `return 0` / `return 1`, so no test ever drives the real body. **Both are required.** The brief already asks for a `systemctl` stub that exits non-zero and writes to stderr; that is the second kind. Do not let it be replaced by a direct construction of the state. ### Filed separately, on purpose The underlying shape is now **#497** — a sentinel that conflates "measured: no" with "could not measure", with three instances across three subsystems. It is explicitly **not** the same family as #494, and the fleet01 lead was firm about not merging the two: #494's fix is *compute the value honestly*, and here the value already is honest — the vocabulary is one symbol short. A worker who reads #494's rule against this code will confirm `none` is genuinely what the probe returned and close it.
ltms commented 2026-09-12 04:19:14 +02:00
Owner
Copy Link
Copy Source

Amending my own requirement twice. One arm was wrong, and one worry does not apply here.

1. *) die is the wrong arm for the reporting switch. Use *) echo.

I asked for *) die … on all three switches. That is wrong for :308.

The fleet01 lead's point, and I agree on sight: :308 is the diagnostic path. A reporting channel that aborts on a value it does not recognise goes silent exactly when the state is novel — which is #497's instance 1 one level up, the failure this whole line of work is ranking against. Stopping the report was never the guard's purpose.

Choose the arm by what the caller does with the value:

Switch Line Caller Arm
report state :308 displays it *) echo "unknown supervisor state: $SUPERVISOR_KIND" — keep the operator informed
stop :419 acts on it *) die …
start :477 acts on it *) die …

A blanket "every switch carries *) die" gets the two drivable ones right and turns the diagnostic one into the defect.

2. Their set -e warning is real bash, and it does not reach this file. Both halves measured.

They warned that *) die is defeated by the calling context, since a die inside a command substitution exits only the subshell. I reproduced all of it on this host — bash 5.3.9 via /usr/bin/env bash, not macOS's system 3.2 — with the guard byte-identical in every case and only the call site varying:

A  plain call                            rc=9  parent_continued=no    <- CONTROL: the guard fires
B  x=$(f unclear)          no set -e     rc=0  parent_continued=yes   x=[]
C  x=$(f unclear)          set -e        rc=9  parent_continued=no
D  if x=$(f unclear); then set -e        rc=0  parent_continued=yes   else-branch taken
E  f unclear | cat         set -e        rc=0  parent_continued=yes
F  f unclear | cat         set -euo pipefail  rc=9  parent_continued=no   <- my addition

A fires, so the guard is correctly written in all six; the difference is entirely context. Their table is right, and F shows pipefail is what closes E.

But the preconditions are not present in this script, and I checked each one rather than assuming:

CONTROL, 'set' lines in the file:                 1
  :50   set -euo pipefail              <- closes B and E already

enclosing construct of each guarded switch:
  :308  case "$SUPERVISOR_KIND" in     <- TOP-LEVEL, not a function in $( )
  :419  case "$SUPERVISOR_KIND" in     <- TOP-LEVEL
  :477  case "$SUPERVISOR_KIND" in     <- TOP-LEVEL

every command substitution over these functions:
  :173  count="$(count_daemon_pids "$pids")"     plain assignment, set -e in force -> case C
  :304  SUPERVISOR_KIND="$(detect_supervisor)"   plain assignment, set -e in force -> case C

CONTROL, 'if ' lines:                             26
case-D shape  ^(if|while|until) VAR=$( :          0 matches

So all three guarded switches are case A, both substitutions are case C, and there is no case D anywhere. The *) die arms will fire.

3. What to carry forward anyway — as a standing constraint, not a fix

The mechanism is real even though it does not bite today, and two single-line edits would make it bite:

  • Do not remove set -euo pipefail from :50. Without it, SUPERVISOR_KIND="$(detect_supervisor)" at :304 silently yields an empty string and the script continues.
  • Never write if VAR="$(detect_supervisor)"; then or pipe these functions. Case D suppresses set -e even when it is set, and no convention about the switch can catch it, because the defect is at the caller.
  • If any guard ever moves into detect_supervisor, :304 becomes the load-bearing site. It is safe today only because set -e is on.

Add those three as a comment block above detect_supervisor so the next contributor reads them before editing.

## Amending my own requirement twice. One arm was wrong, and one worry does not apply here. ### 1. `*) die` is the wrong arm for the reporting switch. Use `*) echo`. I asked for `*) die …` on all three switches. That is wrong for `:308`. The fleet01 lead's point, and I agree on sight: **`:308` is the diagnostic path.** A reporting channel that aborts on a value it does not recognise goes silent exactly when the state is novel — which is #497's instance 1 one level up, the failure this whole line of work is ranking against. Stopping the report was never the guard's purpose. Choose the arm by what the caller does with the value: | Switch | Line | Caller | Arm | |---|---|---|---| | report state | `:308` | **displays** it | `*) echo "unknown supervisor state: $SUPERVISOR_KIND"` — keep the operator informed | | stop | `:419` | **acts** on it | `*) die …` | | start | `:477` | **acts** on it | `*) die …` | A blanket "every switch carries `*) die`" gets the two drivable ones right and turns the diagnostic one into the defect. ### 2. Their `set -e` warning is real bash, and it does not reach this file. Both halves measured. They warned that `*) die` is defeated by the calling context, since a `die` inside a command substitution exits only the subshell. I reproduced all of it on this host — bash **5.3.9** via `/usr/bin/env bash`, not macOS's system 3.2 — with the guard byte-identical in every case and only the call site varying: ``` A plain call rc=9 parent_continued=no <- CONTROL: the guard fires B x=$(f unclear) no set -e rc=0 parent_continued=yes x=[] C x=$(f unclear) set -e rc=9 parent_continued=no D if x=$(f unclear); then set -e rc=0 parent_continued=yes else-branch taken E f unclear | cat set -e rc=0 parent_continued=yes F f unclear | cat set -euo pipefail rc=9 parent_continued=no <- my addition ``` A fires, so the guard is correctly written in all six; the difference is entirely context. Their table is right, and F shows `pipefail` is what closes E. **But the preconditions are not present in this script**, and I checked each one rather than assuming: ``` CONTROL, 'set' lines in the file: 1 :50 set -euo pipefail <- closes B and E already enclosing construct of each guarded switch: :308 case "$SUPERVISOR_KIND" in <- TOP-LEVEL, not a function in $( ) :419 case "$SUPERVISOR_KIND" in <- TOP-LEVEL :477 case "$SUPERVISOR_KIND" in <- TOP-LEVEL every command substitution over these functions: :173 count="$(count_daemon_pids "$pids")" plain assignment, set -e in force -> case C :304 SUPERVISOR_KIND="$(detect_supervisor)" plain assignment, set -e in force -> case C CONTROL, 'if ' lines: 26 case-D shape ^(if|while|until) VAR=$( : 0 matches ``` So all three guarded switches are case A, both substitutions are case C, and there is no case D anywhere. The `*) die` arms will fire. ### 3. What to carry forward anyway — as a standing constraint, not a fix The mechanism is real even though it does not bite today, and two single-line edits would make it bite: - **Do not remove `set -euo pipefail` from `:50`.** Without it, `SUPERVISOR_KIND="$(detect_supervisor)"` at `:304` silently yields an empty string and the script continues. - **Never write `if VAR="$(detect_supervisor)"; then`** or pipe these functions. Case D suppresses `set -e` even when it is set, and no convention about the switch can catch it, because the defect is at the caller. - **If any guard ever moves *into* `detect_supervisor`**, `:304` becomes the load-bearing site. It is safe today only because `set -e` is on. Add those three as a comment block above `detect_supervisor` so the next contributor reads them before editing.
ltms commented 2026-09-12 05:14:07 +02:00
Owner
Copy Link
Copy Source

Closing: this branch's work is already on main, merged as part of PR #499, not abandoned.

PR #499 was built on top of this branch rather than on main — its own description says
"based on origin/worker/492-209647-1 @ dcd5052, NOT main". So merging #499 carried every commit
here in with it.

Measured just now, in the main clone:

$ git rev-parse origin/worker/492-209647-1
dcd505286f035395e25cccc6923524a6ad649391
$ git merge-base --is-ancestor origin/worker/492-209647-1 origin/main && echo contained
contained

main is at 136312f. Nothing from this PR is lost, and there is nothing left to merge here.

Thanks for the work — the systemd-as-third-supervisor change is live in scripts/redeploy-fleetd.sh,
and --check runs clean on this Mac against the merged script (supervisor detected: launchd).

Closing: **this branch's work is already on `main`**, merged as part of PR #499, not abandoned. PR #499 was built on top of this branch rather than on `main` — its own description says "based on `origin/worker/492-209647-1` @ `dcd5052`, NOT main". So merging #499 carried every commit here in with it. Measured just now, in the main clone: ``` $ git rev-parse origin/worker/492-209647-1 dcd505286f035395e25cccc6923524a6ad649391 $ git merge-base --is-ancestor origin/worker/492-209647-1 origin/main && echo contained contained ``` `main` is at `136312f`. Nothing from this PR is lost, and there is nothing left to merge here. Thanks for the work — the systemd-as-third-supervisor change is live in `scripts/redeploy-fleetd.sh`, and `--check` runs clean on this Mac against the merged script (`supervisor detected: launchd`).
ltms closed this pull request 2026-09-12 05:14:13 +02:00
Some checks are pending
CI / contract (pull_request) Successful in 56s
Details
CI / build (pull_request) Successful in 1m40s
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
blocked
Cannot start until something else lands. The body says what.
 needs-live-proof
Merged and green, but never shown working on the running daemon. Not the same as done.
 ready-to-delegate
Scope, files and acceptance criteria are written. A worker can be briefed from the body alone.
 silent-default
A feature that compiles, passes tests, and ships turned off. Nine recurrences and counting.
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent kevin ltms
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#495
Delete Branch "worker/492-209647-1"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?