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

#664 follow-up: the runtime-vs-build path split is unpinned by tests, and nothing checks the installed plist's jar path #680

New Issue
Closed
opened 2026-10-03 21:14:36 +02:00 by ltms · 4 comments
ltms commented 2026-10-03 21:14:36 +02:00
Owner
Copy Link
Copy Source

Two gaps I found while verifying PR #679 before merging it (merged as 5051a06). The implementation is correct — these are guards it did not bring with it. Both live in scripts/redeploy-fleetd.sh and its suite.

Part 1 — reverting the core change leaves the whole suite green

I mutated the merged tree and ran scripts/test-redeploy-fleetd.sh:

mutation result
invert report_jar_state's mismatch condition (!= → =) killed — exit 1
JAR="$MODULE/run/fleetd.jar" → JAR="$MODULE/target/fleetd.jar" survived — exit 0, output byte-identical to baseline

The second mutation reverts the entire point of #664, and all 129 test functions still pass.

Why it survives, measured — this is "no assertion", not "cannot see". Mutation 1 killing proves the suite observes sourced globals, so the mechanism works. The cause is that every test supplies its own paths before calling anything:

570:  JAR="$dir/fleetd.jar"
615:  JAR="$dir/does-not-exist.jar"
662:  BUILD_JAR="$dir/target-fleetd.jar"; JAR="$dir/run-fleetd.jar"
681:  BUILD_JAR="$dir/target-fleetd.jar"; JAR="$dir/run-fleetd.jar"
695:  BUILD_JAR="$dir/no-such-target.jar"; JAR="$dir/no-such-run.jar"

No test ever observes the script's real JAR, and nothing anywhere asserts JAR is outside target/. A test that supplies its own dependency says nothing about the producer.

The property was checked — PR #679's acceptance criterion 1 ran source scripts/redeploy-fleetd.sh; echo $BUILD_JAR; echo $JAR and reported DIFFER: yes. That is a correct check made once by hand. It is not a guard, and the hand-check is exactly what the mutation shows is missing.

Part 2 — the installed launchd plist still names the old path, and nothing notices

Measured on this host 2026-10-03 21:10 CEST:

$ /usr/libexec/PlistBuddy -c 'Print :ProgramArguments' ~/Library/LaunchAgents/dev.ltms.fleetd.plist
    .../scripts/fleetd-launchd-wrapper.sh
    .../bin/java
    -jar
    /Users/dai.ha/LTMS/claude-bridge/fleetd/target/fleetd.jar      <-- OLD path
    fleetd.yaml

$ /usr/libexec/PlistBuddy -c 'Print :ProgramArguments' deploy/dev.ltms.fleetd.plist   # repo, post-#664
    .../fleetd/run/fleetd.jar                                       <-- NEW path

The daemon is launchd-supervised right now (launchctl list → 42543 0 dev.ltms.fleetd, ppid 1). The repo's deploy/dev.ltms.fleetd.plist is a template; the installed copy in ~/Library/LaunchAgents/ is a separate file, last written Aug 25, and editing the template does not touch it.

This matters because the swap is mv "$BUILD_JAR" "$JAR" — a rename, so after a redeploy target/fleetd.jar no longer exists. Any launchd-initiated start from the stale installed plist (a reboot, or KeepAlive after a crash) then runs java -jar …/target/fleetd.jar against a missing file.

The script already has the precedent and the seam. check_log_path_matches_plist() exists for precisely this class of drift, and its own comment says "Nothing forced the two to agree." It reads StandardOutPath only. It never reads ProgramArguments:

$ grep -n 'ProgramArguments' scripts/redeploy-fleetd.sh
1371:# Supervised (launchd): launchd does both — deploy/dev.ltms.fleetd.plist points ProgramArguments at

One comment, no code. This is a one-way gate: a guard written after the log-path incident closes only the direction that incident came from.

I am handling the plist reinstall by hand for today's redeploy, so this ticket is about the guard, not about unblocking me.

Acceptance criteria

Properties under a change, not names of constructs.

Part 1

  1. Changing JAR to any path under $MODULE/target/ makes scripts/test-redeploy-fleetd.sh exit non-zero. Changing it to a different path outside target/ still passes — both directions, so the test is not satisfied by refusing every edit.
  2. The assertion reads the script's own JAR/BUILD_JAR as sourced, without the test assigning them first. A test that sets them and then checks what it set is the defect being fixed, not the fix.
  3. JAR != BUILD_JAR is asserted.

Part 2
4. When the launchd agent is installed and its ProgramArguments jar path does not resolve to $JAR, the script refuses with a message naming both paths and saying the installed plist needs reinstalling. Make it fail the same way check_log_path_matches_plist does.
5. When the two agree, the script proceeds and says so, in the style of the existing ok "log path check: …" line.
6. A host with no installed plist is unaffected — no new failure on the unsupervised path. The existing launchd_installed() seam already answers this.
7. A positive control: a test proves the new check can actually fail, by pointing it at a plist whose jar path differs.

Notes for whoever takes this

  • report_jar_state prints absent for the built jar right after a successful redeploy, because the swap moves target/fleetd.jar rather than copying it. That is correct behaviour and not part of this ticket, but do not "fix" it into a copy — the rename is what makes the swap atomic on one filesystem.
  • Comments go in as the rule here requires: describe the code as it is, no ticket keys, no dates, no history, no measurements. The evidence above belongs in the commit message, not in the script.

Same shape, found and not fixed

PR #679's author reported scripts/rename-checkout.sh:74 (PATTERN="target/$JAR_NAME") and :428 (JAR="$NEW_PATH/$MODULE_SUB/target/$JAR_NAME") as the same build-output-path-doubling-as-locator shape. It is a one-off historical migration tool. Worth a look while in here, but not in scope unless it is trivially the same fix.

Two gaps I found while verifying PR #679 before merging it (merged as `5051a06`). The implementation is correct — these are guards it did not bring with it. Both live in `scripts/redeploy-fleetd.sh` and its suite. ## Part 1 — reverting the core change leaves the whole suite green I mutated the merged tree and ran `scripts/test-redeploy-fleetd.sh`: | mutation | result | |---|---| | invert `report_jar_state`'s mismatch condition (`!=` → `=`) | **killed** — exit 1 | | `JAR="$MODULE/run/fleetd.jar"` → `JAR="$MODULE/target/fleetd.jar"` | **survived** — exit 0, output byte-identical to baseline | The second mutation reverts the entire point of #664, and all 129 test functions still pass. **Why it survives, measured — this is "no assertion", not "cannot see".** Mutation 1 killing proves the suite observes sourced globals, so the mechanism works. The cause is that every test supplies its own paths before calling anything: ``` 570: JAR="$dir/fleetd.jar" 615: JAR="$dir/does-not-exist.jar" 662: BUILD_JAR="$dir/target-fleetd.jar"; JAR="$dir/run-fleetd.jar" 681: BUILD_JAR="$dir/target-fleetd.jar"; JAR="$dir/run-fleetd.jar" 695: BUILD_JAR="$dir/no-such-target.jar"; JAR="$dir/no-such-run.jar" ``` No test ever observes the script's real `JAR`, and nothing anywhere asserts `JAR` is outside `target/`. A test that supplies its own dependency says nothing about the producer. The property *was* checked — PR #679's acceptance criterion 1 ran `source scripts/redeploy-fleetd.sh; echo $BUILD_JAR; echo $JAR` and reported `DIFFER: yes`. That is a correct check made once by hand. It is not a guard, and the hand-check is exactly what the mutation shows is missing. ## Part 2 — the installed launchd plist still names the old path, and nothing notices Measured on this host 2026-10-03 21:10 CEST: ``` $ /usr/libexec/PlistBuddy -c 'Print :ProgramArguments' ~/Library/LaunchAgents/dev.ltms.fleetd.plist .../scripts/fleetd-launchd-wrapper.sh .../bin/java -jar /Users/dai.ha/LTMS/claude-bridge/fleetd/target/fleetd.jar <-- OLD path fleetd.yaml $ /usr/libexec/PlistBuddy -c 'Print :ProgramArguments' deploy/dev.ltms.fleetd.plist # repo, post-#664 .../fleetd/run/fleetd.jar <-- NEW path ``` The daemon is launchd-supervised right now (`launchctl list` → `42543 0 dev.ltms.fleetd`, ppid 1). The repo's `deploy/dev.ltms.fleetd.plist` is a **template**; the installed copy in `~/Library/LaunchAgents/` is a separate file, last written Aug 25, and editing the template does not touch it. This matters because the swap is `mv "$BUILD_JAR" "$JAR"` — a rename, so after a redeploy **`target/fleetd.jar` no longer exists**. Any launchd-initiated start from the stale installed plist (a reboot, or `KeepAlive` after a crash) then runs `java -jar …/target/fleetd.jar` against a missing file. **The script already has the precedent and the seam.** `check_log_path_matches_plist()` exists for precisely this class of drift, and its own comment says *"Nothing forced the two to agree."* It reads `StandardOutPath` only. It never reads `ProgramArguments`: ``` $ grep -n 'ProgramArguments' scripts/redeploy-fleetd.sh 1371:# Supervised (launchd): launchd does both — deploy/dev.ltms.fleetd.plist points ProgramArguments at ``` One comment, no code. This is a one-way gate: a guard written after the log-path incident closes only the direction that incident came from. I am handling the plist reinstall by hand for today's redeploy, so this ticket is about the guard, not about unblocking me. ## Acceptance criteria Properties under a change, not names of constructs. **Part 1** 1. Changing `JAR` to any path under `$MODULE/target/` makes `scripts/test-redeploy-fleetd.sh` exit non-zero. Changing it to a different path outside `target/` still passes — both directions, so the test is not satisfied by refusing every edit. 2. The assertion reads the script's own `JAR`/`BUILD_JAR` as sourced, without the test assigning them first. A test that sets them and then checks what it set is the defect being fixed, not the fix. 3. `JAR != BUILD_JAR` is asserted. **Part 2** 4. When the launchd agent is installed and its `ProgramArguments` jar path does not resolve to `$JAR`, the script refuses with a message naming both paths and saying the installed plist needs reinstalling. Make it fail the same way `check_log_path_matches_plist` does. 5. When the two agree, the script proceeds and says so, in the style of the existing `ok "log path check: …"` line. 6. A host with no installed plist is unaffected — no new failure on the unsupervised path. The existing `launchd_installed()` seam already answers this. 7. A positive control: a test proves the new check can actually fail, by pointing it at a plist whose jar path differs. ## Notes for whoever takes this - `report_jar_state` prints `absent` for the built jar right after a successful redeploy, because the swap *moves* `target/fleetd.jar` rather than copying it. That is correct behaviour and not part of this ticket, but do not "fix" it into a copy — the rename is what makes the swap atomic on one filesystem. - Comments go in as the rule here requires: describe the code as it is, no ticket keys, no dates, no history, no measurements. The evidence above belongs in the commit message, not in the script. ## Same shape, found and not fixed PR #679's author reported `scripts/rename-checkout.sh:74` (`PATTERN="target/$JAR_NAME"`) and `:428` (`JAR="$NEW_PATH/$MODULE_SUB/target/$JAR_NAME"`) as the same build-output-path-doubling-as-locator shape. It is a one-off historical migration tool. Worth a look while in here, but not in scope unless it is trivially the same fix.
ltms commented 2026-10-03 21:16:25 +02:00
Author
Owner
Copy Link
Copy Source

PART 3 — added after the brief. This comment is newer than your brief, so it wins where they differ. Same scope (scripts/redeploy-fleetd.sh + its suite), and it is the most urgent of the three.

The merged script cannot see a daemon started under the old layout

#664 changed PATTERN from target/fleetd.jar to run/fleetd.jar. running_pid() is built on that pattern, and so is assert_single_daemon. Measured on this host just now, against the real live daemon plus a simulated process, with a positive control:

$ grep -n "^PATTERN=" scripts/redeploy-fleetd.sh
88:PATTERN='run/fleetd.jar'

# live daemon 42543 runs from target/fleetd.jar; 94037 is a simulated hand-run from target/
$ pgrep -f 'run/fleetd.jar'
                      <-- EMPTY: finds neither
$ pgrep -f 'target/fleetd.jar'
42543 94037           <-- positive control: both processes were findable

So on the next redeploy, OLD_PID comes back empty, the script prints no daemon running — this will be a cold start, skips the drain gate and the stop-and-wait entirely, and starts a second daemon while the first is still up. Two daemons against one herdr session take each other's members down.

This is a transition hazard, but the transition is the next thing that happens, and it is also permanent for a hand-run: anyone who runs java -jar fleetd/target/fleetd.jar by hand is now invisible to assert_single_daemon, which is the guard whose entire job is noticing a second daemon.

What to change

Make the daemon-detection pattern match a fleetd daemon whatever directory its jar sits in, while JAR stays the single runtime path the script deploys to and starts from. Those are two different jobs that #664 accidentally merged:

  • JAR / BUILD_JAR — where this script puts things and starts from. Keep exactly as merged.
  • the detection pattern — what counts as "a fleetd daemon is running". This must stay broad, because its purpose is to find daemons this script did not start, including wrong ones.

Narrowing a safety guard's pattern to the happy path is how the guard stops guarding.

Mind the existing running_pid() comm = java allowlist — it already filters out shells that merely mention the pattern, so broadening the pattern does not reintroduce the sh -c false positive that #593 fixed. Keep that allowlist.

Acceptance criteria for part 3

  1. With a process whose command line names a jar under target/, running_pid() finds it. With one naming a jar under run/, running_pid() finds it. Both asserted — a test that only checks the second direction is the bug.
  2. The #593 false-positive case still does not match: a non-exec'ing sh -c that merely contains the pattern text is still excluded. Keep a positive control so this test can actually fail.
  3. assert_single_daemon notices two daemons when one runs from target/ and one from run/. This is the case that bites today.

Note on the other two parts

Part 1's mutation (JAR → a path under target/) must still make the suite red. Part 3 broadens only the detection pattern, not JAR, so the two do not conflict. If you find they do, say so in your reply rather than quietly relaxing part 1.

I am holding today's redeploy until this lands, so take the time to get the test directions right. Do not run the redeploy script, and do not touch the live daemon.

**PART 3 — added after the brief. This comment is newer than your brief, so it wins where they differ.** Same scope (`scripts/redeploy-fleetd.sh` + its suite), and it is the most urgent of the three. ## The merged script cannot see a daemon started under the old layout #664 changed `PATTERN` from `target/fleetd.jar` to `run/fleetd.jar`. `running_pid()` is built on that pattern, and so is `assert_single_daemon`. Measured on this host just now, against the real live daemon plus a simulated process, with a positive control: ``` $ grep -n "^PATTERN=" scripts/redeploy-fleetd.sh 88:PATTERN='run/fleetd.jar' # live daemon 42543 runs from target/fleetd.jar; 94037 is a simulated hand-run from target/ $ pgrep -f 'run/fleetd.jar' <-- EMPTY: finds neither $ pgrep -f 'target/fleetd.jar' 42543 94037 <-- positive control: both processes were findable ``` So on the next redeploy, `OLD_PID` comes back empty, the script prints `no daemon running — this will be a cold start`, skips the drain gate and the stop-and-wait entirely, and starts a second daemon while the first is still up. Two daemons against one herdr session take each other's members down. This is a transition hazard, but the transition is the next thing that happens, and it is also permanent for a hand-run: anyone who runs `java -jar fleetd/target/fleetd.jar` by hand is now invisible to `assert_single_daemon`, which is the guard whose entire job is noticing a second daemon. ## What to change Make the daemon-detection pattern match a fleetd daemon **whatever directory its jar sits in**, while `JAR` stays the single runtime path the script deploys to and starts from. Those are two different jobs that #664 accidentally merged: - `JAR` / `BUILD_JAR` — where this script *puts* things and *starts* from. Keep exactly as merged. - the detection pattern — what counts as "a fleetd daemon is running". This must stay broad, because its purpose is to find daemons this script did **not** start, including wrong ones. Narrowing a safety guard's pattern to the happy path is how the guard stops guarding. Mind the existing `running_pid()` `comm = java` allowlist — it already filters out shells that merely mention the pattern, so broadening the pattern does not reintroduce the `sh -c` false positive that #593 fixed. Keep that allowlist. ## Acceptance criteria for part 3 8. With a process whose command line names a jar under `target/`, `running_pid()` finds it. With one naming a jar under `run/`, `running_pid()` finds it. Both asserted — a test that only checks the second direction is the bug. 9. The `#593` false-positive case still does not match: a non-exec'ing `sh -c` that merely contains the pattern text is still excluded. Keep a positive control so this test can actually fail. 10. `assert_single_daemon` notices two daemons when one runs from `target/` and one from `run/`. This is the case that bites today. ## Note on the other two parts Part 1's mutation (`JAR` → a path under `target/`) must still make the suite red. Part 3 broadens only the *detection* pattern, not `JAR`, so the two do not conflict. If you find they do, say so in your reply rather than quietly relaxing part 1. I am holding today's redeploy until this lands, so take the time to get the test directions right. Do not run the redeploy script, and do not touch the live daemon.
ltms commented 2026-10-03 21:16:53 +02:00
Author
Owner
Copy Link
Copy Source

Scope addendum to part 3 — one more file. My brief said "scripts/redeploy-fleetd.sh and scripts/test-redeploy-fleetd.sh only". That was too narrow. Part 3 also covers one line in .claude/skills/fleets-status/SKILL.md.

There are exactly two daemon-locator sites in the repo, and #664 narrowed both:

$ grep -rn "pgrep -f.*fleetd\.jar\|PATTERN=.*fleetd\.jar" --include='*.sh' --include='*.md' . | grep -v '^./wiki/'
.claude/skills/fleets-status/SKILL.md:62:PIDS="$(pgrep -f 'run/fleetd.jar' || true)"
scripts/redeploy-fleetd.sh:88:PATTERN='run/fleetd.jar'
scripts/redeploy-fleetd.sh:234:# though: `PATTERN='run/fleetd.jar'` two lines up already assumes the daemon is a jar, which

(The third hit is a comment inside the script; update it only if your change makes it untrue.)

Against the live daemon today, the fleets-status line prints fleetd: not running — the daemon is up as pid 42543, it just runs from target/. A status tool that reports a running daemon as down is worse than one that says nothing, because the reader acts on it.

Fix that line the same way you fix PATTERN, so the two agree. The skill is a Markdown playbook, not shell the suite can source, so it gets no test — just make the two patterns identical, and say in your reply that you checked they match.

This is the shape worth naming: fleets-status and redeploy-fleetd.sh each keep their own copy of "how to find the daemon". One fact, two places, and #664 updated both to the same wrong value in one pass. Do not add a third copy to fix it. If you see a clean way to make the skill reference the script's value rather than restate it, say so in your reply — but do not build it in this ticket.

**Scope addendum to part 3 — one more file.** My brief said "`scripts/redeploy-fleetd.sh` and `scripts/test-redeploy-fleetd.sh` only". That was too narrow. Part 3 also covers one line in `.claude/skills/fleets-status/SKILL.md`. There are exactly two daemon-locator sites in the repo, and #664 narrowed both: ``` $ grep -rn "pgrep -f.*fleetd\.jar\|PATTERN=.*fleetd\.jar" --include='*.sh' --include='*.md' . | grep -v '^./wiki/' .claude/skills/fleets-status/SKILL.md:62:PIDS="$(pgrep -f 'run/fleetd.jar' || true)" scripts/redeploy-fleetd.sh:88:PATTERN='run/fleetd.jar' scripts/redeploy-fleetd.sh:234:# though: `PATTERN='run/fleetd.jar'` two lines up already assumes the daemon is a jar, which ``` (The third hit is a comment inside the script; update it only if your change makes it untrue.) Against the live daemon today, the `fleets-status` line prints `fleetd: not running` — the daemon is up as pid 42543, it just runs from `target/`. A status tool that reports a running daemon as down is worse than one that says nothing, because the reader acts on it. Fix that line the same way you fix `PATTERN`, so the two agree. The skill is a Markdown playbook, not shell the suite can source, so it gets no test — just make the two patterns identical, and say in your reply that you checked they match. This is the shape worth naming: `fleets-status` and `redeploy-fleetd.sh` each keep their own copy of "how to find the daemon". One fact, two places, and #664 updated both to the same wrong value in one pass. Do not add a third copy to fix it. If you see a clean way to make the skill reference the script's value rather than restate it, say so in your reply — but do **not** build it in this ticket.
ltms commented 2026-10-03 21:18:36 +02:00
Author
Owner
Copy Link
Copy Source

Correction to part 3. I wrote that an empty OLD_PID makes the script "start a second daemon beside pid 42543". That is wrong. I had not read the stop dispatch when I wrote it. The blindness is real and part 3 still stands; the consequence is different, and in one respect worse.

What actually happens with an empty OLD_PID

The stop section branches on $OLD_PID first, then on the supervisor:

if [ -n "$OLD_PID" ]; then
  dispatch_stop "$SUPERVISOR_KIND" "$OLD_PID"
  if ! wait_for_daemon_exit "$STOP_WAIT"; then die ...; fi
  ok "pid $OLD_PID exited"
elif [ "$SUPERVISOR_KIND" = "launchd" ]; then
  unload_launchd_if_loaded
  ok "launchd agent unloaded (was already not running)"

Supervisor detection does not use PATTERN — launchd_loaded() asks launchctl list. So SUPERVISOR_KIND is still launchd, the second branch runs, and launchctl unload -w does stop the running daemon. There is no second daemon. I was wrong.

The real consequences, each read off the code

  1. The drain gate is skipped silently. drain_gate_required() is [ -n "$old_pid" ] && [ "$assume_yes" = 0 ]. With OLD_PID empty it returns false and run_drain_gate returns immediately. No prompt, no "a restart drops every in-flight ticket" warning. Live members lose their reports with nothing asked and nothing printed.
  2. The script prints a false statement. ok "launchd agent unloaded (was already not running)" — while it was running. Anyone reading the transcript afterwards is told the opposite of what happened.
  3. wait_for_daemon_exit is never called on that branch, so the script proceeds without confirming the old process is gone.
  4. The shutdown-drain report is suppressed. HAD_OLD_PID="$(compute_had_old_pid "$OLD_PID")" is 0, so report_shutdown_drain is told no previous daemon was stopped. That report is exactly the signal #664's live probe needs, so the blindness also hides the evidence that would settle #664.
  5. The start then fails anyway, for the separate reason in part 2.

One thing I over-stated the risk of: the swap is mv on one filesystem, which is a rename. An already-open file descriptor follows the inode, so moving the jar does not disturb a JVM that still has it open. Overwriting would. So point 3 is a missing confirmation, not jar corruption.

What changes for you

Nothing in parts 1, 2, or 3's actual work, and nothing about criteria 8 and 9 — a detection pattern that only finds daemons in the directory you expect is still the defect.

Criterion 10 restated. I wrote "assert_single_daemon notices two daemons when one runs from target/ and one from run/ — this is the case that bites today." The property is still worth pinning, but the "bites today" framing was wrong. Treat it as: a daemon running from a jar outside $JAR's directory must still be visible to running_pid() and therefore countable by assert_single_daemon. Drop it if it fights the other criteria, and say so.

One criterion added, because it is the consequence that actually costs something:

  1. When a daemon is running but its jar sits outside $JAR's directory, the script must not reach the stop step with an empty OLD_PID. Assert that running_pid() finds it — that is enough, because the drain gate, wait_for_daemon_exit and HAD_OLD_PID all key off OLD_PID, and fixing detection fixes all four at once. Do not patch the four call sites separately.

Sorry for the churn. The blindness is measured and real; my account of the damage was not.

**Correction to part 3.** I wrote that an empty `OLD_PID` makes the script "start a second daemon beside pid 42543". That is wrong. I had not read the stop dispatch when I wrote it. The blindness is real and part 3 still stands; the consequence is different, and in one respect worse. ## What actually happens with an empty `OLD_PID` The stop section branches on `$OLD_PID` first, then on the supervisor: ```bash if [ -n "$OLD_PID" ]; then dispatch_stop "$SUPERVISOR_KIND" "$OLD_PID" if ! wait_for_daemon_exit "$STOP_WAIT"; then die ...; fi ok "pid $OLD_PID exited" elif [ "$SUPERVISOR_KIND" = "launchd" ]; then unload_launchd_if_loaded ok "launchd agent unloaded (was already not running)" ``` Supervisor detection does **not** use `PATTERN` — `launchd_loaded()` asks `launchctl list`. So `SUPERVISOR_KIND` is still `launchd`, the second branch runs, and `launchctl unload -w` does stop the running daemon. There is no second daemon. I was wrong. ## The real consequences, each read off the code 1. **The drain gate is skipped silently.** `drain_gate_required()` is `[ -n "$old_pid" ] && [ "$assume_yes" = 0 ]`. With `OLD_PID` empty it returns false and `run_drain_gate` returns immediately. No prompt, no "a restart drops every in-flight ticket" warning. Live members lose their reports with nothing asked and nothing printed. 2. **The script prints a false statement.** `ok "launchd agent unloaded (was already not running)"` — while it *was* running. Anyone reading the transcript afterwards is told the opposite of what happened. 3. **`wait_for_daemon_exit` is never called** on that branch, so the script proceeds without confirming the old process is gone. 4. **The shutdown-drain report is suppressed.** `HAD_OLD_PID="$(compute_had_old_pid "$OLD_PID")"` is 0, so `report_shutdown_drain` is told no previous daemon was stopped. That report is exactly the signal #664's live probe needs, so the blindness also hides the evidence that would settle #664. 5. The start then fails anyway, for the separate reason in part 2. **One thing I over-stated the risk of:** the swap is `mv` on one filesystem, which is a rename. An already-open file descriptor follows the inode, so moving the jar does not disturb a JVM that still has it open. Overwriting would. So point 3 is a missing confirmation, not jar corruption. ## What changes for you Nothing in parts 1, 2, or 3's actual work, and nothing about criteria 8 and 9 — a detection pattern that only finds daemons in the directory you expect is still the defect. **Criterion 10 restated.** I wrote "`assert_single_daemon` notices two daemons when one runs from `target/` and one from `run/` — this is the case that bites today." The property is still worth pinning, but the "bites today" framing was wrong. Treat it as: a daemon running from a jar outside `$JAR`'s directory must still be visible to `running_pid()` and therefore countable by `assert_single_daemon`. Drop it if it fights the other criteria, and say so. **One criterion added, because it is the consequence that actually costs something:** 11. When a daemon is running but its jar sits outside `$JAR`'s directory, the script must not reach the stop step with an empty `OLD_PID`. Assert that `running_pid()` finds it — that is enough, because the drain gate, `wait_for_daemon_exit` and `HAD_OLD_PID` all key off `OLD_PID`, and fixing detection fixes all four at once. Do not patch the four call sites separately. Sorry for the churn. The blindness is measured and real; my account of the damage was not.
ltms commented 2026-10-03 21:34:50 +02:00
Author
Owner
Copy Link
Copy Source

PR #681 merged as a6aeda3. All three parts in. Closing.

I re-ran every mutation myself rather than trusting the report

On the merged tree, baseline exit=0, 134 test functions (5 new):

mutation expected result
JAR → under $MODULE/target/ RED exit 1 — FAIL: $JAR must not live under $MODULE/target/
JAR → elsewhere outside target/ PASS exit 0 — not a refuse-everything test
PATTERN → back to run/fleetd.jar RED exit 1 — running_pid() did not find a real second process … naming a jar under target/
plist-jar mismatch die neutralised RED exit 1 — check_jar_path_matches_plist must die when the plist names a different jar
comm = java allowlist removed RED exit 1 — caught by the pre-existing self-match test

The second row is the one that matters most. A test that fires on any edit to JAR would have passed row 1 while pinning nothing; it had to stay green here. It did.

The last row answers the question widening PATTERN raises: broadening it did not weaken the #593 protection against a shell that merely holds the pattern as literal text.

mvn -o clean install from fleetd/: Tests run: 1929, Failures: 0, BUILD SUCCESS, 172 report files, 0 failure lines.

Blocker 1 is closed, verified against the real process

$ source scripts/redeploy-fleetd.sh
JAR       = /Users/dai.ha/LTMS/claude-bridge/fleetd/run/fleetd.jar
BUILD_JAR = /Users/dai.ha/LTMS/claude-bridge/fleetd/target/fleetd.jar
PATTERN   = fleetd.jar
running_pid() -> [42543]

$ lsof -nP -iTCP:8765 -sTCP:LISTEN -t
42543

The script now finds the live daemon it was blind to an hour ago. Both locator sites agree (redeploy-fleetd.sh:89 and fleets-status/SKILL.md:62, both fleetd.jar).

I checked the risk that widening PATTERN creates, because the implementer could not

My worry was assert_single_daemon false-positiving while a worker builds: Maven produces target/fleetd.jar, and surefire forks have comm=java. A count taken on an idle host would have proved nothing, so I ran a full mvn -o clean install in a throwaway worktree and sampled every 5 seconds through it:

MAX java-processes matching 'fleetd.jar' during a build: 1

Only the live daemon. Maven and surefire do not put fleetd.jar in their argv — they use classpath directories, not the shaded jar. So the widened pattern does not block a redeploy while a worker builds.

Credit where it is due

The implementer picked up all three ticket comments — including my own correction walking back the "two daemons" framing — and implemented the restated criterion 11 rather than the superseded criterion 10. It also flagged in its report that part 3 arrived after its brief. That is the ticket-beats-brief rule working as intended.

What this does to the next redeploy

check_jar_path_matches_plist is live, and the installed plist still names …/fleetd/target/fleetd.jar. So my next redeploy will now refuse with the mismatch message instead of starting a daemon against a path the swap is about to empty. That refusal is the guard doing its job; reinstalling the plist is my next step, and it is tracked on #664.

**PR #681 merged as `a6aeda3`.** All three parts in. Closing. ## I re-ran every mutation myself rather than trusting the report On the merged tree, baseline `exit=0`, 134 test functions (5 new): | mutation | expected | result | |---|---|---| | `JAR` → under `$MODULE/target/` | RED | **exit 1** — `FAIL: $JAR must not live under $MODULE/target/` | | `JAR` → elsewhere outside `target/` | **PASS** | **exit 0** — not a refuse-everything test | | `PATTERN` → back to `run/fleetd.jar` | RED | **exit 1** — `running_pid() did not find a real second process … naming a jar under target/` | | plist-jar mismatch `die` neutralised | RED | **exit 1** — `check_jar_path_matches_plist must die when the plist names a different jar` | | `comm = java` allowlist removed | RED | **exit 1** — caught by the pre-existing self-match test | The second row is the one that matters most. A test that fires on any edit to `JAR` would have passed row 1 while pinning nothing; it had to stay green here. It did. The last row answers the question widening `PATTERN` raises: broadening it did **not** weaken the `#593` protection against a shell that merely holds the pattern as literal text. `mvn -o clean install` from `fleetd/`: `Tests run: 1929, Failures: 0`, `BUILD SUCCESS`, 172 report files, 0 failure lines. ## Blocker 1 is closed, verified against the real process ``` $ source scripts/redeploy-fleetd.sh JAR = /Users/dai.ha/LTMS/claude-bridge/fleetd/run/fleetd.jar BUILD_JAR = /Users/dai.ha/LTMS/claude-bridge/fleetd/target/fleetd.jar PATTERN = fleetd.jar running_pid() -> [42543] $ lsof -nP -iTCP:8765 -sTCP:LISTEN -t 42543 ``` The script now finds the live daemon it was blind to an hour ago. Both locator sites agree (`redeploy-fleetd.sh:89` and `fleets-status/SKILL.md:62`, both `fleetd.jar`). ## I checked the risk that widening `PATTERN` creates, because the implementer could not My worry was `assert_single_daemon` false-positiving while a worker builds: Maven produces `target/fleetd.jar`, and surefire forks have `comm=java`. A count taken on an idle host would have proved nothing, so I ran a full `mvn -o clean install` in a throwaway worktree and sampled every 5 seconds through it: ``` MAX java-processes matching 'fleetd.jar' during a build: 1 ``` Only the live daemon. Maven and surefire do not put `fleetd.jar` in their argv — they use classpath directories, not the shaded jar. So the widened pattern does not block a redeploy while a worker builds. ## Credit where it is due The implementer picked up all three ticket comments — including my own correction walking back the "two daemons" framing — and implemented the restated criterion 11 rather than the superseded criterion 10. It also flagged in its report that part 3 arrived after its brief. That is the ticket-beats-brief rule working as intended. ## What this does to the next redeploy `check_jar_path_matches_plist` is live, and the installed plist still names `…/fleetd/target/fleetd.jar`. So my next redeploy will now **refuse** with the mismatch message instead of starting a daemon against a path the swap is about to empty. That refusal is the guard doing its job; reinstalling the plist is my next step, and it is tracked on #664.
ltms closed this issue 2026-10-03 21:34:56 +02:00
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#680
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?