From 7ed2e4e9239f686f3f2f1f45184e81c3b9fd33ef Mon Sep 17 00:00:00 2001 From: root Date: Wed, 9 Sep 2026 09:51:46 +0000 Subject: [PATCH 1/2] fix(zulip-monitor): Abiba leg reads nested zulip.connected; probe failures never restart MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- scripts/zulip-monitor.sh | 81 +++++-- tests/fixtures/zulip-health-connected.json | 25 +++ tests/fixtures/zulip-health-disconnected.json | 25 +++ tests/zulip-monitor-abiba.sh | 206 ++++++++++++++++++ 4 files changed, 318 insertions(+), 19 deletions(-) create mode 100644 tests/fixtures/zulip-health-connected.json create mode 100644 tests/fixtures/zulip-health-disconnected.json create mode 100755 tests/zulip-monitor-abiba.sh diff --git a/scripts/zulip-monitor.sh b/scripts/zulip-monitor.sh index 114dfa7..812f039 100755 --- a/scripts/zulip-monitor.sh +++ b/scripts/zulip-monitor.sh @@ -46,26 +46,69 @@ else fi # ── Platform A: pi (Abiba) ── -PI_HEALTH=$(curl -sf --connect-timeout 5 http://localhost:9200/health 2>/dev/null || echo "{}") -PI_CONNECTED=$(echo "$PI_HEALTH" | python3 -c "import sys,json; d=json.load(sys.stdin); print(d.get('connected',False))" 2>/dev/null) -PI_ERROR=$(echo "$PI_HEALTH" | python3 -c "import sys,json; d=json.load(sys.stdin); print(d.get('last_error') or '')" 2>/dev/null) -PI_RETRIES=$(echo "$PI_HEALTH" | python3 -c "import sys,json; d=json.load(sys.stdin); print(d.get('retry_count',0))" 2>/dev/null) +# Probes the pi Zulip extension health endpoint (:9200/health, served by the +# extension's startHealthServer; shape documented in zulip-health.prose.md). +# FAIL-SAFE contract (pinned by tests/zulip-monitor-abiba.sh): connection state +# lives NESTED at zulip.connected / zulip.last_error β€” there is no top-level +# `connected` and no retry counter in the payload. A fetch error, non-2xx +# response, empty/unparseable body, or payload missing a boolean +# zulip.connected is a PROBE FAILURE: it alerts and NEVER calls pm2 restart. +# pm2 restart runs ONLY on affirmative zulip.connected=false. +# -- abiba-leg-start (verbatim-extracted by tests/zulip-monitor-abiba.sh) +PI_HTTP=$(curl -s -o /dev/null --connect-timeout 5 --max-time 10 -w '%{http_code}' http://localhost:9200/health 2>/dev/null || echo "000") +PI_BODY=$(curl -s --connect-timeout 5 --max-time 10 http://localhost:9200/health 2>/dev/null || true) +PI_STATE=$(printf '%s' "$PI_BODY" | python3 -c ' +import sys, json +code = sys.argv[1] +body = sys.stdin.read() +try: + d = json.loads(body) +except Exception: + sys.stdout.write("probe-failed|unparseable body") + sys.exit(0) +if not code.startswith("2"): + sys.stdout.write("probe-failed|HTTP %s" % code) + sys.exit(0) +if not isinstance(d, dict) or not isinstance(d.get("zulip"), dict): + sys.stdout.write("probe-failed|missing zulip.connected") + sys.exit(0) +z = d["zulip"] +if "connected" not in z or not isinstance(z["connected"], bool): + sys.stdout.write("probe-failed|missing or non-boolean zulip.connected") + sys.exit(0) +err = z.get("last_error") or "" +if z["connected"]: + if err: + sys.stdout.write("degraded|%s" % err) + else: + sys.stdout.write("healthy|%s" % z.get("messages_processed", 0)) +else: + sys.stdout.write("disconnected|") +' "$PI_HTTP" 2>/dev/null) || PI_STATE="probe-failed|python error" +PI_VERDICT=${PI_STATE%%|*} +PI_DETAIL=${PI_STATE#*|} -if [ "$PI_CONNECTED" != "True" ]; then - notify "πŸ”΄" "Abiba pi extension DISCONNECTED β€” restarting" - pm2 restart abiba-zulip 2>/dev/null || true - ISSUES=$((ISSUES + 1)) - echo " Abiba: ❌ Disconnected β€” restarted" >> "$LOG" -elif [ -n "$PI_ERROR" ]; then - notify "🟑" "Abiba pi extension error: ${PI_ERROR:0:100}" - echo " Abiba: 🟑 Error: ${PI_ERROR:0:100}" >> "$LOG" -elif [ "$PI_RETRIES" -ge 3 ]; then - notify "🟑" "Abiba pi extension: $PI_RETRIES retries β€” restarting" - pm2 restart abiba-zulip 2>/dev/null || true - echo " Abiba: 🟑 $PI_RETRIES retries β€” restarted" >> "$LOG" -else - echo " Abiba: βœ… Connected (processed=$(echo "$PI_HEALTH" | python3 -c "import sys,json; d=json.load(sys.stdin); print(d.get('messages_processed',0))" 2>/dev/null))" >> "$LOG" -fi +case "$PI_VERDICT" in + healthy) + echo " Abiba: βœ… Connected (processed=$PI_DETAIL)" >> "$LOG" ;; + degraded) + notify "🟑" "Abiba pi extension error: ${PI_DETAIL:0:100}" + echo " Abiba: 🟑 Error: ${PI_DETAIL:0:100}" >> "$LOG" ;; + disconnected) + notify "πŸ”΄" "Abiba pi extension DISCONNECTED β€” restarting" + pm2 restart abiba-zulip 2>/dev/null || true + ISSUES=$((ISSUES + 1)) + echo " Abiba: ❌ Disconnected β€” restarted" >> "$LOG" ;; + probe-failed) + notify "🟠" "Abiba pi extension health probe FAILED (${PI_DETAIL}; HTTP $PI_HTTP) β€” NOT restarting, manual check needed" + ISSUES=$((ISSUES + 1)) + echo " Abiba: ⚠️ Probe failed (${PI_DETAIL}; HTTP $PI_HTTP) β€” NOT restarted" >> "$LOG" ;; + *) + notify "🟠" "Abiba pi extension health probe returned unexpected verdict (${PI_STATE}) β€” NOT restarting, manual check needed" + ISSUES=$((ISSUES + 1)) + echo " Abiba: ⚠️ Unexpected probe verdict (${PI_STATE}) β€” NOT restarted" >> "$LOG" ;; +esac +# -- abiba-leg-end # ── Platform B: Tanko (DSH dsh-web on amdpve CT 112) ── # Direct SSH to 192.168.68.122 is not a dependency of this monitor β€” per-worker diff --git a/tests/fixtures/zulip-health-connected.json b/tests/fixtures/zulip-health-connected.json new file mode 100644 index 0000000..61ecc6b --- /dev/null +++ b/tests/fixtures/zulip-health-connected.json @@ -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 +} diff --git a/tests/fixtures/zulip-health-disconnected.json b/tests/fixtures/zulip-health-disconnected.json new file mode 100644 index 0000000..0296d77 --- /dev/null +++ b/tests/fixtures/zulip-health-disconnected.json @@ -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 +} diff --git a/tests/zulip-monitor-abiba.sh b/tests/zulip-monitor-abiba.sh new file mode 100755 index 0000000..1de38c6 --- /dev/null +++ b/tests/zulip-monitor-abiba.sh @@ -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 -- 2.54.0 From f57923b4fac5e4cc1932e75647d332b32c9c044f Mon Sep 17 00:00:00 2001 From: root Date: Wed, 9 Sep 2026 10:26:35 +0000 Subject: [PATCH 2/2] no-mistakes(document): test shellcheck hygiene; flagged monitor contract doc drift --- tests/zulip-monitor-abiba.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/zulip-monitor-abiba.sh b/tests/zulip-monitor-abiba.sh index 1de38c6..8f8ae25 100755 --- a/tests/zulip-monitor-abiba.sh +++ b/tests/zulip-monitor-abiba.sh @@ -35,6 +35,11 @@ # # Usage: bash tests/zulip-monitor-abiba.sh [path/to/zulip-monitor.sh] # Exit 0 iff every check passes. +# +# shellcheck disable=SC2034,SC2329,SC1090 +# LOG/ISSUES and the notify/pm2/curl stubs below are consumed at runtime by +# the leg extracted between the marker comments and `source`d in each case; +# the static analyzer cannot see across that dynamic source, so it flags them. set -uo pipefail ROOT=$(cd "$(dirname "$0")/.." && pwd) -- 2.54.0