From e5d103f498fdf62439f1a51f73abac69c0a84048 Mon Sep 17 00:00:00 2001 From: "Claude (CC#2)" Date: Fri, 22 May 2026 10:17:50 -0700 Subject: [PATCH] =?UTF-8?q?chore(kora):=20KR-FRONTEND-CLEANUP=20=E2=80=94?= =?UTF-8?q?=20tsc=20drift=20+=20stale=20mcp=5Fclients=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Part A — tsc enum drift fix (4 errors → 0) ============================================ CC#1's KR-FEAT-HEARTBEAT ST2 (PR #118) added 4 fields to the heartbeat surface; FE consumers in HeartbeatPanel.tsx + DashboardPage.tsx didn't handle the additive shape, leaving `tsc -b` with 4 errors on bare feature/phase2-upgrades. Fixes: * HeartbeatStatus enum gained "unknown" (probe pending — cold-start / auth-missing / in-flight). Panel now renders it as a muted-outline pill with "probe pending" label + a CircleDashed icon. Distinct from "unhealthy" — pending is not failing. * HeartbeatService.last_check_at became nullable. Helper signatures formatTimestamp / formatRelative now accept `string | null`; null path renders "never checked" (matches the MCP-clients health-display convention from PR #117 to keep the operator's mental model consistent). * HeartbeatService.error: string | null surfaced in the expanded detail view as a destructive-toned
 block
    (mirrors MCP-clients last_error). Plain-text rendering;
    no dangerouslySetInnerHTML — probe output is untrusted.
  * HeartbeatServicesResponse.cache_warming: boolean. Panel
    renders "Probes warming up…" Hourglass banner above the
    services list. Dashboard card mutes the headline tone +
    appends "· warming" to suppress false-outage signal
    during daemon cold-start.

Also: dashboard counts.unknown bucketed separately from
healthy/degraded/unhealthy so the operator can distinguish
"scheduler not done yet" from "service down".

Part B — test_web_server_mcp_clients.py: investigation only
============================================================

Root cause is NOT stale assertions (Part B hypothesis was
incorrect). All 13 failures + 5 errors collapse on a single
`ModuleNotFoundError: No module named 'slowapi'` raised when
the test imports `kora_cli.web_server` → which transitively
imports `kora_cli.listeners.__init__` → `kora_cli.listeners.
webhooks` → `slowapi`. That import is unconditional at the
package __init__ level.

The `slowapi` package is declared in pyproject.toml's `web`
extra (`fastapi==0.133.1`, `uvicorn[standard]==0.41.0`,
`slowapi>=0.1.9`), but the test environment only installs
`--extra dev` — so slowapi is missing in CI/dev test runs.

Confirmed: running `uv sync --frozen --extra dev --extra web`
+ pytest passes 19/19 of the mcp_clients tests. No assertion
shape is stale.

This is the existing task #265 (slowapi). Per the bucket
spec's "Genuine bug → STOP" rule, and §3 non-scope
("Investigating tasks #265 (slowapi) — separate"), NOT
applying the fix here. Possible fixes for #265:

  (a) Add `slowapi>=0.1.9` to the `dev` extra in pyproject.toml
      (1-line; tests then run from a single extra).
  (b) Add the `web` extra to CI test runs alongside `dev`.
  (c) Make slowapi imports lazy in kora_cli/listeners/webhooks.py
      so listeners/__init__.py doesn't fail at import time
      when only test/dev deps are installed.

(a) is simplest; recommend that path for the #265 PR.

Tests
=====

  * tests/kora_cli/test_heartbeat_panel_drift_fixes.py — 11
    new source-pin tests: source files exist, "unknown" arm
    in STATUS_TONE / STATUS_LABEL / StatusIcon switch,
    dashboard counts include "unknown", formatTimestamp +
    formatRelative accept string|null with "never checked"
    null path, service.error rendered as JSX child (no
    dangerouslySetInnerHTML), cache_warming banner + dashboard
    headline tone suppression.
  * Full admin-panel regression: 260/260 across 22 suites
    (including the previously-failing mcp_clients suite,
    which passes once --extra web is installed).
  * `pnpm tsc -b` clean (was 4 errors → now 0).
  * `pnpm build` clean.

Refs: PRs #108 (KR-MCP-CLIENTS-FLIP) + #113 (KR-MCP-CONSUMPTION
ST2) + #118 (KR-FEAT-HEARTBEAT ST2); task #269 (closed by
Part B investigation: dependency issue, not stale assertions);
task #265 (slowapi dependency — open, recommended fix above).

Co-Authored-By: Claude Opus 4.7 (1M context) 
---
 .../test_heartbeat_panel_drift_fixes.py       | 179 ++++++++++++++++++
 web/src/pages/DashboardPage.tsx               |  23 ++-
 web/src/pages/HeartbeatPanel.tsx              |  79 +++++++-
 3 files changed, 267 insertions(+), 14 deletions(-)
 create mode 100644 tests/kora_cli/test_heartbeat_panel_drift_fixes.py

diff --git a/tests/kora_cli/test_heartbeat_panel_drift_fixes.py b/tests/kora_cli/test_heartbeat_panel_drift_fixes.py
new file mode 100644
index 000000000000..6fa77f2d9e2f
--- /dev/null
+++ b/tests/kora_cli/test_heartbeat_panel_drift_fixes.py
@@ -0,0 +1,179 @@
+"""Source-pin tests for KR-FRONTEND-CLEANUP Part A.
+
+CC#1's KR-FEAT-HEARTBEAT ST2 (PR #118) added 4 fields to the
+heartbeat surface that the FE consumers didn't handle:
+  * HeartbeatStatus enum gained "unknown" (probe pending)
+  * HeartbeatService.last_check_at became nullable
+  * HeartbeatService.error: string | null
+  * HeartbeatServicesResponse.cache_warming: boolean
+
+This file pins that HeartbeatPanel.tsx + DashboardPage.tsx render
+all four. Follows the CC#2 source-grep pattern (no FE test runner
+in the repo) established by KR-MCP-CLIENTS-HEALTH-DISPLAY (#117).
+"""
+
+import re
+from pathlib import Path
+
+
+_REPO_ROOT = Path(__file__).resolve().parents[2]
+_PANEL_PATH = _REPO_ROOT / "web" / "src" / "pages" / "HeartbeatPanel.tsx"
+_DASHBOARD_PATH = _REPO_ROOT / "web" / "src" / "pages" / "DashboardPage.tsx"
+
+
+def _strip_ts_comments(src: str) -> str:
+    src = re.sub(r"\{/\*.*?\*/\}", "", src, flags=re.DOTALL)
+    src = re.sub(r"/\*.*?\*/", "", src, flags=re.DOTALL)
+    src = re.sub(r"(^|[^:])//[^\n]*", r"\1", src)
+    return src
+
+
+# ---- 1. source files exist ----------------------------------------
+
+
+def test_panel_source_exists():
+    assert _PANEL_PATH.is_file(), f"missing: {_PANEL_PATH}"
+
+
+def test_dashboard_source_exists():
+    assert _DASHBOARD_PATH.is_file(), f"missing: {_DASHBOARD_PATH}"
+
+
+# ---- 2. "unknown" status arm handled --------------------------------
+
+
+def test_panel_status_tone_map_includes_unknown():
+    """HeartbeatStatus enum gained "unknown" — the panel's STATUS_TONE
+    map must include it so TypeScript exhaustiveness holds and the
+    badge gets a tone. Catches a future enum addition that skips
+    this consumer."""
+    src = _PANEL_PATH.read_text()
+    # The literal "unknown:" must appear as a STATUS_TONE map entry
+    # (left-hand side of a key/value pair inside the Record literal).
+    assert re.search(r"\bunknown\s*:", src), (
+        "HeartbeatPanel.tsx STATUS_TONE / STATUS_LABEL maps must "
+        'include the "unknown" arm'
+    )
+
+
+def test_panel_status_icon_handles_unknown():
+    """StatusIcon switch must include a "unknown" case so TS
+    exhaustiveness check passes and the icon renders."""
+    src = _PANEL_PATH.read_text()
+    assert re.search(r'case "unknown":', src), (
+        'HeartbeatPanel.tsx StatusIcon must have a `case "unknown":` arm'
+    )
+
+
+def test_dashboard_heartbeat_counts_include_unknown():
+    """The dashboard's HeartbeatCardBody aggregator must bucket
+    "unknown" — otherwise the Record literal
+    is incomplete and tsc fails."""
+    src = _DASHBOARD_PATH.read_text()
+    # The counts object literal must initialize an `unknown:` field.
+    assert re.search(
+        r"counts:\s*Record\s*=\s*\{[^}]*\bunknown\s*:",
+        src,
+        re.DOTALL,
+    ), (
+        "DashboardPage HeartbeatCardBody counts object must include "
+        'the "unknown" key (matches Record)'
+    )
+
+
+# ---- 3. nullable last_check_at handled -----------------------------
+
+
+def test_format_timestamp_accepts_null():
+    """formatTimestamp + formatRelative must accept `string | null`
+    so HeartbeatService.last_check_at: string | null type-checks.
+    Catches a regression that drops the null arm from the helper
+    signatures."""
+    src = _PANEL_PATH.read_text()
+    # Both helpers must declare nullable parameter types.
+    assert re.search(
+        r"function formatTimestamp\(iso:\s*string\s*\|\s*null\)",
+        src,
+    ), "formatTimestamp must accept string | null"
+    assert re.search(
+        r"function formatRelative\(iso:\s*string\s*\|\s*null\)",
+        src,
+    ), "formatRelative must accept string | null"
+
+
+def test_format_relative_null_path_says_never_checked():
+    """The null branch of formatRelative should produce a user-
+    facing "never checked" label (consistent with the MCP-clients
+    health-display pattern from #117), not an empty string."""
+    src = _PANEL_PATH.read_text()
+    assert '"never checked"' in src, (
+        'formatRelative null path should return "never checked" '
+        "(matches the MCP-clients health-display convention)"
+    )
+
+
+# ---- 4. error field rendered in expanded view ----------------------
+
+
+def test_panel_renders_service_error_field():
+    """KR-FEAT-HEARTBEAT ST2 error field rendered in the expanded
+    detail view. Source-pin: branch on service.error !== null
+    and render in a 
 for multi-line legibility (mirrors
+    MCP-clients last_error pattern from #117)."""
+    src = _PANEL_PATH.read_text()
+    assert "service.error" in src, (
+        "HeartbeatPanel.tsx must reference service.error in render"
+    )
+    # Must appear inside a JSX expression (rendered as text node)
+    assert re.search(r"\{[^{}]*service\.error[^{}]*\}", src), (
+        "service.error should be rendered as a JSX child expression"
+    )
+
+
+def test_panel_error_rendering_uses_no_dangerously_set_inner_html():
+    """The error field is operator-readable text from a probe; same
+    untrusted-input contract as MCP-clients last_error. React's
+    default escaping handles defanging; this guard catches a
+    future edit that switches to dangerouslySetInnerHTML."""
+    code = _strip_ts_comments(_PANEL_PATH.read_text())
+    assert "dangerouslySetInnerHTML" not in code, (
+        "HeartbeatPanel.tsx must not use dangerouslySetInnerHTML — "
+        "error field is untrusted probe output"
+    )
+
+
+# ---- 5. cache_warming banner -------------------------------------
+
+
+def test_panel_renders_cache_warming_banner():
+    """KR-FEAT-HEARTBEAT ST2 cache_warming flag: render a "Probes
+    warming up…" banner so the operator sees the warming context
+    instead of misreading an empty/sparse list as an outage."""
+    src = _PANEL_PATH.read_text()
+    assert "data.cache_warming" in src, (
+        "HeartbeatPanel.tsx must branch on data.cache_warming"
+    )
+    assert "warming" in src.lower(), (
+        "HeartbeatPanel.tsx should render a warming-state affordance"
+    )
+
+
+def test_dashboard_card_suppresses_outage_tone_when_warming():
+    """The dashboard card's headline tone must NOT go destructive
+    when cache_warming is true — an empty heartbeat list during
+    daemon cold-start isn't a real outage. Source-pin: the
+    headlineClass derivation branches on data.cache_warming."""
+    src = _DASHBOARD_PATH.read_text()
+    # Crude but effective: the cache_warming branch must appear in
+    # HeartbeatCardBody's logic, and it must influence the headline
+    # class (text-muted-foreground), not just be a side affordance.
+    hb_body_idx = src.find("function HeartbeatCardBody")
+    next_fn_idx = src.find("\nfunction ", hb_body_idx + 1)
+    body_slice = src[hb_body_idx:next_fn_idx]
+    assert "data.cache_warming" in body_slice, (
+        "HeartbeatCardBody must read data.cache_warming"
+    )
+    assert "text-muted-foreground" in body_slice, (
+        "HeartbeatCardBody should mute the headline tone during "
+        "warming (no false outage signal)"
+    )
diff --git a/web/src/pages/DashboardPage.tsx b/web/src/pages/DashboardPage.tsx
index c95a304d8426..baede927744e 100644
--- a/web/src/pages/DashboardPage.tsx
+++ b/web/src/pages/DashboardPage.tsx
@@ -529,27 +529,32 @@ function truncateMiddle(value: string, head: number): string {
 }
 
 function HeartbeatCardBody({ data }: { data: HeartbeatServicesResponse }) {
-  // Aggregate per-status counts. "5 services / 1 degraded / 0 unhealthy"
-  // (the spec §3(c) example) matches what the panel itself shows in its
-  // summary strip, kept consistent so the dashboard card + panel agree.
+  // Aggregate per-status counts. Matches the panel's summary strip
+  // so the dashboard card + panel agree. KR-FEAT-HEARTBEAT ST2
+  // added the "unknown" arm (probe pending; cold-start, auth-missing,
+  // or in-flight) — it's bucketed separately so the operator can
+  // distinguish "scheduler not done yet" from "service down".
   const total = data.services.length;
   const counts: Record = {
     healthy: 0,
     degraded: 0,
     unhealthy: 0,
+    unknown: 0,
   };
   for (const s of data.services) counts[s.status]++;
   // Worst-status tone drives the headline number colour: unhealthy >
-  // degraded > healthy. operator scans the dashboard for "is anything
-  // wrong" and this surfaces it without making them squint at chips.
+  // degraded > healthy. "unknown" is intentionally NOT a worst-
+  // status driver (it's pending, not failing) — cache_warming
+  // suppresses any pseudo-outage signal when the daemon just booted.
   const worst =
     counts.unhealthy > 0
       ? "unhealthy"
       : counts.degraded > 0
         ? "degraded"
         : "healthy";
-  const headlineClass =
-    worst === "unhealthy"
+  const headlineClass = data.cache_warming
+    ? "text-muted-foreground"
+    : worst === "unhealthy"
       ? "text-destructive"
       : worst === "degraded"
         ? "text-warning"
@@ -560,6 +565,7 @@ function HeartbeatCardBody({ data }: { data: HeartbeatServicesResponse }) {
         {total}
         
           service{total === 1 ? "" : "s"}
+          {data.cache_warming ? " · warming" : ""}
         
       
       
@@ -572,6 +578,9 @@ function HeartbeatCardBody({ data }: { data: HeartbeatServicesResponse }) { {counts.unhealthy > 0 && ( {counts.unhealthy} unhealthy )} + {counts.unknown > 0 && ( + {counts.unknown} pending + )}
); diff --git a/web/src/pages/HeartbeatPanel.tsx b/web/src/pages/HeartbeatPanel.tsx index 0198d2a020d8..af035a80fbed 100644 --- a/web/src/pages/HeartbeatPanel.tsx +++ b/web/src/pages/HeartbeatPanel.tsx @@ -6,8 +6,10 @@ import { CheckCircle2, ChevronDown, ChevronRight, + CircleDashed, Cloud, HelpCircle, + Hourglass, RefreshCw, } from "lucide-react"; import { Badge } from "@nous-research/ui/ui/components/badge"; @@ -24,10 +26,24 @@ import type { HeartbeatStatus, } from "@/lib/api"; -const STATUS_TONE: Record = { +const STATUS_TONE: Record< + HeartbeatStatus, + "success" | "warning" | "destructive" | "outline" +> = { healthy: "success", degraded: "warning", unhealthy: "destructive", + // KR-FEAT-HEARTBEAT ST2: "unknown" = probe hasn't completed a + // roundtrip yet (cold-start, auth-missing, or in-flight). + // Muted outline so it reads as "pending" not "broken". + unknown: "outline", +}; + +const STATUS_LABEL: Record = { + healthy: "healthy", + degraded: "degraded", + unhealthy: "unhealthy", + unknown: "probe pending", }; function StatusIcon({ status }: { status: HeartbeatStatus }) { @@ -38,18 +54,20 @@ function StatusIcon({ status }: { status: HeartbeatStatus }) { return ; case "unhealthy": return ; + case "unknown": + return ; } } -function formatTimestamp(iso: string): string { +function formatTimestamp(iso: string | null): string { if (!iso) return "—"; const d = new Date(iso); if (Number.isNaN(d.getTime())) return iso; return d.toLocaleString(); } -function formatRelative(iso: string): string { - if (!iso) return ""; +function formatRelative(iso: string | null): string { + if (!iso) return "never checked"; const d = new Date(iso); if (Number.isNaN(d.getTime())) return ""; const deltaMs = d.getTime() - Date.now(); @@ -110,9 +128,13 @@ function ServiceRow({ service, expanded, onToggle }: ServiceRowProps) { {service.name} - {service.status} + + {STATUS_LABEL[service.status]} + - {service.latency_ms} ms · last checked{" "} + {service.latency_ms !== null + ? `${service.latency_ms} ms · ` + : "— · "} {formatRelative(service.last_check_at)} @@ -136,8 +158,24 @@ function ServiceRow({ service, expanded, onToggle }: ServiceRowProps) { )}
- last_check_at: {formatTimestamp(service.last_check_at)} + last_check_at:{" "} + {formatTimestamp(service.last_check_at)}
+ {/* KR-FEAT-HEARTBEAT ST2 error field. Plain-text + rendering (React default escaping) — same + MCP-clients pattern as #117. Sanitized at the + backend (per api.ts type comment) but FE still + must not interpret as HTML. */} + {service.error !== null && ( +
+ + error + +
+                  {service.error}
+                
+
+ )} )} @@ -198,6 +236,7 @@ export default function HeartbeatPanel() { healthy: data.services.filter((s) => s.status === "healthy").length, degraded: data.services.filter((s) => s.status === "degraded").length, unhealthy: data.services.filter((s) => s.status === "unhealthy").length, + unknown: data.services.filter((s) => s.status === "unknown").length, } : null; @@ -271,6 +310,12 @@ export default function HeartbeatPanel() { {counts.unhealthy} unhealthy + {counts.unknown > 0 && ( + + + {counts.unknown} probe pending + + )} )} @@ -280,6 +325,26 @@ export default function HeartbeatPanel() { + {/* KR-FEAT-HEARTBEAT ST2: cache_warming banner. The + daemon just started and the first probe cycle hasn't + completed — the empty/sparse services list isn't a + "real" outage signal. */} + {data.cache_warming && ( + + + +
+
Probes warming up…
+
+ Heartbeat scheduler started recently; the first + probe cycle hasn't completed yet. Service health + will populate once probes return. +
+
+
+
+ )} + {/* ── Services list ──────────────────────────────────── */} {data.services.length === 0 ? (