test(assess): guard every early-exit path reaches the drift/GPU advisories (backend#2674)

[aptracebloc]

Why

scripts/lib/assess.sh short-circuits an already-set-up machine before the normal flow reaches _handle_existing_cluster, where _check_existing_cluster_k8s_version (k3s drift, #547/#565) and _check_healthy_cluster_gpu_consistent (GPU, client#835/#852) run. So every early-exit terminal has to run both advisories itself — and we kept re-learning that one instance at a time: the k3s check, then the GPU check, then the cli-behind-latest → upgrade_cli_only path (#864, backend#2253), each patched only after the omission was spotted. @LukasWodka flagged this as the third instance of one shape — "a correct fix that lands above a guard added to close a previous early-exit gap" — and suggested a check that enumerates the early-exit paths and asserts each reaches the warnings, rather than fixing them one at a time.

What

New suite scripts/tests/assess-early-exit-drift.bats (auto-run by bats scripts/tests/*.bats in installer-tests):

• Behavioral — drives the healthy hand-off and upgrade_cli_only, asserts both advisories run (and, for the hand-off, before it — a warning after the home screen takes the terminal is one nobody sees).
• Static enumeration (the class-catcher, fails closed) — pins the exit-bearing functions in assess.sh (_assess_handoff) and install-cli.sh (upgrade_cli_only), pins _assess_handoff to its single call site, and asserts each early-exit decision calls both advisories at the source level. A new, uncovered early-exit (a fresh *_only that exits, or a second _assess_handoff call) trips it with a message to cover the path + update the pin.
• A fixture proving the enumeration actually detects an unguarded early-exit (not a vacuous pass).

Verification

All 7 tests pass. Mutation-verified against the real source: dropping either advisory from upgrade_cli_only fails the behavioral + static tests; adding a new exit-bearing function fails the pin. Recognises both name() { and function name { definition styles so a new terminal can't slip attribution.

Scope note: the enumeration covers assess.sh + install-cli.sh (where the two terminal functions live). A new early-exit added inline in install-k8s.shmain() before _handle_existing_cluster isn't scanned — new terminals are normally added as lib functions (which are covered), and main() has a legitimate normal-flow exit that would complicate scanning it. Happy to extend if you'd prefer.

Closes tracebloc/backend#2674
Follow-up to backend#2253.

— drafted with Claude Code

---

Note

Low Risk
Test-only change with no installer runtime behavior modified; risk is limited to CI maintenance if early-exit structure changes.

Overview
Adds scripts/tests/assess-early-exit-drift.bats, a Bats suite that stops repeat regressions where the assess installer exits early (healthy hand-off via assess_existing_install / _assess_handoff, or upgrade_cli_only) without running the k3s drift and GPU consistency advisories that normally live on _handle_existing_cluster.

Behavioral tests stub the advisory hooks and assert both run on each known terminal path; the healthy path also checks advisories run before the hand-off.

Static “fail closed” guards enumerate exit-bearing functions in assess.sh and install-cli.sh, pin a single _assess_handoff call site, and derive parity of declare -F _check_* advisory guards between early-exit functions so new terminals or dropped checks break CI. A fixture test validates the awk helpers against varied exit spellings and comment/quote edge cases.

^{Reviewed by Cursor Bugbot for commit 07ad6c2. Bugbot is set up for automated code reviews on this repo. Configure here.}

[aptracebloc]

bugbot run

[LukasWodka]

Approving b0d05757 — 37 passing, 4 path-skipped, nothing pending or failing, no open threads, mergeable=MERGEABLE.

I suggested this guard, so I held it to the bar I'd hold any guard to rather than waving through my own idea. It passes, and I proved it against the real production files rather than reading it.

It catches a new early-exit path. I appended an unguarded terminal to the real scripts/lib/assess.sh:

_mut_new_terminal() { info "...";exit 0; }

not ok 3 assess.sh's ONLY exit-bearing function is _assess_handoff

That is the class closing. The next person who adds a terminal to assess.sh cannot land it without either wiring the advisories or consciously updating the pin — which is the whole point, since the last three instances were each patched only after someone noticed.

And it catches an existing path losing an advisory. Removing the GPU call from upgrade_cli_only in the real install-cli.sh:

not ok 2 upgrade_cli_only runs BOTH drift/GPU advisories ← behavioural
not ok 6 every early-exit decision calls BOTH advisories ← source-level

Two independent tests, one driving the function and one reading the source. That redundancy is worth keeping: the behavioural test would survive a refactor that moves the call, the static one would survive a stub that fakes it, and neither alone covers both.

The fixture is what makes the rest trustworthy._funcs_with_exit and _funcs_calling are the real helpers the other tests use, exercised against a synthetic script containing one guarded and one deliberately unguarded terminal, asserting both directions — the enumeration finds sneaky_new_only, and the per-symbol check correctly reports it as not calling the advisory. The inputs are written down independently of the matcher, so it isn't a list checking itself. Without that test, tests 3/4/6 would look identical whether the awk helpers worked or matched nothing at all.

Two smaller things done right: the ordering assertion on the healthy path (an advisory printed after the hand-off takes the terminal is one nobody reads), and ADVISORIES naming only what to grep for while the where is derived by parsing.

On Bugbot's note that the risk is "CI maintenance if exit-bearing functions are added without updating pins" — that is the feature, not the cost. A pin that never needs updating is a pin that isn't watching anything. The failure mode it creates is a red CI with an obvious fix; the failure mode it removes is a silent gap that took three separate incidents to notice.

Value: converts a bug we kept re-learning one instance at a time into one that fails CI the first time. #547/#565 (k3s drift), client#835/#852 (GPU), and #864 (cli-behind-latest) were the same shape three times over — this is the check that would have caught all three.

[aptracebloc]

bugbot run

[aptracebloc]

bugbot run

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 07ad6c2. Configure here.}

[LukasWodka]

Approving 07ad6c25 — 35 checks pass, zero unresolved threads.

My earlier approval was dismissed by a push, so I re-ran the mutation proof against the new head rather than re-stamping it. The push closed a real hole, and it was a hole in the version I had already approved.

The proof

Clean run at 07ad6c25: 7/7. Three mutations against the real libs, each with the anchor verified applied before reading the result:

Mutation	Expected	Result
Plant [ "$rc" = 0 ] && exit 0 in a new fn in assess.sh	test 3 reddens	✅ not ok 3
Delete the GPU advisory from upgrade_cli_only	tests 2 + 6 redden	✅ not ok 2, not ok 6
Add _check_new_advisory to the healthy hand-off only	test 6 reddens	✅ not ok 6

The third is the axis _advisories_in was written for, and it works: a new advisory added to one early-exit and not the other now fails parity. That's the class-catcher the hardcoded ADVISORIES=() pair could never have been.

Why the dismissal was right

I ran the same compound-exit mutation against b0d05757 — the commit I approved:

ok 1 … ok 2 … ok 3 … ok 4 … ok 5 … ok 6 … ok 7 ← 7/7 GREEN, with an unguarded early-exit planted

The old _funcs_with_exit matched ^[[:space:]]*exit, so && exit, ; exit and then exit were invisible to it. A new unguarded terminal written in any of those spellings would have shipped past the guard silently — the precise failure the suite exists to prevent.

That is a miss in my own review, and worth naming. I did mutation-prove the earlier version, and it passed — because my mutation used a line-leading exit, the one spelling the old scanner handled. A mutation proof only covers the shapes you think to write; mine agreed with the scanner's blind spot instead of probing it. CLAUDE.md rule 6 says derive the input domain from the producer's declared surface rather than trusting mutation coverage — the "producer's surface" here is every way bash can spell a terminating exit, and I tested one of four. Bugbot caught what I didn't.

One non-blocking observation

_advisories_in keys on the declare -F _check_* idiom, so an advisory invoked without that guard is invisible to the derivation on both sides — parity would pass while covering less than it appears to. The [ -n "$ref" ] fail-closed catches total emptiness, not a partial drop. In practice the behavioral pair (tests 1–2) pins both known advisories by name and asserts they actually ran, so the two halves cover each other: behavioral catches a drop, derived catches an addition. Worth a comment noting the convention is load-bearing, but nothing to hold the PR for.

Good work — this is materially stronger than what I signed off on.