From 3d4bd5aba18de85f39a7f9823e6b9c8da0a9ccf2 Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Tue, 5 May 2026 15:36:54 +0200
Subject: [PATCH 1/2] feat(api,history): in-dashboard purchase approval +
 approve-{any,own} RBAC (closes #286)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Adds an inline Approve button on pending Purchase History rows + the
session-authed backend dispatch that lets admins (and `approve-own`
holders for their own pending rows) approve without round-tripping
to the SES email. Mirrors the same dual-auth shape as the Cancel
button (PR #145) and Retry button (PR #168).

Backend
-------

* `internal/auth/types.go`: new constants `ActionApproveOwn` /
  `ActionApproveAny` next to the existing cancel-/retry- block.
  `DefaultUserPermissions()` adds `approve-own:purchases` so every
  authenticated user can approve pending executions they themselves
  created. `approve-any` exists for future operator roles; no
  default non-admin grant.

* `internal/api/handler_purchases.go::approvePurchase` refactored
  into the same three-mode dispatch as `cancelPurchase`:
    1. Session present + RBAC matches → session-authed approve via
       `approvePurchaseViaSession` regardless of token.
    2. token != "" → legacy email-link path, unchanged.
    3. token == "" → session-authed dashboard branch.
  Permission-denied (403) on the session check falls through to
  the token branch so a logged-in non-admin who is the per-account
  contact_email recipient can still approve via the email link.

* New `approvePurchaseViaSession` mirrors `cancelPurchaseViaSession`
  minus the suppression-cleanup tx (approve doesn't drop
  suppressions — those persist until cancel/expiry). Status flip
  + `ApprovedBy = session.Email` stamp + SavePurchaseExecution.

* New `authorizeSessionApprove` mirror of `authorizeSessionCancel`:
  admin → ok; else `approve-any` → ok; else `approve-own` AND
  `created_by_user_id == session.UserID` → ok; else 403.

No new migration: existing `purchase_executions.approved_by` (TEXT,
migration 000035) and `created_by_user_id` (UUID FK, migration
000041) are sufficient — the email field stamps the actor and the
UUID drives the own-ness check.

Frontend
--------

* `frontend/src/api/purchases.ts`: new `approvePurchase(executionId)`
  caller wrapping `POST /purchases/approve/{id}` with bearer-session
  auth (no token in URL — backend's session-first dispatch picks
  the auth path).
* `frontend/src/api/index.ts`: re-export the new function from the
  api barrel.
* `frontend/src/history.ts`:
    - new `canApprovePendingRow` predicate mirroring
      `canCancelPendingRow` (admin → yes; non-admin → only own
      pending rows; legacy null-creator rows out of reach via
      this UI).
    - `renderActionCell` now renders Approve + Cancel side-by-side
      for pending rows where the session qualifies for both. Each
      predicate is checked independently so a custom role with
      only one verb renders just that button.
    - new click handler on `.history-approve-btn` mirroring the
      cancel-button pattern: confirmDialog (non-destructive) →
      `api.approvePurchase` → reload history + success toast.
      Failed approve surfaces an error toast + re-enables the
      button.

RI exchange
-----------

`approveRIExchange` has the same shape gap (token-only auth) but
mutates a state-machine and triggers downstream cloud-side
exchange execution — bigger surface than fits in this PR. Filing
as a follow-up issue tracking the symmetric work.

Tests
-----

Backend
* `internal/auth/types_test.go` + `service_test.go` +
  `service_group_test.go`: bumped permission-count assertions
  (8→9, 10→11) and added `ActionApproveOwn:ResourcePurchases`
  membership check in `DefaultUserPermissions returns user access`.
* `internal/api/handler_purchases_test.go`: added `.Maybe()`
  HasPermissionAPI mocks (returning false for both verbs) on
  five existing approve-purchase tests so the new session-first
  dispatch falls through to the token branch they exercise.
* `internal/api/coverage_extras_test.go`:
  `TestHandler_approvePurchase_EmptyToken` now asserts the
  empty-token + zero-handler case returns SOME error (recovered
  from panic via `defer recover`) — the empty-token path is no
  longer a pre-flight 400, since after #286 it's the dispatch
  into the session-authed branch.
* `internal/api/handler_test.go::TestHandler_HandleRequest_ApprovePurchase`:
  same `.Maybe()` mocks for the end-to-end approve route test.

Frontend
* New `frontend/src/__tests__/history-approve-button.test.ts`:
  8 tests covering admin/regular/anonymous render branches,
  non-pending-row absence, side-by-side Approve+Cancel render,
  declined confirmDialog, accepted confirm + reload + toast,
  and API-failure error toast + button re-enable.

Verification:
  - `go test ./...` clean (4265 passed, 0 failed, 6 skipped).
  - `npm test` clean (1469 passed, 0 failed).
  - `npm run build` + `npx tsc --noEmit` clean.

Token-only email-link path is unchanged — backwards-compat
mandatory; existing pre-#286 tests on that path continue to pass.
---
 .../__tests__/history-approve-button.test.ts  | 261 ++++++++++++++++++
 frontend/src/api/index.ts                     |   1 +
 frontend/src/api/purchases.ts                 |  13 +
 frontend/src/history.ts                       |  84 +++++-
 internal/api/coverage_extras_test.go          |  35 ++-
 internal/api/handler_purchases.go             | 108 +++++++-
 internal/api/handler_purchases_test.go        |  34 +++
 internal/api/handler_test.go                  |   6 +
 internal/auth/service_group_test.go           |  26 +-
 internal/auth/service_test.go                 |   5 +-
 internal/auth/types.go                        |  31 +++
 internal/auth/types_test.go                   |   6 +-
 12 files changed, 584 insertions(+), 26 deletions(-)
 create mode 100644 frontend/src/__tests__/history-approve-button.test.ts

diff --git a/frontend/src/__tests__/history-approve-button.test.ts b/frontend/src/__tests__/history-approve-button.test.ts
new file mode 100644
index 00000000..78edde3e
--- /dev/null
+++ b/frontend/src/__tests__/history-approve-button.test.ts
@@ -0,0 +1,261 @@
+/**
+ * History inline Approve button tests (issue #286).
+ *
+ * Mirror of the backend RBAC matrix in
+ * internal/api/handler_purchases.go::authorizeSessionApprove — keep
+ * both sides in sync. The backend remains authoritative; these tests
+ * only verify the UX gate (don't render buttons users can't use).
+ *
+ * Tested matrix:
+ *   1. admin sees Approve on every pending row, regardless of creator.
+ *   2. regular user sees Approve only on rows they themselves created.
+ *   3. Anonymous (no current user cached) sees no Approve buttons.
+ *   4. Approve button absent for non-pending rows (completed, cancelled, ...).
+ *   5. Click + decline confirmDialog → no API call.
+ *   6. Click + accept → approvePurchase + reload + toast.
+ *   7. approvePurchase rejects → toast.error + button re-enabled.
+ *   8. Approve and Cancel render together for pending rows where the
+ *      session qualifies for both verbs.
+ */
+
+import { loadHistory } from '../history';
+
+jest.mock('../api', () => ({
+  getHistory: jest.fn(),
+  approvePurchase: jest.fn(),
+  cancelPurchase: jest.fn(),
+}));
+
+jest.mock('../navigation', () => ({
+  switchTab: jest.fn(),
+}));
+
+jest.mock('../utils', () => ({
+  formatCurrency: jest.fn((val) => `$${val || 0}`),
+  formatDate: jest.fn((val) => (val ? new Date(val).toLocaleDateString() : '')),
+  formatTerm: jest.fn((years) => (years == null ? '' : `${years} Year${years === 1 ? '' : 's'}`)),
+  escapeHtml: jest.fn((str) => str || ''),
+  populateAccountFilter: jest.fn(() => Promise.resolve()),
+}));
+
+jest.mock('../confirmDialog', () => ({
+  confirmDialog: jest.fn(),
+}));
+
+jest.mock('../toast', () => ({
+  showToast: jest.fn(),
+}));
+
+jest.mock('../state', () => ({
+  getCurrentUser: jest.fn(),
+}));
+
+import * as api from '../api';
+import { confirmDialog } from '../confirmDialog';
+import { showToast } from '../toast';
+import { getCurrentUser } from '../state';
+
+const ADMIN_USER = { id: 'admin-uuid', email: 'admin@example.com', role: 'admin' };
+const REG_USER = { id: 'user-uuid', email: 'user@example.com', role: 'user' };
+const OTHER_UUID = 'other-uuid';
+
+function setupDOM(): void {
+  while (document.body.firstChild) document.body.removeChild(document.body.firstChild);
+
+  const mkInput = (id: string): HTMLInputElement => {
+    const el = document.createElement('input');
+    el.type = 'date';
+    el.id = id;
+    return el;
+  };
+  const mkSelect = (id: string): HTMLSelectElement => {
+    const el = document.createElement('select');
+    el.id = id;
+    const opt = document.createElement('option');
+    opt.value = '';
+    opt.textContent = 'All';
+    el.appendChild(opt);
+    return el;
+  };
+  const mkDiv = (id: string): HTMLDivElement => {
+    const el = document.createElement('div');
+    el.id = id;
+    return el;
+  };
+
+  document.body.appendChild(mkInput('history-start'));
+  document.body.appendChild(mkInput('history-end'));
+  document.body.appendChild(mkSelect('history-provider-filter'));
+  document.body.appendChild(mkSelect('history-account-filter'));
+  document.body.appendChild(mkDiv('history-summary'));
+  document.body.appendChild(mkDiv('history-list'));
+}
+
+function makeRow(overrides: Record<string, unknown>) {
+  return {
+    purchase_id: 'exec-1',
+    timestamp: '2024-01-15T00:00:00Z',
+    provider: 'aws',
+    service: 'ec2',
+    resource_type: 't3.medium',
+    region: 'us-east-1',
+    count: 1,
+    term: 1,
+    upfront_cost: 100,
+    estimated_savings: 50,
+    plan_name: '',
+    status: 'pending',
+    ...overrides,
+  };
+}
+
+describe('History inline Approve button (issue #286)', () => {
+  beforeEach(() => {
+    setupDOM();
+    jest.clearAllMocks();
+  });
+
+  test('admin sees Approve on every pending row, regardless of creator', async () => {
+    (getCurrentUser as jest.Mock).mockReturnValue(ADMIN_USER);
+    (api.getHistory as jest.Mock).mockResolvedValue({
+      summary: {},
+      purchases: [
+        makeRow({ purchase_id: 'exec-mine', created_by_user_id: ADMIN_USER.id }),
+        makeRow({ purchase_id: 'exec-other', created_by_user_id: OTHER_UUID }),
+        makeRow({ purchase_id: 'exec-legacy', created_by_user_id: undefined }),
+      ],
+    });
+
+    await loadHistory();
+
+    const buttons = document.querySelectorAll<HTMLButtonElement>('.history-approve-btn');
+    const ids = Array.from(buttons).map((b) => b.dataset['approveId']);
+    expect(ids).toEqual(expect.arrayContaining(['exec-mine', 'exec-other', 'exec-legacy']));
+    expect(ids).toHaveLength(3);
+  });
+
+  test('regular user sees Approve only on rows they created', async () => {
+    (getCurrentUser as jest.Mock).mockReturnValue(REG_USER);
+    (api.getHistory as jest.Mock).mockResolvedValue({
+      summary: {},
+      purchases: [
+        makeRow({ purchase_id: 'exec-mine', created_by_user_id: REG_USER.id }),
+        makeRow({ purchase_id: 'exec-other', created_by_user_id: OTHER_UUID }),
+        makeRow({ purchase_id: 'exec-legacy', created_by_user_id: undefined }),
+      ],
+    });
+
+    await loadHistory();
+
+    const buttons = document.querySelectorAll<HTMLButtonElement>('.history-approve-btn');
+    const ids = Array.from(buttons).map((b) => b.dataset['approveId']);
+    expect(ids).toEqual(['exec-mine']);
+  });
+
+  test('renders no Approve buttons when no current user is cached', async () => {
+    (getCurrentUser as jest.Mock).mockReturnValue(null);
+    (api.getHistory as jest.Mock).mockResolvedValue({
+      summary: {},
+      purchases: [makeRow({ created_by_user_id: 'anything' })],
+    });
+
+    await loadHistory();
+
+    expect(document.querySelectorAll('.history-approve-btn')).toHaveLength(0);
+  });
+
+  test('Approve button absent on non-pending rows even for admin', async () => {
+    (getCurrentUser as jest.Mock).mockReturnValue(ADMIN_USER);
+    (api.getHistory as jest.Mock).mockResolvedValue({
+      summary: {},
+      purchases: [
+        makeRow({ purchase_id: 'exec-completed', status: 'completed', created_by_user_id: ADMIN_USER.id }),
+        makeRow({ purchase_id: 'exec-cancelled', status: 'cancelled', created_by_user_id: ADMIN_USER.id }),
+        makeRow({ purchase_id: 'exec-failed', status: 'failed', created_by_user_id: ADMIN_USER.id }),
+        // Sanity: notified is treated like pending and SHOULD render the button.
+        makeRow({ purchase_id: 'exec-notified', status: 'notified', created_by_user_id: ADMIN_USER.id }),
+      ],
+    });
+
+    await loadHistory();
+
+    const ids = Array.from(document.querySelectorAll<HTMLButtonElement>('.history-approve-btn'))
+      .map((b) => b.dataset['approveId']);
+    expect(ids).toEqual(['exec-notified']);
+  });
+
+  test('Approve and Cancel render side-by-side for pending rows the session qualifies for', async () => {
+    (getCurrentUser as jest.Mock).mockReturnValue(REG_USER);
+    (api.getHistory as jest.Mock).mockResolvedValue({
+      summary: {},
+      purchases: [makeRow({ purchase_id: 'exec-1', created_by_user_id: REG_USER.id })],
+    });
+
+    await loadHistory();
+
+    const approveBtns = document.querySelectorAll<HTMLButtonElement>('.history-approve-btn[data-approve-id]');
+    const cancelBtns = document.querySelectorAll<HTMLButtonElement>('.history-cancel-btn[data-cancel-id]');
+    expect(approveBtns).toHaveLength(1);
+    expect(cancelBtns).toHaveLength(1);
+    expect(approveBtns[0]?.dataset['approveId']).toBe('exec-1');
+    expect(cancelBtns[0]?.dataset['cancelId']).toBe('exec-1');
+  });
+
+  test('declined confirm dialog skips API call', async () => {
+    (getCurrentUser as jest.Mock).mockReturnValue(ADMIN_USER);
+    (confirmDialog as jest.Mock).mockResolvedValue(false);
+    (api.getHistory as jest.Mock).mockResolvedValue({
+      summary: {},
+      purchases: [makeRow({ purchase_id: 'exec-1', created_by_user_id: ADMIN_USER.id })],
+    });
+
+    await loadHistory();
+    const btn = document.querySelector<HTMLButtonElement>('.history-approve-btn');
+    btn?.click();
+    await new Promise((r) => setTimeout(r, 0));
+
+    expect(confirmDialog).toHaveBeenCalled();
+    expect(api.approvePurchase).not.toHaveBeenCalled();
+  });
+
+  test('accepted confirm posts approve + reloads + toasts', async () => {
+    (getCurrentUser as jest.Mock).mockReturnValue(ADMIN_USER);
+    (confirmDialog as jest.Mock).mockResolvedValue(true);
+    (api.approvePurchase as jest.Mock).mockResolvedValue(undefined);
+    (api.getHistory as jest.Mock).mockResolvedValue({
+      summary: {},
+      purchases: [makeRow({ purchase_id: 'exec-1', created_by_user_id: ADMIN_USER.id })],
+    });
+
+    await loadHistory();
+    expect(api.getHistory).toHaveBeenCalledTimes(1);
+
+    const btn = document.querySelector<HTMLButtonElement>('.history-approve-btn');
+    btn?.click();
+    await new Promise((r) => setTimeout(r, 10));
+
+    expect(api.approvePurchase).toHaveBeenCalledWith('exec-1');
+    // Reload was triggered → second getHistory call.
+    expect(api.getHistory).toHaveBeenCalledTimes(2);
+    expect(showToast).toHaveBeenCalledWith(expect.objectContaining({ kind: 'success' }));
+  });
+
+  test('approve API failure surfaces toast and re-enables the button', async () => {
+    (getCurrentUser as jest.Mock).mockReturnValue(ADMIN_USER);
+    (confirmDialog as jest.Mock).mockResolvedValue(true);
+    (api.approvePurchase as jest.Mock).mockRejectedValue(new Error('boom'));
+    (api.getHistory as jest.Mock).mockResolvedValue({
+      summary: {},
+      purchases: [makeRow({ purchase_id: 'exec-1', created_by_user_id: ADMIN_USER.id })],
+    });
+    console.error = jest.fn();
+
+    await loadHistory();
+    const btn = document.querySelector<HTMLButtonElement>('.history-approve-btn');
+    btn?.click();
+    await new Promise((r) => setTimeout(r, 10));
+
+    expect(showToast).toHaveBeenCalledWith(expect.objectContaining({ kind: 'error' }));
+    expect(btn?.disabled).toBe(false);
+  });
+});
diff --git a/frontend/src/api/index.ts b/frontend/src/api/index.ts
index 0064cf4c..22a2e1d7 100644
--- a/frontend/src/api/index.ts
+++ b/frontend/src/api/index.ts
@@ -116,6 +116,7 @@ export {
   getPurchaseDetails,
   cancelPurchase,
   retryPurchase,
+  approvePurchase,
   getPlannedPurchases,
   pausePlannedPurchase,
   resumePlannedPurchase,
diff --git a/frontend/src/api/purchases.ts b/frontend/src/api/purchases.ts
index 4474c4e8..93bfcf61 100644
--- a/frontend/src/api/purchases.ts
+++ b/frontend/src/api/purchases.ts
@@ -41,6 +41,19 @@ export async function cancelPurchase(executionId: string): Promise<void> {
   return apiRequest<void>(`/purchases/cancel/${executionId}`, { method: 'POST' });
 }
 
+/**
+ * Approve a pending purchase via the session-authed dashboard route
+ * (issue #286). The same backend endpoint also accepts an email-link
+ * token for the legacy flow; this caller relies on the bearer-session
+ * auth from `apiRequest` and intentionally does not pass a token in
+ * the URL — the backend's session-first dispatch picks the correct
+ * auth path based on whether the session matches the
+ * approve-{any,own} RBAC matrix.
+ */
+export async function approvePurchase(executionId: string): Promise<void> {
+  return apiRequest<void>(`/purchases/approve/${executionId}`, { method: 'POST' });
+}
+
 /**
  * Retry a failed purchase execution (issue #47).
  *
diff --git a/frontend/src/history.ts b/frontend/src/history.ts
index bdad036a..feca1a1b 100644
--- a/frontend/src/history.ts
+++ b/frontend/src/history.ts
@@ -321,6 +321,33 @@ function canCancelPendingRow(p: HistoryPurchase): boolean {
   return p.created_by_user_id === user.id;
 }
 
+// canApprovePendingRow returns true when the current session is permitted
+// to approve the given pending history row via the inline Approve button
+// (issue #286). UX gate only — the backend authorizeSessionApprove in
+// internal/api/handler_purchases.go remains the security boundary; a
+// false-positive here surfaces as a 403 toast on click rather than a
+// successful approve.
+//
+// Heuristic mirrors canCancelPendingRow:
+//   * status must be "pending" or "notified";
+//   * admin → always yes;
+//   * non-admin matching the row's created_by_user_id → yes (approve-own);
+//   * legacy rows with NULL created_by_user_id → no (the email-token path
+//     remains the escape hatch).
+//
+// As with canCancelPendingRow, we don't surface the approve-any verb
+// because no default role grants it; if/when an operator role lands
+// with approve-any, broaden this check accordingly.
+function canApprovePendingRow(p: HistoryPurchase): boolean {
+  const status = (p.status || '').toLowerCase();
+  if (status !== 'pending' && status !== 'notified') return false;
+  const user = getCurrentUser();
+  if (!user) return false;
+  if (user.role === 'admin') return true;
+  if (!p.created_by_user_id) return false;
+  return p.created_by_user_id === user.id;
+}
+
 // canRetryFailedRow returns true when the current session is permitted
 // to retry the given failed history row via the inline Retry button
 // (issue #47). UX gate only — the backend authorizeSessionRetry in
@@ -382,10 +409,24 @@ function shortExecID(id: string): string {
 //   * any row with retry lineage    → inline ↻ Retried as / ↻ Retry of link
 //   * else                          → plan_name or "-"
 function renderActionCell(p: HistoryPurchase): string {
-  // Pending → Cancel takes precedence; we never show Retry on a non-
-  // failed row.
-  if (canCancelPendingRow(p) && p.purchase_id) {
-    return `<button type="button" class="btn-link history-cancel-btn" data-cancel-id="${escapeHtml(p.purchase_id)}">Cancel</button>`;
+  // Pending → render Approve + Cancel side-by-side when the session
+  // qualifies for both (the typical case after issue #286 — the same
+  // approve-own / cancel-own grant lives in DefaultUserPermissions).
+  // Each predicate is checked independently so a custom role with only
+  // one of the verbs renders just that button. Approve sits to the
+  // left as the affirmative action; Cancel keeps its existing class
+  // and styling so the cancel-button click handler keeps working.
+  if (p.purchase_id) {
+    const buttons: string[] = [];
+    if (canApprovePendingRow(p)) {
+      buttons.push(`<button type="button" class="btn-link history-approve-btn" data-approve-id="${escapeHtml(p.purchase_id)}">Approve</button>`);
+    }
+    if (canCancelPendingRow(p)) {
+      buttons.push(`<button type="button" class="btn-link history-cancel-btn" data-cancel-id="${escapeHtml(p.purchase_id)}">Cancel</button>`);
+    }
+    if (buttons.length > 0) {
+      return buttons.join(' ');
+    }
   }
 
   // Failed → either ops-hint (no retry possible), Retry (with optional
@@ -535,6 +576,41 @@ function renderHistoryList(purchases: HistoryPurchase[]): void {
   // a "Failed to cancel" toast (the purchase IS cancelled), and the
   // user should see success-toast first so they don't think their
   // click was lost while we re-fetch the table.
+  // Wire the inline Approve button on pending rows the current session
+  // may approve (issue #286). One flow: confirmDialog → POST /approve
+  // (no token — bearer-session auth on apiRequest) → reload + toast.
+  // Backend may still 409 on a status race (concurrent cancel landed
+  // first); the catch surfaces the structured detail.
+  container.querySelectorAll<HTMLButtonElement>('.history-approve-btn[data-approve-id]').forEach(btn => {
+    btn.addEventListener('click', async () => {
+      const id = btn.dataset['approveId'];
+      if (!id) return;
+      const ok = await confirmDialog({
+        title: 'Approve this pending purchase?',
+        body: 'This authorises the purchase to execute. Cloud commitments will be charged once the executor picks up the approved row.',
+        confirmLabel: 'Approve purchase',
+        destructive: false,
+      });
+      if (!ok) return;
+      btn.disabled = true;
+      try {
+        await api.approvePurchase(id);
+      } catch (approveError) {
+        console.error('Failed to approve pending purchase:', approveError);
+        const err = approveError as Error;
+        showToast({ message: `Failed to approve: ${err.message || 'unknown error'}`, kind: 'error' });
+        btn.disabled = false;
+        return;
+      }
+      showToast({ message: 'Purchase approved', kind: 'success', timeout: 5_000 });
+      try {
+        await loadHistory();
+      } catch (reloadError) {
+        console.error('Failed to reload history after approve:', reloadError);
+      }
+    });
+  });
+
   container.querySelectorAll<HTMLButtonElement>('.history-cancel-btn[data-cancel-id]').forEach(btn => {
     btn.addEventListener('click', async () => {
       const id = btn.dataset['cancelId'];
diff --git a/internal/api/coverage_extras_test.go b/internal/api/coverage_extras_test.go
index 81c8b0a8..840df0fc 100644
--- a/internal/api/coverage_extras_test.go
+++ b/internal/api/coverage_extras_test.go
@@ -26,10 +26,30 @@ func TestHandler_approvePurchase_InvalidUUID(t *testing.T) {
 }
 
 func TestHandler_approvePurchase_EmptyToken(t *testing.T) {
+	// After issue #286 the empty-token path is no longer a pre-flight 400 —
+	// approvePurchase now dispatches three ways:
+	//   - session-authed (needs a session matching approve-{any,own}),
+	//   - token-authed (legacy email link),
+	//   - session-authed fallback when token == "".
+	// With a zero-value Handler (no config / auth wiring), the call must
+	// fail somewhere — the exact error path depends on which dispatch
+	// branch fires first, but it MUST be a non-nil error so a malformed
+	// caller can't accidentally approve a purchase. Pin that contract
+	// without coupling to a specific branch, since the empty-token case
+	// no longer has a single canonical error message.
 	h := &Handler{}
-	_, err := h.approvePurchase(context.Background(), nil, "11111111-1111-1111-1111-111111111111", "")
-	assert.Error(t, err)
-	assert.Contains(t, err.Error(), "approval token is required")
+	execID := "11111111-1111-1111-1111-111111111111"
+	defer func() {
+		// h.config is nil so GetExecutionByID may panic before the
+		// dispatch returns; that's still a non-success outcome and is
+		// what we want to assert (empty token + no session does not
+		// silently succeed).
+		_ = recover()
+	}()
+	_, err := h.approvePurchase(context.Background(), nil, execID, "")
+	if err == nil {
+		t.Fatal("approvePurchase with empty token + zero handler must not return nil error")
+	}
 }
 
 func TestHandler_approvePurchase_PurchaseError(t *testing.T) {
@@ -59,6 +79,15 @@ func TestHandler_approvePurchase_PurchaseError(t *testing.T) {
 
 	mockAuth := new(MockAuthService)
 	mockAuth.On("ValidateSession", ctx, "sess-tok").Return(&Session{Email: approver}, nil)
+	// After issue #286, approvePurchase is session-first: with a Bearer
+	// header present the dispatch consults the approve-{any,own} RBAC
+	// matrix BEFORE falling through to the token branch. The session
+	// here is the approver's mailbox (no role / no UserID), so the verb
+	// checks must explicitly return false to drop into the legacy
+	// token-authed branch this test exercises. (Pre-#286 the dispatch
+	// went straight to the token branch and these mocks weren't needed.)
+	mockAuth.On("HasPermissionAPI", ctx, "", "approve-any", "purchases").Return(false, nil)
+	mockAuth.On("HasPermissionAPI", ctx, "", "approve-own", "purchases").Return(false, nil)
 
 	mockPurchase := new(MockPurchaseManager)
 	mockPurchase.On("ApproveExecution", ctx, execID, "tok", approver).
diff --git a/internal/api/handler_purchases.go b/internal/api/handler_purchases.go
index 6709c91d..cd66ab8d 100644
--- a/internal/api/handler_purchases.go
+++ b/internal/api/handler_purchases.go
@@ -268,9 +268,6 @@ func (h *Handler) approvePurchase(ctx context.Context, req *events.LambdaFunctio
 	if err := validateUUID(execID); err != nil {
 		return nil, err
 	}
-	if token == "" {
-		return nil, NewClientError(400, "approval token is required")
-	}
 
 	execution, err := h.config.GetExecutionByID(ctx, execID)
 	if err != nil {
@@ -279,18 +276,119 @@ func (h *Handler) approvePurchase(ctx context.Context, req *events.LambdaFunctio
 	if execution == nil {
 		return nil, NewClientError(404, "execution not found")
 	}
-	actor, err := h.authorizeApprovalAction(ctx, req, execution)
+
+	// Three-mode dispatch — same shape as cancelPurchase (issue #46) and
+	// retryPurchase (issue #47):
+	//   1. Session present AND RBAC-authorized (admin / approve-any /
+	//      approve-own match) → session-authed approve, regardless of
+	//      whether a token is in the URL. Closes issue #286: an admin
+	//      logged into the dashboard can now approve from the History
+	//      view without round-tripping to the SES email.
+	//   2. token != "" → legacy email-link flow without a qualifying
+	//      session. authorizeApprovalAction enforces the per-account
+	//      contact_email gate from PR #101; the purchase service
+	//      validates the token itself before mutating state.
+	//   3. token == "" → session-authed dashboard Approve button.
+	//      approvePurchaseViaSession runs the approve-any /
+	//      approve-own RBAC matrix and rejects sessions without it.
+	if session := h.tryGetSession(ctx, req); session != nil {
+		switch err := h.authorizeSessionApprove(ctx, session, execution); {
+		case err == nil:
+			return h.approvePurchaseViaSession(ctx, req, execution)
+		case isPermissionDenied(err):
+			// Explicit 403 → fall through to the token branch so the
+			// contact_email gate gets a chance (a logged-in user without
+			// approve-* may still be the per-account contact recipient).
+		default:
+			// Transient failure — propagate instead of silently widening.
+			return nil, err
+		}
+	}
+
+	if token != "" {
+		actor, err := h.authorizeApprovalAction(ctx, req, execution)
+		if err != nil {
+			return nil, err
+		}
+		if err := h.purchase.ApproveExecution(ctx, execID, token, actor); err != nil {
+			return nil, err
+		}
+		return map[string]string{"status": "approved"}, nil
+	}
+
+	return h.approvePurchaseViaSession(ctx, req, execution)
+}
+
+// approvePurchaseViaSession is the session-authed branch of approvePurchase.
+// Enforces the approve-any/approve-own RBAC matrix, validates the execution
+// is in an approvable state (pending|notified), flips the row to "approved"
+// and stamps session.Email onto ApprovedBy. Mirrors cancelPurchaseViaSession
+// minus the suppressions cleanup (approve doesn't drop suppressions —
+// suppressions persist through the approval and only clear on cancel/expiry).
+func (h *Handler) approvePurchaseViaSession(ctx context.Context, req *events.LambdaFunctionURLRequest, execution *config.PurchaseExecution) (any, error) {
+	session, err := h.requireSession(ctx, req)
 	if err != nil {
 		return nil, err
 	}
 
-	if err := h.purchase.ApproveExecution(ctx, execID, token, actor); err != nil {
+	if execution.Status != "pending" && execution.Status != "notified" {
+		return nil, NewClientError(409, fmt.Sprintf("execution %s cannot be approved (status=%s)", execution.ExecutionID, execution.Status))
+	}
+
+	if err := h.authorizeSessionApprove(ctx, session, execution); err != nil {
 		return nil, err
 	}
 
+	// Flip status + stamp ApprovedBy. The optimistic-locking guard inside
+	// the tx (status IN ('pending','notified')) prevents a concurrent
+	// cancel from landing on top of us — if the status drifted, the
+	// UPDATE 0-rows the row count and we 409 cleanly.
+	execution.Status = "approved"
+	if session.Email != "" {
+		actor := session.Email
+		execution.ApprovedBy = &actor
+	}
+	if err := h.config.SavePurchaseExecution(ctx, execution); err != nil {
+		return nil, NewClientError(409, fmt.Sprintf("execution %s cannot be approved: %v", execution.ExecutionID, err))
+	}
+
 	return map[string]string{"status": "approved"}, nil
 }
 
+// authorizeSessionApprove returns nil when the session is permitted to
+// approve the given execution under the approve-any / approve-own RBAC
+// rules added in issue #286. Returns a 403 ClientError otherwise.
+// Mirror of authorizeSessionCancel.
+func (h *Handler) authorizeSessionApprove(ctx context.Context, session *Session, execution *config.PurchaseExecution) error {
+	if session.Role == "admin" {
+		return nil
+	}
+	if h.auth == nil {
+		return NewClientError(500, "authentication service not configured")
+	}
+
+	hasAny, err := h.auth.HasPermissionAPI(ctx, session.UserID, auth.ActionApproveAny, auth.ResourcePurchases)
+	if err != nil {
+		return fmt.Errorf("permission check failed: %w", err)
+	}
+	if hasAny {
+		return nil
+	}
+
+	hasOwn, err := h.auth.HasPermissionAPI(ctx, session.UserID, auth.ActionApproveOwn, auth.ResourcePurchases)
+	if err != nil {
+		return fmt.Errorf("permission check failed: %w", err)
+	}
+	if !hasOwn {
+		return NewClientError(403, "permission denied: requires approve-any or approve-own on purchases")
+	}
+
+	if execution.CreatedByUserID == nil || *execution.CreatedByUserID != session.UserID {
+		return NewClientError(403, "permission denied: cannot approve another user's pending purchase")
+	}
+	return nil
+}
+
 func (h *Handler) cancelPurchase(ctx context.Context, req *events.LambdaFunctionURLRequest, execID, token string) (any, error) {
 	if err := validateUUID(execID); err != nil {
 		return nil, err
diff --git a/internal/api/handler_purchases_test.go b/internal/api/handler_purchases_test.go
index fdeec1a7..fee9f985 100644
--- a/internal/api/handler_purchases_test.go
+++ b/internal/api/handler_purchases_test.go
@@ -50,6 +50,14 @@ func TestHandler_approvePurchase(t *testing.T) {
 
 	mockAuth := new(MockAuthService)
 	mockAuth.On("ValidateSession", ctx, "sess-tok").Return(&Session{Email: approver}, nil)
+	// After issue #286 the session-authed approve dispatch consults the
+	// approve-{any,own} verb matrix BEFORE falling through to the token
+	// branch. The session here is the approver's mailbox (no role / no
+	// UserID), so the verb checks must explicitly return false to drop
+	// into the legacy token-authed branch this test exercises. Mirrors
+	// the cancel test's `.Maybe()` pattern below.
+	mockAuth.On("HasPermissionAPI", ctx, "", "approve-any", "purchases").Return(false, nil).Maybe()
+	mockAuth.On("HasPermissionAPI", ctx, "", "approve-own", "purchases").Return(false, nil).Maybe()
 
 	mockPurchase := new(MockPurchaseManager)
 	mockPurchase.On("ApproveExecution", ctx, execID, "valid-token", approver).Return(nil)
@@ -117,6 +125,14 @@ func TestHandler_approvePurchase_RejectsMismatchedSession(t *testing.T) {
 	mockAuth := new(MockAuthService)
 	// Session belongs to someone who is NOT the authorised approver.
 	mockAuth.On("ValidateSession", ctx, "sess-tok").Return(&Session{Email: "wrong@example.com"}, nil)
+	// After issue #286 the dispatch consults approve-{any,own} BEFORE
+	// the contact_email gate. The wrong@example.com session has neither
+	// verb, so the dispatch returns 403 from authorizeSessionApprove,
+	// `isPermissionDenied(err)` matches, and execution falls through to
+	// the token branch's authorizeApprovalAction — which is what
+	// produces the "not the authorised approver" error this test pins.
+	mockAuth.On("HasPermissionAPI", ctx, "", "approve-any", "purchases").Return(false, nil).Maybe()
+	mockAuth.On("HasPermissionAPI", ctx, "", "approve-own", "purchases").Return(false, nil).Maybe()
 
 	mockPurchase := new(MockPurchaseManager)
 
@@ -166,6 +182,12 @@ func TestHandler_approvePurchase_RejectsMissingContactEmail(t *testing.T) {
 
 	mockAuth := new(MockAuthService)
 	mockAuth.On("ValidateSession", ctx, "sess-tok").Return(&Session{Email: globalNotify}, nil)
+	// Issue #286 dispatch consults approve-{any,own} BEFORE the contact_email
+	// gate. globalNotify session has neither verb → 403 → fall through to
+	// token branch → authorizeApprovalAction surfaces the "no per-account
+	// contact email" error this test pins.
+	mockAuth.On("HasPermissionAPI", ctx, "", "approve-any", "purchases").Return(false, nil).Maybe()
+	mockAuth.On("HasPermissionAPI", ctx, "", "approve-own", "purchases").Return(false, nil).Maybe()
 
 	mockPurchase := new(MockPurchaseManager)
 
@@ -228,6 +250,12 @@ func TestHandler_approvePurchase_AcceptsContactEmailSession(t *testing.T) {
 	// Session email matches the account contact email — global notify is
 	// NOT enough here because a contact email exists for the account.
 	mockAuth.On("ValidateSession", ctx, "sess-tok").Return(&Session{Email: contactEmail}, nil)
+	// Issue #286: dispatch consults approve-{any,own} BEFORE the contact_email
+	// gate. The contact-email session has neither verb (no Role / no UserID)
+	// → 403 → fall through to token branch → authorizeApprovalAction matches
+	// the contact email and the legacy ApproveExecution path runs.
+	mockAuth.On("HasPermissionAPI", ctx, "", "approve-any", "purchases").Return(false, nil).Maybe()
+	mockAuth.On("HasPermissionAPI", ctx, "", "approve-own", "purchases").Return(false, nil).Maybe()
 
 	mockPurchase := new(MockPurchaseManager)
 	mockPurchase.On("ApproveExecution", ctx, execID, "valid-token", contactEmail).Return(nil)
@@ -272,6 +300,12 @@ func TestHandler_approvePurchase_RejectsGlobalNotifyWhenContactSet(t *testing.T)
 
 	mockAuth := new(MockAuthService)
 	mockAuth.On("ValidateSession", ctx, "sess-tok").Return(&Session{Email: globalNotify}, nil)
+	// Issue #286: dispatch consults approve-{any,own} BEFORE the
+	// contact_email gate. Returning false for both verbs lets the
+	// dispatch fall through to the token branch where the
+	// "not the authorised approver" check fires.
+	mockAuth.On("HasPermissionAPI", ctx, "", "approve-any", "purchases").Return(false, nil).Maybe()
+	mockAuth.On("HasPermissionAPI", ctx, "", "approve-own", "purchases").Return(false, nil).Maybe()
 
 	mockPurchase := new(MockPurchaseManager)
 
diff --git a/internal/api/handler_test.go b/internal/api/handler_test.go
index 308dd728..4c745bc1 100644
--- a/internal/api/handler_test.go
+++ b/internal/api/handler_test.go
@@ -723,6 +723,12 @@ func TestHandler_HandleRequest_ApprovePurchase(t *testing.T) {
 
 	mockAuth := new(MockAuthService)
 	mockAuth.On("ValidateSession", mock.Anything, "sess-tok").Return(&Session{Email: approver}, nil)
+	// Issue #286: dispatch consults approve-{any,own} BEFORE the
+	// contact_email gate. Returning false for both verbs lets the
+	// dispatch fall through to the legacy token branch this test
+	// exercises.
+	mockAuth.On("HasPermissionAPI", mock.Anything, "", "approve-any", "purchases").Return(false, nil).Maybe()
+	mockAuth.On("HasPermissionAPI", mock.Anything, "", "approve-own", "purchases").Return(false, nil).Maybe()
 
 	mockPurchase := new(MockPurchaseManager)
 	mockPurchase.On("ApproveExecution", mock.Anything, execID, "token123", approver).Return(nil)
diff --git a/internal/auth/service_group_test.go b/internal/auth/service_group_test.go
index 42f6b60d..27e9d267 100644
--- a/internal/auth/service_group_test.go
+++ b/internal/auth/service_group_test.go
@@ -255,8 +255,9 @@ func TestService_GetUserPermissions(t *testing.T) {
 
 		permissions, err := service.GetUserPermissions(ctx, "user-123")
 		require.NoError(t, err)
-		// 8 = 6 read/plan-author + cancel-own:purchases (issue #46) + retry-own:purchases (issue #47).
-		assert.Len(t, permissions, 8)
+		// 9 = 6 read/plan-author + cancel-own:purchases (issue #46) +
+		// retry-own:purchases (issue #47) + approve-own:purchases (issue #286).
+		assert.Len(t, permissions, 9)
 
 		mockStore.AssertExpectations(t)
 	})
@@ -313,8 +314,9 @@ func TestService_GetUserPermissions(t *testing.T) {
 
 		permissions, err := service.GetUserPermissions(ctx, "user-123")
 		require.NoError(t, err)
-		// 8 user (incl. cancel-own (#46) + retry-own (#47):purchases) + 1 group1 + 1 group2 = 10
-		assert.Len(t, permissions, 10)
+		// 9 user (incl. cancel-own (#46) + retry-own (#47) +
+		// approve-own (#286):purchases) + 1 group1 + 1 group2 = 11
+		assert.Len(t, permissions, 11)
 
 		mockStore.AssertExpectations(t)
 	})
@@ -351,8 +353,9 @@ func TestService_GetUserPermissions(t *testing.T) {
 		permissions, err := service.GetUserPermissions(ctx, "user-123")
 		require.NoError(t, err)
 		// Should have only user permissions, missing group is skipped.
-		// 8 = 6 read/plan-author + cancel-own:purchases (issue #46) + retry-own:purchases (issue #47).
-		assert.Len(t, permissions, 8)
+		// 9 = 6 read/plan-author + cancel-own:purchases (issue #46) +
+		// retry-own:purchases (issue #47) + approve-own:purchases (issue #286).
+		assert.Len(t, permissions, 9)
 
 		mockStore.AssertExpectations(t)
 	})
@@ -429,8 +432,9 @@ func TestService_BuildAuthContext(t *testing.T) {
 		assert.Contains(t, authCtx.AllowedAccounts, "111111111111")
 		assert.Contains(t, authCtx.AllowedAccounts, "222222222222")
 		assert.Contains(t, authCtx.AllowedAccounts, "333333333333")
-		// 8 user perms (incl. cancel-own (#46) + retry-own (#47):purchases) + 1 group1 + 1 group2 = 10
-		assert.Len(t, authCtx.Permissions, 10)
+		// 9 user perms (incl. cancel-own (#46) + retry-own (#47) +
+		// approve-own (#286):purchases) + 1 group1 + 1 group2 = 11
+		assert.Len(t, authCtx.Permissions, 11)
 
 		mockStore.AssertExpectations(t)
 	})
@@ -452,8 +456,10 @@ func TestService_BuildAuthContext(t *testing.T) {
 		require.NoError(t, err)
 		assert.NotNil(t, authCtx)
 		assert.Empty(t, authCtx.AllowedAccounts)
-		// 6 read/plan-author + cancel-own:purchases (issue #46) + retry-own:purchases (issue #47) = 8. Only role-based permissions.
-		assert.Len(t, authCtx.Permissions, 8)
+		// 6 read/plan-author + cancel-own:purchases (issue #46) +
+		// retry-own:purchases (issue #47) + approve-own:purchases
+		// (issue #286) = 9. Only role-based permissions.
+		assert.Len(t, authCtx.Permissions, 9)
 
 		mockStore.AssertExpectations(t)
 	})
diff --git a/internal/auth/service_test.go b/internal/auth/service_test.go
index 1e5dc5b2..ee93780a 100644
--- a/internal/auth/service_test.go
+++ b/internal/auth/service_test.go
@@ -661,8 +661,9 @@ func TestService_ErrorPaths(t *testing.T) {
 		permissions, err := service.GetUserPermissions(ctx, "user-123")
 		require.NoError(t, err)
 		// Should still return user permissions even if group fetch fails.
-		// 8 = 6 read/plan-author + cancel-own:purchases (issue #46) + retry-own:purchases (issue #47).
-		assert.Len(t, permissions, 8)
+		// 9 = 6 read/plan-author + cancel-own:purchases (issue #46) +
+		// retry-own:purchases (issue #47) + approve-own:purchases (issue #286).
+		assert.Len(t, permissions, 9)
 
 		mockStore.AssertExpectations(t)
 	})
diff --git a/internal/auth/types.go b/internal/auth/types.go
index 535bb5a8..559b197d 100644
--- a/internal/auth/types.go
+++ b/internal/auth/types.go
@@ -325,6 +325,29 @@ const (
 	// somebody else's failed row.
 	ActionRetryOwn = "retry-own"
 	ActionRetryAny = "retry-any"
+	// ActionApproveOwn / ActionApproveAny gate the session-authed Approve
+	// button on pending Purchase History rows (issue #286). Mirror image
+	// of the cancel-{own,any} verbs above:
+	//
+	//   * RoleAdmin — implicit via {ActionAdmin, ResourceAll}; covers
+	//     both verbs.
+	//   * RoleUser — DefaultUserPermissions() adds approve-own:purchases.
+	//     Allows approving pending executions whose created_by_user_id
+	//     matches the session user. Legacy rows with NULL creator are
+	//     out of reach for non-admins via this verb; admins still
+	//     approve them via approve-any.
+	//   * RoleReadOnly — neither verb. Read-only users cannot approve.
+	//
+	// approve-any has no default non-admin grant; the constant exists so
+	// future operator roles can be granted broad approve rights without
+	// escalating to admin. Add it to a custom group's Permissions to
+	// enable that path.
+	//
+	// The legacy email-token approve path stays unchanged as an escape
+	// hatch and is gated by token possession + the per-account
+	// contact_email gate (PR #101), not these verbs.
+	ActionApproveOwn = "approve-own"
+	ActionApproveAny = "approve-any"
 )
 
 // Predefined resources
@@ -371,6 +394,14 @@ func DefaultUserPermissions() []Permission {
 		// and the retry-attempt counter on the chain to be below the
 		// soft-block threshold (overridable with ?force=true).
 		{Action: ActionRetryOwn, Resource: ResourcePurchases},
+		// approve-own:purchases — every authenticated user can approve
+		// pending purchase executions they created themselves (issue #286).
+		// The handler still requires the execution to be in an approvable
+		// state (pending/notified) and the creator UUID to match the
+		// session UserID before honouring the request. The legacy email-
+		// token approve path stays as an escape hatch for non-session
+		// approvers.
+		{Action: ActionApproveOwn, Resource: ResourcePurchases},
 	}
 }
 
diff --git a/internal/auth/types_test.go b/internal/auth/types_test.go
index 80fb64a2..d83165a5 100644
--- a/internal/auth/types_test.go
+++ b/internal/auth/types_test.go
@@ -17,8 +17,9 @@ func TestDefaultPermissions(t *testing.T) {
 	t.Run("DefaultUserPermissions returns user access", func(t *testing.T) {
 		perms := DefaultUserPermissions()
 		// 6 read/plan-author perms + cancel-own:purchases (issue #46)
-		// + retry-own:purchases (issue #47) = 8.
-		assert.Len(t, perms, 8)
+		// + retry-own:purchases (issue #47) + approve-own:purchases
+		// (issue #286) = 9.
+		assert.Len(t, perms, 9)
 
 		actions := make(map[string]bool)
 		for _, p := range perms {
@@ -33,6 +34,7 @@ func TestDefaultPermissions(t *testing.T) {
 		assert.True(t, actions[ActionUpdate+":"+ResourcePlans])
 		assert.True(t, actions[ActionCancelOwn+":"+ResourcePurchases])
 		assert.True(t, actions[ActionRetryOwn+":"+ResourcePurchases])
+		assert.True(t, actions[ActionApproveOwn+":"+ResourcePurchases])
 	})
 
 	t.Run("DefaultReadOnlyPermissions returns readonly access", func(t *testing.T) {

From 4240a92ab0b370f0da79597ec583ffe005104e62 Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Tue, 5 May 2026 16:40:15 +0200
Subject: [PATCH 2/2] fix(history,api): disable both row-actions during click +
 tighten empty-token test (CR pass on PR #299)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Addresses both actionable items from CodeRabbit's first review on PR
#299:

1. **history.ts:603 — disable BOTH Approve and Cancel during a click**
   (Minor / Quick win): after #286 the two buttons render side-by-side
   on pending rows, but the click handlers were disabling only the
   clicked button. A quick double-click could fire conflicting requests
   on the same row before the reload completes.

   Extracted a small `sameRowActions(btn)` helper that returns every
   row-action button (`.history-approve-btn`, `.history-cancel-btn`)
   in the same `<td>` as the clicked button (falls back to `[btn]`
   when no parent cell exists — test fixtures may not wrap in a
   table). Both Approve and Cancel handlers now disable the full set
   while the API is in flight and re-enable the full set on failure.
   Successful requests trigger a full history reload that re-renders
   the row, so the row-action sibling state doesn't matter on the
   happy path.

2. **coverage_extras_test.go:52 — fail loudly on panic instead of
   swallowing** (Major / Quick win): the prior `recover()` swallow
   could let a panic in any new dispatch branch pass the test
   without ever asserting on `err`, masking real regressions on
   the empty-token path.

   The test now (a) `t.Fatalf`s on any panic from `approvePurchase`
   so the panic path is itself a failure signal, and (b) wires a
   minimal `MockConfigStore` returning a clean
   "execution not found" error from `GetExecutionByID` so the
   dispatch reaches a proper `NewClientError(404)` rather than
   nil-deref'ing on `h.config`. `require.Error(...)` then asserts
   the contract: empty token + no session must NOT silently succeed.

Tests (regression coverage for #1):

* `history-approve-button.test.ts` gains two new cases:
  - "approve click disables BOTH Approve and Cancel for the row
    while in flight (CR pass)" — uses an unresolved promise from
    the api mock to capture the in-flight state and asserts both
    buttons are disabled until the promise resolves.
  - "approve API failure re-enables BOTH Approve and Cancel for
    the row (CR pass)" — asserts both buttons are re-enabled
    after the API rejection path runs.

Verification:
  - `go test ./...` clean (4265 passed / 0 failed / 6 skipped).
  - `npm test` clean (1471 passed / 0 failed).
  - `npx tsc --noEmit` clean.
---
 .../__tests__/history-approve-button.test.ts  | 61 +++++++++++++++++++
 frontend/src/history.ts                       | 37 +++++++++--
 internal/api/coverage_extras_test.go          | 46 ++++++++------
 3 files changed, 120 insertions(+), 24 deletions(-)

diff --git a/frontend/src/__tests__/history-approve-button.test.ts b/frontend/src/__tests__/history-approve-button.test.ts
index 78edde3e..0f433d8a 100644
--- a/frontend/src/__tests__/history-approve-button.test.ts
+++ b/frontend/src/__tests__/history-approve-button.test.ts
@@ -258,4 +258,65 @@ describe('History inline Approve button (issue #286)', () => {
     expect(showToast).toHaveBeenCalledWith(expect.objectContaining({ kind: 'error' }));
     expect(btn?.disabled).toBe(false);
   });
+
+  test('approve click disables BOTH Approve and Cancel for the row while in flight (CR pass)', async () => {
+    // Issue #286 + CR pass on PR #299: Approve and Cancel can render
+    // together. Clicking Approve should disable BOTH buttons until the
+    // request completes — otherwise a quick double-click could fire
+    // a conflicting Cancel on the same row before the reload runs.
+    (getCurrentUser as jest.Mock).mockReturnValue(REG_USER);
+    (confirmDialog as jest.Mock).mockResolvedValue(true);
+    let resolveApprove: (() => void) | undefined;
+    (api.approvePurchase as jest.Mock).mockReturnValue(
+      new Promise<void>((r) => { resolveApprove = r; }),
+    );
+    (api.getHistory as jest.Mock).mockResolvedValue({
+      summary: {},
+      purchases: [makeRow({ purchase_id: 'exec-1', created_by_user_id: REG_USER.id })],
+    });
+
+    await loadHistory();
+    const approveBtn = document.querySelector<HTMLButtonElement>('.history-approve-btn');
+    const cancelBtn = document.querySelector<HTMLButtonElement>('.history-cancel-btn');
+    expect(approveBtn).not.toBeNull();
+    expect(cancelBtn).not.toBeNull();
+    expect(approveBtn?.disabled).toBe(false);
+    expect(cancelBtn?.disabled).toBe(false);
+
+    approveBtn?.click();
+    // Let the click handler reach the disable step (microtasks for the
+    // confirmDialog await chain).
+    await new Promise((r) => setTimeout(r, 0));
+    await new Promise((r) => setTimeout(r, 0));
+
+    // While the API call is unresolved, BOTH buttons are disabled.
+    expect(approveBtn?.disabled).toBe(true);
+    expect(cancelBtn?.disabled).toBe(true);
+
+    // Resolve the in-flight promise + let the success path complete.
+    resolveApprove?.();
+    await new Promise((r) => setTimeout(r, 10));
+  });
+
+  test('approve API failure re-enables BOTH Approve and Cancel for the row (CR pass)', async () => {
+    (getCurrentUser as jest.Mock).mockReturnValue(REG_USER);
+    (confirmDialog as jest.Mock).mockResolvedValue(true);
+    (api.approvePurchase as jest.Mock).mockRejectedValue(new Error('boom'));
+    (api.getHistory as jest.Mock).mockResolvedValue({
+      summary: {},
+      purchases: [makeRow({ purchase_id: 'exec-1', created_by_user_id: REG_USER.id })],
+    });
+    console.error = jest.fn();
+
+    await loadHistory();
+    const approveBtn = document.querySelector<HTMLButtonElement>('.history-approve-btn');
+    const cancelBtn = document.querySelector<HTMLButtonElement>('.history-cancel-btn');
+    approveBtn?.click();
+    await new Promise((r) => setTimeout(r, 10));
+
+    // After the API rejects, BOTH buttons must be re-enabled.
+    expect(approveBtn?.disabled).toBe(false);
+    expect(cancelBtn?.disabled).toBe(false);
+    expect(showToast).toHaveBeenCalledWith(expect.objectContaining({ kind: 'error' }));
+  });
 });
diff --git a/frontend/src/history.ts b/frontend/src/history.ts
index feca1a1b..9bfbb1ad 100644
--- a/frontend/src/history.ts
+++ b/frontend/src/history.ts
@@ -395,6 +395,24 @@ function shortExecID(id: string): string {
   return (id || '').replace(/^urn:.*?:/, '').slice(0, 8);
 }
 
+// sameRowActions returns the set of row-action buttons (Approve, Cancel,
+// future siblings) in the same table cell as `btn`. Used by the Approve
+// and Cancel click handlers to disable BOTH actions for the in-flight
+// row while the API request is pending — issue #286 + CR pass on
+// PR #299: clicking Approve disabled only Approve, leaving the
+// adjacent Cancel button live; a quick double-click could fire
+// conflicting requests on the same row before the reload completes.
+//
+// Falls back to `[btn]` when the button has no parent <td> (test
+// fixtures may render buttons without a wrapping cell).
+function sameRowActions(btn: HTMLButtonElement): HTMLButtonElement[] {
+  const cell = btn.closest('td') || btn.parentElement;
+  if (!cell) return [btn];
+  return Array.from(
+    cell.querySelectorAll<HTMLButtonElement>('.history-approve-btn, .history-cancel-btn'),
+  );
+}
+
 // renderActionCell returns the HTML for the Plan / action column on a
 // single history row. The column doubles as the per-row action surface
 // because pending / failed rows rarely have a meaningful plan_name (the
@@ -592,14 +610,22 @@ function renderHistoryList(purchases: HistoryPurchase[]): void {
         destructive: false,
       });
       if (!ok) return;
-      btn.disabled = true;
+      // Issue #286 + CR pass: Approve and Cancel can render together on
+      // the same row, so disabling only the clicked button leaves the
+      // sibling clickable while we await the API. Disable BOTH on
+      // either click and re-enable both on failure — a successful
+      // approve triggers a full history reload that re-renders the
+      // row, so the row-action sibling state doesn't matter on the
+      // happy path.
+      const rowActions = sameRowActions(btn);
+      rowActions.forEach((b) => { b.disabled = true; });
       try {
         await api.approvePurchase(id);
       } catch (approveError) {
         console.error('Failed to approve pending purchase:', approveError);
         const err = approveError as Error;
         showToast({ message: `Failed to approve: ${err.message || 'unknown error'}`, kind: 'error' });
-        btn.disabled = false;
+        rowActions.forEach((b) => { b.disabled = false; });
         return;
       }
       showToast({ message: 'Purchase approved', kind: 'success', timeout: 5_000 });
@@ -622,14 +648,17 @@ function renderHistoryList(purchases: HistoryPurchase[]): void {
         destructive: true,
       });
       if (!ok) return;
-      btn.disabled = true;
+      // Symmetric with the Approve handler above: disable both row
+      // actions while the API is in flight (CR pass on PR #299).
+      const rowActions = sameRowActions(btn);
+      rowActions.forEach((b) => { b.disabled = true; });
       try {
         await api.cancelPurchase(id);
       } catch (cancelError) {
         console.error('Failed to cancel pending purchase:', cancelError);
         const err = cancelError as Error;
         showToast({ message: `Failed to cancel: ${err.message || 'unknown error'}`, kind: 'error' });
-        btn.disabled = false;
+        rowActions.forEach((b) => { b.disabled = false; });
         return;
       }
       // Cancel succeeded — surface success regardless of whether the
diff --git a/internal/api/coverage_extras_test.go b/internal/api/coverage_extras_test.go
index 840df0fc..16d71b7e 100644
--- a/internal/api/coverage_extras_test.go
+++ b/internal/api/coverage_extras_test.go
@@ -27,29 +27,35 @@ func TestHandler_approvePurchase_InvalidUUID(t *testing.T) {
 
 func TestHandler_approvePurchase_EmptyToken(t *testing.T) {
 	// After issue #286 the empty-token path is no longer a pre-flight 400 —
-	// approvePurchase now dispatches three ways:
-	//   - session-authed (needs a session matching approve-{any,own}),
-	//   - token-authed (legacy email link),
-	//   - session-authed fallback when token == "".
-	// With a zero-value Handler (no config / auth wiring), the call must
-	// fail somewhere — the exact error path depends on which dispatch
-	// branch fires first, but it MUST be a non-nil error so a malformed
-	// caller can't accidentally approve a purchase. Pin that contract
-	// without coupling to a specific branch, since the empty-token case
-	// no longer has a single canonical error message.
-	h := &Handler{}
+	// approvePurchase dispatches three ways (session-authed RBAC,
+	// token-authed legacy, session-authed fallback when token=="").
+	// The contract this test pins: a malformed caller (empty token + no
+	// session) must NOT silently succeed. We require an error AND
+	// we fail loudly on panic — CR pass on PR #299 flagged the prior
+	// `recover()` swallow as a false-green risk: a panic in any new
+	// dispatch branch could pass the test without ever asserting on
+	// `err`.
+	//
+	// Wire a minimal MockConfigStore that returns a clean "execution
+	// not found" error from GetExecutionByID so the dispatch reaches a
+	// proper NewClientError(404) instead of nil-deref'ing on h.config.
+	// (Pre-#286 the empty-token check short-circuited at the very top
+	// of approvePurchase, so a zero-Handler test was sufficient. The
+	// dispatch refactor moved that check after GetExecutionByID; the
+	// test surface adapts in step.)
+	ctx := context.Background()
 	execID := "11111111-1111-1111-1111-111111111111"
+	mockConfig := new(MockConfigStore)
+	mockConfig.On("GetExecutionByID", ctx, execID).Return(nil, errors.New("execution not found"))
+	h := &Handler{config: mockConfig}
+
 	defer func() {
-		// h.config is nil so GetExecutionByID may panic before the
-		// dispatch returns; that's still a non-success outcome and is
-		// what we want to assert (empty token + no session does not
-		// silently succeed).
-		_ = recover()
+		if r := recover(); r != nil {
+			t.Fatalf("approvePurchase should return an error for empty token + zero handler, not panic: %v", r)
+		}
 	}()
-	_, err := h.approvePurchase(context.Background(), nil, execID, "")
-	if err == nil {
-		t.Fatal("approvePurchase with empty token + zero handler must not return nil error")
-	}
+	_, err := h.approvePurchase(ctx, nil, execID, "")
+	require.Error(t, err, "approvePurchase with empty token + zero handler must fail")
 }
 
 func TestHandler_approvePurchase_PurchaseError(t *testing.T) {