fix(tests): the default comment opener never reached the stripper (client#788)

[LukasWodka]

Closes#788.

The bug

scripts/tests/k3s-components-agreement.bats, write_ds:

local opener="${1:-\{\{/\*}" mode="${2:-code}"

Bash keeps those backslashes inside double quotes, so the fixture's first line was
never {{/*:

$ bash -c 'set --; opener="${1:-\{\{/\*}"; printf "%s\n" "$opener" | od -c | head -2'
0000000 \ { \ { / \ * \n
0000010

Seven bytes, and {{ never appears adjacently, so the guard's opener regex
/\{\{-?[[:space:]]*\/\*/ cannot match it. Every default-opener case ran against a
daemonset fixture that nothing was stripped from.

The unreachable fixture came with a redundant mechanism sitting behind it, which is
why nobody noticed: the poison prose inside the comment block says
v1beta1.metrics.k8s.ioandfail , so with the block left in ds_body it
satisfied check 3 by itself. The happy path exited 0 whether or not the lookup and
the fail it merely describes were there. That is exactly the
unreachable-fixture + redundant-mechanism pair .cursor/BUGBOT.md already names for
this suite.

The fix — both halves

1. Emit the opener literally. Assigned outside the double-quoted expansion, so no
backslash can creep back in:

local opener="${1-}" mode="${2:-code}"
[ -n"$opener" ] || opener='{{/*'

2. Make an assertion depend on the strip. This is the half that matters — fixing
only the quoting leaves a test that still cannot fail.

Why no exit-0 case can carry this: stripping only ever removes lines, and every
check-3 finding fires on an absence. So an under-stripping stripper can only turn
a finding into a pass; the green half of any pair is blind to it, however loudly it is
written. Only a case that expects exit 1 can see the strip stop happening.

So the default opener is now exercised as a pair:

• the existing happy path stays the green half — it reddens if the stripper
  over-strips and eats the coupling below the block (client#764);
• catches the chart no longer looking up the APIService drops its explicit {{/*
  and takes the default, making it the red half — with the coupling deleted, the
  guard must still refuse, which it can only do if the poison prose was stripped.

That test also now asserts which refusal it got. Exit 1 is equally how a
disagreeing pair of installers reports, so a bare [ "$status" -eq 1 ] is a coin toss
about what was exercised (backend#1729 rule 10). The same assertion is added to its
{{- /* sibling, which had the identical gap.

Plus a direct assertion on the fixture's own quoting, per the ticket's DoD. The
expected pattern is written out independently of write_ds, so a re-escaped opener
cannot agree with itself (#1729 rule 9).

Only scripts/tests/k3s-components-agreement.bats changes. The guard itself is
untouched.

Mutation proof

Two mutations, both on the real stripper in scripts/tests/k3s-components-agreement.sh.
Each is run against develop's suite and against this PR's suite, so the log
shows the gap and not just the coverage.

Mutation 1 — narrow the opener's -? to -, so the plain {{/* no longer opens a block

Anchor, proven applied:

### stripper opener BEFORE:
163: /\{\{-?[[:space:]]*\/\*/ { inc = 1 }
### stripper opener AFTER (mutated):
163: /\{\{-[[:space:]]*\/\*/ { inc = 1 }

develop's default-opener case — survives:

### develop's suite, MUTATED stripper — the default-opener case:
1..1
ok 1 agreeing installers with the coupling intact exit 0

This PR's default-opener cases — reddens:

### this PR's suite, MUTATED stripper — the default-opener cases:
1..2
ok 1 the default opener reaches the stripper: a literal {{/* line, no backslashes
not ok 2 catches the chart no longer looking up the APIService (default opener {{/*)
# (in test file k3s-components-agreement.bats, line 144)
# `[ "$status" -eq 1 ] || { echo "FAIL-OPEN: the prose satisfied check 3"; echo "$output"; return 1; }' failed
# FAIL-OPEN: the prose satisfied check 3
# k3s components disabled at cluster-create time, as declared by each installer:
# scripts/lib/cluster.sh local-storage servicelb traefik
# scripts/install-k8s.ps1 local-storage servicelb traefik
#
#
# k3s components: both installers disable the same set, neither disables metrics-server,
# and the chart coupling that makes metrics-server load-bearing is still in place.

Mutation 2 — disable the strip outright

Anchor, proven applied:

### M2 anchor (strip disabled entirely):
ds_body="$(awk '
/ZZZ_NEVER_MATCHES_ZZZ/ { inc = 1 }
inc { if (/\*\/[[:space:]]*-?\}\}/) inc = 0; next }
/^[[:space:]]*#/ { next }
{ print }
' "$DAEMONSET")"

develop's default-opener case — survives:

### develop's suite, strip DISABLED — default-opener case:
1..1
ok 1 agreeing installers with the coupling intact exit 0

This PR's default-opener cases — reddens:

### this PR's suite, strip DISABLED — default-opener cases:
1..2
ok 1 the default opener reaches the stripper: a literal {{/* line, no backslashes
not ok 2 catches the chart no longer looking up the APIService (default opener {{/*)
# (in test file k3s-components-agreement.bats, line 144)
# `[ "$status" -eq 1 ] || { echo "FAIL-OPEN: the prose satisfied check 3"; echo "$output"; return 1; }' failed
# FAIL-OPEN: the prose satisfied check 3
# k3s components disabled at cluster-create time, as declared by each installer:
# scripts/lib/cluster.sh local-storage servicelb traefik
# scripts/install-k8s.ps1 local-storage servicelb traefik
#
#
# k3s components: both installers disable the same set, neither disables metrics-server,
# and the chart coupling that makes metrics-server load-bearing is still in place.

Note what mutation 1 does not show: develop's suite is not blind to it overall —
its catches the chart no longer looking up the APIService passed '{{/*' explicitly
and does redden. The gap is the default-opener path specifically, which is what
this ticket is about and what the log above isolates.

Restored — green

### stripper restored:
/\{\{-?[[:space:]]*\/\*/ { inc = 1 }
### this PR's suite, RESTORED stripper:
1..12
ok 1 agreeing installers with the coupling intact exit 0
ok 2 catches a component disabled in bash but not in ps1
ok 3 catches a component disabled in ps1 but not in bash
ok 4 catches metrics-server disabled in EITHER installer (the load-bearing one)
ok 5 the default opener reaches the stripper: a literal {{/* line, no backslashes
ok 6 catches the chart no longer looking up the APIService (default opener {{/*)
ok 7 and still catches it when the header uses the chomping opener {{- /*
ok 8 the chomping opener does not break the CLEAN case either
ok 9 an unreadable installer is 'cannot tell' (exit 2), never green
ok 10 an unreadable daemonset is 'cannot tell' (exit 2), never green
ok 11 code below a chomped block closing '*/ -}}' is still seen (not eaten)
ok 12 and its absence below that same block is still caught

Test plan

The guard itself, unchanged, still exits 0 against the real tree:

$ bash scripts/tests/k3s-components-agreement.sh; echo "guard exit=$?"
k3s components disabled at cluster-create time, as declared by each installer:
scripts/lib/cluster.sh local-storage servicelb traefik
scripts/install-k8s.ps1 local-storage servicelb traefik
k3s components: both installers disable the same set, neither disables metrics-server,
and the chart coupling that makes metrics-server load-bearing is still in place.
guard exit=0

Full scripts/tests/*.bats, green — bats (bash unit, mocked), 3m52s, plan 1..1326
with no not ok line in the job log
(run).
The twelve cases of this suite in that run:

ok 760 agreeing installers with the coupling intact exit 0
ok 764 the default opener reaches the stripper: a literal {{/* line, no backslashes
ok 765 catches the chart no longer looking up the APIService (default opener {{/*)
ok 766 and still catches it when the header uses the chomping opener {{- /*
ok 767 the chomping opener does not break the CLEAN case either

(Started locally too, but this box was at load 18 with several agents' suites in flight;
it reached 632 of 1326 with 0 failures before I tore its worktree down, and the clean
runner had already finished by then. The CI run above is the full-suite evidence.)

bats-hygiene.bats is green too (18/18) — the new assertions all end in
|| { …; return 1; }, and unenforced-assertions.awk reports no offenders in this
file.

Why now

This unblocks #786: its Bugbot thread is the one #788 was filed from, and per
.cursor/BUGBOT.md an unresolved cursor thread is a soft gate that holds release-train
promotions for the whole fleet, not just that PR. With this merged the thread can be
answered with a fix rather than a note.

🤖 Generated with Claude Code

[saadqbal]

Approving — and this is my favourite kind of test fix, because the tests were green and proving nothing.

Confirmed the mechanism: "${1:-\{\{/\*}" is inside double quotes, and bash only strips a backslash there before $ \ " ` or newline — so \{ stays two characters and the default opener really was the 7-byte \{\{/\*. Not just unmatchable by the stripper: {{ never appears adjacently in it. So every default-opener case ran against an unstripped file, the poison prose survived into ds_body and satisfied check 3 by itself, and the lookup-and-fail it merely describes never had to exist for those cases to pass.

The fix is right and the quoting is right for the reason the comment gives — opener='{{/*' single-quoted, with ${1-} plus [ -n ] so an explicit '' also takes the default, which is what lets a caller vary only the mode. That matches the new signature comment.

The new fixture-quoting test is the one that would have caught this, and it's written the way it has to be: grep -qxF '{{/*' spelled out independently of write_ds, so a re-escaped opener can't agree with itself.

The pair argument is the part worth keeping in the file, and it's correct: stripping only ever removes lines and every check-3 finding fires on an absence, so an under-stripping stripper can only ever convert a finding into a pass — which the green half structurally cannot see. The red half is the only thing that reddens when the opener stops matching, and the green half is the only thing that catches over-stripping eating the code below. Neither is a test of the stripper alone, and saying so beside them is worth more than either assertion.

[saqlainsyed007]

Verified against the fixture behaviour, not just the description — clean fix.

The bug is real and the fix is correct.local opener="${1:-\{\{/\*}" keeps the backslashes inside double quotes, so the default fixture opened with a 7-byte \{\{/\* the stripper's regex can't match — every default-opener case ran against an unstripped file, and the redundant poison-prose path masked it. Rewriting as opener="${1-}"; [ -n "$opener" ] || opener='{{/*' (single-quoted literal) is right, and the comment correctly warns that a backslash here silently disarms the cases below.

The new guards are mutation-proof and non-vacuous, which is what makes this worth more than a one-liner:

• the default opener reaches the stripper greps -qxF '{{/*' on the fixture's first line, written independently of write_ds — so a re-escaped opener cannot agree with itself (rule 9). Re-introduce the backslashes and this reddens.
• The check-3 cases now assert the specific refusal string (no longer looks up v1beta1.metrics.k8s.io), not bare status -eq 1 — because a disagreeing installer pair also exits 1, so the old assertion could pass for the wrong reason (rule 10).
• The green/red pairing rationale is spelled out correctly: under-stripping only ever turns a finding into a pass, so each opener spelling needs both halves.

CI green, no threads, mergeable clean. LGTM.