fix(plant): review-gate fixes — level-based UPS re-polls, test hardening [#790 #344]
ci / lint (push) Successful in 55s
ci / config-check (push) Failing after 1m5s

Adversarial review verdict FAIL -> addressed: runtime-low + shutdown
chain re-poll /5 min (edge-crossing could miss a page mid-outage);
suppression stamp set after notify; plan body names all 7 hosts
explicitly; test fixes (pipefail SIGPIPE, comment-only assertion).
Reviewer P1 'tripp_lite slug' refuted with live entity registry
evidence (entity_id derives from name; sensor exists, 8f48730 aligned).
On-box ha core check passed with the changed files.

Review thread: #3
This commit is contained in:
2026-09-06 06:49:26 -05:00
parent b06ef7dbf0
commit d18e467484
3 changed files with 70 additions and 21 deletions
+16 -5
View File
@@ -79,14 +79,25 @@
- sensor.pfv_ups_runtime_minutes
- sensor.pfv_tripp_lite_runtime_minutes
below: 10
# 5-min level re-check: numeric_state alone fires only on the threshold
# CROSSING — a blip at that minute (or an HA restart mid-outage) would
# silence the page for the whole discharge. Level-based polling turns
# this into an escalation ladder while the outage persists.
- trigger: time_pattern
minutes: /5
conditions:
- condition: template
value_template: >-
{% set status_map = {
'sensor.pfv_ups_runtime_minutes': 'sensor.tsys1_ups_status',
'sensor.pfv_tripp_lite_runtime_minutes': 'sensor.tsys1_triplite_status'} %}
{{ states(status_map.get(trigger.entity_id, 'sensor.tsys1_ups_status'))
not in ['Online', 'OL', 'unknown', 'unavailable'] }}
{% set ns = namespace(hit=false) %}
{% for rt, st in [
('sensor.pfv_ups_runtime_minutes', 'sensor.tsys1_ups_status'),
('sensor.pfv_tripp_lite_runtime_minutes', 'sensor.tsys1_triplite_status')] %}
{% if is_number(states(rt)) and (states(rt) | float) < 10
and states(st) not in ['Online', 'OL', 'unknown', 'unavailable'] %}
{% set ns.hit = true %}
{% endif %}
{% endfor %}
{{ ns.hit }}
actions:
- action: notify.send_message
target:
+38 -13
View File
@@ -20,6 +20,10 @@
# ============================================================================
input_boolean:
# NOTE: `initial:` wins over restored state at EVERY HA start — a deploy
# mid-test silently reverts these to the safe defaults (armed OFF,
# dry-run ON). Safe direction, but re-arm after any restart during the
# supervised Sept 11-12 live test.
pfv_ups_shutdown_armed:
name: UPS shutdown chain ARMED
description: Founder-level arm. ON + dry-run OFF = real shutdown dispatch.
@@ -61,26 +65,39 @@ automation:
- sensor.tsys1_ups_battery_charge
- sensor.tsys1_triplite_battery_charge
below: 50
# 5-min level re-check — same edge-vs-level reasoning as the
# runtime-low alert: never let a threshold-crossing blip silence
# the plan page for an entire discharge.
- trigger: time_pattern
minutes: /5
conditions:
- condition: template
value_template: >-
{% set sm = {
'sensor.pfv_ups_runtime_minutes': 'sensor.tsys1_ups_status',
'sensor.pfv_tripp_lite_runtime_minutes': 'sensor.tsys1_triplite_status',
'sensor.tsys1_ups_battery_charge': 'sensor.tsys1_ups_status',
'sensor.tsys1_triplite_battery_charge': 'sensor.tsys1_triplite_status'} %}
{{ states(sm.get(trigger.entity_id, 'sensor.tsys1_ups_status'))
not in ['Online', 'OL', 'unknown', 'unavailable'] }}
{% set ns = namespace(hit=false) %}
{% for rt, st in [
('sensor.pfv_ups_runtime_minutes', 'sensor.tsys1_ups_status'),
('sensor.pfv_tripp_lite_runtime_minutes', 'sensor.tsys1_triplite_status')] %}
{% if is_number(states(rt)) and (states(rt) | float) < 20
and states(st) not in ['Online', 'OL', 'unknown', 'unavailable'] %}
{% set ns.hit = true %}
{% endif %}
{% endfor %}
{% for ch, st in [
('sensor.tsys1_ups_battery_charge', 'sensor.tsys1_ups_status'),
('sensor.tsys1_triplite_battery_charge', 'sensor.tsys1_triplite_status')] %}
{% if is_number(states(ch)) and (states(ch) | float) < 50
and states(st) not in ['Online', 'OL', 'unknown', 'unavailable'] %}
{% set ns.hit = true %}
{% endif %}
{% endfor %}
{{ ns.hit }}
# Status 'unavailable' suppresses (anti-noise); UPS COMMS LOST covers
# that failure mode separately after 5 min.
- condition: template
value_template: >-
{{ states('input_datetime.pfv_ups_shutdown_last_page') in ['unknown', 'unavailable', 'none']
or as_timestamp(now()) - as_timestamp(states('input_datetime.pfv_ups_shutdown_last_page')) > 21600 }}
actions:
- action: input_datetime.set_datetime
target:
entity_id: input_datetime.pfv_ups_shutdown_last_page
data:
datetime: "{{ now().strftime('%Y-%m-%d %H:%M:%S') }}"
- variables:
execute_mode: >-
{{ is_state('input_boolean.pfv_ups_shutdown_armed', 'on')
@@ -97,7 +114,8 @@ automation:
(ca LAST — signing path for everything above)
5. pfv-bms VM 100 LAST of the VMs — the brain stays up until now
6. Hypervisors: iDRAC graceful off tsys6/tsys7 (PowerEdge);
tsys1/3/4/5/9 idle out on UPS exhaustion (OptiPlex/Precision)
tsys1, tsys3, tsys4, tsys5, tsys9 idle out on UPS exhaustion
(OptiPlex/Precision, no BMC)
mode_note: >-
{{ 'MODE: EXECUTE (armed, live dispatch)' if execute_mode
else 'MODE: DRY RUN (plan only — nothing will be shut down)' }}
@@ -122,3 +140,10 @@ automation:
data:
title: "{{ 'UPS SHUTDOWN EXECUTING' if execute_mode else 'UPS SHUTDOWN PLAN (dry run)' }}"
message: "{{ plan }}"
# Suppression stamp set LAST — a failed notify must not suppress the
# next 5-min re-page cycle.
- action: input_datetime.set_datetime
target:
entity_id: input_datetime.pfv_ups_shutdown_last_page
data:
datetime: "{{ now().strftime('%Y-%m-%d %H:%M:%S') }}"
+16 -3
View File
@@ -63,17 +63,30 @@ t "trend sensors fed from smoothed sources" \
t "runtime-low covers Tripp Lite" \
"sed -n '/id: pfv_plant_ups_runtime_low/,/^ mode:/p' automations.yaml | grep -q 'sensor.pfv_tripp_lite_runtime_minutes'"
t "runtime-low status check maps per-UPS" \
"sed -n '/id: pfv_plant_ups_runtime_low/,/^ mode:/p' automations.yaml | grep -q 'status_map'"
"sed -n '/id: pfv_plant_ups_runtime_low/,/^ mode:/p' automations.yaml | grep -q \"sensor.pfv_tripp_lite_runtime_minutes', 'sensor.tsys1_triplite_status'\""
echo "=== UPS shutdown chain (#790 — dry-run is the shipped mode) ==="
t "shutdown chain package present, dry-run default ON" \
"grep -q 'id: pfv_ups_shutdown_plan' packages/ups_shutdown.yaml && grep -A4 'pfv_ups_shutdown_dry_run:' packages/ups_shutdown.yaml | grep -q 'initial: true'"
t "armed switch default OFF" \
"grep -A4 'pfv_ups_shutdown_armed:' packages/ups_shutdown.yaml | grep -q 'initial: false'"
t "plan covers all seven hypervisors" \
"for h in tsys1 tsys3 tsys4 tsys5 tsys6 tsys7 tsys9; do grep -q \$h packages/ups_shutdown.yaml || exit 1; done"
# Host coverage must hold in the PLAN BODY, not the header comment —
# scope the grep to the plan template variable.
PLAN_OK=1
for h in tsys1 tsys3 tsys4 tsys5 tsys6 tsys7 tsys9; do
# full-read grep (not -q): the script runs with pipefail — -q's early
# exit SIGPIPEs sed and flips the pipeline status to 141
sed -n '/plan: >-/,/mode_note:/p' packages/ups_shutdown.yaml | grep "$h" >/dev/null || PLAN_OK=0
done
t "plan covers all seven hypervisors (plan body)" "[ \$PLAN_OK -eq 1 ]"
t "pfv-bms VM 100 ordered last of the VMs (#455)" \
"grep -q 'pfv-bms VM 100 LAST' packages/ups_shutdown.yaml"
echo "=== level-based re-checks (review finding: edge-only triggers miss pages) ==="
t "runtime-low re-polls every 5 min" \
"sed -n '/id: pfv_plant_ups_runtime_low/,/^ mode:/p' automations.yaml | grep -q 'minutes: /5'"
t "shutdown chain re-polls every 5 min" \
"grep -A30 'id: pfv_ups_shutdown_plan' packages/ups_shutdown.yaml | grep -q 'minutes: /5'"
echo "=== SDLC SUITE: $FAIL failures ==="
exit "$FAIL"