fix(installer): make the absent-key path in _extract_yaml_value reachable

[LukasWodka]

Summary

_extract_yaml_value (install-client-helm.sh:155) piped into grep. On an absent key grep exits 1; under set -o pipefail that rc propagates out of the pipeline and out of the assignment, so under set -e the function aborts at the assignment — which makes the very next line unreachable in exactly the shape it exists to handle:

line=$(grep -E "^${key}:""$file"2>/dev/null | head -1)# exits 1 when the key is absent
[[ -z"$line" ]] &&return# the "key not found" line — never reached

Latent, not live. All three call sites (lines 209, 651, 652) use the $( ) command-substitution form, which suspends errexit for the function body. But the function's contract is "empty when the key is absent", so a bare call is the natural next refactor — and it would abort the install mid-step.

Fix

|| line="" on the assignment — the house idiom already used at assess.sh:53 and common.sh:308-311. Catching any non-zero also keeps the absent-key path reachable if head -1 ever SIGPIPEs grep (141) — the sibling shape fixed in #522. head -1 kept; this PR is only about the grep-rc=1 abort.

The contract is now written down above the function, so the next reader knows "empty" is the intended answer for both absent-key and unreadable-file, and doesn't have to re-derive it from the callers.

Behaviour change is confined to the absent-key/read-error path. Found-key parsing, quote handling, and _strip_paste_garbage are untouched.

Test plan

New bats case pins the bare-statement call under set -euo pipefail — per #523, asserting the $( ) sites still work proves nothing, since they already did.

Mutation-tested: with || line="" removed, the new case fails (status 1, sentinel never printed); with the fix it passes. So it cannot rot into a no-op.

Verified on bash 3.2.57 / GNU grep:

shape	before	after
bare call, absent key, errexit live	exit 1 — aborts, return never runs	exit 0, empty, execution continues
v="$(_extract_yaml_value …)", absent key	exit 0, ""	unchanged
absent file, bare call	exit 1	exit 0, empty
found key (clientId: "abc-123")	abc-123	unchanged

Gates run locally, all green:

• bats scripts/tests/*.bats → 670/670 ok, 0 failures
• shellcheck --severity=warning --shell=bash scripts/lib/install-client-helm.sh → clean, rc=0 (no hits at this severity)
• bash -n scripts/lib/install-client-helm.sh → parses
• bash scripts/check-style.sh → ok: style + terminology clean
• scripts/gen-manifest.sh re-run + scripts/manifest.sha256 committed (R8 "Static analysis" gate). Exactly one hash line changed — install-client-helm.sh, the only edited installer script. Not hand-edited; re-running is idempotent.

Context

Found while fixing the two Bugbot SIGPIPE findings in #522, deliberately left out of that PR as out of scope, and documented in its "Also spotted" section.

Fixes#523

🤖 Generated with Claude Code

---

Note

Low Risk
Low risk: localized YAML helper hardening in the installer with regression tests; behavior for found keys is unchanged and the fix closes fail-open paths in client detection rather than altering runtime cluster logic.

Overview
_extract_yaml_value in install-client-helm.sh now uses || line="" on the grep assignment (same idiom as assess.sh / common.sh) so a missing YAML key does not abort the installer under set -e and pipefail when the helper is called as a bare statement—not only inside $( ).

The grep | head -1 pipeline is removed: duplicate keys are read with grep alone, then the first line is taken via ${line%%$'\n'*}, avoiding SIGPIPE/pipefail wiping a captured value and weakening detect_installed_client.

The function’s “empty means no value” contract is documented in comments. scripts/manifest.sha256 is updated for the lib change. Bats adds cases for absent-key under set -euo pipefail and duplicate-key first-value behavior.

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

[LukasWodka]

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 ce96a21. Configure here.}

[LukasWodka]

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 b5b77e3. Configure here.}

[aptracebloc]

@LukasWodka — Approved. Clean minimal fix for the pipefail/set -e abort class. I ran it and confirmed all four quadrants:

• bare call + fixed → reaches the not-found path ✓
• bare call + reverted || line="" → aborts before the empty-check (the bug) ✓
• $()-wrapped + reverted → does NOT abort (rc 0, empty value) — so your "latent" framing is empirically correct ✓
• Verified all three call sites (lines 222, 664, 665) are $()-wrapped, so no current caller hits it — this is defensive hardening + making the documented not-found contract hold for any caller, not a live-abort fix.

Other checks: manifest hash matches (R8 intact), parses, and the single || line="" correctly covers both the grep-exit-1 (absent key) and the head SIGPIPE-141 (#522 sibling) shapes.

The standout is the test: it exercises the bare call under set -euo pipefail (the only shape that exhibits the bug) and explicitly notes that asserting the $()-wrapped call would prove nothing — which my own replication confirms. Reaching the sentinel is the proof. Uses the house || var="" idiom (same as assess.sh / _chart_version), manifest kept in lockstep.

No blocking issues — latent today, so no urgency, but correct and worth landing to close the recurring footgun before a future bare caller trips it. Touches your code-owned install-client-helm.sh + manifest.sha256 (R8), so Asad's code-owner review is the backstop; this is the second set of eyes.

🤖 Generated with Claude Code

[LukasWodka]

Resolved the develop conflict: the only collision was scripts/manifest.sha256 (#591's install-k8s.ps1 hash landed on develop after this branch's line for install-client-helm.sh). Resolved by regenerating with scripts/gen-manifest.sh from the merged tree — every line re-verified against the actual file hashes. install-client-helm.bats passes (80/80, 0 failures).

Reviewed the substance while here: the || line="" guard is the right shape (house idiom from _chart_version), and the new bare-call test under set -euo pipefail pins the previously-unreachable path — and its assertions are already || return 1-hardened, so it also passes #527's incoming bats-hygiene gate as-is.

Note: the conflict-resolution push dismissed the earlier approval (stale-review dismissal) — needs a re-approve.

bugbot run

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 9d0a40f. Configure here.}

[LukasWodka]

Fixed the Bugbot finding: dropped head from the pipeline entirely — grep captures every match and the shell takes the first line (${line%%$'\n'*}), so a duplicate key can no longer SIGPIPE grep and have || line="" wipe a successfully captured value. grep's rc is now 1 exactly when there's no match — the one case the fallback exists for. Regression test pins the duplicate-key bare-call shape under set -euo pipefail; manifest regenerated; 81/81 bats green.

bugbot run

[LukasWodka]

Merged develop (now containing #592). Zero conflicts this time — #592 and this PR change different manifest lines, and every merged manifest line re-verified against the actual file hashes. 81/81 bats green.

bugbot run

[shujaatTracebloc]

Reviewed. Correct fix: line=$(grep …) || line="" handles the errexit abort on an absent key (#523), and dropping | head -1 for line="${line%%$'\n'*}" fixes the SIGPIPE-wipes-value bug on a duplicate key (fail-open toward overwrite). Tests exercise the real bare-call set -euo pipefail shape and enforce with || return 1. CI green, Bugbot clean, 0 unresolved. LGTM.