diff --git a/frontend/src/__tests__/plans-overdue-181.test.ts b/frontend/src/__tests__/plans-overdue-181.test.ts index a07989459..f51abd2af 100644 --- a/frontend/src/__tests__/plans-overdue-181.test.ts +++ b/frontend/src/__tests__/plans-overdue-181.test.ts @@ -114,6 +114,15 @@ describe('Planned Purchases: overdue pending rows (#181)', () => { expect(hint?.textContent).toBe('Not run on its date.'); }); + test('a step with no recommendations does not suggest Run now (#1695)', async () => { + setUser(true); + (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ + purchases: [{ ...purchase('pp-bare', '2000-01-03', 'pending'), has_recommendations: false }], + }); + await loadPlans(); + expect(row('pp-bare').querySelector('.overdue-hint')?.textContent).toBe('Not run on its date.'); + }); + test('a paused row past its date is not flagged (pausing is the user\'s choice)', async () => { setUser(true); (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [purchase('pp-paused', '2000-01-03', 'paused')] }); diff --git a/frontend/src/__tests__/plans.test.ts b/frontend/src/__tests__/plans.test.ts index d6f038821..9d5e42ba1 100644 --- a/frontend/src/__tests__/plans.test.ts +++ b/frontend/src/__tests__/plans.test.ts @@ -765,6 +765,56 @@ describe('Plans Module', () => { expect(html).not.toContain('$0'); }); + describe('Run now on a step with no recommendations (#1695)', () => { + const bare = { + id: 'purchase-bare', + plan_id: 'plan-1', + plan_name: 'Azure compute 80% weekly', + scheduled_date: '2026-10-07', + provider: 'azure', + service: 'compute', + resource_type: 'Various', + region: 'Multiple', + count: 0, + term: 1, + payment: 'monthly', + upfront_cost: 0, + estimated_savings: 0, + has_recommendations: false, + status: 'pending', + step_number: 1, + total_steps: 4 + }; + + beforeEach(async () => { + (api.getPlans as jest.Mock).mockResolvedValue({ plans: [] }); + (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [bare] }); + await loadPlans(); + }); + + test('renders the run button disabled with an explanatory title', () => { + const runBtn = document.querySelector('[data-action="run"]') as HTMLButtonElement; + expect(runBtn.disabled).toBe(true); + expect(runBtn.title).toBe('Run now: this step has no recommendations, so there is nothing to run'); + expect(runBtn.getAttribute('aria-label')).toBe('Run now: this step has no recommendations, so there is nothing to run'); + expect(runBtn.getAttribute('data-id')).toBe('purchase-bare'); + }); + + test('the run action never calls the API or opens a confirm', async () => { + const runBtn = document.querySelector('[data-action="run"]') as HTMLButtonElement; + runBtn.disabled = false; + runBtn.click(); + await flushPromises(); + + expect(api.runPlannedPurchase).not.toHaveBeenCalled(); + expect(mockConfirmDialog).not.toHaveBeenCalled(); + expect(mockShowToast).toHaveBeenCalledWith(expect.objectContaining({ + message: 'This step has no recommendations attached, so there is nothing to run.', + kind: 'info', + })); + }); + }); + test('shows empty message when no planned purchases', async () => { (api.getPlans as jest.Mock).mockResolvedValue({ plans: [] }); (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] }); diff --git a/frontend/src/plans.ts b/frontend/src/plans.ts index 6342de40e..5c2958061 100644 --- a/frontend/src/plans.ts +++ b/frontend/src/plans.ts @@ -454,8 +454,9 @@ function renderPlannedPurchaseRow(purchase: PlannedPurchase): string { // #181: a pending step whose date has passed did not run; "pending" alone // reads as on schedule. Flag it and name the next action the user can take. + const runLabel = hasNoRecommendations ? 'Run now: this step has no recommendations, so there is nothing to run' : 'Run now'; const overdue = isPending && isPastScheduledDate(purchase.scheduled_date); - const overdueHint = canRunPurchase ? 'Not run on its date. Run now or pause.' : 'Not run on its date.'; + const overdueHint = canRunPurchase && !hasNoRecommendations ? 'Not run on its date. Run now or pause.' : 'Not run on its date.'; const overdueBadge = overdue ? badgeHtml('danger', 'Overdue', 'Scheduled date has passed') : ''; @@ -478,7 +479,7 @@ function renderPlannedPurchaseRow(purchase: PlannedPurchase): string { ${savingsCell} ${statusBadgeHtml(purchase.status)}${overdueBadge}${overdueHintHtml} - ${canRunPurchase ? `` : ''} + ${canRunPurchase ? `` : ''} ${canPauseOrResumePurchase && isPending ? `` : ''} ${canPauseOrResumePurchase && isPaused ? `` : ''} ${canEditPlan ? `` : ''} @@ -509,9 +510,6 @@ const runNowInFlight = new Set(); function runNowBody(target: PlannedPurchase | undefined): string | HTMLElement { if (!target) return 'This immediately executes the purchase.'; - if (target.has_recommendations === false) { - return 'This step has no recommendations attached, so running it will not buy anything.'; - } const summary = buildCommitmentSummary([{ service: target.service, provider: target.provider, @@ -532,6 +530,10 @@ async function runPlannedPurchaseNow(purchaseId: string): Promise { runNowInFlight.add(purchaseId); try { const target = lastLoadedPurchases.find(p => p.id === purchaseId); + if (target?.has_recommendations === false) { + showToast({ message: 'This step has no recommendations attached, so there is nothing to run.', kind: 'info', timeout: 5_000 }); + return; + } const ok = await confirmDialog({ title: target ? `Run step ${target.step_number}/${target.total_steps} of "${target.plan_name}" now?` : 'Run purchase now?', body: runNowBody(target), diff --git a/internal/api/handler_purchases.go b/internal/api/handler_purchases.go index f293dbac0..764f38756 100644 --- a/internal/api/handler_purchases.go +++ b/internal/api/handler_purchases.go @@ -443,6 +443,12 @@ func (h *Handler) runPlannedPurchase(ctx context.Context, req *events.LambdaFunc if fetchErr != nil { return nil, fmt.Errorf("failed to get execution: %w", fetchErr) } + // An empty step has no constraint sets to evaluate (the auth check fails + // closed on them, which surfaced as a 500), and the executor refuses it + // anyway so it cannot complete and advance the ramp (issue #1695). + if len(execution.Recommendations) == 0 { + return nil, NewClientError(409, fmt.Sprintf("execution %s cannot be started: %v", executionID, purchase.ErrPlanStepNoRecommendations)) + } if constraintErr := h.enforcePurchaseConstraints(ctx, session, "execute", execution.Recommendations); constraintErr != nil { return nil, constraintErr } diff --git a/internal/api/handler_purchases_test.go b/internal/api/handler_purchases_test.go index a707654d0..51220cb5a 100644 --- a/internal/api/handler_purchases_test.go +++ b/internal/api/handler_purchases_test.go @@ -1783,6 +1783,21 @@ func TestHandler_runPlannedPurchase_ZeroCommitment(t *testing.T) { assert.Contains(t, ce.Error(), "zero total commitment") } +// TestHandler_runPlannedPurchase_NoRecommendations: a step with zero recs must +// be refused with a clear 409 before any claim or provider call, not fall +// through to the constraints check and surface as a 500 (issue #1695). +func TestHandler_runPlannedPurchase_NoRecommendations(t *testing.T) { + err := runPlannedPurchaseGuardHarness(t, + auth.Permission{Action: auth.ActionExecute, Resource: auth.ResourcePurchases}, + nil, + ) + require.Error(t, err) + ce, ok := IsClientError(err) + require.True(t, ok, "expected a clientError, got: %v", err) + assert.Equal(t, 409, ce.code) + assert.Contains(t, ce.Error(), "no recommendations") +} + func TestHandler_deletePlannedPurchase(t *testing.T) { ctx := context.Background() mockStore := new(MockConfigStore)