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

The swap guard can be disabled with the suite green — a "successful" redeploy that never puts the new jar in place #521

New Issue
Closed
opened 2026-09-12 06:29:41 +02:00 by ltms · 1 comment
ltms commented 2026-09-12 06:29:41 +02:00
Owner
Copy Link
Copy Source

Flagged by the #520 worker as an out-of-scope observation, and I reproduced it myself before
filing. This is the same shape as #517 defect 1, on a branch whose failure is worse.

The mutation

scripts/redeploy-fleetd.sh:723, immediately above the swap:

-if [ "$DO_BUILD" = 1 ]; then
+if false; then
   say "swap"
   swap_staged_jar "$JAR_STAGED" "$JAR"
   ok "jar in place: $(jar_id)"
 fi

Proven applied, four ways:

grep -n '^if false; then'                    -> 723:if false; then          (rc=0)
sed -n '723p' … | grep -c 'DO_BUILD'         -> 0                           (original gone)
re-read of lines 722-726                     -> shows `if false; then`
shasum -a 256                                -> 809d2f466e7ddb2c…, pristine was 2cb83dc380c72261…

Result:

bash scripts/test-redeploy-fleetd.sh
exit=0
real FAIL lines: 0

Restored byte-identical to 2cb83dc380c7226191d657c40fccdfc856904e40b2d03d0851fb6e522cee2f41,
control rerun exit 0.

Harness proof, on my own invocation of the suite, because a green result is unreadable without
one. I re-applied the jar_id absent → present mutation that #520 had just pinned:

exit=1
FAIL: jar_id with no arguments must report absent when $JAR does not exist: expected absent, got present

So the suite can fail when I run it. It does not fail for the swap guard.

What the mutant does on a live redeploy

The build runs. stage_built_jar moves the freshly built jar off the live path to $JAR_STAGED
(:621). The daemon stops. wait_for_daemon_exit confirms the old pid is gone. The swap never
happens. Then the start step runs.

So the operator sees a redeploy that reports every step succeeding, and the daemon either fails to
start at all — $JAR no longer exists, because staging moved it — or, worse, starts on whatever
stale jar happened to still be at the live path.

That is #493's outcome reached by a different route. #493 was "the build writes into the path a
running process holds"; this is "the staged jar is never put back". Both end with the daemon not
running the code that was just built, and both report success.

Why nothing catches it

test_swap_ordered_after_wait_and_before_start is a source-shape test. It checks the relative
line positions of the call sites in the script's own text — that swap_staged_jar appears after
wait_for_daemon_exit and before the start step.

Mutating if [ "$DO_BUILD" = 1 ] to if false is a one-line in-place edit. Every line position the
test looks at is unchanged. The test passes, and it was never able to fail for this.

This is #517's finding exactly, one branch over: a source-text or source-position assertion pins
what the file says and can never pin whether the code is reached. #517 named it as the fifth
member of the vacuous-test family. This is its second instance in the same file, and the worker found
it by being asked to look for the shape rather than the instance.

Severity

Higher than #517 defect 1. That one produced a wrong abort message during an abort the operator had
already chosen. This one produces a silent wrong outcome during a redeploy the operator believes
succeeded, and the next thing that happens is the fleet running code nobody intended.

The one mitigation already in the script: the final verify step checks /healthz and the jar id. It
is worth checking whether that would catch this mutant — if ok "jar in place: $(jar_id)" is inside
the same disabled block, the reported jar id is never printed either, and the summary line's jar id
comes from elsewhere. Someone should establish that rather than assume it.

The fix

Same as #517 defect 1, and the precedent is now in the file twice (wait_for_daemon_exit,
drain_gate_refusal): extract the decision so the suite can call it.

Something like should_swap(do_build) returning a status, with the main flow calling it, plus tests
for both values. Keep test_swap_ordered_after_wait_and_before_start — it catches a re-ordering,
which the new test cannot.

Then sweep the rest of the file for the same shape and report what you find: any branch whose only
test is a grep of this script's own source. Do not fix those in this ticket; list them.

Acceptance

  • bash scripts/test-redeploy-fleetd.sh exits 0, with the test count up by the number added. Report
    test functions defined and invoked; they must match.
  • bash -n passes on both scripts.
  • The mutation above is killed. Reproduce it, show the suite red, quote the FAIL line and the exit
    code, restore, confirm byte-identical with shasum -a 256, then a green control run.
  • Prove the mutation applied with two greps using different search strings and a grep -n
    re-read of the line. A $-variable inside a double-quoted $( ) is expanded by your own shell
    before grep sees it, so both greps come back empty and it looks like the mutation never applied.
    A genuinely un-applied mutation gives 0 and 1, never 0 and 0. Use single quotes.
  • Do not run scripts/redeploy-fleetd.sh against the live daemon, with any flag. It is the
    channel the fleet talks through.

Related

  • #517 / #520 — the ticket this was found while adjudicating, and the same defect shape.
  • #493 — the incident this mutant reproduces by another route.
  • #512 — the third outstanding defect in this script.
  • #504 — four swallowed-failure sites in the same file, still untouched.
Flagged by the #520 worker as an out-of-scope observation, and **I reproduced it myself** before filing. This is the same shape as #517 defect 1, on a branch whose failure is worse. ## The mutation `scripts/redeploy-fleetd.sh:723`, immediately above the swap: ```diff -if [ "$DO_BUILD" = 1 ]; then +if false; then say "swap" swap_staged_jar "$JAR_STAGED" "$JAR" ok "jar in place: $(jar_id)" fi ``` Proven applied, four ways: ``` grep -n '^if false; then' -> 723:if false; then (rc=0) sed -n '723p' … | grep -c 'DO_BUILD' -> 0 (original gone) re-read of lines 722-726 -> shows `if false; then` shasum -a 256 -> 809d2f466e7ddb2c…, pristine was 2cb83dc380c72261… ``` Result: ``` bash scripts/test-redeploy-fleetd.sh exit=0 real FAIL lines: 0 ``` Restored byte-identical to `2cb83dc380c7226191d657c40fccdfc856904e40b2d03d0851fb6e522cee2f41`, control rerun exit 0. **Harness proof, on my own invocation of the suite**, because a green result is unreadable without one. I re-applied the `jar_id` `absent` → `present` mutation that #520 had just pinned: ``` exit=1 FAIL: jar_id with no arguments must report absent when $JAR does not exist: expected absent, got present ``` So the suite can fail when I run it. It does not fail for the swap guard. ## What the mutant does on a live redeploy The build runs. `stage_built_jar` moves the freshly built jar off the live path to `$JAR_STAGED` (`:621`). The daemon stops. `wait_for_daemon_exit` confirms the old pid is gone. **The swap never happens.** Then the start step runs. So the operator sees a redeploy that reports every step succeeding, and the daemon either fails to start at all — `$JAR` no longer exists, because staging moved it — or, worse, starts on whatever stale jar happened to still be at the live path. That is #493's outcome reached by a different route. #493 was "the build writes into the path a running process holds"; this is "the staged jar is never put back". Both end with the daemon not running the code that was just built, and both report success. ## Why nothing catches it `test_swap_ordered_after_wait_and_before_start` is a **source-shape** test. It checks the relative line positions of the call sites in the script's own text — that `swap_staged_jar` appears after `wait_for_daemon_exit` and before the start step. Mutating `if [ "$DO_BUILD" = 1 ]` to `if false` is a one-line in-place edit. Every line position the test looks at is unchanged. The test passes, and it was never able to fail for this. **This is #517's finding exactly, one branch over:** a source-text or source-position assertion pins what the file *says* and can never pin whether the code is *reached*. #517 named it as the fifth member of the vacuous-test family. This is its second instance in the same file, and the worker found it by being asked to look for the shape rather than the instance. ## Severity Higher than #517 defect 1. That one produced a wrong abort message during an abort the operator had already chosen. This one produces a **silent wrong outcome during a redeploy the operator believes succeeded**, and the next thing that happens is the fleet running code nobody intended. The one mitigation already in the script: the final verify step checks `/healthz` and the jar id. It is worth checking whether that would catch this mutant — if `ok "jar in place: $(jar_id)"` is inside the same disabled block, the reported jar id is never printed either, and the summary line's jar id comes from elsewhere. Someone should establish that rather than assume it. ## The fix Same as #517 defect 1, and the precedent is now in the file twice (`wait_for_daemon_exit`, `drain_gate_refusal`): extract the decision so the suite can call it. Something like `should_swap(do_build)` returning a status, with the main flow calling it, plus tests for both values. Keep `test_swap_ordered_after_wait_and_before_start` — it catches a re-ordering, which the new test cannot. Then sweep the rest of the file for the same shape and report what you find: **any branch whose only test is a grep of this script's own source.** Do not fix those in this ticket; list them. ## Acceptance - `bash scripts/test-redeploy-fleetd.sh` exits 0, with the test count up by the number added. Report test functions defined and invoked; they must match. - `bash -n` passes on both scripts. - **The mutation above is killed.** Reproduce it, show the suite red, quote the FAIL line and the exit code, restore, confirm byte-identical with `shasum -a 256`, then a green control run. - Prove the mutation applied with two greps using *different* search strings **and** a `grep -n` re-read of the line. A `$`-variable inside a double-quoted `$( )` is expanded by your own shell before grep sees it, so both greps come back empty and it looks like the mutation never applied. A genuinely un-applied mutation gives 0 and 1, never 0 and 0. Use single quotes. - Do **not** run `scripts/redeploy-fleetd.sh` against the live daemon, with any flag. It is the channel the fleet talks through. ## Related - #517 / #520 — the ticket this was found while adjudicating, and the same defect shape. - #493 — the incident this mutant reproduces by another route. - #512 — the third outstanding defect in this script. - #504 — four swallowed-failure sites in the same file, still untouched.
ltms commented 2026-09-12 07:03:23 +02:00
Author
Owner
Copy Link
Copy Source

Closed by #526 (merged). Ticked against this ticket's own fix and acceptance sections.

The fix section

asked done
extract the decision — "something like should_swap(do_build) returning a status, with the main flow calling it, plus tests for both values" yes, by the implementer
keep test_swap_ordered_after_wait_and_before_start, since it catches a re-ordering the new test cannot kept; its needle had to follow the call site to swap_if_built "$DO_BUILD"
sweep the file for the same shape, list and do not fix done — exactly two source-text-only tests, both already known, and no third. The implementer went further and listed seven main-flow decisions with no test at all; carried to #528

But the fix this ticket prescribed does not close this ticket, and that is my error. Extracting a predicate pins the decision and never the wiring: with the main flow reading if should_swap "$DO_BUILD"; then, changing it to if false; then left the whole suite at exit 0 with zero FAIL lines. The implementer measured that, reported it in the PR body, and correctly declined to fix it because the ticket asked for the predicate shape. I reproduced it independently, then finished it at the gate.

What actually closes it: the decision and the action now live together in swap_if_built(), which the main flow calls unconditionally, so there is no guard left in the main flow to get wrong. Two tests drive it with a recording stub in place of the real mv, so they fail if the guard is removed, inverted, or stops being consulted. should_swap() stays as the named decision with its own tests, but is no longer the only thing tested.

Acceptance

  • suite exits 0, count up by the number added, defined = invoked — exit 0, 0 ^FAIL: lines, 44 defined / 44 invoked (was 40/40 on main at c71ac23; +2 from the implementer, +2 from me).

  • bash -n on both scripts — rc=0 under /bin/bash 3.2.57 and bash 5.3.9.

  • the mutation is killed — the literal mutation in this ticket no longer exists, because the guard it edits is gone from the main flow. Four mutations covering the same behaviour are killed instead, each with its own named FAIL line, each restored byte-identical by hash, green control after the battery:

    mutation FAIL line
    guard removed inside swap_if_built swap_if_built 0 (--no-build; nothing was staged this run) must not swap, but it did
    guard inverted swap_if_built 1 (a build ran and staged a jar) must perform the swap, and did not
    should_swap's comparison changed should_swap 1 (a build ran and staged a jar) must return true
    main-flow call deleted could not find the swap call site in redeploy-fleetd.sh

  • two greps with different strings, plus a grep -n re-read — done. A correction to my own first battery: three cells read "original gone = 0" because I left \" inside an already-single-quoted pattern, so the backslashes went into the pattern and matched nothing. That is a false zero from a different cause than the $-expansion trap this ticket warns about, with the identical signature. Re-proved with correct patterns plus a control showing each matches 1 in the unmutated file. Worth adding to the warning: both quoting mistakes give 0 and 0.

  • did not run redeploy-fleetd.sh against the live daemon — correct, neither of us did, with any flag.

One correction to this ticket's own text

The severity section says this produces "a silent wrong outcome during a redeploy the operator believes succeeded". That is wrong, and the implementer established it rather than assuming, which is what the ticket asked for. stage_built_jar is unconditional, so $JAR is already absent by the swap step; java -jar fails at once and the script dies at no process appeared or /healthz never answered within ${HEALTH_WAIT}s, both printing the daemon log tail. It is a loud failure. The defect was real — a guard disableable with a green suite — but it never reported success. Not re-verified by me, and not run against the live daemon.

Found while verifying, fixed here

The ordering test's three [ -n ... ] || fail "could not find … call site" guards were dead code. Under set -euo pipefail an absent needle fails the assignment and set -e kills the suite before the guard runs — measured: deleting the swap call gave exit 1 with zero bytes of output and no FAIL line. Each grep now ends || true. Same deletion now names the missing call site. The pattern may exist in other scripts; flagged in #528 for a sweep.

Follow-up

#528 — the same unpinned-call-site shape on drain_gate_refusal, which can silently undo #517 one day after #520 merged, plus the pipefail dead-check sweep.

Closed by #526 (merged). Ticked against this ticket's own fix and acceptance sections. ## The fix section | asked | done | |---|---| | extract the decision — "something like `should_swap(do_build)` returning a status, with the main flow calling it, plus tests for both values" | yes, by the implementer | | keep `test_swap_ordered_after_wait_and_before_start`, since it catches a re-ordering the new test cannot | kept; its needle had to follow the call site to `swap_if_built "$DO_BUILD"` | | sweep the file for the same shape, list and do not fix | done — exactly two source-text-only tests, both already known, and no third. The implementer went further and listed seven main-flow decisions with no test at all; carried to #528 | **But the fix this ticket prescribed does not close this ticket, and that is my error.** Extracting a predicate pins the decision and never the wiring: with the main flow reading `if should_swap "$DO_BUILD"; then`, changing it to `if false; then` left the whole suite at exit 0 with zero FAIL lines. The implementer measured that, reported it in the PR body, and correctly declined to fix it because the ticket asked for the predicate shape. I reproduced it independently, then finished it at the gate. What actually closes it: the decision and the action now live together in `swap_if_built()`, which the main flow calls unconditionally, so there is no guard left in the main flow to get wrong. Two tests drive it with a recording stub in place of the real `mv`, so they fail if the guard is removed, inverted, **or stops being consulted**. `should_swap()` stays as the named decision with its own tests, but is no longer the only thing tested. ## Acceptance - **suite exits 0, count up by the number added, defined = invoked** — exit 0, 0 `^FAIL:` lines, **44 defined / 44 invoked** (was 40/40 on main at c71ac23; +2 from the implementer, +2 from me). - **`bash -n` on both scripts** — rc=0 under `/bin/bash` 3.2.57 and `bash` 5.3.9. - **the mutation is killed** — the literal mutation in this ticket no longer exists, because the guard it edits is gone from the main flow. Four mutations covering the same behaviour are killed instead, each with its own named FAIL line, each restored byte-identical by hash, green control after the battery: | mutation | FAIL line | |---|---| | guard removed inside `swap_if_built` | `swap_if_built 0 (--no-build; nothing was staged this run) must not swap, but it did` | | guard inverted | `swap_if_built 1 (a build ran and staged a jar) must perform the swap, and did not` | | `should_swap`'s comparison changed | `should_swap 1 (a build ran and staged a jar) must return true` | | main-flow call deleted | `could not find the swap call site in redeploy-fleetd.sh` | - **two greps with different strings, plus a `grep -n` re-read** — done. A correction to my own first battery: three cells read "original gone = 0" because I left `\"` inside an already-single-quoted pattern, so the backslashes went into the pattern and matched nothing. That is a false zero from a *different* cause than the `$`-expansion trap this ticket warns about, with the identical signature. Re-proved with correct patterns plus a control showing each matches 1 in the unmutated file. Worth adding to the warning: both quoting mistakes give 0 and 0. - **did not run `redeploy-fleetd.sh` against the live daemon** — correct, neither of us did, with any flag. ## One correction to this ticket's own text The severity section says this produces "a silent wrong outcome during a redeploy the operator believes succeeded". That is wrong, and the implementer established it rather than assuming, which is what the ticket asked for. `stage_built_jar` is unconditional, so `$JAR` is already absent by the swap step; `java -jar` fails at once and the script dies at `no process appeared` or `/healthz never answered within ${HEALTH_WAIT}s`, both printing the daemon log tail. It is a **loud** failure. The defect was real — a guard disableable with a green suite — but it never reported success. Not re-verified by me, and not run against the live daemon. ## Found while verifying, fixed here The ordering test's three `[ -n ... ] || fail "could not find … call site"` guards were dead code. Under `set -euo pipefail` an absent needle fails the assignment and `set -e` kills the suite before the guard runs — measured: deleting the swap call gave exit 1 with **zero bytes** of output and no FAIL line. Each grep now ends `|| true`. Same deletion now names the missing call site. The pattern may exist in other scripts; flagged in #528 for a sweep. ## Follow-up **#528** — the same unpinned-call-site shape on `drain_gate_refusal`, which can silently undo #517 one day after #520 merged, plus the `pipefail` dead-check sweep.
ltms closed this issue 2026-09-12 07:03:29 +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#521
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?