feat(plans): per-plan health-score badge (closes #376 scope)

frontend/src/__tests__/plans.test.ts

@@ -3070,4 +3070,183 @@ describe('Plans Module', () => {
       expect(paymentSelect.value).toBe('all-upfront');
     });
   });
+
+  // Issue #340 follow-up (PR #376): per-plan health-score badge.
+  describe('plan health-score badge', () => {
+    // Mirror of the band thresholds in plans.ts. Declared here rather than
+    // imported because plans.ts does not export them; if they diverge the
+    // boundary cases below fail, which is the point.
+    const HEALTH_SCORE_GOOD_BOUNDARY = 80;
+    const HEALTH_SCORE_WARN_BOUNDARY = 50;
+
+    const basePlan = {
+      id: 'plan-1',
+      name: 'Test Plan',
+      enabled: true,
+      auto_purchase: true,
+      services: {
+        ec2: { provider: 'aws', service: 'ec2', enabled: true, term: 1, payment: 'all-upfront', coverage: 80 }
+      },
+      ramp_schedule: { type: 'immediate', percent_per_step: 100, step_interval_days: 0 }
+    };
+
+    test('renders a green badge for a healthy score (>= 80)', async () => {
+      (api.getPlans as jest.Mock).mockResolvedValue({
+        plans: [{ ...basePlan, health_score: 90, health_factors: [] }]
+      });
+      (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] });
+
+      await loadPlans();
+
+      const list = document.getElementById('plans-list');
+      expect(list?.innerHTML).toContain('badge-success');
+      expect(list?.innerHTML).toContain('Health: 90');
+    });
+
+    test('renders an amber badge for a medium score (50-79)', async () => {
+      (api.getPlans as jest.Mock).mockResolvedValue({
+        plans: [{
+          ...basePlan,
+          health_score: 65,
+          health_factors: [{ code: 'behind_schedule', penalty: 20, note: 'on step 1, expected step 3 by now' }]
+        }]
+      });
+      (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] });
+
+      await loadPlans();
+
+      const list = document.getElementById('plans-list');
+      expect(list?.innerHTML).toContain('badge-warning');
+      expect(list?.innerHTML).toContain('Health: 65');
+    });
+
+    test('renders a red badge for a poor score (< 50)', async () => {
+      (api.getPlans as jest.Mock).mockResolvedValue({
+        plans: [{
+          ...basePlan,
+          health_score: 30,
+          health_factors: [
+            { code: 'overdue', penalty: 30, note: 'next purchase date has passed' },
+            { code: 'failed_executions', penalty: 40, note: '4 failed execution(s)' }
+          ]
+        }]
+      });
+      (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] });
+
+      await loadPlans();
+
+      const list = document.getElementById('plans-list');
+      expect(list?.innerHTML).toContain('badge-danger');
+      expect(list?.innerHTML).toContain('Health: 30');
+    });
+
+    test('omits the badge entirely when health_score is absent (pre-feature API response)', async () => {
+      (api.getPlans as jest.Mock).mockResolvedValue({ plans: [{ ...basePlan }] });
+      (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] });
+
+      await loadPlans();
+
+      const list = document.getElementById('plans-list');
+      expect(list?.innerHTML).not.toContain('Health:');
+    });
+
+    // Regression guard for the fabricated-score bug: when the backend can't
+    // compute a score it sends null, and the UI must say so. Pre-fix the
+    // backend substituted 100 on a store failure, painting every plan with a
+    // confident green "healthy" badge built from data it never read.
+    test('renders an explicit unknown badge when health_score is null', async () => {
+      (api.getPlans as jest.Mock).mockResolvedValue({
+        plans: [{ ...basePlan, health_score: null }]
+      });
+      (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] });
+
+      await loadPlans();
+
+      const list = document.getElementById('plans-list');
+      expect(list?.innerHTML).toContain('Health: unknown');
+      expect(list?.innerHTML).toContain('badge-muted');
+      // Critically, it must NOT be dressed up as a healthy score.
+      expect(list?.innerHTML).not.toContain('badge-success');
+      expect(list?.innerHTML).not.toContain('Health: 100');
+    });
+
+    // Band boundaries, not just mid-band samples: with only 90/65/30 the
+    // suite cannot tell `>=` from `>`, so flipping either comparison would
+    // silently reclassify every plan sitting exactly on a threshold.
+    test.each([
+      [HEALTH_SCORE_GOOD_BOUNDARY, 'badge-success'],
+      [HEALTH_SCORE_GOOD_BOUNDARY - 1, 'badge-warning'],
+      [HEALTH_SCORE_WARN_BOUNDARY, 'badge-warning'],
+      [HEALTH_SCORE_WARN_BOUNDARY - 1, 'badge-danger'],
+    ])('score %i sits in the %s band', async (score, expectedClass) => {
+      (api.getPlans as jest.Mock).mockResolvedValue({
+        plans: [{ ...basePlan, health_score: score, health_factors: [] }]
+      });
+      (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] });
+
+      await loadPlans();
+
+      const list = document.getElementById('plans-list');
+      expect(list?.innerHTML).toContain(expectedClass);
+      expect(list?.innerHTML).toContain(`Health: ${score}`);
+    });
+
+    // A score outside the 0-100 the API contracts to send is no more
+    // trustworthy than a NaN. Banding it would paint -1 red and 101 green as
+    // though both had been measured.
+    test.each([-1, 101, 1000])('renders unknown for out-of-range score %i', async (score) => {
+      (api.getPlans as jest.Mock).mockResolvedValue({
+        plans: [{ ...basePlan, health_score: score }]
+      });
+      (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] });
+
+      await loadPlans();
+
+      const list = document.getElementById('plans-list');
+      expect(list?.innerHTML).toContain('Health: unknown');
+      expect(list?.innerHTML).toContain('badge-muted');
+      expect(list?.innerHTML).not.toContain(`Health: ${score}`);
+    });
+
+    // A garbled score off the wire is an uncomputable score, so it must
+    // reach the same "unknown" badge rather than silently vanishing (which
+    // reads as "this deploy predates the feature").
+    test('renders unknown rather than dropping the badge for a non-finite score', async () => {
+      (api.getPlans as jest.Mock).mockResolvedValue({
+        plans: [{ ...basePlan, health_score: NaN }]
+      });
+      (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] });
+
+      await loadPlans();
+
+      const list = document.getElementById('plans-list');
+      expect(list?.innerHTML).toContain('Health: unknown');
+      expect(list?.innerHTML).not.toContain('Health: NaN');
+    });
+
+    test('escapes health factor notes in the tooltip (XSS regression)', async () => {
+      const maliciousNote = '"><img src=x onerror=alert(1)>';
+      (api.getPlans as jest.Mock).mockResolvedValue({
+        plans: [{
+          ...basePlan,
+          health_score: 40,
+          health_factors: [{ code: 'stalled', penalty: 15, note: maliciousNote }]
+        }]
+      });
+      (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] });
+
+      await loadPlans();
+
+      const list = document.getElementById('plans-list');
+      // Pre-fix, the unescaped `"` would close the title attribute early and
+      // the rest would parse as a live <img onerror> element. Post-fix the
+      // quote is entity-encoded so the whole malicious string stays inert
+      // text inside the title attribute -- assert on the parsed DOM (not the
+      // raw HTML string, since HTML serializers are free to leave `<`/`>`
+      // un-re-escaped inside an already-safe attribute value).
+      expect(list?.querySelector('img')).toBeNull();
+      const badge = list?.querySelector('.badge-danger[title]');
+      expect(badge?.getAttribute('title')).toContain(maliciousNote);
+    });
+  });
 });

frontend/src/api/index.ts

@@ -17,6 +17,7 @@ export type {
   Recommendation,
   RecommendationFilters,
   Plan,
+  PlanHealthFactor,
   CreatePlanRequest,
   PurchaseHistory,
   HistoryFilters,

frontend/src/api/types.ts

@@ -227,6 +227,24 @@ export interface Plan {
   // Such plans are surfaced under an "Unassigned" section in the Plans UI
   // so operators can find and re-scope them (issue #973).
   unassigned?: boolean;
+  // Health score/factors are computed at read time on GET /plans (never
+  // persisted) by computePlanHealth server-side (issue #340 follow-up).
+  // Optional so an older cached response served during a partial deploy
+  // still renders the card cleanly -- the badge just doesn't appear.
+  // Explicitly null when the backend could not compute a score; clients
+  // must render that as "unknown" rather than substituting a default.
+  health_score?: number | null;
+  health_factors?: PlanHealthFactor[];
+}
+
+// PlanHealthFactor is one penalty subtracted from a plan's health score
+// (issue #340 follow-up). `code` is a fixed vocabulary from the backend
+// (see internal/api/plan_health.go); `note` is a human-readable reason
+// suitable for the health-badge tooltip.
+export interface PlanHealthFactor {
+  code: string;
+  penalty: number;
+  note: string;
 }
 
 export interface CreatePlanRequest {

frontend/src/plans.ts

@@ -861,6 +861,78 @@ interface BackendPlan {
   // (issue #973). The backend sets this flag when an account filter is
   // active so the frontend can bucket them under an "Unassigned" section.
   unassigned?: boolean;
+  // Health score/factors are computed at read time on GET /plans (never
+  // persisted) -- see computePlanHealth in internal/api/plan_health.go
+  // (issue #340 follow-up). Optional so an older cached response served
+  // during a partial deploy still renders the card cleanly; explicitly
+  // null when the backend could not compute the score.
+  health_score?: number | null;
+  health_factors?: api.PlanHealthFactor[];
+}
+
+// Score bands for the per-plan health badge (issue #340 follow-up). Green
+// >= 80 (healthy), amber 50-79 (needs attention), red < 50 (action needed).
+// Reuses the existing status-badge palette (badge-success/badge-warning/
+// badge-danger) already used elsewhere on this card rather than introducing
+// new CSS.
+const HEALTH_SCORE_GOOD_THRESHOLD = 80;
+const HEALTH_SCORE_WARN_THRESHOLD = 50;
+
+// Inclusive bounds of the score the API contracts to send (openapi.yaml
+// PlanWithHealth.health_score: minimum 0, maximum 100). Anything outside is
+// treated as unusable rather than banded.
+const HEALTH_SCORE_MIN = 0;
+const HEALTH_SCORE_MAX = 100;
+
+function healthBadgeClass(score: number): string {
+  if (score >= HEALTH_SCORE_GOOD_THRESHOLD) return 'badge-success';
+  if (score >= HEALTH_SCORE_WARN_THRESHOLD) return 'badge-warning';
+  return 'badge-danger';
+}
+
+// healthBadgeHtml renders the per-plan health-score badge, distinguishing
+// three states the API can report:
+//
+//   - field absent: an older API response served during a partial deploy.
+//     Render nothing so the card still looks intentional.
+//   - field null: the backend could not compute the score (execution counts
+//     unavailable). Render an explicit "unknown" badge -- never a stand-in
+//     number, which an operator could not tell apart from a measured score.
+//   - a number within the contract's 0-100 range: the score, banded into the
+//     green/amber/red palette.
+//   - anything else (NaN, Infinity, out of range, a non-number off the wire):
+//     also "unknown". A score outside 0-100 violates the range the API
+//     declares, so it is no more trustworthy than a NaN, and banding it would
+//     paint -1 red and 101 green as though both were measured. Silently
+//     dropping the badge instead would read as "this deploy predates the
+//     feature".
+//
+// The tooltip enumerates every penalty factor so a bad score is actionable
+// instead of opaque; every factor note is escaped since it renders inside an
+// HTML attribute (title) via innerHTML (feedback_innerhtml_xss: all
+// API-sourced values must be escaped here even though the backend generates
+// these notes itself).
+function healthBadgeHtml(plan: BackendPlan): string {
+  if (plan.health_score === undefined) return '';
+  // The typeof/isFinite check is defensive: the type says number, but this
+  // value comes off the wire and is interpolated into innerHTML below.
+  if (plan.health_score === null
+      || typeof plan.health_score !== 'number'
+      || !Number.isFinite(plan.health_score)
+      || plan.health_score < HEALTH_SCORE_MIN
+      || plan.health_score > HEALTH_SCORE_MAX) {
+    // Neutral wording on purpose: null means the backend could not read the
+    // execution history, while a non-finite value means the number itself
+    // arrived unusable. Naming only the first would send an operator
+    // chasing a database problem that may not exist.
+    return `<span class="status-badge badge-muted" title="${escapeHtmlAttr('Plan health is unavailable: this plan\'s score could not be determined.')}">Health: unknown</span>`;
+  }
+  const factors = plan.health_factors || [];
+  const factorLines = factors.length > 0
+    ? factors.map(f => `-${f.penalty}: ${f.note}`)
+    : ['No issues detected'];
+  const tooltip = [`Plan health: ${plan.health_score}/100`, ...factorLines].join('\n');
+  return `<span class="status-badge ${healthBadgeClass(plan.health_score)}" title="${escapeHtmlAttr(tooltip)}">Health: ${plan.health_score}</span>`;
 }
 
 // Pretty label for a service slug used inside the plan card.
@@ -992,6 +1064,7 @@ function renderPlanCard(plan: BackendPlan, canManagePlan: boolean, canDeletePlan
         <h3>${escapeHtml(plan.name)}</h3>
         <div class="plan-status">
           <span class="status-badge ${status.class}">${status.label}</span>
+          ${healthBadgeHtml(plan)}
           ${overdueBadge}
           ${canManagePlan && !isUnassigned ? `
           <label class="toggle-label">

frontend/src/types.ts

@@ -160,6 +160,11 @@ export interface LocalPlan {
   next_execution_date?: string;
   custom_step_percent?: number;
   custom_interval_days?: number;
+  // Read-time-only health-score badge data (issue #340 follow-up). Optional
+  // so a response from before this feature shipped still renders cleanly;
+  // explicitly null when the backend could not compute a score.
+  health_score?: number | null;
+  health_factors?: api.PlanHealthFactor[];
 }
 
 export interface SavePlanData {

internal/analytics/collector_test.go

@@ -188,6 +188,10 @@ func (m *mockConfigStore) GetExecutionsByStatuses(ctx context.Context, statuses
 	return nil, nil
 }
 
+func (m *mockConfigStore) CountExecutionsByPlanAndStatus(ctx context.Context, statuses []string, since time.Time) (map[string]config.ExecutionStatusCounts, error) {
+	return nil, nil
+}
+
 func (m *mockConfigStore) GetPlannedExecutions(ctx context.Context, statuses []string, limit int) ([]config.PurchaseExecution, error) {
 	return nil, nil
 }

internal/api/handler_plans.go

@@ -45,7 +45,46 @@ func (h *Handler) listPlans(ctx context.Context, req *events.LambdaFunctionURLRe
 		}
 	}
 
-	return &PlansResponse{Plans: plans}, nil
+	return &PlansResponse{Plans: h.attachPlanHealth(ctx, plans, now)}, nil
+}
+
+// attachPlanHealth computes the per-plan health-score badge (issue #340
+// follow-up) for every plan in the list, from exact per-plan execution
+// counts over the trailing planHealthLookbackDays
+// (config.StoreInterface.CountExecutionsByPlanAndStatus, aggregated in SQL)
+// rather than from a capped page of execution rows, which would understate
+// plans whose executions fall outside the newest page.
+//
+// If the counts fetch fails, every plan's HealthScore is left nil, which
+// serializes as `"health_score": null` and renders as an explicit "unknown"
+// badge. A score is a number operators make purchasing decisions from, so
+// an uncomputable one must stay absent: substituting a default (100 reads
+// as "healthy", 0 as "broken") would be a fabricated number indistinguishable
+// from a real one. The request itself still succeeds -- the Plans page is
+// not primarily about execution history, so a 500 here would be a worse
+// trade than a visibly-unknown badge.
+func (h *Handler) attachPlanHealth(ctx context.Context, plans []config.PurchasePlan, now time.Time) []PlanWithHealth {
+	result := make([]PlanWithHealth, len(plans))
+	for i := range plans {
+		result[i] = PlanWithHealth{PurchasePlan: plans[i]}
+	}
+	if len(plans) == 0 {
+		return result
+	}
+
+	since := now.AddDate(0, 0, -planHealthLookbackDays)
+	countsByPlan, err := h.config.CountExecutionsByPlanAndStatus(ctx, planHealthExecutionStatuses, since)
+	if err != nil {
+		logging.Warnf("listPlans: CountExecutionsByPlanAndStatus failed, plan health reported as unknown: %v", err)
+		return result
+	}
+
+	for i := range plans {
+		score, factors := computePlanHealth(plans[i], now, countsByPlan[plans[i].ID])
+		result[i].HealthScore = &score
+		result[i].HealthFactors = factors
+	}
+	return result
 }
 
 // calculateNextExecutionDate calculates the next execution date for a plan.