diff --git a/frontend/src/__tests__/plans.test.ts b/frontend/src/__tests__/plans.test.ts
index d35465c18..097679e4c 100644
--- a/frontend/src/__tests__/plans.test.ts
+++ b/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 = '"> ';
+ (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 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);
+ });
+ });
});
diff --git a/frontend/src/api/index.ts b/frontend/src/api/index.ts
index e59ac15d9..8b5c3e22f 100644
--- a/frontend/src/api/index.ts
+++ b/frontend/src/api/index.ts
@@ -17,6 +17,7 @@ export type {
Recommendation,
RecommendationFilters,
Plan,
+ PlanHealthFactor,
CreatePlanRequest,
PurchaseHistory,
HistoryFilters,
diff --git a/frontend/src/api/types.ts b/frontend/src/api/types.ts
index ec7dd3786..84a69737e 100644
--- a/frontend/src/api/types.ts
+++ b/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 {
diff --git a/frontend/src/plans.ts b/frontend/src/plans.ts
index 369f99078..ae02d4d3a 100644
--- a/frontend/src/plans.ts
+++ b/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 `Health: unknown `;
+ }
+ 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 `Health: ${plan.health_score} `;
}
// Pretty label for a service slug used inside the plan card.
@@ -992,6 +1064,7 @@ function renderPlanCard(plan: BackendPlan, canManagePlan: boolean, canDeletePlan
${escapeHtml(plan.name)}
${status.label}
+ ${healthBadgeHtml(plan)}
${overdueBadge}
${canManagePlan && !isUnassigned ? `
diff --git a/frontend/src/types.ts b/frontend/src/types.ts
index e9ff34734..164e7e1d2 100644
--- a/frontend/src/types.ts
+++ b/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 {
diff --git a/internal/analytics/collector_test.go b/internal/analytics/collector_test.go
index a07bb0607..6f3742ddb 100644
--- a/internal/analytics/collector_test.go
+++ b/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
}
diff --git a/internal/api/handler_plans.go b/internal/api/handler_plans.go
index e175d8471..77e954d5e 100644
--- a/internal/api/handler_plans.go
+++ b/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.
diff --git a/internal/api/handler_plans_test.go b/internal/api/handler_plans_test.go
index 10c383ee7..6bff9b0b2 100644
--- a/internal/api/handler_plans_test.go
+++ b/internal/api/handler_plans_test.go
@@ -2,6 +2,7 @@ package api
import (
"context"
+ "encoding/json"
"errors"
"fmt"
"testing"
@@ -33,6 +34,10 @@ func TestHandler_listPlans(t *testing.T) {
mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil)
mockAuth.grantAdmin()
mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return(plans, nil)
+ // listPlans now also counts each plan's failed/canceled executions to
+ // compute its health-score badge (issue #340 follow-up).
+ mockStore.On("CountExecutionsByPlanAndStatus", ctx, planHealthExecutionStatuses, mock.AnythingOfType("time.Time")).
+ Return(map[string]config.ExecutionStatusCounts{}, nil)
handler := &Handler{config: mockStore, auth: mockAuth}
@@ -45,6 +50,12 @@ func TestHandler_listPlans(t *testing.T) {
require.NoError(t, err)
assert.Len(t, result.Plans, 2)
+ // Neither seeded plan has ramp-schedule or execution data that would
+ // trigger a penalty, so both score a perfect 100.
+ require.NotNil(t, result.Plans[0].HealthScore)
+ require.NotNil(t, result.Plans[1].HealthScore)
+ assert.Equal(t, 100, *result.Plans[0].HealthScore)
+ assert.Equal(t, 100, *result.Plans[1].HealthScore)
}
func TestHandler_listPlans_AccountIDsFilter(t *testing.T) {
@@ -67,6 +78,8 @@ func TestHandler_listPlans_AccountIDsFilter(t *testing.T) {
mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil)
mockAuth.grantAdmin()
mockStore.On("ListPurchasePlans", ctx, expectedFilter).Return(plans, nil)
+ mockStore.On("CountExecutionsByPlanAndStatus", ctx, planHealthExecutionStatuses, mock.AnythingOfType("time.Time")).
+ Return(map[string]config.ExecutionStatusCounts{}, nil)
handler := &Handler{config: mockStore, auth: mockAuth}
@@ -83,6 +96,195 @@ func TestHandler_listPlans_AccountIDsFilter(t *testing.T) {
assert.Equal(t, "Account Plan", result.Plans[0].Name)
}
+// TestHandler_listPlans_HealthScoreReflectsFailedExecutions is the
+// end-to-end wiring check for the issue #340 health-score badge: a plan
+// with failed executions in the store must come back from listPlans with a
+// penalized HealthScore and the matching HealthFactors entry, and a
+// failed execution belonging to a DIFFERENT plan must not bleed into this
+// plan's score.
+func TestHandler_listPlans_HealthScoreReflectsFailedExecutions(t *testing.T) {
+ ctx := context.Background()
+ mockStore := new(MockConfigStore)
+ mockAuth := new(MockAuthService)
+
+ adminSession := &Session{UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", Email: "admin@example.com"}
+ mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil)
+ mockAuth.grantAdmin()
+
+ troubledPlanID := "11111111-1111-1111-1111-111111111111"
+ otherPlanID := "22222222-2222-2222-2222-222222222222"
+ plans := []config.PurchasePlan{
+ {ID: troubledPlanID, Name: "Troubled Plan", Enabled: true},
+ {ID: otherPlanID, Name: "Other Plan", Enabled: true},
+ }
+ mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return(plans, nil)
+ mockStore.On("CountExecutionsByPlanAndStatus", ctx, planHealthExecutionStatuses, mock.AnythingOfType("time.Time")).
+ Return(map[string]config.ExecutionStatusCounts{
+ troubledPlanID: {"failed": 2},
+ otherPlanID: {config.StatusCanceled: 1},
+ }, nil)
+
+ handler := &Handler{config: mockStore, auth: mockAuth}
+ req := &events.LambdaFunctionURLRequest{Headers: map[string]string{"Authorization": "Bearer admin-token"}}
+
+ result, err := handler.listPlans(ctx, req, map[string]string{})
+ require.NoError(t, err)
+ require.Len(t, result.Plans, 2)
+
+ byID := make(map[string]PlanWithHealth, 2)
+ for _, p := range result.Plans {
+ byID[p.ID] = p
+ }
+
+ troubled := byID[troubledPlanID]
+ require.NotNil(t, troubled.HealthScore)
+ assert.Equal(t, 100-2*penaltyPerFailedExec, *troubled.HealthScore)
+ require.Len(t, troubled.HealthFactors, 1)
+ assert.Equal(t, HealthFactorFailedExecutions, troubled.HealthFactors[0].Code)
+
+ other := byID[otherPlanID]
+ require.NotNil(t, other.HealthScore)
+ assert.Equal(t, 100-penaltyPerCanceledExec, *other.HealthScore)
+ require.Len(t, other.HealthFactors, 1)
+ assert.Equal(t, HealthFactorCanceledExecutions, other.HealthFactors[0].Code)
+}
+
+// TestHandler_listPlans_HealthScoreUnknownOnCountsFetchError pins the
+// no-fabricated-score contract: when the execution counts can't be read the
+// request still succeeds (the Plans page is not primarily about execution
+// history), but HealthScore stays nil so it serializes as null and the UI
+// renders an explicit "unknown" badge. Substituting any concrete default --
+// 100 reads as healthy, 0 as broken -- would put a number in front of an
+// operator that is indistinguishable from a measured one.
+func TestHandler_listPlans_HealthScoreUnknownOnCountsFetchError(t *testing.T) {
+ ctx := context.Background()
+ mockStore := new(MockConfigStore)
+ mockAuth := new(MockAuthService)
+ t.Cleanup(func() { mockStore.AssertExpectations(t) })
+
+ adminSession := &Session{UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", Email: "admin@example.com"}
+ mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil)
+ mockAuth.grantAdmin()
+
+ plans := []config.PurchasePlan{
+ {ID: "11111111-1111-1111-1111-111111111111", Name: "Some Plan", Enabled: true},
+ }
+ mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return(plans, nil)
+ mockStore.On("CountExecutionsByPlanAndStatus", ctx, planHealthExecutionStatuses, mock.AnythingOfType("time.Time")).
+ Return(nil, errors.New("db unavailable"))
+
+ handler := &Handler{config: mockStore, auth: mockAuth}
+ req := &events.LambdaFunctionURLRequest{Headers: map[string]string{"Authorization": "Bearer admin-token"}}
+
+ result, err := handler.listPlans(ctx, req, map[string]string{})
+ require.NoError(t, err)
+ require.Len(t, result.Plans, 1)
+ assert.Nil(t, result.Plans[0].HealthScore)
+ assert.Empty(t, result.Plans[0].HealthFactors)
+
+ // The wire form must carry an explicit null, not omit the key: the UI
+ // distinguishes null ("unknown") from absent ("older API response").
+ encoded, err := json.Marshal(result.Plans[0])
+ require.NoError(t, err)
+ assert.Contains(t, string(encoded), `"health_score":null`)
+}
+
+// TestHandler_listPlans_HealthUsesExactCountsNotTruncatedPage guards the
+// truncation bug: health scoring must read exact per-plan counts aggregated
+// in SQL, never count rows out of GetExecutionsByStatuses, whose DESC+LIMIT
+// page is capped across ALL plans. With enough execution rows system-wide, a
+// plan's own failures fall outside that page and it renders a confident
+// "healthy" badge while actually being unhealthy.
+func TestHandler_listPlans_HealthUsesExactCountsNotTruncatedPage(t *testing.T) {
+ ctx := context.Background()
+ mockStore := new(MockConfigStore)
+ mockAuth := new(MockAuthService)
+ t.Cleanup(func() { mockStore.AssertExpectations(t) })
+
+ adminSession := &Session{UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", Email: "admin@example.com"}
+ mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil)
+ mockAuth.grantAdmin()
+
+ planID := "11111111-1111-1111-1111-111111111111"
+ mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).
+ Return([]config.PurchasePlan{{ID: planID, Name: "Old Failures", Enabled: true}}, nil)
+ mockStore.On("CountExecutionsByPlanAndStatus", ctx, planHealthExecutionStatuses, mock.AnythingOfType("time.Time")).
+ Return(map[string]config.ExecutionStatusCounts{planID: {"failed": 3}}, nil)
+
+ handler := &Handler{config: mockStore, auth: mockAuth}
+ req := &events.LambdaFunctionURLRequest{Headers: map[string]string{"Authorization": "Bearer admin-token"}}
+
+ result, err := handler.listPlans(ctx, req, map[string]string{})
+ require.NoError(t, err)
+ require.Len(t, result.Plans, 1)
+ require.NotNil(t, result.Plans[0].HealthScore)
+ assert.Equal(t, 100-3*penaltyPerFailedExec, *result.Plans[0].HealthScore)
+
+ // The capped page must not be consulted at all: reintroducing it would
+ // silently reinstate the truncation.
+ mockStore.AssertNotCalled(t, "GetExecutionsByStatuses", mock.Anything, mock.Anything, mock.Anything)
+}
+
+// TestHandler_listPlans_HealthCountsUseTheLookbackWindow pins that the
+// documented window actually reaches the store. Without a bound the counts
+// cover whatever history happened to survive cleanup, and since
+// CleanupOldExecutions purges terminal rows but never "failed" ones, a plan
+// that failed once years ago would stay penalized forever.
+func TestHandler_listPlans_HealthCountsUseTheLookbackWindow(t *testing.T) {
+ ctx := context.Background()
+ mockStore := new(MockConfigStore)
+ mockAuth := new(MockAuthService)
+ t.Cleanup(func() { mockStore.AssertExpectations(t) })
+
+ adminSession := &Session{UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", Email: "admin@example.com"}
+ mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil)
+ mockAuth.grantAdmin()
+ mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).
+ Return([]config.PurchasePlan{{ID: "11111111-1111-1111-1111-111111111111", Enabled: true}}, nil)
+
+ var gotSince time.Time
+ mockStore.On("CountExecutionsByPlanAndStatus", ctx, planHealthExecutionStatuses, mock.MatchedBy(func(since time.Time) bool {
+ gotSince = since
+ return true
+ })).Return(map[string]config.ExecutionStatusCounts{}, nil)
+
+ handler := &Handler{config: mockStore, auth: mockAuth}
+ req := &events.LambdaFunctionURLRequest{Headers: map[string]string{"Authorization": "Bearer admin-token"}}
+
+ before := time.Now()
+ _, err := handler.listPlans(ctx, req, map[string]string{})
+ require.NoError(t, err)
+
+ // The window opens planHealthLookbackDays before the request, give or
+ // take the wall-clock time the handler itself took.
+ expected := before.AddDate(0, 0, -planHealthLookbackDays)
+ assert.WithinDuration(t, expected, gotSince, time.Minute)
+ assert.True(t, gotSince.Before(before), "since must be in the past, got %v", gotSince)
+}
+
+// TestHandler_listPlans_EmptyListSkipsCountsQuery: with no plans to score
+// there is nothing to attach health to, so the aggregate query must not run.
+func TestHandler_listPlans_EmptyListSkipsCountsQuery(t *testing.T) {
+ ctx := context.Background()
+ mockStore := new(MockConfigStore)
+ mockAuth := new(MockAuthService)
+ t.Cleanup(func() { mockStore.AssertExpectations(t) })
+
+ adminSession := &Session{UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", Email: "admin@example.com"}
+ mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil)
+ mockAuth.grantAdmin()
+ mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).
+ Return([]config.PurchasePlan{}, nil)
+
+ handler := &Handler{config: mockStore, auth: mockAuth}
+ req := &events.LambdaFunctionURLRequest{Headers: map[string]string{"Authorization": "Bearer admin-token"}}
+
+ result, err := handler.listPlans(ctx, req, map[string]string{})
+ require.NoError(t, err)
+ assert.Empty(t, result.Plans)
+ mockStore.AssertNotCalled(t, "CountExecutionsByPlanAndStatus", mock.Anything, mock.Anything, mock.Anything)
+}
+
func TestHandler_createPlan(t *testing.T) {
ctx := context.Background()
mockStore := new(MockConfigStore)
diff --git a/internal/api/handler_test.go b/internal/api/handler_test.go
index 52ef69675..b0d4e4793 100644
--- a/internal/api/handler_test.go
+++ b/internal/api/handler_test.go
@@ -661,6 +661,10 @@ func TestHandler_HandleRequest_ListPlans(t *testing.T) {
plans := []config.PurchasePlan{{ID: "11111111-1111-1111-1111-111111111111"}}
mockStore.On("ListPurchasePlans", mock.Anything, mock.Anything).Return(plans, nil)
+ // listPlans now also counts each plan's failed/canceled executions to
+ // compute its health-score badge (issue #340 follow-up).
+ mockStore.On("CountExecutionsByPlanAndStatus", mock.Anything, mock.Anything, mock.Anything).
+ Return(map[string]config.ExecutionStatusCounts{}, nil)
handler := &Handler{config: mockStore, auth: mockAuth, apiKey: "test-key"}
@@ -1473,6 +1477,8 @@ func TestHandler_HandleRequest_ListPlans_UnassignedFlagged(t *testing.T) {
assigned := config.PurchasePlan{ID: "11111111-1111-1111-1111-111111111111", Name: "Assigned Plan", Unassigned: false}
legacy := config.PurchasePlan{ID: "22222222-2222-2222-2222-222222222222", Name: "Legacy Plan", Unassigned: true}
mockStore.On("ListPurchasePlans", mock.Anything, mock.Anything).Return([]config.PurchasePlan{assigned, legacy}, nil)
+ mockStore.On("CountExecutionsByPlanAndStatus", mock.Anything, mock.Anything, mock.Anything).
+ Return(map[string]config.ExecutionStatusCounts{}, nil)
handler := &Handler{config: mockStore, auth: mockAuth, apiKey: "test-key"}
@@ -1501,7 +1507,7 @@ func TestHandler_HandleRequest_ListPlans_UnassignedFlagged(t *testing.T) {
require.Len(t, body.Plans, 2)
// Find plans by ID to avoid order dependence.
- plansByID := make(map[string]config.PurchasePlan, 2)
+ plansByID := make(map[string]PlanWithHealth, 2)
for _, p := range body.Plans {
plansByID[p.ID] = p
}
diff --git a/internal/api/openapi.yaml b/internal/api/openapi.yaml
index 04a9bfcee..590893123 100644
--- a/internal/api/openapi.yaml
+++ b/internal/api/openapi.yaml
@@ -2339,7 +2339,7 @@ components:
plans:
type: array
items:
- $ref: '#/components/schemas/PurchasePlan'
+ $ref: '#/components/schemas/PlanWithHealth'
PlanRequest:
type: object
@@ -2422,6 +2422,55 @@ components:
format: date-time
nullable: true
+ # PlanWithHealth wraps PurchasePlan with a read-time-only health-score
+ # badge (issue #340 follow-up): score/factors are computed from the
+ # plan's attributes and recent executions, never persisted, so they
+ # can't drift into the PurchasePlan schema above.
+ PlanWithHealth:
+ allOf:
+ - $ref: '#/components/schemas/PurchasePlan'
+ - type: object
+ properties:
+ health_score:
+ type: integer
+ nullable: true
+ minimum: 0
+ maximum: 100
+ description: >
+ 0-100 plan health score. Green >= 80, amber 50-79, red < 50.
+ Null when the score could not be computed (the plan's
+ execution counts were unavailable); clients must render that
+ as "unknown" rather than substituting a default.
+ health_factors:
+ type: array
+ items:
+ $ref: '#/components/schemas/PlanHealthFactor'
+
+ PlanHealthFactor:
+ type: object
+ description: One penalty subtracted from a plan's health score, named so a bad score is actionable instead of opaque.
+ properties:
+ code:
+ type: string
+ description: >
+ Which check produced the penalty. The two execution-based codes
+ (failed_executions, canceled_executions) count only executions
+ that entered that state within the trailing execution-retention
+ window (30 days), not over all history, so they describe the
+ plan's recent behaviour rather than its lifetime record. The
+ remaining codes are derived from the plan's current attributes
+ and ramp schedule and are not time-windowed.
+ enum: [overdue, failed_executions, canceled_executions, stalled, behind_schedule, disabled_midway]
+ penalty:
+ type: integer
+ description: Points subtracted from 100 by this factor.
+ note:
+ type: string
+ description: >
+ Human-readable reason for display in the badge tooltip. Free
+ text, and its wording may change; clients must branch on `code`
+ rather than parsing this.
+
RampSchedule:
type: object
properties:
diff --git a/internal/api/plan_health.go b/internal/api/plan_health.go
new file mode 100644
index 000000000..f08bb1683
--- /dev/null
+++ b/internal/api/plan_health.go
@@ -0,0 +1,285 @@
+// Package api provides the HTTP API handlers for the CUDly dashboard.
+package api
+
+import (
+ "fmt"
+ "time"
+
+ "github.com/LeanerCloud/CUDly/internal/config"
+)
+
+// PlanHealthFactorCode identifies a single scoring factor in a plan's
+// health-score breakdown (issue #340 follow-up). Typed so the frontend
+// tooltip renders deterministic wording per factor instead of parsing free
+// text out of Note.
+type PlanHealthFactorCode string
+
+// Health-factor codes, in the same order computePlanHealth evaluates them.
+const (
+ HealthFactorOverdue PlanHealthFactorCode = "overdue"
+ HealthFactorFailedExecutions PlanHealthFactorCode = "failed_executions"
+ HealthFactorCanceledExecutions PlanHealthFactorCode = "canceled_executions"
+ HealthFactorStalled PlanHealthFactorCode = "stalled"
+ HealthFactorBehindSchedule PlanHealthFactorCode = "behind_schedule"
+ HealthFactorDisabledMidway PlanHealthFactorCode = "disabled_midway"
+)
+
+// PlanHealthFactor is one penalty applied when computing a plan's health
+// score. Returned alongside the score so a bad score is actionable instead
+// of opaque: the frontend tooltip enumerates every factor and its penalty.
+type PlanHealthFactor struct {
+ Code PlanHealthFactorCode `json:"code"`
+ Penalty int `json:"penalty"`
+ Note string `json:"note"`
+}
+
+// Score bounds and per-factor weights for computePlanHealth. Named
+// constants rather than inline magic numbers so the scoring table in the
+// package doc / issue #340 has a single source of truth.
+const (
+ planHealthScoreMax = 100
+ planHealthScoreMin = 0
+
+ penaltyOverdue = 30
+ penaltyPerFailedExec = 10
+ penaltyPerCanceledExec = 5
+ penaltyStalled = 15
+ penaltyBehindSchedule = 20
+ penaltyDisabledMidway = 25
+
+ // maxCountedFailedExecs / maxCountedCanceledExecs cap how many rows
+ // count toward their respective penalty, so a plan with a long failure
+ // history can't score below the documented worst case for that factor
+ // (-40 / -20 respectively).
+ maxCountedFailedExecs = 4
+ maxCountedCanceledExecs = 4
+
+ // planHealthLookbackDays bounds how far back the execution factors
+ // count. It is deliberately the SAME constant the execution-retention
+ // sweep uses (config.DefaultExecutionTTLDays, applied by
+ // CleanupOldExecutions via internal/server/handler.go), because the two
+ // have to agree for the score to mean anything:
+ //
+ // That sweep deletes canceled rows past the retention horizon but never
+ // deletes failed ones. Counting over any longer window would therefore
+ // measure the two factors over different spans -- canceled over the
+ // surviving retention period, failed over all of history -- and a plan
+ // that failed a few times years ago would stay penalized forever, with
+ // no number of subsequent clean runs able to clear it. Health is a
+ // statement about the plan now, so both factors count the same recent
+ // period, the one retention guarantees is fully populated.
+ //
+ // Agreeing on the length is necessary but not sufficient: the sweep has
+ // to measure it from the same column. CountExecutionsByPlanAndStatus
+ // windows on updated_at, so CleanupOldExecutions retains canceled rows
+ // on updated_at too (its branch 2), and excludes every status counted
+ // here from its expires_at branch. If either side is moved back onto a
+ // different clock, the sweep starts deleting rows this score is still
+ // counting and a plan's score jumps overnight -- by up to 20 points for
+ // canceled, up to 40 for failed -- with nothing having changed about
+ // the plan. config.HealthScoredExecutionStatuses is the shared list
+ // that keeps the two sides from drifting.
+ //
+ // Every factor note names the window, so the number on screen is never
+ // ambiguous about what it covers.
+ planHealthLookbackDays = config.DefaultExecutionTTLDays
+)
+
+// planHealthExecutionStatuses selects the execution statuses
+// computePlanHealth needs (failed + both spellings of canceled). Passed to
+// config.StoreInterface.CountExecutionsByPlanAndStatus, which aggregates in
+// SQL: the History handler's GetExecutionsByStatuses is capped at
+// config.DefaultListLimit across ALL plans, so counting rows out of it
+// would silently understate any plan whose executions fall outside the
+// newest page and render an unhealthy plan as healthy.
+//
+// Deliberately an alias for config.HealthScoredExecutionStatuses rather than
+// a second literal list: CleanupOldExecutions must exclude exactly these
+// statuses from its expires_at reaping, so a status added to the score here
+// has to extend that exclusion in the same edit. Sharing the slice makes
+// that structural instead of a convention someone has to remember. See the
+// doc on config.HealthScoredExecutionStatuses for the failure it prevents.
+var planHealthExecutionStatuses = config.HealthScoredExecutionStatuses
+
+// computePlanHealth derives a 0-100 health score for a single purchase plan
+// from its ramp-schedule attributes and its own execution counts (the caller
+// passes the counts for this plan's ID only). Starts at 100 and subtracts
+// weighted penalties, clamped to [planHealthScoreMin, planHealthScoreMax].
+// The returned factors document exactly which penalties applied so a bad
+// score is actionable instead of opaque (issue #340 follow-up, PR #376).
+//
+// A completed plan (RampSchedule.IsComplete(), guarded against the
+// degenerate TotalSteps == 0 case) short-circuits to a perfect score:
+// historical failure/cancellation rows and a disabled toggle-off after
+// completion are expected end-of-life noise, not signals an operator
+// should chase.
+func computePlanHealth(plan config.PurchasePlan, now time.Time, counts config.ExecutionStatusCounts) (int, []PlanHealthFactor) {
+ if plan.RampSchedule.TotalSteps > 0 && plan.RampSchedule.IsComplete() {
+ return planHealthScoreMax, nil
+ }
+
+ factors := collectPlanHealthFactors(plan, now, counts)
+
+ score := planHealthScoreMax
+ for _, f := range factors {
+ score -= f.Penalty
+ }
+ if score < planHealthScoreMin {
+ score = planHealthScoreMin
+ }
+ return score, factors
+}
+
+// collectPlanHealthFactors runs every per-factor check and returns the
+// subset that applies to plan, in table order (overdue, failed_executions,
+// canceled_executions, then the mutually-exclusive stalled/behind_schedule
+// pair, then disabled_midway). Split out from computePlanHealth so each
+// factor stays an independent, individually testable function.
+func collectPlanHealthFactors(plan config.PurchasePlan, now time.Time, counts config.ExecutionStatusCounts) []PlanHealthFactor {
+ var factors []PlanHealthFactor
+ if f, ok := overdueFactor(plan, now); ok {
+ factors = append(factors, f)
+ }
+ if f, ok := failedExecutionsFactor(counts); ok {
+ factors = append(factors, f)
+ }
+ if f, ok := canceledExecutionsFactor(counts); ok {
+ factors = append(factors, f)
+ }
+ if f, ok := scheduleFactor(plan, now); ok {
+ factors = append(factors, f)
+ }
+ if f, ok := disabledMidwayFactor(plan); ok {
+ factors = append(factors, f)
+ }
+ return factors
+}
+
+// overdueFactor: enabled AND next_execution_date < now.
+func overdueFactor(plan config.PurchasePlan, now time.Time) (PlanHealthFactor, bool) {
+ if !plan.Enabled || plan.NextExecutionDate == nil || !plan.NextExecutionDate.Before(now) {
+ return PlanHealthFactor{}, false
+ }
+ return PlanHealthFactor{
+ Code: HealthFactorOverdue,
+ Penalty: penaltyOverdue,
+ Note: "next purchase date has passed",
+ }, true
+}
+
+// failedExecutionsFactor: -10 per failed execution in the last
+// planHealthLookbackDays, capped at 4 (-40 max). The note reports the true
+// (uncapped) count and the window it covers, so an operator can see the full
+// extent even when the penalty itself is capped.
+func failedExecutionsFactor(counts config.ExecutionStatusCounts) (PlanHealthFactor, bool) {
+ count := counts[config.StatusFailed]
+ if count == 0 {
+ return PlanHealthFactor{}, false
+ }
+ counted := count
+ if counted > maxCountedFailedExecs {
+ counted = maxCountedFailedExecs
+ }
+ return PlanHealthFactor{
+ Code: HealthFactorFailedExecutions,
+ Penalty: counted * penaltyPerFailedExec,
+ Note: fmt.Sprintf("%d failed execution(s) in the last %d days", count, planHealthLookbackDays),
+ }, true
+}
+
+// canceledExecutionsFactor: -5 per canceled execution in the last
+// planHealthLookbackDays, capped at 4 (-20 max). Counts both spellings of
+// "canceled" (config.StatusCanceled and the legacy
+// config.LegacyStatusCanceled) so pre-#1278 rows aren't invisible to scoring.
+func canceledExecutionsFactor(counts config.ExecutionStatusCounts) (PlanHealthFactor, bool) {
+ count := counts[config.StatusCanceled] + counts[config.LegacyStatusCanceled]
+ if count == 0 {
+ return PlanHealthFactor{}, false
+ }
+ counted := count
+ if counted > maxCountedCanceledExecs {
+ counted = maxCountedCanceledExecs
+ }
+ return PlanHealthFactor{
+ Code: HealthFactorCanceledExecutions,
+ Penalty: counted * penaltyPerCanceledExec,
+ Note: fmt.Sprintf("%d canceled execution(s) in the last %d days", count, planHealthLookbackDays),
+ }, true
+}
+
+// scheduleFactor evaluates the ramp schedule's progress against wall-clock
+// time and returns at most one of two mutually exclusive factors:
+//
+// - stalled (-15): enabled, past the first scheduled interval, but no
+// step has executed yet (CurrentStep == 0).
+// - behind_schedule (-20): CurrentStep lags the step implied by
+// StartDate + StepIntervalDays and capped at TotalSteps, in any other
+// case (including a disabled plan that was never started).
+//
+// Plans with no positive StepIntervalDays (e.g. "immediate" ramp schedules)
+// have no schedule position to fall behind on and are skipped entirely --
+// there is no divide-by-zero guard needed elsewhere because this is the
+// only place StepIntervalDays is used as a divisor.
+//
+// Plans with no StartDate are skipped for the same reason: the schedule
+// position is unknown, and measuring elapsed time from a zero time.Time
+// would report every such plan as ~740000 days behind schedule -- a
+// fabricated penalty derived from a missing input rather than a real one.
+// RampSchedule.GetNextPurchaseDate guards the same field the same way.
+func scheduleFactor(plan config.PurchasePlan, now time.Time) (PlanHealthFactor, bool) {
+ ramp := plan.RampSchedule
+ if ramp.StepIntervalDays <= 0 || ramp.StartDate.IsZero() {
+ return PlanHealthFactor{}, false
+ }
+
+ daysSinceStart := int(now.Sub(ramp.StartDate).Hours() / 24)
+ if daysSinceStart < ramp.StepIntervalDays {
+ return PlanHealthFactor{}, false
+ }
+
+ if plan.Enabled && ramp.CurrentStep == 0 {
+ return PlanHealthFactor{
+ Code: HealthFactorStalled,
+ Penalty: penaltyStalled,
+ Note: "enabled but no purchases have executed since the first scheduled step",
+ }, true
+ }
+
+ // Clamp to the steps the plan actually has. Elapsed time keeps growing
+ // after the ramp's full duration has passed, so the raw quotient runs
+ // past TotalSteps: a 4-step weekly ramp started 90 days ago yields 12.
+ // Reporting "expected step 12 by now" for a 4-step plan quotes a step
+ // the plan does not have, and an operator checking the plan against it
+ // finds nothing to reconcile. The note is what the clamp is for; the
+ // comparison below is clamped by the same variable only so the two can
+ // never disagree. It cannot change the outcome today, because
+ // CurrentStep >= TotalSteps is exactly IsComplete() and computePlanHealth
+ // already short-circuits those plans to a perfect score before reaching
+ // here -- keeping them consistent is cheap insurance against that
+ // short-circuit being relaxed later.
+ expectedStep := daysSinceStart / ramp.StepIntervalDays
+ if ramp.TotalSteps > 0 && expectedStep > ramp.TotalSteps {
+ expectedStep = ramp.TotalSteps
+ }
+ if ramp.CurrentStep >= expectedStep {
+ return PlanHealthFactor{}, false
+ }
+ return PlanHealthFactor{
+ Code: HealthFactorBehindSchedule,
+ Penalty: penaltyBehindSchedule,
+ Note: fmt.Sprintf("on step %d, expected step %d by now", ramp.CurrentStep, expectedStep),
+ }, true
+}
+
+// disabledMidwayFactor: disabled with 0 < current_step < total_steps.
+func disabledMidwayFactor(plan config.PurchasePlan) (PlanHealthFactor, bool) {
+ ramp := plan.RampSchedule
+ if plan.Enabled || ramp.CurrentStep <= 0 || ramp.TotalSteps <= 0 || ramp.CurrentStep >= ramp.TotalSteps {
+ return PlanHealthFactor{}, false
+ }
+ return PlanHealthFactor{
+ Code: HealthFactorDisabledMidway,
+ Penalty: penaltyDisabledMidway,
+ Note: "disabled before completing its ramp schedule",
+ }, true
+}
diff --git a/internal/api/plan_health_test.go b/internal/api/plan_health_test.go
new file mode 100644
index 000000000..461ab96ed
--- /dev/null
+++ b/internal/api/plan_health_test.go
@@ -0,0 +1,436 @@
+package api
+
+import (
+ "fmt"
+ "testing"
+ "time"
+
+ "github.com/LeanerCloud/CUDly/internal/config"
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+)
+
+// counts builds the per-plan execution-status counts computePlanHealth
+// consumes, mirroring what CountExecutionsByPlanAndStatus returns for a
+// single plan.
+func counts(pairs map[string]int) config.ExecutionStatusCounts {
+ return config.ExecutionStatusCounts(pairs)
+}
+
+// factorCodes extracts the Code of each factor, in order, for easy
+// assertion without caring about penalty/note wording in most tests.
+func factorCodes(factors []PlanHealthFactor) []PlanHealthFactorCode {
+ codes := make([]PlanHealthFactorCode, len(factors))
+ for i, f := range factors {
+ codes[i] = f.Code
+ }
+ return codes
+}
+
+// factorByCode returns the single factor carrying code, failing the test if
+// it is absent. Used by the tests that assert on Note wording, which is what
+// the operator actually reads off the tooltip.
+func factorByCode(t *testing.T, factors []PlanHealthFactor, code PlanHealthFactorCode) PlanHealthFactor {
+ t.Helper()
+ for _, f := range factors {
+ if f.Code == code {
+ return f
+ }
+ }
+ require.Failf(t, "factor not found", "no %q factor in %v", code, factorCodes(factors))
+ return PlanHealthFactor{}
+}
+
+func TestComputePlanHealth_HealthyPlanScoresPerfect(t *testing.T) {
+ now := time.Date(2026, 7, 27, 12, 0, 0, 0, time.UTC)
+ plan := config.PurchasePlan{
+ Enabled: true,
+ RampSchedule: config.RampSchedule{
+ StepIntervalDays: 7,
+ CurrentStep: 1,
+ TotalSteps: 4,
+ StartDate: now.AddDate(0, 0, -1), // just started, well within the first interval
+ },
+ }
+
+ score, factors := computePlanHealth(plan, now, nil)
+
+ assert.Equal(t, 100, score)
+ assert.Empty(t, factors)
+}
+
+func TestComputePlanHealth_CompletedPlanShortCircuitsRegardlessOfOtherIssues(t *testing.T) {
+ now := time.Date(2026, 7, 27, 12, 0, 0, 0, time.UTC)
+ overdueDate := now.AddDate(0, 0, -5)
+ plan := config.PurchasePlan{
+ Enabled: false, // would otherwise trip disabled_midway
+ NextExecutionDate: &overdueDate,
+ RampSchedule: config.RampSchedule{
+ StepIntervalDays: 7,
+ CurrentStep: 4,
+ TotalSteps: 4,
+ StartDate: now.AddDate(0, 0, -60),
+ },
+ }
+ execCounts := counts(map[string]int{"failed": 2, config.StatusCanceled: 1})
+
+ score, factors := computePlanHealth(plan, now, execCounts)
+
+ assert.Equal(t, 100, score)
+ assert.Empty(t, factors)
+}
+
+func TestComputePlanHealth_Overdue(t *testing.T) {
+ now := time.Date(2026, 7, 27, 12, 0, 0, 0, time.UTC)
+ past := now.AddDate(0, 0, -1)
+ plan := config.PurchasePlan{
+ Enabled: true,
+ NextExecutionDate: &past,
+ RampSchedule: config.RampSchedule{
+ StepIntervalDays: 30,
+ CurrentStep: 1,
+ TotalSteps: 4,
+ StartDate: now.AddDate(0, 0, -1),
+ },
+ }
+
+ score, factors := computePlanHealth(plan, now, nil)
+
+ require.Len(t, factors, 1)
+ assert.Equal(t, HealthFactorOverdue, factors[0].Code)
+ assert.Equal(t, 30, factors[0].Penalty)
+ assert.Equal(t, 100-30, score)
+}
+
+func TestComputePlanHealth_OverdueRequiresEnabled(t *testing.T) {
+ now := time.Date(2026, 7, 27, 12, 0, 0, 0, time.UTC)
+ past := now.AddDate(0, 0, -1)
+ plan := config.PurchasePlan{
+ Enabled: false,
+ NextExecutionDate: &past,
+ RampSchedule: config.RampSchedule{
+ StepIntervalDays: 30,
+ CurrentStep: 1,
+ TotalSteps: 4,
+ StartDate: now.AddDate(0, 0, -1),
+ },
+ }
+
+ _, factors := computePlanHealth(plan, now, nil)
+
+ assert.NotContains(t, factorCodes(factors), HealthFactorOverdue)
+}
+
+func TestComputePlanHealth_FailedExecutionsPenaltyCapsAtFour(t *testing.T) {
+ now := time.Date(2026, 7, 27, 12, 0, 0, 0, time.UTC)
+ plan := config.PurchasePlan{
+ Enabled: true,
+ RampSchedule: config.RampSchedule{
+ StepIntervalDays: 30,
+ CurrentStep: 1,
+ TotalSteps: 4,
+ StartDate: now.AddDate(0, 0, -1),
+ },
+ }
+ // 6 failed executions: penalty must cap at 4 * 10 = 40, but the note
+ // must still report the true count of 6.
+ execCounts := counts(map[string]int{"failed": 6})
+
+ score, factors := computePlanHealth(plan, now, execCounts)
+
+ require.Len(t, factors, 1)
+ assert.Equal(t, HealthFactorFailedExecutions, factors[0].Code)
+ assert.Equal(t, 40, factors[0].Penalty)
+ assert.Contains(t, factors[0].Note, "6 failed")
+ // The note must disclose its window: an operator reading "6 failed"
+ // with no period cannot tell a plan failing right now from one that
+ // failed six times years ago.
+ assert.Contains(t, factors[0].Note, fmt.Sprintf("last %d days", planHealthLookbackDays))
+ assert.Equal(t, 100-40, score)
+}
+
+func TestComputePlanHealth_CanceledExecutionsPenaltyCapsAtFourAndCountsBothSpellings(t *testing.T) {
+ now := time.Date(2026, 7, 27, 12, 0, 0, 0, time.UTC)
+ plan := config.PurchasePlan{
+ Enabled: true,
+ RampSchedule: config.RampSchedule{
+ StepIntervalDays: 30,
+ CurrentStep: 1,
+ TotalSteps: 4,
+ StartDate: now.AddDate(0, 0, -1),
+ },
+ }
+ // 3 canonical + 2 legacy-spelled = 5 total canceled; penalty caps at
+ // 4 * 5 = 20 but the note reports the true count of 5.
+ execCounts := counts(map[string]int{
+ config.StatusCanceled: 3,
+ config.LegacyStatusCanceled: 2,
+ })
+
+ score, factors := computePlanHealth(plan, now, execCounts)
+
+ require.Len(t, factors, 1)
+ assert.Equal(t, HealthFactorCanceledExecutions, factors[0].Code)
+ assert.Equal(t, 20, factors[0].Penalty)
+ assert.Contains(t, factors[0].Note, "5 canceled")
+ assert.Contains(t, factors[0].Note, fmt.Sprintf("last %d days", planHealthLookbackDays))
+ assert.Equal(t, 100-20, score)
+}
+
+func TestComputePlanHealth_Stalled(t *testing.T) {
+ now := time.Date(2026, 7, 27, 12, 0, 0, 0, time.UTC)
+ plan := config.PurchasePlan{
+ Enabled: true,
+ RampSchedule: config.RampSchedule{
+ StepIntervalDays: 7,
+ CurrentStep: 0,
+ TotalSteps: 4,
+ StartDate: now.AddDate(0, 0, -10), // past the first interval
+ },
+ }
+
+ score, factors := computePlanHealth(plan, now, nil)
+
+ require.Len(t, factors, 1)
+ assert.Equal(t, HealthFactorStalled, factors[0].Code)
+ assert.Equal(t, 15, factors[0].Penalty)
+ assert.Equal(t, 100-15, score)
+}
+
+func TestComputePlanHealth_BehindSchedule(t *testing.T) {
+ now := time.Date(2026, 7, 27, 12, 0, 0, 0, time.UTC)
+ plan := config.PurchasePlan{
+ Enabled: true,
+ RampSchedule: config.RampSchedule{
+ StepIntervalDays: 7,
+ CurrentStep: 1, // expected step by day 30 is 4
+ TotalSteps: 6,
+ StartDate: now.AddDate(0, 0, -30),
+ },
+ }
+
+ score, factors := computePlanHealth(plan, now, nil)
+
+ require.Len(t, factors, 1)
+ assert.Equal(t, HealthFactorBehindSchedule, factors[0].Code)
+ assert.Equal(t, 20, factors[0].Penalty)
+ assert.Contains(t, factors[0].Note, "step 1")
+ assert.Equal(t, 100-20, score)
+}
+
+// TestComputePlanHealth_BehindScheduleNoteClampsExpectedStepToTotalSteps is
+// the regression guard for the fabricated step number in the tooltip: once a
+// plan is past its ramp's full duration, daysSinceStart / StepIntervalDays
+// keeps climbing past TotalSteps, so the note quoted a step the plan does
+// not have ("expected step 12 by now" for a 4-step plan). The penalty was
+// always right; the number an operator was asked to reconcile against was
+// invented by the arithmetic.
+//
+// Asserts the exact rendered note, not just the code: the previous tests
+// covering this same arithmetic (ScoreClampsAtZeroWhenPenaltiesStack) checked
+// only codes and the clamped score, which is why nothing caught it.
+func TestComputePlanHealth_BehindScheduleNoteClampsExpectedStepToTotalSteps(t *testing.T) {
+ now := time.Date(2026, 7, 27, 12, 0, 0, 0, time.UTC)
+ plan := config.PurchasePlan{
+ Enabled: true,
+ RampSchedule: config.RampSchedule{
+ StepIntervalDays: 7,
+ CurrentStep: 2,
+ TotalSteps: 4,
+ // 90 days in: the raw quotient is 12, which is 8 steps beyond
+ // anything this plan will ever have.
+ StartDate: now.AddDate(0, 0, -90),
+ },
+ }
+
+ score, factors := computePlanHealth(plan, now, nil)
+
+ f := factorByCode(t, factors, HealthFactorBehindSchedule)
+ assert.Equal(t, "on step 2, expected step 4 by now", f.Note)
+ assert.Equal(t, 20, f.Penalty)
+ assert.Equal(t, 100-20, score)
+}
+
+func TestComputePlanHealth_StalledAndBehindScheduleAreMutuallyExclusive(t *testing.T) {
+ now := time.Date(2026, 7, 27, 12, 0, 0, 0, time.UTC)
+ plan := config.PurchasePlan{
+ Enabled: true,
+ RampSchedule: config.RampSchedule{
+ StepIntervalDays: 7,
+ CurrentStep: 0,
+ TotalSteps: 6,
+ StartDate: now.AddDate(0, 0, -30),
+ },
+ }
+
+ _, factors := computePlanHealth(plan, now, nil)
+
+ codes := factorCodes(factors)
+ assert.Contains(t, codes, HealthFactorStalled)
+ assert.NotContains(t, codes, HealthFactorBehindSchedule)
+}
+
+func TestComputePlanHealth_ImmediatePlanSkipsScheduleFactors(t *testing.T) {
+ now := time.Date(2026, 7, 27, 12, 0, 0, 0, time.UTC)
+ plan := config.PurchasePlan{
+ Enabled: true,
+ RampSchedule: config.RampSchedule{
+ Type: "immediate",
+ StepIntervalDays: 0, // no positive interval: nothing to fall behind on
+ CurrentStep: 0,
+ TotalSteps: 1,
+ StartDate: now.AddDate(0, 0, -30),
+ },
+ }
+
+ _, factors := computePlanHealth(plan, now, nil)
+
+ codes := factorCodes(factors)
+ assert.NotContains(t, codes, HealthFactorStalled)
+ assert.NotContains(t, codes, HealthFactorBehindSchedule)
+}
+
+// TestComputePlanHealth_ZeroStartDateSkipsScheduleFactors guards the
+// fabricated-penalty case: a ramp schedule with a positive StepIntervalDays
+// but no StartDate (JSONB rows written before start_date was populated, and
+// the case RampSchedule.GetNextPurchaseDate already guards) has no known
+// schedule position. Measuring elapsed time from the zero time.Time reports
+// the plan as ~740000 days behind schedule, which pre-fix subtracted a real
+// -20 penalty derived entirely from a missing input.
+func TestComputePlanHealth_ZeroStartDateSkipsScheduleFactors(t *testing.T) {
+ now := time.Date(2026, 7, 27, 12, 0, 0, 0, time.UTC)
+ plan := config.PurchasePlan{
+ Enabled: true,
+ RampSchedule: config.RampSchedule{
+ Type: "custom",
+ StepIntervalDays: 7,
+ CurrentStep: 1,
+ TotalSteps: 4,
+ // StartDate deliberately left as the zero value.
+ },
+ }
+
+ score, factors := computePlanHealth(plan, now, nil)
+
+ codes := factorCodes(factors)
+ assert.NotContains(t, codes, HealthFactorBehindSchedule)
+ assert.NotContains(t, codes, HealthFactorStalled)
+ assert.Equal(t, 100, score)
+}
+
+// TestComputePlanHealth_ZeroStartDateStillCountsExecutionFactors proves the
+// StartDate guard is scoped to the schedule factors only: an unknown
+// schedule position must not suppress penalties that don't depend on it.
+func TestComputePlanHealth_ZeroStartDateStillCountsExecutionFactors(t *testing.T) {
+ now := time.Date(2026, 7, 27, 12, 0, 0, 0, time.UTC)
+ plan := config.PurchasePlan{
+ Enabled: true,
+ RampSchedule: config.RampSchedule{
+ Type: "custom",
+ StepIntervalDays: 7,
+ CurrentStep: 1,
+ TotalSteps: 4,
+ },
+ }
+
+ score, factors := computePlanHealth(plan, now, counts(map[string]int{"failed": 2}))
+
+ require.Len(t, factors, 1)
+ assert.Equal(t, HealthFactorFailedExecutions, factors[0].Code)
+ assert.Equal(t, 100-2*penaltyPerFailedExec, score)
+}
+
+func TestComputePlanHealth_DisabledMidway(t *testing.T) {
+ now := time.Date(2026, 7, 27, 12, 0, 0, 0, time.UTC)
+ plan := config.PurchasePlan{
+ Enabled: false,
+ RampSchedule: config.RampSchedule{
+ StepIntervalDays: 7,
+ CurrentStep: 2,
+ TotalSteps: 4,
+ StartDate: now.AddDate(0, 0, -1),
+ },
+ }
+
+ score, factors := computePlanHealth(plan, now, nil)
+
+ require.Len(t, factors, 1)
+ assert.Equal(t, HealthFactorDisabledMidway, factors[0].Code)
+ assert.Equal(t, 25, factors[0].Penalty)
+ assert.Equal(t, 100-25, score)
+}
+
+func TestComputePlanHealth_DisabledButNeverStartedIsBehindScheduleNotDisabledMidway(t *testing.T) {
+ now := time.Date(2026, 7, 27, 12, 0, 0, 0, time.UTC)
+ plan := config.PurchasePlan{
+ Enabled: false,
+ RampSchedule: config.RampSchedule{
+ StepIntervalDays: 7,
+ CurrentStep: 0,
+ TotalSteps: 4,
+ StartDate: now.AddDate(0, 0, -30),
+ },
+ }
+
+ _, factors := computePlanHealth(plan, now, nil)
+
+ codes := factorCodes(factors)
+ assert.Contains(t, codes, HealthFactorBehindSchedule)
+ assert.NotContains(t, codes, HealthFactorDisabledMidway)
+ assert.NotContains(t, codes, HealthFactorStalled)
+}
+
+func TestComputePlanHealth_ScoreClampsAtZeroWhenPenaltiesStack(t *testing.T) {
+ now := time.Date(2026, 7, 27, 12, 0, 0, 0, time.UTC)
+ past := now.AddDate(0, 0, -1)
+ plan := config.PurchasePlan{
+ Enabled: false,
+ NextExecutionDate: &past,
+ RampSchedule: config.RampSchedule{
+ StepIntervalDays: 7,
+ CurrentStep: 1, // way behind the expected step
+ TotalSteps: 10,
+ StartDate: now.AddDate(0, 0, -90),
+ },
+ }
+ execCounts := counts(map[string]int{"failed": 6, config.StatusCanceled: 6})
+
+ score, factors := computePlanHealth(plan, now, execCounts)
+
+ assert.Equal(t, 0, score)
+ // disabled_midway note: overdue only requires Enabled, which is false
+ // here, so overdue must NOT fire even though NextExecutionDate is past.
+ assert.NotContains(t, factorCodes(factors), HealthFactorOverdue)
+ assert.Contains(t, factorCodes(factors), HealthFactorDisabledMidway)
+ assert.Contains(t, factorCodes(factors), HealthFactorBehindSchedule)
+ assert.Contains(t, factorCodes(factors), HealthFactorFailedExecutions)
+ assert.Contains(t, factorCodes(factors), HealthFactorCanceledExecutions)
+ // This plan is 90 days into a 10-step weekly ramp, so the raw quotient
+ // is 12. The note must quote the plan's last step, not a step it will
+ // never reach.
+ assert.Equal(t,
+ "on step 1, expected step 10 by now",
+ factorByCode(t, factors, HealthFactorBehindSchedule).Note)
+}
+
+func TestComputePlanHealth_UnrelatedExecutionStatusesAreNotCounted(t *testing.T) {
+ now := time.Date(2026, 7, 27, 12, 0, 0, 0, time.UTC)
+ plan := config.PurchasePlan{
+ Enabled: true,
+ RampSchedule: config.RampSchedule{
+ StepIntervalDays: 30,
+ CurrentStep: 1,
+ TotalSteps: 4,
+ StartDate: now.AddDate(0, 0, -1),
+ },
+ }
+ // CountExecutionsByPlanAndStatus is only ever asked for the statuses in
+ // planHealthExecutionStatuses, but an extra key in the map must not leak
+ // into any penalty.
+ execCounts := counts(map[string]int{"pending": 3, "notified": 1, "completed": 9, "approved": 2})
+
+ score, factors := computePlanHealth(plan, now, execCounts)
+
+ assert.Equal(t, 100, score)
+ assert.Empty(t, factors)
+}
diff --git a/internal/api/types.go b/internal/api/types.go
index 293d21fda..b3a4dd5bb 100644
--- a/internal/api/types.go
+++ b/internal/api/types.go
@@ -442,7 +442,25 @@ type RecommendationDetailResponse struct {
// PlansResponse holds the purchase plans response.
type PlansResponse struct {
- Plans []config.PurchasePlan `json:"plans"`
+ Plans []PlanWithHealth `json:"plans"`
+}
+
+// PlanWithHealth wraps a config.PurchasePlan with its computed health-score
+// badge data for the Plans list view (issue #340 follow-up, PR #376). The
+// score/factors are derived at read time from the plan's own attributes and
+// its recent executions -- see computePlanHealth in plan_health.go -- and
+// are never persisted, so they can't drift into the config.PurchasePlan
+// struct (and by extension the DynamoDB/Postgres columns backing it).
+//
+// HealthScore is a pointer so "not computable" stays distinguishable from a
+// real score: when the execution counts the score depends on can't be read,
+// the field serializes as null and the UI renders an explicit "unknown"
+// badge. Any concrete default would be a fabricated number an operator
+// could not tell apart from a measured one.
+type PlanWithHealth struct {
+ config.PurchasePlan
+ HealthScore *int `json:"health_score"`
+ HealthFactors []PlanHealthFactor `json:"health_factors,omitempty"`
}
// CurrentUserResponse holds the current user response.
diff --git a/internal/config/interfaces.go b/internal/config/interfaces.go
index 3edbe9e79..5af8b3b6d 100644
--- a/internal/config/interfaces.go
+++ b/internal/config/interfaces.go
@@ -53,6 +53,22 @@ type StoreInterface interface {
// this method's status filter) to avoid accidental double-processing of
// failed / expired rows.
GetExecutionsByStatuses(ctx context.Context, statuses []string, limit int) ([]PurchaseExecution, error)
+ // CountExecutionsByPlanAndStatus returns, keyed by plan ID, the exact
+ // number of executions in each of the supplied statuses that last
+ // changed state (updated_at) at or after `since` -- not those merely
+ // scheduled since then, which for a canceled row is a future date. Rows
+ // with a NULL plan_id (direct-execute purchases, and executions orphaned
+ // by a deleted plan) belong to no plan and are excluded.
+ //
+ // Deliberately NOT derived from a GetExecutionsByStatuses page: that
+ // method is capped by `limit` across ALL plans, so any caller counting
+ // per-plan rows out of it silently understates plans whose executions
+ // fall outside the newest `limit` rows. The plan health score
+ // (internal/api/plan_health.go) needs exact counts -- an understated
+ // count renders a confidently-healthy badge for an unhealthy plan --
+ // so the aggregation happens in SQL and the result is bounded by
+ // plans x statuses rather than by execution volume.
+ CountExecutionsByPlanAndStatus(ctx context.Context, statuses []string, since time.Time) (map[string]ExecutionStatusCounts, error)
// GetPlannedExecutions returns executions in any of the given states
// ordered by scheduled_date ASC (soonest first), the order the Planned
// Purchases UI lists rows so the user acts on imminent purchases first.
diff --git a/internal/config/store_postgres.go b/internal/config/store_postgres.go
index 14fee9b4e..9e8d8e514 100644
--- a/internal/config/store_postgres.go
+++ b/internal/config/store_postgres.go
@@ -1204,6 +1204,76 @@ func (s *PostgresStore) GetExecutionsByStatuses(ctx context.Context, statuses []
return s.queryExecutions(ctx, query, statuses, limit)
}
+// CountExecutionsByPlanAndStatus returns, keyed by plan ID, the exact number
+// of executions in each of the supplied statuses that last changed state at
+// or after `since`.
+//
+// Aggregated with GROUP BY rather than counted from a GetExecutionsByStatuses
+// page: that method's DESC + LIMIT truncation would silently understate any
+// plan whose executions fall outside the newest `limit` rows, which for the
+// plan health score means an unhealthy plan quietly renders as healthy once
+// newer rows from other plans push its failures out of the window. The result
+// set here is bounded by plans x statuses, not by execution volume, so it
+// needs no limit of its own.
+//
+// The window is on updated_at, NOT scheduled_date, because for the terminal
+// statuses this method exists to count those are different dates pointing in
+// opposite directions. A plan's executions are created up front for the whole
+// ramp, so a pending row carries a scheduled_date months in the FUTURE;
+// canceling it (CancelExecutionAtomic) leaves that future date untouched.
+// Windowing on scheduled_date would therefore count a purchase cancelled
+// today under a date next year, and answer "how many rows are scheduled
+// recently-or-later" instead of "how many changed state recently".
+// updated_at is stamped by CancelExecutionAtomic and
+// TransitionExecutionStatus, and backstopped by the
+// update_purchase_executions_updated_at trigger, so it is when the row
+// actually entered the status being counted.
+//
+// plan_id has been nullable since migration 000033 (direct-execute purchases
+// from the Recommendations page have no originating plan, and deleting a plan
+// SET NULLs its executions). Those rows belong to no plan and are excluded in
+// SQL; the scan still goes through sql.NullString so a NULL can never turn a
+// per-plan count into a scan error that blanks the score for every plan.
+func (s *PostgresStore) CountExecutionsByPlanAndStatus(ctx context.Context, statuses []string, since time.Time) (map[string]ExecutionStatusCounts, error) {
+ counts := make(map[string]ExecutionStatusCounts)
+ if len(statuses) == 0 {
+ return counts, nil
+ }
+
+ rows, err := s.db.Query(ctx, `
+ SELECT plan_id, status, COUNT(*)
+ FROM purchase_executions
+ WHERE status = ANY($1)
+ AND plan_id IS NOT NULL
+ AND updated_at >= $2
+ GROUP BY plan_id, status
+ `, statuses, since)
+ if err != nil {
+ return nil, fmt.Errorf("failed to count executions by plan and status: %w", err)
+ }
+ defer rows.Close()
+
+ for rows.Next() {
+ var planID sql.NullString
+ var status string
+ var n int
+ if scanErr := rows.Scan(&planID, &status, &n); scanErr != nil {
+ return nil, fmt.Errorf("failed to scan execution status count: %w", scanErr)
+ }
+ if !planID.Valid {
+ continue
+ }
+ if counts[planID.String] == nil {
+ counts[planID.String] = ExecutionStatusCounts{}
+ }
+ counts[planID.String][status] = n
+ }
+ if err := rows.Err(); err != nil {
+ return nil, fmt.Errorf("failed to iterate execution status counts: %w", err)
+ }
+ return counts, nil
+}
+
// GetPlannedExecutions returns executions whose Status is any of the supplied
// values, ordered by scheduled_date ASC (soonest first), capped at `limit`.
// Used by the Planned Purchases handler where the user expects to act on
@@ -1627,50 +1697,117 @@ func (s *PostgresStore) GetScheduledExecutionsDue(ctx context.Context) ([]Purcha
// CleanupOldExecutions deletes purchase executions older than retentionDays.
//
-// Two independent cleanup branches, each with its own retention window so
+// Three independent cleanup branches, each with its own retention window so
// that a row far in one dimension doesn't block cleanup in the other:
//
-// 1. Terminal-state cleanup: `status IN ('completed', 'cancelled', 'canceled') AND
-// scheduled_date < NOW() - retention`. Both spellings are included to
-// cover legacy rows ('cancelled') and new code rows ('canceled', canonical
-// US spelling per StatusCanceled / migration 000089). Keeps recent
-// completions visible in the UI for at least `retention` days before purging.
+// 1. Completed-state cleanup: `status = 'completed' AND scheduled_date <
+// NOW() - retention`. Keeps recent completions visible in the UI for at
+// least `retention` days before purging. `scheduled_date` is the right
+// clock here because a completed row executed on (or very near) the day
+// it was scheduled for.
+//
+// 2. Canceled-state cleanup: `status IN ('cancelled', 'canceled') AND
+// updated_at < NOW() - retention`. Both spellings are included to cover
+// legacy rows ('cancelled') and new code rows ('canceled', canonical US
+// spelling per StatusCanceled / migration 000089).
+//
+// This branch retains on `updated_at`, NOT `scheduled_date`, and must
+// stay that way: CountExecutionsByPlanAndStatus windows the plan health
+// score's canceled count on `updated_at` over exactly this retention
+// period (see internal/api/plan_health.go planHealthLookbackDays), and
+// CancelExecutionAtomic stamps `updated_at` while leaving
+// `scheduled_date` untouched. Retaining on `scheduled_date` therefore
+// purged rows the score was still counting: cancel a purchase whose
+// scheduled_date is already past the horizon (a plan's rows are created
+// up front, and parseCreatePurchasesRequest accepts a past start date)
+// and the next sweep deleted it the same day, silently raising that
+// plan's health score by up to 20 points overnight with no operator
+// action. Nothing accumulates indefinitely: a genuinely old canceled
+// row still has an old `updated_at`.
//
-// 2. Expired-execution cleanup: `expires_at IS NOT NULL AND expires_at <
-// NOW() - retention`. A row whose approval token has been expired
-// for longer than `retention` is dead — the user can no longer act
-// on it, and no transition code ever writes an 'expired' status
-// (the valid_status CHECK doesn't include it), so without this
-// branch the row would accumulate indefinitely.
+// This cuts the other way too, and the change is user-visible: a
+// canceled row with a FUTURE `scheduled_date` used to be retained
+// forever, because `scheduled_date < NOW() - retention` was never true
+// for it. It now leaves History `retention` days after the
+// cancellation rather than `retention` days after the date it was
+// scheduled for, so a purchase canceled today for a 2-year-out date
+// disappears in a month instead of in two years. That closes an
+// unbounded-retention leak and is the behavior the health window
+// already assumed.
//
-// The two branches are OR'd — a row that qualifies under EITHER is
-// deleted, regardless of the other column. An earlier revision of this
-// function incorrectly AND'd the `scheduled_date` gate with both
-// branches, which meant pending rows with a far-future `scheduled_date`
-// but a long-past `expires_at` never got cleaned up (a user scheduling a
-// 2-year-out purchase with a 30-day approval window would leave a dead
-// row accumulating for 1.9 years after the approval expired).
+// 3. Expired-row cleanup: `expires_at IS NOT NULL AND expires_at < NOW() -
+// retention`, for any status the health score does not count. This is
+// the row's own TTL column (SavePurchaseExecution writes it from
+// PurchaseExecution.TTL), NOT the approval-token deadline -- that is a
+// separate column, `approval_token_expires_at`, added by migration
//
-// NULL `expires_at` is excluded from branch 2 so rows that never had an
-// expiration deadline are safe from expiry-based cleanup.
+// 000051. A row whose TTL lapsed longer than `retention` ago is dead, so
+// without this branch it would accumulate indefinitely.
+//
+// Every status in HealthScoredExecutionStatuses is excluded here, which
+// is the whole reason that slice is exported rather than spelled out
+// twice. `expires_at` is written once at insert and never rewritten
+// when a row later reaches a terminal status, so it can be arbitrarily
+// older than the terminal transition. Without the exclusion, a row that
+// failed or was canceled TODAY but carries a lapsed `expires_at` is
+// purged by this branch the same day, while the score is still counting
+// it -- reintroducing through this branch the exact overnight score
+// jump branch 2 exists to prevent, and at up to -40 for `failed`, twice
+// canceled's worst case. Their own terminal-status branches govern them.
+//
+// `failed` deliberately has no cleanup branch of its own: the health
+// score's lookback comment (internal/api/plan_health.go
+// planHealthLookbackDays) documents that this sweep "never deletes
+// failed ones", and the score's window depends on that being true.
+//
+// Note on reachability: `expires_at` currently has exactly one writer,
+// SavePurchaseExecution via timeFromTTL(execution.TTL), and no
+// production path assigns PurchaseExecution.TTL, so rows created by
+// current code leave it NULL and this branch is inert for them. It can
+// still hold non-NULL values on legacy imported rows, which is why the
+// exclusion is written defensively rather than omitted.
+//
+// The branches are OR'd: a row that qualifies under ANY is deleted,
+// regardless of the other columns. An earlier revision of this function
+// incorrectly AND'd the `scheduled_date` gate with both branches, which
+// meant pending rows with a far-future `scheduled_date` but a long-past
+// `expires_at` never got cleaned up (a 2-year-out purchase whose row TTL
+// lapsed would leave a dead row accumulating for 1.9 years). Keep each
+// branch fully parenthesized so a future edit can't change the predicate's
+// meaning through AND/OR precedence.
+//
+// NULL `expires_at` is excluded from branch 3 so rows that never had a TTL
+// are safe from expiry-based cleanup -- which, per the reachability note
+// above, is every row current code writes.
func (s *PostgresStore) CleanupOldExecutions(ctx context.Context, retentionDays int) (int64, error) {
- // Both spellings are included: 'cancelled' (legacy, pre-migration 000089
- // rows) and 'canceled' (canonical, written by new code after the
+ // Branch 2 lists both spellings: 'cancelled' (legacy, pre-migration
+ // 000089 rows) and 'canceled' (canonical, written by new code after the
// follow-up to #1277). #1278 will drop 'cancelled' once all rows are
// normalized.
+ //
+ // Branch 3's exclusion is parameterized from
+ // HealthScoredExecutionStatuses instead of repeating a literal list, so
+ // a status added to the plan health score extends this exclusion
+ // automatically and the two cannot drift. `status` is NOT NULL
+ // (migration 000001), so `<> ALL` is not exposed to three-valued logic.
query := `
DELETE FROM purchase_executions
WHERE (
- status IN ('completed', 'cancelled', 'canceled')
+ status = 'completed'
AND scheduled_date < NOW() - INTERVAL '1 day' * $1
)
OR (
- expires_at IS NOT NULL
- AND expires_at < NOW() - INTERVAL '1 day' * $1
+ status IN ('cancelled', 'canceled')
+ AND updated_at < NOW() - INTERVAL '1 day' * $1
+ )
+ OR (
+ status <> ALL($2)
+ AND expires_at IS NOT NULL
+ AND expires_at < NOW() - INTERVAL '1 day' * $1
)
`
- result, err := s.db.Exec(ctx, query, retentionDays)
+ result, err := s.db.Exec(ctx, query, retentionDays, HealthScoredExecutionStatuses)
if err != nil {
return 0, fmt.Errorf("failed to cleanup old executions: %w", err)
}
diff --git a/internal/config/store_postgres_db_test.go b/internal/config/store_postgres_db_test.go
index a59977987..1bdc06243 100644
--- a/internal/config/store_postgres_db_test.go
+++ b/internal/config/store_postgres_db_test.go
@@ -1248,3 +1248,192 @@ func TestPostgresStoreDB_SaveRIExchangeRecord_DefaultsEmptyPaymentDue(t *testing
require.NoError(t, err, "PaymentDue must parse as a float; got %q", retrieved.PaymentDue)
assert.Equal(t, 0.0, parsed, "empty PaymentDue must round-trip as numeric zero")
}
+
+// TestPostgresStoreDB_CleanupOldExecutions_RetainsCanceledInsideHealthWindow
+// is the regression guard for the retention/health-window mismatch: the
+// cleanup sweep used to retain canceled rows on `scheduled_date` while
+// CountExecutionsByPlanAndStatus counts them on `updated_at`.
+//
+// CancelExecutionAtomic stamps `updated_at` and leaves `scheduled_date`
+// alone, and a plan's executions are created up front (with
+// parseCreatePurchasesRequest accepting a past start date), so a purchase
+// canceled TODAY can carry a `scheduled_date` from 40 days ago. Under the
+// old predicate the very next sweep deleted it, even though the plan health
+// score was still counting it, and the plan's badge silently gained up to
+// 20 points overnight with no operator action.
+//
+// Runs against a real PostgreSQL rather than pgxmock because the defect is
+// in what the predicate MEANS, not in how it is spelled: only the database
+// can answer which rows actually survive.
+func TestPostgresStoreDB_CleanupOldExecutions_RetainsCanceledInsideHealthWindow(t *testing.T) {
+ conn := setupTestContainerDB(t)
+ if conn == nil {
+ return
+ }
+
+ cleanupTestData(t, conn)
+
+ store := NewPostgresStore(conn)
+ ctx := context.Background()
+
+ plan := &PurchasePlan{
+ Name: "Retention Window Plan",
+ Enabled: true,
+ NotificationDaysBefore: 7,
+ Services: map[string]ServiceConfig{},
+ RampSchedule: PresetRampSchedules["immediate"],
+ }
+ require.NoError(t, store.CreatePurchasePlan(ctx, plan))
+
+ // Rows are inserted with raw SQL so scheduled_date / updated_at /
+ // expires_at can be set independently. The updated_at trigger is BEFORE
+ // UPDATE only, so an explicit INSERT value is preserved.
+ const retentionDays = 30
+ cases := []struct {
+ name string
+ status string
+ scheduledDays int // offset in days from now (negative = past)
+ updatedDays int // offset in days from now
+ expiresDays *int // nil => NULL expires_at
+ shouldSurvive bool
+ why string
+ }{
+ {
+ name: "canceled-today-old-scheduled-date",
+ status: StatusCanceled,
+ scheduledDays: -40,
+ updatedDays: 0,
+ shouldSurvive: true,
+ why: "canceled inside the health window; the score still counts it",
+ },
+ {
+ name: "canceled-today-old-scheduled-date-and-expired-token",
+ status: StatusCanceled,
+ scheduledDays: -40,
+ updatedDays: 0,
+ expiresDays: intPtr(-40),
+ shouldSurvive: true,
+ why: "expires_at branch must not purge what the canceled branch retains",
+ },
+ {
+ // Deliberately the LegacyStatusCanceled constant, i.e. the
+ // pre-migration-000089 double-l DB value. The label avoids
+ // spelling it out only to keep the misspell linter quiet.
+ name: "legacy-spelling-canceled-today-old-scheduled-date",
+ status: LegacyStatusCanceled,
+ scheduledDays: -40,
+ updatedDays: 0,
+ shouldSurvive: true,
+ why: "the legacy spelling is retained on updated_at too",
+ },
+ {
+ // The `failed` counterpart of the canceled case above, and the
+ // reason branch 3's exclusion is sourced from
+ // HealthScoredExecutionStatuses rather than a canceled-only
+ // literal. The score counts failed at -10 each up to -40,
+ // twice canceled's worst case, so purging one inside the
+ // window is the larger of the two overnight jumps.
+ name: "failed-today-with-lapsed-expires-at",
+ status: StatusFailed,
+ scheduledDays: -40,
+ updatedDays: 0,
+ expiresDays: intPtr(-40),
+ shouldSurvive: true,
+ why: "failed inside the health window; the score still counts it at up to -40",
+ },
+ {
+ name: "failed-long-ago-no-expiry",
+ status: StatusFailed,
+ scheduledDays: -40,
+ updatedDays: -40,
+ shouldSurvive: true,
+ why: "the sweep never deletes failed rows; plan_health.go's lookback depends on it",
+ },
+ {
+ name: "canceled-long-ago",
+ status: StatusCanceled,
+ scheduledDays: -40,
+ updatedDays: -40,
+ shouldSurvive: false,
+ why: "genuinely past retention; the fix must not disable cleanup",
+ },
+ {
+ name: "completed-old-scheduled-date",
+ status: "completed",
+ scheduledDays: -40,
+ updatedDays: 0,
+ shouldSurvive: false,
+ why: "completed rows still retain on scheduled_date",
+ },
+ {
+ name: "notified-with-long-expired-token",
+ status: "notified",
+ scheduledDays: 400,
+ updatedDays: 0,
+ expiresDays: intPtr(-40),
+ shouldSurvive: false,
+ why: "dead approval token; the expires_at branch still reaps non-canceled rows",
+ },
+ {
+ name: "pending-old-scheduled-date-no-expiry",
+ status: "pending",
+ scheduledDays: -40,
+ updatedDays: 0,
+ shouldSurvive: true,
+ why: "non-terminal with no expiry deadline is never swept",
+ },
+ }
+
+ byExecutionID := make(map[string]string, len(cases))
+ for _, tc := range cases {
+ execID := uuid.New().String()
+ byExecutionID[execID] = tc.name
+
+ var expiresAt any
+ if tc.expiresDays != nil {
+ expiresAt = time.Now().AddDate(0, 0, *tc.expiresDays)
+ }
+
+ _, err := conn.Exec(ctx, `
+ INSERT INTO purchase_executions
+ (plan_id, execution_id, status, step_number,
+ scheduled_date, expires_at, created_at, updated_at)
+ VALUES ($1, $2, $3, 1, $4, $5, NOW(), $6)
+ `,
+ plan.ID,
+ execID,
+ tc.status,
+ time.Now().AddDate(0, 0, tc.scheduledDays),
+ expiresAt,
+ time.Now().AddDate(0, 0, tc.updatedDays),
+ )
+ require.NoErrorf(t, err, "seeding %s", tc.name)
+ }
+
+ deleted, err := store.CleanupOldExecutions(ctx, retentionDays)
+ require.NoError(t, err)
+
+ rows, err := conn.Query(ctx,
+ `SELECT execution_id FROM purchase_executions WHERE plan_id = $1`, plan.ID)
+ require.NoError(t, err)
+ defer rows.Close()
+
+ survived := make(map[string]bool)
+ for rows.Next() {
+ var execID string
+ require.NoError(t, rows.Scan(&execID))
+ survived[byExecutionID[execID]] = true
+ }
+ require.NoError(t, rows.Err())
+
+ wantDeleted := int64(0)
+ for _, tc := range cases {
+ if tc.shouldSurvive {
+ assert.Truef(t, survived[tc.name], "%s must SURVIVE the sweep: %s", tc.name, tc.why)
+ } else {
+ assert.Falsef(t, survived[tc.name], "%s must be DELETED: %s", tc.name, tc.why)
+ wantDeleted++
+ }
+ }
+ assert.Equal(t, wantDeleted, deleted, "RowsAffected must match the rows actually purged")
+}
diff --git a/internal/config/store_postgres_pgxmock_test.go b/internal/config/store_postgres_pgxmock_test.go
index 39e7f9e50..a044862d0 100644
--- a/internal/config/store_postgres_pgxmock_test.go
+++ b/internal/config/store_postgres_pgxmock_test.go
@@ -8,6 +8,7 @@ import (
"database/sql"
"encoding/json"
"errors"
+ "strings"
"testing"
"time"
@@ -2180,7 +2181,9 @@ func TestPGXMock_CleanupOldExecutions_Success(t *testing.T) {
store := storeWith(mock)
ctx := context.Background()
- mock.ExpectExec("DELETE").WithArgs(pgxmock.AnyArg()).
+ // Two args: retentionDays, then the health-scored status list branch 3
+ // excludes (see CleanupOldExecutions).
+ mock.ExpectExec("DELETE").WithArgs(pgxmock.AnyArg(), pgxmock.AnyArg()).
WillReturnResult(pgxmock.NewResult("DELETE", 5))
n, err := store.CleanupOldExecutions(ctx, 90)
@@ -2772,7 +2775,7 @@ func TestPGXMock_CleanupOldExecutions_IncludesCanonicalStatus(t *testing.T) {
// appearing together in the SQL. If either is missing the mock won't
// match, causing an "unexpected query" error from ExpectationsWereMet.
mock.ExpectExec(`'cancelled'.*'canceled'`).
- WithArgs(pgxmock.AnyArg()).
+ WithArgs(pgxmock.AnyArg(), pgxmock.AnyArg()).
WillReturnResult(pgxmock.NewResult("DELETE", 4))
n, err := store.CleanupOldExecutions(ctx, 90)
@@ -2780,3 +2783,222 @@ func TestPGXMock_CleanupOldExecutions_IncludesCanonicalStatus(t *testing.T) {
assert.Equal(t, int64(4), n)
assert.NoError(t, mock.ExpectationsWereMet())
}
+
+// TestPGXMock_CleanupOldExecutions_RetainsCanceledOnUpdatedAt pins the shape
+// of the retention predicate in the fast unit suite. The behavioral proof
+// (which rows actually survive) lives in the integration test
+// TestPostgresStoreDB_CleanupOldExecutions_RetainsCanceledInsideHealthWindow;
+// this guard catches a refactor that quietly moves the canceled branch back
+// onto scheduled_date, which would let the sweep delete rows the plan health
+// score is still counting on updated_at.
+//
+// Each anchor is asserted separately so a failure names the branch that
+// regressed instead of just "the SQL changed".
+func TestPGXMock_CleanupOldExecutions_RetainsCanceledOnUpdatedAt(t *testing.T) {
+ anchors := map[string]string{
+ "canceled branch retains on updated_at": `(?s)status IN \('cancelled', 'canceled'\)\s+AND updated_at`,
+ "completed branch retains on scheduled_date": `(?s)status = 'completed'\s+AND scheduled_date`,
+ "expires_at branch skips scored statuses": `(?s)status <> ALL\(\$2\)\s+AND expires_at IS NOT NULL`,
+ }
+
+ for name, pattern := range anchors {
+ t.Run(name, func(t *testing.T) {
+ mock := newMock(t)
+ store := storeWith(mock)
+ ctx := context.Background()
+
+ mock.ExpectExec(pattern).
+ WithArgs(pgxmock.AnyArg(), pgxmock.AnyArg()).
+ WillReturnResult(pgxmock.NewResult("DELETE", 1))
+
+ n, err := store.CleanupOldExecutions(ctx, 30)
+ require.NoError(t, err)
+ assert.Equal(t, int64(1), n)
+ assert.NoError(t, mock.ExpectationsWereMet())
+ })
+ }
+}
+
+// TestPGXMock_CleanupOldExecutions_ExcludesEveryHealthScoredStatus pins the
+// $2 argument itself, not just the `<> ALL($2)` spelling. The anchors above
+// would still match if someone passed a narrower list (say, the canceled
+// spellings only), which is exactly the drift the shared slice exists to
+// prevent: `failed` is scored at up to -40, so dropping it from the
+// exclusion silently reintroduces the larger of the two overnight score
+// jumps. Asserting on config.HealthScoredExecutionStatuses rather than a
+// literal means adding a scored status extends this guard automatically.
+func TestPGXMock_CleanupOldExecutions_ExcludesEveryHealthScoredStatus(t *testing.T) {
+ mock := newMock(t)
+ store := storeWith(mock)
+ ctx := context.Background()
+
+ mock.ExpectExec("DELETE").
+ WithArgs(30, HealthScoredExecutionStatuses).
+ WillReturnResult(pgxmock.NewResult("DELETE", 2))
+
+ n, err := store.CleanupOldExecutions(ctx, 30)
+ require.NoError(t, err)
+ assert.Equal(t, int64(2), n)
+ assert.NoError(t, mock.ExpectationsWereMet())
+ // Guards the slice's contents independently of how the query uses it.
+ assert.ElementsMatch(t,
+ []string{StatusFailed, StatusCanceled, LegacyStatusCanceled},
+ HealthScoredExecutionStatuses)
+}
+
+// ─── CountExecutionsByPlanAndStatus ──────────────────────────────────────────
+
+// TestPGXMock_CountExecutionsByPlanAndStatus_AggregatesInSQL is the
+// regression guard for the plan-health truncation bug: counting executions
+// per plan out of GetExecutionsByStatuses (ORDER BY ... DESC + LIMIT, capped
+// across ALL plans) silently understates any plan whose rows fall outside
+// the newest page, so an unhealthy plan renders a confident "healthy" badge.
+// The counts must therefore come from a GROUP BY with no LIMIT: the result
+// set is bounded by plans x statuses, not by execution volume. The regex
+// anchors fail the test if a refactor reintroduces a LIMIT, drops the
+// aggregation, or drops the updated_at window.
+func TestPGXMock_CountExecutionsByPlanAndStatus_AggregatesInSQL(t *testing.T) {
+ mock := newMock(t)
+ store := storeWith(mock)
+ ctx := context.Background()
+
+ rows := pgxmock.NewRows([]string{"plan_id", "status", "count"}).
+ AddRow("plan-a", "failed", 7).
+ AddRow("plan-a", StatusCanceled, 2).
+ AddRow("plan-b", LegacyStatusCanceled, 1)
+ mock.ExpectQuery(`(?s)SELECT plan_id, status, COUNT\(\*\).*FROM purchase_executions.*status = ANY\(\$1\).*plan_id IS NOT NULL.*updated_at >= \$2.*GROUP BY plan_id, status`).
+ WithArgs(pgxmock.AnyArg(), pgxmock.AnyArg()).
+ WillReturnRows(rows)
+
+ counts, err := store.CountExecutionsByPlanAndStatus(ctx,
+ []string{"failed", StatusCanceled, LegacyStatusCanceled},
+ time.Now().AddDate(0, 0, -90))
+ require.NoError(t, err)
+
+ // The counts are exact, not capped: 7 failures survive intact even
+ // though DefaultListLimit-style paging would be the temptation here.
+ assert.Equal(t, 7, counts["plan-a"]["failed"])
+ assert.Equal(t, 2, counts["plan-a"][StatusCanceled])
+ assert.Equal(t, 1, counts["plan-b"][LegacyStatusCanceled])
+ // Absent statuses read as a zero count rather than needing a guard.
+ assert.Equal(t, 0, counts["plan-b"]["failed"])
+ assert.Equal(t, 0, counts["plan-missing"]["failed"])
+ assert.NoError(t, mock.ExpectationsWereMet())
+}
+
+// TestPGXMock_CountExecutionsByPlanAndStatus_PassesSinceBound pins that the
+// caller's window actually reaches SQL, and that it is applied to updated_at
+// rather than scheduled_date. The distinction is load-bearing: a plan's
+// executions are created up front for the whole ramp, so a canceled row
+// still carries a scheduled_date months in the future. Bounding on that
+// column would count a purchase cancelled today under next year's date and
+// make the "in the last N days" wording in the factor note a lie.
+func TestPGXMock_CountExecutionsByPlanAndStatus_PassesSinceBound(t *testing.T) {
+ mock := newMock(t)
+ store := storeWith(mock)
+ ctx := context.Background()
+
+ since := time.Date(2026, 4, 28, 0, 0, 0, 0, time.UTC)
+ mock.ExpectQuery(`updated_at >= \$2`).
+ WithArgs(pgxmock.AnyArg(), since).
+ WillReturnRows(pgxmock.NewRows([]string{"plan_id", "status", "count"}))
+
+ _, err := store.CountExecutionsByPlanAndStatus(ctx, []string{"failed"}, since)
+ require.NoError(t, err)
+ assert.NoError(t, mock.ExpectationsWereMet())
+}
+
+// TestPGXMock_CountExecutionsByPlanAndStatus_NullPlanIDRow guards the
+// plan_id-nullability bug. plan_id has been nullable since migration 000033
+// (direct-execute purchases have no originating plan, and deleting a plan
+// SET NULLs its executions), so GROUP BY plan_id emits a NULL group as soon
+// as one such row is failed or canceled. Such a row belongs to no plan and
+// must be dropped, never folded into a plan bucket.
+//
+// Scope note: pgxmock leaves the destination untouched on a NULL rather than
+// reproducing pgx's real "cannot scan NULL into *string" error, so what this
+// test proves is the narrower half -- that a NULL group does not become an
+// empty-string plan key. The production error path is closed by the
+// `plan_id IS NOT NULL` filter in the query itself, which
+// TestPGXMock_CountExecutionsByPlanAndStatus_AggregatesInSQL pins.
+func TestPGXMock_CountExecutionsByPlanAndStatus_NullPlanIDRow(t *testing.T) {
+ mock := newMock(t)
+ store := storeWith(mock)
+ ctx := context.Background()
+
+ rows := pgxmock.NewRows([]string{"plan_id", "status", "count"}).
+ AddRow(nil, "failed", 5).
+ AddRow("plan-a", "failed", 2)
+ mock.ExpectQuery(`GROUP BY plan_id, status`).
+ WithArgs(pgxmock.AnyArg(), pgxmock.AnyArg()).
+ WillReturnRows(rows)
+
+ counts, err := store.CountExecutionsByPlanAndStatus(ctx, []string{"failed"}, time.Now().AddDate(0, 0, -90))
+ require.NoError(t, err)
+ assert.Len(t, counts, 1)
+ assert.Equal(t, 2, counts["plan-a"]["failed"])
+ assert.NoError(t, mock.ExpectationsWereMet())
+}
+
+// TestPGXMock_CountExecutionsByPlanAndStatus_NoLimitInQuery pins the absence
+// of a LIMIT clause explicitly: a LIMIT here would reinstate exactly the
+// truncation this method exists to avoid. Go's RE2 has no negative lookahead,
+// so the SQL is captured through a custom QueryMatcher and asserted directly
+// rather than expressed as a regex.
+func TestPGXMock_CountExecutionsByPlanAndStatus_NoLimitInQuery(t *testing.T) {
+ var executedSQL string
+ mock, err := pgxmock.NewPool(pgxmock.QueryMatcherOption(
+ pgxmock.QueryMatcherFunc(func(expectedSQL, actualSQL string) error {
+ executedSQL = actualSQL
+ return pgxmock.QueryMatcherRegexp.Match(expectedSQL, actualSQL)
+ })))
+ require.NoError(t, err)
+ store := storeWith(mock)
+ ctx := context.Background()
+
+ mock.ExpectQuery(`GROUP BY plan_id, status`).
+ WithArgs(pgxmock.AnyArg(), pgxmock.AnyArg()).
+ WillReturnRows(pgxmock.NewRows([]string{"plan_id", "status", "count"}))
+
+ _, err = store.CountExecutionsByPlanAndStatus(ctx, []string{"failed"}, time.Now().AddDate(0, 0, -90))
+ require.NoError(t, err)
+ // Without this the assertion below would pass vacuously on the empty
+ // string if the matcher were never invoked.
+ require.NotEmpty(t, executedSQL)
+ assert.NotContains(t, strings.ToUpper(executedSQL), "LIMIT")
+ assert.NoError(t, mock.ExpectationsWereMet())
+}
+
+// TestPGXMock_CountExecutionsByPlanAndStatus_EmptyStatuses guards the
+// short-circuit: an empty status list returns an empty (non-nil) map with no
+// SQL roundtrip, so callers can index it without a nil check.
+func TestPGXMock_CountExecutionsByPlanAndStatus_EmptyStatuses(t *testing.T) {
+ mock := newMock(t)
+ store := storeWith(mock)
+ ctx := context.Background()
+
+ counts, err := store.CountExecutionsByPlanAndStatus(ctx, nil, time.Now())
+ require.NoError(t, err)
+ require.NotNil(t, counts)
+ assert.Empty(t, counts)
+ assert.NoError(t, mock.ExpectationsWereMet())
+}
+
+// TestPGXMock_CountExecutionsByPlanAndStatus_QueryError propagates the
+// failure instead of returning partial counts: a partial count would silently
+// inflate a plan's health score.
+func TestPGXMock_CountExecutionsByPlanAndStatus_QueryError(t *testing.T) {
+ mock := newMock(t)
+ store := storeWith(mock)
+ ctx := context.Background()
+
+ mock.ExpectQuery(`GROUP BY plan_id, status`).
+ WithArgs(pgxmock.AnyArg(), pgxmock.AnyArg()).
+ WillReturnError(errors.New("connection reset"))
+
+ counts, err := store.CountExecutionsByPlanAndStatus(ctx, []string{"failed"}, time.Now().AddDate(0, 0, -90))
+ require.Error(t, err)
+ assert.Nil(t, counts)
+ assert.Contains(t, err.Error(), "failed to count executions by plan and status")
+ assert.NoError(t, mock.ExpectationsWereMet())
+}
diff --git a/internal/config/types.go b/internal/config/types.go
index 4416cd365..e972b4bac 100644
--- a/internal/config/types.go
+++ b/internal/config/types.go
@@ -277,6 +277,12 @@ func (r *RampSchedule) IsComplete() bool {
return r.CurrentStep >= r.TotalSteps
}
+// ExecutionStatusCounts maps an execution status value to the number of
+// executions in that status, for a single plan. Returned by
+// CountExecutionsByPlanAndStatus; a status with no rows is simply absent
+// from the map, so a plain lookup yields the correct zero.
+type ExecutionStatusCounts map[string]int
+
// PurchaseExecution represents a single execution of a purchase plan.
type PurchaseExecution struct {
PlanID string `json:"plan_id" dynamodbav:"plan_id"`
@@ -392,6 +398,35 @@ const StatusCanceled = "canceled"
// which every reference to this constant can be deleted.
const LegacyStatusCanceled = "cancel" + "led"
+// StatusFailed is the terminal status written when an execution's purchase
+// attempt errors out, and by the stuck-purchase reaper (internal/purchase/
+// reaper.go) for rows left in approved/running past its threshold.
+const StatusFailed = "failed"
+
+// HealthScoredExecutionStatuses are the execution statuses the plan health
+// score counts (internal/api/plan_health.go). It lives here, in the package
+// that owns both the status constants and the retention sweep, because two
+// unrelated-looking pieces of code have to agree on it exactly:
+//
+// - CountExecutionsByPlanAndStatus is asked for these statuses and windows
+// them on updated_at.
+// - CleanupOldExecutions must not delete a row in one of these statuses
+// while that window still covers it. Retaining on any other clock lets
+// the sweep purge a row the score is still counting, which shows up as a
+// plan's health score jumping overnight with no operator action.
+//
+// Keeping one exported slice rather than a literal list on each side is what
+// makes that agreement structural: adding a status to the score
+// automatically extends the sweep's exclusion, so the two cannot drift.
+//
+// Both spellings of canceled are present for the duration of the
+// expand-contract rename (migration 000089, contract in #1278).
+var HealthScoredExecutionStatuses = []string{
+ StatusFailed,
+ StatusCanceled,
+ LegacyStatusCanceled,
+}
+
// IsCancelable reports whether an execution may still be canceled. Only the
// pre-purchase states ("pending"/"notified"/"scheduled") qualify: once a row
// reaches "approved" or "running" the AWS commitment is being or has been
diff --git a/internal/mocks/stores.go b/internal/mocks/stores.go
index a7d4a53f4..a2cecdd41 100644
--- a/internal/mocks/stores.go
+++ b/internal/mocks/stores.go
@@ -1286,6 +1286,19 @@ func (m *MockConfigStore) GetExecutionsByStatuses(ctx context.Context, statuses
return v, args.Error(1)
}
+// CountExecutionsByPlanAndStatus mocks the CountExecutionsByPlanAndStatus operation.
+func (m *MockConfigStore) CountExecutionsByPlanAndStatus(ctx context.Context, statuses []string, since time.Time) (map[string]config.ExecutionStatusCounts, error) {
+ args := m.Called(ctx, statuses, since)
+ if args.Get(0) == nil {
+ return nil, args.Error(1)
+ }
+ v, ok := args.Get(0).(map[string]config.ExecutionStatusCounts)
+ if !ok {
+ panic(fmt.Sprintf("mock: expected map[string]config.ExecutionStatusCounts, got %T", args.Get(0)))
+ }
+ return v, args.Error(1)
+}
+
// GetPlannedExecutions mocks the GetPlannedExecutions operation.
func (m *MockConfigStore) GetPlannedExecutions(ctx context.Context, statuses []string, limit int) ([]config.PurchaseExecution, error) {
args := m.Called(ctx, statuses, limit)
diff --git a/internal/server/handler.go b/internal/server/handler.go
index a6a74499e..560bd2c58 100644
--- a/internal/server/handler.go
+++ b/internal/server/handler.go
@@ -9,6 +9,7 @@ import (
"log"
"time"
+ "github.com/LeanerCloud/CUDly/internal/config"
"github.com/LeanerCloud/CUDly/internal/purchase"
"github.com/LeanerCloud/CUDly/internal/scheduler"
"github.com/google/uuid"
@@ -286,8 +287,11 @@ func (app *Application) handleCleanupExpiredRecords(ctx context.Context) (map[st
// Clean up old execution records (30+ days)
if app.Config != nil {
- const retentionDays = 30
- deleted, err := app.Config.CleanupOldExecutions(ctx, retentionDays)
+ // Shared with the plan health score's lookback window
+ // (internal/api/plan_health.go): that score counts failed/canceled
+ // executions over exactly the period this sweep guarantees is
+ // retained, so the two must never drift apart.
+ deleted, err := app.Config.CleanupOldExecutions(ctx, config.DefaultExecutionTTLDays)
if err != nil {
log.Printf("Warning: failed to cleanup old executions: %v", err)
} else {
diff --git a/internal/server/test_helpers_test.go b/internal/server/test_helpers_test.go
index c38871acb..36ac77414 100644
--- a/internal/server/test_helpers_test.go
+++ b/internal/server/test_helpers_test.go
@@ -77,6 +77,10 @@ func (m *mockConfigStoreForHealth) GetExecutionsByStatuses(ctx context.Context,
return nil, nil
}
+func (m *mockConfigStoreForHealth) CountExecutionsByPlanAndStatus(ctx context.Context, statuses []string, since time.Time) (map[string]config.ExecutionStatusCounts, error) {
+ return nil, nil
+}
+
func (m *mockConfigStoreForHealth) GetPlannedExecutions(ctx context.Context, statuses []string, limit int) ([]config.PurchaseExecution, error) {
return nil, nil
}