Fix registry badge showing pass despite a partial/failed cycle

fillRegistrySummary derived LastResult from whichever single checks row
happened to have the latest checked_at, not the cycle's actual aggregated
result — a cycle with a mix of passing and failing checks (e.g. one egress
target timed out while the rest, including the chronologically-last check,
succeeded) rendered as a green "pass" badge on /registry, disagreeing with
the correct "partial" badge already shown on /ips for the same address.

Now prefers ip_queue.overall_result (the orchestrator's own aggregation)
when a live queue row has a finished cycle, leaves the badge blank while a
cycle is still in progress, and only falls back to classifying the most
recent cycle's own checks (pass/fail/partial) once the address has been
deleted from the queue and overall_result is no longer available.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

bin/SHA256SUMS

@@ -1,4 +1,4 @@
-62fb7e4e6a05f60e0e2f1d113c11b4ef46840eadb87202c3539a508ea0e996ae  control-api
+d3559945184a9716fc1dae0defbd008b23352f8695ad9b96174838523da9c30d  control-api
-7c11ffe00f09cbfc33bab57d8a889cdfedacc69f0049a5881705dfabc154fa76  validator-agent
+bad182f50f6ec38cb767bd275d91497298188258306a81178547b80a81fcbfe3  validator-agent
-543f7df07939d576b79dc8fdc655ee6ff5577c834d5606a1e99687b99d2a8927  prober
+383858b838e2707b7eae271e5023f9a28806ff8b34b3f88b40e27eb9e4eb8128  prober
-2e9798e86ebaf92e70622cbd76ca0d3de8f90958eadbe7c71fb2118940b4a275  admin-dashboard
+8f5ebc9983dd1aa3cdff212e6d7b17406e49fdd2de7364ae1833a0a5629f0217  admin-dashboard

internal/dashboard/templates/registry.html

@@ -39,7 +39,9 @@
 <td class="num" data-label="Циклов">{{.TotalCycles}}</td>
 <td data-label="Последний результат">
 {{if eq .LastResult "pass"}}<span class="pill pill-success">pass</span>
+{{else if eq .LastResult "partial"}}<span class="pill pill-warning">partial</span>
 {{else if eq .LastResult "fail"}}<span class="pill pill-danger">fail</span>
+{{else if eq .LastResult "cancelled"}}<span class="pill pill-cancel">cancelled</span>
 {{else}}<span class="pill pill-neutral">—</span>{{end}}
 </td>
 <td data-label="Сейчас в очереди">

internal/dashboard/templates/registry_detail.html

@@ -23,6 +23,10 @@
 <p><a href="/registry">&larr; К реестру</a></p>
 <div class="topbar">
 <h1 class="mono" style="display:flex;align-items:center;gap:10px">{{.History.Registry.IPAddress}}
+{{if eq .History.Registry.LastResult "pass"}}<span class="pill pill-success">pass</span>
+{{else if eq .History.Registry.LastResult "partial"}}<span class="pill pill-warning">partial</span>
+{{else if eq .History.Registry.LastResult "fail"}}<span class="pill pill-danger">fail</span>
+{{else if eq .History.Registry.LastResult "cancelled"}}<span class="pill pill-cancel">cancelled</span>{{end}}
 {{if .History.Registry.InQueue}}<a class="pill pill-info" href="/ips/{{.History.Registry.IPAddress}}">{{.History.Registry.CurrentState}} · в очереди</a>{{else}}<span class="pill pill-neutral">не в очереди</span>{{end}}
 </h1>
 </div>

internal/db/queries_registry.go

@@ -110,6 +110,13 @@ func (d *DB) GetRegistryByAddress(ctx context.Context, address string) (*Registr
 	return s, nil
 }
 
+// fillRegistrySummary computes the badge-level summary shown on the
+// registry list/detail pages. LastResult must reflect the *aggregated*
+// outcome of the most recent cycle (pass/partial/fail/cancelled), not the
+// success flag of whichever individual check happens to have the latest
+// checked_at — a cycle with a mix of passing and failing checks (e.g. one
+// egress target timed out while the rest succeeded) is "partial", even
+// though the chronologically-last check to report in might have passed.
 func (d *DB) fillRegistrySummary(ctx context.Context, s *RegistrySummary) error {
 	if err := d.QueryRowContext(ctx, `
 		SELECT COUNT(DISTINCT cycle_id) FROM checks WHERE registry_id=?
@@ -117,37 +124,86 @@ func (d *DB) fillRegistrySummary(ctx context.Context, s *RegistrySummary) error
 		return err
 	}
 
-	var lastResult sql.NullBool
 	var lastCheckedAt sql.NullString
 	if err := d.QueryRowContext(ctx, `
-		SELECT success, checked_at FROM checks WHERE registry_id=? ORDER BY cycle_id DESC, checked_at DESC LIMIT 1
+		SELECT checked_at FROM checks WHERE registry_id=? ORDER BY cycle_id DESC, checked_at DESC LIMIT 1
-	`, s.ID).Scan(&lastResult, &lastCheckedAt); err != nil && err != sql.ErrNoRows {
+	`, s.ID).Scan(&lastCheckedAt); err != nil && err != sql.ErrNoRows {
 		return err
 	}
-	if lastResult.Valid {
-		if lastResult.Bool {
-			s.LastResult = "pass"
-		} else {
-			s.LastResult = "fail"
-		}
-	}
 	t, err := nullStringToTimePtr(lastCheckedAt)
 	if err != nil {
 		return err
 	}
 	s.LastCheckedAt = t
 
-	var state sql.NullString
+	var state, overallResult sql.NullString
-	if err := d.QueryRowContext(ctx, `SELECT state FROM ip_queue WHERE registry_id=?`, s.ID).Scan(&state); err != nil && err != sql.ErrNoRows {
+	err = d.QueryRowContext(ctx, `SELECT state, overall_result FROM ip_queue WHERE registry_id=?`, s.ID).
+		Scan(&state, &overallResult)
+	if err != nil && err != sql.ErrNoRows {
 		return err
 	}
-	if state.Valid {
+	if err == nil {
 		s.InQueue = true
 		s.CurrentState = state.String
 	}
+
+	switch {
+	case overallResult.Valid && overallResult.String != "":
+		// The address has a live ip_queue row with a finished cycle
+		// (done/failed) — overall_result is the orchestrator's own
+		// aggregation (internal/orchestrator.aggregateAndRelease /
+		// db.CancelIP), the authoritative source of truth. Use it as-is
+		// rather than re-deriving it from raw check rows.
+		s.LastResult = overallResult.String
+	case s.InQueue:
+		// A live ip_queue row exists but its current cycle hasn't finished
+		// yet (still queued/checking/etc, overall_result not set) — no
+		// verdict to show yet; leave LastResult empty rather than guessing
+		// from a still-incomplete set of checks.
+	default:
+		// No live ip_queue row (deleted from the queue) — fall back to
+		// classifying the most recent recorded cycle from its own checks,
+		// the same pass/fail/partial rule aggregateAndRelease uses (minus
+		// "missing" checks, which aren't knowable after the fact).
+		result, err := lastCycleResultFromChecks(ctx, d, s.ID)
+		if err != nil {
+			return err
+		}
+		s.LastResult = result
+	}
 	return nil
 }
 
+// lastCycleResultFromChecks classifies the most recent cycle recorded for
+// registryID directly from its checks rows: pass if every recorded check
+// succeeded, fail if every one failed, partial on a mix. Returns "" if no
+// checks are recorded at all.
+func lastCycleResultFromChecks(ctx context.Context, d *DB, registryID int64) (string, error) {
+	var cycle sql.NullInt64
+	if err := d.QueryRowContext(ctx, `SELECT MAX(cycle_id) FROM checks WHERE registry_id=?`, registryID).Scan(&cycle); err != nil {
+		return "", err
+	}
+	if !cycle.Valid {
+		return "", nil
+	}
+	var total, passed int
+	if err := d.QueryRowContext(ctx, `
+		SELECT COUNT(*), COALESCE(SUM(success), 0) FROM checks WHERE registry_id=? AND cycle_id=?
+	`, registryID, cycle.Int64).Scan(&total, &passed); err != nil {
+		return "", err
+	}
+	switch {
+	case total == 0:
+		return "", nil
+	case passed == 0:
+		return ResultFail, nil
+	case passed == total:
+		return ResultPass, nil
+	default:
+		return ResultPartial, nil
+	}
+}
+
 func scanRegistryItems(rows *sql.Rows) ([]RegistryItem, error) {
 	var out []RegistryItem
 	for rows.Next() {

internal/db/queries_registry_test.go

@@ -3,6 +3,7 @@ package db
 import (
 	"errors"
 	"testing"
+	"time"
 
 	"cloudipvalidator/internal/config"
 )
@@ -240,3 +241,130 @@ func TestGetSetHistoryRetentionCyclesRoundTrip(t *testing.T) {
 		t.Fatalf("expected 5, got %d", s.HistoryRetentionCycles)
 	}
 }
+
+// TestRegistryLastResultReflectsAggregatedOutcome guards against the badge
+// bug where LastResult was derived from whichever single check happened to
+// have the latest checked_at, instead of the cycle's actual aggregated
+// result — a cycle with one failing check and several passing ones is
+// "partial", even if the chronologically-last check to report in passed.
+func TestRegistryLastResultReflectsAggregatedOutcome(t *testing.T) {
+	d, ctx := newTestDB(t)
+
+	if _, err := d.SubmitIPs(ctx, []string{"1.2.3.4"}); err != nil {
+		t.Fatalf("submit ips: %v", err)
+	}
+	ip, err := d.GetIPByAddress(ctx, "1.2.3.4")
+	if err != nil {
+		t.Fatalf("get ip: %v", err)
+	}
+
+	// The chronologically-last check (icmp, checked_at later) passes, but
+	// an earlier one (https) failed — the cycle as a whole is partial.
+	now := Now()
+	if err := d.UpsertCheck(ctx, Check{
+		IPID: ip.ID, IPAddress: ip.IPAddress, AttemptNumber: ip.AttemptNumber,
+		Source: SourceEgress, CheckType: "https", Target: "https://example.test",
+		Success: false, CheckedAt: now,
+	}); err != nil {
+		t.Fatalf("upsert check (https, fail): %v", err)
+	}
+	if err := d.UpsertCheck(ctx, Check{
+		IPID: ip.ID, IPAddress: ip.IPAddress, AttemptNumber: ip.AttemptNumber,
+		Source: SourceEgress, CheckType: "icmp", Target: "https://example.test",
+		Success: true, CheckedAt: now.Add(time.Second),
+	}); err != nil {
+		t.Fatalf("upsert check (icmp, pass): %v", err)
+	}
+
+	// Simulate what orchestrator.aggregateAndRelease actually does: compute
+	// the aggregated result and persist it via FinishIP.
+	if err := d.FinishIP(ctx, ip.ID, ResultPartial); err != nil {
+		t.Fatalf("finish ip: %v", err)
+	}
+
+	summary, err := d.GetRegistryByAddress(ctx, "1.2.3.4")
+	if err != nil {
+		t.Fatalf("get registry: %v", err)
+	}
+	if summary.LastResult != ResultPartial {
+		t.Fatalf("expected LastResult=partial (aggregated outcome), got %q", summary.LastResult)
+	}
+	if !summary.InQueue || summary.CurrentState != IPDone {
+		t.Fatalf("expected still in queue as done, got in_queue=%v state=%q", summary.InQueue, summary.CurrentState)
+	}
+
+	all, err := d.ListRegistry(ctx)
+	if err != nil {
+		t.Fatalf("list registry: %v", err)
+	}
+	if len(all) != 1 || all[0].LastResult != ResultPartial {
+		t.Fatalf("expected ListRegistry to agree, got %+v", all)
+	}
+}
+
+// TestRegistryLastResultEmptyWhileCycleInProgress proves an address with a
+// live but not-yet-finished cycle (overall_result still empty) doesn't get
+// a premature pass/fail verdict.
+func TestRegistryLastResultEmptyWhileCycleInProgress(t *testing.T) {
+	d, ctx := newTestDB(t)
+
+	if _, err := d.SubmitIPs(ctx, []string{"1.2.3.4"}); err != nil {
+		t.Fatalf("submit ips: %v", err)
+	}
+
+	summary, err := d.GetRegistryByAddress(ctx, "1.2.3.4")
+	if err != nil {
+		t.Fatalf("get registry: %v", err)
+	}
+	if summary.LastResult != "" {
+		t.Fatalf("expected no verdict yet for a still-queued address, got %q", summary.LastResult)
+	}
+	if !summary.InQueue || summary.CurrentState != IPQueued {
+		t.Fatalf("expected in_queue=true state=queued, got in_queue=%v state=%q", summary.InQueue, summary.CurrentState)
+	}
+}
+
+// TestRegistryLastResultFallsBackToChecksAfterDeletion proves that once an
+// address's ip_queue row is gone (no more overall_result to read), the
+// registry still derives a sensible partial/pass/fail verdict from the
+// last recorded cycle's own checks.
+func TestRegistryLastResultFallsBackToChecksAfterDeletion(t *testing.T) {
+	d, ctx := newTestDB(t)
+
+	if _, err := d.SubmitIPs(ctx, []string{"1.2.3.4"}); err != nil {
+		t.Fatalf("submit ips: %v", err)
+	}
+	ip, err := d.GetIPByAddress(ctx, "1.2.3.4")
+	if err != nil {
+		t.Fatalf("get ip: %v", err)
+	}
+	now := Now()
+	if err := d.UpsertCheck(ctx, Check{
+		IPID: ip.ID, IPAddress: ip.IPAddress, AttemptNumber: ip.AttemptNumber,
+		Source: SourceEgress, CheckType: "https", Target: "https://example.test",
+		Success: false, CheckedAt: now,
+	}); err != nil {
+		t.Fatalf("upsert check (fail): %v", err)
+	}
+	if err := d.UpsertCheck(ctx, Check{
+		IPID: ip.ID, IPAddress: ip.IPAddress, AttemptNumber: ip.AttemptNumber,
+		Source: SourceEgress, CheckType: "icmp", Target: "https://example.test",
+		Success: true, CheckedAt: now.Add(time.Second),
+	}); err != nil {
+		t.Fatalf("upsert check (pass): %v", err)
+	}
+	if err := d.DeleteIP(ctx, ip.ID); err != nil {
+		t.Fatalf("delete ip: %v", err)
+	}
+
+	summary, err := d.GetRegistryByAddress(ctx, "1.2.3.4")
+	if err != nil {
+		t.Fatalf("get registry: %v", err)
+	}
+	if summary.InQueue {
+		t.Fatalf("expected address no longer in queue")
+	}
+	if summary.LastResult != ResultPartial {
+		t.Fatalf("expected fallback classification partial (mixed pass/fail checks), got %q", summary.LastResult)
+	}
+}