cristim · Oct 5, 2026 · Oct 5, 2026
@@ -240,14 +240,6 @@ describe('renderApprovalDetailsBody', () => {
     expect(stats[5]?.querySelector('.approval-details-stat-value')?.textContent).toBe('0');
   });
 
-  it('falls back to a legacy text sentence when recommendations are empty', () => {
-    const details = makeDetails([]);
-    const body = renderApprovalDetailsBody(details, new Map());
-    expect(body.querySelector('.approval-details-fallback')).not.toBeNull();
-    expect(body.querySelector('.approval-details-table')).toBeNull();
-    expect(body.textContent).toContain('Cloud commitments will be charged');
-  });
-
   it('escapes user-controlled string values in the table', () => {
     const rec = makeRec({ resource_type: '<script>alert(1)</script>' });
     const body = renderApprovalDetailsBody(makeDetails([rec]), new Map());

@@ -27,6 +27,13 @@ jest.mock('../api', () => ({
   getConfig: jest.fn().mockResolvedValue({ global: {} }),
   approvePurchase: jest.fn(),
   cancelPurchase: jest.fn(),
+  getPurchaseDetails: jest.fn().mockResolvedValue({
+    total_upfront_cost: 100,
+    estimated_savings: 10,
+    recommendations: [{ provider: 'aws', service: 'ec2', region: 'us-east-1', count: 1, term: 1, upfront_cost: 100, savings: 10 }],
+  }),
+  listAccounts: jest.fn().mockResolvedValue([]),
+  getDeploymentInfo: jest.fn().mockResolvedValue({}),
 }));
 
 jest.mock('../navigation', () => ({

@@ -25,7 +25,7 @@ jest.mock('../api', () => ({
   getConfig: jest.fn().mockResolvedValue({ global: {} }),
   approvePurchase: jest.fn(),
   cancelPurchase: jest.fn(),
-  getPurchaseDetails: jest.fn().mockResolvedValue({ recommendations: [] }),
+  getPurchaseDetails: jest.fn(),
   listAccounts: jest.fn().mockResolvedValue([]),
   getDeploymentInfo: jest.fn().mockResolvedValue({}),
 }));
@@ -112,6 +112,11 @@ const REG_USER = {
   ],
 };
 const OTHER_UUID = 'other-uuid';
+const DETAILS = {
+  total_upfront_cost: 100,
+  estimated_savings: 10,
+  recommendations: [{ provider: 'aws', service: 'ec2', region: 'us-east-1', count: 1, term: 1, payment: 'all-upfront', upfront_cost: 100, savings: 10 }],
+};
 
 function deferred<T>(): { promise: Promise<T>; resolve(value: T): void; reject(error: Error): void } {
   let resolve!: (value: T) => void;
@@ -129,14 +134,14 @@ describe('History action ownership and refresh recovery (#249)', () => {
     jest.clearAllMocks();
     (getCurrentUser as jest.Mock).mockReturnValue(ADMIN_USER);
     (api.getHistory as jest.Mock).mockReset().mockResolvedValue(data);
-    (api.getPurchaseDetails as jest.Mock).mockReset().mockResolvedValue({ recommendations: [] });
+    (api.getPurchaseDetails as jest.Mock).mockReset().mockResolvedValue(DETAILS);
     (api.approvePurchase as jest.Mock).mockReset().mockResolvedValue(undefined);
     (confirmDialog as jest.Mock).mockReset().mockResolvedValue(true);
     (showToast as jest.Mock).mockReset();
   });
 
   test('details fetch owns both actions and same-ID projections while other IDs remain independent', async () => {
-    const details = deferred<{ recommendations: never[] }>();
+    const details = deferred<typeof DETAILS>();
     (api.getPurchaseDetails as jest.Mock).mockReturnValue(details.promise);
     (api.getHistory as jest.Mock).mockResolvedValue({ ...data, purchases: [...data.purchases, makeRow({ purchase_id: 'other' })] });
     (confirmDialog as jest.Mock).mockResolvedValue(false);
@@ -148,7 +153,7 @@ describe('History action ownership and refresh recovery (#249)', () => {
     document.querySelector<HTMLButtonElement>('#purchases-approval-queue [data-approve-id="guarded"]')!.click();
     document.querySelector<HTMLButtonElement>('#history-list [data-approve-id="other"]')!.click();
     const disabled = btn.disabled;
-    details.resolve({ recommendations: [] });
+    details.resolve(DETAILS);
     await settleActions();
     expect(disabled).toBe(true);
     expect(api.getPurchaseDetails).toHaveBeenCalledTimes(2);
@@ -158,6 +163,25 @@ describe('History action ownership and refresh recovery (#249)', () => {
     expect(btn.disabled).toBe(false);
   });
 
+  test.each([
+    ['the details fetch fails', () => (api.getPurchaseDetails as jest.Mock).mockRejectedValue(new Error('403 Forbidden'))],
+    ['the details carry no recommendations', () => (api.getPurchaseDetails as jest.Mock).mockResolvedValue({ ...DETAILS, recommendations: [] })],
+  ])('Approve is not offered when %s (issue #247)', async (_label, arrange) => {
+    arrange();
+    console.error = jest.fn();
+    await loadHistory();
+    const btn = document.querySelector<HTMLButtonElement>('#history-list .history-approve-btn')!;
+    btn.click();
+    await settleActions();
+    expect(confirmDialog).not.toHaveBeenCalled();
+    expect(api.approvePurchase).not.toHaveBeenCalled();
+    expect(showToast).toHaveBeenCalledWith(expect.objectContaining({
+      kind: 'error',
+      message: expect.stringContaining('cannot be approved without showing the amount'),
+    }));
+    expect(btn.disabled).toBe(false);
+  });
+
   test.each(['cancel', 'reject', 'throw', 'api-error'] as const)('%s releases ownership for a later explicit attempt', async outcome => {
     await loadHistory();
     const btn = document.querySelector<HTMLButtonElement>('#history-list .history-approve-btn')!;
@@ -304,6 +328,7 @@ describe('History inline Approve button (issue #286)', () => {
   beforeEach(() => {
     setupDOM();
     jest.clearAllMocks();
+    (api.getPurchaseDetails as jest.Mock).mockResolvedValue(DETAILS);
   });
 
   test('admin sees Approve on every pending row, regardless of creator', async () => {

@@ -1,13 +1,70 @@
 /**
- * Deep-link parser tests. The handler itself is side-effectful (toast,
- * confirmDialog, fetch, location.replaceState) and exercised end-to-end
- * in manual smoke — here we pin the pure parser that decides whether a
- * given URL *is* a deep-link, which is the piece most likely to
- * regress when someone adds a new SPA route that overlaps with the
- * `/purchases/` prefix.
+ * Deep-link parser and approve-handler tests. The parser decides whether a
+ * given URL *is* a deep-link; the handler tests drive the real toast and
+ * confirm dialog with only the network layer mocked.
  */
 
-import { parsePurchaseDeeplink } from '../purchases-deeplink';
+jest.mock('../api', () => ({
+  getPurchaseDetails: jest.fn(),
+  listAccounts: jest.fn().mockResolvedValue([]),
+  getDeploymentInfo: jest.fn().mockResolvedValue({}),
+}));
+jest.mock('../api/client', () => ({ apiRequest: jest.fn() }));
+
+import * as api from '../api';
+import { apiRequest } from '../api/client';
+import { handlePurchaseDeeplink, parsePurchaseDeeplink } from '../purchases-deeplink';
+
+const EXEC_ID = '11111111-1111-1111-1111-111111111111';
+
+describe('handlePurchaseDeeplink approve (issue #247)', () => {
+  beforeEach(() => {
+    document.body.innerHTML = '';
+    jest.clearAllMocks();
+    (apiRequest as jest.Mock).mockResolvedValue({ status: 'approved' });
+    window.history.replaceState({}, '', `/purchases/approve/${EXEC_ID}?token=tok-xyz`);
+    jest.spyOn(console, 'error').mockImplementation(() => undefined);
+  });
+
+  afterEach(() => jest.restoreAllMocks());
+
+  it.each([
+    ['the details fetch fails', () => (api.getPurchaseDetails as jest.Mock).mockRejectedValue(new Error('403 Forbidden'))],
+    ['the details carry no recommendations', () => (api.getPurchaseDetails as jest.Mock).mockResolvedValue({
+      execution_id: EXEC_ID, status: 'pending', total_upfront_cost: 0, estimated_savings: 0, recommendations: [],
+    })],
+  ])('does not offer Approve when %s', async (_label, arrange) => {
+    arrange();
+
+    await expect(handlePurchaseDeeplink()).resolves.toBe(true);
+
+    expect(document.querySelector('.modal-confirm-backdrop')).toBeNull();
+    const buttons = Array.from(document.querySelectorAll('button')).map(b => b.textContent);
+    expect(buttons).not.toContain('Approve purchase');
+    const alert = document.querySelector('[role="alert"]');
+    expect(alert?.textContent).toContain('cannot be approved without showing the amount');
+    expect(apiRequest).not.toHaveBeenCalled();
+    expect(window.location.pathname).toBe('/purchases');
+  });
+
+  it('shows the upfront amount and approves when details load', async () => {
+    (api.getPurchaseDetails as jest.Mock).mockResolvedValue({
+      execution_id: EXEC_ID, status: 'pending', total_upfront_cost: 1200, estimated_savings: 10,
+      recommendations: [{ id: 'r1', provider: 'aws', service: 'ec2', region: 'us-east-1', resource_type: 'm5.large',
+        count: 1, term: 1, payment: 'all-upfront', upfront_cost: 1200, monthly_cost: 0, savings: 10, selected: true, purchased: false }],
+    });
+
+    const done = handlePurchaseDeeplink();
+    await new Promise(resolve => setTimeout(resolve, 0));
+    expect(document.querySelector('.approval-details-header')?.textContent).toContain('$1,200');
+    const approve = Array.from(document.querySelectorAll<HTMLButtonElement>('.modal-confirm-actions button'))
+      .find(b => b.textContent === 'Approve purchase');
+    approve!.click();
+    await done;
+
+    expect(apiRequest).toHaveBeenCalledWith(`/purchases/approve/${EXEC_ID}`, expect.objectContaining({ method: 'POST' }));
+  });
+});
 
 describe('parsePurchaseDeeplink', () => {
   it('parses an approve deep-link with a token', () => {

@@ -18,12 +18,9 @@
  * The pure builder `renderApprovalDetailsBody(details, accountsById)`
  * is split out from the network-fetching wrapper
  * `buildApprovalDetailsBody(executionId)` so unit tests can exercise
- * the rendering without mocking fetch. Both code paths share the same
- * fallback shape: when the GET fails (404, 403, network error) we
- * return a plain-text element preserving the legacy behaviour so the
- * dialog still opens and the Approve button still works — just without
- * the details. Showing a half-built modal would be worse UX than
- * gracefully falling back.
+ * the rendering without mocking fetch. When the details cannot be loaded
+ * or carry no recommendations the wrapper rejects, so callers never offer
+ * Approve without the amount (issue #247).
  */
 
 import * as api from './api';
@@ -44,8 +41,7 @@ export type AccountsById = Map<string, CloudAccount>;
 /**
  * Build an HTMLElement to pass as `body` to confirmDialog when the
  * user is about to approve a purchase. Renders the rich details from
- * `details.recommendations`, falling back to a plain text element
- * (with the legacy approval sentence) when no recs are present.
+ * `details.recommendations`.
  *
  * Exported separately from `buildApprovalDetailsBody` so tests can
  * render the body deterministically — no fetch mocking required.
@@ -55,16 +51,6 @@ export function renderApprovalDetailsBody(details: PurchaseDetails, accountsById
   root.className = 'approval-details';
 
   const recs = details.recommendations ?? [];
-  if (recs.length === 0) {
-    // Direct-execute paths (capacity_percent flow) can sometimes
-    // race the JSONB write — surface the legacy sentence so the user
-    // can still confirm rather than blocking the click on a missing
-    // payload. Shared helper keeps the fallback text identical to
-    // the network-failure branch.
-    root.appendChild(buildApprovalDetailsFallback());
-    return root;
-  }
-
   root.appendChild(renderApprovalDetailsHeader(details, recs, hostAWSAccountID));
   root.appendChild(renderApprovalDetailsTable(recs, accountsById, hostAWSAccountID));
   return root;
@@ -332,50 +318,40 @@ export function computeEffectiveSavingsPct(rec: Recommendation): number | null {
 /**
  * Network-fetching wrapper. Fetches `/api/purchases/{id}` + the
  * accounts list in parallel, builds the rich body, and returns it.
- * Any fetch failure falls back to a plain-text element carrying the
- * legacy approval sentence so confirmDialog still works.
+ * Rejects when the purchase details fail to load or carry no
+ * recommendations; callers must then refuse to offer Approve.
  */
 export async function buildApprovalDetailsBody(executionId: string): Promise<HTMLElement> {
-  try {
-    // Fetch purchase details, account list, and public info in parallel.
-    // listAccounts and getPublicInfo are caught inline so the modal still
-    // renders the full details when either endpoint is unreachable; the
-    // per-rec table degrades gracefully. console.warn keeps failures
-    // traceable rather than silently dropping them.
-    const [details, accounts, deploymentInfo] = await Promise.all([
-      api.getPurchaseDetails(executionId),
-      api.listAccounts().catch((err) => {
-        console.warn('Failed to load accounts for approval modal — falling back to UUID-prefixed labels:', err);
-        return [] as CloudAccount[];
-      }),
-      // getDeploymentInfo requires an authenticated session (AuthUser). The
-      // approval modal is only reachable post-login, so this is safe. On
-      // failure (e.g. token expiry) the orphan label degrades to "Account deleted"
-      // which is the safe default (#633).
-      api.getDeploymentInfo().catch((err) => {
-        console.warn('Failed to load deployment info for approval modal — orphan label will show "Account deleted":', err);
-        return undefined;
-      }),
-    ]);
-    const accountsById = new Map<string, CloudAccount>();
-    for (const acct of accounts) accountsById.set(acct.id, acct);
-    const hostAWSAccountID = deploymentInfo?.deployment_aws_account_id;
-    return renderApprovalDetailsBody(details, accountsById, hostAWSAccountID);
-  } catch (err) {
-    console.error('Failed to load purchase details for approval modal:', err);
-    return buildApprovalDetailsFallback();
+  // Fetch purchase details, account list, and public info in parallel.
+  // listAccounts and getPublicInfo are caught inline so the modal still
+  // renders the full details when either endpoint is unreachable; the
+  // per-rec table degrades gracefully. console.warn keeps failures
+  // traceable rather than silently dropping them.
+  const [details, accounts, deploymentInfo] = await Promise.all([
+    api.getPurchaseDetails(executionId),
+    api.listAccounts().catch((err) => {
+      console.warn('Failed to load accounts for approval modal — falling back to UUID-prefixed labels:', err);
+      return [] as CloudAccount[];
+    }),
+    // getDeploymentInfo requires an authenticated session (AuthUser). The
+    // approval modal is only reachable post-login, so this is safe. On
+    // failure (e.g. token expiry) the orphan label degrades to "Account deleted"
+    // which is the safe default (#633).
+    api.getDeploymentInfo().catch((err) => {
+      console.warn('Failed to load deployment info for approval modal — orphan label will show "Account deleted":', err);
+      return undefined;
+    }),
+  ]);
+  if (!details.recommendations?.length) {
+    throw new Error('the purchase details contain no recommendations');
   }
+  const accountsById = new Map<string, CloudAccount>();
+  for (const acct of accounts) accountsById.set(acct.id, acct);
+  const hostAWSAccountID = deploymentInfo?.deployment_aws_account_id;
+  return renderApprovalDetailsBody(details, accountsById, hostAWSAccountID);
 }
 
-/**
- * Build the legacy approval sentence as a standalone fallback
- * element. Extracted so the two failure paths (empty recommendations
- * + fetch failure) render identical text and class hooks; previously
- * the literal string lived in two places and could drift.
- */
-function buildApprovalDetailsFallback(): HTMLElement {
-  const fallback = document.createElement('div');
-  fallback.className = 'approval-details-fallback';
-  fallback.textContent = 'This authorises the purchase to execute. Cloud commitments will be charged once the executor picks up the approved row.';
-  return fallback;
+export function approvalDetailsUnavailableMessage(err: unknown): string {
+  const cause = err instanceof Error ? err.message : String(err);
+  return `This purchase cannot be approved without showing the amount, and its details could not be loaded (${cause}).`;
 }
@@ -9,7 +9,7 @@ import type { HistoryResponse, HistorySummary, HistoryPurchase } from './types';
 import type { RevokeQuote } from './api';
 import { switchTab } from './navigation';
 import { confirmDialog } from './confirmDialog';
-import { buildApprovalDetailsBody } from './approval-details';
+import { approvalDetailsUnavailableMessage, buildApprovalDetailsBody } from './approval-details';
 import { showToast } from './toast';
 import { getCurrentUser } from './state';
 import { canAccess } from './permissions';
@@ -1295,9 +1295,15 @@ function wireRowActionHandlers(container: HTMLElement): void {
       // Issue #374: show the per-rec details (service / engine /
       // resource / region / count / term + payment / costs) in the
       // modal so the user has informed consent before authorising a
-      // financial commitment. buildApprovalDetailsBody falls back to
-      // the legacy text sentence if the GET fails.
-      const detailsBody = await buildApprovalDetailsBody(id);
+      // financial commitment. Without them, Approve is not offered (#247).
+      let detailsBody: HTMLElement;
+      try {
+        detailsBody = await buildApprovalDetailsBody(id);
+      } catch (detailsError) {
+        console.error('Failed to load purchase details for approval:', detailsError);
+        showToast({ message: `${approvalDetailsUnavailableMessage(detailsError)} Try again.`, kind: 'error' });
+        return;
+      }
       const ok = await confirmDialog({
         title: 'Approve this pending purchase?',
         body: detailsBody,

@@ -21,7 +21,7 @@
 import { apiRequest } from './api/client';
 import { showToast } from './toast';
 import { confirmDialog } from './confirmDialog';
-import { buildApprovalDetailsBody } from './approval-details';
+import { approvalDetailsUnavailableMessage, buildApprovalDetailsBody } from './approval-details';
 
 type DeeplinkAction = 'approve' | 'cancel';
 
@@ -83,7 +83,18 @@ export async function handlePurchaseDeeplink(): Promise<boolean> {
   // approval is non-destructive to AWS.
   let body: string | HTMLElement;
   if (dl.action === 'approve') {
-    body = await buildApprovalDetailsBody(dl.id);
+    try {
+      body = await buildApprovalDetailsBody(dl.id);
+    } catch (err) {
+      console.error('Failed to load purchase details for approval:', err);
+      showToast({
+        message: `${approvalDetailsUnavailableMessage(err)} Open the approval link again to retry.`,
+        kind: 'error',
+        timeout: null,
+      });
+      window.history.replaceState({}, '', '/purchases');
+      return true;
+    }
   } else {
     body = `You're about to ${gerund} purchase execution ${dl.id}. This action will be recorded against your logged-in account.`;
   }

@@ -1048,13 +1048,6 @@ tr.rec-variant-row:hover td:not(.checkbox-col) {
     gap: 1rem;
 }
 
-.approval-details-fallback {
-    margin: 0;
-    color: #444;
-    line-height: 1.5;
-    font-size: 0.95rem;
-}
-
 .approval-details-header {
     display: grid;
     grid-template-columns: repeat(auto-fit, minmax(120px, 1fr));