From 623c6d0fe885dfe4db0d19cd104a738e04d6851b Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 14 Sep 2026 14:20:15 -0400 Subject: [PATCH] fix(rig): verify-pending-updates.sh cannot pass on an empty site list; numeric-safe comparison; earlier cleanup trap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gate could pass vacuously. Every site-list read went through a bare `ROWS=$(site_rows)` with neither the exit status nor the row count checked, and an empty result satisfied every downstream assertion for free: no rows means no malformed hash for B1/B2 to report, and a diff of two empty strings is no difference at all for C1/D2. A down management API would therefore have read as a clean gate. Reads now go through read_rows, which fails loudly on a non-zero CLI exit (site_rows exits 2 on an empty list, and pipefail surfaces a failed call) and aborts unless the line count equals the site count established in step A — keeping the running tally so the abort still prints PASS/FAIL. Verified against an unreachable URL: FAIL 1, exit 1. The join(1)/cut/diff comparisons assumed lexicographic id order matches numeric order, which breaks once a site id reaches 10 alongside single-digit ids (and BSD join silently drops the unmatched rows rather than complaining). Replaced with a python3 comparison keyed by integer id that also asserts the id SETS match, so a site appearing or vanishing between two reads invalidates the comparison instead of quietly shrinking it. Same for the summary table. Also: set -euo pipefail like every sibling script (the cmd && pass || fail guards keep -e safe, and -e does not cover var=$(cmd) — hence the explicit checks above); the cleanup trap is armed BEFORE the shared-script create and falls back to finding the probe by its unique name, so a create that succeeds server-side but whose id we fail to parse can no longer orphan a probe; mktemp instead of /tmp/pu-deploy.$$. PendingUpdatesTests: the rail-scroll check asserted scrollHeight >= clientHeight, which holds for every element. It now asserts what the pane is actually for — the pane is bounded by the viewport and the rail footer holding the indicator stays on screen, so a nav that grows the rail instead of scrolling fails. Live values are genuinely overflowing (scrollHeight 1196 vs clientHeight 680, footer bottom 765 of 900), so the assertion has real content. Re-run against the rig: PASS 13 / FAIL 0; Playwright 3/3. Claude-Session: https://claude.ai/code/session_01Y5ef34iUUMjmwnMpL9xgmB --- docker/verify-pending-updates.sh | 232 ++++++++++++------ .../PendingUpdatesTests.cs | 25 +- 2 files changed, 182 insertions(+), 75 deletions(-) diff --git a/docker/verify-pending-updates.sh b/docker/verify-pending-updates.sh index 95472e2e..45b84eb1 100755 --- a/docker/verify-pending-updates.sh +++ b/docker/verify-pending-updates.sh @@ -1,5 +1,5 @@ #!/bin/bash -set -uo pipefail +set -euo pipefail # Live gate for the PER-SITE ARTIFACT REVISION that backs the pending-updates indicator # (design docs/plans/2026-09-14-pending-updates-design.md, plan task 10). Drives the real @@ -17,11 +17,15 @@ set -uo pipefail # E Central UI /admin/sites answers 200 and serves the Artifacts badge markup (soft) # # NOTE on step E: the Sites page badge and the rail-footer indicator are Blazor Server -# interactive content — the badge text only lands after the circuit's post-render -# LoadUpdatesAsync, which curl never sees. E is therefore a reachability + static-markup -# smoke only; the real browser coverage is +# interactive content — with login ENABLED the badge text only lands after the circuit's +# post-render LoadUpdatesAsync, which curl never sees. E is therefore a reachability + +# static-markup smoke only; the real browser coverage is # tests/ZB.MOM.WW.ScadaBridge.CentralUI.PlaywrightTests/PendingUpdatesTests.cs. # +# EVERY read of the site list is checked for both a failed CLI call and a short row count +# (read_rows below). Without that an empty list would pass the whole gate vacuously: "no +# rows" means no malformed hash to report and no difference between two row sets. +# # Usage: bash docker/verify-pending-updates.sh # SCADABRIDGE_URL=http://localhost:9000 bash docker/verify-pending-updates.sh @@ -36,6 +40,10 @@ PASS=0; FAIL=0 pass() { PASS=$((PASS+1)); echo " PASS $*"; } fail() { FAIL=$((FAIL+1)); echo " FAIL $*"; } +# Bail out mid-run without losing the tally. Used for the failures that make every later +# assertion meaningless (no sites, an unreachable management API, a short row read). +abort() { echo ""; echo "PASS $PASS / FAIL $FAIL"; exit 1; } + [ -x "$CLI" ] || { echo "CLI not found at $CLI — build it: dotnet build src/ZB.MOM.WW.ScadaBridge.CLI"; exit 1; } # The CLI's JSON envelope varies by command (bare array vs {"data": [...]}); normalise once. @@ -46,24 +54,85 @@ if isinstance(d, dict): d = d.get("data", d) ' -# Prints one "\t\t\t" line per site, id-sorted. +# Prints one "\t\t\t" line per site, id-sorted +# NUMERICALLY. Exits non-zero if the CLI call fails (pipefail) or the list is empty. site_rows() { - "$CLI" site list "${A[@]}" 2>/dev/null | python3 -c "$UNWRAP"' -for s in sorted(d, key=lambda x: x["id"]): + "$CLI" site list "${A[@]}" | python3 -c "$UNWRAP"' +import sys +if not d: + sys.exit(2) +for s in sorted(d, key=lambda x: int(x["id"])): print("%s\t%s\t%s\t%s" % (s["id"], s.get("siteIdentifier"), s.get("artifactRevisionHash") or "-", s.get("artifactsDeployedAt") or "-")) ' } -deploy_all() { - "$CLI" site deploy-artifacts "${A[@]}" > /tmp/pu-deploy.$$ 2>&1 - local rc=$? - if [ $rc -ne 0 ]; then - echo " deploy-artifacts exited $rc:"; sed 's/^/ /' /tmp/pu-deploy.$$ +# read_rows