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

redeploy-fleetd.sh knows which supervisor it is talking to (#492) but not which one it is watching: the log source and the pid count are both still macOS-shaped, and on systemd they fail in opposite directions #593

New Issue
Open
opened 2026-09-19 09:56:43 +02:00 by ltms · 1 comment
ltms commented 2026-09-19 09:56:43 +02:00
Owner
Copy Link
Copy Source

The shape

#492 made the script's control plane supervisor-aware: it correctly picks launchd,
systemd, or unsupervised, and the fleet01 run confirmed it used systemctl --user stop and
systemctl --user start rather than kill-and-nohup.

Its observation plane was not changed with it. Two checks still assume the macOS
nohup-era shape. They fail in opposite directions, which is why neither is obvious:

Check On systemd Direction of the error
read fleetd/fleetd.out for the boot lines the file does not exist false "cannot tell" — three real checks silently give up
pgrep -f 'target/fleetd.jar' to count daemons can match the caller's own shell false failure — a good redeploy is aborted

A check that cannot run and a check that ran and found nothing produce the same shape on the
screen. That is the recurring form this repo keeps hitting (#497, #506, and the zero-match rule).

Instance 1 — the log source, reported by the fleet01 lead

Measured on fleet01 on 2026-09-19, during a real redeploy that otherwise succeeded (exit 0, jar
b92db8f42b1a → f90c14e0168a, old pid 1696852 stopped through systemd, new pid 3148354,
/healthz 200). I did not run this myself; the fleet01 lead did and quoted the output:

./scripts/redeploy-fleetd.sh: line 1181: /home/ltms/LTMS/fleetd/fleetd/fleetd.out: No such file or directory

Three checks then reported themselves unable to answer:

  • "no fresh fleetd listening line after the restart"
  • "== config at boot == (nothing reported)"
  • "cannot tell whether the previous daemon's shutdown drain finished"

Under systemd the daemon's stdout goes to the journal, so that file never exists. The first two
were answerable the whole time, from journalctl --user -u fleetd:

07:51:29.737 dev.ltms.fleet.Fleetd - fleetd listening on 127.0.0.1:8765, herdr socket …/sessions/fleet01/herdr.sock
07:51:28.812 member slots: 4 configured [architect:gx, dev:xf, dev:gx, reviewer:gx]
07:51:28.853 reply inbox: AMQP broker (durable) via env var LAVINMQ_URI (prefetch=32)

The third one splits the other way, and this is the part worth keeping. The drain line is
absent from the journal too. So that is a genuine absence, and the script's own first candidate
explanation — "that daemon predates #522's drain-complete line" — is almost certainly correct,
because the old jar was built 2026-09-10 from 4887731. Two instrument artifacts and one real
negative, arriving in one identical-looking block of three.

The script is already better than most here: it says in so many words that this is not a pass
and not a failure. It still offered two explanations that were both wrong for two of the three,
because "the log file is not where I am looking" is not in its list of explanations.

Instance 2 — the pid count, and why it did not reproduce on the Mac

The fleet01 lead reported pgrep -fc 'fleetd.jar' returning 2 on their host, with the second
match being their own shell, and warned that trap 8 may share the flaw. running_pid is exactly
that shape (scripts/redeploy-fleetd.sh:172, PATTERN='target/fleetd.jar' at :79):

running_pid() { pgrep -f "$PATTERN" || true; }

and assert_single_daemon (:504) dies when it counts more than one.

I tried to reproduce it on the Mac and could not, which is a result worth recording rather
than hiding:

$ pgrep -f 'target/fleetd.jar'
30224
$ bash -c "pgrep -f 'target/fleetd.jar'"
30224
$ pgrep -fc 'target/fleetd.jar'
usage: pgrep [-Lfilnoqvx] [-d delim] ...

Two reasons, both platform facts and neither a reason to close this:

  1. pgrep -c does not exist on BSD/macOS. The count form the fleet01 lead used is
    Linux-only, so the two hosts cannot even run the same command.
  2. bash -c "<single command>" execs the command in place, so no parent shell survives holding
    the pattern in its argv. An interactive shell, a pipeline, or an ssh host "…" wrapper does
    leave one, and that is where the self-match appears.

So the defect is real on Linux and latent on macOS, and the script is now run on both. The
consequence is worse than the log one: a self-match makes assert_single_daemon die with
"more than one fleetd process is running after this restart" on a redeploy that worked.

There is a third copy of the flaw in the advice text itself (:510). The die message tells the
operator to "Investigate with pgrep -f \"$PATTERN\"" — typed by hand, over ssh, that is exactly
the self-matching invocation, so the guidance reproduces the false reading it is meant to resolve.

Fix direction

  • Ask the supervisor, do not pattern-match argv. systemctl --user show fleetd -p MainPID
    under systemd, launchctl under launchd, and confirm with /proc/<pid>/exe (Linux) — all
    self-match-proof. Keep a pattern only for the unsupervised case, and say in the message that a
    pattern can match the caller.
  • Read the log where the supervisor puts it. Under systemd that is
    journalctl --user -u fleetd, not a file path derived from the repo.
  • Correct the trap list in the skill: it currently says a Linux host has no supervisor, and
    fleet01 measured systemd --user unit installed: fleetd / supervisor detected: systemd.
    Only the launchd warning is expected there, and it is structural on Linux.

Acceptance criteria

Properties under a change, not the presence of a construct.

  1. Move the log and the check must notice. Point the daemon's output somewhere the script
    does not expect and the run must report "I could not read the log", naming the source it tried
    — distinct from "I read the log and the line was absent". Today both print the same thing.
  2. A missing line stays a real answer. With the log readable and the drain line genuinely
    absent, the run must still say so, and must not attribute it to a missing file.
  3. The pid count cannot match the caller. Invoke the count through a wrapper whose argv
    contains the pattern (ssh host "…", or a non-exec'ing shell) and it must still return the
    true daemon count. A test that only runs it from a plain script stays green while the defect is
    present.
  4. A real second daemon still fails the run. Start two and assert_single_daemon must die.
    Fixing the false positive must not remove the check.

Reported by the fleet01 lead; instances 1 and 3 are their measurement, instance 2's macOS
non-reproduction is mine. Related: #492 (supervisor-aware control plane), #504 (four places a
failed command reads as a clean result), #497 (a sentinel that conflates "no" with "cannot tell").

## The shape `#492` made the script's **control plane** supervisor-aware: it correctly picks launchd, systemd, or unsupervised, and the fleet01 run confirmed it used `systemctl --user stop` and `systemctl --user start` rather than kill-and-nohup. Its **observation plane** was not changed with it. Two checks still assume the macOS nohup-era shape. They fail in opposite directions, which is why neither is obvious: | Check | On systemd | Direction of the error | |---|---|---| | read `fleetd/fleetd.out` for the boot lines | the file does not exist | **false "cannot tell"** — three real checks silently give up | | `pgrep -f 'target/fleetd.jar'` to count daemons | can match the caller's own shell | **false failure** — a good redeploy is aborted | A check that cannot run and a check that ran and found nothing produce the same shape on the screen. That is the recurring form this repo keeps hitting (#497, #506, and the zero-match rule). ## Instance 1 — the log source, reported by the fleet01 lead Measured on fleet01 on 2026-09-19, during a real redeploy that otherwise succeeded (exit 0, jar `b92db8f42b1a` → `f90c14e0168a`, old pid 1696852 stopped through systemd, new pid 3148354, `/healthz` 200). I did not run this myself; the fleet01 lead did and quoted the output: ``` ./scripts/redeploy-fleetd.sh: line 1181: /home/ltms/LTMS/fleetd/fleetd/fleetd.out: No such file or directory ``` Three checks then reported themselves unable to answer: - "no fresh `fleetd listening` line after the restart" - "== config at boot == (nothing reported)" - "cannot tell whether the previous daemon's shutdown drain finished" Under systemd the daemon's stdout goes to the journal, so that file never exists. The first two were answerable the whole time, from `journalctl --user -u fleetd`: ``` 07:51:29.737 dev.ltms.fleet.Fleetd - fleetd listening on 127.0.0.1:8765, herdr socket …/sessions/fleet01/herdr.sock 07:51:28.812 member slots: 4 configured [architect:gx, dev:xf, dev:gx, reviewer:gx] 07:51:28.853 reply inbox: AMQP broker (durable) via env var LAVINMQ_URI (prefetch=32) ``` **The third one splits the other way, and this is the part worth keeping.** The drain line is absent from the journal too. So that is a genuine absence, and the script's own first candidate explanation — "that daemon predates #522's drain-complete line" — is almost certainly correct, because the old jar was built 2026-09-10 from `4887731`. Two instrument artifacts and one real negative, arriving in one identical-looking block of three. The script is already better than most here: it says in so many words that this is not a pass and not a failure. It still offered two explanations that were both wrong for two of the three, because "the log file is not where I am looking" is not in its list of explanations. ## Instance 2 — the pid count, and why it did not reproduce on the Mac The fleet01 lead reported `pgrep -fc 'fleetd.jar'` returning 2 on their host, with the second match being their own shell, and warned that trap 8 may share the flaw. `running_pid` is exactly that shape (`scripts/redeploy-fleetd.sh:172`, `PATTERN='target/fleetd.jar'` at `:79`): ```bash running_pid() { pgrep -f "$PATTERN" || true; } ``` and `assert_single_daemon` (`:504`) dies when it counts more than one. **I tried to reproduce it on the Mac and could not**, which is a result worth recording rather than hiding: ``` $ pgrep -f 'target/fleetd.jar' 30224 $ bash -c "pgrep -f 'target/fleetd.jar'" 30224 $ pgrep -fc 'target/fleetd.jar' usage: pgrep [-Lfilnoqvx] [-d delim] ... ``` Two reasons, both platform facts and neither a reason to close this: 1. **`pgrep -c` does not exist on BSD/macOS.** The count form the fleet01 lead used is Linux-only, so the two hosts cannot even run the same command. 2. `bash -c "<single command>"` execs the command in place, so no parent shell survives holding the pattern in its argv. An interactive shell, a pipeline, or an `ssh host "…"` wrapper does leave one, and that is where the self-match appears. So the defect is **real on Linux and latent on macOS**, and the script is now run on both. The consequence is worse than the log one: a self-match makes `assert_single_daemon` die with "more than one fleetd process is running after this restart" on a redeploy that worked. There is a third copy of the flaw in the advice text itself (`:510`). The die message tells the operator to "Investigate with `pgrep -f \"$PATTERN\"`" — typed by hand, over ssh, that is exactly the self-matching invocation, so the guidance reproduces the false reading it is meant to resolve. ## Fix direction - **Ask the supervisor, do not pattern-match argv.** `systemctl --user show fleetd -p MainPID` under systemd, `launchctl` under launchd, and confirm with `/proc/<pid>/exe` (Linux) — all self-match-proof. Keep a pattern only for the unsupervised case, and say in the message that a pattern can match the caller. - **Read the log where the supervisor puts it.** Under systemd that is `journalctl --user -u fleetd`, not a file path derived from the repo. - Correct the trap list in the skill: it currently says a Linux host has no supervisor, and fleet01 measured `systemd --user unit installed: fleetd` / `supervisor detected: systemd`. Only the launchd warning is expected there, and it is structural on Linux. ## Acceptance criteria Properties under a change, not the presence of a construct. 1. **Move the log and the check must notice.** Point the daemon's output somewhere the script does not expect and the run must report "I could not read the log", naming the source it tried — distinct from "I read the log and the line was absent". Today both print the same thing. 2. **A missing line stays a real answer.** With the log readable and the drain line genuinely absent, the run must still say so, and must not attribute it to a missing file. 3. **The pid count cannot match the caller.** Invoke the count through a wrapper whose argv contains the pattern (`ssh host "…"`, or a non-exec'ing shell) and it must still return the true daemon count. A test that only runs it from a plain script stays green while the defect is present. 4. **A real second daemon still fails the run.** Start two and `assert_single_daemon` must die. Fixing the false positive must not remove the check. Reported by the fleet01 lead; instances 1 and 3 are their measurement, instance 2's macOS non-reproduction is mine. Related: #492 (supervisor-aware control plane), #504 (four places a failed command reads as a clean result), #497 (a sentinel that conflates "no" with "cannot tell").
ltms commented 2026-09-19 10:24:05 +02:00
Author
Owner
Copy Link
Copy Source

CORRECTION 1 — two holes in the PR #597 fix. Both close with one change.

Reviewed at the merge gate. The approach is sound, the reproduction is genuine, and breaking the fix on purpose to watch the test go red is exactly the right evidence. Two defects remain, and they are the same shape as the defect being fixed.

The current fix

running_pid() keeps every pid pgrep -f "$PATTERN" returns, except those whose ps -o comm= names a shell:

case "$comm" in
  sh|bash|zsh|dash|ksh) continue ;;
esac

Hole 1 — an exited pid is counted

pgrep lists a pid, the process exits, then ps -o comm= -p "$pid" returns nothing. || true makes comm empty. An empty string matches no shell name, so continue never fires and the dead pid is counted.

Measured:

$ comm="$(ps -o comm= -p 999999 2>/dev/null || true)"   # a pid that does not exist
  dead pid comm=[] -> COUNTED

That inflates the count and can trip assert_single_daemon with a "racing supervisor" death for a process that is already gone. It is the same false positive the ticket exists to remove, just rarer.

Hole 2 — the exclusion is a denylist, and the daemon has a name

The list covers sh bash zsh dash ksh. Anything else that carries the pattern in its argv is counted:

ssh      -> COUNTED
perl     -> COUNTED
python3  -> COUNTED
ruby     -> COUNTED
tail     -> COUNTED

ssh matters most, because the ticket names ssh as a live route. A denylist of wrappers is a list someone finds a gap in. And we do not need one, because the thing we are looking for has a fixed name.

Measured on the live daemon, names only:

pid=30224 comm=java

The PR's own report states the premise — "The daemon is always java" — and then implements the weaker form.

The fix: allowlist, not denylist

Count a pid only when its comm is java. That closes hole 2 by construction and closes hole 1 for free, because an empty comm is not java either.

running_pid() {
  local pid comm out=''
  for pid in $(pgrep -f "$PATTERN" 2>/dev/null || true); do
    comm="$(ps -o comm= -p "$pid" 2>/dev/null || true)"
    comm="${comm##*/}"; comm="${comm#-}"
    # Allowlist, not a denylist of wrappers: the daemon is a jar, so it is always `java`.
    # A shell, ssh, perl or any other process carrying $PATTERN in its argv is not.
    # An exited pid returns an empty comm here and is correctly dropped too.
    [ "$comm" = java ] || continue
    out="$out$pid"$'\n'
  done
  printf '%s' "$out"
}

The objection to answer, because it is real

An allowlist can under-count: if fleetd ever stops being launched by java — a native image, a renamed launcher — running_pid() returns nothing and assert_single_daemon stops noticing a second daemon. That is a false negative, and for a guard that is the worse direction.

I do not think it blocks the change, because the script already carries the same assumption one line up: PATTERN='target/fleetd.jar'. A jar is run by java. If that stops being true, the pattern breaks before the allowlist does. The allowlist adds no assumption that is not already load-bearing.

But say it in the comment rather than leaving it implicit, and name PATTERN as the sibling assumption, so whoever changes the launch method finds both.

Acceptance for the follow-up

  1. A pid whose comm is not java — including an exited pid, and including a non-shell wrapper such as ssh or perl carrying the pattern — is not counted. Test at least one non-shell case; a test that only covers shells passes today and would have passed before this correction.
  2. A real java-named process matching the pattern is still counted, so assert_single_daemon still fails on a genuine second daemon. The exec -a standin in the PR is the right technique — point it at a java-named standin.
  3. Break the fix on purpose again and watch the new test fail, as you did the first time. That is the part that made the first round's evidence worth reading.

Out of scope, and correctly left alone

Instance 1 (the fleetd.out log source) stays untouched — it needs a systemd host. The peer fleet runs systemd --user and has confirmed fleetd.out is empty there; that half is going to them.

The deliberate choice not to wire pid counting to SUPERVISOR_KIND — because OLD_PID is computed before the supervisor is detected, so it would have meant reordering the main flow — was the right call for this scope, and saying so in the report rather than silently narrowing was the right way to handle it.

## CORRECTION 1 — two holes in the PR #597 fix. Both close with one change. Reviewed at the merge gate. The approach is sound, the reproduction is genuine, and breaking the fix on purpose to watch the test go red is exactly the right evidence. Two defects remain, and they are the same shape as the defect being fixed. ### The current fix `running_pid()` keeps every pid `pgrep -f "$PATTERN"` returns, except those whose `ps -o comm=` names a shell: ```bash case "$comm" in sh|bash|zsh|dash|ksh) continue ;; esac ``` ### Hole 1 — an exited pid is counted `pgrep` lists a pid, the process exits, then `ps -o comm= -p "$pid"` returns nothing. `|| true` makes `comm` empty. An empty string matches no shell name, so `continue` never fires and **the dead pid is counted**. Measured: ``` $ comm="$(ps -o comm= -p 999999 2>/dev/null || true)" # a pid that does not exist dead pid comm=[] -> COUNTED ``` That inflates the count and can trip `assert_single_daemon` with a "racing supervisor" death for a process that is already gone. It is the same false positive the ticket exists to remove, just rarer. ### Hole 2 — the exclusion is a denylist, and the daemon has a name The list covers `sh bash zsh dash ksh`. Anything else that carries the pattern in its argv is counted: ``` ssh -> COUNTED perl -> COUNTED python3 -> COUNTED ruby -> COUNTED tail -> COUNTED ``` `ssh` matters most, because the ticket names ssh as a live route. A denylist of wrappers is a list someone finds a gap in. And we do not need one, because the thing we are looking for has a fixed name. Measured on the live daemon, names only: ``` pid=30224 comm=java ``` The PR's own report states the premise — *"The daemon is always `java`"* — and then implements the weaker form. ### The fix: allowlist, not denylist Count a pid only when its `comm` is `java`. That closes hole 2 by construction and closes hole 1 for free, because an empty `comm` is not `java` either. ```bash running_pid() { local pid comm out='' for pid in $(pgrep -f "$PATTERN" 2>/dev/null || true); do comm="$(ps -o comm= -p "$pid" 2>/dev/null || true)" comm="${comm##*/}"; comm="${comm#-}" # Allowlist, not a denylist of wrappers: the daemon is a jar, so it is always `java`. # A shell, ssh, perl or any other process carrying $PATTERN in its argv is not. # An exited pid returns an empty comm here and is correctly dropped too. [ "$comm" = java ] || continue out="$out$pid"$'\n' done printf '%s' "$out" } ``` ### The objection to answer, because it is real An allowlist can under-count: if fleetd ever stops being launched by `java` — a native image, a renamed launcher — `running_pid()` returns nothing and `assert_single_daemon` stops noticing a second daemon. **That is a false negative, and for a guard that is the worse direction.** I do not think it blocks the change, because the script already carries the same assumption one line up: `PATTERN='target/fleetd.jar'`. A jar is run by `java`. If that stops being true, the pattern breaks before the allowlist does. The allowlist adds no assumption that is not already load-bearing. But say it in the comment rather than leaving it implicit, and name `PATTERN` as the sibling assumption, so whoever changes the launch method finds both. ### Acceptance for the follow-up 1. A pid whose `comm` is not `java` — including an exited pid, and including a non-shell wrapper such as `ssh` or `perl` carrying the pattern — is not counted. Test at least one non-shell case; a test that only covers shells passes today and would have passed before this correction. 2. A real `java`-named process matching the pattern is still counted, so `assert_single_daemon` still fails on a genuine second daemon. The `exec -a` standin in the PR is the right technique — point it at a `java`-named standin. 3. Break the fix on purpose again and watch the new test fail, as you did the first time. That is the part that made the first round's evidence worth reading. ### Out of scope, and correctly left alone Instance 1 (the `fleetd.out` log source) stays untouched — it needs a systemd host. The peer fleet runs `systemd --user` and has confirmed `fleetd.out` is empty there; that half is going to them. The deliberate choice **not** to wire pid counting to `SUPERVISOR_KIND` — because `OLD_PID` is computed before the supervisor is detected, so it would have meant reordering the main flow — was the right call for this scope, and saying so in the report rather than silently narrowing was the right way to handle it.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
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
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#593
Delete Branch "%!s()"

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?