fleetd #555: lift 8 main-flow decisions into tested predicate/dispatch functions #565

Merged

ltms merged 2 commits from worker/555-redeploy-main-flow-seam-65c2f5-2 into main 2026-09-12 12:19:48 +02:00

Conversation 0 Commits 2 Files Changed 2

+774 -145

agent commented 2026-09-12 11:47:28 +02:00

Member

Copy Link

Copy Source

Closes #555.

What changed

scripts/redeploy-fleetd.sh's main flow had 8 bare if/case decisions
living outside any function, so scripts/test-redeploy-fleetd.sh (67 tests)
could not reach them — any one could be silently inverted with the whole
suite green. This follows the existing swap_if_built/refuse_drain_gate
pattern: each bare guard becomes a small predicate or dispatch function,
called unconditionally by the main flow, so the decision is unit-testable.

The 8 lifted decisions (line numbers as measured on this branch, before the
refactor moved things around):

1. if [ "$CHECK_ONLY" = 1 ] → should_stop_for_check / stop_if_check_only
2. drain-gate entry if [ -n "$OLD_PID" ] && [ "$ASSUME_YES" = 0 ] → drain_gate_required
3. drain-gate confirm if [ "$reply" != "yes" ] → drain_confirmed
4. supervisor report case "$SUPERVISOR_KIND" → report_supervisor_state
5. supervisor stop case "$SUPERVISOR_KIND" → dispatch_stop
6. supervisor start case "$SUPERVISOR_KIND" → dispatch_start
7. health poll + if [ -z "$HEALTH_BODY" ] → poll_health_body / health_is_up / report_health
8. HAD_OLD_PID=0; [ -n "$OLD_PID" ] && HAD_OLD_PID=1 → compute_had_old_pid

The structural guard

Adds test_no_untested_main_flow_conditionals, which scans the main flow
(everything after the SOURCED guard) for bare if/elif/case lines
outside any function body — function boundaries are tracked via this file's
one consistent name() { / } convention. Any bare conditional not in the
explicit MAIN_FLOW_ALLOWED_CONDITIONALS allowlist (report-only/display
conditionals, plus the two #504-family supervisor elif branches that are
out of scope here) fails the test, naming the offending line. This is the
"shape, not the eight sites" guard the ticket asked for.

Testing

Full suite run directly (no pipe into tail/head), exit code checked
explicitly rather than trusting grep -c '^FAIL:' alone (a dead/aborted
suite reports 0 the same as a clean one — the #550 bug in this same file):

• bash scripts/test-redeploy-fleetd.sh (bash 5.3.9): exit 0, 0 lines
  matching ^FAIL:, reached the final PASS: redeploy log classifier line.
• /bin/bash scripts/test-redeploy-fleetd.sh (macOS bash 3.2.57): exit 0,
  0 lines matching ^FAIL:, reached the same PASS line.
• bash -n and /bin/bash -n both pass on both changed files.

Each of the 8 lifted decisions was proven with a mutation test: a
line-anchored sed mutation of the pristine source line (never
regex/perl), exact pristine-line-count via awk before and after, observed
RED with the exact new-test failure message, restored the file and verified
byte-identical via shasum -a 256 against the pre-mutation copy, then
re-ran the suite green as a control. The same procedure proved the
structural guard itself: a deliberate unguarded if was inserted into the
main flow, the guard test went RED naming that exact line, the line was
removed, and the suite went green again with a shasum-verified restore.

Out of scope

#504 items 2/3/4 and #528 item 2 are the same "untested main flow"
family and were not touched — this seam generalizes to make them
testable too, but lifting them is left for their own tickets, per the
brief.

Closes #555. ## What changed `scripts/redeploy-fleetd.sh`'s main flow had 8 bare `if`/`case` decisions living outside any function, so `scripts/test-redeploy-fleetd.sh` (67 tests) could not reach them — any one could be silently inverted with the whole suite green. This follows the existing `swap_if_built`/`refuse_drain_gate` pattern: each bare guard becomes a small predicate or dispatch function, called unconditionally by the main flow, so the decision is unit-testable. The 8 lifted decisions (line numbers as measured on this branch, before the refactor moved things around): 1. `if [ "$CHECK_ONLY" = 1 ]` → `should_stop_for_check` / `stop_if_check_only` 2. drain-gate entry `if [ -n "$OLD_PID" ] && [ "$ASSUME_YES" = 0 ]` → `drain_gate_required` 3. drain-gate confirm `if [ "$reply" != "yes" ]` → `drain_confirmed` 4. supervisor report `case "$SUPERVISOR_KIND"` → `report_supervisor_state` 5. supervisor stop `case "$SUPERVISOR_KIND"` → `dispatch_stop` 6. supervisor start `case "$SUPERVISOR_KIND"` → `dispatch_start` 7. health poll + `if [ -z "$HEALTH_BODY" ]` → `poll_health_body` / `health_is_up` / `report_health` 8. `HAD_OLD_PID=0; [ -n "$OLD_PID" ] && HAD_OLD_PID=1` → `compute_had_old_pid` ## The structural guard Adds `test_no_untested_main_flow_conditionals`, which scans the main flow (everything after the `SOURCED` guard) for bare `if`/`elif`/`case` lines outside any function body — function boundaries are tracked via this file's one consistent `name() {` / `}` convention. Any bare conditional not in the explicit `MAIN_FLOW_ALLOWED_CONDITIONALS` allowlist (report-only/display conditionals, plus the two `#504`-family supervisor `elif` branches that are out of scope here) fails the test, naming the offending line. This is the "shape, not the eight sites" guard the ticket asked for. ## Testing Full suite run directly (no pipe into `tail`/`head`), exit code checked explicitly rather than trusting `grep -c '^FAIL:'` alone (a dead/aborted suite reports 0 the same as a clean one — the #550 bug in this same file): - `bash scripts/test-redeploy-fleetd.sh` (bash 5.3.9): exit 0, 0 lines matching `^FAIL:`, reached the final `PASS: redeploy log classifier` line. - `/bin/bash scripts/test-redeploy-fleetd.sh` (macOS bash 3.2.57): exit 0, 0 lines matching `^FAIL:`, reached the same PASS line. - `bash -n` and `/bin/bash -n` both pass on both changed files. Each of the 8 lifted decisions was proven with a mutation test: a line-anchored `sed` mutation of the pristine source line (never regex/perl), exact pristine-line-count via `awk` before and after, observed RED with the exact new-test failure message, restored the file and verified byte-identical via `shasum -a 256` against the pre-mutation copy, then re-ran the suite green as a control. The same procedure proved the structural guard itself: a deliberate unguarded `if` was inserted into the main flow, the guard test went RED naming that exact line, the line was removed, and the suite went green again with a shasum-verified restore. ## Out of scope `#504` items 2/3/4 and `#528` item 2 are the same "untested main flow" family and were **not** touched — this seam generalizes to make them testable too, but lifting them is left for their own tickets, per the brief.

agent added 1 commit 2026-09-12 11:47:28 +02:00

fleetd #555: lift 8 main-flow decisions into tested predicate/dispatch functions ... 8d79d229ff

redeploy-fleetd.sh's main flow had 8 bare if/case decisions (CHECK_ONLY
short-circuit, drain-gate entry+confirm, supervisor report/stop/start
dispatch, health-poll decision, HAD_OLD_PID computation) that lived outside
any function, so the 67-test suite could not reach them and any one could be
silently inverted with the whole suite green.

Follows the existing swap_if_built/refuse_drain_gate pattern: each bare
guard becomes a small predicate or dispatch function (should_stop_for_check,
drain_gate_required/drain_confirmed/run_drain_gate, report_supervisor_state,
dispatch_stop, dispatch_start, health_is_up/report_health,
compute_had_old_pid), called unconditionally by the main flow so the
decision itself is unit-testable in isolation.

Adds a structural guard, test_no_untested_main_flow_conditionals, that scans
the main flow (everything after the SOURCED guard) for bare if/elif/case
lines outside any function body, tracking function boundaries via this
file's one consistent name() { / } convention. It fails on any new bare
conditional not covered by MAIN_FLOW_ALLOWED_CONDITIONALS, an explicit
exact-text allowlist of the report-only/display conditionals and the two
#504-family supervisor elif branches that stay out of scope for this
ticket. This is the "shape, not the eight sites" guard the ticket asked
for: a ninth bare decision fails immediately, naming its line.

Out of scope, not touched: #504 items 2/3/4 and #528 item 2 (same
untested-main-flow family) — the seam here generalizes to make them
testable too, but lifting them was left for their own tickets.

ltms referenced this pull request 2026-09-12 12:07:51 +02:00

redeploy-fleetd.sh: the 67-test suite covers the functions and barely touches the main flow #555

agent added 1 commit 2026-09-12 12:11:19 +02:00

fleetd #555 rework: catch function definitions after the SOURCED guard ... 8f80d267a0

Comment 17012 on #555 found a hole in test_no_untested_main_flow_conditionals:
the guard's function-body detection treats anything inside a function as
"fine, out of scope for this scan" — but a function DEFINED after the
SOURCED guard line can never be reached by sourcing this script (sourcing
stops before the main flow runs), so its body is untestable by construction
while still reading to the guard as safely inside a function.

mainflow_bare_conditionals now also emits a FUNC record for every function
opened after the guard line (reusing the same open-brace detection already
used for depth tracking), and test_no_untested_main_flow_conditionals treats
any such record as a violation on its own, independent of what the function's
body contains or whether the allowlist would otherwise excuse a bare
conditional inside it.

Proof (redeploy-fleetd.sh restored to 4ffacc5185807d39720a3484d85b922413806eb5347318265bd8897dfd61e8d9
after each):

- CONTROL — a bare conditional appended to the main flow is still caught:
  EXIT=1, "found 1 untested main-flow if/elif/case line(s) ... line 1341:
  if [ "$MY_CONTROL_BARE" = 1 ]; then :; fi"
- CANDIDATE — the same conditional wrapped in a function defined after the
  boundary, previously invisible (EXIT=0), is now caught: EXIT=1, "line 1341:
  function defined after the SOURCED guard (line 1038) — it cannot be
  sourced, so it cannot be tested: newfunc_below_the_boundary() {"

Full suite re-run green on both bash 5.3.9 and /bin/bash 3.2.57 (macOS
system bash): exit 0, 0 FAIL lines, reached the final PASS line, on both.

No change to redeploy-fleetd.sh; the 8 lifted decisions, their mutation
proofs, and the allowlist all stand as before.

ltms merged commit ba2f4d16f8 into main 2026-09-12 12:19:48 +02:00

ltms referenced this pull request 2026-09-12 12:21:04 +02:00

redeploy-fleetd.sh: the 67-test suite covers the functions and barely touches the main flow #555

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

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: fleet/fleetd#565