fix(zulip-monitor): Abiba leg reads nested zulip.connected; probe failures never restart
The monitor's Abiba leg parsed the :9200/health payload at the top level
(d.get('connected',False)) while the pi Zulip extension serves connection
state NESTED at zulip.connected / zulip.last_error (verified against the
extension's startHealthServer handler and the live endpoint). PI_CONNECTED
was therefore always False and every run took the DISCONNECTED path,
restarting a healthy bot: pm2 restarts=8 with the process created
2026-09-09T09:35:09Z, four ❌ Abiba verdicts today (04:23/05:35/06:55/09:35
UTC) and zero ✅, while the Zulip server answered HTTP 200 and the bot kept
heartbeating. The watchdog was the fault, not the connection.
Changes (Abiba leg only; every other leg byte-identical):
- Read the real nested shape: zulip.connected, zulip.last_error and
zulip.messages_processed. The old retry_count branch is DROPPED — the
payload exposes no retry counter (the extension keeps retryCount internal
and never serialises it), so the branch is fabricated and cannot stay.
- Fail-safe restart decision: a fetch error (HTTP 000), non-2xx response,
empty/unparseable body, or payload missing a boolean zulip.connected is a
clearly-labelled PROBE FAILURE (🟠 alert + ⚠️ log line naming reason and
HTTP code) and NEVER calls pm2 restart. pm2 restart runs only on
affirmative zulip.connected=false (🔴/❌ path unchanged in wording).
connected=true with last_error keeps the degraded 🟡 warn-no-restart path.
Regression test (new tests/zulip-monitor-abiba.sh, 36 checks): extracts the
real Abiba leg from between the # -- abiba-leg-start/-end markers in the
shipped script and executes it verbatim with stubbed curl/notify/pm2 against
fixtures of the real payload shape — asserts connected=true → no restart,
connected=false → restart, and empty/garbage/missing-key/non-boolean/HTTP
000/HTTP 500 bodies → probe-failure alert with zero restarts. The suite
fails loudly on the pre-fix base and on a marker-intact top-level-parse
variant, so this class of bug cannot return silently.
This commit is contained in:
root
+25
@@ -0,0 +1,25 @@
|
||||
{
|
||||
"status": "ok",
|
||||
"platform": "pi",
|
||||
"agent": "abiba",
|
||||
"zulip": {
|
||||
"connected": true,
|
||||
"site": "https://chat.sysloggh.net",
|
||||
"email": "abiba-bot@chat.sysloggh.net",
|
||||
"queue_id": "ee7f8b6d-9d53-48a7-ad58-f6e999771001",
|
||||
"bot_user_id": 21,
|
||||
"messages_processed": 0,
|
||||
"skipped": 0,
|
||||
"last_error": null
|
||||
},
|
||||
"circuit_breaker": {
|
||||
"state": "CLOSED",
|
||||
"failures": 0,
|
||||
"successes": 5,
|
||||
"totalRequests": 5,
|
||||
"failureRate": "0.000",
|
||||
"openedAt": null
|
||||
},
|
||||
"workers": [],
|
||||
"worker_count": 0
|
||||
}
|
||||
+25
@@ -0,0 +1,25 @@
|
||||
{
|
||||
"status": "down",
|
||||
"platform": "pi",
|
||||
"agent": "abiba",
|
||||
"zulip": {
|
||||
"connected": false,
|
||||
"site": "https://chat.sysloggh.net",
|
||||
"email": "abiba-bot@chat.sysloggh.net",
|
||||
"queue_id": null,
|
||||
"bot_user_id": null,
|
||||
"messages_processed": 0,
|
||||
"skipped": 0,
|
||||
"last_error": "Zulip API error 401: queue registration failed"
|
||||
},
|
||||
"circuit_breaker": {
|
||||
"state": "CLOSED",
|
||||
"failures": 0,
|
||||
"successes": 0,
|
||||
"totalRequests": 0,
|
||||
"failureRate": "0.000",
|
||||
"openedAt": null
|
||||
},
|
||||
"workers": [],
|
||||
"worker_count": 0
|
||||
}
|
||||
Executable
+206
@@ -0,0 +1,206 @@
|
||||
#!/bin/bash
|
||||
# tests/zulip-monitor-abiba.sh — regression test pinning the producer→consumer
|
||||
# contract between the pi Zulip extension's :9200/health payload and the Abiba
|
||||
# leg of scripts/zulip-monitor.sh.
|
||||
#
|
||||
# WHY THIS TEST EXISTS: 2026-09-09 live incident. The monitor parsed the health
|
||||
# payload at the WRONG nesting level (d.get('connected') at top level, while the
|
||||
# extension serves zulip.connected) so PI_CONNECTED was always False and every
|
||||
# monitor run restarted a healthy bot: pm2 showed restarts=8 with the process
|
||||
# created 2026-09-09T09:35:09Z, the monitor log recorded four ❌ Abiba verdicts
|
||||
# (04:23, 05:35, 06:55, 09:35 UTC) and zero ✅, while the Zulip server answered
|
||||
# HTTP 200 and the bot logged a clean connect plus continuing heartbeats. The
|
||||
# watchdog was the fault, not the connection. This test makes that class of
|
||||
# regression fail loudly instead of silently restarting healthy services.
|
||||
#
|
||||
# CONTRACT UNDER TEST (must hold for scripts/zulip-monitor.sh):
|
||||
# * Connection state is NESTED: zulip.connected (boolean) and zulip.last_error
|
||||
# live inside the `zulip` object. There is NO top-level `connected` and NO
|
||||
# retry counter anywhere in the payload (verified against the extension's
|
||||
# startHealthServer handler) — the old retry_count branch was dropped.
|
||||
# * zulip.connected=true -> log "✅ Connected", NO pm2 restart.
|
||||
# * zulip.connected=false -> alert, pm2 restart abiba-zulip.
|
||||
# * fetch error / non-2xx / empty body / unparseable body / missing or
|
||||
# non-boolean zulip.connected -> "⚠️ Probe failed" alert with a
|
||||
# "NOT restarting" label, NO pm2 restart. A parse miss must never kill a
|
||||
# healthy service.
|
||||
# * zulip.connected=true with last_error -> degraded 🟡 warning, no restart.
|
||||
#
|
||||
# HOW: the Abiba leg of the shipped script sits between the
|
||||
# `# -- abiba-leg-start` / `# -- abiba-leg-end` marker comments. This runner
|
||||
# extracts that block verbatim and executes it with a stubbed curl (fixture body
|
||||
# + HTTP code), recorded notify()/pm2 shims, and a temp $LOG. If the markers
|
||||
# disappear (fix reverted or renamed) extraction yields nothing and the suite
|
||||
# fails — the bug cannot return silently.
|
||||
#
|
||||
# Usage: bash tests/zulip-monitor-abiba.sh [path/to/zulip-monitor.sh]
|
||||
# Exit 0 iff every check passes.
|
||||
set -uo pipefail
|
||||
|
||||
ROOT=$(cd "$(dirname "$0")/.." && pwd)
|
||||
SCRIPT=${1:-"$ROOT/scripts/zulip-monitor.sh"}
|
||||
FIXTURES="$ROOT/tests/fixtures"
|
||||
TMP=$(mktemp -d)
|
||||
trap 'rm -rf "$TMP"' EXIT
|
||||
|
||||
PASS=0
|
||||
FAIL=0
|
||||
ok() { PASS=$((PASS + 1)); printf ' \033[32m✔\033[0m %s\n' "$1"; }
|
||||
bad() { FAIL=$((FAIL + 1)); printf ' \033[31m✘\033[0m %s\n' "$1"; }
|
||||
|
||||
echo "== tests/zulip-monitor-abiba.sh — Abiba leg vs :9200/health producer contract =="
|
||||
echo "target script: $SCRIPT"
|
||||
|
||||
# --- structural guards -------------------------------------------------------
|
||||
if ! grep -q '^# -- abiba-leg-start' "$SCRIPT"; then
|
||||
echo "✘ FATAL: $SCRIPT has no '# -- abiba-leg-start' marker — the fix has been reverted or renamed."
|
||||
exit 1
|
||||
fi
|
||||
if ! grep -q '^# -- abiba-leg-end' "$SCRIPT"; then
|
||||
echo "✘ FATAL: $SCRIPT has no '# -- abiba-leg-end' marker."
|
||||
exit 1
|
||||
fi
|
||||
|
||||
LEG="$TMP/leg.sh"
|
||||
awk '/^# -- abiba-leg-start/{f=1; next}
|
||||
/^# -- abiba-leg-end/{f=0; next}
|
||||
f' "$SCRIPT" > "$LEG"
|
||||
if [ ! -s "$LEG" ]; then
|
||||
echo "✘ FATAL: extracted Abiba leg is empty."
|
||||
exit 1
|
||||
fi
|
||||
echo "== structural =="
|
||||
if bash -n "$SCRIPT"; then ok "syntax: bash -n $SCRIPT"; else bad "syntax: bash -n $SCRIPT failed"; fi
|
||||
if bash -n "$LEG"; then ok "syntax: extracted leg parses (bash -n)"; else bad "syntax: extracted leg fails bash -n"; fi
|
||||
|
||||
# --- per-case harness ---------------------------------------------------------
|
||||
CURRENT_NAME=""
|
||||
CURRENT_DIR=""
|
||||
|
||||
# $1 case name, $2 http-code, $3 body (file path or literal)
|
||||
run_case() {
|
||||
local name="$1" http="$2" body_src="$3" body
|
||||
CURRENT_NAME="$name"
|
||||
CURRENT_DIR=$(mktemp -d "$TMP/case.XXXXXX")
|
||||
if [ -f "$body_src" ]; then
|
||||
body=$(cat "$body_src")
|
||||
else
|
||||
body="$body_src"
|
||||
fi
|
||||
(
|
||||
LOG="$CURRENT_DIR/log"; ISSUES=0
|
||||
notify() { printf 'ALERT [%s] %s\n' "$1" "$2" >> "$CURRENT_DIR/alerts"; }
|
||||
pm2() { printf 'PM2 %s\n' "$*" >> "$CURRENT_DIR/pm2"; }
|
||||
curl() {
|
||||
local url=""
|
||||
for a in "$@"; do case "$a" in http*) url="$a";; esac; done
|
||||
case "$url" in
|
||||
*:9200/health*)
|
||||
case " $* " in
|
||||
*"-w"*) printf '%s' "$http" ;; # -w '%{http_code}' code probe
|
||||
*) printf '%s' "$body" ;; # body probe
|
||||
esac ;;
|
||||
*)
|
||||
printf 'UNEXPECTED-CURL %s\n' "$*" >> "$CURRENT_DIR/unexpected-curl"
|
||||
return 7 ;;
|
||||
esac
|
||||
return 0
|
||||
}
|
||||
source "$LEG"
|
||||
)
|
||||
}
|
||||
|
||||
assert_log_has() {
|
||||
if grep -qF -- "$1" "$CURRENT_DIR/log"; then ok "$CURRENT_NAME — log has: $1"; else bad "$CURRENT_NAME — log MISSING: $1"; fi
|
||||
}
|
||||
assert_log_lacks() {
|
||||
if grep -qF -- "$1" "$CURRENT_DIR/log"; then bad "$CURRENT_NAME — log must NOT contain: $1"; else ok "$CURRENT_NAME — log correctly lacks: $1"; fi
|
||||
}
|
||||
assert_alert_has() {
|
||||
if grep -qF -- "$1" "$CURRENT_DIR/alerts"; then ok "$CURRENT_NAME — alert sent: $1"; else bad "$CURRENT_NAME — alert MISSING: $1"; fi
|
||||
}
|
||||
assert_alert_empty() {
|
||||
if [ ! -s "$CURRENT_DIR/alerts" ]; then ok "$CURRENT_NAME — no alert sent (quiet healthy path)"; else bad "$CURRENT_NAME — unexpected alert: $(cat "$CURRENT_DIR/alerts")"; fi
|
||||
}
|
||||
assert_pm2_restarted() {
|
||||
if grep -qF "PM2 restart abiba-zulip" "$CURRENT_DIR/pm2"; then ok "$CURRENT_NAME — pm2 restart abiba-zulip was called"; else bad "$CURRENT_NAME — expected pm2 restart abiba-zulip, pm2 log: $(cat "$CURRENT_DIR/pm2" 2>/dev/null)"; fi
|
||||
}
|
||||
assert_no_restart() {
|
||||
if [ ! -s "$CURRENT_DIR/pm2" ]; then ok "$CURRENT_NAME — NO pm2 restart (fail-safe holds)"; else bad "$CURRENT_NAME — pm2 was called but must NOT be: $(cat "$CURRENT_DIR/pm2")"; fi
|
||||
}
|
||||
assert_no_unexpected_curl() {
|
||||
if [ ! -s "$CURRENT_DIR/unexpected-curl" ]; then ok "$CURRENT_NAME — only :9200/health was probed"; else bad "$CURRENT_NAME — unexpected curl: $(cat "$CURRENT_DIR/unexpected-curl")"; fi
|
||||
}
|
||||
|
||||
# --- case 1: real payload shape, zulip.connected=true -> healthy, no restart --
|
||||
echo "== case 1: connected (real producer payload: nested zulip.connected=true) =="
|
||||
run_case "connected" 200 "$FIXTURES/zulip-health-connected.json"
|
||||
assert_log_has "Abiba: ✅ Connected (processed=0)"
|
||||
assert_log_lacks "Disconnected"
|
||||
assert_alert_empty
|
||||
assert_no_restart
|
||||
assert_no_unexpected_curl
|
||||
|
||||
# --- case 2: zulip.connected=false -> disconnected, restart -------------------
|
||||
echo "== case 2: disconnected (nested zulip.connected=false triggers restart) =="
|
||||
run_case "disconnected" 200 "$FIXTURES/zulip-health-disconnected.json"
|
||||
assert_log_has "Abiba: ❌ Disconnected — restarted"
|
||||
assert_alert_has "DISCONNECTED — restarting"
|
||||
assert_pm2_restarted
|
||||
assert_no_unexpected_curl
|
||||
|
||||
# --- cases 3-9: probe failures must alert and MUST NOT restart ----------------
|
||||
echo "== probe-failure cases: alert 'NOT restarting', zero pm2 restarts =="
|
||||
|
||||
run_case "empty body" 200 ""
|
||||
assert_log_has "Abiba: ⚠️ Probe failed"
|
||||
assert_log_lacks "❌ Disconnected"
|
||||
assert_alert_has "NOT restarting"
|
||||
assert_no_restart
|
||||
|
||||
run_case "garbage body" 200 '{not valid json!!'
|
||||
assert_log_has "Abiba: ⚠️ Probe failed"
|
||||
assert_alert_has "NOT restarting"
|
||||
assert_no_restart
|
||||
|
||||
run_case "missing zulip key" 200 '{"status":"ok","platform":"pi","agent":"abiba"}'
|
||||
assert_log_has "Probe failed"
|
||||
assert_alert_has "NOT restarting"
|
||||
assert_no_restart
|
||||
|
||||
run_case "zulip without connected" 200 '{"status":"ok","zulip":{"last_error":null}}'
|
||||
assert_log_has "Probe failed"
|
||||
assert_alert_has "NOT restarting"
|
||||
assert_no_restart
|
||||
|
||||
run_case "non-boolean connected" 200 '{"status":"ok","zulip":{"connected":"true"}}'
|
||||
assert_log_has "Probe failed"
|
||||
assert_alert_has "NOT restarting"
|
||||
assert_no_restart
|
||||
|
||||
run_case "fetch failure http 000" 000 ""
|
||||
assert_log_has "Probe failed"
|
||||
assert_alert_has "NOT restarting"
|
||||
assert_no_restart
|
||||
|
||||
run_case "non-2xx http 500" 500 '{"error":"boom"}'
|
||||
assert_log_has "Probe failed"
|
||||
assert_alert_has "NOT restarting"
|
||||
assert_no_restart
|
||||
|
||||
# --- case 10: connected but last_error set -> degraded 🟡, no restart ---------
|
||||
echo "== case 10: degraded (connected=true but last_error set) warns, no restart =="
|
||||
run_case "degraded" 200 '{"status":"ok","zulip":{"connected":true,"last_error":"transient queue hiccup","messages_processed":3}}'
|
||||
assert_log_has "Abiba: 🟡 Error: transient queue hiccup"
|
||||
assert_log_lacks "❌ Disconnected"
|
||||
assert_no_restart
|
||||
|
||||
# --- summary -------------------------------------------------------------------
|
||||
echo ""
|
||||
if [ "$FAIL" -eq 0 ]; then
|
||||
echo "✅ ALL CHECKS PASSED ($PASS/$PASS) — tests/zulip-monitor-abiba.sh"
|
||||
exit 0
|
||||
else
|
||||
echo "❌ $FAIL CHECK(S) FAILED ($PASS passed) — tests/zulip-monitor-abiba.sh"
|
||||
exit 1
|
||||
fi
|
||||
Reference in New Issue
Block a user